Skip to content

Add framelength property to i2s_mcux_flexcomm.c driver. #88574

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

janmft
Copy link

@janmft janmft commented Apr 14, 2025

When the number of bit clock on the bus between two neighboring WS value is greater than bitWidth times lineChannels, frameLength needs to be set to the value that is equal to the number of bit clock between two neighboring WS signal.

Copy link

Hello @janmft, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@github-actions github-actions bot added area: I2S platform: NXP Drivers NXP Semiconductors, drivers labels Apr 14, 2025
@decsny decsny requested a review from EmilioCBen April 14, 2025 16:57
@janmft janmft force-pushed the proposal_for_framelength_property_in_i2s_mcux_flexcomm_driver branch from 75440c1 to 20c027d Compare April 22, 2025 13:44
When the number of bit clock on the bus between two neighboring WS
value is greater than bitWidth times lineChannels, frameLength needs
to be set to the value that is equal to the number of bit clock
between two neighboring WS signal.

update dts/bindings/i2s/nxp,lpc-i2s.yaml after yamllint

Signed-off-by: Jan Michaels <[email protected]>
@janmft janmft force-pushed the proposal_for_framelength_property_in_i2s_mcux_flexcomm_driver branch from 20c027d to 0a7fe4e Compare April 22, 2025 14:01
When the number of bit clock on the bus between two neighboring WS
value is greater than bitWidth times lineChannels, frameLength needs
to be set to the value that is equal to the number of bit clock
between two neighboring WS signal.

update dts/bindings/i2s/nxp,lpc-i2s.yaml after yamllint

run clang-format, squashed commits

Signed-off-by: Jan Michaels <[email protected]>
@janmft janmft force-pushed the proposal_for_framelength_property_in_i2s_mcux_flexcomm_driver branch from 0a7fe4e to f71147b Compare April 24, 2025 09:16
@janmft
Copy link
Author

janmft commented Apr 24, 2025

My intention was a very small fix on a driver with about 20 lines of code, most lines are documentation.

There have been failing checks about formatting, and i tried to fix, but checks still fail so I run
clang-format as described in https://docs.zephyrproject.org/latest/contribute/style/index.html#clang-format (My fault).

The result was a 130 change and now checks complain about lines I never want to change for the fix.

Last complain is about:

- static inline void i2s_purge_stream_buffers(struct stream *stream,
-                        struct k_mem_slab *mem_slab,
+ static inline void i2s_purge_stream_buffers(struct stream *stream, struct k_mem_slab *mem_slab,
+                        bool tx)

That struct stream and the pointer has the same name.

When i want to fix it, i have to rewrite the complete driver. Is this expected or wanted?

How to proceed?

We need this change, but the formal hurdles to get it into Zephyr seems to high.

@decsny
Copy link
Member

decsny commented Apr 25, 2025

hi @janmft , it is hard for me to tell what is going on here , can you remove the commit that ran clang format, and only submit your changes, then we can look at what CI is doing with just that

@janmft
Copy link
Author

janmft commented Apr 25, 2025

For me it looks simpler (less work) to close this PR and set up an new one.

Zephyr is very fast, there have been more than 730 commits now. And it feels like also clang or checkpatch config has changed.

When I have prepared a new PR (beginning next week) I will close this with a comment then.

@decsny
Copy link
Member

decsny commented Apr 25, 2025

For me it looks simpler (less work) to close this PR and set up an new one.

Zephyr is very fast, there have been more than 730 commits now. And it feels like also clang or checkpatch config has changed.

When I have prepared a new PR (beginning next week) I will close this with a comment then.

Don't do that, just rebase and force push. It's not harder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: I2S platform: NXP Drivers NXP Semiconductors, drivers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants