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

lpuart: fix build errors for SINGLEWIRE and INVERT without SERIAL_TERMIOS #15183

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

AndreHeinemans-NXP
Copy link
Contributor

Summary

The features CONFIG_IMXRT_LPUART_SINGLEWIRE and CONFIG_IMXRT_LPUART_INVERT will get build errors when CONFIG_SERIAL_TERMIOS or CONFIG_SERIAL_TIOCSERGSTRUCT are not enabled.

This happens because INVERT and SINGLEWIRE rely on variables that are only declared when TERMIOS or TIOCSERGSTRUCT are enabled.

Fixed by making those vars available in all cases.

Impact

arm/imxrt, arm/s32k3xx, arm64/imx9

Testing

@github-actions github-actions bot added Arch: arm Issues related to ARM (32-bit) architecture Arch: arm64 Issues related to ARM64 (64-bit) architecture Size: S The size of the change in this PR is small labels Dec 13, 2024
@nuttxpr
Copy link

nuttxpr commented Dec 13, 2024

[Experimental Bot, please feedback here]

Fill In The Commit Message: This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary.

No, this PR description does not fully meet the NuttX requirements. While it provides a decent summary, it lacks crucial information in the Impact and Testing sections.

Here's a breakdown of what's missing and how to improve it:

Missing Information:

  • Impact: While the affected architectures are mentioned, the other impact categories are completely empty. The PR author must address all impact questions, even if the answer is "NO". For example, if there's no user impact, explicitly state "Impact on user: NO". If the build process doesn't change, state "Impact on build: NO". This confirms the author considered these aspects. For the architectures mentioned, describe the impact. Is it a bug fix? A new feature? Does it improve performance?
  • Testing: This section is entirely empty. The author must provide:
    • Details of their build host environment (OS, CPU, Compiler).
    • Details of the target hardware or simulator used.
    • Most importantly, actual testing logs from before and after the change. These logs should demonstrate the error occurring before the fix and its absence after the fix. Without these, the PR cannot be properly reviewed.

How to Improve:

Impact Example:

* Is new feature added? NO
* Is existing feature changed? YES. Fixes a build error when CONFIG_IMXRT_LPUART_SINGLEWIRE and CONFIG_IMXRT_LPUART_INVERT are enabled without CONFIG_SERIAL_TERMIOS or CONFIG_SERIAL_TIOCSERGSTRUCT.
* Impact on user:  NO.  Users who were encountering the build error will now be able to build successfully.
* Impact on build: NO. The build process itself is unchanged, only the build outcome for certain configurations is affected.
* Impact on hardware: YES. This fix affects arm/imxrt, arm/s32k3xx, and arm64/imx9 architectures by correcting the build issue.  This should enable the proper functionality of the SINGLEWIRE and INVERT features on these platforms.
* Impact on documentation: NO.
* Impact on security: NO.
* Impact on compatibility: YES.  This fix restores backward compatibility by allowing the use of the affected configurations.
* Anything else to consider? NO.

Testing Example:

I confirm that changes are verified on local setup and works as intended:
* Build Host(s): Linux (Ubuntu 22.04), x86_64, GCC 11.2.0
* Target(s): sim:imxrt1060-evk,  arm/imxrt:imxrt1060-evk (physical hardware)


Testing logs before change:

/path/to/nuttx/configs/imxrt1060-evk/nsh/Make.defs:108: recipe for target 'drivers/serial/imxrt_lpuart.o' failed
make[2]: *** [drivers/serial/imxrt_lpuart.o] Error 1
...
'cfsetospeed' undeclared here (not in a function); did you mean 'cfsetispeed'?


Testing logs after change:

...
make[1]: Leaving directory '/path/to/nuttx/nuttx'
Successfully built NuttX

By providing this additional information, the PR will meet the NuttX requirements and be much easier for reviewers to assess and merge.

@acassis acassis merged commit cfd79b8 into apache:master Dec 13, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: arm Issues related to ARM (32-bit) architecture Arch: arm64 Issues related to ARM64 (64-bit) architecture Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants