Messages in this thread | | | Date | Sat, 20 Nov 2021 13:25:13 -0800 | From | Colin Foster <> | Subject | Re: [PATCH v1 net-next 1/3] net: mdio: mscc-miim: convert to a regmap implementation |
| |
Hi Andrew,
Thank you for your feedback! I'll do a sweep for these return cases before I submit v2.
On Sat, Nov 20, 2021 at 04:09:18PM +0100, Andrew Lunn wrote: > > @@ -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); > > You should probably return ret here. If the setup fails, i doubt you > will get anything useful from the hardware. > > > > > 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); > > Same here. > > > + > > 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); > > And here, etc. > > Andrew
| |