[v2] serial: 8250_omap: Implementation of Errata i2310

[PATCH v2] serial: 8250_omap: Implementation of Errata i2310

Posted by Udit Kumar 1 year, 6 months ago

As per Errata i2310[0], Erroneous timeout can be triggered,
if this Erroneous interrupt is not cleared then it may leads
to storm of interrupts, therefore apply Errata i2310 solution.

[0] https://www.ti.com/lit/pdf/sprz536 page 23

Fixes: b67e830d38fa ("serial: 8250: 8250_omap: Fix possible interrupt storm on K3 SoCs")
Signed-off-by: Udit Kumar <u-kumar1@ti.com>
---
Test logs
https://gist.github.com/uditkumarti/7a7ab6994581bbb09cf1a4271c592d8e

Change logs
Changes in v2:
- Added Fixes Tag and typo correction in commit message
- Corrected bit position to UART_OMAP_EFR2_TIMEOUT_BEHAVE
Link to v1
https://lore.kernel.org/all/20240614061314.290840-1-u-kumar1@ti.com/

 drivers/tty/serial/8250/8250_omap.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 170639d12b2a..ddac0a13cf84 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -115,6 +115,10 @@
 /* RX FIFO occupancy indicator */
 #define UART_OMAP_RX_LVL		0x19
 
+/* Timeout low and High */
+#define UART_OMAP_TO_L                 0x26
+#define UART_OMAP_TO_H                 0x27
+
 /*
  * Copy of the genpd flags for the console.
  * Only used if console suspend is disabled
@@ -663,13 +667,24 @@ static irqreturn_t omap8250_irq(int irq, void *dev_id)
 
 	/*
 	 * On K3 SoCs, it is observed that RX TIMEOUT is signalled after
-	 * FIFO has been drained, in which case a dummy read of RX FIFO
-	 * is required to clear RX TIMEOUT condition.
+	 * FIFO has been drained or erroneously.
+	 * So apply solution of Errata i2310 as mentioned in
+	 * https://www.ti.com/lit/pdf/sprz536
 	 */
 	if (priv->habit & UART_RX_TIMEOUT_QUIRK &&
-	    (iir & UART_IIR_RX_TIMEOUT) == UART_IIR_RX_TIMEOUT &&
-	    serial_port_in(port, UART_OMAP_RX_LVL) == 0) {
-		serial_port_in(port, UART_RX);
+		(iir & UART_IIR_RX_TIMEOUT) == UART_IIR_RX_TIMEOUT) {
+		unsigned char efr2, timeout_h, timeout_l;
+
+		efr2 = serial_in(up, UART_OMAP_EFR2);
+		timeout_h = serial_in(up, UART_OMAP_TO_H);
+		timeout_l = serial_in(up, UART_OMAP_TO_L);
+		serial_out(up, UART_OMAP_TO_H, 0xFF);
+		serial_out(up, UART_OMAP_TO_L, 0xFF);
+		serial_out(up, UART_OMAP_EFR2, UART_OMAP_EFR2_TIMEOUT_BEHAVE);
+		serial_in(up, UART_IIR);
+		serial_out(up, UART_OMAP_EFR2, efr2);
+		serial_out(up, UART_OMAP_TO_H, timeout_h);
+		serial_out(up, UART_OMAP_TO_L, timeout_l);
 	}
 
 	/* Stop processing interrupts on input overrun */
-- 
2.34.1

Re: [PATCH v2] serial: 8250_omap: Implementation of Errata i2310

Posted by Greg KH 1 year, 6 months ago

On Mon, Jun 17, 2024 at 10:52:53AM +0530, Udit Kumar wrote:
> As per Errata i2310[0], Erroneous timeout can be triggered,
> if this Erroneous interrupt is not cleared then it may leads
> to storm of interrupts, therefore apply Errata i2310 solution.
> 
> [0] https://www.ti.com/lit/pdf/sprz536 page 23
> 
> Fixes: b67e830d38fa ("serial: 8250: 8250_omap: Fix possible interrupt storm on K3 SoCs")
> Signed-off-by: Udit Kumar <u-kumar1@ti.com>
> ---
> Test logs
> https://gist.github.com/uditkumarti/7a7ab6994581bbb09cf1a4271c592d8e
> 
> Change logs
> Changes in v2:
> - Added Fixes Tag and typo correction in commit message
> - Corrected bit position to UART_OMAP_EFR2_TIMEOUT_BEHAVE
> Link to v1
> https://lore.kernel.org/all/20240614061314.290840-1-u-kumar1@ti.com/
> 
>  drivers/tty/serial/8250/8250_omap.c | 25 ++++++++++++++++++++-----
>  1 file changed, 20 insertions(+), 5 deletions(-)

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You have marked a patch with a "Fixes:" tag for a commit that is in an
  older released kernel, yet you do not have a cc: stable line in the
  signed-off-by area at all, which means that the patch will not be
  applied to any older kernel releases.  To properly fix this, please
  follow the documented rules in the
  Documentation/process/stable-kernel-rules.rst file for how to resolve
  this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH v2] serial: 8250_omap: Implementation of Errata i2310

Posted by Kumar, Udit 1 year, 6 months ago

Hi Greg

On 6/17/2024 11:52 AM, Greg KH wrote:
> On Mon, Jun 17, 2024 at 10:52:53AM +0530, Udit Kumar wrote:
>> As per Errata i2310[0], Erroneous timeout can be triggered,
>> if this Erroneous interrupt is not cleared then it may leads
>> to storm of interrupts, therefore apply Errata i2310 solution.
>>
>> [0] https://www.ti.com/lit/pdf/sprz536 page 23
>>
>> Fixes: b67e830d38fa ("serial: 8250: 8250_omap: Fix possible interrupt storm on K3 SoCs")
>> Signed-off-by: Udit Kumar <u-kumar1@ti.com>
>> ---
>> Test logs
>> https://gist.github.com/uditkumarti/7a7ab6994581bbb09cf1a4271c592d8e
>>
>> Change logs
>> Changes in v2:
>> - Added Fixes Tag and typo correction in commit message
>> - Corrected bit position to UART_OMAP_EFR2_TIMEOUT_BEHAVE
>> Link to v1
>> https://lore.kernel.org/all/20240614061314.290840-1-u-kumar1@ti.com/
>>
>>   drivers/tty/serial/8250/8250_omap.c | 25 ++++++++++++++++++++-----
>>   1 file changed, 20 insertions(+), 5 deletions(-)
> Hi,
>
> This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
> a patch that has triggered this response.  He used to manually respond
> to these common problems, but in order to save his sanity (he kept
> writing the same thing over and over, yet to different people), I was
> created.  Hopefully you will not take offence and will fix the problem
> in your patch and resubmit it so that it can be accepted into the Linux
> kernel tree.
>
> You are receiving this message because of the following common error(s)
> as indicated below:
>
> - You have marked a patch with a "Fixes:" tag for a commit that is in an
>    older released kernel, yet you do not have a cc: stable line in the
>    signed-off-by area at all, which means that the patch will not be
>    applied to any older kernel releases.  To properly fix this, please
>    follow the documented rules in the
>    Documentation/process/stable-kernel-rules.rst file for how to resolve
>    this.
>
> If you wish to discuss this problem further, or you have questions about
> how to resolve this issue, please feel free to respond to this email and
> Greg will reply once he has dug out from the pending patches received
> from other developers.


I realized the same due to other patches, this not getting applied 
cleanly on stable (using git am).

Let me know if you are ok,  I will port this to stable kernel, once this 
is merged into Linux tree.


>
> thanks,
>
> greg k-h's patch email bot

Re: [PATCH v2] serial: 8250_omap: Implementation of Errata i2310

Posted by Greg KH 1 year, 6 months ago

On Mon, Jun 17, 2024 at 03:17:54PM +0530, Kumar, Udit wrote:
> Hi Greg
> 
> On 6/17/2024 11:52 AM, Greg KH wrote:
> > On Mon, Jun 17, 2024 at 10:52:53AM +0530, Udit Kumar wrote:
> > > As per Errata i2310[0], Erroneous timeout can be triggered,
> > > if this Erroneous interrupt is not cleared then it may leads
> > > to storm of interrupts, therefore apply Errata i2310 solution.
> > > 
> > > [0] https://www.ti.com/lit/pdf/sprz536 page 23
> > > 
> > > Fixes: b67e830d38fa ("serial: 8250: 8250_omap: Fix possible interrupt storm on K3 SoCs")
> > > Signed-off-by: Udit Kumar <u-kumar1@ti.com>
> > > ---
> > > Test logs
> > > https://gist.github.com/uditkumarti/7a7ab6994581bbb09cf1a4271c592d8e
> > > 
> > > Change logs
> > > Changes in v2:
> > > - Added Fixes Tag and typo correction in commit message
> > > - Corrected bit position to UART_OMAP_EFR2_TIMEOUT_BEHAVE
> > > Link to v1
> > > https://lore.kernel.org/all/20240614061314.290840-1-u-kumar1@ti.com/
> > > 
> > >   drivers/tty/serial/8250/8250_omap.c | 25 ++++++++++++++++++++-----
> > >   1 file changed, 20 insertions(+), 5 deletions(-)
> > Hi,
> > 
> > This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
> > a patch that has triggered this response.  He used to manually respond
> > to these common problems, but in order to save his sanity (he kept
> > writing the same thing over and over, yet to different people), I was
> > created.  Hopefully you will not take offence and will fix the problem
> > in your patch and resubmit it so that it can be accepted into the Linux
> > kernel tree.
> > 
> > You are receiving this message because of the following common error(s)
> > as indicated below:
> > 
> > - You have marked a patch with a "Fixes:" tag for a commit that is in an
> >    older released kernel, yet you do not have a cc: stable line in the
> >    signed-off-by area at all, which means that the patch will not be
> >    applied to any older kernel releases.  To properly fix this, please
> >    follow the documented rules in the
> >    Documentation/process/stable-kernel-rules.rst file for how to resolve
> >    this.
> > 
> > If you wish to discuss this problem further, or you have questions about
> > how to resolve this issue, please feel free to respond to this email and
> > Greg will reply once he has dug out from the pending patches received
> > from other developers.
> 
> 
> I realized the same due to other patches, this not getting applied cleanly
> on stable (using git am).
> 
> Let me know if you are ok,  I will port this to stable kernel, once this is
> merged into Linux tree.

Ok, but if you properly put a cc: stable line on your commit, the stable
team will notify you when/if something fails to apply, no need for you
to try to remember to do it sometime in the next few months.  Please do
that, just adding a "Fixes:" tag never ensures that this will happen at
all.

thanks,
greg k-h