Message ID | 1426548529-21738-4-git-send-email-moorray3@wp.pl |
---|---|
State | Not Applicable |
Headers | show |
This makes sense. I did have to fix up my user space app to set SER_RS485_RTS_ON_SEND after applying this patch. On Mon, 16 Mar 2015, Jakub Kicinski wrote: > From: Jakub Kicinski <kubakici@wp.pl> > > Hardware is capable of inverting RTS signal when working > in RS-485 mode. Expose this functionality to user space. > Relay on a matching combination of standard flags > (SER_RS485_RTS_ON_SEND and SER_RS485_RTS_AFTER_SEND) to > detect when user space is requesting inverted RTS mode. > > Signed-off-by: Jakub Kicinski <kubakici@wp.pl> Signed-off-by: Jon Ringle <jringle@gridpoint.com> > --- > drivers/tty/serial/sc16is7xx.c | 32 +++++++++++++++++++++++--------- > 1 file changed, 23 insertions(+), 9 deletions(-) > > diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c > index e6b396e584f3..24902d589b07 100644 > --- a/drivers/tty/serial/sc16is7xx.c > +++ b/drivers/tty/serial/sc16is7xx.c > @@ -829,16 +829,30 @@ static void sc16is7xx_set_termios(struct uart_port *port, > } > > static int sc16is7xx_config_rs485(struct uart_port *port, > - struct serial_rs485 *rs485) > + struct serial_rs485 *rs485) > { > - if (port->rs485.flags & SER_RS485_ENABLED) > - sc16is7xx_port_update(port, SC16IS7XX_EFCR_REG, > - SC16IS7XX_EFCR_AUTO_RS485_BIT, > - SC16IS7XX_EFCR_AUTO_RS485_BIT); > - else > - sc16is7xx_port_update(port, SC16IS7XX_EFCR_REG, > - SC16IS7XX_EFCR_AUTO_RS485_BIT, > - 0); > + const u32 mask = SC16IS7XX_EFCR_AUTO_RS485_BIT | > + SC16IS7XX_EFCR_RTS_INVERT_BIT; > + u32 efcr = 0; > + > + if (rs485->flags & SER_RS485_ENABLED) { > + bool rts_during_rx, rts_during_tx; > + > + rts_during_rx = rs485->flags & SER_RS485_RTS_AFTER_SEND; > + rts_during_tx = rs485->flags & SER_RS485_RTS_ON_SEND; > + > + efcr |= SC16IS7XX_EFCR_AUTO_RS485_BIT; > + > + if (!rts_during_rx && rts_during_tx) > + /* default */; > + else if (rts_during_rx && !rts_during_tx) > + efcr |= SC16IS7XX_EFCR_RTS_INVERT_BIT; > + else > + return -EINVAL; > + } > + > + sc16is7xx_port_update(port, SC16IS7XX_EFCR_REG, mask, efcr); > + > port->rs485 = *rs485; > > return 0; > -- > 2.1.0 > > -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 17 Mar 2015 10:45:26 -0400 (EDT), Jon Ringle wrote: > This makes sense. I did have to fix up my user space app to set > SER_RS485_RTS_ON_SEND after applying this patch. Yes, perhaps I should have mentioned that this makes the ioctl return -EINVAL when neither of SER_RS485_RTS_*_SEND flags is set. This definitely has potential to break people's apps. Let's see if anyone else has an opinion on this. I am also very tempted to remove rs485.delay_rts_*_send mdelays from this driver and return -EINVAL in a similar fashion when there are set. Normally when the driver is responsible for wiggling the GPIO/RTS line mdelays can indeed impact when the lines change state but in sc16is7xx case transitions are controlled by HW and there is nothing the driver can do about the the relation between TX/RX mode switch and state of RTS lines... Do you agree? -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" 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 03:57:00PM +0100, Jakub Kiciński wrote: > On Tue, 17 Mar 2015 10:45:26 -0400 (EDT), Jon Ringle wrote: > > This makes sense. I did have to fix up my user space app to set > > SER_RS485_RTS_ON_SEND after applying this patch. > > Yes, perhaps I should have mentioned that this makes the ioctl return > -EINVAL when neither of SER_RS485_RTS_*_SEND flags is set. This > definitely has potential to break people's apps. Let's see if anyone > else has an opinion on this. Yes, don't break people's apps. I can't take this, sorry. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 26 Mar 2015 22:37:01 +0100, Greg Kroah-Hartman wrote: > On Tue, Mar 17, 2015 at 03:57:00PM +0100, Jakub Kiciński wrote: > > On Tue, 17 Mar 2015 10:45:26 -0400 (EDT), Jon Ringle wrote: > > > This makes sense. I did have to fix up my user space app to set > > > SER_RS485_RTS_ON_SEND after applying this patch. > > > > Yes, perhaps I should have mentioned that this makes the ioctl return > > -EINVAL when neither of SER_RS485_RTS_*_SEND flags is set. This > > definitely has potential to break people's apps. Let's see if anyone > > else has an opinion on this. > > Yes, don't break people's apps. Do you mean it would be OK if I ignore the incorrect combinations instead of returning -EINVAL or is any user-visible change in the behaviour of the driver unacceptable? -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index e6b396e584f3..24902d589b07 100644 --- a/drivers/tty/serial/sc16is7xx.c +++ b/drivers/tty/serial/sc16is7xx.c @@ -829,16 +829,30 @@ static void sc16is7xx_set_termios(struct uart_port *port, } static int sc16is7xx_config_rs485(struct uart_port *port, - struct serial_rs485 *rs485) + struct serial_rs485 *rs485) { - if (port->rs485.flags & SER_RS485_ENABLED) - sc16is7xx_port_update(port, SC16IS7XX_EFCR_REG, - SC16IS7XX_EFCR_AUTO_RS485_BIT, - SC16IS7XX_EFCR_AUTO_RS485_BIT); - else - sc16is7xx_port_update(port, SC16IS7XX_EFCR_REG, - SC16IS7XX_EFCR_AUTO_RS485_BIT, - 0); + const u32 mask = SC16IS7XX_EFCR_AUTO_RS485_BIT | + SC16IS7XX_EFCR_RTS_INVERT_BIT; + u32 efcr = 0; + + if (rs485->flags & SER_RS485_ENABLED) { + bool rts_during_rx, rts_during_tx; + + rts_during_rx = rs485->flags & SER_RS485_RTS_AFTER_SEND; + rts_during_tx = rs485->flags & SER_RS485_RTS_ON_SEND; + + efcr |= SC16IS7XX_EFCR_AUTO_RS485_BIT; + + if (!rts_during_rx && rts_during_tx) + /* default */; + else if (rts_during_rx && !rts_during_tx) + efcr |= SC16IS7XX_EFCR_RTS_INVERT_BIT; + else + return -EINVAL; + } + + sc16is7xx_port_update(port, SC16IS7XX_EFCR_REG, mask, efcr); + port->rs485 = *rs485; return 0;