All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] can: at91_can: fix passive-state AERR flooding
@ 2021-10-05 18:30 ` Brandon Maier
  0 siblings, 0 replies; 6+ messages in thread
From: Brandon Maier @ 2021-10-05 18:30 UTC (permalink / raw)
  Cc: Alexandre Belloni, open list, open list:NETWORKING DRIVERS,
	Brandon Maier, open list:CAN NETWORK DRIVERS, Ludovic Desroches,
	Marc Kleine-Budde, moderated list:ARM/Microchip AT91 SoC support,
	Jakub Kicinski, David S. Miller, Wolfgang Grandegger

When the at91_can is a single node on the bus and a user attempts to
transmit, the can state machine will report ack errors and increment the
transmit error count until it reaches the passive-state. Per the
specification, it will then transmit with a passive error, but will stop
incrementing the transmit error count. This results in the host machine
being flooded with the AERR interrupt forever, or until another node
rejoins the bus.

To prevent the AERR flooding, disable the AERR interrupt when we are in
passive mode.

Signed-off-by: Brandon Maier <brandon.maier@rockwellcollins.com>
---
 drivers/net/can/at91_can.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
index b06af90a9964..2a8831127bd0 100644
--- a/drivers/net/can/at91_can.c
+++ b/drivers/net/can/at91_can.c
@@ -804,8 +804,13 @@ static int at91_poll(struct napi_struct *napi, int quota)
 		work_done += at91_poll_err(dev, quota - work_done, reg_sr);
 
 	if (work_done < quota) {
-		/* enable IRQs for frame errors and all mailboxes >= rx_next */
+		/* enable IRQs for frame errors and all mailboxes >= rx_next,
+		 * disable the ack error in passive mode to avoid flooding
+		 * ourselves with interrupts
+		 */
 		u32 reg_ier = AT91_IRQ_ERR_FRAME;
+		if (priv->can.state == CAN_STATE_ERROR_PASSIVE)
+			reg_ier &= ~AT91_IRQ_AERR;
 
 		reg_ier |= get_irq_mb_rx(priv) & ~AT91_MB_MASK(priv->rx_next);
 
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH] can: at91_can: fix passive-state AERR flooding
@ 2021-10-05 18:30 ` Brandon Maier
  0 siblings, 0 replies; 6+ messages in thread
From: Brandon Maier @ 2021-10-05 18:30 UTC (permalink / raw)
  Cc: Brandon Maier, Wolfgang Grandegger, Marc Kleine-Budde,
	David S. Miller, Jakub Kicinski, Nicolas Ferre,
	Alexandre Belloni, Ludovic Desroches,
	open list:CAN NETWORK DRIVERS, open list:NETWORKING DRIVERS,
	moderated list:ARM/Microchip (AT91) SoC support, open list

When the at91_can is a single node on the bus and a user attempts to
transmit, the can state machine will report ack errors and increment the
transmit error count until it reaches the passive-state. Per the
specification, it will then transmit with a passive error, but will stop
incrementing the transmit error count. This results in the host machine
being flooded with the AERR interrupt forever, or until another node
rejoins the bus.

To prevent the AERR flooding, disable the AERR interrupt when we are in
passive mode.

Signed-off-by: Brandon Maier <brandon.maier@rockwellcollins.com>
---
 drivers/net/can/at91_can.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
index b06af90a9964..2a8831127bd0 100644
--- a/drivers/net/can/at91_can.c
+++ b/drivers/net/can/at91_can.c
@@ -804,8 +804,13 @@ static int at91_poll(struct napi_struct *napi, int quota)
 		work_done += at91_poll_err(dev, quota - work_done, reg_sr);
 
 	if (work_done < quota) {
-		/* enable IRQs for frame errors and all mailboxes >= rx_next */
+		/* enable IRQs for frame errors and all mailboxes >= rx_next,
+		 * disable the ack error in passive mode to avoid flooding
+		 * ourselves with interrupts
+		 */
 		u32 reg_ier = AT91_IRQ_ERR_FRAME;
+		if (priv->can.state == CAN_STATE_ERROR_PASSIVE)
+			reg_ier &= ~AT91_IRQ_AERR;
 
 		reg_ier |= get_irq_mb_rx(priv) & ~AT91_MB_MASK(priv->rx_next);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] can: at91_can: fix passive-state AERR flooding
  2021-10-05 18:30 ` Brandon Maier
@ 2021-10-07 13:28   ` Nicolas Ferre
  -1 siblings, 0 replies; 6+ messages in thread
