Re: [RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices
Hi Andrew,
On Mon, Nov 28, 2016 at 10:03 PM, Andrew Lunn wrote:
> On Mon, Nov 28, 2016 at 03:19:14PM +0530, Harini Katakam wrote:
>> This patch is to add support for the hardware with multiple ethernet
>> MAC controllers and a single MDIO bus connected to multiple PHY devices.
>> MDIO lines are connected to any one of the ethernet MAC controllers and
>> all the PHY devices will be accessed using the PHY maintenance interface
>> in that MAC controller. This handling along with PHY functionality is
>> moved to macb_mdio.c
>>
>> Signed-off-by: Punnaiah Choudary Kalluri
>> Signed-off-by: Harini Katakam
>> ---
>> drivers/net/ethernet/cadence/Makefile| 2 +-
>> drivers/net/ethernet/cadence/macb.c | 169 +++-
>> drivers/net/ethernet/cadence/macb.h | 2 +
>> drivers/net/ethernet/cadence/macb_mdio.c | 266
>> +++
>> 4 files changed, 294 insertions(+), 145 deletions(-)
>> create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
>>
>> + bus->irq = devm_kzalloc(&pdev->dev, sizeof(int) * PHY_MAX_ADDR,
>> + GFP_KERNEL);
>
> This looks wrong, or at least old. It used to be a pointer to an array,
> but it is now an actual array.
Sorry, this was a mistake.
I changed this after rebase, will update in next version.
>
>> +static const struct of_device_id macb_mdio_dt_ids[] = {
>> + { .compatible = "cdns,macb-mdio" },
>> +
>> +};
>
>
> I've not looked hard enough to know, but can you keep backwards
> compatibility? Won't old device tree's assume the mdio bus is always
> present? Now you need an explicit node otherwise there will not be an
> mdio bus?
Yes, an explicit MDIO bus is required. But I'm not sure
how to maintain backward compatibility (without using this separate
macb_mdio) and have different MACs use the same MDIO bus
with separate PHYs.
Regards,
Harini
Re: [RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices
On Mon, Nov 28, 2016 at 03:19:14PM +0530, Harini Katakam wrote:
> This patch is to add support for the hardware with multiple ethernet
> MAC controllers and a single MDIO bus connected to multiple PHY devices.
> MDIO lines are connected to any one of the ethernet MAC controllers and
> all the PHY devices will be accessed using the PHY maintenance interface
> in that MAC controller. This handling along with PHY functionality is
> moved to macb_mdio.c
>
> Signed-off-by: Punnaiah Choudary Kalluri
> Signed-off-by: Harini Katakam
> ---
> drivers/net/ethernet/cadence/Makefile| 2 +-
> drivers/net/ethernet/cadence/macb.c | 169 +++-
> drivers/net/ethernet/cadence/macb.h | 2 +
> drivers/net/ethernet/cadence/macb_mdio.c | 266
> +++
> 4 files changed, 294 insertions(+), 145 deletions(-)
> create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
>
> diff --git a/drivers/net/ethernet/cadence/Makefile
> b/drivers/net/ethernet/cadence/Makefile
> index 91f79b1..75c3d84 100644
> --- a/drivers/net/ethernet/cadence/Makefile
> +++ b/drivers/net/ethernet/cadence/Makefile
> @@ -2,4 +2,4 @@
> # Makefile for the Atmel network device drivers.
> #
>
> -obj-$(CONFIG_MACB) += macb.o
> +obj-$(CONFIG_MACB) += macb.o macb_mdio.o
> diff --git a/drivers/net/ethernet/cadence/macb.c
> b/drivers/net/ethernet/cadence/macb.c
> index 80ccfc4..ae2a797 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -232,45 +232,6 @@ static void macb_get_hwaddr(struct macb *bp)
> eth_hw_addr_random(bp->dev);
> }
>
> -static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
> -{
> - struct macb *bp = bus->priv;
> - int value;
> -
> - macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
> - | MACB_BF(RW, MACB_MAN_READ)
> - | MACB_BF(PHYA, mii_id)
> - | MACB_BF(REGA, regnum)
> - | MACB_BF(CODE, MACB_MAN_CODE)));
> -
> - /* wait for end of transfer */
> - while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
> - cpu_relax();
> -
> - value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
> -
> - return value;
> -}
> -
> -static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
> -u16 value)
> -{
> - struct macb *bp = bus->priv;
> -
> - macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
> - | MACB_BF(RW, MACB_MAN_WRITE)
> - | MACB_BF(PHYA, mii_id)
> - | MACB_BF(REGA, regnum)
> - | MACB_BF(CODE, MACB_MAN_CODE)
> - | MACB_BF(DATA, value)));
> -
> - /* wait for end of transfer */
> - while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
> - cpu_relax();
> -
> - return 0;
> -}
> -
> /**
> * macb_set_tx_clk() - Set a clock to a new frequency
> * @clk Pointer to the clock to change
> @@ -385,27 +346,19 @@ static void macb_handle_link_change(struct net_device
> *dev)
> static int macb_mii_probe(struct net_device *dev)
> {
> struct macb *bp = netdev_priv(dev);
> - struct macb_platform_data *pdata;
> struct phy_device *phydev;
> - int phy_irq;
> int ret;
>
> - phydev = phy_find_first(bp->mii_bus);
> + if (dev->phydev)
> + return 0;
> +
> + phydev = of_phy_find_device(bp->phy_node);
> if (!phydev) {
> netdev_err(dev, "no PHY found\n");
> return -ENXIO;
> }
> -
> - pdata = dev_get_platdata(&bp->pdev->dev);
> - if (pdata && gpio_is_valid(pdata->phy_irq_pin)) {
> - ret = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin,
> - "phy int");
> - if (!ret) {
> - phy_irq = gpio_to_irq(pdata->phy_irq_pin);
> - phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
> - }
> - }
> -
> + if (bp->phy_irq)
> + phydev->irq = bp->phy_irq;
> /* attach the mac to the phy */
> ret = phy_connect_direct(dev, phydev, &macb_handle_link_change,
>bp->phy_interface);
> @@ -429,80 +382,9 @@ static int macb_mii_probe(struct net_device *dev)
> bp->speed = 0;
> bp->duplex = -1;
>
> - return 0;
> -}
> -
> -static int macb_mii_init(struct macb *bp)
> -{
> - struct macb_platform_data *pdata;
> - struct device_node *np;
> - int err = -ENXIO, i;
> -
> - /* Enable management port */
> - macb_writel(bp, NCR, MACB_BIT(MPE));
> -
> - bp->mii_bus = mdiobus_alloc();
> - if (!bp->mii_bus) {
> - err = -ENOMEM;
> - goto err_out;
> - }
> -
> - bp->mii_bus->name = "MACB_mii_bus";
> - bp->mii_bus->read = &macb_mdio_read;
> - bp->mii_bus->write = &macb_mdio_write;
> - snprintf(bp->mii_bu
[RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices
This patch is to add support for the hardware with multiple ethernet
MAC controllers and a single MDIO bus connected to multiple PHY devices.
MDIO lines are connected to any one of the ethernet MAC controllers and
all the PHY devices will be accessed using the PHY maintenance interface
in that MAC controller. This handling along with PHY functionality is
moved to macb_mdio.c
Signed-off-by: Punnaiah Choudary Kalluri
Signed-off-by: Harini Katakam
---
drivers/net/ethernet/cadence/Makefile| 2 +-
drivers/net/ethernet/cadence/macb.c | 169 +++-
drivers/net/ethernet/cadence/macb.h | 2 +
drivers/net/ethernet/cadence/macb_mdio.c | 266 +++
4 files changed, 294 insertions(+), 145 deletions(-)
create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
diff --git a/drivers/net/ethernet/cadence/Makefile
b/drivers/net/ethernet/cadence/Makefile
index 91f79b1..75c3d84 100644
--- a/drivers/net/ethernet/cadence/Makefile
+++ b/drivers/net/ethernet/cadence/Makefile
@@ -2,4 +2,4 @@
# Makefile for the Atmel network device drivers.
#
-obj-$(CONFIG_MACB) += macb.o
+obj-$(CONFIG_MACB) += macb.o macb_mdio.o
diff --git a/drivers/net/ethernet/cadence/macb.c
b/drivers/net/ethernet/cadence/macb.c
index 80ccfc4..ae2a797 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -232,45 +232,6 @@ static void macb_get_hwaddr(struct macb *bp)
eth_hw_addr_random(bp->dev);
}
-static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
-{
- struct macb *bp = bus->priv;
- int value;
-
- macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
- | MACB_BF(RW, MACB_MAN_READ)
- | MACB_BF(PHYA, mii_id)
- | MACB_BF(REGA, regnum)
- | MACB_BF(CODE, MACB_MAN_CODE)));
-
- /* wait for end of transfer */
- while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
- cpu_relax();
-
- value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
-
- return value;
-}
-
-static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
- u16 value)
-{
- struct macb *bp = bus->priv;
-
- macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
- | MACB_BF(RW, MACB_MAN_WRITE)
- | MACB_BF(PHYA, mii_id)
- | MACB_BF(REGA, regnum)
- | MACB_BF(CODE, MACB_MAN_CODE)
- | MACB_BF(DATA, value)));
-
- /* wait for end of transfer */
- while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
- cpu_relax();
-
- return 0;
-}
-
/**
* macb_set_tx_clk() - Set a clock to a new frequency
* @clkPointer to the clock to change
@@ -385,27 +346,19 @@ static void macb_handle_link_change(struct net_device
*dev)
static int macb_mii_probe(struct net_device *dev)
{
struct macb *bp = netdev_priv(dev);
- struct macb_platform_data *pdata;
struct phy_device *phydev;
- int phy_irq;
int ret;
- phydev = phy_find_first(bp->mii_bus);
+ if (dev->phydev)
+ return 0;
+
+ phydev = of_phy_find_device(bp->phy_node);
if (!phydev) {
netdev_err(dev, "no PHY found\n");
return -ENXIO;
}
-
- pdata = dev_get_platdata(&bp->pdev->dev);
- if (pdata && gpio_is_valid(pdata->phy_irq_pin)) {
- ret = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin,
- "phy int");
- if (!ret) {
- phy_irq = gpio_to_irq(pdata->phy_irq_pin);
- phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
- }
- }
-
+ if (bp->phy_irq)
+ phydev->irq = bp->phy_irq;
/* attach the mac to the phy */
ret = phy_connect_direct(dev, phydev, &macb_handle_link_change,
bp->phy_interface);
@@ -429,80 +382,9 @@ static int macb_mii_probe(struct net_device *dev)
bp->speed = 0;
bp->duplex = -1;
- return 0;
-}
-
-static int macb_mii_init(struct macb *bp)
-{
- struct macb_platform_data *pdata;
- struct device_node *np;
- int err = -ENXIO, i;
-
- /* Enable management port */
- macb_writel(bp, NCR, MACB_BIT(MPE));
-
- bp->mii_bus = mdiobus_alloc();
- if (!bp->mii_bus) {
- err = -ENOMEM;
- goto err_out;
- }
-
- bp->mii_bus->name = "MACB_mii_bus";
- bp->mii_bus->read = &macb_mdio_read;
- bp->mii_bus->write = &macb_mdio_write;
- snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
-bp->pdev->name, bp->pdev->id);
- bp->mii_bus->priv = bp;
- bp->mii_bus->parent = &bp->pdev->dev;
- pdata = dev_get_platdata(&bp->pdev->dev)
[RFC PATCH 1/2] net: macb: Add MDIO driver for accessing multiple PHY devices
This patch is to add support for the hardware with multiple ethernet
MAC controllers and a single MDIO bus connected to multiple PHY devices.
MDIO lines are connected to any one of the ethernet MAC controllers and
all the PHY devices will be accessed using the PHY maintenance interface
in that MAC controller. This handling along with PHY functionality is
moved to macb_mdio.c
Signed-off-by: Punnaiah Choudary Kalluri
Signed-off-by: Harini Katakam
---
drivers/net/ethernet/cadence/Makefile|2 +-
drivers/net/ethernet/cadence/macb.c | 183
drivers/net/ethernet/cadence/macb.h |2 +
drivers/net/ethernet/cadence/macb_mdio.c | 266 ++
4 files changed, 302 insertions(+), 151 deletions(-)
create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
diff --git a/drivers/net/ethernet/cadence/Makefile
b/drivers/net/ethernet/cadence/Makefile
index 91f79b1..75c3d84 100644
--- a/drivers/net/ethernet/cadence/Makefile
+++ b/drivers/net/ethernet/cadence/Makefile
@@ -2,4 +2,4 @@
# Makefile for the Atmel network device drivers.
#
-obj-$(CONFIG_MACB) += macb.o
+obj-$(CONFIG_MACB) += macb.o macb_mdio.o
diff --git a/drivers/net/ethernet/cadence/macb.c
b/drivers/net/ethernet/cadence/macb.c
index cb07d95..6e8ff4e 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -218,45 +218,6 @@ static void macb_get_hwaddr(struct macb *bp)
eth_hw_addr_random(bp->dev);
}
-static int macb_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
-{
- struct macb *bp = bus->priv;
- int value;
-
- macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
- | MACB_BF(RW, MACB_MAN_READ)
- | MACB_BF(PHYA, mii_id)
- | MACB_BF(REGA, regnum)
- | MACB_BF(CODE, MACB_MAN_CODE)));
-
- /* wait for end of transfer */
- while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
- cpu_relax();
-
- value = MACB_BFEXT(DATA, macb_readl(bp, MAN));
-
- return value;
-}
-
-static int macb_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
- u16 value)
-{
- struct macb *bp = bus->priv;
-
- macb_writel(bp, MAN, (MACB_BF(SOF, MACB_MAN_SOF)
- | MACB_BF(RW, MACB_MAN_WRITE)
- | MACB_BF(PHYA, mii_id)
- | MACB_BF(REGA, regnum)
- | MACB_BF(CODE, MACB_MAN_CODE)
- | MACB_BF(DATA, value)));
-
- /* wait for end of transfer */
- while (!MACB_BFEXT(IDLE, macb_readl(bp, NSR)))
- cpu_relax();
-
- return 0;
-}
-
/**
* macb_set_tx_clk() - Set a clock to a new frequency
* @clkPointer to the clock to change
@@ -371,27 +332,19 @@ static void macb_handle_link_change(struct net_device
*dev)
static int macb_mii_probe(struct net_device *dev)
{
struct macb *bp = netdev_priv(dev);
- struct macb_platform_data *pdata;
struct phy_device *phydev;
- int phy_irq;
int ret;
- phydev = phy_find_first(bp->mii_bus);
+ if (bp->phy_dev)
+ return 0;
+
+ phydev = of_phy_find_device(bp->phy_node);
if (!phydev) {
netdev_err(dev, "no PHY found\n");
return -ENXIO;
}
-
- pdata = dev_get_platdata(&bp->pdev->dev);
- if (pdata && gpio_is_valid(pdata->phy_irq_pin)) {
- ret = devm_gpio_request(&bp->pdev->dev, pdata->phy_irq_pin,
- "phy int");
- if (!ret) {
- phy_irq = gpio_to_irq(pdata->phy_irq_pin);
- phydev->irq = (phy_irq < 0) ? PHY_POLL : phy_irq;
- }
- }
-
+ if (bp->phy_irq)
+ phydev->irq = bp->phy_irq;
/* attach the mac to the phy */
ret = phy_connect_direct(dev, phydev, &macb_handle_link_change,
bp->phy_interface);
@@ -416,80 +369,9 @@ static int macb_mii_probe(struct net_device *dev)
bp->duplex = -1;
bp->phy_dev = phydev;
- return 0;
-}
-
-static int macb_mii_init(struct macb *bp)
-{
- struct macb_platform_data *pdata;
- struct device_node *np;
- int err = -ENXIO, i;
-
- /* Enable management port */
- macb_writel(bp, NCR, MACB_BIT(MPE));
-
- bp->mii_bus = mdiobus_alloc();
- if (!bp->mii_bus) {
- err = -ENOMEM;
- goto err_out;
- }
-
- bp->mii_bus->name = "MACB_mii_bus";
- bp->mii_bus->read = &macb_mdio_read;
- bp->mii_bus->write = &macb_mdio_write;
- snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
-bp->pdev->name, bp->pdev->id);
- bp->mii_bus->priv = bp;
- bp->mii_bus->parent = &bp->pdev->dev;
- pdata = dev_get_platdata(&bp->
[RFC PATCH 1/2] net: macb: Add mdio driver for accessing multiple phy devices
This patch is to add spoort for the design that has multiple ethernet
mac controllers and single mdio bus connected to multiple phy devices.
i.e mdio lines are connected to any of the ethernet mac controller and
all the phy devices will be accessed using the phy maintainance interface
in that mac controller.
Signed-off-by: Punnaiah Choudary Kalluri
---
drivers/net/ethernet/cadence/Makefile|2 +-
drivers/net/ethernet/cadence/macb.c | 93 +-
drivers/net/ethernet/cadence/macb.h |3 +-
drivers/net/ethernet/cadence/macb_mdio.c | 204 ++
4 files changed, 211 insertions(+), 91 deletions(-)
create mode 100644 drivers/net/ethernet/cadence/macb_mdio.c
diff --git a/drivers/net/ethernet/cadence/Makefile
b/drivers/net/ethernet/cadence/Makefile
index 9068b83..73504f4 100644
--- a/drivers/net/ethernet/cadence/Makefile
+++ b/drivers/net/ethernet/cadence/Makefile
@@ -3,4 +3,4 @@
#
obj-$(CONFIG_ARM_AT91_ETHER) += at91_ether.o
-obj-$(CONFIG_MACB) += macb.o
+obj-$(CONFIG_MACB) += macb.o macb_mdio.o
diff --git a/drivers/net/ethernet/cadence/macb.c
b/drivers/net/ethernet/cadence/macb.c
index 4833ba1..df1b928 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -320,7 +320,7 @@ static int macb_mii_probe(struct net_device *dev)
int phy_irq;
int ret;
- phydev = phy_find_first(bp->mii_bus);
+ phydev = of_phy_find_device(bp->phy_node);
if (!phydev) {
netdev_err(dev, "no PHY found\n");
return -ENXIO;
@@ -359,89 +359,6 @@ static int macb_mii_probe(struct net_device *dev)
return 0;
}
-int macb_mii_init(struct macb *bp)
-{
- struct macb_platform_data *pdata;
- struct device_node *np;
- int err = -ENXIO, i;
-
- /* Enable management port */
- macb_writel(bp, NCR, MACB_BIT(MPE));
-
- bp->mii_bus = mdiobus_alloc();
- if (bp->mii_bus == NULL) {
- err = -ENOMEM;
- goto err_out;
- }
-
- bp->mii_bus->name = "MACB_mii_bus";
- bp->mii_bus->read = &macb_mdio_read;
- bp->mii_bus->write = &macb_mdio_write;
- snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
- bp->pdev->name, bp->pdev->id);
- bp->mii_bus->priv = bp;
- bp->mii_bus->parent = &bp->dev->dev;
- pdata = dev_get_platdata(&bp->pdev->dev);
-
- bp->mii_bus->irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);
- if (!bp->mii_bus->irq) {
- err = -ENOMEM;
- goto err_out_free_mdiobus;
- }
-
- dev_set_drvdata(&bp->dev->dev, bp->mii_bus);
-
- np = bp->pdev->dev.of_node;
- if (np) {
- /* try dt phy registration */
- err = of_mdiobus_register(bp->mii_bus, np);
-
- /* fallback to standard phy registration if no phy were
- found during dt phy registration */
- if (!err && !phy_find_first(bp->mii_bus)) {
- for (i = 0; i < PHY_MAX_ADDR; i++) {
- struct phy_device *phydev;
-
- phydev = mdiobus_scan(bp->mii_bus, i);
- if (IS_ERR(phydev)) {
- err = PTR_ERR(phydev);
- break;
- }
- }
-
- if (err)
- goto err_out_unregister_bus;
- }
- } else {
- for (i = 0; i < PHY_MAX_ADDR; i++)
- bp->mii_bus->irq[i] = PHY_POLL;
-
- if (pdata)
- bp->mii_bus->phy_mask = pdata->phy_mask;
-
- err = mdiobus_register(bp->mii_bus);
- }
-
- if (err)
- goto err_out_free_mdio_irq;
-
- err = macb_mii_probe(bp->dev);
- if (err)
- goto err_out_unregister_bus;
-
- return 0;
-
-err_out_unregister_bus:
- mdiobus_unregister(bp->mii_bus);
-err_out_free_mdio_irq:
- kfree(bp->mii_bus->irq);
-err_out_free_mdiobus:
- mdiobus_free(bp->mii_bus);
-err_out:
- return err;
-}
-EXPORT_SYMBOL_GPL(macb_mii_init);
-
static void macb_update_stats(struct macb *bp)
{
u32 __iomem *reg = bp->regs + MACB_PFR;
@@ -2480,7 +2397,10 @@ static int macb_probe(struct platform_device *pdev)
goto err_out_free_netdev;
}
- err = macb_mii_init(bp);
+ bp->phy_node = of_parse_phandle(bp->pdev->dev.of_node,
+ "phy-handle", 0);
+
+ err = macb_mii_probe(bp->dev);
if (err)
goto err_out_unregister_netdev;
@@ -2524,9 +2444,6 @@ static int macb_remove(struct platform_device *pdev)
bp = netdev_priv(dev);
if (bp->phy_dev)
phy_disconnect(bp->phy_dev);
- mdiobus_unr
