> -----Original Message----- > From: Andrew Lunn <[email protected]> > Sent: 2021年3月3日 9:24 > To: Joakim Zhang <[email protected]> > Cc: [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; dl-linux-imx <[email protected]>; > [email protected] > Subject: Re: [RFC V2 resend net-next 1/3] net: stmmac: add clocks > management for gmac driver > > On Mon, Mar 01, 2021 at 06:25:27PM +0800, Joakim Zhang wrote: > > @@ -121,11 +132,22 @@ static int stmmac_xgmac2_mdio_read(struct > mii_bus *bus, int phyaddr, int phyreg) > > > > /* Wait until any existing MII operation is complete */ > > if (readl_poll_timeout(priv->ioaddr + mii_data, tmp, > > - !(tmp & MII_XGMAC_BUSY), 100, 10000)) > > - return -EBUSY; > > + !(tmp & MII_XGMAC_BUSY), 100, 10000)) { > > + ret = -EBUSY; > > + goto err_disable_clks; > > + } > > > > /* Read the data from the MII data register */ > > - return readl(priv->ioaddr + mii_data) & GENMASK(15, 0); > > + data = (int)readl(priv->ioaddr + mii_data) & GENMASK(15, 0); > > + > > + pm_runtime_put(priv->device); > > + > > + return data; > > + > > +err_disable_clks: > > + pm_runtime_put(priv->device); > > + > > + return ret; > > Hi Joakim > > You could do > > ret = (int)readl(priv->ioaddr + mii_data) & GENMASK(15, 0); > > err_disable_clks: > pm_runtime_put(priv->device); > > return ret; > > Slightly simpler.
Hi Andrew, Thanks for you kindly review! Yes, I think this before, but "ret" is always used to check functions' return value, to avoid confusion, I add the variable "data". Maybe I'm thinking too much, I will improve it. Best Regards, Joakim Zhang > > > > /* Read the data from the MII data register */ > > data = (int)readl(priv->ioaddr + mii_data) & MII_DATA_MASK; > > > > + pm_runtime_put(priv->device); > > + > > return data; > > + > > +err_disable_clks: > > + pm_runtime_put(priv->device); > > + > > + return ret; > > } > > Same here. > > Otherwise: > > Reviewed-by: Andrew Lunn <[email protected]> > > Andrew
