Re: [PATCH 03/17] base: fix order of OF initialization

[Frank Rowand]

On 06/08/17 02:05, Mark Rutland wrote:
> On Wed, Jun 07, 2017 at 08:49:43PM -0700, Frank Rowand wrote:
>> On 06/07/17 11:39, Wesley Terpstra wrote:
>>> It was a while ago that I debugged this. I already reported this bug
>>> to Benjamin Herrenschmidt (now in CC), and I believe he has a patch of
>>> his own to fix the same issue.
>>>
>>> As I understand it, of_core_init sets up the OF entries in
>>> /sys/firmware/devicetree. During platform bringup, when the system
>>> describes the cpu + cache hierarchy, it also makes an of_node symlink
>>> into that directory. However, if it doesn't exist yet, you get the
>>> warning.
>>>
>>> # ls -l /sys/devices/system/cpu/cpu3/of_node
>>> lrwxrwxrwx1 root root 0 Jan  1 00:00
>>> /sys/devices/system/cpu/cpu3/of_node ->
>>> ../../../../firmware/devicetree/base/cpus/cpu@3
>>>
>>> On Wed, Jun 7, 2017 at 2:35 AM, Mark Rutland  wrote:
 On Wed, Jun 07, 2017 at 09:07:20AM +0200, Geert Uytterhoeven wrote:
> CC devicetree folks
>
> On Wed, Jun 7, 2017 at 12:59 AM, Palmer Dabbelt  
> wrote:
>> From: "Wesley W. Terpstra" 
>>
>> This fixes: [0.01] cpu cpu0: Error -2 creating of_node link
>> ... which you get for every CPU on all architectures with a OF cpu/ node.

 I take it this means a /cpus node? Or the /cpus/cpu@* nodes?

 I'm not seeing this on arm64 when booting v4.12-rc4 with DT, so clearly
 this doesn't affect all such architectures.

 What path are these errors happening in?
>>
>> On the surface, the patch looks reasonable.  But it is not obvious to me why
>> the error message is occurring.  I would like to understand the cause before
>> saying the patch is good.
>>
>> What kernel version is showing the error?  For a specific architecture
>> (the patch lists 'riscv, nios, etc'), which config and device tree source?
>>
>> And again, what is the calling path?
> 
>>From having grepped around, I think this affects architectures which
> select CONFIG_GENERIC_CPU_DEVICES, which includes nios2.

Thanks Mark!  The "#ifdef CONFIG_GENERIC_CPU_DEVICES" in 
cpu_dev_register_generic()
explains why we don't see the error on ARM, ARM64, etc.  Without the CONFIG
option, register_cpu(cpu, i) is not called.


> In that case, driver_init() calls cpu_dev_init() before calling
> of_core_init(). Then we get the callchain:
> 
>cpu_dev_init()
> -> cpu_dev_register_generic()
> -> register_cpu(cpu, i)
> -> device_register(>dev)
> -> device_add(dev)
> -> device_add_class_symlinks(dev)
> 
> ... in device_add_class_symlinks, we we dev->of_node, and call
> sysfs_create_link(), which fails because we haven't called
> of_core_init() to register the sysfs devicetree directory yet.
> 
> Given that, this patch makes sense to me.
> 
> FWIW, with the commit message updated to describe the particular
> ordering problem:
> 
> Acked-by: Mark Rutland 

Agree with Mark's request to update the commit message, and also
I would like to see this shaken out in the -next tree.

Acked-by: Frank Rowand 

> 
> Thanks,
> Mark.
> 

Re: [PATCH 03/17] base: fix order of OF initialization

[Frank Rowand]

On 06/08/17 02:05, Mark Rutland wrote:
> On Wed, Jun 07, 2017 at 08:49:43PM -0700, Frank Rowand wrote:
>> On 06/07/17 11:39, Wesley Terpstra wrote:
>>> It was a while ago that I debugged this. I already reported this bug
>>> to Benjamin Herrenschmidt (now in CC), and I believe he has a patch of
>>> his own to fix the same issue.
>>>
>>> As I understand it, of_core_init sets up the OF entries in
>>> /sys/firmware/devicetree. During platform bringup, when the system
>>> describes the cpu + cache hierarchy, it also makes an of_node symlink
>>> into that directory. However, if it doesn't exist yet, you get the
>>> warning.
>>>
>>> # ls -l /sys/devices/system/cpu/cpu3/of_node
>>> lrwxrwxrwx1 root root 0 Jan  1 00:00
>>> /sys/devices/system/cpu/cpu3/of_node ->
>>> ../../../../firmware/devicetree/base/cpus/cpu@3
>>>
>>> On Wed, Jun 7, 2017 at 2:35 AM, Mark Rutland  wrote:
 On Wed, Jun 07, 2017 at 09:07:20AM +0200, Geert Uytterhoeven wrote:
> CC devicetree folks
>
> On Wed, Jun 7, 2017 at 12:59 AM, Palmer Dabbelt  
> wrote:
>> From: "Wesley W. Terpstra" 
>>
>> This fixes: [0.01] cpu cpu0: Error -2 creating of_node link
>> ... which you get for every CPU on all architectures with a OF cpu/ node.

 I take it this means a /cpus node? Or the /cpus/cpu@* nodes?

 I'm not seeing this on arm64 when booting v4.12-rc4 with DT, so clearly
 this doesn't affect all such architectures.

 What path are these errors happening in?
>>
>> On the surface, the patch looks reasonable.  But it is not obvious to me why
>> the error message is occurring.  I would like to understand the cause before
>> saying the patch is good.
>>
>> What kernel version is showing the error?  For a specific architecture
>> (the patch lists 'riscv, nios, etc'), which config and device tree source?
>>
>> And again, what is the calling path?
> 
>>From having grepped around, I think this affects architectures which
> select CONFIG_GENERIC_CPU_DEVICES, which includes nios2.

Thanks Mark!  The "#ifdef CONFIG_GENERIC_CPU_DEVICES" in 
cpu_dev_register_generic()
explains why we don't see the error on ARM, ARM64, etc.  Without the CONFIG
option, register_cpu(cpu, i) is not called.


> In that case, driver_init() calls cpu_dev_init() before calling
> of_core_init(). Then we get the callchain:
> 
>cpu_dev_init()
> -> cpu_dev_register_generic()
> -> register_cpu(cpu, i)
> -> device_register(>dev)
> -> device_add(dev)
> -> device_add_class_symlinks(dev)
> 
> ... in device_add_class_symlinks, we we dev->of_node, and call
> sysfs_create_link(), which fails because we haven't called
> of_core_init() to register the sysfs devicetree directory yet.
> 
> Given that, this patch makes sense to me.
> 
> FWIW, with the commit message updated to describe the particular
> ordering problem:
> 
> Acked-by: Mark Rutland 

Agree with Mark's request to update the commit message, and also
I would like to see this shaken out in the -next tree.

Acked-by: Frank Rowand 

> 
> Thanks,
> Mark.
> 

Re: [PATCH 03/17] base: fix order of OF initialization

[Mark Rutland]

On Wed, Jun 07, 2017 at 08:49:43PM -0700, Frank Rowand wrote:
> On 06/07/17 11:39, Wesley Terpstra wrote:
> > It was a while ago that I debugged this. I already reported this bug
> > to Benjamin Herrenschmidt (now in CC), and I believe he has a patch of
> > his own to fix the same issue.
> > 
> > As I understand it, of_core_init sets up the OF entries in
> > /sys/firmware/devicetree. During platform bringup, when the system
> > describes the cpu + cache hierarchy, it also makes an of_node symlink
> > into that directory. However, if it doesn't exist yet, you get the
> > warning.
> > 
> > # ls -l /sys/devices/system/cpu/cpu3/of_node
> > lrwxrwxrwx1 root root 0 Jan  1 00:00
> > /sys/devices/system/cpu/cpu3/of_node ->
> > ../../../../firmware/devicetree/base/cpus/cpu@3
> > 
> > On Wed, Jun 7, 2017 at 2:35 AM, Mark Rutland  wrote:
> >> On Wed, Jun 07, 2017 at 09:07:20AM +0200, Geert Uytterhoeven wrote:
> >>> CC devicetree folks
> >>>
> >>> On Wed, Jun 7, 2017 at 12:59 AM, Palmer Dabbelt  
> >>> wrote:
>  From: "Wesley W. Terpstra" 
> 
>  This fixes: [0.01] cpu cpu0: Error -2 creating of_node link
>  ... which you get for every CPU on all architectures with a OF cpu/ node.
> >>
> >> I take it this means a /cpus node? Or the /cpus/cpu@* nodes?
> >>
> >> I'm not seeing this on arm64 when booting v4.12-rc4 with DT, so clearly
> >> this doesn't affect all such architectures.
> >>
> >> What path are these errors happening in?
> 
> On the surface, the patch looks reasonable.  But it is not obvious to me why
> the error message is occurring.  I would like to understand the cause before
> saying the patch is good.
> 
> What kernel version is showing the error?  For a specific architecture
> (the patch lists 'riscv, nios, etc'), which config and device tree source?
> 
> And again, what is the calling path?

>From having grepped around, I think this affects architectures which
select CONFIG_GENERIC_CPU_DEVICES, which includes nios2.

In that case, driver_init() calls cpu_dev_init() before calling
of_core_init(). Then we get the callchain:

   cpu_dev_init()
-> cpu_dev_register_generic()
-> register_cpu(cpu, i)
-> device_register(>dev)
-> device_add(dev)
-> device_add_class_symlinks(dev)

... in device_add_class_symlinks, we we dev->of_node, and call
sysfs_create_link(), which fails because we haven't called
of_core_init() to register the sysfs devicetree directory yet.

Given that, this patch makes sense to me.

FWIW, with the commit message updated to describe the particular
ordering problem:

Acked-by: Mark Rutland 

Thanks,
Mark.

Re: [PATCH 03/17] base: fix order of OF initialization

[Mark Rutland]

On Wed, Jun 07, 2017 at 08:49:43PM -0700, Frank Rowand wrote:
> On 06/07/17 11:39, Wesley Terpstra wrote:
> > It was a while ago that I debugged this. I already reported this bug
> > to Benjamin Herrenschmidt (now in CC), and I believe he has a patch of
> > his own to fix the same issue.
> > 
> > As I understand it, of_core_init sets up the OF entries in
> > /sys/firmware/devicetree. During platform bringup, when the system
> > describes the cpu + cache hierarchy, it also makes an of_node symlink
> > into that directory. However, if it doesn't exist yet, you get the
> > warning.
> > 
> > # ls -l /sys/devices/system/cpu/cpu3/of_node
> > lrwxrwxrwx1 root root 0 Jan  1 00:00
> > /sys/devices/system/cpu/cpu3/of_node ->
> > ../../../../firmware/devicetree/base/cpus/cpu@3
> > 
> > On Wed, Jun 7, 2017 at 2:35 AM, Mark Rutland  wrote:
> >> On Wed, Jun 07, 2017 at 09:07:20AM +0200, Geert Uytterhoeven wrote:
> >>> CC devicetree folks
> >>>
> >>> On Wed, Jun 7, 2017 at 12:59 AM, Palmer Dabbelt  
> >>> wrote:
>  From: "Wesley W. Terpstra" 
> 
>  This fixes: [0.01] cpu cpu0: Error -2 creating of_node link
>  ... which you get for every CPU on all architectures with a OF cpu/ node.
> >>
> >> I take it this means a /cpus node? Or the /cpus/cpu@* nodes?
> >>
> >> I'm not seeing this on arm64 when booting v4.12-rc4 with DT, so clearly
> >> this doesn't affect all such architectures.
> >>
> >> What path are these errors happening in?
> 
> On the surface, the patch looks reasonable.  But it is not obvious to me why
> the error message is occurring.  I would like to understand the cause before
> saying the patch is good.
> 
> What kernel version is showing the error?  For a specific architecture
> (the patch lists 'riscv, nios, etc'), which config and device tree source?
> 
> And again, what is the calling path?

>From having grepped around, I think this affects architectures which
select CONFIG_GENERIC_CPU_DEVICES, which includes nios2.

In that case, driver_init() calls cpu_dev_init() before calling
of_core_init(). Then we get the callchain:

   cpu_dev_init()
-> cpu_dev_register_generic()
-> register_cpu(cpu, i)
-> device_register(>dev)
-> device_add(dev)
-> device_add_class_symlinks(dev)

... in device_add_class_symlinks, we we dev->of_node, and call
sysfs_create_link(), which fails because we haven't called
of_core_init() to register the sysfs devicetree directory yet.

Given that, this patch makes sense to me.

FWIW, with the commit message updated to describe the particular
ordering problem:

Acked-by: Mark Rutland 

Thanks,
Mark.

Re: [PATCH 03/17] base: fix order of OF initialization

[Frank Rowand]

On 06/07/17 11:39, Wesley Terpstra wrote:
> It was a while ago that I debugged this. I already reported this bug
> to Benjamin Herrenschmidt (now in CC), and I believe he has a patch of
> his own to fix the same issue.
> 
> As I understand it, of_core_init sets up the OF entries in
> /sys/firmware/devicetree. During platform bringup, when the system
> describes the cpu + cache hierarchy, it also makes an of_node symlink
> into that directory. However, if it doesn't exist yet, you get the
> warning.
> 
> # ls -l /sys/devices/system/cpu/cpu3/of_node
> lrwxrwxrwx1 root root 0 Jan  1 00:00
> /sys/devices/system/cpu/cpu3/of_node ->
> ../../../../firmware/devicetree/base/cpus/cpu@3
> 
> On Wed, Jun 7, 2017 at 2:35 AM, Mark Rutland  wrote:
>> On Wed, Jun 07, 2017 at 09:07:20AM +0200, Geert Uytterhoeven wrote:
>>> CC devicetree folks
>>>
>>> On Wed, Jun 7, 2017 at 12:59 AM, Palmer Dabbelt  wrote:
 From: "Wesley W. Terpstra" 

 This fixes: [0.01] cpu cpu0: Error -2 creating of_node link
 ... which you get for every CPU on all architectures with a OF cpu/ node.
>>
>> I take it this means a /cpus node? Or the /cpus/cpu@* nodes?
>>
>> I'm not seeing this on arm64 when booting v4.12-rc4 with DT, so clearly
>> this doesn't affect all such architectures.
>>
>> What path are these errors happening in?

On the surface, the patch looks reasonable.  But it is not obvious to me why
the error message is occurring.  I would like to understand the cause before
saying the patch is good.

What kernel version is showing the error?  For a specific architecture
(the patch lists 'riscv, nios, etc'), which config and device tree source?

And again, what is the calling path?

- Frank

>>
>> Thanks,
>> Mark.
>>

 This affects riscv, nios, etc.

 Signed-off-by: Palmer Dabbelt 
 ---
  drivers/base/init.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/base/init.c b/drivers/base/init.c
 index 48c0e220acc0..0dcd17e561d0 100644
 --- a/drivers/base/init.c
 +++ b/drivers/base/init.c
 @@ -31,9 +31,9 @@ void __init driver_init(void)
 /* These are also core pieces, but must come after the
  * core core pieces.
  */
 +   of_core_init();
 platform_bus_init();
 cpu_dev_init();
 memory_dev_init();
 container_dev_init();
 -   of_core_init();
  }
 --
 2.13.0
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 03/17] base: fix order of OF initialization

[Frank Rowand]

On 06/07/17 11:39, Wesley Terpstra wrote:
> It was a while ago that I debugged this. I already reported this bug
> to Benjamin Herrenschmidt (now in CC), and I believe he has a patch of
> his own to fix the same issue.
> 
> As I understand it, of_core_init sets up the OF entries in
> /sys/firmware/devicetree. During platform bringup, when the system
> describes the cpu + cache hierarchy, it also makes an of_node symlink
> into that directory. However, if it doesn't exist yet, you get the
> warning.
> 
> # ls -l /sys/devices/system/cpu/cpu3/of_node
> lrwxrwxrwx1 root root 0 Jan  1 00:00
> /sys/devices/system/cpu/cpu3/of_node ->
> ../../../../firmware/devicetree/base/cpus/cpu@3
> 
> On Wed, Jun 7, 2017 at 2:35 AM, Mark Rutland  wrote:
>> On Wed, Jun 07, 2017 at 09:07:20AM +0200, Geert Uytterhoeven wrote:
>>> CC devicetree folks
>>>
>>> On Wed, Jun 7, 2017 at 12:59 AM, Palmer Dabbelt  wrote:
 From: "Wesley W. Terpstra" 

 This fixes: [0.01] cpu cpu0: Error -2 creating of_node link
 ... which you get for every CPU on all architectures with a OF cpu/ node.
>>
>> I take it this means a /cpus node? Or the /cpus/cpu@* nodes?
>>
>> I'm not seeing this on arm64 when booting v4.12-rc4 with DT, so clearly
>> this doesn't affect all such architectures.
>>
>> What path are these errors happening in?

On the surface, the patch looks reasonable.  But it is not obvious to me why
the error message is occurring.  I would like to understand the cause before
saying the patch is good.

What kernel version is showing the error?  For a specific architecture
(the patch lists 'riscv, nios, etc'), which config and device tree source?

And again, what is the calling path?

- Frank

>>
>> Thanks,
>> Mark.
>>

 This affects riscv, nios, etc.

 Signed-off-by: Palmer Dabbelt 
 ---
  drivers/base/init.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/base/init.c b/drivers/base/init.c
 index 48c0e220acc0..0dcd17e561d0 100644
 --- a/drivers/base/init.c
 +++ b/drivers/base/init.c
 @@ -31,9 +31,9 @@ void __init driver_init(void)
 /* These are also core pieces, but must come after the
  * core core pieces.
  */
 +   of_core_init();
 platform_bus_init();
 cpu_dev_init();
 memory_dev_init();
 container_dev_init();
 -   of_core_init();
  }
 --
 2.13.0
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 03/17] base: fix order of OF initialization

[Benjamin Herrenschmidt]

On Wed, 2017-06-07 at 11:39 -0700, Wesley Terpstra wrote:
> It was a while ago that I debugged this. I already reported this bug
> to Benjamin Herrenschmidt (now in CC), and I believe he has a patch of
> his own to fix the same issue.
> 
> As I understand it, of_core_init sets up the OF entries in
> /sys/firmware/devicetree. During platform bringup, when the system
> describes the cpu + cache hierarchy, it also makes an of_node symlink
> into that directory. However, if it doesn't exist yet, you get the
> warning.

Ugh... yes I did a patch for that and I think it fell through the
cracks, I can't even find it anymore...

The patch quoted here is fine I think. Everything in the device model
can potentially use OF bits these days, it makes sense to have them
initialized earlier.

Cheers,
Ben.

> # ls -l /sys/devices/system/cpu/cpu3/of_node
> lrwxrwxrwx1 root root 0 Jan  1 00:00
> /sys/devices/system/cpu/cpu3/of_node ->
> ../../../../firmware/devicetree/base/cpus/cpu@3
> 
> On Wed, Jun 7, 2017 at 2:35 AM, Mark Rutland  wrote:
> > On Wed, Jun 07, 2017 at 09:07:20AM +0200, Geert Uytterhoeven wrote:
> > > CC devicetree folks
> > > 
> > > On Wed, Jun 7, 2017 at 12:59 AM, Palmer Dabbelt  
> > > wrote:
> > > > From: "Wesley W. Terpstra" 
> > > > 
> > > > This fixes: [0.01] cpu cpu0: Error -2 creating of_node link
> > > > ... which you get for every CPU on all architectures with a OF cpu/ 
> > > > node.
> > 
> > I take it this means a /cpus node? Or the /cpus/cpu@* nodes?
> > 
> > I'm not seeing this on arm64 when booting v4.12-rc4 with DT, so clearly
> > this doesn't affect all such architectures.
> > 
> > What path are these errors happening in?
> > 
> > Thanks,
> > Mark.
> > 
> > > > 
> > > > This affects riscv, nios, etc.
> > > > 
> > > > Signed-off-by: Palmer Dabbelt 
> > > > ---
> > > >  drivers/base/init.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/base/init.c b/drivers/base/init.c
> > > > index 48c0e220acc0..0dcd17e561d0 100644
> > > > --- a/drivers/base/init.c
> > > > +++ b/drivers/base/init.c
> > > > @@ -31,9 +31,9 @@ void __init driver_init(void)
> > > > /* These are also core pieces, but must come after the
> > > >  * core core pieces.
> > > >  */
> > > > +   of_core_init();
> > > > platform_bus_init();
> > > > cpu_dev_init();
> > > > memory_dev_init();
> > > > container_dev_init();
> > > > -   of_core_init();
> > > >  }
> > > > --
> > > > 2.13.0
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe devicetree" in
> > > the body of a message to majord...@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/17] base: fix order of OF initialization

