From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752653AbbCQIAs (ORCPT ); Tue, 17 Mar 2015 04:00:48 -0400 Received: from mail-we0-f176.google.com ([74.125.82.176]:35201 "EHLO mail-we0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751163AbbCQIAo (ORCPT ); Tue, 17 Mar 2015 04:00:44 -0400 Message-ID: <1426579241.2330.19.camel@linaro.org> Subject: Re: [PATCH v4 1/1] extcon: usb-gpio: Introduce gpio usb extcon driver From: "Ivan T. Ivanov" To: Chanwoo Choi Cc: Roger Quadros , myungjoo.ham@samsung.com, balbi@ti.com, tony@atomide.com, nsekhar@ti.com, devicetree@vger.kernel.org, linux-usb@vger.kernel.org, linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org Date: Tue, 17 Mar 2015 10:00:41 +0200 In-Reply-To: <1426578738.2330.18.camel@linaro.org> References: <1422274532-9488-1-git-send-email-rogerq@ti.com> <1422274532-9488-2-git-send-email-rogerq@ti.com> <54C8D2F4.9050404@ti.com> <54CF4FC7.2050309@ti.com> <1426509149.2330.5.camel@linaro.org> <5506D675.8010006@ti.com> <1426515811.2330.16.camel@linaro.org> <55078B0F.1040800@samsung.com> <1426578738.2330.18.camel@linaro.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.13.7-fta1.2~trusty Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Fixed spelling error. On Tue, 2015-03-17 at 09:52 +0200, Ivan T. Ivanov wrote: > Hi, > > On Tue, 2015-03-17 at 11:01 +0900, Chanwoo Choi wrote: > > Hi Ivan, > > > > On 03/16/2015 11:23 PM, Ivan T. Ivanov wrote: > > > Hi Roger, > > > > > > On Mon, 2015-03-16 at 15:11 +0200, Roger Quadros wrote: > > > > Hi Ivan, > > > > > > > > On 16/03/15 14:32, Ivan T. Ivanov wrote: > > > > > Hi, > > > > > > > > > > On Mon, 2015-02-02 at 12:21 +0200, Roger Quadros wrote: > > > > > > This driver observes the USB ID pin connected over a GPIO and > > > > > > updates the USB cable extcon states accordingly. > > > > > > > > > > > > The existing GPIO extcon driver is not suitable for this purpose > > > > > > as it needs to be taught to understand USB cable states and it > > > > > > can't handle more than one cable per instance. > > > > > > > > > > > > For the USB case we need to handle 2 cable states. > > > > > > 1) USB (attach/detach) > > > > > > 2) USB-HOST (attach/detach) > > > > > > > > > > > > This driver can be easily updated in the future to handle VBUS > > > > > > events in case it happens to be available on GPIO for any platform. > > > > > > > > > > > > Signed-off-by: Roger Quadros > > > > > > --- > > > > > > v4: > > > > > > - got rid of id_irqwake flag. Fail if enable/disable_irq_wake() fails > > > > > > - changed host cable name to "USB-HOST" > > > > > > > > > > I am sorry that I am getting a bit little late into this. > > > > > > > > > > Isn't supposed that we have to use strings defined in > > > > > const char extcon_cable_name[][]? > > > > > > > > > > > > > > > > + > > > > > > +/* List of detectable cables */ > > > > > > +enum { > > > > > > + EXTCON_CABLE_USB = 0, > > > > > > + EXTCON_CABLE_USB_HOST, > > > > > > + > > > > > > > > > > Same here: duplicated with enum extcon_cable_name > > > > > > > > > > > + EXTCON_CABLE_END, > > > > > > +}; > > > > > > + > > > > > > +static const char *usb_extcon_cable[] = { > > > > > > + [EXTCON_CABLE_USB] = "USB", > > > > > > + [EXTCON_CABLE_USB_HOST] = "USB-HOST", > > > > > > + NULL, > > > > > > +}; > > > > > > > > I'm not exactly sure how else it is supposed to work if we > > > > support only a subset of cables from the global extcon_cable_name[][]. > > > > > > I don't see issue that we use just 2 events. I think that we can > > > reuse enum extcon_cable_name > > Now I see that extcon_dev_register() expect NULL terminated array of > pointers, so it will not be possible to use enum extcon_cable_name > as index in the above array, sorry. > > > > and strings already defined in > > > extcon_cable_name[][] global variable. It is defined extern in > > > extcon.h file exactly for this purpose, no? > > > > 'extcon_cable_name' global variable is not used on extcon driver directly. > > It is just recommended cable name. > > Hm, this is what bothers me. How client drivers will know cable name if > every provider start using its own naming scheme? > > If I write client driver I will use: > > extcon_register_interest(obj, name, extcon_cable_name[EXTCON_USB_HOST], nb); > > and this will now work with this driver because it define string differently. ^^^ s/now/not/ > > ... Well, I see that string is changed because your recommendation :-), > then lets fix extcon_cable_name strings and not let drivers define its own > names. > > > > I have plan to use standard cable name for extcon driver instead of that > > each extcon driver define the cable name. > > > > Sound like a good plan :-) > > Regards, > Ivan > > >