Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1461715 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] net: macb: add phy-handle support for the macb David Miller <davem@davemloft.net> - 2016-08-14 11:00 +0200
    Re: [PATCH] net: macb: add phy-handle support for the macb Andrew Lunn <andrew@lunn.ch> - 2016-08-16 22:20 +0200

#1461715 — Re: [PATCH] net: macb: add phy-handle support for the macb

From: Kedareswara rao Appana <appana.durga.rao@xilinx.com>
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.

[toc] | [next] | [standalone]

#1464055

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

What you suggest is better, and is similar to what other drivers use.

In order to keep backwards compatibility with phy nodes in the MAC
node, you have to do of_mdiobus_register() with the MAC node. However,
please change the binding documentation to say this is deprecated, all
phy nodes should be placed inside an mdio node.

Using some better variable names would also help. mac_np and mdio_np
for example.

    Andrew

> 
> 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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web