[PATCH] can: m_can: reset IR_RF0L in message reception loop

[PATCH] can: m_can: reset IR_RF0L in message reception loop

[Mariusz Madej]

Message lost warning is reported in loop without interrupt reset. Besides
redundant log messages it may lead to serious performance problem, where
fifo gets full faster than next reception is scheduled by NAPI. This
patch fixes it.

Signed-off-by: Mariusz Madej <mariusz.madej@xtrack.com>
---
  drivers/net/can/m_can/m_can.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 3752520a7..bd5539435 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -501,8 +501,10 @@ static int m_can_do_rx_poll(struct net_device *dev, int quota)
  	}
  
  	while ((rxfs & RXFS_FFL_MASK) && (quota > 0)) {
-		if (rxfs & RXFS_RFL)
+		if (rxfs & RXFS_RFL) {
  			netdev_warn(dev, "Rx FIFO 0 Message Lost\n");
+			m_can_write(cdev, M_CAN_IR, IR_RF0L);	
+		}
  
  		m_can_read_fifo(dev, rxfs);
  
-- 
2.20.1

Re: [PATCH] can: m_can: reset IR_RF0L in message reception loop

[Torin Cooper-Bennun]

On Mon, Mar 01, 2021 at 10:21:27PM +0100, Mariusz Madej wrote:
> Message lost warning is reported in loop without interrupt reset. Besides
> redundant log messages it may lead to serious performance problem, where
> fifo gets full faster than next reception is scheduled by NAPI. This
> patch fixes it.

Looking at the flow in m_can_rx_handler, it looks as though
m_can_handle_bus_errors -> m_can_handle_lost_msg already handles message
loss properly, and issues a netdev_err. I wonder whether we can remove
the warning from m_can_do_rx_poll entirely:

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 3752520a7d4b..d783c46cac16 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -501,9 +501,6 @@ static int m_can_do_rx_poll(struct net_device *dev, int quota)
 	}
 
 	while ((rxfs & RXFS_FFL_MASK) && (quota > 0)) {
-		if (rxfs & RXFS_RFL)
-			netdev_warn(dev, "Rx FIFO 0 Message Lost\n");
-
 		m_can_read_fifo(dev, rxfs);
 
 		quota--;

---
Regards,

Torin Cooper-Bennun
Software Engineer | maxiluxsystems.com

Re: [PATCH] can: m_can: reset IR_RF0L in message reception loop

[Marc Kleine-Budde]

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

On 03.03.2021 08:08:09, Torin Cooper-Bennun wrote:
> On Mon, Mar 01, 2021 at 10:21:27PM +0100, Mariusz Madej wrote:
> > Message lost warning is reported in loop without interrupt reset. Besides
> > redundant log messages it may lead to serious performance problem, where
> > fifo gets full faster than next reception is scheduled by NAPI. This
> > patch fixes it.
> 
> Looking at the flow in m_can_rx_handler, it looks as though
> m_can_handle_bus_errors -> m_can_handle_lost_msg already handles message
> loss properly, and issues a netdev_err. I wonder whether we can remove
> the warning from m_can_do_rx_poll entirely:
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 3752520a7d4b..d783c46cac16 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -501,9 +501,6 @@ static int m_can_do_rx_poll(struct net_device *dev, int quota)
>  	}
>  
>  	while ((rxfs & RXFS_FFL_MASK) && (quota > 0)) {
> -		if (rxfs & RXFS_RFL)
> -			netdev_warn(dev, "Rx FIFO 0 Message Lost\n");
> -
>  		m_can_read_fifo(dev, rxfs);
>  
>  		quota--;
> 

Looks good to me. Can you send a proper patch?

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] can: m_can: reset IR_RF0L in message reception loop

[Torin Cooper-Bennun]

On Wed, Mar 03, 2021 at 09:31:36AM +0100, Marc Kleine-Budde wrote:
> > Looking at the flow in m_can_rx_handler, it looks as though
> > m_can_handle_bus_errors -> m_can_handle_lost_msg already handles message
> > loss properly, and issues a netdev_err. I wonder whether we can remove
> > the warning from m_can_do_rx_poll entirely:
> > 
> > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> > index 3752520a7d4b..d783c46cac16 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -501,9 +501,6 @@ static int m_can_do_rx_poll(struct net_device *dev, int quota)
> >  	}
> >  
> >  	while ((rxfs & RXFS_FFL_MASK) && (quota > 0)) {
> > -		if (rxfs & RXFS_RFL)
> > -			netdev_warn(dev, "Rx FIFO 0 Message Lost\n");
> > -
> >  		m_can_read_fifo(dev, rxfs);
> >  
> >  		quota--;
> > 
> 
> Looks good to me. Can you send a proper patch?

Of course, will do shortly.

--
Regards,

Torin Cooper-Bennun
Software Engineer | maxiluxsystems.com