[PATCH/RFC v4 0/4] usb: role: rcar-usb3-role-switch: add support for R-Car SoCs

[PATCH/RFC v4 0/4] usb: role: rcar-usb3-role-switch: add support for R-Car SoCs

[Yoshihiro Shimoda]

This patch set is based on Felipe's usb.git / testing/next branch
(commit id = 47265c067c0d129f3a0e94bc221293a780af9d78).

I still marked this patch set as "RFC". I would like to know whether
this way is good or not. About last discusstion with Heikki:
 https://patchwork.kernel.org/patch/10397635/

Changes from RFC v3:
 - Rebase latest usb.git / testing/next branch.
 - Add graph parse into device_connection_find_match().
 - Use workqueue to call _usb3_set_mode() in patch 3.
   (I realized renesas_usb3_role_switch_set() cannot run on atomic because
    device_attach() might sleep.)

Changes from RFC v2:
 - Add registering usb role switch into drivers/usb/gadget/udc/renesas_usb3
   because hardware resource (a register) is shared and remove individual
   usb role switch driver/dt-bindings for R-Car.
 - Remove "usb_role_switch_get_by_graph" API because the renesas_usb3 driver
   doesn't need such API now.

Changes from RFC:
 - Remove "device-connection-id" and "usb role switch driver" dt-bingings.
 - Remove drivers/of code.
 - Add a new API for find the connection by using graph on devcon.c and roles.c.
 - Use each new API on the rcar usb role switch and renesas_usb3 drivers.
 - Update the dtsi file for r8a7795.


Yoshihiro Shimoda (4):
  base: devcon: add graph parse in device_connection_find_match()
  usb: gadget: udc: renesas_usb3: Add register of usb role switch
  usb: gadget: udc: renesas_usb3: use usb role switch API
  arm64: dts: renesas: r8a7795: add OF graph for usb role switch

 .../devicetree/bindings/usb/renesas_usb3.txt       |  15 +++
 Documentation/driver-api/device_connection.rst     |   2 +-
 arch/arm64/boot/dts/renesas/r8a7795.dtsi           |  12 +++
 drivers/base/devcon.c                              |  15 +++
 drivers/usb/gadget/udc/Kconfig                     |   1 +
 drivers/usb/gadget/udc/renesas_usb3.c              | 105 ++++++++++++++++++++-
 include/linux/device.h                             |   2 +
 7 files changed, 150 insertions(+), 2 deletions(-)

-- 
1.9.1

[PATCH/RFC v4 1/4] base: devcon: add graph parse in device_connection_find_match()

[Yoshihiro Shimoda]

This patch adds graph parsing in device_connection_find_match().
The match function will be called with fwnode pointer in struct
device_connection. So, a caller can check the matching by using it.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 Documentation/driver-api/device_connection.rst |  2 +-
 drivers/base/devcon.c                          | 15 +++++++++++++++
 include/linux/device.h                         |  2 ++
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Documentation/driver-api/device_connection.rst b/Documentation/driver-api/device_connection.rst
index affbc556..cc6b1da 100644
--- a/Documentation/driver-api/device_connection.rst
+++ b/Documentation/driver-api/device_connection.rst
@@ -19,7 +19,7 @@ Device connections alone do not create a dependency between the two devices.
 They are only descriptions which are not tied to either of the devices directly.
 A dependency between the two devices exists only if one of the two endpoint
 devices requests a reference to the other. The descriptions themselves can be
-defined in firmware (not yet supported) or they can be built-in.
+defined in firmware or they can be built-in.
 
 Usage
 -----
diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
index d427e80..374bb39 100644
--- a/drivers/base/devcon.c
+++ b/drivers/base/devcon.c
@@ -7,6 +7,7 @@
  */
 
 #include <linux/device.h>
+#include <linux/property.h>
 
 static DEFINE_MUTEX(devcon_lock);
 static LIST_HEAD(devcon_list);
@@ -31,10 +32,24 @@ void *device_connection_find_match(struct device *dev, const char *con_id,
 	struct device_connection *con;
 	void *ret = NULL;
 	int ep;
+	struct device_connection graph;
+	struct fwnode_handle *fwnode_ep;
+	struct fwnode_handle *remote;
 
 	if (!match)
 		return NULL;
 
+	fwnode_graph_for_each_endpoint(dev->fwnode, fwnode_ep) {
+		remote = fwnode_graph_get_remote_port_parent(fwnode_ep);
+		if (!remote)
+			continue;
+
+		graph.fwnode = remote;
+		ret = match(&graph, 0, data);
+		if (!IS_ERR(ret))
+			return ret;
+	}
+
 	mutex_lock(&devcon_lock);
 
 	list_for_each_entry(con, &devcon_list, list) {
diff --git a/include/linux/device.h b/include/linux/device.h
index 0059b99..175907b 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -734,11 +734,13 @@ struct device_dma_parameters {
  * struct device_connection - Device Connection Descriptor
  * @endpoint: The names of the two devices connected together
  * @id: Unique identifier for the connection
+ * @fwnode: fwnode pointer for finding a connection from graph
  * @list: List head, private, for internal use only
  */
 struct device_connection {
 	const char		*endpoint[2];
 	const char		*id;
+	struct fwnode_handle	*fwnode;
 	struct list_head	list;
 };
 
-- 
1.9.1

[PATCH/RFC v4 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch

[Yoshihiro Shimoda]

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@renesas.com>
---
 .../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@ee020000 {
@@ -27,6 +30,12 @@ Example of R-Car H3 ES1.x:
 		reg = <0 0xee020000 0 0x400>;
 		interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cpg CPG_MOD 328>;
+
+		port {
+			usb3_peri0_ep: endpoint {
+				remote-endpoint = <&usb3_host0_ep>;
+			};
+		};
 	};
 
 	usb3_peri1: usb@ee060000 {
@@ -35,4 +44,10 @@ Example of R-Car H3 ES1.x:
 		reg = <0 0xee060000 0 0x400>;
 		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cpg CPG_MOD 327>;
+
+		port {
+			usb3_peri1_ep: endpoint {
+				remote-endpoint = <&usb3_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 <linux/uaccess.h>
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
+#include <linux/usb/role.h>
 
 /* 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);
+
+	return 0;
+}
+
 static ssize_t role_store(struct device *dev, struct device_attribute *attr,
 			  const char *buf, size_t count)
 {
@@ -2417,6 +2456,8 @@ static int renesas_usb3_remove(struct platform_device *pdev)
 	debugfs_remove_recursive(usb3->dentry);
 	device_remove_file(&pdev->dev, &dev_attr_role);
 
+	usb_role_switch_unregister(usb3->role_sw);
+
 	usb_del_gadget_udc(&usb3->gadget);
 	renesas_usb3_dma_free_prd(usb3, &pdev->dev);
 
@@ -2573,6 +2614,33 @@ static void renesas_usb3_init_ram(struct renesas_usb3 *usb3, struct device *dev,
 	EXTCON_NONE,
 };
 
+static int usb3_bus_match(struct device *dev, void *fwnode)
+{
+	if (dev->fwnode != fwnode)
+		return 0;
+
+	return of_device_is_compatible(dev->of_node, "renesas,rcar-gen3-xhci");
+}
+
+static void *usb3_usb_role_match(struct device_connection *con, int ep,
+				 void *data)
+{
+	struct device *dev;
+
+	dev = bus_find_device(&platform_bus_type, NULL, con->fwnode,
+			      usb3_bus_match);
+	if (dev)
+		return dev;
+
+	return ERR_PTR(-EPROBE_DEFER);
+}
+
+static struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
+	.set = renesas_usb3_role_switch_set,
+	.get = renesas_usb3_role_switch_get,
+	.allow_userspace_control = true,
+};
+
 static int renesas_usb3_probe(struct platform_device *pdev)
 {
 	struct renesas_usb3 *usb3;
@@ -2658,6 +2726,20 @@ static int renesas_usb3_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_dev_create;
 
+	usb3->role_sw = usb_role_switch_register(&pdev->dev,
+					&renesas_usb3_role_switch_desc);
+	if (!IS_ERR(usb3->role_sw)) {
+		usb3->host_dev = device_connection_find_match(&pdev->dev,
+					NULL, NULL, usb3_usb_role_match);
+		if (IS_ERR_OR_NULL(usb3->host_dev)) {
+			/* If not found, this driver will not use a role sw */
+			usb_role_switch_unregister(usb3->role_sw);
+			usb3->role_sw = NULL;
+		}
+	} else {
+		usb3->role_sw = NULL;
+	}
+
 	usb3->workaround_for_vbus = priv->workaround_for_vbus;
 
 	renesas_usb3_debugfs_init(usb3, &pdev->dev);
-- 
1.9.1

[PATCH/RFC v4 3/4] usb: gadget: udc: renesas_usb3: use usb role switch API

[Yoshihiro Shimoda]

This patch uses usb role switch APIs if the register suceeeded.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/usb/gadget/udc/renesas_usb3.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
index 9667a5e..d6c11c9 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -338,6 +338,8 @@ struct renesas_usb3 {
 
 	struct usb_role_switch *role_sw;
 	struct device *host_dev;
+	struct work_struct role_work;
+	enum usb_role role;
 
 	struct renesas_usb3_ep *usb3_ep;
 	int num_usb3_eps;
@@ -655,7 +657,15 @@ static void usb3_check_vbus(struct renesas_usb3 *usb3)
 	}
 }
 
-static void usb3_set_mode(struct renesas_usb3 *usb3, bool host)
+static void renesas_usb3_role_work(struct work_struct *work)
+{
+	struct renesas_usb3 *usb3 = container_of(work, struct renesas_usb3,
+						 role_work);
+
+	usb_role_switch_set_role(usb3->role_sw, usb3->role);
+}
+
+static void _usb3_set_mode(struct renesas_usb3 *usb3, bool host)
 {
 	if (host)
 		usb3_clear_bit(usb3, DRD_CON_PERI_CON, USB3_DRD_CON);
@@ -663,6 +673,16 @@ static void usb3_set_mode(struct renesas_usb3 *usb3, bool host)
 		usb3_set_bit(usb3, DRD_CON_PERI_CON, USB3_DRD_CON);
 }
 
+static void usb3_set_mode(struct renesas_usb3 *usb3, bool host)
+{
+	if (usb3->role_sw) {
+		usb3->role = host ? USB_ROLE_HOST : USB_ROLE_DEVICE;
+		schedule_work(&usb3->role_work);
+	} else {
+		_usb3_set_mode(usb3, host);
+	}
+}
+
 static void usb3_vbus_out(struct renesas_usb3 *usb3, bool enable)
 {
 	if (enable)
@@ -2328,10 +2348,10 @@ static int renesas_usb3_role_switch_set(struct device *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);
+		_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);
+		_usb3_set_mode(usb3, true);
 		/* This device_attach() might sleep */
 		if (device_attach(host) < 0)
 			dev_err(dev, "device_attach(usb3_port) failed\n");
@@ -2726,6 +2746,7 @@ static int renesas_usb3_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_dev_create;
 
+	INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
 	usb3->role_sw = usb_role_switch_register(&pdev->dev,
 					&renesas_usb3_role_switch_desc);
 	if (!IS_ERR(usb3->role_sw)) {
-- 
1.9.1

[PATCH/RFC v4 4/4] arm64: dts: renesas: r8a7795: add OF graph for usb role switch

[Yoshihiro Shimoda]

This patch adds OF graph properties for usb role switch in r8a7795
into USB3.0 host/peripheral nodes.

TODO:
 - need patches for other SoCs.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a7795.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
index 1d5e3ac..50d3312 100644
--- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
@@ -1746,6 +1746,12 @@
 			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
 			resets = <&cpg 328>;
 			status = "disabled";
+
+			port {
+				usb3_host0_ep: endpoint {
+					remote-endpoint = <&usb3_peri0_ep>;
+				};
+			};
 		};
 
 		usb3_peri0: usb@ee020000 {
@@ -1757,6 +1763,12 @@
 			power-domains = <&sysc R8A7795_PD_ALWAYS_ON>;
 			resets = <&cpg 328>;
 			status = "disabled";
+
+			port {
+				usb3_peri0_ep: endpoint {
+					remote-endpoint = <&usb3_host0_ep>;
+				};
+			};
 		};
 
 		usb_dmac0: dma-controller@e65a0000 {
-- 
1.9.1

Re: [PATCH/RFC v4 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch

[Rob Herring]

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@renesas.com>
> ---
>  .../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

RE: [PATCH/RFC v4 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch

[Yoshihiro Shimoda]

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@renesas.com>
> > ---
> >  .../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

Re: [PATCH/RFC v4 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch

[Rob Herring]

On Wed, May 23, 2018 at 1:52 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> 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@renesas.com>
>> > ---
>> >  .../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&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

Re: [PATCH/RFC v4 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch

[Geert Uytterhoeven]

Hi Rob,

On Wed, May 23, 2018 at 5:00 PM, Rob Herring <robh@kernel.org> wrote:
> On Wed, May 23, 2018 at 1:52 AM, Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> 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 -- geert@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

[Yoshihiro Shimoda]

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 Herring <robh@kernel.org> wrote:
<snip>
> >>> >  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 -- geert@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