[PATCH 1/1][SRU][M] UBUNTU: SAUCE: platform/x86: dell-uart-backlight: add small delay after write command

[Andrea Righi]

On Thu, Sep 14, 2023 at 02:00:13PM +0200, Andrea Righi wrote:
> On Thu, Sep 14, 2023 at 06:13:41PM +0800, AceLan Kao wrote:
> > From: "Chia-Lin Kao (AceLan)" <acelan.kao at canonical.com>
> > 
> > BugLink: https://bugs.launchpad.net/bug/2035299
> > 
> > After applying fbf84fb368923 ("UBUNTU: SAUCE: platform/x86:
> > dell-uart-backlight: replace chars_in_buffer() with flush_chars()"), it
> > seems that the read() command may fail to receive a response, even when
> > increasing the retry times. It never gets a response when it fails.
> > 
> > To fix this, try adding a small delay after the write() function as a workaround.
> 
> I'm wondering if there's a better way to determine when the "write"
> actually completed, so that we can safely issue a read without doing the
> msleep() that is often a bit unpredictable.
> 
> If msleep() is really the only way to do it, why don't we put that at
> the end of dell_uart_write()? I suppose any write would be affected by
> this issue potentially...
> 
> Thanks,
> -Andrea

Any opinion/update on this? This patch is still unapplied.

Thanks,
-Andrea

> 
> > 
> > Fixes: fbf84fb368923 ("UBUNTU: SAUCE: platform/x86: dell-uart-backlight: replace chars_in_buffer() with flush_chars()")
> > Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao at canonical.com>
> > ---
> >  drivers/platform/x86/dell/dell-uart-backlight.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/platform/x86/dell/dell-uart-backlight.c b/drivers/platform/x86/dell/dell-uart-backlight.c
> > index 120701e5b8b13..c489f7997767f 100644
> > --- a/drivers/platform/x86/dell/dell-uart-backlight.c
> > +++ b/drivers/platform/x86/dell/dell-uart-backlight.c
> > @@ -248,6 +248,7 @@ static int dell_uart_set_bl_power(struct backlight_device *bd, int power)
> >  	}
> >  
> >  	dell_uart_write(uart, bl_cmd->cmd, bl_cmd->tx_len);
> > +	msleep(1);
> >  	rx_len = dell_uart_read(uart, bl_cmd->ret, bl_cmd->rx_len);
> >  
> >  	mutex_unlock(&dell_pdata->brightness_mutex);
> > @@ -275,6 +276,7 @@ static int dell_uart_get_brightness(struct backlight_device *bd)
> >  	}
> >  
> >  	dell_uart_write(uart, bl_cmd->cmd, bl_cmd->tx_len);
> > +	msleep(1);
> >  	rx_len = dell_uart_read(uart, bl_cmd->ret, bl_cmd->rx_len);
> >  
> >  	mutex_unlock(&dell_pdata->brightness_mutex);
> > @@ -304,6 +306,7 @@ static int dell_uart_update_status(struct backlight_device *bd)
> >  	}
> >  
> >  	dell_uart_write(uart, bl_cmd->cmd, bl_cmd->tx_len);
> > +	msleep(1);
> >  	rx_len = dell_uart_read(uart, bl_cmd->ret, bl_cmd->rx_len);
> >  
> >  	mutex_unlock(&dell_pdata->brightness_mutex);
> > @@ -330,6 +333,7 @@ static int dell_uart_show_firmware_ver(struct dell_uart_backlight *dell_pdata)
> >  	}
> >  
> >  	dell_uart_write(uart, bl_cmd->cmd, bl_cmd->tx_len);
> > +	msleep(1);
> >  	while (retry-- > 0) {
> >  		/* first byte is data length */
> >  		dell_uart_read(uart, bl_cmd->ret, 1);
> > @@ -371,6 +375,7 @@ static int dell_uart_get_display_mode(struct dell_uart_backlight *dell_pdata)
> >  		}
> >  
> >  		dell_uart_write(uart, bl_cmd->cmd, bl_cmd->tx_len);
> > +		msleep(1);
> >  		rx_len = dell_uart_read(uart, bl_cmd->ret, bl_cmd->rx_len);
> >  
> >  		mutex_unlock(&dell_pdata->brightness_mutex);
> > -- 
> > 2.34.1
> > 
> > 
> > -- 
> > kernel-team mailing list
> > kernel-team at lists.ubuntu.com
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team