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

[PATCH 0/9] 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 usb_next and renesas-devel-20190304-v5.0 branch.

Biju Das (9):
  dt-bindings: usb: hd3ss3220 device tree binding document
  dt-bindings: usb: renesas_usb3: add extcon support
  usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller
  usb: gadget: udc: renesas_usb3: use extcon framework to receive
    connect/disconnect
  arm64: defconfig: enable TYPEC_HD3SS3220 config option
  arm64: renesas_defconfig: Enable TYPEC_HD3SS3220 config option
  arm64: dts: renesas: r8a774c0-cat874: enable USB3.0 host/peripheral
    device node
  arm64: dts: renesas: r8a774c0-cat874: Enable TI HD3SS3220 support
  arm64: dts: renesas: r8a774c0-cat874: Enable extcon support

 .../devicetree/bindings/usb/renesas_usb3.txt       |   2 +
 .../devicetree/bindings/usb/ti,hd3ss3220.txt       |  15 ++
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts    |  31 +++
 arch/arm64/configs/defconfig                       |   2 +
 arch/arm64/configs/renesas_defconfig               |   2 +
 drivers/usb/gadget/udc/renesas_usb3.c              | 115 +++++++--
 drivers/usb/typec/Kconfig                          |  10 +
 drivers/usb/typec/Makefile                         |   1 +
 drivers/usb/typec/hd3ss3220.c                      | 284 +++++++++++++++++++++
 9 files changed, 446 insertions(+), 16 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
 create mode 100644 drivers/usb/typec/hd3ss3220.c

-- 
2.7.4

[PATCH 1/9] 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>
---
 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt | 15 +++++++++++++++
 1 file changed, 15 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..1fcf6f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
@@ -0,0 +1,15 @@
+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.
+
+Example:
+hd3ss3220@47 {
+	compatible = "ti,hd3ss3220";
+	reg = <0x47>;
+	interrupt-parent = <&gpio6>;
+	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+};
-- 
2.7.4

[1/9] 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>
---
 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt | 15 +++++++++++++++
 1 file changed, 15 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..1fcf6f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
@@ -0,0 +1,15 @@
+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.
+
+Example:
+hd3ss3220@47 {
+	compatible = "ti,hd3ss3220";
+	reg = <0x47>;
+	interrupt-parent = <&gpio6>;
+	interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+};

[PATCH 2/9] dt-bindings: usb: renesas_usb3: add extcon support

[Biju Das]

Add support for renesas_usb3 to receive connect and disconnect notification
using extcon framework.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
 Documentation/devicetree/bindings/usb/renesas_usb3.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
index d366555..ae35674 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
@@ -21,6 +21,8 @@ Required properties:
 Optional properties:
   - phys: phandle + phy specifier pair
   - phy-names: must be "usb"
+  - extcon: phandle for the extcon device renesas usb3 uses to detect
+	    connect/disconnect events.
 
 Example of R-Car H3 ES1.x:
 	usb3_peri0: usb@ee020000 {
-- 
2.7.4

[2/9] dt-bindings: usb: renesas_usb3: add extcon support

[Biju Das]

Add support for renesas_usb3 to receive connect and disconnect notification
using extcon framework.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
 Documentation/devicetree/bindings/usb/renesas_usb3.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
index d366555..ae35674 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
@@ -21,6 +21,8 @@ Required properties:
 Optional properties:
   - phys: phandle + phy specifier pair
   - phy-names: must be "usb"
+  - extcon: phandle for the extcon device renesas usb3 uses to detect
+	    connect/disconnect events.
 
 Example of R-Car H3 ES1.x:
 	usb3_peri0: usb@ee020000 {

[PATCH 3/9] 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>
---
 drivers/usb/typec/Kconfig     |  10 ++
 drivers/usb/typec/Makefile    |   1 +
 drivers/usb/typec/hd3ss3220.c | 284 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 295 insertions(+)
 create mode 100644 drivers/usb/typec/hd3ss3220.c

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 30a847c..91696d2 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -49,6 +49,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..bd3e1ec
--- /dev/null
+++ b/drivers/usb/typec/hd3ss3220.c
@@ -0,0 +1,284 @@
+// 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/extcon-provider.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 extcon_dev *extcon;
+	struct typec_port *port;
+	struct typec_capability typec_cap;
+};
+
+static const unsigned int hd3ss3220_cable[] = {
+	EXTCON_USB,
+	EXTCON_USB_HOST,
+	EXTCON_NONE
+};
+
+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 int hd3ss3220_attached_state_detect(struct hd3ss3220 *hd3ss3220)
+{
+	unsigned int reg_val;
+	int ret, attached_state;
+
+	ret = regmap_read(hd3ss3220->regmap,
+				HD3SS3220_REG_CN_STAT_CTRL, &reg_val);
+	if (ret < 0)
+		return ret;
+
+	reg_val &= HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK;
+	switch (reg_val) {
+	case HD3SS3220_REG_CN_STAT_CTRL_AS_DFP:
+		attached_state = EXTCON_USB_HOST;
+	break;
+	case HD3SS3220_REG_CN_STAT_CTRL_AS_UFP:
+		attached_state = EXTCON_USB;
+	break;
+	default:
+		attached_state = EXTCON_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);
+	int ret = 0;
+
+	if (role == TYPEC_HOST) {
+		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, false);
+		ret = hd3ss3220_set_source_pref(hd3ss3220,
+				HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC);
+		mdelay(100);
+		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB_HOST, true);
+	} else {
+		extcon_set_state_sync(hd3ss3220->extcon,
+						EXTCON_USB_HOST, false);
+		ret = hd3ss3220_set_source_pref(hd3ss3220,
+				HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK);
+		mdelay(100);
+		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, true);
+	}
+
+	typec_set_data_role(hd3ss3220->port, role);
+
+	return ret;
+}
+
+static void hd3ss3220_set_extcon_state(struct hd3ss3220 *hd3ss3220)
+{
+	int attached_state = hd3ss3220_attached_state_detect(hd3ss3220);
+
+	if (attached_state == EXTCON_USB_HOST) {
+		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, false);
+		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB_HOST, true);
+		typec_set_data_role(hd3ss3220->port, TYPEC_HOST);
+	} else if (attached_state == EXTCON_USB) {
+		extcon_set_state_sync(hd3ss3220->extcon,
+						 EXTCON_USB_HOST, false);
+		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, true);
+		typec_set_data_role(hd3ss3220->port, TYPEC_DEVICE);
+	} else {
+		hd3ss3220_set_source_pref(hd3ss3220,
+				HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
+		extcon_set_state_sync(hd3ss3220->extcon,
+						EXTCON_USB_HOST, false);
+		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, false);
+	}
+}
+
+irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220)
+{
+	int err;
+	unsigned int data;
+
+	hd3ss3220_set_extcon_state(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 int hd3ss3220_probe(struct i2c_client *client,
+		const struct i2c_device_id *id)
+{
+	struct hd3ss3220 *hd3ss3220;
+	int ret;
+	unsigned int data;
+	static const struct regmap_config config = {
+		.reg_bits = 8,
+		.val_bits = 8,
+		.max_register = 0x0A,
+	};
+
+	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->extcon = devm_extcon_dev_allocate(&client->dev,
+						hd3ss3220_cable);
+	if (IS_ERR(hd3ss3220->extcon)) {
+		dev_err(&client->dev, "failed to allocate extcon device\n");
+		return PTR_ERR(hd3ss3220->extcon);
+	}
+
+	ret = devm_extcon_dev_register(&client->dev, hd3ss3220->extcon);
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to register extcon device\n");
+		return ret;
+	}
+
+	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_extcon_state(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);
+
+	return ret;
+}
+
+static int hd3ss3220_remove(struct i2c_client *client)
+{
+	struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
+
+	typec_unregister_port(hd3ss3220->port);
+
+	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

[3/9] 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>
---
 drivers/usb/typec/Kconfig     |  10 ++
 drivers/usb/typec/Makefile    |   1 +
 drivers/usb/typec/hd3ss3220.c | 284 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 295 insertions(+)
 create mode 100644 drivers/usb/typec/hd3ss3220.c

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 30a847c..91696d2 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -49,6 +49,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..bd3e1ec
--- /dev/null
+++ b/drivers/usb/typec/hd3ss3220.c
@@ -0,0 +1,284 @@
+// 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/extcon-provider.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 extcon_dev *extcon;
+	struct typec_port *port;
+	struct typec_capability typec_cap;
+};
+
+static const unsigned int hd3ss3220_cable[] = {
+	EXTCON_USB,
+	EXTCON_USB_HOST,
+	EXTCON_NONE
+};
+
+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 int hd3ss3220_attached_state_detect(struct hd3ss3220 *hd3ss3220)
+{
+	unsigned int reg_val;
+	int ret, attached_state;
+
+	ret = regmap_read(hd3ss3220->regmap,
+				HD3SS3220_REG_CN_STAT_CTRL, &reg_val);
+	if (ret < 0)
+		return ret;
+
+	reg_val &= HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK;
+	switch (reg_val) {
+	case HD3SS3220_REG_CN_STAT_CTRL_AS_DFP:
+		attached_state = EXTCON_USB_HOST;
+	break;
+	case HD3SS3220_REG_CN_STAT_CTRL_AS_UFP:
+		attached_state = EXTCON_USB;
+	break;
+	default:
+		attached_state = EXTCON_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);
+	int ret = 0;
+
+	if (role == TYPEC_HOST) {
+		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, false);
+		ret = hd3ss3220_set_source_pref(hd3ss3220,
+				HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC);
+		mdelay(100);
+		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB_HOST, true);
+	} else {
+		extcon_set_state_sync(hd3ss3220->extcon,
+						EXTCON_USB_HOST, false);
+		ret = hd3ss3220_set_source_pref(hd3ss3220,
+				HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK);
+		mdelay(100);
+		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, true);
+	}
+
+	typec_set_data_role(hd3ss3220->port, role);
+
+	return ret;
+}
+
+static void hd3ss3220_set_extcon_state(struct hd3ss3220 *hd3ss3220)
+{
+	int attached_state = hd3ss3220_attached_state_detect(hd3ss3220);
+
+	if (attached_state == EXTCON_USB_HOST) {
+		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, false);
+		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB_HOST, true);
+		typec_set_data_role(hd3ss3220->port, TYPEC_HOST);
+	} else if (attached_state == EXTCON_USB) {
+		extcon_set_state_sync(hd3ss3220->extcon,
+						 EXTCON_USB_HOST, false);
+		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, true);
+		typec_set_data_role(hd3ss3220->port, TYPEC_DEVICE);
+	} else {
+		hd3ss3220_set_source_pref(hd3ss3220,
+				HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
+		extcon_set_state_sync(hd3ss3220->extcon,
+						EXTCON_USB_HOST, false);
+		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, false);
+	}
+}
+
+irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220)
+{
+	int err;
+	unsigned int data;
+
+	hd3ss3220_set_extcon_state(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 int hd3ss3220_probe(struct i2c_client *client,
+		const struct i2c_device_id *id)
+{
+	struct hd3ss3220 *hd3ss3220;
+	int ret;
+	unsigned int data;
+	static const struct regmap_config config = {
+		.reg_bits = 8,
+		.val_bits = 8,
+		.max_register = 0x0A,
+	};
+
+	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->extcon = devm_extcon_dev_allocate(&client->dev,
+						hd3ss3220_cable);
+	if (IS_ERR(hd3ss3220->extcon)) {
+		dev_err(&client->dev, "failed to allocate extcon device\n");
+		return PTR_ERR(hd3ss3220->extcon);
+	}
+
+	ret = devm_extcon_dev_register(&client->dev, hd3ss3220->extcon);
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to register extcon device\n");
+		return ret;
+	}
+
+	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_extcon_state(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);
+
+	return ret;
+}
+
+static int hd3ss3220_remove(struct i2c_client *client)
+{
+	struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
+
+	typec_unregister_port(hd3ss3220->port);
+
+	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 4/9] usb: gadget: udc: renesas_usb3: use extcon framework to receive connect/disconnect

[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
notification using extcon framework.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
 drivers/usb/gadget/udc/renesas_usb3.c | 115 +++++++++++++++++++++++++++++-----
 1 file changed, 99 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
index 7dc2485..2c69d5d 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -351,6 +351,11 @@ struct renesas_usb3 {
 	int disabled_count;
 
 	struct usb_request *ep0_req;
+
+	struct extcon_dev *edev;
+	struct notifier_block ufp_nb;
+	struct notifier_block dfp_nb;
+
 	u16 test_mode;
 	u8 ep0_buf[USB3_EP0_BUF_SIZE];
 	bool softconnect;
@@ -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 =
@@ -724,6 +713,32 @@ 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->edev) {
+			if (extcon_get_state(usb3->edev, EXTCON_USB) == true) {
+				usb3->forced_b_device = true;
+				usb3->start_to_connect = true;
+				usb3_disconnect(usb3);
+				usb3_check_id(usb3);
+			} else if (extcon_get_state(usb3->edev,
+						EXTCON_USB_HOST) == false)
+				usb3_vbus_out(usb3, false);
+		} 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);
@@ -2656,6 +2671,47 @@ static const struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
 	.allow_userspace_control = true,
 };
 
+static int renesas_usb3_ufp_notifier(struct notifier_block *nb,
+					unsigned long event, void *ptr)
+{
+	struct renesas_usb3 *usb3 = container_of(nb,
+					struct renesas_usb3, ufp_nb);
+
+	usb3->start_to_connect = false;
+	if (event && usb3->driver) {
+		usb3->forced_b_device = true;
+		usb3->start_to_connect = true;
+	}
+
+	if (usb3->driver) {
+		usb3_disconnect(usb3);
+		usb3_check_id(usb3);
+	}
+
+	usb3_vbus_out(usb3, false);
+	dev_dbg(usb3_to_dev(usb3), "ufp_notifier event=%ld", event);
+
+	return NOTIFY_DONE;
+}
+
+static int renesas_usb3_dfp_notifier(struct notifier_block *nb,
+					unsigned long event, void *ptr)
+{
+	struct renesas_usb3 *usb3 = container_of(nb,
+					struct renesas_usb3, dfp_nb);
+
+	if (event) {
+		usb3->forced_b_device = false;
+		usb3_disconnect(usb3);
+		usb3_check_id(usb3);
+	} else
+		usb3_vbus_out(usb3, false);
+
+	dev_dbg(usb3_to_dev(usb3), "dfp_notifier event=%ld", event);
+
+	return NOTIFY_DONE;
+}
+
 static int renesas_usb3_probe(struct platform_device *pdev)
 {
 	struct renesas_usb3 *usb3;
@@ -2703,6 +2759,33 @@ static int renesas_usb3_probe(struct platform_device *pdev)
 		return ret;
 
 	INIT_WORK(&usb3->extcon_work, renesas_usb3_extcon_work);
+
+	if (priv->workaround_for_vbus &&
+			of_property_read_bool(pdev->dev.of_node, "extcon")) {
+		usb3->edev = extcon_get_edev_by_phandle(&pdev->dev, 0);
+		if (IS_ERR(usb3->edev))
+			return PTR_ERR(usb3->edev);
+
+		usb3->ufp_nb.notifier_call = renesas_usb3_ufp_notifier;
+		ret = devm_extcon_register_notifier(&pdev->dev, usb3->edev,
+						EXTCON_USB, &usb3->ufp_nb);
+		if (ret < 0)
+			dev_dbg(&pdev->dev, "USB register notifier failed\n");
+
+		usb3->dfp_nb.notifier_call = renesas_usb3_dfp_notifier;
+		ret = devm_extcon_register_notifier(&pdev->dev, usb3->edev,
+						EXTCON_USB_HOST, &usb3->dfp_nb);
+		if (ret < 0)
+			dev_dbg(&pdev->dev,
+					"USB-HOST register notifier failed\n");
+		if (extcon_get_state(usb3->edev, EXTCON_USB) == true) {
+			usb3->forced_b_device = true;
+			usb3->start_to_connect = true;
+		} else	if (extcon_get_state(usb3->edev,
+						 EXTCON_USB_HOST) == false)
+			usb3_vbus_out(usb3, false);
+	}
+
 	usb3->extcon = devm_extcon_dev_allocate(&pdev->dev, renesas_usb3_cable);
 	if (IS_ERR(usb3->extcon))
 		return PTR_ERR(usb3->extcon);
-- 
2.7.4

[4/9] usb: gadget: udc: renesas_usb3: use extcon framework to receive connect/disconnect

[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
notification using extcon framework.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
 drivers/usb/gadget/udc/renesas_usb3.c | 115 +++++++++++++++++++++++++++++-----
 1 file changed, 99 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
index 7dc2485..2c69d5d 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -351,6 +351,11 @@ struct renesas_usb3 {
 	int disabled_count;
 
 	struct usb_request *ep0_req;
+
+	struct extcon_dev *edev;
+	struct notifier_block ufp_nb;
+	struct notifier_block dfp_nb;
+
 	u16 test_mode;
 	u8 ep0_buf[USB3_EP0_BUF_SIZE];
 	bool softconnect;
@@ -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 =
@@ -724,6 +713,32 @@ 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->edev) {
+			if (extcon_get_state(usb3->edev, EXTCON_USB) == true) {
+				usb3->forced_b_device = true;
+				usb3->start_to_connect = true;
+				usb3_disconnect(usb3);
+				usb3_check_id(usb3);
+			} else if (extcon_get_state(usb3->edev,
+						EXTCON_USB_HOST) == false)
+				usb3_vbus_out(usb3, false);
+		} 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);
@@ -2656,6 +2671,47 @@ static const struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
 	.allow_userspace_control = true,
 };
 
+static int renesas_usb3_ufp_notifier(struct notifier_block *nb,
+					unsigned long event, void *ptr)
+{
+	struct renesas_usb3 *usb3 = container_of(nb,
+					struct renesas_usb3, ufp_nb);
+
+	usb3->start_to_connect = false;
+	if (event && usb3->driver) {
+		usb3->forced_b_device = true;
+		usb3->start_to_connect = true;
+	}
+
+	if (usb3->driver) {
+		usb3_disconnect(usb3);
+		usb3_check_id(usb3);
+	}
+
+	usb3_vbus_out(usb3, false);
+	dev_dbg(usb3_to_dev(usb3), "ufp_notifier event=%ld", event);
+
+	return NOTIFY_DONE;
+}
+
+static int renesas_usb3_dfp_notifier(struct notifier_block *nb,
+					unsigned long event, void *ptr)
+{
+	struct renesas_usb3 *usb3 = container_of(nb,
+					struct renesas_usb3, dfp_nb);
+
+	if (event) {
+		usb3->forced_b_device = false;
+		usb3_disconnect(usb3);
+		usb3_check_id(usb3);
+	} else
+		usb3_vbus_out(usb3, false);
+
+	dev_dbg(usb3_to_dev(usb3), "dfp_notifier event=%ld", event);
+
+	return NOTIFY_DONE;
+}
+
 static int renesas_usb3_probe(struct platform_device *pdev)
 {
 	struct renesas_usb3 *usb3;
@@ -2703,6 +2759,33 @@ static int renesas_usb3_probe(struct platform_device *pdev)
 		return ret;
 
 	INIT_WORK(&usb3->extcon_work, renesas_usb3_extcon_work);
+
+	if (priv->workaround_for_vbus &&
+			of_property_read_bool(pdev->dev.of_node, "extcon")) {
+		usb3->edev = extcon_get_edev_by_phandle(&pdev->dev, 0);
+		if (IS_ERR(usb3->edev))
+			return PTR_ERR(usb3->edev);
+
+		usb3->ufp_nb.notifier_call = renesas_usb3_ufp_notifier;
+		ret = devm_extcon_register_notifier(&pdev->dev, usb3->edev,
+						EXTCON_USB, &usb3->ufp_nb);
+		if (ret < 0)
+			dev_dbg(&pdev->dev, "USB register notifier failed\n");
+
+		usb3->dfp_nb.notifier_call = renesas_usb3_dfp_notifier;
+		ret = devm_extcon_register_notifier(&pdev->dev, usb3->edev,
+						EXTCON_USB_HOST, &usb3->dfp_nb);
+		if (ret < 0)
+			dev_dbg(&pdev->dev,
+					"USB-HOST register notifier failed\n");
+		if (extcon_get_state(usb3->edev, EXTCON_USB) == true) {
+			usb3->forced_b_device = true;
+			usb3->start_to_connect = true;
+		} else	if (extcon_get_state(usb3->edev,
+						 EXTCON_USB_HOST) == false)
+			usb3_vbus_out(usb3, false);
+	}
+
 	usb3->extcon = devm_extcon_dev_allocate(&pdev->dev, renesas_usb3_cable);
 	if (IS_ERR(usb3->extcon))
 		return PTR_ERR(usb3->extcon);

[PATCH 5/9] 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>
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index b263b14..40a085a 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -544,6 +544,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

[5/9] 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>
---
 arch/arm64/configs/defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index b263b14..40a085a 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -544,6 +544,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 6/9] arm64: renesas_defconfig: Enable TYPEC_HD3SS3220 config option

[Biju Das]

Enable support for HD3SS320 USB Type-C DRP Port controller, which is used
by RZ/G2E (r8a774c0) based Silicon Linux CAT874 board.

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

diff --git a/arch/arm64/configs/renesas_defconfig b/arch/arm64/configs/renesas_defconfig
index 5561d30..022a875 100644
--- a/arch/arm64/configs/renesas_defconfig
+++ b/arch/arm64/configs/renesas_defconfig
@@ -255,6 +255,8 @@ CONFIG_USB_RENESAS_USBHS_UDC=y
 CONFIG_USB_RENESAS_USB3=y
 CONFIG_USB_SNP_UDC_PLAT=y
 CONFIG_USB_BDC_UDC=y
+CONFIG_TYPEC=y
+CONFIG_TYPEC_HD3SS3220=y
 CONFIG_MMC=y
 CONFIG_MMC_SDHI=y
 CONFIG_NEW_LEDS=y
-- 
2.7.4

[PATCH 7/9] 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>
---
This patch depends on commit 35ed6229c0f0d079f2
("usb: gadget: udc: renesas_usb3: Add bindings for r8a774c0")
---
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
index 959919a..73b51e2 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";
+	};
 };
 
 &scif2 {
@@ -146,3 +151,16 @@
 	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 8/9] arm64: dts: renesas: r8a774c0-cat874: Enable TI HD3SS3220 support

[Biju Das]

This patch enables TI HD3SS3220 device on r8a774c0 cat874 board.

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

diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
index 73b51e2..2a4631d 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
@@ -80,6 +80,18 @@
 	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>;
+	};
+};
+
 &i2c1 {
 	pinctrl-0 = <&i2c1_pins>;
 	pinctrl-names = "default";
-- 
2.7.4

[PATCH 9/9] arm64: dts: renesas: r8a774c0-cat874: Enable extcon support

[Biju Das]

This patch enables extcon framework support to receive connect and
disconnect notification.

Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
 arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
index 2a4631d..1a78254 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
@@ -84,7 +84,7 @@
 	status = "okay";
 	clock-frequency = <100000>;
 
-	hd3ss3220@47 {
+	hd3ss3220: hd3ss3220@47 {
 		compatible = "ti,hd3ss3220";
 		reg = <0x47>;
 		interrupt-parent = <&gpio6>;
@@ -166,6 +166,7 @@
 
 &usb3_peri0 {
 	companion = <&xhci0>;
+	extcon = <&hd3ss3220>;
 
 	status = "okay";
 };
-- 
2.7.4

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

[Heikki Krogerus]

Hi Biju,

On Wed, Mar 06, 2019 at 09:07:20AM +0000, Biju Das wrote:
> 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>
> ---
>  drivers/usb/typec/Kconfig     |  10 ++
>  drivers/usb/typec/Makefile    |   1 +
>  drivers/usb/typec/hd3ss3220.c | 284 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 295 insertions(+)
>  create mode 100644 drivers/usb/typec/hd3ss3220.c
> 
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> index 30a847c..91696d2 100644
> --- a/drivers/usb/typec/Kconfig
> +++ b/drivers/usb/typec/Kconfig
> @@ -49,6 +49,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..bd3e1ec
> --- /dev/null
> +++ b/drivers/usb/typec/hd3ss3220.c
> @@ -0,0 +1,284 @@
> +// 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/extcon-provider.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 extcon_dev *extcon;
> +	struct typec_port *port;
> +	struct typec_capability typec_cap;
> +};
> +
> +static const unsigned int hd3ss3220_cable[] = {
> +	EXTCON_USB,
> +	EXTCON_USB_HOST,
> +	EXTCON_NONE
> +};

You need to use the USB role class instead of extcon. Check
drivers/usb/roles/class/ and include/linux/usb/role.h

You may also want to check the updates Yu Chen is introducing to that
class:
https://lkml.org/lkml/2019/3/2/42

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

Please fix the alignment:

        return regmap_write(hd3ss3220->regmap, HD3SS3220_REG_GEN_CTRL, reg_val);

> +}
> +
> +static int hd3ss3220_attached_state_detect(struct hd3ss3220 *hd3ss3220)
> +{
> +	unsigned int reg_val;
> +	int ret, attached_state;
> +
> +	ret = regmap_read(hd3ss3220->regmap,
> +				HD3SS3220_REG_CN_STAT_CTRL, &reg_val);

ditto:

	ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
                          &reg_val);

> +	if (ret < 0)
> +		return ret;
> +
> +	reg_val &= HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK;
> +	switch (reg_val) {

        switch (reg_val & HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK)

> +	case HD3SS3220_REG_CN_STAT_CTRL_AS_DFP:
> +		attached_state = EXTCON_USB_HOST;
> +	break;
> +	case HD3SS3220_REG_CN_STAT_CTRL_AS_UFP:
> +		attached_state = EXTCON_USB;
> +	break;
> +	default:
> +		attached_state = EXTCON_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);

I think scripts/checkpatch.pl would find all these alignment issues:

	struct hd3ss3220 *hd3ss3220 = container_of(cap, struct hd3ss3220,
                                                   typec_cap);

There are a lot more of those in this patch. Please fix all of them.

> +	int ret = 0;
> +
> +	if (role == TYPEC_HOST) {
> +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, false);
> +		ret = hd3ss3220_set_source_pref(hd3ss3220,
> +				HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC);
> +		mdelay(100);

Whoa! That's a long delay. A bit too long. You should sleep if you
really need to wait that long, but 100 ms?

> +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB_HOST, true);
> +	} else {
> +		extcon_set_state_sync(hd3ss3220->extcon,
> +						EXTCON_USB_HOST, false);
> +		ret = hd3ss3220_set_source_pref(hd3ss3220,
> +				HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK);
> +		mdelay(100);
> +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, true);
> +	}

I think you could just call hd3ss3220_set_source_pref() ones here.
Just store the value to a variable in those conditions:

        if (role == TYPEC_HOST)
                pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC;
        else
                pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK;

        ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
        ...

> +	typec_set_data_role(hd3ss3220->port, role);
> +
> +	return ret;
> +}
> +
> +static void hd3ss3220_set_extcon_state(struct hd3ss3220 *hd3ss3220)
> +{
> +	int attached_state = hd3ss3220_attached_state_detect(hd3ss3220);
> +
> +	if (attached_state == EXTCON_USB_HOST) {
> +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, false);
> +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB_HOST, true);
> +		typec_set_data_role(hd3ss3220->port, TYPEC_HOST);
> +	} else if (attached_state == EXTCON_USB) {
> +		extcon_set_state_sync(hd3ss3220->extcon,
> +						 EXTCON_USB_HOST, false);
> +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, true);
> +		typec_set_data_role(hd3ss3220->port, TYPEC_DEVICE);
> +	} else {
> +		hd3ss3220_set_source_pref(hd3ss3220,
> +				HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
> +		extcon_set_state_sync(hd3ss3220->extcon,
> +						EXTCON_USB_HOST, false);
> +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, false);
> +	}
> +}
> +
> +irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220)
> +{
> +	int err;
> +	unsigned int data;
> +
> +	hd3ss3220_set_extcon_state(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 int hd3ss3220_probe(struct i2c_client *client,
> +		const struct i2c_device_id *id)
> +{
> +	struct hd3ss3220 *hd3ss3220;
> +	int ret;
> +	unsigned int data;
> +	static const struct regmap_config config = {
> +		.reg_bits = 8,
> +		.val_bits = 8,
> +		.max_register = 0x0A,
> +	};

Move that outside of the function.

> +	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->extcon = devm_extcon_dev_allocate(&client->dev,
> +						hd3ss3220_cable);
> +	if (IS_ERR(hd3ss3220->extcon)) {
> +		dev_err(&client->dev, "failed to allocate extcon device\n");
> +		return PTR_ERR(hd3ss3220->extcon);
> +	}
> +
> +	ret = devm_extcon_dev_register(&client->dev, hd3ss3220->extcon);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "failed to register extcon device\n");
> +		return ret;
> +	}
> +
> +	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_extcon_state(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);
> +
> +	return ret;
> +}
> +
> +static int hd3ss3220_remove(struct i2c_client *client)
> +{
> +	struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
> +
> +	typec_unregister_port(hd3ss3220->port);
> +
> +	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");

You will need to use the USB role class instead of extcon. Otherwise
this looks OK to me, assuming you fix the style issues ;-).


thanks,

-- 
heikki

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

[Heikki Krogerus]

Hi Biju,

On Wed, Mar 06, 2019 at 09:07:20AM +0000, Biju Das wrote:
> 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>
> ---
>  drivers/usb/typec/Kconfig     |  10 ++
>  drivers/usb/typec/Makefile    |   1 +
>  drivers/usb/typec/hd3ss3220.c | 284 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 295 insertions(+)
>  create mode 100644 drivers/usb/typec/hd3ss3220.c
> 
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> index 30a847c..91696d2 100644
> --- a/drivers/usb/typec/Kconfig
> +++ b/drivers/usb/typec/Kconfig
> @@ -49,6 +49,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..bd3e1ec
> --- /dev/null
> +++ b/drivers/usb/typec/hd3ss3220.c
> @@ -0,0 +1,284 @@
> +// 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/extcon-provider.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 extcon_dev *extcon;
> +	struct typec_port *port;
> +	struct typec_capability typec_cap;
> +};
> +
> +static const unsigned int hd3ss3220_cable[] = {
> +	EXTCON_USB,
> +	EXTCON_USB_HOST,
> +	EXTCON_NONE
> +};

You need to use the USB role class instead of extcon. Check
drivers/usb/roles/class/ and include/linux/usb/role.h

You may also want to check the updates Yu Chen is introducing to that
class:
https://lkml.org/lkml/2019/3/2/42

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

Please fix the alignment:

        return regmap_write(hd3ss3220->regmap, HD3SS3220_REG_GEN_CTRL, reg_val);

> +}
> +
> +static int hd3ss3220_attached_state_detect(struct hd3ss3220 *hd3ss3220)
> +{
> +	unsigned int reg_val;
> +	int ret, attached_state;
> +
> +	ret = regmap_read(hd3ss3220->regmap,
> +				HD3SS3220_REG_CN_STAT_CTRL, &reg_val);

ditto:

	ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
                          &reg_val);

> +	if (ret < 0)
> +		return ret;
> +
> +	reg_val &= HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK;
> +	switch (reg_val) {

        switch (reg_val & HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK)

> +	case HD3SS3220_REG_CN_STAT_CTRL_AS_DFP:
> +		attached_state = EXTCON_USB_HOST;
> +	break;
> +	case HD3SS3220_REG_CN_STAT_CTRL_AS_UFP:
> +		attached_state = EXTCON_USB;
> +	break;
> +	default:
> +		attached_state = EXTCON_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);

I think scripts/checkpatch.pl would find all these alignment issues:

	struct hd3ss3220 *hd3ss3220 = container_of(cap, struct hd3ss3220,
                                                   typec_cap);

There are a lot more of those in this patch. Please fix all of them.

> +	int ret = 0;
> +
> +	if (role == TYPEC_HOST) {
> +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, false);
> +		ret = hd3ss3220_set_source_pref(hd3ss3220,
> +				HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC);
> +		mdelay(100);

Whoa! That's a long delay. A bit too long. You should sleep if you
really need to wait that long, but 100 ms?

> +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB_HOST, true);
> +	} else {
> +		extcon_set_state_sync(hd3ss3220->extcon,
> +						EXTCON_USB_HOST, false);
> +		ret = hd3ss3220_set_source_pref(hd3ss3220,
> +				HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK);
> +		mdelay(100);
> +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, true);
> +	}

I think you could just call hd3ss3220_set_source_pref() ones here.
Just store the value to a variable in those conditions:

        if (role == TYPEC_HOST)
                pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC;
        else
                pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK;

        ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
        ...

> +	typec_set_data_role(hd3ss3220->port, role);
> +
> +	return ret;
> +}
> +
> +static void hd3ss3220_set_extcon_state(struct hd3ss3220 *hd3ss3220)
> +{
> +	int attached_state = hd3ss3220_attached_state_detect(hd3ss3220);
> +
> +	if (attached_state == EXTCON_USB_HOST) {
> +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, false);
> +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB_HOST, true);
> +		typec_set_data_role(hd3ss3220->port, TYPEC_HOST);
> +	} else if (attached_state == EXTCON_USB) {
> +		extcon_set_state_sync(hd3ss3220->extcon,
> +						 EXTCON_USB_HOST, false);
> +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, true);
> +		typec_set_data_role(hd3ss3220->port, TYPEC_DEVICE);
> +	} else {
> +		hd3ss3220_set_source_pref(hd3ss3220,
> +				HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
> +		extcon_set_state_sync(hd3ss3220->extcon,
> +						EXTCON_USB_HOST, false);
> +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB, false);
> +	}
> +}
> +
> +irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220)
> +{
> +	int err;
> +	unsigned int data;
> +
> +	hd3ss3220_set_extcon_state(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 int hd3ss3220_probe(struct i2c_client *client,
> +		const struct i2c_device_id *id)
> +{
> +	struct hd3ss3220 *hd3ss3220;
> +	int ret;
> +	unsigned int data;
> +	static const struct regmap_config config = {
> +		.reg_bits = 8,
> +		.val_bits = 8,
> +		.max_register = 0x0A,
> +	};

Move that outside of the function.

> +	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->extcon = devm_extcon_dev_allocate(&client->dev,
> +						hd3ss3220_cable);
> +	if (IS_ERR(hd3ss3220->extcon)) {
> +		dev_err(&client->dev, "failed to allocate extcon device\n");
> +		return PTR_ERR(hd3ss3220->extcon);
> +	}
> +
> +	ret = devm_extcon_dev_register(&client->dev, hd3ss3220->extcon);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "failed to register extcon device\n");
> +		return ret;
> +	}
> +
> +	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_extcon_state(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);
> +
> +	return ret;
> +}
> +
> +static int hd3ss3220_remove(struct i2c_client *client)
> +{
> +	struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
> +
> +	typec_unregister_port(hd3ss3220->port);
> +
> +	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");

You will need to use the USB role class instead of extcon. Otherwise
this looks OK to me, assuming you fix the style issues ;-).


thanks,

Re: [PATCH 4/9] usb: gadget: udc: renesas_usb3: use extcon framework to receive connect/disconnect

[Heikki Krogerus]

On Wed, Mar 06, 2019 at 09:07:21AM +0000, Biju Das wrote:
> RZ/G2E cat874 board is capable of detecting cable connect and disconnect
> events. Add support for renesas_usb3 to receive connect and disconnect
> notification using extcon framework.

I think you will need the series from Yu Chen [1], but once we have
that, you can do this with USB role class instead of relying on
extcon, and that is what you need to do.

There is a reason why we need to use a dedicated class with the USB
role switching instead of relying on extcon. We actually originally
planned on using extcon with the USB role switches, but the extcon
maintainers refused some the USB mux drivers. The problem from extcon
perspective is that the consumer/supplier roles seem to be inverted in
some cases. USB role switch can simply be too many things - discrete
mux, integrated mux (like in your case) or something like a dual role
capable USB controller could simply represent one.

On the other hand, using a dedicated API and class now feel like a
much better idea in general compared to a multipurpose thing like
extcon.

[1] https://lkml.org/lkml/2019/3/2/42

thanks,

-- 
heikki

[4/9] usb: gadget: udc: renesas_usb3: use extcon framework to receive connect/disconnect

[Heikki Krogerus]

On Wed, Mar 06, 2019 at 09:07:21AM +0000, Biju Das wrote:
> RZ/G2E cat874 board is capable of detecting cable connect and disconnect
> events. Add support for renesas_usb3 to receive connect and disconnect
> notification using extcon framework.

I think you will need the series from Yu Chen [1], but once we have
that, you can do this with USB role class instead of relying on
extcon, and that is what you need to do.

There is a reason why we need to use a dedicated class with the USB
role switching instead of relying on extcon. We actually originally
planned on using extcon with the USB role switches, but the extcon
maintainers refused some the USB mux drivers. The problem from extcon
perspective is that the consumer/supplier roles seem to be inverted in
some cases. USB role switch can simply be too many things - discrete
mux, integrated mux (like in your case) or something like a dual role
capable USB controller could simply represent one.

On the other hand, using a dedicated API and class now feel like a
much better idea in general compared to a multipurpose thing like
extcon.

[1] https://lkml.org/lkml/2019/3/2/42

thanks,

Re: [PATCH 0/9] Add USB3.0 and TI HD3SS3220 driver support

[Simon Horman]

On Wed, Mar 06, 2019 at 09:07:17AM +0000, Biju Das wrote:
> 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 usb_next and renesas-devel-20190304-v5.0 branch.
> 
> Biju Das (9):
>   dt-bindings: usb: hd3ss3220 device tree binding document
>   dt-bindings: usb: renesas_usb3: add extcon support
>   usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller
>   usb: gadget: udc: renesas_usb3: use extcon framework to receive
>     connect/disconnect

Hi Biju,

I am marking the patches below as deffered pending acceptance of
the dt-bindings patches above.

>   arm64: defconfig: enable TYPEC_HD3SS3220 config option
>   arm64: renesas_defconfig: Enable TYPEC_HD3SS3220 config option
>   arm64: dts: renesas: r8a774c0-cat874: enable USB3.0 host/peripheral
>     device node
>   arm64: dts: renesas: r8a774c0-cat874: Enable TI HD3SS3220 support
>   arm64: dts: renesas: r8a774c0-cat874: Enable extcon support
> 
>  .../devicetree/bindings/usb/renesas_usb3.txt       |   2 +
>  .../devicetree/bindings/usb/ti,hd3ss3220.txt       |  15 ++
>  arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts    |  31 +++
>  arch/arm64/configs/defconfig                       |   2 +
>  arch/arm64/configs/renesas_defconfig               |   2 +
>  drivers/usb/gadget/udc/renesas_usb3.c              | 115 +++++++--
>  drivers/usb/typec/Kconfig                          |  10 +
>  drivers/usb/typec/Makefile                         |   1 +
>  drivers/usb/typec/hd3ss3220.c                      | 284 +++++++++++++++++++++
>  9 files changed, 446 insertions(+), 16 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
>  create mode 100644 drivers/usb/typec/hd3ss3220.c
> 
> -- 
> 2.7.4
> 

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

[Biju Das]

Hi Heikki,

Thanks for the feedback.

> Subject: Re: [PATCH 3/9] usb: typec: driver for TI HD3SS3220 USB Type-C DRP
> port controller
> 
> On Wed, Mar 06, 2019 at 09:07:20AM +0000, Biju Das wrote:
> > 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>
> > ---
> >  drivers/usb/typec/Kconfig     |  10 ++
> >  drivers/usb/typec/Makefile    |   1 +
> >  drivers/usb/typec/hd3ss3220.c | 284
> > ++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 295 insertions(+)
> >  create mode 100644 drivers/usb/typec/hd3ss3220.c
> >
> > diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> > index 30a847c..91696d2 100644
> > --- a/drivers/usb/typec/Kconfig
> > +++ b/drivers/usb/typec/Kconfig
> > @@ -49,6 +49,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..bd3e1ec
> > --- /dev/null
> > +++ b/drivers/usb/typec/hd3ss3220.c
> > @@ -0,0 +1,284 @@
> > +// 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/extcon-provider.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 extcon_dev *extcon;
> > +	struct typec_port *port;
> > +	struct typec_capability typec_cap;
> > +};
> > +
> > +static const unsigned int hd3ss3220_cable[] = {
> > +	EXTCON_USB,
> > +	EXTCON_USB_HOST,
> > +	EXTCON_NONE
> > +};
> 
> You need to use the USB role class instead of extcon. Check
> drivers/usb/roles/class/ and include/linux/usb/role.h
> 
> You may also want to check the updates Yu Chen is introducing to that
> class:
> https://lkml.org/lkml/2019/3/2/42

Ok. Will use USB role class. Basically this driver need to get role_sw handle from
usb3 driver. Based on the  cable attach status/role switch it need to call 
"usb_role_switch_set_role"  function and  driver will get  notified after
setting the role.

> > +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);
> 
> Please fix the alignment:

OK. Will fix it.

>         return regmap_write(hd3ss3220->regmap, HD3SS3220_REG_GEN_CTRL,
> reg_val);
> 
> > +}
> > +
> > +static int hd3ss3220_attached_state_detect(struct hd3ss3220
> > +*hd3ss3220) {
> > +	unsigned int reg_val;
> > +	int ret, attached_state;
> > +
> > +	ret = regmap_read(hd3ss3220->regmap,
> > +				HD3SS3220_REG_CN_STAT_CTRL, &reg_val);
> 
> ditto:
OK. Will fix it.

> 	ret = regmap_read(hd3ss3220->regmap,
> HD3SS3220_REG_CN_STAT_CTRL,
>                           &reg_val);
> 
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	reg_val &=
> HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK;
> > +	switch (reg_val) {
> 
>         switch (reg_val &
> HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK)
> 
> > +	case HD3SS3220_REG_CN_STAT_CTRL_AS_DFP:
> > +		attached_state = EXTCON_USB_HOST;
> > +	break;
> > +	case HD3SS3220_REG_CN_STAT_CTRL_AS_UFP:
> > +		attached_state = EXTCON_USB;
> > +	break;
> > +	default:
> > +		attached_state = EXTCON_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);
> 
> I think scripts/checkpatch.pl would find all these alignment issues:

I have ran check patch and it didn't complained about any alignment issues.
Any way will fix all the alignment issues.

> 	struct hd3ss3220 *hd3ss3220 = container_of(cap, struct hd3ss3220,
>                                                    typec_cap);
> 
> There are a lot more of those in this patch. Please fix all of them.

OK. Will fix.

> > +	int ret = 0;
> > +
> > +	if (role == TYPEC_HOST) {
> > +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB,
> false);
> > +		ret = hd3ss3220_set_source_pref(hd3ss3220,
> > +
> 	HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC);
> > +		mdelay(100);
> 
> Whoa! That's a long delay. A bit too long. You should sleep if you really need
> to wait that long, but 100 ms?

OK. Will check this again and put appropriate delay/sleep.

> > +		extcon_set_state_sync(hd3ss3220->extcon,
> EXTCON_USB_HOST, true);
> > +	} else {
> > +		extcon_set_state_sync(hd3ss3220->extcon,
> > +						EXTCON_USB_HOST, false);
> > +		ret = hd3ss3220_set_source_pref(hd3ss3220,
> > +
> 	HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK);
> > +		mdelay(100);
> > +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB,
> true);
> > +	}
> 
> I think you could just call hd3ss3220_set_source_pref() ones here.
> Just store the value to a variable in those conditions:

OK. Will do.

>         if (role == TYPEC_HOST)
>                 pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC;
>         else
>                 pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK;
> 
>         ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
>         ...
> 
> > +	typec_set_data_role(hd3ss3220->port, role);
> > +
> > +	return ret;
> > +}
> > +
> > +static void hd3ss3220_set_extcon_state(struct hd3ss3220 *hd3ss3220) {
> > +	int attached_state = hd3ss3220_attached_state_detect(hd3ss3220);
> > +
> > +	if (attached_state == EXTCON_USB_HOST) {
> > +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB,
> false);
> > +		extcon_set_state_sync(hd3ss3220->extcon,
> EXTCON_USB_HOST, true);
> > +		typec_set_data_role(hd3ss3220->port, TYPEC_HOST);
> > +	} else if (attached_state == EXTCON_USB) {
> > +		extcon_set_state_sync(hd3ss3220->extcon,
> > +						 EXTCON_USB_HOST, false);
> > +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB,
> true);
> > +		typec_set_data_role(hd3ss3220->port, TYPEC_DEVICE);
> > +	} else {
> > +		hd3ss3220_set_source_pref(hd3ss3220,
> > +
> 	HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
> > +		extcon_set_state_sync(hd3ss3220->extcon,
> > +						EXTCON_USB_HOST, false);
> > +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB,
> false);
> > +	}
> > +}
> > +
> > +irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220) {
> > +	int err;
> > +	unsigned int data;
> > +
> > +	hd3ss3220_set_extcon_state(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 int hd3ss3220_probe(struct i2c_client *client,
> > +		const struct i2c_device_id *id)
> > +{
> > +	struct hd3ss3220 *hd3ss3220;
> > +	int ret;
> > +	unsigned int data;
> > +	static const struct regmap_config config = {
> > +		.reg_bits = 8,
> > +		.val_bits = 8,
> > +		.max_register = 0x0A,
> > +	};
> 
> Move that outside of the function.
 OK. Will do.
> > +	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->extcon = devm_extcon_dev_allocate(&client->dev,
> > +						hd3ss3220_cable);
> > +	if (IS_ERR(hd3ss3220->extcon)) {
> > +		dev_err(&client->dev, "failed to allocate extcon device\n");
> > +		return PTR_ERR(hd3ss3220->extcon);
> > +	}
> > +
> > +	ret = devm_extcon_dev_register(&client->dev, hd3ss3220->extcon);
> > +	if (ret < 0) {
> > +		dev_err(&client->dev, "failed to register extcon device\n");
> > +		return ret;
> > +	}
> > +
> > +	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_extcon_state(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);
> > +
> > +	return ret;
> > +}
> > +
> > +static int hd3ss3220_remove(struct i2c_client *client) {
> > +	struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
> > +
> > +	typec_unregister_port(hd3ss3220->port);
> > +
> > +	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");
> 
> You will need to use the USB role class instead of extcon. Otherwise this
> looks OK to me, assuming you fix the style issues ;-).

Regards,
Biju

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

[Biju Das]

Hi Heikki,

Thanks for the feedback.

> Subject: Re: [PATCH 3/9] usb: typec: driver for TI HD3SS3220 USB Type-C DRP
> port controller
> 
> On Wed, Mar 06, 2019 at 09:07:20AM +0000, Biju Das wrote:
> > 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>
> > ---
> >  drivers/usb/typec/Kconfig     |  10 ++
> >  drivers/usb/typec/Makefile    |   1 +
> >  drivers/usb/typec/hd3ss3220.c | 284
> > ++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 295 insertions(+)
> >  create mode 100644 drivers/usb/typec/hd3ss3220.c
> >
> > diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> > index 30a847c..91696d2 100644
> > --- a/drivers/usb/typec/Kconfig
> > +++ b/drivers/usb/typec/Kconfig
> > @@ -49,6 +49,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..bd3e1ec
> > --- /dev/null
> > +++ b/drivers/usb/typec/hd3ss3220.c
> > @@ -0,0 +1,284 @@
> > +// 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/extcon-provider.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 extcon_dev *extcon;
> > +	struct typec_port *port;
> > +	struct typec_capability typec_cap;
> > +};
> > +
> > +static const unsigned int hd3ss3220_cable[] = {
> > +	EXTCON_USB,
> > +	EXTCON_USB_HOST,
> > +	EXTCON_NONE
> > +};
> 
> You need to use the USB role class instead of extcon. Check
> drivers/usb/roles/class/ and include/linux/usb/role.h
> 
> You may also want to check the updates Yu Chen is introducing to that
> class:
> https://lkml.org/lkml/2019/3/2/42

Ok. Will use USB role class. Basically this driver need to get role_sw handle from
usb3 driver. Based on the  cable attach status/role switch it need to call 
"usb_role_switch_set_role"  function and  driver will get  notified after
setting the role.

> > +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);
> 
> Please fix the alignment:

OK. Will fix it.

>         return regmap_write(hd3ss3220->regmap, HD3SS3220_REG_GEN_CTRL,
> reg_val);
> 
> > +}
> > +
> > +static int hd3ss3220_attached_state_detect(struct hd3ss3220
> > +*hd3ss3220) {
> > +	unsigned int reg_val;
> > +	int ret, attached_state;
> > +
> > +	ret = regmap_read(hd3ss3220->regmap,
> > +				HD3SS3220_REG_CN_STAT_CTRL, &reg_val);
> 
> ditto:
OK. Will fix it.

> 	ret = regmap_read(hd3ss3220->regmap,
> HD3SS3220_REG_CN_STAT_CTRL,
>                           &reg_val);
> 
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	reg_val &=
> HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK;
> > +	switch (reg_val) {
> 
>         switch (reg_val &
> HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK)
> 
> > +	case HD3SS3220_REG_CN_STAT_CTRL_AS_DFP:
> > +		attached_state = EXTCON_USB_HOST;
> > +	break;
> > +	case HD3SS3220_REG_CN_STAT_CTRL_AS_UFP:
> > +		attached_state = EXTCON_USB;
> > +	break;
> > +	default:
> > +		attached_state = EXTCON_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);
> 
> I think scripts/checkpatch.pl would find all these alignment issues:

I have ran check patch and it didn't complained about any alignment issues.
Any way will fix all the alignment issues.

> 	struct hd3ss3220 *hd3ss3220 = container_of(cap, struct hd3ss3220,
>                                                    typec_cap);
> 
> There are a lot more of those in this patch. Please fix all of them.

OK. Will fix.

> > +	int ret = 0;
> > +
> > +	if (role == TYPEC_HOST) {
> > +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB,
> false);
> > +		ret = hd3ss3220_set_source_pref(hd3ss3220,
> > +
> 	HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC);
> > +		mdelay(100);
> 
> Whoa! That's a long delay. A bit too long. You should sleep if you really need
> to wait that long, but 100 ms?

OK. Will check this again and put appropriate delay/sleep.

> > +		extcon_set_state_sync(hd3ss3220->extcon,
> EXTCON_USB_HOST, true);
> > +	} else {
> > +		extcon_set_state_sync(hd3ss3220->extcon,
> > +						EXTCON_USB_HOST, false);
> > +		ret = hd3ss3220_set_source_pref(hd3ss3220,
> > +
> 	HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK);
> > +		mdelay(100);
> > +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB,
> true);
> > +	}
> 
> I think you could just call hd3ss3220_set_source_pref() ones here.
> Just store the value to a variable in those conditions:

OK. Will do.

>         if (role == TYPEC_HOST)
>                 pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC;
>         else
>                 pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK;
> 
>         ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
>         ...
> 
> > +	typec_set_data_role(hd3ss3220->port, role);
> > +
> > +	return ret;
> > +}
> > +
> > +static void hd3ss3220_set_extcon_state(struct hd3ss3220 *hd3ss3220) {
> > +	int attached_state = hd3ss3220_attached_state_detect(hd3ss3220);
> > +
> > +	if (attached_state == EXTCON_USB_HOST) {
> > +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB,
> false);
> > +		extcon_set_state_sync(hd3ss3220->extcon,
> EXTCON_USB_HOST, true);
> > +		typec_set_data_role(hd3ss3220->port, TYPEC_HOST);
> > +	} else if (attached_state == EXTCON_USB) {
> > +		extcon_set_state_sync(hd3ss3220->extcon,
> > +						 EXTCON_USB_HOST, false);
> > +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB,
> true);
> > +		typec_set_data_role(hd3ss3220->port, TYPEC_DEVICE);
> > +	} else {
> > +		hd3ss3220_set_source_pref(hd3ss3220,
> > +
> 	HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
> > +		extcon_set_state_sync(hd3ss3220->extcon,
> > +						EXTCON_USB_HOST, false);
> > +		extcon_set_state_sync(hd3ss3220->extcon, EXTCON_USB,
> false);
> > +	}
> > +}
> > +
> > +irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220) {
> > +	int err;
> > +	unsigned int data;
> > +
> > +	hd3ss3220_set_extcon_state(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 int hd3ss3220_probe(struct i2c_client *client,
> > +		const struct i2c_device_id *id)
> > +{
> > +	struct hd3ss3220 *hd3ss3220;
> > +	int ret;
> > +	unsigned int data;
> > +	static const struct regmap_config config = {
> > +		.reg_bits = 8,
> > +		.val_bits = 8,
> > +		.max_register = 0x0A,
> > +	};
> 
> Move that outside of the function.
 OK. Will do.
> > +	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->extcon = devm_extcon_dev_allocate(&client->dev,
> > +						hd3ss3220_cable);
> > +	if (IS_ERR(hd3ss3220->extcon)) {
> > +		dev_err(&client->dev, "failed to allocate extcon device\n");
> > +		return PTR_ERR(hd3ss3220->extcon);
> > +	}
> > +
> > +	ret = devm_extcon_dev_register(&client->dev, hd3ss3220->extcon);
> > +	if (ret < 0) {
> > +		dev_err(&client->dev, "failed to register extcon device\n");
> > +		return ret;
> > +	}
> > +
> > +	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_extcon_state(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);
> > +
> > +	return ret;
> > +}
> > +
> > +static int hd3ss3220_remove(struct i2c_client *client) {
> > +	struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
> > +
> > +	typec_unregister_port(hd3ss3220->port);
> > +
> > +	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");
> 
> You will need to use the USB role class instead of extcon. Otherwise this
> looks OK to me, assuming you fix the style issues ;-).

Regards,
Biju

RE: [PATCH 4/9] usb: gadget: udc: renesas_usb3: use extcon framework to receive connect/disconnect

[Biju Das]

Hi Heikki,

Thanks for the feedback.

> Subject: Re: [PATCH 4/9] usb: gadget: udc: renesas_usb3: use extcon
> framework to receive connect/disconnect
> 
> On Wed, Mar 06, 2019 at 09:07:21AM +0000, Biju Das wrote:
> > RZ/G2E cat874 board is capable of detecting cable connect and
> > disconnect events. Add support for renesas_usb3 to receive connect and
> > disconnect notification using extcon framework.
> 
> I think you will need the series from Yu Chen [1], but once we have that, you
> can do this with USB role class instead of relying on extcon, and that is what
> you need to do.
> 
> There is a reason why we need to use a dedicated class with the USB role
> switching instead of relying on extcon. We actually originally planned on using
> extcon with the USB role switches, but the extcon maintainers refused some
> the USB mux drivers. The problem from extcon perspective is that the
> consumer/supplier roles seem to be inverted in some cases. USB role switch
> can simply be too many things - discrete mux, integrated mux (like in your
> case) or something like a dual role capable USB controller could simply
> represent one.
> 
> On the other hand, using a dedicated API and class now feel like a much
> better idea in general compared to a multipurpose thing like extcon.
> 
> [1] https://lkml.org/lkml/2019/3/2/42

Ok. Will use usb role class instead of extcon . 

Regards,
Biju

[4/9] usb: gadget: udc: renesas_usb3: use extcon framework to receive connect/disconnect

[Biju Das]

Hi Heikki,

Thanks for the feedback.

> Subject: Re: [PATCH 4/9] usb: gadget: udc: renesas_usb3: use extcon
> framework to receive connect/disconnect
> 
> On Wed, Mar 06, 2019 at 09:07:21AM +0000, Biju Das wrote:
> > RZ/G2E cat874 board is capable of detecting cable connect and
> > disconnect events. Add support for renesas_usb3 to receive connect and
> > disconnect notification using extcon framework.
> 
> I think you will need the series from Yu Chen [1], but once we have that, you
> can do this with USB role class instead of relying on extcon, and that is what
> you need to do.
> 
> There is a reason why we need to use a dedicated class with the USB role
> switching instead of relying on extcon. We actually originally planned on using
> extcon with the USB role switches, but the extcon maintainers refused some
> the USB mux drivers. The problem from extcon perspective is that the
> consumer/supplier roles seem to be inverted in some cases. USB role switch
> can simply be too many things - discrete mux, integrated mux (like in your
> case) or something like a dual role capable USB controller could simply
> represent one.
> 
> On the other hand, using a dedicated API and class now feel like a much
> better idea in general compared to a multipurpose thing like extcon.
> 
> [1] https://lkml.org/lkml/2019/3/2/42

Ok. Will use usb role class instead of extcon . 

Regards,
Biju

Re: [PATCH 2/9] dt-bindings: usb: renesas_usb3: add extcon support

[Rob Herring]

On Wed, Mar 06, 2019 at 09:07:19AM +0000, Biju Das wrote:
> Add support for renesas_usb3 to receive connect and disconnect notification
> using extcon framework.
> 
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
>  Documentation/devicetree/bindings/usb/renesas_usb3.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> index d366555..ae35674 100644
> --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> @@ -21,6 +21,8 @@ Required properties:
>  Optional properties:
>    - phys: phandle + phy specifier pair
>    - phy-names: must be "usb"
> +  - extcon: phandle for the extcon device renesas usb3 uses to detect
> +	    connect/disconnect events.

Please don't use extcon and use usb-connector binding instead.

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

[2/9] dt-bindings: usb: renesas_usb3: add extcon support

[Rob Herring]

On Wed, Mar 06, 2019 at 09:07:19AM +0000, Biju Das wrote:
> Add support for renesas_usb3 to receive connect and disconnect notification
> using extcon framework.
> 
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
>  Documentation/devicetree/bindings/usb/renesas_usb3.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> index d366555..ae35674 100644
> --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> @@ -21,6 +21,8 @@ Required properties:
>  Optional properties:
>    - phys: phandle + phy specifier pair
>    - phy-names: must be "usb"
> +  - extcon: phandle for the extcon device renesas usb3 uses to detect
> +	    connect/disconnect events.

Please don't use extcon and use usb-connector binding instead.

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

RE: [PATCH 2/9] dt-bindings: usb: renesas_usb3: add extcon support

[Biju Das]

Hi Rob,

Thanks for the feedback.

> Subject: Re: [PATCH 2/9] dt-bindings: usb: renesas_usb3: add extcon support
> 
> On Wed, Mar 06, 2019 at 09:07:19AM +0000, Biju Das wrote:
> > Add support for renesas_usb3 to receive connect and disconnect
> > notification using extcon framework.
> >
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > ---
> >  Documentation/devicetree/bindings/usb/renesas_usb3.txt | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > index d366555..ae35674 100644
> > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > @@ -21,6 +21,8 @@ Required properties:
> >  Optional properties:
> >    - phys: phandle + phy specifier pair
> >    - phy-names: must be "usb"
> > +  - extcon: phandle for the extcon device renesas usb3 uses to detect
> > +	    connect/disconnect events.
> 
> Please don't use extcon and use usb-connector binding instead.

I have switched to usb role switch framework  instead of extcon and sent V2 based on the below feedback.
https://patchwork.kernel.org/patch/10840659/

On V2, I have sent  a patch series which uses " usb-connector " binding [1]   and "usb-role-switch" property[2]
[1] https://patchwork.kernel.org/patch/10852495/
[2] https://patchwork.kernel.org/patch/10852497/

Support for  "usb-role-switch" compatible can be  found in the below patch set
https://patchwork.kernel.org/project/linux-usb/list/?series=97707

Please can you comment, is this approach is acceptable or not?

Regards,
Biju 

RE: [PATCH 2/9] dt-bindings: usb: renesas_usb3: add extcon support

[Biju Das]

Hi Rob,

Thanks for the feedback.

> Subject: Re: [PATCH 2/9] dt-bindings: usb: renesas_usb3: add extcon support
> 
> On Wed, Mar 06, 2019 at 09:07:19AM +0000, Biju Das wrote:
> > Add support for renesas_usb3 to receive connect and disconnect
> > notification using extcon framework.
> >
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > ---
> >  Documentation/devicetree/bindings/usb/renesas_usb3.txt | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > index d366555..ae35674 100644
> > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > @@ -21,6 +21,8 @@ Required properties:
> >  Optional properties:
> >    - phys: phandle + phy specifier pair
> >    - phy-names: must be "usb"
> > +  - extcon: phandle for the extcon device renesas usb3 uses to detect
> > +	    connect/disconnect events.
> 
> Please don't use extcon and use usb-connector binding instead.

I have switched to usb role switch framework  instead of extcon and sent V2 based on the below feedback.
https://patchwork.kernel.org/patch/10840659/

On V2, I have sent  a patch series which uses " usb-connector " binding [1]   and "usb-role-switch" property[2]
[1] https://patchwork.kernel.org/patch/10852495/
[2] https://patchwork.kernel.org/patch/10852497/

Support for  "usb-role-switch" compatible can be  found in the below patch set
https://patchwork.kernel.org/project/linux-usb/list/?series=97707

Please can you comment, is this approach is acceptable or not?

Regards,
Biju 

[2/9] dt-bindings: usb: renesas_usb3: add extcon support

[Biju Das]

Hi Rob,

Thanks for the feedback.

> Subject: Re: [PATCH 2/9] dt-bindings: usb: renesas_usb3: add extcon support
> 
> On Wed, Mar 06, 2019 at 09:07:19AM +0000, Biju Das wrote:
> > Add support for renesas_usb3 to receive connect and disconnect
> > notification using extcon framework.
> >
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > ---
> >  Documentation/devicetree/bindings/usb/renesas_usb3.txt | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > index d366555..ae35674 100644
> > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
> > @@ -21,6 +21,8 @@ Required properties:
> >  Optional properties:
> >    - phys: phandle + phy specifier pair
> >    - phy-names: must be "usb"
> > +  - extcon: phandle for the extcon device renesas usb3 uses to detect
> > +	    connect/disconnect events.
> 
> Please don't use extcon and use usb-connector binding instead.

I have switched to usb role switch framework  instead of extcon and sent V2 based on the below feedback.
https://patchwork.kernel.org/patch/10840659/

On V2, I have sent  a patch series which uses " usb-connector " binding [1]   and "usb-role-switch" property[2]
[1] https://patchwork.kernel.org/patch/10852495/
[2] https://patchwork.kernel.org/patch/10852497/

Support for  "usb-role-switch" compatible can be  found in the below patch set
https://patchwork.kernel.org/project/linux-usb/list/?series=97707

Please can you comment, is this approach is acceptable or not?

Regards,
Biju