| Message ID | 1413289246-31650-3-git-send-email-k.kozlowski@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue 2014-10-14 14:20:40, Krzysztof Kozlowski wrote: > Replace direct calls to power supply function attributes with wrappers. > Wrappers provide safe access access in case of unregistering the power > supply (.e.g by removing the driver). Replace: > - get_property -> power_supply_get_property > - set_property -> power_supply_set_property > - property_is_writeable -> power_supply_property_is_writeable > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Acked-by: Pavel Machek <pavel@ucw.cz> > @@ -76,7 +76,7 @@ static ssize_t power_supply_show_property(struct device *dev, > if (off == POWER_SUPPLY_PROP_TYPE) { > value.intval = psy->type; > } else { > - ret = psy->get_property(psy, off, &value); > + ret = power_supply_get_property(psy, off, &value); > One thing.. Your power_supply_get_property (and friends) check for psy == NULL. Is it neccessary / good idea? As far as I can tell, it should not really be NULL... Pavel
On ?ro, 2014-10-15 at 12:32 +0200, Pavel Machek wrote: > On Tue 2014-10-14 14:20:40, Krzysztof Kozlowski wrote: > > Replace direct calls to power supply function attributes with wrappers. > > Wrappers provide safe access access in case of unregistering the power > > supply (.e.g by removing the driver). Replace: > > - get_property -> power_supply_get_property > > - set_property -> power_supply_set_property > > - property_is_writeable -> power_supply_property_is_writeable > > > > Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> > > Acked-by: Pavel Machek <pavel@ucw.cz> Thanks for looking at patches. > > @@ -76,7 +76,7 @@ static ssize_t power_supply_show_property(struct device *dev, > > if (off == POWER_SUPPLY_PROP_TYPE) { > > value.intval = psy->type; > > } else { > > - ret = psy->get_property(psy, off, &value); > > + ret = power_supply_get_property(psy, off, &value); > > > > One thing.. Your power_supply_get_property (and friends) check for psy > == NULL. Is it neccessary / good idea? As far as I can tell, it should > not really be NULL... It is not necessary. I thought it would be a good behavior of such exported function. You're right that this shouldn't be NULL especially because previously it was dereferenced. Other existing power supply exported functions don't check this so maybe I shouldn't introduce inconsistency. I'll remove the check and re-spin. I'll found ugly typos in commit message anyway. Best regards, Krzysztof
> > > @@ -76,7 +76,7 @@ static ssize_t power_supply_show_property(struct device *dev, > > > if (off == POWER_SUPPLY_PROP_TYPE) { > > > value.intval = psy->type; > > > } else { > > > - ret = psy->get_property(psy, off, &value); > > > + ret = power_supply_get_property(psy, off, &value); > > > > > > > One thing.. Your power_supply_get_property (and friends) check for psy > > == NULL. Is it neccessary / good idea? As far as I can tell, it should > > not really be NULL... > > It is not necessary. I thought it would be a good behavior of such > exported function. You're right that this shouldn't be NULL especially > because previously it was dereferenced. > > Other existing power supply exported functions don't check this so maybe > I shouldn't introduce inconsistency. > > I'll remove the check and re-spin. I'll found ugly typos in commit > message anyway. Thanks! Pavel
diff --git a/drivers/power/power_supply_sysfs.c b/drivers/power/power_supply_sysfs.c index 62653f50a524..f817aab80813 100644 --- a/drivers/power/power_supply_sysfs.c +++ b/drivers/power/power_supply_sysfs.c @@ -76,7 +76,7 @@ static ssize_t power_supply_show_property(struct device *dev, if (off == POWER_SUPPLY_PROP_TYPE) { value.intval = psy->type; } else { - ret = psy->get_property(psy, off, &value); + ret = power_supply_get_property(psy, off, &value); if (ret < 0) { if (ret == -ENODATA) @@ -125,7 +125,7 @@ static ssize_t power_supply_store_property(struct device *dev, value.intval = long_val; - ret = psy->set_property(psy, off, &value); + ret = power_supply_set_property(psy, off, &value); if (ret < 0) return ret; @@ -223,7 +223,7 @@ static umode_t power_supply_attr_is_visible(struct kobject *kobj, if (property == attrno) { if (psy->property_is_writeable && - psy->property_is_writeable(psy, property) > 0) + power_supply_property_is_writeable(psy, property) > 0) mode |= S_IWUSR; return mode;
Replace direct calls to power supply function attributes with wrappers. Wrappers provide safe access access in case of unregistering the power supply (.e.g by removing the driver). Replace: - get_property -> power_supply_get_property - set_property -> power_supply_set_property - property_is_writeable -> power_supply_property_is_writeable Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> --- drivers/power/power_supply_sysfs.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)