Skip to contentSkip to conversation

mach-integrator: fix VGA base regression

PATCH New Linus Walleij 3 participants 11 messages Sep 4, 2011 – Sep 20, 2011

Subsystems (2)
Arm (32-bit) Arm sub-architectures
lore
Share & tools
lore

Linus Walleij

author
#0 lore raw

PATCH 1/1 mach-integrator: fix VGA base regression

The changes introduced in commit
cc22b4c18540e5e8bf55c7d124044f9317527d3c
"ARM: set vga memory base at run-time"

Makes the Integrator/AP freeze completely. I appears that this is due to the VGA base address being assigned at PCI init time, while this base is needed earlier than that. Moving the initialization of the base address to the .map_io function solves this problem.

Cc Rob Herring <[email protected]>
Cc Nicolas Pitre <[email protected]>
Cc Arnd Bergmann <[email protected]>
Signed-off-by Linus Walleij <[email protected]>
---
 arch/arm/mach-integrator/integrator_ap.c |    2 ++
 arch/arm/mach-integrator/pci_v3.c        |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/mach-integrator/integrator_ap.c +2 -0
index 2fbbdd5..4d88fc4 100644
@@ -32,6 +32,7 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/mtd/physmap.h>
+#include <video/vga.h>
 
 #include <mach/hardware.h>
 #include <mach/platform.h>

@@ -154,6 +155,7 @@ static struct map_desc ap_io_desc[] __initdata = {
 static void __init ap_map_io(void)
 {
 	iotable_init(ap_io_desc, ARRAY_SIZE(ap_io_desc));
+	vga_base = PCI_MEMORY_VADDR;
 }
 
 #define INTEGRATOR_SC_VALID_INT	0x003fffff
arch/arm/mach-integrator/pci_v3.c +0 -2
index dd56bfb..11b86e5 100644
@@ -27,7 +27,6 @@
 #include <linux/spinlock.h>
 #include <linux/init.h>
 #include <linux/io.h>
-#include <video/vga.h>
 
 #include <mach/hardware.h>
 #include <mach/platform.h>

@@ -505,7 +504,6 @@ void __init pci_v3_preinit(void)
 
 	pcibios_min_io = 0x6000;
 	pcibios_min_mem = 0x00100000;
-	vga_base = PCI_MEMORY_VADDR;
 
 	/*
 	 * Hook in our fault handler for PCI errors
Signature · 1 line
1.7.6
#1 lore raw

PATCH 1/1 mach-integrator: fix VGA base regression

Linus,

Quoted 52 lines · Linus Walleij

The changes introduced in commit
cc22b4c18540e5e8bf55c7d124044f9317527d3c
"ARM: set vga memory base at run-time"

Makes the Integrator/AP freeze completely. I appears that this is due to the VGA base address being assigned at PCI init time, while this base is needed earlier than that. Moving the initialization of the base address to the .map_io function solves this problem.

Cc Rob Herring <[email protected]>
Cc Nicolas Pitre <[email protected]>
Cc Arnd Bergmann <[email protected]>
Signed-off-by Linus Walleij <[email protected]>
---
 arch/arm/mach-integrator/integrator_ap.c |    2 ++
 arch/arm/mach-integrator/pci_v3.c        |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/mach-integrator/integrator_ap.c +2 -0
index 2fbbdd5..4d88fc4 100644
@@ -32,6 +32,7 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/mtd/physmap.h>
+#include <video/vga.h>
 
 #include <mach/hardware.h>
 #include <mach/platform.h>

@@ -154,6 +155,7 @@ static struct map_desc ap_io_desc[] __initdata = {
 static void __init ap_map_io(void)
 {
 	iotable_init(ap_io_desc, ARRAY_SIZE(ap_io_desc));
+	vga_base = PCI_MEMORY_VADDR;
 }
 
 #define INTEGRATOR_SC_VALID_INT	0x003fffff
arch/arm/mach-integrator/pci_v3.c +0 -1
index dd56bfb..11b86e5 100644
@@ -27,7 +27,6 @@
 #include <linux/spinlock.h>
 #include <linux/init.h>
 #include <linux/io.h>
-#include <video/vga.h>
 
 #include <mach/hardware.h>
 #include <mach/platform.h>

arch/arm/mach-integrator/pci_v3.c void __init pci_v3_preinit(void) · line 509

Quoted 7 lines
arch/arm/mach-integrator/pci_v3.c +0 -1
@@ -505,7 +504,6 @@ void __init pci_v3_preinit(void)
 
 	pcibios_min_io = 0x6000;
 	pcibios_min_mem = 0x00100000;
-	vga_base = PCI_MEMORY_VADDR;
 
 	/*
 	 * Hook in our fault handler for PCI errors

Should the VGA memory really be accessed before PCI host is initialized?

Rob

author
#2 lore raw

PATCH 1/1 mach-integrator: fix VGA base regression

Quoted 8 lines · Rob Herring
Quoted 8 lines
(quoted diff) +1 -0
@@ -154,6 +155,7 @@ static struct map_desc ap_io_desc[] __initdata = {
 ?static void __init ap_map_io(void)
 ?{
 ? ? ? iotable_init(ap_io_desc, ARRAY_SIZE(ap_io_desc));
+ ? ? vga_base = PCI_MEMORY_VADDR;
 ?}

@@ -505,7 +504,6 @@ void __init pci_v3_preinit(void)
Quoted 8 lines
Quoted 6 lines
(quoted diff) +0 -1
 ? ? ? pcibios_min_io = 0x6000;
 ? ? ? pcibios_min_mem = 0x00100000;
- ? ? vga_base = PCI_MEMORY_VADDR;

? ? ? /*
? ? ? ?* Hook in our fault handler for PCI errors

Should the VGA memory really be accessed before PCI host is initialized?

I don' know, I don't know one bit about how PCI works and should work, you tell me :-)

What I know is that without this patch the 3.1 rc does not boot on Integrator.

Do you prefer that I revert your commit and wait with this thing until we figured out why it breaks the Integrator instead?

Yours,
Linus Walleij

#3 lore raw

PATCH 1/1 mach-integrator: fix VGA base regression

Quoted 14 lines · Linus Walleij
Quoted 12 lines · Rob Herring
Quoted 12 lines
arch/arm/mach-integrator/integrator_ap.c +1 -1
@@ -154,6 +155,7 @@ static struct map_desc ap_io_desc[] __initdata = {
 static void __init ap_map_io(void)
 {
      iotable_init(ap_io_desc, ARRAY_SIZE(ap_io_desc));
+     vga_base = PCI_MEMORY_VADDR;
 }

@@ -505,7 +504,6 @@ void __init pci_v3_preinit(void)
 
      pcibios_min_io = 0x6000;
      pcibios_min_mem = 0x00100000;
-     vga_base = PCI_MEMORY_VADDR;
Quoted 8 lines
Quoted 4 lines

arch/arm/mach-integrator/pci_v3.c void __init pci_v3_preinit(void) · line 509

Quoted 2 lines
arch/arm/mach-integrator/pci_v3.c +0 -0
      /*
       * Hook in our fault handler for PCI errors

Should the VGA memory really be accessed before PCI host is initialized?

I don' know, I don't know one bit about how PCI works and should work, you tell me :-)

I guess it's normal (from init/main.c):

/*
* HACK ALERT! This is early. We're enabling the console before
* we've done PCI setups etc, and console_init() must be aware of
* this. But we do want output early, in case something goes wrong. */

Quoted 5 lines

What I know is that without this patch the 3.1 rc does not boot on Integrator.

Do you prefer that I revert your commit and wait with this thing until we figured out why it breaks the Integrator instead?

Well, it's really no difference with your patch or reverting mine in terms of init order. I'd rather not have to add back hardware.h as a required mach header.

So FWIW:

Acked-by Rob Herring <[email protected]>

Rob

author
#4 lore raw

PATCH 1/1 mach-integrator: fix VGA base regression

2011/9/5 Rob Herring <[email protected]>:

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

Should the VGA memory really be accessed before PCI host is initialized?

I don' know, I don't know one bit about how PCI works and should work, you tell me :-)

I guess it's normal (from init/main.c):

Ah. I've even seen that comment in main.c.

SHall I go over the other changes in the same and check for potential problems?

Quoted 3 lines

Well, it's really no difference with your patch or reverting mine in terms of init order. I'd rather not have to add back hardware.h as a required mach header.

No please. Let's go for this patch and potentially others if there are related problems.

Quoted 1 line
Acked-by Rob Herring <[email protected]>

Thanks!
Linus Walleij

#5 lore raw

PATCH 1/1 mach-integrator: fix VGA base regression

Quoted 7 lines · Linus Walleij

2011/9/5 Rob Herring <[email protected]>:

Quoted 5 lines
Quoted 4 lines
Quoted 1 line

Should the VGA memory really be accessed before PCI host is initialized?

I don' know, I don't know one bit about how PCI works and should work, you tell me :-)

Quoted 7 lines
Quoted 1 line

I guess it's normal (from init/main.c):

Ah. I've even seen that comment in main.c.

SHall I go over the other changes in the same and check for potential problems?

Integrator and footbridge are the only platforms that would ever actually work with VGA console because the others are all returning a physical address rather than a virtual address. kirkwood, dove, mv78xx0, and orion5x are all wrong. I'm not sure on shark. Seems to be a virtual address, but it's not mapped.

So perhaps the correct fix is just to remove the initialization of vga_base on all these platforms.

Rob

author
#6 lore raw

PATCH 1/1 mach-integrator: fix VGA base regression

2011/9/6 Rob Herring <[email protected]>:

Quoted 2 lines

Integrator and footbridge are the only platforms that would ever actually work with VGA console (...)

Hm I wonder if I can test this. Can I just plug some arbitrary VGA-capable graphics card into an empty PCI socket and expect the console to come up?

Quoted 7 lines

the others are all returning a
physical address rather than a virtual address. kirkwood, dove, mv78xx0, and orion5x are all wrong. I'm not sure on shark. Seems to be a virtual address, but it's not mapped.

So perhaps the correct fix is just to remove the initialization of vga_base on all these platforms.

Sounds like a pretty good idea, but since I ran into the problem, what's actually happening in these systems on boot? Are they reading/writing some arbitrary physical location as if it was a virtual address?

Linus

author
#7 lore raw

PATCH 1/1 mach-integrator: fix VGA base regression

In reply to Linus Walleij
Quoted 3 lines · Linus Walleij

The changes introduced in commit
cc22b4c18540e5e8bf55c7d124044f9317527d3c
"ARM: set vga memory base at run-time"

Arnd can you choose whether to apply this patch or revert the commit making it necessary?

Thanks,
Linus Walleij

#8 lore raw

PATCH 1/1 mach-integrator: fix VGA base regression

Quoted 8 lines · Linus Walleij
Quoted 3 lines · Linus Walleij

The changes introduced in commit
cc22b4c18540e5e8bf55c7d124044f9317527d3c
"ARM: set vga memory base at run-time"

Arnd can you choose whether to apply this patch or revert the commit making it necessary?

I'm still stuck with master.kernel.org being down, so I can't really push any updates right now. I'll do it once it's up again.

Thanks,

Arnd

author
#9 lore raw

PATCH 1/1 mach-integrator: fix VGA base regression

Quoted 7 lines · Arnd Bergmann
Quoted 6 lines · Linus Walleij
Quoted 3 lines · Linus Walleij

The changes introduced in commit
cc22b4c18540e5e8bf55c7d124044f9317527d3c
"ARM: set vga memory base at run-time"

Quoted 5 lines
Quoted 2 lines

Arnd can you choose whether to apply this patch or revert the commit making it necessary?

I'm still stuck with master.kernel.org being down, so I can't really push any updates right now. I'll do it once it's up again.

Ping on this. Rob agreed to merge the patch, this is a regression making Integrator non-bootable in 3.1 so I'd really like to see it merged ASAP.

Yours,
Linus Walleij

Applied
#10 lore raw

PATCH 1/1 mach-integrator: fix VGA base regression

Quoted 11 lines · Linus Walleij
Quoted 10 lines · Arnd Bergmann
Quoted 8 lines · Linus Walleij
Quoted 3 lines · Linus Walleij

The changes introduced in commit
cc22b4c18540e5e8bf55c7d124044f9317527d3c
"ARM: set vga memory base at run-time"

Arnd can you choose whether to apply this patch or revert the commit making it necessary?

Quoted 7 lines
Quoted 2 lines

I'm still stuck with master.kernel.org being down, so I can't really push any updates right now. I'll do it once it's up again.

Ping on this. Rob agreed to merge the patch, this is a regression making Integrator non-bootable in 3.1 so I'd really like to see it merged ASAP.

Applied, thanks for the reminder!

Arnd