On Mon, 29 Nov 2021 10:00:55 +0100, Wolfram Sang wrote:
On Sat, Jun 26, 2021 at 02:41:13PM +0900, Hector Martin wrote:
The i801 controller provides a locking mechanism that the OS is supposed to use to safely share the SMBus with ACPI AML or other firmware.
Previously, Linux attempted to get out of the way of ACPI AML entirely, but left the bus locked if it used it before the first AML access. This causes AML implementations that *do* attempt to safely share the bus to time out if Linux uses it first; notably, this regressed ACPI video backlight controls on 2015 iMacs after 01590f361e started instantiating SPD EEPROMs on boot.
Commit 065b6211a8 fixed the immediate problem of leaving the bus locked, but we can do better. The controller does have a proper locking mechanism, so let's use it as intended. Since we can't rely on the BIOS doing this properly, we implement the following logic:
- If ACPI AML uses the bus at all, we make a note and disable power management. The latter matches already existing behavior.
- When we want to use the bus, we attempt to lock it first. If the locking attempt times out, *and* ACPI hasn't tried to use the bus at all yet, we cautiously go ahead and assume the BIOS forgot to unlock the bus after boot. This preserves existing behavior.
- We always unlock the bus after a transfer.
- If ACPI AML tries to use the bus (except trying to lock it) while we're in the middle of a transfer, or after we've determined locking is broken, we know we cannot safely share the bus and give up.
Upon first usage of SMBus by ACPI AML, if nothing has gone horribly wrong so far, users will see:
i801_smbus 0000:00:1f.4: SMBus controller is shared with ACPI AML. This seems safe so far.
If locking the SMBus times out, users will see:
i801_smbus 0000:00:1f.4: BIOS left SMBus locked
And if ACPI AML tries to use the bus concurrently with Linux, or it previously used the bus and we failed to subsequently lock it as above, the driver will give up and users will get:
i801_smbus 0000:00:1f.4: BIOS uses SMBus unsafely i801_smbus 0000:00:1f.4: Driver SMBus register access inhibited
This fixes the regression introduced by 01590f361e, and further allows safely sharing the SMBus on 2015 iMacs. Tested by running `i2cdump` in a loop while changing backlight levels via the ACPI video device.
Fixes: 01590f361e ("i2c: i801: Instantiate SPD EEPROMs automatically") Cc: stable@vger.kernel.org Signed-off-by: Hector Martin marcan@marcan.st
Jean, Heiner, what do we do with this topic?
I like the idea, I need to give it a try and review the code.