* [patch 0/2] FEC_MPC52XX_PHY: Updates @ 2008-08-16 2:31 Wolfram Sang 2008-08-16 2:32 ` [patch 1/2] FEC_MPC52XX_PHY: Remove obsolete code Wolfram Sang 2008-08-16 2:32 ` [patch 2/2] FEC_MPC52XX_PHY: Refactor read/write routines Wolfram Sang 0 siblings, 2 replies; 6+ messages in thread From: Wolfram Sang @ 2008-08-16 2:31 UTC (permalink / raw) To: linuxppc-embedded Hello, While debugging FEC-problems, I encountered some issues in the FEC_PHY driver. - Patch 1 removes some obsolete code remaining from a (according to old mails) broken attempt to handle irqs. - Patch 2 refactors read/write routines to use common code, so updates regarding bus transfers just have to be made once. Tested on a Phytec phyCORE-MPC5200B-IO board. All the best, Wolfram -- Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de Pengutronix - Linux Solutions for Science and Industry ^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch 1/2] FEC_MPC52XX_PHY: Remove obsolete code 2008-08-16 2:31 [patch 0/2] FEC_MPC52XX_PHY: Updates Wolfram Sang @ 2008-08-16 2:32 ` Wolfram Sang 2008-10-15 14:28 ` Grant Likely 2008-08-16 2:32 ` [patch 2/2] FEC_MPC52XX_PHY: Refactor read/write routines Wolfram Sang 1 sibling, 1 reply; 6+ messages in thread From: Wolfram Sang @ 2008-08-16 2:32 UTC (permalink / raw) To: linuxppc-embedded; +Cc: Wolfram Sang Remove last remains of the former (and broken) irq support code. The driver is polling only. Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> --- drivers/net/fec_mpc52xx_phy.c | 20 -------------------- 1 file changed, 20 deletions(-) Index: .kernel/drivers/net/fec_mpc52xx_phy.c =================================================================== --- .kernel.orig/drivers/net/fec_mpc52xx_phy.c +++ .kernel/drivers/net/fec_mpc52xx_phy.c @@ -76,7 +76,6 @@ { struct device *dev = &of->dev; struct device_node *np = of->node; - struct device_node *child = NULL; struct mii_bus *bus; struct mpc52xx_fec_mdio_priv *priv; struct resource res = {}; @@ -105,15 +104,6 @@ for (i=0; i<PHY_MAX_ADDR; i++) bus->irq[i] = PHY_POLL; - while ((child = of_get_next_child(np, child)) != NULL) { - int irq = irq_of_parse_and_map(child, 0); - if (irq != NO_IRQ) { - const u32 *id = of_get_property(child, "reg", NULL); - if (id) - bus->irq[*id] = irq; - } - } - /* setup registers */ err = of_address_to_resource(np, 0, &res); if (err) @@ -133,9 +123,6 @@ /* set MII speed */ out_be32(&priv->regs->mii_speed, ((mpc52xx_find_ipb_freq(of->node) >> 20) / 5) << 1); - /* enable MII interrupt */ - out_be32(&priv->regs->imask, in_be32(&priv->regs->imask) | FEC_IMASK_MII); - err = mdiobus_register(bus); if (err) goto out_unmap; @@ -145,9 +132,6 @@ out_unmap: iounmap(priv->regs); out_free: - for (i=0; i<PHY_MAX_ADDR; i++) - if (bus->irq[i] != PHY_POLL) - irq_dispose_mapping(bus->irq[i]); kfree(bus->irq); kfree(priv); kfree(bus); @@ -160,15 +144,11 @@ struct device *dev = &of->dev; struct mii_bus *bus = dev_get_drvdata(dev); struct mpc52xx_fec_mdio_priv *priv = bus->priv; - int i; mdiobus_unregister(bus); dev_set_drvdata(dev, NULL); iounmap(priv->regs); - for (i=0; i<PHY_MAX_ADDR; i++) - if (bus->irq[i]) - irq_dispose_mapping(bus->irq[i]); kfree(priv); kfree(bus->irq); kfree(bus); -- Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de Pengutronix - Linux Solutions for Science and Industry ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 1/2] FEC_MPC52XX_PHY: Remove obsolete code 2008-08-16 2:32 ` [patch 1/2] FEC_MPC52XX_PHY: Remove obsolete code Wolfram Sang @ 2008-10-15 14:28 ` Grant Likely 2008-10-16 21:30 ` Wolfram Sang 0 siblings, 1 reply; 6+ messages in thread From: Grant Likely @ 2008-10-15 14:28 UTC (permalink / raw) To: Wolfram Sang; +Cc: linuxppc-embedded On Sat, Aug 16, 2008 at 04:32:00AM +0200, Wolfram Sang wrote: > Remove last remains of the former (and broken) irq support code. The > driver is polling only. > > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> I'm not convinced on this one. At the very least I need a more detailed description of what is wrong and why the code needs to be removed. As far as I can tell, this code affects two different IRQ paths. First is the MDIO irq which tells the processor that an MDIO transaction has completed. Second is an IRQ from the PHY itself which can come in from an entirely different path. As it stands, I'm going to hold off picking this one up. g. > --- > drivers/net/fec_mpc52xx_phy.c | 20 -------------------- > 1 file changed, 20 deletions(-) > > Index: .kernel/drivers/net/fec_mpc52xx_phy.c > =================================================================== > --- .kernel.orig/drivers/net/fec_mpc52xx_phy.c > +++ .kernel/drivers/net/fec_mpc52xx_phy.c > @@ -76,7 +76,6 @@ > { > struct device *dev = &of->dev; > struct device_node *np = of->node; > - struct device_node *child = NULL; > struct mii_bus *bus; > struct mpc52xx_fec_mdio_priv *priv; > struct resource res = {}; > @@ -105,15 +104,6 @@ > for (i=0; i<PHY_MAX_ADDR; i++) > bus->irq[i] = PHY_POLL; > > - while ((child = of_get_next_child(np, child)) != NULL) { > - int irq = irq_of_parse_and_map(child, 0); > - if (irq != NO_IRQ) { > - const u32 *id = of_get_property(child, "reg", NULL); > - if (id) > - bus->irq[*id] = irq; > - } > - } > - > /* setup registers */ > err = of_address_to_resource(np, 0, &res); > if (err) > @@ -133,9 +123,6 @@ > /* set MII speed */ > out_be32(&priv->regs->mii_speed, ((mpc52xx_find_ipb_freq(of->node) >> 20) / 5) << 1); > > - /* enable MII interrupt */ > - out_be32(&priv->regs->imask, in_be32(&priv->regs->imask) | FEC_IMASK_MII); > - > err = mdiobus_register(bus); > if (err) > goto out_unmap; > @@ -145,9 +132,6 @@ > out_unmap: > iounmap(priv->regs); > out_free: > - for (i=0; i<PHY_MAX_ADDR; i++) > - if (bus->irq[i] != PHY_POLL) > - irq_dispose_mapping(bus->irq[i]); > kfree(bus->irq); > kfree(priv); > kfree(bus); > @@ -160,15 +144,11 @@ > struct device *dev = &of->dev; > struct mii_bus *bus = dev_get_drvdata(dev); > struct mpc52xx_fec_mdio_priv *priv = bus->priv; > - int i; > > mdiobus_unregister(bus); > dev_set_drvdata(dev, NULL); > > iounmap(priv->regs); > - for (i=0; i<PHY_MAX_ADDR; i++) > - if (bus->irq[i]) > - irq_dispose_mapping(bus->irq[i]); > kfree(priv); > kfree(bus->irq); > kfree(bus); > > -- > Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de > Pengutronix - Linux Solutions for Science and Industry > _______________________________________________ > Linuxppc-embedded mailing list > Linuxppc-embedded@ozlabs.org > https://ozlabs.org/mailman/listinfo/linuxppc-embedded ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 1/2] FEC_MPC52XX_PHY: Remove obsolete code 2008-10-15 14:28 ` Grant Likely @ 2008-10-16 21:30 ` Wolfram Sang 0 siblings, 0 replies; 6+ messages in thread From: Wolfram Sang @ 2008-10-16 21:30 UTC (permalink / raw) To: Grant Likely; +Cc: linuxppc-embedded [-- Attachment #1: Type: text/plain, Size: 1756 bytes --] On Wed, Oct 15, 2008 at 08:28:55AM -0600, Grant Likely wrote: > description of what is wrong and why the code needs to be removed. As > far as I can tell, this code affects two different IRQ paths. First is Ups, you are right! I am sorry that I missed that somehow :( Here is hopefully a better version, which just skips the really unnecessary interrupt and fixes a glitch in mdio_remove: === [MPC52XX] fec_mpc52xx_phy: Minor cleanups As this driver polls for a complete MDIO transaction, there is no need to enable interrupts for it. Furthermore, make both checks for freeing MDIO-bus irqs consistent. Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> --- drivers/net/fec_mpc52xx_phy.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-) diff --git a/drivers/net/fec_mpc52xx_phy.c b/drivers/net/fec_mpc52xx_phy.c index 08e18bc..3ad3750 100644 --- a/drivers/net/fec_mpc52xx_phy.c +++ b/drivers/net/fec_mpc52xx_phy.c @@ -133,9 +133,6 @@ static int mpc52xx_fec_mdio_probe(struct of_device *of, const struct of_device_i /* set MII speed */ out_be32(&priv->regs->mii_speed, ((mpc52xx_find_ipb_freq(of->node) >> 20) / 5) << 1); - /* enable MII interrupt */ - out_be32(&priv->regs->imask, in_be32(&priv->regs->imask) | FEC_IMASK_MII); - err = mdiobus_register(bus); if (err) goto out_unmap; @@ -167,7 +164,7 @@ static int mpc52xx_fec_mdio_remove(struct of_device *of) iounmap(priv->regs); for (i=0; i<PHY_MAX_ADDR; i++) - if (bus->irq[i]) + if (bus->irq[i] != PHY_POLL) irq_dispose_mapping(bus->irq[i]); kfree(priv); kfree(bus->irq); -- 1.5.6.5 -- Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de Pengutronix - Linux Solutions for Science and Industry [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 197 bytes --] ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [patch 2/2] FEC_MPC52XX_PHY: Refactor read/write routines 2008-08-16 2:31 [patch 0/2] FEC_MPC52XX_PHY: Updates Wolfram Sang 2008-08-16 2:32 ` [patch 1/2] FEC_MPC52XX_PHY: Remove obsolete code Wolfram Sang @ 2008-08-16 2:32 ` Wolfram Sang 2008-10-15 14:29 ` Grant Likely 1 sibling, 1 reply; 6+ messages in thread From: Wolfram Sang @ 2008-08-16 2:32 UTC (permalink / raw) To: linuxppc-embedded; +Cc: Wolfram Sang Read & write functions now call a generic transfer function, so identical code in both routines could be eliminated. The result is easier to maintain and smaller in source and binary code. Also, fix some checkpatch warnings. Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> --- drivers/net/fec_mpc52xx_phy.c | 55 ++++++++++++++++-------------------------- 1 file changed, 22 insertions(+), 33 deletions(-) Index: .kernel/drivers/net/fec_mpc52xx_phy.c =================================================================== --- .kernel.orig/drivers/net/fec_mpc52xx_phy.c +++ .kernel/drivers/net/fec_mpc52xx_phy.c @@ -2,6 +2,7 @@ * Driver for the MPC5200 Fast Ethernet Controller - MDIO bus driver * * Copyright (C) 2007 Domen Puncer, Telargo, Inc. + * Copyright (C) 2008 Wolfram Sang, Pengutronix * * This file is licensed under the terms of the GNU General Public License * version 2. This program is licensed "as is" without any warranty of any @@ -21,58 +22,45 @@ struct mpc52xx_fec __iomem *regs; }; -static int mpc52xx_fec_mdio_read(struct mii_bus *bus, int phy_id, int reg) +static int mpc52xx_fec_mdio_transfer(struct mii_bus *bus, int phy_id, + int reg, u32 value) { struct mpc52xx_fec_mdio_priv *priv = bus->priv; struct mpc52xx_fec __iomem *fec; int tries = 100; - u32 request = FEC_MII_READ_FRAME; + + value |= (phy_id << FEC_MII_DATA_PA_SHIFT) & FEC_MII_DATA_PA_MSK; + value |= (reg << FEC_MII_DATA_RA_SHIFT) & FEC_MII_DATA_RA_MSK; fec = priv->regs; out_be32(&fec->ievent, FEC_IEVENT_MII); - - request |= (phy_id << FEC_MII_DATA_PA_SHIFT) & FEC_MII_DATA_PA_MSK; - request |= (reg << FEC_MII_DATA_RA_SHIFT) & FEC_MII_DATA_RA_MSK; - - out_be32(&priv->regs->mii_data, request); + out_be32(&priv->regs->mii_data, value); /* wait for it to finish, this takes about 23 us on lite5200b */ while (!(in_be32(&fec->ievent) & FEC_IEVENT_MII) && --tries) udelay(5); - if (tries == 0) + if (!tries) return -ETIMEDOUT; - return in_be32(&priv->regs->mii_data) & FEC_MII_DATA_DATAMSK; + return value & FEC_MII_DATA_OP_RD ? + in_be32(&priv->regs->mii_data) & FEC_MII_DATA_DATAMSK : 0; } -static int mpc52xx_fec_mdio_write(struct mii_bus *bus, int phy_id, int reg, u16 data) +static int mpc52xx_fec_mdio_read(struct mii_bus *bus, int phy_id, int reg) { - struct mpc52xx_fec_mdio_priv *priv = bus->priv; - struct mpc52xx_fec __iomem *fec; - u32 value = data; - int tries = 100; - - fec = priv->regs; - out_be32(&fec->ievent, FEC_IEVENT_MII); - - value |= FEC_MII_WRITE_FRAME; - value |= (phy_id << FEC_MII_DATA_PA_SHIFT) & FEC_MII_DATA_PA_MSK; - value |= (reg << FEC_MII_DATA_RA_SHIFT) & FEC_MII_DATA_RA_MSK; - - out_be32(&priv->regs->mii_data, value); - - /* wait for request to finish */ - while (!(in_be32(&fec->ievent) & FEC_IEVENT_MII) && --tries) - udelay(5); - - if (tries == 0) - return -ETIMEDOUT; + return mpc52xx_fec_mdio_transfer(bus, phy_id, reg, FEC_MII_READ_FRAME); +} - return 0; +static int mpc52xx_fec_mdio_write(struct mii_bus *bus, int phy_id, int reg, + u16 data) +{ + return mpc52xx_fec_mdio_transfer(bus, phy_id, reg, + data | FEC_MII_WRITE_FRAME); } -static int mpc52xx_fec_mdio_probe(struct of_device *of, const struct of_device_id *match) +static int mpc52xx_fec_mdio_probe(struct of_device *of, + const struct of_device_id *match) { struct device *dev = &of->dev; struct device_node *np = of->node; @@ -121,7 +109,8 @@ dev_set_drvdata(dev, bus); /* set MII speed */ - out_be32(&priv->regs->mii_speed, ((mpc52xx_find_ipb_freq(of->node) >> 20) / 5) << 1); + out_be32(&priv->regs->mii_speed, + ((mpc52xx_find_ipb_freq(of->node) >> 20) / 5) << 1); err = mdiobus_register(bus); if (err) -- Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de Pengutronix - Linux Solutions for Science and Industry ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 2/2] FEC_MPC52XX_PHY: Refactor read/write routines 2008-08-16 2:32 ` [patch 2/2] FEC_MPC52XX_PHY: Refactor read/write routines Wolfram Sang @ 2008-10-15 14:29 ` Grant Likely 0 siblings, 0 replies; 6+ messages in thread From: Grant Likely @ 2008-10-15 14:29 UTC (permalink / raw) To: Wolfram Sang; +Cc: linuxppc-embedded On Sat, Aug 16, 2008 at 04:32:01AM +0200, Wolfram Sang wrote: > Read & write functions now call a generic transfer function, so identical > code in both routines could be eliminated. The result is easier to maintain > and smaller in source and binary code. Also, fix some checkpatch warnings. This looks good. I'll pick it up. g. > > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> > --- > drivers/net/fec_mpc52xx_phy.c | 55 ++++++++++++++++-------------------------- > 1 file changed, 22 insertions(+), 33 deletions(-) > > Index: .kernel/drivers/net/fec_mpc52xx_phy.c > =================================================================== > --- .kernel.orig/drivers/net/fec_mpc52xx_phy.c > +++ .kernel/drivers/net/fec_mpc52xx_phy.c > @@ -2,6 +2,7 @@ > * Driver for the MPC5200 Fast Ethernet Controller - MDIO bus driver > * > * Copyright (C) 2007 Domen Puncer, Telargo, Inc. > + * Copyright (C) 2008 Wolfram Sang, Pengutronix > * > * This file is licensed under the terms of the GNU General Public License > * version 2. This program is licensed "as is" without any warranty of any > @@ -21,58 +22,45 @@ > struct mpc52xx_fec __iomem *regs; > }; > > -static int mpc52xx_fec_mdio_read(struct mii_bus *bus, int phy_id, int reg) > +static int mpc52xx_fec_mdio_transfer(struct mii_bus *bus, int phy_id, > + int reg, u32 value) > { > struct mpc52xx_fec_mdio_priv *priv = bus->priv; > struct mpc52xx_fec __iomem *fec; > int tries = 100; > - u32 request = FEC_MII_READ_FRAME; > + > + value |= (phy_id << FEC_MII_DATA_PA_SHIFT) & FEC_MII_DATA_PA_MSK; > + value |= (reg << FEC_MII_DATA_RA_SHIFT) & FEC_MII_DATA_RA_MSK; > > fec = priv->regs; > out_be32(&fec->ievent, FEC_IEVENT_MII); > - > - request |= (phy_id << FEC_MII_DATA_PA_SHIFT) & FEC_MII_DATA_PA_MSK; > - request |= (reg << FEC_MII_DATA_RA_SHIFT) & FEC_MII_DATA_RA_MSK; > - > - out_be32(&priv->regs->mii_data, request); > + out_be32(&priv->regs->mii_data, value); > > /* wait for it to finish, this takes about 23 us on lite5200b */ > while (!(in_be32(&fec->ievent) & FEC_IEVENT_MII) && --tries) > udelay(5); > > - if (tries == 0) > + if (!tries) > return -ETIMEDOUT; > > - return in_be32(&priv->regs->mii_data) & FEC_MII_DATA_DATAMSK; > + return value & FEC_MII_DATA_OP_RD ? > + in_be32(&priv->regs->mii_data) & FEC_MII_DATA_DATAMSK : 0; > } > > -static int mpc52xx_fec_mdio_write(struct mii_bus *bus, int phy_id, int reg, u16 data) > +static int mpc52xx_fec_mdio_read(struct mii_bus *bus, int phy_id, int reg) > { > - struct mpc52xx_fec_mdio_priv *priv = bus->priv; > - struct mpc52xx_fec __iomem *fec; > - u32 value = data; > - int tries = 100; > - > - fec = priv->regs; > - out_be32(&fec->ievent, FEC_IEVENT_MII); > - > - value |= FEC_MII_WRITE_FRAME; > - value |= (phy_id << FEC_MII_DATA_PA_SHIFT) & FEC_MII_DATA_PA_MSK; > - value |= (reg << FEC_MII_DATA_RA_SHIFT) & FEC_MII_DATA_RA_MSK; > - > - out_be32(&priv->regs->mii_data, value); > - > - /* wait for request to finish */ > - while (!(in_be32(&fec->ievent) & FEC_IEVENT_MII) && --tries) > - udelay(5); > - > - if (tries == 0) > - return -ETIMEDOUT; > + return mpc52xx_fec_mdio_transfer(bus, phy_id, reg, FEC_MII_READ_FRAME); > +} > > - return 0; > +static int mpc52xx_fec_mdio_write(struct mii_bus *bus, int phy_id, int reg, > + u16 data) > +{ > + return mpc52xx_fec_mdio_transfer(bus, phy_id, reg, > + data | FEC_MII_WRITE_FRAME); > } > > -static int mpc52xx_fec_mdio_probe(struct of_device *of, const struct of_device_id *match) > +static int mpc52xx_fec_mdio_probe(struct of_device *of, > + const struct of_device_id *match) > { > struct device *dev = &of->dev; > struct device_node *np = of->node; > @@ -121,7 +109,8 @@ > dev_set_drvdata(dev, bus); > > /* set MII speed */ > - out_be32(&priv->regs->mii_speed, ((mpc52xx_find_ipb_freq(of->node) >> 20) / 5) << 1); > + out_be32(&priv->regs->mii_speed, > + ((mpc52xx_find_ipb_freq(of->node) >> 20) / 5) << 1); > > err = mdiobus_register(bus); > if (err) > > -- > Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de > Pengutronix - Linux Solutions for Science and Industry > _______________________________________________ > Linuxppc-embedded mailing list > Linuxppc-embedded@ozlabs.org > https://ozlabs.org/mailman/listinfo/linuxppc-embedded ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-10-16 21:30 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-08-16 2:31 [patch 0/2] FEC_MPC52XX_PHY: Updates Wolfram Sang 2008-08-16 2:32 ` [patch 1/2] FEC_MPC52XX_PHY: Remove obsolete code Wolfram Sang 2008-10-15 14:28 ` Grant Likely 2008-10-16 21:30 ` Wolfram Sang 2008-08-16 2:32 ` [patch 2/2] FEC_MPC52XX_PHY: Refactor read/write routines Wolfram Sang 2008-10-15 14:29 ` Grant Likely
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).