[PATCH net-next v2 0/3] net: ethernet: bgmac: PM support and clean-ups

[PATCH net-next v2 0/3] net: ethernet: bgmac: PM support and clean-ups

[Jon Mason]

Changes in v2:
* Reworked the PM patch with Florian's suggestions


Add code to support Power Management (only tested on NS2), and add some
code clean-ups

Joey Zhong (1):
  net: ethernet: bgmac: driver power manangement

Jon Mason (2):
  net: ethernet: bgmac: use #defines for MAX size
  net: ethernet: bgmac: unify code of the same family

 drivers/net/ethernet/broadcom/bgmac-bcma.c     | 64 +++++++++++---------------
 drivers/net/ethernet/broadcom/bgmac-platform.c | 34 ++++++++++++++
 drivers/net/ethernet/broadcom/bgmac.c          | 51 ++++++++++++++++++++
 drivers/net/ethernet/broadcom/bgmac.h          |  4 +-
 4 files changed, 116 insertions(+), 37 deletions(-)

-- 
2.7.4

[PATCH net-next v2 1/3] net: ethernet: bgmac: use #defines for MAX size

[Jon Mason]

The maximum frame size is really just the standard ethernet frame size
and FCS.  So use those existing defines to make the code a little more
beautiful.

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 6d0b5b3..5a518fe 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -402,7 +402,7 @@
 
 #define BGMAC_WEIGHT	64
 
-#define ETHER_MAX_LEN   1518
+#define ETHER_MAX_LEN	(ETH_FRAME_LEN + ETH_FCS_LEN)
 
 /* Feature Flags */
 #define BGMAC_FEAT_TX_MASK_SETUP	BIT(0)
-- 
2.7.4

[PATCH net-next v2 2/3] net: ethernet: bgmac: unify code of the same family

[Jon Mason]

BCM471X and BCM535X are of the same family (from what I can derive from
internal documents).  Group them into the case statement together, which
results in more code reuse.

Also, use existing helper variables to make the code a little more
readable too.

Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac-bcma.c | 64 +++++++++++++-----------------
 1 file changed, 28 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bgmac-bcma.c b/drivers/net/ethernet/broadcom/bgmac-bcma.c
index 5ef60d4..f5c27f4 100644
--- a/drivers/net/ethernet/broadcom/bgmac-bcma.c
+++ b/drivers/net/ethernet/broadcom/bgmac-bcma.c
@@ -192,36 +192,50 @@ static int bgmac_probe(struct bcma_device *core)
 		goto err1;
 	}
 
-	bgmac->has_robosw = !!(core->bus->sprom.boardflags_lo &
-			       BGMAC_BFL_ENETROBO);
+	bgmac->has_robosw = !!(sprom->boardflags_lo & BGMAC_BFL_ENETROBO);
 	if (bgmac->has_robosw)
 		dev_warn(bgmac->dev, "Support for Roboswitch not implemented\n");
 
-	if (core->bus->sprom.boardflags_lo & BGMAC_BFL_ENETADM)
+	if (sprom->boardflags_lo & BGMAC_BFL_ENETADM)
 		dev_warn(bgmac->dev, "Support for ADMtek ethernet switch not implemented\n");
 
 	/* Feature Flags */
-	switch (core->bus->chipinfo.id) {
+	switch (ci->id) {
+	/* BCM 471X/535X family */
+	case BCMA_CHIP_ID_BCM4716:
+		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
+		/* fallthrough */
+	case BCMA_CHIP_ID_BCM47162:
+		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL2;
+		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
+		break;
 	case BCMA_CHIP_ID_BCM5357:
+	case BCMA_CHIP_ID_BCM53572:
 		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
 		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
 		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
 		bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
-		if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM47186) {
-			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
+		if (ci->pkg == BCMA_PKG_ID_BCM47188 ||
+		    ci->pkg == BCMA_PKG_ID_BCM47186) {
 			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
+			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
 		}
-		if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM5358)
+		if (ci->pkg == BCMA_PKG_ID_BCM5358)
 			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_EPHYRMII;
 		break;
-	case BCMA_CHIP_ID_BCM53572:
-		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
+	case BCMA_CHIP_ID_BCM53573:
 		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
-		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
-		bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
-		if (core->bus->chipinfo.pkg == BCMA_PKG_ID_BCM47188) {
-			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
+		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
+		if (ci->pkg == BCMA_PKG_ID_BCM47189)
 			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
+		if (core->core_unit == 0) {
+			bgmac->feature_flags |= BGMAC_FEAT_CC4_IF_SW_TYPE;
+			if (ci->pkg == BCMA_PKG_ID_BCM47189)
+				bgmac->feature_flags |=
+					BGMAC_FEAT_CC4_IF_SW_TYPE_RGMII;
+		} else if (core->core_unit == 1) {
+			bgmac->feature_flags |= BGMAC_FEAT_IRQ_ID_OOB_6;
+			bgmac->feature_flags |= BGMAC_FEAT_CC7_IF_TYPE_RGMII;
 		}
 		break;
 	case BCMA_CHIP_ID_BCM4749:
@@ -229,18 +243,11 @@ static int bgmac_probe(struct bcma_device *core)
 		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
 		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL1;
 		bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_PHY;
-		if (core->bus->chipinfo.pkg == 10) {
+		if (ci->pkg == 10) {
 			bgmac->feature_flags |= BGMAC_FEAT_SW_TYPE_RGMII;
 			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
 		}
 		break;
-	case BCMA_CHIP_ID_BCM4716:
-		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
-		/* fallthrough */
-	case BCMA_CHIP_ID_BCM47162:
-		bgmac->feature_flags |= BGMAC_FEAT_FLW_CTRL2;
-		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
-		break;
 	/* bcm4707_family */
 	case BCMA_CHIP_ID_BCM4707:
 	case BCMA_CHIP_ID_BCM47094:
@@ -249,21 +256,6 @@ static int bgmac_probe(struct bcma_device *core)
 		bgmac->feature_flags |= BGMAC_FEAT_NO_RESET;
 		bgmac->feature_flags |= BGMAC_FEAT_FORCE_SPEED_2500;
 		break;
-	case BCMA_CHIP_ID_BCM53573:
-		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
-		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
-		if (ci->pkg == BCMA_PKG_ID_BCM47189)
-			bgmac->feature_flags |= BGMAC_FEAT_IOST_ATTACHED;
-		if (core->core_unit == 0) {
-			bgmac->feature_flags |= BGMAC_FEAT_CC4_IF_SW_TYPE;
-			if (ci->pkg == BCMA_PKG_ID_BCM47189)
-				bgmac->feature_flags |=
-					BGMAC_FEAT_CC4_IF_SW_TYPE_RGMII;
-		} else if (core->core_unit == 1) {
-			bgmac->feature_flags |= BGMAC_FEAT_IRQ_ID_OOB_6;
-			bgmac->feature_flags |= BGMAC_FEAT_CC7_IF_TYPE_RGMII;
-		}
-		break;
 	default:
 		bgmac->feature_flags |= BGMAC_FEAT_CLKCTLST;
 		bgmac->feature_flags |= BGMAC_FEAT_SET_RXQ_CLK;
-- 
2.7.4

[PATCH net-next v2 3/3] net: ethernet: bgmac: driver power manangement

[Jon Mason]

From: Joey Zhong <zhongx@broadcom.com>

Implement suspend/resume callbacks in the bgmac driver. This makes sure
that we de-initialize and re-initialize the hardware correctly before
entering suspend and when resuming.

Signed-off-by: Joey Zhong <zhongx@broadcom.com>
Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
 drivers/net/ethernet/broadcom/bgmac-platform.c | 34 +++++++++++++++++
 drivers/net/ethernet/broadcom/bgmac.c          | 51 ++++++++++++++++++++++++++
 drivers/net/ethernet/broadcom/bgmac.h          |  2 +
 3 files changed, 87 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c
index 2d153f7..3df91e7 100644
--- a/drivers/net/ethernet/broadcom/bgmac-platform.c
+++ b/drivers/net/ethernet/broadcom/bgmac-platform.c
@@ -21,8 +21,12 @@
 #include <linux/of_net.h>
 #include "bgmac.h"
 
+#define NICPM_PADRING_CFG		0x00000004
 #define NICPM_IOMUX_CTRL		0x00000008
 
+#define NICPM_PADRING_CFG_INIT_VAL	0x74000000
+#define NICPM_IOMUX_CTRL_INIT_VAL_AX	0x21880000
+
 #define NICPM_IOMUX_CTRL_INIT_VAL	0x3196e000
 #define NICPM_IOMUX_CTRL_SPD_SHIFT	10
 #define NICPM_IOMUX_CTRL_SPD_10M	0
@@ -108,6 +112,10 @@ static void bgmac_nicpm_speed_set(struct net_device *net_dev)
 	if (!bgmac->plat.nicpm_base)
 		return;
 
+	/* SET RGMII IO CONFIG */
+	writel(NICPM_PADRING_CFG_INIT_VAL,
+	       bgmac->plat.nicpm_base + NICPM_PADRING_CFG);
+
 	val = NICPM_IOMUX_CTRL_INIT_VAL;
 	switch (bgmac->net_dev->phydev->speed) {
 	default:
@@ -239,6 +247,31 @@ static int bgmac_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int bgmac_suspend(struct device *dev)
+{
+	struct bgmac *bgmac = dev_get_drvdata(dev);
+
+	return bgmac_enet_suspend(bgmac);
+}
+
+static int bgmac_resume(struct device *dev)
+{
+	struct bgmac *bgmac = dev_get_drvdata(dev);
+
+	return bgmac_enet_resume(bgmac);
+}
+
+static const struct dev_pm_ops bgmac_pm_ops = {
+	.suspend = bgmac_suspend,
+	.resume = bgmac_resume
+};
+
+#define BGMAC_PM_OPS (&bgmac_pm_ops)
+#else
+#define BGMAC_PM_OPS NULL
+#endif /* CONFIG_PM */
+
 static const struct of_device_id bgmac_of_enet_match[] = {
 	{.compatible = "brcm,amac",},
 	{.compatible = "brcm,nsp-amac",},
@@ -252,6 +285,7 @@ static struct platform_driver bgmac_enet_driver = {
 	.driver = {
 		.name  = "bgmac-enet",
 		.of_match_table = bgmac_of_enet_match,
+		.pm = BGMAC_PM_OPS
 	},
 	.probe = bgmac_probe,
 	.remove = bgmac_remove,
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index bd549f8..e78c91d 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1478,6 +1478,7 @@ int bgmac_enet_probe(struct bgmac *bgmac)
 
 	net_dev->irq = bgmac->irq;
 	SET_NETDEV_DEV(net_dev, bgmac->dev);
+	dev_set_drvdata(bgmac->dev, bgmac);
 
 	if (!is_valid_ether_addr(bgmac->mac_addr)) {
 		dev_err(bgmac->dev, "Invalid MAC addr: %pM\n",
@@ -1551,5 +1552,55 @@ void bgmac_enet_remove(struct bgmac *bgmac)
 }
 EXPORT_SYMBOL_GPL(bgmac_enet_remove);
 
+int bgmac_enet_suspend(struct bgmac *bgmac)
+{
+	if (!netif_running(bgmac->net_dev))
+		return 0;
+
+	phy_stop(bgmac->net_dev->phydev);
+
+	netif_stop_queue(bgmac->net_dev);
+
+	napi_disable(&bgmac->napi);
+
+	netif_tx_lock(bgmac->net_dev);
+	netif_device_detach(bgmac->net_dev);
+	netif_tx_unlock(bgmac->net_dev);
+
+	bgmac_chip_intrs_off(bgmac);
+	bgmac_chip_reset(bgmac);
+	bgmac_dma_cleanup(bgmac);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(bgmac_enet_suspend);
+
+int bgmac_enet_resume(struct bgmac *bgmac)
+{
+	int rc;
+
+	if (netif_running(bgmac->net_dev))
+		return 0;
+
+	rc = bgmac_dma_init(bgmac);
+	if (rc)
+		return rc;
+
+	bgmac_chip_init(bgmac);
+
+	napi_enable(&bgmac->napi);
+
+	netif_tx_lock(bgmac->net_dev);
+	netif_device_attach(bgmac->net_dev);
+	netif_tx_unlock(bgmac->net_dev);
+
+	netif_start_queue(bgmac->net_dev);
+
+	phy_start(bgmac->net_dev->phydev);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(bgmac_enet_resume);
+
 MODULE_AUTHOR("Rafał Miłecki");
 MODULE_LICENSE("GPL");
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 5a518fe..741ca27 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -538,6 +538,8 @@ int bgmac_enet_probe(struct bgmac *bgmac);
 void bgmac_enet_remove(struct bgmac *bgmac);
 void bgmac_adjust_link(struct net_device *net_dev);
 int bgmac_phy_connect_direct(struct bgmac *bgmac);
+int bgmac_enet_suspend(struct bgmac *bgmac);
+int bgmac_enet_resume(struct bgmac *bgmac);
 
 struct mii_bus *bcma_mdio_mii_register(struct bgmac *bgmac);
 void bcma_mdio_mii_unregister(struct mii_bus *mii_bus);
-- 
2.7.4

Re: [PATCH net-next v2 3/3] net: ethernet: bgmac: driver power manangement

[Florian Fainelli]

On 02/08/2017 01:24 PM, Jon Mason wrote:
> From: Joey Zhong <zhongx@broadcom.com>
> 
> Implement suspend/resume callbacks in the bgmac driver. This makes sure
> that we de-initialize and re-initialize the hardware correctly before
> entering suspend and when resuming.
> 
> Signed-off-by: Joey Zhong <zhongx@broadcom.com>
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---
>  drivers/net/ethernet/broadcom/bgmac-platform.c | 34 +++++++++++++++++
>  drivers/net/ethernet/broadcom/bgmac.c          | 51 ++++++++++++++++++++++++++
>  drivers/net/ethernet/broadcom/bgmac.h          |  2 +
>  3 files changed, 87 insertions(+)
> 
> diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c
> index 2d153f7..3df91e7 100644
> --- a/drivers/net/ethernet/broadcom/bgmac-platform.c
> +++ b/drivers/net/ethernet/broadcom/bgmac-platform.c
> @@ -21,8 +21,12 @@
>  #include <linux/of_net.h>
>  #include "bgmac.h"
>  
> +#define NICPM_PADRING_CFG		0x00000004
>  #define NICPM_IOMUX_CTRL		0x00000008
>  
> +#define NICPM_PADRING_CFG_INIT_VAL	0x74000000
> +#define NICPM_IOMUX_CTRL_INIT_VAL_AX	0x21880000
> +
>  #define NICPM_IOMUX_CTRL_INIT_VAL	0x3196e000
>  #define NICPM_IOMUX_CTRL_SPD_SHIFT	10
>  #define NICPM_IOMUX_CTRL_SPD_10M	0
> @@ -108,6 +112,10 @@ static void bgmac_nicpm_speed_set(struct net_device *net_dev)
>  	if (!bgmac->plat.nicpm_base)
>  		return;
>  
> +	/* SET RGMII IO CONFIG */
> +	writel(NICPM_PADRING_CFG_INIT_VAL,
> +	       bgmac->plat.nicpm_base + NICPM_PADRING_CFG);
> +
>  	val = NICPM_IOMUX_CTRL_INIT_VAL;
>  	switch (bgmac->net_dev->phydev->speed) {
>  	default:
> @@ -239,6 +247,31 @@ static int bgmac_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_PM
> +static int bgmac_suspend(struct device *dev)
> +{
> +	struct bgmac *bgmac = dev_get_drvdata(dev);
> +
> +	return bgmac_enet_suspend(bgmac);
> +}
> +
> +static int bgmac_resume(struct device *dev)
> +{
> +	struct bgmac *bgmac = dev_get_drvdata(dev);
> +
> +	return bgmac_enet_resume(bgmac);
> +}
> +
> +static const struct dev_pm_ops bgmac_pm_ops = {
> +	.suspend = bgmac_suspend,
> +	.resume = bgmac_resume
> +};
> +
> +#define BGMAC_PM_OPS (&bgmac_pm_ops)
> +#else
> +#define BGMAC_PM_OPS NULL
> +#endif /* CONFIG_PM */
> +
>  static const struct of_device_id bgmac_of_enet_match[] = {
>  	{.compatible = "brcm,amac",},
>  	{.compatible = "brcm,nsp-amac",},
> @@ -252,6 +285,7 @@ static struct platform_driver bgmac_enet_driver = {
>  	.driver = {
>  		.name  = "bgmac-enet",
>  		.of_match_table = bgmac_of_enet_match,
> +		.pm = BGMAC_PM_OPS
>  	},
>  	.probe = bgmac_probe,
>  	.remove = bgmac_remove,
> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> index bd549f8..e78c91d 100644
> --- a/drivers/net/ethernet/broadcom/bgmac.c
> +++ b/drivers/net/ethernet/broadcom/bgmac.c
> @@ -1478,6 +1478,7 @@ int bgmac_enet_probe(struct bgmac *bgmac)
>  
>  	net_dev->irq = bgmac->irq;
>  	SET_NETDEV_DEV(net_dev, bgmac->dev);
> +	dev_set_drvdata(bgmac->dev, bgmac);
>  
>  	if (!is_valid_ether_addr(bgmac->mac_addr)) {
>  		dev_err(bgmac->dev, "Invalid MAC addr: %pM\n",
> @@ -1551,5 +1552,55 @@ void bgmac_enet_remove(struct bgmac *bgmac)
>  }
>  EXPORT_SYMBOL_GPL(bgmac_enet_remove);
>  
> +int bgmac_enet_suspend(struct bgmac *bgmac)
> +{
> +	if (!netif_running(bgmac->net_dev))
> +		return 0;
> +
> +	phy_stop(bgmac->net_dev->phydev);
> +
> +	netif_stop_queue(bgmac->net_dev);
> +
> +	napi_disable(&bgmac->napi);
> +
> +	netif_tx_lock(bgmac->net_dev);
> +	netif_device_detach(bgmac->net_dev);
> +	netif_tx_unlock(bgmac->net_dev);
> +
> +	bgmac_chip_intrs_off(bgmac);
> +	bgmac_chip_reset(bgmac);
> +	bgmac_dma_cleanup(bgmac);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(bgmac_enet_suspend);
> +
> +int bgmac_enet_resume(struct bgmac *bgmac)
> +{
> +	int rc;
> +
> +	if (netif_running(bgmac->net_dev))
> +		return 0;

This should be if (!netif_running()) here, if it is running, we need to
do all of what is below.

With that fixed:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

> +
> +	rc = bgmac_dma_init(bgmac);
> +	if (rc)
> +		return rc;
> +
> +	bgmac_chip_init(bgmac);
> +
> +	napi_enable(&bgmac->napi);
> +
> +	netif_tx_lock(bgmac->net_dev);
> +	netif_device_attach(bgmac->net_dev);
> +	netif_tx_unlock(bgmac->net_dev);
> +
> +	netif_start_queue(bgmac->net_dev);
> +
> +	phy_start(bgmac->net_dev->phydev);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(bgmac_enet_resume);
> +
>  MODULE_AUTHOR("Rafał Miłecki");
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
> index 5a518fe..741ca27 100644
> --- a/drivers/net/ethernet/broadcom/bgmac.h
> +++ b/drivers/net/ethernet/broadcom/bgmac.h
> @@ -538,6 +538,8 @@ int bgmac_enet_probe(struct bgmac *bgmac);
>  void bgmac_enet_remove(struct bgmac *bgmac);
>  void bgmac_adjust_link(struct net_device *net_dev);
>  int bgmac_phy_connect_direct(struct bgmac *bgmac);
> +int bgmac_enet_suspend(struct bgmac *bgmac);
> +int bgmac_enet_resume(struct bgmac *bgmac);
>  
>  struct mii_bus *bcma_mdio_mii_register(struct bgmac *bgmac);
>  void bcma_mdio_mii_unregister(struct mii_bus *mii_bus);
> 


-- 
Florian