[Benjamin Herrenschmidt]

On Wed, 2017-06-07 at 11:39 -0700, Wesley Terpstra wrote:
> It was a while ago that I debugged this. I already reported this bug
> to Benjamin Herrenschmidt (now in CC), and I believe he has a patch of
> his own to fix the same issue.
> 
> As I understand it, of_core_init sets up the OF entries in
> /sys/firmware/devicetree. During platform bringup, when the system
> describes the cpu + cache hierarchy, it also makes an of_node symlink
> into that directory. However, if it doesn't exist yet, you get the
> warning.

Ugh... yes I did a patch for that and I think it fell through the
cracks, I can't even find it anymore...

The patch quoted here is fine I think. Everything in the device model
can potentially use OF bits these days, it makes sense to have them
initialized earlier.

Cheers,
Ben.

> # ls -l /sys/devices/system/cpu/cpu3/of_node
> lrwxrwxrwx1 root root 0 Jan  1 00:00
> /sys/devices/system/cpu/cpu3/of_node ->
> ../../../../firmware/devicetree/base/cpus/cpu@3
> 
> On Wed, Jun 7, 2017 at 2:35 AM, Mark Rutland  wrote:
> > On Wed, Jun 07, 2017 at 09:07:20AM +0200, Geert Uytterhoeven wrote:
> > > CC devicetree folks
> > > 
> > > On Wed, Jun 7, 2017 at 12:59 AM, Palmer Dabbelt  
> > > wrote:
> > > > From: "Wesley W. Terpstra" 
> > > > 
> > > > This fixes: [0.01] cpu cpu0: Error -2 creating of_node link
> > > > ... which you get for every CPU on all architectures with a OF cpu/ 
> > > > node.
> > 
> > I take it this means a /cpus node? Or the /cpus/cpu@* nodes?
> > 
> > I'm not seeing this on arm64 when booting v4.12-rc4 with DT, so clearly
> > this doesn't affect all such architectures.
> > 
> > What path are these errors happening in?
> > 
> > Thanks,
> > Mark.
> > 
> > > > 
> > > > This affects riscv, nios, etc.
> > > > 
> > > > Signed-off-by: Palmer Dabbelt 
> > > > ---
> > > >  drivers/base/init.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/base/init.c b/drivers/base/init.c
> > > > index 48c0e220acc0..0dcd17e561d0 100644
> > > > --- a/drivers/base/init.c
> > > > +++ b/drivers/base/init.c
> > > > @@ -31,9 +31,9 @@ void __init driver_init(void)
> > > > /* These are also core pieces, but must come after the
> > > >  * core core pieces.
> > > >  */
> > > > +   of_core_init();
> > > > platform_bus_init();
> > > > cpu_dev_init();
> > > > memory_dev_init();
> > > > container_dev_init();
> > > > -   of_core_init();
> > > >  }
> > > > --
> > > > 2.13.0
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe devicetree" in
> > > the body of a message to majord...@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/17] base: fix order of OF initialization

[Wesley Terpstra]

It was a while ago that I debugged this. I already reported this bug
to Benjamin Herrenschmidt (now in CC), and I believe he has a patch of
his own to fix the same issue.

As I understand it, of_core_init sets up the OF entries in
/sys/firmware/devicetree. During platform bringup, when the system
describes the cpu + cache hierarchy, it also makes an of_node symlink
into that directory. However, if it doesn't exist yet, you get the
warning.

# ls -l /sys/devices/system/cpu/cpu3/of_node
lrwxrwxrwx1 root root 0 Jan  1 00:00
/sys/devices/system/cpu/cpu3/of_node ->
../../../../firmware/devicetree/base/cpus/cpu@3

On Wed, Jun 7, 2017 at 2:35 AM, Mark Rutland  wrote:
> On Wed, Jun 07, 2017 at 09:07:20AM +0200, Geert Uytterhoeven wrote:
>> CC devicetree folks
>>
>> On Wed, Jun 7, 2017 at 12:59 AM, Palmer Dabbelt  wrote:
>> > From: "Wesley W. Terpstra" 
>> >
>> > This fixes: [0.01] cpu cpu0: Error -2 creating of_node link
>> > ... which you get for every CPU on all architectures with a OF cpu/ node.
>
> I take it this means a /cpus node? Or the /cpus/cpu@* nodes?
>
> I'm not seeing this on arm64 when booting v4.12-rc4 with DT, so clearly
> this doesn't affect all such architectures.
>
> What path are these errors happening in?
>
> Thanks,
> Mark.
>
>> >
>> > This affects riscv, nios, etc.
>> >
>> > Signed-off-by: Palmer Dabbelt 
>> > ---
>> >  drivers/base/init.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/base/init.c b/drivers/base/init.c
>> > index 48c0e220acc0..0dcd17e561d0 100644
>> > --- a/drivers/base/init.c
>> > +++ b/drivers/base/init.c
>> > @@ -31,9 +31,9 @@ void __init driver_init(void)
>> > /* These are also core pieces, but must come after the
>> >  * core core pieces.
>> >  */
>> > +   of_core_init();
>> > platform_bus_init();
>> > cpu_dev_init();
>> > memory_dev_init();
>> > container_dev_init();
>> > -   of_core_init();
>> >  }
>> > --
>> > 2.13.0
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/17] base: fix order of OF initialization

[Wesley Terpstra]

It was a while ago that I debugged this. I already reported this bug
to Benjamin Herrenschmidt (now in CC), and I believe he has a patch of
his own to fix the same issue.

As I understand it, of_core_init sets up the OF entries in
/sys/firmware/devicetree. During platform bringup, when the system
describes the cpu + cache hierarchy, it also makes an of_node symlink
into that directory. However, if it doesn't exist yet, you get the
warning.

# ls -l /sys/devices/system/cpu/cpu3/of_node
lrwxrwxrwx1 root root 0 Jan  1 00:00
/sys/devices/system/cpu/cpu3/of_node ->
../../../../firmware/devicetree/base/cpus/cpu@3

On Wed, Jun 7, 2017 at 2:35 AM, Mark Rutland  wrote:
> On Wed, Jun 07, 2017 at 09:07:20AM +0200, Geert Uytterhoeven wrote:
>> CC devicetree folks
>>
>> On Wed, Jun 7, 2017 at 12:59 AM, Palmer Dabbelt  wrote:
>> > From: "Wesley W. Terpstra" 
>> >
>> > This fixes: [0.01] cpu cpu0: Error -2 creating of_node link
>> > ... which you get for every CPU on all architectures with a OF cpu/ node.
>
> I take it this means a /cpus node? Or the /cpus/cpu@* nodes?
>
> I'm not seeing this on arm64 when booting v4.12-rc4 with DT, so clearly
> this doesn't affect all such architectures.
>
> What path are these errors happening in?
>
> Thanks,
> Mark.
>
>> >
>> > This affects riscv, nios, etc.
>> >
>> > Signed-off-by: Palmer Dabbelt 
>> > ---
>> >  drivers/base/init.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/base/init.c b/drivers/base/init.c
>> > index 48c0e220acc0..0dcd17e561d0 100644
>> > --- a/drivers/base/init.c
>> > +++ b/drivers/base/init.c
>> > @@ -31,9 +31,9 @@ void __init driver_init(void)
>> > /* These are also core pieces, but must come after the
>> >  * core core pieces.
>> >  */
>> > +   of_core_init();
>> > platform_bus_init();
>> > cpu_dev_init();
>> > memory_dev_init();
>> > container_dev_init();
>> > -   of_core_init();
>> >  }
>> > --
>> > 2.13.0
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/17] base: fix order of OF initialization

[Mark Rutland]

On Wed, Jun 07, 2017 at 09:07:20AM +0200, Geert Uytterhoeven wrote:
> CC devicetree folks
> 
> On Wed, Jun 7, 2017 at 12:59 AM, Palmer Dabbelt  wrote:
> > From: "Wesley W. Terpstra" 
> >
> > This fixes: [0.01] cpu cpu0: Error -2 creating of_node link
> > ... which you get for every CPU on all architectures with a OF cpu/ node.

I take it this means a /cpus node? Or the /cpus/cpu@* nodes?

I'm not seeing this on arm64 when booting v4.12-rc4 with DT, so clearly
this doesn't affect all such architectures.

What path are these errors happening in?

Thanks,
Mark.

> >
> > This affects riscv, nios, etc.
> >
> > Signed-off-by: Palmer Dabbelt 
> > ---
> >  drivers/base/init.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/init.c b/drivers/base/init.c
> > index 48c0e220acc0..0dcd17e561d0 100644
> > --- a/drivers/base/init.c
> > +++ b/drivers/base/init.c
> > @@ -31,9 +31,9 @@ void __init driver_init(void)
> > /* These are also core pieces, but must come after the
> >  * core core pieces.
> >  */
> > +   of_core_init();
> > platform_bus_init();
> > cpu_dev_init();
> > memory_dev_init();
> > container_dev_init();
> > -   of_core_init();
> >  }
> > --
> > 2.13.0
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/17] base: fix order of OF initialization

[Mark Rutland]

On Wed, Jun 07, 2017 at 09:07:20AM +0200, Geert Uytterhoeven wrote:
> CC devicetree folks
> 
> On Wed, Jun 7, 2017 at 12:59 AM, Palmer Dabbelt  wrote:
> > From: "Wesley W. Terpstra" 
> >
> > This fixes: [0.01] cpu cpu0: Error -2 creating of_node link
> > ... which you get for every CPU on all architectures with a OF cpu/ node.

I take it this means a /cpus node? Or the /cpus/cpu@* nodes?

I'm not seeing this on arm64 when booting v4.12-rc4 with DT, so clearly
this doesn't affect all such architectures.

What path are these errors happening in?

Thanks,
Mark.

> >
> > This affects riscv, nios, etc.
> >
> > Signed-off-by: Palmer Dabbelt 
> > ---
> >  drivers/base/init.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/base/init.c b/drivers/base/init.c
> > index 48c0e220acc0..0dcd17e561d0 100644
> > --- a/drivers/base/init.c
> > +++ b/drivers/base/init.c
> > @@ -31,9 +31,9 @@ void __init driver_init(void)
> > /* These are also core pieces, but must come after the
> >  * core core pieces.
> >  */
> > +   of_core_init();
> > platform_bus_init();
> > cpu_dev_init();
> > memory_dev_init();
> > container_dev_init();
> > -   of_core_init();
> >  }
> > --
> > 2.13.0
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/17] base: fix order of OF initialization

[Geert Uytterhoeven]

CC devicetree folks

On Wed, Jun 7, 2017 at 12:59 AM, Palmer Dabbelt  wrote:
> From: "Wesley W. Terpstra" 
>
> This fixes: [0.01] cpu cpu0: Error -2 creating of_node link
> ... which you get for every CPU on all architectures with a OF cpu/ node.
>
> This affects riscv, nios, etc.
>
> Signed-off-by: Palmer Dabbelt 
> ---
>  drivers/base/init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/base/init.c b/drivers/base/init.c
> index 48c0e220acc0..0dcd17e561d0 100644
> --- a/drivers/base/init.c
> +++ b/drivers/base/init.c
> @@ -31,9 +31,9 @@ void __init driver_init(void)
> /* These are also core pieces, but must come after the
>  * core core pieces.
>  */
> +   of_core_init();
> platform_bus_init();
> cpu_dev_init();
> memory_dev_init();
> container_dev_init();
> -   of_core_init();
>  }
> --
> 2.13.0

Re: [PATCH 03/17] base: fix order of OF initialization

[Geert Uytterhoeven]

CC devicetree folks

On Wed, Jun 7, 2017 at 12:59 AM, Palmer Dabbelt  wrote:
> From: "Wesley W. Terpstra" 
>
> This fixes: [0.01] cpu cpu0: Error -2 creating of_node link
> ... which you get for every CPU on all architectures with a OF cpu/ node.
>
> This affects riscv, nios, etc.
>
> Signed-off-by: Palmer Dabbelt 
> ---
>  drivers/base/init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/base/init.c b/drivers/base/init.c
> index 48c0e220acc0..0dcd17e561d0 100644
> --- a/drivers/base/init.c
> +++ b/drivers/base/init.c
> @@ -31,9 +31,9 @@ void __init driver_init(void)
> /* These are also core pieces, but must come after the
>  * core core pieces.
>  */
> +   of_core_init();
> platform_bus_init();
> cpu_dev_init();
> memory_dev_init();
> container_dev_init();
> -   of_core_init();
>  }
> --
> 2.13.0