stm32: sdp_k1: cdc_acm sample enumeration issue

[ahaslam2]

We are trying to enable the cdc support on a stm32f4 based board: the sdp_k1 from analog devices.
This boards uses an external ULPI phy. We added udc usbotg_hs support to
the device tree: pamolloy@77fc1b4

We are facing a issue were a usb device suspend event is triggered
and the board enumeration fails:

[00:00:00.001,000] cdc_acm_echo: Wait for DTR
[00:00:00.007,000] usb_cdc_acm: Device suspended
[00:00:00.902,000] usb_cdc_acm: Device resumed
[00:00:00.902,000] usb_cdc_acm: from suspend
[00:00:25.526,000] usb_cdc_acm: Device suspended

on the host side we see:

[93947.513549] usb 1-1.8.3.1: new full-speed USB device number 51 using ehci-pci
[93947.593534] usb 1-1.8.3.1: device descriptor read/64, error -32
[93947.781512] usb 1-1.8.3.1: device descriptor read/64, error -32
[93947.969551] usb 1-1.8.3.1: new full-speed USB device number 52 using ehci-pci
[93948.053504] usb 1-1.8.3.1: device descriptor read/64, error -32
[93948.245541] usb 1-1.8.3.1: device descriptor read/64, error -32
[93948.353609] usb 1-1.8.3-port1: attempt power cycle
[93953.657435] usb 1-1.8.3.1: new full-speed USB device number 54 using ehci-pci
[93953.737449] usb 1-1.8.3.1: device descriptor read/64, error -32
[93953.933463] usb 1-1.8.3.1: device descriptor read/64, error -32
[93954.121425] usb 1-1.8.3.1: new low-speed USB device number 55 using ehci-pci
[93954.205416] usb 1-1.8.3.1: device descriptor read/64, error -32
[93954.397445] usb 1-1.8.3.1: device descriptor read/64, error -32
[93954.505485] usb 1-1.8.3-port1: attempt power cycle

The strange thing is that if we enable logging:
CONFIG_LOG_DEFAULT_LEVEL=4

the enumeration completes successfully:

[91233.443672] usb 1-1.8.3.1: new full-speed USB device number 107 using ehci-pci
[91233.553398] usb 1-1.8.3.1: config 1 interface 1 altsetting 0 endpoint 0x82 has invalid maxpacket 512, setting to 64
[91233.553412] usb 1-1.8.3.1: config 1 interface 1 altsetting 0 endpoint 0x1 has invalid maxpacket 512, setting to 64
[91233.554084] usb 1-1.8.3.1: New USB device found, idVendor=2fe3, idProduct=0001, bcdDevice= 3.05
[91233.554091] usb 1-1.8.3.1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[91233.554096] usb 1-1.8.3.1: Product: Zephyr CDC ACM sample
[91233.554099] usb 1-1.8.3.1: Manufacturer: ZEPHYR
[91233.554103] usb 1-1.8.3.1: SerialNumber: 31325118003C0045
[91233.555003] cdc_acm 1-1.8.3.1:1.0: ttyACM2: USB ACM device

However opening the ttyACM device in minicom i dont see the echo working
i tested the same sample on a discovery boards without any issues.

[erwango]

@Desvauxm-st ^^

[ahaslam2]

With this change to not disable the ulpi clock on low power,
Enumeration happens correctly and the demo is working as it should
(even with logs disabled)

diff --git a/drivers/usb/device/usb_dc_stm32.c b/drivers/usb/device/usb_dc_stm32.c
index 326c6e284a..6713b56511 100644
--- a/drivers/usb/device/usb_dc_stm32.c
+++ b/drivers/usb/device/usb_dc_stm32.c
@@ -281,8 +281,10 @@ static int usb_dc_stm32_clock_enable(void)
         * power mode. It is disabled by default in run power mode, no need to
         * disable it.
         */
+#if !USB_OTG_HS_ULPI_PHY
        LL_AHB1_GRP1_DisableClockLowPower(LL_AHB1_GRP1_PERIPH_OTGHSULPI);
 #endif
+#endif
 #endif
 
        return 0;

[ahaslam2]

Here is a branch
https://github.com/ahaslam2/zephyr/commits/sdpk1-usb-cdc/
with the patch:
ahaslam2@8205db9
if this seems like a reasonable fix i can make a PR with it.

[erwango]

Looks reasonable indeed. There maybe a way to factorize the #if !USB_OTG_HS_ULPI_PHY (if this could make this part of code not as bad as it is already)
Also please take care to report in drivers/usb/udc/udc_stm32.c

[ahaslam2]

ok, i agree about the factorizing.

[ahaslam2]

still messy i guess, how about this to replace the block:

#if USB_OTG_HS_EMB_PHY
	LL_AHB1_GRP1_EnableClock(LL_AHB1_GRP1_PERIPH_OTGHSULPI);
	LL_APB2_GRP1_EnableClock(LL_APB2_GRP1_PERIPH_OTGPHYC);
#elif !USB_OTG_HS_ULPI_PHY
#if defined(CONFIG_SOC_SERIES_STM32H7X)
	LL_AHB1_GRP1_DisableClockSleep(LL_AHB1_GRP1_PERIPH_USB1OTGHSULPI);
#else
	LL_AHB1_GRP1_DisableClockLowPower(LL_AHB1_GRP1_PERIPH_OTGHSULPI);
#endif
#endif

[Desvauxm-st]

@ahaslam2
I don't know the board adi_sdp_k1 you use
and i don't know where is connect the Host ?

I have this configuration:

I compile like that:
west build -p auto -b adi_sdp_k1 samples/subsys/usb/cdc_acm
and after flash with ST_link (blue box on the previous image). : flash is ok.

on the Console windows connected to the USB P2 of the board i have that:

on the console window connect to the ST_link ( blue box) I see nothing.

so I don't known how you do to see the host frame ?

[ahaslam2]

The sdp-k1 i have is REV-E, it has a usb-c connector, not a micro connector like i see in your board, but i dont think it will make any difference.

The host logs are form my linux pc when connect the K1 usb. its the output of the dmesg.

the l see in your terminal see from the K1 is exactly the issue i saw, before applying the patch:ahaslam2@8205db9

i think if you apply the above patch you should get a new cdc port that will have the echo working.

[Desvauxm-st]

@ahaslam2
I confirm with the patch we have now that: (OK)

[pamolloy]

This is addressed in #68431