Re: [PATCH v2 1/2] mips: add <asm-generic/io.h> including

["Arnd Bergmann" <arnd@arndb.de>]

On Wed, Mar 1, 2023, at 11:22, Baoquan He 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.
>
> Here, remove the <asm/io.h> including in asm/mmiowb.h, otherwise nested
> including will cause compiling error.
>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> 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

This looks good to me,

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

but it obviously needs to be properly reviewed by the MIPS
maintainers as well. I think others have tried to do this
in the past but did not make it in.

> @@ -548,6 +552,46 @@ 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 inb_p inb_p
> +#define inw_p inw_p
> +#define inl_p inl_p
> +#define insb insb
> +#define insw insw
> +#define insl insl

I would prefer to put the #defines next to the function declarations,
even when they come from macros.

> 
> -#include <asm/io.h>
> -
>  #define mmiowb()	iobarrier_w()
> 

I think this only works as long as asm/spinlock.h also includes
asm/io.h, otherwise linux/spinlock.h will be missing the
iobarrier_w definition.

Most likely this is implicitly included from somewhere else
below linux/spinlock.h, but it would be better not to rely
on that, and instead define mmiowb() to wmb() directly.

     Arnd