Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190429081937.7544-1-geert@linux-m68k.org> <CAK8P3a1NbjYAGCTq2RUfqEX1OgppEV5mKmBJ0t=JiFF13zSytg@mail.gmail.com>
 <CAMuHMdUHnW13j07559peRijggGQwExrJ9TBaAW7ZqWcT1onpSg@mail.gmail.com>
In-Reply-To: <CAMuHMdUHnW13j07559peRijggGQwExrJ9TBaAW7ZqWcT1onpSg@mail.gmail.com>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Mon, 17 Jun 2019 22:23:45 +0200
Message-ID: <CAK8P3a3fvTPZKhASGA5cZE2WwTM8LgA45kgbm0XE2KjwCopTSA@mail.gmail.com>
Subject: Re: [PATCH] m68k: io: Fix io{read,write}{16,32}be() for Coldfire peripherals
To:     Geert Uytterhoeven <geert@linux-m68k.org>
Cc:     Greg Ungerer <gerg@linux-m68k.org>,
        Angelo Dureghello <angelo@sysam.it>,
        Logan Gunthorpe <logang@deltatee.com>,
        linux-m68k <linux-m68k@lists.linux-m68k.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jun 14, 2019 at 4:52 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Mon, Apr 29, 2019 at 2:40 PM Arnd Bergmann <arnd@arndb.de> 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