[PATCH RFC v3 02/10] usb: misc: qcom_eud: add sysfs attribute for port selection

Elson Serrao posted 10 patches 3 weeks, 3 days ago
[PATCH RFC v3 02/10] usb: misc: qcom_eud: add sysfs attribute for port selection
Posted by Elson Serrao 3 weeks, 3 days ago
EUD can be mapped to either the primary USB port or the secondary USB port
depending on the value of the EUD_PORT_SEL register. Add a 'port' sysfs
attribute to allow userspace to select which port EUD should operate on
and update the ABI documentation. This is needed for systems with dual
USB ports where EUD needs to be accessible on either port depending on the
system configuration and use case.

Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
---
 Documentation/ABI/testing/sysfs-driver-eud | 16 ++++++++
 drivers/usb/misc/qcom_eud.c                | 43 ++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-eud b/Documentation/ABI/testing/sysfs-driver-eud
index 2bab0db2d2f0..67223f73ee60 100644
--- a/Documentation/ABI/testing/sysfs-driver-eud
+++ b/Documentation/ABI/testing/sysfs-driver-eud
@@ -7,3 +7,19 @@ Description:
 		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.
+
+What:		/sys/bus/platform/drivers/qcom_eud/.../port
+Date:		January 2026
+Contact:	Elson Serrao <elson.serrao@oss.qualcomm.com>
+Description:
+		Selects which USB port the Embedded USB Debugger (EUD)
+		is mapped to on platforms providing multiple High-Speed
+		USB ports.
+
+		Valid values:
+		  0 - Primary USB port
+		  1 - Secondary USB port
+
+		The attribute is writable only while EUD is disabled.
+		Reading the attribute returns the currently selected
+		USB port number.
diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
index 926419ca560f..1a136f8f1ae5 100644
--- a/drivers/usb/misc/qcom_eud.c
+++ b/drivers/usb/misc/qcom_eud.c
@@ -23,8 +23,11 @@
 #define EUD_REG_VBUS_INT_CLR	0x0080
 #define EUD_REG_CSR_EUD_EN	0x1014
 #define EUD_REG_SW_ATTACH_DET	0x1018
+#define EUD_REG_PORT_SEL	0x1028
 #define EUD_REG_EUD_EN2		0x0000
 
+#define EUD_MAX_PORTS		2
+
 #define EUD_ENABLE		BIT(0)
 #define EUD_INT_PET_EUD		BIT(0)
 #define EUD_INT_VBUS		BIT(2)
@@ -40,6 +43,7 @@ struct eud_chip {
 	int				irq;
 	bool				enabled;
 	bool				usb_attached;
+	u8				port_idx;
 };
 
 static int enable_eud(struct eud_chip *priv)
@@ -104,8 +108,47 @@ static ssize_t enable_store(struct device *dev,
 
 static DEVICE_ATTR_RW(enable);
 
+static ssize_t port_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct eud_chip *chip = dev_get_drvdata(dev);
+
+	return sysfs_emit(buf, "%u\n", chip->port_idx);
+}
+
+static ssize_t port_store(struct device *dev,
+		struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	struct eud_chip *chip = dev_get_drvdata(dev);
+	u8 port;
+	int ret;
+
+	ret = kstrtou8(buf, 0, &port);
+	if (ret)
+		return ret;
+
+	/* Only port 0 and port 1 are valid */
+	if (port >= EUD_MAX_PORTS)
+		return -EINVAL;
+
+	/* Port selection must be done before enabling EUD */
+	if (chip->enabled) {
+		dev_err(chip->dev, "Cannot change port while EUD is enabled\n");
+		return -EBUSY;
+	}
+
+	writel(port, chip->base + EUD_REG_PORT_SEL);
+	chip->port_idx = port;
+
+	return count;
+}
+
+static DEVICE_ATTR_RW(port);
+
 static struct attribute *eud_attrs[] = {
 	&dev_attr_enable.attr,
+	&dev_attr_port.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(eud);
-- 
2.34.1
Re: [PATCH RFC v3 02/10] usb: misc: qcom_eud: add sysfs attribute for port selection
Posted by Greg Kroah-Hartman 3 weeks, 1 day ago
On Mon, Mar 09, 2026 at 01:33:29PM -0700, Elson Serrao wrote:
> EUD can be mapped to either the primary USB port or the secondary USB port
> depending on the value of the EUD_PORT_SEL register. Add a 'port' sysfs
> attribute to allow userspace to select which port EUD should operate on
> and update the ABI documentation. This is needed for systems with dual
> USB ports where EUD needs to be accessible on either port depending on the
> system configuration and use case.
> 
> Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
> ---
>  Documentation/ABI/testing/sysfs-driver-eud | 16 ++++++++
>  drivers/usb/misc/qcom_eud.c                | 43 ++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-eud b/Documentation/ABI/testing/sysfs-driver-eud
> index 2bab0db2d2f0..67223f73ee60 100644
> --- a/Documentation/ABI/testing/sysfs-driver-eud
> +++ b/Documentation/ABI/testing/sysfs-driver-eud
> @@ -7,3 +7,19 @@ Description:
>  		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.
> +
> +What:		/sys/bus/platform/drivers/qcom_eud/.../port
> +Date:		January 2026
> +Contact:	Elson Serrao <elson.serrao@oss.qualcomm.com>
> +Description:
> +		Selects which USB port the Embedded USB Debugger (EUD)
> +		is mapped to on platforms providing multiple High-Speed
> +		USB ports.
> +
> +		Valid values:
> +		  0 - Primary USB port
> +		  1 - Secondary USB port

Why not use "primary" and "secondary" as values instead?  That makes
this much simpler to understand.


> +
> +		The attribute is writable only while EUD is disabled.
> +		Reading the attribute returns the currently selected
> +		USB port number.
> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
> index 926419ca560f..1a136f8f1ae5 100644
> --- a/drivers/usb/misc/qcom_eud.c
> +++ b/drivers/usb/misc/qcom_eud.c
> @@ -23,8 +23,11 @@
>  #define EUD_REG_VBUS_INT_CLR	0x0080
>  #define EUD_REG_CSR_EUD_EN	0x1014
>  #define EUD_REG_SW_ATTACH_DET	0x1018
> +#define EUD_REG_PORT_SEL	0x1028
>  #define EUD_REG_EUD_EN2		0x0000
>  
> +#define EUD_MAX_PORTS		2
> +
>  #define EUD_ENABLE		BIT(0)
>  #define EUD_INT_PET_EUD		BIT(0)
>  #define EUD_INT_VBUS		BIT(2)
> @@ -40,6 +43,7 @@ struct eud_chip {
>  	int				irq;
>  	bool				enabled;
>  	bool				usb_attached;
> +	u8				port_idx;
>  };
>  
>  static int enable_eud(struct eud_chip *priv)
> @@ -104,8 +108,47 @@ static ssize_t enable_store(struct device *dev,
>  
>  static DEVICE_ATTR_RW(enable);
>  
> +static ssize_t port_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct eud_chip *chip = dev_get_drvdata(dev);
> +
> +	return sysfs_emit(buf, "%u\n", chip->port_idx);

Wait, you are returning a 0/1 here, yet using a u8?  How can that work?

confused,

greg k-h
Re: [PATCH RFC v3 02/10] usb: misc: qcom_eud: add sysfs attribute for port selection
Posted by Konrad Dybcio 2 weeks, 6 days ago
On 3/11/26 2:16 PM, Greg Kroah-Hartman wrote:
> On Mon, Mar 09, 2026 at 01:33:29PM -0700, Elson Serrao wrote:
>> EUD can be mapped to either the primary USB port or the secondary USB port
>> depending on the value of the EUD_PORT_SEL register. Add a 'port' sysfs
>> attribute to allow userspace to select which port EUD should operate on
>> and update the ABI documentation. This is needed for systems with dual
>> USB ports where EUD needs to be accessible on either port depending on the
>> system configuration and use case.
>>
>> Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
>> ---
>>  Documentation/ABI/testing/sysfs-driver-eud | 16 ++++++++
>>  drivers/usb/misc/qcom_eud.c                | 43 ++++++++++++++++++++++
>>  2 files changed, 59 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-eud b/Documentation/ABI/testing/sysfs-driver-eud
>> index 2bab0db2d2f0..67223f73ee60 100644
>> --- a/Documentation/ABI/testing/sysfs-driver-eud
>> +++ b/Documentation/ABI/testing/sysfs-driver-eud
>> @@ -7,3 +7,19 @@ Description:
>>  		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.
>> +
>> +What:		/sys/bus/platform/drivers/qcom_eud/.../port
>> +Date:		January 2026
>> +Contact:	Elson Serrao <elson.serrao@oss.qualcomm.com>
>> +Description:
>> +		Selects which USB port the Embedded USB Debugger (EUD)
>> +		is mapped to on platforms providing multiple High-Speed
>> +		USB ports.
>> +
>> +		Valid values:
>> +		  0 - Primary USB port
>> +		  1 - Secondary USB port
> 
> Why not use "primary" and "secondary" as values instead?  That makes
> this much simpler to understand.

It's not inconceivable that future iterations of this hardware will let
one mux between more than two USB PHYs/ports (there are already a multitude
of SoCs where we have 4 or more USB controllers and >= that ports)

Konrad
Re: [PATCH RFC v3 02/10] usb: misc: qcom_eud: add sysfs attribute for port selection
Posted by Greg Kroah-Hartman 2 weeks, 6 days ago
On Fri, Mar 13, 2026 at 01:10:10PM +0100, Konrad Dybcio wrote:
> On 3/11/26 2:16 PM, Greg Kroah-Hartman wrote:
> > On Mon, Mar 09, 2026 at 01:33:29PM -0700, Elson Serrao wrote:
> >> EUD can be mapped to either the primary USB port or the secondary USB port
> >> depending on the value of the EUD_PORT_SEL register. Add a 'port' sysfs
> >> attribute to allow userspace to select which port EUD should operate on
> >> and update the ABI documentation. This is needed for systems with dual
> >> USB ports where EUD needs to be accessible on either port depending on the
> >> system configuration and use case.
> >>
> >> Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
> >> ---
> >>  Documentation/ABI/testing/sysfs-driver-eud | 16 ++++++++
> >>  drivers/usb/misc/qcom_eud.c                | 43 ++++++++++++++++++++++
> >>  2 files changed, 59 insertions(+)
> >>
> >> diff --git a/Documentation/ABI/testing/sysfs-driver-eud b/Documentation/ABI/testing/sysfs-driver-eud
> >> index 2bab0db2d2f0..67223f73ee60 100644
> >> --- a/Documentation/ABI/testing/sysfs-driver-eud
> >> +++ b/Documentation/ABI/testing/sysfs-driver-eud
> >> @@ -7,3 +7,19 @@ Description:
> >>  		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.
> >> +
> >> +What:		/sys/bus/platform/drivers/qcom_eud/.../port
> >> +Date:		January 2026
> >> +Contact:	Elson Serrao <elson.serrao@oss.qualcomm.com>
> >> +Description:
> >> +		Selects which USB port the Embedded USB Debugger (EUD)
> >> +		is mapped to on platforms providing multiple High-Speed
> >> +		USB ports.
> >> +
> >> +		Valid values:
> >> +		  0 - Primary USB port
> >> +		  1 - Secondary USB port
> > 
> > Why not use "primary" and "secondary" as values instead?  That makes
> > this much simpler to understand.
> 
> It's not inconceivable that future iterations of this hardware will let
> one mux between more than two USB PHYs/ports (there are already a multitude
> of SoCs where we have 4 or more USB controllers and >= that ports)

Great, and how will that be defined?  As a number?  Name?  Something
else?

thanks,

greg k-h
Re: [PATCH RFC v3 02/10] usb: misc: qcom_eud: add sysfs attribute for port selection
Posted by Konrad Dybcio 2 weeks, 6 days ago
On 3/13/26 1:45 PM, Greg Kroah-Hartman wrote:
> On Fri, Mar 13, 2026 at 01:10:10PM +0100, Konrad Dybcio wrote:
>> On 3/11/26 2:16 PM, Greg Kroah-Hartman wrote:
>>> On Mon, Mar 09, 2026 at 01:33:29PM -0700, Elson Serrao wrote:
>>>> EUD can be mapped to either the primary USB port or the secondary USB port
>>>> depending on the value of the EUD_PORT_SEL register. Add a 'port' sysfs
>>>> attribute to allow userspace to select which port EUD should operate on
>>>> and update the ABI documentation. This is needed for systems with dual
>>>> USB ports where EUD needs to be accessible on either port depending on the
>>>> system configuration and use case.
>>>>
>>>> Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
>>>> ---
>>>>  Documentation/ABI/testing/sysfs-driver-eud | 16 ++++++++
>>>>  drivers/usb/misc/qcom_eud.c                | 43 ++++++++++++++++++++++
>>>>  2 files changed, 59 insertions(+)
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-driver-eud b/Documentation/ABI/testing/sysfs-driver-eud
>>>> index 2bab0db2d2f0..67223f73ee60 100644
>>>> --- a/Documentation/ABI/testing/sysfs-driver-eud
>>>> +++ b/Documentation/ABI/testing/sysfs-driver-eud
>>>> @@ -7,3 +7,19 @@ Description:
>>>>  		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.
>>>> +
>>>> +What:		/sys/bus/platform/drivers/qcom_eud/.../port
>>>> +Date:		January 2026
>>>> +Contact:	Elson Serrao <elson.serrao@oss.qualcomm.com>
>>>> +Description:
>>>> +		Selects which USB port the Embedded USB Debugger (EUD)
>>>> +		is mapped to on platforms providing multiple High-Speed
>>>> +		USB ports.
>>>> +
>>>> +		Valid values:
>>>> +		  0 - Primary USB port
>>>> +		  1 - Secondary USB port
>>>
>>> Why not use "primary" and "secondary" as values instead?  That makes
>>> this much simpler to understand.
>>
>> It's not inconceivable that future iterations of this hardware will let
>> one mux between more than two USB PHYs/ports (there are already a multitude
>> of SoCs where we have 4 or more USB controllers and >= that ports)
> 
> Great, and how will that be defined?  As a number?  Name?  Something
> else?

Perhaps it makes more sense to me, since if I look at the other
computer, the controllers are named USB0, USB1 and so on.. We follow
this naming in DT too, so it felt natural

How they end up mapping to physical connectors is of course up to the
implementer.

Ultimately, I don't mind either

Konrad
Re: [PATCH RFC v3 02/10] usb: misc: qcom_eud: add sysfs attribute for port selection
Posted by Elson Serrao 3 weeks, 1 day ago

On 3/11/2026 6:16 AM, Greg Kroah-Hartman wrote:
[...]

>> +
>> +What:		/sys/bus/platform/drivers/qcom_eud/.../port
>> +Date:		January 2026
>> +Contact:	Elson Serrao <elson.serrao@oss.qualcomm.com>
>> +Description:
>> +		Selects which USB port the Embedded USB Debugger (EUD)
>> +		is mapped to on platforms providing multiple High-Speed
>> +		USB ports.
>> +
>> +		Valid values:
>> +		  0 - Primary USB port
>> +		  1 - Secondary USB port
> 
> Why not use "primary" and "secondary" as values instead?  That makes
> this much simpler to understand.
> 

[...]

>>  
>> +static ssize_t port_show(struct device *dev,
>> +		struct device_attribute *attr, char *buf)
>> +{
>> +	struct eud_chip *chip = dev_get_drvdata(dev);
>> +
>> +	return sysfs_emit(buf, "%u\n", chip->port_idx);
> 
> Wait, you are returning a 0/1 here, yet using a u8?  How can that work?
> 

Thanks for the review, Greg.
I’ll switch the sysfs attribute to use "primary"/"secondary" values
(instead of numeric 0/1) as suggested and update the ABI doc and driver.
This should also resolve the ambiguity around the current 0/1 output.

Regards,
Elson