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 <chunfeng.yun@xxxxxxxxxxxx> > > Tested-by: Nagarjuna Kristam <nkristam@xxxxxxxxxx> > > --- > > 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 > > +/*