[v3,10/15] serial: stm32-usart: Add STM32 USART Driver

From: Maxime Coquelin <mcoquelin.stm32@gmail.com>

This drivers adds support to the STM32 USART controller, which is a
standard serial driver.

Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
---
 drivers/tty/serial/Kconfig       |  17 +
 drivers/tty/serial/Makefile      |   1 +
 drivers/tty/serial/stm32-usart.c | 695 +++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/serial_core.h |   3 +
 4 files changed, 716 insertions(+)
 create mode 100644 drivers/tty/serial/stm32-usart.c

Just a license nit, I'm afraid.

On Thu, 2015-03-12 at 22:55 +0100, Maxime Coquelin wrote:
> --- /dev/null
> +++ b/drivers/tty/serial/stm32-usart.c
> @@ -0,0 +1,695 @@
> +/*
> + * Copyright (C) Maxime Coquelin 2015
> + * Author:  Maxime Coquelin <mcoquelin.stm32@gmail.com>
> + * License terms:  GNU General Public License (GPL), version 2
> + *
> + * Inspired by st-asc.c from STMicroelectronics (c)
> + */

This states the license is GPL v2.

> +MODULE_LICENSE("GPL");

And
    MODULE_LICENSE("GPL v2");

would match that statement.


Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Thu, Mar 12, 2015 at 11:55 PM, Maxime Coquelin
<mcoquelin.stm32@gmail.com> wrote:
> From: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>
> This drivers adds support to the STM32 USART controller, which is a
> standard serial driver.

My comment below.

>
> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> ---
>  drivers/tty/serial/Kconfig       |  17 +
>  drivers/tty/serial/Makefile      |   1 +
>  drivers/tty/serial/stm32-usart.c | 695 +++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/serial_core.h |   3 +
>  4 files changed, 716 insertions(+)
>  create mode 100644 drivers/tty/serial/stm32-usart.c
>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index d2501f0..880cb4f 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1611,6 +1611,23 @@ config SERIAL_SPRD_CONSOLE
>           with "earlycon" on the kernel command line. The console is
>           enabled when early_param is processed.
>
> +config SERIAL_STM32
> +       tristate "STMicroelectronics STM32 serial port support"
> +       select SERIAL_CORE
> +       depends on ARM || COMPILE_TEST
> +       help
> +         This driver is for the on-chip Serial Controller on
> +         STMicroelectronics STM32 MCUs.
> +         USART supports Rx & Tx functionality.
> +         It support all industry standard baud rates.
> +
> +         If unsure, say N.
> +
> +config SERIAL_STM32_CONSOLE
> +       bool "Support for console on STM32"
> +       depends on SERIAL_STM32=y
> +       select SERIAL_CORE_CONSOLE
> +
>  endmenu
>
>  config SERIAL_MCTRL_GPIO
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index 599be4b..67c5023 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -95,6 +95,7 @@ obj-$(CONFIG_SERIAL_FSL_LPUART)       += fsl_lpuart.o
>  obj-$(CONFIG_SERIAL_CONEXANT_DIGICOLOR)        += digicolor-usart.o
>  obj-$(CONFIG_SERIAL_MEN_Z135)  += men_z135_uart.o
>  obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
> +obj-$(CONFIG_SERIAL_STM32)     += stm32-usart.o
>
>  # GPIOLIB helpers for modem control lines
>  obj-$(CONFIG_SERIAL_MCTRL_GPIO)        += serial_mctrl_gpio.o
> diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
> new file mode 100644
> index 0000000..61bb065
> --- /dev/null
> +++ b/drivers/tty/serial/stm32-usart.c
> @@ -0,0 +1,695 @@
> +/*
> + * Copyright (C) Maxime Coquelin 2015
> + * Author:  Maxime Coquelin <mcoquelin.stm32@gmail.com>
> + * License terms:  GNU General Public License (GPL), version 2
> + *
> + * Inspired by st-asc.c from STMicroelectronics (c)
> + */
> +
> +#if defined(CONFIG_SERIAL_STM32_USART_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
> +#define SUPPORT_SYSRQ
> +#endif
> +
> +#include <linux/module.h>
> +#include <linux/serial.h>
> +#include <linux/console.h>
> +#include <linux/sysrq.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/tty.h>
> +#include <linux/tty_flip.h>
> +#include <linux/delay.h>
> +#include <linux/spinlock.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/serial_core.h>
> +#include <linux/clk.h>
> +
> +#define DRIVER_NAME "stm32-usart"
> +
> +/* Register offsets */
> +#define USART_SR               0x00
> +#define USART_DR               0x04
> +#define USART_BRR              0x08
> +#define USART_CR1              0x0c
> +#define USART_CR2              0x10
> +#define USART_CR3              0x14
> +#define USART_GTPR             0x18
> +
> +/* USART_SR */
> +#define USART_SR_PE            BIT(0)
> +#define USART_SR_FE            BIT(1)
> +#define USART_SR_NF            BIT(2)
> +#define USART_SR_ORE           BIT(3)
> +#define USART_SR_IDLE          BIT(4)
> +#define USART_SR_RXNE          BIT(5)
> +#define USART_SR_TC            BIT(6)
> +#define USART_SR_TXE           BIT(7)
> +#define USART_SR_LBD           BIT(8)
> +#define USART_SR_CTS           BIT(9)
> +#define USART_SR_ERR_MASK      (USART_SR_LBD | USART_SR_ORE | \
> +                                USART_SR_FE | USART_SR_PE)
> +/* Dummy bits */
> +#define USART_SR_DUMMY_RX      BIT(16)
> +
> +/* USART_DR */
> +#define USART_DR_MASK          GENMASK(8, 0)
> +
> +/* USART_BRR */
> +#define USART_BRR_DIV_F_MASK   GENMASK(3, 0)
> +#define USART_BRR_DIV_M_MASK   GENMASK(15, 4)
> +#define USART_BRR_DIV_M_SHIFT  4
> +
> +/* USART_CR1 */
> +#define USART_CR1_SBK          BIT(0)
> +#define USART_CR1_RWU          BIT(1)
> +#define USART_CR1_RE           BIT(2)
> +#define USART_CR1_TE           BIT(3)
> +#define USART_CR1_IDLEIE       BIT(4)
> +#define USART_CR1_RXNEIE       BIT(5)
> +#define USART_CR1_TCIE         BIT(6)
> +#define USART_CR1_TXEIE                BIT(7)
> +#define USART_CR1_PEIE         BIT(8)
> +#define USART_CR1_PS           BIT(9)
> +#define USART_CR1_PCE          BIT(10)
> +#define USART_CR1_WAKE         BIT(11)
> +#define USART_CR1_M            BIT(12)
> +#define USART_CR1_UE           BIT(13)
> +#define USART_CR1_OVER8                BIT(15)
> +#define USART_CR1_IE_MASK      GENMASK(8, 4)
> +
> +/* USART_CR2 */
> +#define USART_CR2_ADD_MASK     GENMASK(3, 0)
> +#define USART_CR2_LBDL         BIT(5)
> +#define USART_CR2_LBDIE                BIT(6)
> +#define USART_CR2_LBCL         BIT(8)
> +#define USART_CR2_CPHA         BIT(9)
> +#define USART_CR2_CPOL         BIT(10)
> +#define USART_CR2_CLKEN                BIT(11)
> +#define USART_CR2_STOP_2B      BIT(13)
> +#define USART_CR2_STOP_MASK    GENMASK(13, 12)
> +#define USART_CR2_LINEN                BIT(14)
> +
> +/* USART_CR3 */
> +#define USART_CR3_EIE          BIT(0)
> +#define USART_CR3_IREN         BIT(1)
> +#define USART_CR3_IRLP         BIT(2)
> +#define USART_CR3_HDSEL                BIT(3)
> +#define USART_CR3_NACK         BIT(4)
> +#define USART_CR3_SCEN         BIT(5)
> +#define USART_CR3_DMAR         BIT(6)
> +#define USART_CR3_DMAT         BIT(7)
> +#define USART_CR3_RTSE         BIT(8)
> +#define USART_CR3_CTSE         BIT(9)
> +#define USART_CR3_CTSIE                BIT(10)
> +#define USART_CR3_ONEBIT       BIT(11)
> +
> +/* USART_GTPR */
> +#define USART_GTPR_PSC_MASK    GENMASK(7, 0)
> +#define USART_GTPR_GT_MASK     GENMASK(15, 8)
> +
> +#define DRIVER_NAME "stm32-usart"
> +#define STM32_SERIAL_NAME "ttyS"
> +#define STM32_MAX_PORTS 6
> +
> +struct stm32_port {
> +       struct uart_port port;
> +       struct clk *clk;
> +};
> +
> +static struct stm32_port stm32_ports[STM32_MAX_PORTS];
> +static struct uart_driver stm32_usart_driver;
> +
> +static void stm32_stop_tx(struct uart_port *port);
> +
> +static void stm32_set_bits(struct uart_port *port, u32 reg, u32 bits)
> +{
> +       u32 val;
> +
> +       val = readl_relaxed(port->membase + reg);
> +       val |= bits;
> +       writel_relaxed(val, port->membase + reg);
> +}
> +
> +static void stm32_clr_bits(struct uart_port *port, u32 reg, u32 bits)
> +{
> +       u32 val;
> +
> +       val = readl_relaxed(port->membase + reg);
> +       val &= ~bits;
> +       writel_relaxed(val, port->membase + reg);
> +}
> +
> +static void stm32_receive_chars(struct uart_port *port)
> +{
> +       struct tty_port *tport = &port->state->port;
> +       unsigned long c;
> +       u32 sr;
> +       char flag;
> +
> +       if (port->irq_wake)
> +               pm_wakeup_event(tport->tty->dev, 0);
> +
> +       while ((sr = readl_relaxed(port->membase + USART_SR)) & USART_SR_RXNE) {
> +               sr |= USART_SR_DUMMY_RX;
> +               c = readl_relaxed(port->membase + USART_DR);
> +               flag = TTY_NORMAL;
> +               port->icount.rx++;
> +
> +               if (sr & USART_SR_ERR_MASK) {
> +                       if (sr & USART_SR_LBD) {
> +                               port->icount.brk++;
> +                               if (uart_handle_break(port))
> +                                       continue;
> +                       } else if (sr & USART_SR_ORE) {
> +                               port->icount.overrun++;
> +                       } else if (sr & USART_SR_PE) {
> +                               port->icount.parity++;
> +                       } else if (sr & USART_SR_FE) {
> +                               port->icount.frame++;
> +                       }
> +
> +                       sr &= port->read_status_mask;
> +
> +                       if (sr & USART_SR_LBD)
> +                               flag = TTY_BREAK;
> +                       else if (sr & USART_SR_PE)
> +                               flag = TTY_PARITY;
> +                       else if (sr & USART_SR_FE)
> +                               flag = TTY_FRAME;
> +               }
> +
> +               if (uart_handle_sysrq_char(port, c))
> +                       continue;
> +               uart_insert_char(port, sr, USART_SR_ORE, c, flag);
> +       }
> +
> +       spin_unlock(&port->lock);
> +       tty_flip_buffer_push(tport);
> +       spin_lock(&port->lock);
> +}
> +
> +static void stm32_transmit_chars(struct uart_port *port)
> +{
> +       struct circ_buf *xmit = &port->state->xmit;
> +
> +       if (port->x_char) {
> +               writel_relaxed(port->x_char, port->membase + USART_DR);
> +               port->x_char = 0;
> +               port->icount.tx++;
> +               return;
> +       }
> +
> +       if (uart_tx_stopped(port)) {
> +               stm32_stop_tx(port);
> +               return;
> +       }
> +
> +       if (uart_circ_empty(xmit)) {
> +               stm32_stop_tx(port);
> +               return;
> +       }
> +
> +       writel_relaxed(xmit->buf[xmit->tail], port->membase + USART_DR);
> +       xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> +       port->icount.tx++;
> +
> +       if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> +               uart_write_wakeup(port);
> +
> +       if (uart_circ_empty(xmit))
> +               stm32_stop_tx(port);
> +}
> +
> +static irqreturn_t stm32_interrupt(int irq, void *ptr)
> +{
> +       struct uart_port *port = ptr;
> +       u32 sr;
> +
> +       spin_lock(&port->lock);
> +
> +       sr = readl_relaxed(port->membase + USART_SR);
> +
> +       if (sr & USART_SR_RXNE)
> +               stm32_receive_chars(port);
> +
> +       if (sr & USART_SR_TXE)
> +               stm32_transmit_chars(port);
> +
> +       spin_unlock(&port->lock);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static unsigned int stm32_tx_empty(struct uart_port *port)
> +{
> +       return readl_relaxed(port->membase + USART_SR) & USART_SR_TXE;
> +}
> +
> +static void stm32_set_mctrl(struct uart_port *port, unsigned int mctrl)
> +{
> +       /*
> +        * This routine is used for seting signals of: DTR, DCD, CTS/RTS
> +        * We use USART's hardware for CTS/RTS, so don't need any for that.
> +        * Some boards have DTR and DCD implemented using PIO pins,
> +        * code to do this should be hooked in here.
> +        */
> +}
> +
> +static unsigned int stm32_get_mctrl(struct uart_port *port)
> +{
> +       /*
> +        * This routine is used for geting signals of: DTR, DCD, DSR, RI,
> +        * and CTS/RTS
> +        */
> +       return TIOCM_CAR | TIOCM_DSR | TIOCM_CTS;
> +}
> +
> +/* There are probably characters waiting to be transmitted. */
> +static void stm32_start_tx(struct uart_port *port)
> +{
> +       struct circ_buf *xmit = &port->state->xmit;
> +
> +       if (uart_circ_empty(xmit))
> +               return;
> +
> +       stm32_set_bits(port, USART_CR1, USART_CR1_TXEIE | USART_CR1_TE);
> +}
> +
> +/* Transmit stop */
> +static void stm32_stop_tx(struct uart_port *port)
> +{
> +       stm32_clr_bits(port, USART_CR1, USART_CR1_TXEIE);
> +}
> +
> +/* Receive stop */
> +static void stm32_stop_rx(struct uart_port *port)
> +{
> +       stm32_clr_bits(port, USART_CR1, USART_CR1_RXNEIE);
> +}
> +
> +/* Handle breaks - ignored by us */
> +static void stm32_break_ctl(struct uart_port *port, int break_state)
> +{
> +       /* Nothing here yet .. */
> +}
> +
> +static int stm32_startup(struct uart_port *port)
> +{
> +       const char *name = to_platform_device(port->dev)->name;
> +       u32 val;
> +
> +       if (request_irq(port->irq, stm32_interrupt, IRQF_NO_SUSPEND,
> +                               name, port)) {
> +               dev_err(port->dev, "cannot allocate irq %d\n", port->irq);
> +               return -ENODEV;
> +       }
> +
> +       val = USART_CR1_RXNEIE | USART_CR1_TE | USART_CR1_RE;
> +       stm32_set_bits(port, USART_CR1, val);
> +
> +       return 0;
> +}
> +
> +static void stm32_shutdown(struct uart_port *port)
> +{
> +       u32 val;
> +
> +       val = USART_CR1_TXEIE | USART_CR1_RXNEIE | USART_CR1_TE | USART_CR1_RE;
> +       stm32_set_bits(port, USART_CR1, val);
> +
> +       free_irq(port->irq, port);
> +}
> +
> +static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
> +                           struct ktermios *old)
> +{
> +       unsigned int baud;
> +       u32 usardiv, mantissa, fraction;
> +       tcflag_t cflag;
> +       u32 cr1, cr2, cr3;
> +       unsigned long flags;
> +
> +       baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk / 16);
> +       cflag = termios->c_cflag;
> +
> +       spin_lock_irqsave(&port->lock, flags);
> +
> +       /* Stop serial port and reset value */
> +       writel_relaxed(0, port->membase + USART_CR1);
> +
> +       cr1 = USART_CR1_TE | USART_CR1_RE | USART_CR1_UE | USART_CR1_RXNEIE;
> +
> +       if (cflag & CSTOPB)
> +               cr2 = USART_CR2_STOP_2B;
> +
> +       if (cflag & PARENB) {
> +               cr1 |= USART_CR1_PCE;
> +               if ((cflag & CSIZE) == CS8)
> +                       cr1 |= USART_CR1_M;
> +       }
> +
> +       if (cflag & PARODD)
> +               cr1 |= USART_CR1_PS;
> +
> +       if (cflag & CRTSCTS)
> +               cr3 = USART_CR3_RTSE | USART_CR3_CTSE;
> +
> +       usardiv = (port->uartclk * 25) / (baud * 4);
> +       mantissa = (usardiv / 100) << USART_BRR_DIV_M_SHIFT;
> +       fraction = DIV_ROUND_CLOSEST((usardiv % 100) * 16, 100);
> +       if (fraction & ~USART_BRR_DIV_F_MASK) {
> +               fraction = 0;
> +               mantissa += (1 << USART_BRR_DIV_M_SHIFT);
> +       }

So, it's a fractional divider. right? Could it be then fractional
divider clock in this first place (see
drivers/clk/clk-fractional-divider.c)?

> +
> +       writel_relaxed(mantissa | fraction, port->membase + USART_BRR);
> +
> +       uart_update_timeout(port, cflag, baud);
> +
> +       port->read_status_mask = USART_SR_ORE;
> +       if (termios->c_iflag & INPCK)
> +               port->read_status_mask |= USART_SR_PE | USART_SR_FE;
> +       if (termios->c_iflag & (IGNBRK | BRKINT | PARMRK))
> +               port->read_status_mask |= USART_SR_LBD;
> +
> +       /* Characters to ignore */
> +       port->ignore_status_mask = 0;
> +       if (termios->c_iflag & IGNPAR)
> +               port->ignore_status_mask = USART_SR_PE | USART_SR_FE;
> +       if (termios->c_iflag & IGNBRK) {
> +               port->ignore_status_mask |= USART_SR_LBD;
> +               /*
> +                * If we're ignoring parity and break indicators,
> +                * ignore overruns too (for real raw support).
> +                */
> +               if (termios->c_iflag & IGNPAR)
> +                       port->ignore_status_mask |= USART_SR_ORE;
> +       }
> +
> +       /*
> +        * Ignore all characters if CREAD is not set.
> +        */
> +       if ((termios->c_cflag & CREAD) == 0)
> +               port->ignore_status_mask |= USART_SR_DUMMY_RX;
> +
> +       writel_relaxed(cr3, port->membase + USART_CR3);
> +       writel_relaxed(cr2, port->membase + USART_CR2);
> +       writel_relaxed(cr1, port->membase + USART_CR1);
> +
> +       spin_unlock_irqrestore(&port->lock, flags);
> +}
> +
> +static const char *stm32_type(struct uart_port *port)
> +{
> +       return (port->type == PORT_STM32) ? DRIVER_NAME : NULL;
> +}
> +
> +static void stm32_release_port(struct uart_port *port)
> +{
> +}
> +
> +static int stm32_request_port(struct uart_port *port)
> +{
> +       return 0;
> +}
> +
> +static void stm32_config_port(struct uart_port *port, int flags)
> +{
> +       if ((flags & UART_CONFIG_TYPE))
> +               port->type = PORT_STM32;
> +}
> +
> +static int
> +stm32_verify_port(struct uart_port *port, struct serial_struct *ser)
> +{
> +       /* No user changeable parameters */
> +       return -EINVAL;
> +}
> +
> +static void stm32_pm(struct uart_port *port, unsigned int state,
> +               unsigned int oldstate)
> +{
> +       struct stm32_port *stm32port = container_of(port,
> +                       struct stm32_port, port);
> +       unsigned long flags = 0;
> +
> +       switch (state) {
> +       case UART_PM_STATE_ON:
> +               clk_prepare_enable(stm32port->clk);
> +               break;
> +       case UART_PM_STATE_OFF:
> +               spin_lock_irqsave(&port->lock, flags);
> +               stm32_clr_bits(port, USART_CR1, USART_CR1_UE);
> +               spin_unlock_irqrestore(&port->lock, flags);
> +               clk_disable_unprepare(stm32port->clk);
> +               break;
> +       }
> +}
> +
> +static struct uart_ops stm32_uart_ops = {
> +       .tx_empty       = stm32_tx_empty,
> +       .set_mctrl      = stm32_set_mctrl,
> +       .get_mctrl      = stm32_get_mctrl,
> +       .start_tx       = stm32_start_tx,
> +       .stop_tx        = stm32_stop_tx,
> +       .stop_rx        = stm32_stop_rx,
> +       .break_ctl      = stm32_break_ctl,
> +       .startup        = stm32_startup,
> +       .shutdown       = stm32_shutdown,
> +       .set_termios    = stm32_set_termios,
> +       .type           = stm32_type,
> +       .release_port   = stm32_release_port,
> +       .request_port   = stm32_request_port,
> +       .config_port    = stm32_config_port,
> +       .verify_port    = stm32_verify_port,
> +       .pm             = stm32_pm,
> +};
> +
> +static int stm32_init_port(struct stm32_port *stm32port,
> +                         struct platform_device *pdev)
> +{
> +       struct uart_port *port = &stm32port->port;
> +       struct resource *res;
> +
> +       port->iotype    = UPIO_MEM;
> +       port->flags     = UPF_BOOT_AUTOCONF;
> +       port->ops       = &stm32_uart_ops;
> +       port->dev       = &pdev->dev;
> +       port->irq       = platform_get_irq(pdev, 0);
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       port->membase = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(port->membase))
> +               return PTR_ERR(port->membase);
> +       port->mapbase = res->start;
> +
> +       spin_lock_init(&port->lock);
> +
> +       stm32port->clk = devm_clk_get(&pdev->dev, NULL);
> +
> +       if (WARN_ON(IS_ERR(stm32port->clk)))
> +               return -EINVAL;
> +
> +       /* ensure that clk rate is correct by enabling the clk */
> +       clk_prepare_enable(stm32port->clk);
> +       stm32port->port.uartclk = clk_get_rate(stm32port->clk);
> +       WARN_ON(stm32port->port.uartclk == 0);
> +       clk_disable_unprepare(stm32port->clk);
> +
> +       return 0;
> +}
> +
> +static struct stm32_port *stm32_of_get_stm32_port(struct platform_device *pdev)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       int id;
> +
> +       if (!np)
> +               return NULL;
> +
> +       id = of_alias_get_id(np, STM32_SERIAL_NAME);
> +
> +       if (id < 0)
> +               id = 0;
> +
> +       if (WARN_ON(id >= STM32_MAX_PORTS))
> +               return NULL;
> +
> +       stm32_ports[id].port.line = id;
> +       return &stm32_ports[id];
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id stm32_match[] = {
> +       { .compatible = "st,stm32-usart", },
> +       {},
> +};
> +
> +MODULE_DEVICE_TABLE(of, stm32_match);
> +#endif
> +
> +static int stm32_serial_probe(struct platform_device *pdev)
> +{
> +       int ret;
> +       struct stm32_port *stm32port;
> +
> +       stm32port = stm32_of_get_stm32_port(pdev);
> +       if (!stm32port)
> +               return -ENODEV;
> +
> +       ret = stm32_init_port(stm32port, pdev);
> +       if (ret)
> +               return ret;
> +
> +       ret = uart_add_one_port(&stm32_usart_driver, &stm32port->port);
> +       if (ret)
> +               return ret;
> +
> +       platform_set_drvdata(pdev, &stm32port->port);
> +
> +       return 0;
> +}
> +
> +static int stm32_serial_remove(struct platform_device *pdev)
> +{
> +       struct uart_port *port = platform_get_drvdata(pdev);
> +
> +       return uart_remove_one_port(&stm32_usart_driver, port);
> +}
> +
> +
> +#ifdef CONFIG_SERIAL_STM32_CONSOLE
> +static void stm32_console_putchar(struct uart_port *port, int ch)
> +{
> +       while (!(readl_relaxed(port->membase + USART_SR) & USART_SR_TXE))
> +               cpu_relax();
> +
> +       writel_relaxed(ch, port->membase + USART_DR);
> +}
> +
> +static void stm32_console_write(struct console *co, const char *s, unsigned cnt)
> +{
> +       struct uart_port *port = &stm32_ports[co->index].port;
> +       unsigned long flags;
> +       u32 old_cr1, new_cr1;
> +       int locked = 1;
> +
> +       if (oops_in_progress) {
> +               locked = spin_trylock_irqsave(&port->lock, flags);
> +       } else {
> +               locked = 1;
> +               spin_lock_irqsave(&port->lock, flags);
> +       }
> +
> +       /* Save and disable interrupts */
> +       old_cr1 = readl_relaxed(port->membase + USART_CR1);
> +       new_cr1 = old_cr1 & ~USART_CR1_IE_MASK;
> +       writel_relaxed(new_cr1, port->membase + USART_CR1);
> +
> +       uart_console_write(port, s, cnt, stm32_console_putchar);
> +
> +       /* Restore interrupt state */
> +       writel_relaxed(old_cr1, port->membase + USART_CR1);
> +
> +       if (locked)
> +               spin_unlock_irqrestore(&port->lock, flags);
> +}
> +
> +static int stm32_console_setup(struct console *co, char *options)
> +{
> +       struct stm32_port *stm32port;
> +       int baud = 9600;
> +       int bits = 8;
> +       int parity = 'n';
> +       int flow = 'n';
> +
> +       if (co->index >= STM32_MAX_PORTS)
> +               return -ENODEV;
> +
> +       stm32port = &stm32_ports[co->index];
> +
> +       /*
> +        * This driver does not support early console initialization
> +        * (use ARM early printk support instead), so we only expect
> +        * this to be called during the uart port registration when the
> +        * driver gets probed and the port should be mapped at that point.
> +        */
> +       if (stm32port->port.mapbase == 0 || stm32port->port.membase == NULL)
> +               return -ENXIO;
> +
> +       if (options)
> +               uart_parse_options(options, &baud, &parity, &bits, &flow);
> +
> +       return uart_set_options(&stm32port->port, co, baud, parity, bits, flow);
> +}
> +
> +static struct console stm32_console = {
> +       .name           = STM32_SERIAL_NAME,
> +       .device         = uart_console_device,
> +       .write          = stm32_console_write,
> +       .setup          = stm32_console_setup,
> +       .flags          = CON_PRINTBUFFER,
> +       .index          = -1,
> +       .data           = &stm32_usart_driver,
> +};
> +
> +#define STM32_SERIAL_CONSOLE (&stm32_console)
> +
> +#else
> +#define STM32_SERIAL_CONSOLE NULL
> +#endif /* CONFIG_SERIAL_STM32_CONSOLE */
> +
> +static struct uart_driver stm32_usart_driver = {
> +       .owner          = THIS_MODULE,
> +       .driver_name    = DRIVER_NAME,
> +       .dev_name       = STM32_SERIAL_NAME,
> +       .major          = 0,
> +       .minor          = 0,
> +       .nr             = STM32_MAX_PORTS,
> +       .cons           = STM32_SERIAL_CONSOLE,
> +};
> +
> +static struct platform_driver stm32_serial_driver = {
> +       .probe          = stm32_serial_probe,
> +       .remove         = stm32_serial_remove,
> +       .driver = {
> +               .name   = DRIVER_NAME,
> +               .owner  = THIS_MODULE,
> +               .of_match_table = of_match_ptr(stm32_match),
> +       },
> +};
> +
> +static int __init usart_init(void)
> +{
> +       int ret;
> +       static char banner[] __initdata =
> +               KERN_INFO "STM32 USART driver initialized\n";
> +
> +       printk(banner);
> +
> +       ret = uart_register_driver(&stm32_usart_driver);
> +       if (ret)
> +               return ret;
> +
> +       ret = platform_driver_register(&stm32_serial_driver);
> +       if (ret)
> +               uart_unregister_driver(&stm32_usart_driver);
> +
> +       return ret;
> +}
> +
> +static void __exit usart_exit(void)
> +{
> +       platform_driver_unregister(&stm32_serial_driver);
> +       uart_unregister_driver(&stm32_usart_driver);
> +}
> +
> +module_init(usart_init);
> +module_exit(usart_exit);
> +
> +MODULE_ALIAS("platform:" DRIVER_NAME);
> +MODULE_DESCRIPTION("STMicroelectronics STM32 serial port driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> index b212281..e22dee5 100644
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -258,4 +258,7 @@
>  /* Cris v10 / v32 SoC */
>  #define PORT_CRIS      112
>
> +/* STM32 USART */
> +#define PORT_STM32     110
> +
>  #endif /* _UAPILINUX_SERIAL_CORE_H */
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

2015-03-13 15:19 GMT+01:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
> On Thu, Mar 12, 2015 at 11:55 PM, Maxime Coquelin
> <mcoquelin.stm32@gmail.com> wrote:
>> From: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>
>> This drivers adds support to the STM32 USART controller, which is a
>> standard serial driver.
>
> My comment below.
>
>>
>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>> ---
>>  drivers/tty/serial/Kconfig       |  17 +
>>  drivers/tty/serial/Makefile      |   1 +
>>  drivers/tty/serial/stm32-usart.c | 695 +++++++++++++++++++++++++++++++++++++++
>>  include/uapi/linux/serial_core.h |   3 +
>>  4 files changed, 716 insertions(+)
>>  create mode 100644 drivers/tty/serial/stm32-usart.c
>>
>> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
>> index d2501f0..880cb4f 100644
>> --- a/drivers/tty/serial/Kconfig
>> +++ b/drivers/tty/serial/Kconfig
>> @@ -1611,6 +1611,23 @@ config SERIAL_SPRD_CONSOLE
>>           with "earlycon" on the kernel command line. The console is
>>           enabled when early_param is processed.
>>
>> +config SERIAL_STM32
>> +       tristate "STMicroelectronics STM32 serial port support"
>> +       select SERIAL_CORE
>> +       depends on ARM || COMPILE_TEST
>> +       help
>> +         This driver is for the on-chip Serial Controller on
>> +         STMicroelectronics STM32 MCUs.
>> +         USART supports Rx & Tx functionality.
>> +         It support all industry standard baud rates.
>> +
>> +         If unsure, say N.
>> +
>> +config SERIAL_STM32_CONSOLE
>> +       bool "Support for console on STM32"
>> +       depends on SERIAL_STM32=y
>> +       select SERIAL_CORE_CONSOLE
>> +
>>  endmenu
>>
>>  config SERIAL_MCTRL_GPIO
>> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
>> index 599be4b..67c5023 100644
>> --- a/drivers/tty/serial/Makefile
>> +++ b/drivers/tty/serial/Makefile
>> @@ -95,6 +95,7 @@ obj-$(CONFIG_SERIAL_FSL_LPUART)       += fsl_lpuart.o
>>  obj-$(CONFIG_SERIAL_CONEXANT_DIGICOLOR)        += digicolor-usart.o
>>  obj-$(CONFIG_SERIAL_MEN_Z135)  += men_z135_uart.o
>>  obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
>> +obj-$(CONFIG_SERIAL_STM32)     += stm32-usart.o
>>
>>  # GPIOLIB helpers for modem control lines
>>  obj-$(CONFIG_SERIAL_MCTRL_GPIO)        += serial_mctrl_gpio.o
>> diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
>> new file mode 100644
>> index 0000000..61bb065
>> --- /dev/null
>> +++ b/drivers/tty/serial/stm32-usart.c
>> @@ -0,0 +1,695 @@

<snip>

>> +
>> +static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
>> +                           struct ktermios *old)
>> +{
>> +       unsigned int baud;
>> +       u32 usardiv, mantissa, fraction;
>> +       tcflag_t cflag;
>> +       u32 cr1, cr2, cr3;
>> +       unsigned long flags;
>> +
>> +       baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk / 16);
>> +       cflag = termios->c_cflag;
>> +
>> +       spin_lock_irqsave(&port->lock, flags);
>> +
>> +       /* Stop serial port and reset value */
>> +       writel_relaxed(0, port->membase + USART_CR1);
>> +
>> +       cr1 = USART_CR1_TE | USART_CR1_RE | USART_CR1_UE | USART_CR1_RXNEIE;
>> +
>> +       if (cflag & CSTOPB)
>> +               cr2 = USART_CR2_STOP_2B;
>> +
>> +       if (cflag & PARENB) {
>> +               cr1 |= USART_CR1_PCE;
>> +               if ((cflag & CSIZE) == CS8)
>> +                       cr1 |= USART_CR1_M;
>> +       }
>> +
>> +       if (cflag & PARODD)
>> +               cr1 |= USART_CR1_PS;
>> +
>> +       if (cflag & CRTSCTS)
>> +               cr3 = USART_CR3_RTSE | USART_CR3_CTSE;
>> +
>> +       usardiv = (port->uartclk * 25) / (baud * 4);
>> +       mantissa = (usardiv / 100) << USART_BRR_DIV_M_SHIFT;
>> +       fraction = DIV_ROUND_CLOSEST((usardiv % 100) * 16, 100);
>> +       if (fraction & ~USART_BRR_DIV_F_MASK) {
>> +               fraction = 0;
>> +               mantissa += (1 << USART_BRR_DIV_M_SHIFT);
>> +       }
>
> So, it's a fractional divider. right? Could it be then fractional
> divider clock in this first place (see
> drivers/clk/clk-fractional-divider.c)?
>

I'm not sure it makes sense to represent this baudrate divider within
the UART IP as a clock.
What would be the gain?

Kind regards,
Maxime
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

2015-03-13 10:41 GMT+01:00 Paul Bolle <pebolle@tiscali.nl>:
> Just a license nit, I'm afraid.
Not a problem, it is not the last round anyway.

>
> On Thu, 2015-03-12 at 22:55 +0100, Maxime Coquelin wrote:
>> --- /dev/null
>> +++ b/drivers/tty/serial/stm32-usart.c
>> @@ -0,0 +1,695 @@
>> +/*
>> + * Copyright (C) Maxime Coquelin 2015
>> + * Author:  Maxime Coquelin <mcoquelin.stm32@gmail.com>
>> + * License terms:  GNU General Public License (GPL), version 2
>> + *
>> + * Inspired by st-asc.c from STMicroelectronics (c)
>> + */
>
> This states the license is GPL v2.
>
>> +MODULE_LICENSE("GPL");
>
> And
>     MODULE_LICENSE("GPL v2");
>
> would match that statement.

Right, it will be fixed in the v4.

Thanks,
Maxime
>
>
> Paul Bolle
>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, Mar 17, 2015 at 7:32 PM, Maxime Coquelin
<mcoquelin.stm32@gmail.com> wrote:
> 2015-03-13 15:19 GMT+01:00 Andy Shevchenko <andy.shevchenko@gmail.com>:

>>> +static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
>>> +                           struct ktermios *old)
>>> +{
>>> +       unsigned int baud;
>>> +       u32 usardiv, mantissa, fraction;
>>> +       tcflag_t cflag;
>>> +       u32 cr1, cr2, cr3;
>>> +       unsigned long flags;
>>> +
>>> +       baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk / 16);
>>> +       cflag = termios->c_cflag;
>>> +
>>> +       spin_lock_irqsave(&port->lock, flags);
>>> +
>>> +       /* Stop serial port and reset value */
>>> +       writel_relaxed(0, port->membase + USART_CR1);
>>> +
>>> +       cr1 = USART_CR1_TE | USART_CR1_RE | USART_CR1_UE | USART_CR1_RXNEIE;
>>> +
>>> +       if (cflag & CSTOPB)
>>> +               cr2 = USART_CR2_STOP_2B;
>>> +
>>> +       if (cflag & PARENB) {
>>> +               cr1 |= USART_CR1_PCE;
>>> +               if ((cflag & CSIZE) == CS8)
>>> +                       cr1 |= USART_CR1_M;
>>> +       }
>>> +
>>> +       if (cflag & PARODD)
>>> +               cr1 |= USART_CR1_PS;
>>> +
>>> +       if (cflag & CRTSCTS)
>>> +               cr3 = USART_CR3_RTSE | USART_CR3_CTSE;
>>> +
>>> +       usardiv = (port->uartclk * 25) / (baud * 4);
>>> +       mantissa = (usardiv / 100) << USART_BRR_DIV_M_SHIFT;
>>> +       fraction = DIV_ROUND_CLOSEST((usardiv % 100) * 16, 100);
>>> +       if (fraction & ~USART_BRR_DIV_F_MASK) {
>>> +               fraction = 0;
>>> +               mantissa += (1 << USART_BRR_DIV_M_SHIFT);
>>> +       }
>>
>> So, it's a fractional divider. right? Could it be then fractional
>> divider clock in this first place (see
>> drivers/clk/clk-fractional-divider.c)?
>>
>
> I'm not sure it makes sense to represent this baudrate divider within
> the UART IP as a clock.

You have it already. I mean it should be fractional divider clock.

stm32port->clk = devm_clk_get(&pdev->dev, NULL);
+
+       if (WARN_ON(IS_ERR(stm32port->clk)))
+               return -EINVAL;
+
+       /* ensure that clk rate is correct by enabling the clk */
+       clk_prepare_enable(stm32port->clk);
+       stm32port->port.uartclk = clk_get_rate(stm32port->clk);

> What would be the gain?

Remove custom implementation of the calculations. It will be just one
line to refresh the baud rate. I think we have a lot of duplicates
here and there in the kernel. It would be nice not to bring another
one.

2015-03-17 18:56 GMT+01:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
> On Tue, Mar 17, 2015 at 7:32 PM, Maxime Coquelin
> <mcoquelin.stm32@gmail.com> wrote:
>> 2015-03-13 15:19 GMT+01:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
>
>>>> +static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
>>>> +                           struct ktermios *old)
>>>> +{
>>>> +       unsigned int baud;
>>>> +       u32 usardiv, mantissa, fraction;
>>>> +       tcflag_t cflag;
>>>> +       u32 cr1, cr2, cr3;
>>>> +       unsigned long flags;
>>>> +
>>>> +       baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk / 16);
>>>> +       cflag = termios->c_cflag;
>>>> +
>>>> +       spin_lock_irqsave(&port->lock, flags);
>>>> +
>>>> +       /* Stop serial port and reset value */
>>>> +       writel_relaxed(0, port->membase + USART_CR1);
>>>> +
>>>> +       cr1 = USART_CR1_TE | USART_CR1_RE | USART_CR1_UE | USART_CR1_RXNEIE;
>>>> +
>>>> +       if (cflag & CSTOPB)
>>>> +               cr2 = USART_CR2_STOP_2B;
>>>> +
>>>> +       if (cflag & PARENB) {
>>>> +               cr1 |= USART_CR1_PCE;
>>>> +               if ((cflag & CSIZE) == CS8)
>>>> +                       cr1 |= USART_CR1_M;
>>>> +       }
>>>> +
>>>> +       if (cflag & PARODD)
>>>> +               cr1 |= USART_CR1_PS;
>>>> +
>>>> +       if (cflag & CRTSCTS)
>>>> +               cr3 = USART_CR3_RTSE | USART_CR3_CTSE;
>>>> +
>>>> +       usardiv = (port->uartclk * 25) / (baud * 4);
>>>> +       mantissa = (usardiv / 100) << USART_BRR_DIV_M_SHIFT;
>>>> +       fraction = DIV_ROUND_CLOSEST((usardiv % 100) * 16, 100);
>>>> +       if (fraction & ~USART_BRR_DIV_F_MASK) {
>>>> +               fraction = 0;
>>>> +               mantissa += (1 << USART_BRR_DIV_M_SHIFT);
>>>> +       }
>>>
>>> So, it's a fractional divider. right? Could it be then fractional
>>> divider clock in this first place (see
>>> drivers/clk/clk-fractional-divider.c)?
>>>
>>
>> I'm not sure it makes sense to represent this baudrate divider within
>> the UART IP as a clock.
>
> You have it already. I mean it should be fractional divider clock.
>
> stm32port->clk = devm_clk_get(&pdev->dev, NULL);
> +
> +       if (WARN_ON(IS_ERR(stm32port->clk)))
> +               return -EINVAL;
> +
> +       /* ensure that clk rate is correct by enabling the clk */
> +       clk_prepare_enable(stm32port->clk);
> +       stm32port->port.uartclk = clk_get_rate(stm32port->clk);

No I don't have it already.
The clock I get here is coming from outside the IP, so this driver is
a clock consumer.
If I follow your advice, this driver will become both the clock
provider and the only clock consumer of this clock.
I think this is something that should be avoided.

Also, it would require to add another clock in between (a by-16 fixed
divider), because the formula is:
baud = fck / (16 * usart_div)

Finally, representing this as a clock is not really matching the
reality, because we are generating a baud rate, not a frequency.

Really, I would prefer keeping this fractional divider as it is
implemented today.

Kind regards,
Maxime

>
>> What would be the gain?
>
> Remove custom implementation of the calculations. It will be just one
> line to refresh the baud rate. I think we have a lot of duplicates
> here and there in the kernel. It would be nice not to bring another
> one.
>
> --
> With Best Regards,
> Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 03/19/2015 09:55 AM, Maxime Coquelin wrote:
>>>>> +static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
>>>>> +                           struct ktermios *old)
[...]
>>>>> +       usardiv = (port->uartclk * 25) / (baud * 4);
>>>>> +       mantissa = (usardiv / 100) << USART_BRR_DIV_M_SHIFT;
>>>>> +       fraction = DIV_ROUND_CLOSEST((usardiv % 100) * 16, 100);
>>>>> +       if (fraction & ~USART_BRR_DIV_F_MASK) {
>>>>> +               fraction = 0;
>>>>> +               mantissa += (1 << USART_BRR_DIV_M_SHIFT);
>>>>> +       }
[...]
> Really, I would prefer keeping this fractional divider as it is
> implemented today.

You have to admit that's basically an unintelligible mess;
how would anyone ever be able to refactor and replace that with a
common divider implementation?

At the very least, please comment on the formula and format.

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

2015-03-19 15:58 GMT+01:00 Peter Hurley <peter@hurleysoftware.com>:
> On 03/19/2015 09:55 AM, Maxime Coquelin wrote:
>>>>>> +static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
>>>>>> +                           struct ktermios *old)
> [...]
>>>>>> +       usardiv = (port->uartclk * 25) / (baud * 4);
>>>>>> +       mantissa = (usardiv / 100) << USART_BRR_DIV_M_SHIFT;
>>>>>> +       fraction = DIV_ROUND_CLOSEST((usardiv % 100) * 16, 100);
>>>>>> +       if (fraction & ~USART_BRR_DIV_F_MASK) {
>>>>>> +               fraction = 0;
>>>>>> +               mantissa += (1 << USART_BRR_DIV_M_SHIFT);
>>>>>> +       }
> [...]
>> Really, I would prefer keeping this fractional divider as it is
>> implemented today.
>
> You have to admit that's basically an unintelligible mess;
> how would anyone ever be able to refactor and replace that with a
> common divider implementation?
>
> At the very least, please comment on the formula and format.

Ok, I will refactor the implementation, and comment it.

Regards,
Maxime

>
> Regards,
> Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Peter,

2015-03-19 18:35 GMT+01:00 Maxime Coquelin <mcoquelin.stm32@gmail.com>:
> 2015-03-19 15:58 GMT+01:00 Peter Hurley <peter@hurleysoftware.com>:
>> On 03/19/2015 09:55 AM, Maxime Coquelin wrote:
>>>>>>> +static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
>>>>>>> +                           struct ktermios *old)
>> [...]
>>>>>>> +       usardiv = (port->uartclk * 25) / (baud * 4);
>>>>>>> +       mantissa = (usardiv / 100) << USART_BRR_DIV_M_SHIFT;
>>>>>>> +       fraction = DIV_ROUND_CLOSEST((usardiv % 100) * 16, 100);
>>>>>>> +       if (fraction & ~USART_BRR_DIV_F_MASK) {
>>>>>>> +               fraction = 0;
>>>>>>> +               mantissa += (1 << USART_BRR_DIV_M_SHIFT);
>>>>>>> +       }
>> [...]
>>> Really, I would prefer keeping this fractional divider as it is
>>> implemented today.
>>
>> You have to admit that's basically an unintelligible mess;
>> how would anyone ever be able to refactor and replace that with a
>> common divider implementation?
>>
>> At the very least, please comment on the formula and format.
>
> Ok, I will refactor the implementation, and comment it.

The implementation was indeed a mess.
I found some time to refactor the code, and also added support for 8
times oversampling (16 by default).
It will allow to achieve higher speeds, with the side effect of being
less tolerant to clock deviations.

What do you think about the code below?


    usartdiv = DIV_ROUND_CLOSEST(port->uartclk, baud);

    /*
     * The USART supports 16 or 8 times oversampling.
     * By default we prefer 16 times oversampling, so that the receiver
     * has a better tolerance to clock deviations.
     * 8 times oversampling is only used to achieve higher speeds.
     */
    if (usartdiv < 16) {
        oversampling = 8;
        stm32_set_bits(port, USART_CR1, USART_CR1_OVER8);
    } else {
        oversampling = 16;
        stm32_clr_bits(port, USART_CR1, USART_CR1_OVER8);
    }

    mantissa = (usartdiv / oversampling) << USART_BRR_DIV_M_SHIFT;
    fraction = usartdiv % oversampling;
    writel_relaxed(mantissa | fraction, port->membase + USART_BRR)

Thanks,
Maxime
>
> Regards,
> Maxime
>
>>
>> Regards,
>> Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Maxime,

On 03/24/2015 01:21 PM, Maxime Coquelin wrote:
> Hi Peter,
> 
> 2015-03-19 18:35 GMT+01:00 Maxime Coquelin <mcoquelin.stm32@gmail.com>:
>> 2015-03-19 15:58 GMT+01:00 Peter Hurley <peter@hurleysoftware.com>:
>>> On 03/19/2015 09:55 AM, Maxime Coquelin wrote:
>>>>>>>> +static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
>>>>>>>> +                           struct ktermios *old)
>>> [...]
>>>>>>>> +       usardiv = (port->uartclk * 25) / (baud * 4);
>>>>>>>> +       mantissa = (usardiv / 100) << USART_BRR_DIV_M_SHIFT;
>>>>>>>> +       fraction = DIV_ROUND_CLOSEST((usardiv % 100) * 16, 100);
>>>>>>>> +       if (fraction & ~USART_BRR_DIV_F_MASK) {
>>>>>>>> +               fraction = 0;
>>>>>>>> +               mantissa += (1 << USART_BRR_DIV_M_SHIFT);
>>>>>>>> +       }
>>> [...]
>>>> Really, I would prefer keeping this fractional divider as it is
>>>> implemented today.
>>>
>>> You have to admit that's basically an unintelligible mess;
>>> how would anyone ever be able to refactor and replace that with a
>>> common divider implementation?
>>>
>>> At the very least, please comment on the formula and format.
>>
>> Ok, I will refactor the implementation, and comment it.
> 
> The implementation was indeed a mess.
> I found some time to refactor the code, and also added support for 8
> times oversampling (16 by default).
> It will allow to achieve higher speeds, with the side effect of being
> less tolerant to clock deviations.
> 
> What do you think about the code below?
> 
> 
>     usartdiv = DIV_ROUND_CLOSEST(port->uartclk, baud);
> 
>     /*
>      * The USART supports 16 or 8 times oversampling.
>      * By default we prefer 16 times oversampling, so that the receiver
>      * has a better tolerance to clock deviations.
>      * 8 times oversampling is only used to achieve higher speeds.
>      */
>     if (usartdiv < 16) {
>         oversampling = 8;
>         stm32_set_bits(port, USART_CR1, USART_CR1_OVER8);
>     } else {
>         oversampling = 16;
>         stm32_clr_bits(port, USART_CR1, USART_CR1_OVER8);
>     }
> 
>     mantissa = (usartdiv / oversampling) << USART_BRR_DIV_M_SHIFT;
>     fraction = usartdiv % oversampling;
>     writel_relaxed(mantissa | fraction, port->membase + USART_BRR)

Thanks! Way better :)
Much more obvious this is a fixed-point divisor.

Regards,
Peter Hurley



--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Maxime,

On 03/12/2015 05:55 PM, Maxime Coquelin wrote:
> From: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> 
> This drivers adds support to the STM32 USART controller, which is a
> standard serial driver.

Comments below.

> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> ---
>  drivers/tty/serial/Kconfig       |  17 +
>  drivers/tty/serial/Makefile      |   1 +
>  drivers/tty/serial/stm32-usart.c | 695 +++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/serial_core.h |   3 +
>  4 files changed, 716 insertions(+)
>  create mode 100644 drivers/tty/serial/stm32-usart.c
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index d2501f0..880cb4f 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1611,6 +1611,23 @@ config SERIAL_SPRD_CONSOLE
>  	  with "earlycon" on the kernel command line. The console is
>  	  enabled when early_param is processed.
>  
> +config SERIAL_STM32
> +	tristate "STMicroelectronics STM32 serial port support"
> +	select SERIAL_CORE
> +	depends on ARM || COMPILE_TEST
> +	help
> +	  This driver is for the on-chip Serial Controller on
> +	  STMicroelectronics STM32 MCUs.
> +	  USART supports Rx & Tx functionality.
> +	  It support all industry standard baud rates.
> +
> +	  If unsure, say N.
> +
> +config SERIAL_STM32_CONSOLE
> +	bool "Support for console on STM32"
> +	depends on SERIAL_STM32=y
> +	select SERIAL_CORE_CONSOLE
> +
>  endmenu
>  
>  config SERIAL_MCTRL_GPIO
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index 599be4b..67c5023 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -95,6 +95,7 @@ obj-$(CONFIG_SERIAL_FSL_LPUART)	+= fsl_lpuart.o
>  obj-$(CONFIG_SERIAL_CONEXANT_DIGICOLOR)	+= digicolor-usart.o
>  obj-$(CONFIG_SERIAL_MEN_Z135)	+= men_z135_uart.o
>  obj-$(CONFIG_SERIAL_SPRD) += sprd_serial.o
> +obj-$(CONFIG_SERIAL_STM32)	+= stm32-usart.o
>  
>  # GPIOLIB helpers for modem control lines
>  obj-$(CONFIG_SERIAL_MCTRL_GPIO)	+= serial_mctrl_gpio.o
> diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
> new file mode 100644
> index 0000000..61bb065
> --- /dev/null
> +++ b/drivers/tty/serial/stm32-usart.c
> @@ -0,0 +1,695 @@
> +/*
> + * Copyright (C) Maxime Coquelin 2015
> + * Author:  Maxime Coquelin <mcoquelin.stm32@gmail.com>
> + * License terms:  GNU General Public License (GPL), version 2
> + *
> + * Inspired by st-asc.c from STMicroelectronics (c)
> + */
> +
> +#if defined(CONFIG_SERIAL_STM32_USART_CONSOLE) && defined(CONFIG_MAGIC_SYSRQ)
> +#define SUPPORT_SYSRQ
> +#endif
> +
> +#include <linux/module.h>
> +#include <linux/serial.h>
> +#include <linux/console.h>
> +#include <linux/sysrq.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/tty.h>
> +#include <linux/tty_flip.h>
> +#include <linux/delay.h>
> +#include <linux/spinlock.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/serial_core.h>
> +#include <linux/clk.h>
> +
> +#define DRIVER_NAME "stm32-usart"
> +
> +/* Register offsets */
> +#define USART_SR		0x00
> +#define USART_DR		0x04
> +#define USART_BRR		0x08
> +#define USART_CR1		0x0c
> +#define USART_CR2		0x10
> +#define USART_CR3		0x14
> +#define USART_GTPR		0x18
> +
> +/* USART_SR */
> +#define USART_SR_PE		BIT(0)
> +#define USART_SR_FE		BIT(1)
> +#define USART_SR_NF		BIT(2)
> +#define USART_SR_ORE		BIT(3)
> +#define USART_SR_IDLE		BIT(4)
> +#define USART_SR_RXNE		BIT(5)
> +#define USART_SR_TC		BIT(6)
> +#define USART_SR_TXE		BIT(7)
> +#define USART_SR_LBD		BIT(8)
> +#define USART_SR_CTS		BIT(9)
> +#define USART_SR_ERR_MASK	(USART_SR_LBD | USART_SR_ORE | \
> +				 USART_SR_FE | USART_SR_PE)
> +/* Dummy bits */
> +#define USART_SR_DUMMY_RX	BIT(16)
> +
> +/* USART_DR */
> +#define USART_DR_MASK		GENMASK(8, 0)
> +
> +/* USART_BRR */
> +#define USART_BRR_DIV_F_MASK	GENMASK(3, 0)
> +#define USART_BRR_DIV_M_MASK	GENMASK(15, 4)
> +#define USART_BRR_DIV_M_SHIFT	4
> +
> +/* USART_CR1 */
> +#define USART_CR1_SBK		BIT(0)
> +#define USART_CR1_RWU		BIT(1)
> +#define USART_CR1_RE		BIT(2)
> +#define USART_CR1_TE		BIT(3)
> +#define USART_CR1_IDLEIE	BIT(4)
> +#define USART_CR1_RXNEIE	BIT(5)
> +#define USART_CR1_TCIE		BIT(6)
> +#define USART_CR1_TXEIE		BIT(7)
> +#define USART_CR1_PEIE		BIT(8)
> +#define USART_CR1_PS		BIT(9)
> +#define USART_CR1_PCE		BIT(10)
> +#define USART_CR1_WAKE		BIT(11)
> +#define USART_CR1_M		BIT(12)
> +#define USART_CR1_UE		BIT(13)
> +#define USART_CR1_OVER8		BIT(15)
> +#define USART_CR1_IE_MASK	GENMASK(8, 4)
> +
> +/* USART_CR2 */
> +#define USART_CR2_ADD_MASK	GENMASK(3, 0)
> +#define USART_CR2_LBDL		BIT(5)
> +#define USART_CR2_LBDIE		BIT(6)
> +#define USART_CR2_LBCL		BIT(8)
> +#define USART_CR2_CPHA		BIT(9)
> +#define USART_CR2_CPOL		BIT(10)
> +#define USART_CR2_CLKEN		BIT(11)
> +#define USART_CR2_STOP_2B	BIT(13)
> +#define USART_CR2_STOP_MASK	GENMASK(13, 12)
> +#define USART_CR2_LINEN		BIT(14)
> +
> +/* USART_CR3 */
> +#define USART_CR3_EIE		BIT(0)
> +#define USART_CR3_IREN		BIT(1)
> +#define USART_CR3_IRLP		BIT(2)
> +#define USART_CR3_HDSEL		BIT(3)
> +#define USART_CR3_NACK		BIT(4)
> +#define USART_CR3_SCEN		BIT(5)
> +#define USART_CR3_DMAR		BIT(6)
> +#define USART_CR3_DMAT		BIT(7)
> +#define USART_CR3_RTSE		BIT(8)
> +#define USART_CR3_CTSE		BIT(9)
> +#define USART_CR3_CTSIE		BIT(10)
> +#define USART_CR3_ONEBIT	BIT(11)
> +
> +/* USART_GTPR */
> +#define USART_GTPR_PSC_MASK	GENMASK(7, 0)
> +#define USART_GTPR_GT_MASK	GENMASK(15, 8)
> +
> +#define DRIVER_NAME "stm32-usart"
> +#define STM32_SERIAL_NAME "ttyS"
> +#define STM32_MAX_PORTS 6
> +
> +struct stm32_port {
> +	struct uart_port port;
> +	struct clk *clk;
> +};
> +
> +static struct stm32_port stm32_ports[STM32_MAX_PORTS];
> +static struct uart_driver stm32_usart_driver;
> +
> +static void stm32_stop_tx(struct uart_port *port);
> +
> +static void stm32_set_bits(struct uart_port *port, u32 reg, u32 bits)
> +{
> +	u32 val;
> +
> +	val = readl_relaxed(port->membase + reg);
> +	val |= bits;
> +	writel_relaxed(val, port->membase + reg);
> +}
> +
> +static void stm32_clr_bits(struct uart_port *port, u32 reg, u32 bits)
> +{
> +	u32 val;
> +
> +	val = readl_relaxed(port->membase + reg);
> +	val &= ~bits;
> +	writel_relaxed(val, port->membase + reg);
> +}
> +
> +static void stm32_receive_chars(struct uart_port *port)
> +{
> +	struct tty_port *tport = &port->state->port;
> +	unsigned long c;
> +	u32 sr;
> +	char flag;
> +
> +	if (port->irq_wake)
> +		pm_wakeup_event(tport->tty->dev, 0);
> +
> +	while ((sr = readl_relaxed(port->membase + USART_SR)) & USART_SR_RXNE) {
> +		sr |= USART_SR_DUMMY_RX;
> +		c = readl_relaxed(port->membase + USART_DR);
> +		flag = TTY_NORMAL;
> +		port->icount.rx++;
> +
> +		if (sr & USART_SR_ERR_MASK) {
> +			if (sr & USART_SR_LBD) {
> +				port->icount.brk++;
> +				if (uart_handle_break(port))
> +					continue;
> +			} else if (sr & USART_SR_ORE) {
> +				port->icount.overrun++;
> +			} else if (sr & USART_SR_PE) {
> +				port->icount.parity++;
> +			} else if (sr & USART_SR_FE) {
> +				port->icount.frame++;
> +			}
> +
> +			sr &= port->read_status_mask;
> +
> +			if (sr & USART_SR_LBD)
> +				flag = TTY_BREAK;
> +			else if (sr & USART_SR_PE)
> +				flag = TTY_PARITY;
> +			else if (sr & USART_SR_FE)
> +				flag = TTY_FRAME;
> +		}
> +
> +		if (uart_handle_sysrq_char(port, c))
> +			continue;
> +		uart_insert_char(port, sr, USART_SR_ORE, c, flag);
> +	}
> +
> +	spin_unlock(&port->lock);
> +	tty_flip_buffer_push(tport);
> +	spin_lock(&port->lock);
> +}
> +
> +static void stm32_transmit_chars(struct uart_port *port)
> +{
> +	struct circ_buf *xmit = &port->state->xmit;
> +
> +	if (port->x_char) {
> +		writel_relaxed(port->x_char, port->membase + USART_DR);
> +		port->x_char = 0;
> +		port->icount.tx++;
> +		return;
> +	}
> +
> +	if (uart_tx_stopped(port)) {
> +		stm32_stop_tx(port);
> +		return;
> +	}
> +
> +	if (uart_circ_empty(xmit)) {
> +		stm32_stop_tx(port);
> +		return;
> +	}
> +
> +	writel_relaxed(xmit->buf[xmit->tail], port->membase + USART_DR);
> +	xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> +	port->icount.tx++;
> +
> +	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> +		uart_write_wakeup(port);
> +
> +	if (uart_circ_empty(xmit))
> +		stm32_stop_tx(port);
> +}
> +
> +static irqreturn_t stm32_interrupt(int irq, void *ptr)
> +{
> +	struct uart_port *port = ptr;
> +	u32 sr;
> +
> +	spin_lock(&port->lock);
> +
> +	sr = readl_relaxed(port->membase + USART_SR);
> +
> +	if (sr & USART_SR_RXNE)
> +		stm32_receive_chars(port);
> +
> +	if (sr & USART_SR_TXE)
> +		stm32_transmit_chars(port);
> +
> +	spin_unlock(&port->lock);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static unsigned int stm32_tx_empty(struct uart_port *port)
> +{
> +	return readl_relaxed(port->membase + USART_SR) & USART_SR_TXE;
> +}
> +
> +static void stm32_set_mctrl(struct uart_port *port, unsigned int mctrl)
> +{
> +	/*
> +	 * This routine is used for seting signals of: DTR, DCD, CTS/RTS
> +	 * We use USART's hardware for CTS/RTS, so don't need any for that.

If this means that you're enabling autoflow control, then you still need
to respond to the state of TIOCM_RTS, so that both serial core and userspace
can halt input flow.

If the hardware doesn't support separate RTS enable/disable control,
then you need to disable autoRTS when TIOCM_RTS is clear, and re-enable
it when raised.


> +	 * Some boards have DTR and DCD implemented using PIO pins,
> +	 * code to do this should be hooked in here.
> +	 */
> +}
> +
> +static unsigned int stm32_get_mctrl(struct uart_port *port)
> +{
> +	/*
> +	 * This routine is used for geting signals of: DTR, DCD, DSR, RI,
> +	 * and CTS/RTS
> +	 */
> +	return TIOCM_CAR | TIOCM_DSR | TIOCM_CTS;
> +}
> +
> +/* There are probably characters waiting to be transmitted. */
> +static void stm32_start_tx(struct uart_port *port)
> +{
> +	struct circ_buf *xmit = &port->state->xmit;
> +
> +	if (uart_circ_empty(xmit))
> +		return;
> +
> +	stm32_set_bits(port, USART_CR1, USART_CR1_TXEIE | USART_CR1_TE);
> +}
> +
> +/* Transmit stop */
> +static void stm32_stop_tx(struct uart_port *port)
> +{
> +	stm32_clr_bits(port, USART_CR1, USART_CR1_TXEIE);
> +}
> +
> +/* Receive stop */
> +static void stm32_stop_rx(struct uart_port *port)
> +{
> +	stm32_clr_bits(port, USART_CR1, USART_CR1_RXNEIE);
> +}
> +
> +/* Handle breaks - ignored by us */
> +static void stm32_break_ctl(struct uart_port *port, int break_state)
> +{
> +	/* Nothing here yet .. */
> +}
> +
> +static int stm32_startup(struct uart_port *port)
> +{
> +	const char *name = to_platform_device(port->dev)->name;
> +	u32 val;
> +
> +	if (request_irq(port->irq, stm32_interrupt, IRQF_NO_SUSPEND,
> +				name, port)) {
> +		dev_err(port->dev, "cannot allocate irq %d\n", port->irq);
> +		return -ENODEV;
> +	}
> +
> +	val = USART_CR1_RXNEIE | USART_CR1_TE | USART_CR1_RE;
> +	stm32_set_bits(port, USART_CR1, val);
> +
> +	return 0;
> +}
> +
> +static void stm32_shutdown(struct uart_port *port)
> +{
> +	u32 val;
> +
> +	val = USART_CR1_TXEIE | USART_CR1_RXNEIE | USART_CR1_TE | USART_CR1_RE;
> +	stm32_set_bits(port, USART_CR1, val);
> +
> +	free_irq(port->irq, port);
> +}
> +
> +static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
> +			    struct ktermios *old)
> +{
> +	unsigned int baud;
> +	u32 usardiv, mantissa, fraction;
> +	tcflag_t cflag;
> +	u32 cr1, cr2, cr3;
> +	unsigned long flags;
> +
> +	baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk / 16);
> +	cflag = termios->c_cflag;
> +
> +	spin_lock_irqsave(&port->lock, flags);
> +
> +	/* Stop serial port and reset value */
> +	writel_relaxed(0, port->membase + USART_CR1);
> +
> +	cr1 = USART_CR1_TE | USART_CR1_RE | USART_CR1_UE | USART_CR1_RXNEIE;
> +
> +	if (cflag & CSTOPB)
> +		cr2 = USART_CR2_STOP_2B;
> +
> +	if (cflag & PARENB) {
> +		cr1 |= USART_CR1_PCE;
> +		if ((cflag & CSIZE) == CS8)
> +			cr1 |= USART_CR1_M;
> +	}
> +
> +	if (cflag & PARODD)
> +		cr1 |= USART_CR1_PS;
> +
> +	if (cflag & CRTSCTS)
> +		cr3 = USART_CR3_RTSE | USART_CR3_CTSE;

