Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(PeriphDrivers, Examples): MSDK-1232: Update I2S driver for confused usage of wsize and bits_word register fields #847

Merged
merged 8 commits into from
Feb 9, 2024
Merged

fix(PeriphDrivers, Examples): MSDK-1232: Update I2S driver for confused usage of wsize and bits_word register fields #847

merged 8 commits into from
Feb 9, 2024

Conversation

ahmetalincak
Copy link
Contributor

Description

Fix I2S Peripheral Driver for mixed usage of sample_size and bits_word registers. As described in the related Jira tickets, there was a confused usage for the word size and bits_word.

image

image

I updated all I2S examples to use new changes and made them buildable. I am going to open a new PR for examples after my changes are approved.

@github-actions github-actions bot added MAX32655 Related to the MAX32655 (ME17) MAX32670 Related to the MAX32670 (ME15) MAX32672 Related to the MAX32672 (ME21) MAX32675 Related to the MAX32675 (ME16) MAX32680 Related to the MAX32680 (ME20) MAX32690 Related to the MAX32690 (ME18) MAX78000 Related to the MAX78000 (AI85) MAX78002 Related to the MAX78002 (AI87) MAX32662 Related to the MAX32662 (ME12) labels Dec 28, 2023
@ahmetalincak ahmetalincak changed the title fix(PeriphDrivers): MSDK-1232: Update I2S driver for confused usage of wsize and bits_word register fields fix(PeriphDrivers, Examples): MSDK-1232: Update I2S driver for confused usage of wsize and bits_word register fields Dec 29, 2023
@ahmetalincak ahmetalincak force-pushed the fix/i2s_driver_enhancements branch 2 times, most recently from c2fb5e9 to 89e62eb Compare January 2, 2024 12:24
@ahmetalincak
Copy link
Contributor Author

Also fixes the #762 .

@ahmetalincak ahmetalincak mentioned this pull request Jan 8, 2024
Closed
sihyung-maxim
sihyung-maxim reviewed Jan 12, 2024
View reviewed changes
Copy link
Contributor

@sihyung-maxim sihyung-maxim left a comment

Choose a reason for hiding this comment

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

Name change complications... will need to discuss with the team.

Libraries/PeriphDrivers/Include/MAX32655/i2s.h Outdated Show resolved Hide resolved
Libraries/PeriphDrivers/Include/MAX32655/i2s.h Outdated Show resolved Hide resolved
Libraries/PeriphDrivers/Include/MAX32655/i2s.h Outdated Show resolved Hide resolved
@sihyung-maxim sihyung-maxim added the Under discussion This issue needs to be discussed with the Team. label Jan 12, 2024
Jake-Carter
Jake-Carter reviewed Jan 13, 2024
View reviewed changes
Copy link
Contributor

@Jake-Carter Jake-Carter left a comment

Choose a reason for hiding this comment

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

Thanks @ahmetalincak, this is a great improvement.

@BrentK-ADI I believe the I2S application you were working on last year would be a good case study to check these changes against. Can you run a build of your application against this branch and give us your perspective? I wonder how significant the build errors/update requirements are. Ideally, it's just a few minor updates to your request structs. Your feedback would be really valuable considering the driver issues you worked around.

@ahmetalincak ahmetalincak force-pushed the fix/i2s_driver_enhancements branch from f193dfd to c4d2ba7 Compare January 16, 2024 06:58
@ahmetalincak ahmetalincak mentioned this pull request Jan 16, 2024
Closed
@ahmetalincak
Copy link
Contributor Author

Hi @Jake-Carter and @sihyung-maxim. After our discussions, I decided to create another PR so that we can compare and pick one solution. Please see #878 for details.

After deciding on the solution, I'll merge the developments on examples.

@ahmetalincak ahmetalincak force-pushed the fix/i2s_driver_enhancements branch 3 times, most recently from 4663abb to ffe81f1 Compare January 29, 2024 20:02
@ahmetalincak ahmetalincak force-pushed the fix/i2s_driver_enhancements branch from ffe81f1 to 78a8928 Compare February 5, 2024 11:59
sihyung-maxim
sihyung-maxim previously requested changes Feb 7, 2024
View reviewed changes
Examples/MAX32655/I2S/main.c Outdated Show resolved Hide resolved
@sihyung-maxim
Copy link
Contributor

Everything else looks good, by the way.

@ahmetalincak
Copy link
Contributor Author

Hi @Jake-Carter, thanks for the latest change!

@Jake-Carter
Copy link
Contributor

Hi @Jake-Carter, thanks for the latest change!

No problem!

@Jake-Carter Jake-Carter merged commit 62fe201 into main Feb 9, 2024
9 checks passed
@Jake-Carter Jake-Carter deleted the fix/i2s_driver_enhancements branch February 9, 2024 20:15
@Jake-Carter Jake-Carter linked an issue Feb 10, 2024 that may be closed by this pull request
The Max78000 FTHR, while running the kws_20_demo with a 3.5mm microphone input, is experiencing issues #762
Closed
EricB-ADI pushed a commit that referenced this pull request Aug 21, 2024
Revert "fix(PeriphDrivers, Examples): MSDK-1232: Update I2S driver fo…
7c85602 
…r confused usage of wsize and bits_word register fields (#847)"

This reverts commit 62fe201.
@selimgullulu
Copy link
Contributor

Hi, I'm having problems using the example on the MAX78000FTHR using codec and external mic.
Please see it here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jake-Carter Jake-Carter Jake-Carter approved these changes

@sihyung-maxim sihyung-maxim sihyung-maxim left review comments

Assignees
No one assigned
Labels
MAX32655 Related to the MAX32655 (ME17) MAX32662 Related to the MAX32662 (ME12) MAX32670 Related to the MAX32670 (ME15) MAX32672 Related to the MAX32672 (ME21) MAX32675 Related to the MAX32675 (ME16) MAX32680 Related to the MAX32680 (ME20) MAX32690 Related to the MAX32690 (ME18) MAX78000 Related to the MAX78000 (AI85) MAX78002 Related to the MAX78002 (AI87) Under discussion This issue needs to be discussed with the Team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The Max78000 FTHR, while running the kws_20_demo with a 3.5mm microphone input, is experiencing issues
4 participants
@ahmetalincak @sihyung-maxim @Jake-Carter @selimgullulu