[PATCH] net: dsa: mv88e6xxx: initialize return variable on declaration

Subsystems: marvell 88e6xxx ethernet switch fabric driver, networking drivers, networking [dsa], the rest

STALE1775d

16 messages, 5 authors, 2021-12-09 · open the first message on its own page

[PATCH] net: dsa: mv88e6xxx: initialize return variable on declaration

From: Ameer Hamza <hidden>
Date: 2021-12-06 11:32:57

Uninitialized err variable defined in mv88e6393x_serdes_power
function may cause undefined behaviour if it is called from
mv88e6xxx_serdes_power_down context.

Addresses-Coverity: 1494644 ("Uninitialized scalar variable")

Signed-off-by: Ameer Hamza <redacted>
---
 drivers/net/dsa/mv88e6xxx/serdes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 55273013bfb5..33727439724a 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c

@@ -1507,7 +1507,7 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			    bool on)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err;
+	int err = 0;
 
 	if (port != 0 && port != 9 && port != 10)
 		return -EOPNOTSUPP;

-- 
2.25.1

Re: [PATCH] net: dsa: mv88e6xxx: initialize return variable on declaration

From: Andrew Lunn <andrew@lunn.ch>
Date: 2021-12-06 13:21:10

On Mon, Dec 06, 2021 at 04:32:19PM +0500, Ameer Hamza wrote:

quoted hunk

Uninitialized err variable defined in mv88e6393x_serdes_power
function may cause undefined behaviour if it is called from
mv88e6xxx_serdes_power_down context.

Addresses-Coverity: 1494644 ("Uninitialized scalar variable")

Signed-off-by: Ameer Hamza <redacted>
---
 drivers/net/dsa/mv88e6xxx/serdes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 55273013bfb5..33727439724a 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c

@@ -1507,7 +1507,7 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			    bool on)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err;
+	int err = 0;
 
 	if (port != 0 && port != 9 && port != 10)
 		return -EOPNOTSUPP;

Hi Marek

This warning likely comes from cmode not being a SERDES mode, and that
is not handles in the switch statementing. Do we want an

default:
	err = EINVAL;

?

	Andrew

Re: [PATCH] net: dsa: mv88e6xxx: initialize return variable on declaration

From: Marek Behún <kabel@kernel.org>
Date: 2021-12-06 22:30:24

On Mon, 6 Dec 2021 14:21:03 +0100
Andrew Lunn [off-list ref] wrote:

On Mon, Dec 06, 2021 at 04:32:19PM +0500, Ameer Hamza wrote:

quoted

Uninitialized err variable defined in mv88e6393x_serdes_power
function may cause undefined behaviour if it is called from
mv88e6xxx_serdes_power_down context.

Addresses-Coverity: 1494644 ("Uninitialized scalar variable")

Signed-off-by: Ameer Hamza <redacted>
---
 drivers/net/dsa/mv88e6xxx/serdes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 55273013bfb5..33727439724a 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c

@@ -1507,7 +1507,7 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			    bool on)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err;
+	int err = 0;
 
 	if (port != 0 && port != 9 && port != 10)
 		return -EOPNOTSUPP;

Hi Marek

This warning likely comes from cmode not being a SERDES mode, and that
is not handles in the switch statementing. Do we want an

default:
	err = EINVAL;

?

	Andrew

Hi Andrew,

currently all the .serdes_power() methods return 0 for non-serdes ports.
This is because the way it is written, these methods are not called if
there is not a serdes lane for a given port.

For this issue with err variable undefined, to fix it we should simply
set int err=0 at the beginning of mv88e6393x_serdes_power(), to make it
behave like other serdes_power() methods do in serdes.c.



But a refactor may be needed for serdes_power() methods, at least
because they are a little weird. But it should be unrelated to this fix.

In serdes.h we have static inline functions
  mv88e6xxx_serdes_power_up(chip, port, lane)
  mv88e6xxx_serdes_power_down(chip, port, lane)

  (These simply call the serdes_power() method of chip ops, with
   additional boolean argument to specify powerup/powerdown.
   Also for these we first need to determine lane for a port. If lane
   does not exists, these should not be called.)

In chip.c we have function
  mv88e6xxx_serdes_power(chip, port, on)
  
  (This finds if the port has a lane, and if so, calls, if on=true
   mv88e6xxx_serdes_power_up()
     from serdes.h, and then
   mv88e6xxx_serdes_irq_request()
     also from serdes.h

   and if on=false, calls _irq_free() & _serdes_power_down()
  )

So if I call
  mv88e6xxx_serdes_power(chip, port, true)
it goes
    mv88e6xxx_serdes_power_up(chip, port, lane)
      chip->info->ops->serdes_power(chip, port, lane, true)
so the `on` argument is used in some places, but in other places there
are two functions instead.

Which I find a little weird.

Marek

Re: [PATCH] net: dsa: mv88e6xxx: initialize return variable on declaration

From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-12-07 00:25:16

On Mon, 6 Dec 2021 23:29:53 +0100 Marek Behún wrote:

On Mon, 6 Dec 2021 14:21:03 +0100
Andrew Lunn [off-list ref] wrote:

quoted

On Mon, Dec 06, 2021 at 04:32:19PM +0500, Ameer Hamza wrote:

quoted

Uninitialized err variable defined in mv88e6393x_serdes_power
function may cause undefined behaviour if it is called from
mv88e6xxx_serdes_power_down context.

Addresses-Coverity: 1494644 ("Uninitialized scalar variable")

Signed-off-by: Ameer Hamza <redacted>
---
 drivers/net/dsa/mv88e6xxx/serdes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 55273013bfb5..33727439724a 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c

@@ -1507,7 +1507,7 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			    bool on)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err;
+	int err = 0;
 
 	if (port != 0 && port != 9 && port != 10)
 		return -EOPNOTSUPP;

Hi Marek

This warning likely comes from cmode not being a SERDES mode, and that
is not handles in the switch statementing. Do we want an

default:
	err = EINVAL;

?

currently all the .serdes_power() methods return 0 for non-serdes ports.
This is because the way it is written, these methods are not called if
there is not a serdes lane for a given port.

For this issue with err variable undefined, to fix it we should simply
set int err=0 at the beginning of mv88e6393x_serdes_power(), to make it
behave like other serdes_power() methods do in serdes.c.

Any objections to using a default case in the switch statement, tho?
I agree with Andrew that default statement would make the reasoning
clearer than just setting the variable at the start of the function.

Re: [PATCH] net: dsa: mv88e6xxx: initialize return variable on declaration

From: Marek Behún <kabel@kernel.org>
Date: 2021-12-07 13:06:59

On Mon, 6 Dec 2021 16:25:10 -0800
Jakub Kicinski [off-list ref] wrote:

On Mon, 6 Dec 2021 23:29:53 +0100 Marek Behún wrote:

quoted

On Mon, 6 Dec 2021 14:21:03 +0100
Andrew Lunn [off-list ref] wrote:

quoted

On Mon, Dec 06, 2021 at 04:32:19PM +0500, Ameer Hamza wrote:

quoted

Uninitialized err variable defined in mv88e6393x_serdes_power
function may cause undefined behaviour if it is called from
mv88e6xxx_serdes_power_down context.

Addresses-Coverity: 1494644 ("Uninitialized scalar variable")

Signed-off-by: Ameer Hamza <redacted>
---
 drivers/net/dsa/mv88e6xxx/serdes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 55273013bfb5..33727439724a 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c

@@ -1507,7 +1507,7 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			    bool on)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err;
+	int err = 0;
 
 	if (port != 0 && port != 9 && port != 10)
 		return -EOPNOTSUPP;

Hi Marek

This warning likely comes from cmode not being a SERDES mode, and that
is not handles in the switch statementing. Do we want an

default:
	err = EINVAL;

?

currently all the .serdes_power() methods return 0 for non-serdes ports.
This is because the way it is written, these methods are not called if
there is not a serdes lane for a given port.

For this issue with err variable undefined, to fix it we should simply
set int err=0 at the beginning of mv88e6393x_serdes_power(), to make it
behave like other serdes_power() methods do in serdes.c.

Any objections to using a default case in the switch statement, tho?
I agree with Andrew that default statement would make the reasoning
clearer than just setting the variable at the start of the function.

No objection, just that it should be done for all the serdes_power()
methods in serdes.c.

Marek

[PATCH v2] net: dsa: mv88e6xxx: error handling for serdes_power functions

From: Ameer Hamza <hidden>
Date: 2021-12-08 14:04:25

mv88e6390_serdes_power() and mv88e6393x_serdes_power() should return
with EINVAL error if cmode is undefined.

Signed-off-by: Ameer Hamza <redacted>
---
 drivers/net/dsa/mv88e6xxx/serdes.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 33727439724a..f3dc1865f291 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c

@@ -830,7 +830,7 @@ int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			   bool up)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err = 0;
+	int err;
 
 	switch (cmode) {
 	case MV88E6XXX_PORT_STS_CMODE_SGMII:

@@ -842,6 +842,8 @@ int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 	case MV88E6XXX_PORT_STS_CMODE_RXAUI:
 		err = mv88e6390_serdes_power_10g(chip, lane, up);
 		break;
+	default:
+		return -EINVAL;
 	}
 
 	if (!err && up)

@@ -1507,7 +1509,7 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			    bool on)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err = 0;
+	int err;
 
 	if (port != 0 && port != 9 && port != 10)
 		return -EOPNOTSUPP;

@@ -1541,6 +1543,8 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 	case MV88E6393X_PORT_STS_CMODE_10GBASER:
 		err = mv88e6390_serdes_power_10g(chip, lane, on);
 		break;
+	default:
+		return -EINVAL;
 	}
 
 	if (err)

-- 
2.25.1

Re: [PATCH v2] net: dsa: mv88e6xxx: error handling for serdes_power functions

From: Ameer Hamza <hidden>
Date: 2021-12-08 14:10:07

On Wed, Dec 08, 2021 at 07:04:13PM +0500, Ameer Hamza wrote:

quoted hunk

mv88e6390_serdes_power() and mv88e6393x_serdes_power() should return
with EINVAL error if cmode is undefined.

Signed-off-by: Ameer Hamza <redacted>
---
 drivers/net/dsa/mv88e6xxx/serdes.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 33727439724a..f3dc1865f291 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c

@@ -830,7 +830,7 @@ int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			   bool up)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err = 0;
+	int err;
 
 	switch (cmode) {
 	case MV88E6XXX_PORT_STS_CMODE_SGMII:

@@ -842,6 +842,8 @@ int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 	case MV88E6XXX_PORT_STS_CMODE_RXAUI:
 		err = mv88e6390_serdes_power_10g(chip, lane, up);
 		break;
+	default:
+		return -EINVAL;
 	}
 
 	if (!err && up)

@@ -1507,7 +1509,7 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			    bool on)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err = 0;
+	int err;
 
 	if (port != 0 && port != 9 && port != 10)
 		return -EOPNOTSUPP;

@@ -1541,6 +1543,8 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 	case MV88E6393X_PORT_STS_CMODE_10GBASER:
 		err = mv88e6390_serdes_power_10g(chip, lane, on);
 		break;
+	default:
+		return -EINVAL;
 	}
 
 	if (err)

-- 
2.25.1

Hi Marek,

I checked serdes.c and I found two methods mv88e6390_serdes_power() and
mv88e6390_serdes_power() that were not returning ENINVAL in case of
undefined cmode. Would be appreciated if  you can review the patch
please.

Best Regards,
Ameer Hamza.

Re: [PATCH v2] net: dsa: mv88e6xxx: error handling for serdes_power functions

From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-12-08 15:04:48

On Wed, 8 Dec 2021 19:09:57 +0500 Ameer Hamza wrote:

I checked serdes.c and I found two methods mv88e6390_serdes_power() and
mv88e6390_serdes_power() that were not returning ENINVAL in case of
undefined cmode. Would be appreciated if  you can review the patch
please.

A note for the future - please put more info into the commit 
message, it shouldn't be necessary to send a follow up email 
with the explanation.

Re: [PATCH v2] net: dsa: mv88e6xxx: error handling for serdes_power functions

From: Marek Behún <kabel@kernel.org>
Date: 2021-12-08 15:40:55

On Wed,  8 Dec 2021 19:04:13 +0500
Ameer Hamza [off-list ref] wrote:

quoted hunk

mv88e6390_serdes_power() and mv88e6393x_serdes_power() should return
with EINVAL error if cmode is undefined.

Signed-off-by: Ameer Hamza <redacted>
---
 drivers/net/dsa/mv88e6xxx/serdes.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 33727439724a..f3dc1865f291 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c

@@ -830,7 +830,7 @@ int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			   bool up)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err = 0;
+	int err;
 
 	switch (cmode) {
 	case MV88E6XXX_PORT_STS_CMODE_SGMII:

@@ -842,6 +842,8 @@ int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 	case MV88E6XXX_PORT_STS_CMODE_RXAUI:
 		err = mv88e6390_serdes_power_10g(chip, lane, up);
 		break;
+	default:
+		return -EINVAL;
 	}
 
 	if (!err && up)

@@ -1507,7 +1509,7 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			    bool on)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err = 0;
+	int err;
 
 	if (port != 0 && port != 9 && port != 10)
 		return -EOPNOTSUPP;

@@ -1541,6 +1543,8 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 	case MV88E6393X_PORT_STS_CMODE_10GBASER:
 		err = mv88e6390_serdes_power_10g(chip, lane, on);
 		break;
+	default:
+		return -EINVAL;
 	}
 
 	if (err)

please make it err = -EINVAL instead of direct return, so that we can
check in the code after and handle the error case.

Marek

[PATCH v3] net: dsa: mv88e6xxx: error handling for serdes_power functions

From: Ameer Hamza <hidden>
Date: 2021-12-08 15:58:21

Added default case to handle undefined cmode scenario in
mv88e6393x_serdes_power() and mv88e6393x_serdes_power() methods.

Signed-off-by: Ameer Hamza <redacted>
---
Changes in v3:
make err = -EINVAL instead of direct return, so that we can
check in the code after and handle the error case.
---
 drivers/net/dsa/mv88e6xxx/serdes.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 33727439724a..2b05ead515cd 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c

@@ -830,7 +830,7 @@ int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			   bool up)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err = 0;
+	int err;
 
 	switch (cmode) {
 	case MV88E6XXX_PORT_STS_CMODE_SGMII:

@@ -842,6 +842,9 @@ int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 	case MV88E6XXX_PORT_STS_CMODE_RXAUI:
 		err = mv88e6390_serdes_power_10g(chip, lane, up);
 		break;
+	default:
+		err = -EINVAL;
+		break;
 	}
 
 	if (!err && up)

@@ -1507,7 +1510,7 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			    bool on)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err = 0;
+	int err;
 
 	if (port != 0 && port != 9 && port != 10)
 		return -EOPNOTSUPP;

@@ -1541,6 +1544,9 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 	case MV88E6393X_PORT_STS_CMODE_10GBASER:
 		err = mv88e6390_serdes_power_10g(chip, lane, on);
 		break;
+	default:
+		err = -EINVAL;
+		break;
 	}
 
 	if (err)

-- 
2.25.1

Re: [PATCH v3] net: dsa: mv88e6xxx: error handling for serdes_power functions

From: Marek Behún <kabel@kernel.org>
Date: 2021-12-08 16:18:35

On Wed,  8 Dec 2021 20:58:09 +0500
Ameer Hamza [off-list ref] wrote:

Added default case to handle undefined cmode scenario in
mv88e6393x_serdes_power() and mv88e6393x_serdes_power() methods.

Signed-off-by: Ameer Hamza <redacted>
---

Reviewed-by: Marek Behún <kabel@kernel.org>

Re: [PATCH v3] net: dsa: mv88e6xxx: error handling for serdes_power functions

From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-12-09 01:28:27

On Wed,  8 Dec 2021 20:58:09 +0500 Ameer Hamza wrote:

quoted hunk

@@ -1507,7 +1510,7 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			    bool on)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err = 0;
+	int err;
 
 	if (port != 0 && port != 9 && port != 10)
 		return -EOPNOTSUPP;

This is on top of v1? It doesn't seem to apply, v1 was not merged.

Also can you please add Fixes tags?

Re: [PATCH v3] net: dsa: mv88e6xxx: error handling for serdes_power functions

From: Ameer Hamza <hidden>
Date: 2021-12-09 04:06:22

On Wed, Dec 08, 2021 at 05:28:20PM -0800, Jakub Kicinski wrote:

On Wed,  8 Dec 2021 20:58:09 +0500 Ameer Hamza wrote:

quoted

@@ -1507,7 +1510,7 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			    bool on)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err = 0;
+	int err;
 
 	if (port != 0 && port != 9 && port != 10)
 		return -EOPNOTSUPP;

This is on top of v1? It doesn't seem to apply, v1 was not merged.

Oh, sorry! Its not incremental but I think my repository was a
few commits old. Let me send the updated patch.

Also can you please add Fixes tags?

Sure.

[PATCH v4] net: dsa: mv88e6xxx: error handling for serdes_power functions

From: Ameer Hamza <hidden>
Date: 2021-12-09 04:07:57

Added default case to handle undefined cmode scenario in
mv88e6393x_serdes_power() and mv88e6393x_serdes_power() methods.

Addresses-Coverity: 1494644 ("Uninitialized scalar variable")
Fixes: 21635d9203e1c (net: mvpp2: fix XDP rx queues registering)
Reviewed-by: Marek Behún <kabel@kernel.org>
Signed-off-by: Ameer Hamza <redacted>
---
Changes in v4:
Added fix tag and synced patch with latest repo
---
 drivers/net/dsa/mv88e6xxx/serdes.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 55273013bfb5..2b05ead515cd 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c

@@ -830,7 +830,7 @@ int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			   bool up)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err = 0;
+	int err;
 
 	switch (cmode) {
 	case MV88E6XXX_PORT_STS_CMODE_SGMII:

@@ -842,6 +842,9 @@ int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 	case MV88E6XXX_PORT_STS_CMODE_RXAUI:
 		err = mv88e6390_serdes_power_10g(chip, lane, up);
 		break;
+	default:
+		err = -EINVAL;
+		break;
 	}
 
 	if (!err && up)

@@ -1541,6 +1544,9 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 	case MV88E6393X_PORT_STS_CMODE_10GBASER:
 		err = mv88e6390_serdes_power_10g(chip, lane, on);
 		break;
+	default:
+		err = -EINVAL;
+		break;
 	}
 
 	if (err)

-- 
2.25.1

[PATCH v4] net: dsa: mv88e6xxx: error handling for serdes_power functions

From: Ameer Hamza <hidden>
Date: 2021-12-09 04:16:04

Added default case to handle undefined cmode scenario in
mv88e6393x_serdes_power() and mv88e6393x_serdes_power() methods.

Addresses-Coverity: 1494644 ("Uninitialized scalar variable")
Fixes: 21635d9203e1c (net: dsa: mv88e6xxx: Fix application of erratum 4.8 for 88E6393X)
Reviewed-by: Marek Behún <kabel@kernel.org>
Signed-off-by: Ameer Hamza <redacted>
---
Changes in v4:
Correct commit message in fix tag
---
 drivers/net/dsa/mv88e6xxx/serdes.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 55273013bfb5..2b05ead515cd 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c

@@ -830,7 +830,7 @@ int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 			   bool up)
 {
 	u8 cmode = chip->ports[port].cmode;
-	int err = 0;
+	int err;
 
 	switch (cmode) {
 	case MV88E6XXX_PORT_STS_CMODE_SGMII:

@@ -842,6 +842,9 @@ int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 	case MV88E6XXX_PORT_STS_CMODE_RXAUI:
 		err = mv88e6390_serdes_power_10g(chip, lane, up);
 		break;
+	default:
+		err = -EINVAL;
+		break;
 	}
 
 	if (!err && up)

@@ -1541,6 +1544,9 @@ int mv88e6393x_serdes_power(struct mv88e6xxx_chip *chip, int port, int lane,
 	case MV88E6393X_PORT_STS_CMODE_10GBASER:
 		err = mv88e6390_serdes_power_10g(chip, lane, on);
 		break;
+	default:
+		err = -EINVAL;
+		break;
 	}
 
 	if (err)

-- 
2.25.1

Re: [PATCH v4] net: dsa: mv88e6xxx: error handling for serdes_power functions

From: patchwork-bot+netdevbpf@kernel.org
Date: 2021-12-09 15:50:20

Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski [off-list ref]:

On Thu,  9 Dec 2021 09:15:52 +0500 you wrote:

Added default case to handle undefined cmode scenario in
mv88e6393x_serdes_power() and mv88e6393x_serdes_power() methods.

Addresses-Coverity: 1494644 ("Uninitialized scalar variable")
Fixes: 21635d9203e1c (net: dsa: mv88e6xxx: Fix application of erratum 4.8 for 88E6393X)
Reviewed-by: Marek Behún <kabel@kernel.org>
Signed-off-by: Ameer Hamza <redacted>

[...]

Here is the summary with links:
  - [v4] net: dsa: mv88e6xxx: error handling for serdes_power functions
    https://git.kernel.org/netdev/net/c/0416e7af2369

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

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help