- 
                Notifications
    You must be signed in to change notification settings 
- Fork 8.2k
drivers: i2s: stm32 sai add support for stm32f4xx series #97829
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
Conversation
aa80eee    to
    5ba3553      
    Compare
  
    5ba3553    to
    900fbe6      
    Compare
  
    These changes enable SAI1 A & B nodes in STM32F4xx series. Signed-off-by: Mario Paja <[email protected]>
STM32F4xx series shares several DMA configurations with the other platforms. These changes aim to enable platform specific DMA configuration and align them to other platforms. Signed-off-by: Mario Paja <[email protected]>
Add nucleo_f429zi board in samples/drivers/i2s/output Signed-off-by: Mario Paja <[email protected]>
900fbe6    to
    cc72746      
    Compare
  
    | Please retry analysis of this Pull-Request directly on SonarQube Cloud | 
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.
I have several suggestions to replace the CONFIG_SOC_SERIES_XXX macros. I think it would ease the porting of other series featuring the SAI, and would also lighten the code a bit.
| #if !defined(CONFIG_SOC_SERIES_STM32H7X) && !defined(CONFIG_SOC_SERIES_STM32L4X) && \ | ||
| !defined(CONFIG_SOC_SERIES_STM32G4X) && !defined(CONFIG_SOC_SERIES_STM32L5X) | ||
| !defined(CONFIG_SOC_SERIES_STM32G4X) && !defined(CONFIG_SOC_SERIES_STM32L5X) && \ | ||
| !defined(CONFIG_SOC_SERIES_STM32F4X) | 
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.
Could we replace all these defined with a single #if DT_HAS_COMPAT_STATUS_OKAY(st_stm32u5_dma) ?
That would probably make adding other series easier.
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.
I will do it on a separate PR after F7 is merged (and then I am node with almost all series), It is already in todo :)
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.
Very well. Approved!
| #if !defined(CONFIG_SOC_SERIES_STM32H7X) && !defined(CONFIG_SOC_SERIES_STM32L4X) && \ | ||
| !defined(CONFIG_SOC_SERIES_STM32G4X) && !defined(CONFIG_SOC_SERIES_STM32L5X) | ||
| !defined(CONFIG_SOC_SERIES_STM32G4X) && !defined(CONFIG_SOC_SERIES_STM32L5X) && \ | ||
| !defined(CONFIG_SOC_SERIES_STM32F4X) | 
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.
Ditto
| #elif !defined(CONFIG_SOC_SERIES_STM32H7X) && !defined(CONFIG_SOC_SERIES_STM32L4X) && \ | ||
| !defined(CONFIG_SOC_SERIES_STM32G4X) && !defined(CONFIG_SOC_SERIES_STM32L5X) | ||
| !defined(CONFIG_SOC_SERIES_STM32G4X) && !defined(CONFIG_SOC_SERIES_STM32L5X) && \ | ||
| !defined(CONFIG_SOC_SERIES_STM32F4X) | 
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.
Ditto
| #if defined(CONFIG_SOC_SERIES_STM32H7X) || defined(CONFIG_SOC_SERIES_STM32L4X) || \ | ||
| defined(CONFIG_SOC_SERIES_STM32G4X) || defined(CONFIG_SOC_SERIES_STM32L5X) | ||
| defined(CONFIG_SOC_SERIES_STM32G4X) || defined(CONFIG_SOC_SERIES_STM32L5X) || \ | ||
| defined(CONFIG_SOC_SERIES_STM32F4X) | 
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.
And here we could maybe use #if DT_HAS_COMPAT_STATUS_OKAY(st_stm32_dma_v1) || DT_HAS_COMPAT_STATUS_OKAY(st_stm32_dma_v2) ?
| /* Control of MCLK output from SAI configuration is not possible on STM32L4xx MCUs */ | ||
| #if !defined(CONFIG_SOC_SERIES_STM32L4X) | ||
| #if !defined(CONFIG_SOC_SERIES_STM32L4X) && !defined(CONFIG_SOC_SERIES_STM32F4X) | 
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.
Comment is outdated (missing F4). This could be replaced with #ifdef SAI_MCK_OUTPUT_ENABLE to avoid having to add other series in future.
| } | ||
|  | ||
| hdma->Instance = STM32_DMA_GET_INSTANCE(stream->reg, stream->dma_channel); | ||
| #if defined(CONFIG_SOC_SERIES_STM32F4X) | 
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.
Here it could probably be #ifdef DMA_CHANNEL_1.
| #endif | ||
|  | ||
| #if defined(CONFIG_SOC_SERIES_STM32H7X) | ||
| #if defined(CONFIG_SOC_SERIES_STM32H7X) || defined(CONFIG_SOC_SERIES_STM32F4X) | 
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.
And here #ifdef DMA_FIFOMODE_DISABLE
| /* MckOverSampling is not supported by all STM32L4xx MCUs */ | ||
| #if !defined(CONFIG_SOC_SERIES_STM32L4X) | ||
| #if !defined(CONFIG_SOC_SERIES_STM32L4X) && !defined(CONFIG_SOC_SERIES_STM32F4X) | 
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.
Same here. Could be replaced with #ifdef SAI_MCK_OVERSAMPLING_DISABLE
This PR enables SAI on STM32F4xx series by:
Audio Test:
2CH, 16bit, 44.1KHz, PCM5102a
stm32f4xx_sai.mp4