Framework Laptops' ACPI exposes the EC with id "PNP0C09". But
"PNP0C09" is part of the ACPI standard; there are lots of computers
with EC chips with this id, and most of them don't support the cros_ec
protocol.
The driver could find the ACPI device by having "PNP0C09" in the
acpi_match_table, but this would match devices which don't support the
cros_ec protocol. Instead, add a new quirk "CROS_EC_LPC_QUIRK_ACPI_ID"
which allows the id to be specified. This quirk is applied after the
DMI check shows that the device is supported.
Signed-off-by: Ben Walsh <ben@jubnut.com>
---
drivers/platform/chrome/cros_ec_lpc.c | 53 ++++++++++++++++++++-------
1 file changed, 40 insertions(+), 13 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 6663141a07d4..c1c26a7ba6d4 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -39,6 +39,11 @@ static bool cros_ec_lpc_acpi_device_found;
* be used as the base port for EC mapped memory.
*/
#define CROS_EC_LPC_QUIRK_REMAP_MEMORY BIT(0)
+/*
+ * Indicates that lpc_driver_data.quirk_acpi_id should be used to find
+ * the ACPI device.
+ */
+#define CROS_EC_LPC_QUIRK_ACPI_ID BIT(1)
/**
* struct lpc_driver_data - driver data attached to a DMI device ID to indicate
@@ -46,10 +51,12 @@ static bool cros_ec_lpc_acpi_device_found;
* @quirks: a bitfield composed of quirks from CROS_EC_LPC_QUIRK_*
* @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.
*/
struct lpc_driver_data {
u32 quirks;
u16 quirk_mmio_memory_base;
+ const char *quirk_acpi_id;
};
/**
@@ -418,6 +425,26 @@ static void cros_ec_lpc_acpi_notify(acpi_handle device, u32 value, void *data)
pm_system_wakeup();
}
+static acpi_status cros_ec_lpc_parse_device(acpi_handle handle, u32 level,
+ void *context, void **retval)
+{
+ *(struct acpi_device **)context = acpi_fetch_acpi_dev(handle);
+ return AE_CTRL_TERMINATE;
+}
+
+static struct acpi_device *cros_ec_lpc_get_device(const char *id)
+{
+ struct acpi_device *adev = NULL;
+ acpi_status status = acpi_get_devices(id, cros_ec_lpc_parse_device,
+ &adev, NULL);
+ if (ACPI_FAILURE(status)) {
+ pr_warn(DRV_NAME ": Looking for %s failed\n", id);
+ return NULL;
+ }
+
+ return adev;
+}
+
static int cros_ec_lpc_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -436,6 +463,8 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
ec_lpc->mmio_memory_base = EC_LPC_ADDR_MEMMAP;
+ adev = ACPI_COMPANION(dev);
+
driver_data = platform_get_drvdata(pdev);
if (driver_data) {
quirks = driver_data->quirks;
@@ -445,6 +474,16 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
if (quirks & CROS_EC_LPC_QUIRK_REMAP_MEMORY)
ec_lpc->mmio_memory_base = driver_data->quirk_mmio_memory_base;
+
+ if (quirks & CROS_EC_LPC_QUIRK_ACPI_ID) {
+ adev = cros_ec_lpc_get_device(driver_data->quirk_acpi_id);
+ if (!adev) {
+ dev_err(dev, "failed to get ACPI device '%s'",
+ driver_data->quirk_acpi_id);
+ return -ENODEV;
+ }
+ ACPI_COMPANION_SET(dev, adev);
+ }
}
/*
@@ -538,7 +577,6 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
* Connect a notify handler to process MKBP messages if we have a
* companion ACPI device.
*/
- adev = ACPI_COMPANION(dev);
if (adev) {
status = acpi_install_notify_handler(adev->handle,
ACPI_ALL_NOTIFY,
@@ -705,23 +743,12 @@ static struct platform_device cros_ec_lpc_device = {
.name = DRV_NAME
};
-static acpi_status cros_ec_lpc_parse_device(acpi_handle handle, u32 level,
- void *context, void **retval)
-{
- *(bool *)context = true;
- return AE_CTRL_TERMINATE;
-}
-
static int __init cros_ec_lpc_init(void)
{
int ret;
- acpi_status status;
const struct dmi_system_id *dmi_match;
- status = acpi_get_devices(ACPI_DRV_NAME, cros_ec_lpc_parse_device,
- &cros_ec_lpc_acpi_device_found, NULL);
- if (ACPI_FAILURE(status))
- pr_warn(DRV_NAME ": Looking for %s failed\n", ACPI_DRV_NAME);
+ cros_ec_lpc_acpi_device_found = !!cros_ec_lpc_get_device(ACPI_DRV_NAME);
dmi_match = dmi_first_match(cros_ec_lpc_dmi_table);
--
2.45.1On Mon, Jun 03, 2024 at 07:38:32AM +0100, Ben Walsh wrote:
> @@ -436,6 +463,8 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
>
> ec_lpc->mmio_memory_base = EC_LPC_ADDR_MEMMAP;
>
> + adev = ACPI_COMPANION(dev);
> +
The change is irrelevant to the patch.
> @@ -538,7 +577,6 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
> * Connect a notify handler to process MKBP messages if we have a
> * companion ACPI device.
> */
> - adev = ACPI_COMPANION(dev);
> if (adev) {
> status = acpi_install_notify_handler(adev->handle,
> ACPI_ALL_NOTIFY,
See above comment.
On 6/3/2024 04:30, Tzung-Bi Shih wrote:
> On Mon, Jun 03, 2024 at 07:38:32AM +0100, Ben Walsh wrote:
>> @@ -436,6 +463,8 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
>>
>> ec_lpc->mmio_memory_base = EC_LPC_ADDR_MEMMAP;
>>
>> + adev = ACPI_COMPANION(dev);
>> +
>
> The change is irrelevant to the patch.
It looks relevant to me. The companion needs to get set before the
quirk overwrites it.
>
>> @@ -538,7 +577,6 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
>> * Connect a notify handler to process MKBP messages if we have a
>> * companion ACPI device.
>> */
>> - adev = ACPI_COMPANION(dev);
>> if (adev) {
>> status = acpi_install_notify_handler(adev->handle,
>> ACPI_ALL_NOTIFY,
>
> See above comment.
On Mon, Jun 03, 2024 at 11:00:22AM -0500, Mario Limonciello wrote: > On 6/3/2024 04:30, Tzung-Bi Shih wrote: > > On Mon, Jun 03, 2024 at 07:38:32AM +0100, Ben Walsh wrote: > > > @@ -436,6 +463,8 @@ static int cros_ec_lpc_probe(struct platform_device *pdev) > > > ec_lpc->mmio_memory_base = EC_LPC_ADDR_MEMMAP; > > > + adev = ACPI_COMPANION(dev); > > > + > > > > The change is irrelevant to the patch. > > It looks relevant to me. The companion needs to get set before the quirk > overwrites it. Please see code for CROS_EC_LPC_QUIRK_ACPI_ID in the patch, `adev` is going to be overridden soon. I don't see why the ACPI_COMPANION() needs to move here.
Mario Limonciello <mario.limonciello@amd.com> writes: > On 6/3/2024 04:30, Tzung-Bi Shih wrote: >> On Mon, Jun 03, 2024 at 07:38:32AM +0100, Ben Walsh wrote: >>> @@ -436,6 +463,8 @@ static int cros_ec_lpc_probe(struct platform_device *pdev) >>> >>> ec_lpc->mmio_memory_base = EC_LPC_ADDR_MEMMAP; >>> >>> + adev = ACPI_COMPANION(dev); >>> + >> >> The change is irrelevant to the patch. > > It looks relevant to me. The companion needs to get set before the > quirk overwrites it. That was my thinking. You can either update "adev" and use it to refer to the ACPI companion device, or (as Tzung-Bi suggests) use "ACPI_COMPANION" / "ACPI_COMPANION_SET" as needed. I really don't mind either way. I'm happy to change it if necessary.
© 2016 - 2026 Red Hat, Inc.