> -----Original Message-----
> From: Andrew Lunn <and...@lunn.ch>
> Sent: 2021年3月3日 9:24
> To: Joakim Zhang <qiangqing.zh...@nxp.com>
> Cc: peppe.cavall...@st.com; alexandre.tor...@st.com;
> joab...@synopsys.com; da...@davemloft.net; k...@kernel.org;
> f.faine...@gmail.com; dl-linux-imx <linux-...@nxp.com>;
> netdev@vger.kernel.org
> 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 <and...@lunn.ch>
>
> Andrew
