Re: [PATCH v4 5/7] power: supply: Add 'connected_type' property and supporting code

[Sebastian Reichel]

Hi,

On Fri, Feb 09, 2018 at 11:28:42AM +0000, Adam Thomson wrote:
> On 08 February 2018 21:31, Sebastian Reichel wrote:
> > On Tue, Jan 02, 2018 at 03:50:53PM +0000, Adam Thomson wrote:
> > > This commit adds the 'connected_type' property to represent supplies
> > > which can report a number of different types of supply based on a
> > > connection event.
> > >
> > > Examples of this already exist in drivers whereby the existing 'type'
> > > property is updated, based on an event, to represent what was
> > > connected (e.g. USB_DCP, USB_ACA, ...). Current implementations
> > > however don't show all supported connectable types, so this knowledge
> > > has to be exlicitly known for each driver that supports this.
> > >
> > > The 'connected_type' property is intended to fill this void and show
> > > users all possible types supported by a driver. The property, when
> > > read, shows all available types for the driver, and the one currently
> > > chosen is highlighted/bracketed. It is expected that the 'type'
> > > property would then just show the top-level type, such as 'USB', and
> > > this would be static.
> > >
> > > Currently the 'conn_type' enum contains all of the USB variant types
> > > that exist for the 'type' enum at this time, and in addition has
> > > the PPS type. In the future this can be extended further for other
> > > types which have multiple connected types supported. The mirroring
> > > is intentional so as to not impact existing usage of the 'type'
> > > property.
> > >
> > > Signed-off-by: Adam Thomson <Adam.Thomson.Opensource@xxxxxxxxxxx>
> >
> > Thanks for the patch. I think, that it's a good idea to provide the
> > subtype in its own property and just set the type property to "USB".
> > I would prefer to name this "usb_type". Otherwise
> >
> > Reviewed-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxxxx>
> >
> > -- Sebastian
> 
> Thanks for your review. I chose 'connected_type' as I thought this would be more
> generic in case other main types would have some dynamic connection element to
> them and could also benefit from this. If we don't see that as being something
> we want though then I can make this just for USB.

If such a thing appears at some point we can add another property.
I think usb_type is easier to understand from a user point of view
and it helps to keep our enums clean.

-- Sebastian

Attachment: signature.asc
Description: PGP signature