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

[Jiaxun Yang]

> 2023年5月19日 22:05，Arnd Bergmann <arnd@xxxxxxxx> 写道：
> 
> 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@xxxxxxxxxx>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx>
>> [jiaxun.yang@xxxxxxxxxxx: Massage more headers, fix ioport defines]
>> Cc: Thomas Bogendoerfer <tsbogend@xxxxxxxxxxxxxxxx>
>> Cc: Huacai Chen <chenhuacai@xxxxxxxxxx>
>> Cc: Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx>
>> Cc: linux-mips@xxxxxxxxxxxxxxx
>> ---
>> 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?

Since now {in,out}{bwlq} have a `_` in prefix, it needs a corresponding
ioswab variant to work.

> 
>> +#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.

Will update in a follow-up patch, I’m planing to do some cleanup for those
headers later.

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

Will opt-out them on 32 bit build.

> 
> 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.

Current implementation of __raw_readq __raw_readq  will check for CPU’s
64bit support and do a 64bit access if possible, it’s helpful for running
32 bit kernel on 64 bit CPUs.

Thanks
- Jiaxun

> 
>     Arnd