linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Updates for asm-generic/pci.h
@ 2022-07-17  3:34 Stafford Horne
  2022-07-17  3:34 ` [PATCH v2 1/2] asm-generic: Remove pci.h copying remaining code to x86 Stafford Horne
  2022-07-17  3:34 ` [PATCH v2 2/2] asm-generic: Add new pci.h and use it Stafford Horne
  0 siblings, 2 replies; 20+ messages in thread
From: Stafford Horne @ 2022-07-17  3:34 UTC (permalink / raw)
  To: LKML
  Cc: Arnd Bergmann, Stafford Horne, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Guo Ren, linux-riscv, linux-csky

When reviewing the OpenRISC PCI support patch Arnd suggested that
we avoid copying arm64 and riscv asm/pci.h and moving that to be
the new asm-generic/pci.h.

This patch does that by first moving the old pci.h definition
of pci_get_legacy_ide_irq out to the architectures that use it,
this turns out to only be x86.

Next, we create the new pci.h definition.

Since v1:

 - Remove definition of pci_get_legacy_ide_irq on architectures
   not using CONFIG_PNP, which eliminated most.
 - Add ifdef around PCIBIOS_MIN_MEM for consistency.

Stafford Horne (2):
  asm-generic: Remove pci.h copying remaining code to x86
  asm-generic: Add new pci.h and use it

 arch/alpha/include/asm/pci.h   |  1 -
 arch/arm64/include/asm/pci.h   | 12 +++------
 arch/csky/include/asm/pci.h    | 24 +++--------------
 arch/ia64/include/asm/pci.h    |  1 -
 arch/m68k/include/asm/pci.h    |  7 +++--
 arch/powerpc/include/asm/pci.h |  1 -
 arch/riscv/include/asm/pci.h   | 25 +++---------------
 arch/s390/include/asm/pci.h    |  1 -
 arch/sparc/include/asm/pci.h   |  9 -------
 arch/um/include/asm/pci.h      | 24 ++---------------
 arch/x86/include/asm/pci.h     |  6 +++--
 arch/xtensa/include/asm/pci.h  |  3 ---
 include/asm-generic/pci.h      | 47 ++++++++++++++++++++++++----------
 13 files changed, 54 insertions(+), 107 deletions(-)

-- 
2.36.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 1/2] asm-generic: Remove pci.h copying remaining code to x86
  2022-07-17  3:34 [PATCH v2 0/2] Updates for asm-generic/pci.h Stafford Horne
@ 2022-07-17  3:34 ` Stafford Horne
  2022-07-17  9:23   ` Geert Uytterhoeven
  2022-07-18  4:33   ` Christoph Hellwig
  2022-07-17  3:34 ` [PATCH v2 2/2] asm-generic: Add new pci.h and use it Stafford Horne
  1 sibling, 2 replies; 20+ messages in thread
From: Stafford Horne @ 2022-07-17  3:34 UTC (permalink / raw)
  To: LKML
  Cc: Arnd Bergmann, Stafford Horne, Richard Henderson,
	Ivan Kokshaysky, Matt Turner, Geert Uytterhoeven,
	Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, David S. Miller,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Chris Zankel, Max Filippov, Bjorn Helgaas,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Nick Child,
	Niklas Schnelle, Matthew Rosato, Pierre Morel, Kees Cook,
	Gustavo A. R. Silva, linux-alpha, linux-ia64, linux-m68k,
	linuxppc-dev, linux-s390, sparclinux, linux-xtensa, linux-pci,
	linux-arch, linux-riscv

The generic pci.h header now only provides a definition of
pci_get_legacy_ide_irq which is used by architectures that support PNP.
Of the architectures that use asm-generic/pci.h this is only x86.

This patch removes the old pci.h in order to make room for a new
pci.h to be used by arm64, riscv, openrisc, etc.

The existing code in pci.h is moved out to x86.  On other architectures
we clean up any outstanding references.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Link: https://lore.kernel.org/lkml/CAK8P3a0JmPeczfmMBE__vn=Jbvf=nkbpVaZCycyv40pZNCJJXQ@mail.gmail.com/
Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 arch/alpha/include/asm/pci.h   |  1 -
 arch/ia64/include/asm/pci.h    |  1 -
 arch/m68k/include/asm/pci.h    |  7 +++++--
 arch/powerpc/include/asm/pci.h |  1 -
 arch/s390/include/asm/pci.h    |  1 -
 arch/sparc/include/asm/pci.h   |  9 ---------
 arch/x86/include/asm/pci.h     |  6 ++++--
 arch/xtensa/include/asm/pci.h  |  3 ---
 include/asm-generic/pci.h      | 17 -----------------
 9 files changed, 9 insertions(+), 37 deletions(-)
 delete mode 100644 include/asm-generic/pci.h

diff --git a/arch/alpha/include/asm/pci.h b/arch/alpha/include/asm/pci.h
index cf6bc1e64d66..8ac5af0fc4da 100644
--- a/arch/alpha/include/asm/pci.h
+++ b/arch/alpha/include/asm/pci.h
@@ -56,7 +56,6 @@ struct pci_controller {
 
 /* IOMMU controls.  */
 
-/* TODO: integrate with include/asm-generic/pci.h ? */
 static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
 {
 	return channel ? 15 : 14;
diff --git a/arch/ia64/include/asm/pci.h b/arch/ia64/include/asm/pci.h
index 8c163d1d0189..218412d963c2 100644
--- a/arch/ia64/include/asm/pci.h
+++ b/arch/ia64/include/asm/pci.h
@@ -63,7 +63,6 @@ static inline int pci_proc_domain(struct pci_bus *bus)
 	return (pci_domain_nr(bus) != 0);
 }
 
-#define HAVE_ARCH_PCI_GET_LEGACY_IDE_IRQ
 static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
 {
 	return channel ? isa_irq_to_vector(15) : isa_irq_to_vector(14);
diff --git a/arch/m68k/include/asm/pci.h b/arch/m68k/include/asm/pci.h
index 5a4bc223743b..0c272ff515cc 100644
--- a/arch/m68k/include/asm/pci.h
+++ b/arch/m68k/include/asm/pci.h
@@ -2,11 +2,14 @@
 #ifndef _ASM_M68K_PCI_H
 #define _ASM_M68K_PCI_H
 
-#include <asm-generic/pci.h>
-
 #define	pcibios_assign_all_busses()	1
 
 #define	PCIBIOS_MIN_IO		0x00000100
 #define	PCIBIOS_MIN_MEM		0x02000000
 
+static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
+{
+	return channel ? 15 : 14;
+}
+
 #endif /* _ASM_M68K_PCI_H */
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 915d6ee4b40a..f9da506751bb 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -39,7 +39,6 @@
 #define pcibios_assign_all_busses() \
 	(pci_has_flag(PCI_REASSIGN_ALL_BUS))
 
-#define HAVE_ARCH_PCI_GET_LEGACY_IDE_IRQ
 static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
 {
 	if (ppc_md.pci_get_legacy_ide_irq)
diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
index fdb9745ee998..5889ddcbc374 100644
--- a/arch/s390/include/asm/pci.h
+++ b/arch/s390/include/asm/pci.h
@@ -6,7 +6,6 @@
 #include <linux/mutex.h>
 #include <linux/iommu.h>
 #include <linux/pci_hotplug.h>
-#include <asm-generic/pci.h>
 #include <asm/pci_clp.h>
 #include <asm/pci_debug.h>
 #include <asm/sclp.h>
diff --git a/arch/sparc/include/asm/pci.h b/arch/sparc/include/asm/pci.h
index 4deddf430e5d..0c58f65bd172 100644
--- a/arch/sparc/include/asm/pci.h
+++ b/arch/sparc/include/asm/pci.h
@@ -40,13 +40,4 @@ static inline int pci_proc_domain(struct pci_bus *bus)
 #define get_pci_unmapped_area get_fb_unmapped_area
 #endif /* CONFIG_SPARC64 */
 
-#if defined(CONFIG_SPARC64) || defined(CONFIG_LEON_PCI)
-static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
-{
-	return PCI_IRQ_NONE;
-}
-#else
-#include <asm-generic/pci.h>
-#endif
-
 #endif /* ___ASM_SPARC_PCI_H */
diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
index f3fd5928bcbb..7da27f665cfe 100644
--- a/arch/x86/include/asm/pci.h
+++ b/arch/x86/include/asm/pci.h
@@ -105,8 +105,10 @@ static inline void early_quirks(void) { }
 
 extern void pci_iommu_alloc(void);
 
-/* generic pci stuff */
-#include <asm-generic/pci.h>
+static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
+{
+	return channel ? 15 : 14;
+}
 
 #ifdef CONFIG_NUMA
 /* Returns the node based on pci bus */
diff --git a/arch/xtensa/include/asm/pci.h b/arch/xtensa/include/asm/pci.h
index 8e2b48a268db..b56de9635b6c 100644
--- a/arch/xtensa/include/asm/pci.h
+++ b/arch/xtensa/include/asm/pci.h
@@ -43,7 +43,4 @@
 #define ARCH_GENERIC_PCI_MMAP_RESOURCE	1
 #define arch_can_pci_mmap_io()		1
 
-/* Generic PCI */
-#include <asm-generic/pci.h>
-
 #endif	/* _XTENSA_PCI_H */
diff --git a/include/asm-generic/pci.h b/include/asm-generic/pci.h
deleted file mode 100644
index 6bb3cd3d695a..000000000000
--- a/include/asm-generic/pci.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * linux/include/asm-generic/pci.h
- *
- *  Copyright (C) 2003 Russell King
- */
-#ifndef _ASM_GENERIC_PCI_H
-#define _ASM_GENERIC_PCI_H
-
-#ifndef HAVE_ARCH_PCI_GET_LEGACY_IDE_IRQ
-static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
-{
-	return channel ? 15 : 14;
-}
-#endif /* HAVE_ARCH_PCI_GET_LEGACY_IDE_IRQ */
-
-#endif /* _ASM_GENERIC_PCI_H */
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 2/2] asm-generic: Add new pci.h and use it
  2022-07-17  3:34 [PATCH v2 0/2] Updates for asm-generic/pci.h Stafford Horne
  2022-07-17  3:34 ` [PATCH v2 1/2] asm-generic: Remove pci.h copying remaining code to x86 Stafford Horne
@ 2022-07-17  3:34 ` Stafford Horne
  2022-07-18  4:37   ` Christoph Hellwig
  1 sibling, 1 reply; 20+ messages in thread
From: Stafford Horne @ 2022-07-17  3:34 UTC (permalink / raw)
  To: LKML
  Cc: Arnd Bergmann, Stafford Horne, Catalin Marinas, Will Deacon,
	Guo Ren, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Richard Weinberger, Anton Ivanov, Johannes Berg, Bjorn Helgaas,
	linux-arm-kernel, linux-csky, linux-riscv, linux-um, linux-pci,
	linux-arch

The asm/pci.h used for many newer architectures share similar
definitions.  Move the common parts to asm-generic/pci.h to allow for
sharing code.

Two things to note are:

 - isa_dma_bridge_buggy, traditionally this is defined in asm/dma.h but
   these architectures avoid creating that file and add the definition
   to asm/pci.h.
 - ARCH_GENERIC_PCI_MMAP_RESOURCE, csky does not define this so we
   undefine it after including asm-generic/pci.h.  Why doesn't csky
   define it?
 - pci_get_legacy_ide_irq, This function is only used on architectures
   that support PNP.  It is only maintained for arm64, in other
   architectures it is removed.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Link: https://lore.kernel.org/lkml/CAK8P3a0JmPeczfmMBE__vn=Jbvf=nkbpVaZCycyv40pZNCJJXQ@mail.gmail.com/
Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 arch/arm64/include/asm/pci.h | 12 +++---------
 arch/csky/include/asm/pci.h  | 24 ++++--------------------
 arch/riscv/include/asm/pci.h | 25 +++----------------------
 arch/um/include/asm/pci.h    | 24 ++----------------------
 include/asm-generic/pci.h    | 36 ++++++++++++++++++++++++++++++++++++
 5 files changed, 48 insertions(+), 73 deletions(-)
 create mode 100644 include/asm-generic/pci.h

diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
index b33ca260e3c9..1180e83712f5 100644
--- a/arch/arm64/include/asm/pci.h
+++ b/arch/arm64/include/asm/pci.h
@@ -9,7 +9,6 @@
 #include <asm/io.h>
 
 #define PCIBIOS_MIN_IO		0x1000
-#define PCIBIOS_MIN_MEM		0
 
 /*
  * Set to 1 if the kernel should re-assign all PCI bus numbers
@@ -18,9 +17,6 @@
 	(pci_has_flag(PCI_REASSIGN_ALL_BUS))
 
 #define arch_can_pci_mmap_wc() 1
-#define ARCH_GENERIC_PCI_MMAP_RESOURCE	1
-
-extern int isa_dma_bridge_buggy;
 
 #ifdef CONFIG_PCI
 static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
@@ -28,11 +24,9 @@ static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
 	/* no legacy IRQ on arm64 */
 	return -ENODEV;
 }
-
-static inline int pci_proc_domain(struct pci_bus *bus)
-{
-	return 1;
-}
 #endif  /* CONFIG_PCI */
 
+/* Generic PCI */
+#include <asm-generic/pci.h>
+
 #endif  /* __ASM_PCI_H */
diff --git a/arch/csky/include/asm/pci.h b/arch/csky/include/asm/pci.h
index ebc765b1f78b..44866c1ad461 100644
--- a/arch/csky/include/asm/pci.h
+++ b/arch/csky/include/asm/pci.h
@@ -9,26 +9,10 @@
 
 #include <asm/io.h>
 
-#define PCIBIOS_MIN_IO		0
-#define PCIBIOS_MIN_MEM		0
+/* Generic PCI */
+#include <asm-generic/pci.h>
 
-/* C-SKY shim does not initialize PCI bus */
-#define pcibios_assign_all_busses() 1
-
-extern int isa_dma_bridge_buggy;
-
-#ifdef CONFIG_PCI
-static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
-{
-	/* no legacy IRQ on csky */
-	return -ENODEV;
-}
-
-static inline int pci_proc_domain(struct pci_bus *bus)
-{
-	/* always show the domain in /proc */
-	return 1;
-}
-#endif  /* CONFIG_PCI */
+/* csky doesn't use generic pci resource mapping */
+#undef ARCH_GENERIC_PCI_MMAP_RESOURCE
 
 #endif  /* __ASM_CSKY_PCI_H */
diff --git a/arch/riscv/include/asm/pci.h b/arch/riscv/include/asm/pci.h
index 7fd52a30e605..12ce8150cfb0 100644
--- a/arch/riscv/include/asm/pci.h
+++ b/arch/riscv/include/asm/pci.h
@@ -12,29 +12,7 @@
 
 #include <asm/io.h>
 
-#define PCIBIOS_MIN_IO		0
-#define PCIBIOS_MIN_MEM		0
-
-/* RISC-V shim does not initialize PCI bus */
-#define pcibios_assign_all_busses() 1
-
-#define ARCH_GENERIC_PCI_MMAP_RESOURCE 1
-
-extern int isa_dma_bridge_buggy;
-
 #ifdef CONFIG_PCI
-static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
-{
-	/* no legacy IRQ on risc-v */
-	return -ENODEV;
-}
-
-static inline int pci_proc_domain(struct pci_bus *bus)
-{
-	/* always show the domain in /proc */
-	return 1;
-}
-
 #ifdef	CONFIG_NUMA
 
 static inline int pcibus_to_node(struct pci_bus *bus)
@@ -50,4 +28,7 @@ static inline int pcibus_to_node(struct pci_bus *bus)
 
 #endif  /* CONFIG_PCI */
 
+/* Generic PCI */
+#include <asm-generic/pci.h>
+
 #endif  /* _ASM_RISCV_PCI_H */
diff --git a/arch/um/include/asm/pci.h b/arch/um/include/asm/pci.h
index da13fd5519ef..34fe4921b5fa 100644
--- a/arch/um/include/asm/pci.h
+++ b/arch/um/include/asm/pci.h
@@ -4,28 +4,8 @@
 #include <linux/types.h>
 #include <asm/io.h>
 
-#define PCIBIOS_MIN_IO		0
-#define PCIBIOS_MIN_MEM		0
-
-#define pcibios_assign_all_busses() 1
-
-extern int isa_dma_bridge_buggy;
-
-#ifdef CONFIG_PCI
-static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
-{
-	/* no legacy IRQs */
-	return -ENODEV;
-}
-#endif
-
-#ifdef CONFIG_PCI_DOMAINS
-static inline int pci_proc_domain(struct pci_bus *bus)
-{
-	/* always show the domain in /proc */
-	return 1;
-}
-#endif  /* CONFIG_PCI */
+/* Generic PCI */
+#include <asm-generic/pci.h>
 
 #ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
 /*
diff --git a/include/asm-generic/pci.h b/include/asm-generic/pci.h
new file mode 100644
index 000000000000..fbc25741696a
--- /dev/null
+++ b/include/asm-generic/pci.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __ASM_GENERIC_PCI_H
+#define __ASM_GENERIC_PCI_H
+
+#include <linux/types.h>
+
+#ifndef PCIBIOS_MIN_IO
+#define PCIBIOS_MIN_IO		0
+#endif
+
+#ifndef PCIBIOS_MIN_MEM
+#define PCIBIOS_MIN_MEM		0
+#endif
+
+#ifndef pcibios_assign_all_busses
+/* For bootloaders that do not initialize the PCI bus */
+#define pcibios_assign_all_busses() 1
+#endif
+
+extern int isa_dma_bridge_buggy;
+
+/* Enable generic resource mapping code in drivers/pci/ */
+#define ARCH_GENERIC_PCI_MMAP_RESOURCE
+
+#ifdef CONFIG_PCI
+
+static inline int pci_proc_domain(struct pci_bus *bus)
+{
+	/* always show the domain in /proc */
+	return 1;
+}
+
+#endif /* CONFIG_PCI */
+
+#endif /* __ASM_GENERIC_PCI_H */
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/2] asm-generic: Remove pci.h copying remaining code to x86
  2022-07-17  3:34 ` [PATCH v2 1/2] asm-generic: Remove pci.h copying remaining code to x86 Stafford Horne
@ 2022-07-17  9:23   ` Geert Uytterhoeven
  2022-07-18  4:33   ` Christoph Hellwig
  1 sibling, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2022-07-17  9:23 UTC (permalink / raw)
  To: Stafford Horne
  Cc: LKML, Arnd Bergmann, Richard Henderson, Ivan Kokshaysky,
	Matt Turner, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, David S. Miller,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	the arch/x86 maintainers, H. Peter Anvin, Chris Zankel,
	Max Filippov, Bjorn Helgaas, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Nick Child, Niklas Schnelle, Matthew Rosato,
	Pierre Morel, Kees Cook, Gustavo A. R. Silva, alpha, linux-ia64,
	linux-m68k, linuxppc-dev, linux-s390, sparclinux,
	open list:TENSILICA XTENSA PORT (xtensa),
	linux-pci, Linux-Arch, linux-riscv

Hi Stafford,

On Sun, Jul 17, 2022 at 5:35 AM Stafford Horne <shorne@gmail.com> wrote:
> The generic pci.h header now only provides a definition of
> pci_get_legacy_ide_irq which is used by architectures that support PNP.
> Of the architectures that use asm-generic/pci.h this is only x86.
>
> This patch removes the old pci.h in order to make room for a new
> pci.h to be used by arm64, riscv, openrisc, etc.
>
> The existing code in pci.h is moved out to x86.  On other architectures
> we clean up any outstanding references.
>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Link: https://lore.kernel.org/lkml/CAK8P3a0JmPeczfmMBE__vn=Jbvf=nkbpVaZCycyv40pZNCJJXQ@mail.gmail.com/
> Signed-off-by: Stafford Horne <shorne@gmail.com>

Thanks for your patch!

> --- a/arch/m68k/include/asm/pci.h
> +++ b/arch/m68k/include/asm/pci.h
> @@ -2,11 +2,14 @@
>  #ifndef _ASM_M68K_PCI_H
>  #define _ASM_M68K_PCI_H
>
> -#include <asm-generic/pci.h>
> -
>  #define        pcibios_assign_all_busses()     1
>
>  #define        PCIBIOS_MIN_IO          0x00000100
>  #define        PCIBIOS_MIN_MEM         0x02000000
>
> +static inline int pci_get_legacy_ide_irq(struct pci_dev *dev, int channel)
> +{
> +       return channel ? 15 : 14;
> +}
> +

I thought you were not going to add this?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/2] asm-generic: Remove pci.h copying remaining code to x86
  2022-07-17  3:34 ` [PATCH v2 1/2] asm-generic: Remove pci.h copying remaining code to x86 Stafford Horne
  2022-07-17  9:23   ` Geert Uytterhoeven
@ 2022-07-18  4:33   ` Christoph Hellwig
  2022-07-18  8:40     ` Arnd Bergmann
  1 sibling, 1 reply; 20+ messages in thread
From: Christoph Hellwig @ 2022-07-18  4:33 UTC (permalink / raw)
  To: Stafford Horne
  Cc: LKML, Arnd Bergmann, Richard Henderson, Ivan Kokshaysky,
	Matt Turner, Geert Uytterhoeven, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Heiko Carstens,
	Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
	Sven Schnelle, David S. Miller, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Chris Zankel,
	Max Filippov, Bjorn Helgaas, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Nick Child, Niklas Schnelle, Matthew Rosato,
	Pierre Morel, Kees Cook, Gustavo A. R. Silva, linux-alpha,
	linux-ia64, linux-m68k, linuxppc-dev, linux-s390, sparclinux,
	linux-xtensa, linux-pci, linux-arch, linux-riscv

On Sun, Jul 17, 2022 at 12:34:52PM +0900, Stafford Horne wrote:
> The generic pci.h header now only provides a definition of
> pci_get_legacy_ide_irq which is used by architectures that support PNP.
> Of the architectures that use asm-generic/pci.h this is only x86.

Please move this into a separate header, ike legacy-ide.h.  It doens't
have anyting to do with actual PCI support.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/2] asm-generic: Add new pci.h and use it
  2022-07-17  3:34 ` [PATCH v2 2/2] asm-generic: Add new pci.h and use it Stafford Horne
@ 2022-07-18  4:37   ` Christoph Hellwig
  2022-07-18  6:56     ` Arnd Bergmann
  0 siblings, 1 reply; 20+ messages in thread
From: Christoph Hellwig @ 2022-07-18  4:37 UTC (permalink / raw)
  To: Stafford Horne
  Cc: LKML, Arnd Bergmann, Catalin Marinas, Will Deacon, Guo Ren,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Bjorn Helgaas, linux-arm-kernel,
	linux-csky, linux-riscv, linux-um, linux-pci, linux-arch

On Sun, Jul 17, 2022 at 12:34:53PM +0900, Stafford Horne wrote:
> Two things to note are:
> 
>  - isa_dma_bridge_buggy, traditionally this is defined in asm/dma.h but
>    these architectures avoid creating that file and add the definition
>    to asm/pci.h.

This doesn't have anyting to do with PCI support.  I think adding a
separate header just for this that always stubs it out unless a config
option is set (which x86 then selects) is the besy idea here.  I also
think the isa_dma_bridge_buggy needs to move out of the PCI code as
well.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/2] asm-generic: Add new pci.h and use it
  2022-07-18  4:37   ` Christoph Hellwig
@ 2022-07-18  6:56     ` Arnd Bergmann
       [not found]       ` <CAAfxs740yz1vJmtFHOPTXT6fqi0+37SR_OhoGsONe4mx_21+_g@mail.gmail.com>
  0 siblings, 1 reply; 20+ messages in thread
From: Arnd Bergmann @ 2022-07-18  6:56 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Stafford Horne, LKML, Arnd Bergmann, Catalin Marinas,
	Will Deacon, Guo Ren, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Richard Weinberger, Anton Ivanov, Johannes Berg, Bjorn Helgaas,
	Linux ARM, linux-csky, linux-riscv, linux-um, linux-pci,
	linux-arch

On Mon, Jul 18, 2022 at 6:37 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Sun, Jul 17, 2022 at 12:34:53PM +0900, Stafford Horne wrote:
> > Two things to note are:
> >
> >  - isa_dma_bridge_buggy, traditionally this is defined in asm/dma.h but
> >    these architectures avoid creating that file and add the definition
> >    to asm/pci.h.
>
> This doesn't have anyting to do with PCI support.  I think adding a
> separate header just for this that always stubs it out unless a config
> option is set (which x86 then selects) is the besy idea here.  I also
> think the isa_dma_bridge_buggy needs to move out of the PCI code as
> well.

Most architectures have it in asm/dma.h, which is probably the right place
(if we end up keeping it), since this is for the ISA DMA API.

I would copy this declaration from x86

#ifdef CONFIG_PCI
extern int isa_dma_bridge_buggy;
#else
#define isa_dma_bridge_buggy (0)
#endif

to asm-generic/dma.h and remove it from arch/sh to avoid the
one duplicate definition. The architectures that have the declaration
in asm/pci.h (arm64, csky, riscv) already get the asm-generic version
of asm/dma.h.

As mentioned before, it would be even better to just remove it
entirely from everything except x86, and enclose the four
references in an explicit "#ifdef X86_32". The variable declaration
only exists because drivers/pci/quirks.c is compiled on all
architecture, but the individual quirk is only active  based on
the PCI device ID of certain early PCI-ISA bridges.

      Arnd

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/2] asm-generic: Remove pci.h copying remaining code to x86
  2022-07-18  4:33   ` Christoph Hellwig
@ 2022-07-18  8:40     ` Arnd Bergmann
  2022-07-19 10:51       ` Stafford Horne
  0 siblings, 1 reply; 20+ messages in thread
From: Arnd Bergmann @ 2022-07-18  8:40 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Stafford Horne, LKML, Arnd Bergmann, Richard Henderson,
	Ivan Kokshaysky, Matt Turner, Geert Uytterhoeven,
	Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, David S. Miller,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
	the arch/x86 maintainers, H. Peter Anvin, Chris Zankel,
	Max Filippov, Bjorn Helgaas, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Nick Child, Niklas Schnelle, Matthew Rosato,
	Pierre Morel, Kees Cook, Gustavo A. R. Silva, alpha,
	open list:IA64 (Itanium) PLATFORM, linux-m68k, linuxppc-dev,
	linux-s390, sparclinux, open list:TENSILICA XTENSA PORT (xtensa),
	linux-pci, linux-arch, linux-riscv

On Mon, Jul 18, 2022 at 6:33 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Sun, Jul 17, 2022 at 12:34:52PM +0900, Stafford Horne wrote:
> > The generic pci.h header now only provides a definition of
> > pci_get_legacy_ide_irq which is used by architectures that support PNP.
> > Of the architectures that use asm-generic/pci.h this is only x86.
>
> Please move this into a separate header, ike legacy-ide.h.  It doens't
> have anyting to do with actual PCI support.

It looks like asm/libata-portmap.h is meant to have this information already,
and this is what libata uses, while drivers/ide used the
pci_get_legacy_ide_irq()
function for the same purpose.

Only ia64 and powerpc have interesting definitions of both, and they
return the same thing, so I think this is sufficient to remove the last caller:

diff --git a/drivers/pnp/resource.c b/drivers/pnp/resource.c
index 2fa0f7d55259..d7a6250589d6 100644
--- a/drivers/pnp/resource.c
+++ b/drivers/pnp/resource.c
@@ -16,7 +16,7 @@
 #include <asm/io.h>
 #include <asm/dma.h>
 #include <asm/irq.h>
-#include <linux/pci.h>
+#include <linux/libata.h>
 #include <linux/ioport.h>
 #include <linux/init.h>

@@ -322,8 +322,8 @@ static int pci_dev_uses_irq(struct pnp_dev *pnp,
struct pci_dev *pci,
                 * treat the compatibility IRQs as busy.
                 */
                if ((progif & 0x5) != 0x5)
-                       if (pci_get_legacy_ide_irq(pci, 0) == irq ||
-                           pci_get_legacy_ide_irq(pci, 1) == irq) {
+                       if (ATA_PRIMARY_IRQ(pci) == irq ||
+                           ATA_SECONDARY_IRQ(pci) == irq) {
                                pnp_dbg(&pnp->dev, "  legacy IDE device %s "
                                        "using irq %d\n", pci_name(pci), irq);
                                return 1;

This is fine on the architectures that currently return an error from
pci_get_legacy_ide_irq() but will change to returning 15/14 instead,
because they do not support ISA devices, so pci_dev_uses_irq()
will never be called either.

        Arnd

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/2] asm-generic: Add new pci.h and use it
       [not found]       ` <CAAfxs740yz1vJmtFHOPTXT6fqi0+37SR_OhoGsONe4mx_21+_g@mail.gmail.com>
@ 2022-07-19  7:45         ` Arnd Bergmann
  2022-07-19 10:55           ` Stafford Horne
  0 siblings, 1 reply; 20+ messages in thread
From: Arnd Bergmann @ 2022-07-19  7:45 UTC (permalink / raw)
  To: Stafford Horne
  Cc: Arnd Bergmann, Christoph Hellwig, LKML, Catalin Marinas,
	Will Deacon, Guo Ren, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Richard Weinberger, Anton Ivanov, Johannes Berg, Bjorn Helgaas,
	Linux ARM, linux-csky, linux-riscv, linux-um, linux-pci,
	linux-arch

On Tue, Jul 19, 2022 at 1:19 AM Stafford Horne <shorne@gmail.com> wrote:
> On Mon, Jul 18, 2022, 3:56 PM Arnd Bergmann <arnd@arndb.de> wrote:
>>
>> As mentioned before, it would be even better to just remove it
>> entirely from everything except x86, and enclose the four
>> references in an explicit "#ifdef X86_32". The variable declaration
>> only exists because drivers/pci/quirks.c is compiled on all
>> architecture, but the individual quirk is only active  based on
>> the PCI device ID of certain early PCI-ISA bridges.
>
>
> Ok, I was thinking of that route but once I saw the pci device IDs I
> wasn't so sure it was limited to x86.  I'll go ahead with that approach.

Ok, thanks!

I checked all the PCI IDs yesterday, and I'm fairly sure they are x86
specific. While some related products are general-purpose PCI-ISA
bridges that have shown up on mips or arm boards, the ones listed
here should all be safe.

         Arnd

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/2] asm-generic: Remove pci.h copying remaining code to x86
  2022-07-18  8:40     ` Arnd Bergmann
@ 2022-07-19 10:51       ` Stafford Horne
  0 siblings, 0 replies; 20+ messages in thread
From: Stafford Horne @ 2022-07-19 10:51 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Christoph Hellwig, LKML, Richard Henderson, Ivan Kokshaysky,
	Matt Turner, Geert Uytterhoeven, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Heiko Carstens,
	Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
	Sven Schnelle, David S. Miller, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, the arch/x86 maintainers,
	H. Peter Anvin, Chris Zankel, Max Filippov, Bjorn Helgaas,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Nick Child,
	Niklas Schnelle, Matthew Rosato, Pierre Morel, Kees Cook,
	Gustavo A. R. Silva, alpha, open list:IA64 (Itanium) PLATFORM,
	linux-m68k, linuxppc-dev, linux-s390, sparclinux,
	open list:TENSILICA XTENSA PORT (xtensa),
	linux-pci, linux-arch, linux-riscv

On Mon, Jul 18, 2022 at 10:40:34AM +0200, Arnd Bergmann wrote:
> On Mon, Jul 18, 2022 at 6:33 AM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Sun, Jul 17, 2022 at 12:34:52PM +0900, Stafford Horne wrote:
> > > The generic pci.h header now only provides a definition of
> > > pci_get_legacy_ide_irq which is used by architectures that support PNP.
> > > Of the architectures that use asm-generic/pci.h this is only x86.
> >
> > Please move this into a separate header, ike legacy-ide.h.  It doens't
> > have anyting to do with actual PCI support.
> 
> It looks like asm/libata-portmap.h is meant to have this information already,
> and this is what libata uses, while drivers/ide used the
> pci_get_legacy_ide_irq()
> function for the same purpose.
> 
> Only ia64 and powerpc have interesting definitions of both, and they
> return the same thing, so I think this is sufficient to remove the last caller:
> 
> diff --git a/drivers/pnp/resource.c b/drivers/pnp/resource.c
> index 2fa0f7d55259..d7a6250589d6 100644
> --- a/drivers/pnp/resource.c
> +++ b/drivers/pnp/resource.c
> @@ -16,7 +16,7 @@
>  #include <asm/io.h>
>  #include <asm/dma.h>
>  #include <asm/irq.h>
> -#include <linux/pci.h>
> +#include <linux/libata.h>
>  #include <linux/ioport.h>
>  #include <linux/init.h>
> 
> @@ -322,8 +322,8 @@ static int pci_dev_uses_irq(struct pnp_dev *pnp,
> struct pci_dev *pci,
>                  * treat the compatibility IRQs as busy.
>                  */
>                 if ((progif & 0x5) != 0x5)
> -                       if (pci_get_legacy_ide_irq(pci, 0) == irq ||
> -                           pci_get_legacy_ide_irq(pci, 1) == irq) {
> +                       if (ATA_PRIMARY_IRQ(pci) == irq ||
> +                           ATA_SECONDARY_IRQ(pci) == irq) {
>                                 pnp_dbg(&pnp->dev, "  legacy IDE device %s "
>                                         "using irq %d\n", pci_name(pci), irq);
>                                 return 1;
> 
> This is fine on the architectures that currently return an error from
> pci_get_legacy_ide_irq() but will change to returning 15/14 instead,
> because they do not support ISA devices, so pci_dev_uses_irq()
> will never be called either.

I like this, I didn't know about the ATA_PRIMARY_IRQ/ATA_SECONDARY_IRQ macro.
Let me add this to the series before 1/2.  I will keep you as the author via
Signed-off-by annotation.

-Stafford

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/2] asm-generic: Add new pci.h and use it
  2022-07-19  7:45         ` Arnd Bergmann
@ 2022-07-19 10:55           ` Stafford Horne
  2022-07-19 11:55             ` Arnd Bergmann
  0 siblings, 1 reply; 20+ messages in thread
From: Stafford Horne @ 2022-07-19 10:55 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Christoph Hellwig, LKML, Catalin Marinas, Will Deacon, Guo Ren,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Bjorn Helgaas, Linux ARM,
	linux-csky, linux-riscv, linux-um, linux-pci, linux-arch

On Tue, Jul 19, 2022 at 09:45:58AM +0200, Arnd Bergmann wrote:
> On Tue, Jul 19, 2022 at 1:19 AM Stafford Horne <shorne@gmail.com> wrote:
> > On Mon, Jul 18, 2022, 3:56 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >>
> >> As mentioned before, it would be even better to just remove it
> >> entirely from everything except x86, and enclose the four
> >> references in an explicit "#ifdef X86_32". The variable declaration
> >> only exists because drivers/pci/quirks.c is compiled on all
> >> architecture, but the individual quirk is only active  based on
> >> the PCI device ID of certain early PCI-ISA bridges.
> >
> >
> > Ok, I was thinking of that route but once I saw the pci device IDs I
> > wasn't so sure it was limited to x86.  I'll go ahead with that approach.
> 
> Ok, thanks!
> 
> I checked all the PCI IDs yesterday, and I'm fairly sure they are x86
> specific. While some related products are general-purpose PCI-ISA
> bridges that have shown up on mips or arm boards, the ones listed
> here should all be safe.

This is what I have now, I will add a similar patch between 1/2 and 2/2, if this
looks ok.

It adds a few ifdef's which might be controversial but I think it provides more
cleanup than added complexity.  I compile tested with x86_64, x86_32 and
OpenRISC.

diff --git a/arch/alpha/include/asm/dma.h b/arch/alpha/include/asm/dma.h
index 28610ea7786d..a04d76b96089 100644
--- a/arch/alpha/include/asm/dma.h
+++ b/arch/alpha/include/asm/dma.h
@@ -365,13 +365,4 @@ extern void free_dma(unsigned int dmanr);	/* release it again */
 #define KERNEL_HAVE_CHECK_DMA
 extern int check_dma(unsigned int dmanr);
 
-/* From PCI */
-
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy 	(0)
-#endif
-
-
 #endif /* _ASM_DMA_H */
diff --git a/arch/arc/include/asm/dma.h b/arch/arc/include/asm/dma.h
index 5b744f4b10a7..02431027ed2f 100644
--- a/arch/arc/include/asm/dma.h
+++ b/arch/arc/include/asm/dma.h
@@ -7,10 +7,5 @@
 #define ASM_ARC_DMA_H
 
 #define MAX_DMA_ADDRESS 0xC0000000
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy	0
-#endif
 
 #endif
diff --git a/arch/arm/include/asm/dma.h b/arch/arm/include/asm/dma.h
index a81dda65c576..907d139be431 100644
--- a/arch/arm/include/asm/dma.h
+++ b/arch/arm/include/asm/dma.h
@@ -143,10 +143,4 @@ extern int  get_dma_residue(unsigned int chan);
 
 #endif /* CONFIG_ISA_DMA_API */
 
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy    (0)
-#endif
-
 #endif /* __ASM_ARM_DMA_H */
diff --git a/arch/ia64/include/asm/dma.h b/arch/ia64/include/asm/dma.h
index 59625e9c1f9c..eaed2626ffda 100644
--- a/arch/ia64/include/asm/dma.h
+++ b/arch/ia64/include/asm/dma.h
@@ -12,8 +12,6 @@
 
 extern unsigned long MAX_DMA_ADDRESS;
 
-extern int isa_dma_bridge_buggy;
-
 #define free_dma(x)
 
 #endif /* _ASM_IA64_DMA_H */
diff --git a/arch/m68k/include/asm/dma.h b/arch/m68k/include/asm/dma.h
index f6c5e0dfb4e5..1c8d9c5bc2fa 100644
--- a/arch/m68k/include/asm/dma.h
+++ b/arch/m68k/include/asm/dma.h
@@ -6,10 +6,4 @@
    bootmem allocator (but this should do it for this) */
 #define MAX_DMA_ADDRESS PAGE_OFFSET
 
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy    (0)
-#endif
-
 #endif /* _M68K_DMA_H */
diff --git a/arch/microblaze/include/asm/dma.h b/arch/microblaze/include/asm/dma.h
index f801582be912..7484c9eb66c4 100644
--- a/arch/microblaze/include/asm/dma.h
+++ b/arch/microblaze/include/asm/dma.h
@@ -9,10 +9,4 @@
 /* Virtual address corresponding to last available physical memory address.  */
 #define MAX_DMA_ADDRESS (CONFIG_KERNEL_START + memory_size - 1)
 
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy     (0)
-#endif
-
 #endif /* _ASM_MICROBLAZE_DMA_H */
diff --git a/arch/mips/include/asm/dma.h b/arch/mips/include/asm/dma.h
index be726b943530..d6186e6bea7e 100644
--- a/arch/mips/include/asm/dma.h
+++ b/arch/mips/include/asm/dma.h
@@ -307,12 +307,4 @@ static __inline__ int get_dma_residue(unsigned int dmanr)
 extern int request_dma(unsigned int dmanr, const char * device_id);	/* reserve a DMA channel */
 extern void free_dma(unsigned int dmanr);	/* release it again */
 
-/* From PCI */
-
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy	(0)
-#endif
-
 #endif /* _ASM_DMA_H */
diff --git a/arch/parisc/include/asm/dma.h b/arch/parisc/include/asm/dma.h
index eea80ed34e6d..9e8c101de902 100644
--- a/arch/parisc/include/asm/dma.h
+++ b/arch/parisc/include/asm/dma.h
@@ -176,10 +176,4 @@ static __inline__ void set_dma_count(unsigned int dmanr, unsigned int count)
 
 #define free_dma(dmanr)
 
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy 	(0)
-#endif
-
 #endif /* _ASM_DMA_H */
diff --git a/arch/powerpc/include/asm/dma.h b/arch/powerpc/include/asm/dma.h
index 6161a9596196..d97c66d9ae34 100644
--- a/arch/powerpc/include/asm/dma.h
+++ b/arch/powerpc/include/asm/dma.h
@@ -340,11 +340,5 @@ extern int request_dma(unsigned int dmanr, const char *device_id);
 /* release it again */
 extern void free_dma(unsigned int dmanr);
 
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy	(0)
-#endif
-
 #endif /* __KERNEL__ */
 #endif	/* _ASM_POWERPC_DMA_H */
diff --git a/arch/s390/include/asm/dma.h b/arch/s390/include/asm/dma.h
index 6f26f35d4a71..dec1c4ce628c 100644
--- a/arch/s390/include/asm/dma.h
+++ b/arch/s390/include/asm/dma.h
@@ -11,10 +11,4 @@
  */
 #define MAX_DMA_ADDRESS         0x80000000
 
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy	(0)
-#endif
-
 #endif /* _ASM_S390_DMA_H */
diff --git a/arch/sh/include/asm/dma.h b/arch/sh/include/asm/dma.h
index 17d23ae98c77..c8bee3f985a2 100644
--- a/arch/sh/include/asm/dma.h
+++ b/arch/sh/include/asm/dma.h
@@ -137,10 +137,4 @@ extern int register_chan_caps(const char *dmac, struct dma_chan_caps *capslist);
 extern int dma_create_sysfs_files(struct dma_channel *, struct dma_info *);
 extern void dma_remove_sysfs_files(struct dma_channel *, struct dma_info *);
 
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy	(0)
-#endif
-
 #endif /* __ASM_SH_DMA_H */
diff --git a/arch/sparc/include/asm/dma.h b/arch/sparc/include/asm/dma.h
index 462e7c794a09..08043f35b110 100644
--- a/arch/sparc/include/asm/dma.h
+++ b/arch/sparc/include/asm/dma.h
@@ -82,14 +82,6 @@
 #define DMA_BURST64      0x40
 #define DMA_BURSTBITS    0x7f
 
-/* From PCI */
-
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy 	(0)
-#endif
-
 #ifdef CONFIG_SPARC32
 struct device;
 
diff --git a/arch/x86/include/asm/dma.h b/arch/x86/include/asm/dma.h
index 8e95aa4b0d17..0c34c658f57d 100644
--- a/arch/x86/include/asm/dma.h
+++ b/arch/x86/include/asm/dma.h
@@ -309,10 +309,12 @@ extern void free_dma(unsigned int dmanr);
 
 /* From PCI */
 
+#ifdef CONFIG_X86_32
 #ifdef CONFIG_PCI
 extern int isa_dma_bridge_buggy;
 #else
 #define isa_dma_bridge_buggy	(0)
-#endif
+#endif /* CONFIG_PCI */
+#endif /* CONFIG_X86_32 */
 
 #endif /* _ASM_X86_DMA_H */
diff --git a/arch/xtensa/include/asm/dma.h b/arch/xtensa/include/asm/dma.h
index bb099a373b5a..172644539032 100644
--- a/arch/xtensa/include/asm/dma.h
+++ b/arch/xtensa/include/asm/dma.h
@@ -52,11 +52,4 @@
 extern int request_dma(unsigned int dmanr, const char * device_id);
 extern void free_dma(unsigned int dmanr);
 
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy 	(0)
-#endif
-
-
 #endif
diff --git a/drivers/comedi/drivers/comedi_isadma.c b/drivers/comedi/drivers/comedi_isadma.c
index 700982464c53..508421809128 100644
--- a/drivers/comedi/drivers/comedi_isadma.c
+++ b/drivers/comedi/drivers/comedi_isadma.c
@@ -104,8 +104,10 @@ unsigned int comedi_isadma_poll(struct comedi_isadma *dma)
 
 	flags = claim_dma_lock();
 	clear_dma_ff(desc->chan);
+#ifdef CONFIG_X86_32
 	if (!isa_dma_bridge_buggy)
 		disable_dma(desc->chan);
+#endif
 	result = get_dma_residue(desc->chan);
 	/*
 	 * Read the counter again and choose higher value in order to
@@ -113,8 +115,10 @@ unsigned int comedi_isadma_poll(struct comedi_isadma *dma)
 	 * isa_dma_bridge_buggy is set.
 	 */
 	result1 = get_dma_residue(desc->chan);
+#ifdef CONFIG_X86_32
 	if (!isa_dma_bridge_buggy)
 		enable_dma(desc->chan);
+#endif
 	release_dma_lock(flags);
 
 	if (result < result1)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index cfaf40a540a8..60c55d2cb2cc 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -41,8 +41,10 @@ const char *pci_power_names[] = {
 };
 EXPORT_SYMBOL_GPL(pci_power_names);
 
+#ifdef CONFIG_X86_32
 int isa_dma_bridge_buggy;
 EXPORT_SYMBOL(isa_dma_bridge_buggy);
+#endif
 
 int pci_pci_problems;
 EXPORT_SYMBOL(pci_pci_problems);
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 41aeaa235132..cb7715a0f339 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -239,6 +239,7 @@ static void quirk_passive_release(struct pci_dev *dev)
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_82441,	quirk_passive_release);
 DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_82441,	quirk_passive_release);
 
+#ifdef CONFIG_X86_32
 /*
  * The VIA VP2/VP3/MVP3 seem to have some 'features'. There may be a
  * workaround but VIA don't answer queries. If you happen to have good
@@ -265,6 +266,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AL,	PCI_DEVICE_ID_AL_M1533,		quirk_isa_dma
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NEC,	PCI_DEVICE_ID_NEC_CBUS_1,	quirk_isa_dma_hangs);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NEC,	PCI_DEVICE_ID_NEC_CBUS_2,	quirk_isa_dma_hangs);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NEC,	PCI_DEVICE_ID_NEC_CBUS_3,	quirk_isa_dma_hangs);
+#endif
 
 /*
  * Intel NM10 "TigerPoint" LPC PM1a_STS.BM_STS must be clear
diff --git a/include/asm-generic/pci.h b/include/asm-generic/pci.h
index 556147983911..3ceb0cb12321 100644
--- a/include/asm-generic/pci.h
+++ b/include/asm-generic/pci.h
@@ -18,8 +18,6 @@
 #define pcibios_assign_all_busses() 1
 #endif
 
-extern int isa_dma_bridge_buggy;
-
 /* Enable generic resource mapping code in drivers/pci/ */
 #define ARCH_GENERIC_PCI_MMAP_RESOURCE
 
diff --git a/sound/core/isadma.c b/sound/core/isadma.c
index 1f45ede023b4..8a6397109c56 100644
--- a/sound/core/isadma.c
+++ b/sound/core/isadma.c
@@ -73,8 +73,10 @@ unsigned int snd_dma_pointer(unsigned long dma, unsigned int size)
 
 	flags = claim_dma_lock();
 	clear_dma_ff(dma);
+#ifdef CONFIG_X86_32
 	if (!isa_dma_bridge_buggy)
 		disable_dma(dma);
+#endif
 	result = get_dma_residue(dma);
 	/*
 	 * HACK - read the counter again and choose higher value in order to
@@ -82,8 +84,10 @@ unsigned int snd_dma_pointer(unsigned long dma, unsigned int size)
 	 * isa_dma_bridge_buggy is set.
 	 */
 	result1 = get_dma_residue(dma);
+#ifdef CONFIG_X86_32
 	if (!isa_dma_bridge_buggy)
 		enable_dma(dma);
+#endif
 	release_dma_lock(flags);
 	if (unlikely(result < result1))
 		result = result1;

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/2] asm-generic: Add new pci.h and use it
  2022-07-19 10:55           ` Stafford Horne
@ 2022-07-19 11:55             ` Arnd Bergmann
  2022-07-19 12:23               ` Stafford Horne
  0 siblings, 1 reply; 20+ messages in thread
From: Arnd Bergmann @ 2022-07-19 11:55 UTC (permalink / raw)
  To: Stafford Horne
  Cc: Arnd Bergmann, Christoph Hellwig, LKML, Catalin Marinas,
	Will Deacon, Guo Ren, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Richard Weinberger, Anton Ivanov, Johannes Berg, Bjorn Helgaas,
	Linux ARM, linux-csky, linux-riscv, linux-um, linux-pci,
	linux-arch

On Tue, Jul 19, 2022 at 12:55 PM Stafford Horne <shorne@gmail.com> wrote:

> diff --git a/drivers/comedi/drivers/comedi_isadma.c b/drivers/comedi/drivers/comedi_isadma.c
> index 700982464c53..508421809128 100644
> --- a/drivers/comedi/drivers/comedi_isadma.c
> +++ b/drivers/comedi/drivers/comedi_isadma.c
> @@ -104,8 +104,10 @@ unsigned int comedi_isadma_poll(struct comedi_isadma *dma)
>
>         flags = claim_dma_lock();
>         clear_dma_ff(desc->chan);
> +#ifdef CONFIG_X86_32
>         if (!isa_dma_bridge_buggy)
>                 disable_dma(desc->chan);
> +#endif

There is a logic mistake here: if we are on something other than x86-32,
this always needs to call the disable_dma()/enable_dma().

Not sure how to best express this in a readable way, something like this
would work:

#ifdef CONFIG_X86_32
        if (!isa_dma_bridge_buggy)
#endif
               disable_dma(desc->chan);


or possibly at the start of this file, a

#ifndef CONFIG_X86_32
#define isa_dma_bridge_buggy 0
#endif

Or we could try to keep the generic definition in a global header
like linux/isa-dma.h.

> --- a/sound/core/isadma.c
> +++ b/sound/core/isadma.c
> @@ -73,8 +73,10 @@ unsigned int snd_dma_pointer(unsigned long dma, unsigned int size)
>
>         flags = claim_dma_lock();
>         clear_dma_ff(dma);
> +#ifdef CONFIG_X86_32
>         if (!isa_dma_bridge_buggy)
>                 disable_dma(dma);
> +#endif
>         result = get_dma_residue(dma);
>         /*

Same here.

         Arnd

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/2] asm-generic: Add new pci.h and use it
  2022-07-19 11:55             ` Arnd Bergmann
@ 2022-07-19 12:23               ` Stafford Horne
  2022-07-19 13:05                 ` Stafford Horne
  2022-07-19 15:09                 ` David Laight
  0 siblings, 2 replies; 20+ messages in thread
From: Stafford Horne @ 2022-07-19 12:23 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Christoph Hellwig, LKML, Catalin Marinas, Will Deacon, Guo Ren,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Bjorn Helgaas, Linux ARM,
	linux-csky, linux-riscv, linux-um, linux-pci, linux-arch

On Tue, Jul 19, 2022 at 01:55:03PM +0200, Arnd Bergmann wrote:
> On Tue, Jul 19, 2022 at 12:55 PM Stafford Horne <shorne@gmail.com> wrote:
> 
> > diff --git a/drivers/comedi/drivers/comedi_isadma.c b/drivers/comedi/drivers/comedi_isadma.c
> > index 700982464c53..508421809128 100644
> > --- a/drivers/comedi/drivers/comedi_isadma.c
> > +++ b/drivers/comedi/drivers/comedi_isadma.c
> > @@ -104,8 +104,10 @@ unsigned int comedi_isadma_poll(struct comedi_isadma *dma)
> >
> >         flags = claim_dma_lock();
> >         clear_dma_ff(desc->chan);
> > +#ifdef CONFIG_X86_32
> >         if (!isa_dma_bridge_buggy)
> >                 disable_dma(desc->chan);
> > +#endif
> 
> There is a logic mistake here: if we are on something other than x86-32,
> this always needs to call the disable_dma()/enable_dma().

Oops, thats right.  Sorry, I should have noticed that.

> Not sure how to best express this in a readable way, something like this
> would work:

Option 1:

> #ifdef CONFIG_X86_32
>         if (!isa_dma_bridge_buggy)
> #endif
>                disable_dma(desc->chan);
> 
> 
> or possibly at the start of this file, a

Option 2:

> #ifndef CONFIG_X86_32
> #define isa_dma_bridge_buggy 0
> #endif

Option 3:

> Or we could try to keep the generic definition in a global header
> like linux/isa-dma.h.

Perhaps option 3 makes the whole patch the most clean.

-Stafford

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/2] asm-generic: Add new pci.h and use it
  2022-07-19 12:23               ` Stafford Horne
@ 2022-07-19 13:05                 ` Stafford Horne
  2022-07-19 13:18                   ` Arnd Bergmann
  2022-07-19 14:32                   ` Christoph Hellwig
  2022-07-19 15:09                 ` David Laight
  1 sibling, 2 replies; 20+ messages in thread
From: Stafford Horne @ 2022-07-19 13:05 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Christoph Hellwig, LKML, Catalin Marinas, Will Deacon, Guo Ren,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Bjorn Helgaas, Linux ARM,
	linux-csky, linux-riscv, linux-um, linux-pci, linux-arch

On Tue, Jul 19, 2022 at 09:23:36PM +0900, Stafford Horne wrote:
> On Tue, Jul 19, 2022 at 01:55:03PM +0200, Arnd Bergmann wrote:
> > On Tue, Jul 19, 2022 at 12:55 PM Stafford Horne <shorne@gmail.com> wrote:
> > 
> > > diff --git a/drivers/comedi/drivers/comedi_isadma.c b/drivers/comedi/drivers/comedi_isadma.c
> > > index 700982464c53..508421809128 100644
> > > --- a/drivers/comedi/drivers/comedi_isadma.c
> > > +++ b/drivers/comedi/drivers/comedi_isadma.c
> > > @@ -104,8 +104,10 @@ unsigned int comedi_isadma_poll(struct comedi_isadma *dma)
> > >
> > >         flags = claim_dma_lock();
> > >         clear_dma_ff(desc->chan);
> > > +#ifdef CONFIG_X86_32
> > >         if (!isa_dma_bridge_buggy)
> > >                 disable_dma(desc->chan);
> > > +#endif
> > 
> > There is a logic mistake here: if we are on something other than x86-32,
> > this always needs to call the disable_dma()/enable_dma().
> 
> Oops, thats right.  Sorry, I should have noticed that.
> 
> > Not sure how to best express this in a readable way, something like this
> > would work:
> 
> Option 1:
> 
> > #ifdef CONFIG_X86_32
> >         if (!isa_dma_bridge_buggy)
> > #endif
> >                disable_dma(desc->chan);
> > 
> > 
> > or possibly at the start of this file, a
> 
> Option 2:
> 
> > #ifndef CONFIG_X86_32
> > #define isa_dma_bridge_buggy 0
> > #endif
> 
> Option 3:
> 
> > Or we could try to keep the generic definition in a global header
> > like linux/isa-dma.h.
> 
> Perhaps option 3 makes the whole patch the most clean.

And this is the result, I will get this into the series and create a v4 tomorrow
if no issues.

-Stafford

--

diff --git a/arch/alpha/include/asm/dma.h b/arch/alpha/include/asm/dma.h
index 28610ea7786d..a04d76b96089 100644
--- a/arch/alpha/include/asm/dma.h
+++ b/arch/alpha/include/asm/dma.h
@@ -365,13 +365,4 @@ extern void free_dma(unsigned int dmanr);	/* release it again */
 #define KERNEL_HAVE_CHECK_DMA
 extern int check_dma(unsigned int dmanr);
 
-/* From PCI */
-
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy 	(0)
-#endif
-
-
 #endif /* _ASM_DMA_H */
diff --git a/arch/arc/include/asm/dma.h b/arch/arc/include/asm/dma.h
index 5b744f4b10a7..02431027ed2f 100644
--- a/arch/arc/include/asm/dma.h
+++ b/arch/arc/include/asm/dma.h
@@ -7,10 +7,5 @@
 #define ASM_ARC_DMA_H
 
 #define MAX_DMA_ADDRESS 0xC0000000
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy	0
-#endif
 
 #endif
diff --git a/arch/arm/include/asm/dma.h b/arch/arm/include/asm/dma.h
index a81dda65c576..907d139be431 100644
--- a/arch/arm/include/asm/dma.h
+++ b/arch/arm/include/asm/dma.h
@@ -143,10 +143,4 @@ extern int  get_dma_residue(unsigned int chan);
 
 #endif /* CONFIG_ISA_DMA_API */
 
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy    (0)
-#endif
-
 #endif /* __ASM_ARM_DMA_H */
diff --git a/arch/ia64/include/asm/dma.h b/arch/ia64/include/asm/dma.h
index 59625e9c1f9c..eaed2626ffda 100644
--- a/arch/ia64/include/asm/dma.h
+++ b/arch/ia64/include/asm/dma.h
@@ -12,8 +12,6 @@
 
 extern unsigned long MAX_DMA_ADDRESS;
 
-extern int isa_dma_bridge_buggy;
-
 #define free_dma(x)
 
 #endif /* _ASM_IA64_DMA_H */
diff --git a/arch/m68k/include/asm/dma.h b/arch/m68k/include/asm/dma.h
index f6c5e0dfb4e5..1c8d9c5bc2fa 100644
--- a/arch/m68k/include/asm/dma.h
+++ b/arch/m68k/include/asm/dma.h
@@ -6,10 +6,4 @@
    bootmem allocator (but this should do it for this) */
 #define MAX_DMA_ADDRESS PAGE_OFFSET
 
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy    (0)
-#endif
-
 #endif /* _M68K_DMA_H */
diff --git a/arch/microblaze/include/asm/dma.h b/arch/microblaze/include/asm/dma.h
index f801582be912..7484c9eb66c4 100644
--- a/arch/microblaze/include/asm/dma.h
+++ b/arch/microblaze/include/asm/dma.h
@@ -9,10 +9,4 @@
 /* Virtual address corresponding to last available physical memory address.  */
 #define MAX_DMA_ADDRESS (CONFIG_KERNEL_START + memory_size - 1)
 
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy     (0)
-#endif
-
 #endif /* _ASM_MICROBLAZE_DMA_H */
diff --git a/arch/mips/include/asm/dma.h b/arch/mips/include/asm/dma.h
index be726b943530..d6186e6bea7e 100644
--- a/arch/mips/include/asm/dma.h
+++ b/arch/mips/include/asm/dma.h
@@ -307,12 +307,4 @@ static __inline__ int get_dma_residue(unsigned int dmanr)
 extern int request_dma(unsigned int dmanr, const char * device_id);	/* reserve a DMA channel */
 extern void free_dma(unsigned int dmanr);	/* release it again */
 
-/* From PCI */
-
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy	(0)
-#endif
-
 #endif /* _ASM_DMA_H */
diff --git a/arch/parisc/include/asm/dma.h b/arch/parisc/include/asm/dma.h
index eea80ed34e6d..9e8c101de902 100644
--- a/arch/parisc/include/asm/dma.h
+++ b/arch/parisc/include/asm/dma.h
@@ -176,10 +176,4 @@ static __inline__ void set_dma_count(unsigned int dmanr, unsigned int count)
 
 #define free_dma(dmanr)
 
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy 	(0)
-#endif
-
 #endif /* _ASM_DMA_H */
diff --git a/arch/powerpc/include/asm/dma.h b/arch/powerpc/include/asm/dma.h
index 6161a9596196..d97c66d9ae34 100644
--- a/arch/powerpc/include/asm/dma.h
+++ b/arch/powerpc/include/asm/dma.h
@@ -340,11 +340,5 @@ extern int request_dma(unsigned int dmanr, const char *device_id);
 /* release it again */
 extern void free_dma(unsigned int dmanr);
 
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy	(0)
-#endif
-
 #endif /* __KERNEL__ */
 #endif	/* _ASM_POWERPC_DMA_H */
diff --git a/arch/s390/include/asm/dma.h b/arch/s390/include/asm/dma.h
index 6f26f35d4a71..dec1c4ce628c 100644
--- a/arch/s390/include/asm/dma.h
+++ b/arch/s390/include/asm/dma.h
@@ -11,10 +11,4 @@
  */
 #define MAX_DMA_ADDRESS         0x80000000
 
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy	(0)
-#endif
-
 #endif /* _ASM_S390_DMA_H */
diff --git a/arch/sh/include/asm/dma.h b/arch/sh/include/asm/dma.h
index 17d23ae98c77..c8bee3f985a2 100644
--- a/arch/sh/include/asm/dma.h
+++ b/arch/sh/include/asm/dma.h
@@ -137,10 +137,4 @@ extern int register_chan_caps(const char *dmac, struct dma_chan_caps *capslist);
 extern int dma_create_sysfs_files(struct dma_channel *, struct dma_info *);
 extern void dma_remove_sysfs_files(struct dma_channel *, struct dma_info *);
 
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy	(0)
-#endif
-
 #endif /* __ASM_SH_DMA_H */
diff --git a/arch/sparc/include/asm/dma.h b/arch/sparc/include/asm/dma.h
index 462e7c794a09..08043f35b110 100644
--- a/arch/sparc/include/asm/dma.h
+++ b/arch/sparc/include/asm/dma.h
@@ -82,14 +82,6 @@
 #define DMA_BURST64      0x40
 #define DMA_BURSTBITS    0x7f
 
-/* From PCI */
-
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy 	(0)
-#endif
-
 #ifdef CONFIG_SPARC32
 struct device;
 
diff --git a/arch/x86/include/asm/dma.h b/arch/x86/include/asm/dma.h
index 8e95aa4b0d17..8ae6e0e11b8b 100644
--- a/arch/x86/include/asm/dma.h
+++ b/arch/x86/include/asm/dma.h
@@ -307,12 +307,4 @@ extern int request_dma(unsigned int dmanr, const char *device_id);
 extern void free_dma(unsigned int dmanr);
 #endif
 
-/* From PCI */
-
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy	(0)
-#endif
-
 #endif /* _ASM_X86_DMA_H */
diff --git a/arch/xtensa/include/asm/dma.h b/arch/xtensa/include/asm/dma.h
index bb099a373b5a..172644539032 100644
--- a/arch/xtensa/include/asm/dma.h
+++ b/arch/xtensa/include/asm/dma.h
@@ -52,11 +52,4 @@
 extern int request_dma(unsigned int dmanr, const char * device_id);
 extern void free_dma(unsigned int dmanr);
 
-#ifdef CONFIG_PCI
-extern int isa_dma_bridge_buggy;
-#else
-#define isa_dma_bridge_buggy 	(0)
-#endif
-
-
 #endif
diff --git a/drivers/comedi/drivers/comedi_isadma.c b/drivers/comedi/drivers/comedi_isadma.c
index 700982464c53..1d470b0da34c 100644
--- a/drivers/comedi/drivers/comedi_isadma.c
+++ b/drivers/comedi/drivers/comedi_isadma.c
@@ -8,6 +8,7 @@
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
+#include <linux/isa-dma.h>
 #include <asm/dma.h>
 #include <linux/comedi/comedidev.h>
 #include <linux/comedi/comedi_isadma.h>
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index cfaf40a540a8..60c55d2cb2cc 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -41,8 +41,10 @@ const char *pci_power_names[] = {
 };
 EXPORT_SYMBOL_GPL(pci_power_names);
 
+#ifdef CONFIG_X86_32
 int isa_dma_bridge_buggy;
 EXPORT_SYMBOL(isa_dma_bridge_buggy);
+#endif
 
 int pci_pci_problems;
 EXPORT_SYMBOL(pci_pci_problems);
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 41aeaa235132..6fc64509eee7 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -17,6 +17,7 @@
 #include <linux/kernel.h>
 #include <linux/export.h>
 #include <linux/pci.h>
+#include <linux/isa-dma.h> /* isa_dma_bridge_buggy */
 #include <linux/init.h>
 #include <linux/delay.h>
 #include <linux/acpi.h>
@@ -30,7 +31,6 @@
 #include <linux/pm_runtime.h>
 #include <linux/suspend.h>
 #include <linux/switchtec.h>
-#include <asm/dma.h>	/* isa_dma_bridge_buggy */
 #include "pci.h"
 
 static ktime_t fixup_debug_start(struct pci_dev *dev,
@@ -239,6 +239,7 @@ static void quirk_passive_release(struct pci_dev *dev)
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_82441,	quirk_passive_release);
 DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_82441,	quirk_passive_release);
 
+#ifdef CONFIG_X86_32
 /*
  * The VIA VP2/VP3/MVP3 seem to have some 'features'. There may be a
  * workaround but VIA don't answer queries. If you happen to have good
@@ -265,6 +266,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AL,	PCI_DEVICE_ID_AL_M1533,		quirk_isa_dma
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NEC,	PCI_DEVICE_ID_NEC_CBUS_1,	quirk_isa_dma_hangs);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NEC,	PCI_DEVICE_ID_NEC_CBUS_2,	quirk_isa_dma_hangs);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NEC,	PCI_DEVICE_ID_NEC_CBUS_3,	quirk_isa_dma_hangs);
+#endif
 
 /*
  * Intel NM10 "TigerPoint" LPC PM1a_STS.BM_STS must be clear
diff --git a/include/asm-generic/pci.h b/include/asm-generic/pci.h
index 556147983911..3ceb0cb12321 100644
--- a/include/asm-generic/pci.h
+++ b/include/asm-generic/pci.h
@@ -18,8 +18,6 @@
 #define pcibios_assign_all_busses() 1
 #endif
 
-extern int isa_dma_bridge_buggy;
-
 /* Enable generic resource mapping code in drivers/pci/ */
 #define ARCH_GENERIC_PCI_MMAP_RESOURCE
 
diff --git a/include/linux/isa-dma.h b/include/linux/isa-dma.h
new file mode 100644
index 000000000000..9514f0949fa1
--- /dev/null
+++ b/include/linux/isa-dma.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __LINUX_ISA_DMA_H
+#define __LINUX_ISA_DMA_H
+
+#if defined(CONFIG_PCI) && defined(CONFIG_X86_32)
+extern int isa_dma_bridge_buggy;
+#else
+#define isa_dma_bridge_buggy	(0)
+#endif
+
+#endif /* __LINUX_ISA_DMA_H */
diff --git a/sound/core/isadma.c b/sound/core/isadma.c
index 1f45ede023b4..9516cfb3d237 100644
--- a/sound/core/isadma.c
+++ b/sound/core/isadma.c
@@ -12,6 +12,7 @@
 #undef HAVE_REALLY_SLOW_DMA_CONTROLLER
 
 #include <linux/export.h>
+#include <linux/isa-dma.h>
 #include <sound/core.h>
 #include <asm/dma.h>
 

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/2] asm-generic: Add new pci.h and use it
  2022-07-19 13:05                 ` Stafford Horne
@ 2022-07-19 13:18                   ` Arnd Bergmann
  2022-07-19 13:33                     ` Stafford Horne
  2022-07-19 14:32                   ` Christoph Hellwig
  1 sibling, 1 reply; 20+ messages in thread
From: Arnd Bergmann @ 2022-07-19 13:18 UTC (permalink / raw)
  To: Stafford Horne
  Cc: Arnd Bergmann, Christoph Hellwig, LKML, Catalin Marinas,
	Will Deacon, Guo Ren, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Richard Weinberger, Anton Ivanov, Johannes Berg, Bjorn Helgaas,
	Linux ARM, linux-csky, linux-riscv, linux-um, linux-pci,
	linux-arch

On Tue, Jul 19, 2022 at 3:05 PM Stafford Horne <shorne@gmail.com> wrote:
> On Tue, Jul 19, 2022 at 09:23:36PM +0900, Stafford Horne wrote:
> > On Tue, Jul 19, 2022 at 01:55:03PM +0200, Arnd Bergmann wrote:

>
> And this is the result, I will get this into the series and create a v4 tomorrow
> if no issues.

Looks good to me, just one detail:

> diff --git a/include/linux/isa-dma.h b/include/linux/isa-dma.h
> new file mode 100644
> index 000000000000..9514f0949fa1
> --- /dev/null
> +++ b/include/linux/isa-dma.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __LINUX_ISA_DMA_H
> +#define __LINUX_ISA_DMA_H
> +
> +#if defined(CONFIG_PCI) && defined(CONFIG_X86_32)
> +extern int isa_dma_bridge_buggy;
> +#else
> +#define isa_dma_bridge_buggy   (0)
> +#endif
> +
> +#endif /* __LINUX_ISA_DMA_H */

I would make this file #include <asm/dma.h> as a step towards making
linux/isa-dma.h the official replacement for it in the driver api.

Including asm/dma.h from a driver is already a bit awkward, since we
are generally moving towards including only linux/*.h type headers, and
the dma.h name is too generic for something that is completely obsolete.

        Arnd

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/2] asm-generic: Add new pci.h and use it
  2022-07-19 13:18                   ` Arnd Bergmann
@ 2022-07-19 13:33                     ` Stafford Horne
  0 siblings, 0 replies; 20+ messages in thread
From: Stafford Horne @ 2022-07-19 13:33 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Christoph Hellwig, LKML, Catalin Marinas, Will Deacon, Guo Ren,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Bjorn Helgaas, Linux ARM,
	linux-csky, linux-riscv, linux-um, linux-pci, linux-arch

On Tue, Jul 19, 2022 at 03:18:17PM +0200, Arnd Bergmann wrote:
> On Tue, Jul 19, 2022 at 3:05 PM Stafford Horne <shorne@gmail.com> wrote:
> > On Tue, Jul 19, 2022 at 09:23:36PM +0900, Stafford Horne wrote:
> > > On Tue, Jul 19, 2022 at 01:55:03PM +0200, Arnd Bergmann wrote:
> 
> >
> > And this is the result, I will get this into the series and create a v4 tomorrow
> > if no issues.
> 
> Looks good to me, just one detail:
> 
> > diff --git a/include/linux/isa-dma.h b/include/linux/isa-dma.h
> > new file mode 100644
> > index 000000000000..9514f0949fa1
> > --- /dev/null
> > +++ b/include/linux/isa-dma.h
> > @@ -0,0 +1,12 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#ifndef __LINUX_ISA_DMA_H
> > +#define __LINUX_ISA_DMA_H
> > +
> > +#if defined(CONFIG_PCI) && defined(CONFIG_X86_32)
> > +extern int isa_dma_bridge_buggy;
> > +#else
> > +#define isa_dma_bridge_buggy   (0)
> > +#endif
> > +
> > +#endif /* __LINUX_ISA_DMA_H */
> 
> I would make this file #include <asm/dma.h> as a step towards making
> linux/isa-dma.h the official replacement for it in the driver api.
> 
> Including asm/dma.h from a driver is already a bit awkward, since we
> are generally moving towards including only linux/*.h type headers, and
> the dma.h name is too generic for something that is completely obsolete.

OK, that makes sense.  Then I can remove the asm/dma.h include from:
 - drivers/comedi/drivers/comedi_isadma.c
 - sound/core/isadma.c

-Stafford

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/2] asm-generic: Add new pci.h and use it
  2022-07-19 13:05                 ` Stafford Horne
  2022-07-19 13:18                   ` Arnd Bergmann
@ 2022-07-19 14:32                   ` Christoph Hellwig
  2022-07-20 13:21                     ` Stafford Horne
  1 sibling, 1 reply; 20+ messages in thread
From: Christoph Hellwig @ 2022-07-19 14:32 UTC (permalink / raw)
  To: Stafford Horne
  Cc: Arnd Bergmann, Christoph Hellwig, LKML, Catalin Marinas,
	Will Deacon, Guo Ren, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Richard Weinberger, Anton Ivanov, Johannes Berg, Bjorn Helgaas,
	Linux ARM, linux-csky, linux-riscv, linux-um, linux-pci,
	linux-arch

On Tue, Jul 19, 2022 at 10:05:46PM +0900, Stafford Horne wrote:
> > > Or we could try to keep the generic definition in a global header
> > > like linux/isa-dma.h.
> > 
> > Perhaps option 3 makes the whole patch the most clean.
> 
> And this is the result, I will get this into the series and create a v4 tomorrow
> if no issues.

Yes, this is what I tried to suggest earlier and it looks fine to me.
If we want to overengineer it we could add a ISA_DMA_BRIDGE_BUGGY
Kconfig symbol and select it from x86.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH v2 2/2] asm-generic: Add new pci.h and use it
  2022-07-19 12:23               ` Stafford Horne
  2022-07-19 13:05                 ` Stafford Horne
@ 2022-07-19 15:09                 ` David Laight
  2022-07-20 13:24                   ` Stafford Horne
  1 sibling, 1 reply; 20+ messages in thread
From: David Laight @ 2022-07-19 15:09 UTC (permalink / raw)
  To: 'Stafford Horne', Arnd Bergmann
  Cc: Christoph Hellwig, LKML, Catalin Marinas, Will Deacon, Guo Ren,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Bjorn Helgaas, Linux ARM,
	linux-csky, linux-riscv, linux-um, linux-pci, linux-arch

From: Stafford Horne
> Sent: 19 July 2022 13:24
> 
> On Tue, Jul 19, 2022 at 01:55:03PM +0200, Arnd Bergmann wrote:
> > On Tue, Jul 19, 2022 at 12:55 PM Stafford Horne <shorne@gmail.com> wrote:
> >
> > > diff --git a/drivers/comedi/drivers/comedi_isadma.c b/drivers/comedi/drivers/comedi_isadma.c
> > > index 700982464c53..508421809128 100644
> > > --- a/drivers/comedi/drivers/comedi_isadma.c
> > > +++ b/drivers/comedi/drivers/comedi_isadma.c
> > > @@ -104,8 +104,10 @@ unsigned int comedi_isadma_poll(struct comedi_isadma *dma)
> > >
> > >         flags = claim_dma_lock();
> > >         clear_dma_ff(desc->chan);
> > > +#ifdef CONFIG_X86_32
> > >         if (!isa_dma_bridge_buggy)
> > >                 disable_dma(desc->chan);
> > > +#endif
> >
> > There is a logic mistake here: if we are on something other than x86-32,
> > this always needs to call the disable_dma()/enable_dma().
> 
> Oops, thats right.  Sorry, I should have noticed that.
> 
> > Not sure how to best express this in a readable way, something like this
> > would work:
> 
> Option 1:
> 
> > #ifdef CONFIG_X86_32
> >         if (!isa_dma_bridge_buggy)
> > #endif
> >                disable_dma(desc->chan);
> >
> >
> > or possibly at the start of this file, a
> 
> Option 2:
> 
> > #ifndef CONFIG_X86_32
> > #define isa_dma_bridge_buggy 0
> > #endif
> 
> Option 3:
> 
> > Or we could try to keep the generic definition in a global header
> > like linux/isa-dma.h.
> 
> Perhaps option 3 makes the whole patch the most clean.

Isn't there a define that can be used inside an if?
So you could do:
		if (!IS_CONFIG_X86_32 || !isa_dma_bridge_buggy)
			disable_dma();
(but I can't remember the name!)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/2] asm-generic: Add new pci.h and use it
  2022-07-19 14:32                   ` Christoph Hellwig
@ 2022-07-20 13:21                     ` Stafford Horne
  0 siblings, 0 replies; 20+ messages in thread
From: Stafford Horne @ 2022-07-20 13:21 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Arnd Bergmann, LKML, Catalin Marinas, Will Deacon, Guo Ren,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Richard Weinberger,
	Anton Ivanov, Johannes Berg, Bjorn Helgaas, Linux ARM,
	linux-csky, linux-riscv, linux-um, linux-pci, linux-arch

On Tue, Jul 19, 2022 at 07:32:46AM -0700, Christoph Hellwig wrote:
> On Tue, Jul 19, 2022 at 10:05:46PM +0900, Stafford Horne wrote:
> > > > Or we could try to keep the generic definition in a global header
> > > > like linux/isa-dma.h.
> > > 
> > > Perhaps option 3 makes the whole patch the most clean.
> > 
> > And this is the result, I will get this into the series and create a v4 tomorrow
> > if no issues.
> 
> Yes, this is what I tried to suggest earlier and it looks fine to me.
> If we want to overengineer it we could add a ISA_DMA_BRIDGE_BUGGY
> Kconfig symbol and select it from x86.

I left it as X86_32, I feel it it would be more confusing/hard to maintain to
add the extra level of indirection.

-Stafford

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/2] asm-generic: Add new pci.h and use it
  2022-07-19 15:09                 ` David Laight
@ 2022-07-20 13:24                   ` Stafford Horne
  0 siblings, 0 replies; 20+ messages in thread
From: Stafford Horne @ 2022-07-20 13:24 UTC (permalink / raw)
  To: David Laight
  Cc: Arnd Bergmann, Christoph Hellwig, LKML, Catalin Marinas,
	Will Deacon, Guo Ren, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Richard Weinberger, Anton Ivanov, Johannes Berg, Bjorn Helgaas,
	Linux ARM, linux-csky, linux-riscv, linux-um, linux-pci,
	linux-arch

On Tue, Jul 19, 2022 at 03:09:35PM +0000, David Laight wrote:
> From: Stafford Horne
> > Sent: 19 July 2022 13:24
> > 
> > On Tue, Jul 19, 2022 at 01:55:03PM +0200, Arnd Bergmann wrote:
> > > On Tue, Jul 19, 2022 at 12:55 PM Stafford Horne <shorne@gmail.com> wrote:

> > Option 3:
> > 
> > > Or we could try to keep the generic definition in a global header
> > > like linux/isa-dma.h.
> > 
> > Perhaps option 3 makes the whole patch the most clean.
> 
> Isn't there a define that can be used inside an if?
> So you could do:
> 		if (!IS_CONFIG_X86_32 || !isa_dma_bridge_buggy)
> 			disable_dma();
> (but I can't remember the name!)

I think you probably mean:

  if (IS_ENABLED(CONFIG_X86_32) ... )

That could work too, but we still want to move the definition of
isa_dma_bridge_buggy out of architectures and into the global header.  I have
gone with option 3 for now.

-Stafford

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2022-07-20 13:24 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-17  3:34 [PATCH v2 0/2] Updates for asm-generic/pci.h Stafford Horne
2022-07-17  3:34 ` [PATCH v2 1/2] asm-generic: Remove pci.h copying remaining code to x86 Stafford Horne
2022-07-17  9:23   ` Geert Uytterhoeven
2022-07-18  4:33   ` Christoph Hellwig
2022-07-18  8:40     ` Arnd Bergmann
2022-07-19 10:51       ` Stafford Horne
2022-07-17  3:34 ` [PATCH v2 2/2] asm-generic: Add new pci.h and use it Stafford Horne
2022-07-18  4:37   ` Christoph Hellwig
2022-07-18  6:56     ` Arnd Bergmann
     [not found]       ` <CAAfxs740yz1vJmtFHOPTXT6fqi0+37SR_OhoGsONe4mx_21+_g@mail.gmail.com>
2022-07-19  7:45         ` Arnd Bergmann
2022-07-19 10:55           ` Stafford Horne
2022-07-19 11:55             ` Arnd Bergmann
2022-07-19 12:23               ` Stafford Horne
2022-07-19 13:05                 ` Stafford Horne
2022-07-19 13:18                   ` Arnd Bergmann
2022-07-19 13:33                     ` Stafford Horne
2022-07-19 14:32                   ` Christoph Hellwig
2022-07-20 13:21                     ` Stafford Horne
2022-07-19 15:09                 ` David Laight
2022-07-20 13:24                   ` Stafford Horne

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).