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

Samr21: cpuid implementation #2052

Merged
merged 1 commit into from
Jan 12, 2015
Merged

Conversation

Troels51
Copy link
Contributor

Reading the 128bit serial number in the samd21 by copying it from memory addresses that point to non-volatile memory.
The addresses come from section 8.3.3 in the datasheet.

@thomaseichinger thomaseichinger 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 Nov 20, 2014
@thomaseichinger thomaseichinger added this to the Release 2014.12 milestone Nov 20, 2014
@thomaseichinger thomaseichinger self-assigned this Nov 20, 2014
/**
* @name CPUID_ID_LEN length of cpuid in bytes
* @{
*/
Copy link
Member

Choose a reason for hiding this comment

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

please add a space here.

@thomaseichinger
Copy link
Member

Two minor comment above. You also need to periph_cpuid to the board's Makefile.features.

@Troels51
Copy link
Contributor Author

Rebased, and added changes based on your comments

@thomaseichinger
Copy link
Member

Needs another rebase because of #2051

#include "cpu-conf.h"
#include "periph/cpuid.h"

#define Word0 (*(volatile uint32_t *)0x0080A00C)
Copy link
Contributor

Choose a reason for hiding this comment

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

better put a prefix on this generic name, and usualy we write constansts all upcase letters

Copy link
Member

Choose a reason for hiding this comment

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

@Troels51 please address this comment

Copy link
Member

Choose a reason for hiding this comment

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

Another remark: usually these addresses are defined in some header, e.g. cpu/samd21/include/component/component_cpuid.h

@thomaseichinger
Copy link
Member

needs rebase because of #2053

@@ -48,5 +48,11 @@
/** @} */


/**
* @name CPUID_ID_LEN length of cpuid in bytes
* @{
Copy link
Member

Choose a reason for hiding this comment

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

@name and and braces not needed. Just document with

/**
 * @brief Length of CPU ID in bytes
 */ 

@thomaseichinger
Copy link
Member

@Troels51 please address comments and rebase.

@Troels51
Copy link
Contributor Author

Sorry for being a bit slow, i addressed the comments and rebased+squashed

@miri64
Copy link
Member

miri64 commented Dec 15, 2014

Still needs rebase to current master.

@miri64
Copy link
Member

miri64 commented Dec 15, 2014

+ please squash only when the reviewing is done. Otherwise it can get hard to trace your changes since the last time someone looked at your code ;-)

@Troels51
Copy link
Contributor Author

Right, my bad :)

@@ -0,0 +1,17 @@

Copy link
Member

Choose a reason for hiding this comment

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

License header missing.

@N8Fear
Copy link

N8Fear commented Dec 17, 2014

I think if you wrote the code you're the copyright holder in the first place. You're free to concede it to someone (person or organization) else.
Unless you actually want to do that you should replace "Freie Universität Berlin" with your name. You could also opt to share the copyright with someone else and put e.g. an organization and your name in the copyright line.

@OlegHahm
Copy link
Member

Rebase is required.

@OlegHahm OlegHahm modified the milestones: Release 2014.12, Release NEXT MAJOR Jan 6, 2015
@LudwigKnuepfer
Copy link
Member

@Troels51 ping - please squash and rebase.

@LudwigKnuepfer
Copy link
Member

oh, and address #2052 (comment) as well, please (Word0 -> SAMD21_CPUID_WORD0 or whatever).

@Troels51
Copy link
Contributor Author

Rebased, and changed constant names

@@ -1 +1 @@
FEATURES_PROVIDED += transceiver periph_gpio periph_spi cpp periph_timer periph_uart periph_i2c cpp periph_rtc
FEATURES_PROVIDED += transceiver periph_gpio periph_spi cpp periph_timer periph_uart periph_i2c cpp periph_rtc periph_cpuid
Copy link
Member

Choose a reason for hiding this comment

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

Please add a newline.

@thomaseichinger thomaseichinger added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Jan 12, 2015
@Troels51
Copy link
Contributor Author

Added newline and squashed

@thomaseichinger thomaseichinger removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Jan 12, 2015
@thomaseichinger
Copy link
Member

ACK & Go

thomaseichinger added a commit that referenced this pull request Jan 12, 2015
@thomaseichinger thomaseichinger merged commit 61f3060 into RIOT-OS:master Jan 12, 2015
@OlegHahm OlegHahm modified the milestone: Release 2015.12 Dec 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

7 participants