Re: [PATCH 3/3] usb : Add sysfs files to control usb port's power

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, Jun 14, 2012 at 09:57:37AM -0400, Alan Stern wrote:
> On Wed, 13 Jun 2012, Greg KH wrote:
> 
> > On Wed, Jun 13, 2012 at 05:15:08PM -0400, Alan Stern wrote:
> > > On Wed, 13 Jun 2012, Greg KH wrote:
> > > 
> > > > > +	struct device_attribute	port_control_attr;
> > > > > +	struct device_attribute	port_state_attr;
> > > > > +	enum port_power_policy port_power_policy;
> > > > 
> > > > Why do you need an attribute per port here?  Shouldn't they just be
> > > > static variables?  Why duplicate them for every port?
> > > 
> > > If they were static, there would be no way for the store and show
> > > methods to know which port they were called for.  That's because the
> > > ports aren't separate kobjects; all these port attributes are bound to
> > > the hub device.
> > 
> > Ports should be a struct device if we are going to hang attributes off
> > of them, otherwise userspace can get very confused.
> 
> Is that really appropriate?  Ports don't have their own driver, they
> can't be probed or removed, they can't be suspended or resumed, you
> can't do I/O to them, they don't have resources, they don't have 
> children...

Ah, but they can have "children" if we want to move the devices attached
to them there.  But that's something to consider in the future, not for
now.

As for the other things, if we applied those rules to all other parts of
the device tree, well, it would be a lot smaller :)

You are showing something "logical" in the tree, that exists, and has
attributes (power, etc.).  Because of that, userspace needs to be
notified of them, as it should be doing things with them.  So, make them
a device so that userspace can actually see them.  Otherwise things
break down really quickly, and you can not use the "standard" userspace
tools to control them (i.e. libusb and friends.)

thanks,

greg k-h