| Message ID | 1261474791.289871.854376051633.1.gpush@pororo (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Grant Likely |
| Headers | show |
On Tue, 2009-12-22 at 17:39 +0800, Jeremy Kerr wrote: > At present, the fdt code sets the kernel-wide initrd_start and > initrd_end variables when parsing /chosen. On ARM, we only set these > once the bootmem has been reserved. > > This change adds an arch callback to setup the initrd from the device > tree: > > void early_init_dt_setup_initrd_arch(unsigned long start, > unsigned long end); arch_early_init_dt_setup_initrd() makes more sense to me, but .. > +#ifdef CONFIG_BLK_DEV_INITRD > +void __init early_init_dt_setup_initrd_arch(unsigned long start, > + unsigned long end) > +{ > + initrd_start = (unsigned long)__va(start); > + initrd_end = (unsigned long)__va(end); > + initrd_below_start_ok = 1; > +} > +#endif Given you have two identical implementations why not make that the default and make it weak, and let ARM override it. cheers
Hi Michael, > > void early_init_dt_setup_initrd_arch(unsigned long start, > > unsigned long end); > > arch_early_init_dt_setup_initrd() makes more sense to me, but .. <foo>_arch has been the general convention for arch-specific hooks in drivers/of/. > > +#ifdef CONFIG_BLK_DEV_INITRD > > +void __init early_init_dt_setup_initrd_arch(unsigned long start, > > + unsigned long end) > > +{ > > + initrd_start = (unsigned long)__va(start); > > + initrd_end = (unsigned long)__va(end); > > + initrd_below_start_ok = 1; > > +} > > +#endif > > Given you have two identical implementations why not make that the > default and make it weak, and let ARM override it. Yeah, that would be good too; just been avoiding weak as a potential source of magic voodoo complexity. Grant - up to you on this one. Cheers, Jeremy
On Tue, 2009-12-22 at 18:54 +0800, Jeremy Kerr wrote: > Hi Michael, > > > > void early_init_dt_setup_initrd_arch(unsigned long start, > > > unsigned long end); > > > > arch_early_init_dt_setup_initrd() makes more sense to me, but .. > > <foo>_arch has been the general convention for arch-specific hooks in > drivers/of/. Yuck, doh, guess I should have read those patches before they went in :) > > > +#ifdef CONFIG_BLK_DEV_INITRD > > > +void __init early_init_dt_setup_initrd_arch(unsigned long start, > > > + unsigned long end) > > > +{ > > > + initrd_start = (unsigned long)__va(start); > > > + initrd_end = (unsigned long)__va(end); > > > + initrd_below_start_ok = 1; > > > +} > > > +#endif > > > > Given you have two identical implementations why not make that the > > default and make it weak, and let ARM override it. > > Yeah, that would be good too; just been avoiding weak as a potential source of > magic voodoo complexity. Grant - up to you on this one. Yeah, depends on what toolchains you're supporting, modern ones should be OK but it can be troublesome. cheers
On Tue, Dec 22, 2009 at 6:17 AM, Michael Ellerman <michael@ellerman.id.au> wrote: > On Tue, 2009-12-22 at 18:54 +0800, Jeremy Kerr wrote: >> Hi Michael, >> >> > > void early_init_dt_setup_initrd_arch(unsigned long start, >> > > unsigned long end); >> > >> > arch_early_init_dt_setup_initrd() makes more sense to me, but .. >> >> <foo>_arch has been the general convention for arch-specific hooks in >> drivers/of/. > > Yuck, doh, guess I should have read those patches before they went in :) It's not necessarily permanent. My first goal is to get the common code merged. Then I want to look closely at it for patterns and refactor how the common code calls out to arch specific hooks (or maybe turn it around and have arch code calling out to the common bits). > >> > > +#ifdef CONFIG_BLK_DEV_INITRD >> > > +void __init early_init_dt_setup_initrd_arch(unsigned long start, >> > > + unsigned long end) >> > > +{ >> > > + initrd_start = (unsigned long)__va(start); >> > > + initrd_end = (unsigned long)__va(end); >> > > + initrd_below_start_ok = 1; >> > > +} >> > > +#endif >> > >> > Given you have two identical implementations why not make that the >> > default and make it weak, and let ARM override it. >> >> Yeah, that would be good too; just been avoiding weak as a potential source of >> magic voodoo complexity. Grant - up to you on this one. > > Yeah, depends on what toolchains you're supporting, modern ones should > be OK but it can be troublesome. I'll look at this. g.
On Tue, 2010-01-12 at 23:43 -0700, Grant Likely wrote: > On Tue, Dec 22, 2009 at 6:17 AM, Michael Ellerman > <michael@ellerman.id.au> wrote: > > On Tue, 2009-12-22 at 18:54 +0800, Jeremy Kerr wrote: > >> Hi Michael, > >> > >> > > void early_init_dt_setup_initrd_arch(unsigned long start, > >> > > unsigned long end); > >> > > >> > arch_early_init_dt_setup_initrd() makes more sense to me, but .. > >> > >> <foo>_arch has been the general convention for arch-specific hooks in > >> drivers/of/. > > > > Yuck, doh, guess I should have read those patches before they went in :) > > It's not necessarily permanent. My first goal is to get the common > code merged. Then I want to look closely at it for patterns and > refactor how the common code calls out to arch specific hooks (or > maybe turn it around and have arch code calling out to the common > bits). Sure, no biggy. There is lots of precedent for arch hooks to be called arch_foo() or topic_arch_foo(), but it's not the end of the world. cheers
diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c index 15853de..4fb5d87 100644 --- a/arch/microblaze/kernel/prom.c +++ b/arch/microblaze/kernel/prom.c @@ -101,6 +101,16 @@ void __init early_init_devtree_arch(void) /* No Microblaze specific code here */ } +#ifdef CONFIG_BLK_DEV_INITRD +void __init early_init_dt_setup_initrd_arch(unsigned long start, + unsigned long end) +{ + initrd_start = (unsigned long)__va(start); + initrd_end = (unsigned long)__va(end); + initrd_below_start_ok = 1; +} +#endif + /******* * * New implementation of the OF "find" APIs, return a refcounted diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index 6fea025..236b02c 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -467,6 +467,16 @@ int __init early_init_dt_scan_memory_arch(unsigned long node, const char *uname, } #endif /* CONFIG_PPC_PSERIES */ +#ifdef CONFIG_BLK_DEV_INITRD +void __init early_init_dt_setup_initrd_arch(unsigned long start, + unsigned long end) +{ + initrd_start = (unsigned long)__va(start); + initrd_end = (unsigned long)__va(end); + initrd_below_start_ok = 1; +} +#endif + void __init early_reserve_mem(void) { u64 base, size; diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index 7cb386c..ad0b09a 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -398,28 +398,23 @@ unsigned long __init unflatten_dt_node(unsigned long mem, */ void __init early_init_dt_check_for_initrd(unsigned long node) { - unsigned long len; + unsigned long start, end, len; __be32 *prop; pr_debug("Looking for initrd properties... "); prop = of_get_flat_dt_prop(node, "linux,initrd-start", &len); if (prop) { - initrd_start = (unsigned long) - __va(of_read_ulong(prop, len/4)); + start = of_read_ulong(prop, len/4); prop = of_get_flat_dt_prop(node, "linux,initrd-end", &len); if (prop) { - initrd_end = (unsigned long) - __va(of_read_ulong(prop, len/4)); - initrd_below_start_ok = 1; - } else { - initrd_start = 0; + end = of_read_ulong(prop, len/4); + early_init_dt_setup_initrd_arch(start, end); } } - pr_debug("initrd_start=0x%lx initrd_end=0x%lx\n", - initrd_start, initrd_end); + pr_debug("initrd_start=0x%lx initrd_end=0x%lx\n", start, end); } #else inline void early_init_dt_check_for_initrd(unsigned long node) diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h index 77ae0a4..4aba9a6 100644 --- a/include/linux/of_fdt.h +++ b/include/linux/of_fdt.h @@ -81,6 +81,16 @@ extern void early_reserve_mem(void); extern void early_init_devtree_arch(void); extern u64 dt_mem_next_cell(int s, __be32 **cellp); +/* + * If BLK_DEV_INITRD, the fdt early init code will call this function, + * to be provided by the arch code. start and end are specified as + * physical addresses. + */ +#ifdef CONFIG_BLK_DEV_INITRD +extern void early_init_dt_setup_initrd_arch(unsigned long start, + unsigned long end); +#endif + /* With CONFIG_HAVE_LMB, we can just use the lmb_ functions to add & allocate * memory; otherwise, the arch has to provide its own functions to do this. */
At present, the fdt code sets the kernel-wide initrd_start and initrd_end variables when parsing /chosen. On ARM, we only set these once the bootmem has been reserved. This change adds an arch callback to setup the initrd from the device tree: void early_init_dt_setup_initrd_arch(unsigned long start, unsigned long end); The arch-specific code can then setup the initrd however it likes. Compiled on powerpc, with CONFIG_BLK_DEV_INITRD=y and =n. Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com> --- arch/microblaze/kernel/prom.c | 10 ++++++++++ arch/powerpc/kernel/prom.c | 10 ++++++++++ drivers/of/fdt.c | 15 +++++---------- include/linux/of_fdt.h | 10 ++++++++++ 4 files changed, 35 insertions(+), 10 deletions(-)