[PATCH net] net: dsa: sja1105: Fix parameters order in sja1110_pcs_mdio_write_c45()

[PATCH net] net: dsa: sja1105: Fix parameters order in sja1110_pcs_mdio_write_c45()

[Christophe JAILLET]

The definition and declaration of sja1110_pcs_mdio_write_c45() don't have
parameters in the same order.

Knowing that sja1110_pcs_mdio_write_c45() is used as a function pointer
in 'sja1105_info' structure with .pcs_mdio_write_c45, and that we have:

   int (*pcs_mdio_write_c45)(struct mii_bus *bus, int phy, int mmd,
				  int reg, u16 val);

it is likely that the definition is the one to change.

Found with cppcheck, funcArgOrderDifferent.

Fixes: ae271547bba6 ("net: dsa: sja1105: C45 only transactions for PCS")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested only.
---
 drivers/net/dsa/sja1105/sja1105_mdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_mdio.c b/drivers/net/dsa/sja1105/sja1105_mdio.c
index 833e55e4b961..52ddb4ef259e 100644
--- a/drivers/net/dsa/sja1105/sja1105_mdio.c
+++ b/drivers/net/dsa/sja1105/sja1105_mdio.c
@@ -94,7 +94,7 @@ int sja1110_pcs_mdio_read_c45(struct mii_bus *bus, int phy, int mmd, int reg)
 	return tmp & 0xffff;
 }
 
-int sja1110_pcs_mdio_write_c45(struct mii_bus *bus, int phy, int reg, int mmd,
+int sja1110_pcs_mdio_write_c45(struct mii_bus *bus, int phy, int mmd, int reg,
 			       u16 val)
 {
 	struct sja1105_mdio_private *mdio_priv = bus->priv;
-- 
2.44.0

Re: [PATCH net] net: dsa: sja1105: Fix parameters order in sja1110_pcs_mdio_write_c45()

[Michael Walle]

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

On Tue Apr 2, 2024 at 8:33 PM CEST, Christophe JAILLET wrote:
> The definition and declaration of sja1110_pcs_mdio_write_c45() don't have
> parameters in the same order.
>
> Knowing that sja1110_pcs_mdio_write_c45() is used as a function pointer
> in 'sja1105_info' structure with .pcs_mdio_write_c45, and that we have:
>
>    int (*pcs_mdio_write_c45)(struct mii_bus *bus, int phy, int mmd,
> 				  int reg, u16 val);
>
> it is likely that the definition is the one to change.

See also "struct mii_bus":

	/** @write_c45: Perform a C45 write transfer on the bus */
	int (*write_c45)(struct mii_bus *bus, int addr, int devnum,
			 int regnum, u16 val);

>
> Found with cppcheck, funcArgOrderDifferent.
>
> Fixes: ae271547bba6 ("net: dsa: sja1105: C45 only transactions for PCS")

> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Compile tested only.
> ---
>  drivers/net/dsa/sja1105/sja1105_mdio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/dsa/sja1105/sja1105_mdio.c b/drivers/net/dsa/sja1105/sja1105_mdio.c
> index 833e55e4b961..52ddb4ef259e 100644
> --- a/drivers/net/dsa/sja1105/sja1105_mdio.c
> +++ b/drivers/net/dsa/sja1105/sja1105_mdio.c
> @@ -94,7 +94,7 @@ int sja1110_pcs_mdio_read_c45(struct mii_bus *bus, int phy, int mmd, int reg)
>  	return tmp & 0xffff;
>  }
>  
> -int sja1110_pcs_mdio_write_c45(struct mii_bus *bus, int phy, int reg, int mmd,
> +int sja1110_pcs_mdio_write_c45(struct mii_bus *bus, int phy, int mmd, int reg,
>  			       u16 val)

Reviewed-by: Michael Walle <mwalle@kernel.org>

Vladimir, do you happen to know if some of your boards will use this
function? Just wondering because it was never noticed.

-michael

>  {
>  	struct sja1105_mdio_private *mdio_priv = bus->priv;


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

Re: [PATCH net] net: dsa: sja1105: Fix parameters order in sja1110_pcs_mdio_write_c45()

[Vladimir Oltean]

On Wed, Apr 03, 2024 at 10:06:51AM +0200, Michael Walle wrote:
> Vladimir, do you happen to know if some of your boards will use this
> function? Just wondering because it was never noticed.
> 
> -michael

The SJA1110 uses the XPCS only for SGMII and 2500Base-X links. On the
Bluebox3 I have (which is currently in a cardboard box for the time
being, so I will have to rely on static analysis just like everybody
else), these links are the cascade ports between switches. However,
those cascade ports are only used for autonomous traffic bridging (the
board has a weird "H" topology). Traffic terminated on the CPU doesn't
go through the SGMII links, so this is why I haven't noticed it during
casual testing.

However, there are other NXP systems using downstream device trees which
are likely impacted, but they are on older kernels and probably haven't
seen the regression just yet. So the fix is welcome.

Re: [PATCH net] net: dsa: sja1105: Fix parameters order in sja1110_pcs_mdio_write_c45()

[Vladimir Oltean]

On Tue, Apr 02, 2024 at 08:33:56PM +0200, Christophe JAILLET wrote:
> The definition and declaration of sja1110_pcs_mdio_write_c45() don't have
> parameters in the same order.
> 
> Knowing that sja1110_pcs_mdio_write_c45() is used as a function pointer
> in 'sja1105_info' structure with .pcs_mdio_write_c45, and that we have:
> 
>    int (*pcs_mdio_write_c45)(struct mii_bus *bus, int phy, int mmd,
> 				  int reg, u16 val);
> 
> it is likely that the definition is the one to change.
> 
> Found with cppcheck, funcArgOrderDifferent.
> 
> Fixes: ae271547bba6 ("net: dsa: sja1105: C45 only transactions for PCS")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Compile tested only.
> ---

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Re: [PATCH net] net: dsa: sja1105: Fix parameters order in sja1110_pcs_mdio_write_c45()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue,  2 Apr 2024 20:33:56 +0200 you wrote:
> The definition and declaration of sja1110_pcs_mdio_write_c45() don't have
> parameters in the same order.
> 
> Knowing that sja1110_pcs_mdio_write_c45() is used as a function pointer
> in 'sja1105_info' structure with .pcs_mdio_write_c45, and that we have:
> 
>    int (*pcs_mdio_write_c45)(struct mii_bus *bus, int phy, int mmd,
> 				  int reg, u16 val);
> 
> [...]

Here is the summary with links:
  - [net] net: dsa: sja1105: Fix parameters order in sja1110_pcs_mdio_write_c45()
    https://git.kernel.org/netdev/net/c/c120209bce34

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