Re: [PATCH 2/2] platform/chrome: reserve only the I/O ports required for the MEC EC

[Dustin Howett]

On Mon, Jan 24, 2022 at 10:17 PM Tzung-Bi Shih <tzungbi@xxxxxxxxxx> wrote:
>
>
> The original code:
> - devm_request_region(dev, EC_LPC_ADDR_MEMMAP, ...) and then
> - cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, ...).
>
> After the patch:
> - devm_request_region(dev, EC_HOST_CMD_REGION0, ...) and then
> - cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, ...).
>
> Does it work if it reads out of request_region range?
>
>
> The 2 request_region are now guarded by the first "if (buf[0] != 'E' || buf[1] != 'C')".  That is, only non-MEC will request the 2 regions.
>
> Doesn't other MECs (e.g. non-Framework Laptop) need the 2 regions?

So, in both cases this should be legal. Here's why:

The MEC protocol multiplexes memory-mapped reads through the same 8
I/O ports (0x800 - 0x807)
as it does host commands. It works by adjusting the base address,
EC_LPC_ADDR_MEMMAP (0x900),
to 0x100 before it initiates a standard MEC LPC xfer.
See cros_ec_lpc_mec_read_bytes line ~101 (as of 881007522c8fcc3785).

Therefore, the adjusted flow in the patch is:

0. Default cros_ec_lpc_ops to the MEC version of read/xfer [unchanged in patch]
1. Request 0x800 - 0x807 (MEC region)
2. read() using the MEC read function (using only the above ports)
3. if it succeeds, great! we have a MEC EC.
--- if it failed ---
4. Map the non-MEC port range 0x900 - 0x9FF for memory-mapped reads
5. read() using the NON-MEC read function (using ports 0x900 - 0x9FF)
6. if it succeeds, map the remaining host command ports 0x808 - 0x8FF

In short, only non-MEC needs the 0x900 - 0x9FF mapping for "mapped
memory". Therefore we can defer the
port allocation until after we've failed to read mapped memory the MEC way. :)

Based on my understanding of the MEC protocol, non-Framework Laptop
MECs hold this invariant true as well.
They should only need ports 0x800 - 0x807.

Hope that helps!

Want me to send a v2 with updated commit messages?
d