diff mbox series

[net-next,v4,1/5] net: stmmac: enable clause 45 mdio support

Message ID 1559149107-14631-2-git-send-email-weifeng.voon@intel.com
State Changes Requested
Delegated to: David Miller
Headers show
Series net: stmmac: enable EHL SGMI | expand

Commit Message

Voon, Weifeng May 29, 2019, 4:58 p.m. UTC 
From: Kweh Hock Leong <hock.leong.kweh@intel.com>

DWMAC4 is capable to support clause 45 mdio communication.
This patch enable the feature on stmmac_mdio_write() and
stmmac_mdio_read() by following phy_write_mmd() and
phy_read_mmd() mdiobus read write implementation format.

Reviewed-by: Li, Yifan <yifan2.li@intel.com>
Signed-off-by: Kweh Hock Leong <hock.leong.kweh@intel.com>
Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
Signed-off-by: Weifeng Voon <weifeng.voon@intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 40 ++++++++++++++++++-----
 include/linux/phy.h                               |  2 ++
 2 files changed, 34 insertions(+), 8 deletions(-)

Comments

Jose Abreu May 29, 2019, 10:16 a.m. UTC | #1 
From: Voon Weifeng <weifeng.voon@intel.com>
Date: Wed, May 29, 2019 at 17:58:23

> +static void stmmac_mdio_c45_setup(struct stmmac_priv *priv, int phyreg,
> +				  u32 *val, u32 *data)
> +{
> +	unsigned int reg_shift = priv->hw->mii.reg_shift;
> +	unsigned int reg_mask = priv->hw->mii.reg_mask;

Reverse christmas tree here. You also should align the function variables 
with the opening parenthesis of the function here and in the remaining 
series.

Otherwise this patch looks good to me.

Thanks,
Jose Miguel Abreu
Voon, Weifeng May 29, 2019, 5:39 p.m. UTC | #2 
> > +static void stmmac_mdio_c45_setup(struct stmmac_priv *priv, int phyreg,
> > +				  u32 *val, u32 *data)
> > +{
> > +	unsigned int reg_shift = priv->hw->mii.reg_shift;
> > +	unsigned int reg_mask = priv->hw->mii.reg_mask;
> 
> Reverse christmas tree here. You also should align the function variables with
> the opening parenthesis of the function here and in the remaining series.
> 
> Otherwise this patch looks good to me.

It is already reversed Christmas tree. Somehow each of the character's width in the
email is not equal. As you can see that the first line of assignment is having more
character than the second line of assignment.
The alignment is also correct when I check it after doing a git am.

Regards,
Weifeng

> 
> Thanks,
> Jose Miguel Abreu
Andrew Lunn May 29, 2019, 11:06 p.m. UTC | #3 
On Wed, May 29, 2019 at 05:39:44PM +0000, Voon, Weifeng wrote:
> > > +static void stmmac_mdio_c45_setup(struct stmmac_priv *priv, int phyreg,
> > > +				  u32 *val, u32 *data)
> > > +{
> > > +	unsigned int reg_shift = priv->hw->mii.reg_shift;
> > > +	unsigned int reg_mask = priv->hw->mii.reg_mask;
> > 
> > Reverse christmas tree here. You also should align the function variables with
> > the opening parenthesis of the function here and in the remaining series.
> > 
> > Otherwise this patch looks good to me.
> 
> It is already reversed Christmas tree.

Yes.

> Somehow each of the character's width in the
> email is not equal.

Sounds like somebody is using a proportional font in there email
client. Bad idea.

	Andrew
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index bdd351597b55..c3d8f1d145ec 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -34,11 +34,27 @@ 
 
 #define MII_BUSY 0x00000001
 #define MII_WRITE 0x00000002
+#define MII_DATA_MASK GENMASK(15, 0)
 
 /* GMAC4 defines */
 #define MII_GMAC4_GOC_SHIFT		2
+#define MII_GMAC4_REG_ADDR_SHIFT	16
 #define MII_GMAC4_WRITE			(1 << MII_GMAC4_GOC_SHIFT)
 #define MII_GMAC4_READ			(3 << MII_GMAC4_GOC_SHIFT)
+#define MII_GMAC4_C45E			BIT(1)
+
+static void stmmac_mdio_c45_setup(struct stmmac_priv *priv, int phyreg,
+				  u32 *val, u32 *data)
+{
+	unsigned int reg_shift = priv->hw->mii.reg_shift;
+	unsigned int reg_mask = priv->hw->mii.reg_mask;
+
+	*val |= MII_GMAC4_C45E;
+	*val &= ~reg_mask;
+	*val |= ((phyreg >> MII_DEVADDR_C45_SHIFT) << reg_shift) & reg_mask;
+
+	*data |= (phyreg & MII_REGADDR_C45_MASK) << MII_GMAC4_REG_ADDR_SHIFT;
+}
 
 /* XGMAC defines */
 #define MII_XGMAC_SADDR			BIT(18)
@@ -165,22 +181,26 @@  static int stmmac_mdio_read(struct mii_bus *bus, int phyaddr, int phyreg)
 	struct stmmac_priv *priv = netdev_priv(ndev);
 	unsigned int mii_address = priv->hw->mii.addr;
 	unsigned int mii_data = priv->hw->mii.data;
-	u32 v;
-	int data;
 	u32 value = MII_BUSY;
+	int data = 0;
+	u32 v;
 
 	value |= (phyaddr << priv->hw->mii.addr_shift)
 		& priv->hw->mii.addr_mask;
 	value |= (phyreg << priv->hw->mii.reg_shift) & priv->hw->mii.reg_mask;
 	value |= (priv->clk_csr << priv->hw->mii.clk_csr_shift)
 		& priv->hw->mii.clk_csr_mask;
-	if (priv->plat->has_gmac4)
+	if (priv->plat->has_gmac4) {
 		value |= MII_GMAC4_READ;
+		if (phyreg & MII_ADDR_C45)
+			stmmac_mdio_c45_setup(priv, phyreg, &value, &data);
+	}
 
 	if (readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & MII_BUSY),
 			       100, 10000))
 		return -EBUSY;
 
+	writel(data, priv->ioaddr + mii_data);
 	writel(value, priv->ioaddr + mii_address);
 
 	if (readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & MII_BUSY),
@@ -188,7 +208,7 @@  static int stmmac_mdio_read(struct mii_bus *bus, int phyaddr, int phyreg)
 		return -EBUSY;
 
 	/* Read the data from the MII data register */
-	data = (int)readl(priv->ioaddr + mii_data);
+	data = (int)readl(priv->ioaddr + mii_data) & MII_DATA_MASK;
 
 	return data;
 }
@@ -208,8 +228,9 @@  static int stmmac_mdio_write(struct mii_bus *bus, int phyaddr, int phyreg,
 	struct stmmac_priv *priv = netdev_priv(ndev);
 	unsigned int mii_address = priv->hw->mii.addr;
 	unsigned int mii_data = priv->hw->mii.data;
-	u32 v;
 	u32 value = MII_BUSY;
+	int data = phydata;
+	u32 v;
 
 	value |= (phyaddr << priv->hw->mii.addr_shift)
 		& priv->hw->mii.addr_mask;
@@ -217,10 +238,13 @@  static int stmmac_mdio_write(struct mii_bus *bus, int phyaddr, int phyreg,
 
 	value |= (priv->clk_csr << priv->hw->mii.clk_csr_shift)
 		& priv->hw->mii.clk_csr_mask;
-	if (priv->plat->has_gmac4)
+	if (priv->plat->has_gmac4) {
 		value |= MII_GMAC4_WRITE;
-	else
+		if (phyreg & MII_ADDR_C45)
+			stmmac_mdio_c45_setup(priv, phyreg, &value, &data);
+	} else {
 		value |= MII_WRITE;
+	}
 
 	/* Wait until any existing MII operation is complete */
 	if (readl_poll_timeout(priv->ioaddr + mii_address, v, !(v & MII_BUSY),
@@ -228,7 +252,7 @@  static int stmmac_mdio_write(struct mii_bus *bus, int phyaddr, int phyreg,
 		return -EBUSY;
 
 	/* Set the MII address register to write */
-	writel(phydata, priv->ioaddr + mii_data);
+	writel(data, priv->ioaddr + mii_data);
 	writel(value, priv->ioaddr + mii_address);
 
 	/* Wait until any existing MII operation is complete */
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7180b1d1e5e3..27d267bde363 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -201,6 +201,8 @@  static inline const char *phy_modes(phy_interface_t interface)
 /* Or MII_ADDR_C45 into regnum for read/write on mii_bus to enable the 21 bit
    IEEE 802.3ae clause 45 addressing mode used by 10GIGE phy chips. */
 #define MII_ADDR_C45 (1<<30)
+#define MII_DEVADDR_C45_SHIFT	16
+#define MII_REGADDR_C45_MASK	GENMASK(15, 0)
 
 struct device;
 struct phylink;