Hi Nicolin, On 09/28/2018 06:39 PM, Nicolin Chen wrote:
An ina3221 chip has three input ports. Each port is used to measure the voltage and current of its input source. The DT binding now has defined bindings for their input sources, so the driver should read these information and handle accordingly. This patch adds a new structure of input source specific information including input source label, shunt resistor value and its connection status. It exposes these labels via in[123]_label sysfs nodes upon available, and also disables those channels where there are no input source being connected. Meanwhile, it also adds in[123]_enable sysfs nodes for users to get control of three channels, and returns -ENODATA code for any sensor read according to hwmon ABI. Signed-off-by: Nicolin Chen <nicoleotsuka@xxxxxxxxx>
[ ... ]
+ +static ssize_t ina3221_set_enable(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct sensor_device_attribute *sd_attr = to_sensor_dev_attr(attr); + struct ina3221_data *ina = dev_get_drvdata(dev); + unsigned int channel = sd_attr->index; + u16 config = ina->reg_config; + bool enable; + int ret; + + ret = kstrtobool(buf, &enable); + if (ret) + return ret; + + if (enable) + config |= INA3221_CONFIG_CHx_EN(channel); + else + config &= ~INA3221_CONFIG_CHx_EN(channel); + + ret = regmap_write(ina->regmap, INA3221_CONFIG, config); + if (ret) + return ret; + + ina->reg_config = config; +
Sorry it too me so long to realize this: The code above is racy. There could be multiple threads enabling and disabling a channel. Without a mutex, there is no guarantee that the final value of reg_config matches the value written into the chip. You'll have to introduce a mutex and implement something like ... ret = kstrtobool(buf, &enable); if (ret) return ret; mutex_lock(&ina->update_lock); config = ina->reg_config; ... ret = regmap_write(ina->regmap, INA3221_CONFIG, config); if (ret) { count = ret; goto error; } ina->reg_config = config; error: mutex_unlock(&ina->update_lock); return count; Guenter