Bjørn Mork <bjorn@xxxxxxx> writes: > Oliver Neukum <oliver@xxxxxxxxxx> writes: > >> Am Dienstag, 10. Januar 2012, 23:15:16 schrieb Bjørn Mork: >>> - goto bad_desc; >>> + /* Fallback to guessing for rndis >>> + * class devices with bogus union >>> + * descriptor. >>> + * Fixes some Samsung Android devices >>> + */ >>> + if (rndis) >>> + info->u = NULL; >>> + else >>> + goto bad_desc; >> >> You are trying something subtle here. Stop doing that. Use a dedicated flag >> or a clearly named goto. > > Not sure I follow. Continuing by *not* doing goto was actually modelled > after the existing workaround for "Ambit USB Cable Modem". > > Is checking for "if (rndis)" OK as a workaround trigger? It is as exact > device match as I know of, based on the descriptor dump at > https://launchpadlibrarian.net/73443645/lsusb_v_v.txt , and it is the > same flag used for enabling the "missing union descriptor" handling this > workaround relies on. > > Would it be OK to just break out of the switch to continue looping over > the descriptors?: > > diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c > index 41a61ef..1808b1e 100644 > --- a/drivers/net/usb/cdc_ether.c > +++ b/drivers/net/usb/cdc_ether.c > @@ -195,6 +195,11 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf) > info->control, > info->u->bSlaveInterface0, > info->data); > + /* Fixes some Samsung Android RNDIS devices */ > + if (rndis) { > + info->u = NULL; > + break; > + } > goto bad_desc; > } > if (info->control != intf) { > > > > Or is it better to use the next_desc label?: > > > diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c > index 41a61ef..4051231 100644 > --- a/drivers/net/usb/cdc_ether.c > +++ b/drivers/net/usb/cdc_ether.c > @@ -195,6 +195,11 @@ int usbnet_generic_cdc_bind(struct usbnet *dev, struct usb_interface *intf) > info->control, > info->u->bSlaveInterface0, > info->data); > + /* Fixes some Samsung Android RNDIS devices */ > + if (rndis) { > + info->u = NULL; > + goto next_desc; > + } > goto bad_desc; > } > if (info->control != intf) { > > > > > I don't really see the need for a new label anywhere as the only place > I want to jump is already labelled "next_desc", and both methods above > go there. > > But if you're point was that it's stuping continuing down through the > union descriptor handling after detecting a buggy such, then I fully > agree. And either solution above should be OK? Hello, I rediscovered this patch while wading through some of my dead git branches. This didn't seem to go anywhere, did it? The problem still exists judging by the state of the original Debian bug report: http://bugs.debian.org/655387 Should I fixup this in some way and resubmit? Bjørn -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
