[PATCH v2 0/7] Add USB3.0 and TI HD3SS3220 driver support

[PATCH v2 0/7] Add USB3.0 and TI HD3SS3220 driver support

[Biju Das]

This series adds USB 3.0 support for the CAT874 platform, including a
new driver for the TI HD3SS3220 USB Type-C DRP port controller.

This patch series supports:
1) Host hotplug operation
2) Device hot plug operation
3) USB type-C data_role switch
   (Tested with 2 RZ/G2E boards connected with a Type-C cable)

This patchset is based on linux_next next-20190306 branch.

V1-->V2
  * Use USB role class instead of extcon to receive connect and disconnect
    events and also for the dual role switch.
  * Dropped patch 6
  * Squashed patch 8 and patch 9
  * https://patchwork.kernel.org/cover/10840641/


Biju Das (7):
  dt-bindings: usb: hd3ss3220 device tree binding document
  dt-bindings: usb: renesas_usb3: add usb-role-switch property
  usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller
  usb: gadget: udc: renesas_usb3: Use usb_role_switch framework
  arm64: defconfig: enable TYPEC_HD3SS3220 config option
  arm64: dts: renesas: r8a774c0-cat874: Enable USB3.0 host/peripheral
    device node
  arm64: dts: renesas: r8a774c0-cat874: Enable usb role switch support

 .../devicetree/bindings/usb/renesas_usb3.txt       |  22 ++
 .../devicetree/bindings/usb/ti,hd3ss3220.txt       |  37 +++
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts    |  56 +++++
 arch/arm64/configs/defconfig                       |   2 +
 drivers/usb/gadget/udc/renesas_usb3.c              | 125 ++++++++--
 drivers/usb/typec/Kconfig                          |  10 +
 drivers/usb/typec/Makefile                         |   1 +
 drivers/usb/typec/hd3ss3220.c                      | 266 +++++++++++++++++++++
 8 files changed, 497 insertions(+), 22 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
 create mode 100644 drivers/usb/typec/hd3ss3220.c

-- 
2.7.4

[PATCH v2 1/7] dt-bindings: usb: hd3ss3220 device tree binding document

[Biju Das]

Add device tree binding document for TI HD3SS3220 Type-C DRP port
controller driver.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
V1-->V2
  * Added connector node.
  * updated the example with connector node.
---
 .../devicetree/bindings/usb/ti,hd3ss3220.txt       | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt

diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
new file mode 100644
index 0000000..7f41400
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
@@ -0,0 +1,37 @@
+TI HD3SS3220 TypeC DRP Port Controller.
+
+Required properties:
+ - compatible: Must be "ti,hd3ss3220".
+ - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin.
+ - interrupts: <a b> where a is the interrupt number and b represents an
+   encoding of the sense and level information for the interrupt.
+
+Required sub-node:
+ - connector : The "usb-c-connector" attached to the hd3ss3220 chip. The
+   bindings of the connector node are specified in:
+
+	Documentation/devicetree/bindings/connector/usb-connector.txt
+
+Example:
+hd3ss3220@47 {
+	compatible = "ti,hd3ss3220";
+	reg = <0x47>;
+	interrupt-parent = <&gpio6>;
+	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+
+	usb_con: connector {
+		compatible = "usb-c-connector";
+		label = "USB-C";
+		data-role = "dual";
+	};
+
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		hd3ss3220_ep: endpoint@0 {
+			reg = <0>;
+			remote-endpoint = <&usb3peri_role_switch>;
+		};
+	};
+};
-- 
2.7.4

[v2,1/7] dt-bindings: usb: hd3ss3220 device tree binding document

[Biju Das]

Add device tree binding document for TI HD3SS3220 Type-C DRP port
controller driver.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
V1-->V2
  * Added connector node.
  * updated the example with connector node.
---
 .../devicetree/bindings/usb/ti,hd3ss3220.txt       | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt

diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
new file mode 100644
index 0000000..7f41400
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
@@ -0,0 +1,37 @@
+TI HD3SS3220 TypeC DRP Port Controller.
+
+Required properties:
+ - compatible: Must be "ti,hd3ss3220".
+ - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin.
+ - interrupts: <a b> where a is the interrupt number and b represents an
+   encoding of the sense and level information for the interrupt.
+
+Required sub-node:
+ - connector : The "usb-c-connector" attached to the hd3ss3220 chip. The
+   bindings of the connector node are specified in:
+
+	Documentation/devicetree/bindings/connector/usb-connector.txt
+
+Example:
+hd3ss3220@47 {
+	compatible = "ti,hd3ss3220";
+	reg = <0x47>;
+	interrupt-parent = <&gpio6>;
+	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+
+	usb_con: connector {
+		compatible = "usb-c-connector";
+		label = "USB-C";
+		data-role = "dual";
+	};
+
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		hd3ss3220_ep: endpoint@0 {
+			reg = <0>;
+			remote-endpoint = <&usb3peri_role_switch>;
+		};
+	};
+};

[PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Biju Das]

Add support for renesas_usb3 to support dual role switch
using usb role switch class framework.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
 V1-->V2
  * Added usb-role-switch-property
  * Updated the example with usb-role-switch property.
---
 .../devicetree/bindings/usb/renesas_usb3.txt       | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
index 35039e7..eecaf4c 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
@@ -22,6 +22,7 @@ Required properties:
 Optional properties:
   - phys: phandle + phy specifier pair
   - phy-names: must be "usb"
+  - usb-role-switch: use USB role switch to support dual-role switch
 
 Example of R-Car H3 ES1.x:
 	usb3_peri0: usb@ee020000 {
@@ -39,3 +40,24 @@ Example of R-Car H3 ES1.x:
 		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cpg CPG_MOD 327>;
 	};
+
+Example of RZ/G2E:
+	usb3_peri0: usb@ee020000 {
+		compatible = "renesas,r8a774c0-usb3-peri",
+			     "renesas,rcar-gen3-usb3-peri";
+		reg = <0 0xee020000 0 0x400>;
+		interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cpg CPG_MOD 328>;
+		companion = <&xhci0>;
+		usb-role-switch;
+
+		port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			usb3peri_role_switch: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&hd3ss3220_ep>;
+			};
+		};
+	};
-- 
2.7.4

[v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Biju Das]

Add support for renesas_usb3 to support dual role switch
using usb role switch class framework.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
 V1-->V2
  * Added usb-role-switch-property
  * Updated the example with usb-role-switch property.
---
 .../devicetree/bindings/usb/renesas_usb3.txt       | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
index 35039e7..eecaf4c 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
@@ -22,6 +22,7 @@ Required properties:
 Optional properties:
   - phys: phandle + phy specifier pair
   - phy-names: must be "usb"
+  - usb-role-switch: use USB role switch to support dual-role switch
 
 Example of R-Car H3 ES1.x:
 	usb3_peri0: usb@ee020000 {
@@ -39,3 +40,24 @@ Example of R-Car H3 ES1.x:
 		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&cpg CPG_MOD 327>;
 	};
+
+Example of RZ/G2E:
+	usb3_peri0: usb@ee020000 {
+		compatible = "renesas,r8a774c0-usb3-peri",
+			     "renesas,rcar-gen3-usb3-peri";
+		reg = <0 0xee020000 0 0x400>;
+		interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cpg CPG_MOD 328>;
+		companion = <&xhci0>;
+		usb-role-switch;
+
+		port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			usb3peri_role_switch: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&hd3ss3220_ep>;
+			};
+		};
+	};

[PATCH v2 3/7] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller

[Biju Das]

Driver for TI HD3SS3220 USB Type-C DRP port controller.

The driver currently registers the port and supports data role swapping.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
 V1-->V2
   * Driver uses usb role class instead of extcon for dual role switch
     and also handles connect/disconnect events.
---
 drivers/usb/typec/Kconfig     |  10 ++
 drivers/usb/typec/Makefile    |   1 +
 drivers/usb/typec/hd3ss3220.c | 266 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 277 insertions(+)
 create mode 100644 drivers/usb/typec/hd3ss3220.c

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 89d9193..92a3717 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -50,6 +50,16 @@ source "drivers/usb/typec/tcpm/Kconfig"
 
 source "drivers/usb/typec/ucsi/Kconfig"
 
+config TYPEC_HD3SS3220
+	tristate "TI HD3SS3220 Type-C DRP Port controller driver"
+	depends on I2C
+	help
+	  Say Y or M here if your system has TI HD3SS3220 Type-C DRP Port
+	  controller driver.
+
+	  If you choose to build this driver as a dynamically linked module, the
+	  module will be called hd3ss3220.ko.
+
 config TYPEC_TPS6598X
 	tristate "TI TPS6598x USB Power Delivery controller driver"
 	depends on I2C
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 6696b72..7753a5c3 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -4,5 +4,6 @@ typec-y				:= class.o mux.o bus.o
 obj-$(CONFIG_TYPEC)		+= altmodes/
 obj-$(CONFIG_TYPEC_TCPM)	+= tcpm/
 obj-$(CONFIG_TYPEC_UCSI)	+= ucsi/
+obj-$(CONFIG_TYPEC_HD3SS3220)	+= hd3ss3220.o
 obj-$(CONFIG_TYPEC_TPS6598X)	+= tps6598x.o
 obj-$(CONFIG_TYPEC)		+= mux/
diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
new file mode 100644
index 0000000..e7a2012
--- /dev/null
+++ b/drivers/usb/typec/hd3ss3220.c
@@ -0,0 +1,266 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * TI HD3SS3220 Type-C DRP Port Controller Driver
+ *
+ * Copyright (C) 2019 Renesas Electronics Corp.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/usb/role.h>
+#include <linux/irqreturn.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/usb/typec.h>
+#include <linux/delay.h>
+
+#define HD3SS3220_REG_CN_STAT_CTRL	0x09
+#define HD3SS3220_REG_GEN_CTRL		0x0A
+#define HD3SS3220_REG_DEV_REV		0xA0
+
+/* Register HD3SS3220_REG_CN_STAT_CTRL*/
+#define HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK	(BIT(7) | BIT(6))
+#define HD3SS3220_REG_CN_STAT_CTRL_AS_DFP		BIT(6)
+#define HD3SS3220_REG_CN_STAT_CTRL_AS_UFP		BIT(7)
+#define HD3SS3220_REG_CN_STAT_CTRL_TO_ACCESSORY		(BIT(7) | BIT(6))
+#define HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS		BIT(4)
+
+/* Register HD3SS3220_REG_GEN_CTRL*/
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK		(BIT(2) | BIT(1))
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT	0x00
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK	BIT(1)
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC	(BIT(2) | BIT(1))
+
+struct hd3ss3220 {
+	struct device *dev;
+	struct regmap *regmap;
+	struct usb_role_switch	*role_sw;
+	struct typec_port *port;
+	struct typec_capability typec_cap;
+};
+
+static int hd3ss3220_set_source_pref(struct hd3ss3220 *hd3ss3220, int src_pref)
+{
+	unsigned int reg_val;
+	int ret;
+
+	ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_GEN_CTRL, &reg_val);
+	if (ret < 0)
+		return ret;
+
+	reg_val &= ~HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK;
+	reg_val |= src_pref;
+
+	return regmap_write(hd3ss3220->regmap, HD3SS3220_REG_GEN_CTRL, reg_val);
+}
+
+static enum usb_role hd3ss3220_get_attached_state(struct hd3ss3220 *hd3ss3220)
+{
+	unsigned int reg_val;
+	enum usb_role attached_state;
+	int ret;
+
+	ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
+			  &reg_val);
+	if (ret < 0)
+		return ret;
+
+	switch (reg_val & HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK) {
+	case HD3SS3220_REG_CN_STAT_CTRL_AS_DFP:
+		attached_state = USB_ROLE_HOST;
+	break;
+	case HD3SS3220_REG_CN_STAT_CTRL_AS_UFP:
+		attached_state = USB_ROLE_DEVICE;
+	break;
+	default:
+		attached_state = USB_ROLE_NONE;
+	}
+
+	return attached_state;
+}
+
+static int hd3ss3220_dr_set(const struct typec_capability *cap,
+			    enum typec_data_role role)
+{
+	struct hd3ss3220 *hd3ss3220 = container_of(cap, struct hd3ss3220,
+						   typec_cap);
+	enum usb_role role_val;
+	int pref, ret = 0;
+
+	if (role == TYPEC_HOST) {
+		role_val = USB_ROLE_HOST;
+		pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC;
+	} else {
+		role_val = USB_ROLE_DEVICE;
+		pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK;
+	}
+
+	ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
+	usleep_range(10, 100);
+
+	usb_role_switch_set_role(hd3ss3220->role_sw, role_val);
+	typec_set_data_role(hd3ss3220->port, role_val);
+
+	return ret;
+}
+
+static void hd3ss3220_set_role(struct hd3ss3220 *hd3ss3220)
+{
+	enum usb_role role_state = hd3ss3220_get_attached_state(hd3ss3220);
+
+	usb_role_switch_set_role(hd3ss3220->role_sw, role_state);
+	if (role_state == USB_ROLE_NONE)
+		hd3ss3220_set_source_pref(hd3ss3220,
+				HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
+
+	switch (role_state) {
+	case USB_ROLE_HOST:
+		typec_set_data_role(hd3ss3220->port, TYPEC_HOST);
+		break;
+	case USB_ROLE_DEVICE:
+		typec_set_data_role(hd3ss3220->port, TYPEC_DEVICE);
+		break;
+	default:
+		break;
+	}
+}
+
+irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220)
+{
+	int err;
+	unsigned int data;
+
+	hd3ss3220_set_role(hd3ss3220);
+
+	err = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL, &data);
+	if (err < 0)
+		return IRQ_NONE;
+
+	err = regmap_write(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
+			   data | HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS);
+	if (err < 0)
+		return IRQ_NONE;
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t hd3ss3220_irq_handler(int irq, void *data)
+{
+	struct i2c_client *client = to_i2c_client(data);
+	struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
+
+	return hd3ss3220_irq(hd3ss3220);
+}
+
+static const struct regmap_config config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0x0A,
+};
+
+static int hd3ss3220_probe(struct i2c_client *client,
+		const struct i2c_device_id *id)
+{
+	struct hd3ss3220 *hd3ss3220;
+	int ret;
+	unsigned int data;
+
+	hd3ss3220 = devm_kzalloc(&client->dev, sizeof(struct hd3ss3220),
+				 GFP_KERNEL);
+	if (!hd3ss3220)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, hd3ss3220);
+
+	hd3ss3220->regmap = devm_regmap_init_i2c(client, &config);
+	if (IS_ERR(hd3ss3220->regmap))
+		return PTR_ERR(hd3ss3220->regmap);
+
+	hd3ss3220_set_source_pref(hd3ss3220,
+				  HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
+	hd3ss3220->role_sw = usb_role_switch_get(&client->dev);
+	if (IS_ERR(hd3ss3220->role_sw))
+		return PTR_ERR(hd3ss3220->role_sw);
+
+	hd3ss3220->dev = &client->dev;
+	hd3ss3220->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
+	hd3ss3220->typec_cap.dr_set = hd3ss3220_dr_set;
+	hd3ss3220->typec_cap.type = TYPEC_PORT_DRP;
+	hd3ss3220->typec_cap.data = TYPEC_PORT_DRD;
+
+	hd3ss3220->port = typec_register_port(&client->dev,
+					      &hd3ss3220->typec_cap);
+	if (IS_ERR(hd3ss3220->port))
+		return PTR_ERR(hd3ss3220->port);
+
+	hd3ss3220_set_role(hd3ss3220);
+	if (client->irq > 0) {
+		ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
+				  &data);
+		if (ret < 0)
+			goto error;
+
+		if (data & HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS) {
+			ret = regmap_write(hd3ss3220->regmap,
+				HD3SS3220_REG_CN_STAT_CTRL,
+				data | HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS);
+			if (ret < 0)
+				goto error;
+		}
+
+		ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+					hd3ss3220_irq_handler,
+					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					"hd3ss3220", &client->dev);
+		if (ret)
+			goto error;
+	}
+
+	ret = i2c_smbus_read_byte_data(client, HD3SS3220_REG_DEV_REV);
+	if (ret < 0)
+		goto error;
+
+	dev_info(&client->dev, "probed revision=0x%x\n", ret);
+
+	return 0;
+error:
+	typec_unregister_port(hd3ss3220->port);
+	usb_role_switch_put(hd3ss3220->role_sw);
+
+	return ret;
+}
+
+static int hd3ss3220_remove(struct i2c_client *client)
+{
+	struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
+
+	typec_unregister_port(hd3ss3220->port);
+	usb_role_switch_put(hd3ss3220->role_sw);
+
+	return 0;
+}
+
+static const struct of_device_id dev_ids[] = {
+	{ .compatible = "ti,hd3ss3220"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, dev_ids);
+
+static struct i2c_driver hd3ss3220_driver = {
+	.driver = {
+		.name = "hd3ss3220",
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(dev_ids),
+	},
+	.probe = hd3ss3220_probe,
+	.remove =  hd3ss3220_remove,
+};
+
+module_i2c_driver(hd3ss3220_driver);
+
+MODULE_AUTHOR("Biju Das <biju.das@bp.renesas.com>");
+MODULE_DESCRIPTION("TI HD3SS3220 DRP Port Controller Driver");
+MODULE_LICENSE("GPL");
-- 
2.7.4

[v2,3/7] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller

[Biju Das]

Driver for TI HD3SS3220 USB Type-C DRP port controller.

The driver currently registers the port and supports data role swapping.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
 V1-->V2
   * Driver uses usb role class instead of extcon for dual role switch
     and also handles connect/disconnect events.
---
 drivers/usb/typec/Kconfig     |  10 ++
 drivers/usb/typec/Makefile    |   1 +
 drivers/usb/typec/hd3ss3220.c | 266 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 277 insertions(+)
 create mode 100644 drivers/usb/typec/hd3ss3220.c

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 89d9193..92a3717 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -50,6 +50,16 @@ source "drivers/usb/typec/tcpm/Kconfig"
 
 source "drivers/usb/typec/ucsi/Kconfig"
 
+config TYPEC_HD3SS3220
+	tristate "TI HD3SS3220 Type-C DRP Port controller driver"
+	depends on I2C
+	help
+	  Say Y or M here if your system has TI HD3SS3220 Type-C DRP Port
+	  controller driver.
+
+	  If you choose to build this driver as a dynamically linked module, the
+	  module will be called hd3ss3220.ko.
+
 config TYPEC_TPS6598X
 	tristate "TI TPS6598x USB Power Delivery controller driver"
 	depends on I2C
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 6696b72..7753a5c3 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -4,5 +4,6 @@ typec-y				:= class.o mux.o bus.o
 obj-$(CONFIG_TYPEC)		+= altmodes/
 obj-$(CONFIG_TYPEC_TCPM)	+= tcpm/
 obj-$(CONFIG_TYPEC_UCSI)	+= ucsi/
+obj-$(CONFIG_TYPEC_HD3SS3220)	+= hd3ss3220.o
 obj-$(CONFIG_TYPEC_TPS6598X)	+= tps6598x.o
 obj-$(CONFIG_TYPEC)		+= mux/
diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
new file mode 100644
index 0000000..e7a2012
--- /dev/null
+++ b/drivers/usb/typec/hd3ss3220.c
@@ -0,0 +1,266 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * TI HD3SS3220 Type-C DRP Port Controller Driver
+ *
+ * Copyright (C) 2019 Renesas Electronics Corp.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/usb/role.h>
+#include <linux/irqreturn.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/usb/typec.h>
+#include <linux/delay.h>
+
+#define HD3SS3220_REG_CN_STAT_CTRL	0x09
+#define HD3SS3220_REG_GEN_CTRL		0x0A
+#define HD3SS3220_REG_DEV_REV		0xA0
+
+/* Register HD3SS3220_REG_CN_STAT_CTRL*/
+#define HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK	(BIT(7) | BIT(6))
+#define HD3SS3220_REG_CN_STAT_CTRL_AS_DFP		BIT(6)
+#define HD3SS3220_REG_CN_STAT_CTRL_AS_UFP		BIT(7)
+#define HD3SS3220_REG_CN_STAT_CTRL_TO_ACCESSORY		(BIT(7) | BIT(6))
+#define HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS		BIT(4)
+
+/* Register HD3SS3220_REG_GEN_CTRL*/
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK		(BIT(2) | BIT(1))
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT	0x00
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK	BIT(1)
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC	(BIT(2) | BIT(1))
+
+struct hd3ss3220 {
+	struct device *dev;
+	struct regmap *regmap;
+	struct usb_role_switch	*role_sw;
+	struct typec_port *port;
+	struct typec_capability typec_cap;
+};
+
+static int hd3ss3220_set_source_pref(struct hd3ss3220 *hd3ss3220, int src_pref)
+{
+	unsigned int reg_val;
+	int ret;
+
+	ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_GEN_CTRL, &reg_val);
+	if (ret < 0)
+		return ret;
+
+	reg_val &= ~HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK;
+	reg_val |= src_pref;
+
+	return regmap_write(hd3ss3220->regmap, HD3SS3220_REG_GEN_CTRL, reg_val);
+}
+
+static enum usb_role hd3ss3220_get_attached_state(struct hd3ss3220 *hd3ss3220)
+{
+	unsigned int reg_val;
+	enum usb_role attached_state;
+	int ret;
+
+	ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
+			  &reg_val);
+	if (ret < 0)
+		return ret;
+
+	switch (reg_val & HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK) {
+	case HD3SS3220_REG_CN_STAT_CTRL_AS_DFP:
+		attached_state = USB_ROLE_HOST;
+	break;
+	case HD3SS3220_REG_CN_STAT_CTRL_AS_UFP:
+		attached_state = USB_ROLE_DEVICE;
+	break;
+	default:
+		attached_state = USB_ROLE_NONE;
+	}
+
+	return attached_state;
+}
+
+static int hd3ss3220_dr_set(const struct typec_capability *cap,
+			    enum typec_data_role role)
+{
+	struct hd3ss3220 *hd3ss3220 = container_of(cap, struct hd3ss3220,
+						   typec_cap);
+	enum usb_role role_val;
+	int pref, ret = 0;
+
+	if (role == TYPEC_HOST) {
+		role_val = USB_ROLE_HOST;
+		pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC;
+	} else {
+		role_val = USB_ROLE_DEVICE;
+		pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK;
+	}
+
+	ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
+	usleep_range(10, 100);
+
+	usb_role_switch_set_role(hd3ss3220->role_sw, role_val);
+	typec_set_data_role(hd3ss3220->port, role_val);
+
+	return ret;
+}
+
+static void hd3ss3220_set_role(struct hd3ss3220 *hd3ss3220)
+{
+	enum usb_role role_state = hd3ss3220_get_attached_state(hd3ss3220);
+
+	usb_role_switch_set_role(hd3ss3220->role_sw, role_state);
+	if (role_state == USB_ROLE_NONE)
+		hd3ss3220_set_source_pref(hd3ss3220,
+				HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
+
+	switch (role_state) {
+	case USB_ROLE_HOST:
+		typec_set_data_role(hd3ss3220->port, TYPEC_HOST);
+		break;
+	case USB_ROLE_DEVICE:
+		typec_set_data_role(hd3ss3220->port, TYPEC_DEVICE);
+		break;
+	default:
+		break;
+	}
+}
+
+irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220)
+{
+	int err;
+	unsigned int data;
+
+	hd3ss3220_set_role(hd3ss3220);
+
+	err = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL, &data);
+	if (err < 0)
+		return IRQ_NONE;
+
+	err = regmap_write(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
+			   data | HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS);
+	if (err < 0)
+		return IRQ_NONE;
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t hd3ss3220_irq_handler(int irq, void *data)
+{
+	struct i2c_client *client = to_i2c_client(data);
+	struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
+
+	return hd3ss3220_irq(hd3ss3220);
+}
+
+static const struct regmap_config config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0x0A,
+};
+
+static int hd3ss3220_probe(struct i2c_client *client,
+		const struct i2c_device_id *id)
+{
+	struct hd3ss3220 *hd3ss3220;
+	int ret;
+	unsigned int data;
+
+	hd3ss3220 = devm_kzalloc(&client->dev, sizeof(struct hd3ss3220),
+				 GFP_KERNEL);
+	if (!hd3ss3220)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, hd3ss3220);
+
+	hd3ss3220->regmap = devm_regmap_init_i2c(client, &config);
+	if (IS_ERR(hd3ss3220->regmap))
+		return PTR_ERR(hd3ss3220->regmap);
+
+	hd3ss3220_set_source_pref(hd3ss3220,
+				  HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
+	hd3ss3220->role_sw = usb_role_switch_get(&client->dev);
+	if (IS_ERR(hd3ss3220->role_sw))
+		return PTR_ERR(hd3ss3220->role_sw);
+
+	hd3ss3220->dev = &client->dev;
+	hd3ss3220->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
+	hd3ss3220->typec_cap.dr_set = hd3ss3220_dr_set;
+	hd3ss3220->typec_cap.type = TYPEC_PORT_DRP;
+	hd3ss3220->typec_cap.data = TYPEC_PORT_DRD;
+
+	hd3ss3220->port = typec_register_port(&client->dev,
+					      &hd3ss3220->typec_cap);
+	if (IS_ERR(hd3ss3220->port))
+		return PTR_ERR(hd3ss3220->port);
+
+	hd3ss3220_set_role(hd3ss3220);
+	if (client->irq > 0) {
+		ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
+				  &data);
+		if (ret < 0)
+			goto error;
+
+		if (data & HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS) {
+			ret = regmap_write(hd3ss3220->regmap,
+				HD3SS3220_REG_CN_STAT_CTRL,
+				data | HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS);
+			if (ret < 0)
+				goto error;
+		}
+
+		ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+					hd3ss3220_irq_handler,
+					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+					"hd3ss3220", &client->dev);
+		if (ret)
+			goto error;
+	}
+
+	ret = i2c_smbus_read_byte_data(client, HD3SS3220_REG_DEV_REV);
+	if (ret < 0)
+		goto error;
+
+	dev_info(&client->dev, "probed revision=0x%x\n", ret);
+
+	return 0;
+error:
+	typec_unregister_port(hd3ss3220->port);
+	usb_role_switch_put(hd3ss3220->role_sw);
+
+	return ret;
+}
+
+static int hd3ss3220_remove(struct i2c_client *client)
+{
+	struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
+
+	typec_unregister_port(hd3ss3220->port);
+	usb_role_switch_put(hd3ss3220->role_sw);
+
+	return 0;
+}
+
+static const struct of_device_id dev_ids[] = {
+	{ .compatible = "ti,hd3ss3220"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, dev_ids);
+
+static struct i2c_driver hd3ss3220_driver = {
+	.driver = {
+		.name = "hd3ss3220",
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(dev_ids),
+	},
+	.probe = hd3ss3220_probe,
+	.remove =  hd3ss3220_remove,
+};
+
+module_i2c_driver(hd3ss3220_driver);
+
+MODULE_AUTHOR("Biju Das <biju.das@bp.renesas.com>");
+MODULE_DESCRIPTION("TI HD3SS3220 DRP Port Controller Driver");
+MODULE_LICENSE("GPL");

[PATCH v2 4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework

[Biju Das]

RZ/G2E cat874 board is capable of detecting cable connect and disconnect
events. Add support for renesas_usb3 to receive connect and disconnect
events and support dual-role switch using usb-role-switch framework.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
 V1-->V2
   * Driver uses usb role clas for handling dual role switch and handling
     connect/disconnect events instead of extcon.
---
 drivers/usb/gadget/udc/renesas_usb3.c | 125 ++++++++++++++++++++++++++++------
 1 file changed, 103 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
index 7dc2485..359a92b 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -351,6 +351,10 @@ struct renesas_usb3 {
 	int disabled_count;
 
 	struct usb_request *ep0_req;
+
+	struct usb_role_switch *typec_role_sw;
+	enum usb_role connection_state;
+
 	u16 test_mode;
 	u8 ep0_buf[USB3_EP0_BUF_SIZE];
 	bool softconnect;
@@ -359,6 +363,7 @@ struct renesas_usb3 {
 	bool extcon_usb;		/* check vbus and set EXTCON_USB */
 	bool forced_b_device;
 	bool start_to_connect;
+	bool usb_role_switch_property;
 };
 
 #define gadget_to_renesas_usb3(_gadget)	\
@@ -644,22 +649,6 @@ static void usb3_disconnect(struct renesas_usb3 *usb3)
 		usb3->driver->disconnect(&usb3->gadget);
 }
 
-static void usb3_check_vbus(struct renesas_usb3 *usb3)
-{
-	if (usb3->workaround_for_vbus) {
-		usb3_connect(usb3);
-	} else {
-		usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
-							USB_STA_VBUS_STA);
-		if (usb3->extcon_usb)
-			usb3_connect(usb3);
-		else
-			usb3_disconnect(usb3);
-
-		schedule_work(&usb3->extcon_work);
-	}
-}
-
 static void renesas_usb3_role_work(struct work_struct *work)
 {
 	struct renesas_usb3 *usb3 =
@@ -699,8 +688,11 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, bool host, bool a_dev)
 	unsigned long flags;
 
 	spin_lock_irqsave(&usb3->lock, flags);
-	usb3_set_mode_by_role_sw(usb3, host);
-	usb3_vbus_out(usb3, a_dev);
+	if ((!usb3->usb_role_switch_property) ||
+				(usb3->connection_state != USB_ROLE_NONE)) {
+		usb3_set_mode_by_role_sw(usb3, host);
+		usb3_vbus_out(usb3, a_dev);
+	}
 	/* for A-Peripheral or forced B-device mode */
 	if ((!host && a_dev) || usb3->start_to_connect)
 		usb3_connect(usb3);
@@ -724,6 +716,28 @@ static void usb3_check_id(struct renesas_usb3 *usb3)
 	schedule_work(&usb3->extcon_work);
 }
 
+static void usb3_check_vbus(struct renesas_usb3 *usb3)
+{
+	if (usb3->workaround_for_vbus) {
+		if (usb3->usb_role_switch_property) {
+			if (usb3->connection_state == USB_ROLE_DEVICE) {
+				usb3_mode_config(usb3, false, false);
+				usb3_connect(usb3);
+			}
+		} else
+			usb3_connect(usb3);
+	} else {
+		usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
+							USB_STA_VBUS_STA);
+		if (usb3->extcon_usb)
+			usb3_connect(usb3);
+		else
+			usb3_disconnect(usb3);
+
+		schedule_work(&usb3->extcon_work);
+	}
+}
+
 static void renesas_usb3_init_controller(struct renesas_usb3 *usb3)
 {
 	usb3_init_axi_bridge(usb3);
@@ -2343,14 +2357,62 @@ static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
 	return cur_role;
 }
 
-static int renesas_usb3_role_switch_set(struct device *dev,
-					enum usb_role role)
+static void handle_ext_role_switch_states(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);
+
+	switch (role) {
+	case USB_ROLE_NONE:
+		usb3->connection_state = USB_ROLE_NONE;
+		if (usb3->driver)
+			usb3_disconnect(usb3);
+		usb3_vbus_out(usb3, false);
+		break;
+	case USB_ROLE_DEVICE:
+		if (usb3->connection_state == USB_ROLE_NONE) {
+			usb3->connection_state = USB_ROLE_DEVICE;
+			usb3_set_mode(usb3, false);
+			if (usb3->driver)
+				usb3_connect(usb3);
+		} else if (cur_role == USB_ROLE_HOST)  {
+			device_release_driver(host);
+			usb3_set_mode(usb3, false);
+			if (usb3->driver)
+				usb3_connect(usb3);
+		}
+		usb3_vbus_out(usb3, false);
+		break;
+	case USB_ROLE_HOST:
+		if (usb3->connection_state == USB_ROLE_NONE) {
+			if (usb3->driver)
+				usb3_disconnect(usb3);
+			usb3_set_mode(usb3, true);
+			usb3_vbus_out(usb3, true);
+			usb3->connection_state = USB_ROLE_HOST;
+		} else if (cur_role == USB_ROLE_DEVICE) {
+			usb3_disconnect(usb3);
+			/* 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(host) failed\n");
+		}
+		break;
+	default:
+		break;
+	}
+}
+
+static void handle_role_switch_states(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);
@@ -2361,6 +2423,20 @@ static int renesas_usb3_role_switch_set(struct device *dev,
 		if (device_attach(host) < 0)
 			dev_err(dev, "device_attach(host) failed\n");
 	}
+}
+
+static int renesas_usb3_role_switch_set(struct device *dev,
+					enum usb_role role)
+{
+	struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+
+	pm_runtime_get_sync(dev);
+
+	if (usb3->usb_role_switch_property)
+		handle_ext_role_switch_states(dev, role);
+	else
+		handle_role_switch_states(dev, role);
+
 	pm_runtime_put(dev);
 
 	return 0;
@@ -2650,7 +2726,7 @@ static const unsigned int renesas_usb3_cable[] = {
 	EXTCON_NONE,
 };
 
-static const struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
+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,
@@ -2741,6 +2817,11 @@ static int renesas_usb3_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_dev_create;
 
+	if (device_property_read_bool(&pdev->dev, "usb-role-switch")) {
+		usb3->usb_role_switch_property = true;
+		renesas_usb3_role_switch_desc.fwnode = dev_fwnode(&pdev->dev);
+	}
+
 	INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
 	usb3->role_sw = usb_role_switch_register(&pdev->dev,
 					&renesas_usb3_role_switch_desc);
-- 
2.7.4

[v2,4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework

[Biju Das]

RZ/G2E cat874 board is capable of detecting cable connect and disconnect
events. Add support for renesas_usb3 to receive connect and disconnect
events and support dual-role switch using usb-role-switch framework.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
 V1-->V2
   * Driver uses usb role clas for handling dual role switch and handling
     connect/disconnect events instead of extcon.
---
 drivers/usb/gadget/udc/renesas_usb3.c | 125 ++++++++++++++++++++++++++++------
 1 file changed, 103 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
index 7dc2485..359a92b 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -351,6 +351,10 @@ struct renesas_usb3 {
 	int disabled_count;
 
 	struct usb_request *ep0_req;
+
+	struct usb_role_switch *typec_role_sw;
+	enum usb_role connection_state;
+
 	u16 test_mode;
 	u8 ep0_buf[USB3_EP0_BUF_SIZE];
 	bool softconnect;
@@ -359,6 +363,7 @@ struct renesas_usb3 {
 	bool extcon_usb;		/* check vbus and set EXTCON_USB */
 	bool forced_b_device;
 	bool start_to_connect;
+	bool usb_role_switch_property;
 };
 
 #define gadget_to_renesas_usb3(_gadget)	\
@@ -644,22 +649,6 @@ static void usb3_disconnect(struct renesas_usb3 *usb3)
 		usb3->driver->disconnect(&usb3->gadget);
 }
 
-static void usb3_check_vbus(struct renesas_usb3 *usb3)
-{
-	if (usb3->workaround_for_vbus) {
-		usb3_connect(usb3);
-	} else {
-		usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
-							USB_STA_VBUS_STA);
-		if (usb3->extcon_usb)
-			usb3_connect(usb3);
-		else
-			usb3_disconnect(usb3);
-
-		schedule_work(&usb3->extcon_work);
-	}
-}
-
 static void renesas_usb3_role_work(struct work_struct *work)
 {
 	struct renesas_usb3 *usb3 =
@@ -699,8 +688,11 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, bool host, bool a_dev)
 	unsigned long flags;
 
 	spin_lock_irqsave(&usb3->lock, flags);
-	usb3_set_mode_by_role_sw(usb3, host);
-	usb3_vbus_out(usb3, a_dev);
+	if ((!usb3->usb_role_switch_property) ||
+				(usb3->connection_state != USB_ROLE_NONE)) {
+		usb3_set_mode_by_role_sw(usb3, host);
+		usb3_vbus_out(usb3, a_dev);
+	}
 	/* for A-Peripheral or forced B-device mode */
 	if ((!host && a_dev) || usb3->start_to_connect)
 		usb3_connect(usb3);
@@ -724,6 +716,28 @@ static void usb3_check_id(struct renesas_usb3 *usb3)
 	schedule_work(&usb3->extcon_work);
 }
 
+static void usb3_check_vbus(struct renesas_usb3 *usb3)
+{
+	if (usb3->workaround_for_vbus) {
+		if (usb3->usb_role_switch_property) {
+			if (usb3->connection_state == USB_ROLE_DEVICE) {
+				usb3_mode_config(usb3, false, false);
+				usb3_connect(usb3);
+			}
+		} else
+			usb3_connect(usb3);
+	} else {
+		usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
+							USB_STA_VBUS_STA);
+		if (usb3->extcon_usb)
+			usb3_connect(usb3);
+		else
+			usb3_disconnect(usb3);
+
+		schedule_work(&usb3->extcon_work);
+	}
+}
+
 static void renesas_usb3_init_controller(struct renesas_usb3 *usb3)
 {
 	usb3_init_axi_bridge(usb3);
@@ -2343,14 +2357,62 @@ static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
 	return cur_role;
 }
 
-static int renesas_usb3_role_switch_set(struct device *dev,
-					enum usb_role role)
+static void handle_ext_role_switch_states(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);
+
+	switch (role) {
+	case USB_ROLE_NONE:
+		usb3->connection_state = USB_ROLE_NONE;
+		if (usb3->driver)
+			usb3_disconnect(usb3);
+		usb3_vbus_out(usb3, false);
+		break;
+	case USB_ROLE_DEVICE:
+		if (usb3->connection_state == USB_ROLE_NONE) {
+			usb3->connection_state = USB_ROLE_DEVICE;
+			usb3_set_mode(usb3, false);
+			if (usb3->driver)
+				usb3_connect(usb3);
+		} else if (cur_role == USB_ROLE_HOST)  {
+			device_release_driver(host);
+			usb3_set_mode(usb3, false);
+			if (usb3->driver)
+				usb3_connect(usb3);
+		}
+		usb3_vbus_out(usb3, false);
+		break;
+	case USB_ROLE_HOST:
+		if (usb3->connection_state == USB_ROLE_NONE) {
+			if (usb3->driver)
+				usb3_disconnect(usb3);
+			usb3_set_mode(usb3, true);
+			usb3_vbus_out(usb3, true);
+			usb3->connection_state = USB_ROLE_HOST;
+		} else if (cur_role == USB_ROLE_DEVICE) {
+			usb3_disconnect(usb3);
+			/* 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(host) failed\n");
+		}
+		break;
+	default:
+		break;
+	}
+}
+
+static void handle_role_switch_states(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);
@@ -2361,6 +2423,20 @@ static int renesas_usb3_role_switch_set(struct device *dev,
 		if (device_attach(host) < 0)
 			dev_err(dev, "device_attach(host) failed\n");
 	}
+}
+
+static int renesas_usb3_role_switch_set(struct device *dev,
+					enum usb_role role)
+{
+	struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+
+	pm_runtime_get_sync(dev);
+
+	if (usb3->usb_role_switch_property)
+		handle_ext_role_switch_states(dev, role);
+	else
+		handle_role_switch_states(dev, role);
+
 	pm_runtime_put(dev);
 
 	return 0;
@@ -2650,7 +2726,7 @@ static const unsigned int renesas_usb3_cable[] = {
 	EXTCON_NONE,
 };
 
-static const struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
+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,
@@ -2741,6 +2817,11 @@ static int renesas_usb3_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_dev_create;
 
+	if (device_property_read_bool(&pdev->dev, "usb-role-switch")) {
+		usb3->usb_role_switch_property = true;
+		renesas_usb3_role_switch_desc.fwnode = dev_fwnode(&pdev->dev);
+	}
+
 	INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
 	usb3->role_sw = usb_role_switch_register(&pdev->dev,
 					&renesas_usb3_role_switch_desc);

[PATCH v2 5/7] arm64: defconfig: enable TYPEC_HD3SS3220 config option

[Biju Das]

Enable support for the TI HD3SS320 USB Type-C DRP Port controller driver
by turning on CONFIG_TYPEC and CONFIG_TYPEC_HD3SS3220 as modules.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
V1-->V2
  * No change
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index c5ec86c..d72e0bc 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -565,6 +565,8 @@ CONFIG_USB_ULPI=y
 CONFIG_USB_GADGET=y
 CONFIG_USB_RENESAS_USBHS_UDC=m
 CONFIG_USB_RENESAS_USB3=m
+CONFIG_TYPEC=m
+CONFIG_TYPEC_HD3SS3220=m
 CONFIG_MMC=y
 CONFIG_MMC_BLOCK_MINORS=32
 CONFIG_MMC_ARMMMCI=y
-- 
2.7.4

[v2,5/7] arm64: defconfig: enable TYPEC_HD3SS3220 config option

[Biju Das]

Enable support for the TI HD3SS320 USB Type-C DRP Port controller driver
by turning on CONFIG_TYPEC and CONFIG_TYPEC_HD3SS3220 as modules.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
V1-->V2
  * No change
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index c5ec86c..d72e0bc 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -565,6 +565,8 @@ CONFIG_USB_ULPI=y
 CONFIG_USB_GADGET=y
 CONFIG_USB_RENESAS_USBHS_UDC=m
 CONFIG_USB_RENESAS_USB3=m
+CONFIG_TYPEC=m
+CONFIG_TYPEC_HD3SS3220=m
 CONFIG_MMC=y
 CONFIG_MMC_BLOCK_MINORS=32
 CONFIG_MMC_ARMMMCI=y

[PATCH v2 6/7] arm64: dts: renesas: r8a774c0-cat874: Enable USB3.0 host/peripheral device node

[Biju Das]

This patch enables USB3.0 host/peripheral device node for r8a774c0
cat874 board.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
V1-->V2
  * No change

This patch depends on commit 35ed6229c0f0d079f2
("usb: gadget: udc: renesas_usb3: Add bindings for r8a774c0")
---
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
index 42c66f9..d9d55c8 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
@@ -124,6 +124,11 @@
 		function = "sdhi0";
 		power-source = <1800>;
 	};
+
+	usb30_pins: usb30 {
+		groups = "usb30", "usb30_id";
+		function = "usb30";
+	};
 };
 
 &rwdt {
@@ -151,3 +156,15 @@
 	sd-uhs-sdr104;
 	status = "okay";
 };
+
+&usb3_peri0 {
+	companion = <&xhci0>;
+	status = "okay";
+};
+
+&xhci0 {
+	pinctrl-0 = <&usb30_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
-- 
2.7.4

[PATCH v2 7/7] arm64: dts: renesas: r8a774c0-cat874: Enable usb role switch support

[Biju Das]

This patch enables TI HD3SS3220 device and support usb role switch
for the CAT 874 platform.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
V1-->V2
  * New patch
---
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 39 +++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
index d9d55c8..ea74d21 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
@@ -80,6 +80,34 @@
 	clock-frequency = <48000000>;
 };
 
+&i2c0 {
+	status = "okay";
+	clock-frequency = <100000>;
+
+	hd3ss3220@47 {
+		compatible = "ti,hd3ss3220";
+		reg = <0x47>;
+		interrupt-parent = <&gpio6>;
+		interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+
+		usb_con: connector {
+			compatible = "usb-c-connector";
+			label = "USB-C";
+			data-role = "dual";
+		};
+
+		port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			hd3ss3220_ep: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&usb3peri_role_switch>;
+			};
+		};
+	};
+};
+
 &i2c1 {
 	pinctrl-0 = <&i2c1_pins>;
 	pinctrl-names = "default";
@@ -160,6 +188,17 @@
 &usb3_peri0 {
 	companion = <&xhci0>;
 	status = "okay";
+	usb-role-switch;
+
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		usb3peri_role_switch: endpoint@0 {
+			reg = <0>;
+			remote-endpoint = <&hd3ss3220_ep>;
+		};
+	};
 };
 
 &xhci0 {
-- 
2.7.4

RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Yoshihiro Shimoda]

Hi Biju-san,

> From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> 
> Add support for renesas_usb3 to support dual role switch
> using usb role switch class framework.
> 
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
>  V1-->V2
>   * Added usb-role-switch-property
>   * Updated the example with usb-role-switch property.
> ---
>  .../devicetree/bindings/usb/renesas_usb3.txt       | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> index 35039e7..eecaf4c 100644
> --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> @@ -22,6 +22,7 @@ Required properties:
>  Optional properties:
>    - phys: phandle + phy specifier pair
>    - phy-names: must be "usb"
> +  - usb-role-switch: use USB role switch to support dual-role switch

I don't think we can add such a property. At least, we have to add "renesas," prefix.

Best regards,
Yoshihiro Shimoda

>  Example of R-Car H3 ES1.x:
>  	usb3_peri0: usb@ee020000 {
> @@ -39,3 +40,24 @@ Example of R-Car H3 ES1.x:
>  		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&cpg CPG_MOD 327>;
>  	};
> +
> +Example of RZ/G2E:
> +	usb3_peri0: usb@ee020000 {
> +		compatible = "renesas,r8a774c0-usb3-peri",
> +			     "renesas,rcar-gen3-usb3-peri";
> +		reg = <0 0xee020000 0 0x400>;
> +		interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&cpg CPG_MOD 328>;
> +		companion = <&xhci0>;
> +		usb-role-switch;
> +
> +		port {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			usb3peri_role_switch: endpoint@0 {
> +				reg = <0>;
> +				remote-endpoint = <&hd3ss3220_ep>;
> +			};
> +		};
> +	};
> --
> 2.7.4

RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Yoshihiro Shimoda]

Hi Biju-san,

> From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> 
> Add support for renesas_usb3 to support dual role switch
> using usb role switch class framework.
> 
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
>  V1-->V2
>   * Added usb-role-switch-property
>   * Updated the example with usb-role-switch property.
> ---
>  .../devicetree/bindings/usb/renesas_usb3.txt       | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> index 35039e7..eecaf4c 100644
> --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> @@ -22,6 +22,7 @@ Required properties:
>  Optional properties:
>    - phys: phandle + phy specifier pair
>    - phy-names: must be "usb"
> +  - usb-role-switch: use USB role switch to support dual-role switch

I don't think we can add such a property. At least, we have to add "renesas," prefix.

Best regards,
Yoshihiro Shimoda

>  Example of R-Car H3 ES1.x:
>  	usb3_peri0: usb@ee020000 {
> @@ -39,3 +40,24 @@ Example of R-Car H3 ES1.x:
>  		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&cpg CPG_MOD 327>;
>  	};
> +
> +Example of RZ/G2E:
> +	usb3_peri0: usb@ee020000 {
> +		compatible = "renesas,r8a774c0-usb3-peri",
> +			     "renesas,rcar-gen3-usb3-peri";
> +		reg = <0 0xee020000 0 0x400>;
> +		interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&cpg CPG_MOD 328>;
> +		companion = <&xhci0>;
> +		usb-role-switch;
> +
> +		port {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			usb3peri_role_switch: endpoint@0 {
> +				reg = <0>;
> +				remote-endpoint = <&hd3ss3220_ep>;
> +			};
> +		};
> +	};
> --
> 2.7.4

[v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Yoshihiro Shimoda]

Hi Biju-san,

> From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> 
> Add support for renesas_usb3 to support dual role switch
> using usb role switch class framework.
> 
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
>  V1-->V2
>   * Added usb-role-switch-property
>   * Updated the example with usb-role-switch property.
> ---
>  .../devicetree/bindings/usb/renesas_usb3.txt       | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> index 35039e7..eecaf4c 100644
> --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> @@ -22,6 +22,7 @@ Required properties:
>  Optional properties:
>    - phys: phandle + phy specifier pair
>    - phy-names: must be "usb"
> +  - usb-role-switch: use USB role switch to support dual-role switch

I don't think we can add such a property. At least, we have to add "renesas," prefix.

Best regards,
Yoshihiro Shimoda

>  Example of R-Car H3 ES1.x:
>  	usb3_peri0: usb@ee020000 {
> @@ -39,3 +40,24 @@ Example of R-Car H3 ES1.x:
>  		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
>  		clocks = <&cpg CPG_MOD 327>;
>  	};
> +
> +Example of RZ/G2E:
> +	usb3_peri0: usb@ee020000 {
> +		compatible = "renesas,r8a774c0-usb3-peri",
> +			     "renesas,rcar-gen3-usb3-peri";
> +		reg = <0 0xee020000 0 0x400>;
> +		interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&cpg CPG_MOD 328>;
> +		companion = <&xhci0>;
> +		usb-role-switch;
> +
> +		port {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			usb3peri_role_switch: endpoint@0 {
> +				reg = <0>;
> +				remote-endpoint = <&hd3ss3220_ep>;
> +			};
> +		};
> +	};
> --
> 2.7.4

RE: [PATCH v2 4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework

[Yoshihiro Shimoda]

Hi Biju-san,

> From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> 
> RZ/G2E cat874 board is capable of detecting cable connect and disconnect
> events. Add support for renesas_usb3 to receive connect and disconnect
> events and support dual-role switch using usb-role-switch framework.
> 
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
>  V1-->V2
>    * Driver uses usb role clas for handling dual role switch and handling
>      connect/disconnect events instead of extcon.
> ---

Thank you for the patch!

>  drivers/usb/gadget/udc/renesas_usb3.c | 125 ++++++++++++++++++++++++++++------
>  1 file changed, 103 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
> index 7dc2485..359a92b 100644
> --- a/drivers/usb/gadget/udc/renesas_usb3.c
> +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> @@ -351,6 +351,10 @@ struct renesas_usb3 {
>  	int disabled_count;
> 
>  	struct usb_request *ep0_req;
> +
> +	struct usb_role_switch *typec_role_sw;

This typec_role_sw is not used.

> +	enum usb_role connection_state;
> +
>  	u16 test_mode;
>  	u8 ep0_buf[USB3_EP0_BUF_SIZE];
>  	bool softconnect;
> @@ -359,6 +363,7 @@ struct renesas_usb3 {
>  	bool extcon_usb;		/* check vbus and set EXTCON_USB */
>  	bool forced_b_device;
>  	bool start_to_connect;
> +	bool usb_role_switch_property;
>  };
> 
>  #define gadget_to_renesas_usb3(_gadget)	\
> @@ -644,22 +649,6 @@ static void usb3_disconnect(struct renesas_usb3 *usb3)
>  		usb3->driver->disconnect(&usb3->gadget);
>  }
> 
> -static void usb3_check_vbus(struct renesas_usb3 *usb3)
> -{
> -	if (usb3->workaround_for_vbus) {
> -		usb3_connect(usb3);
> -	} else {
> -		usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
> -							USB_STA_VBUS_STA);
> -		if (usb3->extcon_usb)
> -			usb3_connect(usb3);
> -		else
> -			usb3_disconnect(usb3);
> -
> -		schedule_work(&usb3->extcon_work);
> -	}
> -}
> -
>  static void renesas_usb3_role_work(struct work_struct *work)
>  {
>  	struct renesas_usb3 *usb3 =
> @@ -699,8 +688,11 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, bool host, bool a_dev)
>  	unsigned long flags;
> 
>  	spin_lock_irqsave(&usb3->lock, flags);
> -	usb3_set_mode_by_role_sw(usb3, host);
> -	usb3_vbus_out(usb3, a_dev);
> +	if ((!usb3->usb_role_switch_property) ||
> +				(usb3->connection_state != USB_ROLE_NONE)) {

We can modify the code as following:
	if (!usb3->usb_role_switch_property ||
	    usb3->connection_state != USB_ROLE_NONE) {

<snip>
> @@ -2650,7 +2726,7 @@ static const unsigned int renesas_usb3_cable[] = {
>  	EXTCON_NONE,
>  };
> 
> -static const struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
> +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,
> @@ -2741,6 +2817,11 @@ static int renesas_usb3_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto err_dev_create;
> 
> +	if (device_property_read_bool(&pdev->dev, "usb-role-switch")) {
> +		usb3->usb_role_switch_property = true;
> +		renesas_usb3_role_switch_desc.fwnode = dev_fwnode(&pdev->dev);

This causes build error because struct usb_role_switch_desc doesn't have fwnode.

Best regards,
Yoshihiro Shimoda

[v2,4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework

[Yoshihiro Shimoda]

Hi Biju-san,

> From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> 
> RZ/G2E cat874 board is capable of detecting cable connect and disconnect
> events. Add support for renesas_usb3 to receive connect and disconnect
> events and support dual-role switch using usb-role-switch framework.
> 
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
>  V1-->V2
>    * Driver uses usb role clas for handling dual role switch and handling
>      connect/disconnect events instead of extcon.
> ---

Thank you for the patch!

>  drivers/usb/gadget/udc/renesas_usb3.c | 125 ++++++++++++++++++++++++++++------
>  1 file changed, 103 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
> index 7dc2485..359a92b 100644
> --- a/drivers/usb/gadget/udc/renesas_usb3.c
> +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> @@ -351,6 +351,10 @@ struct renesas_usb3 {
>  	int disabled_count;
> 
>  	struct usb_request *ep0_req;
> +
> +	struct usb_role_switch *typec_role_sw;

This typec_role_sw is not used.

> +	enum usb_role connection_state;
> +
>  	u16 test_mode;
>  	u8 ep0_buf[USB3_EP0_BUF_SIZE];
>  	bool softconnect;
> @@ -359,6 +363,7 @@ struct renesas_usb3 {
>  	bool extcon_usb;		/* check vbus and set EXTCON_USB */
>  	bool forced_b_device;
>  	bool start_to_connect;
> +	bool usb_role_switch_property;
>  };
> 
>  #define gadget_to_renesas_usb3(_gadget)	\
> @@ -644,22 +649,6 @@ static void usb3_disconnect(struct renesas_usb3 *usb3)
>  		usb3->driver->disconnect(&usb3->gadget);
>  }
> 
> -static void usb3_check_vbus(struct renesas_usb3 *usb3)
> -{
> -	if (usb3->workaround_for_vbus) {
> -		usb3_connect(usb3);
> -	} else {
> -		usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
> -							USB_STA_VBUS_STA);
> -		if (usb3->extcon_usb)
> -			usb3_connect(usb3);
> -		else
> -			usb3_disconnect(usb3);
> -
> -		schedule_work(&usb3->extcon_work);
> -	}
> -}
> -
>  static void renesas_usb3_role_work(struct work_struct *work)
>  {
>  	struct renesas_usb3 *usb3 =
> @@ -699,8 +688,11 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, bool host, bool a_dev)
>  	unsigned long flags;
> 
>  	spin_lock_irqsave(&usb3->lock, flags);
> -	usb3_set_mode_by_role_sw(usb3, host);
> -	usb3_vbus_out(usb3, a_dev);
> +	if ((!usb3->usb_role_switch_property) ||
> +				(usb3->connection_state != USB_ROLE_NONE)) {

We can modify the code as following:
	if (!usb3->usb_role_switch_property ||
	    usb3->connection_state != USB_ROLE_NONE) {

<snip>
> @@ -2650,7 +2726,7 @@ static const unsigned int renesas_usb3_cable[] = {
>  	EXTCON_NONE,
>  };
> 
> -static const struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
> +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,
> @@ -2741,6 +2817,11 @@ static int renesas_usb3_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto err_dev_create;
> 
> +	if (device_property_read_bool(&pdev->dev, "usb-role-switch")) {
> +		usb3->usb_role_switch_property = true;
> +		renesas_usb3_role_switch_desc.fwnode = dev_fwnode(&pdev->dev);

This causes build error because struct usb_role_switch_desc doesn't have fwnode.

Best regards,
Yoshihiro Shimoda

RE: [PATCH v2 4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework

[Biju Das]

Hi Shimoda-San,

Thanks for the feedback.

> Subject: RE: [PATCH v2 4/7] usb: gadget: udc: renesas_usb3: Use
> usb_role_switch framework
> 
> Hi Biju-san,
> 
> > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> >
> > RZ/G2E cat874 board is capable of detecting cable connect and
> > disconnect events. Add support for renesas_usb3 to receive connect and
> > disconnect events and support dual-role switch using usb-role-switch
> framework.
> >
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > ---
> >  V1-->V2
> >    * Driver uses usb role clas for handling dual role switch and handling
> >      connect/disconnect events instead of extcon.
> > ---
> 
> Thank you for the patch!
> 
> >  drivers/usb/gadget/udc/renesas_usb3.c | 125
> > ++++++++++++++++++++++++++++------
> >  1 file changed, 103 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/udc/renesas_usb3.c
> > b/drivers/usb/gadget/udc/renesas_usb3.c
> > index 7dc2485..359a92b 100644
> > --- a/drivers/usb/gadget/udc/renesas_usb3.c
> > +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> > @@ -351,6 +351,10 @@ struct renesas_usb3 {
> >  	int disabled_count;
> >
> >  	struct usb_request *ep0_req;
> > +
> > +	struct usb_role_switch *typec_role_sw;
> 
> This typec_role_sw is not used.

 OK. Will remove this.
> 
> > +	enum usb_role connection_state;
> > +
> >  	u16 test_mode;
> >  	u8 ep0_buf[USB3_EP0_BUF_SIZE];
> >  	bool softconnect;
> > @@ -359,6 +363,7 @@ struct renesas_usb3 {
> >  	bool extcon_usb;		/* check vbus and set EXTCON_USB
> */
> >  	bool forced_b_device;
> >  	bool start_to_connect;
> > +	bool usb_role_switch_property;
> >  };
> >
> >  #define gadget_to_renesas_usb3(_gadget)	\
> > @@ -644,22 +649,6 @@ static void usb3_disconnect(struct renesas_usb3
> *usb3)
> >  		usb3->driver->disconnect(&usb3->gadget);
> >  }
> >
> > -static void usb3_check_vbus(struct renesas_usb3 *usb3) -{
> > -	if (usb3->workaround_for_vbus) {
> > -		usb3_connect(usb3);
> > -	} else {
> > -		usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
> > -
> 	USB_STA_VBUS_STA);
> > -		if (usb3->extcon_usb)
> > -			usb3_connect(usb3);
> > -		else
> > -			usb3_disconnect(usb3);
> > -
> > -		schedule_work(&usb3->extcon_work);
> > -	}
> > -}
> > -
> >  static void renesas_usb3_role_work(struct work_struct *work)  {
> >  	struct renesas_usb3 *usb3 =
> > @@ -699,8 +688,11 @@ static void usb3_mode_config(struct renesas_usb3
> *usb3, bool host, bool a_dev)
> >  	unsigned long flags;
> >
> >  	spin_lock_irqsave(&usb3->lock, flags);
> > -	usb3_set_mode_by_role_sw(usb3, host);
> > -	usb3_vbus_out(usb3, a_dev);
> > +	if ((!usb3->usb_role_switch_property) ||
> > +				(usb3->connection_state !=
> USB_ROLE_NONE)) {
> 
> We can modify the code as following:
> 	if (!usb3->usb_role_switch_property ||
> 	    usb3->connection_state != USB_ROLE_NONE) {

OK. Will change this.

> <snip>
> > @@ -2650,7 +2726,7 @@ static const unsigned int renesas_usb3_cable[] = {
> >  	EXTCON_NONE,
> >  };
> >
> > -static const struct usb_role_switch_desc
> > renesas_usb3_role_switch_desc = {
> > +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,
> > @@ -2741,6 +2817,11 @@ static int renesas_usb3_probe(struct
> platform_device *pdev)
> >  	if (ret < 0)
> >  		goto err_dev_create;
> >
> > +	if (device_property_read_bool(&pdev->dev, "usb-role-switch")) {
> > +		usb3->usb_role_switch_property = true;
> > +		renesas_usb3_role_switch_desc.fwnode =
> dev_fwnode(&pdev->dev);
> 
> This causes build error because struct usb_role_switch_desc doesn't have
> fwnode.

I have compiled this code with linux-next and the commit 
09aa11cfda9d8186046b ("device connection: Add fwnode member to struct device_connection")
defines the property fwnode. The HD3SS3220 port controller driver uses usb-role-switch property to
get role-switch handle using graph api's.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20190306&id=09aa11cfda9d8186046bcd1adcd6498b688114f4

regards,
Biju

[v2,4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework

[Biju Das]

Hi Shimoda-San,

Thanks for the feedback.

> Subject: RE: [PATCH v2 4/7] usb: gadget: udc: renesas_usb3: Use
> usb_role_switch framework
> 
> Hi Biju-san,
> 
> > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> >
> > RZ/G2E cat874 board is capable of detecting cable connect and
> > disconnect events. Add support for renesas_usb3 to receive connect and
> > disconnect events and support dual-role switch using usb-role-switch
> framework.
> >
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > ---
> >  V1-->V2
> >    * Driver uses usb role clas for handling dual role switch and handling
> >      connect/disconnect events instead of extcon.
> > ---
> 
> Thank you for the patch!
> 
> >  drivers/usb/gadget/udc/renesas_usb3.c | 125
> > ++++++++++++++++++++++++++++------
> >  1 file changed, 103 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/udc/renesas_usb3.c
> > b/drivers/usb/gadget/udc/renesas_usb3.c
> > index 7dc2485..359a92b 100644
> > --- a/drivers/usb/gadget/udc/renesas_usb3.c
> > +++ b/drivers/usb/gadget/udc/renesas_usb3.c
> > @@ -351,6 +351,10 @@ struct renesas_usb3 {
> >  	int disabled_count;
> >
> >  	struct usb_request *ep0_req;
> > +
> > +	struct usb_role_switch *typec_role_sw;
> 
> This typec_role_sw is not used.

 OK. Will remove this.
> 
> > +	enum usb_role connection_state;
> > +
> >  	u16 test_mode;
> >  	u8 ep0_buf[USB3_EP0_BUF_SIZE];
> >  	bool softconnect;
> > @@ -359,6 +363,7 @@ struct renesas_usb3 {
> >  	bool extcon_usb;		/* check vbus and set EXTCON_USB
> */
> >  	bool forced_b_device;
> >  	bool start_to_connect;
> > +	bool usb_role_switch_property;
> >  };
> >
> >  #define gadget_to_renesas_usb3(_gadget)	\
> > @@ -644,22 +649,6 @@ static void usb3_disconnect(struct renesas_usb3
> *usb3)
> >  		usb3->driver->disconnect(&usb3->gadget);
> >  }
> >
> > -static void usb3_check_vbus(struct renesas_usb3 *usb3) -{
> > -	if (usb3->workaround_for_vbus) {
> > -		usb3_connect(usb3);
> > -	} else {
> > -		usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
> > -
> 	USB_STA_VBUS_STA);
> > -		if (usb3->extcon_usb)
> > -			usb3_connect(usb3);
> > -		else
> > -			usb3_disconnect(usb3);
> > -
> > -		schedule_work(&usb3->extcon_work);
> > -	}
> > -}
> > -
> >  static void renesas_usb3_role_work(struct work_struct *work)  {
> >  	struct renesas_usb3 *usb3 =
> > @@ -699,8 +688,11 @@ static void usb3_mode_config(struct renesas_usb3
> *usb3, bool host, bool a_dev)
> >  	unsigned long flags;
> >
> >  	spin_lock_irqsave(&usb3->lock, flags);
> > -	usb3_set_mode_by_role_sw(usb3, host);
> > -	usb3_vbus_out(usb3, a_dev);
> > +	if ((!usb3->usb_role_switch_property) ||
> > +				(usb3->connection_state !=
> USB_ROLE_NONE)) {
> 
> We can modify the code as following:
> 	if (!usb3->usb_role_switch_property ||
> 	    usb3->connection_state != USB_ROLE_NONE) {

OK. Will change this.

> <snip>
> > @@ -2650,7 +2726,7 @@ static const unsigned int renesas_usb3_cable[] = {
> >  	EXTCON_NONE,
> >  };
> >
> > -static const struct usb_role_switch_desc
> > renesas_usb3_role_switch_desc = {
> > +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,
> > @@ -2741,6 +2817,11 @@ static int renesas_usb3_probe(struct
> platform_device *pdev)
> >  	if (ret < 0)
> >  		goto err_dev_create;
> >
> > +	if (device_property_read_bool(&pdev->dev, "usb-role-switch")) {
> > +		usb3->usb_role_switch_property = true;
> > +		renesas_usb3_role_switch_desc.fwnode =
> dev_fwnode(&pdev->dev);
> 
> This causes build error because struct usb_role_switch_desc doesn't have
> fwnode.

I have compiled this code with linux-next and the commit 
09aa11cfda9d8186046b ("device connection: Add fwnode member to struct device_connection")
defines the property fwnode. The HD3SS3220 port controller driver uses usb-role-switch property to
get role-switch handle using graph api's.

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20190306&id=09aa11cfda9d8186046bcd1adcd6498b688114f4

regards,
Biju

RE: [PATCH v2 4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework

[Biju Das]

Hi Shimoda-San,

Sorry. I pasted a wrong link.

> > This causes build error because struct usb_role_switch_desc doesn't
> > have fwnode.
> 
> I have compiled this code with linux-next and the commit
> 09aa11cfda9d8186046b ("device connection: Add fwnode member to struct
> device_connection") defines the property fwnode. The HD3SS3220 port
> controller driver uses usb-role-switch property to get role-switch handle
> using graph api's.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-
> next.git/commit/?h=next-
> 20190306&id=09aa11cfda9d8186046bcd1adcd6498b688114f4

Commit id ec69e9533c4879c81eb7122771792864eb49af35 ("usb: roles: Find the muxes by also matching against the device node")
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20190306&id=ec69e9533c4879c81eb7122771792864eb49af35

regards,
Biju

[v2,4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework

[Biju Das]

Hi Shimoda-San,

Sorry. I pasted a wrong link.

> > This causes build error because struct usb_role_switch_desc doesn't
> > have fwnode.
> 
> I have compiled this code with linux-next and the commit
> 09aa11cfda9d8186046b ("device connection: Add fwnode member to struct
> device_connection") defines the property fwnode. The HD3SS3220 port
> controller driver uses usb-role-switch property to get role-switch handle
> using graph api's.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-
> next.git/commit/?h=next-
> 20190306&id=09aa11cfda9d8186046bcd1adcd6498b688114f4

Commit id ec69e9533c4879c81eb7122771792864eb49af35 ("usb: roles: Find the muxes by also matching against the device node")
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20190306&id=ec69e9533c4879c81eb7122771792864eb49af35

regards,
Biju

RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Biju Das]

Hi Shimoda-San,

Thanks for the feedback.

> Subject: RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> switch property
> 
> Hi Biju-san,
> 
> > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> >
> > Add support for renesas_usb3 to support dual role switch using usb
> > role switch class framework.
> >
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > ---
> >  V1-->V2
> >   * Added usb-role-switch-property
> >   * Updated the example with usb-role-switch property.
> > ---
> >  .../devicetree/bindings/usb/renesas_usb3.txt       | 22
> ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > index 35039e7..eecaf4c 100644
> > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > @@ -22,6 +22,7 @@ Required properties:
> >  Optional properties:
> >    - phys: phandle + phy specifier pair
> >    - phy-names: must be "usb"
> > +  - usb-role-switch: use USB role switch to support dual-role switch
> 
> I don't think we can add such a property. At least, we have to add "renesas,"
> prefix.

usb_role_switch_get   api uses  "usb-role-switch"  property to get role switch linked with the device.

HD3SS3220 port controller driver gets role switch handle linked with the device using usb_role_switch_get  api.  
That is the reason, I have added " usb-role-switch" property here.

Do you have any other suggestion to get usb role switch handle?

Please correct me if I am wrong. 
 
> >  Example of R-Car H3 ES1.x:
> >  	usb3_peri0: usb@ee020000 {
> > @@ -39,3 +40,24 @@ Example of R-Car H3 ES1.x:
> >  		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
> >  		clocks = <&cpg CPG_MOD 327>;
> >  	};
> > +
> > +Example of RZ/G2E:
> > +	usb3_peri0: usb@ee020000 {
> > +		compatible = "renesas,r8a774c0-usb3-peri",
> > +			     "renesas,rcar-gen3-usb3-peri";
> > +		reg = <0 0xee020000 0 0x400>;
> > +		interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&cpg CPG_MOD 328>;
> > +		companion = <&xhci0>;
> > +		usb-role-switch;
> > +
> > +		port {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			usb3peri_role_switch: endpoint@0 {
> > +				reg = <0>;
> > +				remote-endpoint = <&hd3ss3220_ep>;
> > +			};
> > +		};
> > +	};
> > --
> > 2.7.4

Regards,
Biju

RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Biju Das]

Hi Shimoda-San,

Thanks for the feedback.

> Subject: RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> switch property
> 
> Hi Biju-san,
> 
> > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> >
> > Add support for renesas_usb3 to support dual role switch using usb
> > role switch class framework.
> >
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > ---
> >  V1-->V2
> >   * Added usb-role-switch-property
> >   * Updated the example with usb-role-switch property.
> > ---
> >  .../devicetree/bindings/usb/renesas_usb3.txt       | 22
> ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > index 35039e7..eecaf4c 100644
> > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > @@ -22,6 +22,7 @@ Required properties:
> >  Optional properties:
> >    - phys: phandle + phy specifier pair
> >    - phy-names: must be "usb"
> > +  - usb-role-switch: use USB role switch to support dual-role switch
> 
> I don't think we can add such a property. At least, we have to add "renesas,"
> prefix.

usb_role_switch_get   api uses  "usb-role-switch"  property to get role switch linked with the device.

HD3SS3220 port controller driver gets role switch handle linked with the device using usb_role_switch_get  api.  
That is the reason, I have added " usb-role-switch" property here.

Do you have any other suggestion to get usb role switch handle?

Please correct me if I am wrong. 
 
> >  Example of R-Car H3 ES1.x:
> >  	usb3_peri0: usb@ee020000 {
> > @@ -39,3 +40,24 @@ Example of R-Car H3 ES1.x:
> >  		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
> >  		clocks = <&cpg CPG_MOD 327>;
> >  	};
> > +
> > +Example of RZ/G2E:
> > +	usb3_peri0: usb@ee020000 {
> > +		compatible = "renesas,r8a774c0-usb3-peri",
> > +			     "renesas,rcar-gen3-usb3-peri";
> > +		reg = <0 0xee020000 0 0x400>;
> > +		interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&cpg CPG_MOD 328>;
> > +		companion = <&xhci0>;
> > +		usb-role-switch;
> > +
> > +		port {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			usb3peri_role_switch: endpoint@0 {
> > +				reg = <0>;
> > +				remote-endpoint = <&hd3ss3220_ep>;
> > +			};
> > +		};
> > +	};
> > --
> > 2.7.4

Regards,
Biju

[v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Biju Das]

Hi Shimoda-San,

Thanks for the feedback.

> Subject: RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> switch property
> 
> Hi Biju-san,
> 
> > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> >
> > Add support for renesas_usb3 to support dual role switch using usb
> > role switch class framework.
> >
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > ---
> >  V1-->V2
> >   * Added usb-role-switch-property
> >   * Updated the example with usb-role-switch property.
> > ---
> >  .../devicetree/bindings/usb/renesas_usb3.txt       | 22
> ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > index 35039e7..eecaf4c 100644
> > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > @@ -22,6 +22,7 @@ Required properties:
> >  Optional properties:
> >    - phys: phandle + phy specifier pair
> >    - phy-names: must be "usb"
> > +  - usb-role-switch: use USB role switch to support dual-role switch
> 
> I don't think we can add such a property. At least, we have to add "renesas,"
> prefix.

usb_role_switch_get   api uses  "usb-role-switch"  property to get role switch linked with the device.

HD3SS3220 port controller driver gets role switch handle linked with the device using usb_role_switch_get  api.  
That is the reason, I have added " usb-role-switch" property here.

Do you have any other suggestion to get usb role switch handle?

Please correct me if I am wrong. 
 
> >  Example of R-Car H3 ES1.x:
> >  	usb3_peri0: usb@ee020000 {
> > @@ -39,3 +40,24 @@ Example of R-Car H3 ES1.x:
> >  		interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
> >  		clocks = <&cpg CPG_MOD 327>;
> >  	};
> > +
> > +Example of RZ/G2E:
> > +	usb3_peri0: usb@ee020000 {
> > +		compatible = "renesas,r8a774c0-usb3-peri",
> > +			     "renesas,rcar-gen3-usb3-peri";
> > +		reg = <0 0xee020000 0 0x400>;
> > +		interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
> > +		clocks = <&cpg CPG_MOD 328>;
> > +		companion = <&xhci0>;
> > +		usb-role-switch;
> > +
> > +		port {
> > +			#address-cells = <1>;
> > +			#size-cells = <0>;
> > +
> > +			usb3peri_role_switch: endpoint@0 {
> > +				reg = <0>;
> > +				remote-endpoint = <&hd3ss3220_ep>;
> > +			};
> > +		};
> > +	};
> > --
> > 2.7.4

Regards,
Biju

Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Heikki Krogerus]

On Thu, Mar 14, 2019 at 10:14:12AM +0000, Biju Das wrote:
> Hi Shimoda-San,
> 
> Thanks for the feedback.
> 
> > Subject: RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > switch property
> > 
> > Hi Biju-san,
> > 
> > > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> > >
> > > Add support for renesas_usb3 to support dual role switch using usb
> > > role switch class framework.
> > >
> > > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > > ---
> > >  V1-->V2
> > >   * Added usb-role-switch-property
> > >   * Updated the example with usb-role-switch property.
> > > ---
> > >  .../devicetree/bindings/usb/renesas_usb3.txt       | 22
> > ++++++++++++++++++++++
> > >  1 file changed, 22 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > index 35039e7..eecaf4c 100644
> > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > @@ -22,6 +22,7 @@ Required properties:
> > >  Optional properties:
> > >    - phys: phandle + phy specifier pair
> > >    - phy-names: must be "usb"
> > > +  - usb-role-switch: use USB role switch to support dual-role switch
> > 
> > I don't think we can add such a property. At least, we have to add "renesas,"
> > prefix.
> 
> usb_role_switch_get   api uses  "usb-role-switch"  property to get role switch linked with the device.
> 
> HD3SS3220 port controller driver gets role switch handle linked with the device using usb_role_switch_get  api.  
> That is the reason, I have added " usb-role-switch" property here.
> 
> Do you have any other suggestion to get usb role switch handle?

We can still change the API. Can we use the compatible for this?


thanks,

-- 
heikki

Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Heikki Krogerus]

On Thu, Mar 14, 2019 at 10:14:12AM +0000, Biju Das wrote:
> Hi Shimoda-San,
> 
> Thanks for the feedback.
> 
> > Subject: RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > switch property
> > 
> > Hi Biju-san,
> > 
> > > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> > >
> > > Add support for renesas_usb3 to support dual role switch using usb
> > > role switch class framework.
> > >
> > > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > > ---
> > >  V1-->V2
> > >   * Added usb-role-switch-property
> > >   * Updated the example with usb-role-switch property.
> > > ---
> > >  .../devicetree/bindings/usb/renesas_usb3.txt       | 22
> > ++++++++++++++++++++++
> > >  1 file changed, 22 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > index 35039e7..eecaf4c 100644
> > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > @@ -22,6 +22,7 @@ Required properties:
> > >  Optional properties:
> > >    - phys: phandle + phy specifier pair
> > >    - phy-names: must be "usb"
> > > +  - usb-role-switch: use USB role switch to support dual-role switch
> > 
> > I don't think we can add such a property. At least, we have to add "renesas,"
> > prefix.
> 
> usb_role_switch_get   api uses  "usb-role-switch"  property to get role switch linked with the device.
> 
> HD3SS3220 port controller driver gets role switch handle linked with the device using usb_role_switch_get  api.  
> That is the reason, I have added " usb-role-switch" property here.
> 
> Do you have any other suggestion to get usb role switch handle?

We can still change the API. Can we use the compatible for this?


thanks,

-- 
heikki

[v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Heikki Krogerus]

On Thu, Mar 14, 2019 at 10:14:12AM +0000, Biju Das wrote:
> Hi Shimoda-San,
> 
> Thanks for the feedback.
> 
> > Subject: RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > switch property
> > 
> > Hi Biju-san,
> > 
> > > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> > >
> > > Add support for renesas_usb3 to support dual role switch using usb
> > > role switch class framework.
> > >
> > > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > > ---
> > >  V1-->V2
> > >   * Added usb-role-switch-property
> > >   * Updated the example with usb-role-switch property.
> > > ---
> > >  .../devicetree/bindings/usb/renesas_usb3.txt       | 22
> > ++++++++++++++++++++++
> > >  1 file changed, 22 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > index 35039e7..eecaf4c 100644
> > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > @@ -22,6 +22,7 @@ Required properties:
> > >  Optional properties:
> > >    - phys: phandle + phy specifier pair
> > >    - phy-names: must be "usb"
> > > +  - usb-role-switch: use USB role switch to support dual-role switch
> > 
> > I don't think we can add such a property. At least, we have to add "renesas,"
> > prefix.
> 
> usb_role_switch_get   api uses  "usb-role-switch"  property to get role switch linked with the device.
> 
> HD3SS3220 port controller driver gets role switch handle linked with the device using usb_role_switch_get  api.  
> That is the reason, I have added " usb-role-switch" property here.
> 
> Do you have any other suggestion to get usb role switch handle?

We can still change the API. Can we use the compatible for this?


thanks,

RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Biju Das]

Hi Heikki,

Thanks for the feedback.

> Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> switch property
> 
> On Thu, Mar 14, 2019 at 10:14:12AM +0000, Biju Das wrote:
> > Hi Shimoda-San,
> >
> > Thanks for the feedback.
> >
> > > Subject: RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add
> > > usb-role- switch property
> > >
> > > Hi Biju-san,
> > >
> > > > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> > > >
> > > > Add support for renesas_usb3 to support dual role switch using usb
> > > > role switch class framework.
> > > >
> > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > > > ---
> > > >  V1-->V2
> > > >   * Added usb-role-switch-property
> > > >   * Updated the example with usb-role-switch property.
> > > > ---
> > > >  .../devicetree/bindings/usb/renesas_usb3.txt       | 22
> > > ++++++++++++++++++++++
> > > >  1 file changed, 22 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > index 35039e7..eecaf4c 100644
> > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > @@ -22,6 +22,7 @@ Required properties:
> > > >  Optional properties:
> > > >    - phys: phandle + phy specifier pair
> > > >    - phy-names: must be "usb"
> > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > + switch
> > >
> > > I don't think we can add such a property. At least, we have to add
> "renesas,"
> > > prefix.
> >
> > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> switch linked with the device.
> >
> > HD3SS3220 port controller driver gets role switch handle linked with the
> device using usb_role_switch_get  api.
> > That is the reason, I have added " usb-role-switch" property here.
> >
> > Do you have any other suggestion to get usb role switch handle?
> 
> We can still change the API. Can we use the compatible for this?

Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
In that case, no need to add  generic "usb-role-switch"  property here.

Can you please confirm my understanding is correct?

Regards,
Biju

RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Biju Das]

Hi Heikki,

Thanks for the feedback.

> Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> switch property
> 
> On Thu, Mar 14, 2019 at 10:14:12AM +0000, Biju Das wrote:
> > Hi Shimoda-San,
> >
> > Thanks for the feedback.
> >
> > > Subject: RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add
> > > usb-role- switch property
> > >
> > > Hi Biju-san,
> > >
> > > > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> > > >
> > > > Add support for renesas_usb3 to support dual role switch using usb
> > > > role switch class framework.
> > > >
> > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > > > ---
> > > >  V1-->V2
> > > >   * Added usb-role-switch-property
> > > >   * Updated the example with usb-role-switch property.
> > > > ---
> > > >  .../devicetree/bindings/usb/renesas_usb3.txt       | 22
> > > ++++++++++++++++++++++
> > > >  1 file changed, 22 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > index 35039e7..eecaf4c 100644
> > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > @@ -22,6 +22,7 @@ Required properties:
> > > >  Optional properties:
> > > >    - phys: phandle + phy specifier pair
> > > >    - phy-names: must be "usb"
> > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > + switch
> > >
> > > I don't think we can add such a property. At least, we have to add
> "renesas,"
> > > prefix.
> >
> > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> switch linked with the device.
> >
> > HD3SS3220 port controller driver gets role switch handle linked with the
> device using usb_role_switch_get  api.
> > That is the reason, I have added " usb-role-switch" property here.
> >
> > Do you have any other suggestion to get usb role switch handle?
> 
> We can still change the API. Can we use the compatible for this?

Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
In that case, no need to add  generic "usb-role-switch"  property here.

Can you please confirm my understanding is correct?

Regards,
Biju

[v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Biju Das]

Hi Heikki,

Thanks for the feedback.

> Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> switch property
> 
> On Thu, Mar 14, 2019 at 10:14:12AM +0000, Biju Das wrote:
> > Hi Shimoda-San,
> >
> > Thanks for the feedback.
> >
> > > Subject: RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add
> > > usb-role- switch property
> > >
> > > Hi Biju-san,
> > >
> > > > From: Biju Das, Sent: Thursday, March 14, 2019 5:40 PM
> > > >
> > > > Add support for renesas_usb3 to support dual role switch using usb
> > > > role switch class framework.
> > > >
> > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > > > ---
> > > >  V1-->V2
> > > >   * Added usb-role-switch-property
> > > >   * Updated the example with usb-role-switch property.
> > > > ---
> > > >  .../devicetree/bindings/usb/renesas_usb3.txt       | 22
> > > ++++++++++++++++++++++
> > > >  1 file changed, 22 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > index 35039e7..eecaf4c 100644
> > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > @@ -22,6 +22,7 @@ Required properties:
> > > >  Optional properties:
> > > >    - phys: phandle + phy specifier pair
> > > >    - phy-names: must be "usb"
> > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > + switch
> > >
> > > I don't think we can add such a property. At least, we have to add
> "renesas,"
> > > prefix.
> >
> > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> switch linked with the device.
> >
> > HD3SS3220 port controller driver gets role switch handle linked with the
> device using usb_role_switch_get  api.
> > That is the reason, I have added " usb-role-switch" property here.
> >
> > Do you have any other suggestion to get usb role switch handle?
> 
> We can still change the API. Can we use the compatible for this?

Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
In that case, no need to add  generic "usb-role-switch"  property here.

Can you please confirm my understanding is correct?

Regards,
Biju

Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Heikki Krogerus]

Thanks,

On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > index 35039e7..eecaf4c 100644
> > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > >  Optional properties:
> > > > >    - phys: phandle + phy specifier pair
> > > > >    - phy-names: must be "usb"
> > > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > > + switch
> > > >
> > > > I don't think we can add such a property. At least, we have to add
> > "renesas,"
> > > > prefix.
> > >
> > > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> > switch linked with the device.
> > >
> > > HD3SS3220 port controller driver gets role switch handle linked with the
> > device using usb_role_switch_get  api.
> > > That is the reason, I have added " usb-role-switch" property here.
> > >
> > > Do you have any other suggestion to get usb role switch handle?
> > 
> > We can still change the API. Can we use the compatible for this?
> 
> Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
> Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
> In that case, no need to add  generic "usb-role-switch"  property here.
> 
> Can you please confirm my understanding is correct?

No, I meant the compatible property would have the value
"usb-role-switch". Your compatible would probable look something
like this:

        compatible = "renesas,r8a774c0-usb3-peri",
                     "usb-role-switch";

So the idea would be that the device supplying USB role switch
functionality, in your case the USB controller, would need to have the
compatible property containing "usb-role-switch".

I'm really not an expert in DT, so I don't know if that is acceptable.
Rob can you comment on this?


thanks,

-- 
heikki

Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Heikki Krogerus]

Thanks,

On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > index 35039e7..eecaf4c 100644
> > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > >  Optional properties:
> > > > >    - phys: phandle + phy specifier pair
> > > > >    - phy-names: must be "usb"
> > > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > > + switch
> > > >
> > > > I don't think we can add such a property. At least, we have to add
> > "renesas,"
> > > > prefix.
> > >
> > > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> > switch linked with the device.
> > >
> > > HD3SS3220 port controller driver gets role switch handle linked with the
> > device using usb_role_switch_get  api.
> > > That is the reason, I have added " usb-role-switch" property here.
> > >
> > > Do you have any other suggestion to get usb role switch handle?
> > 
> > We can still change the API. Can we use the compatible for this?
> 
> Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
> Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
> In that case, no need to add  generic "usb-role-switch"  property here.
> 
> Can you please confirm my understanding is correct?

No, I meant the compatible property would have the value
"usb-role-switch". Your compatible would probable look something
like this:

        compatible = "renesas,r8a774c0-usb3-peri",
                     "usb-role-switch";

So the idea would be that the device supplying USB role switch
functionality, in your case the USB controller, would need to have the
compatible property containing "usb-role-switch".

I'm really not an expert in DT, so I don't know if that is acceptable.
Rob can you comment on this?


thanks,

-- 
heikki

[v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Heikki Krogerus]

Thanks,

On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > index 35039e7..eecaf4c 100644
> > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > >  Optional properties:
> > > > >    - phys: phandle + phy specifier pair
> > > > >    - phy-names: must be "usb"
> > > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > > + switch
> > > >
> > > > I don't think we can add such a property. At least, we have to add
> > "renesas,"
> > > > prefix.
> > >
> > > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> > switch linked with the device.
> > >
> > > HD3SS3220 port controller driver gets role switch handle linked with the
> > device using usb_role_switch_get  api.
> > > That is the reason, I have added " usb-role-switch" property here.
> > >
> > > Do you have any other suggestion to get usb role switch handle?
> > 
> > We can still change the API. Can we use the compatible for this?
> 
> Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
> Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
> In that case, no need to add  generic "usb-role-switch"  property here.
> 
> Can you please confirm my understanding is correct?

No, I meant the compatible property would have the value
"usb-role-switch". Your compatible would probable look something
like this:

        compatible = "renesas,r8a774c0-usb3-peri",
                     "usb-role-switch";

So the idea would be that the device supplying USB role switch
functionality, in your case the USB controller, would need to have the
compatible property containing "usb-role-switch".

I'm really not an expert in DT, so I don't know if that is acceptable.
Rob can you comment on this?


thanks,

Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Rob Herring]

On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote:
> Thanks,
> 
> On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > index 35039e7..eecaf4c 100644
> > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > > >  Optional properties:
> > > > > >    - phys: phandle + phy specifier pair
> > > > > >    - phy-names: must be "usb"
> > > > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > > > + switch
> > > > >
> > > > > I don't think we can add such a property. At least, we have to add
> > > "renesas,"
> > > > > prefix.
> > > >
> > > > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> > > switch linked with the device.
> > > >
> > > > HD3SS3220 port controller driver gets role switch handle linked with the
> > > device using usb_role_switch_get  api.
> > > > That is the reason, I have added " usb-role-switch" property here.
> > > >
> > > > Do you have any other suggestion to get usb role switch handle?
> > > 
> > > We can still change the API. Can we use the compatible for this?
> > 
> > Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
> > Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
> > In that case, no need to add  generic "usb-role-switch"  property here.
> > 
> > Can you please confirm my understanding is correct?
> 
> No, I meant the compatible property would have the value
> "usb-role-switch". Your compatible would probable look something
> like this:
> 
>         compatible = "renesas,r8a774c0-usb3-peri",
>                      "usb-role-switch";
> 
> So the idea would be that the device supplying USB role switch
> functionality, in your case the USB controller, would need to have the
> compatible property containing "usb-role-switch".

That's not really something a driver could bind to nor provides any info 
as to what the h/w is (and how to interact with it).

> 
> I'm really not an expert in DT, so I don't know if that is acceptable.
> Rob can you comment on this?

I would go with Biju's proposal.

Rob

Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Rob Herring]

On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote:
> Thanks,
> 
> On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > index 35039e7..eecaf4c 100644
> > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > > >  Optional properties:
> > > > > >    - phys: phandle + phy specifier pair
> > > > > >    - phy-names: must be "usb"
> > > > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > > > + switch
> > > > >
> > > > > I don't think we can add such a property. At least, we have to add
> > > "renesas,"
> > > > > prefix.
> > > >
> > > > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> > > switch linked with the device.
> > > >
> > > > HD3SS3220 port controller driver gets role switch handle linked with the
> > > device using usb_role_switch_get  api.
> > > > That is the reason, I have added " usb-role-switch" property here.
> > > >
> > > > Do you have any other suggestion to get usb role switch handle?
> > > 
> > > We can still change the API. Can we use the compatible for this?
> > 
> > Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
> > Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
> > In that case, no need to add  generic "usb-role-switch"  property here.
> > 
> > Can you please confirm my understanding is correct?
> 
> No, I meant the compatible property would have the value
> "usb-role-switch". Your compatible would probable look something
> like this:
> 
>         compatible = "renesas,r8a774c0-usb3-peri",
>                      "usb-role-switch";
> 
> So the idea would be that the device supplying USB role switch
> functionality, in your case the USB controller, would need to have the
> compatible property containing "usb-role-switch".

That's not really something a driver could bind to nor provides any info 
as to what the h/w is (and how to interact with it).

> 
> I'm really not an expert in DT, so I don't know if that is acceptable.
> Rob can you comment on this?

I would go with Biju's proposal.

Rob

[v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Rob Herring]

On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote:
> Thanks,
> 
> On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > index 35039e7..eecaf4c 100644
> > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > > >  Optional properties:
> > > > > >    - phys: phandle + phy specifier pair
> > > > > >    - phy-names: must be "usb"
> > > > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > > > + switch
> > > > >
> > > > > I don't think we can add such a property. At least, we have to add
> > > "renesas,"
> > > > > prefix.
> > > >
> > > > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> > > switch linked with the device.
> > > >
> > > > HD3SS3220 port controller driver gets role switch handle linked with the
> > > device using usb_role_switch_get  api.
> > > > That is the reason, I have added " usb-role-switch" property here.
> > > >
> > > > Do you have any other suggestion to get usb role switch handle?
> > > 
> > > We can still change the API. Can we use the compatible for this?
> > 
> > Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
> > Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
> > In that case, no need to add  generic "usb-role-switch"  property here.
> > 
> > Can you please confirm my understanding is correct?
> 
> No, I meant the compatible property would have the value
> "usb-role-switch". Your compatible would probable look something
> like this:
> 
>         compatible = "renesas,r8a774c0-usb3-peri",
>                      "usb-role-switch";
> 
> So the idea would be that the device supplying USB role switch
> functionality, in your case the USB controller, would need to have the
> compatible property containing "usb-role-switch".

That's not really something a driver could bind to nor provides any info 
as to what the h/w is (and how to interact with it).

> 
> I'm really not an expert in DT, so I don't know if that is acceptable.
> Rob can you comment on this?

I would go with Biju's proposal.

Rob

Re: [PATCH v2 1/7] dt-bindings: usb: hd3ss3220 device tree binding document

[Rob Herring]

On Thu, Mar 14, 2019 at 08:39:29AM +0000, Biju Das wrote:
> Add device tree binding document for TI HD3SS3220 Type-C DRP port
> controller driver.
> 
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
> V1-->V2
>   * Added connector node.
>   * updated the example with connector node.
> ---
>  .../devicetree/bindings/usb/ti,hd3ss3220.txt       | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt

Reviewed-by: Rob Herring <robh@kernel.org>

> diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> new file mode 100644
> index 0000000..7f41400
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> @@ -0,0 +1,37 @@
> +TI HD3SS3220 TypeC DRP Port Controller.
> +
> +Required properties:
> + - compatible: Must be "ti,hd3ss3220".
> + - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin.
> + - interrupts: <a b> where a is the interrupt number and b represents an
> +   encoding of the sense and level information for the interrupt.
> +
> +Required sub-node:
> + - connector : The "usb-c-connector" attached to the hd3ss3220 chip. The
> +   bindings of the connector node are specified in:
> +
> +	Documentation/devicetree/bindings/connector/usb-connector.txt
> +
> +Example:
> +hd3ss3220@47 {
> +	compatible = "ti,hd3ss3220";
> +	reg = <0x47>;
> +	interrupt-parent = <&gpio6>;
> +	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> +
> +	usb_con: connector {
> +		compatible = "usb-c-connector";
> +		label = "USB-C";

Arguably, not that useful of a label as it doesn't describe which USB C 
connector.

> +		data-role = "dual";
> +	};
> +
> +	port {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		hd3ss3220_ep: endpoint@0 {
> +			reg = <0>;
> +			remote-endpoint = <&usb3peri_role_switch>;
> +		};
> +	};
> +};
> -- 
> 2.7.4
> 

[v2,1/7] dt-bindings: usb: hd3ss3220 device tree binding document

[Rob Herring]

On Thu, Mar 14, 2019 at 08:39:29AM +0000, Biju Das wrote:
> Add device tree binding document for TI HD3SS3220 Type-C DRP port
> controller driver.
> 
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
> V1-->V2
>   * Added connector node.
>   * updated the example with connector node.
> ---
>  .../devicetree/bindings/usb/ti,hd3ss3220.txt       | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt

Reviewed-by: Rob Herring <robh@kernel.org>

> diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> new file mode 100644
> index 0000000..7f41400
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
> @@ -0,0 +1,37 @@
> +TI HD3SS3220 TypeC DRP Port Controller.
> +
> +Required properties:
> + - compatible: Must be "ti,hd3ss3220".
> + - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin.
> + - interrupts: <a b> where a is the interrupt number and b represents an
> +   encoding of the sense and level information for the interrupt.
> +
> +Required sub-node:
> + - connector : The "usb-c-connector" attached to the hd3ss3220 chip. The
> +   bindings of the connector node are specified in:
> +
> +	Documentation/devicetree/bindings/connector/usb-connector.txt
> +
> +Example:
> +hd3ss3220@47 {
> +	compatible = "ti,hd3ss3220";
> +	reg = <0x47>;
> +	interrupt-parent = <&gpio6>;
> +	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
> +
> +	usb_con: connector {
> +		compatible = "usb-c-connector";
> +		label = "USB-C";

Arguably, not that useful of a label as it doesn't describe which USB C 
connector.

> +		data-role = "dual";
> +	};
> +
> +	port {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		hd3ss3220_ep: endpoint@0 {
> +			reg = <0>;
> +			remote-endpoint = <&usb3peri_role_switch>;
> +		};
> +	};
> +};
> -- 
> 2.7.4
>

Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Heikki Krogerus]

On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote:
> On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote:
> > Thanks,
> > 
> > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > index 35039e7..eecaf4c 100644
> > > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > > > >  Optional properties:
> > > > > > >    - phys: phandle + phy specifier pair
> > > > > > >    - phy-names: must be "usb"
> > > > > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > > > > + switch
> > > > > >
> > > > > > I don't think we can add such a property. At least, we have to add
> > > > "renesas,"
> > > > > > prefix.
> > > > >
> > > > > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> > > > switch linked with the device.
> > > > >
> > > > > HD3SS3220 port controller driver gets role switch handle linked with the
> > > > device using usb_role_switch_get  api.
> > > > > That is the reason, I have added " usb-role-switch" property here.
> > > > >
> > > > > Do you have any other suggestion to get usb role switch handle?
> > > > 
> > > > We can still change the API. Can we use the compatible for this?
> > > 
> > > Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
> > > Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
> > > In that case, no need to add  generic "usb-role-switch"  property here.
> > > 
> > > Can you please confirm my understanding is correct?
> > 
> > No, I meant the compatible property would have the value
> > "usb-role-switch". Your compatible would probable look something
> > like this:
> > 
> >         compatible = "renesas,r8a774c0-usb3-peri",
> >                      "usb-role-switch";
> > 
> > So the idea would be that the device supplying USB role switch
> > functionality, in your case the USB controller, would need to have the
> > compatible property containing "usb-role-switch".
> 
> That's not really something a driver could bind to nor provides any info 
> as to what the h/w is (and how to interact with it).

Fair enough. As I said, I don't know much about DT.

The problem we are trying to solve here is, how to identify the USB
role switch from graph. It's primarily the USB Type-C connector
drivers that need to be able to get a handle to the role switch device
so they can tell it what to do. Basically the caller of
usb_role_switch_get() will have the compatible "usb-c-connector", so
it's of no use to us. We need the other endpoint, the role switch.

Note: The USB role switch device will be a discrete (or integrated)
mux on platforms that have separate USB host and USB device
controllers, and on platforms with dual-role capable USB controller
(like this one) the USB controller will represent it.

At the moment the function usb_role_switch_get() walks trough the
graph of the caller (so the connector) and expects that the
remote-endpoint representing the mux will have a boolean property
"usb-role-switch".

I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt
should not be the file describing that boolean property, but if we had
a separate file describing the bindings for just the USB role switch
devices, would using it still be OK?

If not, then can you propose something else? How do we identify the
USB role switch endpoint from the graph? Would it be OK to expect the
the endpoint subnode to have that boolean property instead?


thanks,

-- 
heikki

Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Heikki Krogerus]

On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote:
> On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote:
> > Thanks,
> > 
> > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > index 35039e7..eecaf4c 100644
> > > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > > > >  Optional properties:
> > > > > > >    - phys: phandle + phy specifier pair
> > > > > > >    - phy-names: must be "usb"
> > > > > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > > > > + switch
> > > > > >
> > > > > > I don't think we can add such a property. At least, we have to add
> > > > "renesas,"
> > > > > > prefix.
> > > > >
> > > > > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> > > > switch linked with the device.
> > > > >
> > > > > HD3SS3220 port controller driver gets role switch handle linked with the
> > > > device using usb_role_switch_get  api.
> > > > > That is the reason, I have added " usb-role-switch" property here.
> > > > >
> > > > > Do you have any other suggestion to get usb role switch handle?
> > > > 
> > > > We can still change the API. Can we use the compatible for this?
> > > 
> > > Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
> > > Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
> > > In that case, no need to add  generic "usb-role-switch"  property here.
> > > 
> > > Can you please confirm my understanding is correct?
> > 
> > No, I meant the compatible property would have the value
> > "usb-role-switch". Your compatible would probable look something
> > like this:
> > 
> >         compatible = "renesas,r8a774c0-usb3-peri",
> >                      "usb-role-switch";
> > 
> > So the idea would be that the device supplying USB role switch
> > functionality, in your case the USB controller, would need to have the
> > compatible property containing "usb-role-switch".
> 
> That's not really something a driver could bind to nor provides any info 
> as to what the h/w is (and how to interact with it).

Fair enough. As I said, I don't know much about DT.

The problem we are trying to solve here is, how to identify the USB
role switch from graph. It's primarily the USB Type-C connector
drivers that need to be able to get a handle to the role switch device
so they can tell it what to do. Basically the caller of
usb_role_switch_get() will have the compatible "usb-c-connector", so
it's of no use to us. We need the other endpoint, the role switch.

Note: The USB role switch device will be a discrete (or integrated)
mux on platforms that have separate USB host and USB device
controllers, and on platforms with dual-role capable USB controller
(like this one) the USB controller will represent it.

At the moment the function usb_role_switch_get() walks trough the
graph of the caller (so the connector) and expects that the
remote-endpoint representing the mux will have a boolean property
"usb-role-switch".

I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt
should not be the file describing that boolean property, but if we had
a separate file describing the bindings for just the USB role switch
devices, would using it still be OK?

If not, then can you propose something else? How do we identify the
USB role switch endpoint from the graph? Would it be OK to expect the
the endpoint subnode to have that boolean property instead?


thanks,

-- 
heikki

[v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Heikki Krogerus]

On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote:
> On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote:
> > Thanks,
> > 
> > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > index 35039e7..eecaf4c 100644
> > > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > > > >  Optional properties:
> > > > > > >    - phys: phandle + phy specifier pair
> > > > > > >    - phy-names: must be "usb"
> > > > > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > > > > + switch
> > > > > >
> > > > > > I don't think we can add such a property. At least, we have to add
> > > > "renesas,"
> > > > > > prefix.
> > > > >
> > > > > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> > > > switch linked with the device.
> > > > >
> > > > > HD3SS3220 port controller driver gets role switch handle linked with the
> > > > device using usb_role_switch_get  api.
> > > > > That is the reason, I have added " usb-role-switch" property here.
> > > > >
> > > > > Do you have any other suggestion to get usb role switch handle?
> > > > 
> > > > We can still change the API. Can we use the compatible for this?
> > > 
> > > Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
> > > Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
> > > In that case, no need to add  generic "usb-role-switch"  property here.
> > > 
> > > Can you please confirm my understanding is correct?
> > 
> > No, I meant the compatible property would have the value
> > "usb-role-switch". Your compatible would probable look something
> > like this:
> > 
> >         compatible = "renesas,r8a774c0-usb3-peri",
> >                      "usb-role-switch";
> > 
> > So the idea would be that the device supplying USB role switch
> > functionality, in your case the USB controller, would need to have the
> > compatible property containing "usb-role-switch".
> 
> That's not really something a driver could bind to nor provides any info 
> as to what the h/w is (and how to interact with it).

Fair enough. As I said, I don't know much about DT.

The problem we are trying to solve here is, how to identify the USB
role switch from graph. It's primarily the USB Type-C connector
drivers that need to be able to get a handle to the role switch device
so they can tell it what to do. Basically the caller of
usb_role_switch_get() will have the compatible "usb-c-connector", so
it's of no use to us. We need the other endpoint, the role switch.

Note: The USB role switch device will be a discrete (or integrated)
mux on platforms that have separate USB host and USB device
controllers, and on platforms with dual-role capable USB controller
(like this one) the USB controller will represent it.

At the moment the function usb_role_switch_get() walks trough the
graph of the caller (so the connector) and expects that the
remote-endpoint representing the mux will have a boolean property
"usb-role-switch".

I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt
should not be the file describing that boolean property, but if we had
a separate file describing the bindings for just the USB role switch
devices, would using it still be OK?

If not, then can you propose something else? How do we identify the
USB role switch endpoint from the graph? Would it be OK to expect the
the endpoint subnode to have that boolean property instead?


thanks,

Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Rob Herring]

On Thu, Mar 28, 2019 at 12:48 PM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote:
> > On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote:
> > > Thanks,
> > >
> > > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > index 35039e7..eecaf4c 100644
> > > > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > > > > >  Optional properties:
> > > > > > > >    - phys: phandle + phy specifier pair
> > > > > > > >    - phy-names: must be "usb"
> > > > > > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > > > > > + switch
> > > > > > >
> > > > > > > I don't think we can add such a property. At least, we have to add
> > > > > "renesas,"
> > > > > > > prefix.
> > > > > >
> > > > > > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> > > > > switch linked with the device.
> > > > > >
> > > > > > HD3SS3220 port controller driver gets role switch handle linked with the
> > > > > device using usb_role_switch_get  api.
> > > > > > That is the reason, I have added " usb-role-switch" property here.
> > > > > >
> > > > > > Do you have any other suggestion to get usb role switch handle?
> > > > >
> > > > > We can still change the API. Can we use the compatible for this?
> > > >
> > > > Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
> > > > Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
> > > > In that case, no need to add  generic "usb-role-switch"  property here.
> > > >
> > > > Can you please confirm my understanding is correct?
> > >
> > > No, I meant the compatible property would have the value
> > > "usb-role-switch". Your compatible would probable look something
> > > like this:
> > >
> > >         compatible = "renesas,r8a774c0-usb3-peri",
> > >                      "usb-role-switch";
> > >
> > > So the idea would be that the device supplying USB role switch
> > > functionality, in your case the USB controller, would need to have the
> > > compatible property containing "usb-role-switch".
> >
> > That's not really something a driver could bind to nor provides any info
> > as to what the h/w is (and how to interact with it).
>
> Fair enough. As I said, I don't know much about DT.
>
> The problem we are trying to solve here is, how to identify the USB
> role switch from graph. It's primarily the USB Type-C connector
> drivers that need to be able to get a handle to the role switch device
> so they can tell it what to do. Basically the caller of
> usb_role_switch_get() will have the compatible "usb-c-connector", so
> it's of no use to us. We need the other endpoint, the role switch.
>
> Note: The USB role switch device will be a discrete (or integrated)
> mux on platforms that have separate USB host and USB device
> controllers, and on platforms with dual-role capable USB controller
> (like this one) the USB controller will represent it.

Either way, it should just be the device connected to the connector's
USB data port in the graph. Though maybe if USB2 and USB3 are
different controllers that would complicate things.

> At the moment the function usb_role_switch_get() walks trough the
> graph of the caller (so the connector) and expects that the
> remote-endpoint representing the mux will have a boolean property
> "usb-role-switch".

Assuming we have that I think it should be in the device
(controller/mux) node rather than the endpoint node itself.

It could also be outside of DT in that the controller driver will know
the h/w can support role switch and can register that capability. IOW,
it can be implied by the compatible string.

>
> I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt
> should not be the file describing that boolean property, but if we had
> a separate file describing the bindings for just the USB role switch
> devices, would using it still be OK?

It should be described somewhere common, yes, but the property itself
belongs in controller nodes. Generally, we still document where common
properties are used.

Though, I prefer the implied by the compatible option. This should
work unless there's a strong need to find the switch in DT without the
switch driver being probed or if this is board specific (I wouldn't
think so).

>
> If not, then can you propose something else? How do we identify the
> USB role switch endpoint from the graph? Would it be OK to expect the
> the endpoint subnode to have that boolean property instead?
>
>
> thanks,
>
> --
> heikki

Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Rob Herring]

On Thu, Mar 28, 2019 at 12:48 PM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote:
> > On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote:
> > > Thanks,
> > >
> > > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > index 35039e7..eecaf4c 100644
> > > > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > > > > >  Optional properties:
> > > > > > > >    - phys: phandle + phy specifier pair
> > > > > > > >    - phy-names: must be "usb"
> > > > > > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > > > > > + switch
> > > > > > >
> > > > > > > I don't think we can add such a property. At least, we have to add
> > > > > "renesas,"
> > > > > > > prefix.
> > > > > >
> > > > > > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> > > > > switch linked with the device.
> > > > > >
> > > > > > HD3SS3220 port controller driver gets role switch handle linked with the
> > > > > device using usb_role_switch_get  api.
> > > > > > That is the reason, I have added " usb-role-switch" property here.
> > > > > >
> > > > > > Do you have any other suggestion to get usb role switch handle?
> > > > >
> > > > > We can still change the API. Can we use the compatible for this?
> > > >
> > > > Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
> > > > Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
> > > > In that case, no need to add  generic "usb-role-switch"  property here.
> > > >
> > > > Can you please confirm my understanding is correct?
> > >
> > > No, I meant the compatible property would have the value
> > > "usb-role-switch". Your compatible would probable look something
> > > like this:
> > >
> > >         compatible = "renesas,r8a774c0-usb3-peri",
> > >                      "usb-role-switch";
> > >
> > > So the idea would be that the device supplying USB role switch
> > > functionality, in your case the USB controller, would need to have the
> > > compatible property containing "usb-role-switch".
> >
> > That's not really something a driver could bind to nor provides any info
> > as to what the h/w is (and how to interact with it).
>
> Fair enough. As I said, I don't know much about DT.
>
> The problem we are trying to solve here is, how to identify the USB
> role switch from graph. It's primarily the USB Type-C connector
> drivers that need to be able to get a handle to the role switch device
> so they can tell it what to do. Basically the caller of
> usb_role_switch_get() will have the compatible "usb-c-connector", so
> it's of no use to us. We need the other endpoint, the role switch.
>
> Note: The USB role switch device will be a discrete (or integrated)
> mux on platforms that have separate USB host and USB device
> controllers, and on platforms with dual-role capable USB controller
> (like this one) the USB controller will represent it.

Either way, it should just be the device connected to the connector's
USB data port in the graph. Though maybe if USB2 and USB3 are
different controllers that would complicate things.

> At the moment the function usb_role_switch_get() walks trough the
> graph of the caller (so the connector) and expects that the
> remote-endpoint representing the mux will have a boolean property
> "usb-role-switch".

Assuming we have that I think it should be in the device
(controller/mux) node rather than the endpoint node itself.

It could also be outside of DT in that the controller driver will know
the h/w can support role switch and can register that capability. IOW,
it can be implied by the compatible string.

>
> I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt
> should not be the file describing that boolean property, but if we had
> a separate file describing the bindings for just the USB role switch
> devices, would using it still be OK?

It should be described somewhere common, yes, but the property itself
belongs in controller nodes. Generally, we still document where common
properties are used.

Though, I prefer the implied by the compatible option. This should
work unless there's a strong need to find the switch in DT without the
switch driver being probed or if this is board specific (I wouldn't
think so).

>
> If not, then can you propose something else? How do we identify the
> USB role switch endpoint from the graph? Would it be OK to expect the
> the endpoint subnode to have that boolean property instead?
>
>
> thanks,
>
> --
> heikki

[v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Rob Herring]

On Thu, Mar 28, 2019 at 12:48 PM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote:
> > On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote:
> > > Thanks,
> > >
> > > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > index 35039e7..eecaf4c 100644
> > > > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > > > > >  Optional properties:
> > > > > > > >    - phys: phandle + phy specifier pair
> > > > > > > >    - phy-names: must be "usb"
> > > > > > > > +  - usb-role-switch: use USB role switch to support dual-role
> > > > > > > > + switch
> > > > > > >
> > > > > > > I don't think we can add such a property. At least, we have to add
> > > > > "renesas,"
> > > > > > > prefix.
> > > > > >
> > > > > > usb_role_switch_get   api uses  "usb-role-switch"  property to get role
> > > > > switch linked with the device.
> > > > > >
> > > > > > HD3SS3220 port controller driver gets role switch handle linked with the
> > > > > device using usb_role_switch_get  api.
> > > > > > That is the reason, I have added " usb-role-switch" property here.
> > > > > >
> > > > > > Do you have any other suggestion to get usb role switch handle?
> > > > >
> > > > > We can still change the API. Can we use the compatible for this?
> > > >
> > > > Do you mean usb_role_switch_get  API needs  to handle compatible "usb-x-connector"  wherex= a,b ,c ?
> > > > Then it uses the graphs api to get the device linked with it and return the usb role switch handle.
> > > > In that case, no need to add  generic "usb-role-switch"  property here.
> > > >
> > > > Can you please confirm my understanding is correct?
> > >
> > > No, I meant the compatible property would have the value
> > > "usb-role-switch". Your compatible would probable look something
> > > like this:
> > >
> > >         compatible = "renesas,r8a774c0-usb3-peri",
> > >                      "usb-role-switch";
> > >
> > > So the idea would be that the device supplying USB role switch
> > > functionality, in your case the USB controller, would need to have the
> > > compatible property containing "usb-role-switch".
> >
> > That's not really something a driver could bind to nor provides any info
> > as to what the h/w is (and how to interact with it).
>
> Fair enough. As I said, I don't know much about DT.
>
> The problem we are trying to solve here is, how to identify the USB
> role switch from graph. It's primarily the USB Type-C connector
> drivers that need to be able to get a handle to the role switch device
> so they can tell it what to do. Basically the caller of
> usb_role_switch_get() will have the compatible "usb-c-connector", so
> it's of no use to us. We need the other endpoint, the role switch.
>
> Note: The USB role switch device will be a discrete (or integrated)
> mux on platforms that have separate USB host and USB device
> controllers, and on platforms with dual-role capable USB controller
> (like this one) the USB controller will represent it.

Either way, it should just be the device connected to the connector's
USB data port in the graph. Though maybe if USB2 and USB3 are
different controllers that would complicate things.

> At the moment the function usb_role_switch_get() walks trough the
> graph of the caller (so the connector) and expects that the
> remote-endpoint representing the mux will have a boolean property
> "usb-role-switch".

Assuming we have that I think it should be in the device
(controller/mux) node rather than the endpoint node itself.

It could also be outside of DT in that the controller driver will know
the h/w can support role switch and can register that capability. IOW,
it can be implied by the compatible string.

>
> I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt
> should not be the file describing that boolean property, but if we had
> a separate file describing the bindings for just the USB role switch
> devices, would using it still be OK?

It should be described somewhere common, yes, but the property itself
belongs in controller nodes. Generally, we still document where common
properties are used.

Though, I prefer the implied by the compatible option. This should
work unless there's a strong need to find the switch in DT without the
switch driver being probed or if this is board specific (I wouldn't
think so).

>
> If not, then can you propose something else? How do we identify the
> USB role switch endpoint from the graph? Would it be OK to expect the
> the endpoint subnode to have that boolean property instead?
>
>
> thanks,
>
> --
> heikki

RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Biju Das]

Hi All,

Thanks for the feedback.

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 29 March 2019 13:58
> To: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Biju Das <biju.das@bp.renesas.com>; Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com>; Mark Rutland
> <mark.rutland@arm.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Felipe Balbi <balbi@kernel.org>; linux-
> usb@vger.kernel.org; devicetree@vger.kernel.org; Simon Horman
> <horms@verge.net.au>; Geert Uytterhoeven <geert+renesas@glider.be>;
> Chris Paterson <Chris.Paterson2@renesas.com>; Fabrizio Castro
> <fabrizio.castro@bp.renesas.com>; linux-renesas-soc@vger.kernel.org
> Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> switch property
> 
> On Thu, Mar 28, 2019 at 12:48 PM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote:
> > > On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote:
> > > > Thanks,
> > > >
> > > > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > > > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3:
> > > > > > add usb-role-
> > > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > > index 35039e7..eecaf4c 100644
> > > > > > > > > ---
> > > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > > +++
> b/Documentation/devicetree/bindings/usb/renesas_usb3
> > > > > > > > > +++ .txt
> > > > > > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > > > > > >  Optional properties:
> > > > > > > > >    - phys: phandle + phy specifier pair
> > > > > > > > >    - phy-names: must be "usb"
> > > > > > > > > +  - usb-role-switch: use USB role switch to support
> > > > > > > > > + dual-role switch
> > > > > > > >
> > > > > > > > I don't think we can add such a property. At least, we
> > > > > > > > have to add
> > > > > > "renesas,"
> > > > > > > > prefix.
> > > > > > >
> > > > > > > usb_role_switch_get   api uses  "usb-role-switch"  property to get
> role
> > > > > > switch linked with the device.
> > > > > > >
> > > > > > > HD3SS3220 port controller driver gets role switch handle
> > > > > > > linked with the
> > > > > > device using usb_role_switch_get  api.
> > > > > > > That is the reason, I have added " usb-role-switch" property here.
> > > > > > >
> > > > > > > Do you have any other suggestion to get usb role switch handle?
> > > > > >
> > > > > > We can still change the API. Can we use the compatible for this?
> > > > >
> > > > > Do you mean usb_role_switch_get  API needs  to handle compatible
> "usb-x-connector"  wherex= a,b ,c ?
> > > > > Then it uses the graphs api to get the device linked with it and return
> the usb role switch handle.
> > > > > In that case, no need to add  generic "usb-role-switch"  property
> here.
> > > > >
> > > > > Can you please confirm my understanding is correct?
> > > >
> > > > No, I meant the compatible property would have the value
> > > > "usb-role-switch". Your compatible would probable look something
> > > > like this:
> > > >
> > > >         compatible = "renesas,r8a774c0-usb3-peri",
> > > >                      "usb-role-switch";
> > > >
> > > > So the idea would be that the device supplying USB role switch
> > > > functionality, in your case the USB controller, would need to have
> > > > the compatible property containing "usb-role-switch".
> > >
> > > That's not really something a driver could bind to nor provides any
> > > info as to what the h/w is (and how to interact with it).
> >
> > Fair enough. As I said, I don't know much about DT.
> >
> > The problem we are trying to solve here is, how to identify the USB
> > role switch from graph. It's primarily the USB Type-C connector
> > drivers that need to be able to get a handle to the role switch device
> > so they can tell it what to do. Basically the caller of
> > usb_role_switch_get() will have the compatible "usb-c-connector", so
> > it's of no use to us. We need the other endpoint, the role switch.
> >
> > Note: The USB role switch device will be a discrete (or integrated)
> > mux on platforms that have separate USB host and USB device
> > controllers, and on platforms with dual-role capable USB controller
> > (like this one) the USB controller will represent it.

Looks like we have a solution now. Heikki already proposed a new API
"add API to get usb_role_switch by node" https://patchwork.kernel.org/patch/10882555/

We can use this API to find remote endpoint of USB role switch device(renesas usb3) connected with type-c DRP port controller driver(TI HD3SS3220).
I believe in this case, we don't need generic Boolean property " usb-role-switch"

Apart from this, we can add Renesas specific property (renesas, usb-role-switch) to differentiate the
USB role switch associated with type C DRP controller driver from others.

I will send V3 based on this solution. Please let me know if you think otherwise.

Regards,
Biju

> Either way, it should just be the device connected to the connector's USB
> data port in the graph. Though maybe if USB2 and USB3 are different
> controllers that would complicate things.
> 
> > At the moment the function usb_role_switch_get() walks trough the
> > graph of the caller (so the connector) and expects that the
> > remote-endpoint representing the mux will have a boolean property
> > "usb-role-switch".
> 
> Assuming we have that I think it should be in the device
> (controller/mux) node rather than the endpoint node itself.
> 
> It could also be outside of DT in that the controller driver will know the h/w
> can support role switch and can register that capability. IOW, it can be implied
> by the compatible string.
> 
> >
> > I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > should not be the file describing that boolean property, but if we had
> > a separate file describing the bindings for just the USB role switch
> > devices, would using it still be OK?
> 
> It should be described somewhere common, yes, but the property itself
> belongs in controller nodes. Generally, we still document where common
> properties are used.
> 
> Though, I prefer the implied by the compatible option. This should work
> unless there's a strong need to find the switch in DT without the switch driver
> being probed or if this is board specific (I wouldn't think so).
> 
> >
> > If not, then can you propose something else? How do we identify the
> > USB role switch endpoint from the graph? Would it be OK to expect the
> > the endpoint subnode to have that boolean property instead?
> >

RE: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Biju Das]

Hi All,

Thanks for the feedback.

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 29 March 2019 13:58
> To: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Biju Das <biju.das@bp.renesas.com>; Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com>; Mark Rutland
> <mark.rutland@arm.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Felipe Balbi <balbi@kernel.org>; linux-
> usb@vger.kernel.org; devicetree@vger.kernel.org; Simon Horman
> <horms@verge.net.au>; Geert Uytterhoeven <geert+renesas@glider.be>;
> Chris Paterson <Chris.Paterson2@renesas.com>; Fabrizio Castro
> <fabrizio.castro@bp.renesas.com>; linux-renesas-soc@vger.kernel.org
> Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> switch property
> 
> On Thu, Mar 28, 2019 at 12:48 PM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote:
> > > On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote:
> > > > Thanks,
> > > >
> > > > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > > > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3:
> > > > > > add usb-role-
> > > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > > index 35039e7..eecaf4c 100644
> > > > > > > > > ---
> > > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > > +++
> b/Documentation/devicetree/bindings/usb/renesas_usb3
> > > > > > > > > +++ .txt
> > > > > > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > > > > > >  Optional properties:
> > > > > > > > >    - phys: phandle + phy specifier pair
> > > > > > > > >    - phy-names: must be "usb"
> > > > > > > > > +  - usb-role-switch: use USB role switch to support
> > > > > > > > > + dual-role switch
> > > > > > > >
> > > > > > > > I don't think we can add such a property. At least, we
> > > > > > > > have to add
> > > > > > "renesas,"
> > > > > > > > prefix.
> > > > > > >
> > > > > > > usb_role_switch_get   api uses  "usb-role-switch"  property to get
> role
> > > > > > switch linked with the device.
> > > > > > >
> > > > > > > HD3SS3220 port controller driver gets role switch handle
> > > > > > > linked with the
> > > > > > device using usb_role_switch_get  api.
> > > > > > > That is the reason, I have added " usb-role-switch" property here.
> > > > > > >
> > > > > > > Do you have any other suggestion to get usb role switch handle?
> > > > > >
> > > > > > We can still change the API. Can we use the compatible for this?
> > > > >
> > > > > Do you mean usb_role_switch_get  API needs  to handle compatible
> "usb-x-connector"  wherex= a,b ,c ?
> > > > > Then it uses the graphs api to get the device linked with it and return
> the usb role switch handle.
> > > > > In that case, no need to add  generic "usb-role-switch"  property
> here.
> > > > >
> > > > > Can you please confirm my understanding is correct?
> > > >
> > > > No, I meant the compatible property would have the value
> > > > "usb-role-switch". Your compatible would probable look something
> > > > like this:
> > > >
> > > >         compatible = "renesas,r8a774c0-usb3-peri",
> > > >                      "usb-role-switch";
> > > >
> > > > So the idea would be that the device supplying USB role switch
> > > > functionality, in your case the USB controller, would need to have
> > > > the compatible property containing "usb-role-switch".
> > >
> > > That's not really something a driver could bind to nor provides any
> > > info as to what the h/w is (and how to interact with it).
> >
> > Fair enough. As I said, I don't know much about DT.
> >
> > The problem we are trying to solve here is, how to identify the USB
> > role switch from graph. It's primarily the USB Type-C connector
> > drivers that need to be able to get a handle to the role switch device
> > so they can tell it what to do. Basically the caller of
> > usb_role_switch_get() will have the compatible "usb-c-connector", so
> > it's of no use to us. We need the other endpoint, the role switch.
> >
> > Note: The USB role switch device will be a discrete (or integrated)
> > mux on platforms that have separate USB host and USB device
> > controllers, and on platforms with dual-role capable USB controller
> > (like this one) the USB controller will represent it.

Looks like we have a solution now. Heikki already proposed a new API
"add API to get usb_role_switch by node" https://patchwork.kernel.org/patch/10882555/

We can use this API to find remote endpoint of USB role switch device(renesas usb3) connected with type-c DRP port controller driver(TI HD3SS3220).
I believe in this case, we don't need generic Boolean property " usb-role-switch"

Apart from this, we can add Renesas specific property (renesas, usb-role-switch) to differentiate the
USB role switch associated with type C DRP controller driver from others.

I will send V3 based on this solution. Please let me know if you think otherwise.

Regards,
Biju

> Either way, it should just be the device connected to the connector's USB
> data port in the graph. Though maybe if USB2 and USB3 are different
> controllers that would complicate things.
> 
> > At the moment the function usb_role_switch_get() walks trough the
> > graph of the caller (so the connector) and expects that the
> > remote-endpoint representing the mux will have a boolean property
> > "usb-role-switch".
> 
> Assuming we have that I think it should be in the device
> (controller/mux) node rather than the endpoint node itself.
> 
> It could also be outside of DT in that the controller driver will know the h/w
> can support role switch and can register that capability. IOW, it can be implied
> by the compatible string.
> 
> >
> > I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > should not be the file describing that boolean property, but if we had
> > a separate file describing the bindings for just the USB role switch
> > devices, would using it still be OK?
> 
> It should be described somewhere common, yes, but the property itself
> belongs in controller nodes. Generally, we still document where common
> properties are used.
> 
> Though, I prefer the implied by the compatible option. This should work
> unless there's a strong need to find the switch in DT without the switch driver
> being probed or if this is board specific (I wouldn't think so).
> 
> >
> > If not, then can you propose something else? How do we identify the
> > USB role switch endpoint from the graph? Would it be OK to expect the
> > the endpoint subnode to have that boolean property instead?
> >

[v2,2/7] dt-bindings: usb: renesas_usb3: add usb-role-switch property

[Biju Das]

Hi All,

Thanks for the feedback.

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 29 March 2019 13:58
> To: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Cc: Biju Das <biju.das@bp.renesas.com>; Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com>; Mark Rutland
> <mark.rutland@arm.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Felipe Balbi <balbi@kernel.org>; linux-
> usb@vger.kernel.org; devicetree@vger.kernel.org; Simon Horman
> <horms@verge.net.au>; Geert Uytterhoeven <geert+renesas@glider.be>;
> Chris Paterson <Chris.Paterson2@renesas.com>; Fabrizio Castro
> <fabrizio.castro@bp.renesas.com>; linux-renesas-soc@vger.kernel.org
> Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role-
> switch property
> 
> On Thu, Mar 28, 2019 at 12:48 PM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote:
> > > On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote:
> > > > Thanks,
> > > >
> > > > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote:
> > > > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3:
> > > > > > add usb-role-
> > > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > > index 35039e7..eecaf4c 100644
> > > > > > > > > ---
> > > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > > > > > > > > +++
> b/Documentation/devicetree/bindings/usb/renesas_usb3
> > > > > > > > > +++ .txt
> > > > > > > > > @@ -22,6 +22,7 @@ Required properties:
> > > > > > > > >  Optional properties:
> > > > > > > > >    - phys: phandle + phy specifier pair
> > > > > > > > >    - phy-names: must be "usb"
> > > > > > > > > +  - usb-role-switch: use USB role switch to support
> > > > > > > > > + dual-role switch
> > > > > > > >
> > > > > > > > I don't think we can add such a property. At least, we
> > > > > > > > have to add
> > > > > > "renesas,"
> > > > > > > > prefix.
> > > > > > >
> > > > > > > usb_role_switch_get   api uses  "usb-role-switch"  property to get
> role
> > > > > > switch linked with the device.
> > > > > > >
> > > > > > > HD3SS3220 port controller driver gets role switch handle
> > > > > > > linked with the
> > > > > > device using usb_role_switch_get  api.
> > > > > > > That is the reason, I have added " usb-role-switch" property here.
> > > > > > >
> > > > > > > Do you have any other suggestion to get usb role switch handle?
> > > > > >
> > > > > > We can still change the API. Can we use the compatible for this?
> > > > >
> > > > > Do you mean usb_role_switch_get  API needs  to handle compatible
> "usb-x-connector"  wherex= a,b ,c ?
> > > > > Then it uses the graphs api to get the device linked with it and return
> the usb role switch handle.
> > > > > In that case, no need to add  generic "usb-role-switch"  property
> here.
> > > > >
> > > > > Can you please confirm my understanding is correct?
> > > >
> > > > No, I meant the compatible property would have the value
> > > > "usb-role-switch". Your compatible would probable look something
> > > > like this:
> > > >
> > > >         compatible = "renesas,r8a774c0-usb3-peri",
> > > >                      "usb-role-switch";
> > > >
> > > > So the idea would be that the device supplying USB role switch
> > > > functionality, in your case the USB controller, would need to have
> > > > the compatible property containing "usb-role-switch".
> > >
> > > That's not really something a driver could bind to nor provides any
> > > info as to what the h/w is (and how to interact with it).
> >
> > Fair enough. As I said, I don't know much about DT.
> >
> > The problem we are trying to solve here is, how to identify the USB
> > role switch from graph. It's primarily the USB Type-C connector
> > drivers that need to be able to get a handle to the role switch device
> > so they can tell it what to do. Basically the caller of
> > usb_role_switch_get() will have the compatible "usb-c-connector", so
> > it's of no use to us. We need the other endpoint, the role switch.
> >
> > Note: The USB role switch device will be a discrete (or integrated)
> > mux on platforms that have separate USB host and USB device
> > controllers, and on platforms with dual-role capable USB controller
> > (like this one) the USB controller will represent it.

Looks like we have a solution now. Heikki already proposed a new API
"add API to get usb_role_switch by node" https://patchwork.kernel.org/patch/10882555/

We can use this API to find remote endpoint of USB role switch device(renesas usb3) connected with type-c DRP port controller driver(TI HD3SS3220).
I believe in this case, we don't need generic Boolean property " usb-role-switch"

Apart from this, we can add Renesas specific property (renesas, usb-role-switch) to differentiate the
USB role switch associated with type C DRP controller driver from others.

I will send V3 based on this solution. Please let me know if you think otherwise.

Regards,
Biju

> Either way, it should just be the device connected to the connector's USB
> data port in the graph. Though maybe if USB2 and USB3 are different
> controllers that would complicate things.
> 
> > At the moment the function usb_role_switch_get() walks trough the
> > graph of the caller (so the connector) and expects that the
> > remote-endpoint representing the mux will have a boolean property
> > "usb-role-switch".
> 
> Assuming we have that I think it should be in the device
> (controller/mux) node rather than the endpoint node itself.
> 
> It could also be outside of DT in that the controller driver will know the h/w
> can support role switch and can register that capability. IOW, it can be implied
> by the compatible string.
> 
> >
> > I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > should not be the file describing that boolean property, but if we had
> > a separate file describing the bindings for just the USB role switch
> > devices, would using it still be OK?
> 
> It should be described somewhere common, yes, but the property itself
> belongs in controller nodes. Generally, we still document where common
> properties are used.
> 
> Though, I prefer the implied by the compatible option. This should work
> unless there's a strong need to find the switch in DT without the switch driver
> being probed or if this is board specific (I wouldn't think so).
> 
> >
> > If not, then can you propose something else? How do we identify the
> > USB role switch endpoint from the graph? Would it be OK to expect the
> > the endpoint subnode to have that boolean property instead?
> >