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

[Michael Ellerman]

frowand.l...@gmail.com writes:

> From: Frank Rowand 
>
> Remove "phandle", "linux,phandle", and "ibm,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
>
> - Add sysfs infrastructure to report np->phandle, as if it was a property.
> - Do not create "phandle" "ibm,phandle", and "linux,phandle" properties
>   in the expanded device tree.
> - Remove phandle properties in of_attach_node(), for nodes dynamically
>   attached to the live tree.  Add the phandle sysfs entry for these nodes.
> - When creating an overlay changeset, duplicate the node phandle in
>   __of_node_dup().
> - Remove no longer needed checks to exclude "phandle" and "linux,phandle"
>   properties in several locations.
> - A side effect of these changes is that the obsolete "linux,phandle" and
>   "ibm,phandle" properties will no longer appear in /proc/device-tree (they
>   will appear as "phandle").
> - A side effect is that the value of property "ibm,phandle" will no longer
>   override the value of properties "phandle" and "linux,phandle".

Can you please Cc linuxppc-dev in future on patches which clearly are
going to impact powerpc?

cheers

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

[Michael Ellerman]

frowand.l...@gmail.com writes:

> From: Frank Rowand 
>
> Remove "phandle", "linux,phandle", and "ibm,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
>
> - Add sysfs infrastructure to report np->phandle, as if it was a property.
> - Do not create "phandle" "ibm,phandle", and "linux,phandle" properties
>   in the expanded device tree.
> - Remove phandle properties in of_attach_node(), for nodes dynamically
>   attached to the live tree.  Add the phandle sysfs entry for these nodes.
> - When creating an overlay changeset, duplicate the node phandle in
>   __of_node_dup().
> - Remove no longer needed checks to exclude "phandle" and "linux,phandle"
>   properties in several locations.
> - A side effect of these changes is that the obsolete "linux,phandle" and
>   "ibm,phandle" properties will no longer appear in /proc/device-tree (they
>   will appear as "phandle").
> - A side effect is that the value of property "ibm,phandle" will no longer
>   override the value of properties "phandle" and "linux,phandle".

Can you please Cc linuxppc-dev in future on patches which clearly are
going to impact powerpc?

cheers

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

[frowand . list]

From: Frank Rowand 

Remove "phandle", "linux,phandle", and "ibm,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

- Add sysfs infrastructure to report np->phandle, as if it was a property.
- Do not create "phandle" "ibm,phandle", and "linux,phandle" properties
  in the expanded device tree.
- Remove phandle properties in of_attach_node(), for nodes dynamically
  attached to the live tree.  Add the phandle sysfs entry for these nodes.
- When creating an overlay changeset, duplicate the node phandle in
  __of_node_dup().
- Remove no longer needed checks to exclude "phandle" and "linux,phandle"
  properties in several locations.
- A side effect of these changes is that the obsolete "linux,phandle" and
  "ibm,phandle" properties will no longer appear in /proc/device-tree (they
  will appear as "phandle").
- A side effect is that the value of property "ibm,phandle" will no longer
  override the value of properties "phandle" and "linux,phandle".

Signed-off-by: Frank Rowand 
---
 drivers/of/base.c   | 48 +++---
 drivers/of/dynamic.c| 55 +
 drivers/of/fdt.c| 43 +++---
 drivers/of/of_private.h |  1 +
 drivers/of/overlay.c|  4 +---
 drivers/of/resolver.c   | 23 +
 include/linux/of.h  |  1 +
 7 files changed, 112 insertions(+), 63 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 28d5f53bc631..941c9a03471d 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -116,6 +116,19 @@ static ssize_t of_node_property_read(struct file *filp, 
struct kobject *kobj,
return memory_read_from_buffer(buf, count, , pp->value, 
pp->length);
 }
 
+static ssize_t of_node_phandle_read(struct file *filp, struct kobject *kobj,
+   struct bin_attribute *bin_attr, char *buf,
+   loff_t offset, size_t count)
+{
+   phandle phandle;
+   struct device_node *np;
+
+   np = container_of(bin_attr, struct device_node, attr_phandle);
+   phandle = cpu_to_be32(np->phandle);
+   return memory_read_from_buffer(buf, count, , ,
+  sizeof(phandle));
+}
+
 /* always return newly allocated name, caller must free after use */
 static const char *safe_name(struct kobject *kobj, const char *orig_name)
 {
@@ -164,6 +177,35 @@ int __of_add_property_sysfs(struct device_node *np, struct 
property *pp)
return rc;
 }
 
+/*
+ * In the imported device tree (fdt), phandle is a property.  In the
+ * internal data structure it is instead stored in the struct device_node.
+ * Make phandle visible in sysfs as if it was a property.
+ */
+int __of_add_phandle_sysfs(struct device_node *np)
+{
+   int rc;
+
+   if (!IS_ENABLED(CONFIG_SYSFS))
+   return 0;
+
+   if (!of_kset || !of_node_is_attached(np))
+   return 0;
+
+   if (!np->phandle || np->phandle == 0x)
+   return 0;
+
+   sysfs_bin_attr_init(>attr_phandle);
+   np->attr_phandle.attr.name = "phandle";
+   np->attr_phandle.attr.mode = 0444;
+   np->attr_phandle.size = sizeof(np->phandle);
+   np->attr_phandle.read = of_node_phandle_read;
+
+   rc = sysfs_create_bin_file(>kobj, >attr_phandle);
+   WARN(rc, "error adding attribute phandle to node %s\n", np->full_name);
+   return rc;
+}
+
 int __of_attach_node_sysfs(struct device_node *np)
 {
const char *name;
@@ -193,6 +235,8 @@ int __of_attach_node_sysfs(struct device_node *np)
if (rc)
return rc;
 
+   __of_add_phandle_sysfs(np);
+
for_each_property_of_node(np, pp)
__of_add_property_sysfs(np, pp);
 
@@ -2128,9 +2172,7 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 
align))
int id, len;
 
