On 11/19/20 10:46 AM, Joakim Zhang wrote: > >> -----Original Message----- >> From: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> >> Sent: 2020年11月19日 16:53 >> To: linux-can@xxxxxxxxxxxxxxx >> Cc: Joakim Zhang <qiangqing.zhang@xxxxxxx>; kernel@xxxxxxxxxxxxxx; Marc >> Kleine-Budde <mkl@xxxxxxxxxxxxxx> >> Subject: [PATCH 5/5] can: flexcan: flexcan_close(): change order if commands >> to properly shut down the controller >> >> There haven been reports, that the flexcan_close() soradically hangs during >> simultanious ifdown, sending of CAN messages and probably open CAN bus: >> >> | (__schedule) from [<808bbd34>] (schedule+0x90/0xb8) >> | (schedule) from [<808bf274>] (schedule_timeout+0x1f8/0x24c) >> | (schedule_timeout) from [<8016be44>] (msleep+0x18/0x1c) >> | (msleep) from [<80746a64>] (napi_disable+0x60/0x70) >> | (napi_disable) from [<8052fdd0>] (flexcan_close+0x2c/0x140) >> | (flexcan_close) from [<80744930>] (__dev_close_many+0xb8/0xd8) >> | (__dev_close_many) from [<8074db9c>] (__dev_change_flags+0xd0/0x1a0) >> | (__dev_change_flags) from [<8074dc84>] (dev_change_flags+0x18/0x48) >> | (dev_change_flags) from [<80760c24>] (do_setlink+0x44c/0x7b4) >> | (do_setlink) from [<80761560>] (rtnl_newlink+0x374/0x68c) >> >> I was unable to reproduce the issue, but a cleanup of the flexcan close >> sequence has probably fixed the problem at the reporting user. >> >> This patch changes the sequence in flexcan_close() to: >> - stop the TX queue >> - disable the interrupts on the chip level and wait via free_irq() >> synchronously for the interrupt handler to finish >> - disable RX offload, which disables synchronously NAPI >> - disable the flexcan on the chip level >> - free RX offload >> - disable the transceiver >> - close the CAN device >> - disable the clocks >> >> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> >> --- >> drivers/net/can/flexcan.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index >> 0fb768dee99f..002e93f2b249 100644 >> --- a/drivers/net/can/flexcan.c >> +++ b/drivers/net/can/flexcan.c >> @@ -1789,15 +1789,16 @@ static int flexcan_close(struct net_device *dev) >> struct flexcan_priv *priv = netdev_priv(dev); >> >> netif_stop_queue(dev); >> + flexcan_chip_interrupts_disable(dev); >> + free_irq(dev->irq, dev); >> can_rx_offload_disable(&priv->offload); >> flexcan_chip_stop_disable_on_error(dev); >> flexcan_chip_interrupts_disable(dev); > > Hi Marc, > > Is it a special treatment? flexcan_chip_interrupts_disable called twice? Thanks for catching this. It's a mistake, will fix. > flexcan_chip_interrupts_disable(dev); > free_irq(dev->irq, dev); > can_rx_offload_disable(&priv->offload); > flexcan_chip_stop_disable_on_error(dev); > flexcan_chip_interrupts_disable(dev); Thanks for the review, 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:
signature.asc
Description: OpenPGP digital signature
