Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38540 )
Change subject: ec/google/chromeec: Add new wrappers for host commands ......................................................................
ec/google/chromeec: Add new wrappers for host commands
Add new functions to get (from the EC): 1) The number of USB-PD ports 2) The capabilities of each port (EC_CMD_GET_PD_PORT_CAPS)
BUG=b:146506369 BRANCH=none TEST=Instrumented calls to these and verified the data
Change-Id: I57edbe1592cd28b005f01679ef8a8b5de3e1f586 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h 2 files changed, 86 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/38540/1
diff --git a/src/ec/google/chromeec/ec.c b/src/ec/google/chromeec/ec.c index 0b23034..7a4f27f 100644 --- a/src/ec/google/chromeec/ec.c +++ b/src/ec/google/chromeec/ec.c @@ -15,16 +15,21 @@
#include <stdint.h> #include <string.h> -#include <cbmem.h> -#include <console/console.h> +#include <arch/acpi.h> +#include <arch/acpi_device.h> +#include <arch/acpigen.h> #include <assert.h> #include <bootmode.h> #include <bootstate.h> +#include <cbmem.h> +#include <console/console.h> #include <delay.h> +#include <device/device.h> +#include <device/path.h> #include <elog.h> #include <rtc.h> -#include <stdlib.h> #include <security/vboot/vboot_common.h> +#include <stdlib.h> #include <timer.h>
#include "chip.h" @@ -1419,6 +1424,60 @@ return ec_image_type; }
+int google_chromeec_get_num_pd_ports(int *num_ports) +{ + struct ec_response_charge_port_count resp = {}; + struct chromeec_command cmd = { + .cmd_code = EC_CMD_CHARGE_PORT_COUNT, + .cmd_version = 0, + .cmd_data_out = &resp, + .cmd_size_in = 0, + .cmd_size_out = sizeof(resp), + .cmd_dev_index = 0, + }; + int rv; + + rv = google_chromeec_command(&cmd); + if (rv) + return rv; + + *num_ports = resp.port_count; + return 0; +} + +int google_chromeec_get_pd_port_caps(int port, + enum ec_pd_power_role_caps *power_role_cap, + enum ec_pd_try_power_role_caps *try_power_role_cap, + enum ec_pd_data_role_caps *data_role_cap, + enum ec_pd_port_location *port_location) +{ + struct ec_params_get_pd_port_caps params = { + .port = port, + }; + struct ec_response_get_pd_port_caps resp = {}; + struct chromeec_command cmd = { + .cmd_code = EC_CMD_GET_PD_PORT_CAPS, + .cmd_version = 0, + .cmd_data_in = ¶ms, + .cmd_size_in = sizeof(params), + .cmd_data_out = &resp, + .cmd_size_out = sizeof(resp), + .cmd_dev_index = 0, + }; + int rv; + + rv = google_chromeec_command(&cmd); + if (rv) + return rv; + + *power_role_cap = resp.pd_power_role_cap; + *try_power_role_cap = resp.pd_try_power_role_cap; + *data_role_cap = resp.pd_data_role_cap; + *port_location = resp.pd_port_location; + + return 0; +} + void google_chromeec_init(void) { google_chromeec_log_uptimeinfo(); diff --git a/src/ec/google/chromeec/ec.h b/src/ec/google/chromeec/ec.h index d90d24c..8051486 100644 --- a/src/ec/google/chromeec/ec.h +++ b/src/ec/google/chromeec/ec.h @@ -299,4 +299,28 @@ */ int google_chromeec_get_cmd_versions(int command, uint32_t *pmask);
+/** + * Get number of PD-capable USB ports from EC. + * + * @param *num_ports If successful, num_ports is the number + * of PD-capable USB ports according to the EC. + * @return 0 on success, -1 on error + */ +int google_chromeec_get_num_pd_ports(int *num_ports); + +/** + * Get role-based capabilities for a USB-PD port + * + * @param port Which port to get information about + * @param *power_role_cap The power-role capabillity of the port + * @param *try_power_role_cap The Try-power-role capability of the port + * @param *data_role_cap The data role capability of the port + * @param *port_location Location of the port on the device + * @return 0 on success, -1 on error + */ +int google_chromeec_get_pd_port_caps(int port, + enum ec_pd_power_role_caps *power_role_cap, + enum ec_pd_try_power_role_caps *try_power_role_cap, + enum ec_pd_data_role_caps *data_role_cap, + enum ec_pd_port_location *port_location); #endif /* _EC_GOOGLE_CHROMEEC_EC_H */
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38540
to look at the new patch set (#2).
Change subject: ec/google/chromeec: Add new wrappers for host commands ......................................................................
ec/google/chromeec: Add new wrappers for host commands
Add new functions to get (from the EC): 1) The number of USB-PD ports 2) The capabilities of each port (EC_CMD_GET_PD_PORT_CAPS)
BUG=b:146506369 BRANCH=none TEST=Instrumented calls to these and verified the data
Change-Id: I57edbe1592cd28b005f01679ef8a8b5de3e1f586 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h 2 files changed, 83 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/38540/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38540 )
Change subject: ec/google/chromeec: Add new wrappers for host commands ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38540/2/src/ec/google/chromeec/ec.h File src/ec/google/chromeec/ec.h:
https://review.coreboot.org/c/coreboot/+/38540/2/src/ec/google/chromeec/ec.h... PS2, Line 309: int *num_ports Just curious: Is this input parameter strictly required? Can we return -1 on error and # of ports otherwise. Or do you want to take in input parameter for consistency with other EC functions here?
https://review.coreboot.org/c/coreboot/+/38540/2/src/ec/google/chromeec/ec.h... PS2, Line 322: enum ec_pd_power_role_caps *power_role_cap, : enum ec_pd_try_power_role_caps *try_power_role_cap, : enum ec_pd_data_role_caps *data_role_cap, : enum ec_pd_port_location *port_location Should these be put in a structure so that in the future if there are more things that need to be returned they can just be added to the structure without having to change each call site?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38540 )
Change subject: ec/google/chromeec: Add new wrappers for host commands ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38540/2/src/ec/google/chromeec/ec.h File src/ec/google/chromeec/ec.h:
https://review.coreboot.org/c/coreboot/+/38540/2/src/ec/google/chromeec/ec.h... PS2, Line 309: int *num_ports
Just curious: Is this input parameter strictly required? Can we return -1 on error and # of ports ot […]
That would be possible, sure. But as you mentioned, most of these functions return the error code and use "out" parameters for passing back data, so I just went with this for consistency.
https://review.coreboot.org/c/coreboot/+/38540/2/src/ec/google/chromeec/ec.h... PS2, Line 322: enum ec_pd_power_role_caps *power_role_cap, : enum ec_pd_try_power_role_caps *try_power_role_cap, : enum ec_pd_data_role_caps *data_role_cap, : enum ec_pd_port_location *port_location
Should these be put in a structure so that in the future if there are more things that need to be re […]
That's a good idea.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38540 )
Change subject: ec/google/chromeec: Add new wrappers for host commands ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38540/2/src/ec/google/chromeec/ec.h File src/ec/google/chromeec/ec.h:
https://review.coreboot.org/c/coreboot/+/38540/2/src/ec/google/chromeec/ec.h... PS2, Line 309: int *num_ports
That would be possible, sure. […]
Ack
Hello Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38540
to look at the new patch set (#3).
Change subject: ec/google/chromeec: Add new wrappers for host commands ......................................................................
ec/google/chromeec: Add new wrappers for host commands
Add new functions to get (from the EC): 1) The number of USB-PD ports 2) The capabilities of each port (EC_CMD_GET_PD_PORT_CAPS)
BUG=b:146506369 BRANCH=none TEST=Instrumented calls to these and verified the data
Change-Id: I57edbe1592cd28b005f01679ef8a8b5de3e1f586 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h 2 files changed, 87 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/38540/3
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38540 )
Change subject: ec/google/chromeec: Add new wrappers for host commands ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38540/3/src/ec/google/chromeec/ec.h File src/ec/google/chromeec/ec.h:
https://review.coreboot.org/c/coreboot/+/38540/3/src/ec/google/chromeec/ec.h... PS3, Line 313: { open brace '{' following struct go on the same line
Hello Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38540
to look at the new patch set (#4).
Change subject: ec/google/chromeec: Add new wrappers for host commands ......................................................................
ec/google/chromeec: Add new wrappers for host commands
Add new functions to get (from the EC): 1) The number of USB-PD ports 2) The capabilities of each port (EC_CMD_GET_PD_PORT_CAPS)
BUG=b:146506369 BRANCH=none TEST=Instrumented calls to these and verified the data
Change-Id: I57edbe1592cd28b005f01679ef8a8b5de3e1f586 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h 2 files changed, 86 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/38540/4
Hello Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38540
to look at the new patch set (#5).
Change subject: ec/google/chromeec: Add new wrappers for host commands ......................................................................
ec/google/chromeec: Add new wrappers for host commands
Add new functions to get (from the EC): 1) The number of USB-PD ports 2) The capabilities of each port (EC_CMD_GET_PD_PORT_CAPS)
BUG=b:146506369 BRANCH=none TEST=Instrumented calls to these and verified the data
Change-Id: I57edbe1592cd28b005f01679ef8a8b5de3e1f586 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h 2 files changed, 87 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/38540/5
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38540 )
Change subject: ec/google/chromeec: Add new wrappers for host commands ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38540/5/src/ec/google/chromeec/ec.h File src/ec/google/chromeec/ec.h:
https://review.coreboot.org/c/coreboot/+/38540/5/src/ec/google/chromeec/ec.h... PS5, Line 313: { open brace '{' following struct go on the same line
Hello Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/38540
to look at the new patch set (#6).
Change subject: ec/google/chromeec: Add new wrappers for host commands ......................................................................
ec/google/chromeec: Add new wrappers for host commands
Add new functions to get (from the EC): 1) The number of USB-PD ports 2) The capabilities of each port (EC_CMD_GET_PD_PORT_CAPS)
BUG=b:146506369 BRANCH=none TEST=Instrumented calls to these and verified the data
Change-Id: I57edbe1592cd28b005f01679ef8a8b5de3e1f586 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h 2 files changed, 86 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/38540/6
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38540 )
Change subject: ec/google/chromeec: Add new wrappers for host commands ......................................................................
Patch Set 6: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38540 )
Change subject: ec/google/chromeec: Add new wrappers for host commands ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38540/7/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/c/coreboot/+/38540/7/src/ec/google/chromeec/ec.c... PS7, Line 1424: int Why not make it unsigned?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38540 )
Change subject: ec/google/chromeec: Add new wrappers for host commands ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38540/7/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/c/coreboot/+/38540/7/src/ec/google/chromeec/ec.c... PS7, Line 1424: int
Why not make it unsigned?
The API for this file is to return negative values on error (usually -1), and 0 for success, while any "returned values" are returned through out arguments, so I'm just keeping it consistent here.
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38540 )
Change subject: ec/google/chromeec: Add new wrappers for host commands ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38540/7/src/ec/google/chromeec/ec.c File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/c/coreboot/+/38540/7/src/ec/google/chromeec/ec.c... PS7, Line 1424: int
The API for this file is to return negative values on error (usually -1), and 0 for success, while a […]
Ack
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38540 )
Change subject: ec/google/chromeec: Add new wrappers for host commands ......................................................................
ec/google/chromeec: Add new wrappers for host commands
Add new functions to get (from the EC): 1) The number of USB-PD ports 2) The capabilities of each port (EC_CMD_GET_PD_PORT_CAPS)
BUG=b:146506369 BRANCH=none TEST=Instrumented calls to these and verified the data
Change-Id: I57edbe1592cd28b005f01679ef8a8b5de3e1f586 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/38540 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/ec/google/chromeec/ec.c M src/ec/google/chromeec/ec.h 2 files changed, 86 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/ec/google/chromeec/ec.c b/src/ec/google/chromeec/ec.c index 0b23034..4bf41ac 100644 --- a/src/ec/google/chromeec/ec.c +++ b/src/ec/google/chromeec/ec.c @@ -15,16 +15,18 @@
#include <stdint.h> #include <string.h> -#include <cbmem.h> -#include <console/console.h> #include <assert.h> #include <bootmode.h> #include <bootstate.h> +#include <cbmem.h> +#include <console/console.h> #include <delay.h> +#include <device/device.h> +#include <device/path.h> #include <elog.h> #include <rtc.h> -#include <stdlib.h> #include <security/vboot/vboot_common.h> +#include <stdlib.h> #include <timer.h>
#include "chip.h" @@ -1419,6 +1421,57 @@ return ec_image_type; }
+int google_chromeec_get_num_pd_ports(int *num_ports) +{ + struct ec_response_charge_port_count resp = {}; + struct chromeec_command cmd = { + .cmd_code = EC_CMD_CHARGE_PORT_COUNT, + .cmd_version = 0, + .cmd_data_out = &resp, + .cmd_size_in = 0, + .cmd_size_out = sizeof(resp), + .cmd_dev_index = 0, + }; + int rv; + + rv = google_chromeec_command(&cmd); + if (rv) + return rv; + + *num_ports = resp.port_count; + return 0; +} + +int google_chromeec_get_pd_port_caps(int port, + struct usb_pd_port_caps *port_caps) +{ + struct ec_params_get_pd_port_caps params = { + .port = port, + }; + struct ec_response_get_pd_port_caps resp = {}; + struct chromeec_command cmd = { + .cmd_code = EC_CMD_GET_PD_PORT_CAPS, + .cmd_version = 0, + .cmd_data_in = ¶ms, + .cmd_size_in = sizeof(params), + .cmd_data_out = &resp, + .cmd_size_out = sizeof(resp), + .cmd_dev_index = 0, + }; + int rv; + + rv = google_chromeec_command(&cmd); + if (rv) + return rv; + + port_caps->power_role_cap = resp.pd_power_role_cap; + port_caps->try_power_role_cap = resp.pd_try_power_role_cap; + port_caps->data_role_cap = resp.pd_data_role_cap; + port_caps->port_location = resp.pd_port_location; + + return 0; +} + void google_chromeec_init(void) { google_chromeec_log_uptimeinfo(); diff --git a/src/ec/google/chromeec/ec.h b/src/ec/google/chromeec/ec.h index d90d24c..5ce375e 100644 --- a/src/ec/google/chromeec/ec.h +++ b/src/ec/google/chromeec/ec.h @@ -299,4 +299,34 @@ */ int google_chromeec_get_cmd_versions(int command, uint32_t *pmask);
+/** + * Get number of PD-capable USB ports from EC. + * + * @param *num_ports If successful, num_ports is the number + * of PD-capable USB ports according to the EC. + * @return 0 on success, -1 on error + */ +int google_chromeec_get_num_pd_ports(int *num_ports); + +/* Structure representing the capabilities of a USB-PD port */ +struct usb_pd_port_caps { + enum ec_pd_power_role_caps power_role_cap; + enum ec_pd_try_power_role_caps try_power_role_cap; + enum ec_pd_data_role_caps data_role_cap; + enum ec_pd_port_location port_location; +}; + +/** + * Get role-based capabilities for a USB-PD port + * + * @param port Which port to get information about + * @param *power_role_cap The power-role capabillity of the port + * @param *try_power_role_cap The Try-power-role capability of the port + * @param *data_role_cap The data role capability of the port + * @param *port_location Location of the port on the device + * @return 0 on success, -1 on error + */ +int google_chromeec_get_pd_port_caps(int port, + struct usb_pd_port_caps *port_caps); + #endif /* _EC_GOOGLE_CHROMEEC_EC_H */
-
build bot (Jenkins) (Code Review) -
Furquan Shaikh (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Tim Wawrzynczak (Code Review)