On Mon, Nov 27, 2017 at 04:54:08PM +0000, Adam Thomson wrote: > On 27 November 2017 14:12, Heikki Krogerus wrote: > > > Hi Adam, > > > > On Fri, Nov 24, 2017 at 02:05:27PM +0000, Adam Thomson wrote: > > > On 24 November 2017 12:19, Heikki Krogerus wrote: > > > > Is it OK to everybody that the type of the psy is changed like that? > > > > Hans?! > > > > > > > > We do have drivers that already change the type, for example > > > > drivers/power/supply/isp1704_charger.c, but what does the user space > > > > expect? The ABI for the power supply class was never documented I > > > > guess. > > > > > > > > > > Hi Heikki, > > > > > > Appreciate your time in reviewing this. > > > > > > Yes, I actually saw that as an example when I considered this approach. I didn't > > > see anything obvious for this in the ABI documentation. Any ideas Sebastian? > > > What is user-space expectation for the 'type' property of a power_supply? I > > > assume having this dynamic is ok given existing drivers can already do something > > > like this, but would be good to have clarification. > > > > > > > I'm not against changing the type, but I think that we should have an > > > > attribute file listing all supported types a psy can have if we go > > > > forward with this. Ideally the type file would just list them as space > > > > separated values, and show the current one with asterisk in front of > > > > it. The output would be similar we have with some of the other files > > > > under /sys/power, at least /sys/power/state, but that would break the > > > > ABI. > > > > > > > > > > I added this as I wanted the user to know what was connected rather than > > > blindly trying to set the 'online' property to enable PPS, even if the attached > > > source partner didn't support this. As you say, am not sure we could change the > > > 'TYPE' property as that to my knowledge has always been a single string. > > > > > > Maybe the addition of a 'SUPPORTED_TYPES' property or something similar could > > > close this gap (as you eluded to), at least by providing a RO list of all > > > supported types? Another option would be to add a type which indicates the > > > supply supports multiple types, and then based on this we can read another > > > property which does as you suggest with multiple strings and one being > > > highlighted? Am certainly open to discussion on this. > > > > It looks like this is USB specific problem. I think the type should > > actually be just USB, and there should be an other USB only attribute > > file which should list the current and supported connection types. > > Well, maybe it does not even need to be USB only. > > I believe in Android (BatteryMonitor) right now it maps the various USB > types that can be reported by a power_supply driver to an internal Android 'AC' > type (or 'USB' for 'PD_DRP') for use in upper level software, so they handle the > various USB reported types but simplify reporting. That code seems like it would > also handle changes in type as well because the type is re-read when dealing > with updates. So there is a wide spread user which already 'copes' with current > driver implementations. That's not saying it's necessarily the correct way to go > in the future but wanted to highlight current usage. > > Personally what you're suggesting seems reasonable to me. I would however like > to get a general consensus on this as I guess this could potentially change > future power_supply driver implementations. Agreed. What ever the final solution will be, the power supply ABI should be documented at the same time. > Sebastian, do you have any thoughts on this topic? Cheers, -- heikki -- 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
