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

USB HS support fixes on STM32U5 #80249

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions boards/st/nucleo_u5a5zj_q/doc/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,8 @@ The Zephyr nucleo_u5a5zj_q board configuration supports the following hardware f
+-----------+------------+-------------------------------------+
| RTC | on-chip | rtc |
+-----------+------------+-------------------------------------+
| USB | on-chip | USB 2.0 HS |
+-----------+------------+-------------------------------------+


Other hardware features are not yet supported on this Zephyr port.
Expand Down Expand Up @@ -245,27 +247,34 @@ Default Zephyr Peripheral Mapping:
- UART_2_TX : PD5
- UART_2_RX : PD6
- USER_PB : PC13
- USB_DM : PA11
- USB_DP : PA12

System Clock
------------

Nucleo U5A5ZJ Q System Clock could be driven by internal or external oscillator,
as well as main PLL clock. By default System clock is driven by PLL clock at
160MHz, driven by 4MHz medium speed internal oscillator.
160MHz, driven by the 16MHz high speed oscillator.

Serial Port
-----------

Nucleo U5A5ZJ Q board has 6 U(S)ARTs. The Zephyr console output is assigned to
USART1. Default settings are 115200 8N1.


Backup SRAM
-----------

In order to test backup SRAM you may want to disconnect VBAT from VDD. You can
do it by removing ``SB50`` jumper on the back side of the board.

Using USB
---------

USB 2.0 high speed (HS) operation requires the HSE clock source to be populated
and enabled. The Nucleo U5A5ZJ-Q includes the 16MHz oscillator and required
jumper settings.

Programming and Debugging
*************************
Expand Down
19 changes: 10 additions & 9 deletions boards/st/nucleo_u5a5zj_q/nucleo_u5a5zj_q-common.dtsi
Original file line number Diff line number Diff line change
Expand Up @@ -55,22 +55,23 @@
status = "okay";
};

&clk_lse {
/* This board has a 16MHz crystal attached */
&clk_hse {
clock-frequency = <DT_FREQ_M(16)>;
status = "okay";
};

&clk_msis {
&clk_lse {
status = "okay";
msi-range = <4>;
msi-pll-mode;
};

&pll1 {
div-m = <1>;
mul-n = <80>;
div-q = <2>;
div-r = <2>;
clocks = <&clk_msis>;
/* HSE 16MHz source, outputting 160MHz to sysclk and apbclk */
div-m = <4>; /* input divisor */
mul-n = <80>; /* VCO multiplication factor */
mniestroj marked this conversation as resolved.
Show resolved Hide resolved
div-q = <2>; /* system clock divisor */
div-r = <2>; /* peripheral clock divisor */
clocks = <&clk_hse>;
erikarn marked this conversation as resolved.
Show resolved Hide resolved
status = "okay";
};

Expand Down
6 changes: 6 additions & 0 deletions boards/st/nucleo_u5a5zj_q/nucleo_u5a5zj_q.dts
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,9 @@
&gpdma1 {
status = "okay";
};

zephyr_udc0: &usbotg_hs {
pinctrl-0 = <&usb_otg_hs_dm_pa11 &usb_otg_hs_dp_pa12>;
pinctrl-names = "default";
status = "okay";
};
1 change: 1 addition & 0 deletions boards/st/nucleo_u5a5zj_q/nucleo_u5a5zj_q.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,6 @@ supported:
- backup_sram
- dma
- rtc
- usb_device
ram: 2450
flash: 4096
22 changes: 19 additions & 3 deletions drivers/usb/device/usb_dc_stm32.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <zephyr/usb/usb_device.h>
#include <zephyr/drivers/clock_control/stm32_clock_control.h>
#include <zephyr/sys/util.h>
#include <zephyr/dt-bindings/phy/stm32u5_otg_hs_phy.h>
#include <zephyr/drivers/gpio.h>
#include <zephyr/drivers/pinctrl.h>
#include "stm32_hsem.h"
Expand All @@ -36,6 +37,21 @@ LOG_MODULE_REGISTER(usb_dc_stm32);
#error "Only one interface should be enabled at a time, OTG FS or OTG HS"
#endif

/*
* Some STM32U5xx parts support a USB HS PHY which is clocked by the HSE clock or PLL1_P
* clock directly. This requires specific configuration.
*/
#if DT_HAS_COMPAT_STATUS_OKAY(st_stm32u5_otghs_phy)
#if DT_NODE_HAS_PROP(DT_PHANDLE_BY_IDX(DT_NODELABEL(usbotg_hs), phys, 0), clock_cfg)
#define OTG_HS_PHY_REFERENCE_CLOCK \
_CONCAT(SYSCFG_OTG_HS_PHY_CLK_SELECT_, \
DT_PROP(DT_PHANDLE(DT_NODELABEL(usbotg_hs), phys), clock_cfg))
#else
#define OTG_HS_PHY_REFERENCE_CLOCK SYSCFG_OTG_HS_PHY_CLK_SELECT_1
#endif
Comment on lines +45 to +51
Copy link
Member

Choose a reason for hiding this comment

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

There are various ways to provide a default value:

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or the property could be marked required: true with no default value provided to force user to input the correct value. Maybe this is the best course of action here.

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, this is even better as this depends on each board implementation.


#endif /* DT_HAS_COMPAT_STATUS_OKAY(st_stm32u5_otghs_phy) */

/*
* Vbus sensing is determined based on the presence of the hardware detection
* pin(s) in the device tree. E.g: pinctrl-0 = <&usb_otg_fs_vbus_pa9 ...>;
Expand Down Expand Up @@ -217,7 +233,7 @@ static int usb_dc_stm32_clock_enable(void)
return -ENODEV;
}

#if DT_HAS_COMPAT_STATUS_OKAY(st_stm32_otghs) && defined(CONFIG_SOC_SERIES_STM32U5X)
#if DT_HAS_COMPAT_STATUS_OKAY(st_stm32u5_otghs_phy)
/* Sequence to enable the power of the OTG HS on a stm32U5 serie : Enable VDDUSB */
bool pwr_clk = LL_AHB3_GRP1_IsEnabledClock(LL_AHB3_GRP1_PERIPH_PWR);

Expand Down Expand Up @@ -246,7 +262,7 @@ static int usb_dc_stm32_clock_enable(void)

/* Set the OTG PHY reference clock selection (through SYSCFG) block */
LL_APB3_GRP1_EnableClock(LL_APB3_GRP1_PERIPH_SYSCFG);
HAL_SYSCFG_SetOTGPHYReferenceClockSelection(SYSCFG_OTG_HS_PHY_CLK_SELECT_1);
HAL_SYSCFG_SetOTGPHYReferenceClockSelection(OTG_HS_PHY_REFERENCE_CLOCK);
/* Configuring the SYSCFG registers OTG_HS PHY : OTG_HS PHY enable*/
HAL_SYSCFG_EnableOTGPHY(SYSCFG_OTG_HS_PHY_ENABLE);
#elif defined(PWR_USBSCR_USB33SV) || defined(PWR_SVMCR_USV)
Expand Down Expand Up @@ -333,7 +349,7 @@ static int usb_dc_stm32_clock_disable(void)
LOG_ERR("Unable to disable USB clock");
return -EIO;
}
#if DT_HAS_COMPAT_STATUS_OKAY(st_stm32_otghs) && defined(CONFIG_SOC_SERIES_STM32U5X)
#if DT_HAS_COMPAT_STATUS_OKAY(st_stm32u5_otghs_phy)
LL_AHB2_GRP1_DisableClock(LL_AHB2_GRP1_PERIPH_USBPHY);
#endif

Expand Down
1 change: 1 addition & 0 deletions dts/arm/st/u5/stm32u5.dtsi
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <zephyr/dt-bindings/memory-controller/stm32-fmc-nor-psram.h>
#include <zephyr/dt-bindings/adc/stm32u5_adc.h>
#include <zephyr/dt-bindings/power/stm32_pwr.h>
#include <zephyr/dt-bindings/phy/stm32u5_otg_hs_phy.h>
#include <freq.h>

/ {
Expand Down
6 changes: 3 additions & 3 deletions dts/arm/st/u5/stm32u595.dtsi
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,15 @@
ram-size = <4096>;
maximum-speed = "high-speed";
clocks = <&rcc STM32_CLOCK(AHB2, 15U)>,
<&rcc STM32_SRC_HSI48 ICKLK_SEL(0)>;
<&rcc STM32_SRC_HSI48 OTGHS_SEL(0)>;
Copy link
Member

Choose a reason for hiding this comment

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

Though STM32_SRC_HSI48 is not used by the driver (only second cell is used to configure clock, i.e. OTGHS_SEL(0)), it is confusing to leave it like that since OTGHS_SEL(0) select HSE and not HSI48 as input clock.

Switching that to STM32_SRC_HSE seems the way to go. However, we need to disable CONFIG_USB_DC_STM32_CLOCK_CHECK so it won't complain about 48MHz frequency. Or we can update that check (it won't be easy to cover all cases, so I guess we can leave that as future enhancement).

phys = <&otghs_phy>;
status = "disabled";
};
};

otghs_phy: otghs_phy {
/* Clock source defined by USBPHYC_SEL in */
compatible = "usb-nop-xceiv";
/* Clock source defined by OTGHS_SEL() in usbotg_hs `clocks` */
compatible = "st,stm32u5-otghs-phy";
#phy-cells = <0>;
};

Expand Down
26 changes: 26 additions & 0 deletions dts/bindings/phy/st,stm32u5-otghs-phy.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Copyright (c) 2024 Meta
# SPDX-License-Identifier: Apache-2.0

description: |
This binding is to be used by the STM32U5xx transceivers which are built-in
with USB HS PHY IP and a configurable HSE clock source.

compatible: "st,stm32u5-otghs-phy"

include: phy-controller.yaml

properties:
"#phy-cells":
const: 0

clock-cfg:
type: int
enum:
- 1 # OTGHS_PHY_CLK_16MHZ
- 2 # OTGHS_PHY_CLK_19P2MHZ
- 3 # OTGHS_PHY_CLK_20MHZ
- 4 # OTGHS_PHY_CLK_24MHZ
- 5 # OTGHS_PHY_CLK_26MHZ
- 6 # OTGHS_PHY_CLK_32MHZ
Comment on lines +19 to +24
Copy link
Member

Choose a reason for hiding this comment

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

Please document values in the description to allow rendering in documentation.
See https://docs.zephyrproject.org/latest/build/dts/api/bindings/timer/st%2Cstm32-timers.html#dtbinding-st-stm32-timers as example

Comment on lines +19 to +24
Copy link
Member

Choose a reason for hiding this comment

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

I think it makes more sense to define list of possible frequencies:

      - 16000000
      - 19200000
      - 20000000
      - 24000000
      - 26000000
      - 32000000

here. That way it will still be pretty straightforward to select specific configuration in devicetree, e.g. with DT_FREQ_M(16), while being able to drop include/zephyr/dt-bindings/phy/stm32u5_otg_hs_phy.h entirely.

description: |
The clock source speed configuration for this PHY.
2 changes: 1 addition & 1 deletion include/zephyr/dt-bindings/clock/stm32u5_clock.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@
#define HSPI_SEL(val) STM32_DOMAIN_CLOCK(val, 3, 22, CCIPR2_REG)
#define I2C5_SEL(val) STM32_DOMAIN_CLOCK(val, 3, 24, CCIPR2_REG)
#define I2C6_SEL(val) STM32_DOMAIN_CLOCK(val, 3, 26, CCIPR2_REG)
#define USBPHYC_SEL(val) STM32_DOMAIN_CLOCK(val, 3, 30, CCIPR2_REG)
#define OTGHS_SEL(val) STM32_DOMAIN_CLOCK(val, 3, 30, CCIPR2_REG)
/** CCIPR3 devices */
#define LPUART1_SEL(val) STM32_DOMAIN_CLOCK(val, 7, 0, CCIPR3_REG)
#define SPI3_SEL(val) STM32_DOMAIN_CLOCK(val, 3, 3, CCIPR3_REG)
Expand Down
16 changes: 16 additions & 0 deletions include/zephyr/dt-bindings/phy/stm32u5_otg_hs_phy.h
erikarn marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* Copyright (c) 2024 Meta
*
* SPDX-License-Identifier: Apache-2.0
*/
#ifndef ZEPHYR_INCLUDE_DT_BINDINGS_PHY_STM32U5_OTG_HS_PHY_H_
#define ZEPHYR_INCLUDE_DT_BINDINGS_PHY_STM32U5_OTG_HS_PHY_H_

#define OTGHS_PHY_CLK_16MHZ 1
#define OTGHS_PHY_CLK_19P2MHZ 2
#define OTGHS_PHY_CLK_20MHZ 3
#define OTGHS_PHY_CLK_24MHZ 4
#define OTGHS_PHY_CLK_26MHZ 5
#define OTGHS_PHY_CLK_32MHZ 6

#endif /* ZEPHYR_INCLUDE_DT_BINDINGS_PHY_STM32U5_OTG_HS_PHY_H_ */
Loading