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

[Greg Kroah-Hartman]

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@xxxxxxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxxx>
> +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