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

cpu/sam0-common: rename mkr ldscript to more a generic name #7646

Merged
merged 2 commits into from Oct 10, 2017

Conversation

aabadie
Copy link
Contributor

@aabadie aabadie commented Sep 26, 2017

This is because this ldscript can be used with any samd21 with an Arduino compatible bootloader preflashed. The idea is just to use a more generic name for the ldscript file.

This is the case for all Arduino MKR boards, Adafruit Feather boards (see #7510) and others.

This PR should address the question of #7645.

@aabadie aabadie added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Sep 26, 2017
@aabadie aabadie added this to the Release 2017.10 milestone Sep 26, 2017
@aabadie aabadie added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 26, 2017
@ynezz
Copy link
Contributor

ynezz commented Sep 28, 2017

The idea is just to use a more generic name for the ldscript file.

Small nitpick :-) From my point of view, it's not generic linker script, it's linker script which targets SAMD21G18A MCU with Arduino bootloader pre-flashed, so probably the more appropriate linker filename would be samd21g18a_arduino_bootloader.ld ?

@aabadie
Copy link
Contributor Author

aabadie commented Sep 28, 2017

probably the more appropriate linker filename would be samd21g18a_arduino_bootloader.ld ?

Good point, I buy it. Thanks!

@jnohlgard
Copy link
Member

jnohlgard commented Sep 28, 2017

using the suffix _bootloader is a bit ambiguous. I mean it can also be interpreted as "a linker script to use when building a bootloader" instead of "a linker script to use when running the application from a bootloader".

Edit: To be constructive: I suggest adding a README.md file to the ldscripts directory to explain which script should be used for what.

@aabadie
Copy link
Contributor Author

aabadie commented Sep 28, 2017

To be constructive: I suggest adding a README.md file to the ldscripts directory to explain which script should be used for what.

Added and comments welcome.

@aabadie aabadie force-pushed the samd21_bootloader branch 2 times, most recently from 14fb08c to 0aef67d Compare September 30, 2017 15:27
Copy link
Member

@jnohlgard jnohlgard left a comment

Choose a reason for hiding this comment

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

Looks fine to me, but I don't have the hardware to test it.

that starts after a preflashed Arduino bootloader. The firmware is copied to
the flash memory using [Bossa](https://github.com/shumatech/BOSSA).
This is the kind of configuration used with Arduino MKR and Adafruit Feather
M0 boards.
Copy link
Member

Choose a reason for hiding this comment

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

Missing newline at end of file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed and directly squashed.

@jnohlgard
Copy link
Member

Thanks for adding the README

@aabadie
Copy link
Contributor Author

aabadie commented Oct 10, 2017

It would be nice to merge this one, it's required for #7510.

@jnohlgard
Copy link
Member

I did not test on actual hardware, but this is only a rename and should be working on real hardware if the build succeeded and it worked before.

@jnohlgard jnohlgard merged commit 7c1c6ac into RIOT-OS:master Oct 10, 2017
@aabadie aabadie deleted the samd21_bootloader branch February 26, 2018 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants