[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250716-godlike-organic-pudu-53deda-mkl@pengutronix.de>
Date: Wed, 16 Jul 2025 07:55:32 +0200
From: Marc Kleine-Budde <mkl@...gutronix.de>
To: Andrei Lalaev <andrey.lalaev@...il.com>
Cc: mailhol.vincent@...adoo.fr, linux-kernel@...r.kernel.org,
linux-can@...r.kernel.org
Subject: Re: [RFC PATCH] can: gs_usb: fix kernel oops during restart
On 16.07.2025 07:45:08, Andrei Lalaev wrote:
> On 15.07.2025 16:29, Marc Kleine-Budde wrote:
> > On 15.07.2025 16:24:22, Andrei Lalaev wrote:
> >> I was also surprised because this callback isn't marked as mandatory
> >> and that there are no additional checks.
> >>
> >>>
> >>> What about this fix?
> >>>
> >>> diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
> >>> index 13826e8a707b..94603c9eb4aa 100644
> >>> --- a/drivers/net/can/dev/netlink.c
> >>> +++ b/drivers/net/can/dev/netlink.c
> >>> @@ -285,6 +285,12 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
> >>> }
> >>>
> >>> if (data[IFLA_CAN_RESTART_MS]) {
> >>> + if (!priv->do_set_mode) {
> >>> + NL_SET_ERR_MSG(extack,
> >>> + "device doesn't support restart from Bus Off");
> >>> + return -EOPNOTSUPP;
> >>> + }
> >>> +
> >>> /* Do not allow changing restart delay while running */
> >>> if (dev->flags & IFF_UP)
> >>> return -EBUSY;
> >>> @@ -292,6 +298,12 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
> >>> }
> >>>
> >>> if (data[IFLA_CAN_RESTART]) {
> >>> + if (!priv->do_set_mode) {
> >>> + NL_SET_ERR_MSG(extack,
> >>> + "device doesn't support restart from Bus Off");
> >>> + return -EOPNOTSUPP;
> >>> + }
> >>> +
> >>> /* Do not allow a restart while not running */
> >>> if (!(dev->flags & IFF_UP))
> >>> return -EINVAL;
> >>
> >> Thanks for the patch. As expected, it fixes the kernel OOPS,
> >> but the interface never leaves the BUS_OFF state.
> >
> > Which device and which firmware are you using?
> >
> > The gs_usb/candlelight interface is un(der)defined when it comes to
> > bus-off handling.
> >
> > I think the original candlelight with the stm32f072 does auto bus-off
> > recovery. Not sure about the candlelight on stm32g0b1.
>
> Sorry, my bad for not mentioning it earlier. I have several USB-CAN adapters:
> - two are based on STM32F072 (not original CandleLight, but using the same FW)
> - one is a original CandleLightFD on STM32G0B1, that I used for testing
>
> And all of them behave exactly as you described:
> - both STM32F072-based automatically recover from BUS_OFF and I see
> it in `ip -details link show can0`
> - STM32G0B1-based doesn't recover and I have to down/up interface to restore it
>
> Since this is expected behavior and no kernel OOPS occurs,
> I will switch to your patch.
At least the behavior can be explained, it's not expected, though :) I
think we have to fix the stm32g0b1 firmware to auto recover from bus
off...and in the long term, extend the candlelight firmware, the USB
protocol and the Linux driver to support proper Bus-Off handling.
> Thanks a lot for the patch and your help!
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 |
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists