[PATCH net-next v3 0/2] net: dsa: realtek: realtek-mdio: reset before setup

[PATCH net-next v3 0/2] net: dsa: realtek: realtek-mdio: reset before setup

[Luiz Angelo Daros de Luca]

This patch series cleans the realtek-smi reset code and copy that to the
realtek-mdio.

v1-v2)
- do not run reset code block if GPIO is missing. It was printing "RESET
  deasserted" even when there is no GPIO configured.
- reset switch after dsa_unregister_switch()
- demote reset messages to debug

v2-v3)
- do not assert the reset on gpiod_get. Do it explicitly aferwards.
- split the commit into two (one for each module)

[PATCH net-next v3 1/2] net: dsa: realtek: realtek-smi: clean-up reset

[Luiz Angelo Daros de Luca]

When reset GPIO was missing, the driver was still printing an info
message and still trying to assert the reset. Although gpiod_set_value()
will silently ignore calls with NULL gpio_desc, it is better to make it
clear the driver might allow gpio_desc to be NULL.

The initial value for the reset pin was changed to GPIOD_OUT_LOW,
followed by a gpiod_set_value() asserting the reset. This way, it will
be easier to spot if and where the reset really happens.

A new "asserted RESET" message was added just after the reset is
asserted, similar to the existing "deasserted RESET" message. Both
messages were demoted to dbg. The code comment is not needed anymore.

Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
---
 drivers/net/dsa/realtek/realtek-smi.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c
index 946fbbd70153..33cf5a0692de 100644
--- a/drivers/net/dsa/realtek/realtek-smi.c
+++ b/drivers/net/dsa/realtek/realtek-smi.c
@@ -420,16 +420,19 @@ static int realtek_smi_probe(struct platform_device *pdev)
 
 	/* TODO: if power is software controlled, set up any regulators here */
 
-	/* Assert then deassert RESET */
-	priv->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+	priv->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
 	if (IS_ERR(priv->reset)) {
 		dev_err(dev, "failed to get RESET GPIO\n");
 		return PTR_ERR(priv->reset);
 	}
-	msleep(REALTEK_SMI_HW_STOP_DELAY);
-	gpiod_set_value(priv->reset, 0);
-	msleep(REALTEK_SMI_HW_START_DELAY);
-	dev_info(dev, "deasserted RESET\n");
+	if (priv->reset) {
+		gpiod_set_value(priv->reset, 1);
+		dev_dbg(dev, "asserted RESET\n");
+		msleep(REALTEK_SMI_HW_STOP_DELAY);
+		gpiod_set_value(priv->reset, 0);
+		msleep(REALTEK_SMI_HW_START_DELAY);
+		dev_dbg(dev, "deasserted RESET\n");
+	}
 
 	/* Fetch MDIO pins */
 	priv->mdc = devm_gpiod_get_optional(dev, "mdc", GPIOD_OUT_LOW);
@@ -474,7 +477,10 @@ static int realtek_smi_remove(struct platform_device *pdev)
 	dsa_unregister_switch(priv->ds);
 	if (priv->slave_mii_bus)
 		of_node_put(priv->slave_mii_bus->dev.of_node);
-	gpiod_set_value(priv->reset, 1);
+
+	/* leave the device reset asserted */
+	if (priv->reset)
+		gpiod_set_value(priv->reset, 1);
 
 	platform_set_drvdata(pdev, NULL);
 
-- 
2.35.1

[PATCH net-next v3 2/2] net: dsa: realtek: realtek-mdio: reset before setup

[Luiz Angelo Daros de Luca]

Some devices, like the switch in Banana Pi BPI R64 only starts to answer
after a HW reset. It is the same reset code from realtek-smi.

Reported-by: Frank Wunderlich <frank-w@public-files.de>
Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
Tested-by: Frank Wunderlich <frank-w@public-files.de>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
 drivers/net/dsa/realtek/realtek-mdio.c | 19 +++++++++++++++++++
 drivers/net/dsa/realtek/realtek-smi.c  |  6 ++----
 drivers/net/dsa/realtek/realtek.h      |  3 +++
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c
index 0c5f2bdced9d..0308be95d00a 100644
--- a/drivers/net/dsa/realtek/realtek-mdio.c
+++ b/drivers/net/dsa/realtek/realtek-mdio.c
@@ -152,6 +152,21 @@ static int realtek_mdio_probe(struct mdio_device *mdiodev)
 	/* TODO: if power is software controlled, set up any regulators here */
 	priv->leds_disabled = of_property_read_bool(np, "realtek,disable-leds");
 
+	priv->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(priv->reset)) {
+		dev_err(dev, "failed to get RESET GPIO\n");
+		return PTR_ERR(priv->reset);
+	}
+
+	if (priv->reset) {
+		gpiod_set_value(priv->reset, 1);
+		dev_dbg(dev, "asserted RESET\n");
+		msleep(REALTEK_HW_STOP_DELAY);
+		gpiod_set_value(priv->reset, 0);
+		msleep(REALTEK_HW_START_DELAY);
+		dev_dbg(dev, "deasserted RESET\n");
+	}
+
 	ret = priv->ops->detect(priv);
 	if (ret) {
 		dev_err(dev, "unable to detect switch\n");
@@ -185,6 +200,10 @@ static void realtek_mdio_remove(struct mdio_device *mdiodev)
 
 	dsa_unregister_switch(priv->ds);
 
+	/* leave the device reset asserted */
+	if (priv->reset)
+		gpiod_set_value(priv->reset, 1);
+
 	dev_set_drvdata(&mdiodev->dev, NULL);
 }
 
diff --git a/drivers/net/dsa/realtek/realtek-smi.c b/drivers/net/dsa/realtek/realtek-smi.c
index 33cf5a0692de..8806b74bd7a8 100644
--- a/drivers/net/dsa/realtek/realtek-smi.c
+++ b/drivers/net/dsa/realtek/realtek-smi.c
@@ -43,8 +43,6 @@
 #include "realtek.h"
 
 #define REALTEK_SMI_ACK_RETRY_COUNT		5
-#define REALTEK_SMI_HW_STOP_DELAY		25	/* msecs */
-#define REALTEK_SMI_HW_START_DELAY		100	/* msecs */
 
 static inline void realtek_smi_clk_delay(struct realtek_priv *priv)
 {
@@ -428,9 +426,9 @@ static int realtek_smi_probe(struct platform_device *pdev)
 	if (priv->reset) {
 		gpiod_set_value(priv->reset, 1);
 		dev_dbg(dev, "asserted RESET\n");
-		msleep(REALTEK_SMI_HW_STOP_DELAY);
+		msleep(REALTEK_HW_STOP_DELAY);
 		gpiod_set_value(priv->reset, 0);
-		msleep(REALTEK_SMI_HW_START_DELAY);
+		msleep(REALTEK_HW_START_DELAY);
 		dev_dbg(dev, "deasserted RESET\n");
 	}
 
diff --git a/drivers/net/dsa/realtek/realtek.h b/drivers/net/dsa/realtek/realtek.h
index ed5abf6cb3d6..443cf51cb918 100644
--- a/drivers/net/dsa/realtek/realtek.h
+++ b/drivers/net/dsa/realtek/realtek.h
@@ -13,6 +13,9 @@
 #include <linux/gpio/consumer.h>
 #include <net/dsa.h>
 
+#define REALTEK_HW_STOP_DELAY		25	/* msecs */
+#define REALTEK_HW_START_DELAY		100	/* msecs */
+
 struct realtek_ops;
 struct dentry;
 struct inode;
-- 
2.35.1

Re: [PATCH net-next v3 1/2] net: dsa: realtek: realtek-smi: clean-up reset

[Arınç ÜNAL]

On 14/02/2022 05:20, Luiz Angelo Daros de Luca wrote:
> When reset GPIO was missing, the driver was still printing an info
> message and still trying to assert the reset. Although gpiod_set_value()
> will silently ignore calls with NULL gpio_desc, it is better to make it
> clear the driver might allow gpio_desc to be NULL.
> 
> The initial value for the reset pin was changed to GPIOD_OUT_LOW,
> followed by a gpiod_set_value() asserting the reset. This way, it will
> be easier to spot if and where the reset really happens.
> 
> A new "asserted RESET" message was added just after the reset is
> asserted, similar to the existing "deasserted RESET" message. Both
> messages were demoted to dbg. The code comment is not needed anymore.
> 
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>

Acked-by: Arınç ÜNAL <arinc.unal@arinc9.com>

Arınç

Re: [PATCH net-next v3 2/2] net: dsa: realtek: realtek-mdio: reset before setup

[Arınç ÜNAL]

On 14/02/2022 05:20, Luiz Angelo Daros de Luca wrote:
> Some devices, like the switch in Banana Pi BPI R64 only starts to answer
> after a HW reset. It is the same reset code from realtek-smi.
> 
> Reported-by: Frank Wunderlich <frank-w@public-files.de>
> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
> Tested-by: Frank Wunderlich <frank-w@public-files.de>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk>

Acked-by: Arınç ÜNAL <arinc.unal@arinc9.com>

Arınç

Re: [PATCH net-next v3 0/2] net: dsa: realtek: realtek-mdio: reset before setup

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Sun, 13 Feb 2022 23:20:10 -0300 you wrote:
> This patch series cleans the realtek-smi reset code and copy that to the
> realtek-mdio.
> 
> v1-v2)
> - do not run reset code block if GPIO is missing. It was printing "RESET
>   deasserted" even when there is no GPIO configured.
> - reset switch after dsa_unregister_switch()
> - demote reset messages to debug
> 
> [...]

Here is the summary with links:
  - [net-next,v3,1/2] net: dsa: realtek: realtek-smi: clean-up reset
    https://git.kernel.org/netdev/net-next/c/9a236b543f6b
  - [net-next,v3,2/2] net: dsa: realtek: realtek-mdio: reset before setup
    https://git.kernel.org/netdev/net-next/c/05f7b042c5a6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html