| Message ID | 20230301034247.136007-2-bhe@redhat.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v5,01/17] asm-generic/iomap.h: remove ARCH_HAS_IOREMAP_xx macros | expand |
On 01/03/2023 03:42, Baoquan He wrote: > diff --git a/drivers/net/ethernet/sfc/io.h b/drivers/net/ethernet/sfc/io.h > index 30439cc83a89..07f99ad14bf3 100644 > --- a/drivers/net/ethernet/sfc/io.h > +++ b/drivers/net/ethernet/sfc/io.h > @@ -70,7 +70,7 @@ > */ > #ifdef CONFIG_X86_64 > /* PIO is a win only if write-combining is possible */ > -#ifdef ARCH_HAS_IOREMAP_WC > +#ifdef ioremap_wc > #define EFX_USE_PIO 1 > #endif > #endif So I don't know how valid what we're doing here is... > diff --git a/include/asm-generic/iomap.h b/include/asm-generic/iomap.h > index 08237ae8b840..196087a8126e 100644 > --- a/include/asm-generic/iomap.h > +++ b/include/asm-generic/iomap.h > @@ -93,15 +93,15 @@ extern void __iomem *ioport_map(unsigned long port, unsigned int nr); > extern void ioport_unmap(void __iomem *); > #endif > > -#ifndef ARCH_HAS_IOREMAP_WC > +#ifndef ioremap_wc > #define ioremap_wc ioremap > #endif ... but it looks like this will break it, since in sfc/io.h `#ifdef ioremap_wc` will always be true (if I'm correctly understanding what we get via #include <linux/io.h>, which I'm probably not because asm includes always confuse me). I.e. we're not just interested in "can code that calls ioremap_wc compile?", we care about whether we actually get WC, because we're making an optimisation decision based on it.
On Wed, Mar 01, 2023 at 04:38:10AM +0000, Edward Cree wrote: > On 01/03/2023 03:42, Baoquan He wrote: > > diff --git a/drivers/net/ethernet/sfc/io.h b/drivers/net/ethernet/sfc/io.h > > index 30439cc83a89..07f99ad14bf3 100644 > > --- a/drivers/net/ethernet/sfc/io.h > > +++ b/drivers/net/ethernet/sfc/io.h > > @@ -70,7 +70,7 @@ > > */ > > #ifdef CONFIG_X86_64 > > /* PIO is a win only if write-combining is possible */ > > -#ifdef ARCH_HAS_IOREMAP_WC > > +#ifdef ioremap_wc > > #define EFX_USE_PIO 1 > > #endif > > #endif > > So I don't know how valid what we're doing here is... Well, x86 defines ARCH_HAS_IOREMAP_WC unconditionally, so it doesn't affect you ... but you raise a good question about how a driver can determine if it's actually getting WC memory.
On 03/01/23 at 05:11am, Matthew Wilcox wrote: > On Wed, Mar 01, 2023 at 04:38:10AM +0000, Edward Cree wrote: > > On 01/03/2023 03:42, Baoquan He wrote: > > > diff --git a/drivers/net/ethernet/sfc/io.h b/drivers/net/ethernet/sfc/io.h > > > index 30439cc83a89..07f99ad14bf3 100644 > > > --- a/drivers/net/ethernet/sfc/io.h > > > +++ b/drivers/net/ethernet/sfc/io.h > > > @@ -70,7 +70,7 @@ > > > */ > > > #ifdef CONFIG_X86_64 > > > /* PIO is a win only if write-combining is possible */ > > > -#ifdef ARCH_HAS_IOREMAP_WC > > > +#ifdef ioremap_wc > > > #define EFX_USE_PIO 1 > > > #endif > > > #endif > > > > So I don't know how valid what we're doing here is... > > Well, x86 defines ARCH_HAS_IOREMAP_WC unconditionally, so it doesn't > affect you ... but you raise a good question about how a driver can > determine if it's actually getting WC memory. Yeah, this change doesn't affect sfc. Because ARCH_HAS_IOREMAP_WC is used to make ioremap_wc defined in <asm/io.h> override the default one in <asm-generic/iomap.h>, this patch has made code have the same effect. Besides, I have a question still in my mind. Surely this is unrelated to this patch. In commit 38d9029a652c (parisc: Define ioremap_uc and ioremap_wc), ioremap_wc definition was added in arch/parisc/include/asm/io.h, and it didn't add ARCH_HAS_IOREMAP_WC definition. However, it won't cause redefinition of ioremap_wc, even though there's "#include <asm-generic/iomap.h>" at below. I could be dizzy on these io.h and iomap.h. When I added ioremap_wt and ioremap_np to debug, ioremap_np will cause redefinition, while ioremap_wt woundn't. Does anyone know what I am missing? diff --git a/arch/parisc/include/asm/io.h b/arch/parisc/include/asm/io.h index c05e781be2f5..20d566eec3b3 100644 --- a/arch/parisc/include/asm/io.h +++ b/arch/parisc/include/asm/io.h @@ -127,6 +127,8 @@ static inline void gsc_writeq(unsigned long long val, unsigned long addr) */ void __iomem *ioremap(unsigned long offset, unsigned long size); #define ioremap_wc ioremap +#define ioremap_wt ioremap +#define ioremap_np ioremap #define ioremap_uc ioremap #define pci_iounmap pci_iounmap
diff --git a/arch/loongarch/include/asm/io.h b/arch/loongarch/include/asm/io.h index 402a7d9e3a53..505e3b583463 100644 --- a/arch/loongarch/include/asm/io.h +++ b/arch/loongarch/include/asm/io.h @@ -5,8 +5,6 @@ #ifndef _ASM_IO_H #define _ASM_IO_H -#define ARCH_HAS_IOREMAP_WC - #include <linux/kernel.h> #include <linux/types.h> diff --git a/arch/m68k/include/asm/io_mm.h b/arch/m68k/include/asm/io_mm.h index d41fa488453b..6a0abd4846c6 100644 --- a/arch/m68k/include/asm/io_mm.h +++ b/arch/m68k/include/asm/io_mm.h @@ -26,8 +26,6 @@ #include <asm/virtconvert.h> #include <asm/kmap.h> -#include <asm-generic/iomap.h> - #ifdef CONFIG_ATARI #define atari_readb raw_inb #define atari_writeb raw_outb diff --git a/arch/m68k/include/asm/kmap.h b/arch/m68k/include/asm/kmap.h index dec05743d426..4efb3efa593a 100644 --- a/arch/m68k/include/asm/kmap.h +++ b/arch/m68k/include/asm/kmap.h @@ -4,8 +4,6 @@ #ifdef CONFIG_MMU -#define ARCH_HAS_IOREMAP_WT - /* Values for nocacheflag and cmode */ #define IOMAP_FULL_CACHING 0 #define IOMAP_NOCACHE_SER 1 diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h index e6d5ccaa309e..cec8347f0b85 100644 --- a/arch/mips/include/asm/io.h +++ b/arch/mips/include/asm/io.h @@ -12,8 +12,6 @@ #ifndef _ASM_IO_H #define _ASM_IO_H -#define ARCH_HAS_IOREMAP_WC - #include <linux/compiler.h> #include <linux/kernel.h> #include <linux/types.h> @@ -25,7 +23,6 @@ #include <asm/byteorder.h> #include <asm/cpu.h> #include <asm/cpu-features.h> -#include <asm-generic/iomap.h> #include <asm/page.h> #include <asm/pgtable-bits.h> #include <asm/processor.h> @@ -210,6 +207,8 @@ void iounmap(const volatile void __iomem *addr); #define ioremap_wc(offset, size) \ ioremap_prot((offset), (size), boot_cpu_data.writecombine) +#include <asm-generic/iomap.h> + #if defined(CONFIG_CPU_CAVIUM_OCTEON) || defined(CONFIG_CPU_LOONGSON64) #define war_io_reorder_wmb() wmb() #else diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h index fc112a91d0c2..7a82e6f70ced 100644 --- a/arch/powerpc/include/asm/io.h +++ b/arch/powerpc/include/asm/io.h @@ -3,11 +3,6 @@ #define _ASM_POWERPC_IO_H #ifdef __KERNEL__ -#define ARCH_HAS_IOREMAP_WC -#ifdef CONFIG_PPC32 -#define ARCH_HAS_IOREMAP_WT -#endif - /* */ @@ -695,9 +690,7 @@ static inline void name at \ #define writel_relaxed(v, addr) writel(v, addr) #define writeq_relaxed(v, addr) writeq(v, addr) -#ifdef CONFIG_GENERIC_IOMAP -#include <asm-generic/iomap.h> -#else +#ifndef CONFIG_GENERIC_IOMAP /* * Here comes the implementation of the IOMAP interfaces. */ diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h index e9025640f634..76238842406a 100644 --- a/arch/x86/include/asm/io.h +++ b/arch/x86/include/asm/io.h @@ -35,9 +35,6 @@ * - Arnaldo Carvalho de Melo <acme@conectiva.com.br> */ -#define ARCH_HAS_IOREMAP_WC -#define ARCH_HAS_IOREMAP_WT - #include <linux/string.h> #include <linux/compiler.h> #include <linux/cc_platform.h> @@ -212,8 +209,6 @@ void memset_io(volatile void __iomem *, int, size_t); #define memcpy_toio memcpy_toio #define memset_io memset_io -#include <asm-generic/iomap.h> - /* * ISA space is 'always mapped' on a typical x86 system, no need to * explicitly ioremap() it. The fact that the ISA IO space is mapped diff --git a/drivers/net/ethernet/sfc/io.h b/drivers/net/ethernet/sfc/io.h index 30439cc83a89..07f99ad14bf3 100644 --- a/drivers/net/ethernet/sfc/io.h +++ b/drivers/net/ethernet/sfc/io.h @@ -70,7 +70,7 @@ */ #ifdef CONFIG_X86_64 /* PIO is a win only if write-combining is possible */ -#ifdef ARCH_HAS_IOREMAP_WC +#ifdef ioremap_wc #define EFX_USE_PIO 1 #endif #endif diff --git a/drivers/net/ethernet/sfc/siena/io.h b/drivers/net/ethernet/sfc/siena/io.h index 30439cc83a89..07f99ad14bf3 100644 --- a/drivers/net/ethernet/sfc/siena/io.h +++ b/drivers/net/ethernet/sfc/siena/io.h @@ -70,7 +70,7 @@ */ #ifdef CONFIG_X86_64 /* PIO is a win only if write-combining is possible */ -#ifdef ARCH_HAS_IOREMAP_WC +#ifdef ioremap_wc #define EFX_USE_PIO 1 #endif #endif diff --git a/include/asm-generic/iomap.h b/include/asm-generic/iomap.h index 08237ae8b840..196087a8126e 100644 --- a/include/asm-generic/iomap.h +++ b/include/asm-generic/iomap.h @@ -93,15 +93,15 @@ extern void __iomem *ioport_map(unsigned long port, unsigned int nr); extern void ioport_unmap(void __iomem *); #endif -#ifndef ARCH_HAS_IOREMAP_WC +#ifndef ioremap_wc #define ioremap_wc ioremap #endif -#ifndef ARCH_HAS_IOREMAP_WT +#ifndef ioremap_wt #define ioremap_wt ioremap #endif -#ifndef ARCH_HAS_IOREMAP_NP +#ifndef ioremap_np /* See the comment in asm-generic/io.h about ioremap_np(). */ #define ioremap_np ioremap_np static inline void __iomem *ioremap_np(phys_addr_t offset, size_t size)
Let's use '#define ioremap_xx' and "#ifdef ioremap_xx" instead. For architectures defined ARCH_HAS_IOREMAP_xx macros in <asm/io.h>, the relevant adjustments are made to avoid compiling error: loongarch: - doesn't include <asm-generic/iomap.h>, defining ARCH_HAS_IOREMAP_WC is redundant, so simply remove it. m68k: - selected GENERIC_IOMAP, so <asm-generic/iomap.h> has been added in <asm-generic/io.h>, and <asm/kmap.h> is included above <asm-generic/iomap.h>, so simply remove ARCH_HAS_IOREMAP_WT defining. mips: - move "#include <asm-generic/iomap.h>" below ioremap_wc definition in <asm/io.h> powerpc: - remove "#include <asm-generic/iomap.h>" in <asm/io.h> because it's duplicated with the one in <asm-generic/io.h>, let's rely on the latter. x86: - selected GENERIC_IOMAP, remove #include <asm-generic/iomap.h> at the middle of <asm/io.h>. Let's rely on <asm-generic/io.h>. This is a preparation patch so that Later patch don't need to add ARCH_HAS_IOREMAP_xx macro. Signed-off-by: Baoquan He <bhe@redhat.com> Cc: loongarch@lists.linux.dev Cc: linux-m68k@lists.linux-m68k.org Cc: linux-mips@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: x86@kernel.org Cc: netdev@vger.kernel.org Cc: linux-arch@vger.kernel.org --- arch/loongarch/include/asm/io.h | 2 -- arch/m68k/include/asm/io_mm.h | 2 -- arch/m68k/include/asm/kmap.h | 2 -- arch/mips/include/asm/io.h | 5 ++--- arch/powerpc/include/asm/io.h | 9 +-------- arch/x86/include/asm/io.h | 5 ----- drivers/net/ethernet/sfc/io.h | 2 +- drivers/net/ethernet/sfc/siena/io.h | 2 +- include/asm-generic/iomap.h | 6 +++--- 9 files changed, 8 insertions(+), 27 deletions(-)