2018-02-16 18:50:41

by Carlo Caione

[permalink] [raw]
Subject: [PATCH v2 0/3] power: supply: Fix AXP288 fallback when not needed

From: Carlo Caione <[email protected]>

With commits af3ec837 and dccfae6d a blacklist was introduced to avoid
using the ACPI drivers for AC and battery when a native PMIC driver was
already present. While this is in general a good idea (because of broken
DSDT or proprietary and undocumented ACPI opregions for the ACPI
AC/battery devices) we have come across at least one CherryTrail laptop
(ECS EF20EA) shipping the AXP288 together with a separate FG controller
(a MAX17047) instead of the one embedded in the AXP288.

This is the interesting analisys done by Hans de Goede (thank you):

Looking at the _BIX method of the BATC/PNP0C0A device, we see it referencing
FG10:

Method (_BIX, 0, NotSerialized) // _BIX: Battery Information Extend
{
If (AVBL == One)
{
BUF2 = FG10 /* \_SB_.PCI0.I2C1.FG10 */

And FG10 is defined as:

Field (DVFG, BufferAcc, NoLock, Preserve)
{
Connection (SMFG),
Offset (0x10),
AccessAs (BufferAcc, AttribBytes (0x02)),
FG10, 8
}

With SMFG being defined as:

Name (SMFG, ResourceTemplate ()
{
I2cSerialBusV2 (0x0036, ControllerInitiated, 0x000186A0,
AddressingMode7Bit, "\\_SB.PCI0.I2C1",
0x00, ResourceConsumer, , Exclusive,
)
})

Looking for I2C1 address 0x0036 we find:

Device (ANFG)
{
Name (_HID, "MAX17047" /* Fuel Gauge Controller */) // _HID: Hardwa
Name (_CID, "MAX17047" /* Fuel Gauge Controller */) // _CID: Compat
Name (_DDN, "Fuel Gauge Controller") // _DDN: DOS Device Name
Name (RBUF, ResourceTemplate ()
{
I2cSerialBusV2 (0x0036, ControllerInitiated, 0x00061A80,
AddressingMode7Bit, "\\_SB.PCI0.I2C1",
0x00, ResourceConsumer, , Exclusive,
)
GpioInt (Level, ActiveLow, ExclusiveAndWake, PullNone, 0x0000,
"\\_SB.GPO3", 0x00, ResourceConsumer, ,
)
{ // Pin list
0x0001
}
})

Where as the AXP288 PMIC is I2C7 address 0x034:

Device (PMI1)
{
Name (_ADR, Zero) // _ADR: Address
Name (_HID, "INT33F4" /* XPOWER PMIC Controller */) // _HID: Ha
Name (_CID, "INT33F4" /* XPOWER PMIC Controller */) // _CID: Co
Name (_DDN, "XPOWER PMIC Controller") // _DDN: DOS Device Name
Name (_HRV, 0x03) // _HRV: Hardware Revision
Name (_UID, One) // _UID: Unique ID

Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Setti
{
Name (SBUF, ResourceTemplate ()
{
I2cSerialBusV2 (0x0034, ControllerInitiated, 0x000F4240,
AddressingMode7Bit, "\\_SB.PCI0.I2C7",
0x00, ResourceConsumer, , Exclusive,
)

So basically this laptopt is using a separate FG chip instead of the one
embedded in the AXP288.

To have this correctly working we need basically to avoid the fallback on the
AXP288 driver enabling again the ACPI AC/battery drivers and at the same time
avoiding that the AXP288 FG driver is probed at all.

I'm still not fully convinced that having two different quirks (one to disable
the blacklist and another to disable the AXP288 FG probing) is the right way to
fix this. So any comment is welcome.

Changelog:

v2:
- Split [PATCH 2/2] in two parts
- Rework subject prefixes

Carlo Caione (3):
ACPI: AC/battery: Add quirk to avoid using PMIC
ACPI: AC/battery: Add quirks for ECS EF20EA
power: supply: axp288_fuel_gauge: Do not register FG on ECS EF20EA

drivers/acpi/ac.c | 33 ++++++++++++++++++++++++--------
drivers/acpi/battery.c | 33 ++++++++++++++++++++++++--------
drivers/power/supply/axp288_fuel_gauge.c | 6 ++++++
3 files changed, 56 insertions(+), 16 deletions(-)

--
2.14.1



2018-02-16 18:50:43

by Carlo Caione

[permalink] [raw]
Subject: [PATCH v2 1/3] ACPI: AC/battery: Add quirk to avoid using PMIC

From: Carlo Caione <[email protected]>

With commits af3ec837 and dccfae6d a blacklist was introduced to avoid
using the ACPI drivers for AC and battery when a native PMIC driver was
already present. While this is in general a good idea (because of broken
DSDT or proprietary and undocumented ACPI opregions for the ACPI
AC/battery devices) we have come across at least one CherryTrail laptop
(ECS EF20EA) shipping the AXP288 together with a separate FG controller
(a MAX17047) instead of the one embedded in the AXP288.

The net effect of blacklisting the ACPI drivers is that on this platform
the AC/battery reporting is broken since the information is coming from
the AXP288 FG driver, not actually used in hardware.

In this case we want to keep using the ACPI AC/battery driver that is
able to interface correctly with the real FG controller.

We introduce therefore a new quirk to avoid the blacklist.

Signed-off-by: Carlo Caione <[email protected]>
---
drivers/acpi/ac.c | 26 ++++++++++++++++++--------
drivers/acpi/battery.c | 26 ++++++++++++++++++--------
2 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index 47a7ed557bd6..b9a4ca720309 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -87,6 +87,7 @@ static int acpi_ac_open_fs(struct inode *inode, struct file *file);


static int ac_sleep_before_get_state_ms;
+static bool ac_not_use_pmic;

static struct acpi_driver acpi_ac_driver = {
.name = "ac",
@@ -316,6 +317,12 @@ static int thinkpad_e530_quirk(const struct dmi_system_id *d)
return 0;
}

+static int ac_not_use_pmic_quirk(const struct dmi_system_id *d)
+{
+ ac_not_use_pmic = true;
+ return 0;
+}
+
static const struct dmi_system_id ac_dmi_table[] = {
{
.callback = thinkpad_e530_quirk,
@@ -384,7 +391,6 @@ static int acpi_ac_add(struct acpi_device *device)
kfree(ac);
}

- dmi_check_system(ac_dmi_table);
return result;
}

@@ -442,13 +448,17 @@ static int __init acpi_ac_init(void)
if (acpi_disabled)
return -ENODEV;

- for (i = 0; i < ARRAY_SIZE(acpi_ac_blacklist); i++)
- if (acpi_dev_present(acpi_ac_blacklist[i].hid, "1",
- acpi_ac_blacklist[i].hrv)) {
- pr_info(PREFIX "AC: found native %s PMIC, not loading\n",
- acpi_ac_blacklist[i].hid);
- return -ENODEV;
- }
+ dmi_check_system(ac_dmi_table);
+
+ if (!ac_not_use_pmic) {
+ for (i = 0; i < ARRAY_SIZE(acpi_ac_blacklist); i++)
+ if (acpi_dev_present(acpi_ac_blacklist[i].hid, "1",
+ acpi_ac_blacklist[i].hrv)) {
+ pr_info(PREFIX "AC: found native %s PMIC, not loading\n",
+ acpi_ac_blacklist[i].hid);
+ return -ENODEV;
+ }
+ }

#ifdef CONFIG_ACPI_PROCFS_POWER
acpi_ac_dir = acpi_lock_ac_dir();
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 7128488a3a72..258d2e66f230 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -71,6 +71,7 @@ static bool battery_driver_registered;
static int battery_bix_broken_package;
static int battery_notification_delay_ms;
static int battery_full_discharging;
+static bool battery_not_use_pmic;
static unsigned int cache_time = 1000;
module_param(cache_time, uint, 0644);
MODULE_PARM_DESC(cache_time, "cache time in milliseconds");
@@ -1176,6 +1177,13 @@ static int __init battery_full_discharging_quirk(const struct dmi_system_id *d)
return 0;
}

+static int __init
+battery_not_use_pmic_quirk(const struct dmi_system_id *d)
+{
+ battery_not_use_pmic = true;
+ return 0;
+}
+
static const struct dmi_system_id bat_dmi_table[] __initconst = {
{
.callback = battery_bix_broken_package_quirk,
@@ -1364,16 +1372,18 @@ static void __init acpi_battery_init_async(void *unused, async_cookie_t cookie)
unsigned int i;
int result;

- for (i = 0; i < ARRAY_SIZE(acpi_battery_blacklist); i++)
- if (acpi_dev_present(acpi_battery_blacklist[i], "1", -1)) {
- pr_info(PREFIX ACPI_BATTERY_DEVICE_NAME
- ": found native %s PMIC, not loading\n",
- acpi_battery_blacklist[i]);
- return;
- }
-
dmi_check_system(bat_dmi_table);

+ if (!battery_not_use_pmic) {
+ for (i = 0; i < ARRAY_SIZE(acpi_battery_blacklist); i++)
+ if (acpi_dev_present(acpi_battery_blacklist[i], "1", -1)) {
+ pr_info(PREFIX ACPI_BATTERY_DEVICE_NAME
+ ": found native %s PMIC, not loading\n",
+ acpi_battery_blacklist[i]);
+ return;
+ }
+ }
+
#ifdef CONFIG_ACPI_PROCFS_POWER
acpi_battery_dir = acpi_lock_battery_dir();
if (!acpi_battery_dir)
--
2.14.1


2018-02-16 18:50:49

by Carlo Caione

[permalink] [raw]
Subject: [PATCH v2 2/3] ACPI: AC/battery: Add quirks for ECS EF20EA

From: Carlo Caione <[email protected]>

On the ECS EF20EA laptop we need to move away from the AXP288 FG driver
and enable again the ACPI AC/battery drivers. Add the required quirks to
do that. We rely only on the product name because all the other DMI
entries are dummy or not filled in on this platform.

Signed-off-by: Carlo Caione <[email protected]>
---
drivers/acpi/ac.c | 7 +++++++
drivers/acpi/battery.c | 7 +++++++
2 files changed, 14 insertions(+)

diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index b9a4ca720309..39f778f954f5 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -332,6 +332,13 @@ static const struct dmi_system_id ac_dmi_table[] = {
DMI_MATCH(DMI_PRODUCT_NAME, "32597CG"),
},
},
+ {
+ .callback = ac_not_use_pmic_quirk,
+ .ident = "ECS EF20EA",
+ .matches = {
+ DMI_MATCH(DMI_PRODUCT_NAME, "EF20EA"),
+ },
+ },
{},
};

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 258d2e66f230..fe024f1a8ad5 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -1233,6 +1233,13 @@ static const struct dmi_system_id bat_dmi_table[] __initconst = {
DMI_MATCH(DMI_PRODUCT_NAME, "UX410UAK"),
},
},
+ {
+ .callback = battery_not_use_pmic_quirk,
+ .ident = "ECS EF20EA",
+ .matches = {
+ DMI_MATCH(DMI_PRODUCT_NAME, "EF20EA"),
+ },
+ },
{},
};

--
2.14.1


2018-02-16 18:51:05

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] ACPI: AC/battery: Add quirk to avoid using PMIC

On Fri, Feb 16, 2018 at 9:26 AM, Carlo Caione <[email protected]> wrote:
> From: Carlo Caione <[email protected]>
>
> With commits af3ec837 and dccfae6d a blacklist was introduced to avoid
> using the ACPI drivers for AC and battery when a native PMIC driver was
> already present. While this is in general a good idea (because of broken
> DSDT or proprietary and undocumented ACPI opregions for the ACPI
> AC/battery devices) we have come across at least one CherryTrail laptop
> (ECS EF20EA) shipping the AXP288 together with a separate FG controller
> (a MAX17047) instead of the one embedded in the AXP288.
>
> The net effect of blacklisting the ACPI drivers is that on this platform
> the AC/battery reporting is broken since the information is coming from
> the AXP288 FG driver, not actually used in hardware.
>
> In this case we want to keep using the ACPI AC/battery driver that is
> able to interface correctly with the real FG controller.
>
> We introduce therefore a new quirk to avoid the blacklist.
>
> Signed-off-by: Carlo Caione <[email protected]>

First, please don't split the patches the way you have split them.

Put all battery material into one patch and all AC material into the
next one. It is better to introduce things like
ac_not_use_pmic_quirk() along with the quirks actually using them.

> ---
> drivers/acpi/ac.c | 26 ++++++++++++++++++--------
> drivers/acpi/battery.c | 26 ++++++++++++++++++--------
> 2 files changed, 36 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
> index 47a7ed557bd6..b9a4ca720309 100644
> --- a/drivers/acpi/ac.c
> +++ b/drivers/acpi/ac.c
> @@ -87,6 +87,7 @@ static int acpi_ac_open_fs(struct inode *inode, struct file *file);
>
>
> static int ac_sleep_before_get_state_ms;
> +static bool ac_not_use_pmic;

I would use a different name here, this one is confusing IMO.

Something like ac_skip_pmic_blacklist would be better.

2018-02-16 18:51:12

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] power: supply: Fix AXP288 fallback when not needed

Hi,

On 16-02-18 09:26, Carlo Caione wrote:
> From: Carlo Caione <[email protected]>
>
> With commits af3ec837 and dccfae6d a blacklist was introduced to avoid
> using the ACPI drivers for AC and battery when a native PMIC driver was
> already present. While this is in general a good idea (because of broken
> DSDT or proprietary and undocumented ACPI opregions for the ACPI
> AC/battery devices) we have come across at least one CherryTrail laptop
> (ECS EF20EA) shipping the AXP288 together with a separate FG controller
> (a MAX17047) instead of the one embedded in the AXP288.

Thank you for the new version. This looks good and surprisingly
clean / small given amounts of warts surrounding this all.

The entire series is:

Reviewed-by: Hans de Goede <[email protected]>

Regards,

Hans




>
> This is the interesting analisys done by Hans de Goede (thank you):
>
> Looking at the _BIX method of the BATC/PNP0C0A device, we see it referencing
> FG10:
>
> Method (_BIX, 0, NotSerialized) // _BIX: Battery Information Extend
> {
> If (AVBL == One)
> {
> BUF2 = FG10 /* \_SB_.PCI0.I2C1.FG10 */
>
> And FG10 is defined as:
>
> Field (DVFG, BufferAcc, NoLock, Preserve)
> {
> Connection (SMFG),
> Offset (0x10),
> AccessAs (BufferAcc, AttribBytes (0x02)),
> FG10, 8
> }
>
> With SMFG being defined as:
>
> Name (SMFG, ResourceTemplate ()
> {
> I2cSerialBusV2 (0x0036, ControllerInitiated, 0x000186A0,
> AddressingMode7Bit, "\\_SB.PCI0.I2C1",
> 0x00, ResourceConsumer, , Exclusive,
> )
> })
>
> Looking for I2C1 address 0x0036 we find:
>
> Device (ANFG)
> {
> Name (_HID, "MAX17047" /* Fuel Gauge Controller */) // _HID: Hardwa
> Name (_CID, "MAX17047" /* Fuel Gauge Controller */) // _CID: Compat
> Name (_DDN, "Fuel Gauge Controller") // _DDN: DOS Device Name
> Name (RBUF, ResourceTemplate ()
> {
> I2cSerialBusV2 (0x0036, ControllerInitiated, 0x00061A80,
> AddressingMode7Bit, "\\_SB.PCI0.I2C1",
> 0x00, ResourceConsumer, , Exclusive,
> )
> GpioInt (Level, ActiveLow, ExclusiveAndWake, PullNone, 0x0000,
> "\\_SB.GPO3", 0x00, ResourceConsumer, ,
> )
> { // Pin list
> 0x0001
> }
> })
>
> Where as the AXP288 PMIC is I2C7 address 0x034:
>
> Device (PMI1)
> {
> Name (_ADR, Zero) // _ADR: Address
> Name (_HID, "INT33F4" /* XPOWER PMIC Controller */) // _HID: Ha
> Name (_CID, "INT33F4" /* XPOWER PMIC Controller */) // _CID: Co
> Name (_DDN, "XPOWER PMIC Controller") // _DDN: DOS Device Name
> Name (_HRV, 0x03) // _HRV: Hardware Revision
> Name (_UID, One) // _UID: Unique ID
>
> Method (_CRS, 0, NotSerialized) // _CRS: Current Resource Setti
> {
> Name (SBUF, ResourceTemplate ()
> {
> I2cSerialBusV2 (0x0034, ControllerInitiated, 0x000F4240,
> AddressingMode7Bit, "\\_SB.PCI0.I2C7",
> 0x00, ResourceConsumer, , Exclusive,
> )
>
> So basically this laptopt is using a separate FG chip instead of the one
> embedded in the AXP288.
>
> To have this correctly working we need basically to avoid the fallback on the
> AXP288 driver enabling again the ACPI AC/battery drivers and at the same time
> avoiding that the AXP288 FG driver is probed at all.
>
> I'm still not fully convinced that having two different quirks (one to disable
> the blacklist and another to disable the AXP288 FG probing) is the right way to
> fix this. So any comment is welcome.
>
> Changelog:
>
> v2:
> - Split [PATCH 2/2] in two parts
> - Rework subject prefixes
>
> Carlo Caione (3):
> ACPI: AC/battery: Add quirk to avoid using PMIC
> ACPI: AC/battery: Add quirks for ECS EF20EA
> power: supply: axp288_fuel_gauge: Do not register FG on ECS EF20EA
>
> drivers/acpi/ac.c | 33 ++++++++++++++++++++++++--------
> drivers/acpi/battery.c | 33 ++++++++++++++++++++++++--------
> drivers/power/supply/axp288_fuel_gauge.c | 6 ++++++
> 3 files changed, 56 insertions(+), 16 deletions(-)
>

2018-02-16 18:52:03

by Carlo Caione

[permalink] [raw]
Subject: [PATCH v2 3/3] power: supply: axp288_fuel_gauge: Do not register FG on ECS EF20EA

From: Carlo Caione <[email protected]>

The ECS EF20EA laptop ships an AXP288 but it is actually using a
different, separate FG chip for AC and battery monitoring. On this
laptop we need to keep using the regular ACPI driver and disable the
AXP288 FG to avoid reporting two batteries to userspace.

Signed-off-by: Carlo Caione <[email protected]>
---
drivers/power/supply/axp288_fuel_gauge.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/power/supply/axp288_fuel_gauge.c b/drivers/power/supply/axp288_fuel_gauge.c
index 4cc6e038dfdd..903891a9bcf0 100644
--- a/drivers/power/supply/axp288_fuel_gauge.c
+++ b/drivers/power/supply/axp288_fuel_gauge.c
@@ -708,6 +708,12 @@ static const struct dmi_system_id axp288_fuel_gauge_blacklist[] = {
DMI_MATCH(DMI_BOARD_VERSION, "V1.1"),
},
},
+ {
+ /* ECS EF20EA */
+ .matches = {
+ DMI_MATCH(DMI_PRODUCT_NAME, "EF20EA"),
+ },
+ },
{}
};

--
2.14.1


2018-02-16 18:52:08

by Carlo Caione

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] power: supply: Fix AXP288 fallback when not needed

On Fri, Feb 16, 2018 at 8:52 AM, Hans de Goede <[email protected]> wrote:
> Hi,

>> Well, to be honest, I very much prefer it when changes are made to one
>> driver at a time.
>
>
> I understand completely, Carlo can you do a v4 with the changes
> Rafael requested please? Feel free to keep my Reviewed-by for the v4.

No problem. Thank you both for the reviews.

Cheers,

--
Carlo Caione | +44.7384.69.16.04 | Endless

2018-02-16 18:52:10

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] power: supply: Fix AXP288 fallback when not needed

Hi,

On 16-02-18 09:51, Rafael J. Wysocki wrote:
> On Fri, Feb 16, 2018 at 9:41 AM, Hans de Goede <[email protected]> wrote:
>> Hi,
>>
>> On 16-02-18 09:26, Carlo Caione wrote:
>>>
>>> From: Carlo Caione <[email protected]>
>>>
>>> With commits af3ec837 and dccfae6d a blacklist was introduced to avoid
>>> using the ACPI drivers for AC and battery when a native PMIC driver was
>>> already present. While this is in general a good idea (because of broken
>>> DSDT or proprietary and undocumented ACPI opregions for the ACPI
>>> AC/battery devices) we have come across at least one CherryTrail laptop
>>> (ECS EF20EA) shipping the AXP288 together with a separate FG controller
>>> (a MAX17047) instead of the one embedded in the AXP288.
>>
>>
>> Thank you for the new version. This looks good and surprisingly
>> clean / small given amounts of warts surrounding this all.
>>
>> The entire series is:
>>
>> Reviewed-by: Hans de Goede <[email protected]>
>
> Well, to be honest, I very much prefer it when changes are made to one
> driver at a time.

I understand completely, Carlo can you do a v4 with the changes
Rafael requested please? Feel free to keep my Reviewed-by for the v4.

Regards,

Hans


2018-02-16 19:13:11

by Sebastian Reichel

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] power: supply: axp288_fuel_gauge: Do not register FG on ECS EF20EA

Hi,

On Fri, Feb 16, 2018 at 08:26:16AM +0000, Carlo Caione wrote:
> From: Carlo Caione <[email protected]>
>
> The ECS EF20EA laptop ships an AXP288 but it is actually using a
> different, separate FG chip for AC and battery monitoring. On this
> laptop we need to keep using the regular ACPI driver and disable the
> AXP288 FG to avoid reporting two batteries to userspace.
>
> Signed-off-by: Carlo Caione <[email protected]>
> ---

It looks like this could be applied independently of the ACPI
patches to the power-supply tree?

-- Sebastian

> drivers/power/supply/axp288_fuel_gauge.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/power/supply/axp288_fuel_gauge.c b/drivers/power/supply/axp288_fuel_gauge.c
> index 4cc6e038dfdd..903891a9bcf0 100644
> --- a/drivers/power/supply/axp288_fuel_gauge.c
> +++ b/drivers/power/supply/axp288_fuel_gauge.c
> @@ -708,6 +708,12 @@ static const struct dmi_system_id axp288_fuel_gauge_blacklist[] = {
> DMI_MATCH(DMI_BOARD_VERSION, "V1.1"),
> },
> },
> + {
> + /* ECS EF20EA */
> + .matches = {
> + DMI_MATCH(DMI_PRODUCT_NAME, "EF20EA"),
> + },
> + },
> {}
> };
>
> --
> 2.14.1
>


Attachments:
(No filename) (1.19 kB)
signature.asc (849.00 B)
Download all attachments

2018-02-16 19:35:20

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] power: supply: Fix AXP288 fallback when not needed

On Fri, Feb 16, 2018 at 9:41 AM, Hans de Goede <[email protected]> wrote:
> Hi,
>
> On 16-02-18 09:26, Carlo Caione wrote:
>>
>> From: Carlo Caione <[email protected]>
>>
>> With commits af3ec837 and dccfae6d a blacklist was introduced to avoid
>> using the ACPI drivers for AC and battery when a native PMIC driver was
>> already present. While this is in general a good idea (because of broken
>> DSDT or proprietary and undocumented ACPI opregions for the ACPI
>> AC/battery devices) we have come across at least one CherryTrail laptop
>> (ECS EF20EA) shipping the AXP288 together with a separate FG controller
>> (a MAX17047) instead of the one embedded in the AXP288.
>
>
> Thank you for the new version. This looks good and surprisingly
> clean / small given amounts of warts surrounding this all.
>
> The entire series is:
>
> Reviewed-by: Hans de Goede <[email protected]>

Well, to be honest, I very much prefer it when changes are made to one
driver at a time.

2018-02-19 09:49:51

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] power: supply: axp288_fuel_gauge: Do not register FG on ECS EF20EA

Hi,

On 16-02-18 15:26, Sebastian Reichel wrote:
> Hi,
>
> On Fri, Feb 16, 2018 at 08:26:16AM +0000, Carlo Caione wrote:
>> From: Carlo Caione <[email protected]>
>>
>> The ECS EF20EA laptop ships an AXP288 but it is actually using a
>> different, separate FG chip for AC and battery monitoring. On this
>> laptop we need to keep using the regular ACPI driver and disable the
>> AXP288 FG to avoid reporting two batteries to userspace.
>>
>> Signed-off-by: Carlo Caione <[email protected]>
>> ---
>
> It looks like this could be applied independently of the ACPI
> patches to the power-supply tree?

Yes this can be applied independently.

Regards,

Hans



>
> -- Sebastian
>
>> drivers/power/supply/axp288_fuel_gauge.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/power/supply/axp288_fuel_gauge.c b/drivers/power/supply/axp288_fuel_gauge.c
>> index 4cc6e038dfdd..903891a9bcf0 100644
>> --- a/drivers/power/supply/axp288_fuel_gauge.c
>> +++ b/drivers/power/supply/axp288_fuel_gauge.c
>> @@ -708,6 +708,12 @@ static const struct dmi_system_id axp288_fuel_gauge_blacklist[] = {
>> DMI_MATCH(DMI_BOARD_VERSION, "V1.1"),
>> },
>> },
>> + {
>> + /* ECS EF20EA */
>> + .matches = {
>> + DMI_MATCH(DMI_PRODUCT_NAME, "EF20EA"),
>> + },
>> + },
>> {}
>> };
>>
>> --
>> 2.14.1
>>