Re: [PATCH 0/5] mfd: replace IORESOURCE_IO by IORESOURCE_MEM

[Arnd Bergmann]

On Tuesday 07 August 2012, Mark Brown wrote:
> On Tue, Aug 07, 2012 at 01:11:57PM +0100, Russell King wrote:
> 
> > The only open questions on this are:
> > 1. Is there another driver you are concerned about.
> 
> As I said elsewhere 88pm* needs this as a stable bugfix and wm831x
> should be converted over too.

I've looked through the remaining MFD drivers and found one more,
max8925-core, which is using IORESOURCE_IO for something that is
not ISA/PCI IO-space.

> > 2. Choosing a better name.
> 
> I'm not sure we need one, _REG seems perfectly fine here unless we want
> to go with Arnd's suggestion of _OTHER.  Do you have any concerns with
> the use of reg?

BenH actually suggested _OTHER. I think either one is fine.

> > But I won't put question marks at the end of those because you never seem
> > to answer questions.  You will just produce yet more justifications why
> > this approach is not one you're willing to accept.  So I really don't know
> > why I wasted my time to produce this patch.
> 
> As I mentioned in my mail a few moments ago I had not replied to your
> mails asking these question about which drivers are affected because
> those mails have been arriving so quickly since the first one I saw with
> the question in that there has not been a chance to do so.
> 
> > index 589e0e7..bfee885 100644
> > --- a/include/linux/ioport.h
> > +++ b/include/linux/ioport.h
> > @@ -31,6 +31,7 @@ struct resource {
> >  #define IORESOURCE_TYPE_BITS 0x00001f00      /* Resource type */
> >  #define IORESOURCE_IO                0x00000100
> >  #define IORESOURCE_MEM               0x00000200
> > +#define IORESOURCE_REG               0x00000300      /* Register offsets */
> >  #define IORESOURCE_IRQ               0x00000400
> >  #define IORESOURCE_DMA               0x00000800
> >  #define IORESOURCE_BUS               0x00001000
> 
> As I've said before I'm fine with the driver changes.  I do feel that it
> would be better to also renumber all the existing resource types while
> we're at it in order to make it clear that these are just plain numbers,
> that's the reason nobody wrote this patch previously.  This will avoid
> any future confusion.

This gets into a lot more tricky territory: We have a bunch of drivers
doing their own bitmask operations on these, like drivers/video/offb.c
testing

        if ((flags & (IORESOURCE_IO | IORESOURCE_MEM)) == 0)
                return NULL;

or drivers/scsi/gdth.c doing

            if (!(base0 & IORESOURCE_MEM) ||
                !(base2 & IORESOURCE_MEM) ||
                !(base1 & IORESOURCE_IO)) 
                return -ENODEV;

Now I've looked at the three drivers with the immediate problem of
IORESOURCE_IO abuse (max8925, wm831x, 88pm860x) and none of them are
doing such bitmask operations, so I'm reasonably sure we are fine
for those drivers. I also agree that renumbering the resources in a
way that makes it impossible to use bitmasks is a good idea, but
that would actually be pretty invasive because then we have to rewrite
all the functions that currently do it.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/