| Message ID | cover.1638430506.git.quic_schowdhu@quicinc.com |
|---|---|
| Headers | show |
| Series | Add Embedded USB Debugger (EUD) driver | expand |
On Thu, Dec 02, 2021 at 03:21:24PM +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 <quic_schowdhu@quicinc.com> > --- > Documentation/ABI/testing/sysfs-driver-eud | 9 + > drivers/soc/qcom/Kconfig | 10 ++ > drivers/soc/qcom/Makefile | 1 + > drivers/soc/qcom/qcom_eud.c | 268 +++++++++++++++++++++++++++++ > 4 files changed, 288 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-driver-eud > create mode 100644 drivers/soc/qcom/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..eaf2e82 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-driver-eud > @@ -0,0 +1,9 @@ > +What: /sys/bus/platform/drivers/eud/.../enable > +Date: October 2021 You sent this patch in December 2021 :( > +Contact: Souradeep Chowdhury <quic_schowdhu@quicinc.com> > +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. By enabling EUD, > + the user is able to activate the mini-usb hub of > + EUD for debug and trace capabilities. > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig > index 79b568f..a4db41b 100644 > --- a/drivers/soc/qcom/Kconfig > +++ b/drivers/soc/qcom/Kconfig > @@ -42,6 +42,16 @@ config QCOM_CPR > To compile this driver as a module, choose M here: the module will > be called qcom-cpr > > +config QCOM_EUD > + tristate "QCOM Embedded USB Debugger(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. On selecting m, the module name > + that is built is qcom_eud.ko > + > config QCOM_GENI_SE > tristate "QCOM GENI Serial Engine Driver" > depends on ARCH_QCOM || COMPILE_TEST > diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile > index ad675a6..3331a40 100644 > --- a/drivers/soc/qcom/Makefile > +++ b/drivers/soc/qcom/Makefile > @@ -4,6 +4,7 @@ obj-$(CONFIG_QCOM_AOSS_QMP) += qcom_aoss.o > obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o > obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o > obj-$(CONFIG_QCOM_CPR) += cpr.o > +obj-$(CONFIG_QCOM_EUD) += qcom_eud.o > obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o > obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o > obj-$(CONFIG_QCOM_OCMEM) += ocmem.o > diff --git a/drivers/soc/qcom/qcom_eud.c b/drivers/soc/qcom/qcom_eud.c > new file mode 100644 > index 0000000..613ac41 > --- /dev/null > +++ b/drivers/soc/qcom/qcom_eud.c > @@ -0,0 +1,268 @@ > +// 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 eud_irq; > + bool enable; > + 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 sysfs_emit(buf, "%d\n", 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); > + bool enable; > + int ret; > + > + if (kstrtobool(buf, &enable)) > + return -EINVAL; > + > + if (enable) { > + ret = enable_eud(chip); > + if (!ret) > + chip->enable = enable; > + } else { > + disable_eud(chip); > + } > + > + return count; > +} > + > +static DEVICE_ATTR_RW(enable); > + > +static struct attribute *eud_attrs[] = { > + &dev_attr_enable.attr, > + NULL, > +}; > + > +static struct attribute_group attr_group = { > + .attrs = eud_attrs, > +}; > + > +static const struct attribute_group *attr_groups[] = { > + &attr_group, > + NULL > +}; ATTRIBUTE_GROUPS()? thanks, greg k-h
On 12/3/2021 4:34 PM, Greg KH wrote: > On Thu, Dec 02, 2021 at 03:21:24PM +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 <quic_schowdhu@quicinc.com> >> --- >> Documentation/ABI/testing/sysfs-driver-eud | 9 + >> drivers/soc/qcom/Kconfig | 10 ++ >> drivers/soc/qcom/Makefile | 1 + >> drivers/soc/qcom/qcom_eud.c | 268 +++++++++++++++++++++++++++++ >> 4 files changed, 288 insertions(+) >> create mode 100644 Documentation/ABI/testing/sysfs-driver-eud >> create mode 100644 drivers/soc/qcom/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..eaf2e82 >> --- /dev/null >> +++ b/Documentation/ABI/testing/sysfs-driver-eud >> @@ -0,0 +1,9 @@ >> +What: /sys/bus/platform/drivers/eud/.../enable >> +Date: October 2021 > You sent this patch in December 2021 :( > Ack > >> +Contact: Souradeep Chowdhury <quic_schowdhu@quicinc.com> >> +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. By enabling EUD, >> + the user is able to activate the mini-usb hub of >> + EUD for debug and trace capabilities. >> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig >> index 79b568f..a4db41b 100644 >> --- a/drivers/soc/qcom/Kconfig >> +++ b/drivers/soc/qcom/Kconfig >> @@ -42,6 +42,16 @@ config QCOM_CPR >> To compile this driver as a module, choose M here: the module will >> be called qcom-cpr >> >> +config QCOM_EUD >> + tristate "QCOM Embedded USB Debugger(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. On selecting m, the module name >> + that is built is qcom_eud.ko >> + >> config QCOM_GENI_SE >> tristate "QCOM GENI Serial Engine Driver" >> depends on ARCH_QCOM || COMPILE_TEST >> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile >> index ad675a6..3331a40 100644 >> --- a/drivers/soc/qcom/Makefile >> +++ b/drivers/soc/qcom/Makefile >> @@ -4,6 +4,7 @@ obj-$(CONFIG_QCOM_AOSS_QMP) += qcom_aoss.o >> obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o >> obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o >> obj-$(CONFIG_QCOM_CPR) += cpr.o >> +obj-$(CONFIG_QCOM_EUD) += qcom_eud.o >> obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o >> obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o >> obj-$(CONFIG_QCOM_OCMEM) += ocmem.o >> diff --git a/drivers/soc/qcom/qcom_eud.c b/drivers/soc/qcom/qcom_eud.c >> new file mode 100644 >> index 0000000..613ac41 >> --- /dev/null >> +++ b/drivers/soc/qcom/qcom_eud.c >> @@ -0,0 +1,268 @@ >> +// 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 eud_irq; >> + bool enable; >> + 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 sysfs_emit(buf, "%d\n", 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); >> + bool enable; >> + int ret; >> + >> + if (kstrtobool(buf, &enable)) >> + return -EINVAL; >> + >> + if (enable) { >> + ret = enable_eud(chip); >> + if (!ret) >> + chip->enable = enable; >> + } else { >> + disable_eud(chip); >> + } >> + >> + return count; >> +} >> + >> +static DEVICE_ATTR_RW(enable); >> + >> +static struct attribute *eud_attrs[] = { >> + &dev_attr_enable.attr, >> + NULL, >> +}; >> + >> +static struct attribute_group attr_group = { >> + .attrs = eud_attrs, >> +}; >> + >> +static const struct attribute_group *attr_groups[] = { >> + &attr_group, >> + NULL >> +}; > ATTRIBUTE_GROUPS()? Ack > > thanks, > > greg k-h
On Thu, Dec 02, 2021 at 03:21:25PM +0530, Souradeep Chowdhury wrote: > Add the Embedded USB Debugger(EUD) device tree node. The > node contains EUD base register region and EUD mode > manager register regions along with the interrupt entry. > Also add the connector to EUD which is mapped as the child > of dwc3. The connector is attached to EUD via port. Also add > the role-switch property to dwc3 node. > > Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com> > --- > arch/arm64/boot/dts/qcom/sc7280.dtsi | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi > index 53a21d0..2d14e5c 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi > @@ -1315,6 +1315,18 @@ > 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"; > + ports { > + port@0 { It is already defined that port@0 of the connector is USB HS data. Is that the case here? What about the SS lines? From the description, it sounds like the data path is DWC3 -> EUD -> connector. The DT structure doesn't match that. > + usb2_role_switch: endpoint { > + remote-endpoint = <&eud_ep>; > + }; > + }; > + }; > + }; > }; > }; > > @@ -1339,6 +1351,19 @@ > interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>; > }; > > + eud: eud@88e0000 { > + compatible = "qcom,sc7280-eud","qcom,eud"; > + reg = <0 0x88e0000 0 0x2000>, > + <0 0x88e2000 0 0x1000>; > + interrupt-parent = <&pdc>; > + interrupts = <11 IRQ_TYPE_LEVEL_HIGH>; > + port { > + eud_ep: endpoint { > + remote-endpoint = <&usb2_role_switch>; > + }; > + }; > + }; > + > nsp_noc: interconnect@a0c0000 { > reg = <0 0x0a0c0000 0 0x10000>; > compatible = "qcom,sc7280-nsp-noc"; > -- > 2.7.4 > >
On Thu, Dec 02, 2021 at 03:21:23PM +0530, Souradeep Chowdhury wrote: > 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 <quic_schowdhu@quicinc.com> > --- > drivers/usb/dwc3/drd.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c > index d7f7683..b4ea55c 100644 > --- a/drivers/usb/dwc3/drd.c > +++ b/drivers/usb/dwc3/drd.c > @@ -8,6 +8,7 @@ > */ > > #include <linux/extcon.h> > +#include <linux/of_platform.h> > #include <linux/of_graph.h> > #include <linux/platform_device.h> > #include <linux/property.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; > + int ret; > + > + of_get_child_by_name(np, "eud_usb_connector"); Connector nodes are named 'connector' or possibly 'usb-connector'. If you are creating an ABI with the node name, it should be documented. However, it's preferred to use 'compatible' for identifying nodes rather than a node name. > + if (!np) { > + dev_dbg(dev, "no usb_connector child node specified\n"); > + return 0; > + } > + > + ret = of_platform_populate(np, NULL, NULL, dev); But why is any of this needed. The connector doesn't have a driver (I expect eventually we will) and the EUD device is not a child. > + 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,10 @@ 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, > -- > 2.7.4 > >
On 12/14/2021 1:33 AM, Rob Herring wrote: > On Thu, Dec 02, 2021 at 03:21:23PM +0530, Souradeep Chowdhury wrote: >> 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 <quic_schowdhu@quicinc.com> >> --- >> drivers/usb/dwc3/drd.c | 26 ++++++++++++++++++++++++++ >> 1 file changed, 26 insertions(+) >> >> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c >> index d7f7683..b4ea55c 100644 >> --- a/drivers/usb/dwc3/drd.c >> +++ b/drivers/usb/dwc3/drd.c >> @@ -8,6 +8,7 @@ >> */ >> >> #include <linux/extcon.h> >> +#include <linux/of_platform.h> >> #include <linux/of_graph.h> >> #include <linux/platform_device.h> >> #include <linux/property.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; >> + int ret; >> + >> + of_get_child_by_name(np, "eud_usb_connector"); > Connector nodes are named 'connector' or possibly 'usb-connector'. If > you are creating an ABI with the node name, it should be documented. > However, it's preferred to use 'compatible' for identifying nodes rather > than a node name. Ack. >> + if (!np) { >> + dev_dbg(dev, "no usb_connector child node specified\n"); >> + return 0; >> + } >> + >> + ret = of_platform_populate(np, NULL, NULL, dev); > But why is any of this needed. The connector doesn't have a driver (I > expect eventually we will) and the EUD device is not a child. Ack. This can be removed as we are no longer mapping EUD as a type C connector. >> + 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,10 @@ 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, >> -- >> 2.7.4 >> >>
On 12/14/2021 1:29 AM, Rob Herring wrote: > On Thu, Dec 02, 2021 at 03:21:25PM +0530, Souradeep Chowdhury wrote: >> Add the Embedded USB Debugger(EUD) device tree node. The >> node contains EUD base register region and EUD mode >> manager register regions along with the interrupt entry. >> Also add the connector to EUD which is mapped as the child >> of dwc3. The connector is attached to EUD via port. Also add >> the role-switch property to dwc3 node. >> >> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/sc7280.dtsi | 25 +++++++++++++++++++++++++ >> 1 file changed, 25 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> index 53a21d0..2d14e5c 100644 >> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi >> @@ -1315,6 +1315,18 @@ >> 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"; >> + ports { >> + port@0 { > It is already defined that port@0 of the connector is USB HS data. Is > that the case here? What about the SS lines? As per the yaml documentation this is to be used for USB HS lines but in this case I am using the port to get the connector fwnode from the eud driver and then perform the role switch from the connector parent which is the dwc3 controller. Should I update the yaml file to include this description? > From the description, it sounds like the data path is DWC3 -> EUD -> > connector. The DT structure doesn't match that. EUD is an independent QCOM IP , it is a mini usb hub which can only function in device mode. That's why it has been mapped as a separate device tree node connected to the type c connector via port endpoint to perform a role switch when necessary. I have kept EUD and connector at the same level as there is no direct data path from EUD to the connector and both are at a level below the controller. > >> + usb2_role_switch: endpoint { >> + remote-endpoint = <&eud_ep>; >> + }; >> + }; >> + }; >> + }; >> }; >> }; >> >> @@ -1339,6 +1351,19 @@ >> interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>; >> }; >> >> + eud: eud@88e0000 { >> + compatible = "qcom,sc7280-eud","qcom,eud"; >> + reg = <0 0x88e0000 0 0x2000>, >> + <0 0x88e2000 0 0x1000>; >> + interrupt-parent = <&pdc>; >> + interrupts = <11 IRQ_TYPE_LEVEL_HIGH>; >> + port { >> + eud_ep: endpoint { >> + remote-endpoint = <&usb2_role_switch>; >> + }; >> + }; >> + }; >> + >> nsp_noc: interconnect@a0c0000 { >> reg = <0 0x0a0c0000 0 0x10000>; >> compatible = "qcom,sc7280-nsp-noc"; >> -- >> 2.7.4 >> >>