[PATCH v2 0/2] platform/chrome: add support for the Framework Laptop

[PATCH v2 0/2] platform/chrome: add support for the Framework Laptop

[Dustin L. Howett]

This patch series adds support for the Framework Laptop to the cros_ec
LPC driver.

The Framework Laptop is a non-Chromebook laptop that uses the ChromeOS
Embedded Controller. Since the machine was designed to present a more
normal device profile, it does not report all 512 I/O ports that are
typically used by cros_ec_lpcs. Because of this, changes to the driver's
port reservation scheme were required.

Since this EC driver probes the MEC range first, and uses only the MEC
range if that probe succeeds[^1], we can get by without requesting the
entire port range required by non-MEC embedded controllers until
absolutely necessary.

[^1]: this includes "memory mapped" read - where the traditional LPC EC
requires I/O ports 0x900-0x9FF, the MEC EC multiplexes reads/writes
over the same eight ports, 0x800-0x807.

Changelog in v2:
  Cleaned up the commit subjects per request.

Dustin L. Howett (2):
  platform/chrome: cros_ec_lpcs: detect the Framework Laptop
  platform/chrome: cros_ec_lpcs: reserve the MEC LPC I/O ports first

 drivers/platform/chrome/cros_ec_lpc.c          |   47 ++++++++++-----
 include/linux/platform_data/cros_ec_commands.h |    4 +
 2 files changed, 38 insertions(+), 13 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] platform/chrome: cros_ec_lpcs: detect the Framework Laptop

[Dustin L. Howett]

The Framework Laptop identifies itself in DMI with manufacturer
"Framework" and product "Laptop".

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 d6306d2a096f..458eb59db2ff 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -500,6 +500,14 @@ static const struct dmi_system_id cros_ec_lpc_dmi_table[] __initconst = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Glimmer"),
 		},
 	},
+	/* A small number of non-Chromebook/box machines also use the ChromeOS EC */
+	{
+		/* the Framework Laptop */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Laptop"),
+		},
+	},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(dmi, cros_ec_lpc_dmi_table);
-- 
2.34.1

[PATCH v2 2/2] platform/chrome: cros_ec_lpcs: reserve the MEC LPC I/O ports first

[Dustin L. Howett]

Some ChromeOS EC devices (such as the Framework Laptop) only map I/O
ports 0x800-0x807. Making the larger reservation required by the non-MEC
LPC (the 0xFF ports for the memory map, and the 0xFF ports for the
parameter region) is non-viable on these devices.

Since we probe the MEC EC first, we can get away with a smaller
reservation that covers the MEC EC ports. If we fall back to classic
LPC, we can grow the reservation to cover the memory map and the
parameter region.

This patch also fixes an issue where we would interact with I/O ports
0x800-0x807 without first making a reservation.

Signed-off-by: Dustin L. Howett <dustin@howett.net>
---
 drivers/platform/chrome/cros_ec_lpc.c         | 39 ++++++++++++-------
 .../linux/platform_data/cros_ec_commands.h    |  4 ++
 2 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 458eb59db2ff..06fdfe365710 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -341,9 +341,14 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 	u8 buf[2];
 	int irq, ret;
 
-	if (!devm_request_region(dev, EC_LPC_ADDR_MEMMAP, EC_MEMMAP_SIZE,
-				 dev_name(dev))) {
-		dev_err(dev, "couldn't reserve memmap region\n");
+	/*
+	 * The Framework Laptop (and possibly other non-ChromeOS devices)
+	 * only exposes the eight I/O ports that are required for the Microchip EC.
+	 * Requesting a larger reservation will fail.
+	 */
+	if (!devm_request_region(dev, EC_HOST_CMD_REGION0,
+				 EC_HOST_CMD_MEC_REGION_SIZE, dev_name(dev))) {
+		dev_err(dev, "couldn't reserve MEC region\n");
 		return -EBUSY;
 	}
 
@@ -357,6 +362,12 @@ 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,
+					 dev_name(dev))) {
+			dev_err(dev, "couldn't reserve memmap region\n");
+			return -EBUSY;
+		}
+
 		/* 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;
@@ -366,17 +377,19 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
 			dev_err(dev, "EC ID not detected\n");
 			return -ENODEV;
 		}
-	}
 
-	if (!devm_request_region(dev, EC_HOST_CMD_REGION0,
-				 EC_HOST_CMD_REGION_SIZE, dev_name(dev))) {
-		dev_err(dev, "couldn't reserve region0\n");
-		return -EBUSY;
-	}
-	if (!devm_request_region(dev, EC_HOST_CMD_REGION1,
-				 EC_HOST_CMD_REGION_SIZE, dev_name(dev))) {
-		dev_err(dev, "couldn't reserve region1\n");
-		return -EBUSY;
+		/* Reserve the remaining I/O ports required by the non-MEC protocol. */
+		if (!devm_request_region(dev, EC_HOST_CMD_REGION0 + EC_HOST_CMD_MEC_REGION_SIZE,
+					 EC_HOST_CMD_REGION_SIZE - EC_HOST_CMD_MEC_REGION_SIZE,
+					 dev_name(dev))) {
+			dev_err(dev, "couldn't reserve remainder of region0\n");
+			return -EBUSY;
+		}
+		if (!devm_request_region(dev, EC_HOST_CMD_REGION1,
+					 EC_HOST_CMD_REGION_SIZE, dev_name(dev))) {
+			dev_err(dev, "couldn't reserve region1\n");
+			return -EBUSY;
+		}
 	}
 
 	ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
index 271bd87bff0a..a85b1176e6c0 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -55,6 +55,10 @@
 #define EC_HOST_CMD_REGION0    0x800
 #define EC_HOST_CMD_REGION1    0x880
 #define EC_HOST_CMD_REGION_SIZE 0x80
+/*
+ * Other machines report only the region spanned by the Microchip MEC EC.
+ */
+#define EC_HOST_CMD_MEC_REGION_SIZE 0x08
 
 /* EC command register bit functions */
 #define EC_LPC_CMDR_DATA	BIT(0)  /* Data ready for host to read */
-- 
2.34.1

Re: [PATCH v2 0/2] platform/chrome: add support for the Framework Laptop

[Tzung-Bi Shih]

On Wed, Jan 26, 2022 at 12:00:18PM -0600, Dustin L. Howett wrote:
> This patch series adds support for the Framework Laptop to the cros_ec
> LPC driver.
> 
> The Framework Laptop is a non-Chromebook laptop that uses the ChromeOS
> Embedded Controller. Since the machine was designed to present a more
> normal device profile, it does not report all 512 I/O ports that are
> typically used by cros_ec_lpcs. Because of this, changes to the driver's
> port reservation scheme were required.
> 
> Since this EC driver probes the MEC range first, and uses only the MEC
> range if that probe succeeds[^1], we can get by without requesting the
> entire port range required by non-MEC embedded controllers until
> absolutely necessary.
> 
> [^1]: this includes "memory mapped" read - where the traditional LPC EC
> requires I/O ports 0x900-0x9FF, the MEC EC multiplexes reads/writes
> over the same eight ports, 0x800-0x807.
> 
> Changelog in v2:
>   Cleaned up the commit subjects per request.
> 
> Dustin L. Howett (2):
>   platform/chrome: cros_ec_lpcs: detect the Framework Laptop
>   platform/chrome: cros_ec_lpcs: reserve the MEC LPC I/O ports first

For the series,
Reviewed-by: Tzung-Bi Shih <tzungbi@google.com>

Re: [PATCH v2 2/2] platform/chrome: cros_ec_lpcs: reserve the MEC LPC I/O ports first

[Prashant Malani]

Hi Dustin,

On Jan 26 12:00, Dustin L. Howett wrote:
> Some ChromeOS EC devices (such as the Framework Laptop) only map I/O
> ports 0x800-0x807. Making the larger reservation required by the non-MEC
> LPC (the 0xFF ports for the memory map, and the 0xFF ports for the
> parameter region) is non-viable on these devices.
> 
> Since we probe the MEC EC first, we can get away with a smaller
> reservation that covers the MEC EC ports. If we fall back to classic
> LPC, we can grow the reservation to cover the memory map and the
> parameter region.
> 
> This patch also fixes an issue where we would interact with I/O ports
> 0x800-0x807 without first making a reservation.
> 
> Signed-off-by: Dustin L. Howett <dustin@howett.net>
> ---
>  drivers/platform/chrome/cros_ec_lpc.c         | 39 ++++++++++++-------
>  .../linux/platform_data/cros_ec_commands.h    |  4 ++
>  2 files changed, 30 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 458eb59db2ff..06fdfe365710 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -341,9 +341,14 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
>  	u8 buf[2];
>  	int irq, ret;
>  
> -	if (!devm_request_region(dev, EC_LPC_ADDR_MEMMAP, EC_MEMMAP_SIZE,
> -				 dev_name(dev))) {
> -		dev_err(dev, "couldn't reserve memmap region\n");
> +	/*
> +	 * The Framework Laptop (and possibly other non-ChromeOS devices)
> +	 * only exposes the eight I/O ports that are required for the Microchip EC.
> +	 * Requesting a larger reservation will fail.
> +	 */
> +	if (!devm_request_region(dev, EC_HOST_CMD_REGION0,
> +				 EC_HOST_CMD_MEC_REGION_SIZE, dev_name(dev))) {
> +		dev_err(dev, "couldn't reserve MEC region\n");
>  		return -EBUSY;
>  	}
>  
> @@ -357,6 +362,12 @@ 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,
> +					 dev_name(dev))) {
> +			dev_err(dev, "couldn't reserve memmap region\n");
> +			return -EBUSY;
> +		}
> +
>  		/* 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;
> @@ -366,17 +377,19 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
>  			dev_err(dev, "EC ID not detected\n");
>  			return -ENODEV;
>  		}
> -	}
>  
> -	if (!devm_request_region(dev, EC_HOST_CMD_REGION0,
> -				 EC_HOST_CMD_REGION_SIZE, dev_name(dev))) {
> -		dev_err(dev, "couldn't reserve region0\n");
> -		return -EBUSY;
> -	}
> -	if (!devm_request_region(dev, EC_HOST_CMD_REGION1,
> -				 EC_HOST_CMD_REGION_SIZE, dev_name(dev))) {
> -		dev_err(dev, "couldn't reserve region1\n");
> -		return -EBUSY;
> +		/* Reserve the remaining I/O ports required by the non-MEC protocol. */
> +		if (!devm_request_region(dev, EC_HOST_CMD_REGION0 + EC_HOST_CMD_MEC_REGION_SIZE,
> +					 EC_HOST_CMD_REGION_SIZE - EC_HOST_CMD_MEC_REGION_SIZE,
> +					 dev_name(dev))) {
> +			dev_err(dev, "couldn't reserve remainder of region0\n");
> +			return -EBUSY;
> +		}
> +		if (!devm_request_region(dev, EC_HOST_CMD_REGION1,
> +					 EC_HOST_CMD_REGION_SIZE, dev_name(dev))) {
> +			dev_err(dev, "couldn't reserve region1\n");
> +			return -EBUSY;
> +		}
>  	}
>  
>  	ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
> diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
> index 271bd87bff0a..a85b1176e6c0 100644
> --- a/include/linux/platform_data/cros_ec_commands.h
> +++ b/include/linux/platform_data/cros_ec_commands.h
> @@ -55,6 +55,10 @@
>  #define EC_HOST_CMD_REGION0    0x800
>  #define EC_HOST_CMD_REGION1    0x880
>  #define EC_HOST_CMD_REGION_SIZE 0x80
> +/*
> + * Other machines report only the region spanned by the Microchip MEC EC.
> + */
> +#define EC_HOST_CMD_MEC_REGION_SIZE 0x08
I can't find this update in the EC code base [1]. Is there any reason
you are not adding this, or is the change in flight (or in some other
location)?

[1] https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/main/include/ec_commands.h

Thanks,

-Prashant

Re: [PATCH v2 2/2] platform/chrome: cros_ec_lpcs: reserve the MEC LPC I/O ports first

[Dustin Howett]

On Thu, Jan 27, 2022 at 12:55 PM Prashant Malani <pmalani@chromium.org> wrote:
>
> Hi Dustin,
>
> I can't find this update in the EC code base [1]. Is there any reason
> you are not adding this, or is the change in flight (or in some other
> location)?
>
> [1] https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/main/include/ec_commands.h
>

Hey Prashant,

The host communication adapters in the EC repo don't support the MEC
protocol at all,
so it did not seem necessary to bring these changes over. I'd be happy
to do so, of
course, if that is desirable.

My understanding (well, my guess) is that protocol support was never
added because
it is already implemented here in cros_ec_lpcs. Userland I/O port
access is(?) less desirable
than having this driver handle it.

d

> Thanks,
>
> -Prashant

Re: [PATCH v2 2/2] platform/chrome: cros_ec_lpcs: reserve the MEC LPC I/O ports first

[Prashant Malani]

+Aseda,

On Jan 27 13:18, Dustin Howett wrote:
> On Thu, Jan 27, 2022 at 12:55 PM Prashant Malani <pmalani@chromium.org> wrote:
> >
> > Hi Dustin,
> >
> > I can't find this update in the EC code base [1]. Is there any reason
> > you are not adding this, or is the change in flight (or in some other
> > location)?
> >
> > [1] https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/main/include/ec_commands.h
> >
> 
> Hey Prashant,
> 
> The host communication adapters in the EC repo don't support the MEC
> protocol at all,
> so it did not seem necessary to bring these changes over. I'd be happy
What source do Framework laptop ECs (MECs?) compile their EC firmware
from?

> to do so, of
> course, if that is desirable.
> 
> My understanding (well, my guess) is that protocol support was never
> added because
> it is already implemented here in cros_ec_lpcs. Userland I/O port
> access is(?) less desirable
> than having this driver handle it.
Yeah, I wasn't thinking about userland i/o port access, but just having
this behaviour/different I/O port mapping described in the EC code base
too.

Re: [PATCH v2 2/2] platform/chrome: cros_ec_lpcs: reserve the MEC LPC I/O ports first

[Prashant Malani]

Forgot 1 more minor nit:

On Jan 27 18:55, Prashant Malani wrote:
> Hi Dustin,
> 
> On Jan 26 12:00, Dustin L. Howett wrote:
> > Some ChromeOS EC devices (such as the Framework Laptop) only map I/O
> > ports 0x800-0x807. Making the larger reservation required by the non-MEC
> > LPC (the 0xFF ports for the memory map, and the 0xFF ports for the
> > parameter region) is non-viable on these devices.
> > 
> > Since we probe the MEC EC first, we can get away with a smaller
> > reservation that covers the MEC EC ports. If we fall back to classic
> > LPC, we can grow the reservation to cover the memory map and the
> > parameter region.
> > 
> > This patch also fixes an issue where we would interact with I/O ports
> > 0x800-0x807 without first making a reservation.
(borrowing this from swboyd):
$ git grep "This patch" -- Documentation/process

i.e. don't use "This patch"

> > 
> > Signed-off-by: Dustin L. Howett <dustin@howett.net>
> > ---
> >  drivers/platform/chrome/cros_ec_lpc.c         | 39 ++++++++++++-------
> >  .../linux/platform_data/cros_ec_commands.h    |  4 ++
> >  2 files changed, 30 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> > index 458eb59db2ff..06fdfe365710 100644
> > --- a/drivers/platform/chrome/cros_ec_lpc.c
> > +++ b/drivers/platform/chrome/cros_ec_lpc.c
> > @@ -341,9 +341,14 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
> >  	u8 buf[2];
> >  	int irq, ret;
> >  
> > -	if (!devm_request_region(dev, EC_LPC_ADDR_MEMMAP, EC_MEMMAP_SIZE,
> > -				 dev_name(dev))) {
> > -		dev_err(dev, "couldn't reserve memmap region\n");
> > +	/*
> > +	 * The Framework Laptop (and possibly other non-ChromeOS devices)
> > +	 * only exposes the eight I/O ports that are required for the Microchip EC.
> > +	 * Requesting a larger reservation will fail.
> > +	 */
> > +	if (!devm_request_region(dev, EC_HOST_CMD_REGION0,
> > +				 EC_HOST_CMD_MEC_REGION_SIZE, dev_name(dev))) {
> > +		dev_err(dev, "couldn't reserve MEC region\n");
> >  		return -EBUSY;
> >  	}
> >  
> > @@ -357,6 +362,12 @@ 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,
> > +					 dev_name(dev))) {
> > +			dev_err(dev, "couldn't reserve memmap region\n");
> > +			return -EBUSY;
> > +		}
> > +
> >  		/* 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;
> > @@ -366,17 +377,19 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
> >  			dev_err(dev, "EC ID not detected\n");
> >  			return -ENODEV;
> >  		}
> > -	}
> >  
> > -	if (!devm_request_region(dev, EC_HOST_CMD_REGION0,
> > -				 EC_HOST_CMD_REGION_SIZE, dev_name(dev))) {
> > -		dev_err(dev, "couldn't reserve region0\n");
> > -		return -EBUSY;
> > -	}
> > -	if (!devm_request_region(dev, EC_HOST_CMD_REGION1,
> > -				 EC_HOST_CMD_REGION_SIZE, dev_name(dev))) {
> > -		dev_err(dev, "couldn't reserve region1\n");
> > -		return -EBUSY;
> > +		/* Reserve the remaining I/O ports required by the non-MEC protocol. */
> > +		if (!devm_request_region(dev, EC_HOST_CMD_REGION0 + EC_HOST_CMD_MEC_REGION_SIZE,
> > +					 EC_HOST_CMD_REGION_SIZE - EC_HOST_CMD_MEC_REGION_SIZE,
> > +					 dev_name(dev))) {
> > +			dev_err(dev, "couldn't reserve remainder of region0\n");
> > +			return -EBUSY;
> > +		}
> > +		if (!devm_request_region(dev, EC_HOST_CMD_REGION1,
> > +					 EC_HOST_CMD_REGION_SIZE, dev_name(dev))) {
> > +			dev_err(dev, "couldn't reserve region1\n");
> > +			return -EBUSY;
> > +		}
> >  	}
> >  
> >  	ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
> > diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
> > index 271bd87bff0a..a85b1176e6c0 100644
> > --- a/include/linux/platform_data/cros_ec_commands.h
> > +++ b/include/linux/platform_data/cros_ec_commands.h
> > @@ -55,6 +55,10 @@
> >  #define EC_HOST_CMD_REGION0    0x800
> >  #define EC_HOST_CMD_REGION1    0x880
> >  #define EC_HOST_CMD_REGION_SIZE 0x80
> > +/*
> > + * Other machines report only the region spanned by the Microchip MEC EC.
> > + */
> > +#define EC_HOST_CMD_MEC_REGION_SIZE 0x08
> I can't find this update in the EC code base [1]. Is there any reason
> you are not adding this, or is the change in flight (or in some other
> location)?
> 
> [1] https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/main/include/ec_commands.h
> 
> Thanks,
> 
> -Prashant

Re: [PATCH v2 2/2] platform/chrome: cros_ec_lpcs: reserve the MEC LPC I/O ports first

[Dustin Howett]

On Thu, Jan 27, 2022 at 1:25 PM Prashant Malani <pmalani@chromium.org> wrote:
>
> What source do Framework laptop ECs (MECs?) compile their EC firmware
> from?

They just released their source here:
https://github.com/FrameworkComputer/EmbeddedController

FWIW, this series was written before they went open, and you're not
likely to see a similar construct
over there.

> Yeah, I wasn't thinking about userland i/o port access, but just having
> this behaviour/different I/O port mapping described in the EC code base
> too.

Happy to submit this over there as well. Are cros_ec_commands.h (here)
and ec_commands.h (there)
intended to be kept in 1:1 sync?

As well, is this a blocking concern?

Thanks!
d

Re: [PATCH v2 2/2] platform/chrome: cros_ec_lpcs: reserve the MEC LPC I/O ports first

[Prashant Malani]

Hi,

On Fri, Jan 28, 2022 at 9:57 AM Aseda Aboagye <aaboagye@google.com> wrote:
>
> Generally ec_commands.h is to be kept in sync with the copies in other projects. Periodically when someone modifies it, we should update the copies as well.
> --
> Aseda Aboagye
>
>
> On Thu, Jan 27, 2022 at 9:16 PM Dustin Howett <dustin@howett.net> wrote:
>>
>> On Thu, Jan 27, 2022 at 1:25 PM Prashant Malani <pmalani@chromium.org> wrote:
>> >
>> > What source do Framework laptop ECs (MECs?) compile their EC firmware
>> > from?
>>
>> They just released their source here:
>> https://github.com/FrameworkComputer/EmbeddedController
>>
>> FWIW, this series was written before they went open, and you're not
>> likely to see a similar construct
>> over there.
>>
>> > Yeah, I wasn't thinking about userland i/o port access, but just having
>> > this behaviour/different I/O port mapping described in the EC code base
>> > too.
>>
>> Happy to submit this over there as well. Are cros_ec_commands.h (here)
>> and ec_commands.h (there)
>> intended to be kept in 1:1 sync?

It's not auto-generated, but FWIW cros_ec_commands.h is a subset of
ec_commands.h (only the parts of ec_commands.h which are relevant to
the kernel drivers are brought over)

>>
>> As well, is this a blocking concern?
I'd prefer it to be first documented in the EC sources via a header
update before we make changes to the kernel here. This ensures that
the authors and maintainers of the various EC sources (Chromium EC,
framework EC etc) are aware of this change and sign-off on it.

I think making this update to the Chromium OS platform/ec code base
will be sufficient (it can flow down to framework when/if they chose
to sync to Chromium platform/ec); but happy to be corrected here by
Aseda or other EC experts.

Thanks,