On 8/30/24 05:30, Tzung-Bi Shih wrote:
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).
Checkers are fine with it as long as each case statement returns. They actually complain about unreachable code if there is a return statement at the end. I've tried to return directly from a case statement if the return value from a call is also the return value from the function. This is to avoid code such as ret = func(...); break; ... return ret; or, worse, ret = func(...) if (ret) return ret; break; ... return 0; and instead use return func(...); directly. On the other side, if there is some code to execute after a function call ret = func(); if (ret) return ret; do_something; break; ... return 0; I tend to use return values and break statements. Maybe that doesn't always work out. I'll keep it in mind for the future.
In either cases, it would be great if make them consistent. With that, Reviewed-by: Tzung-Bi Shih <tzungbi@xxxxxxxxxx>
Thanks a lot for this and all the other reviews! Guenter
