Re: [PATCH] cxacru: ignore cx82310_eth devices

[Ondrej Zary <linux@rainbow-software.org>]

On Monday 06 September 2010, Simon Arlott wrote:
> On Sun, September 5, 2010 22:04, Greg KH wrote:
> > On Sun, Sep 05, 2010 at 10:12:33PM +0200, Ondrej Zary wrote:
> >> Ignore ADSL routers, which can have the same vendor and product IDs
> >> as ADSL modems but should be handled by the cx82310_eth driver.
> >>
> >> This intentionally ignores device IDs that aren't currently handled
> >> by cx82310_eth. There may be other device IDs that perhaps shouldn't
> >> be claimed by cxacru.
> >>
> >> Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
>
> Missing Signed-off-by: you're modifying my changes.

Oops, sorry for that. I wanted to remove only the line between the 
signed-offs...

> >> --- linux-2.6.36-rc3-orig/drivers/usb/atm/cxacru.c	2010-08-29
> >> 17:36:04.000000000 +0200 +++
> >> linux-2.6.36-rc3/drivers/usb/atm/cxacru.c	2010-09-05 18:54:56.000000000
> >> +0200 @@ -1324,8 +1324,22 @@ static struct usbatm_driver cxacru_drive
> >> .tx_padding	= 11,
> >>  };
> >>
> >> -static int cxacru_usb_probe(struct usb_interface *intf, const struct
> >> usb_device_id *id) -{
> >> +static int cxacru_usb_probe(struct usb_interface *intf,
> >> +		const struct usb_device_id *id) {
> >
> > Ick, what?
>
> Sorry, this was my fault. I wasn't thinking when I changed it.
>
> >> +	struct usb_device *usb_dev = interface_to_usbdev(intf);
> >> +	char buf[15];
> >> +
> >> +	/* avoid ADSL routers (cx82310_eth)
> >> +	 * abort if bDeviceClass is 0xff and iProduct is "USB NET CARD" */
> >> +	if (usb_dev->descriptor.bDeviceClass == USB_CLASS_VENDOR_SPEC
> >> +			&& usb_string(usb_dev, usb_dev->descriptor.iProduct,
> >> +				buf, sizeof(buf)) > 0) {
> >> +		if (!strcmp(buf, "USB NET CARD")) {
> >> +			dev_info(&intf->dev, "ignoring cx82310_eth device\n");
> >> +			return -ENODEV;
> >> +		}
> >> +	}
> >
> > In thinking about this a bit more, don't you also want to check the
> > vendor and product id?  You can't always be sure about the string of any
> > old device, right?
>
> This is already checked using the driver's id_table.
>
> I don't know if the vendor and product IDs for the cx82310_eth device
> should be claimed by cxacru or not. Either Conexant are sharing IDs between
> devices, or someone added it without confirming it works. There's no
> comment in the code from 2003 explaining what device it's supposed to be.
> For this reason I'd prefer to ignore all cxacru-claimed devices that appear
> to be an "USB NET CARD", and log that it did so.

Yes, these "USB NET CARD" devices will not work with cxacru either. We don't 
lose anything even if they are ignored by cxacru but not claimed by 
cx82310_eth. If someone has such device, (s)he can report the IDs to be added 
to cx82310_eth.

> The 3 variants of the hardware I have all use an iProduct of "ADSL USB
> MODEM", but I don't want to restrict cxacru to just that in case some
> devices have different values.

-- 
Ondrej Zary