On Tue, Oct 26, 2021 at 12:34:33PM +0200, Hans de Goede wrote:

Hi,

On 10/26/21 10:53, Mika Westerberg wrote:

quoted

Hi,

On Tue, Oct 26, 2021 at 10:17:53AM +0200, Hans de Goede wrote:

quoted

Hi,

On 10/25/21 17:12, Mika Westerberg wrote:

quoted

On Mon, Oct 25, 2021 at 04:54:41PM +0200, Hans de Goede wrote:

quoted

quoted

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.

Well, the force power thing is there just for this reason. It should
only be present on systems using ACPI assisted PCIe hotplug for
Thunderbolt devices. Apparantly some BIOS engineer forgot to remove it
on Surface :( I need to check if it is present on recent reference
BIOSes too. If it is then I'll report an internal sighting about this to
get it removed.

In theory we could also use a heuristic that if there is a TBT
controller present when the driver probes it should fail the probe or
so. Or even look for the PCI host bridge and if it got the PCIe hotplug
capability from the BIOS (through _OSC negotiation) we can assume this
system does not need the force power.

I think adding such heuristics might be a good thing to do, because
I suspect that this problem is much wider then just a couple of
surface devices.

One worry I have about this is probe ordering. We cannot assume the
entire PCI bus has been enumerated when the intel-wmi-thunderbolt's
probe() method runs. So that would mean doing something like
returning -EPROBE_DEFER if no thunderbolt controller is found and
then say 1 minute after boot return -ENODEV to get us of the
probe_deferal devices list...

The whole PCI bus does not need to be enumerated - just the host bridge
which is typically pretty early.

IOW this is going to be ugly so for now I think a DMI list for the
devices where I want to make sure force_power does not poke the
GEXP device is best.

I agree. We can look for the other option later if more devices with
this issue are found.