From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chunfeng Yun Subject: Re: [PATCH v7 09/10] usb: roles: add USB Type-B GPIO connector driver Date: Tue, 25 Jun 2019 16:55:47 +0800 Message-ID: <1561452947.32589.25.camel@mhfsdcap03> References: <1560242680-23844-1-git-send-email-chunfeng.yun@mediatek.com> <1560242680-23844-10-git-send-email-chunfeng.yun@mediatek.com> <20190624095827.GA6501@kuha.fi.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20190624095827.GA6501@kuha.fi.intel.com> Sender: linux-kernel-owner@vger.kernel.org To: Heikki Krogerus Cc: Rob Herring , Greg Kroah-Hartman , Mark Rutland , Matthias Brugger , Adam Thomson , Li Jun , Badhri Jagan Sridharan , Hans de Goede , Andy Shevchenko , Min Guo , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, Biju Das , Linus Walleij , Yu Chen , Nagarjuna Kristam , Felipe Balbi List-Id: devicetree@vger.kernel.org Hi Heikki, On Mon, 2019-06-24 at 12:58 +0300, Heikki Krogerus wrote: > Hi Chunfeng, > > On Tue, Jun 11, 2019 at 04:44:39PM +0800, Chunfeng Yun wrote: > > Due to the requirement of usb-connector.txt binding, the old way > > using extcon to support USB Dual-Role switch is now deprecated > > when use Type-B connector. > > This patch introduces a driver of Type-B connector which typically > > uses an input GPIO to detect USB ID pin, and try to replace the > > function provided by extcon-usb-gpio driver > > I'm sorry for asking this so late, but why is this driver a Type-B > specific driver (I really thought somebody had already asked this > question)? It's mainly used for Type-B connector with ID pin. > > I don't see anything Type-B specific in the driver. It's need add another compatible "usb-b-connector" except the driver provided. > Basically it looks > to me like just a gpio based connection detection driver that would > work fine with for example uAB connectors.. Yes, it is. > > > Signed-off-by: Chunfeng Yun > > Tested-by: Nagarjuna Kristam > > --- > > v7 changes: > > 1. remove macro DEV_PMS_OPS suggested by Andy > > 2. add tested-by Nagarjuna > > > > v6 changes: > > 1. get usb-role-swtich by usb_role_switch_get() > > > > v5 changes: > > 1. put usb_role_switch when error happens suggested by Biju > > 2. don't treat bype-B connector as a virtual device suggested by Rob > > > > v4 changes: > > 1. remove linux/gpio.h suggested by Linus > > 2. put node when error happens > > > > v3 changes: > > 1. treat bype-B connector as a virtual device; > > 2. change file name again > > > > v2 changes: > > 1. file name is changed > > 2. use new compatible > > --- > > drivers/usb/roles/Kconfig | 11 ++ > > drivers/usb/roles/Makefile | 1 + > > drivers/usb/roles/typeb-conn-gpio.c | 284 ++++++++++++++++++++++++++++ > > ..It also drives me crazy that you've put this driver under this > folder. It does not create a role switch so ideally it should not go > under driver/usb/roles/. agree:) > I think a better place for it would be > drivers/usb/misc/, or actually, maybe it should go under > drivers/usb/common/? I'm not sure, but prefer misc/ folder. Hi Greg, would you please give me some suggestions about this? which folder I should put the driver into? > > Could you still rename the driver to something like "usb-gpio.c" or > conn-gpio.c, I think about the name for a long time before, and have some doubt whether it's suitable to add typeb into the name. How about using usb-conn-gpio.c or conn-usb-gpio.c? Thanks a lot > or something else, and also move it under > drivers/usb/misc/ or drivers/usb/common/? > > > 3 files changed, 296 insertions(+) > > create mode 100644 drivers/usb/roles/typeb-conn-gpio.c > > > > diff --git a/drivers/usb/roles/Kconfig b/drivers/usb/roles/Kconfig > > index f8b31aa67526..d1156e18a81a 100644 > > --- a/drivers/usb/roles/Kconfig > > +++ b/drivers/usb/roles/Kconfig > > @@ -26,4 +26,15 @@ config USB_ROLES_INTEL_XHCI > > To compile the driver as a module, choose M here: the module will > > be called intel-xhci-usb-role-switch. > > > > +config TYPEB_CONN_GPIO > > + tristate "USB Type-B GPIO Connector" > > USB GPIO connection detection driver? > > > + depends on GPIOLIB > > + help > > + The driver supports USB role switch between host and device via GPIO > > + based USB cable detection, used typically if an input GPIO is used > > + to detect USB ID pin. > > + > > + To compile the driver as a module, choose M here: the module will > > + be called typeb-conn-gpio.ko > > + > > endif # USB_ROLE_SWITCH > > diff --git a/drivers/usb/roles/Makefile b/drivers/usb/roles/Makefile > > index 757a7d2797eb..5d5620d9d113 100644 > > --- a/drivers/usb/roles/Makefile > > +++ b/drivers/usb/roles/Makefile > > @@ -3,3 +3,4 @@ > > obj-$(CONFIG_USB_ROLE_SWITCH) += roles.o > > roles-y := class.o > > obj-$(CONFIG_USB_ROLES_INTEL_XHCI) += intel-xhci-usb-role-switch.o > > +obj-$(CONFIG_TYPEB_CONN_GPIO) += typeb-conn-gpio.o > > diff --git a/drivers/usb/roles/typeb-conn-gpio.c b/drivers/usb/roles/typeb-conn-gpio.c > > new file mode 100644 > > index 000000000000..e3fba1656069 > > --- /dev/null > > +++ b/drivers/usb/roles/typeb-conn-gpio.c > > @@ -0,0 +1,284 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/*