On Wed, 28 Mar 2012, Lan Tianyu wrote:
> Move child's pointer to the struct usb_hub_port since the child device
> is directly associated with the port. Provide usb_get_hub_child_device()
> to get child's pointer.
> --- a/drivers/usb/core/devices.c
> +++ b/drivers/usb/core/devices.c
> @@ -590,7 +590,9 @@ static ssize_t usb_device_dump(char __user **buffer, size_t *nbytes,
>
> /* Now look at all of this device's children. */
> for (chix = 0; chix < usbdev->maxchild; chix++) {
> - struct usb_device *childdev = usbdev->children[chix];
> + struct usb_device *childdev;
> + if (usb_get_hub_child_device(usbdev, chix + 1, &childdev) < 0)
> + continue;
>
Whitespace issue: The blank line should come before the new executable
statement, not after it.
> @@ -1475,11 +1477,12 @@ bool usb_device_is_owned(struct usb_device *udev)
>
> static void recursively_mark_NOTATTACHED(struct usb_device *udev)
> {
> + struct usb_hub *hub = hdev_to_hub(udev);
I'm not sure this is good. You are calling hdev_to_hub without knowing
beforehand that udev really is a hub -- in fact, most of the time udev
won't be a hub. The routine wasn't meant to be used that way. Have
you checked that hdev_to_hub won't crash when the argument isn't a hub?
What happens if the active configuration has no interfaces?
> @@ -4147,3 +4151,15 @@ void usb_queue_reset_device(struct usb_interface *iface)
> schedule_work(&iface->reset_ws);
> }
> EXPORT_SYMBOL_GPL(usb_queue_reset_device);
> +
> +int usb_get_hub_child_device(struct usb_device *hdev, int port1,
> + struct usb_device **child)
> +{
> + struct usb_hub *hub = hdev_to_hub(hdev);
> +
> + if (port1 > hdev->maxchild || port1 < 1)
> + return -EINVAL;
> + *child = hub->port_data[port1 - 1].child;
> + return 0;
> +}
This new routine is more complicated that it needs to be. You don't
have to return an error code; none of the callers make use of it.
Just return the child pointer, and return NULL if there's an error.
Alan Stern
--
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
