[PATCH 0/5] add NS2 support to bgmac

[PATCH 0/5] add NS2 support to bgmac

[Jon Mason]

Add support for the amac found in the Broadcom Northstar2 SoC to the
bgmac driver.  This necessitates adding support to connect to an
externally defined phy (as described in the device tree) in the driver.
These phy changes are in addition to the changes necessary to get NS2
working.

This series (based on the net-next git tree) has been tested on NSP and
NS2 HW.

Jon Mason (4):
  Documentation: devicetree: net: add NS2 bindings to amac
  net: ethernet: bgmac: device tree phy enablement
  net: ethernet: bgmac: add NS2 support
  arm64: dts: NS2: add AMAC ethernet support

Vikas Soni (1):
  net: phy: broadcom: Add BCM54810 phy entry

 .../devicetree/bindings/net/brcm,amac.txt          |   7 +-
 arch/arm64/boot/dts/broadcom/ns2-svk.dts           |   5 +
 arch/arm64/boot/dts/broadcom/ns2.dtsi              |  12 +++
 drivers/net/ethernet/broadcom/bgmac-bcma.c         |  48 +++++++++
 drivers/net/ethernet/broadcom/bgmac-platform.c     | 108 ++++++++++++++++++++-
 drivers/net/ethernet/broadcom/bgmac.c              |  56 +----------
 drivers/net/ethernet/broadcom/bgmac.h              |   9 ++
 drivers/net/phy/Kconfig                            |   2 +-
 drivers/net/phy/broadcom.c                         |  65 +++++++++++++
 include/linux/brcmphy.h                            |   7 ++
 10 files changed, 264 insertions(+), 55 deletions(-)

-- 
2.7.4

[PATCH 0/5] add NS2 support to bgmac

[Jon Mason]

Add support for the amac found in the Broadcom Northstar2 SoC to the
bgmac driver.  This necessitates adding support to connect to an
externally defined phy (as described in the device tree) in the driver.
These phy changes are in addition to the changes necessary to get NS2
working.

This series (based on the net-next git tree) has been tested on NSP and
NS2 HW.

Jon Mason (4):
  Documentation: devicetree: net: add NS2 bindings to amac
  net: ethernet: bgmac: device tree phy enablement
  net: ethernet: bgmac: add NS2 support
  arm64: dts: NS2: add AMAC ethernet support

Vikas Soni (1):
  net: phy: broadcom: Add BCM54810 phy entry

 .../devicetree/bindings/net/brcm,amac.txt          |   7 +-
 arch/arm64/boot/dts/broadcom/ns2-svk.dts           |   5 +
 arch/arm64/boot/dts/broadcom/ns2.dtsi              |  12 +++
 drivers/net/ethernet/broadcom/bgmac-bcma.c         |  48 +++++++++
 drivers/net/ethernet/broadcom/bgmac-platform.c     | 108 ++++++++++++++++++++-
 drivers/net/ethernet/broadcom/bgmac.c              |  56 +----------
 drivers/net/ethernet/broadcom/bgmac.h              |   9 ++
 drivers/net/phy/Kconfig                            |   2 +-
 drivers/net/phy/broadcom.c                         |  65 +++++++++++++
 include/linux/brcmphy.h                            |   7 ++
 10 files changed, 264 insertions(+), 55 deletions(-)

-- 
2.7.4

[PATCH 1/5] net: phy: broadcom: Add BCM54810 phy entry

[Jon Mason]

From: Vikas Soni <vsoni@broadcom.com>

Add BCM54810 phy entry

Signed-off-by: Vikas Soni <vsoni@broadcom.com>
Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 drivers/net/phy/Kconfig    |  2 +-
 drivers/net/phy/broadcom.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/brcmphy.h    |  7 +++++
 3 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 45f68ea..31967ca 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -217,7 +217,7 @@ config BROADCOM_PHY
 	select BCM_NET_PHYLIB
 	---help---
 	  Currently supports the BCM5411, BCM5421, BCM5461, BCM54616S, BCM5464,
-	  BCM5481 and BCM5482 PHYs.
+	  BCM5481, BCM54810 and BCM5482 PHYs.
 
 config CICADA_PHY
 	tristate "Cicada PHYs"
diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 870327e..cdce761 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -35,6 +35,35 @@ static int bcm54xx_auxctl_write(struct phy_device *phydev, u16 regnum, u16 val)
 	return phy_write(phydev, MII_BCM54XX_AUX_CTL, regnum | val);
 }
 
+static int bcm54810_config(struct phy_device *phydev)
+{
+	int rc;
+
+	/* Disable BroadR-Reach */
+	rc = bcm_phy_write_exp(phydev, BCM54810_EXP_BROADREACH_LRE_MISC_CTL, 0);
+	if (rc < 0)
+		return rc;
+
+	/* SKEW DISABLE */
+	rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
+				  0xF0E0);
+	if (rc < 0)
+		return rc;
+
+	/* DELAY DISABLE */
+	rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
+				  0x7000);
+	if (rc < 0)
+		return rc;
+
+	/* DELAY DISABLE */
+	rc = bcm_phy_write_shadow(phydev, BCM54810_SHD_CLK_CTL, 0);
+	if (rc < 0)
+		return rc;
+
+	return 0;
+}
+
 /* Needs SMDSP clock enabled via bcm54xx_phydsp_config() */
 static int bcm50610_a0_workaround(struct phy_device *phydev)
 {
@@ -207,6 +236,20 @@ static int bcm54xx_config_init(struct phy_device *phydev)
 	    (phydev->dev_flags & PHY_BRCM_AUTO_PWRDWN_ENABLE))
 		bcm54xx_adjust_rxrefclk(phydev);
 
+	if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810) {
+		err = bcm54810_config(phydev);
+		if (err)
+			return err;
+
+		reg = phy_read(phydev, MII_BMCR);
+		if (reg < 0)
+			return reg;
+
+		err = phy_write(phydev, MII_BMCR, reg & ~BMCR_PDOWN);
+		if (err)
+			return err;
+	}
+
 	bcm54xx_phydsp_config(phydev);
 
 	return 0;
@@ -334,6 +377,15 @@ static int bcm5481_config_aneg(struct phy_device *phydev)
 		phy_write(phydev, 0x18, reg);
 	}
 
+	if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810 &&
+	    phydev->dev_flags & PHY_BRCM_EXP_LANE_SWAP) {
+		/* LANE SWAP */
+		ret = bcm_phy_write_exp(phydev, MII_BCM54XX_EXP_SEL_ER + 0x9,
+					0x11B);
+		if (ret < 0)
+			return ret;
+	}
+
 	return ret;
 }
 
@@ -521,6 +573,18 @@ static struct phy_driver broadcom_drivers[] = {
 	.ack_interrupt	= bcm_phy_ack_intr,
 	.config_intr	= bcm_phy_config_intr,
 }, {
+	.phy_id         = PHY_ID_BCM54810,
+	.phy_id_mask    = 0xfffffff0,
+	.name           = "Broadcom BCM54810",
+	.features       = PHY_GBIT_FEATURES |
+			  SUPPORTED_Pause | SUPPORTED_Asym_Pause,
+	.flags          = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.config_init    = bcm54xx_config_init,
+	.config_aneg    = bcm5481_config_aneg,
+	.read_status    = genphy_read_status,
+	.ack_interrupt  = bcm_phy_ack_intr,
+	.config_intr    = bcm_phy_config_intr,
+}, {
 	.phy_id		= PHY_ID_BCM5482,
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Broadcom BCM5482",
@@ -603,6 +667,7 @@ static struct mdio_device_id __maybe_unused broadcom_tbl[] = {
 	{ PHY_ID_BCM54616S, 0xfffffff0 },
 	{ PHY_ID_BCM5464, 0xfffffff0 },
 	{ PHY_ID_BCM5481, 0xfffffff0 },
+	{ PHY_ID_BCM54810, 0xfffffff0 },
 	{ PHY_ID_BCM5482, 0xfffffff0 },
 	{ PHY_ID_BCM50610, 0xfffffff0 },
 	{ PHY_ID_BCM50610M, 0xfffffff0 },
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index e7a3d9d..40e6ead 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -13,6 +13,7 @@
 #define PHY_ID_BCM5241			0x0143bc30
 #define PHY_ID_BCMAC131			0x0143bc70
 #define PHY_ID_BCM5481			0x0143bca0
+#define PHY_ID_BCM54810			0x03625d00
 #define PHY_ID_BCM5482			0x0143bcb0
 #define PHY_ID_BCM5411			0x00206070
 #define PHY_ID_BCM5421			0x002060e0
@@ -55,6 +56,8 @@
 #define PHY_BRCM_EXT_IBND_TX_ENABLE	0x00002000
 #define PHY_BRCM_CLEAR_RGMII_MODE	0x00004000
 #define PHY_BRCM_DIS_TXCRXC_NOENRGY	0x00008000
+#define PHY_BRCM_EXP_LANE_SWAP		0x00010000
+
 /* Broadcom BCM7xxx specific workarounds */
 #define PHY_BRCM_7XXX_REV(x)		(((x) >> 8) & 0xff)
 #define PHY_BRCM_7XXX_PATCH(x)		((x) & 0xff)
@@ -187,6 +190,10 @@
 #define BCM5482_SSD_SGMII_SLAVE_EN	0x0002	/* Slave mode enable */
 #define BCM5482_SSD_SGMII_SLAVE_AD	0x0001	/* Slave auto-detection */
 
+/* BCM54810 Registers */
+#define BCM54810_EXP_BROADREACH_LRE_MISC_CTL	(MII_BCM54XX_EXP_SEL_ER + 0x90)
+#define BCM54810_SHD_CLK_CTL			0x3
+
 
 /*****************************************************************************/
 /* Fast Ethernet Transceiver definitions. */
-- 
2.7.4

[PATCH 1/5] net: phy: broadcom: Add BCM54810 phy entry

[Jon Mason]

From: Vikas Soni <vsoni@broadcom.com>

Add BCM54810 phy entry

Signed-off-by: Vikas Soni <vsoni@broadcom.com>
Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 drivers/net/phy/Kconfig    |  2 +-
 drivers/net/phy/broadcom.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/brcmphy.h    |  7 +++++
 3 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 45f68ea..31967ca 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -217,7 +217,7 @@ config BROADCOM_PHY
 	select BCM_NET_PHYLIB
 	---help---
 	  Currently supports the BCM5411, BCM5421, BCM5461, BCM54616S, BCM5464,
-	  BCM5481 and BCM5482 PHYs.
+	  BCM5481, BCM54810 and BCM5482 PHYs.
 
 config CICADA_PHY
 	tristate "Cicada PHYs"
diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
index 870327e..cdce761 100644
--- a/drivers/net/phy/broadcom.c
+++ b/drivers/net/phy/broadcom.c
@@ -35,6 +35,35 @@ static int bcm54xx_auxctl_write(struct phy_device *phydev, u16 regnum, u16 val)
 	return phy_write(phydev, MII_BCM54XX_AUX_CTL, regnum | val);
 }
 
+static int bcm54810_config(struct phy_device *phydev)
+{
+	int rc;
+
+	/* Disable BroadR-Reach */
+	rc = bcm_phy_write_exp(phydev, BCM54810_EXP_BROADREACH_LRE_MISC_CTL, 0);
+	if (rc < 0)
+		return rc;
+
+	/* SKEW DISABLE */
+	rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
+				  0xF0E0);
+	if (rc < 0)
+		return rc;
+
+	/* DELAY DISABLE */
+	rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
+				  0x7000);
+	if (rc < 0)
+		return rc;
+
+	/* DELAY DISABLE */
+	rc = bcm_phy_write_shadow(phydev, BCM54810_SHD_CLK_CTL, 0);
+	if (rc < 0)
+		return rc;
+
+	return 0;
+}
+
 /* Needs SMDSP clock enabled via bcm54xx_phydsp_config() */
 static int bcm50610_a0_workaround(struct phy_device *phydev)
 {
@@ -207,6 +236,20 @@ static int bcm54xx_config_init(struct phy_device *phydev)
 	    (phydev->dev_flags & PHY_BRCM_AUTO_PWRDWN_ENABLE))
 		bcm54xx_adjust_rxrefclk(phydev);
 
+	if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810) {
+		err = bcm54810_config(phydev);
+		if (err)
+			return err;
+
+		reg = phy_read(phydev, MII_BMCR);
+		if (reg < 0)
+			return reg;
+
+		err = phy_write(phydev, MII_BMCR, reg & ~BMCR_PDOWN);
+		if (err)
+			return err;
+	}
+
 	bcm54xx_phydsp_config(phydev);
 
 	return 0;
@@ -334,6 +377,15 @@ static int bcm5481_config_aneg(struct phy_device *phydev)
 		phy_write(phydev, 0x18, reg);
 	}
 
+	if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810 &&
+	    phydev->dev_flags & PHY_BRCM_EXP_LANE_SWAP) {
+		/* LANE SWAP */
+		ret = bcm_phy_write_exp(phydev, MII_BCM54XX_EXP_SEL_ER + 0x9,
+					0x11B);
+		if (ret < 0)
+			return ret;
+	}
+
 	return ret;
 }
 
@@ -521,6 +573,18 @@ static struct phy_driver broadcom_drivers[] = {
 	.ack_interrupt	= bcm_phy_ack_intr,
 	.config_intr	= bcm_phy_config_intr,
 }, {
+	.phy_id         = PHY_ID_BCM54810,
+	.phy_id_mask    = 0xfffffff0,
+	.name           = "Broadcom BCM54810",
+	.features       = PHY_GBIT_FEATURES |
+			  SUPPORTED_Pause | SUPPORTED_Asym_Pause,
+	.flags          = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.config_init    = bcm54xx_config_init,
+	.config_aneg    = bcm5481_config_aneg,
+	.read_status    = genphy_read_status,
+	.ack_interrupt  = bcm_phy_ack_intr,
+	.config_intr    = bcm_phy_config_intr,
+}, {
 	.phy_id		= PHY_ID_BCM5482,
 	.phy_id_mask	= 0xfffffff0,
 	.name		= "Broadcom BCM5482",
@@ -603,6 +667,7 @@ static struct mdio_device_id __maybe_unused broadcom_tbl[] = {
 	{ PHY_ID_BCM54616S, 0xfffffff0 },
 	{ PHY_ID_BCM5464, 0xfffffff0 },
 	{ PHY_ID_BCM5481, 0xfffffff0 },
+	{ PHY_ID_BCM54810, 0xfffffff0 },
 	{ PHY_ID_BCM5482, 0xfffffff0 },
 	{ PHY_ID_BCM50610, 0xfffffff0 },
 	{ PHY_ID_BCM50610M, 0xfffffff0 },
diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
index e7a3d9d..40e6ead 100644
--- a/include/linux/brcmphy.h
+++ b/include/linux/brcmphy.h
@@ -13,6 +13,7 @@
 #define PHY_ID_BCM5241			0x0143bc30
 #define PHY_ID_BCMAC131			0x0143bc70
 #define PHY_ID_BCM5481			0x0143bca0
+#define PHY_ID_BCM54810			0x03625d00
 #define PHY_ID_BCM5482			0x0143bcb0
 #define PHY_ID_BCM5411			0x00206070
 #define PHY_ID_BCM5421			0x002060e0
@@ -55,6 +56,8 @@
 #define PHY_BRCM_EXT_IBND_TX_ENABLE	0x00002000
 #define PHY_BRCM_CLEAR_RGMII_MODE	0x00004000
 #define PHY_BRCM_DIS_TXCRXC_NOENRGY	0x00008000
+#define PHY_BRCM_EXP_LANE_SWAP		0x00010000
+
 /* Broadcom BCM7xxx specific workarounds */
 #define PHY_BRCM_7XXX_REV(x)		(((x) >> 8) & 0xff)
 #define PHY_BRCM_7XXX_PATCH(x)		((x) & 0xff)
@@ -187,6 +190,10 @@
 #define BCM5482_SSD_SGMII_SLAVE_EN	0x0002	/* Slave mode enable */
 #define BCM5482_SSD_SGMII_SLAVE_AD	0x0001	/* Slave auto-detection */
 
+/* BCM54810 Registers */
+#define BCM54810_EXP_BROADREACH_LRE_MISC_CTL	(MII_BCM54XX_EXP_SEL_ER + 0x90)
+#define BCM54810_SHD_CLK_CTL			0x3
+
 
 /*****************************************************************************/
 /* Fast Ethernet Transceiver definitions. */
-- 
2.7.4

Re: [PATCH 1/5] net: phy: broadcom: Add BCM54810 phy entry

[Andrew Lunn]

On Wed, Oct 26, 2016 at 03:35:57PM -0400, Jon Mason wrote:
> From: Vikas Soni <vsoni@broadcom.com>
> 
> Add BCM54810 phy entry

Hi Jon, Vikis

The subject line is a bit misleading. It does more than add a PHY ID
entry.

> Signed-off-by: Vikas Soni <vsoni@broadcom.com>
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---
>  drivers/net/phy/Kconfig    |  2 +-
>  drivers/net/phy/broadcom.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/brcmphy.h    |  7 +++++
>  3 files changed, 73 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 45f68ea..31967ca 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -217,7 +217,7 @@ config BROADCOM_PHY
>  	select BCM_NET_PHYLIB
>  	---help---
>  	  Currently supports the BCM5411, BCM5421, BCM5461, BCM54616S, BCM5464,
> -	  BCM5481 and BCM5482 PHYs.
> +	  BCM5481, BCM54810 and BCM5482 PHYs.
>  
>  config CICADA_PHY
>  	tristate "Cicada PHYs"
> diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
> index 870327e..cdce761 100644
> --- a/drivers/net/phy/broadcom.c
> +++ b/drivers/net/phy/broadcom.c
> @@ -35,6 +35,35 @@ static int bcm54xx_auxctl_write(struct phy_device *phydev, u16 regnum, u16 val)
>  	return phy_write(phydev, MII_BCM54XX_AUX_CTL, regnum | val);
>  }
>  
> +static int bcm54810_config(struct phy_device *phydev)
> +{
> +	int rc;
> +
> +	/* Disable BroadR-Reach */
> +	rc = bcm_phy_write_exp(phydev, BCM54810_EXP_BROADREACH_LRE_MISC_CTL, 0);
> +	if (rc < 0)
> +		return rc;
> +
> +	/* SKEW DISABLE */
> +	rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
> +				  0xF0E0);
> +	if (rc < 0)
> +		return rc;
> +
> +	/* DELAY DISABLE */
> +	rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
> +				  0x7000);

This driver mostly uses symbolic names, not #defines. Please can you
use #defines here and else were in this patch.


> +	if (rc < 0)
> +		return rc;
> +
> +	/* DELAY DISABLE */
> +	rc = bcm_phy_write_shadow(phydev, BCM54810_SHD_CLK_CTL, 0);
> +	if (rc < 0)
> +		return rc;

Twice the same comment?

> +
> +	return 0;
> +}
> +
>  /* Needs SMDSP clock enabled via bcm54xx_phydsp_config() */
>  static int bcm50610_a0_workaround(struct phy_device *phydev)
>  {
> @@ -207,6 +236,20 @@ static int bcm54xx_config_init(struct phy_device *phydev)
>  	    (phydev->dev_flags & PHY_BRCM_AUTO_PWRDWN_ENABLE))
>  		bcm54xx_adjust_rxrefclk(phydev);
>  
> +	if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810) {
> +		err = bcm54810_config(phydev);
> +		if (err)
> +			return err;
> +
> +		reg = phy_read(phydev, MII_BMCR);
> +		if (reg < 0)
> +			return reg;
> +
> +		err = phy_write(phydev, MII_BMCR, reg & ~BMCR_PDOWN);
> +		if (err)
> +			return err;

This seems a bit odd. I would expect the PHY core correctly handles
the PHY being powered down. Can you explain this a bit more, why it is
needed.

	Thanks
		Andrew

[PATCH 1/5] net: phy: broadcom: Add BCM54810 phy entry

[Andrew Lunn]

On Wed, Oct 26, 2016 at 03:35:57PM -0400, Jon Mason wrote:
> From: Vikas Soni <vsoni@broadcom.com>
> 
> Add BCM54810 phy entry

Hi Jon, Vikis

The subject line is a bit misleading. It does more than add a PHY ID
entry.

> Signed-off-by: Vikas Soni <vsoni@broadcom.com>
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---
>  drivers/net/phy/Kconfig    |  2 +-
>  drivers/net/phy/broadcom.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/brcmphy.h    |  7 +++++
>  3 files changed, 73 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 45f68ea..31967ca 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -217,7 +217,7 @@ config BROADCOM_PHY
>  	select BCM_NET_PHYLIB
>  	---help---
>  	  Currently supports the BCM5411, BCM5421, BCM5461, BCM54616S, BCM5464,
> -	  BCM5481 and BCM5482 PHYs.
> +	  BCM5481, BCM54810 and BCM5482 PHYs.
>  
>  config CICADA_PHY
>  	tristate "Cicada PHYs"
> diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
> index 870327e..cdce761 100644
> --- a/drivers/net/phy/broadcom.c
> +++ b/drivers/net/phy/broadcom.c
> @@ -35,6 +35,35 @@ static int bcm54xx_auxctl_write(struct phy_device *phydev, u16 regnum, u16 val)
>  	return phy_write(phydev, MII_BCM54XX_AUX_CTL, regnum | val);
>  }
>  
> +static int bcm54810_config(struct phy_device *phydev)
> +{
> +	int rc;
> +
> +	/* Disable BroadR-Reach */
> +	rc = bcm_phy_write_exp(phydev, BCM54810_EXP_BROADREACH_LRE_MISC_CTL, 0);
> +	if (rc < 0)
> +		return rc;
> +
> +	/* SKEW DISABLE */
> +	rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
> +				  0xF0E0);
> +	if (rc < 0)
> +		return rc;
> +
> +	/* DELAY DISABLE */
> +	rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
> +				  0x7000);

This driver mostly uses symbolic names, not #defines. Please can you
use #defines here and else were in this patch.


> +	if (rc < 0)
> +		return rc;
> +
> +	/* DELAY DISABLE */
> +	rc = bcm_phy_write_shadow(phydev, BCM54810_SHD_CLK_CTL, 0);
> +	if (rc < 0)
> +		return rc;

Twice the same comment?

> +
> +	return 0;
> +}
> +
>  /* Needs SMDSP clock enabled via bcm54xx_phydsp_config() */
>  static int bcm50610_a0_workaround(struct phy_device *phydev)
>  {
> @@ -207,6 +236,20 @@ static int bcm54xx_config_init(struct phy_device *phydev)
>  	    (phydev->dev_flags & PHY_BRCM_AUTO_PWRDWN_ENABLE))
>  		bcm54xx_adjust_rxrefclk(phydev);
>  
> +	if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810) {
> +		err = bcm54810_config(phydev);
> +		if (err)
> +			return err;
> +
> +		reg = phy_read(phydev, MII_BMCR);
> +		if (reg < 0)
> +			return reg;
> +
> +		err = phy_write(phydev, MII_BMCR, reg & ~BMCR_PDOWN);
> +		if (err)
> +			return err;

This seems a bit odd. I would expect the PHY core correctly handles
the PHY being powered down. Can you explain this a bit more, why it is
needed.

	Thanks
		Andrew

Re: [PATCH 1/5] net: phy: broadcom: Add BCM54810 phy entry

[Jon Mason]

On Thu, Oct 27, 2016 at 11:15:05AM +0200, Andrew Lunn wrote:
> On Wed, Oct 26, 2016 at 03:35:57PM -0400, Jon Mason wrote:
> > From: Vikas Soni <vsoni@broadcom.com>
> > 
> > Add BCM54810 phy entry
> 
> Hi Jon, Vikis
> 
> The subject line is a bit misleading. It does more than add a PHY ID
> entry.

All of the parts are related to adding the BCM54810 Phy, but I agree it
could be more verbose about what is happening.

> > Signed-off-by: Vikas Soni <vsoni@broadcom.com>
> > Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> > ---
> >  drivers/net/phy/Kconfig    |  2 +-
> >  drivers/net/phy/broadcom.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/brcmphy.h    |  7 +++++
> >  3 files changed, 73 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> > index 45f68ea..31967ca 100644
> > --- a/drivers/net/phy/Kconfig
> > +++ b/drivers/net/phy/Kconfig
> > @@ -217,7 +217,7 @@ config BROADCOM_PHY
> >  	select BCM_NET_PHYLIB
> >  	---help---
> >  	  Currently supports the BCM5411, BCM5421, BCM5461, BCM54616S, BCM5464,
> > -	  BCM5481 and BCM5482 PHYs.
> > +	  BCM5481, BCM54810 and BCM5482 PHYs.
> >  
> >  config CICADA_PHY
> >  	tristate "Cicada PHYs"
> > diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
> > index 870327e..cdce761 100644
> > --- a/drivers/net/phy/broadcom.c
> > +++ b/drivers/net/phy/broadcom.c
> > @@ -35,6 +35,35 @@ static int bcm54xx_auxctl_write(struct phy_device *phydev, u16 regnum, u16 val)
> >  	return phy_write(phydev, MII_BCM54XX_AUX_CTL, regnum | val);
> >  }
> >  
> > +static int bcm54810_config(struct phy_device *phydev)
> > +{
> > +	int rc;
> > +
> > +	/* Disable BroadR-Reach */
> > +	rc = bcm_phy_write_exp(phydev, BCM54810_EXP_BROADREACH_LRE_MISC_CTL, 0);
> > +	if (rc < 0)
> > +		return rc;
> > +
> > +	/* SKEW DISABLE */
> > +	rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
> > +				  0xF0E0);
> > +	if (rc < 0)
> > +		return rc;
> > +
> > +	/* DELAY DISABLE */
> > +	rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
> > +				  0x7000);
> 
> This driver mostly uses symbolic names, not #defines. Please can you
> use #defines here and else were in this patch.

Will do.  After looking at this, this appears to be setup for a read
that doesn't follow.  I'll audit this, clean it up and resend.

 
> > +	if (rc < 0)
> > +		return rc;
> > +
> > +	/* DELAY DISABLE */
> > +	rc = bcm_phy_write_shadow(phydev, BCM54810_SHD_CLK_CTL, 0);
> > +	if (rc < 0)
> > +		return rc;
> 
> Twice the same comment?
>
> > +
> > +	return 0;
> > +}
> > +
> >  /* Needs SMDSP clock enabled via bcm54xx_phydsp_config() */
> >  static int bcm50610_a0_workaround(struct phy_device *phydev)
> >  {
> > @@ -207,6 +236,20 @@ static int bcm54xx_config_init(struct phy_device *phydev)
> >  	    (phydev->dev_flags & PHY_BRCM_AUTO_PWRDWN_ENABLE))
> >  		bcm54xx_adjust_rxrefclk(phydev);
> >  
> > +	if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810) {
> > +		err = bcm54810_config(phydev);
> > +		if (err)
> > +			return err;
> > +
> > +		reg = phy_read(phydev, MII_BMCR);
> > +		if (reg < 0)
> > +			return reg;
> > +
> > +		err = phy_write(phydev, MII_BMCR, reg & ~BMCR_PDOWN);
> > +		if (err)
> > +			return err;
> 
> This seems a bit odd. I would expect the PHY core correctly handles
> the PHY being powered down. Can you explain this a bit more, why it is
> needed.

I believe it was needed in earlier versions of the code, but doesn't
seem to be needed anymore.  Removing.

> 
> 	Thanks
> 		Andrew

[PATCH 1/5] net: phy: broadcom: Add BCM54810 phy entry

[Jon Mason]

On Thu, Oct 27, 2016 at 11:15:05AM +0200, Andrew Lunn wrote:
> On Wed, Oct 26, 2016 at 03:35:57PM -0400, Jon Mason wrote:
> > From: Vikas Soni <vsoni@broadcom.com>
> > 
> > Add BCM54810 phy entry
> 
> Hi Jon, Vikis
> 
> The subject line is a bit misleading. It does more than add a PHY ID
> entry.

All of the parts are related to adding the BCM54810 Phy, but I agree it
could be more verbose about what is happening.

> > Signed-off-by: Vikas Soni <vsoni@broadcom.com>
> > Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> > ---
> >  drivers/net/phy/Kconfig    |  2 +-
> >  drivers/net/phy/broadcom.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/brcmphy.h    |  7 +++++
> >  3 files changed, 73 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> > index 45f68ea..31967ca 100644
> > --- a/drivers/net/phy/Kconfig
> > +++ b/drivers/net/phy/Kconfig
> > @@ -217,7 +217,7 @@ config BROADCOM_PHY
> >  	select BCM_NET_PHYLIB
> >  	---help---
> >  	  Currently supports the BCM5411, BCM5421, BCM5461, BCM54616S, BCM5464,
> > -	  BCM5481 and BCM5482 PHYs.
> > +	  BCM5481, BCM54810 and BCM5482 PHYs.
> >  
> >  config CICADA_PHY
> >  	tristate "Cicada PHYs"
> > diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
> > index 870327e..cdce761 100644
> > --- a/drivers/net/phy/broadcom.c
> > +++ b/drivers/net/phy/broadcom.c
> > @@ -35,6 +35,35 @@ static int bcm54xx_auxctl_write(struct phy_device *phydev, u16 regnum, u16 val)
> >  	return phy_write(phydev, MII_BCM54XX_AUX_CTL, regnum | val);
> >  }
> >  
> > +static int bcm54810_config(struct phy_device *phydev)
> > +{
> > +	int rc;
> > +
> > +	/* Disable BroadR-Reach */
> > +	rc = bcm_phy_write_exp(phydev, BCM54810_EXP_BROADREACH_LRE_MISC_CTL, 0);
> > +	if (rc < 0)
> > +		return rc;
> > +
> > +	/* SKEW DISABLE */
> > +	rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
> > +				  0xF0E0);
> > +	if (rc < 0)
> > +		return rc;
> > +
> > +	/* DELAY DISABLE */
> > +	rc = bcm54xx_auxctl_write(phydev, MII_BCM54XX_AUXCTL_SHDWSEL_MISC,
> > +				  0x7000);
> 
> This driver mostly uses symbolic names, not #defines. Please can you
> use #defines here and else were in this patch.

Will do.  After looking at this, this appears to be setup for a read
that doesn't follow.  I'll audit this, clean it up and resend.

 
> > +	if (rc < 0)
> > +		return rc;
> > +
> > +	/* DELAY DISABLE */
> > +	rc = bcm_phy_write_shadow(phydev, BCM54810_SHD_CLK_CTL, 0);
> > +	if (rc < 0)
> > +		return rc;
> 
> Twice the same comment?
>
> > +
> > +	return 0;
> > +}
> > +
> >  /* Needs SMDSP clock enabled via bcm54xx_phydsp_config() */
> >  static int bcm50610_a0_workaround(struct phy_device *phydev)
> >  {
> > @@ -207,6 +236,20 @@ static int bcm54xx_config_init(struct phy_device *phydev)
> >  	    (phydev->dev_flags & PHY_BRCM_AUTO_PWRDWN_ENABLE))
> >  		bcm54xx_adjust_rxrefclk(phydev);
> >  
> > +	if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810) {
> > +		err = bcm54810_config(phydev);
> > +		if (err)
> > +			return err;
> > +
> > +		reg = phy_read(phydev, MII_BMCR);
> > +		if (reg < 0)
> > +			return reg;
> > +
> > +		err = phy_write(phydev, MII_BMCR, reg & ~BMCR_PDOWN);
> > +		if (err)
> > +			return err;
> 
> This seems a bit odd. I would expect the PHY core correctly handles
> the PHY being powered down. Can you explain this a bit more, why it is
> needed.

I believe it was needed in earlier versions of the code, but doesn't
seem to be needed anymore.  Removing.

> 
> 	Thanks
> 		Andrew

[PATCH 2/5] Documentation: devicetree: net: add NS2 bindings to amac

[Jon Mason]

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 Documentation/devicetree/bindings/net/brcm,amac.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/brcm,amac.txt b/Documentation/devicetree/bindings/net/brcm,amac.txt
index ba5ecc1..f92caee 100644
--- a/Documentation/devicetree/bindings/net/brcm,amac.txt
+++ b/Documentation/devicetree/bindings/net/brcm,amac.txt
@@ -2,15 +2,18 @@ Broadcom AMAC Ethernet Controller Device Tree Bindings
 -------------------------------------------------------------
 
 Required properties:
- - compatible:	"brcm,amac" or "brcm,nsp-amac"
+ - compatible:	"brcm,amac", "brcm,nsp-amac", or "brcm,ns2-amac"
  - reg:		Address and length of the GMAC registers,
 		Address and length of the GMAC IDM registers
+		Address and length of the NIC Port Manager registers (optional)
  - reg-names:	Names of the registers.  Must have both "amac_base" and
-		"idm_base"
+		"idm_base". "nicpm_base" is optional (required for NS2)
  - interrupts:	Interrupt number
 
 Optional properties:
 - mac-address:	See ethernet.txt file in the same directory
+- brcm,enet-phy-lane-swap:
+		boolean; Swap the PHY lanes (needed on some SKUs of NS2)
 
 Examples:
 
-- 
2.7.4

[PATCH 2/5] Documentation: devicetree: net: add NS2 bindings to amac

[Jon Mason]

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 Documentation/devicetree/bindings/net/brcm,amac.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/brcm,amac.txt b/Documentation/devicetree/bindings/net/brcm,amac.txt
index ba5ecc1..f92caee 100644
--- a/Documentation/devicetree/bindings/net/brcm,amac.txt
+++ b/Documentation/devicetree/bindings/net/brcm,amac.txt
@@ -2,15 +2,18 @@ Broadcom AMAC Ethernet Controller Device Tree Bindings
 -------------------------------------------------------------
 
 Required properties:
- - compatible:	"brcm,amac" or "brcm,nsp-amac"
+ - compatible:	"brcm,amac", "brcm,nsp-amac", or "brcm,ns2-amac"
  - reg:		Address and length of the GMAC registers,
 		Address and length of the GMAC IDM registers
+		Address and length of the NIC Port Manager registers (optional)
  - reg-names:	Names of the registers.  Must have both "amac_base" and
-		"idm_base"
+		"idm_base". "nicpm_base" is optional (required for NS2)
  - interrupts:	Interrupt number
 
 Optional properties:
 - mac-address:	See ethernet.txt file in the same directory
+- brcm,enet-phy-lane-swap:
+		boolean; Swap the PHY lanes (needed on some SKUs of NS2)
 
 Examples:
 
-- 
2.7.4

Re: [PATCH 2/5] Documentation: devicetree: net: add NS2 bindings to amac

[Andrew Lunn]

On Wed, Oct 26, 2016 at 03:35:58PM -0400, Jon Mason wrote:
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---
>  Documentation/devicetree/bindings/net/brcm,amac.txt | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/brcm,amac.txt b/Documentation/devicetree/bindings/net/brcm,amac.txt
> index ba5ecc1..f92caee 100644
> --- a/Documentation/devicetree/bindings/net/brcm,amac.txt
> +++ b/Documentation/devicetree/bindings/net/brcm,amac.txt
> @@ -2,15 +2,18 @@ Broadcom AMAC Ethernet Controller Device Tree Bindings
>  -------------------------------------------------------------
>  
>  Required properties:
> - - compatible:	"brcm,amac" or "brcm,nsp-amac"
> + - compatible:	"brcm,amac", "brcm,nsp-amac", or "brcm,ns2-amac"
>   - reg:		Address and length of the GMAC registers,
>  		Address and length of the GMAC IDM registers
> +		Address and length of the NIC Port Manager registers (optional)
>   - reg-names:	Names of the registers.  Must have both "amac_base" and
> -		"idm_base"
> +		"idm_base". "nicpm_base" is optional (required for NS2)
>   - interrupts:	Interrupt number
>  
>  Optional properties:
>  - mac-address:	See ethernet.txt file in the same directory
> +- brcm,enet-phy-lane-swap:
> +		boolean; Swap the PHY lanes (needed on some SKUs of NS2)

Maybe i'm missing something here, but the patch to the PHY swapped the
lanes. This seems to be a PHY property, not a MAC property. And it
swapped them unconditionally....

	Andrew

[PATCH 2/5] Documentation: devicetree: net: add NS2 bindings to amac

[Andrew Lunn]

On Wed, Oct 26, 2016 at 03:35:58PM -0400, Jon Mason wrote:
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---
>  Documentation/devicetree/bindings/net/brcm,amac.txt | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/brcm,amac.txt b/Documentation/devicetree/bindings/net/brcm,amac.txt
> index ba5ecc1..f92caee 100644
> --- a/Documentation/devicetree/bindings/net/brcm,amac.txt
> +++ b/Documentation/devicetree/bindings/net/brcm,amac.txt
> @@ -2,15 +2,18 @@ Broadcom AMAC Ethernet Controller Device Tree Bindings
>  -------------------------------------------------------------
>  
>  Required properties:
> - - compatible:	"brcm,amac" or "brcm,nsp-amac"
> + - compatible:	"brcm,amac", "brcm,nsp-amac", or "brcm,ns2-amac"
>   - reg:		Address and length of the GMAC registers,
>  		Address and length of the GMAC IDM registers
> +		Address and length of the NIC Port Manager registers (optional)
>   - reg-names:	Names of the registers.  Must have both "amac_base" and
> -		"idm_base"
> +		"idm_base". "nicpm_base" is optional (required for NS2)
>   - interrupts:	Interrupt number
>  
>  Optional properties:
>  - mac-address:	See ethernet.txt file in the same directory
> +- brcm,enet-phy-lane-swap:
> +		boolean; Swap the PHY lanes (needed on some SKUs of NS2)

Maybe i'm missing something here, but the patch to the PHY swapped the
lanes. This seems to be a PHY property, not a MAC property. And it
swapped them unconditionally....

	Andrew

Re: [PATCH 2/5] Documentation: devicetree: net: add NS2 bindings to amac

[Andrew Lunn]

On Wed, Oct 26, 2016 at 03:35:58PM -0400, Jon Mason wrote:
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---
>  Documentation/devicetree/bindings/net/brcm,amac.txt | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/brcm,amac.txt b/Documentation/devicetree/bindings/net/brcm,amac.txt
> index ba5ecc1..f92caee 100644
> --- a/Documentation/devicetree/bindings/net/brcm,amac.txt
> +++ b/Documentation/devicetree/bindings/net/brcm,amac.txt
> @@ -2,15 +2,18 @@ Broadcom AMAC Ethernet Controller Device Tree Bindings
>  -------------------------------------------------------------
>  
>  Required properties:
> - - compatible:	"brcm,amac" or "brcm,nsp-amac"
> + - compatible:	"brcm,amac", "brcm,nsp-amac", or "brcm,ns2-amac"
>   - reg:		Address and length of the GMAC registers,
>  		Address and length of the GMAC IDM registers
> +		Address and length of the NIC Port Manager registers (optional)
>   - reg-names:	Names of the registers.  Must have both "amac_base" and
> -		"idm_base"
> +		"idm_base". "nicpm_base" is optional (required for NS2)
>   - interrupts:	Interrupt number
>  
>  Optional properties:
>  - mac-address:	See ethernet.txt file in the same directory
> +- brcm,enet-phy-lane-swap:
> +		boolean; Swap the PHY lanes (needed on some SKUs of NS2)

Maybe i'm missing something here, but the patch to the PHY swapped the
lanes. This seems to be a PHY property, not a MAC property. And it
swapped them unconditionally....

	Andrew

Re: [PATCH 2/5] Documentation: devicetree: net: add NS2 bindings to amac

[Jon Mason]

On Thu, Oct 27, 2016 at 11:17:57AM +0200, Andrew Lunn wrote:
> On Wed, Oct 26, 2016 at 03:35:58PM -0400, Jon Mason wrote:
> > Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> > ---
> >  Documentation/devicetree/bindings/net/brcm,amac.txt | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/brcm,amac.txt b/Documentation/devicetree/bindings/net/brcm,amac.txt
> > index ba5ecc1..f92caee 100644
> > --- a/Documentation/devicetree/bindings/net/brcm,amac.txt
> > +++ b/Documentation/devicetree/bindings/net/brcm,amac.txt
> > @@ -2,15 +2,18 @@ Broadcom AMAC Ethernet Controller Device Tree Bindings
> >  -------------------------------------------------------------
> >  
> >  Required properties:
> > - - compatible:	"brcm,amac" or "brcm,nsp-amac"
> > + - compatible:	"brcm,amac", "brcm,nsp-amac", or "brcm,ns2-amac"
> >   - reg:		Address and length of the GMAC registers,
> >  		Address and length of the GMAC IDM registers
> > +		Address and length of the NIC Port Manager registers (optional)
> >   - reg-names:	Names of the registers.  Must have both "amac_base" and
> > -		"idm_base"
> > +		"idm_base". "nicpm_base" is optional (required for NS2)
> >   - interrupts:	Interrupt number
> >  
> >  Optional properties:
> >  - mac-address:	See ethernet.txt file in the same directory
> > +- brcm,enet-phy-lane-swap:
> > +		boolean; Swap the PHY lanes (needed on some SKUs of NS2)
> 
> Maybe i'm missing something here, but the patch to the PHY swapped the
> lanes. This seems to be a PHY property, not a MAC property. And it
> swapped them unconditionally....

It swapped them based on (from patch 1/5)
       if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810 &&
           phydev->dev_flags & PHY_BRCM_EXP_LANE_SWAP)

That flag is being set in the driver based on whether the lanes need
to be swapped (which depends on the SKU of NS2).  The only SKU of NS2
we have upstream right now has it swapped, but one that should be
pushed out in the next few weeks will not have this flag present.
There is no way to detect it, and having a separate compat string
seemed overkill.

Thanks,
Jon

> 
> 	Andrew

[PATCH 2/5] Documentation: devicetree: net: add NS2 bindings to amac

[Jon Mason]

On Thu, Oct 27, 2016 at 11:17:57AM +0200, Andrew Lunn wrote:
> On Wed, Oct 26, 2016 at 03:35:58PM -0400, Jon Mason wrote:
> > Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> > ---
> >  Documentation/devicetree/bindings/net/brcm,amac.txt | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/brcm,amac.txt b/Documentation/devicetree/bindings/net/brcm,amac.txt
> > index ba5ecc1..f92caee 100644
> > --- a/Documentation/devicetree/bindings/net/brcm,amac.txt
> > +++ b/Documentation/devicetree/bindings/net/brcm,amac.txt
> > @@ -2,15 +2,18 @@ Broadcom AMAC Ethernet Controller Device Tree Bindings
> >  -------------------------------------------------------------
> >  
> >  Required properties:
> > - - compatible:	"brcm,amac" or "brcm,nsp-amac"
> > + - compatible:	"brcm,amac", "brcm,nsp-amac", or "brcm,ns2-amac"
> >   - reg:		Address and length of the GMAC registers,
> >  		Address and length of the GMAC IDM registers
> > +		Address and length of the NIC Port Manager registers (optional)
> >   - reg-names:	Names of the registers.  Must have both "amac_base" and
> > -		"idm_base"
> > +		"idm_base". "nicpm_base" is optional (required for NS2)
> >   - interrupts:	Interrupt number
> >  
> >  Optional properties:
> >  - mac-address:	See ethernet.txt file in the same directory
> > +- brcm,enet-phy-lane-swap:
> > +		boolean; Swap the PHY lanes (needed on some SKUs of NS2)
> 
> Maybe i'm missing something here, but the patch to the PHY swapped the
> lanes. This seems to be a PHY property, not a MAC property. And it
> swapped them unconditionally....

It swapped them based on (from patch 1/5)
       if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810 &&
           phydev->dev_flags & PHY_BRCM_EXP_LANE_SWAP)

That flag is being set in the driver based on whether the lanes need
to be swapped (which depends on the SKU of NS2).  The only SKU of NS2
we have upstream right now has it swapped, but one that should be
pushed out in the next few weeks will not have this flag present.
There is no way to detect it, and having a separate compat string
seemed overkill.

Thanks,
Jon

> 
> 	Andrew

Re: [PATCH 2/5] Documentation: devicetree: net: add NS2 bindings to amac

[Florian Fainelli]

On 10/27/2016 02:21 PM, Jon Mason wrote:
> On Thu, Oct 27, 2016 at 11:17:57AM +0200, Andrew Lunn wrote:
>> On Wed, Oct 26, 2016 at 03:35:58PM -0400, Jon Mason wrote:
>>> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>>> ---
>>>  Documentation/devicetree/bindings/net/brcm,amac.txt | 7 +++++--
>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/brcm,amac.txt b/Documentation/devicetree/bindings/net/brcm,amac.txt
>>> index ba5ecc1..f92caee 100644
>>> --- a/Documentation/devicetree/bindings/net/brcm,amac.txt
>>> +++ b/Documentation/devicetree/bindings/net/brcm,amac.txt
>>> @@ -2,15 +2,18 @@ Broadcom AMAC Ethernet Controller Device Tree Bindings
>>>  -------------------------------------------------------------
>>>  
>>>  Required properties:
>>> - - compatible:	"brcm,amac" or "brcm,nsp-amac"
>>> + - compatible:	"brcm,amac", "brcm,nsp-amac", or "brcm,ns2-amac"
>>>   - reg:		Address and length of the GMAC registers,
>>>  		Address and length of the GMAC IDM registers
>>> +		Address and length of the NIC Port Manager registers (optional)
>>>   - reg-names:	Names of the registers.  Must have both "amac_base" and
>>> -		"idm_base"
>>> +		"idm_base". "nicpm_base" is optional (required for NS2)
>>>   - interrupts:	Interrupt number
>>>  
>>>  Optional properties:
>>>  - mac-address:	See ethernet.txt file in the same directory
>>> +- brcm,enet-phy-lane-swap:
>>> +		boolean; Swap the PHY lanes (needed on some SKUs of NS2)
>>
>> Maybe i'm missing something here, but the patch to the PHY swapped the
>> lanes. This seems to be a PHY property, not a MAC property. And it
>> swapped them unconditionally....
> 
> It swapped them based on (from patch 1/5)
>        if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810 &&
>            phydev->dev_flags & PHY_BRCM_EXP_LANE_SWAP)
> 
> That flag is being set in the driver based on whether the lanes need
> to be swapped (which depends on the SKU of NS2).  The only SKU of NS2
> we have upstream right now has it swapped, but one that should be
> pushed out in the next few weeks will not have this flag present.
> There is no way to detect it, and having a separate compat string
> seemed overkill.

Andrew has a point thought that this is a property that is associated
with the PHY, and not with the Ethernet MAC per se, as such, you can put
it in the Ethernet PHY node, and look up the proper from
drivers/net/phy/broadcom.c, and come up with a Broadcom Ethernet PHY
binding document (sorry).

There is no need to have a specific compatible string allocated, using
the (mostly) generic Ethernet PHY binding, plus this property documented
would be good enough.
-- 
Florian

[PATCH 2/5] Documentation: devicetree: net: add NS2 bindings to amac

[Florian Fainelli]

On 10/27/2016 02:21 PM, Jon Mason wrote:
> On Thu, Oct 27, 2016 at 11:17:57AM +0200, Andrew Lunn wrote:
>> On Wed, Oct 26, 2016 at 03:35:58PM -0400, Jon Mason wrote:
>>> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>>> ---
>>>  Documentation/devicetree/bindings/net/brcm,amac.txt | 7 +++++--
>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/brcm,amac.txt b/Documentation/devicetree/bindings/net/brcm,amac.txt
>>> index ba5ecc1..f92caee 100644
>>> --- a/Documentation/devicetree/bindings/net/brcm,amac.txt
>>> +++ b/Documentation/devicetree/bindings/net/brcm,amac.txt
>>> @@ -2,15 +2,18 @@ Broadcom AMAC Ethernet Controller Device Tree Bindings
>>>  -------------------------------------------------------------
>>>  
>>>  Required properties:
>>> - - compatible:	"brcm,amac" or "brcm,nsp-amac"
>>> + - compatible:	"brcm,amac", "brcm,nsp-amac", or "brcm,ns2-amac"
>>>   - reg:		Address and length of the GMAC registers,
>>>  		Address and length of the GMAC IDM registers
>>> +		Address and length of the NIC Port Manager registers (optional)
>>>   - reg-names:	Names of the registers.  Must have both "amac_base" and
>>> -		"idm_base"
>>> +		"idm_base". "nicpm_base" is optional (required for NS2)
>>>   - interrupts:	Interrupt number
>>>  
>>>  Optional properties:
>>>  - mac-address:	See ethernet.txt file in the same directory
>>> +- brcm,enet-phy-lane-swap:
>>> +		boolean; Swap the PHY lanes (needed on some SKUs of NS2)
>>
>> Maybe i'm missing something here, but the patch to the PHY swapped the
>> lanes. This seems to be a PHY property, not a MAC property. And it
>> swapped them unconditionally....
> 
> It swapped them based on (from patch 1/5)
>        if (BRCM_PHY_MODEL(phydev) == PHY_ID_BCM54810 &&
>            phydev->dev_flags & PHY_BRCM_EXP_LANE_SWAP)
> 
> That flag is being set in the driver based on whether the lanes need
> to be swapped (which depends on the SKU of NS2).  The only SKU of NS2
> we have upstream right now has it swapped, but one that should be
> pushed out in the next few weeks will not have this flag present.
> There is no way to detect it, and having a separate compat string
> seemed overkill.

Andrew has a point thought that this is a property that is associated
with the PHY, and not with the Ethernet MAC per se, as such, you can put
it in the Ethernet PHY node, and look up the proper from
drivers/net/phy/broadcom.c, and come up with a Broadcom Ethernet PHY
binding document (sorry).

There is no need to have a specific compatible string allocated, using
the (mostly) generic Ethernet PHY binding, plus this property documented
would be good enough.
-- 
Florian

[PATCH 3/5] net: ethernet: bgmac: device tree phy enablement

[Jon Mason]

Change the bgmac driver to allow for phy's defined by the device tree

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac-bcma.c     | 48 ++++++++++++++++++++++++
 drivers/net/ethernet/broadcom/bgmac-platform.c | 48 +++++++++++++++++++++++-
 drivers/net/ethernet/broadcom/bgmac.c          | 51 +-------------------------
 drivers/net/ethernet/broadcom/bgmac.h          |  7 ++++
 4 files changed, 104 insertions(+), 50 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c b/drivers/net/ethernet/broadcom/bgmac-bcma.c
index c16ec3a..3e3efde 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
@@ -80,6 +80,50 @@ static void bcma_bgmac_cmn_maskset32(struct bgmac *bgmac, u16 offset, u32 mask,
 	bcma_maskset32(bgmac->bcma.cmn, offset, mask, set);
 }
 
+static int bcma_phy_connect(struct bgmac *bgmac)
+{
+	struct phy_device *phy_dev;
+	char bus_id[MII_BUS_ID_SIZE + 3];
+
+	/* Connect to the PHY */
+	snprintf(bus_id, sizeof(bus_id), PHY_ID_FMT, bgmac->mii_bus->id,
+		 bgmac->phyaddr);
+	phy_dev = phy_connect(bgmac->net_dev, bus_id, bgmac_adjust_link,
+			      PHY_INTERFACE_MODE_MII);
+	if (IS_ERR(phy_dev)) {
+		dev_err(bgmac->dev, "PHY connecton failed\n");
+		return PTR_ERR(phy_dev);
+	}
+
+	return 0;
+}
+
+static int bcma_phy_direct_connect(struct bgmac *bgmac)
+{
+	struct fixed_phy_status fphy_status = {
+		.link = 1,
+		.speed = SPEED_1000,
+		.duplex = DUPLEX_FULL,
+	};
+	struct phy_device *phy_dev;
+	int err;
+
+	phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, -1, NULL);
+	if (!phy_dev || IS_ERR(phy_dev)) {
+		dev_err(bgmac->dev, "Failed to register fixed PHY device\n");
+		return -ENODEV;
+	}
+
+	err = phy_connect_direct(bgmac->net_dev, phy_dev, bgmac_adjust_link,
+				 PHY_INTERFACE_MODE_MII);
+	if (err) {
+		dev_err(bgmac->dev, "Connecting PHY failed\n");
+		return err;
+	}
+
+	return err;
+}
+
 static const struct bcma_device_id bgmac_bcma_tbl[] = {
 	BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_4706_MAC_GBIT,
 		  BCMA_ANY_REV, BCMA_ANY_CLASS),
@@ -275,6 +319,10 @@ static int bgmac_probe(struct bcma_device *core)
 	bgmac->cco_ctl_maskset = bcma_bgmac_cco_ctl_maskset;
 	bgmac->get_bus_clock = bcma_bgmac_get_bus_clock;
 	bgmac->cmn_maskset32 = bcma_bgmac_cmn_maskset32;
+	if (bgmac->mii_bus)
+		bgmac->phy_connect = bcma_phy_connect;
+	else
+		bgmac->phy_connect = bcma_phy_direct_connect;
 
 	err = bgmac_enet_probe(bgmac);
 	if (err)
diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c
index be52f27..aed5dc5 100644
--- a/drivers/net/ethernet/broadcom/bgmac-platform.c
+++ b/drivers/net/ethernet/broadcom/bgmac-platform.c
@@ -16,6 +16,7 @@
 #include <linux/bcma/bcma.h>
 #include <linux/etherdevice.h>
 #include <linux/of_address.h>
+#include <linux/of_mdio.h>
 #include <linux/of_net.h>
 #include "bgmac.h"
 
@@ -86,6 +87,46 @@ static void platform_bgmac_cmn_maskset32(struct bgmac *bgmac, u16 offset,
 	WARN_ON(1);
 }
 
+static int platform_phy_connect(struct bgmac *bgmac)
+{
+	struct phy_device *phy_dev;
+
+	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
+					 bgmac_adjust_link);
+	if (!phy_dev) {
+		dev_err(bgmac->dev, "Phy connect failed\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int platform_phy_direct_connect(struct bgmac *bgmac)
+{
+	struct fixed_phy_status fphy_status = {
+		.link = 1,
+		.speed = SPEED_1000,
+		.duplex = DUPLEX_FULL,
+	};
+	struct phy_device *phy_dev;
+	int err;
+
+	phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, -1, NULL);
+	if (!phy_dev || IS_ERR(phy_dev)) {
+		dev_err(bgmac->dev, "Failed to register fixed PHY device\n");
+		return -ENODEV;
+	}
+
+	err = phy_connect_direct(bgmac->net_dev, phy_dev, bgmac_adjust_link,
+				 PHY_INTERFACE_MODE_MII);
+	if (err) {
+		dev_err(bgmac->dev, "Connecting PHY failed\n");
+		return err;
+	}
+
+	return err;
+}
+
 static int bgmac_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
@@ -102,7 +143,6 @@ static int bgmac_probe(struct platform_device *pdev)
 	/* Set the features of the 4707 family */
 	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
 	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
-	bgmac->feature_flags |= BGMAC_FEAT_FORCE_SPEED_2500;
 	bgmac->feature_flags |= BGMAC_FEAT_CMDCFG_SR_REV4;
 	bgmac->feature_flags |= BGMAC_FEAT_TX_MASK_SETUP;
 	bgmac->feature_flags |= BGMAC_FEAT_RX_MASK_SETUP;
@@ -151,6 +191,12 @@ static int bgmac_probe(struct platform_device *pdev)
 	bgmac->cco_ctl_maskset = platform_bgmac_cco_ctl_maskset;
 	bgmac->get_bus_clock = platform_bgmac_get_bus_clock;
 	bgmac->cmn_maskset32 = platform_bgmac_cmn_maskset32;
+	if (of_parse_phandle(np, "phy-handle", 0)) {
+		bgmac->phy_connect = platform_phy_connect;
+	} else {
+		bgmac->phy_connect = platform_phy_direct_connect;
+		bgmac->feature_flags |= BGMAC_FEAT_FORCE_SPEED_2500;
+	}
 
 	return bgmac_enet_probe(bgmac);
 }
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 856379c..38876ec 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1388,7 +1388,7 @@ static const struct ethtool_ops bgmac_ethtool_ops = {
  * MII
  **************************************************/
 
-static void bgmac_adjust_link(struct net_device *net_dev)
+void bgmac_adjust_link(struct net_device *net_dev)
 {
 	struct bgmac *bgmac = netdev_priv(net_dev);
 	struct phy_device *phy_dev = net_dev->phydev;
@@ -1412,50 +1412,6 @@ static void bgmac_adjust_link(struct net_device *net_dev)
 	}
 }
 
-static int bgmac_phy_connect_direct(struct bgmac *bgmac)
-{
-	struct fixed_phy_status fphy_status = {
-		.link = 1,
-		.speed = SPEED_1000,
-		.duplex = DUPLEX_FULL,
-	};
-	struct phy_device *phy_dev;
-	int err;
-
-	phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, -1, NULL);
-	if (!phy_dev || IS_ERR(phy_dev)) {
-		dev_err(bgmac->dev, "Failed to register fixed PHY device\n");
-		return -ENODEV;
-	}
-
-	err = phy_connect_direct(bgmac->net_dev, phy_dev, bgmac_adjust_link,
-				 PHY_INTERFACE_MODE_MII);
-	if (err) {
-		dev_err(bgmac->dev, "Connecting PHY failed\n");
-		return err;
-	}
-
-	return err;
-}
-
-static int bgmac_phy_connect(struct bgmac *bgmac)
-{
-	struct phy_device *phy_dev;
-	char bus_id[MII_BUS_ID_SIZE + 3];
-
-	/* Connect to the PHY */
-	snprintf(bus_id, sizeof(bus_id), PHY_ID_FMT, bgmac->mii_bus->id,
-		 bgmac->phyaddr);
-	phy_dev = phy_connect(bgmac->net_dev, bus_id, &bgmac_adjust_link,
-			      PHY_INTERFACE_MODE_MII);
-	if (IS_ERR(phy_dev)) {
-		dev_err(bgmac->dev, "PHY connecton failed\n");
-		return PTR_ERR(phy_dev);
-	}
-
-	return 0;
-}
-
 int bgmac_enet_probe(struct bgmac *info)
 {
 	struct net_device *net_dev;
@@ -1507,10 +1463,7 @@ int bgmac_enet_probe(struct bgmac *info)
 
 	netif_napi_add(net_dev, &bgmac->napi, bgmac_poll, BGMAC_WEIGHT);
 
-	if (!bgmac->mii_bus)
-		err = bgmac_phy_connect_direct(bgmac);
-	else
-		err = bgmac_phy_connect(bgmac);
+	err = bgmac_phy_connect(bgmac);
 	if (err) {
 		dev_err(bgmac->dev, "Cannot connect to phy\n");
 		goto err_dma_free;
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 80836b4..ea52ac3 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -513,10 +513,12 @@ struct bgmac {
 	u32 (*get_bus_clock)(struct bgmac *bgmac);
 	void (*cmn_maskset32)(struct bgmac *bgmac, u16 offset, u32 mask,
 			      u32 set);
+	int (*phy_connect)(struct bgmac *bgmac);
 };
 
 int bgmac_enet_probe(struct bgmac *info);
 void bgmac_enet_remove(struct bgmac *bgmac);
+void bgmac_adjust_link(struct net_device *net_dev);
 
 struct mii_bus *bcma_mdio_mii_register(struct bcma_device *core, u8 phyaddr);
 void bcma_mdio_mii_unregister(struct mii_bus *mii_bus);
@@ -583,4 +585,9 @@ static inline void bgmac_set(struct bgmac *bgmac, u16 offset, u32 set)
 {
 	bgmac_maskset(bgmac, offset, ~0, set);
 }
+
+static inline int bgmac_phy_connect(struct bgmac *bgmac)
+{
+	return bgmac->phy_connect(bgmac);
+}
 #endif /* _BGMAC_H */
-- 
2.7.4

[PATCH 3/5] net: ethernet: bgmac: device tree phy enablement

[Jon Mason]

Change the bgmac driver to allow for phy's defined by the device tree

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac-bcma.c     | 48 ++++++++++++++++++++++++
 drivers/net/ethernet/broadcom/bgmac-platform.c | 48 +++++++++++++++++++++++-
 drivers/net/ethernet/broadcom/bgmac.c          | 51 +-------------------------
 drivers/net/ethernet/broadcom/bgmac.h          |  7 ++++
 4 files changed, 104 insertions(+), 50 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c b/drivers/net/ethernet/broadcom/bgmac-bcma.c
index c16ec3a..3e3efde 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
@@ -80,6 +80,50 @@ static void bcma_bgmac_cmn_maskset32(struct bgmac *bgmac, u16 offset, u32 mask,
 	bcma_maskset32(bgmac->bcma.cmn, offset, mask, set);
 }
 
+static int bcma_phy_connect(struct bgmac *bgmac)
+{
+	struct phy_device *phy_dev;
+	char bus_id[MII_BUS_ID_SIZE + 3];
+
+	/* Connect to the PHY */
+	snprintf(bus_id, sizeof(bus_id), PHY_ID_FMT, bgmac->mii_bus->id,
+		 bgmac->phyaddr);
+	phy_dev = phy_connect(bgmac->net_dev, bus_id, bgmac_adjust_link,
+			      PHY_INTERFACE_MODE_MII);
+	if (IS_ERR(phy_dev)) {
+		dev_err(bgmac->dev, "PHY connecton failed\n");
+		return PTR_ERR(phy_dev);
+	}
+
+	return 0;
+}
+
+static int bcma_phy_direct_connect(struct bgmac *bgmac)
+{
+	struct fixed_phy_status fphy_status = {
+		.link = 1,
+		.speed = SPEED_1000,
+		.duplex = DUPLEX_FULL,
+	};
+	struct phy_device *phy_dev;
+	int err;
+
+	phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, -1, NULL);
+	if (!phy_dev || IS_ERR(phy_dev)) {
+		dev_err(bgmac->dev, "Failed to register fixed PHY device\n");
+		return -ENODEV;
+	}
+
+	err = phy_connect_direct(bgmac->net_dev, phy_dev, bgmac_adjust_link,
+				 PHY_INTERFACE_MODE_MII);
+	if (err) {
+		dev_err(bgmac->dev, "Connecting PHY failed\n");
+		return err;
+	}
+
+	return err;
+}
+
 static const struct bcma_device_id bgmac_bcma_tbl[] = {
 	BCMA_CORE(BCMA_MANUF_BCM, BCMA_CORE_4706_MAC_GBIT,
 		  BCMA_ANY_REV, BCMA_ANY_CLASS),
@@ -275,6 +319,10 @@ static int bgmac_probe(struct bcma_device *core)
 	bgmac->cco_ctl_maskset = bcma_bgmac_cco_ctl_maskset;
 	bgmac->get_bus_clock = bcma_bgmac_get_bus_clock;
 	bgmac->cmn_maskset32 = bcma_bgmac_cmn_maskset32;
+	if (bgmac->mii_bus)
+		bgmac->phy_connect = bcma_phy_connect;
+	else
+		bgmac->phy_connect = bcma_phy_direct_connect;
 
 	err = bgmac_enet_probe(bgmac);
 	if (err)
diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c
index be52f27..aed5dc5 100644
--- a/drivers/net/ethernet/broadcom/bgmac-platform.c
+++ b/drivers/net/ethernet/broadcom/bgmac-platform.c
@@ -16,6 +16,7 @@
 #include <linux/bcma/bcma.h>
 #include <linux/etherdevice.h>
 #include <linux/of_address.h>
+#include <linux/of_mdio.h>
 #include <linux/of_net.h>
 #include "bgmac.h"
 
@@ -86,6 +87,46 @@ static void platform_bgmac_cmn_maskset32(struct bgmac *bgmac, u16 offset,
 	WARN_ON(1);
 }
 
+static int platform_phy_connect(struct bgmac *bgmac)
+{
+	struct phy_device *phy_dev;
+
+	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
+					 bgmac_adjust_link);
+	if (!phy_dev) {
+		dev_err(bgmac->dev, "Phy connect failed\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int platform_phy_direct_connect(struct bgmac *bgmac)
+{
+	struct fixed_phy_status fphy_status = {
+		.link = 1,
+		.speed = SPEED_1000,
+		.duplex = DUPLEX_FULL,
+	};
+	struct phy_device *phy_dev;
+	int err;
+
+	phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, -1, NULL);
+	if (!phy_dev || IS_ERR(phy_dev)) {
+		dev_err(bgmac->dev, "Failed to register fixed PHY device\n");
+		return -ENODEV;
+	}
+
+	err = phy_connect_direct(bgmac->net_dev, phy_dev, bgmac_adjust_link,
+				 PHY_INTERFACE_MODE_MII);
+	if (err) {
+		dev_err(bgmac->dev, "Connecting PHY failed\n");
+		return err;
+	}
+
+	return err;
+}
+
 static int bgmac_probe(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
@@ -102,7 +143,6 @@ static int bgmac_probe(struct platform_device *pdev)
 	/* Set the features of the 4707 family */
 	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
 	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
-	bgmac->feature_flags |= BGMAC_FEAT_FORCE_SPEED_2500;
 	bgmac->feature_flags |= BGMAC_FEAT_CMDCFG_SR_REV4;
 	bgmac->feature_flags |= BGMAC_FEAT_TX_MASK_SETUP;
 	bgmac->feature_flags |= BGMAC_FEAT_RX_MASK_SETUP;
@@ -151,6 +191,12 @@ static int bgmac_probe(struct platform_device *pdev)
 	bgmac->cco_ctl_maskset = platform_bgmac_cco_ctl_maskset;
 	bgmac->get_bus_clock = platform_bgmac_get_bus_clock;
 	bgmac->cmn_maskset32 = platform_bgmac_cmn_maskset32;
+	if (of_parse_phandle(np, "phy-handle", 0)) {
+		bgmac->phy_connect = platform_phy_connect;
+	} else {
+		bgmac->phy_connect = platform_phy_direct_connect;
+		bgmac->feature_flags |= BGMAC_FEAT_FORCE_SPEED_2500;
+	}
 
 	return bgmac_enet_probe(bgmac);
 }
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 856379c..38876ec 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1388,7 +1388,7 @@ static const struct ethtool_ops bgmac_ethtool_ops = {
  * MII
  **************************************************/
 
-static void bgmac_adjust_link(struct net_device *net_dev)
+void bgmac_adjust_link(struct net_device *net_dev)
 {
 	struct bgmac *bgmac = netdev_priv(net_dev);
 	struct phy_device *phy_dev = net_dev->phydev;
@@ -1412,50 +1412,6 @@ static void bgmac_adjust_link(struct net_device *net_dev)
 	}
 }
 
-static int bgmac_phy_connect_direct(struct bgmac *bgmac)
-{
-	struct fixed_phy_status fphy_status = {
-		.link = 1,
-		.speed = SPEED_1000,
-		.duplex = DUPLEX_FULL,
-	};
-	struct phy_device *phy_dev;
-	int err;
-
-	phy_dev = fixed_phy_register(PHY_POLL, &fphy_status, -1, NULL);
-	if (!phy_dev || IS_ERR(phy_dev)) {
-		dev_err(bgmac->dev, "Failed to register fixed PHY device\n");
-		return -ENODEV;
-	}
-
-	err = phy_connect_direct(bgmac->net_dev, phy_dev, bgmac_adjust_link,
-				 PHY_INTERFACE_MODE_MII);
-	if (err) {
-		dev_err(bgmac->dev, "Connecting PHY failed\n");
-		return err;
-	}
-
-	return err;
-}
-
-static int bgmac_phy_connect(struct bgmac *bgmac)
-{
-	struct phy_device *phy_dev;
-	char bus_id[MII_BUS_ID_SIZE + 3];
-
-	/* Connect to the PHY */
-	snprintf(bus_id, sizeof(bus_id), PHY_ID_FMT, bgmac->mii_bus->id,
-		 bgmac->phyaddr);
-	phy_dev = phy_connect(bgmac->net_dev, bus_id, &bgmac_adjust_link,
-			      PHY_INTERFACE_MODE_MII);
-	if (IS_ERR(phy_dev)) {
-		dev_err(bgmac->dev, "PHY connecton failed\n");
-		return PTR_ERR(phy_dev);
-	}
-
-	return 0;
-}
-
 int bgmac_enet_probe(struct bgmac *info)
 {
 	struct net_device *net_dev;
@@ -1507,10 +1463,7 @@ int bgmac_enet_probe(struct bgmac *info)
 
 	netif_napi_add(net_dev, &bgmac->napi, bgmac_poll, BGMAC_WEIGHT);
 
-	if (!bgmac->mii_bus)
-		err = bgmac_phy_connect_direct(bgmac);
-	else
-		err = bgmac_phy_connect(bgmac);
+	err = bgmac_phy_connect(bgmac);
 	if (err) {
 		dev_err(bgmac->dev, "Cannot connect to phy\n");
 		goto err_dma_free;
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 80836b4..ea52ac3 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -513,10 +513,12 @@ struct bgmac {
 	u32 (*get_bus_clock)(struct bgmac *bgmac);
 	void (*cmn_maskset32)(struct bgmac *bgmac, u16 offset, u32 mask,
 			      u32 set);
+	int (*phy_connect)(struct bgmac *bgmac);
 };
 
 int bgmac_enet_probe(struct bgmac *info);
 void bgmac_enet_remove(struct bgmac *bgmac);
+void bgmac_adjust_link(struct net_device *net_dev);
 
 struct mii_bus *bcma_mdio_mii_register(struct bcma_device *core, u8 phyaddr);
 void bcma_mdio_mii_unregister(struct mii_bus *mii_bus);
@@ -583,4 +585,9 @@ static inline void bgmac_set(struct bgmac *bgmac, u16 offset, u32 set)
 {
 	bgmac_maskset(bgmac, offset, ~0, set);
 }
+
+static inline int bgmac_phy_connect(struct bgmac *bgmac)
+{
+	return bgmac->phy_connect(bgmac);
+}
 #endif /* _BGMAC_H */
-- 
2.7.4

[PATCH 4/5] net: ethernet: bgmac: add NS2 support

[Jon Mason]

Add support for the variant of amac hardware present in the Broadcom
Northstar2 based SoCs.  Northstar2 requires an additional register to be
configured with the port speed/duplexity (NICPM).  This can be added to
the link callback to hide it from the instances that do not use this.
Also, the bgmac_chip_reset() was intentionally removed to prevent the
resetting of the chip to the default values on open.  Finally, clearing
of the pending interrupts on init is required due to observed issues on
some platforms.

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac-platform.c | 64 +++++++++++++++++++++++++-
 drivers/net/ethernet/broadcom/bgmac.c          |  5 +-
 drivers/net/ethernet/broadcom/bgmac.h          |  2 +
 3 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c
index aed5dc5..49fcff4 100644
--- a/drivers/net/ethernet/broadcom/bgmac-platform.c
+++ b/drivers/net/ethernet/broadcom/bgmac-platform.c
@@ -14,12 +14,21 @@
 #define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
 
 #include <linux/bcma/bcma.h>
+#include <linux/brcmphy.h>
 #include <linux/etherdevice.h>
 #include <linux/of_address.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
 #include "bgmac.h"
 
+#define NICPM_IOMUX_CTRL		0x00000008
+
+#define NICPM_IOMUX_CTRL_INIT_VAL	0x3196e000
+#define NICPM_IOMUX_CTRL_SPD_SHIFT	10
+#define NICPM_IOMUX_CTRL_SPD_10M	0
+#define NICPM_IOMUX_CTRL_SPD_100M	1
+#define NICPM_IOMUX_CTRL_SPD_1000M	2
+
 static u32 platform_bgmac_read(struct bgmac *bgmac, u16 offset)
 {
 	return readl(bgmac->plat.base + offset);
@@ -87,17 +96,56 @@ static void platform_bgmac_cmn_maskset32(struct bgmac *bgmac, u16 offset,
 	WARN_ON(1);
 }
 
+static void bgmac_nicpm_speed_set(struct net_device *net_dev)
+{
+	struct bgmac *bgmac = netdev_priv(net_dev);
+	u32 val;
+
+	if (!bgmac->plat.nicpm_base)
+		return;
+
+	val = NICPM_IOMUX_CTRL_INIT_VAL;
+	switch (bgmac->net_dev->phydev->speed) {
+	default:
+		pr_err("Unsupported speed.  Defaulting to 1000Mb\n");
+	case SPEED_1000:
+		val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT;
+		break;
+	case SPEED_100:
+		val |= NICPM_IOMUX_CTRL_SPD_100M << NICPM_IOMUX_CTRL_SPD_SHIFT;
+		break;
+	case SPEED_10:
+		val |= NICPM_IOMUX_CTRL_SPD_10M << NICPM_IOMUX_CTRL_SPD_SHIFT;
+		break;
+	}
+
+	writel(val, bgmac->plat.nicpm_base + NICPM_IOMUX_CTRL);
+
+	usleep_range(10, 100);
+
+	bgmac_adjust_link(bgmac->net_dev);
+}
+
 static int platform_phy_connect(struct bgmac *bgmac)
 {
 	struct phy_device *phy_dev;
 
-	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
-					 bgmac_adjust_link);
+	if (bgmac->plat.nicpm_base)
+		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
+						 bgmac->dev->of_node,
+						 bgmac_nicpm_speed_set);
+	else
+		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
+						 bgmac->dev->of_node,
+						 bgmac_adjust_link);
 	if (!phy_dev) {
 		dev_err(bgmac->dev, "Phy connect failed\n");
 		return -ENODEV;
 	}
 
+	if (bgmac->feature_flags & BGMAC_FEAT_LANE_SWAP)
+		phy_dev->dev_flags |= PHY_BRCM_EXP_LANE_SWAP;
+
 	return 0;
 }
 
@@ -140,6 +188,9 @@ static int bgmac_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, bgmac);
 
+	if (of_property_read_bool(np, "brcm,enet-phy-lane-swap"))
+		bgmac->feature_flags |= BGMAC_FEAT_LANE_SWAP;
+
 	/* Set the features of the 4707 family */
 	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
 	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
@@ -182,6 +233,14 @@ static int bgmac_probe(struct platform_device *pdev)
 	if (IS_ERR(bgmac->plat.idm_base))
 		return PTR_ERR(bgmac->plat.idm_base);
 
+	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base");
+	if (regs) {
+		bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev,
+							       regs);
+		if (IS_ERR(bgmac->plat.nicpm_base))
+			return PTR_ERR(bgmac->plat.nicpm_base);
+	}
+
 	bgmac->read = platform_bgmac_read;
 	bgmac->write = platform_bgmac_write;
 	bgmac->idm_read = platform_bgmac_idm_read;
@@ -213,6 +272,7 @@ static int bgmac_remove(struct platform_device *pdev)
 static const struct of_device_id bgmac_of_enet_match[] = {
 	{.compatible = "brcm,amac",},
 	{.compatible = "brcm,nsp-amac",},
+	{.compatible = "brcm,ns2-amac",},
 	{},
 };
 
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 38876ec..1796208 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1082,6 +1082,9 @@ static void bgmac_enable(struct bgmac *bgmac)
 /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
 static void bgmac_chip_init(struct bgmac *bgmac)
 {
+	/* Clear any erroneously pending interrupts */
+	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
+
 	/* 1 interrupt per received frame */
 	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
 
@@ -1158,8 +1161,6 @@ static int bgmac_open(struct net_device *net_dev)
 	struct bgmac *bgmac = netdev_priv(net_dev);
 	int err = 0;
 
-	bgmac_chip_reset(bgmac);
-
 	err = bgmac_dma_init(bgmac);
 	if (err)
 		return err;
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index ea52ac3..677e685 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -409,6 +409,7 @@
 #define BGMAC_FEAT_CC4_IF_SW_TYPE	BIT(17)
 #define BGMAC_FEAT_CC4_IF_SW_TYPE_RGMII	BIT(18)
 #define BGMAC_FEAT_CC7_IF_TYPE_RGMII	BIT(19)
+#define BGMAC_FEAT_LANE_SWAP		BIT(20)
 
 struct bgmac_slot_info {
 	union {
@@ -463,6 +464,7 @@ struct bgmac {
 		struct {
 			void *base;
 			void *idm_base;
+			void *nicpm_base;
 		} plat;
 		struct {
 			struct bcma_device *core;
-- 
2.7.4

[PATCH 4/5] net: ethernet: bgmac: add NS2 support

[Jon Mason]

Add support for the variant of amac hardware present in the Broadcom
Northstar2 based SoCs.  Northstar2 requires an additional register to be
configured with the port speed/duplexity (NICPM).  This can be added to
the link callback to hide it from the instances that do not use this.
Also, the bgmac_chip_reset() was intentionally removed to prevent the
resetting of the chip to the default values on open.  Finally, clearing
of the pending interrupts on init is required due to observed issues on
some platforms.

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac-platform.c | 64 +++++++++++++++++++++++++-
 drivers/net/ethernet/broadcom/bgmac.c          |  5 +-
 drivers/net/ethernet/broadcom/bgmac.h          |  2 +
 3 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c
index aed5dc5..49fcff4 100644
--- a/drivers/net/ethernet/broadcom/bgmac-platform.c
+++ b/drivers/net/ethernet/broadcom/bgmac-platform.c
@@ -14,12 +14,21 @@
 #define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
 
 #include <linux/bcma/bcma.h>
+#include <linux/brcmphy.h>
 #include <linux/etherdevice.h>
 #include <linux/of_address.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
 #include "bgmac.h"
 
+#define NICPM_IOMUX_CTRL		0x00000008
+
+#define NICPM_IOMUX_CTRL_INIT_VAL	0x3196e000
+#define NICPM_IOMUX_CTRL_SPD_SHIFT	10
+#define NICPM_IOMUX_CTRL_SPD_10M	0
+#define NICPM_IOMUX_CTRL_SPD_100M	1
+#define NICPM_IOMUX_CTRL_SPD_1000M	2
+
 static u32 platform_bgmac_read(struct bgmac *bgmac, u16 offset)
 {
 	return readl(bgmac->plat.base + offset);
@@ -87,17 +96,56 @@ static void platform_bgmac_cmn_maskset32(struct bgmac *bgmac, u16 offset,
 	WARN_ON(1);
 }
 
+static void bgmac_nicpm_speed_set(struct net_device *net_dev)
+{
+	struct bgmac *bgmac = netdev_priv(net_dev);
+	u32 val;
+
+	if (!bgmac->plat.nicpm_base)
+		return;
+
+	val = NICPM_IOMUX_CTRL_INIT_VAL;
+	switch (bgmac->net_dev->phydev->speed) {
+	default:
+		pr_err("Unsupported speed.  Defaulting to 1000Mb\n");
+	case SPEED_1000:
+		val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT;
+		break;
+	case SPEED_100:
+		val |= NICPM_IOMUX_CTRL_SPD_100M << NICPM_IOMUX_CTRL_SPD_SHIFT;
+		break;
+	case SPEED_10:
+		val |= NICPM_IOMUX_CTRL_SPD_10M << NICPM_IOMUX_CTRL_SPD_SHIFT;
+		break;
+	}
+
+	writel(val, bgmac->plat.nicpm_base + NICPM_IOMUX_CTRL);
+
+	usleep_range(10, 100);
+
+	bgmac_adjust_link(bgmac->net_dev);
+}
+
 static int platform_phy_connect(struct bgmac *bgmac)
 {
 	struct phy_device *phy_dev;
 
-	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
-					 bgmac_adjust_link);
+	if (bgmac->plat.nicpm_base)
+		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
+						 bgmac->dev->of_node,
+						 bgmac_nicpm_speed_set);
+	else
+		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
+						 bgmac->dev->of_node,
+						 bgmac_adjust_link);
 	if (!phy_dev) {
 		dev_err(bgmac->dev, "Phy connect failed\n");
 		return -ENODEV;
 	}
 
+	if (bgmac->feature_flags & BGMAC_FEAT_LANE_SWAP)
+		phy_dev->dev_flags |= PHY_BRCM_EXP_LANE_SWAP;
+
 	return 0;
 }
 
@@ -140,6 +188,9 @@ static int bgmac_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, bgmac);
 
+	if (of_property_read_bool(np, "brcm,enet-phy-lane-swap"))
+		bgmac->feature_flags |= BGMAC_FEAT_LANE_SWAP;
+
 	/* Set the features of the 4707 family */
 	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
 	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
@@ -182,6 +233,14 @@ static int bgmac_probe(struct platform_device *pdev)
 	if (IS_ERR(bgmac->plat.idm_base))
 		return PTR_ERR(bgmac->plat.idm_base);
 
+	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base");
+	if (regs) {
+		bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev,
+							       regs);
+		if (IS_ERR(bgmac->plat.nicpm_base))
+			return PTR_ERR(bgmac->plat.nicpm_base);
+	}
+
 	bgmac->read = platform_bgmac_read;
 	bgmac->write = platform_bgmac_write;
 	bgmac->idm_read = platform_bgmac_idm_read;
@@ -213,6 +272,7 @@ static int bgmac_remove(struct platform_device *pdev)
 static const struct of_device_id bgmac_of_enet_match[] = {
 	{.compatible = "brcm,amac",},
 	{.compatible = "brcm,nsp-amac",},
+	{.compatible = "brcm,ns2-amac",},
 	{},
 };
 
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 38876ec..1796208 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1082,6 +1082,9 @@ static void bgmac_enable(struct bgmac *bgmac)
 /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
 static void bgmac_chip_init(struct bgmac *bgmac)
 {
+	/* Clear any erroneously pending interrupts */
+	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
+
 	/* 1 interrupt per received frame */
 	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
 
@@ -1158,8 +1161,6 @@ static int bgmac_open(struct net_device *net_dev)
 	struct bgmac *bgmac = netdev_priv(net_dev);
 	int err = 0;
 
-	bgmac_chip_reset(bgmac);
-
 	err = bgmac_dma_init(bgmac);
 	if (err)
 		return err;
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index ea52ac3..677e685 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -409,6 +409,7 @@
 #define BGMAC_FEAT_CC4_IF_SW_TYPE	BIT(17)
 #define BGMAC_FEAT_CC4_IF_SW_TYPE_RGMII	BIT(18)
 #define BGMAC_FEAT_CC7_IF_TYPE_RGMII	BIT(19)
+#define BGMAC_FEAT_LANE_SWAP		BIT(20)
 
 struct bgmac_slot_info {
 	union {
@@ -463,6 +464,7 @@ struct bgmac {
 		struct {
 			void *base;
 			void *idm_base;
+			void *nicpm_base;
 		} plat;
 		struct {
 			struct bcma_device *core;
-- 
2.7.4

[PATCH 4/5] net: ethernet: bgmac: add NS2 support

[Jon Mason]

Add support for the variant of amac hardware present in the Broadcom
Northstar2 based SoCs.  Northstar2 requires an additional register to be
configured with the port speed/duplexity (NICPM).  This can be added to
the link callback to hide it from the instances that do not use this.
Also, the bgmac_chip_reset() was intentionally removed to prevent the
resetting of the chip to the default values on open.  Finally, clearing
of the pending interrupts on init is required due to observed issues on
some platforms.

Signed-off-by: Jon Mason <jon.mason-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 drivers/net/ethernet/broadcom/bgmac-platform.c | 64 +++++++++++++++++++++++++-
 drivers/net/ethernet/broadcom/bgmac.c          |  5 +-
 drivers/net/ethernet/broadcom/bgmac.h          |  2 +
 3 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c
index aed5dc5..49fcff4 100644
--- a/drivers/net/ethernet/broadcom/bgmac-platform.c
+++ b/drivers/net/ethernet/broadcom/bgmac-platform.c
@@ -14,12 +14,21 @@
 #define pr_fmt(fmt)		KBUILD_MODNAME ": " fmt
 
 #include <linux/bcma/bcma.h>
+#include <linux/brcmphy.h>
 #include <linux/etherdevice.h>
 #include <linux/of_address.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
 #include "bgmac.h"
 
+#define NICPM_IOMUX_CTRL		0x00000008
+
+#define NICPM_IOMUX_CTRL_INIT_VAL	0x3196e000
+#define NICPM_IOMUX_CTRL_SPD_SHIFT	10
+#define NICPM_IOMUX_CTRL_SPD_10M	0
+#define NICPM_IOMUX_CTRL_SPD_100M	1
+#define NICPM_IOMUX_CTRL_SPD_1000M	2
+
 static u32 platform_bgmac_read(struct bgmac *bgmac, u16 offset)
 {
 	return readl(bgmac->plat.base + offset);
@@ -87,17 +96,56 @@ static void platform_bgmac_cmn_maskset32(struct bgmac *bgmac, u16 offset,
 	WARN_ON(1);
 }
 
+static void bgmac_nicpm_speed_set(struct net_device *net_dev)
+{
+	struct bgmac *bgmac = netdev_priv(net_dev);
+	u32 val;
+
+	if (!bgmac->plat.nicpm_base)
+		return;
+
+	val = NICPM_IOMUX_CTRL_INIT_VAL;
+	switch (bgmac->net_dev->phydev->speed) {
+	default:
+		pr_err("Unsupported speed.  Defaulting to 1000Mb\n");
+	case SPEED_1000:
+		val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT;
+		break;
+	case SPEED_100:
+		val |= NICPM_IOMUX_CTRL_SPD_100M << NICPM_IOMUX_CTRL_SPD_SHIFT;
+		break;
+	case SPEED_10:
+		val |= NICPM_IOMUX_CTRL_SPD_10M << NICPM_IOMUX_CTRL_SPD_SHIFT;
+		break;
+	}
+
+	writel(val, bgmac->plat.nicpm_base + NICPM_IOMUX_CTRL);
+
+	usleep_range(10, 100);
+
+	bgmac_adjust_link(bgmac->net_dev);
+}
+
 static int platform_phy_connect(struct bgmac *bgmac)
 {
 	struct phy_device *phy_dev;
 
-	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
-					 bgmac_adjust_link);
+	if (bgmac->plat.nicpm_base)
+		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
+						 bgmac->dev->of_node,
+						 bgmac_nicpm_speed_set);
+	else
+		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
+						 bgmac->dev->of_node,
+						 bgmac_adjust_link);
 	if (!phy_dev) {
 		dev_err(bgmac->dev, "Phy connect failed\n");
 		return -ENODEV;
 	}
 
+	if (bgmac->feature_flags & BGMAC_FEAT_LANE_SWAP)
+		phy_dev->dev_flags |= PHY_BRCM_EXP_LANE_SWAP;
+
 	return 0;
 }
 
@@ -140,6 +188,9 @@ static int bgmac_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, bgmac);
 
+	if (of_property_read_bool(np, "brcm,enet-phy-lane-swap"))
+		bgmac->feature_flags |= BGMAC_FEAT_LANE_SWAP;
+
 	/* Set the features of the 4707 family */
 	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
 	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
@@ -182,6 +233,14 @@ static int bgmac_probe(struct platform_device *pdev)
 	if (IS_ERR(bgmac->plat.idm_base))
 		return PTR_ERR(bgmac->plat.idm_base);
 
+	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base");
+	if (regs) {
+		bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev,
+							       regs);
+		if (IS_ERR(bgmac->plat.nicpm_base))
+			return PTR_ERR(bgmac->plat.nicpm_base);
+	}
+
 	bgmac->read = platform_bgmac_read;
 	bgmac->write = platform_bgmac_write;
 	bgmac->idm_read = platform_bgmac_idm_read;
@@ -213,6 +272,7 @@ static int bgmac_remove(struct platform_device *pdev)
 static const struct of_device_id bgmac_of_enet_match[] = {
 	{.compatible = "brcm,amac",},
 	{.compatible = "brcm,nsp-amac",},
+	{.compatible = "brcm,ns2-amac",},
 	{},
 };
 
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 38876ec..1796208 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1082,6 +1082,9 @@ static void bgmac_enable(struct bgmac *bgmac)
 /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
 static void bgmac_chip_init(struct bgmac *bgmac)
 {
+	/* Clear any erroneously pending interrupts */
+	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
+
 	/* 1 interrupt per received frame */
 	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
 
@@ -1158,8 +1161,6 @@ static int bgmac_open(struct net_device *net_dev)
 	struct bgmac *bgmac = netdev_priv(net_dev);
 	int err = 0;
 
-	bgmac_chip_reset(bgmac);
-
 	err = bgmac_dma_init(bgmac);
 	if (err)
 		return err;
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index ea52ac3..677e685 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -409,6 +409,7 @@
 #define BGMAC_FEAT_CC4_IF_SW_TYPE	BIT(17)
 #define BGMAC_FEAT_CC4_IF_SW_TYPE_RGMII	BIT(18)
 #define BGMAC_FEAT_CC7_IF_TYPE_RGMII	BIT(19)
+#define BGMAC_FEAT_LANE_SWAP		BIT(20)
 
 struct bgmac_slot_info {
 	union {
@@ -463,6 +464,7 @@ struct bgmac {
 		struct {
 			void *base;
 			void *idm_base;
+			void *nicpm_base;
 		} plat;
 		struct {
 			struct bcma_device *core;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] net: ethernet: bgmac: add NS2 support

[Florian Fainelli]

On 10/26/2016 12:36 PM, Jon Mason wrote:
> Add support for the variant of amac hardware present in the Broadcom
> Northstar2 based SoCs.  Northstar2 requires an additional register to be
> configured with the port speed/duplexity (NICPM).  This can be added to
> the link callback to hide it from the instances that do not use this.
> Also, the bgmac_chip_reset() was intentionally removed to prevent the
> resetting of the chip to the default values on open.  Finally, clearing
> of the pending interrupts on init is required due to observed issues on
> some platforms.
> 
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---

> +static void bgmac_nicpm_speed_set(struct net_device *net_dev)
> +{
> +	struct bgmac *bgmac = netdev_priv(net_dev);
> +	u32 val;
> +
> +	if (!bgmac->plat.nicpm_base)
> +		return;
> +
> +	val = NICPM_IOMUX_CTRL_INIT_VAL;
> +	switch (bgmac->net_dev->phydev->speed) {
> +	default:
> +		pr_err("Unsupported speed.  Defaulting to 1000Mb\n");
> +	case SPEED_1000:
> +		val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> +		break;
> +	case SPEED_100:
> +		val |= NICPM_IOMUX_CTRL_SPD_100M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> +		break;
> +	case SPEED_10:
> +		val |= NICPM_IOMUX_CTRL_SPD_10M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> +		break;
> +	}
> +
> +	writel(val, bgmac->plat.nicpm_base + NICPM_IOMUX_CTRL);
> +
> +	usleep_range(10, 100);

Does not seem like a good idea, do you need that sleep?

> +
> +	bgmac_adjust_link(bgmac->net_dev);
> +}
> +
>  static int platform_phy_connect(struct bgmac *bgmac)
>  {
>  	struct phy_device *phy_dev;
>  
> -	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
> -					 bgmac_adjust_link);
> +	if (bgmac->plat.nicpm_base)
> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
> +						 bgmac->dev->of_node,
> +						 bgmac_nicpm_speed_set);
> +	else
> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
> +						 bgmac->dev->of_node,
> +						 bgmac_adjust_link);
>  	if (!phy_dev) {
>  		dev_err(bgmac->dev, "Phy connect failed\n");
>  		return -ENODEV;
>  	}
>  
> +	if (bgmac->feature_flags & BGMAC_FEAT_LANE_SWAP)
> +		phy_dev->dev_flags |= PHY_BRCM_EXP_LANE_SWAP;
> +
>  	return 0;
>  }
>  
> @@ -140,6 +188,9 @@ static int bgmac_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, bgmac);
>  
> +	if (of_property_read_bool(np, "brcm,enet-phy-lane-swap"))
> +		bgmac->feature_flags |= BGMAC_FEAT_LANE_SWAP;
> +
>  	/* Set the features of the 4707 family */
>  	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
>  	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
> @@ -182,6 +233,14 @@ static int bgmac_probe(struct platform_device *pdev)
>  	if (IS_ERR(bgmac->plat.idm_base))
>  		return PTR_ERR(bgmac->plat.idm_base);
>  
> +	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base");
> +	if (regs) {
> +		bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev,
> +							       regs);
> +		if (IS_ERR(bgmac->plat.nicpm_base))
> +			return PTR_ERR(bgmac->plat.nicpm_base);
> +	}
> +
>  	bgmac->read = platform_bgmac_read;
>  	bgmac->write = platform_bgmac_write;
>  	bgmac->idm_read = platform_bgmac_idm_read;
> @@ -213,6 +272,7 @@ static int bgmac_remove(struct platform_device *pdev)
>  static const struct of_device_id bgmac_of_enet_match[] = {
>  	{.compatible = "brcm,amac",},
>  	{.compatible = "brcm,nsp-amac",},
> +	{.compatible = "brcm,ns2-amac",},
>  	{},
>  };
>  
> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> index 38876ec..1796208 100644
> --- a/drivers/net/ethernet/broadcom/bgmac.c
> +++ b/drivers/net/ethernet/broadcom/bgmac.c
> @@ -1082,6 +1082,9 @@ static void bgmac_enable(struct bgmac *bgmac)
>  /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
>  static void bgmac_chip_init(struct bgmac *bgmac)
>  {
> +	/* Clear any erroneously pending interrupts */
> +	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
> +
>  	/* 1 interrupt per received frame */
>  	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
>  
> @@ -1158,8 +1161,6 @@ static int bgmac_open(struct net_device *net_dev)
>  	struct bgmac *bgmac = netdev_priv(net_dev);
>  	int err = 0;
>  
> -	bgmac_chip_reset(bgmac);
> -

Is this removal intentional? Maybe it should be special cased with
checking for a NS2 BGMAC instance and not do it in that case?
-- 
Florian

[PATCH 4/5] net: ethernet: bgmac: add NS2 support

[Florian Fainelli]

On 10/26/2016 12:36 PM, Jon Mason wrote:
> Add support for the variant of amac hardware present in the Broadcom
> Northstar2 based SoCs.  Northstar2 requires an additional register to be
> configured with the port speed/duplexity (NICPM).  This can be added to
> the link callback to hide it from the instances that do not use this.
> Also, the bgmac_chip_reset() was intentionally removed to prevent the
> resetting of the chip to the default values on open.  Finally, clearing
> of the pending interrupts on init is required due to observed issues on
> some platforms.
> 
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---

> +static void bgmac_nicpm_speed_set(struct net_device *net_dev)
> +{
> +	struct bgmac *bgmac = netdev_priv(net_dev);
> +	u32 val;
> +
> +	if (!bgmac->plat.nicpm_base)
> +		return;
> +
> +	val = NICPM_IOMUX_CTRL_INIT_VAL;
> +	switch (bgmac->net_dev->phydev->speed) {
> +	default:
> +		pr_err("Unsupported speed.  Defaulting to 1000Mb\n");
> +	case SPEED_1000:
> +		val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> +		break;
> +	case SPEED_100:
> +		val |= NICPM_IOMUX_CTRL_SPD_100M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> +		break;
> +	case SPEED_10:
> +		val |= NICPM_IOMUX_CTRL_SPD_10M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> +		break;
> +	}
> +
> +	writel(val, bgmac->plat.nicpm_base + NICPM_IOMUX_CTRL);
> +
> +	usleep_range(10, 100);

Does not seem like a good idea, do you need that sleep?

> +
> +	bgmac_adjust_link(bgmac->net_dev);
> +}
> +
>  static int platform_phy_connect(struct bgmac *bgmac)
>  {
>  	struct phy_device *phy_dev;
>  
> -	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
> -					 bgmac_adjust_link);
> +	if (bgmac->plat.nicpm_base)
> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
> +						 bgmac->dev->of_node,
> +						 bgmac_nicpm_speed_set);
> +	else
> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
> +						 bgmac->dev->of_node,
> +						 bgmac_adjust_link);
>  	if (!phy_dev) {
>  		dev_err(bgmac->dev, "Phy connect failed\n");
>  		return -ENODEV;
>  	}
>  
> +	if (bgmac->feature_flags & BGMAC_FEAT_LANE_SWAP)
> +		phy_dev->dev_flags |= PHY_BRCM_EXP_LANE_SWAP;
> +
>  	return 0;
>  }
>  
> @@ -140,6 +188,9 @@ static int bgmac_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, bgmac);
>  
> +	if (of_property_read_bool(np, "brcm,enet-phy-lane-swap"))
> +		bgmac->feature_flags |= BGMAC_FEAT_LANE_SWAP;
> +
>  	/* Set the features of the 4707 family */
>  	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
>  	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
> @@ -182,6 +233,14 @@ static int bgmac_probe(struct platform_device *pdev)
>  	if (IS_ERR(bgmac->plat.idm_base))
>  		return PTR_ERR(bgmac->plat.idm_base);
>  
> +	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base");
> +	if (regs) {
> +		bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev,
> +							       regs);
> +		if (IS_ERR(bgmac->plat.nicpm_base))
> +			return PTR_ERR(bgmac->plat.nicpm_base);
> +	}
> +
>  	bgmac->read = platform_bgmac_read;
>  	bgmac->write = platform_bgmac_write;
>  	bgmac->idm_read = platform_bgmac_idm_read;
> @@ -213,6 +272,7 @@ static int bgmac_remove(struct platform_device *pdev)
>  static const struct of_device_id bgmac_of_enet_match[] = {
>  	{.compatible = "brcm,amac",},
>  	{.compatible = "brcm,nsp-amac",},
> +	{.compatible = "brcm,ns2-amac",},
>  	{},
>  };
>  
> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> index 38876ec..1796208 100644
> --- a/drivers/net/ethernet/broadcom/bgmac.c
> +++ b/drivers/net/ethernet/broadcom/bgmac.c
> @@ -1082,6 +1082,9 @@ static void bgmac_enable(struct bgmac *bgmac)
>  /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
>  static void bgmac_chip_init(struct bgmac *bgmac)
>  {
> +	/* Clear any erroneously pending interrupts */
> +	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
> +
>  	/* 1 interrupt per received frame */
>  	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
>  
> @@ -1158,8 +1161,6 @@ static int bgmac_open(struct net_device *net_dev)
>  	struct bgmac *bgmac = netdev_priv(net_dev);
>  	int err = 0;
>  
> -	bgmac_chip_reset(bgmac);
> -

Is this removal intentional? Maybe it should be special cased with
checking for a NS2 BGMAC instance and not do it in that case?
-- 
Florian

Re: [PATCH 4/5] net: ethernet: bgmac: add NS2 support

[Florian Fainelli]

On 10/26/2016 12:36 PM, Jon Mason wrote:
> Add support for the variant of amac hardware present in the Broadcom
> Northstar2 based SoCs.  Northstar2 requires an additional register to be
> configured with the port speed/duplexity (NICPM).  This can be added to
> the link callback to hide it from the instances that do not use this.
> Also, the bgmac_chip_reset() was intentionally removed to prevent the
> resetting of the chip to the default values on open.  Finally, clearing
> of the pending interrupts on init is required due to observed issues on
> some platforms.
> 
> Signed-off-by: Jon Mason <jon.mason-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> ---

> +static void bgmac_nicpm_speed_set(struct net_device *net_dev)
> +{
> +	struct bgmac *bgmac = netdev_priv(net_dev);
> +	u32 val;
> +
> +	if (!bgmac->plat.nicpm_base)
> +		return;
> +
> +	val = NICPM_IOMUX_CTRL_INIT_VAL;
> +	switch (bgmac->net_dev->phydev->speed) {
> +	default:
> +		pr_err("Unsupported speed.  Defaulting to 1000Mb\n");
> +	case SPEED_1000:
> +		val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> +		break;
> +	case SPEED_100:
> +		val |= NICPM_IOMUX_CTRL_SPD_100M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> +		break;
> +	case SPEED_10:
> +		val |= NICPM_IOMUX_CTRL_SPD_10M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> +		break;
> +	}
> +
> +	writel(val, bgmac->plat.nicpm_base + NICPM_IOMUX_CTRL);
> +
> +	usleep_range(10, 100);

Does not seem like a good idea, do you need that sleep?

> +
> +	bgmac_adjust_link(bgmac->net_dev);
> +}
> +
>  static int platform_phy_connect(struct bgmac *bgmac)
>  {
>  	struct phy_device *phy_dev;
>  
> -	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
> -					 bgmac_adjust_link);
> +	if (bgmac->plat.nicpm_base)
> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
> +						 bgmac->dev->of_node,
> +						 bgmac_nicpm_speed_set);
> +	else
> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
> +						 bgmac->dev->of_node,
> +						 bgmac_adjust_link);
>  	if (!phy_dev) {
>  		dev_err(bgmac->dev, "Phy connect failed\n");
>  		return -ENODEV;
>  	}
>  
> +	if (bgmac->feature_flags & BGMAC_FEAT_LANE_SWAP)
> +		phy_dev->dev_flags |= PHY_BRCM_EXP_LANE_SWAP;
> +
>  	return 0;
>  }
>  
> @@ -140,6 +188,9 @@ static int bgmac_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, bgmac);
>  
> +	if (of_property_read_bool(np, "brcm,enet-phy-lane-swap"))
> +		bgmac->feature_flags |= BGMAC_FEAT_LANE_SWAP;
> +
>  	/* Set the features of the 4707 family */
>  	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
>  	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
> @@ -182,6 +233,14 @@ static int bgmac_probe(struct platform_device *pdev)
>  	if (IS_ERR(bgmac->plat.idm_base))
>  		return PTR_ERR(bgmac->plat.idm_base);
>  
> +	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base");
> +	if (regs) {
> +		bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev,
> +							       regs);
> +		if (IS_ERR(bgmac->plat.nicpm_base))
> +			return PTR_ERR(bgmac->plat.nicpm_base);
> +	}
> +
>  	bgmac->read = platform_bgmac_read;
>  	bgmac->write = platform_bgmac_write;
>  	bgmac->idm_read = platform_bgmac_idm_read;
> @@ -213,6 +272,7 @@ static int bgmac_remove(struct platform_device *pdev)
>  static const struct of_device_id bgmac_of_enet_match[] = {
>  	{.compatible = "brcm,amac",},
>  	{.compatible = "brcm,nsp-amac",},
> +	{.compatible = "brcm,ns2-amac",},
>  	{},
>  };
>  
> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> index 38876ec..1796208 100644
> --- a/drivers/net/ethernet/broadcom/bgmac.c
> +++ b/drivers/net/ethernet/broadcom/bgmac.c
> @@ -1082,6 +1082,9 @@ static void bgmac_enable(struct bgmac *bgmac)
>  /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
>  static void bgmac_chip_init(struct bgmac *bgmac)
>  {
> +	/* Clear any erroneously pending interrupts */
> +	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
> +
>  	/* 1 interrupt per received frame */
>  	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
>  
> @@ -1158,8 +1161,6 @@ static int bgmac_open(struct net_device *net_dev)
>  	struct bgmac *bgmac = netdev_priv(net_dev);
>  	int err = 0;
>  
> -	bgmac_chip_reset(bgmac);
> -

Is this removal intentional? Maybe it should be special cased with
checking for a NS2 BGMAC instance and not do it in that case?
-- 
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] net: ethernet: bgmac: add NS2 support

[Jon Mason]

On Wed, Oct 26, 2016 at 02:50:39PM -0700, Florian Fainelli wrote:
> On 10/26/2016 12:36 PM, Jon Mason wrote:
> > Add support for the variant of amac hardware present in the Broadcom
> > Northstar2 based SoCs.  Northstar2 requires an additional register to be
> > configured with the port speed/duplexity (NICPM).  This can be added to
> > the link callback to hide it from the instances that do not use this.
> > Also, the bgmac_chip_reset() was intentionally removed to prevent the
> > resetting of the chip to the default values on open.  Finally, clearing
> > of the pending interrupts on init is required due to observed issues on
> > some platforms.
> > 
> > Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> > ---
> 
> > +static void bgmac_nicpm_speed_set(struct net_device *net_dev)
> > +{
> > +	struct bgmac *bgmac = netdev_priv(net_dev);
> > +	u32 val;
> > +
> > +	if (!bgmac->plat.nicpm_base)
> > +		return;
> > +
> > +	val = NICPM_IOMUX_CTRL_INIT_VAL;
> > +	switch (bgmac->net_dev->phydev->speed) {
> > +	default:
> > +		pr_err("Unsupported speed.  Defaulting to 1000Mb\n");
> > +	case SPEED_1000:
> > +		val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> > +		break;
> > +	case SPEED_100:
> > +		val |= NICPM_IOMUX_CTRL_SPD_100M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> > +		break;
> > +	case SPEED_10:
> > +		val |= NICPM_IOMUX_CTRL_SPD_10M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> > +		break;
> > +	}
> > +
> > +	writel(val, bgmac->plat.nicpm_base + NICPM_IOMUX_CTRL);
> > +
> > +	usleep_range(10, 100);
> 
> Does not seem like a good idea, do you need that sleep?

Oops, that's not supposed to be there.  Removed.


> > +
> > +	bgmac_adjust_link(bgmac->net_dev);
> > +}
> > +
> >  static int platform_phy_connect(struct bgmac *bgmac)
> >  {
> >  	struct phy_device *phy_dev;
> >  
> > -	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
> > -					 bgmac_adjust_link);
> > +	if (bgmac->plat.nicpm_base)
> > +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
> > +						 bgmac->dev->of_node,
> > +						 bgmac_nicpm_speed_set);
> > +	else
> > +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
> > +						 bgmac->dev->of_node,
> > +						 bgmac_adjust_link);
> >  	if (!phy_dev) {
> >  		dev_err(bgmac->dev, "Phy connect failed\n");
> >  		return -ENODEV;
> >  	}
> >  
> > +	if (bgmac->feature_flags & BGMAC_FEAT_LANE_SWAP)
> > +		phy_dev->dev_flags |= PHY_BRCM_EXP_LANE_SWAP;
> > +
> >  	return 0;
> >  }
> >  
> > @@ -140,6 +188,9 @@ static int bgmac_probe(struct platform_device *pdev)
> >  
> >  	platform_set_drvdata(pdev, bgmac);
> >  
> > +	if (of_property_read_bool(np, "brcm,enet-phy-lane-swap"))
> > +		bgmac->feature_flags |= BGMAC_FEAT_LANE_SWAP;
> > +
> >  	/* Set the features of the 4707 family */
> >  	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
> >  	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
> > @@ -182,6 +233,14 @@ static int bgmac_probe(struct platform_device *pdev)
> >  	if (IS_ERR(bgmac->plat.idm_base))
> >  		return PTR_ERR(bgmac->plat.idm_base);
> >  
> > +	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base");
> > +	if (regs) {
> > +		bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev,
> > +							       regs);
> > +		if (IS_ERR(bgmac->plat.nicpm_base))
> > +			return PTR_ERR(bgmac->plat.nicpm_base);
> > +	}
> > +
> >  	bgmac->read = platform_bgmac_read;
> >  	bgmac->write = platform_bgmac_write;
> >  	bgmac->idm_read = platform_bgmac_idm_read;
> > @@ -213,6 +272,7 @@ static int bgmac_remove(struct platform_device *pdev)
> >  static const struct of_device_id bgmac_of_enet_match[] = {
> >  	{.compatible = "brcm,amac",},
> >  	{.compatible = "brcm,nsp-amac",},
> > +	{.compatible = "brcm,ns2-amac",},
> >  	{},
> >  };
> >  
> > diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> > index 38876ec..1796208 100644
> > --- a/drivers/net/ethernet/broadcom/bgmac.c
> > +++ b/drivers/net/ethernet/broadcom/bgmac.c
> > @@ -1082,6 +1082,9 @@ static void bgmac_enable(struct bgmac *bgmac)
> >  /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
> >  static void bgmac_chip_init(struct bgmac *bgmac)
> >  {
> > +	/* Clear any erroneously pending interrupts */
> > +	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
> > +
> >  	/* 1 interrupt per received frame */
> >  	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
> >  
> > @@ -1158,8 +1161,6 @@ static int bgmac_open(struct net_device *net_dev)
> >  	struct bgmac *bgmac = netdev_priv(net_dev);
> >  	int err = 0;
> >  
> > -	bgmac_chip_reset(bgmac);
> > -
> 
> Is this removal intentional? Maybe it should be special cased with
> checking for a NS2 BGMAC instance and not do it in that case?

The reset seems completely unnecessary.  There is already 2 resets in
the probe routine, another reset serves no purpose.  I can add it
back, as it does not seem to have the negative effect I was seeing
before.

Of course, if I remove this one I should remove the reset in the close
too (which seems even more unnecessary, but I didn't remove it).

Thanks,
Jon

> -- 
> Florian

[PATCH 4/5] net: ethernet: bgmac: add NS2 support

[Jon Mason]

On Wed, Oct 26, 2016 at 02:50:39PM -0700, Florian Fainelli wrote:
> On 10/26/2016 12:36 PM, Jon Mason wrote:
> > Add support for the variant of amac hardware present in the Broadcom
> > Northstar2 based SoCs.  Northstar2 requires an additional register to be
> > configured with the port speed/duplexity (NICPM).  This can be added to
> > the link callback to hide it from the instances that do not use this.
> > Also, the bgmac_chip_reset() was intentionally removed to prevent the
> > resetting of the chip to the default values on open.  Finally, clearing
> > of the pending interrupts on init is required due to observed issues on
> > some platforms.
> > 
> > Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> > ---
> 
> > +static void bgmac_nicpm_speed_set(struct net_device *net_dev)
> > +{
> > +	struct bgmac *bgmac = netdev_priv(net_dev);
> > +	u32 val;
> > +
> > +	if (!bgmac->plat.nicpm_base)
> > +		return;
> > +
> > +	val = NICPM_IOMUX_CTRL_INIT_VAL;
> > +	switch (bgmac->net_dev->phydev->speed) {
> > +	default:
> > +		pr_err("Unsupported speed.  Defaulting to 1000Mb\n");
> > +	case SPEED_1000:
> > +		val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> > +		break;
> > +	case SPEED_100:
> > +		val |= NICPM_IOMUX_CTRL_SPD_100M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> > +		break;
> > +	case SPEED_10:
> > +		val |= NICPM_IOMUX_CTRL_SPD_10M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> > +		break;
> > +	}
> > +
> > +	writel(val, bgmac->plat.nicpm_base + NICPM_IOMUX_CTRL);
> > +
> > +	usleep_range(10, 100);
> 
> Does not seem like a good idea, do you need that sleep?

Oops, that's not supposed to be there.  Removed.


> > +
> > +	bgmac_adjust_link(bgmac->net_dev);
> > +}
> > +
> >  static int platform_phy_connect(struct bgmac *bgmac)
> >  {
> >  	struct phy_device *phy_dev;
> >  
> > -	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
> > -					 bgmac_adjust_link);
> > +	if (bgmac->plat.nicpm_base)
> > +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
> > +						 bgmac->dev->of_node,
> > +						 bgmac_nicpm_speed_set);
> > +	else
> > +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
> > +						 bgmac->dev->of_node,
> > +						 bgmac_adjust_link);
> >  	if (!phy_dev) {
> >  		dev_err(bgmac->dev, "Phy connect failed\n");
> >  		return -ENODEV;
> >  	}
> >  
> > +	if (bgmac->feature_flags & BGMAC_FEAT_LANE_SWAP)
> > +		phy_dev->dev_flags |= PHY_BRCM_EXP_LANE_SWAP;
> > +
> >  	return 0;
> >  }
> >  
> > @@ -140,6 +188,9 @@ static int bgmac_probe(struct platform_device *pdev)
> >  
> >  	platform_set_drvdata(pdev, bgmac);
> >  
> > +	if (of_property_read_bool(np, "brcm,enet-phy-lane-swap"))
> > +		bgmac->feature_flags |= BGMAC_FEAT_LANE_SWAP;
> > +
> >  	/* Set the features of the 4707 family */
> >  	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
> >  	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
> > @@ -182,6 +233,14 @@ static int bgmac_probe(struct platform_device *pdev)
> >  	if (IS_ERR(bgmac->plat.idm_base))
> >  		return PTR_ERR(bgmac->plat.idm_base);
> >  
> > +	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base");
> > +	if (regs) {
> > +		bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev,
> > +							       regs);
> > +		if (IS_ERR(bgmac->plat.nicpm_base))
> > +			return PTR_ERR(bgmac->plat.nicpm_base);
> > +	}
> > +
> >  	bgmac->read = platform_bgmac_read;
> >  	bgmac->write = platform_bgmac_write;
> >  	bgmac->idm_read = platform_bgmac_idm_read;
> > @@ -213,6 +272,7 @@ static int bgmac_remove(struct platform_device *pdev)
> >  static const struct of_device_id bgmac_of_enet_match[] = {
> >  	{.compatible = "brcm,amac",},
> >  	{.compatible = "brcm,nsp-amac",},
> > +	{.compatible = "brcm,ns2-amac",},
> >  	{},
> >  };
> >  
> > diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> > index 38876ec..1796208 100644
> > --- a/drivers/net/ethernet/broadcom/bgmac.c
> > +++ b/drivers/net/ethernet/broadcom/bgmac.c
> > @@ -1082,6 +1082,9 @@ static void bgmac_enable(struct bgmac *bgmac)
> >  /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
> >  static void bgmac_chip_init(struct bgmac *bgmac)
> >  {
> > +	/* Clear any erroneously pending interrupts */
> > +	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
> > +
> >  	/* 1 interrupt per received frame */
> >  	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
> >  
> > @@ -1158,8 +1161,6 @@ static int bgmac_open(struct net_device *net_dev)
> >  	struct bgmac *bgmac = netdev_priv(net_dev);
> >  	int err = 0;
> >  
> > -	bgmac_chip_reset(bgmac);
> > -
> 
> Is this removal intentional? Maybe it should be special cased with
> checking for a NS2 BGMAC instance and not do it in that case?

The reset seems completely unnecessary.  There is already 2 resets in
the probe routine, another reset serves no purpose.  I can add it
back, as it does not seem to have the negative effect I was seeing
before.

Of course, if I remove this one I should remove the reset in the close
too (which seems even more unnecessary, but I didn't remove it).

Thanks,
Jon

> -- 
> Florian

Re: [PATCH 4/5] net: ethernet: bgmac: add NS2 support

[Jon Mason]

On Wed, Oct 26, 2016 at 02:50:39PM -0700, Florian Fainelli wrote:
> On 10/26/2016 12:36 PM, Jon Mason wrote:
> > Add support for the variant of amac hardware present in the Broadcom
> > Northstar2 based SoCs.  Northstar2 requires an additional register to be
> > configured with the port speed/duplexity (NICPM).  This can be added to
> > the link callback to hide it from the instances that do not use this.
> > Also, the bgmac_chip_reset() was intentionally removed to prevent the
> > resetting of the chip to the default values on open.  Finally, clearing
> > of the pending interrupts on init is required due to observed issues on
> > some platforms.
> > 
> > Signed-off-by: Jon Mason <jon.mason-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> > ---
> 
> > +static void bgmac_nicpm_speed_set(struct net_device *net_dev)
> > +{
> > +	struct bgmac *bgmac = netdev_priv(net_dev);
> > +	u32 val;
> > +
> > +	if (!bgmac->plat.nicpm_base)
> > +		return;
> > +
> > +	val = NICPM_IOMUX_CTRL_INIT_VAL;
> > +	switch (bgmac->net_dev->phydev->speed) {
> > +	default:
> > +		pr_err("Unsupported speed.  Defaulting to 1000Mb\n");
> > +	case SPEED_1000:
> > +		val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> > +		break;
> > +	case SPEED_100:
> > +		val |= NICPM_IOMUX_CTRL_SPD_100M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> > +		break;
> > +	case SPEED_10:
> > +		val |= NICPM_IOMUX_CTRL_SPD_10M << NICPM_IOMUX_CTRL_SPD_SHIFT;
> > +		break;
> > +	}
> > +
> > +	writel(val, bgmac->plat.nicpm_base + NICPM_IOMUX_CTRL);
> > +
> > +	usleep_range(10, 100);
> 
> Does not seem like a good idea, do you need that sleep?

Oops, that's not supposed to be there.  Removed.


> > +
> > +	bgmac_adjust_link(bgmac->net_dev);
> > +}
> > +
> >  static int platform_phy_connect(struct bgmac *bgmac)
> >  {
> >  	struct phy_device *phy_dev;
> >  
> > -	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
> > -					 bgmac_adjust_link);
> > +	if (bgmac->plat.nicpm_base)
> > +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
> > +						 bgmac->dev->of_node,
> > +						 bgmac_nicpm_speed_set);
> > +	else
> > +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
> > +						 bgmac->dev->of_node,
> > +						 bgmac_adjust_link);
> >  	if (!phy_dev) {
> >  		dev_err(bgmac->dev, "Phy connect failed\n");
> >  		return -ENODEV;
> >  	}
> >  
> > +	if (bgmac->feature_flags & BGMAC_FEAT_LANE_SWAP)
> > +		phy_dev->dev_flags |= PHY_BRCM_EXP_LANE_SWAP;
> > +
> >  	return 0;
> >  }
> >  
> > @@ -140,6 +188,9 @@ static int bgmac_probe(struct platform_device *pdev)
> >  
> >  	platform_set_drvdata(pdev, bgmac);
> >  
> > +	if (of_property_read_bool(np, "brcm,enet-phy-lane-swap"))
> > +		bgmac->feature_flags |= BGMAC_FEAT_LANE_SWAP;
> > +
> >  	/* Set the features of the 4707 family */
> >  	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
> >  	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
> > @@ -182,6 +233,14 @@ static int bgmac_probe(struct platform_device *pdev)
> >  	if (IS_ERR(bgmac->plat.idm_base))
> >  		return PTR_ERR(bgmac->plat.idm_base);
> >  
> > +	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base");
> > +	if (regs) {
> > +		bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev,
> > +							       regs);
> > +		if (IS_ERR(bgmac->plat.nicpm_base))
> > +			return PTR_ERR(bgmac->plat.nicpm_base);
> > +	}
> > +
> >  	bgmac->read = platform_bgmac_read;
> >  	bgmac->write = platform_bgmac_write;
> >  	bgmac->idm_read = platform_bgmac_idm_read;
> > @@ -213,6 +272,7 @@ static int bgmac_remove(struct platform_device *pdev)
> >  static const struct of_device_id bgmac_of_enet_match[] = {
> >  	{.compatible = "brcm,amac",},
> >  	{.compatible = "brcm,nsp-amac",},
> > +	{.compatible = "brcm,ns2-amac",},
> >  	{},
> >  };
> >  
> > diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> > index 38876ec..1796208 100644
> > --- a/drivers/net/ethernet/broadcom/bgmac.c
> > +++ b/drivers/net/ethernet/broadcom/bgmac.c
> > @@ -1082,6 +1082,9 @@ static void bgmac_enable(struct bgmac *bgmac)
> >  /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
> >  static void bgmac_chip_init(struct bgmac *bgmac)
> >  {
> > +	/* Clear any erroneously pending interrupts */
> > +	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
> > +
> >  	/* 1 interrupt per received frame */
> >  	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
> >  
> > @@ -1158,8 +1161,6 @@ static int bgmac_open(struct net_device *net_dev)
> >  	struct bgmac *bgmac = netdev_priv(net_dev);
> >  	int err = 0;
> >  
> > -	bgmac_chip_reset(bgmac);
> > -
> 
> Is this removal intentional? Maybe it should be special cased with
> checking for a NS2 BGMAC instance and not do it in that case?

The reset seems completely unnecessary.  There is already 2 resets in
the probe routine, another reset serves no purpose.  I can add it
back, as it does not seem to have the negative effect I was seeing
before.

Of course, if I remove this one I should remove the reset in the close
too (which seems even more unnecessary, but I didn't remove it).

Thanks,
Jon

> -- 
> Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] net: ethernet: bgmac: add NS2 support

[Ray Jui]

On 10/27/2016 1:51 PM, Jon Mason wrote:
> On Wed, Oct 26, 2016 at 02:50:39PM -0700, Florian Fainelli wrote:
>> On 10/26/2016 12:36 PM, Jon Mason wrote:
>>> Add support for the variant of amac hardware present in the Broadcom
>>> Northstar2 based SoCs.  Northstar2 requires an additional register to be
>>> configured with the port speed/duplexity (NICPM).  This can be added to
>>> the link callback to hide it from the instances that do not use this.
>>> Also, the bgmac_chip_reset() was intentionally removed to prevent the
>>> resetting of the chip to the default values on open.  Finally, clearing
>>> of the pending interrupts on init is required due to observed issues on
>>> some platforms.
>>>
>>> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>>> ---
>>
>>> +static void bgmac_nicpm_speed_set(struct net_device *net_dev)
>>> +{
>>> +	struct bgmac *bgmac = netdev_priv(net_dev);
>>> +	u32 val;
>>> +
>>> +	if (!bgmac->plat.nicpm_base)
>>> +		return;
>>> +
>>> +	val = NICPM_IOMUX_CTRL_INIT_VAL;
>>> +	switch (bgmac->net_dev->phydev->speed) {
>>> +	default:
>>> +		pr_err("Unsupported speed.  Defaulting to 1000Mb\n");
>>> +	case SPEED_1000:
>>> +		val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>> +		break;
>>> +	case SPEED_100:
>>> +		val |= NICPM_IOMUX_CTRL_SPD_100M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>> +		break;
>>> +	case SPEED_10:
>>> +		val |= NICPM_IOMUX_CTRL_SPD_10M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>> +		break;
>>> +	}
>>> +
>>> +	writel(val, bgmac->plat.nicpm_base + NICPM_IOMUX_CTRL);
>>> +
>>> +	usleep_range(10, 100);
>>
>> Does not seem like a good idea, do you need that sleep?
> 
> Oops, that's not supposed to be there.  Removed.
> 
> 
>>> +
>>> +	bgmac_adjust_link(bgmac->net_dev);
>>> +}
>>> +
>>>  static int platform_phy_connect(struct bgmac *bgmac)
>>>  {
>>>  	struct phy_device *phy_dev;
>>>  
>>> -	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
>>> -					 bgmac_adjust_link);
>>> +	if (bgmac->plat.nicpm_base)
>>> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
>>> +						 bgmac->dev->of_node,
>>> +						 bgmac_nicpm_speed_set);
>>> +	else
>>> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
>>> +						 bgmac->dev->of_node,
>>> +						 bgmac_adjust_link);
>>>  	if (!phy_dev) {
>>>  		dev_err(bgmac->dev, "Phy connect failed\n");
>>>  		return -ENODEV;
>>>  	}
>>>  
>>> +	if (bgmac->feature_flags & BGMAC_FEAT_LANE_SWAP)
>>> +		phy_dev->dev_flags |= PHY_BRCM_EXP_LANE_SWAP;
>>> +
>>>  	return 0;
>>>  }
>>>  
>>> @@ -140,6 +188,9 @@ static int bgmac_probe(struct platform_device *pdev)
>>>  
>>>  	platform_set_drvdata(pdev, bgmac);
>>>  
>>> +	if (of_property_read_bool(np, "brcm,enet-phy-lane-swap"))
>>> +		bgmac->feature_flags |= BGMAC_FEAT_LANE_SWAP;
>>> +
>>>  	/* Set the features of the 4707 family */
>>>  	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
>>>  	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
>>> @@ -182,6 +233,14 @@ static int bgmac_probe(struct platform_device *pdev)
>>>  	if (IS_ERR(bgmac->plat.idm_base))
>>>  		return PTR_ERR(bgmac->plat.idm_base);
>>>  
>>> +	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base");
>>> +	if (regs) {
>>> +		bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev,
>>> +							       regs);
>>> +		if (IS_ERR(bgmac->plat.nicpm_base))
>>> +			return PTR_ERR(bgmac->plat.nicpm_base);
>>> +	}
>>> +
>>>  	bgmac->read = platform_bgmac_read;
>>>  	bgmac->write = platform_bgmac_write;
>>>  	bgmac->idm_read = platform_bgmac_idm_read;
>>> @@ -213,6 +272,7 @@ static int bgmac_remove(struct platform_device *pdev)
>>>  static const struct of_device_id bgmac_of_enet_match[] = {
>>>  	{.compatible = "brcm,amac",},
>>>  	{.compatible = "brcm,nsp-amac",},
>>> +	{.compatible = "brcm,ns2-amac",},
>>>  	{},
>>>  };
>>>  
>>> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
>>> index 38876ec..1796208 100644
>>> --- a/drivers/net/ethernet/broadcom/bgmac.c
>>> +++ b/drivers/net/ethernet/broadcom/bgmac.c
>>> @@ -1082,6 +1082,9 @@ static void bgmac_enable(struct bgmac *bgmac)
>>>  /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
>>>  static void bgmac_chip_init(struct bgmac *bgmac)
>>>  {
>>> +	/* Clear any erroneously pending interrupts */
>>> +	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
>>> +
>>>  	/* 1 interrupt per received frame */
>>>  	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
>>>  
>>> @@ -1158,8 +1161,6 @@ static int bgmac_open(struct net_device *net_dev)
>>>  	struct bgmac *bgmac = netdev_priv(net_dev);
>>>  	int err = 0;
>>>  
>>> -	bgmac_chip_reset(bgmac);
>>> -
>>
>> Is this removal intentional? Maybe it should be special cased with
>> checking for a NS2 BGMAC instance and not do it in that case?
> 
> The reset seems completely unnecessary.  There is already 2 resets in
> the probe routine, another reset serves no purpose.  I can add it
> back, as it does not seem to have the negative effect I was seeing
> before.

After repeated ifconfig down/up, does Ethernet still work with this
reset removed?

Thanks.

> 
> Of course, if I remove this one I should remove the reset in the close
> too (which seems even more unnecessary, but I didn't remove it).
> 
> Thanks,
> Jon
> 
>> -- 
>> Florian

[PATCH 4/5] net: ethernet: bgmac: add NS2 support

[Ray Jui]

On 10/27/2016 1:51 PM, Jon Mason wrote:
> On Wed, Oct 26, 2016 at 02:50:39PM -0700, Florian Fainelli wrote:
>> On 10/26/2016 12:36 PM, Jon Mason wrote:
>>> Add support for the variant of amac hardware present in the Broadcom
>>> Northstar2 based SoCs.  Northstar2 requires an additional register to be
>>> configured with the port speed/duplexity (NICPM).  This can be added to
>>> the link callback to hide it from the instances that do not use this.
>>> Also, the bgmac_chip_reset() was intentionally removed to prevent the
>>> resetting of the chip to the default values on open.  Finally, clearing
>>> of the pending interrupts on init is required due to observed issues on
>>> some platforms.
>>>
>>> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>>> ---
>>
>>> +static void bgmac_nicpm_speed_set(struct net_device *net_dev)
>>> +{
>>> +	struct bgmac *bgmac = netdev_priv(net_dev);
>>> +	u32 val;
>>> +
>>> +	if (!bgmac->plat.nicpm_base)
>>> +		return;
>>> +
>>> +	val = NICPM_IOMUX_CTRL_INIT_VAL;
>>> +	switch (bgmac->net_dev->phydev->speed) {
>>> +	default:
>>> +		pr_err("Unsupported speed.  Defaulting to 1000Mb\n");
>>> +	case SPEED_1000:
>>> +		val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>> +		break;
>>> +	case SPEED_100:
>>> +		val |= NICPM_IOMUX_CTRL_SPD_100M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>> +		break;
>>> +	case SPEED_10:
>>> +		val |= NICPM_IOMUX_CTRL_SPD_10M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>> +		break;
>>> +	}
>>> +
>>> +	writel(val, bgmac->plat.nicpm_base + NICPM_IOMUX_CTRL);
>>> +
>>> +	usleep_range(10, 100);
>>
>> Does not seem like a good idea, do you need that sleep?
> 
> Oops, that's not supposed to be there.  Removed.
> 
> 
>>> +
>>> +	bgmac_adjust_link(bgmac->net_dev);
>>> +}
>>> +
>>>  static int platform_phy_connect(struct bgmac *bgmac)
>>>  {
>>>  	struct phy_device *phy_dev;
>>>  
>>> -	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
>>> -					 bgmac_adjust_link);
>>> +	if (bgmac->plat.nicpm_base)
>>> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
>>> +						 bgmac->dev->of_node,
>>> +						 bgmac_nicpm_speed_set);
>>> +	else
>>> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
>>> +						 bgmac->dev->of_node,
>>> +						 bgmac_adjust_link);
>>>  	if (!phy_dev) {
>>>  		dev_err(bgmac->dev, "Phy connect failed\n");
>>>  		return -ENODEV;
>>>  	}
>>>  
>>> +	if (bgmac->feature_flags & BGMAC_FEAT_LANE_SWAP)
>>> +		phy_dev->dev_flags |= PHY_BRCM_EXP_LANE_SWAP;
>>> +
>>>  	return 0;
>>>  }
>>>  
>>> @@ -140,6 +188,9 @@ static int bgmac_probe(struct platform_device *pdev)
>>>  
>>>  	platform_set_drvdata(pdev, bgmac);
>>>  
>>> +	if (of_property_read_bool(np, "brcm,enet-phy-lane-swap"))
>>> +		bgmac->feature_flags |= BGMAC_FEAT_LANE_SWAP;
>>> +
>>>  	/* Set the features of the 4707 family */
>>>  	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
>>>  	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
>>> @@ -182,6 +233,14 @@ static int bgmac_probe(struct platform_device *pdev)
>>>  	if (IS_ERR(bgmac->plat.idm_base))
>>>  		return PTR_ERR(bgmac->plat.idm_base);
>>>  
>>> +	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base");
>>> +	if (regs) {
>>> +		bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev,
>>> +							       regs);
>>> +		if (IS_ERR(bgmac->plat.nicpm_base))
>>> +			return PTR_ERR(bgmac->plat.nicpm_base);
>>> +	}
>>> +
>>>  	bgmac->read = platform_bgmac_read;
>>>  	bgmac->write = platform_bgmac_write;
>>>  	bgmac->idm_read = platform_bgmac_idm_read;
>>> @@ -213,6 +272,7 @@ static int bgmac_remove(struct platform_device *pdev)
>>>  static const struct of_device_id bgmac_of_enet_match[] = {
>>>  	{.compatible = "brcm,amac",},
>>>  	{.compatible = "brcm,nsp-amac",},
>>> +	{.compatible = "brcm,ns2-amac",},
>>>  	{},
>>>  };
>>>  
>>> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
>>> index 38876ec..1796208 100644
>>> --- a/drivers/net/ethernet/broadcom/bgmac.c
>>> +++ b/drivers/net/ethernet/broadcom/bgmac.c
>>> @@ -1082,6 +1082,9 @@ static void bgmac_enable(struct bgmac *bgmac)
>>>  /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
>>>  static void bgmac_chip_init(struct bgmac *bgmac)
>>>  {
>>> +	/* Clear any erroneously pending interrupts */
>>> +	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
>>> +
>>>  	/* 1 interrupt per received frame */
>>>  	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
>>>  
>>> @@ -1158,8 +1161,6 @@ static int bgmac_open(struct net_device *net_dev)
>>>  	struct bgmac *bgmac = netdev_priv(net_dev);
>>>  	int err = 0;
>>>  
>>> -	bgmac_chip_reset(bgmac);
>>> -
>>
>> Is this removal intentional? Maybe it should be special cased with
>> checking for a NS2 BGMAC instance and not do it in that case?
> 
> The reset seems completely unnecessary.  There is already 2 resets in
> the probe routine, another reset serves no purpose.  I can add it
> back, as it does not seem to have the negative effect I was seeing
> before.

After repeated ifconfig down/up, does Ethernet still work with this
reset removed?

Thanks.

> 
> Of course, if I remove this one I should remove the reset in the close
> too (which seems even more unnecessary, but I didn't remove it).
> 
> Thanks,
> Jon
> 
>> -- 
>> Florian

Re: [PATCH 4/5] net: ethernet: bgmac: add NS2 support

[Ray Jui]

On 10/27/2016 1:51 PM, Jon Mason wrote:
> On Wed, Oct 26, 2016 at 02:50:39PM -0700, Florian Fainelli wrote:
>> On 10/26/2016 12:36 PM, Jon Mason wrote:
>>> Add support for the variant of amac hardware present in the Broadcom
>>> Northstar2 based SoCs.  Northstar2 requires an additional register to be
>>> configured with the port speed/duplexity (NICPM).  This can be added to
>>> the link callback to hide it from the instances that do not use this.
>>> Also, the bgmac_chip_reset() was intentionally removed to prevent the
>>> resetting of the chip to the default values on open.  Finally, clearing
>>> of the pending interrupts on init is required due to observed issues on
>>> some platforms.
>>>
>>> Signed-off-by: Jon Mason <jon.mason-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>> ---
>>
>>> +static void bgmac_nicpm_speed_set(struct net_device *net_dev)
>>> +{
>>> +	struct bgmac *bgmac = netdev_priv(net_dev);
>>> +	u32 val;
>>> +
>>> +	if (!bgmac->plat.nicpm_base)
>>> +		return;
>>> +
>>> +	val = NICPM_IOMUX_CTRL_INIT_VAL;
>>> +	switch (bgmac->net_dev->phydev->speed) {
>>> +	default:
>>> +		pr_err("Unsupported speed.  Defaulting to 1000Mb\n");
>>> +	case SPEED_1000:
>>> +		val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>> +		break;
>>> +	case SPEED_100:
>>> +		val |= NICPM_IOMUX_CTRL_SPD_100M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>> +		break;
>>> +	case SPEED_10:
>>> +		val |= NICPM_IOMUX_CTRL_SPD_10M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>> +		break;
>>> +	}
>>> +
>>> +	writel(val, bgmac->plat.nicpm_base + NICPM_IOMUX_CTRL);
>>> +
>>> +	usleep_range(10, 100);
>>
>> Does not seem like a good idea, do you need that sleep?
> 
> Oops, that's not supposed to be there.  Removed.
> 
> 
>>> +
>>> +	bgmac_adjust_link(bgmac->net_dev);
>>> +}
>>> +
>>>  static int platform_phy_connect(struct bgmac *bgmac)
>>>  {
>>>  	struct phy_device *phy_dev;
>>>  
>>> -	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
>>> -					 bgmac_adjust_link);
>>> +	if (bgmac->plat.nicpm_base)
>>> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
>>> +						 bgmac->dev->of_node,
>>> +						 bgmac_nicpm_speed_set);
>>> +	else
>>> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
>>> +						 bgmac->dev->of_node,
>>> +						 bgmac_adjust_link);
>>>  	if (!phy_dev) {
>>>  		dev_err(bgmac->dev, "Phy connect failed\n");
>>>  		return -ENODEV;
>>>  	}
>>>  
>>> +	if (bgmac->feature_flags & BGMAC_FEAT_LANE_SWAP)
>>> +		phy_dev->dev_flags |= PHY_BRCM_EXP_LANE_SWAP;
>>> +
>>>  	return 0;
>>>  }
>>>  
>>> @@ -140,6 +188,9 @@ static int bgmac_probe(struct platform_device *pdev)
>>>  
>>>  	platform_set_drvdata(pdev, bgmac);
>>>  
>>> +	if (of_property_read_bool(np, "brcm,enet-phy-lane-swap"))
>>> +		bgmac->feature_flags |= BGMAC_FEAT_LANE_SWAP;
>>> +
>>>  	/* Set the features of the 4707 family */
>>>  	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
>>>  	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
>>> @@ -182,6 +233,14 @@ static int bgmac_probe(struct platform_device *pdev)
>>>  	if (IS_ERR(bgmac->plat.idm_base))
>>>  		return PTR_ERR(bgmac->plat.idm_base);
>>>  
>>> +	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base");
>>> +	if (regs) {
>>> +		bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev,
>>> +							       regs);
>>> +		if (IS_ERR(bgmac->plat.nicpm_base))
>>> +			return PTR_ERR(bgmac->plat.nicpm_base);
>>> +	}
>>> +
>>>  	bgmac->read = platform_bgmac_read;
>>>  	bgmac->write = platform_bgmac_write;
>>>  	bgmac->idm_read = platform_bgmac_idm_read;
>>> @@ -213,6 +272,7 @@ static int bgmac_remove(struct platform_device *pdev)
>>>  static const struct of_device_id bgmac_of_enet_match[] = {
>>>  	{.compatible = "brcm,amac",},
>>>  	{.compatible = "brcm,nsp-amac",},
>>> +	{.compatible = "brcm,ns2-amac",},
>>>  	{},
>>>  };
>>>  
>>> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
>>> index 38876ec..1796208 100644
>>> --- a/drivers/net/ethernet/broadcom/bgmac.c
>>> +++ b/drivers/net/ethernet/broadcom/bgmac.c
>>> @@ -1082,6 +1082,9 @@ static void bgmac_enable(struct bgmac *bgmac)
>>>  /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
>>>  static void bgmac_chip_init(struct bgmac *bgmac)
>>>  {
>>> +	/* Clear any erroneously pending interrupts */
>>> +	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
>>> +
>>>  	/* 1 interrupt per received frame */
>>>  	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
>>>  
>>> @@ -1158,8 +1161,6 @@ static int bgmac_open(struct net_device *net_dev)
>>>  	struct bgmac *bgmac = netdev_priv(net_dev);
>>>  	int err = 0;
>>>  
>>> -	bgmac_chip_reset(bgmac);
>>> -
>>
>> Is this removal intentional? Maybe it should be special cased with
>> checking for a NS2 BGMAC instance and not do it in that case?
> 
> The reset seems completely unnecessary.  There is already 2 resets in
> the probe routine, another reset serves no purpose.  I can add it
> back, as it does not seem to have the negative effect I was seeing
> before.

After repeated ifconfig down/up, does Ethernet still work with this
reset removed?

Thanks.

> 
> Of course, if I remove this one I should remove the reset in the close
> too (which seems even more unnecessary, but I didn't remove it).
> 
> Thanks,
> Jon
> 
>> -- 
>> Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] net: ethernet: bgmac: add NS2 support

[Ray Jui]

On 10/27/2016 3:21 PM, Ray Jui wrote:
> 
> 
> On 10/27/2016 1:51 PM, Jon Mason wrote:
>> On Wed, Oct 26, 2016 at 02:50:39PM -0700, Florian Fainelli wrote:
>>> On 10/26/2016 12:36 PM, Jon Mason wrote:
>>>> Add support for the variant of amac hardware present in the Broadcom
>>>> Northstar2 based SoCs.  Northstar2 requires an additional register to be
>>>> configured with the port speed/duplexity (NICPM).  This can be added to
>>>> the link callback to hide it from the instances that do not use this.
>>>> Also, the bgmac_chip_reset() was intentionally removed to prevent the
>>>> resetting of the chip to the default values on open.  Finally, clearing
>>>> of the pending interrupts on init is required due to observed issues on
>>>> some platforms.
>>>>
>>>> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>>>> ---
>>>
>>>> +static void bgmac_nicpm_speed_set(struct net_device *net_dev)
>>>> +{
>>>> +	struct bgmac *bgmac = netdev_priv(net_dev);
>>>> +	u32 val;
>>>> +
>>>> +	if (!bgmac->plat.nicpm_base)
>>>> +		return;
>>>> +
>>>> +	val = NICPM_IOMUX_CTRL_INIT_VAL;
>>>> +	switch (bgmac->net_dev->phydev->speed) {
>>>> +	default:
>>>> +		pr_err("Unsupported speed.  Defaulting to 1000Mb\n");
>>>> +	case SPEED_1000:
>>>> +		val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>>> +		break;
>>>> +	case SPEED_100:
>>>> +		val |= NICPM_IOMUX_CTRL_SPD_100M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>>> +		break;
>>>> +	case SPEED_10:
>>>> +		val |= NICPM_IOMUX_CTRL_SPD_10M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>>> +		break;
>>>> +	}
>>>> +
>>>> +	writel(val, bgmac->plat.nicpm_base + NICPM_IOMUX_CTRL);
>>>> +
>>>> +	usleep_range(10, 100);
>>>
>>> Does not seem like a good idea, do you need that sleep?
>>
>> Oops, that's not supposed to be there.  Removed.
>>
>>
>>>> +
>>>> +	bgmac_adjust_link(bgmac->net_dev);
>>>> +}
>>>> +
>>>>  static int platform_phy_connect(struct bgmac *bgmac)
>>>>  {
>>>>  	struct phy_device *phy_dev;
>>>>  
>>>> -	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
>>>> -					 bgmac_adjust_link);
>>>> +	if (bgmac->plat.nicpm_base)
>>>> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
>>>> +						 bgmac->dev->of_node,
>>>> +						 bgmac_nicpm_speed_set);
>>>> +	else
>>>> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
>>>> +						 bgmac->dev->of_node,
>>>> +						 bgmac_adjust_link);
>>>>  	if (!phy_dev) {
>>>>  		dev_err(bgmac->dev, "Phy connect failed\n");
>>>>  		return -ENODEV;
>>>>  	}
>>>>  
>>>> +	if (bgmac->feature_flags & BGMAC_FEAT_LANE_SWAP)
>>>> +		phy_dev->dev_flags |= PHY_BRCM_EXP_LANE_SWAP;
>>>> +
>>>>  	return 0;
>>>>  }
>>>>  
>>>> @@ -140,6 +188,9 @@ static int bgmac_probe(struct platform_device *pdev)
>>>>  
>>>>  	platform_set_drvdata(pdev, bgmac);
>>>>  
>>>> +	if (of_property_read_bool(np, "brcm,enet-phy-lane-swap"))
>>>> +		bgmac->feature_flags |= BGMAC_FEAT_LANE_SWAP;
>>>> +
>>>>  	/* Set the features of the 4707 family */
>>>>  	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
>>>>  	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
>>>> @@ -182,6 +233,14 @@ static int bgmac_probe(struct platform_device *pdev)
>>>>  	if (IS_ERR(bgmac->plat.idm_base))
>>>>  		return PTR_ERR(bgmac->plat.idm_base);
>>>>  
>>>> +	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base");
>>>> +	if (regs) {
>>>> +		bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev,
>>>> +							       regs);
>>>> +		if (IS_ERR(bgmac->plat.nicpm_base))
>>>> +			return PTR_ERR(bgmac->plat.nicpm_base);
>>>> +	}
>>>> +
>>>>  	bgmac->read = platform_bgmac_read;
>>>>  	bgmac->write = platform_bgmac_write;
>>>>  	bgmac->idm_read = platform_bgmac_idm_read;
>>>> @@ -213,6 +272,7 @@ static int bgmac_remove(struct platform_device *pdev)
>>>>  static const struct of_device_id bgmac_of_enet_match[] = {
>>>>  	{.compatible = "brcm,amac",},
>>>>  	{.compatible = "brcm,nsp-amac",},
>>>> +	{.compatible = "brcm,ns2-amac",},
>>>>  	{},
>>>>  };
>>>>  
>>>> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
>>>> index 38876ec..1796208 100644
>>>> --- a/drivers/net/ethernet/broadcom/bgmac.c
>>>> +++ b/drivers/net/ethernet/broadcom/bgmac.c
>>>> @@ -1082,6 +1082,9 @@ static void bgmac_enable(struct bgmac *bgmac)
>>>>  /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
>>>>  static void bgmac_chip_init(struct bgmac *bgmac)
>>>>  {
>>>> +	/* Clear any erroneously pending interrupts */
>>>> +	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
>>>> +
>>>>  	/* 1 interrupt per received frame */
>>>>  	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
>>>>  
>>>> @@ -1158,8 +1161,6 @@ static int bgmac_open(struct net_device *net_dev)
>>>>  	struct bgmac *bgmac = netdev_priv(net_dev);
>>>>  	int err = 0;
>>>>  
>>>> -	bgmac_chip_reset(bgmac);
>>>> -
>>>
>>> Is this removal intentional? Maybe it should be special cased with
>>> checking for a NS2 BGMAC instance and not do it in that case?
>>
>> The reset seems completely unnecessary.  There is already 2 resets in
>> the probe routine, another reset serves no purpose.  I can add it
>> back, as it does not seem to have the negative effect I was seeing
>> before.
> 
> After repeated ifconfig down/up, does Ethernet still work with this
> reset removed?
> 
> Thanks.
> 

Sorry, I meant to say, after removing both reset calls in the open and
close functions, does Ethernet still work after repeated ifconfig
down/up with data traffic in the background?

I would guess this reset is required, to flush out and clean up the
internal memories and state machine of the AMAC core each time before
you bring up the interface. It might also be required in the close
function, such that after the interface bring down, there's no staled
data coming out of the AMAC core. But I cannot say for sure since I do
not try this out myself.

Thanks.

>>
>> Of course, if I remove this one I should remove the reset in the close
>> too (which seems even more unnecessary, but I didn't remove it).
>>
>> Thanks,
>> Jon
>>
>>> -- 
>>> Florian

[PATCH 4/5] net: ethernet: bgmac: add NS2 support

[Ray Jui]

On 10/27/2016 3:21 PM, Ray Jui wrote:
> 
> 
> On 10/27/2016 1:51 PM, Jon Mason wrote:
>> On Wed, Oct 26, 2016 at 02:50:39PM -0700, Florian Fainelli wrote:
>>> On 10/26/2016 12:36 PM, Jon Mason wrote:
>>>> Add support for the variant of amac hardware present in the Broadcom
>>>> Northstar2 based SoCs.  Northstar2 requires an additional register to be
>>>> configured with the port speed/duplexity (NICPM).  This can be added to
>>>> the link callback to hide it from the instances that do not use this.
>>>> Also, the bgmac_chip_reset() was intentionally removed to prevent the
>>>> resetting of the chip to the default values on open.  Finally, clearing
>>>> of the pending interrupts on init is required due to observed issues on
>>>> some platforms.
>>>>
>>>> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>>>> ---
>>>
>>>> +static void bgmac_nicpm_speed_set(struct net_device *net_dev)
>>>> +{
>>>> +	struct bgmac *bgmac = netdev_priv(net_dev);
>>>> +	u32 val;
>>>> +
>>>> +	if (!bgmac->plat.nicpm_base)
>>>> +		return;
>>>> +
>>>> +	val = NICPM_IOMUX_CTRL_INIT_VAL;
>>>> +	switch (bgmac->net_dev->phydev->speed) {
>>>> +	default:
>>>> +		pr_err("Unsupported speed.  Defaulting to 1000Mb\n");
>>>> +	case SPEED_1000:
>>>> +		val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>>> +		break;
>>>> +	case SPEED_100:
>>>> +		val |= NICPM_IOMUX_CTRL_SPD_100M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>>> +		break;
>>>> +	case SPEED_10:
>>>> +		val |= NICPM_IOMUX_CTRL_SPD_10M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>>> +		break;
>>>> +	}
>>>> +
>>>> +	writel(val, bgmac->plat.nicpm_base + NICPM_IOMUX_CTRL);
>>>> +
>>>> +	usleep_range(10, 100);
>>>
>>> Does not seem like a good idea, do you need that sleep?
>>
>> Oops, that's not supposed to be there.  Removed.
>>
>>
>>>> +
>>>> +	bgmac_adjust_link(bgmac->net_dev);
>>>> +}
>>>> +
>>>>  static int platform_phy_connect(struct bgmac *bgmac)
>>>>  {
>>>>  	struct phy_device *phy_dev;
>>>>  
>>>> -	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
>>>> -					 bgmac_adjust_link);
>>>> +	if (bgmac->plat.nicpm_base)
>>>> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
>>>> +						 bgmac->dev->of_node,
>>>> +						 bgmac_nicpm_speed_set);
>>>> +	else
>>>> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
>>>> +						 bgmac->dev->of_node,
>>>> +						 bgmac_adjust_link);
>>>>  	if (!phy_dev) {
>>>>  		dev_err(bgmac->dev, "Phy connect failed\n");
>>>>  		return -ENODEV;
>>>>  	}
>>>>  
>>>> +	if (bgmac->feature_flags & BGMAC_FEAT_LANE_SWAP)
>>>> +		phy_dev->dev_flags |= PHY_BRCM_EXP_LANE_SWAP;
>>>> +
>>>>  	return 0;
>>>>  }
>>>>  
>>>> @@ -140,6 +188,9 @@ static int bgmac_probe(struct platform_device *pdev)
>>>>  
>>>>  	platform_set_drvdata(pdev, bgmac);
>>>>  
>>>> +	if (of_property_read_bool(np, "brcm,enet-phy-lane-swap"))
>>>> +		bgmac->feature_flags |= BGMAC_FEAT_LANE_SWAP;
>>>> +
>>>>  	/* Set the features of the 4707 family */
>>>>  	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
>>>>  	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
>>>> @@ -182,6 +233,14 @@ static int bgmac_probe(struct platform_device *pdev)
>>>>  	if (IS_ERR(bgmac->plat.idm_base))
>>>>  		return PTR_ERR(bgmac->plat.idm_base);
>>>>  
>>>> +	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base");
>>>> +	if (regs) {
>>>> +		bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev,
>>>> +							       regs);
>>>> +		if (IS_ERR(bgmac->plat.nicpm_base))
>>>> +			return PTR_ERR(bgmac->plat.nicpm_base);
>>>> +	}
>>>> +
>>>>  	bgmac->read = platform_bgmac_read;
>>>>  	bgmac->write = platform_bgmac_write;
>>>>  	bgmac->idm_read = platform_bgmac_idm_read;
>>>> @@ -213,6 +272,7 @@ static int bgmac_remove(struct platform_device *pdev)
>>>>  static const struct of_device_id bgmac_of_enet_match[] = {
>>>>  	{.compatible = "brcm,amac",},
>>>>  	{.compatible = "brcm,nsp-amac",},
>>>> +	{.compatible = "brcm,ns2-amac",},
>>>>  	{},
>>>>  };
>>>>  
>>>> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
>>>> index 38876ec..1796208 100644
>>>> --- a/drivers/net/ethernet/broadcom/bgmac.c
>>>> +++ b/drivers/net/ethernet/broadcom/bgmac.c
>>>> @@ -1082,6 +1082,9 @@ static void bgmac_enable(struct bgmac *bgmac)
>>>>  /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
>>>>  static void bgmac_chip_init(struct bgmac *bgmac)
>>>>  {
>>>> +	/* Clear any erroneously pending interrupts */
>>>> +	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
>>>> +
>>>>  	/* 1 interrupt per received frame */
>>>>  	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
>>>>  
>>>> @@ -1158,8 +1161,6 @@ static int bgmac_open(struct net_device *net_dev)
>>>>  	struct bgmac *bgmac = netdev_priv(net_dev);
>>>>  	int err = 0;
>>>>  
>>>> -	bgmac_chip_reset(bgmac);
>>>> -
>>>
>>> Is this removal intentional? Maybe it should be special cased with
>>> checking for a NS2 BGMAC instance and not do it in that case?
>>
>> The reset seems completely unnecessary.  There is already 2 resets in
>> the probe routine, another reset serves no purpose.  I can add it
>> back, as it does not seem to have the negative effect I was seeing
>> before.
> 
> After repeated ifconfig down/up, does Ethernet still work with this
> reset removed?
> 
> Thanks.
> 

Sorry, I meant to say, after removing both reset calls in the open and
close functions, does Ethernet still work after repeated ifconfig
down/up with data traffic in the background?

I would guess this reset is required, to flush out and clean up the
internal memories and state machine of the AMAC core each time before
you bring up the interface. It might also be required in the close
function, such that after the interface bring down, there's no staled
data coming out of the AMAC core. But I cannot say for sure since I do
not try this out myself.

Thanks.

>>
>> Of course, if I remove this one I should remove the reset in the close
>> too (which seems even more unnecessary, but I didn't remove it).
>>
>> Thanks,
>> Jon
>>
>>> -- 
>>> Florian

Re: [PATCH 4/5] net: ethernet: bgmac: add NS2 support

[Ray Jui]

On 10/27/2016 3:21 PM, Ray Jui wrote:
> 
> 
> On 10/27/2016 1:51 PM, Jon Mason wrote:
>> On Wed, Oct 26, 2016 at 02:50:39PM -0700, Florian Fainelli wrote:
>>> On 10/26/2016 12:36 PM, Jon Mason wrote:
>>>> Add support for the variant of amac hardware present in the Broadcom
>>>> Northstar2 based SoCs.  Northstar2 requires an additional register to be
>>>> configured with the port speed/duplexity (NICPM).  This can be added to
>>>> the link callback to hide it from the instances that do not use this.
>>>> Also, the bgmac_chip_reset() was intentionally removed to prevent the
>>>> resetting of the chip to the default values on open.  Finally, clearing
>>>> of the pending interrupts on init is required due to observed issues on
>>>> some platforms.
>>>>
>>>> Signed-off-by: Jon Mason <jon.mason-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>>> ---
>>>
>>>> +static void bgmac_nicpm_speed_set(struct net_device *net_dev)
>>>> +{
>>>> +	struct bgmac *bgmac = netdev_priv(net_dev);
>>>> +	u32 val;
>>>> +
>>>> +	if (!bgmac->plat.nicpm_base)
>>>> +		return;
>>>> +
>>>> +	val = NICPM_IOMUX_CTRL_INIT_VAL;
>>>> +	switch (bgmac->net_dev->phydev->speed) {
>>>> +	default:
>>>> +		pr_err("Unsupported speed.  Defaulting to 1000Mb\n");
>>>> +	case SPEED_1000:
>>>> +		val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>>> +		break;
>>>> +	case SPEED_100:
>>>> +		val |= NICPM_IOMUX_CTRL_SPD_100M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>>> +		break;
>>>> +	case SPEED_10:
>>>> +		val |= NICPM_IOMUX_CTRL_SPD_10M << NICPM_IOMUX_CTRL_SPD_SHIFT;
>>>> +		break;
>>>> +	}
>>>> +
>>>> +	writel(val, bgmac->plat.nicpm_base + NICPM_IOMUX_CTRL);
>>>> +
>>>> +	usleep_range(10, 100);
>>>
>>> Does not seem like a good idea, do you need that sleep?
>>
>> Oops, that's not supposed to be there.  Removed.
>>
>>
>>>> +
>>>> +	bgmac_adjust_link(bgmac->net_dev);
>>>> +}
>>>> +
>>>>  static int platform_phy_connect(struct bgmac *bgmac)
>>>>  {
>>>>  	struct phy_device *phy_dev;
>>>>  
>>>> -	phy_dev = of_phy_get_and_connect(bgmac->net_dev, bgmac->dev->of_node,
>>>> -					 bgmac_adjust_link);
>>>> +	if (bgmac->plat.nicpm_base)
>>>> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
>>>> +						 bgmac->dev->of_node,
>>>> +						 bgmac_nicpm_speed_set);
>>>> +	else
>>>> +		phy_dev = of_phy_get_and_connect(bgmac->net_dev,
>>>> +						 bgmac->dev->of_node,
>>>> +						 bgmac_adjust_link);
>>>>  	if (!phy_dev) {
>>>>  		dev_err(bgmac->dev, "Phy connect failed\n");
>>>>  		return -ENODEV;
>>>>  	}
>>>>  
>>>> +	if (bgmac->feature_flags & BGMAC_FEAT_LANE_SWAP)
>>>> +		phy_dev->dev_flags |= PHY_BRCM_EXP_LANE_SWAP;
>>>> +
>>>>  	return 0;
>>>>  }
>>>>  
>>>> @@ -140,6 +188,9 @@ static int bgmac_probe(struct platform_device *pdev)
>>>>  
>>>>  	platform_set_drvdata(pdev, bgmac);
>>>>  
>>>> +	if (of_property_read_bool(np, "brcm,enet-phy-lane-swap"))
>>>> +		bgmac->feature_flags |= BGMAC_FEAT_LANE_SWAP;
>>>> +
>>>>  	/* Set the features of the 4707 family */
>>>>  	bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
>>>>  	bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
>>>> @@ -182,6 +233,14 @@ static int bgmac_probe(struct platform_device *pdev)
>>>>  	if (IS_ERR(bgmac->plat.idm_base))
>>>>  		return PTR_ERR(bgmac->plat.idm_base);
>>>>  
>>>> +	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "nicpm_base");
>>>> +	if (regs) {
>>>> +		bgmac->plat.nicpm_base = devm_ioremap_resource(&pdev->dev,
>>>> +							       regs);
>>>> +		if (IS_ERR(bgmac->plat.nicpm_base))
>>>> +			return PTR_ERR(bgmac->plat.nicpm_base);
>>>> +	}
>>>> +
>>>>  	bgmac->read = platform_bgmac_read;
>>>>  	bgmac->write = platform_bgmac_write;
>>>>  	bgmac->idm_read = platform_bgmac_idm_read;
>>>> @@ -213,6 +272,7 @@ static int bgmac_remove(struct platform_device *pdev)
>>>>  static const struct of_device_id bgmac_of_enet_match[] = {
>>>>  	{.compatible = "brcm,amac",},
>>>>  	{.compatible = "brcm,nsp-amac",},
>>>> +	{.compatible = "brcm,ns2-amac",},
>>>>  	{},
>>>>  };
>>>>  
>>>> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
>>>> index 38876ec..1796208 100644
>>>> --- a/drivers/net/ethernet/broadcom/bgmac.c
>>>> +++ b/drivers/net/ethernet/broadcom/bgmac.c
>>>> @@ -1082,6 +1082,9 @@ static void bgmac_enable(struct bgmac *bgmac)
>>>>  /* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipinit */
>>>>  static void bgmac_chip_init(struct bgmac *bgmac)
>>>>  {
>>>> +	/* Clear any erroneously pending interrupts */
>>>> +	bgmac_write(bgmac, BGMAC_INT_STATUS, ~0);
>>>> +
>>>>  	/* 1 interrupt per received frame */
>>>>  	bgmac_write(bgmac, BGMAC_INT_RECV_LAZY, 1 << BGMAC_IRL_FC_SHIFT);
>>>>  
>>>> @@ -1158,8 +1161,6 @@ static int bgmac_open(struct net_device *net_dev)
>>>>  	struct bgmac *bgmac = netdev_priv(net_dev);
>>>>  	int err = 0;
>>>>  
>>>> -	bgmac_chip_reset(bgmac);
>>>> -
>>>
>>> Is this removal intentional? Maybe it should be special cased with
>>> checking for a NS2 BGMAC instance and not do it in that case?
>>
>> The reset seems completely unnecessary.  There is already 2 resets in
>> the probe routine, another reset serves no purpose.  I can add it
>> back, as it does not seem to have the negative effect I was seeing
>> before.
> 
> After repeated ifconfig down/up, does Ethernet still work with this
> reset removed?
> 
> Thanks.
> 

Sorry, I meant to say, after removing both reset calls in the open and
close functions, does Ethernet still work after repeated ifconfig
down/up with data traffic in the background?

I would guess this reset is required, to flush out and clean up the
internal memories and state machine of the AMAC core each time before
you bring up the interface. It might also be required in the close
function, such that after the interface bring down, there's no staled
data coming out of the AMAC core. But I cannot say for sure since I do
not try this out myself.

Thanks.

>>
>> Of course, if I remove this one I should remove the reset in the close
>> too (which seems even more unnecessary, but I didn't remove it).
>>
>> Thanks,
>> Jon
>>
>>> -- 
>>> Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] arm64: dts: NS2: add AMAC ethernet support

[Jon Mason]

Add support for the AMAC ethernet to the Broadcom Northstar2 SoC device
tree

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 arch/arm64/boot/dts/broadcom/ns2-svk.dts |  5 +++++
 arch/arm64/boot/dts/broadcom/ns2.dtsi    | 12 ++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/broadcom/ns2-svk.dts b/arch/arm64/boot/dts/broadcom/ns2-svk.dts
index 2d7872a..362db7e 100644
--- a/arch/arm64/boot/dts/broadcom/ns2-svk.dts
+++ b/arch/arm64/boot/dts/broadcom/ns2-svk.dts
@@ -56,6 +56,11 @@
 	};
 };
 
+&enet {
+	brcm,enet-phy-lane-swap;
+	status = "ok";
+};
+
 &pci_phy0 {
 	status = "ok";
 };
diff --git a/arch/arm64/boot/dts/broadcom/ns2.dtsi b/arch/arm64/boot/dts/broadcom/ns2.dtsi
index d95dc40..773ed59 100644
--- a/arch/arm64/boot/dts/broadcom/ns2.dtsi
+++ b/arch/arm64/boot/dts/broadcom/ns2.dtsi
@@ -191,6 +191,18 @@
 
 		#include "ns2-clock.dtsi"
 
+		enet: ethernet@61000000 {
+			compatible = "brcm,ns2-amac";
+			reg = <0x61000000 0x1000>,
+			      <0x61090000 0x1000>,
+			      <0x61030000 0x100>;
+			reg-names = "amac_base", "idm_base", "nicpm_base";
+			interrupts = <GIC_SPI 341 IRQ_TYPE_LEVEL_HIGH>;
+			phy-handle = <&gphy0>;
+			phy-mode = "rgmii";
+			status = "disabled";
+		};
+
 		dma0: dma@61360000 {
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0x61360000 0x1000>;
-- 
2.7.4

[PATCH 5/5] arm64: dts: NS2: add AMAC ethernet support

[Jon Mason]

Add support for the AMAC ethernet to the Broadcom Northstar2 SoC device
tree

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 arch/arm64/boot/dts/broadcom/ns2-svk.dts |  5 +++++
 arch/arm64/boot/dts/broadcom/ns2.dtsi    | 12 ++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/broadcom/ns2-svk.dts b/arch/arm64/boot/dts/broadcom/ns2-svk.dts
index 2d7872a..362db7e 100644
--- a/arch/arm64/boot/dts/broadcom/ns2-svk.dts
+++ b/arch/arm64/boot/dts/broadcom/ns2-svk.dts
@@ -56,6 +56,11 @@
 	};
 };
 
+&enet {
+	brcm,enet-phy-lane-swap;
+	status = "ok";
+};
+
 &pci_phy0 {
 	status = "ok";
 };
diff --git a/arch/arm64/boot/dts/broadcom/ns2.dtsi b/arch/arm64/boot/dts/broadcom/ns2.dtsi
index d95dc40..773ed59 100644
--- a/arch/arm64/boot/dts/broadcom/ns2.dtsi
+++ b/arch/arm64/boot/dts/broadcom/ns2.dtsi
@@ -191,6 +191,18 @@
 
 		#include "ns2-clock.dtsi"
 
+		enet: ethernet at 61000000 {
+			compatible = "brcm,ns2-amac";
+			reg = <0x61000000 0x1000>,
+			      <0x61090000 0x1000>,
+			      <0x61030000 0x100>;
+			reg-names = "amac_base", "idm_base", "nicpm_base";
+			interrupts = <GIC_SPI 341 IRQ_TYPE_LEVEL_HIGH>;
+			phy-handle = <&gphy0>;
+			phy-mode = "rgmii";
+			status = "disabled";
+		};
+
 		dma0: dma at 61360000 {
 			compatible = "arm,pl330", "arm,primecell";
 			reg = <0x61360000 0x1000>;
-- 
2.7.4