On 5/3/19 3:49 PM, Vivien Didelot wrote:
> The Marvell SOHO switches have several ways to access the internal
> registers. One of them being the System Management Interface (SMI),
> using the MDC and MDIO pins, with direct and indirect variants.
>
> In preparation for adding support for other register accesses, move
> the SMI code into its own files. At the same time, refine the code
> to make it clear that the indirect variant is implemented using the
> direct variant accessing only two registers for command and data.
>
> Signed-off-by: Vivien Didelot <vivien.dide...@gmail.com>
> ---
With some nits below:
Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
[snip]
> assert_reg_lock(chip);
>
> - err = mv88e6xxx_smi_read(chip, addr, reg, val);
> + if (chip->smi_ops)
> + err = chip->smi_ops->read(chip, addr, reg, val);
> + else
You might want to check for smi_ops && smi_ops->read here to be safe.
You could also keep that code unchanged, and just make
mv88e6xxx_smi_read() an inline helper within smi.h:
static inline int mv88e6xxx_smi_read(struct mv88e6xxx_chip *chip, int
addr, int reg, int *val)
{
if (chip->smi_ops && chip->smi_ops->read)
return chip->smi_ops->read(chip, addr, reg, val);
return -EOPNOTSUPP;
}
> + err = -EOPNOTSUPP;
> +
> if (err)
> return err;
>
> @@ -217,7 +79,11 @@ int mv88e6xxx_write(struct mv88e6xxx_chip *chip, int
> addr, int reg, u16 val)
>
> assert_reg_lock(chip);
>
> - err = mv88e6xxx_smi_write(chip, addr, reg, val);
> + if (chip->smi_ops)
> + err = chip->smi_ops->write(chip, addr, reg, val);
> + else
Same here, you might want to check smi_ops && smi_ops->write to avoid
de-referencing a potentially NULL pointer.
--
Florian
