Skip to content

Conversation

@kail
Copy link

@kail kail commented Jul 6, 2022

This addresses the issue opened here: #1407

Background:

This PR introduced a change which appeared to be intended to allow the buffer size to be configurable.

That change, however, is frivolous because MCUBOOT_BOOT_MAX_ALIGN is guaranteed to be between 8 and 32

This PR:

This PR adds a new configuration macro to allow the region copy buffer size to be a value other than 1024, which should allow for a more efficient use of block devices when the block size is a different value.

It is backwards compatible with all version, as the default when that macro is unset is still 1024

Signed-off-by: Mikhail Skobov mskobov@lyft.com

@nvlsianpu nvlsianpu self-requested a review July 7, 2022 15:39
@nvlsianpu nvlsianpu added the area: core Affects core functionality label Jul 7, 2022
#define TARGET_STATIC
#endif

#if BOOT_MAX_ALIGN > 1024
Copy link
Collaborator

Choose a reason for hiding this comment

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

Previous values for undefined MCUBOOT_BOOT_COPY_REGION_SIZE should be kept

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure that I follow. How are they being discarded?

In all prior versions BOOT_MAX_ALIGN > 1024 is never true, so this condition can be removed entirely. And BOOT_COPY_REGION_SIZE is only defined iff MCUBOOT_BOOT_COPY_REGION_SIZE is defined, so by default, if a user does not define MCUBOOT_BOOT_COPY_REGION_SIZE then 1024 will be used

Copy link
Collaborator

@nvlsianpu nvlsianpu Jul 8, 2022

Choose a reason for hiding this comment

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

OK, Can you investigate CI failure?
Missing "Signed-off-by: Mikhail Skobov <mskobov@lyft.com>" in commit 7405424

Copy link
Author

Choose a reason for hiding this comment

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

Sure thing, I will take a look later today

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

Signed-off-by: Mikhail Skobov <skobovm@gmail.com>
@kail kail force-pushed the region_buf_size branch from 7405424 to 482a143 Compare July 8, 2022 16:42
@github-actions
Copy link

github-actions bot commented Jan 5, 2023

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the stale label Jan 5, 2023
@github-actions github-actions bot closed this Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: core Affects core functionality stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants