RE: [PATCH] net: macb: add phy-handle support for the macb

From: Appana Durga Kedareswara Rao
Date: Tue Aug 16 2016 - 03:05:49 EST


Hi David Miller,

Thanks for the review...

>
> From: Kedareswara rao Appana <appana.durga.rao@xxxxxxxxxx>
> Date: Sat, 13 Aug 2016 15:31:49 +0530
>
> > @@ -445,7 +445,13 @@ static int macb_mii_init(struct macb *bp)
> > dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
> >
> > np = bp->pdev->dev.of_node;
> > - if (np) {
> > + np1 = of_get_parent(bp->phy_node);
> > + if (np1) {
> > + of_node_put(np1);
> > + err = of_mdiobus_register(bp->mii_bus, np1);
> > + if (err)
> > + goto err_out_unregister_bus;
> > + } else if (np) {
>
> I don't know about this, all OF nodes other than the root have a parent node
> which is non-NULL. This parent node can be anything.
>
> Just blinding assuming that any parent node of the phy_node is what we are
> looking for, without any other supplementary checks at all, seems to be asking
> for trouble at the very least.

Agree with you my intention is if there is a MDIO bus on the device-tree
The MAC driver should create PHY/MDIO devices using of_mdiobus_register().

How about below code...

- struct device_node *np;
+ struct device_node *np, *np1;
int err = -ENXIO, i;

/* Enable management port */
@@ -445,7 +445,14 @@ static int macb_mii_init(struct macb *bp)
dev_set_drvdata(&bp->dev->dev, bp->mii_bus);

np = bp->pdev->dev.of_node;
- if (np) {
+ np1 = of_get_child_by_name(np, "mdio");
+ if (np1) {
+ of_node_put(np1);
+ err = of_mdiobus_register(bp->mii_bus, np1);
+ if (err)
+ goto err_out_unregister_bus;
+ } else if (np) {
/* try dt phy registration */
err = of_mdiobus_register(bp->mii_bus, np);

If you are ok with the above code please let me know will post it as v2...

Regards,
Kedar.