cpu/samd21: add rtc periph implementation #2197

biboc · 2014-12-15T16:20:15Z

Implementation of RTC for the SAMR21 board
It uses GCLK2 with OSC32K

LudwigKnuepfer · 2014-12-15T16:46:58Z

cpu/samd21/periph/rtc.c

+//#define REFERENCE_YEAR      2000
+static uint16_t reference_year = 2000;
+
+void rtc_init(void){


coding conventions (opening curly brace on new line)

This goes for all functions..

LudwigKnuepfer · 2014-12-15T17:03:36Z

I don't have a device for testing at hand - did you check with #2061 ?

biboc · 2014-12-15T17:12:46Z

Nope, I didn't, why don't you merge it to main repo if it works?

LudwigKnuepfer · 2014-12-15T17:17:31Z

Long story short: It wasn't ready until half an hour ago and is now waiting for squashing and review.

biboc · 2014-12-15T17:21:23Z

Ok :-)
I will test my code when #2061 will be merged

biboc · 2014-12-17T08:28:37Z

@LudwigOrtmann
I tried RTC with your shell command:
Here is what I've got:

rtc init

rtc gettime
3900-02-01 00:00:12

rtc gettime
3900-02-01 00:00:23

rtc getalarm
3900-02-01 00:00:55

rtc settime 2014-12-17 08:56:05
rtc: error setting time

My registers are normal ( RTC.MODE2.CLOCK)

Ok I found what you did in _print_time()
time->tm_year + 1900, time->tm_mon + 1, time->tm_mday, time->tm_hour, time->tm_min, time->tm_sec
Why +1900 and +1? Is that define somewhere? I think each RTC should manage the date itself and you shouldn't add this because it mainly depends on how each RTC is implemented. What do you think?

Btw, I didn't understand why the settime didn't work.

EDIT: I understand now, it's because when you parse the struct time, you remove 1900 and 1 so my rtc_settime return -1

biboc · 2014-12-17T08:37:51Z

Either I change my file to adapt your shell or I leave my file like this. The thing is if someone doesn't use the shell, he has to to copy your code to parse the time as you do. Leaving my code, he would have nothing to do apart from gettime :-)

Let me know what you think about that.

LudwigKnuepfer · 2014-12-17T08:50:56Z

It's really not "my code" but the standard defined for that data type.
This has also been documented from the start: http://riot-os.org/api/group__driver__periph__rtc.html#gaa2d60ad372a3712b875a15e07c517843

biboc · 2014-12-17T09:25:27Z

Ok I see what you mean.
The structure tm is defined like this:
int tm_sec seconds [0,61]
int tm_min minutes [0,59]
int tm_hour hour [0,23]
int tm_mday day of month [1,31]
int tm_mon month of year [0,11]
int tm_year years since 1900
int tm_wday day of week [0,6](Sunday = 0)
int tm_yday day of year [0,365]
int tm_isdst daylight savings flag

So I should return the year from 1900 and a month starting at 0 not one.
Alright, I will do my change

LudwigKnuepfer · 2014-12-17T09:33:40Z

Great!

biboc · 2014-12-17T09:41:42Z

One last question:
Should I complete the entire structure?
For the moment, I don't set:
int tm_wday day of week 0,6
int tm_yday day of year [0,365]
int tm_isdst daylight savings flag

LudwigKnuepfer · 2014-12-17T09:48:07Z

I don't think any platform does this at this time.

biboc · 2014-12-17T09:52:04Z

Good, I made the changes. I only set my default year to 2000 (1900+100) because SAMR21 allows only 64years as I explained at the end of the file.
Anyway, this PR works with RTC shell commands :-)

LudwigKnuepfer · 2014-12-17T10:06:05Z

Feel free to squash.

LudwigKnuepfer · 2014-12-17T10:18:19Z

cpu/samd21/periph/rtc.c

@@ -0,0 +1,201 @@
+/*
+ * Copyright (C) 2014 CLENET Baptiste


I don't really care, but as far as I know, all other files use a "Forename Surname" pattern.

Sorry, just saw this now..

biboc · 2014-12-17T10:40:13Z

Changes done and squashed

LudwigKnuepfer · 2014-12-17T11:22:17Z

boards/samr21-xpro/include/periph_conf.h

+#define I2C_0_PINS          (PORT_PA16 | PORT_PA17)
+/* Default Clock Source on reset OSC8M - 8MHz */
+#define I2C_0_REF_F         (8000000UL)
+


I think there's a closing Doxygen parenthesis missing...

In any case it would be nice to keep this as a separate commit.

hm, this section is already part of master.

Yep, true, I will edit that in a bit
Le 17 déc. 2014 12:59, "Thomas Eichinger" notifications@github.com a
écrit :

In boards/samr21-xpro/include/periph_conf.h
#2197 (diff):

+#define I2C_1_EN 0
+#define I2C_2_EN 0
+#define I2C_3_EN 0
+#define I2C_IRQ_PRIO 1
+
+#define I2C_0_DEV SERCOM3->I2CM
+#define I2C_0_IRQ SERCOM3_IRQn
+#define I2C_0_ISR isr_sercom3
+/* I2C 0 pin configuration /
+#define I2C_0_PORT (PORT->Group[0])
+#define I2C_SDA PIN_PA16
+#define I2C_SCL PIN_PA17
+#define I2C_0_PINS (PORT_PA16 | PORT_PA17)
+/ Default Clock Source on reset OSC8M - 8MHz */
+#define I2C_0_REF_F (8000000UL)
+

hm, this section is already part of master
https://github.com/RIOT-OS/RIOT/blob/master/boards/samr21-xpro/include/periph_conf.h#L133
.

—
Reply to this email directly or view it on GitHub
https://github.com/RIOT-OS/RIOT/pull/2197/files#r21967452.

LudwigKnuepfer · 2014-12-17T11:22:50Z

boards/samr21-xpro/include/periph_conf.h

 /* GPIO channel 2 config */
 #define GPIO_2_DEV          PORT->Group[0]
 #define GPIO_2_PIN          PIN_PA15
-#define GPIO_2_EXTINT		15
+#define GPIO_2_EXTINT       15


I'd also move these into a separate commit.

LudwigKnuepfer · 2014-12-17T11:23:33Z

Sorry for torturing you about the commits ;)

biboc · 2014-12-17T13:10:14Z

Done

LudwigKnuepfer · 2014-12-17T15:42:26Z

Where are the commits?

LudwigKnuepfer · 2014-12-17T15:42:46Z

(They can stay part of this PR..)

biboc · 2014-12-17T15:59:41Z

These comits were not part of this PR.

LudwigKnuepfer · 2014-12-17T16:01:47Z

But aren't they needed? Which PR does contain them?

LudwigKnuepfer · 2014-12-17T16:02:31Z

boards/samr21-xpro/Makefile.features

@@ -1 +1 @@
-FEATURES_PROVIDED += periph_gpio cpp periph_uart periph_timer
+FEATURES_PROVIDED += periph_gpio cpp periph_uart periph_timer periph_i2c periph_rtc


In case the i2c changes are part of an external PR, the periph_i2c here should probably be removed too.

LudwigKnuepfer · 2014-12-17T16:10:55Z

Reassigning to @thomaseichinger for testing.

biboc · 2014-12-17T16:45:14Z

Rebase and squash done.
It was from my previous I2C PR.

(And also because I'm bad at using GitHub :-) )

LudwigKnuepfer · 2014-12-17T16:47:04Z

I guess you need to rebase again.

biboc · 2014-12-17T16:48:11Z

Nope, it should be fine now. (one commit, only my changes)
Is that fine?

LudwigKnuepfer · 2014-12-17T16:59:45Z

The button on this page says there's conflicts so it can't be merged. Maybe all you need to do is bring your own master up-to-date?

LudwigKnuepfer · 2014-12-17T17:03:05Z

boards/samr21-xpro/Makefile.features

@@ -1 +1 @@
-FEATURES_PROVIDED += periph_gpio cpp periph_uart periph_timer
+FEATURES_PROVIDED += periph_gpio cpp periph_uart periph_timer periph_rtc


Sorry, there should be a periph_i2c here, probably it only was indicated as a change because your own master wasn't up to date. I wasn't aware of that PR.

To explain:
This is a problem in the GitHub interface - it shows diffs against the master of the fork (bapclenet/RIOT.git/master in this case), not against the branch it is going to be merged into (in this case RIOT-OS/RIOT.git/master).

So, what you need to do: update your master, rebase on it, push both, your master and this branch.

My repo is "dirty" so I'm going to delete my fork and redo the pull request after a clean fork.
Is that fine?

(Because it tells that it's up to date and rebase is too long, wait for your agreement before doing that)

That would not have been necessary - don't hesitate to ask for help with git in the future ;)

LudwigKnuepfer reviewed Dec 15, 2014
View reviewed changes

LudwigKnuepfer added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: drivers Area: Device drivers labels Dec 15, 2014

LudwigKnuepfer added the State: waiting for other PR State: The PR requires another PR to be merged first label Dec 15, 2014

LudwigKnuepfer removed the State: waiting for other PR State: The PR requires another PR to be merged first label Dec 17, 2014

LudwigKnuepfer added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Dec 17, 2014

LudwigKnuepfer reviewed Dec 17, 2014
View reviewed changes

thomaseichinger mentioned this pull request Dec 17, 2014

board:cpu: RIOT port for the atsamr21xpro board #1646

Closed

21 tasks

LudwigKnuepfer removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Dec 17, 2014

LudwigKnuepfer reviewed Dec 17, 2014
View reviewed changes

LudwigKnuepfer self-assigned this Dec 17, 2014

LudwigKnuepfer assigned thomaseichinger and unassigned LudwigKnuepfer Dec 17, 2014

cpu/samd21: add rtc periph implementation

eec9ddf

LudwigKnuepfer reviewed Dec 17, 2014
View reviewed changes

biboc closed this Dec 18, 2014

		@@ -1 +1 @@
		FEATURES_PROVIDED += periph_gpio cpp periph_uart periph_timer
		FEATURES_PROVIDED += periph_gpio cpp periph_uart periph_timer periph_i2c periph_rtc

cpu/samd21: add rtc periph implementation #2197

cpu/samd21: add rtc periph implementation #2197

Conversation

biboc commented Dec 15, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LudwigKnuepfer commented Dec 15, 2014

biboc commented Dec 15, 2014

LudwigKnuepfer commented Dec 15, 2014

biboc commented Dec 15, 2014

biboc commented Dec 17, 2014

biboc commented Dec 17, 2014

LudwigKnuepfer commented Dec 17, 2014

biboc commented Dec 17, 2014

LudwigKnuepfer commented Dec 17, 2014

biboc commented Dec 17, 2014

LudwigKnuepfer commented Dec 17, 2014

biboc commented Dec 17, 2014

LudwigKnuepfer commented Dec 17, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

biboc commented Dec 17, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LudwigKnuepfer commented Dec 17, 2014

biboc commented Dec 17, 2014

LudwigKnuepfer commented Dec 17, 2014

LudwigKnuepfer commented Dec 17, 2014

biboc commented Dec 17, 2014

LudwigKnuepfer commented Dec 17, 2014

Choose a reason for hiding this comment

LudwigKnuepfer commented Dec 17, 2014

biboc commented Dec 17, 2014

LudwigKnuepfer commented Dec 17, 2014

biboc commented Dec 17, 2014

LudwigKnuepfer commented Dec 17, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment