Re: [U-Boot-Users] AVR32 patchset available

31 Aug 2006


      Dear Haarvard,
in message 20060831150308.22fdbab7@cad-250-152.norway.atmel.com you wrote:
...
...
Yes, please read the README, and follow the rules given there.
Uh...could you please be more specific? I know some of the patches are
too large, but it's hard to split them up any further. I thought I'd
keep them off the list to avoid bothering people who don't really care
about the AVR32 port, but I can of course post them if you want me to.
I mean the Coding Style rules; a cursory scan showed at least
incorrect indentation (not by 8 characters, by space instead of TAB)
in several files (cpu/at32ap/dcache_clean.c,
cpu/at32ap/dcache_invalidate.c, and include/asm-avr32/posix_types.h)
and trailing empty lines in some (include/asm-avr32/sections.h and
include/asm-avr32/setup.h).
Some files look pretty odd and should be cleaned up; for example:
"cpu/at32ap/pio2.h":
    ...
     40 /* Bitfields in PER */
     41
     42 /* Bitfields in PDR */
     43
     44 /* Bitfields in PSR */
     45
     46 /* Bitfields in OER */
     47
     48 /* Bitfields in ODR */
     49
     50 /* Bitfields in OSR */
     51
     52 /* Bitfields in IFER */
     53
     54 /* Bitfields in IFDR */
     55
     56 /* Bitfields in ISFR */
     57
     58 /* Bitfields in SODR */
     59
     60 /* Bitfields in CODR */
     61
     62 /* Bitfields in ODSR */
     63
     64 /* Bitfields in PDSR */
     65
     66 /* Bitfields in IER */
     67
     68 /* Bitfields in IDR */
     69
     70 /* Bitfields in IMR */
     71
     72 /* Bitfields in ISR */
     73
     74 /* Bitfields in MDER */
     75
     76 /* Bitfields in MDDR */
     77
     78 /* Bitfields in MDSR */
     79
     80 /* Bitfields in PUDR */
     81
     82 /* Bitfields in PUER */
     83
     84 /* Bitfields in PUSR */
     85
     86 /* Bitfields in ASR */
     87
     88 /* Bitfields in BSR */
     89
Get rid of this.
90 /* Bitfields in ABSR */
     91 #define PIO2_P0_OFFSET                          0
     92 #define PIO2_P0_SIZE                            1
     93 #define PIO2_P1_OFFSET                          1
     94 #define PIO2_P1_SIZE                            1
     95 #define PIO2_P2_OFFSET                          2
     96 #define PIO2_P2_SIZE                            1
     97 #define PIO2_P3_OFFSET                          3
     98 #define PIO2_P3_SIZE                            1
     99 #define PIO2_P4_OFFSET                          4
    100 #define PIO2_P4_SIZE                            1
    101 #define PIO2_P5_OFFSET                          5
    102 #define PIO2_P5_SIZE                            1
    103 #define PIO2_P6_OFFSET                          6
    104 #define PIO2_P6_SIZE                            1
    105 #define PIO2_P7_OFFSET                          7
    106 #define PIO2_P7_SIZE                            1
    107 #define PIO2_P8_OFFSET                          8
    108 #define PIO2_P8_SIZE                            1
    109 #define PIO2_P9_OFFSET                          9
    110 #define PIO2_P9_SIZE                            1
    111 #define PIO2_P10_OFFSET                         10
    112 #define PIO2_P10_SIZE                           1
    113 #define PIO2_P11_OFFSET                         11
    114 #define PIO2_P11_SIZE                           1
    115 #define PIO2_P12_OFFSET                         12
    116 #define PIO2_P12_SIZE                           1
    117 #define PIO2_P13_OFFSET                         13
    118 #define PIO2_P13_SIZE                           1
    119 #define PIO2_P14_OFFSET                         14
    120 #define PIO2_P14_SIZE                           1
    121 #define PIO2_P15_OFFSET                         15
    122 #define PIO2_P15_SIZE                           1
    123 #define PIO2_P16_OFFSET                         16
    124 #define PIO2_P16_SIZE                           1
    125 #define PIO2_P17_OFFSET                         17
    ... etc.
This just makes to code difficult  to  read  and  understand.  Please
cleanup.
The same applies to other files as well.
Please get rid of include/asm-avr32/errno-base.h and use  justerrno.h
like other architectures do.
Your global data structure contains a field "jt". Please add  comment
what it's needed for.
Please get rid of include/asm-avr32/initcalls.h
...
The only other things I can think about are CHANGELOG and CREDITS
entries, although the Subject: line in each patch should be suitable
for the changelog, no?
Special formatting is required for the CHANGELOG entry.
...
...
The patch might have been built against an  ancient  version  of  the
software - this is of no use.
Indeed. But you really shouldn't apply the "combined" patch -- I
intended that one for people who want to test stuff without having to
figure out the exact git state I made the patchset against.
That's what I did. I just used the latest version.
...
Also, as I mentioned in the previous mail, some of the patches have
been submitted before, and some should not be applied at all. Here are
the patches you may want consider:
http://avr32linux.org/patches/u-boot/1.1.4-hs1/broken-out/avr32-arch.patch
http://avr32linux.org/patches/u-boot/1.1.4-hs1/broken-out/at32ap-cpu.patch
http://avr32linux.org/patches/u-boot/1.1.4-hs1/broken-out/atmel-usart.patch
http://avr32linux.org/patches/u-boot/1.1.4-hs1/broken-out/atstk1000-board.pa...
See comments above. Please clean up and resubmit on the mailing list.
...
If you want me to combine these patches into a single package, please
let me know.
No, please stick to the rules as speicfied in the README.
Best regards,
Wolfgang Denk
-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
"Consistency requires you to be as ignorant today as you were a  year
ago."                                              - Bernard Berenson