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

[Adam Thomson]

On 09 February 2018 15:02, Sebastian Reichel wrote:

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

Ok, fair enough. Will update accordingly.