On Thu, Mar 28 2019, Sergio Paracuellos wrote:

> Device tree has been simplified to don't use child nodes and use
> the #phy-cells property instead. Change the driver accordly implementing
> custom 'xlate' function to return the correct phy for each port.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  .../staging/mt7621-pci-phy/pci-mt7621-phy.c   | 26 ++++++++++++++++---
>  1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c b/drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c
> index 98c06308143c..557e6a53fc1e 100644
> --- a/drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c
> +++ b/drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c
> @@ -78,6 +78,8 @@
>  
>  #define RG_PE1_FRC_MSTCKDIV			BIT(5)
>  
> +#define MAX_PHYS	2
> +
>  /**
>   * struct mt7621_pci_phy_instance - Mt7621 Pcie PHY device
>   * @phy: pointer to the kernel PHY device
> @@ -289,6 +291,20 @@ static const struct phy_ops mt7621_pci_phy_ops = {
>  	.owner		= THIS_MODULE,
>  };
>  
> +static struct phy *mt7621_pcie_phy_of_xlate(struct device *dev,
> +					    struct of_phandle_args *args)
> +{
> +	struct mt7621_pci_phy *mt7621_phy = dev_get_drvdata(dev);
> +
> +	if (args->args_count == 0)
> +		return mt7621_phy->phys[0]->phy;
> +
> +	if (WARN_ON(args->args[0] >= MAX_PHYS))
> +		return ERR_PTR(-ENODEV);
> +
> +	return mt7621_phy->phys[args->args[0]]->phy;
> +}
> +
>  static int mt7621_pci_phy_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -299,6 +315,7 @@ static int mt7621_pci_phy_probe(struct platform_device *pdev)
>  	struct resource res;
>  	int port, ret;
>  	void __iomem *port_base;
> +	u32 phy_num;
>  
>  	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
>  	if (!phy)
> @@ -325,8 +342,9 @@ static int mt7621_pci_phy_probe(struct platform_device *pdev)
>  		return PTR_ERR(port_base);
>  	}
>  
> -	port = 0;
> -	for_each_child_of_node(np, child_np) {

This isn't the old place that you depend on the children nodes.
A little earlier, you have

       phy->nphys = of_get_child_count(np);

which now sets nphys to zero.  I changed this to

       phy->nphys = MAX_PHYS;

> +	of_property_read_u32(dev->of_node, "#phy-cells", &phy_num);
> +
> +	for (port = 0; port < phy_num + 1; port++) {

I don't think it is correct to use #phy-cells as the number of phys.
#phy-cells is the number of arguments - the args_count seen in
mt7621_pci_phy_probe.
I think you should either assume phy_num is MAX_PHYS, or have built-in
knowledge of when it is 1 and when it is too.
There is minimal cost to allocating an extra phy, so I would go with
MAX_PHYS.


>  		struct mt7621_pci_phy_instance *instance;
>  		struct phy *pphy;
>  
> @@ -338,7 +356,7 @@ static int mt7621_pci_phy_probe(struct platform_device *pdev)
>  
>  		phy->phys[port] = instance;
>  
> -		pphy = devm_phy_create(dev, child_np, &mt7621_pci_phy_ops);
> +		pphy = devm_phy_create(dev, dev->of_node, &mt7621_pci_phy_ops);
>  		if (IS_ERR(phy)) {
>  			dev_err(dev, "failed to create phy\n");
>  			ret = PTR_ERR(phy);
> @@ -352,7 +370,7 @@ static int mt7621_pci_phy_probe(struct platform_device *pdev)
>  		port++;

This "port++" duplicates the "port++" that you introduced in the "for"
loop above.

Thanks,
NeilBrown

>  	}
>  
> -	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	provider = devm_of_phy_provider_register(dev, mt7621_pcie_phy_of_xlate);
>  
>  	return PTR_ERR_OR_ZERO(provider);
>  
> -- 
> 2.19.1