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

From: Geert Uytterhoeven
Date: Wed Sep 25 2024 - 03:19:34 EST


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