drivers: use codegen for device instantiation #10888
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,2 @@ | ||
| zephyr_sources_codegen_ifdef(CONFIG_CAN_STM32 stm32_can.c) | ||
| zephyr_sources_ifdef(CONFIG_USERSPACE can_handlers.c) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -841,51 +841,66 @@ static const struct can_driver_api can_api_funcs = { | |
| .detach = can_stm32_detach | ||
| }; | ||
|
|
||
| /** | ||
| * @code{.cogeno.py} | ||
| * cogeno.import_module('devicedeclare') | ||
| * | ||
| * device_configs = ['CONFIG_CAN_1', ] | ||
| * driver_names = ['CAN_1', ] | ||
| * device_inits = 'can_stm32_init' | ||
| * device_levels = 'POST_KERNEL' | ||
| * device_prios = 'CONFIG_KERNEL_INIT_PRIORITY_DEVICE' | ||
| * device_api = 'can_api_funcs' | ||
| * device_info = \ | ||
| * """ | ||
| * DEVICE_DECLARE(${device-name}); | ||
| * static void ${device-config-irq}(CAN_TypeDef *can) | ||
|
There was a problem hiding this comment. This whole piece of code, IRQ configuration functions, should be extracted by API. There was a problem hiding this comment. I know your opinion. This is just to show the principle. You can use your device_declare variant here also. Your variant will fail with STM32F0 UART drivers. A lot of shared interrupts where you definitely need a more sofisticated way to generate interrupt routines. The driver is OOT but I can tell it definitely did not work with the driver_declare interrupt configuration - your one and also not the simple one that I have here. |
||
| * { | ||
| * LOG_DBG("Enable ${driver-name} IRQ"); | ||
| * #ifdef CONFIG_SOC_SERIES_STM32F0X | ||
|
There was a problem hiding this comment. 'compatible' should be used to identify and provide variations of the same driver. There was a problem hiding this comment. Hey this is a proof of concept and not a full blown driver rework, I expect the CAN driver author to do the rework in the end when there is a code generation facility. My focus was the extraction of device tree data not to make that work without some ifdefs. |
||
| * IRQ_CONNECT(${interrupts/0/irq}, ${interrupts/0/priority},\\ | ||
| * can_stm32_isr, | ||
| * DEVICE_GET(${device-name}), 0); | ||
| * irq_enable(${interrupts/0/irq}); | ||
| * #else | ||
| * IRQ_CONNECT(${interrupts/rx0/irq}, ${interrupts/rx0/priority}, \\ | ||
| * can_stm32_rx_isr, | ||
| * DEVICE_GET(${device-name}), 0); | ||
| * irq_enable(${interrupts/rx0/irq}); | ||
| * IRQ_CONNECT(${interrupts/tx/irq}, ${interrupts/tx/priority}, \\ | ||
| * can_stm32_tx_isr, | ||
| * DEVICE_GET(${device-name}), 0); | ||
| * irq_enable(${interrupts/tx/irq}); | ||
| * IRQ_CONNECT(${interrupts/sce/irq}, ${interrupts/sce/priority}, \\ | ||
| * can_stm32_tx_isr, | ||
| * DEVICE_GET(${device-name}), 0); | ||
| * irq_enable(${interrupts/sce/irq}); | ||
| * #endif | ||
| * can->IER |= CAN_IT_TME | CAN_IT_ERR | CAN_IT_FMP0 | CAN_IT_FMP1; | ||
| * } | ||
| * static const struct can_stm32_config ${device-config-info} = { | ||
| * .can = (CAN_TypeDef *)${reg/0/address/0}, | ||
| * .bus_speed = ${bus-speed}, | ||
| * .swj = ${sjw}, | ||
| * .prop_bs1 = ${prop_seg_phase_seg1}, | ||
| * .bs2 = ${phase_seg2}, | ||
| * .pclken = { | ||
| * .enr = ${clocks/0/bits}, | ||
| * .bus = ${clocks/0/bus}, | ||
| * }, | ||
| * .config_irq = ${device-config-irq}, | ||
| * }; | ||
| * static struct can_stm32_data ${device-data}; | ||
| * """ | ||
| * | ||
| * devicedeclare.device_declare_multi( \ | ||
| * device_configs, | ||
| * driver_names, | ||
| * device_inits, | ||
| * device_levels, | ||
| * device_prios, | ||
| * device_api, | ||
| * device_info) | ||
| * @endcode{.cogeno.py} | ||
| */ | ||
| /** @code{.cogeno.ins}@endcode */ | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned several times, this should not be present in API.
This information should be extracted from device tree and drivers <> EDTS binding should be done by 'compatible' property. This is literally its main and only use.
cf #8561
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentionend several times to identify a specific driver instance the compatible is not enough. There are usual several driver instantiations for one compatible. So you need something more to identify a single one and use the 'label' property for that. But if you use the 'label' property in addition to the 'compatible' you find out that the 'label' alone is unique und you really do not need the 'compatible'. Taking the 'compatible' and the 'label' is just a waste of computing.
If you would look into the code of your device_declare variant you could easily identify that in the end only the 'label' is used.