Adds the atmega1284p CPU and the first version of the INGA board to RIOT by roberthartung · Pull Request #7604 · RIOT-OS/RIOT

roberthartung · 2017-09-14T13:19:37Z

No description provided.

lebrush

Looks good. Of course I don't have the hardware to test it. I suggest the following actions:

run uncrustify (many things not RIOTish, like 2 space identation)
separate in 3 PR (CPU, Board and Tool) for easier review

Is this board available to the public? In the sense can someone buy it somewhere?

lebrush · 2017-09-15T09:40:06Z

boards/inga_common/board.c

+ * General Public License v2.1. See the file LICENSE in the top level
+ * directory for more details.
+ */
+


Missing doxygen tags (also in other files)

boards/inga_common/include/board_common.h

boards/inga_common/include/periph_conf_common.h

boards/inga_common/include/board_common.h

cpu/atmega1284p/Makefile.include

cpu/atmega1284p/cpu.c

cpu/atmega_common/periph/spi.c

roberthartung · 2017-09-15T11:37:46Z

It's a public, open source platform ( see https://www.ibr.cs.tu-bs.de/projects/inga/ ). Some bought them off of us and some are spread around some other universities as well.

roberthartung · 2017-09-15T11:50:21Z

@lebrush Thanks for the review. Most changes are done. I am about to add the other versions of INGA as well. Should I will further split the inga-comm/inga-red platform commit into one for each inga-common, inga-red, ...

lebrush · 2017-09-15T13:49:59Z

Personally I would not mind to have all the boards in a PR, but the philosophy so far is to split PR into independent groups :-)

roberthartung · 2017-09-15T14:35:44Z

@lebrush was talking about commits here, not PRs ;-) What do you mean by "independent groups"?

lebrush · 2017-09-15T14:38:18Z

I don't know... guess I was thinking about something else at the same time of writing. What I meant is 1 PR per feature/fix and being a board a new feature.

roberthartung · 2017-09-25T13:52:09Z

This should be merged after #7610

kYc0o

I have some minor comments.

I see also that some periph_conf.h files are empty, are they really needed?

boards/inga_common/include/board_common.h

kYc0o · 2017-09-26T12:05:30Z

boards/inga_green/include/board.h

+#include "periph_conf.h"
+/* For GPIO_IN */
+#include "periph/gpio.h"
+/* from inga_common */


This comments can be removed.

kYc0o · 2017-09-26T12:06:31Z

boards/inga_white/include/board.h

+#include "periph_conf.h"
+/* For GPIO_IN */
+#include "periph/gpio.h"
+/* from inga_common */


roberthartung · 2017-09-26T12:43:09Z

@kYc0o done. We still need to wait for the pcint implementation to be merged before!

boards: adds INGA

PeterKietzmann · 2018-07-31T19:34:46Z

Without checking any details: Is this PR still waiting for others?

maribu · 2019-03-05T12:01:30Z

The ATmega1284P CPU is supported by RIOT by now and seems to work, as it is used by the Mega1284P-Xplained board.

I think this PR could (and should) be updated to add support for the INGA board by using the ATmege1284P implementation merged into RIOT in the meantime. (Are there any missing features in the ATmega1284P? If so, I would suggest to tackle them as separate PR(s). Also: The RTC support seems to be on the right track for merging right now #8842)

roberthartung · 2019-03-05T13:03:34Z

@maribu We are still waiting on comments for #7610 and respectively #9159. I will create a new PR for a new solution for Pin Change Interrupts asap, so we can finally merge INGA to RIOT.

maribu · 2019-03-05T19:30:50Z

@roberthartung: Thanks for pointing that out. I started to read the threads and decided to dive into it tomorrow with a fresh mind...

However: It seems to me that support for the INGA could be added without the AVR implementation of gpio_init_init() supporting GPIO_BOTH for now. While you do have a use case for that for your specific board, I think that code-wise they are not related. (I may be wrong about that, but I guess adding support for interrupts on both flanks for the AVR platform will not touch the code of the individual AVR boards. I guess some code will might end up in boards/common/atmega, but the individual boards will likely not be changed.) So with that in mind: If you want to add INGA support before those issues resolved, I'm happy to review and help.

roberthartung · 2019-03-06T08:12:18Z

@maribu The PRs are not about having support for GPIO_BOTH. It is about a general support for pin change interrupts, which simply does not exist by now! INGA needs this for the radio to work. I will create a PR for our pin change solution and give you a mention to hopefully finally have a simple and good solution to make it work.

maribu · 2019-03-06T08:29:16Z

By "pin changed interrupt" you mean an interrupt is generated when any pin of a port is changed, in contrast to GPIO_BOTH, which generates an interrupt when a specific pin is changed?

roberthartung · 2019-03-06T08:32:15Z

@maribu There are two types of interrupts on atmel MCUs: External Interrupts and Pin Change Interrupts. External Interrupts are only available on very few pins and have dedicated interrupts (ISRs). Pin Change Interrupts can occur on any GPIO, but share one interrupt per port. GPIO_BOTH simply sets the flank (rising or falling).

roberthartung · 2019-03-06T08:48:32Z

Waits for #11114 @maribu

roberthartung · 2019-03-11T12:41:14Z

Once #11122 is merged, we can rebase this on the latest master. Finally!

benpicco · 2019-09-10T19:12:06Z

Well, #11122 is merged now

benpicco · 2019-11-19T15:15:56Z

Adding new Atmega boards should be easier now, maybe give this another try?

maribu · 2019-11-20T21:06:44Z

cpu/atmega1284p/periph/rtt.c

+rtt_cb_t rtt_next_cb;
+volatile int cnt = 0;
+
+void rtt_init(void) {


You could split out periph/rtt as its own PR. Without having looked into it closely, I guess this should also be possible to add to cpu/atmega_common instead.

maribu · 2019-11-20T21:14:48Z

Adding new Atmega boards should be easier now,

Definitely! The CPU is now there anyway and you could reuse a lot in boards/common/atmega, likely out of the box.

I'm not quite convinced that the RIOT repo however is the right place to add the source of the inga_tool. I'd personally just provide a link to its repo in the board doc and say it is a requirement that the tool installed and can be found somewhere in $PATH. I doubt non-INGA board users would contribute to it and I guess it will be quite hard to get reviews for the tool.

tcschmidt · 2020-04-01T01:02:47Z

@roberthartung shouldn't we progress here?

roberthartung · 2020-04-01T06:19:00Z

@tcschmidt @maribu absolutely! As soon as there is some freetime due to the current situation, I will work on that. I am also visiting the institute today, so I might could bring some hardware home.

In addition my student is already working on splitting the power management for cpu/atmega1284p as an independent PR. So we finally have pm_layered for it!

roberthartung · 2020-04-01T06:20:34Z

Adding new Atmega boards should be easier now,

Definitely! The CPU is now there anyway and you could reuse a lot in boards/common/atmega, likely out of the box.

I'm not quite convinced that the RIOT repo however is the right place to add the source of the inga_tool. I'd personally just provide a link to its repo in the board doc and say it is a requirement that the tool installed and can be found somewhere in $PATH. I doubt non-INGA board users would contribute to it and I guess it will be quite hard to get reviews for the tool.

@maribu The inga_tool is interacting with the FTDI to use some pins to reset the circuit. I could imagine that some boards might require such a tool in the future. Maybe renaming it to ftdi_tool would be okay, if it would be a general solution?

benpicco · 2020-04-01T08:20:29Z

You can just download the tool on-demand like bossa, cc2538-bsl or edbg.
This way we don't have to maintain a fork since that tool might also be used outside RIOT. (IIRC you also have Contiki on the inga boards)

maribu · 2020-04-01T08:51:18Z

The inga_tool is interacting with the FTDI to use some pins to reset the circuit.

Is a tool for that really needed? E.g. the MSB-A2 board also has an FTDI chip and uses the DTR pin to reset the board. This can be handled by just passing the right flags to the terminal.

stale · 2020-10-03T15:08:50Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

roberthartung force-pushed the boards/inga branch from 577de27 to b562d9a Compare September 15, 2017 08:59

lebrush suggested changes Sep 15, 2017

View reviewed changes

lebrush added the Platform: AVR Platform: This PR/issue effects AVR-based platforms label Sep 15, 2017

roberthartung mentioned this pull request Sep 15, 2017

atmega/periph: gpio.c does not implemented pin change interrupts. #7609

Closed

miri64 assigned lebrush Sep 19, 2017

roberthartung force-pushed the boards/inga branch from b562d9a to c662004 Compare September 25, 2017 08:18

kYc0o requested changes Sep 26, 2017

View reviewed changes

lebrush added the State: waiting for other PR State: The PR requires another PR to be merged first label Oct 6, 2017

roberthartung force-pushed the boards/inga branch 2 times, most recently from 74cee76 to 7cfe6a3 Compare December 4, 2017 08:38

roberthartung force-pushed the boards/inga branch from f5cb1d6 to fb13ba1 Compare January 10, 2018 08:44

roberthartung added 9 commits January 11, 2018 10:24

dist/tools: adds inga_tool

53b33e9

cpu: adds atmega1284p

6a927cf

boards: adds INGA

boards/inga: adds all four versions

8c81727

fixup! adds missing features

ba108f3

boards/inga_common: moved to boards/common/inga

9e9a16d

fixup! adapts Makefiles

bd187a8

fixup! stack sizes

493797e

adjust stack sizes

6096506

adds simplistic RTT implementation

ad50e08

roberthartung force-pushed the boards/inga branch from fb13ba1 to ad50e08 Compare January 11, 2018 09:25

ZetaR60 mentioned this pull request Mar 16, 2018

Add ATmega1284P and BLU-WMN board. #8786

Closed

ZetaR60 mentioned this pull request Mar 28, 2018

cpu/atmega_common: RTT and RTC support #8842

Closed

TorbenPetersen mentioned this pull request Jun 11, 2018

gpio.h/cb_mux API change and interim compatibilty #9159

Closed

miri64 added the State: waiting for other PR label Oct 5, 2018

miri64 removed the State: waiting for other PR label Oct 12, 2018

maribu reviewed Nov 20, 2019

View reviewed changes

benpicco mentioned this pull request Nov 26, 2019

cpu/atmega_common: RTT support #12815

Merged

benpicco removed the State: waiting for other PR State: The PR requires another PR to be merged first label Feb 12, 2020

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Oct 3, 2020

stale bot closed this Nov 5, 2020

Conversation

roberthartung commented Sep 14, 2017

Uh oh!

lebrush left a comment

Choose a reason for hiding this comment

Uh oh!

lebrush Sep 15, 2017

Choose a reason for hiding this comment

Uh oh!

roberthartung Sep 15, 2017

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

roberthartung commented Sep 15, 2017

Uh oh!

roberthartung commented Sep 15, 2017

Uh oh!

lebrush commented Sep 15, 2017

Uh oh!

roberthartung commented Sep 15, 2017

Uh oh!

lebrush commented Sep 15, 2017

Uh oh!

roberthartung commented Sep 25, 2017

Uh oh!

kYc0o left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kYc0o Sep 26, 2017

Choose a reason for hiding this comment

Uh oh!

kYc0o Sep 26, 2017

Choose a reason for hiding this comment

Uh oh!

roberthartung commented Sep 26, 2017

Uh oh!

PeterKietzmann commented Jul 31, 2018

Uh oh!

maribu commented Mar 5, 2019

Uh oh!

roberthartung commented Mar 5, 2019

Uh oh!

maribu commented Mar 5, 2019

Uh oh!

roberthartung commented Mar 6, 2019

Uh oh!

maribu commented Mar 6, 2019

Uh oh!

roberthartung commented Mar 6, 2019

Uh oh!

roberthartung commented Mar 6, 2019

Uh oh!

roberthartung commented Mar 11, 2019

Uh oh!

benpicco commented Sep 10, 2019

Uh oh!

benpicco commented Nov 19, 2019

Uh oh!

maribu Nov 20, 2019

Choose a reason for hiding this comment

Uh oh!

maribu commented Nov 20, 2019

Uh oh!

tcschmidt commented Apr 1, 2020

Uh oh!

roberthartung commented Apr 1, 2020

Uh oh!

roberthartung commented Apr 1, 2020

Uh oh!

benpicco commented Apr 1, 2020

Uh oh!

maribu commented Apr 1, 2020

Uh oh!

stale bot commented Oct 3, 2020