Messages in this thread | | | From | Vladimir Oltean <> | Subject | Re: [PATCH v1 net-next 1/3] net: mdio: mscc-miim: convert to a regmap implementation | Date | Sun, 21 Nov 2021 17:32:19 +0000 |
| |
On Fri, Nov 19, 2021 at 01:39:16PM -0800, Colin Foster wrote: > Utilize regmap instead of __iomem to perform indirect mdio access. This > will allow for custom regmaps to be used by way of the mscc_miim_setup > function. > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> > --- > drivers/net/mdio/mdio-mscc-miim.c | 150 +++++++++++++++++++++--------- > 1 file changed, 105 insertions(+), 45 deletions(-) > > diff --git a/drivers/net/mdio/mdio-mscc-miim.c b/drivers/net/mdio/mdio-mscc-miim.c > index 17f98f609ec8..f55ad20c28d5 100644 > --- a/drivers/net/mdio/mdio-mscc-miim.c > +++ b/drivers/net/mdio/mdio-mscc-miim.c > @@ -14,6 +14,7 @@ > #include <linux/of_mdio.h> > #include <linux/phy.h> > #include <linux/platform_device.h> > +#include <linux/regmap.h> > > #define MSCC_MIIM_REG_STATUS 0x0 > #define MSCC_MIIM_STATUS_STAT_PENDING BIT(2) > @@ -35,37 +36,47 @@ > #define MSCC_PHY_REG_PHY_STATUS 0x4 > > struct mscc_miim_dev { > - void __iomem *regs; > - void __iomem *phy_regs; > + struct regmap *regs; > + struct regmap *phy_regs; > }; > > /* When high resolution timers aren't built-in: we can't use usleep_range() as > * we would sleep way too long. Use udelay() instead. > */ > -#define mscc_readl_poll_timeout(addr, val, cond, delay_us, timeout_us) \ > -({ \ > - if (!IS_ENABLED(CONFIG_HIGH_RES_TIMERS)) \ > - readl_poll_timeout_atomic(addr, val, cond, delay_us, \ > - timeout_us); \ > - readl_poll_timeout(addr, val, cond, delay_us, timeout_us); \ > +#define mscc_readx_poll_timeout(op, addr, val, cond, delay_us, timeout_us)\ > +({ \ > + if (!IS_ENABLED(CONFIG_HIGH_RES_TIMERS)) \ > + readx_poll_timeout_atomic(op, addr, val, cond, delay_us, \ > + timeout_us); \ > + readx_poll_timeout(op, addr, val, cond, delay_us, timeout_us); \ > }) > > -static int mscc_miim_wait_ready(struct mii_bus *bus) > +static int mscc_miim_status(struct mii_bus *bus) > { > struct mscc_miim_dev *miim = bus->priv; > + int val, err; > + > + err = regmap_read(miim->regs, MSCC_MIIM_REG_STATUS, &val); > + if (err < 0) > + WARN_ONCE(1, "mscc miim status read error %d\n", err); > + > + return val; > +} > + > +static int mscc_miim_wait_ready(struct mii_bus *bus) > +{ > u32 val; > > - return mscc_readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, > + return mscc_readx_poll_timeout(mscc_miim_status, bus, val, > !(val & MSCC_MIIM_STATUS_STAT_BUSY), 50, > 10000); > } > > static int mscc_miim_wait_pending(struct mii_bus *bus) > { > - struct mscc_miim_dev *miim = bus->priv; > u32 val; > > - return mscc_readl_poll_timeout(miim->regs + MSCC_MIIM_REG_STATUS, val, > + return mscc_readx_poll_timeout(mscc_miim_status, bus, val, > !(val & MSCC_MIIM_STATUS_STAT_PENDING), > 50, 10000); > } > @@ -73,22 +84,30 @@ static int mscc_miim_wait_pending(struct mii_bus *bus) > static int mscc_miim_read(struct mii_bus *bus, int mii_id, int regnum) > { > struct mscc_miim_dev *miim = bus->priv; > + int ret, err; > u32 val; > - int ret; > > ret = mscc_miim_wait_pending(bus); > if (ret) > goto out; > > - writel(MSCC_MIIM_CMD_VLD | (mii_id << MSCC_MIIM_CMD_PHYAD_SHIFT) | > - (regnum << MSCC_MIIM_CMD_REGAD_SHIFT) | MSCC_MIIM_CMD_OPR_READ, > - miim->regs + MSCC_MIIM_REG_CMD); > + err = regmap_write(miim->regs, MSCC_MIIM_REG_CMD, MSCC_MIIM_CMD_VLD | > + (mii_id << MSCC_MIIM_CMD_PHYAD_SHIFT) | > + (regnum << MSCC_MIIM_CMD_REGAD_SHIFT) | > + MSCC_MIIM_CMD_OPR_READ); > + > + if (err < 0) > + WARN_ONCE(1, "mscc miim write cmd reg error %d\n", err); > > ret = mscc_miim_wait_ready(bus); > if (ret) > goto out; > > - val = readl(miim->regs + MSCC_MIIM_REG_DATA); > + err = regmap_read(miim->regs, MSCC_MIIM_REG_DATA, &val); > + > + if (err < 0) > + WARN_ONCE(1, "mscc miim read data reg error %d\n", err); > + > if (val & MSCC_MIIM_DATA_ERROR) { > ret = -EIO; > goto out; > @@ -103,18 +122,20 @@ static int mscc_miim_write(struct mii_bus *bus, int mii_id, > int regnum, u16 value) > { > struct mscc_miim_dev *miim = bus->priv; > - int ret; > + int err, ret; > > ret = mscc_miim_wait_pending(bus); > if (ret < 0) > goto out; > > - writel(MSCC_MIIM_CMD_VLD | (mii_id << MSCC_MIIM_CMD_PHYAD_SHIFT) | > - (regnum << MSCC_MIIM_CMD_REGAD_SHIFT) | > - (value << MSCC_MIIM_CMD_WRDATA_SHIFT) | > - MSCC_MIIM_CMD_OPR_WRITE, > - miim->regs + MSCC_MIIM_REG_CMD); > + err = regmap_write(miim->regs, MSCC_MIIM_REG_CMD, MSCC_MIIM_CMD_VLD | > + (mii_id << MSCC_MIIM_CMD_PHYAD_SHIFT) | > + (regnum << MSCC_MIIM_CMD_REGAD_SHIFT) | > + (value << MSCC_MIIM_CMD_WRDATA_SHIFT) | > + MSCC_MIIM_CMD_OPR_WRITE); > > + if (err < 0) > + WARN_ONCE(1, "mscc miim write error %d\n", err); > out: > return ret; > } > @@ -122,24 +143,35 @@ static int mscc_miim_write(struct mii_bus *bus, int mii_id, > static int mscc_miim_reset(struct mii_bus *bus) > { > struct mscc_miim_dev *miim = bus->priv; > + int err; > > if (miim->phy_regs) { > - writel(0, miim->phy_regs + MSCC_PHY_REG_PHY_CFG); > - writel(0x1ff, miim->phy_regs + MSCC_PHY_REG_PHY_CFG); > + err = regmap_write(miim->phy_regs, MSCC_PHY_REG_PHY_CFG, 0); > + if (err < 0) > + WARN_ONCE(1, "mscc reset set error %d\n", err); > + > + err = regmap_write(miim->phy_regs, MSCC_PHY_REG_PHY_CFG, 0x1ff); > + if (err < 0) > + WARN_ONCE(1, "mscc reset clear error %d\n", err); > + > mdelay(500); > } > > return 0; > } > > -static int mscc_miim_probe(struct platform_device *pdev) > +static const struct regmap_config mscc_miim_regmap_config = { > + .reg_bits = 32, > + .val_bits = 32, > + .reg_stride = 4, > +}; > + > +static int mscc_miim_setup(struct device *dev, struct mii_bus *bus, > + struct regmap *mii_regmap, struct regmap *phy_regmap) > { > - struct mscc_miim_dev *dev; > - struct resource *res; > - struct mii_bus *bus; > - int ret; > + struct mscc_miim_dev *miim; > > - bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*dev)); > + bus = devm_mdiobus_alloc_size(dev, sizeof(*miim)); > if (!bus) > return -ENOMEM; > > @@ -147,26 +179,54 @@ static int mscc_miim_probe(struct platform_device *pdev) > bus->read = mscc_miim_read; > bus->write = mscc_miim_write; > bus->reset = mscc_miim_reset; > - snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(&pdev->dev)); > - bus->parent = &pdev->dev; > + snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii", dev_name(dev)); > + bus->parent = dev; > + > + miim = bus->priv; > + > + miim->regs = mii_regmap; > + miim->phy_regs = phy_regmap; > + > + return 0; > +} > > - dev = bus->priv; > - dev->regs = devm_platform_get_and_ioremap_resource(pdev, 0, NULL); > - if (IS_ERR(dev->regs)) { > +static int mscc_miim_probe(struct platform_device *pdev) > +{ > + struct regmap *mii_regmap, *phy_regmap; > + void __iomem *regs, *phy_regs; > + struct mscc_miim_dev *dev; > + struct mii_bus *bus; > + int ret; > + > + regs = devm_platform_get_and_ioremap_resource(pdev, 0, NULL); > + if (IS_ERR(regs)) { > dev_err(&pdev->dev, "Unable to map MIIM registers\n"); > - return PTR_ERR(dev->regs); > + return PTR_ERR(regs); > } > > - /* This resource is optional */ > - res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > - if (res) { > - dev->phy_regs = devm_ioremap_resource(&pdev->dev, res); > - if (IS_ERR(dev->phy_regs)) { > - dev_err(&pdev->dev, "Unable to map internal phy registers\n"); > - return PTR_ERR(dev->phy_regs); > - } > + mii_regmap = devm_regmap_init_mmio(&pdev->dev, regs, > + &mscc_miim_regmap_config); > + > + if (IS_ERR(mii_regmap)) { > + dev_err(&pdev->dev, "Unable to create MIIM regmap\n"); > + return PTR_ERR(mii_regmap); > } > > + phy_regs = devm_platform_ioremap_resource(pdev, 1); > + if (IS_ERR(dev->phy_regs)) { > + dev_err(&pdev->dev, "Unable to map internal phy registers\n"); > + return PTR_ERR(dev->phy_regs); > + } > + > + phy_regmap = devm_regmap_init_mmio(&pdev->dev, phy_regs, > + &mscc_miim_regmap_config); > + if (IS_ERR(phy_regmap)) { > + dev_err(&pdev->dev, "Unable to create phy register regmap\n"); > + return PTR_ERR(dev->phy_regs); > + } > + > + mscc_miim_setup(&pdev->dev, bus, mii_regmap, phy_regmap);
You're ignoring potential errors here.
> + > ret = of_mdiobus_register(bus, pdev->dev.of_node); > if (ret < 0) { > dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret); > -- > 2.25.1 >
| |