| Message ID | 1326219136-1953-1-git-send-email-urwithsughosh@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
> The current implementation invalidates the cache instead of flushing > it. This causes problems on platforms where the spl/u-boot is already > loaded to the RAM, with caches enabled by a first stage bootloader. What platforms are affected? M > > The V bit of the cp15's control register c1 is set to the value of > VINITHI on reset. Do not clear this bit by default, as there are SOC's > with no valid memory region at 0x0. > > Signed-off-by: Sughosh Ganu <urwithsughosh@gmail.com> > Cc: Albert Aribaud <albert.u.boot@aribaud.net> > --- > > Changes since V1 > * Added arm926 keyword to the subject line > * Removed the superfluous setting of r0. > * Fixed the comment to reflect the fact that V is not being cleared > > arch/arm/cpu/arm926ejs/start.S | 9 ++++++--- > 1 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/cpu/arm926ejs/start.S > b/arch/arm/cpu/arm926ejs/start.S index 6a09c02..6e261c2 100644 > --- a/arch/arm/cpu/arm926ejs/start.S > +++ b/arch/arm/cpu/arm926ejs/start.S > @@ -355,17 +355,20 @@ _dynsym_start_ofs: > */ > cpu_init_crit: > /* > - * flush v4 I/D caches > + * flush D cache before disabling it > */ > mov r0, #0 > - mcr p15, 0, r0, c7, c7, 0 /* flush v3/v4 cache */ > +flush_dcache: > + mrc p15, 0, r15, c7, c10, 3 > + bne flush_dcache > + > mcr p15, 0, r0, c8, c7, 0 /* flush v4 TLB */ > > /* > * disable MMU stuff and caches > */ > mrc p15, 0, r0, c1, c0, 0 > - bic r0, r0, #0x00002300 /* clear bits 13, 9:8 (--V- --RS) */ > + bic r0, r0, #0x00000300 /* clear bits 9:8 ( --RS) */ > bic r0, r0, #0x00000087 /* clear bits 7, 2:0 (B--- -CAM) */ > orr r0, r0, #0x00000002 /* set bit 2 (A) Align */ > orr r0, r0, #0x00001000 /* set bit 12 (I) I-Cache */
On Tue Jan 10, 2012 at 09:07:58PM +0100, Marek Vasut wrote: > > The current implementation invalidates the cache instead of flushing > > it. This causes problems on platforms where the spl/u-boot is already > > loaded to the RAM, with caches enabled by a first stage bootloader. > > What platforms are affected? It is causing a problem on the hawkboard, where the spl is loaded directly to the RAM by a rom bootloader. We did not see this earlier since cpu_init_crit was not getting called due to CONFIG_SKIP_LOWLEVEL_INIT. -sughosh
> On Tue Jan 10, 2012 at 09:07:58PM +0100, Marek Vasut wrote: > > > The current implementation invalidates the cache instead of flushing > > > it. This causes problems on platforms where the spl/u-boot is already > > > loaded to the RAM, with caches enabled by a first stage bootloader. > > > > What platforms are affected? > > It is causing a problem on the hawkboard, where the spl is loaded > directly to the RAM by a rom bootloader. We did not see this earlier > since cpu_init_crit was not getting called due to > CONFIG_SKIP_LOWLEVEL_INIT. > > -sughosh I see ... why don't you directly load U-Boot to DRAM then ? M
On Wed Jan 11, 2012 at 11:47:27AM +0100, Marek Vasut wrote: > > On Tue Jan 10, 2012 at 09:07:58PM +0100, Marek Vasut wrote: > > > > The current implementation invalidates the cache instead of flushing > > > > it. This causes problems on platforms where the spl/u-boot is already > > > > loaded to the RAM, with caches enabled by a first stage bootloader. > > > > > > What platforms are affected? > > > > It is causing a problem on the hawkboard, where the spl is loaded > > directly to the RAM by a rom bootloader. We did not see this earlier > > since cpu_init_crit was not getting called due to > > CONFIG_SKIP_LOWLEVEL_INIT. > > > > -sughosh > > I see ... why don't you directly load U-Boot to DRAM then ? The rom bootloader(rbl) uses a different ecc layout from the one used by the davinci nand driver(u-boot and linux). Using rbl to load the u-boot to dram would mean that i have to use the TI's external flashing utility every time i upgrade u-boot. Also, i would not be able to flash u-boot from the kernel. -sughosh
> On Wed Jan 11, 2012 at 11:47:27AM +0100, Marek Vasut wrote: > > > On Tue Jan 10, 2012 at 09:07:58PM +0100, Marek Vasut wrote: > > > > > The current implementation invalidates the cache instead of > > > > > flushing it. This causes problems on platforms where the > > > > > spl/u-boot is already loaded to the RAM, with caches enabled by a > > > > > first stage bootloader. > > > > > > > > What platforms are affected? > > > > > > > It is causing a problem on the hawkboard, where the spl is loaded > > > directly to the RAM by a rom bootloader. We did not see this earlier > > > since cpu_init_crit was not getting called due to > > > CONFIG_SKIP_LOWLEVEL_INIT. > > > > > > -sughosh > > > > I see ... why don't you directly load U-Boot to DRAM then ? > > The rom bootloader(rbl) uses a different ecc layout from the one > used by the davinci nand driver(u-boot and linux). Can't you just tell the driver to use the correct ecc layout when flashing then? > Using rbl to load > the u-boot to dram would mean that i have to use the TI's external > flashing utility every time i upgrade u-boot. Not really, just adjust the NAND driver to handle this special case. > Also, i would not be > able to flash u-boot from the kernel. I think it's possible though, right? M > > -sughosh
On Wed Jan 11, 2012 at 01:42:38PM +0100, Marek Vasut wrote: > > On Wed Jan 11, 2012 at 11:47:27AM +0100, Marek Vasut wrote: > > > > On Tue Jan 10, 2012 at 09:07:58PM +0100, Marek Vasut wrote: > > > > > > The current implementation invalidates the cache instead of > > > > > > flushing it. This causes problems on platforms where the > > > > > > spl/u-boot is already loaded to the RAM, with caches enabled by a > > > > > > first stage bootloader. > > > > > > > > > > What platforms are affected? > > > > > > > > > It is causing a problem on the hawkboard, where the spl is loaded > > > > directly to the RAM by a rom bootloader. We did not see this earlier > > > > since cpu_init_crit was not getting called due to > > > > CONFIG_SKIP_LOWLEVEL_INIT. > > > > > > > > -sughosh > > > > > > I see ... why don't you directly load U-Boot to DRAM then ? > > > > The rom bootloader(rbl) uses a different ecc layout from the one > > used by the davinci nand driver(u-boot and linux). > > Can't you just tell the driver to use the correct ecc layout when > flashing then? Changing the ecc layout for a single board, hmm not sure. Using a spl instead does me no harm whatsoever -- I don't need to update the spl frequently in any case, and then can use the nand driver as is. -sughosh
> On Wed Jan 11, 2012 at 01:42:38PM +0100, Marek Vasut wrote: > > > On Wed Jan 11, 2012 at 11:47:27AM +0100, Marek Vasut wrote: > > > > > On Tue Jan 10, 2012 at 09:07:58PM +0100, Marek Vasut wrote: > > > > > > > The current implementation invalidates the cache instead of > > > > > > > flushing it. This causes problems on platforms where the > > > > > > > spl/u-boot is already loaded to the RAM, with caches enabled by > > > > > > > a first stage bootloader. > > > > > > > > > > > > What platforms are affected? > > > > > > > > > > > It is causing a problem on the hawkboard, where the spl is loaded > > > > > directly to the RAM by a rom bootloader. We did not see this > > > > > earlier since cpu_init_crit was not getting called due to > > > > > CONFIG_SKIP_LOWLEVEL_INIT. > > > > > > > > > > -sughosh > > > > > > > > I see ... why don't you directly load U-Boot to DRAM then ? > > > > > > > The rom bootloader(rbl) uses a different ecc layout from the one > > > used by the davinci nand driver(u-boot and linux). > > > > Can't you just tell the driver to use the correct ecc layout when > > flashing then? > > Changing the ecc layout for a single board, hmm not sure. Using a > spl instead does me no harm whatsoever -- I don't need to update the > spl frequently in any case, and then can use the nand driver as is. And how do you replace the SPL? Either way, I think the correct approach is to allow the driver to handle the SPL update too. M > > -sughosh
> > On Wed Jan 11, 2012 at 01:42:38PM +0100, Marek Vasut wrote: > > > > On Wed Jan 11, 2012 at 11:47:27AM +0100, Marek Vasut wrote: > > > > > > On Tue Jan 10, 2012 at 09:07:58PM +0100, Marek Vasut wrote: > > > > > > > > The current implementation invalidates the cache instead of > > > > > > > > flushing it. This causes problems on platforms where the > > > > > > > > spl/u-boot is already loaded to the RAM, with caches enabled > > > > > > > > by a first stage bootloader. > > > > > > > > > > > > > > What platforms are affected? > > > > > > > > > > > > > It is causing a problem on the hawkboard, where the spl is > > > > > > loaded directly to the RAM by a rom bootloader. We did not see > > > > > > this earlier since cpu_init_crit was not getting called due to > > > > > > CONFIG_SKIP_LOWLEVEL_INIT. > > > > > > > > > > > > -sughosh > > > > > > > > > > I see ... why don't you directly load U-Boot to DRAM then ? > > > > > > > > > The rom bootloader(rbl) uses a different ecc layout from the one > > > > used by the davinci nand driver(u-boot and linux). > > > > > > Can't you just tell the driver to use the correct ecc layout when > > > flashing then? > > > > > Changing the ecc layout for a single board, hmm not sure. Using a > > spl instead does me no harm whatsoever -- I don't need to update the > > spl frequently in any case, and then can use the nand driver as is. > > And how do you replace the SPL? > > Either way, I think the correct approach is to allow the driver to handle > the SPL update too. Or rather -- to be able to load u-boot directly. It seems to be more correct solution. The SPL you're using is just a workaround and also you need to TI flasher to replace it, right ? > > M > > > -sughosh
On Wed Jan 11, 2012 at 02:52:44PM +0100, Marek Vasut wrote: > > > Changing the ecc layout for a single board, hmm not sure. Using a > > > spl instead does me no harm whatsoever -- I don't need to update the > > > spl frequently in any case, and then can use the nand driver as is. > > > > And how do you replace the SPL? > > > > Either way, I think the correct approach is to allow the driver to handle > > the SPL update too. > > Or rather -- to be able to load u-boot directly. It seems to be more correct > solution. The SPL you're using is just a workaround and also you need to TI > flasher to replace it, right ? Yes, using the spl is a workaround, and like i mentioned in my previous mail, i thought having the spl was not a problem on the hawkboard. I had thought putting a new oob layout in the common davinci nand driver for a single board to be not a clean fix either. More so, given the fact that we don't have any control over rbl -- so if rbl changes it's layout for any subsequent board, we'd have to add that as well to the nand driver, and both in u-boot as well as the kernel. I guess the cleanest solution would have been for the rbl to have used the same layout as the one used by u-boot and linux. Btw, can you please review this change, that this patch fixes. ACK/NAK? -sughosh
> On Wed Jan 11, 2012 at 02:52:44PM +0100, Marek Vasut wrote: > > > > Changing the ecc layout for a single board, hmm not sure. Using a > > > > spl instead does me no harm whatsoever -- I don't need to update > > > > the spl frequently in any case, and then can use the nand driver > > > > as is. > > > > > > And how do you replace the SPL? > > > > > > Either way, I think the correct approach is to allow the driver to > > > handle the SPL update too. > > > > Or rather -- to be able to load u-boot directly. It seems to be more > > correct solution. The SPL you're using is just a workaround and also you > > need to TI flasher to replace it, right ? > > Yes, using the spl is a workaround, and like i mentioned in my > previous mail, i thought having the spl was not a problem on the > hawkboard. I had thought putting a new oob layout in the common > davinci nand driver for a single board to be not a clean fix > either. Then just allow the driver to be passed different OOB layout and make the current one a default > More so, given the fact that we don't have any control over > rbl -- so if rbl changes it's layout for any subsequent board, we'd > have to add that as well to the nand driver, and both in u-boot as > well as the kernel. > > I guess the cleanest solution would have been for the rbl to have > used the same layout as the one used by u-boot and linux. Yep, why not do that then? > > Btw, can you please review this change, that this patch > fixes. ACK/NAK? I'll think about it. It's ok, but it piles one workaround on top of another one, I don't like that approach. M > > -sughosh
On Wed Jan 11, 2012 at 04:01:31PM +0100, Marek Vasut wrote: > > More so, given the fact that we don't have any control over > > rbl -- so if rbl changes it's layout for any subsequent board, we'd > > have to add that as well to the nand driver, and both in u-boot as > > well as the kernel. > > > > I guess the cleanest solution would have been for the rbl to have > > used the same layout as the one used by u-boot and linux. > > Yep, why not do that then? Because rbl is a proprietary bootloader from TI. > > > > Btw, can you please review this change, that this patch > > fixes. ACK/NAK? > > I'll think about it. It's ok, but it piles one workaround on top of another one, > I don't like that approach. Flushing the data cache before disabling it is not a workaround, it's a fix. The current code is invalidating the cache instead of flushing it. -sughosh
> On Wed Jan 11, 2012 at 04:01:31PM +0100, Marek Vasut wrote: > > > More so, given the fact that we don't have any control over > > > rbl -- so if rbl changes it's layout for any subsequent board, we'd > > > have to add that as well to the nand driver, and both in u-boot as > > > well as the kernel. > > > > > > I guess the cleanest solution would have been for the rbl to have > > > used the same layout as the one used by u-boot and linux. > > > > Yep, why not do that then? > > Because rbl is a proprietary bootloader from TI. Don't we actually have replacement bootloader in uboot already ? You don't need xloader with uboot anymore I think. > > > > Btw, can you please review this change, that this patch > > > fixes. ACK/NAK? > > > > I'll think about it. It's ok, but it piles one workaround on top of > > another one, I don't like that approach. > > Flushing the data cache before disabling it is not a workaround, > it's a fix. The current code is invalidating the cache instead of > flushing it. Ok, but I'd like to see proper solution eventually. > > -sughosh
Hi, On Wednesday, January 11, 2012, Marek Vasut <marek.vasut@gmail.com> wrote: >> On Wed Jan 11, 2012 at 04:01:31PM +0100, Marek Vasut wrote: >> > > More so, given the fact that we don't have any control over >> > > rbl -- so if rbl changes it's layout for any subsequent board, we'd >> > > have to add that as well to the nand driver, and both in u-boot as >> > > well as the kernel. >> > > >> > > I guess the cleanest solution would have been for the rbl to have >> > > used the same layout as the one used by u-boot and linux. >> > >> > Yep, why not do that then? >> >> Because rbl is a proprietary bootloader from TI. > > Don't we actually have replacement bootloader in uboot already ? You don't need > xloader with uboot anymore I think. > RBL ist the ROM bootloader in the SoC, it's not only proprietary but also in ROM and hence cannot be changed. RBL executes an AIS script. Sughosh, could you please explain what your AIS does or how you create it? Regards, Christian
> Hi, > > On Wednesday, January 11, 2012, Marek Vasut <marek.vasut@gmail.com> wrote: > >> On Wed Jan 11, 2012 at 04:01:31PM +0100, Marek Vasut wrote: > >> > > More so, given the fact that we don't have any control over > >> > > rbl -- so if rbl changes it's layout for any subsequent board, > >> > > we'd have to add that as well to the nand driver, and both in > >> > > u-boot as well as the kernel. > >> > > > >> > > I guess the cleanest solution would have been for the rbl to have > >> > > used the same layout as the one used by u-boot and linux. > >> > > >> > Yep, why not do that then? > >> > > >> Because rbl is a proprietary bootloader from TI. > > > > Don't we actually have replacement bootloader in uboot already ? You > > don't need > > > xloader with uboot anymore I think. > > RBL ist the ROM bootloader in the SoC, it's not only proprietary but also > in ROM and hence cannot be changed. > > RBL executes an AIS script. Sughosh, could you please explain what your AIS > does or how you create it? So basically, this SPL business can be avoided and this all can be done in a standard way? M > > Regards, Christian
On Wednesday, January 11, 2012, Marek Vasut <marek.vasut@gmail.com> wrote: >> Hi, >> >> On Wednesday, January 11, 2012, Marek Vasut <marek.vasut@gmail.com> wrote: >> >> On Wed Jan 11, 2012 at 04:01:31PM +0100, Marek Vasut wrote: >> >> > > More so, given the fact that we don't have any control over >> >> > > rbl -- so if rbl changes it's layout for any subsequent board, >> >> > > we'd have to add that as well to the nand driver, and both in >> >> > > u-boot as well as the kernel. >> >> > > >> >> > > I guess the cleanest solution would have been for the rbl to have >> >> > > used the same layout as the one used by u-boot and linux. >> >> > >> >> > Yep, why not do that then? >> >> > >> >> Because rbl is a proprietary bootloader from TI. >> > >> > Don't we actually have replacement bootloader in uboot already ? You >> >> don't need >> >> > xloader with uboot anymore I think. >> >> RBL ist the ROM bootloader in the SoC, it's not only proprietary but also >> in ROM and hence cannot be changed. >> >> RBL executes an AIS script. Sughosh, could you please explain what your AIS >> does or how you create it? > > So basically, this SPL business can be avoided and this all can be done in a > standard way? I don't know, I never had to deal with booting from NAND. I was just wondering what Sughosh's AIS is doing that he gets these SPL problems. Christian
On Wed Jan 11, 2012 at 07:50:50PM +0100, Marek Vasut wrote: > > On Wed Jan 11, 2012 at 04:01:31PM +0100, Marek Vasut wrote: > > > > More so, given the fact that we don't have any control over > > > > rbl -- so if rbl changes it's layout for any subsequent board, we'd > > > > have to add that as well to the nand driver, and both in u-boot as > > > > well as the kernel. > > > > > > > > I guess the cleanest solution would have been for the rbl to have > > > > used the same layout as the one used by u-boot and linux. > > > > > > Yep, why not do that then? > > > > Because rbl is a proprietary bootloader from TI. > > Don't we actually have replacement bootloader in uboot already ? You don't need > xloader with uboot anymore I think. No, this rbl resides in the rom area on the SOC and cannot be done away with. > > > > > > Btw, can you please review this change, that this patch > > > > fixes. ACK/NAK? > > > > > > I'll think about it. It's ok, but it piles one workaround on top of > > > another one, I don't like that approach. > > > > Flushing the data cache before disabling it is not a workaround, > > it's a fix. The current code is invalidating the cache instead of > > flushing it. > > Ok, but I'd like to see proper solution eventually. I think patch holds true irrespective of whether we rbl boots spl, or u-boot. -sughosh
On Thu Jan 12, 2012 at 06:56:01AM +0100, Christian Riesch wrote: > On Wednesday, January 11, 2012, Marek Vasut <marek.vasut@gmail.com> wrote: <snip> > >> RBL executes an AIS script. Sughosh, could you please explain what your > AIS > >> does or how you create it? > > > > So basically, this SPL business can be avoided and this all can be done > in a > > standard way? > > I don't know, I never had to deal with booting from NAND. I was just > wondering what Sughosh's AIS is doing that he gets these SPL problems. > Christian I have checked my ais ini file, and it does the normal pll/ddr settings. I think it is the rbl which might be turning the cache ON. In any case, this patch holds true irrespective of whether rbl boots u-boot or spl. As for the question of doing away with spl and booting u-boot directly on hawkboard, i would want to stay with spl for now. I would check if we can pass the ecc layout to the nand driver in a clean elegant way, as Marek has suggested. -sughosh
Hi Sughosh, On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: > The current implementation invalidates the cache instead of flushing > it. This causes problems on platforms where the spl/u-boot is already > loaded to the RAM, with caches enabled by a first stage bootloader. > > The V bit of the cp15's control register c1 is set to the value of > VINITHI on reset. Do not clear this bit by default, as there are SOC's > with no valid memory region at 0x0. > > Signed-off-by: Sughosh Ganu <urwithsughosh@gmail.com> > Cc: Albert Aribaud <albert.u.boot@aribaud.net> > --- > > Changes since V1 > * Added arm926 keyword to the subject line > * Removed the superfluous setting of r0. > * Fixed the comment to reflect the fact that V is not being cleared I did a few tests of this patch with the da850evm (on an AM1808 experimenter's kit) and the calimain (on our custom board) configurations. I used u-boot from git://git.denx.de/u-boot-ti.git master, head is 14023683951b9a2bd277e999b0798b4917aca5d5. On top of this I applied these three patches: [U-Boot,1/2,V2] arm926: Flush the data cache before disabling it. http://patchwork.ozlabs.org/patch/135275/ [U-Boot,2/2,V4] Changes to move hawkboard to the new spl infrastructure http://patchwork.ozlabs.org/patch/135433/ [U-Boot,v2] arm, davinci: Add support for the Calimain board from OMICRON electronics http://patchwork.ozlabs.org/patch/135593/ First I compiled for the da850evm. make mrproper make da850evm_config make -j3 -s u-boot.ais I flashed the resulting u-boot.ais to the SPI flash of the AM1808 experimenter's kit: mono dvflashutils/OMAP-L138/GNU/sfh_OMAP-L138.exe -targetType AM1808 -flashType SPI_MEM -p /dev/ttyUSB0 -flash_noubl ../u-boot/u-boot.ais And tried to boot -> SUCCESS I also tried the TI way (using the ubl): mono dvflashutils/OMAP-L138/GNU/sfh_OMAP-L138.exe -targetType AM1808 -flashType SPI_MEM -p /dev/ttyUSB0 -flash dvflashutils/OMAP-L138/GNU/ubl/ubl_AM1808_SPI_MEM.bin ../u-boot/u-boot.bin And tried to boot -> SUCCESS Countercheck: I reverted patch "[U-Boot,1/2,V2] arm926: Flush the data cache before disabling it." and rebuilt git revert HEAD~2 make mrproper make da850evm_config make -j3 -s u-boot.ais Again I tried with ubl and without -> both worked -> SUCCESS Since you state that problems arise with the AIS scripts I also did a test loading u-boot with an AIS. This is my da850evm.ini [General] busWidth=8 BootMode=SPIMASTER crcCheckType=NO_CRC [PLLANDCLOCKCONFIG] PLL0CFG0 = 0x00180001 PLL0CFG1 = 0x00000205 PERIPHCLKCFG = 0x00000002 [EMIF3DDR] PLL1CFG0 = 0x15010001 PLL1CFG1 = 0x00000002 DDRPHYC1R = 0x000000C4 SDCR = 0x0A034622 SDTIMR = 0x184929C8 SDTIMR2 = 0xB80FC700 SDRCR = 0x00000406 CLK2XSRC = 0x00000000 [INPUTFILE] FILENAME=u-boot.bin LOADADDRESS=0xC1080000 ENTRYPOINTADDRESS=0xC1080000 I run mono dvflashutils/OMAP-L138/GNU/AISUtils/HexAIS_OMAP-L138.exe -ini da850evm.ini -o u-boot.ais program to flash and run -> SUCCESS I undo the revert git reset --hard HEAD^ and rebuild make mrproper make da850evm_config make -j3 -s u-boot.ais program and run -> SUCCESS Similar tests with the calimain board were also successful. Since all my tests were successful I wonder what issues did you have with the cache? Can you describe the problems you had? I think the difference is that you are booting from NAND and have an OMAP-L138, whereas I boot from SPI (on the da850evm) or from NOR (on calimain) and have an AM1808 on both boards, right? Regards, Christian
hi Christian, On Thu Jan 12, 2012 at 01:03:05PM +0100, Christian Riesch wrote: > Hi Sughosh, > > On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: > > The current implementation invalidates the cache instead of flushing > > it. This causes problems on platforms where the spl/u-boot is already > > loaded to the RAM, with caches enabled by a first stage bootloader. > > > > The V bit of the cp15's control register c1 is set to the value of > > VINITHI on reset. Do not clear this bit by default, as there are SOC's > > with no valid memory region at 0x0. > > > > Signed-off-by: Sughosh Ganu <urwithsughosh@gmail.com> > > Cc: Albert Aribaud <albert.u.boot@aribaud.net> > > --- > > > > Changes since V1 > > * Added arm926 keyword to the subject line > > * Removed the superfluous setting of r0. > > * Fixed the comment to reflect the fact that V is not being cleared > > I did a few tests of this patch with the da850evm (on an AM1808 > experimenter's kit) and the calimain (on our custom board) > configurations. > > I used u-boot from git://git.denx.de/u-boot-ti.git master, head is > 14023683951b9a2bd277e999b0798b4917aca5d5. On top of this I applied > these three patches: > > [U-Boot,1/2,V2] arm926: Flush the data cache before disabling it. > http://patchwork.ozlabs.org/patch/135275/ > > [U-Boot,2/2,V4] Changes to move hawkboard to the new spl infrastructure > http://patchwork.ozlabs.org/patch/135433/ > > [U-Boot,v2] arm, davinci: Add support for the Calimain board from > OMICRON electronics > http://patchwork.ozlabs.org/patch/135593/ > > First I compiled for the da850evm. > make mrproper > make da850evm_config > make -j3 -s u-boot.ais > > I flashed the resulting u-boot.ais to the SPI flash of the AM1808 > experimenter's kit: > mono dvflashutils/OMAP-L138/GNU/sfh_OMAP-L138.exe -targetType AM1808 > -flashType SPI_MEM -p /dev/ttyUSB0 -flash_noubl ../u-boot/u-boot.ais > And tried to boot -> SUCCESS > > I also tried the TI way (using the ubl): > mono dvflashutils/OMAP-L138/GNU/sfh_OMAP-L138.exe -targetType AM1808 > -flashType SPI_MEM -p /dev/ttyUSB0 -flash > dvflashutils/OMAP-L138/GNU/ubl/ubl_AM1808_SPI_MEM.bin > ../u-boot/u-boot.bin > And tried to boot -> SUCCESS > > Countercheck: I reverted patch "[U-Boot,1/2,V2] arm926: Flush the data > cache before disabling it." and rebuilt > git revert HEAD~2 > make mrproper > make da850evm_config > make -j3 -s u-boot.ais > > Again I tried with ubl and without -> both worked -> SUCCESS > > Since you state that problems arise with the AIS scripts I also did a > test loading u-boot with an AIS. <snip> > I run > mono dvflashutils/OMAP-L138/GNU/AISUtils/HexAIS_OMAP-L138.exe -ini > da850evm.ini -o u-boot.ais > program to flash and run -> SUCCESS > > I undo the revert > git reset --hard HEAD^ > and rebuild > make mrproper > make da850evm_config > make -j3 -s u-boot.ais > program and run -> SUCCESS > > Similar tests with the calimain board were also successful. > > Since all my tests were successful I wonder what issues did you have > with the cache? Can you describe the problems you had? I think the > difference is that you are booting from NAND and have an OMAP-L138, > whereas I boot from SPI (on the da850evm) or from NOR (on calimain) > and have an AM1808 on both boards, right? Thanks a lot for all the testing. One difference i think we have on these boards and hawkboard, is that on hawkboard, the rbl configures the memory and loads the target image(spl in this case) directly to the ram. Looking at da850evm's lds file, it looks like the spl gets loaded to the sram, configures dram and then copies u-boot to the target load address. I think the rbl is enabling the caches in case of hawkboard, and on loading spl, it causes a problem when the cache is invalidated instead of being flushed. I'm pretty sure this is the problem, as the hawkboard does not boot without this fix. Thanks for the time taken for doing such elaborate tests. -sughosh
On Thu, Jan 12, 2012 at 2:53 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: > On Thu Jan 12, 2012 at 01:03:05PM +0100, Christian Riesch wrote: >> On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: >> > The current implementation invalidates the cache instead of flushing >> > it. This causes problems on platforms where the spl/u-boot is already >> > loaded to the RAM, with caches enabled by a first stage bootloader. >> > >> > The V bit of the cp15's control register c1 is set to the value of >> > VINITHI on reset. Do not clear this bit by default, as there are SOC's >> > with no valid memory region at 0x0. >> > >> > Signed-off-by: Sughosh Ganu <urwithsughosh@gmail.com> >> > Cc: Albert Aribaud <albert.u.boot@aribaud.net> >> > --- >> > >> > Changes since V1 >> > * Added arm926 keyword to the subject line >> > * Removed the superfluous setting of r0. >> > * Fixed the comment to reflect the fact that V is not being cleared >> >> I did a few tests of this patch with the da850evm (on an AM1808 >> experimenter's kit) and the calimain (on our custom board) >> configurations. >> >> I used u-boot from git://git.denx.de/u-boot-ti.git master, head is >> 14023683951b9a2bd277e999b0798b4917aca5d5. On top of this I applied >> these three patches: >> >> [U-Boot,1/2,V2] arm926: Flush the data cache before disabling it. >> http://patchwork.ozlabs.org/patch/135275/ >> >> [U-Boot,2/2,V4] Changes to move hawkboard to the new spl infrastructure >> http://patchwork.ozlabs.org/patch/135433/ >> >> [U-Boot,v2] arm, davinci: Add support for the Calimain board from >> OMICRON electronics >> http://patchwork.ozlabs.org/patch/135593/ >> >> First I compiled for the da850evm. >> make mrproper >> make da850evm_config >> make -j3 -s u-boot.ais >> >> I flashed the resulting u-boot.ais to the SPI flash of the AM1808 >> experimenter's kit: >> mono dvflashutils/OMAP-L138/GNU/sfh_OMAP-L138.exe -targetType AM1808 >> -flashType SPI_MEM -p /dev/ttyUSB0 -flash_noubl ../u-boot/u-boot.ais >> And tried to boot -> SUCCESS >> >> I also tried the TI way (using the ubl): >> mono dvflashutils/OMAP-L138/GNU/sfh_OMAP-L138.exe -targetType AM1808 >> -flashType SPI_MEM -p /dev/ttyUSB0 -flash >> dvflashutils/OMAP-L138/GNU/ubl/ubl_AM1808_SPI_MEM.bin >> ../u-boot/u-boot.bin >> And tried to boot -> SUCCESS >> >> Countercheck: I reverted patch "[U-Boot,1/2,V2] arm926: Flush the data >> cache before disabling it." and rebuilt >> git revert HEAD~2 >> make mrproper >> make da850evm_config >> make -j3 -s u-boot.ais >> >> Again I tried with ubl and without -> both worked -> SUCCESS >> >> Since you state that problems arise with the AIS scripts I also did a >> test loading u-boot with an AIS. > > <snip> > >> I run >> mono dvflashutils/OMAP-L138/GNU/AISUtils/HexAIS_OMAP-L138.exe -ini >> da850evm.ini -o u-boot.ais >> program to flash and run -> SUCCESS >> >> I undo the revert >> git reset --hard HEAD^ >> and rebuild >> make mrproper >> make da850evm_config >> make -j3 -s u-boot.ais >> program and run -> SUCCESS >> >> Similar tests with the calimain board were also successful. >> >> Since all my tests were successful I wonder what issues did you have >> with the cache? Can you describe the problems you had? I think the >> difference is that you are booting from NAND and have an OMAP-L138, >> whereas I boot from SPI (on the da850evm) or from NOR (on calimain) >> and have an AM1808 on both boards, right? > > Thanks a lot for all the testing. One difference i think we have on > these boards and hawkboard, is that on hawkboard, the rbl configures > the memory and loads the target image(spl in this case) directly to > the ram. Looking at da850evm's lds file, it looks like the spl > gets loaded to the sram, configures dram and then copies u-boot to > the target load address. This is only true if the SPL is actually used. Have a closer look at my test report, I tested three different methods: 1) The first test was done with the SPL and yes, here the RBL loads the SPL into SRAM, initializes DDR memory and then copies u-boot.bin to DDR memory. 2) The second test was done with TI's UBL. Here, the RBL loads the UBL into SRAM, the UBL initializes DDR memory and then copies u-boot.bin to DDR memory. 3) The third test was done without SPL and without UBL: Here the DDR memory init is in the AIS, so in fact the RBL does memory initialization and then RBL loads u-boot.bin to DDR memory. This is the same case that you have on the hawkboard (only that you have the OMAP-L138 and NAND flash instead) and it works for me regardless of your patch. Christian > > I think the rbl is enabling the caches in case of hawkboard, and > on loading spl, it causes a problem when the cache is invalidated > instead of being flushed. I'm pretty sure this is the problem, as > the hawkboard does not boot without this fix. Thanks for the time > taken for doing such elaborate tests. > > -sughosh > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
hi Christian, On Thu Jan 12, 2012 at 03:04:37PM +0100, Christian Riesch wrote: > On Thu, Jan 12, 2012 at 2:53 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: > > On Thu Jan 12, 2012 at 01:03:05PM +0100, Christian Riesch wrote: <snip> > >> > >> Since all my tests were successful I wonder what issues did you have > >> with the cache? Can you describe the problems you had? I think the > >> difference is that you are booting from NAND and have an OMAP-L138, > >> whereas I boot from SPI (on the da850evm) or from NOR (on calimain) > >> and have an AM1808 on both boards, right? > > > > Thanks a lot for all the testing. One difference i think we have on > > these boards and hawkboard, is that on hawkboard, the rbl configures > > the memory and loads the target image(spl in this case) directly to > > the ram. Looking at da850evm's lds file, it looks like the spl > > gets loaded to the sram, configures dram and then copies u-boot to > > the target load address. > > This is only true if the SPL is actually used. Have a closer look at > my test report, I tested three different methods: > > 1) The first test was done with the SPL and yes, here the RBL loads > the SPL into SRAM, initializes DDR memory and then copies u-boot.bin > to DDR memory. > 2) The second test was done with TI's UBL. Here, the RBL loads the UBL > into SRAM, the UBL initializes DDR memory and then copies u-boot.bin > to DDR memory. > 3) The third test was done without SPL and without UBL: Here the DDR > memory init is in the AIS, so in fact the RBL does memory > initialization and then RBL loads u-boot.bin to DDR memory. This is > the same case that you have on the hawkboard (only that you have the > OMAP-L138 and NAND flash instead) and it works for me regardless of > your patch. Yes, the third case is similar to the one used in hawkboard. I'm not sure as to why it causes a problem on my board, though i'm not sure if we can compare the two cases, as we have different rbl's. It could be that the rbl used on hawkboard initialises the caches, as the caches are off by default on reset. Here are the values i use in my ini file for ddr init. [EMIF3DDR] PLL1CFG0 = 0x15010001 PLL1CFG1 = 0x00000002 DDRPHYC1R = 0x00000043 SDCR = 0x00134632 SDTIMR = 0x26492a09 SDTIMR2 = 0x7d13c722 SDRCR = 0x00000249 CLK2XSRC = 0x00000000 -sughosh
Hi Sughosh, I had a look at the patch and I tried to understand what's going on here (I must confess that I didn't know anything about this cache stuff). On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: > The current implementation invalidates the cache instead of flushing > it. This causes problems on platforms where the spl/u-boot is already > loaded to the RAM, with caches enabled by a first stage bootloader. > > The V bit of the cp15's control register c1 is set to the value of > VINITHI on reset. Do not clear this bit by default, as there are SOC's > with no valid memory region at 0x0. [...] > diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S > index 6a09c02..6e261c2 100644 > --- a/arch/arm/cpu/arm926ejs/start.S > +++ b/arch/arm/cpu/arm926ejs/start.S > @@ -355,17 +355,20 @@ _dynsym_start_ofs: > */ > cpu_init_crit: > /* > - * flush v4 I/D caches > + * flush D cache before disabling it > */ > mov r0, #0 > - mcr p15, 0, r0, c7, c7, 0 /* flush v3/v4 cache */ > +flush_dcache: > + mrc p15, 0, r15, c7, c10, 3 > + bne flush_dcache > + Ok, instead of invalidating the D-cache you are cleaning it. From the ARM926EJ-S Technical Reference Manual [1]: "To guarantee that memory coherency is maintained, the DCache must be cleaned of dirty data before it is disabled." So since we are disabling D-Cache a few lines later (bic r0, r0, #0x00000087), this must be the right way to do it. > mcr p15, 0, r0, c8, c7, 0 /* flush v4 TLB */ > > /* > * disable MMU stuff and caches > */ > mrc p15, 0, r0, c1, c0, 0 > - bic r0, r0, #0x00002300 /* clear bits 13, 9:8 (--V- --RS) */ > + bic r0, r0, #0x00000300 /* clear bits 9:8 ( --RS) */ Ok, I read your comment above. > bic r0, r0, #0x00000087 /* clear bits 7, 2:0 (B--- -CAM) */ > orr r0, r0, #0x00000002 /* set bit 2 (A) Align */ > orr r0, r0, #0x00001000 /* set bit 12 (I) I-Cache */ Although this is not changed in your patch, the last line makes me wonder. The comment says "disable MMU stuff and cached", but actually the last line sets bit 12 (I), which means that I-Cache gets enabled according to [1]. Regards, Christian [1] http://infocenter.arm.com/help/index.jsp
hi Christian, On Fri Jan 13, 2012 at 09:06:26AM +0100, Christian Riesch wrote: > Hi Sughosh, > I had a look at the patch and I tried to understand what's going on > here (I must confess that I didn't know anything about this cache > stuff). Ok, thanks for taking time off to understand this issue. > > On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: > > The current implementation invalidates the cache instead of flushing > > it. This causes problems on platforms where the spl/u-boot is already > > loaded to the RAM, with caches enabled by a first stage bootloader. > > > > The V bit of the cp15's control register c1 is set to the value of > > VINITHI on reset. Do not clear this bit by default, as there are SOC's > > with no valid memory region at 0x0. > [...] > > diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S > > index 6a09c02..6e261c2 100644 > > --- a/arch/arm/cpu/arm926ejs/start.S > > +++ b/arch/arm/cpu/arm926ejs/start.S > > @@ -355,17 +355,20 @@ _dynsym_start_ofs: > > */ > > cpu_init_crit: > > /* > > - * flush v4 I/D caches > > + * flush D cache before disabling it > > */ > > mov r0, #0 > > - mcr p15, 0, r0, c7, c7, 0 /* flush v3/v4 cache */ > > +flush_dcache: > > + mrc p15, 0, r15, c7, c10, 3 > > + bne flush_dcache > > + > > Ok, instead of invalidating the D-cache you are cleaning it. From the > ARM926EJ-S Technical Reference Manual [1]: "To guarantee that memory > coherency is maintained, the DCache must be cleaned of dirty data > before it is disabled." So since we are disabling D-Cache a few lines > later (bic r0, r0, #0x00000087), this must be the right way to do it. Right, so the problem here is that instead of flushing the cache, the code currently invalidates it. This would not be a problem on platforms where cache is not turned ON(which would be the majority of cases), but on my board, it seems to be turned ON by the rbl, due to which it does not boot. If you check the manual, the loop that i have introduced is one for "Testing and Cleaning", which means that in case the lines are not dirty, it won't be flushed. So this should not break any platforms where the cache isn't enabled. > > > mcr p15, 0, r0, c8, c7, 0 /* flush v4 TLB */ > > > > /* > > * disable MMU stuff and caches > > */ > > mrc p15, 0, r0, c1, c0, 0 > > - bic r0, r0, #0x00002300 /* clear bits 13, 9:8 (--V- --RS) */ > > + bic r0, r0, #0x00000300 /* clear bits 9:8 ( --RS) */ > > Ok, I read your comment above. > > > bic r0, r0, #0x00000087 /* clear bits 7, 2:0 (B--- -CAM) */ > > orr r0, r0, #0x00000002 /* set bit 2 (A) Align */ > > orr r0, r0, #0x00001000 /* set bit 12 (I) I-Cache */ > > Although this is not changed in your patch, the last line makes me > wonder. The comment says "disable MMU stuff and cached", but actually > the last line sets bit 12 (I), which means that I-Cache gets enabled > according to [1]. Yeah, this seems to be copied code, with discrepancies in the code and comments. You would see that the line i have removed has a comment for flushing the cache, but instead it is invalidating the cache. I have just fixed the comments for the lines that i made changes to. -sughosh
On Fri, Jan 13, 2012 at 1:26 AM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: > hi Christian, > > On Fri Jan 13, 2012 at 09:06:26AM +0100, Christian Riesch wrote: >> Hi Sughosh, >> I had a look at the patch and I tried to understand what's going on >> here (I must confess that I didn't know anything about this cache >> stuff). > > Ok, thanks for taking time off to understand this issue. > >> >> On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: >> > The current implementation invalidates the cache instead of flushing >> > it. This causes problems on platforms where the spl/u-boot is already >> > loaded to the RAM, with caches enabled by a first stage bootloader. >> > >> > The V bit of the cp15's control register c1 is set to the value of >> > VINITHI on reset. Do not clear this bit by default, as there are SOC's >> > with no valid memory region at 0x0. >> [...] >> > diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S >> > index 6a09c02..6e261c2 100644 >> > --- a/arch/arm/cpu/arm926ejs/start.S >> > +++ b/arch/arm/cpu/arm926ejs/start.S >> > @@ -355,17 +355,20 @@ _dynsym_start_ofs: >> > */ >> > cpu_init_crit: >> > /* >> > - * flush v4 I/D caches >> > + * flush D cache before disabling it >> > */ >> > mov r0, #0 >> > - mcr p15, 0, r0, c7, c7, 0 /* flush v3/v4 cache */ >> > +flush_dcache: >> > + mrc p15, 0, r15, c7, c10, 3 >> > + bne flush_dcache >> > + >> >> Ok, instead of invalidating the D-cache you are cleaning it. From the >> ARM926EJ-S Technical Reference Manual [1]: "To guarantee that memory >> coherency is maintained, the DCache must be cleaned of dirty data >> before it is disabled." So since we are disabling D-Cache a few lines >> later (bic r0, r0, #0x00000087), this must be the right way to do it. > > Right, so the problem here is that instead of flushing the cache, > the code currently invalidates it. This would not be a problem on > platforms where cache is not turned ON(which would be the majority > of cases), but on my board, it seems to be turned ON by the rbl, > due to which it does not boot. > > If you check the manual, the loop that i have introduced is one for > "Testing and Cleaning", which means that in case the lines are not > dirty, it won't be flushed. So this should not break any platforms > where the cache isn't enabled. > >> >> > mcr p15, 0, r0, c8, c7, 0 /* flush v4 TLB */ >> > >> > /* >> > * disable MMU stuff and caches >> > */ >> > mrc p15, 0, r0, c1, c0, 0 >> > - bic r0, r0, #0x00002300 /* clear bits 13, 9:8 (--V- --RS) */ >> > + bic r0, r0, #0x00000300 /* clear bits 9:8 ( --RS) */ >> >> Ok, I read your comment above. >> >> > bic r0, r0, #0x00000087 /* clear bits 7, 2:0 (B--- -CAM) */ >> > orr r0, r0, #0x00000002 /* set bit 2 (A) Align */ >> > orr r0, r0, #0x00001000 /* set bit 12 (I) I-Cache */ >> >> Although this is not changed in your patch, the last line makes me >> wonder. The comment says "disable MMU stuff and cached", but actually >> the last line sets bit 12 (I), which means that I-Cache gets enabled >> according to [1]. > > Yeah, this seems to be copied code, with discrepancies in the code > and comments. You would see that the line i have removed has a > comment for flushing the cache, but instead it is invalidating the > cache. I have just fixed the comments for the lines that i made > changes to. I think while we're in here and noticing these things we should fix the comments at least.
Hello Christian, Christian Riesch wrote: > Hi Sughosh, > I had a look at the patch and I tried to understand what's going on > here (I must confess that I didn't know anything about this cache > stuff). > > On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: >> The current implementation invalidates the cache instead of flushing >> it. This causes problems on platforms where the spl/u-boot is already >> loaded to the RAM, with caches enabled by a first stage bootloader. >> >> The V bit of the cp15's control register c1 is set to the value of >> VINITHI on reset. Do not clear this bit by default, as there are SOC's >> with no valid memory region at 0x0. > [...] >> diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S >> index 6a09c02..6e261c2 100644 >> --- a/arch/arm/cpu/arm926ejs/start.S >> +++ b/arch/arm/cpu/arm926ejs/start.S >> @@ -355,17 +355,20 @@ _dynsym_start_ofs: >> */ >> cpu_init_crit: >> /* >> - * flush v4 I/D caches >> + * flush D cache before disabling it >> */ >> mov r0, #0 >> - mcr p15, 0, r0, c7, c7, 0 /* flush v3/v4 cache */ >> +flush_dcache: >> + mrc p15, 0, r15, c7, c10, 3 >> + bne flush_dcache >> + > > Ok, instead of invalidating the D-cache you are cleaning it. From the > ARM926EJ-S Technical Reference Manual [1]: "To guarantee that memory > coherency is maintained, the DCache must be cleaned of dirty data > before it is disabled." So since we are disabling D-Cache a few lines > later (bic r0, r0, #0x00000087), this must be the right way to do it. Yes, thats sounds reasonable to me, Albert? >> mcr p15, 0, r0, c8, c7, 0 /* flush v4 TLB */ >> >> /* >> * disable MMU stuff and caches >> */ >> mrc p15, 0, r0, c1, c0, 0 >> - bic r0, r0, #0x00002300 /* clear bits 13, 9:8 (--V- --RS) */ >> + bic r0, r0, #0x00000300 /* clear bits 9:8 ( --RS) */ > > Ok, I read your comment above. Hmm.. what should we do with the V-Bit? Is it OK not to clear it for all cases? Tested this patch on the cam_enc_4xx and enbw_cmc board, works fine on that boards. > >> bic r0, r0, #0x00000087 /* clear bits 7, 2:0 (B--- -CAM) */ >> orr r0, r0, #0x00000002 /* set bit 2 (A) Align */ >> orr r0, r0, #0x00001000 /* set bit 12 (I) I-Cache */ > > Although this is not changed in your patch, the last line makes me > wonder. The comment says "disable MMU stuff and cached", but actually > the last line sets bit 12 (I), which means that I-Cache gets enabled > according to [1]. Yes, the last line enables the I-Cache. So we should at least add a better comment here. bye, Heiko
Hello Sugosh, Sughosh Ganu wrote: > hi Christian, > > On Fri Jan 13, 2012 at 09:06:26AM +0100, Christian Riesch wrote: >> Hi Sughosh, >> I had a look at the patch and I tried to understand what's going on >> here (I must confess that I didn't know anything about this cache >> stuff). > > Ok, thanks for taking time off to understand this issue. > >> On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: >>> The current implementation invalidates the cache instead of flushing >>> it. This causes problems on platforms where the spl/u-boot is already >>> loaded to the RAM, with caches enabled by a first stage bootloader. Hmm.. how did u-boot work on such boards? How can u-boot work with D-Cache enabled, if u-boot is not initializing it? (And I think, on davinci SoC we have a none working uboot ethernet driver if d-cache is enabled too). There must be a page_table in DRAM for using D-Cache in U-Boot, if u-boot don't initialize it, it maybe overrides it ... or miss I something? Are you sure, the RBL enables the D-Cache on your board? Nevertheless, I think, we must disable the D-Cache here with "cleaning" it (as your patch did) instead only invalidating it, as current code did. bye, Heiko
hi Heiko, On Fri Jan 13, 2012 at 04:06:22PM +0100, Heiko Schocher wrote: <snip> > > >> mcr p15, 0, r0, c8, c7, 0 /* flush v4 TLB */ > >> > >> /* > >> * disable MMU stuff and caches > >> */ > >> mrc p15, 0, r0, c1, c0, 0 > >> - bic r0, r0, #0x00002300 /* clear bits 13, 9:8 (--V- --RS) */ > >> + bic r0, r0, #0x00000300 /* clear bits 9:8 ( --RS) */ > > > > Ok, I read your comment above. > > Hmm.. what should we do with the V-Bit? Is it OK not to clear it for all > cases? The V bit gets set to the value of VINITHI input pin on reset, which is the default value. This setting clears the V bit by default, which shifts the vector table to 0x0. Certain SoC's(e.g omap-l138) don't have any valid memory at 0x0. Hence i think this setting should not be made here, but on a SOC level. > > Tested this patch on the cam_enc_4xx and enbw_cmc board, works fine > on that boards. > > > > >> bic r0, r0, #0x00000087 /* clear bits 7, 2:0 (B--- -CAM) */ > >> orr r0, r0, #0x00000002 /* set bit 2 (A) Align */ > >> orr r0, r0, #0x00001000 /* set bit 12 (I) I-Cache */ > > > > Although this is not changed in your patch, the last line makes me > > wonder. The comment says "disable MMU stuff and cached", but actually > > the last line sets bit 12 (I), which means that I-Cache gets enabled > > according to [1]. > > Yes, the last line enables the I-Cache. So we should at least add a > better comment here. I will fix the other comments too in the next version. -sughosh
On Fri Jan 13, 2012 at 07:41:37AM -0700, Tom Rini wrote: > On Fri, Jan 13, 2012 at 1:26 AM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: <snip> > >> > bic r0, r0, #0x00000087 /* clear bits 7, 2:0 (B--- -CAM) */ > >> > orr r0, r0, #0x00000002 /* set bit 2 (A) Align */ > >> > orr r0, r0, #0x00001000 /* set bit 12 (I) I-Cache */ > >> > >> Although this is not changed in your patch, the last line makes me > >> wonder. The comment says "disable MMU stuff and cached", but actually > >> the last line sets bit 12 (I), which means that I-Cache gets enabled > >> according to [1]. > > > > Yeah, this seems to be copied code, with discrepancies in the code > > and comments. You would see that the line i have removed has a > > comment for flushing the cache, but instead it is invalidating the > > cache. I have just fixed the comments for the lines that i made > > changes to. > > I think while we're in here and noticing these things we should fix > the comments at least. Will fix them in the next version. -sughosh
hi Heiko, On Fri Jan 13, 2012 at 04:29:29PM +0100, Heiko Schocher wrote: > Hello Sugosh, > > Sughosh Ganu wrote: > > hi Christian, > > > > On Fri Jan 13, 2012 at 09:06:26AM +0100, Christian Riesch wrote: > >> Hi Sughosh, > >> I had a look at the patch and I tried to understand what's going on > >> here (I must confess that I didn't know anything about this cache > >> stuff). > > > > Ok, thanks for taking time off to understand this issue. > > > >> On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: > >>> The current implementation invalidates the cache instead of flushing > >>> it. This causes problems on platforms where the spl/u-boot is already > >>> loaded to the RAM, with caches enabled by a first stage bootloader. > > Hmm.. how did u-boot work on such boards? How can u-boot work with D-Cache > enabled, if u-boot is not initializing it? (And I think, on davinci SoC > we have a none working uboot ethernet driver if d-cache is enabled too). > There must be a page_table in DRAM for using D-Cache in U-Boot, if u-boot > don't initialize it, it maybe overrides it ... or miss I something? Well, there is some data in the cache, which if not flushed creates problems on my board. I get the board to boot just by commenting out cpu_init_crit call. My hypothesis that the D-cache is enabled is simply because cache invalidation followed by cache disabling breaks the board, while flushing it prior to disabling gets it to boot fine. This(invalidation) would not have been a problem if the cache was in the disabled state. > > Are you sure, the RBL enables the D-Cache on your board? Nevertheless, > I think, we must disable the D-Cache here with "cleaning" it (as your > patch did) instead only invalidating it, as current code did. I am not sure, this is just my guess. By default, the caches are disabled on reset, so the only possible source is the rbl. But I don't have access to the rbl code to say for sure. Unfortunately i also don't have JTAG or any other debugger to dig more into this. But yes, like you mention, we must be cleaning the cache before disabling it, so this fix is correct. -sughosh
On Friday 13 January 2012 11:08 PM, Sughosh Ganu wrote: > hi Heiko, > > On Fri Jan 13, 2012 at 04:29:29PM +0100, Heiko Schocher wrote: >> Hello Sugosh, >> >> Sughosh Ganu wrote: >>> hi Christian, >>> >>> On Fri Jan 13, 2012 at 09:06:26AM +0100, Christian Riesch wrote: >>>> Hi Sughosh, >>>> I had a look at the patch and I tried to understand what's going on >>>> here (I must confess that I didn't know anything about this cache >>>> stuff). >>> >>> Ok, thanks for taking time off to understand this issue. >>> >>>> On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu<urwithsughosh@gmail.com> wrote: >>>>> The current implementation invalidates the cache instead of flushing >>>>> it. This causes problems on platforms where the spl/u-boot is already >>>>> loaded to the RAM, with caches enabled by a first stage bootloader. >> >> Hmm.. how did u-boot work on such boards? How can u-boot work with D-Cache >> enabled, if u-boot is not initializing it? (And I think, on davinci SoC >> we have a none working uboot ethernet driver if d-cache is enabled too). >> There must be a page_table in DRAM for using D-Cache in U-Boot, if u-boot >> don't initialize it, it maybe overrides it ... or miss I something? > > Well, there is some data in the cache, which if not flushed creates > problems on my board. I get the board to boot just by commenting out > cpu_init_crit call. My hypothesis that the D-cache is enabled is > simply because cache invalidation followed by cache disabling breaks > the board, while flushing it prior to disabling gets it to boot > fine. This(invalidation) would not have been a problem if the cache > was in the disabled state. It would have affected if the state of dirty bits and valid bits are not guaranteed at reset. But looks like cache entries are guaranteed to be invalidated on reset in ARM926ejs per the spec (which is not the case in ARMv7). In this case, I agree that flush shouldn't harm anything ideally. > >> >> Are you sure, the RBL enables the D-Cache on your board? Nevertheless, >> I think, we must disable the D-Cache here with "cleaning" it (as your >> patch did) instead only invalidating it, as current code did. > > I am not sure, this is just my guess. By default, the caches are > disabled on reset, so the only possible source is the rbl. But I > don't have access to the rbl code to say for sure. Unfortunately i > also don't have JTAG or any other debugger to dig more into I will be surprised too if D-cache is enabled by ROM code. But I agree that your problem and the solution seems to indicate something like that. To be sure can you check the value of CP15 System control register on SPL entry? You are already reading it. Can you save it somewhere and spit it out later? > this. But yes, like you mention, we must be cleaning the cache > before disabling it, so this fix is correct. I think it will be good to add an invalidate of I-cache too. You have replaced an invalidate of I & D cache with a flush of only D-cache. br, Aneesh
On Fri Jan 13, 2012 at 11:49:57PM +0530, Aneesh V wrote: > On Friday 13 January 2012 11:08 PM, Sughosh Ganu wrote: <snip> > >> > >>Are you sure, the RBL enables the D-Cache on your board? Nevertheless, > >>I think, we must disable the D-Cache here with "cleaning" it (as your > >>patch did) instead only invalidating it, as current code did. > > > > I am not sure, this is just my guess. By default, the caches are > > disabled on reset, so the only possible source is the rbl. But I > > don't have access to the rbl code to say for sure. Unfortunately i > > also don't have JTAG or any other debugger to dig more into > > I will be surprised too if D-cache is enabled by ROM code. But I agree > that your problem and the solution seems to indicate something like > that. To be sure can you check the value of CP15 System control > register on SPL entry? You are already reading it. Can you save it > somewhere and spit it out later? Yes, i can try this out. Spitting it out as is won't be straightforward given the limited resources we have with spl(no printf). I'll see if i can pass the value to board_init_f, where i can check for it, and output some debug message. > > > this. But yes, like you mention, we must be cleaning the cache > > before disabling it, so this fix is correct. > > I think it will be good to add an invalidate of I-cache too. You have > replaced an invalidate of I & D cache with a flush of only D-cache. Ok, will add invalidation of the I cache. -sughosh
Le 12/01/2012 07:29, Sughosh Ganu a écrit : > On Thu Jan 12, 2012 at 06:56:01AM +0100, Christian Riesch wrote: >> On Wednesday, January 11, 2012, Marek Vasut<marek.vasut@gmail.com> wrote: > > <snip> > >>>> RBL executes an AIS script. Sughosh, could you please explain what your >> AIS >>>> does or how you create it? >>> >>> So basically, this SPL business can be avoided and this all can be done >> in a >>> standard way? >> >> I don't know, I never had to deal with booting from NAND. I was just >> wondering what Sughosh's AIS is doing that he gets these SPL problems. >> Christian > > I have checked my ais ini file, and it does the normal pll/ddr > settings. I think it is the rbl which might be turning the cache > ON. I do understand it is ROM code so no change can be done to it, but that a bootloader pass control to its payload with the cache still enabled and, worse yet, dirty, is Bad(tm). Can the AIS not be augmented with instructions to flush and disable the cache? Note: I do NOT intend to reject the U-Boot patch if the AIS can indeed be modified; I am just trying to apply the Postel principe fully, and while the patch would make U-Boot be (more) liberal in what it receives from the ROM bootloader, I would like the bootloader to be (more) conservative in what it gives U-Boot, i.e. give it a clean system with as little assumptions to make or constraints to respect. Amicalement,
Hi Albert, On Saturday, January 14, 2012, Albert ARIBAUD <albert.u.boot@aribaud.net> wrote: > Le 12/01/2012 07:29, Sughosh Ganu a écrit : >> >> On Thu Jan 12, 2012 at 06:56:01AM +0100, Christian Riesch wrote: >>> >>> On Wednesday, January 11, 2012, Marek Vasut<marek.vasut@gmail.com> wrote: >> >> <snip> >> >>>>> RBL executes an AIS script. Sughosh, could you please explain what your >>> >>> AIS >>>>> >>>>> does or how you create it? >>>> >>>> So basically, this SPL business can be avoided and this all can be done >>> >>> in a >>>> >>>> standard way? >>> >>> I don't know, I never had to deal with booting from NAND. I was just >>> wondering what Sughosh's AIS is doing that he gets these SPL problems. >>> Christian >> >> I have checked my ais ini file, and it does the normal pll/ddr >> settings. I think it is the rbl which might be turning the cache >> ON. > > I do understand it is ROM code so no change can be done to it, but that a bootloader pass control to its payload with the cache still enabled and, worse yet, dirty, is Bad(tm). > > Can the AIS not be augmented with instructions to flush and disable the cache? I checked the AIS documentation and I don't think this can be done... AIS has a command to write arbitrary memory regions, but not registers. Christian
Hi Sughosh, On Thursday, January 12, 2012, Sughosh Ganu <urwithsughosh@gmail.com> wrote: > hi Christian, > > On Thu Jan 12, 2012 at 03:04:37PM +0100, Christian Riesch wrote: >> On Thu, Jan 12, 2012 at 2:53 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: >> > On Thu Jan 12, 2012 at 01:03:05PM +0100, Christian Riesch wrote: > > <snip> > >> >> >> >> Since all my tests were successful I wonder what issues did you have >> >> with the cache? Can you describe the problems you had? I think the >> >> difference is that you are booting from NAND and have an OMAP-L138, >> >> whereas I boot from SPI (on the da850evm) or from NOR (on calimain) >> >> and have an AM1808 on both boards, right? >> > >> > Thanks a lot for all the testing. One difference i think we have on >> > these boards and hawkboard, is that on hawkboard, the rbl configures >> > the memory and loads the target image(spl in this case) directly to >> > the ram. Looking at da850evm's lds file, it looks like the spl >> > gets loaded to the sram, configures dram and then copies u-boot to >> > the target load address. >> >> This is only true if the SPL is actually used. Have a closer look at >> my test report, I tested three different methods: >> >> 1) The first test was done with the SPL and yes, here the RBL loads >> the SPL into SRAM, initializes DDR memory and then copies u-boot.bin >> to DDR memory. >> 2) The second test was done with TI's UBL. Here, the RBL loads the UBL >> into SRAM, the UBL initializes DDR memory and then copies u-boot.bin >> to DDR memory. >> 3) The third test was done without SPL and without UBL: Here the DDR >> memory init is in the AIS, so in fact the RBL does memory >> initialization and then RBL loads u-boot.bin to DDR memory. This is >> the same case that you have on the hawkboard (only that you have the >> OMAP-L138 and NAND flash instead) and it works for me regardless of >> your patch. > > Yes, the third case is similar to the one used in hawkboard. I'm not > sure as to why it causes a problem on my board, though i'm not sure > if we can compare the two cases, as we have different rbl's. It > could be that the rbl used on hawkboard initialises the caches, as > the caches are off by default on reset. > > Here are the values i use in my ini file for ddr init. > > [EMIF3DDR] > PLL1CFG0 = 0x15010001 > PLL1CFG1 = 0x00000002 > > DDRPHYC1R = 0x00000043 > SDCR = 0x00134632 > SDTIMR = 0x26492a09 > SDTIMR2 = 0x7d13c722 > SDRCR = 0x00000249 > CLK2XSRC = 0x00000000 > Just for curiosity, could you please send the full ini file? Thanks, Christian
hi Christian, On Sat Jan 14, 2012 at 06:20:06PM +0100, Christian Riesch wrote: > Hi Sughosh, <snip> > On Thursday, January 12, 2012, Sughosh Ganu <urwithsughosh@gmail.com> wrote: > >> 1) The first test was done with the SPL and yes, here the RBL loads > >> the SPL into SRAM, initializes DDR memory and then copies u-boot.bin > >> to DDR memory. > >> 2) The second test was done with TI's UBL. Here, the RBL loads the UBL > >> into SRAM, the UBL initializes DDR memory and then copies u-boot.bin > >> to DDR memory. > >> 3) The third test was done without SPL and without UBL: Here the DDR > >> memory init is in the AIS, so in fact the RBL does memory > >> initialization and then RBL loads u-boot.bin to DDR memory. This is > >> the same case that you have on the hawkboard (only that you have the > >> OMAP-L138 and NAND flash instead) and it works for me regardless of > >> your patch. > > > > Yes, the third case is similar to the one used in hawkboard. I'm not > > sure as to why it causes a problem on my board, though i'm not sure > > if we can compare the two cases, as we have different rbl's. It > > could be that the rbl used on hawkboard initialises the caches, as > > the caches are off by default on reset. > > > > Here are the values i use in my ini file for ddr init. > > > > [EMIF3DDR] > > PLL1CFG0 = 0x15010001 > > PLL1CFG1 = 0x00000002 > > > > DDRPHYC1R = 0x00000043 > > SDCR = 0x00134632 > > SDTIMR = 0x26492a09 > > SDTIMR2 = 0x7d13c722 > > SDRCR = 0x00000249 > > CLK2XSRC = 0x00000000 Here it is. [General] busWidth=8 BootMode=NAND crcCheckType=NO_CRC [PLL0CONFIG] PLL0CFG0 = 0x00180001 PLL0CFG1 = 0x00000205 [EMIF3DDR] PLL1CFG0 = 0x15010001 PLL1CFG1 = 0x00000002 DDRPHYC1R = 0x00000043 SDCR = 0x00134632 SDTIMR = 0x26492a09 SDTIMR2 = 0x7d13c722 SDRCR = 0x00000249 CLK2XSRC = 0x00000000 [ARM_EMIF3DDR_PATCHFXN] -sughosh
Hello Sughosh, Sughosh Ganu wrote: > hi Heiko, > > On Fri Jan 13, 2012 at 04:29:29PM +0100, Heiko Schocher wrote: >> Hello Sugosh, >> >> Sughosh Ganu wrote: >>> hi Christian, >>> >>> On Fri Jan 13, 2012 at 09:06:26AM +0100, Christian Riesch wrote: >>>> Hi Sughosh, >>>> I had a look at the patch and I tried to understand what's going on >>>> here (I must confess that I didn't know anything about this cache >>>> stuff). >>> Ok, thanks for taking time off to understand this issue. >>> >>>> On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: >>>>> The current implementation invalidates the cache instead of flushing >>>>> it. This causes problems on platforms where the spl/u-boot is already >>>>> loaded to the RAM, with caches enabled by a first stage bootloader. >> Hmm.. how did u-boot work on such boards? How can u-boot work with D-Cache >> enabled, if u-boot is not initializing it? (And I think, on davinci SoC >> we have a none working uboot ethernet driver if d-cache is enabled too). >> There must be a page_table in DRAM for using D-Cache in U-Boot, if u-boot >> don't initialize it, it maybe overrides it ... or miss I something? > > Well, there is some data in the cache, which if not flushed creates > problems on my board. I get the board to boot just by commenting out > cpu_init_crit call. My hypothesis that the D-cache is enabled is That is what I not understand, because, if the RBL really enables the D-Cache, how could u-boot work, if it not disables it! > simply because cache invalidation followed by cache disabling breaks > the board, while flushing it prior to disabling gets it to boot > fine. This(invalidation) would not have been a problem if the cache > was in the disabled state. > >> Are you sure, the RBL enables the D-Cache on your board? Nevertheless, >> I think, we must disable the D-Cache here with "cleaning" it (as your >> patch did) instead only invalidating it, as current code did. > > I am not sure, this is just my guess. By default, the caches are > disabled on reset, so the only possible source is the rbl. But I > don't have access to the rbl code to say for sure. Unfortunately i > also don't have JTAG or any other debugger to dig more into :-( You maybe could read the interesting values, and store them some- where, and print it later? > this. But yes, like you mention, we must be cleaning the cache > before disabling it, so this fix is correct. Yes, the fix is correct, but we should try to understand, what is really the problem on your hw! bye, Heiko
On Fri, Jan 13, 2012 at 10:38 AM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: > hi Heiko, > > On Fri Jan 13, 2012 at 04:29:29PM +0100, Heiko Schocher wrote: >> Hello Sugosh, >> >> Sughosh Ganu wrote: >> > hi Christian, >> > >> > On Fri Jan 13, 2012 at 09:06:26AM +0100, Christian Riesch wrote: >> >> Hi Sughosh, >> >> I had a look at the patch and I tried to understand what's going on >> >> here (I must confess that I didn't know anything about this cache >> >> stuff). >> > >> > Ok, thanks for taking time off to understand this issue. >> > >> >> On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: >> >>> The current implementation invalidates the cache instead of flushing >> >>> it. This causes problems on platforms where the spl/u-boot is already >> >>> loaded to the RAM, with caches enabled by a first stage bootloader. >> >> Hmm.. how did u-boot work on such boards? How can u-boot work with D-Cache >> enabled, if u-boot is not initializing it? (And I think, on davinci SoC >> we have a none working uboot ethernet driver if d-cache is enabled too). >> There must be a page_table in DRAM for using D-Cache in U-Boot, if u-boot >> don't initialize it, it maybe overrides it ... or miss I something? > > Well, there is some data in the cache, which if not flushed creates > problems on my board. I get the board to boot just by commenting out > cpu_init_crit call. My hypothesis that the D-cache is enabled is > simply because cache invalidation followed by cache disabling breaks > the board, while flushing it prior to disabling gets it to boot > fine. This(invalidation) would not have been a problem if the cache > was in the disabled state. Putting my TI hat on, I've confirmed with the RBL folks that they aren't turning on ICACHE/DCACHE.
Hello Tom, Tom Rini wrote: > On Fri, Jan 13, 2012 at 10:38 AM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: >> hi Heiko, >> >> On Fri Jan 13, 2012 at 04:29:29PM +0100, Heiko Schocher wrote: >>> Hello Sugosh, >>> >>> Sughosh Ganu wrote: >>>> hi Christian, >>>> >>>> On Fri Jan 13, 2012 at 09:06:26AM +0100, Christian Riesch wrote: >>>>> Hi Sughosh, >>>>> I had a look at the patch and I tried to understand what's going on >>>>> here (I must confess that I didn't know anything about this cache >>>>> stuff). >>>> Ok, thanks for taking time off to understand this issue. >>>> >>>>> On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: >>>>>> The current implementation invalidates the cache instead of flushing >>>>>> it. This causes problems on platforms where the spl/u-boot is already >>>>>> loaded to the RAM, with caches enabled by a first stage bootloader. >>> Hmm.. how did u-boot work on such boards? How can u-boot work with D-Cache >>> enabled, if u-boot is not initializing it? (And I think, on davinci SoC >>> we have a none working uboot ethernet driver if d-cache is enabled too). >>> There must be a page_table in DRAM for using D-Cache in U-Boot, if u-boot >>> don't initialize it, it maybe overrides it ... or miss I something? >> Well, there is some data in the cache, which if not flushed creates >> problems on my board. I get the board to boot just by commenting out >> cpu_init_crit call. My hypothesis that the D-cache is enabled is >> simply because cache invalidation followed by cache disabling breaks >> the board, while flushing it prior to disabling gets it to boot >> fine. This(invalidation) would not have been a problem if the cache >> was in the disabled state. > > Putting my TI hat on, I've confirmed with the RBL folks that they > aren't turning on ICACHE/DCACHE. That was my thought too, thanks for the clarification! bye, Heiko
On Mon Jan 16, 2012 at 10:57:05AM -0700, Tom Rini wrote: > On Fri, Jan 13, 2012 at 10:38 AM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: > > hi Heiko, > > > > On Fri Jan 13, 2012 at 04:29:29PM +0100, Heiko Schocher wrote: > >> Hello Sugosh, > >> > >> Sughosh Ganu wrote: > >> > hi Christian, > >> > > >> > On Fri Jan 13, 2012 at 09:06:26AM +0100, Christian Riesch wrote: > >> >> Hi Sughosh, > >> >> I had a look at the patch and I tried to understand what's going on > >> >> here (I must confess that I didn't know anything about this cache > >> >> stuff). > >> > > >> > Ok, thanks for taking time off to understand this issue. > >> > > >> >> On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: > >> >>> The current implementation invalidates the cache instead of flushing > >> >>> it. This causes problems on platforms where the spl/u-boot is already > >> >>> loaded to the RAM, with caches enabled by a first stage bootloader. > >> > >> Hmm.. how did u-boot work on such boards? How can u-boot work with D-Cache > >> enabled, if u-boot is not initializing it? (And I think, on davinci SoC > >> we have a none working uboot ethernet driver if d-cache is enabled too). > >> There must be a page_table in DRAM for using D-Cache in U-Boot, if u-boot > >> don't initialize it, it maybe overrides it ... or miss I something? > > > > Well, there is some data in the cache, which if not flushed creates > > problems on my board. I get the board to boot just by commenting out > > cpu_init_crit call. My hypothesis that the D-cache is enabled is > > simply because cache invalidation followed by cache disabling breaks > > the board, while flushing it prior to disabling gets it to boot > > fine. This(invalidation) would not have been a problem if the cache > > was in the disabled state. > > Putting my TI hat on, I've confirmed with the RBL folks that they > aren't turning on ICACHE/DCACHE. Well, i'm not sure then why does the cache invalidation cause a problem on my platform. Btw, will this patch be accepted. Irrespective of the cache state on my board, it is not a wrong fix, and moreover results in the booting of my board. I haven't yet tried out reading the cache bit state in the spl. Will try out this experiment in a day or two, and share the results. -sughosh
On Mon, Jan 16, 2012 at 11:46 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: > On Mon Jan 16, 2012 at 10:57:05AM -0700, Tom Rini wrote: >> On Fri, Jan 13, 2012 at 10:38 AM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: >> > hi Heiko, >> > >> > On Fri Jan 13, 2012 at 04:29:29PM +0100, Heiko Schocher wrote: >> >> Hello Sugosh, >> >> >> >> Sughosh Ganu wrote: >> >> > hi Christian, >> >> > >> >> > On Fri Jan 13, 2012 at 09:06:26AM +0100, Christian Riesch wrote: >> >> >> Hi Sughosh, >> >> >> I had a look at the patch and I tried to understand what's going on >> >> >> here (I must confess that I didn't know anything about this cache >> >> >> stuff). >> >> > >> >> > Ok, thanks for taking time off to understand this issue. >> >> > >> >> >> On Tue, Jan 10, 2012 at 7:12 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: >> >> >>> The current implementation invalidates the cache instead of flushing >> >> >>> it. This causes problems on platforms where the spl/u-boot is already >> >> >>> loaded to the RAM, with caches enabled by a first stage bootloader. >> >> >> >> Hmm.. how did u-boot work on such boards? How can u-boot work with D-Cache >> >> enabled, if u-boot is not initializing it? (And I think, on davinci SoC >> >> we have a none working uboot ethernet driver if d-cache is enabled too). >> >> There must be a page_table in DRAM for using D-Cache in U-Boot, if u-boot >> >> don't initialize it, it maybe overrides it ... or miss I something? >> > >> > Well, there is some data in the cache, which if not flushed creates >> > problems on my board. I get the board to boot just by commenting out >> > cpu_init_crit call. My hypothesis that the D-cache is enabled is >> > simply because cache invalidation followed by cache disabling breaks >> > the board, while flushing it prior to disabling gets it to boot >> > fine. This(invalidation) would not have been a problem if the cache >> > was in the disabled state. >> >> Putting my TI hat on, I've confirmed with the RBL folks that they >> aren't turning on ICACHE/DCACHE. > > Well, i'm not sure then why does the cache invalidation cause a > problem on my platform. Btw, will this patch be > accepted. Irrespective of the cache state on my board, it is not a > wrong fix, and moreover results in the booting of my board. > > I haven't yet tried out reading the cache bit state in the spl. Will > try out this experiment in a day or two, and share the results. I think someone needs to look over this init code very carefully. If I can summarize from memory quickly, when we enable the CP_CRITICAL_INITS code for everyone that exposed a problem on the hawkboard that _looks_like_ DCACHE is enabled by RBL as changing the code from doing an invalidate to a flush+invalidate fixes a problem. But the manual says we should be doing flush+invalidate anyhow and the RBL code is not turning on DCACHE. Maybe we've got something else being done not as the manual says and that's tickling another issue?
On Tue Jan 17, 2012 at 08:27:58AM -0700, Tom Rini wrote: > On Mon, Jan 16, 2012 at 11:46 PM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: > >> >> Hmm.. how did u-boot work on such boards? How can u-boot work with D-Cache > >> >> enabled, if u-boot is not initializing it? (And I think, on davinci SoC > >> >> we have a none working uboot ethernet driver if d-cache is enabled too). > >> >> There must be a page_table in DRAM for using D-Cache in U-Boot, if u-boot > >> >> don't initialize it, it maybe overrides it ... or miss I something? > >> > > >> > Well, there is some data in the cache, which if not flushed creates > >> > problems on my board. I get the board to boot just by commenting out > >> > cpu_init_crit call. My hypothesis that the D-cache is enabled is > >> > simply because cache invalidation followed by cache disabling breaks > >> > the board, while flushing it prior to disabling gets it to boot > >> > fine. This(invalidation) would not have been a problem if the cache > >> > was in the disabled state. > >> > >> Putting my TI hat on, I've confirmed with the RBL folks that they > >> aren't turning on ICACHE/DCACHE. > > > > Well, i'm not sure then why does the cache invalidation cause a > > problem on my platform. Btw, will this patch be > > accepted. Irrespective of the cache state on my board, it is not a > > wrong fix, and moreover results in the booting of my board. > > > > I haven't yet tried out reading the cache bit state in the spl. Will > > try out this experiment in a day or two, and share the results. > > I think someone needs to look over this init code very carefully. If > I can summarize from memory quickly, when we enable the > CP_CRITICAL_INITS code for everyone that exposed a problem on the > hawkboard that _looks_like_ DCACHE is enabled by RBL as changing the > code from doing an invalidate to a flush+invalidate fixes a problem. > But the manual says we should be doing flush+invalidate anyhow and the > RBL code is not turning on DCACHE. Maybe we've got something else > being done not as the manual says and that's tickling another issue? Tried a few things on my end. * Read the D-cache value in the spl, and confirmed that the data cache is indeed not enabled. * Enabled the data cache explicitly in cpu_init_crit, and booted u-boot. u-boot comes up fine, and trying a ping switches off the data cache with the warning message. So it definitely looks like the cache is not enabled. But still not able to figure out as to why does the flushing operation help. Really need to get a debugger :) -sughosh
Hi Sughosh, On Thursday 19 January 2012 12:23 PM, Sughosh Ganu wrote: > On Tue Jan 17, 2012 at 08:27:58AM -0700, Tom Rini wrote: >> On Mon, Jan 16, 2012 at 11:46 PM, Sughosh Ganu<urwithsughosh@gmail.com> wrote: > >>>>>> Hmm.. how did u-boot work on such boards? How can u-boot work with D-Cache >>>>>> enabled, if u-boot is not initializing it? (And I think, on davinci SoC >>>>>> we have a none working uboot ethernet driver if d-cache is enabled too). >>>>>> There must be a page_table in DRAM for using D-Cache in U-Boot, if u-boot >>>>>> don't initialize it, it maybe overrides it ... or miss I something? >>>>> >>>>> Well, there is some data in the cache, which if not flushed creates >>>>> problems on my board. I get the board to boot just by commenting out >>>>> cpu_init_crit call. My hypothesis that the D-cache is enabled is >>>>> simply because cache invalidation followed by cache disabling breaks >>>>> the board, while flushing it prior to disabling gets it to boot >>>>> fine. This(invalidation) would not have been a problem if the cache >>>>> was in the disabled state. >>>> >>>> Putting my TI hat on, I've confirmed with the RBL folks that they >>>> aren't turning on ICACHE/DCACHE. >>> >>> Well, i'm not sure then why does the cache invalidation cause a >>> problem on my platform. Btw, will this patch be >>> accepted. Irrespective of the cache state on my board, it is not a >>> wrong fix, and moreover results in the booting of my board. >>> >>> I haven't yet tried out reading the cache bit state in the spl. Will >>> try out this experiment in a day or two, and share the results. >> >> I think someone needs to look over this init code very carefully. If >> I can summarize from memory quickly, when we enable the >> CP_CRITICAL_INITS code for everyone that exposed a problem on the >> hawkboard that _looks_like_ DCACHE is enabled by RBL as changing the >> code from doing an invalidate to a flush+invalidate fixes a problem. >> But the manual says we should be doing flush+invalidate anyhow and the >> RBL code is not turning on DCACHE. Maybe we've got something else >> being done not as the manual says and that's tickling another issue? > > Tried a few things on my end. > * Read the D-cache value in the spl, and confirmed that the data > cache is indeed not enabled. What is the value of the B bit in CP15 SCR register? I wonder if RBL is doing all the IMB operations required after copying the SPL image and before executing it. IMB is required for consistency between data and instruction sides. For instance after copying the SPL, the memory and instruction cache could be incoherent. So, instruction cache needs to be invalidated. In fact more needs to be done and there is an entire chapter in the ARM926EJS TRM that discusses this (Chapter 9 - Instruction Memory Barrier). Here is the key excerpt: 9.2 IMB operation To ensure consistency between data and instruction sides, you must take the following steps: 1. Clean the DCache 2. Drain the write buffer 3. Synchronize data and instruction streams in level two AHB subsystems 4. Invalidate the ICache on page 9-4 5. Flush the prefetch buffer on page 9-4. Ideally RBL should be doing all this before jumping to SPL. But, I wonder if anything is missing in RBL and was getting done as a side-effect of your flush. Just a thought. Do you care to try 2-5 in SPL and see if it makes any difference?(and avoid 1. in fact 1 seems to be the least useful thing in our case!) > > * Enabled the data cache explicitly in cpu_init_crit, and booted > u-boot. u-boot comes up fine, and trying a ping switches off the > data cache with the warning message. How are you enabling D-cache. Please note that just setting C bit doesn't enable D-cache. MMU needs to be enabled (M bit) for D-cache to be enabled. MMU in turn needs page-table. I wonder if you are doing all this in cpu_init_crit()? br, Aneesh
Hi Aneesh, On Thu, Jan 19, 2012 at 11:17 AM, Aneesh V <aneesh@ti.com> wrote: > On Thursday 19 January 2012 12:23 PM, Sughosh Ganu wrote: >> Tried a few things on my end. >> * Read the D-cache value in the spl, and confirmed that the data >> cache is indeed not enabled. > What is the value of the B bit in CP15 SCR register? I wonder if RBL is > doing all the IMB operations required after copying the SPL image and > before executing it. IMB is required for consistency between data and > instruction sides. Only if caches are used, right? Or also without caches? Tom wrote that RBL does not turn on cache. Regards, Christian
On Thursday 19 January 2012 05:00 PM, Christian Riesch wrote: > Hi Aneesh, > > On Thu, Jan 19, 2012 at 11:17 AM, Aneesh V<aneesh@ti.com> wrote: >> On Thursday 19 January 2012 12:23 PM, Sughosh Ganu wrote: >>> Tried a few things on my end. >>> * Read the D-cache value in the spl, and confirmed that the data >>> cache is indeed not enabled. >> What is the value of the B bit in CP15 SCR register? I wonder if RBL is >> doing all the IMB operations required after copying the SPL image and >> before executing it. IMB is required for consistency between data and >> instruction sides. > > Only if caches are used, right? Or also without caches? > Tom wrote that RBL does not turn on cache. > Regards, Christian Only D-cache seems to be disabled in this case. I-cache and Write buffer are likely to be enabled. If so, all the IMB operations except the data-cache flushing are still relevant. br, Aneesh
On Thu, Jan 19, 2012 at 12:54 PM, Aneesh V <aneesh@ti.com> wrote: > On Thursday 19 January 2012 05:00 PM, Christian Riesch wrote: >> On Thu, Jan 19, 2012 at 11:17 AM, Aneesh V<aneesh@ti.com> wrote: >>> On Thursday 19 January 2012 12:23 PM, Sughosh Ganu wrote: >>>> Tried a few things on my end. >>>> * Read the D-cache value in the spl, and confirmed that the data >>>> cache is indeed not enabled. >>> >>> What is the value of the B bit in CP15 SCR register? I wonder if RBL is >>> doing all the IMB operations required after copying the SPL image and >>> before executing it. IMB is required for consistency between data and >>> instruction sides. >> >> Only if caches are used, right? Or also without caches? >> Tom wrote that RBL does not turn on cache. >> Regards, Christian > > Only D-cache seems to be disabled in this case. I-cache and Write > buffer are likely to be enabled. If so, all the IMB operations except > the data-cache flushing are still relevant. Tom, when you wrote that RBL does not turn on caches, did you mean it never turns it on or it turns some of them on and turns them off before exit? Christian
Sughosh, On Friday 20 January 2012 12:58 PM, Christian Riesch wrote: > On Thu, Jan 19, 2012 at 12:54 PM, Aneesh V<aneesh@ti.com> wrote: >> On Thursday 19 January 2012 05:00 PM, Christian Riesch wrote: >>> On Thu, Jan 19, 2012 at 11:17 AM, Aneesh V<aneesh@ti.com> wrote: >>>> On Thursday 19 January 2012 12:23 PM, Sughosh Ganu wrote: >>>>> Tried a few things on my end. >>>>> * Read the D-cache value in the spl, and confirmed that the data >>>>> cache is indeed not enabled. >>>> >>>> What is the value of the B bit in CP15 SCR register? I wonder if RBL is >>>> doing all the IMB operations required after copying the SPL image and >>>> before executing it. IMB is required for consistency between data and >>>> instruction sides. >>> >>> Only if caches are used, right? Or also without caches? >>> Tom wrote that RBL does not turn on cache. >>> Regards, Christian >> >> Only D-cache seems to be disabled in this case. I-cache and Write >> buffer are likely to be enabled. If so, all the IMB operations except >> the data-cache flushing are still relevant. > > Tom, when you wrote that RBL does not turn on caches, did you mean it > never turns it on or it turns some of them on and turns them off > before exit? > Christian Can you send the value of SCR you found at SPL entry? This will clarify what's enabled and what's not.
Hi Aneesh, On Fri, Jan 20, 2012 at 9:52 AM, Aneesh V <aneesh@ti.com> wrote: > Sughosh, [...] > Can you send the value of SCR you found at SPL entry? This will clarify > what's enabled and what's not. I would like to try that on my board as well for comparison. Could you please tell me how this register can be read? In the ARM manuals SCR seems to have several meanings... Thank you! Regards, Christian
so sorry to you, i think it's difference between DISABLE and Flush. be careful. On Wed, Jan 11, 2012 at 2:12 AM, Sughosh Ganu <urwithsughosh@gmail.com>wrote: > The current implementation invalidates the cache instead of flushing > it. This causes problems on platforms where the spl/u-boot is already > loaded to the RAM, with caches enabled by a first stage bootloader. > > The V bit of the cp15's control register c1 is set to the value of > VINITHI on reset. Do not clear this bit by default, as there are SOC's > with no valid memory region at 0x0. > > Signed-off-by: Sughosh Ganu <urwithsughosh@gmail.com> > Cc: Albert Aribaud <albert.u.boot@aribaud.net> > --- > > Changes since V1 > * Added arm926 keyword to the subject line > * Removed the superfluous setting of r0. > * Fixed the comment to reflect the fact that V is not being cleared > > arch/arm/cpu/arm926ejs/start.S | 9 ++++++--- > 1 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/cpu/arm926ejs/start.S > b/arch/arm/cpu/arm926ejs/start.S > index 6a09c02..6e261c2 100644 > --- a/arch/arm/cpu/arm926ejs/start.S > +++ b/arch/arm/cpu/arm926ejs/start.S > @@ -355,17 +355,20 @@ _dynsym_start_ofs: > */ > cpu_init_crit: > /* > - * flush v4 I/D caches > + * flush D cache before disabling it > */ > mov r0, #0 > - mcr p15, 0, r0, c7, c7, 0 /* flush v3/v4 cache */ > +flush_dcache: > + mrc p15, 0, r15, c7, c10, 3 > + bne flush_dcache > + > mcr p15, 0, r0, c8, c7, 0 /* flush v4 TLB */ > > /* > * disable MMU stuff and caches > */ > mrc p15, 0, r0, c1, c0, 0 > - bic r0, r0, #0x00002300 /* clear bits 13, 9:8 (--V- --RS) > */ > + bic r0, r0, #0x00000300 /* clear bits 9:8 ( --RS) */ > bic r0, r0, #0x00000087 /* clear bits 7, 2:0 (B--- -CAM) */ > orr r0, r0, #0x00000002 /* set bit 2 (A) Align */ > orr r0, r0, #0x00001000 /* set bit 12 (I) I-Cache */ > -- > 1.7.5.4 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot >
On Fri, Jan 20, 2012 at 12:28 AM, Christian Riesch <christian.riesch@omicron.at> wrote: > On Thu, Jan 19, 2012 at 12:54 PM, Aneesh V <aneesh@ti.com> wrote: >> On Thursday 19 January 2012 05:00 PM, Christian Riesch wrote: >>> On Thu, Jan 19, 2012 at 11:17 AM, Aneesh V<aneesh@ti.com> wrote: >>>> On Thursday 19 January 2012 12:23 PM, Sughosh Ganu wrote: >>>>> Tried a few things on my end. >>>>> * Read the D-cache value in the spl, and confirmed that the data >>>>> cache is indeed not enabled. >>>> >>>> What is the value of the B bit in CP15 SCR register? I wonder if RBL is >>>> doing all the IMB operations required after copying the SPL image and >>>> before executing it. IMB is required for consistency between data and >>>> instruction sides. >>> >>> Only if caches are used, right? Or also without caches? >>> Tom wrote that RBL does not turn on cache. >>> Regards, Christian >> >> Only D-cache seems to be disabled in this case. I-cache and Write >> buffer are likely to be enabled. If so, all the IMB operations except >> the data-cache flushing are still relevant. > > Tom, when you wrote that RBL does not turn on caches, did you mean it > never turns it on or it turns some of them on and turns them off > before exit? I'm away from the code atm (and when I get back I can point Aneesh at it as well), but DCACHE is never enabled and I'm thinking ICACHE too.
On Friday 20 January 2012 02:51 PM, Christian Riesch wrote: > Hi Aneesh, > > On Fri, Jan 20, 2012 at 9:52 AM, Aneesh V<aneesh@ti.com> wrote: >> Sughosh, > [...] >> Can you send the value of SCR you found at SPL entry? This will clarify >> what's enabled and what's not. > > I would like to try that on my board as well for comparison. Could you > please tell me how this register can be read? In the ARM manuals SCR > seems to have several meanings... Thank you! > Regards, Christian If you have a JTAG based debugger that has the capability of displaying CP15 registers, look for "CP15 System Control Register". Otherwise you will have to read it using an assembly instruction like below: mrc p15, 0, r0, c1, c0, 0 After this instruction r0 will contain the SCR value. arm926ejs/start.S has this instruction at line #367. You can put a breakpoint after this and look at r0. br, Aneesh
Hi Aneesh, On Fri, Jan 20, 2012 at 1:13 PM, Aneesh V <aneesh@ti.com> wrote: > On Friday 20 January 2012 02:51 PM, Christian Riesch wrote: >> On Fri, Jan 20, 2012 at 9:52 AM, Aneesh V<aneesh@ti.com> wrote: >>> Sughosh, >> >> [...] >>> >>> Can you send the value of SCR you found at SPL entry? This will clarify >>> what's enabled and what's not. >> >> I would like to try that on my board as well for comparison. Could you >> please tell me how this register can be read? In the ARM manuals SCR >> seems to have several meanings... Thank you! >> Regards, Christian > > If you have a JTAG based debugger that has the capability of displaying > CP15 registers, look for "CP15 System Control Register". Otherwise you > will have to read it using an assembly instruction like below: > > > mrc p15, 0, r0, c1, c0, 0 > > After this instruction r0 will contain the SCR value. arm926ejs/start.S > has this instruction at line #367. You can put a breakpoint after this > and look at r0. Thank you! I don't have a JTAG debugger so I stored it in a register, pushed it later to the stack and then read it with md.l from the memory. I tried it on my custom board (AM1808 SoC, direct boot from NOR flash) and on both the da850evm (with AM1808 SoC, AIS boot from SPI flash). The result was the same for both cases, 0x00052078. So DCache and ICache are disabled after the RBL. Regards, Christian
Hi Christian, On Friday 20 January 2012 06:18 PM, Christian Riesch wrote: > Hi Aneesh, > > On Fri, Jan 20, 2012 at 1:13 PM, Aneesh V<aneesh@ti.com> wrote: >> On Friday 20 January 2012 02:51 PM, Christian Riesch wrote: >>> On Fri, Jan 20, 2012 at 9:52 AM, Aneesh V<aneesh@ti.com> wrote: >>>> Sughosh, >>> >>> [...] >>>> >>>> Can you send the value of SCR you found at SPL entry? This will clarify >>>> what's enabled and what's not. >>> >>> I would like to try that on my board as well for comparison. Could you >>> please tell me how this register can be read? In the ARM manuals SCR >>> seems to have several meanings... Thank you! >>> Regards, Christian >> >> If you have a JTAG based debugger that has the capability of displaying >> CP15 registers, look for "CP15 System Control Register". Otherwise you >> will have to read it using an assembly instruction like below: >> >> >> mrc p15, 0, r0, c1, c0, 0 >> >> After this instruction r0 will contain the SCR value. arm926ejs/start.S >> has this instruction at line #367. You can put a breakpoint after this >> and look at r0. > > Thank you! > > I don't have a JTAG debugger so I stored it in a register, pushed it > later to the stack and then read it with md.l from the memory. I tried > it on my custom board (AM1808 SoC, direct boot from NOR flash) and on > both the da850evm (with AM1808 SoC, AIS boot from SPI flash). The > result was the same for both cases, 0x00052078. So DCache and ICache > are disabled after the RBL. > Regards, Christian Hmm.. That's different from the OMAP processors I have seen. At least OMAP4, that I verified again now, leaves the I-cache enabled (0x00C51878) So, Sughosh's problem still remains a mystery:) br, Aneesh
On Fri, Jan 20, 2012 at 6:06 AM, Aneesh V <aneesh@ti.com> wrote: > Hi Christian, > > > On Friday 20 January 2012 06:18 PM, Christian Riesch wrote: >> >> Hi Aneesh, >> >> On Fri, Jan 20, 2012 at 1:13 PM, Aneesh V<aneesh@ti.com> wrote: >>> >>> On Friday 20 January 2012 02:51 PM, Christian Riesch wrote: >>>> >>>> On Fri, Jan 20, 2012 at 9:52 AM, Aneesh V<aneesh@ti.com> wrote: >>>>> >>>>> Sughosh, >>>> >>>> >>>> [...] >>>>> >>>>> >>>>> Can you send the value of SCR you found at SPL entry? This will clarify >>>>> what's enabled and what's not. >>>> >>>> >>>> I would like to try that on my board as well for comparison. Could you >>>> please tell me how this register can be read? In the ARM manuals SCR >>>> seems to have several meanings... Thank you! >>>> Regards, Christian >>> >>> >>> If you have a JTAG based debugger that has the capability of displaying >>> CP15 registers, look for "CP15 System Control Register". Otherwise you >>> will have to read it using an assembly instruction like below: >>> >>> >>> mrc p15, 0, r0, c1, c0, 0 >>> >>> After this instruction r0 will contain the SCR value. arm926ejs/start.S >>> has this instruction at line #367. You can put a breakpoint after this >>> and look at r0. >> >> >> Thank you! >> >> I don't have a JTAG debugger so I stored it in a register, pushed it >> later to the stack and then read it with md.l from the memory. I tried >> it on my custom board (AM1808 SoC, direct boot from NOR flash) and on >> both the da850evm (with AM1808 SoC, AIS boot from SPI flash). The >> result was the same for both cases, 0x00052078. So DCache and ICache >> are disabled after the RBL. >> Regards, Christian > > > Hmm.. That's different from the OMAP processors I have seen. At least > OMAP4, that I verified again now, leaves the I-cache enabled > (0x00C51878) > > So, Sughosh's problem still remains a mystery:) So, what do we want to do here? We really want to get this fix in so we can get the hawkboard SPL changes in, and the other platforms / fixups that are gated by that. If I can sum it up, in the relevant section of code we have incorrect comments and the init code is not following what the manual says the correct sequence is. However, given the (potentially wide) impact the changes would have, Albert had previously requested making the change opt-in (but I believe this request came before the "the manual says we must do ..."). If this is still the case? If so, can we get an updated patch? Thanks!
Hi all, On Fri, Jan 27, 2012 at 7:33 PM, Tom Rini <tom.rini@gmail.com> wrote: > So, what do we want to do here? We really want to get this fix in so > we can get the hawkboard SPL changes in, and the other platforms / > fixups that are gated by that. > > If I can sum it up, in the relevant section of code we have incorrect > comments and the init code is not following what the manual says the > correct sequence is. However, given the (potentially wide) impact the > changes would have, Albert had previously requested making the change > opt-in (but I believe this request came before the "the manual says we > must do ..."). If this is still the case? If so, can we get an > updated patch? Thanks! A few thoughts: 1) Before commit ca4b55800ed74207c35271bf7335a092d4955416 the low level initialization code that we are discussing here was only executed if CONFIG_SKIP_LOWLEVEL_INIT is not defined. For ARM926EJS the relevant section in start.S looked like this #ifndef CONFIG_SKIP_LOWLEVEL_INIT flush caches turn off dcache, do some other configurations of the CPU, enable icache call the SoC specific lowlevel_init #endif For DA850 SoCs we had no lowlevel_init routine and therefore CONFIG_SKIP_LOWLEVEL_INIT defined in all board configurations. The lowlevel initialization was done by TI's UBL boot loader. Now we have boards that don't use UBL (e.g. enbw_cmc, calimain, da850evm when used with the SPL), therefore we need some lowlevel init. Most important configuration is enabling icache, otherwise u-boot startup gets really slow. Since commit ca4b55800ed74207c35271bf7335a092d4955416 it is flush caches turn off dcache, do some other configurations of the CPU, enable icache #ifndef CONFIG_SKIP_LOWLEVEL_INIT call the SoC specific lowlevel_init #endif So the change that has a big impact is already done and in mainline. Perhaps we should revert that change and instead remove CONFIG_SKIP_LOWLEVEL_INIT from the da850 board config files. But since we don't need the lowlevel_init function for DA850 SoCs we must either add a dummy function or add some additional defines that allow removing the CONFIG_SKIP_LOWLEVEL_INIT define without calling lowlevel_init. Any suggestions how such defines should be called or how the logic behind them should be so it doesn't cause breakage of existing boards? What do you think? Or is reverting to dangerous since we would change the behavior of start.S twice if we do so? 2) The current version of Sughosh's patch does not change the logic behind the LOWLEVEL_INIT defines but just fixes the code to agree with ARM's manual. Instead of invalidating the cache it now is flushed. I think we should therefore merge his patch (@Tom: Yes, the manual says we must do.). The big change that Albert fears was already done earlier in commit ca4b55800ed74207c35271bf7335a092d4955416. And while we are doing this we also get the comments right. 3) As Sughosh pointed out, the current code changes the V bit (location of exceptions). Sughosh's patch removes this code that does this change. I'm not sure if this is correct or not, so maybe you, Tom, could put your TI hat on again and clarify this? 4) The current code turns on icache. This is great since my board executes u-boot directly from flash until it relocates itself and thus the startup time is greatly reduced by using icache. However, there is this CONFIG_SYS_ICACHE_OFF define. Should the code be changed to respect this define? Regards, Christian
Hello Christian, Christian Riesch wrote: > Hi all, > > On Fri, Jan 27, 2012 at 7:33 PM, Tom Rini <tom.rini@gmail.com> wrote: >> So, what do we want to do here? We really want to get this fix in so >> we can get the hawkboard SPL changes in, and the other platforms / >> fixups that are gated by that. >> >> If I can sum it up, in the relevant section of code we have incorrect >> comments and the init code is not following what the manual says the >> correct sequence is. However, given the (potentially wide) impact the >> changes would have, Albert had previously requested making the change >> opt-in (but I believe this request came before the "the manual says we >> must do ..."). If this is still the case? If so, can we get an >> updated patch? Thanks! > > A few thoughts: > > 1) Before commit ca4b55800ed74207c35271bf7335a092d4955416 the low > level initialization code that we are discussing here was only > executed if CONFIG_SKIP_LOWLEVEL_INIT is not defined. For ARM926EJS > the relevant section in start.S looked like this > > #ifndef CONFIG_SKIP_LOWLEVEL_INIT > flush caches > turn off dcache, do some other configurations of the CPU, enable icache > call the SoC specific lowlevel_init > #endif > > For DA850 SoCs we had no lowlevel_init routine and therefore > CONFIG_SKIP_LOWLEVEL_INIT defined in all board configurations. The > lowlevel initialization was done by TI's UBL boot loader. Now we have > boards that don't use UBL (e.g. enbw_cmc, calimain, da850evm when used > with the SPL), therefore we need some lowlevel init. Most important > configuration is enabling icache, otherwise u-boot startup gets really > slow. > > Since commit ca4b55800ed74207c35271bf7335a092d4955416 it is > > flush caches > turn off dcache, do some other configurations of the CPU, enable icache > #ifndef CONFIG_SKIP_LOWLEVEL_INIT > call the SoC specific lowlevel_init > #endif > > So the change that has a big impact is already done and in mainline. Yep. > Perhaps we should revert that change and instead remove > CONFIG_SKIP_LOWLEVEL_INIT from the da850 board config files. But since > we don't need the lowlevel_init function for DA850 SoCs we must either > add a dummy function or add some additional defines that allow > removing the CONFIG_SKIP_LOWLEVEL_INIT define without calling > lowlevel_init. Any suggestions how such defines should be called or > how the logic behind them should be so it doesn't cause breakage of > existing boards? or we should introduce a "CONFIG_SKIP_CPU_CRIT_INIT" define, which if defined, prevent the jump to cpu_init_crit ... so we have the same behaviour as before commit "ca4b55800ed74207c35271bf7335a092d4955416" Boards which have problems with cpu_crit_init, should define this new define ... but it would be better to find out, what is really going on here ... > What do you think? Or is reverting to dangerous since we would change > the behavior of start.S twice if we do so? See comment above. I can send such a patch for this, if we decide to go this direction ... as I need the cpu_init_crit part for the enbw_cmc board, but do not need the branch to "lowlevel_init" ... > 2) The current version of Sughosh's patch does not change the logic > behind the LOWLEVEL_INIT defines but just fixes the code to agree with > ARM's manual. Instead of invalidating the cache it now is flushed. I > think we should therefore merge his patch (@Tom: Yes, the manual says > we must do.). The big change that Albert fears was already done > earlier in commit ca4b55800ed74207c35271bf7335a092d4955416. And while > we are doing this we also get the comments right. What do you mean with commit "ca4b55800ed74207c35271bf7335a092d4955416"? I could not find it in mainline ... > 3) As Sughosh pointed out, the current code changes the V bit > (location of exceptions). Sughosh's patch removes this code that does > this change. I'm not sure if this is correct or not, so maybe you, > Tom, could put your TI hat on again and clarify this? Yes, I am also not sure, what to do with this V Bit ... > 4) The current code turns on icache. This is great since my board > executes u-boot directly from flash until it relocates itself and thus > the startup time is greatly reduced by using icache. However, there is > this CONFIG_SYS_ICACHE_OFF define. Should the code be changed to > respect this define? Yep, that should be done. Default should be icache on, so we do not change exisiting behaviour. bye, Heiko
hi Christian, On Sun Jan 29, 2012 at 02:36:39PM +0100, Christian Riesch wrote: > Hi all, > > On Fri, Jan 27, 2012 at 7:33 PM, Tom Rini <tom.rini@gmail.com> wrote: > > So, what do we want to do here? We really want to get this fix in so > > we can get the hawkboard SPL changes in, and the other platforms / > > fixups that are gated by that. > > > > If I can sum it up, in the relevant section of code we have incorrect > > comments and the init code is not following what the manual says the > > correct sequence is. However, given the (potentially wide) impact the > > changes would have, Albert had previously requested making the change > > opt-in (but I believe this request came before the "the manual says we > > must do ..."). If this is still the case? If so, can we get an > > updated patch? Thanks! > > A few thoughts: > > 1) Before commit ca4b55800ed74207c35271bf7335a092d4955416 the low > level initialization code that we are discussing here was only > executed if CONFIG_SKIP_LOWLEVEL_INIT is not defined. For ARM926EJS > the relevant section in start.S looked like this > > #ifndef CONFIG_SKIP_LOWLEVEL_INIT > flush caches > turn off dcache, do some other configurations of the CPU, enable icache > call the SoC specific lowlevel_init > #endif This is correct, the only differece being, flush caches, is what the comment said, but it was actually doing a invalidate caches. <snip> > So the change that has a big impact is already done and in mainline. > > Perhaps we should revert that change and instead remove > CONFIG_SKIP_LOWLEVEL_INIT from the da850 board config files. But since > we don't need the lowlevel_init function for DA850 SoCs we must either > add a dummy function or add some additional defines that allow > removing the CONFIG_SKIP_LOWLEVEL_INIT define without calling > lowlevel_init. Any suggestions how such defines should be called or > how the logic behind them should be so it doesn't cause breakage of > existing boards? Actually, even i can do with the enabling of the icache :). The only change i made was that instead of invalidating the data cache, i do doing a "test and flush". > What do you think? Or is reverting to dangerous since we would change > the behavior of start.S twice if we do so? > > 2) The current version of Sughosh's patch does not change the logic > behind the LOWLEVEL_INIT defines but just fixes the code to agree with > ARM's manual. Instead of invalidating the cache it now is flushed. I > think we should therefore merge his patch (@Tom: Yes, the manual says > we must do.). The big change that Albert fears was already done > earlier in commit ca4b55800ed74207c35271bf7335a092d4955416. And while > we are doing this we also get the comments right. Actually, the part of the code that Albert NAK'ed was changing the setting of the V bit. To this he had proposed to make this a config option. The current behaviour clears the V bit by default in the cpu_init_crit function, which maps the exception vectors at 0x0. But some SoC's don't have anything mapped at 0x0(eg. omapl-138), so i had proposed not to clear this bit by default. http://www.mail-archive.com/u-boot@lists.denx.de/msg75271.html > > 3) As Sughosh pointed out, the current code changes the V bit > (location of exceptions). Sughosh's patch removes this code that does > this change. I'm not sure if this is correct or not, so maybe you, > Tom, could put your TI hat on again and clarify this? Please check the link i have provided above. -sughosh
Hi, On Monday, January 30, 2012, Heiko Schocher <hs@denx.de> wrote: > Hello Christian, > > Christian Riesch wrote: >> Hi all, >> >> On Fri, Jan 27, 2012 at 7:33 PM, Tom Rini <tom.rini@gmail.com> wrote: >>> So, what do we want to do here? We really want to get this fix in so >>> we can get the hawkboard SPL changes in, and the other platforms / >>> fixups that are gated by that. >>> >>> If I can sum it up, in the relevant section of code we have incorrect >>> comments and the init code is not following what the manual says the >>> correct sequence is. However, given the (potentially wide) impact the >>> changes would have, Albert had previously requested making the change >>> opt-in (but I believe this request came before the "the manual says we >>> must do ..."). If this is still the case? If so, can we get an >>> updated patch? Thanks! >> >> A few thoughts: >> >> 1) Before commit ca4b55800ed74207c35271bf7335a092d4955416 the low >> level initialization code that we are discussing here was only >> executed if CONFIG_SKIP_LOWLEVEL_INIT is not defined. For ARM926EJS >> the relevant section in start.S looked like this >> >> #ifndef CONFIG_SKIP_LOWLEVEL_INIT >> flush caches >> turn off dcache, do some other configurations of the CPU, enable icache >> call the SoC specific lowlevel_init >> #endif >> >> For DA850 SoCs we had no lowlevel_init routine and therefore >> CONFIG_SKIP_LOWLEVEL_INIT defined in all board configurations. The >> lowlevel initialization was done by TI's UBL boot loader. Now we have >> boards that don't use UBL (e.g. enbw_cmc, calimain, da850evm when used >> with the SPL), therefore we need some lowlevel init. Most important >> configuration is enabling icache, otherwise u-boot startup gets really >> slow. >> >> Since commit ca4b55800ed74207c35271bf7335a092d4955416 it is >> >> flush caches >> turn off dcache, do some other configurations of the CPU, enable icache >> #ifndef CONFIG_SKIP_LOWLEVEL_INIT >> call the SoC specific lowlevel_init >> #endif >> >> So the change that has a big impact is already done and in mainline. > > Yep. > >> Perhaps we should revert that change and instead remove >> CONFIG_SKIP_LOWLEVEL_INIT from the da850 board config files. But since >> we don't need the lowlevel_init function for DA850 SoCs we must either >> add a dummy function or add some additional defines that allow >> removing the CONFIG_SKIP_LOWLEVEL_INIT define without calling >> lowlevel_init. Any suggestions how such defines should be called or >> how the logic behind them should be so it doesn't cause breakage of >> existing boards? > > or we should introduce a "CONFIG_SKIP_CPU_CRIT_INIT" define, which > if defined, prevent the jump to cpu_init_crit ... so we have the same > behaviour as before commit "ca4b55800ed74207c35271bf7335a092d4955416" > > Boards which have problems with cpu_crit_init, should define > this new define ... but it would be better to find out, what > is really going on here ... Yes, that would be good of course. Another idea: Actually the init code that we are discussing here is executed twice. First in the SPL and then in the full-blown u-boot. Sughosh, are you sure it is the execution of the code in SPL that causes the trouble? I'm asking since we don't do any memory barrier related stuff in the code that loads u-boot from flash in SPL. Of course, dcache is off but icache is on. Regards, Christian
hi Christian, On Mon Jan 30, 2012 at 09:10:46AM +0100, Christian Riesch wrote: <snip> > >> Perhaps we should revert that change and instead remove > >> CONFIG_SKIP_LOWLEVEL_INIT from the da850 board config files. But since > >> we don't need the lowlevel_init function for DA850 SoCs we must either > >> add a dummy function or add some additional defines that allow > >> removing the CONFIG_SKIP_LOWLEVEL_INIT define without calling > >> lowlevel_init. Any suggestions how such defines should be called or > >> how the logic behind them should be so it doesn't cause breakage of > >> existing boards? > > > > or we should introduce a "CONFIG_SKIP_CPU_CRIT_INIT" define, which > > if defined, prevent the jump to cpu_init_crit ... so we have the same > > behaviour as before commit "ca4b55800ed74207c35271bf7335a092d4955416" > > > > Boards which have problems with cpu_crit_init, should define > > this new define ... but it would be better to find out, what > > is really going on here ... > > Yes, that would be good of course. > > Another idea: Actually the init code that we are discussing here is > executed twice. First in the SPL and then in the full-blown u-boot. > Sughosh, are you sure it is the execution of the code in SPL that causes > the trouble? I'm asking since we don't do any memory barrier related stuff > in the code that loads u-boot from flash in SPL. Of course, dcache is off > but icache is on. Yes, it is the spl that is executing the code. There is a spl specific string that gets displayed on the console, before spl goes on to load the u-boot image and jump to it, and i don't see that string on the console. -sughosh
Hello Heiko, On Mon, Jan 30, 2012 at 7:39 AM, Heiko Schocher <hs@denx.de> wrote: > Christian Riesch wrote: >> 2) The current version of Sughosh's patch does not change the logic >> behind the LOWLEVEL_INIT defines but just fixes the code to agree with >> ARM's manual. Instead of invalidating the cache it now is flushed. I >> think we should therefore merge his patch (@Tom: Yes, the manual says >> we must do.). The big change that Albert fears was already done >> earlier in commit ca4b55800ed74207c35271bf7335a092d4955416. And while >> we are doing this we also get the comments right. > > What do you mean with commit "ca4b55800ed74207c35271bf7335a092d4955416"? > I could not find it in mainline ... commit ca4b55800ed74207c35271bf7335a092d4955416 Author: Heiko Schocher <hs@denx.de> Date: Wed Nov 9 20:06:23 2011 +0000 arm, arm926ejs: always do cpu critical inits Regards, Christian
On Sun, Jan 29, 2012 at 6:36 AM, Christian Riesch <christian.riesch@omicron.at> wrote: > 3) As Sughosh pointed out, the current code changes the V bit > (location of exceptions). Sughosh's patch removes this code that does > this change. I'm not sure if this is correct or not, so maybe you, > Tom, could put your TI hat on again and clarify this? OK, I've asked Christian for questions I can pass along, and here's what's I've learned: Q1) Currently, the low level initialization code for ARM926EJS CPUs in the u-boot bootloader clears the V-bit of the cp15 control register c1. By default, this bit is set on AM1808 and OMAP-L138 before u-boot ist started. Sughosh Ganu now submitted a patch to remove the code that clears the V bit because he states that these SoCs have no valid memory region at 0x0. I had a look at the memory map of the AM1808 and yes, there is no valid memory at 0x0, so the V bit should be set and not cleared. My question is: Since the AM1808 has no valid memory at 0x0, does clearing the V bit have any effect on the operation of the processor? Or is is just a don't care bit since it doesn't make any sense to clear it? A1) This may be a design question, but I can say that the default value of the V-bit is set to 1 because of tie-offs to the core, but I'm not sure if the functionality of the V-bit is still active. I would guess that it is because I see no reason we would have mucked around in the ARM core design to remove that functionality, so unless there is another tie-off to the core that disables the V-bit functionality entirely, I would guess clearing the V-bit is not a good idea. In fact, I don't see why the u-boot init code needs to mess with the default value of that bit ever, for any device or arch. Q2) RBL: In an earlier post to the u-boot mailing list Tom Rini wrote that the RBLs of AM1808 and OMAP-L138 do not turn on caches. Does this mean that caches (dcache and/or icache) are *never* enabled or does it mean that they get enabled and then disabled before RBL exits. In the latter case, does RBL do everything that is necessary to ensure consistency between data and instruction streams (as described in the ARM926EJ-S Technical Reference Manual, http://infocenter.arm.com/help/index.jsp, Section "Instruction Memory Barrier.1. About the instruction memory barrier operation")? Are there maybe earlier versions of RBL in earlier versions of the SoC that have a bug here? A2) The RBL NEVER enables the caches. Unfortunately, this does slow the operation of the ROM boot loader, but it is what is
On Mon Jan 30, 2012 at 10:03:40AM -0700, Tom Rini wrote: > Q1) Currently, the low level initialization code for ARM926EJS CPUs in > the u-boot bootloader clears the V-bit of the cp15 control register > c1. By default, this bit is set on AM1808 and OMAP-L138 before u-boot > ist started. Sughosh Ganu now submitted a patch to remove the code > that clears the V bit because he states that these SoCs have no valid > memory region at 0x0. I had a look at the memory map of the AM1808 and > yes, there is no valid memory at 0x0, so the V bit should be set and > not cleared. My question is: Since the AM1808 has no valid memory at > 0x0, does clearing the V bit have any effect on the operation of the > processor? Or is is just a don't care bit since it doesn't make any > sense to clear it? > > A1) This may be a design question, but I can say that the default > value of the V-bit is set to 1 because of tie-offs to the core, but > I'm not sure if the functionality of the V-bit is still active. I > would guess that it is because I see no reason we would have mucked > around in the ARM core design to remove that functionality, so unless > there is another tie-off to the core that disables the V-bit > functionality entirely, I would guess clearing the V-bit is not a good > idea. In fact, I don't see why the u-boot init code needs to mess > with the default value of that bit ever, for any device or arch. I'm not sure about whether the V-bit functionality is disabled by some setting, but the omapl-138 ref manual states this(section 2.4 "Exceptions and Exception Vectors"). " NOTE: The VINTH signal is configurable by way of the register setting in CP15. However, it is not recommended to set VINTH = 0, as the device has no physical memory in the 0000 0000h address region." -sughosh
Hi Tom, On Mon, Jan 30, 2012 at 6:03 PM, Tom Rini <tom.rini@gmail.com> wrote: > On Sun, Jan 29, 2012 at 6:36 AM, Christian Riesch > <christian.riesch@omicron.at> wrote: > >> 3) As Sughosh pointed out, the current code changes the V bit >> (location of exceptions). Sughosh's patch removes this code that does >> this change. I'm not sure if this is correct or not, so maybe you, >> Tom, could put your TI hat on again and clarify this? > > OK, I've asked Christian for questions I can pass along, and here's > what's I've learned: Thanks a lot! > Q1) Currently, the low level initialization code for ARM926EJS CPUs in > the u-boot bootloader clears the V-bit of the cp15 control register > c1. By default, this bit is set on AM1808 and OMAP-L138 before u-boot > ist started. Sughosh Ganu now submitted a patch to remove the code > that clears the V bit because he states that these SoCs have no valid > memory region at 0x0. I had a look at the memory map of the AM1808 and > yes, there is no valid memory at 0x0, so the V bit should be set and > not cleared. My question is: Since the AM1808 has no valid memory at > 0x0, does clearing the V bit have any effect on the operation of the > processor? Or is is just a don't care bit since it doesn't make any > sense to clear it? > > A1) This may be a design question, but I can say that the default > value of the V-bit is set to 1 because of tie-offs to the core, but > I'm not sure if the functionality of the V-bit is still active. I > would guess that it is because I see no reason we would have mucked > around in the ARM core design to remove that functionality, so unless > there is another tie-off to the core that disables the V-bit > functionality entirely, I would guess clearing the V-bit is not a good > idea. In fact, I don't see why the u-boot init code needs to mess > with the default value of that bit ever, for any device or arch. Ok. So this should not be cleared, at least not for davinci. > Q2) RBL: In an earlier post to the u-boot mailing list Tom Rini wrote > that the RBLs of AM1808 and OMAP-L138 do not turn on caches. Does this > mean that caches (dcache and/or icache) are *never* enabled or does it > mean that they get enabled and then disabled before RBL exits. In the > latter case, does RBL do everything that is necessary to ensure > consistency between data and instruction streams (as described in the > ARM926EJ-S Technical Reference Manual, > http://infocenter.arm.com/help/index.jsp, Section "Instruction Memory > Barrier.1. About the instruction memory barrier operation")? Are there > maybe earlier versions of RBL in earlier versions of the SoC that have > a bug here? > > A2) The RBL NEVER enables the caches. Unfortunately, this does slow > the operation of the ROM boot loader, but it is what is I think this ends our speculations about RBL causing the hawkboard problems. We will need someone with a hawkboard and a JTAG debugger to find out more... For now I think we should make these changes: 1) correct the comments 2) replace invalidating the cache with flushing the cache since this is the way to do it according to the ARM manual (Sughosh's patch) 3) do not clear the V bit on DA850 SoCs 4) revert Heiko's patch that changes the behavior of CONFIG_SKIP_LOWLEVEL_INIT and add a possibility to remove this option on boards that need lowlevel config 2) respect CONFIG_SYS_ICACHE_OFF I prepared a patchset that also includes Sughosh's patches and have just submitted it (The people on Cc received it twice, sorry for this...). Regards, Christian
diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S index 6a09c02..6e261c2 100644 --- a/arch/arm/cpu/arm926ejs/start.S +++ b/arch/arm/cpu/arm926ejs/start.S @@ -355,17 +355,20 @@ _dynsym_start_ofs: */ cpu_init_crit: /* - * flush v4 I/D caches + * flush D cache before disabling it */ mov r0, #0 - mcr p15, 0, r0, c7, c7, 0 /* flush v3/v4 cache */ +flush_dcache: + mrc p15, 0, r15, c7, c10, 3 + bne flush_dcache + mcr p15, 0, r0, c8, c7, 0 /* flush v4 TLB */ /* * disable MMU stuff and caches */ mrc p15, 0, r0, c1, c0, 0 - bic r0, r0, #0x00002300 /* clear bits 13, 9:8 (--V- --RS) */ + bic r0, r0, #0x00000300 /* clear bits 9:8 ( --RS) */ bic r0, r0, #0x00000087 /* clear bits 7, 2:0 (B--- -CAM) */ orr r0, r0, #0x00000002 /* set bit 2 (A) Align */ orr r0, r0, #0x00001000 /* set bit 12 (I) I-Cache */
The current implementation invalidates the cache instead of flushing it. This causes problems on platforms where the spl/u-boot is already loaded to the RAM, with caches enabled by a first stage bootloader. The V bit of the cp15's control register c1 is set to the value of VINITHI on reset. Do not clear this bit by default, as there are SOC's with no valid memory region at 0x0. Signed-off-by: Sughosh Ganu <urwithsughosh@gmail.com> Cc: Albert Aribaud <albert.u.boot@aribaud.net> --- Changes since V1 * Added arm926 keyword to the subject line * Removed the superfluous setting of r0. * Fixed the comment to reflect the fact that V is not being cleared arch/arm/cpu/arm926ejs/start.S | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)