Skip to content

cpu/atmega*: unify common code for atmega CPUs#5529

Merged
aabadie merged 3 commits intoRIOT-OS:masterfrom
kYc0o:atmega_common_unify
Jun 22, 2016
Merged

cpu/atmega*: unify common code for atmega CPUs#5529
aabadie merged 3 commits intoRIOT-OS:masterfrom
kYc0o:atmega_common_unify

Conversation

@kYc0o
Copy link
Copy Markdown
Contributor

@kYc0o kYc0o commented Jun 7, 2016

The atmega family has very few differences between CPUs.

This PR aims to unify the common code to ease the portability to other atmega CPUs.

All drivers were tested on an arduino-mega2560.

It also includes a reworked version of the UART implementation, based on #5026.

@kYc0o kYc0o added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Platform: AVR Platform: This PR/issue effects AVR-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Jun 7, 2016
@kYc0o kYc0o added this to the Release 2016.07 milestone Jun 7, 2016
This was referenced Jun 7, 2016
Merged
Merged
@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jun 8, 2016

@aabadie can you take a look?

@kYc0o kYc0o force-pushed the atmega_common_unify branch from aa6eefa to c87e641 Compare June 9, 2016 14:53
@kYc0o kYc0o added the State: waiting for other PR State: The PR requires another PR to be merged first label Jun 9, 2016
@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jun 9, 2016

just cleaned and make it depend on #5537

@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jun 16, 2016

@aabadie this one it's next :P

@kYc0o kYc0o mentioned this pull request Jun 16, 2016
Closed
@PeterKietzmann PeterKietzmann removed the State: waiting for other PR State: The PR requires another PR to be merged first label Jun 20, 2016
@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Jun 20, 2016

#5537 was merged. PR shouldn't be waiting for other PR any more. Is it still WIP?

@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jun 20, 2016

Yes it shouldn't wait. It's wip because I was aiming to also unify the makefiles. I can try to do that today and integrate it asap.

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Jun 20, 2016

No stress from my side. Just wanted to clarify.

@mali
Copy link
Copy Markdown
Contributor

mali commented Jun 21, 2016

No stress, but I'm waiting for it to rebase my PR #5529 upon this one ;-)

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Jun 21, 2016

Guess you meant #5451

@mali
Copy link
Copy Markdown
Contributor

mali commented Jun 21, 2016

Ooops, indeed ...

@kYc0o kYc0o force-pushed the atmega_common_unify branch from c87e641 to a0fc9e7 Compare June 21, 2016 15:04
@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jun 21, 2016

Actually the Makefile refactoring is for the boards based on atmega CPUs, so no needed for this PR. Thus, this is ready.

@kYc0o kYc0o removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Jun 21, 2016
aabadie
aabadie reviewed Jun 21, 2016
View reviewed changes
cpu/atmega_common/include/atmega_regs_common.h
@@ -1,23 +1,25 @@
/*
* Copyright (C) 2016 Freie Universität Berlin
* 2016 INRIA
Copy link
Copy Markdown
Contributor

@aabadie aabadie Jun 21, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

For consistency, one should write Inria. Ref, see the Reminder section. There are a few others.

@aabadie aabadie added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Jun 21, 2016
@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jun 21, 2016 •
edited
Loading

ACK, once commits are squashed.

@kYc0o kYc0o force-pushed the atmega_common_unify branch from 094ccb0 to e4b2e51 Compare June 21, 2016 16:20
@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jun 21, 2016

squashed

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jun 21, 2016

ACK

@aabadie aabadie added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable labels Jun 21, 2016
@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jun 22, 2016

@kYc0o, Murdock is unhappy, can you check ? Details link points to a blank page, is this normal ?

@kaspar030 kaspar030 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 22, 2016
@kaspar030
Copy link
Copy Markdown
Contributor

kaspar030 commented Jun 22, 2016

Details link points to a blank page, is this normal ?

nope, should be fixed now. let's see.

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jun 22, 2016

should be fixed now. let's see.

Thanks @kaspar030 ... but still blank ;)

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jun 22, 2016

but still blank ;)

Ah no, works after a refresh.

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Jun 22, 2016

cpu/atmega_common/periph/timer.c:188: error (arrayIndexOutOfBounds): Array 'ctx[1]' accessed at index 1, which is out of bounds.

@PeterKietzmann PeterKietzmann added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 22, 2016
@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jun 22, 2016

@kYc0o, Murdock is still complaining:

cpu/atmega_common/periph/timer.c:188: error (arrayIndexOutOfBounds): Array 'ctx[1]' accessed at index 1, which is out of bounds.
cpu/atmega_common/periph/timer.c:189: error (arrayIndexOutOfBounds): Array 'ctx[1]' accessed at index 1, which is out of bounds.
cpu/atmega_common/periph/timer.c:188: error (arrayIndexOutOfBounds): Array 'ctx[1]' accessed at index 2, which is out of bounds.
cpu/atmega_common/periph/timer.c:189: error (arrayIndexOutOfBounds): Array 'ctx[1]' accessed at index 2, which is out of bounds.

@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jun 22, 2016

Yes I know, I'm trying to fix it.

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jun 22, 2016

Murdock is happy now. Thanks @kYc0o

(Can I click the green button ? 😉 )

@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jun 22, 2016

Murdock is happy now, but I think it needs sqaushing is it?

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jun 22, 2016

I think it needs sqaushing is it?

Well yes. You know how to do it now ?

@kYc0o kYc0o force-pushed the atmega_common_unify branch from 5027350 to abba25e Compare June 22, 2016 12:26
@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jun 22, 2016

@aabadie can you test the timers again? Just in case this fix breaks them...

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jun 22, 2016

can you test the timers again?

I need one of your boards.

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jun 22, 2016

Tested again periph_timer (with TIM_SPEED = 250000 instead of 1000000) and it works.

ACK again.

@kYc0o
Copy link
Copy Markdown
Contributor Author

kYc0o commented Jun 22, 2016

Then go! :D (you can push the green button xD)

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Jun 22, 2016

you can push the green button xD

Yeah !

@aabadie aabadie merged commit 42d34e6 into RIOT-OS:master Jun 22, 2016
@kYc0o kYc0o deleted the atmega_common_unify branch June 22, 2016 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@aabadie aabadie

@PeterKietzmann PeterKietzmann

Labels

CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Platform: AVR Platform: This PR/issue effects AVR-based platforms Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@kYc0o @PeterKietzmann @mali @aabadie @kaspar030