From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>,
linux-can@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
Subject: Re: [PATCH] can: etas_es58x: do not send disable channel command if device is unplugged
Date: Thu, 2 Feb 2023 16:35:40 +0100 [thread overview]
Message-ID: <Y9vYTPURT4hDSH4n@kroah.com> (raw)
In-Reply-To: <20230202151622.sotqfwmgwwtgv4dl@pengutronix.de>
On Thu, Feb 02, 2023 at 04:16:22PM +0100, Marc Kleine-Budde wrote:
> On 28.01.2023 22:38:15, Vincent Mailhol wrote:
> > When turning the network interface down, es58x_stop() is called and
> > will send a command to the ES58x device to disable the channel
> > c.f. es58x_ops::disable_channel().
> >
> > However, if the device gets unplugged while the network interface is
> > still up, es58x_ops::disable_channel() will obviously fail to send the
> > URB command and the driver emits below error message:
> >
> > es58x_submit_urb: USB send urb failure: -ENODEV
> >
> > Check the usb device state before sending the disable channel command
> > in order to silence above error message.
> >
> > Update the documentation of es58x_stop() accordingly.
> >
> > The check being added in es58x_stop() is:
> >
> > if (es58x_dev->udev->state >= USB_STATE_UNAUTHENTICATED)
> >
> > This is just the negation of the check done in usb_submit_urb()[1].
> >
> > [1] usb_submit_urb(), verify usb device's state.
> > Link: https://elixir.bootlin.com/linux/v6.1/source/drivers/usb/core/urb.c#L384
> >
> > Fixes: 8537257874e9 ("can: etas_es58x: add core support for ETAS ES58X CAN USB interfaces")
> > Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> > ---
> > As far as I know, there doesn't seem to be an helper function to check
> > udev->state values. If anyone is aware of such helper function, let me
> > know..
>
> The constant USB_STATE_UNAUTHENTICATED is not used very often in the
> kernel. Maybe Greg can point out what is best to do here.
Don't do anything in the driver here, except maybe turn off that useless
"error message" in the urb submission function? A USB driver shouldn't
be poking around in the state of the device at all, as it's about to go
away if it's been physically removed.
The urb submission will fail if the device is removed, handle the error
and move on, that's a totally normal situation as you say it happens a
lot :)
Don't spam error logs for common things please.
thanks,
greg k-h
prev parent reply other threads:[~2023-02-02 15:38 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-28 13:38 [PATCH] can: etas_es58x: do not send disable channel command if device is unplugged Vincent Mailhol
2023-02-02 15:16 ` Marc Kleine-Budde
2023-02-02 15:35 ` Greg Kroah-Hartman [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y9vYTPURT4hDSH4n@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=linux-can@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mailhol.vincent@wanadoo.fr \
--cc=mkl@pengutronix.de \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).