Re: [PATCH v7 2/6] PCI: spacemit-k1: Add multiple PHY handles support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] undeclared dependency on non-existent phy_bulk API
--

commit fbbd5a12f65d0fd19aa308617ff68c1880b18ff5
Author: Inochi Amaoto <inochiama@gmail.com>

    PCI: spacemit-k1: Add multiple PHY handles support
    
    The PCIe controller on Spacemit K3 may use multiple PHYs at the
    same time. The feature is not supported by the current driver.
    So extend the PHY definition to support multiple PHY handles.

> diff --git a/drivers/pci/controller/dwc/pcie-spacemit-k1.c b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
> index 0564c46e2f484..8c4f0b09fe24d 100644
> --- a/drivers/pci/controller/dwc/pcie-spacemit-k1.c
> +++ b/drivers/pci/controller/dwc/pcie-spacemit-k1.c
> @@ -45,9 +45,20 @@

[ ... ]

>  struct k1_pcie {
>  	struct dw_pcie pci;
> -	struct phy *phy;
> +	const struct k1_pcie_device_data *data;
> +	struct phy_bulk_data *phys;

[Severity: Low]
Does struct phy_bulk_data exist in the kernel tree?

It looks like this introduces a dependency on a type that hasn't been
defined, which will cause a build failure.

> +	unsigned int phy_count;
>  	void __iomem *link;
>  	struct regmap *pmu;	/* Errors ignored; MMIO-backed regmap */
>  	u32 pmu_off;
> @@ -106,6 +117,25 @@ static void k1_pcie_disable_resources(struct k1_pcie *k1)
>  	clk_bulk_disable_unprepare(ARRAY_SIZE(pci->app_clks), pci->app_clks);
>  }
>  
> +static int k1_pcie_get_phy_handle(struct k1_pcie *k1, struct device_node *node)
> +{
> +	const struct k1_pcie_device_data *data = k1->data;
> +	struct device *dev = k1->pci.dev;
> +	int count;
> +
> +	count = devm_of_phy_bulk_get_all(dev, node, &k1->phys);

[Severity: Low]
Is devm_of_phy_bulk_get_all() implemented anywhere?

It appears to be an undeclared function, which will cause this to fail at
compile time.

> +	if (count < 0)
> +		return count;

[ ... ]

> @@ -161,12 +191,13 @@ static int k1_pcie_init(struct dw_pcie_rp *pp)
>  	 */
>  	regmap_set_bits(k1->pmu, reset_ctrl, DEVICE_TYPE_RC | PCIE_AUX_PWR_DET);
>  
> -	ret = phy_init(k1->phy);
> -	if (ret) {
> -		k1_pcie_disable_resources(k1);
> +	ret = phy_bulk_init(k1->phy_count, k1->phys);

[Severity: Low]
Are the phy_bulk_init() and phy_bulk_power_on() functions available in
the current tree?

Calling non-existent APIs will result in a build breakage.

> +	if (ret)
> +		goto failed_phy_init;
>  
> -		return ret;
> -	}
> +	ret = phy_bulk_power_on(k1->phy_count, k1->phys);
> +	if (ret)
> +		goto failed_phy_power_on;
>  
>  	/* Deassert fundamental reset (drive PERST# high) */
>  	regmap_clear_bits(k1->pmu, reset_ctrl, PCIE_RC_PERST);

[ ... ]

> @@ -186,7 +225,8 @@ static void k1_pcie_deinit(struct dw_pcie_rp *pp)
>  	regmap_set_bits(k1->pmu, k1->pmu_off + PCIE_CLK_RESET_CONTROL,
>  			PCIE_RC_PERST);
>  
> -	phy_exit(k1->phy);
> +	phy_bulk_power_off(k1->phy_count, k1->phys);
> +	phy_bulk_exit(k1->phy_count, k1->phys);

[Severity: Low]
Do phy_bulk_power_off() and phy_bulk_exit() exist?

It appears these functions are missing, which will prevent the driver from
compiling.

>  
>  	k1_pcie_disable_resources(k1);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929083755.458238-1-inochiama@gmail.com?part=2