Messages in this thread | ![/](/images/icornerl.gif) | | From | Wei Fang <> | Subject | RE: [PATCH RFC net-next v2 09/12] net: ethernet: freescale: fec: Separate C22 and C45 transactions for xgmac | Date | Wed, 28 Dec 2022 03:31:53 +0000 |
| |
> -----Original Message----- > From: Michael Walle <michael@walle.cc> > Sent: 2022年12月28日 7:07 > To: Heiner Kallweit <hkallweit1@gmail.com>; Russell King > <linux@armlinux.org.uk>; David S. Miller <davem@davemloft.net>; Eric > Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo > Abeni <pabeni@redhat.com>; Jose Abreu <Jose.Abreu@synopsys.com>; > Sergey Shtylyov <s.shtylyov@omp.ru>; Wei Fang <wei.fang@nxp.com>; > Shenwei Wang <shenwei.wang@nxp.com>; Clark Wang > <xiaoning.wang@nxp.com>; dl-linux-imx <linux-imx@nxp.com>; Sean Wang > <sean.wang@mediatek.com>; Landen Chao <Landen.Chao@mediatek.com>; > DENG Qingfang <dqfext@gmail.com>; Florian Fainelli <f.fainelli@gmail.com>; > Vladimir Oltean <olteanv@gmail.com>; Matthias Brugger > <matthias.bgg@gmail.com> > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > linux-renesas-soc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linux-mediatek@lists.infradead.org; Andrew Lunn <andrew@lunn.ch>; Geert > Uytterhoeven <geert+renesas@glider.be>; Michael Walle <michael@walle.cc> > Subject: [PATCH RFC net-next v2 09/12] net: ethernet: freescale: fec: Separate > C22 and C45 transactions for xgmac > > From: Andrew Lunn <andrew@lunn.ch> > > The fec MDIO bus driver can perform both C22 and C45 transfers. > Create separate functions for each and register the C45 versions using > the new API calls where appropriate. > > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > Signed-off-by: Michael Walle <michael@walle.cc> > --- > v2: > - [al] Fixup some indentation > --- > drivers/net/ethernet/freescale/fec_main.c | 153 > ++++++++++++++++++++---------- > 1 file changed, 103 insertions(+), 50 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/fec_main.c > b/drivers/net/ethernet/freescale/fec_main.c > index 644f3c963730..e6238e53940d 100644 > --- a/drivers/net/ethernet/freescale/fec_main.c > +++ b/drivers/net/ethernet/freescale/fec_main.c > @@ -1987,47 +1987,74 @@ static int fec_enet_mdio_wait(struct > fec_enet_private *fep) > return ret; > } > > -static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum) > +static int fec_enet_mdio_read_c22(struct mii_bus *bus, int mii_id, int > regnum) > { > struct fec_enet_private *fep = bus->priv; > struct device *dev = &fep->pdev->dev; > int ret = 0, frame_start, frame_addr, frame_op; > - bool is_c45 = !!(regnum & MII_ADDR_C45); > > ret = pm_runtime_resume_and_get(dev); > if (ret < 0) > return ret; > > - if (is_c45) { > - frame_start = FEC_MMFR_ST_C45; > + /* C22 read */ > + frame_op = FEC_MMFR_OP_READ; > + frame_start = FEC_MMFR_ST; > + frame_addr = regnum; > > - /* write address */ > - frame_addr = (regnum >> 16); > - writel(frame_start | FEC_MMFR_OP_ADDR_WRITE | > - FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) | > - FEC_MMFR_TA | (regnum & 0xFFFF), > - fep->hwp + FEC_MII_DATA); > + /* start a read op */ > + writel(frame_start | frame_op | > + FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) | > + FEC_MMFR_TA, fep->hwp + FEC_MII_DATA); > > - /* wait for end of transfer */ > - ret = fec_enet_mdio_wait(fep); > - if (ret) { > - netdev_err(fep->netdev, "MDIO address write timeout\n"); > - goto out; > - } > + /* wait for end of transfer */ > + ret = fec_enet_mdio_wait(fep); > + if (ret) { > + netdev_err(fep->netdev, "MDIO read timeout\n"); > + goto out; > + } > > - frame_op = FEC_MMFR_OP_READ_C45; > + ret = FEC_MMFR_DATA(readl(fep->hwp + FEC_MII_DATA)); > > - } else { > - /* C22 read */ > - frame_op = FEC_MMFR_OP_READ; > - frame_start = FEC_MMFR_ST; > - frame_addr = regnum; > +out: > + pm_runtime_mark_last_busy(dev); > + pm_runtime_put_autosuspend(dev); > + > + return ret; > +} > + > +static int fec_enet_mdio_read_c45(struct mii_bus *bus, int mii_id, > + int devad, int regnum) > +{ > + struct fec_enet_private *fep = bus->priv; > + struct device *dev = &fep->pdev->dev; > + int ret = 0, frame_start, frame_op; > + > + ret = pm_runtime_resume_and_get(dev); > + if (ret < 0) > + return ret; > + > + frame_start = FEC_MMFR_ST_C45; > + > + /* write address */ > + writel(frame_start | FEC_MMFR_OP_ADDR_WRITE | > + FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) | > + FEC_MMFR_TA | (regnum & 0xFFFF), > + fep->hwp + FEC_MII_DATA); > + > + /* wait for end of transfer */ > + ret = fec_enet_mdio_wait(fep); > + if (ret) { > + netdev_err(fep->netdev, "MDIO address write timeout\n"); > + goto out; > } > > + frame_op = FEC_MMFR_OP_READ_C45; > + > /* start a read op */ > writel(frame_start | frame_op | > - FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) | > - FEC_MMFR_TA, fep->hwp + FEC_MII_DATA); > + FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) | > + FEC_MMFR_TA, fep->hwp + FEC_MII_DATA); > > /* wait for end of transfer */ > ret = fec_enet_mdio_wait(fep); > @@ -2045,45 +2072,69 @@ static int fec_enet_mdio_read(struct mii_bus > *bus, int mii_id, int regnum) > return ret; > } > > -static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum, > - u16 value) > +static int fec_enet_mdio_write_c22(struct mii_bus *bus, int mii_id, int > regnum, > + u16 value) > { > struct fec_enet_private *fep = bus->priv; > struct device *dev = &fep->pdev->dev; > int ret, frame_start, frame_addr; > - bool is_c45 = !!(regnum & MII_ADDR_C45); > > ret = pm_runtime_resume_and_get(dev); > if (ret < 0) > return ret; > > - if (is_c45) { > - frame_start = FEC_MMFR_ST_C45; > + /* C22 write */ > + frame_start = FEC_MMFR_ST; > + frame_addr = regnum; > > - /* write address */ > - frame_addr = (regnum >> 16); > - writel(frame_start | FEC_MMFR_OP_ADDR_WRITE | > - FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) | > - FEC_MMFR_TA | (regnum & 0xFFFF), > - fep->hwp + FEC_MII_DATA); > + /* start a write op */ > + writel(frame_start | FEC_MMFR_OP_WRITE | > + FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) | > + FEC_MMFR_TA | FEC_MMFR_DATA(value), > + fep->hwp + FEC_MII_DATA); > > - /* wait for end of transfer */ > - ret = fec_enet_mdio_wait(fep); > - if (ret) { > - netdev_err(fep->netdev, "MDIO address write timeout\n"); > - goto out; > - } > - } else { > - /* C22 write */ > - frame_start = FEC_MMFR_ST; > - frame_addr = regnum; > + /* wait for end of transfer */ > + ret = fec_enet_mdio_wait(fep); > + if (ret) > + netdev_err(fep->netdev, "MDIO write timeout\n"); > + > + pm_runtime_mark_last_busy(dev); > + pm_runtime_put_autosuspend(dev); > + > + return ret; > +} > + > +static int fec_enet_mdio_write_c45(struct mii_bus *bus, int mii_id, > + int devad, int regnum, u16 value) > +{ > + struct fec_enet_private *fep = bus->priv; > + struct device *dev = &fep->pdev->dev; > + int ret, frame_start; > + > + ret = pm_runtime_resume_and_get(dev); > + if (ret < 0) > + return ret; > + > + frame_start = FEC_MMFR_ST_C45; > + > + /* write address */ > + writel(frame_start | FEC_MMFR_OP_ADDR_WRITE | > + FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) | > + FEC_MMFR_TA | (regnum & 0xFFFF), > + fep->hwp + FEC_MII_DATA); > + > + /* wait for end of transfer */ > + ret = fec_enet_mdio_wait(fep); > + if (ret) { > + netdev_err(fep->netdev, "MDIO address write timeout\n"); > + goto out; > } > > /* start a write op */ > writel(frame_start | FEC_MMFR_OP_WRITE | > - FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) | > - FEC_MMFR_TA | FEC_MMFR_DATA(value), > - fep->hwp + FEC_MII_DATA); > + FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) | > + FEC_MMFR_TA | FEC_MMFR_DATA(value), > + fep->hwp + FEC_MII_DATA); > > /* wait for end of transfer */ > ret = fec_enet_mdio_wait(fep); > @@ -2381,8 +2432,10 @@ static int fec_enet_mii_init(struct platform_device > *pdev) > } > > fep->mii_bus->name = "fec_enet_mii_bus"; > - fep->mii_bus->read = fec_enet_mdio_read; > - fep->mii_bus->write = fec_enet_mdio_write; > + fep->mii_bus->read = fec_enet_mdio_read_c22; > + fep->mii_bus->write = fec_enet_mdio_write_c22; > + fep->mii_bus->read_c45 = fec_enet_mdio_read_c45; > + fep->mii_bus->write_c45 = fec_enet_mdio_write_c45; > snprintf(fep->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x", > pdev->name, fep->dev_id + 1); > fep->mii_bus->priv = fep; >
It looks good to me. Reviewed-by: Wei Fang <wei.fang@nxp.com>
| ![\](/images/icornerr.gif) |