| Message ID | 20211125201301.3748513-2-colin.foster@in-advantage.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | update seville to use shared MDIO driver | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 6 of 6 maintainers |
| netdev/build_clang | fail | Errors and warnings before: 0 this patch: 3 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 237 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Thu, Nov 25, 2021 at 12:12:59PM -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 | 161 ++++++++++++++++++++++-------- > 1 file changed, 119 insertions(+), 42 deletions(-) > > diff --git a/drivers/net/mdio/mdio-mscc-miim.c b/drivers/net/mdio/mdio-mscc-miim.c > index 17f98f609ec8..5a9c0b311bdb 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,49 @@ > #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, ret; > + > + ret = regmap_read(miim->regs, MSCC_MIIM_REG_STATUS, &val); > + if (ret < 0) { > + WARN_ONCE(1, "mscc miim status read error %d\n", ret); > + return ret; > + } > + > + 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); > } > @@ -80,15 +93,27 @@ static int mscc_miim_read(struct mii_bus *bus, int mii_id, int regnum) > 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); > + ret = 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 (ret < 0) { > + WARN_ONCE(1, "mscc miim write cmd reg error %d\n", ret); > + goto out; > + } > > ret = mscc_miim_wait_ready(bus); > if (ret) > goto out; > > - val = readl(miim->regs + MSCC_MIIM_REG_DATA); > + ret = regmap_read(miim->regs, MSCC_MIIM_REG_DATA, &val); > + > + if (ret < 0) { > + WARN_ONCE(1, "mscc miim read data reg error %d\n", ret); > + goto out; > + } > + > if (val & MSCC_MIIM_DATA_ERROR) { > ret = -EIO; > goto out; > @@ -109,12 +134,14 @@ static int mscc_miim_write(struct mii_bus *bus, int mii_id, > 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); > + ret = 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 (ret < 0) > + WARN_ONCE(1, "mscc miim write error %d\n", ret); > out: > return ret; > } > @@ -122,24 +149,40 @@ 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 ret; > > if (miim->phy_regs) { > - writel(0, miim->phy_regs + MSCC_PHY_REG_PHY_CFG); > - writel(0x1ff, miim->phy_regs + MSCC_PHY_REG_PHY_CFG); > + ret = regmap_write(miim->phy_regs, MSCC_PHY_REG_PHY_CFG, 0); > + if (ret < 0) { > + WARN_ONCE(1, "mscc reset set error %d\n", ret); > + return ret; > + } > + > + ret = regmap_write(miim->phy_regs, MSCC_PHY_REG_PHY_CFG, 0x1ff); > + if (ret < 0) { > + WARN_ONCE(1, "mscc reset clear error %d\n", ret); > + return ret; > + } > + > 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 **pbus, > + struct regmap *mii_regmap, struct regmap *phy_regmap) > { > - struct mscc_miim_dev *dev; > - struct resource *res; > + struct mscc_miim_dev *miim; > struct mii_bus *bus; > - int ret; > > - bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*dev)); > + bus = devm_mdiobus_alloc_size(dev, sizeof(*miim)); > if (!bus) > return -ENOMEM; > > @@ -147,24 +190,58 @@ 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; > + > + *pbus = bus; > + > + miim->regs = mii_regmap; > + miim->phy_regs = phy_regmap; > + > + return 0; > +} > + > +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; > > - dev = bus->priv; > - dev->regs = devm_platform_get_and_ioremap_resource(pdev, 0, NULL); > - if (IS_ERR(dev->regs)) { > + 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)) { Oops. I'll fix this in V3. > + 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); > + } > + > + ret = mscc_miim_setup(&pdev->dev, &bus, mii_regmap, phy_regmap); > + if (ret < 0) { > + dev_err(&pdev->dev, "Unable to setup the MDIO bus\n"); > + return ret; > } > > ret = of_mdiobus_register(bus, pdev->dev.of_node); > -- > 2.25.1 >
On Thu, Nov 25, 2021 at 12:12:59PM -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> > --- Tested-by: Vladimir Oltean <vladimir.oltean@nxp.com>
diff --git a/drivers/net/mdio/mdio-mscc-miim.c b/drivers/net/mdio/mdio-mscc-miim.c index 17f98f609ec8..5a9c0b311bdb 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,49 @@ #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, ret; + + ret = regmap_read(miim->regs, MSCC_MIIM_REG_STATUS, &val); + if (ret < 0) { + WARN_ONCE(1, "mscc miim status read error %d\n", ret); + return ret; + } + + 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); } @@ -80,15 +93,27 @@ static int mscc_miim_read(struct mii_bus *bus, int mii_id, int regnum) 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); + ret = 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 (ret < 0) { + WARN_ONCE(1, "mscc miim write cmd reg error %d\n", ret); + goto out; + } ret = mscc_miim_wait_ready(bus); if (ret) goto out; - val = readl(miim->regs + MSCC_MIIM_REG_DATA); + ret = regmap_read(miim->regs, MSCC_MIIM_REG_DATA, &val); + + if (ret < 0) { + WARN_ONCE(1, "mscc miim read data reg error %d\n", ret); + goto out; + } + if (val & MSCC_MIIM_DATA_ERROR) { ret = -EIO; goto out; @@ -109,12 +134,14 @@ static int mscc_miim_write(struct mii_bus *bus, int mii_id, 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); + ret = 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 (ret < 0) + WARN_ONCE(1, "mscc miim write error %d\n", ret); out: return ret; } @@ -122,24 +149,40 @@ 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 ret; if (miim->phy_regs) { - writel(0, miim->phy_regs + MSCC_PHY_REG_PHY_CFG); - writel(0x1ff, miim->phy_regs + MSCC_PHY_REG_PHY_CFG); + ret = regmap_write(miim->phy_regs, MSCC_PHY_REG_PHY_CFG, 0); + if (ret < 0) { + WARN_ONCE(1, "mscc reset set error %d\n", ret); + return ret; + } + + ret = regmap_write(miim->phy_regs, MSCC_PHY_REG_PHY_CFG, 0x1ff); + if (ret < 0) { + WARN_ONCE(1, "mscc reset clear error %d\n", ret); + return ret; + } + 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 **pbus, + struct regmap *mii_regmap, struct regmap *phy_regmap) { - struct mscc_miim_dev *dev; - struct resource *res; + struct mscc_miim_dev *miim; struct mii_bus *bus; - int ret; - bus = devm_mdiobus_alloc_size(&pdev->dev, sizeof(*dev)); + bus = devm_mdiobus_alloc_size(dev, sizeof(*miim)); if (!bus) return -ENOMEM; @@ -147,24 +190,58 @@ 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; + + *pbus = bus; + + miim->regs = mii_regmap; + miim->phy_regs = phy_regmap; + + return 0; +} + +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; - dev = bus->priv; - dev->regs = devm_platform_get_and_ioremap_resource(pdev, 0, NULL); - if (IS_ERR(dev->regs)) { + 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); + } + + ret = mscc_miim_setup(&pdev->dev, &bus, mii_regmap, phy_regmap); + if (ret < 0) { + dev_err(&pdev->dev, "Unable to setup the MDIO bus\n"); + return ret; } ret = of_mdiobus_register(bus, pdev->dev.of_node);
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 | 161 ++++++++++++++++++++++-------- 1 file changed, 119 insertions(+), 42 deletions(-)