2022-08-18 21:51:39

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 1/4] ARM: footbridge: remove addin mode

From: Arnd Bergmann <[email protected]>

This does not appear to have been used in many years, we can
kill off some of the uglier code.

Among other things, it avoids a randconfig issue when both modes
are disabled:

arch/arm/mach-footbridge/common.c:149:24: error: 'ebsa285_host_io_desc' defined but not used [-Werror=unused-variable]
149 | static struct map_desc ebsa285_host_io_desc[] __initdata = {
| ^~~~~~~~~~~~~~~~~~~~
arch/arm/mach-footbridge/common.c:136:24: error: 'fb_common_io_desc' defined but not used [-Werror=unused-variable]
136 | static struct map_desc fb_common_io_desc[] __initdata = {
| ^~~~~~~~~~~~~~~~~

The recently added phys_to_dma() functions are now trivial and
could probably be removed again as a follow-up, if anyone knows
how.

Cc: Christoph Hellwig <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
---
arch/arm/include/asm/hardware/dec21285.h | 13 ----
arch/arm/mach-footbridge/Kconfig | 28 +------
arch/arm/mach-footbridge/Makefile | 2 +-
arch/arm/mach-footbridge/common.c | 73 ++-----------------
arch/arm/mach-footbridge/dc21285.c | 61 +++++-----------
arch/arm/mach-footbridge/dma.c | 3 +-
.../arm/mach-footbridge/include/mach/memory.h | 22 ------
arch/arm/mach-footbridge/isa.c | 3 -
8 files changed, 30 insertions(+), 175 deletions(-)

diff --git a/arch/arm/include/asm/hardware/dec21285.h b/arch/arm/include/asm/hardware/dec21285.h
index 3f18a56a025d..eee390e8d4dc 100644
--- a/arch/arm/include/asm/hardware/dec21285.h
+++ b/arch/arm/include/asm/hardware/dec21285.h
@@ -81,19 +81,6 @@
#define SA110_CNTL_XCSDIR(x) ((x)<<28)
#define SA110_CNTL_PCICFN (1 << 31)

-/*
- * footbridge_cfn_mode() is used when we want
- * to check whether we are the central function
- */
-#define __footbridge_cfn_mode() (*CSR_SA110_CNTL & SA110_CNTL_PCICFN)
-#if defined(CONFIG_FOOTBRIDGE_HOST) && defined(CONFIG_FOOTBRIDGE_ADDIN)
-#define footbridge_cfn_mode() __footbridge_cfn_mode()
-#elif defined(CONFIG_FOOTBRIDGE_HOST)
-#define footbridge_cfn_mode() (1)
-#else
-#define footbridge_cfn_mode() (0)
-#endif
-
#define CSR_PCIADDR_EXTN DC21285_IO(0x0140)
#define CSR_PREFETCHMEMRANGE DC21285_IO(0x0144)
#define CSR_XBUS_CYCLE DC21285_IO(0x0148)
diff --git a/arch/arm/mach-footbridge/Kconfig b/arch/arm/mach-footbridge/Kconfig
index acc10b1caa69..3497ea8e91e3 100644
--- a/arch/arm/mach-footbridge/Kconfig
+++ b/arch/arm/mach-footbridge/Kconfig
@@ -8,31 +8,17 @@ config ARCH_CATS
depends on UNUSED_BOARD_FILES
select CLKEVT_I8253
select CLKSRC_I8253
- select FOOTBRIDGE_HOST
select ISA
- select ISA_DMA
select FORCE_PCI
help
Say Y here if you intend to run this kernel on the CATS.

Saying N will reduce the size of the Footbridge kernel.

-config ARCH_EBSA285_ADDIN
- bool "EBSA285 (addin mode)"
- select ARCH_EBSA285
- select FOOTBRIDGE_ADDIN
- help
- Say Y here if you intend to run this kernel on the EBSA285 card
- in addin mode.
-
- Saying N will reduce the size of the Footbridge kernel.
-
config ARCH_EBSA285_HOST
bool "EBSA285 (host mode)"
select ARCH_EBSA285
- select FOOTBRIDGE_HOST
select ISA
- select ISA_DMA
select ARCH_MAY_HAVE_PC_FDC
select FORCE_PCI
help
@@ -45,9 +31,7 @@ config ARCH_NETWINDER
bool "NetWinder"
select CLKEVT_I8253
select CLKSRC_I8253
- select FOOTBRIDGE_HOST
select ISA
- select ISA_DMA
select FORCE_PCI
help
Say Y here if you intend to run this kernel on the Rebel.COM
@@ -61,19 +45,11 @@ endmenu

# Footbridge support
config FOOTBRIDGE
+ def_bool y
select ARCH_HAS_PHYS_TO_DMA
- bool
-
-# Footbridge in host mode
-config FOOTBRIDGE_HOST
- bool
select ARCH_MIGHT_HAVE_PC_SERIO
+ select ISA_DMA

-# Footbridge in addin mode
-config FOOTBRIDGE_ADDIN
- bool
-
-# EBSA285 board in either host or addin mode
config ARCH_EBSA285
bool

diff --git a/arch/arm/mach-footbridge/Makefile b/arch/arm/mach-footbridge/Makefile
index 6262993c0555..30bae6684dce 100644
--- a/arch/arm/mach-footbridge/Makefile
+++ b/arch/arm/mach-footbridge/Makefile
@@ -9,7 +9,7 @@ obj-y := common.o dma.o isa-irq.o

pci-y += dc21285.o
pci-$(CONFIG_ARCH_CATS) += cats-pci.o
-pci-$(CONFIG_ARCH_EBSA285_HOST) += ebsa285-pci.o
+pci-$(CONFIG_ARCH_EBSA285) += ebsa285-pci.o
pci-$(CONFIG_ARCH_NETWINDER) += netwinder-pci.o

obj-$(CONFIG_ARCH_CATS) += cats-hw.o isa-timer.o
diff --git a/arch/arm/mach-footbridge/common.c b/arch/arm/mach-footbridge/common.c
index 5020eb96b025..9483eccea5ae 100644
--- a/arch/arm/mach-footbridge/common.c
+++ b/arch/arm/mach-footbridge/common.c
@@ -198,9 +198,6 @@ void __init footbridge_init_irq(void)

__fb_init_irq();

- if (!footbridge_cfn_mode())
- return;
-
if (machine_is_ebsa285())
/* The following is dependent on which slot
* you plug the Southbridge card into. We
@@ -221,21 +218,13 @@ void __init footbridge_init_irq(void)
* commented out since there is a "No Fix" problem with it. Not mapping
* it means that we have extra bullet protection on our feet.
*/
-static struct map_desc fb_common_io_desc[] __initdata = {
+static struct map_desc ebsa285_host_io_desc[] __initdata = {
{
.virtual = ARMCSR_BASE,
.pfn = __phys_to_pfn(DC21285_ARMCSR_BASE),
.length = ARMCSR_SIZE,
.type = MT_DEVICE,
- }
-};
-
-/*
- * The mapping when the footbridge is in host mode. We don't map any of
- * this when we are in add-in mode.
- */
-static struct map_desc ebsa285_host_io_desc[] __initdata = {
-#if defined(CONFIG_ARCH_FOOTBRIDGE) && defined(CONFIG_FOOTBRIDGE_HOST)
+ },
{
.virtual = PCIMEM_BASE,
.pfn = __phys_to_pfn(DC21285_PCI_MEM),
@@ -257,26 +246,12 @@ static struct map_desc ebsa285_host_io_desc[] __initdata = {
.length = PCIIACK_SIZE,
.type = MT_DEVICE,
},
-#endif
};

void __init footbridge_map_io(void)
{
- /*
- * Set up the common mapping first; we need this to
- * determine whether we're in host mode or not.
- */
- iotable_init(fb_common_io_desc, ARRAY_SIZE(fb_common_io_desc));
-
- /*
- * Now, work out what we've got to map in addition on this
- * platform.
- */
- if (footbridge_cfn_mode()) {
- iotable_init(ebsa285_host_io_desc, ARRAY_SIZE(ebsa285_host_io_desc));
- pci_map_io_early(__phys_to_pfn(DC21285_PCI_IO));
- }
-
+ iotable_init(ebsa285_host_io_desc, ARRAY_SIZE(ebsa285_host_io_desc));
+ pci_map_io_early(__phys_to_pfn(DC21285_PCI_IO));
vga_base = PCIMEM_BASE;
}

@@ -307,48 +282,12 @@ void footbridge_restart(enum reboot_mode mode, const char *cmd)
}
}

-#ifdef CONFIG_FOOTBRIDGE_ADDIN
-
-static inline unsigned long fb_bus_sdram_offset(void)
-{
- return *CSR_PCISDRAMBASE & 0xfffffff0;
-}
-
-/*
- * These two functions convert virtual addresses to PCI addresses and PCI
- * addresses to virtual addresses. Note that it is only legal to use these
- * on memory obtained via get_zeroed_page or kmalloc.
- */
-unsigned long __virt_to_bus(unsigned long res)
-{
- WARN_ON(res < PAGE_OFFSET || res >= (unsigned long)high_memory);
-
- return res + (fb_bus_sdram_offset() - PAGE_OFFSET);
-}
-EXPORT_SYMBOL(__virt_to_bus);
-
-unsigned long __bus_to_virt(unsigned long res)
-{
- res = res - (fb_bus_sdram_offset() - PAGE_OFFSET);
-
- WARN_ON(res < PAGE_OFFSET || res >= (unsigned long)high_memory);
-
- return res;
-}
-EXPORT_SYMBOL(__bus_to_virt);
-#else
-static inline unsigned long fb_bus_sdram_offset(void)
-{
- return BUS_OFFSET;
-}
-#endif /* CONFIG_FOOTBRIDGE_ADDIN */
-
dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
{
- return paddr + (fb_bus_sdram_offset() - PHYS_OFFSET);
+ return paddr + (BUS_OFFSET - PHYS_OFFSET);
}

phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dev_addr)
{
- return dev_addr - (fb_bus_sdram_offset() - PHYS_OFFSET);
+ return dev_addr - (BUS_OFFSET - PHYS_OFFSET);
}
diff --git a/arch/arm/mach-footbridge/dc21285.c b/arch/arm/mach-footbridge/dc21285.c
index f9713dc561cf..372600bc15c8 100644
--- a/arch/arm/mach-footbridge/dc21285.c
+++ b/arch/arm/mach-footbridge/dc21285.c
@@ -245,9 +245,6 @@ int __init dc21285_setup(int nr, struct pci_sys_data *sys)
{
struct resource *res;

- if (nr || !footbridge_cfn_mode())
- return 0;
-
res = kcalloc(2, sizeof(struct resource), GFP_KERNEL);
if (!res) {
printk("out of memory for root bus resources");
@@ -278,7 +275,6 @@ int __init dc21285_setup(int nr, struct pci_sys_data *sys)
void __init dc21285_preinit(void)
{
unsigned int mem_size, mem_mask;
- int cfn_mode;

pcibios_min_mem = 0x81000000;

@@ -298,21 +294,15 @@ void __init dc21285_preinit(void)
*CSR_CSRBASEOFFSET = 0;
*CSR_PCIADDR_EXTN = 0;

- cfn_mode = __footbridge_cfn_mode();
-
printk(KERN_INFO "PCI: DC21285 footbridge, revision %02lX, in "
- "%s mode\n", *CSR_CLASSREV & 0xff, cfn_mode ?
- "central function" : "addin");
-
- if (footbridge_cfn_mode()) {
- /*
- * Clear any existing errors - we aren't
- * interested in historical data...
- */
- *CSR_SA110_CNTL = (*CSR_SA110_CNTL & 0xffffde07) |
- SA110_CNTL_RXSERR;
- *CSR_PCICMD = (*CSR_PCICMD & 0xffff) | PCICMD_ERROR_BITS;
- }
+ "central function mode\n", *CSR_CLASSREV & 0xff);
+
+ /*
+ * Clear any existing errors - we aren't
+ * interested in historical data...
+ */
+ *CSR_SA110_CNTL = (*CSR_SA110_CNTL & 0xffffde07) | SA110_CNTL_RXSERR;
+ *CSR_PCICMD = (*CSR_PCICMD & 0xffff) | PCICMD_ERROR_BITS;

timer_setup(&serr_timer, dc21285_enable_error, 0);
timer_setup(&perr_timer, dc21285_enable_error, 0);
@@ -331,29 +321,18 @@ void __init dc21285_preinit(void)
dc21285_request_irq(IRQ_PCI_DPERR, dc21285_dparity_irq, 0,
"PCI data parity", NULL);

- if (cfn_mode) {
- /*
- * Map our SDRAM at a known address in PCI space, just in case
- * the firmware had other ideas. Using a nonzero base is
- * necessary, since some VGA cards forcefully use PCI addresses
- * in the range 0x000a0000 to 0x000c0000. (eg, S3 cards).
- */
- *CSR_PCICSRBASE = 0xf4000000;
- *CSR_PCICSRIOBASE = 0;
- *CSR_PCISDRAMBASE = __virt_to_bus(PAGE_OFFSET);
- *CSR_PCIROMBASE = 0;
- *CSR_PCICMD = PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER |
- PCI_COMMAND_INVALIDATE | PCICMD_ERROR_BITS;
- } else if (footbridge_cfn_mode() != 0) {
- /*
- * If we are not compiled to accept "add-in" mode, then
- * we are using a constant virt_to_bus translation which
- * can not hope to cater for the way the host BIOS has
- * set up the machine.
- */
- panic("PCI: this kernel is compiled for central "
- "function mode only");
- }
+ /*
+ * Map our SDRAM at a known address in PCI space, just in case
+ * the firmware had other ideas. Using a nonzero base is
+ * necessary, since some VGA cards forcefully use PCI addresses
+ * in the range 0x000a0000 to 0x000c0000. (eg, S3 cards).
+ */
+ *CSR_PCICSRBASE = 0xf4000000;
+ *CSR_PCICSRIOBASE = 0;
+ *CSR_PCISDRAMBASE = __virt_to_bus(PAGE_OFFSET);
+ *CSR_PCIROMBASE = 0;
+ *CSR_PCICMD = PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER |
+ PCI_COMMAND_INVALIDATE | PCICMD_ERROR_BITS;
}

void __init dc21285_postinit(void)
diff --git a/arch/arm/mach-footbridge/dma.c b/arch/arm/mach-footbridge/dma.c
index 86618074a7a5..228757c8e06f 100644
--- a/arch/arm/mach-footbridge/dma.c
+++ b/arch/arm/mach-footbridge/dma.c
@@ -50,8 +50,7 @@ static int __init fb_dma_init(void)
dma[_DC21285_DMA(1)].d_ops = &fb_dma_ops;
#endif
#ifdef CONFIG_ISA_DMA
- if (footbridge_cfn_mode())
- isa_init_dma();
+ isa_init_dma();
#endif
return 0;
}
diff --git a/arch/arm/mach-footbridge/include/mach/memory.h b/arch/arm/mach-footbridge/include/mach/memory.h
index 3a5d2638c18f..8f0ca8e8b37e 100644
--- a/arch/arm/mach-footbridge/include/mach/memory.h
+++ b/arch/arm/mach-footbridge/include/mach/memory.h
@@ -16,22 +16,6 @@
#ifndef __ASM_ARCH_MEMORY_H
#define __ASM_ARCH_MEMORY_H

-
-#if defined(CONFIG_FOOTBRIDGE_ADDIN)
-/*
- * If we may be using add-in footbridge mode, then we must
- * use the out-of-line translation that makes use of the
- * PCI BAR
- */
-#ifndef __ASSEMBLY__
-extern unsigned long __virt_to_bus(unsigned long);
-extern unsigned long __bus_to_virt(unsigned long);
-#endif
-#define __virt_to_bus __virt_to_bus
-#define __bus_to_virt __bus_to_virt
-
-#elif defined(CONFIG_FOOTBRIDGE_HOST)
-
/*
* The footbridge is programmed to expose the system RAM at 0xe0000000.
* The requirement is that the RAM isn't placed at bus address 0, which
@@ -41,12 +25,6 @@ extern unsigned long __bus_to_virt(unsigned long);
#define __virt_to_bus(x) ((x) + (BUS_OFFSET - PAGE_OFFSET))
#define __bus_to_virt(x) ((x) - (BUS_OFFSET - PAGE_OFFSET))

-#else
-
-#error "Undefined footbridge mode"
-
-#endif
-
/*
* Cache flushing area.
*/
diff --git a/arch/arm/mach-footbridge/isa.c b/arch/arm/mach-footbridge/isa.c
index ec5af521cf95..3375ac23c046 100644
--- a/arch/arm/mach-footbridge/isa.c
+++ b/arch/arm/mach-footbridge/isa.c
@@ -79,9 +79,6 @@ static int __init footbridge_isa_init(void)
{
int err = 0;

- if (!footbridge_cfn_mode())
- return 0;
-
/* Personal server doesn't have RTC */
if (!machine_is_personal_server()) {
isa_rtc_init();
--
2.29.2


2022-08-21 06:08:31

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 1/4] ARM: footbridge: remove addin mode

On Thu, Aug 18, 2022 at 11:15:47PM +0200, Arnd Bergmann wrote:
> The recently added phys_to_dma() functions are now trivial and
> could probably be removed again as a follow-up, if anyone knows
> how.

The normal way to that would be with a call to dma_direct_set_offset in
the init code. But for that we'd need to find someone to care enough
about footbridge to test it.

2022-08-24 11:13:01

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 1/4] ARM: footbridge: remove addin mode

On Sun, Aug 21, 2022 at 7:53 AM Christoph Hellwig <[email protected]> wrote:
>
> On Thu, Aug 18, 2022 at 11:15:47PM +0200, Arnd Bergmann wrote:
> > The recently added phys_to_dma() functions are now trivial and
> > could probably be removed again as a follow-up, if anyone knows
> > how.
>
> The normal way to that would be with a call to dma_direct_set_offset in
> the init code.

Ah good, that is what I was looking for. Unfortunately it seems that the
platform does not itself create any 'struct device'. The DMA masters
are either ISA devices, which I think go through a dummy device an
should also use offset 0 (the same offset seems to get added
in dma-mapping code but subtracted again in the ISA DMA API)
and in PCI bus masters.

Is it enough to just call dma_direct_set_offset() on the PCI root
device, or do I have to add a pci_host_bridge callback to get
called from pcibios_bus_add_device()?

> But for that we'd need to find someone to care enough
> about footbridge to test it.

LinusW, MarcZ and RMK all have one of these, I'm fairly sure
one of them can test the PCI side, though possibly not
the ISA DMA ones (parport, floppy, ne2000, and soundblaster
in case of netwinder).

Arnd

2022-08-25 09:37:46

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH 5/4] ARM: footbridge: remove custom DMA address handling

