[Pdbg] [PATCH v2 14/16] libpdbg: Rework target compatible

Alistair Popple alistair at popple.id.au
Thu Nov 8 11:48:13 AEDT 2018


On Wednesday, 7 November 2018 6:11:06 PM AEDT Amitay Isaacs wrote:
> On Wed, 2018-11-07 at 16:39 +1100, Alistair Popple wrote:
> > Rework the target compatible code to reuse existing libpdbg code
> > where
> > possible. Renames and exports these functions for use by external
> > libraries.
> > 
> > Signed-off-by: Alistair Popple <alistair at popple.id.au>
> > ---
> > 
> >  libpdbg/device.c  | 48 ++++++++++++++++++++++-----------------------
> > 
> > ---
> > 
> >  libpdbg/device.h  | 12 ------------
> >  libpdbg/htm.c     |  8 ++++----
> >  libpdbg/libpdbg.h |  8 ++++++++
> >  libpdbg/p8chip.c  |  2 +-
> >  5 files changed, 35 insertions(+), 43 deletions(-)
> > 
> > diff --git a/libpdbg/device.c b/libpdbg/device.c
> > index 288891b..acc6da2 100644
> > --- a/libpdbg/device.c
> > +++ b/libpdbg/device.c
> > @@ -464,41 +464,37 @@ static const struct dt_property
> > *dt_require_property(const struct pdbg_target *n
> > 
> >  	return p;
> >  
> >  }
> > 
> > -bool dt_prop_find_string(const struct dt_property *p, const char *s)
> > +bool pdbg_target_compatible(struct pdbg_target *target, const char
> > *compatible)
> > 
> >  {
> > 
> > -	const char *c, *end;
> > +        char *c, *end;
> > +        size_t len;
> > 
> > -	if (!p)
> > -		return false;
> > -	c = p->prop;
> > -	end = c + p->len;
> > +        c = pdbg_get_target_property(target, "compatible", &len);
> 
> s/pdbg_get_target_property/pdbg_target_property/

Thanks, will change.

> > +        if (!c)
> > +                return false;
> > 
> > -	while(c < end) {
> > -		if (!strcasecmp(s, c))
> > -			return true;
> > -		c += strlen(c) + 1;
> > -	}
> 
> I could not find how stringlist values are returned from libfdt.  Are
> they returned as concatenated null terminated strings?

I guess? Must admit I didn't dig into all the details here as it was taken 
from Skiboot so I just assumed it worked :-)

- Alistair