If this means autoflow control, then you need to define
throttle()/unthrottle() methods, otherwise the serial core won't
be able to throttle the remote when input buffers are about
to overflow.

And you should only enable the autoCTS and let the serial
core enable autoRTS through set_mctrl(TIOCM_RTS).

Just let me know if you need more info about how to do this.

> +
> +	usardiv = (port->uartclk * 25) / (baud * 4);
> +	mantissa = (usardiv / 100) << USART_BRR_DIV_M_SHIFT;
> +	fraction = DIV_ROUND_CLOSEST((usardiv % 100) * 16, 100);
> +	if (fraction & ~USART_BRR_DIV_F_MASK) {
> +		fraction = 0;
> +		mantissa += (1 << USART_BRR_DIV_M_SHIFT);
> +	}
> +
> +	writel_relaxed(mantissa | fraction, port->membase + USART_BRR);
> +
> +	uart_update_timeout(port, cflag, baud);
> +
> +	port->read_status_mask = USART_SR_ORE;
> +	if (termios->c_iflag & INPCK)
> +		port->read_status_mask |= USART_SR_PE | USART_SR_FE;
> +	if (termios->c_iflag & (IGNBRK | BRKINT | PARMRK))
> +		port->read_status_mask |= USART_SR_LBD;
> +
> +	/* Characters to ignore */
> +	port->ignore_status_mask = 0;
> +	if (termios->c_iflag & IGNPAR)
> +		port->ignore_status_mask = USART_SR_PE | USART_SR_FE;
> +	if (termios->c_iflag & IGNBRK) {
> +		port->ignore_status_mask |= USART_SR_LBD;
> +		/*
> +		 * If we're ignoring parity and break indicators,
> +		 * ignore overruns too (for real raw support).
> +		 */
> +		if (termios->c_iflag & IGNPAR)
> +			port->ignore_status_mask |= USART_SR_ORE;
> +	}
> +
> +	/*
> +	 * Ignore all characters if CREAD is not set.
> +	 */
> +	if ((termios->c_cflag & CREAD) == 0)
> +		port->ignore_status_mask |= USART_SR_DUMMY_RX;
> +
> +	writel_relaxed(cr3, port->membase + USART_CR3);
> +	writel_relaxed(cr2, port->membase + USART_CR2);
> +	writel_relaxed(cr1, port->membase + USART_CR1);
> +
> +	spin_unlock_irqrestore(&port->lock, flags);
> +}
> +
> +static const char *stm32_type(struct uart_port *port)
> +{
> +	return (port->type == PORT_STM32) ? DRIVER_NAME : NULL;
> +}
> +
> +static void stm32_release_port(struct uart_port *port)
> +{
> +}
> +
> +static int stm32_request_port(struct uart_port *port)
> +{
> +	return 0;
> +}
> +
> +static void stm32_config_port(struct uart_port *port, int flags)
> +{
> +	if ((flags & UART_CONFIG_TYPE))

Single parens.

> +		port->type = PORT_STM32;
> +}
> +
> +static int
> +stm32_verify_port(struct uart_port *port, struct serial_struct *ser)
> +{
> +	/* No user changeable parameters */
> +	return -EINVAL;
> +}
> +
> +static void stm32_pm(struct uart_port *port, unsigned int state,
> +		unsigned int oldstate)
> +{
> +	struct stm32_port *stm32port = container_of(port,
> +			struct stm32_port, port);
> +	unsigned long flags = 0;
> +
> +	switch (state) {
> +	case UART_PM_STATE_ON:
> +		clk_prepare_enable(stm32port->clk);
> +		break;
> +	case UART_PM_STATE_OFF:
> +		spin_lock_irqsave(&port->lock, flags);
> +		stm32_clr_bits(port, USART_CR1, USART_CR1_UE);
> +		spin_unlock_irqrestore(&port->lock, flags);
> +		clk_disable_unprepare(stm32port->clk);
> +		break;
> +	}
> +}
> +
> +static struct uart_ops stm32_uart_ops = {
> +	.tx_empty	= stm32_tx_empty,
> +	.set_mctrl	= stm32_set_mctrl,
> +	.get_mctrl	= stm32_get_mctrl,
> +	.start_tx	= stm32_start_tx,
> +	.stop_tx	= stm32_stop_tx,
> +	.stop_rx	= stm32_stop_rx,
> +	.break_ctl	= stm32_break_ctl,
> +	.startup	= stm32_startup,
> +	.shutdown	= stm32_shutdown,
> +	.set_termios	= stm32_set_termios,
> +	.type		= stm32_type,
> +	.release_port	= stm32_release_port,
> +	.request_port	= stm32_request_port,
> +	.config_port	= stm32_config_port,
> +	.verify_port	= stm32_verify_port,
> +	.pm		= stm32_pm,
> +};
> +
> +static int stm32_init_port(struct stm32_port *stm32port,
> +			  struct platform_device *pdev)
> +{
> +	struct uart_port *port = &stm32port->port;
> +	struct resource *res;
> +
> +	port->iotype	= UPIO_MEM;
> +	port->flags	= UPF_BOOT_AUTOCONF;
> +	port->ops	= &stm32_uart_ops;
> +	port->dev	= &pdev->dev;
> +	port->irq	= platform_get_irq(pdev, 0);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	port->membase = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(port->membase))
> +		return PTR_ERR(port->membase);
> +	port->mapbase = res->start;
> +
> +	spin_lock_init(&port->lock);
> +
> +	stm32port->clk = devm_clk_get(&pdev->dev, NULL);
> +
> +	if (WARN_ON(IS_ERR(stm32port->clk)))

Do you really need a stack trace here? Could this be dev_err() instead?

> +		return -EINVAL;
> +
> +	/* ensure that clk rate is correct by enabling the clk */
> +	clk_prepare_enable(stm32port->clk);
> +	stm32port->port.uartclk = clk_get_rate(stm32port->clk);
> +	WARN_ON(stm32port->port.uartclk == 0);

Or here?

> +	clk_disable_unprepare(stm32port->clk);
> +
> +	return 0;
> +}
> +
> +static struct stm32_port *stm32_of_get_stm32_port(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	int id;
> +
> +	if (!np)
> +		return NULL;
> +
> +	id = of_alias_get_id(np, STM32_SERIAL_NAME);
> +
> +	if (id < 0)
> +		id = 0;
> +
> +	if (WARN_ON(id >= STM32_MAX_PORTS))

Or here?

> +		return NULL;
> +
> +	stm32_ports[id].port.line = id;
> +	return &stm32_ports[id];
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id stm32_match[] = {
> +	{ .compatible = "st,stm32-usart", },
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, stm32_match);
> +#endif
> +
> +static int stm32_serial_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	struct stm32_port *stm32port;
> +
> +	stm32port = stm32_of_get_stm32_port(pdev);
> +	if (!stm32port)
> +		return -ENODEV;
> +
> +	ret = stm32_init_port(stm32port, pdev);
> +	if (ret)
> +		return ret;
> +
> +	ret = uart_add_one_port(&stm32_usart_driver, &stm32port->port);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, &stm32port->port);
> +
> +	return 0;
> +}
> +
> +static int stm32_serial_remove(struct platform_device *pdev)
> +{
> +	struct uart_port *port = platform_get_drvdata(pdev);
> +
> +	return uart_remove_one_port(&stm32_usart_driver, port);
> +}
> +
> +
> +#ifdef CONFIG_SERIAL_STM32_CONSOLE
> +static void stm32_console_putchar(struct uart_port *port, int ch)
> +{
> +	while (!(readl_relaxed(port->membase + USART_SR) & USART_SR_TXE))
> +		cpu_relax();
> +
> +	writel_relaxed(ch, port->membase + USART_DR);
> +}
> +
> +static void stm32_console_write(struct console *co, const char *s, unsigned cnt)
> +{
> +	struct uart_port *port = &stm32_ports[co->index].port;
> +	unsigned long flags;
> +	u32 old_cr1, new_cr1;
> +	int locked = 1;
> +
> +	if (oops_in_progress) {
> +		locked = spin_trylock_irqsave(&port->lock, flags);
> +	} else {
> +		locked = 1;
> +		spin_lock_irqsave(&port->lock, flags);
> +	}
> +
> +	/* Save and disable interrupts */
> +	old_cr1 = readl_relaxed(port->membase + USART_CR1);
> +	new_cr1 = old_cr1 & ~USART_CR1_IE_MASK;
> +	writel_relaxed(new_cr1, port->membase + USART_CR1);
> +
> +	uart_console_write(port, s, cnt, stm32_console_putchar);
> +
> +	/* Restore interrupt state */
> +	writel_relaxed(old_cr1, port->membase + USART_CR1);
> +
> +	if (locked)
> +		spin_unlock_irqrestore(&port->lock, flags);
> +}
> +
> +static int stm32_console_setup(struct console *co, char *options)
> +{
> +	struct stm32_port *stm32port;
> +	int baud = 9600;
> +	int bits = 8;
> +	int parity = 'n';
> +	int flow = 'n';
> +
> +	if (co->index >= STM32_MAX_PORTS)
> +		return -ENODEV;
> +
> +	stm32port = &stm32_ports[co->index];
> +
> +	/*
> +	 * This driver does not support early console initialization
> +	 * (use ARM early printk support instead), so we only expect
> +	 * this to be called during the uart port registration when the
> +	 * driver gets probed and the port should be mapped at that point.
> +	 */
> +	if (stm32port->port.mapbase == 0 || stm32port->port.membase == NULL)
> +		return -ENXIO;
> +
> +	if (options)
> +		uart_parse_options(options, &baud, &parity, &bits, &flow);
> +
> +	return uart_set_options(&stm32port->port, co, baud, parity, bits, flow);
> +}
> +
> +static struct console stm32_console = {
> +	.name		= STM32_SERIAL_NAME,
> +	.device		= uart_console_device,
> +	.write		= stm32_console_write,
> +	.setup		= stm32_console_setup,
> +	.flags		= CON_PRINTBUFFER,
> +	.index		= -1,
> +	.data		= &stm32_usart_driver,
> +};
> +
> +#define STM32_SERIAL_CONSOLE (&stm32_console)
> +
> +#else
> +#define STM32_SERIAL_CONSOLE NULL
> +#endif /* CONFIG_SERIAL_STM32_CONSOLE */
> +
> +static struct uart_driver stm32_usart_driver = {
> +	.owner		= THIS_MODULE,
> +	.driver_name	= DRIVER_NAME,
> +	.dev_name	= STM32_SERIAL_NAME,
> +	.major		= 0,
> +	.minor		= 0,
> +	.nr		= STM32_MAX_PORTS,
> +	.cons		= STM32_SERIAL_CONSOLE,
> +};
> +
> +static struct platform_driver stm32_serial_driver = {
> +	.probe		= stm32_serial_probe,
> +	.remove		= stm32_serial_remove,
> +	.driver	= {
> +		.name	= DRIVER_NAME,
> +		.owner	= THIS_MODULE,
> +		.of_match_table = of_match_ptr(stm32_match),
> +	},
> +};
> +
> +static int __init usart_init(void)
> +{
> +	int ret;
> +	static char banner[] __initdata =
> +		KERN_INFO "STM32 USART driver initialized\n";
> +
> +	printk(banner);
> +
> +	ret = uart_register_driver(&stm32_usart_driver);
> +	if (ret)
> +		return ret;
> +
> +	ret = platform_driver_register(&stm32_serial_driver);
> +	if (ret)
> +		uart_unregister_driver(&stm32_usart_driver);
> +
> +	return ret;
> +}
> +
> +static void __exit usart_exit(void)
> +{
> +	platform_driver_unregister(&stm32_serial_driver);
> +	uart_unregister_driver(&stm32_usart_driver);
> +}
> +
> +module_init(usart_init);
> +module_exit(usart_exit);
> +
> +MODULE_ALIAS("platform:" DRIVER_NAME);
> +MODULE_DESCRIPTION("STMicroelectronics STM32 serial port driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> index b212281..e22dee5 100644
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -258,4 +258,7 @@
>  /* Cris v10 / v32 SoC */
>  #define PORT_CRIS	112
>  
> +/* STM32 USART */
> +#define PORT_STM32	110

Already taken.

You'll want to make sure v4 applies cleanly to Greg's tty-next branch.

Regards,
Peter Hurley

> +
>  #endif /* _UAPILINUX_SERIAL_CORE_H */
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, Mar 24, 2015 at 02:23:38PM -0400, Peter Hurley wrote:
> Hi Maxime,
> 
> On 03/12/2015 05:55 PM, Maxime Coquelin wrote:
> > +static unsigned int stm32_get_mctrl(struct uart_port *port)
> > +{
> > +	/*
> > +	 * This routine is used for geting signals of: DTR, DCD, DSR, RI,
> > +	 * and CTS/RTS

It's also worth noting that this comment is wrong.  This is used to get
the state of DCD, DSR, RI and CTS.  DTR and RTS are *not* included
because the core tracks their state.

...

> > +	stm32port->clk = devm_clk_get(&pdev->dev, NULL);
> > +
> > +	if (WARN_ON(IS_ERR(stm32port->clk)))
> 
> Do you really need a stack trace here? Could this be dev_err() instead?
> 
> > +		return -EINVAL;

Also, propagate the error code.

> > +
> > +	/* ensure that clk rate is correct by enabling the clk */
> > +	clk_prepare_enable(stm32port->clk);

And remember to check the return value of clk_prepare_enable().

HI Peter

2015-03-24 19:23 GMT+01:00 Peter Hurley <peter@hurleysoftware.com>:
> Hi Maxime,
>
> On 03/12/2015 05:55 PM, Maxime Coquelin wrote:
>> From: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>
>> This drivers adds support to the STM32 USART controller, which is a
>> standard serial driver.
>
> Comments below.

Thanks for the review, please find my answsers below
>
>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>> ---
>>  drivers/tty/serial/Kconfig       |  17 +
>>  drivers/tty/serial/Makefile      |   1 +
>>  drivers/tty/serial/stm32-usart.c | 695 +++++++++++++++++++++++++++++++++++++++
>>  include/uapi/linux/serial_core.h |   3 +
>>  4 files changed, 716 insertions(+)
>>  create mode 100644 drivers/tty/serial/stm32-usart.c
>>

...

>> +static unsigned int stm32_tx_empty(struct uart_port *port)
>> +{
>> +     return readl_relaxed(port->membase + USART_SR) & USART_SR_TXE;
>> +}
>> +
>> +static void stm32_set_mctrl(struct uart_port *port, unsigned int mctrl)
>> +{
>> +     /*
>> +      * This routine is used for seting signals of: DTR, DCD, CTS/RTS
>> +      * We use USART's hardware for CTS/RTS, so don't need any for that.
>
> If this means that you're enabling autoflow control, then you still need
> to respond to the state of TIOCM_RTS, so that both serial core and userspace
> can halt input flow.
>
> If the hardware doesn't support separate RTS enable/disable control,
> then you need to disable autoRTS when TIOCM_RTS is clear, and re-enable
> it when raised.
>
>
>> +      * Some boards have DTR and DCD implemented using PIO pins,
>> +      * code to do this should be hooked in here.
>> +      */
>> +}
>> +
>> +static unsigned int stm32_get_mctrl(struct uart_port *port)
>> +{
>> +     /*
>> +      * This routine is used for geting signals of: DTR, DCD, DSR, RI,
>> +      * and CTS/RTS
>> +      */
>> +     return TIOCM_CAR | TIOCM_DSR | TIOCM_CTS;
>> +}
>> +

...
>> +
>> +static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
>> +                         struct ktermios *old)
>> +{
>> +     unsigned int baud;
>> +     u32 usardiv, mantissa, fraction;
>> +     tcflag_t cflag;
>> +     u32 cr1, cr2, cr3;
>> +     unsigned long flags;
>> +
>> +     baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk / 16);
>> +     cflag = termios->c_cflag;
>> +
>> +     spin_lock_irqsave(&port->lock, flags);
>> +
>> +     /* Stop serial port and reset value */
>> +     writel_relaxed(0, port->membase + USART_CR1);
>> +
>> +     cr1 = USART_CR1_TE | USART_CR1_RE | USART_CR1_UE | USART_CR1_RXNEIE;
>> +
>> +     if (cflag & CSTOPB)
>> +             cr2 = USART_CR2_STOP_2B;
>> +
>> +     if (cflag & PARENB) {
>> +             cr1 |= USART_CR1_PCE;
>> +             if ((cflag & CSIZE) == CS8)
>> +                     cr1 |= USART_CR1_M;
>> +     }
>> +
>> +     if (cflag & PARODD)
>> +             cr1 |= USART_CR1_PS;
>> +
>> +     if (cflag & CRTSCTS)
>> +             cr3 = USART_CR3_RTSE | USART_CR3_CTSE;
>
> If this means autoflow control, then you need to define
> throttle()/unthrottle() methods, otherwise the serial core won't
> be able to throttle the remote when input buffers are about
> to overflow.
>
> And you should only enable the autoCTS and let the serial
> core enable autoRTS through set_mctrl(TIOCM_RTS).
>
> Just let me know if you need more info about how to do this.

Ok, let's see if I have well understood.

USART_CR3_RTSE should be set/cleared in set_mctrl(), depending on
TIOCM_RTS  value.
The throttle callback should disable the rx interrupt, and the
unthrottle enable it.
For CTS, it should be enabled in set_termios() if CRTSCTS, as done here.

Am I right?
>
>> +
>> +     usardiv = (port->uartclk * 25) / (baud * 4);
>> +     mantissa = (usardiv / 100) << USART_BRR_DIV_M_SHIFT;
>> +     fraction = DIV_ROUND_CLOSEST((usardiv % 100) * 16, 100);
>> +     if (fraction & ~USART_BRR_DIV_F_MASK) {
>> +             fraction = 0;
>> +             mantissa += (1 << USART_BRR_DIV_M_SHIFT);
>> +     }
>> +
>> +     writel_relaxed(mantissa | fraction, port->membase + USART_BRR);
>> +
>> +     uart_update_timeout(port, cflag, baud);
>> +
>> +     port->read_status_mask = USART_SR_ORE;
>> +     if (termios->c_iflag & INPCK)
>> +             port->read_status_mask |= USART_SR_PE | USART_SR_FE;
>> +     if (termios->c_iflag & (IGNBRK | BRKINT | PARMRK))
>> +             port->read_status_mask |= USART_SR_LBD;
>> +
>> +     /* Characters to ignore */
>> +     port->ignore_status_mask = 0;
>> +     if (termios->c_iflag & IGNPAR)
>> +             port->ignore_status_mask = USART_SR_PE | USART_SR_FE;
>> +     if (termios->c_iflag & IGNBRK) {
>> +             port->ignore_status_mask |= USART_SR_LBD;
>> +             /*
>> +              * If we're ignoring parity and break indicators,
>> +              * ignore overruns too (for real raw support).
>> +              */
>> +             if (termios->c_iflag & IGNPAR)
>> +                     port->ignore_status_mask |= USART_SR_ORE;
>> +     }
>> +
>> +     /*
>> +      * Ignore all characters if CREAD is not set.
>> +      */
>> +     if ((termios->c_cflag & CREAD) == 0)
>> +             port->ignore_status_mask |= USART_SR_DUMMY_RX;
>> +
>> +     writel_relaxed(cr3, port->membase + USART_CR3);
>> +     writel_relaxed(cr2, port->membase + USART_CR2);
>> +     writel_relaxed(cr1, port->membase + USART_CR1);
>> +
>> +     spin_unlock_irqrestore(&port->lock, flags);
>> +}
>> +
...

