Re: [PATCH] mfd: ucb1x00: remove NO_IRQ check

[Russell King - ARM Linux]

On Wed, Sep 07, 2016 at 01:48:18PM +0100, Lee Jones wrote:
> On Wed, 07 Sep 2016, Russell King - ARM Linux wrote:
> 
> > On Wed, Sep 07, 2016 at 11:27:37AM +0100, Lee Jones wrote:
> > > The *only* logical reason you're making such a fuss now is because of
> > > the discussion we had last week.  This behaviour is unacceptable.
> > 
> > No.  I'm making a fuss because the series you picked it from was not
> > ready for you (or anyone) to cherry-pick patches from.  Experienced
> > maintainers will _not_ cherry-pick patches from a series, at least
> 
> An experienced contributor who is used to working with subsystems
> which are maintained by someone other than themselves would have sent
> a set which is not ready for anyone to apply as an [RFC] and not a
> [PATCH] set.  [PATCH]es are good-to-go.

Right, and with the default being [PATCH], and not [RFC], it got sent
as [PATCH] BY MISTAKE which is something I already covered in the email
you are replying to.  I'm not going to repeat that yet again, just
because you think that repeating the same point several times in your
reply makes it any more valid than the first time.

> > without first asking - because there may be non-obvious dependencies
> > (which there were) which cherry-picking would break.
> 
> You only sent me one patch, and I am not a psychic.  If there were
> dependencies, you should have a) sent the whole set, so we could see
> what was going on, or b) mentioned that in the cover letter.  We could
> have subsequently started to sort something out -- in the form of an
> immutable branch.

You got the cover letter, and I quoted a bit from it which clearly
indicated that I was not intending the patch to be applied.  Only
someone with a political point to make, or someone being obtuse
would ignore that.

> > Patches are normally sent as a series because there _is_ indeed some
> > dependency issue between the patches.  If there is no dependency then
> > they would be sent as stand-alone patches.
> 
> Wrong.  I receive the majority (almost all in fact) of my patches via
> sets which cover multiple subsystems and most of them do not have
> *build time* or like this set *API* dependencies and are thus applied
> separately.

That is where you are totally and utterly wrong.  Sorry, but you are
definitely completely wrong on that.

If they are stand-alone, then they should not be sent as a series.

> > Even the sub-series introduction email said:
> > 
> >   This last part of the series is predominantly a set of cleanup patches
> >   removing definitions and files that aren't required, and moving some
> >   files out of public view.  The ucb1x00 patch could arguably have
> >   been sent in the first set, something that I'll arrange for the next
> >   iteration.
> > 
> > which clearly shows that I did _not_ expect the patch to be picked up -
> > if it was picked up, how could I include it in the next iteration of
> > the patches.
> 
> Then why send it?  That makes no sense whatsoever.

Oh for fuck sake, what the hell is up with you.

It got sent for REVIEW COMMENTS and TESTING for people like Robert
Jarzmik and Adam, to get some sense as to the _entire_ series
acceptability to people.  This is a _massive_ series, and it's still
growing.  The series is now at more than 100 patches.

> > And the last point is that you were on the Cc line - but I know that
> > you take no notice of that what so ever, so maybe I should have omitted
> > you completely - but then I'd expect that you'd be whining that you
> > didn't receive a personal copy of the patch.
> 
> Correct.

So you've just admitted that I can't win - if I don't send you a copy
of a patch you should be notified about, you whine.  If I send you a
copy, it's liable to get applied even when it shouldn't be.

Sorry, I can't work with you if that's how you're going to operate.

> > > > 2) If you review the driver and consider the effect of the change (which,
> > > >    as you don't know the driver, you should have done before replying if
> > > >    you're wanting to claim to be responsible for it) you would have
> > > 
> > > This is why I asked the question.  I doubt any subsystem Maintainer
> > > knows the intricacies of every driver they maintain.  We rely on
> > > original driver writers and other experts (e.g. assigned vendor
> > > engineers and the like) for guidance on the specifics.
> > 
> > Right, which is why you decided to take my patch because you thought
> > it was simple, rather than deferring to me - the author.  You are
> > being arrogant.
> 
> I took your patch because you send it to me, on its own, with no
> *clear* indication of its dependencies or that it shouldn't be taken
> as a normal patch.  When decent contributors do this, they normally
> send as an [RFC] or a forthcoming message of their intentions.  This
> set contained neither.

It was _NOT_ sent on it's own.  You're making this up, trying to
stupidly justify your idiotic position.  What's really funny is
that everyone on linux-arm-kernel can see that you're talking
total crap here.

So the fact that (a) you were copied on the cover letter to the
sub-series, (b) that the patch had a numeric patch number in the
subject line with a numeric total of patches didn't make you
_think_ for one bit that it was part of a series?

How could such a change be "on its own"?

> > Yea, soo quick that you failed to read even the covering message on the
> > sub-series.  Maybe you need to slow down a bit?
> 
> I certainly don't have time to decrypt hidden meanings in
> cover-letters, no.  If you have a request or a message, make it
> clear:

Look, (and I've had enough of your crap) the cover letter was _very_
clear that this particular patch should have been included _EARLIER_
in the overall series, and that I was _GOING_ to resend it.

That's not cryptic.  You're making this up as you're going along.

> > The fact of the matter is that the ucb1x00 drivers (by your own statement
> > above) receive very few patches, so the argument of avoiding conflicts
> > doesn't really apply.
> 
> With *this* driver maybe.  But providing exceptions to the rule is a
> slippery slope.  It's simpler if we work as a team.  Give the driver
> experts a chance to comment, in cases of in-depth issues.  Then funnel
> the changes through one repo.
> 
> Why do you insist of being different to everyone else?

Because you've proven to be incompetent in this instance.

> > I don't expect you to read all those, but I do expect you to read at
> > least the cover message to which the patch you took was attached to.
> 
> I did.  It made one cryptic reference to a subsequent set, and that
> was it.  I'll not reiterate, please see my comments above.

You clearly have not read it.  Subsequent?  No, none of the cover
messages made any reference to a follow-on set of patches.

Let me re-quote it again, for the hard of understanding (aka thick)
with underlining for the relevant part:

   This last part of the series is predominantly a set of cleanup patches
   removing definitions and files that aren't required, and moving some
   files out of public view.  The ucb1x00 patch could arguably have
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   been sent in the first set, something that I'll arrange for the next
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   iteration.
   ^^^^^^^^^^

The reason that was added is because by the time I'd got to posting
the series, I realised that I should have moved it earlier in the
overall series, near to the GPIO patch which it depended on.  However,
as that had been sent the previous day, I wasn't about to reshuffle
the patches and start the process from the very beginning again.

Now, to help your English comprehension:

- "The ucb1x00 patch could arguably have been sent in the first set"
   -- oops I made a mistake, I'm acknowledging that I made a mistake
      here

- "something that I'll arrange for the next iteration."
   -- an indication that I will fix the mistake when I resend the
      _entire_ series.

There's nothing cryptic there.  You're just being difficult, trying
to protect your position with absurd arguments.

> > That was to stop you blindly taking it based on our conversation last
> > week - and at that point I hadn't realised you'd inappropriately
> > cherry-picked my previous patch.  I haven't really been following
> > email because I've been working on this series.
> 
> Who's fault is that?  I think you need to start taking resposibilty
> for your own lack for forthcomingness and lack of clarity with regards
> to your intentions.

Stop trying to protect your position with absurd arguments.

> > Now, consider that difference between two maintainer behaviours -
> > yours and LinusW's.  One is co-operative, working with the patch
> > submitter, eyes open to issues that may be caused by taking patches.
> > The other is arrogant, with an attitude of the maintainer knows
> > better than the submitter.  I can work with LinusW, but I seemingly
> > can't work with you.
> 
> That's because you didn't cock-up when you sent your set to Linus.
> You had "[RFC]" in the subject line and you were *very* forthcoming
> with the fact that this is a set of sets and with your intentions.

You are right and you are wrong.  The _cover_ letter only had RFC,
not the patches - which was an oversight and a mistake on my part.

However, rather than making stuff up, you ought to actually read
some emails.  Linus didn't just offer an ack, Linus asked _how_
the series was going to be dealt with - something that you failed
to do.  So that actually makes you wrong.

In fact, Linus realised (as you will see if you read his reply) that
the patches were probably going to have to go through a tree other
than his own.

> > I hope, in future, that you'll discuss with your submitters if you
> > want to cherry-pick patches out of a series before doing so to
> > avoid creating problems like this, and avoiding giving other people
> > more work.
> 
> This is the first issue I've had like this, and I totally put blame in
> the way you submitted the set.  Read up, inwardly digest and hopefully
> it will help you to provide better submissions in the future!

No, you need to change your behaviour.

I acknowledge that I made mistakes while posting the patch set (I
even acknowledged there were mistakes _while_ posting the set) but
I still maintain that this entire situation is of your making
because you think that you have a right to cherry-pick from patch
series as you please without first talking to the submitter or
reading the cover message.

Everyone I have worked with has asked how the series is going to be
delt with before merging any of it.  If people have wanted to take
a patch from a series, they have always asked first.

Anyway, I'm starting to repeat myself, so it's time to draw this
exchange to a close.  No, I'm not changing my position, so don't
try to ask me to "digest" what you've said - as far as I'm concerned
you are in the wrong on this subject, and you're not going to change
my view on that, sorry.

What I want from you is an acknowledgement that you will avoid
cherry-picking from any of my patch series again without first
asking the question - like everyone else does.

Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.