-
Notifications
You must be signed in to change notification settings - Fork 8k
stm32h747 camera support + stm32h7s related DT fixes #96178
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
stm32h747 camera support + stm32h7s related DT fixes #96178
Conversation
42f9345
to
f82c440
Compare
Channel ID should be numbered starting from 1. Correct the dma properties of dcmi devices in L4 series and use dma1 by default to make it same for all L4 series. Signed-off-by: Alain Volmat <[email protected]>
f82c440
to
d2b2414
Compare
Update the dma channel id for the L4 in the similar way as what has been done for the H7 on PR #95803 |
boards/shields/st_stm32f4dis_cam/boards/stm32l4r9i_disco.overlay
Outdated
Show resolved
Hide resolved
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.
Minor question, otherwise LGTM
boards/shields/st_stm32f4dis_cam/boards/stm32h747i_disco_stm32h747xx_m7.overlay
Outdated
Show resolved
Hide resolved
The dma_stm32_slot_to_channel function is only applicable on the STM32Fx series since the LL_DMA_CHANNEL_x macros only exist for those platforms among the one based on st,stm32-dma-v1. While checking for !defined(CONFIG_DMAMUX_STM32) is ok to enable this on STM32Fx, it prevents to disable dmamux on other platforms since required symbols do not exist. Signed-off-by: Alain Volmat <[email protected]>
Function stm32_dma_config_channel_function is unused hence remove it from headers and dma_stm32_v1. In case of dma_stm32_v1, the related configuration is already part of the LL_DMA_Init call done in another place in dma_stm32. Signed-off-by: Alain Volmat <[email protected]>
d2b2414
to
cf74c98
Compare
5ac13a5
to
4dcda09
Compare
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.
Coding style comments only.
#define MCO_SEL_PLLCLK 5 | ||
#define MCO_SEL_LSI 6 | ||
#define MCO_SEL_LSE 7 | ||
#define MCO_SEL_HSI48 8 |
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.
Nitpicking: indentation of values would be nice:
#define MCO_PRE_DIV_1 0
#define MCO_PRE_DIV_2 1
#define MCO_PRE_DIV_4 2
...
#define MCO_SEL_LSE 7
#define MCO_SEL_HSI48 8
Ditto in the next commit.
pinctrl-0 = <&dcmi_d0_pc6 &dcmi_d1_pc7 &dcmi_d2_pg10 &dcmi_d3_pc9 | ||
&dcmi_d4_pc11 &dcmi_d5_pd3 &dcmi_d6_pb8 &dcmi_d7_pb9 | ||
&dcmi_pixclk_pa6 &dcmi_hsync_pa4 &dcmi_vsync_pb7>; | ||
|
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.
Nitpicking: could remove this empty line.
}; | ||
|
||
&mco1 { | ||
status = "okay"; |
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.
Prefer status
last in among the node properties. Not blocking though.
Add macros in order to select the MCO input clock and prescaler value. Signed-off-by: Alain Volmat <[email protected]>
Add macros in order to select the MCO input clock and prescaler value. Signed-off-by: Alain Volmat <[email protected]>
Add connector, i2c and dcmi labels and pinctrl in order to allow usage of a camera module shield with this the stm32h747i_disco board. Signed-off-by: Alain Volmat <[email protected]>
The pin alternate function settings are boards related so configure the dcmi pinctrl within the board dts file in order to remove them from the shield board specific overlay. Signed-off-by: Alain Volmat <[email protected]>
The i2c bus configuration and dcmi pins alternate function settings are already done within the board dts since this is fixed for each board so remove then from the shield overlay. The dmamux node enabling is also removed since currently the DCMI/DMA cannot rely on dmamux. In the shield main overlay file, ensure that the i2c bus is properly enabled. Signed-off-by: Alain Volmat <[email protected]>
Move the selection of the dma channel and the sensor input clock into a board specific overlay since this differ from a board to another. Signed-off-by: Alain Volmat <[email protected]>
Switch to the usage of MCO configuration macros provided by stm32h7_clock.h clock bindings. Signed-off-by: Alain Volmat <[email protected]>
Add an overlay allowing to use the stm32f4dis_cam shield on the stm32h747i_disco board. Signed-off-by: Alain Volmat <[email protected]>
4dcda09
to
4b28ba8
Compare
Thanks for the review @etienne-lms. Hopefully corrected all. |
|
} | ||
|
||
#if !defined(CONFIG_DMAMUX_STM32) | ||
#if !defined(CONFIG_SOC_SERIES_STM32H7X) && !defined(CONFIG_SOC_SERIES_STM32MP1X) |
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.
Non blocking: Then, what about applying the same to dma_stm32_slot_to_channel()
declaration in header:
zephyr/drivers/dma/dma_stm32.h
Lines 53 to 55 in 5fd0412
#if !defined(CONFIG_DMAMUX_STM32) | |
uint32_t dma_stm32_slot_to_channel(uint32_t id); | |
#endif |
[Out of scope of this PR]
This also raises the question on another user of this function: stm32_dma_config_channel_function
, also defined under #if !defined(CONFIG_DMAMUX_STM32)
, but this one is never called, so maybe it could be purely removed ?
@FRASTM can you have a look ?
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.
Addressed in next commit! Thanks :)
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.
yeah I split the fix commit and the removal of dead code commit.
PR containing support of the STM32H747 camera (dcmi) support as well as some other fixes done.
In order to have this working, PR #95803 have to be merged as well.
The ST-B-CAMS-OMV shield is not yet functionnal with the STM32H747I-DISCO hence for the time being I am pushing this PR with the addition of the ST_STM32F4DIS_CAM shield support.
The OV5640 currently has a pixelclk configured too high, leading to the DCMI not able to capture everything and getting an partially broken image.
In addition to the STM32H747 bits, I also update few points regarding the STM32H7S and the ST-B-CAMS-OMV shield which I found while trying to enable the ST-B-CAMS-OMV with the platform.
I added the DCMI pinctrl within the stm32h747i-disco.dtsi file instead of the //m7 specific file. Indeed, I think this is where it belong since this is board related and not related to the point of using the M4 or the M7. Following this logic, I think that there are existing nodes containing pinctrl within the //m7 dts file which should also be moved into the dtsi file. This is a point to be discussed, maybe for another PR (or this one if this is preferred).