>> +static void stm32_config_port(struct uart_port *port, int flags)
>> +{
>> +     if ((flags & UART_CONFIG_TYPE))
>
> Single parens.

Sure.

>
>> +             port->type = PORT_STM32;
>> +}
>> +
...

>> +
>> +static int stm32_init_port(struct stm32_port *stm32port,
>> +                       struct platform_device *pdev)
>> +{
>> +     struct uart_port *port = &stm32port->port;
>> +     struct resource *res;
>> +
>> +     port->iotype    = UPIO_MEM;
>> +     port->flags     = UPF_BOOT_AUTOCONF;
>> +     port->ops       = &stm32_uart_ops;
>> +     port->dev       = &pdev->dev;
>> +     port->irq       = platform_get_irq(pdev, 0);
>> +
>> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +     port->membase = devm_ioremap_resource(&pdev->dev, res);
>> +     if (IS_ERR(port->membase))
>> +             return PTR_ERR(port->membase);
>> +     port->mapbase = res->start;
>> +
>> +     spin_lock_init(&port->lock);
>> +
>> +     stm32port->clk = devm_clk_get(&pdev->dev, NULL);
>> +
>> +     if (WARN_ON(IS_ERR(stm32port->clk)))
>
> Do you really need a stack trace here? Could this be dev_err() instead?

No I don't, dev_err() will be enough.
>
>> +             return -EINVAL;
>> +
>> +     /* ensure that clk rate is correct by enabling the clk */
>> +     clk_prepare_enable(stm32port->clk);
>> +     stm32port->port.uartclk = clk_get_rate(stm32port->clk);
>> +     WARN_ON(stm32port->port.uartclk == 0);
>
> Or here?
Same here.

>
>> +     clk_disable_unprepare(stm32port->clk);
>> +
>> +     return 0;
>> +}
>> +
>> +static struct stm32_port *stm32_of_get_stm32_port(struct platform_device *pdev)
>> +{
>> +     struct device_node *np = pdev->dev.of_node;
>> +     int id;
>> +
>> +     if (!np)
>> +             return NULL;
>> +
>> +     id = of_alias_get_id(np, STM32_SERIAL_NAME);
>> +
>> +     if (id < 0)
>> +             id = 0;
>> +
>> +     if (WARN_ON(id >= STM32_MAX_PORTS))
>
> Or here?

I will return PTR_ERR(-EINVAL); instead.
>
>> +             return NULL;
>> +
>> +     stm32_ports[id].port.line = id;
>> +     return &stm32_ports[id];
>> +}
>> +
...

>> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
>> index b212281..e22dee5 100644
>> --- a/include/uapi/linux/serial_core.h
>> +++ b/include/uapi/linux/serial_core.h
>> @@ -258,4 +258,7 @@
>>  /* Cris v10 / v32 SoC */
>>  #define PORT_CRIS    112
>>
>> +/* STM32 USART */
>> +#define PORT_STM32   110
>
> Already taken.
>
> You'll want to make sure v4 applies cleanly to Greg's tty-next branch.

Sure, I made a mistake during the rebase conflict resolution.

Thanks,
Maxime

>
> Regards,
> Peter Hurley
>
>> +
>>  #endif /* _UAPILINUX_SERIAL_CORE_H */
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

2015-03-26 16:46 GMT+01:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Tue, Mar 24, 2015 at 02:23:38PM -0400, Peter Hurley wrote:
>> Hi Maxime,
>>
>> On 03/12/2015 05:55 PM, Maxime Coquelin wrote:
>> > +static unsigned int stm32_get_mctrl(struct uart_port *port)
>> > +{
>> > +   /*
>> > +    * This routine is used for geting signals of: DTR, DCD, DSR, RI,
>> > +    * and CTS/RTS
>
> It's also worth noting that this comment is wrong.  This is used to get
> the state of DCD, DSR, RI and CTS.  DTR and RTS are *not* included
> because the core tracks their state.

OK, thanks for pointing this.
I will fix the comment in the v4.

>
> ...
>
>> > +   stm32port->clk = devm_clk_get(&pdev->dev, NULL);
>> > +
>> > +   if (WARN_ON(IS_ERR(stm32port->clk)))
>>
>> Do you really need a stack trace here? Could this be dev_err() instead?
>>
>> > +           return -EINVAL;
>
> Also, propagate the error code.
Ok.

>
>> > +
>> > +   /* ensure that clk rate is correct by enabling the clk */
>> > +   clk_prepare_enable(stm32port->clk);
>
> And remember to check the return value of clk_prepare_enable().
I will.

Thanks for the review,
Maxime

>
> --
> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
> according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 03/26/2015 06:03 PM, Maxime Coquelin wrote:
>>> +static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
>>> +                         struct ktermios *old)
>>> +{
>>> +     unsigned int baud;
>>> +     u32 usardiv, mantissa, fraction;
>>> +     tcflag_t cflag;
>>> +     u32 cr1, cr2, cr3;
>>> +     unsigned long flags;
>>> +
>>> +     baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk / 16);
>>> +     cflag = termios->c_cflag;
>>> +
>>> +     spin_lock_irqsave(&port->lock, flags);
>>> +
>>> +     /* Stop serial port and reset value */
>>> +     writel_relaxed(0, port->membase + USART_CR1);
>>> +
>>> +     cr1 = USART_CR1_TE | USART_CR1_RE | USART_CR1_UE | USART_CR1_RXNEIE;
>>> +
>>> +     if (cflag & CSTOPB)
>>> +             cr2 = USART_CR2_STOP_2B;
>>> +
>>> +     if (cflag & PARENB) {
>>> +             cr1 |= USART_CR1_PCE;
>>> +             if ((cflag & CSIZE) == CS8)
>>> +                     cr1 |= USART_CR1_M;
>>> +     }
>>> +
>>> +     if (cflag & PARODD)
>>> +             cr1 |= USART_CR1_PS;
>>> +
>>> +     if (cflag & CRTSCTS)
>>> +             cr3 = USART_CR3_RTSE | USART_CR3_CTSE;
>>
>> If this means autoflow control, then you need to define
>> throttle()/unthrottle() methods, otherwise the serial core won't
>> be able to throttle the remote when input buffers are about
>> to overflow.
>>
>> And you should only enable the autoCTS and let the serial
>> core enable autoRTS through set_mctrl(TIOCM_RTS).
>>
>> Just let me know if you need more info about how to do this.
> 
> Ok, let's see if I have well understood.
> 
> USART_CR3_RTSE should be set/cleared in set_mctrl(), depending on
> TIOCM_RTS  value.
> The throttle callback should disable the rx interrupt, and the
> unthrottle enable it.
> For CTS, it should be enabled in set_termios() if CRTSCTS, as done here.
> 
> Am I right?

Yeah, basically. You also have to indicate to the serial core that you
require throttle/unthrottle handling in this mode by setting port->status.

Your set_termios() method would look like:

	port->status &= ~(UPSTAT_AUTOCTS | UPSTAT_AUTORTS);
	if (cflag & CRTSCTS) {
		port->status |= UPSTAT_AUTOCTS | UPSTAT_AUTORTS;
		cr3 = USART_CR3_CTSE;
	}

and your set_mctrl() method would look like:

	if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS))
		stm32_set_bits(port, USART_CR3, USART_CR3_RTSE);
	else
		stm32_clear_bits(port, USART_CR3, USART_CR3_RTSE);

The UPSTAT_AUTOCTS doesn't really do anything right now but please
use it anyway to indicate this driver has that functionality.

Regards,
Peter Hurley

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

2015-03-27 12:32 GMT+01:00 Peter Hurley <peter@hurleysoftware.com>:
> On 03/26/2015 06:03 PM, Maxime Coquelin wrote:
>>>> +static void stm32_set_termios(struct uart_port *port, struct ktermios *termios,
>>>> +                         struct ktermios *old)
>>>> +{
>>>> +     unsigned int baud;
>>>> +     u32 usardiv, mantissa, fraction;
>>>> +     tcflag_t cflag;
>>>> +     u32 cr1, cr2, cr3;
>>>> +     unsigned long flags;
>>>> +
>>>> +     baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk / 16);
>>>> +     cflag = termios->c_cflag;
>>>> +
>>>> +     spin_lock_irqsave(&port->lock, flags);
>>>> +
>>>> +     /* Stop serial port and reset value */
>>>> +     writel_relaxed(0, port->membase + USART_CR1);
>>>> +
>>>> +     cr1 = USART_CR1_TE | USART_CR1_RE | USART_CR1_UE | USART_CR1_RXNEIE;
>>>> +
>>>> +     if (cflag & CSTOPB)
>>>> +             cr2 = USART_CR2_STOP_2B;
>>>> +
>>>> +     if (cflag & PARENB) {
>>>> +             cr1 |= USART_CR1_PCE;
>>>> +             if ((cflag & CSIZE) == CS8)
>>>> +                     cr1 |= USART_CR1_M;
>>>> +     }
>>>> +
>>>> +     if (cflag & PARODD)
>>>> +             cr1 |= USART_CR1_PS;
>>>> +
>>>> +     if (cflag & CRTSCTS)
>>>> +             cr3 = USART_CR3_RTSE | USART_CR3_CTSE;
>>>
>>> If this means autoflow control, then you need to define
>>> throttle()/unthrottle() methods, otherwise the serial core won't
>>> be able to throttle the remote when input buffers are about
>>> to overflow.
>>>
>>> And you should only enable the autoCTS and let the serial
>>> core enable autoRTS through set_mctrl(TIOCM_RTS).
>>>
>>> Just let me know if you need more info about how to do this.
>>
>> Ok, let's see if I have well understood.
>>
>> USART_CR3_RTSE should be set/cleared in set_mctrl(), depending on
>> TIOCM_RTS  value.
>> The throttle callback should disable the rx interrupt, and the
>> unthrottle enable it.
>> For CTS, it should be enabled in set_termios() if CRTSCTS, as done here.
>>
>> Am I right?
>
> Yeah, basically. You also have to indicate to the serial core that you
> require throttle/unthrottle handling in this mode by setting port->status.
>
> Your set_termios() method would look like:
>
>         port->status &= ~(UPSTAT_AUTOCTS | UPSTAT_AUTORTS);
>         if (cflag & CRTSCTS) {
>                 port->status |= UPSTAT_AUTOCTS | UPSTAT_AUTORTS;
>                 cr3 = USART_CR3_CTSE;
>         }
>
> and your set_mctrl() method would look like:
>
>         if ((mctrl & TIOCM_RTS) && (port->status & UPSTAT_AUTORTS))
>                 stm32_set_bits(port, USART_CR3, USART_CR3_RTSE);
>         else
>                 stm32_clear_bits(port, USART_CR3, USART_CR3_RTSE);
>
> The UPSTAT_AUTOCTS doesn't really do anything right now but please
> use it anyway to indicate this driver has that functionality.

Ok, thanks for your support.

I will implement this in the v4.

Regards,
Maxime

>
> Regards,
> Peter Hurley
>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html