Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Rob Herring]

On Tue, Nov 28, 2017 at 7:13 AM, Geert Uytterhoeven
 wrote:
> Hi Rob,
>
> On Mon, Aug 21, 2017 at 5:16 PM, Rob Herring  wrote:
>> With dependencies on a statically allocated full path name converted to
>> use %pOF format specifier, we can store just the basename of node, and
>> the unflattening of the FDT can be simplified.
>>
>> This commit will affect the remaining users of full_name. After
>> analyzing these users, the remaining cases should only change some print
>> messages. The main users of full_name are providing a name for struct
>> resource. The resource names shouldn't be important other than providing
>> /proc/iomem names.
>
> I guess the plan is to get rid in a subsequent step of all calls to 
> kbasename()
> on a full name, which is now futile?

No. Sparc (PDT) is still the full path and I don't plan to change that.

Rob

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Rob Herring]

On Tue, Nov 28, 2017 at 7:13 AM, Geert Uytterhoeven
 wrote:
> Hi Rob,
>
> On Mon, Aug 21, 2017 at 5:16 PM, Rob Herring  wrote:
>> With dependencies on a statically allocated full path name converted to
>> use %pOF format specifier, we can store just the basename of node, and
>> the unflattening of the FDT can be simplified.
>>
>> This commit will affect the remaining users of full_name. After
>> analyzing these users, the remaining cases should only change some print
>> messages. The main users of full_name are providing a name for struct
>> resource. The resource names shouldn't be important other than providing
>> /proc/iomem names.
>
> I guess the plan is to get rid in a subsequent step of all calls to 
> kbasename()
> on a full name, which is now futile?

No. Sparc (PDT) is still the full path and I don't plan to change that.

Rob

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Geert Uytterhoeven]

Hi Rob,

On Mon, Aug 21, 2017 at 5:16 PM, Rob Herring  wrote:
> With dependencies on a statically allocated full path name converted to
> use %pOF format specifier, we can store just the basename of node, and
> the unflattening of the FDT can be simplified.
>
> This commit will affect the remaining users of full_name. After
> analyzing these users, the remaining cases should only change some print
> messages. The main users of full_name are providing a name for struct
> resource. The resource names shouldn't be important other than providing
> /proc/iomem names.

I guess the plan is to get rid in a subsequent step of all calls to kbasename()
on a full name, which is now futile?

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Geert Uytterhoeven]

Hi Rob,

On Mon, Aug 21, 2017 at 5:16 PM, Rob Herring  wrote:
> With dependencies on a statically allocated full path name converted to
> use %pOF format specifier, we can store just the basename of node, and
> the unflattening of the FDT can be simplified.
>
> This commit will affect the remaining users of full_name. After
> analyzing these users, the remaining cases should only change some print
> messages. The main users of full_name are providing a name for struct
> resource. The resource names shouldn't be important other than providing
> /proc/iomem names.

I guess the plan is to get rid in a subsequent step of all calls to kbasename()
on a full name, which is now futile?

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Pantelis Antoniou]

Hi Frank,

> On Oct 20, 2017, at 00:46 , Frank Rowand  wrote:
> 
> On 10/19/17 13:06, Moritz Fischer wrote:
> 
> < snip >
> 
>> We also have plenty of code that is just not aware of overlays, and
>> assumes certain parts of the tree to stay static.
> 
> I would state that somewhat differently.  :-)  There is very little
> code that is aware of overlays, and most code assumes the device tree
> does not change after early boot.
> 
> This is one of the areas where the creation of overlays needs to be
> done with care.
> 

Correct. But this is not breaking the kernel.

In general we have the following case where we load overlays (at least
well formed overlays that are not doing stupid things).

1. Activation of a new device. Usually this works since is something that’s
normally done at boot.

2. Deactivation of a device. This might break because the removal paths
of platform device especially are not well tested (or never executed for that
matter).

3. Modification of properties in an already activated device. If the device 
driver
has not installed a device tree modification hook (as in almost 99% of the 
devices)
it will do absolutely nothing, since the device tree is parsed only at probe 
time.
I can argue that for these cases we could have a catch-all hook that displays a
message that nothing happened.

4. Modification of some part of the tree that’s not part of a device driver, 
perhaps
the aliases or chosen node. This may potentially be harmful or harmless 
depending on
what has been modified. For instance modifying an already existing alias might 
cause
internal inconsistency about device naming, while adding a new aliases should 
be harmless.
This is a matter of policy per board, whether to allow or not.

Are there other cases that are potentially more harmful?

> 
>> I ran into that issue when I tried to add thermal zones via an overlay,
>> I've been investigating how to fix the thermal framework to work with
>> overlays since then and have some partially working code.
>> Currently the thermal framework parses the thermal-zones node at boot,
>> and assumes this stays static. This breaks with overlays.
>> 
>> I agree we eventually need to fix the parts that break when we use
>> overlays.
> 

Regards

— Pantelis

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Pantelis Antoniou]

Hi Frank,

> On Oct 20, 2017, at 00:46 , Frank Rowand  wrote:
> 
> On 10/19/17 13:06, Moritz Fischer wrote:
> 
> < snip >
> 
>> We also have plenty of code that is just not aware of overlays, and
>> assumes certain parts of the tree to stay static.
> 
> I would state that somewhat differently.  :-)  There is very little
> code that is aware of overlays, and most code assumes the device tree
> does not change after early boot.
> 
> This is one of the areas where the creation of overlays needs to be
> done with care.
> 

Correct. But this is not breaking the kernel.

In general we have the following case where we load overlays (at least
well formed overlays that are not doing stupid things).

1. Activation of a new device. Usually this works since is something that’s
normally done at boot.

2. Deactivation of a device. This might break because the removal paths
of platform device especially are not well tested (or never executed for that
matter).

3. Modification of properties in an already activated device. If the device 
driver
has not installed a device tree modification hook (as in almost 99% of the 
devices)
it will do absolutely nothing, since the device tree is parsed only at probe 
time.
I can argue that for these cases we could have a catch-all hook that displays a
message that nothing happened.

4. Modification of some part of the tree that’s not part of a device driver, 
perhaps
the aliases or chosen node. This may potentially be harmful or harmless 
depending on
what has been modified. For instance modifying an already existing alias might 
cause
internal inconsistency about device naming, while adding a new aliases should 
be harmless.
This is a matter of policy per board, whether to allow or not.

Are there other cases that are potentially more harmful?

> 
>> I ran into that issue when I tried to add thermal zones via an overlay,
>> I've been investigating how to fix the thermal framework to work with
>> overlays since then and have some partially working code.
>> Currently the thermal framework parses the thermal-zones node at boot,
>> and assumes this stays static. This breaks with overlays.
>> 
>> I agree we eventually need to fix the parts that break when we use
>> overlays.
> 

Regards

— Pantelis

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Frank Rowand]

On 10/19/17 13:06, Moritz Fischer wrote:

< snip >

> We also have plenty of code that is just not aware of overlays, and
> assumes certain parts of the tree to stay static.

I would state that somewhat differently.  :-)  There is very little
code that is aware of overlays, and most code assumes the device tree
does not change after early boot.

This is one of the areas where the creation of overlays needs to be
done with care.


> I ran into that issue when I tried to add thermal zones via an overlay,
> I've been investigating how to fix the thermal framework to work with
> overlays since then and have some partially working code.
> Currently the thermal framework parses the thermal-zones node at boot,
> and assumes this stays static. This breaks with overlays.
> 
> I agree we eventually need to fix the parts that break when we use
> overlays.

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Frank Rowand]

On 10/19/17 13:06, Moritz Fischer wrote:

< snip >

> We also have plenty of code that is just not aware of overlays, and
> assumes certain parts of the tree to stay static.

I would state that somewhat differently.  :-)  There is very little
code that is aware of overlays, and most code assumes the device tree
does not change after early boot.

This is one of the areas where the creation of overlays needs to be
done with care.


> I ran into that issue when I tried to add thermal zones via an overlay,
> I've been investigating how to fix the thermal framework to work with
> overlays since then and have some partially working code.
> Currently the thermal framework parses the thermal-zones node at boot,
> and assumes this stays static. This breaks with overlays.
> 
> I agree we eventually need to fix the parts that break when we use
> overlays.

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Moritz Fischer]

On Thu, Oct 19, 2017 at 11:51:40AM +0300, Pantelis Antoniou wrote:
> Hi Rob,
> 
> > On Oct 18, 2017, at 21:30 , Rob Herring  wrote:
> > 
> > On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou
> >  wrote:
> >> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:
> >>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
>  On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  
>  wrote:
> > On 10/17/17 14:46, Rob Herring wrote:
> >> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
> >>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
> >>> 
> >>> Hi Rob,
> >>> 
>  With dependencies on a statically allocated full path name converted 
>  to
>  use %pOF format specifier, we can store just the basename of node, 
>  and
>  the unflattening of the FDT can be simplified.
>  
>  This commit will affect the remaining users of full_name. After
>  analyzing these users, the remaining cases should only change some 
>  print
>  messages. The main users of full_name are providing a name for struct
>  resource. The resource names shouldn't be important other than 
>  providing
>  /proc/iomem names.
>  
>  We no longer distinguish between pre and post 0x10 dtb formats as 
>  either
>  a full path or basename will work. However, less than 0x10 formats 
>  have
>  been broken since the conversion to use libfdt (and no one has 
>  cared).
>  The conversion of the unflattening code to be non-recursive also 
>  broke
>  pre 0x10 formats as the populate_node function would return 0 in that
>  case.
>  
>  Signed-off-by: Rob Herring 
>  ---
>  v2:
>  - rebase to linux-next
>  
>  drivers/of/fdt.c | 69 
>  +---
>  1 file changed, 11 insertions(+), 58 deletions(-)
> >>> 
> >>> I've just updated to the latest next branch and am finding problems
> >>> applying overlays.   Reverting this commit alleviates things.  The
> >>> errors I get are:
> >>> 
> >>> [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
> >>> [   88.513447] OF: overlay: apply failed '/__symbols__'
> >>> [   88.518423] create_overlay: Failed to create overlay (err=-12)
> >> 
> >> Frank's series with overlay updates should fix this.
> > 
> > Yes, it does:
> > 
> >  [PATCH v3 11/12] of: overlay: remove a dependency on device node 
> > full_name
>  
>  Thanks for the fast response.  I fetched the dt/next branch to test
>  this but there are sufficient changes that Pantelis' "OF: DT-Overlay
>  configfs interface (v7)" is broken now.  I've been adding that
>  downstream since 4.4.  We're using it as an interface for applying
>  overlays to program FPGAs.  If we fix it again, is there any chance
>  that can go upstream now?
> >>> 
> >>> With a drive-by posting once every few years, no.
> >>> 
> >> 
> >> I take offense to that. There's nothing changed in the patch for years.
> >> Reposting the same patch without changes would achieve nothing.
> > 
> > Are you still expecting review comments on it or something?
> > Furthermore, If something is posted infrequently, then I'm not
> > inclined to comment or care if the next posting is going to be after I
> > forget what I previously said (which is not very long).
> > 
> > I'm just saying, don't expect to forward port, post and it will be
> > accepted. Below is minimally one of the issues that needs to be
> > addressed.
> > 
> >>> The issue remains that the kernel is not really setup to deal with any
> >>> random property or node to be changed at any point in run-time. I
> >>> think there needs to be some restrictions around what the overlays can
> >>> touch. We can't have it be wide open and then lock things down later
> >>> and break users. One example of what you could do is you can only add
> >>> sub-trees to whitelisted nodes. That's probably acceptable for your
> >>> usecase.
> >>> 
> >> 
> >> Defining what can and what cannot be changed is not as trivial as a
> >> list of white-listed nodes.
> > 
> > No, but we have to start somewhere and we are not starting with any
> > change allowed anywhere at anytime. If that is what people want, then
> > they are going to get to maintain that out of tree.
> > 
> 
> I am still not sold on this ‘dangerous’ idea. No-one is crazy enough to
> suggest overlays to be loadable by an unprivileged user. It’s exactly the
> same ‘danger’ as loading a kernel module, while is sure capable of much
> greater mischief.

Agreed.
> 
> >> In some cases there is a whole node hierarchy being inserted (like in
> >> a 

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Moritz Fischer]

On Thu, Oct 19, 2017 at 11:51:40AM +0300, Pantelis Antoniou wrote:
> Hi Rob,
> 
> > On Oct 18, 2017, at 21:30 , Rob Herring  wrote:
> > 
> > On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou
> >  wrote:
> >> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:
> >>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
>  On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  
>  wrote:
> > On 10/17/17 14:46, Rob Herring wrote:
> >> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
> >>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
> >>> 
> >>> Hi Rob,
> >>> 
>  With dependencies on a statically allocated full path name converted 
>  to
>  use %pOF format specifier, we can store just the basename of node, 
>  and
>  the unflattening of the FDT can be simplified.
>  
>  This commit will affect the remaining users of full_name. After
>  analyzing these users, the remaining cases should only change some 
>  print
>  messages. The main users of full_name are providing a name for struct
>  resource. The resource names shouldn't be important other than 
>  providing
>  /proc/iomem names.
>  
>  We no longer distinguish between pre and post 0x10 dtb formats as 
>  either
>  a full path or basename will work. However, less than 0x10 formats 
>  have
>  been broken since the conversion to use libfdt (and no one has 
>  cared).
>  The conversion of the unflattening code to be non-recursive also 
>  broke
>  pre 0x10 formats as the populate_node function would return 0 in that
>  case.
>  
>  Signed-off-by: Rob Herring 
>  ---
>  v2:
>  - rebase to linux-next
>  
>  drivers/of/fdt.c | 69 
>  +---
>  1 file changed, 11 insertions(+), 58 deletions(-)
> >>> 
> >>> I've just updated to the latest next branch and am finding problems
> >>> applying overlays.   Reverting this commit alleviates things.  The
> >>> errors I get are:
> >>> 
> >>> [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
> >>> [   88.513447] OF: overlay: apply failed '/__symbols__'
> >>> [   88.518423] create_overlay: Failed to create overlay (err=-12)
> >> 
> >> Frank's series with overlay updates should fix this.
> > 
> > Yes, it does:
> > 
> >  [PATCH v3 11/12] of: overlay: remove a dependency on device node 
> > full_name
>  
>  Thanks for the fast response.  I fetched the dt/next branch to test
>  this but there are sufficient changes that Pantelis' "OF: DT-Overlay
>  configfs interface (v7)" is broken now.  I've been adding that
>  downstream since 4.4.  We're using it as an interface for applying
>  overlays to program FPGAs.  If we fix it again, is there any chance
>  that can go upstream now?
> >>> 
> >>> With a drive-by posting once every few years, no.
> >>> 
> >> 
> >> I take offense to that. There's nothing changed in the patch for years.
> >> Reposting the same patch without changes would achieve nothing.
> > 
> > Are you still expecting review comments on it or something?
> > Furthermore, If something is posted infrequently, then I'm not
> > inclined to comment or care if the next posting is going to be after I
> > forget what I previously said (which is not very long).
> > 
> > I'm just saying, don't expect to forward port, post and it will be
> > accepted. Below is minimally one of the issues that needs to be
> > addressed.
> > 
> >>> The issue remains that the kernel is not really setup to deal with any
> >>> random property or node to be changed at any point in run-time. I
> >>> think there needs to be some restrictions around what the overlays can
> >>> touch. We can't have it be wide open and then lock things down later
> >>> and break users. One example of what you could do is you can only add
> >>> sub-trees to whitelisted nodes. That's probably acceptable for your
> >>> usecase.
> >>> 
> >> 
> >> Defining what can and what cannot be changed is not as trivial as a
> >> list of white-listed nodes.
> > 
> > No, but we have to start somewhere and we are not starting with any
> > change allowed anywhere at anytime. If that is what people want, then
> > they are going to get to maintain that out of tree.
> > 
> 
> I am still not sold on this ‘dangerous’ idea. No-one is crazy enough to
> suggest overlays to be loadable by an unprivileged user. It’s exactly the
> same ‘danger’ as loading a kernel module, while is sure capable of much
> greater mischief.

Agreed.
> 
> >> In some cases there is a whole node hierarchy being inserted (like in
> >> a FPGA).
> > 
> > Yes, so you'd have a target fpga region. That sounds fine to me. Maybe
> > its not a static whitelist, but drivers have to 

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Pantelis Antoniou]

Hi Rob,

> On Oct 18, 2017, at 21:30 , Rob Herring  wrote:
> 
> On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou
>  wrote:
>> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:
>>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
 On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  
 wrote:
> On 10/17/17 14:46, Rob Herring wrote:
>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
>>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
>>> 
>>> Hi Rob,
>>> 
 With dependencies on a statically allocated full path name converted to
 use %pOF format specifier, we can store just the basename of node, and
 the unflattening of the FDT can be simplified.
 
 This commit will affect the remaining users of full_name. After
 analyzing these users, the remaining cases should only change some 
 print
 messages. The main users of full_name are providing a name for struct
 resource. The resource names shouldn't be important other than 
 providing
 /proc/iomem names.
 
 We no longer distinguish between pre and post 0x10 dtb formats as 
 either
 a full path or basename will work. However, less than 0x10 formats have
 been broken since the conversion to use libfdt (and no one has cared).
 The conversion of the unflattening code to be non-recursive also broke
 pre 0x10 formats as the populate_node function would return 0 in that
 case.
 
 Signed-off-by: Rob Herring 
 ---
 v2:
 - rebase to linux-next
 
 drivers/of/fdt.c | 69 
 +---
 1 file changed, 11 insertions(+), 58 deletions(-)
>>> 
>>> I've just updated to the latest next branch and am finding problems
>>> applying overlays.   Reverting this commit alleviates things.  The
>>> errors I get are:
>>> 
>>> [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
>>> [   88.513447] OF: overlay: apply failed '/__symbols__'
>>> [   88.518423] create_overlay: Failed to create overlay (err=-12)
>> 
>> Frank's series with overlay updates should fix this.
> 
> Yes, it does:
> 
>  [PATCH v3 11/12] of: overlay: remove a dependency on device node 
> full_name
 
 Thanks for the fast response.  I fetched the dt/next branch to test
 this but there are sufficient changes that Pantelis' "OF: DT-Overlay
 configfs interface (v7)" is broken now.  I've been adding that
 downstream since 4.4.  We're using it as an interface for applying
 overlays to program FPGAs.  If we fix it again, is there any chance
 that can go upstream now?
>>> 
>>> With a drive-by posting once every few years, no.
>>> 
>> 
>> I take offense to that. There's nothing changed in the patch for years.
>> Reposting the same patch without changes would achieve nothing.
> 
> Are you still expecting review comments on it or something?
> Furthermore, If something is posted infrequently, then I'm not
> inclined to comment or care if the next posting is going to be after I
> forget what I previously said (which is not very long).
> 
> I'm just saying, don't expect to forward port, post and it will be
> accepted. Below is minimally one of the issues that needs to be
> addressed.
> 
>>> The issue remains that the kernel is not really setup to deal with any
>>> random property or node to be changed at any point in run-time. I
>>> think there needs to be some restrictions around what the overlays can
>>> touch. We can't have it be wide open and then lock things down later
>>> and break users. One example of what you could do is you can only add
>>> sub-trees to whitelisted nodes. That's probably acceptable for your
>>> usecase.
>>> 
>> 
>> Defining what can and what cannot be changed is not as trivial as a
>> list of white-listed nodes.
> 
> No, but we have to start somewhere and we are not starting with any
> change allowed anywhere at anytime. If that is what people want, then
> they are going to get to maintain that out of tree.
> 

I am still not sold on this ‘dangerous’ idea. No-one is crazy enough to
suggest overlays to be loadable by an unprivileged user. It’s exactly the
same ‘danger’ as loading a kernel module, while is sure capable of much
greater mischief.

>> In some cases there is a whole node hierarchy being inserted (like in
>> a FPGA).
> 
> Yes, so you'd have a target fpga region. That sounds fine to me. Maybe
> its not a static whitelist, but drivers have to register target
> nodes/paths.
> 
>> In others, it's merely changing a status property to "okay" and
>> a few device parameters.
> 
> That seems fine too. Disabled nodes could be allowed. But what if you
> add/change 

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Pantelis Antoniou]

Hi Rob,

> On Oct 18, 2017, at 21:30 , Rob Herring  wrote:
> 
> On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou
>  wrote:
>> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:
>>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
 On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  
 wrote:
> On 10/17/17 14:46, Rob Herring wrote:
>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
>>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
>>> 
>>> Hi Rob,
>>> 
 With dependencies on a statically allocated full path name converted to
 use %pOF format specifier, we can store just the basename of node, and
 the unflattening of the FDT can be simplified.
 
 This commit will affect the remaining users of full_name. After
 analyzing these users, the remaining cases should only change some 
 print
 messages. The main users of full_name are providing a name for struct
 resource. The resource names shouldn't be important other than 
 providing
 /proc/iomem names.
 
 We no longer distinguish between pre and post 0x10 dtb formats as 
 either
 a full path or basename will work. However, less than 0x10 formats have
 been broken since the conversion to use libfdt (and no one has cared).
 The conversion of the unflattening code to be non-recursive also broke
 pre 0x10 formats as the populate_node function would return 0 in that
 case.
 
 Signed-off-by: Rob Herring 
 ---
 v2:
 - rebase to linux-next
 
 drivers/of/fdt.c | 69 
 +---
 1 file changed, 11 insertions(+), 58 deletions(-)
>>> 
>>> I've just updated to the latest next branch and am finding problems
>>> applying overlays.   Reverting this commit alleviates things.  The
>>> errors I get are:
>>> 
>>> [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
>>> [   88.513447] OF: overlay: apply failed '/__symbols__'
>>> [   88.518423] create_overlay: Failed to create overlay (err=-12)
>> 
>> Frank's series with overlay updates should fix this.
> 
> Yes, it does:
> 
>  [PATCH v3 11/12] of: overlay: remove a dependency on device node 
> full_name
 
 Thanks for the fast response.  I fetched the dt/next branch to test
 this but there are sufficient changes that Pantelis' "OF: DT-Overlay
 configfs interface (v7)" is broken now.  I've been adding that
 downstream since 4.4.  We're using it as an interface for applying
 overlays to program FPGAs.  If we fix it again, is there any chance
 that can go upstream now?
>>> 
>>> With a drive-by posting once every few years, no.
>>> 
>> 
>> I take offense to that. There's nothing changed in the patch for years.
>> Reposting the same patch without changes would achieve nothing.
> 
> Are you still expecting review comments on it or something?
> Furthermore, If something is posted infrequently, then I'm not
> inclined to comment or care if the next posting is going to be after I
> forget what I previously said (which is not very long).
> 
> I'm just saying, don't expect to forward port, post and it will be
> accepted. Below is minimally one of the issues that needs to be
> addressed.
> 
>>> The issue remains that the kernel is not really setup to deal with any
>>> random property or node to be changed at any point in run-time. I
>>> think there needs to be some restrictions around what the overlays can
>>> touch. We can't have it be wide open and then lock things down later
>>> and break users. One example of what you could do is you can only add
>>> sub-trees to whitelisted nodes. That's probably acceptable for your
>>> usecase.
>>> 
>> 
>> Defining what can and what cannot be changed is not as trivial as a
>> list of white-listed nodes.
> 
> No, but we have to start somewhere and we are not starting with any
> change allowed anywhere at anytime. If that is what people want, then
> they are going to get to maintain that out of tree.
> 

I am still not sold on this ‘dangerous’ idea. No-one is crazy enough to
suggest overlays to be loadable by an unprivileged user. It’s exactly the
same ‘danger’ as loading a kernel module, while is sure capable of much
greater mischief.

>> In some cases there is a whole node hierarchy being inserted (like in
>> a FPGA).
> 
> Yes, so you'd have a target fpga region. That sounds fine to me. Maybe
> its not a static whitelist, but drivers have to register target
> nodes/paths.
> 
>> In others, it's merely changing a status property to "okay" and
>> a few device parameters.
> 
> That seems fine too. Disabled nodes could be allowed. But what if you
> add/change properties on a node that is not disabled? Once a node is
> enabled, who is responsible for registering the device?
> 
> What about changing a 

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Pantelis Antoniou]

Hi Frank,

> On Oct 19, 2017, at 00:46 , Frank Rowand  wrote:
> 
> On 10/18/17 11:30, Rob Herring wrote:
>> On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou
>>  wrote:
>>> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:
 On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
> On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  
> wrote:
>> On 10/17/17 14:46, Rob Herring wrote:
>>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
 On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
 
 Hi Rob,
 
> With dependencies on a statically allocated full path name converted 
> to
> use %pOF format specifier, we can store just the basename of node, and
> the unflattening of the FDT can be simplified.
> 
> This commit will affect the remaining users of full_name. After
> analyzing these users, the remaining cases should only change some 
> print
> messages. The main users of full_name are providing a name for struct
> resource. The resource names shouldn't be important other than 
> providing
> /proc/iomem names.
> 
> We no longer distinguish between pre and post 0x10 dtb formats as 
> either
> a full path or basename will work. However, less than 0x10 formats 
> have
> been broken since the conversion to use libfdt (and no one has cared).
> The conversion of the unflattening code to be non-recursive also broke
> pre 0x10 formats as the populate_node function would return 0 in that
> case.
> 
> Signed-off-by: Rob Herring 
> ---
> v2:
> - rebase to linux-next
> 
> drivers/of/fdt.c | 69 
> +---
> 1 file changed, 11 insertions(+), 58 deletions(-)
 
 I've just updated to the latest next branch and am finding problems
 applying overlays.   Reverting this commit alleviates things.  The
 errors I get are:
 
 [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
 [   88.513447] OF: overlay: apply failed '/__symbols__'
 [   88.518423] create_overlay: Failed to create overlay (err=-12)
>>> 
>>> Frank's series with overlay updates should fix this.
>> 
>> Yes, it does:
>> 
>>  [PATCH v3 11/12] of: overlay: remove a dependency on device node 
>> full_name
> 
> Thanks for the fast response.  I fetched the dt/next branch to test
> this but there are sufficient changes that Pantelis' "OF: DT-Overlay
> configfs interface (v7)" is broken now.  I've been adding that
> downstream since 4.4.  We're using it as an interface for applying
> overlays to program FPGAs.  If we fix it again, is there any chance
> that can go upstream now?
 
 With a drive-by posting once every few years, no.
 
>>> 
>>> I take offense to that. There's nothing changed in the patch for years.
>>> Reposting the same patch without changes would achieve nothing.
>> 
>> Are you still expecting review comments on it or something?
>> Furthermore, If something is posted infrequently, then I'm not
>> inclined to comment or care if the next posting is going to be after I
>> forget what I previously said (which is not very long).
>> 
>> I'm just saying, don't expect to forward port, post and it will be
>> accepted. Below is minimally one of the issues that needs to be
>> addressed.
>> 
> 
> 
 The issue remains that the kernel is not really setup to deal with any
 random property or node to be changed at any point in run-time. I
 think there needs to be some restrictions around what the overlays can
 touch. We can't have it be wide open and then lock things down later
 and break users.
> 
> That paragraph is key to any discussion of accepting code to apply overlays.
> Solving that issue has been stated to be a gating factor for such code from
> the beginning of overlay development.
> 

Overlays are not only used only for cases where you have external expansion 
boards, or
FPGAs where every change is contained under a few designated nodes, so that’s 
why I’m
pushing for a in-kernel validator that’s more flexible than a single whitelist.

An eBPF validator would handle a whitelist trivially easy, and would be 
flexible enough
for any other more complicated use case.

> 
 One example of what you could do is you can only add
 sub-trees to whitelisted nodes. That's probably acceptable for your
 usecase.
 
>>> 
>>> Defining what can and what cannot be changed is not as trivial as a
>>> list of white-listed nodes.
>> 
>> No, but we have to start somewhere and we are not starting with any
>> change allowed anywhere 

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Pantelis Antoniou]

Hi Frank,

> On Oct 19, 2017, at 00:46 , Frank Rowand  wrote:
> 
> On 10/18/17 11:30, Rob Herring wrote:
>> On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou
>>  wrote:
>>> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:
 On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
> On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  
> wrote:
>> On 10/17/17 14:46, Rob Herring wrote:
>>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
 On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
 
 Hi Rob,
 
> With dependencies on a statically allocated full path name converted 
> to
> use %pOF format specifier, we can store just the basename of node, and
> the unflattening of the FDT can be simplified.
> 
> This commit will affect the remaining users of full_name. After
> analyzing these users, the remaining cases should only change some 
> print
> messages. The main users of full_name are providing a name for struct
> resource. The resource names shouldn't be important other than 
> providing
> /proc/iomem names.
> 
> We no longer distinguish between pre and post 0x10 dtb formats as 
> either
> a full path or basename will work. However, less than 0x10 formats 
> have
> been broken since the conversion to use libfdt (and no one has cared).
> The conversion of the unflattening code to be non-recursive also broke
> pre 0x10 formats as the populate_node function would return 0 in that
> case.
> 
> Signed-off-by: Rob Herring 
> ---
> v2:
> - rebase to linux-next
> 
> drivers/of/fdt.c | 69 
> +---
> 1 file changed, 11 insertions(+), 58 deletions(-)
 
 I've just updated to the latest next branch and am finding problems
 applying overlays.   Reverting this commit alleviates things.  The
 errors I get are:
 
 [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
 [   88.513447] OF: overlay: apply failed '/__symbols__'
 [   88.518423] create_overlay: Failed to create overlay (err=-12)
>>> 
>>> Frank's series with overlay updates should fix this.
>> 
>> Yes, it does:
>> 
>>  [PATCH v3 11/12] of: overlay: remove a dependency on device node 
>> full_name
> 
> Thanks for the fast response.  I fetched the dt/next branch to test
> this but there are sufficient changes that Pantelis' "OF: DT-Overlay
> configfs interface (v7)" is broken now.  I've been adding that
> downstream since 4.4.  We're using it as an interface for applying
> overlays to program FPGAs.  If we fix it again, is there any chance
> that can go upstream now?
 
 With a drive-by posting once every few years, no.
 
>>> 
>>> I take offense to that. There's nothing changed in the patch for years.
>>> Reposting the same patch without changes would achieve nothing.
>> 
>> Are you still expecting review comments on it or something?
>> Furthermore, If something is posted infrequently, then I'm not
>> inclined to comment or care if the next posting is going to be after I
>> forget what I previously said (which is not very long).
>> 
>> I'm just saying, don't expect to forward port, post and it will be
>> accepted. Below is minimally one of the issues that needs to be
>> addressed.
>> 
> 
> 
 The issue remains that the kernel is not really setup to deal with any
 random property or node to be changed at any point in run-time. I
 think there needs to be some restrictions around what the overlays can
 touch. We can't have it be wide open and then lock things down later
 and break users.
> 
> That paragraph is key to any discussion of accepting code to apply overlays.
> Solving that issue has been stated to be a gating factor for such code from
> the beginning of overlay development.
> 

Overlays are not only used only for cases where you have external expansion 
boards, or
FPGAs where every change is contained under a few designated nodes, so that’s 
why I’m
pushing for a in-kernel validator that’s more flexible than a single whitelist.

An eBPF validator would handle a whitelist trivially easy, and would be 
flexible enough
for any other more complicated use case.

> 
 One example of what you could do is you can only add
 sub-trees to whitelisted nodes. That's probably acceptable for your
 usecase.
 
>>> 
>>> Defining what can and what cannot be changed is not as trivial as a
>>> list of white-listed nodes.
>> 
>> No, but we have to start somewhere and we are not starting with any
>> change allowed anywhere at anytime. If that is what people want, then
>> they are going to get to maintain that out of tree.
>> 
>>> In some cases there is a whole node 

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Frank Rowand]

On 10/18/17 11:30, Rob Herring wrote:
> On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou
>  wrote:
>> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:
>>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
 On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  
 wrote:
> On 10/17/17 14:46, Rob Herring wrote:
>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
>>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
>>>
>>> Hi Rob,
>>>
 With dependencies on a statically allocated full path name converted to
 use %pOF format specifier, we can store just the basename of node, and
 the unflattening of the FDT can be simplified.

 This commit will affect the remaining users of full_name. After
 analyzing these users, the remaining cases should only change some 
 print
 messages. The main users of full_name are providing a name for struct
 resource. The resource names shouldn't be important other than 
 providing
 /proc/iomem names.

 We no longer distinguish between pre and post 0x10 dtb formats as 
 either
 a full path or basename will work. However, less than 0x10 formats have
 been broken since the conversion to use libfdt (and no one has cared).
 The conversion of the unflattening code to be non-recursive also broke
 pre 0x10 formats as the populate_node function would return 0 in that
 case.

 Signed-off-by: Rob Herring 
 ---
 v2:
 - rebase to linux-next

  drivers/of/fdt.c | 69 
 +---
  1 file changed, 11 insertions(+), 58 deletions(-)
>>>
>>> I've just updated to the latest next branch and am finding problems
>>> applying overlays.   Reverting this commit alleviates things.  The
>>> errors I get are:
>>>
>>> [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
>>> [   88.513447] OF: overlay: apply failed '/__symbols__'
>>> [   88.518423] create_overlay: Failed to create overlay (err=-12)
>>
>> Frank's series with overlay updates should fix this.
>
> Yes, it does:
>
>   [PATCH v3 11/12] of: overlay: remove a dependency on device node 
> full_name

 Thanks for the fast response.  I fetched the dt/next branch to test
 this but there are sufficient changes that Pantelis' "OF: DT-Overlay
 configfs interface (v7)" is broken now.  I've been adding that
 downstream since 4.4.  We're using it as an interface for applying
 overlays to program FPGAs.  If we fix it again, is there any chance
 that can go upstream now?
>>>
>>> With a drive-by posting once every few years, no.
>>>
>>
>> I take offense to that. There's nothing changed in the patch for years.
>> Reposting the same patch without changes would achieve nothing.
> 
> Are you still expecting review comments on it or something?
> Furthermore, If something is posted infrequently, then I'm not
> inclined to comment or care if the next posting is going to be after I
> forget what I previously said (which is not very long).
> 
> I'm just saying, don't expect to forward port, post and it will be
> accepted. Below is minimally one of the issues that needs to be
> addressed.
> 


>>> The issue remains that the kernel is not really setup to deal with any
>>> random property or node to be changed at any point in run-time. I
>>> think there needs to be some restrictions around what the overlays can
>>> touch. We can't have it be wide open and then lock things down later
>>> and break users.

That paragraph is key to any discussion of accepting code to apply overlays.
Solving that issue has been stated to be a gating factor for such code from
the beginning of overlay development.


>>> One example of what you could do is you can only add
>>> sub-trees to whitelisted nodes. That's probably acceptable for your
>>> usecase.
>>>
>>
>> Defining what can and what cannot be changed is not as trivial as a
>> list of white-listed nodes.
> 
> No, but we have to start somewhere and we are not starting with any
> change allowed anywhere at anytime. If that is what people want, then
> they are going to get to maintain that out of tree.
> 
>> In some cases there is a whole node hierarchy being inserted (like in
>> a FPGA).
> 
> Yes, so you'd have a target fpga region. That sounds fine to me. Maybe
> its not a static whitelist, but drivers have to register target
> nodes/paths.
> 
>> In others, it's merely changing a status property to "okay" and
>> a few device parameters.
> 
> That seems fine too. Disabled nodes could be allowed. But what if you
> add/change properties on a node that is not disabled? Once a node is
> enabled, who is responsible for 

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Frank Rowand]

On 10/18/17 11:30, Rob Herring wrote:
> On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou
>  wrote:
>> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:
>>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
 On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  
 wrote:
> On 10/17/17 14:46, Rob Herring wrote:
>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
>>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
>>>
>>> Hi Rob,
>>>
 With dependencies on a statically allocated full path name converted to
 use %pOF format specifier, we can store just the basename of node, and
 the unflattening of the FDT can be simplified.

 This commit will affect the remaining users of full_name. After
 analyzing these users, the remaining cases should only change some 
 print
 messages. The main users of full_name are providing a name for struct
 resource. The resource names shouldn't be important other than 
 providing
 /proc/iomem names.

 We no longer distinguish between pre and post 0x10 dtb formats as 
 either
 a full path or basename will work. However, less than 0x10 formats have
 been broken since the conversion to use libfdt (and no one has cared).
 The conversion of the unflattening code to be non-recursive also broke
 pre 0x10 formats as the populate_node function would return 0 in that
 case.

 Signed-off-by: Rob Herring 
 ---
 v2:
 - rebase to linux-next

  drivers/of/fdt.c | 69 
 +---
  1 file changed, 11 insertions(+), 58 deletions(-)
>>>
>>> I've just updated to the latest next branch and am finding problems
>>> applying overlays.   Reverting this commit alleviates things.  The
>>> errors I get are:
>>>
>>> [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
>>> [   88.513447] OF: overlay: apply failed '/__symbols__'
>>> [   88.518423] create_overlay: Failed to create overlay (err=-12)
>>
>> Frank's series with overlay updates should fix this.
>
> Yes, it does:
>
>   [PATCH v3 11/12] of: overlay: remove a dependency on device node 
> full_name

 Thanks for the fast response.  I fetched the dt/next branch to test
 this but there are sufficient changes that Pantelis' "OF: DT-Overlay
 configfs interface (v7)" is broken now.  I've been adding that
 downstream since 4.4.  We're using it as an interface for applying
 overlays to program FPGAs.  If we fix it again, is there any chance
 that can go upstream now?
>>>
>>> With a drive-by posting once every few years, no.
>>>
>>
>> I take offense to that. There's nothing changed in the patch for years.
>> Reposting the same patch without changes would achieve nothing.
> 
> Are you still expecting review comments on it or something?
> Furthermore, If something is posted infrequently, then I'm not
> inclined to comment or care if the next posting is going to be after I
> forget what I previously said (which is not very long).
> 
> I'm just saying, don't expect to forward port, post and it will be
> accepted. Below is minimally one of the issues that needs to be
> addressed.
> 


>>> The issue remains that the kernel is not really setup to deal with any
>>> random property or node to be changed at any point in run-time. I
>>> think there needs to be some restrictions around what the overlays can
>>> touch. We can't have it be wide open and then lock things down later
>>> and break users.

That paragraph is key to any discussion of accepting code to apply overlays.
Solving that issue has been stated to be a gating factor for such code from
the beginning of overlay development.


>>> One example of what you could do is you can only add
>>> sub-trees to whitelisted nodes. That's probably acceptable for your
>>> usecase.
>>>
>>
>> Defining what can and what cannot be changed is not as trivial as a
>> list of white-listed nodes.
> 
> No, but we have to start somewhere and we are not starting with any
> change allowed anywhere at anytime. If that is what people want, then
> they are going to get to maintain that out of tree.
> 
>> In some cases there is a whole node hierarchy being inserted (like in
>> a FPGA).
> 
> Yes, so you'd have a target fpga region. That sounds fine to me. Maybe
> its not a static whitelist, but drivers have to register target
> nodes/paths.
> 
>> In others, it's merely changing a status property to "okay" and
>> a few device parameters.
> 
> That seems fine too. Disabled nodes could be allowed. But what if you
> add/change properties on a node that is not disabled? Once a node is
> enabled, who is responsible for registering the device?
> 
> What about changing a node from enabled to disabled? The kernel would
> need to handle that or 

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Frank Rowand]

On 10/18/17 11:39, Alan Tull wrote:
> On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou
>  wrote:
>> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:
>>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
 On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  
 wrote:
> On 10/17/17 14:46, Rob Herring wrote:
>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
>>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
>>>
>>> Hi Rob,
>>>
 With dependencies on a statically allocated full path name converted to
 use %pOF format specifier, we can store just the basename of node, and
 the unflattening of the FDT can be simplified.

 This commit will affect the remaining users of full_name. After
 analyzing these users, the remaining cases should only change some 
 print
 messages. The main users of full_name are providing a name for struct
 resource. The resource names shouldn't be important other than 
 providing
 /proc/iomem names.

 We no longer distinguish between pre and post 0x10 dtb formats as 
 either
 a full path or basename will work. However, less than 0x10 formats have
 been broken since the conversion to use libfdt (and no one has cared).
 The conversion of the unflattening code to be non-recursive also broke
 pre 0x10 formats as the populate_node function would return 0 in that
 case.

 Signed-off-by: Rob Herring 
 ---
 v2:
 - rebase to linux-next

  drivers/of/fdt.c | 69 
 +---
  1 file changed, 11 insertions(+), 58 deletions(-)
>>>
>>> I've just updated to the latest next branch and am finding problems
>>> applying overlays.   Reverting this commit alleviates things.  The
>>> errors I get are:
>>>
>>> [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
>>> [   88.513447] OF: overlay: apply failed '/__symbols__'
>>> [   88.518423] create_overlay: Failed to create overlay (err=-12)
>>
>> Frank's series with overlay updates should fix this.
>
> Yes, it does:
>
>   [PATCH v3 11/12] of: overlay: remove a dependency on device node 
> full_name

 Thanks for the fast response.  I fetched the dt/next branch to test
 this but there are sufficient changes that Pantelis' "OF: DT-Overlay
 configfs interface (v7)" is broken now.  I've been adding that
 downstream since 4.4.  We're using it as an interface for applying
 overlays to program FPGAs.  If we fix it again, is there any chance
 that can go upstream now?
>>>
>>> With a drive-by posting once every few years, no.
>>>
>> I take offense to that. There's nothing changed in the patch for years.
>> Reposting the same patch without changes would achieve nothing.
>>
>>> The issue remains that the kernel is not really setup to deal with any
>>> random property or node to be changed at any point in run-time.
> 
> Yeah, I'm not super surprised :)  I have some whitelist ideas below.
> 
>>> I
>>> think there needs to be some restrictions around what the overlays can
>>> touch. We can't have it be wide open and then lock things down later
>>> and break users. One example of what you could do is you can only add
>>> sub-trees to whitelisted nodes. That's probably acceptable for your
>>> usecase.
> 
> I can take a look at making OF_OVERLAY_PRE_APPLY and
> OF_OVERLAY_PRE_REMOVE notifiers mandatory if that's interesting.  The
> behavior would be: If an overlay is applied, there's got to be some
> handler in the kernel that verifies that it is acceptable.   In my
> case, the handler for FPGA regions would look at the overlay and see
> it only added stuff under a FPGA region.
> 
> And we would change the default to be: if there is no handler, reject
> the overlay.

I think the responsibility belongs to the device tree core code.  It
should be centralized and consistent.

In the fpga case, I think the connector paradigm should be adequate.
The connector describes what is available to the fpga and provides
access to those things.  The fpga overlay does not reach outside
the connector to touch anything else in the device tree.


>> Defining what can and what cannot be changed is not as trivial as a
>> list of white-listed nodes.
>>
>> In some cases there is a whole node hierarchy being inserted (like in
>> a FPGA).
> 
> For FPGA, the insertion points are always FPGA regions.
> 
>> In others, it's merely changing a status property to "okay" and
>> a few device parameters.
>>
>> The real issue is that the kernel has no way to verify that a given
>> device tree, either at boot time or at overlay application time, is
>> correct.
>>
>> When the tree 

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Frank Rowand]

On 10/18/17 11:39, Alan Tull wrote:
> On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou
>  wrote:
>> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:
>>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
 On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  
 wrote:
> On 10/17/17 14:46, Rob Herring wrote:
>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
>>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
>>>
>>> Hi Rob,
>>>
 With dependencies on a statically allocated full path name converted to
 use %pOF format specifier, we can store just the basename of node, and
 the unflattening of the FDT can be simplified.

 This commit will affect the remaining users of full_name. After
 analyzing these users, the remaining cases should only change some 
 print
 messages. The main users of full_name are providing a name for struct
 resource. The resource names shouldn't be important other than 
 providing
 /proc/iomem names.

 We no longer distinguish between pre and post 0x10 dtb formats as 
 either
 a full path or basename will work. However, less than 0x10 formats have
 been broken since the conversion to use libfdt (and no one has cared).
 The conversion of the unflattening code to be non-recursive also broke
 pre 0x10 formats as the populate_node function would return 0 in that
 case.

 Signed-off-by: Rob Herring 
 ---
 v2:
 - rebase to linux-next

  drivers/of/fdt.c | 69 
 +---
  1 file changed, 11 insertions(+), 58 deletions(-)
>>>
>>> I've just updated to the latest next branch and am finding problems
>>> applying overlays.   Reverting this commit alleviates things.  The
>>> errors I get are:
>>>
>>> [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
>>> [   88.513447] OF: overlay: apply failed '/__symbols__'
>>> [   88.518423] create_overlay: Failed to create overlay (err=-12)
>>
>> Frank's series with overlay updates should fix this.
>
> Yes, it does:
>
>   [PATCH v3 11/12] of: overlay: remove a dependency on device node 
> full_name

 Thanks for the fast response.  I fetched the dt/next branch to test
 this but there are sufficient changes that Pantelis' "OF: DT-Overlay
 configfs interface (v7)" is broken now.  I've been adding that
 downstream since 4.4.  We're using it as an interface for applying
 overlays to program FPGAs.  If we fix it again, is there any chance
 that can go upstream now?
>>>
>>> With a drive-by posting once every few years, no.
>>>
>> I take offense to that. There's nothing changed in the patch for years.
>> Reposting the same patch without changes would achieve nothing.
>>
>>> The issue remains that the kernel is not really setup to deal with any
>>> random property or node to be changed at any point in run-time.
> 
> Yeah, I'm not super surprised :)  I have some whitelist ideas below.
> 
>>> I
>>> think there needs to be some restrictions around what the overlays can
>>> touch. We can't have it be wide open and then lock things down later
>>> and break users. One example of what you could do is you can only add
>>> sub-trees to whitelisted nodes. That's probably acceptable for your
>>> usecase.
> 
> I can take a look at making OF_OVERLAY_PRE_APPLY and
> OF_OVERLAY_PRE_REMOVE notifiers mandatory if that's interesting.  The
> behavior would be: If an overlay is applied, there's got to be some
> handler in the kernel that verifies that it is acceptable.   In my
> case, the handler for FPGA regions would look at the overlay and see
> it only added stuff under a FPGA region.
> 
> And we would change the default to be: if there is no handler, reject
> the overlay.

I think the responsibility belongs to the device tree core code.  It
should be centralized and consistent.

In the fpga case, I think the connector paradigm should be adequate.
The connector describes what is available to the fpga and provides
access to those things.  The fpga overlay does not reach outside
the connector to touch anything else in the device tree.


>> Defining what can and what cannot be changed is not as trivial as a
>> list of white-listed nodes.
>>
>> In some cases there is a whole node hierarchy being inserted (like in
>> a FPGA).
> 
> For FPGA, the insertion points are always FPGA regions.
> 
>> In others, it's merely changing a status property to "okay" and
>> a few device parameters.
>>
>> The real issue is that the kernel has no way to verify that a given
>> device tree, either at boot time or at overlay application time, is
>> correct.
>>
>> When the tree is wrong at boot-time you'll hang (if you're lucky).
>> If the tree is wrong at run-time you'll get some into some 

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Alan Tull]

On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou
 wrote:
> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:
>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
>> > On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  
>> > wrote:
>> >> On 10/17/17 14:46, Rob Herring wrote:
>> >>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
>>  On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
>> 
>>  Hi Rob,
>> 
>> > With dependencies on a statically allocated full path name converted to
>> > use %pOF format specifier, we can store just the basename of node, and
>> > the unflattening of the FDT can be simplified.
>> >
>> > This commit will affect the remaining users of full_name. After
>> > analyzing these users, the remaining cases should only change some 
>> > print
>> > messages. The main users of full_name are providing a name for struct
>> > resource. The resource names shouldn't be important other than 
>> > providing
>> > /proc/iomem names.
>> >
>> > We no longer distinguish between pre and post 0x10 dtb formats as 
>> > either
>> > a full path or basename will work. However, less than 0x10 formats have
>> > been broken since the conversion to use libfdt (and no one has cared).
>> > The conversion of the unflattening code to be non-recursive also broke
>> > pre 0x10 formats as the populate_node function would return 0 in that
>> > case.
>> >
>> > Signed-off-by: Rob Herring 
>> > ---
>> > v2:
>> > - rebase to linux-next
>> >
>> >  drivers/of/fdt.c | 69 
>> > +---
>> >  1 file changed, 11 insertions(+), 58 deletions(-)
>> 
>>  I've just updated to the latest next branch and am finding problems
>>  applying overlays.   Reverting this commit alleviates things.  The
>>  errors I get are:
>> 
>>  [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
>>  [   88.513447] OF: overlay: apply failed '/__symbols__'
>>  [   88.518423] create_overlay: Failed to create overlay (err=-12)
>> >>>
>> >>> Frank's series with overlay updates should fix this.
>> >>
>> >> Yes, it does:
>> >>
>> >>   [PATCH v3 11/12] of: overlay: remove a dependency on device node 
>> >> full_name
>> >
>> > Thanks for the fast response.  I fetched the dt/next branch to test
>> > this but there are sufficient changes that Pantelis' "OF: DT-Overlay
>> > configfs interface (v7)" is broken now.  I've been adding that
>> > downstream since 4.4.  We're using it as an interface for applying
>> > overlays to program FPGAs.  If we fix it again, is there any chance
>> > that can go upstream now?
>>
>> With a drive-by posting once every few years, no.
>>
> I take offense to that. There's nothing changed in the patch for years.
> Reposting the same patch without changes would achieve nothing.
>
>> The issue remains that the kernel is not really setup to deal with any
>> random property or node to be changed at any point in run-time.

Yeah, I'm not super surprised :)  I have some whitelist ideas below.

>> I
>> think there needs to be some restrictions around what the overlays can
>> touch. We can't have it be wide open and then lock things down later
>> and break users. One example of what you could do is you can only add
>> sub-trees to whitelisted nodes. That's probably acceptable for your
>> usecase.

I can take a look at making OF_OVERLAY_PRE_APPLY and
OF_OVERLAY_PRE_REMOVE notifiers mandatory if that's interesting.  The
behavior would be: If an overlay is applied, there's got to be some
handler in the kernel that verifies that it is acceptable.   In my
case, the handler for FPGA regions would look at the overlay and see
it only added stuff under a FPGA region.

And we would change the default to be: if there is no handler, reject
the overlay.

>>
>
> Defining what can and what cannot be changed is not as trivial as a
> list of white-listed nodes.
>
> In some cases there is a whole node hierarchy being inserted (like in
> a FPGA).

For FPGA, the insertion points are always FPGA regions.

> In others, it's merely changing a status property to "okay" and
> a few device parameters.
>
> The real issue is that the kernel has no way to verify that a given
> device tree, either at boot time or at overlay application time, is
> correct.
>
> When the tree is wrong at boot-time you'll hang (if you're lucky).
> If the tree is wrong at run-time you'll get some into some unidentified
> funky state.
>
> Finally what is, and what is not 'correct' is not for the kernel to
> decide arbitrarily, it's a matter of policy, different for each
> use-case.
>
>> Rob
>
> Regards
>
> -- Pantelis

