Dear Wolfgang, I'd like to clarify what is the problem with the timeout and the Intel flash (even if the following comments can be obvious or already well known) and to ask you an opinion on a small code change.
The flash has an internal busy flag that is polled in function flash_status_check() and that function is the only one in cfi_flash.c that uses the get_timer() func. and the CONFIG_SYS_HZ definition. In many Altera/Nios boards the CONFIG_SYS_HZ constant evaluate to 999 due to rounding errors. With the current implementation 999 != 1000 evaluate to 1 so the CONFIG_SYS_HZ/1000 division is done and returns 0. This lead to a (forced) 0 timeout in the flash_status_check() that corresponds to erroneous flash clear, program, etc.. I've proposed to change the code from #if CONFIG_SYS_HZ != 1000 tout *= CONFIG_SYS_HZ/1000; #endif to #if CONFIG_SYS_HZ != 1000 unsigned long long ull; ull = tout*CONFIG_SYS_HZ + CONFIG_SYS_HZ/2; tout = ull/1000; /* Compute: tout *= CONFIG_SYS_HZ/1000; */ #endif but this, as you told me and I agree, is too much architecture dependent (it uses a 64bit unsigned long). The alternative I've proposed, suggested by A.Rubini, is as follow: if (CONFIG_SYS_HZ > 10000) tout *= CONFIG_SYS_HZ/1000; /* for a big HZ, avoid overflow */ else tout = (tout * CONFIG_SYS_HZ) / 1000 + 1; that leads to an evaluation of the timeout in excess of 1 timer tick. I think that an expression like this #if CONFIG_SYS_HZ != 1000 if ((ulong)CONFIG_SYS_HZ > 10000) tout *= ((ulong)CONFIG_SYS_HZ)/1000; /* for a big HZ, avoid overflow */ else tout = (tout * (ulong)CONFIG_SYS_HZ + 500) / 1000; #endif could be better because - it forces the data type of the system dependent CONFIG_SYS_HZ value to ulong (no float!) - it rounds tout to 0.5 timer tick and leaves tout unchanged if CONFIG_SYS_HZ == 1000 What do you think about? Best regards, Renato Andreola The polling time is I've seen that the Wolfgang Denk wrote: > Dear Jean-Christophe PLAGNIOL-VILLARD, > > In message <20090806202615.gh13...@game.jcrosoft.org> you wrote: > >> as we are all supposed to have CONFIG_SYS_HZ at 1000 (mandtory) >> to have cfi, tftp & co working perfectly I do not thing this is a good idea >> > > Yes, this is the rule, and we would like to enforce it. > > >> as you will need to fix each part of u-boot that use CONFIG_SYS_HZ >> which make no sense >> >> the best will be to simply fix your timer >> > > However, the current situation is this: more than 60 boards (all of > them ARM, it seems) use very different settings: > > include/configs/EB+MCF-EV123.h: 10000000 > include/configs/EP1C20.h: > (CONFIG_SYS_CLK_FREQ/(CONFIG_SYS_NIOS_TMRCNT + 1)) > include/configs/EP1S10.h: > (CONFIG_SYS_CLK_FREQ/(CONFIG_SYS_NIOS_TMRCNT + 1)) > include/configs/EP1S40.h: > (CONFIG_SYS_CLK_FREQ/(CONFIG_SYS_NIOS_TMRCNT + 1)) > include/configs/KAREF.h: 100 > include/configs/M5271EVB.h: 1000000 > include/configs/METROBOX.h: 100 > include/configs/MVBLUE.h: 10000 > include/configs/PCI5441.h: > (CONFIG_SYS_CLK_FREQ/(CONFIG_SYS_NIOS_TMRCNT + 1)) > include/configs/PK1C20.h: > (CONFIG_SYS_CLK_FREQ/(CONFIG_SYS_NIOS_TMRCNT + 1)) > include/configs/SMN42.h: 2048 > include/configs/VCMA9.h: 1562500 > include/configs/actux1.h: 3333333 > include/configs/actux2.h: 3333333 > include/configs/actux3.h: 3333333 > include/configs/actux4.h: 3333333 > include/configs/apollon.h: ((CONFIG_SYS_CLK_FREQ)/(2 << > CONFIG_SYS_PTV)) > include/configs/armadillo.h: 2000 > include/configs/assabet.h: 3686400 > include/configs/at91rm9200dk.h: AT91C_MASTER_CLOCK/2 > include/configs/at91rm9200ek.h: (AT91C_MASTER_CLOCK / 2) > include/configs/cmc_pu2.h: (AT91C_MASTER_CLOCK/2) > include/configs/csb637.h: AT91C_MASTER_CLOCK/2 > include/configs/davinci_dm355evm.h: 24000000 > include/configs/davinci_dvevm.h: 27000000 > include/configs/davinci_schmoogie.h: 27000000 > include/configs/davinci_sffsdr.h: 27000000 > include/configs/davinci_sonata.h: 27000000 > include/configs/dnp1110.h: 3686400 > include/configs/eNET.h: 1024 > include/configs/ep7312.h: 2000 > include/configs/gcplus.h: 3686400 > include/configs/idmr.h: (50000000 / 64) > include/configs/impa7.h: 2000 > include/configs/integratorap.h: 24000000 > include/configs/integratorcp.h: 1000000 > include/configs/ixdp425.h: 3333333 > include/configs/kb9202.h: AT91C_MASTER_CLOCK/2 > include/configs/lart.h: 3686400 > include/configs/lpc2292sodimm.h: 2048 > include/configs/lpd7a400-10.h: (508469) > include/configs/lpd7a404-10.h: (508469) > include/configs/m501sk.h: AT91C_MASTER_CLOCK/2 > include/configs/modnet50.h: 900 > include/configs/mp2usb.h: (AT91C_MASTER_CLOCK/2) > include/configs/mx1ads.h: 3686400 > include/configs/mx1fs2.h: 3686400 > include/configs/ns9750dev.h: (CPU_CLK_FREQ/64) > include/configs/omap1610h2.h: ((CONFIG_SYS_CLK_FREQ)/(2 << > CONFIG_SYS_PTV)) > include/configs/omap1610inn.h: ((CONFIG_SYS_CLK_FREQ)/(2 << > CONFIG_SYS_PTV)) > include/configs/omap2420h4.h: ((CONFIG_SYS_CLK_FREQ)/(2 << > CONFIG_SYS_PTV)) > include/configs/omap3_zoom2.h: ((V_SCLK) / (2 << > CONFIG_SYS_PTV)) > include/configs/omap5912osk.h: ((CONFIG_SYS_CLK_FREQ)/(2 << > CONFIG_SYS_PTV)) > include/configs/omap730p2.h: ((CONFIG_SYS_CLK_FREQ)/(2 << > CONFIG_SYS_PTV)) > include/configs/rsk7203.h: (CONFIG_SYS_CLK_FREQ / CMT_CLK_DIVIDER) > include/configs/sbc2410x.h: 1562500 > include/configs/sc520_cdp.h: 1024 > include/configs/sc520_spunk.h: 1024 > include/configs/scb9328.h: 3686400 > include/configs/shannon.h: 3686400 > include/configs/smdk2400.h: 1562500 > include/configs/smdk2410.h: 1562500 > include/configs/trab.h: 1562500 > include/configs/versatile.h: (1000000 / 256) > > > Until all these are fixed, it indeed makes sense to work around issues > cause by such incorrect settings. > > Best regards, > > Wolfgang Denk > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot