Re: [PATCH] can: m_can: Enable NAPI before enabling interrupts

[Marc Kleine-Budde]

On 06.09.2024 23:19:51, Hamby, Jake (US) wrote:
> If any error flags are set when bringing up the CAN device, e.g. due to
> CAN bus traffic before initializing the device, when m_can_start is
> called and interrupts are enabled, m_can_isr is called immediately,
> which disables all CAN interrupts and calls napi_schedule.
> 
> Because napi_enable isn't called until later in m_can_open, the call to
> napi_schedule never schedules the m_can_poll callback and the device is
> left with interrupts disabled and can't receive any CAN packets until
> rebooted. This can be verified by running "cansend" from another device
> before setting the bitrate and calling "ip link set up can0" on the test
> device. Adding debug lines to m_can_isr shows it's called with flags
> (IR_EP | IR_EW | IR_CRCE), which calls m_can_disable_all_interrupts and
> napi_schedule, and then m_can_poll is never called.
> 
> Move the call to napi_enable above the call to m_can_start to enable any
> initial interrupt flags to be handled by m_can_poll so that interrupts
> are reenabled. Add a call to napi_disable in the error handling section
> of m_can_open, to handle the case where later functions return errors.
> 
> Also, in m_can_close, move the call to napi_disable below the call to
> m_can_stop to ensure all interrupts are handled when bringing down the
> device. This race condition is much less likely to occur.
> 
> While testing, I noticed that IR_TSW (timestamp wraparound) fires at
> about 1 Hz, but the driver doesn't care about it. Add it to the list of
> interrupts to disable in m_can_chip_config to reduce unneeded wakeups.
> 
> Tested on a Microchip SAMA7G54 MPU. The fix should be applicable to any
> SoC with a Bosch M_CAN controller.

Thank you very much for your contribution! I believe you have solved the
problem I was investigating last Friday. I still need to test it with
your solution though.

This patch does not contain your Signed-off-by [1]. Add it to your next
patches; "git commit -s" will add it automatically. For this patch, you
can reply and include it in your email!

[1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

Attachment: signature.asc
Description: PGP signature