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

[sashiko-bot@kernel.org]

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()`.
--

--- Patch [2]: [PATCH v2 2/2] phy: add a driver for T-Head TH1520 USB PHY ---
commit 0212e7cbab21327c62c749f25a7e96aa29c7d1a3
Author: Icenowy Zheng <zhengxingda@iscas.ac.cn>

phy: add a driver for T-Head TH1520 USB PHY
    
The USB PHY on T-Head TH1520 SoC is a Synopsys USB 3.0 FemtoPHY, with
some PHY parameters exported as another system controller along with it.
This commit adds a driver configuring these parameters before letting the
PHY run, in addition to clock/reset/regulator management.

> diff --git a/drivers/phy/thead/phy-th1520-usb.c b/drivers/phy/thead/phy-th1520-usb.c
> new file mode 100644
> index 0000000000000..802af72353a11
> --- /dev/null
> +++ b/drivers/phy/thead/phy-th1520-usb.c
> @@ -0,0 +1,180 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2026 Institute of Software, Chinese Academy of Sciences (ISCAS)
> + *
> + * Authors:
> + * Icenowy Zheng <zhengxingda@iscas.ac.cn>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>

[ ... ]

> +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?

> +
> +	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723084316.3573714-1-zhengxingda@iscas.ac.cn?part=2

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