Alan Tull

>
>

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Alan Tull]

On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou
 wrote:
> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:
>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
>> > On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  
>> > wrote:
>> >> On 10/17/17 14:46, Rob Herring wrote:
>> >>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
>>  On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
>> 
>>  Hi Rob,
>> 
>> > With dependencies on a statically allocated full path name converted to
>> > use %pOF format specifier, we can store just the basename of node, and
>> > the unflattening of the FDT can be simplified.
>> >
>> > This commit will affect the remaining users of full_name. After
>> > analyzing these users, the remaining cases should only change some 
>> > print
>> > messages. The main users of full_name are providing a name for struct
>> > resource. The resource names shouldn't be important other than 
>> > providing
>> > /proc/iomem names.
>> >
>> > We no longer distinguish between pre and post 0x10 dtb formats as 
>> > either
>> > a full path or basename will work. However, less than 0x10 formats have
>> > been broken since the conversion to use libfdt (and no one has cared).
>> > The conversion of the unflattening code to be non-recursive also broke
>> > pre 0x10 formats as the populate_node function would return 0 in that
>> > case.
>> >
>> > Signed-off-by: Rob Herring 
>> > ---
>> > v2:
>> > - rebase to linux-next
>> >
>> >  drivers/of/fdt.c | 69 
>> > +---
>> >  1 file changed, 11 insertions(+), 58 deletions(-)
>> 
>>  I've just updated to the latest next branch and am finding problems
>>  applying overlays.   Reverting this commit alleviates things.  The
>>  errors I get are:
>> 
>>  [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
>>  [   88.513447] OF: overlay: apply failed '/__symbols__'
>>  [   88.518423] create_overlay: Failed to create overlay (err=-12)
>> >>>
>> >>> Frank's series with overlay updates should fix this.
>> >>
>> >> Yes, it does:
>> >>
>> >>   [PATCH v3 11/12] of: overlay: remove a dependency on device node 
>> >> full_name
>> >
>> > Thanks for the fast response.  I fetched the dt/next branch to test
>> > this but there are sufficient changes that Pantelis' "OF: DT-Overlay
>> > configfs interface (v7)" is broken now.  I've been adding that
>> > downstream since 4.4.  We're using it as an interface for applying
>> > overlays to program FPGAs.  If we fix it again, is there any chance
>> > that can go upstream now?
>>
>> With a drive-by posting once every few years, no.
>>
> I take offense to that. There's nothing changed in the patch for years.
> Reposting the same patch without changes would achieve nothing.
>
>> The issue remains that the kernel is not really setup to deal with any
>> random property or node to be changed at any point in run-time.

Yeah, I'm not super surprised :)  I have some whitelist ideas below.

>> I
>> think there needs to be some restrictions around what the overlays can
>> touch. We can't have it be wide open and then lock things down later
>> and break users. One example of what you could do is you can only add
>> sub-trees to whitelisted nodes. That's probably acceptable for your
>> usecase.

I can take a look at making OF_OVERLAY_PRE_APPLY and
OF_OVERLAY_PRE_REMOVE notifiers mandatory if that's interesting.  The
behavior would be: If an overlay is applied, there's got to be some
handler in the kernel that verifies that it is acceptable.   In my
case, the handler for FPGA regions would look at the overlay and see
it only added stuff under a FPGA region.

And we would change the default to be: if there is no handler, reject
the overlay.

>>
>
> Defining what can and what cannot be changed is not as trivial as a
> list of white-listed nodes.
>
> In some cases there is a whole node hierarchy being inserted (like in
> a FPGA).

For FPGA, the insertion points are always FPGA regions.

> In others, it's merely changing a status property to "okay" and
> a few device parameters.
>
> The real issue is that the kernel has no way to verify that a given
> device tree, either at boot time or at overlay application time, is
> correct.
>
> When the tree is wrong at boot-time you'll hang (if you're lucky).
> If the tree is wrong at run-time you'll get some into some unidentified
> funky state.
>
> Finally what is, and what is not 'correct' is not for the kernel to
> decide arbitrarily, it's a matter of policy, different for each
> use-case.
>
>> Rob
>
> Regards
>
> -- Pantelis

Alan Tull

>
>

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Rob Herring]

On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou
 wrote:
> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:
>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
>> > On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  
>> > wrote:
>> >> On 10/17/17 14:46, Rob Herring wrote:
>> >>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
>>  On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
>> 
>>  Hi Rob,
>> 
>> > With dependencies on a statically allocated full path name converted to
>> > use %pOF format specifier, we can store just the basename of node, and
>> > the unflattening of the FDT can be simplified.
>> >
>> > This commit will affect the remaining users of full_name. After
>> > analyzing these users, the remaining cases should only change some 
>> > print
>> > messages. The main users of full_name are providing a name for struct
>> > resource. The resource names shouldn't be important other than 
>> > providing
>> > /proc/iomem names.
>> >
>> > We no longer distinguish between pre and post 0x10 dtb formats as 
>> > either
>> > a full path or basename will work. However, less than 0x10 formats have
>> > been broken since the conversion to use libfdt (and no one has cared).
>> > The conversion of the unflattening code to be non-recursive also broke
>> > pre 0x10 formats as the populate_node function would return 0 in that
>> > case.
>> >
>> > Signed-off-by: Rob Herring 
>> > ---
>> > v2:
>> > - rebase to linux-next
>> >
>> >  drivers/of/fdt.c | 69 
>> > +---
>> >  1 file changed, 11 insertions(+), 58 deletions(-)
>> 
>>  I've just updated to the latest next branch and am finding problems
>>  applying overlays.   Reverting this commit alleviates things.  The
>>  errors I get are:
>> 
>>  [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
>>  [   88.513447] OF: overlay: apply failed '/__symbols__'
>>  [   88.518423] create_overlay: Failed to create overlay (err=-12)
>> >>>
>> >>> Frank's series with overlay updates should fix this.
>> >>
>> >> Yes, it does:
>> >>
>> >>   [PATCH v3 11/12] of: overlay: remove a dependency on device node 
>> >> full_name
>> >
>> > Thanks for the fast response.  I fetched the dt/next branch to test
>> > this but there are sufficient changes that Pantelis' "OF: DT-Overlay
>> > configfs interface (v7)" is broken now.  I've been adding that
>> > downstream since 4.4.  We're using it as an interface for applying
>> > overlays to program FPGAs.  If we fix it again, is there any chance
>> > that can go upstream now?
>>
>> With a drive-by posting once every few years, no.
>>
>
> I take offense to that. There's nothing changed in the patch for years.
> Reposting the same patch without changes would achieve nothing.

Are you still expecting review comments on it or something?
Furthermore, If something is posted infrequently, then I'm not
inclined to comment or care if the next posting is going to be after I
forget what I previously said (which is not very long).

I'm just saying, don't expect to forward port, post and it will be
accepted. Below is minimally one of the issues that needs to be
addressed.

>> The issue remains that the kernel is not really setup to deal with any
>> random property or node to be changed at any point in run-time. I
>> think there needs to be some restrictions around what the overlays can
>> touch. We can't have it be wide open and then lock things down later
>> and break users. One example of what you could do is you can only add
>> sub-trees to whitelisted nodes. That's probably acceptable for your
>> usecase.
>>
>
> Defining what can and what cannot be changed is not as trivial as a
> list of white-listed nodes.

No, but we have to start somewhere and we are not starting with any
change allowed anywhere at anytime. If that is what people want, then
they are going to get to maintain that out of tree.

> In some cases there is a whole node hierarchy being inserted (like in
> a FPGA).

Yes, so you'd have a target fpga region. That sounds fine to me. Maybe
its not a static whitelist, but drivers have to register target
nodes/paths.

> In others, it's merely changing a status property to "okay" and
> a few device parameters.

That seems fine too. Disabled nodes could be allowed. But what if you
add/change properties on a node that is not disabled? Once a node is
enabled, who is responsible for registering the device?

What about changing a node from enabled to disabled? The kernel would
need to handle that or not allow it.

> The real issue is that the kernel has no way to verify that a given
> device tree, either at boot time or at overlay application time, is
> correct.
>
> When the tree is wrong at 

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Rob Herring]

On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou
 wrote:
> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:
>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
>> > On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  
>> > wrote:
>> >> On 10/17/17 14:46, Rob Herring wrote:
>> >>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
>>  On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
>> 
>>  Hi Rob,
>> 
>> > With dependencies on a statically allocated full path name converted to
>> > use %pOF format specifier, we can store just the basename of node, and
>> > the unflattening of the FDT can be simplified.
>> >
>> > This commit will affect the remaining users of full_name. After
>> > analyzing these users, the remaining cases should only change some 
>> > print
>> > messages. The main users of full_name are providing a name for struct
>> > resource. The resource names shouldn't be important other than 
>> > providing
>> > /proc/iomem names.
>> >
>> > We no longer distinguish between pre and post 0x10 dtb formats as 
>> > either
>> > a full path or basename will work. However, less than 0x10 formats have
>> > been broken since the conversion to use libfdt (and no one has cared).
>> > The conversion of the unflattening code to be non-recursive also broke
>> > pre 0x10 formats as the populate_node function would return 0 in that
>> > case.
>> >
>> > Signed-off-by: Rob Herring 
>> > ---
>> > v2:
>> > - rebase to linux-next
>> >
>> >  drivers/of/fdt.c | 69 
>> > +---
>> >  1 file changed, 11 insertions(+), 58 deletions(-)
>> 
>>  I've just updated to the latest next branch and am finding problems
>>  applying overlays.   Reverting this commit alleviates things.  The
>>  errors I get are:
>> 
>>  [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
>>  [   88.513447] OF: overlay: apply failed '/__symbols__'
>>  [   88.518423] create_overlay: Failed to create overlay (err=-12)
>> >>>
>> >>> Frank's series with overlay updates should fix this.
>> >>
>> >> Yes, it does:
>> >>
>> >>   [PATCH v3 11/12] of: overlay: remove a dependency on device node 
>> >> full_name
>> >
>> > Thanks for the fast response.  I fetched the dt/next branch to test
>> > this but there are sufficient changes that Pantelis' "OF: DT-Overlay
>> > configfs interface (v7)" is broken now.  I've been adding that
>> > downstream since 4.4.  We're using it as an interface for applying
>> > overlays to program FPGAs.  If we fix it again, is there any chance
>> > that can go upstream now?
>>
>> With a drive-by posting once every few years, no.
>>
>
> I take offense to that. There's nothing changed in the patch for years.
> Reposting the same patch without changes would achieve nothing.

Are you still expecting review comments on it or something?
Furthermore, If something is posted infrequently, then I'm not
inclined to comment or care if the next posting is going to be after I
forget what I previously said (which is not very long).

I'm just saying, don't expect to forward port, post and it will be
accepted. Below is minimally one of the issues that needs to be
addressed.

>> The issue remains that the kernel is not really setup to deal with any
>> random property or node to be changed at any point in run-time. I
>> think there needs to be some restrictions around what the overlays can
>> touch. We can't have it be wide open and then lock things down later
>> and break users. One example of what you could do is you can only add
>> sub-trees to whitelisted nodes. That's probably acceptable for your
>> usecase.
>>
>
> Defining what can and what cannot be changed is not as trivial as a
> list of white-listed nodes.

No, but we have to start somewhere and we are not starting with any
change allowed anywhere at anytime. If that is what people want, then
they are going to get to maintain that out of tree.

> In some cases there is a whole node hierarchy being inserted (like in
> a FPGA).

Yes, so you'd have a target fpga region. That sounds fine to me. Maybe
its not a static whitelist, but drivers have to register target
nodes/paths.

> In others, it's merely changing a status property to "okay" and
> a few device parameters.

That seems fine too. Disabled nodes could be allowed. But what if you
add/change properties on a node that is not disabled? Once a node is
enabled, who is responsible for registering the device?

What about changing a node from enabled to disabled? The kernel would
need to handle that or not allow it.

> The real issue is that the kernel has no way to verify that a given
> device tree, either at boot time or at overlay application time, is
> correct.
>
> When the tree is wrong at boot-time you'll hang (if you're lucky).
> If the tree is wrong at run-time you'll get some into some unidentified
> funky 

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Pantelis Antoniou]

On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:
> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
> > On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  
> > wrote:
> >> On 10/17/17 14:46, Rob Herring wrote:
> >>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
>  On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
> 
>  Hi Rob,
> 
> > With dependencies on a statically allocated full path name converted to
> > use %pOF format specifier, we can store just the basename of node, and
> > the unflattening of the FDT can be simplified.
> >
> > This commit will affect the remaining users of full_name. After
> > analyzing these users, the remaining cases should only change some print
> > messages. The main users of full_name are providing a name for struct
> > resource. The resource names shouldn't be important other than providing
> > /proc/iomem names.
> >
> > We no longer distinguish between pre and post 0x10 dtb formats as either
> > a full path or basename will work. However, less than 0x10 formats have
> > been broken since the conversion to use libfdt (and no one has cared).
> > The conversion of the unflattening code to be non-recursive also broke
> > pre 0x10 formats as the populate_node function would return 0 in that
> > case.
> >
> > Signed-off-by: Rob Herring 
> > ---
> > v2:
> > - rebase to linux-next
> >
> >  drivers/of/fdt.c | 69 
> > +---
> >  1 file changed, 11 insertions(+), 58 deletions(-)
> 
>  I've just updated to the latest next branch and am finding problems
>  applying overlays.   Reverting this commit alleviates things.  The
>  errors I get are:
> 
>  [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
>  [   88.513447] OF: overlay: apply failed '/__symbols__'
>  [   88.518423] create_overlay: Failed to create overlay (err=-12)
> >>>
> >>> Frank's series with overlay updates should fix this.
> >>
> >> Yes, it does:
> >>
> >>   [PATCH v3 11/12] of: overlay: remove a dependency on device node 
> >> full_name
> >
> > Thanks for the fast response.  I fetched the dt/next branch to test
> > this but there are sufficient changes that Pantelis' "OF: DT-Overlay
> > configfs interface (v7)" is broken now.  I've been adding that
> > downstream since 4.4.  We're using it as an interface for applying
> > overlays to program FPGAs.  If we fix it again, is there any chance
> > that can go upstream now?
> 
> With a drive-by posting once every few years, no.
> 

I take offense to that. There's nothing changed in the patch for years.
Reposting the same patch without changes would achieve nothing.

> The issue remains that the kernel is not really setup to deal with any
> random property or node to be changed at any point in run-time. I
> think there needs to be some restrictions around what the overlays can
> touch. We can't have it be wide open and then lock things down later
> and break users. One example of what you could do is you can only add
> sub-trees to whitelisted nodes. That's probably acceptable for your
> usecase.
> 

Defining what can and what cannot be changed is not as trivial as a
list of white-listed nodes.

In some cases there is a whole node hierarchy being inserted (like in
a FPGA). In others, it's merely changing a status property to "okay" and
a few device parameters.

The real issue is that the kernel has no way to verify that a given
device tree, either at boot time or at overlay application time, is
correct.

When the tree is wrong at boot-time you'll hang (if you're lucky).
If the tree is wrong at run-time you'll get some into some unidentified
funky state.

Finally what is, and what is not 'correct' is not for the kernel to
decide arbitrarily, it's a matter of policy, different for each
use-case. 

> Rob

Regards

-- Pantelis

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Pantelis Antoniou]

On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:
> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
> > On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  
> > wrote:
> >> On 10/17/17 14:46, Rob Herring wrote:
> >>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
>  On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
> 
>  Hi Rob,
> 
> > With dependencies on a statically allocated full path name converted to
> > use %pOF format specifier, we can store just the basename of node, and
> > the unflattening of the FDT can be simplified.
> >
> > This commit will affect the remaining users of full_name. After
> > analyzing these users, the remaining cases should only change some print
> > messages. The main users of full_name are providing a name for struct
> > resource. The resource names shouldn't be important other than providing
> > /proc/iomem names.
> >
> > We no longer distinguish between pre and post 0x10 dtb formats as either
> > a full path or basename will work. However, less than 0x10 formats have
> > been broken since the conversion to use libfdt (and no one has cared).
> > The conversion of the unflattening code to be non-recursive also broke
> > pre 0x10 formats as the populate_node function would return 0 in that
> > case.
> >
> > Signed-off-by: Rob Herring 
> > ---
> > v2:
> > - rebase to linux-next
> >
> >  drivers/of/fdt.c | 69 
> > +---
> >  1 file changed, 11 insertions(+), 58 deletions(-)
> 
>  I've just updated to the latest next branch and am finding problems
>  applying overlays.   Reverting this commit alleviates things.  The
>  errors I get are:
> 
>  [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
>  [   88.513447] OF: overlay: apply failed '/__symbols__'
>  [   88.518423] create_overlay: Failed to create overlay (err=-12)
> >>>
> >>> Frank's series with overlay updates should fix this.
> >>
> >> Yes, it does:
> >>
> >>   [PATCH v3 11/12] of: overlay: remove a dependency on device node 
> >> full_name
> >
> > Thanks for the fast response.  I fetched the dt/next branch to test
> > this but there are sufficient changes that Pantelis' "OF: DT-Overlay
> > configfs interface (v7)" is broken now.  I've been adding that
> > downstream since 4.4.  We're using it as an interface for applying
> > overlays to program FPGAs.  If we fix it again, is there any chance
> > that can go upstream now?
> 
> With a drive-by posting once every few years, no.
> 

I take offense to that. There's nothing changed in the patch for years.
Reposting the same patch without changes would achieve nothing.

> The issue remains that the kernel is not really setup to deal with any
> random property or node to be changed at any point in run-time. I
> think there needs to be some restrictions around what the overlays can
> touch. We can't have it be wide open and then lock things down later
> and break users. One example of what you could do is you can only add
> sub-trees to whitelisted nodes. That's probably acceptable for your
> usecase.
> 

Defining what can and what cannot be changed is not as trivial as a
list of white-listed nodes.

In some cases there is a whole node hierarchy being inserted (like in
a FPGA). In others, it's merely changing a status property to "okay" and
a few device parameters.

The real issue is that the kernel has no way to verify that a given
device tree, either at boot time or at overlay application time, is
correct.

When the tree is wrong at boot-time you'll hang (if you're lucky).
If the tree is wrong at run-time you'll get some into some unidentified
funky state.

Finally what is, and what is not 'correct' is not for the kernel to
decide arbitrarily, it's a matter of policy, different for each
use-case. 

> Rob

Regards

-- Pantelis

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Rob Herring]

On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
> On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  wrote:
>> On 10/17/17 14:46, Rob Herring wrote:
>>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
 On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:

 Hi Rob,

> With dependencies on a statically allocated full path name converted to
> use %pOF format specifier, we can store just the basename of node, and
> the unflattening of the FDT can be simplified.
>
> This commit will affect the remaining users of full_name. After
> analyzing these users, the remaining cases should only change some print
> messages. The main users of full_name are providing a name for struct
> resource. The resource names shouldn't be important other than providing
> /proc/iomem names.
>
> We no longer distinguish between pre and post 0x10 dtb formats as either
> a full path or basename will work. However, less than 0x10 formats have
> been broken since the conversion to use libfdt (and no one has cared).
> The conversion of the unflattening code to be non-recursive also broke
> pre 0x10 formats as the populate_node function would return 0 in that
> case.
>
> Signed-off-by: Rob Herring 
> ---
> v2:
> - rebase to linux-next
>
>  drivers/of/fdt.c | 69 
> +---
>  1 file changed, 11 insertions(+), 58 deletions(-)

 I've just updated to the latest next branch and am finding problems
 applying overlays.   Reverting this commit alleviates things.  The
 errors I get are:

 [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
 [   88.513447] OF: overlay: apply failed '/__symbols__'
 [   88.518423] create_overlay: Failed to create overlay (err=-12)
>>>
>>> Frank's series with overlay updates should fix this.
>>
>> Yes, it does:
>>
>>   [PATCH v3 11/12] of: overlay: remove a dependency on device node full_name
>
> Thanks for the fast response.  I fetched the dt/next branch to test
> this but there are sufficient changes that Pantelis' "OF: DT-Overlay
> configfs interface (v7)" is broken now.  I've been adding that
> downstream since 4.4.  We're using it as an interface for applying
> overlays to program FPGAs.  If we fix it again, is there any chance
> that can go upstream now?

With a drive-by posting once every few years, no.

The issue remains that the kernel is not really setup to deal with any
random property or node to be changed at any point in run-time. I
think there needs to be some restrictions around what the overlays can
touch. We can't have it be wide open and then lock things down later
and break users. One example of what you could do is you can only add
sub-trees to whitelisted nodes. That's probably acceptable for your
usecase.

Rob

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Rob Herring]

On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull  wrote:
> On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  wrote:
>> On 10/17/17 14:46, Rob Herring wrote:
>>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
 On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:

 Hi Rob,

> With dependencies on a statically allocated full path name converted to
> use %pOF format specifier, we can store just the basename of node, and
> the unflattening of the FDT can be simplified.
>
> This commit will affect the remaining users of full_name. After
> analyzing these users, the remaining cases should only change some print
> messages. The main users of full_name are providing a name for struct
> resource. The resource names shouldn't be important other than providing
> /proc/iomem names.
>
> We no longer distinguish between pre and post 0x10 dtb formats as either
> a full path or basename will work. However, less than 0x10 formats have
> been broken since the conversion to use libfdt (and no one has cared).
> The conversion of the unflattening code to be non-recursive also broke
> pre 0x10 formats as the populate_node function would return 0 in that
> case.
>
> Signed-off-by: Rob Herring 
> ---
> v2:
> - rebase to linux-next
>
>  drivers/of/fdt.c | 69 
> +---
>  1 file changed, 11 insertions(+), 58 deletions(-)

 I've just updated to the latest next branch and am finding problems
 applying overlays.   Reverting this commit alleviates things.  The
 errors I get are:

 [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
 [   88.513447] OF: overlay: apply failed '/__symbols__'
 [   88.518423] create_overlay: Failed to create overlay (err=-12)
>>>
>>> Frank's series with overlay updates should fix this.
>>
>> Yes, it does:
>>
>>   [PATCH v3 11/12] of: overlay: remove a dependency on device node full_name
>
> Thanks for the fast response.  I fetched the dt/next branch to test
> this but there are sufficient changes that Pantelis' "OF: DT-Overlay
> configfs interface (v7)" is broken now.  I've been adding that
> downstream since 4.4.  We're using it as an interface for applying
> overlays to program FPGAs.  If we fix it again, is there any chance
> that can go upstream now?

With a drive-by posting once every few years, no.

The issue remains that the kernel is not really setup to deal with any
random property or node to be changed at any point in run-time. I
think there needs to be some restrictions around what the overlays can
touch. We can't have it be wide open and then lock things down later
and break users. One example of what you could do is you can only add
sub-trees to whitelisted nodes. That's probably acceptable for your
usecase.

Rob

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Alan Tull]

On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  wrote:
> On 10/17/17 14:46, Rob Herring wrote:
>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
>>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
>>>
>>> Hi Rob,
>>>
 With dependencies on a statically allocated full path name converted to
 use %pOF format specifier, we can store just the basename of node, and
 the unflattening of the FDT can be simplified.

 This commit will affect the remaining users of full_name. After
 analyzing these users, the remaining cases should only change some print
 messages. The main users of full_name are providing a name for struct
 resource. The resource names shouldn't be important other than providing
 /proc/iomem names.

 We no longer distinguish between pre and post 0x10 dtb formats as either
 a full path or basename will work. However, less than 0x10 formats have
 been broken since the conversion to use libfdt (and no one has cared).
 The conversion of the unflattening code to be non-recursive also broke
 pre 0x10 formats as the populate_node function would return 0 in that
 case.

 Signed-off-by: Rob Herring 
 ---
 v2:
 - rebase to linux-next

  drivers/of/fdt.c | 69 
 +---
  1 file changed, 11 insertions(+), 58 deletions(-)
>>>
>>> I've just updated to the latest next branch and am finding problems
>>> applying overlays.   Reverting this commit alleviates things.  The
>>> errors I get are:
>>>
>>> [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
>>> [   88.513447] OF: overlay: apply failed '/__symbols__'
>>> [   88.518423] create_overlay: Failed to create overlay (err=-12)
>>
>> Frank's series with overlay updates should fix this.
>
> Yes, it does:
>
>   [PATCH v3 11/12] of: overlay: remove a dependency on device node full_name

Thanks for the fast response.  I fetched the dt/next branch to test
this but there are sufficient changes that Pantelis' "OF: DT-Overlay
configfs interface (v7)" is broken now.  I've been adding that
downstream since 4.4.  We're using it as an interface for applying
overlays to program FPGAs.  If we fix it again, is there any chance
that can go upstream now?

Alan

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Alan Tull]

On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand  wrote:
> On 10/17/17 14:46, Rob Herring wrote:
>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
>>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
>>>
>>> Hi Rob,
>>>
 With dependencies on a statically allocated full path name converted to
 use %pOF format specifier, we can store just the basename of node, and
 the unflattening of the FDT can be simplified.

 This commit will affect the remaining users of full_name. After
 analyzing these users, the remaining cases should only change some print
 messages. The main users of full_name are providing a name for struct
 resource. The resource names shouldn't be important other than providing
 /proc/iomem names.

 We no longer distinguish between pre and post 0x10 dtb formats as either
 a full path or basename will work. However, less than 0x10 formats have
 been broken since the conversion to use libfdt (and no one has cared).
 The conversion of the unflattening code to be non-recursive also broke
 pre 0x10 formats as the populate_node function would return 0 in that
 case.

 Signed-off-by: Rob Herring 
 ---
 v2:
 - rebase to linux-next

  drivers/of/fdt.c | 69 
 +---
  1 file changed, 11 insertions(+), 58 deletions(-)
>>>
>>> I've just updated to the latest next branch and am finding problems
>>> applying overlays.   Reverting this commit alleviates things.  The
>>> errors I get are:
>>>
>>> [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
>>> [   88.513447] OF: overlay: apply failed '/__symbols__'
>>> [   88.518423] create_overlay: Failed to create overlay (err=-12)
>>
>> Frank's series with overlay updates should fix this.
>
> Yes, it does:
>
>   [PATCH v3 11/12] of: overlay: remove a dependency on device node full_name

Thanks for the fast response.  I fetched the dt/next branch to test
this but there are sufficient changes that Pantelis' "OF: DT-Overlay
configfs interface (v7)" is broken now.  I've been adding that
downstream since 4.4.  We're using it as an interface for applying
overlays to program FPGAs.  If we fix it again, is there any chance
that can go upstream now?

Alan

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Frank Rowand]

On 10/17/17 14:46, Rob Herring wrote:
> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
>>
>> Hi Rob,
>>
>>> With dependencies on a statically allocated full path name converted to
>>> use %pOF format specifier, we can store just the basename of node, and
>>> the unflattening of the FDT can be simplified.
>>>
>>> This commit will affect the remaining users of full_name. After
>>> analyzing these users, the remaining cases should only change some print
>>> messages. The main users of full_name are providing a name for struct
>>> resource. The resource names shouldn't be important other than providing
>>> /proc/iomem names.
>>>
>>> We no longer distinguish between pre and post 0x10 dtb formats as either
>>> a full path or basename will work. However, less than 0x10 formats have
>>> been broken since the conversion to use libfdt (and no one has cared).
>>> The conversion of the unflattening code to be non-recursive also broke
>>> pre 0x10 formats as the populate_node function would return 0 in that
>>> case.
>>>
>>> Signed-off-by: Rob Herring 
>>> ---
>>> v2:
>>> - rebase to linux-next
>>>
>>>  drivers/of/fdt.c | 69 
>>> +---
>>>  1 file changed, 11 insertions(+), 58 deletions(-)
>>
>> I've just updated to the latest next branch and am finding problems
>> applying overlays.   Reverting this commit alleviates things.  The
>> errors I get are:
>>
>> [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
>> [   88.513447] OF: overlay: apply failed '/__symbols__'
>> [   88.518423] create_overlay: Failed to create overlay (err=-12)
> 
> Frank's series with overlay updates should fix this.

Yes, it does:

  [PATCH v3 11/12] of: overlay: remove a dependency on device node full_name

-Frank

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Frank Rowand]

On 10/17/17 14:46, Rob Herring wrote:
> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
>>
>> Hi Rob,
>>
>>> With dependencies on a statically allocated full path name converted to
>>> use %pOF format specifier, we can store just the basename of node, and
>>> the unflattening of the FDT can be simplified.
>>>
>>> This commit will affect the remaining users of full_name. After
>>> analyzing these users, the remaining cases should only change some print
>>> messages. The main users of full_name are providing a name for struct
>>> resource. The resource names shouldn't be important other than providing
>>> /proc/iomem names.
>>>
>>> We no longer distinguish between pre and post 0x10 dtb formats as either
>>> a full path or basename will work. However, less than 0x10 formats have
>>> been broken since the conversion to use libfdt (and no one has cared).
>>> The conversion of the unflattening code to be non-recursive also broke
>>> pre 0x10 formats as the populate_node function would return 0 in that
>>> case.
>>>
>>> Signed-off-by: Rob Herring 
>>> ---
>>> v2:
>>> - rebase to linux-next
>>>
>>>  drivers/of/fdt.c | 69 
>>> +---
>>>  1 file changed, 11 insertions(+), 58 deletions(-)
>>
>> I've just updated to the latest next branch and am finding problems
>> applying overlays.   Reverting this commit alleviates things.  The
>> errors I get are:
>>
>> [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
>> [   88.513447] OF: overlay: apply failed '/__symbols__'
>> [   88.518423] create_overlay: Failed to create overlay (err=-12)
> 
> Frank's series with overlay updates should fix this.

Yes, it does:

  [PATCH v3 11/12] of: overlay: remove a dependency on device node full_name

-Frank

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Rob Herring]

On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
>
> Hi Rob,
>
>> With dependencies on a statically allocated full path name converted to
>> use %pOF format specifier, we can store just the basename of node, and
>> the unflattening of the FDT can be simplified.
>>
>> This commit will affect the remaining users of full_name. After
>> analyzing these users, the remaining cases should only change some print
>> messages. The main users of full_name are providing a name for struct
>> resource. The resource names shouldn't be important other than providing
>> /proc/iomem names.
>>
>> We no longer distinguish between pre and post 0x10 dtb formats as either
>> a full path or basename will work. However, less than 0x10 formats have
>> been broken since the conversion to use libfdt (and no one has cared).
>> The conversion of the unflattening code to be non-recursive also broke
>> pre 0x10 formats as the populate_node function would return 0 in that
>> case.
>>
>> Signed-off-by: Rob Herring 
>> ---
>> v2:
>> - rebase to linux-next
>>
>>  drivers/of/fdt.c | 69 
>> +---
>>  1 file changed, 11 insertions(+), 58 deletions(-)
>
> I've just updated to the latest next branch and am finding problems
> applying overlays.   Reverting this commit alleviates things.  The
> errors I get are:
>
> [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
> [   88.513447] OF: overlay: apply failed '/__symbols__'
> [   88.518423] create_overlay: Failed to create overlay (err=-12)

Frank's series with overlay updates should fix this.

Rob

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Rob Herring]

On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull  wrote:
> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:
>
> Hi Rob,
>
>> With dependencies on a statically allocated full path name converted to
>> use %pOF format specifier, we can store just the basename of node, and
>> the unflattening of the FDT can be simplified.
>>
>> This commit will affect the remaining users of full_name. After
>> analyzing these users, the remaining cases should only change some print
>> messages. The main users of full_name are providing a name for struct
>> resource. The resource names shouldn't be important other than providing
>> /proc/iomem names.
>>
>> We no longer distinguish between pre and post 0x10 dtb formats as either
>> a full path or basename will work. However, less than 0x10 formats have
>> been broken since the conversion to use libfdt (and no one has cared).
>> The conversion of the unflattening code to be non-recursive also broke
>> pre 0x10 formats as the populate_node function would return 0 in that
>> case.
>>
>> Signed-off-by: Rob Herring 
>> ---
>> v2:
>> - rebase to linux-next
>>
>>  drivers/of/fdt.c | 69 
>> +---
>>  1 file changed, 11 insertions(+), 58 deletions(-)
>
> I've just updated to the latest next branch and am finding problems
> applying overlays.   Reverting this commit alleviates things.  The
> errors I get are:
>
> [   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
> [   88.513447] OF: overlay: apply failed '/__symbols__'
> [   88.518423] create_overlay: Failed to create overlay (err=-12)

Frank's series with overlay updates should fix this.

Rob

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Alan Tull]

On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:

Hi Rob,

> With dependencies on a statically allocated full path name converted to
> use %pOF format specifier, we can store just the basename of node, and
> the unflattening of the FDT can be simplified.
>
> This commit will affect the remaining users of full_name. After
> analyzing these users, the remaining cases should only change some print
> messages. The main users of full_name are providing a name for struct
> resource. The resource names shouldn't be important other than providing
> /proc/iomem names.
>
> We no longer distinguish between pre and post 0x10 dtb formats as either
> a full path or basename will work. However, less than 0x10 formats have
> been broken since the conversion to use libfdt (and no one has cared).
> The conversion of the unflattening code to be non-recursive also broke
> pre 0x10 formats as the populate_node function would return 0 in that
> case.
>
> Signed-off-by: Rob Herring 
> ---
> v2:
> - rebase to linux-next
>
>  drivers/of/fdt.c | 69 
> +---
>  1 file changed, 11 insertions(+), 58 deletions(-)

I've just updated to the latest next branch and am finding problems
applying overlays.   Reverting this commit alleviates things.  The
errors I get are:

[   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
[   88.513447] OF: overlay: apply failed '/__symbols__'
[   88.518423] create_overlay: Failed to create overlay (err=-12)

My branch also includes Pantelis' patch that turns on symbols [1].

Alan

[1] 
https://github.com/pantoniou/linux-beagle-track-mainline/commit/486c87d450f2895a766c6097328d0c6538ec7a31

Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name

[Alan Tull]

On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring  wrote:

Hi Rob,

> With dependencies on a statically allocated full path name converted to
> use %pOF format specifier, we can store just the basename of node, and
> the unflattening of the FDT can be simplified.
>
> This commit will affect the remaining users of full_name. After
> analyzing these users, the remaining cases should only change some print
> messages. The main users of full_name are providing a name for struct
> resource. The resource names shouldn't be important other than providing
> /proc/iomem names.
>
> We no longer distinguish between pre and post 0x10 dtb formats as either
> a full path or basename will work. However, less than 0x10 formats have
> been broken since the conversion to use libfdt (and no one has cared).
> The conversion of the unflattening code to be non-recursive also broke
> pre 0x10 formats as the populate_node function would return 0 in that
> case.
>
> Signed-off-by: Rob Herring 
> ---
> v2:
> - rebase to linux-next
>
>  drivers/of/fdt.c | 69 
> +---
>  1 file changed, 11 insertions(+), 58 deletions(-)

I've just updated to the latest next branch and am finding problems
applying overlays.   Reverting this commit alleviates things.  The
errors I get are:

[   88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0
[   88.513447] OF: overlay: apply failed '/__symbols__'
[   88.518423] create_overlay: Failed to create overlay (err=-12)

My branch also includes Pantelis' patch that turns on symbols [1].

Alan

[1] 
https://github.com/pantoniou/linux-beagle-track-mainline/commit/486c87d450f2895a766c6097328d0c6538ec7a31