Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers: usb_c:: support Nuvoton's NuMaker M2L31 series #73635

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ccli8
Copy link
Contributor

@ccli8 ccli8 commented Jun 3, 2024

This PR adds Nuvoton's NuMaker M2L31 series UTCPD support in usb_c:

  1. Implement usb_c drivers: tcpc, ppc, and vbus, where tcpc is the major, ppc and vbus rely on tcpc for their implementations.
  2. Support sink-only currently.

@zephyrbot
Copy link
Collaborator

zephyrbot commented Jun 3, 2024

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff

Note: This message is automatically posted and updated by the Manifest GitHub Action.

description: |
Rate of timer-triggered EADC measurement (Hz).
This is ignored when none of above is specified.
default: 100
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explained the default

}
NUMAKER_UTCPD_REG_WRITE_BY_NAME(dev, CMD, cmd);

cleanup:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cleanup: labels should be removed along with the other in the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally, goto cleanup is embedded in NUMAKER_UTCPD_REG_WRITE_BY_NAME. Now it is moved outside.


cleanup:

return rc;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just return 0. Same in other function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed as above


/* Functions below with name pattern "*_tcpc_ppc_*" are to invoke by NuMaker PPC driver */

int numaker_tcpc_ppc_is_dead_battery_mode(const struct device *dev)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these wrapper functions necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, they are to be invoked by ppc/vbus drivers. tcpc driver implements all the functions to match UTCPD H/W

};

/* TCPC exported */
int numaker_tcpc_ppc_is_dead_battery_mode(const struct device *dev);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These prototypes should be commented and placed in a header file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are moved to one internal header file.

*/

/* Invalid or missing value */
#define NUMAKER_INVALID_VALUE ((uint32_t)-1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0xFFFFFFFF

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UINT32_MAX is also an option

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Chose UINT32_MAX

enum adc_reference reference;

if (data->vref_mv) {
goto cleanup;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove goto cleanup and just return error value. Same in other functions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed most goto cleanup, keep only needed

@sambhurst
Copy link
Contributor

Please run clang-format and follow Zephyr Code Documentation

west.yml Outdated Show resolved Hide resolved
*/

/* Invalid or missing value */
#define NUMAKER_INVALID_VALUE ((uint32_t)-1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UINT32_MAX is also an option

drivers/usb_c/tcpc/ucpd_numaker.c Outdated Show resolved Hide resolved
drivers/usb_c/tcpc/ucpd_numaker.c Outdated Show resolved Hide resolved
drivers/usb_c/tcpc/ucpd_numaker.c Show resolved Hide resolved
drivers/usb_c/tcpc/ucpd_numaker.c Show resolved Hide resolved
drivers/usb_c/vbus/usbc_vbus_numaker.c Outdated Show resolved Hide resolved
@ccli8
Copy link
Contributor Author

ccli8 commented Jun 5, 2024

Please run clang-format and follow Zephyr Code Documentation

Run clang-format and follow doxygen to comment public functions

drivers/usb_c/vbus/usbc_vbus_numaker.c Outdated Show resolved Hide resolved
drivers/usb_c/tcpc/ucpd_numaker.c Show resolved Hide resolved
drivers/usb_c/tcpc/ucpd_numaker.c Outdated Show resolved Hide resolved
drivers/usb_c/tcpc/ucpd_numaker.c Outdated Show resolved Hide resolved
drivers/usb_c/tcpc/ucpd_numaker.c Outdated Show resolved Hide resolved
drivers/usb_c/tcpc/ucpd_numaker.c Outdated Show resolved Hide resolved
drivers/usb_c/tcpc/ucpd_numaker.c Outdated Show resolved Hide resolved
drivers/usb_c/tcpc/ucpd_numaker.c Outdated Show resolved Hide resolved
Comment on lines 40 to 42
struct numaker_ppc_data {
uint32_t dummy;
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop this too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, dropped this too

@ccli8
Copy link
Contributor Author

ccli8 commented Jun 12, 2024

rebase on main to fix conflict

@ccli8 ccli8 requested a review from fabiobaltieri June 13, 2024 09:59
@ccli8
Copy link
Contributor Author

ccli8 commented Jun 18, 2024

@fabiobaltieri Please re-review if you are available

1. Support USB-C drivers TCPC, PPC, and VBUS with UTCPD H/W IP
2. UTCPD is interconnected with Timer-triggered EADC for updating
   VBUS/VCONN voltage periodically

Signed-off-by: Chun-Chieh Li <ccli8@nuvoton.com>
Add support for NuMaker-M2L31 board

Signed-off-by: Chun-Chieh Li <ccli8@nuvoton.com>
@ccli8
Copy link
Contributor Author

ccli8 commented Jun 21, 2024

Rebase on main to merge in #73632, which fixes samples/subsys/usb_c/sink test failure because usb-c stack uses smf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: Samples Samples area: USB-C platform: Nuvoton NPCX Nuvoton NPCX platform: Nuvoton Numicro Numaker Nuvoton Technology Corporation, Numicro Numaker platform: Nuvoton Nuvoton
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants