2024-06-05 06:43:35

by Ben Walsh

[permalink] [raw]
Subject: [PATCH v3 0/5] platform/chrome: Fix MEC concurrency problems for Framework Laptop

Framework Laptops with the Microchip EC have a problem where the EC
"stops working" after a while. Symptoms include the Fn key not
working, and "bad packet checksum" errors appearing in the system log.

The problem is caused by ACPI code which accesses the Microchip EC
(MEC) memory using the Microchip EMI protocol. It uses an AML mutex to
prevent concurrent access. But the cros_ec_lpc driver is not aware of
this mutex. The ACPI code and LPC driver both attempt to talk to the
EC at the same time, messing up communication with the EC.

The solution is to have the cros_ec_lpc_mec code find and use the AML
mutex. But to make it all work we have to do a few more things:

* Allow the cros_ec_lpc_mec code to return error codes in case it
can't lock the mutex.

* Have the cros_ec_lpc code find the correct ACPI device (PNP0C09)
and then the AML mutex itself.

* Use the quirks mechanism to specify the AML mutex name.

Tested-on: link, hx20, azalea
Tested-by: Dustin L. Howett <[email protected]>

Changes in v3:

* Use ACPI_COMPANION instead of adev variable.
* Stricter error checking in probe.
* Remove a few blank lines.
* Rebase to latest ChromeOS for-next branch.

Changes in v2:

* Put ACPI id in a quirk, not the acpi_match_table.
* Add return code check for mutex unlock.
* Remove "in_range" check which belongs in a separate patch.
* Use "ret" not "sum" variable (same value but clearer).
* Remove excessive error logging.
* Rebase to latest ChromeOS for-next branch.

Ben Walsh (5):
platform/chrome: cros_ec_lpc: MEC access can return error code
platform/chrome: cros_ec_lpc: MEC access can use an AML mutex
platform/chrome: cros_ec_lpc: Add a new quirk for ACPI id
platform/chrome: cros_ec_lpc: Add a new quirk for AML mutex
platform/chrome: cros_ec_lpc: Add quirks for Framework Laptop

drivers/platform/chrome/cros_ec_lpc.c | 206 +++++++++++++++------
drivers/platform/chrome/cros_ec_lpc_mec.c | 85 ++++++++-
drivers/platform/chrome/cros_ec_lpc_mec.h | 18 +-
drivers/platform/chrome/wilco_ec/mailbox.c | 22 ++-
4 files changed, 261 insertions(+), 70 deletions(-)


base-commit: 106d6739823369c734a8fc3b13634274eee4f60e
--
2.45.1



2024-06-05 06:43:43

by Ben Walsh

[permalink] [raw]
Subject: [PATCH v3 5/5] platform/chrome: cros_ec_lpc: Add quirks for Framework Laptop

For Framework Laptops with Microchip EC (MEC), use the ACPI id
"PNP0C09" to find the ACPI device, and AML mutex "ECMT" to protect EC
memory access.

Signed-off-by: Ben Walsh <[email protected]>
---
drivers/platform/chrome/cros_ec_lpc.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 5d9cc8df208b..ebe9fb143840 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -636,6 +636,12 @@ static const struct lpc_driver_data framework_laptop_amd_lpc_driver_data __initc
.quirk_mmio_memory_base = 0xE00,
};

+static const struct lpc_driver_data framework_laptop_11_lpc_driver_data __initconst = {
+ .quirks = CROS_EC_LPC_QUIRK_ACPI_ID|CROS_EC_LPC_QUIRK_AML_MUTEX,
+ .quirk_acpi_id = "PNP0C09",
+ .quirk_aml_mutex_name = "ECMT",
+};
+
static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
{
/*
@@ -704,6 +710,7 @@ static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
DMI_MATCH(DMI_PRODUCT_NAME, "Laptop"),
},
+ .driver_data = (void *)&framework_laptop_11_lpc_driver_data,
},
{ /* sentinel */ }
};
--
2.45.1


Subject: Re: [PATCH v3 0/5] platform/chrome: Fix MEC concurrency problems for Framework Laptop

Hello:

This series was applied to chrome-platform/linux.git (for-kernelci)
by Tzung-Bi Shih <[email protected]>:

On Wed, 5 Jun 2024 07:33:46 +0100 you wrote:
> Framework Laptops with the Microchip EC have a problem where the EC
> "stops working" after a while. Symptoms include the Fn key not
> working, and "bad packet checksum" errors appearing in the system log.
>
> The problem is caused by ACPI code which accesses the Microchip EC
> (MEC) memory using the Microchip EMI protocol. It uses an AML mutex to
> prevent concurrent access. But the cros_ec_lpc driver is not aware of
> this mutex. The ACPI code and LPC driver both attempt to talk to the
> EC at the same time, messing up communication with the EC.
>
> [...]

Here is the summary with links:
- [v3,1/5] platform/chrome: cros_ec_lpc: MEC access can return error code
https://git.kernel.org/chrome-platform/c/68dbac0a58ef
- [v3,2/5] platform/chrome: cros_ec_lpc: MEC access can use an AML mutex
https://git.kernel.org/chrome-platform/c/60c7df66450e
- [v3,3/5] platform/chrome: cros_ec_lpc: Add a new quirk for ACPI id
https://git.kernel.org/chrome-platform/c/040159e0912c
- [v3,4/5] platform/chrome: cros_ec_lpc: Add a new quirk for AML mutex
https://git.kernel.org/chrome-platform/c/38c31b1d737b
- [v3,5/5] platform/chrome: cros_ec_lpc: Add quirks for Framework Laptop
https://git.kernel.org/chrome-platform/c/04ca0a51f1e6

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



Subject: Re: [PATCH v3 0/5] platform/chrome: Fix MEC concurrency problems for Framework Laptop

Hello:

This series was applied to chrome-platform/linux.git (for-next)
by Tzung-Bi Shih <[email protected]>:

On Wed, 5 Jun 2024 07:33:46 +0100 you wrote:
> Framework Laptops with the Microchip EC have a problem where the EC
> "stops working" after a while. Symptoms include the Fn key not
> working, and "bad packet checksum" errors appearing in the system log.
>
> The problem is caused by ACPI code which accesses the Microchip EC
> (MEC) memory using the Microchip EMI protocol. It uses an AML mutex to
> prevent concurrent access. But the cros_ec_lpc driver is not aware of
> this mutex. The ACPI code and LPC driver both attempt to talk to the
> EC at the same time, messing up communication with the EC.
>
> [...]

Here is the summary with links:
- [v3,1/5] platform/chrome: cros_ec_lpc: MEC access can return error code
https://git.kernel.org/chrome-platform/c/68dbac0a58ef
- [v3,2/5] platform/chrome: cros_ec_lpc: MEC access can use an AML mutex
https://git.kernel.org/chrome-platform/c/60c7df66450e
- [v3,3/5] platform/chrome: cros_ec_lpc: Add a new quirk for ACPI id
https://git.kernel.org/chrome-platform/c/040159e0912c
- [v3,4/5] platform/chrome: cros_ec_lpc: Add a new quirk for AML mutex
https://git.kernel.org/chrome-platform/c/38c31b1d737b
- [v3,5/5] platform/chrome: cros_ec_lpc: Add quirks for Framework Laptop
https://git.kernel.org/chrome-platform/c/04ca0a51f1e6

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



2024-06-06 07:52:08

by Ben Walsh

[permalink] [raw]
Subject: [PATCH v3 4/5] platform/chrome: cros_ec_lpc: Add a new quirk for AML mutex

Add a new quirk "CROS_EC_LPC_QUIRK_AML_MUTEX" which provides the name
of an AML mutex to protect MEC memory access.

Signed-off-by: Ben Walsh <[email protected]>
---
drivers/platform/chrome/cros_ec_lpc.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index fa6606da802a..5d9cc8df208b 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -44,6 +44,11 @@ static bool cros_ec_lpc_acpi_device_found;
* the ACPI device.
*/
#define CROS_EC_LPC_QUIRK_ACPI_ID BIT(1)
+/*
+ * Indicates that lpc_driver_data.quirk_aml_mutex_name should be used
+ * to find an AML mutex to protect access to Microchip EC.
+ */
+#define CROS_EC_LPC_QUIRK_AML_MUTEX BIT(2)

/**
* struct lpc_driver_data - driver data attached to a DMI device ID to indicate
@@ -52,11 +57,14 @@ static bool cros_ec_lpc_acpi_device_found;
* @quirk_mmio_memory_base: The first I/O port addressing EC mapped memory (used
* when quirk ...REMAP_MEMORY is set.)
* @quirk_acpi_id: An ACPI HID to be used to find the ACPI device.
+ * @quirk_aml_mutex_name: The name of an AML mutex to be used to protect access
+ * to Microchip EC.
*/
struct lpc_driver_data {
u32 quirks;
u16 quirk_mmio_memory_base;
const char *quirk_acpi_id;
+ const char *quirk_aml_mutex_name;
};

/**
@@ -482,6 +490,17 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
}
ACPI_COMPANION_SET(dev, adev);
}
+
+ if (quirks & CROS_EC_LPC_QUIRK_AML_MUTEX) {
+ const char *name
+ = driver_data->quirk_aml_mutex_name;
+ ret = cros_ec_lpc_mec_acpi_mutex(ACPI_COMPANION(dev), name);
+ if (ret) {
+ dev_err(dev, "failed to get AML mutex '%s'", name);
+ return ret;
+ }
+ dev_info(dev, "got AML mutex '%s'", name);
+ }
}

/*
--
2.45.1