linux-kernel - Re: [RFC PATCH 1/1] ACPI / PMIC: Add i2c address to intel_pmic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 17 Oct 2021 22:08:16 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Tsuchiya Yuto <kitakar@...il.com>
Cc:     Hans de Goede <hdegoede@...hat.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Len Brown <lenb@...nel.org>, Andy Shevchenko <andy@...nel.org>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 1/1] ACPI / PMIC: Add i2c address to intel_pmic_bytcrc driver

On Sun, Oct 17, 2021 at 7:16 PM Tsuchiya Yuto <kitakar@...il.com> 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