[RFC v2 0/2] ACPI/power-suppy add fuel-gauge support on cht-wc PMIC without USB-PD support devs

[RFC v2 0/2] ACPI/power-suppy add fuel-gauge support on cht-wc PMIC without USB-PD support devs

[Hans de Goede]

Hi All,

Here is a new RFC to add battery monitoring support on the Xiaomi Mi Pad 2
this implements option 2 of the different options which I gave in v1:
https://lore.kernel.org/platform-driver-x86/20211031162428.22368-1-hdegoede@redhat.com/

To solve the challenges of getting this to work on this device, as
you can see compared to option 1, the first patch has gone from
adding 3 separate quirks to adding only one; and the 2 gpiolib-acpi
and 2 bq27xxx_battery driver patches are now no longer necessary.

These are now all 4 replaced by a new special xiaomi-mipad2 module
under drivers/platform/x86 which uses a DMI modalias to autoload
and thus will only be auto-loaded on the actual Mi Pad 2.

I like this option much better then the previous option, but this
still a RFC for now, since I would like to also see what is necessary
to get everything PMIC / micro-USB connector related to work on the
Yoga Book YB1-X91L which is on its way to me.

Regards,

Hans


p.s.

While working on this I realized that there also is a 4th option,
which is basically option 1 from the v1 RFC minus the 2 gpiolib-acpi
patches.

With the 2nd option (as implemented by this RFC) we leave the
_AEI handler in place and run the fuel-gauge without interrupt,
we can do the same when marking the fuel-gauge as always present
by treating IRQs on ACPI devices the same way as in the
max17042_battery code, which has already solved the IRQ problem
without disabling the _AEI handler:

		/*
		 * On ACPI systems the IRQ may be handled by ACPI-event code,
		 * so we need to share (if the ACPI code is willing to share).
		 */
		if (acpi_id)
			flags |= IRQF_SHARED | IRQF_PROBE_SHARED;

This is a pretty decent option too, it requires:

1. 2 more always_present quirks in the ACPI scan code which is part of
the main kernel image.

2. Patches to the bq27xxx_battery code to support ACPI enumeration.




Hans de Goede (2):
  ACPI / x86: Add PWM2 on the Xiaomi Mi Pad 2 to the always_present list
  platform/x86: xiaomi-mipad2: New driver for Xiaomi Mi Pad 2 tablets

 drivers/acpi/x86/utils.c             |   5 ++
 drivers/platform/x86/Kconfig         |  13 +++
 drivers/platform/x86/Makefile        |   1 +
 drivers/platform/x86/xiaomi-mipad2.c | 126 +++++++++++++++++++++++++++
 4 files changed, 145 insertions(+)
 create mode 100644 drivers/platform/x86/xiaomi-mipad2.c

-- 
2.31.1

[RFC v2 1/2] ACPI / x86: Add PWM2 on the Xiaomi Mi Pad 2 to the always_present list

[Hans de Goede]

The Xiaomi Mi Pad 2 has backlit LEDs behind the capacitive menu, home
and back buttons below the screen which are controlled by the PWM2
controller of the CHT SoC. This PWM2 controller gets hidden by the
firmware, add it to the always_present_ids table so that we can use
the PWM controller to control the backlighting of the buttons.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/x86/utils.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/acpi/x86/utils.c b/drivers/acpi/x86/utils.c
index f22f23933063..cb988f9b23a1 100644
--- a/drivers/acpi/x86/utils.c
+++ b/drivers/acpi/x86/utils.c
@@ -58,6 +58,11 @@ static const struct always_present_id always_present_ids[] = {
 	ENTRY("80862289", "2", X86_MATCH(ATOM_AIRMONT), {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Lenovo YB1-X9"),
 		}),
+	/* The Xiaomi Mi Pad 2 uses PWM2 for touchkeys backlight control */
+	ENTRY("80862289", "2", X86_MATCH(ATOM_AIRMONT), {
+		DMI_MATCH(DMI_SYS_VENDOR, "Xiaomi Inc"),
+		DMI_MATCH(DMI_PRODUCT_NAME, "Mipad2"),
+	      }),
 	/*
 	 * The INT0002 device is necessary to clear wakeup interrupt sources
 	 * on Cherry Trail devices, without it we get nobody cared IRQ msgs.
-- 
2.31.1

Re: [RFC v2 1/2] ACPI / x86: Add PWM2 on the Xiaomi Mi Pad 2 to the always_present list

[Rafael J. Wysocki]

On Tue, Nov 2, 2021 at 11:40 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> The Xiaomi Mi Pad 2 has backlit LEDs behind the capacitive menu, home
> and back buttons below the screen which are controlled by the PWM2
> controller of the CHT SoC. This PWM2 controller gets hidden by the
> firmware, add it to the always_present_ids table so that we can use
> the PWM controller to control the backlighting of the buttons.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  drivers/acpi/x86/utils.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/acpi/x86/utils.c b/drivers/acpi/x86/utils.c
> index f22f23933063..cb988f9b23a1 100644
> --- a/drivers/acpi/x86/utils.c
> +++ b/drivers/acpi/x86/utils.c
> @@ -58,6 +58,11 @@ static const struct always_present_id always_present_ids[] = {
>         ENTRY("80862289", "2", X86_MATCH(ATOM_AIRMONT), {
>                         DMI_MATCH(DMI_PRODUCT_NAME, "Lenovo YB1-X9"),
>                 }),
> +       /* The Xiaomi Mi Pad 2 uses PWM2 for touchkeys backlight control */
> +       ENTRY("80862289", "2", X86_MATCH(ATOM_AIRMONT), {
> +               DMI_MATCH(DMI_SYS_VENDOR, "Xiaomi Inc"),
> +               DMI_MATCH(DMI_PRODUCT_NAME, "Mipad2"),
> +             }),
>         /*
>          * The INT0002 device is necessary to clear wakeup interrupt sources
>          * on Cherry Trail devices, without it we get nobody cared IRQ msgs.
> --
> 2.31.1
>

[RFC v2 2/2] platform/x86: xiaomi-mipad2: New driver for Xiaomi Mi Pad 2 tablets

[Hans de Goede]

The ACPI tables on the Xiaomi Mi Pad 2 hide a number of devices from Linux,
such as the fuel-gauge IC used for battery monitoring.

This adds a xiaomi-mipad2 "board-file" which manually instantiates
device-objects for the missing devices.

When build as a module this will only ever get auto-loaded on Xiaomi
Mi Pad 2 tablets, allowing us to support these tablets in generic X86
kernels at the cost of just a small amount of diskspace for the module.

As discussed previously this is the least ugly option to get these
devices to fully work and to do so without adding any extra code to
the main kernel image (vmlinuz) when built as a module.

Link: https://lore.kernel.org/platform-driver-x86/20211031162428.22368-1-hdegoede@redhat.com/
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/Kconfig         |  13 +++
 drivers/platform/x86/Makefile        |   1 +
 drivers/platform/x86/xiaomi-mipad2.c | 126 +++++++++++++++++++++++++++
 3 files changed, 140 insertions(+)
 create mode 100644 drivers/platform/x86/xiaomi-mipad2.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 9cb8d33cc6e1..df8101fdfc6a 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -958,6 +958,19 @@ config TOPSTAR_LAPTOP
 
 	  If you have a Topstar laptop, say Y or M here.
 
+config XIAOMI_MIPAD2
+	tristate "Xiaomi Mi Pad 2 support"
+	depends on I2C && ACPI
+	help
+	  The ACPI tables on the Xiaomi Mi Pad 2 hide a number of devices
+	  from Linux, such as the fuel-gauge IC used for battery monitoring.
+
+	  Say Y or M here to enable building a xiaomi-mipad2 board-file which
+	  manually instantiates device-objects for the missing devices.
+
+	  If you have a Xiaomi Mi Pad 2 say Y or M here, for a generic X86
+	  distro config say M here.
+
 config I2C_MULTI_INSTANTIATE
 	tristate "I2C multi instantiate pseudo device driver"
 	depends on I2C && ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 3f610332b556..631f1b948cd0 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -105,6 +105,7 @@ obj-$(CONFIG_PANASONIC_LAPTOP)	+= panasonic-laptop.o
 obj-$(CONFIG_SONY_LAPTOP)	+= sony-laptop.o
 obj-$(CONFIG_SYSTEM76_ACPI)	+= system76_acpi.o
 obj-$(CONFIG_TOPSTAR_LAPTOP)	+= topstar-laptop.o
+obj-$(CONFIG_XIAOMI_MIPAD2)	+= xiaomi-mipad2.o
 
 # Platform drivers
 obj-$(CONFIG_DMI_DEVICE_PROPERTIES)	+= dmi_device_properties.o
diff --git a/drivers/platform/x86/xiaomi-mipad2.c b/drivers/platform/x86/xiaomi-mipad2.c
new file mode 100644
index 000000000000..bf333aaa0edf
--- /dev/null
+++ b/drivers/platform/x86/xiaomi-mipad2.c
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * On the Xiaomi Mi Pad 2 X86 tablet a bunch of devices are hidden when
+ * the EFI if the tablet does thinks the OS it is booting is Windows
+ * (OSID in the DSDT is set to 1); and the EFI code thinks this as soon
+ * as the EFI bootloader is not Xiaomi's own signed Android loader :|
+ *
+ * This "board-file" takes care of instantiating the hidden devices manually.
+ *
+ * Copyright (C) 2021 Hans de Goede <hdegoede@redhat.com>
+ */
+
+#define pr_fmt(fmt) "xiaomi-mipad2: " fmt
+
+#include <linux/acpi.h>
+#include <linux/dmi.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+
+/********** BQ27520 fuel-gauge info **********/
+#define BQ27520_ADAPTER "\\_SB_.PCI0.I2C1"
+
+static const char * const bq27520_suppliers[] = { "bq25890-charger" };
+
+static const struct property_entry bq27520_props[] = {
+	PROPERTY_ENTRY_STRING_ARRAY("supplied-from", bq27520_suppliers),
+	{ }
+};
+
+static const struct software_node bq27520_node = {
+	.properties = bq27520_props,
+};
+
+static const struct i2c_board_info bq27520_board_info = {
+	.type = "bq27520",
+	.addr = 0x55,
+	.dev_name = "bq27520",
+	.swnode = &bq27520_node,
+};
+
+static struct i2c_client *bq27520_client;
+
+/********** KTD2026 RGB notification LED controller **********/
+#define KTD2026_ADAPTER "\\_SB_.PCI0.I2C3"
+
+static const struct i2c_board_info ktd2026_board_info = {
+	.type = "ktd2026",
+	.addr = 0x30,
+	.dev_name = "ktd2026",
+};
+
+static struct i2c_client *ktd2026_client;
+
+/********** DMI-match, probe(), etc. **********/
+static const struct dmi_system_id xiaomi_mipad2_ids[] __initconst = {
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Xiaomi Inc"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Mipad2"),
+		},
+	},
+	{} /* Terminating entry */
+};
+MODULE_DEVICE_TABLE(dmi, xiaomi_mipad2_ids);
+
+static __init struct i2c_client *xiaomi_instantiate_i2c_client(
+				char *adapter_path,
+				const struct i2c_board_info *board_info)
+{
+	struct i2c_client *client;
+	struct i2c_adapter *adap;
+	acpi_handle handle;
+	acpi_status status;
+
+	status = acpi_get_handle(NULL, adapter_path, &handle);
+	if (ACPI_FAILURE(status)) {
+		pr_err("Error could not get %s handle\n", adapter_path);
+		return ERR_PTR(-ENODEV);
+	}
+
+	adap = i2c_acpi_find_adapter_by_handle(handle);
+	if (!adap) {
+		pr_err("Error could not get %s adapter\n", adapter_path);
+		return ERR_PTR(-ENODEV);
+	}
+
+	client = i2c_new_client_device(adap, board_info);
+
+	put_device(&adap->dev);
+
+	return client;
+}
+
+static __init int xiaomi_mipad2_init(void)
+{
+	if (!dmi_first_match(xiaomi_mipad2_ids))
+		return -ENODEV;
+
+	bq27520_client = xiaomi_instantiate_i2c_client(BQ27520_ADAPTER,
+						       &bq27520_board_info);
+	if (IS_ERR(bq27520_client))
+		return PTR_ERR(bq27520_client);
+
+	ktd2026_client = xiaomi_instantiate_i2c_client(KTD2026_ADAPTER,
+						       &ktd2026_board_info);
+	if (IS_ERR(ktd2026_client)) {
+		i2c_unregister_device(bq27520_client);
+		return PTR_ERR(ktd2026_client);
+	}
+
+	return 0;
+}
+
+static __exit void xiaomi_mipad2_cleanup(void)
+{
+	i2c_unregister_device(ktd2026_client);
+	i2c_unregister_device(bq27520_client);
+}
+
+module_init(xiaomi_mipad2_init);
+module_exit(xiaomi_mipad2_cleanup);
+
+MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com");
+MODULE_DESCRIPTION("Xiaomi Mi Pad 2 board-file");
+MODULE_LICENSE("GPL");
-- 
2.31.1

Re: [RFC v2 0/2] ACPI/power-suppy add fuel-gauge support on cht-wc PMIC without USB-PD support devs

[Andy Shevchenko]

On Wed, Nov 3, 2021 at 12:40 AM Hans de Goede <hdegoede@redhat.com> wrote:

...

> While working on this I realized that there also is a 4th option,
> which is basically option 1 from the v1 RFC minus the 2 gpiolib-acpi
> patches.
>
> With the 2nd option (as implemented by this RFC) we leave the
> _AEI handler in place and run the fuel-gauge without interrupt,
> we can do the same when marking the fuel-gauge as always present
> by treating IRQs on ACPI devices the same way as in the
> max17042_battery code, which has already solved the IRQ problem
> without disabling the _AEI handler:
>
>                 /*
>                  * On ACPI systems the IRQ may be handled by ACPI-event code,
>                  * so we need to share (if the ACPI code is willing to share).
>                  */

>                 if (acpi_id)

(Side remark: wouldn't it be better to have has_acpi_companion() and
use device_get_match_data()?)

>                         flags |= IRQF_SHARED | IRQF_PROBE_SHARED;
>
> This is a pretty decent option too, it requires:
>
> 1. 2 more always_present quirks in the ACPI scan code which is part of
> the main kernel image.
>
> 2. Patches to the bq27xxx_battery code to support ACPI enumeration.

If it works, why not try it?

I like the common base for the FG drivers that can be used as a pattern then.


-- 
With Best Regards,
Andy Shevchenko

Re: [RFC v2 0/2] ACPI/power-suppy add fuel-gauge support on cht-wc PMIC without USB-PD support devs

[Hans de Goede]

Hi,

On 11/3/21 10:18, Andy Shevchenko wrote:
> On Wed, Nov 3, 2021 at 12:40 AM Hans de Goede <hdegoede@redhat.com> wrote:
> 
> ...
> 
>> While working on this I realized that there also is a 4th option,
>> which is basically option 1 from the v1 RFC minus the 2 gpiolib-acpi
>> patches.
>>
>> With the 2nd option (as implemented by this RFC) we leave the
>> _AEI handler in place and run the fuel-gauge without interrupt,
>> we can do the same when marking the fuel-gauge as always present
>> by treating IRQs on ACPI devices the same way as in the
>> max17042_battery code, which has already solved the IRQ problem
>> without disabling the _AEI handler:
>>
>>                 /*
>>                  * On ACPI systems the IRQ may be handled by ACPI-event code,
>>                  * so we need to share (if the ACPI code is willing to share).
>>                  */
> 
>>                 if (acpi_id)
>>                         flags |= IRQF_SHARED | IRQF_PROBE_SHARED;
>>
>> This is a pretty decent option too, it requires:
>>
>> 1. 2 more always_present quirks in the ACPI scan code which is part of
>> the main kernel image.
>>
>> 2. Patches to the bq27xxx_battery code to support ACPI enumeration.
> 
> If it works, why not try it?

I'm 99.9% sure it will work, so I see no reason to actually try it unless
we decide that this is a better option.

This option would basically be patches 1 + 4-5 from option 1 / the v1 RFC:
https://lore.kernel.org/platform-driver-x86/20211031162428.22368-1-hdegoede@redhat.com/

plus extra changes to the bq27xxx_battery code IRQ handling like above.

So this would mean 2 extra quirks in drivers/apci/x86/utils.c vs this
RFC v2 patches + various changes to the bq27xxx_battery code, where as
this (option 2/RFC v2) approach requires no changes to the bq27xxx_battery
code at all (1).

So at the cost of a board_file (patch 2/2 of this RFC v2 series) we save 2 quirks
which would be in the main vmlinuz image for everyone, as well as not having
to add any special code to the bq27xxx_battery code just for this one board;
and as mentioned the board-file .ko will only ever auto-load on the actual
board, so it just costs diskspace when enabled in generic distro kernels,
where as e.g. the 2 quirks take op memory for everyone.

So all in all I still believe that the approach in this v2 RFC is the best.

Also note that we already have an i2c-client being instantiated for a
bq27xxx_battery fuel-gauge (rather then forcing an ACPI-device as always-
present) here:

drivers/platform/x86/intel/int33fe/intel_cht_int33fe_microb.c

and for the max17042 fg here:

drivers/platform/x86/intel/int33fe/intel_cht_int33fe_typec.c

So doing the manual instantiation already is somewhat of a pattern
and IMHO is the best (least bad) option to deal with this.

> I like the common base for the FG drivers that can be used as a pattern then.

I don't believe this is a pattern which we want to introduce, with the
exception of one buggy BIOS version (2) fuel-gauges on ACPI devices
are never directly instantiated through an ACPI-device / fwnode.

The whole business of using native fuel-gauge drivers on X86/ACPI is
a special case just for BYT/CHT devices to begin with (because of
broken or outright missing ACPI battery class support) and on all but
the broken BIOS version the platform_dev or i2c-client for the
fuel-gauge is instantiated by specific kernel-code, rather then
directly from ACPI tables. So having ACPI enumeration support
in fuel-gauge drivers is something which we do not need and
should not want.

(Arguably we also need an always_hidden option for the ACPI quirks
and then use that to hide the anomaly of the MAX17042 ACPI device
not being hidden on the one troublesome BIOS version, this would
allow removing a nice amount of code in a bunch of places.)

Regards,

Hans



1) Well I notices some issues there which need fixes but those issues impact
all users of the bq27xxx_battery code and are not related to the discussed
enumeration issues

2) BIOS version, not board, other BIOS versions for the same board are fine