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 WWDT STM32 drivers to DT_INST #22807

Closed
wants to merge 1 commit into from

Conversation

galak
Copy link
Collaborator

@galak galak commented Feb 13, 2020

No description provided.

@galak
Copy link
Collaborator Author

galak commented Feb 13, 2020

drivers/adc/adc_stm32.c
drivers/can/can_stm32.c
drivers/counter/counter_ll_stm32_rtc.c
drivers/ethernet/eth_stm32_hal.c
drivers/flash/flash_stm32.c
drivers/flash/flash_stm32f3x.c
drivers/flash/flash_stm32l4x.c
drivers/gpio/gpio_stm32.c
drivers/i2c/i2c_ll_stm32.c
drivers/i2s/i2s_ll_stm32.c
drivers/pwm/pwm_stm32.c
drivers/serial/uart_stm32.c
drivers/spi/spi_ll_stm32.c
drivers/timer/stm32_lptim_timer.c
drivers/usb/device/usb_dc_stm32.c

@zephyrbot zephyrbot added area: Watchdog Watchdog area: Samples Samples area: Tests Issues related to a particular existing or missing test labels Feb 13, 2020
@zephyrbot
Copy link
Collaborator

All checks passed.

checkpatch (informational only, not a failure)

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

- total: 0 errors, 1 warnings, 218 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.

@galak galak changed the base branch from master to topic-devicetree February 18, 2020 21:36
@erwango
Copy link
Member

erwango commented Feb 28, 2020

I'm currently looking into this, will push some additional commits, though I've noted following drivers will not be able to easily use DT_INST_:

  • can_stm32: CAN_1 / CAN_2 specific code in instances definitions. Some inconsistencies could be fixed in generic can driver itself, some other might require dt-binding rework. DT_NODELABEL would also help by passing issues.
  • flash_stm32: Each STM32 soc series is currently using a different compatible. DT_INST can not be used directly since compatible information is hold in DT_INST string macro. I've been able to do the conversion thanks to the DT generated macros DT_INST_0_SOC_NV_FLASH_, which remove the compatible and replace it by SOC_NV_FLASH. This seems to be a flash specific hack. Not sure it will be kept. Other solutions: Get all soc using the same compatible, either by replacing current one, either by adding a common generic compatible.
  • gpio_stm32: Driver is currently using a .port field holding a constant (STM32_PORT[A-K]) . This information is used as parameter in some functions and passed to STM32Cube LL API. Using DT_INST_ doesn't allow conversion to alphabetical index nor instance specific usage. DT_NODELABEL would be required in this case
  • i2c_stm32: There are 2 compatibles in use for this driver, defining 2 different IPs. The variation in the different IP are not visible in the device instantiation code, but has impact in the driver functions. Problem is that the DT_INST holds the compatible information string and this prevents the use or pre-processor macros (DT_INST_##index##COMPAT) to generate various instances.
    Second issue is that in one of the I2C IP (V2), there is a sub variation, some soc using 2 IRQs (event, error), some using only one (combined IRQ). This has of course an impact in the instantiation code. So far this is identified by a config symbol (I2C_STM32_COMBINED_INTERRUPT). Eventually we should get the info directly from dts, either with a new compatible either by checking presence of a _IRQ_COMBINED generated macro. For pre-processor reasons (#ifdef FOO_##index is not allowed), the only way would be to introduce a new compatible.
    Last, init function holds instance specific code (https://github.com/zephyrproject-rtos/zephyr/blob/3ac4c9659c2821650ede72eb1e74955369b057f2/drivers/i2c/i2c_ll_stm32.c#L211) and since DT_INST would not allow to code this properly (starts from 0 and index relates to the enabled instance, I2C_4 can get index 0).
  • pwm_stm32: Here the dts bindings is shaky (implicit relationship between pwm node and parent timer node that would need to be rework). Though, no impact on conversion to DT_INST
  • i2s_stm32: Device instantiation data define a instance specific value not defined through device tree (is2_clk_sel). This prevents conversion to DT_INST. Solution to this should be extension of device tree description to clock selection, which is not available yet, or ad-hoc new property for the time being.
  • uart_stm32: Instantiation code made reference to UART/USART or LPUART. Though all LPUART/USART nodes used st,stm32_uart node as well to DT_INST_X_ST_STM32_UART could be used to overcome simplify code.

Sum up: DT_INST doesn't fit well when:

  • We need instance specific code
  • drivers are used by nodes defining different compatibles. In this case, solution may be to update the node compatible to provide an additional common compatible. Though, we lack rules in this situation. DT_NODELABEL macros, which don't convey compatible string, allow to by pass this issue easily.

Current work is available here: https://github.com/erwango/zephyr/tree/dt_inst_conversion

Convert driver to use DT_INST_ defines.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
@galak galak changed the base branch from topic-devicetree to master March 11, 2020 14:10
@galak galak changed the title Convert STM32 drivers to DT_INST Convert WWDT STM32 drivers to DT_INST Mar 11, 2020
@galak galak added the platform: STM32 ST Micro STM32 label Mar 11, 2020
@galak
Copy link
Collaborator Author

galak commented Mar 11, 2020

Closing this in favor of #23285

@galak galak closed this Mar 11, 2020
@galak galak deleted the dt-inst-stm32 branch March 11, 2020 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Samples Samples 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.

3 participants