| Message ID | 20211114170335.66994-14-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | power-suppy/i2c/extcon: Fix charger setup on Xiaomi Mi Pad 2 and Lenovo Yogabook | expand |
On Sun, Nov 14, 2021 at 7:04 PM Hans de Goede <hdegoede@redhat.com> wrote: > > From: Yauhen Kharuzhy <jekhor@gmail.com> > > 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 ... > +#define PUMP_EXPRESS_START_DELAY (5 * HZ) > +#define PUMP_EXPRESS_MAX_TRIES 6 > +#define PUMP_EXPRESS_VBUS_MARGIN 1000000 Units? Perhaps "_uV"? ... > + dev_dbg(bq->dev, "input voltage = %d mV\n", voltage); Just to be sure, is it indeed "mV" and not "uV"? ... > + while (bq25890_field_read(bq, F_PUMPX_UP) == 1) > + msleep(100); Infinite loop? Sounds like a good candidate to switch to read_poll_timeout() // note it accepts any type of (op) with a variadic number of args. ... > +error: error_print: ? > + dev_err(bq->dev, "Failed to request hi-voltage charging\n"); ... > + ret = device_property_read_u32(bq->dev, "linux,pump-express-vbus-max", > + &bq->pump_express_vbus_max); > + if (ret < 0) > + bq->pump_express_vbus_max = 0; Isn't it 0 by default? Anyway, for all optional properties one may use bq->...property... = $default; device_property_read_u32(bq->dev, "linux,...property...", &bq->...property...); I.e. no conditional needed.
Hi, On 11/16/21 12:14, Andy Shevchenko wrote: > On Sun, Nov 14, 2021 at 7:04 PM Hans de Goede <hdegoede@redhat.com> wrote: >> >> From: Yauhen Kharuzhy <jekhor@gmail.com> >> >> 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 > > ... > >> +#define PUMP_EXPRESS_START_DELAY (5 * HZ) >> +#define PUMP_EXPRESS_MAX_TRIES 6 >> +#define PUMP_EXPRESS_VBUS_MARGIN 1000000 > > Units? Perhaps "_uV"? > > ... > >> + dev_dbg(bq->dev, "input voltage = %d mV\n", voltage); > > Just to be sure, is it indeed "mV" and not "uV"? It is uV, will fix for the next version. > > ... > >> + while (bq25890_field_read(bq, F_PUMPX_UP) == 1) >> + msleep(100); > > Infinite loop? > > Sounds like a good candidate to switch to read_poll_timeout() // note> it accepts any type of (op) with a variadic number of args. Good catch, will fix. > > ... > >> +error: > > error_print: ? > >> + dev_err(bq->dev, "Failed to request hi-voltage charging\n"); > > ... > >> + ret = device_property_read_u32(bq->dev, "linux,pump-express-vbus-max", >> + &bq->pump_express_vbus_max); >> + if (ret < 0) >> + bq->pump_express_vbus_max = 0; > > Isn't it 0 by default? > > Anyway, for all optional properties one may use > > bq->...property... = $default; > device_property_read_u32(bq->dev, "linux,...property...", &bq->...property...); > > I.e. no conditional needed. Ack, will fix. Regards, Hans
diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c index 57e4034bc9cd..1c59d4f71389 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 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, @@ -308,6 +315,7 @@ static const union { [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 */ @@ -410,6 +418,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) @@ -583,6 +602,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: @@ -847,6 +871,50 @@ 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; + + for (i = 0; i < PUMP_EXPRESS_MAX_TRIES; i++) { + voltage = bq25890_get_vbus_voltage(bq); + if (voltage < 0) + goto error; + dev_dbg(bq->dev, "input voltage = %d mV\n", voltage); + + if ((voltage + PUMP_EXPRESS_VBUS_MARGIN) > + bq->pump_express_vbus_max) + break; + + ret = bq25890_field_write(bq, F_PUMPX_UP, 1); + if (ret < 0) + goto error; + + while (bq25890_field_read(bq, F_PUMPX_UP) == 1) + msleep(100); + + /* 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: + dev_err(bq->dev, "Failed to request hi-voltage charging\n"); +} + static void bq25890_usb_work(struct work_struct *data) { int ret; @@ -1037,6 +1105,11 @@ static int bq25890_fw_probe(struct bq25890_device *bq) int ret; struct bq25890_init_data *init = &bq->init_data; + ret = device_property_read_u32(bq->dev, "linux,pump-express-vbus-max", + &bq->pump_express_vbus_max); + if (ret < 0) + bq->pump_express_vbus_max = 0; + 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"); @@ -1069,6 +1142,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))