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

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-20190514 branch.
V5-->V6
  * Updated bindings patch by using usb-role-switch property
  * Used graph api's to get role switch supported by the usb-c-connector
V4-->V5
  * Incorporated Heikki's review comment.
    (https://patchwork.kernel.org/patch/10902531/)
  * Patch 1 is a dependency patch to make the compilation successful.
    (https://patchwork.kernel.org/patch/10909971/)
  * Patch 2 is also dependency patch to make the compilation successful.
    Rebased on top of Patch1 and fixed Heikki's review comments
    (https://patchwork.kernel.org/patch/10882555/)
  * Incorporated Shimoda-San's review comment.
    (https://patchwork.kernel.org/patch/10902535/)
V3-->V4
  * Incorporated Chunfeng Yun's review comment
    (https://patchwork.kernel.org/project/linux-usb/list/?submitter=133171)
  * Used fwnode API's to get roleswitch handle

V2-->V3
  * Used the new API to usb_role_switch by node to find the remote endpoint
    (https://patchwork.kernel.org/patch/10882555/)
  * Added renesas,usb-role-switch property
  * Incorporated shimoda-san's review comment
    (https://patchwork.kernel.org/patch/10852507/)

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: Document usb role switch support
  usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller
  usb: gadget: udc: renesas_usb3: Add dual role switch support
  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       |  26 ++
 .../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              | 121 +++++++++-
 drivers/usb/typec/Kconfig                          |  10 +
 drivers/usb/typec/Makefile                         |   1 +
 drivers/usb/typec/hd3ss3220.c                      | 263 +++++++++++++++++++++
 8 files changed, 509 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
 create mode 100644 drivers/usb/typec/hd3ss3220.c

-- 
2.7.4

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

Signed-off-by: Biju Das <redacted>
Reviewed-by: Rob Herring <robh@kernel.org>
---
V5-->V6
  * No change.
V4-->V5
  * No Change.
V3-->V4
  * No Change.
V2-->V3
  * Added Rob's Reviewed by tag.
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

Update the DT bindings documentation to support usb role switch
for USB Type-C connector using USB role switch class framework.

Signed-off-by: Biju Das <redacted>
---
 V5-->V6
  * Updated description
  * Added usb-role-switch-property
 V4-->V5
  * No Change
 V3-->V4
  * No Change
 V2-->V3
  * Added optional renesas,usb-role-switch property.
 V1-->V2
  * Added usb-role-switch-property
  * Updated the example with usb-role-switch property.
---
 .../devicetree/bindings/usb/renesas_usb3.txt       | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
index 35039e7..ea6c63c 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt

@@ -22,6 +22,11 @@ Required properties:
 Optional properties:
   - phys: phandle + phy specifier pair
   - phy-names: must be "usb"
+  - usb-role-switch: support role switch. see usb/generic.txt
+
+Sub-nodes:
+The port would be added as a subnode if the "usb-role-switch" property is used.
+	see graph.txt
 
 Example of R-Car H3 ES1.x:
 	usb3_peri0: usb@ee020000 {

@@ -39,3 +44,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

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 <redacted>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 V5-->V6
   * No change
   Note: This patch depend on [1]
   [1]: [v5,4/6] usb: roles: add API to get usb_role_switch by node
   https://patchwork.kernel.org/patch/10942499/
 V4-->V5
   * Incorporated Heikki's review comment
     (https://patchwork.kernel.org/patch/10902531/)
   * Added Heikki's Reviewed-by tag
 V3-->V4
   * Incorporated Chunfeng Yun's review comment
   * Used fwnode API's to get usb role switch handle.
 
 V2-->V3
   * Used the new api "usb_role_switch by node" for getting
     remote endpoint associated with Type-C USB DRP port
     controller devices.
 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 | 263 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 274 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..7ef7e85
--- /dev/null
+++ b/drivers/usb/typec/hd3ss3220.c

@@ -0,0 +1,263 @@
+// 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/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)
+{
+	return regmap_update_bits(hd3ss3220->regmap, HD3SS3220_REG_GEN_CTRL,
+				  HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK,
+				  src_pref);
+}
+
+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;
+		break;
+	}
+
+	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);
+
+	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;
+
+	hd3ss3220_set_role(hd3ss3220);
+	err = regmap_update_bits_base(hd3ss3220->regmap,
+				      HD3SS3220_REG_CN_STAT_CTRL,
+				      HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
+				      HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
+				      NULL, false, true);
+	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;
+	struct fwnode_handle *parent, *child;
+	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->dev = &client->dev;
+	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);
+	child = fwnode_graph_get_next_endpoint(dev_fwnode(hd3ss3220->dev),
+					       NULL);
+	parent = fwnode_graph_get_remote_port_parent(child);
+	hd3ss3220->role_sw = fwnode_usb_role_switch_get(parent);
+	if (IS_ERR_OR_NULL(hd3ss3220->role_sw)) {
+		fwnode_handle_put(child);
+		fwnode_handle_put(parent);
+		return PTR_ERR(hd3ss3220->role_sw);
+	}
+
+	fwnode_handle_put(child);
+	fwnode_handle_put(parent);
+
+	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);
+	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;
+	}
+
+	if (client->irq > 0) {
+		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",
+		.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

The RZ/G2E cat874 board has a type-c connector connected to hd3ss3220 usb
type-c drp port controller. This patch adds dual role switch support for
the type-c connector using the usb role switch class framework.

Signed-off-by: Biju Das <redacted>
---
 V5-->V6
   * Added graph api's to find the role supported by the connector.
 V4-->V5
   * Incorporated Shimoda-san's review comment
    (https://patchwork.kernel.org/patch/10902537/)
 V3-->V4
   * No Change
 V2-->V3
   * Incorporated Shimoda-san's review comment
     (https://patchwork.kernel.org/patch/10852507/)
   * Used renesas,usb-role-switch property for differentiating USB
     role switch associated with Type-C port controller driver.
 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 | 121 ++++++++++++++++++++++++++++++++--
 1 file changed, 114 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
index 7dc2485..1d41998 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c

@@ -24,6 +24,7 @@
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/of.h>
+#include <linux/of_graph.h>
 #include <linux/usb/role.h>
 
 /* register definitions */

@@ -351,6 +352,8 @@ struct renesas_usb3 {
 	int disabled_count;
 
 	struct usb_request *ep0_req;
+
+	enum usb_role connection_state;
 	u16 test_mode;
 	u8 ep0_buf[USB3_EP0_BUF_SIZE];
 	bool softconnect;

@@ -359,6 +362,7 @@ struct renesas_usb3 {
 	bool extcon_usb;		/* check vbus and set EXTCON_USB */
 	bool forced_b_device;
 	bool start_to_connect;
+	bool dual_role_sw;
 };
 
 #define gadget_to_renesas_usb3(_gadget)	\

@@ -699,8 +703,10 @@ 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->dual_role_sw || 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);

@@ -716,7 +722,8 @@ static void usb3_check_id(struct renesas_usb3 *usb3)
 {
 	usb3->extcon_host = usb3_is_a_device(usb3);
 
-	if (usb3->extcon_host && !usb3->forced_b_device)
+	if ((!usb3->dual_role_sw && usb3->extcon_host && !usb3->forced_b_device)
+	    || usb3->connection_state == USB_ROLE_HOST)
 		usb3_mode_config(usb3, true, true);
 	else
 		usb3_mode_config(usb3, false, false);

@@ -2343,14 +2350,65 @@ 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->connection_state = USB_ROLE_HOST;
+			usb3_set_mode(usb3, true);
+			usb3_vbus_out(usb3, true);
+			if (device_attach(host) < 0)
+				dev_err(dev, "device_attach(host) failed\n");
+		} 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 +2419,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->dual_role_sw)
+		handle_ext_role_switch_states(dev, role);
+	else
+		handle_role_switch_states(dev, role);
+
 	pm_runtime_put(dev);
 
 	return 0;

@@ -2650,12 +2722,41 @@ 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,
 };
 
+static bool is_usb_dual_role_switch(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct device_node *parent;
+	struct device_node *child;
+	bool ret = false;
+	const char *role_type = NULL;
+
+	child = of_graph_get_endpoint_by_regs(np, -1, -1);
+	if (!child)
+		return ret;
+
+	parent = of_graph_get_remote_port_parent(child);
+	of_node_put(child);
+	child = of_get_child_by_name(parent, "connector");
+	of_node_put(parent);
+	if (!child)
+		return ret;
+
+	if (of_device_is_compatible(child, "usb-c-connector")) {
+		of_property_read_string(child, "data-role", &role_type);
+		if (role_type && (!strncmp(role_type, "dual", strlen("dual"))))
+			ret = true;
+	}
+
+	of_node_put(child);
+	return ret;
+}
+
 static int renesas_usb3_probe(struct platform_device *pdev)
 {
 	struct renesas_usb3 *usb3;

@@ -2741,6 +2842,12 @@ 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") &&
+	    is_usb_dual_role_switch(&pdev->dev)) {
+		usb3->dual_role_sw = 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

Hello!

On 05/15/2019 03:09 PM, Biju Das wrote:

   This depends on an interrupt controller used. I'd just said "an interrupt
specifier", w/o further details.

[...]

MBR, Sergei

On Wed, 2019-05-15 at 13:09 +0100, Biju Das wrote:

To me, it's not good idea to pay an attention to specific consumer of
the role switch, assume any device could assign role if it get this USB
role switch, not only type-c connector

+ Rob

Hi Chunfeng Yun,

Thanks for the feedback.

Subject: Re: [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role
switch support

On Wed, 2019-05-15 at 13:09 +0100, Biju Das wrote:

quoted

The RZ/G2E cat874 board has a type-c connector connected to hd3ss3220
usb type-c drp port controller. This patch adds dual role switch
support for the type-c connector using the usb role switch class framework.

Signed-off-by: Biju Das <redacted>
---
 V5-->V6
   * Added graph api's to find the role supported by the connector.
 V4-->V5
   * Incorporated Shimoda-san's review comment
    (https://patchwork.kernel.org/patch/10902537/)
 V3-->V4
   * No Change
 V2-->V3
   * Incorporated Shimoda-san's review comment
     (https://patchwork.kernel.org/patch/10852507/)
   * Used renesas,usb-role-switch property for differentiating USB
     role switch associated with Type-C port controller driver.
 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 | 121
++++++++++++++++++++++++++++++++--
 1 file changed, 114 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c

b/drivers/usb/gadget/udc/renesas_usb3.c
index 7dc2485..1d41998 100644

--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c

@@ -24,6 +24,7 @@
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/of.h>
+#include <linux/of_graph.h>
 #include <linux/usb/role.h>

 /* register definitions */

@@ -351,6 +352,8 @@ struct renesas_usb3 {
 	int disabled_count;

 	struct usb_request *ep0_req;
+
+	enum usb_role connection_state;
 	u16 test_mode;
 	u8 ep0_buf[USB3_EP0_BUF_SIZE];
 	bool softconnect;

@@ -359,6 +362,7 @@ struct renesas_usb3 {
 	bool extcon_usb;		/* check vbus and set EXTCON_USB

*/

quoted

 	bool forced_b_device;
 	bool start_to_connect;
+	bool dual_role_sw;
 };

 #define gadget_to_renesas_usb3(_gadget)	\

@@ -699,8 +703,10 @@ static void usb3_mode_config(struct renesas_usb3

*usb3, bool host, bool a_dev)

quoted

 	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->dual_role_sw || usb3->connection_state !=

USB_ROLE_NONE) {

quoted

+		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);

@@ -716,7 +722,8 @@ static void usb3_check_id(struct renesas_usb3

*usb3)  {
 	usb3->extcon_host = usb3_is_a_device(usb3);

-	if (usb3->extcon_host && !usb3->forced_b_device)
+	if ((!usb3->dual_role_sw && usb3->extcon_host && !usb3-
forced_b_device)
+	    || usb3->connection_state == USB_ROLE_HOST)
 		usb3_mode_config(usb3, true, true);
 	else
 		usb3_mode_config(usb3, false, false); @@ -2343,14 +2350,65

@@

quoted

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->connection_state = USB_ROLE_HOST;
+			usb3_set_mode(usb3, true);
+			usb3_vbus_out(usb3, true);
+			if (device_attach(host) < 0)
+				dev_err(dev, "device_attach(host)

failed\n");

quoted

+		} else if (cur_role == USB_ROLE_DEVICE) {
+			usb3_disconnect(usb3);
+			/* Must set the mode before device_attach of the

host */

quoted

+			usb3_set_mode(usb3, true);
+			/* This device_attach() might sleep */
+			if (device_attach(host) < 0)
+				dev_err(dev, "device_attach(host)

failed\n");

quoted

+		}
+		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 +2419,20 @@ static int renesas_usb3_role_switch_set(struct

device *dev,

quoted

 		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->dual_role_sw)
+		handle_ext_role_switch_states(dev, role);
+	else
+		handle_role_switch_states(dev, role);
+
 	pm_runtime_put(dev);

 	return 0;

@@ -2650,12 +2722,41 @@ static const unsigned int renesas_usb3_cable[]

= {

quoted

 	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,
 };

+static bool is_usb_dual_role_switch(struct device *dev)

To me, it's not good idea to pay an attention to specific consumer of the role
switch, assume any device could assign role if it get this USB role switch, not
only type-c connector

Yes, I agree. 

I have previously posted a patch based on this [1].
[1]. https://patchwork.kernel.org/patch/10852505/

Then on the binding patch, Rob suggested   to walk the graph to the connector
and determine if dual role is supported by the connector type [2]
[2]. https://patchwork.kernel.org/patch/10914379/

The  purpose of "usb role switch" is to  assign the roles. So looks like, we don't need this function.
Please correct me, if I am wrong.

Regards,
Biju 

quoted

+{
+	struct device_node *np = dev->of_node;
+	struct device_node *parent;
+	struct device_node *child;
+	bool ret = false;
+	const char *role_type = NULL;
+
+	child = of_graph_get_endpoint_by_regs(np, -1, -1);
+	if (!child)
+		return ret;
+
+	parent = of_graph_get_remote_port_parent(child);
+	of_node_put(child);
+	child = of_get_child_by_name(parent, "connector");
+	of_node_put(parent);
+	if (!child)
+		return ret;
+
+	if (of_device_is_compatible(child, "usb-c-connector")) {
+		of_property_read_string(child, "data-role", &role_type);
+		if (role_type && (!strncmp(role_type, "dual",

strlen("dual"))))

quoted

+			ret = true;
+	}
+
+	of_node_put(child);
+	return ret;
+}
+
 static int renesas_usb3_probe(struct platform_device *pdev)  {
 	struct renesas_usb3 *usb3;

@@ -2741,6 +2842,12 @@ static int renesas_usb3_probe(struct

platform_device *pdev)

quoted

 	if (ret < 0)
 		goto err_dev_create;

+	if (device_property_read_bool(&pdev->dev, "usb-role-switch") &&
+	    is_usb_dual_role_switch(&pdev->dev)) {
+		usb3->dual_role_sw = true;
+		renesas_usb3_role_switch_desc.fwnode =

dev_fwnode(&pdev->dev);

quoted

+	}
+
 	INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
 	usb3->role_sw = usb_role_switch_register(&pdev->dev,
 					&renesas_usb3_role_switch_desc);

Hi Biju-san,

Thank you for the patch!

From: Biju Das, Sent: Wednesday, May 15, 2019 9:09 PM
Subject: [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role switch support

Now I'm confusing about the "Add dual role switch support" mean... Especially,
this driver has already supports dual role switch support by own sysfs or debugfs.

The RZ/G2E cat874 board has a type-c connector connected to hd3ss3220 usb
type-c drp port controller. This patch adds dual role switch support for
the type-c connector using the usb role switch class framework.

IIUC, after this patch is applied, the hs3ss3220 type-c driver can switch
the role by using the usb role switch class framework.

I'm also confusing this name.

 };

 #define gadget_to_renesas_usb3(_gadget)	\

<snip>

I think either one of the conditions is enough. (Only "usb-role-switch"
checking is enough, IIUC).

JFYI, according to the binding document [1], this "usb-role-switch" means:
---
+ - usb-role-switch: boolean, indicates that the device is capable of assigning
+			the USB data role (USB host or USB device) for a given
+			USB connector, such as Type-C, Type-B(micro).
+			see connector/usb-connector.txt.
---

So, R-Car Gen3 / Salvator-XS cannot have this property because the board
has Type-A connector.

[1] https://patchwork.kernel.org/patch/10934835/

+		usb3->dual_role_sw = true;

So, "role_sw_by_connector" matches with my image.
What do you think?

Best regards,
Yoshihiro Shimoda

+		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

Hi Shimoda-San,

Thanks for the feedback.

quoted

From: Biju Das, Sent: Wednesday, May 15, 2019 9:09 PM
Subject: [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role
switch support

Now I'm confusing about the "Add dual role switch support" mean...
Especially, this driver has already supports dual role switch support by own
sysfs or debugfs.

Sorry for the confusion. 
What about "Enhance role switch support" ?

quoted

The RZ/G2E cat874 board has a type-c connector connected to hd3ss3220
usb type-c drp port controller. This patch adds dual role switch
support for the type-c connector using the usb role switch class framework.

IIUC, after this patch is applied, the hs3ss3220 type-c driver can switch the
role by using the usb role switch class framework.

Yes, That is correct. HD3SS3220 driver detects host/device  connection events (attach/detach) and 
It calls "usb_role_switch_set_role"  to assign/switch the role.

quoted

Signed-off-by: Biju Das <redacted>
---
 V5-->V6
   * Added graph api's to find the role supported by the connector.
 V4-->V5
   * Incorporated Shimoda-san's review comment
    (https://patchwork.kernel.org/patch/10902537/)
 V3-->V4
   * No Change
 V2-->V3
   * Incorporated Shimoda-san's review comment
     (https://patchwork.kernel.org/patch/10852507/)
   * Used renesas,usb-role-switch property for differentiating USB
     role switch associated with Type-C port controller driver.
 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 | 121
++++++++++++++++++++++++++++++++--
 1 file changed, 114 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c

b/drivers/usb/gadget/udc/renesas_usb3.c
index 7dc2485..1d41998 100644

--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c

@@ -24,6 +24,7 @@
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/of.h>
+#include <linux/of_graph.h>
 #include <linux/usb/role.h>

 /* register definitions */

@@ -351,6 +352,8 @@ struct renesas_usb3 {
 	int disabled_count;

 	struct usb_request *ep0_req;
+
+	enum usb_role connection_state;
 	u16 test_mode;
 	u8 ep0_buf[USB3_EP0_BUF_SIZE];
 	bool softconnect;

@@ -359,6 +362,7 @@ struct renesas_usb3 {
 	bool extcon_usb;		/* check vbus and set EXTCON_USB

*/

quoted

 	bool forced_b_device;
 	bool start_to_connect;
+	bool dual_role_sw;

I'm also confusing this name.

OK. will change it to "role_sw_by_connector"

quoted

 };

 #define gadget_to_renesas_usb3(_gadget)	\

<snip>

quoted

@@ -699,8 +703,10 @@ static void usb3_mode_config(struct renesas_usb3

*usb3, bool host, bool a_dev)

quoted

 	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->dual_role_sw || usb3->connection_state !=

USB_ROLE_NONE) {

quoted

+		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);

@@ -716,7 +722,8 @@ static void usb3_check_id(struct renesas_usb3

*usb3)  {
 	usb3->extcon_host = usb3_is_a_device(usb3);

-	if (usb3->extcon_host && !usb3->forced_b_device)
+	if ((!usb3->dual_role_sw && usb3->extcon_host && !usb3-
forced_b_device)
+	    || usb3->connection_state == USB_ROLE_HOST)
 		usb3_mode_config(usb3, true, true);
 	else
 		usb3_mode_config(usb3, false, false); @@ -2343,14 +2350,65

@@

quoted

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->connection_state = USB_ROLE_HOST;
+			usb3_set_mode(usb3, true);
+			usb3_vbus_out(usb3, true);
+			if (device_attach(host) < 0)
+				dev_err(dev, "device_attach(host)

failed\n");

quoted

+		} else if (cur_role == USB_ROLE_DEVICE) {
+			usb3_disconnect(usb3);
+			/* Must set the mode before device_attach of the

host */

quoted

+			usb3_set_mode(usb3, true);
+			/* This device_attach() might sleep */
+			if (device_attach(host) < 0)
+				dev_err(dev, "device_attach(host)

failed\n");

quoted

+		}
+		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 +2419,20 @@ static int renesas_usb3_role_switch_set(struct

device *dev,

quoted

 		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->dual_role_sw)
+		handle_ext_role_switch_states(dev, role);
+	else
+		handle_role_switch_states(dev, role);
+
 	pm_runtime_put(dev);

 	return 0;

@@ -2650,12 +2722,41 @@ static const unsigned int renesas_usb3_cable[]

= {

quoted

 	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,
 };

+static bool is_usb_dual_role_switch(struct device *dev) {
+	struct device_node *np = dev->of_node;
+	struct device_node *parent;
+	struct device_node *child;
+	bool ret = false;
+	const char *role_type = NULL;
+
+	child = of_graph_get_endpoint_by_regs(np, -1, -1);
+	if (!child)
+		return ret;
+
+	parent = of_graph_get_remote_port_parent(child);
+	of_node_put(child);
+	child = of_get_child_by_name(parent, "connector");
+	of_node_put(parent);
+	if (!child)
+		return ret;
+
+	if (of_device_is_compatible(child, "usb-c-connector")) {
+		of_property_read_string(child, "data-role", &role_type);
+		if (role_type && (!strncmp(role_type, "dual",

strlen("dual"))))

quoted

+			ret = true;
+	}
+
+	of_node_put(child);
+	return ret;
+}
+
 static int renesas_usb3_probe(struct platform_device *pdev)  {
 	struct renesas_usb3 *usb3;

@@ -2741,6 +2842,12 @@ static int renesas_usb3_probe(struct

platform_device *pdev)

quoted

 	if (ret < 0)
 		goto err_dev_create;

+	if (device_property_read_bool(&pdev->dev, "usb-role-switch") &&
+	    is_usb_dual_role_switch(&pdev->dev)) {

I think either one of the conditions is enough. (Only "usb-role-switch"
checking is enough, IIUC).

OK, Will remove the other check.

JFYI, according to the binding document [1], this "usb-role-switch" means:
---
+ - usb-role-switch: boolean, indicates that the device is capable of assigning
+			the USB data role (USB host or USB device) for a
given
+			USB connector, such as Type-C, Type-B(micro).
+			see connector/usb-connector.txt.
---

So, R-Car Gen3 / Salvator-XS cannot have this property because the board
has Type-A connector.

[1] https://patchwork.kernel.org/patch/10934835/

I have updated the  binding patch to cover this[1]
[1]. https://patchwork.kernel.org/patch/10944631/
Are you ok with this binding patch?

quoted

+		usb3->dual_role_sw = true;

So, "role_sw_by_connector" matches with my image.
What do you think?

OK, will use " role_sw_by_connector"

Regards,
Biju
 

quoted

+		renesas_usb3_role_switch_desc.fwnode =

dev_fwnode(&pdev->dev);

quoted

+	}
+
 	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

Hi Biju-san,

From: Biju Das, Sent: Tuesday, May 21, 2019 4:10 PM

Hi Shimoda-San,

Thanks for the feedback.

quoted

quoted

From: Biju Das, Sent: Wednesday, May 15, 2019 9:09 PM
Subject: [PATCH v6 4/7] usb: gadget: udc: renesas_usb3: Add dual role
switch support

Now I'm confusing about the "Add dual role switch support" mean...
Especially, this driver has already supports dual role switch support by own
sysfs or debugfs.

Sorry for the confusion.
What about "Enhance role switch support" ?

Thank you for the suggestion. It's good to me.

quoted

quoted

The RZ/G2E cat874 board has a type-c connector connected to hd3ss3220
usb type-c drp port controller. This patch adds dual role switch
support for the type-c connector using the usb role switch class framework.

IIUC, after this patch is applied, the hs3ss3220 type-c driver can switch the
role by using the usb role switch class framework.

Yes, That is correct. HD3SS3220 driver detects host/device  connection events (attach/detach) and
It calls "usb_role_switch_set_role"  to assign/switch the role.

I got it.

<snip>

quoted

JFYI, according to the binding document [1], this "usb-role-switch" means:
---
+ - usb-role-switch: boolean, indicates that the device is capable of assigning
+			the USB data role (USB host or USB device) for a
given
+			USB connector, such as Type-C, Type-B(micro).
+			see connector/usb-connector.txt.
---

So, R-Car Gen3 / Salvator-XS cannot have this property because the board
has Type-A connector.

[1] https://patchwork.kernel.org/patch/10934835/

I have updated the  binding patch to cover this[1]
[1]. https://patchwork.kernel.org/patch/10944631/
Are you ok with this binding patch?

I'm sorry, I overlooked the patch. I'll check it later.

Best regards,
Yoshihiro Shimoda

Hi Biju-san,

Thank you for the patch!

s/connector :/connector:/

According to the connector/usb-connector.txt, should the connector node
have ports, port@1 and an endpoint nodes like below?

	usb_con: connector {
		compatible = "usb-c-connector";
		label = "USB-C";
		data-role = "dual";

		ports {
			#address-cells = <1>;
			#size-cells = <0>;

			port@1 {
				reg = <1>;
				hd3ss3220_ep: endpoint {
					remote-endpoint = <&usb3peri_role_switch>;
				};
			};
		};
	};

Best regards,
Yoshihiro Shimoda

Hi

quoted

+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

s/connector :/connector:/

Maybe it is for alignment ?

According to the connector/usb-connector.txt, should the connector node
have ports, port@1 and an endpoint nodes like below?

"ports" is needed if it has multiple "port",
otherwise, single port is allowed from OF-graph point of view.

Thank you for your help !!
Best regards
---
Kuninori Morimoto

Hi Biju-san,

Thank you for the patch!

I think we should describe which type of a subnode is needed.
I made an example below. This is based on the usb-connector.txt.
---
Sub-nodes:
- any connector to the data bus of this controller should be modeled using the OF graph
  bindings specified in bindings/graph.txt.
---

I'm not sure, but I don't think this endpoint@0 and reg = <0> are needed.
In other words, can we use have following node?
			usb3peri_role_switch: endpoint {
				remote-endpoint = <&hd3ss3220_ep>;
			};

Best regards,
Yoshihiro Shimoda

Hi Sergei,

Thanks for the feedback.

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

Hello!

On 05/15/2019 03:09 PM, Biju Das wrote:

quoted

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

Signed-off-by: Biju Das <redacted>
Reviewed-by: Rob Herring <robh@kernel.org>
---
V5-->V6
  * No change.
V4-->V5
  * No Change.
V3-->V4
  * No Change.
V2-->V3
  * Added Rob's Reviewed by tag.
V1-->V2
  * Added connector node.
  * updated the example with connector node.
---
 .../devicetree/bindings/usb/ti,hd3ss3220.txt       | 37

++++++++++++++++++++++

quoted

 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.

   This depends on an interrupt controller used. I'd just said "an interrupt
specifier", w/o further details.

Fine , If it is ok for everyone. 

Regards,
Biju

quoted

+
+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
+

[...]

MBR, Sergei

Hi Morimoto-San and Shimoda-San,

Thanks for the feedback.

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


Hi

quoted

quoted

+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

s/connector :/connector:/

Maybe it is for alignment ?

Yes, I need to fix the extra space.

quoted

According to the connector/usb-connector.txt, should the connector
node have ports, port@1 and an endpoint nodes like below?

"ports" is needed if it has multiple "port", otherwise, single port is allowed
from OF-graph point of view.

OK. I will use single port on  the next patch series.

Regards,
Biju

Hi Biju-san, Morimoto-san,

From: Biju Das, Sent: Wednesday, May 22, 2019 8:00 PM

<snip>

quoted

quoted

According to the connector/usb-connector.txt, should the connector
node have ports, port@1 and an endpoint nodes like below?

"ports" is needed if it has multiple "port", otherwise, single port is allowed
from OF-graph point of view.

OK. I will use single port on  the next patch series.

According to the connector/usb-connector.txt [1], even if this device uses a single port,
we should describe ports node and port@1 (for SuperSpeed) subnode like usb/typec-tcpci.txt.

[1]
Required nodes:
- any data bus to the connector should be modeled using the OF graph bindings
  specified in bindings/graph.txt, unless the bus is between parent node and
  the connector. Since single connector can have multiple data buses every bus
  has assigned OF graph port number as follows:
    0: High Speed (HS), present in all connectors,
    1: Super Speed (SS), present in SS capable connectors,
    2: Sideband use (SBU), present in USB-C.

Best regards,
Yoshihiro Shimoda

Hi Shimoda-San,

Thanks for the feedback.

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

Hi Biju-san, Morimoto-san,

quoted

From: Biju Das, Sent: Wednesday, May 22, 2019 8:00 PM

<snip>

quoted

quoted

quoted

According to the connector/usb-connector.txt, should the connector
node have ports, port@1 and an endpoint nodes like below?

"ports" is needed if it has multiple "port", otherwise, single port
is allowed from OF-graph point of view.

OK. I will use single port on  the next patch series.

According to the connector/usb-connector.txt [1], even if this device uses a
single port, we should describe ports node and port@1 (for SuperSpeed)
subnode like usb/typec-tcpci.txt.

OK.  I will update the example like below.

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";

		ports {
			#address-cells = <1>;
			#size-cells = <0>;

			port@1 {
				reg = <1>;
				hd3ss3220_ep: endpoint {
					remote-endpoint = <&usb3_role_switch>;
				};
			};
		};
	};
};

Regards,
Biju

[1]
Required nodes:
- any data bus to the connector should be modeled using the OF graph
bindings
  specified in bindings/graph.txt, unless the bus is between parent node and
  the connector. Since single connector can have multiple data buses every
bus
  has assigned OF graph port number as follows:
    0: High Speed (HS), present in all connectors,
    1: Super Speed (SS), present in SS capable connectors,
    2: Sideband use (SBU), present in USB-C.

HI Shimoda-San,

Thanks for the feedback.

Subject: RE: [PATCH v6 2/7] dt-bindings: usb: renesas_usb3: Document usb
role switch support

Hi Biju-san,

Thank you for the patch!

quoted

From: Biju Das, Sent: Wednesday, May 15, 2019 9:09 PM

Update the DT bindings documentation to support usb role switch for
USB Type-C connector using USB role switch class framework.

Signed-off-by: Biju Das <redacted>
---
 V5-->V6
  * Updated description
  * Added usb-role-switch-property
 V4-->V5
  * No Change
 V3-->V4
  * No Change
 V2-->V3
  * Added optional renesas,usb-role-switch property.
 V1-->V2
  * Added usb-role-switch-property
  * Updated the example with usb-role-switch property.
---
 .../devicetree/bindings/usb/renesas_usb3.txt       | 26

++++++++++++++++++++++

quoted

 1 file changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt

b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
index 35039e7..ea6c63c 100644

--- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt

@@ -22,6 +22,11 @@ Required properties:
 Optional properties:
   - phys: phandle + phy specifier pair
   - phy-names: must be "usb"
+  - usb-role-switch: support role switch. see usb/generic.txt
+
+Sub-nodes:
+The port would be added as a subnode if the "usb-role-switch" property is

used.

quoted

+	see graph.txt

I think we should describe which type of a subnode is needed.
I made an example below. This is based on the usb-connector.txt.
---
Sub-nodes:
- any connector to the data bus of this controller should be modeled using
the OF graph
bindings specified in bindings/graph.txt

OK to me. I will add the following  at the end.  ", if the "usb-role-switch" property is
used."

---

quoted

 Example of R-Car H3 ES1.x:
 	usb3_peri0: usb@ee020000 {

@@ -39,3 +44,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>;

I'm not sure, but I don't think this endpoint@0 and reg = <0> are needed.
In other words, can we use have following node?
			usb3peri_role_switch: endpoint {
				remote-endpoint = <&hd3ss3220_ep>;
			};

OK . Will update  the example with

        port {                                                                   
                usb3_role_switch: endpoint {                                     
                        remote-endpoint = <&hd3ss3220_ep>;                       
                 };                                                               
         };

Regards,
Biju