Re: [PATCH v9 5/8] usb: misc: qcom_eud: improve enable_store API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] usb: qcom_eud: uninitialized software state blocks disabling EUD on boot
--

commit 16556b4a5664eacf6eb75fc1fe5031d31c087dc0
Author: Elson Serrao <elson.serrao@oss.qualcomm.com>

usb: misc: qcom_eud: improve enable_store API

This commit adds a state check in enable_store() to skip redundant hardware
operations when the EUD is already in the desired state. It also improves
error handling and logging when enabling or disabling the EUD.

> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
> index 48e474bdf818c..e227d05cfecd4 100644
> --- a/drivers/usb/misc/qcom_eud.c
> +++ b/drivers/usb/misc/qcom_eud.c
[ ... ]
> @@ -164,18 +164,27 @@ static ssize_t enable_store(struct device *dev,
>  	if (kstrtobool(buf, &enable))
>  		return -EINVAL;
>  
> +	/* Skip operation if already in desired state */
> +	if (chip->enabled == enable)
> +		return count;

[Severity: Medium]
Will this early exit prevent users from disabling the hardware if the
bootloader left the EUD enabled?

Since eud_probe() initializes chip->enabled to false (typically via
devm_kzalloc), it does not verify or synchronize the software state with
the actual hardware state by reading the hardware control registers (e.g.,
EUD_REG_CSR_EUD_EN).

If the bootloader leaves the EUD enabled, the hardware is active but
chip->enabled is false.

If a user then writes 0 to the sysfs attribute to explicitly disable the
EUD, this check evaluates to true (false == false) and returns early.

This bypasses the disable_eud() hardware routine, preventing the hardware
from being disabled and potentially impacting power consumption or role
switching.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929213513.2401005-1-elson.serrao@oss.qualcomm.com?part=5