David:

I've just started looking in detail at your patch, and there is at least 
one error plus a couple of other things that perhaps could be cleaned up.

The error is in usb_alloc_dev.  Instead of

        dev->dev.parent = parent->dev.parent;

it should say

        dev->dev.parent = &parent->dev;

Otherwise everything ends up with the same parent!  For that matter, does 
it really make sense to have a parent pointer in struct usb_device when it 
will always echo the parent pointer in the embedded struct device?

In usb_new_device() the failure pathway does not call 
usb_destroy_configuration().  It does change the device state incorrectly 
and remove the device address bit (which it shouldn't -- the caller should 
take care of that.)

hub_port_connect_change() doesn't acquire dev->serialize.

Is the SET_CONFIG_TRIES loop in hub_port_connect_change() really needed?  
It was in the old code, and it probably was unnecessary there too, though
I can't be sure of that.  There's no comment indicating why it should be
there, and hub_port_init() has its own retry loops.

Speaking of which, in hub_port_init() there's no reason to have 
usb_set_address() called within the GET_DESCRIPTOR_TRIES loop.  If it 
needs a retry loop, it should have one of its own.

In hub_port_init(), the failure paths don't consistently set retval.

Nowhere does the core call disable_endpoint for ep0 at address 0.  I 
suppose the host drivers are supposed to take care of that themselves when 
they exit.

Alan Stern



-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials.
Become an expert in LINUX or just sharpen your skills.  Sign up for IBM's
Free Linux Tutorials.  Learn everything from the bash shell to sys admin.
Click now! http://ads.osdn.com/?ad_id=1278&alloc_id=3371&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to