From: Nicolas Ferre @ 2021-10-07 13:28 UTC (permalink / raw)
  To: Brandon Maier
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
	Jakub Kicinski, Alexandre Belloni, Ludovic Desroches,
	open list:CAN NETWORK DRIVERS, open list:NETWORKING DRIVERS,
	moderated list:ARM/Microchip (AT91) SoC support, open list

On 05/10/2021 at 20:30, Brandon Maier wrote:
> When the at91_can is a single node on the bus and a user attempts to
> transmit, the can state machine will report ack errors and increment the
> transmit error count until it reaches the passive-state. Per the
> specification, it will then transmit with a passive error, but will stop
> incrementing the transmit error count. This results in the host machine
> being flooded with the AERR interrupt forever, or until another node
> rejoins the bus.
> 
> To prevent the AERR flooding, disable the AERR interrupt when we are in
> passive mode.
> 
> Signed-off-by: Brandon Maier <brandon.maier@rockwellcollins.com>

Even if I'm not familiar with the matter, the explanation above makes sense:
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Thanks Brandon, best regards,
   Nicolas

> ---
>   drivers/net/can/at91_can.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
> index b06af90a9964..2a8831127bd0 100644
> --- a/drivers/net/can/at91_can.c
> +++ b/drivers/net/can/at91_can.c
> @@ -804,8 +804,13 @@ static int at91_poll(struct napi_struct *napi, int quota)
>                  work_done += at91_poll_err(dev, quota - work_done, reg_sr);
> 
>          if (work_done < quota) {
> -               /* enable IRQs for frame errors and all mailboxes >= rx_next */
> +               /* enable IRQs for frame errors and all mailboxes >= rx_next,
> +                * disable the ack error in passive mode to avoid flooding
> +                * ourselves with interrupts
> +                */
>                  u32 reg_ier = AT91_IRQ_ERR_FRAME;
> +               if (priv->can.state == CAN_STATE_ERROR_PASSIVE)
> +                       reg_ier &= ~AT91_IRQ_AERR;
> 
>                  reg_ier |= get_irq_mb_rx(priv) & ~AT91_MB_MASK(priv->rx_next);
> 
> --
> 2.30.2
> 


-- 
Nicolas Ferre

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] can: at91_can: fix passive-state AERR flooding
@ 2021-10-07 13:28   ` Nicolas Ferre
  0 siblings, 0 replies; 6+ messages in thread
From: Nicolas Ferre @ 2021-10-07 13:28 UTC (permalink / raw)
  To: Brandon Maier
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
	Jakub Kicinski, Alexandre Belloni, Ludovic Desroches,
	open list:CAN NETWORK DRIVERS, open list:NETWORKING DRIVERS,
	moderated list:ARM/Microchip (AT91) SoC support, open list

On 05/10/2021 at 20:30, Brandon Maier wrote:
> When the at91_can is a single node on the bus and a user attempts to
> transmit, the can state machine will report ack errors and increment the
> transmit error count until it reaches the passive-state. Per the
> specification, it will then transmit with a passive error, but will stop
> incrementing the transmit error count. This results in the host machine
> being flooded with the AERR interrupt forever, or until another node
> rejoins the bus.
> 
> To prevent the AERR flooding, disable the AERR interrupt when we are in
> passive mode.
> 
> Signed-off-by: Brandon Maier <brandon.maier@rockwellcollins.com>

Even if I'm not familiar with the matter, the explanation above makes sense:
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Thanks Brandon, best regards,
   Nicolas

> ---
>   drivers/net/can/at91_can.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
> index b06af90a9964..2a8831127bd0 100644
> --- a/drivers/net/can/at91_can.c
> +++ b/drivers/net/can/at91_can.c
> @@ -804,8 +804,13 @@ static int at91_poll(struct napi_struct *napi, int quota)
>                  work_done += at91_poll_err(dev, quota - work_done, reg_sr);
> 
>          if (work_done < quota) {
> -               /* enable IRQs for frame errors and all mailboxes >= rx_next */
> +               /* enable IRQs for frame errors and all mailboxes >= rx_next,
> +                * disable the ack error in passive mode to avoid flooding
> +                * ourselves with interrupts
> +                */
>                  u32 reg_ier = AT91_IRQ_ERR_FRAME;
> +               if (priv->can.state == CAN_STATE_ERROR_PASSIVE)
> +                       reg_ier &= ~AT91_IRQ_AERR;
> 
>                  reg_ier |= get_irq_mb_rx(priv) & ~AT91_MB_MASK(priv->rx_next);
> 
> --
> 2.30.2
> 


-- 
Nicolas Ferre

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] can: at91_can: fix passive-state AERR flooding
  2021-10-05 18:30 ` Brandon Maier
