kconfig: Deprecate BOOTLOADER_SRAM_SIZE

[nordicjm]

This Kconfig is seemingly unused with exception to a single board,
whose maintainers have been unresponsive for a long time. It is
not useful to any other board and can be set in dts, therefore
deprecate it and schedule removal after the Zephyr 3.7 release

Fixes #65121

[Laczen]

@nordicjm it is to be expected that this Kconfig option is not used in the zephyr tree, this is mainly used by a bootloader to setup the ram usage at the end of ram. Bootloaders are not part of the zephyr tree and even mcuboot has no in tree zephyr configuration that shows how to setup ram loading.
The fact that even when this Kconfig option is set to 0 allows bootloaders to boot the ram image in 99.9 % of the cases does not mean it can be removed because a bootloader needs it to define its ram usage.
I agree that this can be done using the dts, but an example of this would also not be part of the zephyr tree (as it is a bootloader setup). At least it should be documented somewhere how to setup the ram for bootloader usage in the dts as an alternative to the present Kconfig option.

[nordicjm]

At least it should be documented somewhere how to setup the ram for bootloader usage in the dts as an alternative to the present Kconfig option.

This would be down to users and their choice of bootloader, the in-tree supported bootloader does not need this with the default configuration of swap using move/scratch

[nordicjm]

@vaishnavachath @vanti @tejlmand @de-nordic @d3zd3z @microbuilder please review

[tejlmand]

LGTM, thanks for this cleanup.

Note, this issue also seems to have been discussed earlier: #49484

[Laczen]

At least it should be documented somewhere how to setup the ram for bootloader usage in the dts as an alternative to the present Kconfig option.

This would be down to users and their choice of bootloader, the in-tree supported bootloader does not need this with the default configuration of swap using move/scratch

The default configuration should not be determining this, it should be all configurations of the in-tree supported bootloader. The documentation is needed for mcuboot running images from ram. This documentation should also show that it is possible to reuse the bootloader ram.

[Laczen]

Requires documentation for setting up a bootloader that starts images from ram.

[nordicjm]

Requires documentation for setting up a bootloader that starts images from ram.

How is that of any relevance to this change? Point out exactly why that is necessary to deprecate a feature that is undocumented, unused, unsupported and didn't have anything to do with what you're talking about here since it never worked with it:

[Laczen]

Requires documentation for setting up a bootloader that starts images from ram.

How is that of any relevance to this change? Point out exactly why that is necessary to deprecate a feature that is undocumented, unused, unsupported and didn't have anything to do with what you're talking about here since it never worked with it:

BOOTLOADER_SRAM_SIZE is used for 2 things:

1. For a XIP bootloader define the used ram size AND move it to the end of the available ram,
2. For a non-XIP image determine what ram is not available by reducing the available ram,

So let's make mcuboot (XIP) for a system with ram loading of images: Set CONFIG_BOOTLOADER_SRAM_SIZE to e.g. 16kB, mcuboot will use this 16kB at the end of RAM,
Make a RAM image for this case, as long as the end of the heap is smaller than end of RAM - 16kB it is possible to generate a image that fully utilizes the RAM for the execution slot, so for the application we can set CONFIG_BOOTLOADER_SRAM_SIZE equal to 0. The 16kB is available for the stack.

How is this relevant to the change: The change removes this kind of setup completely while not providing a solution of how to make this setup.

[nordicjm]

For a XIP bootloader define the used ram size AND move it to the end of the available ram,

Not supported by anything in-tree in zephyr, not relevant

For a non-XIP image determine what ram is not available by reducing the available ram,

Not supported by anything in-tree in zephyr, not relevant

So let's make mcuboot (XIP) for a system with ram loading of images:

You're talking about a new feature or sample, nothing to do with this PR

[Laczen]

@nordicjm,
since v3.6 nordic devices are using imply CONFIG_XIP to allow creating ram based images by setting CONFIG_XIP=n (although at the moment this doesn't seem to work).
Mcuboot has been supporting ram loading of images since ... (probably from the start), and this is supported by zephyr (illustrated by https://github.com/mcu-tools/mcuboot/blob/main/boot/zephyr/main.c#L152 and https://github.com/mcu-tools/mcuboot/blob/main/boot/zephyr/ram_load.conf).

[nordicjm]

Right and where do any of them set BOOTLOADER_SRAM_SIZE ?

[Laczen]

Right and where do any of them set BOOTLOADER_SRAM_SIZE ?

They don't because the example for ram loading seems to have a lot of ram that doesn't need to reuse it. An example configuration shouldn't be used as a determining factor for removing a Kconfig option. Instead it is better to look at an example where the Kconfig option would be used and to determine if this can really be removed.

As said, mcuboot and zephyr clearly support ram loading of images on all boards. When removing this Kconfig option all of them would need a dts showing how to use this. The existing Kconfig options could also be documented, this would be a lot less work.

[nordicjm]

They don't because the example for ram loading seems to have a lot of ram that doesn't need to reuse it. An example configuration shouldn't be used as a determining factor for removing a Kconfig option. Instead it is better to look at an example where the Kconfig option would be used and to determine if this can really be removed.

Quite right, an example would be great

As said, mcuboot and zephyr clearly support ram loading of images on all boards. When removing this Kconfig option all of them would need a dts showing how to use this. The existing Kconfig options could also be documented, this would be a lot less work.

A great idea for an enhancement, not for deprecating something that has no in-tree usage.

To make things simple, as a MCUboot collaborator, build system collaborator and nordic employee, because your NACK comments are actually of no relevance to this change I will be dismissing your review 2 days from now at which point this PR can be merged. You are will within your rights to escalate this should you wish to do so by following the information presented on https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#pr-technical-escalation or alternatively can withdraw your NACK and (optionally) create a new enhancement bug describing the additions you would like to see for a RAM loading sample.

Unrelated to deprecation in this PR, user has been told to raise an enhancement request

[Laczen]

@MaureenHelm, a quick summary of my view as a preparation of the dev-review meeting:

1. The PR proposes a removal of configuration value(s) that are valuable when building zephyr based bootloaders or zephyr based applications for ram execution that use an out-of-tree bootloader that is configured to use a region of ram at the end of the available ram,
2. With the values ram load images can be generated that are located at the start of ram and in specific conditions can utilize the complete available ram (although this is not implied by the configuration values),
3. The PR is justified by the ascertainment that the configuration value is not used in-tree. This justification is questionnable,

My proposal is:

1. Keep the configuration values, document the use in more detail (especially CONFIG_IS_BOOTLOADER) and inform the mcuboot developers on how to use these configuration values,
2. If it is really considered not an option to keep these configuration values the PR should include documentation on how the configuration can be created using an alternative method (dts ?) so that it is possible to evaluate if it is better to keep these configuration values or the alternative method.

[nordicjm]

The PR proposes a removal of configuration value(s) that are valuable when building zephyr based bootloaders or zephyr based applications for ram execution that use an out-of-tree bootloader that is configured to use a region of ram at the end of the available ram,

It is not supported by the official zephyr bootloader: MCUboot

The PR is justified by the ascertainment that the configuration value is not used in-tree. This justification is questionnable,

Yet in the whole thread you've be unable to point to a single project making use of this

Keep the configuration values, document the use in more detail (especially CONFIG_IS_BOOTLOADER) and inform the mcuboot developers on how to use these configuration values,

As previously said as a collaborator for MCUboot: no

If it is really considered not an option to keep these configuration values the PR should include documentation on how the configuration can be created using an alternative method (dts ?) so that it is possible to evaluate if it is better to keep these configuration values or the alternative method.

So where is the documentation for what this deprecates?

[carlescufi]

@Laczen @nordicjm I did not realize this had the dev-review label when I merged this. Let me bring this to the attention of others.

[Laczen]

@nordicjm, it is needed to deprecate CONFIG_IS_BOOTLOADER as well. When CONFIG_BOOTLOADER_SRAM_SIZE is not available it is not possible to generate a bootloader with the CONFIG_IS_BOOTLOADER option.