linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mfd: ucb1x00: remove NO_IRQ check
@ 2016-09-06 13:03 Arnd Bergmann
  2016-09-06 13:17 ` Russell King - ARM Linux
  0 siblings, 1 reply; 16+ messages in thread
From: Arnd Bergmann @ 2016-09-06 13:03 UTC (permalink / raw)
  To: Lee Jones
  Cc: Arnd Bergmann, Linus Walleij, Hans-Christian Egtvedt,
	Dmitry Torokhov, Thomas Gleixner, Russell King, linux-kernel

probe_irq_off() returns '0' on failure, not NO_IRQ, so the check
in this driver is clearly wrong. This replaces it with the
regular '!irq' check used in other drivers.

The sa1100 platform that this driver is used on originally numbered
all its interrupts starting at '0', which would have conflicted with
this change, but as of commit 18f3aec ("ARM: 8230/1: sa1100: shift
IRQs by one"), this is not a problem any more.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/mfd/ucb1x00-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/ucb1x00-core.c b/drivers/mfd/ucb1x00-core.c
index 48bea5038654..d6fb2e1a759a 100644
--- a/drivers/mfd/ucb1x00-core.c
+++ b/drivers/mfd/ucb1x00-core.c
@@ -537,7 +537,7 @@ static int ucb1x00_probe(struct mcp *mcp)
 	ucb1x00_enable(ucb);
 	ucb->irq = ucb1x00_detect_irq(ucb);
 	ucb1x00_disable(ucb);
-	if (ucb->irq == NO_IRQ) {
+	if (!ucb->irq) {
 		dev_err(&ucb->dev, "IRQ probe failed\n");
 		ret = -ENODEV;
 		goto err_no_irq;
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] mfd: ucb1x00: remove NO_IRQ check
  2016-09-06 13:03 [PATCH] mfd: ucb1x00: remove NO_IRQ check Arnd Bergmann
@ 2016-09-06 13:17 ` Russell King - ARM Linux
  2016-09-06 13:49   ` Arnd Bergmann
  2016-09-06 15:45   ` Lee Jones
  0 siblings, 2 replies; 16+ messages in thread
From: Russell King - ARM Linux @ 2016-09-06 13:17 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Lee Jones, Linus Walleij, Hans-Christian Egtvedt,
	Dmitry Torokhov, Thomas Gleixner, linux-kernel

You need to send this _to_ me as I need to merge it with my other
changes.  This patch on its own does not make sense - it only makes
sense with the rest of my SA11x0 patch stack.

NAK for Lee to merge this.

On Tue, Sep 06, 2016 at 03:03:57PM +0200, Arnd Bergmann wrote:
> probe_irq_off() returns '0' on failure, not NO_IRQ, so the check
> in this driver is clearly wrong. This replaces it with the
> regular '!irq' check used in other drivers.
> 
> The sa1100 platform that this driver is used on originally numbered
> all its interrupts starting at '0', which would have conflicted with
> this change, but as of commit 18f3aec ("ARM: 8230/1: sa1100: shift
> IRQs by one"), this is not a problem any more.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/mfd/ucb1x00-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/ucb1x00-core.c b/drivers/mfd/ucb1x00-core.c
> index 48bea5038654..d6fb2e1a759a 100644
> --- a/drivers/mfd/ucb1x00-core.c
> +++ b/drivers/mfd/ucb1x00-core.c
> @@ -537,7 +537,7 @@ static int ucb1x00_probe(struct mcp *mcp)
>  	ucb1x00_enable(ucb);
>  	ucb->irq = ucb1x00_detect_irq(ucb);
>  	ucb1x00_disable(ucb);
> -	if (ucb->irq == NO_IRQ) {
> +	if (!ucb->irq) {
>  		dev_err(&ucb->dev, "IRQ probe failed\n");
>  		ret = -ENODEV;
>  		goto err_no_irq;
> -- 
> 2.9.0
> 

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mfd: ucb1x00: remove NO_IRQ check
  2016-09-06 13:17 ` Russell King - ARM Linux
@ 2016-09-06 13:49   ` Arnd Bergmann
  2016-09-06 15:45   ` Lee Jones
  1 sibling, 0 replies; 16+ messages in thread
From: Arnd Bergmann @ 2016-09-06 13:49 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Lee Jones, Linus Walleij, Hans-Christian Egtvedt,
	Dmitry Torokhov, Thomas Gleixner, linux-kernel

On Tuesday, September 6, 2016 2:17:30 PM CEST Russell King - ARM Linux wrote:
> You need to send this _to_ me as I need to merge it with my other
> changes.  This patch on its own does not make sense - it only makes
> sense with the rest of my SA11x0 patch stack.
> 
> NAK for Lee to merge this.
> 

Ok, I wasn't aware you were working on a series for this already.
I'll send you all the related NO_IRQ cleanup as a series then.

I've sent off a couple of driver fixes for other platforms (mostly
dmaengine) regarding NO_IRQ last week, the main parts remaining now
are mvebu and sa1100/pxa, which should both be one series each I guess.
	
	Arnd

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mfd: ucb1x00: remove NO_IRQ check
  2016-09-06 13:17 ` Russell King - ARM Linux
  2016-09-06 13:49   ` Arnd Bergmann
@ 2016-09-06 15:45   ` Lee Jones
  2016-09-06 16:28     ` Russell King - ARM Linux
  1 sibling, 1 reply; 16+ messages in thread
From: Lee Jones @ 2016-09-06 15:45 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Arnd Bergmann, Linus Walleij, Hans-Christian Egtvedt,
	Dmitry Torokhov, Thomas Gleixner, linux-kernel

On Tue, 06 Sep 2016, Russell King - ARM Linux wrote:

> You need to send this _to_ me as I need to merge it with my other
> changes.  This patch on its own does not make sense - it only makes
> sense with the rest of my SA11x0 patch stack.
> 
> NAK for Lee to merge this.

So if I were to accept this patch, would anything break?  In other
words, is there an ordering issue where this this change relies on
something you have in your tree?

> On Tue, Sep 06, 2016 at 03:03:57PM +0200, Arnd Bergmann wrote:
> > probe_irq_off() returns '0' on failure, not NO_IRQ, so the check
> > in this driver is clearly wrong. This replaces it with the
> > regular '!irq' check used in other drivers.
> > 
> > The sa1100 platform that this driver is used on originally numbered
> > all its interrupts starting at '0', which would have conflicted with
> > this change, but as of commit 18f3aec ("ARM: 8230/1: sa1100: shift
> > IRQs by one"), this is not a problem any more.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  drivers/mfd/ucb1x00-core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mfd/ucb1x00-core.c b/drivers/mfd/ucb1x00-core.c
> > index 48bea5038654..d6fb2e1a759a 100644
> > --- a/drivers/mfd/ucb1x00-core.c
> > +++ b/drivers/mfd/ucb1x00-core.c
> > @@ -537,7 +537,7 @@ static int ucb1x00_probe(struct mcp *mcp)
> >  	ucb1x00_enable(ucb);
> >  	ucb->irq = ucb1x00_detect_irq(ucb);
> >  	ucb1x00_disable(ucb);
> > -	if (ucb->irq == NO_IRQ) {
> > +	if (!ucb->irq) {
> >  		dev_err(&ucb->dev, "IRQ probe failed\n");
> >  		ret = -ENODEV;
> >  		goto err_no_irq;
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mfd: ucb1x00: remove NO_IRQ check
  2016-09-06 15:45   ` Lee Jones
@ 2016-09-06 16:28     ` Russell King - ARM Linux
  2016-09-07 10:27       ` Lee Jones
  0 siblings, 1 reply; 16+ messages in thread
From: Russell King - ARM Linux @ 2016-09-06 16:28 UTC (permalink / raw)
  To: Lee Jones
  Cc: Arnd Bergmann, Linus Walleij, Hans-Christian Egtvedt,
	Dmitry Torokhov, Thomas Gleixner, linux-kernel

On Tue, Sep 06, 2016 at 04:45:30PM +0100, Lee Jones wrote:
> On Tue, 06 Sep 2016, Russell King - ARM Linux wrote:
> 
> > You need to send this _to_ me as I need to merge it with my other
> > changes.  This patch on its own does not make sense - it only makes
> > sense with the rest of my SA11x0 patch stack.
> > 
> > NAK for Lee to merge this.
> 
> So if I were to accept this patch, would anything break?  In other
> words, is there an ordering issue where this this change relies on
> something you have in your tree?

1) This is my driver which I've maintained in the past myself, including
   handling all patches for.  This pre-dates your decision to take over
   the mfd stuff.  I'm still maintaining this driver and I have not
   passed the responsibility to you.

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
   noticed that the patch on its own subsitutes one form of brokenness
   with a different form of brokenness - ucb1x00_detect_irq() can return
   NO_IRQ.  So checking for !ucb->irq breaks when ucb1x00_detect_irq()
   does so.

3) I already have a patch which removes that NO_IRQ usage, so the only
   way that _this_ patch makes sense is when combined with my patch.

4) I find it annoying that you've picked up on this patch in the way
   that you have (as a result of my NAK), yet you have failed to make
   any comment what so ever on _my_ patch, which you were copied with
   on the 30th August.

