[Pdbg] [PATCH 24/29] htm: Check that nodes aren't disabled
Cyril Bur
cyrilbur at gmail.com
Tue Feb 13 11:14:20 AEDT 2018
On Mon, 2018-02-12 at 13:55 +1100, Alistair Popple wrote:
> On Friday, 9 February 2018 3:38:52 PM AEDT Cyril Bur wrote:
> > Signed-off-by: Cyril Bur <cyrilbur at gmail.com>
> > ---
> > libpdbg/device.c | 10 ++++++++
> > libpdbg/device.h | 3 +++
> > src/htm.c | 75 ++++++++++++++++++++++++++++++++++++++++++--------------
> > src/main.h | 5 ++++
> > 4 files changed, 75 insertions(+), 18 deletions(-)
> >
> > diff --git a/libpdbg/device.c b/libpdbg/device.c
> > index e7b8ee0..55c6ea7 100644
> > --- a/libpdbg/device.c
> > +++ b/libpdbg/device.c
> > @@ -942,3 +942,13 @@ bool dt_node_is_enabled(struct dt_node *node)
> >
> > return p->len > 1 && p->prop[0] == 'o' && p->prop[1] == 'k';
> > }
> > +
> > +bool dt_node_is_disabled(struct dt_node *node)
> > +{
> > + const struct dt_property *p = dt_find_property(node, "status");
> > +
> > + if (!p)
> > + return false;
> > +
> > + return strcmp(p->prop, "disabled") == 0;
> > +}
>
> Why add this function rather than fix up dt_node_is_enabled() above? Note that
> device.c was originally taken in its entirety from Skiboot so there is a no
> doubt a bunch of code in there which is either wrong or unecessary, feel free to
> butcher it as required.
>
Sorry this hunk is left over from my original work before your API
patch. I'll remove it.
> In any case we shouldn't need either of these - why not just use
> pdbg_target_status(target) == PDBG_TARGET_DISABLED?
>
Yeah that's the correct way now that you've made a concrete API.
> - Alistair
>
> > diff --git a/libpdbg/device.h b/libpdbg/device.h
> > index 4bc60a7..0033288 100644
> > --- a/libpdbg/device.h
> > +++ b/libpdbg/device.h
> > @@ -167,6 +167,9 @@ struct dt_node *dt_find_compatible_node_on_chip(struct dt_node *root,
> > /* Check status property */
> > bool dt_node_is_enabled(struct dt_node *node);
> >
> > +/* Check status property */
> > +bool dt_node_is_disabled(struct dt_node *node);
> > +
> > /* Build the full path for a node. Return a new block of memory, caller
> > * shall free() it
> > */
> > diff --git a/src/htm.c b/src/htm.c
> > index b03aca0..d6638ea 100644
> > --- a/src/htm.c
> > +++ b/src/htm.c
> > @@ -23,6 +23,8 @@
> > #include <target.h>
> > #include <operations.h>
> >
> > +#include "main.h"
> > +
> > #define HTM_DUMP_BASENAME "htm.dump"
> >
> > static char *get_htm_dump_filename(void)
> > @@ -50,11 +52,15 @@ int run_htm_start(int optind, int argc, char *argv[])
> > struct pdbg_target *target;
> > int rc = 0;
> >
> > - pdbg_for_each_class_target("htm", target) {
> > - uint32_t index = pdbg_target_index(target);
> > + pdbg_for_each_class_target("nhtm", target) {
> > uint64_t chip_id;
> > + uint32_t index;
> > +
> > + if (target_is_disabled(target))
> > + continue;
> >
> > assert(!pdbg_get_u64_property(target, "chip-id", &chip_id));
> > + index = pdbg_target_index(target);
> > printf("Starting HTM@%" PRIu64 "#%d\n", chip_id, index);
> > if (htm_start(target) != 1)
> > printf("Couldn't start HTM@%" PRIu64 "#%d\n", chip_id, index);
> > @@ -69,10 +75,14 @@ int run_htm_stop(int optind, int argc, char *argv[])
> > struct pdbg_target *target;
> > int rc = 0;
> >
> > - pdbg_for_each_class_target("htm", target) {
> > - uint32_t index = pdbg_target_index(target);
> > + pdbg_for_each_class_target("nhtm", target) {
> > uint64_t chip_id;
> > + uint32_t index;
> >
> > + if (target_is_disabled(target))
> > + continue;
> > +
> > + index = pdbg_target_index(target);
> > assert(!pdbg_get_u64_property(target, "chip-id", &chip_id));
> > printf("Stopping HTM@%" PRIu64 "#%d\n", chip_id, index);
> > if (htm_stop(target) != 1)
> > @@ -88,10 +98,14 @@ int run_htm_status(int optind, int argc, char *argv[])
> > struct pdbg_target *target;
> > int rc = 0;
> >
> > - pdbg_for_each_class_target("htm", target) {
> > - uint32_t index = pdbg_target_index(target);
> > + pdbg_for_each_class_target("nhtm", target) {
> > uint64_t chip_id;
> > + uint32_t index;
> > +
> > + if (target_is_disabled(target))
> > + continue;
> >
> > + index = pdbg_target_index(target);
> > assert(!pdbg_get_u64_property(target, "chip-id", &chip_id));
> > printf("HTM@%" PRIu64 "#%d\n", chip_id, index);
> > if (htm_status(target) != 1)
> > @@ -109,10 +123,14 @@ int run_htm_reset(int optind, int argc, char *argv[])
> > struct pdbg_target *target;
> > int rc = 0;
> >
> > - pdbg_for_each_class_target("htm", target) {
> > - uint32_t index = pdbg_target_index(target);
> > + pdbg_for_each_class_target("nhtm", target) {
> > uint64_t chip_id;
> > + uint32_t index;
> >
> > + if (target_is_disabled(target))
> > + continue;
> > +
> > + index = pdbg_target_index(target);
> > assert(!pdbg_get_u64_property(target, "chip-id", &chip_id));
> > printf("Resetting HTM@%" PRIu64 "#%d\n", chip_id, index);
> > if (htm_reset(target, &base, &size) != 1)
> > @@ -143,10 +161,14 @@ int run_htm_dump(int optind, int argc, char *argv[])
> >
> > /* size = 0 will dump everything */
> > printf("Dumping HTM trace to file [chip].[#]%s\n", filename);
> > - pdbg_for_each_class_target("htm", target) {
> > - uint32_t index = pdbg_target_index(target);
> > + pdbg_for_each_class_target("nhtm", target) {
> > uint64_t chip_id;
> > + uint32_t index;
> > +
> > + if (target_is_disabled(target))
> > + continue;
> >
> > + index = pdbg_target_index(target);
> > assert(!pdbg_get_u64_property(target, "chip-id", &chip_id));
> > printf("Dumping HTM@%" PRIu64 "#%d\n", chip_id, index);
> > if (htm_dump(target, 0, filename) == 1)
> > @@ -164,15 +186,20 @@ int run_htm_trace(int optind, int argc, char *argv[])
> > struct pdbg_target *target;
> > int rc = 0;
> >
> > - pdbg_for_each_class_target("htm", target) {
> > - uint32_t index = pdbg_target_index(target);
> > + pdbg_for_each_class_target("nhtm", target) {
> > uint64_t chip_id;
> > + uint32_t index;
> > +
> > + if (target_is_disabled(target))
> > + continue;
> > +
> > + index = pdbg_target_index(target);
> > + assert(!pdbg_get_u64_property(target, "chip-id", &chip_id));
> >
> > /*
> > * Don't mind if stop fails, it will fail if it wasn't
> > * running, if anything bad is happening reset will fail
> > */
> > - assert(!pdbg_get_u64_property(target, "chip-id", &chip_id));
> > htm_stop(target);
> > printf("Resetting HTM@%" PRIu64 "#%d\n", chip_id, index);
> > if (htm_reset(target, &base, &size) != 1)
> > @@ -187,10 +214,14 @@ int run_htm_trace(int optind, int argc, char *argv[])
> > old_base = base;
> > }
> >
> > - pdbg_for_each_class_target("htm", target) {
> > - uint32_t index = pdbg_target_index(target);
> > + pdbg_for_each_class_target("nhtm", target) {
> > uint64_t chip_id;
> > + uint32_t index;
> > +
> > + if (target_is_disabled(target))
> > + continue;
> >
> > + index = pdbg_target_index(target);
> > assert(!pdbg_get_u64_property(target, "chip-id", &chip_id));
> > printf("Starting HTM@%" PRIu64 "#%d\n", chip_id, index);
> > if (htm_start(target) != 1)
> > @@ -207,18 +238,26 @@ int run_htm_analyse(int optind, int argc, char *argv[])
> > char *filename;
> > int rc = 0;
> >
> > - pdbg_for_each_class_target("htm", target)
> > + pdbg_for_each_class_target("nhtm", target) {
> > + if (target_is_disabled(target))
> > + continue;
> > +
> > htm_stop(target);
> > + }
> >
> > filename = get_htm_dump_filename();
> > if (!filename)
> > return 0;
> >
> > printf("Dumping HTM trace to file [chip].[#]%s\n", filename);
> > - pdbg_for_each_class_target("htm", target) {
> > - uint32_t index = pdbg_target_index(target);
> > + pdbg_for_each_class_target("nhtm", target) {
> > uint64_t chip_id;
> > + uint32_t index;
> > +
> > + if (target_is_disabled(target))
> > + continue;
> >
> > + index = pdbg_target_index(target);
> > assert(!pdbg_get_u64_property(target, "chip-id", &chip_id));
> > printf("Dumping HTM@%" PRIu64 "#%d\n", chip_id, index);
> > if (htm_dump(target, 0, filename) != 1)
> > diff --git a/src/main.h b/src/main.h
> > index e47d3dc..1bfdd99 100644
> > --- a/src/main.h
> > +++ b/src/main.h
> > @@ -17,6 +17,11 @@
> >
> > #include <target.h>
> >
> > +static inline bool target_is_disabled(struct pdbg_target *target)
> > +{
> > + return pdbg_target_index(target) == PDBG_TARGET_DISABLED;
> > +}
> > +
> > /* Returns the sum of return codes. This can be used to count how many targets the callback was run on. */
> > int for_each_child_target(char *class, struct pdbg_target *parent,
> > int (*cb)(struct pdbg_target *, uint32_t, uint64_t *, uint64_t *),
> >
>
>
More information about the Pdbg
mailing list