Re: [PATCH 2/2] power: bq27xxx_battery: allow kernel poll_interval parameter runtime update

2016-09-19 Thread Sebastian Reichel
Hi,

On Fri, Sep 16, 2016 at 08:42:55PM -0700, Matt Ranostay wrote:
> Fix issue with poll_interval being not updated till the previous
> interval expired.
> 
> Signed-off-by: Matt Ranostay 
> ---
>  drivers/power/supply/bq27xxx_battery.c | 38 
> +-
>  include/linux/power/bq27xxx_battery.h  |  1 +
>  2 files changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/power/supply/bq27xxx_battery.c 
> b/drivers/power/supply/bq27xxx_battery.c
> index 955424e10ae2..96fd3eec98da 100644
> --- a/drivers/power/supply/bq27xxx_battery.c
> +++ b/drivers/power/supply/bq27xxx_battery.c
> @@ -39,6 +39,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -390,8 +391,35 @@ static struct {
>   BQ27XXX_PROP(BQ27421, bq27421_battery_props),
>  };
>  
> +static DEFINE_MUTEX(param_lock);
> +static LIST_HEAD(bq27xxx_battery_devices);
> +
> +static int poll_interval_param_set(const char *val, const struct 
> kernel_param *kp)
> +{
> + struct bq27xxx_device_info *di;
> + int ret;
> +
> + ret = param_set_uint(val, kp);
> + if (ret < 0)
> + return ret;
> +
> + mutex_lock(¶m_lock);
> + list_for_each_entry(di, &bq27xxx_battery_devices, list) {
> + cancel_delayed_work_sync(&di->work);
> + schedule_delayed_work(&di->work, 0);
> + }
> + mutex_unlock(¶m_lock);
> +
> + return ret;
> +}
> +
> +static const struct kernel_param_ops param_ops_poll_interval = {
> + .get = param_get_ushort,

param_get_uint?

> + .set = poll_interval_param_set,
> +};
> +
>  static unsigned int poll_interval = 360;
> -module_param(poll_interval, uint, 0644);
> +module_param_cb(poll_interval, ¶m_ops_poll_interval, &poll_interval, 
> 0644);
>  MODULE_PARM_DESC(poll_interval,
>"battery poll interval in seconds - 0 disables polling");
>  
> @@ -1011,6 +1039,10 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info 
> *di)
>  
>   bq27xxx_battery_update(di);
>  
> + mutex_lock(¶m_lock);
> + list_add(&di->list, &bq27xxx_battery_devices);
> + mutex_unlock(¶m_lock);
> +
>   return 0;
>  
>  err_out:
> @@ -1036,6 +1068,10 @@ void bq27xxx_battery_teardown(struct 
> bq27xxx_device_info *di)
>  
>   power_supply_unregister(di->bat);
>  
> + mutex_lock(¶m_lock);
> + list_del(&di->list);
> + mutex_unlock(¶m_lock);
> +
>   mutex_destroy(&di->lock);
>  }
>  EXPORT_SYMBOL_GPL(bq27xxx_battery_teardown);
> diff --git a/include/linux/power/bq27xxx_battery.h 
> b/include/linux/power/bq27xxx_battery.h
> index b50c0492629d..e30deb046156 100644
> --- a/include/linux/power/bq27xxx_battery.h
> +++ b/include/linux/power/bq27xxx_battery.h
> @@ -58,6 +58,7 @@ struct bq27xxx_device_info {
>   unsigned long last_update;
>   struct delayed_work work;
>   struct power_supply *bat;
> + struct list_head list;
>   struct mutex lock;
>   u8 *regs;
>  };

Looks fine otherwise. When you redo the patch it would be nice to
rename param_lock into something like bq27xxx_list_lock instead,
though.

-- Sebastian


signature.asc
Description: PGP signature


[PATCH 2/2] power: bq27xxx_battery: allow kernel poll_interval parameter runtime update

2016-09-16 Thread Matt Ranostay
Fix issue with poll_interval being not updated till the previous
interval expired.

Signed-off-by: Matt Ranostay 
---
 drivers/power/supply/bq27xxx_battery.c | 38 +-
 include/linux/power/bq27xxx_battery.h  |  1 +
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/power/supply/bq27xxx_battery.c 
b/drivers/power/supply/bq27xxx_battery.c
index 955424e10ae2..96fd3eec98da 100644
--- a/drivers/power/supply/bq27xxx_battery.c
+++ b/drivers/power/supply/bq27xxx_battery.c
@@ -39,6 +39,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -390,8 +391,35 @@ static struct {
BQ27XXX_PROP(BQ27421, bq27421_battery_props),
 };
 
+static DEFINE_MUTEX(param_lock);
+static LIST_HEAD(bq27xxx_battery_devices);
+
+static int poll_interval_param_set(const char *val, const struct kernel_param 
*kp)
+{
+   struct bq27xxx_device_info *di;
+   int ret;
+
+   ret = param_set_uint(val, kp);
+   if (ret < 0)
+   return ret;
+
+   mutex_lock(¶m_lock);
+   list_for_each_entry(di, &bq27xxx_battery_devices, list) {
+   cancel_delayed_work_sync(&di->work);
+   schedule_delayed_work(&di->work, 0);
+   }
+   mutex_unlock(¶m_lock);
+
+   return ret;
+}
+
+static const struct kernel_param_ops param_ops_poll_interval = {
+   .get = param_get_ushort,
+   .set = poll_interval_param_set,
+};
+
 static unsigned int poll_interval = 360;
-module_param(poll_interval, uint, 0644);
+module_param_cb(poll_interval, ¶m_ops_poll_interval, &poll_interval, 0644);
 MODULE_PARM_DESC(poll_interval,
 "battery poll interval in seconds - 0 disables polling");
 
@@ -1011,6 +1039,10 @@ int bq27xxx_battery_setup(struct bq27xxx_device_info *di)
 
bq27xxx_battery_update(di);
 
+   mutex_lock(¶m_lock);
+   list_add(&di->list, &bq27xxx_battery_devices);
+   mutex_unlock(¶m_lock);
+
return 0;
 
 err_out:
@@ -1036,6 +1068,10 @@ void bq27xxx_battery_teardown(struct bq27xxx_device_info 
*di)
 
power_supply_unregister(di->bat);
 
+   mutex_lock(¶m_lock);
+   list_del(&di->list);
+   mutex_unlock(¶m_lock);
+
mutex_destroy(&di->lock);
 }
 EXPORT_SYMBOL_GPL(bq27xxx_battery_teardown);
diff --git a/include/linux/power/bq27xxx_battery.h 
b/include/linux/power/bq27xxx_battery.h
index b50c0492629d..e30deb046156 100644
--- a/include/linux/power/bq27xxx_battery.h
+++ b/include/linux/power/bq27xxx_battery.h
@@ -58,6 +58,7 @@ struct bq27xxx_device_info {
unsigned long last_update;
struct delayed_work work;
struct power_supply *bat;
+   struct list_head list;
struct mutex lock;
u8 *regs;
 };
-- 
2.7.4