Moreover, I reserve the right to keep it in my tree as the driver author,
possibly publishing it in linux-next.  I reserve the right to review
patches to my driver.  I reserve the right to NAK patches to my driver
for whatever reason I deem appropriate.

This is part of the problem I have here with your attitude with mfd:
you decided yourself to become maintainer of everything in drivers/mfd,
riding rough-shod over your fellow maintainers.  And when your fellow
maintainers try to reassert control, you get upset about it.  Sorry,
you can't have it both ways.

Now, if you had discussed my patch from the 30th _first_ then I would
not be NAKing this patch, and I probably would not be making such a
big deal about this.  But let's put that aside and stick to the
technicalities.

There are two dependencies here.  Right now, the probe_irq_on()
returning zero on SA11x0 platforms, causing ucb1x00_detect_irq() to
return NO_IRQ, which then causes the probe function to fail.  Whether
that happens on PXA or not, I don't know and I have no way to test
anymore as I donated all my PXA platforms to Robert.

Applying Arnd's patch on its own, or applying my patch on its own will
cause ucb1x00 to initialise with either NO_IRQ or 0 in ucb->irq, which
causes the device to have no interrupts - or worse, it screws up any
configuration of IRQ0 that the platform may have (eg, PXA).  Applying
both together results in the probe function continuing to fail.

My patch is not supposed to be applied on its own; it is supposed to
be applied with the third patch already in place - a GPIO patch.  So
yes, there _were_ dependencies here.

That dependency can be solved by taking _both_ my patch (first) and
Arnd's patch.  However, given that each patch introduces its own
different form of breakage, it makes sense to combine the two patches
to avoid that breakage.  So, when I'm less busy sorting out other
SA11x0 stuff, I want to discuss with Arnd about combining them into
a single patch.

_Then_ we need to have a discussion about how to handle the patch.

So, it's not as simple as you can see, because you don't have the
knowledge to make the decisions for this driver.

I hope this is clear enough.

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mfd: ucb1x00: remove NO_IRQ check
  2016-09-06 16:28     ` Russell King - ARM Linux
@ 2016-09-07 10:27       ` Lee Jones
  2016-09-07 11:27         ` Russell King - ARM Linux
  0 siblings, 1 reply; 16+ messages in thread
From: Lee Jones @ 2016-09-07 10:27 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Arnd Bergmann, Linus Walleij, Hans-Christian Egtvedt,
	Dmitry Torokhov, Thomas Gleixner, linux-kernel

On Tue, 06 Sep 2016, Russell King - ARM Linux wrote:

> On Tue, Sep 06, 2016 at 04:45:30PM +0100, Lee Jones wrote:
> > On Tue, 06 Sep 2016, Russell King - ARM Linux wrote:
> > 
> > > You need to send this _to_ me as I need to merge it with my other
> > > changes.  This patch on its own does not make sense - it only makes
> > > sense with the rest of my SA11x0 patch stack.
> > > 
> > > NAK for Lee to merge this.
> > 
> > So if I were to accept this patch, would anything break?  In other
> > words, is there an ordering issue where this this change relies on
> > something you have in your tree?
> > 
> 1) This is my driver which I've maintained in the past myself, including
>    handling all patches for.  This pre-dates your decision to take over
>    the mfd stuff.  I'm still maintaining this driver and I have not
>    passed the responsibility to you.

There is no mention of you maintaining this driver in MAINTAINERS.
This is the first I've heard of it.  You haven't taken patches for
this driver since January 2012 (4 years, 8 months).  Over that period
I have accepted 9 patches and conducted more reviews and you haven't
taken part or shown any interest whatsoever.

The *only* logical reason you're making such a fuss now is because of
the discussion we had last week.  This behaviour is unacceptable.

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

>    noticed that the patch on its own subsitutes one form of brokenness
>    with a different form of brokenness - ucb1x00_detect_irq() can return
>    NO_IRQ.  So checking for !ucb->irq breaks when ucb1x00_detect_irq()
>    does so.

Great review comment.  Thanks.

> 3) I already have a patch which removes that NO_IRQ usage, so the only
>    way that _this_ patch makes sense is when combined with my patch.

No problem.  I can do that for you (see below).

> 4) I find it annoying that you've picked up on this patch in the way
>    that you have (as a result of my NAK), yet you have failed to make
>    any comment what so ever on _my_ patch, which you were copied with
>    on the 30th August.

I'd seriously consider checking your mail filters if I were you (the
reply was even addressed To: you, just how you like it.  It took me
exactly an hour from you sending the patch to me reviewing and
accepting it:

  http://www.spinics.net/lists/arm-kernel/msg527414.html

It's even in -next, if you'd cared enough to look:

  http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=159aed02b0074bac1c21544befb773dce39b9fcb
 
> Moreover, I reserve the right to keep it in my tree as the driver author,
> possibly publishing it in linux-next.  I reserve the right to review
> patches to my driver.  I reserve the right to NAK patches to my driver
> for whatever reason I deem appropriate.

I fully support all of those rights.  I find keeping it in -next a bit
odd.  Especially since I already put in there for you, but I guess, so
long as you don't try to submit it to Linus which might likely cause a
merge-conflict, it's okay.

> This is part of the problem I have here with your attitude with mfd:
> you decided yourself to become maintainer of everything in drivers/mfd,
> riding rough-shod over your fellow maintainers.  And when your fellow
> maintainers try to reassert control, you get upset about it.  Sorry,
> you can't have it both ways.

Not sure what you're trying to say here to be honest.  I offered my
help to Sam, the then MFD Maintainer [0], which was subsequently
accepted.  He then became busy with NFC, so I took the driver's seat.

  [0] http://www.spinics.net/lists/kernel/msg1525957.html

I try to Maintain MFD in a simple, effective manner, where my
priorities are letting good code easily pass through, keeping bad code
out and most relevant here, trying to prevent merge-conflicts so Linus
has an easy time during the merge-window.

It doesn't make sense for drivers which reside in the same subsystem
to be applied to multiple trees.  I like for all changes to MFD to be
reflected in the MFD pull-request.  If they *need* to go in via other
repos *as well*, that's fine too.  I am usually very happy to provide
pull-requests when those needs arise.

If you are insistent on applying this yourself, so long as you have a
*technical* (rather than territorial/spiteful) reason for doing so, I,
as always, will be happy to oblige you with a pull-request too.

> Now, if you had discussed my patch from the 30th _first_ then I would
> not be NAKing this patch, and I probably would not be making such a
> big deal about this.  But let's put that aside and stick to the
> technicalities.

I think this *rant* of yours might be based on very shakey foundations
then, because I *did* reply to your patch.  From what I saw, it was a
reasonable patch, so I took it.

Maybe if you would have known the driver as well as you profess to,
you would have fixed the issue properly and there would be no need for
this patch in the first place.  I tease of course!  ;)

> There are two dependencies here.  Right now, the probe_irq_on()
> returning zero on SA11x0 platforms, causing ucb1x00_detect_irq() to
> return NO_IRQ, which then causes the probe function to fail.  Whether
> that happens on PXA or not, I don't know and I have no way to test
> anymore as I donated all my PXA platforms to Robert.
> 
> Applying Arnd's patch on its own, or applying my patch on its own will
> cause ucb1x00 to initialise with either NO_IRQ or 0 in ucb->irq, which
> causes the device to have no interrupts - or worse, it screws up any
> configuration of IRQ0 that the platform may have (eg, PXA).  Applying
> both together results in the probe function continuing to fail.
> 
> My patch is not supposed to be applied on its own; it is supposed to
> be applied with the third patch already in place - a GPIO patch.  So
> yes, there _were_ dependencies here.
> 
> That dependency can be solved by taking _both_ my patch (first) and
> Arnd's patch.  However, given that each patch introduces its own
> different form of breakage, it makes sense to combine the two patches
> to avoid that breakage.  So, when I'm less busy sorting out other
> SA11x0 stuff, I want to discuss with Arnd about combining them into
> a single patch.
> 
> _Then_ we need to have a discussion about how to handle the patch.

Good explanation.  Just the sort of information I'd expect during a
patch review.  Much more helpful than "this patch doesn't make
sense.  Give it to me.  NACK".  Now I can work with you both to
resolve this issue properly and professionally.

So, since I already have half of the resolution, all I need to do is
squash this into it, right?  Or do we need to carry this GPIO patch
too?  If so, where is that currently?

The only issue I envisage is that is someone has to loose patch
authorship.

I propose to squash this patch into Russell's, keep both SoBs and
give Arnd written creds in the commit message.

> So, it's not as simple as you can see, because you don't have the
> knowledge to make the decisions for this driver.

So, long as I'm supplied with the facts I'm fine (continuing to)
making decisions, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mfd: ucb1x00: remove NO_IRQ check
  2016-09-07 10:27       ` Lee Jones
@ 2016-09-07 11:27         ` Russell King - ARM Linux
  2016-09-07 12:48           ` Lee Jones
  0 siblings, 1 reply; 16+ messages in thread
From: Russell King - ARM Linux @ 2016-09-07 11:27 UTC (permalink / raw)
  To: Lee Jones
  Cc: Arnd Bergmann, Linus Walleij, Hans-Christian Egtvedt,
	Dmitry Torokhov, Thomas Gleixner, linux-kernel

On Wed, Sep 07, 2016 at 11:27:37AM +0100, Lee Jones wrote:
> On Tue, 06 Sep 2016, Russell King - ARM Linux wrote:
> 
> > On Tue, Sep 06, 2016 at 04:45:30PM +0100, Lee Jones wrote:
> > > On Tue, 06 Sep 2016, Russell King - ARM Linux wrote:
> > > 
> > > > You need to send this _to_ me as I need to merge it with my other
> > > > changes.  This patch on its own does not make sense - it only makes
> > > > sense with the rest of my SA11x0 patch stack.
> > > > 
> > > > NAK for Lee to merge this.
> > > 
> > > So if I were to accept this patch, would anything break?  In other
> > > words, is there an ordering issue where this this change relies on
> > > something you have in your tree?
> > > 
> > 1) This is my driver which I've maintained in the past myself, including
> >    handling all patches for.  This pre-dates your decision to take over
> >    the mfd stuff.  I'm still maintaining this driver and I have not
> >    passed the responsibility to you.
> 
> There is no mention of you maintaining this driver in MAINTAINERS.
> This is the first I've heard of it.  You haven't taken patches for
> this driver since January 2012 (4 years, 8 months).  Over that period
> I have accepted 9 patches and conducted more reviews and you haven't
> taken part or shown any interest whatsoever.

Maybe I haven't taken any patches because I haven't seen any of these
patches?

Yes, you're right that I'm not listed in MAINTAINERS for it, that's
because it was merged a long time ago when I was maintaining almost
everything ARM, and listing everything in MAINTAINERS was not really
viable - there wasn't even the regexp patterns in MAINTAINERS to
describe what was there.

Thanks for mentioning it though, I'll add myself so that I get the
patches in future.

> 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
without first asking - because there may be non-obvious dependencies
(which there were) which cherry-picking would break.

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.

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.

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.

You messed up, plain and simple - I think purposely to make a political
point given our discussion last week.

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

> > 4) I find it annoying that you've picked up on this patch in the way
> >    that you have (as a result of my NAK), yet you have failed to make
> >    any comment what so ever on _my_ patch, which you were copied with
> >    on the 30th August.
> 
> I'd seriously consider checking your mail filters if I were you (the
> reply was even addressed To: you, just how you like it.  It took me
> exactly an hour from you sending the patch to me reviewing and
> accepting it:
> 
>   http://www.spinics.net/lists/arm-kernel/msg527414.html

Yea, soo quick that you failed to read even the covering message on the
sub-series.  Maybe you need to slow down a bit?

> It's even in -next, if you'd cared enough to look:
> 
>   http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=159aed02b0074bac1c21544befb773dce39b9fcb

