Re: [PATCH v1 1/2] m68k: io_mm.h - add APNE 100 MBit support

[Michael Schmitz <schmitzmic@gmail.com>]

Hi Geert,

thanks for the explanation - and thanks Finn and Andreas for clarifying. 
I've removed the annotation in v2.

Cheers,

     Michael

On 10/06/21 7:32 pm, Geert Uytterhoeven wrote:
> Hi Finn,
>
> On Thu, Jun 10, 2021 at 3:09 AM Finn Thain <fthain@linux-m68k.org> wrote:
>> On Thu, 10 Jun 2021, Michael Schmitz wrote:
>>> On 9/06/21 8:04 pm, Andreas Schwab wrote:
>>>> On Jun 09 2021, Michael Schmitz wrote:
>>>>
>>>>> @@ -136,6 +141,9 @@ static inline u8 __iomem *isa_itb(unsigned long addr)
>>>>>        case ISA_TYPE_Q40: return (u8 __iomem *)Q40_ISA_IO_B(addr);
>>>>>    #endif
>>>>>    #ifdef CONFIG_AMIGA_PCMCIA
>>>>> +#ifdef CONFIG_APNE100MBIT
>>>>> +    case ISA_TYPE_AG16: fallthrough;
>>>>> +#endif
>>>>>        case ISA_TYPE_AG: return (u8 __iomem *)AG_ISA_IO_B(addr);
>>>> Is the fallthrough annotation really needed?
>>> Just to shut up compiler warnings, and even that I haven't seen myself.
>>>
>>> I have seen a number of patches adding either comments or this annotation in
>>> the core NCR5380 driver (which Finn maintains, who suggested this annotation
>>> to an earlier version of the Q40/Atari io_mm.h patch), so adding annotations
>>> appears to be encouraged now.
>>>
>>> I personally think these annotations are over the top generally, but I've
>>> learned to program when computed goto statements were still en vogue.
>>>
>>> In this particular case, there can be no doubt that the fallthrough is
>>> intentional, so on balance, I'll remove those annotations as well (unless Finn
>>> strongly objects?).
>>>
>> I don't object to removing it. On the contrary, in a previous message I
>> also questioned adding this particular 'fallthrough' (though I did
>> recommended adding a different one).
>>
>> In general, there's no way to predict which static checkers are going to
>> complain about any given line of code. They don't all agree about
>> correctness and they are a moving target, just like fashion or reviewers'
>> preferred code styles.
> AFAIK, they only complain when the switch() operates on an enum,
> and not all enum values are handled.
>
> When operating on an int, there's not enough address space on
> 32-bit machines to handle all cases ;-)
>
> Gr{oetje,eeting}s,
>
>                          Geert
>