bq25890_charger: Rename IILIM field to IINLIM
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.
--- drivers/power/supply/bq25890_charger.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
drivers/power/supply/bq25890_charger.c +6 -6
@@ -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 :
Signature · 1 line
2.33.1
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.
--- drivers/power/supply/bq25890_charger.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/power/supply/bq25890_charger.c +4 -0
@@ -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,
Signature · 1 line
2.33.1
Re: [PATCH 2/4] bq25890: Add max input current limit property
Hi Yauhen,
Quoted 4 lines · Yauhen Kharuzhy
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
Quoted 39 lines
--- drivers/power/supply/bq25890_charger.c | 4 ++++ 1 file changed, 4 insertions(+)drivers/power/supply/bq25890_charger.c +4
-0@@ -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,
Re: [PATCH 2/4] bq25890: Add max input current limit property
Quoted 8 lines · Hans de Goede
Hi Yauhen,
Quoted 4 lines · Yauhen Kharuzhy
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.
Quoted 6 lines
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.
Quoted 11 lines
My plan for this is to have
drivers/extcon/extcon-intel-cht-wc.calso register apower_supplydevice which models the detected charger / negotiated external charger/power-brick settings and which is the supplier of the bq25892 charger.Then an
external_power_changedhandler can be added to thebq25892_chargercode using thepower_supply_set_input_current_limit_from_supplier()helper to dynamically set iinlim based on the detected "power-brick"/external-charger.
Quoted 8 lines
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.
Quoted 11 lines
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_changedhandler .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.
Quoted 42 lines
Quoted 41 lines
--- drivers/power/supply/bq25890_charger.c | 4 ++++ 1 file changed, 4 insertions(+)drivers/power/supply/bq25890_charger.c +4
-0@@ -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,
Signature · 1 line
Yauhen Kharuzhy
Re: [PATCH 2/4] bq25890: Add max input current limit property
Hi,
Quoted 51 lines · Yauhen Kharuzhy
Quoted 8 lines · Hans de Goede
Hi Yauhen,
Quoted 4 lines · Yauhen Kharuzhy
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.
Quoted 6 lines
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.
Quoted 11 lines
My plan for this is to have
drivers/extcon/extcon-intel-cht-wc.calso register apower_supplydevice which models the detected charger / negotiated external charger/power-brick settings and which is the supplier of the bq25892 charger.Then an
external_power_changedhandler can be added to thebq25892_chargercode using thepower_supply_set_input_current_limit_from_supplier()helper to dynamically set iinlim based on the detected "power-brick"/external-charger.Quoted 8 lines
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.
Quoted 10 lines
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_changedhandler .
Quoted 7 lines
Quoted 1 line
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.
Yeah 5A is bad, I find it a bit weird that drivers/usb/phy/phy.c uses that, but my plan for X86 devices with this charger is to not use the drivers/usb/phy/phy.c code, that is not really a good fit for X86 devices.
For the axp288 code (found on many other X86 Cherry Trail devices) we use 2A when detecting a DCP.
And the same for the full-featured Type-C case with a C to A cable identified with a Pull Up resistor indicating max 0.5, combined with BC1.2 DCP detection, then we too set the input-current-limit to 2A.
Also I believe that the ILIM pin limits things to 2A on the yogabook, I've seen the firmware set IINLIM to 3.25A and even with a 3A capable charger it only draws 2A.
Regards,
Hans
Quoted 40 lines
Quoted 39 lines
Quoted 38 lines
drivers/power/supply/bq25890_charger.c | 4 ++++ 1 file changed, 4 insertions(+)drivers/power/supply/bq25890_charger.c +4
-0@@ -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,
PATCH 3/4 power supply bq25890-charger: Handle temperature faults
In reply to Yauhen KharuzhyAdd 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).
--- drivers/power/supply/bq25890_charger.c | 33 +++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-)
drivers/power/supply/bq25890_charger.c +30 -3
@@ -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 ; }
Signature · 1 line
2.33.1
Re: [PATCH 3/4] power supply bq25890-charger: Handle temperature faults
Hi,
Quoted 9 lines · Yauhen Kharuzhy
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_MAXvalue as it described in datasheet (in cold conditions a current limit is decreased to 20% or 50% of ICHG field value depended onJEITA_ISETfield).Signed-off-by Yauhen Kharuzhy <[email protected]>
Thank you for your patch,
Patch subject prefix should be: "power: supply: bq25890: "
Quoted 32 lines
--- drivers/power/supply/bq25890_charger.c | 33 +++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-)drivers/power/supply/bq25890_charger.c +9
-0@@ -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) {
drivers/power/supply/bq25890_charger.c static int bq25890_power_supply_get_property(struct power_supply *psy, · line 489
Quoted 7 lines
drivers/power/supply/bq25890_charger.c +4
-0@@ -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.
Quoted 24 lines
drivers/power/supply/bq25890_charger.c +12
-0val->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 ;
drivers/power/supply/bq25890_charger.c static int bq25890_get_chip_state(struct bq25890_device *bq, · line 579
Quoted 6 lines
drivers/power/supply/bq25890_charger.c +2
-1@@ -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 ?
Quoted 7 lines
drivers/power/supply/bq25890_charger.c +0
-0};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; }
drivers/power/supply/bq25890_charger.c static int bq25890_get_chip_state(struct bq25890_device *bq, · line 596
Quoted 10 lines
drivers/power/supply/bq25890_charger.c +3
-2-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
PATCH 4/4 bq25890_charger: Enable continuous conversion for ADC at charging
In reply to Yauhen KharuzhyInstead of one shot run of ADC at beginning of charging, run continuous conversion to ensure that all charging-related values are monitored properly (input voltage, input current, themperature etc.).
--- drivers/power/supply/bq25890_charger.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/power/supply/bq25890_charger.c +2 -2
@@ -609,12 +609,12 @@ static irqreturn_t __bq25890_handle_irq(struct bq25890_device *bq)if (!new_state.online && bq->state.online) {/* power removed */ /* disable ADC */ - ret =bq25890_field_write (bq,F_CONV_START ,0 ); + ret =bq25890_field_write (bq,F_CONV_RATE ,0 );if (ret <0 ) goto error; }else if (new_state.online && !bq->state.online) {/* power inserted */ /* enable ADC, to have control of charge current/voltage */ - ret =bq25890_field_write (bq,F_CONV_START ,1 ); + ret =bq25890_field_write (bq,F_CONV_RATE ,1 );if (ret <0 ) goto error; }
Signature · 1 line
2.33.1
Re: [PATCH 4/4] bq25890_charger: Enable continuous conversion for ADC at charging
Hi,
Quoted 5 lines · Yauhen Kharuzhy
Instead of one shot run of ADC at beginning of charging, run continuous conversion to ensure that all charging-related values are monitored properly (input voltage, input current, themperature etc.).
Signed-off-by Yauhen Kharuzhy <[email protected]>
Thank you for finding this! This explains why all the ADC returned values like current_now where 0 when charging, I thought this was just something which was only supported while not charging, heh.
As before, the patch subject prefix should be: "power: supply: bq25890: "
Otherwise the patch looks good to me:
Sebastian, this really is a pure bug-fix patch, any chance you can pick this up for a future 5.16-rc# pull-req ?
Regards,
Hans
Quoted 24 lines
--- drivers/power/supply/bq25890_charger.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/power/supply/bq25890_charger.c +2
-2@@ -609,12 +609,12 @@ static irqreturn_t __bq25890_handle_irq(struct bq25890_device *bq)if (!new_state.online && bq->state.online) {/* power removed */ /* disable ADC */ - ret =bq25890_field_write (bq,F_CONV_START ,0 ); + ret =bq25890_field_write (bq,F_CONV_RATE ,0 );if (ret <0 ) goto error; }else if (new_state.online && !bq->state.online) {/* power inserted */ /* enable ADC, to have control of charge current/voltage */ - ret =bq25890_field_write (bq,F_CONV_START ,1 ); + ret =bq25890_field_write (bq,F_CONV_RATE ,1 );if (ret <0 ) goto error; }
Re: [PATCH 4/4] bq25890_charger: Enable continuous conversion for ADC at charging
Hi,
Quoted 17 lines · Hans de Goede
Hi,
Quoted 5 lines · Yauhen Kharuzhy
Instead of one shot run of ADC at beginning of charging, run continuous conversion to ensure that all charging-related values are monitored properly (input voltage, input current, themperature etc.).
Signed-off-by Yauhen Kharuzhy <[email protected]>Thank you for finding this! This explains why all the ADC returned values like
current_nowwhere 0 when charging, I thought this was just something which was only supported while not charging, heh.As before, the patch subject prefix should be: "power: supply: bq25890: "
Otherwise the patch looks good to me:
Quoted 8 lines
Reviewed-by Hans de Goede <[email protected]>Sebastian, this really is a pure bug-fix patch, any chance you can pick this up for a future 5.16-rc# pull-req ?
Regards,
Hans
Thanks, queued to power-supply's fixes branch.
-- Sebastian
Re: [PATCH 4/4] bq25890_charger: Enable continuous conversion for ADC at charging
Hi,
Quoted 22 lines · Sebastian Reichel
Hi,
Quoted 19 lines · Hans de Goede
Hi,
Quoted 5 lines · Yauhen Kharuzhy
Instead of one shot run of ADC at beginning of charging, run continuous conversion to ensure that all charging-related values are monitored properly (input voltage, input current, themperature etc.).
Signed-off-by Yauhen Kharuzhy <[email protected]>Thank you for finding this! This explains why all the ADC returned values like
current_nowwhere 0 when charging, I thought this was just something which was only supported while not charging, heh.As before, the patch subject prefix should be: "power: supply: bq25890: "
Otherwise the patch looks good to me:
Reviewed-by Hans de Goede <[email protected]>
Quoted 8 lines
Quoted 6 lines
Sebastian, this really is a pure bug-fix patch, any chance you can pick this up for a future 5.16-rc# pull-req ?
Regards,
Hans
Thanks, queued to power-supply's fixes branch.
Thank you.
Note I also added this to my:
[PATCH v2 00/20] power-suppy/i2c/extcon: Fix charger setup on Xiaomi Mi Pad 2 and Lenovo Yogabook
Series, together with 2 of the 3 other patches from this series (with some minor fixes applied to one of them).
So you can skip:
[PATCH v2 02/20] power: supply: bq25890: Fix ADC continuous conversion setting when charging
from that series (I reworded the commit msg subject a bit, but it is the exact same patch).
If I end up doing a v3, I'll drop this patch.
Regards,
Hans
Hi,
Quoted 5 lines · Yauhen Kharuzhy
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:
Regards,
Hans
Quoted 59 lines
--- drivers/power/supply/bq25890_charger.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)drivers/power/supply/bq25890_charger.c +6
-6@@ -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 :