linux-kernel - Re: [PATCHv11 2/3] usb: USB Type-C connector class

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]

Date:   Mon, 21 Nov 2016 15:27:41 +0200
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Guenter Roeck <linux@...ck-us.net>,
        Badhri Jagan Sridharan <badhri@...gle.com>,
        Oliver Neukum <oneukum@...e.com>, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org
Subject: Re: [PATCHv11 2/3] usb: USB Type-C connector class

On Mon, Nov 21, 2016 at 11:33:11AM +0100, Greg KH wrote:
> On Thu, Nov 17, 2016 at 12:50:35PM +0200, Heikki Krogerus wrote:
> > The purpose of USB Type-C connector class is to provide
> > unified interface for the user space to get the status and
> > basic information about USB Type-C connectors on a system,
> > control over data role swapping, and when the port supports
> > USB Power Delivery, also control over power role swapping
> > and Alternate Modes.
> > 
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
> 
> I'd like to get some acks or reviewed-by from others, but one thing
> jumped out at me, which implies you did not run checkpatch.pl on the
> patch:
> 
> 644

OK.

> > --- a/drivers/usb/Kconfig
> > +++ b/drivers/usb/Kconfig
> > @@ -152,6 +152,8 @@ source "drivers/usb/phy/Kconfig"
> >  
> >  source "drivers/usb/gadget/Kconfig"
> >  
> > +source "drivers/usb/typec/Kconfig"
> > +
> >  config USB_LED_TRIG
> >  	bool "USB LED Triggers"
> >  	depends on LEDS_CLASS && LEDS_TRIGGERS
> > diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> > index 7791af6..c7f4098 100644
> > --- a/drivers/usb/Makefile
> > +++ b/drivers/usb/Makefile
> > @@ -62,3 +62,5 @@ obj-$(CONFIG_USB_GADGET)	+= gadget/
> >  obj-$(CONFIG_USB_COMMON)	+= common/
> >  
> >  obj-$(CONFIG_USBIP_CORE)	+= usbip/
> > +
> > +obj-$(CONFIG_TYPEC)		+= typec/
> > diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> > new file mode 100644
> > index 0000000..b229fb9
> > --- /dev/null
> > +++ b/drivers/usb/typec/Kconfig
> > @@ -0,0 +1,7 @@
> > +
> > +menu "USB PD and Type-C drivers"
> 
> What is "PD"?  (yes, I know, but very few people do...)  Spell it out.

OK.

> > +
> > +config TYPEC
> > +	tristate
> 
> Hah, that says NOTHING about what this code is at all.

Alone the class driver does nothing. Why would the user need to be
aware of if it when selecting the Type-C drivers, and what can the
user use that information for?


Thanks,

-- 
heikki