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

2018-02-16 Thread Carlo Caione 
On Fri, Feb 16, 2018 at 8:52 AM, Hans de Goede  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

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

2018-02-16 Thread Carlo Caione 
On Fri, Feb 16, 2018 at 8:52 AM, Hans de Goede  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

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

2018-02-16 Thread Hans de Goede 

Hi,

On 16-02-18 09:51, Rafael J. Wysocki wrote:

On Fri, Feb 16, 2018 at 9:41 AM, Hans de Goede  wrote:

Hi,

On 16-02-18 09:26, Carlo Caione wrote:


From: Carlo Caione 

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 


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

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

2018-02-16 Thread Hans de Goede 

Hi,

On 16-02-18 09:51, Rafael J. Wysocki wrote:

On Fri, Feb 16, 2018 at 9:41 AM, Hans de Goede  wrote:

Hi,

On 16-02-18 09:26, Carlo Caione wrote:


From: Carlo Caione 

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 


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

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

2018-02-16 Thread Rafael J. Wysocki 
On Fri, Feb 16, 2018 at 9:41 AM, Hans de Goede  wrote:
> Hi,
>
> On 16-02-18 09:26, Carlo Caione wrote:
>>
>> From: Carlo Caione 
>>
>> 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 

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

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

2018-02-16 Thread Rafael J. Wysocki 
On Fri, Feb 16, 2018 at 9:41 AM, Hans de Goede  wrote:
> Hi,
>
> On 16-02-18 09:26, Carlo Caione wrote:
>>
>> From: Carlo Caione 
>>
>> 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 

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

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

2018-02-16 Thread Hans de Goede 

Hi,

On 16-02-18 09:26, Carlo Caione wrote:

From: Carlo Caione 

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 

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, 0x,
 "\\_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(-)

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

2018-02-16 Thread Hans de Goede 

Hi,

On 16-02-18 09:26, Carlo Caione wrote:

From: Carlo Caione 

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 

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, 0x,
 "\\_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(-)

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

2018-02-16 Thread Carlo Caione 
From: Carlo Caione 

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, 0x,
"\\_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

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

2018-02-16 Thread Carlo Caione 
From: Carlo Caione 

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, 0x,
"\\_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