On Wed, Aug 15, 2012 at 09:58:33AM +0800, Lan Tianyu wrote:
> On 2012年08月15日 08:27, Greg Kroah-Hartman wrote:
> > On Tue, Jul 17, 2012 at 03:28:42PM -0700, Sarah Sharp wrote:
> >> From: Lan Tianyu <tianyu....@intel.com>
> >>
> >> This patch turns each USB port on a hub into a new struct device.  This
> >> new device has the USB hub interface device as its parent.  The port
> >> devices are stored in a new structure (usb_port), and an array of
> >> usb_ports are dynamically allocated once we know how many ports the USB
> >> hub has.
> >>
> >> Move the port_owner variable out of usb_hub and into this new structure.
> >>
> >> A new file will be created in the hub interface sysfs directory, so
> >> add documentation.
> >>
> >> Acked-by: Alan Stern <st...@rowland.harvard.edu>
> >> Signed-off-by: Lan Tianyu <tianyu....@intel.com>
> >> Signed-off-by: Sarah Sharp <sarah.a.sh...@linux.intel.com>
> >> ---
> >>  Documentation/ABI/testing/sysfs-bus-usb |    7 +++
> >>  drivers/usb/core/hub.c                  |   91 
> >> +++++++++++++++++++++++++------
> >>  2 files changed, 82 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/Documentation/ABI/testing/sysfs-bus-usb 
> >> b/Documentation/ABI/testing/sysfs-bus-usb
> >> index 5f75f8f..f59dc8c 100644
> >> --- a/Documentation/ABI/testing/sysfs-bus-usb
> >> +++ b/Documentation/ABI/testing/sysfs-bus-usb
> >> @@ -220,3 +220,10 @@ Description:
> >>            If the device doesn't support LTM, the file will read "no".
> >>            The file will be present for all speeds of USB devices, and will
> >>            always read "no" for USB 1.1 and USB 2.0 devices.
> >> +
> >> +What:             /sys/bus/usb/devices/.../(hub interface)/portX
> >> +Date:             July 2012
> >> +Contact:  Lan Tianyu <tianyu....@intel.com>
> >> +Description:
> >> +          The /sys/bus/usb/devices/.../(hub interface)/portX
> >> +          is usb port device's sysfs directory.
> >> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> >> index 540f20b..f704c07 100644
> >> --- a/drivers/usb/core/hub.c
> >> +++ b/drivers/usb/core/hub.c
> >> @@ -38,6 +38,11 @@
> >>  #endif
> >>  #endif
> >>  
> >> +struct usb_port {
> >> +  struct device dev;
> >> +  struct dev_state *port_owner;
> >> +};
> >> +
> >>  struct usb_hub {
> >>    struct device           *intfdev;       /* the "interface" device */
> >>    struct usb_device       *hdev;
> >> @@ -82,7 +87,11 @@ struct usb_hub {
> >>    u8                      indicator[USB_MAXCHILDREN];
> >>    struct delayed_work     leds;
> >>    struct delayed_work     init_work;
> >> -  struct dev_state        **port_owners;
> >> +  struct usb_port         **ports;
> >> +};
> >> +
> >> +struct device_type usb_port_device_type = {
> >> +  .name =         "usb_port",
> >>  };
> > 
> > In looking at this further, why isn't the release field set here?  Why
> > are you setting it directly in the struct device?  Shouldn't the type
> > handle this properly?
> > 
> > Same goes for the uevent, don't you want to generate an event for this
> > device so that userspace can do something with it?
> > 
> > At the least, please set the release type here, not directly in the
> > device itself (yes, it works as-is today, but I'd prefer it to use the
> > type, the way the rest of the usb core does.)
> Ok. I get it. I will add the uevent in the later patch. I also find
> endpoint driver also doesn't release field in the device type. So it
> also should be modified?

Yes.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to