Change in coreboot[master]: ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS
Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38539 )
Change subject: ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS ......................................................................
ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS
The new host command provides these static capabilities of each USB-PD port: 1) Port number 2) Power role: source, sink, dual 3) Try-power role: none, sink, source 4) Data role: dfp, ufp, dual 5) Port location: these come from power_manager
BUG=b:146506369 BRANCH=none TEST=compiles
Change-Id: I923e4b637a2f41ce173d378ba5030f1ae8c22222 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/ec/google/chromeec/ec_commands.h 1 file changed, 68 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/38539/1
diff --git a/src/ec/google/chromeec/ec_commands.h b/src/ec/google/chromeec/ec_commands.h index 86bd4f6..7b5a067 100644 --- a/src/ec/google/chromeec/ec_commands.h +++ b/src/ec/google/chromeec/ec_commands.h @@ -5853,6 +5853,74 @@ #define EC_CMD_REBOOT_AP_ON_G3 0x0127
/*****************************************************************************/ +/* Get PD port capabilities + * + * Returns the following static *capabilities* of the given port: + * 1) Power role: source, sink, or dual. It is not anticipated that + * future CrOS devices would ever be only a source, so the options are + * sink or dual. + * 2) Try-power role: source, sink, or none (practically speaking, I don't + * believe any CrOS device would support Try.SNK, so this would be source + * or none). + * 3) Data role: dfp, ufp, or dual. This will probably only be DFP or dual + * for CrOS devices. + */ +#define EC_CMD_GET_PD_PORT_CAPS 0x0128 + +enum ec_pd_power_role_caps { + EC_PD_POWER_ROLE_SOURCE = 0, + EC_PD_POWER_ROLE_SINK = 1, + EC_PD_POWER_ROLE_DUAL = 2, +}; + +enum ec_pd_try_power_role_caps { + EC_PD_TRY_POWER_ROLE_NONE = 0, + EC_PD_TRY_POWER_ROLE_SINK = 1, + EC_PD_TRY_POWER_ROLE_SOURCE = 2, +}; + +enum ec_pd_data_role_caps { + EC_PD_DATA_ROLE_DFP = 0, + EC_PD_DATA_ROLE_UFP = 1, + EC_PD_DATA_ROLE_DUAL = 2, +}; + +/* From: power_manager/power_supply_properties.proto */ +enum ec_pd_port_location { + /* The location of the port is unknown, or there's only one port. */ + EC_PD_PORT_LOCATION_UNKNOWN = 0, + + /* + * Various positions on the device. The first word describes the side of + * the device where the port is located while the second clarifies the + * position. For example, LEFT_BACK means the farthest-back port on the + * left side, while BACK_LEFT means the leftmost port on the back of the + * device. + */ + EC_PD_PORT_LOCATION_LEFT = 1, + EC_PD_PORT_LOCATION_RIGHT = 2, + EC_PD_PORT_LOCATION_BACK = 3, + EC_PD_PORT_LOCATION_FRONT = 4, + EC_PD_PORT_LOCATION_LEFT_FRONT = 5, + EC_PD_PORT_LOCATION_LEFT_BACK = 6, + EC_PD_PORT_LOCATION_RIGHT_FRONT = 7, + EC_PD_PORT_LOCATION_RIGHT_BACK = 8, + EC_PD_PORT_LOCATION_BACK_LEFT = 9, + EC_PD_PORT_LOCATION_BACK_RIGHT = 10, +}; + +struct ec_params_get_pd_port_caps { + uint8_t port; /* Which port to interrogate */ +} __ec_align1; + +struct ec_response_get_pd_port_caps { + uint8_t pd_power_role_cap; /* enum ec_pd_power_role_caps */ + uint8_t pd_try_power_role_cap; /* enum ec_pd_try_power_role_caps */ + uint8_t pd_data_role_cap; /* enum ec_pd_data_role_caps */ + uint8_t pd_port_location; /* enum ec_pd_port_location */ +} __ec_align1; + +/*****************************************************************************/ /* The command range 0x200-0x2FF is reserved for Rotor. */
/*****************************************************************************/
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38539
to look at the new patch set (#2).
Change subject: ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS ......................................................................
ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS
The new host command provides these static capabilities of each USB-PD port: 1) Port number 2) Power role: source, sink, dual 3) Try-power role: none, sink, source 4) Data role: dfp, ufp, dual 5) Port location: these come from power_manager
BUG=b:146506369 BRANCH=none TEST=compiles
Change-Id: I923e4b637a2f41ce173d378ba5030f1ae8c22222 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/ec/google/chromeec/ec_commands.h 1 file changed, 68 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/38539/2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38539 )
Change subject: ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38539/2/src/ec/google/chromeec/ec_c... File src/ec/google/chromeec/ec_commands.h:
https://review.coreboot.org/c/coreboot/+/38539/2/src/ec/google/chromeec/ec_c... PS2, Line 5898: * device. Reflow for 96 character text width?
https://review.coreboot.org/c/coreboot/+/38539/2/src/ec/google/chromeec/ec_c... PS2, Line 5900: Tabs?
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38539 )
Change subject: ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38539/2/src/ec/google/chromeec/ec_c... File src/ec/google/chromeec/ec_commands.h:
https://review.coreboot.org/c/coreboot/+/38539/2/src/ec/google/chromeec/ec_c... PS2, Line 5898: * device.
Reflow for 96 character text width?
This header is used in multiple codebases, most of which still maintain 80 char width.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38539 )
Change subject: ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38539/2/src/ec/google/chromeec/ec_c... File src/ec/google/chromeec/ec_commands.h:
https://review.coreboot.org/c/coreboot/+/38539/2/src/ec/google/chromeec/ec_c... PS2, Line 5900:
Tabs?
This file is actually copied over from EC code base: https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/master... and so it would be good to maintain consistency with that.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38539 )
Change subject: ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS ......................................................................
Patch Set 2: Code-Review+2
Hello Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38539
to look at the new patch set (#3).
Change subject: ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS ......................................................................
ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS
The new host command provides these static capabilities of each USB-PD port: 1) Port number 2) Power role: source, sink, dual 3) Try-power role: none, sink, source 4) Data role: dfp, ufp, dual 5) Port location: these come from power_manager
BUG=b:146506369 BRANCH=none TEST=compiles
Change-Id: I923e4b637a2f41ce173d378ba5030f1ae8c22222 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/ec/google/chromeec/ec_commands.h 1 file changed, 67 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/38539/3
Hello Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38539
to look at the new patch set (#4).
Change subject: ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS ......................................................................
ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS
The new host command provides these static capabilities of each USB-PD port: 1) Port number 2) Power role: source, sink, dual 3) Try-power role: none, sink, source 4) Data role: dfp, ufp, dual 5) Port location: these come from power_manager
BUG=b:146506369 BRANCH=none TEST=compiles
Change-Id: I923e4b637a2f41ce173d378ba5030f1ae8c22222 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/ec/google/chromeec/ec_commands.h 1 file changed, 67 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/38539/4
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38539 )
Change subject: ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38539/4/src/ec/google/chromeec/ec_c... File src/ec/google/chromeec/ec_commands.h:
https://review.coreboot.org/c/coreboot/+/38539/4/src/ec/google/chromeec/ec_c... PS4, Line 5894: Various positions on the device. The first word describes the side of the device I understand that you have changed this to make it more aligned with coreboot coding style, but this file is generally copied over from EC codebase as is. And even if you make the change here today, in the future any time the file is copied over again, this will get overwritten and show as a diff. As Duncan mentioned on an earlier patchset, this header is used in multiple codebases and so it would be good to maintain consistency with the EC source.
Hello Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38539
to look at the new patch set (#5).
Change subject: ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS ......................................................................
ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS
The new host command provides these static capabilities of each USB-PD port: 1) Port number 2) Power role: source, sink, dual 3) Try-power role: none, sink, source 4) Data role: dfp, ufp, dual 5) Port location: these come from power_manager
BUG=b:146506369 BRANCH=none TEST=compiles
Change-Id: I923e4b637a2f41ce173d378ba5030f1ae8c22222 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/ec/google/chromeec/ec_commands.h 1 file changed, 68 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/38539/5
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38539 )
Change subject: ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38539/4/src/ec/google/chromeec/ec_c... File src/ec/google/chromeec/ec_commands.h:
https://review.coreboot.org/c/coreboot/+/38539/4/src/ec/google/chromeec/ec_c... PS4, Line 5894: Various positions on the device. The first word describes the side of the device
I understand that you have changed this to make it more aligned with coreboot coding style, but this […]
Fair, will revert.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38539 )
Change subject: ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38539/2/src/ec/google/chromeec/ec_c... File src/ec/google/chromeec/ec_commands.h:
https://review.coreboot.org/c/coreboot/+/38539/2/src/ec/google/chromeec/ec_c... PS2, Line 5898: * device.
This header is used in multiple codebases, most of which still maintain 80 char width.
Done
https://review.coreboot.org/c/coreboot/+/38539/2/src/ec/google/chromeec/ec_c... PS2, Line 5900:
This file is actually copied over from EC code base: https://chromium.googlesource. […]
Done
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38539 )
Change subject: ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS ......................................................................
ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS
The new host command provides these static capabilities of each USB-PD port: 1) Port number 2) Power role: source, sink, dual 3) Try-power role: none, sink, source 4) Data role: dfp, ufp, dual 5) Port location: these come from power_manager
BUG=b:146506369 BRANCH=none TEST=compiles
Change-Id: I923e4b637a2f41ce173d378ba5030f1ae8c22222 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/38539 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/ec/google/chromeec/ec_commands.h 1 file changed, 68 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/ec/google/chromeec/ec_commands.h b/src/ec/google/chromeec/ec_commands.h index 86bd4f6..7b5a067 100644 --- a/src/ec/google/chromeec/ec_commands.h +++ b/src/ec/google/chromeec/ec_commands.h @@ -5853,6 +5853,74 @@ #define EC_CMD_REBOOT_AP_ON_G3 0x0127
/*****************************************************************************/ +/* Get PD port capabilities + * + * Returns the following static *capabilities* of the given port: + * 1) Power role: source, sink, or dual. It is not anticipated that + * future CrOS devices would ever be only a source, so the options are + * sink or dual. + * 2) Try-power role: source, sink, or none (practically speaking, I don't + * believe any CrOS device would support Try.SNK, so this would be source + * or none). + * 3) Data role: dfp, ufp, or dual. This will probably only be DFP or dual + * for CrOS devices. + */ +#define EC_CMD_GET_PD_PORT_CAPS 0x0128 + +enum ec_pd_power_role_caps { + EC_PD_POWER_ROLE_SOURCE = 0, + EC_PD_POWER_ROLE_SINK = 1, + EC_PD_POWER_ROLE_DUAL = 2, +}; + +enum ec_pd_try_power_role_caps { + EC_PD_TRY_POWER_ROLE_NONE = 0, + EC_PD_TRY_POWER_ROLE_SINK = 1, + EC_PD_TRY_POWER_ROLE_SOURCE = 2, +}; + +enum ec_pd_data_role_caps { + EC_PD_DATA_ROLE_DFP = 0, + EC_PD_DATA_ROLE_UFP = 1, + EC_PD_DATA_ROLE_DUAL = 2, +}; + +/* From: power_manager/power_supply_properties.proto */ +enum ec_pd_port_location { + /* The location of the port is unknown, or there's only one port. */ + EC_PD_PORT_LOCATION_UNKNOWN = 0, + + /* + * Various positions on the device. The first word describes the side of + * the device where the port is located while the second clarifies the + * position. For example, LEFT_BACK means the farthest-back port on the + * left side, while BACK_LEFT means the leftmost port on the back of the + * device. + */ + EC_PD_PORT_LOCATION_LEFT = 1, + EC_PD_PORT_LOCATION_RIGHT = 2, + EC_PD_PORT_LOCATION_BACK = 3, + EC_PD_PORT_LOCATION_FRONT = 4, + EC_PD_PORT_LOCATION_LEFT_FRONT = 5, + EC_PD_PORT_LOCATION_LEFT_BACK = 6, + EC_PD_PORT_LOCATION_RIGHT_FRONT = 7, + EC_PD_PORT_LOCATION_RIGHT_BACK = 8, + EC_PD_PORT_LOCATION_BACK_LEFT = 9, + EC_PD_PORT_LOCATION_BACK_RIGHT = 10, +}; + +struct ec_params_get_pd_port_caps { + uint8_t port; /* Which port to interrogate */ +} __ec_align1; + +struct ec_response_get_pd_port_caps { + uint8_t pd_power_role_cap; /* enum ec_pd_power_role_caps */ + uint8_t pd_try_power_role_cap; /* enum ec_pd_try_power_role_caps */ + uint8_t pd_data_role_cap; /* enum ec_pd_data_role_caps */ + uint8_t pd_port_location; /* enum ec_pd_port_location */ +} __ec_align1; + +/*****************************************************************************/ /* The command range 0x200-0x2FF is reserved for Rotor. */
/*****************************************************************************/
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38539 )
Change subject: ec/google/chromeec: Add new host command, EC_CMD_GET_PD_PORT_CAPS ......................................................................
Patch Set 6:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/383 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/382 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/381
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Duncan Laurie (Code Review)
-
Furquan Shaikh (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Tim Wawrzynczak (Code Review)