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

Add ability to use multiple UARTs on STM32L0, STM32G0 when IRQ is shared #15221

Merged
merged 4 commits into from Feb 22, 2022

Conversation

amcnicoll
Copy link
Contributor

Summary of changes

This commit is a partial solution to issue #15211

Description of the issue with repro steps are included above.

This change allows the concurrent use of multiple UARTs on STM32L0 and STM32G0 platforms, which share an interrupt among multiple UARTs. This is especially critical for STM32G0, where some parts share the same ISR for USART 3, 4, 5, 6 and LPUART1.

Impact of changes

The STM32L0 change has been tested to resolve my particular use case (using USART 4 and 5 and the same time).
I have not tested the changes for STM32G0.

The only side-effect I can think of is that more time will be spent inside the interrupt, particularly for USART3_4_5_6_LPUART1_IRQn on STM32G0. The interrupt handler will call uart_irq() for each of the UARTs. But it will bail out of those calls pretty quickly if the UART is not in use (no interrupt flags set), so the impact seems minimal and worthwhile.

Migration actions required

None that I'm aware of, unless the user is already patching around this issue themselves in unconventional ways.

Documentation

None that I'm aware of - could not find any documentation calling this issue out.


Pull request type

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[X] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

I'm not familiar with how mbed tests. I've only tested the STM32L0 on a real application/hardware. The STM32G0 test should at minimum be unit tested, if something covers that.


Reviewers

Thanks @0xc0170 for sending me here.


@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Feb 5, 2022
@ciarmcom ciarmcom requested a review from a team February 5, 2022 17:30
@ciarmcom
Copy link
Member

ciarmcom commented Feb 5, 2022

@amcnicoll, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

Waiting for @ARMmbed/team-st-mcd review as well

static void uart5_irq(void)
{
#endif
#if defined(USART4_BASE)
Copy link
Contributor

Choose a reason for hiding this comment

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

Preprocessor macros always at the line beginning, as it was in the file before, please align

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just pushed this.

@mergify mergify bot dismissed 0xc0170’s stale review February 12, 2022 17:36

Pull request has been modified.

Copy link
Collaborator

@jeromecoutant jeromecoutant left a comment

Choose a reason for hiding this comment

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

NUCLEO_G071RB doesn't compile

[Error] serial_device.c@368,21: use of undeclared identifier 'USART3_IRQn'
[Error] serial_device.c@373,21: use of undeclared identifier 'USART4_IRQn'
[Error] serial_device.c@388,21: use of undeclared identifier 'LPUART1_IRQn'; did you mean 'USART1_IRQn'?

@jeromecoutant
Copy link
Collaborator

This commit is a partial solution to issue #15211

Why partial ?

@amcnicoll
Copy link
Contributor Author

Ah, my bad. Just pushed; give that a shot. Let me know if we're approaching squash territory.

"Partial" because I have (and will) only take the time to fix STM32L0 and STM32G0 since these are heavy use entry-level lines. I assume this pattern/issue still exists on G4, L1, L4, L5, etc.

Copy link
Collaborator

@jeromecoutant jeromecoutant left a comment

Choose a reason for hiding this comment

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

Please try to build NUCLEO_G071RB
Thx

{
#if defined(USART2_BASE)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line has to be removed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually it appeared to be missing an #endif, fixed in fe698ea

@amcnicoll
Copy link
Contributor Author

I'm sorry, but I'm not sure what you mean by

Please try to build NUCLEO_G071RB

I've read the contribution guide and can't quickly sort out how to test specific platforms, whether there are standard test projects, or anything of the sort. Are you suggesting I take the time to set up my own dummy project for STM32G071RB?

I think I fixed the remaining issue - if your test passes then I think we're all set anyway.

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 21, 2022

CI started

@mbed-ci
Copy link

mbed-ci commented Feb 21, 2022

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️

@0xc0170 0xc0170 merged commit 48b1b8e into ARMmbed:master Feb 22, 2022
@mergify mergify bot removed the ready for merge label Feb 22, 2022
@mbedmain mbedmain added release-version: 6.16.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants