Skip to content

kinetis/rtc: move rtt-to-rtc wrapper to drivers/periph_common#4572

Closed
basilfx wants to merge 1 commit intoRIOT-OS:masterfrom
basilfx:bugfix/common_rtc
Closed

kinetis/rtc: move rtt-to-rtc wrapper to drivers/periph_common#4572
basilfx wants to merge 1 commit intoRIOT-OS:masterfrom
basilfx:bugfix/common_rtc

Conversation

@basilfx
Copy link
Copy Markdown
Member

@basilfx basilfx commented Jan 4, 2016

This should fix #4565.

There is no change in the code. I only moved the file, changed docs and added the defines (similar to the common methods for SPI).

I don't own a kinetis board. However, I have ensured that tests/periph_rtc would still compile (using frdm-k64f). Also, I tested this code on my local EFM32 port, which works too.

@kaspar030 kaspar030 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: drivers Area: Device drivers labels Jan 4, 2016
@kaspar030 kaspar030 self-assigned this Jan 4, 2016
@haukepetersen
Copy link
Copy Markdown
Contributor

Hmm, I don't really like this PR. Wrapping an RTT to fill the RTC interface kind of blows the concept of the periph drivers: they are meant to abstract the MCU hardware. If the hardware does not provide some peripheral, it should just not implement the periph driver for the peripheral. I should be then up to the higher level modules, do work with what is there. E.g. the xtimer should take care to select the RTC or RTT, depending on what the CPU provides.

@kaspar030
Copy link
Copy Markdown
Contributor

@haukepetersen Almost all MCUs support RTT (counting) mode. And the RTT interface just sucks (see sizeof(struct tm) and the codesize + deps of the corresponding conversion functions).

That's why I proposed to just move it to sys/, so that if people want, they can use "struct tm", but our general recommendation is to use xtimer (+ maybe some wall clock API based on that / RTT).

@basilfx
Copy link
Copy Markdown
Member Author

basilfx commented Jan 6, 2016

@haukepetersen @kaspar030 I won't join that discussion, I just needed an RTC driver :-)

@jnohlgard
Copy link
Copy Markdown
Member

Related: #4758

@miri64 miri64 modified the milestone: Release 2016.07 Mar 29, 2016
@basilfx
Copy link
Copy Markdown
Member Author

basilfx commented Apr 1, 2016

Just realized that I still had this PR open.

I've concluded that this fix isn't common enough. For instance, it assumes times are 32 bits, which wasn't the case for the EFM32 I am doing (only 24 bits). This means that I have to keep track of the overflows to support times and alarms that exceed the 'current' interval (apart from that, I implemented both RTT and RTC but not via this wrapper)

Because the discussion of whether this should be fixed in the driver is not concluded, I abandon this PR.

@basilfx basilfx closed this Apr 1, 2016
@basilfx basilfx deleted the bugfix/common_rtc branch April 4, 2016 05:37
@jnohlgard jnohlgard mentioned this pull request Aug 1, 2016
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: drivers Area: Device drivers 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.

kinetis/rtc: move to periph_common?

5 participants