| Message ID | 1315172408-18957-1-git-send-email-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Linus, On 09/04/2011 04:40 PM, Linus Walleij wrote: > 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 <rob.herring@calxeda.com> > Cc: Nicolas Pitre <nicolas.pitre@linaro.org> > Cc: Arnd Bergmann <arnd@arndb.de> > Signed-off-by: Linus Walleij <linus.walleij@stericsson.com> > --- > arch/arm/mach-integrator/integrator_ap.c | 2 ++ > arch/arm/mach-integrator/pci_v3.c | 2 -- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-integrator/integrator_ap.c b/arch/arm/mach-integrator/integrator_ap.c > index 2fbbdd5..4d88fc4 100644 > --- a/arch/arm/mach-integrator/integrator_ap.c > +++ b/arch/arm/mach-integrator/integrator_ap.c > @@ -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 > diff --git a/arch/arm/mach-integrator/pci_v3.c b/arch/arm/mach-integrator/pci_v3.c > index dd56bfb..11b86e5 100644 > --- a/arch/arm/mach-integrator/pci_v3.c > +++ b/arch/arm/mach-integrator/pci_v3.c > @@ -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 Should the VGA memory really be accessed before PCI host is initialized? Rob
On Mon, Sep 5, 2011 at 6:00 AM, Rob Herring <robherring2@gmail.com> wrote: >> @@ -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; >> >> /* >> * 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
On Sun, Sep 4, 2011 at 11:40 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > 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
On Monday 05 September 2011, Linus Walleij wrote: > On Sun, Sep 4, 2011 at 11:40 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > > > 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
On 09/05/2011 01:46 AM, Linus Walleij wrote: > On Mon, Sep 5, 2011 at 6:00 AM, Rob Herring <robherring2@gmail.com> wrote: > >>> @@ -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; >>> >>> /* >>> * 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. */ > 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 <rob.herring@calxeda.com> Rob
2011/9/5 Rob Herring <robherring2@gmail.com>: >>> 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? > 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. > Acked-by: Rob Herring <rob.herring@calxeda.com> Thanks! Linus Walleij
On 09/06/2011 12:25 AM, Linus Walleij wrote: > 2011/9/5 Rob Herring <robherring2@gmail.com>: > >>>> 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? > 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
2011/9/6 Rob Herring <robherring2@gmail.com>: > 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? > 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
diff --git a/arch/arm/mach-integrator/integrator_ap.c b/arch/arm/mach-integrator/integrator_ap.c index 2fbbdd5..4d88fc4 100644 --- a/arch/arm/mach-integrator/integrator_ap.c +++ b/arch/arm/mach-integrator/integrator_ap.c @@ -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 diff --git a/arch/arm/mach-integrator/pci_v3.c b/arch/arm/mach-integrator/pci_v3.c index dd56bfb..11b86e5 100644 --- a/arch/arm/mach-integrator/pci_v3.c +++ b/arch/arm/mach-integrator/pci_v3.c @@ -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
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 <rob.herring@calxeda.com> Cc: Nicolas Pitre <nicolas.pitre@linaro.org> Cc: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Linus Walleij <linus.walleij@stericsson.com> --- arch/arm/mach-integrator/integrator_ap.c | 2 ++ arch/arm/mach-integrator/pci_v3.c | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-)