Re: [PATCH 09/20] of/fdt: create common debugfs

[Michal Simek <monstr@monstr.eu>]

[-- Attachment #1: Type: text/plain, Size: 4876 bytes --]

On 04/04/2014 03:32 PM, Rob Herring wrote:
> 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.

Early console on uart16650 at 0x40401000
bootconsole [earlyser0] enabled
Ramdisk addr 0x00000000,
FDT at 0x806b96d4
Linux version 3.14.0-rc8-next-20140331-12541-g43b5fdb-dirty (monstr@monstr-desktop) (gcc version 4.6.4 20120924 (prerelease) (crosstool-NG 1.18.0) ) #51 Fri Apr 4 16:09:17 CEST 2014
 -> unflatten_device_tree()
Unflattening device tree:
magic: d00dfeed
size: 00014e20
version: 00000011
unflatten_dt_node blob c0298b00, c0331f84 0
unflatten_dt_node 1a c0298b3c/ -- len 0
unflatten_dt_node 8 old offset 0
unflatten_dt_node 8 new offset 6c
unflatten_dt_node blob c0298b00, c0331f84 6c
unflatten_dt_node 1a c0298ba8/aliases -- len 7
unflatten_dt_node 8 old offset 6c
unflatten_dt_node 8 new offset cc
unflatten_dt_node blob c0298b00, c0331f84 cc
unflatten_dt_node 1a c0298c08/chosen -- len 6
unflatten_dt_node 8 old offset cc
unflatten_dt_node 8 new offset 130
unflatten_dt_node blob c0298b00, c0331f84 130
unflatten_dt_node 1a c0298c6c/cpus -- len 4
unflatten_dt_node 8 old offset 130
unflatten_dt_node 8 new offset 16c
unflatten_dt_node blob c0298b00, c0331f84 16c
unflatten_dt_node 1a c0298ca8/cpu@0 -- len 5
unflatten_dt_node 8 old offset 16c
unflatten_dt_node 8 new offset 7bc
unflatten_dt_node blob c0298b00, c0331f84 7bc
unflatten_dt_node 1a   (null)/NULL -- len fffffffc

Below is the possition of debug messages.

Thanks,
Michal

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index ee8853c..5422e4e 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -8,6 +8,7 @@
  * modify it under the terms of the GNU General Public License
  * version 2 as published by the Free Software Foundation.
  */
+#define DEBUG

 #include <linux/kernel.h>
 #include <linux/initrd.h>
@@ -116,9 +117,15 @@ static void * unflatten_dt_node(struct boot_param_header *blob,
        int has_name = 0;
        int new_format = 0;

+printk("%s blob %x, %x %x\n", __func__, blob, poffset,  *poffset);
+
        pathp = fdt_get_name(blob, *poffset, &l);
+printk("%s 1a %p/%s -- len %x\n", __func__, pathp, pathp ? pathp : "NULL", l);
+
        allocl = l++;

+       if (!pathp)
+               while (1);
        /* 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
@@ -267,7 +274,9 @@ static void * unflatten_dt_node(struct boot_param_header *blob,
        }

        old_depth = depth;
+printk("%s 8 old offset %x\n", __func__, *poffset);
        *poffset = fdt_next_node(blob, *poffset, &depth);
+printk("%s 8 new offset %x\n", __func__, *poffset);
        while (*poffset > 0 && depth > old_depth) {
                mem = unflatten_dt_node(blob, mem, poffset, np, allnextpp,
                                                fpsize);






-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]