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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx> 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




[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux