diff mbox series

[v2,13/20] power: supply: bq25890: Support higher charging voltages through Pump Express+ protocol

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

Commit Message

Hans de Goede Nov. 14, 2021, 5:03 p.m. UTC
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

Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/bq25890_charger.c | 74 ++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

Comments

Andy Shevchenko Nov. 16, 2021, 11:14 a.m. UTC | #1
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.
Hans de Goede Nov. 16, 2021, 11:40 a.m. UTC | #2
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 mbox series

Patch

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))