ec: Consider removing cros_ec_readmem / CROS_EC_DEV_IOCRDMEM IOCTL |
||||
Issue description- CROS_EC_DEV_IOCRDMEM is not supported by non-LPC platforms - CROS_EC_DEV_IOCRDMEM is redundant and can be replaced (with slightly less efficiency on LPC platforms) with CROS_EC_DEV_IOCXCMD + EC_CMD_READ_MEMMAP cmd - flashrom and mosys don't even use it -- it's only used in the ec comm-dev library (eg. for ectool)
,
Apr 13 2016
,
Apr 13 2016
Slight complication.
When using LPC transport, the command EC_CMD_READ_MEMMAP is compiled out in the EC.
The way ectool is working:
- if dev interface present (which is always true on 3.14+ kernels)
- if LPC transport,
use CROS_EC_DEV_IOCRDMEM ioctl for reading memory.
use CROS_EC_DEV_IOCXCMD for commands.
- else
use CROS_EC_DEV_IOCXCMD for everything
- else
use direct LPC access for everything.
Using direct access for commands is deprecated for 3.14+ kernel,
the kernel is also sending commands.
If propose the following:
- if dev interface present (which is always true on 3.14+ kernels)
-if CROS_EC_DEV_IOCXCMD + EC_CMD_READ_MEMMAP works,
use it
-else
use direct LPC access.
...
This method allows removal of CROS_EC_DEV_IOCRDMEM ioctl, but requires an assumption that the EC is frugal.
,
Apr 13 2016
EC_CMD_READ_MEMMAP should be supported by all production ECs going back to link. Do we have any kernels in production today where the cros-dev interface is supported, CROS_EC_DEV_IOCRDMEM is supported, but CROS_EC_DEV_IOCXCMD is not?
,
Apr 13 2016
EC_CMD_READ_MEMMAP is not supported if LPC is enabled. See https://cs.corp.google.com/chromeos_public/src/platform/ec/common/host_command.c?rcl=52f7b3a0c842810d119b63894ce5c12d88363b5c&l=478 If ros-dev interface is supported, CROS_EC_DEV_IOCXCMD is as well. I sent a CL to remove CROS_EC_DEV_IOCRDMEM support.
,
Apr 15 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/de45353bbdf0c6c4afb88420d3cb2e182eae7450 commit de45353bbdf0c6c4afb88420d3cb2e182eae7450 Author: Gwendal Grignou <gwendal@chromium.org> Date: Wed Apr 13 19:34:08 2016 ectool: Remove CROS_EC_DEV_IOCRDMEM On !LPC EC, we can read memory via CROS_EC_DEV_IOCXCMD ioctl, using command EC_CMD_READ_MEMMAP. On platform that supports direct memory access (lpc), we access the memory directly, bypassing the ioctl. BUG= chromium:602832 TEST=On gnawty and veyron, check 'ectool battery' works. Verify that gnawty use io mapped registers. BRANCH=none Change-Id: I9bfcddcf450bf8df63ead78e1df97dd7392289e6 Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/338853 Reviewed-by: Randall Spangler <rspangler@chromium.org> [modify] https://crrev.com/de45353bbdf0c6c4afb88420d3cb2e182eae7450/util/comm-host.c [modify] https://crrev.com/de45353bbdf0c6c4afb88420d3cb2e182eae7450/util/comm-host.h [modify] https://crrev.com/de45353bbdf0c6c4afb88420d3cb2e182eae7450/util/comm-lpc.c [modify] https://crrev.com/de45353bbdf0c6c4afb88420d3cb2e182eae7450/util/comm-dev.c
,
Apr 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel-next/+/c0567473579e03598a15d4601b184f516944735b commit c0567473579e03598a15d4601b184f516944735b Author: Gwendal Grignou <gwendal@chromium.org> Date: Wed Apr 13 16:55:39 2016 cros_ec: Remove unused ioctl CROS_EC_DEV_IOCRDMEM This ioctl is a duplicate of CROS_EC_DEV_IOCXCMD with command EC_CMD_READ_MEMMAP. BUG= chromium:602832 CQ-DEPEND=I9bfcddcf450bf8df63ead78e1df97dd7392289e6 TEST=On gnawty, check 'ectool battery' works. Change-Id: I81a8d0dcb0d4a7b0a142e17753a18369d4099bb0 Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/338861 Reviewed-by: Randall Spangler <rspangler@chromium.org> Reviewed-by: Shawn N <shawnn@chromium.org> [modify] https://crrev.com/c0567473579e03598a15d4601b184f516944735b/drivers/platform/chrome/cros_ec_dev.h [modify] https://crrev.com/c0567473579e03598a15d4601b184f516944735b/drivers/platform/chrome/cros_ec_dev.c
,
Apr 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/e54d1284ffbd1d7c3b1f77a2a696ce275c527de4 commit e54d1284ffbd1d7c3b1f77a2a696ce275c527de4 Author: Gwendal Grignou <gwendal@chromium.org> Date: Fri Apr 22 18:09:33 2016 ectool: Revert "ectool: Remove CROS_EC_DEV_IOCRDMEM" CROS_EC_DEV_IOCRDMEM must be used on architecture where legacy IO mapped registers are accessed inderectly via EMI. The kernel is taking care of the translation. TEST=Check on reks that we need to use the IOCTL. BUG=chrome-os-partner:52550, chromium:602832 BRANCH=none This reverts commit de45353bbdf0 ("ectool: Remove CROS_EC_DEV_IOCRDMEM"). Change-Id: I8efad56df90c58c25bdc9ccd70a508547e629a77 Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/340348 Reviewed-by: Shawn N <shawnn@chromium.org> [modify] https://crrev.com/e54d1284ffbd1d7c3b1f77a2a696ce275c527de4/util/comm-host.c [modify] https://crrev.com/e54d1284ffbd1d7c3b1f77a2a696ce275c527de4/util/comm-host.h [modify] https://crrev.com/e54d1284ffbd1d7c3b1f77a2a696ce275c527de4/util/comm-lpc.c [modify] https://crrev.com/e54d1284ffbd1d7c3b1f77a2a696ce275c527de4/util/comm-dev.c
,
Apr 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/35a1e4401dc5e49ad3db9ad581816296808637fa commit 35a1e4401dc5e49ad3db9ad581816296808637fa Author: Gwendal Grignou <gwendal@chromium.org> Date: Fri Apr 22 18:04:30 2016 CHROMIUM: Revert "cros_ec: Remove unused ioctl CROS_EC_DEV_IOCRDMEM" Removing CROS_EC_DEV_IOCRDMEM is not possible. On some architecture reading legacy IO mapped memory register is done through EMI indirect access. To make it transparent from the user space, re-add the ioctl. TEST=Compile. BUG=chrome-os-partner:52550, chromium:602832 This reverts commit c0567473579e ("cros_ec: Remove unused ioctl CROS_EC_DEV_IOCRDMEM") Change-Id: Id09a62b52290a7824998fe3a6729b9c520602992 Signed-off-by: Gwendal Grignou <gwendal@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/340296 Reviewed-by: Guenter Roeck <groeck@google.com> [modify] https://crrev.com/35a1e4401dc5e49ad3db9ad581816296808637fa/drivers/platform/chrome/cros_ec_dev.h [modify] https://crrev.com/35a1e4401dc5e49ad3db9ad581816296808637fa/drivers/platform/chrome/cros_ec_dev.c
,
Apr 28 2016
CROS_EC_DEV_IOCRDMEM is needed after all. |
||||
►
Sign in to add a comment |
||||
Comment 1 by rspangler@chromium.org
, Apr 12 2016