On 16.09.2015 02:58, Andreas Dannenberg wrote: > This patch adds support for enabling/disabling optional device specific > features through sysfs properties at runtime. > > * High-impedance mode enable/disable > * Sysoff enable/disable > > Refer to the respective device datasheets for more information: > > http://www.ti.com/product/bq24250 > http://www.ti.com/product/bq24251 > http://www.ti.com/product/bq24257 > > Signed-off-by: Andreas Dannenberg <dannenberg@xxxxxx> > --- > drivers/power/bq24257_charger.c | 53 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 53 insertions(+) > > diff --git a/drivers/power/bq24257_charger.c b/drivers/power/bq24257_charger.c > index 517a522..a0cb33c 100644 > --- a/drivers/power/bq24257_charger.c > +++ b/drivers/power/bq24257_charger.c > @@ -794,12 +794,65 @@ static ssize_t bq24257_show_in_dpm_voltage(struct device *dev, > bq24257_vindpm_map[bq->init_data.vindpm]); > } > > +static ssize_t bq24257_sysfs_show_enable(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct power_supply *psy = dev_get_drvdata(dev); > + struct bq24257_device *bq = power_supply_get_drvdata(psy); > + int ret; > + > + if (strcmp(attr->attr.name, "high_impedance_enable") == 0) > + ret = bq24257_field_read(bq, F_HZ_MODE); > + else if (strcmp(attr->attr.name, "sysoff_enable") == 0) > + ret = bq24257_field_read(bq, F_SYSOFF); > + else > + return -EINVAL; > + > + if (ret < 0) > + return ret; > + > + return scnprintf(buf, PAGE_SIZE, "%d\n", ret); > +} > + > +static ssize_t bq24257_sysfs_set_enable(struct device *dev, > + struct device_attribute *attr, > + const char *buf, > + size_t count) > +{ > + struct power_supply *psy = dev_get_drvdata(dev); > + struct bq24257_device *bq = power_supply_get_drvdata(psy); > + long val; > + int ret; > + > + if (kstrtol(buf, 10, &val) < 0) > + return -EINVAL; > + There is no validation for input number. Although this is harmless but one may expect that writing value of 1, 2 or 3 would give the same result. But it would not. Value of 2 would be equal to 0, right? > + if (strcmp(attr->attr.name, "high_impedance_enable") == 0) > + ret = bq24257_field_write(bq, F_HZ_MODE, val); > + else if (strcmp(attr->attr.name, "sysoff_enable") == 0) > + ret = bq24257_field_write(bq, F_SYSOFF, val); > + else > + return -EINVAL; > + > + if (ret < 0) > + return ret; > + > + return count; > +} > + > static DEVICE_ATTR(ovp_voltage, S_IRUGO, bq24257_show_ovp_voltage, NULL); > static DEVICE_ATTR(in_dpm_voltage, S_IRUGO, bq24257_show_in_dpm_voltage, NULL); This applies to previous patches actually: DEVICE_ATTR_RO? > +static DEVICE_ATTR(high_impedance_enable, S_IWUSR | S_IRUGO, > + bq24257_sysfs_show_enable, bq24257_sysfs_set_enable); > +static DEVICE_ATTR(sysoff_enable, S_IWUSR | S_IRUGO, > + bq24257_sysfs_show_enable, bq24257_sysfs_set_enable); DEVICE_ATTR_RW? Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
