Added stm32l476 support, trivial kernel source code cleanup #148
Conversation
Kochise
changed the title
|
@Kochise, Thanks again for sending pull request. Could you please use "git rebase -i` to rework these commits so that they can look cleaner and functioned? |
|
Thanks for the guidance, since I'm not a git expert. |
|
Done it, doesn't seems to have changed anything. More advices ? |
There was a problem hiding this comment.
Thanks @Kochise for Pull Request for new board!
There have some small coding style problem and should be easy to fix. Also, C99 declaration style I think it shouldn't need to change, for example:
int i ;
for (i = 0; ...
We will prefer
for (int i = 0; ....
As for many place I saw this kind of change , please help to revert this. thanks!
board/nucleol476rg/board.c
Outdated
| @@ -0,0 +1,32 @@ | |||
| /* Copyright (c) 2016 The F9 Microkernel Project. All rights reserved. | |||
There was a problem hiding this comment.
Please change the copyright year to 2017
include/platform/stm32l476/exti.h
Outdated
| /* EXTI trigger type */ | ||
| #define EXTI_RISING_TRIGGER 0x8 | ||
| #define EXTI_FALLING_TRIGGER 0xc | ||
| #define EXTI_RISING_FALLING_TRIGGER 0x10 |
There was a problem hiding this comment.
Please help to modify the style to make it consist.
#define foo 0x0
#define bar 0x.4
...etc
include/platform/stm32l476/gpio.h
Outdated
| #define af_spi3 AF6 | ||
| #define af_usart1 AF7 | ||
| #define af_usart2 AF7 | ||
| #define af_usart3 AF7 |
include/platform/stm32l476/syscfg.h
Outdated
| *SYSCFG_MEMRMP = 0x1; | ||
| } | ||
|
|
||
|
|
include/platform/stm32l476/usart.h
Outdated
| #define USART_FE ((uint16_t) (0x1 << 1)) | ||
| #define USART_PE ((uint16_t) (0x1 << 0)) | ||
|
|
||
|
|
kernel/interrupt.c
Outdated
| struct user_irq **iter, *uirq; | ||
|
|
||
| uirq = user_irqs[irq]; | ||
| for (iter = &user_irq_queue.head ; *iter != NULL ; |
There was a problem hiding this comment.
Please use C99 style as before
kernel/interrupt.c
Outdated
| for (int i = 0 ; i < IRQn_NUM ; i++) | ||
| int i; | ||
|
|
||
| for (i = 0 ; i < IRQn_NUM ; i++) |
kernel/kprobes.c
Outdated
| kp->addr = (void *) ((uint32_t) kp->addr & ~(1UL)); | ||
| if (is_thumb32(*(uint16_t *) kp->addr)) | ||
| kp->step_addr = kp->addr + 4; | ||
| else | ||
| kp->step_addr = kp->addr + 2; | ||
|
|
||
| if (kprobe_arch_add(kp) < 0) | ||
| ret = kprobe_arch_add(kp); |
There was a problem hiding this comment.
no need to seperate this, since it will return int itself.
|
Also, there is no need for rebase / sqashing commit, since when finish the PR, member can squash via GitHub UI, thanks! |
|
"for (int i = 0;" is considered bad coding practices, the declaration and affectation should be separated. All members and local variables should be placed at one location only to ease maintenance and avoid scope conflicts. Btw I separated these into two commits, the one regarding C99 styling is isolated, you just have to ditch it out if the changes really doesn't fits your taste. Bear in mind that I developed my own 'personal' coding style, far beyond the 70s bearded nerd habits and/or the ioccc craze, which aims are to avoid all common pitfalls, ease debugging, maintenance and readability. If you dare to review my 'SkinProgress' code from 2003, I bet you'll call me a heretic. And my coding style even 'improved' since then, yet none of my public repos shows it actually. I separated the 'ret =' because it is considered best practice and easier to debug to have a variable holding the value. You might want to use it in an 'assert' for instance. Clever 'optimizations' like this should be left to the compiler. Copyright year is 2016 because the code changes were made last year, but only submitted this year. If that is really important, I will do so, just confirm. About rebasing/sqashing, I have no much idea what it serves as a purpose and how to do it. Thanks for your reviews and feedback. Your truly. edit : disambiguation |
|
@Kochise friend, I think we both want this thing better, I must apologize for that I didn't catch up the previous PR discussion. Your point about these variable make sense, and I think the copyright year probelm, if that is a new code (e.g. new board specific) need to move to 2017, others don't need to change. Again, thanks for your helping and contribute. |
|
No problem, I'm rather happy to contribute to this nice project of yours :) Will change copyright date to 2017 of the stm32l4 files. Sorry for my rants about coding style. This is a very sensitive subject to me. |
Cleaned things into two separate commits