| Message ID | 20150111204508.GB8999@linux |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
On 01/11/2015 09:45 PM, Ahmed S. Darwish wrote: > From: Ahmed S. Darwish <ahmed.darwish@valeo.com> > > (This is a draft patch, I'm not sure if this fixes the USB > bug or only its psymptom. Feedback from the linux-usb folks > is really appreciated.) > > When plugging the Kvaser USB/CAN dongle the first time, everything > works as expected and all of the transfers from and to the USB > device succeeds. > > Meanwhile, after unplugging the device and plugging it again, the > first bulk transfer _always_ returns an -ETIMEDOUT. The following > behaviour was observied: > > - Setting higher timeout values for the first bulk transfer never > solved the issue. > > - Unloading, then loading, our kvaser_usb module in question > __always__ solved the issue. > > - Checking first bulk transfer status, and retry the transfer > again in case of an -ETIMEDOUT also __always__ solved the issue. > This is what the patch below does. > > - In the testing done so far, this issue appears only on laptops > but never on PCs (possibly power related?) > > Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com> Does this patch apply apply between 3 and 4? If not, please re-arrange the series. As this is a bug fix, patches 1, 2 and 4 will go via net/master, 3 will go via net-next/master. Marc
On Sun, Jan 11, 2015 at 09:51:10PM +0100, Marc Kleine-Budde wrote: > On 01/11/2015 09:45 PM, Ahmed S. Darwish wrote: > > From: Ahmed S. Darwish <ahmed.darwish@valeo.com> > > > > (This is a draft patch, I'm not sure if this fixes the USB > > bug or only its psymptom. Feedback from the linux-usb folks > > is really appreciated.) > > > > When plugging the Kvaser USB/CAN dongle the first time, everything > > works as expected and all of the transfers from and to the USB > > device succeeds. > > > > Meanwhile, after unplugging the device and plugging it again, the > > first bulk transfer _always_ returns an -ETIMEDOUT. The following > > behaviour was observied: > > > > - Setting higher timeout values for the first bulk transfer never > > solved the issue. > > > > - Unloading, then loading, our kvaser_usb module in question > > __always__ solved the issue. > > > > - Checking first bulk transfer status, and retry the transfer > > again in case of an -ETIMEDOUT also __always__ solved the issue. > > This is what the patch below does. > > > > - In the testing done so far, this issue appears only on laptops > > but never on PCs (possibly power related?) > > > > Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com> > > Does this patch apply apply between 3 and 4? If not, please re-arrange > the series. As this is a bug fix, patches 1, 2 and 4 will go via > net/master, 3 will go via net-next/master. > Since no one complained earlier, I guess this issue only affects USBCAN devices. That's why I've based it above patch #3: adding USBCAN hardware support. Nonetheless, it won't do any harm for the current Leaf-only driver. So _if_ this is the correct fix, I will update the commit log, refactor the check into a 'do { } while()' loop, and then base it above the Leaf-only net/master fixes on patch #1, and #2. Any feedback on the USB side of things? Thanks, Darwish -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/12/2015 11:14 AM, Ahmed S. Darwish wrote: > On Sun, Jan 11, 2015 at 09:51:10PM +0100, Marc Kleine-Budde wrote: >> On 01/11/2015 09:45 PM, Ahmed S. Darwish wrote: >>> From: Ahmed S. Darwish <ahmed.darwish@valeo.com> >>> >>> (This is a draft patch, I'm not sure if this fixes the USB >>> bug or only its psymptom. Feedback from the linux-usb folks >>> is really appreciated.) >>> >>> When plugging the Kvaser USB/CAN dongle the first time, everything >>> works as expected and all of the transfers from and to the USB >>> device succeeds. >>> >>> Meanwhile, after unplugging the device and plugging it again, the >>> first bulk transfer _always_ returns an -ETIMEDOUT. The following >>> behaviour was observied: >>> >>> - Setting higher timeout values for the first bulk transfer never >>> solved the issue. >>> >>> - Unloading, then loading, our kvaser_usb module in question >>> __always__ solved the issue. >>> >>> - Checking first bulk transfer status, and retry the transfer >>> again in case of an -ETIMEDOUT also __always__ solved the issue. >>> This is what the patch below does. >>> >>> - In the testing done so far, this issue appears only on laptops >>> but never on PCs (possibly power related?) >>> >>> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com> >> >> Does this patch apply apply between 3 and 4? If not, please re-arrange >> the series. As this is a bug fix, patches 1, 2 and 4 will go via >> net/master, 3 will go via net-next/master. > > Since no one complained earlier, I guess this issue only affects > USBCAN devices. That's why I've based it above patch #3: adding > USBCAN hardware support. > > Nonetheless, it won't do any harm for the current Leaf-only > driver. So _if_ this is the correct fix, I will update the commit > log, refactor the check into a 'do { } while()' loop, and then > base it above the Leaf-only net/master fixes on patch #1, and #2. > > Any feedback on the USB side of things? Maybe you have to change the subject of this patch to be more visible on the USB list and/or add the right USB people on Cc. Marc
Hello, On Mon, Jan 12, 2015 at 05:14:07AM -0500, Ahmed S. Darwish wrote: > On Sun, Jan 11, 2015 at 09:51:10PM +0100, Marc Kleine-Budde wrote: > > On 01/11/2015 09:45 PM, Ahmed S. Darwish wrote: > > > From: Ahmed S. Darwish <ahmed.darwish@valeo.com> > > > > > > (This is a draft patch, I'm not sure if this fixes the USB > > > bug or only its psymptom. Feedback from the linux-usb folks > > > is really appreciated.) > > > > > > When plugging the Kvaser USB/CAN dongle the first time, everything > > > works as expected and all of the transfers from and to the USB > > > device succeeds. > > > > > > Meanwhile, after unplugging the device and plugging it again, the > > > first bulk transfer _always_ returns an -ETIMEDOUT. The following > > > behaviour was observied: > > > > > > - Setting higher timeout values for the first bulk transfer never > > > solved the issue. > > > > > > - Unloading, then loading, our kvaser_usb module in question > > > __always__ solved the issue. > > > > > > - Checking first bulk transfer status, and retry the transfer > > > again in case of an -ETIMEDOUT also __always__ solved the issue. > > > This is what the patch below does. > > > > > > - In the testing done so far, this issue appears only on laptops > > > but never on PCs (possibly power related?) > > > > > > Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com> > > > > Does this patch apply apply between 3 and 4? If not, please re-arrange > > the series. As this is a bug fix, patches 1, 2 and 4 will go via > > net/master, 3 will go via net-next/master. > > > > Since no one complained earlier, I guess this issue only affects > USBCAN devices. That's why I've based it above patch #3: adding > USBCAN hardware support. > > Nonetheless, it won't do any harm for the current Leaf-only > driver. So _if_ this is the correct fix, I will update the commit > log, refactor the check into a 'do { } while()' loop, and then > base it above the Leaf-only net/master fixes on patch #1, and #2. > > Any feedback on the USB side of things? Can you take a wireshark capture showing the problem? It can maybe help people to figure out what happens. What kind of usbcan device do you use? Which firmware revision is loaded on the device? Kr,
On Mon, Jan 12, 2015 at 02:33:30PM +0100, Olivier Sobrie wrote: > Hello, > > On Mon, Jan 12, 2015 at 05:14:07AM -0500, Ahmed S. Darwish wrote: > > On Sun, Jan 11, 2015 at 09:51:10PM +0100, Marc Kleine-Budde wrote: > > > On 01/11/2015 09:45 PM, Ahmed S. Darwish wrote: > > > > From: Ahmed S. Darwish <ahmed.darwish@valeo.com> > > > > > > > > (This is a draft patch, I'm not sure if this fixes the USB > > > > bug or only its psymptom. Feedback from the linux-usb folks > > > > is really appreciated.) > > > > > > > > When plugging the Kvaser USB/CAN dongle the first time, everything > > > > works as expected and all of the transfers from and to the USB > > > > device succeeds. > > > > > > > > Meanwhile, after unplugging the device and plugging it again, the > > > > first bulk transfer _always_ returns an -ETIMEDOUT. The following > > > > behaviour was observied: > > > > > > > > - Setting higher timeout values for the first bulk transfer never > > > > solved the issue. > > > > > > > > - Unloading, then loading, our kvaser_usb module in question > > > > __always__ solved the issue. > > > > > > > > - Checking first bulk transfer status, and retry the transfer > > > > again in case of an -ETIMEDOUT also __always__ solved the issue. > > > > This is what the patch below does. > > > > > > > > - In the testing done so far, this issue appears only on laptops > > > > but never on PCs (possibly power related?) > > > > > > > > Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com> > > > > > > Does this patch apply apply between 3 and 4? If not, please re-arrange > > > the series. As this is a bug fix, patches 1, 2 and 4 will go via > > > net/master, 3 will go via net-next/master. > > > > > > > Since no one complained earlier, I guess this issue only affects > > USBCAN devices. That's why I've based it above patch #3: adding > > USBCAN hardware support. > > > > Nonetheless, it won't do any harm for the current Leaf-only > > driver. So _if_ this is the correct fix, I will update the commit > > log, refactor the check into a 'do { } while()' loop, and then > > base it above the Leaf-only net/master fixes on patch #1, and #2. > > > > Any feedback on the USB side of things? > > Can you take a wireshark capture showing the problem? > It can maybe help people to figure out what happens. > Yeah, I'm planning on doing something similar. > What kind of usbcan device do you use? "Kvaser USBcan II HS/LS" > Which firmware revision is loaded on the device? > The device reports firmware version 2.9.410. Interesting. The changelog of their latest firmware states that it "fixed USB configuration issue during USB attach." That might be the problem. I have two devices here. I'll update the firmware only for one of them and see what happens. Thanks, Darwish -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c index da47d17..5925637 100644 --- a/drivers/net/can/usb/kvaser_usb.c +++ b/drivers/net/can/usb/kvaser_usb.c @@ -1927,7 +1927,7 @@ static int kvaser_usb_probe(struct usb_interface *intf, { struct kvaser_usb *dev; int err = -ENOMEM; - int i; + int i, retry = 3; dev = devm_kzalloc(&intf->dev, sizeof(*dev), GFP_KERNEL); if (!dev) @@ -1956,7 +1956,16 @@ static int kvaser_usb_probe(struct usb_interface *intf, usb_set_intfdata(intf, dev); - err = kvaser_usb_get_software_info(dev); + /* On some x86 laptops, plugging a USBCAN device again after + * an unplug makes the firmware always ignore the very first + * command. For such a case, provide some room for retries + * instead of completly exiting the driver. + */ + while (retry--) { + err = kvaser_usb_get_software_info(dev); + if (err != -ETIMEDOUT) + break; + } if (err) { dev_err(&intf->dev, "Cannot get software infos, error %d\n", err);