Bearing in mind that any changes are moot until we know whether the patch will even be acceptable with them (Spencer Oliver's concern, for instance)...
On Tue, Dec 16, 2008 at 07:30:59PM -0800, Rick Altherr wrote: > > On Dec 16, 2008, at 9:25 AM, [email protected] wrote: > >> Attached, is a patch against the svn trunk that adds support for the >> Raisonance RLink JTAG adapter. ... > > If you include speed_table.pl to build the speed_table.c file, should > the table be rebuilt during the build process rather than having it be > part of the source base? It seems redundant to include both. Of course. And I initially wanted to do that. However: *) I don't know enough about the build system (autoconf/automake and anything project-specific about it) to make that work. Help is welcome. I will read a document if there is one that will help. *) Building speed_table requires m4 and Perl, and see the above bullet point about me knowing how to properly enforce build-time dependencies like that. *) The xscale target does something similar with its debug binary, which tells me it's acceptable if not preferable. Having it prebuilt addresses some issues. Having everything present that is needed to rebuild it means not just blindly supplying blobs with no idea of where they came from. It also means that the functionality of those blobs can be changed at a later time if need be. > Can the patch and ${PATCHFILE} targets in the Makefile be dropped? Yes. > They > seem to be part of your development process. They are. For one thing, they created the patch you're looking at. They're not hurting anything being there, but they can go. Also, that Makefile is only of concern were one to wish to rebuild speed_table.c. It is just a tool. Like src/target/xscale/build.sh. > > Since you made a new directory for the rlink files, could rlink.c be > moved in there and the jtag directory's Makefile adjusted to build the > rlink directory as a static archive that the jtag.a links against. The > other approach would be to move all of the rlink files into the jtag > folder. Possibly. See above about my understanding of the build system. Moving the rlink.c file into the subdirectory would have the disadvantage of making it even more different from the other drivers than it already is. That disadvantage may be minimal. I'm just trying to consider everything. I created the new directory to try to keep down namespace pollution in the common directory. If all of the interfaces (and targets and flash and whatever else) had their own directories, and already did the static archive thing you mention, there'd be no issue here, but they don't. I'd just be doing the same thing that they do. I was trying to a) come up with something that worked, b) have minimal impact on other stuff. Don't think that I'm against doing it better than it is. I'm just explaining why it is like it is. The biggest reason for the subdirectory is the source and intermediate files for building speed_table.c. The header files just go along for the ride. They could alternately exist in src/jtag with rlink_ prefixes, and that may even be the right and proper thing to do, but it is a slippery slope of a busy namespace in that directory if everybody were to do that. > > speed_table.c is #included. Either speed_table.c should be > speed_table.h I considered that, but something about defining actual storage in a .h file rubbed me the wrong way. It's just a filename. I could change it. It would make no operational difference. > or speed_table.c should be compiled separately and linked > in. It could be. Would I need to do anything other than add rlink_speed_table.o to RLINKFILES in src/jtag/Makefile.am, rename rlink/speed_table.c to rlink_speed_table.c, change static speed_table to extern rlink_speed_table, and somehow convey how long it is? The latter could, of course, just be a const int called rlink_speed_table_length or something like that, since I can't use the sizeof trick to tell how large an array is if it's defined elsewhere and all the C file using it knows is that it's an extern pointer. That would, of course, also require the addition of rlink_speed_table.h, which would be included from both rlink.c and rlink_speed_table.c, and that leads us well down that slippery slope of namespace clutter I alluded to earlier. It would probably just be a lot easier to rename to .h, than to do this. Previously, there wasn't even a speed_table.c. The khz/prescaler table was directly in rlink.c, and the DTC blobs were loaded from binary files at runtime. That still had the same issue of how/when to build those files and added the possibility of mismatch between the version of the rlink driver and the version of the blob files found on a given system. While I generally dislike programs embedding blobs in themselves like that, the cons of having the files separate but in the search path outweighed the niceness of not embedding. _______________________________________________ Openocd-development mailing list [email protected] https://lists.berlios.de/mailman/listinfo/openocd-development
