Re: [PATCH v7 2/2] hwmon: (isl28022) new driver for ISL28022 power monitor

[Geert Uytterhoeven]

Hi Yikai,

On Wed, Sep 25, 2024 at 5:11 AM Yikai Tsai <yikai.tsai.wiwynn@xxxxxxxxx> wrote:
> Driver for Renesas ISL28022 power monitor with I2C interface.
> The device monitors voltage, current via shunt resistor
> and calculated power.
>
> Signed-off-by: Carsten Spieß <mail@xxxxxxxxxxxxxxxxx>
> Signed-off-by: Yikai Tsai <yikai.tsai.wiwynn@xxxxxxxxx>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/hwmon/isl28022.c

> +static int isl28022_read_properties(struct device *dev, struct isl28022_data *data)
> +{
> +       u32 val;
> +       int err;
> +
> +       err = device_property_read_u32(dev, "shunt-resistor-micro-ohms", &val);
> +       if (err == -EINVAL)
> +               val = 10000;
> +       else if (err < 0)
> +               return err;
> +       data->shunt = val;
> +
> +       err = device_property_read_u32(dev, "renesas,shunt-range-microvolt", &val);
> +       if (err == -EINVAL)
> +               val = 320000;
> +       else if (err < 0)
> +               return err;
> +
> +       switch (val) {
> +       case 40000:
> +               data->gain = 1;
> +               if (data->shunt < 800)
> +                       goto shunt_invalid;
> +               break;
> +       case 80000:
> +               data->gain = 2;
> +               if (data->shunt < 1600)
> +                       goto shunt_invalid;
> +               break;
> +       case 160000:
> +               data->gain = 4;
> +               if (data->shunt < 3200)
> +                       goto shunt_invalid;
> +               break;
> +       case 320000:
> +               data->gain = 8;
> +               if (data->shunt < 6400)
> +                       goto shunt_invalid;
> +               break;
> +       default:
> +               dev_err_probe(dev, "renesas,shunt-range-microvolt invalid value %d\n", val);

I doubt this compiles well?

> +               return -EINVAL;

return dev_err_probe(dev, -EINVAL, "renesas,shunt-range-microvolt
invalid value %d\n", val);

> +       }
> +
> +       err = device_property_read_u32(dev, "renesas,average-samples", &val);
> +       if (err == -EINVAL)
> +               val = 1;
> +       else if (err < 0)
> +               return err;
> +       if (val > 128 || hweight32(val) != 1) {
> +               dev_err_probe(dev, "renesas,average-samples invalid value %d\n", val);
> +               return -EINVAL;

Likewise.

> +       }
> +       data->average = val;
> +
> +       return 0;
> +
> +shunt_invalid:
> +       dev_err_probe(dev, "renesas,shunt-resistor-microvolt invalid value %d\n", data->shunt);
> +       return -EINVAL;

One more.

> +}

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds