SA_TRIGGER_* vs. SA_SAMPLE_RANDOM

SA_TRIGGER_* vs. SA_SAMPLE_RANDOM

[David Miller]

Since SA_SAMPLE_RANDOM is defined as "SA_RESTART", it
could be just about any value.

On sparc, it's value is "2", so it aliases some of
the SA_TRIGGER_* defines the new genirq code adds.
And therefore we get a bunch of these on sparc64:

[   16.650540] setup_irq(2) SA_TRIGGERset. No set_type function available

(btw: missing space in the kernel log message between 'SA_TRIGGER'
      and 'set' :-)

I can't see any reason why SA_SAMPLE_RANDOM is set to
a signal mask value, or why IRQ flags are defined in
linux/signal.h :-)

Anyways, probably the best bet for now is to define
SA_SAMPLE_RANDOM explicitly to some value instead of
relying on the arbitrary platform definition of SA_RANDOM.

Ingo could you cook up and submit a patch which does this?
Thanks.

Re: SA_TRIGGER_* vs. SA_SAMPLE_RANDOM

[Thomas Gleixner]

David,

On Thu, 2006-06-29 at 14:17 -0700, David Miller wrote:
> Since SA_SAMPLE_RANDOM is defined as "SA_RESTART", it
> could be just about any value.
> 
> On sparc, it's value is "2", so it aliases some of
> the SA_TRIGGER_* defines the new genirq code adds.
> And therefore we get a bunch of these on sparc64:
> 
> [   16.650540] setup_irq(2) SA_TRIGGERset. No set_type function available
> 
> (btw: missing space in the kernel log message between 'SA_TRIGGER'
>       and 'set' :-)
> 
> I can't see any reason why SA_SAMPLE_RANDOM is set to
> a signal mask value, or why IRQ flags are defined in
> linux/signal.h :-)
> 
> Anyways, probably the best bet for now is to define
> SA_SAMPLE_RANDOM explicitly to some value instead of
> relying on the arbitrary platform definition of SA_RANDOM.
> 
> Ingo could you cook up and submit a patch which does this?
> Thanks.

We have the same hassle with SA_INTERRUPT. The question arises, if we
should move the SA_XX flags for interrupts completely out of the signal
SA name space. Rename to IRQ_xxx and put them into interrupt.h.

	tglx

Re: SA_TRIGGER_* vs. SA_SAMPLE_RANDOM

[Russell King]

On Fri, Jun 30, 2006 at 04:00:07PM +0200, Thomas Gleixner wrote:
> On Thu, 2006-06-29 at 14:17 -0700, David Miller wrote:
> > Since SA_SAMPLE_RANDOM is defined as "SA_RESTART", it
> > could be just about any value.
> > 
> > On sparc, it's value is "2", so it aliases some of
> > the SA_TRIGGER_* defines the new genirq code adds.
> > And therefore we get a bunch of these on sparc64:
> > 
> > [   16.650540] setup_irq(2) SA_TRIGGERset. No set_type function available
> > 
> > (btw: missing space in the kernel log message between 'SA_TRIGGER'
> >       and 'set' :-)
> > 
> > I can't see any reason why SA_SAMPLE_RANDOM is set to
> > a signal mask value, or why IRQ flags are defined in
> > linux/signal.h :-)
> > 
> > Anyways, probably the best bet for now is to define
> > SA_SAMPLE_RANDOM explicitly to some value instead of
> > relying on the arbitrary platform definition of SA_RANDOM.
> > 
> > Ingo could you cook up and submit a patch which does this?
> > Thanks.
> 
> We have the same hassle with SA_INTERRUPT. The question arises, if we
> should move the SA_XX flags for interrupts completely out of the signal
> SA name space. Rename to IRQ_xxx and put them into interrupt.h.

It would probably be sensible, but isn't there rather a lot of
drivers to update?  We could do it as a transitional thing -
#define the old SA_* names to the new in interrupt.h for a while.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: SA_TRIGGER_* vs. SA_SAMPLE_RANDOM

[David Miller]

From: Russell King <rmk+lkml@arm.linux.org.uk>
Date: Fri, 30 Jun 2006 19:47:45 +0100

> On Fri, Jun 30, 2006 at 04:00:07PM +0200, Thomas Gleixner wrote:
> > We have the same hassle with SA_INTERRUPT. The question arises, if we
> > should move the SA_XX flags for interrupts completely out of the signal
> > SA name space. Rename to IRQ_xxx and put them into interrupt.h.
> 
> It would probably be sensible, but isn't there rather a lot of
> drivers to update?  We could do it as a transitional thing -
> #define the old SA_* names to the new in interrupt.h for a while.

This seems like a sane plan.

Someone skilled in sed and awk could probably do the whole
current tree up in a short amount of time though :-)

I would then only keep the existing defines around for the
sake of being polite to out-of-tree folks :)  Put them in
the deprecation schedule, then zap them for good a few months
from now.

Re: SA_TRIGGER_* vs. SA_SAMPLE_RANDOM

[Thomas Gleixner]

On Fri, 2006-06-30 at 13:21 -0700, David Miller wrote:
> From: Russell King <rmk+lkml@arm.linux.org.uk>
> Date: Fri, 30 Jun 2006 19:47:45 +0100
> 
> > On Fri, Jun 30, 2006 at 04:00:07PM +0200, Thomas Gleixner wrote:
> > > We have the same hassle with SA_INTERRUPT. The question arises, if we
> > > should move the SA_XX flags for interrupts completely out of the signal
> > > SA name space. Rename to IRQ_xxx and put them into interrupt.h.
> > 
> > It would probably be sensible, but isn't there rather a lot of
> > drivers to update?  We could do it as a transitional thing -
> > #define the old SA_* names to the new in interrupt.h for a while.
> 
> This seems like a sane plan.
> 
> Someone skilled in sed and awk could probably do the whole
> current tree up in a short amount of time though :-)
> 
> I would then only keep the existing defines around for the
> sake of being polite to out-of-tree folks :)  Put them in
> the deprecation schedule, then zap them for good a few months
> from now.

I'll cook it up tomorrow.

	tglx

Re: SA_TRIGGER_* vs. SA_SAMPLE_RANDOM

[David Miller]

From: Thomas Gleixner <tglx@linutronix.de>
Date: Fri, 30 Jun 2006 22:27:27 +0200

> I'll cook it up tomorrow.

Thanks a lot Thomas. :)

Re: SA_TRIGGER_* vs. SA_SAMPLE_RANDOM

[Thomas Gleixner]

On Fri, 2006-06-30 at 13:31 -0700, David Miller wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> Date: Fri, 30 Jun 2006 22:27:27 +0200
> 
> > I'll cook it up tomorrow.
> 
> Thanks a lot Thomas. :)

That's what I came up with:

SA_INTERRUPT		IRQF_IRQS_DISABLED
SA_SAMPLE_RANDOM	IRQF_SAMPLE_RANDOM
SA_SHIRQ		IRQF_SHARE_IRQ
SA_PROBEIRQ		IRQF_PROBE_IRQ
SA_TRIGGER_LOW		IRQF_TRIGGER_LOW
SA_TRIGGER_HIGH		IRQF_TRIGGER_HIGH
SA_TRIGGER_FALLING	IRQF_TRIGGER_FALLING
SA_TRIGGER_RISING	IRQF_TRIGGER_RISING
SA_TRIGGER_MASK		IRQF_TRIGGER_MASK
SA_TIMER		IRQF_TIMER

It affects only 720 files :)

	tglx

Re: SA_TRIGGER_* vs. SA_SAMPLE_RANDOM

[Benjamin Herrenschmidt]

> We have the same hassle with SA_INTERRUPT. The question arises, if we
> should move the SA_XX flags for interrupts completely out of the signal
> SA name space. Rename to IRQ_xxx and put them into interrupt.h.

Agreed. In addition, if you look specifically at triggers, I've seen so
far:

 - The new SA_* trigger stuff
 - The IRQ_TYPE_* stuff
 - The definitions of bits in struct resource of interrupts in ioport.h
 - The old IRQ_LEVEL
 - Various arch specific things that duplicate it again

In addition, it's different bits with different organisation (for
example, the IRQ_TYPE can express dual edge), etc... 

In my new powerpc IRQ rework, I'm wiping out our own definitions for
triggers and using IRQ_TYPE_* accross the range. I'm howveer not using
the SA_* stuff at all at this point (that is, I'm neither converting the
trigger type of a given interrupt to SA_* and putting it in the
action/desc, nor honoring the SA_* triggers passed in request_irq at
this point). It might be nice to do so, but I find that there is just
way too much confusion at the moment.

Ben.

Re: SA_TRIGGER_* vs. SA_SAMPLE_RANDOM

[Benjamin Herrenschmidt]

On Sat, 2006-07-01 at 00:25 +0200, Thomas Gleixner wrote:
> On Fri, 2006-06-30 at 13:31 -0700, David Miller wrote:
> > From: Thomas Gleixner <tglx@linutronix.de>
> > Date: Fri, 30 Jun 2006 22:27:27 +0200
> > 
> > > I'll cook it up tomorrow.
> > 
> > Thanks a lot Thomas. :)
> 
> That's what I came up with:
> 
> SA_INTERRUPT		IRQF_IRQS_DISABLED
> SA_SAMPLE_RANDOM	IRQF_SAMPLE_RANDOM
> SA_SHIRQ		IRQF_SHARE_IRQ
> SA_PROBEIRQ		IRQF_PROBE_IRQ
> SA_TRIGGER_LOW		IRQF_TRIGGER_LOW
> SA_TRIGGER_HIGH		IRQF_TRIGGER_HIGH
> SA_TRIGGER_FALLING	IRQF_TRIGGER_FALLING
> SA_TRIGGER_RISING	IRQF_TRIGGER_RISING
> SA_TRIGGER_MASK		IRQF_TRIGGER_MASK
> SA_TIMER		IRQF_TIMER

Looks good to me. Do we want to keep a PERCPU flag too ? I don't really
need it anymore on powerpc as I just use the percpu flow handler and I'm
not allowing sharing of IPIs but others might.

Also, I'd like to store the IRQ types in the irq_desc regardless of the
actions that have been registered or not. Any suggestion where to put
that ? The current type values conflict with other desc->status bits at
the moment unless we shift the whole thing up...

Ben.