On Mon, 2021-10-18 at 11:16 +0200, Hans de Goede wrote: > 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@xxxxxxxxx> > > 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. Thanks! I tried your attached patch, and I can confirm that it's working as expected. Now it's using cht one: $ ls /sys/devices/pci0000:00/808622C1:05/i2c-5/i2c-INT33FD:00 cht_crystal_cove_pmic crystal_cove_gpio crystal_cove_pwm driver firmware_node modalias name power subsystem uevent and the function intel_soc_pmic_exec_mipi_pmic_seq_element() is also working with atomisp driver. Regards, Tsuchiya Yuto