| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAKvHMgQMgkA=b_jGFMhs7-Ta=+WYjh2u8YNjON3MapA8tTju9w@mail.gmail.com>
Date: Wed, 16 Aug 2017 13:28:41 -0700
From: Liam Breck <liam@...workimprov.net>
To: Hans de Goede <hdegoede@...hat.com>
Cc: Wolfram Sang <wsa@...-dreams.de>,
Guenter Roeck <linux@...ck-us.net>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
Sebastian Reichel <sre@...nel.org>,
Darren Hart <dvhart@...radead.org>,
Andy Shevchenko <andy@...radead.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Tony Lindgren <tony@...mide.com>, linux-i2c@...r.kernel.org,
linux-pm@...r.kernel.org, platform-driver-x86@...r.kernel.org,
linux-kernel <linux-kernel@...r.kernel.org>,
devel@...verdev.osuosl.org
Subject: Re: [PATCH v2 11/14] power: supply: bq24190_charger: Get
input_current_limit from our supplier
Hi Hans,
On Tue, Aug 15, 2017 at 1:04 PM, Hans de Goede <hdegoede@...hat.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 <hdegoede@...hat.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;
> char model_name[I2C_NAME_SIZE];
> bool initialized;
> bool irq_event;
> + bool input_current_limit_from_supplier;
> struct mutex f_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, &bdi->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(&bdi->f_reg_lock);
> bdi->f_reg = 0;
> bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state */
> + INIT_DELAYED_WORK(&bdi->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(&bdi->input_current_limit_work,
bq24190_input_current_limit_work);
bdi->input_current_limit_from_supplier = true; // or use a field
in bdi->input_current_limit_work as the flag?
}
Also should document property for DT use assuming Sebastian is ok with
new power_supply method.
And can you rebase to my pending series in next pass? There's nothing
controversial in it :-)
Powered by blists - more mailing lists