[riot-notifications] [RIOT-OS/RIOT] cpu/atmega: use power.h defines instead of direct register access (#7601)

lebrush notifications at github.com
Thu Sep 14 20:19:26 CEST 2017


@kYc0o Why did this got merged so fast!!!???? Almost no time for review (3h time window). 

IMO this complicates definition of peripheral in the "new" style using structs and will fill the code of `#if` `#else`. See #7542 as an example.

Now four more defines are required:

```C
#define power_pwm0_enable power_timer0_enable 
#define power_pwm0_disable power_timer0_disable
#define power_pwm1_enable power_timer1_enable 
#define power_pwm1_disable power_timer1_disable
```

And they will have to be rewritten every time the struct changes. Furthermore, it requires now extra code:


```C

if (dev == 0) {
    power_pwm0_enable();
}
else if (dev == 1) {
    power_pwm1_enable();
}
```

And this will happen for every single peripheral defined in the "new" style if we do not want to list all of them. I'm not sure this is an advantage in all the situations.

I disagree with part of this PR and suggest to revert the merge. The functions should be used whenever possible but should not be a must. Makes no sense. Besides they are simple defines which do the same thing.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/7601#issuecomment-329566847
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20170914/929088d1/attachment.html>


More information about the notifications mailing list