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

boot_serial: Fix serial recovery for LPC55x and MCXNx #2095

Merged

Conversation

butok
Copy link
Contributor

@butok butok commented Oct 14, 2024

  • Increases the unaligned buffer default value for LPC55x and MCXNx.
  • Fixes the serial recovery for LPC55x and MCXNx.

Comment on lines 68 to 71
default 64
range 0 128
default 512
range 0 1024
Copy link
Collaborator

Choose a reason for hiding this comment

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

the default should be changed for your boards/socs, not every board. There is no reason every board with serial recovery should suddenly have an extra 448 bytes of RAM taken up for literally nothing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @nordicjm
The goal to have the same user experience as for Zephyr MCUMgr that works for all cases out of the box.
Having the higher value bring only benefits.
It works for all boards, faster, and without any resource penalty as the MCUBoot stack is too high - set to 10KB (used <20%).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure, but you bring surprise to people who have tuned down stack and build of MCUboot, after this is brought in, topples into stack fault.
So now they have to go to their config files and bring that down.
I understand what you are trying to fix but you are not fixing that for everyone, quite an opposite.
The right approach is to have tuning done for device/soc/board.

Most people are not aware of the config or just briefly know of existence, because they have never had to deal with it as default worked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, but you bring surprise to people who have tuned down stack and build of MCUboot, after this is brought in, topples into stack fault. So now they have to go to their config files and bring that down. I understand what you are trying to fix but you are not fixing that for everyone, quite an opposite. The right approach is to have tuning done for device/soc/board.

Most people are not aware of the config or just briefly know of existence, because they have never had to deal with it as default worked.

Updated.
Default value increased only for LPC55 and MCXN, for all other platforms unchanged.

- Increases the unaligned buffer default value
  for LPC55x and MCXNx.
- Fixes the serial recovery for LPC55x and MCXNx.

Signed-off-by: Andrej Butok <andrey.butok@nxp.com>
@butok butok force-pushed the serial_recovery_increase_buffer_range branch from dd3121f to 2ea791d Compare October 24, 2024 13:20
@butok butok changed the title boot_serial: increase the unaligned buffer range boot_serial: Fix serial recovery for LPC55x and MCXNx Oct 24, 2024
@de-nordic de-nordic added the NXP Changes related to NXP platforms label Oct 25, 2024
@de-nordic de-nordic merged commit 439930a into mcu-tools:main Oct 25, 2024
58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NXP Changes related to NXP platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants