[v4] serial: 8250_dw: Preserve original value of DLF register

[PATCH v4] serial: 8250_dw: Preserve original value of DLF register

Posted by Ruihong Luo 2 years, 6 months ago

Preserve the original value of the Divisor Latch Fraction (DLF) register.
When the DLF register is modified without preservation, it can disrupt
the baudrate settings established by firmware or bootloader, leading to
data corruption and the generation of unreadable or distorted characters.

Fixes: 701c5e73b296 ("serial: 8250_dw: add fractional divisor support")
Signed-off-by: Ruihong Luo <colorsu1922@gmail.com>
---
v4:
* Use the old_dlf to hold the DLF register value

 drivers/tty/serial/8250/8250_dwlib.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
index 75f32f054ebb..84843e204a5e 100644
--- a/drivers/tty/serial/8250/8250_dwlib.c
+++ b/drivers/tty/serial/8250/8250_dwlib.c
@@ -244,7 +244,7 @@ void dw8250_setup_port(struct uart_port *p)
 	struct dw8250_port_data *pd = p->private_data;
 	struct dw8250_data *data = to_dw8250_data(pd);
 	struct uart_8250_port *up = up_to_u8250p(p);
-	u32 reg;
+	u32 reg, old_dlf;
 
 	pd->hw_rs485_support = dw8250_detect_rs485_hw(p);
 	if (pd->hw_rs485_support) {
@@ -270,9 +270,11 @@ void dw8250_setup_port(struct uart_port *p)
 	dev_dbg(p->dev, "Designware UART version %c.%c%c\n",
 		(reg >> 24) & 0xff, (reg >> 16) & 0xff, (reg >> 8) & 0xff);
 
+	/* Preserve value written by firmware or bootloader  */
+	old_dlf = dw8250_readl_ext(p, DW_UART_DLF);
 	dw8250_writel_ext(p, DW_UART_DLF, ~0U);
 	reg = dw8250_readl_ext(p, DW_UART_DLF);
-	dw8250_writel_ext(p, DW_UART_DLF, 0);
+	dw8250_writel_ext(p, DW_UART_DLF, old_dlf);
 
 	if (reg) {
 		pd->dlf_size = fls(reg);
-- 
2.39.2

Re: [PATCH v4] serial: 8250_dw: Preserve original value of DLF register

Posted by Andy Shevchenko 2 years, 6 months ago

On Thu, Jul 13, 2023 at 08:42:36AM +0800, Ruihong Luo wrote:
> Preserve the original value of the Divisor Latch Fraction (DLF) register.
> When the DLF register is modified without preservation, it can disrupt
> the baudrate settings established by firmware or bootloader, leading to
> data corruption and the generation of unreadable or distorted characters.

You forgot to add my tag. Why? Do you think the name of variable warrants this?
Whatever,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Next time if you don't pick up somebody's tag, care to explain in the changelog
why.

> Fixes: 701c5e73b296 ("serial: 8250_dw: add fractional divisor support")
> Signed-off-by: Ruihong Luo <colorsu1922@gmail.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4] serial: 8250_dw: Preserve original value of DLF register

Posted by Ilpo Järvinen 2 years, 6 months ago

On Thu, 13 Jul 2023, Ruihong Luo wrote:

> Preserve the original value of the Divisor Latch Fraction (DLF) register.
> When the DLF register is modified without preservation, it can disrupt
> the baudrate settings established by firmware or bootloader, leading to
> data corruption and the generation of unreadable or distorted characters.
> 
> Fixes: 701c5e73b296 ("serial: 8250_dw: add fractional divisor support")

You forgot to add:

Cc: stable@vger.kernel.org

> Signed-off-by: Ruihong Luo <colorsu1922@gmail.com>

Other than that,

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

> ---
> v4:
> * Use the old_dlf to hold the DLF register value
> 
>  drivers/tty/serial/8250/8250_dwlib.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
> index 75f32f054ebb..84843e204a5e 100644
> --- a/drivers/tty/serial/8250/8250_dwlib.c
> +++ b/drivers/tty/serial/8250/8250_dwlib.c
> @@ -244,7 +244,7 @@ void dw8250_setup_port(struct uart_port *p)
>  	struct dw8250_port_data *pd = p->private_data;
>  	struct dw8250_data *data = to_dw8250_data(pd);
>  	struct uart_8250_port *up = up_to_u8250p(p);
> -	u32 reg;
> +	u32 reg, old_dlf;
>  
>  	pd->hw_rs485_support = dw8250_detect_rs485_hw(p);
>  	if (pd->hw_rs485_support) {
> @@ -270,9 +270,11 @@ void dw8250_setup_port(struct uart_port *p)
>  	dev_dbg(p->dev, "Designware UART version %c.%c%c\n",
>  		(reg >> 24) & 0xff, (reg >> 16) & 0xff, (reg >> 8) & 0xff);
>  
> +	/* Preserve value written by firmware or bootloader  */
> +	old_dlf = dw8250_readl_ext(p, DW_UART_DLF);
>  	dw8250_writel_ext(p, DW_UART_DLF, ~0U);
>  	reg = dw8250_readl_ext(p, DW_UART_DLF);
> -	dw8250_writel_ext(p, DW_UART_DLF, 0);
> +	dw8250_writel_ext(p, DW_UART_DLF, old_dlf);
>  
>  	if (reg) {
>  		pd->dlf_size = fls(reg);
>

Re: [PATCH v4] serial: 8250_dw: Preserve original value of DLF register

Posted by Andy Shevchenko 2 years, 6 months ago

On Fri, Jul 14, 2023 at 03:07:42PM +0300, Ilpo Järvinen wrote:
> On Thu, 13 Jul 2023, Ruihong Luo wrote:
> 
> > Preserve the original value of the Divisor Latch Fraction (DLF) register.
> > When the DLF register is modified without preservation, it can disrupt
> > the baudrate settings established by firmware or bootloader, leading to
> > data corruption and the generation of unreadable or distorted characters.
> > 
> > Fixes: 701c5e73b296 ("serial: 8250_dw: add fractional divisor support")

> You forgot to add:
> 
> Cc: stable@vger.kernel.org

It's there. Just not in the commit message. It's fine.

> > Signed-off-by: Ruihong Luo <colorsu1922@gmail.com>
> 
> Other than that,
> 
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4] serial: 8250_dw: Preserve original value of DLF register

Posted by Greg Kroah-Hartman 2 years, 6 months ago

On Fri, Jul 14, 2023 at 03:31:11PM +0300, Andy Shevchenko wrote:
> On Fri, Jul 14, 2023 at 03:07:42PM +0300, Ilpo Järvinen wrote:
> > On Thu, 13 Jul 2023, Ruihong Luo wrote:
> > 
> > > Preserve the original value of the Divisor Latch Fraction (DLF) register.
> > > When the DLF register is modified without preservation, it can disrupt
> > > the baudrate settings established by firmware or bootloader, leading to
> > > data corruption and the generation of unreadable or distorted characters.
> > > 
> > > Fixes: 701c5e73b296 ("serial: 8250_dw: add fractional divisor support")
> 
> > You forgot to add:
> > 
> > Cc: stable@vger.kernel.org
> 
> It's there. Just not in the commit message. It's fine.

No it isn't, that's not how to have a patch added to the stable tree, as
my form letter says:

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

I'll go fix it up by hand...

Re: [PATCH v4] serial: 8250_dw: Preserve original value of DLF register

Posted by Andy Shevchenko 2 years, 6 months ago

On Tue, Jul 25, 2023 at 08:31:17PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 14, 2023 at 03:31:11PM +0300, Andy Shevchenko wrote:
> > On Fri, Jul 14, 2023 at 03:07:42PM +0300, Ilpo Järvinen wrote:
> > > On Thu, 13 Jul 2023, Ruihong Luo wrote:
> > > 
> > > > Preserve the original value of the Divisor Latch Fraction (DLF) register.
> > > > When the DLF register is modified without preservation, it can disrupt
> > > > the baudrate settings established by firmware or bootloader, leading to
> > > > data corruption and the generation of unreadable or distorted characters.
> > > > 
> > > > Fixes: 701c5e73b296 ("serial: 8250_dw: add fractional divisor support")
> > 
> > > You forgot to add:
> > > 
> > > Cc: stable@vger.kernel.org
> > 
> > It's there. Just not in the commit message. It's fine.
> 
> No it isn't, that's not how to have a patch added to the stable tree, as
> my form letter says:
> 
> <formletter>
> 
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
> 
> </formletter>
> 
> I'll go fix it up by hand...

Good to know, thank you for the clarification!


-- 
With Best Regards,
Andy Shevchenko