On 02.01.2011 13:43, Alexander Holler wrote: > Am 01.01.2011 20:21, schrieb Dirk Behme: >> On 01.01.2011 19:47, Alexander Holler wrote: >>> Am 01.01.2011 19:25, schrieb Dirk Behme: >>>> On 01.01.2011 18:52, Alexander Holler wrote: >>>>> Hello, >>>>> >>>>> Am 01.01.2011 13:04, schrieb Dirk Behme: >>>>>> On 22.12.2010 12:04, Alexander Holler wrote: >>>>>>> gcc 4.5.1 seems to ignore (at least some) volatile definitions, >>>>>>> avoid that as done in the kernel. >>>>>>> >>>>>>> Reading C99 6.7.3 8 and the comment 114) there, I think it is a >>>>>>> bug of >>>>>>> that >>>>>>> gcc version to ignore the volatile type qualifier used e.g. in >>>>>>> __arch_getl(). >>>>>>> Anyway, using a definition as in the kernel headers avoids such >>>>>>> optimizations when >>>>>>> gcc 4.5.1 is used. >>>>>>> >>>>>>> Maybe the headers as used in the current linux-kernel should be >>>>>>> used, >>>>>>> but to avoid large changes, I've just added a small change to the >>>>>>> current headers. >>>>> >>>>>> Do you like to test the patch in the attachment? I named it 'v4'. >>>>>> >>>>>> After some thinking and testing, it seems to me that the volatile >>>>>> optimization issue this patch shall fix is only with the readx() >>>>>> macros. >>>>>> So the idea is to drop all writex() changes done in the v3 >>>>>> version of >>>>>> this patch. With dropping the writex() changes, we would drop all >>>>>> issues >>>>>> we discussed with e.g. the GCC statement-expression and the do >>>>>> while >>>>>> workaround, too. >>>>> >>>>> I've come across a bug which reads as the problem might be fixed in >>>>> gcc 4.5.2: >>>>> >>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45052 >>>>> >>>>> I will test gcc 4.5.2 in the next days. >>>> >>>> Have you been able to test v4 of the patch I sent with gcc 4.5.1? >>> >>> No, sorry, I don't have a test case for consequent write* and I will >>> have to write one. >> >> ? >> >> If I remember correctly, the test case for this patch was compiling >> U-Boot with 4.5.1 and then check >> >> a) if it boots at Beagle (correct clock.c) >> b) if NAND works ok (correct omap_gpmc.c) >> >> ? > > No. None of those must fail when the compiler optimizes consequent > write* to one write* because the compiler ignores the volatile keyword. > I've only found the problem with consequent read* (in clock.c), but > there might be problems with consequent write* somewhere else too. So > if you remove the change for those write* some other problems might > arise and just through booting a kernel those might not be found. So I > think it would be dangerous to remove the change for write* when using > gcc 4.5.x > > And because the patch fixes only write* and read* some stuff in u-boot > which uses volatile in another context might still fail, therefore I > vote to use the current kernel headers where other things besides > read* and write* are using those barriers too.
Just to understand correctly: Do you want to say that we should ignore your v3 patch http://lists.denx.de/pipermail/u-boot/2010-December/084132.html ? And that you didn't test the v4 patch http://lists.denx.de/pipermail/u-boot/2011-January/084481.html with the test you did in http://lists.denx.de/pipermail/u-boot/2010-December/084134.html ("tested with both gcc 4.3.5 and gcc 4.5.1 using binutils 2.20.1") because you now think this test isn't sufficient? Thanks Dirk _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot