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

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

On Tue, Aug 07, 2012 at 02:47:50PM +0100, Russell King wrote:

> If you want to be constructive, then actually take a bloody look at my
> suggestion, try it out and report back whether it works.  Stop attacking
> my proposal in whatever weak ways you can, especially in ways that I've
> already dismissed as being totally irrelevant.

I have looked at your suggestion, and I have repeatedly agreed that your
suggestion is the best suggestion going forwards.  I don't think there
is any way in which I could be clearer on that.

> > 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.

> Sigh.  Here we go a fucking again.  I've already covered this.  But
> in case you haven't read, here it is again.

> And that combination is illegal today.  But the amount of code which
> the value will be exposed to is limited to _just_ the platform code,
> which, for the parts affeced, I'm the author of.  And I've read it
> before creating the patch to make sure it will work as I expect it
> do.

Well, I guess we have to agree to disagree on that.  I'm just very much
more conservative than you about what I'd be willing to put into a
stable release - my instincts on this are very strongly towards avoiding
any possibility of introducing unintended side effects and one way of
doing that is minimising the scope of any change.  Even where things
should be correct and look correct minimising the risk of exposing any
latent bugs (including those in systems derived from the stable release)
is a major consideration for me with any sort of stable fix.  Clearly
any code that's affected is buggy but having people be able to trust
stable releases is a very big factor.

This is all that we're disagreeing on, everyone including me agrees that
your change is clearly the best change going forwards but I'm much more
paranoid about stable releases than you are.

> > 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.

> Twice I asked.  The first time was with the IORESOURCE_FOO mail, which
> you had time to read, and reply to - and your reply was yet another
> "why we can't do it this way" whinge.  Yes, you had time to read it,
> you had time to answer it with reasons why not, but not to give any
> useful information back.

Hrm, if you're referring to <20120807111331.GC24257@flint.arm.linux.org.uk>
I don't actually seem to be able to see the question in your mail.

> Instead, you'll notice that many of my replies have been outlining
> solutions to the problem, and *actually* contain patches to address
> the issue.  Yours?  All whinges about why not.

The main reason I haven't send any patches was that by the time we were
satisfied that this was OK people were already sending code changes so
it seemed like that was in hand (and indeed you have sent a patch for
this).

> > 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.

> "someone having the time to add the new resource type" ?  Oh for fuck sake
> Mark, what the hell are you talking about?  How long does it take to apply
> a bloody patch?

It's the bit with confirming that we're OK not treating the resource
types as a bitmask that's time consuming; until this thread nobody had
gone through and checked that this would be OK.