Hi again,
> > If I understand it well, this changes the previous default behaviour;
> > intf->cur_altsetting will be intf->altsetting[0], like before, only if
> > there's no intf->altsetting[i].desc.bAlternateSetting == 0.
>
> That's right. However, the oops you saw shouldn't happen so long as
> intf->cur_altsetting points to something valid. I got the impression
that
> in the cdc-acm probe routine maybe it was a null pointer.
Additionally, there's still a reference to altsetting[0] in cdc-acm.c (in
acm_ctrl_msg()), I don't know if it's intentional or should have been
converted from
acm->control->altsetting[0].desc.bInterfaceNumber
to
acm->control->cur_altsetting.desc.bInterfaceNumber
?
--
Colin
This message represents the official view of the voices
in my head.
On Thu, 25 Mar 2004, Colin Leroy wrote:
> Additionally, there's still a reference to altsetting[0] in cdc-acm.c (in
> acm_ctrl_msg()), I don't know if it's intentional or should have been
> converted from
> acm->control->altsetting[0].desc.bInterfaceNumber
> to
> acm->control->cur_altsetting.desc.bInterfaceNumber
> ?
It's not as bad as it looks. All the altsetting entries for an interface
are guaranteed to have the same value stored in desc.bInterfaceNumber, so
it really doesn't matter whether that value is retrieved from the first
entry in the altsetting array or the current entry -- provided of course
that cur_altsetting is a valid pointer! It's mostly just a matter of
taste. I generally prefer to use cur_altsetting because that's the one
currently installed in the device.
Alan Stern
On 25 Mar 2004 at 11h03, Alan Stern wrote:
> It's not as bad as it looks.
Ok, thanks for the clarification. In the mean time here's the lsusb -v output for my phone. I'll try to find out why we have a null pointer.
Bus 003 Device 003: ID 22b8:3802
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 10.01
bDeviceClass 2 Communications
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 8
idVendor 0x22b8
idProduct 0x3802
bcdDevice 1.00
iManufacturer 1 Motorola Inc.
iProduct 2 Motorola Phone (C350)
iSerial 0
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 67
bNumInterfaces 2
bConfigurationValue 1
iConfiguration 4
bmAttributes 0xc0
Self Powered
MaxPower 20mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 2 Communications
bInterfaceSubClass 2 Abstract (modem)
bInterfaceProtocol 1 AT-commands (v.25ter)
iInterface 5 Motorola Communication Interface
unknown descriptor type: 05 24 00 01 01
unknown descriptor type: 05 24 01 03 01
unknown descriptor type: 05 24 06 00 01
unknown descriptor type: 04 24 02 02
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x89 EP 9 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type none
wMaxPacketSize 16
bInterval 10
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 0
bNumEndpoints 2
bInterfaceClass 10 Data
bInterfaceSubClass 0 Unused
bInterfaceProtocol 0
iInterface 16 Motorola Data Interface
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x01 EP 1 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type none
wMaxPacketSize 16
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82 EP 2 IN
bmAttributes 2
Transfer Type Bulk
Synch Type none
wMaxPacketSize 16
bInterval 0
Language IDs: (length=4)
0409 English(US)
Thanks!
--
Colin
On 25 Mar 2004 at 11h03, Alan Stern wrote:
Hi,
Found out !
cdc-acm wants both interfaces to be ready (cur_altsetting initialized) when acm_probe() is called. Hence, we have to make two parts out of the loop in message.c::usb_set_configuration(): one to init things, one to register them.
The attached patch does that. It fixes the oops, and doesn't break any of my USB peripheral (printer, scanner, mouse, and diskonkey).
I hope it's fine enough to go in :)
--
Colin
On Thu, 25 Mar 2004, Colin Leroy wrote:
> Hi,
>
> Found out !
> cdc-acm wants both interfaces to be ready (cur_altsetting initialized) when acm_probe() is called. Hence, we have to make two parts out of the loop in message.c::usb_set_configuration(): one to init things, one to register them.
>
> The attached patch does that. It fixes the oops, and doesn't break any of my USB peripheral (printer, scanner, mouse, and diskonkey).
>
> I hope it's fine enough to go in :)
It's obvious once you see the problem, isn't it?
Duncan Sands found exactly the same problem and the same solution a few
months ago, but it was never added in.
In this case, your patch could be improved by calling device_initialize()
during the first loop and device_add() during the second. However, that
region of code is kind of in flux right at the moment. When things settle
down, I promise to remember your change and make sure it gets in.
Alan Stern
On 25 Mar 2004 at 13h03, Alan Stern wrote:
Hi,
> In this case, your patch could be improved by calling device_initialize()
> during the first loop and device_add() during the second. However, that
> region of code is kind of in flux right at the moment. When things settle
> down, I promise to remember your change and make sure it gets in.
ok :)
Will this get stabilized before 2.6.5 or after ? (just so I remember to patch it myself if needed...)
Thanks,
--
Colin
On 25 Mar 2004 at 13h03, Alan Stern wrote:
Hi,
> In this case, your patch could be improved by calling device_initialize()
> during the first loop and device_add() during the second.
Here you are :)
--
Colin
On Thu, 25 Mar 2004, Colin Leroy wrote:
> On 25 Mar 2004 at 13h03, Alan Stern wrote:
>
> Hi,
>
> > In this case, your patch could be improved by calling device_initialize()
> > during the first loop and device_add() during the second. However, that
> > region of code is kind of in flux right at the moment. When things settle
> > down, I promise to remember your change and make sure it gets in.
>
> ok :)
> Will this get stabilized before 2.6.5 or after ? (just so I remember to patch it myself if needed...)
I don't know. "After" is my best guess.
Alan Stern