This is a small set of patches that address build breakage with
allyesconfig / allmodconfig.
This solves some, but not all, build breakage.
The parport fix depends on the previous patch, the rest are independent
fixes.
With v2 there is a extra patch that drops ZONE_DMA support.
It does not fix any build failure, but a nice cleanup.
Cc: Miquel Raynal <[email protected]>
To: Maciej W. Rozycki <[email protected]>
To: <[email protected]>
Cc: <[email protected]>
Cc: David S. Miller <[email protected]>
To: Andreas Larsson <[email protected]>
To: Randy Dunlap <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: <[email protected]>
Changes in v2:
- Added r-b/tested by (thanks to Randy and Maciej)
- Dropped patch for uhci-grlib.c as it is already upstream (Randy)
- Added a few Fixes (Maciej)
- Fixed commit message when dropping GENERIC_ISA_DMA (Maciej)
- Added new patch that drop ZONE_DMA (Maciej)
- Added new patch to fix section mismatch error
In an allmodconfig build I see a lot of:
modpost: "__udelay" [module] has no CRC!
Similar for a handful of other symbols.
Any hint how to get rid of them would be nice.
I have tried to add the prototype to asm-prototypes.h with no luck.
On top of this the link fails, but I assume this the kernel that grows
too big which is no surprise.
- Link to v1: https://lore.kernel.org/r/20240223-sam-fix-sparc32-all-builds-v1-0-5c60fd5c9250@ravnborg.org
---
Sam Ravnborg (7):
sparc32: Use generic cmpdi2/ucmpdi2 variants
sparc32: Fix build with trapbase
mtd: maps: sun_uflash: Declare uflash_devinit static
sparc32: Do not select ZONE_DMA
sparc32: Do not select GENERIC_ISA_DMA
sparc32: Fix parport build with sparc32
sparc32: Fix section mismatch in leon_pci_grpci
arch/sparc/Kconfig | 7 +-
arch/sparc/include/asm/parport.h | 259 +-----------------------------------
arch/sparc/include/asm/parport_64.h | 256 +++++++++++++++++++++++++++++++++++
arch/sparc/kernel/irq_32.c | 6 +-
arch/sparc/kernel/kernel.h | 8 +-
arch/sparc/kernel/kgdb_32.c | 4 +-
arch/sparc/kernel/leon_pci_grpci1.c | 2 +-
arch/sparc/kernel/leon_pci_grpci2.c | 2 +-
arch/sparc/kernel/leon_smp.c | 6 +-
arch/sparc/kernel/setup_32.c | 4 +-
arch/sparc/lib/Makefile | 4 +-
arch/sparc/lib/cmpdi2.c | 28 ----
arch/sparc/lib/ucmpdi2.c | 20 ---
arch/sparc/mm/srmmu.c | 1 -
drivers/mtd/maps/sun_uflash.c | 2 +-
15 files changed, 284 insertions(+), 325 deletions(-)
---
base-commit: 626db6ee8ee1edac206610db407114aa83b53fd3
change-id: 20240223-sam-fix-sparc32-all-builds-0a0403d6e1b3
Best regards,
--
Sam Ravnborg <[email protected]>
From: Sam Ravnborg <[email protected]>
This fixes the following warning:
sun_uflash.c:50:5: error: no previous prototype for 'uflash_devinit'
Signed-off-by: Sam Ravnborg <[email protected]>
Fixes: 0fcb70851fbf ("Makefile.extrawarn: turn on missing-prototypes globally")
Reviewed-by: Randy Dunlap <[email protected]>
Tested-by: Randy Dunlap <[email protected]> # build-tested
Cc: Andreas Larsson <[email protected]>
Cc: "David S. Miller" <[email protected]>
---
drivers/mtd/maps/sun_uflash.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/maps/sun_uflash.c b/drivers/mtd/maps/sun_uflash.c
index f58cfb15d6e8..b69dade3f7ad 100644
--- a/drivers/mtd/maps/sun_uflash.c
+++ b/drivers/mtd/maps/sun_uflash.c
@@ -47,7 +47,7 @@ struct map_info uflash_map_templ = {
.bankwidth = UFLASH_BUSWIDTH,
};
-int uflash_devinit(struct platform_device *op, struct device_node *dp)
+static int uflash_devinit(struct platform_device *op, struct device_node *dp)
{
struct uflash_dev *up;
--
2.34.1
From: Sam Ravnborg <[email protected]>
Fix the following build errors:
irq_32.c:258:7: error: array subscript [16, 79] is outside array bounds of 'struct tt_entry[1]
irq_32.c:271:14: error: assignment to 'struct tt_entry *' from incompatible pointer type 'struct tt_entry (*)[]
trapbase is a pointer to an array of tt_entry, but the code declared it
as a pointer so the compiler see a single entry and not an array.
Fix this by modifyinf the declaration to be an array, and modify all
users to take the address of the first member.
Signed-off-by: Sam Ravnborg <[email protected]>
Acked-by: Randy Dunlap <[email protected]>
Tested-by: Randy Dunlap <[email protected]> # build-tested
Cc: Andreas Larsson <[email protected]>
Cc: "David S. Miller" <[email protected]>
---
arch/sparc/kernel/irq_32.c | 6 +++---
arch/sparc/kernel/kernel.h | 8 ++++----
arch/sparc/kernel/kgdb_32.c | 4 ++--
arch/sparc/kernel/leon_smp.c | 6 +++---
arch/sparc/kernel/setup_32.c | 4 ++--
5 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/sparc/kernel/irq_32.c b/arch/sparc/kernel/irq_32.c
index e8452be5123b..8605dd710f3c 100644
--- a/arch/sparc/kernel/irq_32.c
+++ b/arch/sparc/kernel/irq_32.c
@@ -268,11 +268,11 @@ int sparc_floppy_request_irq(unsigned int irq, irq_handler_t irq_handler)
if (sparc_cpu_model != sparc_leon) {
struct tt_entry *trap_table;
- trap_table = &trapbase_cpu1;
+ trap_table = &trapbase_cpu1[0];
INSTANTIATE(trap_table)
- trap_table = &trapbase_cpu2;
+ trap_table = &trapbase_cpu2[0];
INSTANTIATE(trap_table)
- trap_table = &trapbase_cpu3;
+ trap_table = &trapbase_cpu3[0];
INSTANTIATE(trap_table)
}
#endif
diff --git a/arch/sparc/kernel/kernel.h b/arch/sparc/kernel/kernel.h
index 15da3c0597a5..a8fb7c0bf053 100644
--- a/arch/sparc/kernel/kernel.h
+++ b/arch/sparc/kernel/kernel.h
@@ -138,10 +138,10 @@ extern unsigned int t_nmi[];
extern unsigned int linux_trap_ipi15_sun4d[];
extern unsigned int linux_trap_ipi15_sun4m[];
-extern struct tt_entry trapbase;
-extern struct tt_entry trapbase_cpu1;
-extern struct tt_entry trapbase_cpu2;
-extern struct tt_entry trapbase_cpu3;
+extern struct tt_entry trapbase[];
+extern struct tt_entry trapbase_cpu1[];
+extern struct tt_entry trapbase_cpu2[];
+extern struct tt_entry trapbase_cpu3[];
extern char cputypval[];
diff --git a/arch/sparc/kernel/kgdb_32.c b/arch/sparc/kernel/kgdb_32.c
index 58ad3f7de1fb..3b2c673ec627 100644
--- a/arch/sparc/kernel/kgdb_32.c
+++ b/arch/sparc/kernel/kgdb_32.c
@@ -37,7 +37,7 @@ void pt_regs_to_gdb_regs(unsigned long *gdb_regs, struct pt_regs *regs)
gdb_regs[GDB_Y] = regs->y;
gdb_regs[GDB_PSR] = regs->psr;
gdb_regs[GDB_WIM] = 0;
- gdb_regs[GDB_TBR] = (unsigned long) &trapbase;
+ gdb_regs[GDB_TBR] = (unsigned long) &trapbase[0];
gdb_regs[GDB_PC] = regs->pc;
gdb_regs[GDB_NPC] = regs->npc;
gdb_regs[GDB_FSR] = 0;
@@ -72,7 +72,7 @@ void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, struct task_struct *p)
gdb_regs[GDB_PSR] = t->kpsr;
gdb_regs[GDB_WIM] = t->kwim;
- gdb_regs[GDB_TBR] = (unsigned long) &trapbase;
+ gdb_regs[GDB_TBR] = (unsigned long) &trapbase[0];
gdb_regs[GDB_PC] = t->kpc;
gdb_regs[GDB_NPC] = t->kpc + 4;
gdb_regs[GDB_FSR] = 0;
diff --git a/arch/sparc/kernel/leon_smp.c b/arch/sparc/kernel/leon_smp.c
index 991e9ad3d3e8..1ee393abc463 100644
--- a/arch/sparc/kernel/leon_smp.c
+++ b/arch/sparc/kernel/leon_smp.c
@@ -245,13 +245,13 @@ void __init leon_smp_done(void)
/* Free unneeded trap tables */
if (!cpu_present(1)) {
- free_reserved_page(virt_to_page(&trapbase_cpu1));
+ free_reserved_page(virt_to_page(&trapbase_cpu1[0]));
}
if (!cpu_present(2)) {
- free_reserved_page(virt_to_page(&trapbase_cpu2));
+ free_reserved_page(virt_to_page(&trapbase_cpu2[0]));
}
if (!cpu_present(3)) {
- free_reserved_page(virt_to_page(&trapbase_cpu3));
+ free_reserved_page(virt_to_page(&trapbase_cpu3[0]));
}
/* Ok, they are spinning and ready to go. */
smp_processors_ready = 1;
diff --git a/arch/sparc/kernel/setup_32.c b/arch/sparc/kernel/setup_32.c
index e3b72a7b46d3..704375c061e7 100644
--- a/arch/sparc/kernel/setup_32.c
+++ b/arch/sparc/kernel/setup_32.c
@@ -67,7 +67,7 @@ static void prom_sync_me(void)
__asm__ __volatile__("wr %0, 0x0, %%tbr\n\t"
"nop\n\t"
"nop\n\t"
- "nop\n\t" : : "r" (&trapbase));
+ "nop\n\t" : : "r" (&trapbase[0]));
prom_printf("PROM SYNC COMMAND...\n");
show_mem();
@@ -285,7 +285,7 @@ void __init setup_arch(char **cmdline_p)
int i;
unsigned long highest_paddr;
- sparc_ttable = &trapbase;
+ sparc_ttable = &trapbase[0];
/* Initialize PROM console and command line. */
*cmdline_p = prom_getbootargs();
--
2.34.1
From: Sam Ravnborg <[email protected]>
sparc32 has no limited DMA zone so there is no need to select ZONE_DMA.
Based on analysis from Marciej:
"
Actually I think ZONE_DMA should go too (it's linked to GENERIC_ISA_DMA,
isn't it? -- cf. commit 5ac6da669e24 ("[PATCH] Set CONFIG_ZONE_DMA for
arches with GENERIC_ISA_DMA")), and the whole thing use:
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
The GENERIC_ISA_DMA option itself was added to arch/sparc/config.in with
2.5.31 as:
define_bool CONFIG_GENERIC_ISA_DMA y
despite of:
define_bool CONFIG_ISA n
"
The sparc32 code did not differ between ZONE_NORMAL and ZONE_DMA,
which confirms the above. This patch drop ZONE_DMA.
Signed-off-by: Sam Ravnborg <[email protected]>
Reported-by: "Maciej W. Rozycki" <[email protected]>
Cc: Andreas Larsson <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: Maciej W. Rozycki <[email protected]>
Cc: Arnd Bergmann <[email protected]>
---
arch/sparc/Kconfig | 1 -
arch/sparc/mm/srmmu.c | 1 -
2 files changed, 2 deletions(-)
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 734f23daecca..bdbde506c01e 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -62,7 +62,6 @@ config SPARC32
select HAVE_UID16
select LOCK_MM_AND_FIND_VMA
select OLD_SIGACTION
- select ZONE_DMA
config SPARC64
def_bool 64BIT
diff --git a/arch/sparc/mm/srmmu.c b/arch/sparc/mm/srmmu.c
index 852085ada368..7aae2f6f4973 100644
--- a/arch/sparc/mm/srmmu.c
+++ b/arch/sparc/mm/srmmu.c
@@ -975,7 +975,6 @@ void __init srmmu_paging_init(void)
{
unsigned long max_zone_pfn[MAX_NR_ZONES] = { 0 };
- max_zone_pfn[ZONE_DMA] = max_low_pfn;
max_zone_pfn[ZONE_NORMAL] = max_low_pfn;
max_zone_pfn[ZONE_HIGHMEM] = highend_pfn;
--
2.34.1
From: Sam Ravnborg <[email protected]>
Passing a datastructre marked _initconst to platform_driver_register()
is wrong. Drop the __initconst notation.
This fixes the following warnings:
WARNING: modpost: vmlinux: section mismatch in reference: grpci1_of_driver+0x30 (section: .data) -> grpci1_of_match (section: .init.rodata)
WARNING: modpost: vmlinux: section mismatch in reference: grpci2_of_driver+0x30 (section: .data) -> grpci2_of_match (section: .init.rodata)
Signed-off-by: Sam Ravnborg <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Andreas Larsson <[email protected]>
---
arch/sparc/kernel/leon_pci_grpci1.c | 2 +-
arch/sparc/kernel/leon_pci_grpci2.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/sparc/kernel/leon_pci_grpci1.c b/arch/sparc/kernel/leon_pci_grpci1.c
index 8700a0e3b0df..b2b639bee068 100644
--- a/arch/sparc/kernel/leon_pci_grpci1.c
+++ b/arch/sparc/kernel/leon_pci_grpci1.c
@@ -697,7 +697,7 @@ static int grpci1_of_probe(struct platform_device *ofdev)
return err;
}
-static const struct of_device_id grpci1_of_match[] __initconst = {
+static const struct of_device_id grpci1_of_match[] = {
{
.name = "GAISLER_PCIFBRG",
},
diff --git a/arch/sparc/kernel/leon_pci_grpci2.c b/arch/sparc/kernel/leon_pci_grpci2.c
index 282b49d496ea..9f662340b5b2 100644
--- a/arch/sparc/kernel/leon_pci_grpci2.c
+++ b/arch/sparc/kernel/leon_pci_grpci2.c
@@ -889,7 +889,7 @@ static int grpci2_of_probe(struct platform_device *ofdev)
return err;
}
-static const struct of_device_id grpci2_of_match[] __initconst = {
+static const struct of_device_id grpci2_of_match[] = {
{
.name = "GAISLER_GRPCI2",
},
--
2.34.1
From: Sam Ravnborg <[email protected]>
sparc32 do not support generic isa dma, so do not select the symbol.
This fixes the following warnings:
dma.c:70:5: error: no previous prototype for 'request_dma' [-Werror=missing-prototypes]
dma.c:88:6: error: no previous prototype for 'free_dma' [-Werror=missing-prototypes]
Signed-off-by: Sam Ravnborg <[email protected]>
Fixes: 0fcb70851fbf ("Makefile.extrawarn: turn on missing-prototypes globally")
Acked-by: Randy Dunlap <[email protected]>
Tested-by: Randy Dunlap <[email protected]> # build-tested
Cc: Andreas Larsson <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: Maciej W. Rozycki <[email protected]>
---
arch/sparc/Kconfig | 4 ----
1 file changed, 4 deletions(-)
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index bdbde506c01e..440e53d37d20 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -137,10 +137,6 @@ config HIGHMEM
default y if SPARC32
select KMAP_LOCAL
-config GENERIC_ISA_DMA
- bool
- default y if SPARC32
-
config PGTABLE_LEVELS
default 4 if 64BIT
default 3
--
2.34.1
From: Sam Ravnborg <[email protected]>
include/asm/parport.h is sparc64 specific.
Rename it to parport_64.h and use the generic version for sparc32.
This fixed all{mod,yes}config build errors like:
parport_pc.c:(.text):undefined-reference-to-ebus_dma_enable
parport_pc.c:(.text):undefined-reference-to-ebus_dma_irq_enable
parport_pc.c:(.text):undefined-reference-to-ebus_dma_register
The errors occur as the sparc32 build references sparc64 symbols.
Signed-off-by: Sam Ravnborg <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Andreas Larsson <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: Maciej W. Rozycki <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]/
Fixes: 66bcd06099bb ("parport_pc: Also enable driver for PCI systems")
Cc: [email protected] # v5.18+
---
arch/sparc/include/asm/parport.h | 259 +-----------------------------------
arch/sparc/include/asm/parport_64.h | 256 +++++++++++++++++++++++++++++++++++
2 files changed, 263 insertions(+), 252 deletions(-)
diff --git a/arch/sparc/include/asm/parport.h b/arch/sparc/include/asm/parport.h
index 0a7ffcfd59cd..e2eed8f97665 100644
--- a/arch/sparc/include/asm/parport.h
+++ b/arch/sparc/include/asm/parport.h
@@ -1,256 +1,11 @@
/* SPDX-License-Identifier: GPL-2.0 */
-/* parport.h: sparc64 specific parport initialization and dma.
- *
- * Copyright (C) 1999 Eddie C. Dost ([email protected])
- */
+#ifndef ___ASM_SPARC_PARPORT_H
+#define ___ASM_SPARC_PARPORT_H
-#ifndef _ASM_SPARC64_PARPORT_H
-#define _ASM_SPARC64_PARPORT_H 1
-
-#include <linux/of.h>
-#include <linux/platform_device.h>
-
-#include <asm/ebus_dma.h>
-#include <asm/ns87303.h>
-#include <asm/prom.h>
-
-#define PARPORT_PC_MAX_PORTS PARPORT_MAX
-
-/*
- * While sparc64 doesn't have an ISA DMA API, we provide something that looks
- * close enough to make parport_pc happy
- */
-#define HAS_DMA
-
-#ifdef CONFIG_PARPORT_PC_FIFO
-static DEFINE_SPINLOCK(dma_spin_lock);
-
-#define claim_dma_lock() \
-({ unsigned long flags; \
- spin_lock_irqsave(&dma_spin_lock, flags); \
- flags; \
-})
-
-#define release_dma_lock(__flags) \
- spin_unlock_irqrestore(&dma_spin_lock, __flags);
+#if defined(__sparc__) && defined(__arch64__)
+#include <asm/parport_64.h>
+#else
+#include <asm-generic/parport.h>
+#endif
#endif
-static struct sparc_ebus_info {
- struct ebus_dma_info info;
- unsigned int addr;
- unsigned int count;
- int lock;
-
- struct parport *port;
-} sparc_ebus_dmas[PARPORT_PC_MAX_PORTS];
-
-static DECLARE_BITMAP(dma_slot_map, PARPORT_PC_MAX_PORTS);
-
-static inline int request_dma(unsigned int dmanr, const char *device_id)
-{
- if (dmanr >= PARPORT_PC_MAX_PORTS)
- return -EINVAL;
- if (xchg(&sparc_ebus_dmas[dmanr].lock, 1) != 0)
- return -EBUSY;
- return 0;
-}
-
-static inline void free_dma(unsigned int dmanr)
-{
- if (dmanr >= PARPORT_PC_MAX_PORTS) {
- printk(KERN_WARNING "Trying to free DMA%d\n", dmanr);
- return;
- }
- if (xchg(&sparc_ebus_dmas[dmanr].lock, 0) == 0) {
- printk(KERN_WARNING "Trying to free free DMA%d\n", dmanr);
- return;
- }
-}
-
-static inline void enable_dma(unsigned int dmanr)
-{
- ebus_dma_enable(&sparc_ebus_dmas[dmanr].info, 1);
-
- if (ebus_dma_request(&sparc_ebus_dmas[dmanr].info,
- sparc_ebus_dmas[dmanr].addr,
- sparc_ebus_dmas[dmanr].count))
- BUG();
-}
-
-static inline void disable_dma(unsigned int dmanr)
-{
- ebus_dma_enable(&sparc_ebus_dmas[dmanr].info, 0);
-}
-
-static inline void clear_dma_ff(unsigned int dmanr)
-{
- /* nothing */
-}
-
-static inline void set_dma_mode(unsigned int dmanr, char mode)
-{
- ebus_dma_prepare(&sparc_ebus_dmas[dmanr].info, (mode != DMA_MODE_WRITE));
-}
-
-static inline void set_dma_addr(unsigned int dmanr, unsigned int addr)
-{
- sparc_ebus_dmas[dmanr].addr = addr;
-}
-
-static inline void set_dma_count(unsigned int dmanr, unsigned int count)
-{
- sparc_ebus_dmas[dmanr].count = count;
-}
-
-static inline unsigned int get_dma_residue(unsigned int dmanr)
-{
- return ebus_dma_residue(&sparc_ebus_dmas[dmanr].info);
-}
-
-static int ecpp_probe(struct platform_device *op)
-{
- unsigned long base = op->resource[0].start;
- unsigned long config = op->resource[1].start;
- unsigned long d_base = op->resource[2].start;
- unsigned long d_len;
- struct device_node *parent;
- struct parport *p;
- int slot, err;
-
- parent = op->dev.of_node->parent;
- if (of_node_name_eq(parent, "dma")) {
- p = parport_pc_probe_port(base, base + 0x400,
- op->archdata.irqs[0], PARPORT_DMA_NOFIFO,
- op->dev.parent->parent, 0);
- if (!p)
- return -ENOMEM;
- dev_set_drvdata(&op->dev, p);
- return 0;
- }
-
- for (slot = 0; slot < PARPORT_PC_MAX_PORTS; slot++) {
- if (!test_and_set_bit(slot, dma_slot_map))
- break;
- }
- err = -ENODEV;
- if (slot >= PARPORT_PC_MAX_PORTS)
- goto out_err;
-
- spin_lock_init(&sparc_ebus_dmas[slot].info.lock);
-
- d_len = (op->resource[2].end - d_base) + 1UL;
- sparc_ebus_dmas[slot].info.regs =
- of_ioremap(&op->resource[2], 0, d_len, "ECPP DMA");
-
- if (!sparc_ebus_dmas[slot].info.regs)
- goto out_clear_map;
-
- sparc_ebus_dmas[slot].info.flags = 0;
- sparc_ebus_dmas[slot].info.callback = NULL;
- sparc_ebus_dmas[slot].info.client_cookie = NULL;
- sparc_ebus_dmas[slot].info.irq = 0xdeadbeef;
- strcpy(sparc_ebus_dmas[slot].info.name, "parport");
- if (ebus_dma_register(&sparc_ebus_dmas[slot].info))
- goto out_unmap_regs;
-
- ebus_dma_irq_enable(&sparc_ebus_dmas[slot].info, 1);
-
- /* Configure IRQ to Push Pull, Level Low */
- /* Enable ECP, set bit 2 of the CTR first */
- outb(0x04, base + 0x02);
- ns87303_modify(config, PCR,
- PCR_EPP_ENABLE |
- PCR_IRQ_ODRAIN,
- PCR_ECP_ENABLE |
- PCR_ECP_CLK_ENA |
- PCR_IRQ_POLAR);
-
- /* CTR bit 5 controls direction of port */
- ns87303_modify(config, PTR,
- 0, PTR_LPT_REG_DIR);
-
- p = parport_pc_probe_port(base, base + 0x400,
- op->archdata.irqs[0],
- slot,
- op->dev.parent,
- 0);
- err = -ENOMEM;
- if (!p)
- goto out_disable_irq;
-
- dev_set_drvdata(&op->dev, p);
-
- return 0;
-
-out_disable_irq:
- ebus_dma_irq_enable(&sparc_ebus_dmas[slot].info, 0);
- ebus_dma_unregister(&sparc_ebus_dmas[slot].info);
-
-out_unmap_regs:
- of_iounmap(&op->resource[2], sparc_ebus_dmas[slot].info.regs, d_len);
-
-out_clear_map:
- clear_bit(slot, dma_slot_map);
-
-out_err:
- return err;
-}
-
-static int ecpp_remove(struct platform_device *op)
-{
- struct parport *p = dev_get_drvdata(&op->dev);
- int slot = p->dma;
-
- parport_pc_unregister_port(p);
-
- if (slot != PARPORT_DMA_NOFIFO) {
- unsigned long d_base = op->resource[2].start;
- unsigned long d_len;
-
- d_len = (op->resource[2].end - d_base) + 1UL;
-
- ebus_dma_irq_enable(&sparc_ebus_dmas[slot].info, 0);
- ebus_dma_unregister(&sparc_ebus_dmas[slot].info);
- of_iounmap(&op->resource[2],
- sparc_ebus_dmas[slot].info.regs,
- d_len);
- clear_bit(slot, dma_slot_map);
- }
-
- return 0;
-}
-
-static const struct of_device_id ecpp_match[] = {
- {
- .name = "ecpp",
- },
- {
- .name = "parallel",
- .compatible = "ecpp",
- },
- {
- .name = "parallel",
- .compatible = "ns87317-ecpp",
- },
- {
- .name = "parallel",
- .compatible = "pnpALI,1533,3",
- },
- {},
-};
-
-static struct platform_driver ecpp_driver = {
- .driver = {
- .name = "ecpp",
- .of_match_table = ecpp_match,
- },
- .probe = ecpp_probe,
- .remove = ecpp_remove,
-};
-
-static int parport_pc_find_nonpci_ports(int autoirq, int autodma)
-{
- return platform_driver_register(&ecpp_driver);
-}
-
-#endif /* !(_ASM_SPARC64_PARPORT_H */
diff --git a/arch/sparc/include/asm/parport_64.h b/arch/sparc/include/asm/parport_64.h
new file mode 100644
index 000000000000..0a7ffcfd59cd
--- /dev/null
+++ b/arch/sparc/include/asm/parport_64.h
@@ -0,0 +1,256 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* parport.h: sparc64 specific parport initialization and dma.
+ *
+ * Copyright (C) 1999 Eddie C. Dost ([email protected])
+ */
+
+#ifndef _ASM_SPARC64_PARPORT_H
+#define _ASM_SPARC64_PARPORT_H 1
+
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+#include <asm/ebus_dma.h>
+#include <asm/ns87303.h>
+#include <asm/prom.h>
+
+#define PARPORT_PC_MAX_PORTS PARPORT_MAX
+
+/*
+ * While sparc64 doesn't have an ISA DMA API, we provide something that looks
+ * close enough to make parport_pc happy
+ */
+#define HAS_DMA
+
+#ifdef CONFIG_PARPORT_PC_FIFO
+static DEFINE_SPINLOCK(dma_spin_lock);
+
+#define claim_dma_lock() \
+({ unsigned long flags; \
+ spin_lock_irqsave(&dma_spin_lock, flags); \
+ flags; \
+})
+
+#define release_dma_lock(__flags) \
+ spin_unlock_irqrestore(&dma_spin_lock, __flags);
+#endif
+
+static struct sparc_ebus_info {
+ struct ebus_dma_info info;
+ unsigned int addr;
+ unsigned int count;
+ int lock;
+
+ struct parport *port;
+} sparc_ebus_dmas[PARPORT_PC_MAX_PORTS];
+
+static DECLARE_BITMAP(dma_slot_map, PARPORT_PC_MAX_PORTS);
+
+static inline int request_dma(unsigned int dmanr, const char *device_id)
+{
+ if (dmanr >= PARPORT_PC_MAX_PORTS)
+ return -EINVAL;
+ if (xchg(&sparc_ebus_dmas[dmanr].lock, 1) != 0)
+ return -EBUSY;
+ return 0;
+}
+
+static inline void free_dma(unsigned int dmanr)
+{
+ if (dmanr >= PARPORT_PC_MAX_PORTS) {
+ printk(KERN_WARNING "Trying to free DMA%d\n", dmanr);
+ return;
+ }
+ if (xchg(&sparc_ebus_dmas[dmanr].lock, 0) == 0) {
+ printk(KERN_WARNING "Trying to free free DMA%d\n", dmanr);
+ return;
+ }
+}
+
+static inline void enable_dma(unsigned int dmanr)
+{
+ ebus_dma_enable(&sparc_ebus_dmas[dmanr].info, 1);
+
+ if (ebus_dma_request(&sparc_ebus_dmas[dmanr].info,
+ sparc_ebus_dmas[dmanr].addr,
+ sparc_ebus_dmas[dmanr].count))
+ BUG();
+}
+
+static inline void disable_dma(unsigned int dmanr)
+{
+ ebus_dma_enable(&sparc_ebus_dmas[dmanr].info, 0);
+}
+
+static inline void clear_dma_ff(unsigned int dmanr)
+{
+ /* nothing */
+}
+
+static inline void set_dma_mode(unsigned int dmanr, char mode)
+{
+ ebus_dma_prepare(&sparc_ebus_dmas[dmanr].info, (mode != DMA_MODE_WRITE));
+}
+
+static inline void set_dma_addr(unsigned int dmanr, unsigned int addr)
+{
+ sparc_ebus_dmas[dmanr].addr = addr;
+}
+
+static inline void set_dma_count(unsigned int dmanr, unsigned int count)
+{
+ sparc_ebus_dmas[dmanr].count = count;
+}
+
+static inline unsigned int get_dma_residue(unsigned int dmanr)
+{
+ return ebus_dma_residue(&sparc_ebus_dmas[dmanr].info);
+}
+
+static int ecpp_probe(struct platform_device *op)
+{
+ unsigned long base = op->resource[0].start;
+ unsigned long config = op->resource[1].start;
+ unsigned long d_base = op->resource[2].start;
+ unsigned long d_len;
+ struct device_node *parent;
+ struct parport *p;
+ int slot, err;
+
+ parent = op->dev.of_node->parent;
+ if (of_node_name_eq(parent, "dma")) {
+ p = parport_pc_probe_port(base, base + 0x400,
+ op->archdata.irqs[0], PARPORT_DMA_NOFIFO,
+ op->dev.parent->parent, 0);
+ if (!p)
+ return -ENOMEM;
+ dev_set_drvdata(&op->dev, p);
+ return 0;
+ }
+
+ for (slot = 0; slot < PARPORT_PC_MAX_PORTS; slot++) {
+ if (!test_and_set_bit(slot, dma_slot_map))
+ break;
+ }
+ err = -ENODEV;
+ if (slot >= PARPORT_PC_MAX_PORTS)
+ goto out_err;
+
+ spin_lock_init(&sparc_ebus_dmas[slot].info.lock);
+
+ d_len = (op->resource[2].end - d_base) + 1UL;
+ sparc_ebus_dmas[slot].info.regs =
+ of_ioremap(&op->resource[2], 0, d_len, "ECPP DMA");
+
+ if (!sparc_ebus_dmas[slot].info.regs)
+ goto out_clear_map;
+
+ sparc_ebus_dmas[slot].info.flags = 0;
+ sparc_ebus_dmas[slot].info.callback = NULL;
+ sparc_ebus_dmas[slot].info.client_cookie = NULL;
+ sparc_ebus_dmas[slot].info.irq = 0xdeadbeef;
+ strcpy(sparc_ebus_dmas[slot].info.name, "parport");
+ if (ebus_dma_register(&sparc_ebus_dmas[slot].info))
+ goto out_unmap_regs;
+
+ ebus_dma_irq_enable(&sparc_ebus_dmas[slot].info, 1);
+
+ /* Configure IRQ to Push Pull, Level Low */
+ /* Enable ECP, set bit 2 of the CTR first */
+ outb(0x04, base + 0x02);
+ ns87303_modify(config, PCR,
+ PCR_EPP_ENABLE |
+ PCR_IRQ_ODRAIN,
+ PCR_ECP_ENABLE |
+ PCR_ECP_CLK_ENA |
+ PCR_IRQ_POLAR);
+
+ /* CTR bit 5 controls direction of port */
+ ns87303_modify(config, PTR,
+ 0, PTR_LPT_REG_DIR);
+
+ p = parport_pc_probe_port(base, base + 0x400,
+ op->archdata.irqs[0],
+ slot,
+ op->dev.parent,
+ 0);
+ err = -ENOMEM;
+ if (!p)
+ goto out_disable_irq;
+
+ dev_set_drvdata(&op->dev, p);
+
+ return 0;
+
+out_disable_irq:
+ ebus_dma_irq_enable(&sparc_ebus_dmas[slot].info, 0);
+ ebus_dma_unregister(&sparc_ebus_dmas[slot].info);
+
+out_unmap_regs:
+ of_iounmap(&op->resource[2], sparc_ebus_dmas[slot].info.regs, d_len);
+
+out_clear_map:
+ clear_bit(slot, dma_slot_map);
+
+out_err:
+ return err;
+}
+
+static int ecpp_remove(struct platform_device *op)
+{
+ struct parport *p = dev_get_drvdata(&op->dev);
+ int slot = p->dma;
+
+ parport_pc_unregister_port(p);
+
+ if (slot != PARPORT_DMA_NOFIFO) {
+ unsigned long d_base = op->resource[2].start;
+ unsigned long d_len;
+
+ d_len = (op->resource[2].end - d_base) + 1UL;
+
+ ebus_dma_irq_enable(&sparc_ebus_dmas[slot].info, 0);
+ ebus_dma_unregister(&sparc_ebus_dmas[slot].info);
+ of_iounmap(&op->resource[2],
+ sparc_ebus_dmas[slot].info.regs,
+ d_len);
+ clear_bit(slot, dma_slot_map);
+ }
+
+ return 0;
+}
+
+static const struct of_device_id ecpp_match[] = {
+ {
+ .name = "ecpp",
+ },
+ {
+ .name = "parallel",
+ .compatible = "ecpp",
+ },
+ {
+ .name = "parallel",
+ .compatible = "ns87317-ecpp",
+ },
+ {
+ .name = "parallel",
+ .compatible = "pnpALI,1533,3",
+ },
+ {},
+};
+
+static struct platform_driver ecpp_driver = {
+ .driver = {
+ .name = "ecpp",
+ .of_match_table = ecpp_match,
+ },
+ .probe = ecpp_probe,
+ .remove = ecpp_remove,
+};
+
+static int parport_pc_find_nonpci_ports(int autoirq, int autodma)
+{
+ return platform_driver_register(&ecpp_driver);
+}
+
+#endif /* !(_ASM_SPARC64_PARPORT_H */
--
2.34.1
On Sun, Feb 25, 2024 at 12:57:42AM +0000, Maciej W. Rozycki wrote:
> On Sat, 24 Feb 2024, Sam Ravnborg via B4 Relay wrote:
>
> > sparc32 has no limited DMA zone so there is no need to select ZONE_DMA.
> >
> > Based on analysis from Marciej:
>
> Can you please use the correct spelling of my name in change descriptions
> (and preferably everywhere)? Thank you.
Sorry, will fix in v3.
Sam
Hi Sam,
[email protected] wrote on Sat, 24 Feb 2024 18:42:24
+0100:
> From: Sam Ravnborg <[email protected]>
>
> This fixes the following warning:
> sun_uflash.c:50:5: error: no previous prototype for 'uflash_devinit'
>
> Signed-off-by: Sam Ravnborg <[email protected]>
> Fixes: 0fcb70851fbf ("Makefile.extrawarn: turn on missing-prototypes globally")
> Reviewed-by: Randy Dunlap <[email protected]>
> Tested-by: Randy Dunlap <[email protected]> # build-tested
> Cc: Andreas Larsson <[email protected]>
> Cc: "David S. Miller" <[email protected]>
I was about to take this but linux-mtd is not Cced, so I'm letting this
to the Sparc maintainers. Please resend to the mtd list if I need to
take it.
Acked-by: Miquel Raynal <[email protected]>
Thanks,
Miquèl
On Sat, 24 Feb 2024, Sam Ravnborg via B4 Relay wrote:
> sparc32 has no limited DMA zone so there is no need to select ZONE_DMA.
>
> Based on analysis from Marciej:
Can you please use the correct spelling of my name in change descriptions
(and preferably everywhere)? Thank you.
Maciej
On 2/24/24 09:42, Sam Ravnborg via B4 Relay wrote:
> From: Sam Ravnborg <[email protected]>
>
> sparc32 has no limited DMA zone so there is no need to select ZONE_DMA.
>
> Based on analysis from Marciej:
> "
> Actually I think ZONE_DMA should go too (it's linked to GENERIC_ISA_DMA,
> isn't it? -- cf. commit 5ac6da669e24 ("[PATCH] Set CONFIG_ZONE_DMA for
> arches with GENERIC_ISA_DMA")), and the whole thing use:
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>
> The GENERIC_ISA_DMA option itself was added to arch/sparc/config.in with
> 2.5.31 as:
>
> define_bool CONFIG_GENERIC_ISA_DMA y
>
> despite of:
>
> define_bool CONFIG_ISA n
> "
>
> The sparc32 code did not differ between ZONE_NORMAL and ZONE_DMA,
> which confirms the above. This patch drop ZONE_DMA.
>
> Signed-off-by: Sam Ravnborg <[email protected]>
> Reported-by: "Maciej W. Rozycki" <[email protected]>
> Cc: Andreas Larsson <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Randy Dunlap <[email protected]>
> Cc: Maciej W. Rozycki <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
Tested-by: Randy Dunlap <[email protected]> # build-tested
Thanks.
> ---
> arch/sparc/Kconfig | 1 -
> arch/sparc/mm/srmmu.c | 1 -
> 2 files changed, 2 deletions(-)
>
> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> index 734f23daecca..bdbde506c01e 100644
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -62,7 +62,6 @@ config SPARC32
> select HAVE_UID16
> select LOCK_MM_AND_FIND_VMA
> select OLD_SIGACTION
> - select ZONE_DMA
>
> config SPARC64
> def_bool 64BIT
> diff --git a/arch/sparc/mm/srmmu.c b/arch/sparc/mm/srmmu.c
> index 852085ada368..7aae2f6f4973 100644
> --- a/arch/sparc/mm/srmmu.c
> +++ b/arch/sparc/mm/srmmu.c
> @@ -975,7 +975,6 @@ void __init srmmu_paging_init(void)
> {
> unsigned long max_zone_pfn[MAX_NR_ZONES] = { 0 };
>
> - max_zone_pfn[ZONE_DMA] = max_low_pfn;
> max_zone_pfn[ZONE_NORMAL] = max_low_pfn;
> max_zone_pfn[ZONE_HIGHMEM] = highend_pfn;
>
>
--
#Randy
On 2/24/24 09:42, Sam Ravnborg via B4 Relay wrote:
> From: Sam Ravnborg <[email protected]>
>
> Passing a datastructre marked _initconst to platform_driver_register()
> is wrong. Drop the __initconst notation.
>
> This fixes the following warnings:
>
> WARNING: modpost: vmlinux: section mismatch in reference: grpci1_of_driver+0x30 (section: .data) -> grpci1_of_match (section: .init.rodata)
> WARNING: modpost: vmlinux: section mismatch in reference: grpci2_of_driver+0x30 (section: .data) -> grpci2_of_match (section: .init.rodata)
>
> Signed-off-by: Sam Ravnborg <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Andreas Larsson <[email protected]>
Tested-by: Randy Dunlap <[email protected]> # build-tested
Thanks.
> ---
> arch/sparc/kernel/leon_pci_grpci1.c | 2 +-
> arch/sparc/kernel/leon_pci_grpci2.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/sparc/kernel/leon_pci_grpci1.c b/arch/sparc/kernel/leon_pci_grpci1.c
> index 8700a0e3b0df..b2b639bee068 100644
> --- a/arch/sparc/kernel/leon_pci_grpci1.c
> +++ b/arch/sparc/kernel/leon_pci_grpci1.c
> @@ -697,7 +697,7 @@ static int grpci1_of_probe(struct platform_device *ofdev)
> return err;
> }
>
> -static const struct of_device_id grpci1_of_match[] __initconst = {
> +static const struct of_device_id grpci1_of_match[] = {
> {
> .name = "GAISLER_PCIFBRG",
> },
> diff --git a/arch/sparc/kernel/leon_pci_grpci2.c b/arch/sparc/kernel/leon_pci_grpci2.c
> index 282b49d496ea..9f662340b5b2 100644
> --- a/arch/sparc/kernel/leon_pci_grpci2.c
> +++ b/arch/sparc/kernel/leon_pci_grpci2.c
> @@ -889,7 +889,7 @@ static int grpci2_of_probe(struct platform_device *ofdev)
> return err;
> }
>
> -static const struct of_device_id grpci2_of_match[] __initconst = {
> +static const struct of_device_id grpci2_of_match[] = {
> {
> .name = "GAISLER_GRPCI2",
> },
>
--
#Randy
On 2/24/24 09:42, Sam Ravnborg via B4 Relay wrote:
> From: Sam Ravnborg <[email protected]>
>
> include/asm/parport.h is sparc64 specific.
> Rename it to parport_64.h and use the generic version for sparc32.
>
> This fixed all{mod,yes}config build errors like:
>
> parport_pc.c:(.text):undefined-reference-to-ebus_dma_enable
> parport_pc.c:(.text):undefined-reference-to-ebus_dma_irq_enable
> parport_pc.c:(.text):undefined-reference-to-ebus_dma_register
>
> The errors occur as the sparc32 build references sparc64 symbols.
>
> Signed-off-by: Sam Ravnborg <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Andreas Larsson <[email protected]>
> Cc: Randy Dunlap <[email protected]>
> Cc: Maciej W. Rozycki <[email protected]>
> Closes: https://lore.kernel.org/r/[email protected]/
> Fixes: 66bcd06099bb ("parport_pc: Also enable driver for PCI systems")
> Cc: [email protected] # v5.18+
Tested-by: Randy Dunlap <[email protected]> # build-tested
Thanks.
> ---
> arch/sparc/include/asm/parport.h | 259 +-----------------------------------
> arch/sparc/include/asm/parport_64.h | 256 +++++++++++++++++++++++++++++++++++
> 2 files changed, 263 insertions(+), 252 deletions(-)
>
> diff --git a/arch/sparc/include/asm/parport.h b/arch/sparc/include/asm/parport.h
> index 0a7ffcfd59cd..e2eed8f97665 100644
> --- a/arch/sparc/include/asm/parport.h
> +++ b/arch/sparc/include/asm/parport.h
> @@ -1,256 +1,11 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> -/* parport.h: sparc64 specific parport initialization and dma.
> - *
> - * Copyright (C) 1999 Eddie C. Dost ([email protected])
> - */
> +#ifndef ___ASM_SPARC_PARPORT_H
> +#define ___ASM_SPARC_PARPORT_H
>
> -#ifndef _ASM_SPARC64_PARPORT_H
> -#define _ASM_SPARC64_PARPORT_H 1
> -
> -#include <linux/of.h>
> -#include <linux/platform_device.h>
> -
> -#include <asm/ebus_dma.h>
> -#include <asm/ns87303.h>
> -#include <asm/prom.h>
> -
> -#define PARPORT_PC_MAX_PORTS PARPORT_MAX
> -
> -/*
> - * While sparc64 doesn't have an ISA DMA API, we provide something that looks
> - * close enough to make parport_pc happy
> - */
> -#define HAS_DMA
> -
> -#ifdef CONFIG_PARPORT_PC_FIFO
> -static DEFINE_SPINLOCK(dma_spin_lock);
> -
> -#define claim_dma_lock() \
> -({ unsigned long flags; \
> - spin_lock_irqsave(&dma_spin_lock, flags); \
> - flags; \
> -})
> -
> -#define release_dma_lock(__flags) \
> - spin_unlock_irqrestore(&dma_spin_lock, __flags);
> +#if defined(__sparc__) && defined(__arch64__)
> +#include <asm/parport_64.h>
> +#else
> +#include <asm-generic/parport.h>
> +#endif
> #endif
>
> -static struct sparc_ebus_info {
> - struct ebus_dma_info info;
> - unsigned int addr;
> - unsigned int count;
> - int lock;
> -
> - struct parport *port;
> -} sparc_ebus_dmas[PARPORT_PC_MAX_PORTS];
> -
> -static DECLARE_BITMAP(dma_slot_map, PARPORT_PC_MAX_PORTS);
> -
> -static inline int request_dma(unsigned int dmanr, const char *device_id)
> -{
> - if (dmanr >= PARPORT_PC_MAX_PORTS)
> - return -EINVAL;
> - if (xchg(&sparc_ebus_dmas[dmanr].lock, 1) != 0)
> - return -EBUSY;
> - return 0;
> -}
> -
> -static inline void free_dma(unsigned int dmanr)
> -{
> - if (dmanr >= PARPORT_PC_MAX_PORTS) {
> - printk(KERN_WARNING "Trying to free DMA%d\n", dmanr);
> - return;
> - }
> - if (xchg(&sparc_ebus_dmas[dmanr].lock, 0) == 0) {
> - printk(KERN_WARNING "Trying to free free DMA%d\n", dmanr);
> - return;
> - }
> -}
> -
> -static inline void enable_dma(unsigned int dmanr)
> -{
> - ebus_dma_enable(&sparc_ebus_dmas[dmanr].info, 1);
> -
> - if (ebus_dma_request(&sparc_ebus_dmas[dmanr].info,
> - sparc_ebus_dmas[dmanr].addr,
> - sparc_ebus_dmas[dmanr].count))
> - BUG();
> -}
> -
> -static inline void disable_dma(unsigned int dmanr)
> -{
> - ebus_dma_enable(&sparc_ebus_dmas[dmanr].info, 0);
> -}
> -
> -static inline void clear_dma_ff(unsigned int dmanr)
> -{
> - /* nothing */
> -}
> -
> -static inline void set_dma_mode(unsigned int dmanr, char mode)
> -{
> - ebus_dma_prepare(&sparc_ebus_dmas[dmanr].info, (mode != DMA_MODE_WRITE));
> -}
> -
> -static inline void set_dma_addr(unsigned int dmanr, unsigned int addr)
> -{
> - sparc_ebus_dmas[dmanr].addr = addr;
> -}
> -
> -static inline void set_dma_count(unsigned int dmanr, unsigned int count)
> -{
> - sparc_ebus_dmas[dmanr].count = count;
> -}
> -
> -static inline unsigned int get_dma_residue(unsigned int dmanr)
> -{
> - return ebus_dma_residue(&sparc_ebus_dmas[dmanr].info);
> -}
> -
> -static int ecpp_probe(struct platform_device *op)
> -{
> - unsigned long base = op->resource[0].start;
> - unsigned long config = op->resource[1].start;
> - unsigned long d_base = op->resource[2].start;
> - unsigned long d_len;
> - struct device_node *parent;
> - struct parport *p;
> - int slot, err;
> -
> - parent = op->dev.of_node->parent;
> - if (of_node_name_eq(parent, "dma")) {
> - p = parport_pc_probe_port(base, base + 0x400,
> - op->archdata.irqs[0], PARPORT_DMA_NOFIFO,
> - op->dev.parent->parent, 0);
> - if (!p)
> - return -ENOMEM;
> - dev_set_drvdata(&op->dev, p);
> - return 0;
> - }
> -
> - for (slot = 0; slot < PARPORT_PC_MAX_PORTS; slot++) {
> - if (!test_and_set_bit(slot, dma_slot_map))
> - break;
> - }
> - err = -ENODEV;
> - if (slot >= PARPORT_PC_MAX_PORTS)
> - goto out_err;
> -
> - spin_lock_init(&sparc_ebus_dmas[slot].info.lock);
> -
> - d_len = (op->resource[2].end - d_base) + 1UL;
> - sparc_ebus_dmas[slot].info.regs =
> - of_ioremap(&op->resource[2], 0, d_len, "ECPP DMA");
> -
> - if (!sparc_ebus_dmas[slot].info.regs)
> - goto out_clear_map;
> -
> - sparc_ebus_dmas[slot].info.flags = 0;
> - sparc_ebus_dmas[slot].info.callback = NULL;
> - sparc_ebus_dmas[slot].info.client_cookie = NULL;
> - sparc_ebus_dmas[slot].info.irq = 0xdeadbeef;
> - strcpy(sparc_ebus_dmas[slot].info.name, "parport");
> - if (ebus_dma_register(&sparc_ebus_dmas[slot].info))
> - goto out_unmap_regs;
> -
> - ebus_dma_irq_enable(&sparc_ebus_dmas[slot].info, 1);
> -
> - /* Configure IRQ to Push Pull, Level Low */
> - /* Enable ECP, set bit 2 of the CTR first */
> - outb(0x04, base + 0x02);
> - ns87303_modify(config, PCR,
> - PCR_EPP_ENABLE |
> - PCR_IRQ_ODRAIN,
> - PCR_ECP_ENABLE |
> - PCR_ECP_CLK_ENA |
> - PCR_IRQ_POLAR);
> -
> - /* CTR bit 5 controls direction of port */
> - ns87303_modify(config, PTR,
> - 0, PTR_LPT_REG_DIR);
> -
> - p = parport_pc_probe_port(base, base + 0x400,
> - op->archdata.irqs[0],
> - slot,
> - op->dev.parent,
> - 0);
> - err = -ENOMEM;
> - if (!p)
> - goto out_disable_irq;
> -
> - dev_set_drvdata(&op->dev, p);
> -
> - return 0;
> -
> -out_disable_irq:
> - ebus_dma_irq_enable(&sparc_ebus_dmas[slot].info, 0);
> - ebus_dma_unregister(&sparc_ebus_dmas[slot].info);
> -
> -out_unmap_regs:
> - of_iounmap(&op->resource[2], sparc_ebus_dmas[slot].info.regs, d_len);
> -
> -out_clear_map:
> - clear_bit(slot, dma_slot_map);
> -
> -out_err:
> - return err;
> -}
> -
> -static int ecpp_remove(struct platform_device *op)
> -{
> - struct parport *p = dev_get_drvdata(&op->dev);
> - int slot = p->dma;
> -
> - parport_pc_unregister_port(p);
> -
> - if (slot != PARPORT_DMA_NOFIFO) {
> - unsigned long d_base = op->resource[2].start;
> - unsigned long d_len;
> -
> - d_len = (op->resource[2].end - d_base) + 1UL;
> -
> - ebus_dma_irq_enable(&sparc_ebus_dmas[slot].info, 0);
> - ebus_dma_unregister(&sparc_ebus_dmas[slot].info);
> - of_iounmap(&op->resource[2],
> - sparc_ebus_dmas[slot].info.regs,
> - d_len);
> - clear_bit(slot, dma_slot_map);
> - }
> -
> - return 0;
> -}
> -
> -static const struct of_device_id ecpp_match[] = {
> - {
> - .name = "ecpp",
> - },
> - {
> - .name = "parallel",
> - .compatible = "ecpp",
> - },
> - {
> - .name = "parallel",
> - .compatible = "ns87317-ecpp",
> - },
> - {
> - .name = "parallel",
> - .compatible = "pnpALI,1533,3",
> - },
> - {},
> -};
> -
> -static struct platform_driver ecpp_driver = {
> - .driver = {
> - .name = "ecpp",
> - .of_match_table = ecpp_match,
> - },
> - .probe = ecpp_probe,
> - .remove = ecpp_remove,
> -};
> -
> -static int parport_pc_find_nonpci_ports(int autoirq, int autodma)
> -{
> - return platform_driver_register(&ecpp_driver);
> -}
> -
> -#endif /* !(_ASM_SPARC64_PARPORT_H */
> diff --git a/arch/sparc/include/asm/parport_64.h b/arch/sparc/include/asm/parport_64.h
> new file mode 100644
> index 000000000000..0a7ffcfd59cd
> --- /dev/null
> +++ b/arch/sparc/include/asm/parport_64.h
> @@ -0,0 +1,256 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* parport.h: sparc64 specific parport initialization and dma.
> + *
> + * Copyright (C) 1999 Eddie C. Dost ([email protected])
> + */
> +
> +#ifndef _ASM_SPARC64_PARPORT_H
> +#define _ASM_SPARC64_PARPORT_H 1
> +
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +#include <asm/ebus_dma.h>
> +#include <asm/ns87303.h>
> +#include <asm/prom.h>
> +
> +#define PARPORT_PC_MAX_PORTS PARPORT_MAX
> +
> +/*
> + * While sparc64 doesn't have an ISA DMA API, we provide something that looks
> + * close enough to make parport_pc happy
> + */
> +#define HAS_DMA
> +
> +#ifdef CONFIG_PARPORT_PC_FIFO
> +static DEFINE_SPINLOCK(dma_spin_lock);
> +
> +#define claim_dma_lock() \
> +({ unsigned long flags; \
> + spin_lock_irqsave(&dma_spin_lock, flags); \
> + flags; \
> +})
> +
> +#define release_dma_lock(__flags) \
> + spin_unlock_irqrestore(&dma_spin_lock, __flags);
> +#endif
> +
> +static struct sparc_ebus_info {
> + struct ebus_dma_info info;
> + unsigned int addr;
> + unsigned int count;
> + int lock;
> +
> + struct parport *port;
> +} sparc_ebus_dmas[PARPORT_PC_MAX_PORTS];
> +
> +static DECLARE_BITMAP(dma_slot_map, PARPORT_PC_MAX_PORTS);
> +
> +static inline int request_dma(unsigned int dmanr, const char *device_id)
> +{
> + if (dmanr >= PARPORT_PC_MAX_PORTS)
> + return -EINVAL;
> + if (xchg(&sparc_ebus_dmas[dmanr].lock, 1) != 0)
> + return -EBUSY;
> + return 0;
> +}
> +
> +static inline void free_dma(unsigned int dmanr)
> +{
> + if (dmanr >= PARPORT_PC_MAX_PORTS) {
> + printk(KERN_WARNING "Trying to free DMA%d\n", dmanr);
> + return;
> + }
> + if (xchg(&sparc_ebus_dmas[dmanr].lock, 0) == 0) {
> + printk(KERN_WARNING "Trying to free free DMA%d\n", dmanr);
> + return;
> + }
> +}
> +
> +static inline void enable_dma(unsigned int dmanr)
> +{
> + ebus_dma_enable(&sparc_ebus_dmas[dmanr].info, 1);
> +
> + if (ebus_dma_request(&sparc_ebus_dmas[dmanr].info,
> + sparc_ebus_dmas[dmanr].addr,
> + sparc_ebus_dmas[dmanr].count))
> + BUG();
> +}
> +
> +static inline void disable_dma(unsigned int dmanr)
> +{
> + ebus_dma_enable(&sparc_ebus_dmas[dmanr].info, 0);
> +}
> +
> +static inline void clear_dma_ff(unsigned int dmanr)
> +{
> + /* nothing */
> +}
> +
> +static inline void set_dma_mode(unsigned int dmanr, char mode)
> +{
> + ebus_dma_prepare(&sparc_ebus_dmas[dmanr].info, (mode != DMA_MODE_WRITE));
> +}
> +
> +static inline void set_dma_addr(unsigned int dmanr, unsigned int addr)
> +{
> + sparc_ebus_dmas[dmanr].addr = addr;
> +}
> +
> +static inline void set_dma_count(unsigned int dmanr, unsigned int count)
> +{
> + sparc_ebus_dmas[dmanr].count = count;
> +}
> +
> +static inline unsigned int get_dma_residue(unsigned int dmanr)
> +{
> + return ebus_dma_residue(&sparc_ebus_dmas[dmanr].info);
> +}
> +
> +static int ecpp_probe(struct platform_device *op)
> +{
> + unsigned long base = op->resource[0].start;
> + unsigned long config = op->resource[1].start;
> + unsigned long d_base = op->resource[2].start;
> + unsigned long d_len;
> + struct device_node *parent;
> + struct parport *p;
> + int slot, err;
> +
> + parent = op->dev.of_node->parent;
> + if (of_node_name_eq(parent, "dma")) {
> + p = parport_pc_probe_port(base, base + 0x400,
> + op->archdata.irqs[0], PARPORT_DMA_NOFIFO,
> + op->dev.parent->parent, 0);
> + if (!p)
> + return -ENOMEM;
> + dev_set_drvdata(&op->dev, p);
> + return 0;
> + }
> +
> + for (slot = 0; slot < PARPORT_PC_MAX_PORTS; slot++) {
> + if (!test_and_set_bit(slot, dma_slot_map))
> + break;
> + }
> + err = -ENODEV;
> + if (slot >= PARPORT_PC_MAX_PORTS)
> + goto out_err;
> +
> + spin_lock_init(&sparc_ebus_dmas[slot].info.lock);
> +
> + d_len = (op->resource[2].end - d_base) + 1UL;
> + sparc_ebus_dmas[slot].info.regs =
> + of_ioremap(&op->resource[2], 0, d_len, "ECPP DMA");
> +
> + if (!sparc_ebus_dmas[slot].info.regs)
> + goto out_clear_map;
> +
> + sparc_ebus_dmas[slot].info.flags = 0;
> + sparc_ebus_dmas[slot].info.callback = NULL;
> + sparc_ebus_dmas[slot].info.client_cookie = NULL;
> + sparc_ebus_dmas[slot].info.irq = 0xdeadbeef;
> + strcpy(sparc_ebus_dmas[slot].info.name, "parport");
> + if (ebus_dma_register(&sparc_ebus_dmas[slot].info))
> + goto out_unmap_regs;
> +
> + ebus_dma_irq_enable(&sparc_ebus_dmas[slot].info, 1);
> +
> + /* Configure IRQ to Push Pull, Level Low */
> + /* Enable ECP, set bit 2 of the CTR first */
> + outb(0x04, base + 0x02);
> + ns87303_modify(config, PCR,
> + PCR_EPP_ENABLE |
> + PCR_IRQ_ODRAIN,
> + PCR_ECP_ENABLE |
> + PCR_ECP_CLK_ENA |
> + PCR_IRQ_POLAR);
> +
> + /* CTR bit 5 controls direction of port */
> + ns87303_modify(config, PTR,
> + 0, PTR_LPT_REG_DIR);
> +
> + p = parport_pc_probe_port(base, base + 0x400,
> + op->archdata.irqs[0],
> + slot,
> + op->dev.parent,
> + 0);
> + err = -ENOMEM;
> + if (!p)
> + goto out_disable_irq;
> +
> + dev_set_drvdata(&op->dev, p);
> +
> + return 0;
> +
> +out_disable_irq:
> + ebus_dma_irq_enable(&sparc_ebus_dmas[slot].info, 0);
> + ebus_dma_unregister(&sparc_ebus_dmas[slot].info);
> +
> +out_unmap_regs:
> + of_iounmap(&op->resource[2], sparc_ebus_dmas[slot].info.regs, d_len);
> +
> +out_clear_map:
> + clear_bit(slot, dma_slot_map);
> +
> +out_err:
> + return err;
> +}
> +
> +static int ecpp_remove(struct platform_device *op)
> +{
> + struct parport *p = dev_get_drvdata(&op->dev);
> + int slot = p->dma;
> +
> + parport_pc_unregister_port(p);
> +
> + if (slot != PARPORT_DMA_NOFIFO) {
> + unsigned long d_base = op->resource[2].start;
> + unsigned long d_len;
> +
> + d_len = (op->resource[2].end - d_base) + 1UL;
> +
> + ebus_dma_irq_enable(&sparc_ebus_dmas[slot].info, 0);
> + ebus_dma_unregister(&sparc_ebus_dmas[slot].info);
> + of_iounmap(&op->resource[2],
> + sparc_ebus_dmas[slot].info.regs,
> + d_len);
> + clear_bit(slot, dma_slot_map);
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id ecpp_match[] = {
> + {
> + .name = "ecpp",
> + },
> + {
> + .name = "parallel",
> + .compatible = "ecpp",
> + },
> + {
> + .name = "parallel",
> + .compatible = "ns87317-ecpp",
> + },
> + {
> + .name = "parallel",
> + .compatible = "pnpALI,1533,3",
> + },
> + {},
> +};
> +
> +static struct platform_driver ecpp_driver = {
> + .driver = {
> + .name = "ecpp",
> + .of_match_table = ecpp_match,
> + },
> + .probe = ecpp_probe,
> + .remove = ecpp_remove,
> +};
> +
> +static int parport_pc_find_nonpci_ports(int autoirq, int autodma)
> +{
> + return platform_driver_register(&ecpp_driver);
> +}
> +
> +#endif /* !(_ASM_SPARC64_PARPORT_H */
>
--
#Randy
On 2024-02-24 18:42, Sam Ravnborg via B4 Relay wrote:
> From: Sam Ravnborg <[email protected]>
>
> Passing a datastructre marked _initconst to platform_driver_register()
> is wrong. Drop the __initconst notation.
>
> This fixes the following warnings:
>
> WARNING: modpost: vmlinux: section mismatch in reference: grpci1_of_driver+0x30 (section: .data) -> grpci1_of_match (section: .init.rodata)
> WARNING: modpost: vmlinux: section mismatch in reference: grpci2_of_driver+0x30 (section: .data) -> grpci2_of_match (section: .init.rodata)
>
> Signed-off-by: Sam Ravnborg <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Andreas Larsson <[email protected]>
Could you also add
Fixes: 4154bb821f0b ("sparc: leon: grpci1: constify of_device_id")
Fixes: 03949b1cb9f1 ("sparc: leon: grpci2: constify of_device_id")
for these fixes in your v3 of the series as well as CCing stable?
> ---
> arch/sparc/kernel/leon_pci_grpci1.c | 2 +-
> arch/sparc/kernel/leon_pci_grpci2.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/sparc/kernel/leon_pci_grpci1.c b/arch/sparc/kernel/leon_pci_grpci1.c
> index 8700a0e3b0df..b2b639bee068 100644
> --- a/arch/sparc/kernel/leon_pci_grpci1.c
> +++ b/arch/sparc/kernel/leon_pci_grpci1.c
> @@ -697,7 +697,7 @@ static int grpci1_of_probe(struct platform_device *ofdev)
> return err;
> }
>
> -static const struct of_device_id grpci1_of_match[] __initconst = {
> +static const struct of_device_id grpci1_of_match[] = {
> {
> .name = "GAISLER_PCIFBRG",
> },
> diff --git a/arch/sparc/kernel/leon_pci_grpci2.c b/arch/sparc/kernel/leon_pci_grpci2.c
> index 282b49d496ea..9f662340b5b2 100644
> --- a/arch/sparc/kernel/leon_pci_grpci2.c
> +++ b/arch/sparc/kernel/leon_pci_grpci2.c
> @@ -889,7 +889,7 @@ static int grpci2_of_probe(struct platform_device *ofdev)
> return err;
> }
>
> -static const struct of_device_id grpci2_of_match[] __initconst = {
> +static const struct of_device_id grpci2_of_match[] = {
> {
> .name = "GAISLER_GRPCI2",
> },
>
Thanks,
Andreas
On 2024-02-24 18:42, Sam Ravnborg via B4 Relay wrote:
> From: Sam Ravnborg <[email protected]>
>
> sparc32 has no limited DMA zone so there is no need to select ZONE_DMA.
>
> Based on analysis from Marciej:
> "
> Actually I think ZONE_DMA should go too (it's linked to GENERIC_ISA_DMA,
> isn't it? -- cf. commit 5ac6da669e24 ("[PATCH] Set CONFIG_ZONE_DMA for
> arches with GENERIC_ISA_DMA")), and the whole thing use:
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>
> The GENERIC_ISA_DMA option itself was added to arch/sparc/config.in with
> 2.5.31 as:
>
> define_bool CONFIG_GENERIC_ISA_DMA y
>
> despite of:
>
> define_bool CONFIG_ISA n
> "
>
> The sparc32 code did not differ between ZONE_NORMAL and ZONE_DMA,
> which confirms the above. This patch drop ZONE_DMA.
>
> Signed-off-by: Sam Ravnborg <[email protected]>
> Reported-by: "Maciej W. Rozycki" <[email protected]>
> Cc: Andreas Larsson <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Randy Dunlap <[email protected]>
> Cc: Maciej W. Rozycki <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> ---
> arch/sparc/Kconfig | 1 -
> arch/sparc/mm/srmmu.c | 1 -
> 2 files changed, 2 deletions(-)
>
> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> index 734f23daecca..bdbde506c01e 100644
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -62,7 +62,6 @@ config SPARC32
> select HAVE_UID16
> select LOCK_MM_AND_FIND_VMA
> select OLD_SIGACTION
> - select ZONE_DMA
This however makes a number of PCI drivers that depend on
ZONE_DMA unselectable.
>
> config SPARC64
> def_bool 64BIT
> diff --git a/arch/sparc/mm/srmmu.c b/arch/sparc/mm/srmmu.c
> index 852085ada368..7aae2f6f4973 100644
> --- a/arch/sparc/mm/srmmu.c
> +++ b/arch/sparc/mm/srmmu.c
> @@ -975,7 +975,6 @@ void __init srmmu_paging_init(void)
> {
> unsigned long max_zone_pfn[MAX_NR_ZONES] = { 0 };
>
> - max_zone_pfn[ZONE_DMA] = max_low_pfn;
> max_zone_pfn[ZONE_NORMAL] = max_low_pfn;
> max_zone_pfn[ZONE_HIGHMEM] = highend_pfn;
>
>
Thanks,
Andreas
On Tue, Mar 5, 2024, at 16:06, Andreas Larsson wrote:
> On 2024-02-24 18:42, Sam Ravnborg via B4 Relay wrote:
>> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
>> index 734f23daecca..bdbde506c01e 100644
>> --- a/arch/sparc/Kconfig
>> +++ b/arch/sparc/Kconfig
>> @@ -62,7 +62,6 @@ config SPARC32
>> select HAVE_UID16
>> select LOCK_MM_AND_FIND_VMA
>> select OLD_SIGACTION
>> - select ZONE_DMA
>
> This however makes a number of PCI drivers that depend on
> ZONE_DMA unselectable.
I think that is the correct thing to do then: the only
drivers that I see with this dependency are PCI sound cards
that apparently rely on DMA to the 16MB ISA range, which is
not provided by sparc.
Arnd
On 2024-03-05 16:26, Arnd Bergmann wrote:
> On Tue, Mar 5, 2024, at 16:06, Andreas Larsson wrote:
>> On 2024-02-24 18:42, Sam Ravnborg via B4 Relay wrote:
>
>>> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
>>> index 734f23daecca..bdbde506c01e 100644
>>> --- a/arch/sparc/Kconfig
>>> +++ b/arch/sparc/Kconfig
>>> @@ -62,7 +62,6 @@ config SPARC32
>>> select HAVE_UID16
>>> select LOCK_MM_AND_FIND_VMA
>>> select OLD_SIGACTION
>>> - select ZONE_DMA
>>
>> This however makes a number of PCI drivers that depend on
>> ZONE_DMA unselectable.
>
> I think that is the correct thing to do then: the only
> drivers that I see with this dependency are PCI sound cards
> that apparently rely on DMA to the 16MB ISA range, which is
> not provided by sparc.
The ZONE_DMA dependency does not seem related to ISA per se. Commit
80ab8eae70e5 ("ALSA: Enable CONFIG_ZONE_DMA for smaller PCI DMA masks")
that started to introduce it did were about ensuring 32-bit masks.
Some of those sound card drivers sets a 24 bit mask, i.e. a 0-16MB
range, but some among those sets a 28, and 30 bit DMA mask with
dma_set_mask_and_coherent. Testing, in a different driver, setting and
allocating under a 30-bit DMA mask (or even a 28-bit DMA mask depending
on where the physical memory resides) is possible before removing
ZONE_DMA, but not after.
I am also a bit concerned if removing ZONE_DMA will let DMA be allocated
in highmem and what that could lead to.
Cheers,
Andreas
On Wed, Mar 6, 2024, at 15:19, Andreas Larsson wrote:
> On 2024-03-05 16:26, Arnd Bergmann wrote:
>> On Tue, Mar 5, 2024, at 16:06, Andreas Larsson wrote:
>>> On 2024-02-24 18:42, Sam Ravnborg via B4 Relay wrote:
>>
>>>> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
>>>> index 734f23daecca..bdbde506c01e 100644
>>>> --- a/arch/sparc/Kconfig
>>>> +++ b/arch/sparc/Kconfig
>>>> @@ -62,7 +62,6 @@ config SPARC32
>>>> select HAVE_UID16
>>>> select LOCK_MM_AND_FIND_VMA
>>>> select OLD_SIGACTION
>>>> - select ZONE_DMA
>>>
>>> This however makes a number of PCI drivers that depend on
>>> ZONE_DMA unselectable.
>>
>> I think that is the correct thing to do then: the only
>> drivers that I see with this dependency are PCI sound cards
>> that apparently rely on DMA to the 16MB ISA range, which is
>> not provided by sparc.
>
> The ZONE_DMA dependency does not seem related to ISA per se. Commit
> 80ab8eae70e5 ("ALSA: Enable CONFIG_ZONE_DMA for smaller PCI DMA masks")
> that started to introduce it did were about ensuring 32-bit masks.
>
> Some of those sound card drivers sets a 24 bit mask, i.e. a 0-16MB
> range, but some among those sets a 28, and 30 bit DMA mask with
> dma_set_mask_and_coherent.
Ah right, I see it now.
> Testing, in a different driver, setting and
> allocating under a 30-bit DMA mask (or even a 28-bit DMA mask depending
> on where the physical memory resides) is possible before removing
> ZONE_DMA, but not after.
I still don't see how that changes anything if
max_zone_pfn[ZONE_DMA] and max_zone_pfn[ZONE_NORMAL] are set
to the same value. Did you test this on a mainline kernel, or
do you have any patches on top that might have set up
the zones differently?
More specifically, what do you see in the boot log for the
size of each zone?
> I am also a bit concerned if removing ZONE_DMA will let DMA be allocated
> in highmem and what that could lead to.
It's not supposed to make a difference, but this is a bit
more complex:
- Any kernel allocation (kmalloc etc) by definition comes from
lowmem, regardless of GFP_DMA/GFP_KERNEL.
- user pages that get mapped using dma_map_sg() or dma_map_page()
can be in highmem, but this should not depend on the presence
of ZONE_DMA
- If you have devices that can only access a subset of the RAM
and there is no IOMMU, you really need to use swiotlb to
make it use bounce buffers, at least for the streaming
(dma_map_*) API. A driver using only the coherent (dma_alloc_*)
API should use ZONE_DMA if ZONE_NORMAL goes beyond the
dma mask.
Arnd
On Wed, Mar 6, 2024, at 16:04, Christoph Hellwig wrote:
> On Wed, Mar 06, 2024 at 03:19:52PM +0100, Andreas Larsson wrote:
>> > I think that is the correct thing to do then: the only
>> > drivers that I see with this dependency are PCI sound cards
>> > that apparently rely on DMA to the 16MB ISA range, which is
>> > not provided by sparc.
>>
>> The ZONE_DMA dependency does not seem related to ISA per se. Commit
>> 80ab8eae70e5 ("ALSA: Enable CONFIG_ZONE_DMA for smaller PCI DMA masks")
>> that started to introduce it did were about ensuring 32-bit masks.
>
> Yikes! That commit is just unbelievable buggy. CONFIG_ZONE_DMA
> is only for architetures to select, not drivers. A driver randomly
> enabling such an arch zone is just crazy.
It looks like it did not remain that way for long, as
2db1a57986d3 ("ALSA: pci: depend on ZONE_DMA") removed
the broken select again.
> I've been wondering for a while if we need some Kconfig magic
> so that certain symbols can only be select from arch/* and not
> elsewhere to prevent this (we had a few other similar cases like
> DMA_MAP_OPS).
It's a nice idea, but it would require a lot of reworks to get right,
with things like drivers/platform/x86 or drivers/{parisc,s390,sh}
that are still somewhat architecture code, and subsystems like
drivers/gpu/drm that typically just select whatever they want
instead of using dependencies.
Arnd
On 2024-03-06 15:45, Arnd Bergmann wrote:
>> Testing, in a different driver, setting and
>> allocating under a 30-bit DMA mask (or even a 28-bit DMA mask depending
>> on where the physical memory resides) is possible before removing
>> ZONE_DMA, but not after.
>
> I still don't see how that changes anything if
> max_zone_pfn[ZONE_DMA] and max_zone_pfn[ZONE_NORMAL] are set
> to the same value. Did you test this on a mainline kernel, or
> do you have any patches on top that might have set up
> the zones differently?
This was tested on my for-next plus this patch series, i.e. v6.8-rc1
plus these ones:
(local) sparc32: Fix section mismatch in leon_pci_grpci
(local) sparc32: Fix parport build with sparc32
(local) sparc32: Do not select GENERIC_ISA_DMA
(local) sparc32: Do not select ZONE_DMA
(local) mtd: maps: sun_uflash: Declare uflash_devinit static
(local) sparc32: Fix build with trapbase
(local) sparc32: Use generic cmpdi2/ucmpdi2 variants
626db6ee8ee1e sparc: select FRAME_POINTER instead of redefining it
5378f00c935be sparc: vDSO: fix return value of __setup handler
3ed7c61e49d65 sparc64: NMI watchdog: fix return value of __setup handler
079431ea9ed3e sparc: vio: make vio_bus_type const
3cc208ffa84a7 sparc: Fix typos
0f1991949d9bd sparc: Use shared font data
0955723ef9358 sparc: remove obsolete config ARCH_ATU
so nothing else that should affect zones.
> More specifically, what do you see in the boot log for the
> size of each zone?
dma_set_mask_and_coherent(dev, DMA_BIT_MASK(28)) fails with
the ZONE_DMA removed, and no other differences. Boot log:
831MB HIGHMEM available.
Zone ranges:
Normal [mem 0x0000000000000000-0x000000000bffffff]
HighMem [mem 0x000000000c000000-0x000000003fff7fff]
Movable zone start for each node
Early memory node ranges
node 0: [mem 0x0000000000000000-0x000000003fff7fff]
Initmem setup node 0 [mem 0x0000000000000000-0x000000003fff7fff]
dma_set_mask_and_coherent(dev, DMA_BIT_MASK(28)) succeeds with
ZONE_DMA still in place (i.e. the above plus the ZONE_DMA patch
reverted and no other differences). Boot log:
831MB HIGHMEM available.
Zone ranges:
DMA [mem 0x0000000000000000-0x000000000bffffff]
Normal empty
HighMem [mem 0x000000000c000000-0x000000003fff7fff]
Movable zone start for each node
Early memory node ranges
node 0: [mem 0x0000000000000000-0x000000003fff7fff]
Initmem setup node 0 [mem 0x0000000000000000-0x000000003fff7fff]
>
>> I am also a bit concerned if removing ZONE_DMA will let DMA be allocated
>> in highmem and what that could lead to.
>
> It's not supposed to make a difference, but this is a bit
> more complex:
>
> - Any kernel allocation (kmalloc etc) by definition comes from
> lowmem, regardless of GFP_DMA/GFP_KERNEL.
>
> - user pages that get mapped using dma_map_sg() or dma_map_page()
> can be in highmem, but this should not depend on the presence
> of ZONE_DMA
>
> - If you have devices that can only access a subset of the RAM
> and there is no IOMMU, you really need to use swiotlb to
> make it use bounce buffers, at least for the streaming
> (dma_map_*) API. A driver using only the coherent (dma_alloc_*)
> API should use ZONE_DMA if ZONE_NORMAL goes beyond the
> dma mask.
Thanks for the explanation!
Cheers,
Andreas
On Wed, Mar 6, 2024, at 16:31, Andreas Larsson wrote:
> On 2024-03-06 15:45, Arnd Bergmann wrote:
>> More specifically, what do you see in the boot log for the
>> size of each zone?
>
> dma_set_mask_and_coherent(dev, DMA_BIT_MASK(28)) fails with
> the ZONE_DMA removed, and no other differences. Boot log:
>
> 831MB HIGHMEM available.
> Zone ranges:
> Normal [mem 0x0000000000000000-0x000000000bffffff]
> HighMem [mem 0x000000000c000000-0x000000003fff7fff]
> dma_set_mask_and_coherent(dev, DMA_BIT_MASK(28)) succeeds with
> ZONE_DMA still in place (i.e. the above plus the ZONE_DMA patch
> reverted and no other differences). Boot log:
>
> 831MB HIGHMEM available.
> Zone ranges:
> DMA [mem 0x0000000000000000-0x000000000bffffff]
> Normal empty
> HighMem [mem 0x000000000c000000-0x000000003fff7fff]
That sounds like a bug somewhere else. As Sam explained in
the patch description, ZONE_NORMAL and ZONE_DMA always have
the same limit, which explains that without the patch you
only have DMA and highmem, but not normal.
What we expected to happen here is that anything that asks
for ZONE_DMA memory just uses ZONE_NORMAL instead and the
behavior never changes.
It looks like this is not how dma_direct_supported()
works though:
u64 min_mask = (max_pfn - 1) << PAGE_SHIFT;
[...]
/*
* This check needs to be against the actual bit mask value, so use
* phys_to_dma_unencrypted() here so that the SME encryption mask isn't
* part of the check.
*/
if (IS_ENABLED(CONFIG_ZONE_DMA))
min_mask = min_t(u64, min_mask, DMA_BIT_MASK(zone_dma_bits));
return mask >= phys_to_dma_unencrypted(dev, min_mask);
Without ZONE_DMA, it checks for the highest page of any
zone, but that is ZONE_HIGHMEM in your case, which apparently
is outside of the device's mask, while ZONE_NORMAL is inside
the mask.
Not sure if it's worth changing the generic code for this,
or if we want to just keep the existing version without
Sam's patch now that we understand the issue.
On a relate note, it does seem odd to have such a small
lowmem area, and I wonder if that could be extended.
The 192MB lowmem limit comes from
#define SRMMU_MAXMEM 0x0c000000
but I don't understand if that is a hardware limitation
or a design choice that can be changed, and if it is
even valid on leon or only on the old sun machines.
There is a recurring discussion about eventually
killing off support for CONFIG_HIGHMEM in the kernel,
so if you have a hardware limit of 192MB of lowmem,
this would hit you particularly hard.
Arnd
On Wed, Mar 6, 2024, at 17:22, Arnd Bergmann wrote:
> On Wed, Mar 6, 2024, at 16:31, Andreas Larsson wrote:
> On a relate note, it does seem odd to have such a small
> lowmem area, and I wonder if that could be extended.
> The 192MB lowmem limit comes from
>
> #define SRMMU_MAXMEM 0x0c000000
>
> but I don't understand if that is a hardware limitation
> or a design choice that can be changed, and if it is
> even valid on leon or only on the old sun machines.
I had another look and found that this is a result of
arch/sparc/include/asm/page_32.h:#define PAGE_OFFSET 0xf0000000
which gives 3840MiB to userspace addresses, leaving only
256MiB for kernel lowmem and vmalloc space, which is
less than any other architectures.
I still don't know the history behind this choice, but I
see this was already configured the same when arch/sparc/
was originally merged. You can probably change it to a more
sensible 0xc0000000 or 0x80000000 like on other
architectures and run without highmem on anything with
less than 2GB of total RAM.
How much RAM do Leon machines have typically, or at the
maximum?
Arnd
On Wed, Mar 06, 2024 at 03:19:52PM +0100, Andreas Larsson wrote:
> > I think that is the correct thing to do then: the only
> > drivers that I see with this dependency are PCI sound cards
> > that apparently rely on DMA to the 16MB ISA range, which is
> > not provided by sparc.
>
> The ZONE_DMA dependency does not seem related to ISA per se. Commit
> 80ab8eae70e5 ("ALSA: Enable CONFIG_ZONE_DMA for smaller PCI DMA masks")
> that started to introduce it did were about ensuring 32-bit masks.
Yikes! That commit is just unbelievable buggy. CONFIG_ZONE_DMA
is only for architetures to select, not drivers. A driver randomly
enabling such an arch zone is just crazy.
I've been wondering for a while if we need some Kconfig magic
so that certain symbols can only be select from arch/* and not
elsewhere to prevent this (we had a few other similar cases like
DMA_MAP_OPS).
On 2024-03-06 19:19, Arnd Bergmann wrote:
> On Wed, Mar 6, 2024, at 17:22, Arnd Bergmann wrote:
>> On Wed, Mar 6, 2024, at 16:31, Andreas Larsson wrote:
>
>> On a relate note, it does seem odd to have such a small
>> lowmem area, and I wonder if that could be extended.
>> The 192MB lowmem limit comes from
>>
>> #define SRMMU_MAXMEM 0x0c000000
>>
>> but I don't understand if that is a hardware limitation
>> or a design choice that can be changed, and if it is
>> even valid on leon or only on the old sun machines.
>
> I had another look and found that this is a result of
>
> arch/sparc/include/asm/page_32.h:#define PAGE_OFFSET 0xf0000000
>
> which gives 3840MiB to userspace addresses, leaving only
> 256MiB for kernel lowmem and vmalloc space, which is
> less than any other architectures.
>
> I still don't know the history behind this choice, but I
> see this was already configured the same when arch/sparc/
> was originally merged. You can probably change it to a more
> sensible 0xc0000000 or 0x80000000 like on other
> architectures and run without highmem on anything with
> less than 2GB of total RAM.
>
> How much RAM do Leon machines have typically, or at the
> maximum?
The amount of RAM can vary greatly between systems, from less that
128 MiB up to 2 GiB. An upcoming design uses the entire 36-bit
physical address space and have the possibility of having up to 60
GiB memory.
Thanks,
Andreas
On Thu, Mar 7, 2024, at 13:42, Andreas Larsson wrote:
> On 2024-03-06 19:19, Arnd Bergmann wrote:
>
>> I still don't know the history behind this choice, but I
>> see this was already configured the same when arch/sparc/
>> was originally merged. You can probably change it to a more
>> sensible 0xc0000000 or 0x80000000 like on other
>> architectures and run without highmem on anything with
>> less than 2GB of total RAM.
>>
>> How much RAM do Leon machines have typically, or at the
>> maximum?
> The amount of RAM can vary greatly between systems, from less that
> 128 MiB up to 2 GiB. An upcoming design uses the entire 36-bit
> physical address space and have the possibility of having up to 60
> GiB memory.
If the current maximum is 2GiB, that would be easily handled
by making PAGE_OFFSET configurable the same way that x86 or
arm do, where the common CONFIG_VMSPLIT_3G would extend lowmem
from 192MiB to a little under 1GiB, with PAGE_OFFSET 0xc0000000,
and VMSPLIT_2G_OPT allows exactly 2GiB of lowmem, plus a little
under 2GiB of user addressing.
This would also be an improvement over using highmem for
most applications today, and it means you can keep using the
same setup once highmem support gets removed from the kernel.
For a new design, I would strongly advise against advertising
support for more than 2GiB on a 32-bit Linux kernel, as this
is likely to not be supportable in the future. Note that
in the best case today (40 byte struct page), the mem_map[]
array uses 10MiB of lowmem per GiB installed memory, so you
quickly run out of lowmem when you allow more RAM.
Arnd
On 2024-03-05 16:06, Andreas Larsson wrote:
> On 2024-02-24 18:42, Sam Ravnborg via B4 Relay wrote:
>> From: Sam Ravnborg <[email protected]>
>>
>> Passing a datastructre marked _initconst to platform_driver_register()
>> is wrong. Drop the __initconst notation.
>>
>> This fixes the following warnings:
>>
>> WARNING: modpost: vmlinux: section mismatch in reference: grpci1_of_driver+0x30 (section: .data) -> grpci1_of_match (section: .init.rodata)
>> WARNING: modpost: vmlinux: section mismatch in reference: grpci2_of_driver+0x30 (section: .data) -> grpci2_of_match (section: .init.rodata)
>>
>> Signed-off-by: Sam Ravnborg <[email protected]>
>> Cc: "David S. Miller" <[email protected]>
>> Cc: Andreas Larsson <[email protected]>
>
> Could you also add
>
> Fixes: 4154bb821f0b ("sparc: leon: grpci1: constify of_device_id")
> Fixes: 03949b1cb9f1 ("sparc: leon: grpci2: constify of_device_id")
>
> for these fixes in your v3 of the series as well as CCing stable?
I'll pick up the whole series apart for the ZONE_DMA removal, so there
is no other need for a v3. I can add the Fixes lines to this one, if it
is ok with you Sam, or take it as is.
Thanks,
Andreas
On Thu, 7 Mar 2024, Andreas Larsson wrote:
> I'll pick up the whole series apart for the ZONE_DMA removal, so there
> is no other need for a v3. I can add the Fixes lines to this one, if it
> is ok with you Sam, or take it as is.
Can I have the spelling of my name fixed though (which was supposed to be
included with v3)?
Maciej
On 2024-03-07 21:16, Maciej W. Rozycki wrote:
> On Thu, 7 Mar 2024, Andreas Larsson wrote:
>
>> I'll pick up the whole series apart for the ZONE_DMA removal, so there
>> is no other need for a v3. I can add the Fixes lines to this one, if it
>> is ok with you Sam, or take it as is.
>
> Can I have the spelling of my name fixed though (which was supposed to be
> included with v3)?
>
> Maciej
Based on the outcome of the testing and discussions that followed I will
not pick up the "sparc32: Do not select ZONE_DMA" patch from
https://lore.kernel.org/sparclinux/20240224-sam-fix-sparc32-all-builds-v2-4-1f186603c5c4@ravnborg.org/
which was the one that got your name misspelled, unless I missed some
other occurrence.
Cheers,
Andreas
On 2024-02-24 18:42, Sam Ravnborg via B4 Relay wrote:
> From: Sam Ravnborg <[email protected]>
>
> Fix the following build errors:
> irq_32.c:258:7: error: array subscript [16, 79] is outside array bounds of 'struct tt_entry[1]
> irq_32.c:271:14: error: assignment to 'struct tt_entry *' from incompatible pointer type 'struct tt_entry (*)[]
>
> trapbase is a pointer to an array of tt_entry, but the code declared it
> as a pointer so the compiler see a single entry and not an array.
> Fix this by modifyinf the declaration to be an array, and modify all
> users to take the address of the first member.
>
> Signed-off-by: Sam Ravnborg <[email protected]>
> Acked-by: Randy Dunlap <[email protected]>
> Tested-by: Randy Dunlap <[email protected]> # build-tested
> Cc: Andreas Larsson <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> ---
> arch/sparc/kernel/irq_32.c | 6 +++---
> arch/sparc/kernel/kernel.h | 8 ++++----
> arch/sparc/kernel/kgdb_32.c | 4 ++--
> arch/sparc/kernel/leon_smp.c | 6 +++---
> arch/sparc/kernel/setup_32.c | 4 ++--
> 5 files changed, 14 insertions(+), 14 deletions(-)
Reviewed-by: Andreas Larsson <[email protected]>
Tested-by: Andreas Larsson <[email protected]>
Picking this up to my for-next.
Thanks,
Andreas
On 2024-02-24 18:42, Sam Ravnborg via B4 Relay wrote:
> From: Sam Ravnborg <[email protected]>
>
> This fixes the following warning:
> sun_uflash.c:50:5: error: no previous prototype for 'uflash_devinit'
>
> Signed-off-by: Sam Ravnborg <[email protected]>
> Fixes: 0fcb70851fbf ("Makefile.extrawarn: turn on missing-prototypes globally")
> Reviewed-by: Randy Dunlap <[email protected]>
> Tested-by: Randy Dunlap <[email protected]> # build-tested
> Cc: Andreas Larsson <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> ---
> drivers/mtd/maps/sun_uflash.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Andreas Larsson <[email protected]>
Picking this up to my for-next.
Thanks,
Andreas
On 2024-02-24 18:42, Sam Ravnborg via B4 Relay wrote:
> From: Sam Ravnborg <[email protected]>
>
> sparc32 do not support generic isa dma, so do not select the symbol.
>
> This fixes the following warnings:
> dma.c:70:5: error: no previous prototype for 'request_dma' [-Werror=missing-prototypes]
> dma.c:88:6: error: no previous prototype for 'free_dma' [-Werror=missing-prototypes]
>
> Signed-off-by: Sam Ravnborg <[email protected]>
> Fixes: 0fcb70851fbf ("Makefile.extrawarn: turn on missing-prototypes globally")
> Acked-by: Randy Dunlap <[email protected]>
> Tested-by: Randy Dunlap <[email protected]> # build-tested
> Cc: Andreas Larsson <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Randy Dunlap <[email protected]>
> Cc: Maciej W. Rozycki <[email protected]>
> ---
> arch/sparc/Kconfig | 4 ----
> 1 file changed, 4 deletions(-)
Reviewed-by: Andreas Larsson <[email protected]>
Picking this up to my for-next.
Thanks,
Andreas
On 2024-02-24 18:42, Sam Ravnborg via B4 Relay wrote:
> From: Sam Ravnborg <[email protected]>
>
> include/asm/parport.h is sparc64 specific.
> Rename it to parport_64.h and use the generic version for sparc32.
>
> This fixed all{mod,yes}config build errors like:
>
> parport_pc.c:(.text):undefined-reference-to-ebus_dma_enable
> parport_pc.c:(.text):undefined-reference-to-ebus_dma_irq_enable
> parport_pc.c:(.text):undefined-reference-to-ebus_dma_register
>
> The errors occur as the sparc32 build references sparc64 symbols.
>
> Signed-off-by: Sam Ravnborg <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Andreas Larsson <[email protected]>
> Cc: Randy Dunlap <[email protected]>
> Cc: Maciej W. Rozycki <[email protected]>
> Closes: https://lore.kernel.org/r/[email protected]/
> Fixes: 66bcd06099bb ("parport_pc: Also enable driver for PCI systems")
> Cc: [email protected] # v5.18+
> ---
> arch/sparc/include/asm/parport.h | 259 +-----------------------------------
> arch/sparc/include/asm/parport_64.h | 256 +++++++++++++++++++++++++++++++++++
> 2 files changed, 263 insertions(+), 252 deletions(-)
Reviewed-by: Andreas Larsson <[email protected]>
Picking this up to my for-next
Thanks,
Andreas
On 2024-02-24 18:42, Sam Ravnborg via B4 Relay wrote:
> From: Sam Ravnborg <[email protected]>
>
> Passing a datastructre marked _initconst to platform_driver_register()
> is wrong. Drop the __initconst notation.
>
> This fixes the following warnings:
>
> WARNING: modpost: vmlinux: section mismatch in reference: grpci1_of_driver+0x30 (section: .data) -> grpci1_of_match (section: .init.rodata)
> WARNING: modpost: vmlinux: section mismatch in reference: grpci2_of_driver+0x30 (section: .data) -> grpci2_of_match (section: .init.rodata)
>
> Signed-off-by: Sam Ravnborg <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Andreas Larsson <[email protected]>
> ---
> arch/sparc/kernel/leon_pci_grpci1.c | 2 +-
> arch/sparc/kernel/leon_pci_grpci2.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Andreas Larsson <[email protected]>
Tested-by: Andreas Larsson <[email protected]>
Picking this up to my for-next.
Thanks,
Andreas
Hi Andreas,
On Thu, Mar 07, 2024 at 07:20:11PM +0100, Andreas Larsson wrote:
> On 2024-03-05 16:06, Andreas Larsson wrote:
> > On 2024-02-24 18:42, Sam Ravnborg via B4 Relay wrote:
> >> From: Sam Ravnborg <[email protected]>
> >>
> >> Passing a datastructre marked _initconst to platform_driver_register()
> >> is wrong. Drop the __initconst notation.
> >>
> >> This fixes the following warnings:
> >>
> >> WARNING: modpost: vmlinux: section mismatch in reference: grpci1_of_driver+0x30 (section: .data) -> grpci1_of_match (section: .init.rodata)
> >> WARNING: modpost: vmlinux: section mismatch in reference: grpci2_of_driver+0x30 (section: .data) -> grpci2_of_match (section: .init.rodata)
> >>
> >> Signed-off-by: Sam Ravnborg <[email protected]>
> >> Cc: "David S. Miller" <[email protected]>
> >> Cc: Andreas Larsson <[email protected]>
> >
> > Could you also add
> >
> > Fixes: 4154bb821f0b ("sparc: leon: grpci1: constify of_device_id")
> > Fixes: 03949b1cb9f1 ("sparc: leon: grpci2: constify of_device_id")
> >
> > for these fixes in your v3 of the series as well as CCing stable?
>
> I'll pick up the whole series apart for the ZONE_DMA removal, so there
> is no other need for a v3. I can add the Fixes lines to this one, if it
> is ok with you Sam, or take it as is.
I did not verify the two fixes lines - but seems legit.
So would be super if you add them before applying.
Thanks for picking up the other patches, we are now much closer to
an all{yes,mod}config that can build (but not link).
Sam
On Fri, 8 Mar 2024, Andreas Larsson wrote:
> > From: Sam Ravnborg <[email protected]>
> >
> > sparc32 do not support generic isa dma, so do not select the symbol.
> >
> > This fixes the following warnings:
> > dma.c:70:5: error: no previous prototype for 'request_dma' [-Werror=missing-prototypes]
> > dma.c:88:6: error: no previous prototype for 'free_dma' [-Werror=missing-prototypes]
> >
> > Signed-off-by: Sam Ravnborg <[email protected]>
> > Fixes: 0fcb70851fbf ("Makefile.extrawarn: turn on missing-prototypes globally")
> > Acked-by: Randy Dunlap <[email protected]>
> > Tested-by: Randy Dunlap <[email protected]> # build-tested
> > Cc: Andreas Larsson <[email protected]>
> > Cc: "David S. Miller" <[email protected]>
> > Cc: Randy Dunlap <[email protected]>
> > Cc: Maciej W. Rozycki <[email protected]>
> > ---
> > arch/sparc/Kconfig | 4 ----
> > 1 file changed, 4 deletions(-)
>
> Reviewed-by: Andreas Larsson <[email protected]>
Reviewed-by: Maciej W. Rozycki <[email protected]>
Tested-by: Maciej W. Rozycki <[email protected]> # build-tested
Maciej
On Fri, 8 Mar 2024, Andreas Larsson wrote:
> > include/asm/parport.h is sparc64 specific.
> > Rename it to parport_64.h and use the generic version for sparc32.
> >
> > This fixed all{mod,yes}config build errors like:
> >
> > parport_pc.c:(.text):undefined-reference-to-ebus_dma_enable
> > parport_pc.c:(.text):undefined-reference-to-ebus_dma_irq_enable
> > parport_pc.c:(.text):undefined-reference-to-ebus_dma_register
> >
> > The errors occur as the sparc32 build references sparc64 symbols.
> >
> > Signed-off-by: Sam Ravnborg <[email protected]>
> > Cc: "David S. Miller" <[email protected]>
> > Cc: Andreas Larsson <[email protected]>
> > Cc: Randy Dunlap <[email protected]>
> > Cc: Maciej W. Rozycki <[email protected]>
> > Closes: https://lore.kernel.org/r/[email protected]/
> > Fixes: 66bcd06099bb ("parport_pc: Also enable driver for PCI systems")
> > Cc: [email protected] # v5.18+
> > ---
> > arch/sparc/include/asm/parport.h | 259 +-----------------------------------
> > arch/sparc/include/asm/parport_64.h | 256 +++++++++++++++++++++++++++++++++++
> > 2 files changed, 263 insertions(+), 252 deletions(-)
>
> Reviewed-by: Andreas Larsson <[email protected]>
Reviewed-by: Maciej W. Rozycki <[email protected]>
Tested-by: Maciej W. Rozycki <[email protected]> # build-tested
Maciej
On Fri, 8 Mar 2024, Andreas Larsson wrote:
> Based on the outcome of the testing and discussions that followed I will
> not pick up the "sparc32: Do not select ZONE_DMA" patch from
>
> https://lore.kernel.org/sparclinux/20240224-sam-fix-sparc32-all-builds-v2-4-1f186603c5c4@ravnborg.org/
>
> which was the one that got your name misspelled, unless I missed some
> other occurrence.
Ah, OK then, thank you for double-checking.
Maciej