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]

Message-ID: <3e6428f1-9411-fac6-9172-1dfe6de58c28@redhat.com>
Date:   Mon, 18 Oct 2021 11:16:09 +0200
From:   Hans de Goede <hdegoede@...hat.com>
To:     Tsuchiya Yuto <kitakar@...il.com>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Len Brown <lenb@...nel.org>, Andy Shevchenko <andy@...nel.org>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 1/1] ACPI / PMIC: Add i2c address to intel_pmic_bytcrc
 driver

Hi,

On 10/17/21 18:15, Tsuchiya Yuto 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")
> Signed-off-by: Tsuchiya Yuto <kitakar@...il.com>

I believe that it is very unlikely that a device with a Cherry Trail SoC is actually using
the Bay Trail version of the PMIC, I would expect that to not necessarily work all that well.

So as Andy said, the right fix here is something like the:

+	hrv = 0x03;

Workaround from your cover-letter.

As Andy said we could use a DMI quirk for this, but chances are that the Microsoft Surface
DSDT is not the only one with the wrong HRV value. So instead it might be better to
just test for the SoC type as the attached patch does.

Tsuchiya, can you give the attached patch a try.

Andy, what do you think, should we go with the attached patch or would you prefer using
a DMI quirk ?

Regards,

Hans






> ---
>  drivers/acpi/pmic/intel_pmic_bytcrc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/acpi/pmic/intel_pmic_bytcrc.c b/drivers/acpi/pmic/intel_pmic_bytcrc.c
> index 2a692cc4b7ae..a64f50a42c54 100644
> --- a/drivers/acpi/pmic/intel_pmic_bytcrc.c
> +++ b/drivers/acpi/pmic/intel_pmic_bytcrc.c
> @@ -282,6 +282,7 @@ static struct intel_pmic_opregion_data intel_crc_pmic_opregion_data = {
>  	.power_table_count= ARRAY_SIZE(power_table),
>  	.thermal_table	= thermal_table,
>  	.thermal_table_count = ARRAY_SIZE(thermal_table),
> +	.pmic_i2c_address = 0x6e,
>  };
>  
>  static int intel_crc_pmic_opregion_probe(struct platform_device *pdev)
> 
View attachment "0001-mfd-intel_soc_pmic-Use-CPU-id-check-instead-of-_HRV-.patch" of type "text/x-patch" (3237 bytes)