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

Convert STM32 drivers to DT_INST #23285

Merged
merged 11 commits into from
Mar 17, 2020

Conversation

erwango
Copy link
Member

@erwango erwango commented Mar 5, 2020

Superseeds #22807

  • drivers/adc/adc_stm32.c
  • drivers/can/can_stm32.c (cf below comment)
  • drivers/counter/counter_ll_stm32_rtc.c
  • drivers/ethernet/eth_stm32_hal.c (Only impact DT_DTCM_BASE_ADDRESS. To be removed?)
  • drivers/flash/flash_stm32.c
  • drivers/flash/flash_stm32f3x.c (Only impact DT_FLASH_SIZE. To be removed ?)
  • drivers/flash/flash_stm32l4x.c (Only impact DT_FLASH_SIZE. be removed ?)
  • drivers/gpio/gpio_stm32.c (cf below comment)
  • drivers/i2c/i2c_ll_stm32.c (cf below comment)
  • drivers/i2s/i2s_ll_stm32.c (cf below comment)
  • drivers/pwm/pwm_stm32.c
  • drivers/serial/uart_stm32.c
  • drivers/spi/spi_ll_stm32.c
  • drivers/timer/stm32_lptim_timer.c (To be converted to DT first)
  • drivers/usb/device/usb_dc_stm32.c

Conversion of some drivers to DT_INST not possible yet, cf #22807 (comment)

@zephyrbot zephyrbot added area: PWM Pulse Width Modulation area: Watchdog Watchdog area: ADC Analog-to-Digital Converter (ADC) area: Counter area: SPI SPI bus area: Boards area: Samples Samples area: Tests Issues related to a particular existing or missing test labels Mar 5, 2020
@zephyrbot
Copy link
Collaborator

zephyrbot commented Mar 5, 2020

All checks passed.

checkpatch (informational only, not a failure)

-:838: WARNING:LONG_LINE: line over 80 characters
#838: FILE: drivers/watchdog/wdt_wwdg_stm32.c:265:
+	.Instance = (WWDG_TypeDef *)DT_INST_0_ST_STM32_WINDOW_WATCHDOG_BASE_ADDRESS,

-:1402: WARNING:LONG_LINE: line over 80 characters
#1402: FILE: soc/arm/st_stm32/stm32f0/dts_fixup.h:89:
+#define DT_FLASH_DEV_NAME		DT_INST_0_ST_STM32F0_FLASH_CONTROLLER_LABEL

-:1583: WARNING:LONG_LINE: line over 80 characters
#1583: FILE: soc/arm/st_stm32/stm32f1/dts_fixup.h:107:
+#define DT_FLASH_DEV_NAME		DT_INST_0_ST_STM32F1_FLASH_CONTROLLER_LABEL

-:1790: WARNING:LONG_LINE: line over 80 characters
#1790: FILE: soc/arm/st_stm32/stm32f3/dts_fixup.h:91:
+#define DT_FLASH_DEV_NAME		DT_INST_0_ST_STM32F3_FLASH_CONTROLLER_LABEL

-:2393: WARNING:LONG_LINE: line over 80 characters
#2393: FILE: soc/arm/st_stm32/stm32f7/dts_fixup.h:150:
+#define DT_FLASH_DEV_NAME		DT_INST_0_ST_STM32F7_FLASH_CONTROLLER_LABEL

-:2426: WARNING:LONG_LINE: line over 80 characters
#2426: FILE: soc/arm/st_stm32/stm32g0/dts_fixup.h:11:
+#define DT_FLASH_DEV_NAME			DT_INST_0_ST_STM32G0_FLASH_CONTROLLER_LABEL

-:2517: WARNING:LONG_LINE: line over 80 characters
#2517: FILE: soc/arm/st_stm32/stm32g4/dts_fixup.h:74:
+#define DT_FLASH_DEV_NAME			DT_INST_0_ST_STM32G4_FLASH_CONTROLLER_LABEL

-:3075: WARNING:LONG_LINE: line over 80 characters
#3075: FILE: soc/arm/st_stm32/stm32l4/dts_fixup.h:130:
+#define DT_FLASH_DEV_NAME		DT_INST_0_ST_STM32L4_FLASH_CONTROLLER_LABEL

-:3343: WARNING:LONG_LINE: line over 80 characters
#3343: FILE: soc/arm/st_stm32/stm32wb/dts_fixup.h:67:
+#define DT_FLASH_DEV_NAME			DT_INST_0_ST_STM32WB_FLASH_CONTROLLER_LABEL

- total: 0 errors, 9 warnings, 3111 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@erwango erwango force-pushed the dt_inst_conversion branch 4 times, most recently from 056abc2 to 59b9b31 Compare March 9, 2020 09:52
@erwango erwango requested a review from tbursztyka as a code owner March 9, 2020 09:52
@erwango erwango force-pushed the dt_inst_conversion branch from 59b9b31 to bf3cdef Compare March 9, 2020 12:34
@erwango
Copy link
Member Author

erwango commented Mar 10, 2020

Following test is failed:
902/919 qemu_xtensa tests/kernel/sched/schedule_api/kernel.scheduler.multiq FAILED Failed (qemu 6.259s)

@erwango erwango closed this Mar 10, 2020
@erwango erwango reopened this Mar 10, 2020
@erwango
Copy link
Member Author

erwango commented Mar 12, 2020

Failing test:
Failures Sanitycheck / qemu_x86_64:tests/kernel/mem_protect/syscalls/kernel.memory_protection.syscalls / tests/kernel/mem_protect/syscalls/kernel.memory_protection.syscalls

@galak
Copy link
Collaborator

galak commented Mar 12, 2020

@erwango would it make sense to merge this and have new PRs so we don't have CI growing to large? I know we'll need a second pass conversion based on what we are doing in PR #23245.

@erwango
Copy link
Member Author

erwango commented Mar 12, 2020

@erwango would it make sense to merge this and have new PRs

@galak Definitely!

galak and others added 8 commits March 17, 2020 16:03
Convert driver to use DT_INST_ defines.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
Convert driver to use DT_INST_ defines.

Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
Convert driver to use DT_INST_ defines.
Removed DT_RTC_0 fixup macros but keep DT_RTC_NAME_0 as it is
still in use across RTC users.

Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
Convert driver to use DT_INST_ defines.
Removed DT_FLASH_DEV fixup macros, except DT_FLASH_DEV_NAME
used in applications.

Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
Convert driver to use DT_INST_ defines.
Removed DT_FLASH_DEV fixup macros.

Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
On stm32 spi devices, there are 2 main IP variants, with and w/o
fifo. Fifo is not really used today, but still there is some
additional code handling fifo. Today this code is protected under
Kconfig symbol SPI_STM32_HAS_FIFO.
This code carries redundant information vs dedicated compatible
"st,stm32-spi-fifo", which is provided as unique driver compatible
for devices supporting this IP as opposed to use of "st,stm32-spi"
when fifo is not supported.

Having these 2 compatibles defined exclusively is not convenient for
migration to DT_INST as DT_INST macros contain compatible string and
hence it cannot be used to provide common compatible code for devices
defining different compatibles.

Based on these observations, review stm32 spi devices compatible
declarations. Devices supporting fifo will now declare both
compatibles, as proposed by dt spec: "[compatible] property value
consists of a concatenated list of null terminated strings,
from most specific to most general". Hence field will now be:
"st,stm32-spi-fifo", "st,stm32-spi"

This way, fifo enabled stm32 spi devices will generate both:
DT_INST_STM32_SPI_FOO and DT_INST_STM32_SPI_FIFO_FOO
As well as:
DT_COMPAT_ST_STM32_SPI and DT_COMPAT_ST_STM32_SPI_FIFO
So, DT_INST_STM32_SPI_FOO could be used for device initialization.
Also DT_COMPAT_ST_STM32_SPI_FIFO could be used for FIFO handling
code inside driver. Hence use it to replace Kconfig symbol
SPI_STM32_HAS_FIFO.


Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
Convert driver to use DT_INST_ defines.

Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
Convert driver to use DT_INST_ defines.
In the process, we've removed all UART/USART/LPUART code in
device instanciation code which had no impact.
Since all uart/usart/lpuart nodes declare compatibility with
st,stm32uart, DT_INST_X_ST_STM32_UART_FOO could be used.

Removed DT_UART fixup macros.

Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
@erwango erwango force-pushed the dt_inst_conversion branch from b3dc346 to a570e84 Compare March 17, 2020 15:21
{STM32_PIN_PA11, STM32F4_PINMUX_FUNC_PA11_OTG_FS_DM},
{STM32_PIN_PA12, STM32F4_PINMUX_FUNC_PA12_OTG_FS_DP},
#endif /* DT_USB_BASE_ADDRESS */
#ifdef DT_USB_HS_BASE_ADDRESS
#endif /* DT_COMPAT_ST_STM32_OTGFS */
Copy link
Collaborator

Choose a reason for hiding this comment

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

endif comment doesn't match ifdef

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, txs

{STM32_PIN_PB14, STM32F4_PINMUX_FUNC_PB14_OTG_HS_DM},
{STM32_PIN_PB15, STM32F4_PINMUX_FUNC_PB15_OTG_HS_DP},
#endif /* DT_USB_HS_BASE_ADDRESS */
#endif /* CONFIG_USB_DC_STM32 */
#endif /* DT_COMPAT_ST_STM32_OTGHS */
Copy link
Collaborator

Choose a reason for hiding this comment

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

endif comment doesn't match ifdef

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, txs

erwango added 3 commits March 17, 2020 16:35
Convert usb_stm32 driver to use of DT_INST macros.

Since driver is compatible with 3 different dt compatibles and
compatible string is included in DT_INST macros, I've kept the
DT_USB_ compatible agnostic macros based on DT_INST ones, which
allowed to remove fixup definitions.
Use of DT_USB symbols is now limited to usb_dc_stm32.

Additionally, compatible "st,stm32-otgfs" is removed from list
of compatibles for usbotg_hs ips.

Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
stm32f3_disco default configuration was enabling I2C and SPI.
Fix this.

Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
Various kconfig symbols where defined specifically for board
nucleo_l432kc. Though, most are generic to the sample so should moved
directly to sample prj.conf
Some others parts of boards default configuration so could be removed
as well.
Last remove CONFIG_OPENOCD_SUPPORT as sample has no dependency with
this symbol.

Signed-off-by: Erwan Gouriou <erwan.gouriou@linaro.org>
@erwango erwango force-pushed the dt_inst_conversion branch from a570e84 to 842bf3b Compare March 17, 2020 15:35
@galak galak merged commit e13bb7e into zephyrproject-rtos:master Mar 17, 2020
@erwango erwango mentioned this pull request Mar 17, 2020
@@ -271,30 +271,6 @@
#define DT_I2S_6_DMA_FEATURES_RX \
DT_ST_STM32_I2S_40015400_RX_DMAS_FEATURES

#define DT_FLASH_DEV_NAME DT_INST_0_ST_STM32F4_FLASH_CONTROLLER_LABEL
Copy link
Member

Choose a reason for hiding this comment

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

The DT_FLASH_DEV_NAME was accidentally removed by "drivers/usb: usb_dc_stm32: Convert to DT_INST" commit a778bcd. We need a follow up patch. Some applications, e.g. MCUBoot fail to compile.

@@ -119,8 +119,7 @@
#define DT_CAN_1_CLOCK_BUS DT_ST_STM32_CAN_40006400_CLOCK_BUS
#define DT_CAN_1_CLOCK_BITS DT_ST_STM32_CAN_40006400_CLOCK_BITS

#define DT_FLASH_DEV_BASE_ADDRESS DT_ST_STM32F0_FLASH_CONTROLLER_40022000_BASE_ADDRESS
#define DT_FLASH_DEV_NAME DT_ST_STM32F0_FLASH_CONTROLLER_40022000_LABEL
#define DT_FLASH_DEV_NAME DT_INST_0_ST_STM32F0_FLASH_CONTROLLER_LABEL
Copy link
Member

Choose a reason for hiding this comment

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

The flash_stm32.c driver is using DT_INST_0_SOC_NV_FLASH_LABEL as a flash name

DT_INST_0_SOC_NV_FLASH_LABEL="FLASH_STM32"

however the DT_FLASH_DEV_NAME is mapped to the flash controller name

DT_INST_0_ST_STM32F0_FLASH_CONTROLLER_LABEL="FLASH_CTRL"

The two don't match. This is causing run time error in MCUBoot.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mnkp, thanks for reporting. I'm fixing it.

@erwango erwango deleted the dt_inst_conversion branch June 5, 2020 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ADC Analog-to-Digital Converter (ADC) area: Boards area: Counter area: Devicetree area: PWM Pulse Width Modulation area: Samples Samples area: SPI SPI bus area: Tests Issues related to a particular existing or missing test area: Watchdog Watchdog platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants