ectool: --name=cros_pd does not work properly if the system has only EC. |
|||
Issue descriptionThis is derived from issue crosbug.com/p/62848 . What steps will reproduce the problem? (1) On reef, do ectool --name=cros_pd version or, flashrom -p ec:type=pd --get-size What is the expected result? Should fail since there's no ChromeEC based PD on reef What happens instead? ectool returns EC info instead of failure. flashrom returns EC contents instead of failure. The factory needs a way to know if we should probe PD information and if we should try to write-protect it. Our assumption was - If ectool can find Chrome PD, then we should probe and encode into HWID, and enable WP on MP. - If ectool finds a PD that is not Chrome PD, we can still probe or check in HWID, but not doing WP. - Otherwise, consider the system as no PD. There are few commands we've tried to check PD status: - flashrom: broken as indicated - 'ec:dev=n' works but we don't know if it's PD or something else. 'ec:type=pd' does not work on reef since it returns EC. - ectool: also incorrect on reef with we do --name=cros_pd. - mosys pd: "seems" correct if it always return "Chrome PD", but it's unclear if it'll change to support 3rd party PD in future.
,
Feb 14 2017
Fix is submitted for review: https://chromium-review.googlesource.com/441951
,
Feb 14 2017
Thanks for the patch. In general, I think we can check for the existence of /dev/cros_pd (which is a prerequisite for ectool working with '--name=cros_pd', post-patch). But this method can fail if something goes wrong in kernel device probing (which may happen if the PD is wedged, or held in reset). So I suggest something like this: 1. Check some static info (eg. mosys) to determine whether the board should have Cros PD. 2. Check for the existence of /dev/cros_pd. If (1) and (2) disagree then throw a fatal error, otherwise use the consensus. Unfortunately I don't think (1) exists right now (but maybe I missed something?). "mosys pd info" also relies on /dev/cros_pd.
,
Feb 14 2017
Third party PD chips can be probed by ectool pdchipinfo <port>. mosys will support third party PD chips by mosys pd chip <port>. Changes are being reviewed.
,
Feb 15 2017
Re#3 True and False regarding mosys. 1. Mosys uses a platform-specific way to include drivers and commands. For example, 'pd' command is hard-coded to only available on glados and samus (plus oak) now. 2. The default pd driver in mosys relies on /dev/cros_pd. There's no guarantee that "mosys pd" should be a "cros_pd" or "partner pd" - maybe in future we'll want to include pd info on mosys, just like ectool pdchipinfo. So, if we want to go with mosys, I thin one approach is to add a "type" into pd info and print 'cros_pd' if it's a cros_pd.
,
Feb 15 2017
Particular for factory, I think /dev/cros_pd is good enough since we'll verify hardware existence by HWID & probe results first, so if cros_pd kernel driver was not setup properly then it should be caught before finalization.
,
Feb 15 2017
Mosys change is being reviewed here: https://chromium-review.googlesource.com/439864. Please be specific about your request if you have any.
,
Feb 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/factory/+/76674b1bb2399cbfa1881bed6a8de5b77547d4dd commit 76674b1bb2399cbfa1881bed6a8de5b77547d4dd Author: Hung-Te Lin <hungte@chromium.org> Date: Fri Feb 17 09:47:31 2017 gooftool: Read PD flashrom only if cros_pd driver exists. 'flashrom -p ec:type=pd' may not reliably return PD flashrom chipset. For now, we only want to process flash of Chrome EC based PD (cros_pd) (for reading firmware hash and write protection) so we can check /dev/cros_pd first before accessing PD via flashrom command. BUG=chromium:691901 TEST=make test Change-Id: I58131cf4c9fc76a4df915464ae016869656238e8 Reviewed-on: https://chromium-review.googlesource.com/443005 Commit-Ready: Hung-Te Lin <hungte@chromium.org> Tested-by: Hung-Te Lin <hungte@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> [modify] https://crrev.com/76674b1bb2399cbfa1881bed6a8de5b77547d4dd/py/gooftool/crosfw.py
,
Mar 31 2017
,
Mar 31 2017
Does the patch still need to land?
,
Mar 31 2017
#8 is a workaround to avoid calling flashrom/ectool hwhen /dev/cros_pd is not available. What happens is: ectool starts with the 'dev' method, call comm_init_dev(): it tries to open /dev/cros_pd, does not exists, return 1. ectool then goes to try the lpc method, but comm_init_lpc only supports ec. The proper way of fixing the problem would be to add a check that device_name is indeed 'cros_ec' before calling i2c or lpc method.
,
Apr 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/factory/+/bb0d85f4d83bf67961d7882032434443c900f75c commit bb0d85f4d83bf67961d7882032434443c900f75c Author: Hung-Te Lin <hungte@chromium.org> Date: Thu Apr 27 19:53:10 2017 gooftool: Read PD flashrom only if cros_pd driver exists. 'flashrom -p ec:type=pd' may not reliably return PD flashrom chipset. For now, we only want to process flash of Chrome EC based PD (cros_pd) (for reading firmware hash and write protection) so we can check /dev/cros_pd first before accessing PD via flashrom command. BUG=chromium:691901 TEST=make test Change-Id: I58131cf4c9fc76a4df915464ae016869656238e8 Reviewed-on: https://chromium-review.googlesource.com/443005 Commit-Ready: Hung-Te Lin <hungte@chromium.org> Tested-by: Hung-Te Lin <hungte@chromium.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/487838 Commit-Queue: Philip Chen <philipchen@chromium.org> Tested-by: Philip Chen <philipchen@chromium.org> [modify] https://crrev.com/bb0d85f4d83bf67961d7882032434443c900f75c/py/gooftool/crosfw.py |
|||
►
Sign in to add a comment |
|||
Comment 1 Deleted