On 12/05/2013 04:52 PM, Florian Fainelli wrote:
> From: Florian Fainelli <f.fainelli@xxxxxxxxx>
>
> Since commit 779d835e ("net: of_mdio: scan mdiobus for PHYs without reg
> property") we have two foreach loops which do pretty much the same
> thing. Factor the PHY device registration in a function helper:
> of_mdiobus_register_phy() which takes care of the details and allows for
> future PHY specific extensions.
>
> Signed-off-by: Florian Fainelli <f.fainelli@xxxxxxxxx>
A couple of formatting nits, but otherwise:
Acked-by: Rob Herring <rob.herring@xxxxxxxxxxx>
BTW, unless David has objections, I'd like to see this file moved to
drivers/net. He tends to merge the of_mdio.c changes anyway. We've moved
most other subsystem parsing code out of drivers/of.
> ---
> drivers/of/of_mdio.c | 109 ++++++++++++++++++++++-----------------------------
> 1 file changed, 46 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index d5a57a9..82485d2 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -22,6 +22,47 @@
> MODULE_AUTHOR("Grant Likely <grant.likely@xxxxxxxxxxxx>");
> MODULE_LICENSE("GPL");
>
> +static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *child,
> + u32 addr)
> +{
> + struct phy_device *phy;
> + bool is_c45;
> + int rc;
> +
> + is_c45 = of_device_is_compatible(child,
> + "ethernet-phy-ieee802.3-c45");
This looks like it can be 1 line.
> +
> + phy = get_phy_device(mdio, addr, is_c45);
> + if (!phy || IS_ERR(phy))
> + return 1;
> +
> + if (mdio->irq) {
> + mdio->irq[addr] =
> + irq_of_parse_and_map(child, 0);
ditto
> + if (!mdio->irq[addr])
> + mdio->irq[addr] = PHY_POLL;
> + }
> +
> + /* Associate the OF node with the device structure so it
> + * can be looked up later */
> + of_node_get(child);
> + phy->dev.of_node = child;
> +
> + /* All data is now stored in the phy struct;
> + * register it */
> + rc = phy_device_register(phy);
> + if (rc) {
> + phy_device_free(phy);
> + of_node_put(child);
> + return 1;
> + }
> +
> + dev_dbg(&mdio->dev, "registered phy %s at address %i\n",
> + child->name, addr);
> +
> + return 0;
> +}
> +
> /**
> * of_mdiobus_register - Register mii_bus and create PHYs from the device tree
> * @mdio: pointer to mii_bus structure
> @@ -32,11 +73,10 @@ MODULE_LICENSE("GPL");
> */
> int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
> {
> - struct phy_device *phy;
> struct device_node *child;
> const __be32 *paddr;
> u32 addr;
> - bool is_c45, scanphys = false;
> + bool scanphys = false;
> int rc, i, len;
>
> /* Mask out all PHYs from auto probing. Instead the PHYs listed in
> @@ -73,38 +113,9 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
> continue;
> }
>
> - if (mdio->irq) {
> - mdio->irq[addr] = irq_of_parse_and_map(child, 0);
> - if (!mdio->irq[addr])
> - mdio->irq[addr] = PHY_POLL;
> - }
> -
> - is_c45 = of_device_is_compatible(child,
> - "ethernet-phy-ieee802.3-c45");
> - phy = get_phy_device(mdio, addr, is_c45);
> -
> - if (!phy || IS_ERR(phy)) {
> - dev_err(&mdio->dev,
> - "cannot get PHY at address %i\n",
> - addr);
> - continue;
> - }
> -
> - /* Associate the OF node with the device structure so it
> - * can be looked up later */
> - of_node_get(child);
> - phy->dev.of_node = child;
> -
> - /* All data is now stored in the phy struct; register it */
> - rc = phy_device_register(phy);
> - if (rc) {
> - phy_device_free(phy);
> - of_node_put(child);
> + rc = of_mdiobus_register_phy(mdio, child, addr);
> + if (rc)
> continue;
> - }
> -
> - dev_dbg(&mdio->dev, "registered phy %s at address %i\n",
> - child->name, addr);
> }
>
> if (!scanphys)
> @@ -117,9 +128,6 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
> if (paddr)
> continue;
>
> - is_c45 = of_device_is_compatible(child,
> - "ethernet-phy-ieee802.3-c45");
> -
> for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
> /* skip already registered PHYs */
> if (mdio->phy_map[addr])
> @@ -129,34 +137,9 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
> dev_info(&mdio->dev, "scan phy %s at address %i\n",
> child->name, addr);
>
> - phy = get_phy_device(mdio, addr, is_c45);
> - if (!phy || IS_ERR(phy))
> + rc = of_mdiobus_register_phy(mdio, child, addr);
> + if (rc)
> continue;
> -
> - if (mdio->irq) {
> - mdio->irq[addr] =
> - irq_of_parse_and_map(child, 0);
> - if (!mdio->irq[addr])
> - mdio->irq[addr] = PHY_POLL;
> - }
> -
> - /* Associate the OF node with the device structure so it
> - * can be looked up later */
> - of_node_get(child);
> - phy->dev.of_node = child;
> -
> - /* All data is now stored in the phy struct;
> - * register it */
> - rc = phy_device_register(phy);
> - if (rc) {
> - phy_device_free(phy);
> - of_node_put(child);
> - continue;
> - }
> -
> - dev_info(&mdio->dev, "registered phy %s at address %i\n",
> - child->name, addr);
> - break;
> }
> }
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
