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.

Sign up for GitHub

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
wants to merge 2 commits into from
Closed

USB HS support fixes on STM32U5 #80249

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50a3173
boards: add USB HS 2.0 support for the Nucleo u5a5jz-q board (Issue #…
Oct 17, 2024
e21b99c
dts: include: arm: st: usb: Add USB PHY clock rate support for STM32U…
Oct 17, 2024
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
22 changes: 19 additions & 3 deletions drivers/usb/device/usb_dc_stm32.c
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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