subject:"\[PATCH v2 11\/14\] power\: supply\: bq24190_charger\: Get input_current

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

2017-08-29 Thread Sebastian Reichel

Hi,

On Tue, Aug 29, 2017 at 01:53:24PM +0200, Hans de Goede wrote:
> Hi,
> 
> Thank you for your reviews / queuing!
> 
> On 29-08-17 13:40, Sebastian Reichel wrote:
> > Hi,
> > 
> > On Tue, Aug 15, 2017 at 10:04:59PM +0200, Hans de Goede wrote:
> > > On some devices the USB Type-C port power (USB PD 2.0) negotiation is
> > > done by a separate port-controller IC, while the current limit is
> > > controlled through another (charger) IC.
> > > 
> > > It has been decided to model this by modelling the external Type-C
> > > power brick (adapter/charger) as a power-supply class device which
> > > supplies the charger-IC, with its voltage-now and current-max representing
> > > the negotiated voltage and max current draw.
> > > 
> > > This commit adds support for this to the bq24190_charger driver by calling
> > > power_supply_set_input_current_limit_from_supplier helper if the
> > > "input-current-limit-from-supplier" device-property is set.
> > > 
> > > Note this replaces the functionality to get the current-limit from an
> > > extcon device, which will be removed in a follow-up commit.
> > 
> > I'm fine with the general approach, but ...
> > 
> > > [...]
> > > + bdi->input_current_limit_from_supplier =
> > > + device_property_read_bool(dev,
> > > +   "input-current-limit-from-supplier");
> > > [...]
> > 
> > I wonder if we actually need this. I think we can just enable it
> > unconditionally when we have a parent power supply providing the
> > information.
> 
> I was thinking the same when implementing this, so this is fine with
> me. I think it is best to just unconditionally call
> power_supply_set_input_current_limit_from_supplier from the
> external_power_changed callback, that will only get called if we've
> a parent supply and that function will check that the parent has
> a current-max property itself.
> 
> Please let me know if just unconditionally calling
> power_supply_set_input_current_limit_from_supplier from the
> external_power_changed callback is ok with you then I will do that
> for v3 of the patch-set (from which I will drop the patches you've
> already queued).

Makes sense to me.

-- Sebastian


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

2017-08-29 Thread Hans de Goede


Hi,

Thank you for your reviews / queuing!

On 29-08-17 13:40, Sebastian Reichel wrote:

Hi,

On Tue, Aug 15, 2017 at 10:04:59PM +0200, Hans de Goede wrote:

On some devices the USB Type-C port power (USB PD 2.0) negotiation is
done by a separate port-controller IC, while the current limit is
controlled through another (charger) IC.

It has been decided to model this by modelling the external Type-C
power brick (adapter/charger) as a power-supply class device which
supplies the charger-IC, with its voltage-now and current-max representing
the negotiated voltage and max current draw.

This commit adds support for this to the bq24190_charger driver by calling
power_supply_set_input_current_limit_from_supplier helper if the
"input-current-limit-from-supplier" device-property is set.

Note this replaces the functionality to get the current-limit from an
extcon device, which will be removed in a follow-up commit.


I'm fine with the general approach, but ...


[...]
+   bdi->input_current_limit_from_supplier =
+   device_property_read_bool(dev,
+ "input-current-limit-from-supplier");
[...]


I wonder if we actually need this. I think we can just enable it
unconditionally when we have a parent power supply providing the
information.


I was thinking the same when implementing this, so this is fine with
me. I think it is best to just unconditionally call
power_supply_set_input_current_limit_from_supplier from the
external_power_changed callback, that will only get called if we've
a parent supply and that function will check that the parent has
a current-max property itself.

Please let me know if just unconditionally calling
power_supply_set_input_current_limit_from_supplier from the
external_power_changed callback is ok with you then I will do that
for v3 of the patch-set (from which I will drop the patches you've
already queued).

Regards,

Hans
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

2017-08-29 Thread Sebastian Reichel

Hi,

On Tue, Aug 15, 2017 at 10:04:59PM +0200, Hans de Goede wrote:
> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
> done by a separate port-controller IC, while the current limit is
> controlled through another (charger) IC.
> 
> It has been decided to model this by modelling the external Type-C
> power brick (adapter/charger) as a power-supply class device which
> supplies the charger-IC, with its voltage-now and current-max representing
> the negotiated voltage and max current draw.
> 
> This commit adds support for this to the bq24190_charger driver by calling
> power_supply_set_input_current_limit_from_supplier helper if the
> "input-current-limit-from-supplier" device-property is set.
> 
> Note this replaces the functionality to get the current-limit from an
> extcon device, which will be removed in a follow-up commit.

I'm fine with the general approach, but ...

> [...]
> + bdi->input_current_limit_from_supplier =
> + device_property_read_bool(dev,
> +   "input-current-limit-from-supplier");
> [...]

I wonder if we actually need this. I think we can just enable it
unconditionally when we have a parent power supply providing the
information.

-- Sebastian


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

2017-08-28 Thread Hans de Goede


Hi,

On 28-08-17 20:07, Liam Breck wrote:

Hi Hans, I sent too soon...

On Mon, Aug 28, 2017 at 9:04 AM, Hans de Goede  wrote:

Hi,


On 16-08-17 22:28, Liam Breck wrote:


Hi Hans,

On Tue, Aug 15, 2017 at 1:04 PM, Hans de Goede 
wrote:


On some devices the USB Type-C port power (USB PD 2.0) negotiation is
done by a separate port-controller IC, while the current limit is
controlled through another (charger) IC.

It has been decided to model this by modelling the external Type-C
power brick (adapter/charger) as a power-supply class device which
supplies the charger-IC, with its voltage-now and current-max
representing
the negotiated voltage and max current draw.

This commit adds support for this to the bq24190_charger driver by
calling
power_supply_set_input_current_limit_from_supplier helper if the
"input-current-limit-from-supplier" device-property is set.

Note this replaces the functionality to get the current-limit from an
extcon device, which will be removed in a follow-up commit.

Signed-off-by: Hans de Goede 
---
Changes in v2:
-Wait a bit before applying current-max from our supplier as
input-current-limit
   the bq24190 may still be in its power-good wait-state while our
supplier is
   done negotating current-max and if we apply the limit to early then the
   input-current-limit will be reset to 0.5A by the bq24190 after its
   power-good wait is done.
---
   drivers/power/supply/bq24190_charger.c | 35
++
   1 file changed, 35 insertions(+)

diff --git a/drivers/power/supply/bq24190_charger.c
b/drivers/power/supply/bq24190_charger.c
index f13f892..6f75c8e 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -159,9 +159,11 @@ struct bq24190_dev_info {
  struct extcon_dev   *extcon;
  struct notifier_block   extcon_nb;
  struct delayed_work extcon_work;
+   struct delayed_work input_current_limit_work;
  charmodel_name[I2C_NAME_SIZE];
  boolinitialized;
  boolirq_event;
+   bool
input_current_limit_from_supplier;
  struct mutexf_reg_lock;
  u8  f_reg;
  u8  ss_reg;
@@ -1142,6 +1144,32 @@ static int
bq24190_charger_property_is_writeable(struct power_supply *psy,
  return ret;
   }

+static void bq24190_input_current_limit_work(struct work_struct *work)
+{
+   struct bq24190_dev_info *bdi =
+   container_of(work, struct bq24190_dev_info,
+input_current_limit_work.work);
+
+   power_supply_set_input_current_limit_from_supplier(bdi->charger);
+}
+
+static void bq24190_charger_external_power_changed(struct power_supply
*psy)
+{
+   struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy);
+
+   /*
+* The Power-Good detection may take up to 220ms, sometimes
+* the external charger detection is quicker, and the bq24190
will
+* reset to iinlim based on its own charger detection (which is
not
+* hooked up when using external charger detection) resulting in
a
+* too low default 500mA iinlim. Delay setting the
input-current-limit
+* for 300ms to avoid this.
+*/
+   if (bdi->input_current_limit_from_supplier)
+   queue_delayed_work(system_wq,
>input_current_limit_work,
+  msecs_to_jiffies(300));
+}
+
   static enum power_supply_property bq24190_charger_properties[] = {
  POWER_SUPPLY_PROP_CHARGE_TYPE,
  POWER_SUPPLY_PROP_HEALTH,
@@ -1170,6 +1198,7 @@ static const struct power_supply_desc
bq24190_charger_desc = {
  .get_property   = bq24190_charger_get_property,
  .set_property   = bq24190_charger_set_property,
  .property_is_writeable  = bq24190_charger_property_is_writeable,
+   .external_power_changed = bq24190_charger_external_power_changed,
   };

   /* Battery power supply property routines */
@@ -1651,6 +1680,8 @@ static int bq24190_probe(struct i2c_client *client,
  mutex_init(>f_reg_lock);
  bdi->f_reg = 0;
  bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state
*/
+   INIT_DELAYED_WORK(>input_current_limit_work,
+ bq24190_input_current_limit_work);

  i2c_set_clientdata(client, bdi);

@@ -1659,6 +1690,10 @@ static int bq24190_probe(struct i2c_client
*client,
  return -EINVAL;
  }

+   bdi->input_current_limit_from_supplier =
+   device_property_read_bool(dev,
+
"input-current-limit-from-supplier");
+



Maybe
if (device_property_read_bool(dev,
"linux,input-current-limit-from-supplier")) {

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

2017-08-28 Thread Liam Breck

Hi Hans, I sent too soon...

On Mon, Aug 28, 2017 at 9:04 AM, Hans de Goede  wrote:
> Hi,
>
>
> On 16-08-17 22:28, Liam Breck wrote:
>>
>> Hi Hans,
>>
>> On Tue, Aug 15, 2017 at 1:04 PM, Hans de Goede 
>> wrote:
>>>
>>> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
>>> done by a separate port-controller IC, while the current limit is
>>> controlled through another (charger) IC.
>>>
>>> It has been decided to model this by modelling the external Type-C
>>> power brick (adapter/charger) as a power-supply class device which
>>> supplies the charger-IC, with its voltage-now and current-max
>>> representing
>>> the negotiated voltage and max current draw.
>>>
>>> This commit adds support for this to the bq24190_charger driver by
>>> calling
>>> power_supply_set_input_current_limit_from_supplier helper if the
>>> "input-current-limit-from-supplier" device-property is set.
>>>
>>> Note this replaces the functionality to get the current-limit from an
>>> extcon device, which will be removed in a follow-up commit.
>>>
>>> Signed-off-by: Hans de Goede 
>>> ---
>>> Changes in v2:
>>> -Wait a bit before applying current-max from our supplier as
>>> input-current-limit
>>>   the bq24190 may still be in its power-good wait-state while our
>>> supplier is
>>>   done negotating current-max and if we apply the limit to early then the
>>>   input-current-limit will be reset to 0.5A by the bq24190 after its
>>>   power-good wait is done.
>>> ---
>>>   drivers/power/supply/bq24190_charger.c | 35
>>> ++
>>>   1 file changed, 35 insertions(+)
>>>
>>> diff --git a/drivers/power/supply/bq24190_charger.c
>>> b/drivers/power/supply/bq24190_charger.c
>>> index f13f892..6f75c8e 100644
>>> --- a/drivers/power/supply/bq24190_charger.c
>>> +++ b/drivers/power/supply/bq24190_charger.c
>>> @@ -159,9 +159,11 @@ struct bq24190_dev_info {
>>>  struct extcon_dev   *extcon;
>>>  struct notifier_block   extcon_nb;
>>>  struct delayed_work extcon_work;
>>> +   struct delayed_work input_current_limit_work;
>>>  charmodel_name[I2C_NAME_SIZE];
>>>  boolinitialized;
>>>  boolirq_event;
>>> +   bool
>>> input_current_limit_from_supplier;
>>>  struct mutexf_reg_lock;
>>>  u8  f_reg;
>>>  u8  ss_reg;
>>> @@ -1142,6 +1144,32 @@ static int
>>> bq24190_charger_property_is_writeable(struct power_supply *psy,
>>>  return ret;
>>>   }
>>>
>>> +static void bq24190_input_current_limit_work(struct work_struct *work)
>>> +{
>>> +   struct bq24190_dev_info *bdi =
>>> +   container_of(work, struct bq24190_dev_info,
>>> +input_current_limit_work.work);
>>> +
>>> +   power_supply_set_input_current_limit_from_supplier(bdi->charger);
>>> +}
>>> +
>>> +static void bq24190_charger_external_power_changed(struct power_supply
>>> *psy)
>>> +{
>>> +   struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy);
>>> +
>>> +   /*
>>> +* The Power-Good detection may take up to 220ms, sometimes
>>> +* the external charger detection is quicker, and the bq24190
>>> will
>>> +* reset to iinlim based on its own charger detection (which is
>>> not
>>> +* hooked up when using external charger detection) resulting in
>>> a
>>> +* too low default 500mA iinlim. Delay setting the
>>> input-current-limit
>>> +* for 300ms to avoid this.
>>> +*/
>>> +   if (bdi->input_current_limit_from_supplier)
>>> +   queue_delayed_work(system_wq,
>>> >input_current_limit_work,
>>> +  msecs_to_jiffies(300));
>>> +}
>>> +
>>>   static enum power_supply_property bq24190_charger_properties[] = {
>>>  POWER_SUPPLY_PROP_CHARGE_TYPE,
>>>  POWER_SUPPLY_PROP_HEALTH,
>>> @@ -1170,6 +1198,7 @@ static const struct power_supply_desc
>>> bq24190_charger_desc = {
>>>  .get_property   = bq24190_charger_get_property,
>>>  .set_property   = bq24190_charger_set_property,
>>>  .property_is_writeable  = bq24190_charger_property_is_writeable,
>>> +   .external_power_changed = bq24190_charger_external_power_changed,
>>>   };
>>>
>>>   /* Battery power supply property routines */
>>> @@ -1651,6 +1680,8 @@ static int bq24190_probe(struct i2c_client *client,
>>>  mutex_init(>f_reg_lock);
>>>  bdi->f_reg = 0;
>>>  bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state
>>> */
>>> +   INIT_DELAYED_WORK(>input_current_limit_work,
>>> + bq24190_input_current_limit_work);
>>>
>>>  i2c_set_clientdata(client, bdi);
>>>
>>> @@ -1659,6 +1690,10 @@

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

2017-08-28 Thread Liam Breck

Hi Hans,

On Mon, Aug 28, 2017 at 9:04 AM, Hans de Goede  wrote:
> Hi,
>
>
> On 16-08-17 22:28, Liam Breck wrote:
>>
>> Hi Hans,
>>
>> On Tue, Aug 15, 2017 at 1:04 PM, Hans de Goede 
>> wrote:
>>>
>>> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
>>> done by a separate port-controller IC, while the current limit is
>>> controlled through another (charger) IC.
>>>
>>> It has been decided to model this by modelling the external Type-C
>>> power brick (adapter/charger) as a power-supply class device which
>>> supplies the charger-IC, with its voltage-now and current-max
>>> representing
>>> the negotiated voltage and max current draw.
>>>
>>> This commit adds support for this to the bq24190_charger driver by
>>> calling
>>> power_supply_set_input_current_limit_from_supplier helper if the
>>> "input-current-limit-from-supplier" device-property is set.
>>>
>>> Note this replaces the functionality to get the current-limit from an
>>> extcon device, which will be removed in a follow-up commit.
>>>
>>> Signed-off-by: Hans de Goede 
>>> ---
>>> Changes in v2:
>>> -Wait a bit before applying current-max from our supplier as
>>> input-current-limit
>>>   the bq24190 may still be in its power-good wait-state while our
>>> supplier is
>>>   done negotating current-max and if we apply the limit to early then the
>>>   input-current-limit will be reset to 0.5A by the bq24190 after its
>>>   power-good wait is done.
>>> ---
>>>   drivers/power/supply/bq24190_charger.c | 35
>>> ++
>>>   1 file changed, 35 insertions(+)
>>>
>>> diff --git a/drivers/power/supply/bq24190_charger.c
>>> b/drivers/power/supply/bq24190_charger.c
>>> index f13f892..6f75c8e 100644
>>> --- a/drivers/power/supply/bq24190_charger.c
>>> +++ b/drivers/power/supply/bq24190_charger.c
>>> @@ -159,9 +159,11 @@ struct bq24190_dev_info {
>>>  struct extcon_dev   *extcon;
>>>  struct notifier_block   extcon_nb;
>>>  struct delayed_work extcon_work;
>>> +   struct delayed_work input_current_limit_work;
>>>  charmodel_name[I2C_NAME_SIZE];
>>>  boolinitialized;
>>>  boolirq_event;
>>> +   bool
>>> input_current_limit_from_supplier;
>>>  struct mutexf_reg_lock;
>>>  u8  f_reg;
>>>  u8  ss_reg;
>>> @@ -1142,6 +1144,32 @@ static int
>>> bq24190_charger_property_is_writeable(struct power_supply *psy,
>>>  return ret;
>>>   }
>>>
>>> +static void bq24190_input_current_limit_work(struct work_struct *work)
>>> +{
>>> +   struct bq24190_dev_info *bdi =
>>> +   container_of(work, struct bq24190_dev_info,
>>> +input_current_limit_work.work);
>>> +
>>> +   power_supply_set_input_current_limit_from_supplier(bdi->charger);
>>> +}
>>> +
>>> +static void bq24190_charger_external_power_changed(struct power_supply
>>> *psy)
>>> +{
>>> +   struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy);
>>> +
>>> +   /*
>>> +* The Power-Good detection may take up to 220ms, sometimes
>>> +* the external charger detection is quicker, and the bq24190
>>> will
>>> +* reset to iinlim based on its own charger detection (which is
>>> not
>>> +* hooked up when using external charger detection) resulting in
>>> a
>>> +* too low default 500mA iinlim. Delay setting the
>>> input-current-limit
>>> +* for 300ms to avoid this.
>>> +*/
>>> +   if (bdi->input_current_limit_from_supplier)
>>> +   queue_delayed_work(system_wq,
>>> >input_current_limit_work,
>>> +  msecs_to_jiffies(300));
>>> +}
>>> +
>>>   static enum power_supply_property bq24190_charger_properties[] = {
>>>  POWER_SUPPLY_PROP_CHARGE_TYPE,
>>>  POWER_SUPPLY_PROP_HEALTH,
>>> @@ -1170,6 +1198,7 @@ static const struct power_supply_desc
>>> bq24190_charger_desc = {
>>>  .get_property   = bq24190_charger_get_property,
>>>  .set_property   = bq24190_charger_set_property,
>>>  .property_is_writeable  = bq24190_charger_property_is_writeable,
>>> +   .external_power_changed = bq24190_charger_external_power_changed,
>>>   };
>>>
>>>   /* Battery power supply property routines */
>>> @@ -1651,6 +1680,8 @@ static int bq24190_probe(struct i2c_client *client,
>>>  mutex_init(>f_reg_lock);
>>>  bdi->f_reg = 0;
>>>  bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state
>>> */
>>> +   INIT_DELAYED_WORK(>input_current_limit_work,
>>> + bq24190_input_current_limit_work);
>>>
>>>  i2c_set_clientdata(client, bdi);
>>>
>>> @@ -1659,6 +1690,10 @@ static int

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

2017-08-28 Thread Hans de Goede


Hi,

On 16-08-17 22:28, Liam Breck wrote:

Hi Hans,

On Tue, Aug 15, 2017 at 1:04 PM, Hans de Goede  wrote:

On some devices the USB Type-C port power (USB PD 2.0) negotiation is
done by a separate port-controller IC, while the current limit is
controlled through another (charger) IC.

It has been decided to model this by modelling the external Type-C
power brick (adapter/charger) as a power-supply class device which
supplies the charger-IC, with its voltage-now and current-max representing
the negotiated voltage and max current draw.

This commit adds support for this to the bq24190_charger driver by calling
power_supply_set_input_current_limit_from_supplier helper if the
"input-current-limit-from-supplier" device-property is set.

Note this replaces the functionality to get the current-limit from an
extcon device, which will be removed in a follow-up commit.

Signed-off-by: Hans de Goede 
---
Changes in v2:
-Wait a bit before applying current-max from our supplier as input-current-limit
  the bq24190 may still be in its power-good wait-state while our supplier is
  done negotating current-max and if we apply the limit to early then the
  input-current-limit will be reset to 0.5A by the bq24190 after its
  power-good wait is done.
---
  drivers/power/supply/bq24190_charger.c | 35 ++
  1 file changed, 35 insertions(+)

diff --git a/drivers/power/supply/bq24190_charger.c 
b/drivers/power/supply/bq24190_charger.c
index f13f892..6f75c8e 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -159,9 +159,11 @@ struct bq24190_dev_info {
 struct extcon_dev   *extcon;
 struct notifier_block   extcon_nb;
 struct delayed_work extcon_work;
+   struct delayed_work input_current_limit_work;
 charmodel_name[I2C_NAME_SIZE];
 boolinitialized;
 boolirq_event;
+   boolinput_current_limit_from_supplier;
 struct mutexf_reg_lock;
 u8  f_reg;
 u8  ss_reg;
@@ -1142,6 +1144,32 @@ static int bq24190_charger_property_is_writeable(struct 
power_supply *psy,
 return ret;
  }

+static void bq24190_input_current_limit_work(struct work_struct *work)
+{
+   struct bq24190_dev_info *bdi =
+   container_of(work, struct bq24190_dev_info,
+input_current_limit_work.work);
+
+   power_supply_set_input_current_limit_from_supplier(bdi->charger);
+}
+
+static void bq24190_charger_external_power_changed(struct power_supply *psy)
+{
+   struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy);
+
+   /*
+* The Power-Good detection may take up to 220ms, sometimes
+* the external charger detection is quicker, and the bq24190 will
+* reset to iinlim based on its own charger detection (which is not
+* hooked up when using external charger detection) resulting in a
+* too low default 500mA iinlim. Delay setting the input-current-limit
+* for 300ms to avoid this.
+*/
+   if (bdi->input_current_limit_from_supplier)
+   queue_delayed_work(system_wq, >input_current_limit_work,
+  msecs_to_jiffies(300));
+}
+
  static enum power_supply_property bq24190_charger_properties[] = {
 POWER_SUPPLY_PROP_CHARGE_TYPE,
 POWER_SUPPLY_PROP_HEALTH,
@@ -1170,6 +1198,7 @@ static const struct power_supply_desc 
bq24190_charger_desc = {
 .get_property   = bq24190_charger_get_property,
 .set_property   = bq24190_charger_set_property,
 .property_is_writeable  = bq24190_charger_property_is_writeable,
+   .external_power_changed = bq24190_charger_external_power_changed,
  };

  /* Battery power supply property routines */
@@ -1651,6 +1680,8 @@ static int bq24190_probe(struct i2c_client *client,
 mutex_init(>f_reg_lock);
 bdi->f_reg = 0;
 bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state */
+   INIT_DELAYED_WORK(>input_current_limit_work,
+ bq24190_input_current_limit_work);

 i2c_set_clientdata(client, bdi);

@@ -1659,6 +1690,10 @@ static int bq24190_probe(struct i2c_client *client,
 return -EINVAL;
 }

+   bdi->input_current_limit_from_supplier =
+   device_property_read_bool(dev,
+ "input-current-limit-from-supplier");
+


Maybe
   if (device_property_read_bool(dev,
"linux,input-current-limit-from-supplier")) {
INIT_DELAYED_WORK(>input_current_limit_work,
  bq24190_input_current_limit_work);
   bdi->input_current_limit_from_supplier =

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

2017-08-16 Thread Liam Breck

Hi Hans,

On Tue, Aug 15, 2017 at 1:04 PM, Hans de Goede  wrote:
> On some devices the USB Type-C port power (USB PD 2.0) negotiation is
> done by a separate port-controller IC, while the current limit is
> controlled through another (charger) IC.
>
> It has been decided to model this by modelling the external Type-C
> power brick (adapter/charger) as a power-supply class device which
> supplies the charger-IC, with its voltage-now and current-max representing
> the negotiated voltage and max current draw.
>
> This commit adds support for this to the bq24190_charger driver by calling
> power_supply_set_input_current_limit_from_supplier helper if the
> "input-current-limit-from-supplier" device-property is set.
>
> Note this replaces the functionality to get the current-limit from an
> extcon device, which will be removed in a follow-up commit.
>
> Signed-off-by: Hans de Goede 
> ---
> Changes in v2:
> -Wait a bit before applying current-max from our supplier as 
> input-current-limit
>  the bq24190 may still be in its power-good wait-state while our supplier is
>  done negotating current-max and if we apply the limit to early then the
>  input-current-limit will be reset to 0.5A by the bq24190 after its
>  power-good wait is done.
> ---
>  drivers/power/supply/bq24190_charger.c | 35 
> ++
>  1 file changed, 35 insertions(+)
>
> diff --git a/drivers/power/supply/bq24190_charger.c 
> b/drivers/power/supply/bq24190_charger.c
> index f13f892..6f75c8e 100644
> --- a/drivers/power/supply/bq24190_charger.c
> +++ b/drivers/power/supply/bq24190_charger.c
> @@ -159,9 +159,11 @@ struct bq24190_dev_info {
> struct extcon_dev   *extcon;
> struct notifier_block   extcon_nb;
> struct delayed_work extcon_work;
> +   struct delayed_work input_current_limit_work;
> charmodel_name[I2C_NAME_SIZE];
> boolinitialized;
> boolirq_event;
> +   boolinput_current_limit_from_supplier;
> struct mutexf_reg_lock;
> u8  f_reg;
> u8  ss_reg;
> @@ -1142,6 +1144,32 @@ static int 
> bq24190_charger_property_is_writeable(struct power_supply *psy,
> return ret;
>  }
>
> +static void bq24190_input_current_limit_work(struct work_struct *work)
> +{
> +   struct bq24190_dev_info *bdi =
> +   container_of(work, struct bq24190_dev_info,
> +input_current_limit_work.work);
> +
> +   power_supply_set_input_current_limit_from_supplier(bdi->charger);
> +}
> +
> +static void bq24190_charger_external_power_changed(struct power_supply *psy)
> +{
> +   struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy);
> +
> +   /*
> +* The Power-Good detection may take up to 220ms, sometimes
> +* the external charger detection is quicker, and the bq24190 will
> +* reset to iinlim based on its own charger detection (which is not
> +* hooked up when using external charger detection) resulting in a
> +* too low default 500mA iinlim. Delay setting the input-current-limit
> +* for 300ms to avoid this.
> +*/
> +   if (bdi->input_current_limit_from_supplier)
> +   queue_delayed_work(system_wq, >input_current_limit_work,
> +  msecs_to_jiffies(300));
> +}
> +
>  static enum power_supply_property bq24190_charger_properties[] = {
> POWER_SUPPLY_PROP_CHARGE_TYPE,
> POWER_SUPPLY_PROP_HEALTH,
> @@ -1170,6 +1198,7 @@ static const struct power_supply_desc 
> bq24190_charger_desc = {
> .get_property   = bq24190_charger_get_property,
> .set_property   = bq24190_charger_set_property,
> .property_is_writeable  = bq24190_charger_property_is_writeable,
> +   .external_power_changed = bq24190_charger_external_power_changed,
>  };
>
>  /* Battery power supply property routines */
> @@ -1651,6 +1680,8 @@ static int bq24190_probe(struct i2c_client *client,
> mutex_init(>f_reg_lock);
> bdi->f_reg = 0;
> bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state */
> +   INIT_DELAYED_WORK(>input_current_limit_work,
> + bq24190_input_current_limit_work);
>
> i2c_set_clientdata(client, bdi);
>
> @@ -1659,6 +1690,10 @@ static int bq24190_probe(struct i2c_client *client,
> return -EINVAL;
> }
>
> +   bdi->input_current_limit_from_supplier =
> +   device_property_read_bool(dev,
> + 
> "input-current-limit-from-supplier");
> +

Maybe
  if (device_property_read_bool(dev,
"linux,input-current-limit-from-supplier")) {

[PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

2017-08-15 Thread Hans de Goede

On some devices the USB Type-C port power (USB PD 2.0) negotiation is
done by a separate port-controller IC, while the current limit is
controlled through another (charger) IC.

It has been decided to model this by modelling the external Type-C
power brick (adapter/charger) as a power-supply class device which
supplies the charger-IC, with its voltage-now and current-max representing
the negotiated voltage and max current draw.

This commit adds support for this to the bq24190_charger driver by calling
power_supply_set_input_current_limit_from_supplier helper if the
"input-current-limit-from-supplier" device-property is set.

Note this replaces the functionality to get the current-limit from an
extcon device, which will be removed in a follow-up commit.

Signed-off-by: Hans de Goede 
---
Changes in v2:
-Wait a bit before applying current-max from our supplier as input-current-limit
 the bq24190 may still be in its power-good wait-state while our supplier is
 done negotating current-max and if we apply the limit to early then the
 input-current-limit will be reset to 0.5A by the bq24190 after its
 power-good wait is done.
---
 drivers/power/supply/bq24190_charger.c | 35 ++
 1 file changed, 35 insertions(+)

diff --git a/drivers/power/supply/bq24190_charger.c 
b/drivers/power/supply/bq24190_charger.c
index f13f892..6f75c8e 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -159,9 +159,11 @@ struct bq24190_dev_info {
struct extcon_dev   *extcon;
struct notifier_block   extcon_nb;
struct delayed_work extcon_work;
+   struct delayed_work input_current_limit_work;
charmodel_name[I2C_NAME_SIZE];
boolinitialized;
boolirq_event;
+   boolinput_current_limit_from_supplier;
struct mutexf_reg_lock;
u8  f_reg;
u8  ss_reg;
@@ -1142,6 +1144,32 @@ static int bq24190_charger_property_is_writeable(struct 
power_supply *psy,
return ret;
 }
 
+static void bq24190_input_current_limit_work(struct work_struct *work)
+{
+   struct bq24190_dev_info *bdi =
+   container_of(work, struct bq24190_dev_info,
+input_current_limit_work.work);
+
+   power_supply_set_input_current_limit_from_supplier(bdi->charger);
+}
+
+static void bq24190_charger_external_power_changed(struct power_supply *psy)
+{
+   struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy);
+
+   /*
+* The Power-Good detection may take up to 220ms, sometimes
+* the external charger detection is quicker, and the bq24190 will
+* reset to iinlim based on its own charger detection (which is not
+* hooked up when using external charger detection) resulting in a
+* too low default 500mA iinlim. Delay setting the input-current-limit
+* for 300ms to avoid this.
+*/
+   if (bdi->input_current_limit_from_supplier)
+   queue_delayed_work(system_wq, >input_current_limit_work,
+  msecs_to_jiffies(300));
+}
+
 static enum power_supply_property bq24190_charger_properties[] = {
POWER_SUPPLY_PROP_CHARGE_TYPE,
POWER_SUPPLY_PROP_HEALTH,
@@ -1170,6 +1198,7 @@ static const struct power_supply_desc 
bq24190_charger_desc = {
.get_property   = bq24190_charger_get_property,
.set_property   = bq24190_charger_set_property,
.property_is_writeable  = bq24190_charger_property_is_writeable,
+   .external_power_changed = bq24190_charger_external_power_changed,
 };
 
 /* Battery power supply property routines */
@@ -1651,6 +1680,8 @@ static int bq24190_probe(struct i2c_client *client,
mutex_init(>f_reg_lock);
bdi->f_reg = 0;
bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state */
+   INIT_DELAYED_WORK(>input_current_limit_work,
+ bq24190_input_current_limit_work);
 
i2c_set_clientdata(client, bdi);
 
@@ -1659,6 +1690,10 @@ static int bq24190_probe(struct i2c_client *client,
return -EINVAL;
}
 
+   bdi->input_current_limit_from_supplier =
+   device_property_read_bool(dev,
+ "input-current-limit-from-supplier");
+
/*
 * Devicetree platforms should get extcon via phandle (not yet 
supported).
 * On ACPI platforms, extcon clients may invoke us with:
-- 
2.9.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

[PATCH v2 11/14] power: supply: bq24190_charger: Get input_current_limit from our supplier

9 matches

Site Navigation

Mail list logo

Footer information