/* Skip those we do not want to proceed */
-   if (!strcmp(pp->name, "name") ||
-   !strcmp(pp->name, "phandle") ||
-   !strcmp(pp->name, "linux,phandle"))
+   if (!strcmp(pp->name, "name"))
continue;
 
np = of_find_node_by_path(pp->value);
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 888fdbc09992..be320082178f 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -218,19 +218,6 @@ int of_property_notify(int action, struct device_node *np,
 
 void __of_attach_node(struct device_node *np)
 {
-   const __be32 *phandle;
-   int sz;
-
-   

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

[frowand . list]

From: Frank Rowand 

Remove "phandle", "linux,phandle", and "ibm,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

- Add sysfs infrastructure to report np->phandle, as if it was a property.
- Do not create "phandle" "ibm,phandle", and "linux,phandle" properties
  in the expanded device tree.
- Remove phandle properties in of_attach_node(), for nodes dynamically
  attached to the live tree.  Add the phandle sysfs entry for these nodes.
- When creating an overlay changeset, duplicate the node phandle in
  __of_node_dup().
- Remove no longer needed checks to exclude "phandle" and "linux,phandle"
  properties in several locations.
- A side effect of these changes is that the obsolete "linux,phandle" and
  "ibm,phandle" properties will no longer appear in /proc/device-tree (they
  will appear as "phandle").
- A side effect is that the value of property "ibm,phandle" will no longer
  override the value of properties "phandle" and "linux,phandle".

Signed-off-by: Frank Rowand 
---
 drivers/of/base.c   | 48 +++---
 drivers/of/dynamic.c| 55 +
 drivers/of/fdt.c| 43 +++---
 drivers/of/of_private.h |  1 +
 drivers/of/overlay.c|  4 +---
 drivers/of/resolver.c   | 23 +
 include/linux/of.h  |  1 +
 7 files changed, 112 insertions(+), 63 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 28d5f53bc631..941c9a03471d 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -116,6 +116,19 @@ static ssize_t of_node_property_read(struct file *filp, 
struct kobject *kobj,
return memory_read_from_buffer(buf, count, , pp->value, 
pp->length);
 }
 
+static ssize_t of_node_phandle_read(struct file *filp, struct kobject *kobj,
+   struct bin_attribute *bin_attr, char *buf,
+   loff_t offset, size_t count)
+{
+   phandle phandle;
+   struct device_node *np;
+
+   np = container_of(bin_attr, struct device_node, attr_phandle);
+   phandle = cpu_to_be32(np->phandle);
+   return memory_read_from_buffer(buf, count, , ,
+  sizeof(phandle));
+}
+
 /* always return newly allocated name, caller must free after use */
 static const char *safe_name(struct kobject *kobj, const char *orig_name)
 {
@@ -164,6 +177,35 @@ int __of_add_property_sysfs(struct device_node *np, struct 
property *pp)
return rc;
 }
 
+/*
+ * In the imported device tree (fdt), phandle is a property.  In the
+ * internal data structure it is instead stored in the struct device_node.
+ * Make phandle visible in sysfs as if it was a property.
+ */
+int __of_add_phandle_sysfs(struct device_node *np)
+{
+   int rc;
+
+   if (!IS_ENABLED(CONFIG_SYSFS))
+   return 0;
+
+   if (!of_kset || !of_node_is_attached(np))
+   return 0;
+
+   if (!np->phandle || np->phandle == 0x)
+   return 0;
+
+   sysfs_bin_attr_init(>attr_phandle);
+   np->attr_phandle.attr.name = "phandle";
+   np->attr_phandle.attr.mode = 0444;
+   np->attr_phandle.size = sizeof(np->phandle);
+   np->attr_phandle.read = of_node_phandle_read;
+
+   rc = sysfs_create_bin_file(>kobj, >attr_phandle);
+   WARN(rc, "error adding attribute phandle to node %s\n", np->full_name);
+   return rc;
+}
+
 int __of_attach_node_sysfs(struct device_node *np)
 {
const char *name;
@@ -193,6 +235,8 @@ int __of_attach_node_sysfs(struct device_node *np)
if (rc)
return rc;
 
+   __of_add_phandle_sysfs(np);
+
for_each_property_of_node(np, pp)
__of_add_property_sysfs(np, pp);
 
@@ -2128,9 +2172,7 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 
align))
int id, len;
 
/* Skip those we do not want to proceed */
-   if (!strcmp(pp->name, "name") ||
-   !strcmp(pp->name, "phandle") ||
-   !strcmp(pp->name, "linux,phandle"))
+   if (!strcmp(pp->name, "name"))
continue;
 
np = of_find_node_by_path(pp->value);
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 888fdbc09992..be320082178f 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -218,19 +218,6 @@ int of_property_notify(int action, struct device_node *np,
 
 void __of_attach_node(struct device_node *np)
 {
-   const __be32 *phandle;
-   int sz;
-
-   np->name = __of_get_property(np, "name",