On Thu, Aug 29, 2024 at 06:05:51PM -0700, Guenter Roeck wrote:
> +static int ina2xx_chip_read(struct device *dev, u32 attr, long *val)
> {
> - struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
> struct ina2xx_data *data = dev_get_drvdata(dev);
> - unsigned int mask;
> - int alarm = 0;
> + u32 regval;
> int ret;
>
> - ret = regmap_read_bypassed(data->regmap, INA226_MASK_ENABLE, &mask);
> + switch (attr) {
> + case hwmon_chip_update_interval:
> + ret = regmap_read(data->regmap, INA2XX_CONFIG, ®val);
> + if (ret)
> + return ret;
> +
> + *val = ina226_reg_to_interval(regval);
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> + return 0;
......(1)
> +static int ina2xx_power_read(struct device *dev, u32 attr, long *val)
> +{
> + struct ina2xx_data *data = dev_get_drvdata(dev);
> +
> + switch (attr) {
> + case hwmon_power_input:
> + return ina2xx_read_init(dev, INA2XX_POWER, val);
> + case hwmon_power_crit:
> + return ina226_alert_limit_read(data, INA226_POWER_OVER_LIMIT_MASK,
> + INA2XX_POWER, val);
> + case hwmon_power_crit_alarm:
> + return ina226_alert_read(data->regmap, INA226_POWER_OVER_LIMIT_MASK, val);
> + default:
> + return -EOPNOTSUPP;
> + }
......(2)
Just noticed a nit: there are some *_read() and *_write() functions mainly
contain a switch-case block. Some of them returns 0 at the end of the function
(1); some of them don't (2). (1) should be unreachable, however, I'm not sure
whether some checkers might complain about case (2).
In either cases, it would be great if make them consistent.
With that,
Reviewed-by: Tzung-Bi Shih <tzungbi@xxxxxxxxxx>
