linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V0 0/7] Add Embedded USB Debugger (EUD) driver
@ 2021-10-04 11:16 Souradeep Chowdhury
  2021-10-04 11:16 ` [PATCH V0 1/7] dt-bindings: connector: Add property for eud type c connector Souradeep Chowdhury
                   ` (6 more replies)
  0 siblings, 7 replies; 26+ messages in thread
From: Souradeep Chowdhury @ 2021-10-04 11:16 UTC (permalink / raw)
  To: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Bjorn Andersson, Greg KH
  Cc: linux-kernel, ckadabi, tsoni, bryanh, psodagud, satyap, pheragu,
	Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan,
	Souradeep Chowdhury

This is a series of patches that implements a driver for the control
peripheral, EUD (Embedded USB Debugger). The EUD is a mini-USB hub
implemented on chip to support the USB-based debug and trace capabilities.
Apart from debug capabilities, EUD has a control peripheral. Control
Peripheral is on when EUD is on and gets signals like USB attach, pet
EUD etc.EUD driver listens to events like USB attach or detach and then
informs the USB about these events via ROLE-SWITCH. At regular intervals,
the EUD driver receives an interrupt to pet the driver indicating that
the software is functional.

Souradeep Chowdhury (7):
  dt-bindings: connector: Add property for eud type c connector
  dt-bindings: usb: dwc3: Update dwc3 properties for EUD connector
  usb: dwc3: drd: Register the eud connector child node for dwc3
  usb: common: eud:Added the driver support for Embedded USB
    Debugger(EUD)
  arm64: dts: qcom: sc7280: Add EUD connector node
  arm64: dts: qcom: sc7280: Set the default dr_mode for usb2
  MAINTAINERS: Add maintainer entry for EUD

 Documentation/ABI/testing/sysfs-driver-eud         |   7 +
 .../bindings/connector/usb-connector.yaml          |  15 ++
 .../devicetree/bindings/usb/snps,dwc3.yaml         |  15 ++
 MAINTAINERS                                        |   7 +
 arch/arm64/boot/dts/qcom/sc7280-idp.dts            |   4 +
 arch/arm64/boot/dts/qcom/sc7280.dtsi               |  12 +
 drivers/usb/common/Kconfig                         |   9 +
 drivers/usb/common/Makefile                        |   1 +
 drivers/usb/common/qcom_eud.c                      | 256 +++++++++++++++++++++
 drivers/usb/dwc3/drd.c                             |  25 ++
 10 files changed, 351 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-eud
 create mode 100644 drivers/usb/common/qcom_eud.c

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH V0 1/7] dt-bindings: connector: Add property for eud type c connector
  2021-10-04 11:16 [PATCH V0 0/7] Add Embedded USB Debugger (EUD) driver Souradeep Chowdhury
@ 2021-10-04 11:16 ` Souradeep Chowdhury
  2021-10-04 16:37   ` Rob Herring
  2021-10-04 11:16 ` [PATCH V0 2/7] dt-bindings: usb: dwc3: Update dwc3 properties for EUD connector Souradeep Chowdhury
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 26+ messages in thread
From: Souradeep Chowdhury @ 2021-10-04 11:16 UTC (permalink / raw)
  To: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Bjorn Andersson, Greg KH
  Cc: linux-kernel, ckadabi, tsoni, bryanh, psodagud, satyap, pheragu,
	Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan,
	Souradeep Chowdhury

Added the property for EUD(Embedded USB Debug) connector.Added
the "reg" and "interrupts" property which is needed for EUD.

Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
---
 .../devicetree/bindings/connector/usb-connector.yaml      | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
index 7eb8659..908129f 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
+++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
@@ -30,6 +30,21 @@ properties:
           - const: samsung,usb-connector-11pin
           - const: usb-b-connector
 
+      - items:
+          - enum:
+              - qcom,sc7280-usb-connector-eud
+          - const: qcom,usb-connector-eud
+          - const: usb-c-connector
+
+  reg:
+    items:
+      - description: EUD Base Register Region
+      - description: EUD Mode Manager Region
+
+  interrupts:
+    description:
+      EUD interrupt
+
   label:
     description: Symbolic name for the connector.
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH V0 2/7] dt-bindings: usb: dwc3: Update dwc3 properties for EUD connector
  2021-10-04 11:16 [PATCH V0 0/7] Add Embedded USB Debugger (EUD) driver Souradeep Chowdhury
  2021-10-04 11:16 ` [PATCH V0 1/7] dt-bindings: connector: Add property for eud type c connector Souradeep Chowdhury
@ 2021-10-04 11:16 ` Souradeep Chowdhury
  2021-10-04 15:50   ` Bjorn Andersson
  2021-10-04 11:16 ` [PATCH V0 3/7] usb: dwc3: drd: Register the eud connector child node for dwc3 Souradeep Chowdhury
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 26+ messages in thread
From: Souradeep Chowdhury @ 2021-10-04 11:16 UTC (permalink / raw)
  To: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Bjorn Andersson, Greg KH
  Cc: linux-kernel, ckadabi, tsoni, bryanh, psodagud, satyap, pheragu,
	Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan,
	Souradeep Chowdhury

Adding the address size,cell size and ranges property for EUD connector.
Adding the connector property for EUD which is child of dwc3 node.

Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
---
 Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index 078fb78..3e71205 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -36,6 +36,14 @@ properties:
         - const: synopsys,dwc3
           deprecated: true
 
+  "#address-cells":
+    enum: [ 1, 2 ]
+
+  "#size-cells":
+    enum: [ 1, 2 ]
+
+  ranges: true
+
   interrupts:
     description:
       It's either a single common DWC3 interrupt (dwc_usb3) or individual
@@ -318,6 +326,13 @@ properties:
     items:
       enum: [1, 4, 8, 16, 32, 64, 128, 256]
 
+  connector:
+    type: object
+    $ref: /connector/usb-connector.yaml#
+    description:
+      Connector for dual role switch, especially for "eud-usb-c-connector"
+
+
 unevaluatedProperties: false
 
 required:
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH V0 3/7] usb: dwc3: drd: Register the eud connector child node for dwc3
  2021-10-04 11:16 [PATCH V0 0/7] Add Embedded USB Debugger (EUD) driver Souradeep Chowdhury
  2021-10-04 11:16 ` [PATCH V0 1/7] dt-bindings: connector: Add property for eud type c connector Souradeep Chowdhury
  2021-10-04 11:16 ` [PATCH V0 2/7] dt-bindings: usb: dwc3: Update dwc3 properties for EUD connector Souradeep Chowdhury
@ 2021-10-04 11:16 ` Souradeep Chowdhury
  2021-10-05  4:42   ` kernel test robot
  2021-10-05 12:32   ` kernel test robot
  2021-10-04 11:16 ` [PATCH V0 4/7] usb: common: eud: Added the driver support for Embedded USB Debugger(EUD) Souradeep Chowdhury
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 26+ messages in thread
From: Souradeep Chowdhury @ 2021-10-04 11:16 UTC (permalink / raw)
  To: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Bjorn Andersson, Greg KH
  Cc: linux-kernel, ckadabi, tsoni, bryanh, psodagud, satyap, pheragu,
	Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan,
	Souradeep Chowdhury

Register the child node for dwc3 which is the "eud_usb_connector".
The eud driver will be able to switch the usb role from device to
host and vice versa using the role switch property of dwc3 node.

Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
---
 drivers/usb/dwc3/drd.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
index d7f7683..f55e473 100644
--- a/drivers/usb/dwc3/drd.c
+++ b/drivers/usb/dwc3/drd.c
@@ -11,6 +11,7 @@
 #include <linux/of_graph.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
+#include <linux/of_platform.h>
 
 #include "debug.h"
 #include "core.h"
@@ -164,6 +165,27 @@ static int dwc3_otg_get_irq(struct dwc3 *dwc)
 	return irq;
 }
 
+static int dwc3_register_eud(struct dwc3 *dwc)
+{
+	struct device           *dev = dwc->dev;
+	struct device_node      *np = dev->of_node, *con_np;
+	int                     ret;
+
+	con_np = of_get_child_by_name(np, "eud_usb_connector");
+	if (!np) {
+		dev_dbg(dev, "no usb_connector child node specified\n");
+		return 0;
+	}
+
+	ret = of_platform_populate(np, NULL, NULL, dev);
+	if (ret) {
+		dev_err(dev, "failed to register usb_connector - %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 void dwc3_otg_init(struct dwc3 *dwc)
 {
 	u32 reg;
@@ -580,6 +602,9 @@ int dwc3_drd_init(struct dwc3 *dwc)
 		ret = dwc3_setup_role_switch(dwc);
 		if (ret < 0)
 			return ret;
+		ret = dwc3_register_eud(dwc);
+		if (ret < 0)
+			return ret;
 	} else if (dwc->edev) {
 		dwc->edev_nb.notifier_call = dwc3_drd_notifier;
 		ret = extcon_register_notifier(dwc->edev, EXTCON_USB_HOST,
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH V0 4/7] usb: common: eud: Added the driver support for Embedded USB Debugger(EUD)
  2021-10-04 11:16 [PATCH V0 0/7] Add Embedded USB Debugger (EUD) driver Souradeep Chowdhury
                   ` (2 preceding siblings ...)
  2021-10-04 11:16 ` [PATCH V0 3/7] usb: dwc3: drd: Register the eud connector child node for dwc3 Souradeep Chowdhury
@ 2021-10-04 11:16 ` Souradeep Chowdhury
  2021-10-04 11:31   ` Greg KH
                     ` (2 more replies)
  2021-10-04 11:16 ` [PATCH V0 5/7] arm64: dts: qcom: sc7280: Add EUD connector node Souradeep Chowdhury
                   ` (2 subsequent siblings)
  6 siblings, 3 replies; 26+ messages in thread
From: Souradeep Chowdhury @ 2021-10-04 11:16 UTC (permalink / raw)
  To: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Bjorn Andersson, Greg KH
  Cc: linux-kernel, ckadabi, tsoni, bryanh, psodagud, satyap, pheragu,
	Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan,
	Souradeep Chowdhury

Add support for control peripheral of EUD (Embedded USB Debugger) to
listen to events such as USB attach/detach, pet EUD to indicate software
is functional.Reusing the platform device kobj, sysfs entry 'enable' is
created to enable or disable EUD.

To enable the eud the following needs to be done
echo 1 > /sys/bus/platform/.../enable

To disable eud, following is the command
echo 0 > /sys/bus/platform/.../enable

Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
---
 Documentation/ABI/testing/sysfs-driver-eud |   7 +
 drivers/usb/common/Kconfig                 |   9 +
 drivers/usb/common/Makefile                |   1 +
 drivers/usb/common/qcom_eud.c              | 256 +++++++++++++++++++++++++++++
 4 files changed, 273 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-eud
 create mode 100644 drivers/usb/common/qcom_eud.c

diff --git a/Documentation/ABI/testing/sysfs-driver-eud b/Documentation/ABI/testing/sysfs-driver-eud
new file mode 100644
index 0000000..14a02da
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-eud
@@ -0,0 +1,7 @@
+What:		/sys/bus/platform/.../enable
+Date:           October 2021
+Contact:        Souradeep Chowdhury <schowdhu@codeaurora.org>
+Description:
+		The Enable/Disable sysfs interface for Embedded
+		USB Debugger(EUD).This enables and disables the
+		EUD based on a 1 or a 0 value.
diff --git a/drivers/usb/common/Kconfig b/drivers/usb/common/Kconfig
index 5e8a04e..669b3fe 100644
--- a/drivers/usb/common/Kconfig
+++ b/drivers/usb/common/Kconfig
@@ -50,3 +50,12 @@ config USB_CONN_GPIO

 	  To compile the driver as a module, choose M here: the module will
 	  be called usb-conn-gpio.ko
+
+config USB_QCOM_EUD
+	tristate "USB EUD Driver"
+	select USB_ROLE_SWITCH
+	help
+	  This module enables support for Qualcomm Technologies, Inc.
+	  Embedded USB Debugger (EUD). The EUD is a control peripheral
+	  which reports VBUS attach/detach events and has USB-based
+	  debug and trace capabilities.
diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
index 8ac4d21..eb66045 100644
--- a/drivers/usb/common/Makefile
+++ b/drivers/usb/common/Makefile
@@ -11,3 +11,4 @@ usb-common-$(CONFIG_USB_LED_TRIG) += led.o
 obj-$(CONFIG_USB_CONN_GPIO)	+= usb-conn-gpio.o
 obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
 obj-$(CONFIG_USB_ULPI_BUS)	+= ulpi.o
+obj-$(CONFIG_USB_QCOM_EUD)      += qcom_eud.o
diff --git a/drivers/usb/common/qcom_eud.c b/drivers/usb/common/qcom_eud.c
new file mode 100644
index 0000000..7a92fff
--- /dev/null
+++ b/drivers/usb/common/qcom_eud.c
@@ -0,0 +1,256 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+#include <linux/usb/role.h>
+
+#define EUD_REG_INT1_EN_MASK	0x0024
+#define EUD_REG_INT_STATUS_1	0x0044
+#define EUD_REG_CTL_OUT_1	0x0074
+#define EUD_REG_VBUS_INT_CLR	0x0080
+#define EUD_REG_CSR_EUD_EN	0x1014
+#define EUD_REG_SW_ATTACH_DET	0x1018
+#define EUD_REG_EUD_EN2         0x0000
+
+#define EUD_ENABLE		BIT(0)
+#define EUD_INT_PET_EUD		BIT(0)
+#define EUD_INT_VBUS		BIT(2)
+#define EUD_INT_SAFE_MODE	BIT(4)
+#define EUD_INT_ALL		(EUD_INT_VBUS|EUD_INT_SAFE_MODE)
+
+struct eud_chip {
+	struct device			*dev;
+	struct usb_role_switch		*role_sw;
+	void __iomem			*eud_reg_base;
+	void __iomem			*eud_mode_mgr2_phys_base;
+	unsigned int			int_status;
+	int				enable;
+	int				eud_irq;
+	bool				usb_attach;
+
+};
+
+static int enable_eud(struct eud_chip *priv)
+{
+	writel(EUD_ENABLE, priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
+	writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
+			priv->eud_reg_base + EUD_REG_INT1_EN_MASK);
+	writel(1, priv->eud_mode_mgr2_phys_base + EUD_REG_EUD_EN2);
+
+	return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE);
+}
+
+static void disable_eud(struct eud_chip *priv)
+{
+	writel(0, priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
+	writel(0, priv->eud_mode_mgr2_phys_base + EUD_REG_EUD_EN2);
+}
+
+static ssize_t enable_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct eud_chip *chip = dev_get_drvdata(dev);
+
+	return sprintf(buf, "%d", chip->enable);
+}
+
+static ssize_t enable_store(struct device *dev,
+		struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	struct eud_chip *chip = dev_get_drvdata(dev);
+	unsigned long enable;
+	int ret;
+
+	if (kstrtoul(buf, 16, &enable))
+		return -EINVAL;
+
+	if (enable == 1) {
+		ret = enable_eud(chip);
+		if (!ret)
+			chip->enable = enable;
+	} else if (enable == 0) {
+		disable_eud(chip);
+	} else {
+		return -EINVAL;
+	}
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(enable);
+
+static const struct device_attribute *eud_attrs[] = {
+	&dev_attr_enable,
+	NULL,
+};
+
+static void usb_attach_detach(struct eud_chip *chip)
+{
+	u32 reg;
+
+	/* read ctl_out_1[4] to find USB attach or detach event */
+	reg = readl(chip->eud_reg_base + EUD_REG_CTL_OUT_1);
+	if (reg & EUD_INT_SAFE_MODE)
+		chip->usb_attach = true;
+	else
+		chip->usb_attach = false;
+
+	/* set and clear vbus_int_clr[0] to clear interrupt */
+	writel(BIT(0), chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
+	writel(0, chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
+}
+
+static void pet_eud(struct eud_chip *chip)
+{
+	u32 reg;
+	int ret;
+
+	/* read sw_attach_det[0] to find attach/detach event */
+	reg = readl(chip->eud_reg_base +  EUD_REG_SW_ATTACH_DET);
+	if (reg & EUD_INT_PET_EUD) {
+		/* Detach & Attach pet for EUD */
+		writel(0, chip->eud_reg_base + EUD_REG_SW_ATTACH_DET);
+		/* Delay to make sure detach pet is done before attach pet */
+		ret = readl_poll_timeout(chip->eud_reg_base + EUD_REG_SW_ATTACH_DET,
+					reg, (reg == 0), 1, 100);
+		if (ret) {
+			dev_err(chip->dev, "Detach pet failed\n");
+			return;
+		}
+
+		writel(EUD_INT_PET_EUD, chip->eud_reg_base +
+				EUD_REG_SW_ATTACH_DET);
+	} else {
+		/* Attach pet for EUD */
+		writel(EUD_INT_PET_EUD, chip->eud_reg_base +
+				EUD_REG_SW_ATTACH_DET);
+	}
+}
+
+static irqreturn_t handle_eud_irq(int irq, void *data)
+{
+	struct eud_chip *chip = data;
+	u32 reg;
+
+	/* read status register and find out which interrupt triggered */
+	reg = readl(chip->eud_reg_base +  EUD_REG_INT_STATUS_1);
+	switch (reg & EUD_INT_ALL) {
+	case EUD_INT_VBUS:
+		chip->int_status = EUD_INT_VBUS;
+		usb_attach_detach(chip);
+		return IRQ_WAKE_THREAD;
+	case EUD_INT_SAFE_MODE:
+		pet_eud(chip);
+		break;
+	default:
+		return IRQ_NONE;
+	}
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t handle_eud_irq_thread(int irq, void *data)
+{
+	struct eud_chip *chip = data;
+	int ret;
+
+	if (chip->int_status == EUD_INT_VBUS) {
+		if (chip->usb_attach)
+			ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_DEVICE);
+		else
+			ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_HOST);
+		if (ret)
+			dev_err(chip->dev, "failed to set role switch\n");
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int eud_probe(struct platform_device *pdev)
+{
+	struct eud_chip *chip;
+	int ret;
+
+	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->dev = &pdev->dev;
+
+	chip->role_sw = usb_role_switch_get(chip->dev);
+	if (IS_ERR(chip->role_sw)) {
+		if (PTR_ERR(chip->role_sw) != -EPROBE_DEFER)
+			dev_err(chip->dev, "failed to get role switch\n");
+
+		return PTR_ERR(chip->role_sw);
+	}
+
+	chip->eud_reg_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(chip->eud_reg_base))
+		return PTR_ERR(chip->eud_reg_base);
+
+	chip->eud_mode_mgr2_phys_base = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(chip->eud_mode_mgr2_phys_base))
+		return PTR_ERR(chip->eud_mode_mgr2_phys_base);
+
+	chip->eud_irq = platform_get_irq(pdev, 0);
+	ret = devm_request_threaded_irq(&pdev->dev, chip->eud_irq, handle_eud_irq,
+			handle_eud_irq_thread, IRQF_ONESHOT, NULL, chip);
+	if (ret)
+		return ret;
+
+	device_init_wakeup(&pdev->dev, true);
+	enable_irq_wake(chip->eud_irq);
+
+	platform_set_drvdata(pdev, chip);
+
+	ret = device_create_file(&pdev->dev, eud_attrs[0]);
+
+	return ret;
+}
+
+static int eud_remove(struct platform_device *pdev)
+{
+	struct eud_chip *chip = platform_get_drvdata(pdev);
+
+	if (chip->enable)
+		disable_eud(chip);
+
+	device_remove_file(&pdev->dev, eud_attrs[0]);
+	device_init_wakeup(&pdev->dev, false);
+	disable_irq_wake(chip->eud_irq);
+
+	return 0;
+}
+
+static const struct of_device_id eud_dt_match[] = {
+	{ .compatible = "qcom,usb-connector-eud" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, eud_dt_match);
+
+static struct platform_driver eud_driver = {
+	.probe		= eud_probe,
+	.remove		= eud_remove,
+	.driver		= {
+		.name		= "eud",
+		.of_match_table = eud_dt_match,
+	},
+};
+module_platform_driver(eud_driver);
+
+MODULE_DESCRIPTION("QTI EUD driver");
+MODULE_LICENSE("GPL v2");
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH V0 5/7] arm64: dts: qcom: sc7280: Add EUD connector node
  2021-10-04 11:16 [PATCH V0 0/7] Add Embedded USB Debugger (EUD) driver Souradeep Chowdhury
                   ` (3 preceding siblings ...)
  2021-10-04 11:16 ` [PATCH V0 4/7] usb: common: eud: Added the driver support for Embedded USB Debugger(EUD) Souradeep Chowdhury
@ 2021-10-04 11:16 ` Souradeep Chowdhury
  2021-10-04 11:16 ` [PATCH V0 6/7] arm64: dts: qcom: sc7280: Set the default dr_mode for usb2 Souradeep Chowdhury
  2021-10-04 11:16 ` [PATCH V0 7/7] MAINTAINERS: Add maintainer entry for EUD Souradeep Chowdhury
  6 siblings, 0 replies; 26+ messages in thread
From: Souradeep Chowdhury @ 2021-10-04 11:16 UTC (permalink / raw)
  To: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Bjorn Andersson, Greg KH
  Cc: linux-kernel, ckadabi, tsoni, bryanh, psodagud, satyap, pheragu,
	Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan,
	Souradeep Chowdhury

Add the Embedded USB Debugger(EUD) connector node
as the child of dwc3 node under usb2. The node contains
EUD base register region and EUD mode manager register
regions along with the interrupt entry.

Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7280.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 53a21d0..21b16da 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -1308,6 +1308,9 @@
 			usb_2_dwc3: usb@8c00000 {
 				compatible = "snps,dwc3";
 				reg = <0 0x08c00000 0 0xe000>;
+				#address-cells = <2>;
+				#size-cells = <2>;
+				ranges;
 				interrupts = <GIC_SPI 242 IRQ_TYPE_LEVEL_HIGH>;
 				iommus = <&apps_smmu 0xa0 0x0>;
 				snps,dis_u2_susphy_quirk;
@@ -1315,6 +1318,15 @@
 				phys = <&usb_2_hsphy>;
 				phy-names = "usb2-phy";
 				maximum-speed = "high-speed";
+				usb-role-switch;
+				usb_con: eud_usb_connector {
+					compatible = "qcom,usb-connector-eud", "usb-c-connector",
+						     "qcom,sc7280-usb-connector-eud";
+					reg = <0 0x88e0000 0 0x2000>,
+					      <0 0x88e2000 0 0x1000>;
+					interrupt-parent = <&pdc>;
+					interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
+				};
 			};
 		};
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH V0 6/7] arm64: dts: qcom: sc7280: Set the default dr_mode for usb2
  2021-10-04 11:16 [PATCH V0 0/7] Add Embedded USB Debugger (EUD) driver Souradeep Chowdhury
                   ` (4 preceding siblings ...)
  2021-10-04 11:16 ` [PATCH V0 5/7] arm64: dts: qcom: sc7280: Add EUD connector node Souradeep Chowdhury
@ 2021-10-04 11:16 ` Souradeep Chowdhury
  2021-10-04 11:16 ` [PATCH V0 7/7] MAINTAINERS: Add maintainer entry for EUD Souradeep Chowdhury
  6 siblings, 0 replies; 26+ messages in thread
From: Souradeep Chowdhury @ 2021-10-04 11:16 UTC (permalink / raw)
  To: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Bjorn Andersson, Greg KH
  Cc: linux-kernel, ckadabi, tsoni, bryanh, psodagud, satyap, pheragu,
	Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan,
	Souradeep Chowdhury

Set the default dr_mode for usb2 node to "otg" to enable
role-switch for EUD(Embedded USB Debugger) connector node.

Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7280-idp.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
index 64fc22a..7c69c78 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
@@ -61,6 +61,10 @@
 	modem-init;
 };
 
+&usb_2_dwc3 {
+	dr_mode = "otg";
+};
+
 &pmk8350_vadc {
 	pmr735a_die_temp {
 		reg = <PMR735A_ADC7_DIE_TEMP>;
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH V0 7/7] MAINTAINERS: Add maintainer entry for EUD
  2021-10-04 11:16 [PATCH V0 0/7] Add Embedded USB Debugger (EUD) driver Souradeep Chowdhury
                   ` (5 preceding siblings ...)
  2021-10-04 11:16 ` [PATCH V0 6/7] arm64: dts: qcom: sc7280: Set the default dr_mode for usb2 Souradeep Chowdhury
@ 2021-10-04 11:16 ` Souradeep Chowdhury
  6 siblings, 0 replies; 26+ messages in thread
From: Souradeep Chowdhury @ 2021-10-04 11:16 UTC (permalink / raw)
  To: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Bjorn Andersson, Greg KH
  Cc: linux-kernel, ckadabi, tsoni, bryanh, psodagud, satyap, pheragu,
	Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan,
	Souradeep Chowdhury

Add the entry for maintainer for EUD driver
and other associated files.

Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ca6d6fd..5ef4832 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7019,6 +7019,13 @@ F:	include/trace/events/mdio.h
 F:	include/uapi/linux/mdio.h
 F:	include/uapi/linux/mii.h
 
+EUD-QCOM
+M:	Souradeep Chowdhury <schowdhu@codeaurora.org>
+L:	linux-arm-msm@vger.kernel.org
+S:	Maintained
+F:	Documentation/ABI/testing/sysfs-driver-eud
+F:	drivers/usb/common/qcom_eud.c
+
 EXFAT FILE SYSTEM
 M:	Namjae Jeon <linkinjeon@kernel.org>
 M:	Sungjong Seo <sj1557.seo@samsung.com>
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 4/7] usb: common: eud: Added the driver support for Embedded USB Debugger(EUD)
  2021-10-04 11:16 ` [PATCH V0 4/7] usb: common: eud: Added the driver support for Embedded USB Debugger(EUD) Souradeep Chowdhury
@ 2021-10-04 11:31   ` Greg KH
  2021-10-05 12:54     ` schowdhu
  2021-10-04 16:42   ` Bjorn Andersson
  2021-10-04 20:08   ` Randy Dunlap
  2 siblings, 1 reply; 26+ messages in thread
From: Greg KH @ 2021-10-04 11:31 UTC (permalink / raw)
  To: Souradeep Chowdhury
  Cc: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Bjorn Andersson, linux-kernel, ckadabi, tsoni, bryanh, psodagud,
	satyap, pheragu, Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan

On Mon, Oct 04, 2021 at 04:46:22PM +0530, Souradeep Chowdhury wrote:
> Add support for control peripheral of EUD (Embedded USB Debugger) to
> listen to events such as USB attach/detach, pet EUD to indicate software
> is functional.Reusing the platform device kobj, sysfs entry 'enable' is
> created to enable or disable EUD.
> 
> To enable the eud the following needs to be done
> echo 1 > /sys/bus/platform/.../enable
> 
> To disable eud, following is the command
> echo 0 > /sys/bus/platform/.../enable
> 
> Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
> ---
>  Documentation/ABI/testing/sysfs-driver-eud |   7 +
>  drivers/usb/common/Kconfig                 |   9 +
>  drivers/usb/common/Makefile                |   1 +
>  drivers/usb/common/qcom_eud.c              | 256 +++++++++++++++++++++++++++++
>  4 files changed, 273 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-eud
>  create mode 100644 drivers/usb/common/qcom_eud.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-eud b/Documentation/ABI/testing/sysfs-driver-eud
> new file mode 100644
> index 0000000..14a02da
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-eud
> @@ -0,0 +1,7 @@
> +What:		/sys/bus/platform/.../enable

Um, that's a _very_ generic regex, you just matched with any platform
device that might have a "enable" sysfs entry :(

Please use the dwc's name in here to help out with figuring this out.

> +Date:           October 2021
> +Contact:        Souradeep Chowdhury <schowdhu@codeaurora.org>

No tabs?

> +Description:
> +		The Enable/Disable sysfs interface for Embedded
> +		USB Debugger(EUD).This enables and disables the
> +		EUD based on a 1 or a 0 value.

What does enabling or disabling actually do?

And please use a ' ' after a '.'.

> diff --git a/drivers/usb/common/Kconfig b/drivers/usb/common/Kconfig
> index 5e8a04e..669b3fe 100644
> --- a/drivers/usb/common/Kconfig
> +++ b/drivers/usb/common/Kconfig
> @@ -50,3 +50,12 @@ config USB_CONN_GPIO
> 
>  	  To compile the driver as a module, choose M here: the module will
>  	  be called usb-conn-gpio.ko
> +
> +config USB_QCOM_EUD
> +	tristate "USB EUD Driver"
> +	select USB_ROLE_SWITCH
> +	help
> +	  This module enables support for Qualcomm Technologies, Inc.
> +	  Embedded USB Debugger (EUD). The EUD is a control peripheral
> +	  which reports VBUS attach/detach events and has USB-based
> +	  debug and trace capabilities.

What is the module name if this is built?

> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
> index 8ac4d21..eb66045 100644
> --- a/drivers/usb/common/Makefile
> +++ b/drivers/usb/common/Makefile
> @@ -11,3 +11,4 @@ usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>  obj-$(CONFIG_USB_CONN_GPIO)	+= usb-conn-gpio.o
>  obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
>  obj-$(CONFIG_USB_ULPI_BUS)	+= ulpi.o
> +obj-$(CONFIG_USB_QCOM_EUD)      += qcom_eud.o
> diff --git a/drivers/usb/common/qcom_eud.c b/drivers/usb/common/qcom_eud.c
> new file mode 100644
> index 0000000..7a92fff
> --- /dev/null
> +++ b/drivers/usb/common/qcom_eud.c
> @@ -0,0 +1,256 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +#include <linux/usb/role.h>
> +
> +#define EUD_REG_INT1_EN_MASK	0x0024
> +#define EUD_REG_INT_STATUS_1	0x0044
> +#define EUD_REG_CTL_OUT_1	0x0074
> +#define EUD_REG_VBUS_INT_CLR	0x0080
> +#define EUD_REG_CSR_EUD_EN	0x1014
> +#define EUD_REG_SW_ATTACH_DET	0x1018
> +#define EUD_REG_EUD_EN2         0x0000
> +
> +#define EUD_ENABLE		BIT(0)
> +#define EUD_INT_PET_EUD		BIT(0)
> +#define EUD_INT_VBUS		BIT(2)
> +#define EUD_INT_SAFE_MODE	BIT(4)
> +#define EUD_INT_ALL		(EUD_INT_VBUS|EUD_INT_SAFE_MODE)
> +
> +struct eud_chip {
> +	struct device			*dev;
> +	struct usb_role_switch		*role_sw;
> +	void __iomem			*eud_reg_base;
> +	void __iomem			*eud_mode_mgr2_phys_base;
> +	unsigned int			int_status;
> +	int				enable;

bool?

> +	int				eud_irq;
> +	bool				usb_attach;
> +

No need for a blank line.

> +};
> +
> +static int enable_eud(struct eud_chip *priv)
> +{
> +	writel(EUD_ENABLE, priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
> +	writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
> +			priv->eud_reg_base + EUD_REG_INT1_EN_MASK);
> +	writel(1, priv->eud_mode_mgr2_phys_base + EUD_REG_EUD_EN2);
> +
> +	return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE);
> +}
> +
> +static void disable_eud(struct eud_chip *priv)
> +{
> +	writel(0, priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
> +	writel(0, priv->eud_mode_mgr2_phys_base + EUD_REG_EUD_EN2);
> +}
> +
> +static ssize_t enable_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct eud_chip *chip = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%d", chip->enable);

sysfs_emit() please.

> +}
> +
> +static ssize_t enable_store(struct device *dev,
> +		struct device_attribute *attr,
> +		const char *buf, size_t count)
> +{
> +	struct eud_chip *chip = dev_get_drvdata(dev);
> +	unsigned long enable;
> +	int ret;
> +
> +	if (kstrtoul(buf, 16, &enable))
> +		return -EINVAL;

Use the default sysfs function to parse 0/1/y/n/Y/N please.

> +
> +	if (enable == 1) {
> +		ret = enable_eud(chip);
> +		if (!ret)
> +			chip->enable = enable;
> +	} else if (enable == 0) {
> +		disable_eud(chip);
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR_RW(enable);
> +
> +static const struct device_attribute *eud_attrs[] = {
> +	&dev_attr_enable,
> +	NULL,
> +};

You create a list of attributes and then never use it?

Who reviewed this thing?

You were so close in getting this right, see below...

> +
> +static void usb_attach_detach(struct eud_chip *chip)
> +{
> +	u32 reg;
> +
> +	/* read ctl_out_1[4] to find USB attach or detach event */
> +	reg = readl(chip->eud_reg_base + EUD_REG_CTL_OUT_1);
> +	if (reg & EUD_INT_SAFE_MODE)
> +		chip->usb_attach = true;
> +	else
> +		chip->usb_attach = false;
> +
> +	/* set and clear vbus_int_clr[0] to clear interrupt */
> +	writel(BIT(0), chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
> +	writel(0, chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
> +}
> +
> +static void pet_eud(struct eud_chip *chip)
> +{
> +	u32 reg;
> +	int ret;
> +
> +	/* read sw_attach_det[0] to find attach/detach event */
> +	reg = readl(chip->eud_reg_base +  EUD_REG_SW_ATTACH_DET);
> +	if (reg & EUD_INT_PET_EUD) {
> +		/* Detach & Attach pet for EUD */
> +		writel(0, chip->eud_reg_base + EUD_REG_SW_ATTACH_DET);
> +		/* Delay to make sure detach pet is done before attach pet */
> +		ret = readl_poll_timeout(chip->eud_reg_base + EUD_REG_SW_ATTACH_DET,
> +					reg, (reg == 0), 1, 100);
> +		if (ret) {
> +			dev_err(chip->dev, "Detach pet failed\n");
> +			return;
> +		}
> +
> +		writel(EUD_INT_PET_EUD, chip->eud_reg_base +
> +				EUD_REG_SW_ATTACH_DET);
> +	} else {
> +		/* Attach pet for EUD */
> +		writel(EUD_INT_PET_EUD, chip->eud_reg_base +
> +				EUD_REG_SW_ATTACH_DET);
> +	}
> +}
> +
> +static irqreturn_t handle_eud_irq(int irq, void *data)
> +{
> +	struct eud_chip *chip = data;
> +	u32 reg;
> +
> +	/* read status register and find out which interrupt triggered */
> +	reg = readl(chip->eud_reg_base +  EUD_REG_INT_STATUS_1);
> +	switch (reg & EUD_INT_ALL) {
> +	case EUD_INT_VBUS:
> +		chip->int_status = EUD_INT_VBUS;
> +		usb_attach_detach(chip);
> +		return IRQ_WAKE_THREAD;
> +	case EUD_INT_SAFE_MODE:
> +		pet_eud(chip);
> +		break;
> +	default:
> +		return IRQ_NONE;
> +	}
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t handle_eud_irq_thread(int irq, void *data)
> +{
> +	struct eud_chip *chip = data;
> +	int ret;
> +
> +	if (chip->int_status == EUD_INT_VBUS) {
> +		if (chip->usb_attach)
> +			ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_DEVICE);
> +		else
> +			ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_HOST);
> +		if (ret)
> +			dev_err(chip->dev, "failed to set role switch\n");
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int eud_probe(struct platform_device *pdev)
> +{
> +	struct eud_chip *chip;
> +	int ret;
> +
> +	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
> +	if (!chip)
> +		return -ENOMEM;
> +
> +	chip->dev = &pdev->dev;
> +
> +	chip->role_sw = usb_role_switch_get(chip->dev);
> +	if (IS_ERR(chip->role_sw)) {
> +		if (PTR_ERR(chip->role_sw) != -EPROBE_DEFER)
> +			dev_err(chip->dev, "failed to get role switch\n");
> +
> +		return PTR_ERR(chip->role_sw);
> +	}
> +
> +	chip->eud_reg_base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(chip->eud_reg_base))
> +		return PTR_ERR(chip->eud_reg_base);
> +
> +	chip->eud_mode_mgr2_phys_base = devm_platform_ioremap_resource(pdev, 1);
> +	if (IS_ERR(chip->eud_mode_mgr2_phys_base))
> +		return PTR_ERR(chip->eud_mode_mgr2_phys_base);
> +
> +	chip->eud_irq = platform_get_irq(pdev, 0);
> +	ret = devm_request_threaded_irq(&pdev->dev, chip->eud_irq, handle_eud_irq,
> +			handle_eud_irq_thread, IRQF_ONESHOT, NULL, chip);
> +	if (ret)
> +		return ret;
> +
> +	device_init_wakeup(&pdev->dev, true);
> +	enable_irq_wake(chip->eud_irq);
> +
> +	platform_set_drvdata(pdev, chip);
> +
> +	ret = device_create_file(&pdev->dev, eud_attrs[0]);

You raced with userspace and lost :(

Please properly attach the sysfs file to the platform driver to have the
driver core register this correctly.

> +
> +	return ret;
> +}
> +
> +static int eud_remove(struct platform_device *pdev)
> +{
> +	struct eud_chip *chip = platform_get_drvdata(pdev);
> +
> +	if (chip->enable)
> +		disable_eud(chip);
> +
> +	device_remove_file(&pdev->dev, eud_attrs[0]);

No need for this if you do the above correctly.

> +	device_init_wakeup(&pdev->dev, false);

Why is this needed?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 2/7] dt-bindings: usb: dwc3: Update dwc3 properties for EUD connector
  2021-10-04 11:16 ` [PATCH V0 2/7] dt-bindings: usb: dwc3: Update dwc3 properties for EUD connector Souradeep Chowdhury
@ 2021-10-04 15:50   ` Bjorn Andersson
  2021-10-05 12:57     ` schowdhu
  0 siblings, 1 reply; 26+ messages in thread
From: Bjorn Andersson @ 2021-10-04 15:50 UTC (permalink / raw)
  To: Souradeep Chowdhury
  Cc: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Greg KH, linux-kernel, ckadabi, tsoni, bryanh, psodagud, satyap,
	pheragu, Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan

On Mon 04 Oct 04:16 PDT 2021, Souradeep Chowdhury wrote:

> Adding the address size,cell size and ranges property for EUD connector.
> Adding the connector property for EUD which is child of dwc3 node.
> 

When we have a Type-C controller involved, the connector is described
using of_graph, so will we not then have two different connectors
described, in two different ways?

Regards,
Bjorn

> Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> index 078fb78..3e71205 100644
> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
> @@ -36,6 +36,14 @@ properties:
>          - const: synopsys,dwc3
>            deprecated: true
>  
> +  "#address-cells":
> +    enum: [ 1, 2 ]
> +
> +  "#size-cells":
> +    enum: [ 1, 2 ]
> +
> +  ranges: true
> +
>    interrupts:
>      description:
>        It's either a single common DWC3 interrupt (dwc_usb3) or individual
> @@ -318,6 +326,13 @@ properties:
>      items:
>        enum: [1, 4, 8, 16, 32, 64, 128, 256]
>  
> +  connector:
> +    type: object
> +    $ref: /connector/usb-connector.yaml#
> +    description:
> +      Connector for dual role switch, especially for "eud-usb-c-connector"
> +
> +
>  unevaluatedProperties: false
>  
>  required:
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 1/7] dt-bindings: connector: Add property for eud type c connector
  2021-10-04 11:16 ` [PATCH V0 1/7] dt-bindings: connector: Add property for eud type c connector Souradeep Chowdhury
@ 2021-10-04 16:37   ` Rob Herring
  2021-10-05 13:11     ` schowdhu
  0 siblings, 1 reply; 26+ messages in thread
From: Rob Herring @ 2021-10-04 16:37 UTC (permalink / raw)
  To: Souradeep Chowdhury
  Cc: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Bjorn Andersson, Greg KH, linux-kernel, ckadabi, tsoni, bryanh,
	psodagud, satyap, pheragu, Rajendra Nayak, Sibi Sankar,
	Sai Prakash Ranjan

On Mon, Oct 04, 2021 at 04:46:19PM +0530, Souradeep Chowdhury wrote:
> Added the property for EUD(Embedded USB Debug) connector.Added
> the "reg" and "interrupts" property which is needed for EUD.

You are going to need a better explanation of this h/w.

> 
> Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
> ---
>  .../devicetree/bindings/connector/usb-connector.yaml      | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> index 7eb8659..908129f 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> @@ -30,6 +30,21 @@ properties:
>            - const: samsung,usb-connector-11pin
>            - const: usb-b-connector
>  
> +      - items:
> +          - enum:
> +              - qcom,sc7280-usb-connector-eud
> +          - const: qcom,usb-connector-eud
> +          - const: usb-c-connector
> +
> +  reg:
> +    items:
> +      - description: EUD Base Register Region
> +      - description: EUD Mode Manager Region

A connector node represents the physical connector on a board. That 
can't really be an MMIO peripheral. Maybe you need a node for EUD and 
then it should have a connector child node? Don't really know without 
understanding this h/w.

> +
> +  interrupts:
> +    description:
> +      EUD interrupt
> +
>    label:
>      description: Symbolic name for the connector.
>  
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> 
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 4/7] usb: common: eud: Added the driver support for Embedded USB Debugger(EUD)
  2021-10-04 11:16 ` [PATCH V0 4/7] usb: common: eud: Added the driver support for Embedded USB Debugger(EUD) Souradeep Chowdhury
  2021-10-04 11:31   ` Greg KH
@ 2021-10-04 16:42   ` Bjorn Andersson
  2021-10-05  7:14     ` Joe Perches
  2021-10-05 13:01     ` schowdhu
  2021-10-04 20:08   ` Randy Dunlap
  2 siblings, 2 replies; 26+ messages in thread
From: Bjorn Andersson @ 2021-10-04 16:42 UTC (permalink / raw)
  To: Souradeep Chowdhury
  Cc: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Greg KH, linux-kernel, ckadabi, tsoni, bryanh, psodagud, satyap,
	pheragu, Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan

On Mon 04 Oct 04:16 PDT 2021, Souradeep Chowdhury wrote:

> Add support for control peripheral of EUD (Embedded USB Debugger) to
> listen to events such as USB attach/detach, pet EUD to indicate software
> is functional.Reusing the platform device kobj, sysfs entry 'enable' is
> created to enable or disable EUD.
> 
> To enable the eud the following needs to be done
> echo 1 > /sys/bus/platform/.../enable
> 
> To disable eud, following is the command
> echo 0 > /sys/bus/platform/.../enable
> 
> Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
> ---
>  Documentation/ABI/testing/sysfs-driver-eud |   7 +
>  drivers/usb/common/Kconfig                 |   9 +
>  drivers/usb/common/Makefile                |   1 +
>  drivers/usb/common/qcom_eud.c              | 256 +++++++++++++++++++++++++++++
>  4 files changed, 273 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-eud
>  create mode 100644 drivers/usb/common/qcom_eud.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-eud b/Documentation/ABI/testing/sysfs-driver-eud
> new file mode 100644
> index 0000000..14a02da
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-eud
> @@ -0,0 +1,7 @@
> +What:		/sys/bus/platform/.../enable
> +Date:           October 2021
> +Contact:        Souradeep Chowdhury <schowdhu@codeaurora.org>
> +Description:
> +		The Enable/Disable sysfs interface for Embedded
> +		USB Debugger(EUD).This enables and disables the
> +		EUD based on a 1 or a 0 value.
> diff --git a/drivers/usb/common/Kconfig b/drivers/usb/common/Kconfig
> index 5e8a04e..669b3fe 100644
> --- a/drivers/usb/common/Kconfig
> +++ b/drivers/usb/common/Kconfig
> @@ -50,3 +50,12 @@ config USB_CONN_GPIO
> 
>  	  To compile the driver as a module, choose M here: the module will
>  	  be called usb-conn-gpio.ko
> +
> +config USB_QCOM_EUD
> +	tristate "USB EUD Driver"
> +	select USB_ROLE_SWITCH
> +	help
> +	  This module enables support for Qualcomm Technologies, Inc.
> +	  Embedded USB Debugger (EUD). The EUD is a control peripheral
> +	  which reports VBUS attach/detach events and has USB-based
> +	  debug and trace capabilities.
> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
> index 8ac4d21..eb66045 100644
> --- a/drivers/usb/common/Makefile
> +++ b/drivers/usb/common/Makefile
> @@ -11,3 +11,4 @@ usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>  obj-$(CONFIG_USB_CONN_GPIO)	+= usb-conn-gpio.o
>  obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
>  obj-$(CONFIG_USB_ULPI_BUS)	+= ulpi.o
> +obj-$(CONFIG_USB_QCOM_EUD)      += qcom_eud.o
> diff --git a/drivers/usb/common/qcom_eud.c b/drivers/usb/common/qcom_eud.c
> new file mode 100644
> index 0000000..7a92fff
> --- /dev/null
> +++ b/drivers/usb/common/qcom_eud.c
> @@ -0,0 +1,256 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +#include <linux/usb/role.h>
> +
> +#define EUD_REG_INT1_EN_MASK	0x0024
> +#define EUD_REG_INT_STATUS_1	0x0044
> +#define EUD_REG_CTL_OUT_1	0x0074
> +#define EUD_REG_VBUS_INT_CLR	0x0080
> +#define EUD_REG_CSR_EUD_EN	0x1014
> +#define EUD_REG_SW_ATTACH_DET	0x1018
> +#define EUD_REG_EUD_EN2         0x0000
> +
> +#define EUD_ENABLE		BIT(0)
> +#define EUD_INT_PET_EUD		BIT(0)
> +#define EUD_INT_VBUS		BIT(2)
> +#define EUD_INT_SAFE_MODE	BIT(4)
> +#define EUD_INT_ALL		(EUD_INT_VBUS|EUD_INT_SAFE_MODE)
> +
> +struct eud_chip {
> +	struct device			*dev;
> +	struct usb_role_switch		*role_sw;
> +	void __iomem			*eud_reg_base;
> +	void __iomem			*eud_mode_mgr2_phys_base;
> +	unsigned int			int_status;
> +	int				enable;
> +	int				eud_irq;
> +	bool				usb_attach;
> +
> +};
> +
> +static int enable_eud(struct eud_chip *priv)
> +{
> +	writel(EUD_ENABLE, priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
> +	writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
> +			priv->eud_reg_base + EUD_REG_INT1_EN_MASK);
> +	writel(1, priv->eud_mode_mgr2_phys_base + EUD_REG_EUD_EN2);
> +
> +	return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE);
> +}
> +
> +static void disable_eud(struct eud_chip *priv)
> +{
> +	writel(0, priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
> +	writel(0, priv->eud_mode_mgr2_phys_base + EUD_REG_EUD_EN2);
> +}
> +
> +static ssize_t enable_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct eud_chip *chip = dev_get_drvdata(dev);
> +
> +	return sprintf(buf, "%d", chip->enable);
> +}
> +
> +static ssize_t enable_store(struct device *dev,
> +		struct device_attribute *attr,
> +		const char *buf, size_t count)
> +{
> +	struct eud_chip *chip = dev_get_drvdata(dev);
> +	unsigned long enable;
> +	int ret;
> +
> +	if (kstrtoul(buf, 16, &enable))
> +		return -EINVAL;
> +
> +	if (enable == 1) {
> +		ret = enable_eud(chip);
> +		if (!ret)
> +			chip->enable = enable;
> +	} else if (enable == 0) {
> +		disable_eud(chip);
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR_RW(enable);
> +
> +static const struct device_attribute *eud_attrs[] = {
> +	&dev_attr_enable,
> +	NULL,
> +};
> +
> +static void usb_attach_detach(struct eud_chip *chip)
> +{
> +	u32 reg;
> +
> +	/* read ctl_out_1[4] to find USB attach or detach event */
> +	reg = readl(chip->eud_reg_base + EUD_REG_CTL_OUT_1);
> +	if (reg & EUD_INT_SAFE_MODE)
> +		chip->usb_attach = true;
> +	else
> +		chip->usb_attach = false;
> +
> +	/* set and clear vbus_int_clr[0] to clear interrupt */
> +	writel(BIT(0), chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
> +	writel(0, chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
> +}
> +
> +static void pet_eud(struct eud_chip *chip)
> +{
> +	u32 reg;
> +	int ret;
> +
> +	/* read sw_attach_det[0] to find attach/detach event */
> +	reg = readl(chip->eud_reg_base +  EUD_REG_SW_ATTACH_DET);
> +	if (reg & EUD_INT_PET_EUD) {
> +		/* Detach & Attach pet for EUD */
> +		writel(0, chip->eud_reg_base + EUD_REG_SW_ATTACH_DET);
> +		/* Delay to make sure detach pet is done before attach pet */
> +		ret = readl_poll_timeout(chip->eud_reg_base + EUD_REG_SW_ATTACH_DET,
> +					reg, (reg == 0), 1, 100);
> +		if (ret) {
> +			dev_err(chip->dev, "Detach pet failed\n");
> +			return;
> +		}
> +
> +		writel(EUD_INT_PET_EUD, chip->eud_reg_base +
> +				EUD_REG_SW_ATTACH_DET);
> +	} else {
> +		/* Attach pet for EUD */
> +		writel(EUD_INT_PET_EUD, chip->eud_reg_base +
> +				EUD_REG_SW_ATTACH_DET);
> +	}
> +}
> +
> +static irqreturn_t handle_eud_irq(int irq, void *data)
> +{
> +	struct eud_chip *chip = data;
> +	u32 reg;
> +
> +	/* read status register and find out which interrupt triggered */
> +	reg = readl(chip->eud_reg_base +  EUD_REG_INT_STATUS_1);
> +	switch (reg & EUD_INT_ALL) {
> +	case EUD_INT_VBUS:
> +		chip->int_status = EUD_INT_VBUS;
> +		usb_attach_detach(chip);
> +		return IRQ_WAKE_THREAD;
> +	case EUD_INT_SAFE_MODE:
> +		pet_eud(chip);
> +		break;
> +	default:
> +		return IRQ_NONE;
> +	}
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t handle_eud_irq_thread(int irq, void *data)
> +{
> +	struct eud_chip *chip = data;
> +	int ret;
> +
> +	if (chip->int_status == EUD_INT_VBUS) {
> +		if (chip->usb_attach)
> +			ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_DEVICE);
> +		else
> +			ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_HOST);

How does this work if I have a Type-C controller wired up to the dwc3
and it has negotiated that we're supposed to be in device mode?

Shouldn't this driver somehow work as an override only when EUD is
enabled, but otherwise let the Type-C controller deal with things?

Or is this interrupt simply not delivered when EUD is disabled, so that
it happens to work as expected?

> +		if (ret)
> +			dev_err(chip->dev, "failed to set role switch\n");
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int eud_probe(struct platform_device *pdev)
> +{
> +	struct eud_chip *chip;
> +	int ret;
> +
> +	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
> +	if (!chip)
> +		return -ENOMEM;
> +
> +	chip->dev = &pdev->dev;
> +
> +	chip->role_sw = usb_role_switch_get(chip->dev);
> +	if (IS_ERR(chip->role_sw)) {
> +		if (PTR_ERR(chip->role_sw) != -EPROBE_DEFER)
> +			dev_err(chip->dev, "failed to get role switch\n");
> +
> +		return PTR_ERR(chip->role_sw);

Please use
		dev_err_probe(chip->dev, PTR_ERR(chip->role_sw), "failed to...\n");

> +	}
> +
> +	chip->eud_reg_base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(chip->eud_reg_base))
> +		return PTR_ERR(chip->eud_reg_base);
> +
> +	chip->eud_mode_mgr2_phys_base = devm_platform_ioremap_resource(pdev, 1);
> +	if (IS_ERR(chip->eud_mode_mgr2_phys_base))
> +		return PTR_ERR(chip->eud_mode_mgr2_phys_base);
> +
> +	chip->eud_irq = platform_get_irq(pdev, 0);
> +	ret = devm_request_threaded_irq(&pdev->dev, chip->eud_irq, handle_eud_irq,
> +			handle_eud_irq_thread, IRQF_ONESHOT, NULL, chip);
> +	if (ret)

This deserved another dev_err_probe()

Thanks,
Bjorn

> +		return ret;
> +
> +	device_init_wakeup(&pdev->dev, true);
> +	enable_irq_wake(chip->eud_irq);
> +
> +	platform_set_drvdata(pdev, chip);
> +
> +	ret = device_create_file(&pdev->dev, eud_attrs[0]);
> +
> +	return ret;
> +}
> +
> +static int eud_remove(struct platform_device *pdev)
> +{
> +	struct eud_chip *chip = platform_get_drvdata(pdev);
> +
> +	if (chip->enable)
> +		disable_eud(chip);
> +
> +	device_remove_file(&pdev->dev, eud_attrs[0]);
> +	device_init_wakeup(&pdev->dev, false);
> +	disable_irq_wake(chip->eud_irq);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id eud_dt_match[] = {
> +	{ .compatible = "qcom,usb-connector-eud" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, eud_dt_match);
> +
> +static struct platform_driver eud_driver = {
> +	.probe		= eud_probe,
> +	.remove		= eud_remove,
> +	.driver		= {
> +		.name		= "eud",
> +		.of_match_table = eud_dt_match,
> +	},
> +};
> +module_platform_driver(eud_driver);
> +
> +MODULE_DESCRIPTION("QTI EUD driver");
> +MODULE_LICENSE("GPL v2");
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 4/7] usb: common: eud: Added the driver support for Embedded USB Debugger(EUD)
  2021-10-04 11:16 ` [PATCH V0 4/7] usb: common: eud: Added the driver support for Embedded USB Debugger(EUD) Souradeep Chowdhury
  2021-10-04 11:31   ` Greg KH
  2021-10-04 16:42   ` Bjorn Andersson
@ 2021-10-04 20:08   ` Randy Dunlap
  2021-10-05 13:57     ` schowdhu
  2 siblings, 1 reply; 26+ messages in thread
From: Randy Dunlap @ 2021-10-04 20:08 UTC (permalink / raw)
  To: Souradeep Chowdhury, linux-arm-msm, linux-usb, devicetree,
	Bryan O'Donoghue, Bjorn Andersson, Greg KH
  Cc: linux-kernel, ckadabi, tsoni, bryanh, psodagud, satyap, pheragu,
	Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan

On 10/4/21 4:16 AM, Souradeep Chowdhury wrote:
> Add support for control peripheral of EUD (Embedded USB Debugger) to
> listen to events such as USB attach/detach, pet EUD to indicate software

   I don't quite understand: what pets the EUD? how does it do that?

> is functional.Reusing the platform device kobj, sysfs entry 'enable' is

      functional. Reusing

> created to enable or disable EUD.
> 
> To enable the eud the following needs to be done
> echo 1 >/sys/bus/platform/.../enable
> 
> To disable eud, following is the command
> echo 0 >/sys/bus/platform/.../enable
> 
> Signed-off-by: Souradeep Chowdhury<schowdhu@codeaurora.org>
> ---
>   Documentation/ABI/testing/sysfs-driver-eud |   7 +
>   drivers/usb/common/Kconfig                 |   9 +
>   drivers/usb/common/Makefile                |   1 +
>   drivers/usb/common/qcom_eud.c              | 256 +++++++++++++++++++++++++++++
>   4 files changed, 273 insertions(+)
>   create mode 100644 Documentation/ABI/testing/sysfs-driver-eud
>   create mode 100644 drivers/usb/common/qcom_eud.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-eud b/Documentation/ABI/testing/sysfs-driver-eud
> new file mode 100644
> index 0000000..14a02da
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-eud
> @@ -0,0 +1,7 @@
> +What:		/sys/bus/platform/.../enable
> +Date:           October 2021
> +Contact:        Souradeep Chowdhury<schowdhu@codeaurora.org>
> +Description:
> +		The Enable/Disable sysfs interface for Embedded
> +		USB Debugger(EUD).This enables and disables the

		    Debugger (EUD). This enables

> +		EUD based on a 1 or a 0 value.


-- 
~Randy

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 3/7] usb: dwc3: drd: Register the eud connector child node for dwc3
  2021-10-04 11:16 ` [PATCH V0 3/7] usb: dwc3: drd: Register the eud connector child node for dwc3 Souradeep Chowdhury
@ 2021-10-05  4:42   ` kernel test robot
  2021-10-05 12:32   ` kernel test robot
  1 sibling, 0 replies; 26+ messages in thread
From: kernel test robot @ 2021-10-05  4:42 UTC (permalink / raw)
  To: Souradeep Chowdhury, linux-arm-msm, linux-usb, devicetree,
	Bryan O'Donoghue, Bjorn Andersson, Greg KH
  Cc: llvm, kbuild-all, linux-kernel, ckadabi, tsoni, bryanh

[-- Attachment #1: Type: text/plain, Size: 2642 bytes --]

Hi Souradeep,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on robh/for-next linus/master v5.15-rc3 next-20210922]
[cannot apply to balbi-usb/testing/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Souradeep-Chowdhury/Add-Embedded-USB-Debugger-EUD-driver/20211004-191901
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: i386-randconfig-a004-20211004 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c0039de2953d15815448b4b3c3bafb45607781e0)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/4f53df1d9b6786f951384f59e3ffa7fed1817a2d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Souradeep-Chowdhury/Add-Embedded-USB-Debugger-EUD-driver/20211004-191901
        git checkout 4f53df1d9b6786f951384f59e3ffa7fed1817a2d
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/usb/dwc3/drd.c:171:47: warning: variable 'con_np' set but not used [-Wunused-but-set-variable]
           struct device_node      *np = dev->of_node, *con_np;
                                                        ^
   1 warning generated.


vim +/con_np +171 drivers/usb/dwc3/drd.c

   167	
   168	static int dwc3_register_eud(struct dwc3 *dwc)
   169	{
   170		struct device           *dev = dwc->dev;
 > 171		struct device_node      *np = dev->of_node, *con_np;
   172		int                     ret;
   173	
   174		con_np = of_get_child_by_name(np, "eud_usb_connector");
   175		if (!np) {
   176			dev_dbg(dev, "no usb_connector child node specified\n");
   177			return 0;
   178		}
   179	
   180		ret = of_platform_populate(np, NULL, NULL, dev);
   181		if (ret) {
   182			dev_err(dev, "failed to register usb_connector - %d\n", ret);
   183			return ret;
   184		}
   185	
   186		return 0;
   187	}
   188	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32125 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 4/7] usb: common: eud: Added the driver support for Embedded USB Debugger(EUD)
  2021-10-04 16:42   ` Bjorn Andersson
@ 2021-10-05  7:14     ` Joe Perches
  2021-10-05 13:58       ` schowdhu
  2021-10-05 13:01     ` schowdhu
  1 sibling, 1 reply; 26+ messages in thread
From: Joe Perches @ 2021-10-05  7:14 UTC (permalink / raw)
  To: Bjorn Andersson, Souradeep Chowdhury
  Cc: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Greg KH, linux-kernel, ckadabi, tsoni, bryanh, psodagud, satyap,
	pheragu, Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan

On Mon, 2021-10-04 at 09:42 -0700, Bjorn Andersson wrote:
> On Mon 04 Oct 04:16 PDT 2021, Souradeep Chowdhury wrote:
> 
> > Add support for control peripheral of EUD (Embedded USB Debugger) to
> > listen to events such as USB attach/detach, pet EUD to indicate software
> > is functional.Reusing the platform device kobj, sysfs entry 'enable' is
> > created to enable or disable EUD.
[]
> > diff --git a/drivers/usb/common/qcom_eud.c b/drivers/usb/common/qcom_eud.c
[]
> > +static ssize_t enable_show(struct device *dev,
> > +		struct device_attribute *attr, char *buf)
> > +{
> > +	struct eud_chip *chip = dev_get_drvdata(dev);
> > +
> > +	return sprintf(buf, "%d", chip->enable);

trivia:

should probably use sysfs_emit and have a trailing '\n' in the format.



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 3/7] usb: dwc3: drd: Register the eud connector child node for dwc3
  2021-10-04 11:16 ` [PATCH V0 3/7] usb: dwc3: drd: Register the eud connector child node for dwc3 Souradeep Chowdhury
  2021-10-05  4:42   ` kernel test robot
@ 2021-10-05 12:32   ` kernel test robot
  1 sibling, 0 replies; 26+ messages in thread
From: kernel test robot @ 2021-10-05 12:32 UTC (permalink / raw)
  To: Souradeep Chowdhury, linux-arm-msm, linux-usb, devicetree,
	Bryan O'Donoghue, Bjorn Andersson, Greg KH
  Cc: kbuild-all, linux-kernel, ckadabi, tsoni, bryanh

