All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/16] ACPI graph support
@ 2017-02-02 16:22 Sakari Ailus
  2017-02-02 16:22 ` [PATCH v2 01/16] ACPI / property: Add possiblity to retrieve parent firmware node Sakari Ailus
                   ` (13 more replies)
  0 siblings, 14 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi, devicetree
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, robh, ahs3

Hello everyone, 

I posted a previous RFC labelled set of ACPI graph support a while ago: 

<URL:http://www.spinics.net/lists/linux-acpi/msg69547.html> 

Since then, the matter of how the properties should be used as in ACPI
_DSD was discussed in Ksummit and LPC, and a document detailing the rules 
was written [1].

I've additionally posted v1 which can be found here:

<URL:http://www.spinics.net/lists/linux-acpi/msg71661.html>

This set contains patches written by Mika Westerberg and by myself. The 
patchset brings support for graphs to ACPI. The functionality achieved by 
these patches is very similar to what the Device tree provides: the port
and the endpoint concept are being employed. The patches make use of the
_DSD property and data extensions to achieve this. The fwnode interface is
extended by graph functionality; this way graph information originating 
from both OF and ACPI may be accessed using the same interface, without 
being aware of the underlying firmware interface. 

The last patch of the set contains ASL documentation including an example.

The entire set may also be found here (on mediatree.git master, but it
also applies cleanly on linux-next):

<URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=acpi-graph> 

The resulting fwnode graph interface has been tested using V4L2 async with
fwnode matching and smiapp and omap3isp drivers, with appropriate changes 
to make use of the fwnode interface in drivers. 

The V4L2 patches can be found here. The fwnode graph interface is used by 
the newly added V4L2 fwnode framework which replaces the V4L2 OF
framework, with equivalent functionality. 

<URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=v4l2-acpi>

changes since v1:

- Fix a few checkpatch.pl warnings in Mika's patches (too long lines),

- remove the "endpoint" property specifying the endpoint id. The endpoint
  id is a software concept and the index in the endpoint array can be used
  instead if needed. The changes are in patches "device property: Add
  support for fwnode endpoints" and "ACPI / DSD: Document references,
  ports and endpoints" and

- add patch "irqchip/gic: Add missing forward declaration for    
  struct device" (patch 9) to fix compilation warning on arm64 caused
  by "of: No need to include linux/property.h, linux/fwnode.h is
  sufficient" (now patch 10)

changes since RFC v1:

- Rebased the set --- there were a few conflicts. 

- Fixed a bug in ACPI graph parsing. (Thanks to Mika!)

- Remove one layer (the "ports" node) of the _DSD hierarchical data 
  structure. Change the documentation accordingly. Instead, rely on the 
  presence of "port" and "endpoint" properties to identify port and 
  endpoint nodes. 

- Add a reference the DSD property rule document [1]. 


Feedback is welcome.

-- 
Kind regards,
Sakari


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 01/16] ACPI / property: Add possiblity to retrieve parent firmware node
  2017-02-02 16:22 [PATCH v2 00/16] ACPI graph support Sakari Ailus
@ 2017-02-02 16:22 ` Sakari Ailus
  2017-02-02 16:22 ` [PATCH v2 02/16] device property: Add fwnode_get_parent() Sakari Ailus
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi, devicetree
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, robh, ahs3

From: Mika Westerberg <mika.westerberg@linux.intel.com>

Sometimes it is useful to be able to navigate firmware node hierarchy
upwards toward parent nodes. ACPI device nodes are pretty much already
supported because ACPICA provides acpi_get_parent(). ACPI data nodes,
however, are all below the same parent ACPI device. Their hierarchy is
created by "linking" each other using references in the value field.

Add parent pointer to the parent data node while we create them so it is
easy to navigate the hierarchy backwards. We use this parent pointer in a
new function acpi_node_get_parent() that is able to extract parent of both
ACPI firmware node types.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/acpi/property.c | 72 ++++++++++++++++++++++++++++++++++++++-----------
 include/acpi/acpi_bus.h |  1 +
 include/linux/acpi.h    |  7 +++++
 3 files changed, 65 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 3afddcd..587c9d0 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -37,14 +37,16 @@ static const u8 ads_uuid[16] = {
 
 static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
 					   const union acpi_object *desc,
-					   struct acpi_device_data *data);
+					   struct acpi_device_data *data,
+					   struct fwnode_handle *parent);
 static bool acpi_extract_properties(const union acpi_object *desc,
 				    struct acpi_device_data *data);
 
 static bool acpi_nondev_subnode_extract(const union acpi_object *desc,
 					acpi_handle handle,
 					const union acpi_object *link,
-					struct list_head *list)
+					struct list_head *list,
+					struct fwnode_handle *parent)
 {
 	struct acpi_data_node *dn;
 	bool result;
@@ -55,6 +57,7 @@ static bool acpi_nondev_subnode_extract(const union acpi_object *desc,
 
 	dn->name = link->package.elements[0].string.pointer;
 	dn->fwnode.type = FWNODE_ACPI_DATA;
+	dn->parent = parent;
 	INIT_LIST_HEAD(&dn->data.subnodes);
 
 	result = acpi_extract_properties(desc, &dn->data);
@@ -71,9 +74,11 @@ static bool acpi_nondev_subnode_extract(const union acpi_object *desc,
 		 */
 		status = acpi_get_parent(handle, &scope);
 		if (ACPI_SUCCESS(status)
-		    && acpi_enumerate_nondev_subnodes(scope, desc, &dn->data))
+		    && acpi_enumerate_nondev_subnodes(scope, desc, &dn->data,
+						      &dn->fwnode))
 			result = true;
-	} else if (acpi_enumerate_nondev_subnodes(NULL, desc, &dn->data)) {
+	} else if (acpi_enumerate_nondev_subnodes(NULL, desc, &dn->data,
+						  &dn->fwnode)) {
 		result = true;
 	}
 
@@ -91,7 +96,8 @@ static bool acpi_nondev_subnode_extract(const union acpi_object *desc,
 
 static bool acpi_nondev_subnode_data_ok(acpi_handle handle,
 					const union acpi_object *link,
-					struct list_head *list)
+					struct list_head *list,
+					struct fwnode_handle *parent)
 {
 	struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
 	acpi_status status;
@@ -101,7 +107,8 @@ static bool acpi_nondev_subnode_data_ok(acpi_handle handle,
 	if (ACPI_FAILURE(status))
 		return false;
 
-	if (acpi_nondev_subnode_extract(buf.pointer, handle, link, list))
+	if (acpi_nondev_subnode_extract(buf.pointer, handle, link, list,
+					parent))
 		return true;
 
 	ACPI_FREE(buf.pointer);
@@ -110,7 +117,8 @@ static bool acpi_nondev_subnode_data_ok(acpi_handle handle,
 
 static bool acpi_nondev_subnode_ok(acpi_handle scope,
 				   const union acpi_object *link,
-				   struct list_head *list)
+				   struct list_head *list,
+				   struct fwnode_handle *parent)
 {
 	acpi_handle handle;
 	acpi_status status;
@@ -123,12 +131,13 @@ static bool acpi_nondev_subnode_ok(acpi_handle scope,
 	if (ACPI_FAILURE(status))
 		return false;
 
-	return acpi_nondev_subnode_data_ok(handle, link, list);
+	return acpi_nondev_subnode_data_ok(handle, link, list, parent);
 }
 
 static int acpi_add_nondev_subnodes(acpi_handle scope,
 				    const union acpi_object *links,
-				    struct list_head *list)
+				    struct list_head *list,
+				    struct fwnode_handle *parent)
 {
 	bool ret = false;
 	int i;
@@ -150,15 +159,18 @@ static int acpi_add_nondev_subnodes(acpi_handle scope,
 		/* The second one may be a string, a reference or a package. */
 		switch (link->package.elements[1].type) {
 		case ACPI_TYPE_STRING:
-			result = acpi_nondev_subnode_ok(scope, link, list);
+			result = acpi_nondev_subnode_ok(scope, link, list,
+							 parent);
 			break;
 		case ACPI_TYPE_LOCAL_REFERENCE:
 			handle = link->package.elements[1].reference.handle;
-			result = acpi_nondev_subnode_data_ok(handle, link, list);
+			result = acpi_nondev_subnode_data_ok(handle, link, list,
+							     parent);
 			break;
 		case ACPI_TYPE_PACKAGE:
 			desc = &link->package.elements[1];
-			result = acpi_nondev_subnode_extract(desc, NULL, link, list);
+			result = acpi_nondev_subnode_extract(desc, NULL, link,
+							     list, parent);
 			break;
 		default:
 			result = false;
@@ -172,7 +184,8 @@ static int acpi_add_nondev_subnodes(acpi_handle scope,
 
 static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
 					   const union acpi_object *desc,
-					   struct acpi_device_data *data)
+					   struct acpi_device_data *data,
+					   struct fwnode_handle *parent)
 {
 	int i;
 
@@ -194,7 +207,8 @@ static bool acpi_enumerate_nondev_subnodes(acpi_handle scope,
 		if (memcmp(uuid->buffer.pointer, ads_uuid, sizeof(ads_uuid)))
 			continue;
 
-		return acpi_add_nondev_subnodes(scope, links, &data->subnodes);
+		return acpi_add_nondev_subnodes(scope, links, &data->subnodes,
+						parent);
 	}
 
 	return false;
@@ -345,7 +359,8 @@ void acpi_init_properties(struct acpi_device *adev)
 		if (acpi_of)
 			acpi_init_of_compatible(adev);
 	}
-	if (acpi_enumerate_nondev_subnodes(adev->handle, buf.pointer, &adev->data))
+	if (acpi_enumerate_nondev_subnodes(adev->handle, buf.pointer,
+					&adev->data, acpi_fwnode_handle(adev)))
 		adev->data.pointer = buf.pointer;
 
 	if (!adev->data.pointer) {
@@ -920,3 +935,30 @@ struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
 	}
 	return NULL;
 }
+
+/**
+ * acpi_node_get_parent - Return parent fwnode of this fwnode
+ * @fwnode: Firmware node whose parent to get
+ *
+ * Returns parent node of an ACPI device or data firmware node or %NULL if
+ * not available.
+ */
+struct fwnode_handle *acpi_node_get_parent(struct fwnode_handle *fwnode)
+{
+	if (is_acpi_data_node(fwnode)) {
+		/* All data nodes have parent pointer so just return that */
+		return to_acpi_data_node(fwnode)->parent;
+	} else if (is_acpi_device_node(fwnode)) {
+		acpi_handle handle, parent_handle;
+
+		handle = to_acpi_device_node(fwnode)->handle;
+		if (ACPI_SUCCESS(acpi_get_parent(handle, &parent_handle))) {
+			struct acpi_device *adev;
+
+			if (!acpi_bus_get_device(parent_handle, &adev))
+				return acpi_fwnode_handle(adev);
+		}
+	}
+
+	return NULL;
+}
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 4242c31..81129e2 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -386,6 +386,7 @@ struct acpi_data_node {
 	const char *name;
 	acpi_handle handle;
 	struct fwnode_handle fwnode;
+	struct fwnode_handle *parent;
 	struct acpi_device_data data;
 	struct list_head sibling;
 	struct kobject kobj;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 5b36974..868f923 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1001,6 +1001,7 @@ int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
 
 struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
 					    struct fwnode_handle *subnode);
+struct fwnode_handle *acpi_node_get_parent(struct fwnode_handle *fwnode);
 
 struct acpi_probe_entry;
 typedef bool (*acpi_probe_entry_validate_subtbl)(struct acpi_subtable_header *,
@@ -1123,6 +1124,12 @@ static inline struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
 	return NULL;
 }
 
+static inline struct fwnode_handle *
+acpi_node_get_parent(struct fwnode_handle *fwnode)
+{
+	return NULL;
+}
+
 #define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, valid, data, fn) \
 	static const void * __acpi_table_##name[]			\
 		__attribute__((unused))					\
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 02/16] device property: Add fwnode_get_parent()
  2017-02-02 16:22 [PATCH v2 00/16] ACPI graph support Sakari Ailus
  2017-02-02 16:22 ` [PATCH v2 01/16] ACPI / property: Add possiblity to retrieve parent firmware node Sakari Ailus
@ 2017-02-02 16:22 ` Sakari Ailus
  2017-02-02 16:22 ` [PATCH v2 03/16] ACPI / property: Add fwnode_get_next_child_node() Sakari Ailus
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi, devicetree
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, robh, ahs3

From: Mika Westerberg <mika.westerberg@linux.intel.com>

Now that ACPI has support for returning parent firmware node for both types
of nodes we can expose this to others as well. This adds a new function
fwnode_get_parent() that can be used for DT and ACPI nodes to retrieve the
parent firmware node.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/base/property.c  | 25 +++++++++++++++++++++++++
 include/linux/property.h |  2 ++
 2 files changed, 27 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 43a36d6..384998a 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -867,6 +867,31 @@ int device_add_properties(struct device *dev, struct property_entry *properties)
 EXPORT_SYMBOL_GPL(device_add_properties);
 
 /**
+ * fwnode_get_parent - Return parent firwmare node
+ * @fwnode: Firmware whose parent is retrieved
+ *
+ * Return parent firmware node of the given node if possible or %NULL if no
+ * parent was available.
+ */
+struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode)
+{
+	struct fwnode_handle *parent = NULL;
+
+	if (is_of_node(fwnode)) {
+		struct device_node *node;
+
+		node = of_get_parent(to_of_node(fwnode));
+		if (node)
+			parent = &node->fwnode;
+	} else if (is_acpi_node(fwnode)) {
+		parent = acpi_node_get_parent(fwnode);
+	}
+
+	return parent;
+}
+EXPORT_SYMBOL_GPL(fwnode_get_parent);
+
+/**
  * device_get_next_child_node - Return the next child node handle for a device
  * @dev: Device to find the next child node for.
  * @child: Handle to one of the device's child nodes or a null handle.
diff --git a/include/linux/property.h b/include/linux/property.h
index 856e50b..85060bf 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -70,6 +70,8 @@ int fwnode_property_read_string(struct fwnode_handle *fwnode,
 int fwnode_property_match_string(struct fwnode_handle *fwnode,
 				 const char *propname, const char *string);
 
+struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode);
+
 struct fwnode_handle *device_get_next_child_node(struct device *dev,
 						 struct fwnode_handle *child);
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 03/16] ACPI / property: Add fwnode_get_next_child_node()
  2017-02-02 16:22 [PATCH v2 00/16] ACPI graph support Sakari Ailus
  2017-02-02 16:22 ` [PATCH v2 01/16] ACPI / property: Add possiblity to retrieve parent firmware node Sakari Ailus
  2017-02-02 16:22 ` [PATCH v2 02/16] device property: Add fwnode_get_parent() Sakari Ailus
@ 2017-02-02 16:22 ` Sakari Ailus
  2017-02-02 16:22 ` [PATCH v2 04/16] device property: Add fwnode_get_named_child_node() Sakari Ailus
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi, devicetree
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, robh, ahs3

From: Mika Westerberg <mika.westerberg@linux.intel.com>

The ACPI _DSD hierarchical data extension [1] makes it possible to have
hierarchies deeper than one level in similar way than DT allows. These
"subsubnodes" have not been accessible because device property
implementation only provides device_get_next_child_node() that is limited
to direct descendants of a device.

We need this ability in order support things like remote endpoints
currently supported in DT with of_graph_* APIs.

Modify acpi_get_next_subnode() to accept fwnode handle instead and update
callers accordingly. Also add a new function fwnode_get_next_child_node()
that works directly with fwnodes and modify device_get_next_child_node() to
call it directly. While there add a macro fwnode_for_each_child_node()
analogous to the current device_for_each_child_node() but it works with
fwnodes instead of devices.

[1] http://www.uefi.org/sites/default/files/resources/_DSD-hierarchical-data-extension-UUID-v1.pdf

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/acpi/property.c  | 27 +++++++++++++++++----------
 drivers/base/property.c  | 38 ++++++++++++++++++++++++++++++--------
 include/linux/acpi.h     |  8 ++++----
 include/linux/property.h |  6 ++++++
 4 files changed, 57 insertions(+), 22 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 587c9d0..8730ce7 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -880,21 +880,22 @@ int acpi_node_prop_read(struct fwnode_handle *fwnode,  const char *propname,
 }
 
 /**
- * acpi_get_next_subnode - Return the next child node handle for a device.
- * @dev: Device to find the next child node for.
+ * acpi_get_next_subnode - Return the next child node handle for a fwnode
+ * @fwnode: Firmware node to find the next child node for.
  * @child: Handle to one of the device's child nodes or a null handle.
  */
-struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
+struct fwnode_handle *acpi_get_next_subnode(struct fwnode_handle *fwnode,
 					    struct fwnode_handle *child)
 {
-	struct acpi_device *adev = ACPI_COMPANION(dev);
+	struct acpi_device *adev = to_acpi_device_node(fwnode);
 	struct list_head *head, *next;
 
-	if (!adev)
-		return NULL;
-
 	if (!child || child->type == FWNODE_ACPI) {
-		head = &adev->children;
+		if (adev)
+			head = &adev->children;
+		else
+			goto nondev;
+
 		if (list_empty(head))
 			goto nondev;
 
@@ -903,7 +904,6 @@ struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
 			next = adev->node.next;
 			if (next == head) {
 				child = NULL;
-				adev = ACPI_COMPANION(dev);
 				goto nondev;
 			}
 			adev = list_entry(next, struct acpi_device, node);
@@ -915,9 +915,16 @@ struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
 
  nondev:
 	if (!child || child->type == FWNODE_ACPI_DATA) {
+		struct acpi_data_node *data = to_acpi_data_node(fwnode);
 		struct acpi_data_node *dn;
 
-		head = &adev->data.subnodes;
+		if (adev)
+			head = &adev->data.subnodes;
+		else if (data)
+			head = &data->data.subnodes;
+		else
+			return NULL;
+
 		if (list_empty(head))
 			return NULL;
 
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 384998a..15403f7 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -892,24 +892,46 @@ struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode)
 EXPORT_SYMBOL_GPL(fwnode_get_parent);
 
 /**
- * device_get_next_child_node - Return the next child node handle for a device
- * @dev: Device to find the next child node for.
- * @child: Handle to one of the device's child nodes or a null handle.
+ * fwnode_get_next_child_node - Return the next child node handle for a node
+ * @fwnode: Firmware node to find the next child node for.
+ * @child: Handle to one of the node's child nodes or a %NULL handle.
  */
-struct fwnode_handle *device_get_next_child_node(struct device *dev,
+struct fwnode_handle *fwnode_get_next_child_node(struct fwnode_handle *fwnode,
 						 struct fwnode_handle *child)
 {
-	if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
+	if (is_of_node(fwnode)) {
 		struct device_node *node;
 
-		node = of_get_next_available_child(dev->of_node, to_of_node(child));
+		node = of_get_next_available_child(to_of_node(fwnode),
+						   to_of_node(child));
 		if (node)
 			return &node->fwnode;
-	} else if (IS_ENABLED(CONFIG_ACPI)) {
-		return acpi_get_next_subnode(dev, child);
+	} else if (is_acpi_node(fwnode)) {
+		return acpi_get_next_subnode(fwnode, child);
 	}
+
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(fwnode_get_next_child_node);
+
+/**
+ * device_get_next_child_node - Return the next child node handle for a device
+ * @dev: Device to find the next child node for.
+ * @child: Handle to one of the device's child nodes or a null handle.
+ */
+struct fwnode_handle *device_get_next_child_node(struct device *dev,
+						 struct fwnode_handle *child)
+{
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+	struct fwnode_handle *fwnode = NULL;
+
+	if (dev->of_node)
+		fwnode = &dev->of_node->fwnode;
+	else if (adev)
+		fwnode = acpi_fwnode_handle(adev);
+
+	return fwnode_get_next_child_node(fwnode, child);
+}
 EXPORT_SYMBOL_GPL(device_get_next_child_node);
 
 /**
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 868f923..f6f8655 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -999,8 +999,8 @@ int acpi_node_prop_read(struct fwnode_handle *fwnode, const char *propname,
 int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
 		       enum dev_prop_type proptype, void *val, size_t nval);
 
-struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
-					    struct fwnode_handle *subnode);
+struct fwnode_handle *acpi_get_next_subnode(struct fwnode_handle *fwnode,
+					    struct fwnode_handle *child);
 struct fwnode_handle *acpi_node_get_parent(struct fwnode_handle *fwnode);
 
 struct acpi_probe_entry;
@@ -1118,8 +1118,8 @@ static inline int acpi_dev_prop_read(struct acpi_device *adev,
 	return -ENXIO;
 }
 
-static inline struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
-						struct fwnode_handle *subnode)
+static inline struct fwnode_handle *
+acpi_get_next_subnode(struct fwnode_handle *fwnode, struct fwnode_handle *child)
 {
 	return NULL;
 }
diff --git a/include/linux/property.h b/include/linux/property.h
index 85060bf..a3d4f52 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -71,6 +71,12 @@ int fwnode_property_match_string(struct fwnode_handle *fwnode,
 				 const char *propname, const char *string);
 
 struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode);
+struct fwnode_handle *fwnode_get_next_child_node(struct fwnode_handle *fwnode,
+						 struct fwnode_handle *child);
+
+#define fwnode_for_each_child_node(fwnode, child)			\
+	for (child = fwnode_get_next_child_node(fwnode, NULL); child;	\
+	     child = fwnode_get_next_child_node(fwnode, child))
 
 struct fwnode_handle *device_get_next_child_node(struct device *dev,
 						 struct fwnode_handle *child);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 04/16] device property: Add fwnode_get_named_child_node()
  2017-02-02 16:22 [PATCH v2 00/16] ACPI graph support Sakari Ailus
                   ` (2 preceding siblings ...)
  2017-02-02 16:22 ` [PATCH v2 03/16] ACPI / property: Add fwnode_get_next_child_node() Sakari Ailus
@ 2017-02-02 16:22 ` Sakari Ailus
  2017-02-02 16:22 ` [PATCH v2 05/16] ACPI / property: Add support for remote endpoints Sakari Ailus
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi, devicetree
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, robh, ahs3

From: Mika Westerberg <mika.westerberg@linux.intel.com>

Since now we have means to enumerate all children of any fwnode even in
ACPI we can implement fwnode_get_named_child_node(). This is similar than
device_get_named_child_node() with the exception that it can be called to
any fwnode handle. Make device_get_named_child_node() call directly this
new function.

This is useful in cases where we need to be able to find child nodes which
are not direct descendants of the parent device.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/base/property.c  | 22 +++++++++++++++++-----
 include/linux/property.h |  2 ++
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 15403f7..c0011cf 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -935,20 +935,20 @@ struct fwnode_handle *device_get_next_child_node(struct device *dev,
 EXPORT_SYMBOL_GPL(device_get_next_child_node);
 
 /**
- * device_get_named_child_node - Return first matching named child node handle
- * @dev: Device to find the named child node for.
+ * fwnode_get_named_child_node - Return first matching named child node handle
+ * @fwnode: Firmware node to find the named child node for.
  * @childname: String to match child node name against.
  */
-struct fwnode_handle *device_get_named_child_node(struct device *dev,
+struct fwnode_handle *fwnode_get_named_child_node(struct fwnode_handle *fwnode,
 						  const char *childname)
 {
 	struct fwnode_handle *child;
 
 	/*
-	 * Find first matching named child node of this device.
+	 * Find first matching named child node of this fwnode.
 	 * For ACPI this will be a data only sub-node.
 	 */
-	device_for_each_child_node(dev, child) {
+	fwnode_for_each_child_node(fwnode, child) {
 		if (is_of_node(child)) {
 			if (!of_node_cmp(to_of_node(child)->name, childname))
 				return child;
@@ -960,6 +960,18 @@ struct fwnode_handle *device_get_named_child_node(struct device *dev,
 
 	return NULL;
 }
+EXPORT_SYMBOL_GPL(fwnode_get_named_child_node);
+
+/**
+ * device_get_named_child_node - Return first matching named child node handle
+ * @dev: Device to find the named child node for.
+ * @childname: String to match child node name against.
+ */
+struct fwnode_handle *device_get_named_child_node(struct device *dev,
+						  const char *childname)
+{
+	return fwnode_get_named_child_node(dev_fwnode(dev), childname);
+}
 EXPORT_SYMBOL_GPL(device_get_named_child_node);
 
 /**
diff --git a/include/linux/property.h b/include/linux/property.h
index a3d4f52..66530c3 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -85,6 +85,8 @@ struct fwnode_handle *device_get_next_child_node(struct device *dev,
 	for (child = device_get_next_child_node(dev, NULL); child;	\
 	     child = device_get_next_child_node(dev, child))
 
+struct fwnode_handle *fwnode_get_named_child_node(struct fwnode_handle *fwnode,
+						  const char *childname);
 struct fwnode_handle *device_get_named_child_node(struct device *dev,
 						  const char *childname);
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 05/16] ACPI / property: Add support for remote endpoints
  2017-02-02 16:22 [PATCH v2 00/16] ACPI graph support Sakari Ailus
                   ` (3 preceding siblings ...)
  2017-02-02 16:22 ` [PATCH v2 04/16] device property: Add fwnode_get_named_child_node() Sakari Ailus
@ 2017-02-02 16:22 ` Sakari Ailus
  2017-02-02 16:22 ` [PATCH v2 06/16] device " Sakari Ailus
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi, devicetree
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, robh, ahs3

From: Mika Westerberg <mika.westerberg@linux.intel.com>

DT has had concept of remote endpoints for some time already. It makes
possible to reference another firmware node through a property called
remote-endpoint. This is already used by some subsystems like v4l2 for
parsing hardware properties related to camera.

This patch adds ACPI support for remote endpoints utilizing _DSD
hierarchical data extensions.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/acpi.h    |  23 ++++++++
 2 files changed, 162 insertions(+)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 8730ce7..6e776de 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -969,3 +969,142 @@ struct fwnode_handle *acpi_node_get_parent(struct fwnode_handle *fwnode)
 
 	return NULL;
 }
+
+/**
+ * acpi_graph_get_next_endpoint - Get next endpoint ACPI firmware node
+ * @fwnode: Pointer to the parent firmware node
+ * @prev: Previous endpoint node or %NULL to get the first
+ *
+ * Looks up next endpoint ACPI firmware node below a given @fwnode. Returns
+ * %NULL if there is no next endpoint, ERR_PTR() in case of error. In case
+ * of success the next endpoint is returned.
+ */
+struct fwnode_handle *acpi_graph_get_next_endpoint(struct fwnode_handle *fwnode,
+						   struct fwnode_handle *prev)
+{
+	struct fwnode_handle *port = NULL;
+	struct fwnode_handle *endpoint;
+
+	if (!prev) {
+		do {
+			port = fwnode_get_next_child_node(fwnode, port);
+			/* Ports must have port property */
+			if (fwnode_property_present(port, "port"))
+				break;
+		} while (port);
+	} else {
+		port = fwnode_get_parent(prev);
+	}
+
+	if (!port)
+		return NULL;
+
+	endpoint = fwnode_get_next_child_node(port, prev);
+	while (!endpoint) {
+		port = fwnode_get_next_child_node(fwnode, port);
+		if (!port)
+			break;
+		if (fwnode_property_present(port, "port"))
+			endpoint = fwnode_get_next_child_node(port, NULL);
+	}
+
+	if (endpoint) {
+		/* Endpoints must have "endpoint" property */
+		if (!fwnode_property_present(endpoint, "endpoint"))
+			return ERR_PTR(-EPROTO);
+	}
+
+	return endpoint;
+}
+
+/**
+ * acpi_graph_get_child_at - Return a data extension child at given index
+ * @fwnode: parent node
+ * @at: index of the child node in the data extension package
+ *
+ * Find the child node at index of a data extension. Returns the child
+ * node on success, NULL otherwise.
+ */
+static struct fwnode_handle *acpi_graph_get_child_at(
+	struct fwnode_handle *fwnode, unsigned int at)
+{
+	struct fwnode_handle *child;
+	unsigned int i = 0;
+
+	fwnode_for_each_child_node(fwnode, child) {
+		if (i++ < at)
+			continue;
+
+		return child;
+	}
+
+	return NULL;
+}
+
+/**
+ * acpi_graph_get_remote_enpoint - Parses and returns remote end of an endpoint
+ * @fwnode: Endpoint firmware node pointing to a remote device
+ * @parent: Firmware node of remote port parent is filled here if not %NULL
+ * @port: Firmware node of remote port is filled here if not %NULL
+ * @endpoint: Firmware node of remote endpoint is filled here if not %NULL
+ *
+ * Function parses remote end of ACPI firmware remote endpoint and fills in
+ * fields requested by the caller. Returns %0 in case of success and
+ * negative errno otherwise.
+ */
+int acpi_graph_get_remote_endpoint(struct fwnode_handle *fwnode,
+				   struct fwnode_handle **parent,
+				   struct fwnode_handle **port,
+				   struct fwnode_handle **endpoint)
+{
+	unsigned int port_idx, endpoint_idx;
+	struct acpi_reference_args args;
+	int ret;
+
+	memset(&args, 0, sizeof(args));
+	ret = acpi_node_get_property_reference(fwnode, "remote-endpoint", 0,
+					       &args);
+	if (ret)
+		return ret;
+
+	/*
+	 * Always require two arguments with the reference: port and
+	 * endpoint indices.
+	 */
+	if (args.nargs != 2)
+		return -EPROTO;
+
+	fwnode = acpi_fwnode_handle(args.adev);
+	port_idx = args.args[0];
+	endpoint_idx = args.args[1];
+
+	if (parent)
+		*parent = fwnode;
+
+	if (!port && !endpoint)
+		return 0;
+
+	fwnode = acpi_graph_get_child_at(fwnode, port_idx);
+	if (!fwnode)
+		return -EPROTO;
+
+	if (!fwnode_property_present(fwnode, "port"))
+		return -EPROTO;
+
+	if (port)
+		*port = fwnode;
+
+	if (!endpoint)
+		return 0;
+
+	fwnode = acpi_graph_get_child_at(fwnode, endpoint_idx);
+	if (!fwnode)
+		return -EPROTO;
+
+	if (!fwnode_property_present(fwnode, "endpoint"))
+		return -EPROTO;
+
+	*endpoint = fwnode;
+
+	return 0;
+}
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index f6f8655..05ff957 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1003,6 +1003,13 @@ struct fwnode_handle *acpi_get_next_subnode(struct fwnode_handle *fwnode,
 					    struct fwnode_handle *child);
 struct fwnode_handle *acpi_node_get_parent(struct fwnode_handle *fwnode);
 
+struct fwnode_handle *acpi_graph_get_next_endpoint(struct fwnode_handle *fwnode,
+						   struct fwnode_handle *prev);
+int acpi_graph_get_remote_endpoint(struct fwnode_handle *fwnode,
+				   struct fwnode_handle **remote,
+				   struct fwnode_handle **port,
+				   struct fwnode_handle **endpoint);
+
 struct acpi_probe_entry;
 typedef bool (*acpi_probe_entry_validate_subtbl)(struct acpi_subtable_header *,
 						 struct acpi_probe_entry *);
@@ -1130,6 +1137,22 @@ acpi_node_get_parent(struct fwnode_handle *fwnode)
 	return NULL;
 }
 
+static inline struct fwnode_handle *
+acpi_graph_get_next_endpoint(struct fwnode_handle *fwnode,
+			     struct fwnode_handle *prev)
+{
+	return ERR_PTR(-ENXIO);
+}
+
+static inline int
+acpi_graph_get_remote_endpoint(struct fwnode_handle *fwnode,
+			       struct fwnode_handle **remote,
+			       struct fwnode_handle **port,
+			       struct fwnode_handle **endpoint)
+{
+	return -ENXIO;
+}
+
 #define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, valid, data, fn) \
 	static const void * __acpi_table_##name[]			\
 		__attribute__((unused))					\
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 06/16] device property: Add support for remote endpoints
  2017-02-02 16:22 [PATCH v2 00/16] ACPI graph support Sakari Ailus
                   ` (4 preceding siblings ...)
  2017-02-02 16:22 ` [PATCH v2 05/16] ACPI / property: Add support for remote endpoints Sakari Ailus
@ 2017-02-02 16:22 ` Sakari Ailus
       [not found] ` <1486052546-19257-1-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi, devicetree
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, robh, ahs3

From: Mika Westerberg <mika.westerberg@linux.intel.com>

This follows DT implementation of of_graph_* APIs but we call them
fwnode_graph_* instead. For DT nodes the existing of_graph_* implementation
will be used. For ACPI we use the new ACPI graph implementation instead.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/base/property.c  | 123 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/property.h |   9 ++++
 2 files changed, 132 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index c0011cf..63a30dc 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/of_graph.h>
 #include <linux/property.h>
 #include <linux/etherdevice.h>
 #include <linux/phy.h>
@@ -1111,3 +1112,125 @@ void *device_get_mac_address(struct device *dev, char *addr, int alen)
 	return device_get_mac_addr(dev, "address", addr, alen);
 }
 EXPORT_SYMBOL(device_get_mac_address);
+
+/**
+ * device_graph_get_next_endpoint - Get next endpoint firmware node
+ * @fwnode: Pointer to the parent firmware node
+ * @prev: Previous endpoint node or %NULL to get the first
+ *
+ * Returns an endpoint firmware node pointer or %NULL if no more endpoints
+ * are available.
+ */
+struct fwnode_handle *
+fwnode_graph_get_next_endpoint(struct fwnode_handle *fwnode,
+			       struct fwnode_handle *prev)
+{
+	struct fwnode_handle *endpoint = NULL;
+
+	if (is_of_node(fwnode)) {
+		struct device_node *node;
+
+		node = of_graph_get_next_endpoint(to_of_node(fwnode),
+						  to_of_node(prev));
+
+		if (node)
+			endpoint = &node->fwnode;
+	} else if (is_acpi_node(fwnode)) {
+		endpoint = acpi_graph_get_next_endpoint(fwnode, prev);
+		if (IS_ERR(endpoint))
+			endpoint = NULL;
+	}
+
+	return endpoint;
+
+}
+EXPORT_SYMBOL_GPL(fwnode_graph_get_next_endpoint);
+
+/**
+ * fwnode_graph_get_remote_port_parent - Return fwnode of a remote device
+ * @fwnode: Endpoint firmware node pointing to the remote endpoint
+ *
+ * Extracts firmware node of a remote device the @fwnode points to.
+ */
+struct fwnode_handle *
+fwnode_graph_get_remote_port_parent(struct fwnode_handle *fwnode)
+{
+	struct fwnode_handle *parent = NULL;
+
+	if (is_of_node(fwnode)) {
+		struct device_node *node;
+
+		node = of_graph_get_remote_port_parent(to_of_node(fwnode));
+		if (node)
+			parent = &node->fwnode;
+	} else if (is_acpi_node(fwnode)) {
+		int ret;
+
+		ret = acpi_graph_get_remote_endpoint(fwnode, &parent, NULL,
+						     NULL);
+		if (ret)
+			return NULL;
+	}
+
+	return parent;
+}
+EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_port_parent);
+
+/**
+ * fwnode_graph_get_remote_port - Return fwnode of a remote port
+ * @fwnode: Endpoint firmware node pointing to the remote endpoint
+ *
+ * Extracts firmware node of a remote port the @fwnode points to.
+ */
+struct fwnode_handle *fwnode_graph_get_remote_port(struct fwnode_handle *fwnode)
+{
+	struct fwnode_handle *port = NULL;
+
+	if (is_of_node(fwnode)) {
+		struct device_node *node;
+
+		node = of_graph_get_remote_port(to_of_node(fwnode));
+		if (node)
+			port = &node->fwnode;
+	} else if (is_acpi_node(fwnode)) {
+		int ret;
+
+		ret = acpi_graph_get_remote_endpoint(fwnode, NULL, &port, NULL);
+		if (ret)
+			return NULL;
+	}
+
+	return port;
+}
+EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_port);
+
+/**
+ * fwnode_graph_get_remote_endpoint - Return fwnode of a remote endpoint
+ * @fwnode: Endpoint firmware node pointing to the remote endpoint
+ *
+ * Extracts firmware node of a remote endpoint the @fwnode points to.
+ */
+struct fwnode_handle *
+fwnode_graph_get_remote_endpoint(struct fwnode_handle *fwnode)
+{
+	struct fwnode_handle *endpoint = NULL;
+
+	if (is_of_node(fwnode)) {
+		struct device_node *node;
+
+		node = of_parse_phandle(to_of_node(fwnode), "remote-endpoint",
+					0);
+		if (node)
+			endpoint = &node->fwnode;
+	} else if (is_acpi_node(fwnode)) {
+		int ret;
+
+		ret = acpi_graph_get_remote_endpoint(fwnode, NULL, NULL,
+						     &endpoint);
+		if (ret)
+			return NULL;
+	}
+
+	return endpoint;
+}
+EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_endpoint);
diff --git a/include/linux/property.h b/include/linux/property.h
index 66530c3..86c47ad 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -263,4 +263,13 @@ int device_get_phy_mode(struct device *dev);
 
 void *device_get_mac_address(struct device *dev, char *addr, int alen);
 
+struct fwnode_handle *fwnode_graph_get_next_endpoint(
+	struct fwnode_handle *fwnode, struct fwnode_handle *prev);
+struct fwnode_handle *fwnode_graph_get_remote_port_parent(
+	struct fwnode_handle *fwnode);
+struct fwnode_handle *fwnode_graph_get_remote_port(
+	struct fwnode_handle *fwnode);
+struct fwnode_handle *fwnode_graph_get_remote_endpoint(
+	struct fwnode_handle *fwnode);
+
 #endif /* _LINUX_PROPERTY_H_ */
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 07/16] device property: Add fwnode_handle_get()
       [not found] ` <1486052546-19257-1-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2017-02-02 16:22   ` Sakari Ailus
  2017-02-02 16:22   ` [PATCH v2 09/16] driver core: Arrange headers alphabetically Sakari Ailus
  2017-02-02 16:22   ` [PATCH v2 11/16] of: No need to include linux/property.h, linux/fwnode.h is sufficient Sakari Ailus
  2 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: sudeep.holla-5wv7dgnIgG8, lorenzo.pieralisi-5wv7dgnIgG8,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
	rafael-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	broonie-DgEjT+Ai2ygdnm+yROfE0A, robh-DgEjT+Ai2ygdnm+yROfE0A,
	ahs3-H+wXaHxf7aLQT0dZR+AlfA

fwnode_handle_get() is used to obtain a reference to a fwnode_handle
container. In this case this is OF specific struct device_node.

This complements fwnode_handle_put() which is already implemented.

Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/base/property.c  | 11 +++++++++++
 include/linux/property.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 63a30dc..760c6c5 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -976,6 +976,17 @@ struct fwnode_handle *device_get_named_child_node(struct device *dev,
 EXPORT_SYMBOL_GPL(device_get_named_child_node);
 
 /**
+ * fwnode_handle_get - Obtain a reference to a device node
+ * @fwnode: Pointer to the device node to obtain the reference to.
+ */
+void fwnode_handle_get(struct fwnode_handle *fwnode)
+{
+	if (is_of_node(fwnode))
+		of_node_get(to_of_node(fwnode));
+}
+EXPORT_SYMBOL_GPL(fwnode_handle_get);
+
+/**
  * fwnode_handle_put - Drop reference to a device node
  * @fwnode: Pointer to the device node to drop the reference to.
  *
diff --git a/include/linux/property.h b/include/linux/property.h
index 86c47ad..b4b1545 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -90,6 +90,7 @@ struct fwnode_handle *fwnode_get_named_child_node(struct fwnode_handle *fwnode,
 struct fwnode_handle *device_get_named_child_node(struct device *dev,
 						  const char *childname);
 
+void fwnode_handle_get(struct fwnode_handle *fwnode);
 void fwnode_handle_put(struct fwnode_handle *fwnode);
 
 unsigned int device_get_child_node_count(struct device *dev);
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 08/16] of: Add of_fwnode_handle() to convert device nodes to fwnode_handle
  2017-02-02 16:22 [PATCH v2 00/16] ACPI graph support Sakari Ailus
                   ` (6 preceding siblings ...)
       [not found] ` <1486052546-19257-1-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2017-02-02 16:22 ` Sakari Ailus
  2017-02-02 16:22 ` [PATCH v2 10/16] irqchip/gic: Add missing forward declaration for struct device Sakari Ailus
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi, devicetree
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, robh, ahs3

of_fwnode_handle() returns a struct fwnode_handle of the struct
device_node. This may be used on the fwnode property API.

Use a macro instead of a function in order to support const and non-const
arguments.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 include/linux/of.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/of.h b/include/linux/of.h
index d72f010..55eee5d 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -159,6 +159,8 @@ static inline struct device_node *to_of_node(struct fwnode_handle *fwnode)
 		container_of(fwnode, struct device_node, fwnode) : NULL;
 }
 
+#define of_fwnode_handle(node) (&(node)->fwnode)
+
 static inline bool of_have_populated_dt(void)
 {
 	return of_root != NULL;
@@ -601,6 +603,8 @@ static inline struct device_node *of_find_node_with_property(
 	return NULL;
 }
 
+#define of_fwnode_handle(node) NULL
+
 static inline bool of_have_populated_dt(void)
 {
 	return false;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 09/16] driver core: Arrange headers alphabetically
       [not found] ` <1486052546-19257-1-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
  2017-02-02 16:22   ` [PATCH v2 07/16] device property: Add fwnode_handle_get() Sakari Ailus
@ 2017-02-02 16:22   ` Sakari Ailus
  2017-02-02 16:22   ` [PATCH v2 11/16] of: No need to include linux/property.h, linux/fwnode.h is sufficient Sakari Ailus
  2 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: sudeep.holla-5wv7dgnIgG8, lorenzo.pieralisi-5wv7dgnIgG8,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
	rafael-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	broonie-DgEjT+Ai2ygdnm+yROfE0A, robh-DgEjT+Ai2ygdnm+yROfE0A,
	ahs3-H+wXaHxf7aLQT0dZR+AlfA

Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/base/core.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 020ea7f..bd87807 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -13,19 +13,19 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/fwnode.h>
+#include <linux/genhd.h>
 #include <linux/init.h>
-#include <linux/module.h>
-#include <linux/slab.h>
-#include <linux/string.h>
+#include <linux/kallsyms.h>
 #include <linux/kdev_t.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/netdevice.h>
 #include <linux/notifier.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
-#include <linux/genhd.h>
-#include <linux/kallsyms.h>
-#include <linux/mutex.h>
 #include <linux/pm_runtime.h>
-#include <linux/netdevice.h>
+#include <linux/slab.h>
+#include <linux/string.h>
 #include <linux/sysfs.h>
 
 #include "base.h"
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 10/16] irqchip/gic: Add missing forward declaration for struct device
  2017-02-02 16:22 [PATCH v2 00/16] ACPI graph support Sakari Ailus
                   ` (7 preceding siblings ...)
  2017-02-02 16:22 ` [PATCH v2 08/16] of: Add of_fwnode_handle() to convert device nodes to fwnode_handle Sakari Ailus
@ 2017-02-02 16:22 ` Sakari Ailus
  2017-02-02 16:22 ` [PATCH v2 12/16] device property: Obtain device's fwnode independently of FW type Sakari Ailus
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi, devicetree
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, robh, ahs3

A pointer to struct device is used in the header as a function argument
but there's no declaration for it. Add one.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 include/linux/irqchip/arm-gic.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
index eafc965..b16afa1 100644
--- a/include/linux/irqchip/arm-gic.h
+++ b/include/linux/irqchip/arm-gic.h
@@ -100,6 +100,7 @@
 
 #include <linux/irqdomain.h>
 
+struct device;
 struct device_node;
 struct gic_chip_data;
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 11/16] of: No need to include linux/property.h, linux/fwnode.h is sufficient
       [not found] ` <1486052546-19257-1-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
  2017-02-02 16:22   ` [PATCH v2 07/16] device property: Add fwnode_handle_get() Sakari Ailus
  2017-02-02 16:22   ` [PATCH v2 09/16] driver core: Arrange headers alphabetically Sakari Ailus
@ 2017-02-02 16:22   ` Sakari Ailus
  2017-02-20 10:37     ` [PATCH v2.1 " Sakari Ailus
  2 siblings, 1 reply; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: sudeep.holla-5wv7dgnIgG8, lorenzo.pieralisi-5wv7dgnIgG8,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
	rafael-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	broonie-DgEjT+Ai2ygdnm+yROfE0A, robh-DgEjT+Ai2ygdnm+yROfE0A,
	ahs3-H+wXaHxf7aLQT0dZR+AlfA

of.h requires a definition of struct fwnode_handle, and for that it
includes linux/property.h. struct fwnode_handle, however, is defined in
linux/fwnode.h. Include linux/fwnode.h directly.

A number of users were however depending on linux/property.h, thus fix
them by including that header directly as well.

Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/base/core.c                       | 1 +
 drivers/input/keyboard/gpio_keys.c        | 1 +
 drivers/input/misc/drv260x.c              | 1 +
 drivers/input/misc/gpio_decoder.c         | 1 +
 drivers/input/touchscreen/ad7879.c        | 1 +
 drivers/input/touchscreen/edt-ft5x06.c    | 1 +
 drivers/net/ethernet/faraday/ftgmac100.c  | 1 +
 drivers/net/ethernet/smsc/smc91x.c        | 1 +
 drivers/phy/phy-tusb1210.c                | 1 +
 drivers/power/supply/bq24735-charger.c    | 1 +
 drivers/usb/common/common.c               | 1 +
 drivers/usb/dwc2/params.c                 | 1 +
 drivers/usb/dwc2/pci.c                    | 3 ++-
 drivers/usb/dwc3/host.c                   | 1 +
 include/linux/of.h                        | 2 +-
 sound/soc/codecs/rt5514.c                 | 1 +
 sound/soc/codecs/ts3a227e.c               | 1 +
 sound/soc/mediatek/mt8173/mt8173-rt5650.c | 1 +
 sound/soc/rockchip/rk3399_gru_sound.c     | 1 +
 19 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index bd87807..ad5a4f7 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -24,6 +24,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/sysfs.h>
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 582462d..a560fad 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -20,6 +20,7 @@
 #include <linux/slab.h>
 #include <linux/sysctl.h>
 #include <linux/proc_fs.h>
+#include <linux/property.h>
 #include <linux/delay.h>
 #include <linux/platform_device.h>
 #include <linux/input.h>
diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index 0a2b865..1642219 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -22,6 +22,7 @@
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
+#include <linux/property.h>
 #include <linux/regulator/consumer.h>
 
 #include <dt-bindings/input/ti-drv260x.h>
diff --git a/drivers/input/misc/gpio_decoder.c b/drivers/input/misc/gpio_decoder.c
index ca7e0ba..a498848 100644
--- a/drivers/input/misc/gpio_decoder.c
+++ b/drivers/input/misc/gpio_decoder.c
@@ -22,6 +22,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 
 struct gpio_decoder {
 	struct input_polled_dev *poll_dev;
diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
index e16a446..38ad365 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -34,6 +34,7 @@
 #include <linux/input/touchscreen.h>
 #include <linux/platform_data/ad7879.h>
 #include <linux/module.h>
+#include <linux/property.h>
 #include "ad7879.h"
 
 #define AD7879_REG_ZEROS		0
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 28466e3..1978f8b 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -39,6 +39,7 @@
 #include <linux/input/mt.h>
 #include <linux/input/touchscreen.h>
 #include <linux/of_device.h>
+#include <linux/property.h>
 
 #define WORK_REGISTER_THRESHOLD		0x00
 #define WORK_REGISTER_REPORT_RATE	0x08
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 2625872..69f2693 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -30,6 +30,7 @@
 #include <linux/netdevice.h>
 #include <linux/phy.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <net/ip.h>
 #include <net/ncsi.h>
 
diff --git a/drivers/net/ethernet/smsc/smc91x.c b/drivers/net/ethernet/smsc/smc91x.c
index 65077c7..5352a08 100644
--- a/drivers/net/ethernet/smsc/smc91x.c
+++ b/drivers/net/ethernet/smsc/smc91x.c
@@ -75,6 +75,7 @@ static const char version[] =
 #include <linux/ioport.h>
 #include <linux/crc32.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/spinlock.h>
 #include <linux/ethtool.h>
 #include <linux/mii.h>
diff --git a/drivers/phy/phy-tusb1210.c b/drivers/phy/phy-tusb1210.c
index 4f6d5e7..f7dd21aa 100644
--- a/drivers/phy/phy-tusb1210.c
+++ b/drivers/phy/phy-tusb1210.c
@@ -12,6 +12,7 @@
 #include <linux/module.h>
 #include <linux/ulpi/driver.h>
 #include <linux/gpio/consumer.h>
+#include <linux/property.h>
 
 #include "ulpi_phy.h"
 
diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
index eb7783b..038f806 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -27,6 +27,7 @@
 #include <linux/of.h>
 #include <linux/gpio/consumer.h>
 #include <linux/power_supply.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 
 #include <linux/power/bq24735-charger.h>
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 5ef8da6..534a498 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -14,6 +14,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/property.h>
 #include <linux/usb/ch9.h>
 #include <linux/usb/of.h>
 #include <linux/usb/otg.h>
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index a786256..7e6da29 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -35,6 +35,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/property.h>
 
 #include "core.h"
 
diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c
index a23329e..abf6140 100644
--- a/drivers/usb/dwc2/pci.c
+++ b/drivers/usb/dwc2/pci.c
@@ -44,8 +44,9 @@
 #include <linux/spinlock.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
-#include <linux/slab.h>
 #include <linux/pci.h>
+#include <linux/property.h>
+#include <linux/slab.h>
 #include <linux/usb.h>
 
 #include <linux/usb/hcd.h>
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index 487f0ff..562cbde 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -16,6 +16,7 @@
  */
 
 #include <linux/platform_device.h>
+#include <linux/property.h>
 
 #include "core.h"
 
diff --git a/include/linux/of.h b/include/linux/of.h
index 55eee5d..16b2054 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -23,7 +23,7 @@
 #include <linux/spinlock.h>
 #include <linux/topology.h>
 #include <linux/notifier.h>
-#include <linux/property.h>
+#include <linux/fwnode.h>
 #include <linux/list.h>
 
 #include <asm/byteorder.h>
diff --git a/sound/soc/codecs/rt5514.c b/sound/soc/codecs/rt5514.c
index b281a46..0749219 100644
--- a/sound/soc/codecs/rt5514.c
+++ b/sound/soc/codecs/rt5514.c
@@ -15,6 +15,7 @@
 #include <linux/init.h>
 #include <linux/delay.h>
 #include <linux/pm.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/i2c.h>
 #include <linux/platform_device.h>
diff --git a/sound/soc/codecs/ts3a227e.c b/sound/soc/codecs/ts3a227e.c
index 4356843..e80b28b 100644
--- a/sound/soc/codecs/ts3a227e.c
+++ b/sound/soc/codecs/ts3a227e.c
@@ -14,6 +14,7 @@
 #include <linux/input.h>
 #include <linux/module.h>
 #include <linux/of_gpio.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 
 #include <sound/core.h>
diff --git a/sound/soc/mediatek/mt8173/mt8173-rt5650.c b/sound/soc/mediatek/mt8173/mt8173-rt5650.c
index ba65f41..433ae4f 100644
--- a/sound/soc/mediatek/mt8173/mt8173-rt5650.c
+++ b/sound/soc/mediatek/mt8173/mt8173-rt5650.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
+#include <linux/property.h>
 #include <sound/soc.h>
 #include <sound/jack.h>
 #include "../../codecs/rt5645.h"
diff --git a/sound/soc/rockchip/rk3399_gru_sound.c b/sound/soc/rockchip/rk3399_gru_sound.c
index 3475c61..9134b46 100644
--- a/sound/soc/rockchip/rk3399_gru_sound.c
+++ b/sound/soc/rockchip/rk3399_gru_sound.c
@@ -22,6 +22,7 @@
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
 #include <linux/delay.h>
+#include <linux/property.h>
 #include <linux/spi/spi.h>
 #include <linux/input.h>
 #include <sound/core.h>
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 12/16] device property: Obtain device's fwnode independently of FW type
  2017-02-02 16:22 [PATCH v2 00/16] ACPI graph support Sakari Ailus
                   ` (8 preceding siblings ...)
  2017-02-02 16:22 ` [PATCH v2 10/16] irqchip/gic: Add missing forward declaration for struct device Sakari Ailus
@ 2017-02-02 16:22 ` Sakari Ailus
  2017-02-02 16:22 ` [PATCH v2 13/16] device property: Get endpoint index from the ACPI tables Sakari Ailus
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi, devicetree
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, robh, ahs3

OF uses struct device's of_node field (of type struct device_node) to
refer to the device's OF properties. ACPI employs the fwnode field
instead. The latter is of type fwnode_handle, which is also embedded in
struct device_node.

The struct fwnode_handle pointer in both cases can be used as an argument
for the device property API which is firmware agnostic. Also make
obtaining the firmware node independent of the type of the firmware.

On ACPI, rely on the endpoint node child index in determining the endpoint
ID. There's no need for a separate "endpoint" property as such: this is a
software concept and the value is deterministically derived from the ACPI
table.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/base/property.c  | 33 +++++++++++++++++++++++++++++++++
 include/linux/fwnode.h   |  6 ++++++
 include/linux/property.h | 13 +++++++++++++
 3 files changed, 52 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 760c6c5..3f77404 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1245,3 +1245,36 @@ fwnode_graph_get_remote_endpoint(struct fwnode_handle *fwnode)
 	return endpoint;
 }
 EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_endpoint);
+
+/*
+ * fwnode_graph_parse_endpoint() - parse common endpoint node properties
+ * @node: pointer to endpoint device_node
+ * @endpoint: pointer to the fwnode endpoint data structure
+ *
+ * The caller should hold a reference to @node.
+ */
+int fwnode_graph_parse_endpoint(struct fwnode_handle *fwnode,
+				struct fwnode_endpoint *endpoint)
+{
+	struct fwnode_handle *port_fwnode = fwnode_get_parent(fwnode);
+
+	memset(endpoint, 0, sizeof(*endpoint));
+
+	endpoint->local_fwnode = fwnode;
+	/*
+	 * It doesn't matter whether the two calls below succeed.
+	 * If they don't then the default value 0 is used.
+	 */
+	if (is_acpi_node(port_fwnode)) {
+		fwnode_property_read_u32(port_fwnode, "port", &endpoint->port);
+		fwnode_property_read_u32(fwnode, "endpoint", &endpoint->id);
+	} else {
+		fwnode_property_read_u32(port_fwnode, "reg", &endpoint->port);
+		fwnode_property_read_u32(fwnode, "reg", &endpoint->id);
+	}
+
+	fwnode_handle_put(port_fwnode);
+
+	return 0;
+}
+EXPORT_SYMBOL(fwnode_graph_parse_endpoint);
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 8bd28ce..cb60f29 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -27,4 +27,10 @@ struct fwnode_handle {
 	struct fwnode_handle *secondary;
 };
 
+struct fwnode_endpoint {
+	unsigned int port;
+	unsigned int id;
+	const struct fwnode_handle *local_fwnode;
+};
+
 #endif
diff --git a/include/linux/property.h b/include/linux/property.h
index b4b1545..eb7ba73 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -13,7 +13,9 @@
 #ifndef _LINUX_PROPERTY_H_
 #define _LINUX_PROPERTY_H_
 
+#include <linux/device.h>
 #include <linux/fwnode.h>
+#include <linux/of.h>
 #include <linux/types.h>
 
 struct device;
@@ -273,4 +275,15 @@ struct fwnode_handle *fwnode_graph_get_remote_port(
 struct fwnode_handle *fwnode_graph_get_remote_endpoint(
 	struct fwnode_handle *fwnode);
 
+static inline struct fwnode_handle *device_fwnode_handle(struct device *dev)
+{
+	if (dev->of_node)
+		return of_fwnode_handle(dev->of_node);
+	else
+		return dev->fwnode;
+}
+
+int fwnode_graph_parse_endpoint(struct fwnode_handle *fwnode,
+				struct fwnode_endpoint *endpoint);
+
 #endif /* _LINUX_PROPERTY_H_ */
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 13/16] device property: Get endpoint index from the ACPI tables
  2017-02-02 16:22 [PATCH v2 00/16] ACPI graph support Sakari Ailus
                   ` (9 preceding siblings ...)
  2017-02-02 16:22 ` [PATCH v2 12/16] device property: Obtain device's fwnode independently of FW type Sakari Ailus
@ 2017-02-02 16:22 ` Sakari Ailus
  2017-02-02 16:22 ` [PATCH v2 14/16] of: Add nop implementation of of_get_next_parent() Sakari Ailus
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi, devicetree
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, robh, ahs3

On ACPI, find the child node index instead of relying on the "endpoint"
property.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/base/property.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 3f77404..28125a5 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1266,8 +1266,14 @@ int fwnode_graph_parse_endpoint(struct fwnode_handle *fwnode,
 	 * If they don't then the default value 0 is used.
 	 */
 	if (is_acpi_node(port_fwnode)) {
+		struct fwnode_handle *iter;
+
 		fwnode_property_read_u32(port_fwnode, "port", &endpoint->port);
-		fwnode_property_read_u32(fwnode, "endpoint", &endpoint->id);
+
+		for (iter = fwnode_get_next_child_node(port_fwnode, NULL);
+		     iter != fwnode;
+		     iter = fwnode_get_next_child_node(port_fwnode, iter))
+			endpoint->id++;
 	} else {
 		fwnode_property_read_u32(port_fwnode, "reg", &endpoint->port);
 		fwnode_property_read_u32(fwnode, "reg", &endpoint->id);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 14/16] of: Add nop implementation of of_get_next_parent()
  2017-02-02 16:22 [PATCH v2 00/16] ACPI graph support Sakari Ailus
                   ` (10 preceding siblings ...)
  2017-02-02 16:22 ` [PATCH v2 13/16] device property: Get endpoint index from the ACPI tables Sakari Ailus
@ 2017-02-02 16:22 ` Sakari Ailus
  2017-02-02 16:22 ` [PATCH v2 15/16] device property: Add fwnode_get_next_parent() Sakari Ailus
  2017-02-02 16:22 ` [PATCH v2 16/16] ACPI / DSD: Document references, ports and endpoints Sakari Ailus
  13 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi, devicetree
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, robh, ahs3

To avoid #ifdefs where the function is used.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 include/linux/of.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/of.h b/include/linux/of.h
index 16b2054..e0ddb22 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -585,6 +585,12 @@ static inline struct device_node *of_get_parent(const struct device_node *node)
 	return NULL;
 }
 
+static inline struct device_node *of_get_next_parent(
+	const struct device_node *node)
+{
+	return NULL;
+}
+
 static inline struct device_node *of_get_next_child(
 	const struct device_node *node, struct device_node *prev)
 {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 15/16] device property: Add fwnode_get_next_parent()
  2017-02-02 16:22 [PATCH v2 00/16] ACPI graph support Sakari Ailus
                   ` (11 preceding siblings ...)
  2017-02-02 16:22 ` [PATCH v2 14/16] of: Add nop implementation of of_get_next_parent() Sakari Ailus
@ 2017-02-02 16:22 ` Sakari Ailus
  2017-02-07 18:57   ` Al Stone
  2017-02-02 16:22 ` [PATCH v2 16/16] ACPI / DSD: Document references, ports and endpoints Sakari Ailus
  13 siblings, 1 reply; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi, devicetree
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, robh, ahs3

In order to differentiate the functionality between dropping a reference
to the node (or not) for the benefit of OF, introduce
fwnode_get_next_parent().

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/base/property.c  | 29 +++++++++++++++++++++++++++++
 include/linux/property.h |  1 +
 2 files changed, 30 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 28125a5..8e4398f 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -868,6 +868,35 @@ int device_add_properties(struct device *dev, struct property_entry *properties)
 EXPORT_SYMBOL_GPL(device_add_properties);
 
 /**
+ * fwnode_get_next_parent - Iterate to the node's parent
+ * @fwnode: Firmware whose parent is retrieved
+ *
+ * This is like fwnode_get_parent() except that it drops the refcount
+ * on the passed node, making it suitable for iterating through a
+ * node's parents.
+ *
+ * Returns a node pointer with refcount incremented, use
+ * fwnode_handle_node() on it when done.
+ */
+struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
+{
+	struct fwnode_handle *parent = NULL;
+
+	if (is_of_node(fwnode)) {
+		struct device_node *node;
+
+		node = of_get_next_parent(to_of_node(fwnode));
+		if (node)
+			parent = &node->fwnode;
+	} else if (is_acpi_node(fwnode)) {
+		parent = acpi_node_get_parent(fwnode);
+	}
+
+	return parent;
+}
+EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
+
+/**
  * fwnode_get_parent - Return parent firwmare node
  * @fwnode: Firmware whose parent is retrieved
  *
diff --git a/include/linux/property.h b/include/linux/property.h
index eb7ba73..760df98 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -73,6 +73,7 @@ int fwnode_property_match_string(struct fwnode_handle *fwnode,
 				 const char *propname, const char *string);
 
 struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode);
+struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode);
 struct fwnode_handle *fwnode_get_next_child_node(struct fwnode_handle *fwnode,
 						 struct fwnode_handle *child);
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 16/16] ACPI / DSD: Document references, ports and endpoints
  2017-02-02 16:22 [PATCH v2 00/16] ACPI graph support Sakari Ailus
                   ` (12 preceding siblings ...)
  2017-02-02 16:22 ` [PATCH v2 15/16] device property: Add fwnode_get_next_parent() Sakari Ailus
@ 2017-02-02 16:22 ` Sakari Ailus
  13 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-02 16:22 UTC (permalink / raw)
  To: linux-acpi, devicetree
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, robh, ahs3

Document the use of references into the hierarchical data extension
structure, as well as the use of port and endpoint concepts that are very
similar to those in Devicetree.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/acpi/dsd/graph.txt | 164 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 164 insertions(+)
 create mode 100644 Documentation/acpi/dsd/graph.txt

diff --git a/Documentation/acpi/dsd/graph.txt b/Documentation/acpi/dsd/graph.txt
new file mode 100644
index 0000000..6195936
--- /dev/null
+++ b/Documentation/acpi/dsd/graph.txt
@@ -0,0 +1,164 @@
+Graphs
+
+
+_DSD
+----
+
+_DSD (Device Specific Data) [7] is a predefined ACPI device
+configuration object that can be used to convey information on
+hardware features which are not specifically covered by the ACPI
+specification [1][6]. There are two _DSD extensions that are relevant
+for graphs: property [4] and hierarchical data extensions [5]. The
+property extension provides generic key-value pairs whereas the
+hierarchical data extension supports nodes with references to other
+nodes, forming a tree. The nodes in the tree may contain properties as
+defined by the property extension. The two extensions together provide
+a tree-like structure with zero or more properties (key-value pairs)
+in each node of the tree.
+
+The data structure may be accessed at runtime by using the device_*
+and fwnode_* functions defined in include/linux/fwnode.h .
+
+Fwnode represents a generic firmware node object. It is independent on
+the firmware type. In ACPI, fwnodes are _DSD hierarchical data
+extensions objects. A device's _DSD object is represented by an
+fwnode.
+
+The data structure may be referenced to elsewhere in the ACPI tables
+by using a hard reference to the device itself and an index to the
+hierarchical data extension array on each depth.
+
+
+Ports and endpoints
+-------------------
+
+The port and endpoint concepts are very similar to those in Devicetree
+[3]. The port represent represent interface in a device, and an
+endpoint represents a connection to that interface.
+
+All port nodes are located under the device's "_DSD" node in
+hierarchical data extension tree. The first package list entry of the
+hierarchical data extension related to each port node must begin with
+"port" string and must be followed by the number of the port.
+
+Further on, endpoints are located under the individual port nodes. The
+first hierarchical data extension package list entry of the endpoint
+nodes must begin with "endpoint" and must be followed by the number
+of the endpoint.
+
+Each port node contains a property extension key "port", the value of
+which is the number of the port node. The number of the endpoint node is
+the index of the endpoint node in the endpoint node array under the port
+node, starting from 0.
+
+The endpoint reference uses property extension with "remote-endpoint"
+property name followed by a reference in the same package. Such references
+consist of the name of the device, index of the port node and finally the
+index of the endpoint node. The port index is indeed index to the referred
+port array, not the number of the port that is related to numbering of
+actual hardware interfaces in the respective hardware. The endpoint nodes
+are always referred to using an index to the endpoint node array.
+Individual references thus appear as:
+
+    Package() { device, port_node_index, endpoint_node_index }
+
+The references to endpoints must be always done both ways, to the
+remote endpoint and back from the referred remote endpoint node.
+
+A simple example of this is show below:
+
+    Scope (\_SB.PCI0.I2C2)
+    {
+	Device (CAM0)
+	{
+	    Name (_DSD, Package () {
+		ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+		Package () {
+		    Package () { "compatible", Package () { "nokia,smia" } },
+		},
+		ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
+		Package () {
+		    Package () { "port0", "PRT0" },
+		}
+	    })
+	    Name (PRT0, Package() {
+		ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+		Package () {
+		    Package () { "port", 0 },
+		},
+		ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
+		Package () {
+		    Package () { "endpoint0", "EP0" },
+		}
+	    })
+	    Name (EP0, Package() {
+		ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+		Package () {
+		    Package () { "remote-endpoint", Package() { \_SB.PCI0.ISP, 0, 0 } },
+		}
+	    })
+	}
+    }
+
+    Scope (\_SB.PCI0)
+    {
+	Device (ISP)
+	{
+	    Name (_DSD, Package () {
+		ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
+		Package () {
+		    Package () { "port4", "PRT4" },
+		}
+	    })
+
+	    Name (PRT4, Package() {
+		ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+		Package () {
+		    Package () { "port", 4 }, /* CSI-2 port number */
+		},
+		ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
+		Package () {
+		    Package () { "endpoint0", "EP0" },
+		}
+	    })
+
+	    Name (EP0, Package() {
+		ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+		Package () {
+		    Package () { "remote-endpoint", Package () { \_SB.PCI0.I2C2.CAM0, 0, 0 } },
+		}
+	    })
+	}
+    }
+
+Here, the port 0 of the "CAM0" device is connected to the port 4 of
+the "ISP" device. Note that the port index in the reference is still
+0, as it refers to an entry in the table and not the port node number
+itself.
+
+
+References
+----------
+
+[1] _DSD (Device Specific Data) Implementation Guide.
+    <URL:http://www.uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel-1_1.htm>,
+    referenced 2016-10-03.
+
+[2] Devicetree. <URL:http://www.devicetree.org>, referenced 2016-10-03.
+
+[3] Documentation/devicetree/bindings/graph.txt
+
+[4] Device Properties UUID For _DSD.
+    <URL:http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf>,
+    referenced 2016-10-04.
+
+[5] Hierarchical Data Extension UUID For _DSD.
+    <URL:http://www.uefi.org/sites/default/files/resources/_DSD-hierarchical-data-extension-UUID-v1.pdf>,
+    referenced 2016-10-04.
+
+[6] Advanced Configuration and Power Interface Specification.
+    <URL:http://www.uefi.org/sites/default/files/resources/ACPI_6_1.pdf>,
+    referenced 2016-10-04.
+
+[7] _DSD Device Properties Usage Rules.
+    Documentation/acpi/DSD-properties-rules.txt
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 15/16] device property: Add fwnode_get_next_parent()
  2017-02-02 16:22 ` [PATCH v2 15/16] device property: Add fwnode_get_next_parent() Sakari Ailus
@ 2017-02-07 18:57   ` Al Stone
       [not found]     ` <9a481aec-92e6-ac01-b825-167bf73dfb11-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 28+ messages in thread
From: Al Stone @ 2017-02-07 18:57 UTC (permalink / raw)
  To: Sakari Ailus, linux-acpi, devicetree
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, robh

On 02/02/2017 09:22 AM, Sakari Ailus wrote:
> In order to differentiate the functionality between dropping a reference
> to the node (or not) for the benefit of OF, introduce
> fwnode_get_next_parent().
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/base/property.c  | 29 +++++++++++++++++++++++++++++
>  include/linux/property.h |  1 +
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 28125a5..8e4398f 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -868,6 +868,35 @@ int device_add_properties(struct device *dev, struct property_entry *properties)
>  EXPORT_SYMBOL_GPL(device_add_properties);
>  
>  /**
> + * fwnode_get_next_parent - Iterate to the node's parent
> + * @fwnode: Firmware whose parent is retrieved
> + *
> + * This is like fwnode_get_parent() except that it drops the refcount
> + * on the passed node, making it suitable for iterating through a
> + * node's parents.
> + *
> + * Returns a node pointer with refcount incremented, use
> + * fwnode_handle_node() on it when done.
> + */
> +struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
> +{
> +	struct fwnode_handle *parent = NULL;
> +
> +	if (is_of_node(fwnode)) {
> +		struct device_node *node;
> +
> +		node = of_get_next_parent(to_of_node(fwnode));
> +		if (node)
> +			parent = &node->fwnode;
> +	} else if (is_acpi_node(fwnode)) {
> +		parent = acpi_node_get_parent(fwnode);
> +	}
> +
> +	return parent;
> +}
> +EXPORT_SYMBOL_GPL(fwnode_get_next_parent);

I think I agree with Rob's prior comments about making an ops struct for DT
vs ACPI.  Out of the 16 patches, 2/16, 3/16, 5/16 (multiple times), and this
patch all end up using the same construct.  Maybe it needs to be a separate
refactoring effort, but if it's happening this often just in this patch set,
it seems like it's getting time to clean things up.

> +/**
>   * fwnode_get_parent - Return parent firwmare node
>   * @fwnode: Firmware whose parent is retrieved
>   *
> diff --git a/include/linux/property.h b/include/linux/property.h
> index eb7ba73..760df98 100644
> --- a/include/linux/property.h
> +++ b/include/linux/property.h
> @@ -73,6 +73,7 @@ int fwnode_property_match_string(struct fwnode_handle *fwnode,
>  				 const char *propname, const char *string);
>  
>  struct fwnode_handle *fwnode_get_parent(struct fwnode_handle *fwnode);
> +struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode);
>  struct fwnode_handle *fwnode_get_next_child_node(struct fwnode_handle *fwnode,
>  						 struct fwnode_handle *child);
>  
> 


-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@redhat.com
-----------------------------------

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 15/16] device property: Add fwnode_get_next_parent()
       [not found]     ` <9a481aec-92e6-ac01-b825-167bf73dfb11-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2017-02-08 11:50       ` Rafael J. Wysocki
       [not found]         ` <6293579.fEEljmGD4Z-yvgW3jdyMHm1GS7QM15AGw@public.gmane.org>
  0 siblings, 1 reply; 28+ messages in thread
From: Rafael J. Wysocki @ 2017-02-08 11:50 UTC (permalink / raw)
  To: ahs3-H+wXaHxf7aLQT0dZR+AlfA, Sakari Ailus
  Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, sudeep.holla-5wv7dgnIgG8,
	lorenzo.pieralisi-5wv7dgnIgG8,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
	rafael-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	broonie-DgEjT+Ai2ygdnm+yROfE0A, robh-DgEjT+Ai2ygdnm+yROfE0A

On Tuesday, February 07, 2017 11:57:55 AM Al Stone wrote:
> On 02/02/2017 09:22 AM, Sakari Ailus wrote:
> > In order to differentiate the functionality between dropping a reference
> > to the node (or not) for the benefit of OF, introduce
> > fwnode_get_next_parent().
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > ---
> >  drivers/base/property.c  | 29 +++++++++++++++++++++++++++++
> >  include/linux/property.h |  1 +
> >  2 files changed, 30 insertions(+)
> > 
> > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > index 28125a5..8e4398f 100644
> > --- a/drivers/base/property.c
> > +++ b/drivers/base/property.c
> > @@ -868,6 +868,35 @@ int device_add_properties(struct device *dev, struct property_entry *properties)
> >  EXPORT_SYMBOL_GPL(device_add_properties);
> >  
> >  /**
> > + * fwnode_get_next_parent - Iterate to the node's parent
> > + * @fwnode: Firmware whose parent is retrieved
> > + *
> > + * This is like fwnode_get_parent() except that it drops the refcount
> > + * on the passed node, making it suitable for iterating through a
> > + * node's parents.
> > + *
> > + * Returns a node pointer with refcount incremented, use
> > + * fwnode_handle_node() on it when done.
> > + */
> > +struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
> > +{
> > +	struct fwnode_handle *parent = NULL;
> > +
> > +	if (is_of_node(fwnode)) {
> > +		struct device_node *node;
> > +
> > +		node = of_get_next_parent(to_of_node(fwnode));
> > +		if (node)
> > +			parent = &node->fwnode;
> > +	} else if (is_acpi_node(fwnode)) {
> > +		parent = acpi_node_get_parent(fwnode);
> > +	}
> > +
> > +	return parent;
> > +}
> > +EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
> 
> I think I agree with Rob's prior comments about making an ops struct for DT
> vs ACPI.  Out of the 16 patches, 2/16, 3/16, 5/16 (multiple times), and this
> patch all end up using the same construct.  Maybe it needs to be a separate
> refactoring effort, but if it's happening this often just in this patch set,
> it seems like it's getting time to clean things up.

As long as there are two cases only (ACPI vs DT), an ops struct wouldn't
really make things simpler and it would make the code more difficult to
follow.

But we do have a third case (static or built-in properties) and it doesn't
seem to be covered at all.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 15/16] device property: Add fwnode_get_next_parent()
       [not found]         ` <6293579.fEEljmGD4Z-yvgW3jdyMHm1GS7QM15AGw@public.gmane.org>
@ 2017-02-08 12:19           ` Rafael J. Wysocki
       [not found]             ` <7605668.5ZxNsBFnEG-yvgW3jdyMHm1GS7QM15AGw@public.gmane.org>
  2017-02-21  8:17             ` Sakari Ailus
  0 siblings, 2 replies; 28+ messages in thread
From: Rafael J. Wysocki @ 2017-02-08 12:19 UTC (permalink / raw)
  To: ahs3-H+wXaHxf7aLQT0dZR+AlfA, Sakari Ailus,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA
  Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, sudeep.holla-5wv7dgnIgG8,
	lorenzo.pieralisi-5wv7dgnIgG8, rafael-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	robh-DgEjT+Ai2ygdnm+yROfE0A

On Wednesday, February 08, 2017 12:50:25 PM Rafael J. Wysocki wrote:
> On Tuesday, February 07, 2017 11:57:55 AM Al Stone wrote:
> > On 02/02/2017 09:22 AM, Sakari Ailus wrote:
> > > In order to differentiate the functionality between dropping a reference
> > > to the node (or not) for the benefit of OF, introduce
> > > fwnode_get_next_parent().
> > > 
> > > Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > > ---
> > >  drivers/base/property.c  | 29 +++++++++++++++++++++++++++++
> > >  include/linux/property.h |  1 +
> > >  2 files changed, 30 insertions(+)
> > > 
> > > diff --git a/drivers/base/property.c b/drivers/base/property.c
> > > index 28125a5..8e4398f 100644
> > > --- a/drivers/base/property.c
> > > +++ b/drivers/base/property.c
> > > @@ -868,6 +868,35 @@ int device_add_properties(struct device *dev, struct property_entry *properties)
> > >  EXPORT_SYMBOL_GPL(device_add_properties);
> > >  
> > >  /**
> > > + * fwnode_get_next_parent - Iterate to the node's parent
> > > + * @fwnode: Firmware whose parent is retrieved
> > > + *
> > > + * This is like fwnode_get_parent() except that it drops the refcount
> > > + * on the passed node, making it suitable for iterating through a
> > > + * node's parents.
> > > + *
> > > + * Returns a node pointer with refcount incremented, use
> > > + * fwnode_handle_node() on it when done.
> > > + */
> > > +struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
> > > +{
> > > +	struct fwnode_handle *parent = NULL;
> > > +
> > > +	if (is_of_node(fwnode)) {
> > > +		struct device_node *node;
> > > +
> > > +		node = of_get_next_parent(to_of_node(fwnode));
> > > +		if (node)
> > > +			parent = &node->fwnode;
> > > +	} else if (is_acpi_node(fwnode)) {
> > > +		parent = acpi_node_get_parent(fwnode);
> > > +	}
> > > +
> > > +	return parent;
> > > +}
> > > +EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
> > 
> > I think I agree with Rob's prior comments about making an ops struct for DT
> > vs ACPI.  Out of the 16 patches, 2/16, 3/16, 5/16 (multiple times), and this
> > patch all end up using the same construct.  Maybe it needs to be a separate
> > refactoring effort, but if it's happening this often just in this patch set,
> > it seems like it's getting time to clean things up.
> 
> As long as there are two cases only (ACPI vs DT), an ops struct wouldn't
> really make things simpler and it would make the code more difficult to
> follow.
> 
> But we do have a third case (static or built-in properties) and it doesn't
> seem to be covered at all.

That said the ops struct could be introduced on top of this series just fine.
It even might be cleaner to do it this way, so I'm not asking for a redesign
here.

I'd like the built-in properties to be covered too, however.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 15/16] device property: Add fwnode_get_next_parent()
       [not found]             ` <7605668.5ZxNsBFnEG-yvgW3jdyMHm1GS7QM15AGw@public.gmane.org>
@ 2017-02-08 16:49               ` Al Stone
  2017-02-14  8:01                 ` Sakari Ailus
  0 siblings, 1 reply; 28+ messages in thread
From: Al Stone @ 2017-02-08 16:49 UTC (permalink / raw)
  To: Rafael J. Wysocki, Sakari Ailus, mika.westerberg-VuQAYsv1563Yd54FQh9/CA
  Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, sudeep.holla-5wv7dgnIgG8,
	lorenzo.pieralisi-5wv7dgnIgG8, rafael-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, broonie-DgEjT+Ai2ygdnm+yROfE0A,
	robh-DgEjT+Ai2ygdnm+yROfE0A

On 02/08/2017 05:19 AM, Rafael J. Wysocki wrote:
> On Wednesday, February 08, 2017 12:50:25 PM Rafael J. Wysocki wrote:
>> On Tuesday, February 07, 2017 11:57:55 AM Al Stone wrote:
>>> On 02/02/2017 09:22 AM, Sakari Ailus wrote:
>>>> In order to differentiate the functionality between dropping a reference
>>>> to the node (or not) for the benefit of OF, introduce
>>>> fwnode_get_next_parent().
>>>>
>>>> Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>>>> ---
>>>>  drivers/base/property.c  | 29 +++++++++++++++++++++++++++++
>>>>  include/linux/property.h |  1 +
>>>>  2 files changed, 30 insertions(+)
>>>>
>>>> diff --git a/drivers/base/property.c b/drivers/base/property.c
>>>> index 28125a5..8e4398f 100644
>>>> --- a/drivers/base/property.c
>>>> +++ b/drivers/base/property.c
>>>> @@ -868,6 +868,35 @@ int device_add_properties(struct device *dev, struct property_entry *properties)
>>>>  EXPORT_SYMBOL_GPL(device_add_properties);
>>>>  
>>>>  /**
>>>> + * fwnode_get_next_parent - Iterate to the node's parent
>>>> + * @fwnode: Firmware whose parent is retrieved
>>>> + *
>>>> + * This is like fwnode_get_parent() except that it drops the refcount
>>>> + * on the passed node, making it suitable for iterating through a
>>>> + * node's parents.
>>>> + *
>>>> + * Returns a node pointer with refcount incremented, use
>>>> + * fwnode_handle_node() on it when done.
>>>> + */
>>>> +struct fwnode_handle *fwnode_get_next_parent(struct fwnode_handle *fwnode)
>>>> +{
>>>> +	struct fwnode_handle *parent = NULL;
>>>> +
>>>> +	if (is_of_node(fwnode)) {
>>>> +		struct device_node *node;
>>>> +
>>>> +		node = of_get_next_parent(to_of_node(fwnode));
>>>> +		if (node)
>>>> +			parent = &node->fwnode;
>>>> +	} else if (is_acpi_node(fwnode)) {
>>>> +		parent = acpi_node_get_parent(fwnode);
>>>> +	}
>>>> +
>>>> +	return parent;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
>>>
>>> I think I agree with Rob's prior comments about making an ops struct for DT
>>> vs ACPI.  Out of the 16 patches, 2/16, 3/16, 5/16 (multiple times), and this
>>> patch all end up using the same construct.  Maybe it needs to be a separate
>>> refactoring effort, but if it's happening this often just in this patch set,
>>> it seems like it's getting time to clean things up.
>>
>> As long as there are two cases only (ACPI vs DT), an ops struct wouldn't
>> really make things simpler and it would make the code more difficult to
>> follow.
>>
>> But we do have a third case (static or built-in properties) and it doesn't
>> seem to be covered at all.
> 
> That said the ops struct could be introduced on top of this series just fine.
> It even might be cleaner to do it this way, so I'm not asking for a redesign
> here.

Right.  I don't think it necessarily needs to be covered in this series, but it
sure does feel like the time has come.  The number of "if (dt) then x else if
(acpi) then y;" really stood out for me in this series.

> I'd like the built-in properties to be covered too, however.

Yeah, good point.  I was focused on ACPI and DT, and forgot about those.

Not sure if I have time, but I can start looking into this.

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
-----------------------------------
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 15/16] device property: Add fwnode_get_next_parent()
  2017-02-08 16:49               ` Al Stone
@ 2017-02-14  8:01                 ` Sakari Ailus
  2017-02-14 17:09                   ` Al Stone
  0 siblings, 1 reply; 28+ messages in thread
From: Sakari Ailus @ 2017-02-14  8:01 UTC (permalink / raw)
  To: ahs3, Rafael J. Wysocki, mika.westerberg
  Cc: linux-acpi, devicetree, sudeep.holla, lorenzo.pieralisi, rafael,
	mark.rutland, broonie, robh

Hi Al,

> Not sure if I have time, but I can start looking into this.

Let's see who gets there first. :-)

I'm quite busy in the coming few months but after that I may have time
for this. Please let me know if you're starting to work on that so we
don't end up doing duplicated work; I'll do the same.

Thanks.

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 15/16] device property: Add fwnode_get_next_parent()
  2017-02-14  8:01                 ` Sakari Ailus
@ 2017-02-14 17:09                   ` Al Stone
  0 siblings, 0 replies; 28+ messages in thread
From: Al Stone @ 2017-02-14 17:09 UTC (permalink / raw)
  To: Sakari Ailus, Rafael J. Wysocki, mika.westerberg
  Cc: linux-acpi, devicetree, sudeep.holla, lorenzo.pieralisi, rafael,
	mark.rutland, broonie, robh

On 02/14/2017 01:01 AM, Sakari Ailus wrote:
> Hi Al,
> 
>> Not sure if I have time, but I can start looking into this.
> 
> Let's see who gets there first. :-)
> 
> I'm quite busy in the coming few months but after that I may have time
> for this. Please let me know if you're starting to work on that so we
> don't end up doing duplicated work; I'll do the same.
> 
> Thanks.
> 

Good plan :-).  Yeah, I'm in the same situation.  If I suddenly get
bored or all my day job tasks magically disappear, I can get to it
sooner :).

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@redhat.com
-----------------------------------

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2.1 11/16] of: No need to include linux/property.h, linux/fwnode.h is sufficient
  2017-02-02 16:22   ` [PATCH v2 11/16] of: No need to include linux/property.h, linux/fwnode.h is sufficient Sakari Ailus
@ 2017-02-20 10:37     ` Sakari Ailus
  0 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-20 10:37 UTC (permalink / raw)
  To: linux-acpi, devicetree
  Cc: sudeep.holla, lorenzo.pieralisi, mika.westerberg, rafael,
	mark.rutland, broonie, robh, ahs3

of.h requires a definition of struct fwnode_handle, and for that it
includes linux/property.h. struct fwnode_handle, however, is defined in
linux/fwnode.h. Include linux/fwnode.h directly.

A number of users were however depending on linux/property.h, thus fix
them by including that header directly as well.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
since v2:

Add changes required for device property API usage in tsc200x-core.c and
dwc2/debugfs.c . Tested on linux-next and linux-pm.

 drivers/base/core.c                       | 1 +
 drivers/input/keyboard/gpio_keys.c        | 1 +
 drivers/input/misc/drv260x.c              | 1 +
 drivers/input/misc/gpio_decoder.c         | 1 +
 drivers/input/touchscreen/ad7879.c        | 1 +
 drivers/input/touchscreen/edt-ft5x06.c    | 1 +
 drivers/input/touchscreen/tsc200x-core.c  | 2 +-
 drivers/net/ethernet/faraday/ftgmac100.c  | 1 +
 drivers/net/ethernet/smsc/smc91x.c        | 1 +
 drivers/phy/phy-tusb1210.c                | 1 +
 drivers/power/supply/bq24735-charger.c    | 1 +
 drivers/usb/common/common.c               | 1 +
 drivers/usb/dwc2/debugfs.c                | 3 ++-
 drivers/usb/dwc2/params.c                 | 1 +
 drivers/usb/dwc2/pci.c                    | 3 ++-
 drivers/usb/dwc3/host.c                   | 1 +
 include/linux/of.h                        | 2 +-
 sound/soc/codecs/rt5514.c                 | 1 +
 sound/soc/codecs/ts3a227e.c               | 1 +
 sound/soc/mediatek/mt8173/mt8173-rt5650.c | 1 +
 sound/soc/rockchip/rk3399_gru_sound.c     | 1 +
 21 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index bd87807..ad5a4f7 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -24,6 +24,7 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/sysfs.h>
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 582462d..a560fad 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -20,6 +20,7 @@
 #include <linux/slab.h>
 #include <linux/sysctl.h>
 #include <linux/proc_fs.h>
+#include <linux/property.h>
 #include <linux/delay.h>
 #include <linux/platform_device.h>
 #include <linux/input.h>
diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index 0a2b865..1642219 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -22,6 +22,7 @@
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
+#include <linux/property.h>
 #include <linux/regulator/consumer.h>
 
 #include <dt-bindings/input/ti-drv260x.h>
diff --git a/drivers/input/misc/gpio_decoder.c b/drivers/input/misc/gpio_decoder.c
index ca7e0ba..a498848 100644
--- a/drivers/input/misc/gpio_decoder.c
+++ b/drivers/input/misc/gpio_decoder.c
@@ -22,6 +22,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 
 struct gpio_decoder {
 	struct input_polled_dev *poll_dev;
diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
index e16a446..38ad365 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -34,6 +34,7 @@
 #include <linux/input/touchscreen.h>
 #include <linux/platform_data/ad7879.h>
 #include <linux/module.h>
+#include <linux/property.h>
 #include "ad7879.h"
 
 #define AD7879_REG_ZEROS		0
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 28466e3..1978f8b 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -39,6 +39,7 @@
 #include <linux/input/mt.h>
 #include <linux/input/touchscreen.h>
 #include <linux/of_device.h>
+#include <linux/property.h>
 
 #define WORK_REGISTER_THRESHOLD		0x00
 #define WORK_REGISTER_REPORT_RATE	0x08
diff --git a/drivers/input/touchscreen/tsc200x-core.c b/drivers/input/touchscreen/tsc200x-core.c
index b7059ed..8849f51 100644
--- a/drivers/input/touchscreen/tsc200x-core.c
+++ b/drivers/input/touchscreen/tsc200x-core.c
@@ -25,11 +25,11 @@
 #include <linux/input/touchscreen.h>
 #include <linux/interrupt.h>
 #include <linux/delay.h>
-#include <linux/pm.h>
 #include <linux/of.h>
 #include <linux/spi/tsc2005.h>
 #include <linux/regulator/consumer.h>
 #include <linux/regmap.h>
+#include <linux/property.h>
 #include <linux/gpio/consumer.h>
 #include "tsc200x-core.h"
 
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 2625872..69f2693 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -30,6 +30,7 @@
 #include <linux/netdevice.h>
 #include <linux/phy.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <net/ip.h>
 #include <net/ncsi.h>
 
diff --git a/drivers/net/ethernet/smsc/smc91x.c b/drivers/net/ethernet/smsc/smc91x.c
index 65077c7..5352a08 100644
--- a/drivers/net/ethernet/smsc/smc91x.c
+++ b/drivers/net/ethernet/smsc/smc91x.c
@@ -75,6 +75,7 @@ static const char version[] =
 #include <linux/ioport.h>
 #include <linux/crc32.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/spinlock.h>
 #include <linux/ethtool.h>
 #include <linux/mii.h>
diff --git a/drivers/phy/phy-tusb1210.c b/drivers/phy/phy-tusb1210.c
index 4f6d5e7..f7dd21aa 100644
--- a/drivers/phy/phy-tusb1210.c
+++ b/drivers/phy/phy-tusb1210.c
@@ -12,6 +12,7 @@
 #include <linux/module.h>
 #include <linux/ulpi/driver.h>
 #include <linux/gpio/consumer.h>
+#include <linux/property.h>
 
 #include "ulpi_phy.h"
 
diff --git a/drivers/power/supply/bq24735-charger.c b/drivers/power/supply/bq24735-charger.c
index eb7783b..038f806 100644
--- a/drivers/power/supply/bq24735-charger.c
+++ b/drivers/power/supply/bq24735-charger.c
@@ -27,6 +27,7 @@
 #include <linux/of.h>
 #include <linux/gpio/consumer.h>
 #include <linux/power_supply.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 
 #include <linux/power/bq24735-charger.h>
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 5ef8da6..534a498 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -14,6 +14,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/property.h>
 #include <linux/usb/ch9.h>
 #include <linux/usb/of.h>
 #include <linux/usb/otg.h>
diff --git a/drivers/usb/dwc2/debugfs.c b/drivers/usb/dwc2/debugfs.c
index 0a13091..86736e4 100644
--- a/drivers/usb/dwc2/debugfs.c
+++ b/drivers/usb/dwc2/debugfs.c
@@ -14,9 +14,10 @@
  * GNU General Public License for more details.
  */
 
-#include <linux/spinlock.h>
 #include <linux/debugfs.h>
+#include <linux/property.h>
 #include <linux/seq_file.h>
+#include <linux/spinlock.h>
 #include <linux/uaccess.h>
 
 #include "core.h"
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index a786256..7e6da29 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -35,6 +35,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/property.h>
 
 #include "core.h"
 
diff --git a/drivers/usb/dwc2/pci.c b/drivers/usb/dwc2/pci.c
index a23329e..abf6140 100644
--- a/drivers/usb/dwc2/pci.c
+++ b/drivers/usb/dwc2/pci.c
@@ -44,8 +44,9 @@
 #include <linux/spinlock.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
-#include <linux/slab.h>
 #include <linux/pci.h>
+#include <linux/property.h>
+#include <linux/slab.h>
 #include <linux/usb.h>
 
 #include <linux/usb/hcd.h>
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index 487f0ff..562cbde 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -16,6 +16,7 @@
  */
 
 #include <linux/platform_device.h>
+#include <linux/property.h>
 
 #include "core.h"
 
diff --git a/include/linux/of.h b/include/linux/of.h
index 55eee5d..16b2054 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -23,7 +23,7 @@
 #include <linux/spinlock.h>
 #include <linux/topology.h>
 #include <linux/notifier.h>
-#include <linux/property.h>
+#include <linux/fwnode.h>
 #include <linux/list.h>
 
 #include <asm/byteorder.h>
diff --git a/sound/soc/codecs/rt5514.c b/sound/soc/codecs/rt5514.c
index b281a46..0749219 100644
--- a/sound/soc/codecs/rt5514.c
+++ b/sound/soc/codecs/rt5514.c
@@ -15,6 +15,7 @@
 #include <linux/init.h>
 #include <linux/delay.h>
 #include <linux/pm.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/i2c.h>
 #include <linux/platform_device.h>
diff --git a/sound/soc/codecs/ts3a227e.c b/sound/soc/codecs/ts3a227e.c
index 4356843..e80b28b 100644
--- a/sound/soc/codecs/ts3a227e.c
+++ b/sound/soc/codecs/ts3a227e.c
@@ -14,6 +14,7 @@
 #include <linux/input.h>
 #include <linux/module.h>
 #include <linux/of_gpio.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 
 #include <sound/core.h>
diff --git a/sound/soc/mediatek/mt8173/mt8173-rt5650.c b/sound/soc/mediatek/mt8173/mt8173-rt5650.c
index ba65f41..433ae4f 100644
--- a/sound/soc/mediatek/mt8173/mt8173-rt5650.c
+++ b/sound/soc/mediatek/mt8173/mt8173-rt5650.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
+#include <linux/property.h>
 #include <sound/soc.h>
 #include <sound/jack.h>
 #include "../../codecs/rt5645.h"
diff --git a/sound/soc/rockchip/rk3399_gru_sound.c b/sound/soc/rockchip/rk3399_gru_sound.c
index 3475c61..9134b46 100644
--- a/sound/soc/rockchip/rk3399_gru_sound.c
+++ b/sound/soc/rockchip/rk3399_gru_sound.c
@@ -22,6 +22,7 @@
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
 #include <linux/delay.h>
+#include <linux/property.h>
 #include <linux/spi/spi.h>
 #include <linux/input.h>
 #include <sound/core.h>
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 15/16] device property: Add fwnode_get_next_parent()
  2017-02-08 12:19           ` Rafael J. Wysocki
       [not found]             ` <7605668.5ZxNsBFnEG-yvgW3jdyMHm1GS7QM15AGw@public.gmane.org>
@ 2017-02-21  8:17             ` Sakari Ailus
       [not found]               ` <20170221081749.GC16975-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
  2017-02-23 17:01               ` Sakari Ailus
  1 sibling, 2 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-21  8:17 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: ahs3, Sakari Ailus, mika.westerberg, linux-acpi, devicetree,
	sudeep.holla, lorenzo.pieralisi, rafael, mark.rutland, broonie,
	robh

Hi Rafael,

On Wed, Feb 08, 2017 at 01:19:57PM +0100, Rafael J. Wysocki wrote:
...
> > > I think I agree with Rob's prior comments about making an ops struct for DT
> > > vs ACPI.  Out of the 16 patches, 2/16, 3/16, 5/16 (multiple times), and this
> > > patch all end up using the same construct.  Maybe it needs to be a separate
> > > refactoring effort, but if it's happening this often just in this patch set,
> > > it seems like it's getting time to clean things up.
> > 
> > As long as there are two cases only (ACPI vs DT), an ops struct wouldn't
> > really make things simpler and it would make the code more difficult to
> > follow.
> > 
> > But we do have a third case (static or built-in properties) and it doesn't
> > seem to be covered at all.
> 
> That said the ops struct could be introduced on top of this series just fine.
> It even might be cleaner to do it this way, so I'm not asking for a redesign
> here.
> 
> I'd like the built-in properties to be covered too, however.

That sounds good to me. 

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 15/16] device property: Add fwnode_get_next_parent()
       [not found]               ` <20170221081749.GC16975-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
@ 2017-02-21 23:00                 ` Rafael J. Wysocki
  2017-02-22 13:12                   ` Sakari Ailus
  0 siblings, 1 reply; 28+ messages in thread
From: Rafael J. Wysocki @ 2017-02-21 23:00 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Rafael J. Wysocki, Al Stone, Sakari Ailus, Mika Westerberg,
	ACPI Devel Maling List, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Sudeep Holla, Lorenzo Pieralisi, Rafael J. Wysocki, Mark Rutland,
	Mark Brown, Rob Herring

On Tue, Feb 21, 2017 at 9:17 AM, Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org> wrote:
> Hi Rafael,
>
> On Wed, Feb 08, 2017 at 01:19:57PM +0100, Rafael J. Wysocki wrote:
> ...
>> > > I think I agree with Rob's prior comments about making an ops struct for DT
>> > > vs ACPI.  Out of the 16 patches, 2/16, 3/16, 5/16 (multiple times), and this
>> > > patch all end up using the same construct.  Maybe it needs to be a separate
>> > > refactoring effort, but if it's happening this often just in this patch set,
>> > > it seems like it's getting time to clean things up.
>> >
>> > As long as there are two cases only (ACPI vs DT), an ops struct wouldn't
>> > really make things simpler and it would make the code more difficult to
>> > follow.
>> >
>> > But we do have a third case (static or built-in properties) and it doesn't
>> > seem to be covered at all.
>>
>> That said the ops struct could be introduced on top of this series just fine.
>> It even might be cleaner to do it this way, so I'm not asking for a redesign
>> here.
>>
>> I'd like the built-in properties to be covered too, however.
>
> That sounds good to me.

OK

Please update the patches to built-in properties into account, then. :-)

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 15/16] device property: Add fwnode_get_next_parent()
  2017-02-21 23:00                 ` Rafael J. Wysocki
@ 2017-02-22 13:12                   ` Sakari Ailus
  0 siblings, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-22 13:12 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J. Wysocki, Al Stone, Sakari Ailus, Mika Westerberg,
	ACPI Devel Maling List, devicetree, Sudeep Holla,
	Lorenzo Pieralisi, Mark Rutland, Mark Brown, Rob Herring

Hi Rafael,

On Wed, Feb 22, 2017 at 12:00:07AM +0100, Rafael J. Wysocki wrote:
> On Tue, Feb 21, 2017 at 9:17 AM, Sakari Ailus <sakari.ailus@iki.fi> wrote:
> > Hi Rafael,
> >
> > On Wed, Feb 08, 2017 at 01:19:57PM +0100, Rafael J. Wysocki wrote:
> > ...
> >> > > I think I agree with Rob's prior comments about making an ops struct for DT
> >> > > vs ACPI.  Out of the 16 patches, 2/16, 3/16, 5/16 (multiple times), and this
> >> > > patch all end up using the same construct.  Maybe it needs to be a separate
> >> > > refactoring effort, but if it's happening this often just in this patch set,
> >> > > it seems like it's getting time to clean things up.
> >> >
> >> > As long as there are two cases only (ACPI vs DT), an ops struct wouldn't
> >> > really make things simpler and it would make the code more difficult to
> >> > follow.
> >> >
> >> > But we do have a third case (static or built-in properties) and it doesn't
> >> > seem to be covered at all.
> >>
> >> That said the ops struct could be introduced on top of this series just fine.
> >> It even might be cleaner to do it this way, so I'm not asking for a redesign
> >> here.
> >>
> >> I'd like the built-in properties to be covered too, however.
> >
> > That sounds good to me.
> 
> OK
> 
> Please update the patches to built-in properties into account, then. :-)

Will do.

I also noticed that I had accidentally merged two patches from RFC v1 to
PATCH v1 sets (device property: Obtain device's fwnode independently of FW
type). There's a redundant patch adding device_fwnode_handle() function as
well, I'll drop that patch (the same subject).

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 15/16] device property: Add fwnode_get_next_parent()
  2017-02-21  8:17             ` Sakari Ailus
       [not found]               ` <20170221081749.GC16975-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
@ 2017-02-23 17:01               ` Sakari Ailus
  1 sibling, 0 replies; 28+ messages in thread
From: Sakari Ailus @ 2017-02-23 17:01 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: ahs3, Sakari Ailus, mika.westerberg, linux-acpi, devicetree,
	sudeep.holla, lorenzo.pieralisi, rafael, mark.rutland, broonie,
	robh

On Tue, Feb 21, 2017 at 10:17:49AM +0200, Sakari Ailus wrote:
> Hi Rafael,
> 
> On Wed, Feb 08, 2017 at 01:19:57PM +0100, Rafael J. Wysocki wrote:
> ...
> > > > I think I agree with Rob's prior comments about making an ops struct for DT
> > > > vs ACPI.  Out of the 16 patches, 2/16, 3/16, 5/16 (multiple times), and this
> > > > patch all end up using the same construct.  Maybe it needs to be a separate
> > > > refactoring effort, but if it's happening this often just in this patch set,
> > > > it seems like it's getting time to clean things up.
> > > 
> > > As long as there are two cases only (ACPI vs DT), an ops struct wouldn't
> > > really make things simpler and it would make the code more difficult to
> > > follow.
> > > 
> > > But we do have a third case (static or built-in properties) and it doesn't
> > > seem to be covered at all.
> > 
> > That said the ops struct could be introduced on top of this series just fine.
> > It even might be cleaner to do it this way, so I'm not asking for a redesign
> > here.
> > 
> > I'd like the built-in properties to be covered too, however.
> 
> That sounds good to me. 

After looking at the implementation, the fwnode interface is rather complete
for property sets --- graphs are inherently cross-device concepts and
property sets don't really lend themselves to that. I think it'd be good to
have at least a use case for that before implementing it based on a thought
that it might be needed for something.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2017-02-23 17:08 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-02 16:22 [PATCH v2 00/16] ACPI graph support Sakari Ailus
2017-02-02 16:22 ` [PATCH v2 01/16] ACPI / property: Add possiblity to retrieve parent firmware node Sakari Ailus
2017-02-02 16:22 ` [PATCH v2 02/16] device property: Add fwnode_get_parent() Sakari Ailus
2017-02-02 16:22 ` [PATCH v2 03/16] ACPI / property: Add fwnode_get_next_child_node() Sakari Ailus
2017-02-02 16:22 ` [PATCH v2 04/16] device property: Add fwnode_get_named_child_node() Sakari Ailus
2017-02-02 16:22 ` [PATCH v2 05/16] ACPI / property: Add support for remote endpoints Sakari Ailus
2017-02-02 16:22 ` [PATCH v2 06/16] device " Sakari Ailus
     [not found] ` <1486052546-19257-1-git-send-email-sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-02-02 16:22   ` [PATCH v2 07/16] device property: Add fwnode_handle_get() Sakari Ailus
2017-02-02 16:22   ` [PATCH v2 09/16] driver core: Arrange headers alphabetically Sakari Ailus
2017-02-02 16:22   ` [PATCH v2 11/16] of: No need to include linux/property.h, linux/fwnode.h is sufficient Sakari Ailus
2017-02-20 10:37     ` [PATCH v2.1 " Sakari Ailus
2017-02-02 16:22 ` [PATCH v2 08/16] of: Add of_fwnode_handle() to convert device nodes to fwnode_handle Sakari Ailus
2017-02-02 16:22 ` [PATCH v2 10/16] irqchip/gic: Add missing forward declaration for struct device Sakari Ailus
2017-02-02 16:22 ` [PATCH v2 12/16] device property: Obtain device's fwnode independently of FW type Sakari Ailus
2017-02-02 16:22 ` [PATCH v2 13/16] device property: Get endpoint index from the ACPI tables Sakari Ailus
2017-02-02 16:22 ` [PATCH v2 14/16] of: Add nop implementation of of_get_next_parent() Sakari Ailus
2017-02-02 16:22 ` [PATCH v2 15/16] device property: Add fwnode_get_next_parent() Sakari Ailus
2017-02-07 18:57   ` Al Stone
     [not found]     ` <9a481aec-92e6-ac01-b825-167bf73dfb11-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2017-02-08 11:50       ` Rafael J. Wysocki
     [not found]         ` <6293579.fEEljmGD4Z-yvgW3jdyMHm1GS7QM15AGw@public.gmane.org>
2017-02-08 12:19           ` Rafael J. Wysocki
     [not found]             ` <7605668.5ZxNsBFnEG-yvgW3jdyMHm1GS7QM15AGw@public.gmane.org>
2017-02-08 16:49               ` Al Stone
2017-02-14  8:01                 ` Sakari Ailus
2017-02-14 17:09                   ` Al Stone
2017-02-21  8:17             ` Sakari Ailus
     [not found]               ` <20170221081749.GC16975-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
2017-02-21 23:00                 ` Rafael J. Wysocki
2017-02-22 13:12                   ` Sakari Ailus
2017-02-23 17:01               ` Sakari Ailus
2017-02-02 16:22 ` [PATCH v2 16/16] ACPI / DSD: Document references, ports and endpoints Sakari Ailus

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.