New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 681240 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Feature



Sign in to add a comment

EC: Add presubmit hook to enforce host command definitions.

Project Member Reported by aaboagye@chromium.org, Jan 14 2017

Issue description

With the recent landing of this CL: https://chromium-review.googlesource.com/419702, host command codes must now be defined using 4 hexdigits and be upper case. The only way to enforce this is to make sure that it is caught during review time (or to try using CONFIG_HOSTCMD_SECTION_SORTED and then find something break).

A better way would be to create presubmit check for when new host commands are added.


 

Comment 1 by shurst@google.com, Jan 30 2017

Owner: shu...@chromium.org

Comment 2 by shu...@chromium.org, Feb 21 2017

Status: Started (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/fc75244eb90920d6c4954586d00f15fe511d0c11

commit fc75244eb90920d6c4954586d00f15fe511d0c11
Author: Sam Hurst <shurst@google.com>
Date: Sat Feb 25 10:08:37 2017

ec:Add presubmit hook to enforce host command definitions

Make sure all public and private host commands starting with
EC_CMD_ and EC_PRV_CMD_ are properly formed

BUG= chromium:681240 
TEST=manual.
    Added following host commands and verified that they were flagged
    #define EC_CMD_TESTA 1234
    #define EC_CMD_TESTB 0xabcd
    #define EC_CMD_TESTC 0x1ABCD
    #define EC_CMD_TESTD 0xXEF01
    #define EC_PRV_CMD_TESTA 1234
    #define EC_PRV_CMD_TESTB 0xabcd
    #define EC_PRV_CMD_TESTC 0x1ABCD
    #define EC_PRV_CMD_TESTD 0XEF01

    These were also flagged by the script
    include/ec_commands.h:#define EC_CMD_ACPI_READ 0x80
    include/ec_commands.h:#define EC_CMD_ACPI_WRITE 0x81
    include/ec_commands.h:#define EC_CMD_ACPI_BURST_ENABLE 0x82
    include/ec_commands.h:#define EC_CMD_ACPI_BURST_DISABLE 0x83
    include/ec_commands.h:#define EC_CMD_ACPI_QUERY_EVENT 0x84
CQ-DEPEND=CL:445809
BRANCH=none

Change-Id: I4630d6a887ed289a68178e8f1a8f07f5141c80bc
Reviewed-on: https://chromium-review.googlesource.com/445811
Commit-Ready: Sam Hurst <shurst@google.com>
Tested-by: Sam Hurst <shurst@google.com>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Shawn N <shawnn@chromium.org>

[add] https://crrev.com/fc75244eb90920d6c4954586d00f15fe511d0c11/util/host_command_check.sh
[modify] https://crrev.com/fc75244eb90920d6c4954586d00f15fe511d0c11/PRESUBMIT.cfg

Comment 4 by shu...@chromium.org, Feb 27 2017

Status: Verified (was: Started)

Sign in to add a comment