@ 2021-10-17 12:46   ` Marc Kleine-Budde
  -1 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2021-10-17 12:46 UTC (permalink / raw)
  To: Brandon Maier
  Cc: Wolfgang Grandegger, David S. Miller, Jakub Kicinski,
	Nicolas Ferre, Alexandre Belloni, Ludovic Desroches,
	open list:CAN NETWORK DRIVERS, open list:NETWORKING DRIVERS,
	moderated list:ARM/Microchip (AT91) SoC support, open list

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

On 05.10.2021 13:30:23, Brandon Maier wrote:
> When the at91_can is a single node on the bus and a user attempts to
> transmit, the can state machine will report ack errors and increment the
> transmit error count until it reaches the passive-state. Per the
> specification, it will then transmit with a passive error, but will stop
> incrementing the transmit error count. This results in the host machine
> being flooded with the AERR interrupt forever, or until another node
> rejoins the bus.
> 
> To prevent the AERR flooding, disable the AERR interrupt when we are in
> passive mode.

Can you implement Bus Error Reporting?

| https://elixir.bootlin.com/linux/v5.14/source/include/uapi/linux/can/netlink.h#L99

This way the user can control if bus errors, and the ACK error is one of
them, should be reported. Bus error reporting is disabled by default. I
think enabling AT91_IRQ_ERR_FRAME only if CAN_CTRLMODE_BERR_REPORTING is
active should do the trick.

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 --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] can: at91_can: fix passive-state AERR flooding
@ 2021-10-17 12:46   ` Marc Kleine-Budde
  0 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2021-10-17 12:46 UTC (permalink / raw)
  To: Brandon Maier
  Cc: Alexandre Belloni, open list, open list:NETWORKING DRIVERS,
	open list:CAN NETWORK DRIVERS, Ludovic Desroches,
	moderated list:ARM/Microchip (AT91) SoC support, Jakub Kicinski,
	David S. Miller, Wolfgang Grandegger


[-- Attachment #1.1: Type: text/plain, Size: 1272 bytes --]

On 05.10.2021 13:30:23, Brandon Maier wrote:
> When the at91_can is a single node on the bus and a user attempts to
> transmit, the can state machine will report ack errors and increment the
> transmit error count until it reaches the passive-state. Per the
> specification, it will then transmit with a passive error, but will stop
> incrementing the transmit error count. This results in the host machine
> being flooded with the AERR interrupt forever, or until another node
> rejoins the bus.
> 
> To prevent the AERR flooding, disable the AERR interrupt when we are in
> passive mode.

Can you implement Bus Error Reporting?

| https://elixir.bootlin.com/linux/v5.14/source/include/uapi/linux/can/netlink.h#L99

This way the user can control if bus errors, and the ACK error is one of
them, should be reported. Bus error reporting is disabled by default. I
think enabling AT91_IRQ_ERR_FRAME only if CAN_CTRLMODE_BERR_REPORTING is
active should do the trick.

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 #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-17 12:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-05 18:30 [PATCH] can: at91_can: fix passive-state AERR flooding Brandon Maier
2021-10-05 18:30 ` Brandon Maier
2021-10-07 13:28 ` Nicolas Ferre
2021-10-07 13:28   ` Nicolas Ferre
2021-10-17 12:46 ` Marc Kleine-Budde
2021-10-17 12:46   ` Marc Kleine-Budde

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.