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 -- 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
