Re: [RFC PATCH v2 6/7] typec: tcpm: Represent source supply through power_supply class
From: Heikki Krogerus
Date: Mon Nov 27 2017 - 09:16:31 EST
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.
Thanks,
--
heikki