Subject: Re: [PATCH 4/6] hw_breakpoints: Breakpoints arch ability don't need perf events

From: Frederic Weisbecker <[email-masked]>
Date: Mon, 4 Jul 2011 15:37:54 +0200

On Mon, Jul 04, 2011 at 07:02:23PM +0530, K.Prasad wrote:
> On Tue, May 24, 2011 at 11:52:25PM +0200, Frederic Weisbecker wrote:
> > The breakpoint support ability in an arch is not related
> > to the fact perf events is built or not. HAVE_HW_BREAKPOINT
> > only shows an ability so this dependency makes no sense
> > anymore. Archs that select HAVE_HW_BREAKPOINT already
> > ensure that perf event is built.
> > 
> > Remove that dependency.
> > 
> > Signed-off-by: Frederic Weisbecker <[email-masked]>
> > Cc: Ingo Molnar <[email-masked]>
> > Cc: Peter Zijlstra <[email-masked]>
> > Cc: Will Deacon <[email-masked]>
> > Cc: Prasad <[email-masked]>
> > Cc: Paul Mundt <[email-masked]>
> > ---
> >  arch/Kconfig |    1 -
> >  1 files changed, 0 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index f78c2be..ce4be89 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -149,7 +149,6 @@ config HAVE_DEFAULT_NO_SPIN_MUTEXES
> > 
> >  config HAVE_HW_BREAKPOINT
> >  	bool
> > -	depends on PERF_EVENTS
> > 
> >  config HAVE_MIXED_BREAKPOINTS_REGS
> >  	bool
> > -- 
> 
> Just a thought you might want to consider...
> 
> The need to keep the ability (HAVE_HW_BREAKPOINT) and the user-choice to
> enable hardware breakpoints (through HW_BREAKPOINT) in separate config
> options isn't very clear to me (and is a bit confusing with very similar
> names).
> 
> Why not make HAVE_HW_BREAKPOINT selectable by the user (which in turn
> would turn on PERF_EVENTS) for a given architecture?

But then how kconfig knows if the allows that? You need to know if the
arch has the ability to support breakpoints.

This is a commin pattern in Linux Kconfig. Things are often seperated between
ability (some constant value provided by the arch) and the user choice that
depends on that ability.
The advantage of doing this is that you can centralize the generic dependencies,
help menu, etc... into a single place.
 
> Thanks,
> K.Prasad