Usbc source #52993

sambhurst · 2022-12-12T21:43:08Z

Adds Power Delivery Source Support to the USB-C Stack. See Issue 38371 for details

fabiobaltieri · 2022-12-13T18:15:34Z

Hey @sambhurst, do you think it would make sense to regroup the commints in smaller PRs? Looks like there's a mix of refactoring/preparation, board specific changes, bug fixes and then the main feature. Thinking having more small PRs may make it easier to review and hopefully get it merged faster, and also make it easier to maintain the stack through the reviews.

subsys/usb/usb_c/usbc_pe_common.c

keith-zephyr · 2023-04-18T15:58:07Z

include/zephyr/usb_c/usbc.h

+ * @brief Callback type used to get the Sink Capabilities
+ *
+ * @param dev dev Runtime device structure
+ * @param pdos pointer where pdos are stored
+ * @param num_pdos pointer where number of pdos is stored
+ * @return 0 on success


Nit- put the callback descriptions for the SNK into a separate commit.

Also, please put the src/snk/common callbacks into their own doxygen groups.

You need to add this group to the API reference section of ucds.rst.

keith-zephyr · 2023-04-18T16:02:12Z

doc/connectivity/usb_c/ucds.rst

@@ -14,7 +14,7 @@ It provides the following functionalities:
  The APIs is described in
  :zephyr_file:`include/zephyr/usb_c/usbc.h`

-Currently the device stack only support implementation of Sink devices.
+Currently the device stack supports implementation of Sink and Source devices.


Suggested change

Currently the device stack supports implementation of Sink and Source devices.

Currently the device stack supports implementation of Sink only and Source only devices. Dual Role Power (DRP) devices are not yet supported.

Transmit errors must be treated differently than Protocol Errors. This change sets a flag that informs the stack of a Message Transmit error. Signed-off-by: Sam Hurst <[email protected]>

alevkoy

I still have some comments open from my March 27 and 28 reviews. Mostly LGTM.

subsys/usb/usb_c/usbc_pe_common.c

keith-zephyr · 2023-04-24T19:24:44Z

include/zephyr/usb_c/usbc.h

+ * @brief Callback type used to get the Sink Capabilities
+ *
+ * @param dev dev Runtime device structure
+ * @param pdos pointer where pdos are stored
+ * @param num_pdos pointer where number of pdos is stored
+ * @return 0 on success


You need to add this group to the API reference section of ucds.rst.

keith-zephyr · 2023-04-24T19:30:33Z

include/zephyr/usb_c/usbc.h

+/**
+ * @brief Callback type used to get the Sink Capabilities
+ *
+ * @param dev dev Runtime device structure


Please specify the type of device passed in for all these prototypes.

Suggested change

* @param dev dev Runtime device structure

* @param dev USB-C Connector Instance

include/zephyr/usb_c/usbc.h

keith-zephyr · 2023-04-24T19:45:01Z

include/zephyr/usb_c/usbc.h

+typedef enum usbc_snk_req_reply_t (*policy_cb_check_sink_request_t)(
+				const struct device *dev,
+				const uint32_t request_msg);


Looks like this file needs clang-format.

keith-zephyr · 2023-04-24T19:45:59Z

include/zephyr/usb_c/usbc.h

+ * @param dev dev Runtime device structure
+ * @return true if power supply is ready, else false
+ */
+typedef bool (*policy_cb_check_if_ps_ready_t)(const struct device *dev);


Nit - you could name this policy_cb_is_ps_ready_t

keith-zephyr · 2023-04-24T19:49:24Z

include/zephyr/usb_c/usbc.h

+ * @param present_contract present contract
+ * @return true if present contract is still valid
+ */
+typedef bool (*policy_cb_check_present_contract_t)(const struct device *dev,


Nit - policy_cb_present_contract_is_valid

keith-zephyr · 2023-04-24T19:53:54Z

include/zephyr/usb_c/usbc.h

+typedef bool (*policy_cb_change_src_caps_t)(const struct device *dev);
+
+/**
+ * @brief Callback type used to store the Capabilities received from


Suggested change

* @brief Callback type used to store the Capabilities received from

* @brief Callback type used to report the Capabilities received from

subsys/usb/usb_c/usbc_pe_common.c

keith-zephyr · 2023-04-26T20:12:49Z

include/zephyr/usb_c/usbc.h

+/**
+ * @brief Callback type used to check a policy
+ *
+ * @param dev dev Runtime device structure


Suggested change

* @param dev dev Runtime device structure

* @param dev USB-C Connector Instance

include/zephyr/usb_c/usbc.h

keith-zephyr · 2023-04-26T20:15:53Z

include/zephyr/usb_c/usbc.h

+ * @brief Callback type used to get the Rp value that should be placed on
+ *        the CC lines
+ *
+ * @param dev dev Runtime device structure


Suggested change

* @param dev dev Runtime device structure

* @param dev USB-C Connector Instance

Replace all instances for the APIs your are introducing in this commit.

Document the sink callbacks Signed-off-by: Sam Hurst <[email protected]>

Detect and flag the reception of the first AMS mesage Signed-off-by: Sam Hurst <[email protected]>

Refactor soft reset code Signed-off-by: Sam Hurst <[email protected]>

keith-zephyr

One last nit on the documentation from me.

keith-zephyr · 2023-04-27T16:30:07Z

include/zephyr/usb_c/usbc.h

+ *        the CC lines
+ *
+ * @param dev USB-C Connector Instance
+ * @param rp


Missing description for the rp parameter. I thought the CI had checks for this.

Add USB-C Power Delivery Source Support to USB-C Subsystem Signed-off-by: Sam Hurst <[email protected]>

Implementing USB-C Source functionality can be difficult. This sample application serves as an example of how to create an application with Power Delivery Source functionality. Signed-off-by: Sam Hurst <[email protected]>

Documents the USB-C Source Subsystem API. Signed-off-by: Sam Hurst <[email protected]>

subsys/usb/usb_c/usbc_pe_src_states.c

kartben · 2023-05-16T09:29:53Z

include/zephyr/usb_c/usbc.h

+ * @brief Callback type used to get the Rp value that should be placed on
+ *        the CC lines
+ *
+ * @param dev USB-C Connector Instance
+ * @param rp Rp value
+ * @return 0 on success
+ */
+typedef int (*policy_cb_get_src_rp_t)(const struct device *dev, enum tc_rp_value *rp);
+
+/**
+ * @brief Callback type used to enable VBUS
+ *
+ * @param dev USB-C Connector Instance
+ * @param en true to enable VBUS, else disable it
+ * @return 0 on success
+ */
+typedef int (*policy_cb_src_en_t)(const struct device *dev, bool en);


@sambhurst this is defined again in in L324-340. I am not sure why this wasn't caught by Doxygen (although I just had Doxygen 1.9.4 spit out a warning). I will submit a fix.

sambhurst requested review from nashif, carlescufi, galak, MaureenHelm, erwango, ABOSTM and FRASTM as code owners December 12, 2022 21:43

zephyrbot added platform: STM32 ST Micro STM32 area: USB-C area: USB Universal Serial Bus labels Dec 12, 2022

zephyrbot requested review from desowin, gautierg-st, GeorgeCGV, jfischer-no and keith-zephyr December 12, 2022 21:43

zephyrbot assigned sambhurst Dec 12, 2022

sambhurst removed request for desowin, GeorgeCGV, jfischer-no and gautierg-st December 12, 2022 21:43

sambhurst force-pushed the usbc_source branch from 578889e to 7893733 Compare December 12, 2022 22:35

zephyrbot requested review from desowin, gautierg-st, GeorgeCGV and jfischer-no December 12, 2022 22:35

sambhurst force-pushed the usbc_source branch from 7893733 to d85f317 Compare December 12, 2022 22:53

keith-zephyr requested a review from alevkoy December 12, 2022 22:55

sambhurst force-pushed the usbc_source branch from d85f317 to 160d0f8 Compare December 12, 2022 23:36

keith-zephyr requested a review from DianaZig December 13, 2022 19:01

sambhurst force-pushed the usbc_source branch from 68e7c28 to 5241d95 Compare April 16, 2023 19:28

keith-zephyr requested changes Apr 18, 2023

View reviewed changes

usb_c: Transmit Error are not Protocol Errors

cdc2e97

Transmit errors must be treated differently than Protocol Errors. This change sets a flag that informs the stack of a Message Transmit error. Signed-off-by: Sam Hurst <[email protected]>

sambhurst force-pushed the usbc_source branch 2 times, most recently from e0c9c30 to e2e3a4e Compare April 24, 2023 01:51

sambhurst requested review from keith-zephyr and DianaZig April 24, 2023 02:46

alevkoy reviewed Apr 24, 2023

View reviewed changes

keith-zephyr requested changes Apr 24, 2023

View reviewed changes

sambhurst force-pushed the usbc_source branch from e2e3a4e to a48957c Compare April 26, 2023 14:10

sambhurst requested review from keith-zephyr and alevkoy April 26, 2023 14:13

sambhurst force-pushed the usbc_source branch from a48957c to c8d20a7 Compare April 26, 2023 15:00

keith-zephyr requested changes Apr 26, 2023

View reviewed changes

sambhurst added 3 commits April 27, 2023 10:48

usb-c: Document the sink callbacks

074e5a9

Document the sink callbacks Signed-off-by: Sam Hurst <[email protected]>

usb-c: Properly detect reception of first AMS message

6010a57

Detect and flag the reception of the first AMS mesage Signed-off-by: Sam Hurst <[email protected]>

usb-c: Refactor soft reset code

e83dfa6

Refactor soft reset code Signed-off-by: Sam Hurst <[email protected]>

sambhurst force-pushed the usbc_source branch from c8d20a7 to ae8b330 Compare April 27, 2023 15:15

sambhurst requested a review from keith-zephyr April 27, 2023 15:17

keith-zephyr reviewed Apr 27, 2023

View reviewed changes

sambhurst added 3 commits April 27, 2023 14:45

usb-c: Add USB-C Subsystem Source PD Support

cf32ab8

Add USB-C Power Delivery Source Support to USB-C Subsystem Signed-off-by: Sam Hurst <[email protected]>

samples: usb-c: source: Add USB-C Subsystem Source Sample

270d258

Implementing USB-C Source functionality can be difficult. This sample application serves as an example of how to create an application with Power Delivery Source functionality. Signed-off-by: Sam Hurst <[email protected]>

doc: Document the USB-C Source Subsystem

4923970

Documents the USB-C Source Subsystem API. Signed-off-by: Sam Hurst <[email protected]>

sambhurst force-pushed the usbc_source branch from ae8b330 to 4923970 Compare April 27, 2023 18:46

sambhurst requested a review from keith-zephyr April 27, 2023 18:55

keith-zephyr approved these changes Apr 27, 2023

View reviewed changes

alevkoy approved these changes Apr 27, 2023

View reviewed changes

subsys/usb/usb_c/usbc_pe_src_states.c Show resolved Hide resolved

carlescufi merged commit 714eec8 into zephyrproject-rtos:main Apr 28, 2023

sambhurst deleted the usbc_source branch May 2, 2023 17:59

kartben reviewed May 16, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Usbc source #52993

Usbc source #52993

sambhurst commented Dec 12, 2022

fabiobaltieri commented Dec 13, 2022

keith-zephyr Apr 18, 2023

keith-zephyr Apr 18, 2023

sambhurst Apr 24, 2023

keith-zephyr Apr 24, 2023

keith-zephyr Apr 18, 2023

sambhurst Apr 24, 2023

alevkoy left a comment

keith-zephyr Apr 24, 2023

keith-zephyr Apr 24, 2023

sambhurst Apr 26, 2023

keith-zephyr Apr 24, 2023

sambhurst Apr 26, 2023

keith-zephyr Apr 24, 2023

sambhurst Apr 26, 2023

keith-zephyr Apr 24, 2023

sambhurst Apr 26, 2023

keith-zephyr Apr 24, 2023

sambhurst Apr 26, 2023

keith-zephyr Apr 26, 2023

sambhurst Apr 27, 2023

keith-zephyr Apr 26, 2023

sambhurst Apr 27, 2023

keith-zephyr left a comment

keith-zephyr Apr 27, 2023

sambhurst Apr 27, 2023

kartben May 16, 2023

kartben May 16, 2023

	Currently the device stack supports implementation of Sink and Source devices.
	Currently the device stack supports implementation of Sink only and Source only devices. Dual Role Power (DRP) devices are not yet supported.

	* @param dev dev Runtime device structure
	* @param dev USB-C Connector Instance

	* @brief Callback type used to store the Capabilities received from
	* @brief Callback type used to report the Capabilities received from

Usbc source #52993

Usbc source #52993

Conversation

sambhurst commented Dec 12, 2022

fabiobaltieri commented Dec 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alevkoy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keith-zephyr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment