Re: [PATCH v8 3/8] usb: misc: qcom_eud: add per-path High-Speed PHY control

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Early return in disable_eud skips PHY disablement, leaking High-Speed PHY resources.

Pre-existing issues:
- [High] Calling readl_poll_timeout from a hardirq handler sleeps in atomic context.
--

commit 895071c1614ba92a198085df0882e77e4e133a74
Author: Elson Serrao <elson.serrao@oss.qualcomm.com>

usb: misc: qcom_eud: add per-path High-Speed PHY control

EUD hardware can support multiple High-Speed USB paths, each routed
through its own PHY. This patch restructures the driver to implement
per-path PHY management, ensuring that the PHY for the currently selected
path is properly initialized and powered.

Reviewed-by: Peter Chen <peter.chen@kernel.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>

> diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c
> index 25a6a8f110ec8..868725ad35e09 100644
> --- a/drivers/usb/misc/qcom_eud.c
> +++ b/drivers/usb/misc/qcom_eud.c
[ ... ]
> @@ -75,6 +135,8 @@ static int disable_eud(struct eud_chip *priv)
>  		return ret;
>  
>  	writel(0, priv->base + EUD_REG_CSR_EUD_EN);
> +	eud_phy_disable(priv);
> +
>  	return 0;
>  }

[Severity: Medium]
Does this early return bypass the newly introduced PHY disablement on error?

If qcom_scm_io_writel() fails, the function returns early and skips
eud_phy_disable(priv). Will this leave the High-Speed PHY resources
initialized and powered on, leaking the hardware state?

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to call
readl_poll_timeout() from a hardirq handler?

When the EUD_INT_SAFE_MODE hardware interrupt fires, the primary hardirq
handler directly calls pet_eud():

drivers/usb/misc/qcom_eud.c:handle_eud_irq() {
	...
	case EUD_INT_SAFE_MODE:
		pet_eud(chip);
		return IRQ_HANDLED;
}

Which then calls readl_poll_timeout() with a non-zero delay:

drivers/usb/misc/qcom_eud.c:pet_eud() {
	...
	ret = readl_poll_timeout(chip->base + EUD_REG_SW_ATTACH_DET,
				reg, (reg == 0), 1, 100);
	...
}

Since readl_poll_timeout() internally uses usleep_range() which sleeps,
can this cause a panic or deadlock by sleeping in an atomic context?

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