Re: [RFC PATCH 1/1] ACPI / PMIC: Add i2c address to intel_pmic_bytcrc driver

[Andy Shevchenko]

On Sun, Oct 17, 2021 at 7:16 PM Tsuchiya Yuto <kitakar@xxxxxxxxx> wrote:
> On Microsoft Surface 3 (uses Intel's Atom Cherry Trail SoC), executing
> intel_soc_pmic_exec_mipi_pmic_seq_element() results in the following
> error message:
>
>         [ 7196.356682] intel_soc_pmic_exec_mipi_pmic_seq_element: Not implemented
>         [ 7196.356686] intel_soc_pmic_exec_mipi_pmic_seq_element: i2c-addr: 0x6e reg-addr 0x57 value 0x63 mask 0xff
>
> Surface 3 uses the PMIC device INT33FD, and the DSDT describes its _HRV
> value is 0x02 [1]:
>
>         Scope (PCI0.I2C7)
>         {
>             Device (PMIC)
>             {
>                 Name (_ADR, Zero)  // _ADR: Address
>                 Name (_HID, "INT33FD" /* Intel Baytrail Power Management IC */)  // _HID: Hardware ID
>                 Name (_CID, "INT33FD" /* Intel Baytrail Power Management IC */)  // _CID: Compatible ID
>                 Name (_DDN, "CRYSTAL COVE+ AIC")  // _DDN: DOS Device Name
>                 Name (_HRV, 0x02)  // _HRV: Hardware Revision
>                 Name (_UID, One)  // _UID: Unique ID
>                 Name (_DEP, Package (0x01)  // _DEP: Dependencies
>                 {
>                     I2C7
>                 })
>         [...]
>
> Due to this _HRV value, intel_pmic_bytcrc is used as the PMIC driver.
> However, the i2c address is currently not defined in this driver.
> This commit adds the missing i2c address 0x6e to the intel_pmic_bytcrc
> driver.
>
> [1] https://github.com/linux-surface/acpidumps/blob/f8db3d150815aa21530635b7e646eee271e3b8fe/surface_3/dsdt.dsl#L10868

> References: cc0594c4b0ef ("ACPI / PMIC: Add i2c address for thermal control")

Not sure what this tag means.

After reading a bit of code I think the best approach is to quirk the
drivers/mfd/intel_soc_pmic_core.c with DMI to supply HRV. Setting an
address to all BYT devices may not be the best since I have no idea if
there are different addresses in use. It may be the case, but the
problem is that we have no proof. Also BYT driver uses hardcoded power
and thermal tables, I'm completely unsure that this would not damage
hardware in some circumstances.

Hans, what's your opinion on this?

P.S. I dunno if _UID correlates to the type of the chipset (BYT/CHT).

-- 
With Best Regards,
Andy Shevchenko