From: Arnd Bergmann <[email protected]>

Footbridge is the last Arm platform that has its own
__virt_to_bus()/__bus_to_virt()/phys_to_dma()/dma_to_phys() abstraction,
but this is just a simple offset now.

For ISA DMA (floppy etc), the offset is actually zero, so the special
hack to subtract the bus offset can be removed from common code.

For PCI devices, the offset that is programmed into the PCI bridge
must also be set in each device using dma_direct_set_offset().
As Arm does not have a pcibios_bus_add_device() helper yet, just
use a bus notifier for this.

Signed-off-by: Arnd Bergmann <[email protected]>
---
I added this patch on top of the footbridge series,
the other four are unmodified.

arch/arm/include/asm/dma-direct.h | 1 -
arch/arm/include/asm/dma.h | 2 +-
arch/arm/include/asm/memory.h | 11 --------
arch/arm/mach-footbridge/Kconfig | 1 -
arch/arm/mach-footbridge/common.c | 10 -------
arch/arm/mach-footbridge/dc21285.c | 28 ++++++++++++++++++-
.../mach-footbridge/include/mach/dma-direct.h | 8 ------
.../arm/mach-footbridge/include/mach/memory.h | 9 ------
8 files changed, 28 insertions(+), 42 deletions(-)
delete mode 100644 arch/arm/include/asm/dma-direct.h
delete mode 100644 arch/arm/mach-footbridge/include/mach/dma-direct.h

diff --git a/arch/arm/include/asm/dma-direct.h b/arch/arm/include/asm/dma-direct.h
deleted file mode 100644
index 4f7bcde03abb..000000000000
--- a/arch/arm/include/asm/dma-direct.h
+++ /dev/null
@@ -1 +0,0 @@
-#include <mach/dma-direct.h>
diff --git a/arch/arm/include/asm/dma.h b/arch/arm/include/asm/dma.h
index 05f29a72150b..ab3863a3cdf2 100644
--- a/arch/arm/include/asm/dma.h
+++ b/arch/arm/include/asm/dma.h
@@ -106,7 +106,7 @@ extern void set_dma_sg(unsigned int chan, struct scatterlist *sg, int nr_sg);
*/
extern void __set_dma_addr(unsigned int chan, void *addr);
#define set_dma_addr(chan, addr) \
- __set_dma_addr(chan, (void *)__bus_to_virt(addr))
+ __set_dma_addr(chan, (void *)(addr))

/* Set the DMA byte count for this channel
*
diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index a55a9038abc8..d8eef4bd8c71 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -369,17 +369,6 @@ static inline unsigned long __virt_to_idmap(unsigned long x)

#define virt_to_idmap(x) __virt_to_idmap((unsigned long)(x))

-/*
- * Virtual <-> DMA view memory address translations
- * Again, these are *only* valid on the kernel direct mapped RAM
- * memory. Use of these is *deprecated* (and that doesn't mean
- * use the __ prefixed forms instead.) See dma-mapping.h.
- */
-#ifndef __virt_to_bus
-#define __virt_to_bus __virt_to_phys
-#define __bus_to_virt __phys_to_virt
-#endif
-
/*
* Conversion between a struct page and a physical address.
*
diff --git a/arch/arm/mach-footbridge/Kconfig b/arch/arm/mach-footbridge/Kconfig
index cd18871fd03f..f452663576d1 100644
--- a/arch/arm/mach-footbridge/Kconfig
+++ b/arch/arm/mach-footbridge/Kconfig
@@ -44,7 +44,6 @@ endmenu
# Footbridge support
config FOOTBRIDGE
def_bool y
- select ARCH_HAS_PHYS_TO_DMA
select ARCH_MIGHT_HAVE_PC_SERIO
select ISA_DMA_API

diff --git a/arch/arm/mach-footbridge/common.c b/arch/arm/mach-footbridge/common.c
index 9483eccea5ae..629e4676ed77 100644
--- a/arch/arm/mach-footbridge/common.c
+++ b/arch/arm/mach-footbridge/common.c
@@ -281,13 +281,3 @@ void footbridge_restart(enum reboot_mode mode, const char *cmd)
*CSR_SA110_CNTL |= (1 << 13);
}
}
-
-dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
-{
- return paddr + (BUS_OFFSET - PHYS_OFFSET);
-}
-
-phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dev_addr)
-{
- return dev_addr - (BUS_OFFSET - PHYS_OFFSET);
-}
diff --git a/arch/arm/mach-footbridge/dc21285.c b/arch/arm/mach-footbridge/dc21285.c
index 372600bc15c8..e34b33a955e2 100644
--- a/arch/arm/mach-footbridge/dc21285.c
+++ b/arch/arm/mach-footbridge/dc21285.c
@@ -5,6 +5,7 @@
* Copyright (C) 1998-2001 Russell King
* Copyright (C) 1998-2000 Phil Blundell
*/
+#include <linux/dma-map-ops.h>
#include <linux/kernel.h>
#include <linux/pci.h>
#include <linux/interrupt.h>
@@ -241,6 +242,29 @@ static irqreturn_t dc21285_parity_irq(int irq, void *dev_id)
return IRQ_HANDLED;
}

+/*
+ * The footbridge is programmed to expose the system RAM at 0xe0000000.
+ * The requirement is that the RAM isn't placed at bus address 0, which
+ * would clash with VGA cards.
+ */
+#define BUS_OFFSET 0xe0000000
+
+static int dc21285_pci_bus_notifier(struct notifier_block *nb,
+ unsigned long action,
+ void *data)
+{
+ if (action != BUS_NOTIFY_ADD_DEVICE)
+ return NOTIFY_DONE;
+
+ dma_direct_set_offset(data, PHYS_OFFSET, BUS_OFFSET, SZ_256M);
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block dc21285_pci_bus_nb = {
+ .notifier_call = dc21285_pci_bus_notifier,
+};
+
int __init dc21285_setup(int nr, struct pci_sys_data *sys)
{
struct resource *res;
@@ -266,6 +290,8 @@ int __init dc21285_setup(int nr, struct pci_sys_data *sys)
pci_add_resource_offset(&sys->resources, &res[0], sys->mem_offset);
pci_add_resource_offset(&sys->resources, &res[1], sys->mem_offset);

+ bus_register_notifier(&pci_bus_type, &dc21285_pci_bus_nb);
+
return 1;
}

@@ -329,7 +355,7 @@ void __init dc21285_preinit(void)
*/
*CSR_PCICSRBASE = 0xf4000000;
*CSR_PCICSRIOBASE = 0;
- *CSR_PCISDRAMBASE = __virt_to_bus(PAGE_OFFSET);
+ *CSR_PCISDRAMBASE = BUS_OFFSET;
*CSR_PCIROMBASE = 0;
*CSR_PCICMD = PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER |
PCI_COMMAND_INVALIDATE | PCICMD_ERROR_BITS;
diff --git a/arch/arm/mach-footbridge/include/mach/dma-direct.h b/arch/arm/mach-footbridge/include/mach/dma-direct.h
deleted file mode 100644
index 01f9e8367c00..000000000000
--- a/arch/arm/mach-footbridge/include/mach/dma-direct.h
+++ /dev/null
@@ -1,8 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef MACH_FOOTBRIDGE_DMA_DIRECT_H
-#define MACH_FOOTBRIDGE_DMA_DIRECT_H 1
-
-dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr);
-phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dev_addr);
-
-#endif /* MACH_FOOTBRIDGE_DMA_DIRECT_H */
diff --git a/arch/arm/mach-footbridge/include/mach/memory.h b/arch/arm/mach-footbridge/include/mach/memory.h
index 8f0ca8e8b37e..9516877667d7 100644
--- a/arch/arm/mach-footbridge/include/mach/memory.h
+++ b/arch/arm/mach-footbridge/include/mach/memory.h
@@ -16,15 +16,6 @@
#ifndef __ASM_ARCH_MEMORY_H
#define __ASM_ARCH_MEMORY_H

-/*
- * The footbridge is programmed to expose the system RAM at 0xe0000000.
- * The requirement is that the RAM isn't placed at bus address 0, which
- * would clash with VGA cards.
- */
-#define BUS_OFFSET 0xe0000000
-#define __virt_to_bus(x) ((x) + (BUS_OFFSET - PAGE_OFFSET))
-#define __bus_to_virt(x) ((x) - (BUS_OFFSET - PAGE_OFFSET))
-
/*
* Cache flushing area.
*/
--
2.29.2

2022-08-25 10:43:33

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH 5/4] ARM: footbridge: remove custom DMA address handling

On Thu, Aug 25, 2022 at 11:17:52AM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> Footbridge is the last Arm platform that has its own
> __virt_to_bus()/__bus_to_virt()/phys_to_dma()/dma_to_phys() abstraction,
> but this is just a simple offset now.
>
> For ISA DMA (floppy etc), the offset is actually zero, so the special
> hack to subtract the bus offset can be removed from common code.

No, the offset is definitely not zero for ISA devices. ISA devices have
the same view of system memory as PCI devices, since ISA devices go
through a southbridge and on to the PCI bus. So their view of the system
is exactly the same as any other PCI device.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

2022-08-25 11:07:33

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH 5/4] ARM: footbridge: remove custom DMA address handling

On Thu, Aug 25, 2022 at 11:17:52AM +0200, Arnd Bergmann wrote:
> -/*
> - * The footbridge is programmed to expose the system RAM at 0xe0000000.
> - * The requirement is that the RAM isn't placed at bus address 0, which
> - * would clash with VGA cards.
> - */
> -#define BUS_OFFSET 0xe0000000
> -#define __virt_to_bus(x) ((x) + (BUS_OFFSET - PAGE_OFFSET))
> -#define __bus_to_virt(x) ((x) - (BUS_OFFSET - PAGE_OFFSET))

... and this tells you why that is with ISA devices.

So, one hell of a big NAK to this patch, sorry.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

2022-08-25 17:20:23

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 5/4] ARM: footbridge: remove custom DMA address handling

On Thu, Aug 25, 2022 at 12:35 PM Russell King (Oracle)
<[email protected]> wrote:
>
> On Thu, Aug 25, 2022 at 11:17:52AM +0200, Arnd Bergmann wrote:
> > -/*
> > - * The footbridge is programmed to expose the system RAM at 0xe0000000.
> > - * The requirement is that the RAM isn't placed at bus address 0, which
> > - * would clash with VGA cards.
> > - */
> > -#define BUS_OFFSET 0xe0000000
> > -#define __virt_to_bus(x) ((x) + (BUS_OFFSET - PAGE_OFFSET))
> > -#define __bus_to_virt(x) ((x) - (BUS_OFFSET - PAGE_OFFSET))
>
> ... and this tells you why that is with ISA devices.
>
> So, one hell of a big NAK to this patch, sorry.

Thanks for taking a look. Apparently I misread the interface, and I assumed
that the __bus_to_virt() BUS_OFFSET in set_dma_addr() would
just cancel out the dma_map_single() in isa_enable_dma().

It turns out the logic is actually simpler and we just convert between
virtual address and bus address three times. for most drivers.

I found one driver (cs89x0-isa) that uses isa_virt_to_bus()/virt_to_phys()
instead of dma_map_single(), but that is probably never used here.
Once we move the DMA offset into the device, all other ISA devices will
also get the default zero-offset, but I think that will be fine as long as the
"isa_dma_dev" that is used for the final translation has the correct offset.

I'll try to come up with a new version tomorrow.

Arnd

2022-08-25 20:38:39

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH v2 5/4] ARM: footbridge: remove custom DMA address handling

From: Arnd Bergmann <[email protected]>

Footbridge is the last Arm platform that has its own
__virt_to_bus()/__bus_to_virt()/phys_to_dma()/dma_to_phys() abstraction,
but this is just a simple offset now.

For PCI devices, the offset that is programmed into the PCI bridge must
also be set in each device using dma_direct_set_offset(). As Arm does
not have a pcibios_bus_add_device() helper yet, just use a bus notifier
for this.

For the ISA DMA, drivers now pass a non-translated physical address into
set_dma_addr(), so they have to be converted back with the corresponding
isa_bus_to_virt() function and then into the correct bus address with
the offset using the isa_dma_dev.

Signed-off-by: Arnd Bergmann <[email protected]>
---
v2: fix ISA DMA offset

arch/arm/include/asm/dma-direct.h | 1 -
arch/arm/include/asm/dma.h | 2 +-
arch/arm/include/asm/hardware/dec21285.h | 7 +++++++
arch/arm/include/asm/memory.h | 11 ----------
arch/arm/mach-footbridge/Kconfig | 1 -
arch/arm/mach-footbridge/common.c | 10 ---------
arch/arm/mach-footbridge/dc21285.c | 21 ++++++++++++++++++-
arch/arm/mach-footbridge/dma-isa.c | 4 ++++
.../mach-footbridge/include/mach/dma-direct.h | 8 -------
.../arm/mach-footbridge/include/mach/memory.h | 9 --------
10 files changed, 32 insertions(+), 42 deletions(-)
delete mode 100644 arch/arm/include/asm/dma-direct.h
delete mode 100644 arch/arm/mach-footbridge/include/mach/dma-direct.h

diff --git a/arch/arm/include/asm/dma-direct.h b/arch/arm/include/asm/dma-direct.h
deleted file mode 100644
index 4f7bcde03abb..000000000000
--- a/arch/arm/include/asm/dma-direct.h
+++ /dev/null
@@ -1 +0,0 @@
-#include <mach/dma-direct.h>
diff --git a/arch/arm/include/asm/dma.h b/arch/arm/include/asm/dma.h
index 05f29a72150b..c6aded1b069c 100644
--- a/arch/arm/include/asm/dma.h
+++ b/arch/arm/include/asm/dma.h
@@ -106,7 +106,7 @@ extern void set_dma_sg(unsigned int chan, struct scatterlist *sg, int nr_sg);
*/
extern void __set_dma_addr(unsigned int chan, void *addr);
#define set_dma_addr(chan, addr) \
- __set_dma_addr(chan, (void *)__bus_to_virt(addr))
+ __set_dma_addr(chan, (void *)isa_bus_to_virt(addr))

/* Set the DMA byte count for this channel
*
diff --git a/arch/arm/include/asm/hardware/dec21285.h b/arch/arm/include/asm/hardware/dec21285.h
index eee390e8d4dc..894f2a635cbb 100644
--- a/arch/arm/include/asm/hardware/dec21285.h
+++ b/arch/arm/include/asm/hardware/dec21285.h
@@ -22,6 +22,13 @@
#define DC21285_IO(x) (x)
#endif

+/*
+ * The footbridge is programmed to expose the system RAM at 0xe0000000.
+ * The requirement is that the RAM isn't placed at bus address 0, which
+ * would clash with VGA cards.
+ */
+#define BUS_OFFSET 0xe0000000
+
#define CSR_PCICMD DC21285_IO(0x0004)
#define CSR_CLASSREV DC21285_IO(0x0008)
#define CSR_PCICACHELINESIZE DC21285_IO(0x000c)
diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index a55a9038abc8..d8eef4bd8c71 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -369,17 +369,6 @@ static inline unsigned long __virt_to_idmap(unsigned long x)

#define virt_to_idmap(x) __virt_to_idmap((unsigned long)(x))

-/*
- * Virtual <-> DMA view memory address translations
- * Again, these are *only* valid on the kernel direct mapped RAM
- * memory. Use of these is *deprecated* (and that doesn't mean
- * use the __ prefixed forms instead.) See dma-mapping.h.
- */
-#ifndef __virt_to_bus
-#define __virt_to_bus __virt_to_phys
-#define __bus_to_virt __phys_to_virt
-#endif
-
/*
* Conversion between a struct page and a physical address.
*
diff --git a/arch/arm/mach-footbridge/Kconfig b/arch/arm/mach-footbridge/Kconfig
index cd18871fd03f..f452663576d1 100644
--- a/arch/arm/mach-footbridge/Kconfig
+++ b/arch/arm/mach-footbridge/Kconfig
@@ -44,7 +44,6 @@ endmenu
# Footbridge support
config FOOTBRIDGE
def_bool y
- select ARCH_HAS_PHYS_TO_DMA
select ARCH_MIGHT_HAVE_PC_SERIO
select ISA_DMA_API

diff --git a/arch/arm/mach-footbridge/common.c b/arch/arm/mach-footbridge/common.c
index 9483eccea5ae..629e4676ed77 100644
--- a/arch/arm/mach-footbridge/common.c
+++ b/arch/arm/mach-footbridge/common.c
@@ -281,13 +281,3 @@ void footbridge_restart(enum reboot_mode mode, const char *cmd)
*CSR_SA110_CNTL |= (1 << 13);
}
}
-
-dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
-{
- return paddr + (BUS_OFFSET - PHYS_OFFSET);
-}
-
-phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dev_addr)
-{
- return dev_addr - (BUS_OFFSET - PHYS_OFFSET);
-}
diff --git a/arch/arm/mach-footbridge/dc21285.c b/arch/arm/mach-footbridge/dc21285.c
index 372600bc15c8..f8920d0010de 100644
--- a/arch/arm/mach-footbridge/dc21285.c
+++ b/arch/arm/mach-footbridge/dc21285.c
@@ -5,6 +5,7 @@
* Copyright (C) 1998-2001 Russell King
* Copyright (C) 1998-2000 Phil Blundell
*/
+#include <linux/dma-map-ops.h>
#include <linux/kernel.h>
#include <linux/pci.h>
#include <linux/interrupt.h>
@@ -241,6 +242,22 @@ static irqreturn_t dc21285_parity_irq(int irq, void *dev_id)
return IRQ_HANDLED;
}

