diff mbox

[5/9] ARM: BCM2836: Add io map initialization for bcm2836.

Message ID 1429639796-2169-6-git-send-email-eric@anholt.net (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Anholt April 21, 2015, 6:09 p.m. UTC 
---
 arch/arm/mach-bcm/board_bcm2835.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

Comments

Arnd Bergmann April 21, 2015, 6:53 p.m. UTC | #1 
On Tuesday 21 April 2015 11:09:52 Eric Anholt wrote:
> +static struct map_desc bcm2836_io_map __initdata = {
> +       .virtual = BCM2835_PERIPH_VIRT,
> +       .pfn = __phys_to_pfn(BCM2836_PERIPH_PHYS),
> +       .length = BCM2835_PERIPH_SIZE,
> +       .type = MT_DEVICE
> +};
> +
>  static void __init bcm2835_map_io(void)
>  {
> -       iotable_init(&io_map, 1);
> +       iotable_init(&bcm2835_io_map, 1);
> +}
> +
> +static void __init bcm2836_map_io(void)
> +{
> +       iotable_init(&bcm2836_io_map, 1);
>  }
> 

Can you explain what this is needed for? Most platform ports don't
do this any more.

	Arnd
Eric Anholt April 21, 2015, 8:37 p.m. UTC | #2 
Arnd Bergmann <arnd@arndb.de> writes:

> On Tuesday 21 April 2015 11:09:52 Eric Anholt wrote:
>> +static struct map_desc bcm2836_io_map __initdata = {
>> +       .virtual = BCM2835_PERIPH_VIRT,
>> +       .pfn = __phys_to_pfn(BCM2836_PERIPH_PHYS),
>> +       .length = BCM2835_PERIPH_SIZE,
>> +       .type = MT_DEVICE
>> +};
>> +
>>  static void __init bcm2835_map_io(void)
>>  {
>> -       iotable_init(&io_map, 1);
>> +       iotable_init(&bcm2835_io_map, 1);
>> +}
>> +
>> +static void __init bcm2836_map_io(void)
>> +{
>> +       iotable_init(&bcm2836_io_map, 1);
>>  }
>> 
>
> Can you explain what this is needed for? Most platform ports don't
> do this any more.

Nope, I can't!  I'm not sure what the bcm2835 side of it does, and I was
just replicating that for 2836.

Should it be removed from 2835, too?
Arnd Bergmann April 21, 2015, 9:11 p.m. UTC | #3 
On Tuesday 21 April 2015 13:37:13 Eric Anholt wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
> 
> > On Tuesday 21 April 2015 11:09:52 Eric Anholt wrote:
> >> +static struct map_desc bcm2836_io_map __initdata = {
> >> +       .virtual = BCM2835_PERIPH_VIRT,
> >> +       .pfn = __phys_to_pfn(BCM2836_PERIPH_PHYS),
> >> +       .length = BCM2835_PERIPH_SIZE,
> >> +       .type = MT_DEVICE
> >> +};
> >> +
> >>  static void __init bcm2835_map_io(void)
> >>  {
> >> -       iotable_init(&io_map, 1);
> >> +       iotable_init(&bcm2835_io_map, 1);
> >> +}
> >> +
> >> +static void __init bcm2836_map_io(void)
> >> +{
> >> +       iotable_init(&bcm2836_io_map, 1);
> >>  }
> >> 
> >
> > Can you explain what this is needed for? Most platform ports don't
> > do this any more.
> 
> Nope, I can't!  I'm not sure what the bcm2835 side of it does, and I was
> just replicating that for 2836.
> 
> Should it be removed from 2835, too?
> 

Hard to know. Does anything reference BCM2835_PERIPH_VIRT? Does it work
if you remove it?

	Arnd
Eric Anholt April 21, 2015, 11:02 p.m. UTC | #4 
Arnd Bergmann <arnd@arndb.de> writes:

> On Tuesday 21 April 2015 13:37:13 Eric Anholt wrote:
>> Arnd Bergmann <arnd@arndb.de> writes:
>> 
>> > On Tuesday 21 April 2015 11:09:52 Eric Anholt wrote:
>> >> +static struct map_desc bcm2836_io_map __initdata = {
>> >> +       .virtual = BCM2835_PERIPH_VIRT,
>> >> +       .pfn = __phys_to_pfn(BCM2836_PERIPH_PHYS),
>> >> +       .length = BCM2835_PERIPH_SIZE,
>> >> +       .type = MT_DEVICE
>> >> +};
>> >> +
>> >>  static void __init bcm2835_map_io(void)
>> >>  {
>> >> -       iotable_init(&io_map, 1);
>> >> +       iotable_init(&bcm2835_io_map, 1);
>> >> +}
>> >> +
>> >> +static void __init bcm2836_map_io(void)
>> >> +{
>> >> +       iotable_init(&bcm2836_io_map, 1);
>> >>  }
>> >> 
>> >
>> > Can you explain what this is needed for? Most platform ports don't
>> > do this any more.
>> 
>> Nope, I can't!  I'm not sure what the bcm2835 side of it does, and I was
>> just replicating that for 2836.
>> 
>> Should it be removed from 2835, too?
>> 
>
> Hard to know. Does anything reference BCM2835_PERIPH_VIRT? Does it work
> if you remove it?

Well, that's clear enough. It dies early with:

Uncompressing Linux... done, booting the kernel.
[    0.000000] Booting Linux on physical CPU 0xf00
[    0.000000] Initializing cgroup subsys cpu
[    0.000000] Initializing cgroup subsys cpuacct
[    0.000000] Linux version 4.0.0-rc1-v7+ (anholt@eliezer) (gcc version 4.9.2 ( 4.9.2-10) ) #487 SMP PREEMPT Tue Apr 21 15:58:29 PDT 2015
[    0.000000] CPU: ARMv7 Processor [410fc075] revision 5 (ARMv7), cr=10c5387d
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
[    0.000000] Machine model: Raspberry Pi 2 Model B+
[    0.000000] bootconsole [earlycon0] enabled
[    0.000000] cma: Reserved 64 MiB at 0x37000000
[    0.000000] Memory policy: Data cache writealloc

(hung)

The only thing I see using this 0xf0000000 range is DEBUG_BCM2836's
serial stuff, though.
Arnd Bergmann April 22, 2015, 7:22 a.m. UTC | #5 
On Tuesday 21 April 2015 16:02:19 Eric Anholt wrote:
> > Hard to know. Does anything reference BCM2835_PERIPH_VIRT? Does it work
> > if you remove it?
> Well, that's clear enough. It dies early with:
> 
> Uncompressing Linux... done, booting the kernel.
> [    0.000000] Booting Linux on physical CPU 0xf00
> [    0.000000] Initializing cgroup subsys cpu
> [    0.000000] Initializing cgroup subsys cpuacct
> [    0.000000] Linux version 4.0.0-rc1-v7+ (anholt@eliezer) (gcc version 4.9.2 ( 4.9.2-10) ) #487 SMP PREEMPT Tue Apr 21 15:58:29 PDT 2015
> [    0.000000] CPU: ARMv7 Processor [410fc075] revision 5 (ARMv7), cr=10c5387d
> [    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
> [    0.000000] Machine model: Raspberry Pi 2 Model B+
> [    0.000000] bootconsole [earlycon0] enabled
> [    0.000000] cma: Reserved 64 MiB at 0x37000000
> [    0.000000] Memory policy: Data cache writealloc
> 
> (hung)
> 
> The only thing I see using this 0xf0000000 range is DEBUG_BCM2836's
> serial stuff, though.
> 

Ok. Just to be clear: by removing that mapping, I meant removing the
".map_io = bcm2835_map_io" line as well, so the default debug_ll_io_init()
function gets called. If you have a map_io function that does not call
debug_ll_io_init() or something equivalent, you cannot use DEBUG_LL.

It's possible that this mapping was just added for supporting DEBUG_LL,
using the default debug_ll_io_init() is a better way to do that.

	Arnd
Arnd Bergmann April 24, 2015, 7:16 a.m. UTC | #6 
On Tuesday 21 April 2015 11:09:52 Eric Anholt wrote:
> +
> +DT_MACHINE_START(BCM2836, "BCM2836")
> +       .map_io = bcm2836_map_io,
> +       .init_irq = irqchip_init,
> +       .init_machine = bcm2835_init,
> +       .restart = bcm2835_restart,
> +       .dt_compat = bcm2836_compat
> +MACHINE_END

I've just looked at the other callbacks you have here, and I think
it would be easy enough to remove them as well:

- .init_irq is already pointless, you can just remove the line here
  without any effect

- bcm2835_restart can be moved to the drivers/watchdog/bcm2835_wdt.c
  driver, which already knows the registers. Just use
  register_restart_handler() in its probe function, and perhaps add
  a 'select' statement to ensure that the driver is included in kernel
  builds

- I'm not completely sure about the plans for the clock handler, but
  no proper clock driver has surfaced so far, and the clocks could just
  all be declared in DT as fixed rate clocks.

  Is anyone working on a proper clk driver here?

- once bcm2835_restart is gone from bcm2835.c and the clocks are handled
  in DT, the bcm2835_init() function can be removed as well.

	Arnd
diff mbox

Patch

diff --git a/arch/arm/mach-bcm/board_bcm2835.c b/arch/arm/mach-bcm/board_bcm2835.c
index 70f2f39..69f5821 100644
--- a/arch/arm/mach-bcm/board_bcm2835.c
+++ b/arch/arm/mach-bcm/board_bcm2835.c
@@ -32,6 +32,7 @@ 
 #define PM_RSTS_HADWRH_SET		0x00000040
 
 #define BCM2835_PERIPH_PHYS	0x20000000
+#define BCM2836_PERIPH_PHYS	0x3f000000
 #define BCM2835_PERIPH_VIRT	0xf0000000
 #define BCM2835_PERIPH_SIZE	SZ_16M
 
@@ -93,16 +94,28 @@  static void bcm2835_power_off(void)
 	bcm2835_restart(REBOOT_HARD, "");
 }
 
-static struct map_desc io_map __initdata = {
+static struct map_desc bcm2835_io_map __initdata = {
 	.virtual = BCM2835_PERIPH_VIRT,
 	.pfn = __phys_to_pfn(BCM2835_PERIPH_PHYS),
 	.length = BCM2835_PERIPH_SIZE,
 	.type = MT_DEVICE
 };
 
+static struct map_desc bcm2836_io_map __initdata = {
+	.virtual = BCM2835_PERIPH_VIRT,
+	.pfn = __phys_to_pfn(BCM2836_PERIPH_PHYS),
+	.length = BCM2835_PERIPH_SIZE,
+	.type = MT_DEVICE
+};
+
 static void __init bcm2835_map_io(void)
 {
-	iotable_init(&io_map, 1);
+	iotable_init(&bcm2835_io_map, 1);
+}
+
+static void __init bcm2836_map_io(void)
+{
+	iotable_init(&bcm2836_io_map, 1);
 }
 
 static void __init bcm2835_init(void)
@@ -128,6 +141,11 @@  static const char * const bcm2835_compat[] = {
 	NULL
 };
 
+static const char * const bcm2836_compat[] = {
+	"brcm,bcm2836",
+	NULL
+};
+
 DT_MACHINE_START(BCM2835, "BCM2835")
 	.map_io = bcm2835_map_io,
 	.init_irq = irqchip_init,
@@ -135,3 +153,11 @@  DT_MACHINE_START(BCM2835, "BCM2835")
 	.restart = bcm2835_restart,
 	.dt_compat = bcm2835_compat
 MACHINE_END
+
+DT_MACHINE_START(BCM2836, "BCM2836")
+	.map_io = bcm2836_map_io,
+	.init_irq = irqchip_init,
+	.init_machine = bcm2835_init,
+	.restart = bcm2835_restart,
+	.dt_compat = bcm2836_compat
+MACHINE_END