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

samd21: merge samr21 variant into samd21 cmsis headers #1

Closed
wants to merge 3 commits into from

Conversation

kaspar030
Copy link

No description provided.

@keestux
Copy link
Owner

keestux commented Aug 14, 2016

I'm not happy with this.

  1. Can you confirm that you have closely investigated to differences between the samd21 and samr21 include files? Just so you know, I have and I don't want to take responsibility to use samd21 for samr21 or vice versa. There are differences that I don't know enough about. For example instance/evsys.h or instance/nvmctrl.h.
    You are right that the samd21 and samr21 files are mostly the same. But with an emphasis on "mostly".

  2. I want a better explanation why we need to undef LITTLE_ENDIAN. The comment states it is needed for newlib. But I want to see for myself how that becomes a conflict. What do I need to build?

In general I want a clear documented description of the process of adding / changing the CMSIS files. The samr21 originated from ASF 3.17, or 3.18, or 3.19. It is not good that we don't know.

And the process must be repeatable. Atmel maintains these files. The latest version is 3.32, maybe we want take an update at some point. I've spend lots of hours already to figure out what we have today (which ASF version? What changes were applied? Renamed files. Etc.). Let's not have to waste our precious time again.

@kaspar030
Copy link
Author

  1. Can you confirm that you have closely investigated to differences between the samd21 and samr21 include files? Just so you know, I have and I don't want to take responsibility to use samd21 for samr21 or vice versa. There are differences that I don't know enough about. For example instance/evsys.h or instance/nvmctrl.h.
    You are right that the samd21 and samr21 files are mostly the same. But with an emphasis on "mostly".

Well, the samd21 evsys.h includes event descriptions for the second
analog comparator. The samr21 file doesnt have those.
The specs of both AC peripherals are identical, so either the samr21
specs are wrong or it's CMSIS header is.

So unless someone down the line will use the event subsystem of the
second AC within RIOT, and the spec proves wrong (compared to the
cmsis header), there will be a problem.

For nvmctrl, the samd21 hard-codes 64k flashsize, which is clearly wrong
for at least a part of the chips, as they ship with 32k/64k/128k/256k,
meaning that without the (blobbed) special handling of those defines
from the CMSIS library code, the define is useless anyways.

  1. I want a better explanation why we need to undef LITTLE_ENDIAN. The comment states it is needed for newlib. But I want to see for myself how that becomes a conflict. What do I need to build?

For me, everything that includes both the cmsis header and newlib
headers breaks with a redefinition error (e.g., 'examples/default').
That was worked around by @OlegHahm in the headers you're replacing in
your PR.

In general I want a clear documented description of the process of adding / changing the CMSIS files. The samr21 originated from ASF 3.17, or 3.18, or 3.19. It is not good that we don't know.

And the process must be repeatable. Atmel maintains these files. The latest version is 3.32, maybe we want take an update at some point. I've spend lots of hours already to figure out what we have today (which ASF version? What changes were applied? Renamed files. Etc.). Let's not have to waste our precious time again.

Ok, I see now that changing the cmsis files or structure is troublesome.
Will close this. Let's ontinue in RIOT-OS#5743.

@kaspar030 kaspar030 closed this Aug 15, 2016
@kaspar030 kaspar030 deleted the pr5743 branch February 7, 2017 22:13
keestux pushed a commit that referenced this pull request Dec 24, 2018
* drivers/ccs811: fix types in debug messages

* drivers/driver_ccs811_full: fix unused variable build error
keestux added a commit that referenced this pull request Jul 7, 2019
The evtimer_msg test is expanded to also delete entries.

Furthermore the messages that are printed should now show
numbers that are very close (if not equal). Something like
this:
At    740 ms received msg 0: "RIOT-OS#2 supposed to be 740"
At   1081 ms received msg 1: "#0 supposed to be 1081"
At   1581 ms received msg 2: "#1 supposed to be 1581"
At   4035 ms received msg 3: "RIOT-OS#3 supposed to be 4035"

The function evtimer_print is also called to show the
intermediate status of evtimer entries.
keestux added a commit that referenced this pull request Jul 9, 2019
The evtimer_msg test is expanded to also delete entries.

Furthermore the messages that are printed should now show
numbers that are very close (if not equal). Something like
this:
At    740 ms received msg 0: "RIOT-OS#2 supposed to be 740"
At   1081 ms received msg 1: "#0 supposed to be 1081"
At   1581 ms received msg 2: "#1 supposed to be 1581"
At   4035 ms received msg 3: "RIOT-OS#3 supposed to be 4035"

The function evtimer_print is also called to show the
intermediate status of evtimer entries.
keestux added a commit that referenced this pull request Jul 15, 2019
The evtimer_msg test is expanded to also delete entries.

Furthermore the messages that are printed should now show
numbers that are very close (if not equal). Something like
this:
At    740 ms received msg 0: "RIOT-OS#2 supposed to be 740"
At   1081 ms received msg 1: "#0 supposed to be 1081"
At   1581 ms received msg 2: "#1 supposed to be 1581"
At   4035 ms received msg 3: "RIOT-OS#3 supposed to be 4035"

The function evtimer_print is also called to show the
intermediate status of evtimer entries.
keestux added a commit that referenced this pull request Jul 16, 2019
The evtimer_msg test is expanded to also delete entries.

Furthermore the messages that are printed should now show
numbers that are very close (if not equal). Something like
this:
At    740 ms received msg 0: "RIOT-OS#2 supposed to be 740"
At   1081 ms received msg 1: "#0 supposed to be 1081"
At   1581 ms received msg 2: "#1 supposed to be 1581"
At   4035 ms received msg 3: "RIOT-OS#3 supposed to be 4035"

The function evtimer_print is also called to show the
intermediate status of evtimer entries.
keestux added a commit that referenced this pull request Jul 17, 2019
The evtimer_msg test is expanded to also delete entries.

Furthermore the messages that are printed should now show
numbers that are very close (if not equal). Something like
this:
At    740 ms received msg 0: "RIOT-OS#2 supposed to be 740"
At   1081 ms received msg 1: "#0 supposed to be 1081"
At   1581 ms received msg 2: "#1 supposed to be 1581"
At   4035 ms received msg 3: "RIOT-OS#3 supposed to be 4035"

The function evtimer_print is also called to show the
intermediate status of evtimer entries.
keestux pushed a commit that referenced this pull request Sep 3, 2019
The test randomly fails on `native` due to timers being not accurate but
it cannot be otherwise. So better disable it than raising fake errors.

    main(): This is RIOT! (Version: buildtest)
    Testing generic evtimer
    This should list 2 items
    ev #1 offset=1000
    ev RIOT-OS#2 offset=500
    This should list 4 items
    ev #1 offset=659
    ev RIOT-OS#2 offset=341
    ev RIOT-OS#3 offset=500
    ev RIOT-OS#4 offset=2454
    Are the reception times of all 4 msgs close to the supposed values?
    At    662 ms received msg 0: "RIOT-OS#2 supposed to be 659"
    At   1009 ms received msg 1: "#0 supposed to be 1000"
    At   1511 ms received msg 2: "#1 supposed to be 1500"

    Traceback (most recent call last):
      File "/tmp/dwq.0.3125418833043728/ef3af88c4b3615788b164464a437df5c/tests/evtimer_msg/tests/01-run.py", line 33, in <module>
        sys.exit(run(testfunc))
      File "/tmp/dwq.0.3125418833043728/ef3af88c4b3615788b164464a437df5c/dist/pythonlibs/testrunner/__init__.py", line 29, in run
        testfunc(child)
      File "/tmp/dwq.0.3125418833043728/ef3af88c4b3615788b164464a437df5c/tests/evtimer_msg/tests/01-run.py", line 26, in testfunc
        assert(actual in range(expected - ACCEPTED_ERROR, expected + ACCEPTED_ERROR))
    AssertionError
keestux pushed a commit that referenced this pull request May 24, 2020
The ROM size is encoded in the part number of the Atmel SAM chips.
RAM size is not encoded directly, so get it by parsing the chip's vendor file.

The file remains in the page cache for the compiler to use, so the overhead
should be minimal:

on master:

  Benchmark #1: make BOARD=samr21-xpro -j
    Time (mean ± σ):     527.9 ms ±   4.9 ms    [User: 503.1 ms, System: 69.6 ms]
    Range (min … max):   519.7 ms … 537.2 ms    10 runs

with this patch:

  Benchmark #1: make BOARD=samr21-xpro -j
    Time (mean ± σ):     535.6 ms ±   4.0 ms    [User: 507.6 ms, System: 75.1 ms]
    Range (min … max):   530.6 ms … 542.0 ms    10 runs
keestux pushed a commit that referenced this pull request Sep 5, 2020
Coverty scan found this:

> CID 298279 (#1 of 1): Out-of-bounds read (OVERRUN)
> 21. overrun-local: Overrunning array of 16 bytes at byte offset 64 by dereferencing pointer

The original intention was probably to advance the destination pointer by 4 bytes, not
4 * the destination type size.
keestux pushed a commit that referenced this pull request Sep 5, 2020
Coverty scan found this:

> CID 298295 (#1 of 1): Operands don't affect result (CONSTANT_EXPRESSION_RESULT) result_independent_of_operands:
> (ipv6_hdr_get_fl(ipv6_hdr) & 255) >> 8 is 0 regardless of the values of its operands.

Looking at the code, this appears to be a copy & paste error from the previous line.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants