[PATCH net-next 0/2] Fixups for qca8k ds->user_mii_bus cleanup

[PATCH net-next 0/2] Fixups for qca8k ds->user_mii_bus cleanup

[Vladimir Oltean]

The series "ds->user_mii_bus cleanup (part 1)" from the last development
cycle:
https://patchwork.kernel.org/project/netdevbpf/cover/20240104140037.374166-1-vladimir.oltean@nxp.com/

had some review comments I didn't have the time to address at the time.
One from Alvin and one from Luiz. They can reasonably be treated as
improvements for v6.9.

Vladimir Oltean (2):
  net: dsa: qca8k: put MDIO controller OF node if unavailable
  net: dsa: qca8k: consistently use "ret" rather than "err" for error
    codes

 drivers/net/dsa/qca/qca8k-8xxx.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

-- 
2.34.1

[PATCH net-next 1/2] net: dsa: qca8k: put MDIO controller OF node if unavailable

[Vladimir Oltean]

It was pointed out during the review [1] of commit e66bf63a7f67 ("net:
dsa: qca8k: skip MDIO bus creation if its OF node has status =
"disabled"") that we now leak a reference to the "mdio" OF node if it is
disabled.

This is only a concern when using dynamic OF as far as I can tell (like
probing on an overlay), since OF nodes are never freed in the regular
case. Additionally, I'm unaware of any actual device trees (in
production or elsewhere) which have status = "disabled" for the MDIO OF
node. So handling this as a simple enhancement.

[1] https://lore.kernel.org/netdev/CAJq09z4--Ug+3FAmp=EimQ8HTQYOWOuVon-PUMGB5a1N=RPv4g@mail.gmail.com/

Suggested-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/qca/qca8k-8xxx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 7a864329cb72..95d78b3181d1 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -954,7 +954,7 @@ qca8k_mdio_register(struct qca8k_priv *priv)
 
 	mdio = of_get_child_by_name(dev->of_node, "mdio");
 	if (mdio && !of_device_is_available(mdio))
-		goto out;
+		goto out_put_node;
 
 	bus = devm_mdiobus_alloc(dev);
 	if (!bus) {
@@ -988,7 +988,6 @@ qca8k_mdio_register(struct qca8k_priv *priv)
 
 out_put_node:
 	of_node_put(mdio);
-out:
 	return err;
 }
 
-- 
2.34.1

[PATCH net-next 2/2] net: dsa: qca8k: consistently use "ret" rather than "err" for error codes

[Vladimir Oltean]

It was pointed out during the review [1] of commit 68e1010cda79 ("net:
dsa: qca8k: put MDIO bus OF node on qca8k_mdio_register() failure") that
the rest of the qca8k driver uses "int ret" rather than "int err".

Make everything consistent in that regard, not only
qca8k_mdio_register(), but also qca8k_setup_mdio_bus().

[1] https://lore.kernel.org/netdev/qyl2w3ownx5q7363kqxib52j5htar4y6pkn7gen27rj45xr4on@pvy5agi6o2te/

Suggested-by: Alvin Šipraga <alsi@bang-olufsen.dk>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/dsa/qca/qca8k-8xxx.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 95d78b3181d1..dab66c0c6f64 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -950,7 +950,7 @@ qca8k_mdio_register(struct qca8k_priv *priv)
 	struct device *dev = ds->dev;
 	struct device_node *mdio;
 	struct mii_bus *bus;
-	int err = 0;
+	int ret = 0;
 
 	mdio = of_get_child_by_name(dev->of_node, "mdio");
 	if (mdio && !of_device_is_available(mdio))
@@ -958,7 +958,7 @@ qca8k_mdio_register(struct qca8k_priv *priv)
 
 	bus = devm_mdiobus_alloc(dev);
 	if (!bus) {
-		err = -ENOMEM;
+		ret = -ENOMEM;
 		goto out_put_node;
 	}
 
@@ -984,11 +984,11 @@ qca8k_mdio_register(struct qca8k_priv *priv)
 		bus->write = qca8k_legacy_mdio_write;
 	}
 
-	err = devm_of_mdiobus_register(dev, bus, mdio);
+	ret = devm_of_mdiobus_register(dev, bus, mdio);
 
 out_put_node:
 	of_node_put(mdio);
-	return err;
+	return ret;
 }
 
 static int
@@ -997,7 +997,7 @@ qca8k_setup_mdio_bus(struct qca8k_priv *priv)
 	u32 internal_mdio_mask = 0, external_mdio_mask = 0, reg;
 	struct device_node *ports, *port;
 	phy_interface_t mode;
-	int err;
+	int ret;
 
 	ports = of_get_child_by_name(priv->dev->of_node, "ports");
 	if (!ports)
@@ -1007,11 +1007,11 @@ qca8k_setup_mdio_bus(struct qca8k_priv *priv)
 		return -EINVAL;
 
 	for_each_available_child_of_node(ports, port) {
-		err = of_property_read_u32(port, "reg", &reg);
-		if (err) {
+		ret = of_property_read_u32(port, "reg", &reg);
+		if (ret) {
 			of_node_put(port);
 			of_node_put(ports);
-			return err;
+			return ret;
 		}
 
 		if (!dsa_is_user_port(priv->ds, reg))
-- 
2.34.1

Re: [PATCH net-next 2/2] net: dsa: qca8k: consistently use "ret" rather than "err" for error codes

[Andrew Lunn]

On Fri, Feb 02, 2024 at 06:36:26PM +0200, Vladimir Oltean wrote:
> It was pointed out during the review [1] of commit 68e1010cda79 ("net:
> dsa: qca8k: put MDIO bus OF node on qca8k_mdio_register() failure") that
> the rest of the qca8k driver uses "int ret" rather than "int err".
> 
> Make everything consistent in that regard, not only
> qca8k_mdio_register(), but also qca8k_setup_mdio_bus().
> 
> [1] https://lore.kernel.org/netdev/qyl2w3ownx5q7363kqxib52j5htar4y6pkn7gen27rj45xr4on@pvy5agi6o2te/
> 
> Suggested-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 2/2] net: dsa: qca8k: consistently use "ret" rather than "err" for error codes

[Christian Marangi]

On Fri, Feb 02, 2024 at 06:36:26PM +0200, Vladimir Oltean wrote:
> It was pointed out during the review [1] of commit 68e1010cda79 ("net:
> dsa: qca8k: put MDIO bus OF node on qca8k_mdio_register() failure") that
> the rest of the qca8k driver uses "int ret" rather than "int err".
> 
> Make everything consistent in that regard, not only
> qca8k_mdio_register(), but also qca8k_setup_mdio_bus().
> 
> [1] https://lore.kernel.org/netdev/qyl2w3ownx5q7363kqxib52j5htar4y6pkn7gen27rj45xr4on@pvy5agi6o2te/
> 
> Suggested-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Christian Marangi <ansuelsmth@gmail.com>

-- 
	Ansuel

Re: [PATCH net-next 0/2] Fixups for qca8k ds->user_mii_bus cleanup

[patchwork-bot+netdevbpf]

Hello:

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

On Fri,  2 Feb 2024 18:36:24 +0200 you wrote:
> The series "ds->user_mii_bus cleanup (part 1)" from the last development
> cycle:
> https://patchwork.kernel.org/project/netdevbpf/cover/20240104140037.374166-1-vladimir.oltean@nxp.com/
> 
> had some review comments I didn't have the time to address at the time.
> One from Alvin and one from Luiz. They can reasonably be treated as
> improvements for v6.9.
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] net: dsa: qca8k: put MDIO controller OF node if unavailable
    https://git.kernel.org/netdev/net-next/c/08932323ccf7
  - [net-next,2/2] net: dsa: qca8k: consistently use "ret" rather than "err" for error codes
    https://git.kernel.org/netdev/net-next/c/709776ea8562

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