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