lkml.org 
[lkml]   [2022]   [Jan]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 2/2] platform/chrome: cros_ec_lpcs: reserve the MEC LPC I/O ports first
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

\
 
 \ /
  Last update: 2022-01-27 20:31    [W:0.150 / U:0.584 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site