netdev - RE: [RFC V2 resend net-next 1/3] net: stmmac: add clocks management for gmac driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 3 Mar 2021 04:13:53 +0000
From:   Joakim Zhang <qiangqing.zhang@....com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     "peppe.cavallaro@...com" <peppe.cavallaro@...com>,
        "alexandre.torgue@...com" <alexandre.torgue@...com>,
        "joabreu@...opsys.com" <joabreu@...opsys.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        dl-linux-imx <linux-imx@....com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [RFC V2 resend net-next 1/3] net: stmmac: add clocks management
 for gmac driver


> -----Original Message-----
> From: Andrew Lunn <andrew@...n.ch>
> Sent: 2021年3月3日 9:24
> To: Joakim Zhang <qiangqing.zhang@....com>
> Cc: peppe.cavallaro@...com; alexandre.torgue@...com;
> joabreu@...opsys.com; davem@...emloft.net; kuba@...nel.org;
> f.fainelli@...il.com; dl-linux-imx <linux-imx@....com>;
> netdev@...r.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 <andrew@...n.ch>
> 
>     Andrew