Hi Wolfgang, Thank you for your review.
On Sun, Sep 2, 2012 at 10:10 PM, Wolfgang Denk <[email protected]> wrote: > Dear Bob Liu, > > In message <[email protected]> you wrote: >> Add header files for blackfin new processor bf60x. >> >> Signed-off-by: Bob Liu <[email protected]> >> --- >> arch/blackfin/include/asm/blackfin_cdef.h | 3 + >> arch/blackfin/include/asm/blackfin_def.h | 5 + >> arch/blackfin/include/asm/blackfin_local.h | 3 + >> arch/blackfin/include/asm/mach-bf609/BF609_cdef.h | 543 +++ >> arch/blackfin/include/asm/mach-bf609/BF609_def.h | 3758 >> +++++++++++++++++++++ >> arch/blackfin/include/asm/mach-bf609/anomaly.h | 128 + >> arch/blackfin/include/asm/mach-bf609/def_local.h | 5 + >> arch/blackfin/include/asm/mach-bf609/portmux.h | 257 ++ >> arch/blackfin/include/asm/mach-bf609/ports.h | 103 + >> arch/blackfin/include/asm/mach-common/bits/cgu.h | 80 + >> arch/blackfin/include/asm/mach-common/bits/dde.h | 88 + >> arch/blackfin/include/asm/mach-common/bits/mpu.h | 6 +- >> arch/blackfin/include/asm/mach-common/bits/pll.h | 5 + >> 13 files changed, 4983 insertions(+), 1 deletion(-) >> create mode 100644 arch/blackfin/include/asm/mach-bf609/BF609_cdef.h >> create mode 100644 arch/blackfin/include/asm/mach-bf609/BF609_def.h >> create mode 100644 arch/blackfin/include/asm/mach-bf609/anomaly.h >> create mode 100644 arch/blackfin/include/asm/mach-bf609/def_local.h >> create mode 100644 arch/blackfin/include/asm/mach-bf609/portmux.h >> create mode 100644 arch/blackfin/include/asm/mach-bf609/ports.h >> create mode 100644 arch/blackfin/include/asm/mach-common/bits/cgu.h >> create mode 100644 arch/blackfin/include/asm/mach-common/bits/dde.h > > Please make sure to have the string "PATCH" included with all patch > submissions, otherwise your patches are lost to patchwork, and most > likely to the respective custodian as well. > >> --- /dev/null >> +++ b/arch/blackfin/include/asm/mach-bf609/BF609_cdef.h > ... >> +#define bfin_read_CGU_STAT() bfin_read32(CGU_STAT) >> +#define bfin_read_CGU_CLKOUTSEL() bfin_read32(CGU_CLKOUTSEL) >> +#define bfin_read_CGU_CTL() bfin_read32(CGU_CTL) >> +#define bfin_write_CGU_CTL(val) bfin_write32(CGU_CTL, val) >> +#define bfin_read_CGU_DIV() bfin_read32(CGU_DIV) >> +#define bfin_write_CGU_DIV(val) bfin_write32(CGU_DIV, val) > > We don't allow CamelCaps identifiers. Please fix globally. > Sorry, i didn't get your idea here. > >> +#define CNT_CFG 0xFFC00400 /* CNT0 >> Configuration Register */ >> +#define CNT_IMSK 0xFFC00404 /* CNT0 Interrupt >> Mask Register */ >> +#define CNT_STAT 0xFFC00408 /* CNT0 Status >> Register */ >> +#define CNT_CMD 0xFFC0040C /* CNT0 Command >> Register */ >> +#define CNT_DEBNCE 0xFFC00410 /* CNT0 Debounce >> Register */ >> +#define CNT_CNTR 0xFFC00414 /* CNT0 Counter >> Register */ >> +#define CNT_MAX 0xFFC00418 /* CNT0 Maximum >> Count Register */ >> +#define CNT_MIN 0xFFC0041C /* CNT0 Minimum >> Count Register */ > > We don't allow register access based on raw addresses or base address > plus offset. Please define proper C structs to describe your > hardware. Please fix globally. > C structs can be defined to access these registers in other place. But this file is come from our hardware team i can't change it. > >> --- /dev/null >> +++ b/arch/blackfin/include/asm/mach-bf609/anomaly.h >> @@ -0,0 +1,128 @@ >> +/* >> + * DO NOT EDIT THIS FILE >> + * This file is under version control at >> + * >> svn://sources.blackfin.uclinux.org/toolchain/trunk/proc-defs/header-frags/ >> + * and can be replaced with that version at any time >> + * DO NOT EDIT THIS FILE > > This is bullshit. If you submit code to U-Boot, it gets maintained in > the U-Boot git repository. Dump this. > >> + * Copyright 2004-2010 Analog Devices Inc. >> + * Licensed under the ADI BSD license. >> + * https://docs.blackfin.uclinux.org/doku.php?id=adi_bsd > > Is this GPL compatible?? > > The link is dead and returns only > > This topic does not exist yet > >> +#define ANOMALY_05000074 (1) > > Please do not put parens around simple defines. Please fix globally. > Will be fixed. > Checkpatch throws a ton of errors that all need fixing. > Which Checkpatch script are you using? In my test there are only WARNING: line over 80 characters. total: 0 errors, 3546 warnings, 5018 lines checked NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE /home/bob/u-boot2/0001-Blackfin-BF60x-new-processor-header-files.patch has style problems, please review. > Also note that we don't allow fixed network parameters (like > CONFIG_ETHADDR) in board config files. > Will be fixed. > Review stops here (except for the last part with the licensing stuff). > Thank you ! -- Regards, --Bob _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