> > -	return false;
> > -}
> > +        end = c + len;
> > +        while(c < end) {
> > +                if (!strcasecmp(compatible, c))
> > +                        return true;
> > 
> > -bool dt_node_is_compatible(const struct pdbg_target *node, const
> > char *compat)
> > -{
> > -	const struct dt_property *p = dt_find_property(node,
> > "compatible");
> > +                c += strlen(c) + 1;
> > +        }
> > 
> > -	return dt_prop_find_string(p, compat);
> > +        return false;
> > 
> >  }
> > 
> > -struct pdbg_target *dt_find_compatible_node(struct pdbg_target
> > *root,
> > -					struct pdbg_target *prev,
> > -					const char *compat)
> > +struct pdbg_target *__pdbg_next_compatible_node(struct pdbg_target
> > *root,
> > +                                                struct pdbg_target
> > *prev,
> > +                                                const char *compat)
> > 
> >  {
> > 
> > -	struct pdbg_target *node;
> > +        struct pdbg_target *target;
> > 
> > -	node = prev ? dt_next(root, prev) : root;
> > -	for (; node; node = dt_next(root, node))
> > -		if (dt_node_is_compatible(node, compat))
> > -			return node;
> > -	return NULL;
> > +        target = prev ? dt_next(root, prev) : root;
> > +        for (; target; target = dt_next(root, target))
> > +                if (pdbg_target_compatible(target, compat))
> > +                        return target;
> > +        return NULL;
> > 
> >  }
> >  
> >  static uint32_t dt_prop_get_u32_def(const struct pdbg_target *node,
> > 
> > const char *prop, u32 def)
> > diff --git a/libpdbg/device.h b/libpdbg/device.h
> > index ac265e9..92d7da3 100644
> > --- a/libpdbg/device.h
> > +++ b/libpdbg/device.h
> > @@ -25,16 +25,4 @@
> > 
> >  extern struct pdbg_target *dt_root;
> > 
> > -/* Check a compatible property */
> > -bool dt_node_is_compatible(const struct pdbg_target *node, const
> > char *compat);
> > -
> > -/* Find a node based on compatible property */
> > -struct pdbg_target *dt_find_compatible_node(struct pdbg_target
> > *root,
> > -					struct pdbg_target *prev,
> > -					const char *compat);
> > -
> > -#define dt_for_each_compatible(root, node, compat)	\
> > -	for (node = NULL; 			        \
> > -	     (node = dt_find_compatible_node(root, node, compat)) !=
> > NULL;)
> > -
> > 
> >  #endif /* __DEVICE_H */
> > 
> > diff --git a/libpdbg/htm.c b/libpdbg/htm.c
> > index 1ea2c3a..5f9dd85 100644
> > --- a/libpdbg/htm.c
> > +++ b/libpdbg/htm.c
> > @@ -520,7 +520,7 @@ static int configure_nhtm(struct htm *htm, bool
> > wrap)
> > 
> >  			NHTM_TTYPE_SIZE_MASK ))) /* no pattern matching
> > 
> > */
> > 
> >  		return -1;
> > 
> > -	if (dt_node_is_compatible(&htm->target, "ibm,power9-nhtm")) {
> > +	if (pdbg_target_compatible(&htm->target, "ibm,power9-nhtm")) {
> > 
> >  		if (HTM_ERR(pib_read(&htm->target, NHTM_FLEX_MUX,
> > 
> > &val)))
> > 
> >  			return -1;
> > 
> > @@ -740,7 +740,7 @@ static int htm_toggle_debug_bit(struct htm *htm)
> > 
> >  		return 0; /* nhtm case */
> >  	
> >  	/* FIXME: this is a hack for P8 */
> > 
> > -	if (!dt_node_is_compatible(core, "ibm,power8-core")) {
> > +	if (!pdbg_target_compatible(core, "ibm,power8-core")) {
> > 
> >  		PR_ERROR("HTM is POWER8 only currently\n");
> >  		return -1;
> >  	
> >  	}
> > 
> > @@ -867,7 +867,7 @@ static int do_htm_status(struct htm *htm)
> > 
> >  	uint64_t val, total;
> >  	int i, regs = 9;
> > 
> > -	if (dt_node_is_compatible(&htm->target, "ibm,power9-nhtm"))
> > +	if (pdbg_target_compatible(&htm->target, "ibm,power9-nhtm"))
> > 
> >  		regs++;
> >  	
> >  	PR_INFO("HTM register dump:\n");
> > 
> > @@ -1101,7 +1101,7 @@ static int nhtm_probe(struct pdbg_target
> > *target)
> > 
> >  	if (!is_debugfs_memtrace_ok() || !is_debugfs_scom_ok())
> >  	
> >  		return -1;
> > 
> > -	if (dt_node_is_compatible(target, "ibm,power9-nhtm")) {
> > +	if (pdbg_target_compatible(target, "ibm,power9-nhtm")) {
> > 
> >  		pib_read(target, NHTM_FLEX_MUX, &val);
> >  		if (GETFIELD(NHTM_FLEX_MUX_MASK, val) !=
> > 
> > NHTM_FLEX_DEFAULT) {
> > 
> >  			PR_DEBUG("FLEX_MUX not default\n");
> > 
> > diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h
> > index 6108af9..b61a75c 100644
> > --- a/libpdbg/libpdbg.h
> > +++ b/libpdbg/libpdbg.h
> > @@ -12,6 +12,9 @@ struct pdbg_target;
> > 
> >  struct pdbg_target_class;
> >  
> >  /* loops/iterators */
> > 
> > +struct pdbg_target *__pdbg_next_compatible_node(struct pdbg_target
> > *root,
> > +                                                struct pdbg_target
> > *prev,
> > +                                                const char *compat);
> > 
> >  struct pdbg_target *__pdbg_next_target(const char *klass, struct
> > 
> > pdbg_target *parent, struct pdbg_target *last);
> > 
> >  struct pdbg_target *__pdbg_next_child_target(struct pdbg_target
> > 
> > *parent, struct pdbg_target *last);
> > 
> > @@ -45,6 +48,10 @@ enum pdbg_target_status {PDBG_TARGET_UNKNOWN = 0,
> > PDBG_TARGET_ENABLED,
> > 
> >  			 PDBG_TARGET_DISABLED, PDBG_TARGET_MUSTEXIST,
> >  			 PDBG_TARGET_NONEXISTENT,
> > 
> > PDBG_TARGET_RELEASED};
> > 
> > +#define pdbg_for_each_compatible(parent, target, compat)
> > \
> > +        for (target =
> > NULL;                                             \
> > +             (target = __pdbg_next_compatible_node(parent, target,
> > compat)) != NULL;)
> > +
> > 
> >  #define pdbg_for_each_target(class, parent, target)
> > 
> > \
> > 
> >  	for (target = __pdbg_next_target(class, parent, NULL);
> > 
> > \
> > 
> >  	     target;
> > 
> > \
> > @@ -106,6 +113,7 @@ const char *pdbg_target_dn_name(struct
> > pdbg_target *target);
> > 
> >  void *pdbg_target_priv(struct pdbg_target *target);
> >  void pdbg_target_priv_set(struct pdbg_target *target, void *priv);
> >  struct pdbg_target *pdbg_target_root(void);
> > 
> > +bool pdbg_target_compatible(struct pdbg_target *target, const char
> > *compatible);
> > 
> >  /* Procedures */
> >  int fsi_read(struct pdbg_target *target, uint32_t addr, uint32_t
> > 
> > *val);
> > diff --git a/libpdbg/p8chip.c b/libpdbg/p8chip.c
> > index f3e71a0..914c335 100644
> > --- a/libpdbg/p8chip.c
> > +++ b/libpdbg/p8chip.c
> > @@ -355,7 +355,7 @@ static int p8_ram_setup(struct thread *thread)
> > 
> >  	/* We can only ram a thread if all the threads on the core/chip
> > 
> > are
> > 
> >  	 * quiesced */
> > 
> > -	dt_for_each_compatible(&chip->target, target, "ibm,power8-
> > thread") {
> > +	pdbg_for_each_compatible(&chip->target, target, "ibm,power8-
> > thread") {
> > 
> >  		struct thread *tmp;
> >  		
> >  		/* If this thread wasn't enabled it may not yet have
> > 
> > been probed
> 
> Amitay.




More information about the Pdbg mailing list