Message ID | 1294816806-32614-2-git-send-email-hs@denx.de |
---|---|
State | Changes Requested |
Headers | show |
Dear Heiko Schocher, In message <1294816806-32614-2-git-send-email-hs@denx.de> you wrote: > Global question: do we really need an CONFIG_DIGSY_REV5? Is there not a way to determine the revision by probing the hardware? For example, the RTC's show up at different addresses on the bus - but ther emight be even easier ways? > diff --git a/board/digsy_mtc/digsy_mtc.c b/board/digsy_mtc/digsy_mtc.c > index cc6087b..2b0c574 100644 > --- a/board/digsy_mtc/digsy_mtc.c > +++ b/board/digsy_mtc/digsy_mtc.c .... > #endif /* CONFIG_IDE_RESET */ > +#endif /* CONFIG_CMD_IDE */ This looks wrong to me. You did not add a matching "#if" or "#ifdef" anywhere? > +/* Update the Flash Baseaddr settings */ > +int update_flash_size (int flash_size) > +{ > + volatile struct mpc5xxx_mmap_ctl *mm = > + (struct mpc5xxx_mmap_ctl *) CONFIG_SYS_MBAR; > + flash_info_t *dev; > + int i; > + int size = 0; > + unsigned long base = 0x0; > + u32 *cs_reg = (u32 *)&mm->cs0_start; > + > + for (i = 0; i < 2; i++) { > + dev = &flash_info[i]; > + > + if (dev->size) { > + /* calculate new base addr for this chipselect */ > + base -= dev->size; > + out_be32(cs_reg, START_REG(base)); > + cs_reg++; > + out_be32(cs_reg, STOP_REG(base, dev->size)); > + cs_reg++; > + /* recalculate the sectoraddr in the cfi driver */ > + size += flash_get_size(base, i); > + } > + } > +#if defined(CONFIG_DIGSY_REV5) > + gd->bd->bi_flashstart = base; > +#endif Why is this #if needed? Why not always set bi_flashstart ? > void ft_board_setup(void *blob, bd_t *bd) > { > ft_cpu_setup(blob, bd); > + /* remove RTC */ > +#if defined(CONFIG_DIGSY_REV5) > + ft_delete_node(blob, "dallas,ds1339"); > +#else > + ft_delete_node(blob, "mc,rv3029c2"); > +#endif You should add a comment here what you are doing, and why. ft_delete_node() returns int - why do you ignore the return codes? > +#if defined(CONFIG_SYS_UPDATE_FLASH_SIZE) > + ft_adapt_flash_base(blob); > +#endif ft_adapt_flash_base() returns int - why do you ignore the return code? > } > #endif /* defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) */ > - > -#endif /* CONFIG_CMD_IDE */ Ah! So this is a bug fix? > diff --git a/board/digsy_mtc/is45s16800a2.h b/board/digsy_mtc/is45s16800a2.h > new file mode 100644 > index 0000000..029e6cd > --- /dev/null > +++ b/board/digsy_mtc/is45s16800a2.h > @@ -0,0 +1,27 @@ > +/* > + * (C) Copyright 2004-2009 > + * Mark Jonas, Freescale Semiconductor, mark.jonas@motorola.com. Are you sure that Mark wrote any of this code? > diff --git a/boards.cfg b/boards.cfg > index 94b8745..9e1fc14 100644 > --- a/boards.cfg > +++ b/boards.cfg > @@ -241,6 +241,9 @@ cm5200 powerpc mpc5xxx > digsy_mtc powerpc mpc5xxx digsy_mtc > digsy_mtc_LOWBOOT powerpc mpc5xxx digsy_mtc - - digsy_mtc:SYS_TEXT_BASE=0xFF000000 > digsy_mtc_RAMBOOT powerpc mpc5xxx digsy_mtc - - digsy_mtc:SYS_TEXT_BASE=0x00100000 > +digsy_mtc_rev5 powerpc mpc5xxx digsy_mtc - - digsy_mtc:DIGSY_REV5 > +digsy_mtc_rev5_LOWBOOT powerpc mpc5xxx digsy_mtc - - digsy_mtc:SYS_TEXT_BASE=0xFF000000,DIGSY_REV5 > +digsy_mtc_rev5_RAMBOOT powerpc mpc5xxx digsy_mtc - - digsy_mtc:SYS_TEXT_BASE=0x00100000,DIGSY_REV5 Do we really need all these 6 configurations for the digsy_mtc board? Are all of them actively being used? > diff --git a/doc/README.cfi b/doc/README.cfi > new file mode 100644 > index 0000000..fa35108 > --- /dev/null > +++ b/doc/README.cfi > @@ -0,0 +1,15 @@ > +known issues: > + > +using M29W128GH from Numonyx: > + > +You need to add a board specific flash_cmd_reset() function > +for this chip to work correctly. Something like this should > +work (tested on the digsy_mtc board): > + > +void flash_cmd_reset(flash_info_t *info) > +{ > + flash_write_cmd(info, 0, 0, AMD_CMD_RESET); > +} Stefan, can you please send an explicit ACK for this part? > +#if defined(CONFIG_DIGSY_REV5) > +#define CONFIG_SYS_FLASH_BASE 0xFE000000 > +#define CONFIG_SYS_FLASH_BASE_CS1 0xFC000000 > +#define CONFIG_SYS_MAX_FLASH_BANKS 2 > +#define CONFIG_SYS_FLASH_BANKS_LIST { CONFIG_SYS_FLASH_BASE_CS1, \ > + CONFIG_SYS_FLASH_BASE} > +#define CONFIG_SYS_UPDATE_FLASH_SIZE > +#define CONFIG_FDT_FIXUP_NOR_FLASH_SIZE > +#else > #define CONFIG_SYS_FLASH_BASE 0xFF000000 > -#define CONFIG_SYS_FLASH_SIZE 0x01000000 > - > #define CONFIG_SYS_MAX_FLASH_BANKS 1 > +#define CONFIG_SYS_FLASH_BANKS_LIST { CONFIG_SYS_FLASH_BASE } > +#endif Is this really needed? I understand you map the flash at the end of the address space? I used to use code like this in similar situations: #define CONFIG_SYS_FLASH_BASE (0-flash_info[0].size) This will auto-adjust depending on the actual size of the flash, and avoids all these ifdef's. Maybe you can do something similar? Best regards, Wolfgang Denk
Hello Wolfgang, Wolfgang Denk wrote: > In message <1294816806-32614-2-git-send-email-hs@denx.de> you wrote: > > Global question: do we really need an CONFIG_DIGSY_REV5? Is there not > a way to determine the revision by probing the hardware? For example, > the RTC's show up at different addresses on the bus - but ther emight > be even easier ways? Good question ... as I know, there is no possibility to detect the hardware on an other way then over the RTC ... and I don;t want to go this way ... what if the RTC is not responding? >> diff --git a/board/digsy_mtc/digsy_mtc.c b/board/digsy_mtc/digsy_mtc.c >> index cc6087b..2b0c574 100644 >> --- a/board/digsy_mtc/digsy_mtc.c >> +++ b/board/digsy_mtc/digsy_mtc.c > .... >> #endif /* CONFIG_IDE_RESET */ >> +#endif /* CONFIG_CMD_IDE */ > > This looks wrong to me. You did not add a matching "#if" or > "#ifdef" anywhere? The "#endif" is actual on the wrong place ... >> +/* Update the Flash Baseaddr settings */ >> +int update_flash_size (int flash_size) >> +{ >> + volatile struct mpc5xxx_mmap_ctl *mm = >> + (struct mpc5xxx_mmap_ctl *) CONFIG_SYS_MBAR; >> + flash_info_t *dev; >> + int i; >> + int size = 0; >> + unsigned long base = 0x0; >> + u32 *cs_reg = (u32 *)&mm->cs0_start; >> + >> + for (i = 0; i < 2; i++) { >> + dev = &flash_info[i]; >> + >> + if (dev->size) { >> + /* calculate new base addr for this chipselect */ >> + base -= dev->size; >> + out_be32(cs_reg, START_REG(base)); >> + cs_reg++; >> + out_be32(cs_reg, STOP_REG(base, dev->size)); >> + cs_reg++; >> + /* recalculate the sectoraddr in the cfi driver */ >> + size += flash_get_size(base, i); >> + } >> + } >> +#if defined(CONFIG_DIGSY_REV5) >> + gd->bd->bi_flashstart = base; >> +#endif > > Why is this #if needed? Why not always set bi_flashstart ? It is set in arch/powerpc/lib/board.c before calling update_flash_size(), so this adaption is only needed, if the baseaddr is changing on different flash sizes, what is valid for the rev5 board ... >> void ft_board_setup(void *blob, bd_t *bd) >> { >> ft_cpu_setup(blob, bd); >> + /* remove RTC */ >> +#if defined(CONFIG_DIGSY_REV5) >> + ft_delete_node(blob, "dallas,ds1339"); >> +#else >> + ft_delete_node(blob, "mc,rv3029c2"); >> +#endif > > You should add a comment here what you are doing, and why. Ok, you are right. (In the DTS there are two RTC nodes defined, and this code removes the not existing one in the dts) > ft_delete_node() returns int - why do you ignore the return codes? You are right, add a warning, thanks. >> +#if defined(CONFIG_SYS_UPDATE_FLASH_SIZE) >> + ft_adapt_flash_base(blob); >> +#endif > > ft_adapt_flash_base() returns int - why do you ignore the return code? Yep, add here also a warning. >> } >> #endif /* defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) */ >> - >> -#endif /* CONFIG_CMD_IDE */ > > Ah! So this is a bug fix? Yep. Should I seperate this in an extra patch? >> diff --git a/board/digsy_mtc/is45s16800a2.h b/board/digsy_mtc/is45s16800a2.h >> new file mode 100644 >> index 0000000..029e6cd >> --- /dev/null >> +++ b/board/digsy_mtc/is45s16800a2.h >> @@ -0,0 +1,27 @@ >> +/* >> + * (C) Copyright 2004-2009 >> + * Mark Jonas, Freescale Semiconductor, mark.jonas@motorola.com. > > Are you sure that Mark wrote any of this code? No, I just copied this from board/digsy_mtc/is42s16800a-7t.h, so add my name to it. >> diff --git a/boards.cfg b/boards.cfg >> index 94b8745..9e1fc14 100644 >> --- a/boards.cfg >> +++ b/boards.cfg >> @@ -241,6 +241,9 @@ cm5200 powerpc mpc5xxx >> digsy_mtc powerpc mpc5xxx digsy_mtc >> digsy_mtc_LOWBOOT powerpc mpc5xxx digsy_mtc - - digsy_mtc:SYS_TEXT_BASE=0xFF000000 >> digsy_mtc_RAMBOOT powerpc mpc5xxx digsy_mtc - - digsy_mtc:SYS_TEXT_BASE=0x00100000 >> +digsy_mtc_rev5 powerpc mpc5xxx digsy_mtc - - digsy_mtc:DIGSY_REV5 >> +digsy_mtc_rev5_LOWBOOT powerpc mpc5xxx digsy_mtc - - digsy_mtc:SYS_TEXT_BASE=0xFF000000,DIGSY_REV5 >> +digsy_mtc_rev5_RAMBOOT powerpc mpc5xxx digsy_mtc - - digsy_mtc:SYS_TEXT_BASE=0x00100000,DIGSY_REV5 > > Do we really need all these 6 configurations for the digsy_mtc board? > Are all of them actively being used? Good question, I just used digsy_mtc_rev5, digsy_mtc_rev5_RAMBOOT, maybe the LOWBOOT case could be deleted. >> diff --git a/doc/README.cfi b/doc/README.cfi >> new file mode 100644 >> index 0000000..fa35108 >> --- /dev/null >> +++ b/doc/README.cfi >> @@ -0,0 +1,15 @@ >> +known issues: >> + >> +using M29W128GH from Numonyx: >> + >> +You need to add a board specific flash_cmd_reset() function >> +for this chip to work correctly. Something like this should >> +work (tested on the digsy_mtc board): >> + >> +void flash_cmd_reset(flash_info_t *info) >> +{ >> + flash_write_cmd(info, 0, 0, AMD_CMD_RESET); >> +} > > Stefan, can you please send an explicit ACK for this part? > > >> +#if defined(CONFIG_DIGSY_REV5) >> +#define CONFIG_SYS_FLASH_BASE 0xFE000000 >> +#define CONFIG_SYS_FLASH_BASE_CS1 0xFC000000 >> +#define CONFIG_SYS_MAX_FLASH_BANKS 2 >> +#define CONFIG_SYS_FLASH_BANKS_LIST { CONFIG_SYS_FLASH_BASE_CS1, \ >> + CONFIG_SYS_FLASH_BASE} >> +#define CONFIG_SYS_UPDATE_FLASH_SIZE >> +#define CONFIG_FDT_FIXUP_NOR_FLASH_SIZE >> +#else >> #define CONFIG_SYS_FLASH_BASE 0xFF000000 >> -#define CONFIG_SYS_FLASH_SIZE 0x01000000 >> - >> #define CONFIG_SYS_MAX_FLASH_BANKS 1 >> +#define CONFIG_SYS_FLASH_BANKS_LIST { CONFIG_SYS_FLASH_BASE } >> +#endif > > Is this really needed? I understand you map the flash at the end of > the address space? I used to use code like this in similar > situations: > > #define CONFIG_SYS_FLASH_BASE (0-flash_info[0].size) > > This will auto-adjust depending on the actual size of the flash, and > avoids all these ifdef's. Maybe you can do something similar? Hmm.. I have only one flash on the !rev5 board, so I need this differentiation here, or? Hmm.. after looking in code, can your proposal work?: Is flash_info[0].size valid, when the cfi driver detects the flash? I see in drivers/mtd/cfi_flash.c: flash_info_t flash_info[CFI_MAX_FLASH_BANKS]; /* FLASH chips info */ [...] static phys_addr_t __cfi_flash_bank_addr(int i) { return ((phys_addr_t [])CONFIG_SYS_FLASH_BANKS_LIST)[i]; } [...] ulong flash_get_size (phys_addr_t base, int banknum) { flash_info_t *info = &flash_info[banknum]; [...] info->start[0] = (ulong)map_physmem(base, info->portwidth, MAP_NOCACHE); if (flash_detect_cfi (info, &qry)) { [...] unsigned long flash_init (void) [...] if (!flash_detect_legacy(cfi_flash_bank_addr(i), i)) flash_get_size(cfi_flash_bank_addr(i), i); and flash_info[0].size is initialized only in flash_get_size() after detecting the flash (so, after accessing it with base = 0-flash_info[0].size)... that must fail, or did I miss something? bye, Heiko
Dear Heiko Schocher, In message <4D2D7122.60909@denx.de> you wrote: > > > Global question: do we really need an CONFIG_DIGSY_REV5? Is there not > > a way to determine the revision by probing the hardware? For example, > > the RTC's show up at different addresses on the bus - but ther emight > > be even easier ways? > > Good question ... as I know, there is no possibility to detect the > hardware on an other way then over the RTC ... and I don;t want to > go this way ... what if the RTC is not responding? Maybe you can ask the hardware guys again? > >> + for (i = 0; i < 2; i++) { > >> + dev = &flash_info[i]; > >> + > >> + if (dev->size) { > >> + /* calculate new base addr for this chipselect */ > >> + base -= dev->size; > >> + out_be32(cs_reg, START_REG(base)); > >> + cs_reg++; > >> + out_be32(cs_reg, STOP_REG(base, dev->size)); > >> + cs_reg++; > >> + /* recalculate the sectoraddr in the cfi driver */ > >> + size += flash_get_size(base, i); > >> + } > >> + } > >> +#if defined(CONFIG_DIGSY_REV5) > >> + gd->bd->bi_flashstart = base; > >> +#endif > > > > Why is this #if needed? Why not always set bi_flashstart ? > > It is set in arch/powerpc/lib/board.c before calling update_flash_size(), > so this adaption is only needed, if the baseaddr is changing on different > flash sizes, what is valid for the rev5 board ... It may not be needed, but I think it should compute the same result, so you should be able to omit the "#if" and the "#endif". If you should compute a different result, then I'd wonder if the code is actually correct? > >> -#endif /* CONFIG_CMD_IDE */ > > > > Ah! So this is a bug fix? > > Yep. Should I seperate this in an extra patch? At least mention it in the commit message. > Hmm.. I have only one flash on the !rev5 board, so I need this > differentiation here, or? Yes. > Hmm.. after looking in code, can your proposal work?: > Is flash_info[0].size valid, when the cfi driver detects the flash? For the probing, a temporary address can be used. You set up the final mapping only after the sizes are knows, similar to what we do when we have several banks of RAM. Best regards, Wolfgang Denk
Hi Wolfgang, > Dear Heiko Schocher, > > In message <4D2D7122.60909@denx.de> you wrote: >> >> > Global question: do we really need an CONFIG_DIGSY_REV5? Is there not >> > a way to determine the revision by probing the hardware? For example, >> > the RTC's show up at different addresses on the bus - but ther emight >> > be even easier ways? >> >> Good question ... as I know, there is no possibility to detect the >> hardware on an other way then over the RTC ... and I don;t want to >> go this way ... what if the RTC is not responding? > > Maybe you can ask the hardware guys again? We did this and we are sure that there is no better way. Cheers Detlev
Hello Heiko, [...] >>> diff --git a/boards.cfg b/boards.cfg >>> index 94b8745..9e1fc14 100644 >>> --- a/boards.cfg >>> +++ b/boards.cfg >>> @@ -241,6 +241,9 @@ cm5200 powerpc mpc5xxx >>> digsy_mtc powerpc mpc5xxx digsy_mtc >>> digsy_mtc_LOWBOOT powerpc mpc5xxx digsy_mtc - - >>> digsy_mtc:SYS_TEXT_BASE=0xFF000000 >>> digsy_mtc_RAMBOOT powerpc mpc5xxx digsy_mtc - - >>> digsy_mtc:SYS_TEXT_BASE=0x00100000 >>> +digsy_mtc_rev5 powerpc mpc5xxx digsy_mtc - - digsy_mtc:DIGSY_REV5 >>> +digsy_mtc_rev5_LOWBOOT powerpc mpc5xxx digsy_mtc - - >>> digsy_mtc:SYS_TEXT_BASE=0xFF000000,DIGSY_REV5 >>> +digsy_mtc_rev5_RAMBOOT powerpc mpc5xxx digsy_mtc - - >>> digsy_mtc:SYS_TEXT_BASE=0x00100000,DIGSY_REV5 >> >> Do we really need all these 6 configurations for the digsy_mtc board? >> Are all of them actively being used? > > Good question, I just used digsy_mtc_rev5, digsy_mtc_rev5_RAMBOOT, > maybe the LOWBOOT case could be deleted. Yes, the LOWBOOT cases can be removed. The hardware can not be configured in a way to use them. Cheers Detlev
Hello Wolfgang, Wolfgang Denk wrote: > In message <4D2D7122.60909@denx.de> you wrote: >>> Global question: do we really need an CONFIG_DIGSY_REV5? Is there not >>> a way to determine the revision by probing the hardware? For example, >>> the RTC's show up at different addresses on the bus - but ther emight >>> be even easier ways? >> Good question ... as I know, there is no possibility to detect the >> hardware on an other way then over the RTC ... and I don;t want to >> go this way ... what if the RTC is not responding? > > Maybe you can ask the hardware guys again? Ok. done. As Detlev mentioned in the meantime the "LOWBOOT" cases can be removed, done. >>>> + for (i = 0; i < 2; i++) { >>>> + dev = &flash_info[i]; >>>> + >>>> + if (dev->size) { >>>> + /* calculate new base addr for this chipselect */ >>>> + base -= dev->size; >>>> + out_be32(cs_reg, START_REG(base)); >>>> + cs_reg++; >>>> + out_be32(cs_reg, STOP_REG(base, dev->size)); >>>> + cs_reg++; >>>> + /* recalculate the sectoraddr in the cfi driver */ >>>> + size += flash_get_size(base, i); >>>> + } >>>> + } >>>> +#if defined(CONFIG_DIGSY_REV5) >>>> + gd->bd->bi_flashstart = base; >>>> +#endif >>> Why is this #if needed? Why not always set bi_flashstart ? >> It is set in arch/powerpc/lib/board.c before calling update_flash_size(), >> so this adaption is only needed, if the baseaddr is changing on different >> flash sizes, what is valid for the rev5 board ... > > It may not be needed, but I think it should compute the same result, > so you should be able to omit the "#if" and the "#endif". > > If you should compute a different result, then I'd wonder if the code > is actually correct? Yep, you are right, the "#if ... #endif" is not needed here. >>>> -#endif /* CONFIG_CMD_IDE */ >>> Ah! So this is a bug fix? >> Yep. Should I seperate this in an extra patch? > > At least mention it in the commit message. Ok, add this to the commit message. >> Hmm.. I have only one flash on the !rev5 board, so I need this >> differentiation here, or? > > Yes. > >> Hmm.. after looking in code, can your proposal work?: >> Is flash_info[0].size valid, when the cfi driver detects the flash? > > For the probing, a temporary address can be used. You set up the > final mapping only after the sizes are knows, similar to what we do > when we have several banks of RAM. But thats exactly what I do with the defines in include/configs/digsy_mtc.h #define CONFIG_SYS_FLASH_BASE 0xFE000000 #define CONFIG_SYS_FLASH_BASE_CS1 0xFC000000 #define CONFIG_SYS_MAX_FLASH_BANKS 2 #define CONFIG_SYS_FLASH_BANKS_LIST { CONFIG_SYS_FLASH_BASE_CS1, \ CONFIG_SYS_FLASH_BASE} and after detecting the real size through the cfi driver on this temporary addresses, the flash_info gets updated with the new baseaddr through the call: int update_flash_size (int flash_size) [...] if (dev->size) { /* calculate new base addr for this chipselect */ base -= dev->size; [...] /* recalculate the sectoraddr in the cfi driver */ size += flash_get_size(base, i); so after that, flash_info has the right values and I update the chip selects in board specific code ... bye, Heiko
Hi Wolfgang & Heiko, On Wednesday 12 January 2011 08:59:20 Wolfgang Denk wrote: > > diff --git a/doc/README.cfi b/doc/README.cfi > > new file mode 100644 > > index 0000000..fa35108 > > --- /dev/null > > +++ b/doc/README.cfi > > @@ -0,0 +1,15 @@ > > +known issues: > > + > > +using M29W128GH from Numonyx: > > + > > +You need to add a board specific flash_cmd_reset() function > > +for this chip to work correctly. Something like this should > > +work (tested on the digsy_mtc board): > > + > > +void flash_cmd_reset(flash_info_t *info) > > +{ > > + flash_write_cmd(info, 0, 0, AMD_CMD_RESET); > > +} > > Stefan, can you please send an explicit ACK for this part? If we put something like this into this new README, we should make the description a bit better. Something like: --- <snip> --- The common CFI driver provides this weak default implementation for flash_cmd_reset(): void __flash_cmd_reset(flash_info_t *info) { /* * We do not yet know what kind of commandset to use, so we issue * the reset command in both Intel and AMD variants, in the hope * that AMD flash roms ignore the Intel command. */ flash_write_cmd(info, 0, 0, AMD_CMD_RESET); flash_write_cmd(info, 0, 0, FLASH_CMD_RESET); } void flash_cmd_reset(flash_info_t *info) __attribute__((weak,alias("__flash_cmd_reset"))); Some flash chips seems to have trouble with this reset sequence. In this case the board specific code can override this weak default version with a board specific function. For example the digsy_mtc board equipped with the M29W128GH from Numonyx needs this version to function properly: void flash_cmd_reset(flash_info_t *info) { flash_write_cmd(info, 0, 0, AMD_CMD_RESET); } --- <snip> --- Heiko, if nobody objects then please include this version into your next patch version. Here my: Signed-off-by: Stefan Roese <sr@denx.de> Thanks. Cheers, Stefan -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office@denx.de
Dear Heiko Schocher, In message <4D2D81C3.7010807@denx.de> you wrote: > > > Maybe you can ask the hardware guys again? > > Ok. done. As Detlev mentioned in the meantime the "LOWBOOT" cases > can be removed, done. Thanks. > > For the probing, a temporary address can be used. You set up the > > final mapping only after the sizes are knows, similar to what we do > > when we have several banks of RAM. > > But thats exactly what I do with the defines in include/configs/digsy_mtc.h Ah, OK. did not notice that. Best regards, Wolfgang Denk
difference to previous board version: - M29W128GH flash from Numonyx - SDRAM ISSI IS45S16800 (Option A2 105°C) - rev5 uses RTC RV-3029-C2 - update cs0 and cs1 baseaddr and length depending on the detected flash size. - added Werner Pfister <Pfister_Werner@intercontrol.de> as maintainer for the digsy board variants - As the M29W128GH needs a special flash_cmd_reset() document that in the new file doc/README.cfi. Signed-off-by: Heiko Schocher <hs@denx.de> cc: Wolfgang Denk <hs@denx.de> cc: Stefan Roese <sr@denx.de> cc: Werner Pfister <Pfister_Werner@intercontrol.de> --- MAINTAINERS | 4 ++ board/digsy_mtc/digsy_mtc.c | 102 +++++++++++++++++++++++++++++++++++++++- board/digsy_mtc/is45s16800a2.h | 27 +++++++++++ boards.cfg | 3 + doc/README.cfi | 15 ++++++ include/configs/digsy_mtc.h | 27 +++++++++- 6 files changed, 173 insertions(+), 5 deletions(-) create mode 100644 board/digsy_mtc/is45s16800a2.h create mode 100644 doc/README.cfi diff --git a/MAINTAINERS b/MAINTAINERS index 553930a..bd18f7b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -340,6 +340,10 @@ Denis Peter <d.peter@mpl.ch> MIP405 PPC4xx PIP405 PPC4xx +Werner Pfister <Pfister_Werner@intercontrol.de> + digsy_mtc mpc5200 + digsy_mtc_rev5 mpc5200 + Kim Phillips <kim.phillips@freescale.com> MPC8349EMDS MPC8349 diff --git a/board/digsy_mtc/digsy_mtc.c b/board/digsy_mtc/digsy_mtc.c index cc6087b..2b0c574 100644 --- a/board/digsy_mtc/digsy_mtc.c +++ b/board/digsy_mtc/digsy_mtc.c @@ -39,12 +39,29 @@ #include <asm/processor.h> #include <asm/io.h> #include "eeprom.h" +#if defined(CONFIG_DIGSY_REV5) +#include "is45s16800a2.h" +#include <mtd/cfi_flash.h> +#else #include "is42s16800a-7t.h" +#endif +#include <libfdt.h> DECLARE_GLOBAL_DATA_PTR; extern int usb_cpu_init(void); +#if defined(CONFIG_DIGSY_REV5) +/* + * The M29W128GH needs a specail reset command function, + * details see the doc/README.cfi file + */ +void flash_cmd_reset(flash_info_t *info) +{ + flash_write_cmd(info, 0, 0, AMD_CMD_RESET); +} +#endif + #ifndef CONFIG_SYS_RAMBOOT static void sdram_start(int hi_addr) { @@ -175,6 +192,9 @@ int checkboard(void) char *s = getenv("serial#"); puts ("Board: InterControl digsyMTC"); +#if defined(CONFIG_DIGSY_REV5) + puts (" rev5"); +#endif if (s != NULL) { puts(", "); puts(s); @@ -305,12 +325,90 @@ void ide_set_reset(int idereset) setbits_be32((void *)MPC5XXX_WU_GPIO_ENABLE, (1 << 25)); } #endif /* CONFIG_IDE_RESET */ +#endif /* CONFIG_CMD_IDE */ #if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) +static int ft_delete_node(void *fdt, const char *compat) +{ + int off = -1; + + off = fdt_node_offset_by_compatible(fdt, -1, compat); + if (off < 0) + return off; + + return(fdt_del_node(fdt, off)); +} +#if defined(CONFIG_SYS_UPDATE_FLASH_SIZE) +static int ft_adapt_flash_base(void *blob) +{ + flash_info_t *dev = &flash_info[0]; + int off; + struct fdt_property *prop; + int len; + u32 *reg, *reg2; + + off = fdt_node_offset_by_compatible(blob, -1, "fsl,mpc5200b-lpb"); + if (off < 0) + return off; + + /* found compatible property */ + prop = fdt_get_property_w(blob, off, "ranges", &len); + if (prop) { + reg = reg2 = (u32 *)&prop->data[0]; + + reg[2] = dev->start[0]; + reg[3] = dev->size; + fdt_setprop(blob, off, "ranges", reg2, len); + } + + return 0; +} + +extern ulong flash_get_size (phys_addr_t base, int banknum); + +/* Update the Flash Baseaddr settings */ +int update_flash_size (int flash_size) +{ + volatile struct mpc5xxx_mmap_ctl *mm = + (struct mpc5xxx_mmap_ctl *) CONFIG_SYS_MBAR; + flash_info_t *dev; + int i; + int size = 0; + unsigned long base = 0x0; + u32 *cs_reg = (u32 *)&mm->cs0_start; + + for (i = 0; i < 2; i++) { + dev = &flash_info[i]; + + if (dev->size) { + /* calculate new base addr for this chipselect */ + base -= dev->size; + out_be32(cs_reg, START_REG(base)); + cs_reg++; + out_be32(cs_reg, STOP_REG(base, dev->size)); + cs_reg++; + /* recalculate the sectoraddr in the cfi driver */ + size += flash_get_size(base, i); + } + } +#if defined(CONFIG_DIGSY_REV5) + gd->bd->bi_flashstart = base; +#endif + return 0; +} +#endif /* defined(CONFIG_SYS_UPDATE_FLASH_SIZE) */ + void ft_board_setup(void *blob, bd_t *bd) { ft_cpu_setup(blob, bd); + /* remove RTC */ +#if defined(CONFIG_DIGSY_REV5) + ft_delete_node(blob, "dallas,ds1339"); +#else + ft_delete_node(blob, "mc,rv3029c2"); +#endif +#if defined(CONFIG_SYS_UPDATE_FLASH_SIZE) + ft_adapt_flash_base(blob); +#endif } #endif /* defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) */ - -#endif /* CONFIG_CMD_IDE */ diff --git a/board/digsy_mtc/is45s16800a2.h b/board/digsy_mtc/is45s16800a2.h new file mode 100644 index 0000000..029e6cd --- /dev/null +++ b/board/digsy_mtc/is45s16800a2.h @@ -0,0 +1,27 @@ +/* + * (C) Copyright 2004-2009 + * Mark Jonas, Freescale Semiconductor, mark.jonas@motorola.com. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#define SDRAM_MODE 0x00CD0000 +#define SDRAM_CONTROL 0x50470000 +#define SDRAM_CONFIG1 0xD2322900 +#define SDRAM_CONFIG2 0x8AD70000 diff --git a/boards.cfg b/boards.cfg index 94b8745..9e1fc14 100644 --- a/boards.cfg +++ b/boards.cfg @@ -241,6 +241,9 @@ cm5200 powerpc mpc5xxx digsy_mtc powerpc mpc5xxx digsy_mtc digsy_mtc_LOWBOOT powerpc mpc5xxx digsy_mtc - - digsy_mtc:SYS_TEXT_BASE=0xFF000000 digsy_mtc_RAMBOOT powerpc mpc5xxx digsy_mtc - - digsy_mtc:SYS_TEXT_BASE=0x00100000 +digsy_mtc_rev5 powerpc mpc5xxx digsy_mtc - - digsy_mtc:DIGSY_REV5 +digsy_mtc_rev5_LOWBOOT powerpc mpc5xxx digsy_mtc - - digsy_mtc:SYS_TEXT_BASE=0xFF000000,DIGSY_REV5 +digsy_mtc_rev5_RAMBOOT powerpc mpc5xxx digsy_mtc - - digsy_mtc:SYS_TEXT_BASE=0x00100000,DIGSY_REV5 galaxy5200 powerpc mpc5xxx galaxy5200 - - galaxy5200:galaxy5200 galaxy5200_LOWBOOT powerpc mpc5xxx galaxy5200 - - galaxy5200:galaxy5200_LOWBOOT icecube_5200 powerpc mpc5xxx icecube - - IceCube diff --git a/doc/README.cfi b/doc/README.cfi new file mode 100644 index 0000000..fa35108 --- /dev/null +++ b/doc/README.cfi @@ -0,0 +1,15 @@ +known issues: + +using M29W128GH from Numonyx: + +You need to add a board specific flash_cmd_reset() function +for this chip to work correctly. Something like this should +work (tested on the digsy_mtc board): + +void flash_cmd_reset(flash_info_t *info) +{ + flash_write_cmd(info, 0, 0, AMD_CMD_RESET); +} + +see also: +http://www.mail-archive.com/u-boot@lists.denx.de/msg24368.html diff --git a/include/configs/digsy_mtc.h b/include/configs/digsy_mtc.h index d541160..bfbec6a 100644 --- a/include/configs/digsy_mtc.h +++ b/include/configs/digsy_mtc.h @@ -249,9 +249,14 @@ /* * RTC configuration */ +#if defined(CONFIG_DIGSY_REV5) +#define CONFIG_SYS_I2C_RTC_ADDR 0x56 +#define CONFIG_RTC_RV3029 +#else #define CONFIG_RTC_DS1337 #define CONFIG_SYS_I2C_RTC_ADDR 0x68 #define CONFIG_SYS_DS1339_TCR_VAL 0xAB /* diode + 4k resistor */ +#endif /* * Flash configuration @@ -259,14 +264,24 @@ #define CONFIG_SYS_FLASH_CFI 1 #define CONFIG_FLASH_CFI_DRIVER 1 +#if defined(CONFIG_DIGSY_REV5) +#define CONFIG_SYS_FLASH_BASE 0xFE000000 +#define CONFIG_SYS_FLASH_BASE_CS1 0xFC000000 +#define CONFIG_SYS_MAX_FLASH_BANKS 2 +#define CONFIG_SYS_FLASH_BANKS_LIST { CONFIG_SYS_FLASH_BASE_CS1, \ + CONFIG_SYS_FLASH_BASE} +#define CONFIG_SYS_UPDATE_FLASH_SIZE +#define CONFIG_FDT_FIXUP_NOR_FLASH_SIZE +#else #define CONFIG_SYS_FLASH_BASE 0xFF000000 -#define CONFIG_SYS_FLASH_SIZE 0x01000000 - #define CONFIG_SYS_MAX_FLASH_BANKS 1 +#define CONFIG_SYS_FLASH_BANKS_LIST { CONFIG_SYS_FLASH_BASE } +#endif + #define CONFIG_SYS_MAX_FLASH_SECT 256 #define CONFIG_FLASH_16BIT #define CONFIG_SYS_FLASH_CFI_WIDTH FLASH_CFI_16BIT -#define CONFIG_SYS_FLASH_BANKS_LIST { CONFIG_SYS_FLASH_BASE } +#define CONFIG_SYS_FLASH_SIZE 0x01000000 #define CONFIG_SYS_FLASH_ERASE_TOUT 240000 #define CONFIG_SYS_FLASH_WRITE_TOUT 500 @@ -409,6 +424,12 @@ #define CONFIG_SYS_CS0_SIZE CONFIG_SYS_FLASH_SIZE #define CONFIG_SYS_CS0_CFG 0x0002DD00 +#if defined(CONFIG_DIGSY_REV5) +#define CONFIG_SYS_CS1_START CONFIG_SYS_FLASH_BASE_CS1 +#define CONFIG_SYS_CS1_SIZE CONFIG_SYS_FLASH_SIZE +#define CONFIG_SYS_CS1_CFG 0x0002DD00 +#endif + #define CONFIG_SYS_CS_BURST 0x00000000 #define CONFIG_SYS_CS_DEADCYCLE 0x11111111