RE: [PATCH/RFC v4 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch
Hi Rob, Geert-san, > From: Geert Uytterhoeven, Sent: Thursday, May 24, 2018 5:18 PM > > Hi Rob, > > On Wed, May 23, 2018 at 5:00 PM, Rob Herringwrote: > >>> > Optional properties: > >>> >- phys: phandle + phy specifier pair > >>> >- phy-names: must be "usb" > >>> > + - The connection to a usb3.0 host node needs by using OF graph > >>> > bindings for > >>> > +usb role switch. > >>> > + - port@0 = USB3.0 host port. > >>> > >>> On the host side, this might conflict with the USB connector binding. > >>> > >>> I would either make sure this can work with the connector binding by > >>> having 2 endpoints on the HS or SS port or just use the 'companion' > >>> property defined in usb-generic.txt. > >> > >> I don't understand the first one now... This means the renesas_usb3 should > >> follow > >> USB connector binding and have 2 endpoints for the usb role switch to avoid > >> the conflict like below? > >> - port1@0: Super Speed (SS), present in SS capable connectors (from > >> usb-connector.txt). > >> - port1@1: USB3.0 host port. > > > > I'm confused, SS and USB3.0 are the same essentially. It would be: > > > > port@1/endpoint@0: SS host port > > port@1/endpoint@1: SS device port Thank you for the comment. It's better than my description. > >> About the 'companion' in usb-generic.txt, the property intends to be used > >> for EHCI or host side > >> like the commit log [1]. If there is accept to use 'companion' for this > >> patch, I think it will > >> be simple to achieve this role switch feature. However, in last month, I > >> submitted a similar patch [2] > >> that has "renesas,host" property, but I got reply from Andy [3] and Heikki > >> [4]. So, I'm > >> trying to improve the device connection framework [5] now. > > > > I think this case is rare enough that we don't need a general solution > > using OF graph, so I'm fine with a simple, single property to link the > > 2 nodes. Either reusing "companion" or "renesas,host" is fine by me. > > I'd go for the standard "companion" over "renesas,host"[*]. > > [*] Doh, we have another one ("renesas,bonding"), invented when I wasn't > aware of the existence of "companion" yet... Thank you for the comments. So, I'll reuse "companion" for it. Best regards, Yoshihiro Shimoda > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > ge...@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like > that. > -- Linus Torvalds
Re: [PATCH/RFC v4 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch
Hi Rob, On Wed, May 23, 2018 at 5:00 PM, Rob Herringwrote: > On Wed, May 23, 2018 at 1:52 AM, Yoshihiro Shimoda > wrote: >>> From: Rob Herring, Sent: Wednesday, May 23, 2018 2:13 AM >>> On Tue, May 22, 2018 at 09:01:07PM +0900, Yoshihiro Shimoda wrote: >>> > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt >>> > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt >>> > @@ -19,6 +19,9 @@ Required properties: >>> > Optional properties: >>> >- phys: phandle + phy specifier pair >>> >- phy-names: must be "usb" >>> > + - The connection to a usb3.0 host node needs by using OF graph >>> > bindings for >>> > +usb role switch. >>> > + - port@0 = USB3.0 host port. >>> >>> On the host side, this might conflict with the USB connector binding. >>> >>> I would either make sure this can work with the connector binding by >>> having 2 endpoints on the HS or SS port or just use the 'companion' >>> property defined in usb-generic.txt. >> >> I don't understand the first one now... This means the renesas_usb3 should >> follow >> USB connector binding and have 2 endpoints for the usb role switch to avoid >> the conflict like below? >> - port1@0: Super Speed (SS), present in SS capable connectors (from >> usb-connector.txt). >> - port1@1: USB3.0 host port. > > I'm confused, SS and USB3.0 are the same essentially. It would be: > > port@1/endpoint@0: SS host port > port@1/endpoint@1: SS device port > >> About the 'companion' in usb-generic.txt, the property intends to be used >> for EHCI or host side >> like the commit log [1]. If there is accept to use 'companion' for this >> patch, I think it will >> be simple to achieve this role switch feature. However, in last month, I >> submitted a similar patch [2] >> that has "renesas,host" property, but I got reply from Andy [3] and Heikki >> [4]. So, I'm >> trying to improve the device connection framework [5] now. > > I think this case is rare enough that we don't need a general solution > using OF graph, so I'm fine with a simple, single property to link the > 2 nodes. Either reusing "companion" or "renesas,host" is fine by me. I'd go for the standard "companion" over "renesas,host"[*]. [*] Doh, we have another one ("renesas,bonding"), invented when I wasn't aware of the existence of "companion" yet... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH/RFC v4 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch
On Wed, May 23, 2018 at 1:52 AM, Yoshihiro Shimodawrote: > Hi Rob, > > Thank you for the review! > >> From: Rob Herring, Sent: Wednesday, May 23, 2018 2:13 AM >> >> On Tue, May 22, 2018 at 09:01:07PM +0900, Yoshihiro Shimoda wrote: >> > This patch adds role switch support for R-Car SoCs into the USB 3.0 >> > peripheral driver. Some R-Car SoCs (e.g. R-Car H3) have USB 3.0 >> > dual-role device controller which has the USB 3.0 xHCI host and >> > Renesas USB 3.0 peripheral. >> > >> > Unfortunately, the mode change register contains the USB 3.0 peripheral >> > controller side only. So, the USB 3.0 peripheral driver (renesas_usb3) >> > manages this register now. However, in peripheral mode, the host >> > should stop. Also the host hardware needs to reinitialize its own >> > registers when the mode changes from peripheral to host mode. >> > Otherwise, the host cannot work correctly (e.g. detect a device as >> > high-speed). >> > >> > To achieve this by a driver, this role switch driver manages >> > the mode change register and attach/release the xhci-plat driver. >> > >> > Signed-off-by: Yoshihiro Shimoda >> > --- >> > .../devicetree/bindings/usb/renesas_usb3.txt | 15 >> >> Please split bindings to a separate patch. > > Oops. I got it. > >> > drivers/usb/gadget/udc/Kconfig | 1 + >> > drivers/usb/gadget/udc/renesas_usb3.c | 82 >> > ++ >> > 3 files changed, 98 insertions(+) >> > >> > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt >> b/Documentation/devicetree/bindings/usb/renesas_usb3.txt >> > index 2c071bb5..f6105aa 100644 >> > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt >> > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt >> > @@ -19,6 +19,9 @@ Required properties: >> > Optional properties: >> >- phys: phandle + phy specifier pair >> >- phy-names: must be "usb" >> > + - The connection to a usb3.0 host node needs by using OF graph bindings >> > for >> > +usb role switch. >> > + - port@0 = USB3.0 host port. >> >> On the host side, this might conflict with the USB connector binding. >> >> I would either make sure this can work with the connector binding by >> having 2 endpoints on the HS or SS port or just use the 'companion' >> property defined in usb-generic.txt. > > I don't understand the first one now... This means the renesas_usb3 should > follow > USB connector binding and have 2 endpoints for the usb role switch to avoid > the conflict like below? > - port1@0: Super Speed (SS), present in SS capable connectors (from > usb-connector.txt). > - port1@1: USB3.0 host port. I'm confused, SS and USB3.0 are the same essentially. It would be: port@1/endpoint@0: SS host port port@1/endpoint@1: SS device port > About the 'companion' in usb-generic.txt, the property intends to be used for > EHCI or host side > like the commit log [1]. If there is accept to use 'companion' for this > patch, I think it will > be simple to achieve this role switch feature. However, in last month, I > submitted a similar patch [2] > that has "renesas,host" property, but I got reply from Andy [3] and Heikki > [4]. So, I'm > trying to improve the device connection framework [5] now. I think this case is rare enough that we don't need a general solution using OF graph, so I'm fine with a simple, single property to link the 2 nodes. Either reusing "companion" or "renesas,host" is fine by me. Rob > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Documentation/devicetree/bindings/usb/generic.txt?h=v4.17-rc6=5095cb89c62acc78b4cfaeb9a4072979d010510a > > [2] > https://www.spinics.net/lists/linux-usb/msg167773.html > > [3] > https://www.spinics.net/lists/linux-usb/msg167780.html > > [4] > https://www.spinics.net/lists/linux-usb/msg167806.html > > [5] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/driver-api/device_connection.rst > > Best regards, > Yoshihiro Shimoda > >> Rob
Re: [PATCH/RFC v4 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch
On Tue, May 22, 2018 at 09:01:07PM +0900, Yoshihiro Shimoda wrote: > This patch adds role switch support for R-Car SoCs into the USB 3.0 > peripheral driver. Some R-Car SoCs (e.g. R-Car H3) have USB 3.0 > dual-role device controller which has the USB 3.0 xHCI host and > Renesas USB 3.0 peripheral. > > Unfortunately, the mode change register contains the USB 3.0 peripheral > controller side only. So, the USB 3.0 peripheral driver (renesas_usb3) > manages this register now. However, in peripheral mode, the host > should stop. Also the host hardware needs to reinitialize its own > registers when the mode changes from peripheral to host mode. > Otherwise, the host cannot work correctly (e.g. detect a device as > high-speed). > > To achieve this by a driver, this role switch driver manages > the mode change register and attach/release the xhci-plat driver. > > Signed-off-by: Yoshihiro Shimoda> --- > .../devicetree/bindings/usb/renesas_usb3.txt | 15 Please split bindings to a separate patch. > drivers/usb/gadget/udc/Kconfig | 1 + > drivers/usb/gadget/udc/renesas_usb3.c | 82 > ++ > 3 files changed, 98 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > index 2c071bb5..f6105aa 100644 > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > @@ -19,6 +19,9 @@ Required properties: > Optional properties: >- phys: phandle + phy specifier pair >- phy-names: must be "usb" > + - The connection to a usb3.0 host node needs by using OF graph bindings for > +usb role switch. > + - port@0 = USB3.0 host port. On the host side, this might conflict with the USB connector binding. I would either make sure this can work with the connector binding by having 2 endpoints on the HS or SS port or just use the 'companion' property defined in usb-generic.txt. Rob
[PATCH/RFC v4 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch
This patch adds role switch support for R-Car SoCs into the USB 3.0 peripheral driver. Some R-Car SoCs (e.g. R-Car H3) have USB 3.0 dual-role device controller which has the USB 3.0 xHCI host and Renesas USB 3.0 peripheral. Unfortunately, the mode change register contains the USB 3.0 peripheral controller side only. So, the USB 3.0 peripheral driver (renesas_usb3) manages this register now. However, in peripheral mode, the host should stop. Also the host hardware needs to reinitialize its own registers when the mode changes from peripheral to host mode. Otherwise, the host cannot work correctly (e.g. detect a device as high-speed). To achieve this by a driver, this role switch driver manages the mode change register and attach/release the xhci-plat driver. Signed-off-by: Yoshihiro Shimoda--- .../devicetree/bindings/usb/renesas_usb3.txt | 15 drivers/usb/gadget/udc/Kconfig | 1 + drivers/usb/gadget/udc/renesas_usb3.c | 82 ++ 3 files changed, 98 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt b/Documentation/devicetree/bindings/usb/renesas_usb3.txt index 2c071bb5..f6105aa 100644 --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt @@ -19,6 +19,9 @@ Required properties: Optional properties: - phys: phandle + phy specifier pair - phy-names: must be "usb" + - The connection to a usb3.0 host node needs by using OF graph bindings for +usb role switch. + - port@0 = USB3.0 host port. Example of R-Car H3 ES1.x: usb3_peri0: usb@ee02 { @@ -27,6 +30,12 @@ Example of R-Car H3 ES1.x: reg = <0 0xee02 0 0x400>; interrupts = ; clocks = < CPG_MOD 328>; + + port { + usb3_peri0_ep: endpoint { + remote-endpoint = <_host0_ep>; + }; + }; }; usb3_peri1: usb@ee06 { @@ -35,4 +44,10 @@ Example of R-Car H3 ES1.x: reg = <0 0xee06 0 0x400>; interrupts = ; clocks = < CPG_MOD 327>; + + port { + usb3_peri1_ep: endpoint { + remote-endpoint = <_host1_ep>; + }; + }; }; diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig index b838cae..78823cd 100644 --- a/drivers/usb/gadget/udc/Kconfig +++ b/drivers/usb/gadget/udc/Kconfig @@ -193,6 +193,7 @@ config USB_RENESAS_USB3 tristate 'Renesas USB3.0 Peripheral controller' depends on ARCH_RENESAS || COMPILE_TEST depends on EXTCON && HAS_DMA + select USB_ROLE_SWITCH help Renesas USB3.0 Peripheral controller is a USB peripheral controller that supports super, high, and full speed USB 3.0 data transfers. diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c index 5caf78b..9667a5e 100644 --- a/drivers/usb/gadget/udc/renesas_usb3.c +++ b/drivers/usb/gadget/udc/renesas_usb3.c @@ -23,6 +23,7 @@ #include #include #include +#include /* register definitions */ #define USB3_AXI_INT_STA 0x008 @@ -335,6 +336,9 @@ struct renesas_usb3 { struct phy *phy; struct dentry *dentry; + struct usb_role_switch *role_sw; + struct device *host_dev; + struct renesas_usb3_ep *usb3_ep; int num_usb3_eps; @@ -2302,6 +2306,41 @@ static int renesas_usb3_set_selfpowered(struct usb_gadget *gadget, int is_self) .set_selfpowered= renesas_usb3_set_selfpowered, }; +static enum usb_role renesas_usb3_role_switch_get(struct device *dev) +{ + struct renesas_usb3 *usb3 = dev_get_drvdata(dev); + enum usb_role cur_role; + + pm_runtime_get_sync(dev); + cur_role = usb3_is_host(usb3) ? USB_ROLE_HOST : USB_ROLE_DEVICE; + pm_runtime_put(dev); + + return cur_role; +} + +static int renesas_usb3_role_switch_set(struct device *dev, + enum usb_role role) +{ + struct renesas_usb3 *usb3 = dev_get_drvdata(dev); + struct device *host = usb3->host_dev; + enum usb_role cur_role = renesas_usb3_role_switch_get(dev); + + pm_runtime_get_sync(dev); + if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) { + device_release_driver(host); + usb3_set_mode(usb3, false); + } else if (cur_role == USB_ROLE_DEVICE && role == USB_ROLE_HOST) { + /* Must set the mode before device_attach of the host */ + usb3_set_mode(usb3, true); + /* This device_attach() might sleep */ + if (device_attach(host) < 0) + dev_err(dev, "device_attach(usb3_port) failed\n"); + } + pm_runtime_put(dev); +