diff mbox series

[v9,1/3] serial: sc16is7xx: announce support of SER_RS485_RTS_ON_SEND

Message ID 20240425183251.174412-2-rilian.la.te@ya.ru
State Superseded
Headers show
Series add support for EXAR XR20M1172 UART | expand

Commit Message

Konstantin Pugin April 25, 2024, 6:32 p.m. UTC
From: Konstantin Pugin <ria.freelander@gmail.com>

When specifying flag SER_RS485_RTS_ON_SEND in RS485 configuration,
we get the following warning after commit 4afeced55baa ("serial: core:
fix sanitizing check for RTS settings"):

    invalid RTS setting, using RTS_AFTER_SEND instead

This results in SER_RS485_RTS_AFTER_SEND being set and the
driver always write to the register field SC16IS7XX_EFCR_RTS_INVERT_BIT,
which breaks some hardware using these chips.

The hardware supports both RTS_ON_SEND and RTS_AFTER_SEND modes, so fix
this by announcing support for RTS_ON_SEND.

Fixes: 267913ecf737 ("serial: sc16is7xx: Fill in rs485_supported")
Tested-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Reviewed-by: Andy Shevchenko <andy@kernel.org>
Signed-off-by: Konstantin Pugin <ria.freelander@gmail.com>
---
 drivers/tty/serial/sc16is7xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andy Shevchenko April 26, 2024, 2:32 p.m. UTC | #1
On Thu, Apr 25, 2024 at 09:32:33PM +0300, Konstantin Pugin wrote:
> From: Konstantin Pugin <ria.freelander@gmail.com>
> 
> When specifying flag SER_RS485_RTS_ON_SEND in RS485 configuration,
> we get the following warning after commit 4afeced55baa ("serial: core:
> fix sanitizing check for RTS settings"):
> 
>     invalid RTS setting, using RTS_AFTER_SEND instead
> 
> This results in SER_RS485_RTS_AFTER_SEND being set and the
> driver always write to the register field SC16IS7XX_EFCR_RTS_INVERT_BIT,
> which breaks some hardware using these chips.
> 
> The hardware supports both RTS_ON_SEND and RTS_AFTER_SEND modes, so fix
> this by announcing support for RTS_ON_SEND.

Greg KH, who is maintainer of TTY/serial subsystem, usually asks to separate
fixes from new features. So, sending this patch separately may not only help
him, but let's move forward with your stuff.
Konstantin P. April 26, 2024, 3:02 p.m. UTC | #2
On Fri, Apr 26, 2024 at 5:36 PM Andy Shevchenko <andy@kernel.org> wrote:
>
> On Thu, Apr 25, 2024 at 09:32:33PM +0300, Konstantin Pugin wrote:
> > From: Konstantin Pugin <ria.freelander@gmail.com>
> >
> > When specifying flag SER_RS485_RTS_ON_SEND in RS485 configuration,
> > we get the following warning after commit 4afeced55baa ("serial: core:
> > fix sanitizing check for RTS settings"):
> >
> >     invalid RTS setting, using RTS_AFTER_SEND instead
> >
> > This results in SER_RS485_RTS_AFTER_SEND being set and the
> > driver always write to the register field SC16IS7XX_EFCR_RTS_INVERT_BIT,
> > which breaks some hardware using these chips.
> >
> > The hardware supports both RTS_ON_SEND and RTS_AFTER_SEND modes, so fix
> > this by announcing support for RTS_ON_SEND.
>
> Greg KH, who is maintainer of TTY/serial subsystem, usually asks to separate
> fixes from new features. So, sending this patch separately may not only help
> him, but let's move forward with your stuff.
>

Do I need to increase the version number in split send? And if I need
to do so, then how I should do it? Only on new driver? Or only on fix?
Should I CC linux-stable in fix patch?
> --
> With Best Regards,
> Andy Shevchenko
>
>
Andy Shevchenko April 26, 2024, 3:10 p.m. UTC | #3
On Fri, Apr 26, 2024 at 6:06 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Apr 26, 2024 at 6:00 PM Konstantin P. <ria.freelander@gmail.com> wrote:
> > On Fri, Apr 26, 2024 at 5:36 PM Andy Shevchenko <andy@kernel.org> wrote:
> > > On Thu, Apr 25, 2024 at 09:32:33PM +0300, Konstantin Pugin wrote:

...

> > > Greg KH, who is maintainer of TTY/serial subsystem, usually asks to separate
> > > fixes from new features. So, sending this patch separately may not only help
> > > him, but let's move forward with your stuff.
> >
> > Do I need to increase the version number in split send?
>
> Nope, the opposite, i.e. drop it to v1 (and mention in the comments
> area, that's after the cutter '---' line, that it's a split from this
> series).
>
> >  And if I need
> > to do so, then how I should do it? Only on new driver? Or only on fix?
> > Should I CC linux-stable in fix patch?
>
> Everything else is documented:
> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

Just to be crystal clear:
- the split of the fix goes as v1
- the rest as v10 (or what is the number of the next revision?) after
you address all review comments, confirmed testing, etc.
diff mbox series

Patch

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 03cf30e20b75..dfcc804f558f 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1449,7 +1449,7 @@  static int sc16is7xx_setup_mctrl_ports(struct sc16is7xx_port *s,
 }
 
 static const struct serial_rs485 sc16is7xx_rs485_supported = {
-	.flags = SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND,
+	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND,
 	.delay_rts_before_send = 1,
 	.delay_rts_after_send = 1,	/* Not supported but keep returning -EINVAL */
 };