[Pdbg] [PATCH 04/10] path: Add device tree path based targeting

[Amitay Isaacs]

On Thu, 2018-11-01 at 12:56 +1100, Alistair Popple wrote:
> > +/**
> > + * @brief Iterator for list of path targets
> > + *
> > + * @param[in]  prev The last pdbg target
> > + * @param[in]  only_enabled Whether to list enabled targets or all
> 
> Just a minor documentation nit-pick ... reading that makes me think
> it would 
> just check the status. However it also has the side-effect of trying
> to probe 
> the device first.

Hmm..  I added probing in line with the current code.  But now I think
that this code should not probe targets at all.  That way there are no
side-effects.

I think we should add explicit probing before running the command and
explicit release after the command has executed.  That way you don't
need incremental probing as done in the for_each_target and
for_each_child_target macros.

How about something like:

   for_each_path_target(target)
      pdbg_target_probe(target);

   rc = cmd(args, flags);

   for_each_path_target(target)
      pdbg_target_release(target); 

Then each command does not have to worry about the probing targets and
only use for_each_path_target_enabled() or
for_each_path_target_class().  May be we will also need
for_each_path_target_class_enabled().

> In general I think it would be nicer to have seperate macros/calls
> for probed 
> and enabled only vs. iterating over all rather than a bool flag whos
> meaning is 
> a little less obvious when reading the calls in code.

Sure. 

Amitay.
-- 

The best God is one that gives nothing and demands nothing. That is 
the only God you can count on.  - Conan