Greg, Oleg --
I tested this in 2.6.21-rc5 and it no longer works.
The problem is a change in usb-serial.c, in usb_serial_probe().
In 2.6.20 get_free_serial() is called before the attach function.
In 2.6.21-rc5 get_free_serial() is called after the attach function.
If the attach function returns non-zero (as it does in ti_usb_3410_5052
when changing the configuration) then the device is disconnected,
which calls usb_serial_disconnect(), usb_serial_put(), destroy_serial(),
return_serial(), which NULLs out the serial table for minor number 0
(the initial value of the usb_serial minor field before get_free_serial()
is called.) That is a problem if minor number 0 has already been allocated
to another usb serial device.
It would be easy to work around this by setting the usb_serial minor
field to -1 in create_serial() and check for that in return_serial().
I will send you a patch that does that, though I am not sure that is
the best way to fix this. I will look at it more.
-- Al
Quoting Oleg Verych <[EMAIL PROTECTED]>:
> usb-serial, ti_usb: fix usb configuration changing
>
> Remove bogus error messages, comments with script.
> Use new usb-configuration interface for usb-interface drivers.
>
> Cc: Linus Torvalds <[EMAIL PROTECTED]>
> Cc: Al Borchers <[EMAIL PROTECTED]>
> Signed-off-by: Oleg Verych <[EMAIL PROTECTED]>
> ---
> This tiny change uses new interface merged in 2.6.19 (yet author of
> it didn't find this driver to fix).
>
> Greg KH waits driver's author ack, while i already have it (in
> the linux-usb list):
>
> > I tried this on 3410 and 5052 with and without firmware--worked
> > great.
> >
> > Please
> >
> > 1) Change return value to 1, rather than 0xO1E and 0xA1B.
> > (Clever, but it will just confuse readers.)
>
> Here i was using any values greater than zero, and not negative, i.e.
> errors.
>
> > 2) Drop (void) cast on usb_driver_set_configuration.
>
> I didn't check return status of it, because it doesn't show real
> progress in config. changing (see description of that function).
>
> > 3) Resend the patch with a signed-off line.
> >
> > Thanks for doing this--so glad we can get rid of the hotplug
> > script!
> >
> > -- Al
>
> While they are very busy with another things, i thought they could
> spend a few tens of minutes (in last 2 or so months) to have this
> driver better-looking.
>
> ,-*- this patch -*-
> |usb 2-2: new full speed USB device using ohci_hcd and address 6
> |usb 2-2: configuration #1 chosen from 1 choice
> |ti_usb_3410_5052 2-2:1.0: TI USB 3410 1 port adapter converter detected
> |usb 2-2: reset full speed USB device using ohci_hcd and address 6
> - *compare start* -
> |usb 2-2: device firmware changed
> |usb 2-2: USB disconnect, address 6
> |ti_usb_3410_5052 2-2:1.0: device disconnected
> |usb 2-2: new full speed USB device using ohci_hcd and address 7
> |usb 2-2: configuration #1 chosen from 2 choices
> |ti_usb_3410_5052 2-2:1.0: TI USB 3410 1 port adapter converter detected
> |usb 2-2: setting configuration #2 (activating)
> |ti_usb_3410_5052 2-2:1.0: device disconnected
> |ti_usb_3410_5052 2-2:2.0: TI USB 3410 1 port adapter converter detected
> |usb 2-2: TI USB 3410 1 port adapter converter now attached to ttyUSB0
> `-*-
>
> Below is original behavior and without udev scripting, note that even
> today udev package in most distros doesn't have support of this and
> based on this interface devices (very ugly with dups of dev. IDs and
> same silly script).
>
> ,-*- from compare start point -*-
> |usb 3-3: device firmware changed
> |ti_usb_3410_5052: probe of 3-3:1.0 failed with error -5
> |usb 3-3: USB disconnect, address 25
> |usb 3-3: new full speed USB device using ohci_hcd and address 26
> |usb 3-3: configuration #1 chosen from 2 choices
> |ti_usb_3410_5052 3-3:1.0: TI USB 3410 1 port adapter converter detected
> |ti_usb_3410_5052: probe of 3-3:1.0 failed with error -5
> `-*-
>
> I think it's worth having in .21, just for people, who is known to have
> troubles with udev settings for this driver, and just to have more beauty
> there. It's not clear when authors will start to merge their stuff, and
> i'm still waiting to help with firmware loading (bigger patch-set i was
> originally working on). Also, i think, Greg is wrong, doing nothing to
> remove such ugliness.
>
> Thus, i'm sending this to our big Wizard, Linus.
>
> (two: small and fat white spaces were removed in the function i've
> changed, still remembering my makefile patches with
> one-whitespace-to-much-noise
> bug on the beginning of 2.6.21 ;)
>
> Thanks anyways!
>
> drivers/usb/serial/ti_usb_3410_5052.c | 68
> +++++-----------------------------
> 1 file changed, 11 insertions(+), 57 deletions(-)
>
> Index: linux-2.6.21-rc5/drivers/usb/serial/ti_usb_3410_5052.c
> ===================================================================
> --- linux-2.6.21-rc5.orig/drivers/usb/serial/ti_usb_3410_5052.c
> 2007-03-27
> 08:00:26.338095634 +0200
> +++ linux-2.6.21-rc5/drivers/usb/serial/ti_usb_3410_5052.c 2007-03-27
> 08:09:36.803596727 +0200
> @@ -1,4 +1,3 @@
> -/* vi: ts=8 sw=8
> - *
> +/*
> * TI 3410/5052 USB Serial Driver
> *
> @@ -17,54 +16,4 @@
> * technical support, or Al Borchers <[EMAIL PROTECTED]>, or
> * Peter Berger <[EMAIL PROTECTED]>.
> - *
> - * This driver needs this hotplug script in
> /etc/hotplug/usb/ti_usb_3410_5052
> - * or in /etc/hotplug.d/usb/ti_usb_3410_5052.hotplug to set the device
> - * configuration.
> - *
> - * #!/bin/bash
> - *
> - * BOOT_CONFIG=1
> - * ACTIVE_CONFIG=2
> - *
> - * if [[ "$ACTION" != "add" ]]
> - * then
> - * exit
> - * fi
> - *
> - * CONFIG_PATH=/sys${DEVPATH%/?*}/bConfigurationValue
> - *
> - * if [[ 0`cat $CONFIG_PATH` -ne $BOOT_CONFIG ]]
> - * then
> - * exit
> - * fi
> - *
> - * PRODUCT=${PRODUCT%/?*} # delete version
> - * VENDOR_ID=`printf "%d" 0x${PRODUCT%/?*}`
> - * PRODUCT_ID=`printf "%d" 0x${PRODUCT#*?/}`
> - *
> - * PARAM_PATH=/sys/module/ti_usb_3410_5052/parameters
> - *
> - * function scan() {
> - * s=$1
> - * shift
> - * for i
> - * do
> - * if [[ $s -eq $i ]]
> - * then
> - * return 0
> - * fi
> - * done
> - * return 1
> - * }
> - *
> - * IFS=$IFS,
> - *
> - * if (scan $VENDOR_ID 1105 `cat $PARAM_PATH/vendor_3410` &&
> - * scan $PRODUCT_ID 13328 `cat $PARAM_PATH/product_3410`) ||
> - * (scan $VENDOR_ID 1105 `cat $PARAM_PATH/vendor_5052` &&
> - * scan $PRODUCT_ID 20562 20818 20570 20575 `cat $PARAM_PATH/product_5052`)
> - * then
> - * echo $ACTIVE_CONFIG > $CONFIG_PATH
> - * fi
> */
>
> @@ -452,11 +401,16 @@ static int ti_startup(struct usb_serial
> }
>
> - status = -ENODEV;
> + status = 1; /* positive status -- device to be reconfigured */
> goto free_tdev;
> - }
> + }
>
> - /* the second configuration must be set (in sysfs by hotplug script) */
> + /* active configuration must be set */
> if (dev->actconfig->desc.bConfigurationValue == TI_BOOT_CONFIG) {
> - status = -ENODEV;
> + dev_info(&dev->dev, "setting configuration #%u (activating)\n",
> + TI_ACTIVE_CONFIG);
> +
> + status = usb_driver_set_configuration(dev, TI_ACTIVE_CONFIG);
> + status = status ? status: 1;
> +
> goto free_tdev;
> }
> @@ -488,5 +442,5 @@ static int ti_startup(struct usb_serial
> tport->tp_uart_mode = 0; /* default is RS232 */
> }
> -
> +
> return 0;
>
> ____
>
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
