netdev - Re: [PATCH v1 net-next] net: stmmac: enable clause 45 mdio support

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190704155217.GI18473@lunn.ch>
Date:   Thu, 4 Jul 2019 17:52:17 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     "Voon, Weifeng" <weifeng.voon@...el.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Jose Abreu <joabreu@...opsys.com>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Alexandre Torgue <alexandre.torgue@...com>,
        biao huang <biao.huang@...iatek.com>,
        "Ong, Boon Leong" <boon.leong.ong@...el.com>,
        "Kweh, Hock Leong" <hock.leong.kweh@...el.com>
Subject: Re: [PATCH v1 net-next] net: stmmac: enable clause 45 mdio support

> Yes, the top 16 bit of the data register only valid when C45 is enable.
> It contains the Register address which MDIO c45 frame intended for.

I think there is too much passing variables around by reference than
by value, to make this code easy to understand.

Maybe a better structure would be

static int stmmac_mdion_c45_read(struct stmmac_priv *priv, int phyaddr, int phyreg)
{

	unsigned int reg_shift = priv->hw->mii.reg_shift;
	unsigned int reg_mask = priv->hw->mii.reg_mask;
	u32 mii_addr_val, mii_data_val;

	mii_addr_val = MII_GMAC4_C45E |
                       ((phyreg >> MII_DEVADDR_C45_SHIFT) << reg_shift) & reg_mask;
        mii_data_val = (phyreg & MII_REGADDR_C45_MASK) << MII_GMAC4_REG_ADDR_SHIFT;

	writel(mii_data_val, priv->ioaddr + priv->hw->mii_data);
	writel(mii_addr_val, priv->ioaddr + priv->hw->mii_addrress);

	return (int)readl(priv->ioaddr + mii_data) & MII_DATA_MASK;
}		   

static int stmmac_mdio_read(struct mii_bus *bus, int phyaddr, int phyreg)
{

...
	if (readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & MII_BUSY),
 	   		      100, 10000))
 		return -EBUSY;

      if (priv->plat->has_gmac4 && phyreg & MII_ADDR_C45)
      	return stmmac_mdio_c45_read(priv, phyaddr, phyreg);

	Andrew