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

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

[Grygorii Strashko]

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 

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       |   3 +-
 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                     |  88 +++++++++--
 drivers/net/ethernet/ti/davinci_mdio.c             | 169 +++++++++++++--------
 6 files changed, 189 insertions(+), 77 deletions(-)

-- 
2.8.4

[PATCH 01/15] drivers: net: cpsw: fix suspend when all ethX devices are down

[Grygorii Strashko]

The cpsw_suspend() could trigger L3 error and CPSW will stop
functioning if System enters suspend when all ethX net-devices are
down - in this case CPSW could be already suspended by PM runtime, but
cpsw_suspend() will try to call soft_reset_slave() unconditionally
and access CPSW registers.

Hence, fix it by moving soft_reset_slave() from cpsw_suspend() to
cpsw_slave_stop(). This way slave ports will be reset when CPSW is
active and will be in proper state during Suspend.

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

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index e6bb0ec..736c77a 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1244,6 +1244,7 @@ static void cpsw_slave_stop(struct cpsw_slave *slave, struct cpsw_priv *priv)
 	slave->phy = NULL;
 	cpsw_ale_control_set(priv->ale, slave_port,
 			     ALE_PORT_STATE, ALE_PORT_STATE_DISABLE);
+	soft_reset_slave(slave);
 }
 
 static int cpsw_ndo_open(struct net_device *ndev)
@@ -2558,12 +2559,10 @@ static int cpsw_suspend(struct device *dev)
 		for (i = 0; i < priv->data.slaves; i++) {
 			if (netif_running(priv->slaves[i].ndev))
 				cpsw_ndo_stop(priv->slaves[i].ndev);
-			soft_reset_slave(priv->slaves + i);
 		}
 	} else {
 		if (netif_running(ndev))
 			cpsw_ndo_stop(ndev);
-		for_each_slave(priv, soft_reset_slave);
 	}
 
 	pm_runtime_put_sync(&pdev->dev);
-- 
2.8.4

[PATCH 02/15] drivers: net: cpsw: check return code from pm runtime calls

[Grygorii Strashko]

Add missed check of return codes from PM runtime get() calls.

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

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 736c77a..c76f9db 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1253,7 +1253,11 @@ static int cpsw_ndo_open(struct net_device *ndev)
 	int i, ret;
 	u32 reg;
 
-	pm_runtime_get_sync(&priv->pdev->dev);
+	ret = pm_runtime_get_sync(&priv->pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&priv->pdev->dev);
+		return ret;
+	}
 
 	if (!cpsw_common_res_usage_state(priv))
 		cpsw_intr_disable(priv);
@@ -2322,7 +2326,11 @@ static int cpsw_probe(struct platform_device *pdev)
 	/* Need to enable clocks with runtime PM api to access module
 	 * registers
 	 */
-	pm_runtime_get_sync(&pdev->dev);
+	ret = pm_runtime_get_sync(&pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&pdev->dev);
+		goto clean_runtime_disable_ret;
+	}
 	priv->version = readl(&priv->regs->id_ver);
 	pm_runtime_put_sync(&pdev->dev);
 
-- 
2.8.4

[PATCH 03/15] drivers: net: cpsw: remove pm runtime calls from suspend callbacks

[Grygorii Strashko]

PM runtime is properly handled in cpsw_ndo_open/stop(), as result it
isn't required to duplicate these calls in .suspend()/.resume()
callbacks. Moreover, it might cause unnecessary RPM resume of CPSW
during System suspend in the case it's already suspended because
all ethX interfaces are down, before System suspend started.

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

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index c76f9db..ba81d4e 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -2573,8 +2573,6 @@ static int cpsw_suspend(struct device *dev)
 			cpsw_ndo_stop(ndev);
 	}
 
-	pm_runtime_put_sync(&pdev->dev);
-
 	/* Select sleep pin state */
 	pinctrl_pm_select_sleep_state(&pdev->dev);
 
@@ -2587,8 +2585,6 @@ static int cpsw_resume(struct device *dev)
 	struct net_device	*ndev = platform_get_drvdata(pdev);
 	struct cpsw_priv	*priv = netdev_priv(ndev);
 
-	pm_runtime_get_sync(&pdev->dev);
-
 	/* Select default pin state */
 	pinctrl_pm_select_default_state(&pdev->dev);
 
-- 
2.8.4

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

[Grygorii Strashko]

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.

Hence, fix it by adding RPM get/put calls in ethtool callbcaks which
can access CPSW registers: .set_coalesce(), .get_ethtool_stats(),
.set_pauseparam(), .get_regs()

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

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index ba81d4e..1ba0c09 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -931,6 +931,13 @@ static int cpsw_set_coalesce(struct net_device *ndev,
 	u32 prescale = 0;
 	u32 addnl_dvdr = 1;
 	u32 coal_intvl = 0;
+	int ret;
+
+	ret = pm_runtime_get_sync(&priv->pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&priv->pdev->dev);
+		return ret;
+	}
 
 	coal_intvl = coal->rx_coalesce_usecs;
 
@@ -985,6 +992,8 @@ update_return:
 		priv->coal_intvl = coal_intvl;
 	}
 
+	pm_runtime_put(&priv->pdev->dev);
+
 	return 0;
 }
 
@@ -1022,7 +1031,13 @@ static void cpsw_get_ethtool_stats(struct net_device *ndev,
 	struct cpdma_chan_stats tx_stats;
 	u32 val;
 	u8 *p;
-	int i;
+	int i, ret;
+
+	ret = pm_runtime_get_sync(&priv->pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&priv->pdev->dev);
+		return;
+	}
 
 	/* Collect Davinci CPDMA stats for Rx and Tx Channel */
 	cpdma_chan_get_stats(priv->rxch, &rx_stats);
@@ -1049,6 +1064,8 @@ static void cpsw_get_ethtool_stats(struct net_device *ndev,
 			break;
 		}
 	}
+
+	pm_runtime_put(&priv->pdev->dev);
 }
 
 static int cpsw_common_res_usage_state(struct cpsw_priv *priv)
@@ -1780,11 +1797,20 @@ static void cpsw_get_regs(struct net_device *ndev,
 {
 	struct cpsw_priv *priv = netdev_priv(ndev);
 	u32 *reg = p;
+	int ret;
+
+	ret = pm_runtime_get_sync(&priv->pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&priv->pdev->dev);
+		return;
+	}
 
 	/* update CPSW IP version */
 	regs->version = priv->version;
 
 	cpsw_ale_dump(priv->ale, reg);
+
+	pm_runtime_put(&priv->pdev->dev);
 }
 
 static void cpsw_get_drvinfo(struct net_device *ndev,
@@ -1902,12 +1928,20 @@ static int cpsw_set_pauseparam(struct net_device *ndev,
 {
 	struct cpsw_priv *priv = netdev_priv(ndev);
 	bool link;
+	int ret;
+
+	ret = pm_runtime_get_sync(&priv->pdev->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(&priv->pdev->dev);
+		return ret;
+	}
 
 	priv->rx_pause = pause->rx_pause ? true : false;
 	priv->tx_pause = pause->tx_pause ? true : false;
 
 	for_each_slave(priv, _cpsw_adjust_link, priv, &link);
 
+	pm_runtime_put(&priv->pdev->dev);
 	return 0;
 }
 
-- 
2.8.4

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

[Grygorii Strashko]

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 can 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 1ba0c09..591d1c3 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1633,10 +1633,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;
@@ -1651,6 +1658,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;
 }
 
@@ -1715,10 +1724,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
@@ -1733,7 +1749,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,
@@ -1745,6 +1764,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;
 
@@ -1764,8 +1789,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.8.4

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

[Grygorii Strashko]

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.8.4

[PATCH 07/15] drivers: net: davinci_mdio: remove pm runtime calls from suspend callbacks

[Grygorii Strashko]

PM runtime is disabled when Davinci MDIO .suspend_late() and
.resume_early() callbacks are called. As result, any PM runtime calls here will
be just a nop and can be removed.

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

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 2e19dd1..291c42e 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -436,7 +436,6 @@ static int davinci_mdio_suspend(struct device *dev)
 
 	data->suspended = true;
 	spin_unlock(&data->lock);
-	pm_runtime_put_sync(data->dev);
 
 	/* Select sleep pin state */
 	pinctrl_pm_select_sleep_state(dev);
@@ -451,8 +450,6 @@ static int davinci_mdio_resume(struct device *dev)
 	/* Select default pin state */
 	pinctrl_pm_select_default_state(dev);
 
-	pm_runtime_get_sync(data->dev);
-
 	spin_lock(&data->lock);
 	/* restart the scan state machine */
 	__davinci_mdio_reset(data);
-- 
2.8.4

[PATCH 08/15] drivers: net: davinci_mdio: drop suspended and lock fields from mdio_data

[Grygorii Strashko]

The Davinci MDIO is not expected to be accessible after its suspend
callbacks has been called:
 - all consumers of Davinci MDIO will stop/disconnect their phys at Device
suspend stage;
 - all phys are expected to be suspended already by PHY/MDIO core;
 - MDIO locking is done by MDIO Bus code.

Hence, it's safe to drop "suspended" and "lock" fields from mdio_data.

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

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 291c42e..b6d0059 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -90,11 +90,9 @@ static const struct mdio_platform_data default_pdata = {
 struct davinci_mdio_data {
 	struct mdio_platform_data pdata;
 	struct davinci_mdio_regs __iomem *regs;
-	spinlock_t	lock;
 	struct clk	*clk;
 	struct device	*dev;
 	struct mii_bus	*bus;
-	bool		suspended;
 	unsigned long	access_time; /* jiffies */
 	/* Indicates that driver shouldn't modify phy_mask in case
 	 * if MDIO bus is registered from DT.
@@ -225,13 +223,6 @@ static int davinci_mdio_read(struct mii_bus *bus, int phy_id, int phy_reg)
 	if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK)
 		return -EINVAL;
 
-	spin_lock(&data->lock);
-
-	if (data->suspended) {
-		spin_unlock(&data->lock);
-		return -ENODEV;
-	}
-
 	reg = (USERACCESS_GO | USERACCESS_READ | (phy_reg << 21) |
 	       (phy_id << 16));
 
@@ -255,8 +246,6 @@ static int davinci_mdio_read(struct mii_bus *bus, int phy_id, int phy_reg)
 		break;
 	}
 
-	spin_unlock(&data->lock);
-
 	return ret;
 }
 
@@ -270,13 +259,6 @@ static int davinci_mdio_write(struct mii_bus *bus, int phy_id,
 	if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK)
 		return -EINVAL;
 
-	spin_lock(&data->lock);
-
-	if (data->suspended) {
-		spin_unlock(&data->lock);
-		return -ENODEV;
-	}
-
 	reg = (USERACCESS_GO | USERACCESS_WRITE | (phy_reg << 21) |
 		   (phy_id << 16) | (phy_data & USERACCESS_DATA));
 
@@ -295,8 +277,6 @@ static int davinci_mdio_write(struct mii_bus *bus, int phy_id,
 		break;
 	}
 
-	spin_unlock(&data->lock);
-
 	return 0;
 }
 
@@ -364,7 +344,6 @@ static int davinci_mdio_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(dev, data);
 	data->dev = dev;
-	spin_lock_init(&data->lock);
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	data->regs = devm_ioremap_resource(dev, res);
@@ -426,17 +405,12 @@ static int davinci_mdio_suspend(struct device *dev)
 	struct davinci_mdio_data *data = dev_get_drvdata(dev);
 	u32 ctrl;
 
-	spin_lock(&data->lock);
-
 	/* shutdown the scan state machine */
 	ctrl = __raw_readl(&data->regs->control);
 	ctrl &= ~CONTROL_ENABLE;
 	__raw_writel(ctrl, &data->regs->control);
 	wait_for_idle(data);
 
-	data->suspended = true;
-	spin_unlock(&data->lock);
-
 	/* Select sleep pin state */
 	pinctrl_pm_select_sleep_state(dev);
 
@@ -450,13 +424,9 @@ static int davinci_mdio_resume(struct device *dev)
 	/* Select default pin state */
 	pinctrl_pm_select_default_state(dev);
 
-	spin_lock(&data->lock);
 	/* restart the scan state machine */
 	__davinci_mdio_reset(data);
 
-	data->suspended = false;
-	spin_unlock(&data->lock);
-
 	return 0;
 }
 #endif
-- 
2.8.4

[PATCH 09/15] drivers: net: davinci_mdio: split reset function on init_clk and enable

[Grygorii Strashko]

The Davinci MDIO MDIO_CONTROL.CLKDIV can be calculated only once
during probe, hence split __davinci_mdio_reset() on
davinci_mdio_init_clk() and davinci_mdio_enable(). Initialize and
save CLKDIV in .probe(). Then just use saved value.

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

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index b6d0059..b206fd3 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -98,9 +98,10 @@ struct davinci_mdio_data {
 	 * if MDIO bus is registered from DT.
 	 */
 	bool		skip_scan;
+	u32		clk_div;
 };
 
-static void __davinci_mdio_reset(struct davinci_mdio_data *data)
+static void davinci_mdio_init_clk(struct davinci_mdio_data *data)
 {
 	u32 mdio_in, div, mdio_out_khz, access_time;
 
@@ -109,9 +110,7 @@ static void __davinci_mdio_reset(struct davinci_mdio_data *data)
 	if (div > CONTROL_MAX_DIV)
 		div = CONTROL_MAX_DIV;
 
-	/* set enable and clock divider */
-	__raw_writel(div | CONTROL_ENABLE, &data->regs->control);
-
+	data->clk_div = div;
 	/*
 	 * One mdio transaction consists of:
 	 *	32 bits of preamble
@@ -132,12 +131,18 @@ static void __davinci_mdio_reset(struct davinci_mdio_data *data)
 		data->access_time = 1;
 }
 
+static void davinci_mdio_enable(struct davinci_mdio_data *data)
+{
+	/* set enable and clock divider */
+	__raw_writel(data->clk_div | CONTROL_ENABLE, &data->regs->control);
+}
+
 static int davinci_mdio_reset(struct mii_bus *bus)
 {
 	struct davinci_mdio_data *data = bus->priv;
 	u32 phy_mask, ver;
 
-	__davinci_mdio_reset(data);
+	davinci_mdio_enable(data);
 
 	/* wait for scan logic to settle */
 	msleep(PHY_MAX_ADDR * data->access_time);
@@ -188,7 +193,7 @@ static inline int wait_for_user_access(struct davinci_mdio_data *data)
 		 * operation
 		 */
 		dev_warn(data->dev, "resetting idled controller\n");
-		__davinci_mdio_reset(data);
+		davinci_mdio_enable(data);
 		return -EAGAIN;
 	}
 
@@ -350,6 +355,8 @@ static int davinci_mdio_probe(struct platform_device *pdev)
 	if (IS_ERR(data->regs))
 		return PTR_ERR(data->regs);
 
+	davinci_mdio_init_clk(data);
+
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
 
@@ -425,7 +432,7 @@ static int davinci_mdio_resume(struct device *dev)
 	pinctrl_pm_select_default_state(dev);
 
 	/* restart the scan state machine */
-	__davinci_mdio_reset(data);
+	davinci_mdio_enable(data);
 
 	return 0;
 }
-- 
2.8.4

[PATCH 10/15] drivers: net: davinci_mdio: add pm runtime callbacks

[Grygorii Strashko]

Add PM runtime .runtime_suspend()/.runtime_resume() callbacks and
perform Davinci MDIO enabling/disabling from these callbacks. This
allows to reuse pm_runtime_force_suspend/resume() APIs during System
suspend and required for further implementation of PM runtime
autosuspend.

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

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index b206fd3..13f5080 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -406,8 +406,8 @@ static int davinci_mdio_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int davinci_mdio_suspend(struct device *dev)
+#ifdef CONFIG_PM
+static int davinci_mdio_runtime_suspend(struct device *dev)
 {
 	struct davinci_mdio_data *data = dev_get_drvdata(dev);
 	u32 ctrl;
@@ -418,6 +418,28 @@ static int davinci_mdio_suspend(struct device *dev)
 	__raw_writel(ctrl, &data->regs->control);
 	wait_for_idle(data);
 
+	return 0;
+}
+
+static int davinci_mdio_runtime_resume(struct device *dev)
+{
+	struct davinci_mdio_data *data = dev_get_drvdata(dev);
+
+	davinci_mdio_enable(data);
+	return 0;
+}
+#endif
+
+#ifdef CONFIG_PM_SLEEP
+static int davinci_mdio_suspend(struct device *dev)
+{
+	struct davinci_mdio_data *data = dev_get_drvdata(dev);
+	int ret = 0;
+
+	ret = pm_runtime_force_suspend(dev);
+	if (ret < 0)
+		return ret;
+
 	/* Select sleep pin state */
 	pinctrl_pm_select_sleep_state(dev);
 
@@ -431,14 +453,15 @@ static int davinci_mdio_resume(struct device *dev)
 	/* Select default pin state */
 	pinctrl_pm_select_default_state(dev);
 
-	/* restart the scan state machine */
-	davinci_mdio_enable(data);
+	pm_runtime_force_resume(dev);
 
 	return 0;
 }
 #endif
 
 static const struct dev_pm_ops davinci_mdio_pm_ops = {
+	SET_RUNTIME_PM_OPS(davinci_mdio_runtime_suspend,
+			   davinci_mdio_runtime_resume, NULL)
 	SET_LATE_SYSTEM_SLEEP_PM_OPS(davinci_mdio_suspend, davinci_mdio_resume)
 };
 
-- 
2.8.4

[PATCH 11/15] drivers: net: davinci_mdio: implement pm runtime auto mode

[Grygorii Strashko]

Davinci MDIO is always used as slave device which services
read/write requests from MDIO/PHY core. It doesn't use IRQ also.

As result, It's possible to relax PM runtime constraints for Davinci
MDIO and enable it on demand, instead of powering it during probe
and powering off during removal.

Hence, 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 expected to work corectly with RPM
 autosuspend enabled:
- expected to work on SoCs where MDIO is defined as part of TI CPSW in DT
(cpsw.c DRA7/am57x, am437x, am335x, dm814x)
- not verified on Keystone 2 and other SoCs where MDIO is used with TI EMAC IP
(davinci_emac.c:  dm6467-emac, am3517-emac, dm816-emac).

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

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

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 13f5080..ce3ec42 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -93,6 +93,7 @@ struct davinci_mdio_data {
 	struct clk	*clk;
 	struct device	*dev;
 	struct mii_bus	*bus;
+	bool            active_in_suspend;
 	unsigned long	access_time; /* jiffies */
 	/* Indicates that driver shouldn't modify phy_mask in case
 	 * if MDIO bus is registered from DT.
@@ -141,8 +142,13 @@ static int davinci_mdio_reset(struct mii_bus *bus)
 {
 	struct davinci_mdio_data *data = bus->priv;
 	u32 phy_mask, ver;
+	int ret;
 
-	davinci_mdio_enable(data);
+	ret = pm_runtime_get_sync(data->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(data->dev);
+		return ret;
+	}
 
 	/* wait for scan logic to settle */
 	msleep(PHY_MAX_ADDR * data->access_time);
@@ -153,7 +159,7 @@ static int davinci_mdio_reset(struct mii_bus *bus)
 		 (ver >> 8) & 0xff, ver & 0xff);
 
 	if (data->skip_scan)
-		return 0;
+		goto done;
 
 	/* get phy mask from the alive register */
 	phy_mask = __raw_readl(&data->regs->alive);
@@ -168,6 +174,10 @@ static int davinci_mdio_reset(struct mii_bus *bus)
 	}
 	data->bus->phy_mask = phy_mask;
 
+done:
+	pm_runtime_mark_last_busy(data->dev);
+	pm_runtime_put_autosuspend(data->dev);
+
 	return 0;
 }
 
@@ -228,6 +238,12 @@ static int davinci_mdio_read(struct mii_bus *bus, int phy_id, int phy_reg)
 	if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK)
 		return -EINVAL;
 
+	ret = pm_runtime_get_sync(data->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(data->dev);
+		return ret;
+	}
+
 	reg = (USERACCESS_GO | USERACCESS_READ | (phy_reg << 21) |
 	       (phy_id << 16));
 
@@ -251,6 +267,8 @@ static int davinci_mdio_read(struct mii_bus *bus, int phy_id, int phy_reg)
 		break;
 	}
 
+	pm_runtime_mark_last_busy(data->dev);
+	pm_runtime_put_autosuspend(data->dev);
 	return ret;
 }
 
@@ -264,6 +282,12 @@ static int davinci_mdio_write(struct mii_bus *bus, int phy_id,
 	if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK)
 		return -EINVAL;
 
+	ret = pm_runtime_get_sync(data->dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(data->dev);
+		return ret;
+	}
+
 	reg = (USERACCESS_GO | USERACCESS_WRITE | (phy_reg << 21) |
 		   (phy_id << 16) | (phy_data & USERACCESS_DATA));
 
@@ -282,7 +306,10 @@ static int davinci_mdio_write(struct mii_bus *bus, int phy_id,
 		break;
 	}
 
-	return 0;
+	pm_runtime_mark_last_busy(data->dev);
+	pm_runtime_put_autosuspend(data->dev);
+
+	return ret;
 }
 
 #if IS_ENABLED(CONFIG_OF)
@@ -357,8 +384,9 @@ static int davinci_mdio_probe(struct platform_device *pdev)
 
 	davinci_mdio_init_clk(data);
 
+	pm_runtime_set_autosuspend_delay(&pdev->dev, -1);
+	pm_runtime_use_autosuspend(&pdev->dev);
 	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
@@ -387,9 +415,8 @@ static int davinci_mdio_probe(struct platform_device *pdev)
 	return 0;
 
 bail_out:
-	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-
 	return ret;
 }
 
@@ -400,7 +427,7 @@ static int davinci_mdio_remove(struct platform_device *pdev)
 	if (data->bus)
 		mdiobus_unregister(data->bus);
 
-	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
 	return 0;
@@ -436,7 +463,9 @@ static int davinci_mdio_suspend(struct device *dev)
 	struct davinci_mdio_data *data = dev_get_drvdata(dev);
 	int ret = 0;
 
-	ret = pm_runtime_force_suspend(dev);
+	data->active_in_suspend = !pm_runtime_status_suspended(dev);
+	if (data->active_in_suspend)
+		ret = pm_runtime_force_suspend(dev);
 	if (ret < 0)
 		return ret;
 
@@ -453,7 +482,8 @@ static int davinci_mdio_resume(struct device *dev)
 	/* Select default pin state */
 	pinctrl_pm_select_default_state(dev);
 
-	pm_runtime_force_resume(dev);
+	if (data->active_in_suspend)
+		pm_runtime_force_resume(dev);
 
 	return 0;
 }
-- 
2.8.4

[PATCH 12/15] net: davinci_mdio: document missed "ti,am4372-mdio" compat string

[Grygorii Strashko]

Document missed "ti,am4372-mdio" compat string used for TI am437x SoC
(am4372.dtsi).

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 Documentation/devicetree/bindings/net/davinci-mdio.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/davinci-mdio.txt b/Documentation/devicetree/bindings/net/davinci-mdio.txt
index 0369e25..f2bba50 100644
--- a/Documentation/devicetree/bindings/net/davinci-mdio.txt
+++ b/Documentation/devicetree/bindings/net/davinci-mdio.txt
@@ -2,7 +2,8 @@ TI SoC Davinci/Keystone2 MDIO Controller Device Tree Bindings
 ---------------------------------------------------
 
 Required properties:
-- compatible		: Should be "ti,davinci_mdio" or "ti,keystone_mdio"
+- compatible		: Should be "ti,davinci_mdio", "ti,keystone_mdio",
+			  "ti,am4372-mdio"
 - reg			: physical base address and size of the davinci mdio
 			  registers map
 - bus_freq		: Mdio Bus frequency
-- 
2.8.4

[PATCH 13/15] net: davinci_mdio: introduce "ti,cpsw-mdio" compat string

[Grygorii Strashko]

Introduce "ti,cpsw-mdio" compatible string for Davinci MDIO, because
it's required to distinguish the case when MDIO is part of TI CPSW to
enable features supported by TI CPSW (for example, enable PM
management).

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 Documentation/devicetree/bindings/net/davinci-mdio.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/davinci-mdio.txt b/Documentation/devicetree/bindings/net/davinci-mdio.txt
index f2bba50..a3d6d4b 100644
--- a/Documentation/devicetree/bindings/net/davinci-mdio.txt
+++ b/Documentation/devicetree/bindings/net/davinci-mdio.txt
@@ -3,7 +3,7 @@ TI SoC Davinci/Keystone2 MDIO Controller Device Tree Bindings
 
 Required properties:
 - compatible		: Should be "ti,davinci_mdio", "ti,keystone_mdio",
-			  "ti,am4372-mdio"
+			  "ti,am4372-mdio", "ti,cpsw-mdio"
 - reg			: physical base address and size of the davinci mdio
 			  registers map
 - bus_freq		: Mdio Bus frequency
-- 
2.8.4

[PATCH 14/15] drivers: net: davinci_mdio: enable pm runtime auto for ti cpsw-mdio

[Grygorii Strashko]

Use "ti,cpsw-mdio" to enable PM runtime auto-suspend on supported
platforms, where MDIO is implemented as part of TI CPSW.

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

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index ce3ec42..d1fb734 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -53,6 +53,10 @@
 
 #define DEF_OUT_FREQ		2200000		/* 2.2 MHz */
 
+struct davinci_mdio_of_param {
+	int autosuspend_delay_ms;
+};
+
 struct davinci_mdio_regs {
 	u32	version;
 	u32	control;
@@ -332,6 +336,19 @@ static int davinci_mdio_probe_dt(struct mdio_platform_data *data,
 }
 #endif
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct davinci_mdio_of_param of_cpsw_mdio_data = {
+	.autosuspend_delay_ms = 100,
+};
+
+static const struct of_device_id davinci_mdio_of_mtable[] = {
+	{ .compatible = "ti,davinci_mdio", },
+	{ .compatible = "ti,cpsw-mdio", .data = &of_cpsw_mdio_data},
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, davinci_mdio_of_mtable);
+#endif
+
 static int davinci_mdio_probe(struct platform_device *pdev)
 {
 	struct mdio_platform_data *pdata = dev_get_platdata(&pdev->dev);
@@ -340,6 +357,7 @@ static int davinci_mdio_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct phy_device *phy;
 	int ret, addr;
+	int autosuspend_delay_ms = -1;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
@@ -352,9 +370,22 @@ static int davinci_mdio_probe(struct platform_device *pdev)
 	}
 
 	if (dev->of_node) {
-		if (davinci_mdio_probe_dt(&data->pdata, pdev))
-			data->pdata = default_pdata;
+		const struct of_device_id	*of_id;
+
+		ret = davinci_mdio_probe_dt(data, pdev);
+		if (ret)
+			return ret;
 		snprintf(data->bus->id, MII_BUS_ID_SIZE, "%s", pdev->name);
+
+		of_id = of_match_device(davinci_mdio_of_mtable, &pdev->dev);
+		if (of_id) {
+			const struct davinci_mdio_of_param *of_mdio_data;
+
+			of_mdio_data = of_id->data;
+			if (of_mdio_data)
+				autosuspend_delay_ms =
+					of_mdio_data->autosuspend_delay_ms;
+		}
 	} else {
 		data->pdata = pdata ? (*pdata) : default_pdata;
 		snprintf(data->bus->id, MII_BUS_ID_SIZE, "%s-%x",
@@ -384,7 +415,7 @@ static int davinci_mdio_probe(struct platform_device *pdev)
 
 	davinci_mdio_init_clk(data);
 
-	pm_runtime_set_autosuspend_delay(&pdev->dev, -1);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, autosuspend_delay_ms);
 	pm_runtime_use_autosuspend(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
 
@@ -495,14 +526,6 @@ static const struct dev_pm_ops davinci_mdio_pm_ops = {
 	SET_LATE_SYSTEM_SLEEP_PM_OPS(davinci_mdio_suspend, davinci_mdio_resume)
 };
 
-#if IS_ENABLED(CONFIG_OF)
-static const struct of_device_id davinci_mdio_of_mtable[] = {
-	{ .compatible = "ti,davinci_mdio", },
-	{ /* sentinel */ },
-};
-MODULE_DEVICE_TABLE(of, davinci_mdio_of_mtable);
-#endif
-
 static struct platform_driver davinci_mdio_driver = {
 	.driver = {
 		.name	 = "davinci_mdio",
-- 
2.8.4

[PATCH 15/15] ARM: dts: am335x/am437x/dra7: use new "ti,cpsw-mdio" compat string

[Grygorii Strashko]

Add "ti,cpsw-mdio" for am335x/am437x/dra7 SoCs where MDIO is
implemented as part of TI CPSW and, this way, enable PM runtime auto
suspend for Davinci MDIO driver on these paltforms.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 arch/arm/boot/dts/am33xx.dtsi | 2 +-
 arch/arm/boot/dts/am4372.dtsi | 2 +-
 arch/arm/boot/dts/dra7.dtsi   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 52be48b..7fa9a1d 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -789,7 +789,7 @@
 			status = "disabled";
 
 			davinci_mdio: mdio@4a101000 {
-				compatible = "ti,davinci_mdio";
+				compatible = "ti,cpsw-mdio","ti,davinci_mdio";
 				#address-cells = <1>;
 				#size-cells = <0>;
 				ti,hwmods = "davinci_mdio";
diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index 12fcde4..ea76fa7 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -636,7 +636,7 @@
 			syscon = <&scm_conf>;
 
 			davinci_mdio: mdio@4a101000 {
-				compatible = "ti,am4372-mdio","ti,davinci_mdio";
+				compatible = "ti,am4372-mdio","ti,cpsw-mdio","ti,davinci_mdio";
 				reg = <0x4a101000 0x100>;
 				#address-cells = <1>;
 				#size-cells = <0>;
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index e007401..911ef85 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -1661,7 +1661,7 @@
 			status = "disabled";
 
 			davinci_mdio: mdio@48485000 {
-				compatible = "ti,davinci_mdio";
+				compatible = "ti,cpsw-mdio","ti,davinci_mdio";
 				#address-cells = <1>;
 				#size-cells = <0>;
 				ti,hwmods = "davinci_mdio";
-- 
2.8.4

Re: [PATCH 14/15] drivers: net: davinci_mdio: enable pm runtime auto for ti cpsw-mdio

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1850 bytes --]

Hi,

[auto build test ERROR on net-next/master]
[also build test ERROR on v4.7-rc3 next-20160615]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Grygorii-Strashko/drivers-net-cpsw-improve-runtime-pm/20160615-200750
config: arm-multi_v7_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/net/ethernet/ti/davinci_mdio.c: In function 'davinci_mdio_probe':
>> drivers/net/ethernet/ti/davinci_mdio.c:375:31: error: passing argument 1 of 'davinci_mdio_probe_dt' from incompatible pointer type [-Werror=incompatible-pointer-types]
      ret = davinci_mdio_probe_dt(data, pdev);
                                  ^
   drivers/net/ethernet/ti/davinci_mdio.c:320:12: note: expected 'struct mdio_platform_data *' but argument is of type 'struct davinci_mdio_data *'
    static int davinci_mdio_probe_dt(struct mdio_platform_data *data,
               ^
   cc1: some warnings being treated as errors

vim +/davinci_mdio_probe_dt +375 drivers/net/ethernet/ti/davinci_mdio.c

   369			return -ENOMEM;
   370		}
   371	
   372		if (dev->of_node) {
   373			const struct of_device_id	*of_id;
   374	
 > 375			ret = davinci_mdio_probe_dt(data, pdev);
   376			if (ret)
   377				return ret;
   378			snprintf(data->bus->id, MII_BUS_ID_SIZE, "%s", pdev->name);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 37714 bytes --]

Re: [PATCH 04/15] drivers: net: cpsw: ethtool: fix accessing to suspended device

[Florian Fainelli]

On 06/15/2016 04:55 AM, Grygorii Strashko wrote:
> 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.
> 
> Hence, fix it by adding RPM get/put calls in ethtool callbcaks which
> can access CPSW registers: .set_coalesce(), .get_ethtool_stats(),
> .set_pauseparam(), .get_regs()

Provided that you implement an ethtool_ops::begin, it will be called
before each ethtool operation runs, so that could allow you to eliminate
some of the duplication here. Conversely ethtool_ops::end terminates
each operation and can be used for that purpose.

> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  drivers/net/ethernet/ti/cpsw.c | 36 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index ba81d4e..1ba0c09 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -931,6 +931,13 @@ static int cpsw_set_coalesce(struct net_device *ndev,
>  	u32 prescale = 0;
>  	u32 addnl_dvdr = 1;
>  	u32 coal_intvl = 0;
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(&priv->pdev->dev);
> +	if (ret < 0) {
> +		pm_runtime_put_noidle(&priv->pdev->dev);
> +		return ret;
> +	}
>  
>  	coal_intvl = coal->rx_coalesce_usecs;
>  
> @@ -985,6 +992,8 @@ update_return:
>  		priv->coal_intvl = coal_intvl;
>  	}
>  
> +	pm_runtime_put(&priv->pdev->dev);
> +
>  	return 0;
>  }
>  
> @@ -1022,7 +1031,13 @@ static void cpsw_get_ethtool_stats(struct net_device *ndev,
>  	struct cpdma_chan_stats tx_stats;
>  	u32 val;
>  	u8 *p;
> -	int i;
> +	int i, ret;
> +
> +	ret = pm_runtime_get_sync(&priv->pdev->dev);
> +	if (ret < 0) {
> +		pm_runtime_put_noidle(&priv->pdev->dev);
> +		return;
> +	}
>  
>  	/* Collect Davinci CPDMA stats for Rx and Tx Channel */
>  	cpdma_chan_get_stats(priv->rxch, &rx_stats);
> @@ -1049,6 +1064,8 @@ static void cpsw_get_ethtool_stats(struct net_device *ndev,
>  			break;
>  		}
>  	}
> +
> +	pm_runtime_put(&priv->pdev->dev);
>  }
>  
>  static int cpsw_common_res_usage_state(struct cpsw_priv *priv)
> @@ -1780,11 +1797,20 @@ static void cpsw_get_regs(struct net_device *ndev,
>  {
>  	struct cpsw_priv *priv = netdev_priv(ndev);
>  	u32 *reg = p;
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(&priv->pdev->dev);
> +	if (ret < 0) {
> +		pm_runtime_put_noidle(&priv->pdev->dev);
> +		return;
> +	}
>  
>  	/* update CPSW IP version */
>  	regs->version = priv->version;
>  
>  	cpsw_ale_dump(priv->ale, reg);
> +
> +	pm_runtime_put(&priv->pdev->dev);
>  }
>  
>  static void cpsw_get_drvinfo(struct net_device *ndev,
> @@ -1902,12 +1928,20 @@ static int cpsw_set_pauseparam(struct net_device *ndev,
>  {
>  	struct cpsw_priv *priv = netdev_priv(ndev);
>  	bool link;
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(&priv->pdev->dev);
> +	if (ret < 0) {
> +		pm_runtime_put_noidle(&priv->pdev->dev);
> +		return ret;
> +	}
>  
>  	priv->rx_pause = pause->rx_pause ? true : false;
>  	priv->tx_pause = pause->tx_pause ? true : false;
>  
>  	for_each_slave(priv, _cpsw_adjust_link, priv, &link);
>  
> +	pm_runtime_put(&priv->pdev->dev);
>  	return 0;
>  }
>  
> 


-- 
Florian

Re: [PATCH 04/15] drivers: net: cpsw: ethtool: fix accessing to suspended device

[Grygorii Strashko]

On 06/15/2016 07:14 PM, Florian Fainelli wrote:
> On 06/15/2016 04:55 AM, Grygorii Strashko wrote:
>> 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.
>>
>> Hence, fix it by adding RPM get/put calls in ethtool callbcaks which
>> can access CPSW registers: .set_coalesce(), .get_ethtool_stats(),
>> .set_pauseparam(), .get_regs()
>
> Provided that you implement an ethtool_ops::begin, it will be called
> before each ethtool operation runs, so that could allow you to eliminate
> some of the duplication here. Conversely ethtool_ops::end terminates
> each operation and can be used for that purpose.

Ah. Thanks for the advice. (assume you've meant .complete())



-- 
regards,
-grygorii

Re: [PATCH 04/15] drivers: net: cpsw: ethtool: fix accessing to suspended device

[Ivan Khoronzhuk]

On 15.06.16 19:45, Grygorii Strashko wrote:
> On 06/15/2016 07:14 PM, Florian Fainelli wrote:
>> On 06/15/2016 04:55 AM, Grygorii Strashko wrote:
>>> 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.
>>>
>>> Hence, fix it by adding RPM get/put calls in ethtool callbcaks which
>>> can access CPSW registers: .set_coalesce(), .get_ethtool_stats(),
>>> .set_pauseparam(), .get_regs()
>>
>> Provided that you implement an ethtool_ops::begin, it will be called
>> before each ethtool operation runs, so that could allow you to eliminate
>> some of the duplication here. Conversely ethtool_ops::end terminates
>> each operation and can be used for that purpose.
>
> Ah. Thanks for the advice. (assume you've meant .complete())

Maybe better leave as is, I'm going to add two additional ehttool ops
containing RPM sensitive part and not, better to split code on those,
if possible...and hiding put/get in begin op can call rpm when it's not
needed at all..

>
>
>

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH 04/15] drivers: net: cpsw: ethtool: fix accessing to suspended device

[Ivan Khoronzhuk]

On 16.06.16 15:48, Ivan Khoronzhuk wrote:
>
>
> On 15.06.16 19:45, Grygorii Strashko wrote:
>> On 06/15/2016 07:14 PM, Florian Fainelli wrote:
>>> On 06/15/2016 04:55 AM, Grygorii Strashko wrote:
>>>> 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.
>>>>
>>>> Hence, fix it by adding RPM get/put calls in ethtool callbcaks which
>>>> can access CPSW registers: .set_coalesce(), .get_ethtool_stats(),
>>>> .set_pauseparam(), .get_regs()
>>>
>>> Provided that you implement an ethtool_ops::begin, it will be called
>>> before each ethtool operation runs, so that could allow you to eliminate
>>> some of the duplication here. Conversely ethtool_ops::end terminates
>>> each operation and can be used for that purpose.
>>
>> Ah. Thanks for the advice. (assume you've meant .complete())
>
> Maybe better leave as is, I'm going to add two additional ehttool ops
> containing RPM sensitive part and not, better to split code on those,
> if possible...and hiding put/get in begin op can call rpm when it's not
> needed at all..

But it's not a big deal, ethtool operations are rarelly anyway, and have
impact only when interface is down that makes it even more rarely.

>
>>
>>
>>
>

-- 
Regards,
Ivan Khoronzhuk

Re: [PATCH 12/15] net: davinci_mdio: document missed "ti,am4372-mdio" compat string

[Rob Herring]

On Wed, Jun 15, 2016 at 02:56:00PM +0300, Grygorii Strashko wrote:
> Document missed "ti,am4372-mdio" compat string used for TI am437x SoC
> (am4372.dtsi).
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  Documentation/devicetree/bindings/net/davinci-mdio.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/davinci-mdio.txt b/Documentation/devicetree/bindings/net/davinci-mdio.txt
> index 0369e25..f2bba50 100644
> --- a/Documentation/devicetree/bindings/net/davinci-mdio.txt
> +++ b/Documentation/devicetree/bindings/net/davinci-mdio.txt
> @@ -2,7 +2,8 @@ TI SoC Davinci/Keystone2 MDIO Controller Device Tree Bindings
>  ---------------------------------------------------
>  
>  Required properties:
> -- compatible		: Should be "ti,davinci_mdio" or "ti,keystone_mdio"
> +- compatible		: Should be "ti,davinci_mdio", "ti,keystone_mdio",
> +			  "ti,am4372-mdio"

This is still an OR relationship?

It's preferred to list one per line.

>  - reg			: physical base address and size of the davinci mdio
>  			  registers map
>  - bus_freq		: Mdio Bus frequency
> -- 
> 2.8.4
> 

Re: [PATCH 15/15] ARM: dts: am335x/am437x/dra7: use new "ti,cpsw-mdio" compat string

[Tony Lindgren]

* Grygorii Strashko <grygorii.strashko@ti.com> [160615 05:05]:
> Add "ti,cpsw-mdio" for am335x/am437x/dra7 SoCs where MDIO is
> implemented as part of TI CPSW and, this way, enable PM runtime auto
> suspend for Davinci MDIO driver on these paltforms.

This one should not cause merge conflicts, please feel free
to merge along with the other CPSW driver patches after the
pending comments are dealt with:

Acked-by: Tony Lindgren <tony@atomide.com>

Re: [PATCH 12/15] net: davinci_mdio: document missed "ti,am4372-mdio" compat string

[Grygorii Strashko]

On 06/19/2016 05:35 PM, Rob Herring wrote:
> On Wed, Jun 15, 2016 at 02:56:00PM +0300, Grygorii Strashko wrote:
>> Document missed "ti,am4372-mdio" compat string used for TI am437x SoC
>> (am4372.dtsi).
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   Documentation/devicetree/bindings/net/davinci-mdio.txt | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/davinci-mdio.txt b/Documentation/devicetree/bindings/net/davinci-mdio.txt
>> index 0369e25..f2bba50 100644
>> --- a/Documentation/devicetree/bindings/net/davinci-mdio.txt
>> +++ b/Documentation/devicetree/bindings/net/davinci-mdio.txt
>> @@ -2,7 +2,8 @@ TI SoC Davinci/Keystone2 MDIO Controller Device Tree Bindings
>>   ---------------------------------------------------
>>   
>>   Required properties:
>> -- compatible		: Should be "ti,davinci_mdio" or "ti,keystone_mdio"
>> +- compatible		: Should be "ti,davinci_mdio", "ti,keystone_mdio",
>> +			  "ti,am4372-mdio"
> 
> This is still an OR relationship?

I think it's AND relation. It should be smth. like this:
 [SoC,][IP family,] "ti,davinci_mdio"
where "ti,davinci_mdio" is mandatory always
 [IP family] - mandatory for IP/SOC family and can be "ti,keystone_mdio" or "ti,cpsw-mdio" (last one is added by subsequent patch)
 [SoC] - mandatory for SoC. now can be "ti,am4372-mdio"

I can update it as below:
- compatible		: Should be "ti,davinci_mdio"  
			  and "ti,keystone_mdio" for Keystone 2 SoCs
 			  and "ti,am4372-mdio" for am472x SoC
and in subsequent patch
			 and "ti,cpsw-mdio" for am335x, am472x, am57xx/dra7, dm814x SoCs
			 and "ti,am4372-mdio" for am472x SoC



> 
> It's preferred to list one per line.

ok



-- 
regards,
-grygorii