On Monday 07 December 2009, Zach Welch wrote:
> Hi all,
>
> The following list describes the things that might still be accomplished
> before the 0.4.0-rc cycle starts. These features are all "mostly done"
> in my tree; they work fine for me in limited testing. They should all
> be safe incremental steps to take.
That "mostly done" bothers me... we need to be winding down current
work which is almost completely merged, not starting a new merge
cycle just to get in "under the wire". So I'm going to read this
instead as mostly about things which might be done in 0.5 ... and
most of which doesn't relate to $SUBJECT, for that matter!
If by $SUBJECT you mean "remove JTAG-related global state from
all interfaces", I'm all for it.
But probably not for 0.4 since it's a huge change ... it would
touch *EVERYTHING* that gets anywhere near JTAG. Like for example
the TAP state logic, and SRST handling, "reset_config", and more.
Such a change would need to go in phases. Maybe some of them could
fit in the remaining part of the 0.4 cycle; but not most of them.
(None of that is sanely factored; the "fact" that there's just one
current JTAG scan chain, TAP state, SRST etc, is *everywhere* in the
code. Even when it's not actually true, e.g. SRST is a pretty rude
signal to try using, unless there's only a single device.)
And then that would need to bubble up to the command language
too. Maybe in a different release -- but eventually. In short,
supporting multiple scan chains would be a major change, not
IMO suitable for rushing into the 0.4 release.
> - generic driver/instance helper module
> - struct object: root of driver-instantiated objects
> - struct driver: root of driver objects (and is-a object itself)
> - Allows adding 'struct module' to provide loadable drivers someday.
Any time I hear about a proposed "struct object", warning
sirens start blaring loudly. They're almost never needed,
except maybe as part of enabling GC in a language runtime.
You can add the notion of a loadable module separately from
all that other stuff.
Loadable _driver_ does call for a clean framework into which
that driver plugs ... and none of our drivers are that clean:
we rely on tables, rather than ${TYPE}_driver_{add,remove}()
calls that get triggered somehow. Where TYPE could of course
be JTAG adapter, CPU/target, NOR driver, NAND driver, and more.
It ought to be easy to drop lots of drivers out of the tree,
by just not compiling and linking them -- without changing
the framework into which they plug. Then later to tweak
those frameworks to say "hmm, no <driver X>, try modprobe
to see if it appears". The enabling comes from just having
clean register-a-driver-to-this-subsystem interfaces.
> - flash driver/instance unification
> - NOR tree is converted to use 'object' and 'driver' model.
> - flash_banks are 'object's of flash_driver device 'driver's
> - NAND tree can be converted similarly (when done with other patches).
Maybe you could start a separate thread on this subject,
showing precisely what you mean. Your single quotes prevent
me from understanding what that "flash_banks are..." means.
There's obvious messiness to clean up there, but I'd have to
guess what you're intending to suggest...
> - multiple JTAG interface API support (internal to the JTAG module only)
> - adds jtag_device (object) to instantiate jtag_interface (driver)
Seems to me the notions we need here would be:
- scan chain, associated with
* state ... collected from all over: reset config, etc
* set of TAPs ... state; some TAPs have targets
* jtag adapter ... driver, with internal state
So hook up multiple adapters, and get multiple scan chains each with
their own TAPs and each of which gets reset differently.
A "jtag_device" would be conceptually confusing; everything we touch
is some flavor of that. Focus instead on what role the device serves,
and make type names match the role.
> - converts internal APIs: jtag_interface, driver commands, bitbang
> - converts drivers to use new APIs, with dummy support many instances
> - moves some global variables into structures
Hmm, I'd rather see the "what" (data structures) before the "how do
we get there" (conversion steps).
> So far, the JTAG conversion has been completely internal to that module,
> so it may not be desirable to introduce this support in 0.4.0 without
> "finishing" the work. Specifically, support must be added at all levels
> of the system; many of the public JTAG APIs must be updated so callers
> pass the required jtag_device structure. This process of associating
> all of the other parts of the system with a specific interface will take
> a larger concerted effort, and it may be too late to get this done.
> Even worse, we must integrate this somehow with the current commands. :/
It's way too late for 0.4, IMO.
The "easy" way to package such a change would be to have one telnet and
TCL port per scan chain, so the *runtime* commands don't need to change.
But that'd still leave a mess for the configuration problem. I think
all the work you've done refactoring the command handling ought to be
able to help sort that out.
> Today, the patches allow updating the internal JTAG APIs to support
> multiple interfaces and allow drivers to encapsulate their own state.
> Indeed, I have updated the 'dummy' driver to show how multiple device
> instances can be supported in one driver with the new interface APIs.
Getting rid of static (presumably!) per-driver state sounds like it's
something that might safely get merged for 0.4 ... at least, "enabling",
plus maybe converting a few drivers. It wouldn't suffice for making
multiple JTAG interfaces work, but it's obviously a necessary step.
Maybe you could post the interface change patch (there is just one??)
for discussion.
Also, the Zylin stuff seems to presuppose there can't be multiple
JTAG adapters -- so that it can inline fragments implementing the
single adapter all over the place. That's not necessarily in big
conflict with well designed hooks; just a compile time option to
turn most of them into stubs. (And for that matter, GCC does have
the "-fwhole-program --combine" optimizations which have helped in
similar cases.)
> When the high-level APIs switch away from using a global device pointer,
> that driver should just work as expected. Others maintainers should
> find it easy to update their own drivers to add this support, even
> though this requires removing all post-configuration global variables.
>
> Mirrored in my 'object', 'mem', and 'jtag' branches on repo.or.cz:
>
> http://repo.or.cz/w/openocd/ztw.git
>
> While I am going to continue looking at the JTAG layer, I believe that
> this current work would be relatively safe to push. It might be best to
> do this integration in stages so each phase gets good testing, but what
> do others think about these changes and overall strategy?
Hmm, I'll have to make time to look at all that. For now, my bias
is that none of this is *needed* for 0.4 ... so I'd say to hold off.
Reviewing code in a repository isn't a good way to go, IMO. I'd
rather see emails proposing interface changes -- first as overview
for discussion, eventually in the form of a reviewable patch -- with
perhaps a brief overview of where those changes are leading, in some
cases.
Plus, having a *small* number of changes to review at a time is a
lot easier to cope with than a wall of patches...
- Dave
_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
