On Sun, 25 Nov 2007, Catalin Patulea wrote:
There are at least two issues with it that I would appreciate feedback on: - It's not platform-selective. Right now, it's in the dependency tree for all platforms. I'm guessing all it would take to fix this is some sort of ifdef on the dependency at firmware/Makefile line 27. What can I test in the Makefile for the platform?
You can, but I consider it nicer if you check for a particular value or config that is set in the configure that isn't the exact model number/name. So that for example future dms320-targets can take advantage of the fix without adding the target names to the test.
- Because I'm using recursive make, the dependencies aren't quite right. There's a workaround in place at firmware/Makefile line 52 (the source files are dependencies of the top-level rule). Without that workaround, if I change, say registers.h and type "make" in the build directory, Rockbox doesn't get rebuilt (because registers.h isn't in the top-level dsp-image.h rule).
That's a general problem we have in several places in the build system actually, but it is a hard nut and so far we've mostly closed our eyes for the issues and tried to live on anyway! ;-) (There's even a dedicated hack in make.inc to fix the lang.h problem that is similar.)
My biggest thought when I read the patch was: why is this generating a .h file? Isn't that stuff generating some kind of data or something that is better (more accurately) placed in a .c file?
-- Daniel Stenberg -- http://www.rockbox.org/ -- http://daniel.haxx.se/
