| Message ID | 1393930704-24374-3-git-send-email-thomas.petazzoni@free-electrons.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
2014-03-04 2:58 GMT-08:00 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>: > The existing fixed_phy_add() function has several drawbacks that > prevents it from being used as is for OF-based declaration of fixed > PHYs: > > * The address of the PHY on the fake bus needs to be passed, while a > dynamic allocation is desired. > > * Since the phy_device instantiation is post-poned until the next > mdiobus scan, there is no way to associate the fixed PHY with its > OF node, which later prevents of_phy_connect() from finding this > fixed PHY from a given OF node. > > To solve this, this commit introduces fixed_phy_register(), which will > allocate an available PHY address, add the PHY using fixed_phy_add() > and instantiate the phy_device structure associated with the provided > OF node. Besides the !CONFIG_FIXED_PHY hiccup (see below), this looks good to me: Acked-by: Florian Fainelli <f.fainelli@gmail.com> > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > drivers/net/phy/fixed.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/phy_fixed.h | 11 +++++++++ > 2 files changed, 72 insertions(+) > > diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c > index 0f02403..82095cc 100644 > --- a/drivers/net/phy/fixed.c > +++ b/drivers/net/phy/fixed.c > @@ -21,6 +21,7 @@ > #include <linux/phy_fixed.h> > #include <linux/err.h> > #include <linux/slab.h> > +#include <linux/of.h> > > #define MII_REGS_NUM 29 > > @@ -203,6 +204,66 @@ err_regs: > } > EXPORT_SYMBOL_GPL(fixed_phy_add); > > +void fixed_phy_del(int phy_addr) > +{ > + struct fixed_mdio_bus *fmb = &platform_fmb; > + struct fixed_phy *fp, *tmp; > + > + list_for_each_entry_safe(fp, tmp, &fmb->phys, node) { > + if (fp->addr == phy_addr) { > + list_del(&fp->node); > + kfree(fp); > + return; > + } > + } > +} > +EXPORT_SYMBOL_GPL(fixed_phy_del); > + > +static int phy_fixed_addr; > +static DEFINE_SPINLOCK(phy_fixed_addr_lock); > + > +int fixed_phy_register(unsigned int irq, > + struct fixed_phy_status *status, > + struct device_node *np) > +{ > + struct fixed_mdio_bus *fmb = &platform_fmb; > + struct phy_device *phy; > + int phy_addr; > + int ret; > + > + /* Get the next available PHY address, up to PHY_MAX_ADDR */ > + spin_lock(&phy_fixed_addr_lock); > + if (phy_fixed_addr == PHY_MAX_ADDR) { > + spin_unlock(&phy_fixed_addr_lock); > + return -ENOSPC; > + } > + phy_addr = phy_fixed_addr++; > + spin_unlock(&phy_fixed_addr_lock); > + > + ret = fixed_phy_add(PHY_POLL, phy_addr, status); > + if (ret < 0) > + return ret; > + > + phy = get_phy_device(fmb->mii_bus, phy_addr, false); > + if (!phy || IS_ERR(phy)) { > + fixed_phy_del(phy_addr); > + return -EINVAL; > + } > + > + of_node_get(np); > + phy->dev.of_node = np; > + > + ret = phy_device_register(phy); > + if (ret) { > + phy_device_free(phy); > + of_node_put(np); > + fixed_phy_del(phy_addr); > + return ret; > + } > + > + return 0; > +} > + > static int __init fixed_mdio_bus_init(void) > { > struct fixed_mdio_bus *fmb = &platform_fmb; > diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h > index 509d8f5..902e8a1 100644 > --- a/include/linux/phy_fixed.h > +++ b/include/linux/phy_fixed.h > @@ -9,15 +9,26 @@ struct fixed_phy_status { > int asym_pause; > }; > > +struct device_node; > + > #ifdef CONFIG_FIXED_PHY > extern int fixed_phy_add(unsigned int irq, int phy_id, > struct fixed_phy_status *status); > +extern int fixed_phy_register(unsigned int irq, > + struct fixed_phy_status *status, > + struct device_node *np); > #else > static inline int fixed_phy_add(unsigned int irq, int phy_id, > struct fixed_phy_status *status) > { > return -ENODEV; > } > +extern int fixed_phy_register(unsigned int irq, > + struct fixed_phy_status *status, > + struct device_node *np) > +{ > + return -ENODEV; > +} s/extern/static/ otherwise I doubt this builds correctly.
On Tue, 4 Mar 2014 11:58:22 +0100, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > The existing fixed_phy_add() function has several drawbacks that > prevents it from being used as is for OF-based declaration of fixed > PHYs: > > * The address of the PHY on the fake bus needs to be passed, while a > dynamic allocation is desired. > > * Since the phy_device instantiation is post-poned until the next > mdiobus scan, there is no way to associate the fixed PHY with its > OF node, which later prevents of_phy_connect() from finding this > fixed PHY from a given OF node. > > To solve this, this commit introduces fixed_phy_register(), which will > allocate an available PHY address, add the PHY using fixed_phy_add() > and instantiate the phy_device structure associated with the provided > OF node. The phy address assignment is a little naive, but that can be fixed in a follow-up patch (see below for a suggestion) Acked-by: Grant Likely <grant.likely@linaro.org> > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > drivers/net/phy/fixed.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/phy_fixed.h | 11 +++++++++ > 2 files changed, 72 insertions(+) > > diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c > index 0f02403..82095cc 100644 > --- a/drivers/net/phy/fixed.c > +++ b/drivers/net/phy/fixed.c > @@ -21,6 +21,7 @@ > #include <linux/phy_fixed.h> > #include <linux/err.h> > #include <linux/slab.h> > +#include <linux/of.h> > > #define MII_REGS_NUM 29 > > @@ -203,6 +204,66 @@ err_regs: > } > EXPORT_SYMBOL_GPL(fixed_phy_add); > > +void fixed_phy_del(int phy_addr) > +{ > + struct fixed_mdio_bus *fmb = &platform_fmb; > + struct fixed_phy *fp, *tmp; > + > + list_for_each_entry_safe(fp, tmp, &fmb->phys, node) { > + if (fp->addr == phy_addr) { > + list_del(&fp->node); > + kfree(fp); > + return; > + } > + } > +} > +EXPORT_SYMBOL_GPL(fixed_phy_del); > + > +static int phy_fixed_addr; > +static DEFINE_SPINLOCK(phy_fixed_addr_lock); > + > +int fixed_phy_register(unsigned int irq, > + struct fixed_phy_status *status, > + struct device_node *np) > +{ > + struct fixed_mdio_bus *fmb = &platform_fmb; > + struct phy_device *phy; > + int phy_addr; > + int ret; > + > + /* Get the next available PHY address, up to PHY_MAX_ADDR */ > + spin_lock(&phy_fixed_addr_lock); > + if (phy_fixed_addr == PHY_MAX_ADDR) { > + spin_unlock(&phy_fixed_addr_lock); > + return -ENOSPC; > + } > + phy_addr = phy_fixed_addr++; If this instead looked over the fixed mdio bus list then an unallocated address could be assigned without the global static. > + spin_unlock(&phy_fixed_addr_lock); > + > + ret = fixed_phy_add(PHY_POLL, phy_addr, status); > + if (ret < 0) > + return ret; > + > + phy = get_phy_device(fmb->mii_bus, phy_addr, false); > + if (!phy || IS_ERR(phy)) { > + fixed_phy_del(phy_addr); > + return -EINVAL; > + } > + > + of_node_get(np); > + phy->dev.of_node = np; > + > + ret = phy_device_register(phy); > + if (ret) { > + phy_device_free(phy); > + of_node_put(np); > + fixed_phy_del(phy_addr); > + return ret; > + } > + > + return 0; > +} > + > static int __init fixed_mdio_bus_init(void) > { > struct fixed_mdio_bus *fmb = &platform_fmb; > diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h > index 509d8f5..902e8a1 100644 > --- a/include/linux/phy_fixed.h > +++ b/include/linux/phy_fixed.h > @@ -9,15 +9,26 @@ struct fixed_phy_status { > int asym_pause; > }; > > +struct device_node; > + > #ifdef CONFIG_FIXED_PHY > extern int fixed_phy_add(unsigned int irq, int phy_id, > struct fixed_phy_status *status); > +extern int fixed_phy_register(unsigned int irq, > + struct fixed_phy_status *status, > + struct device_node *np); > #else > static inline int fixed_phy_add(unsigned int irq, int phy_id, > struct fixed_phy_status *status) > { > return -ENODEV; > } > +extern int fixed_phy_register(unsigned int irq, > + struct fixed_phy_status *status, > + struct device_node *np) > +{ > + return -ENODEV; > +} > #endif /* CONFIG_FIXED_PHY */ > > /* > -- > 1.8.3.2 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/phy/fixed.c b/drivers/net/phy/fixed.c index 0f02403..82095cc 100644 --- a/drivers/net/phy/fixed.c +++ b/drivers/net/phy/fixed.c @@ -21,6 +21,7 @@ #include <linux/phy_fixed.h> #include <linux/err.h> #include <linux/slab.h> +#include <linux/of.h> #define MII_REGS_NUM 29 @@ -203,6 +204,66 @@ err_regs: } EXPORT_SYMBOL_GPL(fixed_phy_add); +void fixed_phy_del(int phy_addr) +{ + struct fixed_mdio_bus *fmb = &platform_fmb; + struct fixed_phy *fp, *tmp; + + list_for_each_entry_safe(fp, tmp, &fmb->phys, node) { + if (fp->addr == phy_addr) { + list_del(&fp->node); + kfree(fp); + return; + } + } +} +EXPORT_SYMBOL_GPL(fixed_phy_del); + +static int phy_fixed_addr; +static DEFINE_SPINLOCK(phy_fixed_addr_lock); + +int fixed_phy_register(unsigned int irq, + struct fixed_phy_status *status, + struct device_node *np) +{ + struct fixed_mdio_bus *fmb = &platform_fmb; + struct phy_device *phy; + int phy_addr; + int ret; + + /* Get the next available PHY address, up to PHY_MAX_ADDR */ + spin_lock(&phy_fixed_addr_lock); + if (phy_fixed_addr == PHY_MAX_ADDR) { + spin_unlock(&phy_fixed_addr_lock); + return -ENOSPC; + } + phy_addr = phy_fixed_addr++; + spin_unlock(&phy_fixed_addr_lock); + + ret = fixed_phy_add(PHY_POLL, phy_addr, status); + if (ret < 0) + return ret; + + phy = get_phy_device(fmb->mii_bus, phy_addr, false); + if (!phy || IS_ERR(phy)) { + fixed_phy_del(phy_addr); + return -EINVAL; + } + + of_node_get(np); + phy->dev.of_node = np; + + ret = phy_device_register(phy); + if (ret) { + phy_device_free(phy); + of_node_put(np); + fixed_phy_del(phy_addr); + return ret; + } + + return 0; +} + static int __init fixed_mdio_bus_init(void) { struct fixed_mdio_bus *fmb = &platform_fmb; diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h index 509d8f5..902e8a1 100644 --- a/include/linux/phy_fixed.h +++ b/include/linux/phy_fixed.h @@ -9,15 +9,26 @@ struct fixed_phy_status { int asym_pause; }; +struct device_node; + #ifdef CONFIG_FIXED_PHY extern int fixed_phy_add(unsigned int irq, int phy_id, struct fixed_phy_status *status); +extern int fixed_phy_register(unsigned int irq, + struct fixed_phy_status *status, + struct device_node *np); #else static inline int fixed_phy_add(unsigned int irq, int phy_id, struct fixed_phy_status *status) { return -ENODEV; } +extern int fixed_phy_register(unsigned int irq, + struct fixed_phy_status *status, + struct device_node *np) +{ + return -ENODEV; +} #endif /* CONFIG_FIXED_PHY */ /*
The existing fixed_phy_add() function has several drawbacks that prevents it from being used as is for OF-based declaration of fixed PHYs: * The address of the PHY on the fake bus needs to be passed, while a dynamic allocation is desired. * Since the phy_device instantiation is post-poned until the next mdiobus scan, there is no way to associate the fixed PHY with its OF node, which later prevents of_phy_connect() from finding this fixed PHY from a given OF node. To solve this, this commit introduces fixed_phy_register(), which will allocate an available PHY address, add the PHY using fixed_phy_add() and instantiate the phy_device structure associated with the provided OF node. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- drivers/net/phy/fixed.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++ include/linux/phy_fixed.h | 11 +++++++++ 2 files changed, 72 insertions(+)