On Fri, 2009-12-11 at 11:53 +1100, David Gibson wrote:
> On Thu, Dec 10, 2009 at 01:57:21PM -0200, Dave Kleikamp wrote:
> > powerpc: Add definitions for Debug Registers on BookE Platforms
> > 
> > From: Torez Smith <lnxto...@linux.vnet.ibm.com>
> > 
> > This patch adds additional definitions for BookE Debug Registers
> > to the reg_booke.h header file.
> > 
> > Signed-off-by: Torez Smith  <lnxto...@linux.vnet.ibm.com>
> > Signed-off-by: Dave Kleikamp <sha...@linux.vnet.ibm.com>
> 
> As with patch 1/3, none of the comments below is anything that
> couldn't be fixed up after merging.  So,
> 
> Acked-by: David Gibson <d...@au1.ibm.com>
> 
> > Cc: Benjamin Herrenschmidt <b...@kernel.crashing.org>
> > Cc: Thiago Jung Bauermann <bauer...@br.ibm.com>
> > Cc: Sergio Durigan Junior <sergi...@br.ibm.com>
> > Cc: David Gibson <d...@au1.ibm.com>
> > Cc: linuxppc-dev list <linuxppc-...@ozlabs.org>
> > ---
> > 
> >  arch/powerpc/include/asm/processor.h |   30 +++++-
> >  arch/powerpc/include/asm/reg_booke.h |  176 
> > +++++++++++++++++++++++++++++-----
> >  2 files changed, 178 insertions(+), 28 deletions(-)
> 
> [snip]
> > +   /*
> > +    * The following will contain addresses used by debug applications
> > +    * to help trace and trap on particular address locations.
> > +    * The bits in the Debug Control Registers above help define which
> > +    * of the following registers will contain valid data and/or addresses.
> > +    */
> > +   unsigned long   iac1;
> > +   unsigned long   iac2;
> > +   unsigned long   iac3;
> > +   unsigned long   iac4;
> > +   unsigned long   dac1;
> > +   unsigned long   dac2;
> > +   unsigned long   dvc1;
> > +   unsigned long   dvc2;
> 
> I think you'd make the logic in patch 3 substantially easier, if you
> defined these as
>       unsigned long iac[4];
>       unsigned long dac[2];
>       unsigned long dvc[2];
> instead of as individual structure members.

I'll give that a look.  You're probably right.

> [snip]
> > +#define DBCR0_USER_DEBUG   (DBCR0_IDM | DBCR0_ICMP | DBCR0_IAC1 | \
> > +                            DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4)
> > +#define DBCR0_BASE_REG_VALUE       0
> 
> These constants are left over from when the interface allowed
> more-or-less direct access to the debug regs.  I don't think the
> USER_DEBUG constant is used at all any more, and the BASE_REG_VALUE is
> just used in the load_default function, and might as well be inline
> there.

Right.

> [snip]
> > +
> > +#define dbcr_iac_range(task)       ((task)->thread.dbcr0)
> 
> Hrm, I think the way these macros work to do the 40x vs. BookE
> abstration is kind of ugly.  But an unequivocally better way doesn't
> immediately occur to me.

Without this, the ifdef's were horrendous.  I'm open to renaming this or
redefining it to be more intuitive if anyone has a better idea.
-- 
David Kleikamp
IBM Linux Technology Center

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to