Re: [PATCH v2 2/5] spi: spi-fsl-dspi: Exit the ISR with IRQ_NONE when it's not ours

From: Mark Brown
Date: Fri Aug 23 2019 - 17:04:11 EST


On Fri, Aug 23, 2019 at 03:06:52PM +0300, Vladimir Oltean wrote:

> - You left change requests in the initial patchset I submitted, but
> you partially applied the series anyway. You didn't give me a chance
> to respin the whole series and put the shared IRQ fix on top, so it
> applies on old trees as well. No problem, I sent two versions of the
> patch.

Right, and this is fine. A big part of this is that it's just
generally bad practice to not have fixes at the front of the
series, I'd flag this up as a problem even if the code was all
new and there was no question of applying as a bug fix. It's
something that's noticable just at the level of looking at the
shape of the series without even looking at the contents of the
patches, if the fix is actually a good one or anything like that.
In the context of this it made it look like the reason you'd had
to do two versions.

> So I didn't put any target version in the patch titles this time,
> although arguably it would have been clearer to you that there's a
> patch for-5.4 and another version of it for-4.20 (which i *think* is
> how I should submit a fix, I don't see any branch for inclusion in
> stable trees per se).

Not for 4.20, for v5.3 - we basically only fix Linus' tree
directly, anything else gets backported from there unless it's
super important. I don't think anyone is updating v4.20 at all
these days, the version number change from v4 to v5 was totally
arbatrary.

> Yes, I did send a cover letter for a single patch. I thought it's
> harder to miss than a note hidden under patch 2/5 of one series, and
> in the note section of the other's. I think you could have also made

If you're sending a multi-patch series it's of course good to
send a cover letter, it's just single patches where it's adding
overhead.

> No problem, you missed the link between the two. I sent you a link to
> the lkml archive. You said "I'm not online enough to readily follow
> that link right now". Please teach me - I really don't know - how can

It's not that I missed the link between them, it's that what I'd
expected to see was the fix being the first patch in the series
for -next and for that fix to look substantially the same with at
most some context difference. I wasn't expecting to see a
completely different patch that wasn't at the start of the
series, had the fix been at the start of the series it'd have
been fairly clear what was going on but the refactoring patch
looked like the main reason you'd needed different versions (it's
certainly why they don't visually resemble each other).

In other words it looked like you'd sent a different fix because
the fix you'd done for -next was based on the first patch in the
series rather than there also being some context changes.

> I make links between patchsets easier for you to follow, if you don't
> read cover letters and you can't access lkml? I promise I'll use that
> method next time.

Like I said include a plain text description of what you're
linking to (eg, the subject line from a mail).

> > I do frequently catch up on my mail on flights or while otherwise
> > travelling so this is even more pressing for me than just being about
> > making things a bit easier to read.

> Maybe you simply should do something else while traveling, just saying.

I could also add in the coffee shop I sometimes work from which
doesn't have WiFi or mobile coverage. Besides, like that part of
the text does say it's also a usability thing, having to fire up
a web browser to figure out what's being described is a stumbling
block.

Attachment: signature.asc
Description: PGP signature