On Tue, Nov 17, 2009 at 01:26:55AM +0100, Tanguy Pruvot wrote: > Hello, > > This is my last email for patches, > You can find (last) whole sources + binary here : > http://tanguy.ath.cx/index.php?q=M900 > > Please apply my patch to the last git commit... Thanks
Hi Tanguy, Thanks for providing the patches. I have some comments on them. The 0001 patch includes some changes to include/mach-types.h. Instead of patching this file, we just update to the latest - see the file docs/build-mach-types.txt. I've just updated to the latest, but I see that the ids in your patch don't match the official version. Please take a look at this. I noticed that the patch series adds files/lines, and then later deletes them. This makes it hard to review what's changing. Can you send a patch (or patch series) with the final goal known? There are changes to haret.vcp - what is this for? As far as I knew, that file was just a leftover from before cegcc was used. What are the new icons for? Is it just an update of the picture or is it for a different screen resolution? > +++ b/src/mach/arch-s3.cpp > @@ -1,8 +1,11 @@ > +#include <windows.h> // Sleep You can't call Sleep (or any Windows functions) in hardwareShutdown(). Doing so will likely cause random machine hangs. When the shutdown is called, interrupts are off, and calling a Windows function could lead to a fault, or it could let Windows reenable interrupts which could then cause a fault in the linux boot phase. > /**************************************************************** > - * S3c64x0 > + * S3c64x0 - Tanguy Pruvot on Gmail > ****************************************************************/ Please don't add "I changed this" type messages to the code - that's what WHATSNEW is for. > +/* "addlist GPIOS p2v(0x7f008000)\n" // GPACON Also, please don't commit dead code. If it's no longer needed, just delete it. > + volatile uint32 * dma_base[4]; Adding volatile to a data structure member is almost never a good idea. Best thing is to put the volatile on each access or use a readl/writel type function. -Kevin _______________________________________________ Haret mailing list [email protected] https://handhelds.org/mailman/listinfo/haret
