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

From: Mark Brown
Date: Tue Aug 07 2012 - 08:58:17 EST


On Tue, Aug 07, 2012 at 12:51:40PM +0100, Russell King wrote:

> For fuck sake Mark. You are insane.

Please take a step back from the ad hominem remarks.

> How can:

> #define IORESOURCE_FOO 0x00000300

> in ioport.h be called "invasive" ? The best chance of error is that the
> identifier is already in use. So learn to use grep to check the whole
> sodding tree first to make sure that the identifier you're choosing to
> use isn't already in use somewhere.

My concern there (and that of others who've looked at adding a new
resource type) is that this value can also be written as

#define IORESOURCE_FOO (IORESOURCE_IO | IORESOURCE_MEM)

and the selection of values chosen for the resource types clearly looks
like it's supposed to be interpreted as a bitmask for some reason. This
is the main reason nobody touched the code already, it sets off alarm
bells from a code review point of view.

It may well be the case that the constants are only ever viewed as plain
old numbers in which case we're fine but it really doesn't seem too
clevr for stable.

> And in any case, I suspect you've lost the plot, because I suspect the
> driver you are referring to is wm831x, which has already had your solution
> patched into it by you back in May.

The urgent issue is for the Marvell PMIC drivers (drivers/mfd/88pm*)
which are also affected but have not had a fix implemented so have been
broken since v3.4. The wm831x drivers should be updated to use the new
API too but don't now have an issue in stable right now. There may be
others but presumably they're not even testing stable releases so again
probably not urgent.

> And you still haven't done me the curtesy of answering my repeated
> questions about WHAT BLOODY DRIVER you are referring to has the problem.

You've only asked this once that I've seen, in the mail where you posted
your patch (which is a helpful step forward, thanks!) which very recent.
It's possible that you've asked this elsewhere, though I did just scan
through a good chunk of the mails and didn't see the question.

> There is no point in discussing this any further unless you START answering
> some of the basic questions, rather than constantly trying to poke holes
> in a solution you did not invent. (Do you suffer from not-invented-here
> syndrome? Because you *are* showing all the signs of that.)

As I keep saying I don't think there's been any disagreement that adding
one or more new resource types is the best approach going forward, the
only issues have been around what happens in stable and someone having
the time to add the new resource type.
--
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/