Hi Dan,
On Fri, Mar 29, 2019 at 8:54 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> On Thu, Mar 28, 2019 at 06:55:31PM +0100, Sergio Paracuellos wrote:
> > 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) {
> > + of_property_read_u32(dev->of_node, "#phy-cells", &phy_num);
> > +
> > + for (port = 0; port < phy_num + 1; port++) {
> ^^^^^^^^^^^^^^^^^^ ^^^^^^
> > 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++;
> ^^^^^^
> > }
>
> Incrementing "port" twice is probably wrong... Or anyway, less readable
> than "port += 2".
Yes, that was a mistake in my code becase I did not delete it when the
loop was changed.
>
> To be honest, I don't know anything about device tree. Does "phy_num"
> come from the device tree stuff that you just changed in the ealier
> patches? (I never read those so I never learn anything about device
> tree so I am stuck in an endless doom cycle).
The first approach in v1 was to read this from #phy-cell property from
device tree. Neil points me
out this was not the correct approach and was changed to a fixed
MAX_PHYS for both phy's in v2 patches.
>
> Anyway, I am a real newbie. Where does the plus one in
> "port < phy_num + 1" come from? How do I verify that phy_num is less
> than phy->nphys?
In the same way, this is now a fixed port < MAX_PHYS in for loop sent
in v2 of the patches.
>
> regards,
> dan carpenter
>
Best regards,
Sergio Paracuellos
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
