Re: [PATCH v5 0/4] of: remove *phandle properties from expanded device tree

[Frank Rowand]

On 06/15/17 17:06, Benjamin Herrenschmidt wrote:
> On Fri, 2017-06-09 at 22:47 -0700, frowand.l...@gmail.com wrote:
>> From: Frank Rowand 
>>
>> Remove "phandle" and "linux,phandle" properties from the internal
>> device tree.  The phandle will still be in the struct device_node
>> phandle field.
>>
>> This is to resolve the issue found by Stephen Boyd [1] when he changed
>> the type of struct property.value from void * to const void *.  As
>> a result of the type change, the overlay code had compile errors
>> where the resolver updates phandle values.
> 
> Piggy back on the powerpc breakage thread...
> 
> So various things in userspace will consume these. The main one is
> kexec which needs them to rebuild a fdt. So if you're going to do that
> you need to modify the sysfs code to expose a phandle attribute from
> the device node ->phandle.

Hi Ben,

Thanks for the comment.  My devicetree knowledge related to powerpc
is more limited than I would like, so any insights from you are
greatly appreciated.

/proc/device-tree is a documented ABI, so the patches do contain files
there for phandle.  (See the new function __of_add_phandle_sysfs() if
you want details.)  One difference from before is that if more than one of
"phandle", "linux,phandle", or "ibm,phandle" exist for a node, there will
only be one file "phandle" for the node under /proc/device-tree.  This
reflects the reality that the struct device_node phandle field has been
collapsing the three different properties into a single value.

For any interested parties, the current status (thanks to Guenter's
debugging efforts) is that the struct device_node type field is
NULL when it should contain a pointer to a type.  I am in the process
of setting up a qemu environment so I can debug the problem directly
instead of burdening Guenter with the extra work.  Rob is planning
to (or may have already) removed the patch series from the branch
that feeds into -next.

Thanks,

Frank

Re: [PATCH v5 0/4] of: remove *phandle properties from expanded device tree

[Frank Rowand]

On 06/15/17 17:06, Benjamin Herrenschmidt wrote:
> On Fri, 2017-06-09 at 22:47 -0700, frowand.l...@gmail.com wrote:
>> From: Frank Rowand 
>>
>> Remove "phandle" and "linux,phandle" properties from the internal
>> device tree.  The phandle will still be in the struct device_node
>> phandle field.
>>
>> This is to resolve the issue found by Stephen Boyd [1] when he changed
>> the type of struct property.value from void * to const void *.  As
>> a result of the type change, the overlay code had compile errors
>> where the resolver updates phandle values.
> 
> Piggy back on the powerpc breakage thread...
> 
> So various things in userspace will consume these. The main one is
> kexec which needs them to rebuild a fdt. So if you're going to do that
> you need to modify the sysfs code to expose a phandle attribute from
> the device node ->phandle.

Hi Ben,

Thanks for the comment.  My devicetree knowledge related to powerpc
is more limited than I would like, so any insights from you are
greatly appreciated.

/proc/device-tree is a documented ABI, so the patches do contain files
there for phandle.  (See the new function __of_add_phandle_sysfs() if
you want details.)  One difference from before is that if more than one of
"phandle", "linux,phandle", or "ibm,phandle" exist for a node, there will
only be one file "phandle" for the node under /proc/device-tree.  This
reflects the reality that the struct device_node phandle field has been
collapsing the three different properties into a single value.

For any interested parties, the current status (thanks to Guenter's
debugging efforts) is that the struct device_node type field is
NULL when it should contain a pointer to a type.  I am in the process
of setting up a qemu environment so I can debug the problem directly
instead of burdening Guenter with the extra work.  Rob is planning
to (or may have already) removed the patch series from the branch
that feeds into -next.

Thanks,

Frank

Re: [PATCH v5 0/4] of: remove *phandle properties from expanded device tree

[Benjamin Herrenschmidt]

On Fri, 2017-06-09 at 22:47 -0700, frowand.l...@gmail.com wrote:
> From: Frank Rowand 
> 
> Remove "phandle" and "linux,phandle" properties from the internal
> device tree.  The phandle will still be in the struct device_node
> phandle field.
> 
> This is to resolve the issue found by Stephen Boyd [1] when he changed
> the type of struct property.value from void * to const void *.  As
> a result of the type change, the overlay code had compile errors
> where the resolver updates phandle values.

Piggy back on the powerpc breakage thread...

So various things in userspace will consume these. The main one is
kexec which needs them to rebuild a fdt. So if you're going to do that
you need to modify the sysfs code to expose a phandle attribute from
the device node ->phandle.

Cheers,
Ben.

Re: [PATCH v5 0/4] of: remove *phandle properties from expanded device tree

[Benjamin Herrenschmidt]

On Fri, 2017-06-09 at 22:47 -0700, frowand.l...@gmail.com wrote:
> From: Frank Rowand 
> 
> Remove "phandle" and "linux,phandle" properties from the internal
> device tree.  The phandle will still be in the struct device_node
> phandle field.
> 
> This is to resolve the issue found by Stephen Boyd [1] when he changed
> the type of struct property.value from void * to const void *.  As
> a result of the type change, the overlay code had compile errors
> where the resolver updates phandle values.

Piggy back on the powerpc breakage thread...

So various things in userspace will consume these. The main one is
kexec which needs them to rebuild a fdt. So if you're going to do that
you need to modify the sysfs code to expose a phandle attribute from
the device node ->phandle.

Cheers,
Ben.

Re: [PATCH v5 0/4] of: remove *phandle properties from expanded device tree

[Rob Herring]

On Sat, Jun 10, 2017 at 12:47 AM,   wrote:
> From: Frank Rowand 
>
> Remove "phandle" and "linux,phandle" properties from the internal
> device tree.  The phandle will still be in the struct device_node
> phandle field.
>
> This is to resolve the issue found by Stephen Boyd [1] when he changed
> the type of struct property.value from void * to const void *.  As
> a result of the type change, the overlay code had compile errors
> where the resolver updates phandle values.
>
>   [1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html
>
> Patch 1 is the phandle related changes.
>
> Patches 2 - 4 are minor fixups for issues that became visible
> while implementing patch 1.
>
> Changes from v4:
>- rebase on 4.12-rc1
>- Add reason for "" in of_attach_node()
>- Simplify and consolidate phandle detection logic in
>  populate_properties().  This results in a change of behaviour,
>  the value of property "ibm,phandle" will no longer override the
>  value of properties "phandle" and "linux,phandle".
>
> Changes from v3:
>- patch 1: fix incorrect variable name in __of_add_phandle_sysfs().
>  Problem was reported by the kbuild test robot
>
> Changes from v2:
>- patch 1: Remove check in __of_add_phandle_sysfs() that would not
>  add a sysfs entry if IS_ENABLED(CONFIG_PPC_PSERIES)
>
> Changes from v1:
>- Remove phandle properties in of_attach_node(), before attaching
>  the node to the live tree.
>- Add the phandle sysfs entry for the node in of_attach_node().
>- When creating an overlay changeset, duplicate the node phandle in
>  __of_node_dup().
>
> Frank Rowand (4):
>   of: remove *phandle properties from expanded device tree
>   of: make __of_attach_node() static
>   of: be consistent in form of file mode
>   of: detect invalid phandle in overlay
>
>  drivers/of/base.c   | 50 +++
>  drivers/of/dynamic.c| 57 
> +
>  drivers/of/fdt.c| 43 ++---
>  drivers/of/of_private.h |  2 +-
>  drivers/of/overlay.c|  8 ---
>  drivers/of/resolver.c   | 23 +---
>  include/linux/of.h  |  1 +
>  7 files changed, 118 insertions(+), 66 deletions(-)

Series applied.

Rob

Re: [PATCH v5 0/4] of: remove *phandle properties from expanded device tree

[Rob Herring]

On Sat, Jun 10, 2017 at 12:47 AM,   wrote:
> From: Frank Rowand 
>
> Remove "phandle" and "linux,phandle" properties from the internal
> device tree.  The phandle will still be in the struct device_node
> phandle field.
>
> This is to resolve the issue found by Stephen Boyd [1] when he changed
> the type of struct property.value from void * to const void *.  As
> a result of the type change, the overlay code had compile errors
> where the resolver updates phandle values.
>
>   [1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html
>
> Patch 1 is the phandle related changes.
>
> Patches 2 - 4 are minor fixups for issues that became visible
> while implementing patch 1.
>
> Changes from v4:
>- rebase on 4.12-rc1
>- Add reason for "" in of_attach_node()
>- Simplify and consolidate phandle detection logic in
>  populate_properties().  This results in a change of behaviour,
>  the value of property "ibm,phandle" will no longer override the
>  value of properties "phandle" and "linux,phandle".
>
> Changes from v3:
>- patch 1: fix incorrect variable name in __of_add_phandle_sysfs().
>  Problem was reported by the kbuild test robot
>
> Changes from v2:
>- patch 1: Remove check in __of_add_phandle_sysfs() that would not
>  add a sysfs entry if IS_ENABLED(CONFIG_PPC_PSERIES)
>
> Changes from v1:
>- Remove phandle properties in of_attach_node(), before attaching
>  the node to the live tree.
>- Add the phandle sysfs entry for the node in of_attach_node().
>- When creating an overlay changeset, duplicate the node phandle in
>  __of_node_dup().
>
> Frank Rowand (4):
>   of: remove *phandle properties from expanded device tree
>   of: make __of_attach_node() static
>   of: be consistent in form of file mode
>   of: detect invalid phandle in overlay
>
>  drivers/of/base.c   | 50 +++
>  drivers/of/dynamic.c| 57 
> +
>  drivers/of/fdt.c| 43 ++---
>  drivers/of/of_private.h |  2 +-
>  drivers/of/overlay.c|  8 ---
>  drivers/of/resolver.c   | 23 +---
>  include/linux/of.h  |  1 +
>  7 files changed, 118 insertions(+), 66 deletions(-)

Series applied.

Rob

[PATCH v5 0/4] of: remove *phandle properties from expanded device tree

[frowand . list]

From: Frank Rowand 

Remove "phandle" and "linux,phandle" properties from the internal
device tree.  The phandle will still be in the struct device_node
phandle field.

This is to resolve the issue found by Stephen Boyd [1] when he changed
the type of struct property.value from void * to const void *.  As
a result of the type change, the overlay code had compile errors
where the resolver updates phandle values.

  [1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html

Patch 1 is the phandle related changes.

Patches 2 - 4 are minor fixups for issues that became visible
while implementing patch 1.

Changes from v4:
   - rebase on 4.12-rc1
   - Add reason for "" in of_attach_node()
   - Simplify and consolidate phandle detection logic in
 populate_properties().  This results in a change of behaviour,
 the value of property "ibm,phandle" will no longer override the
 value of properties "phandle" and "linux,phandle".

Changes from v3:
   - patch 1: fix incorrect variable name in __of_add_phandle_sysfs().
 Problem was reported by the kbuild test robot

Changes from v2:
   - patch 1: Remove check in __of_add_phandle_sysfs() that would not
 add a sysfs entry if IS_ENABLED(CONFIG_PPC_PSERIES)

Changes from v1:
   - Remove phandle properties in of_attach_node(), before attaching
 the node to the live tree.
   - Add the phandle sysfs entry for the node in of_attach_node().
   - When creating an overlay changeset, duplicate the node phandle in
 __of_node_dup().

Frank Rowand (4):
  of: remove *phandle properties from expanded device tree
  of: make __of_attach_node() static
  of: be consistent in form of file mode
  of: detect invalid phandle in overlay

 drivers/of/base.c   | 50 +++
 drivers/of/dynamic.c| 57 +
 drivers/of/fdt.c| 43 ++---
 drivers/of/of_private.h |  2 +-
 drivers/of/overlay.c|  8 ---
 drivers/of/resolver.c   | 23 +---
 include/linux/of.h  |  1 +
 7 files changed, 118 insertions(+), 66 deletions(-)

-- 
Frank Rowand 

[PATCH v5 0/4] of: remove *phandle properties from expanded device tree

[frowand . list]

From: Frank Rowand 

Remove "phandle" and "linux,phandle" properties from the internal
device tree.  The phandle will still be in the struct device_node
phandle field.

This is to resolve the issue found by Stephen Boyd [1] when he changed
the type of struct property.value from void * to const void *.  As
a result of the type change, the overlay code had compile errors
where the resolver updates phandle values.

  [1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html

Patch 1 is the phandle related changes.

Patches 2 - 4 are minor fixups for issues that became visible
while implementing patch 1.

Changes from v4:
   - rebase on 4.12-rc1
   - Add reason for "" in of_attach_node()
   - Simplify and consolidate phandle detection logic in
 populate_properties().  This results in a change of behaviour,
 the value of property "ibm,phandle" will no longer override the
 value of properties "phandle" and "linux,phandle".

Changes from v3:
   - patch 1: fix incorrect variable name in __of_add_phandle_sysfs().
 Problem was reported by the kbuild test robot

Changes from v2:
   - patch 1: Remove check in __of_add_phandle_sysfs() that would not
 add a sysfs entry if IS_ENABLED(CONFIG_PPC_PSERIES)

Changes from v1:
   - Remove phandle properties in of_attach_node(), before attaching
 the node to the live tree.
   - Add the phandle sysfs entry for the node in of_attach_node().
   - When creating an overlay changeset, duplicate the node phandle in
 __of_node_dup().

Frank Rowand (4):
  of: remove *phandle properties from expanded device tree
  of: make __of_attach_node() static
  of: be consistent in form of file mode
  of: detect invalid phandle in overlay

 drivers/of/base.c   | 50 +++
 drivers/of/dynamic.c| 57 +
 drivers/of/fdt.c| 43 ++---
 drivers/of/of_private.h |  2 +-
 drivers/of/overlay.c|  8 ---
 drivers/of/resolver.c   | 23 +---
 include/linux/of.h  |  1 +
 7 files changed, 118 insertions(+), 66 deletions(-)

-- 
Frank Rowand