On Thu, Nov 17 2016, Mark Brown wrote: > [ Unknown signature status ] > On Tue, Nov 15, 2016 at 08:35:13AM +1100, NeilBrown wrote: >> On Mon, Nov 14 2016, Mark Brown wrote: > >> > Conflating the two seems like the whole point here. We're looking for >> > something that sits between the power supply code and the USB code and >> > tells the power supply code what it's allowed to do which is the result >> > of a combination of physical cable detection and USB protocol. It seems >> > reasonable that extcon drivers ought to be part of this but it doesn't >> > seem like they are the whole story. > >> I don't think "between the power supply code and the USB code" is where >> this thing sits. I think it sits inside the power-supply driver. >> We already have extcon which sits between the phy and the power_supply >> code, and the usb_notifier which sits between the USB code and the >> power supply code. We don't need another go-between. > > ... > >> correct determinations and set the current limits itself. All this >> could be done entirely internally, without the help of any new >> subsystem. >> Do you agree? > >> Clearly doing it that way would result in lots of code duplication and >> would mean that each driver probably gets its own private set of bugs, >> but it would be possible. Just undesirable. > > I think this is the key here - the fact that it's technically possible > to implement doesn't really matter if it's sufficiently fiddly and non > obvious that nobody is actually joining everything up (bits are done > intermittently but not as a whole as far as I can see). > >> So yes, it makes perfect to provide common code which handles the >> registrations, and captures the events, and translates the different >> events into current levels and feeds those back to the driver. This >> isn't some new subsystem, this is just a resource, provided by a >> library, that power drivers can allocate and initialize if the want to. > > To me that's pretty much what's being done here, the code just happens > to sit in USB instead but fundamentally it's just a blob of helper code, > you could replace the notifier with a callback if that's the big concern > here. It is a lot more than "just a blob of helper code". It duplicates existing infrastructure instead of fixing and using the infrastructure.... but I've said all this before. Repeatedly. NeilBrown