Re: [PATCH 1/2] power: bq27xxx_battery: add configurable poll_interval by sysfs

2016-10-24 Thread Matt Ranostay 
On Mon, Oct 24, 2016 at 12:51 PM, Pavel Machek  wrote:
> On Sun 2016-10-23 20:08:22, Matt Ranostay wrote:
>> On Fri, Oct 7, 2016 at 11:49 AM, Pavel Machek  wrote:
>> > On Fri 2016-09-16 20:42:54, Matt Ranostay wrote:
>> >> Allow the poll_interval to be runtime configurable via an sysfs entry.
>> >> This is needed for udev control of the poll interval.
>> >>
>> >> Signed-off-by: Matt Ranostay 
>> >
>> > working mail address would be nice here.
>> >
>> > sysfs files should be documented.
>> >
>>
>> Ok can do in next revision
>>
>> > Also... what is it good for?
>> >
>> > Do you have a device that needs non-standard interval?
>>
>> Basically we need to have the ability to dynamically change the
>> intervals. So closer to a battery drain we need to up the reporting
>> intervals.
>
> Umm, there seems to be mechanism there to change that already...?
>

Ah right. Commenting on the wrong patchset oops :).

> static const struct kernel_param_ops param_ops_poll_interval = {
> .get = param_get_uint,
> .set = poll_interval_param_set,
> };
>
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) 
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 1/2] power: bq27xxx_battery: add configurable poll_interval by sysfs

2016-10-24 Thread Matt Ranostay 
On Mon, Oct 24, 2016 at 12:51 PM, Pavel Machek  wrote:
> On Sun 2016-10-23 20:08:22, Matt Ranostay wrote:
>> On Fri, Oct 7, 2016 at 11:49 AM, Pavel Machek  wrote:
>> > On Fri 2016-09-16 20:42:54, Matt Ranostay wrote:
>> >> Allow the poll_interval to be runtime configurable via an sysfs entry.
>> >> This is needed for udev control of the poll interval.
>> >>
>> >> Signed-off-by: Matt Ranostay 
>> >
>> > working mail address would be nice here.
>> >
>> > sysfs files should be documented.
>> >
>>
>> Ok can do in next revision
>>
>> > Also... what is it good for?
>> >
>> > Do you have a device that needs non-standard interval?
>>
>> Basically we need to have the ability to dynamically change the
>> intervals. So closer to a battery drain we need to up the reporting
>> intervals.
>
> Umm, there seems to be mechanism there to change that already...?
>

Ah right. Commenting on the wrong patchset oops :).

> static const struct kernel_param_ops param_ops_poll_interval = {
> .get = param_get_uint,
> .set = poll_interval_param_set,
> };
>
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) 
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 1/2] power: bq27xxx_battery: add configurable poll_interval by sysfs

2016-10-24 Thread Pavel Machek 
On Sun 2016-10-23 20:08:22, Matt Ranostay wrote:
> On Fri, Oct 7, 2016 at 11:49 AM, Pavel Machek  wrote:
> > On Fri 2016-09-16 20:42:54, Matt Ranostay wrote:
> >> Allow the poll_interval to be runtime configurable via an sysfs entry.
> >> This is needed for udev control of the poll interval.
> >>
> >> Signed-off-by: Matt Ranostay 
> >
> > working mail address would be nice here.
> >
> > sysfs files should be documented.
> >
> 
> Ok can do in next revision
> 
> > Also... what is it good for?
> >
> > Do you have a device that needs non-standard interval?
> 
> Basically we need to have the ability to dynamically change the
> intervals. So closer to a battery drain we need to up the reporting
> intervals.

Umm, there seems to be mechanism there to change that already...?

static const struct kernel_param_ops param_ops_poll_interval = {
.get = param_get_uint,
.set = poll_interval_param_set,
};


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 1/2] power: bq27xxx_battery: add configurable poll_interval by sysfs

2016-10-24 Thread Pavel Machek 
On Sun 2016-10-23 20:08:22, Matt Ranostay wrote:
> On Fri, Oct 7, 2016 at 11:49 AM, Pavel Machek  wrote:
> > On Fri 2016-09-16 20:42:54, Matt Ranostay wrote:
> >> Allow the poll_interval to be runtime configurable via an sysfs entry.
> >> This is needed for udev control of the poll interval.
> >>
> >> Signed-off-by: Matt Ranostay 
> >
> > working mail address would be nice here.
> >
> > sysfs files should be documented.
> >
> 
> Ok can do in next revision
> 
> > Also... what is it good for?
> >
> > Do you have a device that needs non-standard interval?
> 
> Basically we need to have the ability to dynamically change the
> intervals. So closer to a battery drain we need to up the reporting
> intervals.

Umm, there seems to be mechanism there to change that already...?

static const struct kernel_param_ops param_ops_poll_interval = {
.get = param_get_uint,
.set = poll_interval_param_set,
};


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 1/2] power: bq27xxx_battery: add configurable poll_interval by sysfs

2016-10-24 Thread Pavel Machek 

> Ok can do in next revision
> 
> > Also... what is it good for?
> >
> > Do you have a device that needs non-standard interval?
> 
> Basically we need to have the ability to dynamically change the
> intervals. So closer to a battery drain we need to up the reporting
> intervals.

Could you elaborate on that? What intervals do you normally need, what
do you need around empty battery, and why is that?

I'm thinking about adding kernel thread to shut down the system if
battery goes critically empty. It is kernel job to protect the
hardware, and that should include the battery...

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 1/2] power: bq27xxx_battery: add configurable poll_interval by sysfs

2016-10-24 Thread Pavel Machek 

> Ok can do in next revision
> 
> > Also... what is it good for?
> >
> > Do you have a device that needs non-standard interval?
> 
> Basically we need to have the ability to dynamically change the
> intervals. So closer to a battery drain we need to up the reporting
> intervals.

Could you elaborate on that? What intervals do you normally need, what
do you need around empty battery, and why is that?

I'm thinking about adding kernel thread to shut down the system if
battery goes critically empty. It is kernel job to protect the
hardware, and that should include the battery...

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 1/2] power: bq27xxx_battery: add configurable poll_interval by sysfs

2016-10-23 Thread Matt Ranostay 
On Fri, Oct 7, 2016 at 11:49 AM, Pavel Machek  wrote:
> On Fri 2016-09-16 20:42:54, Matt Ranostay wrote:
>> Allow the poll_interval to be runtime configurable via an sysfs entry.
>> This is needed for udev control of the poll interval.
>>
>> Signed-off-by: Matt Ranostay 
>
> working mail address would be nice here.
>
> sysfs files should be documented.
>

Ok can do in next revision

> Also... what is it good for?
>
> Do you have a device that needs non-standard interval?

Basically we need to have the ability to dynamically change the
intervals. So closer to a battery drain we need to up the reporting
intervals.

Thanks,

Matt

> Pavel
>
>> ---
>>  drivers/power/supply/bq27xxx_battery.c | 48 
>> +-
>>  1 file changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/power/supply/bq27xxx_battery.c 
>> b/drivers/power/supply/bq27xxx_battery.c
>> index 3f57dd54803a..955424e10ae2 100644
>> --- a/drivers/power/supply/bq27xxx_battery.c
>> +++ b/drivers/power/supply/bq27xxx_battery.c
>> @@ -395,6 +395,36 @@ module_param(poll_interval, uint, 0644);
>>  MODULE_PARM_DESC(poll_interval,
>>"battery poll interval in seconds - 0 disables polling");
>>
>> +
>> +static ssize_t show_poll_interval(struct device *dev,
>> +   struct device_attribute *attr, char *buf)
>> +{
>> + return sprintf(buf, "%d\n", poll_interval);
>> +}
>> +
>> +static ssize_t store_poll_interval(struct device *dev,
>> +struct device_attribute *attr,
>> +const char *buf, size_t size)
>> +{
>> + struct bq27xxx_device_info *di = dev_get_drvdata(dev);
>> + int tmp = poll_interval;
>> +
>> + if (sscanf(buf, "%d\n", _interval) != 1)
>> + return -EINVAL;
>> +
>> + if (poll_interval < 0)
>> + return -EINVAL;
>> +
>> + if (tmp != poll_interval) {
>> + cancel_delayed_work_sync(>work);
>> + schedule_delayed_work(>work, 0);
>> + }
>> +
>> + return size;
>> +}
>> +
>> +static DEVICE_ATTR(poll_interval, 0644, show_poll_interval, 
>> store_poll_interval);
>> +
>>  /*
>>   * Common code for BQ27xxx devices
>>   */
>> @@ -946,6 +976,7 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>>  {
>>   struct power_supply_desc *psy_desc;
>>   struct power_supply_config psy_cfg = { .drv_data = di, };
>> + int ret;
>>
>>   INIT_DELAYED_WORK(>work, bq27xxx_battery_poll);
>>   mutex_init(>lock);
>> @@ -961,11 +992,19 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info 
>> *di)
>>   psy_desc->num_properties = bq27xxx_battery_props[di->chip].size;
>>   psy_desc->get_property = bq27xxx_battery_get_property;
>>   psy_desc->external_power_changed = bq27xxx_external_power_changed;
>> + dev_set_drvdata(di->dev, di);
>> +
>> + ret = sysfs_create_file(>dev->kobj, _attr_poll_interval.attr);
>> + if (ret) {
>> + dev_err(di->dev, "failed to register poll_interval sysfs 
>> entry");
>> + return ret;
>> + }
>>
>>   di->bat = power_supply_register_no_ws(di->dev, psy_desc, _cfg);
>>   if (IS_ERR(di->bat)) {
>>   dev_err(di->dev, "failed to register battery\n");
>> - return PTR_ERR(di->bat);
>> + ret = PTR_ERR(di->bat);
>> + goto err_out;
>>   }
>>
>>   dev_info(di->dev, "support ver. %s enabled\n", DRIVER_VERSION);
>> @@ -973,6 +1012,11 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info 
>> *di)
>>   bq27xxx_battery_update(di);
>>
>>   return 0;
>> +
>> +err_out:
>> + sysfs_remove_file(>dev->kobj, _attr_poll_interval.attr);
>> +
>> + return ret;
>>  }
>>  EXPORT_SYMBOL_GPL(bq27xxx_battery_setup);
>>
>> @@ -988,6 +1032,8 @@ void bq27xxx_battery_teardown(struct 
>> bq27xxx_device_info *di)
>>
>>   cancel_delayed_work_sync(>work);
>>
>> + sysfs_remove_file(>dev->kobj, _attr_poll_interval.attr);
>> +
>>   power_supply_unregister(di->bat);
>>
>>   mutex_destroy(>lock);
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) 
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 1/2] power: bq27xxx_battery: add configurable poll_interval by sysfs

2016-10-23 Thread Matt Ranostay 
On Fri, Oct 7, 2016 at 11:49 AM, Pavel Machek  wrote:
> On Fri 2016-09-16 20:42:54, Matt Ranostay wrote:
>> Allow the poll_interval to be runtime configurable via an sysfs entry.
>> This is needed for udev control of the poll interval.
>>
>> Signed-off-by: Matt Ranostay 
>
> working mail address would be nice here.
>
> sysfs files should be documented.
>

Ok can do in next revision

> Also... what is it good for?
>
> Do you have a device that needs non-standard interval?

Basically we need to have the ability to dynamically change the
intervals. So closer to a battery drain we need to up the reporting
intervals.

Thanks,

Matt

> Pavel
>
>> ---
>>  drivers/power/supply/bq27xxx_battery.c | 48 
>> +-
>>  1 file changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/power/supply/bq27xxx_battery.c 
>> b/drivers/power/supply/bq27xxx_battery.c
>> index 3f57dd54803a..955424e10ae2 100644
>> --- a/drivers/power/supply/bq27xxx_battery.c
>> +++ b/drivers/power/supply/bq27xxx_battery.c
>> @@ -395,6 +395,36 @@ module_param(poll_interval, uint, 0644);
>>  MODULE_PARM_DESC(poll_interval,
>>"battery poll interval in seconds - 0 disables polling");
>>
>> +
>> +static ssize_t show_poll_interval(struct device *dev,
>> +   struct device_attribute *attr, char *buf)
>> +{
>> + return sprintf(buf, "%d\n", poll_interval);
>> +}
>> +
>> +static ssize_t store_poll_interval(struct device *dev,
>> +struct device_attribute *attr,
>> +const char *buf, size_t size)
>> +{
>> + struct bq27xxx_device_info *di = dev_get_drvdata(dev);
>> + int tmp = poll_interval;
>> +
>> + if (sscanf(buf, "%d\n", _interval) != 1)
>> + return -EINVAL;
>> +
>> + if (poll_interval < 0)
>> + return -EINVAL;
>> +
>> + if (tmp != poll_interval) {
>> + cancel_delayed_work_sync(>work);
>> + schedule_delayed_work(>work, 0);
>> + }
>> +
>> + return size;
>> +}
>> +
>> +static DEVICE_ATTR(poll_interval, 0644, show_poll_interval, 
>> store_poll_interval);
>> +
>>  /*
>>   * Common code for BQ27xxx devices
>>   */
>> @@ -946,6 +976,7 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>>  {
>>   struct power_supply_desc *psy_desc;
>>   struct power_supply_config psy_cfg = { .drv_data = di, };
>> + int ret;
>>
>>   INIT_DELAYED_WORK(>work, bq27xxx_battery_poll);
>>   mutex_init(>lock);
>> @@ -961,11 +992,19 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info 
>> *di)
>>   psy_desc->num_properties = bq27xxx_battery_props[di->chip].size;
>>   psy_desc->get_property = bq27xxx_battery_get_property;
>>   psy_desc->external_power_changed = bq27xxx_external_power_changed;
>> + dev_set_drvdata(di->dev, di);
>> +
>> + ret = sysfs_create_file(>dev->kobj, _attr_poll_interval.attr);
>> + if (ret) {
>> + dev_err(di->dev, "failed to register poll_interval sysfs 
>> entry");
>> + return ret;
>> + }
>>
>>   di->bat = power_supply_register_no_ws(di->dev, psy_desc, _cfg);
>>   if (IS_ERR(di->bat)) {
>>   dev_err(di->dev, "failed to register battery\n");
>> - return PTR_ERR(di->bat);
>> + ret = PTR_ERR(di->bat);
>> + goto err_out;
>>   }
>>
>>   dev_info(di->dev, "support ver. %s enabled\n", DRIVER_VERSION);
>> @@ -973,6 +1012,11 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info 
>> *di)
>>   bq27xxx_battery_update(di);
>>
>>   return 0;
>> +
>> +err_out:
>> + sysfs_remove_file(>dev->kobj, _attr_poll_interval.attr);
>> +
>> + return ret;
>>  }
>>  EXPORT_SYMBOL_GPL(bq27xxx_battery_setup);
>>
>> @@ -988,6 +1032,8 @@ void bq27xxx_battery_teardown(struct 
>> bq27xxx_device_info *di)
>>
>>   cancel_delayed_work_sync(>work);
>>
>> + sysfs_remove_file(>dev->kobj, _attr_poll_interval.attr);
>> +
>>   power_supply_unregister(di->bat);
>>
>>   mutex_destroy(>lock);
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) 
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 1/2] power: bq27xxx_battery: add configurable poll_interval by sysfs

2016-10-07 Thread Pavel Machek 
On Fri 2016-09-16 20:42:54, Matt Ranostay wrote:
> Allow the poll_interval to be runtime configurable via an sysfs entry.
> This is needed for udev control of the poll interval.
> 
> Signed-off-by: Matt Ranostay 

working mail address would be nice here.

sysfs files should be documented.

Also... what is it good for?

Do you have a device that needs non-standard interval?
Pavel

> ---
>  drivers/power/supply/bq27xxx_battery.c | 48 
> +-
>  1 file changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/bq27xxx_battery.c 
> b/drivers/power/supply/bq27xxx_battery.c
> index 3f57dd54803a..955424e10ae2 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -395,6 +395,36 @@ module_param(poll_interval, uint, 0644);
>  MODULE_PARM_DESC(poll_interval,
>"battery poll interval in seconds - 0 disables polling");
>  
> +
> +static ssize_t show_poll_interval(struct device *dev,
> +   struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%d\n", poll_interval);
> +}
> +
> +static ssize_t store_poll_interval(struct device *dev,
> +struct device_attribute *attr,
> +const char *buf, size_t size)
> +{
> + struct bq27xxx_device_info *di = dev_get_drvdata(dev);
> + int tmp = poll_interval;
> +
> + if (sscanf(buf, "%d\n", _interval) != 1)
> + return -EINVAL;
> +
> + if (poll_interval < 0)
> + return -EINVAL;
> +
> + if (tmp != poll_interval) {
> + cancel_delayed_work_sync(>work);
> + schedule_delayed_work(>work, 0);
> + }
> +
> + return size;
> +}
> +
> +static DEVICE_ATTR(poll_interval, 0644, show_poll_interval, 
> store_poll_interval);
> +
>  /*
>   * Common code for BQ27xxx devices
>   */
> @@ -946,6 +976,7 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>  {
>   struct power_supply_desc *psy_desc;
>   struct power_supply_config psy_cfg = { .drv_data = di, };
> + int ret;
>  
>   INIT_DELAYED_WORK(>work, bq27xxx_battery_poll);
>   mutex_init(>lock);
> @@ -961,11 +992,19 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info 
> *di)
>   psy_desc->num_properties = bq27xxx_battery_props[di->chip].size;
>   psy_desc->get_property = bq27xxx_battery_get_property;
>   psy_desc->external_power_changed = bq27xxx_external_power_changed;
> + dev_set_drvdata(di->dev, di);
> +
> + ret = sysfs_create_file(>dev->kobj, _attr_poll_interval.attr);
> + if (ret) {
> + dev_err(di->dev, "failed to register poll_interval sysfs 
> entry");
> + return ret;
> + }
>  
>   di->bat = power_supply_register_no_ws(di->dev, psy_desc, _cfg);
>   if (IS_ERR(di->bat)) {
>   dev_err(di->dev, "failed to register battery\n");
> - return PTR_ERR(di->bat);
> + ret = PTR_ERR(di->bat);
> + goto err_out;
>   }
>  
>   dev_info(di->dev, "support ver. %s enabled\n", DRIVER_VERSION);
> @@ -973,6 +1012,11 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info 
> *di)
>   bq27xxx_battery_update(di);
>  
>   return 0;
> +
> +err_out:
> + sysfs_remove_file(>dev->kobj, _attr_poll_interval.attr);
> +
> + return ret;
>  }
>  EXPORT_SYMBOL_GPL(bq27xxx_battery_setup);
>  
> @@ -988,6 +1032,8 @@ void bq27xxx_battery_teardown(struct bq27xxx_device_info 
> *di)
>  
>   cancel_delayed_work_sync(>work);
>  
> + sysfs_remove_file(>dev->kobj, _attr_poll_interval.attr);
> +
>   power_supply_unregister(di->bat);
>  
>   mutex_destroy(>lock);

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 1/2] power: bq27xxx_battery: add configurable poll_interval by sysfs

2016-10-07 Thread Pavel Machek 
On Fri 2016-09-16 20:42:54, Matt Ranostay wrote:
> Allow the poll_interval to be runtime configurable via an sysfs entry.
> This is needed for udev control of the poll interval.
> 
> Signed-off-by: Matt Ranostay 

working mail address would be nice here.

sysfs files should be documented.

Also... what is it good for?

Do you have a device that needs non-standard interval?
Pavel

> ---
>  drivers/power/supply/bq27xxx_battery.c | 48 
> +-
>  1 file changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/bq27xxx_battery.c 
> b/drivers/power/supply/bq27xxx_battery.c
> index 3f57dd54803a..955424e10ae2 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -395,6 +395,36 @@ module_param(poll_interval, uint, 0644);
>  MODULE_PARM_DESC(poll_interval,
>"battery poll interval in seconds - 0 disables polling");
>  
> +
> +static ssize_t show_poll_interval(struct device *dev,
> +   struct device_attribute *attr, char *buf)
> +{
> + return sprintf(buf, "%d\n", poll_interval);
> +}
> +
> +static ssize_t store_poll_interval(struct device *dev,
> +struct device_attribute *attr,
> +const char *buf, size_t size)
> +{
> + struct bq27xxx_device_info *di = dev_get_drvdata(dev);
> + int tmp = poll_interval;
> +
> + if (sscanf(buf, "%d\n", _interval) != 1)
> + return -EINVAL;
> +
> + if (poll_interval < 0)
> + return -EINVAL;
> +
> + if (tmp != poll_interval) {
> + cancel_delayed_work_sync(>work);
> + schedule_delayed_work(>work, 0);
> + }
> +
> + return size;
> +}
> +
> +static DEVICE_ATTR(poll_interval, 0644, show_poll_interval, 
> store_poll_interval);
> +
>  /*
>   * Common code for BQ27xxx devices
>   */
> @@ -946,6 +976,7 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
>  {
>   struct power_supply_desc *psy_desc;
>   struct power_supply_config psy_cfg = { .drv_data = di, };
> + int ret;
>  
>   INIT_DELAYED_WORK(>work, bq27xxx_battery_poll);
>   mutex_init(>lock);
> @@ -961,11 +992,19 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info 
> *di)
>   psy_desc->num_properties = bq27xxx_battery_props[di->chip].size;
>   psy_desc->get_property = bq27xxx_battery_get_property;
>   psy_desc->external_power_changed = bq27xxx_external_power_changed;
> + dev_set_drvdata(di->dev, di);
> +
> + ret = sysfs_create_file(>dev->kobj, _attr_poll_interval.attr);
> + if (ret) {
> + dev_err(di->dev, "failed to register poll_interval sysfs 
> entry");
> + return ret;
> + }
>  
>   di->bat = power_supply_register_no_ws(di->dev, psy_desc, _cfg);
>   if (IS_ERR(di->bat)) {
>   dev_err(di->dev, "failed to register battery\n");
> - return PTR_ERR(di->bat);
> + ret = PTR_ERR(di->bat);
> + goto err_out;
>   }
>  
>   dev_info(di->dev, "support ver. %s enabled\n", DRIVER_VERSION);
> @@ -973,6 +1012,11 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info 
> *di)
>   bq27xxx_battery_update(di);
>  
>   return 0;
> +
> +err_out:
> + sysfs_remove_file(>dev->kobj, _attr_poll_interval.attr);
> +
> + return ret;
>  }
>  EXPORT_SYMBOL_GPL(bq27xxx_battery_setup);
>  
> @@ -988,6 +1032,8 @@ void bq27xxx_battery_teardown(struct bq27xxx_device_info 
> *di)
>  
>   cancel_delayed_work_sync(>work);
>  
> + sysfs_remove_file(>dev->kobj, _attr_poll_interval.attr);
> +
>   power_supply_unregister(di->bat);
>  
>   mutex_destroy(>lock);

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 1/2] power: bq27xxx_battery: add configurable poll_interval by sysfs

2016-09-19 Thread Matt Ranostay 
On Mon, Sep 19, 2016 at 12:46 PM, Sebastian Reichel  wrote:
> Hi,
>
> On Fri, Sep 16, 2016 at 08:42:54PM -0700, Matt Ranostay wrote:
>> Allow the poll_interval to be runtime configurable via an sysfs
>> entry.  This is needed for udev control of the poll interval.
>>
>> Signed-off-by: Matt Ranostay 
>> ---
>>  drivers/power/supply/bq27xxx_battery.c | 48 
>> +-
>>  1 file changed, 47 insertions(+), 1 deletion(-)
>
> New sysfs attributes should be documented in Documentation/ABI.

Yeah I should know better :)

>
> Also I'm not too keen to add this, as there is already the sysfs
> entry for the module parameter. I don't see any reason why udev
> should not be able to change that value, so fix udev instead of
> duplicating functionality in the kernel.

Yeah duplication is bad.  We are wondering if having a
POWER_SUPPLY_PROP_UPDATE_INTERVAL would be an more acceptable
solution. Of course this would need to be made generic and not a per
driver solution as it is now.

Thanks,

Matt

>
> -- Sebastian

Re: [PATCH 1/2] power: bq27xxx_battery: add configurable poll_interval by sysfs

2016-09-19 Thread Matt Ranostay 
On Mon, Sep 19, 2016 at 12:46 PM, Sebastian Reichel  wrote:
> Hi,
>
> On Fri, Sep 16, 2016 at 08:42:54PM -0700, Matt Ranostay wrote:
>> Allow the poll_interval to be runtime configurable via an sysfs
>> entry.  This is needed for udev control of the poll interval.
>>
>> Signed-off-by: Matt Ranostay 
>> ---
>>  drivers/power/supply/bq27xxx_battery.c | 48 
>> +-
>>  1 file changed, 47 insertions(+), 1 deletion(-)
>
> New sysfs attributes should be documented in Documentation/ABI.

Yeah I should know better :)

>
> Also I'm not too keen to add this, as there is already the sysfs
> entry for the module parameter. I don't see any reason why udev
> should not be able to change that value, so fix udev instead of
> duplicating functionality in the kernel.

Yeah duplication is bad.  We are wondering if having a
POWER_SUPPLY_PROP_UPDATE_INTERVAL would be an more acceptable
solution. Of course this would need to be made generic and not a per
driver solution as it is now.

Thanks,

Matt

>
> -- Sebastian

Re: [PATCH 1/2] power: bq27xxx_battery: add configurable poll_interval by sysfs

2016-09-19 Thread Sebastian Reichel 
Hi,

On Fri, Sep 16, 2016 at 08:42:54PM -0700, Matt Ranostay wrote:
> Allow the poll_interval to be runtime configurable via an sysfs
> entry.  This is needed for udev control of the poll interval.
> 
> Signed-off-by: Matt Ranostay 
> ---
>  drivers/power/supply/bq27xxx_battery.c | 48 
> +-
>  1 file changed, 47 insertions(+), 1 deletion(-)

New sysfs attributes should be documented in Documentation/ABI.

Also I'm not too keen to add this, as there is already the sysfs
entry for the module parameter. I don't see any reason why udev
should not be able to change that value, so fix udev instead of
duplicating functionality in the kernel.

-- Sebastian


signature.asc
Description: PGP signature

Re: [PATCH 1/2] power: bq27xxx_battery: add configurable poll_interval by sysfs

2016-09-19 Thread Sebastian Reichel 
Hi,

On Fri, Sep 16, 2016 at 08:42:54PM -0700, Matt Ranostay wrote:
> Allow the poll_interval to be runtime configurable via an sysfs
> entry.  This is needed for udev control of the poll interval.
> 
> Signed-off-by: Matt Ranostay 
> ---
>  drivers/power/supply/bq27xxx_battery.c | 48 
> +-
>  1 file changed, 47 insertions(+), 1 deletion(-)

New sysfs attributes should be documented in Documentation/ABI.

Also I'm not too keen to add this, as there is already the sysfs
entry for the module parameter. I don't see any reason why udev
should not be able to change that value, so fix udev instead of
duplicating functionality in the kernel.

-- Sebastian


signature.asc
Description: PGP signature

[PATCH 1/2] power: bq27xxx_battery: add configurable poll_interval by sysfs

2016-09-16 Thread Matt Ranostay 
Allow the poll_interval to be runtime configurable via an sysfs entry.
This is needed for udev control of the poll interval.

Signed-off-by: Matt Ranostay 
---
 drivers/power/supply/bq27xxx_battery.c | 48 +-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/bq27xxx_battery.c 
b/drivers/power/supply/bq27xxx_battery.c
index 3f57dd54803a..955424e10ae2 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -395,6 +395,36 @@ module_param(poll_interval, uint, 0644);
 MODULE_PARM_DESC(poll_interval,
 "battery poll interval in seconds - 0 disables polling");
 
+
+static ssize_t show_poll_interval(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+   return sprintf(buf, "%d\n", poll_interval);
+}
+
+static ssize_t store_poll_interval(struct device *dev,
+  struct device_attribute *attr,
+  const char *buf, size_t size)
+{
+   struct bq27xxx_device_info *di = dev_get_drvdata(dev);
+   int tmp = poll_interval;
+
+   if (sscanf(buf, "%d\n", _interval) != 1)
+   return -EINVAL;
+
+   if (poll_interval < 0)
+   return -EINVAL;
+
+   if (tmp != poll_interval) {
+   cancel_delayed_work_sync(>work);
+   schedule_delayed_work(>work, 0);
+   }
+
+   return size;
+}
+
+static DEVICE_ATTR(poll_interval, 0644, show_poll_interval, 
store_poll_interval);
+
 /*
  * Common code for BQ27xxx devices
  */
@@ -946,6 +976,7 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
 {
struct power_supply_desc *psy_desc;
struct power_supply_config psy_cfg = { .drv_data = di, };
+   int ret;
 
INIT_DELAYED_WORK(>work, bq27xxx_battery_poll);
mutex_init(>lock);
@@ -961,11 +992,19 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
psy_desc->num_properties = bq27xxx_battery_props[di->chip].size;
psy_desc->get_property = bq27xxx_battery_get_property;
psy_desc->external_power_changed = bq27xxx_external_power_changed;
+   dev_set_drvdata(di->dev, di);
+
+   ret = sysfs_create_file(>dev->kobj, _attr_poll_interval.attr);
+   if (ret) {
+   dev_err(di->dev, "failed to register poll_interval sysfs 
entry");
+   return ret;
+   }
 
di->bat = power_supply_register_no_ws(di->dev, psy_desc, _cfg);
if (IS_ERR(di->bat)) {
dev_err(di->dev, "failed to register battery\n");
-   return PTR_ERR(di->bat);
+   ret = PTR_ERR(di->bat);
+   goto err_out;
}
 
dev_info(di->dev, "support ver. %s enabled\n", DRIVER_VERSION);
@@ -973,6 +1012,11 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
bq27xxx_battery_update(di);
 
return 0;
+
+err_out:
+   sysfs_remove_file(>dev->kobj, _attr_poll_interval.attr);
+
+   return ret;
 }
 EXPORT_SYMBOL_GPL(bq27xxx_battery_setup);
 
@@ -988,6 +1032,8 @@ void bq27xxx_battery_teardown(struct bq27xxx_device_info 
*di)
 
cancel_delayed_work_sync(>work);
 
+   sysfs_remove_file(>dev->kobj, _attr_poll_interval.attr);
+
power_supply_unregister(di->bat);
 
mutex_destroy(>lock);
-- 
2.7.4

[PATCH 1/2] power: bq27xxx_battery: add configurable poll_interval by sysfs

2016-09-16 Thread Matt Ranostay 
Allow the poll_interval to be runtime configurable via an sysfs entry.
This is needed for udev control of the poll interval.

Signed-off-by: Matt Ranostay 
---
 drivers/power/supply/bq27xxx_battery.c | 48 +-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/bq27xxx_battery.c 
b/drivers/power/supply/bq27xxx_battery.c
index 3f57dd54803a..955424e10ae2 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -395,6 +395,36 @@ module_param(poll_interval, uint, 0644);
 MODULE_PARM_DESC(poll_interval,
 "battery poll interval in seconds - 0 disables polling");
 
+
+static ssize_t show_poll_interval(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+   return sprintf(buf, "%d\n", poll_interval);
+}
+
+static ssize_t store_poll_interval(struct device *dev,
+  struct device_attribute *attr,
+  const char *buf, size_t size)
+{
+   struct bq27xxx_device_info *di = dev_get_drvdata(dev);
+   int tmp = poll_interval;
+
+   if (sscanf(buf, "%d\n", _interval) != 1)
+   return -EINVAL;
+
+   if (poll_interval < 0)
+   return -EINVAL;
+
+   if (tmp != poll_interval) {
+   cancel_delayed_work_sync(>work);
+   schedule_delayed_work(>work, 0);
+   }
+
+   return size;
+}
+
+static DEVICE_ATTR(poll_interval, 0644, show_poll_interval, 
store_poll_interval);
+
 /*
  * Common code for BQ27xxx devices
  */
@@ -946,6 +976,7 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
 {
struct power_supply_desc *psy_desc;
struct power_supply_config psy_cfg = { .drv_data = di, };
+   int ret;
 
INIT_DELAYED_WORK(>work, bq27xxx_battery_poll);
mutex_init(>lock);
@@ -961,11 +992,19 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
psy_desc->num_properties = bq27xxx_battery_props[di->chip].size;
psy_desc->get_property = bq27xxx_battery_get_property;
psy_desc->external_power_changed = bq27xxx_external_power_changed;
+   dev_set_drvdata(di->dev, di);
+
+   ret = sysfs_create_file(>dev->kobj, _attr_poll_interval.attr);
+   if (ret) {
+   dev_err(di->dev, "failed to register poll_interval sysfs 
entry");
+   return ret;
+   }
 
di->bat = power_supply_register_no_ws(di->dev, psy_desc, _cfg);
if (IS_ERR(di->bat)) {
dev_err(di->dev, "failed to register battery\n");
-   return PTR_ERR(di->bat);
+   ret = PTR_ERR(di->bat);
+   goto err_out;
}
 
dev_info(di->dev, "support ver. %s enabled\n", DRIVER_VERSION);
@@ -973,6 +1012,11 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
bq27xxx_battery_update(di);
 
return 0;
+
+err_out:
+   sysfs_remove_file(>dev->kobj, _attr_poll_interval.attr);
+
+   return ret;
 }
 EXPORT_SYMBOL_GPL(bq27xxx_battery_setup);
 
@@ -988,6 +1032,8 @@ void bq27xxx_battery_teardown(struct bq27xxx_device_info 
*di)
 
cancel_delayed_work_sync(>work);
 
+   sysfs_remove_file(>dev->kobj, _attr_poll_interval.attr);
+
power_supply_unregister(di->bat);
 
mutex_destroy(>lock);
-- 
2.7.4