| Message ID | 1396563423-30893-10-git-send-email-robherring2@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On 04/04/2014 12:16 AM, Rob Herring wrote: > From: Rob Herring <robh@kernel.org> > > Both powerpc and microblaze have the same FDT blob in debugfs feature. > Move this to common location and remove the powerpc and microblaze > implementations. This feature could become more useful when FDT > overlay support is added. > > This changes the path of the blob from "$arch/flat-device-tree" to > "device-tree/flat-device-tree". > > Signed-off-by: Rob Herring <robh@kernel.org> > Cc: Michal Simek <monstr@monstr.eu> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Cc: Paul Mackerras <paulus@samba.org> > Cc: linuxppc-dev@lists.ozlabs.org > --- > arch/microblaze/kernel/prom.c | 31 ------------------------------- > arch/powerpc/kernel/prom.c | 21 --------------------- > drivers/of/fdt.c | 24 ++++++++++++++++++++++++ > 3 files changed, 24 insertions(+), 52 deletions(-) > > diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c > index abdfb10..1312cd2 100644 > --- a/arch/microblaze/kernel/prom.c > +++ b/arch/microblaze/kernel/prom.c > @@ -114,34 +114,3 @@ void __init early_init_devtree(void *params) > > pr_debug(" <- early_init_devtree()\n"); > } > - > -/******* > - * > - * New implementation of the OF "find" APIs, return a refcounted > - * object, call of_node_put() when done. The device tree and list > - * are protected by a rw_lock. > - * > - * Note that property management will need some locking as well, > - * this isn't dealt with yet. > - * > - *******/ > - > -#if defined(CONFIG_DEBUG_FS) && defined(DEBUG) > -static struct debugfs_blob_wrapper flat_dt_blob; > - > -static int __init export_flat_device_tree(void) > -{ > - struct dentry *d; > - > - flat_dt_blob.data = initial_boot_params; > - flat_dt_blob.size = initial_boot_params->totalsize; As I see even microblaze version was buggy. ... > diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c > index fa16a91..2085d47 100644 > --- a/drivers/of/fdt.c > +++ b/drivers/of/fdt.c > @@ -20,6 +20,7 @@ > #include <linux/string.h> > #include <linux/errno.h> > #include <linux/slab.h> > +#include <linux/debugfs.h> > > #include <asm/setup.h> /* for COMMAND_LINE_SIZE */ > #ifdef CONFIG_PPC > @@ -1084,4 +1085,27 @@ void __init unflatten_and_copy_device_tree(void) > unflatten_device_tree(); > } > > +#if defined(CONFIG_DEBUG_FS) && defined(DEBUG) > +static struct debugfs_blob_wrapper flat_dt_blob; > + > +static int __init of_flat_dt_debugfs_export_fdt(void) > +{ > + struct dentry *d = debugfs_create_dir("device-tree", NULL); > + > + if (!d) > + return -ENOENT; > + > + flat_dt_blob.data = initial_boot_params; > + flat_dt_blob.size = fdt_totalsize(initial_boot_params); Have you tried to compile this? From my tests fdt_totalsize is not available for target just for host from libfdt.h drivers/of/fdt.c: In function 'of_flat_dt_debugfs_export_fdt': drivers/of/fdt.c:957:2: error: implicit declaration of function 'fdt_totalsize' [-Werror=implicit-function-declaration] Thanks, Michal
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c >> index fa16a91..2085d47 100644 >> --- a/drivers/of/fdt.c >> +++ b/drivers/of/fdt.c >> @@ -20,6 +20,7 @@ >> #include <linux/string.h> >> #include <linux/errno.h> >> #include <linux/slab.h> >> +#include <linux/debugfs.h> >> >> #include <asm/setup.h> /* for COMMAND_LINE_SIZE */ >> #ifdef CONFIG_PPC >> @@ -1084,4 +1085,27 @@ void __init unflatten_and_copy_device_tree(void) >> unflatten_device_tree(); >> } >> >> +#if defined(CONFIG_DEBUG_FS) && defined(DEBUG) >> +static struct debugfs_blob_wrapper flat_dt_blob; >> + >> +static int __init of_flat_dt_debugfs_export_fdt(void) >> +{ >> + struct dentry *d = debugfs_create_dir("device-tree", NULL); >> + >> + if (!d) >> + return -ENOENT; >> + >> + flat_dt_blob.data = initial_boot_params; >> + flat_dt_blob.size = fdt_totalsize(initial_boot_params); > > Have you tried to compile this? > > From my tests fdt_totalsize is not available for target just for host > from libfdt.h > > drivers/of/fdt.c: In function 'of_flat_dt_debugfs_export_fdt': > drivers/of/fdt.c:957:2: error: implicit declaration of function 'fdt_totalsize' [-Werror=implicit-function-declaration] Ignore this one - there is no compilation problem. Thanks, Michal
On Fri, Apr 4, 2014 at 7:16 AM, Michal Simek <monstr@monstr.eu> wrote: > On 04/04/2014 12:16 AM, Rob Herring wrote: >> From: Rob Herring <robh@kernel.org> >> >> Both powerpc and microblaze have the same FDT blob in debugfs feature. >> Move this to common location and remove the powerpc and microblaze >> implementations. This feature could become more useful when FDT >> overlay support is added. >> >> This changes the path of the blob from "$arch/flat-device-tree" to >> "device-tree/flat-device-tree". [snip] >> -#if defined(CONFIG_DEBUG_FS) && defined(DEBUG) >> -static struct debugfs_blob_wrapper flat_dt_blob; >> - >> -static int __init export_flat_device_tree(void) >> -{ >> - struct dentry *d; >> - >> - flat_dt_blob.data = initial_boot_params; >> - flat_dt_blob.size = initial_boot_params->totalsize; > > As I see even microblaze version was buggy. How so? > ... > >> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c >> index fa16a91..2085d47 100644 >> --- a/drivers/of/fdt.c >> +++ b/drivers/of/fdt.c >> @@ -20,6 +20,7 @@ >> #include <linux/string.h> >> #include <linux/errno.h> >> #include <linux/slab.h> >> +#include <linux/debugfs.h> >> >> #include <asm/setup.h> /* for COMMAND_LINE_SIZE */ >> #ifdef CONFIG_PPC >> @@ -1084,4 +1085,27 @@ void __init unflatten_and_copy_device_tree(void) >> unflatten_device_tree(); >> } >> >> +#if defined(CONFIG_DEBUG_FS) && defined(DEBUG) >> +static struct debugfs_blob_wrapper flat_dt_blob; >> + >> +static int __init of_flat_dt_debugfs_export_fdt(void) >> +{ >> + struct dentry *d = debugfs_create_dir("device-tree", NULL); >> + >> + if (!d) >> + return -ENOENT; >> + >> + flat_dt_blob.data = initial_boot_params; >> + flat_dt_blob.size = fdt_totalsize(initial_boot_params); > > Have you tried to compile this? > > From my tests fdt_totalsize is not available for target just for host > from libfdt.h > > drivers/of/fdt.c: In function 'of_flat_dt_debugfs_export_fdt': > drivers/of/fdt.c:957:2: error: implicit declaration of function 'fdt_totalsize' [-Werror=implicit-function-declaration] Ah, it needs to be re-ordered after the libfdt conversion when libfdt.h gets added. Rob
On 04/04/2014 03:00 PM, Rob Herring wrote: > On Fri, Apr 4, 2014 at 7:16 AM, Michal Simek <monstr@monstr.eu> wrote: >> On 04/04/2014 12:16 AM, Rob Herring wrote: >>> From: Rob Herring <robh@kernel.org> >>> >>> Both powerpc and microblaze have the same FDT blob in debugfs feature. >>> Move this to common location and remove the powerpc and microblaze >>> implementations. This feature could become more useful when FDT >>> overlay support is added. >>> >>> This changes the path of the blob from "$arch/flat-device-tree" to >>> "device-tree/flat-device-tree". > > [snip] > >>> -#if defined(CONFIG_DEBUG_FS) && defined(DEBUG) >>> -static struct debugfs_blob_wrapper flat_dt_blob; >>> - >>> -static int __init export_flat_device_tree(void) >>> -{ >>> - struct dentry *d; >>> - >>> - flat_dt_blob.data = initial_boot_params; >>> - flat_dt_blob.size = initial_boot_params->totalsize; >> >> As I see even microblaze version was buggy. > > How so? if you compare it with powerpc version here is missing be to cpu conversion. > >> ... >> >>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c >>> index fa16a91..2085d47 100644 >>> --- a/drivers/of/fdt.c >>> +++ b/drivers/of/fdt.c >>> @@ -20,6 +20,7 @@ >>> #include <linux/string.h> >>> #include <linux/errno.h> >>> #include <linux/slab.h> >>> +#include <linux/debugfs.h> >>> >>> #include <asm/setup.h> /* for COMMAND_LINE_SIZE */ >>> #ifdef CONFIG_PPC >>> @@ -1084,4 +1085,27 @@ void __init unflatten_and_copy_device_tree(void) >>> unflatten_device_tree(); >>> } >>> >>> +#if defined(CONFIG_DEBUG_FS) && defined(DEBUG) >>> +static struct debugfs_blob_wrapper flat_dt_blob; >>> + >>> +static int __init of_flat_dt_debugfs_export_fdt(void) >>> +{ >>> + struct dentry *d = debugfs_create_dir("device-tree", NULL); >>> + >>> + if (!d) >>> + return -ENOENT; >>> + >>> + flat_dt_blob.data = initial_boot_params; >>> + flat_dt_blob.size = fdt_totalsize(initial_boot_params); >> >> Have you tried to compile this? >> >> From my tests fdt_totalsize is not available for target just for host >> from libfdt.h >> >> drivers/of/fdt.c: In function 'of_flat_dt_debugfs_export_fdt': >> drivers/of/fdt.c:957:2: error: implicit declaration of function 'fdt_totalsize' [-Werror=implicit-function-declaration] > > Ah, it needs to be re-ordered after the libfdt conversion when > libfdt.h gets added. I just pick some of them not all of them and send email. :-( Anyway I am testing it for microblaze and getting problem caused by this patch: commit 3d2ee8571ac0580d49c3f41fa28336289934900a Author: Rob Herring <robh@kernel.org> Date: Wed Apr 2 15:10:14 2014 -0500 of/fdt: Convert FDT functions to use libfdt And reason is that in unflatten_dt_node() pathp = fdt_get_name(blob, *poffset, &l); is returning NULL and here /* version 0x10 has a more compact unit name here instead of the full * path. we accumulate the full path size using "fpsize", we'll rebuild * it later. We detect this because the first character of the name is * not '/'. */ if ((*pathp) != '/') { code is trying to read it which is causing this kernel bug: Oops: kernel access of bad area, sig: 11 It means fdt_next_node(is doing something wrong) Any easy way how to debug it? Thanks, Michal
On Fri, Apr 4, 2014 at 8:22 AM, Michal Simek <monstr@monstr.eu> wrote: > On 04/04/2014 03:00 PM, Rob Herring wrote: >> On Fri, Apr 4, 2014 at 7:16 AM, Michal Simek <monstr@monstr.eu> wrote: >>> On 04/04/2014 12:16 AM, Rob Herring wrote: >>>> From: Rob Herring <robh@kernel.org> >>>> >>>> Both powerpc and microblaze have the same FDT blob in debugfs feature. >>>> Move this to common location and remove the powerpc and microblaze >>>> implementations. This feature could become more useful when FDT >>>> overlay support is added. >> >> [snip] > Anyway I am testing it for microblaze and getting problem > caused by this patch: > commit 3d2ee8571ac0580d49c3f41fa28336289934900a > Author: Rob Herring <robh@kernel.org> > Date: Wed Apr 2 15:10:14 2014 -0500 > > of/fdt: Convert FDT functions to use libfdt > > And reason is that in unflatten_dt_node() > > pathp = fdt_get_name(blob, *poffset, &l); > > is returning NULL > and here > /* version 0x10 has a more compact unit name here instead of the full > * path. we accumulate the full path size using "fpsize", we'll rebuild > * it later. We detect this because the first character of the name is > * not '/'. > */ > if ((*pathp) != '/') { > > code is trying to read it which is causing this kernel bug: > Oops: kernel access of bad area, sig: 11 > > It means fdt_next_node(is doing something wrong) > > Any easy way how to debug it? I didn't think fdt_get_path should fail. Can you add a print of *poffset and pathp values. Rob
diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c index abdfb10..1312cd2 100644 --- a/arch/microblaze/kernel/prom.c +++ b/arch/microblaze/kernel/prom.c @@ -114,34 +114,3 @@ void __init early_init_devtree(void *params) pr_debug(" <- early_init_devtree()\n"); } - -/******* - * - * New implementation of the OF "find" APIs, return a refcounted - * object, call of_node_put() when done. The device tree and list - * are protected by a rw_lock. - * - * Note that property management will need some locking as well, - * this isn't dealt with yet. - * - *******/ - -#if defined(CONFIG_DEBUG_FS) && defined(DEBUG) -static struct debugfs_blob_wrapper flat_dt_blob; - -static int __init export_flat_device_tree(void) -{ - struct dentry *d; - - flat_dt_blob.data = initial_boot_params; - flat_dt_blob.size = initial_boot_params->totalsize; - - d = debugfs_create_blob("flat-device-tree", S_IFREG | S_IRUSR, - of_debugfs_root, &flat_dt_blob); - if (!d) - return 1; - - return 0; -} -device_initcall(export_flat_device_tree); -#endif diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c index dd72beb..7c2f90c 100644 --- a/arch/powerpc/kernel/prom.c +++ b/arch/powerpc/kernel/prom.c @@ -29,7 +29,6 @@ #include <linux/bitops.h> #include <linux/export.h> #include <linux/kexec.h> -#include <linux/debugfs.h> #include <linux/irq.h> #include <linux/memblock.h> #include <linux/of.h> @@ -918,23 +917,3 @@ bool arch_match_cpu_phys_id(int cpu, u64 phys_id) { return (int)phys_id == get_hard_smp_processor_id(cpu); } - -#if defined(CONFIG_DEBUG_FS) && defined(DEBUG) -static struct debugfs_blob_wrapper flat_dt_blob; - -static int __init export_flat_device_tree(void) -{ - struct dentry *d; - - flat_dt_blob.data = initial_boot_params; - flat_dt_blob.size = be32_to_cpu(initial_boot_params->totalsize); - - d = debugfs_create_blob("flat-device-tree", S_IFREG | S_IRUSR, - powerpc_debugfs_root, &flat_dt_blob); - if (!d) - return 1; - - return 0; -} -__initcall(export_flat_device_tree); -#endif diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index fa16a91..2085d47 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -20,6 +20,7 @@ #include <linux/string.h> #include <linux/errno.h> #include <linux/slab.h> +#include <linux/debugfs.h> #include <asm/setup.h> /* for COMMAND_LINE_SIZE */ #ifdef CONFIG_PPC @@ -1084,4 +1085,27 @@ void __init unflatten_and_copy_device_tree(void) unflatten_device_tree(); } +#if defined(CONFIG_DEBUG_FS) && defined(DEBUG) +static struct debugfs_blob_wrapper flat_dt_blob; + +static int __init of_flat_dt_debugfs_export_fdt(void) +{ + struct dentry *d = debugfs_create_dir("device-tree", NULL); + + if (!d) + return -ENOENT; + + flat_dt_blob.data = initial_boot_params; + flat_dt_blob.size = fdt_totalsize(initial_boot_params); + + d = debugfs_create_blob("flat-device-tree", S_IFREG | S_IRUSR, + d, &flat_dt_blob); + if (!d) + return -ENOENT; + + return 0; +} +module_init(of_flat_dt_debugfs_export_fdt); +#endif + #endif /* CONFIG_OF_EARLY_FLATTREE */