public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeff Garzik <jeff@garzik.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	Rene Herman <rene.herman@keyaccess.nl>,
	Adrian Bunk <bunk@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	rmk@arm.linux.org.uk, Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>
Subject: Re: [git patch] free_irq() fixes
Date: Thu, 24 Apr 2008 12:16:11 -0400	[thread overview]
Message-ID: <4810B24B.20309@garzik.org> (raw)
In-Reply-To: <alpine.LFD.1.10.0804240847230.2779@woody.linux-foundation.org>

Linus Torvalds wrote:
> 
> On Thu, 24 Apr 2008, Jeff Garzik wrote:
>> However, it does not follow that an int is what _must_ be passed around.  We
>> already have design patterns like
>>
>> 	cookie_pointer = ioremap(raw bus resource)
>>
>> Not that I am the one pushing for that, just noting.
> 
> I do agree that we could use something more type-safe.
> 
> So a "pointer" to a structure that doesn't actually exist would be fine 
> and would give us some C type checking. 
> 
> But then you'd have to have some way to "printk" the information, which is 
> a very common requirement (and the printk still needs to be a number, 
> because you want to match up 'dmesg' output with the '/proc/interrupts' 
> file etc).
> 
> That, in turn, would effectively force a whole new function, and then 
> you'd have things like
> 
> 	printk(.. irq %d .., irq_number(irqcookie) ..)
> 
> which while not ugly isn't really all that clean either. And I guarantee 
> that people would misuse that "irq_number(cookie)" exactly in the same 
> ways they'd misuse "irq" (ie not very much).
> 
> Quite frankly, I'd much prefer a
> 
> 	typedef int __bitwise irq_t;
> 
> and then we can use sparse to do this testing, without breaking any 
> existing use at all (because it will still be an "int" to gcc, but sparse 
> will make "irq_t" a type of its own and make sure that people pass it 
> around as such and not do arithmetic ops on it etc).

sparse checking sure would be nice for drivers......


As an aside it would be nice to associate a struct device with an irq at 
request_irq() time.  I wonder if we could perform this transition in 
situ, by creating a dev_request_irq() that used the new irq_t, while 
leaving all existing drivers to be converted as time permits.


>>> EVERYTHING else would be architecture-specific. And that is exactly what we
>>> do not want. EVER. 
>> Not true -- you have metadata/OOB data like MSI messages, where you are passed
>> a value from the PCI hardware in the PCI message, not just an "interrupt
>> asserted" condition.  Or s/value/values/ if you enable PCI MSI's multiple
>> message support.
> 
> The point is, MSI *is* architecture-specific. In fact, it's even 
> motherboard-specific, in that you are going to have (for the forseeable 
> future) drivers that have to work with or witgout MSI even on the same 
> architecture.

Sure, so what would a driver API look like that works for both situations?

We have two models:

     For each interrupt event delivered:

	old model: "irq asserted" datum is passed to driver implicitly,
	via the fact that your irq handler is called.

	new model: we have a dword, or two or three, passed to driver
	explicitly, in addition to implicit "irq asserted" fact

It's easier to handle both models if you could pass something other than 
an int to the driver --for each interrupt event--.

Having something pointer-sized rather than int-sized would be flexible 
enough to cover all situations, now and in the future, presumably.

	Jeff



  parent reply	other threads:[~2008-04-24 16:16 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-22 22:17 [git patch] free_irq() fixes Jeff Garzik
2008-04-22 22:25 ` Linus Torvalds
2008-04-22 22:59   ` Jeff Garzik
2008-04-22 23:20     ` Linus Torvalds
2008-04-22 23:49       ` Jeff Garzik
2008-04-22 23:52         ` Linus Torvalds
2008-04-23  0:05           ` Adrian Bunk
2008-04-23  0:16             ` Linus Torvalds
2008-04-23 13:51               ` Rene Herman
2008-04-24  2:10                 ` Jeff Garzik
2008-04-24  2:19                   ` Linus Torvalds
2008-04-24  5:59                     ` Eric W. Biederman
2008-04-24 10:53                       ` Jeff Garzik
2008-04-24 15:16                         ` Linus Torvalds
2008-04-24 15:40                           ` Jeff Garzik
2008-04-24 15:55                             ` Linus Torvalds
2008-04-24 15:37                               ` Alan Cox
2008-04-24 16:20                                 ` Jeff Garzik
2008-04-24 16:16                               ` Jeff Garzik [this message]
2008-04-24 16:48                               ` Eric W. Biederman
2008-04-24 16:58                                 ` Linus Torvalds
2008-04-24 18:15                                   ` Eric W. Biederman
2008-04-24 17:30                                 ` Jeff Garzik
2008-04-25  2:53                                   ` Eric W. Biederman
2008-04-25  3:33                                     ` MSI, fun for the whole family (was Re: [git patch] free_irq() fixes) Jeff Garzik
2008-04-25  3:57                                       ` MSI, fun for the whole family Roland Dreier
2008-04-25  4:19                                         ` David Miller
2008-04-25  4:35                                         ` Jeff Garzik
2008-04-25  5:48                                           ` Eric W. Biederman
2008-04-25 22:44                                           ` Roland Dreier
2008-04-25  5:08                                       ` Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4810B24B.20309@garzik.org \
    --to=jeff@garzik.org \
    --cc=akpm@linux-foundation.org \
    --cc=bunk@kernel.org \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=rene.herman@keyaccess.nl \
    --cc=rmk@arm.linux.org.uk \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox