Re: [RFC/PATCH 4/7] Powerpc MSI implementation

[Matthew Wilcox <matthew@wil.cx>]

On Tue, Nov 07, 2006 at 08:48:53PM +0000, Russell King wrote:
> On Tue, Nov 07, 2006 at 01:44:32PM -0700, Matthew Wilcox wrote:
> > On Tue, Nov 07, 2006 at 08:14:36PM +0000, Russell King wrote:
> > > Bah.  chip_data is supposed to be __iomem.  I bet if you build ARM
> > > with sparse it'll kick out lots of warnings as a result of that loss.
> > 
> > Erm, since when?  When I introduced it (back in January 2005 [1]), it
> > was called handler_data and pointed to a struct which is chip-type
> > dependent.
> 
> Since before the generic irq merge.  If I was more expert with git
> I'd post a URL, but I'm not so I won't.  But I'm sure you can find
> it - look at the history of include/asm-arm/mach/irq.h.

OK.  Looks like the first mention of this is in 
4a2581a080098ca3a0c4e416d7a282e96c75ebf8 from July 2006
which is signed-off by you, Ingo Molnar and Thomas Gleixner:

-       void __iomem    *base;
[...]
+#define set_irq_chipdata(irq, d)       set_irq_chip_data(irq, d)
+#define get_irq_chipdata(irq)          get_irq_chip_data(irq)
-#define set_irq_chipdata(irq,d)                        do { irq_desc[irq].base = d; } while (0)
-#define get_irq_chipdata(irq)                  (irq_desc[irq].base)

Now, true, the __iomem has disappeared.  But there was never an __iomem
on chip_data, nor on the handler_data before it.  It went away back in
July when you signed off on the conversion to use the generic irq
handler, and apparently haven't noticed the problem since then, so I
don't see it as being a big problem.

As other architectures, you could embed the iomem pointer in a
struct (would it be useful to you to have more than an iomem pointer
in your handlers?), or you could put the cast adding __iomem in
get_irq_chipdata().