From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Joe Perches <joe@perches.com>,
"Gustavo A. R. Silva" <gustavo@embeddedor.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Timur Tabi <timur@kernel.org>, Li Yang <leoyang.li@nxp.com>,
Anton Vorontsov <avorontsov@ru.mvista.com>,
kbuild test robot <lkp@intel.com>,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] usb: host: fhci-hcd: annotate PIPE_CONTROL switch case with fallthrough
Date: Mon, 17 Feb 2020 20:44:04 +0100 [thread overview]
Message-ID: <f0e804c2-f7a3-da91-9929-38ac7f017081@rasmusvillemoes.dk> (raw)
In-Reply-To: <e1ec529a1a01bc38513c05308757bc45b53597c3.camel@perches.com>
On 17/02/2020 18.33, Joe Perches wrote:
> On Mon, 2020-02-17 at 11:12 -0600, Gustavo A. R. Silva wrote:
>>
>>>> Reported-by: kbuild test robot <lkp@intel.com>
>>>> Fixes: 5a35435ef4e6 (soc: fsl: qe: remove PPC32 dependency from CONFIG_QUICC_ENGINE)
>>>> Fixes: a035d552a93b (Makefile: Globally enable fall-through warning)
>>
>> By the way, the "Fixes" tag above makes no sense. There is nothing wrong about
>> that commit. It just enabled the fall-through warning globally. Why would you
>> "fix" that?"
Depends on whether you consider a change that introduces a warning in an
otherwise warning-free build a regression or not. That commit claimed
Now that all the fall-through warnings have been addressed in the
kernel, enable the fall-through warning globally.
but as I explained below the fold, any CONFIG_PPC32+CONFIG_USB_FHCI_HCD
.config grew a warning due to a035d552a93b. So at least in that sense
there is something wrong about that commit - the above claim is simply
false. Please note that I don't expect anybody to ever be able to
actually cover everything before doing something like what a035d552a93b
does, so I'm not complaining, just explaining.
Then I introduced a change which made that code compile for a ppc64
allmodconfig, which apparently 0day does cover, which is why I added
that other tag.
> There could be some effort made to better specify when "Fixes:"
> tags should be used.
Indeed. I explicitly chose not to cc stable because I don't think it's
for -stable. But in case somebody (or Sasha's ML) decides it is, I went
out of my way to include relevant commits and an explanation for the
somewhat odd dual Fixes:. So no, I don't think Fixes implies or should
imply Cc stable - and I think this is all consistent with
submitting-patches.rst:
Patches that fix a severe bug in a released kernel should be directed
toward the stable maintainers...
and
A Fixes: tag indicates that the patch fixes an issue in a previous commit.
Nothing says that Fixes is reserved for -stable material.
> I believe "Fixes:" should be used only when changes have some
> runtime impact.
Perhaps. But it's hard to make the rules completely rigid - suppose
commit A does fix a real bug and is backported, however, in some configs
it introduces some warnings; that gets fixed by B which doesn't change
generated code. Should B be backported, or should the -stable tree(s)
live with those warnings?
"Fixes:" should not be used for changes that
> just silence compiler warnings using W=<123>.
I tend to agree, but that's completely irrelevant in this case, as this
is not a warning that only appears for W=<123>.
Rasmus
next prev parent reply other threads:[~2020-02-17 19:44 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-13 8:54 [PATCH] usb: host: fhci-hcd: annotate PIPE_CONTROL switch case with fallthrough Rasmus Villemoes
2020-02-13 12:56 ` Greg Kroah-Hartman
2020-02-13 13:35 ` Rasmus Villemoes
2020-02-13 15:23 ` [PATCH] checkpatch: Prefer fallthrough; over fallthrough comments Joe Perches
2020-02-17 9:38 ` [PATCH] usb: host: fhci-hcd: annotate PIPE_CONTROL switch case with fallthrough Greg Kroah-Hartman
2020-02-17 14:12 ` Rasmus Villemoes
2020-02-17 14:18 ` Greg Kroah-Hartman
2020-02-17 16:15 ` Gustavo A. R. Silva
2020-02-17 16:29 ` Greg Kroah-Hartman
2020-02-17 17:12 ` Gustavo A. R. Silva
2020-02-17 17:33 ` Joe Perches
2020-02-17 19:44 ` Rasmus Villemoes [this message]
2020-02-13 21:11 ` Leo Li
2020-02-17 17:28 ` Gustavo A. R. Silva
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f0e804c2-f7a3-da91-9929-38ac7f017081@rasmusvillemoes.dk \
--to=linux@rasmusvillemoes.dk \
--cc=avorontsov@ru.mvista.com \
--cc=gregkh@linuxfoundation.org \
--cc=gustavo@embeddedor.com \
--cc=joe@perches.com \
--cc=leoyang.li@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=lkp@intel.com \
--cc=timur@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).