diff mbox series

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

Message ID 20170821151651.25096-6-robh@kernel.org
State Accepted
Commit a7e4cfb0a7ca4773e7d0dd1d9c018ab27a15360e
Headers show
Series [v2,1/5] powerpc: Convert to using %pOF instead of full_name | expand

Commit Message

Rob Herring Aug. 21, 2017, 3:16 p.m. UTC
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 <robh@kernel.org>

---
v2:
- rebase to linux-next

 drivers/of/fdt.c | 69 +++++++++-----------------------------------------------
 1 file changed, 11 insertions(+), 58 deletions(-)

-- 
2.11.0

Comments

Rob Herring Oct. 18, 2017, 3:44 p.m. UTC | #1
On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull <atull@kernel.org> wrote:
> On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand <frowand.list@gmail.com> wrote:

>> On 10/17/17 14:46, Rob Herring wrote:

>>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull <atull@kernel.org> wrote:

>>>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring <robh@kernel.org> 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 <robh@kernel.org>

>>>>> ---

>>>>> 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
Rob Herring Oct. 18, 2017, 6:30 p.m. UTC | #2
On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou
<pantelis.antoniou@konsulko.com> wrote:
> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:

>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull <atull@kernel.org> wrote:

>> > On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand <frowand.list@gmail.com> wrote:

>> >> On 10/17/17 14:46, Rob Herring wrote:

>> >>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull <atull@kernel.org> wrote:

>> >>>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring <robh@kernel.org> 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 <robh@kernel.org>

>> >>>>> ---

>> >>>>> 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 state.


Or have some security hole or a mechanism for userspace to crash the system.

> 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.


It is if the kernel will break doing so.

Rob
Frank Rowand Oct. 18, 2017, 9:46 p.m. UTC | #3
On 10/18/17 11:30, Rob Herring wrote:
> On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou

> <pantelis.antoniou@konsulko.com> wrote:

>> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:

>>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull <atull@kernel.org> wrote:

>>>> On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand <frowand.list@gmail.com> wrote:

>>>>> On 10/17/17 14:46, Rob Herring wrote:

>>>>>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull <atull@kernel.org> wrote:

>>>>>>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring <robh@kernel.org> 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 <robh@kernel.org>

>>>>>>>> ---

>>>>>>>> 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 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 state.

> 

> Or have some security hole or a mechanism for userspace to crash the system.

> 

>> 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.

> 

> It is if the kernel will break doing so.

> 

> Rob

>
Moritz Fischer Oct. 19, 2017, 8:06 p.m. UTC | #4
On Thu, Oct 19, 2017 at 11:51:40AM +0300, Pantelis Antoniou wrote:
> Hi Rob,

> 

> > On Oct 18, 2017, at 21:30 , Rob Herring <robh@kernel.org> wrote:

> > 

> > On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou

> > <pantelis.antoniou@konsulko.com> wrote:

> >> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote:

> >>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull <atull@kernel.org> wrote:

> >>>> On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand <frowand.list@gmail.com> wrote:

> >>>>> On 10/17/17 14:46, Rob Herring wrote:

> >>>>>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull <atull@kernel.org> wrote:

> >>>>>>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring <robh@kernel.org> 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 <robh@kernel.org>

> >>>>>>>> ---

> >>>>>>>> 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 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.

> > 

> 

> So it seems a simple whitelist won’t cut it. We’re already talking about

> special casing for this or that property.

> 

> My argument is that this kind of validation is not part of the core-device tree,

> but instead is a policy decision different for each board.

>  

> >> 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.

> > 

> > Or have some security hole or a mechanism for userspace to crash the system.

> > 

> 

> User-space as in regular users should never have enough privileges to apply an

> overlay, same as in loading a kernel module.

> 

> >> 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.

> > 

> > It is if the kernel will break doing so.

> > 

> 

> I still haven’t seen a real example of the kernel breaking.

> 

> I have seen a lot of cases where the kernel is crashing due to the device

> removal path being broken, but those are kernel bugs to fix, not something

> to use to hold back functionality that people want to use.


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

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.

> 

> > Rob

> 

> Regards

> 

> — Pantelis

> 

> --

> To unsubscribe from this list: send the line "unsubscribe linux-fpga" in

> the body of a message to majordomo@vger.kernel.org

> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers,

Moritz
Frank Rowand Oct. 19, 2017, 9:46 p.m. UTC | #5
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.
diff mbox series

Patch

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index ce30c9a588a4..27c535af0be8 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -266,74 +266,32 @@  static void populate_properties(const void *blob,
 		*pprev = NULL;
 }
 
-static unsigned int populate_node(const void *blob,
-				  int offset,
-				  void **mem,
-				  struct device_node *dad,
-				  unsigned int fpsize,
-				  struct device_node **pnp,
-				  bool dryrun)
+static bool populate_node(const void *blob,
+			  int offset,
+			  void **mem,
+			  struct device_node *dad,
+			  struct device_node **pnp,
+			  bool dryrun)
 {
 	struct device_node *np;
 	const char *pathp;
 	unsigned int l, allocl;
-	int new_format = 0;
 
 	pathp = fdt_get_name(blob, offset, &l);
 	if (!pathp) {
 		*pnp = NULL;
-		return 0;
+		return false;
 	}
 
 	allocl = ++l;
 
-	/* version 0x10 has a more compact unit name here instead of the full
-	 * path. we accumulate the full path size using "fpsize", we'll rebuild
-	 * it later. We detect this because the first character of the name is
-	 * not '/'.
-	 */
-	if ((*pathp) != '/') {
-		new_format = 1;
-		if (fpsize == 0) {
-			/* root node: special case. fpsize accounts for path
-			 * plus terminating zero. root node only has '/', so
-			 * fpsize should be 2, but we want to avoid the first
-			 * level nodes to have two '/' so we use fpsize 1 here
-			 */
-			fpsize = 1;
-			allocl = 2;
-			l = 1;
-			pathp = "";
-		} else {
-			/* account for '/' and path size minus terminal 0
-			 * already in 'l'
-			 */
-			fpsize += l;
-			allocl = fpsize;
-		}
-	}
-
 	np = unflatten_dt_alloc(mem, sizeof(struct device_node) + allocl,
 				__alignof__(struct device_node));
 	if (!dryrun) {
 		char *fn;
 		of_node_init(np);
 		np->full_name = fn = ((char *)np) + sizeof(*np);
-		if (new_format) {
-			/* rebuild full path for new format */
-			if (dad && dad->parent) {
-				strcpy(fn, dad->full_name);
-#ifdef DEBUG
-				if ((strlen(fn) + l + 1) != allocl) {
-					pr_debug("%s: p: %d, l: %d, a: %d\n",
-						pathp, (int)strlen(fn),
-						l, allocl);
-				}
-#endif
-				fn += strlen(fn);
-			}
-			*(fn++) = '/';
-		}
+
 		memcpy(fn, pathp, l);
 
 		if (dad != NULL) {
@@ -355,7 +313,7 @@  static unsigned int populate_node(const void *blob,
 	}
 
 	*pnp = np;
-	return fpsize;
+	return true;
 }
 
 static void reverse_nodes(struct device_node *parent)
@@ -399,7 +357,6 @@  static int unflatten_dt_nodes(const void *blob,
 	struct device_node *root;
 	int offset = 0, depth = 0, initial_depth = 0;
 #define FDT_MAX_DEPTH	64
-	unsigned int fpsizes[FDT_MAX_DEPTH];
 	struct device_node *nps[FDT_MAX_DEPTH];
 	void *base = mem;
 	bool dryrun = !base;
@@ -418,7 +375,6 @@  static int unflatten_dt_nodes(const void *blob,
 		depth = initial_depth = 1;
 
 	root = dad;
-	fpsizes[depth] = dad ? strlen(of_node_full_name(dad)) : 0;
 	nps[depth] = dad;
 
 	for (offset = 0;
@@ -427,11 +383,8 @@  static int unflatten_dt_nodes(const void *blob,
 		if (WARN_ON_ONCE(depth >= FDT_MAX_DEPTH))
 			continue;
 
-		fpsizes[depth+1] = populate_node(blob, offset, &mem,
-						 nps[depth],
-						 fpsizes[depth],
-						 &nps[depth+1], dryrun);
-		if (!fpsizes[depth+1])
+		if (!populate_node(blob, offset, &mem, nps[depth],
+				   &nps[depth+1], dryrun))
 			return mem - base;
 
 		if (!dryrun && nodepp && !*nodepp)