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

cpu/lpc2387: Cleanup #11920

Merged
merged 3 commits into from Sep 11, 2019
Merged

cpu/lpc2387: Cleanup #11920

merged 3 commits into from Sep 11, 2019

Conversation

maribu
Copy link
Member

@maribu maribu commented Jul 26, 2019

Note: The boot up sequence of the LPC2387 is quite weird, with clock setup in the boards/ rather than in cpu/. I intend to clean this up with a series of PRs. A side effect from the code organization is that this PR touches files in seemingly unrelated subsystems...

Contribution description

  • Removed unused and defunct code from the clock configuration during boot up
  • Removed obsolete ISR implementations that are not registered anywhere anymore and thus never get triggered
  • Removed function declarations of functions that are neither used nor implemented anywhere
  • Previously boot up code and ISR implementations where mixed together in bootloader.c
    • Moved ISR implementations to vector.c and applied naming scheme used by other modules in cpu/
    • Moved boot up code to arm7_init.c.

Testing procedure

Try to build, flash, and run some applications and tests currently working on the MSB-A2. They should still work

Issues/PRs references

One of many steps to address #4693

The MSB-A2 and the Avsextrem board once had support for using the USB
interface of the LPC2387. The code setting up the USB clock has been unused
for ages and is now defunct, as the required value for `USBCLKDivValue` is no
longer present in RIOT's code base. This commits removes the defunct and unused
code.
- Functions bl_uart_init(), bl_blink(), bl_config_init() declared but never
  implemented
    --> Removed declarations
- Check for c preprocessor macro CPU_MC1322X is obsolete, as CPU_MC1322X is
  nowhere defined in RIOT's code base
- IRQ_Routine() is never used, nor is it ever configured as ISR
- DEBUG_Routine() is never used, nor is it ever configured as ISR
- split up interrupt vector code from bootloader.c to vectors.c
- moved bootloader.c to arm7_init.c
- Use consistent naming:
    - use lower case for everything but preprocessor stuff
    - ISRs now named isr_foo()
@maribu maribu added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: cpu Area: CPU/MCU ports labels Jul 26, 2019
@maribu
Copy link
Member Author

maribu commented Jul 26, 2019

@PeterKietzmann: Mind to review also this one?

@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Aug 30, 2019
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

I don't have the hardware to test, but even though this looks scary at first, it's actually no big changes:

  • the first two commits just remove dead code. CI makes sure that those #ifdefs really always have been false.
  • the third commit just moves code around and renames a few functions. No functional change here. CI will make sure that the old names are not used anymore.

@benpicco benpicco merged commit 6f63ef4 into RIOT-OS:master Sep 11, 2019
@maribu
Copy link
Member Author

maribu commented Sep 11, 2019

Thanks for the review! :-)

@maribu maribu deleted the lpc2387 branch September 11, 2019 08:16
@kb2ma kb2ma added this to the Release 2019.10 milestone Sep 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants