* [PATCH V4 1/7] dt-bindings: usb: hd3ss3220 device tree binding document
@ 2019-04-16 9:38 ` Biju Das
0 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-16 9:38 UTC (permalink / raw)
To: Rob Herring, Mark Rutland
Cc: Biju Das, Greg Kroah-Hartman, Heikki Krogerus, Felipe Balbi,
linux-usb, devicetree, Simon Horman, Yoshihiro Shimoda,
Geert Uytterhoeven, Chris Paterson, Fabrizio Castro,
linux-renesas-soc
Add device tree binding document for TI HD3SS3220 Type-C DRP port
controller driver.
Signed-off-by: Biju Das <biju.das@bp.renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
V3-->V4
* No Change.
V2-->V3
* Added Rob's Reviewed by tag.
V1-->V2
* Added connector node.
* updated the example with connector node.
---
.../devicetree/bindings/usb/ti,hd3ss3220.txt | 37 ++++++++++++++++++++++
1 file changed, 37 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
new file mode 100644
index 0000000..7f41400
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
@@ -0,0 +1,37 @@
+TI HD3SS3220 TypeC DRP Port Controller.
+
+Required properties:
+ - compatible: Must be "ti,hd3ss3220".
+ - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin.
+ - interrupts: <a b> where a is the interrupt number and b represents an
+ encoding of the sense and level information for the interrupt.
+
+Required sub-node:
+ - connector : The "usb-c-connector" attached to the hd3ss3220 chip. The
+ bindings of the connector node are specified in:
+
+ Documentation/devicetree/bindings/connector/usb-connector.txt
+
+Example:
+hd3ss3220@47 {
+ compatible = "ti,hd3ss3220";
+ reg = <0x47>;
+ interrupt-parent = <&gpio6>;
+ interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+
+ usb_con: connector {
+ compatible = "usb-c-connector";
+ label = "USB-C";
+ data-role = "dual";
+ };
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ hd3ss3220_ep: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&usb3peri_role_switch>;
+ };
+ };
+};
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [V4,1/7] dt-bindings: usb: hd3ss3220 device tree binding document
@ 2019-04-16 9:38 ` Biju Das
0 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-16 9:38 UTC (permalink / raw)
To: Rob Herring, Mark Rutland
Cc: Biju Das, Greg Kroah-Hartman, Heikki Krogerus, Felipe Balbi,
linux-usb, devicetree, Simon Horman, Yoshihiro Shimoda,
Geert Uytterhoeven, Chris Paterson, Fabrizio Castro,
linux-renesas-soc
Add device tree binding document for TI HD3SS3220 Type-C DRP port
controller driver.
Signed-off-by: Biju Das <biju.das@bp.renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
V3-->V4
* No Change.
V2-->V3
* Added Rob's Reviewed by tag.
V1-->V2
* Added connector node.
* updated the example with connector node.
---
.../devicetree/bindings/usb/ti,hd3ss3220.txt | 37 ++++++++++++++++++++++
1 file changed, 37 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
diff --git a/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
new file mode 100644
index 0000000..7f41400
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/ti,hd3ss3220.txt
@@ -0,0 +1,37 @@
+TI HD3SS3220 TypeC DRP Port Controller.
+
+Required properties:
+ - compatible: Must be "ti,hd3ss3220".
+ - reg: I2C slave address, must be 0x47 or 0x67 based on ADDR pin.
+ - interrupts: <a b> where a is the interrupt number and b represents an
+ encoding of the sense and level information for the interrupt.
+
+Required sub-node:
+ - connector : The "usb-c-connector" attached to the hd3ss3220 chip. The
+ bindings of the connector node are specified in:
+
+ Documentation/devicetree/bindings/connector/usb-connector.txt
+
+Example:
+hd3ss3220@47 {
+ compatible = "ti,hd3ss3220";
+ reg = <0x47>;
+ interrupt-parent = <&gpio6>;
+ interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+
+ usb_con: connector {
+ compatible = "usb-c-connector";
+ label = "USB-C";
+ data-role = "dual";
+ };
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ hd3ss3220_ep: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&usb3peri_role_switch>;
+ };
+ };
+};
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH V4 2/7] dt-bindings: usb: renesas_usb3: Add renesas,usb-role-switch property
@ 2019-04-16 9:38 ` Biju Das
0 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-16 9:38 UTC (permalink / raw)
To: Rob Herring, Mark Rutland
Cc: Biju Das, Greg Kroah-Hartman, Heikki Krogerus, Felipe Balbi,
linux-usb, devicetree, Simon Horman, Yoshihiro Shimoda,
Geert Uytterhoeven, Chris Paterson, Fabrizio Castro,
linux-renesas-soc
Add an optional property renesas,usb-role-switch to support
dual role switch for USB Type-C DRP port controller devices
using USB role switch class framework.
Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
V3-->V4
* No Change
V2-->V3
* Added optional renesas,usb-role-switch property.
V1-->V2
* Added usb-role-switch-property
* Updated the example with usb-role-switch property.
---
.../devicetree/bindings/usb/renesas_usb3.txt | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
index 35039e7..f1cb06a 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
@@ -22,6 +22,7 @@ Required properties:
Optional properties:
- phys: phandle + phy specifier pair
- phy-names: must be "usb"
+ - renesas,usb-role-switch: use USB role switch to handle role switch events
Example of R-Car H3 ES1.x:
usb3_peri0: usb@ee020000 {
@@ -39,3 +40,24 @@ Example of R-Car H3 ES1.x:
interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cpg CPG_MOD 327>;
};
+
+Example of RZ/G2E:
+ usb3_peri0: usb@ee020000 {
+ compatible = "renesas,r8a774c0-usb3-peri",
+ "renesas,rcar-gen3-usb3-peri";
+ reg = <0 0xee020000 0 0x400>;
+ interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD 328>;
+ companion = <&xhci0>;
+ renesas,usb-role-switch;
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ usb3peri_role_switch: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&hd3ss3220_ep>;
+ };
+ };
+ };
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [V4,2/7] dt-bindings: usb: renesas_usb3: Add renesas,usb-role-switch property
@ 2019-04-16 9:38 ` Biju Das
0 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-16 9:38 UTC (permalink / raw)
To: Rob Herring, Mark Rutland
Cc: Biju Das, Greg Kroah-Hartman, Heikki Krogerus, Felipe Balbi,
linux-usb, devicetree, Simon Horman, Yoshihiro Shimoda,
Geert Uytterhoeven, Chris Paterson, Fabrizio Castro,
linux-renesas-soc
Add an optional property renesas,usb-role-switch to support
dual role switch for USB Type-C DRP port controller devices
using USB role switch class framework.
Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
V3-->V4
* No Change
V2-->V3
* Added optional renesas,usb-role-switch property.
V1-->V2
* Added usb-role-switch-property
* Updated the example with usb-role-switch property.
---
.../devicetree/bindings/usb/renesas_usb3.txt | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/renesas_usb3.txt b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
index 35039e7..f1cb06a 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
@@ -22,6 +22,7 @@ Required properties:
Optional properties:
- phys: phandle + phy specifier pair
- phy-names: must be "usb"
+ - renesas,usb-role-switch: use USB role switch to handle role switch events
Example of R-Car H3 ES1.x:
usb3_peri0: usb@ee020000 {
@@ -39,3 +40,24 @@ Example of R-Car H3 ES1.x:
interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cpg CPG_MOD 327>;
};
+
+Example of RZ/G2E:
+ usb3_peri0: usb@ee020000 {
+ compatible = "renesas,r8a774c0-usb3-peri",
+ "renesas,rcar-gen3-usb3-peri";
+ reg = <0 0xee020000 0 0x400>;
+ interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD 328>;
+ companion = <&xhci0>;
+ renesas,usb-role-switch;
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ usb3peri_role_switch: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&hd3ss3220_ep>;
+ };
+ };
+ };
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH V4 3/7] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller
@ 2019-04-16 9:38 ` Biju Das
0 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-16 9:38 UTC (permalink / raw)
To: Heikki Krogerus, Chunfeng Yun
Cc: Biju Das, Greg Kroah-Hartman, Felipe Balbi, linux-usb,
Simon Horman, Yoshihiro Shimoda, Geert Uytterhoeven,
Chris Paterson, Fabrizio Castro, linux-renesas-soc
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>
---
Note: This patch has compilation dependency on
https://patchwork.kernel.org/patch/10882555/
V3-->V4
* Incorporated Chunfeng Yun's review comment
* Used fwnode API's to get usb role switch handle.
V2-->V3
* Used the new api "usb_role_switch by node" for getting
remote endpoint associated with Type-C USB DRP port
controller devices.
V1-->V2
* Driver uses usb role class instead of extcon for dual role switch
and also handles connect/disconnect events.
---
drivers/usb/typec/Kconfig | 10 ++
drivers/usb/typec/Makefile | 1 +
drivers/usb/typec/hd3ss3220.c | 263 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 274 insertions(+)
create mode 100644 drivers/usb/typec/hd3ss3220.c
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 89d9193..92a3717 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -50,6 +50,16 @@ source "drivers/usb/typec/tcpm/Kconfig"
source "drivers/usb/typec/ucsi/Kconfig"
+config TYPEC_HD3SS3220
+ tristate "TI HD3SS3220 Type-C DRP Port controller driver"
+ depends on I2C
+ help
+ Say Y or M here if your system has TI HD3SS3220 Type-C DRP Port
+ controller driver.
+
+ If you choose to build this driver as a dynamically linked module, the
+ module will be called hd3ss3220.ko.
+
config TYPEC_TPS6598X
tristate "TI TPS6598x USB Power Delivery controller driver"
depends on I2C
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 6696b72..7753a5c3 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -4,5 +4,6 @@ typec-y := class.o mux.o bus.o
obj-$(CONFIG_TYPEC) += altmodes/
obj-$(CONFIG_TYPEC_TCPM) += tcpm/
obj-$(CONFIG_TYPEC_UCSI) += ucsi/
+obj-$(CONFIG_TYPEC_HD3SS3220) += hd3ss3220.o
obj-$(CONFIG_TYPEC_TPS6598X) += tps6598x.o
obj-$(CONFIG_TYPEC) += mux/
diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
new file mode 100644
index 0000000..e98a38f
--- /dev/null
+++ b/drivers/usb/typec/hd3ss3220.c
@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * TI HD3SS3220 Type-C DRP Port Controller Driver
+ *
+ * Copyright (C) 2019 Renesas Electronics Corp.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/usb/role.h>
+#include <linux/irqreturn.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/usb/typec.h>
+#include <linux/delay.h>
+
+#define HD3SS3220_REG_CN_STAT_CTRL 0x09
+#define HD3SS3220_REG_GEN_CTRL 0x0A
+#define HD3SS3220_REG_DEV_REV 0xA0
+
+/* Register HD3SS3220_REG_CN_STAT_CTRL*/
+#define HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK (BIT(7) | BIT(6))
+#define HD3SS3220_REG_CN_STAT_CTRL_AS_DFP BIT(6)
+#define HD3SS3220_REG_CN_STAT_CTRL_AS_UFP BIT(7)
+#define HD3SS3220_REG_CN_STAT_CTRL_TO_ACCESSORY (BIT(7) | BIT(6))
+#define HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS BIT(4)
+
+/* Register HD3SS3220_REG_GEN_CTRL*/
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK (BIT(2) | BIT(1))
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT 0x00
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK BIT(1)
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC (BIT(2) | BIT(1))
+
+struct hd3ss3220 {
+ struct device *dev;
+ struct regmap *regmap;
+ struct usb_role_switch *role_sw;
+ struct typec_port *port;
+ struct typec_capability typec_cap;
+};
+
+static int hd3ss3220_set_source_pref(struct hd3ss3220 *hd3ss3220, int src_pref)
+{
+ return regmap_update_bits(hd3ss3220->regmap, HD3SS3220_REG_GEN_CTRL,
+ HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK,
+ src_pref);
+}
+
+static enum usb_role hd3ss3220_get_attached_state(struct hd3ss3220 *hd3ss3220)
+{
+ unsigned int reg_val;
+ enum usb_role attached_state;
+ int ret;
+
+ ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
+ ®_val);
+ if (ret < 0)
+ return ret;
+
+ switch (reg_val & HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK) {
+ case HD3SS3220_REG_CN_STAT_CTRL_AS_DFP:
+ attached_state = USB_ROLE_HOST;
+ break;
+ case HD3SS3220_REG_CN_STAT_CTRL_AS_UFP:
+ attached_state = USB_ROLE_DEVICE;
+ break;
+ default:
+ attached_state = USB_ROLE_NONE;
+ }
+
+ return attached_state;
+}
+
+static int hd3ss3220_dr_set(const struct typec_capability *cap,
+ enum typec_data_role role)
+{
+ struct hd3ss3220 *hd3ss3220 = container_of(cap, struct hd3ss3220,
+ typec_cap);
+ enum usb_role role_val;
+ int pref, ret = 0;
+
+ if (role == TYPEC_HOST) {
+ role_val = USB_ROLE_HOST;
+ pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC;
+ } else {
+ role_val = USB_ROLE_DEVICE;
+ pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK;
+ }
+
+ ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
+ usleep_range(10, 100);
+
+ usb_role_switch_set_role(hd3ss3220->role_sw, role_val);
+ typec_set_data_role(hd3ss3220->port, role);
+
+ return ret;
+}
+
+static void hd3ss3220_set_role(struct hd3ss3220 *hd3ss3220)
+{
+ enum usb_role role_state = hd3ss3220_get_attached_state(hd3ss3220);
+
+ usb_role_switch_set_role(hd3ss3220->role_sw, role_state);
+ if (role_state == USB_ROLE_NONE)
+ hd3ss3220_set_source_pref(hd3ss3220,
+ HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
+
+ switch (role_state) {
+ case USB_ROLE_HOST:
+ typec_set_data_role(hd3ss3220->port, TYPEC_HOST);
+ break;
+ case USB_ROLE_DEVICE:
+ typec_set_data_role(hd3ss3220->port, TYPEC_DEVICE);
+ break;
+ default:
+ break;
+ }
+}
+
+irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220)
+{
+ int err;
+
+ hd3ss3220_set_role(hd3ss3220);
+ err = regmap_update_bits_base(hd3ss3220->regmap,
+ HD3SS3220_REG_CN_STAT_CTRL,
+ HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
+ HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
+ NULL, false, true);
+ if (err < 0)
+ return IRQ_NONE;
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t hd3ss3220_irq_handler(int irq, void *data)
+{
+ struct i2c_client *client = to_i2c_client(data);
+ struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
+
+ return hd3ss3220_irq(hd3ss3220);
+}
+
+static const struct regmap_config config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = 0x0A,
+};
+
+static int hd3ss3220_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct hd3ss3220 *hd3ss3220;
+ struct fwnode_handle *parent, *child;
+ int ret;
+ unsigned int data;
+
+ hd3ss3220 = devm_kzalloc(&client->dev, sizeof(struct hd3ss3220),
+ GFP_KERNEL);
+ if (!hd3ss3220)
+ return -ENOMEM;
+
+ i2c_set_clientdata(client, hd3ss3220);
+
+ hd3ss3220->dev = &client->dev;
+ hd3ss3220->regmap = devm_regmap_init_i2c(client, &config);
+ if (IS_ERR(hd3ss3220->regmap))
+ return PTR_ERR(hd3ss3220->regmap);
+
+ hd3ss3220_set_source_pref(hd3ss3220,
+ HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
+ child = fwnode_graph_get_next_endpoint(dev_fwnode(hd3ss3220->dev),
+ NULL);
+ parent = fwnode_graph_get_remote_port_parent(child);
+ hd3ss3220->role_sw = fwnode_usb_role_switch_get(parent);
+ if (IS_ERR_OR_NULL(hd3ss3220->role_sw)) {
+ fwnode_handle_put(child);
+ fwnode_handle_put(parent);
+ return PTR_ERR(hd3ss3220->role_sw);
+ }
+
+ fwnode_handle_put(child);
+ fwnode_handle_put(parent);
+
+ hd3ss3220->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
+ hd3ss3220->typec_cap.dr_set = hd3ss3220_dr_set;
+ hd3ss3220->typec_cap.type = TYPEC_PORT_DRP;
+ hd3ss3220->typec_cap.data = TYPEC_PORT_DRD;
+
+ hd3ss3220->port = typec_register_port(&client->dev,
+ &hd3ss3220->typec_cap);
+ if (IS_ERR(hd3ss3220->port))
+ return PTR_ERR(hd3ss3220->port);
+
+ hd3ss3220_set_role(hd3ss3220);
+ if (client->irq > 0) {
+ ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
+ &data);
+ if (ret < 0)
+ goto error;
+
+ if (data & HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS) {
+ ret = regmap_write(hd3ss3220->regmap,
+ HD3SS3220_REG_CN_STAT_CTRL,
+ data | HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS);
+ if (ret < 0)
+ goto error;
+ }
+
+ ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+ hd3ss3220_irq_handler,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ "hd3ss3220", &client->dev);
+ if (ret)
+ goto error;
+ }
+
+ ret = i2c_smbus_read_byte_data(client, HD3SS3220_REG_DEV_REV);
+ if (ret < 0)
+ goto error;
+
+ dev_info(&client->dev, "probed revision=0x%x\n", ret);
+
+ return 0;
+error:
+ typec_unregister_port(hd3ss3220->port);
+ usb_role_switch_put(hd3ss3220->role_sw);
+
+ return ret;
+}
+
+static int hd3ss3220_remove(struct i2c_client *client)
+{
+ struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
+
+ typec_unregister_port(hd3ss3220->port);
+ usb_role_switch_put(hd3ss3220->role_sw);
+
+ return 0;
+}
+
+static const struct of_device_id dev_ids[] = {
+ { .compatible = "ti,hd3ss3220"},
+ {}
+};
+MODULE_DEVICE_TABLE(of, dev_ids);
+
+static struct i2c_driver hd3ss3220_driver = {
+ .driver = {
+ .name = "hd3ss3220",
+ .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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [V4,3/7] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller
@ 2019-04-16 9:38 ` Biju Das
0 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-16 9:38 UTC (permalink / raw)
To: Heikki Krogerus, Chunfeng Yun
Cc: Biju Das, Greg Kroah-Hartman, Felipe Balbi, linux-usb,
Simon Horman, Yoshihiro Shimoda, Geert Uytterhoeven,
Chris Paterson, Fabrizio Castro, linux-renesas-soc
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>
---
Note: This patch has compilation dependency on
https://patchwork.kernel.org/patch/10882555/
V3-->V4
* Incorporated Chunfeng Yun's review comment
* Used fwnode API's to get usb role switch handle.
V2-->V3
* Used the new api "usb_role_switch by node" for getting
remote endpoint associated with Type-C USB DRP port
controller devices.
V1-->V2
* Driver uses usb role class instead of extcon for dual role switch
and also handles connect/disconnect events.
---
drivers/usb/typec/Kconfig | 10 ++
drivers/usb/typec/Makefile | 1 +
drivers/usb/typec/hd3ss3220.c | 263 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 274 insertions(+)
create mode 100644 drivers/usb/typec/hd3ss3220.c
diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 89d9193..92a3717 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -50,6 +50,16 @@ source "drivers/usb/typec/tcpm/Kconfig"
source "drivers/usb/typec/ucsi/Kconfig"
+config TYPEC_HD3SS3220
+ tristate "TI HD3SS3220 Type-C DRP Port controller driver"
+ depends on I2C
+ help
+ Say Y or M here if your system has TI HD3SS3220 Type-C DRP Port
+ controller driver.
+
+ If you choose to build this driver as a dynamically linked module, the
+ module will be called hd3ss3220.ko.
+
config TYPEC_TPS6598X
tristate "TI TPS6598x USB Power Delivery controller driver"
depends on I2C
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 6696b72..7753a5c3 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -4,5 +4,6 @@ typec-y := class.o mux.o bus.o
obj-$(CONFIG_TYPEC) += altmodes/
obj-$(CONFIG_TYPEC_TCPM) += tcpm/
obj-$(CONFIG_TYPEC_UCSI) += ucsi/
+obj-$(CONFIG_TYPEC_HD3SS3220) += hd3ss3220.o
obj-$(CONFIG_TYPEC_TPS6598X) += tps6598x.o
obj-$(CONFIG_TYPEC) += mux/
diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
new file mode 100644
index 0000000..e98a38f
--- /dev/null
+++ b/drivers/usb/typec/hd3ss3220.c
@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * TI HD3SS3220 Type-C DRP Port Controller Driver
+ *
+ * Copyright (C) 2019 Renesas Electronics Corp.
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/usb/role.h>
+#include <linux/irqreturn.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/usb/typec.h>
+#include <linux/delay.h>
+
+#define HD3SS3220_REG_CN_STAT_CTRL 0x09
+#define HD3SS3220_REG_GEN_CTRL 0x0A
+#define HD3SS3220_REG_DEV_REV 0xA0
+
+/* Register HD3SS3220_REG_CN_STAT_CTRL*/
+#define HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK (BIT(7) | BIT(6))
+#define HD3SS3220_REG_CN_STAT_CTRL_AS_DFP BIT(6)
+#define HD3SS3220_REG_CN_STAT_CTRL_AS_UFP BIT(7)
+#define HD3SS3220_REG_CN_STAT_CTRL_TO_ACCESSORY (BIT(7) | BIT(6))
+#define HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS BIT(4)
+
+/* Register HD3SS3220_REG_GEN_CTRL*/
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK (BIT(2) | BIT(1))
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT 0x00
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK BIT(1)
+#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC (BIT(2) | BIT(1))
+
+struct hd3ss3220 {
+ struct device *dev;
+ struct regmap *regmap;
+ struct usb_role_switch *role_sw;
+ struct typec_port *port;
+ struct typec_capability typec_cap;
+};
+
+static int hd3ss3220_set_source_pref(struct hd3ss3220 *hd3ss3220, int src_pref)
+{
+ return regmap_update_bits(hd3ss3220->regmap, HD3SS3220_REG_GEN_CTRL,
+ HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK,
+ src_pref);
+}
+
+static enum usb_role hd3ss3220_get_attached_state(struct hd3ss3220 *hd3ss3220)
+{
+ unsigned int reg_val;
+ enum usb_role attached_state;
+ int ret;
+
+ ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
+ ®_val);
+ if (ret < 0)
+ return ret;
+
+ switch (reg_val & HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK) {
+ case HD3SS3220_REG_CN_STAT_CTRL_AS_DFP:
+ attached_state = USB_ROLE_HOST;
+ break;
+ case HD3SS3220_REG_CN_STAT_CTRL_AS_UFP:
+ attached_state = USB_ROLE_DEVICE;
+ break;
+ default:
+ attached_state = USB_ROLE_NONE;
+ }
+
+ return attached_state;
+}
+
+static int hd3ss3220_dr_set(const struct typec_capability *cap,
+ enum typec_data_role role)
+{
+ struct hd3ss3220 *hd3ss3220 = container_of(cap, struct hd3ss3220,
+ typec_cap);
+ enum usb_role role_val;
+ int pref, ret = 0;
+
+ if (role == TYPEC_HOST) {
+ role_val = USB_ROLE_HOST;
+ pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC;
+ } else {
+ role_val = USB_ROLE_DEVICE;
+ pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK;
+ }
+
+ ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
+ usleep_range(10, 100);
+
+ usb_role_switch_set_role(hd3ss3220->role_sw, role_val);
+ typec_set_data_role(hd3ss3220->port, role);
+
+ return ret;
+}
+
+static void hd3ss3220_set_role(struct hd3ss3220 *hd3ss3220)
+{
+ enum usb_role role_state = hd3ss3220_get_attached_state(hd3ss3220);
+
+ usb_role_switch_set_role(hd3ss3220->role_sw, role_state);
+ if (role_state == USB_ROLE_NONE)
+ hd3ss3220_set_source_pref(hd3ss3220,
+ HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
+
+ switch (role_state) {
+ case USB_ROLE_HOST:
+ typec_set_data_role(hd3ss3220->port, TYPEC_HOST);
+ break;
+ case USB_ROLE_DEVICE:
+ typec_set_data_role(hd3ss3220->port, TYPEC_DEVICE);
+ break;
+ default:
+ break;
+ }
+}
+
+irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220)
+{
+ int err;
+
+ hd3ss3220_set_role(hd3ss3220);
+ err = regmap_update_bits_base(hd3ss3220->regmap,
+ HD3SS3220_REG_CN_STAT_CTRL,
+ HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
+ HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
+ NULL, false, true);
+ if (err < 0)
+ return IRQ_NONE;
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t hd3ss3220_irq_handler(int irq, void *data)
+{
+ struct i2c_client *client = to_i2c_client(data);
+ struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
+
+ return hd3ss3220_irq(hd3ss3220);
+}
+
+static const struct regmap_config config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = 0x0A,
+};
+
+static int hd3ss3220_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct hd3ss3220 *hd3ss3220;
+ struct fwnode_handle *parent, *child;
+ int ret;
+ unsigned int data;
+
+ hd3ss3220 = devm_kzalloc(&client->dev, sizeof(struct hd3ss3220),
+ GFP_KERNEL);
+ if (!hd3ss3220)
+ return -ENOMEM;
+
+ i2c_set_clientdata(client, hd3ss3220);
+
+ hd3ss3220->dev = &client->dev;
+ hd3ss3220->regmap = devm_regmap_init_i2c(client, &config);
+ if (IS_ERR(hd3ss3220->regmap))
+ return PTR_ERR(hd3ss3220->regmap);
+
+ hd3ss3220_set_source_pref(hd3ss3220,
+ HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
+ child = fwnode_graph_get_next_endpoint(dev_fwnode(hd3ss3220->dev),
+ NULL);
+ parent = fwnode_graph_get_remote_port_parent(child);
+ hd3ss3220->role_sw = fwnode_usb_role_switch_get(parent);
+ if (IS_ERR_OR_NULL(hd3ss3220->role_sw)) {
+ fwnode_handle_put(child);
+ fwnode_handle_put(parent);
+ return PTR_ERR(hd3ss3220->role_sw);
+ }
+
+ fwnode_handle_put(child);
+ fwnode_handle_put(parent);
+
+ hd3ss3220->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
+ hd3ss3220->typec_cap.dr_set = hd3ss3220_dr_set;
+ hd3ss3220->typec_cap.type = TYPEC_PORT_DRP;
+ hd3ss3220->typec_cap.data = TYPEC_PORT_DRD;
+
+ hd3ss3220->port = typec_register_port(&client->dev,
+ &hd3ss3220->typec_cap);
+ if (IS_ERR(hd3ss3220->port))
+ return PTR_ERR(hd3ss3220->port);
+
+ hd3ss3220_set_role(hd3ss3220);
+ if (client->irq > 0) {
+ ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
+ &data);
+ if (ret < 0)
+ goto error;
+
+ if (data & HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS) {
+ ret = regmap_write(hd3ss3220->regmap,
+ HD3SS3220_REG_CN_STAT_CTRL,
+ data | HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS);
+ if (ret < 0)
+ goto error;
+ }
+
+ ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
+ hd3ss3220_irq_handler,
+ IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+ "hd3ss3220", &client->dev);
+ if (ret)
+ goto error;
+ }
+
+ ret = i2c_smbus_read_byte_data(client, HD3SS3220_REG_DEV_REV);
+ if (ret < 0)
+ goto error;
+
+ dev_info(&client->dev, "probed revision=0x%x\n", ret);
+
+ return 0;
+error:
+ typec_unregister_port(hd3ss3220->port);
+ usb_role_switch_put(hd3ss3220->role_sw);
+
+ return ret;
+}
+
+static int hd3ss3220_remove(struct i2c_client *client)
+{
+ struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
+
+ typec_unregister_port(hd3ss3220->port);
+ usb_role_switch_put(hd3ss3220->role_sw);
+
+ return 0;
+}
+
+static const struct of_device_id dev_ids[] = {
+ { .compatible = "ti,hd3ss3220"},
+ {}
+};
+MODULE_DEVICE_TABLE(of, dev_ids);
+
+static struct i2c_driver hd3ss3220_driver = {
+ .driver = {
+ .name = "hd3ss3220",
+ .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");
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH V4 3/7] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller
@ 2019-04-17 9:57 ` Heikki Krogerus
0 siblings, 0 replies; 25+ messages in thread
From: Heikki Krogerus @ 2019-04-17 9:57 UTC (permalink / raw)
To: Biju Das
Cc: Chunfeng Yun, Greg Kroah-Hartman, Felipe Balbi, linux-usb,
Simon Horman, Yoshihiro Shimoda, Geert Uytterhoeven,
Chris Paterson, Fabrizio Castro, linux-renesas-soc
On Tue, Apr 16, 2019 at 10:38:03AM +0100, 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>
It's too bad they call it "port controller" even though it's not Port
Controller Interface compliant. I put a few minor nitpicks below.
Otherwise this looks okay to me, so if there are no other comments:
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> Note: This patch has compilation dependency on
> https://patchwork.kernel.org/patch/10882555/
>
> V3-->V4
> * Incorporated Chunfeng Yun's review comment
> * Used fwnode API's to get usb role switch handle.
>
> V2-->V3
> * Used the new api "usb_role_switch by node" for getting
> remote endpoint associated with Type-C USB DRP port
> controller devices.
> V1-->V2
> * Driver uses usb role class instead of extcon for dual role switch
> and also handles connect/disconnect events.
> ---
> drivers/usb/typec/Kconfig | 10 ++
> drivers/usb/typec/Makefile | 1 +
> drivers/usb/typec/hd3ss3220.c | 263 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 274 insertions(+)
> create mode 100644 drivers/usb/typec/hd3ss3220.c
>
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> index 89d9193..92a3717 100644
> --- a/drivers/usb/typec/Kconfig
> +++ b/drivers/usb/typec/Kconfig
> @@ -50,6 +50,16 @@ source "drivers/usb/typec/tcpm/Kconfig"
>
> source "drivers/usb/typec/ucsi/Kconfig"
>
> +config TYPEC_HD3SS3220
> + tristate "TI HD3SS3220 Type-C DRP Port controller driver"
> + depends on I2C
> + help
> + Say Y or M here if your system has TI HD3SS3220 Type-C DRP Port
> + controller driver.
> +
> + If you choose to build this driver as a dynamically linked module, the
> + module will be called hd3ss3220.ko.
> +
> config TYPEC_TPS6598X
> tristate "TI TPS6598x USB Power Delivery controller driver"
> depends on I2C
> diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
> index 6696b72..7753a5c3 100644
> --- a/drivers/usb/typec/Makefile
> +++ b/drivers/usb/typec/Makefile
> @@ -4,5 +4,6 @@ typec-y := class.o mux.o bus.o
> obj-$(CONFIG_TYPEC) += altmodes/
> obj-$(CONFIG_TYPEC_TCPM) += tcpm/
> obj-$(CONFIG_TYPEC_UCSI) += ucsi/
> +obj-$(CONFIG_TYPEC_HD3SS3220) += hd3ss3220.o
> obj-$(CONFIG_TYPEC_TPS6598X) += tps6598x.o
> obj-$(CONFIG_TYPEC) += mux/
> diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
> new file mode 100644
> index 0000000..e98a38f
> --- /dev/null
> +++ b/drivers/usb/typec/hd3ss3220.c
> @@ -0,0 +1,263 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * TI HD3SS3220 Type-C DRP Port Controller Driver
> + *
> + * Copyright (C) 2019 Renesas Electronics Corp.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/usb/role.h>
> +#include <linux/irqreturn.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/usb/typec.h>
> +#include <linux/delay.h>
> +
> +#define HD3SS3220_REG_CN_STAT_CTRL 0x09
> +#define HD3SS3220_REG_GEN_CTRL 0x0A
> +#define HD3SS3220_REG_DEV_REV 0xA0
> +
> +/* Register HD3SS3220_REG_CN_STAT_CTRL*/
> +#define HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK (BIT(7) | BIT(6))
> +#define HD3SS3220_REG_CN_STAT_CTRL_AS_DFP BIT(6)
> +#define HD3SS3220_REG_CN_STAT_CTRL_AS_UFP BIT(7)
> +#define HD3SS3220_REG_CN_STAT_CTRL_TO_ACCESSORY (BIT(7) | BIT(6))
> +#define HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS BIT(4)
> +
> +/* Register HD3SS3220_REG_GEN_CTRL*/
> +#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK (BIT(2) | BIT(1))
> +#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT 0x00
> +#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK BIT(1)
> +#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC (BIT(2) | BIT(1))
> +
> +struct hd3ss3220 {
> + struct device *dev;
> + struct regmap *regmap;
> + struct usb_role_switch *role_sw;
> + struct typec_port *port;
> + struct typec_capability typec_cap;
> +};
> +
> +static int hd3ss3220_set_source_pref(struct hd3ss3220 *hd3ss3220, int src_pref)
> +{
> + return regmap_update_bits(hd3ss3220->regmap, HD3SS3220_REG_GEN_CTRL,
> + HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK,
> + src_pref);
> +}
> +
> +static enum usb_role hd3ss3220_get_attached_state(struct hd3ss3220 *hd3ss3220)
> +{
> + unsigned int reg_val;
> + enum usb_role attached_state;
> + int ret;
> +
> + ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
> + ®_val);
> + if (ret < 0)
> + return ret;
> +
> + switch (reg_val & HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK) {
> + case HD3SS3220_REG_CN_STAT_CTRL_AS_DFP:
> + attached_state = USB_ROLE_HOST;
> + break;
> + case HD3SS3220_REG_CN_STAT_CTRL_AS_UFP:
> + attached_state = USB_ROLE_DEVICE;
> + break;
It looks like your line indentation has collapsed here. I pretty sure
scripts/checkpatch.pl would have found these.
> + default:
> + attached_state = USB_ROLE_NONE;
> + }
> +
> + return attached_state;
> +}
> +
> +static int hd3ss3220_dr_set(const struct typec_capability *cap,
> + enum typec_data_role role)
> +{
> + struct hd3ss3220 *hd3ss3220 = container_of(cap, struct hd3ss3220,
> + typec_cap);
> + enum usb_role role_val;
> + int pref, ret = 0;
> +
> + if (role == TYPEC_HOST) {
> + role_val = USB_ROLE_HOST;
> + pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC;
> + } else {
> + role_val = USB_ROLE_DEVICE;
> + pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK;
> + }
> +
> + ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
> + usleep_range(10, 100);
> +
> + usb_role_switch_set_role(hd3ss3220->role_sw, role_val);
> + typec_set_data_role(hd3ss3220->port, role);
> +
> + return ret;
> +}
> +
> +static void hd3ss3220_set_role(struct hd3ss3220 *hd3ss3220)
> +{
> + enum usb_role role_state = hd3ss3220_get_attached_state(hd3ss3220);
> +
> + usb_role_switch_set_role(hd3ss3220->role_sw, role_state);
> + if (role_state == USB_ROLE_NONE)
> + hd3ss3220_set_source_pref(hd3ss3220,
> + HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
> +
> + switch (role_state) {
> + case USB_ROLE_HOST:
> + typec_set_data_role(hd3ss3220->port, TYPEC_HOST);
> + break;
> + case USB_ROLE_DEVICE:
> + typec_set_data_role(hd3ss3220->port, TYPEC_DEVICE);
> + break;
> + default:
> + break;
> + }
> +}
> +
> +irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220)
> +{
> + int err;
> +
> + hd3ss3220_set_role(hd3ss3220);
> + err = regmap_update_bits_base(hd3ss3220->regmap,
> + HD3SS3220_REG_CN_STAT_CTRL,
> + HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
> + HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
> + NULL, false, true);
> + if (err < 0)
> + return IRQ_NONE;
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t hd3ss3220_irq_handler(int irq, void *data)
> +{
> + struct i2c_client *client = to_i2c_client(data);
> + struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
> +
> + return hd3ss3220_irq(hd3ss3220);
> +}
> +
> +static const struct regmap_config config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = 0x0A,
> +};
> +
> +static int hd3ss3220_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct hd3ss3220 *hd3ss3220;
> + struct fwnode_handle *parent, *child;
> + int ret;
> + unsigned int data;
> +
> + hd3ss3220 = devm_kzalloc(&client->dev, sizeof(struct hd3ss3220),
> + GFP_KERNEL);
> + if (!hd3ss3220)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(client, hd3ss3220);
> +
> + hd3ss3220->dev = &client->dev;
> + hd3ss3220->regmap = devm_regmap_init_i2c(client, &config);
> + if (IS_ERR(hd3ss3220->regmap))
> + return PTR_ERR(hd3ss3220->regmap);
> +
> + hd3ss3220_set_source_pref(hd3ss3220,
> + HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
> + child = fwnode_graph_get_next_endpoint(dev_fwnode(hd3ss3220->dev),
> + NULL);
> + parent = fwnode_graph_get_remote_port_parent(child);
> + hd3ss3220->role_sw = fwnode_usb_role_switch_get(parent);
> + if (IS_ERR_OR_NULL(hd3ss3220->role_sw)) {
> + fwnode_handle_put(child);
> + fwnode_handle_put(parent);
> + return PTR_ERR(hd3ss3220->role_sw);
> + }
> +
> + fwnode_handle_put(child);
> + fwnode_handle_put(parent);
> +
> + hd3ss3220->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
> + hd3ss3220->typec_cap.dr_set = hd3ss3220_dr_set;
> + hd3ss3220->typec_cap.type = TYPEC_PORT_DRP;
> + hd3ss3220->typec_cap.data = TYPEC_PORT_DRD;
> +
> + hd3ss3220->port = typec_register_port(&client->dev,
> + &hd3ss3220->typec_cap);
> + if (IS_ERR(hd3ss3220->port))
> + return PTR_ERR(hd3ss3220->port);
> +
> + hd3ss3220_set_role(hd3ss3220);
> + 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;
> + }
> + 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;
> + }
I would move the above if (client->irq > 0) case to it's own function.
> + ret = i2c_smbus_read_byte_data(client, HD3SS3220_REG_DEV_REV);
> + if (ret < 0)
> + goto error;
> +
> + dev_info(&client->dev, "probed revision=0x%x\n", ret);
> +
> + return 0;
> +error:
> + typec_unregister_port(hd3ss3220->port);
> + usb_role_switch_put(hd3ss3220->role_sw);
> +
> + return ret;
> +}
> +
> +static int hd3ss3220_remove(struct i2c_client *client)
> +{
> + struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
> +
> + typec_unregister_port(hd3ss3220->port);
> + usb_role_switch_put(hd3ss3220->role_sw);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id dev_ids[] = {
> + { .compatible = "ti,hd3ss3220"},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, dev_ids);
> +
> +static struct i2c_driver hd3ss3220_driver = {
> + .driver = {
> + .name = "hd3ss3220",
> + .of_match_table = of_match_ptr(dev_ids),
> + },
> + .probe = hd3ss3220_probe,
> + .remove = hd3ss3220_remove,
> +};
> +
> +module_i2c_driver(hd3ss3220_driver);
> +
> +MODULE_AUTHOR("Biju Das <biju.das@bp.renesas.com>");
> +MODULE_DESCRIPTION("TI HD3SS3220 DRP Port Controller Driver");
> +MODULE_LICENSE("GPL");
> --
> 2.7.4
thanks,
--
heikki
^ permalink raw reply [flat|nested] 25+ messages in thread
* [V4,3/7] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller
@ 2019-04-17 9:57 ` Heikki Krogerus
0 siblings, 0 replies; 25+ messages in thread
From: Heikki Krogerus @ 2019-04-17 9:57 UTC (permalink / raw)
To: Biju Das
Cc: Chunfeng Yun, Greg Kroah-Hartman, Felipe Balbi, linux-usb,
Simon Horman, Yoshihiro Shimoda, Geert Uytterhoeven,
Chris Paterson, Fabrizio Castro, linux-renesas-soc
On Tue, Apr 16, 2019 at 10:38:03AM +0100, 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>
It's too bad they call it "port controller" even though it's not Port
Controller Interface compliant. I put a few minor nitpicks below.
Otherwise this looks okay to me, so if there are no other comments:
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> Note: This patch has compilation dependency on
> https://patchwork.kernel.org/patch/10882555/
>
> V3-->V4
> * Incorporated Chunfeng Yun's review comment
> * Used fwnode API's to get usb role switch handle.
>
> V2-->V3
> * Used the new api "usb_role_switch by node" for getting
> remote endpoint associated with Type-C USB DRP port
> controller devices.
> V1-->V2
> * Driver uses usb role class instead of extcon for dual role switch
> and also handles connect/disconnect events.
> ---
> drivers/usb/typec/Kconfig | 10 ++
> drivers/usb/typec/Makefile | 1 +
> drivers/usb/typec/hd3ss3220.c | 263 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 274 insertions(+)
> create mode 100644 drivers/usb/typec/hd3ss3220.c
>
> diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> index 89d9193..92a3717 100644
> --- a/drivers/usb/typec/Kconfig
> +++ b/drivers/usb/typec/Kconfig
> @@ -50,6 +50,16 @@ source "drivers/usb/typec/tcpm/Kconfig"
>
> source "drivers/usb/typec/ucsi/Kconfig"
>
> +config TYPEC_HD3SS3220
> + tristate "TI HD3SS3220 Type-C DRP Port controller driver"
> + depends on I2C
> + help
> + Say Y or M here if your system has TI HD3SS3220 Type-C DRP Port
> + controller driver.
> +
> + If you choose to build this driver as a dynamically linked module, the
> + module will be called hd3ss3220.ko.
> +
> config TYPEC_TPS6598X
> tristate "TI TPS6598x USB Power Delivery controller driver"
> depends on I2C
> diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
> index 6696b72..7753a5c3 100644
> --- a/drivers/usb/typec/Makefile
> +++ b/drivers/usb/typec/Makefile
> @@ -4,5 +4,6 @@ typec-y := class.o mux.o bus.o
> obj-$(CONFIG_TYPEC) += altmodes/
> obj-$(CONFIG_TYPEC_TCPM) += tcpm/
> obj-$(CONFIG_TYPEC_UCSI) += ucsi/
> +obj-$(CONFIG_TYPEC_HD3SS3220) += hd3ss3220.o
> obj-$(CONFIG_TYPEC_TPS6598X) += tps6598x.o
> obj-$(CONFIG_TYPEC) += mux/
> diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
> new file mode 100644
> index 0000000..e98a38f
> --- /dev/null
> +++ b/drivers/usb/typec/hd3ss3220.c
> @@ -0,0 +1,263 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * TI HD3SS3220 Type-C DRP Port Controller Driver
> + *
> + * Copyright (C) 2019 Renesas Electronics Corp.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/usb/role.h>
> +#include <linux/irqreturn.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/usb/typec.h>
> +#include <linux/delay.h>
> +
> +#define HD3SS3220_REG_CN_STAT_CTRL 0x09
> +#define HD3SS3220_REG_GEN_CTRL 0x0A
> +#define HD3SS3220_REG_DEV_REV 0xA0
> +
> +/* Register HD3SS3220_REG_CN_STAT_CTRL*/
> +#define HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK (BIT(7) | BIT(6))
> +#define HD3SS3220_REG_CN_STAT_CTRL_AS_DFP BIT(6)
> +#define HD3SS3220_REG_CN_STAT_CTRL_AS_UFP BIT(7)
> +#define HD3SS3220_REG_CN_STAT_CTRL_TO_ACCESSORY (BIT(7) | BIT(6))
> +#define HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS BIT(4)
> +
> +/* Register HD3SS3220_REG_GEN_CTRL*/
> +#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK (BIT(2) | BIT(1))
> +#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT 0x00
> +#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK BIT(1)
> +#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC (BIT(2) | BIT(1))
> +
> +struct hd3ss3220 {
> + struct device *dev;
> + struct regmap *regmap;
> + struct usb_role_switch *role_sw;
> + struct typec_port *port;
> + struct typec_capability typec_cap;
> +};
> +
> +static int hd3ss3220_set_source_pref(struct hd3ss3220 *hd3ss3220, int src_pref)
> +{
> + return regmap_update_bits(hd3ss3220->regmap, HD3SS3220_REG_GEN_CTRL,
> + HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK,
> + src_pref);
> +}
> +
> +static enum usb_role hd3ss3220_get_attached_state(struct hd3ss3220 *hd3ss3220)
> +{
> + unsigned int reg_val;
> + enum usb_role attached_state;
> + int ret;
> +
> + ret = regmap_read(hd3ss3220->regmap, HD3SS3220_REG_CN_STAT_CTRL,
> + ®_val);
> + if (ret < 0)
> + return ret;
> +
> + switch (reg_val & HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK) {
> + case HD3SS3220_REG_CN_STAT_CTRL_AS_DFP:
> + attached_state = USB_ROLE_HOST;
> + break;
> + case HD3SS3220_REG_CN_STAT_CTRL_AS_UFP:
> + attached_state = USB_ROLE_DEVICE;
> + break;
It looks like your line indentation has collapsed here. I pretty sure
scripts/checkpatch.pl would have found these.
> + default:
> + attached_state = USB_ROLE_NONE;
> + }
> +
> + return attached_state;
> +}
> +
> +static int hd3ss3220_dr_set(const struct typec_capability *cap,
> + enum typec_data_role role)
> +{
> + struct hd3ss3220 *hd3ss3220 = container_of(cap, struct hd3ss3220,
> + typec_cap);
> + enum usb_role role_val;
> + int pref, ret = 0;
> +
> + if (role == TYPEC_HOST) {
> + role_val = USB_ROLE_HOST;
> + pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC;
> + } else {
> + role_val = USB_ROLE_DEVICE;
> + pref = HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK;
> + }
> +
> + ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
> + usleep_range(10, 100);
> +
> + usb_role_switch_set_role(hd3ss3220->role_sw, role_val);
> + typec_set_data_role(hd3ss3220->port, role);
> +
> + return ret;
> +}
> +
> +static void hd3ss3220_set_role(struct hd3ss3220 *hd3ss3220)
> +{
> + enum usb_role role_state = hd3ss3220_get_attached_state(hd3ss3220);
> +
> + usb_role_switch_set_role(hd3ss3220->role_sw, role_state);
> + if (role_state == USB_ROLE_NONE)
> + hd3ss3220_set_source_pref(hd3ss3220,
> + HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
> +
> + switch (role_state) {
> + case USB_ROLE_HOST:
> + typec_set_data_role(hd3ss3220->port, TYPEC_HOST);
> + break;
> + case USB_ROLE_DEVICE:
> + typec_set_data_role(hd3ss3220->port, TYPEC_DEVICE);
> + break;
> + default:
> + break;
> + }
> +}
> +
> +irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220)
> +{
> + int err;
> +
> + hd3ss3220_set_role(hd3ss3220);
> + err = regmap_update_bits_base(hd3ss3220->regmap,
> + HD3SS3220_REG_CN_STAT_CTRL,
> + HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
> + HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
> + NULL, false, true);
> + if (err < 0)
> + return IRQ_NONE;
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t hd3ss3220_irq_handler(int irq, void *data)
> +{
> + struct i2c_client *client = to_i2c_client(data);
> + struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
> +
> + return hd3ss3220_irq(hd3ss3220);
> +}
> +
> +static const struct regmap_config config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = 0x0A,
> +};
> +
> +static int hd3ss3220_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct hd3ss3220 *hd3ss3220;
> + struct fwnode_handle *parent, *child;
> + int ret;
> + unsigned int data;
> +
> + hd3ss3220 = devm_kzalloc(&client->dev, sizeof(struct hd3ss3220),
> + GFP_KERNEL);
> + if (!hd3ss3220)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(client, hd3ss3220);
> +
> + hd3ss3220->dev = &client->dev;
> + hd3ss3220->regmap = devm_regmap_init_i2c(client, &config);
> + if (IS_ERR(hd3ss3220->regmap))
> + return PTR_ERR(hd3ss3220->regmap);
> +
> + hd3ss3220_set_source_pref(hd3ss3220,
> + HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
> + child = fwnode_graph_get_next_endpoint(dev_fwnode(hd3ss3220->dev),
> + NULL);
> + parent = fwnode_graph_get_remote_port_parent(child);
> + hd3ss3220->role_sw = fwnode_usb_role_switch_get(parent);
> + if (IS_ERR_OR_NULL(hd3ss3220->role_sw)) {
> + fwnode_handle_put(child);
> + fwnode_handle_put(parent);
> + return PTR_ERR(hd3ss3220->role_sw);
> + }
> +
> + fwnode_handle_put(child);
> + fwnode_handle_put(parent);
> +
> + hd3ss3220->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
> + hd3ss3220->typec_cap.dr_set = hd3ss3220_dr_set;
> + hd3ss3220->typec_cap.type = TYPEC_PORT_DRP;
> + hd3ss3220->typec_cap.data = TYPEC_PORT_DRD;
> +
> + hd3ss3220->port = typec_register_port(&client->dev,
> + &hd3ss3220->typec_cap);
> + if (IS_ERR(hd3ss3220->port))
> + return PTR_ERR(hd3ss3220->port);
> +
> + hd3ss3220_set_role(hd3ss3220);
> + 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;
> + }
> + 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;
> + }
I would move the above if (client->irq > 0) case to it's own function.
> + ret = i2c_smbus_read_byte_data(client, HD3SS3220_REG_DEV_REV);
> + if (ret < 0)
> + goto error;
> +
> + dev_info(&client->dev, "probed revision=0x%x\n", ret);
> +
> + return 0;
> +error:
> + typec_unregister_port(hd3ss3220->port);
> + usb_role_switch_put(hd3ss3220->role_sw);
> +
> + return ret;
> +}
> +
> +static int hd3ss3220_remove(struct i2c_client *client)
> +{
> + struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
> +
> + typec_unregister_port(hd3ss3220->port);
> + usb_role_switch_put(hd3ss3220->role_sw);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id dev_ids[] = {
> + { .compatible = "ti,hd3ss3220"},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, dev_ids);
> +
> +static struct i2c_driver hd3ss3220_driver = {
> + .driver = {
> + .name = "hd3ss3220",
> + .of_match_table = of_match_ptr(dev_ids),
> + },
> + .probe = hd3ss3220_probe,
> + .remove = hd3ss3220_remove,
> +};
> +
> +module_i2c_driver(hd3ss3220_driver);
> +
> +MODULE_AUTHOR("Biju Das <biju.das@bp.renesas.com>");
> +MODULE_DESCRIPTION("TI HD3SS3220 DRP Port Controller Driver");
> +MODULE_LICENSE("GPL");
> --
> 2.7.4
thanks,
^ permalink raw reply [flat|nested] 25+ messages in thread
* RE: [PATCH V4 3/7] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller
@ 2019-04-17 11:07 ` Biju Das
0 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-17 11:07 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Chunfeng Yun, Greg Kroah-Hartman, Felipe Balbi, linux-usb,
Simon Horman, Yoshihiro Shimoda, Geert Uytterhoeven,
Chris Paterson, Fabrizio Castro, linux-renesas-soc
Hi Heikki,
Thanks for the feedback.
> Subject: Re: [PATCH V4 3/7] usb: typec: driver for TI HD3SS3220 USB Type-C
> DRP port controller
>
> On Tue, Apr 16, 2019 at 10:38:03AM +0100, 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>
>
> It's too bad they call it "port controller" even though it's not Port Controller
> Interface compliant. I put a few minor nitpicks below.
> Otherwise this looks okay to me, so if there are no other comments:
>
> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>
> > ---
> > Note: This patch has compilation dependency on
> > https://patchwork.kernel.org/patch/10882555/
> >
> > V3-->V4
> > * Incorporated Chunfeng Yun's review comment
> > * Used fwnode API's to get usb role switch handle.
> >
> > V2-->V3
> > * Used the new api "usb_role_switch by node" for getting
> > remote endpoint associated with Type-C USB DRP port
> > controller devices.
> > V1-->V2
> > * Driver uses usb role class instead of extcon for dual role switch
> > and also handles connect/disconnect events.
> > ---
> > drivers/usb/typec/Kconfig | 10 ++
> > drivers/usb/typec/Makefile | 1 +
> > drivers/usb/typec/hd3ss3220.c | 263
> > ++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 274 insertions(+)
> > create mode 100644 drivers/usb/typec/hd3ss3220.c
> >
> > diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> > index 89d9193..92a3717 100644
> > --- a/drivers/usb/typec/Kconfig
> > +++ b/drivers/usb/typec/Kconfig
> > @@ -50,6 +50,16 @@ source "drivers/usb/typec/tcpm/Kconfig"
> >
> > source "drivers/usb/typec/ucsi/Kconfig"
> >
> > +config TYPEC_HD3SS3220
> > + tristate "TI HD3SS3220 Type-C DRP Port controller driver"
> > + depends on I2C
> > + help
> > + Say Y or M here if your system has TI HD3SS3220 Type-C DRP Port
> > + controller driver.
> > +
> > + If you choose to build this driver as a dynamically linked module, the
> > + module will be called hd3ss3220.ko.
> > +
> > config TYPEC_TPS6598X
> > tristate "TI TPS6598x USB Power Delivery controller driver"
> > depends on I2C
> > diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
> > index 6696b72..7753a5c3 100644
> > --- a/drivers/usb/typec/Makefile
> > +++ b/drivers/usb/typec/Makefile
> > @@ -4,5 +4,6 @@ typec-y := class.o mux.o bus.o
> > obj-$(CONFIG_TYPEC) += altmodes/
> > obj-$(CONFIG_TYPEC_TCPM) += tcpm/
> > obj-$(CONFIG_TYPEC_UCSI) += ucsi/
> > +obj-$(CONFIG_TYPEC_HD3SS3220) += hd3ss3220.o
> > obj-$(CONFIG_TYPEC_TPS6598X) += tps6598x.o
> > obj-$(CONFIG_TYPEC) += mux/
> > diff --git a/drivers/usb/typec/hd3ss3220.c
> > b/drivers/usb/typec/hd3ss3220.c new file mode 100644 index
> > 0000000..e98a38f
> > --- /dev/null
> > +++ b/drivers/usb/typec/hd3ss3220.c
> > @@ -0,0 +1,263 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * TI HD3SS3220 Type-C DRP Port Controller Driver
> > + *
> > + * Copyright (C) 2019 Renesas Electronics Corp.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/i2c.h>
> > +#include <linux/usb/role.h>
> > +#include <linux/irqreturn.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +#include <linux/slab.h>
> > +#include <linux/usb/typec.h>
> > +#include <linux/delay.h>
> > +
> > +#define HD3SS3220_REG_CN_STAT_CTRL 0x09
> > +#define HD3SS3220_REG_GEN_CTRL 0x0A
> > +#define HD3SS3220_REG_DEV_REV 0xA0
> > +
> > +/* Register HD3SS3220_REG_CN_STAT_CTRL*/
> > +#define HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK
> (BIT(7) | BIT(6))
> > +#define HD3SS3220_REG_CN_STAT_CTRL_AS_DFP BIT(6)
> > +#define HD3SS3220_REG_CN_STAT_CTRL_AS_UFP BIT(7)
> > +#define HD3SS3220_REG_CN_STAT_CTRL_TO_ACCESSORY
> (BIT(7) | BIT(6))
> > +#define HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS BIT(4)
> > +
> > +/* Register HD3SS3220_REG_GEN_CTRL*/
> > +#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK
> (BIT(2) | BIT(1))
> > +#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT 0x00
> > +#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK BIT(1)
> > +#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC
> (BIT(2) | BIT(1))
> > +
> > +struct hd3ss3220 {
> > + struct device *dev;
> > + struct regmap *regmap;
> > + struct usb_role_switch *role_sw;
> > + struct typec_port *port;
> > + struct typec_capability typec_cap;
> > +};
> > +
> > +static int hd3ss3220_set_source_pref(struct hd3ss3220 *hd3ss3220, int
> > +src_pref) {
> > + return regmap_update_bits(hd3ss3220->regmap,
> HD3SS3220_REG_GEN_CTRL,
> > +
> HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK,
> > + src_pref);
> > +}
> > +
> > +static enum usb_role hd3ss3220_get_attached_state(struct hd3ss3220
> > +*hd3ss3220) {
> > + unsigned int reg_val;
> > + enum usb_role attached_state;
> > + int ret;
> > +
> > + ret = regmap_read(hd3ss3220->regmap,
> HD3SS3220_REG_CN_STAT_CTRL,
> > + ®_val);
> > + if (ret < 0)
> > + return ret;
> > +
> > + switch (reg_val &
> HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK) {
> > + case HD3SS3220_REG_CN_STAT_CTRL_AS_DFP:
> > + attached_state = USB_ROLE_HOST;
> > + break;
> > + case HD3SS3220_REG_CN_STAT_CTRL_AS_UFP:
> > + attached_state = USB_ROLE_DEVICE;
> > + break;
>
> It looks like your line indentation has collapsed here. I pretty sure
> scripts/checkpatch.pl would have found these.
OK. Will fix this in V5. I have ran Check patch before sending the patch and it didn't complain this.
> > + default:
> > + attached_state = USB_ROLE_NONE;
> > + }
> > +
> > + return attached_state;
> > +}
> > +
> > +static int hd3ss3220_dr_set(const struct typec_capability *cap,
> > + enum typec_data_role role)
> > +{
> > + struct hd3ss3220 *hd3ss3220 = container_of(cap, struct hd3ss3220,
> > + typec_cap);
> > + enum usb_role role_val;
> > + int pref, ret = 0;
> > +
> > + if (role == TYPEC_HOST) {
> > + role_val = USB_ROLE_HOST;
> > + pref =
> HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC;
> > + } else {
> > + role_val = USB_ROLE_DEVICE;
> > + pref =
> HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK;
> > + }
> > +
> > + ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
> > + usleep_range(10, 100);
> > +
> > + usb_role_switch_set_role(hd3ss3220->role_sw, role_val);
> > + typec_set_data_role(hd3ss3220->port, role);
> > +
> > + return ret;
> > +}
> > +
> > +static void hd3ss3220_set_role(struct hd3ss3220 *hd3ss3220) {
> > + enum usb_role role_state =
> hd3ss3220_get_attached_state(hd3ss3220);
> > +
> > + usb_role_switch_set_role(hd3ss3220->role_sw, role_state);
> > + if (role_state == USB_ROLE_NONE)
> > + hd3ss3220_set_source_pref(hd3ss3220,
> > +
> HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
> > +
> > + switch (role_state) {
> > + case USB_ROLE_HOST:
> > + typec_set_data_role(hd3ss3220->port, TYPEC_HOST);
> > + break;
> > + case USB_ROLE_DEVICE:
> > + typec_set_data_role(hd3ss3220->port, TYPEC_DEVICE);
> > + break;
> > + default:
> > + break;
> > + }
> > +}
> > +
> > +irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220) {
> > + int err;
> > +
> > + hd3ss3220_set_role(hd3ss3220);
> > + err = regmap_update_bits_base(hd3ss3220->regmap,
> > + HD3SS3220_REG_CN_STAT_CTRL,
> > +
> HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
> > +
> HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
> > + NULL, false, true);
> > + if (err < 0)
> > + return IRQ_NONE;
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static irqreturn_t hd3ss3220_irq_handler(int irq, void *data) {
> > + struct i2c_client *client = to_i2c_client(data);
> > + struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
> > +
> > + return hd3ss3220_irq(hd3ss3220);
> > +}
> > +
> > +static const struct regmap_config config = {
> > + .reg_bits = 8,
> > + .val_bits = 8,
> > + .max_register = 0x0A,
> > +};
> > +
> > +static int hd3ss3220_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct hd3ss3220 *hd3ss3220;
> > + struct fwnode_handle *parent, *child;
> > + int ret;
> > + unsigned int data;
> > +
> > + hd3ss3220 = devm_kzalloc(&client->dev, sizeof(struct hd3ss3220),
> > + GFP_KERNEL);
> > + if (!hd3ss3220)
> > + return -ENOMEM;
> > +
> > + i2c_set_clientdata(client, hd3ss3220);
> > +
> > + hd3ss3220->dev = &client->dev;
> > + hd3ss3220->regmap = devm_regmap_init_i2c(client, &config);
> > + if (IS_ERR(hd3ss3220->regmap))
> > + return PTR_ERR(hd3ss3220->regmap);
> > +
> > + hd3ss3220_set_source_pref(hd3ss3220,
> > +
> HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
> > + child = fwnode_graph_get_next_endpoint(dev_fwnode(hd3ss3220-
> >dev),
> > + NULL);
> > + parent = fwnode_graph_get_remote_port_parent(child);
> > + hd3ss3220->role_sw = fwnode_usb_role_switch_get(parent);
> > + if (IS_ERR_OR_NULL(hd3ss3220->role_sw)) {
> > + fwnode_handle_put(child);
> > + fwnode_handle_put(parent);
> > + return PTR_ERR(hd3ss3220->role_sw);
> > + }
> > +
> > + fwnode_handle_put(child);
> > + fwnode_handle_put(parent);
> > +
> > + hd3ss3220->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
> > + hd3ss3220->typec_cap.dr_set = hd3ss3220_dr_set;
> > + hd3ss3220->typec_cap.type = TYPEC_PORT_DRP;
> > + hd3ss3220->typec_cap.data = TYPEC_PORT_DRD;
> > +
> > + hd3ss3220->port = typec_register_port(&client->dev,
> > + &hd3ss3220->typec_cap);
> > + if (IS_ERR(hd3ss3220->port))
> > + return PTR_ERR(hd3ss3220->port);
> > +
> > + hd3ss3220_set_role(hd3ss3220);
> > + 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;
> > + }
> > + 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;
> > + }
>
> I would move the above if (client->irq > 0) case to it's own function.
OK. Will fix this in V5.
Regards,
Biju
> > + ret = i2c_smbus_read_byte_data(client, HD3SS3220_REG_DEV_REV);
> > + if (ret < 0)
> > + goto error;
> > +
> > + dev_info(&client->dev, "probed revision=0x%x\n", ret);
> > +
> > + return 0;
> > +error:
> > + typec_unregister_port(hd3ss3220->port);
> > + usb_role_switch_put(hd3ss3220->role_sw);
> > +
> > + return ret;
> > +}
> > +
> > +static int hd3ss3220_remove(struct i2c_client *client) {
> > + struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
> > +
> > + typec_unregister_port(hd3ss3220->port);
> > + usb_role_switch_put(hd3ss3220->role_sw);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id dev_ids[] = {
> > + { .compatible = "ti,hd3ss3220"},
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, dev_ids);
> > +
> > +static struct i2c_driver hd3ss3220_driver = {
> > + .driver = {
> > + .name = "hd3ss3220",
> > + .of_match_table = of_match_ptr(dev_ids),
> > + },
> > + .probe = hd3ss3220_probe,
> > + .remove = hd3ss3220_remove,
> > +};
> > +
> > +module_i2c_driver(hd3ss3220_driver);
> > +
> > +MODULE_AUTHOR("Biju Das <biju.das@bp.renesas.com>");
> > +MODULE_DESCRIPTION("TI HD3SS3220 DRP Port Controller Driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.7.4
>
> thanks,
>
> --
> heikki
^ permalink raw reply [flat|nested] 25+ messages in thread
* [V4,3/7] usb: typec: driver for TI HD3SS3220 USB Type-C DRP port controller
@ 2019-04-17 11:07 ` Biju Das
0 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-17 11:07 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Chunfeng Yun, Greg Kroah-Hartman, Felipe Balbi, linux-usb,
Simon Horman, Yoshihiro Shimoda, Geert Uytterhoeven,
Chris Paterson, Fabrizio Castro, linux-renesas-soc
Hi Heikki,
Thanks for the feedback.
> Subject: Re: [PATCH V4 3/7] usb: typec: driver for TI HD3SS3220 USB Type-C
> DRP port controller
>
> On Tue, Apr 16, 2019 at 10:38:03AM +0100, 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>
>
> It's too bad they call it "port controller" even though it's not Port Controller
> Interface compliant. I put a few minor nitpicks below.
> Otherwise this looks okay to me, so if there are no other comments:
>
> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>
> > ---
> > Note: This patch has compilation dependency on
> > https://patchwork.kernel.org/patch/10882555/
> >
> > V3-->V4
> > * Incorporated Chunfeng Yun's review comment
> > * Used fwnode API's to get usb role switch handle.
> >
> > V2-->V3
> > * Used the new api "usb_role_switch by node" for getting
> > remote endpoint associated with Type-C USB DRP port
> > controller devices.
> > V1-->V2
> > * Driver uses usb role class instead of extcon for dual role switch
> > and also handles connect/disconnect events.
> > ---
> > drivers/usb/typec/Kconfig | 10 ++
> > drivers/usb/typec/Makefile | 1 +
> > drivers/usb/typec/hd3ss3220.c | 263
> > ++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 274 insertions(+)
> > create mode 100644 drivers/usb/typec/hd3ss3220.c
> >
> > diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
> > index 89d9193..92a3717 100644
> > --- a/drivers/usb/typec/Kconfig
> > +++ b/drivers/usb/typec/Kconfig
> > @@ -50,6 +50,16 @@ source "drivers/usb/typec/tcpm/Kconfig"
> >
> > source "drivers/usb/typec/ucsi/Kconfig"
> >
> > +config TYPEC_HD3SS3220
> > + tristate "TI HD3SS3220 Type-C DRP Port controller driver"
> > + depends on I2C
> > + help
> > + Say Y or M here if your system has TI HD3SS3220 Type-C DRP Port
> > + controller driver.
> > +
> > + If you choose to build this driver as a dynamically linked module, the
> > + module will be called hd3ss3220.ko.
> > +
> > config TYPEC_TPS6598X
> > tristate "TI TPS6598x USB Power Delivery controller driver"
> > depends on I2C
> > diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
> > index 6696b72..7753a5c3 100644
> > --- a/drivers/usb/typec/Makefile
> > +++ b/drivers/usb/typec/Makefile
> > @@ -4,5 +4,6 @@ typec-y := class.o mux.o bus.o
> > obj-$(CONFIG_TYPEC) += altmodes/
> > obj-$(CONFIG_TYPEC_TCPM) += tcpm/
> > obj-$(CONFIG_TYPEC_UCSI) += ucsi/
> > +obj-$(CONFIG_TYPEC_HD3SS3220) += hd3ss3220.o
> > obj-$(CONFIG_TYPEC_TPS6598X) += tps6598x.o
> > obj-$(CONFIG_TYPEC) += mux/
> > diff --git a/drivers/usb/typec/hd3ss3220.c
> > b/drivers/usb/typec/hd3ss3220.c new file mode 100644 index
> > 0000000..e98a38f
> > --- /dev/null
> > +++ b/drivers/usb/typec/hd3ss3220.c
> > @@ -0,0 +1,263 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * TI HD3SS3220 Type-C DRP Port Controller Driver
> > + *
> > + * Copyright (C) 2019 Renesas Electronics Corp.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/i2c.h>
> > +#include <linux/usb/role.h>
> > +#include <linux/irqreturn.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +#include <linux/slab.h>
> > +#include <linux/usb/typec.h>
> > +#include <linux/delay.h>
> > +
> > +#define HD3SS3220_REG_CN_STAT_CTRL 0x09
> > +#define HD3SS3220_REG_GEN_CTRL 0x0A
> > +#define HD3SS3220_REG_DEV_REV 0xA0
> > +
> > +/* Register HD3SS3220_REG_CN_STAT_CTRL*/
> > +#define HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK
> (BIT(7) | BIT(6))
> > +#define HD3SS3220_REG_CN_STAT_CTRL_AS_DFP BIT(6)
> > +#define HD3SS3220_REG_CN_STAT_CTRL_AS_UFP BIT(7)
> > +#define HD3SS3220_REG_CN_STAT_CTRL_TO_ACCESSORY
> (BIT(7) | BIT(6))
> > +#define HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS BIT(4)
> > +
> > +/* Register HD3SS3220_REG_GEN_CTRL*/
> > +#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK
> (BIT(2) | BIT(1))
> > +#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT 0x00
> > +#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK BIT(1)
> > +#define HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC
> (BIT(2) | BIT(1))
> > +
> > +struct hd3ss3220 {
> > + struct device *dev;
> > + struct regmap *regmap;
> > + struct usb_role_switch *role_sw;
> > + struct typec_port *port;
> > + struct typec_capability typec_cap;
> > +};
> > +
> > +static int hd3ss3220_set_source_pref(struct hd3ss3220 *hd3ss3220, int
> > +src_pref) {
> > + return regmap_update_bits(hd3ss3220->regmap,
> HD3SS3220_REG_GEN_CTRL,
> > +
> HD3SS3220_REG_GEN_CTRL_SRC_PREF_MASK,
> > + src_pref);
> > +}
> > +
> > +static enum usb_role hd3ss3220_get_attached_state(struct hd3ss3220
> > +*hd3ss3220) {
> > + unsigned int reg_val;
> > + enum usb_role attached_state;
> > + int ret;
> > +
> > + ret = regmap_read(hd3ss3220->regmap,
> HD3SS3220_REG_CN_STAT_CTRL,
> > + ®_val);
> > + if (ret < 0)
> > + return ret;
> > +
> > + switch (reg_val &
> HD3SS3220_REG_CN_STAT_CTRL_ATTACHED_STATE_MASK) {
> > + case HD3SS3220_REG_CN_STAT_CTRL_AS_DFP:
> > + attached_state = USB_ROLE_HOST;
> > + break;
> > + case HD3SS3220_REG_CN_STAT_CTRL_AS_UFP:
> > + attached_state = USB_ROLE_DEVICE;
> > + break;
>
> It looks like your line indentation has collapsed here. I pretty sure
> scripts/checkpatch.pl would have found these.
OK. Will fix this in V5. I have ran Check patch before sending the patch and it didn't complain this.
> > + default:
> > + attached_state = USB_ROLE_NONE;
> > + }
> > +
> > + return attached_state;
> > +}
> > +
> > +static int hd3ss3220_dr_set(const struct typec_capability *cap,
> > + enum typec_data_role role)
> > +{
> > + struct hd3ss3220 *hd3ss3220 = container_of(cap, struct hd3ss3220,
> > + typec_cap);
> > + enum usb_role role_val;
> > + int pref, ret = 0;
> > +
> > + if (role == TYPEC_HOST) {
> > + role_val = USB_ROLE_HOST;
> > + pref =
> HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SRC;
> > + } else {
> > + role_val = USB_ROLE_DEVICE;
> > + pref =
> HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_TRY_SNK;
> > + }
> > +
> > + ret = hd3ss3220_set_source_pref(hd3ss3220, pref);
> > + usleep_range(10, 100);
> > +
> > + usb_role_switch_set_role(hd3ss3220->role_sw, role_val);
> > + typec_set_data_role(hd3ss3220->port, role);
> > +
> > + return ret;
> > +}
> > +
> > +static void hd3ss3220_set_role(struct hd3ss3220 *hd3ss3220) {
> > + enum usb_role role_state =
> hd3ss3220_get_attached_state(hd3ss3220);
> > +
> > + usb_role_switch_set_role(hd3ss3220->role_sw, role_state);
> > + if (role_state == USB_ROLE_NONE)
> > + hd3ss3220_set_source_pref(hd3ss3220,
> > +
> HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
> > +
> > + switch (role_state) {
> > + case USB_ROLE_HOST:
> > + typec_set_data_role(hd3ss3220->port, TYPEC_HOST);
> > + break;
> > + case USB_ROLE_DEVICE:
> > + typec_set_data_role(hd3ss3220->port, TYPEC_DEVICE);
> > + break;
> > + default:
> > + break;
> > + }
> > +}
> > +
> > +irqreturn_t hd3ss3220_irq(struct hd3ss3220 *hd3ss3220) {
> > + int err;
> > +
> > + hd3ss3220_set_role(hd3ss3220);
> > + err = regmap_update_bits_base(hd3ss3220->regmap,
> > + HD3SS3220_REG_CN_STAT_CTRL,
> > +
> HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
> > +
> HD3SS3220_REG_CN_STAT_CTRL_INT_STATUS,
> > + NULL, false, true);
> > + if (err < 0)
> > + return IRQ_NONE;
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static irqreturn_t hd3ss3220_irq_handler(int irq, void *data) {
> > + struct i2c_client *client = to_i2c_client(data);
> > + struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
> > +
> > + return hd3ss3220_irq(hd3ss3220);
> > +}
> > +
> > +static const struct regmap_config config = {
> > + .reg_bits = 8,
> > + .val_bits = 8,
> > + .max_register = 0x0A,
> > +};
> > +
> > +static int hd3ss3220_probe(struct i2c_client *client,
> > + const struct i2c_device_id *id)
> > +{
> > + struct hd3ss3220 *hd3ss3220;
> > + struct fwnode_handle *parent, *child;
> > + int ret;
> > + unsigned int data;
> > +
> > + hd3ss3220 = devm_kzalloc(&client->dev, sizeof(struct hd3ss3220),
> > + GFP_KERNEL);
> > + if (!hd3ss3220)
> > + return -ENOMEM;
> > +
> > + i2c_set_clientdata(client, hd3ss3220);
> > +
> > + hd3ss3220->dev = &client->dev;
> > + hd3ss3220->regmap = devm_regmap_init_i2c(client, &config);
> > + if (IS_ERR(hd3ss3220->regmap))
> > + return PTR_ERR(hd3ss3220->regmap);
> > +
> > + hd3ss3220_set_source_pref(hd3ss3220,
> > +
> HD3SS3220_REG_GEN_CTRL_SRC_PREF_DRP_DEFAULT);
> > + child = fwnode_graph_get_next_endpoint(dev_fwnode(hd3ss3220-
> >dev),
> > + NULL);
> > + parent = fwnode_graph_get_remote_port_parent(child);
> > + hd3ss3220->role_sw = fwnode_usb_role_switch_get(parent);
> > + if (IS_ERR_OR_NULL(hd3ss3220->role_sw)) {
> > + fwnode_handle_put(child);
> > + fwnode_handle_put(parent);
> > + return PTR_ERR(hd3ss3220->role_sw);
> > + }
> > +
> > + fwnode_handle_put(child);
> > + fwnode_handle_put(parent);
> > +
> > + hd3ss3220->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
> > + hd3ss3220->typec_cap.dr_set = hd3ss3220_dr_set;
> > + hd3ss3220->typec_cap.type = TYPEC_PORT_DRP;
> > + hd3ss3220->typec_cap.data = TYPEC_PORT_DRD;
> > +
> > + hd3ss3220->port = typec_register_port(&client->dev,
> > + &hd3ss3220->typec_cap);
> > + if (IS_ERR(hd3ss3220->port))
> > + return PTR_ERR(hd3ss3220->port);
> > +
> > + hd3ss3220_set_role(hd3ss3220);
> > + 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;
> > + }
> > + 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;
> > + }
>
> I would move the above if (client->irq > 0) case to it's own function.
OK. Will fix this in V5.
Regards,
Biju
> > + ret = i2c_smbus_read_byte_data(client, HD3SS3220_REG_DEV_REV);
> > + if (ret < 0)
> > + goto error;
> > +
> > + dev_info(&client->dev, "probed revision=0x%x\n", ret);
> > +
> > + return 0;
> > +error:
> > + typec_unregister_port(hd3ss3220->port);
> > + usb_role_switch_put(hd3ss3220->role_sw);
> > +
> > + return ret;
> > +}
> > +
> > +static int hd3ss3220_remove(struct i2c_client *client) {
> > + struct hd3ss3220 *hd3ss3220 = i2c_get_clientdata(client);
> > +
> > + typec_unregister_port(hd3ss3220->port);
> > + usb_role_switch_put(hd3ss3220->role_sw);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id dev_ids[] = {
> > + { .compatible = "ti,hd3ss3220"},
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, dev_ids);
> > +
> > +static struct i2c_driver hd3ss3220_driver = {
> > + .driver = {
> > + .name = "hd3ss3220",
> > + .of_match_table = of_match_ptr(dev_ids),
> > + },
> > + .probe = hd3ss3220_probe,
> > + .remove = hd3ss3220_remove,
> > +};
> > +
> > +module_i2c_driver(hd3ss3220_driver);
> > +
> > +MODULE_AUTHOR("Biju Das <biju.das@bp.renesas.com>");
> > +MODULE_DESCRIPTION("TI HD3SS3220 DRP Port Controller Driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.7.4
>
> thanks,
>
> --
> heikki
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework
@ 2019-04-16 9:38 ` Biju Das
0 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-16 9:38 UTC (permalink / raw)
To: Felipe Balbi
Cc: Biju Das, Greg Kroah-Hartman, Heikki Krogerus, Yoshihiro Shimoda,
Simon Horman, Fabrizio Castro, Kees Cook, linux-usb,
Simon Horman, Geert Uytterhoeven, Chris Paterson,
linux-renesas-soc
RZ/G2E cat874 board is capable of detecting cable connect and disconnect
events. Add support for renesas_usb3 to receive connect and disconnect
events and support dual-role switch using usb-role-switch framework.
Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
V3-->V4
* No Change
V2-->V3
* Incorporated Shimoda-san's review comment
(https://patchwork.kernel.org/patch/10852507/)
* Used renesas,usb-role-switch property for differentiating USB
role switch associated with Type-C port controller driver.
V1-->V2
* Driver uses usb role clas for handling dual role switch and handling
connect/disconnect events instead of extcon.
---
drivers/usb/gadget/udc/renesas_usb3.c | 126 ++++++++++++++++++++++++++++------
1 file changed, 104 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
index 7dc2485..efee047 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -351,6 +351,8 @@ struct renesas_usb3 {
int disabled_count;
struct usb_request *ep0_req;
+
+ enum usb_role connection_state;
u16 test_mode;
u8 ep0_buf[USB3_EP0_BUF_SIZE];
bool softconnect;
@@ -359,6 +361,7 @@ struct renesas_usb3 {
bool extcon_usb; /* check vbus and set EXTCON_USB */
bool forced_b_device;
bool start_to_connect;
+ bool usb_role_switch_property;
};
#define gadget_to_renesas_usb3(_gadget) \
@@ -644,22 +647,6 @@ static void usb3_disconnect(struct renesas_usb3 *usb3)
usb3->driver->disconnect(&usb3->gadget);
}
-static void usb3_check_vbus(struct renesas_usb3 *usb3)
-{
- if (usb3->workaround_for_vbus) {
- usb3_connect(usb3);
- } else {
- usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
- USB_STA_VBUS_STA);
- if (usb3->extcon_usb)
- usb3_connect(usb3);
- else
- usb3_disconnect(usb3);
-
- schedule_work(&usb3->extcon_work);
- }
-}
-
static void renesas_usb3_role_work(struct work_struct *work)
{
struct renesas_usb3 *usb3 =
@@ -699,8 +686,11 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, bool host, bool a_dev)
unsigned long flags;
spin_lock_irqsave(&usb3->lock, flags);
- usb3_set_mode_by_role_sw(usb3, host);
- usb3_vbus_out(usb3, a_dev);
+ if (!usb3->usb_role_switch_property ||
+ usb3->connection_state != USB_ROLE_NONE) {
+ usb3_set_mode_by_role_sw(usb3, host);
+ usb3_vbus_out(usb3, a_dev);
+ }
/* for A-Peripheral or forced B-device mode */
if ((!host && a_dev) || usb3->start_to_connect)
usb3_connect(usb3);
@@ -724,6 +714,28 @@ static void usb3_check_id(struct renesas_usb3 *usb3)
schedule_work(&usb3->extcon_work);
}
+static void usb3_check_vbus(struct renesas_usb3 *usb3)
+{
+ if (usb3->workaround_for_vbus) {
+ if (usb3->usb_role_switch_property) {
+ if (usb3->connection_state == USB_ROLE_DEVICE) {
+ usb3_mode_config(usb3, false, false);
+ usb3_connect(usb3);
+ }
+ } else
+ usb3_connect(usb3);
+ } else {
+ usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
+ USB_STA_VBUS_STA);
+ if (usb3->extcon_usb)
+ usb3_connect(usb3);
+ else
+ usb3_disconnect(usb3);
+
+ schedule_work(&usb3->extcon_work);
+ }
+}
+
static void renesas_usb3_init_controller(struct renesas_usb3 *usb3)
{
usb3_init_axi_bridge(usb3);
@@ -2343,14 +2355,65 @@ static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
return cur_role;
}
-static int renesas_usb3_role_switch_set(struct device *dev,
- enum usb_role role)
+static void handle_ext_role_switch_states(struct device *dev,
+ enum usb_role role)
+{
+ struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+ struct device *host = usb3->host_dev;
+ enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
+
+ switch (role) {
+ case USB_ROLE_NONE:
+ usb3->connection_state = USB_ROLE_NONE;
+ if (usb3->driver)
+ usb3_disconnect(usb3);
+ usb3_vbus_out(usb3, false);
+ break;
+ case USB_ROLE_DEVICE:
+ if (usb3->connection_state == USB_ROLE_NONE) {
+ usb3->connection_state = USB_ROLE_DEVICE;
+ usb3_set_mode(usb3, false);
+ if (usb3->driver)
+ usb3_connect(usb3);
+ } else if (cur_role == USB_ROLE_HOST) {
+ device_release_driver(host);
+ usb3_set_mode(usb3, false);
+ if (usb3->driver)
+ usb3_connect(usb3);
+ }
+ usb3_vbus_out(usb3, false);
+ break;
+ case USB_ROLE_HOST:
+ if (usb3->connection_state == USB_ROLE_NONE) {
+ if (usb3->driver)
+ usb3_disconnect(usb3);
+
+ usb3->connection_state = USB_ROLE_HOST;
+ usb3_set_mode(usb3, true);
+ usb3_vbus_out(usb3, true);
+ if (device_attach(host) < 0)
+ dev_err(dev, "device_attach(host) failed\n");
+ } else if (cur_role == USB_ROLE_DEVICE) {
+ usb3_disconnect(usb3);
+ /* Must set the mode before device_attach of the host */
+ usb3_set_mode(usb3, true);
+ /* This device_attach() might sleep */
+ if (device_attach(host) < 0)
+ dev_err(dev, "device_attach(host) failed\n");
+ }
+ break;
+ default:
+ break;
+ }
+}
+
+static void handle_role_switch_states(struct device *dev,
+ enum usb_role role)
{
struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
struct device *host = usb3->host_dev;
enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
- pm_runtime_get_sync(dev);
if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) {
device_release_driver(host);
usb3_set_mode(usb3, false);
@@ -2361,6 +2424,20 @@ static int renesas_usb3_role_switch_set(struct device *dev,
if (device_attach(host) < 0)
dev_err(dev, "device_attach(host) failed\n");
}
+}
+
+static int renesas_usb3_role_switch_set(struct device *dev,
+ enum usb_role role)
+{
+ struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+
+ pm_runtime_get_sync(dev);
+
+ if (usb3->usb_role_switch_property)
+ handle_ext_role_switch_states(dev, role);
+ else
+ handle_role_switch_states(dev, role);
+
pm_runtime_put(dev);
return 0;
@@ -2650,7 +2727,7 @@ static const unsigned int renesas_usb3_cable[] = {
EXTCON_NONE,
};
-static const struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
+static struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
.set = renesas_usb3_role_switch_set,
.get = renesas_usb3_role_switch_get,
.allow_userspace_control = true,
@@ -2741,6 +2818,11 @@ static int renesas_usb3_probe(struct platform_device *pdev)
if (ret < 0)
goto err_dev_create;
+ if (device_property_read_bool(&pdev->dev, "renesas,usb-role-switch")) {
+ usb3->usb_role_switch_property = true;
+ renesas_usb3_role_switch_desc.fwnode = dev_fwnode(&pdev->dev);
+ }
+
INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
usb3->role_sw = usb_role_switch_register(&pdev->dev,
&renesas_usb3_role_switch_desc);
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [V4,4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework
@ 2019-04-16 9:38 ` Biju Das
0 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-16 9:38 UTC (permalink / raw)
To: Felipe Balbi
Cc: Biju Das, Greg Kroah-Hartman, Heikki Krogerus, Yoshihiro Shimoda,
Simon Horman, Fabrizio Castro, Kees Cook, linux-usb,
Simon Horman, Geert Uytterhoeven, Chris Paterson,
linux-renesas-soc
RZ/G2E cat874 board is capable of detecting cable connect and disconnect
events. Add support for renesas_usb3 to receive connect and disconnect
events and support dual-role switch using usb-role-switch framework.
Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
V3-->V4
* No Change
V2-->V3
* Incorporated Shimoda-san's review comment
(https://patchwork.kernel.org/patch/10852507/)
* Used renesas,usb-role-switch property for differentiating USB
role switch associated with Type-C port controller driver.
V1-->V2
* Driver uses usb role clas for handling dual role switch and handling
connect/disconnect events instead of extcon.
---
drivers/usb/gadget/udc/renesas_usb3.c | 126 ++++++++++++++++++++++++++++------
1 file changed, 104 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/gadget/udc/renesas_usb3.c b/drivers/usb/gadget/udc/renesas_usb3.c
index 7dc2485..efee047 100644
--- a/drivers/usb/gadget/udc/renesas_usb3.c
+++ b/drivers/usb/gadget/udc/renesas_usb3.c
@@ -351,6 +351,8 @@ struct renesas_usb3 {
int disabled_count;
struct usb_request *ep0_req;
+
+ enum usb_role connection_state;
u16 test_mode;
u8 ep0_buf[USB3_EP0_BUF_SIZE];
bool softconnect;
@@ -359,6 +361,7 @@ struct renesas_usb3 {
bool extcon_usb; /* check vbus and set EXTCON_USB */
bool forced_b_device;
bool start_to_connect;
+ bool usb_role_switch_property;
};
#define gadget_to_renesas_usb3(_gadget) \
@@ -644,22 +647,6 @@ static void usb3_disconnect(struct renesas_usb3 *usb3)
usb3->driver->disconnect(&usb3->gadget);
}
-static void usb3_check_vbus(struct renesas_usb3 *usb3)
-{
- if (usb3->workaround_for_vbus) {
- usb3_connect(usb3);
- } else {
- usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
- USB_STA_VBUS_STA);
- if (usb3->extcon_usb)
- usb3_connect(usb3);
- else
- usb3_disconnect(usb3);
-
- schedule_work(&usb3->extcon_work);
- }
-}
-
static void renesas_usb3_role_work(struct work_struct *work)
{
struct renesas_usb3 *usb3 =
@@ -699,8 +686,11 @@ static void usb3_mode_config(struct renesas_usb3 *usb3, bool host, bool a_dev)
unsigned long flags;
spin_lock_irqsave(&usb3->lock, flags);
- usb3_set_mode_by_role_sw(usb3, host);
- usb3_vbus_out(usb3, a_dev);
+ if (!usb3->usb_role_switch_property ||
+ usb3->connection_state != USB_ROLE_NONE) {
+ usb3_set_mode_by_role_sw(usb3, host);
+ usb3_vbus_out(usb3, a_dev);
+ }
/* for A-Peripheral or forced B-device mode */
if ((!host && a_dev) || usb3->start_to_connect)
usb3_connect(usb3);
@@ -724,6 +714,28 @@ static void usb3_check_id(struct renesas_usb3 *usb3)
schedule_work(&usb3->extcon_work);
}
+static void usb3_check_vbus(struct renesas_usb3 *usb3)
+{
+ if (usb3->workaround_for_vbus) {
+ if (usb3->usb_role_switch_property) {
+ if (usb3->connection_state == USB_ROLE_DEVICE) {
+ usb3_mode_config(usb3, false, false);
+ usb3_connect(usb3);
+ }
+ } else
+ usb3_connect(usb3);
+ } else {
+ usb3->extcon_usb = !!(usb3_read(usb3, USB3_USB_STA) &
+ USB_STA_VBUS_STA);
+ if (usb3->extcon_usb)
+ usb3_connect(usb3);
+ else
+ usb3_disconnect(usb3);
+
+ schedule_work(&usb3->extcon_work);
+ }
+}
+
static void renesas_usb3_init_controller(struct renesas_usb3 *usb3)
{
usb3_init_axi_bridge(usb3);
@@ -2343,14 +2355,65 @@ static enum usb_role renesas_usb3_role_switch_get(struct device *dev)
return cur_role;
}
-static int renesas_usb3_role_switch_set(struct device *dev,
- enum usb_role role)
+static void handle_ext_role_switch_states(struct device *dev,
+ enum usb_role role)
+{
+ struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+ struct device *host = usb3->host_dev;
+ enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
+
+ switch (role) {
+ case USB_ROLE_NONE:
+ usb3->connection_state = USB_ROLE_NONE;
+ if (usb3->driver)
+ usb3_disconnect(usb3);
+ usb3_vbus_out(usb3, false);
+ break;
+ case USB_ROLE_DEVICE:
+ if (usb3->connection_state == USB_ROLE_NONE) {
+ usb3->connection_state = USB_ROLE_DEVICE;
+ usb3_set_mode(usb3, false);
+ if (usb3->driver)
+ usb3_connect(usb3);
+ } else if (cur_role == USB_ROLE_HOST) {
+ device_release_driver(host);
+ usb3_set_mode(usb3, false);
+ if (usb3->driver)
+ usb3_connect(usb3);
+ }
+ usb3_vbus_out(usb3, false);
+ break;
+ case USB_ROLE_HOST:
+ if (usb3->connection_state == USB_ROLE_NONE) {
+ if (usb3->driver)
+ usb3_disconnect(usb3);
+
+ usb3->connection_state = USB_ROLE_HOST;
+ usb3_set_mode(usb3, true);
+ usb3_vbus_out(usb3, true);
+ if (device_attach(host) < 0)
+ dev_err(dev, "device_attach(host) failed\n");
+ } else if (cur_role == USB_ROLE_DEVICE) {
+ usb3_disconnect(usb3);
+ /* Must set the mode before device_attach of the host */
+ usb3_set_mode(usb3, true);
+ /* This device_attach() might sleep */
+ if (device_attach(host) < 0)
+ dev_err(dev, "device_attach(host) failed\n");
+ }
+ break;
+ default:
+ break;
+ }
+}
+
+static void handle_role_switch_states(struct device *dev,
+ enum usb_role role)
{
struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
struct device *host = usb3->host_dev;
enum usb_role cur_role = renesas_usb3_role_switch_get(dev);
- pm_runtime_get_sync(dev);
if (cur_role == USB_ROLE_HOST && role == USB_ROLE_DEVICE) {
device_release_driver(host);
usb3_set_mode(usb3, false);
@@ -2361,6 +2424,20 @@ static int renesas_usb3_role_switch_set(struct device *dev,
if (device_attach(host) < 0)
dev_err(dev, "device_attach(host) failed\n");
}
+}
+
+static int renesas_usb3_role_switch_set(struct device *dev,
+ enum usb_role role)
+{
+ struct renesas_usb3 *usb3 = dev_get_drvdata(dev);
+
+ pm_runtime_get_sync(dev);
+
+ if (usb3->usb_role_switch_property)
+ handle_ext_role_switch_states(dev, role);
+ else
+ handle_role_switch_states(dev, role);
+
pm_runtime_put(dev);
return 0;
@@ -2650,7 +2727,7 @@ static const unsigned int renesas_usb3_cable[] = {
EXTCON_NONE,
};
-static const struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
+static struct usb_role_switch_desc renesas_usb3_role_switch_desc = {
.set = renesas_usb3_role_switch_set,
.get = renesas_usb3_role_switch_get,
.allow_userspace_control = true,
@@ -2741,6 +2818,11 @@ static int renesas_usb3_probe(struct platform_device *pdev)
if (ret < 0)
goto err_dev_create;
+ if (device_property_read_bool(&pdev->dev, "renesas,usb-role-switch")) {
+ usb3->usb_role_switch_property = true;
+ renesas_usb3_role_switch_desc.fwnode = dev_fwnode(&pdev->dev);
+ }
+
INIT_WORK(&usb3->role_work, renesas_usb3_role_work);
usb3->role_sw = usb_role_switch_register(&pdev->dev,
&renesas_usb3_role_switch_desc);
^ permalink raw reply related [flat|nested] 25+ messages in thread
* RE: [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework
@ 2019-04-23 2:27 ` Yoshihiro Shimoda
0 siblings, 0 replies; 25+ messages in thread
From: Yoshihiro Shimoda @ 2019-04-23 2:27 UTC (permalink / raw)
To: Biju Das
Cc: Biju Das, Greg Kroah-Hartman, Heikki Krogerus, Simon Horman,
Fabrizio Castro, Kees Cook, linux-usb, Simon Horman,
Geert Uytterhoeven, Chris Paterson, linux-renesas-soc,
Felipe Balbi
Hi Biju-san,
Thank you for the patch!
> From: Biju Das, Sent: Tuesday, April 16, 2019 6:38 PM
>
> RZ/G2E cat874 board is capable of detecting cable connect and disconnect
> events. Add support for renesas_usb3 to receive connect and disconnect
> events and support dual-role switch using usb-role-switch framework.
>
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
> V3-->V4
> * No Change
> V2-->V3
> * Incorporated Shimoda-san's review comment
> (https://patchwork.kernel.org/patch/10852507/)
> * Used renesas,usb-role-switch property for differentiating USB
> role switch associated with Type-C port controller driver.
> V1-->V2
> * Driver uses usb role clas for handling dual role switch and handling
> connect/disconnect events instead of extcon.
<snip>
> +static void usb3_check_vbus(struct renesas_usb3 *usb3)
> +{
> + if (usb3->workaround_for_vbus) {
> + if (usb3->usb_role_switch_property) {
> + if (usb3->connection_state == USB_ROLE_DEVICE) {
> + usb3_mode_config(usb3, false, false);
I should have pointed it out the previous version though,
why does this usb3_mode_config() calling need?
My guess is:
- renesas_usb3_start() calls renesas_usb3_init_controller().
-- renesas_usb3_init_controller() calls usb3_check_id() and then usb_check_vbus().
--- usb_check_id() calls usb3_mode_config(usb3, true, true) and then the HW acts as host mode.
----> So, you'd like the HW to acts as peripheral mode when the connection_state is USB_ROLE_DEVICE,
you added that the usb3_check_vbus() calls usb3_mode_config(usb3, false, false).
Is my guess correct? If so, I'd like to add such code into usb3_check_id() like below:
if ((usb3->extcon_host && !usb3->forced_b_device) ||
(usb3->usb_role_switch_property &&
usb3->connection_state == USB_ROLE_HOST))
usb3_mode_config(usb3, true, true);
else
usb3_mode_config(usb3, false, false);
What do you think?
Best regards,
Yoshihiro Shimoda
^ permalink raw reply [flat|nested] 25+ messages in thread
* [V4,4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework
@ 2019-04-23 2:27 ` Yoshihiro Shimoda
0 siblings, 0 replies; 25+ messages in thread
From: Yoshihiro Shimoda @ 2019-04-23 2:27 UTC (permalink / raw)
To: Biju Das
Cc: Greg Kroah-Hartman, Heikki Krogerus, Simon Horman,
Fabrizio Castro, Kees Cook, linux-usb, Simon Horman,
Geert Uytterhoeven, Chris Paterson, linux-renesas-soc,
Felipe Balbi
Hi Biju-san,
Thank you for the patch!
> From: Biju Das, Sent: Tuesday, April 16, 2019 6:38 PM
>
> RZ/G2E cat874 board is capable of detecting cable connect and disconnect
> events. Add support for renesas_usb3 to receive connect and disconnect
> events and support dual-role switch using usb-role-switch framework.
>
> Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> ---
> V3-->V4
> * No Change
> V2-->V3
> * Incorporated Shimoda-san's review comment
> (https://patchwork.kernel.org/patch/10852507/)
> * Used renesas,usb-role-switch property for differentiating USB
> role switch associated with Type-C port controller driver.
> V1-->V2
> * Driver uses usb role clas for handling dual role switch and handling
> connect/disconnect events instead of extcon.
<snip>
> +static void usb3_check_vbus(struct renesas_usb3 *usb3)
> +{
> + if (usb3->workaround_for_vbus) {
> + if (usb3->usb_role_switch_property) {
> + if (usb3->connection_state == USB_ROLE_DEVICE) {
> + usb3_mode_config(usb3, false, false);
I should have pointed it out the previous version though,
why does this usb3_mode_config() calling need?
My guess is:
- renesas_usb3_start() calls renesas_usb3_init_controller().
-- renesas_usb3_init_controller() calls usb3_check_id() and then usb_check_vbus().
--- usb_check_id() calls usb3_mode_config(usb3, true, true) and then the HW acts as host mode.
----> So, you'd like the HW to acts as peripheral mode when the connection_state is USB_ROLE_DEVICE,
you added that the usb3_check_vbus() calls usb3_mode_config(usb3, false, false).
Is my guess correct? If so, I'd like to add such code into usb3_check_id() like below:
if ((usb3->extcon_host && !usb3->forced_b_device) ||
(usb3->usb_role_switch_property &&
usb3->connection_state == USB_ROLE_HOST))
usb3_mode_config(usb3, true, true);
else
usb3_mode_config(usb3, false, false);
What do you think?
Best regards,
Yoshihiro Shimoda
^ permalink raw reply [flat|nested] 25+ messages in thread
* RE: [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework
@ 2019-04-23 9:07 ` Biju Das
0 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-23 9:07 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Greg Kroah-Hartman, Heikki Krogerus, Simon Horman,
Fabrizio Castro, Kees Cook, linux-usb, Simon Horman,
Geert Uytterhoeven, Chris Paterson, linux-renesas-soc,
Felipe Balbi
Hi Shimoda-San,
Thanks for the feedback.
> Subject: RE: [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use
> usb_role_switch framework
>
> Hi Biju-san,
>
> Thank you for the patch!
>
> > From: Biju Das, Sent: Tuesday, April 16, 2019 6:38 PM
> >
> > RZ/G2E cat874 board is capable of detecting cable connect and
> > disconnect events. Add support for renesas_usb3 to receive connect and
> > disconnect events and support dual-role switch using usb-role-switch
> framework.
> >
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > ---
> > V3-->V4
> > * No Change
> > V2-->V3
> > * Incorporated Shimoda-san's review comment
> > (https://patchwork.kernel.org/patch/10852507/)
> > * Used renesas,usb-role-switch property for differentiating USB
> > role switch associated with Type-C port controller driver.
> > V1-->V2
> > * Driver uses usb role clas for handling dual role switch and handling
> > connect/disconnect events instead of extcon.
> <snip>
>
> > +static void usb3_check_vbus(struct renesas_usb3 *usb3) {
> > + if (usb3->workaround_for_vbus) {
> > + if (usb3->usb_role_switch_property) {
> > + if (usb3->connection_state == USB_ROLE_DEVICE) {
> > + usb3_mode_config(usb3, false, false);
>
> I should have pointed it out the previous version though, why does this
> usb3_mode_config() calling need?
> My guess is:
> - renesas_usb3_start() calls renesas_usb3_init_controller().
> -- renesas_usb3_init_controller() calls usb3_check_id() and then
> usb_check_vbus().
> --- usb_check_id() calls usb3_mode_config(usb3, true, true) and then the
> HW acts as host mode.
> ----> So, you'd like the HW to acts as peripheral mode when the
> connection_state is USB_ROLE_DEVICE,
> you added that the usb3_check_vbus() calls usb3_mode_config(usb3,
> false, false).
>
> Is my guess correct? If so, I'd like to add such code into usb3_check_id() like
> below:
Yes, it is almost correct. The scenario I am trying is
[1] USB type C cable connected to a Host Machine(TI chip identifies as Device connection. But we haven't installed Gadget module for Device operation)
[2] After that trying to install gadget module. In this case, it calls usb_check_id() as mentioned above
and configure it as Host mode.
> if ((usb3->extcon_host && !usb3->forced_b_device) ||
> (usb3->usb_role_switch_property &&
> usb3->connection_state == USB_ROLE_HOST))
> usb3_mode_config(usb3, true, true);
> else
> usb3_mode_config(usb3, false, false);
>
> What do you think?
Since as per [1], usb3->extcon_host=1 and !usb3->forced_b_device ==1 , The above code always enter into Host Mode configuration.
To make it work, I need to update " usb3->forced_b_device" based on connection_state from TI chip. So the new code look like
1) There is no change in usb_check_id() call.
if (usb3->extcon_host && !usb3->forced_b_device)
usb3_mode_config(usb3, true, true);
else
usb3_mode_config(usb3, false, false);
2) Update "usb3->forced_b_device" variable based on connection_state.
@@ -2370,6 +2376,7 @@ static void handle_ext_role_switch_states(struct device *dev,
usb3_vbus_out(usb3, false);
break;
case USB_ROLE_DEVICE:
+ usb3->forced_b_device = true;
if (usb3->connection_state == USB_ROLE_NONE) {
usb3->connection_state = USB_ROLE_DEVICE;
usb3_set_mode(usb3, false);
@@ -2384,6 +2391,7 @@ static void handle_ext_role_switch_states(struct device *dev,
usb3_vbus_out(usb3, false);
break;
case USB_ROLE_HOST:
+ usb3->forced_b_device = false;
Can you please confirm are you ok with this changes? Or do you prefer the previous one?
Note:-
I have done only sanity testing with this changes.
Regards,
Biju
^ permalink raw reply [flat|nested] 25+ messages in thread
* [V4,4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework
@ 2019-04-23 9:07 ` Biju Das
0 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-23 9:07 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Greg Kroah-Hartman, Heikki Krogerus, Simon Horman,
Fabrizio Castro, Kees Cook, linux-usb, Simon Horman,
Geert Uytterhoeven, Chris Paterson, linux-renesas-soc,
Felipe Balbi
Hi Shimoda-San,
Thanks for the feedback.
> Subject: RE: [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use
> usb_role_switch framework
>
> Hi Biju-san,
>
> Thank you for the patch!
>
> > From: Biju Das, Sent: Tuesday, April 16, 2019 6:38 PM
> >
> > RZ/G2E cat874 board is capable of detecting cable connect and
> > disconnect events. Add support for renesas_usb3 to receive connect and
> > disconnect events and support dual-role switch using usb-role-switch
> framework.
> >
> > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > ---
> > V3-->V4
> > * No Change
> > V2-->V3
> > * Incorporated Shimoda-san's review comment
> > (https://patchwork.kernel.org/patch/10852507/)
> > * Used renesas,usb-role-switch property for differentiating USB
> > role switch associated with Type-C port controller driver.
> > V1-->V2
> > * Driver uses usb role clas for handling dual role switch and handling
> > connect/disconnect events instead of extcon.
> <snip>
>
> > +static void usb3_check_vbus(struct renesas_usb3 *usb3) {
> > + if (usb3->workaround_for_vbus) {
> > + if (usb3->usb_role_switch_property) {
> > + if (usb3->connection_state == USB_ROLE_DEVICE) {
> > + usb3_mode_config(usb3, false, false);
>
> I should have pointed it out the previous version though, why does this
> usb3_mode_config() calling need?
> My guess is:
> - renesas_usb3_start() calls renesas_usb3_init_controller().
> -- renesas_usb3_init_controller() calls usb3_check_id() and then
> usb_check_vbus().
> --- usb_check_id() calls usb3_mode_config(usb3, true, true) and then the
> HW acts as host mode.
> ----> So, you'd like the HW to acts as peripheral mode when the
> connection_state is USB_ROLE_DEVICE,
> you added that the usb3_check_vbus() calls usb3_mode_config(usb3,
> false, false).
>
> Is my guess correct? If so, I'd like to add such code into usb3_check_id() like
> below:
Yes, it is almost correct. The scenario I am trying is
[1] USB type C cable connected to a Host Machine(TI chip identifies as Device connection. But we haven't installed Gadget module for Device operation)
[2] After that trying to install gadget module. In this case, it calls usb_check_id() as mentioned above
and configure it as Host mode.
> if ((usb3->extcon_host && !usb3->forced_b_device) ||
> (usb3->usb_role_switch_property &&
> usb3->connection_state == USB_ROLE_HOST))
> usb3_mode_config(usb3, true, true);
> else
> usb3_mode_config(usb3, false, false);
>
> What do you think?
Since as per [1], usb3->extcon_host=1 and !usb3->forced_b_device ==1 , The above code always enter into Host Mode configuration.
To make it work, I need to update " usb3->forced_b_device" based on connection_state from TI chip. So the new code look like
1) There is no change in usb_check_id() call.
if (usb3->extcon_host && !usb3->forced_b_device)
usb3_mode_config(usb3, true, true);
else
usb3_mode_config(usb3, false, false);
2) Update "usb3->forced_b_device" variable based on connection_state.
@@ -2370,6 +2376,7 @@ static void handle_ext_role_switch_states(struct device *dev,
usb3_vbus_out(usb3, false);
break;
case USB_ROLE_DEVICE:
+ usb3->forced_b_device = true;
if (usb3->connection_state == USB_ROLE_NONE) {
usb3->connection_state = USB_ROLE_DEVICE;
usb3_set_mode(usb3, false);
@@ -2384,6 +2391,7 @@ static void handle_ext_role_switch_states(struct device *dev,
usb3_vbus_out(usb3, false);
break;
case USB_ROLE_HOST:
+ usb3->forced_b_device = false;
Can you please confirm are you ok with this changes? Or do you prefer the previous one?
Note:-
I have done only sanity testing with this changes.
Regards,
Biju
^ permalink raw reply [flat|nested] 25+ messages in thread
* RE: [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework
@ 2019-04-23 11:00 ` Yoshihiro Shimoda
0 siblings, 0 replies; 25+ messages in thread
From: Yoshihiro Shimoda @ 2019-04-23 11:00 UTC (permalink / raw)
To: Biju Das
Cc: Greg Kroah-Hartman, Heikki Krogerus, Simon Horman,
Fabrizio Castro, Kees Cook, linux-usb, Simon Horman,
Geert Uytterhoeven, Chris Paterson, linux-renesas-soc,
Felipe Balbi
Hi Biju-san,
> From: Biju Das, Sent: Tuesday, April 23, 2019 6:08 PM
>
> Hi Shimoda-San,
>
> Thanks for the feedback.
>
> > Subject: RE: [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use
> > usb_role_switch framework
> >
> > Hi Biju-san,
> >
> > Thank you for the patch!
> >
> > > From: Biju Das, Sent: Tuesday, April 16, 2019 6:38 PM
> > >
> > > RZ/G2E cat874 board is capable of detecting cable connect and
> > > disconnect events. Add support for renesas_usb3 to receive connect and
> > > disconnect events and support dual-role switch using usb-role-switch
> > framework.
> > >
> > > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > > ---
> > > V3-->V4
> > > * No Change
> > > V2-->V3
> > > * Incorporated Shimoda-san's review comment
> > > (https://patchwork.kernel.org/patch/10852507/)
> > > * Used renesas,usb-role-switch property for differentiating USB
> > > role switch associated with Type-C port controller driver.
> > > V1-->V2
> > > * Driver uses usb role clas for handling dual role switch and handling
> > > connect/disconnect events instead of extcon.
> > <snip>
> >
> > > +static void usb3_check_vbus(struct renesas_usb3 *usb3) {
> > > + if (usb3->workaround_for_vbus) {
> > > + if (usb3->usb_role_switch_property) {
> > > + if (usb3->connection_state == USB_ROLE_DEVICE) {
> > > + usb3_mode_config(usb3, false, false);
> >
> > I should have pointed it out the previous version though, why does this
> > usb3_mode_config() calling need?
> > My guess is:
> > - renesas_usb3_start() calls renesas_usb3_init_controller().
> > -- renesas_usb3_init_controller() calls usb3_check_id() and then
> > usb_check_vbus().
> > --- usb_check_id() calls usb3_mode_config(usb3, true, true) and then the
> > HW acts as host mode.
> > ----> So, you'd like the HW to acts as peripheral mode when the
> > connection_state is USB_ROLE_DEVICE,
> > you added that the usb3_check_vbus() calls usb3_mode_config(usb3,
> > false, false).
> >
> > Is my guess correct? If so, I'd like to add such code into usb3_check_id() like
> > below:
>
> Yes, it is almost correct. The scenario I am trying is
>
> [1] USB type C cable connected to a Host Machine(TI chip identifies as Device connection. But we haven't installed Gadget
> module for Device operation)
>
> [2] After that trying to install gadget module. In this case, it calls usb_check_id() as mentioned above
> and configure it as Host mode.
Thank you for the explanation.
> > if ((usb3->extcon_host && !usb3->forced_b_device) ||
> > (usb3->usb_role_switch_property &&
> > usb3->connection_state == USB_ROLE_HOST))
> > usb3_mode_config(usb3, true, true);
> > else
> > usb3_mode_config(usb3, false, false);
> >
> > What do you think?
>
> Since as per [1], usb3->extcon_host=1 and !usb3->forced_b_device ==1 , The above code always enter into Host Mode
> configuration.
Oops. Thank you for the pointed it out.
> To make it work, I need to update " usb3->forced_b_device" based on connection_state from TI chip. So the new code look
> like
Since the forced_b_device is related to debug purpose (controlled by debugfs), I don't want to use the value for type-c.
> 1) There is no change in usb_check_id() call.
>
> if (usb3->extcon_host && !usb3->forced_b_device)
> usb3_mode_config(usb3, true, true);
> else
> usb3_mode_config(usb3, false, false);
>
> 2) Update "usb3->forced_b_device" variable based on connection_state.
>
> @@ -2370,6 +2376,7 @@ static void handle_ext_role_switch_states(struct device *dev,
> usb3_vbus_out(usb3, false);
> break;
> case USB_ROLE_DEVICE:
> + usb3->forced_b_device = true;
> if (usb3->connection_state == USB_ROLE_NONE) {
> usb3->connection_state = USB_ROLE_DEVICE;
> usb3_set_mode(usb3, false);
> @@ -2384,6 +2391,7 @@ static void handle_ext_role_switch_states(struct device *dev,
> usb3_vbus_out(usb3, false);
> break;
> case USB_ROLE_HOST:
> + usb3->forced_b_device = false;
>
> Can you please confirm are you ok with this changes? Or do you prefer the previous one?
I'd like to change usb3_check_id() somehow.
How about the following conditions? In type-c environment,
since usb3->usb_role_switch_property is true, it should be OK for it.
if ((!usb3->usb_role_switch_property &&
usb3->extcon_host && !usb3->forced_b_device) ||
(usb3->usb_role_switch_property &&
usb3->connection_state == USB_ROLE_HOST))
usb3_mode_config(usb3, true, true);
else
usb3_mode_config(usb3, false, false);
Best regards,
Yoshihiro Shimoda
> Note:-
> I have done only sanity testing with this changes.
>
> Regards,
> Biju
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [V4,4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework
@ 2019-04-23 11:00 ` Yoshihiro Shimoda
0 siblings, 0 replies; 25+ messages in thread
From: Yoshihiro Shimoda @ 2019-04-23 11:00 UTC (permalink / raw)
To: Biju Das
Cc: Greg Kroah-Hartman, Heikki Krogerus, Simon Horman,
Fabrizio Castro, Kees Cook, linux-usb, Simon Horman,
Geert Uytterhoeven, Chris Paterson, linux-renesas-soc,
Felipe Balbi
Hi Biju-san,
> From: Biju Das, Sent: Tuesday, April 23, 2019 6:08 PM
>
> Hi Shimoda-San,
>
> Thanks for the feedback.
>
> > Subject: RE: [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use
> > usb_role_switch framework
> >
> > Hi Biju-san,
> >
> > Thank you for the patch!
> >
> > > From: Biju Das, Sent: Tuesday, April 16, 2019 6:38 PM
> > >
> > > RZ/G2E cat874 board is capable of detecting cable connect and
> > > disconnect events. Add support for renesas_usb3 to receive connect and
> > > disconnect events and support dual-role switch using usb-role-switch
> > framework.
> > >
> > > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > > ---
> > > V3-->V4
> > > * No Change
> > > V2-->V3
> > > * Incorporated Shimoda-san's review comment
> > > (https://patchwork.kernel.org/patch/10852507/)
> > > * Used renesas,usb-role-switch property for differentiating USB
> > > role switch associated with Type-C port controller driver.
> > > V1-->V2
> > > * Driver uses usb role clas for handling dual role switch and handling
> > > connect/disconnect events instead of extcon.
> > <snip>
> >
> > > +static void usb3_check_vbus(struct renesas_usb3 *usb3) {
> > > + if (usb3->workaround_for_vbus) {
> > > + if (usb3->usb_role_switch_property) {
> > > + if (usb3->connection_state == USB_ROLE_DEVICE) {
> > > + usb3_mode_config(usb3, false, false);
> >
> > I should have pointed it out the previous version though, why does this
> > usb3_mode_config() calling need?
> > My guess is:
> > - renesas_usb3_start() calls renesas_usb3_init_controller().
> > -- renesas_usb3_init_controller() calls usb3_check_id() and then
> > usb_check_vbus().
> > --- usb_check_id() calls usb3_mode_config(usb3, true, true) and then the
> > HW acts as host mode.
> > ----> So, you'd like the HW to acts as peripheral mode when the
> > connection_state is USB_ROLE_DEVICE,
> > you added that the usb3_check_vbus() calls usb3_mode_config(usb3,
> > false, false).
> >
> > Is my guess correct? If so, I'd like to add such code into usb3_check_id() like
> > below:
>
> Yes, it is almost correct. The scenario I am trying is
>
> [1] USB type C cable connected to a Host Machine(TI chip identifies as Device connection. But we haven't installed Gadget
> module for Device operation)
>
> [2] After that trying to install gadget module. In this case, it calls usb_check_id() as mentioned above
> and configure it as Host mode.
Thank you for the explanation.
> > if ((usb3->extcon_host && !usb3->forced_b_device) ||
> > (usb3->usb_role_switch_property &&
> > usb3->connection_state == USB_ROLE_HOST))
> > usb3_mode_config(usb3, true, true);
> > else
> > usb3_mode_config(usb3, false, false);
> >
> > What do you think?
>
> Since as per [1], usb3->extcon_host=1 and !usb3->forced_b_device ==1 , The above code always enter into Host Mode
> configuration.
Oops. Thank you for the pointed it out.
> To make it work, I need to update " usb3->forced_b_device" based on connection_state from TI chip. So the new code look
> like
Since the forced_b_device is related to debug purpose (controlled by debugfs), I don't want to use the value for type-c.
> 1) There is no change in usb_check_id() call.
>
> if (usb3->extcon_host && !usb3->forced_b_device)
> usb3_mode_config(usb3, true, true);
> else
> usb3_mode_config(usb3, false, false);
>
> 2) Update "usb3->forced_b_device" variable based on connection_state.
>
> @@ -2370,6 +2376,7 @@ static void handle_ext_role_switch_states(struct device *dev,
> usb3_vbus_out(usb3, false);
> break;
> case USB_ROLE_DEVICE:
> + usb3->forced_b_device = true;
> if (usb3->connection_state == USB_ROLE_NONE) {
> usb3->connection_state = USB_ROLE_DEVICE;
> usb3_set_mode(usb3, false);
> @@ -2384,6 +2391,7 @@ static void handle_ext_role_switch_states(struct device *dev,
> usb3_vbus_out(usb3, false);
> break;
> case USB_ROLE_HOST:
> + usb3->forced_b_device = false;
>
> Can you please confirm are you ok with this changes? Or do you prefer the previous one?
I'd like to change usb3_check_id() somehow.
How about the following conditions? In type-c environment,
since usb3->usb_role_switch_property is true, it should be OK for it.
if ((!usb3->usb_role_switch_property &&
usb3->extcon_host && !usb3->forced_b_device) ||
(usb3->usb_role_switch_property &&
usb3->connection_state == USB_ROLE_HOST))
usb3_mode_config(usb3, true, true);
else
usb3_mode_config(usb3, false, false);
Best regards,
Yoshihiro Shimoda
> Note:-
> I have done only sanity testing with this changes.
>
> Regards,
> Biju
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* RE: [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework
@ 2019-04-23 15:06 ` Biju Das
0 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-23 15:06 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Greg Kroah-Hartman, Heikki Krogerus, Simon Horman,
Fabrizio Castro, Kees Cook, linux-usb, Simon Horman,
Geert Uytterhoeven, Chris Paterson, linux-renesas-soc,
Felipe Balbi
Hi Shimoda-San,
Thanks for the feedback.
> Subject: RE: [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use
> usb_role_switch framework
>
> Hi Biju-san,
>
> > From: Biju Das, Sent: Tuesday, April 23, 2019 6:08 PM
> >
> > Hi Shimoda-San,
> >
> > Thanks for the feedback.
> >
> > > Subject: RE: [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use
> > > usb_role_switch framework
> > >
> > > Hi Biju-san,
> > >
> > > Thank you for the patch!
> > >
> > > > From: Biju Das, Sent: Tuesday, April 16, 2019 6:38 PM
> > > >
> > > > RZ/G2E cat874 board is capable of detecting cable connect and
> > > > disconnect events. Add support for renesas_usb3 to receive connect
> > > > and disconnect events and support dual-role switch using
> > > > usb-role-switch
> > > framework.
> > > >
> > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > > > ---
> > > > V3-->V4
> > > > * No Change
> > > > V2-->V3
> > > > * Incorporated Shimoda-san's review comment
> > > > (https://patchwork.kernel.org/patch/10852507/)
> > > > * Used renesas,usb-role-switch property for differentiating USB
> > > > role switch associated with Type-C port controller driver.
> > > > V1-->V2
> > > > * Driver uses usb role clas for handling dual role switch and handling
> > > > connect/disconnect events instead of extcon.
> > > <snip>
> > >
> > > > +static void usb3_check_vbus(struct renesas_usb3 *usb3) {
> > > > + if (usb3->workaround_for_vbus) {
> > > > + if (usb3->usb_role_switch_property) {
> > > > + if (usb3->connection_state == USB_ROLE_DEVICE) {
> > > > + usb3_mode_config(usb3, false, false);
> > >
> > > I should have pointed it out the previous version though, why does
> > > this
> > > usb3_mode_config() calling need?
> > > My guess is:
> > > - renesas_usb3_start() calls renesas_usb3_init_controller().
> > > -- renesas_usb3_init_controller() calls usb3_check_id() and then
> > > usb_check_vbus().
> > > --- usb_check_id() calls usb3_mode_config(usb3, true, true) and
> > > then the HW acts as host mode.
> > > ----> So, you'd like the HW to acts as peripheral mode when the
> > > connection_state is USB_ROLE_DEVICE,
> > > you added that the usb3_check_vbus() calls
> > > usb3_mode_config(usb3, false, false).
> > >
> > > Is my guess correct? If so, I'd like to add such code into
> > > usb3_check_id() like
> > > below:
> >
> > Yes, it is almost correct. The scenario I am trying is
> >
> > [1] USB type C cable connected to a Host Machine(TI chip identifies
> > as Device connection. But we haven't installed Gadget module for
> > Device operation)
> >
> > [2] After that trying to install gadget module. In this case, it
> > calls usb_check_id() as mentioned above and configure it as Host mode.
>
> Thank you for the explanation.
>
> > > if ((usb3->extcon_host && !usb3->forced_b_device) ||
> > > (usb3->usb_role_switch_property &&
> > > usb3->connection_state == USB_ROLE_HOST))
> > > usb3_mode_config(usb3, true, true);
> > > else
> > > usb3_mode_config(usb3, false, false);
> > >
> > > What do you think?
> >
> > Since as per [1], usb3->extcon_host=1 and !usb3->forced_b_device ==1
> > , The above code always enter into Host Mode configuration.
>
> Oops. Thank you for the pointed it out.
>
> > To make it work, I need to update " usb3->forced_b_device" based on
> > connection_state from TI chip. So the new code look like
>
> Since the forced_b_device is related to debug purpose (controlled by
> debugfs), I don't want to use the value for type-c.
>
> > 1) There is no change in usb_check_id() call.
> >
> > if (usb3->extcon_host && !usb3->forced_b_device)
> > usb3_mode_config(usb3, true, true);
> > else
> > usb3_mode_config(usb3, false, false);
> >
> > 2) Update "usb3->forced_b_device" variable based on connection_state.
> >
> > @@ -2370,6 +2376,7 @@ static void handle_ext_role_switch_states(struct
> device *dev,
> > usb3_vbus_out(usb3, false);
> > break;
> > case USB_ROLE_DEVICE:
> > + usb3->forced_b_device = true;
> > if (usb3->connection_state == USB_ROLE_NONE) {
> > usb3->connection_state = USB_ROLE_DEVICE;
> > usb3_set_mode(usb3, false); @@ -2384,6 +2391,7
> > @@ static void handle_ext_role_switch_states(struct device *dev,
> > usb3_vbus_out(usb3, false);
> > break;
> > case USB_ROLE_HOST:
> > + usb3->forced_b_device = false;
> >
> > Can you please confirm are you ok with this changes? Or do you prefer the
> previous one?
>
> I'd like to change usb3_check_id() somehow.
> How about the following conditions? In type-c environment, since usb3-
> >usb_role_switch_property is true, it should be OK for it.
>
> if ((!usb3->usb_role_switch_property &&
> usb3->extcon_host && !usb3->forced_b_device) ||
> (usb3->usb_role_switch_property &&
> usb3->connection_state == USB_ROLE_HOST))
> usb3_mode_config(usb3, true, true);
> else
> usb3_mode_config(usb3, false, false);
>
OK. I will send V5 with this changes.
Regards,
Biju
^ permalink raw reply [flat|nested] 25+ messages in thread
* [V4,4/7] usb: gadget: udc: renesas_usb3: Use usb_role_switch framework
@ 2019-04-23 15:06 ` Biju Das
0 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-23 15:06 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Greg Kroah-Hartman, Heikki Krogerus, Simon Horman,
Fabrizio Castro, Kees Cook, linux-usb, Simon Horman,
Geert Uytterhoeven, Chris Paterson, linux-renesas-soc,
Felipe Balbi
Hi Shimoda-San,
Thanks for the feedback.
> Subject: RE: [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use
> usb_role_switch framework
>
> Hi Biju-san,
>
> > From: Biju Das, Sent: Tuesday, April 23, 2019 6:08 PM
> >
> > Hi Shimoda-San,
> >
> > Thanks for the feedback.
> >
> > > Subject: RE: [PATCH V4 4/7] usb: gadget: udc: renesas_usb3: Use
> > > usb_role_switch framework
> > >
> > > Hi Biju-san,
> > >
> > > Thank you for the patch!
> > >
> > > > From: Biju Das, Sent: Tuesday, April 16, 2019 6:38 PM
> > > >
> > > > RZ/G2E cat874 board is capable of detecting cable connect and
> > > > disconnect events. Add support for renesas_usb3 to receive connect
> > > > and disconnect events and support dual-role switch using
> > > > usb-role-switch
> > > framework.
> > > >
> > > > Signed-off-by: Biju Das <biju.das@bp.renesas.com>
> > > > ---
> > > > V3-->V4
> > > > * No Change
> > > > V2-->V3
> > > > * Incorporated Shimoda-san's review comment
> > > > (https://patchwork.kernel.org/patch/10852507/)
> > > > * Used renesas,usb-role-switch property for differentiating USB
> > > > role switch associated with Type-C port controller driver.
> > > > V1-->V2
> > > > * Driver uses usb role clas for handling dual role switch and handling
> > > > connect/disconnect events instead of extcon.
> > > <snip>
> > >
> > > > +static void usb3_check_vbus(struct renesas_usb3 *usb3) {
> > > > + if (usb3->workaround_for_vbus) {
> > > > + if (usb3->usb_role_switch_property) {
> > > > + if (usb3->connection_state == USB_ROLE_DEVICE) {
> > > > + usb3_mode_config(usb3, false, false);
> > >
> > > I should have pointed it out the previous version though, why does
> > > this
> > > usb3_mode_config() calling need?
> > > My guess is:
> > > - renesas_usb3_start() calls renesas_usb3_init_controller().
> > > -- renesas_usb3_init_controller() calls usb3_check_id() and then
> > > usb_check_vbus().
> > > --- usb_check_id() calls usb3_mode_config(usb3, true, true) and
> > > then the HW acts as host mode.
> > > ----> So, you'd like the HW to acts as peripheral mode when the
> > > connection_state is USB_ROLE_DEVICE,
> > > you added that the usb3_check_vbus() calls
> > > usb3_mode_config(usb3, false, false).
> > >
> > > Is my guess correct? If so, I'd like to add such code into
> > > usb3_check_id() like
> > > below:
> >
> > Yes, it is almost correct. The scenario I am trying is
> >
> > [1] USB type C cable connected to a Host Machine(TI chip identifies
> > as Device connection. But we haven't installed Gadget module for
> > Device operation)
> >
> > [2] After that trying to install gadget module. In this case, it
> > calls usb_check_id() as mentioned above and configure it as Host mode.
>
> Thank you for the explanation.
>
> > > if ((usb3->extcon_host && !usb3->forced_b_device) ||
> > > (usb3->usb_role_switch_property &&
> > > usb3->connection_state == USB_ROLE_HOST))
> > > usb3_mode_config(usb3, true, true);
> > > else
> > > usb3_mode_config(usb3, false, false);
> > >
> > > What do you think?
> >
> > Since as per [1], usb3->extcon_host=1 and !usb3->forced_b_device ==1
> > , The above code always enter into Host Mode configuration.
>
> Oops. Thank you for the pointed it out.
>
> > To make it work, I need to update " usb3->forced_b_device" based on
> > connection_state from TI chip. So the new code look like
>
> Since the forced_b_device is related to debug purpose (controlled by
> debugfs), I don't want to use the value for type-c.
>
> > 1) There is no change in usb_check_id() call.
> >
> > if (usb3->extcon_host && !usb3->forced_b_device)
> > usb3_mode_config(usb3, true, true);
> > else
> > usb3_mode_config(usb3, false, false);
> >
> > 2) Update "usb3->forced_b_device" variable based on connection_state.
> >
> > @@ -2370,6 +2376,7 @@ static void handle_ext_role_switch_states(struct
> device *dev,
> > usb3_vbus_out(usb3, false);
> > break;
> > case USB_ROLE_DEVICE:
> > + usb3->forced_b_device = true;
> > if (usb3->connection_state == USB_ROLE_NONE) {
> > usb3->connection_state = USB_ROLE_DEVICE;
> > usb3_set_mode(usb3, false); @@ -2384,6 +2391,7
> > @@ static void handle_ext_role_switch_states(struct device *dev,
> > usb3_vbus_out(usb3, false);
> > break;
> > case USB_ROLE_HOST:
> > + usb3->forced_b_device = false;
> >
> > Can you please confirm are you ok with this changes? Or do you prefer the
> previous one?
>
> I'd like to change usb3_check_id() somehow.
> How about the following conditions? In type-c environment, since usb3-
> >usb_role_switch_property is true, it should be OK for it.
>
> if ((!usb3->usb_role_switch_property &&
> usb3->extcon_host && !usb3->forced_b_device) ||
> (usb3->usb_role_switch_property &&
> usb3->connection_state == USB_ROLE_HOST))
> usb3_mode_config(usb3, true, true);
> else
> usb3_mode_config(usb3, false, false);
>
OK. I will send V5 with this changes.
Regards,
Biju
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH V4 5/7] arm64: defconfig: enable TYPEC_HD3SS3220 config option
@ 2019-04-16 9:38 ` Biju Das
0 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-16 9:38 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Biju Das, Greg Kroah-Hartman, Felipe Balbi, linux-usb,
Simon Horman, Yoshihiro Shimoda, Geert Uytterhoeven,
Chris Paterson, Fabrizio Castro, linux-renesas-soc
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>
---
V3-->V4
* No change
V2-->V3
* No change
V1-->V2
* No change
---
arch/arm64/configs/defconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index d06825f..16cb687 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -578,6 +578,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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [V4,5/7] arm64: defconfig: enable TYPEC_HD3SS3220 config option
@ 2019-04-16 9:38 ` Biju Das
0 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-16 9:38 UTC (permalink / raw)
To: Heikki Krogerus
Cc: Biju Das, Greg Kroah-Hartman, Felipe Balbi, linux-usb,
Simon Horman, Yoshihiro Shimoda, Geert Uytterhoeven,
Chris Paterson, Fabrizio Castro, linux-renesas-soc
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>
---
V3-->V4
* No change
V2-->V3
* No change
V1-->V2
* No change
---
arch/arm64/configs/defconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index d06825f..16cb687 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -578,6 +578,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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH V4 6/7] arm64: dts: renesas: r8a774c0-cat874: Enable USB3.0 host/peripheral device node
2019-04-16 9:38 [PATCH V4 0/7] Add USB3.0 and TI HD3SS3220 driver support Biju Das
` (4 preceding siblings ...)
2019-04-16 9:38 ` [V4,5/7] " Biju Das
@ 2019-04-16 9:38 ` Biju Das
2019-04-16 9:38 ` [PATCH V4 7/7] arm64: dts: renesas: r8a774c0-cat874: Enable usb role switch support Biju Das
6 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-16 9:38 UTC (permalink / raw)
To: Rob Herring, Mark Rutland
Cc: Biju Das, Greg Kroah-Hartman, Heikki Krogerus, Felipe Balbi,
Simon Horman, Yoshihiro Shimoda, Magnus Damm, linux-renesas-soc,
devicetree, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro
This patch enables USB3.0 host/peripheral device node for r8a774c0
cat874 board.
Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
V3-->V4
* No change
V2-->V3
* No change
V1-->V2
* No change
---
arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
index 013a48c..b9ae7db 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
@@ -134,6 +134,11 @@
function = "sdhi0";
power-source = <1800>;
};
+
+ usb30_pins: usb30 {
+ groups = "usb30", "usb30_id";
+ function = "usb30";
+ };
};
&rwdt {
@@ -166,3 +171,15 @@
renesas,no-otg-pins;
status = "okay";
};
+
+&usb3_peri0 {
+ companion = <&xhci0>;
+ status = "okay";
+};
+
+&xhci0 {
+ pinctrl-0 = <&usb30_pins>;
+ pinctrl-names = "default";
+
+ status = "okay";
+};
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH V4 7/7] arm64: dts: renesas: r8a774c0-cat874: Enable usb role switch support
2019-04-16 9:38 [PATCH V4 0/7] Add USB3.0 and TI HD3SS3220 driver support Biju Das
` (5 preceding siblings ...)
2019-04-16 9:38 ` [PATCH V4 6/7] arm64: dts: renesas: r8a774c0-cat874: Enable USB3.0 host/peripheral device node Biju Das
@ 2019-04-16 9:38 ` Biju Das
6 siblings, 0 replies; 25+ messages in thread
From: Biju Das @ 2019-04-16 9:38 UTC (permalink / raw)
To: Rob Herring, Mark Rutland
Cc: Biju Das, Greg Kroah-Hartman, Heikki Krogerus, Felipe Balbi,
Simon Horman, Yoshihiro Shimoda, Magnus Damm, linux-renesas-soc,
devicetree, Geert Uytterhoeven, Chris Paterson, Fabrizio Castro
This patch enables TI HD3SS3220 device and support usb role switch
for the CAT 874 platform.
Signed-off-by: Biju Das <biju.das@bp.renesas.com>
---
V3-->V4
* No change
V2-->V3
* Used "renesas,usb-role-switch" instead of generic "usb-role-switch"
property
V1-->V2
* New patch
---
arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts | 39 +++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
index b9ae7db..746826c 100644
--- a/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
+++ b/arch/arm64/boot/dts/renesas/r8a774c0-cat874.dts
@@ -85,6 +85,34 @@
clock-frequency = <48000000>;
};
+&i2c0 {
+ status = "okay";
+ clock-frequency = <100000>;
+
+ hd3ss3220@47 {
+ compatible = "ti,hd3ss3220";
+ reg = <0x47>;
+ interrupt-parent = <&gpio6>;
+ interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+
+ usb_con: connector {
+ compatible = "usb-c-connector";
+ label = "USB-C";
+ data-role = "dual";
+ };
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ hd3ss3220_ep: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&usb3peri_role_switch>;
+ };
+ };
+ };
+};
+
&i2c1 {
pinctrl-0 = <&i2c1_pins>;
pinctrl-names = "default";
@@ -175,6 +203,17 @@
&usb3_peri0 {
companion = <&xhci0>;
status = "okay";
+ renesas,usb-role-switch;
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ usb3peri_role_switch: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&hd3ss3220_ep>;
+ };
+ };
};
&xhci0 {
--
2.7.4
^ permalink raw reply related [flat|nested] 25+ messages in thread