| Message ID | 1424292068-28197-7-git-send-email-u.kleine-koenig@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
Hello, 2015-02-18 21:41 GMT+01:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>: > The transmitter is expected to be controlled by the UART's RTS pin. You definately can be more open in using the last patch I have sent the 11th of februrary using rs485.padding[0] that give in parameter the GPIO to use for transmit pin > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/tty/serial/imx.c | 94 +++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 81 insertions(+), 13 deletions(-) > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > index 806b3cfb7f55..ad43019a4880 100644 > --- a/drivers/tty/serial/imx.c > +++ b/drivers/tty/serial/imx.c > @@ -364,8 +364,20 @@ static void imx_stop_tx(struct uart_port *port) > if (sport->dma_is_enabled && sport->dma_is_txing) > return; > > - temp = readl(sport->port.membase + UCR1); > - writel(temp & ~UCR1_TXMPTYEN, sport->port.membase + UCR1); > + temp = readl(port->membase + UCR1); > + writel(temp & ~UCR1_TXMPTYEN, port->membase + UCR1); > + > + /* in rs485 mode disable transmitter if shifter is empty */ > + if (port->rs485.flags & SER_RS485_ENABLED && > + readl(port->membase + USR2) & USR2_TXDC) { > + temp = readl(port->membase + UCR2); > + temp |= UCR2_CTS; > + writel(temp, port->membase + UCR2); > + > + temp = readl(port->membase + UCR4); > + temp &= ~UCR4_TCEN; > + writel(temp, port->membase + UCR4); > + } > } > > /* > @@ -520,6 +532,17 @@ static void imx_start_tx(struct uart_port *port) > struct imx_port *sport = (struct imx_port *)port; > unsigned long temp; > > + if (port->rs485.flags & SER_RS485_ENABLED) { > + /* enable transmitter and shifter empty irq */ > + temp = readl(port->membase + UCR2); > + temp &= ~UCR2_CTS; > + writel(temp, port->membase + UCR2); > + > + temp = readl(port->membase + UCR4); > + temp |= UCR4_TCEN; > + writel(temp, port->membase + UCR4); > + } > + > if (!sport->dma_is_enabled) { > temp = readl(sport->port.membase + UCR1); > writel(temp | UCR1_TXMPTYEN, sport->port.membase + UCR1); > @@ -661,6 +684,7 @@ static irqreturn_t imx_int(int irq, void *dev_id) > unsigned int sts2; > > sts = readl(sport->port.membase + USR1); > + sts2 = readl(sport->port.membase + USR2); > > if (sts & USR1_RRDY) { > if (sport->dma_is_enabled) > @@ -669,8 +693,10 @@ static irqreturn_t imx_int(int irq, void *dev_id) > imx_rxint(irq, dev_id); > } > > - if (sts & USR1_TRDY && > - readl(sport->port.membase + UCR1) & UCR1_TXMPTYEN) > + if ((sts & USR1_TRDY && > + readl(sport->port.membase + UCR1) & UCR1_TXMPTYEN) || > + (sts2 & USR2_TXDC && > + readl(sport->port.membase + UCR4) & UCR4_TCEN)) > imx_txint(irq, dev_id); > > if (sts & USR1_RTSD) > @@ -679,7 +705,6 @@ static irqreturn_t imx_int(int irq, void *dev_id) > if (sts & USR1_AWAKE) > writel(USR1_AWAKE, sport->port.membase + USR1); > > - sts2 = readl(sport->port.membase + USR2); > if (sts2 & USR2_ORE) { > dev_err(sport->port.dev, "Rx FIFO overrun\n"); > sport->port.icount.overrun++; > @@ -731,11 +756,13 @@ static void imx_set_mctrl(struct uart_port *port, unsigned int mctrl) > struct imx_port *sport = (struct imx_port *)port; > unsigned long temp; > > - temp = readl(sport->port.membase + UCR2) & ~(UCR2_CTS | UCR2_CTSC); > - if (mctrl & TIOCM_RTS) > - temp |= UCR2_CTS | UCR2_CTSC; > - > - writel(temp, sport->port.membase + UCR2); > + if (!(port->rs485.flags & SER_RS485_ENABLED)) { > + temp = readl(sport->port.membase + UCR2); > + temp &= ~(UCR2_CTS | UCR2_CTSC); > + if (mctrl & TIOCM_RTS) > + temp |= UCR2_CTS | UCR2_CTSC; > + writel(temp, sport->port.membase + UCR2); > + } > > temp = readl(sport->port.membase + uts_reg(sport)) & ~UTS_LOOP; > if (mctrl & TIOCM_LOOP) > @@ -1029,7 +1056,7 @@ static int imx_startup(struct uart_port *port) > temp |= UCR1_RRDYEN | UCR1_RTSDEN | UCR1_UARTEN; > writel(temp, sport->port.membase + UCR1); > > - temp = read(sport->port.membase + UCR4); > + temp = readl(sport->port.membase + UCR4); > temp |= UCR4_OREN; > writel(temp, sport->port.membase + UCR4); > > @@ -1144,7 +1171,17 @@ imx_set_termios(struct uart_port *port, struct ktermios *termios, > if (termios->c_cflag & CRTSCTS) { > if (sport->have_rtscts) { > ucr2 &= ~UCR2_IRTS; > - ucr2 |= UCR2_CTSC; > + > + if (port->rs485.flags & SER_RS485_ENABLED) > + /* > + * RTS is mandatory for rs485 operation, so keep > + * it under manual control and keep transmitter > + * disabled. > + */ > + ucr2 |= UCR2_CTS; > + else > + ucr2 |= UCR2_CTSC; > + > > /* Can we enable the DMA support? */ > if (is_imx6q_uart(sport) && !uart_console(port) > @@ -1153,7 +1190,9 @@ imx_set_termios(struct uart_port *port, struct ktermios *termios, > } else { > termios->c_cflag &= ~CRTSCTS; > } > - } > + } else if (port->rs485.flags & SER_RS485_ENABLED) > + /* disable transmitter */ > + ucr2 |= UCR2_CTS; > > if (termios->c_cflag & CSTOPB) > ucr2 |= UCR2_STPB; > @@ -1374,6 +1413,34 @@ static void imx_poll_put_char(struct uart_port *port, unsigned char c) > } > #endif > > +static int imx_rs485_config(struct uart_port *port, > + struct serial_rs485 *rs485conf) > +{ > + struct imx_port *sport = (struct imx_port *)port; > + > + /* unimplemented */ > + rs485conf->delay_rts_before_send = 0; > + rs485conf->delay_rts_after_send = 0; > + > + /* RTS is required to control the transmitter */ > + if (!sport->have_rtscts) > + rs485conf->flags &= ~SER_RS485_ENABLED; > + > + if (rs485conf->flags & SER_RS485_ENABLED) { > + unsigned long temp; > + > + /* disable transmitter */ > + temp = readl(sport->port.membase + UCR2); > + temp &= ~UCR2_CTSC; > + temp |= UCR2_CTS; > + writel(temp, sport->port.membase + UCR2); > + } > + > + port->rs485 = *rs485conf; > + > + return 0; > +} > + > static struct uart_ops imx_pops = { > .tx_empty = imx_tx_empty, > .set_mctrl = imx_set_mctrl, > @@ -1731,6 +1798,7 @@ static int serial_imx_probe(struct platform_device *pdev) > sport->port.irq = rxirq; > sport->port.fifosize = 32; > sport->port.ops = &imx_pops; > + sport->port.rs485_config = imx_rs485_config; > sport->port.flags = UPF_BOOT_AUTOCONF; > init_timer(&sport->timer); > sport->timer.function = imx_timeout; > -- > 2.1.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-serial" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Aurélien, On Thu, Feb 19, 2015 at 10:15:10AM +0100, aurélien bouin wrote: > 2015-02-18 21:41 GMT+01:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>: > > The transmitter is expected to be controlled by the UART's RTS pin. > > You definately can be more open in using the last patch I have sent > the 11th of februrary using rs485.padding[0] that give in parameter > the GPIO to use for transmit pin you have a link? Without having seen that patch this can well be a separate patch. And (also without having seen the patch) I wonder why the information which gpio to use doesn't come from the device tree but is supplied by userspace. I thought about expanding mctrl-gpio for that, but as there are a few conflicting patches to mctrl-gpio out I wanted to wait until the dust settles. Best regards Uwe
I hope this link will be ok : http://marc.info/?l=linux-kernel&m=142366865609389&q=raw Best regards, 2015-02-19 10:37 GMT+01:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>: > Hello Aurélien, > > On Thu, Feb 19, 2015 at 10:15:10AM +0100, aurélien bouin wrote: >> 2015-02-18 21:41 GMT+01:00 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>: >> > The transmitter is expected to be controlled by the UART's RTS pin. >> >> You definately can be more open in using the last patch I have sent >> the 11th of februrary using rs485.padding[0] that give in parameter >> the GPIO to use for transmit pin > you have a link? Without having seen that patch this can well be a > separate patch. And (also without having seen the patch) I wonder why > the information which gpio to use doesn't come from the device tree but > is supplied by userspace. I thought about expanding mctrl-gpio for that, > but as there are a few conflicting patches to mctrl-gpio out I wanted to > wait until the dust settles. > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König | > Industrial Linux Solutions | http://www.pengutronix.de/ |
On Thu, Feb 19, 2015 at 10:45:47AM +0100, aurélien bouin wrote: > I hope this link will be ok : > http://marc.info/?l=linux-kernel&m=142366865609389&q=raw Good heavens, this patch is broken. It depends on user space to get the reference counting for the gpio right if at all. It uses a reserved part of the struct serial_rs485 while being driver specific. It ignores gpio_request returning an error (well, it does a dev_err in this case, but continues to use the gpio). It does change how frame error are handled even if rs485 is off without mentioning in the changelog. Are you aware what happens if a user issues the rs485 callback without being aware that padding[0] has this meaning? I'm sorry, I like my patch better. Best regards Uwe
Hello, speaking of serial patch for rs485, I wrote a device driver for mxs (iMX28): (it is half-duplex too) It rely on device tree node as: >> rs485_tx_en: rx485-tx-en { >> fsl,pinmux-ids = < >> 0x2133 /* MX28_PAD_SSP2_SS0__GPIO_2_19 UART2_DIR */ >> >; >> fsl,drive-strength = <0>; >> fsl,pull-up = <1>; >> }; and this: >> auart2: serial@8006e000 { >> compatible = "fsl,imx28-ekuart"; >> pinctrl-names = "default"; >> pinctrl-0 = <&auart2_2pins_a >> &rs485_tx_en>; >> rs485-rts-active-low; >> rts-gpio = <&gpio2 19 1>; >> linux,rs485-enabled-at-boot-time; >> rs485-rts-delay = <0 0>; >> status = "okay"; >> }; And in attachment the driver. The hr_timer implementation is working correctly even with a big load and 3/4 serials at the time. Give it a look if you want. On 02/20/2015 04:34 PM, Uwe Kleine-König wrote: > On Thu, Feb 19, 2015 at 10:45:47AM +0100, aurélien bouin wrote: >> I hope this link will be ok : >> http://marc.info/?l=linux-kernel&m=142366865609389&q=raw > Good heavens, this patch is broken. It depends on user space to get the > reference counting for the gpio right if at all. It uses a reserved part > of the struct serial_rs485 while being driver specific. It ignores > gpio_request returning an error (well, it does a dev_err in this case, > but continues to use the gpio). It does change how frame error are > handled even if rs485 is off without mentioning in the changelog. > > Are you aware what happens if a user issues the rs485 callback without > being aware that padding[0] has this meaning? > > I'm sorry, I like my patch better. > > Best regards > Uwe >
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index 806b3cfb7f55..ad43019a4880 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -364,8 +364,20 @@ static void imx_stop_tx(struct uart_port *port) if (sport->dma_is_enabled && sport->dma_is_txing) return; - temp = readl(sport->port.membase + UCR1); - writel(temp & ~UCR1_TXMPTYEN, sport->port.membase + UCR1); + temp = readl(port->membase + UCR1); + writel(temp & ~UCR1_TXMPTYEN, port->membase + UCR1); + + /* in rs485 mode disable transmitter if shifter is empty */ + if (port->rs485.flags & SER_RS485_ENABLED && + readl(port->membase + USR2) & USR2_TXDC) { + temp = readl(port->membase + UCR2); + temp |= UCR2_CTS; + writel(temp, port->membase + UCR2); + + temp = readl(port->membase + UCR4); + temp &= ~UCR4_TCEN; + writel(temp, port->membase + UCR4); + } } /* @@ -520,6 +532,17 @@ static void imx_start_tx(struct uart_port *port) struct imx_port *sport = (struct imx_port *)port; unsigned long temp; + if (port->rs485.flags & SER_RS485_ENABLED) { + /* enable transmitter and shifter empty irq */ + temp = readl(port->membase + UCR2); + temp &= ~UCR2_CTS; + writel(temp, port->membase + UCR2); + + temp = readl(port->membase + UCR4); + temp |= UCR4_TCEN; + writel(temp, port->membase + UCR4); + } + if (!sport->dma_is_enabled) { temp = readl(sport->port.membase + UCR1); writel(temp | UCR1_TXMPTYEN, sport->port.membase + UCR1); @@ -661,6 +684,7 @@ static irqreturn_t imx_int(int irq, void *dev_id) unsigned int sts2; sts = readl(sport->port.membase + USR1); + sts2 = readl(sport->port.membase + USR2); if (sts & USR1_RRDY) { if (sport->dma_is_enabled) @@ -669,8 +693,10 @@ static irqreturn_t imx_int(int irq, void *dev_id) imx_rxint(irq, dev_id); } - if (sts & USR1_TRDY && - readl(sport->port.membase + UCR1) & UCR1_TXMPTYEN) + if ((sts & USR1_TRDY && + readl(sport->port.membase + UCR1) & UCR1_TXMPTYEN) || + (sts2 & USR2_TXDC && + readl(sport->port.membase + UCR4) & UCR4_TCEN)) imx_txint(irq, dev_id); if (sts & USR1_RTSD) @@ -679,7 +705,6 @@ static irqreturn_t imx_int(int irq, void *dev_id) if (sts & USR1_AWAKE) writel(USR1_AWAKE, sport->port.membase + USR1); - sts2 = readl(sport->port.membase + USR2); if (sts2 & USR2_ORE) { dev_err(sport->port.dev, "Rx FIFO overrun\n"); sport->port.icount.overrun++; @@ -731,11 +756,13 @@ static void imx_set_mctrl(struct uart_port *port, unsigned int mctrl) struct imx_port *sport = (struct imx_port *)port; unsigned long temp; - temp = readl(sport->port.membase + UCR2) & ~(UCR2_CTS | UCR2_CTSC); - if (mctrl & TIOCM_RTS) - temp |= UCR2_CTS | UCR2_CTSC; - - writel(temp, sport->port.membase + UCR2); + if (!(port->rs485.flags & SER_RS485_ENABLED)) { + temp = readl(sport->port.membase + UCR2); + temp &= ~(UCR2_CTS | UCR2_CTSC); + if (mctrl & TIOCM_RTS) + temp |= UCR2_CTS | UCR2_CTSC; + writel(temp, sport->port.membase + UCR2); + } temp = readl(sport->port.membase + uts_reg(sport)) & ~UTS_LOOP; if (mctrl & TIOCM_LOOP) @@ -1029,7 +1056,7 @@ static int imx_startup(struct uart_port *port) temp |= UCR1_RRDYEN | UCR1_RTSDEN | UCR1_UARTEN; writel(temp, sport->port.membase + UCR1); - temp = read(sport->port.membase + UCR4); + temp = readl(sport->port.membase + UCR4); temp |= UCR4_OREN; writel(temp, sport->port.membase + UCR4); @@ -1144,7 +1171,17 @@ imx_set_termios(struct uart_port *port, struct ktermios *termios, if (termios->c_cflag & CRTSCTS) { if (sport->have_rtscts) { ucr2 &= ~UCR2_IRTS; - ucr2 |= UCR2_CTSC; + + if (port->rs485.flags & SER_RS485_ENABLED) + /* + * RTS is mandatory for rs485 operation, so keep + * it under manual control and keep transmitter + * disabled. + */ + ucr2 |= UCR2_CTS; + else + ucr2 |= UCR2_CTSC; + /* Can we enable the DMA support? */ if (is_imx6q_uart(sport) && !uart_console(port) @@ -1153,7 +1190,9 @@ imx_set_termios(struct uart_port *port, struct ktermios *termios, } else { termios->c_cflag &= ~CRTSCTS; } - } + } else if (port->rs485.flags & SER_RS485_ENABLED) + /* disable transmitter */ + ucr2 |= UCR2_CTS; if (termios->c_cflag & CSTOPB) ucr2 |= UCR2_STPB; @@ -1374,6 +1413,34 @@ static void imx_poll_put_char(struct uart_port *port, unsigned char c) } #endif +static int imx_rs485_config(struct uart_port *port, + struct serial_rs485 *rs485conf) +{ + struct imx_port *sport = (struct imx_port *)port; + + /* unimplemented */ + rs485conf->delay_rts_before_send = 0; + rs485conf->delay_rts_after_send = 0; + + /* RTS is required to control the transmitter */ + if (!sport->have_rtscts) + rs485conf->flags &= ~SER_RS485_ENABLED; + + if (rs485conf->flags & SER_RS485_ENABLED) { + unsigned long temp; + + /* disable transmitter */ + temp = readl(sport->port.membase + UCR2); + temp &= ~UCR2_CTSC; + temp |= UCR2_CTS; + writel(temp, sport->port.membase + UCR2); + } + + port->rs485 = *rs485conf; + + return 0; +} + static struct uart_ops imx_pops = { .tx_empty = imx_tx_empty, .set_mctrl = imx_set_mctrl, @@ -1731,6 +1798,7 @@ static int serial_imx_probe(struct platform_device *pdev) sport->port.irq = rxirq; sport->port.fifosize = 32; sport->port.ops = &imx_pops; + sport->port.rs485_config = imx_rs485_config; sport->port.flags = UPF_BOOT_AUTOCONF; init_timer(&sport->timer); sport->timer.function = imx_timeout;
The transmitter is expected to be controlled by the UART's RTS pin. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/tty/serial/imx.c | 94 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 81 insertions(+), 13 deletions(-)