+static int dc21285_pci_bus_notifier(struct notifier_block *nb,
+ unsigned long action,
+ void *data)
+{
+ if (action != BUS_NOTIFY_ADD_DEVICE)
+ return NOTIFY_DONE;
+
+ dma_direct_set_offset(data, PHYS_OFFSET, BUS_OFFSET, SZ_256M);
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block dc21285_pci_bus_nb = {
+ .notifier_call = dc21285_pci_bus_notifier,
+};
+
int __init dc21285_setup(int nr, struct pci_sys_data *sys)
{
struct resource *res;
@@ -266,6 +283,8 @@ int __init dc21285_setup(int nr, struct pci_sys_data *sys)
pci_add_resource_offset(&sys->resources, &res[0], sys->mem_offset);
pci_add_resource_offset(&sys->resources, &res[1], sys->mem_offset);

+ bus_register_notifier(&pci_bus_type, &dc21285_pci_bus_nb);
+
return 1;
}

@@ -329,7 +348,7 @@ void __init dc21285_preinit(void)
*/
*CSR_PCICSRBASE = 0xf4000000;
*CSR_PCICSRIOBASE = 0;
- *CSR_PCISDRAMBASE = __virt_to_bus(PAGE_OFFSET);
+ *CSR_PCISDRAMBASE = BUS_OFFSET;
*CSR_PCIROMBASE = 0;
*CSR_PCICMD = PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER |
PCI_COMMAND_INVALIDATE | PCICMD_ERROR_BITS;
diff --git a/arch/arm/mach-footbridge/dma-isa.c b/arch/arm/mach-footbridge/dma-isa.c
index 0fc0705fd58b..937f5376d5e7 100644
--- a/arch/arm/mach-footbridge/dma-isa.c
+++ b/arch/arm/mach-footbridge/dma-isa.c
@@ -11,6 +11,7 @@
* arch/arm/kernel/dma-ebsa285.c
* Copyright (C) 1998 Phil Blundell
*/
+#include <linux/dma-map-ops.h>
#include <linux/ioport.h>
#include <linux/init.h>
#include <linux/dma-mapping.h>
@@ -18,6 +19,7 @@

#include <asm/dma.h>
#include <asm/mach/dma.h>
+#include <asm/hardware/dec21285.h>

#define ISA_DMA_MASK 0
#define ISA_DMA_MODE 1
@@ -221,6 +223,8 @@ static int __init isa_dma_init(void)
request_dma(DMA_ISA_CASCADE, "cascade");
}

+ dma_direct_set_offset(&isa_dma_dev, PHYS_OFFSET, BUS_OFFSET, SZ_256M);
+
return 0;
}
core_initcall(isa_dma_init);
diff --git a/arch/arm/mach-footbridge/include/mach/dma-direct.h b/arch/arm/mach-footbridge/include/mach/dma-direct.h
deleted file mode 100644
index 01f9e8367c00..000000000000
--- a/arch/arm/mach-footbridge/include/mach/dma-direct.h
+++ /dev/null
@@ -1,8 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef MACH_FOOTBRIDGE_DMA_DIRECT_H
-#define MACH_FOOTBRIDGE_DMA_DIRECT_H 1
-
-dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr);
-phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dev_addr);
-
-#endif /* MACH_FOOTBRIDGE_DMA_DIRECT_H */
diff --git a/arch/arm/mach-footbridge/include/mach/memory.h b/arch/arm/mach-footbridge/include/mach/memory.h
index 8f0ca8e8b37e..9516877667d7 100644
--- a/arch/arm/mach-footbridge/include/mach/memory.h
+++ b/arch/arm/mach-footbridge/include/mach/memory.h
@@ -16,15 +16,6 @@
#ifndef __ASM_ARCH_MEMORY_H
#define __ASM_ARCH_MEMORY_H

-/*
- * The footbridge is programmed to expose the system RAM at 0xe0000000.
- * The requirement is that the RAM isn't placed at bus address 0, which
- * would clash with VGA cards.
- */
-#define BUS_OFFSET 0xe0000000
-#define __virt_to_bus(x) ((x) + (BUS_OFFSET - PAGE_OFFSET))
-#define __bus_to_virt(x) ((x) - (BUS_OFFSET - PAGE_OFFSET))
-
/*
* Cache flushing area.
*/
--
2.29.2

2022-08-30 12:30:32

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH v2 5/4] ARM: footbridge: remove custom DMA address handling

On 2022-08-25 21:15, Arnd Bergmann wrote:
[...]
> diff --git a/arch/arm/mach-footbridge/dc21285.c b/arch/arm/mach-footbridge/dc21285.c
> index 372600bc15c8..f8920d0010de 100644
> --- a/arch/arm/mach-footbridge/dc21285.c
> +++ b/arch/arm/mach-footbridge/dc21285.c
> @@ -5,6 +5,7 @@
> * Copyright (C) 1998-2001 Russell King
> * Copyright (C) 1998-2000 Phil Blundell
> */
> +#include <linux/dma-map-ops.h>
> #include <linux/kernel.h>
> #include <linux/pci.h>
> #include <linux/interrupt.h>
> @@ -241,6 +242,22 @@ static irqreturn_t dc21285_parity_irq(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +static int dc21285_pci_bus_notifier(struct notifier_block *nb,
> + unsigned long action,
> + void *data)
> +{
> + if (action != BUS_NOTIFY_ADD_DEVICE)
> + return NOTIFY_DONE;
> +
> + dma_direct_set_offset(data, PHYS_OFFSET, BUS_OFFSET, SZ_256M);

FWIW I've always had it in mind that for this kind of special case the
platform could just statically allocate the dma_range_map and assign it
to all devices. However in practice that needs some kfree_const() tweaks
in the driver core, which I'm sure are out-of-scope for this series, so
I'm mostly flagging this up to help myself remember that it might be
worth doing a dma_direct_set_offset() cleanup sweep soon.

As-is the patch at least makes things a lot clearer, and presumably
there aren't likely to be enough PCI devices for the extra allocations
to have noticeable impact in the meantime.

Cheers,
Robin.