[PATCH 1/3] device property: Move FW type specific functionality to FW specific files

[PATCH 1/3] device property: Move FW type specific functionality to FW specific files

[Sakari Ailus]

The device and fwnode property API supports Devicetree, ACPI and pset
properties. The implementation of this functionality for each firmware
type was embedded in the fwnode property core. Move it out to firmware
type specific locations, making it easier to maintain.

Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/acpi/property.c |  87 +++++++++++++++
 drivers/acpi/scan.c     |   1 +
 drivers/base/property.c | 280 +++++++++++++++++++-----------------------------
 drivers/of/property.c   |  99 +++++++++++++++++
 include/linux/acpi.h    |   4 +
 include/linux/fwnode.h  |  53 +++++++++
 include/linux/of.h      |   2 +
 7 files changed, 355 insertions(+), 171 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 9eb7c13..99b81f5 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -57,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->fwnode.ops = &acpi_fwnode_ops;
 	dn->parent = parent;
 	INIT_LIST_HEAD(&dn->data.subnodes);
 
@@ -1107,3 +1108,89 @@ int acpi_graph_get_remote_endpoint(struct fwnode_handle *fwnode,
 
 	return 0;
 }
+
+static bool acpi_fwnode_property_present(struct fwnode_handle *fwnode,
+					 const char *propname)
+{
+	return !acpi_node_prop_get(fwnode, propname, NULL);
+}
+
+static int acpi_fwnode_property_read_int_array(
+	struct fwnode_handle *fwnode, const char *propname,
+	unsigned int elem_size, void *val, size_t nval)
+{
+	enum dev_prop_type type;
+
+	switch (elem_size) {
+	case sizeof(u8):
+		type = DEV_PROP_U8;
+		break;
+	case sizeof(u16):
+		type = DEV_PROP_U16;
+		break;
+	case sizeof(u32):
+		type = DEV_PROP_U32;
+		break;
+	case sizeof(u64):
+		type = DEV_PROP_U64;
+		break;
+	default:
+		return -ENXIO;
+	}
+
+	return acpi_node_prop_read(fwnode, propname, type, val, nval);
+}
+
+static int acpi_fwnode_property_read_string_array(
+	struct fwnode_handle *fwnode, const char *propname, const char **val,
+	size_t nval)
+{
+	int array_len = acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
+					    NULL, nval);
+	int read_len;
+	int ret;
+
+	/* Return if val is NULL or if there was an error */
+	if (!val || array_len < 0)
+		return array_len;
+
+	read_len = min_t(int, nval, array_len);
+
+	ret = acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
+				  val, read_len);
+	if (ret < 0)
+		return ret;
+
+	return read_len;
+}
+
+static struct fwnode_handle *acpi_fwnode_get_parent(
+	struct fwnode_handle *fwnode)
+{
+	return acpi_node_get_parent(fwnode);
+}
+
+static struct fwnode_handle *acpi_fwnode_get_named_child_node(
+	struct fwnode_handle *fwnode, const char *childname)
+{
+	struct fwnode_handle *child;
+
+	/*
+	 * Find first matching named child node of this fwnode.
+	 * For ACPI this will be a data only sub-node.
+	 */
+	fwnode_for_each_child_node(fwnode, child)
+		if (acpi_data_node_match(child, childname))
+			return child;
+
+	return NULL;
+}
+
+const struct fwnode_operations acpi_fwnode_ops = {
+	.property_present = acpi_fwnode_property_present,
+	.property_read_int_array = acpi_fwnode_property_read_int_array,
+	.property_read_string_array = acpi_fwnode_property_read_string_array,
+	.get_parent = acpi_fwnode_get_parent,
+	.get_next_child_node = acpi_get_next_subnode,
+	.get_named_child_node = acpi_fwnode_get_named_child_node,
+};
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 1926918..901720f 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1441,6 +1441,7 @@ void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
 	device->handle = handle;
 	device->parent = acpi_bus_get_parent(handle);
 	device->fwnode.type = FWNODE_ACPI;
+	device->fwnode.ops = &acpi_fwnode_ops;
 	acpi_set_device_status(device, sta);
 	acpi_device_get_busid(device);
 	acpi_set_pnp_ids(handle, &device->pnp, type);
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 325dd7f..3fcb13c 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -165,6 +165,77 @@ struct fwnode_handle *dev_fwnode(struct device *dev)
 }
 EXPORT_SYMBOL_GPL(dev_fwnode);
 
+static bool pset_fwnode_property_present(struct fwnode_handle *fwnode,
+					 const char *propname)
+{
+	return !!pset_prop_get(to_pset_node(fwnode), propname);
+}
+
+static int pset_fwnode_read_int_array(
+	struct fwnode_handle *fwnode, const char *propname,
+	unsigned int elem_size, void *val, size_t nval)
+{
+	struct property_set *node = to_pset_node(fwnode);
+
+	if (!val)
+		return pset_prop_count_elems_of_size(node, propname, elem_size);
+
+	switch (elem_size) {
+	case sizeof(u8):
+		return pset_prop_read_u8_array(node, propname, val, nval);
+	case sizeof(u16):
+		return pset_prop_read_u16_array(node, propname, val, nval);
+	case sizeof(u32):
+		return pset_prop_read_u32_array(node, propname, val, nval);
+	case sizeof(u64):
+		return pset_prop_read_u64_array(node, propname, val, nval);
+	}
+
+	return -ENXIO;
+}
+
+static int pset_fwnode_property_read_string_array(
+	struct fwnode_handle *fwnode, const char *propname, const char **val,
+	size_t nval)
+{
+	struct property_set *node = to_pset_node(fwnode);
+	const struct property_entry *prop;
+	/* The array length for a non-array string property is 1. */
+	int array_len = 1;
+	int read_len;
+	int ret;
+
+	prop = pset_prop_get(node, propname);
+	if (!prop)
+		return -EINVAL;
+
+	/* Read the length of an array property. */
+	if (prop->is_array)
+		array_len = pset_prop_count_elems_of_size(
+			node, propname, sizeof(const char *));
+
+
+	/* Return if val is NULL or if there was an error */
+	if (!val || array_len < 0)
+		return array_len;
+
+	read_len = min_t(int, nval, array_len);
+
+	ret = pset_prop_read_string_array(node, propname, val, read_len);
+
+	if (ret < 0)
+		return ret;
+
+	/* Return the length of an array. */
+	return read_len;
+}
+
+static const struct fwnode_operations pset_fwnode_ops = {
+	.property_present = pset_fwnode_property_present,
+	.property_read_int_array = pset_fwnode_read_int_array,
+	.property_read_string_array = pset_fwnode_property_read_string_array,
+};
+
 /**
  * device_property_present - check if a property of a device is present
  * @dev: Device whose property is being checked
@@ -178,18 +249,6 @@ bool device_property_present(struct device *dev, const char *propname)
 }
 EXPORT_SYMBOL_GPL(device_property_present);
 
-static bool __fwnode_property_present(struct fwnode_handle *fwnode,
-				      const char *propname)
-{
-	if (is_of_node(fwnode))
-		return of_property_read_bool(to_of_node(fwnode), propname);
-	else if (is_acpi_node(fwnode))
-		return !acpi_node_prop_get(fwnode, propname, NULL);
-	else if (is_pset_node(fwnode))
-		return !!pset_prop_get(to_pset_node(fwnode), propname);
-	return false;
-}
-
 /**
  * fwnode_property_present - check if a property of a firmware node is present
  * @fwnode: Firmware node whose property to check
@@ -199,10 +258,11 @@ bool fwnode_property_present(struct fwnode_handle *fwnode, const char *propname)
 {
 	bool ret;
 
-	ret = __fwnode_property_present(fwnode, propname);
+	ret = fwnode_call_int_op(fwnode, property_present, propname);
 	if (ret == false && !IS_ERR_OR_NULL(fwnode) &&
 	    !IS_ERR_OR_NULL(fwnode->secondary))
-		ret = __fwnode_property_present(fwnode->secondary, propname);
+		ret = fwnode_call_int_op(fwnode->secondary, property_present,
+					 propname);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(fwnode_property_present);
@@ -376,42 +436,22 @@ int device_property_match_string(struct device *dev, const char *propname,
 }
 EXPORT_SYMBOL_GPL(device_property_match_string);
 
-#define OF_DEV_PROP_READ_ARRAY(node, propname, type, val, nval)				\
-	(val) ? of_property_read_##type##_array((node), (propname), (val), (nval))	\
-	      : of_property_count_elems_of_size((node), (propname), sizeof(type))
-
-#define PSET_PROP_READ_ARRAY(node, propname, type, val, nval)				\
-	(val) ? pset_prop_read_##type##_array((node), (propname), (val), (nval))	\
-	      : pset_prop_count_elems_of_size((node), (propname), sizeof(type))
-
-#define FWNODE_PROP_READ(_fwnode_, _propname_, _type_, _proptype_, _val_, _nval_)	\
-({											\
-	int _ret_;									\
-	if (is_of_node(_fwnode_))							\
-		_ret_ = OF_DEV_PROP_READ_ARRAY(to_of_node(_fwnode_), _propname_,	\
-					       _type_, _val_, _nval_);			\
-	else if (is_acpi_node(_fwnode_))						\
-		_ret_ = acpi_node_prop_read(_fwnode_, _propname_, _proptype_,		\
-					    _val_, _nval_);				\
-	else if (is_pset_node(_fwnode_)) 						\
-		_ret_ = PSET_PROP_READ_ARRAY(to_pset_node(_fwnode_), _propname_,	\
-					     _type_, _val_, _nval_);			\
-	else										\
-		_ret_ = -ENXIO;								\
-	_ret_;										\
-})
-
-#define FWNODE_PROP_READ_ARRAY(_fwnode_, _propname_, _type_, _proptype_, _val_, _nval_)	\
-({											\
-	int _ret_;									\
-	_ret_ = FWNODE_PROP_READ(_fwnode_, _propname_, _type_, _proptype_,		\
-				 _val_, _nval_);					\
-	if (_ret_ == -EINVAL && !IS_ERR_OR_NULL(_fwnode_) &&				\
-	    !IS_ERR_OR_NULL(_fwnode_->secondary))					\
-		_ret_ = FWNODE_PROP_READ(_fwnode_->secondary, _propname_, _type_,	\
-				_proptype_, _val_, _nval_);				\
-	_ret_;										\
-})
+static int fwnode_property_read_int_array(
+	struct fwnode_handle *fwnode, const char *propname,
+	unsigned int elem_size, void *val, size_t nval)
+{
+	int ret;
+
+	ret = fwnode_call_int_op(fwnode, property_read_int_array, propname,
+				 elem_size, val, nval);
+	if (ret == -EINVAL && !IS_ERR_OR_NULL(fwnode) &&
+	    !IS_ERR_OR_NULL(fwnode->secondary))
+		ret = fwnode_call_int_op(
+			fwnode->secondary, property_read_int_array, propname,
+			elem_size, val, nval);
+
+	return ret;
+}
 
 /**
  * fwnode_property_read_u8_array - return a u8 array property of firmware node
@@ -434,8 +474,8 @@ EXPORT_SYMBOL_GPL(device_property_match_string);
 int fwnode_property_read_u8_array(struct fwnode_handle *fwnode,
 				  const char *propname, u8 *val, size_t nval)
 {
-	return FWNODE_PROP_READ_ARRAY(fwnode, propname, u8, DEV_PROP_U8,
-				      val, nval);
+	return fwnode_property_read_int_array(fwnode, propname, sizeof(u8),
+					      val, nval);
 }
 EXPORT_SYMBOL_GPL(fwnode_property_read_u8_array);
 
@@ -460,8 +500,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u8_array);
 int fwnode_property_read_u16_array(struct fwnode_handle *fwnode,
 				   const char *propname, u16 *val, size_t nval)
 {
-	return FWNODE_PROP_READ_ARRAY(fwnode, propname, u16, DEV_PROP_U16,
-				      val, nval);
+	return fwnode_property_read_int_array(fwnode, propname, sizeof(u16),
+					      val, nval);
 }
 EXPORT_SYMBOL_GPL(fwnode_property_read_u16_array);
 
@@ -486,8 +526,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u16_array);
 int fwnode_property_read_u32_array(struct fwnode_handle *fwnode,
 				   const char *propname, u32 *val, size_t nval)
 {
-	return FWNODE_PROP_READ_ARRAY(fwnode, propname, u32, DEV_PROP_U32,
-				      val, nval);
+	return fwnode_property_read_int_array(fwnode, propname, sizeof(u32),
+					      val, nval);
 }
 EXPORT_SYMBOL_GPL(fwnode_property_read_u32_array);
 
@@ -512,75 +552,11 @@ EXPORT_SYMBOL_GPL(fwnode_property_read_u32_array);
 int fwnode_property_read_u64_array(struct fwnode_handle *fwnode,
 				   const char *propname, u64 *val, size_t nval)
 {
-	return FWNODE_PROP_READ_ARRAY(fwnode, propname, u64, DEV_PROP_U64,
-				      val, nval);
+	return fwnode_property_read_int_array(fwnode, propname, sizeof(u64),
+					      val, nval);
 }
 EXPORT_SYMBOL_GPL(fwnode_property_read_u64_array);
 
-static int __fwnode_property_read_string_array(struct fwnode_handle *fwnode,
-					       const char *propname,
-					       const char **val, size_t nval)
-{
-	if (is_of_node(fwnode))
-		return val ?
-			of_property_read_string_array(to_of_node(fwnode),
-						      propname, val, nval) :
-			of_property_count_strings(to_of_node(fwnode), propname);
-	else if (is_acpi_node(fwnode)) {
-		int array_len =
-			acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
-					    NULL, nval);
-		int read_len;
-		int ret;
-
-		/* Return if val is NULL or if there was an error */
-		if (!val || array_len < 0)
-			return array_len;
-
-		read_len = min_t(int, nval, array_len);
-
-		ret = acpi_node_prop_read(fwnode, propname, DEV_PROP_STRING,
-					  val, read_len);
-		if (ret < 0)
-			return ret;
-
-		return read_len;
-	} else if (is_pset_node(fwnode)) {
-		struct property_set *node = to_pset_node(fwnode);
-		const struct property_entry *prop;
-		/* The array length for a non-array string property is 1. */
-		int array_len = 1;
-		int read_len;
-		int ret;
-
-		prop = pset_prop_get(node, propname);
-		if (!prop)
-			return -EINVAL;
-
-		/* Read the length of an array property. */
-		if (prop->is_array)
-			array_len = pset_prop_count_elems_of_size(
-				node, propname, sizeof(const char *));
-
-
-		/* Return if val is NULL or if there was an error */
-		if (!val || array_len < 0)
-			return array_len;
-
-		read_len = min_t(int, nval, array_len);
-
-		ret = pset_prop_read_string_array(node, propname, val,
-						  read_len);
-
-		if (ret < 0)
-			return ret;
-
-		/* Return the length of an array. */
-		return read_len;
-	}
-	return -ENXIO;
-}
-
 /**
  * fwnode_property_read_string_array - return string array property of a node
  * @fwnode: Firmware node to get the property of
@@ -605,11 +581,13 @@ int fwnode_property_read_string_array(struct fwnode_handle *fwnode,
 {
 	int ret;
 
-	ret = __fwnode_property_read_string_array(fwnode, propname, val, nval);
+	ret = fwnode_call_int_op(fwnode, property_read_string_array, propname,
+				 val, nval);
 	if (ret == -EINVAL && !IS_ERR_OR_NULL(fwnode) &&
 	    !IS_ERR_OR_NULL(fwnode->secondary))
-		ret = __fwnode_property_read_string_array(fwnode->secondary,
-							  propname, val, nval);
+		ret = fwnode_call_int_op(fwnode->secondary,
+					 property_read_string_array, propname,
+					 val, nval);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(fwnode_property_read_string_array);
@@ -927,6 +905,7 @@ int device_add_properties(struct device *dev,
 		return PTR_ERR(p);
 
 	p->fwnode.type = FWNODE_PDATA;
+	p->fwnode.ops = &pset_fwnode_ops;
 	set_secondary_fwnode(dev, &p->fwnode);
 	return 0;
 }
@@ -962,19 +941,7 @@ EXPORT_SYMBOL_GPL(fwnode_get_next_parent);
  */
 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;
+	return fwnode_call_ptr_op(fwnode, get_parent);
 }
 EXPORT_SYMBOL_GPL(fwnode_get_parent);
 
@@ -986,18 +953,7 @@ EXPORT_SYMBOL_GPL(fwnode_get_parent);
 struct fwnode_handle *fwnode_get_next_child_node(struct fwnode_handle *fwnode,
 						 struct fwnode_handle *child)
 {
-	if (is_of_node(fwnode)) {
-		struct device_node *node;
-
-		node = of_get_next_available_child(to_of_node(fwnode),
-						   to_of_node(child));
-		if (node)
-			return &node->fwnode;
-	} else if (is_acpi_node(fwnode)) {
-		return acpi_get_next_subnode(fwnode, child);
-	}
-
-	return NULL;
+	return fwnode_call_ptr_op(fwnode, get_next_child_node, child);
 }
 EXPORT_SYMBOL_GPL(fwnode_get_next_child_node);
 
@@ -1029,23 +985,7 @@ EXPORT_SYMBOL_GPL(device_get_next_child_node);
 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 fwnode.
-	 * For ACPI this will be a data only sub-node.
-	 */
-	fwnode_for_each_child_node(fwnode, child) {
-		if (is_of_node(child)) {
-			if (!of_node_cmp(to_of_node(child)->name, childname))
-				return child;
-		} else if (is_acpi_data_node(child)) {
-			if (acpi_data_node_match(child, childname))
-				return child;
-		}
-	}
-
-	return NULL;
+	return fwnode_call_ptr_op(fwnode, get_named_child_node, childname);
 }
 EXPORT_SYMBOL_GPL(fwnode_get_named_child_node);
 
@@ -1067,8 +1007,7 @@ EXPORT_SYMBOL_GPL(device_get_named_child_node);
  */
 void fwnode_handle_get(struct fwnode_handle *fwnode)
 {
-	if (is_of_node(fwnode))
-		of_node_get(to_of_node(fwnode));
+	fwnode_call_void_op(fwnode, get);
 }
 EXPORT_SYMBOL_GPL(fwnode_handle_get);
 
@@ -1082,8 +1021,7 @@ EXPORT_SYMBOL_GPL(fwnode_handle_get);
  */
 void fwnode_handle_put(struct fwnode_handle *fwnode)
 {
-	if (is_of_node(fwnode))
-		of_node_put(to_of_node(fwnode));
+	fwnode_call_void_op(fwnode, put);
 }
 EXPORT_SYMBOL_GPL(fwnode_handle_put);
 
diff --git a/drivers/of/property.c b/drivers/of/property.c
index afcf00a..6a2d6c4 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -725,3 +725,102 @@ struct device_node *of_graph_get_remote_node(const struct device_node *node,
 	return remote;
 }
 EXPORT_SYMBOL(of_graph_get_remote_node);
+
+static void of_fwnode_get(struct fwnode_handle *fwnode)
+{
+	of_node_get(to_of_node(fwnode));
+}
+
+static void of_fwnode_put(struct fwnode_handle *fwnode)
+{
+	of_node_put(to_of_node(fwnode));
+}
+
+static bool of_fwnode_property_present(struct fwnode_handle *fwnode,
+				       const char *propname)
+{
+	return of_property_read_bool(to_of_node(fwnode), propname);
+}
+
+static int of_fwnode_property_read_int_array(
+	struct fwnode_handle *fwnode, const char *propname,
+	unsigned int elem_size, void *val, size_t nval)
+{
+	struct device_node *node = to_of_node(fwnode);
+
+	if (!val)
+		return of_property_count_elems_of_size(node, propname,
+						       elem_size);
+
+	switch (elem_size) {
+	case sizeof(u8):
+		return of_property_read_u8_array(node, propname, val, nval);
+	case sizeof(u16):
+		return of_property_read_u16_array(node, propname, val, nval);
+	case sizeof(u32):
+		return of_property_read_u32_array(node, propname, val, nval);
+	case sizeof(u64):
+		return of_property_read_u64_array(node, propname, val, nval);
+	}
+
+	return -ENXIO;
+}
+
+static int of_fwnode_property_read_string_array(
+	struct fwnode_handle *fwnode, const char *propname,
+	const char **val, size_t nval)
+{
+	struct device_node *node = to_of_node(fwnode);
+
+	return val ?
+		of_property_read_string_array(node, propname, val, nval) :
+		of_property_count_strings(node, propname);
+}
+
+static struct fwnode_handle *of_fwnode_get_parent(struct fwnode_handle *fwnode)
+{
+	struct device_node *node;
+
+	node = of_get_parent(to_of_node(fwnode));
+	if (node)
+		return of_fwnode_handle(node);
+
+	return NULL;
+}
+
+static struct fwnode_handle *of_fwnode_get_next_child_node(
+	struct fwnode_handle *fwnode, struct fwnode_handle *child)
+{
+	struct device_node *node;
+
+	node = of_get_next_available_child(to_of_node(fwnode),
+					   to_of_node(child));
+	if (node)
+		return of_fwnode_handle(node);
+
+	return NULL;
+}
+
+static struct fwnode_handle *of_fwnode_get_named_child_node(
+	struct fwnode_handle *fwnode, const char *childname)
+{
+	struct device_node *node = to_of_node(fwnode);
+	struct device_node *child;
+
+	for_each_available_child_of_node(node, child)
+		if (!of_node_cmp(child->name, childname))
+			return of_fwnode_handle(child);
+
+	return NULL;
+}
+
+const struct fwnode_operations of_fwnode_ops = {
+	.get = of_fwnode_get,
+	.put = of_fwnode_put,
+	.property_present = of_fwnode_property_present,
+	.property_read_int_array = of_fwnode_property_read_int_array,
+	.property_read_string_array = of_fwnode_property_read_string_array,
+	.get_parent = of_fwnode_get_parent,
+	.get_next_child_node = of_fwnode_get_next_child_node,
+	.get_named_child_node = of_fwnode_get_named_child_node,
+};
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 34674053..4307182 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -56,6 +56,9 @@ static inline acpi_handle acpi_device_handle(struct acpi_device *adev)
 	acpi_fwnode_handle(adev) : NULL)
 #define ACPI_HANDLE(dev)		acpi_device_handle(ACPI_COMPANION(dev))
 
+
+extern const struct fwnode_operations acpi_fwnode_ops;
+
 static inline struct fwnode_handle *acpi_alloc_fwnode_static(void)
 {
 	struct fwnode_handle *fwnode;
@@ -65,6 +68,7 @@ static inline struct fwnode_handle *acpi_alloc_fwnode_static(void)
 		return NULL;
 
 	fwnode->type = FWNODE_ACPI_STATIC;
+	fwnode->ops = &acpi_fwnode_ops;
 
 	return fwnode;
 }
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 3dff239..e05aaef 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -12,6 +12,8 @@
 #ifndef _LINUX_FWNODE_H_
 #define _LINUX_FWNODE_H_
 
+#include <linux/types.h>
+
 enum fwnode_type {
 	FWNODE_INVALID = 0,
 	FWNODE_OF,
@@ -22,9 +24,12 @@ enum fwnode_type {
 	FWNODE_IRQCHIP
 };
 
+struct fwnode_operations;
+
 struct fwnode_handle {
 	enum fwnode_type type;
 	struct fwnode_handle *secondary;
+	const struct fwnode_operations *ops;
 };
 
 /**
@@ -39,4 +44,52 @@ struct fwnode_endpoint {
 	const struct fwnode_handle *local_fwnode;
 };
 
+/**
+ * struct fwnode_operations - Operations for fwnode interface
+ * @get: Get a reference to an fwnode.
+ * @put: Put a reference to an fwnode.
+ * @property_present: Return true if a property is present.
+ * @property_read_integer_array: Read an array of integer properties. Return
+ *				 zero on success, a negative error code
+ *				 otherwise.
+ * @property_read_string_array: Read an array of string properties. Return zero
+ *				on success, a negative error code otherwise.
+ * @get_parent: Return the parent of an fwnode.
+ * @get_next_child_node: Return the next child node in an iteration.
+ * @get_named_child_node: Return a child node with a given name.
+ */
+struct fwnode_operations {
+	void (*get)(struct fwnode_handle *fwnode);
+	void (*put)(struct fwnode_handle *fwnode);
+	bool (*property_present)(struct fwnode_handle *fwnode,
+				 const char *propname);
+	int (*property_read_int_array)(struct fwnode_handle *fwnode,
+				       const char *propname,
+				       unsigned int elem_size, void *val,
+				       size_t nval);
+	int (*property_read_string_array)(struct fwnode_handle *fwnode_handle,
+					  const char *propname,
+					  const char **val, size_t nval);
+	struct fwnode_handle *(*get_parent)(struct fwnode_handle *fwnode);
+	struct fwnode_handle *(*get_next_child_node)(
+		struct fwnode_handle *fwnode, struct fwnode_handle *child);
+	struct fwnode_handle *(*get_named_child_node)(
+		struct fwnode_handle *fwnode, const char *name);
+};
+
+#define fwnode_has_op(fwnode, op)				\
+	((fwnode) && (fwnode)->ops && (fwnode)->ops->op)
+#define fwnode_call_int_op(fwnode, op, ...)				\
+	(fwnode ? (fwnode_has_op(fwnode, op) ?				\
+		   (fwnode)->ops->op(fwnode, ## __VA_ARGS__) : -ENXIO) : \
+	 -EINVAL)
+#define fwnode_call_ptr_op(fwnode, op, ...)		\
+	(fwnode_has_op(fwnode, op) ?			\
+	 (fwnode)->ops->op(fwnode, ## __VA_ARGS__) : NULL)
+#define fwnode_call_void_op(fwnode, op, ...)				\
+	do {								\
+		if (fwnode_has_op(fwnode, op))				\
+			(fwnode)->ops->op(fwnode, ## __VA_ARGS__);	\
+	} while (false)
+
 #endif
diff --git a/include/linux/of.h b/include/linux/of.h
index e5d4225f..3b3eb5b 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -100,10 +100,12 @@ struct of_reconfig_data {
 
 /* initialize a node */
 extern struct kobj_type of_node_ktype;
+extern const struct fwnode_operations of_fwnode_ops;
 static inline void of_node_init(struct device_node *node)
 {
 	kobject_init(&node->kobj, &of_node_ktype);
 	node->fwnode.type = FWNODE_OF;
+	node->fwnode.ops = &of_fwnode_ops;
 }
 
 /* true when node is initialized */
-- 
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

[PATCH 2/3] device property: Move fwnode graph ops to firmware specific locations

[Sakari Ailus]

Move firmware specific implementations of the fwnode graph operations to
firmware specific locations.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 60 ++++++++++++++++++++++++++++++++
 drivers/base/property.c | 92 +++----------------------------------------------
 drivers/of/property.c   | 70 +++++++++++++++++++++++++++++++++++++
 include/linux/fwnode.h  | 16 +++++++++
 4 files changed, 151 insertions(+), 87 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 99b81f5..d839563 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1186,6 +1186,61 @@ static struct fwnode_handle *acpi_fwnode_get_named_child_node(
 	return NULL;
 }
 
+static struct fwnode_handle *acpi_fwnode_graph_get_next_endpoint(
+	struct fwnode_handle *fwnode, struct fwnode_handle *prev)
+{
+	struct fwnode_handle *endpoint;
+
+	endpoint = acpi_graph_get_next_endpoint(fwnode, prev);
+	if (IS_ERR(endpoint))
+		return NULL;
+
+	return endpoint;
+}
+
+static struct fwnode_handle *acpi_fwnode_graph_get_remote_endpoint(
+	struct fwnode_handle *fwnode)
+{
+	struct fwnode_handle *endpoint = NULL;
+
+	acpi_graph_get_remote_endpoint(fwnode, NULL, NULL, &endpoint);
+
+	return endpoint;
+}
+
+static struct fwnode_handle *acpi_fwnode_graph_get_remote_port(
+	struct fwnode_handle *fwnode)
+{
+	struct fwnode_handle *port = NULL;
+
+	acpi_graph_get_remote_endpoint(fwnode, NULL, &port, NULL);
+
+	return port;
+}
+
+static struct fwnode_handle *acpi_fwnode_graph_get_remote_port_parent(
+	struct fwnode_handle *fwnode)
+{
+	struct fwnode_handle *parent = NULL;
+
+	acpi_graph_get_remote_endpoint(fwnode, &parent, NULL, NULL);
+
+	return parent;
+}
+
+static int acpi_fwnode_graph_parse_endpoint(struct fwnode_handle *fwnode,
+					    struct fwnode_endpoint *endpoint)
+{
+	struct fwnode_handle *port_fwnode = fwnode_get_parent(fwnode);
+
+	endpoint->local_fwnode = fwnode;
+
+	fwnode_property_read_u32(port_fwnode, "port", &endpoint->port);
+	fwnode_property_read_u32(fwnode, "endpoint", &endpoint->id);
+
+	return 0;
+}
+
 const struct fwnode_operations acpi_fwnode_ops = {
 	.property_present = acpi_fwnode_property_present,
 	.property_read_int_array = acpi_fwnode_property_read_int_array,
@@ -1193,4 +1248,9 @@ const struct fwnode_operations acpi_fwnode_ops = {
 	.get_parent = acpi_fwnode_get_parent,
 	.get_next_child_node = acpi_get_next_subnode,
 	.get_named_child_node = acpi_fwnode_get_named_child_node,
+	.graph_get_next_endpoint = acpi_fwnode_graph_get_next_endpoint,
+	.graph_get_remote_endpoint = acpi_fwnode_graph_get_remote_endpoint,
+	.graph_get_remote_port = acpi_fwnode_graph_get_remote_port,
+	.graph_get_remote_port_parent = acpi_fwnode_graph_get_remote_port_parent,
+	.graph_parse_endpoint = acpi_fwnode_graph_parse_endpoint,
 };
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 3fcb13c..4f355df 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1160,24 +1160,7 @@ 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;
-
+	return fwnode_call_ptr_op(fwnode, graph_get_next_endpoint, prev);
 }
 EXPORT_SYMBOL_GPL(fwnode_graph_get_next_endpoint);
 
@@ -1190,24 +1173,7 @@ EXPORT_SYMBOL_GPL(fwnode_graph_get_next_endpoint);
 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;
+	return fwnode_call_ptr_op(fwnode, graph_get_remote_port_parent);
 }
 EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_port_parent);
 
@@ -1219,23 +1185,7 @@ EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_port_parent);
  */
 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;
+	return fwnode_call_ptr_op(fwnode, graph_get_remote_endpoint);
 }
 EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_port);
 
@@ -1248,25 +1198,7 @@ EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_port);
 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;
+	return fwnode_call_ptr_op(fwnode, graph_get_remote_endpoint);
 }
 EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_endpoint);
 
@@ -1282,22 +1214,8 @@ EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_endpoint);
 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;
-
-	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;
+	return fwnode_call_int_op(fwnode, graph_parse_endpoint, endpoint);
 }
 EXPORT_SYMBOL(fwnode_graph_parse_endpoint);
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 6a2d6c4..572c36c 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -814,6 +814,71 @@ static struct fwnode_handle *of_fwnode_get_named_child_node(
 	return NULL;
 }
 
+static struct fwnode_handle *of_fwnode_graph_get_next_endpoint(
+	struct fwnode_handle *fwnode, struct fwnode_handle *prev)
+{
+	struct device_node *node;
+
+	node = of_graph_get_next_endpoint(to_of_node(fwnode),
+					  to_of_node(prev));
+	if (node)
+		return of_fwnode_handle(node);
+
+	return NULL;
+}
+
+static struct fwnode_handle *of_fwnode_graph_get_remote_endpoint(
+	struct fwnode_handle *fwnode)
+{
+	struct device_node *node;
+
+	node = of_parse_phandle(to_of_node(fwnode), "remote-endpoint", 0);
+	if (node)
+		return of_fwnode_handle(node);
+
+	return NULL;
+}
+
+static struct fwnode_handle *of_fwnode_graph_get_remote_port(
+	struct fwnode_handle *fwnode)
+{
+	struct device_node *node;
+
+	node = of_graph_get_remote_port(to_of_node(fwnode));
+	if (node)
+		return of_fwnode_handle(node);
+
+	return NULL;
+}
+
+static struct fwnode_handle *of_fwnode_graph_get_remote_port_parent(
+	struct fwnode_handle *fwnode)
+{
+	struct device_node *node;
+
+	node = of_graph_get_remote_port_parent(to_of_node(fwnode));
+	if (node)
+		return of_fwnode_handle(node);
+
+	return NULL;
+}
+
+static int of_fwnode_graph_parse_endpoint(struct fwnode_handle *fwnode,
+					  struct fwnode_endpoint *endpoint)
+{
+	struct device_node *node = to_of_node(fwnode);
+	struct device_node *port_node = of_get_parent(node);
+
+	endpoint->local_fwnode = fwnode;
+
+	of_property_read_u32(port_node, "reg", &endpoint->port);
+	of_property_read_u32(node, "reg", &endpoint->id);
+
+	of_node_put(port_node);
+
+	return 0;
+}
+
 const struct fwnode_operations of_fwnode_ops = {
 	.get = of_fwnode_get,
 	.put = of_fwnode_put,
@@ -823,4 +888,9 @@ const struct fwnode_operations of_fwnode_ops = {
 	.get_parent = of_fwnode_get_parent,
 	.get_next_child_node = of_fwnode_get_next_child_node,
 	.get_named_child_node = of_fwnode_get_named_child_node,
+	.graph_get_next_endpoint = of_fwnode_graph_get_next_endpoint,
+	.graph_get_remote_endpoint = of_fwnode_graph_get_remote_endpoint,
+	.graph_get_remote_port = of_fwnode_graph_get_remote_port,
+	.graph_get_remote_port_parent = of_fwnode_graph_get_remote_port_parent,
+	.graph_parse_endpoint = of_fwnode_graph_parse_endpoint,
 };
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index e05aaef..351c38d 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -57,6 +57,12 @@ struct fwnode_endpoint {
  * @get_parent: Return the parent of an fwnode.
  * @get_next_child_node: Return the next child node in an iteration.
  * @get_named_child_node: Return a child node with a given name.
+ * @graph_get_next_endpoint: Return an endpoint node in an iteration.
+ * @graph_get_remote_endpoint: Return the remote endpoint node of an endpoint
+ *			       node.
+ * @graph_get_remote_port: Return the remote port node of an endpoint node.
+ * @graph_get_remote_port_parent: Return the parent of a port node of the
+ *				  remote endpoint node of an endpoint node.
  */
 struct fwnode_operations {
 	void (*get)(struct fwnode_handle *fwnode);
@@ -75,6 +81,16 @@ struct fwnode_operations {
 		struct fwnode_handle *fwnode, struct fwnode_handle *child);
 	struct fwnode_handle *(*get_named_child_node)(
 		struct fwnode_handle *fwnode, const char *name);
+	struct fwnode_handle *(*graph_get_next_endpoint)(
+		struct fwnode_handle *fwnode, struct fwnode_handle *prev);
+	struct fwnode_handle *(*graph_get_remote_endpoint)(
+		struct fwnode_handle *fwnode);
+	struct fwnode_handle *(*graph_get_remote_port)(
+		struct fwnode_handle *fwnode);
+	struct fwnode_handle *(*graph_get_remote_port_parent)(
+		struct fwnode_handle *fwnode);
+	int (*graph_parse_endpoint)(struct fwnode_handle *fwnode,
+				    struct fwnode_endpoint *endpoint);
 };
 
 #define fwnode_has_op(fwnode, op)				\
-- 
2.7.4

[PATCH 3/3] device property: Add FW type agnostic fwnode_graph_get_remote_node

[Sakari Ailus]

Add fwnode_graph_get_remote_node() function which is equivalent to
of_graph_get_remote_node() on OF.

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

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index d839563..0efcabb 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -1228,6 +1228,38 @@ static struct fwnode_handle *acpi_fwnode_graph_get_remote_port_parent(
 	return parent;
 }
 
+static struct fwnode_handle *acpi_fwnode_graph_get_remote_node(
+	struct fwnode_handle *fwnode, u32 port_id, u32 endpoint_id)
+{
+	struct fwnode_handle *endpoint = NULL;
+
+	while ((endpoint = fwnode_graph_get_next_endpoint(fwnode, endpoint))) {
+		struct fwnode_endpoint fwnode_ep;
+		struct fwnode_handle *remote;
+		struct acpi_device *adev;
+		int ret;
+
+		ret = fwnode_graph_parse_endpoint(endpoint, &fwnode_ep);
+		if (ret < 0)
+			continue;
+
+		if (fwnode_ep.port != port_id || fwnode_ep.id != endpoint_id)
+			continue;
+
+		remote = fwnode_graph_get_remote_port_parent(endpoint);
+		if (!remote)
+			return NULL;
+
+		adev = to_acpi_device_node(remote);
+		if (!adev)
+			return NULL;
+
+		return acpi_device_is_present(adev) ? remote : NULL;
+	}
+
+	return NULL;
+}
+
 static int acpi_fwnode_graph_parse_endpoint(struct fwnode_handle *fwnode,
 					    struct fwnode_endpoint *endpoint)
 {
@@ -1252,5 +1284,6 @@ const struct fwnode_operations acpi_fwnode_ops = {
 	.graph_get_remote_endpoint = acpi_fwnode_graph_get_remote_endpoint,
 	.graph_get_remote_port = acpi_fwnode_graph_get_remote_port,
 	.graph_get_remote_port_parent = acpi_fwnode_graph_get_remote_port_parent,
+	.graph_get_remote_node = acpi_fwnode_graph_get_remote_node,
 	.graph_parse_endpoint = acpi_fwnode_graph_parse_endpoint,
 };
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 4f355df..ce98e01 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1203,6 +1203,22 @@ fwnode_graph_get_remote_endpoint(struct fwnode_handle *fwnode)
 EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_endpoint);
 
 /**
+ * fwnode_graph_get_remote_node - get remote parent fwnode_handle for given port/endpoint
+ * @fwnode: pointer to parent fwnode_handle containing graph port/endpoint
+ * @port: identifier of the parent port node
+ * @endpoint: identifier of the endpoint node
+ *
+ * Return: Remote fwnode handle associated with remote endpoint node linked
+ *	   to @node. Use fwnode_node_put() on it when done.
+ */
+struct fwnode_handle *fwnode_graph_get_remote_node(
+	struct fwnode_handle *fwnode, u32 port, u32 endpoint)
+{
+	return fwnode_call_ptr_op(fwnode, graph_get_remote_node, port, endpoint);
+}
+EXPORT_SYMBOL_GPL(fwnode_graph_get_remote_node);
+
+/**
  * fwnode_graph_parse_endpoint - parse common endpoint node properties
  * @fwnode: pointer to endpoint fwnode_handle
  * @endpoint: pointer to the fwnode endpoint data structure
diff --git a/drivers/of/property.c b/drivers/of/property.c
index 572c36c..c73d181 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -863,6 +863,16 @@ static struct fwnode_handle *of_fwnode_graph_get_remote_port_parent(
 	return NULL;
 }
 
+static struct fwnode_handle_node *of_fwnode_graph_get_remote_node(
+	const struct fwnode_handle *fwnode, u32 port, u32 endpoint)
+{
+	struct fwnode_handle *remote;
+
+	remote = of_graph_get_remote_node(to_of_node(fwnode), port, endpoint);
+
+	return remote ? of_fwnode_handle(remote) : NULL;
+}
+
 static int of_fwnode_graph_parse_endpoint(struct fwnode_handle *fwnode,
 					  struct fwnode_endpoint *endpoint)
 {
@@ -892,5 +902,6 @@ const struct fwnode_operations of_fwnode_ops = {
 	.graph_get_remote_endpoint = of_fwnode_graph_get_remote_endpoint,
 	.graph_get_remote_port = of_fwnode_graph_get_remote_port,
 	.graph_get_remote_port_parent = of_fwnode_graph_get_remote_port_parent,
+	.graph_get_remote_node = of_fwnode_graph_get_remote_node,
 	.graph_parse_endpoint = of_fwnode_graph_parse_endpoint,
 };
diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 351c38d..e1168c0 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -89,6 +89,8 @@ struct fwnode_operations {
 		struct fwnode_handle *fwnode);
 	struct fwnode_handle *(*graph_get_remote_port_parent)(
 		struct fwnode_handle *fwnode);
+	struct fwnode_handle *(*graph_get_remote_node)(
+		struct fwnode_handle *fwnode, u32 port, u32 endpoint);
 	int (*graph_parse_endpoint)(struct fwnode_handle *fwnode,
 				    struct fwnode_endpoint *endpoint);
 };
diff --git a/include/linux/property.h b/include/linux/property.h
index 2f48261..720c30a 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -280,6 +280,8 @@ struct fwnode_handle *fwnode_graph_get_remote_port(
 	struct fwnode_handle *fwnode);
 struct fwnode_handle *fwnode_graph_get_remote_endpoint(
 	struct fwnode_handle *fwnode);
+struct fwnode_handle *fwnode_graph_get_remote_node(
+	struct fwnode_handle *fwnode, u32 port, u32 endpoint);
 
 int fwnode_graph_parse_endpoint(struct fwnode_handle *fwnode,
 				struct fwnode_endpoint *endpoint);
-- 
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

Re: [PATCH 1/3] device property: Move FW type specific functionality to FW specific files

[Sakari Ailus]

On Fri, Mar 24, 2017 at 01:03:50PM +0200, Sakari Ailus wrote:
> The device and fwnode property API supports Devicetree, ACPI and pset
> properties. The implementation of this functionality for each firmware
> type was embedded in the fwnode property core. Move it out to firmware
> type specific locations, making it easier to maintain.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Oh, this escaped me a bit early.

This set is the other part split off from the earlier "Move firmware
specific code to firmware specific locations" set. It depends on my earlier
the fwnode fixes, ACPI graph support patchsets and patch "device property:
Read strings using string array reading functions" that are intended for the
PM tree. They also depend on patch "of: Move OF property and graph API from
base.c to property.c" which is intended for the DT tree --- this patch
additionally depends on the patches on DT tree.

The only change in the patches from the previous set is rebase, and a new
patch adding support for fwnode_graph_get_remote_node() has been added. The
last patch is currently untested. I'll test it in the near future; this set
is not mergeable for a while until the dependent patches have been merged to
the respective trees and a new rc1 release is available.

I'm uploading the patches to the two branches here:

Fwnode fixes + ACPI graph support + "device property:
Read strings using string array reading functions":

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

The previous patches on Rob H's for-next branch + "of: Move OF property
and graph API from base.c to property.c" + this patchset:

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

It may still take a while until the patches get there.

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@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

Re: [PATCH 1/3] device property: Move FW type specific functionality to FW specific files

[Rafael J. Wysocki]

On Friday, March 24, 2017 01:42:13 PM Sakari Ailus wrote:
> On Fri, Mar 24, 2017 at 01:03:50PM +0200, Sakari Ailus wrote:
> > The device and fwnode property API supports Devicetree, ACPI and pset
> > properties. The implementation of this functionality for each firmware
> > type was embedded in the fwnode property core. Move it out to firmware
> > type specific locations, making it easier to maintain.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> Oh, this escaped me a bit early.
> 
> This set is the other part split off from the earlier "Move firmware
> specific code to firmware specific locations" set. It depends on my earlier
> the fwnode fixes, ACPI graph support patchsets and patch "device property:
> Read strings using string array reading functions" that are intended for the
> PM tree. They also depend on patch "of: Move OF property and graph API from
> base.c to property.c" which is intended for the DT tree --- this patch
> additionally depends on the patches on DT tree.
> 
> The only change in the patches from the previous set is rebase, and a new
> patch adding support for fwnode_graph_get_remote_node() has been added. The
> last patch is currently untested. I'll test it in the near future; this set
> is not mergeable for a while until the dependent patches have been merged to
> the respective trees and a new rc1 release is available.
> 
> I'm uploading the patches to the two branches here:
> 
> Fwnode fixes + ACPI graph support + "device property:
> Read strings using string array reading functions":
> 
> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=acpi-graph-all>
> 
> The previous patches on Rob H's for-next branch + "of: Move OF property
> and graph API from base.c to property.c" + this patchset:
> 
> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=acpi-graph-cleaned-on-next>
> 
> It may still take a while until the patches get there.

I'd prefer to see them in Patchwork, though.

Can you please repost all stuff you want to go into 4.12 that has no
dependencies in other trees afresh?

Thanks,
Rafael

Re: [PATCH 1/3] device property: Move FW type specific functionality to FW specific files

[Mika Westerberg]

On Fri, Mar 24, 2017 at 01:03:50PM +0200, Sakari Ailus wrote:
> +static int acpi_fwnode_property_read_int_array(

This is really ugly way to split arguments into multiple lines IMHO.

> +	struct fwnode_handle *fwnode, const char *propname,
> +	unsigned int elem_size, void *val, size_t nval)


I would much more see something like:

static int
acpi_fwnode_property_read_int_array(struct fwnode_handle *fwnode,
	const char *propname, unsigned int elem_size, void *val, size_t nval)

or even

static int
acpi_fwnode_property_read_int_array(struct fwnode_handle *fwnode,
				    const char *propname,
				    unsigned int elem_size,
				    void *val, size_t nval)

ditto for all occurences.

Other than that this patch looks reasonable to me.

Re: [PATCH 2/3] device property: Move fwnode graph ops to firmware specific locations

[Mika Westerberg]

On Fri, Mar 24, 2017 at 01:03:51PM +0200, Sakari Ailus wrote:
>  const struct fwnode_operations acpi_fwnode_ops = {
>  	.property_present = acpi_fwnode_property_present,
>  	.property_read_int_array = acpi_fwnode_property_read_int_array,
> @@ -1193,4 +1248,9 @@ const struct fwnode_operations acpi_fwnode_ops = {
>  	.get_parent = acpi_fwnode_get_parent,
>  	.get_next_child_node = acpi_get_next_subnode,
>  	.get_named_child_node = acpi_fwnode_get_named_child_node,
> +	.graph_get_next_endpoint = acpi_fwnode_graph_get_next_endpoint,
> +	.graph_get_remote_endpoint = acpi_fwnode_graph_get_remote_endpoint,
> +	.graph_get_remote_port = acpi_fwnode_graph_get_remote_port,
> +	.graph_get_remote_port_parent = acpi_fwnode_graph_get_remote_port_parent,
> +	.graph_parse_endpoint = acpi_fwnode_graph_parse_endpoint,
>  };

Not sure if it is possible but it would be nice to have a single
primitive implementation specific graph callback and then build
everything else on top of that in generic code. Here you have 5
callbacks just for graph support.
--
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

Re: [PATCH 2/3] device property: Move fwnode graph ops to firmware specific locations

[Sakari Ailus]

Hi Mika,

Thank you for the review.

On Mon, Mar 27, 2017 at 02:52:00PM +0300, Mika Westerberg wrote:
> On Fri, Mar 24, 2017 at 01:03:51PM +0200, Sakari Ailus wrote:
> >  const struct fwnode_operations acpi_fwnode_ops = {
> >  	.property_present = acpi_fwnode_property_present,
> >  	.property_read_int_array = acpi_fwnode_property_read_int_array,
> > @@ -1193,4 +1248,9 @@ const struct fwnode_operations acpi_fwnode_ops = {
> >  	.get_parent = acpi_fwnode_get_parent,
> >  	.get_next_child_node = acpi_get_next_subnode,
> >  	.get_named_child_node = acpi_fwnode_get_named_child_node,
> > +	.graph_get_next_endpoint = acpi_fwnode_graph_get_next_endpoint,
> > +	.graph_get_remote_endpoint = acpi_fwnode_graph_get_remote_endpoint,
> > +	.graph_get_remote_port = acpi_fwnode_graph_get_remote_port,
> > +	.graph_get_remote_port_parent = acpi_fwnode_graph_get_remote_port_parent,
> > +	.graph_parse_endpoint = acpi_fwnode_graph_parse_endpoint,
> >  };
> 
> Not sure if it is possible but it would be nice to have a single
> primitive implementation specific graph callback and then build
> everything else on top of that in generic code. Here you have 5
> callbacks just for graph support.

Getting the parent of the port in OF graph is OF specific, the port parent
is not necessarily a direct parent node of the port (in presence of the
"ports" node that contains all port nodes).

I could potentially remove graph_get_remote_port() and use
graph_get_remote_endpoint() and graph_get_parent() instead. I didn't
originally do that as I thought it could be better ot leave it up to the
implementation.

What do you think?

-- 
Regards,

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

Re: [PATCH 2/3] device property: Move fwnode graph ops to firmware specific locations

[Mika Westerberg]

On Thu, Mar 30, 2017 at 06:10:19PM +0300, Sakari Ailus wrote:
> Hi Mika,
> 
> Thank you for the review.
> 
> On Mon, Mar 27, 2017 at 02:52:00PM +0300, Mika Westerberg wrote:
> > On Fri, Mar 24, 2017 at 01:03:51PM +0200, Sakari Ailus wrote:
> > >  const struct fwnode_operations acpi_fwnode_ops = {
> > >  	.property_present = acpi_fwnode_property_present,
> > >  	.property_read_int_array = acpi_fwnode_property_read_int_array,
> > > @@ -1193,4 +1248,9 @@ const struct fwnode_operations acpi_fwnode_ops = {
> > >  	.get_parent = acpi_fwnode_get_parent,
> > >  	.get_next_child_node = acpi_get_next_subnode,
> > >  	.get_named_child_node = acpi_fwnode_get_named_child_node,
> > > +	.graph_get_next_endpoint = acpi_fwnode_graph_get_next_endpoint,
> > > +	.graph_get_remote_endpoint = acpi_fwnode_graph_get_remote_endpoint,
> > > +	.graph_get_remote_port = acpi_fwnode_graph_get_remote_port,
> > > +	.graph_get_remote_port_parent = acpi_fwnode_graph_get_remote_port_parent,
> > > +	.graph_parse_endpoint = acpi_fwnode_graph_parse_endpoint,
> > >  };
> > 
> > Not sure if it is possible but it would be nice to have a single
> > primitive implementation specific graph callback and then build
> > everything else on top of that in generic code. Here you have 5
> > callbacks just for graph support.
> 
> Getting the parent of the port in OF graph is OF specific, the port parent
> is not necessarily a direct parent node of the port (in presence of the
> "ports" node that contains all port nodes).
> 
> I could potentially remove graph_get_remote_port() and use
> graph_get_remote_endpoint() and graph_get_parent() instead. I didn't
> originally do that as I thought it could be better ot leave it up to the
> implementation.
> 
> What do you think?

Well we are dealing with two kinds of objects here, ports and endpoins.
Maybe we can the low level functions parse and return these and then add
generic code on top of that only deals with the port and endpoint
objects. Something like get_graph() which parses the firmware structure
and returns Linux port and endpoint object tree.

Not sure if it is overkill in this case but adding 5 callbacks just for
graphs smells like the design could be improved ;-)

Re: [PATCH 1/3] device property: Move FW type specific functionality to FW specific files

[Sakari Ailus]

Hi Mika,

Thanks for the review again!

Mika Westerberg wrote:
> On Fri, Mar 24, 2017 at 01:03:50PM +0200, Sakari Ailus wrote:
>> +static int acpi_fwnode_property_read_int_array(
>
> This is really ugly way to split arguments into multiple lines IMHO.
>
>> +	struct fwnode_handle *fwnode, const char *propname,
>> +	unsigned int elem_size, void *val, size_t nval)
>
>
> I would much more see something like:
>
> static int
> acpi_fwnode_property_read_int_array(struct fwnode_handle *fwnode,
> 	const char *propname, unsigned int elem_size, void *val, size_t nval)
>
> or even
>
> static int
> acpi_fwnode_property_read_int_array(struct fwnode_handle *fwnode,
> 				    const char *propname,
> 				    unsigned int elem_size,
> 				    void *val, size_t nval)
>
> ditto for all occurences.
>
> Other than that this patch looks reasonable to me.

I don't think CodingStyle defines any particular points where to wrap 
the lines. Different conventions seem to exist:

$ git grep '^\(static\|const\|inline\|int\|struct\|u\[0-9\).*($' |wc -l
3106

Not all of the occurrences found exhibit such indentation but then again 
the above does not catch all of those that do either.

I can change that if you insist but I'm telling you that it wouldn't be 
alone in the kernel. :-)

-- 
Kind regards,

Sakari Ailus
sakari.ailus-VuQAYsv1563Yd54FQh9/CA@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