Hi Marek, On Sun, 25 Aug 2019 05:59:12 +0200, Marek Behún <marek.be...@nic.cz> wrote: > void mv88e6390x_serdes_irq_free(struct mv88e6xxx_chip *chip, int port) > { > - int lane = mv88e6390x_serdes_get_lane(chip, port); > + int err; > + s8 lane; > > - if (lane == -ENODEV) > + err = mv88e6xxx_serdes_get_lane(chip, port, &lane); > + if (err) { > + dev_err(chip->dev, "Unable to free SERDES irq: %d\n", err); > return; > - > + } > if (lane < 0) > return;
[...] > -int mv88e6390x_serdes_get_lane(struct mv88e6xxx_chip *chip, int port); > +/* Put the SERDES lane address a port is using into *lane. If a port has > + * multiple lanes, should put the first lane the port is using. If a port > does > + * not have a lane, put -1 into *lane. > + */ > +static inline int mv88e6xxx_serdes_get_lane(struct mv88e6xxx_chip *chip, > + int port, s8 *lane) > +{ > + if (!chip->info->ops->serdes_get_lane) > + return -EOPNOTSUPP; > + > + return chip->info->ops->serdes_get_lane(chip, port, lane); > +} Using an invalid value is only useful if it can be interpreted by the other functions of the API. So I would've make lane an u8, assuming it gets modified only on success, which would result in calls like this one: u8 lane; int err; err = mv88e6xxx_serdes_get_lane(chip, port, &lane); if (err) { if (err == -ENODEV) err = 0; return err; } But at least it is well documented, so we can eventually fine tuned later: Reviewed-by: Vivien Didelot <vivien.dide...@gmail.com> Thank you! Vivien