Re: [PATCH] m68k: io: Fix io{read,write}{16,32}be() for Coldfire peripherals

[Arnd Bergmann]

On Fri, Jun 14, 2019 at 4:52 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> On Mon, Apr 29, 2019 at 2:40 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
> > On Mon, Apr 29, 2019 at 10:19 AM Geert Uytterhoeven
> > This looks correct to me, but there are two points that stick out to me:
> >
> > - why do you only do this for mmio and not for pio?
>
> Because no one had a need for it? ;-)
>
> Now seriously, m68k only has MMIO, no PIO.  Any PIO, if used, is for ISA or
> PCMCIA I/O accesses, which are little endian.

I suppose the majority of PIO operations are single-byte only,
so endianess doesn't apply at all ;-). You are certainly right
that big-endian PIO access has close to zero uses in the
kernel (I could not find any either).

If you don't have ISA or PCMCIA devices, then using the
trivial readl/writel based wrappers is clearly the easiest way.

If you do have PIO based devices, then defining pio_read16be
etc would save a few bytes in lib/iomap.c.

> > - why do you even use the generic_iomap wrappers rather than
> >   the trivial asm-generic versions of those functions?
>
> Looking at git history, that was done to fix some link errors, which no
> longer seem to happen with the more mature asm-generic infrastructure we
> have now.
>
> So probably we can drop the selection of GENERIC_IOMAP and inclusion
> of <asm-generic/iomap.h>, after fixing a few compiler warnings like:
>
>     include/asm-generic/io.h: In function âioread8_repâ
>     arch/m68k/include/asm/io_mm.h:371:44: warning: passing argument 1
> of âraw_insbâ discard âconstâ qualifier from pointer target type
> [-Wdiscarded-qualifiers]
>      #define readsb(port, buf, nr)     raw_insb((port), (u8 *)(buf), (nr))
>     arch/m68k/include/asm/raw_io.h:101:50: note: expected âvolatile u8
> *â {aka âvolatile unsigned char *â} but argument is of type âconst
> volatile void *â
>     static inline void raw_insb(volatile u8 __iomem *port, u8 *buf,
> unsigned int len)

Those should just be missing 'volatile' and 'const' modifiers
on the arguments.

         Arnd