linux-kernel - Re: [PATCH v4 5/7] power: supply: Add 'connected

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180209150148.624fb4qb34wuml6l@earth.universe>
Date:   Fri, 9 Feb 2018 16:01:48 +0100
From:   Sebastian Reichel <sre@...nel.org>
To:     Adam Thomson <Adam.Thomson.Opensource@...semi.com>
Cc:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Hans de Goede <hdegoede@...hat.com>,
        Yueyao Zhu <yueyao.zhu@...il.com>,
        Rui Miguel Silva <rmfrfs@...il.com>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Support Opensource <Support.Opensource@...semi.com>
Subject: Re: [PATCH v4 5/7] power: supply: Add 'connected_type' property and
 supporting code

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@...semi.com>
> >
> > 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@...labora.co.uk>
> >
> > -- 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

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)