Thread (101 messages) flat view 101 messages, 8 authors, 2021-01-09

Re: [PATCH v7 3/4] net: dsa: mv88e6xxx: Change serdes lane parameter from u8 type to int

From: Andrew Lunn <andrew@lunn.ch>
Date: 2020-11-02 13:40:11
Also in: lkml

On Mon, Nov 02, 2020 at 04:43:09PM +1000, Pavana Sharma wrote:
quoted hunk ↗ jump to hunk
Returning 0 is no more an error case with MV88E6393 family
which has serdes lane numbers 0, 9 or 10.
So with this change .serdes_get_lane will return lane number
or error (-ENODEV).

Signed-off-by: Pavana Sharma <redacted>
---
 drivers/net/dsa/mv88e6xxx/chip.c   | 28 +++++------
 drivers/net/dsa/mv88e6xxx/chip.h   | 16 +++----
 drivers/net/dsa/mv88e6xxx/port.c   |  6 +--
 drivers/net/dsa/mv88e6xxx/serdes.c | 76 +++++++++++++++---------------
 drivers/net/dsa/mv88e6xxx/serdes.h | 50 ++++++++++----------
 5 files changed, 88 insertions(+), 88 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index f0dbc05e30a4..4994b8eee659 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -484,12 +484,12 @@ static int mv88e6xxx_serdes_pcs_get_state(struct dsa_switch *ds, int port,
 					  struct phylink_link_state *state)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
-	u8 lane;
+	int lane = -ENODEV;
 	int err;
You have added a lot of initialises which are not needed.
 
 	mv88e6xxx_reg_lock(chip);
 	lane = mv88e6xxx_serdes_get_lane(chip, port);

lane is always set, so there is no point in setting it to -ENODEV
first.
quoted hunk ↗ jump to hunk
-	if (lane && chip->info->ops->serdes_pcs_get_state)
+	if ((lane >= 0) && chip->info->ops->serdes_pcs_get_state)
 		err = chip->info->ops->serdes_pcs_get_state(chip, port, lane,
 							    state);
 	else
@@ -505,11 +505,11 @@ static int mv88e6xxx_serdes_pcs_config(struct mv88e6xxx_chip *chip, int port,
 				       const unsigned long *advertise)
 {
 	const struct mv88e6xxx_ops *ops = chip->info->ops;
-	u8 lane;
+	int lane = -ENODEV;
 
 	if (ops->serdes_pcs_config) {
 		lane = mv88e6xxx_serdes_get_lane(chip, port);
-		if (lane)
+		if (lane >= 0)
 			return ops->serdes_pcs_config(chip, port, lane, mode,
 						      interface, advertise);
 	}
@@ -521,15 +521,15 @@ static void mv88e6xxx_serdes_pcs_an_restart(struct dsa_switch *ds, int port)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
 	const struct mv88e6xxx_ops *ops;
+	int lane = -ENODEV;
 	int err = 0;
-	u8 lane;
 
 	ops = chip->info->ops;
 
 	if (ops->serdes_pcs_an_restart) {
 		mv88e6xxx_reg_lock(chip);
 		lane = mv88e6xxx_serdes_get_lane(chip, port);
-		if (lane)
lane is always set inside this if statement, and is never used outside
of it.
quoted hunk ↗ jump to hunk
+		if (lane >= 0)
 			err = ops->serdes_pcs_an_restart(chip, port, lane);
 		mv88e6xxx_reg_unlock(chip);
 
@@ -543,11 +543,11 @@ static int mv88e6xxx_serdes_pcs_link_up(struct mv88e6xxx_chip *chip, int port,
 					int speed, int duplex)
 {
 	const struct mv88e6xxx_ops *ops = chip->info->ops;
-	u8 lane;
+	int lane = -ENODEV;
 
 	if (!phylink_autoneg_inband(mode) && ops->serdes_pcs_link_up) {
 		lane = mv88e6xxx_serdes_get_lane(chip, port);
-		if (lane)
+		if (lane >= 0)
 			return ops->serdes_pcs_link_up(chip, port, lane,
lane is always set, and never used outside of the if ....

     Andrew
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help