Hi, On 10/25/21 17:12, Mika Westerberg wrote: > On Mon, Oct 25, 2021 at 04:54:41PM +0200, Hans de Goede wrote: >>> Yes that's exactly what is supposed to happen that this attribute is made. >>> What exactly happens when you write into it? >> >> The _SB.CGWR ACPI method gets called, with arguments coming from ACPI >> settings stored in memory. Depending on those settings this function >> either directly pokes some MMIO or tries to talk to an I2C GPIO >> expander which is not present on the Surface Go, causing it to >> MMIO poke an I2C controller which it should not touch. >> >> In either case the AML code ends up poking stuff it should not touch >> and the entire force_power sysfs attribute should simply not be >> there on devices without thunderbolt. > > That's right - it should not be there in the first place if there is no > Thunderbolt controller on that thing. > > I guess most of the systems that have this actually do support > Thunderbolt so maybe we can work this around by quirking all the Surface > models in that driver? I was hoping that we could avoid this, but yes if there is no easy / clean way to detect if there are any Thunderbolt controllers on the system then a DMI table is necessary. My reason for looking into this is because force_power writes may end up accessing the _SB.GEXP ACPI device which is present in most DSDTs for devices with recent Intel Core CPUs (this device seems to represent an I2C attached GPIO expander). This _SB.GEXP ACPI device claims the MMIO region / PCI BAR of the I2C4 controller but on the Surface the I2C4 bus is used for the front camera sensor; and i2cdetect shows there is no GPIO-expander. So I plan to add an exception for acpi_enforce_resources for the Surface Go and Surface Go 2 so that Linux can use I2C4. To make sure this is safe I audited all GEXP accesses in the DSDT. And depending on the value of the FPAT value from the ACPI config memory area writing force_power may access the GEXP device, so to make sure this does not happen; and thus it is safe to add an override for acpi_enforce_resources, we will need something akin to a no_thunderbolt_dmi_ids DMI table inside intel-wmi-thunderbolt. Regards, Hans
