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

i2c_mcux_flexcomm: add transaction timeout option #68315

Merged
merged 1 commit into from
Feb 2, 2024
Merged

i2c_mcux_flexcomm: add transaction timeout option #68315

merged 1 commit into from
Feb 2, 2024

Conversation

guicnDell
Copy link
Contributor

With the device_sync_sem semaphore, there is the possibility of the code not returning to give it back (mcux_flexcomm_master_transfer_callback is never called), causing it to get stuck in k_sem_take(&data->device_sync_sem, K_FOREVER) in subsequent calls.

The i2c driver recovers by other means (enabling FSL_FEATURE_I2C_TIMEOUT_RECOVERY) but the callback might not return.

Adding a timeout option allows for this occurrence to be avoided.

@zephyrbot zephyrbot added platform: NXP Drivers NXP Semiconductors, drivers area: I2C labels Jan 30, 2024
@github-actions GitHub Actions
Copy link

Hello @guicnDell, 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. 😊

@DerekSnell
Copy link
Contributor

Hi @guicnDell ,
Thank you for contributing. Can you please fix the compliance issues?

@guicnDell guicnDell force-pushed the add-i2c-transaction-timeout branch from d64468e to 2fb9aef Compare January 31, 2024 13:33
@dleach02 dleach02 added this to the v3.6.0 milestone Jan 31, 2024
@guicnDell guicnDell requested a review from decsny February 1, 2024 01:32
@guicnDell guicnDell force-pushed the add-i2c-transaction-timeout branch from 2fb9aef to 6298d13 Compare February 1, 2024 15:30
danieldegrasse
danieldegrasse previously approved these changes Feb 1, 2024
View reviewed changes
decsny
decsny requested changes Feb 1, 2024
View reviewed changes
Copy link
Member

@decsny decsny left a comment

Choose a reason for hiding this comment

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

https://docs.zephyrproject.org/latest/contribute/coding_guidelines/index.html#coding-guidelines

Rule 20.8 is not followed

@guicnDell guicnDell force-pushed the add-i2c-transaction-timeout branch from 6298d13 to 996ed52 Compare February 1, 2024 21:48
@zephyrbot zephyrbot added the area: Coding Guidelines Coding guidelines and style label Feb 1, 2024
@guicnDell
i2c_mcux_flexcomm: add transaction timeout option
0df8bd6 
With the device_sync_sem semaphore, there is the possibility of
the code not returning to give it back
(mcux_flexcomm_master_transfer_callback is never called),
causing it to get stuck in k_sem_take(&data->device_sync_sem, K_FOREVER)
in subsequent calls.

The i2c driver recovers by other means
(enabling FSL_FEATURE_I2C_TIMEOUT_RECOVERY)
but the callback might not return.

Adding a timeout option allows for this occurrence to be avoided.

Signed-off-by: Guilherme Casa Nova <guilherme.casa_nova@dell.com>
@guicnDell guicnDell force-pushed the add-i2c-transaction-timeout branch from 996ed52 to 0df8bd6 Compare February 1, 2024 21:50
@dleach02 dleach02 merged commit 583f24e into zephyrproject-rtos:main Feb 2, 2024
19 checks passed
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 2, 2024

Hi @guicnDell!
Congratulations on getting your very first Zephyr pull request merged 🎉🥳. This is a fantastic achievement, and we're thrilled to have you as part of our community!

To celebrate this milestone and showcase your contribution, we'd love to award you the Zephyr Technical Contributor badge. If you're interested, please claim your badge by filling out this form: Claim Your Zephyr Badge.

Thank you for your valuable input, and we look forward to seeing more of your contributions in the future! 🪁

@guicnDell guicnDell deleted the add-i2c-transaction-timeout branch February 2, 2024 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@decsny decsny decsny approved these changes

@danieldegrasse danieldegrasse danieldegrasse approved these changes

@teburd teburd teburd approved these changes

@mmahadevan108 mmahadevan108 Awaiting requested review from mmahadevan108

@dleach02 dleach02 Awaiting requested review from dleach02

@dbaluta dbaluta Awaiting requested review from dbaluta

@manuargue manuargue Awaiting requested review from manuargue

@carlescufi carlescufi Awaiting requested review from carlescufi

@jfischer-no jfischer-no Awaiting requested review from jfischer-no

@keith-zephyr keith-zephyr Awaiting requested review from keith-zephyr

@nashif nashif Awaiting requested review from nashif

@henrikbrixandersen henrikbrixandersen Awaiting requested review from henrikbrixandersen

Assignees

@teburd teburd

Labels
area: Coding Guidelines Coding guidelines and style area: I2C platform: NXP Drivers NXP Semiconductors, drivers
Projects
None yet
Milestone
v3.6.0
Development

Successfully merging this pull request may close these issues.
8 participants
@guicnDell @DerekSnell @teburd @henrikbrixandersen @danieldegrasse @decsny @zephyrbot @dleach02