All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next RESEND 0/5] dwmac-sun8i cleanup and shutdown hook
@ 2021-02-08  6:28 ` Samuel Holland
  0 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:28 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe
  Cc: Ondrej Jirman, netdev, linux-arm-kernel, linux-kernel,
	linux-sunxi, Samuel Holland

These patches clean up some things I noticed while fixing suspend/resume
behavior. The first four are minor code improvements. The last one adds
a shutdown hook to minimize power consumption on boards without a PMIC.

Now that the fixes series is merged, I'm resending this series rebased
on top of net-next and with Chen-Yu's Reviewed-by tags.

Samuel Holland (5):
  net: stmmac: dwmac-sun8i: Return void from PHY unpower
  net: stmmac: dwmac-sun8i: Remove unnecessary PHY power check
  net: stmmac: dwmac-sun8i: Use reset_control_reset
  net: stmmac: dwmac-sun8i: Minor probe function cleanup
  net: stmmac: dwmac-sun8i: Add a shutdown callback

 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 31 ++++++++++++-------
 1 file changed, 19 insertions(+), 12 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH net-next RESEND 0/5] dwmac-sun8i cleanup and shutdown hook
@ 2021-02-08  6:28 ` Samuel Holland
  0 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:28 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe
  Cc: Ondrej Jirman, Samuel Holland, netdev, linux-kernel, linux-sunxi,
	linux-arm-kernel

These patches clean up some things I noticed while fixing suspend/resume
behavior. The first four are minor code improvements. The last one adds
a shutdown hook to minimize power consumption on boards without a PMIC.

Now that the fixes series is merged, I'm resending this series rebased
on top of net-next and with Chen-Yu's Reviewed-by tags.

Samuel Holland (5):
  net: stmmac: dwmac-sun8i: Return void from PHY unpower
  net: stmmac: dwmac-sun8i: Remove unnecessary PHY power check
  net: stmmac: dwmac-sun8i: Use reset_control_reset
  net: stmmac: dwmac-sun8i: Minor probe function cleanup
  net: stmmac: dwmac-sun8i: Add a shutdown callback

 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 31 ++++++++++++-------
 1 file changed, 19 insertions(+), 12 deletions(-)

-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] i2c: mv64xxx: Fix check for missing clock
  2021-02-08  6:28 ` Samuel Holland
@ 2021-02-08  6:28   ` Samuel Holland
  -1 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:28 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe
  Cc: Ondrej Jirman, netdev, linux-arm-kernel, linux-kernel,
	linux-sunxi, Samuel Holland, Dan Carpenter

In commit e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support"), error
pointers to optional clocks were replaced by NULL to simplify the resume
callback implementation. However, that commit missed that the IS_ERR
check in mv64xxx_of_config should be replaced with a NULL check. As a
result, the check always passes, even for an invalid device tree.

Fixes: e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/i2c/busses/i2c-mv64xxx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index b03c344323d1..c590d36b5fd1 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -813,7 +813,7 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
 	 * need to know tclk in order to calculate bus clock
 	 * factors.
 	 */
-	if (IS_ERR(drv_data->clk)) {
+	if (!drv_data->clk) {
 		rc = -ENODEV;
 		goto out;
 	}
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH] i2c: mv64xxx: Fix check for missing clock
@ 2021-02-08  6:28   ` Samuel Holland
  0 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:28 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe
  Cc: Ondrej Jirman, Samuel Holland, netdev, linux-kernel, linux-sunxi,
	Dan Carpenter, linux-arm-kernel

In commit e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support"), error
pointers to optional clocks were replaced by NULL to simplify the resume
callback implementation. However, that commit missed that the IS_ERR
check in mv64xxx_of_config should be replaced with a NULL check. As a
result, the check always passes, even for an invalid device tree.

Fixes: e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/i2c/busses/i2c-mv64xxx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index b03c344323d1..c590d36b5fd1 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -813,7 +813,7 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
 	 * need to know tclk in order to calculate bus clock
 	 * factors.
 	 */
-	if (IS_ERR(drv_data->clk)) {
+	if (!drv_data->clk) {
 		rc = -ENODEV;
 		goto out;
 	}
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH net-next RESEND 1/5] net: stmmac: dwmac-sun8i: Return void from PHY unpower
  2021-02-08  6:28 ` Samuel Holland
@ 2021-02-08  6:28   ` Samuel Holland
  -1 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:28 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe
  Cc: Ondrej Jirman, netdev, linux-arm-kernel, linux-kernel,
	linux-sunxi, Samuel Holland

This is a deinitialization function that always returned zero, and that
return value was always ignored. Have it return void instead.

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index a5e0eff4a387..8e505019adf8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -820,15 +820,14 @@ static int sun8i_dwmac_power_internal_phy(struct stmmac_priv *priv)
 	return 0;
 }
 
-static int sun8i_dwmac_unpower_internal_phy(struct sunxi_priv_data *gmac)
+static void sun8i_dwmac_unpower_internal_phy(struct sunxi_priv_data *gmac)
 {
 	if (!gmac->internal_phy_powered)
-		return 0;
+		return;
 
 	clk_disable_unprepare(gmac->ephy_clk);
 	reset_control_assert(gmac->rst_ephy);
 	gmac->internal_phy_powered = false;
-	return 0;
 }
 
 /* MDIO multiplexing switch function
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH net-next RESEND 1/5] net: stmmac: dwmac-sun8i: Return void from PHY unpower
@ 2021-02-08  6:28   ` Samuel Holland
  0 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:28 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe
  Cc: Ondrej Jirman, Samuel Holland, netdev, linux-kernel, linux-sunxi,
	linux-arm-kernel

This is a deinitialization function that always returned zero, and that
return value was always ignored. Have it return void instead.

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index a5e0eff4a387..8e505019adf8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -820,15 +820,14 @@ static int sun8i_dwmac_power_internal_phy(struct stmmac_priv *priv)
 	return 0;
 }
 
-static int sun8i_dwmac_unpower_internal_phy(struct sunxi_priv_data *gmac)
+static void sun8i_dwmac_unpower_internal_phy(struct sunxi_priv_data *gmac)
 {
 	if (!gmac->internal_phy_powered)
-		return 0;
+		return;
 
 	clk_disable_unprepare(gmac->ephy_clk);
 	reset_control_assert(gmac->rst_ephy);
 	gmac->internal_phy_powered = false;
-	return 0;
 }
 
 /* MDIO multiplexing switch function
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH net-next RESEND 2/5] net: stmmac: dwmac-sun8i: Remove unnecessary PHY power check
  2021-02-08  6:28 ` Samuel Holland
@ 2021-02-08  6:28   ` Samuel Holland
  -1 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:28 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe
  Cc: Ondrej Jirman, netdev, linux-arm-kernel, linux-kernel,
	linux-sunxi, Samuel Holland

sun8i_dwmac_unpower_internal_phy already checks if the PHY is powered,
so there is no need to do it again here.

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 8e505019adf8..3c3d0b99d3e8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -1018,10 +1018,8 @@ static void sun8i_dwmac_exit(struct platform_device *pdev, void *priv)
 {
 	struct sunxi_priv_data *gmac = priv;
 
-	if (gmac->variant->soc_has_internal_phy) {
-		if (gmac->internal_phy_powered)
-			sun8i_dwmac_unpower_internal_phy(gmac);
-	}
+	if (gmac->variant->soc_has_internal_phy)
+		sun8i_dwmac_unpower_internal_phy(gmac);
 
 	clk_disable_unprepare(gmac->tx_clk);
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH net-next RESEND 2/5] net: stmmac: dwmac-sun8i: Remove unnecessary PHY power check
@ 2021-02-08  6:28   ` Samuel Holland
  0 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:28 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe
  Cc: Ondrej Jirman, Samuel Holland, netdev, linux-kernel, linux-sunxi,
	linux-arm-kernel

sun8i_dwmac_unpower_internal_phy already checks if the PHY is powered,
so there is no need to do it again here.

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 8e505019adf8..3c3d0b99d3e8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -1018,10 +1018,8 @@ static void sun8i_dwmac_exit(struct platform_device *pdev, void *priv)
 {
 	struct sunxi_priv_data *gmac = priv;
 
-	if (gmac->variant->soc_has_internal_phy) {
-		if (gmac->internal_phy_powered)
-			sun8i_dwmac_unpower_internal_phy(gmac);
-	}
+	if (gmac->variant->soc_has_internal_phy)
+		sun8i_dwmac_unpower_internal_phy(gmac);
 
 	clk_disable_unprepare(gmac->tx_clk);
 
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH net-next RESEND 3/5] net: stmmac: dwmac-sun8i: Use reset_control_reset
  2021-02-08  6:28 ` Samuel Holland
@ 2021-02-08  6:28   ` Samuel Holland
  -1 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:28 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe
  Cc: Ondrej Jirman, netdev, linux-arm-kernel, linux-kernel,
	linux-sunxi, Samuel Holland

Use the appropriate function instead of reimplementing it,
and update the error message to match the code.

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 3c3d0b99d3e8..0e8d88417251 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -806,11 +806,9 @@ static int sun8i_dwmac_power_internal_phy(struct stmmac_priv *priv)
 	/* Make sure the EPHY is properly reseted, as U-Boot may leave
 	 * it at deasserted state, and thus it may fail to reset EMAC.
 	 */
-	reset_control_assert(gmac->rst_ephy);
-
-	ret = reset_control_deassert(gmac->rst_ephy);
+	ret = reset_control_reset(gmac->rst_ephy);
 	if (ret) {
-		dev_err(priv->device, "Cannot deassert internal phy\n");
+		dev_err(priv->device, "Cannot reset internal PHY\n");
 		clk_disable_unprepare(gmac->ephy_clk);
 		return ret;
 	}
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH net-next RESEND 3/5] net: stmmac: dwmac-sun8i: Use reset_control_reset
@ 2021-02-08  6:28   ` Samuel Holland
  0 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:28 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe
  Cc: Ondrej Jirman, Samuel Holland, netdev, linux-kernel, linux-sunxi,
	linux-arm-kernel

Use the appropriate function instead of reimplementing it,
and update the error message to match the code.

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 3c3d0b99d3e8..0e8d88417251 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -806,11 +806,9 @@ static int sun8i_dwmac_power_internal_phy(struct stmmac_priv *priv)
 	/* Make sure the EPHY is properly reseted, as U-Boot may leave
 	 * it at deasserted state, and thus it may fail to reset EMAC.
 	 */
-	reset_control_assert(gmac->rst_ephy);
-
-	ret = reset_control_deassert(gmac->rst_ephy);
+	ret = reset_control_reset(gmac->rst_ephy);
 	if (ret) {
-		dev_err(priv->device, "Cannot deassert internal phy\n");
+		dev_err(priv->device, "Cannot reset internal PHY\n");
 		clk_disable_unprepare(gmac->ephy_clk);
 		return ret;
 	}
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH net-next RESEND 4/5] net: stmmac: dwmac-sun8i: Minor probe function cleanup
  2021-02-08  6:28 ` Samuel Holland
@ 2021-02-08  6:28   ` Samuel Holland
  -1 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:28 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe
  Cc: Ondrej Jirman, netdev, linux-arm-kernel, linux-kernel,
	linux-sunxi, Samuel Holland

Adjust the spacing and use an explicit "return 0" in the success path
to make the function easier to parse.

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 0e8d88417251..4638d4203af5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -1227,6 +1227,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 
 	ndev = dev_get_drvdata(&pdev->dev);
 	priv = netdev_priv(ndev);
+
 	/* The mux must be registered after parent MDIO
 	 * so after stmmac_dvr_probe()
 	 */
@@ -1245,7 +1246,8 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 			goto dwmac_remove;
 	}
 
-	return ret;
+	return 0;
+
 dwmac_mux:
 	reset_control_put(gmac->rst_ephy);
 	clk_put(gmac->ephy_clk);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH net-next RESEND 4/5] net: stmmac: dwmac-sun8i: Minor probe function cleanup
@ 2021-02-08  6:28   ` Samuel Holland
  0 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:28 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe
  Cc: Ondrej Jirman, Samuel Holland, netdev, linux-kernel, linux-sunxi,
	linux-arm-kernel

Adjust the spacing and use an explicit "return 0" in the success path
to make the function easier to parse.

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 0e8d88417251..4638d4203af5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -1227,6 +1227,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 
 	ndev = dev_get_drvdata(&pdev->dev);
 	priv = netdev_priv(ndev);
+
 	/* The mux must be registered after parent MDIO
 	 * so after stmmac_dvr_probe()
 	 */
@@ -1245,7 +1246,8 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 			goto dwmac_remove;
 	}
 
-	return ret;
+	return 0;
+
 dwmac_mux:
 	reset_control_put(gmac->rst_ephy);
 	clk_put(gmac->ephy_clk);
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH net-next RESEND 5/5] net: stmmac: dwmac-sun8i: Add a shutdown callback
  2021-02-08  6:28 ` Samuel Holland
@ 2021-02-08  6:28   ` Samuel Holland
  -1 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:28 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe
  Cc: Ondrej Jirman, netdev, linux-arm-kernel, linux-kernel,
	linux-sunxi, Samuel Holland

The Ethernet MAC and PHY are usually major consumers of power on boards
which may not be able to fully power off (those with no PMIC). Powering
down the MAC and internal PHY saves power while these boards are "off".

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 4638d4203af5..926e8d5e8963 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -1282,6 +1282,15 @@ static int sun8i_dwmac_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static void sun8i_dwmac_shutdown(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
+
+	sun8i_dwmac_exit(pdev, gmac);
+}
+
 static const struct of_device_id sun8i_dwmac_match[] = {
 	{ .compatible = "allwinner,sun8i-h3-emac",
 		.data = &emac_variant_h3 },
@@ -1302,6 +1311,7 @@ MODULE_DEVICE_TABLE(of, sun8i_dwmac_match);
 static struct platform_driver sun8i_dwmac_driver = {
 	.probe  = sun8i_dwmac_probe,
 	.remove = sun8i_dwmac_remove,
+	.shutdown = sun8i_dwmac_shutdown,
 	.driver = {
 		.name           = "dwmac-sun8i",
 		.pm		= &stmmac_pltfr_pm_ops,
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH net-next RESEND 5/5] net: stmmac: dwmac-sun8i: Add a shutdown callback
@ 2021-02-08  6:28   ` Samuel Holland
  0 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:28 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe
  Cc: Ondrej Jirman, Samuel Holland, netdev, linux-kernel, linux-sunxi,
	linux-arm-kernel

The Ethernet MAC and PHY are usually major consumers of power on boards
which may not be able to fully power off (those with no PMIC). Powering
down the MAC and internal PHY saves power while these boards are "off".

Reviewed-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 4638d4203af5..926e8d5e8963 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -1282,6 +1282,15 @@ static int sun8i_dwmac_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static void sun8i_dwmac_shutdown(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
+
+	sun8i_dwmac_exit(pdev, gmac);
+}
+
 static const struct of_device_id sun8i_dwmac_match[] = {
 	{ .compatible = "allwinner,sun8i-h3-emac",
 		.data = &emac_variant_h3 },
@@ -1302,6 +1311,7 @@ MODULE_DEVICE_TABLE(of, sun8i_dwmac_match);
 static struct platform_driver sun8i_dwmac_driver = {
 	.probe  = sun8i_dwmac_probe,
 	.remove = sun8i_dwmac_remove,
+	.shutdown = sun8i_dwmac_shutdown,
 	.driver = {
 		.name           = "dwmac-sun8i",
 		.pm		= &stmmac_pltfr_pm_ops,
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH] i2c: mv64xxx: Fix check for missing clock
  2021-02-08  6:28   ` Samuel Holland
@ 2021-02-08  6:31     ` Samuel Holland
  -1 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:31 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe
  Cc: Ondrej Jirman, netdev, linux-arm-kernel, linux-kernel,
	linux-sunxi, Dan Carpenter

On 2/8/21 12:28 AM, Samuel Holland wrote:
> In commit e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support"), error
> pointers to optional clocks were replaced by NULL to simplify the resume
> callback implementation. However, that commit missed that the IS_ERR
> check in mv64xxx_of_config should be replaced with a NULL check. As a
> result, the check always passes, even for an invalid device tree.

Sorry, please ignore this unrelated patch. I accidentally copied it to
the wrong directory before sending this series.

Samuel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] i2c: mv64xxx: Fix check for missing clock
@ 2021-02-08  6:31     ` Samuel Holland
  0 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:31 UTC (permalink / raw)
  To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe
  Cc: Ondrej Jirman, netdev, linux-kernel, linux-sunxi,
	linux-arm-kernel, Dan Carpenter

On 2/8/21 12:28 AM, Samuel Holland wrote:
> In commit e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support"), error
> pointers to optional clocks were replaced by NULL to simplify the resume
> callback implementation. However, that commit missed that the IS_ERR
> check in mv64xxx_of_config should be replaced with a NULL check. As a
> result, the check always passes, even for an invalid device tree.

Sorry, please ignore this unrelated patch. I accidentally copied it to
the wrong directory before sending this series.

Samuel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] i2c: mv64xxx: Fix check for missing clock
  2021-02-08  6:31     ` Samuel Holland
@ 2021-02-08 13:20       ` Andrew Lunn
  -1 siblings, 0 replies; 28+ messages in thread
From: Andrew Lunn @ 2021-02-08 13:20 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe, Ondrej Jirman, netdev,
	linux-arm-kernel, linux-kernel, linux-sunxi, Dan Carpenter

On Mon, Feb 08, 2021 at 12:31:34AM -0600, Samuel Holland wrote:
> On 2/8/21 12:28 AM, Samuel Holland wrote:
> > In commit e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support"), error
> > pointers to optional clocks were replaced by NULL to simplify the resume
> > callback implementation. However, that commit missed that the IS_ERR
> > check in mv64xxx_of_config should be replaced with a NULL check. As a
> > result, the check always passes, even for an invalid device tree.
> 
> Sorry, please ignore this unrelated patch. I accidentally copied it to
> the wrong directory before sending this series.

Hi Samuel

This patch looks correct. But i don't see it in i2c/for-next, where as
e5c02cf54154 is. I just want to make sure it does not get lost...

	     Andrew

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] i2c: mv64xxx: Fix check for missing clock
@ 2021-02-08 13:20       ` Andrew Lunn
  0 siblings, 0 replies; 28+ messages in thread
From: Andrew Lunn @ 2021-02-08 13:20 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Ondrej Jirman, Jernej Skrabec, Alexandre Torgue, netdev,
	linux-sunxi, linux-kernel, Maxime Ripard, Chen-Yu Tsai,
	Jose Abreu, Corentin Labbe, Jakub Kicinski, Giuseppe Cavallaro,
	Dan Carpenter, David S. Miller, linux-arm-kernel

On Mon, Feb 08, 2021 at 12:31:34AM -0600, Samuel Holland wrote:
> On 2/8/21 12:28 AM, Samuel Holland wrote:
> > In commit e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support"), error
> > pointers to optional clocks were replaced by NULL to simplify the resume
> > callback implementation. However, that commit missed that the IS_ERR
> > check in mv64xxx_of_config should be replaced with a NULL check. As a
> > result, the check always passes, even for an invalid device tree.
> 
> Sorry, please ignore this unrelated patch. I accidentally copied it to
> the wrong directory before sending this series.

Hi Samuel

This patch looks correct. But i don't see it in i2c/for-next, where as
e5c02cf54154 is. I just want to make sure it does not get lost...

	     Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH net-next RESEND 3/5] net: stmmac: dwmac-sun8i: Use reset_control_reset
  2021-02-08  6:28   ` Samuel Holland
@ 2021-02-08 16:29     ` Alexander Duyck
  -1 siblings, 0 replies; 28+ messages in thread
From: Alexander Duyck @ 2021-02-08 16:29 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe, Ondrej Jirman, Netdev,
	linux-arm-kernel, LKML, linux-sunxi

On Sun, Feb 7, 2021 at 10:32 PM Samuel Holland <samuel@sholland.org> wrote:
>
> Use the appropriate function instead of reimplementing it,
> and update the error message to match the code.
>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index 3c3d0b99d3e8..0e8d88417251 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> @@ -806,11 +806,9 @@ static int sun8i_dwmac_power_internal_phy(struct stmmac_priv *priv)
>         /* Make sure the EPHY is properly reseted, as U-Boot may leave
>          * it at deasserted state, and thus it may fail to reset EMAC.
>          */
> -       reset_control_assert(gmac->rst_ephy);
> -
> -       ret = reset_control_deassert(gmac->rst_ephy);
> +       ret = reset_control_reset(gmac->rst_ephy);
>         if (ret) {
> -               dev_err(priv->device, "Cannot deassert internal phy\n");
> +               dev_err(priv->device, "Cannot reset internal PHY\n");
>                 clk_disable_unprepare(gmac->ephy_clk);
>                 return ret;
>         }

I'm assuming you have exclusive access to the phy and this isn't a
shared line? Just wanting to confirm since the function call has the
following comment in the header for the documentation.

 * Consumers must not use reset_control_(de)assert on shared reset lines when
 * reset_control_reset has been used.
 *

If that is the case it might not hurt to add some documentation to
your call to reset_control_reset here explaining that it is safe to do
so since you have exclusive access.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH net-next RESEND 3/5] net: stmmac: dwmac-sun8i: Use reset_control_reset
@ 2021-02-08 16:29     ` Alexander Duyck
  0 siblings, 0 replies; 28+ messages in thread
From: Alexander Duyck @ 2021-02-08 16:29 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Ondrej Jirman, Jernej Skrabec, Alexandre Torgue, Netdev,
	linux-sunxi, LKML, Maxime Ripard, Chen-Yu Tsai, Jose Abreu,
	Corentin Labbe, Jakub Kicinski, Giuseppe Cavallaro,
	David S. Miller, linux-arm-kernel

On Sun, Feb 7, 2021 at 10:32 PM Samuel Holland <samuel@sholland.org> wrote:
>
> Use the appropriate function instead of reimplementing it,
> and update the error message to match the code.
>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index 3c3d0b99d3e8..0e8d88417251 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> @@ -806,11 +806,9 @@ static int sun8i_dwmac_power_internal_phy(struct stmmac_priv *priv)
>         /* Make sure the EPHY is properly reseted, as U-Boot may leave
>          * it at deasserted state, and thus it may fail to reset EMAC.
>          */
> -       reset_control_assert(gmac->rst_ephy);
> -
> -       ret = reset_control_deassert(gmac->rst_ephy);
> +       ret = reset_control_reset(gmac->rst_ephy);
>         if (ret) {
> -               dev_err(priv->device, "Cannot deassert internal phy\n");
> +               dev_err(priv->device, "Cannot reset internal PHY\n");
>                 clk_disable_unprepare(gmac->ephy_clk);
>                 return ret;
>         }

I'm assuming you have exclusive access to the phy and this isn't a
shared line? Just wanting to confirm since the function call has the
following comment in the header for the documentation.

 * Consumers must not use reset_control_(de)assert on shared reset lines when
 * reset_control_reset has been used.
 *

If that is the case it might not hurt to add some documentation to
your call to reset_control_reset here explaining that it is safe to do
so since you have exclusive access.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] i2c: mv64xxx: Fix check for missing clock
  2021-02-08  6:31     ` Samuel Holland
@ 2021-02-08 21:11       ` Jakub Kicinski
  -1 siblings, 0 replies; 28+ messages in thread
From: Jakub Kicinski @ 2021-02-08 21:11 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Maxime Ripard, Chen-Yu Tsai, Jernej Skrabec,
	Corentin Labbe, Ondrej Jirman, netdev, linux-arm-kernel,
	linux-kernel, linux-sunxi, Dan Carpenter

On Mon, 8 Feb 2021 00:31:34 -0600 Samuel Holland wrote:
> On 2/8/21 12:28 AM, Samuel Holland wrote:
> > In commit e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support"), error
> > pointers to optional clocks were replaced by NULL to simplify the resume
> > callback implementation. However, that commit missed that the IS_ERR
> > check in mv64xxx_of_config should be replaced with a NULL check. As a
> > result, the check always passes, even for an invalid device tree.  
> 
> Sorry, please ignore this unrelated patch. I accidentally copied it to
> the wrong directory before sending this series.

Unfortunately patchwork decided to take this patch in instead of the
real 1/5 patch. Please make a clean repost even if there are no review
comments to address.

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] i2c: mv64xxx: Fix check for missing clock
@ 2021-02-08 21:11       ` Jakub Kicinski
  0 siblings, 0 replies; 28+ messages in thread
From: Jakub Kicinski @ 2021-02-08 21:11 UTC (permalink / raw)
  To: Samuel Holland
  Cc: Ondrej Jirman, Jernej Skrabec, Alexandre Torgue, netdev,
	linux-sunxi, linux-kernel, Maxime Ripard, Chen-Yu Tsai,
	Jose Abreu, Corentin Labbe, Giuseppe Cavallaro, Dan Carpenter,
	David S. Miller, linux-arm-kernel

On Mon, 8 Feb 2021 00:31:34 -0600 Samuel Holland wrote:
> On 2/8/21 12:28 AM, Samuel Holland wrote:
> > In commit e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support"), error
> > pointers to optional clocks were replaced by NULL to simplify the resume
> > callback implementation. However, that commit missed that the IS_ERR
> > check in mv64xxx_of_config should be replaced with a NULL check. As a
> > result, the check always passes, even for an invalid device tree.  
> 
> Sorry, please ignore this unrelated patch. I accidentally copied it to
> the wrong directory before sending this series.

Unfortunately patchwork decided to take this patch in instead of the
real 1/5 patch. Please make a clean repost even if there are no review
comments to address.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] i2c: mv64xxx: Fix check for missing clock
  2021-02-08 13:20       ` Andrew Lunn
@ 2021-02-09  3:05         ` Samuel Holland
  -1 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-09  3:05 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe, Ondrej Jirman, netdev,
	linux-arm-kernel, linux-kernel, linux-sunxi, Dan Carpenter

On 2/8/21 7:20 AM, Andrew Lunn wrote:
> On Mon, Feb 08, 2021 at 12:31:34AM -0600, Samuel Holland wrote:
>> On 2/8/21 12:28 AM, Samuel Holland wrote:
>>> In commit e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support"), error
>>> pointers to optional clocks were replaced by NULL to simplify the resume
>>> callback implementation. However, that commit missed that the IS_ERR
>>> check in mv64xxx_of_config should be replaced with a NULL check. As a
>>> result, the check always passes, even for an invalid device tree.
>>
>> Sorry, please ignore this unrelated patch. I accidentally copied it to
>> the wrong directory before sending this series.
> 
> Hi Samuel
> 
> This patch looks correct. But i don't see it in i2c/for-next, where as
> e5c02cf54154 is. I just want to make sure it does not get lost...

Thanks for the concern. I had already sent it separately[0], to the
appropriate maintainers, so this submission was a duplicate.

Cheers,
Samuel

[0]:
https://lore.kernel.org/lkml/20210208061922.10073-1-samuel@sholland.org/

> 	     Andrew
> 


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] i2c: mv64xxx: Fix check for missing clock
@ 2021-02-09  3:05         ` Samuel Holland
  0 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-09  3:05 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Ondrej Jirman, Jernej Skrabec, Alexandre Torgue, netdev,
	linux-sunxi, linux-kernel, Maxime Ripard, Chen-Yu Tsai,
	Jose Abreu, Corentin Labbe, Jakub Kicinski, Giuseppe Cavallaro,
	Dan Carpenter, David S. Miller, linux-arm-kernel

On 2/8/21 7:20 AM, Andrew Lunn wrote:
> On Mon, Feb 08, 2021 at 12:31:34AM -0600, Samuel Holland wrote:
>> On 2/8/21 12:28 AM, Samuel Holland wrote:
>>> In commit e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support"), error
>>> pointers to optional clocks were replaced by NULL to simplify the resume
>>> callback implementation. However, that commit missed that the IS_ERR
>>> check in mv64xxx_of_config should be replaced with a NULL check. As a
>>> result, the check always passes, even for an invalid device tree.
>>
>> Sorry, please ignore this unrelated patch. I accidentally copied it to
>> the wrong directory before sending this series.
> 
> Hi Samuel
> 
> This patch looks correct. But i don't see it in i2c/for-next, where as
> e5c02cf54154 is. I just want to make sure it does not get lost...

Thanks for the concern. I had already sent it separately[0], to the
appropriate maintainers, so this submission was a duplicate.

Cheers,
Samuel

[0]:
https://lore.kernel.org/lkml/20210208061922.10073-1-samuel@sholland.org/

> 	     Andrew
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH net-next RESEND 3/5] net: stmmac: dwmac-sun8i: Use reset_control_reset
  2021-02-08 16:29     ` Alexander Duyck
@ 2021-02-09  3:24       ` Samuel Holland
  -1 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-09  3:24 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Jakub Kicinski, Maxime Ripard, Chen-Yu Tsai,
	Jernej Skrabec, Corentin Labbe, Ondrej Jirman, Netdev,
	linux-arm-kernel, LKML, linux-sunxi

On 2/8/21 10:29 AM, Alexander Duyck wrote:
> On Sun, Feb 7, 2021 at 10:32 PM Samuel Holland <samuel@sholland.org> wrote:
>>
>> Use the appropriate function instead of reimplementing it,
>> and update the error message to match the code.
>>
>> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>> index 3c3d0b99d3e8..0e8d88417251 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>> @@ -806,11 +806,9 @@ static int sun8i_dwmac_power_internal_phy(struct stmmac_priv *priv)
>>         /* Make sure the EPHY is properly reseted, as U-Boot may leave
>>          * it at deasserted state, and thus it may fail to reset EMAC.
>>          */
>> -       reset_control_assert(gmac->rst_ephy);
>> -
>> -       ret = reset_control_deassert(gmac->rst_ephy);
>> +       ret = reset_control_reset(gmac->rst_ephy);
>>         if (ret) {
>> -               dev_err(priv->device, "Cannot deassert internal phy\n");
>> +               dev_err(priv->device, "Cannot reset internal PHY\n");
>>                 clk_disable_unprepare(gmac->ephy_clk);
>>                 return ret;
>>         }
> 
> I'm assuming you have exclusive access to the phy and this isn't a
> shared line? Just wanting to confirm since the function call has the
> following comment in the header for the documentation.

Yes, this driver has exclusive access:

	gmac->rst_ephy = of_reset_control_get_exclusive(iphynode, NULL);

And this is a reset line for the Ethernet PHY inside the SoC, that as
far as I can tell is not shared with anything else.

>  * Consumers must not use reset_control_(de)assert on shared reset lines when
>  * reset_control_reset has been used.
>  *
> 
> If that is the case it might not hurt to add some documentation to
> your call to reset_control_reset here explaining that it is safe to do
> so since you have exclusive access.

I can expand the comment above this line for v2.

Cheers,
Samuel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH net-next RESEND 3/5] net: stmmac: dwmac-sun8i: Use reset_control_reset
@ 2021-02-09  3:24       ` Samuel Holland
  0 siblings, 0 replies; 28+ messages in thread
From: Samuel Holland @ 2021-02-09  3:24 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Ondrej Jirman, Jernej Skrabec, Alexandre Torgue, Netdev,
	linux-sunxi, LKML, Maxime Ripard, Chen-Yu Tsai, Jose Abreu,
	Corentin Labbe, Jakub Kicinski, Giuseppe Cavallaro,
	David S. Miller, linux-arm-kernel

On 2/8/21 10:29 AM, Alexander Duyck wrote:
> On Sun, Feb 7, 2021 at 10:32 PM Samuel Holland <samuel@sholland.org> wrote:
>>
>> Use the appropriate function instead of reimplementing it,
>> and update the error message to match the code.
>>
>> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>> ---
>>  drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>> index 3c3d0b99d3e8..0e8d88417251 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>> @@ -806,11 +806,9 @@ static int sun8i_dwmac_power_internal_phy(struct stmmac_priv *priv)
>>         /* Make sure the EPHY is properly reseted, as U-Boot may leave
>>          * it at deasserted state, and thus it may fail to reset EMAC.
>>          */
>> -       reset_control_assert(gmac->rst_ephy);
>> -
>> -       ret = reset_control_deassert(gmac->rst_ephy);
>> +       ret = reset_control_reset(gmac->rst_ephy);
>>         if (ret) {
>> -               dev_err(priv->device, "Cannot deassert internal phy\n");
>> +               dev_err(priv->device, "Cannot reset internal PHY\n");
>>                 clk_disable_unprepare(gmac->ephy_clk);
>>                 return ret;
>>         }
> 
> I'm assuming you have exclusive access to the phy and this isn't a
> shared line? Just wanting to confirm since the function call has the
> following comment in the header for the documentation.

Yes, this driver has exclusive access:

	gmac->rst_ephy = of_reset_control_get_exclusive(iphynode, NULL);

And this is a reset line for the Ethernet PHY inside the SoC, that as
far as I can tell is not shared with anything else.

>  * Consumers must not use reset_control_(de)assert on shared reset lines when
>  * reset_control_reset has been used.
>  *
> 
> If that is the case it might not hurt to add some documentation to
> your call to reset_control_reset here explaining that it is safe to do
> so since you have exclusive access.

I can expand the comment above this line for v2.

Cheers,
Samuel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH] i2c: mv64xxx: Fix check for missing clock
  2021-02-08  6:19 [PATCH] i2c: mv64xxx: Fix check for missing clock Samuel Holland
@ 2021-02-09 10:42 ` Wolfram Sang
  0 siblings, 0 replies; 28+ messages in thread
From: Wolfram Sang @ 2021-02-09 10:42 UTC (permalink / raw)
  To: Samuel Holland; +Cc: Gregory CLEMENT, linux-i2c, linux-kernel, Dan Carpenter

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

On Mon, Feb 08, 2021 at 12:19:22AM -0600, Samuel Holland wrote:
> In commit e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support"), error
> pointers to optional clocks were replaced by NULL to simplify the resume
> callback implementation. However, that commit missed that the IS_ERR
> check in mv64xxx_of_config should be replaced with a NULL check. As a
> result, the check always passes, even for an invalid device tree.
> 
> Fixes: e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Samuel Holland <samuel@sholland.org>

Added "RPM" to $subject and applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH] i2c: mv64xxx: Fix check for missing clock
@ 2021-02-08  6:19 Samuel Holland
  2021-02-09 10:42 ` Wolfram Sang
  0 siblings, 1 reply; 28+ messages in thread
From: Samuel Holland @ 2021-02-08  6:19 UTC (permalink / raw)
  To: Gregory CLEMENT, Wolfram Sang, linux-i2c
  Cc: linux-kernel, Samuel Holland, Dan Carpenter

In commit e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support"), error
pointers to optional clocks were replaced by NULL to simplify the resume
callback implementation. However, that commit missed that the IS_ERR
check in mv64xxx_of_config should be replaced with a NULL check. As a
result, the check always passes, even for an invalid device tree.

Fixes: e5c02cf54154 ("i2c: mv64xxx: Add runtime PM support")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/i2c/busses/i2c-mv64xxx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index b03c344323d1..c590d36b5fd1 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -813,7 +813,7 @@ mv64xxx_of_config(struct mv64xxx_i2c_data *drv_data,
 	 * need to know tclk in order to calculate bus clock
 	 * factors.
 	 */
-	if (IS_ERR(drv_data->clk)) {
+	if (!drv_data->clk) {
 		rc = -ENODEV;
 		goto out;
 	}
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2021-02-09 10:52 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-08  6:28 [PATCH net-next RESEND 0/5] dwmac-sun8i cleanup and shutdown hook Samuel Holland
2021-02-08  6:28 ` Samuel Holland
2021-02-08  6:28 ` [PATCH] i2c: mv64xxx: Fix check for missing clock Samuel Holland
2021-02-08  6:28   ` Samuel Holland
2021-02-08  6:31   ` Samuel Holland
2021-02-08  6:31     ` Samuel Holland
2021-02-08 13:20     ` Andrew Lunn
2021-02-08 13:20       ` Andrew Lunn
2021-02-09  3:05       ` Samuel Holland
2021-02-09  3:05         ` Samuel Holland
2021-02-08 21:11     ` Jakub Kicinski
2021-02-08 21:11       ` Jakub Kicinski
2021-02-08  6:28 ` [PATCH net-next RESEND 1/5] net: stmmac: dwmac-sun8i: Return void from PHY unpower Samuel Holland
2021-02-08  6:28   ` Samuel Holland
2021-02-08  6:28 ` [PATCH net-next RESEND 2/5] net: stmmac: dwmac-sun8i: Remove unnecessary PHY power check Samuel Holland
2021-02-08  6:28   ` Samuel Holland
2021-02-08  6:28 ` [PATCH net-next RESEND 3/5] net: stmmac: dwmac-sun8i: Use reset_control_reset Samuel Holland
2021-02-08  6:28   ` Samuel Holland
2021-02-08 16:29   ` Alexander Duyck
2021-02-08 16:29     ` Alexander Duyck
2021-02-09  3:24     ` Samuel Holland
2021-02-09  3:24       ` Samuel Holland
2021-02-08  6:28 ` [PATCH net-next RESEND 4/5] net: stmmac: dwmac-sun8i: Minor probe function cleanup Samuel Holland
2021-02-08  6:28   ` Samuel Holland
2021-02-08  6:28 ` [PATCH net-next RESEND 5/5] net: stmmac: dwmac-sun8i: Add a shutdown callback Samuel Holland
2021-02-08  6:28   ` Samuel Holland
  -- strict thread matches above, loose matches on Subject: below --
2021-02-08  6:19 [PATCH] i2c: mv64xxx: Fix check for missing clock Samuel Holland
2021-02-09 10:42 ` Wolfram Sang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.