2021-11-08 03:19:02

by Yauhen Kharuzhy

[permalink] [raw]
Subject: [PATCH 1/4] bq25890_charger: Rename IILIM field to IINLIM

Rename the Input Current Limit field in the REG00 from IILIM to IINLIM
accordingly with the bq2589x datasheet. This is just cosmetical change
to reduce confusion.

Signed-off-by: Yauhen Kharuzhy <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 34ec186a2e9a..34467bfb9537 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -40,7 +40,7 @@ static const char *const bq25890_chip_name[] = {
};

enum bq25890_fields {
- F_EN_HIZ, F_EN_ILIM, F_IILIM, /* Reg00 */
+ F_EN_HIZ, F_EN_ILIM, F_IINLIM, /* Reg00 */
F_BHOT, F_BCOLD, F_VINDPM_OFS, /* Reg01 */
F_CONV_START, F_CONV_RATE, F_BOOSTF, F_ICO_EN,
F_HVDCP_EN, F_MAXC_EN, F_FORCE_DPM, F_AUTO_DPDM_EN, /* Reg02 */
@@ -153,7 +153,7 @@ static const struct reg_field bq25890_reg_fields[] = {
/* REG00 */
[F_EN_HIZ] = REG_FIELD(0x00, 7, 7),
[F_EN_ILIM] = REG_FIELD(0x00, 6, 6),
- [F_IILIM] = REG_FIELD(0x00, 0, 5),
+ [F_IINLIM] = REG_FIELD(0x00, 0, 5),
/* REG01 */
[F_BHOT] = REG_FIELD(0x01, 6, 7),
[F_BCOLD] = REG_FIELD(0x01, 5, 5),
@@ -256,7 +256,7 @@ enum bq25890_table_ids {
/* range tables */
TBL_ICHG,
TBL_ITERM,
- TBL_IILIM,
+ TBL_IINLIM,
TBL_VREG,
TBL_BOOSTV,
TBL_SYSVMIN,
@@ -299,7 +299,7 @@ static const union {
/* TODO: BQ25896 has max ICHG 3008 mA */
[TBL_ICHG] = { .rt = {0, 5056000, 64000} }, /* uA */
[TBL_ITERM] = { .rt = {64000, 1024000, 64000} }, /* uA */
- [TBL_IILIM] = { .rt = {100000, 3250000, 50000} }, /* uA */
+ [TBL_IINLIM] = { .rt = {100000, 3250000, 50000} }, /* uA */
[TBL_VREG] = { .rt = {3840000, 4608000, 16000} }, /* uV */
[TBL_BOOSTV] = { .rt = {4550000, 5510000, 64000} }, /* uV */
[TBL_SYSVMIN] = { .rt = {3000000, 3700000, 100000} }, /* uV */
@@ -503,11 +503,11 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
break;

case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
- ret = bq25890_field_read(bq, F_IILIM);
+ ret = bq25890_field_read(bq, F_IINLIM);
if (ret < 0)
return ret;

- val->intval = bq25890_find_val(ret, TBL_IILIM);
+ val->intval = bq25890_find_val(ret, TBL_IINLIM);
break;

case POWER_SUPPLY_PROP_VOLTAGE_NOW:
--
2.33.1


2021-11-08 03:24:22

by Yauhen Kharuzhy

[permalink] [raw]
Subject: [PATCH 2/4] bq25890: Add max input current limit property

Add property 'ti,input-max-current' to define input current limit if
needed. It will be applied if automatic charger type detection is
disabled and using of ILIM pin is disabled or such pin defines greater
limit than IINLIM field.

Signed-off-by: Yauhen Kharuzhy <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 34467bfb9537..1c43555d5bd8 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -85,6 +85,7 @@ struct bq25890_init_data {
u8 treg; /* thermal regulation threshold */
u8 rbatcomp; /* IBAT sense resistor value */
u8 vclamp; /* IBAT compensation voltage limit */
+ u8 iinlim_max; /* maximum input current limit allowed */
};

struct bq25890_state {
@@ -657,6 +658,7 @@ static int bq25890_hw_init(struct bq25890_device *bq)
{F_TREG, bq->init_data.treg},
{F_BATCMP, bq->init_data.rbatcomp},
{F_VCLAMP, bq->init_data.vclamp},
+ {F_IINLIM, bq->init_data.iinlim_max},
};

ret = bq25890_chip_reset(bq);
@@ -870,11 +872,13 @@ static int bq25890_fw_read_u32_props(struct bq25890_device *bq)
{"ti,thermal-regulation-threshold", true, TBL_TREG, &init->treg},
{"ti,ibatcomp-micro-ohms", true, TBL_RBATCOMP, &init->rbatcomp},
{"ti,ibatcomp-clamp-microvolt", true, TBL_VBATCOMP, &init->vclamp},
+ {"ti,input-max-current", true, TBL_IINLIM, &init->iinlim_max},
};

/* initialize data for optional properties */
init->treg = 3; /* 120 degrees Celsius */
init->rbatcomp = init->vclamp = 0; /* IBAT compensation disabled */
+ init->iinlim_max = 0x3f;

for (i = 0; i < ARRAY_SIZE(props); i++) {
ret = device_property_read_u32(bq->dev, props[i].name,
--
2.33.1

2021-11-08 03:32:12

by Yauhen Kharuzhy

[permalink] [raw]
Subject: [PATCH 3/4] power supply bq25890-charger: Handle temperature faults

Add debug info about thermal failure to message in
bq25890_get_chip_state().

Take into account possible thermal failure when calculating a
POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX value as it described in
datasheet (in cold conditions a current limit is decreased to 20% or 50% of
ICHG field value depended on JEITA_ISET field).

Signed-off-by: Yauhen Kharuzhy <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 33 +++++++++++++++++++++++---
1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 1c43555d5bd8..fb2f1578503c 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -95,6 +95,7 @@ struct bq25890_state {
u8 vsys_status;
u8 boost_fault;
u8 bat_fault;
+ u8 ntc_fault;
};

struct bq25890_device {
@@ -384,6 +385,14 @@ enum bq25890_chrg_fault {
CHRG_FAULT_TIMER_EXPIRED,
};

+enum bq25890_ntc_fault {
+ NTC_FAULT_NORMAL = 0,
+ NTC_FAULT_WARM = 2,
+ NTC_FAULT_COOL = 3,
+ NTC_FAULT_COLD = 5,
+ NTC_FAULT_HOT = 6,
+};
+
static bool bq25890_is_adc_property(enum power_supply_property psp)
{
switch (psp) {
@@ -474,7 +483,19 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
break;

case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
+ ret = bq25890_field_read(bq, F_JEITA_ISET);
+ if (ret < 0)
+ return ret;
+
val->intval = bq25890_find_val(bq->init_data.ichg, TBL_ICHG);
+
+ /* When temperature is too low, charge current is decreased */
+ if (bq->state.ntc_fault == NTC_FAULT_COOL) {
+ if (ret)
+ val->intval /= 5;
+ else
+ val->intval /= 2;
+ }
break;

case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
@@ -487,6 +508,10 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
if (ret < 0)
return ret;

+ ret = bq25890_field_read(bq, F_JEITA_VSET);
+ if (ret < 0)
+ return ret;
+
/* converted_val = 2.304V + ADC_val * 20mV (table 10.3.15) */
val->intval = 2304000 + ret * 20000;
break;
@@ -550,7 +575,8 @@ static int bq25890_get_chip_state(struct bq25890_device *bq,
{F_VSYS_STAT, &state->vsys_status},
{F_BOOST_FAULT, &state->boost_fault},
{F_BAT_FAULT, &state->bat_fault},
- {F_CHG_FAULT, &state->chrg_fault}
+ {F_CHG_FAULT, &state->chrg_fault},
+ {F_CHG_FAULT, &state->ntc_fault}
};

for (i = 0; i < ARRAY_SIZE(state_fields); i++) {
@@ -561,9 +587,10 @@ static int bq25890_get_chip_state(struct bq25890_device *bq,
*state_fields[i].data = ret;
}

- dev_dbg(bq->dev, "S:CHG/PG/VSYS=%d/%d/%d, F:CHG/BOOST/BAT=%d/%d/%d\n",
+ dev_dbg(bq->dev, "S:CHG/PG/VSYS=%d/%d/%d, F:CHG/BOOST/BAT/NTC=%d/%d/%d/%d\n",
state->chrg_status, state->online, state->vsys_status,
- state->chrg_fault, state->boost_fault, state->bat_fault);
+ state->chrg_fault, state->boost_fault, state->bat_fault,
+ state->ntc_fault);

return 0;
}
--
2.33.1

2021-11-08 03:52:48

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 1/4] bq25890_charger: Rename IILIM field to IINLIM

Hi,

On 11/7/21 21:19, Yauhen Kharuzhy wrote:
> Rename the Input Current Limit field in the REG00 from IILIM to IINLIM
> accordingly with the bq2589x datasheet. This is just cosmetical change
> to reduce confusion.
>
> Signed-off-by: Yauhen Kharuzhy <[email protected]>

Thank you, this typo was annoying me too :)

Patch subject prefix should be: "power: supply: bq25890: "

Otherwise this looks good to me:

Reviewed-by: Hans de Goede <[email protected]>

Regards,

Hans



> ---
> drivers/power/supply/bq25890_charger.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index 34ec186a2e9a..34467bfb9537 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -40,7 +40,7 @@ static const char *const bq25890_chip_name[] = {
> };
>
> enum bq25890_fields {
> - F_EN_HIZ, F_EN_ILIM, F_IILIM, /* Reg00 */
> + F_EN_HIZ, F_EN_ILIM, F_IINLIM, /* Reg00 */
> F_BHOT, F_BCOLD, F_VINDPM_OFS, /* Reg01 */
> F_CONV_START, F_CONV_RATE, F_BOOSTF, F_ICO_EN,
> F_HVDCP_EN, F_MAXC_EN, F_FORCE_DPM, F_AUTO_DPDM_EN, /* Reg02 */
> @@ -153,7 +153,7 @@ static const struct reg_field bq25890_reg_fields[] = {
> /* REG00 */
> [F_EN_HIZ] = REG_FIELD(0x00, 7, 7),
> [F_EN_ILIM] = REG_FIELD(0x00, 6, 6),
> - [F_IILIM] = REG_FIELD(0x00, 0, 5),
> + [F_IINLIM] = REG_FIELD(0x00, 0, 5),
> /* REG01 */
> [F_BHOT] = REG_FIELD(0x01, 6, 7),
> [F_BCOLD] = REG_FIELD(0x01, 5, 5),
> @@ -256,7 +256,7 @@ enum bq25890_table_ids {
> /* range tables */
> TBL_ICHG,
> TBL_ITERM,
> - TBL_IILIM,
> + TBL_IINLIM,
> TBL_VREG,
> TBL_BOOSTV,
> TBL_SYSVMIN,
> @@ -299,7 +299,7 @@ static const union {
> /* TODO: BQ25896 has max ICHG 3008 mA */
> [TBL_ICHG] = { .rt = {0, 5056000, 64000} }, /* uA */
> [TBL_ITERM] = { .rt = {64000, 1024000, 64000} }, /* uA */
> - [TBL_IILIM] = { .rt = {100000, 3250000, 50000} }, /* uA */
> + [TBL_IINLIM] = { .rt = {100000, 3250000, 50000} }, /* uA */
> [TBL_VREG] = { .rt = {3840000, 4608000, 16000} }, /* uV */
> [TBL_BOOSTV] = { .rt = {4550000, 5510000, 64000} }, /* uV */
> [TBL_SYSVMIN] = { .rt = {3000000, 3700000, 100000} }, /* uV */
> @@ -503,11 +503,11 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
> break;
>
> case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
> - ret = bq25890_field_read(bq, F_IILIM);
> + ret = bq25890_field_read(bq, F_IINLIM);
> if (ret < 0)
> return ret;
>
> - val->intval = bq25890_find_val(ret, TBL_IILIM);
> + val->intval = bq25890_find_val(ret, TBL_IINLIM);
> break;
>
> case POWER_SUPPLY_PROP_VOLTAGE_NOW:
>

2021-11-08 04:20:42

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 3/4] power supply bq25890-charger: Handle temperature faults

Hi,

On 11/7/21 21:20, Yauhen Kharuzhy wrote:
> Add debug info about thermal failure to message in
> bq25890_get_chip_state().
>
> Take into account possible thermal failure when calculating a
> POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX value as it described in
> datasheet (in cold conditions a current limit is decreased to 20% or 50% of
> ICHG field value depended on JEITA_ISET field).
>
> Signed-off-by: Yauhen Kharuzhy <[email protected]>

Thank you for your patch,

Patch subject prefix should be: "power: supply: bq25890: "


> ---
> drivers/power/supply/bq25890_charger.c | 33 +++++++++++++++++++++++---
> 1 file changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index 1c43555d5bd8..fb2f1578503c 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -95,6 +95,7 @@ struct bq25890_state {
> u8 vsys_status;
> u8 boost_fault;
> u8 bat_fault;
> + u8 ntc_fault;
> };
>
> struct bq25890_device {
> @@ -384,6 +385,14 @@ enum bq25890_chrg_fault {
> CHRG_FAULT_TIMER_EXPIRED,
> };
>
> +enum bq25890_ntc_fault {
> + NTC_FAULT_NORMAL = 0,
> + NTC_FAULT_WARM = 2,
> + NTC_FAULT_COOL = 3,
> + NTC_FAULT_COLD = 5,
> + NTC_FAULT_HOT = 6,
> +};
> +
> static bool bq25890_is_adc_property(enum power_supply_property psp)
> {
> switch (psp) {
> @@ -474,7 +483,19 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
> break;
>
> case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
> + ret = bq25890_field_read(bq, F_JEITA_ISET);
> + if (ret < 0)
> + return ret;
> +

You only use the read value if bq->state.ntc_fault == NTC_FAULT_COOL
and i2c reads are somewhat expensive (or at least slow), can you
please move this to inside the
"if (bq->state.ntc_fault == NTC_FAULT_COOL) {" block ?

This is esp. relevant since this condition will normally be false,
so normally this will save us an i2c_read for every read of the
POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX property.

> val->intval = bq25890_find_val(bq->init_data.ichg, TBL_ICHG);
> +
> + /* When temperature is too low, charge current is decreased */
> + if (bq->state.ntc_fault == NTC_FAULT_COOL) {
> + if (ret)
> + val->intval /= 5;
> + else
> + val->intval /= 2;
> + }
> break;
>
> case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
> @@ -487,6 +508,10 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
> if (ret < 0)
> return ret;
>
> + ret = bq25890_field_read(bq, F_JEITA_VSET);
> + if (ret < 0)
> + return ret;
> +
> /* converted_val = 2.304V + ADC_val * 20mV (table 10.3.15) */
> val->intval = 2304000 + ret * 20000;
> break;
> @@ -550,7 +575,8 @@ static int bq25890_get_chip_state(struct bq25890_device *bq,
> {F_VSYS_STAT, &state->vsys_status},
> {F_BOOST_FAULT, &state->boost_fault},
> {F_BAT_FAULT, &state->bat_fault},
> - {F_CHG_FAULT, &state->chrg_fault}
> + {F_CHG_FAULT, &state->chrg_fault},
> + {F_CHG_FAULT, &state->ntc_fault}

Copy paste error? Surely the field in the second line must be F_NTG_FAULT ?

> };
>
> for (i = 0; i < ARRAY_SIZE(state_fields); i++) {
> @@ -561,9 +587,10 @@ static int bq25890_get_chip_state(struct bq25890_device *bq,
> *state_fields[i].data = ret;
> }
>
> - dev_dbg(bq->dev, "S:CHG/PG/VSYS=%d/%d/%d, F:CHG/BOOST/BAT=%d/%d/%d\n",
> + dev_dbg(bq->dev, "S:CHG/PG/VSYS=%d/%d/%d, F:CHG/BOOST/BAT/NTC=%d/%d/%d/%d\n",
> state->chrg_status, state->online, state->vsys_status,
> - state->chrg_fault, state->boost_fault, state->bat_fault);
> + state->chrg_fault, state->boost_fault, state->bat_fault,
> + state->ntc_fault);
>
> return 0;
> }
>

Regards,

Hans

2021-11-08 04:26:31

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 2/4] bq25890: Add max input current limit property

Hi Yauhen,

On 11/7/21 21:19, Yauhen Kharuzhy wrote:
> Add property 'ti,input-max-current' to define input current limit if
> needed. It will be applied if automatic charger type detection is
> disabled and using of ILIM pin is disabled or such pin defines greater
> limit than IINLIM field.

Sorry, but this makes no sense, as the datasheet says the charger
itself updates iinlim dynamically when it has done charger-type
detection (for the bq25890 version) and for the bq25892 version
which does not have the D+ + D- USB pins for BC1.2 detection,
the iinlim should be updated based on the charger detection
done elsewhere (by the Whiskey Cove PMIC in case of the Yoga Book).

My plan for this is to have drivers/extcon/extcon-intel-cht-wc.c
also register a power_supply device which models the detected
charger / negotiated external charger/power-brick settings and
which is the supplier of the bq25892 charger.

Then an external_power_changed handler can be added to the
bq25892_charger code using the
power_supply_set_input_current_limit_from_supplier()
helper to dynamically set iinlim based on the detected
"power-brick"/external-charger.

This is also how this is done for (X86) devices with an
full-featured USB Type-C port where this needs to be handled
by the kernel (rather then it being done in firmware) in
this case the current-max property of the Type-C power-supply
class device gets set either based on the Type-C pull-up
resistor in the charger (setting 0.5A / 1.5A / 3A), with
a fallback to BC1.2 for the 0.5A case, or based on the
USB-PD negotiated max-current.

Since we will need this mechanism to dynamically set
iinlim based on the PMIC-s charger-detection it seems
to me that setting it at boot is both unnecessary and a bad
idea, since we don't know the correct value to set at boot.

The extcon code will start a charger-detection cycle
as soon as it loads (if there is Vbus present) and then
trigger the external_power_changed handler .

TL;DR: I don't really see a need for this ?

Regards,

Hans




> ---
> drivers/power/supply/bq25890_charger.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> index 34467bfb9537..1c43555d5bd8 100644
> --- a/drivers/power/supply/bq25890_charger.c
> +++ b/drivers/power/supply/bq25890_charger.c
> @@ -85,6 +85,7 @@ struct bq25890_init_data {
> u8 treg; /* thermal regulation threshold */
> u8 rbatcomp; /* IBAT sense resistor value */
> u8 vclamp; /* IBAT compensation voltage limit */
> + u8 iinlim_max; /* maximum input current limit allowed */
> };
>
> struct bq25890_state {
> @@ -657,6 +658,7 @@ static int bq25890_hw_init(struct bq25890_device *bq)
> {F_TREG, bq->init_data.treg},
> {F_BATCMP, bq->init_data.rbatcomp},
> {F_VCLAMP, bq->init_data.vclamp},
> + {F_IINLIM, bq->init_data.iinlim_max},
> };
>
> ret = bq25890_chip_reset(bq);
> @@ -870,11 +872,13 @@ static int bq25890_fw_read_u32_props(struct bq25890_device *bq)
> {"ti,thermal-regulation-threshold", true, TBL_TREG, &init->treg},
> {"ti,ibatcomp-micro-ohms", true, TBL_RBATCOMP, &init->rbatcomp},
> {"ti,ibatcomp-clamp-microvolt", true, TBL_VBATCOMP, &init->vclamp},
> + {"ti,input-max-current", true, TBL_IINLIM, &init->iinlim_max},
> };
>
> /* initialize data for optional properties */
> init->treg = 3; /* 120 degrees Celsius */
> init->rbatcomp = init->vclamp = 0; /* IBAT compensation disabled */
> + init->iinlim_max = 0x3f;
>
> for (i = 0; i < ARRAY_SIZE(props); i++) {
> ret = device_property_read_u32(bq->dev, props[i].name,
>

2021-11-08 04:47:23

by Yauhen Kharuzhy

[permalink] [raw]
Subject: Re: [PATCH 2/4] bq25890: Add max input current limit property

On Sun, Nov 07, 2021 at 09:41:55PM +0100, Hans de Goede wrote:
> Hi Yauhen,
>
> On 11/7/21 21:19, Yauhen Kharuzhy wrote:
> > Add property 'ti,input-max-current' to define input current limit if
> > needed. It will be applied if automatic charger type detection is
> > disabled and using of ILIM pin is disabled or such pin defines greater
> > limit than IINLIM field.
>
> Sorry, but this makes no sense, as the datasheet says the charger
> itself updates iinlim dynamically when it has done charger-type
> detection (for the bq25890 version) and for the bq25892 version
> which does not have the D+ + D- USB pins for BC1.2 detection,
> the iinlim should be updated based on the charger detection
> done elsewhere (by the Whiskey Cove PMIC in case of the Yoga Book).

For Yoga Book, charger detection in the bq25892 is disabled and done in
the extcon driver.

> My plan for this is to have drivers/extcon/extcon-intel-cht-wc.c
> also register a power_supply device which models the detected
> charger / negotiated external charger/power-brick settings and
> which is the supplier of the bq25892 charger.
>
> Then an external_power_changed handler can be added to the
> bq25892_charger code using the
> power_supply_set_input_current_limit_from_supplier()
> helper to dynamically set iinlim based on the detected
> "power-brick"/external-charger.
>
> This is also how this is done for (X86) devices with an
> full-featured USB Type-C port where this needs to be handled
> by the kernel (rather then it being done in firmware) in
> this case the current-max property of the Type-C power-supply
> class device gets set either based on the Type-C pull-up
> resistor in the charger (setting 0.5A / 1.5A / 3A), with
> a fallback to BC1.2 for the 0.5A case, or based on the
> USB-PD negotiated max-current.

Agree, looks reasonable.

>
> Since we will need this mechanism to dynamically set
> iinlim based on the PMIC-s charger-detection it seems
> to me that setting it at boot is both unnecessary and a bad
> idea, since we don't know the correct value to set at boot.
>
> The extcon code will start a charger-detection cycle
> as soon as it loads (if there is Vbus present) and then
> trigger the external_power_changed handler .
>
> TL;DR: I don't really see a need for this ?

Hmm... I think you are rigth. The only case when such property can be
needed – if the device may be damaged by maximum current supported by
charging source. I use it to limit current by 2A when original Lenovo adapter
is connected because the linux kernel has default max current limit
5A for DCP (drivers/usb/phy/phy.c) but adapter supports only 2A.

>
>
> > ---
> > drivers/power/supply/bq25890_charger.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
> > index 34467bfb9537..1c43555d5bd8 100644
> > --- a/drivers/power/supply/bq25890_charger.c
> > +++ b/drivers/power/supply/bq25890_charger.c
> > @@ -85,6 +85,7 @@ struct bq25890_init_data {
> > u8 treg; /* thermal regulation threshold */
> > u8 rbatcomp; /* IBAT sense resistor value */
> > u8 vclamp; /* IBAT compensation voltage limit */
> > + u8 iinlim_max; /* maximum input current limit allowed */
> > };
> >
> > struct bq25890_state {
> > @@ -657,6 +658,7 @@ static int bq25890_hw_init(struct bq25890_device *bq)
> > {F_TREG, bq->init_data.treg},
> > {F_BATCMP, bq->init_data.rbatcomp},
> > {F_VCLAMP, bq->init_data.vclamp},
> > + {F_IINLIM, bq->init_data.iinlim_max},
> > };
> >
> > ret = bq25890_chip_reset(bq);
> > @@ -870,11 +872,13 @@ static int bq25890_fw_read_u32_props(struct bq25890_device *bq)
> > {"ti,thermal-regulation-threshold", true, TBL_TREG, &init->treg},
> > {"ti,ibatcomp-micro-ohms", true, TBL_RBATCOMP, &init->rbatcomp},
> > {"ti,ibatcomp-clamp-microvolt", true, TBL_VBATCOMP, &init->vclamp},
> > + {"ti,input-max-current", true, TBL_IINLIM, &init->iinlim_max},
> > };
> >
> > /* initialize data for optional properties */
> > init->treg = 3; /* 120 degrees Celsius */
> > init->rbatcomp = init->vclamp = 0; /* IBAT compensation disabled */
> > + init->iinlim_max = 0x3f;
> >
> > for (i = 0; i < ARRAY_SIZE(props); i++) {
> > ret = device_property_read_u32(bq->dev, props[i].name,
> >
>

--
Yauhen Kharuzhy