Skip to contentSkip to conversation

Add Embedded USB Debugger (EUD) driver

PATCH In review Prakruthi Deepak Heragu 5 participants 9 messages Nov 15, 2018 – Jan 24, 2020

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

v3 cover Add Embedded USB Debugger (EUD) driver

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, charge phone etc. EUD driver listens to events like USB attach or detach and charger enable or disable and then notifies the USB driver or PMIC driver respectively about these events via EXTCON. At regular intervals, the EUD driver receives an interrupt to pet the driver indicating that the software is functional.

Changes since v2:
- Remove module_param and add sysfs support instead
- Simplify if-else condition
- Change if-elseif to switch case
- Return -ENOMEM
- Got rid of unnecessary checks in sysfs store function
- Updated the dt-binding
Changes since v1:
- Remove EUD_NR as it is an unused macro
Changes since v0:
- Remove select SERIAL_CORE from Kconfig as this patch doesn't involve anything related to serial console
- Changed the dt-bindings to remove extcon and replace it with graphs to represent a connection with client

Prakruthi Deepak Heragu (2):
dt-bindings: Documentation for qcom,eud
Embedded USB Debugger (EUD) driver

 .../devicetree/bindings/soc/qcom/qcom,msm-eud.txt  |  41 +++
 drivers/soc/qcom/Kconfig                           |  12 +
 drivers/soc/qcom/Makefile                          |   1 +
 drivers/soc/qcom/eud.c                             | 338 +++++++++++++++++++++
 4 files changed, 392 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
 create mode 100644 drivers/soc/qcom/eud.c
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#1 lore raw

PATCH v3 1/2 dt-bindings: Documentation for qcom,eud

Documentation for Embedded USB Debugger (EUD) device tree bindings.

---
 .../devicetree/bindings/soc/qcom/qcom,msm-eud.txt  | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +43 -0
new file mode 100644
index 0000000..6d339e7
@@ -0,0 +1,43 @@
+* Qualcomm Technologies Inc Embedded USB Debugger (EUD)
+
+The EUD (Embedded USB Debugger) is a mini-USB hub implemented
+on chip to support the USB-based debug and trace capabilities.
+
+Required properties:
+
+ - compatible:  Should be "qcom,msm-eud"
+ - interrupts:  Interrupt number
+ - reg: Should be address and size of EUD register space
+
+EUD notifies clients for VBUS attach/detach and charger enable/disable
+events. The link between client and EUD is established via a directed
+graph. EUD has one endpoint of the graph link mentioning EUD as an
+output link and clients of EUD should have the other endpoint of the
+graph link as an input link.
+
+An example for EUD device node:
+
+	eud: qcom,msm-eud@88e0000 {
+		compatible = "qcom,msm-eud";
+		interrupts = <GIC_SPI 492 IRQ_TYPE_LEVEL_HIGH>;
+		reg = <0x88e0000 0x4000>;
+		usb_con: connector {
+                compatible = "usb-c-connector";
+                label = "USB-C";
+                port {
+                	    eud_usb_output: endpoint {
+                        	    remote-endpoint = <&eud_usb3_input>;
+                        };
+        	    };
+        };
+	};
+
+An example for EUD client:
+
+	usb3 {
+		port {
+                eud_usb3_input: endpoint {
+                        remote-endpoint = <&eud_usb_output>;
+                };
+        };
+	};
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
maintainer
#2 lore raw

Re: [PATCH v3 1/2] dt-bindings: Documentation for qcom,eud

Quoted 1 line · Prakruthi Deepak Heragu

Documentation for Embedded USB Debugger (EUD) device tree bindings.

I asked questions on v2 which were never answered.

Quoted 61 lines
---
 .../devicetree/bindings/soc/qcom/qcom,msm-eud.txt  | 43 ++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt
Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +43 -0
new file mode 100644
index 0000000..6d339e7
@@ -0,0 +1,43 @@
+* Qualcomm Technologies Inc Embedded USB Debugger (EUD)
+
+The EUD (Embedded USB Debugger) is a mini-USB hub implemented
+on chip to support the USB-based debug and trace capabilities.
+
+Required properties:
+
+ - compatible:  Should be "qcom,msm-eud"
+ - interrupts:  Interrupt number
+ - reg: Should be address and size of EUD register space
+
+EUD notifies clients for VBUS attach/detach and charger enable/disable
+events. The link between client and EUD is established via a directed
+graph. EUD has one endpoint of the graph link mentioning EUD as an
+output link and clients of EUD should have the other endpoint of the
+graph link as an input link.
+
+An example for EUD device node:
+
+	eud: qcom,msm-eud@88e0000 {
+		compatible = "qcom,msm-eud";
+		interrupts = <GIC_SPI 492 IRQ_TYPE_LEVEL_HIGH>;
+		reg = <0x88e0000 0x4000>;
+		usb_con: connector {
+                compatible = "usb-c-connector";
+                label = "USB-C";
+                port {
+                	    eud_usb_output: endpoint {
+                        	    remote-endpoint = <&eud_usb3_input>;
+                        };
+        	    };
+        };
+	};
+
+An example for EUD client:
+
+	usb3 {
+		port {
+                eud_usb3_input: endpoint {
+                        remote-endpoint = <&eud_usb_output>;
+                };
+        };
+	};
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
#3 lore raw

Re: [PATCH v3 1/2] dt-bindings: Documentation for qcom,eud

Quoted 3 lines · Rob Herring
Quoted 1 line · Prakruthi Deepak Heragu

Documentation for Embedded USB Debugger (EUD) device tree bindings.

I asked questions on v2 which were never answered.

HI Rob,

Can you please review my replies against your comments on patchset v2?

Quoted 61 lines
Quoted 61 lines
---
  .../devicetree/bindings/soc/qcom/qcom,msm-eud.txt  | 43 ++++++++++++++++++++++
  1 file changed, 43 insertions(+)

create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt

Documentation/devicetree/bindings/soc/qcom/qcom,msm-eud.txt new file +43 -0
new file mode 100644
index 0000000..6d339e7
@@ -0,0 +1,43 @@
+* Qualcomm Technologies Inc Embedded USB Debugger (EUD)
+
+The EUD (Embedded USB Debugger) is a mini-USB hub implemented
+on chip to support the USB-based debug and trace capabilities.
+
+Required properties:
+
+ - compatible:  Should be "qcom,msm-eud"
+ - interrupts:  Interrupt number
+ - reg: Should be address and size of EUD register space
+
+EUD notifies clients for VBUS attach/detach and charger enable/disable
+events. The link between client and EUD is established via a directed
+graph. EUD has one endpoint of the graph link mentioning EUD as an
+output link and clients of EUD should have the other endpoint of the
+graph link as an input link.
+
+An example for EUD device node:
+
+	eud: qcom,msm-eud@88e0000 {
+		compatible = "qcom,msm-eud";
+		interrupts = <GIC_SPI 492 IRQ_TYPE_LEVEL_HIGH>;
+		reg = <0x88e0000 0x4000>;
+		usb_con: connector {
+                compatible = "usb-c-connector";
+                label = "USB-C";
+                port {
+                	    eud_usb_output: endpoint {
+                        	    remote-endpoint = <&eud_usb3_input>;
+                        };
+        	    };
+        };
+	};
+
+An example for EUD client:
+
+	usb3 {
+		port {
+                eud_usb3_input: endpoint {
+                        remote-endpoint = <&eud_usb_output>;
+                };
+        };
+	};
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
#4 lore raw

PATCH v3 2/2 Embedded USB Debugger (EUD) driver

In reply to Prakruthi Deepak Heragu

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

---
 drivers/soc/qcom/Kconfig  |  12 ++
 drivers/soc/qcom/Makefile |   1 +
 drivers/soc/qcom/eud.c    | 344 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 357 insertions(+)
 create mode 100644 drivers/soc/qcom/eud.c
drivers/soc/qcom/Kconfig +12 -0
index 5856e79..12669ec 100644
@@ -136,4 +136,16 @@ config QCOM_APR
           application processor and QDSP6. APR is
           used by audio driver to configure QDSP6
           ASM, ADM and AFE modules.
+
+config QCOM_EUD
+	tristate "QTI Embedded USB Debugger (EUD)"
+	depends on ARCH_QCOM
+	help
+	  The Embedded USB Debugger (EUD) driver is a driver for the
+	  control peripheral which waits on events like USB attach/detach
+	  and charger enable/disable. The control peripheral further helps
+	  support the USB-based debug and trace capabilities.
+	  This module enables support for Qualcomm Technologies, Inc.
+	  Embedded USB Debugger (EUD).
+	  If unsure, say N.
 endmenu
drivers/soc/qcom/Makefile +1 -0
index 19dcf95..dd4701b 100644
@@ -15,3 +15,4 @@ obj-$(CONFIG_QCOM_SMP2P)	+= smp2p.o
 obj-$(CONFIG_QCOM_SMSM)	+= smsm.o
 obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
 obj-$(CONFIG_QCOM_APR) += apr.o
+obj-$(CONFIG_QCOM_EUD) += eud.o
drivers/soc/qcom/eud.c new file +344 -0

344 lines not on this page · raw message

Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
maintainer
#5 lore raw

Re: [PATCH v3 2/2] Embedded USB Debugger (EUD) driver

Quoted 4 lines · Prakruthi Deepak Heragu

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

If you add/remove/change a sysfs file, you need to also have a Documentation/ABI/ file update as well. Please do that here.

thanks,

greg k-h

#6 lore raw

Re: [PATCH v3 2/2] Embedded USB Debugger (EUD) driver

Quoted 11 lines · Greg KH
Quoted 4 lines · Prakruthi Deepak Heragu

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

If you add/remove/change a sysfs file, you need to also have a Documentation/ABI/ file update as well. Please do that here.

thanks,

greg k-h

Thank you very much Greg for your time to review, Shall i go ahead posting next patch set v4 addressing your comments?

I was awaiting Rob's review of patch set v3 and also have posted replies to his comments on patch setv-v2.

Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
maintainer
#7 lore raw

Re: [PATCH v3 2/2] Embedded USB Debugger (EUD) driver

Quoted 10 lines · Avaneesh Kumar (avani)
Quoted 8 lines · Greg KH
Quoted 4 lines · Prakruthi Deepak Heragu

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

If you add/remove/change a sysfs file, you need to also have a Documentation/ABI/ file update as well. Please do that here.

Quoted 6 lines
Quoted 3 lines

thanks,

greg k-h

Thank you very much Greg for your time to review, Shall i go ahead posting next patch set v4 addressing your comments?

Why wouldn't you? :)

#8 lore raw

Re: [PATCH v3 2/2] Embedded USB Debugger (EUD) driver

In reply to Prakruthi Deepak Heragu

drivers/soc/qcom/eud.c line 145

Quoted 6 lines · Prakruthi Deepak Heragu
drivers/soc/qcom/eud.c new file +6 -0
+struct device_attribute eud_attribute = {
+	.attr.name = "enable",
+	.attr.mode = 0644,
+	.show = eud_enable_show,
+	.store = eud_enable_store,
+};

Please use:

	static DEVICE_ATTR_RW(enable);

instead of open-coding all of that mess.

Also it makes it static, this should not be a global symbol.

Quoted 119 lines
drivers/soc/qcom/eud.c new file +119 -0
+
+static void eud_event_notifier(struct work_struct *eud_work)
+{
+	struct eud_chip *chip = container_of(eud_work, struct eud_chip,
+					eud_work);
+	int ret;
+
+	if (chip->int_status == EUD_INT_VBUS) {
+		ret = extcon_set_state_sync(chip->extcon, chip->extcon_id,
+					chip->usb_attach);
+		if (ret)
+			return;
+	} else if (chip->int_status == EUD_INT_CHGR) {
+		ret = extcon_set_state_sync(chip->extcon, chip->extcon_id,
+					chip->chgr_enable);
+		if (ret)
+			return;
+	}
+}
+
+static void usb_attach_detach(struct eud_chip *chip)
+{
+	u32 reg;
+
+	chip->extcon_id = EXTCON_USB;
+	/* read ctl_out_1[4] to find USB attach or detach event */
+	reg = readl_relaxed(chip->eud_reg_base + EUD_REG_CTL_OUT_1);
+	if (reg & BIT(4))
+		chip->usb_attach = true;
+	else
+		chip->usb_attach = false;
+
+	schedule_work(&chip->eud_work);
+
+	/* set and clear vbus_int_clr[0] to clear interrupt */
+	writel_relaxed(BIT(0), chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
+	/* Ensure Register Writes Complete */
+	wmb();
+	writel_relaxed(0, chip->eud_reg_base + EUD_REG_VBUS_INT_CLR);
+}
+
+static void chgr_enable_disable(struct eud_chip *chip)
+{
+	u32 reg;
+
+	chip->extcon_id = EXTCON_CHG_USB_SDP;
+	/* read ctl_out_1[6] to find charger enable or disable event */
+	reg = readl_relaxed(chip->eud_reg_base + EUD_REG_CTL_OUT_1);
+	if (reg & BIT(6))
+		chip->chgr_enable = true;
+	else
+		chip->chgr_enable = false;
+
+	schedule_work(&chip->eud_work);
+
+	/* set and clear chgr_int_clr[0] to clear interrupt */
+	writel_relaxed(BIT(0), chip->eud_reg_base + EUD_REG_CHGR_INT_CLR);
+	/* Ensure Register Writes Complete */
+	wmb();
+	writel_relaxed(0, chip->eud_reg_base + EUD_REG_CHGR_INT_CLR);
+}
+
+static void pet_eud(struct eud_chip *chip)
+{
+	u32 reg;
+
+	/* read sw_attach_det[0] to find attach/detach event */
+	reg = readl_relaxed(chip->eud_reg_base + EUD_REG_SW_ATTACH_DET);
+	if (reg & BIT(0)) {
+		/* Detach & Attach pet for EUD */
+		writel_relaxed(0, chip->eud_reg_base + EUD_REG_SW_ATTACH_DET);
+		/* Ensure Register Writes Complete */
+		wmb();
+		/* Delay to make sure detach pet is done before attach pet */
+		udelay(100);
+		writel_relaxed(BIT(0), chip->eud_reg_base +
+					EUD_REG_SW_ATTACH_DET);
+		/* Ensure Register Writes Complete */
+		wmb();
+	} else {
+		/* Attach pet for EUD */
+		writel_relaxed(BIT(0), chip->eud_reg_base +
+					EUD_REG_SW_ATTACH_DET);
+		/* Ensure Register Writes Complete */
+		wmb();
+	}
+}
+
+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_relaxed(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);
+		break;
+	case EUD_INT_CHGR:
+		chip->int_status = EUD_INT_CHGR;
+		chgr_enable_disable(chip);
+		break;
+	case EUD_INT_SAFE_MODE:
+		pet_eud(chip);
+		break;
+	default:
+		return IRQ_NONE;
+	}
+	return IRQ_HANDLED;
+}
+
+static int msm_eud_probe(struct platform_device *pdev)
+{
+	struct eud_chip *chip;
+	struct resource *res;
+	int ret;
+

drivers/soc/qcom/eud.c line 274

Quoted 10 lines
drivers/soc/qcom/eud.c new file +10 -0
+	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->dev = &pdev->dev;
+	platform_set_drvdata(pdev, chip);
+
+	ret = device_create_file(&pdev->dev, &eud_attribute);
+	if (ret)
+		return ret;

You just raced with userspace and lost :(

Isn't there a way to add the attribute to the platform device as a group to have it correctly added by the driver core?

Also, you are adding an attribute to a structure that you do not control/own, are you _sure_ you want to do that? It's a bit odd and you don't really control the lifetime of that device.

drivers/soc/qcom/eud.c line 278

Quoted 4 lines
drivers/soc/qcom/eud.c new file +4 -0
+
+	chip->extcon = devm_extcon_dev_allocate(&pdev->dev, eud_extcon_cable);
+	if (IS_ERR(chip->extcon))
+		return PTR_ERR(chip->extcon);

As an example of this problem, the sysfs file is now there if this was an error. Yet that sysfs file now means nothing and if userspace touches it bad things will happen.

So please at the very least, properly clean up your error paths. And look into doing this correctly.

thanks,

greg k-h