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

From: Heikki Krogerus
Date: Mon Nov 21 2016 - 08:29:58 EST


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@xxxxxxxxxxxxxxx>
>
> 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