From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S932329AbcKUOp7 (ORCPT <rfc822;w@1wt.eu>);
        Mon, 21 Nov 2016 09:45:59 -0500
Received: from mail.linuxfoundation.org ([140.211.169.12]:37822 "EHLO
        mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1753830AbcKUOp6 (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 21 Nov 2016 09:45:58 -0500
Date: Mon, 21 Nov 2016 15:46:08 +0100
From: Greg KH <gregkh@linuxfoundation.org>
To: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Guenter Roeck <linux@roeck-us.net>,
        Badhri Jagan Sridharan <badhri@google.com>,
        Oliver Neukum <oneukum@suse.com>, linux-kernel@vger.kernel.org,
        linux-usb@vger.kernel.org
Subject: Re: [PATCHv11 2/3] usb: USB Type-C connector class
Message-ID: <20161121144608.GB28410@kroah.com>
References: <20161117105036.133406-1-heikki.krogerus@linux.intel.com>
 <20161117105036.133406-3-heikki.krogerus@linux.intel.com>
 <20161121103311.GA2233@kroah.com>
 <20161121132741.GB18501@kuha.fi.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20161121132741.GB18501@kuha.fi.intel.com>
User-Agent: Mutt/1.7.1 (2016-10-04)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Nov 21, 2016 at 03:27:41PM +0200, Heikki Krogerus wrote:
> 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@linux.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?

If you see a blank Kconfig option, what are you supposed to do with it?
How do you know if you need to enable it or not?  Are you just supposed
to guess?

That is what the help text is for...

thanks,

greg k-h