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

Added NM180410 Board Support #1058

Merged
merged 12 commits into from
Dec 7, 2023
Merged

Conversation

joshua-nmi
Copy link
Contributor

  • added NM180410 support
  • modified LPC55Sxx to support variants with smaller flash sizes (ex: 256kB)

@joshua-nmi joshua-nmi marked this pull request as ready for review October 23, 2023 23:37
@mathias-arm
Copy link
Collaborator

This looks mostly good.

The change of flash size is problematic because it impacts existing lpc55s69 bootloader and interface firmwares. I think there might have been some misunderstanding about the reserved pages on models with 256 KiB (table 4 in UM11126 only mentions 640 kB Flash, but it might apply to models with less flash). One possibility would be to define some lpc55(s)26 (or lpc55(s)16?) configuration. CC @flit.

@joshua-nmi
Copy link
Contributor Author

Thanks Mathias for your feedback. We've bought an LPC55S69EVB, an LPC55S28EVB, and we also verified this on the LPC55S26JEV98 which is what we used on the NM180410 and could confirm in all flash size configurations, the last 30kb of flash causes issue.

What's your thoughts on the following proposal:

  1. Under the source/hic_hal/nxp/lpc55xx directory, we create an additional variant directory similar to the LPC55S69 directory. In this case, it would be LPC55S26.
  2. In each variant directory, create a copy of daplink_addr.h with the relevant memory addresses. In the LPC55S69 directory, it will be a copy of the current daplink_addr.h. Whereas in the LPC55S26, it will contain the daplink_addr.h in this PR having smaller memory addresses.
  3. Remove the current daplink_addr.h from nxp/lpc55xx.
  4. Define a new build configuration file lpc55s26.yaml and point the include paths to source/hic_hal/nxp/lpc55xx/LPC55S26

This should have minimal impact to the existing projects using the LPC55S69 as the HIC and lay the foundation for other users who would like to use different flash sizes (there is one for 512kB LPC55S28JBD100K for example). Please let us know your preference and how you would like us to proceed.

Thanks,
Josh

@mathias-arm
Copy link
Collaborator

mathias-arm commented Oct 24, 2023

Thanks Mathias for your feedback. We've bought an LPC55S69EVB, an LPC55S28EVB, and we also verified this on the LPC55S26JEV98 which is what we used on the NM180410 and could confirm in all flash size configurations, the last 30kb of flash causes issue.

I would like the new memory map to be able to support LPC5516, which compared to LPC55S26 should mean restricting to 64 KiB of RAM and I think the "S" vs non-"S" should make a difference (speaking under the control of @flit).

I am puzzled that you needed to reduce flash size by 30 KiB. LPC551x datasheet says "The last 17 pages (12 KB) are reserved on the 256KB flash devices resulting in 244 KB internal flash memory.".

Updated proposal:

  1. Under the source/hic_hal/nxp/lpc55xx directory, we create an additional variant directory similar to the LPC55S69 directory. In this case, it would be LPC5516. Ideally if we could find the files with the same version. Alternatively we might want to update.
  2. Move source/hic_hal/nxp/lpc55xx/LPC55S69/drivers to source/hic_hal/nxp/lpc55xx/drivers.
  3. Keep using the same daplink_addr.h file but use CPU_LPC55S69JBD64_cm33_core0 and CPU_LPC5516JBD64_cm33_core0 macros to distinguish both SRAM / Flash sizes.
  4. Update file lpc55s69.yaml to add source/hic_hal/nxp/lpc55xx/LPC55S69/drivers
  5. Update file source/daplink/daplink.h to add #define DAPLINK_HIC_ID_LPC551X 0x4C509955 // 'LPc\x55'
  6. Define a new build configuration file lpc5516.yaml based on lpc55s69.yaml (with appropriate changes)
  7. Reduce SWO_BUFFER_SIZE in source/hic_hal/nxp/lpc55xx/DAP_config.h if CPU_LPC5516JBD64_cm33_core0 macro is defined.

This should have minimal impact to the existing projects using the LPC55S69 as the HIC and lay the foundation for other users who would like to use different flash sizes (there is one for 512kB LPC55S28JBD100K for example).

Current interface firmware requires about ~76 KiB Flash and the new upper limit would go down from 190 KiB to 160 KiB. This should be enough headroom for a while.

@joshua-nmi
Copy link
Contributor Author

Thanks for the direction. We will proceed with your proposal. We will close this pull request and re-open when we are ready.

Regarding the last 30k, AN12949 Chapter 3 Section 3.2 Caution on PFR region (page 5) states: Special caution needs to be taken when you operate last few pages near PFR. Do not perform a sector erase (erase 32KB) of the last flash sector. Based on the three different variants that we've tried, this might be universal across the entire LPC55xx series. Anyway, we will implement the change under LPC5516 as you've suggested.

Many thanks for your guidance and support on this.

@joshua-nmi joshua-nmi closed this Oct 25, 2023
@joshua-nmi joshua-nmi reopened this Oct 26, 2023
@joshua-nmi
Copy link
Contributor Author

Hi @mathias-arm

We've implemented all the proposed changes listed in our Slack channel conversations and had successfully ran the automated tests. I've took the liberty to re-organize the code under lpc55xx to better delineate the library and startup files for LPC55S26 and LPC55S69. Each subvariant now consists of a gcc and an armcc directory. We believe we are ready to merge. Could you please have a look?

Thanks,
Josh

source/board/lpc55s26_bl.c Outdated Show resolved Hide resolved
source/board/lpc55s26_bl.c Outdated Show resolved Hide resolved
records/hic_hal/lpc55s26.yaml Outdated Show resolved Hide resolved
source/hic_hal/nxp/lpc55xx/LPC55S26/gcc/startup_LPC55S26.S Outdated Show resolved Hide resolved
@joshua-nmi
Copy link
Contributor Author

@mathias-arm , thanks for looking at this during thanksgiving week. We will get started on implementing your feedback and let you when they are done.

@joshua-nmi
Copy link
Contributor Author

@mathias-arm Hope you had a good thanksgiving. All the changes you've requested were implemented and re-tested. Thanks for your feedback.

@mathias-arm
Copy link
Collaborator

Item 5 on my list above was still relevant (with some changes):

  • Update file source/daplink/daplink.h to add #define DAPLINK_HIC_ID_LPC552X 0x4C509955 // 'LPc\x55'

This is necessary to avoid flashing incompatible fimwares.

macros:
- INTERFACE_LPC55XX
- CPU_LPC55S26JBD64
- DAPLINK_HIC_ID=0x4C504355 # DAPLINK_HIC_ID_LPC55XX
Copy link
Collaborator

Choose a reason for hiding this comment

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

DAPLINK_HIC_ID=0x4C509955 # DAPLINK_HIC_ID_LPC552X

#include "daplink.h"

// This GPIO configuration is only valid for the LPC55XX HIC
COMPILER_ASSERT(DAPLINK_HIC_ID == DAPLINK_HIC_ID_LPC55XX);
Copy link
Collaborator

Choose a reason for hiding this comment

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

COMPILER_ASSERT((DAPLINK_HIC_ID == DAPLINK_HIC_ID_LPC55XX) ||
                (DAPLINK_HIC_ID == DAPLINK_HIC_ID_LPC552X));

Copy link
Collaborator

@mathias-arm mathias-arm left a comment

Choose a reason for hiding this comment

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

  • Update file source/daplink/daplink.h to add #define DAPLINK_HIC_ID_LPC552X 0x4C509955 // 'LPc\x55'
  • Add/change DAPLINK_HIC_ID_LPC552X where needed

Otherwise the rest looks good

@mathias-arm mathias-arm self-requested a review December 7, 2023 14:55
@mathias-arm mathias-arm merged commit 7cf42f1 into ARMmbed:main Dec 7, 2023
1 check passed
mathias-arm pushed a commit to mathias-arm/DAPLink that referenced this pull request Dec 7, 2023
@mathias-arm
Copy link
Collaborator

I made a mistake, I wanted to merge this to develop. I will cherry-pick on develop and remove commit from main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants