Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1430874 > unrolled thread

[PATCH v2 00/15] drivers: net: cpsw: improve runtime pm

Started byGrygorii Strashko <grygorii.strashko@ti.com>
First post2016-06-24 20:30 +0200
Last post2016-06-28 15:00 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v2 00/15] drivers: net: cpsw: improve runtime pm Grygorii Strashko <grygorii.strashko@ti.com> - 2016-06-24 20:30 +0200
    [PATCH v2 05/15] drivers: net: cpsw: ndev: fix accessing to suspended device Grygorii Strashko <grygorii.strashko@ti.com> - 2016-06-24 20:30 +0200
    [PATCH v2 04/15] drivers: net: cpsw: ethtool: fix accessing to suspended device Grygorii Strashko <grygorii.strashko@ti.com> - 2016-06-24 20:30 +0200
    [PATCH v2 06/15] drivers: net: davinci_mdio: do pm runtime initialization later in probe Grygorii Strashko <grygorii.strashko@ti.com> - 2016-06-24 20:30 +0200
    Re: [PATCH v2 00/15] drivers: net: cpsw: improve runtime pm Mugunthan V N <mugunthanvnm@ti.com> - 2016-06-27 11:20 +0200
    Re: [PATCH v2 00/15] drivers: net: cpsw: improve runtime pm David Miller <davem@davemloft.net> - 2016-06-28 15:00 +0200

#1430874 — [PATCH v2 00/15] drivers: net: cpsw: improve runtime pm

FromGrygorii Strashko <grygorii.strashko@ti.com>
Date2016-06-24 20:30 +0200
Subject[PATCH v2 00/15] drivers: net: cpsw: improve runtime pm
Message-ID<rNE1z-3iA-3@gated-at.bofh.it>
This series intended to improve runtime PM and allow CPSW to be
RPM suspended when all ethX netdevices are down.

To achieve above goal it is required to relax runtime PM constraints for
Davinci MDIO which blocks CPSW runtime PM now, because Davinci MDIO is always
powered on during probe and powered off only when it's going to be removed.
- Patches 6-11 implement PM runtime autosuspend for Davinci MDIO, but keep it
disabled by default, because Davinci MDIO is integrated in big set of TI devices
and not all of them verified to work correctly with RPM autosuspend enabled:
 expected to work on SoCs where MDIO is defined as part of CPSW in DT
 (cpsw.c DRA7/am57x, am437x, am335x)
The CPSW need to be fixed before RPM suspended can be allowed:
 - Patches 1-5 ensure that CPSW will not cause L3 errors while it is in RPM
   suspended state.

Davinci MDIO RPM autosuspend can be enabled through sysfs:
 echo 100 > /sys/devices/../48484000.ethernet/48485000.mdio/power/autosuspend_delay_ms

Patches 12 - 15: introduce new compatible string "ti,cpsw-mdio" which is used
then to enable RPM for am335x/am437x/dra7 SoCs.

Tested on am335x, am437x, am572x and k2g (on k2g with RPM disabled for Davinci MDIO)
These changes should not affect on errata i877 implementation on DRA7.

Power measurement on am335x GP EVM:
 Without this series:  547.60 mW total SoC power
 With this series + "ifconfig eth0 down": 477.32 mW Total Soc Power

Changes in v2:
- CPSW ethtool interface updated to use .begin()/.complete() callbacks
- kbuild failure fixed
- davinci_mdio DT updated with proper description of allowed compatible strings
  combinations

Link on v1:
 https://lkml.org/lkml/2016/6/15/362

Grygorii Strashko (15):
  drivers: net: cpsw: fix suspend when all ethX devices are down
  drivers: net: cpsw: check return code from pm runtime calls
  drivers: net: cpsw: remove pm runtime calls from suspend callbacks
  drivers: net: cpsw: ethtool: fix accessing to suspended device
  drivers: net: cpsw: ndev: fix accessing to suspended device
  drivers: net: davinci_mdio: do pm runtime initialization later in probe
  drivers: net: davinci_mdio: remove pm runtime calls from suspend callbacks
  drivers: net: davinci_mdio: drop suspended and lock fields from mdio_data
  drivers: net: davinci_mdio: split reset function on init_clk and enable
  drivers: net: davinci_mdio: add pm runtime callbacks
  drivers: net: davinci_mdio: implement pm runtime auto mode
  net: davinci_mdio: document missed "ti,am4372-mdio" compat string
  net: davinci_mdio: introduce "ti,cpsw-mdio" compat string
  drivers: net: davinci_mdio: enable pm runtime auto for ti cpsw-mdio
  ARM: dts: am335x/am437x/dra7: use new "ti,cpsw-mdio" compat string

 .../devicetree/bindings/net/davinci-mdio.txt       |   5 +-
 arch/arm/boot/dts/am33xx.dtsi                      |   2 +-
 arch/arm/boot/dts/am4372.dtsi                      |   2 +-
 arch/arm/boot/dts/dra7.dtsi                        |   2 +-
 drivers/net/ethernet/ti/cpsw.c                     |  79 ++++++++--
 drivers/net/ethernet/ti/davinci_mdio.c             | 169 +++++++++++++--------
 6 files changed, 182 insertions(+), 77 deletions(-)

-- 
2.9.0

[toc] | [next] | [standalone]


#1430875 — [PATCH v2 05/15] drivers: net: cpsw: ndev: fix accessing to suspended device

FromGrygorii Strashko <grygorii.strashko@ti.com>
Date2016-06-24 20:30 +0200
Subject[PATCH v2 05/15] drivers: net: cpsw: ndev: fix accessing to suspended device
Message-ID<rNE1A-3iA-47@gated-at.bofh.it>
In reply to#1430874
The CPSW might be suspended by RPM if all ethX interfaces are down,
but it still could be accesible through net_device_ops interfce. In
this case net_device_ops operations requiring registers access will
cause L3 errors and CPSW crash.

Hence, fix it by adding RPM get/put calls in net_device_ops callbacks
which need to access CPSW registers: .ndo_set_mac_address(),
.ndo_vlan_rx_add_vid(), .ndo_vlan_rx_kill_vid().

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/cpsw.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 5fea986..33f9957 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1616,10 +1616,17 @@ static int cpsw_ndo_set_mac_address(struct net_device *ndev, void *p)
 	struct sockaddr *addr = (struct sockaddr *)p;
 	int flags = 0;
 	u16 vid = 0;
+	int ret;
 
 	if (!is_valid_ether_addr(addr->sa_data))
 		return -EADDRNOTAVAIL;
 
+	ret = pm_runtime_get_sync(&priv->pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&priv->pdev->dev);
+		return ret;
+	}
+
 	if (priv->data.dual_emac) {
 		vid = priv->slaves[priv->emac_port].port_vlan;
 		flags = ALE_VLAN;
@@ -1634,6 +1641,8 @@ static int cpsw_ndo_set_mac_address(struct net_device *ndev, void *p)
 	memcpy(ndev->dev_addr, priv->mac_addr, ETH_ALEN);
 	for_each_slave(priv, cpsw_set_slave_mac, priv);
 
+	pm_runtime_put(&priv->pdev->dev);
+
 	return 0;
 }
 
@@ -1698,10 +1707,17 @@ static int cpsw_ndo_vlan_rx_add_vid(struct net_device *ndev,
 				    __be16 proto, u16 vid)
 {
 	struct cpsw_priv *priv = netdev_priv(ndev);
+	int ret;
 
 	if (vid == priv->data.default_vlan)
 		return 0;
 
+	ret = pm_runtime_get_sync(&priv->pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&priv->pdev->dev);
+		return ret;
+	}
+
 	if (priv->data.dual_emac) {
 		/* In dual EMAC, reserved VLAN id should not be used for
 		 * creating VLAN interfaces as this can break the dual
@@ -1716,7 +1732,10 @@ static int cpsw_ndo_vlan_rx_add_vid(struct net_device *ndev,
 	}
 
 	dev_info(priv->dev, "Adding vlanid %d to vlan filter\n", vid);
-	return cpsw_add_vlan_ale_entry(priv, vid);
+	ret = cpsw_add_vlan_ale_entry(priv, vid);
+
+	pm_runtime_put(&priv->pdev->dev);
+	return ret;
 }
 
 static int cpsw_ndo_vlan_rx_kill_vid(struct net_device *ndev,
@@ -1728,6 +1747,12 @@ static int cpsw_ndo_vlan_rx_kill_vid(struct net_device *ndev,
 	if (vid == priv->data.default_vlan)
 		return 0;
 
+	ret = pm_runtime_get_sync(&priv->pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&priv->pdev->dev);
+		return ret;
+	}
+
 	if (priv->data.dual_emac) {
 		int i;
 
@@ -1747,8 +1772,10 @@ static int cpsw_ndo_vlan_rx_kill_vid(struct net_device *ndev,
 	if (ret != 0)
 		return ret;
 
-	return cpsw_ale_del_mcast(priv->ale, priv->ndev->broadcast,
-				  0, ALE_VLAN, vid);
+	ret = cpsw_ale_del_mcast(priv->ale, priv->ndev->broadcast,
+				 0, ALE_VLAN, vid);
+	pm_runtime_put(&priv->pdev->dev);
+	return ret;
 }
 
 static const struct net_device_ops cpsw_netdev_ops = {
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]


#1430876 — [PATCH v2 04/15] drivers: net: cpsw: ethtool: fix accessing to suspended device

FromGrygorii Strashko <grygorii.strashko@ti.com>
Date2016-06-24 20:30 +0200
Subject[PATCH v2 04/15] drivers: net: cpsw: ethtool: fix accessing to suspended device
Message-ID<rNE1A-3iA-53@gated-at.bofh.it>
In reply to#1430874
The CPSW might be suspended by RPM if all ethX interfaces are down,
but it still could be accesible through ethtool interfce. In this case
ethtool operations, requiring registers access, will cause L3 errors and
CPSW crash.

ethtool callbcaks which need to access CPSW registers now:
.set_coalesce(), .get_ethtool_stats(), .set_pauseparam(), .get_regs()

Hence, fix it by adding .begin()/.complete() ethtool callbacks, which
will be called before/after each ethtool operation runs, and do CPSW
RPM handling in these callbacks. That way CPSW will be active while
handling ethtool requests.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/cpsw.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index ba81d4e..5fea986 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1907,10 +1907,33 @@ static int cpsw_set_pauseparam(struct net_device *ndev,
 	priv->tx_pause = pause->tx_pause ? true : false;
 
 	for_each_slave(priv, _cpsw_adjust_link, priv, &link);
-
 	return 0;
 }
 
+static int cpsw_ethtool_op_begin(struct net_device *ndev)
+{
+	struct cpsw_priv *priv = netdev_priv(ndev);
+	int ret;
+
+	ret = pm_runtime_get_sync(&priv->pdev->dev);
+	if (ret < 0) {
+		cpsw_err(priv, drv, "ethtool begin failed %d\n", ret);
+		pm_runtime_put_noidle(&priv->pdev->dev);
+	}
+
+	return ret;
+}
+
+static void cpsw_ethtool_op_complete(struct net_device *ndev)
+{
+	struct cpsw_priv *priv = netdev_priv(ndev);
+	int ret;
+
+	ret = pm_runtime_put(&priv->pdev->dev);
+	if (ret < 0)
+		cpsw_err(priv, drv, "ethtool complete failed %d\n", ret);
+}
+
 static const struct ethtool_ops cpsw_ethtool_ops = {
 	.get_drvinfo	= cpsw_get_drvinfo,
 	.get_msglevel	= cpsw_get_msglevel,
@@ -1930,6 +1953,8 @@ static const struct ethtool_ops cpsw_ethtool_ops = {
 	.set_wol	= cpsw_set_wol,
 	.get_regs_len	= cpsw_get_regs_len,
 	.get_regs	= cpsw_get_regs,
+	.begin		= cpsw_ethtool_op_begin,
+	.complete	= cpsw_ethtool_op_complete,
 };
 
 static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv,
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]


#1430877 — [PATCH v2 06/15] drivers: net: davinci_mdio: do pm runtime initialization later in probe

FromGrygorii Strashko <grygorii.strashko@ti.com>
Date2016-06-24 20:30 +0200
Subject[PATCH v2 06/15] drivers: net: davinci_mdio: do pm runtime initialization later in probe
Message-ID<rNE1A-3iA-51@gated-at.bofh.it>
In reply to#1430874
Do PM runtime initialization later in probe - this allows to simplify
error handling a bit.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/ti/davinci_mdio.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 4e7c9b9..2e19dd1 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -356,14 +356,10 @@ static int davinci_mdio_probe(struct platform_device *pdev)
 	data->bus->parent	= dev;
 	data->bus->priv		= data;
 
-	pm_runtime_enable(&pdev->dev);
-	pm_runtime_get_sync(&pdev->dev);
 	data->clk = devm_clk_get(dev, "fck");
 	if (IS_ERR(data->clk)) {
 		dev_err(dev, "failed to get device clock\n");
-		ret = PTR_ERR(data->clk);
-		data->clk = NULL;
-		goto bail_out;
+		return PTR_ERR(data->clk);
 	}
 
 	dev_set_drvdata(dev, data);
@@ -372,10 +368,11 @@ static int davinci_mdio_probe(struct platform_device *pdev)
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	data->regs = devm_ioremap_resource(dev, res);
-	if (IS_ERR(data->regs)) {
-		ret = PTR_ERR(data->regs);
-		goto bail_out;
-	}
+	if (IS_ERR(data->regs))
+		return PTR_ERR(data->regs);
+
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_get_sync(&pdev->dev);
 
 	/* register the mii bus
 	 * Create PHYs from DT only in case if PHY child nodes are explicitly
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]


#1431851

FromMugunthan V N <mugunthanvnm@ti.com>
Date2016-06-27 11:20 +0200
Message-ID<rOARY-6GU-17@gated-at.bofh.it>
In reply to#1430874
On Friday 24 June 2016 11:53 PM, Grygorii Strashko wrote:
> This series intended to improve runtime PM and allow CPSW to be
> RPM suspended when all ethX netdevices are down.
> 
> To achieve above goal it is required to relax runtime PM constraints for
> Davinci MDIO which blocks CPSW runtime PM now, because Davinci MDIO is always
> powered on during probe and powered off only when it's going to be removed.
> - Patches 6-11 implement PM runtime autosuspend for Davinci MDIO, but keep it
> disabled by default, because Davinci MDIO is integrated in big set of TI devices
> and not all of them verified to work correctly with RPM autosuspend enabled:
>  expected to work on SoCs where MDIO is defined as part of CPSW in DT
>  (cpsw.c DRA7/am57x, am437x, am335x)
> The CPSW need to be fixed before RPM suspended can be allowed:
>  - Patches 1-5 ensure that CPSW will not cause L3 errors while it is in RPM
>    suspended state.
> 
> Davinci MDIO RPM autosuspend can be enabled through sysfs:
>  echo 100 > /sys/devices/../48484000.ethernet/48485000.mdio/power/autosuspend_delay_ms
> 
> Patches 12 - 15: introduce new compatible string "ti,cpsw-mdio" which is used
> then to enable RPM for am335x/am437x/dra7 SoCs.
> 
> Tested on am335x, am437x, am572x and k2g (on k2g with RPM disabled for Davinci MDIO)
> These changes should not affect on errata i877 implementation on DRA7.
> 
> Power measurement on am335x GP EVM:
>  Without this series:  547.60 mW total SoC power
>  With this series + "ifconfig eth0 down": 477.32 mW Total Soc Power
> 
> Changes in v2:
> - CPSW ethtool interface updated to use .begin()/.complete() callbacks
> - kbuild failure fixed
> - davinci_mdio DT updated with proper description of allowed compatible strings
>   combinations
> 
> Link on v1:
>  https://lkml.org/lkml/2016/6/15/362

For the series.
Reviewed-by: Mugunthan V N <mugunthanvnm@ti.com>

Regards
Mugunthan V N

> 
> Grygorii Strashko (15):
>   drivers: net: cpsw: fix suspend when all ethX devices are down
>   drivers: net: cpsw: check return code from pm runtime calls
>   drivers: net: cpsw: remove pm runtime calls from suspend callbacks
>   drivers: net: cpsw: ethtool: fix accessing to suspended device
>   drivers: net: cpsw: ndev: fix accessing to suspended device
>   drivers: net: davinci_mdio: do pm runtime initialization later in probe
>   drivers: net: davinci_mdio: remove pm runtime calls from suspend callbacks
>   drivers: net: davinci_mdio: drop suspended and lock fields from mdio_data
>   drivers: net: davinci_mdio: split reset function on init_clk and enable
>   drivers: net: davinci_mdio: add pm runtime callbacks
>   drivers: net: davinci_mdio: implement pm runtime auto mode
>   net: davinci_mdio: document missed "ti,am4372-mdio" compat string
>   net: davinci_mdio: introduce "ti,cpsw-mdio" compat string
>   drivers: net: davinci_mdio: enable pm runtime auto for ti cpsw-mdio
>   ARM: dts: am335x/am437x/dra7: use new "ti,cpsw-mdio" compat string
> 
>  .../devicetree/bindings/net/davinci-mdio.txt       |   5 +-
>  arch/arm/boot/dts/am33xx.dtsi                      |   2 +-
>  arch/arm/boot/dts/am4372.dtsi                      |   2 +-
>  arch/arm/boot/dts/dra7.dtsi                        |   2 +-
>  drivers/net/ethernet/ti/cpsw.c                     |  79 ++++++++--
>  drivers/net/ethernet/ti/davinci_mdio.c             | 169 +++++++++++++--------
>  6 files changed, 182 insertions(+), 77 deletions(-)
> 

[toc] | [prev] | [next] | [standalone]


#1432974

FromDavid Miller <davem@davemloft.net>
Date2016-06-28 15:00 +0200
Message-ID<rP0Mq-7aW-19@gated-at.bofh.it>
In reply to#1430874
From: Grygorii Strashko <grygorii.strashko@ti.com>
Date: Fri, 24 Jun 2016 21:23:40 +0300

> This series intended to improve runtime PM and allow CPSW to be
> RPM suspended when all ethX netdevices are down.

Series applied, thanks.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web