[-- Attachment #1: Type: text/plain, Size: 2651 bytes --]

Hi Souradeep,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on robh/for-next linus/master v5.15-rc3 next-20210922]
[cannot apply to balbi-usb/testing/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Souradeep-Chowdhury/Add-Embedded-USB-Debugger-EUD-driver/20211004-191901
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: arc-allyesconfig (attached as .config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/4f53df1d9b6786f951384f59e3ffa7fed1817a2d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Souradeep-Chowdhury/Add-Embedded-USB-Debugger-EUD-driver/20211004-191901
        git checkout 4f53df1d9b6786f951384f59e3ffa7fed1817a2d
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=arc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/usb/dwc3/drd.c: In function 'dwc3_register_eud':
>> drivers/usb/dwc3/drd.c:171:54: error: variable 'con_np' set but not used [-Werror=unused-but-set-variable]
     171 |         struct device_node      *np = dev->of_node, *con_np;
         |                                                      ^~~~~~
   cc1: all warnings being treated as errors


vim +/con_np +171 drivers/usb/dwc3/drd.c

   167	
   168	static int dwc3_register_eud(struct dwc3 *dwc)
   169	{
   170		struct device           *dev = dwc->dev;
 > 171		struct device_node      *np = dev->of_node, *con_np;
   172		int                     ret;
   173	
   174		con_np = of_get_child_by_name(np, "eud_usb_connector");
   175		if (!np) {
   176			dev_dbg(dev, "no usb_connector child node specified\n");
   177			return 0;
   178		}
   179	
   180		ret = of_platform_populate(np, NULL, NULL, dev);
   181		if (ret) {
   182			dev_err(dev, "failed to register usb_connector - %d\n", ret);
   183			return ret;
   184		}
   185	
   186		return 0;
   187	}
   188	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 69189 bytes --]

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 4/7] usb: common: eud: Added the driver support for Embedded USB Debugger(EUD)
  2021-10-04 11:31   ` Greg KH
@ 2021-10-05 12:54     ` schowdhu
  0 siblings, 0 replies; 26+ messages in thread
From: schowdhu @ 2021-10-05 12:54 UTC (permalink / raw)
  To: Greg KH
  Cc: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Bjorn Andersson, linux-kernel, ckadabi, tsoni, bryanh, psodagud,
	satyap, pheragu, Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan

On 2021-10-04 17:01, Greg KH wrote:
> On Mon, Oct 04, 2021 at 04:46:22PM +0530, Souradeep Chowdhury wrote:
>> Add support for control peripheral of EUD (Embedded USB Debugger) to
>> listen to events such as USB attach/detach, pet EUD to indicate 
>> software
>> is functional.Reusing the platform device kobj, sysfs entry 'enable' 
>> is
>> created to enable or disable EUD.
>> 
>> To enable the eud the following needs to be done
>> echo 1 > /sys/bus/platform/.../enable
>> 
>> To disable eud, following is the command
>> echo 0 > /sys/bus/platform/.../enable
>> 
>> Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
>> ---
>>  Documentation/ABI/testing/sysfs-driver-eud |   7 +
>>  drivers/usb/common/Kconfig                 |   9 +
>>  drivers/usb/common/Makefile                |   1 +
>>  drivers/usb/common/qcom_eud.c              | 256 
>> +++++++++++++++++++++++++++++
>>  4 files changed, 273 insertions(+)
>>  create mode 100644 Documentation/ABI/testing/sysfs-driver-eud
>>  create mode 100644 drivers/usb/common/qcom_eud.c
>> 
>> diff --git a/Documentation/ABI/testing/sysfs-driver-eud 
>> b/Documentation/ABI/testing/sysfs-driver-eud
>> new file mode 100644
>> index 0000000..14a02da
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-driver-eud
>> @@ -0,0 +1,7 @@
>> +What:		/sys/bus/platform/.../enable
> 
> Um, that's a _very_ generic regex, you just matched with any platform
> device that might have a "enable" sysfs entry :(
> 
> Please use the dwc's name in here to help out with figuring this out.

Ack

> 
>> +Date:           October 2021
>> +Contact:        Souradeep Chowdhury <schowdhu@codeaurora.org>
> 
> No tabs?
> 
>> +Description:
>> +		The Enable/Disable sysfs interface for Embedded
>> +		USB Debugger(EUD).This enables and disables the
>> +		EUD based on a 1 or a 0 value.
> 
> What does enabling or disabling actually do?
> 
> And please use a ' ' after a '.'.

Ack. On enabling, the debug mode is set for EUD. EUD is a mux, that sits 
between the connector and the
controller, routing UTMI signals to an internal USB hub, which in-turn 
has debug functions attached to
the hub. On disabling, this routing stops as EUD gets disabled.

> 
>> diff --git a/drivers/usb/common/Kconfig b/drivers/usb/common/Kconfig
>> index 5e8a04e..669b3fe 100644
>> --- a/drivers/usb/common/Kconfig
>> +++ b/drivers/usb/common/Kconfig
>> @@ -50,3 +50,12 @@ config USB_CONN_GPIO
>> 
>>  	  To compile the driver as a module, choose M here: the module will
>>  	  be called usb-conn-gpio.ko
>> +
>> +config USB_QCOM_EUD
>> +	tristate "USB EUD Driver"
>> +	select USB_ROLE_SWITCH
>> +	help
>> +	  This module enables support for Qualcomm Technologies, Inc.
>> +	  Embedded USB Debugger (EUD). The EUD is a control peripheral
>> +	  which reports VBUS attach/detach events and has USB-based
>> +	  debug and trace capabilities.
> 
> What is the module name if this is built?

Ack. The module name for this is same as the driver name qcom_eud.ko, 
will update the same here.

> 
>> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
>> index 8ac4d21..eb66045 100644
>> --- a/drivers/usb/common/Makefile
>> +++ b/drivers/usb/common/Makefile
>> @@ -11,3 +11,4 @@ usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>>  obj-$(CONFIG_USB_CONN_GPIO)	+= usb-conn-gpio.o
>>  obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
>>  obj-$(CONFIG_USB_ULPI_BUS)	+= ulpi.o
>> +obj-$(CONFIG_USB_QCOM_EUD)      += qcom_eud.o
>> diff --git a/drivers/usb/common/qcom_eud.c 
>> b/drivers/usb/common/qcom_eud.c
>> new file mode 100644
>> index 0000000..7a92fff
>> --- /dev/null
>> +++ b/drivers/usb/common/qcom_eud.c
>> @@ -0,0 +1,256 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2015-2021, The Linux Foundation. All rights 
>> reserved.
>> + */
>> +
>> +#include <linux/bitops.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <linux/sysfs.h>
>> +#include <linux/usb/role.h>
>> +
>> +#define EUD_REG_INT1_EN_MASK	0x0024
>> +#define EUD_REG_INT_STATUS_1	0x0044
>> +#define EUD_REG_CTL_OUT_1	0x0074
>> +#define EUD_REG_VBUS_INT_CLR	0x0080
>> +#define EUD_REG_CSR_EUD_EN	0x1014
>> +#define EUD_REG_SW_ATTACH_DET	0x1018
>> +#define EUD_REG_EUD_EN2         0x0000
>> +
>> +#define EUD_ENABLE		BIT(0)
>> +#define EUD_INT_PET_EUD		BIT(0)
>> +#define EUD_INT_VBUS		BIT(2)
>> +#define EUD_INT_SAFE_MODE	BIT(4)
>> +#define EUD_INT_ALL		(EUD_INT_VBUS|EUD_INT_SAFE_MODE)
>> +
>> +struct eud_chip {
>> +	struct device			*dev;
>> +	struct usb_role_switch		*role_sw;
>> +	void __iomem			*eud_reg_base;
>> +	void __iomem			*eud_mode_mgr2_phys_base;
>> +	unsigned int			int_status;
>> +	int				enable;
> 
> bool?

Ack.

> 
>> +	int				eud_irq;
>> +	bool				usb_attach;
>> +
> 
> No need for a blank line.

Ack

> 
>> +};
>> +
>> +static int enable_eud(struct eud_chip *priv)
>> +{
>> +	writel(EUD_ENABLE, priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
>> +	writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
>> +			priv->eud_reg_base + EUD_REG_INT1_EN_MASK);
>> +	writel(1, priv->eud_mode_mgr2_phys_base + EUD_REG_EUD_EN2);
>> +
>> +	return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE);
>> +}
>> +
>> +static void disable_eud(struct eud_chip *priv)
>> +{
>> +	writel(0, priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
>> +	writel(0, priv->eud_mode_mgr2_phys_base + EUD_REG_EUD_EN2);
>> +}
>> +
>> +static ssize_t enable_show(struct device *dev,
>> +		struct device_attribute *attr, char *buf)
>> +{
>> +	struct eud_chip *chip = dev_get_drvdata(dev);
>> +
>> +	return sprintf(buf, "%d", chip->enable);
> 
> sysfs_emit() please.

Ack

> 
>> +}
>> +
>> +static ssize_t enable_store(struct device *dev,
>> +		struct device_attribute *attr,
>> +		const char *buf, size_t count)
>> +{
>> +	struct eud_chip *chip = dev_get_drvdata(dev);
>> +	unsigned long enable;
>> +	int ret;
>> +
>> +	if (kstrtoul(buf, 16, &enable))
>> +		return -EINVAL;
> 
> Use the default sysfs function to parse 0/1/y/n/Y/N please.

Ack

> 
>> +
>> +	if (enable == 1) {
>> +		ret = enable_eud(chip);
>> +		if (!ret)
>> +			chip->enable = enable;
>> +	} else if (enable == 0) {
>> +		disable_eud(chip);
>> +	} else {
>> +		return -EINVAL;
>> +	}
>> +
>> +	return count;
>> +}
>> +
>> +static DEVICE_ATTR_RW(enable);
>> +
>> +static const struct device_attribute *eud_attrs[] = {
>> +	&dev_attr_enable,
>> +	NULL,
>> +};
> 
> You create a list of attributes and then never use it?
> 
> Who reviewed this thing?
> 
> You were so close in getting this right, see below...

Ack

> 
>> +
>> +static void usb_attach_detach(struct eud_chip *chip)
>> +{
>> +	u32 reg;
>> +
>> +	/* read ctl_out_1[4] to find USB attach or detach event */
>> +	reg = readl(chip->eud_reg_base + EUD_REG_CTL_OUT_1);
>> +	if (reg & EUD_INT_SAFE_MODE)
>> +		chip->usb_attach = true;
>> +	else
>> +		chip->usb_attach = false;
>> +
>> +	/* set and clear vbus_int_clr[0] to clear interrupt */
>> +	writel(BIT(0), chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
>> +	writel(0, chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
>> +}
>> +
>> +static void pet_eud(struct eud_chip *chip)
>> +{
>> +	u32 reg;
>> +	int ret;
>> +
>> +	/* read sw_attach_det[0] to find attach/detach event */
>> +	reg = readl(chip->eud_reg_base +  EUD_REG_SW_ATTACH_DET);
>> +	if (reg & EUD_INT_PET_EUD) {
>> +		/* Detach & Attach pet for EUD */
>> +		writel(0, chip->eud_reg_base + EUD_REG_SW_ATTACH_DET);
>> +		/* Delay to make sure detach pet is done before attach pet */
>> +		ret = readl_poll_timeout(chip->eud_reg_base + 
>> EUD_REG_SW_ATTACH_DET,
>> +					reg, (reg == 0), 1, 100);
>> +		if (ret) {
>> +			dev_err(chip->dev, "Detach pet failed\n");
>> +			return;
>> +		}
>> +
>> +		writel(EUD_INT_PET_EUD, chip->eud_reg_base +
>> +				EUD_REG_SW_ATTACH_DET);
>> +	} else {
>> +		/* Attach pet for EUD */
>> +		writel(EUD_INT_PET_EUD, chip->eud_reg_base +
>> +				EUD_REG_SW_ATTACH_DET);
>> +	}
>> +}
>> +
>> +static irqreturn_t handle_eud_irq(int irq, void *data)
>> +{
>> +	struct eud_chip *chip = data;
>> +	u32 reg;
>> +
>> +	/* read status register and find out which interrupt triggered */
>> +	reg = readl(chip->eud_reg_base +  EUD_REG_INT_STATUS_1);
>> +	switch (reg & EUD_INT_ALL) {
>> +	case EUD_INT_VBUS:
>> +		chip->int_status = EUD_INT_VBUS;
>> +		usb_attach_detach(chip);
>> +		return IRQ_WAKE_THREAD;
>> +	case EUD_INT_SAFE_MODE:
>> +		pet_eud(chip);
>> +		break;
>> +	default:
>> +		return IRQ_NONE;
>> +	}
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static irqreturn_t handle_eud_irq_thread(int irq, void *data)
>> +{
>> +	struct eud_chip *chip = data;
>> +	int ret;
>> +
>> +	if (chip->int_status == EUD_INT_VBUS) {
>> +		if (chip->usb_attach)
>> +			ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_DEVICE);
>> +		else
>> +			ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_HOST);
>> +		if (ret)
>> +			dev_err(chip->dev, "failed to set role switch\n");
>> +	}
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int eud_probe(struct platform_device *pdev)
>> +{
>> +	struct eud_chip *chip;
>> +	int ret;
>> +
>> +	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
>> +	if (!chip)
>> +		return -ENOMEM;
>> +
>> +	chip->dev = &pdev->dev;
>> +
>> +	chip->role_sw = usb_role_switch_get(chip->dev);
>> +	if (IS_ERR(chip->role_sw)) {
>> +		if (PTR_ERR(chip->role_sw) != -EPROBE_DEFER)
>> +			dev_err(chip->dev, "failed to get role switch\n");
>> +
>> +		return PTR_ERR(chip->role_sw);
>> +	}
>> +
>> +	chip->eud_reg_base = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(chip->eud_reg_base))
>> +		return PTR_ERR(chip->eud_reg_base);
>> +
>> +	chip->eud_mode_mgr2_phys_base = devm_platform_ioremap_resource(pdev, 
>> 1);
>> +	if (IS_ERR(chip->eud_mode_mgr2_phys_base))
>> +		return PTR_ERR(chip->eud_mode_mgr2_phys_base);
>> +
>> +	chip->eud_irq = platform_get_irq(pdev, 0);
>> +	ret = devm_request_threaded_irq(&pdev->dev, chip->eud_irq, 
>> handle_eud_irq,
>> +			handle_eud_irq_thread, IRQF_ONESHOT, NULL, chip);
>> +	if (ret)
>> +		return ret;
>> +
>> +	device_init_wakeup(&pdev->dev, true);
>> +	enable_irq_wake(chip->eud_irq);
>> +
>> +	platform_set_drvdata(pdev, chip);
>> +
>> +	ret = device_create_file(&pdev->dev, eud_attrs[0]);
> 
> You raced with userspace and lost :(
> 
> Please properly attach the sysfs file to the platform driver to have 
> the
> driver core register this correctly.

Ok.

> 
>> +
>> +	return ret;
>> +}
>> +
>> +static int eud_remove(struct platform_device *pdev)
>> +{
>> +	struct eud_chip *chip = platform_get_drvdata(pdev);
>> +
>> +	if (chip->enable)
>> +		disable_eud(chip);
>> +
>> +	device_remove_file(&pdev->dev, eud_attrs[0]);
> 
> No need for this if you do the above correctly.

Ack.

> 
>> +	device_init_wakeup(&pdev->dev, false);
> 
> Why is this needed?

This is already handled in device_unregister() path so will remove this.

> 
> thanks,
> 
> greg k-h

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 2/7] dt-bindings: usb: dwc3: Update dwc3 properties for EUD connector
  2021-10-04 15:50   ` Bjorn Andersson
@ 2021-10-05 12:57     ` schowdhu
  0 siblings, 0 replies; 26+ messages in thread
From: schowdhu @ 2021-10-05 12:57 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Greg KH, linux-kernel, ckadabi, tsoni, bryanh, psodagud, satyap,
	pheragu, Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan

On 2021-10-04 21:20, Bjorn Andersson wrote:
> On Mon 04 Oct 04:16 PDT 2021, Souradeep Chowdhury wrote:
> 
>> Adding the address size,cell size and ranges property for EUD 
>> connector.
>> Adding the connector property for EUD which is child of dwc3 node.
>> 
> 
> When we have a Type-C controller involved, the connector is described
> using of_graph, so will we not then have two different connectors
> described, in two different ways?
> 
> Regards,
> Bjorn

Ack. Will have this updated with the of_graph in the next version.

> 
>> Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
>> ---
>>  Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 15 
>> +++++++++++++++
>>  1 file changed, 15 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml 
>> b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> index 078fb78..3e71205 100644
>> --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
>> @@ -36,6 +36,14 @@ properties:
>>          - const: synopsys,dwc3
>>            deprecated: true
>> 
>> +  "#address-cells":
>> +    enum: [ 1, 2 ]
>> +
>> +  "#size-cells":
>> +    enum: [ 1, 2 ]
>> +
>> +  ranges: true
>> +
>>    interrupts:
>>      description:
>>        It's either a single common DWC3 interrupt (dwc_usb3) or 
>> individual
>> @@ -318,6 +326,13 @@ properties:
>>      items:
>>        enum: [1, 4, 8, 16, 32, 64, 128, 256]
>> 
>> +  connector:
>> +    type: object
>> +    $ref: /connector/usb-connector.yaml#
>> +    description:
>> +      Connector for dual role switch, especially for 
>> "eud-usb-c-connector"
>> +
>> +
>>  unevaluatedProperties: false
>> 
>>  required:
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
>> member
>> of Code Aurora Forum, hosted by The Linux Foundation
>> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 4/7] usb: common: eud: Added the driver support for Embedded USB Debugger(EUD)
  2021-10-04 16:42   ` Bjorn Andersson
  2021-10-05  7:14     ` Joe Perches
@ 2021-10-05 13:01     ` schowdhu
  1 sibling, 0 replies; 26+ messages in thread
From: schowdhu @ 2021-10-05 13:01 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Greg KH, linux-kernel, ckadabi, tsoni, bryanh, psodagud, satyap,
	pheragu, Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan

On 2021-10-04 22:12, Bjorn Andersson wrote:
> On Mon 04 Oct 04:16 PDT 2021, Souradeep Chowdhury wrote:
> 
>> Add support for control peripheral of EUD (Embedded USB Debugger) to
>> listen to events such as USB attach/detach, pet EUD to indicate 
>> software
>> is functional.Reusing the platform device kobj, sysfs entry 'enable' 
>> is
>> created to enable or disable EUD.
>> 
>> To enable the eud the following needs to be done
>> echo 1 > /sys/bus/platform/.../enable
>> 
>> To disable eud, following is the command
>> echo 0 > /sys/bus/platform/.../enable
>> 
>> Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
>> ---
>>  Documentation/ABI/testing/sysfs-driver-eud |   7 +
>>  drivers/usb/common/Kconfig                 |   9 +
>>  drivers/usb/common/Makefile                |   1 +
>>  drivers/usb/common/qcom_eud.c              | 256 
>> +++++++++++++++++++++++++++++
>>  4 files changed, 273 insertions(+)
>>  create mode 100644 Documentation/ABI/testing/sysfs-driver-eud
>>  create mode 100644 drivers/usb/common/qcom_eud.c
>> 
>> diff --git a/Documentation/ABI/testing/sysfs-driver-eud 
>> b/Documentation/ABI/testing/sysfs-driver-eud
>> new file mode 100644
>> index 0000000..14a02da
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-driver-eud
>> @@ -0,0 +1,7 @@
>> +What:		/sys/bus/platform/.../enable
>> +Date:           October 2021
>> +Contact:        Souradeep Chowdhury <schowdhu@codeaurora.org>
>> +Description:
>> +		The Enable/Disable sysfs interface for Embedded
>> +		USB Debugger(EUD).This enables and disables the
>> +		EUD based on a 1 or a 0 value.
>> diff --git a/drivers/usb/common/Kconfig b/drivers/usb/common/Kconfig
>> index 5e8a04e..669b3fe 100644
>> --- a/drivers/usb/common/Kconfig
>> +++ b/drivers/usb/common/Kconfig
>> @@ -50,3 +50,12 @@ config USB_CONN_GPIO
>> 
>>  	  To compile the driver as a module, choose M here: the module will
>>  	  be called usb-conn-gpio.ko
>> +
>> +config USB_QCOM_EUD
>> +	tristate "USB EUD Driver"
>> +	select USB_ROLE_SWITCH
>> +	help
>> +	  This module enables support for Qualcomm Technologies, Inc.
>> +	  Embedded USB Debugger (EUD). The EUD is a control peripheral
>> +	  which reports VBUS attach/detach events and has USB-based
>> +	  debug and trace capabilities.
>> diff --git a/drivers/usb/common/Makefile b/drivers/usb/common/Makefile
>> index 8ac4d21..eb66045 100644
>> --- a/drivers/usb/common/Makefile
>> +++ b/drivers/usb/common/Makefile
>> @@ -11,3 +11,4 @@ usb-common-$(CONFIG_USB_LED_TRIG) += led.o
>>  obj-$(CONFIG_USB_CONN_GPIO)	+= usb-conn-gpio.o
>>  obj-$(CONFIG_USB_OTG_FSM) += usb-otg-fsm.o
>>  obj-$(CONFIG_USB_ULPI_BUS)	+= ulpi.o
>> +obj-$(CONFIG_USB_QCOM_EUD)      += qcom_eud.o
>> diff --git a/drivers/usb/common/qcom_eud.c 
>> b/drivers/usb/common/qcom_eud.c
>> new file mode 100644
>> index 0000000..7a92fff
>> --- /dev/null
>> +++ b/drivers/usb/common/qcom_eud.c
>> @@ -0,0 +1,256 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2015-2021, The Linux Foundation. All rights 
>> reserved.
>> + */
>> +
>> +#include <linux/bitops.h>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <linux/sysfs.h>
>> +#include <linux/usb/role.h>
>> +
>> +#define EUD_REG_INT1_EN_MASK	0x0024
>> +#define EUD_REG_INT_STATUS_1	0x0044
>> +#define EUD_REG_CTL_OUT_1	0x0074
>> +#define EUD_REG_VBUS_INT_CLR	0x0080
>> +#define EUD_REG_CSR_EUD_EN	0x1014
>> +#define EUD_REG_SW_ATTACH_DET	0x1018
>> +#define EUD_REG_EUD_EN2         0x0000
>> +
>> +#define EUD_ENABLE		BIT(0)
>> +#define EUD_INT_PET_EUD		BIT(0)
>> +#define EUD_INT_VBUS		BIT(2)
>> +#define EUD_INT_SAFE_MODE	BIT(4)
>> +#define EUD_INT_ALL		(EUD_INT_VBUS|EUD_INT_SAFE_MODE)
>> +
>> +struct eud_chip {
>> +	struct device			*dev;
>> +	struct usb_role_switch		*role_sw;
>> +	void __iomem			*eud_reg_base;
>> +	void __iomem			*eud_mode_mgr2_phys_base;
>> +	unsigned int			int_status;
>> +	int				enable;
>> +	int				eud_irq;
>> +	bool				usb_attach;
>> +
>> +};
>> +
>> +static int enable_eud(struct eud_chip *priv)
>> +{
>> +	writel(EUD_ENABLE, priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
>> +	writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE,
>> +			priv->eud_reg_base + EUD_REG_INT1_EN_MASK);
>> +	writel(1, priv->eud_mode_mgr2_phys_base + EUD_REG_EUD_EN2);
>> +
>> +	return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE);
>> +}
>> +
>> +static void disable_eud(struct eud_chip *priv)
>> +{
>> +	writel(0, priv->eud_reg_base + EUD_REG_CSR_EUD_EN);
>> +	writel(0, priv->eud_mode_mgr2_phys_base + EUD_REG_EUD_EN2);
>> +}
>> +
>> +static ssize_t enable_show(struct device *dev,
>> +		struct device_attribute *attr, char *buf)
>> +{
>> +	struct eud_chip *chip = dev_get_drvdata(dev);
>> +
>> +	return sprintf(buf, "%d", chip->enable);
>> +}
>> +
>> +static ssize_t enable_store(struct device *dev,
>> +		struct device_attribute *attr,
>> +		const char *buf, size_t count)
>> +{
>> +	struct eud_chip *chip = dev_get_drvdata(dev);
>> +	unsigned long enable;
>> +	int ret;
>> +
>> +	if (kstrtoul(buf, 16, &enable))
>> +		return -EINVAL;
>> +
>> +	if (enable == 1) {
>> +		ret = enable_eud(chip);
>> +		if (!ret)
>> +			chip->enable = enable;
>> +	} else if (enable == 0) {
>> +		disable_eud(chip);
>> +	} else {
>> +		return -EINVAL;
>> +	}
>> +
>> +	return count;
>> +}
>> +
>> +static DEVICE_ATTR_RW(enable);
>> +
>> +static const struct device_attribute *eud_attrs[] = {
>> +	&dev_attr_enable,
>> +	NULL,
>> +};
>> +
>> +static void usb_attach_detach(struct eud_chip *chip)
>> +{
>> +	u32 reg;
>> +
>> +	/* read ctl_out_1[4] to find USB attach or detach event */
>> +	reg = readl(chip->eud_reg_base + EUD_REG_CTL_OUT_1);
>> +	if (reg & EUD_INT_SAFE_MODE)
>> +		chip->usb_attach = true;
>> +	else
>> +		chip->usb_attach = false;
>> +
>> +	/* set and clear vbus_int_clr[0] to clear interrupt */
>> +	writel(BIT(0), chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
>> +	writel(0, chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
>> +}
>> +
>> +static void pet_eud(struct eud_chip *chip)
>> +{
>> +	u32 reg;
>> +	int ret;
>> +
>> +	/* read sw_attach_det[0] to find attach/detach event */
>> +	reg = readl(chip->eud_reg_base +  EUD_REG_SW_ATTACH_DET);
>> +	if (reg & EUD_INT_PET_EUD) {
>> +		/* Detach & Attach pet for EUD */
>> +		writel(0, chip->eud_reg_base + EUD_REG_SW_ATTACH_DET);
>> +		/* Delay to make sure detach pet is done before attach pet */
>> +		ret = readl_poll_timeout(chip->eud_reg_base + 
>> EUD_REG_SW_ATTACH_DET,
>> +					reg, (reg == 0), 1, 100);
>> +		if (ret) {
>> +			dev_err(chip->dev, "Detach pet failed\n");
>> +			return;
>> +		}
>> +
>> +		writel(EUD_INT_PET_EUD, chip->eud_reg_base +
>> +				EUD_REG_SW_ATTACH_DET);
>> +	} else {
>> +		/* Attach pet for EUD */
>> +		writel(EUD_INT_PET_EUD, chip->eud_reg_base +
>> +				EUD_REG_SW_ATTACH_DET);
>> +	}
>> +}
>> +
>> +static irqreturn_t handle_eud_irq(int irq, void *data)
>> +{
>> +	struct eud_chip *chip = data;
>> +	u32 reg;
>> +
>> +	/* read status register and find out which interrupt triggered */
>> +	reg = readl(chip->eud_reg_base +  EUD_REG_INT_STATUS_1);
>> +	switch (reg & EUD_INT_ALL) {
>> +	case EUD_INT_VBUS:
>> +		chip->int_status = EUD_INT_VBUS;
>> +		usb_attach_detach(chip);
>> +		return IRQ_WAKE_THREAD;
>> +	case EUD_INT_SAFE_MODE:
>> +		pet_eud(chip);
>> +		break;
>> +	default:
>> +		return IRQ_NONE;
>> +	}
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static irqreturn_t handle_eud_irq_thread(int irq, void *data)
>> +{
>> +	struct eud_chip *chip = data;
>> +	int ret;
>> +
>> +	if (chip->int_status == EUD_INT_VBUS) {
>> +		if (chip->usb_attach)
>> +			ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_DEVICE);
>> +		else
>> +			ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_HOST);
> 
> How does this work if I have a Type-C controller wired up to the dwc3
> and it has negotiated that we're supposed to be in device mode?
> 
> Shouldn't this driver somehow work as an override only when EUD is
> enabled, but otherwise let the Type-C controller deal with things?
> 
> Or is this interrupt simply not delivered when EUD is disabled, so that
> it happens to work as expected?

That is correct. When EUD is disabled, the interrupt lines are also 
disabled.

> 
>> +		if (ret)
>> +			dev_err(chip->dev, "failed to set role switch\n");
>> +	}
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int eud_probe(struct platform_device *pdev)
>> +{
>> +	struct eud_chip *chip;
>> +	int ret;
>> +
>> +	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
>> +	if (!chip)
>> +		return -ENOMEM;
>> +
>> +	chip->dev = &pdev->dev;
>> +
>> +	chip->role_sw = usb_role_switch_get(chip->dev);
>> +	if (IS_ERR(chip->role_sw)) {
>> +		if (PTR_ERR(chip->role_sw) != -EPROBE_DEFER)
>> +			dev_err(chip->dev, "failed to get role switch\n");
>> +
>> +		return PTR_ERR(chip->role_sw);
> 
> Please use
> 		dev_err_probe(chip->dev, PTR_ERR(chip->role_sw), "failed to...\n");

Ack

> 
>> +	}
>> +
>> +	chip->eud_reg_base = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(chip->eud_reg_base))
>> +		return PTR_ERR(chip->eud_reg_base);
>> +
>> +	chip->eud_mode_mgr2_phys_base = devm_platform_ioremap_resource(pdev, 
>> 1);
>> +	if (IS_ERR(chip->eud_mode_mgr2_phys_base))
>> +		return PTR_ERR(chip->eud_mode_mgr2_phys_base);
>> +
>> +	chip->eud_irq = platform_get_irq(pdev, 0);
>> +	ret = devm_request_threaded_irq(&pdev->dev, chip->eud_irq, 
>> handle_eud_irq,
>> +			handle_eud_irq_thread, IRQF_ONESHOT, NULL, chip);
>> +	if (ret)
> 
> This deserved another dev_err_probe()

Ack

> 
> Thanks,
> Bjorn
> 
>> +		return ret;
>> +
>> +	device_init_wakeup(&pdev->dev, true);
>> +	enable_irq_wake(chip->eud_irq);
>> +
>> +	platform_set_drvdata(pdev, chip);
>> +
>> +	ret = device_create_file(&pdev->dev, eud_attrs[0]);
>> +
>> +	return ret;
>> +}
>> +
>> +static int eud_remove(struct platform_device *pdev)
>> +{
>> +	struct eud_chip *chip = platform_get_drvdata(pdev);
>> +
>> +	if (chip->enable)
>> +		disable_eud(chip);
>> +
>> +	device_remove_file(&pdev->dev, eud_attrs[0]);
>> +	device_init_wakeup(&pdev->dev, false);
>> +	disable_irq_wake(chip->eud_irq);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id eud_dt_match[] = {
>> +	{ .compatible = "qcom,usb-connector-eud" },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, eud_dt_match);
>> +
>> +static struct platform_driver eud_driver = {
>> +	.probe		= eud_probe,
>> +	.remove		= eud_remove,
>> +	.driver		= {
>> +		.name		= "eud",
>> +		.of_match_table = eud_dt_match,
>> +	},
>> +};
>> +module_platform_driver(eud_driver);
>> +
>> +MODULE_DESCRIPTION("QTI EUD driver");
>> +MODULE_LICENSE("GPL v2");
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
>> member
>> of Code Aurora Forum, hosted by The Linux Foundation
>> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 1/7] dt-bindings: connector: Add property for eud type c connector
  2021-10-04 16:37   ` Rob Herring
@ 2021-10-05 13:11     ` schowdhu
  2021-10-05 16:37       ` Bjorn Andersson
  0 siblings, 1 reply; 26+ messages in thread
From: schowdhu @ 2021-10-05 13:11 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Bjorn Andersson, Greg KH, linux-kernel, ckadabi, tsoni, bryanh,
	psodagud, satyap, pheragu, Rajendra Nayak, Sibi Sankar,
	Sai Prakash Ranjan

On 2021-10-04 22:07, Rob Herring wrote:
> On Mon, Oct 04, 2021 at 04:46:19PM +0530, Souradeep Chowdhury wrote:
>> Added the property for EUD(Embedded USB Debug) connector.Added
>> the "reg" and "interrupts" property which is needed for EUD.
> 
> You are going to need a better explanation of this h/w.

Ack. Will update this with the detailed hardware description
in the next version.

> 
>> 
>> Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
>> ---
>>  .../devicetree/bindings/connector/usb-connector.yaml      | 15 
>> +++++++++++++++
>>  1 file changed, 15 insertions(+)
>> 
>> diff --git 
>> a/Documentation/devicetree/bindings/connector/usb-connector.yaml 
>> b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> index 7eb8659..908129f 100644
>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> @@ -30,6 +30,21 @@ properties:
>>            - const: samsung,usb-connector-11pin
>>            - const: usb-b-connector
>> 
>> +      - items:
>> +          - enum:
>> +              - qcom,sc7280-usb-connector-eud
>> +          - const: qcom,usb-connector-eud
>> +          - const: usb-c-connector
>> +
>> +  reg:
>> +    items:
>> +      - description: EUD Base Register Region
>> +      - description: EUD Mode Manager Region
> 
> A connector node represents the physical connector on a board. That
> can't really be an MMIO peripheral. Maybe you need a node for EUD and
> then it should have a connector child node? Don't really know without
> understanding this h/w.

As per the previous discussion on the EUD, it was agreed upon to map EUD
as a type C connector and use Role-Switch to change the USB role instead
of extcon interface that was being used previously. The link for the 
same
is as follows:-

https://lore.kernel.org/lkml/5db1a666-62ec-c850-6626-ad33d337b452@codeaurora.org/

> 
>> +
>> +  interrupts:
>> +    description:
>> +      EUD interrupt
>> +
>>    label:
>>      description: Symbolic name for the connector.
>> 
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
>> member
>> of Code Aurora Forum, hosted by The Linux Foundation
>> 
>> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 4/7] usb: common: eud: Added the driver support for Embedded USB Debugger(EUD)
  2021-10-04 20:08   ` Randy Dunlap
@ 2021-10-05 13:57     ` schowdhu
  0 siblings, 0 replies; 26+ messages in thread
From: schowdhu @ 2021-10-05 13:57 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Bjorn Andersson, Greg KH, linux-kernel, ckadabi, tsoni, bryanh,
	psodagud, satyap, pheragu, Rajendra Nayak, Sibi Sankar,
	Sai Prakash Ranjan

On 2021-10-05 01:38, Randy Dunlap wrote:
> On 10/4/21 4:16 AM, Souradeep Chowdhury wrote:
>> Add support for control peripheral of EUD (Embedded USB Debugger) to
>> listen to events such as USB attach/detach, pet EUD to indicate 
>> software
> 
>   I don't quite understand: what pets the EUD? how does it do that?

Pet EUD is an interrupt that the EUD driver receives periodically to
indicate if the software is functional. On getting the interrupt the
EUD driver first does a detach pet and then followed by the an attach
pet.

> 
>> is functional.Reusing the platform device kobj, sysfs entry 'enable' 
>> is
> 
>      functional. Reusing

Ack

> 
>> created to enable or disable EUD.
>> 
>> To enable the eud the following needs to be done
>> echo 1 >/sys/bus/platform/.../enable
>> 
>> To disable eud, following is the command
>> echo 0 >/sys/bus/platform/.../enable
>> 
>> Signed-off-by: Souradeep Chowdhury<schowdhu@codeaurora.org>
>> ---
>>   Documentation/ABI/testing/sysfs-driver-eud |   7 +
>>   drivers/usb/common/Kconfig                 |   9 +
>>   drivers/usb/common/Makefile                |   1 +
>>   drivers/usb/common/qcom_eud.c              | 256 
>> +++++++++++++++++++++++++++++
>>   4 files changed, 273 insertions(+)
>>   create mode 100644 Documentation/ABI/testing/sysfs-driver-eud
>>   create mode 100644 drivers/usb/common/qcom_eud.c
>> 
>> diff --git a/Documentation/ABI/testing/sysfs-driver-eud 
>> b/Documentation/ABI/testing/sysfs-driver-eud
>> new file mode 100644
>> index 0000000..14a02da
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-driver-eud
>> @@ -0,0 +1,7 @@
>> +What:		/sys/bus/platform/.../enable
>> +Date:           October 2021
>> +Contact:        Souradeep Chowdhury<schowdhu@codeaurora.org>
>> +Description:
>> +		The Enable/Disable sysfs interface for Embedded
>> +		USB Debugger(EUD).This enables and disables the
> 
> 		    Debugger (EUD). This enables

Ack

> 
>> +		EUD based on a 1 or a 0 value.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 4/7] usb: common: eud: Added the driver support for Embedded USB Debugger(EUD)
  2021-10-05  7:14     ` Joe Perches
@ 2021-10-05 13:58       ` schowdhu
  0 siblings, 0 replies; 26+ messages in thread
From: schowdhu @ 2021-10-05 13:58 UTC (permalink / raw)
  To: Joe Perches
  Cc: Bjorn Andersson, linux-arm-msm, linux-usb, devicetree,
	Bryan O'Donoghue, Greg KH, linux-kernel, ckadabi, tsoni,
	bryanh, psodagud, satyap, pheragu, Rajendra Nayak, Sibi Sankar,
	Sai Prakash Ranjan

On 2021-10-05 12:44, Joe Perches wrote:
> On Mon, 2021-10-04 at 09:42 -0700, Bjorn Andersson wrote:
>> On Mon 04 Oct 04:16 PDT 2021, Souradeep Chowdhury wrote:
>> 
>> > Add support for control peripheral of EUD (Embedded USB Debugger) to
>> > listen to events such as USB attach/detach, pet EUD to indicate software
>> > is functional.Reusing the platform device kobj, sysfs entry 'enable' is
>> > created to enable or disable EUD.
> []
>> > diff --git a/drivers/usb/common/qcom_eud.c b/drivers/usb/common/qcom_eud.c
> []
>> > +static ssize_t enable_show(struct device *dev,
>> > +		struct device_attribute *attr, char *buf)
>> > +{
>> > +	struct eud_chip *chip = dev_get_drvdata(dev);
>> > +
>> > +	return sprintf(buf, "%d", chip->enable);
> 
> trivia:
> 
> should probably use sysfs_emit and have a trailing '\n' in the format.

Ack

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 1/7] dt-bindings: connector: Add property for eud type c connector
  2021-10-05 13:11     ` schowdhu
@ 2021-10-05 16:37       ` Bjorn Andersson
  2021-10-07  9:25         ` schowdhu
  0 siblings, 1 reply; 26+ messages in thread
From: Bjorn Andersson @ 2021-10-05 16:37 UTC (permalink / raw)
  To: schowdhu
  Cc: Rob Herring, linux-arm-msm, linux-usb, devicetree,
	Bryan O'Donoghue, Greg KH, linux-kernel, ckadabi, tsoni,
	bryanh, psodagud, satyap, pheragu, Rajendra Nayak, Sibi Sankar,
	Sai Prakash Ranjan

On Tue 05 Oct 06:11 PDT 2021, schowdhu@codeaurora.org wrote:

> On 2021-10-04 22:07, Rob Herring wrote:
> > On Mon, Oct 04, 2021 at 04:46:19PM +0530, Souradeep Chowdhury wrote:
> > > Added the property for EUD(Embedded USB Debug) connector.Added
> > > the "reg" and "interrupts" property which is needed for EUD.
> > 
> > You are going to need a better explanation of this h/w.
> 
> Ack. Will update this with the detailed hardware description
> in the next version.
> 
> > 
> > > 
> > > Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
> > > ---
> > >  .../devicetree/bindings/connector/usb-connector.yaml      | 15
> > > +++++++++++++++
> > >  1 file changed, 15 insertions(+)
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > index 7eb8659..908129f 100644
> > > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > @@ -30,6 +30,21 @@ properties:
> > >            - const: samsung,usb-connector-11pin
> > >            - const: usb-b-connector
> > > 
> > > +      - items:
> > > +          - enum:
> > > +              - qcom,sc7280-usb-connector-eud
> > > +          - const: qcom,usb-connector-eud
> > > +          - const: usb-c-connector
> > > +
> > > +  reg:
> > > +    items:
> > > +      - description: EUD Base Register Region
> > > +      - description: EUD Mode Manager Region
> > 
> > A connector node represents the physical connector on a board. That
> > can't really be an MMIO peripheral. Maybe you need a node for EUD and
> > then it should have a connector child node? Don't really know without
> > understanding this h/w.
> 
> As per the previous discussion on the EUD, it was agreed upon to map EUD
> as a type C connector and use Role-Switch to change the USB role instead
> of extcon interface that was being used previously. The link for the same
> is as follows:-
> 
> https://lore.kernel.org/lkml/5db1a666-62ec-c850-6626-ad33d337b452@codeaurora.org/
> 

Not using extcon is the right thing, but perhaps we should make the EUD
a role_switch provider and client, so that we can describe how it sits
inbetween the connector and the controller.

That way it has the power to pass through or override requests from the
upstream role-switcher, based on the status of EUD.


That said, I'm still curious to what happens if I renegotiate the roles
dynamically in a Type-C environment, while enabling EUD. How would the
device on the other end of the cable know that it's supposed to be a
host? Or there's simply a reset of the link when this happens?

Thanks,
Bjorn

> > 
> > > +
> > > +  interrupts:
> > > +    description:
> > > +      EUD interrupt
> > > +
> > >    label:
> > >      description: Symbolic name for the connector.
> > > 
> > > --
> > > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
> > > member
> > > of Code Aurora Forum, hosted by The Linux Foundation
> > > 
> > > 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 1/7] dt-bindings: connector: Add property for eud type c connector
  2021-10-05 16:37       ` Bjorn Andersson
@ 2021-10-07  9:25         ` schowdhu
  2021-10-12  3:22           ` Bjorn Andersson
  0 siblings, 1 reply; 26+ messages in thread
From: schowdhu @ 2021-10-07  9:25 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Rob Herring, linux-arm-msm, linux-usb, devicetree,
	Bryan O'Donoghue, Greg KH, linux-kernel, ckadabi, tsoni,
	bryanh, psodagud, satyap, pheragu, Rajendra Nayak, Sibi Sankar,
	Sai Prakash Ranjan

On 2021-10-05 22:07, Bjorn Andersson wrote:
> On Tue 05 Oct 06:11 PDT 2021, schowdhu@codeaurora.org wrote:
> 
>> On 2021-10-04 22:07, Rob Herring wrote:
>> > On Mon, Oct 04, 2021 at 04:46:19PM +0530, Souradeep Chowdhury wrote:
>> > > Added the property for EUD(Embedded USB Debug) connector.Added
>> > > the "reg" and "interrupts" property which is needed for EUD.
>> >
>> > You are going to need a better explanation of this h/w.
>> 
>> Ack. Will update this with the detailed hardware description
>> in the next version.
>> 
>> >
>> > >
>> > > Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
>> > > ---
>> > >  .../devicetree/bindings/connector/usb-connector.yaml      | 15
>> > > +++++++++++++++
>> > >  1 file changed, 15 insertions(+)
>> > >
>> > > diff --git
>> > > a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> > > b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> > > index 7eb8659..908129f 100644
>> > > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> > > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> > > @@ -30,6 +30,21 @@ properties:
>> > >            - const: samsung,usb-connector-11pin
>> > >            - const: usb-b-connector
>> > >
>> > > +      - items:
>> > > +          - enum:
>> > > +              - qcom,sc7280-usb-connector-eud
>> > > +          - const: qcom,usb-connector-eud
>> > > +          - const: usb-c-connector
>> > > +
>> > > +  reg:
>> > > +    items:
>> > > +      - description: EUD Base Register Region
>> > > +      - description: EUD Mode Manager Region
>> >
>> > A connector node represents the physical connector on a board. That
>> > can't really be an MMIO peripheral. Maybe you need a node for EUD and
>> > then it should have a connector child node? Don't really know without
>> > understanding this h/w.
>> 
>> As per the previous discussion on the EUD, it was agreed upon to map 
>> EUD
>> as a type C connector and use Role-Switch to change the USB role 
>> instead
>> of extcon interface that was being used previously. The link for the 
>> same
>> is as follows:-
>> 
>> https://lore.kernel.org/lkml/5db1a666-62ec-c850-6626-ad33d337b452@codeaurora.org/
>> 
> 
> Not using extcon is the right thing, but perhaps we should make the EUD
> a role_switch provider and client, so that we can describe how it sits
> inbetween the connector and the controller.
> 
> That way it has the power to pass through or override requests from the
> upstream role-switcher, based on the status of EUD.
> 
> 
> That said, I'm still curious to what happens if I renegotiate the roles
> dynamically in a Type-C environment, while enabling EUD. How would the
> device on the other end of the cable know that it's supposed to be a
> host? Or there's simply a reset of the link when this happens?
> 
> Thanks,
> Bjorn

Hi Bjorn,

By making EUD Role-Switch provider and client do you mean that
we should have a EUD node which will have a connector node as
child and this connector node will have a port that points towards
the drd role-switch?

So that my device tree node will look like the following in that case

eud@88e0000 {
         compatible = "qcom,usb-connector-eud";
         reg = <0 0x88e0000 0 0x2000>,
               <0 0x88e2000 0 0x1000>;
         interrupt-parent = <&pdc>;
         interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
         usb_con: connector {
                 compatible = "usb-c-connector";
                 label = "USB-C";
                 port {
                       eud_usb_output: endpoint {
                       remote-endpoint = <&eud_usb3_input>;
                  };
         };

};


@usb2 {
     dwc3 {
        usb-role-switch;
        port {
              eud_usb3_input: endpoint {
                    remote-endpoint = <&eud_usb_output>;
              };
      };
};

Also EUD functions only in device mode, so when the role-switch is done 
by the controller
to set the device mode, the PC on the other end becomes the host.

Thanks,
Souradeep

> 
>> >
>> > > +
>> > > +  interrupts:
>> > > +    description:
>> > > +      EUD interrupt
>> > > +
>> > >    label:
>> > >      description: Symbolic name for the connector.
>> > >
>> > > --
>> > > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
>> > > member
>> > > of Code Aurora Forum, hosted by The Linux Foundation
>> > >
>> > >

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 1/7] dt-bindings: connector: Add property for eud type c connector
  2021-10-07  9:25         ` schowdhu
@ 2021-10-12  3:22           ` Bjorn Andersson
  2021-11-02  9:20             ` schowdhu
  0 siblings, 1 reply; 26+ messages in thread
From: Bjorn Andersson @ 2021-10-12  3:22 UTC (permalink / raw)
  To: schowdhu
  Cc: Rob Herring, linux-arm-msm, linux-usb, devicetree,
	Bryan O'Donoghue, Greg KH, linux-kernel, ckadabi, tsoni,
	bryanh, psodagud, satyap, pheragu, Rajendra Nayak, Sibi Sankar,
	Sai Prakash Ranjan

On Thu 07 Oct 04:25 CDT 2021, schowdhu@codeaurora.org wrote:

> On 2021-10-05 22:07, Bjorn Andersson wrote:
> > On Tue 05 Oct 06:11 PDT 2021, schowdhu@codeaurora.org wrote:
> > 
> > > On 2021-10-04 22:07, Rob Herring wrote:
> > > > On Mon, Oct 04, 2021 at 04:46:19PM +0530, Souradeep Chowdhury wrote:
> > > > > Added the property for EUD(Embedded USB Debug) connector.Added
> > > > > the "reg" and "interrupts" property which is needed for EUD.
> > > >
> > > > You are going to need a better explanation of this h/w.
> > > 
> > > Ack. Will update this with the detailed hardware description
> > > in the next version.
> > > 
> > > >
> > > > >
> > > > > Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
> > > > > ---
> > > > >  .../devicetree/bindings/connector/usb-connector.yaml      | 15
> > > > > +++++++++++++++
> > > > >  1 file changed, 15 insertions(+)
> > > > >
> > > > > diff --git
> > > > > a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > > > b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > > > index 7eb8659..908129f 100644
> > > > > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > > > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > > > @@ -30,6 +30,21 @@ properties:
> > > > >            - const: samsung,usb-connector-11pin
> > > > >            - const: usb-b-connector
> > > > >
> > > > > +      - items:
> > > > > +          - enum:
> > > > > +              - qcom,sc7280-usb-connector-eud
> > > > > +          - const: qcom,usb-connector-eud
> > > > > +          - const: usb-c-connector
> > > > > +
> > > > > +  reg:
> > > > > +    items:
> > > > > +      - description: EUD Base Register Region
> > > > > +      - description: EUD Mode Manager Region
> > > >
> > > > A connector node represents the physical connector on a board. That
> > > > can't really be an MMIO peripheral. Maybe you need a node for EUD and
> > > > then it should have a connector child node? Don't really know without
> > > > understanding this h/w.
> > > 
> > > As per the previous discussion on the EUD, it was agreed upon to map
> > > EUD
> > > as a type C connector and use Role-Switch to change the USB role
> > > instead
> > > of extcon interface that was being used previously. The link for the
> > > same
> > > is as follows:-
> > > 
> > > https://lore.kernel.org/lkml/5db1a666-62ec-c850-6626-ad33d337b452@codeaurora.org/
> > > 
> > 
> > Not using extcon is the right thing, but perhaps we should make the EUD
> > a role_switch provider and client, so that we can describe how it sits
> > inbetween the connector and the controller.
> > 
> > That way it has the power to pass through or override requests from the
> > upstream role-switcher, based on the status of EUD.
> > 
> > 
> > That said, I'm still curious to what happens if I renegotiate the roles
> > dynamically in a Type-C environment, while enabling EUD. How would the
> > device on the other end of the cable know that it's supposed to be a
> > host? Or there's simply a reset of the link when this happens?
> > 
> > Thanks,
> > Bjorn
> 
> Hi Bjorn,
> 

Hi Souradeep

> By making EUD Role-Switch provider and client do you mean that
> we should have a EUD node which will have a connector node as
> child and this connector node will have a port that points towards
> the drd role-switch?
> 
> So that my device tree node will look like the following in that case
> 
> eud@88e0000 {
>         compatible = "qcom,usb-connector-eud";
>         reg = <0 0x88e0000 0 0x2000>,
>               <0 0x88e2000 0 0x1000>;
>         interrupt-parent = <&pdc>;
>         interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
>         usb_con: connector {
>                 compatible = "usb-c-connector";
>                 label = "USB-C";
>                 port {
>                       eud_usb_output: endpoint {
>                       remote-endpoint = <&eud_usb3_input>;
>                  };
>         };
> 
> };
> 
> 
> @usb2 {
>     dwc3 {
>        usb-role-switch;
>        port {
>              eud_usb3_input: endpoint {
>                    remote-endpoint = <&eud_usb_output>;
>              };
>      };
> };

While your "output" and "input" matches the direction of the role
switching, I think the connection should be describe in the other
direction.

Also my suggestion was that EUD is both connector for the dwc3 and has a
reference to the connector described in the TypeC controller - to
properly describe the relationship:

  DWC -> EUD -> connector

With the role switching request going from the connector (pmic_glink
driver) to DWC through the EUD, which can override the vote.


That said, this is just my suggestion. You need to ensure that Rob
understands the hardware design well enough to approve your proposed
binding.


E.g. The connector in the EUD isn't a usb-c-connector, it's some
type of internal connection, the next step in that chain is the actual
usb-c-connector.

Regards,
Bjorn

> 
> Also EUD functions only in device mode, so when the role-switch is done by
> the controller
> to set the device mode, the PC on the other end becomes the host.
> 
> Thanks,
> Souradeep
> 
> > 
> > > >
> > > > > +
> > > > > +  interrupts:
> > > > > +    description:
> > > > > +      EUD interrupt
> > > > > +
> > > > >    label:
> > > > >      description: Symbolic name for the connector.
> > > > >
> > > > > --
> > > > > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
> > > > > member
> > > > > of Code Aurora Forum, hosted by The Linux Foundation
> > > > >
> > > > >

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH V0 1/7] dt-bindings: connector: Add property for eud type c connector
  2021-10-12  3:22           ` Bjorn Andersson
@ 2021-11-02  9:20             ` schowdhu
  0 siblings, 0 replies; 26+ messages in thread
From: schowdhu @ 2021-11-02  9:20 UTC (permalink / raw)
  To: Bjorn Andersson, Rob Herring
  Cc: linux-arm-msm, linux-usb, devicetree, Bryan O'Donoghue,
	Greg KH, linux-kernel, ckadabi, tsoni, bryanh, psodagud, satyap,
	pheragu, Rajendra Nayak, Sibi Sankar, Sai Prakash Ranjan

On 2021-10-12 08:52, Bjorn Andersson wrote:
> On Thu 07 Oct 04:25 CDT 2021, schowdhu@codeaurora.org wrote:
> 
>> On 2021-10-05 22:07, Bjorn Andersson wrote:
>> > On Tue 05 Oct 06:11 PDT 2021, schowdhu@codeaurora.org wrote:
>> >
>> > > On 2021-10-04 22:07, Rob Herring wrote:
>> > > > On Mon, Oct 04, 2021 at 04:46:19PM +0530, Souradeep Chowdhury wrote:
>> > > > > Added the property for EUD(Embedded USB Debug) connector.Added
>> > > > > the "reg" and "interrupts" property which is needed for EUD.
>> > > >
>> > > > You are going to need a better explanation of this h/w.
>> > >
>> > > Ack. Will update this with the detailed hardware description
>> > > in the next version.
>> > >
>> > > >
>> > > > >
>> > > > > Signed-off-by: Souradeep Chowdhury <schowdhu@codeaurora.org>
>> > > > > ---
>> > > > >  .../devicetree/bindings/connector/usb-connector.yaml      | 15
>> > > > > +++++++++++++++
>> > > > >  1 file changed, 15 insertions(+)
>> > > > >
>> > > > > diff --git
>> > > > > a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> > > > > b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> > > > > index 7eb8659..908129f 100644
>> > > > > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> > > > > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> > > > > @@ -30,6 +30,21 @@ properties:
>> > > > >            - const: samsung,usb-connector-11pin
>> > > > >            - const: usb-b-connector
>> > > > >
>> > > > > +      - items:
>> > > > > +          - enum:
>> > > > > +              - qcom,sc7280-usb-connector-eud
>> > > > > +          - const: qcom,usb-connector-eud
>> > > > > +          - const: usb-c-connector
>> > > > > +
>> > > > > +  reg:
>> > > > > +    items:
>> > > > > +      - description: EUD Base Register Region
>> > > > > +      - description: EUD Mode Manager Region
>> > > >
>> > > > A connector node represents the physical connector on a board. That
>> > > > can't really be an MMIO peripheral. Maybe you need a node for EUD and
>> > > > then it should have a connector child node? Don't really know without
>> > > > understanding this h/w.
>> > >
>> > > As per the previous discussion on the EUD, it was agreed upon to map
>> > > EUD
>> > > as a type C connector and use Role-Switch to change the USB role
>> > > instead
>> > > of extcon interface that was being used previously. The link for the
>> > > same
>> > > is as follows:-
>> > >
>> > > https://lore.kernel.org/lkml/5db1a666-62ec-c850-6626-ad33d337b452@codeaurora.org/
>> > >
>> >
>> > Not using extcon is the right thing, but perhaps we should make the EUD
>> > a role_switch provider and client, so that we can describe how it sits
>> > inbetween the connector and the controller.
>> >
>> > That way it has the power to pass through or override requests from the
>> > upstream role-switcher, based on the status of EUD.
>> >
>> >
>> > That said, I'm still curious to what happens if I renegotiate the roles
>> > dynamically in a Type-C environment, while enabling EUD. How would the
>> > device on the other end of the cable know that it's supposed to be a
>> > host? Or there's simply a reset of the link when this happens?
>> >
>> > Thanks,
>> > Bjorn
>> 
>> Hi Bjorn,
>> 
> 
> Hi Souradeep
> 
>> By making EUD Role-Switch provider and client do you mean that
>> we should have a EUD node which will have a connector node as
>> child and this connector node will have a port that points towards
>> the drd role-switch?
>> 
>> So that my device tree node will look like the following in that case
>> 
>> eud@88e0000 {
>>         compatible = "qcom,usb-connector-eud";
>>         reg = <0 0x88e0000 0 0x2000>,
>>               <0 0x88e2000 0 0x1000>;
>>         interrupt-parent = <&pdc>;
>>         interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
>>         usb_con: connector {
>>                 compatible = "usb-c-connector";
>>                 label = "USB-C";
>>                 port {
>>                       eud_usb_output: endpoint {
>>                       remote-endpoint = <&eud_usb3_input>;
>>                  };
>>         };
>> 
>> };
>> 
>> 
>> @usb2 {
>>     dwc3 {
>>        usb-role-switch;
>>        port {
>>              eud_usb3_input: endpoint {
>>                    remote-endpoint = <&eud_usb_output>;
>>              };
>>      };
>> };
> 
> While your "output" and "input" matches the direction of the role
> switching, I think the connection should be describe in the other
> direction.
> 
> Also my suggestion was that EUD is both connector for the dwc3 and has 
> a
> reference to the connector described in the TypeC controller - to
> properly describe the relationship:
> 
>   DWC -> EUD -> connector
> 
> With the role switching request going from the connector (pmic_glink
> driver) to DWC through the EUD, which can override the vote.
> 
> 
> That said, this is just my suggestion. You need to ensure that Rob
> understands the hardware design well enough to approve your proposed
> binding.
> 
> 
> E.g. The connector in the EUD isn't a usb-c-connector, it's some
> type of internal connection, the next step in that chain is the actual
> usb-c-connector.
> 
> Regards,
> Bjorn
> 
>> 
>> Also EUD functions only in device mode, so when the role-switch is 
>> done by
>> the controller
>> to set the device mode, the PC on the other end becomes the host.
>> 
>> Thanks,
>> Souradeep

Hi Bjorn/Robb,

Following is how my EUD design looks like

My device tree node will look like as follows


eud@88e0000 {
         compatible = "qcom,usb-connector-eud";
         reg = <0 0x88e0000 0 0x2000>,
                   <0 0x88e2000 0 0x1000>;
         interrupt-parent = <&pdc>;
         interrupts = <11 IRQ_TYPE_LEVEL_HIGH>;
          ports {
                 #address-cells = <2>;
                 #size-cells = <2>;
                 port@0 {
                         reg = <0>;
                         usb2_eud: endpoint {
                                 remote-endpoint = <&eud_ep>;
                         };
                 };
         };
};


@usb2 {
     dwc3 {
        usb-role-switch;
         usb_con: eud_usb_connector {
                 compatible = “usb-connector-eud", "usb-c-connector";
                 ports {
                         #address-cells = <2>;
                         #size-cells = <2>;
                         port@0 {
                                 reg = <0>;
                                 eud_ep: endpoint {
                                         remote-endpoint = <&usb2_eud>;
                                 };
                         };
               };
};


So, I am putting a connector node as a child of dwc3 node which can be 
connected to the EUD node via ports. I am keeping EUD as a separate 
device tree node.

I can get the device equivalent to the connector by doing the following 
in EUD driver

dev = usb_of_get_companion_dev(&pdev->dev);

Once I get the device, I can then do

role_sw = usb_role_switch_get(dev);

This will give me the role switch reference which can be used to set 
device or host role.

The advantage here is that

-> I am keeping the existing yamls untouched which won't be the case if 
I map EUD as child of dwc3.

->This is also consistent with EUD hardware since EUD is not a connector 
in itself but it needs a connector to function
   because it can work only in device mode so the role-switch needs to be 
done.

Let me know your thoughts regarding this.

Thanks,
Souradeep


>> 
>> >
>> > > >
>> > > > > +
>> > > > > +  interrupts:
>> > > > > +    description:
>> > > > > +      EUD interrupt
>> > > > > +
>> > > > >    label:
>> > > > >      description: Symbolic name for the connector.
>> > > > >
>> > > > > --
>> > > > > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
>> > > > > member
>> > > > > of Code Aurora Forum, hosted by The Linux Foundation
>> > > > >
>> > > > >

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2021-11-02  9:20 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-04 11:16 [PATCH V0 0/7] Add Embedded USB Debugger (EUD) driver Souradeep Chowdhury
2021-10-04 11:16 ` [PATCH V0 1/7] dt-bindings: connector: Add property for eud type c connector Souradeep Chowdhury
2021-10-04 16:37   ` Rob Herring
2021-10-05 13:11     ` schowdhu
2021-10-05 16:37       ` Bjorn Andersson
2021-10-07  9:25         ` schowdhu
2021-10-12  3:22           ` Bjorn Andersson
2021-11-02  9:20             ` schowdhu
2021-10-04 11:16 ` [PATCH V0 2/7] dt-bindings: usb: dwc3: Update dwc3 properties for EUD connector Souradeep Chowdhury
2021-10-04 15:50   ` Bjorn Andersson
2021-10-05 12:57     ` schowdhu
2021-10-04 11:16 ` [PATCH V0 3/7] usb: dwc3: drd: Register the eud connector child node for dwc3 Souradeep Chowdhury
2021-10-05  4:42   ` kernel test robot
2021-10-05 12:32   ` kernel test robot
2021-10-04 11:16 ` [PATCH V0 4/7] usb: common: eud: Added the driver support for Embedded USB Debugger(EUD) Souradeep Chowdhury
2021-10-04 11:31   ` Greg KH
2021-10-05 12:54     ` schowdhu
2021-10-04 16:42   ` Bjorn Andersson
2021-10-05  7:14     ` Joe Perches
2021-10-05 13:58       ` schowdhu
2021-10-05 13:01     ` schowdhu
2021-10-04 20:08   ` Randy Dunlap
2021-10-05 13:57     ` schowdhu
2021-10-04 11:16 ` [PATCH V0 5/7] arm64: dts: qcom: sc7280: Add EUD connector node Souradeep Chowdhury
2021-10-04 11:16 ` [PATCH V0 6/7] arm64: dts: qcom: sc7280: Set the default dr_mode for usb2 Souradeep Chowdhury
2021-10-04 11:16 ` [PATCH V0 7/7] MAINTAINERS: Add maintainer entry for EUD Souradeep Chowdhury

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).