Message ID | 1479587152-25065-53-git-send-email-sjg@chromium.org |
---|---|
State | Superseded |
Delegated to: | Bin Meng |
Headers | show |
Hi Simon, On Sun, Nov 20, 2016 at 4:25 AM, Simon Glass <sjg@chromium.org> wrote: > To avoid using BSS in SPL before SDRAM is set up, move this field to > global_data. > Why is this needed? pirq routing table setup is done after SDRAM initialization. Isn't SPL doing this with a different order? > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > Changes in v2: None > > arch/x86/cpu/irq.c | 12 +++++------- > arch/x86/include/asm/global_data.h | 1 + > 2 files changed, 6 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/cpu/irq.c b/arch/x86/cpu/irq.c > index e3e928b..442d373 100644 > --- a/arch/x86/cpu/irq.c > +++ b/arch/x86/cpu/irq.c > @@ -17,8 +17,6 @@ > > DECLARE_GLOBAL_DATA_PTR; > > -static struct irq_routing_table *pirq_routing_table; > - > bool pirq_check_irq_routed(struct udevice *dev, int link, u8 irq) > { > struct irq_router *priv = dev_get_priv(dev); > @@ -219,7 +217,7 @@ static int create_pirq_routing_table(struct udevice *dev) > /* Fix up the table checksum */ > rt->checksum = table_compute_checksum(rt, rt->size); > > - pirq_routing_table = rt; > + gd->arch.pirq_routing_table = rt; > > return 0; > } > @@ -250,8 +248,8 @@ int irq_router_common_init(struct udevice *dev) > return ret; > } > /* Route PIRQ */ > - pirq_route_irqs(dev, pirq_routing_table->slots, > - get_irq_slot_count(pirq_routing_table)); > + pirq_route_irqs(dev, gd->arch.pirq_routing_table->slots, > + get_irq_slot_count(gd->arch.pirq_routing_table)); > > if (IS_ENABLED(CONFIG_GENERATE_ACPI_TABLE)) > irq_enable_sci(dev); > @@ -266,10 +264,10 @@ int irq_router_probe(struct udevice *dev) > > ulong write_pirq_routing_table(ulong addr) > { > - if (!pirq_routing_table) > + if (!gd->arch.pirq_routing_table) > return addr; > > - return copy_pirq_routing_table(addr, pirq_routing_table); > + return copy_pirq_routing_table(addr, gd->arch.pirq_routing_table); > } > > static const struct udevice_id irq_router_ids[] = { > diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h > index ce9e5cc..e24cee7 100644 > --- a/arch/x86/include/asm/global_data.h > +++ b/arch/x86/include/asm/global_data.h > @@ -93,6 +93,7 @@ struct arch_global_data { > char *mrc_output; > unsigned int mrc_output_len; > ulong table; /* Table pointer from previous loader */ > + struct irq_routing_table *pirq_routing_table; > #ifdef CONFIG_SEABIOS > u32 high_table_ptr; > u32 high_table_limit; > -- Regards, Bin
Hi Bin, On 14 January 2017 at 06:32, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Sun, Nov 20, 2016 at 4:25 AM, Simon Glass <sjg@chromium.org> wrote: >> To avoid using BSS in SPL before SDRAM is set up, move this field to >> global_data. >> > > Why is this needed? pirq routing table setup is done after SDRAM > initialization. Isn't SPL doing this with a different order? I'm not sure why it should. SPL sets up SDRAM so it should be able to set up interrupts, shouldn't it? Some device init may need interrupts, and my plan was to do all the 32-bit init in SPL. > >> Signed-off-by: Simon Glass <sjg@chromium.org> >> --- >> >> Changes in v2: None >> >> arch/x86/cpu/irq.c | 12 +++++------- >> arch/x86/include/asm/global_data.h | 1 + >> 2 files changed, 6 insertions(+), 7 deletions(-) >> >> diff --git a/arch/x86/cpu/irq.c b/arch/x86/cpu/irq.c >> index e3e928b..442d373 100644 >> --- a/arch/x86/cpu/irq.c >> +++ b/arch/x86/cpu/irq.c >> @@ -17,8 +17,6 @@ >> >> DECLARE_GLOBAL_DATA_PTR; >> >> -static struct irq_routing_table *pirq_routing_table; >> - >> bool pirq_check_irq_routed(struct udevice *dev, int link, u8 irq) >> { >> struct irq_router *priv = dev_get_priv(dev); >> @@ -219,7 +217,7 @@ static int create_pirq_routing_table(struct udevice *dev) >> /* Fix up the table checksum */ >> rt->checksum = table_compute_checksum(rt, rt->size); >> >> - pirq_routing_table = rt; >> + gd->arch.pirq_routing_table = rt; >> >> return 0; >> } >> @@ -250,8 +248,8 @@ int irq_router_common_init(struct udevice *dev) >> return ret; >> } >> /* Route PIRQ */ >> - pirq_route_irqs(dev, pirq_routing_table->slots, >> - get_irq_slot_count(pirq_routing_table)); >> + pirq_route_irqs(dev, gd->arch.pirq_routing_table->slots, >> + get_irq_slot_count(gd->arch.pirq_routing_table)); >> >> if (IS_ENABLED(CONFIG_GENERATE_ACPI_TABLE)) >> irq_enable_sci(dev); >> @@ -266,10 +264,10 @@ int irq_router_probe(struct udevice *dev) >> >> ulong write_pirq_routing_table(ulong addr) >> { >> - if (!pirq_routing_table) >> + if (!gd->arch.pirq_routing_table) >> return addr; >> >> - return copy_pirq_routing_table(addr, pirq_routing_table); >> + return copy_pirq_routing_table(addr, gd->arch.pirq_routing_table); >> } >> >> static const struct udevice_id irq_router_ids[] = { >> diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h >> index ce9e5cc..e24cee7 100644 >> --- a/arch/x86/include/asm/global_data.h >> +++ b/arch/x86/include/asm/global_data.h >> @@ -93,6 +93,7 @@ struct arch_global_data { >> char *mrc_output; >> unsigned int mrc_output_len; >> ulong table; /* Table pointer from previous loader */ >> + struct irq_routing_table *pirq_routing_table; >> #ifdef CONFIG_SEABIOS >> u32 high_table_ptr; >> u32 high_table_limit; >> -- > > Regards, > Bin Regards, Simon
Hi Simon, On Mon, Jan 16, 2017 at 10:08 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 14 January 2017 at 06:32, Bin Meng <bmeng.cn@gmail.com> wrote: >> Hi Simon, >> >> On Sun, Nov 20, 2016 at 4:25 AM, Simon Glass <sjg@chromium.org> wrote: >>> To avoid using BSS in SPL before SDRAM is set up, move this field to >>> global_data. >>> >> >> Why is this needed? pirq routing table setup is done after SDRAM >> initialization. Isn't SPL doing this with a different order? > > I'm not sure why it should. SPL sets up SDRAM so it should be able to > set up interrupts, shouldn't it? Some device init may need interrupts, > and my plan was to do all the 32-bit init in SPL. Yes, but I see interrupt_init() is called after dram_init() in arch/x86/lib/spl.c, where BSS is already cleared after dram_init(). Am I missing anything? Regards, Bin
Hi Simon, On Tue, Jan 17, 2017 at 12:34 PM, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Mon, Jan 16, 2017 at 10:08 PM, Simon Glass <sjg@chromium.org> wrote: >> Hi Bin, >> >> On 14 January 2017 at 06:32, Bin Meng <bmeng.cn@gmail.com> wrote: >>> Hi Simon, >>> >>> On Sun, Nov 20, 2016 at 4:25 AM, Simon Glass <sjg@chromium.org> wrote: >>>> To avoid using BSS in SPL before SDRAM is set up, move this field to >>>> global_data. >>>> >>> >>> Why is this needed? pirq routing table setup is done after SDRAM >>> initialization. Isn't SPL doing this with a different order? >> >> I'm not sure why it should. SPL sets up SDRAM so it should be able to >> set up interrupts, shouldn't it? Some device init may need interrupts, >> and my plan was to do all the 32-bit init in SPL. > > Yes, but I see interrupt_init() is called after dram_init() in > arch/x86/lib/spl.c, where BSS is already cleared after dram_init(). Am > I missing anything? Could you have a look at my comments? Regards, Bin
Hi Bin, On 3 February 2017 at 22:01, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Tue, Jan 17, 2017 at 12:34 PM, Bin Meng <bmeng.cn@gmail.com> wrote: >> Hi Simon, >> >> On Mon, Jan 16, 2017 at 10:08 PM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Bin, >>> >>> On 14 January 2017 at 06:32, Bin Meng <bmeng.cn@gmail.com> wrote: >>>> Hi Simon, >>>> >>>> On Sun, Nov 20, 2016 at 4:25 AM, Simon Glass <sjg@chromium.org> wrote: >>>>> To avoid using BSS in SPL before SDRAM is set up, move this field to >>>>> global_data. >>>>> >>>> >>>> Why is this needed? pirq routing table setup is done after SDRAM >>>> initialization. Isn't SPL doing this with a different order? >>> >>> I'm not sure why it should. SPL sets up SDRAM so it should be able to >>> set up interrupts, shouldn't it? Some device init may need interrupts, >>> and my plan was to do all the 32-bit init in SPL. >> >> Yes, but I see interrupt_init() is called after dram_init() in >> arch/x86/lib/spl.c, where BSS is already cleared after dram_init(). Am >> I missing anything? > > Could you have a look at my comments? I have not got back to this yet... But if BSS is in ROM then we cannot use it. And I think that is the case with SPL. We don't really know the RAM address so cannot set up BSS to go somewhere else. Or at least it's tricky... Regards, Simon
diff --git a/arch/x86/cpu/irq.c b/arch/x86/cpu/irq.c index e3e928b..442d373 100644 --- a/arch/x86/cpu/irq.c +++ b/arch/x86/cpu/irq.c @@ -17,8 +17,6 @@ DECLARE_GLOBAL_DATA_PTR; -static struct irq_routing_table *pirq_routing_table; - bool pirq_check_irq_routed(struct udevice *dev, int link, u8 irq) { struct irq_router *priv = dev_get_priv(dev); @@ -219,7 +217,7 @@ static int create_pirq_routing_table(struct udevice *dev) /* Fix up the table checksum */ rt->checksum = table_compute_checksum(rt, rt->size); - pirq_routing_table = rt; + gd->arch.pirq_routing_table = rt; return 0; } @@ -250,8 +248,8 @@ int irq_router_common_init(struct udevice *dev) return ret; } /* Route PIRQ */ - pirq_route_irqs(dev, pirq_routing_table->slots, - get_irq_slot_count(pirq_routing_table)); + pirq_route_irqs(dev, gd->arch.pirq_routing_table->slots, + get_irq_slot_count(gd->arch.pirq_routing_table)); if (IS_ENABLED(CONFIG_GENERATE_ACPI_TABLE)) irq_enable_sci(dev); @@ -266,10 +264,10 @@ int irq_router_probe(struct udevice *dev) ulong write_pirq_routing_table(ulong addr) { - if (!pirq_routing_table) + if (!gd->arch.pirq_routing_table) return addr; - return copy_pirq_routing_table(addr, pirq_routing_table); + return copy_pirq_routing_table(addr, gd->arch.pirq_routing_table); } static const struct udevice_id irq_router_ids[] = { diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h index ce9e5cc..e24cee7 100644 --- a/arch/x86/include/asm/global_data.h +++ b/arch/x86/include/asm/global_data.h @@ -93,6 +93,7 @@ struct arch_global_data { char *mrc_output; unsigned int mrc_output_len; ulong table; /* Table pointer from previous loader */ + struct irq_routing_table *pirq_routing_table; #ifdef CONFIG_SEABIOS u32 high_table_ptr; u32 high_table_limit;
To avoid using BSS in SPL before SDRAM is set up, move this field to global_data. Signed-off-by: Simon Glass <sjg@chromium.org> --- Changes in v2: None arch/x86/cpu/irq.c | 12 +++++------- arch/x86/include/asm/global_data.h | 1 + 2 files changed, 6 insertions(+), 7 deletions(-)