2013-03-14 18:20:28

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH 0/4] mv643xx_eth: use mvmdio MDIO bus driver

Hi all,

This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
instead of rolling its own implementation. As a result, all users of this
mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
The mvmdio driver is also updated to support an interrupt line which reports
SMI error/completion, and to allow traditionnal platform device registration
instead of just device tree.

David, I think it makes sense for you to merge all of this, since we do
not want the architecture files to be desynchronized from the mv643xx_eth to
avoid runtime breakage. The potential for merge conflicts should be very small.

Florian Fainelli (4):
net: mvmdio: allow platform device style registration
net: mvmdio: rename base register cookie from smireg to regs
net: mvmdio: enhance driver to support SMI error/done interrupts
mv643xx_eth: convert to use the Marvell Orion MDIO driver

.../devicetree/bindings/net/marvell-orion-mdio.txt | 3 +
arch/arm/plat-orion/common.c | 97 +++++++---
arch/powerpc/platforms/chrp/pegasos_eth.c | 20 +++
arch/powerpc/sysdev/mv64x60_dev.c | 14 +-
drivers/net/ethernet/marvell/Kconfig | 1 +
drivers/net/ethernet/marvell/mv643xx_eth.c | 186 +-------------------
drivers/net/ethernet/marvell/mvmdio.c | 111 +++++++++---
include/linux/mv643xx_eth.h | 1 -
8 files changed, 207 insertions(+), 226 deletions(-)

--
1.7.10.4


2013-03-14 18:20:23

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH 2/4 v2] net: mvmdio: rename base register cookie from smireg to regs

This patch renames the base register cookie in the mvmdio drive from
"smireg" to "regs" since a subsequent patch is going to use an ioremap()
cookie whose size is larger than a single register of 4 bytes. No
functionnal code change introduced.

Acked-by: Thomas Petazzoni <[email protected]>
Signed-off-by: Florian Fainelli <[email protected]>
---
Changes since v1:
- added Thomas Acked-by tag

drivers/net/ethernet/marvell/mvmdio.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 8a182f1..595deea 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -38,7 +38,7 @@

struct orion_mdio_dev {
struct mutex lock;
- void __iomem *smireg;
+ void __iomem *regs;
};

/* Wait for the SMI unit to be ready for another operation
@@ -51,7 +51,7 @@ static int orion_mdio_wait_ready(struct mii_bus *bus)

count = 0;
while (1) {
- val = readl(dev->smireg);
+ val = readl(dev->regs);
if (!(val & MVMDIO_SMI_BUSY))
break;

@@ -86,12 +86,12 @@ static int orion_mdio_read(struct mii_bus *bus, int mii_id,
writel(((mii_id << MVMDIO_SMI_PHY_ADDR_SHIFT) |
(regnum << MVMDIO_SMI_PHY_REG_SHIFT) |
MVMDIO_SMI_READ_OPERATION),
- dev->smireg);
+ dev->regs);

/* Wait for the value to become available */
count = 0;
while (1) {
- val = readl(dev->smireg);
+ val = readl(dev->regs);
if (val & MVMDIO_SMI_READ_VALID)
break;

@@ -128,7 +128,7 @@ static int orion_mdio_write(struct mii_bus *bus, int mii_id,
(regnum << MVMDIO_SMI_PHY_REG_SHIFT) |
MVMDIO_SMI_WRITE_OPERATION |
(value << MVMDIO_SMI_DATA_SHIFT)),
- dev->smireg);
+ dev->regs);

mutex_unlock(&dev->lock);

@@ -177,8 +177,8 @@ static int orion_mdio_probe(struct platform_device *pdev)
bus->irq[i] = PHY_POLL;

dev = bus->priv;
- dev->smireg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
- if (!dev->smireg) {
+ dev->regs = devm_ioremap(&pdev->dev, r->start, resource_size(r));
+ if (!dev->regs) {
dev_err(&pdev->dev, "Unable to remap SMI register\n");
kfree(bus->irq);
mdiobus_free(bus);
--
1.7.10.4

2013-03-14 18:20:40

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH 4/4 v2] mv643xx_eth: convert to use the Marvell Orion MDIO driver

This patch converts the Marvell MV643XX ethernet driver to use the
Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
registering the Marvell MV643XX ethernet driver are also updated to
register a Marvell Orion MDIO driver. This driver voluntarily overlaps
with the Marvell Ethernet shared registers because it will use a subset
of this shared register (shared_base + 0x4 - shared_base + 0x84). The
Ethernet driver is also updated to look up for a PHY device using the
Orion MDIO bus driver.

Signed-off-by: Florian Fainelli <[email protected]>
---
Changes since v1:
- removed one too many mdio bus registration for Orion5x/Kirkwood

arch/arm/plat-orion/common.c | 97 +++++++++++----
arch/powerpc/platforms/chrp/pegasos_eth.c | 20 +++
arch/powerpc/sysdev/mv64x60_dev.c | 14 ++-
drivers/net/ethernet/marvell/Kconfig | 1 +
drivers/net/ethernet/marvell/mv643xx_eth.c | 186 ++--------------------------
include/linux/mv643xx_eth.h | 1 -
6 files changed, 117 insertions(+), 202 deletions(-)

diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index 2d4b641..dc4345e 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -238,6 +238,7 @@ static __init void ge_complete(
struct mv643xx_eth_shared_platform_data *orion_ge_shared_data,
struct resource *orion_ge_resource, unsigned long irq,
struct platform_device *orion_ge_shared,
+ struct platform_device *orion_ge_mvmdio,
struct mv643xx_eth_platform_data *eth_data,
struct platform_device *orion_ge)
{
@@ -247,6 +248,8 @@ static __init void ge_complete(
orion_ge->dev.platform_data = eth_data;

platform_device_register(orion_ge_shared);
+ if (orion_ge_mvmdio)
+ platform_device_register(orion_ge_mvmdio);
platform_device_register(orion_ge);
}

@@ -258,8 +261,6 @@ struct mv643xx_eth_shared_platform_data orion_ge00_shared_data;
static struct resource orion_ge00_shared_resources[] = {
{
.name = "ge00 base",
- }, {
- .name = "ge00 err irq",
},
};

@@ -271,6 +272,19 @@ static struct platform_device orion_ge00_shared = {
},
};

+static struct resource orion_ge00_mvmdio_resources[] = {
+ {
+ .name = "ge00 mvmdio base",
+ }, {
+ .name = "ge00 mvmdio irr irq",
+ },
+};
+
+static struct platform_device orion_ge00_mvmdio = {
+ .name = "orion-mdio",
+ .id = 0,
+};
+
static struct resource orion_ge00_resources[] = {
{
.name = "ge00 irq",
@@ -295,26 +309,25 @@ void __init orion_ge00_init(struct mv643xx_eth_platform_data *eth_data,
unsigned int tx_csum_limit)
{
fill_resources(&orion_ge00_shared, orion_ge00_shared_resources,
- mapbase + 0x2000, SZ_16K - 1, irq_err);
+ mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+ fill_resources(&orion_ge00_mvmdio, orion_ge00_mvmdio_resources,
+ mapbase + 0x2004, 0x84 - 1, irq_err);
orion_ge00_shared_data.tx_csum_limit = tx_csum_limit;
ge_complete(&orion_ge00_shared_data,
orion_ge00_resources, irq, &orion_ge00_shared,
+ &orion_ge00_mvmdio,
eth_data, &orion_ge00);
}

/*****************************************************************************
* GE01
****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge01_shared_data = {
- .shared_smi = &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge01_shared_data;

static struct resource orion_ge01_shared_resources[] = {
{
.name = "ge01 base",
- }, {
- .name = "ge01 err irq",
- },
+ }
};

static struct platform_device orion_ge01_shared = {
@@ -325,6 +338,19 @@ static struct platform_device orion_ge01_shared = {
},
};

+static struct resource orion_ge01_mvmdio_resources[] = {
+ {
+ .name = "ge01 mdio base",
+ }, {
+ .name = "ge01 mdio err irq",
+ },
+};
+
+static struct platform_device orion_ge01_mvmdio = {
+ .name = "orion-mdio",
+ .id = 1,
+};
+
static struct resource orion_ge01_resources[] = {
{
.name = "ge01 irq",
@@ -349,26 +375,25 @@ void __init orion_ge01_init(struct mv643xx_eth_platform_data *eth_data,
unsigned int tx_csum_limit)
{
fill_resources(&orion_ge01_shared, orion_ge01_shared_resources,
- mapbase + 0x2000, SZ_16K - 1, irq_err);
+ mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+ fill_resources(&orion_ge01_mvmdio, orion_ge01_mvmdio_resources,
+ mapbase + 0x2004, 0x84 - 1, irq_err);
orion_ge01_shared_data.tx_csum_limit = tx_csum_limit;
ge_complete(&orion_ge01_shared_data,
orion_ge01_resources, irq, &orion_ge01_shared,
+ NULL,
eth_data, &orion_ge01);
}

/*****************************************************************************
* GE10
****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge10_shared_data = {
- .shared_smi = &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge10_shared_data;

static struct resource orion_ge10_shared_resources[] = {
{
.name = "ge10 base",
- }, {
- .name = "ge10 err irq",
- },
+ }
};

static struct platform_device orion_ge10_shared = {
@@ -379,6 +404,19 @@ static struct platform_device orion_ge10_shared = {
},
};

+static struct resource orion_ge10_mvmdio_resources[] = {
+ {
+ .name = "ge10 mvmdio base",
+ }, {
+ .name = "ge10 mvmdio err irq",
+ },
+};
+
+static struct platform_device orion_ge10_mvmdio = {
+ .name = "orion-mdio",
+ .id = 1,
+};
+
static struct resource orion_ge10_resources[] = {
{
.name = "ge10 irq",
@@ -403,23 +441,22 @@ void __init orion_ge10_init(struct mv643xx_eth_platform_data *eth_data,
{
fill_resources(&orion_ge10_shared, orion_ge10_shared_resources,
mapbase + 0x2000, SZ_16K - 1, irq_err);
+ fill_resources(&orion_ge10_mvmdio, orion_ge10_mvmdio_resources,
+ mapbase + 0x2004, 0x84 - 1, irq_err);
ge_complete(&orion_ge10_shared_data,
orion_ge10_resources, irq, &orion_ge10_shared,
+ &orion_ge10_mvmdio,
eth_data, &orion_ge10);
}

/*****************************************************************************
* GE11
****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge11_shared_data = {
- .shared_smi = &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge11_shared_data;

static struct resource orion_ge11_shared_resources[] = {
{
.name = "ge11 base",
- }, {
- .name = "ge11 err irq",
},
};

@@ -431,6 +468,19 @@ static struct platform_device orion_ge11_shared = {
},
};

+static struct resource orion_ge11_mvmdio_resources[] = {
+ {
+ .name = "ge11 mvmdio base",
+ }, {
+ .name = "ge11 mvmdio err irq",
+ },
+};
+
+static struct platform_device orion_ge11_mvmdio = {
+ .name = "orion-mdio",
+ .id = 1,
+};
+
static struct resource orion_ge11_resources[] = {
{
.name = "ge11 irq",
@@ -454,9 +504,12 @@ void __init orion_ge11_init(struct mv643xx_eth_platform_data *eth_data,
unsigned long irq_err)
{
fill_resources(&orion_ge11_shared, orion_ge11_shared_resources,
- mapbase + 0x2000, SZ_16K - 1, irq_err);
+ mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+ fill_resources(&orion_ge11_mvmdio, orion_ge11_mvmdio_resources,
+ mapbase + 0x2004, 0x84 - 1, irq_err);
ge_complete(&orion_ge11_shared_data,
orion_ge11_resources, irq, &orion_ge11_shared,
+ NULL,
eth_data, &orion_ge11);
}

diff --git a/arch/powerpc/platforms/chrp/pegasos_eth.c b/arch/powerpc/platforms/chrp/pegasos_eth.c
index 039fc8e..a671508 100644
--- a/arch/powerpc/platforms/chrp/pegasos_eth.c
+++ b/arch/powerpc/platforms/chrp/pegasos_eth.c
@@ -47,6 +47,25 @@ static struct platform_device mv643xx_eth_shared_device = {
.resource = mv643xx_eth_shared_resources,
};

+/*
+ * The orion mdio driver only covers shared + 0x4 up to shared + 0x84 - 1
+ */
+static struct resource mv643xx_eth_mvmdio_resources[] = {
+ [0] = {
+ .name = "ethernet mdio base",
+ .start = 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x4,
+ .end = 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x83,
+ .flags = IORESOURCE_MEM,
+ },
+};
+
+static struct platform_device mv643xx_eth_mvmdio_device = {
+ .name = "orion-mdio",
+ .id = 0,
+ .num_resources = ARRAY_SIZE(mv643xx_eth_mvmdio_resources),
+ .resource = mv643xx_eth_shared_resources,
+};
+
static struct resource mv643xx_eth_port1_resources[] = {
[0] = {
.name = "eth port1 irq",
@@ -82,6 +101,7 @@ static struct platform_device eth_port1_device = {

static struct platform_device *mv643xx_eth_pd_devs[] __initdata = {
&mv643xx_eth_shared_device,
+ &mv643xx_eth_mvmdio_device,
&eth_port1_device,
};

diff --git a/arch/powerpc/sysdev/mv64x60_dev.c b/arch/powerpc/sysdev/mv64x60_dev.c
index 0f6af41..630cea3 100644
--- a/arch/powerpc/sysdev/mv64x60_dev.c
+++ b/arch/powerpc/sysdev/mv64x60_dev.c
@@ -214,15 +214,25 @@ static struct platform_device * __init mv64x60_eth_register_shared_pdev(
struct device_node *np, int id)
{
struct platform_device *pdev;
- struct resource r[1];
+ struct resource r[2];
int err;

err = of_address_to_resource(np, 0, &r[0]);
if (err)
return ERR_PTR(err);

+ /* register an orion mdio bus driver */
+ r[1].start = r[0].start + 0x4;
+ r[1].end = r[0].start + 0x84 - 1;
+ r[1].flags = IORESOURCE_MEM;
+
+ pdev = platform_device_register_simple("orion-mdio", id, &r[1], 1);
+ if (!pdev)
+ return pdev;
+
pdev = platform_device_register_simple(MV643XX_ETH_SHARED_NAME, id,
- r, 1);
+ &r[0], 1);
+
return pdev;
}

diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
index edfba93..df06061 100644
--- a/drivers/net/ethernet/marvell/Kconfig
+++ b/drivers/net/ethernet/marvell/Kconfig
@@ -23,6 +23,7 @@ config MV643XX_ETH
depends on (MV64X60 || PPC32 || PLAT_ORION) && INET
select INET_LRO
select PHYLIB
+ select MVMDIO
---help---
This driver supports the gigabit ethernet MACs in the
Marvell Discovery PPC/MIPS chipset family (MV643XX) and
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index d1ecf4b..df04bee 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -69,14 +69,6 @@ static char mv643xx_eth_driver_version[] = "1.4";
* Registers shared between all ports.
*/
#define PHY_ADDR 0x0000
-#define SMI_REG 0x0004
-#define SMI_BUSY 0x10000000
-#define SMI_READ_VALID 0x08000000
-#define SMI_OPCODE_READ 0x04000000
-#define SMI_OPCODE_WRITE 0x00000000
-#define ERR_INT_CAUSE 0x0080
-#define ERR_INT_SMI_DONE 0x00000010
-#define ERR_INT_MASK 0x0084
#define WINDOW_BASE(w) (0x0200 + ((w) << 3))
#define WINDOW_SIZE(w) (0x0204 + ((w) << 3))
#define WINDOW_REMAP_HIGH(w) (0x0280 + ((w) << 2))
@@ -266,25 +258,6 @@ struct mv643xx_eth_shared_private {
void __iomem *base;

/*
- * Points at the right SMI instance to use.
- */
- struct mv643xx_eth_shared_private *smi;
-
- /*
- * Provides access to local SMI interface.
- */
- struct mii_bus *smi_bus;
-
- /*
- * If we have access to the error interrupt pin (which is
- * somewhat misnamed as it not only reflects internal errors
- * but also reflects SMI completion), use that to wait for
- * SMI access completion instead of polling the SMI busy bit.
- */
- int err_interrupt;
- wait_queue_head_t smi_busy_wait;
-
- /*
* Per-port MBUS window access register value.
*/
u32 win_protect;
@@ -1122,97 +1095,6 @@ out_write:
wrlp(mp, PORT_SERIAL_CONTROL, pscr);
}

-static irqreturn_t mv643xx_eth_err_irq(int irq, void *dev_id)
-{
- struct mv643xx_eth_shared_private *msp = dev_id;
-
- if (readl(msp->base + ERR_INT_CAUSE) & ERR_INT_SMI_DONE) {
- writel(~ERR_INT_SMI_DONE, msp->base + ERR_INT_CAUSE);
- wake_up(&msp->smi_busy_wait);
- return IRQ_HANDLED;
- }
-
- return IRQ_NONE;
-}
-
-static int smi_is_done(struct mv643xx_eth_shared_private *msp)
-{
- return !(readl(msp->base + SMI_REG) & SMI_BUSY);
-}
-
-static int smi_wait_ready(struct mv643xx_eth_shared_private *msp)
-{
- if (msp->err_interrupt == NO_IRQ) {
- int i;
-
- for (i = 0; !smi_is_done(msp); i++) {
- if (i == 10)
- return -ETIMEDOUT;
- msleep(10);
- }
-
- return 0;
- }
-
- if (!smi_is_done(msp)) {
- wait_event_timeout(msp->smi_busy_wait, smi_is_done(msp),
- msecs_to_jiffies(100));
- if (!smi_is_done(msp))
- return -ETIMEDOUT;
- }
-
- return 0;
-}
-
-static int smi_bus_read(struct mii_bus *bus, int addr, int reg)
-{
- struct mv643xx_eth_shared_private *msp = bus->priv;
- void __iomem *smi_reg = msp->base + SMI_REG;
- int ret;
-
- if (smi_wait_ready(msp)) {
- pr_warn("SMI bus busy timeout\n");
- return -ETIMEDOUT;
- }
-
- writel(SMI_OPCODE_READ | (reg << 21) | (addr << 16), smi_reg);
-
- if (smi_wait_ready(msp)) {
- pr_warn("SMI bus busy timeout\n");
- return -ETIMEDOUT;
- }
-
- ret = readl(smi_reg);
- if (!(ret & SMI_READ_VALID)) {
- pr_warn("SMI bus read not valid\n");
- return -ENODEV;
- }
-
- return ret & 0xffff;
-}
-
-static int smi_bus_write(struct mii_bus *bus, int addr, int reg, u16 val)
-{
- struct mv643xx_eth_shared_private *msp = bus->priv;
- void __iomem *smi_reg = msp->base + SMI_REG;
-
- if (smi_wait_ready(msp)) {
- pr_warn("SMI bus busy timeout\n");
- return -ETIMEDOUT;
- }
-
- writel(SMI_OPCODE_WRITE | (reg << 21) |
- (addr << 16) | (val & 0xffff), smi_reg);
-
- if (smi_wait_ready(msp)) {
- pr_warn("SMI bus busy timeout\n");
- return -ETIMEDOUT;
- }
-
- return 0;
-}
-
-
/* statistics ***************************************************************/
static struct net_device_stats *mv643xx_eth_get_stats(struct net_device *dev)
{
@@ -2688,47 +2570,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
goto out_free;

/*
- * Set up and register SMI bus.
- */
- if (pd == NULL || pd->shared_smi == NULL) {
- msp->smi_bus = mdiobus_alloc();
- if (msp->smi_bus == NULL)
- goto out_unmap;
-
- msp->smi_bus->priv = msp;
- msp->smi_bus->name = "mv643xx_eth smi";
- msp->smi_bus->read = smi_bus_read;
- msp->smi_bus->write = smi_bus_write,
- snprintf(msp->smi_bus->id, MII_BUS_ID_SIZE, "%s-%d",
- pdev->name, pdev->id);
- msp->smi_bus->parent = &pdev->dev;
- msp->smi_bus->phy_mask = 0xffffffff;
- if (mdiobus_register(msp->smi_bus) < 0)
- goto out_free_mii_bus;
- msp->smi = msp;
- } else {
- msp->smi = platform_get_drvdata(pd->shared_smi);
- }
-
- msp->err_interrupt = NO_IRQ;
- init_waitqueue_head(&msp->smi_busy_wait);
-
- /*
- * Check whether the error interrupt is hooked up.
- */
- res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (res != NULL) {
- int err;
-
- err = request_irq(res->start, mv643xx_eth_err_irq,
- IRQF_SHARED, "mv643xx_eth", msp);
- if (!err) {
- writel(ERR_INT_SMI_DONE, msp->base + ERR_INT_MASK);
- msp->err_interrupt = res->start;
- }
- }
-
- /*
* (Re-)program MBUS remapping windows if we are asked to.
*/
dram = mv_mbus_dram_info();
@@ -2743,10 +2584,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)

return 0;

-out_free_mii_bus:
- mdiobus_free(msp->smi_bus);
-out_unmap:
- iounmap(msp->base);
out_free:
kfree(msp);
out:
@@ -2756,14 +2593,7 @@ out:
static int mv643xx_eth_shared_remove(struct platform_device *pdev)
{
struct mv643xx_eth_shared_private *msp = platform_get_drvdata(pdev);
- struct mv643xx_eth_shared_platform_data *pd = pdev->dev.platform_data;

- if (pd == NULL || pd->shared_smi == NULL) {
- mdiobus_unregister(msp->smi_bus);
- mdiobus_free(msp->smi_bus);
- }
- if (msp->err_interrupt != NO_IRQ)
- free_irq(msp->err_interrupt, msp);
iounmap(msp->base);
kfree(msp);

@@ -2829,11 +2659,11 @@ static void set_params(struct mv643xx_eth_private *mp,
static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
int phy_addr)
{
- struct mii_bus *bus = mp->shared->smi->smi_bus;
struct phy_device *phydev;
int start;
int num;
int i;
+ char phy_id[MII_BUS_ID_SIZE + 3];

if (phy_addr == MV643XX_ETH_PHY_ADDR_DEFAULT) {
start = phy_addr_get(mp) & 0x1f;
@@ -2843,17 +2673,19 @@ static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
num = 1;
}

+ /* Attempt to connect to the PHY using orion-mdio */
phydev = NULL;
for (i = 0; i < num; i++) {
int addr = (start + i) & 0x1f;

- if (bus->phy_map[addr] == NULL)
- mdiobus_scan(bus, addr);
+ snprintf(phy_id, sizeof(phy_id), PHY_ID_FMT,
+ "orion-mdio-mii", addr);

- if (phydev == NULL) {
- phydev = bus->phy_map[addr];
- if (phydev != NULL)
- phy_addr_set(mp, addr);
+ phydev = phy_connect(mp->dev, phy_id, NULL,
+ PHY_INTERFACE_MODE_GMII);
+ if (!IS_ERR(phydev)) {
+ phy_addr_set(mp, addr);
+ break;
}
}

diff --git a/include/linux/mv643xx_eth.h b/include/linux/mv643xx_eth.h
index 49258e0..141d395 100644
--- a/include/linux/mv643xx_eth.h
+++ b/include/linux/mv643xx_eth.h
@@ -19,7 +19,6 @@

struct mv643xx_eth_shared_platform_data {
struct mbus_dram_target_info *dram;
- struct platform_device *shared_smi;
/*
* Max packet size for Tx IP/Layer 4 checksum, when set to 0, default
* limit of 9KiB will be used.
--
1.7.10.4

2013-03-14 18:20:36

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH 3/4 v2] net: mvmdio: enhance driver to support SMI error/done interrupts

This patch enhances the "mvmdio" to support a SMI error/done interrupt
line which can be used along with a wait queue instead of doing
busy-waiting on the registers. This is a feature which is available in
the mv643xx_eth SMI code and thus reduces again the gap between the two.

Signed-off-by: Florian Fainelli <[email protected]>
---
Chances since v1:
- always use orion_smi_is_done() helper
- make interrupt/non-interrupt code path entirely independant

.../devicetree/bindings/net/marvell-orion-mdio.txt | 3 +
drivers/net/ethernet/marvell/mvmdio.c | 83 +++++++++++++++++---
2 files changed, 74 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
index 34e7aaf..052b5f2 100644
--- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
+++ b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
@@ -9,6 +9,9 @@ Required properties:
- compatible: "marvell,orion-mdio"
- reg: address and length of the SMI register

+Optional properties:
+- interrupts: interrupt line number for the SMI error/done interrupt
+
The child nodes of the MDIO driver are the individual PHY devices
connected to this MDIO bus. They must have a "reg" property given the
PHY address on the MDIO bus.
diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 595deea..7ac83de 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -24,9 +24,12 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/phy.h>
+#include <linux/interrupt.h>
#include <linux/platform_device.h>
#include <linux/delay.h>
#include <linux/io.h>
+#include <linux/sched.h>
+#include <linux/wait.h>

#define MVMDIO_SMI_DATA_SHIFT 0
#define MVMDIO_SMI_PHY_ADDR_SHIFT 16
@@ -35,33 +38,58 @@
#define MVMDIO_SMI_WRITE_OPERATION 0
#define MVMDIO_SMI_READ_VALID BIT(27)
#define MVMDIO_SMI_BUSY BIT(28)
+#define MVMDIO_ERR_INT_CAUSE 0x007C
+#define MVMDIO_ERR_INT_SMI_DONE 0x00000010
+#define MVMDIO_ERR_INT_MASK 0x0080

struct orion_mdio_dev {
struct mutex lock;
void __iomem *regs;
+ /*
+ * If we have access to the error interrupt pin (which is
+ * somewhat misnamed as it not only reflects internal errors
+ * but also reflects SMI completion), use that to wait for
+ * SMI access completion instead of polling the SMI busy bit.
+ */
+ int err_interrupt;
+ wait_queue_head_t smi_busy_wait;
};

+static int orion_mdio_smi_is_done(struct orion_mdio_dev *dev)
+{
+ return !(readl(dev->regs) & MVMDIO_SMI_BUSY);
+}
+
/* Wait for the SMI unit to be ready for another operation
*/
static int orion_mdio_wait_ready(struct mii_bus *bus)
{
struct orion_mdio_dev *dev = bus->priv;
int count;
- u32 val;

- count = 0;
- while (1) {
- val = readl(dev->regs);
- if (!(val & MVMDIO_SMI_BUSY))
- break;
+ if (dev->err_interrupt == NO_IRQ) {
+ count = 0;
+ while (1) {
+ if (orion_mdio_smi_is_done(dev))
+ break;

- if (count > 100) {
- dev_err(bus->parent, "Timeout: SMI busy for too long\n");
- return -ETIMEDOUT;
- }
+ if (count > 100) {
+ dev_err(bus->parent,
+ "Timeout: SMI busy for too long\n");
+ return -ETIMEDOUT;
+ }

- udelay(10);
- count++;
+ udelay(10);
+ count++;
+ }
+ } else {
+ if (!orion_mdio_smi_is_done(dev)) {
+ wait_event_timeout(dev->smi_busy_wait,
+ orion_mdio_smi_is_done(dev),
+ msecs_to_jiffies(100));
+ if (!orion_mdio_smi_is_done(dev))
+ return -ETIMEDOUT;
+ }
}

return 0;
@@ -140,6 +168,21 @@ static int orion_mdio_reset(struct mii_bus *bus)
return 0;
}

+static irqreturn_t orion_mdio_err_irq(int irq, void *dev_id)
+{
+ struct orion_mdio_dev *dev = dev_id;
+
+ if (readl(dev->regs + MVMDIO_ERR_INT_CAUSE) &
+ MVMDIO_ERR_INT_SMI_DONE) {
+ writel(~MVMDIO_ERR_INT_SMI_DONE,
+ dev->regs + MVMDIO_ERR_INT_CAUSE);
+ wake_up(&dev->smi_busy_wait);
+ return IRQ_HANDLED;
+ }
+
+ return IRQ_NONE;
+}
+
static int orion_mdio_probe(struct platform_device *pdev)
{
struct resource *r;
@@ -185,6 +228,19 @@ static int orion_mdio_probe(struct platform_device *pdev)
return -ENODEV;
}

+ init_waitqueue_head(&dev->smi_busy_wait);
+
+ dev->err_interrupt = platform_get_irq(pdev, 0);
+ if (dev->err_interrupt != -ENXIO) {
+ ret = devm_request_irq(&pdev->dev, dev->err_interrupt,
+ orion_mdio_err_irq,
+ IRQF_SHARED, pdev->name, dev);
+ if (!ret)
+ writel(MVMDIO_ERR_INT_SMI_DONE,
+ dev->regs + MVMDIO_ERR_INT_MASK);
+ } else
+ dev->err_interrupt = NO_IRQ;
+
mutex_init(&dev->lock);

ret = mdiobus_register(bus);
@@ -203,6 +259,9 @@ static int orion_mdio_probe(struct platform_device *pdev)
static int orion_mdio_remove(struct platform_device *pdev)
{
struct mii_bus *bus = platform_get_drvdata(pdev);
+ struct orion_mdio_dev *dev = bus->priv;
+
+ writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
mdiobus_unregister(bus);
kfree(bus->irq);
mdiobus_free(bus);
--
1.7.10.4

2013-03-14 18:20:32

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH] mv643xx_eth: convert to use the Marvell Orion MDIO driver

This patch converts the Marvell MV643XX ethernet driver to use the
Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
registering the Marvell MV643XX ethernet driver are also updated to
register a Marvell Orion MDIO driver. This driver voluntarily overlaps
with the Marvell Ethernet shared registers because it will use a subset
of this shared register (shared_base + 0x4 - shared_base + 0x84). The
Ethernet driver is also updated to look up for a PHY device using the
Orion MDIO bus driver.

Signed-off-by: Florian Fainelli <[email protected]>
---
arch/arm/plat-orion/common.c | 97 +++++++++++----
arch/powerpc/platforms/chrp/pegasos_eth.c | 20 +++
arch/powerpc/sysdev/mv64x60_dev.c | 14 ++-
drivers/net/ethernet/marvell/Kconfig | 1 +
drivers/net/ethernet/marvell/mv643xx_eth.c | 186 ++--------------------------
include/linux/mv643xx_eth.h | 1 -
6 files changed, 117 insertions(+), 202 deletions(-)

diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index 2d4b641..dc4345e 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -238,6 +238,7 @@ static __init void ge_complete(
struct mv643xx_eth_shared_platform_data *orion_ge_shared_data,
struct resource *orion_ge_resource, unsigned long irq,
struct platform_device *orion_ge_shared,
+ struct platform_device *orion_ge_mvmdio,
struct mv643xx_eth_platform_data *eth_data,
struct platform_device *orion_ge)
{
@@ -247,6 +248,8 @@ static __init void ge_complete(
orion_ge->dev.platform_data = eth_data;

platform_device_register(orion_ge_shared);
+ if (orion_ge_mvmdio)
+ platform_device_register(orion_ge_mvmdio);
platform_device_register(orion_ge);
}

@@ -258,8 +261,6 @@ struct mv643xx_eth_shared_platform_data orion_ge00_shared_data;
static struct resource orion_ge00_shared_resources[] = {
{
.name = "ge00 base",
- }, {
- .name = "ge00 err irq",
},
};

@@ -271,6 +272,19 @@ static struct platform_device orion_ge00_shared = {
},
};

+static struct resource orion_ge00_mvmdio_resources[] = {
+ {
+ .name = "ge00 mvmdio base",
+ }, {
+ .name = "ge00 mvmdio irr irq",
+ },
+};
+
+static struct platform_device orion_ge00_mvmdio = {
+ .name = "orion-mdio",
+ .id = 0,
+};
+
static struct resource orion_ge00_resources[] = {
{
.name = "ge00 irq",
@@ -295,26 +309,25 @@ void __init orion_ge00_init(struct mv643xx_eth_platform_data *eth_data,
unsigned int tx_csum_limit)
{
fill_resources(&orion_ge00_shared, orion_ge00_shared_resources,
- mapbase + 0x2000, SZ_16K - 1, irq_err);
+ mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+ fill_resources(&orion_ge00_mvmdio, orion_ge00_mvmdio_resources,
+ mapbase + 0x2004, 0x84 - 1, irq_err);
orion_ge00_shared_data.tx_csum_limit = tx_csum_limit;
ge_complete(&orion_ge00_shared_data,
orion_ge00_resources, irq, &orion_ge00_shared,
+ &orion_ge00_mvmdio,
eth_data, &orion_ge00);
}

/*****************************************************************************
* GE01
****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge01_shared_data = {
- .shared_smi = &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge01_shared_data;

static struct resource orion_ge01_shared_resources[] = {
{
.name = "ge01 base",
- }, {
- .name = "ge01 err irq",
- },
+ }
};

static struct platform_device orion_ge01_shared = {
@@ -325,6 +338,19 @@ static struct platform_device orion_ge01_shared = {
},
};

+static struct resource orion_ge01_mvmdio_resources[] = {
+ {
+ .name = "ge01 mdio base",
+ }, {
+ .name = "ge01 mdio err irq",
+ },
+};
+
+static struct platform_device orion_ge01_mvmdio = {
+ .name = "orion-mdio",
+ .id = 1,
+};
+
static struct resource orion_ge01_resources[] = {
{
.name = "ge01 irq",
@@ -349,26 +375,25 @@ void __init orion_ge01_init(struct mv643xx_eth_platform_data *eth_data,
unsigned int tx_csum_limit)
{
fill_resources(&orion_ge01_shared, orion_ge01_shared_resources,
- mapbase + 0x2000, SZ_16K - 1, irq_err);
+ mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+ fill_resources(&orion_ge01_mvmdio, orion_ge01_mvmdio_resources,
+ mapbase + 0x2004, 0x84 - 1, irq_err);
orion_ge01_shared_data.tx_csum_limit = tx_csum_limit;
ge_complete(&orion_ge01_shared_data,
orion_ge01_resources, irq, &orion_ge01_shared,
+ NULL,
eth_data, &orion_ge01);
}

/*****************************************************************************
* GE10
****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge10_shared_data = {
- .shared_smi = &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge10_shared_data;

static struct resource orion_ge10_shared_resources[] = {
{
.name = "ge10 base",
- }, {
- .name = "ge10 err irq",
- },
+ }
};

static struct platform_device orion_ge10_shared = {
@@ -379,6 +404,19 @@ static struct platform_device orion_ge10_shared = {
},
};

+static struct resource orion_ge10_mvmdio_resources[] = {
+ {
+ .name = "ge10 mvmdio base",
+ }, {
+ .name = "ge10 mvmdio err irq",
+ },
+};
+
+static struct platform_device orion_ge10_mvmdio = {
+ .name = "orion-mdio",
+ .id = 1,
+};
+
static struct resource orion_ge10_resources[] = {
{
.name = "ge10 irq",
@@ -403,23 +441,22 @@ void __init orion_ge10_init(struct mv643xx_eth_platform_data *eth_data,
{
fill_resources(&orion_ge10_shared, orion_ge10_shared_resources,
mapbase + 0x2000, SZ_16K - 1, irq_err);
+ fill_resources(&orion_ge10_mvmdio, orion_ge10_mvmdio_resources,
+ mapbase + 0x2004, 0x84 - 1, irq_err);
ge_complete(&orion_ge10_shared_data,
orion_ge10_resources, irq, &orion_ge10_shared,
+ &orion_ge10_mvmdio,
eth_data, &orion_ge10);
}

/*****************************************************************************
* GE11
****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge11_shared_data = {
- .shared_smi = &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge11_shared_data;

static struct resource orion_ge11_shared_resources[] = {
{
.name = "ge11 base",
- }, {
- .name = "ge11 err irq",
},
};

@@ -431,6 +468,19 @@ static struct platform_device orion_ge11_shared = {
},
};

+static struct resource orion_ge11_mvmdio_resources[] = {
+ {
+ .name = "ge11 mvmdio base",
+ }, {
+ .name = "ge11 mvmdio err irq",
+ },
+};
+
+static struct platform_device orion_ge11_mvmdio = {
+ .name = "orion-mdio",
+ .id = 1,
+};
+
static struct resource orion_ge11_resources[] = {
{
.name = "ge11 irq",
@@ -454,9 +504,12 @@ void __init orion_ge11_init(struct mv643xx_eth_platform_data *eth_data,
unsigned long irq_err)
{
fill_resources(&orion_ge11_shared, orion_ge11_shared_resources,
- mapbase + 0x2000, SZ_16K - 1, irq_err);
+ mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+ fill_resources(&orion_ge11_mvmdio, orion_ge11_mvmdio_resources,
+ mapbase + 0x2004, 0x84 - 1, irq_err);
ge_complete(&orion_ge11_shared_data,
orion_ge11_resources, irq, &orion_ge11_shared,
+ NULL,
eth_data, &orion_ge11);
}

diff --git a/arch/powerpc/platforms/chrp/pegasos_eth.c b/arch/powerpc/platforms/chrp/pegasos_eth.c
index 039fc8e..a671508 100644
--- a/arch/powerpc/platforms/chrp/pegasos_eth.c
+++ b/arch/powerpc/platforms/chrp/pegasos_eth.c
@@ -47,6 +47,25 @@ static struct platform_device mv643xx_eth_shared_device = {
.resource = mv643xx_eth_shared_resources,
};

+/*
+ * The orion mdio driver only covers shared + 0x4 up to shared + 0x84 - 1
+ */
+static struct resource mv643xx_eth_mvmdio_resources[] = {
+ [0] = {
+ .name = "ethernet mdio base",
+ .start = 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x4,
+ .end = 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x83,
+ .flags = IORESOURCE_MEM,
+ },
+};
+
+static struct platform_device mv643xx_eth_mvmdio_device = {
+ .name = "orion-mdio",
+ .id = 0,
+ .num_resources = ARRAY_SIZE(mv643xx_eth_mvmdio_resources),
+ .resource = mv643xx_eth_shared_resources,
+};
+
static struct resource mv643xx_eth_port1_resources[] = {
[0] = {
.name = "eth port1 irq",
@@ -82,6 +101,7 @@ static struct platform_device eth_port1_device = {

static struct platform_device *mv643xx_eth_pd_devs[] __initdata = {
&mv643xx_eth_shared_device,
+ &mv643xx_eth_mvmdio_device,
&eth_port1_device,
};

diff --git a/arch/powerpc/sysdev/mv64x60_dev.c b/arch/powerpc/sysdev/mv64x60_dev.c
index 0f6af41..630cea3 100644
--- a/arch/powerpc/sysdev/mv64x60_dev.c
+++ b/arch/powerpc/sysdev/mv64x60_dev.c
@@ -214,15 +214,25 @@ static struct platform_device * __init mv64x60_eth_register_shared_pdev(
struct device_node *np, int id)
{
struct platform_device *pdev;
- struct resource r[1];
+ struct resource r[2];
int err;

err = of_address_to_resource(np, 0, &r[0]);
if (err)
return ERR_PTR(err);

+ /* register an orion mdio bus driver */
+ r[1].start = r[0].start + 0x4;
+ r[1].end = r[0].start + 0x84 - 1;
+ r[1].flags = IORESOURCE_MEM;
+
+ pdev = platform_device_register_simple("orion-mdio", id, &r[1], 1);
+ if (!pdev)
+ return pdev;
+
pdev = platform_device_register_simple(MV643XX_ETH_SHARED_NAME, id,
- r, 1);
+ &r[0], 1);
+
return pdev;
}

diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
index edfba93..df06061 100644
--- a/drivers/net/ethernet/marvell/Kconfig
+++ b/drivers/net/ethernet/marvell/Kconfig
@@ -23,6 +23,7 @@ config MV643XX_ETH
depends on (MV64X60 || PPC32 || PLAT_ORION) && INET
select INET_LRO
select PHYLIB
+ select MVMDIO
---help---
This driver supports the gigabit ethernet MACs in the
Marvell Discovery PPC/MIPS chipset family (MV643XX) and
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index d1ecf4b..df04bee 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -69,14 +69,6 @@ static char mv643xx_eth_driver_version[] = "1.4";
* Registers shared between all ports.
*/
#define PHY_ADDR 0x0000
-#define SMI_REG 0x0004
-#define SMI_BUSY 0x10000000
-#define SMI_READ_VALID 0x08000000
-#define SMI_OPCODE_READ 0x04000000
-#define SMI_OPCODE_WRITE 0x00000000
-#define ERR_INT_CAUSE 0x0080
-#define ERR_INT_SMI_DONE 0x00000010
-#define ERR_INT_MASK 0x0084
#define WINDOW_BASE(w) (0x0200 + ((w) << 3))
#define WINDOW_SIZE(w) (0x0204 + ((w) << 3))
#define WINDOW_REMAP_HIGH(w) (0x0280 + ((w) << 2))
@@ -266,25 +258,6 @@ struct mv643xx_eth_shared_private {
void __iomem *base;

/*
- * Points at the right SMI instance to use.
- */
- struct mv643xx_eth_shared_private *smi;
-
- /*
- * Provides access to local SMI interface.
- */
- struct mii_bus *smi_bus;
-
- /*
- * If we have access to the error interrupt pin (which is
- * somewhat misnamed as it not only reflects internal errors
- * but also reflects SMI completion), use that to wait for
- * SMI access completion instead of polling the SMI busy bit.
- */
- int err_interrupt;
- wait_queue_head_t smi_busy_wait;
-
- /*
* Per-port MBUS window access register value.
*/
u32 win_protect;
@@ -1122,97 +1095,6 @@ out_write:
wrlp(mp, PORT_SERIAL_CONTROL, pscr);
}

-static irqreturn_t mv643xx_eth_err_irq(int irq, void *dev_id)
-{
- struct mv643xx_eth_shared_private *msp = dev_id;
-
- if (readl(msp->base + ERR_INT_CAUSE) & ERR_INT_SMI_DONE) {
- writel(~ERR_INT_SMI_DONE, msp->base + ERR_INT_CAUSE);
- wake_up(&msp->smi_busy_wait);
- return IRQ_HANDLED;
- }
-
- return IRQ_NONE;
-}
-
-static int smi_is_done(struct mv643xx_eth_shared_private *msp)
-{
- return !(readl(msp->base + SMI_REG) & SMI_BUSY);
-}
-
-static int smi_wait_ready(struct mv643xx_eth_shared_private *msp)
-{
- if (msp->err_interrupt == NO_IRQ) {
- int i;
-
- for (i = 0; !smi_is_done(msp); i++) {
- if (i == 10)
- return -ETIMEDOUT;
- msleep(10);
- }
-
- return 0;
- }
-
- if (!smi_is_done(msp)) {
- wait_event_timeout(msp->smi_busy_wait, smi_is_done(msp),
- msecs_to_jiffies(100));
- if (!smi_is_done(msp))
- return -ETIMEDOUT;
- }
-
- return 0;
-}
-
-static int smi_bus_read(struct mii_bus *bus, int addr, int reg)
-{
- struct mv643xx_eth_shared_private *msp = bus->priv;
- void __iomem *smi_reg = msp->base + SMI_REG;
- int ret;
-
- if (smi_wait_ready(msp)) {
- pr_warn("SMI bus busy timeout\n");
- return -ETIMEDOUT;
- }
-
- writel(SMI_OPCODE_READ | (reg << 21) | (addr << 16), smi_reg);
-
- if (smi_wait_ready(msp)) {
- pr_warn("SMI bus busy timeout\n");
- return -ETIMEDOUT;
- }
-
- ret = readl(smi_reg);
- if (!(ret & SMI_READ_VALID)) {
- pr_warn("SMI bus read not valid\n");
- return -ENODEV;
- }
-
- return ret & 0xffff;
-}
-
-static int smi_bus_write(struct mii_bus *bus, int addr, int reg, u16 val)
-{
- struct mv643xx_eth_shared_private *msp = bus->priv;
- void __iomem *smi_reg = msp->base + SMI_REG;
-
- if (smi_wait_ready(msp)) {
- pr_warn("SMI bus busy timeout\n");
- return -ETIMEDOUT;
- }
-
- writel(SMI_OPCODE_WRITE | (reg << 21) |
- (addr << 16) | (val & 0xffff), smi_reg);
-
- if (smi_wait_ready(msp)) {
- pr_warn("SMI bus busy timeout\n");
- return -ETIMEDOUT;
- }
-
- return 0;
-}
-
-
/* statistics ***************************************************************/
static struct net_device_stats *mv643xx_eth_get_stats(struct net_device *dev)
{
@@ -2688,47 +2570,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
goto out_free;

/*
- * Set up and register SMI bus.
- */
- if (pd == NULL || pd->shared_smi == NULL) {
- msp->smi_bus = mdiobus_alloc();
- if (msp->smi_bus == NULL)
- goto out_unmap;
-
- msp->smi_bus->priv = msp;
- msp->smi_bus->name = "mv643xx_eth smi";
- msp->smi_bus->read = smi_bus_read;
- msp->smi_bus->write = smi_bus_write,
- snprintf(msp->smi_bus->id, MII_BUS_ID_SIZE, "%s-%d",
- pdev->name, pdev->id);
- msp->smi_bus->parent = &pdev->dev;
- msp->smi_bus->phy_mask = 0xffffffff;
- if (mdiobus_register(msp->smi_bus) < 0)
- goto out_free_mii_bus;
- msp->smi = msp;
- } else {
- msp->smi = platform_get_drvdata(pd->shared_smi);
- }
-
- msp->err_interrupt = NO_IRQ;
- init_waitqueue_head(&msp->smi_busy_wait);
-
- /*
- * Check whether the error interrupt is hooked up.
- */
- res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (res != NULL) {
- int err;
-
- err = request_irq(res->start, mv643xx_eth_err_irq,
- IRQF_SHARED, "mv643xx_eth", msp);
- if (!err) {
- writel(ERR_INT_SMI_DONE, msp->base + ERR_INT_MASK);
- msp->err_interrupt = res->start;
- }
- }
-
- /*
* (Re-)program MBUS remapping windows if we are asked to.
*/
dram = mv_mbus_dram_info();
@@ -2743,10 +2584,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)

return 0;

-out_free_mii_bus:
- mdiobus_free(msp->smi_bus);
-out_unmap:
- iounmap(msp->base);
out_free:
kfree(msp);
out:
@@ -2756,14 +2593,7 @@ out:
static int mv643xx_eth_shared_remove(struct platform_device *pdev)
{
struct mv643xx_eth_shared_private *msp = platform_get_drvdata(pdev);
- struct mv643xx_eth_shared_platform_data *pd = pdev->dev.platform_data;

- if (pd == NULL || pd->shared_smi == NULL) {
- mdiobus_unregister(msp->smi_bus);
- mdiobus_free(msp->smi_bus);
- }
- if (msp->err_interrupt != NO_IRQ)
- free_irq(msp->err_interrupt, msp);
iounmap(msp->base);
kfree(msp);

@@ -2829,11 +2659,11 @@ static void set_params(struct mv643xx_eth_private *mp,
static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
int phy_addr)
{
- struct mii_bus *bus = mp->shared->smi->smi_bus;
struct phy_device *phydev;
int start;
int num;
int i;
+ char phy_id[MII_BUS_ID_SIZE + 3];

if (phy_addr == MV643XX_ETH_PHY_ADDR_DEFAULT) {
start = phy_addr_get(mp) & 0x1f;
@@ -2843,17 +2673,19 @@ static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
num = 1;
}

+ /* Attempt to connect to the PHY using orion-mdio */
phydev = NULL;
for (i = 0; i < num; i++) {
int addr = (start + i) & 0x1f;

- if (bus->phy_map[addr] == NULL)
- mdiobus_scan(bus, addr);
+ snprintf(phy_id, sizeof(phy_id), PHY_ID_FMT,
+ "orion-mdio-mii", addr);

- if (phydev == NULL) {
- phydev = bus->phy_map[addr];
- if (phydev != NULL)
- phy_addr_set(mp, addr);
+ phydev = phy_connect(mp->dev, phy_id, NULL,
+ PHY_INTERFACE_MODE_GMII);
+ if (!IS_ERR(phydev)) {
+ phy_addr_set(mp, addr);
+ break;
}
}

diff --git a/include/linux/mv643xx_eth.h b/include/linux/mv643xx_eth.h
index 49258e0..141d395 100644
--- a/include/linux/mv643xx_eth.h
+++ b/include/linux/mv643xx_eth.h
@@ -19,7 +19,6 @@

struct mv643xx_eth_shared_platform_data {
struct mbus_dram_target_info *dram;
- struct platform_device *shared_smi;
/*
* Max packet size for Tx IP/Layer 4 checksum, when set to 0, default
* limit of 9KiB will be used.
--
1.7.10.4

2013-03-14 18:25:22

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH 1/4 v2] net: mvmdio: allow platform device style registration

This patch changes the mvmdio driver not to use device tree
helper functions such as of_mdiobus_register() and of_iomap() so we can
instantiate this driver using a classic platform_device approach. Use
the device manager helper to ioremap() the base register cookie so we
get automatic freeing upon error and removal. This change is harmless
for Device Tree platforms because they will get the driver be registered
the same way as it was before.

Signed-off-by: Florian Fainelli <[email protected]>
---
Changes since v1:
- use device managed helpers to fix an ioremap leak in remove function
- remove the use of Device Tree specific helpers to allow platform-style
registration

drivers/net/ethernet/marvell/mvmdio.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 77b7c80..8a182f1 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -24,10 +24,9 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/phy.h>
-#include <linux/of_address.h>
-#include <linux/of_mdio.h>
#include <linux/platform_device.h>
#include <linux/delay.h>
+#include <linux/io.h>

#define MVMDIO_SMI_DATA_SHIFT 0
#define MVMDIO_SMI_PHY_ADDR_SHIFT 16
@@ -143,11 +142,17 @@ static int orion_mdio_reset(struct mii_bus *bus)

static int orion_mdio_probe(struct platform_device *pdev)
{
- struct device_node *np = pdev->dev.of_node;
+ struct resource *r;
struct mii_bus *bus;
struct orion_mdio_dev *dev;
int i, ret;

+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!r) {
+ dev_err(&pdev->dev, "No SMI register address given\n");
+ return -ENODEV;
+ }
+
bus = mdiobus_alloc_size(sizeof(struct orion_mdio_dev));
if (!bus) {
dev_err(&pdev->dev, "Cannot allocate MDIO bus\n");
@@ -172,9 +177,9 @@ static int orion_mdio_probe(struct platform_device *pdev)
bus->irq[i] = PHY_POLL;

dev = bus->priv;
- dev->smireg = of_iomap(pdev->dev.of_node, 0);
+ dev->smireg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
if (!dev->smireg) {
- dev_err(&pdev->dev, "No SMI register address given in DT\n");
+ dev_err(&pdev->dev, "Unable to remap SMI register\n");
kfree(bus->irq);
mdiobus_free(bus);
return -ENODEV;
@@ -182,10 +187,9 @@ static int orion_mdio_probe(struct platform_device *pdev)

mutex_init(&dev->lock);

- ret = of_mdiobus_register(bus, np);
+ ret = mdiobus_register(bus);
if (ret < 0) {
dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
- iounmap(dev->smireg);
kfree(bus->irq);
mdiobus_free(bus);
return ret;
--
1.7.10.4

2013-03-14 19:02:56

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH] mv643xx_eth: convert to use the Marvell Orion MDIO driver

Florian,

On Thu, Mar 14, 2013 at 07:08:32PM +0100, Florian Fainelli wrote:
> This patch converts the Marvell MV643XX ethernet driver to use the
> Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
> registering the Marvell MV643XX ethernet driver are also updated to
> register a Marvell Orion MDIO driver. This driver voluntarily overlaps
> with the Marvell Ethernet shared registers because it will use a subset
> of this shared register (shared_base + 0x4 - shared_base + 0x84). The
> Ethernet driver is also updated to look up for a PHY device using the
> Orion MDIO bus driver.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> arch/arm/plat-orion/common.c | 97 +++++++++++----
> arch/powerpc/platforms/chrp/pegasos_eth.c | 20 +++
> arch/powerpc/sysdev/mv64x60_dev.c | 14 ++-
> drivers/net/ethernet/marvell/Kconfig | 1 +
> drivers/net/ethernet/marvell/mv643xx_eth.c | 186 ++--------------------------
> include/linux/mv643xx_eth.h | 1 -
> 6 files changed, 117 insertions(+), 202 deletions(-)

Isn't this the old version of 4/4 ?

thx,

Jason.

>
> diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
> index 2d4b641..dc4345e 100644
> --- a/arch/arm/plat-orion/common.c
> +++ b/arch/arm/plat-orion/common.c
> @@ -238,6 +238,7 @@ static __init void ge_complete(
> struct mv643xx_eth_shared_platform_data *orion_ge_shared_data,
> struct resource *orion_ge_resource, unsigned long irq,
> struct platform_device *orion_ge_shared,
> + struct platform_device *orion_ge_mvmdio,
> struct mv643xx_eth_platform_data *eth_data,
> struct platform_device *orion_ge)
> {
> @@ -247,6 +248,8 @@ static __init void ge_complete(
> orion_ge->dev.platform_data = eth_data;
>
> platform_device_register(orion_ge_shared);
> + if (orion_ge_mvmdio)
> + platform_device_register(orion_ge_mvmdio);
> platform_device_register(orion_ge);
> }
>
> @@ -258,8 +261,6 @@ struct mv643xx_eth_shared_platform_data orion_ge00_shared_data;
> static struct resource orion_ge00_shared_resources[] = {
> {
> .name = "ge00 base",
> - }, {
> - .name = "ge00 err irq",
> },
> };
>
> @@ -271,6 +272,19 @@ static struct platform_device orion_ge00_shared = {
> },
> };
>
> +static struct resource orion_ge00_mvmdio_resources[] = {
> + {
> + .name = "ge00 mvmdio base",
> + }, {
> + .name = "ge00 mvmdio irr irq",
> + },
> +};
> +
> +static struct platform_device orion_ge00_mvmdio = {
> + .name = "orion-mdio",
> + .id = 0,
> +};
> +
> static struct resource orion_ge00_resources[] = {
> {
> .name = "ge00 irq",
> @@ -295,26 +309,25 @@ void __init orion_ge00_init(struct mv643xx_eth_platform_data *eth_data,
> unsigned int tx_csum_limit)
> {
> fill_resources(&orion_ge00_shared, orion_ge00_shared_resources,
> - mapbase + 0x2000, SZ_16K - 1, irq_err);
> + mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
> + fill_resources(&orion_ge00_mvmdio, orion_ge00_mvmdio_resources,
> + mapbase + 0x2004, 0x84 - 1, irq_err);
> orion_ge00_shared_data.tx_csum_limit = tx_csum_limit;
> ge_complete(&orion_ge00_shared_data,
> orion_ge00_resources, irq, &orion_ge00_shared,
> + &orion_ge00_mvmdio,
> eth_data, &orion_ge00);
> }
>
> /*****************************************************************************
> * GE01
> ****************************************************************************/
> -struct mv643xx_eth_shared_platform_data orion_ge01_shared_data = {
> - .shared_smi = &orion_ge00_shared,
> -};
> +struct mv643xx_eth_shared_platform_data orion_ge01_shared_data;
>
> static struct resource orion_ge01_shared_resources[] = {
> {
> .name = "ge01 base",
> - }, {
> - .name = "ge01 err irq",
> - },
> + }
> };
>
> static struct platform_device orion_ge01_shared = {
> @@ -325,6 +338,19 @@ static struct platform_device orion_ge01_shared = {
> },
> };
>
> +static struct resource orion_ge01_mvmdio_resources[] = {
> + {
> + .name = "ge01 mdio base",
> + }, {
> + .name = "ge01 mdio err irq",
> + },
> +};
> +
> +static struct platform_device orion_ge01_mvmdio = {
> + .name = "orion-mdio",
> + .id = 1,
> +};
> +
> static struct resource orion_ge01_resources[] = {
> {
> .name = "ge01 irq",
> @@ -349,26 +375,25 @@ void __init orion_ge01_init(struct mv643xx_eth_platform_data *eth_data,
> unsigned int tx_csum_limit)
> {
> fill_resources(&orion_ge01_shared, orion_ge01_shared_resources,
> - mapbase + 0x2000, SZ_16K - 1, irq_err);
> + mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
> + fill_resources(&orion_ge01_mvmdio, orion_ge01_mvmdio_resources,
> + mapbase + 0x2004, 0x84 - 1, irq_err);
> orion_ge01_shared_data.tx_csum_limit = tx_csum_limit;
> ge_complete(&orion_ge01_shared_data,
> orion_ge01_resources, irq, &orion_ge01_shared,
> + NULL,
> eth_data, &orion_ge01);
> }
>
> /*****************************************************************************
> * GE10
> ****************************************************************************/
> -struct mv643xx_eth_shared_platform_data orion_ge10_shared_data = {
> - .shared_smi = &orion_ge00_shared,
> -};
> +struct mv643xx_eth_shared_platform_data orion_ge10_shared_data;
>
> static struct resource orion_ge10_shared_resources[] = {
> {
> .name = "ge10 base",
> - }, {
> - .name = "ge10 err irq",
> - },
> + }
> };
>
> static struct platform_device orion_ge10_shared = {
> @@ -379,6 +404,19 @@ static struct platform_device orion_ge10_shared = {
> },
> };
>
> +static struct resource orion_ge10_mvmdio_resources[] = {
> + {
> + .name = "ge10 mvmdio base",
> + }, {
> + .name = "ge10 mvmdio err irq",
> + },
> +};
> +
> +static struct platform_device orion_ge10_mvmdio = {
> + .name = "orion-mdio",
> + .id = 1,
> +};
> +
> static struct resource orion_ge10_resources[] = {
> {
> .name = "ge10 irq",
> @@ -403,23 +441,22 @@ void __init orion_ge10_init(struct mv643xx_eth_platform_data *eth_data,
> {
> fill_resources(&orion_ge10_shared, orion_ge10_shared_resources,
> mapbase + 0x2000, SZ_16K - 1, irq_err);
> + fill_resources(&orion_ge10_mvmdio, orion_ge10_mvmdio_resources,
> + mapbase + 0x2004, 0x84 - 1, irq_err);
> ge_complete(&orion_ge10_shared_data,
> orion_ge10_resources, irq, &orion_ge10_shared,
> + &orion_ge10_mvmdio,
> eth_data, &orion_ge10);
> }
>
> /*****************************************************************************
> * GE11
> ****************************************************************************/
> -struct mv643xx_eth_shared_platform_data orion_ge11_shared_data = {
> - .shared_smi = &orion_ge00_shared,
> -};
> +struct mv643xx_eth_shared_platform_data orion_ge11_shared_data;
>
> static struct resource orion_ge11_shared_resources[] = {
> {
> .name = "ge11 base",
> - }, {
> - .name = "ge11 err irq",
> },
> };
>
> @@ -431,6 +468,19 @@ static struct platform_device orion_ge11_shared = {
> },
> };
>
> +static struct resource orion_ge11_mvmdio_resources[] = {
> + {
> + .name = "ge11 mvmdio base",
> + }, {
> + .name = "ge11 mvmdio err irq",
> + },
> +};
> +
> +static struct platform_device orion_ge11_mvmdio = {
> + .name = "orion-mdio",
> + .id = 1,
> +};
> +
> static struct resource orion_ge11_resources[] = {
> {
> .name = "ge11 irq",
> @@ -454,9 +504,12 @@ void __init orion_ge11_init(struct mv643xx_eth_platform_data *eth_data,
> unsigned long irq_err)
> {
> fill_resources(&orion_ge11_shared, orion_ge11_shared_resources,
> - mapbase + 0x2000, SZ_16K - 1, irq_err);
> + mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
> + fill_resources(&orion_ge11_mvmdio, orion_ge11_mvmdio_resources,
> + mapbase + 0x2004, 0x84 - 1, irq_err);
> ge_complete(&orion_ge11_shared_data,
> orion_ge11_resources, irq, &orion_ge11_shared,
> + NULL,
> eth_data, &orion_ge11);
> }
>
> diff --git a/arch/powerpc/platforms/chrp/pegasos_eth.c b/arch/powerpc/platforms/chrp/pegasos_eth.c
> index 039fc8e..a671508 100644
> --- a/arch/powerpc/platforms/chrp/pegasos_eth.c
> +++ b/arch/powerpc/platforms/chrp/pegasos_eth.c
> @@ -47,6 +47,25 @@ static struct platform_device mv643xx_eth_shared_device = {
> .resource = mv643xx_eth_shared_resources,
> };
>
> +/*
> + * The orion mdio driver only covers shared + 0x4 up to shared + 0x84 - 1
> + */
> +static struct resource mv643xx_eth_mvmdio_resources[] = {
> + [0] = {
> + .name = "ethernet mdio base",
> + .start = 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x4,
> + .end = 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x83,
> + .flags = IORESOURCE_MEM,
> + },
> +};
> +
> +static struct platform_device mv643xx_eth_mvmdio_device = {
> + .name = "orion-mdio",
> + .id = 0,
> + .num_resources = ARRAY_SIZE(mv643xx_eth_mvmdio_resources),
> + .resource = mv643xx_eth_shared_resources,
> +};
> +
> static struct resource mv643xx_eth_port1_resources[] = {
> [0] = {
> .name = "eth port1 irq",
> @@ -82,6 +101,7 @@ static struct platform_device eth_port1_device = {
>
> static struct platform_device *mv643xx_eth_pd_devs[] __initdata = {
> &mv643xx_eth_shared_device,
> + &mv643xx_eth_mvmdio_device,
> &eth_port1_device,
> };
>
> diff --git a/arch/powerpc/sysdev/mv64x60_dev.c b/arch/powerpc/sysdev/mv64x60_dev.c
> index 0f6af41..630cea3 100644
> --- a/arch/powerpc/sysdev/mv64x60_dev.c
> +++ b/arch/powerpc/sysdev/mv64x60_dev.c
> @@ -214,15 +214,25 @@ static struct platform_device * __init mv64x60_eth_register_shared_pdev(
> struct device_node *np, int id)
> {
> struct platform_device *pdev;
> - struct resource r[1];
> + struct resource r[2];
> int err;
>
> err = of_address_to_resource(np, 0, &r[0]);
> if (err)
> return ERR_PTR(err);
>
> + /* register an orion mdio bus driver */
> + r[1].start = r[0].start + 0x4;
> + r[1].end = r[0].start + 0x84 - 1;
> + r[1].flags = IORESOURCE_MEM;
> +
> + pdev = platform_device_register_simple("orion-mdio", id, &r[1], 1);
> + if (!pdev)
> + return pdev;
> +
> pdev = platform_device_register_simple(MV643XX_ETH_SHARED_NAME, id,
> - r, 1);
> + &r[0], 1);
> +
> return pdev;
> }
>
> diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
> index edfba93..df06061 100644
> --- a/drivers/net/ethernet/marvell/Kconfig
> +++ b/drivers/net/ethernet/marvell/Kconfig
> @@ -23,6 +23,7 @@ config MV643XX_ETH
> depends on (MV64X60 || PPC32 || PLAT_ORION) && INET
> select INET_LRO
> select PHYLIB
> + select MVMDIO
> ---help---
> This driver supports the gigabit ethernet MACs in the
> Marvell Discovery PPC/MIPS chipset family (MV643XX) and
> diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
> index d1ecf4b..df04bee 100644
> --- a/drivers/net/ethernet/marvell/mv643xx_eth.c
> +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
> @@ -69,14 +69,6 @@ static char mv643xx_eth_driver_version[] = "1.4";
> * Registers shared between all ports.
> */
> #define PHY_ADDR 0x0000
> -#define SMI_REG 0x0004
> -#define SMI_BUSY 0x10000000
> -#define SMI_READ_VALID 0x08000000
> -#define SMI_OPCODE_READ 0x04000000
> -#define SMI_OPCODE_WRITE 0x00000000
> -#define ERR_INT_CAUSE 0x0080
> -#define ERR_INT_SMI_DONE 0x00000010
> -#define ERR_INT_MASK 0x0084
> #define WINDOW_BASE(w) (0x0200 + ((w) << 3))
> #define WINDOW_SIZE(w) (0x0204 + ((w) << 3))
> #define WINDOW_REMAP_HIGH(w) (0x0280 + ((w) << 2))
> @@ -266,25 +258,6 @@ struct mv643xx_eth_shared_private {
> void __iomem *base;
>
> /*
> - * Points at the right SMI instance to use.
> - */
> - struct mv643xx_eth_shared_private *smi;
> -
> - /*
> - * Provides access to local SMI interface.
> - */
> - struct mii_bus *smi_bus;
> -
> - /*
> - * If we have access to the error interrupt pin (which is
> - * somewhat misnamed as it not only reflects internal errors
> - * but also reflects SMI completion), use that to wait for
> - * SMI access completion instead of polling the SMI busy bit.
> - */
> - int err_interrupt;
> - wait_queue_head_t smi_busy_wait;
> -
> - /*
> * Per-port MBUS window access register value.
> */
> u32 win_protect;
> @@ -1122,97 +1095,6 @@ out_write:
> wrlp(mp, PORT_SERIAL_CONTROL, pscr);
> }
>
> -static irqreturn_t mv643xx_eth_err_irq(int irq, void *dev_id)
> -{
> - struct mv643xx_eth_shared_private *msp = dev_id;
> -
> - if (readl(msp->base + ERR_INT_CAUSE) & ERR_INT_SMI_DONE) {
> - writel(~ERR_INT_SMI_DONE, msp->base + ERR_INT_CAUSE);
> - wake_up(&msp->smi_busy_wait);
> - return IRQ_HANDLED;
> - }
> -
> - return IRQ_NONE;
> -}
> -
> -static int smi_is_done(struct mv643xx_eth_shared_private *msp)
> -{
> - return !(readl(msp->base + SMI_REG) & SMI_BUSY);
> -}
> -
> -static int smi_wait_ready(struct mv643xx_eth_shared_private *msp)
> -{
> - if (msp->err_interrupt == NO_IRQ) {
> - int i;
> -
> - for (i = 0; !smi_is_done(msp); i++) {
> - if (i == 10)
> - return -ETIMEDOUT;
> - msleep(10);
> - }
> -
> - return 0;
> - }
> -
> - if (!smi_is_done(msp)) {
> - wait_event_timeout(msp->smi_busy_wait, smi_is_done(msp),
> - msecs_to_jiffies(100));
> - if (!smi_is_done(msp))
> - return -ETIMEDOUT;
> - }
> -
> - return 0;
> -}
> -
> -static int smi_bus_read(struct mii_bus *bus, int addr, int reg)
> -{
> - struct mv643xx_eth_shared_private *msp = bus->priv;
> - void __iomem *smi_reg = msp->base + SMI_REG;
> - int ret;
> -
> - if (smi_wait_ready(msp)) {
> - pr_warn("SMI bus busy timeout\n");
> - return -ETIMEDOUT;
> - }
> -
> - writel(SMI_OPCODE_READ | (reg << 21) | (addr << 16), smi_reg);
> -
> - if (smi_wait_ready(msp)) {
> - pr_warn("SMI bus busy timeout\n");
> - return -ETIMEDOUT;
> - }
> -
> - ret = readl(smi_reg);
> - if (!(ret & SMI_READ_VALID)) {
> - pr_warn("SMI bus read not valid\n");
> - return -ENODEV;
> - }
> -
> - return ret & 0xffff;
> -}
> -
> -static int smi_bus_write(struct mii_bus *bus, int addr, int reg, u16 val)
> -{
> - struct mv643xx_eth_shared_private *msp = bus->priv;
> - void __iomem *smi_reg = msp->base + SMI_REG;
> -
> - if (smi_wait_ready(msp)) {
> - pr_warn("SMI bus busy timeout\n");
> - return -ETIMEDOUT;
> - }
> -
> - writel(SMI_OPCODE_WRITE | (reg << 21) |
> - (addr << 16) | (val & 0xffff), smi_reg);
> -
> - if (smi_wait_ready(msp)) {
> - pr_warn("SMI bus busy timeout\n");
> - return -ETIMEDOUT;
> - }
> -
> - return 0;
> -}
> -
> -
> /* statistics ***************************************************************/
> static struct net_device_stats *mv643xx_eth_get_stats(struct net_device *dev)
> {
> @@ -2688,47 +2570,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
> goto out_free;
>
> /*
> - * Set up and register SMI bus.
> - */
> - if (pd == NULL || pd->shared_smi == NULL) {
> - msp->smi_bus = mdiobus_alloc();
> - if (msp->smi_bus == NULL)
> - goto out_unmap;
> -
> - msp->smi_bus->priv = msp;
> - msp->smi_bus->name = "mv643xx_eth smi";
> - msp->smi_bus->read = smi_bus_read;
> - msp->smi_bus->write = smi_bus_write,
> - snprintf(msp->smi_bus->id, MII_BUS_ID_SIZE, "%s-%d",
> - pdev->name, pdev->id);
> - msp->smi_bus->parent = &pdev->dev;
> - msp->smi_bus->phy_mask = 0xffffffff;
> - if (mdiobus_register(msp->smi_bus) < 0)
> - goto out_free_mii_bus;
> - msp->smi = msp;
> - } else {
> - msp->smi = platform_get_drvdata(pd->shared_smi);
> - }
> -
> - msp->err_interrupt = NO_IRQ;
> - init_waitqueue_head(&msp->smi_busy_wait);
> -
> - /*
> - * Check whether the error interrupt is hooked up.
> - */
> - res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> - if (res != NULL) {
> - int err;
> -
> - err = request_irq(res->start, mv643xx_eth_err_irq,
> - IRQF_SHARED, "mv643xx_eth", msp);
> - if (!err) {
> - writel(ERR_INT_SMI_DONE, msp->base + ERR_INT_MASK);
> - msp->err_interrupt = res->start;
> - }
> - }
> -
> - /*
> * (Re-)program MBUS remapping windows if we are asked to.
> */
> dram = mv_mbus_dram_info();
> @@ -2743,10 +2584,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
>
> return 0;
>
> -out_free_mii_bus:
> - mdiobus_free(msp->smi_bus);
> -out_unmap:
> - iounmap(msp->base);
> out_free:
> kfree(msp);
> out:
> @@ -2756,14 +2593,7 @@ out:
> static int mv643xx_eth_shared_remove(struct platform_device *pdev)
> {
> struct mv643xx_eth_shared_private *msp = platform_get_drvdata(pdev);
> - struct mv643xx_eth_shared_platform_data *pd = pdev->dev.platform_data;
>
> - if (pd == NULL || pd->shared_smi == NULL) {
> - mdiobus_unregister(msp->smi_bus);
> - mdiobus_free(msp->smi_bus);
> - }
> - if (msp->err_interrupt != NO_IRQ)
> - free_irq(msp->err_interrupt, msp);
> iounmap(msp->base);
> kfree(msp);
>
> @@ -2829,11 +2659,11 @@ static void set_params(struct mv643xx_eth_private *mp,
> static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
> int phy_addr)
> {
> - struct mii_bus *bus = mp->shared->smi->smi_bus;
> struct phy_device *phydev;
> int start;
> int num;
> int i;
> + char phy_id[MII_BUS_ID_SIZE + 3];
>
> if (phy_addr == MV643XX_ETH_PHY_ADDR_DEFAULT) {
> start = phy_addr_get(mp) & 0x1f;
> @@ -2843,17 +2673,19 @@ static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
> num = 1;
> }
>
> + /* Attempt to connect to the PHY using orion-mdio */
> phydev = NULL;
> for (i = 0; i < num; i++) {
> int addr = (start + i) & 0x1f;
>
> - if (bus->phy_map[addr] == NULL)
> - mdiobus_scan(bus, addr);
> + snprintf(phy_id, sizeof(phy_id), PHY_ID_FMT,
> + "orion-mdio-mii", addr);
>
> - if (phydev == NULL) {
> - phydev = bus->phy_map[addr];
> - if (phydev != NULL)
> - phy_addr_set(mp, addr);
> + phydev = phy_connect(mp->dev, phy_id, NULL,
> + PHY_INTERFACE_MODE_GMII);
> + if (!IS_ERR(phydev)) {
> + phy_addr_set(mp, addr);
> + break;
> }
> }
>
> diff --git a/include/linux/mv643xx_eth.h b/include/linux/mv643xx_eth.h
> index 49258e0..141d395 100644
> --- a/include/linux/mv643xx_eth.h
> +++ b/include/linux/mv643xx_eth.h
> @@ -19,7 +19,6 @@
>
> struct mv643xx_eth_shared_platform_data {
> struct mbus_dram_target_info *dram;
> - struct platform_device *shared_smi;
> /*
> * Max packet size for Tx IP/Layer 4 checksum, when set to 0, default
> * limit of 9KiB will be used.
> --
> 1.7.10.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

2013-03-14 19:33:05

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH] mv643xx_eth: convert to use the Marvell Orion MDIO driver

Hello Jason,

Le 14/03/2013 20:02, Jason Cooper a ?crit :
> Florian,
>
> On Thu, Mar 14, 2013 at 07:08:32PM +0100, Florian Fainelli wrote:
>> This patch converts the Marvell MV643XX ethernet driver to use the
>> Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
>> registering the Marvell MV643XX ethernet driver are also updated to
>> register a Marvell Orion MDIO driver. This driver voluntarily overlaps
>> with the Marvell Ethernet shared registers because it will use a subset
>> of this shared register (shared_base + 0x4 - shared_base + 0x84). The
>> Ethernet driver is also updated to look up for a PHY device using the
>> Orion MDIO bus driver.
>>
>> Signed-off-by: Florian Fainelli <[email protected]>
>> ---
>> arch/arm/plat-orion/common.c | 97 +++++++++++----
>> arch/powerpc/platforms/chrp/pegasos_eth.c | 20 +++
>> arch/powerpc/sysdev/mv64x60_dev.c | 14 ++-
>> drivers/net/ethernet/marvell/Kconfig | 1 +
>> drivers/net/ethernet/marvell/mv643xx_eth.c | 186 ++--------------------------
>> include/linux/mv643xx_eth.h | 1 -
>> 6 files changed, 117 insertions(+), 202 deletions(-)
>
> Isn't this the old version of 4/4 ?

No this really is v2, but I reformatted just that one and sent it
without the v2 changelog, you should have received the proper version a
couple minutes later once I realized my mistake.
--
Florian

2013-03-14 20:36:55

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH] mv643xx_eth: convert to use the Marvell Orion MDIO driver

On Thu, Mar 14, 2013 at 08:32:57PM +0100, Florian Fainelli wrote:
> Hello Jason,
>
> Le 14/03/2013 20:02, Jason Cooper a ?crit :
> >Florian,
> >
> >On Thu, Mar 14, 2013 at 07:08:32PM +0100, Florian Fainelli wrote:
> >>This patch converts the Marvell MV643XX ethernet driver to use the
> >>Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
> >>registering the Marvell MV643XX ethernet driver are also updated to
> >>register a Marvell Orion MDIO driver. This driver voluntarily overlaps
> >>with the Marvell Ethernet shared registers because it will use a subset
> >>of this shared register (shared_base + 0x4 - shared_base + 0x84). The
> >>Ethernet driver is also updated to look up for a PHY device using the
> >>Orion MDIO bus driver.
> >>
> >>Signed-off-by: Florian Fainelli <[email protected]>
> >>---
> >> arch/arm/plat-orion/common.c | 97 +++++++++++----
> >> arch/powerpc/platforms/chrp/pegasos_eth.c | 20 +++
> >> arch/powerpc/sysdev/mv64x60_dev.c | 14 ++-
> >> drivers/net/ethernet/marvell/Kconfig | 1 +
> >> drivers/net/ethernet/marvell/mv643xx_eth.c | 186 ++--------------------------
> >> include/linux/mv643xx_eth.h | 1 -
> >> 6 files changed, 117 insertions(+), 202 deletions(-)
> >
> >Isn't this the old version of 4/4 ?
>
> No this really is v2, but I reformatted just that one and sent it
> without the v2 changelog, you should have received the proper
> version a couple minutes later once I realized my mistake.

Yes, I received both, just making sure.

thx,

Jason.

2013-03-15 11:10:51

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 4/4 v2] mv643xx_eth: convert to use the Marvell Orion MDIO driver

Le 03/14/13 19:08, Florian Fainelli a écrit :
> This patch converts the Marvell MV643XX ethernet driver to use the
> Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
> registering the Marvell MV643XX ethernet driver are also updated to
> register a Marvell Orion MDIO driver. This driver voluntarily overlaps
> with the Marvell Ethernet shared registers because it will use a subset
> of this shared register (shared_base + 0x4 - shared_base + 0x84). The
> Ethernet driver is also updated to look up for a PHY device using the
> Orion MDIO bus driver.

Thanks to the help of Andrew Lunn, there is at least two known issues
with this patch version:

- we need to move up the mvmdio line in
drivers/net/ethernet/marvell/Makefile to make sure that configs having
both mvmdio and mv643xx_eth built-in get the probing order right
- the bus name used by mv643xx_eth is not the right now (orion-mdio.0 vs
expected orion-mdio) so the PHY device will not be found during
phy_connect()

I will fix these two issues in the next version of the patchset.
--
Florian

2013-03-15 11:42:17

by Thomas Petazzoni

[permalink] [raw]
Subject: Re: [PATCH 4/4 v2] mv643xx_eth: convert to use the Marvell Orion MDIO driver

Dear Florian Fainelli,

On Fri, 15 Mar 2013 12:07:12 +0100, Florian Fainelli wrote:

> Thanks to the help of Andrew Lunn, there is at least two known issues
> with this patch version:
>
> - we need to move up the mvmdio line in
> drivers/net/ethernet/marvell/Makefile to make sure that configs having
> both mvmdio and mv643xx_eth built-in get the probing order right

I don't think it's the right way of fixing the problem. If there is no
dependency on the two devices through the device model (i.e they don't
have a parent->child relationship), then the mv643xx_eth driver should
probably return -EPROBE_DEFER when it can't find its PHY so that its
->probe() operation gets called once again by the kernel when other
drivers (including mvmdio) have been probed.

Best regards,

Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

2013-03-15 12:53:10

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 0/4] mv643xx_eth: use mvmdio MDIO bus driver

From: Florian Fainelli <[email protected]>
Date: Thu, 14 Mar 2013 19:08:31 +0100

> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
> instead of rolling its own implementation. As a result, all users of this
> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
> The mvmdio driver is also updated to support an interrupt line which reports
> SMI error/completion, and to allow traditionnal platform device registration
> instead of just device tree.
>
> David, I think it makes sense for you to merge all of this, since we do
> not want the architecture files to be desynchronized from the mv643xx_eth to
> avoid runtime breakage. The potential for merge conflicts should be very small.

All applied to net-next, thanks.

2013-03-15 12:54:38

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 0/4] mv643xx_eth: use mvmdio MDIO bus driver

Le 03/15/13 13:53, David Miller a écrit :
> From: Florian Fainelli <[email protected]>
> Date: Thu, 14 Mar 2013 19:08:31 +0100
>
>> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
>> instead of rolling its own implementation. As a result, all users of this
>> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
>> The mvmdio driver is also updated to support an interrupt line which reports
>> SMI error/completion, and to allow traditionnal platform device registration
>> instead of just device tree.
>>
>> David, I think it makes sense for you to merge all of this, since we do
>> not want the architecture files to be desynchronized from the mv643xx_eth to
>> avoid runtime breakage. The potential for merge conflicts should be very small.
>
> All applied to net-next, thanks.
>

Oh woah that was fast, maybe too fast, I will submit a follow-up patch
for patch 4 to address the issues I mentionned earlier.
--
Florian

2013-03-15 12:54:59

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 0/4] mv643xx_eth: use mvmdio MDIO bus driver

From: David Miller <[email protected]>
Date: Fri, 15 Mar 2013 08:53:21 -0400 (EDT)

> From: Florian Fainelli <[email protected]>
> Date: Thu, 14 Mar 2013 19:08:31 +0100
>
>> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
>> instead of rolling its own implementation. As a result, all users of this
>> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
>> The mvmdio driver is also updated to support an interrupt line which reports
>> SMI error/completion, and to allow traditionnal platform device registration
>> instead of just device tree.
>>
>> David, I think it makes sense for you to merge all of this, since we do
>> not want the architecture files to be desynchronized from the mv643xx_eth to
>> avoid runtime breakage. The potential for merge conflicts should be very small.
>
> All applied to net-next, thanks.

Actually, reverted. Please send me code which actually compiles:

drivers/net/ethernet/marvell/mvmdio.c: In function ?orion_mdio_wait_ready?:
drivers/net/ethernet/marvell/mvmdio.c:70:28: error: ?NO_IRQ? undeclared (first use in this function)
drivers/net/ethernet/marvell/mvmdio.c:70:28: note: each undeclared identifier is reported only once for each function it appears in
drivers/net/ethernet/marvell/mvmdio.c: In function ?orion_mdio_probe?:
drivers/net/ethernet/marvell/mvmdio.c:242:24: error: ?NO_IRQ? undeclared (first use in this function)
make[4]: *** [drivers/net/ethernet/marvell/mvmdio.o] Error 1

And don't use Kconfig dependencies to work around this, fix it properly.
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-03-15 12:56:48

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 0/4] mv643xx_eth: use mvmdio MDIO bus driver

Le 03/15/13 13:55, David Miller a écrit :
> From: David Miller <[email protected]>
> Date: Fri, 15 Mar 2013 08:53:21 -0400 (EDT)
>
>> From: Florian Fainelli <[email protected]>
>> Date: Thu, 14 Mar 2013 19:08:31 +0100
>>
>>> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
>>> instead of rolling its own implementation. As a result, all users of this
>>> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
>>> The mvmdio driver is also updated to support an interrupt line which reports
>>> SMI error/completion, and to allow traditionnal platform device registration
>>> instead of just device tree.
>>>
>>> David, I think it makes sense for you to merge all of this, since we do
>>> not want the architecture files to be desynchronized from the mv643xx_eth to
>>> avoid runtime breakage. The potential for merge conflicts should be very small.
>>
>> All applied to net-next, thanks.
>
> Actually, reverted. Please send me code which actually compiles:
>
> drivers/net/ethernet/marvell/mvmdio.c: In function ‘orion_mdio_wait_ready’:
> drivers/net/ethernet/marvell/mvmdio.c:70:28: error: ‘NO_IRQ’ undeclared (first use in this function)
> drivers/net/ethernet/marvell/mvmdio.c:70:28: note: each undeclared identifier is reported only once for each function it appears in
> drivers/net/ethernet/marvell/mvmdio.c: In function ‘orion_mdio_probe’:
> drivers/net/ethernet/marvell/mvmdio.c:242:24: error: ‘NO_IRQ’ undeclared (first use in this function)
> make[4]: *** [drivers/net/ethernet/marvell/mvmdio.o] Error 1
>
> And don't use Kconfig dependencies to work around this, fix it properly.

Is there any platform out there for which we do not have a NO_IRQ
definition by now? If so, what is it?
--
Florian

2013-03-15 13:05:07

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 0/4] mv643xx_eth: use mvmdio MDIO bus driver

From: Florian Fainelli <[email protected]>
Date: Fri, 15 Mar 2013 13:53:10 +0100

> Le 03/15/13 13:55, David Miller a écrit :
>> From: David Miller <[email protected]>
>> Date: Fri, 15 Mar 2013 08:53:21 -0400 (EDT)
>>
>>> From: Florian Fainelli <[email protected]>
>>> Date: Thu, 14 Mar 2013 19:08:31 +0100
>>>
>>>> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus
>>>> driver
>>>> instead of rolling its own implementation. As a result, all users of
>>>> this
>>>> mv643xx_eth driver are converted to register an "orion-mdio"
>>>> platform_device.
>>>> The mvmdio driver is also updated to support an interrupt line which
>>>> reports
>>>> SMI error/completion, and to allow traditionnal platform device
>>>> registration
>>>> instead of just device tree.
>>>>
>>>> David, I think it makes sense for you to merge all of this, since we
>>>> do
>>>> not want the architecture files to be desynchronized from the
>>>> mv643xx_eth to
>>>> avoid runtime breakage. The potential for merge conflicts should be
>>>> very small.
>>>
>>> All applied to net-next, thanks.
>>
>> Actually, reverted. Please send me code which actually compiles:
>>
>> drivers/net/ethernet/marvell/mvmdio.c: In function
>> ‘orion_mdio_wait_ready’:
>> drivers/net/ethernet/marvell/mvmdio.c:70:28: error: ‘NO_IRQ’
>> undeclared (first use in this function)
>> drivers/net/ethernet/marvell/mvmdio.c:70:28: note: each undeclared
>> identifier is reported only once for each function it appears in
>> drivers/net/ethernet/marvell/mvmdio.c: In function ‘orion_mdio_probe’:
>> drivers/net/ethernet/marvell/mvmdio.c:242:24: error: ‘NO_IRQ’
>> undeclared (first use in this function)
>> make[4]: *** [drivers/net/ethernet/marvell/mvmdio.o] Error 1
>>
>> And don't use Kconfig dependencies to work around this, fix it
>> properly.
>
> Is there any platform out there for which we do not have a NO_IRQ
> definition by now? If so, what is it?

Obviously if x86_64 doesn't even build your changes, that is one such
platform. Also, is grep not working on your computer?

Platforms are absolutely no required to have this define, zero is the
only valid "no IRQ" which is portable in any way.

This is an old and tired topic, portable code does not use NO_IRQ, and
that's simply the end of it.
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-03-15 13:06:48

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 0/4] mv643xx_eth: use mvmdio MDIO bus driver

Le 03/15/13 14:05, David Miller a écrit :
> From: Florian Fainelli <[email protected]>
> Date: Fri, 15 Mar 2013 13:53:10 +0100
>
>> Le 03/15/13 13:55, David Miller a écrit :
>>> From: David Miller <[email protected]>
>>> Date: Fri, 15 Mar 2013 08:53:21 -0400 (EDT)
>>>
>>>> From: Florian Fainelli <[email protected]>
>>>> Date: Thu, 14 Mar 2013 19:08:31 +0100
>>>>
>>>>> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus
>>>>> driver
>>>>> instead of rolling its own implementation. As a result, all users of
>>>>> this
>>>>> mv643xx_eth driver are converted to register an "orion-mdio"
>>>>> platform_device.
>>>>> The mvmdio driver is also updated to support an interrupt line which
>>>>> reports
>>>>> SMI error/completion, and to allow traditionnal platform device
>>>>> registration
>>>>> instead of just device tree.
>>>>>
>>>>> David, I think it makes sense for you to merge all of this, since we
>>>>> do
>>>>> not want the architecture files to be desynchronized from the
>>>>> mv643xx_eth to
>>>>> avoid runtime breakage. The potential for merge conflicts should be
>>>>> very small.
>>>>
>>>> All applied to net-next, thanks.
>>>
>>> Actually, reverted. Please send me code which actually compiles:
>>>
>>> drivers/net/ethernet/marvell/mvmdio.c: In function
>>> ‘orion_mdio_wait_ready’:
>>> drivers/net/ethernet/marvell/mvmdio.c:70:28: error: ‘NO_IRQ’
>>> undeclared (first use in this function)
>>> drivers/net/ethernet/marvell/mvmdio.c:70:28: note: each undeclared
>>> identifier is reported only once for each function it appears in
>>> drivers/net/ethernet/marvell/mvmdio.c: In function ‘orion_mdio_probe’:
>>> drivers/net/ethernet/marvell/mvmdio.c:242:24: error: ‘NO_IRQ’
>>> undeclared (first use in this function)
>>> make[4]: *** [drivers/net/ethernet/marvell/mvmdio.o] Error 1
>>>
>>> And don't use Kconfig dependencies to work around this, fix it
>>> properly.
>>
>> Is there any platform out there for which we do not have a NO_IRQ
>> definition by now? If so, what is it?
>
> Obviously if x86_64 doesn't even build your changes, that is one such
> platform. Also, is grep not working on your computer?

I built tested on PowerPC and ARM which are the platforms actually
*using* these drivers and forgot that you build for x86_64.

>
> Platforms are absolutely no required to have this define, zero is the
> only valid "no IRQ" which is portable in any way.
>
> This is an old and tired topic, portable code does not use NO_IRQ, and
> that's simply the end of it.

I changed not to rely on NO_IRQ anymore. Thanks!
--
Florian

2013-03-15 18:06:31

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 3/4 v2] net: mvmdio: enhance driver to support SMI error/done interrupts

On Thu, Mar 14, 2013 at 07:08:34PM +0100, Florian Fainelli wrote:
> + if (dev->err_interrupt == NO_IRQ) {
...
> + init_waitqueue_head(&dev->smi_busy_wait);
> +
> + dev->err_interrupt = platform_get_irq(pdev, 0);
> + if (dev->err_interrupt != -ENXIO) {
...
> + } else
> + dev->err_interrupt = NO_IRQ;


FYI, NO_IRQ is not supposed to be used anymore (we're supposed to be
removing it). platform_get_irq() returns negative numbers for failure,
so why not test for < 0 in both the above tests, or maybe <= 0 (as
IRQ0 is also not supposed to be valid.)?

2013-03-22 13:39:43

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH 1/4 v3] net: mvmdio: allow platform device style registration

This patch changes the mvmdio driver not to use device tree
helper functions such as of_mdiobus_register() and of_iomap() so we can
instantiate this driver using a classic platform_device approach. Use
the device manager helper to ioremap() the base register cookie so we
get automatic freeing upon error and removal. This change is harmless
for Device Tree platforms because they will get the driver be registered
the same way as it was before.

Signed-off-by: Florian Fainelli <[email protected]>
---
Changes since v2:
- use of_mdiobus_register if device tree node is available to ensure that
all child PHY nodes get also registered (fixes PHY probing on DT-only)

Changes since v1:
- use device managed helpers to fix an ioremap leak in remove function
- remove the use of Device Tree specific helpers to allow platform-style
registration

drivers/net/ethernet/marvell/mvmdio.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 77b7c80..bbc5fde 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -24,10 +24,10 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/phy.h>
-#include <linux/of_address.h>
-#include <linux/of_mdio.h>
#include <linux/platform_device.h>
#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of_mdio.h>

#define MVMDIO_SMI_DATA_SHIFT 0
#define MVMDIO_SMI_PHY_ADDR_SHIFT 16
@@ -143,11 +143,17 @@ static int orion_mdio_reset(struct mii_bus *bus)

static int orion_mdio_probe(struct platform_device *pdev)
{
- struct device_node *np = pdev->dev.of_node;
+ struct resource *r;
struct mii_bus *bus;
struct orion_mdio_dev *dev;
int i, ret;

+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!r) {
+ dev_err(&pdev->dev, "No SMI register address given\n");
+ return -ENODEV;
+ }
+
bus = mdiobus_alloc_size(sizeof(struct orion_mdio_dev));
if (!bus) {
dev_err(&pdev->dev, "Cannot allocate MDIO bus\n");
@@ -172,9 +178,9 @@ static int orion_mdio_probe(struct platform_device *pdev)
bus->irq[i] = PHY_POLL;

dev = bus->priv;
- dev->smireg = of_iomap(pdev->dev.of_node, 0);
+ dev->smireg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
if (!dev->smireg) {
- dev_err(&pdev->dev, "No SMI register address given in DT\n");
+ dev_err(&pdev->dev, "Unable to remap SMI register\n");
kfree(bus->irq);
mdiobus_free(bus);
return -ENODEV;
@@ -182,10 +188,12 @@ static int orion_mdio_probe(struct platform_device *pdev)

mutex_init(&dev->lock);

- ret = of_mdiobus_register(bus, np);
+ if (pdev->dev.of_node)
+ ret = of_mdiobus_register(bus, pdev->dev.of_node);
+ else
+ ret = mdiobus_register(bus);
if (ret < 0) {
dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
- iounmap(dev->smireg);
kfree(bus->irq);
mdiobus_free(bus);
return ret;
--
1.7.10.4

2013-03-22 13:40:32

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH 2/4 v3] net: mvmdio: rename base register cookie from smireg to regs

This patch renames the base register cookie in the mvmdio drive from
"smireg" to "regs" since a subsequent patch is going to use an ioremap()
cookie whose size is larger than a single register of 4 bytes. No
functionnal code change introduced.

Acked-by: Thomas Petazzoni <[email protected]>
Signed-off-by: Florian Fainelli <[email protected]>
---
Changes since v2:
- rebased against latest version

Changes since v1:
- added Thomas Acked-by tag

drivers/net/ethernet/marvell/mvmdio.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index bbc5fde..3e2711d 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -39,7 +39,7 @@

struct orion_mdio_dev {
struct mutex lock;
- void __iomem *smireg;
+ void __iomem *regs;
};

/* Wait for the SMI unit to be ready for another operation
@@ -52,7 +52,7 @@ static int orion_mdio_wait_ready(struct mii_bus *bus)

count = 0;
while (1) {
- val = readl(dev->smireg);
+ val = readl(dev->regs);
if (!(val & MVMDIO_SMI_BUSY))
break;

@@ -87,12 +87,12 @@ static int orion_mdio_read(struct mii_bus *bus, int mii_id,
writel(((mii_id << MVMDIO_SMI_PHY_ADDR_SHIFT) |
(regnum << MVMDIO_SMI_PHY_REG_SHIFT) |
MVMDIO_SMI_READ_OPERATION),
- dev->smireg);
+ dev->regs);

/* Wait for the value to become available */
count = 0;
while (1) {
- val = readl(dev->smireg);
+ val = readl(dev->regs);
if (val & MVMDIO_SMI_READ_VALID)
break;

@@ -129,7 +129,7 @@ static int orion_mdio_write(struct mii_bus *bus, int mii_id,
(regnum << MVMDIO_SMI_PHY_REG_SHIFT) |
MVMDIO_SMI_WRITE_OPERATION |
(value << MVMDIO_SMI_DATA_SHIFT)),
- dev->smireg);
+ dev->regs);

mutex_unlock(&dev->lock);

@@ -178,8 +178,8 @@ static int orion_mdio_probe(struct platform_device *pdev)
bus->irq[i] = PHY_POLL;

dev = bus->priv;
- dev->smireg = devm_ioremap(&pdev->dev, r->start, resource_size(r));
- if (!dev->smireg) {
+ dev->regs = devm_ioremap(&pdev->dev, r->start, resource_size(r));
+ if (!dev->regs) {
dev_err(&pdev->dev, "Unable to remap SMI register\n");
kfree(bus->irq);
mdiobus_free(bus);
--
1.7.10.4

2013-03-22 13:40:27

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH 0/4 v3] mv643xx_eth: use mvmdio MDIO bus driver

Hi all,

This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
instead of rolling its own implementation. As a result, all users of this
mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
The mvmdio driver is also updated to support an interrupt line which reports
SMI error/completion, and to allow traditionnal platform device registration
instead of just device tree.

David, I think it makes sense for you to merge all of this, since we do
not want the architecture files to be desynchronized from the mv643xx_eth to
avoid runtime breakage. The potential for merge conflicts should be very small.

Florian Fainelli (4):
net: mvmdio: allow platform device style registration
net: mvmdio: rename base register cookie from smireg to regs
net: mvmdio: enhance driver to support SMI error/done interrupts
mv643xx_eth: convert to use the Marvell Orion MDIO driver

.../devicetree/bindings/net/marvell-orion-mdio.txt | 3 +
arch/arm/plat-orion/common.c | 54 +++---
arch/powerpc/platforms/chrp/pegasos_eth.c | 20 ++
arch/powerpc/sysdev/mv64x60_dev.c | 16 +-
drivers/net/ethernet/marvell/Kconfig | 5 +-
drivers/net/ethernet/marvell/Makefile | 2 +-
drivers/net/ethernet/marvell/mv643xx_eth.c | 195 ++------------------
drivers/net/ethernet/marvell/mvmdio.c | 130 ++++++++++---
include/linux/mv643xx_eth.h | 1 -
9 files changed, 187 insertions(+), 239 deletions(-)

--
1.7.10.4

2013-03-22 13:40:25

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH 3/4 v3] net: mvmdio: enhance driver to support SMI error/done interrupts

This patch enhances the "mvmdio" to support a SMI error/done interrupt
line which can be used along with a wait queue instead of doing
busy-waiting on the registers. This is a feature which is available in
the mv643xx_eth SMI code and thus reduces again the gap between the two.

Signed-off-by: Florian Fainelli <[email protected]>
---
Changes since v2:
- remove NO_IRQ usage which is not portable, check for irq >= 0
instead
- factor out mdiobus cleaning under the out_mdio label in probe()

Chances since v1:
- always use orion_smi_is_done() helper
- make interrupt/non-interrupt code path entirely independant

.../devicetree/bindings/net/marvell-orion-mdio.txt | 3 +
drivers/net/ethernet/marvell/mvmdio.c | 98 ++++++++++++++++----
2 files changed, 83 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
index 34e7aaf..052b5f2 100644
--- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
+++ b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
@@ -9,6 +9,9 @@ Required properties:
- compatible: "marvell,orion-mdio"
- reg: address and length of the SMI register

+Optional properties:
+- interrupts: interrupt line number for the SMI error/done interrupt
+
The child nodes of the MDIO driver are the individual PHY devices
connected to this MDIO bus. They must have a "reg" property given the
PHY address on the MDIO bus.
diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 3e2711d..3472574 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -24,10 +24,13 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/phy.h>
+#include <linux/interrupt.h>
#include <linux/platform_device.h>
#include <linux/delay.h>
#include <linux/io.h>
#include <linux/of_mdio.h>
+#include <linux/sched.h>
+#include <linux/wait.h>

#define MVMDIO_SMI_DATA_SHIFT 0
#define MVMDIO_SMI_PHY_ADDR_SHIFT 16
@@ -36,33 +39,58 @@
#define MVMDIO_SMI_WRITE_OPERATION 0
#define MVMDIO_SMI_READ_VALID BIT(27)
#define MVMDIO_SMI_BUSY BIT(28)
+#define MVMDIO_ERR_INT_CAUSE 0x007C
+#define MVMDIO_ERR_INT_SMI_DONE 0x00000010
+#define MVMDIO_ERR_INT_MASK 0x0080

struct orion_mdio_dev {
struct mutex lock;
void __iomem *regs;
+ /*
+ * If we have access to the error interrupt pin (which is
+ * somewhat misnamed as it not only reflects internal errors
+ * but also reflects SMI completion), use that to wait for
+ * SMI access completion instead of polling the SMI busy bit.
+ */
+ int err_interrupt;
+ wait_queue_head_t smi_busy_wait;
};

+static int orion_mdio_smi_is_done(struct orion_mdio_dev *dev)
+{
+ return !(readl(dev->regs) & MVMDIO_SMI_BUSY);
+}
+
/* Wait for the SMI unit to be ready for another operation
*/
static int orion_mdio_wait_ready(struct mii_bus *bus)
{
struct orion_mdio_dev *dev = bus->priv;
int count;
- u32 val;

- count = 0;
- while (1) {
- val = readl(dev->regs);
- if (!(val & MVMDIO_SMI_BUSY))
- break;
+ if (dev->err_interrupt <= 0) {
+ count = 0;
+ while (1) {
+ if (orion_mdio_smi_is_done(dev))
+ break;

- if (count > 100) {
- dev_err(bus->parent, "Timeout: SMI busy for too long\n");
- return -ETIMEDOUT;
- }
+ if (count > 100) {
+ dev_err(bus->parent,
+ "Timeout: SMI busy for too long\n");
+ return -ETIMEDOUT;
+ }

- udelay(10);
- count++;
+ udelay(10);
+ count++;
+ }
+ } else {
+ if (!orion_mdio_smi_is_done(dev)) {
+ wait_event_timeout(dev->smi_busy_wait,
+ orion_mdio_smi_is_done(dev),
+ msecs_to_jiffies(100));
+ if (!orion_mdio_smi_is_done(dev))
+ return -ETIMEDOUT;
+ }
}

return 0;
@@ -141,6 +169,21 @@ static int orion_mdio_reset(struct mii_bus *bus)
return 0;
}

+static irqreturn_t orion_mdio_err_irq(int irq, void *dev_id)
+{
+ struct orion_mdio_dev *dev = dev_id;
+
+ if (readl(dev->regs + MVMDIO_ERR_INT_CAUSE) &
+ MVMDIO_ERR_INT_SMI_DONE) {
+ writel(~MVMDIO_ERR_INT_SMI_DONE,
+ dev->regs + MVMDIO_ERR_INT_CAUSE);
+ wake_up(&dev->smi_busy_wait);
+ return IRQ_HANDLED;
+ }
+
+ return IRQ_NONE;
+}
+
static int orion_mdio_probe(struct platform_device *pdev)
{
struct resource *r;
@@ -181,9 +224,22 @@ static int orion_mdio_probe(struct platform_device *pdev)
dev->regs = devm_ioremap(&pdev->dev, r->start, resource_size(r));
if (!dev->regs) {
dev_err(&pdev->dev, "Unable to remap SMI register\n");
- kfree(bus->irq);
- mdiobus_free(bus);
- return -ENODEV;
+ ret = -ENODEV;
+ goto out_mdio;
+ }
+
+ init_waitqueue_head(&dev->smi_busy_wait);
+
+ dev->err_interrupt = platform_get_irq(pdev, 0);
+ if (dev->err_interrupt != -ENXIO) {
+ ret = devm_request_irq(&pdev->dev, dev->err_interrupt,
+ orion_mdio_err_irq,
+ IRQF_SHARED, pdev->name, dev);
+ if (ret)
+ goto out_mdio;
+
+ writel(MVMDIO_ERR_INT_SMI_DONE,
+ dev->regs + MVMDIO_ERR_INT_MASK);
}

mutex_init(&dev->lock);
@@ -194,19 +250,25 @@ static int orion_mdio_probe(struct platform_device *pdev)
ret = mdiobus_register(bus);
if (ret < 0) {
dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
- kfree(bus->irq);
- mdiobus_free(bus);
- return ret;
+ goto out_mdio;
}

platform_set_drvdata(pdev, bus);

return 0;
+
+out_mdio:
+ kfree(bus->irq);
+ mdiobus_free(bus);
+ return ret;
}

static int orion_mdio_remove(struct platform_device *pdev)
{
struct mii_bus *bus = platform_get_drvdata(pdev);
+ struct orion_mdio_dev *dev = bus->priv;
+
+ writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
mdiobus_unregister(bus);
kfree(bus->irq);
mdiobus_free(bus);
--
1.7.10.4

2013-03-22 13:40:23

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH 4/4 v3] mv643xx_eth: convert to use the Marvell Orion MDIO driver

This patch converts the Marvell MV643XX ethernet driver to use the
Marvell Orion MDIO driver. As a result, PowerPC and ARM platforms
registering the Marvell MV643XX ethernet driver are also updated to
register a Marvell Orion MDIO driver. This driver voluntarily overlaps
with the Marvell Ethernet shared registers because it will use a subset
of this shared register (shared_base + 0x4 to shared_base + 0x84). The
Ethernet driver is also updated to look up for a PHY device using the
Orion MDIO bus driver.

For ARM and PowerPC we register a single instance of the "mvmdio" driver
in the system like it used to be done with the use of the "shared_smi"
platform_data cookie on ARM.

Note that it is safe to register the mvmdio driver only for the "ge00"
instance of the driver because this "ge00" interface is guaranteed to
always be explicitely registered by consumers of
arch/arm/plat-orion/common.c and other instances (ge01, ge10 and ge11)
were all pointing their shared_smi to ge00. For PowerPC the in-tree
Device Tree Source files mention only one MV643XX ethernet MAC instance
so the MDIO bus driver is registered only when id == 0.

Signed-off-by: Florian Fainelli <[email protected]>
---
Changes since v2:
- added missing adjust_link callback to phy_connect
- removed superfluous phy_attach() call
- modified PowerPC code to register a single orion-mdio instance
- modified arm code to register a single orion-mdio instance for
all GE* interfaces

Changes since v1:
- removed one too many mdio bus registration for Orion5x/Kirkwood


arch/arm/plat-orion/common.c | 54 ++++----
arch/powerpc/platforms/chrp/pegasos_eth.c | 20 +++
arch/powerpc/sysdev/mv64x60_dev.c | 16 ++-
drivers/net/ethernet/marvell/Kconfig | 5 +-
drivers/net/ethernet/marvell/Makefile | 2 +-
drivers/net/ethernet/marvell/mv643xx_eth.c | 195 +++-------------------------
include/linux/mv643xx_eth.h | 1 -
7 files changed, 84 insertions(+), 209 deletions(-)

diff --git a/arch/arm/plat-orion/common.c b/arch/arm/plat-orion/common.c
index 2d4b641..251f827 100644
--- a/arch/arm/plat-orion/common.c
+++ b/arch/arm/plat-orion/common.c
@@ -238,6 +238,7 @@ static __init void ge_complete(
struct mv643xx_eth_shared_platform_data *orion_ge_shared_data,
struct resource *orion_ge_resource, unsigned long irq,
struct platform_device *orion_ge_shared,
+ struct platform_device *orion_ge_mvmdio,
struct mv643xx_eth_platform_data *eth_data,
struct platform_device *orion_ge)
{
@@ -247,6 +248,8 @@ static __init void ge_complete(
orion_ge->dev.platform_data = eth_data;

platform_device_register(orion_ge_shared);
+ if (orion_ge_mvmdio)
+ platform_device_register(orion_ge_mvmdio);
platform_device_register(orion_ge);
}

@@ -258,8 +261,6 @@ struct mv643xx_eth_shared_platform_data orion_ge00_shared_data;
static struct resource orion_ge00_shared_resources[] = {
{
.name = "ge00 base",
- }, {
- .name = "ge00 err irq",
},
};

@@ -271,6 +272,19 @@ static struct platform_device orion_ge00_shared = {
},
};

+static struct resource orion_ge_mvmdio_resources[] = {
+ {
+ .name = "ge00 mvmdio base",
+ }, {
+ .name = "ge00 mvmdio err irq",
+ },
+};
+
+static struct platform_device orion_ge_mvmdio = {
+ .name = "orion-mdio",
+ .id = -1,
+};
+
static struct resource orion_ge00_resources[] = {
{
.name = "ge00 irq",
@@ -295,26 +309,25 @@ void __init orion_ge00_init(struct mv643xx_eth_platform_data *eth_data,
unsigned int tx_csum_limit)
{
fill_resources(&orion_ge00_shared, orion_ge00_shared_resources,
- mapbase + 0x2000, SZ_16K - 1, irq_err);
+ mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
+ fill_resources(&orion_ge_mvmdio, orion_ge_mvmdio_resources,
+ mapbase + 0x2004, 0x84 - 1, irq_err);
orion_ge00_shared_data.tx_csum_limit = tx_csum_limit;
ge_complete(&orion_ge00_shared_data,
orion_ge00_resources, irq, &orion_ge00_shared,
+ &orion_ge_mvmdio,
eth_data, &orion_ge00);
}

/*****************************************************************************
* GE01
****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge01_shared_data = {
- .shared_smi = &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge01_shared_data;

static struct resource orion_ge01_shared_resources[] = {
{
.name = "ge01 base",
- }, {
- .name = "ge01 err irq",
- },
+ }
};

static struct platform_device orion_ge01_shared = {
@@ -349,26 +362,23 @@ void __init orion_ge01_init(struct mv643xx_eth_platform_data *eth_data,
unsigned int tx_csum_limit)
{
fill_resources(&orion_ge01_shared, orion_ge01_shared_resources,
- mapbase + 0x2000, SZ_16K - 1, irq_err);
+ mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
orion_ge01_shared_data.tx_csum_limit = tx_csum_limit;
ge_complete(&orion_ge01_shared_data,
orion_ge01_resources, irq, &orion_ge01_shared,
+ NULL,
eth_data, &orion_ge01);
}

/*****************************************************************************
* GE10
****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge10_shared_data = {
- .shared_smi = &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge10_shared_data;

static struct resource orion_ge10_shared_resources[] = {
{
.name = "ge10 base",
- }, {
- .name = "ge10 err irq",
- },
+ }
};

static struct platform_device orion_ge10_shared = {
@@ -402,24 +412,21 @@ void __init orion_ge10_init(struct mv643xx_eth_platform_data *eth_data,
unsigned long irq_err)
{
fill_resources(&orion_ge10_shared, orion_ge10_shared_resources,
- mapbase + 0x2000, SZ_16K - 1, irq_err);
+ mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
ge_complete(&orion_ge10_shared_data,
orion_ge10_resources, irq, &orion_ge10_shared,
+ NULL,
eth_data, &orion_ge10);
}

/*****************************************************************************
* GE11
****************************************************************************/
-struct mv643xx_eth_shared_platform_data orion_ge11_shared_data = {
- .shared_smi = &orion_ge00_shared,
-};
+struct mv643xx_eth_shared_platform_data orion_ge11_shared_data;

static struct resource orion_ge11_shared_resources[] = {
{
.name = "ge11 base",
- }, {
- .name = "ge11 err irq",
},
};

@@ -454,9 +461,10 @@ void __init orion_ge11_init(struct mv643xx_eth_platform_data *eth_data,
unsigned long irq_err)
{
fill_resources(&orion_ge11_shared, orion_ge11_shared_resources,
- mapbase + 0x2000, SZ_16K - 1, irq_err);
+ mapbase + 0x2000, SZ_16K - 1, NO_IRQ);
ge_complete(&orion_ge11_shared_data,
orion_ge11_resources, irq, &orion_ge11_shared,
+ NULL,
eth_data, &orion_ge11);
}

diff --git a/arch/powerpc/platforms/chrp/pegasos_eth.c b/arch/powerpc/platforms/chrp/pegasos_eth.c
index 039fc8e..2b4dc6a 100644
--- a/arch/powerpc/platforms/chrp/pegasos_eth.c
+++ b/arch/powerpc/platforms/chrp/pegasos_eth.c
@@ -47,6 +47,25 @@ static struct platform_device mv643xx_eth_shared_device = {
.resource = mv643xx_eth_shared_resources,
};

+/*
+ * The orion mdio driver only covers shared + 0x4 up to shared + 0x84 - 1
+ */
+static struct resource mv643xx_eth_mvmdio_resources[] = {
+ [0] = {
+ .name = "ethernet mdio base",
+ .start = 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x4,
+ .end = 0xf1000000 + MV643XX_ETH_SHARED_REGS + 0x83,
+ .flags = IORESOURCE_MEM,
+ },
+};
+
+static struct platform_device mv643xx_eth_mvmdio_device = {
+ .name = "orion-mdio",
+ .id = -1,
+ .num_resources = ARRAY_SIZE(mv643xx_eth_mvmdio_resources),
+ .resource = mv643xx_eth_shared_resources,
+};
+
static struct resource mv643xx_eth_port1_resources[] = {
[0] = {
.name = "eth port1 irq",
@@ -82,6 +101,7 @@ static struct platform_device eth_port1_device = {

static struct platform_device *mv643xx_eth_pd_devs[] __initdata = {
&mv643xx_eth_shared_device,
+ &mv643xx_eth_mvmdio_device,
&eth_port1_device,
};

diff --git a/arch/powerpc/sysdev/mv64x60_dev.c b/arch/powerpc/sysdev/mv64x60_dev.c
index 0f6af41..4a25c26 100644
--- a/arch/powerpc/sysdev/mv64x60_dev.c
+++ b/arch/powerpc/sysdev/mv64x60_dev.c
@@ -214,15 +214,27 @@ static struct platform_device * __init mv64x60_eth_register_shared_pdev(
struct device_node *np, int id)
{
struct platform_device *pdev;
- struct resource r[1];
+ struct resource r[2];
int err;

err = of_address_to_resource(np, 0, &r[0]);
if (err)
return ERR_PTR(err);

+ /* register an orion mdio bus driver */
+ r[1].start = r[0].start + 0x4;
+ r[1].end = r[0].start + 0x84 - 1;
+ r[1].flags = IORESOURCE_MEM;
+
+ if (id == 0) {
+ pdev = platform_device_register_simple("orion-mdio", -1, &r[1], 1);
+ if (!pdev)
+ return pdev;
+ }
+
pdev = platform_device_register_simple(MV643XX_ETH_SHARED_NAME, id,
- r, 1);
+ &r[0], 1);
+
return pdev;
}

diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
index edfba93..5170ecb 100644
--- a/drivers/net/ethernet/marvell/Kconfig
+++ b/drivers/net/ethernet/marvell/Kconfig
@@ -23,6 +23,7 @@ config MV643XX_ETH
depends on (MV64X60 || PPC32 || PLAT_ORION) && INET
select INET_LRO
select PHYLIB
+ select MVMDIO
---help---
This driver supports the gigabit ethernet MACs in the
Marvell Discovery PPC/MIPS chipset family (MV643XX) and
@@ -38,9 +39,7 @@ config MVMDIO
interface units of the Marvell EBU SoCs (Kirkwood, Orion5x,
Dove, Armada 370 and Armada XP).

- For now, this driver is only needed for the MVNETA driver
- (used on Armada 370 and XP), but it could be used in the
- future by the MV643XX_ETH driver.
+ This driver is used by the MV643XX_ETH and MVNETA drivers.

config MVNETA
tristate "Marvell Armada 370/XP network interface support"
diff --git a/drivers/net/ethernet/marvell/Makefile b/drivers/net/ethernet/marvell/Makefile
index 7f63b4a..5c4a776 100644
--- a/drivers/net/ethernet/marvell/Makefile
+++ b/drivers/net/ethernet/marvell/Makefile
@@ -2,8 +2,8 @@
# Makefile for the Marvell device drivers.
#

-obj-$(CONFIG_MV643XX_ETH) += mv643xx_eth.o
obj-$(CONFIG_MVMDIO) += mvmdio.o
+obj-$(CONFIG_MV643XX_ETH) += mv643xx_eth.o
obj-$(CONFIG_MVNETA) += mvneta.o
obj-$(CONFIG_PXA168_ETH) += pxa168_eth.o
obj-$(CONFIG_SKGE) += skge.o
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index d1ecf4b..a65a92e 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -69,14 +69,6 @@ static char mv643xx_eth_driver_version[] = "1.4";
* Registers shared between all ports.
*/
#define PHY_ADDR 0x0000
-#define SMI_REG 0x0004
-#define SMI_BUSY 0x10000000
-#define SMI_READ_VALID 0x08000000
-#define SMI_OPCODE_READ 0x04000000
-#define SMI_OPCODE_WRITE 0x00000000
-#define ERR_INT_CAUSE 0x0080
-#define ERR_INT_SMI_DONE 0x00000010
-#define ERR_INT_MASK 0x0084
#define WINDOW_BASE(w) (0x0200 + ((w) << 3))
#define WINDOW_SIZE(w) (0x0204 + ((w) << 3))
#define WINDOW_REMAP_HIGH(w) (0x0280 + ((w) << 2))
@@ -266,25 +258,6 @@ struct mv643xx_eth_shared_private {
void __iomem *base;

/*
- * Points at the right SMI instance to use.
- */
- struct mv643xx_eth_shared_private *smi;
-
- /*
- * Provides access to local SMI interface.
- */
- struct mii_bus *smi_bus;
-
- /*
- * If we have access to the error interrupt pin (which is
- * somewhat misnamed as it not only reflects internal errors
- * but also reflects SMI completion), use that to wait for
- * SMI access completion instead of polling the SMI busy bit.
- */
- int err_interrupt;
- wait_queue_head_t smi_busy_wait;
-
- /*
* Per-port MBUS window access register value.
*/
u32 win_protect;
@@ -1122,97 +1095,6 @@ out_write:
wrlp(mp, PORT_SERIAL_CONTROL, pscr);
}

-static irqreturn_t mv643xx_eth_err_irq(int irq, void *dev_id)
-{
- struct mv643xx_eth_shared_private *msp = dev_id;
-
- if (readl(msp->base + ERR_INT_CAUSE) & ERR_INT_SMI_DONE) {
- writel(~ERR_INT_SMI_DONE, msp->base + ERR_INT_CAUSE);
- wake_up(&msp->smi_busy_wait);
- return IRQ_HANDLED;
- }
-
- return IRQ_NONE;
-}
-
-static int smi_is_done(struct mv643xx_eth_shared_private *msp)
-{
- return !(readl(msp->base + SMI_REG) & SMI_BUSY);
-}
-
-static int smi_wait_ready(struct mv643xx_eth_shared_private *msp)
-{
- if (msp->err_interrupt == NO_IRQ) {
- int i;
-
- for (i = 0; !smi_is_done(msp); i++) {
- if (i == 10)
- return -ETIMEDOUT;
- msleep(10);
- }
-
- return 0;
- }
-
- if (!smi_is_done(msp)) {
- wait_event_timeout(msp->smi_busy_wait, smi_is_done(msp),
- msecs_to_jiffies(100));
- if (!smi_is_done(msp))
- return -ETIMEDOUT;
- }
-
- return 0;
-}
-
-static int smi_bus_read(struct mii_bus *bus, int addr, int reg)
-{
- struct mv643xx_eth_shared_private *msp = bus->priv;
- void __iomem *smi_reg = msp->base + SMI_REG;
- int ret;
-
- if (smi_wait_ready(msp)) {
- pr_warn("SMI bus busy timeout\n");
- return -ETIMEDOUT;
- }
-
- writel(SMI_OPCODE_READ | (reg << 21) | (addr << 16), smi_reg);
-
- if (smi_wait_ready(msp)) {
- pr_warn("SMI bus busy timeout\n");
- return -ETIMEDOUT;
- }
-
- ret = readl(smi_reg);
- if (!(ret & SMI_READ_VALID)) {
- pr_warn("SMI bus read not valid\n");
- return -ENODEV;
- }
-
- return ret & 0xffff;
-}
-
-static int smi_bus_write(struct mii_bus *bus, int addr, int reg, u16 val)
-{
- struct mv643xx_eth_shared_private *msp = bus->priv;
- void __iomem *smi_reg = msp->base + SMI_REG;
-
- if (smi_wait_ready(msp)) {
- pr_warn("SMI bus busy timeout\n");
- return -ETIMEDOUT;
- }
-
- writel(SMI_OPCODE_WRITE | (reg << 21) |
- (addr << 16) | (val & 0xffff), smi_reg);
-
- if (smi_wait_ready(msp)) {
- pr_warn("SMI bus busy timeout\n");
- return -ETIMEDOUT;
- }
-
- return 0;
-}
-
-
/* statistics ***************************************************************/
static struct net_device_stats *mv643xx_eth_get_stats(struct net_device *dev)
{
@@ -2688,47 +2570,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)
goto out_free;

/*
- * Set up and register SMI bus.
- */
- if (pd == NULL || pd->shared_smi == NULL) {
- msp->smi_bus = mdiobus_alloc();
- if (msp->smi_bus == NULL)
- goto out_unmap;
-
- msp->smi_bus->priv = msp;
- msp->smi_bus->name = "mv643xx_eth smi";
- msp->smi_bus->read = smi_bus_read;
- msp->smi_bus->write = smi_bus_write,
- snprintf(msp->smi_bus->id, MII_BUS_ID_SIZE, "%s-%d",
- pdev->name, pdev->id);
- msp->smi_bus->parent = &pdev->dev;
- msp->smi_bus->phy_mask = 0xffffffff;
- if (mdiobus_register(msp->smi_bus) < 0)
- goto out_free_mii_bus;
- msp->smi = msp;
- } else {
- msp->smi = platform_get_drvdata(pd->shared_smi);
- }
-
- msp->err_interrupt = NO_IRQ;
- init_waitqueue_head(&msp->smi_busy_wait);
-
- /*
- * Check whether the error interrupt is hooked up.
- */
- res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (res != NULL) {
- int err;
-
- err = request_irq(res->start, mv643xx_eth_err_irq,
- IRQF_SHARED, "mv643xx_eth", msp);
- if (!err) {
- writel(ERR_INT_SMI_DONE, msp->base + ERR_INT_MASK);
- msp->err_interrupt = res->start;
- }
- }
-
- /*
* (Re-)program MBUS remapping windows if we are asked to.
*/
dram = mv_mbus_dram_info();
@@ -2743,10 +2584,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev)

return 0;

-out_free_mii_bus:
- mdiobus_free(msp->smi_bus);
-out_unmap:
- iounmap(msp->base);
out_free:
kfree(msp);
out:
@@ -2756,14 +2593,7 @@ out:
static int mv643xx_eth_shared_remove(struct platform_device *pdev)
{
struct mv643xx_eth_shared_private *msp = platform_get_drvdata(pdev);
- struct mv643xx_eth_shared_platform_data *pd = pdev->dev.platform_data;

- if (pd == NULL || pd->shared_smi == NULL) {
- mdiobus_unregister(msp->smi_bus);
- mdiobus_free(msp->smi_bus);
- }
- if (msp->err_interrupt != NO_IRQ)
- free_irq(msp->err_interrupt, msp);
iounmap(msp->base);
kfree(msp);

@@ -2826,14 +2656,21 @@ static void set_params(struct mv643xx_eth_private *mp,
mp->txq_count = pd->tx_queue_count ? : 1;
}

+static void mv643xx_eth_adjust_link(struct net_device *dev)
+{
+ struct mv643xx_eth_private *mp = netdev_priv(dev);
+
+ mv643xx_adjust_pscr(mp);
+}
+
static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
int phy_addr)
{
- struct mii_bus *bus = mp->shared->smi->smi_bus;
struct phy_device *phydev;
int start;
int num;
int i;
+ char phy_id[MII_BUS_ID_SIZE + 3];

if (phy_addr == MV643XX_ETH_PHY_ADDR_DEFAULT) {
start = phy_addr_get(mp) & 0x1f;
@@ -2843,17 +2680,19 @@ static struct phy_device *phy_scan(struct mv643xx_eth_private *mp,
num = 1;
}

+ /* Attempt to connect to the PHY using orion-mdio */
phydev = NULL;
for (i = 0; i < num; i++) {
int addr = (start + i) & 0x1f;

- if (bus->phy_map[addr] == NULL)
- mdiobus_scan(bus, addr);
+ snprintf(phy_id, sizeof(phy_id), PHY_ID_FMT,
+ "orion-mdio-mii", addr);

- if (phydev == NULL) {
- phydev = bus->phy_map[addr];
- if (phydev != NULL)
- phy_addr_set(mp, addr);
+ phydev = phy_connect(mp->dev, phy_id, mv643xx_eth_adjust_link,
+ PHY_INTERFACE_MODE_GMII);
+ if (!IS_ERR(phydev)) {
+ phy_addr_set(mp, addr);
+ break;
}
}

@@ -2866,8 +2705,6 @@ static void phy_init(struct mv643xx_eth_private *mp, int speed, int duplex)

phy_reset(mp);

- phy_attach(mp->dev, dev_name(&phy->dev), PHY_INTERFACE_MODE_GMII);
-
if (speed == 0) {
phy->autoneg = AUTONEG_ENABLE;
phy->speed = 0;
diff --git a/include/linux/mv643xx_eth.h b/include/linux/mv643xx_eth.h
index 49258e0..141d395 100644
--- a/include/linux/mv643xx_eth.h
+++ b/include/linux/mv643xx_eth.h
@@ -19,7 +19,6 @@

struct mv643xx_eth_shared_platform_data {
struct mbus_dram_target_info *dram;
- struct platform_device *shared_smi;
/*
* Max packet size for Tx IP/Layer 4 checksum, when set to 0, default
* limit of 9KiB will be used.
--
1.7.10.4

2013-03-22 13:53:43

by Thomas Petazzoni

[permalink] [raw]
Subject: Re: [PATCH 0/4 v3] mv643xx_eth: use mvmdio MDIO bus driver

Hello,

On Fri, 22 Mar 2013 14:39:24 +0100, Florian Fainelli wrote:
> Hi all,
>
> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
> instead of rolling its own implementation. As a result, all users of this
> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
> The mvmdio driver is also updated to support an interrupt line which reports
> SMI error/completion, and to allow traditionnal platform device registration
> instead of just device tree.
>
> David, I think it makes sense for you to merge all of this, since we do
> not want the architecture files to be desynchronized from the mv643xx_eth to
> avoid runtime breakage. The potential for merge conflicts should be very small.
>
> Florian Fainelli (4):
> net: mvmdio: allow platform device style registration
> net: mvmdio: rename base register cookie from smireg to regs
> net: mvmdio: enhance driver to support SMI error/done interrupts
> mv643xx_eth: convert to use the Marvell Orion MDIO driver

For the entire series:

Tested-by: Thomas Petazzoni <[email protected]>

tested on:

* Armada XP DB, DT-based, which uses the mvneta driver. It is affected
by the 3 first commits since Armada XP also uses the mvmdio driver.

* Kirkwood development board, non-DT, which uses the mv643xx_eth
driver.

Thanks Florian for this work,

Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

2013-03-22 14:15:15

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 0/4 v3] mv643xx_eth: use mvmdio MDIO bus driver

On Fri, Mar 22, 2013 at 02:39:24PM +0100, Florian Fainelli wrote:
> Hi all,
>
> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
> instead of rolling its own implementation. As a result, all users of this
> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
> The mvmdio driver is also updated to support an interrupt line which reports
> SMI error/completion, and to allow traditionnal platform device registration
> instead of just device tree.
>
> David, I think it makes sense for you to merge all of this, since we do
> not want the architecture files to be desynchronized from the mv643xx_eth to
> avoid runtime breakage. The potential for merge conflicts should be very small.
>
> Florian Fainelli (4):
> net: mvmdio: allow platform device style registration
> net: mvmdio: rename base register cookie from smireg to regs
> net: mvmdio: enhance driver to support SMI error/done interrupts
> mv643xx_eth: convert to use the Marvell Orion MDIO driver
>
> .../devicetree/bindings/net/marvell-orion-mdio.txt | 3 +
> arch/arm/plat-orion/common.c | 54 +++---
> arch/powerpc/platforms/chrp/pegasos_eth.c | 20 ++
> arch/powerpc/sysdev/mv64x60_dev.c | 16 +-
> drivers/net/ethernet/marvell/Kconfig | 5 +-
> drivers/net/ethernet/marvell/Makefile | 2 +-
> drivers/net/ethernet/marvell/mv643xx_eth.c | 195 ++------------------
> drivers/net/ethernet/marvell/mvmdio.c | 130 ++++++++++---
> include/linux/mv643xx_eth.h | 1 -
> 9 files changed, 187 insertions(+), 239 deletions(-)

Whole series applied on top of v3.9-rc3 and tested on dreamplug
(kirkwood DT boot with legacy mv643xx_eth init)

Tested-by: Jason Cooper <[email protected]>

also, for the bits changing plat-orion:

Acked-by: Jason Cooper <[email protected]>

thx,

Jason.

2013-03-22 14:25:13

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 0/4 v3] mv643xx_eth: use mvmdio MDIO bus driver

Le 03/22/13 15:14, Jason Cooper a écrit :
> On Fri, Mar 22, 2013 at 02:39:24PM +0100, Florian Fainelli wrote:
>> Hi all,
>>
>> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
>> instead of rolling its own implementation. As a result, all users of this
>> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
>> The mvmdio driver is also updated to support an interrupt line which reports
>> SMI error/completion, and to allow traditionnal platform device registration
>> instead of just device tree.
>>
>> David, I think it makes sense for you to merge all of this, since we do
>> not want the architecture files to be desynchronized from the mv643xx_eth to
>> avoid runtime breakage. The potential for merge conflicts should be very small.
>>
>> Florian Fainelli (4):
>> net: mvmdio: allow platform device style registration
>> net: mvmdio: rename base register cookie from smireg to regs
>> net: mvmdio: enhance driver to support SMI error/done interrupts
>> mv643xx_eth: convert to use the Marvell Orion MDIO driver
>>
>> .../devicetree/bindings/net/marvell-orion-mdio.txt | 3 +
>> arch/arm/plat-orion/common.c | 54 +++---
>> arch/powerpc/platforms/chrp/pegasos_eth.c | 20 ++
>> arch/powerpc/sysdev/mv64x60_dev.c | 16 +-
>> drivers/net/ethernet/marvell/Kconfig | 5 +-
>> drivers/net/ethernet/marvell/Makefile | 2 +-
>> drivers/net/ethernet/marvell/mv643xx_eth.c | 195 ++------------------
>> drivers/net/ethernet/marvell/mvmdio.c | 130 ++++++++++---
>> include/linux/mv643xx_eth.h | 1 -
>> 9 files changed, 187 insertions(+), 239 deletions(-)
>
> Whole series applied on top of v3.9-rc3 and tested on dreamplug
> (kirkwood DT boot with legacy mv643xx_eth init)

Ok, thanks! Does that mean that you want these changes to go via your
tree? David initially applied my v2 of this patchset, and since it
thouches mostly ethernet driver stuff, I would rather make it go via his
tree if both of you agree.
--
Florian

2013-03-22 14:26:33

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 0/4 v3] mv643xx_eth: use mvmdio MDIO bus driver

From: Florian Fainelli <[email protected]>
Date: Fri, 22 Mar 2013 14:39:24 +0100

> This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
> instead of rolling its own implementation. As a result, all users of this
> mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
> The mvmdio driver is also updated to support an interrupt line which reports
> SMI error/completion, and to allow traditionnal platform device registration
> instead of just device tree.
>
> David, I think it makes sense for you to merge all of this, since we do
> not want the architecture files to be desynchronized from the mv643xx_eth to
> avoid runtime breakage. The potential for merge conflicts should be very small.

Series applied to net-next, thanks Florian.

2013-03-22 14:29:53

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH 0/4 v3] mv643xx_eth: use mvmdio MDIO bus driver

On Fri, Mar 22, 2013 at 03:24:55PM +0100, Florian Fainelli wrote:
> Le 03/22/13 15:14, Jason Cooper a ?crit :
> >On Fri, Mar 22, 2013 at 02:39:24PM +0100, Florian Fainelli wrote:
> >>Hi all,
> >>
> >>This patch converts the mv643xx_eth driver to use the mvmdio MDIO bus driver
> >>instead of rolling its own implementation. As a result, all users of this
> >>mv643xx_eth driver are converted to register an "orion-mdio" platform_device.
> >>The mvmdio driver is also updated to support an interrupt line which reports
> >>SMI error/completion, and to allow traditionnal platform device registration
> >>instead of just device tree.
> >>
> >>David, I think it makes sense for you to merge all of this, since we do
> >>not want the architecture files to be desynchronized from the mv643xx_eth to
> >>avoid runtime breakage. The potential for merge conflicts should be very small.
> >>
> >>Florian Fainelli (4):
> >> net: mvmdio: allow platform device style registration
> >> net: mvmdio: rename base register cookie from smireg to regs
> >> net: mvmdio: enhance driver to support SMI error/done interrupts
> >> mv643xx_eth: convert to use the Marvell Orion MDIO driver
> >>
> >> .../devicetree/bindings/net/marvell-orion-mdio.txt | 3 +
> >> arch/arm/plat-orion/common.c | 54 +++---
> >> arch/powerpc/platforms/chrp/pegasos_eth.c | 20 ++
> >> arch/powerpc/sysdev/mv64x60_dev.c | 16 +-
> >> drivers/net/ethernet/marvell/Kconfig | 5 +-
> >> drivers/net/ethernet/marvell/Makefile | 2 +-
> >> drivers/net/ethernet/marvell/mv643xx_eth.c | 195 ++------------------
> >> drivers/net/ethernet/marvell/mvmdio.c | 130 ++++++++++---
> >> include/linux/mv643xx_eth.h | 1 -
> >> 9 files changed, 187 insertions(+), 239 deletions(-)
> >
> >Whole series applied on top of v3.9-rc3 and tested on dreamplug
> >(kirkwood DT boot with legacy mv643xx_eth init)
>
> Ok, thanks! Does that mean that you want these changes to go via
> your tree? David initially applied my v2 of this patchset, and since
> it thouches mostly ethernet driver stuff, I would rather make it go
> via his tree if both of you agree.

Yeah, I thought I should have reworded that after I hit send :) I
simply meant it applied cleanly against v3.9-rc3, booted, and worked. I
Acked it so David could take the whole series through his tree. Sorry
for the confusion.

Now that I can build mv643xx_eth DT on top of this, I'll structure it so
those changes go on top of yours (in David's tree) and try to avoid the
external dependency for the DT bits going though arm-soc.

thx,

Jason.

2013-03-22 14:31:19

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH 0/4 v3] mv643xx_eth: use mvmdio MDIO bus driver

Le 03/22/13 15:29, Jason Cooper a écrit :
>> Ok, thanks! Does that mean that you want these changes to go via
>> your tree? David initially applied my v2 of this patchset, and since
>> it thouches mostly ethernet driver stuff, I would rather make it go
>> via his tree if both of you agree.
>
> Yeah, I thought I should have reworded that after I hit send :) I
> simply meant it applied cleanly against v3.9-rc3, booted, and worked. I
> Acked it so David could take the whole series through his tree. Sorry
> for the confusion.
>
> Now that I can build mv643xx_eth DT on top of this, I'll structure it so
> those changes go on top of yours (in David's tree) and try to avoid the
> external dependency for the DT bits going though arm-soc.

Sounds good! FYI, I am finishing up DSA Device Tree bindings so that we
can finally get rid of board-specific Kirkwood files.
--
Florian