Add support for UDC for Apollo4P family #79547

zeonchew · 2024-10-08T10:00:57Z

This PR adds support for UDC for Ambiq Apollo4p family. The boards tested are:

apollo4p_evb
apollo4p_blue_kxr_evb

Followings are the USB samples tested:

cdc_acm
console
hid-keyboard
hid-mouse
mass

zephyrbot · 2024-10-08T10:01:51Z

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.

tmon-nordic · 2024-10-08T10:15:16Z

drivers/usb/udc/udc_ambiq.c

+/* Max Packet Size for USB endpoints */
+#define EP0_MPS   64U
+#define EP_FS_MPS 64U
+#define EP_HS_MPS 512U


Later the code sets the iso capability on the endpoints. Are the endpoints limited in hardware to these specific values and not the maximum allowed values?

Hi @tmon-nordic, there is a limitation that the end of transaction is not able to be identified when OUT endpoint received data that is multiple of endpoint MPS, when the data count is lower than the triggered size. Hence, the safest way to expose this now is to limit it to 64 bytes for FS and 512 bytes for HS.

Where is this limitation, in your shim driver, HAL driver, or controller?

I think it can be categorized as controller limitation.

samples/subsys/usb/hid-keyboard/boards/apollo4p_blue_kxr_evb.overlay

drivers/usb/udc/udc_ambiq.c

drivers/usb/udc/Kconfig.ambiq

drivers/usb/udc/udc_ambiq.c

aaronyegx

The boards and dts changes look good to me.

zeonchew · 2024-10-18T02:45:59Z

Hi @jfischer-no, I believe your comments above are addressed, please help to have a review again. Thank you 😉

jfischer-no · 2024-10-21T22:03:25Z

drivers/usb/udc/udc_ambiq.c

+/* Max Packet Size for USB endpoints */
+#define EP0_MPS   64U
+#define EP_FS_MPS 64U
+#define EP_HS_MPS 512U


Where is this limitation, in your shim driver, HAL driver, or controller?

boards/ambiq/apollo4p_blue_kxr_evb/apollo4p_blue_kxr_evb.dts

drivers/usb/udc/udc_ambiq.c

Added user button for apollo4p_blue_kxr_evb and apollo4p_evb Signed-off-by: Chew Zeh Yang <[email protected]>

drivers/usb/udc/udc_ambiq.c

Added ambiq-usb bindings needed by udc_ambiq. Signed-off-by: Chew Zeh Yang <[email protected]>

jfischer-no · 2024-10-25T08:57:59Z

drivers/usb/udc/udc_ambiq.c

+	struct gpio_dt_spec usb_vddusb33_gpio;
+	struct gpio_dt_spec usb_vddusb0p9_gpio;


As mentioned above, there is no need to prefix it with usb_.

struct gpio_dt_spec vddusb33_gpio; struct gpio_dt_spec vddusb0p9_gpio;

Hi @jfischer-no, I think I have missed that point earlier. removed usb_ from the 2 variable names

jfischer-no · 2024-10-25T09:00:22Z

drivers/usb/udc/udc_ambiq.c

+		am_hal_usb_intr_usb_enable(priv->usb_handle,
+					   USB_CFG2_SOFE_Msk | USB_CFG2_ResumeE_Msk |
+						   USB_CFG2_SuspendE_Msk | USB_CFG2_ResetE_Msk);


am_hal_usb_intr_usb_enable(priv->usb_handle, USB_CFG2_SOFE_Msk | USB_CFG2_ResumeE_Msk | USB_CFG2_SuspendE_Msk | USB_CFG2_ResetE_Msk);

clang-format keeps add a tab there. I have manually reverted the change by clang-format and push. Let's see whether the CI will let us through 😄

jfischer-no · 2024-10-25T09:04:30Z

drivers/usb/udc/udc_ambiq.c

+	am_hal_usb_interrupt_service(priv->usb_handle, int_status[0], int_status[1], int_status[2]);
+}
+
+static int usb_power_rails_set(const struct device *dev, bool on)


The return value is never checked, maybe static void usb_power_rails_set(const struct device *dev, bool on)?

Hi @jfischer-no, since the error status return might be useful to indicate that the power rail control is not successful, I have made the change to propagate the error to upper layer. Please help to have a look at the change and see whether it is okay.

jfischer-no · 2024-10-25T09:21:46Z

drivers/usb/udc/udc_ambiq.c

+	/* power rails set */
+	ret = gpio_pin_set_dt(&cfg->usb_vddusb33_gpio, on == true ? 1 : 0);
+	if (ret) {
+		return ret;
+	}
+	ret = gpio_pin_set_dt(&cfg->usb_vddusb0p9_gpio, on == true ? 1 : 0);
+	if (ret) {
+		return ret;
+	}


Just on, bool is either 0 or 1 and can be promoted to any integer type.

/* power rails set */ ret = gpio_pin_set_dt(&cfg->usb_vddusb33_gpio, on);

Hi @jfischer-no, understood and made the change. Will use implicit bool to integer casting in the future.

Added implementation for udc_ambiq with its required Kconfig and CMakelist updates. Signed-off-by: Chew Zeh Yang <[email protected]>

Add USB node to apollo4p and apollo4p_blue qualifier, and apollo4p_evb and apollo4p_blue_kxr_evb board to enableUSB support on the MCU and its EVB. Signed-off-by: Chew Zeh Yang <[email protected]>

zephyrbot added area: USB Universal Serial Bus platform: Ambiq Ambiq area: Samples Samples labels Oct 8, 2024

zephyrbot requested review from aaronyegx, AlessandroLuo, jfischer-no, kartben, nashif, RichardSWheatley and tmon-nordic October 8, 2024 10:01

zephyrbot assigned jfischer-no Oct 8, 2024

zephyrbot added manifest manifest-hal_ambiq labels Oct 8, 2024

tmon-nordic reviewed Oct 8, 2024

View reviewed changes

zeonchew force-pushed the apollo4p-usb-udc branch 2 times, most recently from 9290c51 to a13fc8c Compare October 8, 2024 10:41

jfischer-no requested changes Oct 8, 2024

View reviewed changes

zeonchew force-pushed the apollo4p-usb-udc branch from a13fc8c to 15e03ff Compare October 9, 2024 06:27

zeonchew requested review from jfischer-no and tmon-nordic October 9, 2024 06:44

zeonchew force-pushed the apollo4p-usb-udc branch 2 times, most recently from b8f2d43 to 7c3fdab Compare October 9, 2024 10:36

aaronyegx previously approved these changes Oct 15, 2024

View reviewed changes

tmon-nordic previously approved these changes Oct 18, 2024

View reviewed changes

RichardSWheatley previously approved these changes Oct 19, 2024

View reviewed changes

zeonchew dismissed stale reviews from RichardSWheatley, tmon-nordic, and aaronyegx via 6e6a278 October 21, 2024 08:12

zeonchew force-pushed the apollo4p-usb-udc branch from 7c3fdab to 6e6a278 Compare October 21, 2024 08:12

zeonchew requested review from RichardSWheatley, aaronyegx and tmon-nordic October 21, 2024 08:12

AlessandroLuo previously approved these changes Oct 21, 2024

View reviewed changes

jfischer-no requested changes Oct 21, 2024

View reviewed changes

fabiobaltieri mentioned this pull request Oct 22, 2024

boards: rakwireless: Use proper vendor prefix as board folder #80160

Merged

zeonchew dismissed AlessandroLuo’s stale review via ce491e0 October 23, 2024 04:25

zeonchew force-pushed the apollo4p-usb-udc branch from 6e6a278 to ce491e0 Compare October 23, 2024 04:25

zeonchew requested a review from jfischer-no October 23, 2024 04:35

zeonchew force-pushed the apollo4p-usb-udc branch from ce491e0 to 681060f Compare October 23, 2024 06:05

zephyrbot removed manifest manifest-hal_ambiq labels Oct 23, 2024

boards: ambiq: add user button

2c64e6c

Added user button for apollo4p_blue_kxr_evb and apollo4p_evb Signed-off-by: Chew Zeh Yang <[email protected]>

zeonchew force-pushed the apollo4p-usb-udc branch from 681060f to 3ff319e Compare October 23, 2024 09:45

tmon-nordic previously approved these changes Oct 24, 2024

View reviewed changes

jfischer-no reviewed Oct 24, 2024

View reviewed changes

drivers/usb/udc/udc_ambiq.c Outdated Show resolved Hide resolved

dt-bindings: apollo4p: add ambiq usb binding

5b9c7f3

Added ambiq-usb bindings needed by udc_ambiq. Signed-off-by: Chew Zeh Yang <[email protected]>

zeonchew force-pushed the apollo4p-usb-udc branch from 3ff319e to 6ce193f Compare October 25, 2024 03:14

zeonchew dismissed tmon-nordic’s stale review via 7594166 October 25, 2024 05:16

zeonchew force-pushed the apollo4p-usb-udc branch from 6ce193f to 7594166 Compare October 25, 2024 05:16

jfischer-no reviewed Oct 25, 2024

View reviewed changes

zeonchew added 2 commits October 26, 2024 07:35

drivers: udc_ambiq: Implemented UDC driver for apollo4p

3fd3602

Added implementation for udc_ambiq with its required Kconfig and CMakelist updates. Signed-off-by: Chew Zeh Yang <[email protected]>

boards: ambiq: apollo4p: Add USB nodes

f6a7d61

Add USB node to apollo4p and apollo4p_blue qualifier, and apollo4p_evb and apollo4p_blue_kxr_evb board to enableUSB support on the MCU and its EVB. Signed-off-by: Chew Zeh Yang <[email protected]>

zeonchew force-pushed the apollo4p-usb-udc branch from 7594166 to f6a7d61 Compare October 25, 2024 23:37

RichardSWheatley approved these changes Oct 26, 2024

View reviewed changes

jfischer-no approved these changes Oct 28, 2024

View reviewed changes

jfischer-no added this to the v4.1.0 milestone Oct 28, 2024

nashif merged commit 0facdd8 into zephyrproject-rtos:main Nov 16, 2024
26 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for UDC for Apollo4P family #79547

Add support for UDC for Apollo4P family #79547

zeonchew commented Oct 8, 2024

zephyrbot commented Oct 8, 2024 •

edited

Loading

tmon-nordic Oct 8, 2024

zeonchew Oct 9, 2024

jfischer-no Oct 21, 2024

zeonchew Oct 23, 2024

aaronyegx left a comment

zeonchew commented Oct 18, 2024

jfischer-no Oct 21, 2024

jfischer-no Oct 25, 2024

zeonchew Oct 25, 2024

jfischer-no Oct 25, 2024

zeonchew Oct 25, 2024

jfischer-no Oct 25, 2024

zeonchew Oct 25, 2024

jfischer-no Oct 25, 2024

zeonchew Oct 25, 2024

		struct gpio_dt_spec usb_vddusb33_gpio;
		struct gpio_dt_spec usb_vddusb0p9_gpio;

Add support for UDC for Apollo4P family #79547

Add support for UDC for Apollo4P family #79547

Conversation

zeonchew commented Oct 8, 2024

zephyrbot commented Oct 8, 2024 • edited Loading

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

aaronyegx left a comment

Choose a reason for hiding this comment

zeonchew commented Oct 18, 2024

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

zephyrbot commented Oct 8, 2024 •

edited

Loading