from:"Liam Breck"

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 <hdego...@redhat.com> 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 <hdego...@redhat.com>
>> 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 <hdego...@redhat.com>
>>> ---
>>> 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,
>>>

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 <hdego...@redhat.com> 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 <hdego...@redhat.com>
>> 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 <hdego...@redhat.com>
>>> ---
>>> 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_

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")) {

Re: [PATCH 13/18] power: supply: bq24190_charger: Export 5V boost converter as regulator

2017-08-08 Thread Liam Breck

Hi Hans,

On Sun, Aug 6, 2017 at 5:35 AM, Hans de Goede  wrote:
> Register the 5V boost converter as a regulator named
> "regulator-bq24190-usb-vbus". Note the name includes "bq24190" because
> the bq24190 family is also used on ACPI devices where there are no
> device-tree phandles, so regulator_get will fallback to the name and thus
> it must be unique on the system.

What we're enabling here is 5V boost for otg host mode, not vbus
generally, so maybe the name should indicate that...

regulator-bq24190-usb-5volt
regulator-bq24190-usb-host
regulator-bq24190-usb-otg-5v

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

Re: [PATCH 16/18] power: supply: bq24190_charger: Remove extcon handling

2017-08-08 Thread Liam Breck

Hi Hans,

On Sun, Aug 6, 2017 at 5:35 AM, Hans de Goede  wrote:
> Now that drivers/i2c/busses/i2c-cht-wc.c uses
> "input-current-limit-from-supplier" instead of "extcon-name" the last
> user of the bq24190 extcon code is gone, remove it.
>
> Signed-off-by: Hans de Goede 
> ---
>  drivers/power/supply/bq24190_charger.c | 107 
> -
>  1 file changed, 107 deletions(-)
>
> diff --git a/drivers/power/supply/bq24190_charger.c 
> b/drivers/power/supply/bq24190_charger.c
> index 1f6424f0772f..0376de6d8e70 100644
> --- a/drivers/power/supply/bq24190_charger.c
> +++ b/drivers/power/supply/bq24190_charger.c
> @@ -11,7 +11,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -155,9 +154,6 @@ struct bq24190_dev_info {
> struct device   *dev;
> struct power_supply *charger;
> struct power_supply *battery;
> -   struct extcon_dev   *extcon;
> -   struct notifier_block   extcon_nb;
> -   struct delayed_work extcon_work;
> charmodel_name[I2C_NAME_SIZE];
> boolinitialized;
> boolirq_event;
> @@ -1530,75 +1526,6 @@ static irqreturn_t bq24190_irq_handler_thread(int irq, 
> void *data)
> return IRQ_HANDLED;
>  }
>
> -static void bq24190_extcon_work(struct work_struct *work)
> -{
> -   struct bq24190_dev_info *bdi =
> -   container_of(work, struct bq24190_dev_info, extcon_work.work);
> -   int error, iinlim = 0;
> -   u8 v;
> -
> -   error = pm_runtime_get_sync(bdi->dev);
> -   if (error < 0) {
> -   dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", error);
> -   pm_runtime_put_noidle(bdi->dev);
> -   return;
> -   }
> -
> -   if  (extcon_get_state(bdi->extcon, EXTCON_CHG_USB_SDP) == 1)
> -   iinlim =  50;
> -   else if (extcon_get_state(bdi->extcon, EXTCON_CHG_USB_CDP) == 1 ||
> -extcon_get_state(bdi->extcon, EXTCON_CHG_USB_ACA) == 1)
> -   iinlim = 150;
> -   else if (extcon_get_state(bdi->extcon, EXTCON_CHG_USB_DCP) == 1)
> -   iinlim = 200;
> -
> -   if (iinlim) {
> -   error = bq24190_set_field_val(bdi, BQ24190_REG_ISC,
> - BQ24190_REG_ISC_IINLIM_MASK,
> - BQ24190_REG_ISC_IINLIM_SHIFT,
> - bq24190_isc_iinlim_values,
> - 
> ARRAY_SIZE(bq24190_isc_iinlim_values),
> - iinlim);
> -   if (error < 0)
> -   dev_err(bdi->dev, "Can't set IINLIM: %d\n", error);
> -   }
> -
> -   /* if no charger found and in USB host mode, set OTG 5V boost, else 
> normal */
> -   if (!iinlim && extcon_get_state(bdi->extcon, EXTCON_USB_HOST) == 1)
> -   v = BQ24190_REG_POC_CHG_CONFIG_OTG;
> -   else
> -   v = BQ24190_REG_POC_CHG_CONFIG_CHARGE;
> -
> -   error = bq24190_write_mask(bdi, BQ24190_REG_POC,
> -  BQ24190_REG_POC_CHG_CONFIG_MASK,
> -  BQ24190_REG_POC_CHG_CONFIG_SHIFT,
> -  v);
> -   if (error < 0)
> -   dev_err(bdi->dev, "Can't set CHG_CONFIG: %d\n", error);
> -
> -   pm_runtime_mark_last_busy(bdi->dev);
> -   pm_runtime_put_autosuspend(bdi->dev);
> -}
> -
> -static int bq24190_extcon_event(struct notifier_block *nb, unsigned long 
> event,
> -   void *param)
> -{
> -   struct bq24190_dev_info *bdi =
> -   container_of(nb, struct bq24190_dev_info, extcon_nb);
> -
> -   /*
> -* 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 applying the extcon value
> -* for 300ms to avoid this.
> -*/
> -   queue_delayed_work(system_wq, >extcon_work, 
> msecs_to_jiffies(300));
> -
> -   return NOTIFY_OK;
> -}
> -
>  static int bq24190_hw_init(struct bq24190_dev_info *bdi)
>  {
> u8 v;
> @@ -1636,7 +1563,6 @@ static int bq24190_probe(struct i2c_client *client,
> struct device *dev = >dev;
> struct power_supply_config charger_cfg = {}, battery_cfg = {};
> struct bq24190_dev_info *bdi;
> -   const char *name;
> int ret;
>
> if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
> @@ -1668,25 +1594,6 @@ static int

Re: [PATCH 15/18] power: supply: bq24190_charger: Get input_current_limit from our supplier

2017-08-08 Thread Liam Breck

Hallo Hans :-)


On Sun, Aug 6, 2017 at 5:35 AM, 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 
> ---
>  drivers/power/supply/bq24190_charger.c | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/power/supply/bq24190_charger.c 
> b/drivers/power/supply/bq24190_charger.c
> index d78e2c6dc127..1f6424f0772f 100644
> --- a/drivers/power/supply/bq24190_charger.c
> +++ b/drivers/power/supply/bq24190_charger.c
> @@ -161,6 +161,7 @@ struct bq24190_dev_info {
> charmodel_name[I2C_NAME_SIZE];
> boolinitialized;
> boolirq_event;
> +   boolinput_current_limit_from_supplier;
> struct mutexf_reg_lock;
> u8  f_reg;
> u8  ss_reg;
> @@ -1137,6 +1138,14 @@ static int 
> bq24190_charger_property_is_writeable(struct power_supply *psy,
> return ret;
>  }
>
> +static void bq24190_charger_external_power_changed(struct power_supply *psy)
> +{
> +   struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy);
> +
> +   if (bdi->input_current_limit_from_supplier)
> +   power_supply_set_input_current_limit_from_supplier(psy);
> +}
> +
>  static enum power_supply_property bq24190_charger_properties[] = {
> POWER_SUPPLY_PROP_CHARGE_TYPE,
> POWER_SUPPLY_PROP_HEALTH,
> @@ -1165,6 +1174,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 */
> @@ -1654,6 +1664,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");

Since this invokes the new power_supply_class function, should this be
a devicetree property, not just a driver-to-driver switch? If so, the
property name should probably be defined in
Doc...bindings/power/supply/power_supply.txt.

My latest bq24190 series has a new DT doc, which should ref a new DT property.
___
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 13/18] power: supply: bq24190_charger: Export 5V boost converter as regulator

Re: [PATCH 16/18] power: supply: bq24190_charger: Remove extcon handling

Re: [PATCH 15/18] power: supply: bq24190_charger: Get input_current_limit from our supplier

6 matches

Site Navigation

Mail list logo

Footer information