Skip to content

Conversation

@mariopaja
Copy link
Contributor

@mariopaja mariopaja commented Jun 11, 2025

This property enables the user to configure the Master Clock Divider
The property sets nodiv or osr value

  • If NODIV = 1 :
    MCKDIV[5:0] = SAI_CK_x / (FS * (FRL + 1))
  • Otherwise
    MCKDIV[5:0] = SAI_CK_x / (FS * (OSR + 1) * 256)

Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

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

Minor comments

@mariopaja mariopaja force-pushed the i2s_stm32_sai_frame_sync branch from 9c89e5f to 4afd8a5 Compare June 12, 2025 07:01
@mariopaja mariopaja changed the title drivers: i2s: stm32 sai add mclk_oversampling_enable property drivers: i2s: stm32 sai add mclk-oversampling-512 property Jun 12, 2025
etienne-lms
etienne-lms previously approved these changes Jun 12, 2025
@mariopaja mariopaja force-pushed the i2s_stm32_sai_frame_sync branch 2 times, most recently from cf21f4c to ec1b131 Compare June 12, 2025 12:08
@mariopaja mariopaja changed the title drivers: i2s: stm32 sai add mclk-oversampling-512 property drivers: i2s: stm32 sai add mclk-divider property Jun 12, 2025
@mariopaja mariopaja force-pushed the i2s_stm32_sai_frame_sync branch from ec1b131 to 41d0ef2 Compare June 12, 2025 12:18
@anangl anangl assigned erwango and unassigned anangl Jun 12, 2025
@mariopaja mariopaja force-pushed the i2s_stm32_sai_frame_sync branch from 41d0ef2 to 38af4aa Compare June 12, 2025 13:22
@mariopaja
Copy link
Contributor Author

Hi @erwango @etienne-lms,

FYI, I have created a draft to add sai support to stm32h5 series #91555.
I will wait for this PR to merge before opening the other one

@mariopaja mariopaja force-pushed the i2s_stm32_sai_frame_sync branch from 38af4aa to ccaa92e Compare June 13, 2025 09:29
This property enables the user to configure the Master Clock Divider.

Signed-off-by: Mario Paja <mariopaja@hotmail.com>
@mariopaja mariopaja force-pushed the i2s_stm32_sai_frame_sync branch from ccaa92e to 37265ce Compare June 13, 2025 09:55
@sonarqubecloud
Copy link

Copy link
Contributor

@mathieuchopstm mathieuchopstm left a comment

Choose a reason for hiding this comment

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

I think this change requires a note to be added in the migration guide? @erwango

.pcfg = PINCTRL_DT_INST_DEV_CONFIG_GET(index), \
.mclk_enable = DT_INST_PROP(index, mclk_enable), \
.mclk_div_enable = DT_INST_PROP(index, mclk_div_enable), \
.mclk_div = (enum mclk_divider)DT_ENUM_IDX(DT_DRV_INST(index), mclk_divider), \
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: instead of the unstable DT_ENUM_IDX, something more stable like this could be used:

Suggested change
.mclk_div = (enum mclk_divider)DT_ENUM_IDX(DT_DRV_INST(index), mclk_divider), \
.mclk_div = CONCAT(MCLK_, DT_INST_STRING_UPPER_TOKEN(index, mclk_divider)), \

hsai->Init.NoDivider = SAI_MASTERDIVIDER_DISABLED;
} else {
hsai->Init.NoDivider = SAI_MASTERDIVIDER_ENABLE;
if (cfg->mclk_div == (enum mclk_divider)MCLK_DIV_256) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: cast should not be needed?

Suggested change
if (cfg->mclk_div == (enum mclk_divider)MCLK_DIV_256) {
if (cfg->mclk_div == MCLK_DIV_256) {

if (cfg->mclk_div_enable) {
hsai->Init.NoDivider = SAI_MASTERDIVIDER_ENABLE;
} else {
if (cfg->mclk_div == (enum mclk_divider)MCLK_NO_DIV) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: cast should not be needed?

Suggested change
if (cfg->mclk_div == (enum mclk_divider)MCLK_NO_DIV) {
if (cfg->mclk_div == MCLK_NO_DIV) {

@erwango
Copy link
Member

erwango commented Jun 20, 2025

I think this change requires a note to be added in the migration guide? @erwango

Not required. SAI was introduced in current release.

Copy link
Contributor

@mathieuchopstm mathieuchopstm left a comment

Choose a reason for hiding this comment

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

+1 since note in migration guide is not required. My previous nits still apply but are non-blocking.

@mariopaja
Copy link
Contributor Author

@mathieuchopstm I can make the changes only starting from next week.

If it doesn't get merged I can implement your suggestions immediately then.

@nashif nashif merged commit 0637ec4 into zephyrproject-rtos:main Jun 20, 2025
26 checks passed
@mariopaja mariopaja mentioned this pull request Jun 13, 2025
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants