Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-431-g1d6a3ebb56-fm-20230511.001-g1d6a3ebb
Mime-Version: 1.0
Message-Id: <dae342ed-8999-4fa5-b719-322182580025@app.fastmail.com>
In-Reply-To: <20230519195135.79600-1-jiaxun.yang@flygoat.com>
References: <20230519195135.79600-1-jiaxun.yang@flygoat.com>
Date:   Fri, 19 May 2023 23:05:15 +0200
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Jiaxun Yang" <jiaxun.yang@flygoat.com>, linux-mips@vger.kernel.org
Cc:     linux-kernel@vger.kernel.org,
        "Thomas Bogendoerfer" <tsbogend@alpha.franken.de>,
        Linux-Arch <linux-arch@vger.kernel.org>,
        "Baoquan He" <bhe@redhat.com>,
        "Huacai Chen" <chenhuacai@kernel.org>
Subject: Re: [PATCH v4] mips: add <asm-generic/io.h> including
Content-Type: text/plain
Precedence: bulk

On Fri, May 19, 2023, at 21:51, Jiaxun Yang wrote:
> With the adding, some default ioremap_xx methods defined in
> asm-generic/io.h can be used. E.g the default ioremap_uc() returning
> NULL.
>
> We also massaged various headers to avoid nested includes.
>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> [jiaxun.yang@flygoat.com: Massage more headers, fix ioport defines]
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Huacai Chen <chenhuacai@kernel.org>
> Cc: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Cc: linux-mips@vger.kernel.org
> ---
> Tested against qemu malta 34Kf, boston I6500, Loongson64, hopefully
> everything is fine now.

Thanks a lot for figuring this out!
> 
> @@ -44,6 +42,11 @@
>  # define __raw_ioswabq(a, x)	(x)
>  # define ____raw_ioswabq(a, x)	(x)
> 
> +# define _ioswabb ioswabb
> +# define _ioswabw ioswabw
> +# define _ioswabl ioswabl
> +# define _ioswabq ioswabq
> +

I'm missing something here, what are these macros used for in addition
to the non-underscore versions?

> +#define memset_io memset_io
>  static inline void memset_io(volatile void __iomem *addr, unsigned 
> char val, int count)
>  {
>  	memset((void __force *) addr, val, count);
>  }
> +#define memcpy_fromio memcpy_fromio
>  static inline void memcpy_fromio(void *dst, const volatile void 
> __iomem *src, int count)
>  {
>  	memcpy(dst, (void __force *) src, count);
>  }
> +#define memcpy_toio memcpy_toio
>  static inline void memcpy_toio(volatile void __iomem *dst, const void 
> *src, int count)
>  {
>  	memcpy((void __force *) dst, src, count);

These three could probably go away now, as they are identical
to the asm-generic version. Not important though.

> @@ -549,6 +555,47 @@ extern void (*_dma_cache_inv)(unsigned long start, 
> unsigned long size);
>  #define csr_out32(v, a) (*(volatile u32 *)((unsigned long)(a) + 
> __CSR_32_ADJUST) = (v))
>  #define csr_in32(a)    (*(volatile u32 *)((unsigned long)(a) + 
> __CSR_32_ADJUST))
> 
> +
> +#define __raw_readb __raw_readb
> +#define __raw_readw __raw_readw
> +#define __raw_readl __raw_readl
> +#define __raw_readq __raw_readq
> +#define __raw_writeb __raw_writeb
> +#define __raw_writew __raw_writew
> +#define __raw_writel __raw_writel
> +#define __raw_writeq __raw_writeq
> +
> +#define readb readb
> +#define readw readw
> +#define readl readl
> +#define writeb writeb
> +#define writew writew
> +#define writel writel
> +
> +#define readsb readsb
> +#define readsw readsw
> +#define readsl readsl
> +#define readsq readsq
> +#define writesb writesb
> +#define writesw writesw
> +#define writesl writesl
> +#define writesq writesq

As far as I can tell, the readsq()/writesq() helpers are currently
only defined on 64-bit, it's probably best to leave it that way.

On most other architectures, we also don't define __raw_readq()
and __raw_writeq() on 32-bit because they lose the atomicity that
might be required for FIFO accesses, but the existing MIPS version
has them, so changing those should be a separate patch after it
can be shown to not break anything.

     Arnd