On Tue, 19 Jun 2012, Lan Tianyu wrote:
> This patch is to make usb port a real device under usb hub interface.
> +static int usb_hub_create_port_device(struct usb_hub *hub,
> + int port1)
> +{
> + struct usb_port *port_dev = &hub->ports[port1 - 1];
Now that these things contain a struct device, they can't be allocated
in an array. Each usb_port structure must be allocated dynamically;
you'll have to store a pointer to it in hub->ports[port1 - 1].
> + int retval;
> +
> + port_dev->udev = hub->hdev;
> + port_dev->dev.parent = hub->intfdev;
> + port_dev->dev.type = &usb_port_device_type;
Where's the "release" callback?
> + dev_set_name(&port_dev->dev, "port%d", port1);
> +
> + retval = device_register(&port_dev->dev);
> + if (retval)
> + goto error_register;
> +
> +error_register:
> + put_device(&port_dev->dev);
> + return retval;
This doesn't look right. What does the "goto" statement do?
Also, don't you want to create the attributes when the port device is
registered?
> @@ -1528,7 +1571,7 @@ static void hub_disconnect(struct usb_interface *intf)
>
> usb_free_urb(hub->urb);
> kfree(hdev->children);
> - kfree(hub->port_owners);
> + kfree(hub->ports);
No, you can't just deallocate a bunch of device structures. There may
still be outstanding references to them. You have to use a release
routine.
> @@ -1604,8 +1647,15 @@ descriptor_error:
> if (hdev->speed == USB_SPEED_HIGH)
> highspeed_hubs++;
>
> - if (hub_configure(hub, endpoint) >= 0)
> + if (hub_configure(hub, endpoint) >= 0) {
> + int i;
> +
> + for (i = 0; i < hdev->maxchild; i++)
> + if (usb_hub_create_port_device(hub, i + 1) < 0)
> + dev_err(&intf->dev, "couldn't create port%d "
> + "device.\n", i + 1);
> return 0;
This code belongs in hub_configure, not here.
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
