[REGRESSION][PATCH] serial: 8250_pci: Fix broken RS485 for F81504/508/512

[REGRESSION][PATCH] serial: 8250_pci: Fix broken RS485 for F81504/508/512

[Marnix Rijnart]

Commit 4afeced ("serial: core: fix sanitizing check for RTS settings")
introduced a regression making it impossible to unset
SER_RS485_RTS_ON_SEND from userspace if SER_RS485_RTS_AFTER_SEND is
unsupported. Because these devices need RTS to be low on TX (fecf27a)
they are effectively broken.

The hardware supports both RTS_ON_SEND and RTS_AFTER_SEND,
so fix this by announcing support for SER_RS485_RTS_AFTER_SEND,
similar to commit 068d35a.

Signed-off-by: Marnix Rijnart <marnix.rijnart@iwell.eu>
---
 drivers/tty/serial/8250/8250_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 152f914c599d..a9da222bd174 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1645,7 +1645,7 @@ static int pci_fintek_rs485_config(struct uart_port *port, struct ktermios *term
 }
 
 static const struct serial_rs485 pci_fintek_rs485_supported = {
-	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND,
+	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND,
 	/* F81504/508/512 does not support RTS delay before or after send */
 };
 
-- 
2.51.0

Re: [REGRESSION][PATCH] serial: 8250_pci: Fix broken RS485 for F81504/508/512

[Ilpo Järvinen]

On Wed, 24 Sep 2025, Marnix Rijnart wrote:

> Commit 4afeced ("serial: core: fix sanitizing check for RTS settings")
> introduced a regression making it impossible to unset
> SER_RS485_RTS_ON_SEND from userspace if SER_RS485_RTS_AFTER_SEND is
> unsupported. Because these devices need RTS to be low on TX (fecf27a)
> they are effectively broken.
> 
> The hardware supports both RTS_ON_SEND and RTS_AFTER_SEND,
> so fix this by announcing support for SER_RS485_RTS_AFTER_SEND,
> similar to commit 068d35a.
> 
> Signed-off-by: Marnix Rijnart <marnix.rijnart@iwell.eu>

The Fixes tag is missing?

(And Cc stable tag will be necessary as well).

> ---
>  drivers/tty/serial/8250/8250_pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index 152f914c599d..a9da222bd174 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -1645,7 +1645,7 @@ static int pci_fintek_rs485_config(struct uart_port *port, struct ktermios *term
>  }
>  
>  static const struct serial_rs485 pci_fintek_rs485_supported = {
> -	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND,
> +	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND,
>  	/* F81504/508/512 does not support RTS delay before or after send */
>  };
>  
> 

-- 
 i.

Re: [REGRESSION][PATCH] serial: 8250_pci: Fix broken RS485 for F81504/508/512

[Marnix Rijnart]

This is my first interaction with the kernel community, so I did my
best to read up on the expected practices but maybe I missed
something.

I didn't add a Fixes tag for commit 4afeced because it doesn't cause
the bug by itself, it's the interaction between it and an older commit
(456d523) that create the issue together. What's the right way to tag
this?

I didn't Cc stable because I understood this to be only required for
point release regressions within a stable series
(https://docs.kernel.org/admin-guide/reporting-regressions.html).
Release 6.8 introduced this regression. Should I still Cc them?

If these tags need to added, what's the best way to do so? Resubmit
the patch (as a v2)?

Thanks,
Marnix

Op di 30 sep 2025 om 13:32 schreef Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com>:
>
> On Wed, 24 Sep 2025, Marnix Rijnart wrote:
>
> > Commit 4afeced ("serial: core: fix sanitizing check for RTS settings")
> > introduced a regression making it impossible to unset
> > SER_RS485_RTS_ON_SEND from userspace if SER_RS485_RTS_AFTER_SEND is
> > unsupported. Because these devices need RTS to be low on TX (fecf27a)
> > they are effectively broken.
> >
> > The hardware supports both RTS_ON_SEND and RTS_AFTER_SEND,
> > so fix this by announcing support for SER_RS485_RTS_AFTER_SEND,
> > similar to commit 068d35a.
> >
> > Signed-off-by: Marnix Rijnart <marnix.rijnart@iwell.eu>
>
> The Fixes tag is missing?
>
> (And Cc stable tag will be necessary as well).
>
> > ---
> >  drivers/tty/serial/8250/8250_pci.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> > index 152f914c599d..a9da222bd174 100644
> > --- a/drivers/tty/serial/8250/8250_pci.c
> > +++ b/drivers/tty/serial/8250/8250_pci.c
> > @@ -1645,7 +1645,7 @@ static int pci_fintek_rs485_config(struct uart_port *port, struct ktermios *term
> >  }
> >
> >  static const struct serial_rs485 pci_fintek_rs485_supported = {
> > -     .flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND,
> > +     .flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND,
> >       /* F81504/508/512 does not support RTS delay before or after send */
> >  };
> >
> >
>
> --
>  i.
>

Re: [REGRESSION][PATCH] serial: 8250_pci: Fix broken RS485 for F81504/508/512

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 3492 bytes --]

On Tue, 30 Sep 2025, Marnix Rijnart wrote:

> This is my first interaction with the kernel community, so I did my
> best to read up on the expected practices but maybe I missed
> something.

Hi,

No need to feel down because of it. We try to help submitters to improve 
their patches and even more so when the submitter is a newcomer.

(And I'm sorry this reply got stuck into my postponed messages for more 
than a week.)

> I didn't add a Fixes tag for commit 4afeced because it doesn't cause
> the bug by itself, it's the interaction between it and an older commit
> (456d523) that create the issue together. What's the right way to tag
> this?

It is fixing the newer interacting commit too as it failed to consider 
the older commit properly.

You can add two Fixes tags in such a case.

> I didn't Cc stable because I understood this to be only required for
> point release regressions within a stable series
> (https://docs.kernel.org/admin-guide/reporting-regressions.html).
> Release 6.8 introduced this regression. Should I still Cc them?

If a problematic commit is old enough to be in any stable kernel, please 
add Cc stable. (If the problematic commit would have been added only in 
the latest cycle and only be in -rcX releases, stable needs not to be 
Cc'ed.)

Here's some guidance on submitting patches that are to be included also 
into stable kernels:

Documentation/process/stable-kernel-rules.rst

> If these tags need to added, what's the best way to do so? Resubmit
> the patch (as a v2)?

Yes, update and send v2. Also remember to include the changes you've made
in v2 under --- line (it's sort of version history of the patch itself, 
see the ML archives for examples if you don't know what I'm talking about).

-- 
 i.

> 
> Thanks,
> Marnix
> 
> Op di 30 sep 2025 om 13:32 schreef Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com>:
> >
> > On Wed, 24 Sep 2025, Marnix Rijnart wrote:
> >
> > > Commit 4afeced ("serial: core: fix sanitizing check for RTS settings")
> > > introduced a regression making it impossible to unset
> > > SER_RS485_RTS_ON_SEND from userspace if SER_RS485_RTS_AFTER_SEND is
> > > unsupported. Because these devices need RTS to be low on TX (fecf27a)
> > > they are effectively broken.
> > >
> > > The hardware supports both RTS_ON_SEND and RTS_AFTER_SEND,
> > > so fix this by announcing support for SER_RS485_RTS_AFTER_SEND,
> > > similar to commit 068d35a.
> > >
> > > Signed-off-by: Marnix Rijnart <marnix.rijnart@iwell.eu>
> >
> > The Fixes tag is missing?
> >
> > (And Cc stable tag will be necessary as well).
> >
> > > ---
> > >  drivers/tty/serial/8250/8250_pci.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> > > index 152f914c599d..a9da222bd174 100644
> > > --- a/drivers/tty/serial/8250/8250_pci.c
> > > +++ b/drivers/tty/serial/8250/8250_pci.c
> > > @@ -1645,7 +1645,7 @@ static int pci_fintek_rs485_config(struct uart_port *port, struct ktermios *term
> > >  }
> > >
> > >  static const struct serial_rs485 pci_fintek_rs485_supported = {
> > > -     .flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND,
> > > +     .flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND,
> > >       /* F81504/508/512 does not support RTS delay before or after send */
> > >  };
> > >
> > >
> >
> > --
> >  i.
> >
>