Skip to content

I2S: Revert changes done in STM32cube and fix the i2s driver instead #12637

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

Merged
merged 2 commits into from
Jan 24, 2019

Conversation

avisconti
Copy link
Collaborator

@avisconti avisconti commented Jan 22, 2019

Fix PR #12609.

Tested on ArgonKey board using microphone sample.

@avisconti avisconti self-assigned this Jan 22, 2019
@avisconti avisconti requested a review from erwango January 22, 2019 13:06
@avisconti avisconti requested a review from dbkinder as a code owner January 22, 2019 13:06
Revert 71ba2de:
ext: stm32cube: stm32f4xx: shift I2SR field in PLLI2SCFGR register
(ST Bug tracker ID: 50086)

Revert 9a89320:
ext: stm32cube: stm32f7xx: shift I2SR field in PLLI2SCFGR register
(ST Bug tracker ID: 50108)

This two commits were introduced to shift the PLLR parameter according
to the PLL register field position. After analysis, it appears that
function is actually correct, and issue was actually in parameters
provided that didn't match the API. (see PR zephyrproject-rtos#12609)

Signed-off-by: Armando Visconti <[email protected]>
@codecov-io
Copy link

codecov-io commented Jan 22, 2019

Codecov Report

Merging #12637 into master will increase coverage by 5.62%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12637      +/-   ##
==========================================
+ Coverage   48.31%   53.94%   +5.62%     
==========================================
  Files         280      242      -38     
  Lines       43348    27654   -15694     
  Branches    10380     6717    -3663     
==========================================
- Hits        20945    14917    -6028     
+ Misses      18251     9932    -8319     
+ Partials     4152     2805    -1347
Impacted Files Coverage Δ
include/bluetooth/buf.h 0% <0%> (-100%) ⬇️
include/drivers/bluetooth/hci_driver.h 0% <0%> (-100%) ⬇️
include/bluetooth/hci.h 0% <0%> (-66.67%) ⬇️
include/misc/byteorder.h 56.81% <0%> (-40.91%) ⬇️
subsys/bluetooth/host/hci_core.c 2.64% <0%> (-35.77%) ⬇️
subsys/bluetooth/host/uuid.c 0% <0%> (-34.15%) ⬇️
subsys/net/ip/ipv6.h 66.66% <0%> (-33.34%) ⬇️
include/bluetooth/bluetooth.h 0% <0%> (-31.58%) ⬇️
subsys/bluetooth/common/log.c 0% <0%> (-21.43%) ⬇️
subsys/shell/shell_cmds.c 44.53% <0%> (-19.6%) ⬇️
... and 173 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b8d2e19...073302b. Read the comment docs.

@@ -109,6 +109,34 @@ static int i2s_stm32_enable_clock(struct device *dev)
#ifdef CONFIG_I2S_STM32_USE_PLLI2S_ENABLE
#define PLLI2S_MAX_MS_TIME 1 /* PLLI2S lock time is 300us max */
static u16_t plli2s_ms_count;

static u32_t shift_pllr(u8_t pllr)
Copy link
Member

Choose a reason for hiding this comment

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

Since CONFIG_I2S_STM32_PLLI2S_PLLR is defined at compile time, it would be legit use a pre-processor macro as done in clock controller and save some code:

#define _pllr(v) LL_RCC_PLLI2SR_DIV ## v
#define pllr(v) _pllr(v)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, it seems really smart!

The PLLR parameter in LL_RCC_PLLI2S_ConfigDomain_I2S() API should
be selected among the following list of (already shifted) values:

  *         @arg @ref LL_RCC_PLLI2SR_DIV_2
  *         @arg @ref LL_RCC_PLLI2SR_DIV_3
  *         @arg @ref LL_RCC_PLLI2SR_DIV_4
  *         @arg @ref LL_RCC_PLLI2SR_DIV_5
  *         @arg @ref LL_RCC_PLLI2SR_DIV_6
  *         @arg @ref LL_RCC_PLLI2SR_DIV_7

This commit fixes PR zephyrproject-rtos#12609.

Signed-off-by: Armando Visconti <[email protected]>
@avisconti
Copy link
Collaborator Author

@erwango
re-pushed using the smart way :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants