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.

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

[Backport v2.7-branch] uart_stm32: Fix conflict between poll_out and irq API #42786

Closed
wants to merge 1 commit into from

Conversation

zephyrbot
Copy link
Collaborator

Backport d42cef1 from #40173

A lock was added to manage situation where the API poll_out and irq API
are used in same time.

Signed-off-by: Julien D'ascenzio <[email protected]>
@github-actions github-actions bot added the platform: STM32 ST Micro STM32 label Feb 14, 2022
@ycsin
Copy link
Member

ycsin commented Feb 14, 2022

Associated issue #40775

@ycsin
Copy link
Member

ycsin commented Feb 14, 2022

Backport of #40782 & #41116 depends on this, I'm not sure if we should create a branch that patch the uart driver using the final version or backport all of the related and dependent PRs, thoughts? @cfriedt @erwango

@cfriedt
Copy link
Member

cfriedt commented Feb 14, 2022

Backport of #40782 & #41116 depends on this, I'm not sure if we should create a branch that patch the uart driver using the final version or backport all of the related and dependent PRs, thoughts? @cfriedt @erwango

I'm impartial - as long as there is an issue filed ;-)

@ycsin
Copy link
Member

ycsin commented Feb 15, 2022

I'm impartial - as long as there is an issue filed ;-)

The initial PR #39752 is merged and backported without an issue filed, the author described the bug directly in the PR. The second PR #40173 is a continuation to fix the same issue and is merged without an issue filed; the bug and the reproduction steps are described directly in the PR. After this PR, it seems to generate some other issue as described by @erwango in #40775, which #40782 & #41116 are aimed to fix.

@cfriedt cfriedt changed the title [Backport v2.7-branch] uart_stm32: Fix conflit between poll_out and irq API [Backport v2.7-branch] uart_stm32: Fix conflict between poll_out and irq API Feb 22, 2022
@cfriedt
Copy link
Member

cfriedt commented Feb 23, 2022

@erwango - can you approve the backport?

@erwango
Copy link
Member

erwango commented Feb 24, 2022

@erwango - can you approve the backport?

Approved but, we now need to complete the work and back port #40782 & #41116. @ycsin would you mind taking care of those ?

@ycsin
Copy link
Member

ycsin commented Feb 24, 2022

@erwango - can you approve the backport?

Approved but, we now need to complete the work and back port #40782 & #41116. @ycsin would you mind taking care of those ?

I think they should probably just work? I had no issue cherry-picking them into my local 2.7 LTS branch, I guess we just need to apply the backport label to them one by one after this one is merged

@erwango
Copy link
Member

erwango commented Feb 24, 2022

I think they should probably just work? I had no issue cherry-picking them into my local 2.7 LTS branch, I guess we just need to apply the backport label to them one by one after this one is merged

Point is that backport label is already added, but the backport PR didn't show up from what I can see. Unless I've missed them ?

@ycsin
Copy link
Member

ycsin commented Feb 24, 2022

I think they should probably just work? I had no issue cherry-picking them into my local 2.7 LTS branch, I guess we just need to apply the backport label to them one by one after this one is merged

Point is that backport label is already added, but the backport PR didn't show up from what I can see. Unless I've missed them ?

Yup because they were missing this backport, should be a merge conflict.

Do you think if we should close this PR and have a new backport PR that cherry-pick all the commits from these 3 PR?

@erwango
Copy link
Member

erwango commented Feb 24, 2022

Do you think if we should close this PR and have a new backport PR that cherry-pick all the commits from these 3 PR?

Since the PRs are small enough and impact a single file I think that would be fine.
Then, I don't know if we should keep 3 different commits or a single one. Maybe @cfriedt could give his advice on this particular point.
Let's start with 3 commits and we'll discuss with a squash is a better option.

@ycsin
Copy link
Member

ycsin commented Feb 24, 2022

@erwango

I'm impartial - as long as there is an issue filed ;-)

#42786 (comment)

@ycsin ycsin added the DNM This PR should not be merged (Do Not Merge) label Feb 24, 2022
@ycsin
Copy link
Member

ycsin commented Feb 24, 2022

superseded by #43156

@ycsin ycsin closed this Feb 25, 2022
@carlescufi carlescufi deleted the backport-40173-to-v2.7-branch branch May 17, 2022 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DNM This PR should not be merged (Do Not Merge) platform: STM32 ST Micro STM32
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants