Re: [PATCH v2 2/2] phy: add a driver for T-Head TH1520 USB PHY

From: Drew Fustini <fustini@kernel.org>
To: Icenowy Zheng <zhengxingda@iscas.ac.cn>, sashiko-reviews@lists.linux.dev
Cc: neil.armstrong@linaro.org, linux-phy@lists.infradead.org,
	robh@kernel.org, conor+dt@kernel.org, olteanv@gmail.com,
	vkoul@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v2 2/2] phy: add a driver for T-Head TH1520 USB PHY
Date: Wed, 29 Jul 2026 08:51:38 -0700	[thread overview]
Message-ID: <amohirbdNtDLtXeH@x1> (raw)
In-Reply-To: <20260723085527.BDB5A1F000E9@smtp.kernel.org>

On Thu, Jul 23, 2026 at 08:55:27AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Missing explicit inclusion of `<linux/delay.h>` for `udelay()`.
[..]
> > +static int th1520_usb_phy_init(struct phy *phy)
> > +{
> > +	struct th1520_usb_phy *th1520_phy = phy_get_drvdata(phy);
> > +	int ret;
> > +
> > +	ret = clk_prepare_enable(th1520_phy->ref_clk);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = reset_control_assert(th1520_phy->phy_reset);
> > +	if (ret)
> > +		goto err_disable_clk;
> > +
> > +	/*
> > +	 * Do some initial PHY setup:
> > +	 * - Set COMMONONN to allow the PHY to automatically power down.
> > +	 * - Set REF_SSP_EN to enable feeding reference clock to SuperSpeed
> > +	 *   PHY clock PLL.
> > +	 */
> > +	regmap_set_bits(th1520_phy->regmap, USB_SYS, USB_SYS_COMMONONN);
> > +	regmap_set_bits(th1520_phy->regmap, USB_SSP_EN, USB_SSP_EN_REF_SSP_EN);
> > +
> > +	ret = reset_control_deassert(th1520_phy->phy_reset);
> > +	if (ret)
> > +		goto err_disable_clk;
> > +
> > +	udelay(10);
> 
> [Severity: Low]
> This isn't a bug, but this code uses udelay() without explicitly including
> <linux/delay.h>.
> 
> While it currently compiles due to indirect inclusions, should the header be
> explicitly included to prevent build failures if the include chain is later
> refactored?

I noticed that checkpatch complains about 'udelay()':

CHECK: usleep_range is preferred over udelay; see function description
of usleep_range() and udelay().

However, it seems that fsleep(10) may be a better choice than
usleep_range(). It is used by phy-snps-eusb2.c, phy-rzg3e-usb3.c and
phy-exynos5-usbdrd.c.

From include/linux/delay.h:

static inline void fsleep(unsigned long usecs)
{
        if (usecs <= 10)
                udelay(usecs);
        else if (usecs < USLEEP_RANGE_UPPER_BOUND)
                usleep_range(usecs, usecs + (usecs >> max_slack_shift));
        else
                msleep(DIV_ROUND_UP(usecs, USEC_PER_MSEC));
}

Thanks,
Drew

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy