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] STM32 UART fixes #43156

Merged
merged 3 commits into from
Mar 2, 2022

Conversation

ycsin
Copy link
Member

@ycsin ycsin commented Feb 24, 2022

This backport consists of #40173, #40782 & #41116

Fixes #40775

@ycsin ycsin removed the area: ARM64 ARM (64-bit) Architecture label Feb 24, 2022
@ycsin ycsin marked this pull request as draft February 24, 2022 13:52
@ycsin ycsin changed the base branch from main to v2.7-branch February 24, 2022 13:54
@ycsin
Copy link
Member Author

ycsin commented Feb 24, 2022

all, Sorry for the mess, I chose the wrong branch

@ycsin ycsin marked this pull request as ready for review February 24, 2022 13:55
@ycsin ycsin requested review from erwango, ABOSTM and FRASTM February 24, 2022 13:55
@github-actions github-actions bot added the platform: STM32 ST Micro STM32 label Feb 24, 2022
@erwango erwango requested a review from cfriedt February 24, 2022 14:30
@ycsin
Copy link
Member Author

ycsin commented Feb 24, 2022

I think I messed up the CI pipelines

@erwango
Copy link
Member

erwango commented Feb 24, 2022

I think I messed up the CI pipelines

We need a rerun. Would you mind editing your branch with a minor change and force push to trigger CI again?

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]>
A dead lock could happen if 2 threads with differents priorities use
poll_out. In fact, the lock data->tx_lock could be lock by a thread with
lower priority and then a thread with higher priority can't take the
lock. There was a race condition here:

/* Wait for TXE flag to be raised */
while (1) {
	if (atomic_cas(&data->tx_lock, 0, 1)) {
		/* !!!!!!!! RACE CONDITION !!!!!!!!!!!!!!
		if (LL_USART_IsActiveFlag_TXE(UartInstance)) {
			break;
		}
		atomic_set(&data->tx_lock, 0);
	}
}

To fix race condition, the interrupts are locked in poll_out.

Signed-off-by: Julien D'ascenzio <[email protected]>
Some tests like:
    tests/kernel/sched/metairq/kernel.scheduler.metairq
    tests/kernel/profiling/profiling_api/kernel.common.profiling
    tests/kernel/sched/schedule_api/kernel.scheduler
    tests/kernel/sched/schedule_api/kernel.scheduler.multiq
    tests/kernel/profiling/profiling_api/kernel.common.profiling
    tests/kernel/workq/work_queue/kernel.workqueue

don't support that the current thread change when writing a message with
printk (which uses poll_out). So, we remove the call to k_yield which is
useful only for optimizing cpu usage by forcing a thread change if the
usart send stack is full.

Signed-off-by: Julien D'ascenzio <[email protected]>
@mbolivar-nordic
Copy link
Contributor

We need a rerun. Would you mind editing your branch with a minor change and force push to trigger CI again?

I clicked the rebase button

@ycsin ycsin added area: UART Universal Asynchronous Receiver-Transmitter area: Drivers labels Feb 25, 2022
@erwango erwango added this to the v2.7.2 milestone Mar 2, 2022
@cfriedt
Copy link
Member

cfriedt commented Mar 2, 2022

Looks good - I just wanted to double check that the superceded backport PRs were closed, and it looks like thats the case.

@cfriedt cfriedt merged commit f77cb92 into zephyrproject-rtos:v2.7-branch Mar 2, 2022
@ycsin ycsin deleted the pr/stm32_uart_bp branch March 3, 2022 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Drivers area: UART Universal Asynchronous Receiver-Transmitter platform: STM32 ST Micro STM32
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants