[PATCH v1 0/4] cros_ec: add support for newer versions of the Framework Laptop

[PATCH v1 0/4] cros_ec: add support for newer versions of the Framework Laptop

[Dustin L. Howett]

This patch series adds support for newer models of the Framework Laptop (both
13" and 16").

Currently-shipping models of the Framework Laptop use a Microchip embedded
controller that conforms to the MEC EC protocol -- that is, data and host
command exchanges occur on I/O ports 0x800 to 0x807 (inclusive). Newer models
have switched to an NPCX embedded controller, which uses the original Chrome EC
"Linear" memory-mapped I/O model.

However, those devices move the MMIO region for EC memory from the original
Chrome EC port range of 0x900-0x9FF to 0xE00-0xEFF. In addition, the ACPI node
for the EC device on these laptops indicates an I/O resource spanning 0x800 to
0x8FE when in truth, the device supports communication all the way through
0x8FF. 

To address these differences without impacting cros_ec_lpcs' compatibility with
Chromebook/Chromebox devices, this patch series adds DMI-match-specific driver
data through which we can detect per-machine "quirks". Quirks indicate changes
in behavior from cros_ec_lpcs' expectation, such as remapping the memory MMIO
window and handling I/O reservation windows differently.

Dustin L. Howett (4):
  cros_ec_lpc: introduce lpc_driver_data, a priv struct for the lpc
    device
  cros_ec_lpc: pass driver_data from DMI down to the device
  cros_ec_lpc: add a quirks system, and propagate quirks from DMI
  cros_ec_lpc: add quirks for the Framework Laptop

 drivers/platform/chrome/cros_ec_lpc.c | 61 ++++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 6 deletions(-)

-- 
2.42.0

[PATCH v1 1/4] cros_ec_lpc: introduce cros_ec_lpc, a priv struct for the lpc device

[Dustin L. Howett]

cros_ec_lpc stores the MMIO port base for EC mapped memory.
cros_ec_lpc_readmem uses this port base instead of hardcoding
EC_LPC_ADDR_MEMMAP.

Signed-off-by: Dustin L. Howett <dustin@howett.net>
---
 drivers/platform/chrome/cros_ec_lpc.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 500a61b093e4..477e22e31757 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -34,6 +34,14 @@
 /* True if ACPI device is present */
 static bool cros_ec_lpc_acpi_device_found;
 
+/**
+ * struct cros_ec_lpc - LPC device-specific data
+ * @mmio_memory_base: The first I/O port addressing EC mapped memory.
+ */
+struct cros_ec_lpc {
+	u16 mmio_memory_base;
+};
+
 /**
  * struct lpc_driver_ops - LPC driver operations
  * @read: Copy length bytes from EC address offset into buffer dest. Returns
@@ -290,6 +298,7 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
 static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset,
 			       unsigned int bytes, void *dest)
 {
+	struct cros_ec_lpc *ec_lpc = ec->priv;
 	int i = offset;
 	char *s = dest;
 	int cnt = 0;
@@ -299,13 +308,13 @@ static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset,
 
 	/* fixed length */
 	if (bytes) {
-		cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + offset, bytes, s);
+		cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + offset, bytes, s);
 		return bytes;
 	}
 
 	/* string */
 	for (; i < EC_MEMMAP_SIZE; i++, s++) {
-		cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + i, 1, s);
+		cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + i, 1, s);
 		cnt++;
 		if (!*s)
 			break;
@@ -353,9 +362,16 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 	struct acpi_device *adev;
 	acpi_status status;
 	struct cros_ec_device *ec_dev;
+	struct cros_ec_lpc *ec_lpc;
 	u8 buf[2] = {};
 	int irq, ret;
 
+	ec_lpc = devm_kzalloc(dev, sizeof(*ec_lpc), GFP_KERNEL);
+	if (!ec_lpc)
+		return -ENOMEM;
+
+	ec_lpc->mmio_memory_base = EC_LPC_ADDR_MEMMAP;
+
 	/*
 	 * The Framework Laptop (and possibly other non-ChromeOS devices)
 	 * only exposes the eight I/O ports that are required for the Microchip EC.
@@ -380,7 +396,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 	cros_ec_lpc_ops.write = cros_ec_lpc_mec_write_bytes;
 	cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2, buf);
 	if (buf[0] != 'E' || buf[1] != 'C') {
-		if (!devm_request_region(dev, EC_LPC_ADDR_MEMMAP, EC_MEMMAP_SIZE,
+		if (!devm_request_region(dev, ec_lpc->mmio_memory_base, EC_MEMMAP_SIZE,
 					 dev_name(dev))) {
 			dev_err(dev, "couldn't reserve memmap region\n");
 			return -EBUSY;
@@ -389,7 +405,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 		/* Re-assign read/write operations for the non MEC variant */
 		cros_ec_lpc_ops.read = cros_ec_lpc_read_bytes;
 		cros_ec_lpc_ops.write = cros_ec_lpc_write_bytes;
-		cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2,
+		cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + EC_MEMMAP_ID, 2,
 				     buf);
 		if (buf[0] != 'E' || buf[1] != 'C') {
 			dev_err(dev, "EC ID not detected\n");
@@ -423,6 +439,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 	ec_dev->din_size = sizeof(struct ec_host_response) +
 			   sizeof(struct ec_response_get_protocol_info);
 	ec_dev->dout_size = sizeof(struct ec_host_request);
+	ec_dev->priv = (void *)ec_lpc;
 
 	/*
 	 * Some boards do not have an IRQ allotted for cros_ec_lpc,
-- 
2.42.0

Re: [PATCH v1 1/4] cros_ec_lpc: introduce cros_ec_lpc, a priv struct for the lpc device

[Tzung-Bi Shih]

On Thu, Oct 05, 2023 at 11:06:59AM -0500, Dustin L. Howett wrote:
> @@ -423,6 +439,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
>  	ec_dev->din_size = sizeof(struct ec_host_response) +
>  			   sizeof(struct ec_response_get_protocol_info);
>  	ec_dev->dout_size = sizeof(struct ec_host_request);
> +	ec_dev->priv = (void *)ec_lpc;

Does it really need the cast?

[PATCH v1 2/4] cros_ec_lpc: pass driver_data from DMI down to the device

[Dustin L. Howett]

lpc_driver_data will be stored in drvdata until probe is complete, at
which point it will be replaced with an updated copy.

Signed-off-by: Dustin L. Howett <dustin@howett.net>
---
 drivers/platform/chrome/cros_ec_lpc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 477e22e31757..ef7943e6a01d 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -615,14 +615,17 @@ 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);
 
+	dmi_match = dmi_first_match(cros_ec_lpc_dmi_table);
+
 	if (!cros_ec_lpc_acpi_device_found &&
-	    !dmi_check_system(cros_ec_lpc_dmi_table)) {
+	    !dmi_match) {
 		pr_err(DRV_NAME ": unsupported system.\n");
 		return -ENODEV;
 	}
@@ -635,6 +638,9 @@ static int __init cros_ec_lpc_init(void)
 	}
 
 	if (!cros_ec_lpc_acpi_device_found) {
+		/* Pass the DMI match's driver data down to the platform device */
+		platform_set_drvdata(&cros_ec_lpc_device, dmi_match->driver_data);
+
 		/* Register the device, and it'll get hooked up automatically */
 		ret = platform_device_register(&cros_ec_lpc_device);
 		if (ret) {
-- 
2.42.0

Re: [PATCH v1 2/4] cros_ec_lpc: pass driver_data from DMI down to the device

[Tzung-Bi Shih]

On Thu, Oct 05, 2023 at 11:07:00AM -0500, Dustin L. Howett wrote:
> @@ -615,14 +615,17 @@ 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);
>  
> +	dmi_match = dmi_first_match(cros_ec_lpc_dmi_table);
> +
>  	if (!cros_ec_lpc_acpi_device_found &&
> -	    !dmi_check_system(cros_ec_lpc_dmi_table)) {
> +	    !dmi_match) {

The `!dmi_match` then can join to previous line.

[PATCH v1 3/4] cros_ec_lpc: add a quirks system, and propagate quirks from DMI

[Dustin L. Howett]

Some devices ship a ChromeOS EC in a non-standard configuration; quirks
allow cros_ec_lpc to account for these non-standard configurations.

It supports the following quirks:
- CROS_EC_LPC_QUIRK_REMAP_MEMORY: use a different port I/O base for
  MMIO to the EC's memory region
- CROS_EC_LPC_QUIRK_SHORT_HOSTCMD_RESERVATION: only attempt to reserve
  0xff (rather than 0x100) I/O ports for the host command region

Signed-off-by: Dustin L. Howett <dustin@howett.net>
---
 drivers/platform/chrome/cros_ec_lpc.c | 41 ++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index ef7943e6a01d..c06575625d2f 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -34,6 +34,27 @@
 /* True if ACPI device is present */
 static bool cros_ec_lpc_acpi_device_found;
 
+/* If this quirk is enabled, the driver will only reserve 0xFF I/O ports
+ * (rather than 0x100) for the host command mapped memory region.
+ */
+#define CROS_EC_LPC_QUIRK_SHORT_HOSTCMD_RESERVATION BIT(0)
+/* If this quirk is enabled, lpc_driver_data.quirk_mmio_memory_base will be used
+ * as the base port for EC mapped memory.
+ */
+#define CROS_EC_LPC_QUIRK_REMAP_MEMORY              BIT(1)
+
+/**
+ * struct lpc_driver_data - driver data attached to a DMI device ID to indicate
+ *                          hardware quirks.
+ * @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 quirks (...REMAP_MEMORY) is set.
+ */
+struct lpc_driver_data {
+	u32 quirks;
+	u16 quirk_mmio_memory_base;
+};
+
 /**
  * struct cros_ec_lpc - LPC device-specific data
  * @mmio_memory_base: The first I/O port addressing EC mapped memory.
@@ -363,14 +384,32 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 	acpi_status status;
 	struct cros_ec_device *ec_dev;
 	struct cros_ec_lpc *ec_lpc;
+	struct lpc_driver_data *driver_data;
+	int region1_size;
 	u8 buf[2] = {};
 	int irq, ret;
+	u32 quirks = 0;
 
 	ec_lpc = devm_kzalloc(dev, sizeof(*ec_lpc), GFP_KERNEL);
 	if (!ec_lpc)
 		return -ENOMEM;
 
 	ec_lpc->mmio_memory_base = EC_LPC_ADDR_MEMMAP;
+	region1_size = EC_HOST_CMD_REGION_SIZE;
+
+	driver_data = platform_get_drvdata(pdev);
+	if (driver_data) {
+		quirks = driver_data->quirks;
+
+		if (quirks)
+			dev_warn(dev, "loaded with quirks %8.08x\n", quirks);
+
+		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_SHORT_HOSTCMD_RESERVATION)
+			region1_size -= 1;
+	}
 
 	/*
 	 * The Framework Laptop (and possibly other non-ChromeOS devices)
@@ -420,7 +459,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 			return -EBUSY;
 		}
 		if (!devm_request_region(dev, EC_HOST_CMD_REGION1,
-					 EC_HOST_CMD_REGION_SIZE, dev_name(dev))) {
+					 region1_size, dev_name(dev))) {
 			dev_err(dev, "couldn't reserve region1\n");
 			return -EBUSY;
 		}
-- 
2.42.0

Re: [PATCH v1 3/4] cros_ec_lpc: add a quirks system, and propagate quirks from DMI

[Tzung-Bi Shih]

On Thu, Oct 05, 2023 at 11:07:01AM -0500, Dustin L. Howett wrote:
> - CROS_EC_LPC_QUIRK_SHORT_HOSTCMD_RESERVATION: only attempt to reserve
>   0xff (rather than 0x100) I/O ports for the host command region

What if it still reserves 0x100 I/O ports for these deivces?

> @@ -34,6 +34,27 @@
>  /* True if ACPI device is present */
>  static bool cros_ec_lpc_acpi_device_found;
>  
> +/* If this quirk is enabled, the driver will only reserve 0xFF I/O ports
> + * (rather than 0x100) for the host command mapped memory region.
> + */
[...]
> +/* If this quirk is enabled, lpc_driver_data.quirk_mmio_memory_base will be used
> + * as the base port for EC mapped memory.
> + */

See https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting

> @@ -363,14 +384,32 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
>  	acpi_status status;
>  	struct cros_ec_device *ec_dev;
>  	struct cros_ec_lpc *ec_lpc;
> +	struct lpc_driver_data *driver_data;
> +	int region1_size;
>  	u8 buf[2] = {};
>  	int irq, ret;
> +	u32 quirks = 0;
>  
>  	ec_lpc = devm_kzalloc(dev, sizeof(*ec_lpc), GFP_KERNEL);
>  	if (!ec_lpc)
>  		return -ENOMEM;
>  
>  	ec_lpc->mmio_memory_base = EC_LPC_ADDR_MEMMAP;
> +	region1_size = EC_HOST_CMD_REGION_SIZE;

`quirks` doesn't need to initialize; or move it to a more limited scope.
Instead, `region1_size` can just initialize to EC_HOST_CMD_REGION_SIZE.

Re: [PATCH v1 3/4] cros_ec_lpc: add a quirks system, and propagate quirks from DMI

[Thomas Weißschuh]

On 2023-10-05 11:07:01-0500, Dustin L. Howett wrote:
> Some devices ship a ChromeOS EC in a non-standard configuration; quirks
> allow cros_ec_lpc to account for these non-standard configurations.
> 
> It supports the following quirks:
> - CROS_EC_LPC_QUIRK_REMAP_MEMORY: use a different port I/O base for
>   MMIO to the EC's memory region
> - CROS_EC_LPC_QUIRK_SHORT_HOSTCMD_RESERVATION: only attempt to reserve
>   0xff (rather than 0x100) I/O ports for the host command region
> 
> Signed-off-by: Dustin L. Howett <dustin@howett.net>

[..]

> +
> +	driver_data = platform_get_drvdata(pdev);
> +	if (driver_data) {
> +		quirks = driver_data->quirks;
> +
> +		if (quirks)
> +			dev_warn(dev, "loaded with quirks %8.08x\n", quirks);

dev_warn() is too loud.

[..]

[PATCH v1 4/4] cros_ec_lpc: add quirks for the Framework Laptop

[Dustin L. Howett]

The original Framework Laptop contains a Microchip EC, but the newer
ones have switched to an NPCX EC which uses the standard linear MMIO
protocol for host commands and EC memory.

However, those newer Framework Laptops diverge from the standard
ChromeOS EC in two important ways:

1. The ACPI table for PNP0C09 only claims I/O ports 0x800 to 0x8FE
   instead of the traditional 0x8FF
2. The MMIO region for EC memory begins at port 0xE00 rather than the
   traditional 0x900.

cros_ec_lpc's quirks system addresses both of these issues.

Signed-off-by: Dustin L. Howett <dustin@howett.net>
---
 drivers/platform/chrome/cros_ec_lpc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index c06575625d2f..66285c66ff22 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -537,6 +537,13 @@ static const struct acpi_device_id cros_ec_lpc_acpi_device_ids[] = {
 };
 MODULE_DEVICE_TABLE(acpi, cros_ec_lpc_acpi_device_ids);
 
+static const struct lpc_driver_data framework_laptop_lpc_driver_data __initconst = {
+	.quirks =
+		CROS_EC_LPC_QUIRK_REMAP_MEMORY |
+		CROS_EC_LPC_QUIRK_SHORT_HOSTCMD_RESERVATION,
+	.quirk_mmio_memory_base = 0xE00,
+};
+
 static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
 	{
 		/*
@@ -596,6 +603,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_lpc_driver_data,
 	},
 	{ /* sentinel */ }
 };
-- 
2.42.0

Re: [PATCH v1 4/4] cros_ec_lpc: add quirks for the Framework Laptop

[Mario Limonciello]

On 10/5/2023 11:07, Dustin L. Howett wrote:
> The original Framework Laptop contains a Microchip EC, but the newer
> ones have switched to an NPCX EC which uses the standard linear MMIO
> protocol for host commands and EC memory.
> 
> However, those newer Framework Laptops diverge from the standard
> ChromeOS EC in two important ways:
> 
> 1. The ACPI table for PNP0C09 only claims I/O ports 0x800 to 0x8FE
>     instead of the traditional 0x8FF
> 2. The MMIO region for EC memory begins at port 0xE00 rather than the
>     traditional 0x900.
> 
> cros_ec_lpc's quirks system addresses both of these issues.
> 
> Signed-off-by: Dustin L. Howett <dustin@howett.net>
> ---
>   drivers/platform/chrome/cros_ec_lpc.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index c06575625d2f..66285c66ff22 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -537,6 +537,13 @@ static const struct acpi_device_id cros_ec_lpc_acpi_device_ids[] = {
>   };
>   MODULE_DEVICE_TABLE(acpi, cros_ec_lpc_acpi_device_ids);
>   
> +static const struct lpc_driver_data framework_laptop_lpc_driver_data __initconst = {
> +	.quirks =
> +		CROS_EC_LPC_QUIRK_REMAP_MEMORY |
> +		CROS_EC_LPC_QUIRK_SHORT_HOSTCMD_RESERVATION,
> +	.quirk_mmio_memory_base = 0xE00,
> +};

I think it would be really helpful for anyone that looks at this commit 
and scratches their head to have a decoder here as a comment along these 
lines to explain how this works (correct it for any inaccuracies please):

/*
  * 13" Gen 11 Intel uses Microchip EC
  * 13" Gen 12 Intel and newer use NPCX EC
  * 13"/16" Gen 1 AMD and newer use NPCX EC
  * These quirks set up the values used in cros_ec_lpc_probe() to be 
able to discover them properly with accordance to the fallback probing 
logic.
  */

If those comments grow stale because Gen 14 Intel or Gen 2 AMD change 
offsets or vendors, then the comment can be changed too, or a second 
quirk be introduced.

In the event that these need to change again in any way in the future 
models, it may be better to introduce a _DSD in the BIOS to allow the 
kernel to be able to discover values that would otherwise be included in 
a quirk.

> +
>   static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
>   	{
>   		/*
> @@ -596,6 +603,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_lpc_driver_data,
>   	},
>   	{ /* sentinel */ }
>   };

Re: [PATCH v1 4/4] cros_ec_lpc: add quirks for the Framework Laptop

[Tzung-Bi Shih]

On Thu, Oct 05, 2023 at 11:07:02AM -0500, Dustin L. Howett wrote:
> @@ -596,6 +603,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_lpc_driver_data,

Does it really need the cast?

How about the devices with MEC?  It looks like all Framework Laptop applies
the quirks by the patch.

Re: [PATCH v1 0/4] cros_ec: add support for newer versions of the Framework Laptop

[Tzung-Bi Shih]

On Thu, Oct 05, 2023 at 11:06:57AM -0500, Dustin L. Howett wrote:
> However, those devices move the MMIO region for EC memory from the original
> Chrome EC port range of 0x900-0x9FF to 0xE00-0xEFF. In addition, the ACPI node
> for the EC device on these laptops indicates an I/O resource spanning 0x800 to
> 0x8FE when in truth, the device supports communication all the way through
> 0x8FF. 

I don't understand the description about 0x8FF.  From patches in the series,
it looks like 0x8FF is unused if CROS_EC_LPC_QUIRK_SHORT_HOSTCMD_RESERVATION.

> Dustin L. Howett (4):
>   cros_ec_lpc: introduce lpc_driver_data, a priv struct for the lpc
>     device
>   cros_ec_lpc: pass driver_data from DMI down to the device
>   cros_ec_lpc: add a quirks system, and propagate quirks from DMI
>   cros_ec_lpc: add quirks for the Framework Laptop

The series should prefix with "platform/chrome: cros_ec_lpc".

Re: [PATCH v1 0/4] cros_ec: add support for newer versions of the Framework Laptop

[Dustin Howett]

On Wed, Oct 11, 2023 at 12:29 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
>
> On Thu, Oct 05, 2023 at 11:06:57AM -0500, Dustin L. Howett wrote:
> I don't understand the description about 0x8FF.  From patches in the series,
> it looks like 0x8FF is unused if CROS_EC_LPC_QUIRK_SHORT_HOSTCMD_RESERVATION.

You're right, this is unclear.

The ACPI node for [...]EC0 only claims to use 0x800 to 0x8FE -
*however*, the EC does use 0x8FF!
The bug is only in the ACPI table.
The quirk is used so that devm_request_region doesn't fail
because it would try to reserve up to 0x8FF.

Does that make it clearer? I have been struggling to reword this for a
while. Hah.
I tried to explain this better in the patch bodies.

I have been considering a different form for this quirk.
Right now, it means "do not reserve 0x8ff, but continue to USE 0x8ff".
Perhaps "ignore devm_request_region failures" is a better quirk? It
felt too broad.

>
> > Dustin L. Howett (4):
> > [...]
>
> The series should prefix with "platform/chrome: cros_ec_lpc".

Fixed. Sending an updated patch series that addresses yours and other
folks' comments across the entire series.

Thanks for the review,
d

Re: [PATCH v1 0/4] cros_ec: add support for newer versions of the Framework Laptop

[Tzung-Bi Shih]

On Sun, Nov 26, 2023 at 01:22:23PM -0600, Dustin Howett wrote:
> On Wed, Oct 11, 2023 at 12:29 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >
> > On Thu, Oct 05, 2023 at 11:06:57AM -0500, Dustin L. Howett wrote:
> > I don't understand the description about 0x8FF.  From patches in the series,
> > it looks like 0x8FF is unused if CROS_EC_LPC_QUIRK_SHORT_HOSTCMD_RESERVATION.
> 
> You're right, this is unclear.
> 
> The ACPI node for [...]EC0 only claims to use 0x800 to 0x8FE -
> *however*, the EC does use 0x8FF!
> The bug is only in the ACPI table.
> The quirk is used so that devm_request_region doesn't fail
> because it would try to reserve up to 0x8FF.

I am not familiar to ACPI, so this may be a dumb question: if the bug is in
the ACPI table, is it possible to correct the table to claim to use 0x800
to 0x8FF?

> I have been considering a different form for this quirk.
> Right now, it means "do not reserve 0x8ff, but continue to USE 0x8ff".
> Perhaps "ignore devm_request_region failures" is a better quirk? It
> felt too broad.

Either way is suboptimal to me.

[PATCH v2 0/4] platform/chrome: cros_ec_lpc: add support for AMD Framework Laptops

[Dustin L. Howett]

This patch series adds support for the AMD models of the Framework
Laptop (both 13" and 16").

These models of Framework Laptop have switched to an NPCX embedded
controller, which uses the original Chrome EC linear memory-mapped I/O
model.

However, these devices are not configured in a way that is compatible
with the cros_ec driver. Instead of mapping EC memory to I/O ports
[0x900, 0x9FF], they map it to ports [0xE00, 0xEFF]. Furthermore, the
ACPI node for the embedded controller incorrectly indicates that it does
not use port 0x8FF for host commands. It does use port 0x8FF.

To address these issues without impacting cros_ec_lpcs' compatibility
with Chromebook/Chromebox devices or the existing Framework Laptop
platform, these patches add DMI match-specific driver data through
which we can detect per-machine "quirks".

Quirks toggle changes in cros_ec_lpcs' behavior, such as remapping the
memory MMIO window and reserving I/O ports differently.

Changes in v2:
* Separated Framework Laptop (Intel) and Framework Laptop (AMD)
* dev_warn has been demoted to dev_info
* Style fixes
* Reworded the individual patch bodies for clarity and style

Dustin L. Howett (4):
  platform/chrome: cros_ec_lpc: introduce a priv struct for the lpc
    device
  platform/chrome: cros_ec_lpc: pass driver_data from DMI to the device
  platform/chrome: cros_ec_lpc: add a "quirks" system
  platform/chrome: cros_ec_lpc: add quirks for the Framework Laptop
    (AMD)

 drivers/platform/chrome/cros_ec_lpc.c | 94 ++++++++++++++++++++++++---
 1 file changed, 86 insertions(+), 8 deletions(-)

-- 
2.42.0

[PATCH v2 1/4] platform/chrome: cros_ec_lpc: introduce a priv struct for the lpc device

[Dustin L. Howett]

lpc_driver_data stores the MMIO port base for EC mapped memory.
cros_ec_lpc_readmem uses this port base instead of hardcoding
EC_LPC_ADDR_MEMMAP.

Signed-off-by: Dustin L. Howett <dustin@howett.net>
---
 drivers/platform/chrome/cros_ec_lpc.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 356572452898..9f2ea75c76b6 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -34,6 +34,14 @@
 /* True if ACPI device is present */
 static bool cros_ec_lpc_acpi_device_found;
 
+/**
+ * struct cros_ec_lpc - LPC device-specific data
+ * @mmio_memory_base: The first I/O port addressing EC mapped memory.
+ */
+struct cros_ec_lpc {
+	u16 mmio_memory_base;
+};
+
 /**
  * struct lpc_driver_ops - LPC driver operations
  * @read: Copy length bytes from EC address offset into buffer dest. Returns
@@ -290,6 +298,7 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
 static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset,
 			       unsigned int bytes, void *dest)
 {
+	struct cros_ec_lpc *ec_lpc = ec->priv;
 	int i = offset;
 	char *s = dest;
 	int cnt = 0;
@@ -299,13 +308,13 @@ static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset,
 
 	/* fixed length */
 	if (bytes) {
-		cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + offset, bytes, s);
+		cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + offset, bytes, s);
 		return bytes;
 	}
 
 	/* string */
 	for (; i < EC_MEMMAP_SIZE; i++, s++) {
-		cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + i, 1, s);
+		cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + i, 1, s);
 		cnt++;
 		if (!*s)
 			break;
@@ -353,9 +362,16 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 	struct acpi_device *adev;
 	acpi_status status;
 	struct cros_ec_device *ec_dev;
+	struct cros_ec_lpc *ec_lpc;
 	u8 buf[2] = {};
 	int irq, ret;
 
+	ec_lpc = devm_kzalloc(dev, sizeof(*ec_lpc), GFP_KERNEL);
+	if (!ec_lpc)
+		return -ENOMEM;
+
+	ec_lpc->mmio_memory_base = EC_LPC_ADDR_MEMMAP;
+
 	/*
 	 * The Framework Laptop (and possibly other non-ChromeOS devices)
 	 * only exposes the eight I/O ports that are required for the Microchip EC.
@@ -380,7 +396,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 	cros_ec_lpc_ops.write = cros_ec_lpc_mec_write_bytes;
 	cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2, buf);
 	if (buf[0] != 'E' || buf[1] != 'C') {
-		if (!devm_request_region(dev, EC_LPC_ADDR_MEMMAP, EC_MEMMAP_SIZE,
+		if (!devm_request_region(dev, ec_lpc->mmio_memory_base, EC_MEMMAP_SIZE,
 					 dev_name(dev))) {
 			dev_err(dev, "couldn't reserve memmap region\n");
 			return -EBUSY;
@@ -389,7 +405,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 		/* Re-assign read/write operations for the non MEC variant */
 		cros_ec_lpc_ops.read = cros_ec_lpc_read_bytes;
 		cros_ec_lpc_ops.write = cros_ec_lpc_write_bytes;
-		cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2,
+		cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + EC_MEMMAP_ID, 2,
 				     buf);
 		if (buf[0] != 'E' || buf[1] != 'C') {
 			dev_err(dev, "EC ID not detected\n");
@@ -423,6 +439,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 	ec_dev->din_size = sizeof(struct ec_host_response) +
 			   sizeof(struct ec_response_get_protocol_info);
 	ec_dev->dout_size = sizeof(struct ec_host_request);
+	ec_dev->priv = ec_lpc;
 
 	/*
 	 * Some boards do not have an IRQ allotted for cros_ec_lpc,
-- 
2.42.0

[PATCH v2 2/4] platform/chrome: cros_ec_lpc: pass driver_data from DMI to the device

[Dustin L. Howett]

lpc_driver_data will be stored in drvdata until probe is complete, at
which point it will be replaced with a cros_ec_device.

Signed-off-by: Dustin L. Howett <dustin@howett.net>
---
 drivers/platform/chrome/cros_ec_lpc.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 9f2ea75c76b6..f1d1615d9b37 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -615,14 +615,16 @@ 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);
 
-	if (!cros_ec_lpc_acpi_device_found &&
-	    !dmi_check_system(cros_ec_lpc_dmi_table)) {
+	dmi_match = dmi_first_match(cros_ec_lpc_dmi_table);
+
+	if (!cros_ec_lpc_acpi_device_found && !dmi_match) {
 		pr_err(DRV_NAME ": unsupported system.\n");
 		return -ENODEV;
 	}
@@ -635,6 +637,9 @@ static int __init cros_ec_lpc_init(void)
 	}
 
 	if (!cros_ec_lpc_acpi_device_found) {
+		/* Pass the DMI match's driver data down to the platform device */
+		platform_set_drvdata(&cros_ec_lpc_device, dmi_match->driver_data);
+
 		/* Register the device, and it'll get hooked up automatically */
 		ret = platform_device_register(&cros_ec_lpc_device);
 		if (ret) {
-- 
2.42.0

[PATCH v2 3/4] platform/chrome: cros_ec_lpc: add a "quirks" system

[Dustin L. Howett]

Some devices ship a ChromeOS EC in a non-standard configuration. Quirks
allow cros_ec_lpc to account for these non-standard configurations.

It supports the following quirks:
- CROS_EC_LPC_QUIRK_REMAP_MEMORY: use a different port I/O base for
  MMIO to the EC's memory region
- CROS_EC_LPC_QUIRK_SHORT_HOSTCMD_RESERVATION: only attempt to reserve
  0xff (rather than 0x100) I/O ports for the host command region

Signed-off-by: Dustin L. Howett <dustin@howett.net>
---
 drivers/platform/chrome/cros_ec_lpc.c | 42 ++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index f1d1615d9b37..a65c9a8bca5e 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -34,6 +34,29 @@
 /* True if ACPI device is present */
 static bool cros_ec_lpc_acpi_device_found;
 
+/*
+ * Indicates that the driver should only reserve 0xFF I/O ports
+ * (rather than 0x100) for the host command mapped memory region.
+ */
+#define CROS_EC_LPC_QUIRK_SHORT_HOSTCMD_RESERVATION BIT(0)
+/*
+ * Indicates that lpc_driver_data.quirk_mmio_memory_base should
+ * be used as the base port for EC mapped memory.
+ */
+#define CROS_EC_LPC_QUIRK_REMAP_MEMORY              BIT(1)
+
+/**
+ * struct lpc_driver_data - driver data attached to a DMI device ID to indicate
+ *                          hardware quirks.
+ * @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 quirks (...REMAP_MEMORY) is set.
+ */
+struct lpc_driver_data {
+	u32 quirks;
+	u16 quirk_mmio_memory_base;
+};
+
 /**
  * struct cros_ec_lpc - LPC device-specific data
  * @mmio_memory_base: The first I/O port addressing EC mapped memory.
@@ -363,8 +386,11 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 	acpi_status status;
 	struct cros_ec_device *ec_dev;
 	struct cros_ec_lpc *ec_lpc;
+	struct lpc_driver_data *driver_data;
+	int region1_size = EC_HOST_CMD_REGION_SIZE;
 	u8 buf[2] = {};
 	int irq, ret;
+	u32 quirks = 0;
 
 	ec_lpc = devm_kzalloc(dev, sizeof(*ec_lpc), GFP_KERNEL);
 	if (!ec_lpc)
@@ -372,6 +398,20 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 
 	ec_lpc->mmio_memory_base = EC_LPC_ADDR_MEMMAP;
 
+	driver_data = platform_get_drvdata(pdev);
+	if (driver_data) {
+		quirks = driver_data->quirks;
+
+		if (quirks)
+			dev_info(dev, "loaded with quirks %8.08x\n", quirks);
+
+		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_SHORT_HOSTCMD_RESERVATION)
+			region1_size -= 1;
+	}
+
 	/*
 	 * The Framework Laptop (and possibly other non-ChromeOS devices)
 	 * only exposes the eight I/O ports that are required for the Microchip EC.
@@ -420,7 +460,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 			return -EBUSY;
 		}
 		if (!devm_request_region(dev, EC_HOST_CMD_REGION1,
-					 EC_HOST_CMD_REGION_SIZE, dev_name(dev))) {
+					 region1_size, dev_name(dev))) {
 			dev_err(dev, "couldn't reserve region1\n");
 			return -EBUSY;
 		}
-- 
2.42.0

Re: [PATCH v2 3/4] platform/chrome: cros_ec_lpc: add a "quirks" system

[Tzung-Bi Shih]

On Sun, Nov 26, 2023 at 01:24:51PM -0600, Dustin L. Howett wrote:
> @@ -363,8 +386,11 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
>  	acpi_status status;
>  	struct cros_ec_device *ec_dev;
>  	struct cros_ec_lpc *ec_lpc;
> +	struct lpc_driver_data *driver_data;
> +	int region1_size = EC_HOST_CMD_REGION_SIZE;
>  	u8 buf[2] = {};
>  	int irq, ret;
> +	u32 quirks = 0;
>  
>  	ec_lpc = devm_kzalloc(dev, sizeof(*ec_lpc), GFP_KERNEL);
>  	if (!ec_lpc)
> @@ -372,6 +398,20 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
>  
>  	ec_lpc->mmio_memory_base = EC_LPC_ADDR_MEMMAP;
>  
> +	driver_data = platform_get_drvdata(pdev);
> +	if (driver_data) {
> +		quirks = driver_data->quirks;
> +

From readability's perspective:
If seeing a variable is initialized, the code reader would expect: "there must
be some reasons" (at least for me).  For the case, I think the reason is
unobvious.

[PATCH v2 4/4] platform/chrome: cros_ec_lpc: add quirks for the Framework Laptop (AMD)

[Dustin L. Howett]

The original Framework Laptop 13 platform (Intel 11th, 12th, and 13th
Generation at this time) uses a Microchip embedded controller in a
standard configuration.

The newer devices in this product line--Framework Laptop 13 and 16 (AMD
Ryzen)--use a NPCX embedded controller. However, they deviate from the
configuration of ChromeOS platforms built with the NPCX EC.

* ACPI device _SB.PCI0.LPC0.EC0 only indicates I/O ports [0x800, 0x8FE]
  as being used, rather than the expected [0x800, 0x8FF].
  *However*, the embedded controller does make use of all 0x100 ports.
* The MMIO region for EC memory begins at port 0xE00 rather than the
  expected 0x900.

cros_ec_lpc's quirks system is used to address both of these issues.

Signed-off-by: Dustin L. Howett <dustin@howett.net>
---
 drivers/platform/chrome/cros_ec_lpc.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index a65c9a8bca5e..9373c39c3ed9 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -538,6 +538,13 @@ static const struct acpi_device_id cros_ec_lpc_acpi_device_ids[] = {
 };
 MODULE_DEVICE_TABLE(acpi, cros_ec_lpc_acpi_device_ids);
 
+static const struct lpc_driver_data framework_laptop_amd_lpc_driver_data __initconst = {
+	.quirks =
+		CROS_EC_LPC_QUIRK_REMAP_MEMORY |
+		CROS_EC_LPC_QUIRK_SHORT_HOSTCMD_RESERVATION,
+	.quirk_mmio_memory_base = 0xE00,
+};
+
 static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
 	{
 		/*
@@ -592,7 +599,16 @@ static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
 	},
 	/* A small number of non-Chromebook/box machines also use the ChromeOS EC */
 	{
-		/* the Framework Laptop */
+		/* the Framework Laptop 13 (AMD Ryzen) and 16 (AMD Ryzen) */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "AMD Ryzen"),
+			DMI_MATCH(DMI_PRODUCT_FAMILY, "Laptop"),
+		},
+		.driver_data = (void *)&framework_laptop_amd_lpc_driver_data,
+	},
+	{
+		/* the Framework Laptop (Intel 11th, 12th, 13th Generation) */
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
 			DMI_MATCH(DMI_PRODUCT_NAME, "Laptop"),
-- 
2.42.0

Re: [PATCH v2 0/4] platform/chrome: cros_ec_lpc: add support for AMD Framework Laptops

[Thomas Weißschuh]

Hi Dustin,

On 2023-11-26 13:24:48-0600, Dustin L. Howett wrote:
> This patch series adds support for the AMD models of the Framework
> Laptop (both 13" and 16").
> 
> These models of Framework Laptop have switched to an NPCX embedded
> controller, which uses the original Chrome EC linear memory-mapped I/O
> model.
> 
> However, these devices are not configured in a way that is compatible
> with the cros_ec driver. Instead of mapping EC memory to I/O ports
> [0x900, 0x9FF], they map it to ports [0xE00, 0xEFF]. Furthermore, the
> ACPI node for the embedded controller incorrectly indicates that it does
> not use port 0x8FF for host commands. It does use port 0x8FF.

Do you know if this incompatibility is intended, a technical necessity
and/or possible to be fixed upstream?

> To address these issues without impacting cros_ec_lpcs' compatibility
> with Chromebook/Chromebox devices or the existing Framework Laptop
> platform, these patches add DMI match-specific driver data through
> which we can detect per-machine "quirks".
> 
> Quirks toggle changes in cros_ec_lpcs' behavior, such as remapping the
> memory MMIO window and reserving I/O ports differently.
> 
> Changes in v2:
> * Separated Framework Laptop (Intel) and Framework Laptop (AMD)
> * dev_warn has been demoted to dev_info
> * Style fixes
> * Reworded the individual patch bodies for clarity and style
> 
> Dustin L. Howett (4):
>   platform/chrome: cros_ec_lpc: introduce a priv struct for the lpc
>     device
>   platform/chrome: cros_ec_lpc: pass driver_data from DMI to the device
>   platform/chrome: cros_ec_lpc: add a "quirks" system
>   platform/chrome: cros_ec_lpc: add quirks for the Framework Laptop
>     (AMD)
> 
>  drivers/platform/chrome/cros_ec_lpc.c | 94 ++++++++++++++++++++++++---
>  1 file changed, 86 insertions(+), 8 deletions(-)

[PATCH v3 0/4] platform/chrome: cros_ec_lpc: add support for AMD Framework Laptops

[Dustin L. Howett]

This patch series adds support for the AMD models of the Framework
Laptop (both 13" and 16").

These models of Framework Laptop have switched to an NPCX embedded
controller, which uses the original Chrome EC linear memory-mapped I/O
model.

However, these devices are not configured in a way that is compatible
with the cros_ec driver. Instead of mapping EC memory to I/O ports
[0x900, 0x9FF], they map it to ports [0xE00, 0xEFF].

To address this difference without impacting cros_ec_lpcs' compatibility
with Chromebook/Chromebox devices or the existing Framework Laptop
platform, these patches add DMI match-specific driver data through which
we can detect per-machine "quirks".

Quirks toggle changes in cros_ec_lpcs' behavior, such as remapping the
memory MMIO window.

Changes in v2:
* Separated Framework Laptop (Intel) and Framework Laptop (AMD)
* dev_warn has been demoted to dev_info
* Style fixes
* Reworded the individual patch bodies for clarity and style

Changes in v3:
* Addressed dev_warn feedback from Thomas Weissschuh
* Removed the host command region quirk; Framework Computer is slated to
  release a firmware update that addresses the I/O region issue from
  this series' v1 and v2. This leaves just one quirk, with the potential
  for future quirks (e.g. changes required to reduce EC bus contention
  on the Intel Framework Laptops)


Dustin L. Howett (4):
  platform/chrome: cros_ec_lpc: introduce a priv struct for the lpc
    device
  platform/chrome: cros_ec_lpc: pass driver_data from DMI to the device
  platform/chrome: cros_ec_lpc: add a "quirks" system
  platform/chrome: cros_ec_lpc: add quirks for the Framework Laptop
    (AMD)

 drivers/platform/chrome/cros_ec_lpc.c | 81 ++++++++++++++++++++++++---
 1 file changed, 74 insertions(+), 7 deletions(-)

-- 
2.43.0

[PATCH v3 1/4] platform/chrome: cros_ec_lpc: introduce a priv struct for the lpc device

[Dustin L. Howett]

lpc_driver_data stores the MMIO port base for EC mapped memory.
cros_ec_lpc_readmem uses this port base instead of hardcoding
EC_LPC_ADDR_MEMMAP.

Signed-off-by: Dustin L. Howett <dustin@howett.net>
---
 drivers/platform/chrome/cros_ec_lpc.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index f0f3d3d56157..5e2856c5185b 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -34,6 +34,14 @@
 /* True if ACPI device is present */
 static bool cros_ec_lpc_acpi_device_found;
 
+/**
+ * struct cros_ec_lpc - LPC device-specific data
+ * @mmio_memory_base: The first I/O port addressing EC mapped memory.
+ */
+struct cros_ec_lpc {
+	u16 mmio_memory_base;
+};
+
 /**
  * struct lpc_driver_ops - LPC driver operations
  * @read: Copy length bytes from EC address offset into buffer dest. Returns
@@ -290,6 +298,7 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
 static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset,
 			       unsigned int bytes, void *dest)
 {
+	struct cros_ec_lpc *ec_lpc = ec->priv;
 	int i = offset;
 	char *s = dest;
 	int cnt = 0;
@@ -299,13 +308,13 @@ static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset,
 
 	/* fixed length */
 	if (bytes) {
-		cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + offset, bytes, s);
+		cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + offset, bytes, s);
 		return bytes;
 	}
 
 	/* string */
 	for (; i < EC_MEMMAP_SIZE; i++, s++) {
-		cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + i, 1, s);
+		cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + i, 1, s);
 		cnt++;
 		if (!*s)
 			break;
@@ -353,9 +362,16 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 	struct acpi_device *adev;
 	acpi_status status;
 	struct cros_ec_device *ec_dev;
+	struct cros_ec_lpc *ec_lpc;
 	u8 buf[2] = {};
 	int irq, ret;
 
+	ec_lpc = devm_kzalloc(dev, sizeof(*ec_lpc), GFP_KERNEL);
+	if (!ec_lpc)
+		return -ENOMEM;
+
+	ec_lpc->mmio_memory_base = EC_LPC_ADDR_MEMMAP;
+
 	/*
 	 * The Framework Laptop (and possibly other non-ChromeOS devices)
 	 * only exposes the eight I/O ports that are required for the Microchip EC.
@@ -380,7 +396,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 	cros_ec_lpc_ops.write = cros_ec_lpc_mec_write_bytes;
 	cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2, buf);
 	if (buf[0] != 'E' || buf[1] != 'C') {
-		if (!devm_request_region(dev, EC_LPC_ADDR_MEMMAP, EC_MEMMAP_SIZE,
+		if (!devm_request_region(dev, ec_lpc->mmio_memory_base, EC_MEMMAP_SIZE,
 					 dev_name(dev))) {
 			dev_err(dev, "couldn't reserve memmap region\n");
 			return -EBUSY;
@@ -389,7 +405,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 		/* Re-assign read/write operations for the non MEC variant */
 		cros_ec_lpc_ops.read = cros_ec_lpc_read_bytes;
 		cros_ec_lpc_ops.write = cros_ec_lpc_write_bytes;
-		cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2,
+		cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + EC_MEMMAP_ID, 2,
 				     buf);
 		if (buf[0] != 'E' || buf[1] != 'C') {
 			dev_err(dev, "EC ID not detected\n");
@@ -423,6 +439,7 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 	ec_dev->din_size = sizeof(struct ec_host_response) +
 			   sizeof(struct ec_response_get_protocol_info);
 	ec_dev->dout_size = sizeof(struct ec_host_request);
+	ec_dev->priv = ec_lpc;
 
 	/*
 	 * Some boards do not have an IRQ allotted for cros_ec_lpc,
-- 
2.43.0

[PATCH v3 2/4] platform/chrome: cros_ec_lpc: pass driver_data from DMI to the device

[Dustin L. Howett]

lpc_driver_data will be stored in drvdata until probe is complete.

Signed-off-by: Dustin L. Howett <dustin@howett.net>
---
 drivers/platform/chrome/cros_ec_lpc.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 5e2856c5185b..b3aa60e0feb3 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -627,14 +627,16 @@ 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);
 
-	if (!cros_ec_lpc_acpi_device_found &&
-	    !dmi_check_system(cros_ec_lpc_dmi_table)) {
+	dmi_match = dmi_first_match(cros_ec_lpc_dmi_table);
+
+	if (!cros_ec_lpc_acpi_device_found && !dmi_match) {
 		pr_err(DRV_NAME ": unsupported system.\n");
 		return -ENODEV;
 	}
@@ -647,6 +649,9 @@ static int __init cros_ec_lpc_init(void)
 	}
 
 	if (!cros_ec_lpc_acpi_device_found) {
+		/* Pass the DMI match's driver data down to the platform device */
+		platform_set_drvdata(&cros_ec_lpc_device, dmi_match->driver_data);
+
 		/* Register the device, and it'll get hooked up automatically */
 		ret = platform_device_register(&cros_ec_lpc_device);
 		if (ret) {
-- 
2.43.0

[PATCH v3 3/4] platform/chrome: cros_ec_lpc: add a "quirks" system

[Dustin L. Howett]

Some devices ship a ChromeOS EC in a non-standard configuration. Quirks
allow cros_ec_lpc to account for these non-standard configurations.

It only supports one quirk right now:
- CROS_EC_LPC_QUIRK_REMAP_MEMORY: use a different port I/O base for
  MMIO to the EC's memory region

Signed-off-by: Dustin L. Howett <dustin@howett.net>
---
 drivers/platform/chrome/cros_ec_lpc.c | 31 +++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index b3aa60e0feb3..087131f159d4 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -34,6 +34,24 @@
 /* True if ACPI device is present */
 static bool cros_ec_lpc_acpi_device_found;
 
+/*
+ * Indicates that lpc_driver_data.quirk_mmio_memory_base should
+ * be used as the base port for EC mapped memory.
+ */
+#define CROS_EC_LPC_QUIRK_REMAP_MEMORY              BIT(0)
+
+/**
+ * struct lpc_driver_data - driver data attached to a DMI device ID to indicate
+ *                          hardware quirks.
+ * @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.)
+ */
+struct lpc_driver_data {
+	u32 quirks;
+	u16 quirk_mmio_memory_base;
+};
+
 /**
  * struct cros_ec_lpc - LPC device-specific data
  * @mmio_memory_base: The first I/O port addressing EC mapped memory.
@@ -363,8 +381,10 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 	acpi_status status;
 	struct cros_ec_device *ec_dev;
 	struct cros_ec_lpc *ec_lpc;
+	struct lpc_driver_data *driver_data;
 	u8 buf[2] = {};
 	int irq, ret;
+	u32 quirks;
 
 	ec_lpc = devm_kzalloc(dev, sizeof(*ec_lpc), GFP_KERNEL);
 	if (!ec_lpc)
@@ -372,6 +392,17 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 
 	ec_lpc->mmio_memory_base = EC_LPC_ADDR_MEMMAP;
 
+	driver_data = platform_get_drvdata(pdev);
+	if (driver_data) {
+		quirks = driver_data->quirks;
+
+		if (quirks)
+			dev_info(dev, "loaded with quirks %8.08x\n", quirks);
+
+		if (quirks & CROS_EC_LPC_QUIRK_REMAP_MEMORY)
+			ec_lpc->mmio_memory_base = driver_data->quirk_mmio_memory_base;
+	}
+
 	/*
 	 * The Framework Laptop (and possibly other non-ChromeOS devices)
 	 * only exposes the eight I/O ports that are required for the Microchip EC.
-- 
2.43.0

[PATCH v3 4/4] platform/chrome: cros_ec_lpc: add quirks for the Framework Laptop (AMD)

[Dustin L. Howett]

The original Framework Laptop 13 platform (Intel 11th, 12th, and 13th
Generation at this time) uses a Microchip embedded controller in a
standard configuration.

The newer devices in this product line--Framework Laptop 13 and 16 (AMD
Ryzen)--use a NPCX embedded controller. However, they deviate from the
configuration of ChromeOS platforms built with the NPCX EC.

* The MMIO region for EC memory begins at port 0xE00 rather than the
  expected 0x900.

cros_ec_lpc's quirks system is used to address this issue.

Signed-off-by: Dustin L. Howett <dustin@howett.net>
---
 drivers/platform/chrome/cros_ec_lpc.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 087131f159d4..beaf714e8568 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -527,6 +527,11 @@ static const struct acpi_device_id cros_ec_lpc_acpi_device_ids[] = {
 };
 MODULE_DEVICE_TABLE(acpi, cros_ec_lpc_acpi_device_ids);
 
+static const struct lpc_driver_data framework_laptop_amd_lpc_driver_data __initconst = {
+	.quirks = CROS_EC_LPC_QUIRK_REMAP_MEMORY,
+	.quirk_mmio_memory_base = 0xE00,
+};
+
 static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
 	{
 		/*
@@ -581,7 +586,16 @@ static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
 	},
 	/* A small number of non-Chromebook/box machines also use the ChromeOS EC */
 	{
-		/* the Framework Laptop */
+		/* the Framework Laptop 13 (AMD Ryzen) and 16 (AMD Ryzen) */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "AMD Ryzen"),
+			DMI_MATCH(DMI_PRODUCT_FAMILY, "Laptop"),
+		},
+		.driver_data = (void *)&framework_laptop_amd_lpc_driver_data,
+	},
+	{
+		/* the Framework Laptop (Intel 11th, 12th, 13th Generation) */
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
 			DMI_MATCH(DMI_PRODUCT_NAME, "Laptop"),
-- 
2.43.0

Re: [PATCH v3 0/4] platform/chrome: cros_ec_lpc: add support for AMD Framework Laptops

[Thomas Weißschuh]

On 2024-04-02 19:47:09-0500, Dustin L. Howett wrote:
> This patch series adds support for the AMD models of the Framework
> Laptop (both 13" and 16").
> 
> These models of Framework Laptop have switched to an NPCX embedded
> controller, which uses the original Chrome EC linear memory-mapped I/O
> model.
> 
> However, these devices are not configured in a way that is compatible
> with the cros_ec driver. Instead of mapping EC memory to I/O ports
> [0x900, 0x9FF], they map it to ports [0xE00, 0xEFF].
> 
> To address this difference without impacting cros_ec_lpcs' compatibility
> with Chromebook/Chromebox devices or the existing Framework Laptop
> platform, these patches add DMI match-specific driver data through which
> we can detect per-machine "quirks".
> 
> Quirks toggle changes in cros_ec_lpcs' behavior, such as remapping the
> memory MMIO window.
> 
> Changes in v2:
> * Separated Framework Laptop (Intel) and Framework Laptop (AMD)
> * dev_warn has been demoted to dev_info
> * Style fixes
> * Reworded the individual patch bodies for clarity and style
> 
> Changes in v3:
> * Addressed dev_warn feedback from Thomas Weissschuh
> * Removed the host command region quirk; Framework Computer is slated to
>   release a firmware update that addresses the I/O region issue from
>   this series' v1 and v2. This leaves just one quirk, with the potential
>   for future quirks (e.g. changes required to reduce EC bus contention
>   on the Intel Framework Laptops)

For me this works with todays 3.0.5 firmware.

> Dustin L. Howett (4):
>   platform/chrome: cros_ec_lpc: introduce a priv struct for the lpc
>     device
>   platform/chrome: cros_ec_lpc: pass driver_data from DMI to the device
>   platform/chrome: cros_ec_lpc: add a "quirks" system
>   platform/chrome: cros_ec_lpc: add quirks for the Framework Laptop
>     (AMD)
> 
>  drivers/platform/chrome/cros_ec_lpc.c | 81 ++++++++++++++++++++++++---
>  1 file changed, 74 insertions(+), 7 deletions(-)

For the series:

Reviewed-and-tested-by: Thomas Weißschuh <linux@weissschuh.net>


Note:
For new revisions it makes sense to Cc people who interacted with
the previous ones. They are likely to have an interest and provide
feedback.
I only saw this by accident.

Re: [PATCH v3 0/4] platform/chrome: cros_ec_lpc: add support for AMD Framework Laptops

[Dustin Howett]

On Thu, Apr 4, 2024 at 1:53 PM Thomas Weißschuh <linux@weissschuh.net> wrote:
>
> On 2024-04-02 19:47:09-0500, Dustin L. Howett wrote:
> > This patch series adds support for the AMD models of the Framework
>
> For me this works with todays 3.0.5 firmware.

Thank you for testing! I've been seeing an issue where this
module--even with these fixes--is not being loaded with the right DMI
match table when it is automatically detected on boot.

My running theory is that it is matching the less-specific entry for
"Framework" + "Laptop" with no quirks.
I've been looking into making the matches more specific for all
Framework laptops, to future proof this change as well as make it work
properly via automatic loading.

>
> For the series:
>
> Reviewed-and-tested-by: Thomas Weißschuh <linux@weissschuh.net>

Thanks!

I wouldn't land this changeset until after we can diagnose the load on
boot issue.

>
>
> Note:
> For new revisions it makes sense to Cc people who interacted with
> the previous ones. They are likely to have an interest and provide
> feedback.
> I only saw this by accident.

I'm sorry about that. I'll keep it in mind for the future. I also
think I messed up by replying to the v2 thread with the v3 series, as
I realize I have not seen anybody else do this. :)

d

Re: [PATCH v3 0/4] platform/chrome: cros_ec_lpc: add support for AMD Framework Laptops

[Thomas Weißschuh]

On 2024-04-04 13:59:53-0500, Dustin Howett wrote:
> On Thu, Apr 4, 2024 at 1:53 PM Thomas Weißschuh <linux@weissschuh.net> wrote:
> > On 2024-04-02 19:47:09-0500, Dustin L. Howett wrote:
> > > This patch series adds support for the AMD models of the Framework
> >
> > For me this works with todays 3.0.5 firmware.
> 
> Thank you for testing! I've been seeing an issue where this
> module--even with these fixes--is not being loaded with the right DMI
> match table when it is automatically detected on boot.
> 
> My running theory is that it is matching the less-specific entry for
> "Framework" + "Laptop" with no quirks.
> I've been looking into making the matches more specific for all
> Framework laptops, to future proof this change as well as make it work
> properly via automatic loading.

This doesn't sound correct. The code is using dmi_first_match() which
always iterates the dmi_id table in order.

I'll try to reproduce this issue.

FYI in the case when probing didn't work you can check the current DMI
information in /sys/class/dmi/id/.

> > For the series:
> >
> > Reviewed-and-tested-by: Thomas Weißschuh <linux@weissschuh.net>
> 
> Thanks!
> 
> I wouldn't land this changeset until after we can diagnose the load on
> boot issue.

Sounds reasonable.

> > Note:
> > For new revisions it makes sense to Cc people who interacted with
> > the previous ones. They are likely to have an interest and provide
> > feedback.
> > I only saw this by accident.
> 
> I'm sorry about that. I'll keep it in mind for the future. I also
> think I messed up by replying to the v2 thread with the v3 series, as
> I realize I have not seen anybody else do this. :)

You could use b4 which can take care of this.
It can also read additional recipients from its cover letter, so you
don't have to manually keep track of them for each submission.


Thomas

Re: [PATCH v3 0/4] platform/chrome: cros_ec_lpc: add support for AMD Framework Laptops

[Dustin Howett]

On Thu, Apr 4, 2024 at 2:57 PM Thomas Weißschuh <linux@weissschuh.net> wrote:
>
> This doesn't sound correct. The code is using dmi_first_match() which
> always iterates the dmi_id table in order.
>
> I'll try to reproduce this issue.

No need! It turns out that I was tricked by my own initramfs.
Good tip not to let your distribution scripts pack modules you're
actively working on. :)

In that case: This series is ready to land!

> You could use b4 which can take care of this.
> It can also read additional recipients from its cover letter, so you
> don't have to manually keep track of them for each submission.

Excellent! Excited to use it for the next salvo. Thanks.

d

Re: [PATCH v3 0/4] platform/chrome: cros_ec_lpc: add support for AMD Framework Laptops

[Mario Limonciello]

On 4/2/24 19:47, Dustin L. Howett wrote:
> This patch series adds support for the AMD models of the Framework
> Laptop (both 13" and 16").
> 
> These models of Framework Laptop have switched to an NPCX embedded
> controller, which uses the original Chrome EC linear memory-mapped I/O
> model.
> 
> However, these devices are not configured in a way that is compatible
> with the cros_ec driver. Instead of mapping EC memory to I/O ports
> [0x900, 0x9FF], they map it to ports [0xE00, 0xEFF].
> 
> To address this difference without impacting cros_ec_lpcs' compatibility
> with Chromebook/Chromebox devices or the existing Framework Laptop
> platform, these patches add DMI match-specific driver data through which
> we can detect per-machine "quirks".
> 
> Quirks toggle changes in cros_ec_lpcs' behavior, such as remapping the
> memory MMIO window.
> 
> Changes in v2:
> * Separated Framework Laptop (Intel) and Framework Laptop (AMD)
> * dev_warn has been demoted to dev_info
> * Style fixes
> * Reworded the individual patch bodies for clarity and style
> 
> Changes in v3:
> * Addressed dev_warn feedback from Thomas Weissschuh
> * Removed the host command region quirk; Framework Computer is slated to
>    release a firmware update that addresses the I/O region issue from
>    this series' v1 and v2. This leaves just one quirk, with the potential
>    for future quirks (e.g. changes required to reduce EC bus contention
>    on the Intel Framework Laptops)
> 
> 
> Dustin L. Howett (4):
>    platform/chrome: cros_ec_lpc: introduce a priv struct for the lpc
>      device
>    platform/chrome: cros_ec_lpc: pass driver_data from DMI to the device
>    platform/chrome: cros_ec_lpc: add a "quirks" system
>    platform/chrome: cros_ec_lpc: add quirks for the Framework Laptop
>      (AMD)
> 
>   drivers/platform/chrome/cros_ec_lpc.c | 81 ++++++++++++++++++++++++---
>   1 file changed, 74 insertions(+), 7 deletions(-)
> 

I tested on top of a Framework 13 AMD running BIOS 03.05.

I found that there are some errors shown related to charging abilities 
for the PD controller.  Should cros-usbpd-charger support be quirked out 
when run on a FW laptop?

[    5.545352] cros-usbpd-charger cros-usbpd-charger.4.auto: No USB PD 
charging ports found
[    5.546722] cros-usbpd-charger cros-usbpd-charger.4.auto: Unexpected 
number of charge port count
[    5.546730] cros-usbpd-charger cros-usbpd-charger.4.auto: Failing 
probe (err:0xffffffb9)
[    5.546735] cros-usbpd-charger cros-usbpd-charger.4.auto: probe with 
driver cros-usbpd-charger failed with error -71

Re: [PATCH v3 0/4] platform/chrome: cros_ec_lpc: add support for AMD Framework Laptops

[Mario Limonciello]

On 4/6/24 13:48, Mario Limonciello wrote:
> 
> 
> On 4/2/24 19:47, Dustin L. Howett wrote:
>> This patch series adds support for the AMD models of the Framework
>> Laptop (both 13" and 16").
>>
>> These models of Framework Laptop have switched to an NPCX embedded
>> controller, which uses the original Chrome EC linear memory-mapped I/O
>> model.
>>
>> However, these devices are not configured in a way that is compatible
>> with the cros_ec driver. Instead of mapping EC memory to I/O ports
>> [0x900, 0x9FF], they map it to ports [0xE00, 0xEFF].
>>
>> To address this difference without impacting cros_ec_lpcs' compatibility
>> with Chromebook/Chromebox devices or the existing Framework Laptop
>> platform, these patches add DMI match-specific driver data through which
>> we can detect per-machine "quirks".
>>
>> Quirks toggle changes in cros_ec_lpcs' behavior, such as remapping the
>> memory MMIO window.
>>
>> Changes in v2:
>> * Separated Framework Laptop (Intel) and Framework Laptop (AMD)
>> * dev_warn has been demoted to dev_info
>> * Style fixes
>> * Reworded the individual patch bodies for clarity and style
>>
>> Changes in v3:
>> * Addressed dev_warn feedback from Thomas Weissschuh
>> * Removed the host command region quirk; Framework Computer is slated to
>>    release a firmware update that addresses the I/O region issue from
>>    this series' v1 and v2. This leaves just one quirk, with the potential
>>    for future quirks (e.g. changes required to reduce EC bus contention
>>    on the Intel Framework Laptops)
>>
>>
>> Dustin L. Howett (4):
>>    platform/chrome: cros_ec_lpc: introduce a priv struct for the lpc
>>      device
>>    platform/chrome: cros_ec_lpc: pass driver_data from DMI to the device
>>    platform/chrome: cros_ec_lpc: add a "quirks" system
>>    platform/chrome: cros_ec_lpc: add quirks for the Framework Laptop
>>      (AMD)
>>
>>   drivers/platform/chrome/cros_ec_lpc.c | 81 ++++++++++++++++++++++++---
>>   1 file changed, 74 insertions(+), 7 deletions(-)
>>
> 
> I tested on top of a Framework 13 AMD running BIOS 03.05.
> 
> I found that there are some errors shown related to charging abilities 
> for the PD controller.  Should cros-usbpd-charger support be quirked out 
> when run on a FW laptop?
> 
> [    5.545352] cros-usbpd-charger cros-usbpd-charger.4.auto: No USB PD 
> charging ports found
> [    5.546722] cros-usbpd-charger cros-usbpd-charger.4.auto: Unexpected 
> number of charge port count
> [    5.546730] cros-usbpd-charger cros-usbpd-charger.4.auto: Failing 
> probe (err:0xffffffb9)
> [    5.546735] cros-usbpd-charger cros-usbpd-charger.4.auto: probe with 
> driver cros-usbpd-charger failed with error -71
> 

They're all stemming from a missing return statement when no ports were 
found.  It shouldn't block this series which otherwise works fine.

Tested-by: Mario Limonciello <superm1@gmail.com>

I'll send out another patch to add that return code which can be 
discussed separately.

Re: [PATCH v3 0/4] platform/chrome: cros_ec_lpc: add support for AMD Framework Laptops

[patchwork-bot+chrome-platform]

Hello:

This series was applied to chrome-platform/linux.git (for-kernelci)
by Tzung-Bi Shih <tzungbi@kernel.org>:

On Tue,  2 Apr 2024 19:47:09 -0500 you wrote:
> This patch series adds support for the AMD models of the Framework
> Laptop (both 13" and 16").
> 
> These models of Framework Laptop have switched to an NPCX embedded
> controller, which uses the original Chrome EC linear memory-mapped I/O
> model.
> 
> [...]

Here is the summary with links:
  - [v3,1/4] platform/chrome: cros_ec_lpc: introduce a priv struct for the lpc device
    https://git.kernel.org/chrome-platform/c/8ae8e4e694b1
  - [v3,2/4] platform/chrome: cros_ec_lpc: pass driver_data from DMI to the device
    https://git.kernel.org/chrome-platform/c/839720336e8c
  - [v3,3/4] platform/chrome: cros_ec_lpc: add a "quirks" system
    https://git.kernel.org/chrome-platform/c/6b0ab6de2d86
  - [v3,4/4] platform/chrome: cros_ec_lpc: add quirks for the Framework Laptop (AMD)
    https://git.kernel.org/chrome-platform/c/3e8d1b5350e0

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

Re: [PATCH v3 0/4] platform/chrome: cros_ec_lpc: add support for AMD Framework Laptops

[patchwork-bot+chrome-platform]

Hello:

This series was applied to chrome-platform/linux.git (for-next)
by Tzung-Bi Shih <tzungbi@kernel.org>:

On Tue,  2 Apr 2024 19:47:09 -0500 you wrote:
> This patch series adds support for the AMD models of the Framework
> Laptop (both 13" and 16").
> 
> These models of Framework Laptop have switched to an NPCX embedded
> controller, which uses the original Chrome EC linear memory-mapped I/O
> model.
> 
> [...]

Here is the summary with links:
  - [v3,1/4] platform/chrome: cros_ec_lpc: introduce a priv struct for the lpc device
    https://git.kernel.org/chrome-platform/c/8ae8e4e694b1
  - [v3,2/4] platform/chrome: cros_ec_lpc: pass driver_data from DMI to the device
    https://git.kernel.org/chrome-platform/c/839720336e8c
  - [v3,3/4] platform/chrome: cros_ec_lpc: add a "quirks" system
    https://git.kernel.org/chrome-platform/c/6b0ab6de2d86
  - [v3,4/4] platform/chrome: cros_ec_lpc: add quirks for the Framework Laptop (AMD)
    https://git.kernel.org/chrome-platform/c/3e8d1b5350e0

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