On Wed, 2009-12-02 at 23:50 +0100, Øyvind Harboe wrote:
> I was thinking about splitting this patch into at least a few:
> 
> The first would be to get rid of some -I's.

Ummm... hold off on this.

> The second would clarify that the "classic" JTAG driver
> is just another minidriver.
> 
> The third would then allow a minidriver to grab the
> JTAG stack from jtag_xxx() and down to the metal.

The driver should be exposed via jtag/interface.h, so only code that
needs to define portions of the interface drivers can ever see it.
That header is also part of our public API.  However, the regular driver
and the minidrivers and mutually exclusive.

> The performance gain potential here is, over time with
> a few more fixes like it, ca. 10-25%. I currently get 550kBytes/s
> @ 16MHz w/arm7/9 for gdb load, so say up to 600-700kBytes/s
> somewhere eventually due to this. Performance is a long
> term & fun project for me :-)
> 
> I actually saw that jtag_add_dr_out() was no longer inlined
> because it is creeping up on the gprof list for gdb load...

Okay.  The solution that I have tried to propose will get this back,
without exposing anything during a normal non-minidriver installation.

At the point, I would prefer you wait to work on this until I push two
of my pending clean-up series, both of which I have nearly done:
- Split flash/ into flash/nor/ and flash/nand/.
- Change headers from "foo.h" to <module/foo.h>

The last is fully done, on my mirror in the 'hdr' branch.  The first is
half-done, with the nand changes on my mirror master branch.  That part
took an hour, so I expect to finish the nor and push it all soonish. 
Likewise, the JTAG changes are trivial to accomplish.  The 'hdr' branch
can then follow shortly.  So, get a good night's sleep and expect the
landscape to have changed a little bit while you dream....  

The changes will require using full path names in the public header
files, so you cannot use "minidriver.h".  Thinking about this, we can:

     1. move the drivers into jtag/drivers and create the empty
        minidriver.h in that location, 
     2. Copy jtag/{drivers,minidriver}/minidriver.h to jtag/minidriver.h
        using a Makefile magic.
     3. jtag.h can include <jtag/minidriver.h>

When the "minidriver/minidriver.h" file is used, it can include
"jtag_minidriver.h" -- as you have already done, but with the -I set via
Makefile magic to remove confusing #if logic.  The downside of this is
that it forces us to include <jtag/minidriver.h> in the installed
headers, and it will always be empty.  Okay, so this turns out to suck.

Another possible solution is to add <jtag/imp.h>, which #includes
<jtag/jtag.h> (the public API) and does not get installed.  This file is
#included in the tree and can #include "minidriver.h".  This is probably
the simplest idea, yeah?  The public API _must_ be a non-inline version
of the API that calls the inline version.  Period.  But the public API
will never use a minidriver; such builds will always be monolithic. Yah?

Inline functions may not be used in public APIs, because they prevent
upgrading shared libraries.  They are included in application, so
changes to their implementation will not be updated if the library is.
While real backwards compatibility will not be an issue until 1.0, these
kinds of issues need to be considered as we move toward that milestone,
or it will be impossible to fix bugs _and_ remain ABI compatible.

Honestly, I am more convinced that this reflects fundamental design
flaws in the system.  It really feels like we are polishing a turd.

Cheers,

Zach

_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

Reply via email to