Hi Sebastian,
Here is a resend of v4 of my patch series to fix the charger setup on
Xiaomi Mi Pad 2 and Lenovo Yogabook devices, as well as fix host/device
mode switching.
I'm resending this because this has not made it into 5.17, this resend has
been rebased on top of 5.17-rc1.
Since patches 14-20 rely on the power-supply changes from patch 1-13 and
since they all touch files which generally do not see much changes, the
intention is for this entire series to be merged through your (Sebastian's)
linux-power-supply tree. Patches 14-20 all have ackes from the relevant
subsystem maintainers for merging them through the linux-power-supply tree.
###
For more details on this series, here is some info from the v2
cover-letter:
So far almost all the kernel code surrounding the Cherry Trail Whiskey Cove
PMIC has been developed on the GPD win / pocket devices and it has various
assumption based on that. In the mean time I've learned (and gotten access
to) about 2 more designs and none of the 3 now known designs use a single
standard setup for the charger, fuel-gauge and other chips surrounding the
PMIC / charging+data USB port:
1. The GPD Win and GPD Pocket mini-laptops, these are really 2 models
but the Pocket re-uses the GPD Win's design in a different housing:
The WC PMIC is connected to a TI BQ24292i charger, paired with
a Maxim MAX17047 fuelgauge + a FUSB302 USB Type-C Controller +
a PI3USB30532 USB switch, for a fully functional Type-C port.
2. The Xiaomi Mi Pad 2:
The WC PMIC is connected to a TI BQ25890 charger, paired with
a TI BQ27520 fuelgauge, using the TI BQ25890 for BC1.2 charger type
detection, for a USB-2 only Type-C port without PD.
3. The Lenovo Yoga Book YB1-X90 / Lenovo Yoga Book YB1-X91 series:
The WC PMIC is connected to a TI BQ25892 charger, paired with
a TI BQ27542 fuelgauge, using the WC PMIC for BC1.2 charger type
detection and using the BQ25892's Mediatek Pump Express+ (1.0)
Unlike what is normal on X86 this diversity in designs is not handled /
abstracted away by the ACPI tables.
This series takes care of making sure that charging and device/host mode
switching also works on the Xiaomi Mi Pad 2 and the Lenovo Yogabook.
Patches 1-13: Prepare the bq25890 power_supply driver to fully support
the Mi Pad 2 and the Yogabook. Note this includes a new
version of 2 bq25890 patches send earlier by Yauhen Kharuzhy
Patch 14: Adds the intel_cht_wc_get_model() helper
Patch 15: Uses this intel_cht_wc_get_model() value to instantiate an
i2c-client with the right type and properties for the charger
IC used on the board (instead of harcoding the GPD values)
Patches 16-20: Modify the extcon code to provide charger-detection results
to the charger driver and to take care of the Vbus boost
regulator control (for host-mode) and device/host mode
switching
I've tried to keep the power_supply patches as generic as possible while
focussing some of the special handling these boards need in the
WC PMIC MFD and cell drivers, which will only get loaded on these boards.
Regards,
Hans
p.s.
Sebastian, I also have another power-supply series pending
for merging into -next:
https://lore.kernel.org/linux-pm/[email protected]/
Hans de Goede (17):
power: supply: core: Refactor
power_supply_set_input_current_limit_from_supplier()
power: supply: bq25890: Add a bq25890_rw_init_data() helper
power: supply: bq25890: Add support to skip reset at probe() /
remove()
power: supply: bq25890: Add support to read back the settings from the
chip
power: supply: bq25890: Enable charging on boards where we skip reset
power: supply: bq25890: Drop dev->platform_data == NULL check
power: supply: bq25890: Add bq25890_set_otg_cfg() helper
power: supply: bq25890: Add support for registering the Vbus boost
converter as a regulator
power: supply: bq25890: On the bq25892 set the IINLIM based on
external charger detection
power: supply: bq25890: Use the devm_regmap_field_bulk_alloc() helper
mfd: intel_soc_pmic_chtwc: Add cht_wc_model data to struct
intel_soc_pmic
i2c: cht-wc: Make charger i2c-client instantiation board/device-model
specific
extcon: intel-cht-wc: Use new cht_wc_model intel_soc_pmic field
extcon: intel-cht-wc: Support devs with Micro-B / USB-2 only Type-C
connectors
extcon: intel-cht-wc: Refactor cht_wc_extcon_get_charger()
extcon: intel-cht-wc: Add support for registering a power_supply
class-device
extcon: intel-cht-wc: Report RID_A for ACA adapters
Yauhen Kharuzhy (3):
power: supply: bq25890: Rename IILIM field to IINLIM
power: supply: bq25890: Reduce reported CONSTANT_CHARGE_CURRENT_MAX
for low temperatures
power: supply: bq25890: Support higher charging voltages through Pump
Express+ protocol
drivers/extcon/Kconfig | 2 +
drivers/extcon/extcon-intel-cht-wc.c | 240 ++++++++++++--
drivers/i2c/busses/i2c-cht-wc.c | 120 +++++--
drivers/mfd/intel_soc_pmic_chtwc.c | 40 +++
drivers/power/supply/bq24190_charger.c | 10 +-
drivers/power/supply/bq25890_charger.c | 396 ++++++++++++++++++-----
drivers/power/supply/power_supply_core.c | 57 ++--
include/linux/mfd/intel_soc_pmic.h | 8 +
include/linux/power/bq25890_charger.h | 15 +
include/linux/power_supply.h | 5 +-
10 files changed, 742 insertions(+), 151 deletions(-)
create mode 100644 include/linux/power/bq25890_charger.h
--
2.33.1
From: Yauhen Kharuzhy <[email protected]>
Take into account possible current reduction due to low-temperature when
reading POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX. As described in
the datasheet in cool (0-20° Celcius) conditions the current limit is
decreased to 20% or 50% of ICHG field value depended on JEITA_ISET field.
Also add NTC_FAULT field value to the debug message in
bq25890_get_chip_state().
Changed by Hans de Goede:
- Fix reading F_CHG_FAULT instead of F_NTC_FIELD for state->ntc_fault
- Only read JEITA_ISET field if necessary
- Tweak commit message a bit
Signed-off-by: Yauhen Kharuzhy <[email protected]>
Co-developed-by: Hans de Goede <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
Changes in v3:
- Drop chunk adding a F_JEITA_VSET read to bq25890_power_supply_get_property()
which accidentally got added to this patch
---
drivers/power/supply/bq25890_charger.c | 29 +++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index fb987579d05a..7a3269c06b38 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -94,6 +94,7 @@ struct bq25890_state {
u8 vsys_status;
u8 boost_fault;
u8 bat_fault;
+ u8 ntc_fault;
};
struct bq25890_device {
@@ -407,6 +408,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) {
@@ -499,6 +508,18 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
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) {
+ ret = bq25890_field_read(bq, F_JEITA_ISET);
+ if (ret < 0)
+ return ret;
+
+ if (ret)
+ val->intval /= 5;
+ else
+ val->intval /= 2;
+ }
break;
case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE:
@@ -583,7 +604,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_NTC_FAULT, &state->ntc_fault}
};
for (i = 0; i < ARRAY_SIZE(state_fields); i++) {
@@ -594,9 +616,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
From: Yauhen Kharuzhy <[email protected]>
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]>
Reviewed-by: Hans de Goede <[email protected]>
Signed-off-by: Hans de Goede <[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 e62da9dc4f35..fb987579d05a 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,
@@ -322,7 +322,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 */
@@ -528,11 +528,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
Some (USB) charger ICs have variants with USB D+ and D- pins to do their
own builtin charger-type detection, like e.g. the bq24190 and bq25890 and
also variants which lack this functionality, e.g. the bq24192 and bq25892.
In case the charger-type; and thus the input-current-limit detection is
done outside the charger IC then we need some way to communicate this to
the charger IC. In the past extcon was used for this, but if the external
detection does e.g. full USB PD negotiation then the extcon cable-types do
not convey enough information.
For these setups it was decided to model the external charging "brick"
and the parameters negotiated with it as a power_supply class-device
itself; and power_supply_set_input_current_limit_from_supplier() was
introduced to allow drivers to get the input-current-limit this way.
But in some cases psy drivers may want to know other properties, e.g. the
bq25892 can do "quick-charge" negotiation by pulsing its current draw,
but this should only be done if the usb_type psy-property of its supplier
is set to DCP (and device-properties indicate the board allows higher
voltages).
Instead of adding extra helper functions for each property which
a psy-driver wants to query from its supplier, refactor
power_supply_set_input_current_limit_from_supplier() into a
more generic power_supply_get_property_from_supplier() function.
Signed-off-by: Hans de Goede <[email protected]>
---
drivers/power/supply/bq24190_charger.c | 10 ++++-
drivers/power/supply/power_supply_core.c | 57 +++++++++++++-----------
include/linux/power_supply.h | 5 ++-
3 files changed, 42 insertions(+), 30 deletions(-)
diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index 06c34b09349c..c8a06ee6bd51 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -1206,8 +1206,16 @@ 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);
+ union power_supply_propval val;
+ int ret;
- power_supply_set_input_current_limit_from_supplier(bdi->charger);
+ ret = power_supply_get_property_from_supplier(bdi->charger,
+ POWER_SUPPLY_PROP_CURRENT_MAX,
+ &val);
+ if (ret == 0)
+ bq24190_charger_set_property(bdi->charger,
+ POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+ &val);
}
/* Sync the input-current-limit with our parent supply (if we have one) */
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index ec838c9bcc0a..df4471e50d33 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -376,46 +376,49 @@ int power_supply_is_system_supplied(void)
}
EXPORT_SYMBOL_GPL(power_supply_is_system_supplied);
-static int __power_supply_get_supplier_max_current(struct device *dev,
- void *data)
+struct psy_get_supplier_prop_data {
+ struct power_supply *psy;
+ enum power_supply_property psp;
+ union power_supply_propval *val;
+};
+
+static int __power_supply_get_supplier_property(struct device *dev, void *_data)
{
- union power_supply_propval ret = {0,};
struct power_supply *epsy = dev_get_drvdata(dev);
- struct power_supply *psy = data;
+ struct psy_get_supplier_prop_data *data = _data;
- if (__power_supply_is_supplied_by(epsy, psy))
- if (!epsy->desc->get_property(epsy,
- POWER_SUPPLY_PROP_CURRENT_MAX,
- &ret))
- return ret.intval;
+ if (__power_supply_is_supplied_by(epsy, data->psy))
+ if (!epsy->desc->get_property(epsy, data->psp, data->val))
+ return 1; /* Success */
- return 0;
+ return 0; /* Continue iterating */
}
-int power_supply_set_input_current_limit_from_supplier(struct power_supply *psy)
+int power_supply_get_property_from_supplier(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
{
- union power_supply_propval val = {0,};
- int curr;
-
- if (!psy->desc->set_property)
- return -EINVAL;
+ struct psy_get_supplier_prop_data data = {
+ .psy = psy,
+ .psp = psp,
+ .val = val,
+ };
+ int ret;
/*
* This function is not intended for use with a supply with multiple
- * suppliers, we simply pick the first supply to report a non 0
- * max-current.
+ * suppliers, we simply pick the first supply to report the psp.
*/
- curr = class_for_each_device(power_supply_class, NULL, psy,
- __power_supply_get_supplier_max_current);
- if (curr <= 0)
- return (curr == 0) ? -ENODEV : curr;
-
- val.intval = curr;
+ ret = class_for_each_device(power_supply_class, NULL, &data,
+ __power_supply_get_supplier_property);
+ if (ret < 0)
+ return ret;
+ if (ret == 0)
+ return -ENODEV;
- return psy->desc->set_property(psy,
- POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT, &val);
+ return 0;
}
-EXPORT_SYMBOL_GPL(power_supply_set_input_current_limit_from_supplier);
+EXPORT_SYMBOL_GPL(power_supply_get_property_from_supplier);
int power_supply_set_battery_charged(struct power_supply *psy)
{
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index e218041cc000..006111917d1a 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -597,8 +597,9 @@ power_supply_temp2resist_simple(struct power_supply_resistance_temp_table *table
int table_len, int temp);
extern void power_supply_changed(struct power_supply *psy);
extern int power_supply_am_i_supplied(struct power_supply *psy);
-extern int power_supply_set_input_current_limit_from_supplier(
- struct power_supply *psy);
+int power_supply_get_property_from_supplier(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val);
extern int power_supply_set_battery_charged(struct power_supply *psy);
#ifdef CONFIG_POWER_SUPPLY
--
2.33.1
On most x86/ACPI devices the firmware already fully initializes
the bq25890 charger at boot, in this case it is best to not reset
it at probe() time.
At support for a new "linux,skip-reset" boolean property to support this.
So far this new property is only used on x86/ACPI (non devicetree) devs,
IOW it is not used in actual devicetree files. The devicetree-bindings
maintainers have requested properties like these to not be added to the
devicetree-bindings, so the new property is deliberately not added
to the existing devicetree-bindings.
Signed-off-by: Hans de Goede <[email protected]>
---
Changes in v2:
- New patch in v2 of this series, together with the "linux,read-back-settings"
patch, this patch replaces the "ti,skip-init" patch from v1
---
drivers/power/supply/bq25890_charger.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index eaf0400b632f..cd80d748df92 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -110,6 +110,7 @@ struct bq25890_device {
struct regmap *rmap;
struct regmap_field *rmap_fields[F_MAX_FIELDS];
+ bool skip_reset;
enum bq25890_chip_version chip_version;
struct bq25890_init_data init_data;
struct bq25890_state state;
@@ -739,10 +740,12 @@ static int bq25890_hw_init(struct bq25890_device *bq)
{
int ret;
- ret = bq25890_chip_reset(bq);
- if (ret < 0) {
- dev_dbg(bq->dev, "Reset failed %d\n", ret);
- return ret;
+ if (!bq->skip_reset) {
+ ret = bq25890_chip_reset(bq);
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Reset failed %d\n", ret);
+ return ret;
+ }
}
/* disable watchdog */
@@ -977,6 +980,8 @@ static int bq25890_fw_probe(struct bq25890_device *bq)
int ret;
struct bq25890_init_data *init = &bq->init_data;
+ bq->skip_reset = device_property_read_bool(bq->dev, "linux,skip-reset");
+
ret = bq25890_fw_read_u32_props(bq);
if (ret < 0)
return ret;
@@ -1089,8 +1094,10 @@ static int bq25890_remove(struct i2c_client *client)
if (!IS_ERR_OR_NULL(bq->usb_phy))
usb_unregister_notifier(bq->usb_phy, &bq->usb_nb);
- /* reset all registers to default values */
- bq25890_chip_reset(bq);
+ if (!bq->skip_reset) {
+ /* reset all registers to default values */
+ bq25890_chip_reset(bq);
+ }
return 0;
}
--
2.33.1
On most x86/ACPI devices there is no devicetree to supply the necessary
init-data. Instead the firmware already fully initializes the bq25890
charger at boot. To support this, add support for reading back the
settings from the chip through a new "linux,read-back-settings" boolean.
So far this new property is only used on x86/ACPI (non devicetree) devs,
IOW it is not used in actual devicetree files. The devicetree-bindings
maintainers have requested properties like these to not be added to the
devicetree-bindings, so the new property is deliberately not added
to the existing devicetree-bindings.
Signed-off-by: Hans de Goede <[email protected]>
---
Changes in v2:
- New patch in v2 of this series, together with the "linux,skip-reset" patch
this patch replaces the "ti,skip-init" patch from v1
---
drivers/power/supply/bq25890_charger.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index cd80d748df92..f758e28046e5 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -111,6 +111,7 @@ struct bq25890_device {
struct regmap_field *rmap_fields[F_MAX_FIELDS];
bool skip_reset;
+ bool read_back_init_data;
enum bq25890_chip_version chip_version;
struct bq25890_init_data init_data;
struct bq25890_state state;
@@ -696,7 +697,7 @@ static int bq25890_chip_reset(struct bq25890_device *bq)
static int bq25890_rw_init_data(struct bq25890_device *bq)
{
- bool write = true;
+ bool write = !bq->read_back_init_data;
int ret;
int i;
@@ -981,6 +982,10 @@ static int bq25890_fw_probe(struct bq25890_device *bq)
struct bq25890_init_data *init = &bq->init_data;
bq->skip_reset = device_property_read_bool(bq->dev, "linux,skip-reset");
+ bq->read_back_init_data = device_property_read_bool(bq->dev,
+ "linux,read-back-settings");
+ if (bq->read_back_init_data)
+ return 0;
ret = bq25890_fw_read_u32_props(bq);
if (ret < 0)
--
2.33.1
On boards where the "linux,skip-reset" boolean property is set we don't
reset the charger; and on some boards where the fw takes care of
initalizition F_CHG_CFG is set to 0 before handing control over to the OS.
Explicitly set F_CHG_CFG to 1 on boards where we don't reset the charger,
so that charging is always enabled on these boards, like it is always
enabled on boards where we do reset the charger.
Signed-off-by: Hans de Goede <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index f758e28046e5..d185299db9c3 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -747,6 +747,17 @@ static int bq25890_hw_init(struct bq25890_device *bq)
dev_dbg(bq->dev, "Reset failed %d\n", ret);
return ret;
}
+ } else {
+ /*
+ * Ensure charging is enabled, on some boards where the fw
+ * takes care of initalizition F_CHG_CFG is set to 0 before
+ * handing control over to the OS.
+ */
+ ret = bq25890_field_write(bq, F_CHG_CFG, 1);
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Enabling charging failed %d\n", ret);
+ return ret;
+ }
}
/* disable watchdog */
--
2.33.1
Add a bq25890_set_otg_cfg() helper function, this is a preparation
patch for adding regulator support.
Signed-off-by: Hans de Goede <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 28 ++++++++++++++------------
1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 548d1a793e31..162bffb02410 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -832,6 +832,17 @@ static int bq25890_power_supply_init(struct bq25890_device *bq)
return PTR_ERR_OR_ZERO(bq->charger);
}
+static int bq25890_set_otg_cfg(struct bq25890_device *bq, u8 val)
+{
+ int ret;
+
+ ret = bq25890_field_write(bq, F_OTG_CFG, val);
+ if (ret < 0)
+ dev_err(bq->dev, "Error switching to boost/charger mode: %d\n", ret);
+
+ return ret;
+}
+
static void bq25890_usb_work(struct work_struct *data)
{
int ret;
@@ -841,25 +852,16 @@ static void bq25890_usb_work(struct work_struct *data)
switch (bq->usb_event) {
case USB_EVENT_ID:
/* Enable boost mode */
- ret = bq25890_field_write(bq, F_OTG_CFG, 1);
- if (ret < 0)
- goto error;
+ bq25890_set_otg_cfg(bq, 1);
break;
case USB_EVENT_NONE:
/* Disable boost mode */
- ret = bq25890_field_write(bq, F_OTG_CFG, 0);
- if (ret < 0)
- goto error;
-
- power_supply_changed(bq->charger);
+ ret = bq25890_set_otg_cfg(bq, 0);
+ if (ret == 0)
+ power_supply_changed(bq->charger);
break;
}
-
- return;
-
-error:
- dev_err(bq->dev, "Error switching to boost/charger mode.\n");
}
static int bq25890_usb_notifier(struct notifier_block *nb, unsigned long val,
--
2.33.1
The bq25890_charger code supports enabling/disabling the boost converter
based on usb-phy notifications. But the usb-phy framework is not used on
all boards/platforms. At support for registering the Vbus boost converter
as a standard regulator when there is no usb-phy on the board.
Also add support for providing regulator_init_data through platform_data
for use on boards where device-tree is not used and the platform code must
thus provide the regulator_init_data.
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
Changes in v2:
- When the usb-phy framework is not used, turn off the Vboost regulator
on shutdown
- Some minor code-tweaks based on Andy's review
---
drivers/power/supply/bq25890_charger.c | 80 ++++++++++++++++++++++++++
include/linux/power/bq25890_charger.h | 15 +++++
2 files changed, 95 insertions(+)
create mode 100644 include/linux/power/bq25890_charger.h
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 162bffb02410..637cdd3b6b11 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -8,7 +8,9 @@
#include <linux/module.h>
#include <linux/i2c.h>
#include <linux/power_supply.h>
+#include <linux/power/bq25890_charger.h>
#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
#include <linux/types.h>
#include <linux/gpio/consumer.h>
#include <linux/interrupt.h>
@@ -876,6 +878,45 @@ static int bq25890_usb_notifier(struct notifier_block *nb, unsigned long val,
return NOTIFY_OK;
}
+#ifdef CONFIG_REGULATOR
+static int bq25890_vbus_enable(struct regulator_dev *rdev)
+{
+ struct bq25890_device *bq = rdev_get_drvdata(rdev);
+
+ return bq25890_set_otg_cfg(bq, 1);
+}
+
+static int bq25890_vbus_disable(struct regulator_dev *rdev)
+{
+ struct bq25890_device *bq = rdev_get_drvdata(rdev);
+
+ return bq25890_set_otg_cfg(bq, 0);
+}
+
+static int bq25890_vbus_is_enabled(struct regulator_dev *rdev)
+{
+ struct bq25890_device *bq = rdev_get_drvdata(rdev);
+
+ return bq25890_field_read(bq, F_OTG_CFG);
+}
+
+static const struct regulator_ops bq25890_vbus_ops = {
+ .enable = bq25890_vbus_enable,
+ .disable = bq25890_vbus_disable,
+ .is_enabled = bq25890_vbus_is_enabled,
+};
+
+static const struct regulator_desc bq25890_vbus_desc = {
+ .name = "usb_otg_vbus",
+ .of_match = "usb-otg-vbus",
+ .type = REGULATOR_VOLTAGE,
+ .owner = THIS_MODULE,
+ .ops = &bq25890_vbus_ops,
+ .fixed_uV = 5000000,
+ .n_voltages = 1,
+};
+#endif
+
static int bq25890_get_chip_version(struct bq25890_device *bq)
{
int id, rev;
@@ -1075,6 +1116,22 @@ static int bq25890_probe(struct i2c_client *client,
bq->usb_nb.notifier_call = bq25890_usb_notifier;
usb_register_notifier(bq->usb_phy, &bq->usb_nb);
}
+#ifdef CONFIG_REGULATOR
+ else {
+ struct bq25890_platform_data *pdata = dev_get_platdata(dev);
+ struct regulator_config cfg = { };
+ struct regulator_dev *reg;
+
+ cfg.dev = dev;
+ cfg.driver_data = bq;
+ if (pdata)
+ cfg.init_data = pdata->regulator_init_data;
+
+ reg = devm_regulator_register(dev, &bq25890_vbus_desc, &cfg);
+ if (IS_ERR(reg))
+ return dev_err_probe(dev, PTR_ERR(reg), "registering regulator");
+ }
+#endif
ret = bq25890_power_supply_init(bq);
if (ret < 0) {
@@ -1113,6 +1170,28 @@ static int bq25890_remove(struct i2c_client *client)
return 0;
}
+static void bq25890_shutdown(struct i2c_client *client)
+{
+ struct bq25890_device *bq = i2c_get_clientdata(client);
+
+ /*
+ * TODO this if + return should probably be removed, but that would
+ * introduce a function change for boards using the usb-phy framework.
+ * This needs to be tested on such a board before making this change.
+ */
+ if (!IS_ERR_OR_NULL(bq->usb_phy))
+ return;
+
+ /*
+ * Turn off the 5v Boost regulator which outputs Vbus to the device's
+ * Micro-USB or Type-C USB port. Leaving this on drains power and
+ * this avoids the PMIC on some device-models seeing this as Vbus
+ * getting inserted after shutdown, causing the device to immediately
+ * power-up again.
+ */
+ bq25890_set_otg_cfg(bq, 0);
+}
+
#ifdef CONFIG_PM_SLEEP
static int bq25890_suspend(struct device *dev)
{
@@ -1192,6 +1271,7 @@ static struct i2c_driver bq25890_driver = {
},
.probe = bq25890_probe,
.remove = bq25890_remove,
+ .shutdown = bq25890_shutdown,
.id_table = bq25890_i2c_ids,
};
module_i2c_driver(bq25890_driver);
diff --git a/include/linux/power/bq25890_charger.h b/include/linux/power/bq25890_charger.h
new file mode 100644
index 000000000000..c706ddb77a08
--- /dev/null
+++ b/include/linux/power/bq25890_charger.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Platform data for the TI bq25890 battery charger driver.
+ */
+
+#ifndef _BQ25890_CHARGER_H_
+#define _BQ25890_CHARGER_H_
+
+struct regulator_init_data;
+
+struct bq25890_platform_data {
+ const struct regulator_init_data *regulator_init_data;
+};
+
+#endif
--
2.33.1
The bq25892 does not have builtin charger-type detection like the bq25980,
there might be some external charger detection capability, which will be
modelled as a power_supply class-device supplying the bq25892.
Use the usb_type property value from the supplier psy-device to set the
input-current-limit (when available).
Signed-off-by: Hans de Goede <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 33 ++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 637cdd3b6b11..3de72f0fbf3e 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -594,6 +594,38 @@ static int bq25890_power_supply_get_property(struct power_supply *psy,
return 0;
}
+/* On the BQ25892 try to get charger-type info from our supplier */
+static void bq25890_charger_external_power_changed(struct power_supply *psy)
+{
+ struct bq25890_device *bq = power_supply_get_drvdata(psy);
+ union power_supply_propval val;
+ int input_current_limit, ret;
+
+ if (bq->chip_version != BQ25892)
+ return;
+
+ ret = power_supply_get_property_from_supplier(bq->charger,
+ POWER_SUPPLY_PROP_USB_TYPE,
+ &val);
+ if (ret)
+ return;
+
+ switch (val.intval) {
+ case POWER_SUPPLY_USB_TYPE_DCP:
+ input_current_limit = bq25890_find_idx(2000000, TBL_IINLIM);
+ break;
+ case POWER_SUPPLY_USB_TYPE_CDP:
+ case POWER_SUPPLY_USB_TYPE_ACA:
+ input_current_limit = bq25890_find_idx(1500000, TBL_IINLIM);
+ break;
+ case POWER_SUPPLY_USB_TYPE_SDP:
+ default:
+ input_current_limit = bq25890_find_idx(500000, TBL_IINLIM);
+ }
+
+ bq25890_field_write(bq, F_IINLIM, input_current_limit);
+}
+
static int bq25890_get_chip_state(struct bq25890_device *bq,
struct bq25890_state *state)
{
@@ -818,6 +850,7 @@ static const struct power_supply_desc bq25890_power_supply_desc = {
.properties = bq25890_power_supply_props,
.num_properties = ARRAY_SIZE(bq25890_power_supply_props),
.get_property = bq25890_power_supply_get_property,
+ .external_power_changed = bq25890_charger_external_power_changed,
};
static int bq25890_power_supply_init(struct bq25890_device *bq)
--
2.33.1
From: Yauhen Kharuzhy <[email protected]>
Add a "linux,pump-express-vbus-max" property which indicates if the Pump
Express+ protocol should be used to increase the charging protocol.
If this new property is set and a DCP charger is detected then request
a higher charging voltage through the Pump Express+ protocol.
So far this new property is only used on x86/ACPI (non devicetree) devs,
IOW it is not used in actual devicetree files. The devicetree-bindings
maintainers have requested properties like these to not be added to the
devicetree-bindings, so the new property is deliberately not added
to the existing devicetree-bindings.
Changes by Hans de Goede:
- Port to my bq25890 patch-series + various cleanups
- Make behavior configurable through a new "linux,pump-express-vbus-max"
device-property
- Sleep 1 second before re-checking the Vbus voltage after requesting
it to be raised, to ensure that the ADC has time to sampled the new Vbus
- Add VBUSV bq25890_tables[] entry and use it in bq25890_get_vbus_voltage()
- Tweak commit message
Signed-off-by: Yauhen Kharuzhy <[email protected]>
Co-developed-by: Hans de Goede <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
Changes in v3:
- Re-indent bq25890_tables to make checkpatch happy
- Add _uV postfix to PUMP_EXPRESS_VBUS_MARGIN
- Use regmap_field_read_poll_timeout() to wait for F_PUMPX_UP bit to clear
- Don't error check device_property_read_u32() call for optional prop
Changes in v2:
- New patch in v2 of this series, also see "Changes by Hans de Goede"
---
drivers/power/supply/bq25890_charger.c | 92 +++++++++++++++++++++++---
1 file changed, 84 insertions(+), 8 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 3de72f0fbf3e..179abed92f9b 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -27,6 +27,10 @@
#define BQ25895_ID 7
#define BQ25896_ID 0
+#define PUMP_EXPRESS_START_DELAY (5 * HZ)
+#define PUMP_EXPRESS_MAX_TRIES 6
+#define PUMP_EXPRESS_VBUS_MARGIN_uV 1000000
+
enum bq25890_chip_version {
BQ25890,
BQ25892,
@@ -107,6 +111,7 @@ struct bq25890_device {
struct usb_phy *usb_phy;
struct notifier_block usb_nb;
struct work_struct usb_work;
+ struct delayed_work pump_express_work;
unsigned long usb_event;
struct regmap *rmap;
@@ -114,6 +119,7 @@ struct bq25890_device {
bool skip_reset;
bool read_back_init_data;
+ u32 pump_express_vbus_max;
enum bq25890_chip_version chip_version;
struct bq25890_init_data init_data;
struct bq25890_state state;
@@ -265,6 +271,7 @@ enum bq25890_table_ids {
TBL_VREG,
TBL_BOOSTV,
TBL_SYSVMIN,
+ TBL_VBUSV,
TBL_VBATCOMP,
TBL_RBATCOMP,
@@ -325,14 +332,15 @@ static const union {
} bq25890_tables[] = {
/* range tables */
/* TODO: BQ25896 has max ICHG 3008 mA */
- [TBL_ICHG] = { .rt = {0, 5056000, 64000} }, /* uA */
- [TBL_ITERM] = { .rt = {64000, 1024000, 64000} }, /* 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 */
- [TBL_VBATCOMP] ={ .rt = {0, 224000, 32000} }, /* uV */
- [TBL_RBATCOMP] ={ .rt = {0, 140000, 20000} }, /* uOhm */
+ [TBL_ICHG] = { .rt = {0, 5056000, 64000} }, /* uA */
+ [TBL_ITERM] = { .rt = {64000, 1024000, 64000} }, /* 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 */
+ [TBL_VBUSV] = { .rt = {2600000, 15300000, 100000} }, /* uV */
+ [TBL_VBATCOMP] = { .rt = {0, 224000, 32000} }, /* uV */
+ [TBL_RBATCOMP] = { .rt = {0, 140000, 20000} }, /* uOhm */
/* lookup tables */
[TBL_TREG] = { .lt = {bq25890_treg_tbl, BQ25890_TREG_TBL_SIZE} },
@@ -435,6 +443,17 @@ static bool bq25890_is_adc_property(enum power_supply_property psp)
static irqreturn_t __bq25890_handle_irq(struct bq25890_device *bq);
+static int bq25890_get_vbus_voltage(struct bq25890_device *bq)
+{
+ int ret;
+
+ ret = bq25890_field_read(bq, F_VBUSV);
+ if (ret < 0)
+ return ret;
+
+ return bq25890_find_val(ret, TBL_VBUSV);
+}
+
static int bq25890_power_supply_get_property(struct power_supply *psy,
enum power_supply_property psp,
union power_supply_propval *val)
@@ -613,6 +632,11 @@ static void bq25890_charger_external_power_changed(struct power_supply *psy)
switch (val.intval) {
case POWER_SUPPLY_USB_TYPE_DCP:
input_current_limit = bq25890_find_idx(2000000, TBL_IINLIM);
+ if (bq->pump_express_vbus_max) {
+ queue_delayed_work(system_power_efficient_wq,
+ &bq->pump_express_work,
+ PUMP_EXPRESS_START_DELAY);
+ }
break;
case POWER_SUPPLY_USB_TYPE_CDP:
case POWER_SUPPLY_USB_TYPE_ACA:
@@ -878,6 +902,53 @@ static int bq25890_set_otg_cfg(struct bq25890_device *bq, u8 val)
return ret;
}
+static void bq25890_pump_express_work(struct work_struct *data)
+{
+ struct bq25890_device *bq =
+ container_of(data, struct bq25890_device, pump_express_work.work);
+ int voltage, i, ret;
+
+ dev_dbg(bq->dev, "Start to request input voltage increasing\n");
+
+ /* Enable current pulse voltage control protocol */
+ ret = bq25890_field_write(bq, F_PUMPX_EN, 1);
+ if (ret < 0)
+ goto error_print;
+
+ for (i = 0; i < PUMP_EXPRESS_MAX_TRIES; i++) {
+ voltage = bq25890_get_vbus_voltage(bq);
+ if (voltage < 0)
+ goto error_print;
+ dev_dbg(bq->dev, "input voltage = %d uV\n", voltage);
+
+ if ((voltage + PUMP_EXPRESS_VBUS_MARGIN_uV) >
+ bq->pump_express_vbus_max)
+ break;
+
+ ret = bq25890_field_write(bq, F_PUMPX_UP, 1);
+ if (ret < 0)
+ goto error_print;
+
+ /* Note a single PUMPX up pulse-sequence takes 2.1s */
+ ret = regmap_field_read_poll_timeout(bq->rmap_fields[F_PUMPX_UP],
+ ret, !ret, 100000, 3000000);
+ if (ret < 0)
+ goto error_print;
+
+ /* Make sure ADC has sampled Vbus before checking again */
+ msleep(1000);
+ }
+
+ bq25890_field_write(bq, F_PUMPX_EN, 0);
+
+ dev_info(bq->dev, "Hi-voltage charging requested, input voltage is %d mV\n",
+ voltage);
+
+ return;
+error_print:
+ dev_err(bq->dev, "Failed to request hi-voltage charging\n");
+}
+
static void bq25890_usb_work(struct work_struct *data)
{
int ret;
@@ -1068,6 +1139,10 @@ static int bq25890_fw_probe(struct bq25890_device *bq)
int ret;
struct bq25890_init_data *init = &bq->init_data;
+ /* Optional, left at 0 if property is not present */
+ device_property_read_u32(bq->dev, "linux,pump-express-vbus-max",
+ &bq->pump_express_vbus_max);
+
bq->skip_reset = device_property_read_bool(bq->dev, "linux,skip-reset");
bq->read_back_init_data = device_property_read_bool(bq->dev,
"linux,read-back-settings");
@@ -1100,6 +1175,7 @@ static int bq25890_probe(struct i2c_client *client,
bq->dev = dev;
mutex_init(&bq->lock);
+ INIT_DELAYED_WORK(&bq->pump_express_work, bq25890_pump_express_work);
bq->rmap = devm_regmap_init_i2c(client, &bq25890_regmap_config);
if (IS_ERR(bq->rmap))
--
2.33.1
On most x86/ACPI devices there is no devicetree to supply the necessary
init-data. Instead the firmware already fully initializes the bq25890
charger at boot.
Factor out the current code to write all the init_data from devicetree
into a new bq25890_rw_init_data() helper which can both write the data
to the charger (the current behavior) as well as read it back from
the charger into the init_data struct.
This is a preparation patch for adding support for x86/ACPI device's
where the init_data must be read back from the bq25890 charger.
Signed-off-by: Hans de Goede <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 62 +++++++++++++++++---------
1 file changed, 40 insertions(+), 22 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 7a3269c06b38..eaf0400b632f 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -693,29 +693,52 @@ static int bq25890_chip_reset(struct bq25890_device *bq)
return 0;
}
-static int bq25890_hw_init(struct bq25890_device *bq)
+static int bq25890_rw_init_data(struct bq25890_device *bq)
{
+ bool write = true;
int ret;
int i;
const struct {
enum bq25890_fields id;
- u32 value;
+ u8 *value;
} init_data[] = {
- {F_ICHG, bq->init_data.ichg},
- {F_VREG, bq->init_data.vreg},
- {F_ITERM, bq->init_data.iterm},
- {F_IPRECHG, bq->init_data.iprechg},
- {F_SYSVMIN, bq->init_data.sysvmin},
- {F_BOOSTV, bq->init_data.boostv},
- {F_BOOSTI, bq->init_data.boosti},
- {F_BOOSTF, bq->init_data.boostf},
- {F_EN_ILIM, bq->init_data.ilim_en},
- {F_TREG, bq->init_data.treg},
- {F_BATCMP, bq->init_data.rbatcomp},
- {F_VCLAMP, bq->init_data.vclamp},
+ {F_ICHG, &bq->init_data.ichg},
+ {F_VREG, &bq->init_data.vreg},
+ {F_ITERM, &bq->init_data.iterm},
+ {F_IPRECHG, &bq->init_data.iprechg},
+ {F_SYSVMIN, &bq->init_data.sysvmin},
+ {F_BOOSTV, &bq->init_data.boostv},
+ {F_BOOSTI, &bq->init_data.boosti},
+ {F_BOOSTF, &bq->init_data.boostf},
+ {F_EN_ILIM, &bq->init_data.ilim_en},
+ {F_TREG, &bq->init_data.treg},
+ {F_BATCMP, &bq->init_data.rbatcomp},
+ {F_VCLAMP, &bq->init_data.vclamp},
};
+ for (i = 0; i < ARRAY_SIZE(init_data); i++) {
+ if (write) {
+ ret = bq25890_field_write(bq, init_data[i].id,
+ *init_data[i].value);
+ } else {
+ ret = bq25890_field_read(bq, init_data[i].id);
+ if (ret >= 0)
+ *init_data[i].value = ret;
+ }
+ if (ret < 0) {
+ dev_dbg(bq->dev, "Accessing init data failed %d\n", ret);
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static int bq25890_hw_init(struct bq25890_device *bq)
+{
+ int ret;
+
ret = bq25890_chip_reset(bq);
if (ret < 0) {
dev_dbg(bq->dev, "Reset failed %d\n", ret);
@@ -730,14 +753,9 @@ static int bq25890_hw_init(struct bq25890_device *bq)
}
/* initialize currents/voltages and other parameters */
- for (i = 0; i < ARRAY_SIZE(init_data); i++) {
- ret = bq25890_field_write(bq, init_data[i].id,
- init_data[i].value);
- if (ret < 0) {
- dev_dbg(bq->dev, "Writing init data failed %d\n", ret);
- return ret;
- }
- }
+ ret = bq25890_rw_init_data(bq);
+ if (ret)
+ return ret;
ret = bq25890_get_chip_state(bq, &bq->state);
if (ret < 0) {
--
2.33.1
Use the devm_regmap_field_bulk_alloc() helper function instead of
open-coding this ourselves.
Signed-off-by: Hans de Goede <[email protected]>
---
Changes in v3:
- This is a new patch in v3 of this patch-series
---
drivers/power/supply/bq25890_charger.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 179abed92f9b..852a6fec4339 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -1165,7 +1165,6 @@ static int bq25890_probe(struct i2c_client *client,
struct device *dev = &client->dev;
struct bq25890_device *bq;
int ret;
- int i;
bq = devm_kzalloc(dev, sizeof(*bq), GFP_KERNEL);
if (!bq)
@@ -1182,15 +1181,10 @@ static int bq25890_probe(struct i2c_client *client,
return dev_err_probe(dev, PTR_ERR(bq->rmap),
"failed to allocate register map\n");
- for (i = 0; i < ARRAY_SIZE(bq25890_reg_fields); i++) {
- const struct reg_field *reg_fields = bq25890_reg_fields;
-
- bq->rmap_fields[i] = devm_regmap_field_alloc(dev, bq->rmap,
- reg_fields[i]);
- if (IS_ERR(bq->rmap_fields[i]))
- return dev_err_probe(dev, PTR_ERR(bq->rmap_fields[i]),
- "cannot allocate regmap field\n");
- }
+ ret = devm_regmap_field_bulk_alloc(dev, bq->rmap, bq->rmap_fields,
+ bq25890_reg_fields, F_MAX_FIELDS);
+ if (ret)
+ return ret;
i2c_set_clientdata(client, bq);
--
2.33.1
The CHT_WC_VBUS_GPIO_CTLO GPIO actually driving an external 5V Vboost
converter for Vbus depends on the board on which the Cherry Trail -
Whiskey Cove PMIC is actually used.
Since the information about the exact PMIC setup is necessary in other
places too, struct intel_soc_pmic now has a new cht_wc_model field
indicating the board model.
Only poke the CHT_WC_VBUS_GPIO_CTLO GPIO if this new field is set to
INTEL_CHT_WC_GPD_WIN_POCKET, which indicates the Type-C (with PD and
DP-altmode) setup used on the GPD pocket and GPD win; and on which
this GPIO actually controls an external 5V Vboost converter.
Acked-by: Chanwoo Choi <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
Changes in v3:
- Use the new cht_wc_model intel_soc_pmic field which replaces the
intel_cht_wc_get_model() helper and adjust the commit msg to match
---
drivers/extcon/extcon-intel-cht-wc.c | 35 +++++++++++++++++-----------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
index 771f6f4cf92e..81cae8c75850 100644
--- a/drivers/extcon/extcon-intel-cht-wc.c
+++ b/drivers/extcon/extcon-intel-cht-wc.c
@@ -14,6 +14,7 @@
#include <linux/module.h>
#include <linux/mod_devicetable.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/slab.h>
@@ -358,20 +359,26 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
if (IS_ERR(ext->edev))
return PTR_ERR(ext->edev);
- /*
- * When a host-cable is detected the BIOS enables an external 5v boost
- * converter to power connected devices there are 2 problems with this:
- * 1) This gets seen by the external battery charger as a valid Vbus
- * supply and it then tries to feed Vsys from this creating a
- * feedback loop which causes aprox. 300 mA extra battery drain
- * (and unless we drive the external-charger-disable pin high it
- * also tries to charge the battery causing even more feedback).
- * 2) This gets seen by the pwrsrc block as a SDP USB Vbus supply
- * Since the external battery charger has its own 5v boost converter
- * which does not have these issues, we simply turn the separate
- * external 5v boost converter off and leave it off entirely.
- */
- cht_wc_extcon_set_5v_boost(ext, false);
+ switch (pmic->cht_wc_model) {
+ case INTEL_CHT_WC_GPD_WIN_POCKET:
+ /*
+ * When a host-cable is detected the BIOS enables an external 5v boost
+ * converter to power connected devices there are 2 problems with this:
+ * 1) This gets seen by the external battery charger as a valid Vbus
+ * supply and it then tries to feed Vsys from this creating a
+ * feedback loop which causes aprox. 300 mA extra battery drain
+ * (and unless we drive the external-charger-disable pin high it
+ * also tries to charge the battery causing even more feedback).
+ * 2) This gets seen by the pwrsrc block as a SDP USB Vbus supply
+ * Since the external battery charger has its own 5v boost converter
+ * which does not have these issues, we simply turn the separate
+ * external 5v boost converter off and leave it off entirely.
+ */
+ cht_wc_extcon_set_5v_boost(ext, false);
+ break;
+ default:
+ break;
+ }
/* Enable sw control */
ret = cht_wc_extcon_sw_control(ext, true);
--
2.33.1
So far the extcon-intel-cht-wc code has only been tested on devices with
a Type-C connector with USB-PD, USB3 (superspeed) and DP-altmode support
through a FUSB302 Type-C controller.
Some devices with the intel-cht-wc PMIC however come with an USB-micro-B
connector, or an USB-2 only Type-C connector without USB-PD.
Which device-model we are running on can be identified with the new
cht_wc_model intel_soc_pmic field. On models without a Type-C controller
the extcon code must control the Vbus 5V boost converter and the USB role
switch depending on the detected cable-type.
Acked-by: Chanwoo Choi <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
Changes in v3:
- Some minor tweaks / spelling fixes based on Andy's review
---
drivers/extcon/Kconfig | 2 +
drivers/extcon/extcon-intel-cht-wc.c | 91 ++++++++++++++++++++++++++++
2 files changed, 93 insertions(+)
diff --git a/drivers/extcon/Kconfig b/drivers/extcon/Kconfig
index aab87c9b35c8..0d42e49105dd 100644
--- a/drivers/extcon/Kconfig
+++ b/drivers/extcon/Kconfig
@@ -61,6 +61,8 @@ config EXTCON_INTEL_INT3496
config EXTCON_INTEL_CHT_WC
tristate "Intel Cherrytrail Whiskey Cove PMIC extcon driver"
depends on INTEL_SOC_PMIC_CHTWC
+ depends on USB_SUPPORT
+ select USB_ROLE_SWITCH
help
Say Y here to enable extcon support for charger detection / control
on the Intel Cherrytrail Whiskey Cove PMIC.
diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
index 81cae8c75850..edc386937dee 100644
--- a/drivers/extcon/extcon-intel-cht-wc.c
+++ b/drivers/extcon/extcon-intel-cht-wc.c
@@ -16,7 +16,9 @@
#include <linux/platform_device.h>
#include <linux/property.h>
#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
#include <linux/slab.h>
+#include <linux/usb/role.h>
#include "extcon-intel.h"
@@ -102,8 +104,11 @@ struct cht_wc_extcon_data {
struct device *dev;
struct regmap *regmap;
struct extcon_dev *edev;
+ struct usb_role_switch *role_sw;
+ struct regulator *vbus_boost;
unsigned int previous_cable;
bool usb_host;
+ bool vbus_boost_enabled;
};
static int cht_wc_extcon_get_id(struct cht_wc_extcon_data *ext, int pwrsrc_sts)
@@ -217,6 +222,18 @@ static void cht_wc_extcon_set_otgmode(struct cht_wc_extcon_data *ext,
CHT_WC_CHGRCTRL1_OTGMODE, val);
if (ret)
dev_err(ext->dev, "Error updating CHGRCTRL1 reg: %d\n", ret);
+
+ if (ext->vbus_boost && ext->vbus_boost_enabled != enable) {
+ if (enable)
+ ret = regulator_enable(ext->vbus_boost);
+ else
+ ret = regulator_disable(ext->vbus_boost);
+
+ if (ret == 0)
+ ext->vbus_boost_enabled = enable;
+ else
+ dev_err(ext->dev, "Error updating Vbus boost regulator: %d\n", ret);
+ }
}
static void cht_wc_extcon_enable_charging(struct cht_wc_extcon_data *ext,
@@ -246,6 +263,7 @@ static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
unsigned int cable = EXTCON_NONE;
/* Ignore errors in host mode, as the 5v boost converter is on then */
bool ignore_get_charger_errors = ext->usb_host;
+ enum usb_role role;
ret = regmap_read(ext->regmap, CHT_WC_PWRSRC_STS, &pwrsrc_sts);
if (ret) {
@@ -289,6 +307,18 @@ static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
ext->usb_host = ((id == INTEL_USB_ID_GND) || (id == INTEL_USB_RID_A));
extcon_set_state_sync(ext->edev, EXTCON_USB_HOST, ext->usb_host);
+
+ if (ext->usb_host)
+ role = USB_ROLE_HOST;
+ else if (pwrsrc_sts & CHT_WC_PWRSRC_VBUS)
+ role = USB_ROLE_DEVICE;
+ else
+ role = USB_ROLE_NONE;
+
+ /* Note: this is a no-op when ext->role_sw is NULL */
+ ret = usb_role_switch_set_role(ext->role_sw, role);
+ if (ret)
+ dev_err(ext->dev, "Error setting USB-role: %d\n", ret);
}
static irqreturn_t cht_wc_extcon_isr(int irq, void *data)
@@ -334,6 +364,61 @@ static int cht_wc_extcon_sw_control(struct cht_wc_extcon_data *ext, bool enable)
return ret;
}
+static int cht_wc_extcon_find_role_sw(struct cht_wc_extcon_data *ext)
+{
+ const struct software_node *swnode;
+ struct fwnode_handle *fwnode;
+
+ swnode = software_node_find_by_name(NULL, "intel-xhci-usb-sw");
+ if (!swnode)
+ return -EPROBE_DEFER;
+
+ fwnode = software_node_fwnode(swnode);
+ ext->role_sw = usb_role_switch_find_by_fwnode(fwnode);
+ fwnode_handle_put(fwnode);
+
+ return ext->role_sw ? 0 : -EPROBE_DEFER;
+}
+
+static void cht_wc_extcon_put_role_sw(void *data)
+{
+ struct cht_wc_extcon_data *ext = data;
+
+ usb_role_switch_put(ext->role_sw);
+}
+
+/* Some boards require controlling the role-sw and Vbus based on the id-pin */
+static int cht_wc_extcon_get_role_sw_and_regulator(struct cht_wc_extcon_data *ext)
+{
+ int ret;
+
+ ret = cht_wc_extcon_find_role_sw(ext);
+ if (ret)
+ return ret;
+
+ ret = devm_add_action_or_reset(ext->dev, cht_wc_extcon_put_role_sw, ext);
+ if (ret)
+ return ret;
+
+ /*
+ * On x86/ACPI platforms the regulator <-> consumer link is provided
+ * by platform_data passed to the regulator driver. This means that
+ * this info is not available before the regulator driver has bound.
+ * Use devm_regulator_get_optional() to avoid getting a dummy
+ * regulator and wait for the regulator to show up if necessary.
+ */
+ ext->vbus_boost = devm_regulator_get_optional(ext->dev, "vbus");
+ if (IS_ERR(ext->vbus_boost)) {
+ ret = PTR_ERR(ext->vbus_boost);
+ if (ret == -ENODEV)
+ ret = -EPROBE_DEFER;
+
+ return dev_err_probe(ext->dev, ret, "getting Vbus regulator");
+ }
+
+ return 0;
+}
+
static int cht_wc_extcon_probe(struct platform_device *pdev)
{
struct intel_soc_pmic *pmic = dev_get_drvdata(pdev->dev.parent);
@@ -376,6 +461,12 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
*/
cht_wc_extcon_set_5v_boost(ext, false);
break;
+ case INTEL_CHT_WC_LENOVO_YOGABOOK1:
+ case INTEL_CHT_WC_XIAOMI_MIPAD2:
+ ret = cht_wc_extcon_get_role_sw_and_regulator(ext);
+ if (ret)
+ return ret;
+ break;
default:
break;
}
--
2.33.1
This is a preparation patch for adding support for registering
a power_supply class device.
Setting usbsrc to "CHT_WC_USBSRC_TYPE_SDP << CHT_WC_USBSRC_TYPE_SHIFT"
will make the following switch-case return EXTCON_CHG_USB_SDP
just as before, so there is no functional change.
Acked-by: Chanwoo Choi <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
Changes in v3:
- Reword the commit message
---
drivers/extcon/extcon-intel-cht-wc.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
index edc386937dee..150637bea417 100644
--- a/drivers/extcon/extcon-intel-cht-wc.c
+++ b/drivers/extcon/extcon-intel-cht-wc.c
@@ -153,14 +153,15 @@ static int cht_wc_extcon_get_charger(struct cht_wc_extcon_data *ext,
} while (time_before(jiffies, timeout));
if (status != CHT_WC_USBSRC_STS_SUCCESS) {
- if (ignore_errors)
- return EXTCON_CHG_USB_SDP; /* Save fallback */
+ if (!ignore_errors) {
+ if (status == CHT_WC_USBSRC_STS_FAIL)
+ dev_warn(ext->dev, "Could not detect charger type\n");
+ else
+ dev_warn(ext->dev, "Timeout detecting charger type\n");
+ }
- if (status == CHT_WC_USBSRC_STS_FAIL)
- dev_warn(ext->dev, "Could not detect charger type\n");
- else
- dev_warn(ext->dev, "Timeout detecting charger type\n");
- return EXTCON_CHG_USB_SDP; /* Save fallback */
+ /* Save fallback */
+ usbsrc = CHT_WC_USBSRC_TYPE_SDP << CHT_WC_USBSRC_TYPE_SHIFT;
}
usbsrc = (usbsrc & CHT_WC_USBSRC_TYPE_MASK) >> CHT_WC_USBSRC_TYPE_SHIFT;
--
2.33.1
The bq25890 used on the Yogabook YB1-X90 / -X91 models relies on
the extcon-driver's BC-1.2 charger detection, and the bq25890 driver
expect this info to be available through a parent power_supply
class-device which models the detected charger (idem to how the Type-C
TCPM code registers a power_supply classdev for the connected charger).
Add support for registering the power_supply class-device expected
by this setup.
Acked-by: Chanwoo Choi <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
Changes in v3:
- Some minor tweaks based on Andy's review
---
drivers/extcon/extcon-intel-cht-wc.c | 81 ++++++++++++++++++++++++++++
1 file changed, 81 insertions(+)
diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
index 150637bea417..144cb5d8cd47 100644
--- a/drivers/extcon/extcon-intel-cht-wc.c
+++ b/drivers/extcon/extcon-intel-cht-wc.c
@@ -14,6 +14,7 @@
#include <linux/module.h>
#include <linux/mod_devicetable.h>
#include <linux/platform_device.h>
+#include <linux/power_supply.h>
#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
@@ -106,6 +107,8 @@ struct cht_wc_extcon_data {
struct extcon_dev *edev;
struct usb_role_switch *role_sw;
struct regulator *vbus_boost;
+ struct power_supply *psy;
+ enum power_supply_usb_type usb_type;
unsigned int previous_cable;
bool usb_host;
bool vbus_boost_enabled;
@@ -170,18 +173,23 @@ static int cht_wc_extcon_get_charger(struct cht_wc_extcon_data *ext,
dev_warn(ext->dev,
"Unhandled charger type %d, defaulting to SDP\n",
ret);
+ ext->usb_type = POWER_SUPPLY_USB_TYPE_SDP;
return EXTCON_CHG_USB_SDP;
case CHT_WC_USBSRC_TYPE_SDP:
case CHT_WC_USBSRC_TYPE_FLOATING:
case CHT_WC_USBSRC_TYPE_OTHER:
+ ext->usb_type = POWER_SUPPLY_USB_TYPE_SDP;
return EXTCON_CHG_USB_SDP;
case CHT_WC_USBSRC_TYPE_CDP:
+ ext->usb_type = POWER_SUPPLY_USB_TYPE_CDP;
return EXTCON_CHG_USB_CDP;
case CHT_WC_USBSRC_TYPE_DCP:
case CHT_WC_USBSRC_TYPE_DCP_EXTPHY:
case CHT_WC_USBSRC_TYPE_MHL: /* MHL2+ delivers upto 2A, treat as DCP */
+ ext->usb_type = POWER_SUPPLY_USB_TYPE_DCP;
return EXTCON_CHG_USB_DCP;
case CHT_WC_USBSRC_TYPE_ACA:
+ ext->usb_type = POWER_SUPPLY_USB_TYPE_ACA;
return EXTCON_CHG_USB_ACA;
}
}
@@ -266,6 +274,8 @@ static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
bool ignore_get_charger_errors = ext->usb_host;
enum usb_role role;
+ ext->usb_type = POWER_SUPPLY_USB_TYPE_UNKNOWN;
+
ret = regmap_read(ext->regmap, CHT_WC_PWRSRC_STS, &pwrsrc_sts);
if (ret) {
dev_err(ext->dev, "Error reading pwrsrc status: %d\n", ret);
@@ -320,6 +330,9 @@ static void cht_wc_extcon_pwrsrc_event(struct cht_wc_extcon_data *ext)
ret = usb_role_switch_set_role(ext->role_sw, role);
if (ret)
dev_err(ext->dev, "Error setting USB-role: %d\n", ret);
+
+ if (ext->psy)
+ power_supply_changed(ext->psy);
}
static irqreturn_t cht_wc_extcon_isr(int irq, void *data)
@@ -420,6 +433,59 @@ static int cht_wc_extcon_get_role_sw_and_regulator(struct cht_wc_extcon_data *ex
return 0;
}
+static int cht_wc_extcon_psy_get_prop(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct cht_wc_extcon_data *ext = power_supply_get_drvdata(psy);
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_USB_TYPE:
+ val->intval = ext->usb_type;
+ break;
+ case POWER_SUPPLY_PROP_ONLINE:
+ val->intval = ext->usb_type ? 1 : 0;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static const enum power_supply_usb_type cht_wc_extcon_psy_usb_types[] = {
+ POWER_SUPPLY_USB_TYPE_SDP,
+ POWER_SUPPLY_USB_TYPE_CDP,
+ POWER_SUPPLY_USB_TYPE_DCP,
+ POWER_SUPPLY_USB_TYPE_ACA,
+ POWER_SUPPLY_USB_TYPE_UNKNOWN,
+};
+
+static const enum power_supply_property cht_wc_extcon_psy_props[] = {
+ POWER_SUPPLY_PROP_USB_TYPE,
+ POWER_SUPPLY_PROP_ONLINE,
+};
+
+static const struct power_supply_desc cht_wc_extcon_psy_desc = {
+ .name = "cht_wcove_pwrsrc",
+ .type = POWER_SUPPLY_TYPE_USB,
+ .usb_types = cht_wc_extcon_psy_usb_types,
+ .num_usb_types = ARRAY_SIZE(cht_wc_extcon_psy_usb_types),
+ .properties = cht_wc_extcon_psy_props,
+ .num_properties = ARRAY_SIZE(cht_wc_extcon_psy_props),
+ .get_property = cht_wc_extcon_psy_get_prop,
+};
+
+static int cht_wc_extcon_register_psy(struct cht_wc_extcon_data *ext)
+{
+ struct power_supply_config psy_cfg = { .drv_data = ext };
+
+ ext->psy = devm_power_supply_register(ext->dev,
+ &cht_wc_extcon_psy_desc,
+ &psy_cfg);
+ return PTR_ERR_OR_ZERO(ext->psy);
+}
+
static int cht_wc_extcon_probe(struct platform_device *pdev)
{
struct intel_soc_pmic *pmic = dev_get_drvdata(pdev->dev.parent);
@@ -463,6 +529,21 @@ static int cht_wc_extcon_probe(struct platform_device *pdev)
cht_wc_extcon_set_5v_boost(ext, false);
break;
case INTEL_CHT_WC_LENOVO_YOGABOOK1:
+ /* Do this first, as it may very well return -EPROBE_DEFER. */
+ ret = cht_wc_extcon_get_role_sw_and_regulator(ext);
+ if (ret)
+ return ret;
+ /*
+ * The bq25890 used here relies on this driver's BC-1.2 charger
+ * detection, and the bq25890 driver expect this info to be
+ * available through a parent power_supply class device which
+ * models the detected charger (idem to how the Type-C TCPM code
+ * registers a power_supply classdev for the connected charger).
+ */
+ ret = cht_wc_extcon_register_psy(ext);
+ if (ret)
+ return ret;
+ break;
case INTEL_CHT_WC_XIAOMI_MIPAD2:
ret = cht_wc_extcon_get_role_sw_and_regulator(ext);
if (ret)
--
2.33.1
Make cht_wc_extcon_get_id() report RID_A for ACA adapters, instead of
reporting ID_FLOAT.
According to the spec. we should read the USB-ID pin ADC value
to determine the resistance of the used pull-down resister and
then return RID_A / RID_B / RID_C based on this. But all "Accessory
Charger Adapter"s (ACAs) which users can actually buy always use
a combination of a charging port with one or more USB-A ports, so
they should always use a resistor indicating RID_A. But the spec
is hard to read / badly-worded so some of them actually indicate
they are a RID_B ACA even though they clearly are a RID_A ACA.
To workaround this simply always return INTEL_USB_RID_A, which
matches all the ACAs which users can actually buy.
Acked-by: Chanwoo Choi <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
drivers/extcon/extcon-intel-cht-wc.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c
index 144cb5d8cd47..2a8d41cbf41c 100644
--- a/drivers/extcon/extcon-intel-cht-wc.c
+++ b/drivers/extcon/extcon-intel-cht-wc.c
@@ -121,13 +121,21 @@ static int cht_wc_extcon_get_id(struct cht_wc_extcon_data *ext, int pwrsrc_sts)
return INTEL_USB_ID_GND;
case CHT_WC_PWRSRC_RID_FLOAT:
return INTEL_USB_ID_FLOAT;
+ /*
+ * According to the spec. we should read the USB-ID pin ADC value here
+ * to determine the resistance of the used pull-down resister and then
+ * return RID_A / RID_B / RID_C based on this. But all "Accessory
+ * Charger Adapter"s (ACAs) which users can actually buy always use
+ * a combination of a charging port with one or more USB-A ports, so
+ * they should always use a resistor indicating RID_A. But the spec
+ * is hard to read / badly-worded so some of them actually indicate
+ * they are a RID_B ACA evnen though they clearly are a RID_A ACA.
+ * To workaround this simply always return INTEL_USB_RID_A, which
+ * matches all the ACAs which users can actually buy.
+ */
case CHT_WC_PWRSRC_RID_ACA:
+ return INTEL_USB_RID_A;
default:
- /*
- * Once we have IIO support for the GPADC we should read
- * the USBID GPADC channel here and determine ACA role
- * based on that.
- */
return INTEL_USB_ID_FLOAT;
}
}
--
2.33.1
Tablet / laptop designs using an Intel Cherry Trail x86 main SoC with
an Intel Whiskey Cove PMIC do not use a single standard setup for
the charger, fuel-gauge and other chips surrounding the PMIC /
charging+data USB port.
Unlike what is normal on x86 this diversity in designs is not handled
by the ACPI tables. On 2 of the 3 known designs there are no standard
(PNP0C0A) ACPI battery devices and on the 3th design the ACPI battery
device does not work under Linux due to it requiring non-standard
and undocumented ACPI behavior.
So to make things work under Linux we use native charger and fuel-gauge
drivers on these devices, re-using the native drivers used on ARM boards
with the same charger / fuel-gauge ICs.
This requires various MFD-cell drivers for the CHT-WC PMIC cells to
know which model they are exactly running on so that they can e.g.
instantiate an I2C-client for the right model charger-IC (the charger
is connected to an I2C-controller which is part of the PMIC).
Rather then duplicating DMI-id matching to check which model we are
running on in each MFD-cell driver, add a check for this to the
shared drivers/mfd/intel_soc_pmic_chtwc.c code by using a
DMI table for all 3 known models:
1. The GPD Win and GPD Pocket mini-laptops, these are really 2 models
but the Pocket re-uses the GPD Win's design in a different housing:
The WC PMIC is connected to a TI BQ24292i charger, paired with
a Maxim MAX17047 fuelgauge + a FUSB302 USB Type-C Controller +
a PI3USB30532 USB switch, for a fully functional Type-C port.
2. The Xiaomi Mi Pad 2:
The WC PMIC is connected to a TI BQ25890 charger, paired with
a TI BQ27520 fuelgauge, using the TI BQ25890 for BC1.2 charger type
detection, for a USB-2 only Type-C port without PD.
3. The Lenovo Yoga Book YB1-X90 / Lenovo Yoga Book YB1-X91 series:
The WC PMIC is connected to a TI BQ25892 charger, paired with
a TI BQ27542 fuelgauge, using the WC PMIC for BC1.2 charger type
detection and using the BQ25892's Mediatek Pump Express+ (1.0)
support to enable charging with up to 12V through a micro-USB port.
Reviewed-by: Andy Shevchenko <[email protected]>
Acked-by: Lee Jones <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
Changes in v4:
- Put '{' and comment of DMI entries on separate lines (requested by Lee)
- Drop comment on terminating empty entry in DMI table
Changes in v3:
- Store the model in struct intel_soc_pmic instead of adding a helper
function to retreive it
Changes in v2:
- New patch in v2 of this patch-set
---
drivers/mfd/intel_soc_pmic_chtwc.c | 40 ++++++++++++++++++++++++++++++
include/linux/mfd/intel_soc_pmic.h | 8 ++++++
2 files changed, 48 insertions(+)
diff --git a/drivers/mfd/intel_soc_pmic_chtwc.c b/drivers/mfd/intel_soc_pmic_chtwc.c
index 49c5f71664bc..4eab191e053a 100644
--- a/drivers/mfd/intel_soc_pmic_chtwc.c
+++ b/drivers/mfd/intel_soc_pmic_chtwc.c
@@ -10,6 +10,7 @@
#include <linux/acpi.h>
#include <linux/delay.h>
+#include <linux/dmi.h>
#include <linux/err.h>
#include <linux/i2c.h>
#include <linux/interrupt.h>
@@ -134,9 +135,44 @@ static const struct regmap_irq_chip cht_wc_regmap_irq_chip = {
.num_regs = 1,
};
+static const struct dmi_system_id cht_wc_model_dmi_ids[] = {
+ {
+ /* GPD win / GPD pocket mini laptops */
+ .driver_data = (void *)(long)INTEL_CHT_WC_GPD_WIN_POCKET,
+ /*
+ * This DMI match may not seem unique, but it is. In the 67000+
+ * DMI decode dumps from linux-hardware.org only 116 have
+ * board_vendor set to "AMI Corporation" and of those 116 only
+ * the GPD win's and pocket's board_name is "Default string".
+ */
+ .matches = {
+ DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "AMI Corporation"),
+ DMI_EXACT_MATCH(DMI_BOARD_NAME, "Default string"),
+ DMI_EXACT_MATCH(DMI_BOARD_SERIAL, "Default string"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Default string"),
+ },
+ }, {
+ /* Xiaomi Mi Pad 2 */
+ .driver_data = (void *)(long)INTEL_CHT_WC_XIAOMI_MIPAD2,
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Xiaomi Inc"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Mipad2"),
+ },
+ }, {
+ /* Lenovo Yoga Book X90F / X91F / X91L */
+ .driver_data = (void *)(long)INTEL_CHT_WC_LENOVO_YOGABOOK1,
+ .matches = {
+ /* Non exact match to match all versions */
+ DMI_MATCH(DMI_PRODUCT_NAME, "Lenovo YB1-X9"),
+ },
+ },
+ { }
+};
+
static int cht_wc_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
+ const struct dmi_system_id *id;
struct intel_soc_pmic *pmic;
acpi_status status;
unsigned long long hrv;
@@ -160,6 +196,10 @@ static int cht_wc_probe(struct i2c_client *client)
if (!pmic)
return -ENOMEM;
+ id = dmi_first_match(cht_wc_model_dmi_ids);
+ if (id)
+ pmic->cht_wc_model = (long)id->driver_data;
+
pmic->irq = client->irq;
pmic->dev = dev;
i2c_set_clientdata(client, pmic);
diff --git a/include/linux/mfd/intel_soc_pmic.h b/include/linux/mfd/intel_soc_pmic.h
index 6a88e34cb955..945bde1fe55c 100644
--- a/include/linux/mfd/intel_soc_pmic.h
+++ b/include/linux/mfd/intel_soc_pmic.h
@@ -13,6 +13,13 @@
#include <linux/regmap.h>
+enum intel_cht_wc_models {
+ INTEL_CHT_WC_UNKNOWN,
+ INTEL_CHT_WC_GPD_WIN_POCKET,
+ INTEL_CHT_WC_XIAOMI_MIPAD2,
+ INTEL_CHT_WC_LENOVO_YOGABOOK1,
+};
+
/**
* struct intel_soc_pmic - Intel SoC PMIC data
* @irq: Master interrupt number of the parent PMIC device
@@ -39,6 +46,7 @@ struct intel_soc_pmic {
struct regmap_irq_chip_data *irq_chip_data_crit;
struct device *dev;
struct intel_scu_ipc_dev *scu;
+ enum intel_cht_wc_models cht_wc_model;
};
int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
--
2.33.1
Drop the "if (!dev->platform_data)" check, this seems to be an attempt
for allowing loading the driver on devices without devicetree stemming
from the initial commit of the driver (with the presumed intention being
the "return -ENODEV" else branch getting replaced with something else).
With the new "linux,skip-init" and "linux,read-back-settings" properties
the driver can actually supports devices without devicetree and this
check no longer makes sense.
While at it, also switch to dev_err_probe(), which is already used in
various other places in the driver.
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
drivers/power/supply/bq25890_charger.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index d185299db9c3..548d1a793e31 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -1048,16 +1048,9 @@ static int bq25890_probe(struct i2c_client *client,
return ret;
}
- if (!dev->platform_data) {
- ret = bq25890_fw_probe(bq);
- if (ret < 0) {
- dev_err(dev, "Cannot read device properties: %d\n",
- ret);
- return ret;
- }
- } else {
- return -ENODEV;
- }
+ ret = bq25890_fw_probe(bq);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "reading device properties\n");
ret = bq25890_hw_init(bq);
if (ret < 0) {
--
2.33.1
The i2c-controller on the Cherry Trail - Whiskey Cove PMIC is special
in that it is always connected to the I2C charger IC of the board on
which the PMIC is used; and the charger IC is not described in ACPI,
so the i2c-cht-wc code needs to instantiate an i2c-client for it itself.
So far this was hardcoded to instantiate an i2c-client for the
bq24292i, with all properties, etc. set to match how this charger
is used on the GPD win and GPD pocket devices.
There is a rudimentary check to make sure the ACPI tables are at least
somewhat as expected, but this is far from accurate, leading to
a wrong i2c-client being instantiated for the charger on some boards.
Switch to the new DMI based intel_cht_wc_get_model() helper which is
exported by the MFD driver for the CHT Whiskey Cove PMIC to help PMIC
cell drivers like the i2c-cht-wc code reliably detect which board
they are running on.
And add board_info for the charger ICs as found on the other 2 known
boards with a Whisky Cove PMIC.
This has been tested on all 3 known boards.
Acked-by: Wolfram Sang <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
Changes in v3:
- Add Wolfram's Ack for taking this upstream through another tree
then the i2c tree
- Some minor tweaks / spelling fixes based on Andy's review
---
drivers/i2c/busses/i2c-cht-wc.c | 120 +++++++++++++++++++++++++++-----
1 file changed, 102 insertions(+), 18 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cht-wc.c b/drivers/i2c/busses/i2c-cht-wc.c
index 1cf68f85b2e1..54e909f9eab6 100644
--- a/drivers/i2c/busses/i2c-cht-wc.c
+++ b/drivers/i2c/busses/i2c-cht-wc.c
@@ -18,6 +18,7 @@
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/power/bq24190_charger.h>
+#include <linux/power/bq25890_charger.h>
#include <linux/slab.h>
#define CHT_WC_I2C_CTRL 0x5e24
@@ -270,6 +271,7 @@ static const struct irq_chip cht_wc_i2c_irq_chip = {
.name = "cht_wc_ext_chrg_irq_chip",
};
+/********** GPD Win / Pocket charger IC settings **********/
static const char * const bq24190_suppliers[] = {
"tcpm-source-psy-i2c-fusb302" };
@@ -304,17 +306,92 @@ static struct bq24190_platform_data bq24190_pdata = {
.regulator_init_data = &bq24190_vbus_init_data,
};
+static struct i2c_board_info gpd_win_board_info = {
+ .type = "bq24190",
+ .addr = 0x6b,
+ .dev_name = "bq24190",
+ .swnode = &bq24190_node,
+ .platform_data = &bq24190_pdata,
+};
+
+/********** Xiaomi Mi Pad 2 charger IC settings **********/
+static struct regulator_consumer_supply bq2589x_vbus_consumer = {
+ .supply = "vbus",
+ .dev_name = "cht_wcove_pwrsrc",
+};
+
+static const struct regulator_init_data bq2589x_vbus_init_data = {
+ .constraints = {
+ .valid_ops_mask = REGULATOR_CHANGE_STATUS,
+ },
+ .consumer_supplies = &bq2589x_vbus_consumer,
+ .num_consumer_supplies = 1,
+};
+
+static struct bq25890_platform_data bq2589x_pdata = {
+ .regulator_init_data = &bq2589x_vbus_init_data,
+};
+
+static const struct property_entry xiaomi_mipad2_props[] = {
+ PROPERTY_ENTRY_BOOL("linux,skip-reset"),
+ PROPERTY_ENTRY_BOOL("linux,read-back-settings"),
+ { }
+};
+
+static const struct software_node xiaomi_mipad2_node = {
+ .properties = xiaomi_mipad2_props,
+};
+
+static struct i2c_board_info xiaomi_mipad2_board_info = {
+ .type = "bq25890",
+ .addr = 0x6a,
+ .dev_name = "bq25890",
+ .swnode = &xiaomi_mipad2_node,
+ .platform_data = &bq2589x_pdata,
+};
+
+/********** Lenovo Yogabook YB1-X90F/-X91F/-X91L charger settings **********/
+static const char * const lenovo_yb1_bq25892_suppliers[] = { "cht_wcove_pwrsrc" };
+
+static const struct property_entry lenovo_yb1_bq25892_props[] = {
+ PROPERTY_ENTRY_STRING_ARRAY("supplied-from",
+ lenovo_yb1_bq25892_suppliers),
+ PROPERTY_ENTRY_U32("linux,pump-express-vbus-max", 12000000),
+ PROPERTY_ENTRY_BOOL("linux,skip-reset"),
+ /*
+ * The firmware sets everything to the defaults, which leads to a
+ * somewhat low charge-current of 2048mA and worse to a battery-voltage
+ * of 4.2V instead of 4.35V (when booted without a charger connected).
+ * Use our own values instead of "linux,read-back-settings" to fix this.
+ */
+ PROPERTY_ENTRY_U32("ti,charge-current", 4224000),
+ PROPERTY_ENTRY_U32("ti,battery-regulation-voltage", 4352000),
+ PROPERTY_ENTRY_U32("ti,termination-current", 256000),
+ PROPERTY_ENTRY_U32("ti,precharge-current", 128000),
+ PROPERTY_ENTRY_U32("ti,minimum-sys-voltage", 3500000),
+ PROPERTY_ENTRY_U32("ti,boost-voltage", 4998000),
+ PROPERTY_ENTRY_U32("ti,boost-max-current", 1400000),
+ PROPERTY_ENTRY_BOOL("ti,use-ilim-pin"),
+ { }
+};
+
+static const struct software_node lenovo_yb1_bq25892_node = {
+ .properties = lenovo_yb1_bq25892_props,
+};
+
+static struct i2c_board_info lenovo_yogabook1_board_info = {
+ .type = "bq25892",
+ .addr = 0x6b,
+ .dev_name = "bq25892",
+ .swnode = &lenovo_yb1_bq25892_node,
+ .platform_data = &bq2589x_pdata,
+};
+
static int cht_wc_i2c_adap_i2c_probe(struct platform_device *pdev)
{
struct intel_soc_pmic *pmic = dev_get_drvdata(pdev->dev.parent);
+ struct i2c_board_info *board_info = NULL;
struct cht_wc_i2c_adap *adap;
- struct i2c_board_info board_info = {
- .type = "bq24190",
- .addr = 0x6b,
- .dev_name = "bq24190",
- .swnode = &bq24190_node,
- .platform_data = &bq24190_pdata,
- };
int ret, reg, irq;
irq = platform_get_irq(pdev, 0);
@@ -379,17 +456,24 @@ static int cht_wc_i2c_adap_i2c_probe(struct platform_device *pdev)
if (ret)
goto remove_irq_domain;
- /*
- * Normally the Whiskey Cove PMIC is paired with a TI bq24292i charger,
- * connected to this i2c bus, and a max17047 fuel-gauge and a fusb302
- * USB Type-C controller connected to another i2c bus. In this setup
- * the max17047 and fusb302 devices are enumerated through an INT33FE
- * ACPI device. If this device is present register an i2c-client for
- * the TI bq24292i charger.
- */
- if (acpi_dev_present("INT33FE", NULL, -1)) {
- board_info.irq = adap->client_irq;
- adap->client = i2c_new_client_device(&adap->adapter, &board_info);
+ switch (pmic->cht_wc_model) {
+ case INTEL_CHT_WC_GPD_WIN_POCKET:
+ board_info = &gpd_win_board_info;
+ break;
+ case INTEL_CHT_WC_XIAOMI_MIPAD2:
+ board_info = &xiaomi_mipad2_board_info;
+ break;
+ case INTEL_CHT_WC_LENOVO_YOGABOOK1:
+ board_info = &lenovo_yogabook1_board_info;
+ break;
+ default:
+ dev_warn(&pdev->dev, "Unknown model, not instantiating charger device\n");
+ break;
+ }
+
+ if (board_info) {
+ board_info->irq = adap->client_irq;
+ adap->client = i2c_new_client_device(&adap->adapter, board_info);
if (IS_ERR(adap->client)) {
ret = PTR_ERR(adap->client);
goto del_adapter;
--
2.33.1
On Mon, Jan 31, 2022 at 03:37:16PM +0200, Andy Shevchenko wrote:
> On Sun, Jan 30, 2022 at 09:45:38PM +0100, Hans de Goede wrote:
> > Some (USB) charger ICs have variants with USB D+ and D- pins to do their
> > own builtin charger-type detection, like e.g. the bq24190 and bq25890 and
> > also variants which lack this functionality, e.g. the bq24192 and bq25892.
> >
> > In case the charger-type; and thus the input-current-limit detection is
> > done outside the charger IC then we need some way to communicate this to
> > the charger IC. In the past extcon was used for this, but if the external
> > detection does e.g. full USB PD negotiation then the extcon cable-types do
> > not convey enough information.
> >
> > For these setups it was decided to model the external charging "brick"
> > and the parameters negotiated with it as a power_supply class-device
> > itself; and power_supply_set_input_current_limit_from_supplier() was
> > introduced to allow drivers to get the input-current-limit this way.
> >
> > But in some cases psy drivers may want to know other properties, e.g. the
> > bq25892 can do "quick-charge" negotiation by pulsing its current draw,
> > but this should only be done if the usb_type psy-property of its supplier
> > is set to DCP (and device-properties indicate the board allows higher
> > voltages).
> >
> > Instead of adding extra helper functions for each property which
> > a psy-driver wants to query from its supplier, refactor
> > power_supply_set_input_current_limit_from_supplier() into a
> > more generic power_supply_get_property_from_supplier() function.
>
> ...
>
> > + ret = power_supply_get_property_from_supplier(bdi->charger,
> > + POWER_SUPPLY_PROP_CURRENT_MAX,
> > + &val);
> > + if (ret == 0)
>
> Can it be as simple as
>
> if (ret)
> return;
>
> ...
>
>
> ?
>
> Or did I misunderstand the meaning of 0?
Despite on this comment being addressed or not, FWIW,
Reviewed-by: Andy Shevchenko <[email protected]>
I don't see any blocking issues with the bq25890 part neither, so
Reviewed-by: Andy Shevchenko <[email protected]>
for the BQ25890 part.
--
With Best Regards,
Andy Shevchenko
On Sun, Jan 30, 2022 at 09:45:38PM +0100, Hans de Goede wrote:
> Some (USB) charger ICs have variants with USB D+ and D- pins to do their
> own builtin charger-type detection, like e.g. the bq24190 and bq25890 and
> also variants which lack this functionality, e.g. the bq24192 and bq25892.
>
> In case the charger-type; and thus the input-current-limit detection is
> done outside the charger IC then we need some way to communicate this to
> the charger IC. In the past extcon was used for this, but if the external
> detection does e.g. full USB PD negotiation then the extcon cable-types do
> not convey enough information.
>
> For these setups it was decided to model the external charging "brick"
> and the parameters negotiated with it as a power_supply class-device
> itself; and power_supply_set_input_current_limit_from_supplier() was
> introduced to allow drivers to get the input-current-limit this way.
>
> But in some cases psy drivers may want to know other properties, e.g. the
> bq25892 can do "quick-charge" negotiation by pulsing its current draw,
> but this should only be done if the usb_type psy-property of its supplier
> is set to DCP (and device-properties indicate the board allows higher
> voltages).
>
> Instead of adding extra helper functions for each property which
> a psy-driver wants to query from its supplier, refactor
> power_supply_set_input_current_limit_from_supplier() into a
> more generic power_supply_get_property_from_supplier() function.
...
> + ret = power_supply_get_property_from_supplier(bdi->charger,
> + POWER_SUPPLY_PROP_CURRENT_MAX,
> + &val);
> + if (ret == 0)
Can it be as simple as
if (ret)
return;
...
?
Or did I misunderstand the meaning of 0?
--
With Best Regards,
Andy Shevchenko
On Sun, Jan 30, 2022 at 09:45:52PM +0100, Hans de Goede wrote:
> The i2c-controller on the Cherry Trail - Whiskey Cove PMIC is special
> in that it is always connected to the I2C charger IC of the board on
> which the PMIC is used; and the charger IC is not described in ACPI,
> so the i2c-cht-wc code needs to instantiate an i2c-client for it itself.
>
> So far this was hardcoded to instantiate an i2c-client for the
> bq24292i, with all properties, etc. set to match how this charger
> is used on the GPD win and GPD pocket devices.
>
> There is a rudimentary check to make sure the ACPI tables are at least
> somewhat as expected, but this is far from accurate, leading to
> a wrong i2c-client being instantiated for the charger on some boards.
>
> Switch to the new DMI based intel_cht_wc_get_model() helper which is
> exported by the MFD driver for the CHT Whiskey Cove PMIC to help PMIC
> cell drivers like the i2c-cht-wc code reliably detect which board
> they are running on.
>
> And add board_info for the charger ICs as found on the other 2 known
> boards with a Whisky Cove PMIC.
>
> This has been tested on all 3 known boards.
Reviewed-by: Andy Shevchenko <[email protected]>
> Acked-by: Wolfram Sang <[email protected]>
> Signed-off-by: Hans de Goede <[email protected]>
> ---
> Changes in v3:
> - Add Wolfram's Ack for taking this upstream through another tree
> then the i2c tree
> - Some minor tweaks / spelling fixes based on Andy's review
> ---
> drivers/i2c/busses/i2c-cht-wc.c | 120 +++++++++++++++++++++++++++-----
> 1 file changed, 102 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-cht-wc.c b/drivers/i2c/busses/i2c-cht-wc.c
> index 1cf68f85b2e1..54e909f9eab6 100644
> --- a/drivers/i2c/busses/i2c-cht-wc.c
> +++ b/drivers/i2c/busses/i2c-cht-wc.c
> @@ -18,6 +18,7 @@
> #include <linux/module.h>
> #include <linux/platform_device.h>
> #include <linux/power/bq24190_charger.h>
> +#include <linux/power/bq25890_charger.h>
> #include <linux/slab.h>
>
> #define CHT_WC_I2C_CTRL 0x5e24
> @@ -270,6 +271,7 @@ static const struct irq_chip cht_wc_i2c_irq_chip = {
> .name = "cht_wc_ext_chrg_irq_chip",
> };
>
> +/********** GPD Win / Pocket charger IC settings **********/
> static const char * const bq24190_suppliers[] = {
> "tcpm-source-psy-i2c-fusb302" };
>
> @@ -304,17 +306,92 @@ static struct bq24190_platform_data bq24190_pdata = {
> .regulator_init_data = &bq24190_vbus_init_data,
> };
>
> +static struct i2c_board_info gpd_win_board_info = {
> + .type = "bq24190",
> + .addr = 0x6b,
> + .dev_name = "bq24190",
> + .swnode = &bq24190_node,
> + .platform_data = &bq24190_pdata,
> +};
> +
> +/********** Xiaomi Mi Pad 2 charger IC settings **********/
> +static struct regulator_consumer_supply bq2589x_vbus_consumer = {
> + .supply = "vbus",
> + .dev_name = "cht_wcove_pwrsrc",
> +};
> +
> +static const struct regulator_init_data bq2589x_vbus_init_data = {
> + .constraints = {
> + .valid_ops_mask = REGULATOR_CHANGE_STATUS,
> + },
> + .consumer_supplies = &bq2589x_vbus_consumer,
> + .num_consumer_supplies = 1,
> +};
> +
> +static struct bq25890_platform_data bq2589x_pdata = {
> + .regulator_init_data = &bq2589x_vbus_init_data,
> +};
> +
> +static const struct property_entry xiaomi_mipad2_props[] = {
> + PROPERTY_ENTRY_BOOL("linux,skip-reset"),
> + PROPERTY_ENTRY_BOOL("linux,read-back-settings"),
> + { }
> +};
> +
> +static const struct software_node xiaomi_mipad2_node = {
> + .properties = xiaomi_mipad2_props,
> +};
> +
> +static struct i2c_board_info xiaomi_mipad2_board_info = {
> + .type = "bq25890",
> + .addr = 0x6a,
> + .dev_name = "bq25890",
> + .swnode = &xiaomi_mipad2_node,
> + .platform_data = &bq2589x_pdata,
> +};
> +
> +/********** Lenovo Yogabook YB1-X90F/-X91F/-X91L charger settings **********/
> +static const char * const lenovo_yb1_bq25892_suppliers[] = { "cht_wcove_pwrsrc" };
> +
> +static const struct property_entry lenovo_yb1_bq25892_props[] = {
> + PROPERTY_ENTRY_STRING_ARRAY("supplied-from",
> + lenovo_yb1_bq25892_suppliers),
> + PROPERTY_ENTRY_U32("linux,pump-express-vbus-max", 12000000),
> + PROPERTY_ENTRY_BOOL("linux,skip-reset"),
> + /*
> + * The firmware sets everything to the defaults, which leads to a
> + * somewhat low charge-current of 2048mA and worse to a battery-voltage
> + * of 4.2V instead of 4.35V (when booted without a charger connected).
> + * Use our own values instead of "linux,read-back-settings" to fix this.
> + */
> + PROPERTY_ENTRY_U32("ti,charge-current", 4224000),
> + PROPERTY_ENTRY_U32("ti,battery-regulation-voltage", 4352000),
> + PROPERTY_ENTRY_U32("ti,termination-current", 256000),
> + PROPERTY_ENTRY_U32("ti,precharge-current", 128000),
> + PROPERTY_ENTRY_U32("ti,minimum-sys-voltage", 3500000),
> + PROPERTY_ENTRY_U32("ti,boost-voltage", 4998000),
> + PROPERTY_ENTRY_U32("ti,boost-max-current", 1400000),
> + PROPERTY_ENTRY_BOOL("ti,use-ilim-pin"),
> + { }
> +};
> +
> +static const struct software_node lenovo_yb1_bq25892_node = {
> + .properties = lenovo_yb1_bq25892_props,
> +};
> +
> +static struct i2c_board_info lenovo_yogabook1_board_info = {
> + .type = "bq25892",
> + .addr = 0x6b,
> + .dev_name = "bq25892",
> + .swnode = &lenovo_yb1_bq25892_node,
> + .platform_data = &bq2589x_pdata,
> +};
> +
> static int cht_wc_i2c_adap_i2c_probe(struct platform_device *pdev)
> {
> struct intel_soc_pmic *pmic = dev_get_drvdata(pdev->dev.parent);
> + struct i2c_board_info *board_info = NULL;
> struct cht_wc_i2c_adap *adap;
> - struct i2c_board_info board_info = {
> - .type = "bq24190",
> - .addr = 0x6b,
> - .dev_name = "bq24190",
> - .swnode = &bq24190_node,
> - .platform_data = &bq24190_pdata,
> - };
> int ret, reg, irq;
>
> irq = platform_get_irq(pdev, 0);
> @@ -379,17 +456,24 @@ static int cht_wc_i2c_adap_i2c_probe(struct platform_device *pdev)
> if (ret)
> goto remove_irq_domain;
>
> - /*
> - * Normally the Whiskey Cove PMIC is paired with a TI bq24292i charger,
> - * connected to this i2c bus, and a max17047 fuel-gauge and a fusb302
> - * USB Type-C controller connected to another i2c bus. In this setup
> - * the max17047 and fusb302 devices are enumerated through an INT33FE
> - * ACPI device. If this device is present register an i2c-client for
> - * the TI bq24292i charger.
> - */
> - if (acpi_dev_present("INT33FE", NULL, -1)) {
> - board_info.irq = adap->client_irq;
> - adap->client = i2c_new_client_device(&adap->adapter, &board_info);
> + switch (pmic->cht_wc_model) {
> + case INTEL_CHT_WC_GPD_WIN_POCKET:
> + board_info = &gpd_win_board_info;
> + break;
> + case INTEL_CHT_WC_XIAOMI_MIPAD2:
> + board_info = &xiaomi_mipad2_board_info;
> + break;
> + case INTEL_CHT_WC_LENOVO_YOGABOOK1:
> + board_info = &lenovo_yogabook1_board_info;
> + break;
> + default:
> + dev_warn(&pdev->dev, "Unknown model, not instantiating charger device\n");
> + break;
> + }
> +
> + if (board_info) {
> + board_info->irq = adap->client_irq;
> + adap->client = i2c_new_client_device(&adap->adapter, board_info);
> if (IS_ERR(adap->client)) {
> ret = PTR_ERR(adap->client);
> goto del_adapter;
> --
> 2.33.1
>
--
With Best Regards,
Andy Shevchenko
On Sun, Jan 30, 2022 at 09:45:55PM +0100, Hans de Goede wrote:
> This is a preparation patch for adding support for registering
> a power_supply class device.
>
> Setting usbsrc to "CHT_WC_USBSRC_TYPE_SDP << CHT_WC_USBSRC_TYPE_SHIFT"
> will make the following switch-case return EXTCON_CHG_USB_SDP
> just as before, so there is no functional change.
...
> - return EXTCON_CHG_USB_SDP; /* Save fallback */
> + /* Save fallback */
I see it's in the previous code, but what does it mean?
I would read it as "Safe fallback", bit I have no clue.
--
With Best Regards,
Andy Shevchenko
On Sun, Jan 30, 2022 at 09:45:54PM +0100, Hans de Goede wrote:
> So far the extcon-intel-cht-wc code has only been tested on devices with
> a Type-C connector with USB-PD, USB3 (superspeed) and DP-altmode support
> through a FUSB302 Type-C controller.
>
> Some devices with the intel-cht-wc PMIC however come with an USB-micro-B
> connector, or an USB-2 only Type-C connector without USB-PD.
>
> Which device-model we are running on can be identified with the new
> cht_wc_model intel_soc_pmic field. On models without a Type-C controller
> the extcon code must control the Vbus 5V boost converter and the USB role
> switch depending on the detected cable-type.
...
> + if (ext->vbus_boost && ext->vbus_boost_enabled != enable) {
> + if (enable)
> + ret = regulator_enable(ext->vbus_boost);
> + else
> + ret = regulator_disable(ext->vbus_boost);
> + if (ret == 0)
> + ext->vbus_boost_enabled = enable;
> + else
> + dev_err(ext->dev, "Error updating Vbus boost regulator: %d\n", ret);
Can we go with
if (ret)
dev_err(ext->dev, "Error updating Vbus boost regulator: %d\n", ret);
else
ext->vbus_boost_enabled = enable;
?
> + }
--
With Best Regards,
Andy Shevchenko
Hi Andy,
Thank you for the reviews.
On 1/31/22 14:37, Andy Shevchenko wrote:
> On Sun, Jan 30, 2022 at 09:45:38PM +0100, Hans de Goede wrote:
>> Some (USB) charger ICs have variants with USB D+ and D- pins to do their
>> own builtin charger-type detection, like e.g. the bq24190 and bq25890 and
>> also variants which lack this functionality, e.g. the bq24192 and bq25892.
>>
>> In case the charger-type; and thus the input-current-limit detection is
>> done outside the charger IC then we need some way to communicate this to
>> the charger IC. In the past extcon was used for this, but if the external
>> detection does e.g. full USB PD negotiation then the extcon cable-types do
>> not convey enough information.
>>
>> For these setups it was decided to model the external charging "brick"
>> and the parameters negotiated with it as a power_supply class-device
>> itself; and power_supply_set_input_current_limit_from_supplier() was
>> introduced to allow drivers to get the input-current-limit this way.
>>
>> But in some cases psy drivers may want to know other properties, e.g. the
>> bq25892 can do "quick-charge" negotiation by pulsing its current draw,
>> but this should only be done if the usb_type psy-property of its supplier
>> is set to DCP (and device-properties indicate the board allows higher
>> voltages).
>>
>> Instead of adding extra helper functions for each property which
>> a psy-driver wants to query from its supplier, refactor
>> power_supply_set_input_current_limit_from_supplier() into a
>> more generic power_supply_get_property_from_supplier() function.
>
> ...
>
>> + ret = power_supply_get_property_from_supplier(bdi->charger,
>> + POWER_SUPPLY_PROP_CURRENT_MAX,
>> + &val);
>> + if (ret == 0)
>
> Can it be as simple as
>
> if (ret)
> return;
>
> ...
>
>
> ?
>
> Or did I misunderstand the meaning of 0?
Yes that would be better and together with some of the other tweaks
you've suggested I believe that this warrants a version 5. So I'll
address a v5 addressing all your comment.
Regards,
Hans
On Sun, Jan 30, 2022 at 09:45:49PM +0100, Hans de Goede wrote:
> From: Yauhen Kharuzhy <[email protected]>
>
> Add a "linux,pump-express-vbus-max" property which indicates if the Pump
> Express+ protocol should be used to increase the charging protocol.
>
> If this new property is set and a DCP charger is detected then request
> a higher charging voltage through the Pump Express+ protocol.
>
> So far this new property is only used on x86/ACPI (non devicetree) devs,
> IOW it is not used in actual devicetree files. The devicetree-bindings
> maintainers have requested properties like these to not be added to the
> devicetree-bindings, so the new property is deliberately not added
> to the existing devicetree-bindings.
>
> Changes by Hans de Goede:
> - Port to my bq25890 patch-series + various cleanups
> - Make behavior configurable through a new "linux,pump-express-vbus-max"
> device-property
> - Sleep 1 second before re-checking the Vbus voltage after requesting
> it to be raised, to ensure that the ADC has time to sampled the new Vbus
> - Add VBUSV bq25890_tables[] entry and use it in bq25890_get_vbus_voltage()
> - Tweak commit message
...
> +static void bq25890_pump_express_work(struct work_struct *data)
> +{
> + struct bq25890_device *bq =
> + container_of(data, struct bq25890_device, pump_express_work.work);
> + int voltage, i, ret;
> +
> + dev_dbg(bq->dev, "Start to request input voltage increasing\n");
> +
> + /* Enable current pulse voltage control protocol */
> + ret = bq25890_field_write(bq, F_PUMPX_EN, 1);
> + if (ret < 0)
> + goto error_print;
> +
> + for (i = 0; i < PUMP_EXPRESS_MAX_TRIES; i++) {
> + voltage = bq25890_get_vbus_voltage(bq);
> + if (voltage < 0)
> + goto error_print;
It also can be (at least in align with the rest error paths)
ret = bq25890_get_vbus_voltage(bq);
if (ret < 0)
goto error_print;
voltage = ret;
followed up (but not necessarily)...
> + dev_dbg(bq->dev, "input voltage = %d uV\n", voltage);
> +
> + if ((voltage + PUMP_EXPRESS_VBUS_MARGIN_uV) >
> + bq->pump_express_vbus_max)
> + break;
> +
> + ret = bq25890_field_write(bq, F_PUMPX_UP, 1);
> + if (ret < 0)
> + goto error_print;
> +
> + /* Note a single PUMPX up pulse-sequence takes 2.1s */
> + ret = regmap_field_read_poll_timeout(bq->rmap_fields[F_PUMPX_UP],
> + ret, !ret, 100000, 3000000);
> + if (ret < 0)
> + goto error_print;
> +
> + /* Make sure ADC has sampled Vbus before checking again */
> + msleep(1000);
> + }
> +
> + bq25890_field_write(bq, F_PUMPX_EN, 0);
> +
> + dev_info(bq->dev, "Hi-voltage charging requested, input voltage is %d mV\n",
> + voltage);
> + return;
> +error_print:
if (ret < 0)
But it's up to you.
> + dev_err(bq->dev, "Failed to request hi-voltage charging\n");
> +}
--
With Best Regards,
Andy Shevchenko
Hi,
On 1/31/22 14:48, Andy Shevchenko wrote:
> On Sun, Jan 30, 2022 at 09:45:49PM +0100, Hans de Goede wrote:
>> From: Yauhen Kharuzhy <[email protected]>
>>
>> Add a "linux,pump-express-vbus-max" property which indicates if the Pump
>> Express+ protocol should be used to increase the charging protocol.
>>
>> If this new property is set and a DCP charger is detected then request
>> a higher charging voltage through the Pump Express+ protocol.
>>
>> So far this new property is only used on x86/ACPI (non devicetree) devs,
>> IOW it is not used in actual devicetree files. The devicetree-bindings
>> maintainers have requested properties like these to not be added to the
>> devicetree-bindings, so the new property is deliberately not added
>> to the existing devicetree-bindings.
>>
>> Changes by Hans de Goede:
>> - Port to my bq25890 patch-series + various cleanups
>> - Make behavior configurable through a new "linux,pump-express-vbus-max"
>> device-property
>> - Sleep 1 second before re-checking the Vbus voltage after requesting
>> it to be raised, to ensure that the ADC has time to sampled the new Vbus
>> - Add VBUSV bq25890_tables[] entry and use it in bq25890_get_vbus_voltage()
>> - Tweak commit message
>
> ...
>
>> +static void bq25890_pump_express_work(struct work_struct *data)
>> +{
>> + struct bq25890_device *bq =
>> + container_of(data, struct bq25890_device, pump_express_work.work);
>> + int voltage, i, ret;
>> +
>> + dev_dbg(bq->dev, "Start to request input voltage increasing\n");
>> +
>> + /* Enable current pulse voltage control protocol */
>> + ret = bq25890_field_write(bq, F_PUMPX_EN, 1);
>> + if (ret < 0)
>> + goto error_print;
>> +
>> + for (i = 0; i < PUMP_EXPRESS_MAX_TRIES; i++) {
>
>> + voltage = bq25890_get_vbus_voltage(bq);
>> + if (voltage < 0)
>> + goto error_print;
>
> It also can be (at least in align with the rest error paths)
>
> ret = bq25890_get_vbus_voltage(bq);
> if (ret < 0)
> goto error_print;
> voltage = ret;
>
> followed up (but not necessarily)...
The suggested pattern is useful when ret needs to be set on the error-exit
path, but we are not doing that here. So I prefer to just keep this as is.
Regards,
Hans
>
>> + dev_dbg(bq->dev, "input voltage = %d uV\n", voltage);
>> +
>> + if ((voltage + PUMP_EXPRESS_VBUS_MARGIN_uV) >
>> + bq->pump_express_vbus_max)
>> + break;
>> +
>> + ret = bq25890_field_write(bq, F_PUMPX_UP, 1);
>> + if (ret < 0)
>> + goto error_print;
>> +
>> + /* Note a single PUMPX up pulse-sequence takes 2.1s */
>> + ret = regmap_field_read_poll_timeout(bq->rmap_fields[F_PUMPX_UP],
>> + ret, !ret, 100000, 3000000);
>> + if (ret < 0)
>> + goto error_print;
>> +
>> + /* Make sure ADC has sampled Vbus before checking again */
>> + msleep(1000);
>> + }
>> +
>> + bq25890_field_write(bq, F_PUMPX_EN, 0);
>> +
>> + dev_info(bq->dev, "Hi-voltage charging requested, input voltage is %d mV\n",
>> + voltage);
>
>> + return;
>> +error_print:
>
> if (ret < 0)
>
> But it's up to you.
>
>> + dev_err(bq->dev, "Failed to request hi-voltage charging\n");
>> +}
>
Hi,
On 1/31/22 14:56, Andy Shevchenko wrote:
> On Sun, Jan 30, 2022 at 09:45:55PM +0100, Hans de Goede wrote:
>> This is a preparation patch for adding support for registering
>> a power_supply class device.
>>
>> Setting usbsrc to "CHT_WC_USBSRC_TYPE_SDP << CHT_WC_USBSRC_TYPE_SHIFT"
>> will make the following switch-case return EXTCON_CHG_USB_SDP
>> just as before, so there is no functional change.
>
> ...
>
>> - return EXTCON_CHG_USB_SDP; /* Save fallback */
>
>> + /* Save fallback */
>
> I see it's in the previous code, but what does it mean?
> I would read it as "Safe fallback", bit I have no clue.
Ah yes that should be safe not save, sorry will fix for v5.
Regards,
Hans
Hi,
On 1/31/22 14:54, Andy Shevchenko wrote:
> On Sun, Jan 30, 2022 at 09:45:54PM +0100, Hans de Goede wrote:
>> So far the extcon-intel-cht-wc code has only been tested on devices with
>> a Type-C connector with USB-PD, USB3 (superspeed) and DP-altmode support
>> through a FUSB302 Type-C controller.
>>
>> Some devices with the intel-cht-wc PMIC however come with an USB-micro-B
>> connector, or an USB-2 only Type-C connector without USB-PD.
>>
>> Which device-model we are running on can be identified with the new
>> cht_wc_model intel_soc_pmic field. On models without a Type-C controller
>> the extcon code must control the Vbus 5V boost converter and the USB role
>> switch depending on the detected cable-type.
>
> ...
>
>> + if (ext->vbus_boost && ext->vbus_boost_enabled != enable) {
>> + if (enable)
>> + ret = regulator_enable(ext->vbus_boost);
>> + else
>> + ret = regulator_disable(ext->vbus_boost);
>
>> + if (ret == 0)
>> + ext->vbus_boost_enabled = enable;
>> + else
>> + dev_err(ext->dev, "Error updating Vbus boost regulator: %d\n", ret);
>
> Can we go with
>
> if (ret)
> dev_err(ext->dev, "Error updating Vbus boost regulator: %d\n", ret);
> else
> ext->vbus_boost_enabled = enable;
>
> ?
Ack, fixed for v5.
Regards,
Hans
On Mon, Jan 31, 2022 at 04:18:23PM +0100, Hans de Goede wrote:
> On 1/31/22 14:48, Andy Shevchenko wrote:
> > On Sun, Jan 30, 2022 at 09:45:49PM +0100, Hans de Goede wrote:
...
> >> + for (i = 0; i < PUMP_EXPRESS_MAX_TRIES; i++) {
> >
> >> + voltage = bq25890_get_vbus_voltage(bq);
> >> + if (voltage < 0)
> >> + goto error_print;
> >
> > It also can be (at least in align with the rest error paths)
> >
> > ret = bq25890_get_vbus_voltage(bq);
> > if (ret < 0)
> > goto error_print;
> > voltage = ret;
> >
> > followed up (but not necessarily)...
>
> The suggested pattern is useful when ret needs to be set on the error-exit
> path, but we are not doing that here. So I prefer to just keep this as is.
Are you talking about above proposal?
Still wouldn't be better to use it that if we want, for example, to print an
error code, it can be done easily? For the sake of consistency.
> >> + }
> >> +
> >> + bq25890_field_write(bq, F_PUMPX_EN, 0);
> >> +
> >> + dev_info(bq->dev, "Hi-voltage charging requested, input voltage is %d mV\n",
> >> + voltage);
> >
> >> + return;
> >> +error_print:
> >
> > if (ret < 0)
> >
> > But it's up to you.
> >
> >> + dev_err(bq->dev, "Failed to request hi-voltage charging\n");
--
With Best Regards,
Andy Shevchenko