Re: [PATCH] usb: host: fhci-hcd: annotate PIPE_CONTROL switch case with fallthrough

[Rasmus Villemoes <linux@rasmusvillemoes.dk>]

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