I don't look at -next, sorry.  Anything that needs me to go off and do
extra work is pretty much a no-no at the moment (and is a no-no at the
best of times.)  I'm still pretty much working solidly on this series,
and it remains a work-in-progress - as the purpose of posting it is to
get feedback and testing - and I've been getting results from people
testing, which has been provoking further changes to the whole series.
That's pretty much a full-time job at the moment.

> > This is part of the problem I have here with your attitude with mfd:
> > you decided yourself to become maintainer of everything in drivers/mfd,
> > riding rough-shod over your fellow maintainers.  And when your fellow
> > maintainers try to reassert control, you get upset about it.  Sorry,
> > you can't have it both ways.
> 
> Not sure what you're trying to say here to be honest.  I offered my
> help to Sam, the then MFD Maintainer [0], which was subsequently
> accepted.  He then became busy with NFC, so I took the driver's seat.
> 
>   [0] http://www.spinics.net/lists/kernel/msg1525957.html
> 
> I try to Maintain MFD in a simple, effective manner, where my
> priorities are letting good code easily pass through, keeping bad code
> out and most relevant here, trying to prevent merge-conflicts so Linus
> has an easy time during the merge-window.
> 
> It doesn't make sense for drivers which reside in the same subsystem
> to be applied to multiple trees.  I like for all changes to MFD to be
> reflected in the MFD pull-request.  If they *need* to go in via other
> repos *as well*, that's fine too.  I am usually very happy to provide
> pull-requests when those needs arise.
> 
> If you are insistent on applying this yourself, so long as you have a
> *technical* (rather than territorial/spiteful) reason for doing so, I,
> as always, will be happy to oblige you with a pull-request too.

I think I've already pointed out the technical argument here, and why
your attitude towards total ownership of MFD is broken.

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.

> > Now, if you had discussed my patch from the 30th _first_ then I would
> > not be NAKing this patch, and I probably would not be making such a
> > big deal about this.  But let's put that aside and stick to the
> > technicalities.
> 
> I think this *rant* of yours might be based on very shakey foundations
> then, because I *did* reply to your patch.  From what I saw, it was a
> reasonable patch, so I took it.
> 
> Maybe if you would have known the driver as well as you profess to,
> you would have fixed the issue properly and there would be no need for
> this patch in the first place.  I tease of course!  ;)

As I said above, the whole series (of some 60-90 patches) is still very
much a work in progress.  It took two days to post the whole series
(29th and 30th August) so getting all the details into every message
wasn't practical.  However, there's enough of a clue there: the size
of the overall series, the comments in the sub-series cover message,
the fact that earlier cover message had RFC in, over sub-series cover
messages were asking for acks, etc.

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.

> > There are two dependencies here.  Right now, the probe_irq_on()
> > returning zero on SA11x0 platforms, causing ucb1x00_detect_irq() to
> > return NO_IRQ, which then causes the probe function to fail.  Whether
> > that happens on PXA or not, I don't know and I have no way to test
> > anymore as I donated all my PXA platforms to Robert.
> > 
> > Applying Arnd's patch on its own, or applying my patch on its own will
> > cause ucb1x00 to initialise with either NO_IRQ or 0 in ucb->irq, which
> > causes the device to have no interrupts - or worse, it screws up any
> > configuration of IRQ0 that the platform may have (eg, PXA).  Applying
> > both together results in the probe function continuing to fail.
> > 
> > My patch is not supposed to be applied on its own; it is supposed to
> > be applied with the third patch already in place - a GPIO patch.  So
> > yes, there _were_ dependencies here.
> > 
> > That dependency can be solved by taking _both_ my patch (first) and
> > Arnd's patch.  However, given that each patch introduces its own
> > different form of breakage, it makes sense to combine the two patches
> > to avoid that breakage.  So, when I'm less busy sorting out other
> > SA11x0 stuff, I want to discuss with Arnd about combining them into
> > a single patch.
> > 
> > _Then_ we need to have a discussion about how to handle the patch.
> 
> Good explanation.  Just the sort of information I'd expect during a
> patch review.  Much more helpful than "this patch doesn't make
> sense.  Give it to me.  NACK".  Now I can work with you both to
> resolve this issue properly and professionally.

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.

> So, since I already have half of the resolution, all I need to do is
> squash this into it, right?  Or do we need to carry this GPIO patch
> too?  If so, where is that currently?

So, let's apply your maintainership style to GPIO... and let's say
that LinusW just applied the patch within hours because it "looked
simple enough".  Then what?  The dependencies are split between two
trees, potentially with other patches behind them.

No, LinusW didn't do that.  Linus read the cover message, realised
that it was part of a larger series, and decided not to take the
patch, because there was a huge number of dependent patches on top
of it touching _many_ different subsystems.

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.

As a result of your actions, I'm now having to ask LinusW to take
the GPIO patch and get it into mainline ASAP, preferably for -rc6,
so at least the straight-line tips of LinusT's tree don't end up
with weirdness - and so I can rebase my entire series on top of
-rc6 (or maybe later) to avoid conflicts.

It's all far from ideal, and it all stems from your arrogant actions.

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.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mfd: ucb1x00: remove NO_IRQ check
  2016-09-07 11:27         ` Russell King - ARM Linux
@ 2016-09-07 12:48           ` Lee Jones
  2016-09-07 13:44             ` Russell King - ARM Linux
  0 siblings, 1 reply; 16+ messages in thread
From: Lee Jones @ 2016-09-07 12:48 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Arnd Bergmann, Linus Walleij, Hans-Christian Egtvedt,
	Dmitry Torokhov, Thomas Gleixner, linux-kernel

On Wed, 07 Sep 2016, Russell King - ARM Linux wrote:

> On Wed, Sep 07, 2016 at 11:27:37AM +0100, Lee Jones wrote:
> > On Tue, 06 Sep 2016, Russell King - ARM Linux wrote:
> > 
> > > On Tue, Sep 06, 2016 at 04:45:30PM +0100, Lee Jones wrote:
> > > > On Tue, 06 Sep 2016, Russell King - ARM Linux wrote:
> > > > 
> > > > > You need to send this _to_ me as I need to merge it with my other
> > > > > changes.  This patch on its own does not make sense - it only makes
> > > > > sense with the rest of my SA11x0 patch stack.
> > > > > 
> > > > > NAK for Lee to merge this.
> > > > 
> > > > So if I were to accept this patch, would anything break?  In other
> > > > words, is there an ordering issue where this this change relies on
> > > > something you have in your tree?
> > > > 
> > > 1) This is my driver which I've maintained in the past myself, including
> > >    handling all patches for.  This pre-dates your decision to take over
> > >    the mfd stuff.  I'm still maintaining this driver and I have not
> > >    passed the responsibility to you.
> > 
> > There is no mention of you maintaining this driver in MAINTAINERS.
> > This is the first I've heard of it.  You haven't taken patches for
> > this driver since January 2012 (4 years, 8 months).  Over that period
> > I have accepted 9 patches and conducted more reviews and you haven't
> > taken part or shown any interest whatsoever.
> 
> Maybe I haven't taken any patches because I haven't seen any of these
> patches?
> 
> Yes, you're right that I'm not listed in MAINTAINERS for it, that's
> because it was merged a long time ago when I was maintaining almost
> everything ARM, and listing everything in MAINTAINERS was not really
> viable - there wasn't even the regexp patterns in MAINTAINERS to
> describe what was there.
> 
> Thanks for mentioning it though, I'll add myself so that I get the
> patches in future.

Jolly good.  I'll happily Ack that patch.

I would very much welcome your expertise and review comments, so long
as when finally applied to Mainline, they are merged with the
remainder of the MFD changes.

In the interest of merge-conflict avoidance, this still doesn't mean
you have right to apply patches from a subsystem which is actively
maintained without Maintainer consent though.  If we let everyone do
that there would be chaos, which I am not prepared to tolerate.

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

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

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

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

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

> You messed up, plain and simple - I think purposely to make a political
> point given our discussion last week.

This point is tit-for-tat based on what I said.  I'm not going to
substantiate it with an eloquent response.

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

I took your upstream credibility into account and assumed you knew
what you were doing, (and yes the patch looked trivial,) as I do with
quite a few of my quality contributors.  I'll try not to make that
mistake with you in future.

> > > 4) I find it annoying that you've picked up on this patch in the way
> > >    that you have (as a result of my NAK), yet you have failed to make
> > >    any comment what so ever on _my_ patch, which you were copied with
> > >    on the 30th August.
> > 
> > I'd seriously consider checking your mail filters if I were you (the
> > reply was even addressed To: you, just how you like it.  It took me
> > exactly an hour from you sending the patch to me reviewing and
> > accepting it:
> > 
> >   http://www.spinics.net/lists/arm-kernel/msg527414.html
> 
> 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:

"Lee, I intend to take patch X through my tree because Y."

"Okay Russell, no problem.  Can you send me a pull-request when you
do, which I can rebase on in the case of merge-conflict?"

Or, even better in this case, since there is only MFD patch:

"Certainly Russell.  I'll stick the patch on an immutable branch for
you and provide you with a pull-request."

This is how it works for *all* the other Maintainers I work with,
which is quite a few.  I know you have some pretty odd systems and
processes in place, but working with you shouldn't be different to
working with anyone else.

> > It's even in -next, if you'd cared enough to look:
> > 
> >   http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=159aed02b0074bac1c21544befb773dce39b9fcb
> 
> I don't look at -next, sorry.  Anything that needs me to go off and do
> extra work is pretty much a no-no at the moment (and is a no-no at the
> best of times.)  I'm still pretty much working solidly on this series,
> and it remains a work-in-progress - as the purpose of posting it is to
> get feedback and testing - and I've been getting results from people
> testing, which has been provoking further changes to the whole series.
> That's pretty much a full-time job at the moment.

I can certainly empathise with that.

> > > This is part of the problem I have here with your attitude with mfd:
> > > you decided yourself to become maintainer of everything in drivers/mfd,
> > > riding rough-shod over your fellow maintainers.  And when your fellow
> > > maintainers try to reassert control, you get upset about it.  Sorry,
> > > you can't have it both ways.
> > 
> > Not sure what you're trying to say here to be honest.  I offered my
> > help to Sam, the then MFD Maintainer [0], which was subsequently
> > accepted.  He then became busy with NFC, so I took the driver's seat.
> > 
> >   [0] http://www.spinics.net/lists/kernel/msg1525957.html
> > 
> > I try to Maintain MFD in a simple, effective manner, where my
> > priorities are letting good code easily pass through, keeping bad code
> > out and most relevant here, trying to prevent merge-conflicts so Linus
> > has an easy time during the merge-window.
> > 
> > It doesn't make sense for drivers which reside in the same subsystem
> > to be applied to multiple trees.  I like for all changes to MFD to be
> > reflected in the MFD pull-request.  If they *need* to go in via other
> > repos *as well*, that's fine too.  I am usually very happy to provide
> > pull-requests when those needs arise.
> > 
> > If you are insistent on applying this yourself, so long as you have a
> > *technical* (rather than territorial/spiteful) reason for doing so, I,
> > as always, will be happy to oblige you with a pull-request too.
> 
> I think I've already pointed out the technical argument here, and why
> your attitude towards total ownership of MFD is broken.

I disagree.  It's been going well for over 3 years now.

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

> > > Now, if you had discussed my patch from the 30th _first_ then I would
> > > not be NAKing this patch, and I probably would not be making such a
> > > big deal about this.  But let's put that aside and stick to the
> > > technicalities.
> > 
> > I think this *rant* of yours might be based on very shakey foundations
> > then, because I *did* reply to your patch.  From what I saw, it was a
> > reasonable patch, so I took it.
> > 
> > Maybe if you would have known the driver as well as you profess to,
> > you would have fixed the issue properly and there would be no need for
> > this patch in the first place.  I tease of course!  ;)
> 
> As I said above, the whole series (of some 60-90 patches) is still very
> much a work in progress.  It took two days to post the whole series
> (29th and 30th August) so getting all the details into every message
> wasn't practical.  However, there's enough of a clue there: the size
> of the overall series,

The size of the over-all series, as far as my vision goes was 8.  And
the other 7 patches were based in arm/*.  It's *very* rare for patches
to be required to to into MFD and ARM simultaneously.

> the comments in the sub-series cover message,

... were cryptic.

> the fact that earlier cover message had RFC in, over sub-series cover
> messages were asking for acks, etc.

You didn't send anything to me with "RFC", and no mention of this
being part of a collection of series'.  I think you've made way too
many assumptions here and not been forthcoming or clear at all.  You
only have yourself to blame!

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

> > > There are two dependencies here.  Right now, the probe_irq_on()
> > > returning zero on SA11x0 platforms, causing ucb1x00_detect_irq() to
> > > return NO_IRQ, which then causes the probe function to fail.  Whether
> > > that happens on PXA or not, I don't know and I have no way to test
> > > anymore as I donated all my PXA platforms to Robert.
> > > 
> > > Applying Arnd's patch on its own, or applying my patch on its own will
> > > cause ucb1x00 to initialise with either NO_IRQ or 0 in ucb->irq, which
> > > causes the device to have no interrupts - or worse, it screws up any
> > > configuration of IRQ0 that the platform may have (eg, PXA).  Applying
> > > both together results in the probe function continuing to fail.
> > > 
> > > My patch is not supposed to be applied on its own; it is supposed to
> > > be applied with the third patch already in place - a GPIO patch.  So
> > > yes, there _were_ dependencies here.
> > > 
> > > That dependency can be solved by taking _both_ my patch (first) and
> > > Arnd's patch.  However, given that each patch introduces its own
> > > different form of breakage, it makes sense to combine the two patches
> > > to avoid that breakage.  So, when I'm less busy sorting out other
> > > SA11x0 stuff, I want to discuss with Arnd about combining them into
> > > a single patch.
> > > 
> > > _Then_ we need to have a discussion about how to handle the patch.
> > 
> > Good explanation.  Just the sort of information I'd expect during a
> > patch review.  Much more helpful than "this patch doesn't make
> > sense.  Give it to me.  NACK".  Now I can work with you both to
> > resolve this issue properly and professionally.
> 
> 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.

> > So, since I already have half of the resolution, all I need to do is
> > squash this into it, right?  Or do we need to carry this GPIO patch
> > too?  If so, where is that currently?
> 
> So, let's apply your maintainership style to GPIO... and let's say
> that LinusW just applied the patch within hours because it "looked
> simple enough".  Then what?  The dependencies are split between two
> trees, potentially with other patches behind them.
> 
> No, LinusW didn't do that.  Linus read the cover message, realised
> that it was part of a larger series, and decided not to take the
> patch, because there was a huge number of dependent patches on top
> of it touching _many_ different subsystems.
> 
> 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.

Go read them again and take a look at the differences.

You have no one to blame but yourself.

> As a result of your actions, I'm now having to ask LinusW to take
> the GPIO patch and get it into mainline ASAP, preferably for -rc6,
> so at least the straight-line tips of LinusT's tree don't end up
> with weirdness - and so I can rebase my entire series on top of
> -rc6 (or maybe later) to avoid conflicts.

That's your choice.  It's not the only (or best) way to solve this.

I work with Linus all the time.  We use the methods I described.
Works well.

> It's all far from ideal, and it all stems from your arrogant actions.

Wrong!  Read up!

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

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mfd: ucb1x00: remove NO_IRQ check
  2016-09-07 12:48           ` Lee Jones
@ 2016-09-07 13:44             ` Russell King - ARM Linux
  2016-09-07 15:08               ` Lee Jones
  0 siblings, 1 reply; 16+ messages in thread
From: Russell King - ARM Linux @ 2016-09-07 13:44 UTC (permalink / raw)
  To: Lee Jones
  Cc: Arnd Bergmann, Linus Walleij, Hans-Christian Egtvedt,
	Dmitry Torokhov, Thomas Gleixner, linux-kernel

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mfd: ucb1x00: remove NO_IRQ check
  2016-09-07 13:44             ` Russell King - ARM Linux
@ 2016-09-07 15:08               ` Lee Jones
  2016-09-07 16:07                 ` Russell King - ARM Linux
  0 siblings, 1 reply; 16+ messages in thread
From: Lee Jones @ 2016-09-07 15:08 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Arnd Bergmann, Linus Walleij, Hans-Christian Egtvedt,
	Dmitry Torokhov, Thomas Gleixner, linux-kernel

On Wed, 07 Sep 2016, Russell King - ARM Linux wrote:

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

Mistakes happen.  I'm fine with that.  I'm only taking exception
because you are trying to put the blame on me, when clearly you are
the one at fault.

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

That's my point.  You did *not* do that.

> Only
> someone with a political point to make, or someone being obtuse
> would ignore that.

Tit-for-tat - I already accused you of that.  I had no agenda.  Just
maintaining MFD the same was as I always do.  I think we know who's
trying to make the political point here!

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

I think the issue here is the definition of dependant.  Because this
is MFD, and it's entire reason for being is to register and interact
with leaf drivers, almost all of my submitters send patches as sets
which contain changes to said leaf drivers.  A lot of times they are
"related" but not necessarily "dependant" (as described before,
remember; *build* and *API* are the only type of dependency I  will
accept for cross-subsystem patches/sets).

More often than not when people send sets, it's because they patches
are functionally related, but do not depend on each other per say.
Another example of this is when people send changes to files in
drivers/ and also send their DTS counterparts in the same set.  It's
*very* common, but certainly does not mean that all the patches should
go through one repo.

Disagree all you like, but that's just the way it is.

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

Take control of yourself Russell.  It's just an email.

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

We've already covered the fact that you should have sent it as an
[RFC].  None of this would have happened if you'd done so.  Let's
leave it at that.

OOI, why are you doing this massive overhaul for next to zero users?
Isn't that a massive waste of your valuable time?

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

That's not what I've been saying is it?

What I'm saying is; follow the processes;

[A]
 - Send me the patch
 - Be forthcoming with your intentions
 - We'll work something out between us using commonly used processes

If the only option for you is to make changes all over the tree and
only you take them in, then that is not going to work!

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

I'm telling you outright -- you sent *me* one patch with a
cover-letter which did not state your intentions and was not an RFC:

30 2016 Russell King - AR (  0) [PATCH 0/8] SA11x0/PXA remainder & cleanups
30 2016 Russell King      (  0) └>[PATCH 1/8] mfd: ucb1x00: allow IRQ probing to work with IRQs > 32

Take responsibility for your own actions.

> 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"?

We've been over this.  As I already mentioned, most people send me
sets and most of the time they aren't *build* or *api* dependent,
so they can normally be applied on their own.

I also looked at the subjects of the other patches (because that's all
you sent me), and noticed that all of the remaining patches were ARM
patches.  Which do not normally have decencies with drivers/.  And
when they do, the contributor mentions the fact so we can deal with
that accordingly.  You did not do that.

As I'm sure you can appreciate, this is a very abnormal submission and
you supplied me with no valid information to that fact, so I treated
it as a normal submission.  I would have never let you apply the patch
on your own, for reasons I've already explained.

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

That's the problem, it was not clear, at all.  You said you "could
have arguably applied it earlier in the set".  But without knowing
that this wasn't a stand-alone set (how could I, you didn't mention
that), what does the really mean?

As a comparison, a *clear* message would be like the one I quoted to
you in the last message.  I believe it's further down from here.  Take
a look.

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

Insults aren't going to get you anywhere with me I'm afraid.

It is you who have admitted making mistakes.  The fallout are a direct
result of *your* incompetence and failure to understand the rules.

Other Maintainers might allow you to subvert their processes and do
your own thing, but I'm not willing to tolerate that kind of nonsense.
Not even from you!

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

Yawn!

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

  -------

This is the point you are missing!  *You* made no reference to a
larger "overall" series, or the fact that this was a "sub-set".  If
you can get your head around that, you can start to understand where
*you* went wrong.

The fact of the matter is, because *you* didn't provide the relevant
information with regards to your intent, it made your comment
cryptic.

I sincerely believe that that if you made it clear what was going on,
there would be no crossed wires and we wouldn't be having this lovely
conversation.

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

Wrong again.  Read up to save me for labouring points.

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

Nothing absurd about them.

I am happy with the way I have operated given the information I had
at the time.  I'm pretty sure there is little I could have
*reasonably* done to prevent this, besides reading L(A)KML.

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

See my list above, I'm going to mark it with a [A] so you can find
it.  If you would have completed those items, as any reasonable
contributor would, asking you what your plans are is *exactly* what I
would have done as part of that process.

You completed that list with Linus, and he acted accordingly.  You did
*not* do that in the MFD case, thus mine and Linus' actions would be
different, wouldn't they?  I'm sure Linus' would have done the same as
me if he received the same submission.

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

Are you reading what I write, because this statement would assume not.

The submission you sent me what not at the same *quality* level as the
one you sent Linus.  I guarantee you Linus and I would have acted the
same way with the same submission.

If there is any doubt, I always ask submitters what their intentions
are.  That level of doubt was not evident in your somewhat lacking
submission.

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

Sorry, you're wrong here.  I can reference 100's of patches which were
part of sets that were merged on their own without asking. That's just
the nature of MFD and their leaf drivers.

I grant you the submissions you see are different to the ones I have
to deal with, but take my word for it, I receive *way* more patch sets
that are "related", but not {build,API} "dependant", than I do the
other way around.

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

We've covered this.  If you with to divert from the accepted process,
you need be more forthcoming in the description.

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

Then I guess we'll have to agree to disagree.

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

If you allude to dependencies in the set like everyone else does,
absolutely!

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mfd: ucb1x00: remove NO_IRQ check
  2016-09-07 15:08               ` Lee Jones
@ 2016-09-07 16:07                 ` Russell King - ARM Linux
  2016-09-07 16:27                   ` Lee Jones
  0 siblings, 1 reply; 16+ messages in thread
From: Russell King - ARM Linux @ 2016-09-07 16:07 UTC (permalink / raw)
  To: Lee Jones
  Cc: Arnd Bergmann, Linus Walleij, Hans-Christian Egtvedt,
	Dmitry Torokhov, Thomas Gleixner, linux-kernel

On Wed, Sep 07, 2016 at 04:08:46PM +0100, Lee Jones wrote:
> On Wed, 07 Sep 2016, Russell King - ARM Linux wrote:
> > 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.
> 
> We've already covered the fact that you should have sent it as an
> [RFC].  None of this would have happened if you'd done so.  Let's
> leave it at that.

I wonder if you realise, or even known, given your relative inexperience,
that many people actually _ignore_ patches with a RFC tag, and provide
no review or comments against them.  Remember, by your own admission,
there's twenty years experience difference between us.

I'm going to take one last issue with your comments:

> That's the problem, it was not clear, at all.  You said you "could
> have arguably applied it earlier in the set".  But without knowing
> that this wasn't a stand-alone set (how could I, you didn't mention
> that), what does the really mean?

So by your own admission, you weren't sure of the understanding, and
from the extract of your mailbox that you kindly provided earlier in
your reply:

> 30 2016 Russell King - AR (  0) [PATCH 0/8] SA11x0/PXA remainder & cleanups
> 30 2016 Russell King      (  0) └>[PATCH 1/8] mfd: ucb1x00: allow IRQ probing to work with IRQs > 32

if that's all you saw, "earlier in the set" in the first message
wouldn't make any sense, and should've set alarm bells ringing that
something had gone wrong, or you were without complete information.

The reasonable thing to have done - especially by your own admission
that you found it confusing - would have been to ask for clarification.
You did not, you chose after just one hour (again, your admission) to
apply the patch.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mfd: ucb1x00: remove NO_IRQ check
  2016-09-07 16:07                 ` Russell King - ARM Linux
@ 2016-09-07 16:27                   ` Lee Jones
  2016-09-07 16:36                     ` Russell King - ARM Linux
       [not found]                     ` <1473265954.29864.15.camel@perches.com>
  0 siblings, 2 replies; 16+ messages in thread
From: Lee Jones @ 2016-09-07 16:27 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Arnd Bergmann, Linus Walleij, Hans-Christian Egtvedt,
	Dmitry Torokhov, Thomas Gleixner, linux-kernel

On Wed, 07 Sep 2016, Russell King - ARM Linux wrote:
> On Wed, Sep 07, 2016 at 04:08:46PM +0100, Lee Jones wrote:
> > On Wed, 07 Sep 2016, Russell King - ARM Linux wrote:
> > > 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.
> > 
> > We've already covered the fact that you should have sent it as an
> > [RFC].  None of this would have happened if you'd done so.  Let's
> > leave it at that.
> 
> I wonder if you realise, or even known, given your relative inexperience,
> that many people actually _ignore_ patches with a RFC tag, and provide
> no review or comments against them.

That's their prerogative.  I would take that to mean that the set is
reasonable, and would subsequently follow up with a full submission.

No problem there.

> Remember, by your own admission,
> there's twenty years experience difference between us.

True.  And times have changed a lot since the 'good ol' days'.  I
guess for you this means a lot less freedom than you're used to which
I'm truly sorry about.  However, the processes I (and most of the guys
I work with, including your besty LinusW) are in place for the better.

> I'm going to take one last issue with your comments:
> 
> > That's the problem, it was not clear, at all.  You said you "could
> > have arguably applied it earlier in the set".  But without knowing
> > that this wasn't a stand-alone set (how could I, you didn't mention
> > that), what does the really mean?
> 
> So by your own admission, you weren't sure of the understanding, and
> from the extract of your mailbox that you kindly provided earlier in
> your reply:
> 
> > 30 2016 Russell King - AR (  0) [PATCH 0/8] SA11x0/PXA remainder & cleanups
> > 30 2016 Russell King      (  0) └>[PATCH 1/8] mfd: ucb1x00: allow IRQ probing to work with IRQs > 32
> 
> if that's all you saw, "earlier in the set" in the first message
> wouldn't make any sense, and should've set alarm bells ringing that
> something had gone wrong, or you were without complete information.
> 
> The reasonable thing to have done - especially by your own admission
> that you found it confusing - would have been to ask for clarification.
> You did not, you chose after just one hour (again, your admission) to
> apply the patch.

If I queried every little oddity I read in commit messages and cover
letters, it would either eat up all of my time, ensuring that I am not
functional as an Engineer or Maintainer, or it would drive me to
distraction where I would subsequently end up in some kind of asylum.

Last time; "I see no issue with the way I operated given the
information that was provided."

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] mfd: ucb1x00: remove NO_IRQ check
  2016-09-07 16:27                   ` Lee Jones
@ 2016-09-07 16:36                     ` Russell King - ARM Linux
       [not found]                     ` <1473265954.29864.15.camel@perches.com>
  1 sibling, 0 replies; 16+ messages in thread
From: Russell King - ARM Linux @ 2016-09-07 16:36 UTC (permalink / raw)
  To: Lee Jones
  Cc: Arnd Bergmann, Linus Walleij, Hans-Christian Egtvedt,
	Dmitry Torokhov, Thomas Gleixner, linux-kernel

On Wed, Sep 07, 2016 at 05:27:13PM +0100, Lee Jones wrote:
> On Wed, 07 Sep 2016, Russell King - ARM Linux wrote:
> > On Wed, Sep 07, 2016 at 04:08:46PM +0100, Lee Jones wrote:
> > > On Wed, 07 Sep 2016, Russell King - ARM Linux wrote:
> > > > 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.
> > > 
> > > We've already covered the fact that you should have sent it as an
> > > [RFC].  None of this would have happened if you'd done so.  Let's
> > > leave it at that.
> > 
> > I wonder if you realise, or even known, given your relative inexperience,
> > that many people actually _ignore_ patches with a RFC tag, and provide
> > no review or comments against them.
> 
> That's their prerogative.  I would take that to mean that the set is
> reasonable, and would subsequently follow up with a full submission.
> 
> No problem there.

Total lack of understanding that there _is_ a problem there.

> > Remember, by your own admission,
> > there's twenty years experience difference between us.
> 
> True.  And times have changed a lot since the 'good ol' days'.  I
> guess for you this means a lot less freedom than you're used to which
> I'm truly sorry about.  However, the processes I (and most of the guys
> I work with, including your besty LinusW) are in place for the better.

Some things have changed, but some things haven't, and they haven't
changed in the way you think they have.

> > I'm going to take one last issue with your comments:
> > 
> > > That's the problem, it was not clear, at all.  You said you "could
> > > have arguably applied it earlier in the set".  But without knowing
> > > that this wasn't a stand-alone set (how could I, you didn't mention
> > > that), what does the really mean?
> > 
> > So by your own admission, you weren't sure of the understanding, and
> > from the extract of your mailbox that you kindly provided earlier in
> > your reply:
> > 
> > > 30 2016 Russell King - AR (  0) [PATCH 0/8] SA11x0/PXA remainder & cleanups
> > > 30 2016 Russell King      (  0) └>[PATCH 1/8] mfd: ucb1x00: allow IRQ probing to work with IRQs > 32
> > 
> > if that's all you saw, "earlier in the set" in the first message
> > wouldn't make any sense, and should've set alarm bells ringing that
> > something had gone wrong, or you were without complete information.
> > 
> > The reasonable thing to have done - especially by your own admission
> > that you found it confusing - would have been to ask for clarification.
> > You did not, you chose after just one hour (again, your admission) to
> > apply the patch.
> 
> If I queried every little oddity I read in commit messages and cover
> letters, it would either eat up all of my time, ensuring that I am not
> functional as an Engineer or Maintainer, or it would drive me to
> distraction where I would subsequently end up in some kind of asylum.

Thank you for admitting that you made a mistake, even though you
don't directly acknowledge it, but it's sad that you persist in
thinking that you're whiter than white when you are in fact not.

It is very rare that interactions between two people that go wrong
are solely one person's fault - a lesson that I guess you haven't
learnt yet.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* rfc: Updating SubmittingPatches with [RFC PATCH] and/or [WIP PATCH]
       [not found]                       ` <20160907163854.GE4921@dell>
@ 2016-09-07 19:47                         ` Joe Perches
  2016-09-07 21:49                           ` Randy Dunlap
  2016-09-14 19:03                           ` Jonathan Corbet
  0 siblings, 2 replies; 16+ messages in thread
From: Joe Perches @ 2016-09-07 19:47 UTC (permalink / raw)
  To: Lee Jones; +Cc: Russell King - ARM Linux, lkml, Jonathan Corbet

On Wed, 2016-09-07 at 17:38 +0100, Lee Jones wrote:
> On Wed, 07 Sep 2016, Joe Perches wrote:
[]
> > And another patch series prefix that could be used
> > instead of RFC is WIP.
> Certainly sounds reasonable. Is there a difference in the meaning?

Request for Comment and Work In Progress differences:

Maybe add something like this to Documentation/SubmittingPatches
---
 Documentation/SubmittingPatches | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 8c79f1d..aa76057 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -404,6 +404,12 @@ convention to prefix your subject line with [PATCH].  This 
 and other kernel developers more easily distinguish patches from other
 e-mail discussions.
 
+If the patch or patch series is just a proposal to garner comments
+use [RFC PATCH] before each patch subject.
+
+If the patch or patch seriies is incomplete or possibly contains known
+defects and you would like others to see the work to date use
+[WIP PATCH] before each patch subject.
 
 
 11) Sign your work

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: rfc: Updating SubmittingPatches with [RFC PATCH] and/or [WIP PATCH]
  2016-09-07 19:47                         ` rfc: Updating SubmittingPatches with [RFC PATCH] and/or [WIP PATCH] Joe Perches
@ 2016-09-07 21:49                           ` Randy Dunlap
  2016-09-14 19:03                           ` Jonathan Corbet
  1 sibling, 0 replies; 16+ messages in thread
From: Randy Dunlap @ 2016-09-07 21:49 UTC (permalink / raw)
  To: Joe Perches, Lee Jones; +Cc: Russell King - ARM Linux, lkml, Jonathan Corbet

On 09/07/16 12:47, Joe Perches wrote:

> Request for Comment and Work In Progress differences:
> 
> Maybe add something like this to Documentation/SubmittingPatches
> ---
>  Documentation/SubmittingPatches | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index 8c79f1d..aa76057 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -404,6 +404,12 @@ convention to prefix your subject line with [PATCH].  This 
>  and other kernel developers more easily distinguish patches from other
>  e-mail discussions.
>  
> +If the patch or patch series is just a proposal to garner comments
> +use [RFC PATCH] before each patch subject.
> +
> +If the patch or patch seriies is incomplete or possibly contains known

                         series

> +defects and you would like others to see the work to date use
> +[WIP PATCH] before each patch subject.
>  
>  
>  11) Sign your work
> 


-- 
~Randy

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: rfc: Updating SubmittingPatches with [RFC PATCH] and/or [WIP PATCH]
  2016-09-07 19:47                         ` rfc: Updating SubmittingPatches with [RFC PATCH] and/or [WIP PATCH] Joe Perches
  2016-09-07 21:49                           ` Randy Dunlap
@ 2016-09-14 19:03                           ` Jonathan Corbet
  1 sibling, 0 replies; 16+ messages in thread
From: Jonathan Corbet @ 2016-09-14 19:03 UTC (permalink / raw)
  To: Joe Perches; +Cc: Lee Jones, Russell King - ARM Linux, lkml

On Wed, 07 Sep 2016 12:47:41 -0700
Joe Perches <joe@perches.com> wrote:

> +If the patch or patch series is just a proposal to garner comments
> +use [RFC PATCH] before each patch subject.
> +
> +If the patch or patch seriies is incomplete or possibly contains known
> +defects and you would like others to see the work to date use
> +[WIP PATCH] before each patch subject.

It would be good to apply Randy's typo fix here.

Also, though, if we're going to do this, it's probably worth mentioning
that one should not expect patches marked RFC to be applied, and that
some maintainers may not even seriously review them. That seems to
surprise people regularly.

jon

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-09-14 19:03 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-06 13:03 [PATCH] mfd: ucb1x00: remove NO_IRQ check Arnd Bergmann
2016-09-06 13:17 ` Russell King - ARM Linux
2016-09-06 13:49   ` Arnd Bergmann
2016-09-06 15:45   ` Lee Jones
2016-09-06 16:28     ` Russell King - ARM Linux
2016-09-07 10:27       ` Lee Jones
2016-09-07 11:27         ` Russell King - ARM Linux
2016-09-07 12:48           ` Lee Jones
2016-09-07 13:44             ` Russell King - ARM Linux
2016-09-07 15:08               ` Lee Jones
2016-09-07 16:07                 ` Russell King - ARM Linux
2016-09-07 16:27                   ` Lee Jones
2016-09-07 16:36                     ` Russell King - ARM Linux
     [not found]                     ` <1473265954.29864.15.camel@perches.com>
     [not found]                       ` <20160907163854.GE4921@dell>
2016-09-07 19:47                         ` rfc: Updating SubmittingPatches with [RFC PATCH] and/or [WIP PATCH] Joe Perches
2016-09-07 21:49                           ` Randy Dunlap
2016-09-14 19:03                           ` Jonathan Corbet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).