2013-06-17 00:50:51

by Grant Likely

[permalink] [raw]
Subject: [RFC] arm: Remove sa1111 special case from arm_mm_memblock_reserve()

The machine desc structure has a hook for doing machine-specific
memblock code, but the SA1111 still has a platform-specific hook in the
generic code. This patch merely moves the needed memblock_reserve()
into a callback.

This still leaves a special case in mem_init() to call
free_reserved_area() on the same region, but there isn't a suitable hook
in mdesc for making that call so the change is left to another patch.

Compile tested only.

Signed-off-by: Grant Likely <[email protected]>
Cc: Russell King <[email protected]>
Cc: Nicolas Pitre <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Arnd Bergmann <[email protected]>
---
arch/arm/mach-sa1100/assabet.c | 1 +
arch/arm/mach-sa1100/badge4.c | 1 +
arch/arm/mach-sa1100/generic.c | 12 ++++++++++++
arch/arm/mach-sa1100/generic.h | 1 +
arch/arm/mach-sa1100/jornada720.c | 1 +
arch/arm/mm/mmu.c | 8 --------
6 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-sa1100/assabet.c b/arch/arm/mach-sa1100/assabet.c
index e838ba2..aab247a 100644
--- a/arch/arm/mach-sa1100/assabet.c
+++ b/arch/arm/mach-sa1100/assabet.c
@@ -621,6 +621,7 @@ MACHINE_START(ASSABET, "Intel-Assabet")
.fixup = fixup_assabet,
.map_io = assabet_map_io,
.nr_irqs = SA1100_NR_IRQS,
+ .reserve = sa1100_reserve,
.init_irq = sa1100_init_irq,
.init_time = sa1100_timer_init,
.init_machine = assabet_init,
diff --git a/arch/arm/mach-sa1100/badge4.c b/arch/arm/mach-sa1100/badge4.c
index 63361b6..9033d74 100644
--- a/arch/arm/mach-sa1100/badge4.c
+++ b/arch/arm/mach-sa1100/badge4.c
@@ -334,6 +334,7 @@ MACHINE_START(BADGE4, "Hewlett-Packard Laboratories BadgePAD 4")
.atag_offset = 0x100,
.map_io = badge4_map_io,
.nr_irqs = SA1100_NR_IRQS,
+ .reserve = sa1100_reserve,
.init_irq = sa1100_init_irq,
.init_late = sa11x0_init_late,
.init_time = sa1100_timer_init,
diff --git a/arch/arm/mach-sa1100/generic.c b/arch/arm/mach-sa1100/generic.c
index 9db3e98..1e7164e 100644
--- a/arch/arm/mach-sa1100/generic.c
+++ b/arch/arm/mach-sa1100/generic.c
@@ -15,6 +15,7 @@
#include <linux/init.h>
#include <linux/delay.h>
#include <linux/dma-mapping.h>
+#include <linux/memblock.h>
#include <linux/pm.h>
#include <linux/cpufreq.h>
#include <linux/ioport.h>
@@ -26,6 +27,7 @@
#include <asm/mach/map.h>
#include <asm/mach/flash.h>
#include <asm/irq.h>
+#include <asm/pgtable.h>
#include <asm/system_misc.h>

#include <mach/hardware.h>
@@ -351,6 +353,16 @@ static struct platform_device *sa11x0_devices[] __initdata = {
&sa11x0dma_device,
};

+void __init sa1100_reserve(void)
+{
+ /*
+ * Because of the SA1111 DMA bug, we want to preserve our
+ * precious DMA-able memory...
+ */
+ if (IS_ENABLED(CONFIG_SA1111))
+ memblock_reserve(PHYS_OFFSET, __pa(swapper_pg_dir) - PHYS_OFFSET);
+}
+
static int __init sa1100_init(void)
{
pm_power_off = sa1100_power_off;
diff --git a/arch/arm/mach-sa1100/generic.h b/arch/arm/mach-sa1100/generic.h
index 2abc6a1..5d8398c 100644
--- a/arch/arm/mach-sa1100/generic.h
+++ b/arch/arm/mach-sa1100/generic.h
@@ -5,6 +5,7 @@
*/

extern void sa1100_timer_init(void);
+extern void sa1100_reserve(void);
extern void __init sa1100_map_io(void);
extern void __init sa1100_init_irq(void);
extern void __init sa1100_init_gpio(void);
diff --git a/arch/arm/mach-sa1100/jornada720.c b/arch/arm/mach-sa1100/jornada720.c
index c0b1f5b..d95961a 100644
--- a/arch/arm/mach-sa1100/jornada720.c
+++ b/arch/arm/mach-sa1100/jornada720.c
@@ -345,6 +345,7 @@ MACHINE_START(JORNADA720, "HP Jornada 720")
.atag_offset = 0x100,
.map_io = jornada720_map_io,
.nr_irqs = SA1100_NR_IRQS,
+ .reserve = sa1100_reserve,
.init_irq = sa1100_init_irq,
.init_time = sa1100_timer_init,
.init_machine = jornada720_mach_init,
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index e0d8565..f281a24 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1147,14 +1147,6 @@ void __init arm_mm_memblock_reserve(void)
* and can only be in node 0.
*/
memblock_reserve(__pa(swapper_pg_dir), SWAPPER_PG_DIR_SIZE);
-
-#ifdef CONFIG_SA1111
- /*
- * Because of the SA1111 DMA bug, we want to preserve our
- * precious DMA-able memory...
- */
- memblock_reserve(PHYS_OFFSET, __pa(swapper_pg_dir) - PHYS_OFFSET);
-#endif
}

/*
--
1.8.1.2


2013-06-17 08:41:12

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [RFC] arm: Remove sa1111 special case from arm_mm_memblock_reserve()

On Mon, Jun 17, 2013 at 01:43:19AM +0100, Grant Likely wrote:
> The machine desc structure has a hook for doing machine-specific
> memblock code, but the SA1111 still has a platform-specific hook in the
> generic code. This patch merely moves the needed memblock_reserve()
> into a callback.

And... the reason I left this in generic code was to avoid the problem
which you've created by doing this change - there is at least one PXA
platform which also can have SA1111 enabled, and by omitting this
reservation, you will make that platform blow up at boot time because
we will try and free the same region of RAM twice into the buddy
allocator.

It was left there because one of these currently has to stay in generic
code, so they both need to in order to ensure that both are properly
paired.

What problem, other than a distaste for having such things in generic
code, are you having with this?

2013-06-17 22:20:18

by Grant Likely

[permalink] [raw]
Subject: Re: [RFC] arm: Remove sa1111 special case from arm_mm_memblock_reserve()

On Mon, Jun 17, 2013 at 9:40 AM, Russell King - ARM Linux
<[email protected]> wrote:
> On Mon, Jun 17, 2013 at 01:43:19AM +0100, Grant Likely wrote:
>> The machine desc structure has a hook for doing machine-specific
>> memblock code, but the SA1111 still has a platform-specific hook in the
>> generic code. This patch merely moves the needed memblock_reserve()
>> into a callback.
>
> And... the reason I left this in generic code was to avoid the problem
> which you've created by doing this change - there is at least one PXA
> platform which also can have SA1111 enabled, and by omitting this
> reservation, you will make that platform blow up at boot time because
> we will try and free the same region of RAM twice into the buddy
> allocator.
>
> It was left there because one of these currently has to stay in generic
> code, so they both need to in order to ensure that both are properly
> paired.
>
> What problem, other than a distaste for having such things in generic
> code, are you having with this?

Merely trying to clean up generic code. That's all. It was something I
noticed that would be nice to clean up. I could remove it I suppose by
adding a new machine_desc hook, but it's not worth adding another
callback for exactly one user. I'm not going to bother spending any
more time on this.

g.