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

MCUboot should not need a DTS overlay for building with Zephyr #915

Closed
ioannisg opened this issue Jan 13, 2021 · 8 comments
Closed

MCUboot should not need a DTS overlay for building with Zephyr #915

ioannisg opened this issue Jan 13, 2021 · 8 comments

Comments

@ioannisg
Copy link
Collaborator

ioannisg commented Jan 13, 2021

MCUboot has a .dts overlay, when building using Zephyr which "choses" the boot partition where the mcuboot binary will reside:

/ {
	chosen {
		zephyr,code-partition = &boot_partition;
	};
};

I think this should, instead, be present in Zephyr, since, anyways, the boot partition is defined in the Zephyr board DTS files. The current solution depends on Zephyr defining a flash partition named as "boot_partition"; this is a cross-project dependency which is unnecessary. Better to let Zephyr choose the code partition (as it does in various other usecases) based on config MCUBOOT being enabled.

@ioannisg
Copy link
Collaborator Author

@utzig
Copy link
Member

utzig commented Jan 13, 2021

Also some previous discussion in #604

@mbolivar-nordic
Copy link
Contributor

Better to let Zephyr choose the code partition (as it does in various other usecases) based on config MCUBOOT being enabled.

Why is MCUboot an exception to the general idea that zephyr-specific glue should be in the zephyr repository?

It seems that an even better solution would be to move the zephyr application stub to zephyr, no?

@ioannisg
Copy link
Collaborator Author

Well it should not be an exception, but since MCUboot is a Zephyr application it traditionally have had zephyr-specific code. Maybe @nvlsianpu recalls the details of why the mcuboot dts overlay was introduced, in the first place. my whole point is to remove this overlay from mcuboot.

@nvlsianpu
Copy link
Collaborator

Why? As you mention MCUboot is an application, DTS was chosen to be a place for configure boot code area, and this has to be used by by the application.

Why is MCUboot an exception to the general idea that zephyr-specific glue should be in the zephyr repository?

MCUboot is the application. Also It was used by zephyr long before where the general idea that zephyr-specific glue should be defined.

Other MCUboot's ports did similar - glue layer (mcuboot porting layer implementation) is in this repo.
I'm not against moving this chosen record to the zephyr.

And I think this is not real cross-dependency. If something will be not match, then MCUboot will be courted but zephyr won't be.

@nvlsianpu
Copy link
Collaborator

Better to let Zephyr choose the code partition (as it does in various other usecases) based on config MCUBOOT being enabled.

What usercases?

@d3zd3z
Copy link
Member

d3zd3z commented Jan 15, 2021

This DTC fragment was added before support was added to Zephyr for the concept of MCUboot. I do feel strongly that it should be possible to still build MCUboot as an ordinary Zephyr application (think of someone using Zephyr to make MCUboot, and using a different RTOS for the application).

One could argue both sides of whether the Zephyr specific code should be in mcuboot or in Zephyr. I think most of it should stay here. MCUboot is unlike other things that are simply brought into Zephyr. MCUboot is an application that uses Zephyr effectively as a BSP. The other platforms supported by Zephyr have their support code in the mcuboot repo.

That isn't to say it is a problem to do things that reduce the size of this code, (like implementing things like the flash API in Zephyr).

The main disadvantage of having this code in MCUboot is that it is a little more difficult to track changes to APIs in Zephyr that affect it. But, this would be the case with any Zephyr app, and it is probably good for people working on Zephyr to at least get a glimpse of the kind of pain these kinds of changes cause.

I would be fine removing the DTS fragment, as long as the relevant config files (see samples/zephyr) are updated to make sure the right things are defined to build correctly.

@github-actions
Copy link

This issue 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 issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

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

No branches or pull requests

5 participants