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 <yoshihiro.shimoda.uh@xxxxxxxxxxx> > > --- > > .../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. 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. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Documentation/devicetree/bindings/usb/generic.txt?h=v4.17-rc6&id=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
