[RFC 00/15] ACPI graph support

[RFC 00/15] ACPI graph support

[Sakari Ailus]

Hello everyone,

I've been working awhile with my collegue Mika Westerberg to bring
firmware graph support to ACPI based systems. In practice 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.

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

The entire set may also be found here:

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

The V4L2 patches can be found here --- I'll send them to the linux-media
list in the near future:

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

Both sets can be found here, applied on the same tree (the V4L2 patches
depend on other patches not in Linus's tree yet):

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

Feedback would be very welcome.

-- 
Kind regards,
Sakari

[RFC 01/15] ACPI / property: Add possiblity to retrieve parent firmware node

[Sakari Ailus]

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 | 52 +++++++++++++++++++++++++++++++++++++++++--------
 include/acpi/acpi_bus.h |  1 +
 include/linux/acpi.h    |  7 +++++++
 3 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index f2fd3fe..181e4c5 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -37,13 +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_ok(acpi_handle scope,
 				   const union acpi_object *link,
-				   struct list_head *list)
+				   struct list_head *list,
+				   struct fwnode_handle *parent)
 {
 	struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
 	struct acpi_data_node *dn;
@@ -56,6 +59,7 @@ static bool acpi_nondev_subnode_ok(acpi_handle scope,
 
 	dn->name = link->package.elements[0].string.pointer;
 	dn->fwnode.type = FWNODE_ACPI_DATA;
+	dn->parent = parent;
 	INIT_LIST_HEAD(&dn->data.subnodes);
 
 	status = acpi_get_handle(scope, link->package.elements[1].string.pointer,
@@ -78,7 +82,8 @@ static bool acpi_nondev_subnode_ok(acpi_handle scope,
 	 */
 	status = acpi_get_parent(handle, &scope);
 	if (ACPI_SUCCESS(status)
-	    && acpi_enumerate_nondev_subnodes(scope, buf.pointer, &dn->data))
+	    && acpi_enumerate_nondev_subnodes(scope, buf.pointer, &dn->data,
+					      &dn->fwnode))
 		dn->handle = handle;
 
 	if (dn->handle) {
@@ -97,7 +102,8 @@ static bool acpi_nondev_subnode_ok(acpi_handle scope,
 
 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;
@@ -110,7 +116,7 @@ static int acpi_add_nondev_subnodes(acpi_handle scope,
 		if (link->package.count == 2
 		    && link->package.elements[0].type == ACPI_TYPE_STRING
 		    && link->package.elements[1].type == ACPI_TYPE_STRING
-		    && acpi_nondev_subnode_ok(scope, link, list))
+		    && acpi_nondev_subnode_ok(scope, link, list, parent))
 			ret = true;
 	}
 
@@ -119,7 +125,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;
 
@@ -141,7 +148,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;
@@ -292,7 +300,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) {
@@ -848,3 +857,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 c1a524d..8504716 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 19e650c..6906f7a 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -961,6 +961,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 *,
@@ -1075,6 +1076,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

[RFC 02/15] device property: Add fwnode_get_parent()

[Sakari Ailus]

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

[RFC 03/15] ACPI / property: Add fwnode_get_next_child_node()

[Sakari Ailus]

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 181e4c5..f4e812e 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -802,21 +802,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;
 
@@ -825,7 +826,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);
@@ -837,9 +837,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 6906f7a..1424cce 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -959,8 +959,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;
@@ -1070,8 +1070,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

[RFC 04/15] device property: Add fwnode_get_named_child_node()

[Sakari Ailus]

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

[RFC 05/15] ACPI / property: Add support for remote endpoints

[Sakari Ailus]

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

DT has had concept of remote endpoins for some time already. It makes
possible to reference another firmware node through a property named
"remote-endpoint". This is already used by some subsystems such as V4L2
for parsing hardware properties related to camera.

This adds ACPI implementation of remote endpoints.

Each device must have data extension property with name "ports". This lists
all ports within the device. Each port hold similar data extension that
contains all the endpoints available. The "remote-endpoint" reference then
can be used to match the device, port and endpoind accordingly.

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

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index f4e812e..6f561e7 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -891,3 +891,150 @@ 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 *ports, *port, *endpoint;
+
+	ports = fwnode_get_named_child_node(fwnode, "ports");
+	if (!ports)
+		return NULL;
+
+	if (!prev) {
+		port = fwnode_get_next_child_node(ports, NULL);
+		if (!port)
+			return NULL;
+	} else {
+		port = fwnode_get_parent(prev);
+	}
+
+	/* Ports must have "port" property */
+	if (!fwnode_property_present(port, "port"))
+		return ERR_PTR(-EPROTO);
+
+	endpoint = fwnode_get_next_child_node(port, prev);
+	if (!endpoint) {
+		/* Find the next port under ports */
+		port = fwnode_get_next_child_node(ports, port);
+		if (port) {
+			if (!fwnode_property_present(port, "port"))
+				return ERR_PTR(-EPROTO);
+			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 ports_idx, 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 three arguments with the reference: ports
+	 * list and a given port and a given endpoint.
+	 */
+	if (args.nargs != 3)
+		return -EPROTO;
+
+	fwnode = acpi_fwnode_handle(args.adev);
+	ports_idx = args.args[0];
+	port_idx = args.args[1];
+	endpoint_idx = args.args[2];
+
+	if (parent)
+		*parent = fwnode;
+
+	fwnode = acpi_graph_get_child_at(fwnode, ports_idx);
+	if (!fwnode || !acpi_data_node_match(fwnode, "ports"))
+		return -EPROTO;
+
+	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 1424cce..5d7fabd 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -963,6 +963,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 *);
@@ -1082,6 +1089,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

[RFC 06/15] device property: Add support for remote endpoints

[Sakari Ailus]

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.

This commit includes material from Sakari Ailus.

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

diff --git a/drivers/base/property.c b/drivers/base/property.c
index c0011cf..0c9a8b2 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,124 @@ 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(node, "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..1e39c4e 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -263,4 +263,14 @@ 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

[RFC 07/15] device property: Add fwnode_handle_get()

[Sakari Ailus]

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@linux.intel.com>
---
 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 0c9a8b2..1ade4b7 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 1e39c4e..305a065 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

[RFC 08/15] of: Add of_fwnode_handle() to convert device nodes to fwnode_handle

[Sakari Ailus]

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

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

diff --git a/include/linux/of.h b/include/linux/of.h
index 3d9ff8e..c9a7962 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -159,6 +159,11 @@ static inline struct device_node *to_of_node(struct fwnode_handle *fwnode)
 		container_of(fwnode, struct device_node, fwnode) : NULL;
 }
 
+static inline struct fwnode_handle *of_fwnode_handle(struct device_node *node)
+{
+	return &node->fwnode;
+}
+
 static inline bool of_have_populated_dt(void)
 {
 	return of_root != NULL;
@@ -481,6 +486,11 @@ static inline struct device_node *of_find_node_with_property(
 	return NULL;
 }
 
+static inline struct fwnode_handle *of_fwnode_handle(struct device_node *node)
+{
+	return NULL;
+}
+
 static inline bool of_have_populated_dt(void)
 {
 	return false;
-- 
2.7.4

[RFC 09/15] driver core: Arrange headers alphabetically

[Sakari Ailus]

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 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 70c5be5..93c1d3f 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

[RFC 10/15] of: No need to include property.h, fwnode.h is sufficient

[Sakari Ailus]

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.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/base/core.c                    | 1 +
 drivers/input/touchscreen/ad7879.c     | 1 +
 drivers/input/touchscreen/edt-ft5x06.c | 1 +
 drivers/phy/phy-tusb1210.c             | 1 +
 drivers/usb/common/common.c            | 1 +
 drivers/usb/dwc3/host.c                | 1 +
 include/linux/of.h                     | 2 +-
 sound/soc/codecs/ts3a227e.c            | 1 +
 8 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 93c1d3f..353d78a 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/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 703e295..81d114d 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/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/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/dwc3/host.c b/drivers/usb/dwc3/host.c
index f6533c6..4cf6546 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 c9a7962..91b2308 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/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>
-- 
2.7.4

[RFC 11/15] device property: Obtain device's fwnode independently of FW type

[Sakari Ailus]

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.

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

diff --git a/include/linux/property.h b/include/linux/property.h
index 305a065..b1c5365c 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;
@@ -274,4 +276,12 @@ fwnode_graph_get_remote_port(struct fwnode_handle *fwnode);
 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;
+}
+
 #endif /* _LINUX_PROPERTY_H_ */
-- 
2.7.4

[RFC 12/15] device property: Add support for fwnode endpoints

[Sakari Ailus]

Similar to OF endpoints, endpoint type nodes can be also supported on
ACPI. In order to make it possible for drivers to ignore the matter,
add a type for fwnode_endpoint and a function to parse them.

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

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 1ade4b7..d173960 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1244,3 +1244,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 8516717..7940b79 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -26,4 +26,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 b1c5365c..33fbca6 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -284,4 +284,7 @@ static inline struct fwnode_handle *device_fwnode_handle(struct device *dev)
 		return dev->fwnode;
 }
 
+int fwnode_graph_parse_endpoint(struct fwnode_handle *fwnode,
+				struct fwnode_endpoint *endpoint);
+
 #endif /* _LINUX_PROPERTY_H_ */
-- 
2.7.4

[RFC 13/15] of: Add nop implementation of of_get_next_parent()

[Sakari Ailus]

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 91b2308..78613d9 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -468,6 +468,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

[RFC 14/15] device property: Add fwnode_get_next_parent()

[Sakari Ailus]

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 d173960..65d5f13 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 33fbca6..df8392d 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

[RFC 15/15] ACPI / DSD: Document references, ports and endpoints

[Sakari Ailus]

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 | 166 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 166 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..fe6eb57
--- /dev/null
+++ b/Documentation/acpi/dsd/graph.txt
@@ -0,0 +1,166 @@
+Graphs
+
+
+DSD
+---
+
+Device specific data (DSD) [1] is an ACPI feature which can be used to
+describe hardware features that are not directly defined by an
+existing ACPI standard [6]. Besides the usual DSD device and fwnode
+properties [4], the DSD extension also supports hierarchical data
+structure [5] the nodes of which may contain additional properties.
+Functionality-wise this is somewhat equivalent to the phandles in
+Devicetree [2].
+
+The hierarchical data extension [5] may be used to construct a
+tree-like structure with nodes which may contain other nodes (again
+using the hierarchical data extension) or properties (using device and
+fwnode properties [4]).
+
+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 a child node under the device's
+"_DSD" node in hierarchical data extension tree. The first
+hierarchical data extension package list entry of this node must be
+called "ports". Individual ports are declared as child nodes of this
+node. The first package list entry of the hierarchical data extension
+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. Similarly, each endpoint node
+must contain "endpoint" property which is the number of the endpoint
+node.
+
+The endpoint reference uses property extension with "remote-endpoint"
+property name followed by a reference in the same package. The
+references to endpoints must be always done both to and from the
+endpoint node. Individual references thus appear as:
+
+    Package() { device, ports_node_index,
+		port_node_index, endpoint_node_index }
+
+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 () { "ports", "PRTS" },
+		}
+	    })
+	    Name (PRTS, Package() {
+		ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
+		Package () {
+		    Package () { "port@0", "PRT0" },
+		}
+	    })
+	    Name (PRT0, Package() {
+		ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+		Package () {
+		    Package () { "port", 0 },
+		},
+		ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
+		Package () {
+		    Package () { "endpoint@0", "EP0" },
+		}
+	    })
+	    Name (EP0, Package() {
+		ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+		Package () {
+		    Package () { "endpoint", 0 },
+		    Package () { "remote-endpoint", Package() { \_SB.PCI0.ISP, 0, 0, 0 } },
+		}
+	    })
+	}
+    }
+
+    Scope (\_SB.PCI0)
+    {
+	Device (ISP)
+	{
+	    Name (_DSD, Package () {
+		ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
+		Package () {
+		    Package () { "ports", "PRTS" },
+		}
+	    })
+
+	    Name (PRTS, Package() {
+		ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
+		Package () {
+		    Package () { "port@4", "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 () { "endpoint@0", "EP0" },
+		}
+	    })
+
+	    Name (EP0, Package() {
+		ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+		Package () {
+		    Package () { "endpoint", 0 },
+		    Package () { "remote-endpoint", Package () { \_SB.PCI0.I2C2.CAM0, 0, 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.
-- 
2.7.4

Re: [RFC 00/15] ACPI graph support

[Lorenzo Pieralisi]

[ +MarkR, MarkB, Rob, Al - I suspect they may want to have a say]

On Wed, Oct 05, 2016 at 01:45:33AM +0300, Sakari Ailus wrote:
> Hello everyone,
> 
> I've been working awhile with my collegue Mika Westerberg to bring
> firmware graph support to ACPI based systems. In practice 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.

There is an ongoing effort to avoid wholesale import of DT bindings
into ACPI, I am not a V4L2 expert but it seems to me that with patches
like the one you have submitted we are getting closer and closer to
achieving it instead of avoiding it.

For your information, Al is working on a process to submit _DSD
bindings and this patchset should at least be vetted within that
context.

It is an RFC and my comment is that I do not like the direction
this ACPI->_DSD->DT is taking, I would like to understand where
this is intended to stop because I am getting worried.

Thanks,
Lorenzo

> The last patch of the set contains ASL documentation including and
> example.
> 
> The entire set may also be found here:
> 
> <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.
> 
> The V4L2 patches can be found here --- I'll send them to the linux-media
> list in the near future:
> 
> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=v4l2-acpi>
> 
> Both sets can be found here, applied on the same tree (the V4L2 patches
> depend on other patches not in Linus's tree yet):
> 
> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=v4l2-acpi-on-graph>
> 
> Feedback would be very welcome.
> 
> -- 
> Kind regards,
> Sakari
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [RFC 00/15] ACPI graph support

[Mika Westerberg]

On Wed, Oct 05, 2016 at 10:22:15AM +0100, Lorenzo Pieralisi wrote:
> [ +MarkR, MarkB, Rob, Al - I suspect they may want to have a say]
> 
> On Wed, Oct 05, 2016 at 01:45:33AM +0300, Sakari Ailus wrote:
> > Hello everyone,
> > 
> > I've been working awhile with my collegue Mika Westerberg to bring
> > firmware graph support to ACPI based systems. In practice 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.
> 
> There is an ongoing effort to avoid wholesale import of DT bindings
> into ACPI, I am not a V4L2 expert but it seems to me that with patches
> like the one you have submitted we are getting closer and closer to
> achieving it instead of avoiding it.

The whole purpose of PRP0001 ID is to allow DT bindings to be reused in
ACPI systems, so that the drivers can just call device_property_* and
get the properties regardless of the underlying firmware interface.

Are you saying that's not wanted?

> For your information, Al is working on a process to submit _DSD
> bindings and this patchset should at least be vetted within that
> context.

Of course but is it more like "use these properties for our hardware
XYZ"? I mean remote endpoints is a generic concept and not tied to a
certain hardware.

> It is an RFC and my comment is that I do not like the direction
> this ACPI->_DSD->DT is taking, I would like to understand where
> this is intended to stop because I am getting worried.

I understand that if there is already an existing native ACPI way of
doing things, that should be used. However, we do not have such thing
for remote endpoints used between camera components, and on the other
hand there is an exiting DT binding which only requires small changes to
the v4l2 framework (convert it to use fwnodes) to get the thing
supported on both DT and ACPI systems.

Re: [RFC 00/15] ACPI graph support

[Lorenzo Pieralisi]

On Wed, Oct 05, 2016 at 02:41:29PM +0300, Mika Westerberg wrote:
> On Wed, Oct 05, 2016 at 10:22:15AM +0100, Lorenzo Pieralisi wrote:
> > [ +MarkR, MarkB, Rob, Al - I suspect they may want to have a say]
> > 
> > On Wed, Oct 05, 2016 at 01:45:33AM +0300, Sakari Ailus wrote:
> > > Hello everyone,
> > > 
> > > I've been working awhile with my collegue Mika Westerberg to bring
> > > firmware graph support to ACPI based systems. In practice 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.
> > 
> > There is an ongoing effort to avoid wholesale import of DT bindings
> > into ACPI, I am not a V4L2 expert but it seems to me that with patches
> > like the one you have submitted we are getting closer and closer to
> > achieving it instead of avoiding it.
> 
> The whole purpose of PRP0001 ID is to allow DT bindings to be reused in
> ACPI systems, so that the drivers can just call device_property_* and
> get the properties regardless of the underlying firmware interface.
> 
> Are you saying that's not wanted?

Not wholesale DT bindings import into ACPI, just no way.

> > For your information, Al is working on a process to submit _DSD
> > bindings and this patchset should at least be vetted within that
> > context.
> 
> Of course but is it more like "use these properties for our hardware
> XYZ"? I mean remote endpoints is a generic concept and not tied to a
> certain hardware.
> 
> > It is an RFC and my comment is that I do not like the direction
> > this ACPI->_DSD->DT is taking, I would like to understand where
> > this is intended to stop because I am getting worried.
> 
> I understand that if there is already an existing native ACPI way of
> doing things, that should be used. However, we do not have such thing
> for remote endpoints used between camera components, and on the other
> hand there is an exiting DT binding which only requires small changes to
> the v4l2 framework (convert it to use fwnodes) to get the thing
> supported on both DT and ACPI systems.

FWIW I had a quick look at dts bindings with "compatible = nokia,smia"
and related kernel driver.

Those nodes require properties like clocks and power supplies, it
seems to me that there are already ACPI PM methods to control those
properties and therefore they should not be handled with PRP0001,
I am happy to be corrected if I am wrong.

When you start matching whole subsystem through PRP0001 and related
compatible strings you can end up in a situation where DT and ACPI
FW handling clash and that's why we were opposed to mixing them from
the beginning, in ARM world if we need a DT we boot with a devicetree.

If PRP0001 is used for leaf-nodes drivers properties it may work,
everything else, frankly, is a bit of a gamble you are taking.

At least you should CC devicetree mailing lists and reach out to a
wider audience than linux-acpi, you will get the comments you are
requesting then.

Thanks,
Lorenzo

Re: [RFC 00/15] ACPI graph support

[Mika Westerberg]

On Wed, Oct 05, 2016 at 04:06:41PM +0100, Lorenzo Pieralisi wrote:
> On Wed, Oct 05, 2016 at 02:41:29PM +0300, Mika Westerberg wrote:
> > On Wed, Oct 05, 2016 at 10:22:15AM +0100, Lorenzo Pieralisi wrote:
> > > [ +MarkR, MarkB, Rob, Al - I suspect they may want to have a say]
> > > 
> > > On Wed, Oct 05, 2016 at 01:45:33AM +0300, Sakari Ailus wrote:
> > > > Hello everyone,
> > > > 
> > > > I've been working awhile with my collegue Mika Westerberg to bring
> > > > firmware graph support to ACPI based systems. In practice 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.
> > > 
> > > There is an ongoing effort to avoid wholesale import of DT bindings
> > > into ACPI, I am not a V4L2 expert but it seems to me that with patches
> > > like the one you have submitted we are getting closer and closer to
> > > achieving it instead of avoiding it.
> > 
> > The whole purpose of PRP0001 ID is to allow DT bindings to be reused in
> > ACPI systems, so that the drivers can just call device_property_* and
> > get the properties regardless of the underlying firmware interface.
> > 
> > Are you saying that's not wanted?
> 
> Not wholesale DT bindings import into ACPI, just no way.

Of course not all DT bindings. Only those that do not have a native ACPI
representation.

The PRP0001 is there so that we do not end up with drivers looking like
this:

	if (dev->of_node)
		device_property_read_xx(dev, "my-dt-property", &value);
	else if (has_acpi_handle(dev))
		device_property_read_xx(dev, "my-acpi-property", &value);
	else
		device_property_read_xx(dev, "my-builtin-property", &value)

but instead they can use the single "my-property" regardless from which
firmware interface is used.

If there exists a native ACPI representation, like a PowerResource or
GPIO, it should be used instead.

> > > For your information, Al is working on a process to submit _DSD
> > > bindings and this patchset should at least be vetted within that
> > > context.
> > 
> > Of course but is it more like "use these properties for our hardware
> > XYZ"? I mean remote endpoints is a generic concept and not tied to a
> > certain hardware.
> > 
> > > It is an RFC and my comment is that I do not like the direction
> > > this ACPI->_DSD->DT is taking, I would like to understand where
> > > this is intended to stop because I am getting worried.
> > 
> > I understand that if there is already an existing native ACPI way of
> > doing things, that should be used. However, we do not have such thing
> > for remote endpoints used between camera components, and on the other
> > hand there is an exiting DT binding which only requires small changes to
> > the v4l2 framework (convert it to use fwnodes) to get the thing
> > supported on both DT and ACPI systems.
> 
> FWIW I had a quick look at dts bindings with "compatible = nokia,smia"
> and related kernel driver.
> 
> Those nodes require properties like clocks and power supplies, it
> seems to me that there are already ACPI PM methods to control those
> properties and therefore they should not be handled with PRP0001,
> I am happy to be corrected if I am wrong.

Clocks and power supplies should be handled as native ACPI
PowerResources. We are not trying to represent those here.

> When you start matching whole subsystem through PRP0001 and related
> compatible strings you can end up in a situation where DT and ACPI
> FW handling clash and that's why we were opposed to mixing them from
> the beginning, in ARM world if we need a DT we boot with a devicetree.
> 
> If PRP0001 is used for leaf-nodes drivers properties it may work,
> everything else, frankly, is a bit of a gamble you are taking.

The hardware we are describing is exactly the same, only thing that
changes is the firmware interface. So if there is a hardware property
that cannot be discovered automatically it needs to be provided by the
firmware, like ACPI.

If it happens that the property already has an existing DT binding, why
do you think it is gambling to use it instead of inventing the same with
annother name for ACPI?

Re: [RFC 00/15] ACPI graph support

[Lorenzo Pieralisi]

On Wed, Oct 05, 2016 at 06:32:29PM +0300, Mika Westerberg wrote:

[...]

> > FWIW I had a quick look at dts bindings with "compatible = nokia,smia"
> > and related kernel driver.
> > 
> > Those nodes require properties like clocks and power supplies, it
> > seems to me that there are already ACPI PM methods to control those
> > properties and therefore they should not be handled with PRP0001,
> > I am happy to be corrected if I am wrong.
> 
> Clocks and power supplies should be handled as native ACPI
> PowerResources. We are not trying to represent those here.
> 
> > When you start matching whole subsystem through PRP0001 and related
> > compatible strings you can end up in a situation where DT and ACPI
> > FW handling clash and that's why we were opposed to mixing them from
> > the beginning, in ARM world if we need a DT we boot with a devicetree.
> > 
> > If PRP0001 is used for leaf-nodes drivers properties it may work,
> > everything else, frankly, is a bit of a gamble you are taking.
> 
> The hardware we are describing is exactly the same, only thing that
> changes is the firmware interface. So if there is a hardware property
> that cannot be discovered automatically it needs to be provided by the
> firmware, like ACPI.

We certainly agree that HW is the same and that the firmware interfaces
differ, that's why mixing them is not exactly ideal.

> If it happens that the property already has an existing DT binding, why
> do you think it is gambling to use it instead of inventing the same with
> annother name for ACPI?

Do not spin the argument. I am telling you that what I am worried
about is mixing the interfaces because that might trigger kernel
control paths clashing while controlling HW (DT native vs ACPI control
methods). I am pretty sure this won't happen, still, if you do not mind
please post this series (and drivers actually making use of it) to a
wider audience (which includes devicetree and ARM mailing list) and we
will restart the discussion from there.

Thanks,
Lorenzo

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Wed, Oct 5, 2016 at 6:18 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Wed, Oct 05, 2016 at 06:32:29PM +0300, Mika Westerberg wrote:
>
> [...]
>
>> > FWIW I had a quick look at dts bindings with "compatible = nokia,smia"
>> > and related kernel driver.
>> >
>> > Those nodes require properties like clocks and power supplies, it
>> > seems to me that there are already ACPI PM methods to control those
>> > properties and therefore they should not be handled with PRP0001,
>> > I am happy to be corrected if I am wrong.
>>
>> Clocks and power supplies should be handled as native ACPI
>> PowerResources. We are not trying to represent those here.
>>
>> > When you start matching whole subsystem through PRP0001 and related
>> > compatible strings you can end up in a situation where DT and ACPI
>> > FW handling clash and that's why we were opposed to mixing them from
>> > the beginning, in ARM world if we need a DT we boot with a devicetree.
>> >
>> > If PRP0001 is used for leaf-nodes drivers properties it may work,
>> > everything else, frankly, is a bit of a gamble you are taking.
>>
>> The hardware we are describing is exactly the same, only thing that
>> changes is the firmware interface. So if there is a hardware property
>> that cannot be discovered automatically it needs to be provided by the
>> firmware, like ACPI.
>
> We certainly agree that HW is the same and that the firmware interfaces
> differ, that's why mixing them is not exactly ideal.
>
>> If it happens that the property already has an existing DT binding, why
>> do you think it is gambling to use it instead of inventing the same with
>> annother name for ACPI?
>
> Do not spin the argument. I am telling you that what I am worried
> about is mixing the interfaces because that might trigger kernel
> control paths clashing while controlling HW (DT native vs ACPI control
> methods).

That would have been possible, had the *kernel* supported _DSD
properties that could have conflict with native ACPI stuff at the
framework level.  Yet, it doesn't and there are no plans to add any
support like that to it I'm aware of.

So far, we have been careful enough to avoid supporting any _DSD
properties that may potentially conflict with ACPI-defined HW control,
this way or another, and as long as we continue to do that, all should
be fine.  So the way to go, to me, is not to reject support for any
kind of generic _DSD properties that follow DT bindings, but to look
at every case carefully and see if they conflict with ACPI-defined HW
control in any way.  If they don't, I see no reason for not supporting
them.

> I am pretty sure this won't happen, still, if you do not mind
> please post this series (and drivers actually making use of it) to a
> wider audience (which includes devicetree and ARM mailing list) and we
> will restart the discussion from there.

I think that the target subsystem (V4L in this particular case) should
be notified of this in the first place as they are the user of the
bindings in question.

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Lorenzo Pieralisi]

On Wed, Oct 05, 2016 at 10:33:47PM +0200, Rafael J. Wysocki wrote:
> On Wed, Oct 5, 2016 at 6:18 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > On Wed, Oct 05, 2016 at 06:32:29PM +0300, Mika Westerberg wrote:
> >
> > [...]
> >
> >> > FWIW I had a quick look at dts bindings with "compatible = nokia,smia"
> >> > and related kernel driver.
> >> >
> >> > Those nodes require properties like clocks and power supplies, it
> >> > seems to me that there are already ACPI PM methods to control those
> >> > properties and therefore they should not be handled with PRP0001,
> >> > I am happy to be corrected if I am wrong.
> >>
> >> Clocks and power supplies should be handled as native ACPI
> >> PowerResources. We are not trying to represent those here.
> >>
> >> > When you start matching whole subsystem through PRP0001 and related
> >> > compatible strings you can end up in a situation where DT and ACPI
> >> > FW handling clash and that's why we were opposed to mixing them from
> >> > the beginning, in ARM world if we need a DT we boot with a devicetree.
> >> >
> >> > If PRP0001 is used for leaf-nodes drivers properties it may work,
> >> > everything else, frankly, is a bit of a gamble you are taking.
> >>
> >> The hardware we are describing is exactly the same, only thing that
> >> changes is the firmware interface. So if there is a hardware property
> >> that cannot be discovered automatically it needs to be provided by the
> >> firmware, like ACPI.
> >
> > We certainly agree that HW is the same and that the firmware interfaces
> > differ, that's why mixing them is not exactly ideal.
> >
> >> If it happens that the property already has an existing DT binding, why
> >> do you think it is gambling to use it instead of inventing the same with
> >> annother name for ACPI?
> >
> > Do not spin the argument. I am telling you that what I am worried
> > about is mixing the interfaces because that might trigger kernel
> > control paths clashing while controlling HW (DT native vs ACPI control
> > methods).
> 
> That would have been possible, had the *kernel* supported _DSD
> properties that could have conflict with native ACPI stuff at the
> framework level.  Yet, it doesn't and there are no plans to add any
> support like that to it I'm aware of.
> 
> So far, we have been careful enough to avoid supporting any _DSD
> properties that may potentially conflict with ACPI-defined HW control,
> this way or another, and as long as we continue to do that, all should
> be fine.  So the way to go, to me, is not to reject support for any
> kind of generic _DSD properties that follow DT bindings, but to look
> at every case carefully and see if they conflict with ACPI-defined HW
> control in any way.  If they don't, I see no reason for not supporting
> them.

I agree with that, I am less optimistic at how we can vet code
once the fwnode API will allow us to handle DT in ACPI with same
capabilities as native DT, because let's face it, by augmenting
the fwnode API through patches like this we are reaching DT kernel
handling equivalence.

We are coming to this from opposite directions: x86, with FW people
used to writing ACPI FW (and therefore power resources, etc. usage)
and ARM FW developers, who could be very very tempted to reuse the
same DT properties used for clocks/voltage and whatnot into PRP0001
equivalent completely overriding ACPI control methods and we can't
argue it is not ACPI standard anymore (or can we ?).

As you said this can only happen once the fwnode API usage trickles
into the respective subsystems. Can we prevent it ? I hope so and
we are keeping an eye on that too (that's the reason why I asked
Mika to widen the audience, BTW), but that's the *only* way to
prevent this FW bindings mix-up and it is almost impossible to
vet all code getting into subsystems IMHO.

I am trying to understand why x86 wants to do this, please understand
our point of view too, we do not want to block progress we want to
prevent a mess.

> > I am pretty sure this won't happen, still, if you do not mind
> > please post this series (and drivers actually making use of it) to a
> > wider audience (which includes devicetree and ARM mailing list) and we
> > will restart the discussion from there.
> 
> I think that the target subsystem (V4L in this particular case) should
> be notified of this in the first place as they are the user of the
> bindings in question.

Exactly. This patchset should at least reach DT people and the respective
subsystem maintainers, I do not think that's too much to ask.

Thanks,
Lorenzo

Re: [RFC 00/15] ACPI graph support

[Mika Westerberg]

On Thu, Oct 06, 2016 at 09:57:03AM +0100, Lorenzo Pieralisi wrote:
> I am trying to understand why x86 wants to do this, please understand
> our point of view too, we do not want to block progress we want to
> prevent a mess.

One reason is that we have boards like Joule where developers are
allowed to connect different peripherals using buses such as I2C and SPI
where there is no native enumeration mechanism. This includes camera
sensors and related so there needs to be a way for a developer to
describe this in ACPI. Just as can be done when using ARM and DT.

Re: [RFC 00/15] ACPI graph support

[Lorenzo Pieralisi]

On Thu, Oct 06, 2016 at 12:11:33PM +0300, Mika Westerberg wrote:
> On Thu, Oct 06, 2016 at 09:57:03AM +0100, Lorenzo Pieralisi wrote:
> > I am trying to understand why x86 wants to do this, please understand
> > our point of view too, we do not want to block progress we want to
> > prevent a mess.
> 
> One reason is that we have boards like Joule where developers are
> allowed to connect different peripherals using buses such as I2C and SPI
> where there is no native enumeration mechanism. This includes camera
> sensors and related so there needs to be a way for a developer to
> describe this in ACPI. Just as can be done when using ARM and DT.

I am sorry I think we are at loggerheads on this. If you need a DT boot
with a DT, I could have converted all the ACPI tables to DT nodes on
ARM64 if I followed your reasoning (because we could not boot with ACPI
till relatively recently), we did not do it because ACPI and DT are
different specifications, incompatible with one another and governed by
different entities in a *very* different way.

You are saying that just re-using leaf nodes properties (well, it
is not just leaf-nodes properties any longer, is it ?) is just
fine; I(We) am not convinced, time will tell. In the interim
please notify the respective subsystems maintainers and DT people
of this patch intentions, again I hope I am not asking too much.

Thanks,
Lorenzo

Re: [RFC 00/15] ACPI graph support

[Mika Westerberg]

On Thu, Oct 06, 2016 at 10:57:39AM +0100, Lorenzo Pieralisi wrote:
> On Thu, Oct 06, 2016 at 12:11:33PM +0300, Mika Westerberg wrote:
> > On Thu, Oct 06, 2016 at 09:57:03AM +0100, Lorenzo Pieralisi wrote:
> > > I am trying to understand why x86 wants to do this, please understand
> > > our point of view too, we do not want to block progress we want to
> > > prevent a mess.
> > 
> > One reason is that we have boards like Joule where developers are
> > allowed to connect different peripherals using buses such as I2C and SPI
> > where there is no native enumeration mechanism. This includes camera
> > sensors and related so there needs to be a way for a developer to
> > describe this in ACPI. Just as can be done when using ARM and DT.
> 
> I am sorry I think we are at loggerheads on this. If you need a DT boot
> with a DT, I could have converted all the ACPI tables to DT nodes on
> ARM64 if I followed your reasoning (because we could not boot with ACPI
> till relatively recently), we did not do it because ACPI and DT are
> different specifications, incompatible with one another and governed by
> different entities in a *very* different way.

I don't need a DT, I need that my existing firmware (in this case BIOS)
can describe camera device(s) and the OS can take advantage of this,
preferably with minimal changes to the drivers.

Currently there is no way in ACPI specification to do that.

> You are saying that just re-using leaf nodes properties (well, it
> is not just leaf-nodes properties any longer, is it ?) is just
> fine;

Yes.

Not sure where this remote-endpoint thing belongs, to be honest. It is
used inside V4L2 to determine which parts of the system make up a camera
device so in that sense it is "leaf-node" property but it is not limited
to just V4L2.

> I(We) am not convinced, time will tell. In the interim
> please notify the respective subsystems maintainers and DT people
> of this patch intentions, again I hope I am not asking too much.

I think Sakari already did that for V4L2. Next version will include all
the relevant mailing lists.

Re: [RFC 00/15] ACPI graph support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]

On Thu, Oct 06, 2016 at 02:19:22PM +0300, Mika Westerberg wrote:

> I don't need a DT, I need that my existing firmware (in this case BIOS)
> can describe camera device(s) and the OS can take advantage of this,
> preferably with minimal changes to the drivers.

> Currently there is no way in ACPI specification to do that.

That's not exactly true - the way Windows handles audio devices (which
follow a similar pattern) is to register the control interfaces of the
individual components of the system using existing bindings and then
bind them together with a driver that matches the board level
identification.  This isn't super awesome but it's definitely a thing
you can do.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Thu, Oct 6, 2016 at 5:31 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Oct 06, 2016 at 02:19:22PM +0300, Mika Westerberg wrote:
>
>> I don't need a DT, I need that my existing firmware (in this case BIOS)
>> can describe camera device(s) and the OS can take advantage of this,
>> preferably with minimal changes to the drivers.
>
>> Currently there is no way in ACPI specification to do that.
>
> That's not exactly true - the way Windows handles audio devices (which
> follow a similar pattern) is to register the control interfaces of the
> individual components of the system using existing bindings and then
> bind them together with a driver that matches the board level
> identification.  This isn't super awesome but it's definitely a thing
> you can do.

But that would mean writing new Linux code to support hardware that
already is supported by the Linux kernel.

That would be a bit like saying "We have a driver for this, but you
are not allowed to use it, because your platform is not a DT one".
That doesn't sound good to me, honestly.

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1296 bytes --]

On Thu, Oct 06, 2016 at 06:00:39PM +0200, Rafael J. Wysocki wrote:
> On Thu, Oct 6, 2016 at 5:31 PM, Mark Brown <broonie@kernel.org> wrote:
> > On Thu, Oct 06, 2016 at 02:19:22PM +0300, Mika Westerberg wrote:

> >> I don't need a DT, I need that my existing firmware (in this case BIOS)
> >> can describe camera device(s) and the OS can take advantage of this,
> >> preferably with minimal changes to the drivers.

> >> Currently there is no way in ACPI specification to do that.

> > That's not exactly true - the way Windows handles audio devices (which
> > follow a similar pattern) is to register the control interfaces of the
> > individual components of the system using existing bindings and then
> > bind them together with a driver that matches the board level
> > identification.  This isn't super awesome but it's definitely a thing
> > you can do.

> But that would mean writing new Linux code to support hardware that
> already is supported by the Linux kernel.

> That would be a bit like saying "We have a driver for this, but you
> are not allowed to use it, because your platform is not a DT one".
> That doesn't sound good to me, honestly.

Well, no - it just means that you need a bit of translation code to glue
things together.  There's no need to replace the entire driver.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Thu, Oct 6, 2016 at 6:14 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Oct 06, 2016 at 06:00:39PM +0200, Rafael J. Wysocki wrote:
>> On Thu, Oct 6, 2016 at 5:31 PM, Mark Brown <broonie@kernel.org> wrote:
>> > On Thu, Oct 06, 2016 at 02:19:22PM +0300, Mika Westerberg wrote:
>
>> >> I don't need a DT, I need that my existing firmware (in this case BIOS)
>> >> can describe camera device(s) and the OS can take advantage of this,
>> >> preferably with minimal changes to the drivers.
>
>> >> Currently there is no way in ACPI specification to do that.
>
>> > That's not exactly true - the way Windows handles audio devices (which
>> > follow a similar pattern) is to register the control interfaces of the
>> > individual components of the system using existing bindings and then
>> > bind them together with a driver that matches the board level
>> > identification.  This isn't super awesome but it's definitely a thing
>> > you can do.
>
>> But that would mean writing new Linux code to support hardware that
>> already is supported by the Linux kernel.
>
>> That would be a bit like saying "We have a driver for this, but you
>> are not allowed to use it, because your platform is not a DT one".
>> That doesn't sound good to me, honestly.
>
> Well, no - it just means that you need a bit of translation code to glue
> things together.  There's no need to replace the entire driver.

Fair enough, but also depending on what is provided by the ACPI tables
and what isn't.

In some cases kernel frameworks expect information that - without _DSD
properties - is not available in the ACPI tables in any form, but can
be provided by DT, so the driver would need to contain some sort of a
static table mapping device IDs to the missing information which would
not be fantastic.

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Mark Rutland]

On Thu, Oct 06, 2016 at 06:00:39PM +0200, Rafael J. Wysocki wrote:
> On Thu, Oct 6, 2016 at 5:31 PM, Mark Brown <broonie@kernel.org> wrote:
> > On Thu, Oct 06, 2016 at 02:19:22PM +0300, Mika Westerberg wrote:
> >
> >> I don't need a DT, I need that my existing firmware (in this case BIOS)
> >> can describe camera device(s) and the OS can take advantage of this,
> >> preferably with minimal changes to the drivers.
> >
> >> Currently there is no way in ACPI specification to do that.
> >
> > That's not exactly true - the way Windows handles audio devices (which
> > follow a similar pattern) is to register the control interfaces of the
> > individual components of the system using existing bindings and then
> > bind them together with a driver that matches the board level
> > identification.  This isn't super awesome but it's definitely a thing
> > you can do.

... so at least one OS *already* has an OS-specific bodge around what is a
clear ACPI deficiency...

> But that would mean writing new Linux code to support hardware that
> already is supported by the Linux kernel.
> 
> That would be a bit like saying "We have a driver for this, but you
> are not allowed to use it, because your platform is not a DT one".
> That doesn't sound good to me, honestly.

... and none of us like any of the proposed OS-specific bodges.

So why is no-one trying to solve the issue? Why has this not been raised as an
issue to be solved by the ACPI spec?

Thanks,
Mark.

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Tuesday, October 11, 2016 01:44:20 PM Mark Rutland wrote:
> On Thu, Oct 06, 2016 at 06:00:39PM +0200, Rafael J. Wysocki wrote:
> > On Thu, Oct 6, 2016 at 5:31 PM, Mark Brown <broonie@kernel.org> wrote:
> > > On Thu, Oct 06, 2016 at 02:19:22PM +0300, Mika Westerberg wrote:
> > >
> > >> I don't need a DT, I need that my existing firmware (in this case BIOS)
> > >> can describe camera device(s) and the OS can take advantage of this,
> > >> preferably with minimal changes to the drivers.
> > >
> > >> Currently there is no way in ACPI specification to do that.
> > >
> > > That's not exactly true - the way Windows handles audio devices (which
> > > follow a similar pattern) is to register the control interfaces of the
> > > individual components of the system using existing bindings and then
> > > bind them together with a driver that matches the board level
> > > identification.  This isn't super awesome but it's definitely a thing
> > > you can do.
> 
> ... so at least one OS *already* has an OS-specific bodge around what is a
> clear ACPI deficiency...
> 
> > But that would mean writing new Linux code to support hardware that
> > already is supported by the Linux kernel.
> > 
> > That would be a bit like saying "We have a driver for this, but you
> > are not allowed to use it, because your platform is not a DT one".
> > That doesn't sound good to me, honestly.
> 
> ... and none of us like any of the proposed OS-specific bodges.

I'm taking this as your personal opinion.

> So why is no-one trying to solve the issue? Why has this not been raised as an
> issue to be solved by the ACPI spec?

How exactly do you think it could be solved there?

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Thu, Oct 6, 2016 at 11:57 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Thu, Oct 06, 2016 at 12:11:33PM +0300, Mika Westerberg wrote:
>> On Thu, Oct 06, 2016 at 09:57:03AM +0100, Lorenzo Pieralisi wrote:
>> > I am trying to understand why x86 wants to do this, please understand
>> > our point of view too, we do not want to block progress we want to
>> > prevent a mess.
>>
>> One reason is that we have boards like Joule where developers are
>> allowed to connect different peripherals using buses such as I2C and SPI
>> where there is no native enumeration mechanism. This includes camera
>> sensors and related so there needs to be a way for a developer to
>> describe this in ACPI. Just as can be done when using ARM and DT.
>
> I am sorry I think we are at loggerheads on this. If you need a DT boot
> with a DT, I could have converted all the ACPI tables to DT nodes on
> ARM64 if I followed your reasoning (because we could not boot with ACPI
> till relatively recently), we did not do it because ACPI and DT are
> different specifications, incompatible with one another and governed by
> different entities in a *very* different way.

You are talking about core code and Mika is talking about device drivers.

As far as the core code is concerned, I agree with you.  As far as
device drivers are concerned, I agree with Mika.

You are both right. :-)

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1139 bytes --]

On Thu, Oct 06, 2016 at 12:11:33PM +0300, Mika Westerberg wrote:

> One reason is that we have boards like Joule where developers are
> allowed to connect different peripherals using buses such as I2C and SPI
> where there is no native enumeration mechanism. This includes camera
> sensors and related so there needs to be a way for a developer to
> describe this in ACPI. Just as can be done when using ARM and DT.

One of my biggest concerns with this approach is that I'm really not
clear to me that that this has broad buy in from the x86/ACPI community
and therefore that we might end up needing to support several different
styles of ACPI bindings.  Reuse would be great but it can be confusing
if there's multiple different styles of bindings in use at the same
time.

The audio systems that have this issue (which include production laptops
and tablets with both Windows and ChromeOS) don't seem to be showing
much interest in reusing any of the DT work beyond the device level
properties and I didn't think the OS level support for using _DSD in
Windows was great.  There were also the pinctrl bindings which had some
issues too.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC 00/15] ACPI graph support

[Mika Westerberg]

On Thu, Oct 06, 2016 at 12:40:58PM +0200, Mark Brown wrote:
> On Thu, Oct 06, 2016 at 12:11:33PM +0300, Mika Westerberg wrote:
> 
> > One reason is that we have boards like Joule where developers are
> > allowed to connect different peripherals using buses such as I2C and SPI
> > where there is no native enumeration mechanism. This includes camera
> > sensors and related so there needs to be a way for a developer to
> > describe this in ACPI. Just as can be done when using ARM and DT.
> 
> One of my biggest concerns with this approach is that I'm really not
> clear to me that that this has broad buy in from the x86/ACPI community
> and therefore that we might end up needing to support several different
> styles of ACPI bindings.  Reuse would be great but it can be confusing
> if there's multiple different styles of bindings in use at the same
> time.

Yes, that's possible unfortunately. And we can't force people to use
whatever _DSD properties we came up. Instead they use whatever is
convenient for them, even if it is already available in the _DSD
properties database.

> The audio systems that have this issue (which include production laptops
> and tablets with both Windows and ChromeOS) don't seem to be showing
> much interest in reusing any of the DT work beyond the device level
> properties and I didn't think the OS level support for using _DSD in
> Windows was great.  There were also the pinctrl bindings which had some
> issues too.

Right and it does not always follow DT work even on device level.

Here is an example what Windows is supporting. They use _DSD device
properties but it is completely different to what Linux and DT use:

https://msdn.microsoft.com/en-us/windows/uwp/devices-sensors/enable-usermode-access

We have had all that stuff in DT already but they chose not to use it.

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Thu, Oct 6, 2016 at 12:40 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Oct 06, 2016 at 12:11:33PM +0300, Mika Westerberg wrote:
>
>> One reason is that we have boards like Joule where developers are
>> allowed to connect different peripherals using buses such as I2C and SPI
>> where there is no native enumeration mechanism. This includes camera
>> sensors and related so there needs to be a way for a developer to
>> describe this in ACPI. Just as can be done when using ARM and DT.
>
> One of my biggest concerns with this approach is that I'm really not
> clear to me that that this has broad buy in from the x86/ACPI community
> and therefore that we might end up needing to support several different
> styles of ACPI bindings.  Reuse would be great but it can be confusing
> if there's multiple different styles of bindings in use at the same
> time.

My view on that is a bit different.

Even if there's more than one style in use, they all can be supported
at the same time.  Avoiding confusion is only a matter of prioritizing
them, then.

It should be clear which style is the default one, which is going to
be taken into consideration next and so on.  If that is made clear, I
don't see a big problem here, at least in principle.

> The audio systems that have this issue (which include production laptops
> and tablets with both Windows and ChromeOS) don't seem to be showing
> much interest in reusing any of the DT work beyond the device level
> properties and I didn't think the OS level support for using _DSD in
> Windows was great.  There were also the pinctrl bindings which had some
> issues too.

The pinctrl thing is being dealt with (and not in a way of adding
pinctrl _DSD properties support to the kernel FWIW).  Other things
potentially conflicting with ACPI-defined HW control should be dealt
with analogously.

The point here is that some things just don't conflict with
ACPI-defined HW control even potentially and they can be handled with
the help of _DSD properties just fine.  Thus, there are no technical
obstacles to do that, at least not in principle.

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2207 bytes --]

On Thu, Oct 06, 2016 at 03:15:36PM +0200, Rafael J. Wysocki wrote:
> On Thu, Oct 6, 2016 at 12:40 PM, Mark Brown <broonie@kernel.org> wrote:

> > One of my biggest concerns with this approach is that I'm really not
> > clear to me that that this has broad buy in from the x86/ACPI community
> > and therefore that we might end up needing to support several different
> > styles of ACPI bindings.  Reuse would be great but it can be confusing
> > if there's multiple different styles of bindings in use at the same
> > time.

> My view on that is a bit different.

> Even if there's more than one style in use, they all can be supported
> at the same time.  Avoiding confusion is only a matter of prioritizing
> them, then.

> It should be clear which style is the default one, which is going to
> be taken into consideration next and so on.  If that is made clear, I
> don't see a big problem here, at least in principle.

My concern isn't just people using multiple styles, it's also people
trying to mix and match.  Sometimes that will work well enough and it's
fine but it can go wrong.

> The pinctrl thing is being dealt with (and not in a way of adding
> pinctrl _DSD properties support to the kernel FWIW).  Other things
> potentially conflicting with ACPI-defined HW control should be dealt
> with analogously.

Right, I was mentioning it mainly as an example of the general pattern
of ACPI bindings springing up without coordination rather than as a
specific problem that needs resolution.

> The point here is that some things just don't conflict with
> ACPI-defined HW control even potentially and they can be handled with
> the help of _DSD properties just fine.  Thus, there are no technical
> obstacles to do that, at least not in principle.

It feels like we should be aiming for a higher bar with defining things
like this than simple technical possibility - my fear is that we end up
with a mess down the line with people being far too gung ho about
defining new bindings without trying to work on standards.  Perhaps I'm
just worrying too much here but I'm not sure there's enough
communication going on.  The private nature of ACPI standardization
discussion doesn't help here of course.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Thu, Oct 6, 2016 at 5:23 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Oct 06, 2016 at 03:15:36PM +0200, Rafael J. Wysocki wrote:
>> On Thu, Oct 6, 2016 at 12:40 PM, Mark Brown <broonie@kernel.org> wrote:
>
>> > One of my biggest concerns with this approach is that I'm really not
>> > clear to me that that this has broad buy in from the x86/ACPI community
>> > and therefore that we might end up needing to support several different
>> > styles of ACPI bindings.  Reuse would be great but it can be confusing
>> > if there's multiple different styles of bindings in use at the same
>> > time.
>
>> My view on that is a bit different.
>
>> Even if there's more than one style in use, they all can be supported
>> at the same time.  Avoiding confusion is only a matter of prioritizing
>> them, then.
>
>> It should be clear which style is the default one, which is going to
>> be taken into consideration next and so on.  If that is made clear, I
>> don't see a big problem here, at least in principle.
>
> My concern isn't just people using multiple styles, it's also people
> trying to mix and match.  Sometimes that will work well enough and it's
> fine but it can go wrong.

Yes, it can.

I'm not against being careful, but I'd rather not reject things as a
matter of principle.

>> The pinctrl thing is being dealt with (and not in a way of adding
>> pinctrl _DSD properties support to the kernel FWIW).  Other things
>> potentially conflicting with ACPI-defined HW control should be dealt
>> with analogously.
>
> Right, I was mentioning it mainly as an example of the general pattern
> of ACPI bindings springing up without coordination rather than as a
> specific problem that needs resolution.
>
>> The point here is that some things just don't conflict with
>> ACPI-defined HW control even potentially and they can be handled with
>> the help of _DSD properties just fine.  Thus, there are no technical
>> obstacles to do that, at least not in principle.
>
> It feels like we should be aiming for a higher bar with defining things
> like this than simple technical possibility - my fear is that we end up
> with a mess down the line with people being far too gung ho about
> defining new bindings without trying to work on standards.

But here we are talking about using bindings that (a) have already
been defined in DT and (b) are actively used by the existing code.

I guess you mean using existing DT bindings instead of working on
standards, but let's face it, DT bindings de facto are the standard in
some parts of the kernel.

> Perhaps I'm just worrying too much here but I'm not sure there's enough
> communication going on.  The private nature of ACPI standardization
> discussion doesn't help here of course.

Agreed, and that doesn't apply to ACPI only, of course.

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Sudeep Holla]

On 06/10/16 16:23, Mark Brown wrote:
> On Thu, Oct 06, 2016 at 03:15:36PM +0200, Rafael J. Wysocki wrote:
>> On Thu, Oct 6, 2016 at 12:40 PM, Mark Brown <broonie@kernel.org> wrote:
>


[...]

>> The point here is that some things just don't conflict with
>> ACPI-defined HW control even potentially and they can be handled with
>> the help of _DSD properties just fine.  Thus, there are no technical
>> obstacles to do that, at least not in principle.
>
> It feels like we should be aiming for a higher bar with defining things
> like this than simple technical possibility - my fear is that we end up
> with a mess down the line with people being far too gung ho about
> defining new bindings without trying to work on standards.  Perhaps I'm
> just worrying too much here but I'm not sure there's enough
> communication going on.  The private nature of ACPI standardization
> discussion doesn't help here of course.
>

+1

I too expressed similar concern, as you say may be that's just too much
over thinking. Or not, because every-time we think ARM vendors should
not do something stupid like this, they end up doing exactly same thing
in no time :))

-- 
Regards,
Sudeep

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Thu, Oct 6, 2016 at 10:57 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Wed, Oct 05, 2016 at 10:33:47PM +0200, Rafael J. Wysocki wrote:
>> On Wed, Oct 5, 2016 at 6:18 PM, Lorenzo Pieralisi
>> <lorenzo.pieralisi@arm.com> wrote:
>> > On Wed, Oct 05, 2016 at 06:32:29PM +0300, Mika Westerberg wrote:
>> >
>> > [...]
>> >
>> >> > FWIW I had a quick look at dts bindings with "compatible = nokia,smia"
>> >> > and related kernel driver.
>> >> >
>> >> > Those nodes require properties like clocks and power supplies, it
>> >> > seems to me that there are already ACPI PM methods to control those
>> >> > properties and therefore they should not be handled with PRP0001,
>> >> > I am happy to be corrected if I am wrong.
>> >>
>> >> Clocks and power supplies should be handled as native ACPI
>> >> PowerResources. We are not trying to represent those here.
>> >>
>> >> > When you start matching whole subsystem through PRP0001 and related
>> >> > compatible strings you can end up in a situation where DT and ACPI
>> >> > FW handling clash and that's why we were opposed to mixing them from
>> >> > the beginning, in ARM world if we need a DT we boot with a devicetree.
>> >> >
>> >> > If PRP0001 is used for leaf-nodes drivers properties it may work,
>> >> > everything else, frankly, is a bit of a gamble you are taking.
>> >>
>> >> The hardware we are describing is exactly the same, only thing that
>> >> changes is the firmware interface. So if there is a hardware property
>> >> that cannot be discovered automatically it needs to be provided by the
>> >> firmware, like ACPI.
>> >
>> > We certainly agree that HW is the same and that the firmware interfaces
>> > differ, that's why mixing them is not exactly ideal.
>> >
>> >> If it happens that the property already has an existing DT binding, why
>> >> do you think it is gambling to use it instead of inventing the same with
>> >> annother name for ACPI?
>> >
>> > Do not spin the argument. I am telling you that what I am worried
>> > about is mixing the interfaces because that might trigger kernel
>> > control paths clashing while controlling HW (DT native vs ACPI control
>> > methods).
>>
>> That would have been possible, had the *kernel* supported _DSD
>> properties that could have conflict with native ACPI stuff at the
>> framework level.  Yet, it doesn't and there are no plans to add any
>> support like that to it I'm aware of.
>>
>> So far, we have been careful enough to avoid supporting any _DSD
>> properties that may potentially conflict with ACPI-defined HW control,
>> this way or another, and as long as we continue to do that, all should
>> be fine.  So the way to go, to me, is not to reject support for any
>> kind of generic _DSD properties that follow DT bindings, but to look
>> at every case carefully and see if they conflict with ACPI-defined HW
>> control in any way.  If they don't, I see no reason for not supporting
>> them.
>
> I agree with that, I am less optimistic at how we can vet code
> once the fwnode API will allow us to handle DT in ACPI with same
> capabilities as native DT, because let's face it, by augmenting
> the fwnode API through patches like this we are reaching DT kernel
> handling equivalence.
>
> We are coming to this from opposite directions: x86, with FW people
> used to writing ACPI FW (and therefore power resources, etc. usage)
> and ARM FW developers, who could be very very tempted to reuse the
> same DT properties used for clocks/voltage and whatnot into PRP0001
> equivalent completely overriding ACPI control methods and we can't
> argue it is not ACPI standard anymore (or can we ?).

Yes and no, depending on what angle you are looking at it.

On the one hand, it is proper ASL/AML using constructs defined by the
spec and nothing else.  From that perspective, it is following the
standard.

OTOH, the interpretation of it is not defined by the spec proper and
it is Linux-specific, so other OSes won't use this information even if
it is there in your ACPI tables.

> As you said this can only happen once the fwnode API usage trickles
> into the respective subsystems. Can we prevent it ? I hope so and
> we are keeping an eye on that too (that's the reason why I asked
> Mika to widen the audience, BTW), but that's the *only* way to
> prevent this FW bindings mix-up and it is almost impossible to
> vet all code getting into subsystems IMHO.
>
> I am trying to understand why x86 wants to do this, please understand
> our point of view too, we do not want to block progress we want to
> prevent a mess.

It is not "x86" who wants to do that.

It is people who work on support for boards with ACPI firmware and
containing devices that in Linux are handled by DT-centric code.

Of course, the reason why that code is DT-centric is because it was
developed on systems using DT and there were no uses on ACPI-based
systems for it back then.  Still, it is DT-centric as a matter of fact
and *something* has to be done in order to make it work with ACPI.

Basically, there are two choices.  One would be to (a) make ACPI cover
the cases in question and (b) implement code in accordance with that,
but the problem with this approach is the timing (the boards are here
today already).  Another one is to make it possible to provide the
Linux code in question with information it expects from DT, but using
the ACPI interface, limited to information that *can* be provided this
way, and that's where this is going.

And it is not an option for those boards to use DT in the firmware.

>> > I am pretty sure this won't happen, still, if you do not mind
>> > please post this series (and drivers actually making use of it) to a
>> > wider audience (which includes devicetree and ARM mailing list) and we
>> > will restart the discussion from there.
>>
>> I think that the target subsystem (V4L in this particular case) should
>> be notified of this in the first place as they are the user of the
>> bindings in question.
>
> Exactly. This patchset should at least reach DT people and the respective
> subsystem maintainers, I do not think that's too much to ask.

Fair enough.

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2086 bytes --]

On Thu, Oct 06, 2016 at 02:26:50PM +0200, Rafael J. Wysocki wrote:
> On Thu, Oct 6, 2016 at 10:57 AM, Lorenzo Pieralisi

> > I am trying to understand why x86 wants to do this, please understand
> > our point of view too, we do not want to block progress we want to
> > prevent a mess.

> It is not "x86" who wants to do that.

> It is people who work on support for boards with ACPI firmware and
> containing devices that in Linux are handled by DT-centric code.

Yes, it's important to remember that there's a whole world of other
people using ACPI on x86 who are doing different things.  I have to say
I'm not sure how DT centric we really are, there's a lot of things that
have been around since before DT.

> Of course, the reason why that code is DT-centric is because it was
> developed on systems using DT and there were no uses on ACPI-based
> systems for it back then.  Still, it is DT-centric as a matter of fact
> and *something* has to be done in order to make it work with ACPI.

Personally I don't have that big a concern around per device
properties other than the need to go through yet another round of
churn for them (though it is just mechanical which will make it less
painful).  I do worry when it goes to generic things and inter-device
relationships.

> And it is not an option for those boards to use DT in the firmware.

There's nothing stopping these systems defining a DSD that contains a
DTB which overrides some or all of the ACPI if the system supports it
(or otherwise providing both system descriptions).  The two can coexist
happily enough as arm64 has shown and it seems like it ought to save a
whole lot of work especially around the bits that need inter device
links and are hence need some new ACPI bindings defining.

I can see that removing ACPI entirely would present serious difficulties
but it's less clear to me how much is really gained by having an
embedded Linux specific ACPI variant over having some Linux specific
data in ACPI that happens to be parsable as DT.  The circumstances that
the two platforms face appear to be very similar.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Thu, Oct 6, 2016 at 11:37 PM, Mark Brown <broonie@kernel.org> wrote:
> On Thu, Oct 06, 2016 at 02:26:50PM +0200, Rafael J. Wysocki wrote:
>> On Thu, Oct 6, 2016 at 10:57 AM, Lorenzo Pieralisi
>
>> > I am trying to understand why x86 wants to do this, please understand
>> > our point of view too, we do not want to block progress we want to
>> > prevent a mess.
>
>> It is not "x86" who wants to do that.
>
>> It is people who work on support for boards with ACPI firmware and
>> containing devices that in Linux are handled by DT-centric code.
>
> Yes, it's important to remember that there's a whole world of other
> people using ACPI on x86 who are doing different things.  I have to say
> I'm not sure how DT centric we really are, there's a lot of things that
> have been around since before DT.

When I said "DT-centric" I really meant "full of of_* calls retrieving
data from a DT".

>> Of course, the reason why that code is DT-centric is because it was
>> developed on systems using DT and there were no uses on ACPI-based
>> systems for it back then.  Still, it is DT-centric as a matter of fact
>> and *something* has to be done in order to make it work with ACPI.
>
> Personally I don't have that big a concern around per device
> properties other than the need to go through yet another round of
> churn for them (though it is just mechanical which will make it less
> painful).  I do worry when it goes to generic things and inter-device
> relationships.

Well, that was my first reaction to this series, but then I thought
"Let's see what can go wrong with this specifically" and then I
couldn't find anything.

If you see something like that, please let me know, because I may be
overlooking it, but otherwise I would prefer to focus on the technical
side of things instead of wast^Hspending time on theoretical worries.

>> And it is not an option for those boards to use DT in the firmware.
>
> There's nothing stopping these systems defining a DSD that contains a
> DTB which overrides some or all of the ACPI if the system supports it
> (or otherwise providing both system descriptions).  The two can coexist
> happily enough as arm64 has shown

I'm not sure to what extent it has shown that and even so, it doesn't
mean this is a good idea.

> and it seems like it ought to save a
> whole lot of work especially around the bits that need inter device
> links and are hence need some new ACPI bindings defining.

There is at least one major problem with this approach.  If the ACPI
part needs to point to anything in the DTB or if the DTB part needs to
point to the ACPI part outside of it, there's no clean way that could
be done.  I actually am not aware of any way whatever, but if there
are some, I kind of don't expect them to be pretty.

> I can see that removing ACPI entirely would present serious difficulties
> but it's less clear to me how much is really gained by having an
> embedded Linux specific ACPI variant over having some Linux specific
> data in ACPI that happens to be parsable as DT.  The circumstances that
> the two platforms face appear to be very similar.

I'm not really sure what you mean here.

_DSD properties can convey the same information as DT in the same
layout.  The only difference is that DT uses phandles to point to
things and _DSD properties use ACPI namespace paths for this purpose.
That actually is where code changes need to be made, the rest is
already there.

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Tue, Oct 11, 2016 at 1:44 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Thu, Oct 6, 2016 at 11:37 PM, Mark Brown <broonie@kernel.org> wrote:
>> On Thu, Oct 06, 2016 at 02:26:50PM +0200, Rafael J. Wysocki wrote:
>>> On Thu, Oct 6, 2016 at 10:57 AM, Lorenzo Pieralisi
>>
>>> > I am trying to understand why x86 wants to do this, please understand
>>> > our point of view too, we do not want to block progress we want to
>>> > prevent a mess.
>>
>>> It is not "x86" who wants to do that.
>>
>>> It is people who work on support for boards with ACPI firmware and
>>> containing devices that in Linux are handled by DT-centric code.
>>
>> Yes, it's important to remember that there's a whole world of other
>> people using ACPI on x86 who are doing different things.  I have to say
>> I'm not sure how DT centric we really are, there's a lot of things that
>> have been around since before DT.
>
> When I said "DT-centric" I really meant "full of of_* calls retrieving
> data from a DT".
>
>>> Of course, the reason why that code is DT-centric is because it was
>>> developed on systems using DT and there were no uses on ACPI-based
>>> systems for it back then.  Still, it is DT-centric as a matter of fact
>>> and *something* has to be done in order to make it work with ACPI.
>>
>> Personally I don't have that big a concern around per device
>> properties other than the need to go through yet another round of
>> churn for them (though it is just mechanical which will make it less
>> painful).  I do worry when it goes to generic things and inter-device
>> relationships.
>
> Well, that was my first reaction to this series, but then I thought
> "Let's see what can go wrong with this specifically" and then I
> couldn't find anything.
>
> If you see something like that, please let me know, because I may be
> overlooking it, but otherwise I would prefer to focus on the technical
> side of things instead of wast^Hspending time on theoretical worries.
>
>>> And it is not an option for those boards to use DT in the firmware.
>>
>> There's nothing stopping these systems defining a DSD that contains a
>> DTB which overrides some or all of the ACPI if the system supports it
>> (or otherwise providing both system descriptions).  The two can coexist
>> happily enough as arm64 has shown
>
> I'm not sure to what extent it has shown that and even so, it doesn't
> mean this is a good idea.
>
>> and it seems like it ought to save a
>> whole lot of work especially around the bits that need inter device
>> links and are hence need some new ACPI bindings defining.
>
> There is at least one major problem with this approach.  If the ACPI
> part needs to point to anything in the DTB or if the DTB part needs to
> point to the ACPI part outside of it, there's no clean way that could
> be done.  I actually am not aware of any way whatever, but if there
> are some, I kind of don't expect them to be pretty.
>
>> I can see that removing ACPI entirely would present serious difficulties
>> but it's less clear to me how much is really gained by having an
>> embedded Linux specific ACPI variant over having some Linux specific
>> data in ACPI that happens to be parsable as DT.  The circumstances that
>> the two platforms face appear to be very similar.
>
> I'm not really sure what you mean here.
>
> _DSD properties can convey the same information as DT in the same
> layout.  The only difference is that DT uses phandles to point to
> things and _DSD properties use ACPI namespace paths for this purpose.
> That actually is where code changes need to be made, the rest is
> already there.

In fact, from the utility perspective, _DSD properties are just like
an "embedded DTB" you talked about, except that they are represented
in ASL (which makes them easier to handle in a couple of ways) and use
namespace paths instead of phandles to point to things.

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1700 bytes --]

On Tue, Oct 11, 2016 at 02:11:06PM +0200, Rafael J. Wysocki wrote:
> On Tue, Oct 11, 2016 at 1:44 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> > On Thu, Oct 6, 2016 at 11:37 PM, Mark Brown <broonie@kernel.org> wrote:

> >> I can see that removing ACPI entirely would present serious difficulties
> >> but it's less clear to me how much is really gained by having an
> >> embedded Linux specific ACPI variant over having some Linux specific
> >> data in ACPI that happens to be parsable as DT.  The circumstances that
> >> the two platforms face appear to be very similar.

> > I'm not really sure what you mean here.

> > _DSD properties can convey the same information as DT in the same
> > layout.  The only difference is that DT uses phandles to point to
> > things and _DSD properties use ACPI namespace paths for this purpose.
> > That actually is where code changes need to be made, the rest is
> > already there.

ACPI *can* be used in this way but it's not clear that this is a
mainstream usage of ACPI.

> In fact, from the utility perspective, _DSD properties are just like
> an "embedded DTB" you talked about, except that they are represented
> in ASL (which makes them easier to handle in a couple of ways) and use
> namespace paths instead of phandles to point to things.

It does require an awful lot of code churn to implement and it does
seem like it's going to be ending up with a lot of non-standard ACPI
bindings available to system integrators targeting Linux that perhaps
lack broad buy in.  This would be a bit annoying if we end up with bad
bindings that way but it'd also be sad if we were getting good bindings
that weren't getting broad adoption for whatever reason.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC 00/15] ACPI graph support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2297 bytes --]

On Tue, Oct 11, 2016 at 01:44:46AM +0200, Rafael J. Wysocki wrote:
> On Thu, Oct 6, 2016 at 11:37 PM, Mark Brown <broonie@kernel.org> wrote:

> > Personally I don't have that big a concern around per device
> > properties other than the need to go through yet another round of
> > churn for them (though it is just mechanical which will make it less
> > painful).  I do worry when it goes to generic things and inter-device
> > relationships.

> Well, that was my first reaction to this series, but then I thought
> "Let's see what can go wrong with this specifically" and then I
> couldn't find anything.

> If you see something like that, please let me know, because I may be
> overlooking it, but otherwise I would prefer to focus on the technical
> side of things instead of wast^Hspending time on theoretical worries.

My primary concern is the addition of what appear to be phandles
introduced as part of this patch set.  The previous discussion had been
that we'd enable simple DT bindings which don't need inter-device
references and that those needed more careful study.  This appears to
be changing that.

> >> And it is not an option for those boards to use DT in the firmware.

> > There's nothing stopping these systems defining a DSD that contains a
> > DTB which overrides some or all of the ACPI if the system supports it
> > (or otherwise providing both system descriptions).  The two can coexist
> > happily enough as arm64 has shown

> I'm not sure to what extent it has shown that and even so, it doesn't
> mean this is a good idea.

People seem reasonably happy with it so far, YMMV.

> > and it seems like it ought to save a
> > whole lot of work especially around the bits that need inter device
> > links and are hence need some new ACPI bindings defining.

> There is at least one major problem with this approach.  If the ACPI
> part needs to point to anything in the DTB or if the DTB part needs to
> point to the ACPI part outside of it, there's no clean way that could
> be done.  I actually am not aware of any way whatever, but if there
> are some, I kind of don't expect them to be pretty.

The way ARM implements this is that you don't get the DT and ACPI
simultaneously, they're both present in the firmware and the OS picks
which one it wants to use at runtime.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC 00/15] ACPI graph support

[Mark Rutland]

On Tue, Oct 11, 2016 at 02:44:29PM +0200, Mark Brown wrote:
> On Tue, Oct 11, 2016 at 01:44:46AM +0200, Rafael J. Wysocki wrote:
> > On Thu, Oct 6, 2016 at 11:37 PM, Mark Brown <broonie@kernel.org> wrote:
> My primary concern is the addition of what appear to be phandles
> introduced as part of this patch set.  The previous discussion had been
> that we'd enable simple DT bindings which don't need inter-device
> references and that those needed more careful study.  This appears to
> be changing that.

Agreed.

To be quite clear, it doesn't matter that the mechanism isn't phandles per-se.

The fact that we're trying to describe relationships between devices (which has
fuzzy overlap with other parts of ACPI) is the major concern.

Thanks,
Mark.

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

[-- Attachment #1: Type: text/plain, Size: 2662 bytes --]

On Tuesday, October 11, 2016 02:44:29 PM Mark Brown wrote:
> On Tue, Oct 11, 2016 at 01:44:46AM +0200, Rafael J. Wysocki wrote:
> > On Thu, Oct 6, 2016 at 11:37 PM, Mark Brown <broonie@kernel.org> wrote:
> 
> > > Personally I don't have that big a concern around per device
> > > properties other than the need to go through yet another round of
> > > churn for them (though it is just mechanical which will make it less
> > > painful).  I do worry when it goes to generic things and inter-device
> > > relationships.
> 
> > Well, that was my first reaction to this series, but then I thought
> > "Let's see what can go wrong with this specifically" and then I
> > couldn't find anything.
> 
> > If you see something like that, please let me know, because I may be
> > overlooking it, but otherwise I would prefer to focus on the technical
> > side of things instead of wast^Hspending time on theoretical worries.
> 
> My primary concern is the addition of what appear to be phandles
> introduced as part of this patch set.  The previous discussion had been
> that we'd enable simple DT bindings which don't need inter-device
> references and that those needed more careful study.  This appears to
> be changing that.

Yes, it does, but why exactly do you think this is wrong?

Is there any specific problem it creates that you can point to?

> > >> And it is not an option for those boards to use DT in the firmware.
> 
> > > There's nothing stopping these systems defining a DSD that contains a
> > > DTB which overrides some or all of the ACPI if the system supports it
> > > (or otherwise providing both system descriptions).  The two can coexist
> > > happily enough as arm64 has shown
> 
> > I'm not sure to what extent it has shown that and even so, it doesn't
> > mean this is a good idea.
> 
> People seem reasonably happy with it so far, YMMV.
> 
> > > and it seems like it ought to save a
> > > whole lot of work especially around the bits that need inter device
> > > links and are hence need some new ACPI bindings defining.
> 
> > There is at least one major problem with this approach.  If the ACPI
> > part needs to point to anything in the DTB or if the DTB part needs to
> > point to the ACPI part outside of it, there's no clean way that could
> > be done.  I actually am not aware of any way whatever, but if there
> > are some, I kind of don't expect them to be pretty.
> 
> The way ARM implements this is that you don't get the DT and ACPI
> simultaneously, they're both present in the firmware and the OS picks
> which one it wants to use at runtime.

So for the boards I'm talking about ACPI is the only realistic choice.

Thanks,
Rafael

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC 00/15] ACPI graph support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1148 bytes --]

On Wed, Oct 12, 2016 at 02:32:04AM +0200, Rafael J. Wysocki wrote:
> On Tuesday, October 11, 2016 02:44:29 PM Mark Brown wrote:
> > On Tue, Oct 11, 2016 at 01:44:46AM +0200, Rafael J. Wysocki wrote:

> > My primary concern is the addition of what appear to be phandles
> > introduced as part of this patch set.  The previous discussion had been
> > that we'd enable simple DT bindings which don't need inter-device
> > references and that those needed more careful study.  This appears to
> > be changing that.

> Yes, it does, but why exactly do you think this is wrong?

> Is there any specific problem it creates that you can point to?

It means there's now nothing in the code that says that we shouldn't
just map a DT binding with inter-device references into ACPI and that
any future conversions just look like API usage cleanups.

> > The way ARM implements this is that you don't get the DT and ACPI
> > simultaneously, they're both present in the firmware and the OS picks
> > which one it wants to use at runtime.

> So for the boards I'm talking about ACPI is the only realistic choice.

There shouldn't be any technical limiation here.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Wed, Oct 12, 2016 at 2:05 PM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Oct 12, 2016 at 02:32:04AM +0200, Rafael J. Wysocki wrote:
>> On Tuesday, October 11, 2016 02:44:29 PM Mark Brown wrote:
>> > On Tue, Oct 11, 2016 at 01:44:46AM +0200, Rafael J. Wysocki wrote:
>
>> > My primary concern is the addition of what appear to be phandles
>> > introduced as part of this patch set.  The previous discussion had been
>> > that we'd enable simple DT bindings which don't need inter-device
>> > references and that those needed more careful study.  This appears to
>> > be changing that.
>
>> Yes, it does, but why exactly do you think this is wrong?
>
>> Is there any specific problem it creates that you can point to?
>
> It means there's now nothing in the code that says that we shouldn't
> just map a DT binding with inter-device references into ACPI

Are you sure about that?

This particular series doesn't make it so general AFAICS.

> and that any future conversions just look like API usage cleanups.

But it won't actually work, will it?

AFAICS, that would have required support in subsystems and the only
one that supports kind of generic _DSD properties is the GPIO one (and
these properties are based on _CRS resources anyway FWIW).

It would require explicit changes to subsystem code to do that and
surely they can be objected to by the respective maintainers of those
(including you).  IOW, for example, any patch attempting to change
of_* into device_property_* in the regulator subsystem will be highly
suspicious and objectionable, won't it?

>> > The way ARM implements this is that you don't get the DT and ACPI
>> > simultaneously, they're both present in the firmware and the OS picks
>> > which one it wants to use at runtime.
>
>> So for the boards I'm talking about ACPI is the only realistic choice.
>
> There shouldn't be any technical limiation here.

The limitation is the information necessary to implement low-level
support of things in there which is not and may never be available.

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Mark Rutland]

On Thu, Oct 06, 2016 at 11:37:04PM +0200, Mark Brown wrote:
> On Thu, Oct 06, 2016 at 02:26:50PM +0200, Rafael J. Wysocki wrote:
> > On Thu, Oct 6, 2016 at 10:57 AM, Lorenzo Pieralisi

> > And it is not an option for those boards to use DT in the firmware.
> 
> There's nothing stopping these systems defining a DSD that contains a
> DTB which overrides some or all of the ACPI if the system supports it
> (or otherwise providing both system descriptions). 

Please, no. We very deliberately avoided this mix-and-match scheme for arm64
(it was proposed in discussions several times), because it suffers form worse
issues than PRP (since you can't corss-reference between DT and ACPI).

The arm64 kernel needs a DTB to pass some OS-specific stuff like bootargs, but
when using ACPI almost everything else is ignored -- we don't unflatten the
tree and we don't instanciate devices from it.

> The two can coexist happily enough as arm64 has shown and it seems like it
> ought to save a whole lot of work especially around the bits that need inter
> device links and are hence need some new ACPI bindings defining.

A single kernel binary can happily support both, yes. But not a mixture at
runtime.

Thanks,
Mark.

Re: [RFC 00/15] ACPI graph support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1273 bytes --]

On Tue, Oct 11, 2016 at 02:01:21PM +0100, Mark Rutland wrote:
> On Thu, Oct 06, 2016 at 11:37:04PM +0200, Mark Brown wrote:

> > There's nothing stopping these systems defining a DSD that contains a
> > DTB which overrides some or all of the ACPI if the system supports it
> > (or otherwise providing both system descriptions). 

> Please, no. We very deliberately avoided this mix-and-match scheme for arm64
> (it was proposed in discussions several times), because it suffers form worse
> issues than PRP (since you can't corss-reference between DT and ACPI).

Right, I was mainly thinking of the all case here rather than the some
case.  It's not clear to me that we're not going to end up with the
mixed case anyway as a result of the FPGA work but that's definitely not
my preferred option.

> The arm64 kernel needs a DTB to pass some OS-specific stuff like bootargs, but
> when using ACPI almost everything else is ignored -- we don't unflatten the
> tree and we don't instanciate devices from it.

The important thing here is that that DTB can in fact be a real DT
rather than just the stub and in that case the kernel can pick and
choose (and by default it'll just ignore the ACPI if it can though
some of the distros change that default).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC 00/15] ACPI graph support

[Mark Rutland]

On Thu, Oct 06, 2016 at 02:26:50PM +0200, Rafael J. Wysocki wrote:
> On Thu, Oct 6, 2016 at 10:57 AM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
 
> > As you said this can only happen once the fwnode API usage trickles
> > into the respective subsystems. Can we prevent it ? I hope so and
> > we are keeping an eye on that too (that's the reason why I asked
> > Mika to widen the audience, BTW), but that's the *only* way to
> > prevent this FW bindings mix-up and it is almost impossible to
> > vet all code getting into subsystems IMHO.
> >
> > I am trying to understand why x86 wants to do this, please understand
> > our point of view too, we do not want to block progress we want to
> > prevent a mess.
> 
> It is not "x86" who wants to do that.
> 
> It is people who work on support for boards with ACPI firmware and
> containing devices that in Linux are handled by DT-centric code.
> 
> Of course, the reason why that code is DT-centric is because it was
> developed on systems using DT and there were no uses on ACPI-based
> systems for it back then.  Still, it is DT-centric as a matter of fact
> and *something* has to be done in order to make it work with ACPI.

We often create DT bindings for devices whose drivers were previously
ACPI-centric. If necessary, we expect that the driver or subsystem is
refactored in order to accomadate this.

I fail to see why the same cannot apply the other way around.

Mindlessly appying an s/of_/device_/ regex just adds to the mess.

Thanks,
Mark.

Re: [RFC 00/15] ACPI graph support

[Sakari Ailus]

Hi Rafael,

Rafael J. Wysocki wrote:
> I think that the target subsystem (V4L in this particular case) should
> be notified of this in the first place as they are the user of the
> bindings in question.

The related V4L2 patchset is here and it links to this discussion as well:

<URL:http://www.spinics.net/lists/linux-media/msg106160.html>

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

Re: [RFC 00/15] ACPI graph support

[Mark Rutland]

On Wed, Oct 05, 2016 at 06:32:29PM +0300, Mika Westerberg wrote:
> On Wed, Oct 05, 2016 at 04:06:41PM +0100, Lorenzo Pieralisi wrote:
> > On Wed, Oct 05, 2016 at 02:41:29PM +0300, Mika Westerberg wrote:
> > > On Wed, Oct 05, 2016 at 10:22:15AM +0100, Lorenzo Pieralisi wrote:
> > > > On Wed, Oct 05, 2016 at 01:45:33AM +0300, Sakari Ailus wrote:
> > > The whole purpose of PRP0001 ID is to allow DT bindings to be reused in
> > > ACPI systems, so that the drivers can just call device_property_* and
> > > get the properties regardless of the underlying firmware interface.
> > > 
> > > Are you saying that's not wanted?
> > 
> > Not wholesale DT bindings import into ACPI, just no way.
> 
> Of course not all DT bindings. Only those that do not have a native ACPI
> representation.

... yet.

For self-contained devices, this isn't much of a concern, but inter-device
relationships are the sort of thing ACPI *needs* to know about, and define a
model for. By trying to bodge this into _DSD, we're making matters worse by
both delaying the inevitable and creating a tonne of technical debt that we
have to deal with forever.

By copying DT, but changing a few things, we're in effect creating a new
ill-defined Linux-specific standard. If we're going to create a new standard,
we should go through the ASWG, and make an actual standard. If we're not going
to create a new standard, we should use DT directly, rather than trying to
force DT into ACPI.

Thanks,
Mark.

Re: [RFC 00/15] ACPI graph support

[Mika Westerberg]

On Tue, Oct 11, 2016 at 01:35:32PM +0100, Mark Rutland wrote:
> On Wed, Oct 05, 2016 at 06:32:29PM +0300, Mika Westerberg wrote:
> > On Wed, Oct 05, 2016 at 04:06:41PM +0100, Lorenzo Pieralisi wrote:
> > > On Wed, Oct 05, 2016 at 02:41:29PM +0300, Mika Westerberg wrote:
> > > > On Wed, Oct 05, 2016 at 10:22:15AM +0100, Lorenzo Pieralisi wrote:
> > > > > On Wed, Oct 05, 2016 at 01:45:33AM +0300, Sakari Ailus wrote:
> > > > The whole purpose of PRP0001 ID is to allow DT bindings to be reused in
> > > > ACPI systems, so that the drivers can just call device_property_* and
> > > > get the properties regardless of the underlying firmware interface.
> > > > 
> > > > Are you saying that's not wanted?
> > > 
> > > Not wholesale DT bindings import into ACPI, just no way.
> > 
> > Of course not all DT bindings. Only those that do not have a native ACPI
> > representation.
> 
> ... yet.
> 
> For self-contained devices, this isn't much of a concern, but inter-device
> relationships are the sort of thing ACPI *needs* to know about, and define a
> model for. By trying to bodge this into _DSD, we're making matters worse by
> both delaying the inevitable and creating a tonne of technical debt that we
> have to deal with forever.

ACPI has had references from the beginning and most probably one reason
these "inter device" relationships are not in that spec, is because
nobody thinks it is relevant to put them there. If you check any
existing ACPI tables they have lots of references between devices. Some
of the ASL code even calls methods of another device through these
refences and it is not forbidden by the spec by any means.

A good example of this is the audio machine driver the other Mark
mentioned.

Here is an example from Microsoft Surface3:

  Device (AMCR)
  {
      Name (_HID, "AMCR22A8")
      ...

      Name (_DEP, Package (0x02)  // _DEP: Dependencies
      {
          GPO2,
          ^I2C2.RTEK
      })
      ...
  }

The _DEP method should be used for Operation Region dependencies but
here it is used for functional dependencies so that the audio machine
driver can find the corresponding codec. Why Microsoft used it like this
and not pushed it to ASWG to be added to the ACPI spec? Should we now
refuse to support it in Linux on the basis that it has not been
discussed with ASWG and it abuses _DEP?

Basically _DSD here is used to add relevant names for these links so
that they are compatible with the names used by the existing drivers in
Linux (this is the part that is missing from the ACPI spec). This makes
the same drivers to work with both DT and ACPI with minor modifications.

> By copying DT, but changing a few things, we're in effect creating a new
> ill-defined Linux-specific standard. If we're going to create a new standard,
> we should go through the ASWG, and make an actual standard. If we're not going
> to create a new standard, we should use DT directly, rather than trying to
> force DT into ACPI.

These boards we are talking about ship with ACPI based firmware. We
should not expect users of those boards to be cabable of replacing the
existing firmware with DT one.

Re: [RFC 00/15] ACPI graph support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1808 bytes --]

On Wed, Oct 12, 2016 at 12:00:03PM +0300, Mika Westerberg wrote:

> The _DEP method should be used for Operation Region dependencies but
> here it is used for functional dependencies so that the audio machine
> driver can find the corresponding codec. Why Microsoft used it like this
> and not pushed it to ASWG to be added to the ACPI spec? Should we now
> refuse to support it in Linux on the basis that it has not been
> discussed with ASWG and it abuses _DEP?

The fact that the ACPI community hasn't been doing a good job of working
together is essentially the issue that people are pushing back on here.  
We appear to not even be trying to set a better standard for how to do
things here.

Sitting externally to the group at Intel doing this it really looks like
there's been a decision to mirror DT into ACPI en masse.  If we really
want to do that we should actually take that decision, preferrably with
at least buy in from other ACPI users on Linux and with some review of
existing ACPI standardization efforts to make sure we're not duplicating
work there.  Ideally we'd also be pushing anything we do towards ASWG
even if just as a fiat accomplait.

> > By copying DT, but changing a few things, we're in effect creating a new
> > ill-defined Linux-specific standard. If we're going to create a new standard,
> > we should go through the ASWG, and make an actual standard. If we're not going
> > to create a new standard, we should use DT directly, rather than trying to
> > force DT into ACPI.

> These boards we are talking about ship with ACPI based firmware. We
> should not expect users of those boards to be cabable of replacing the
> existing firmware with DT one.

Since we're still at the point of defining bindings hopefully we're not
shipping yet...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC 00/15] ACPI graph support

[Mika Westerberg]

On Wed, Oct 12, 2016 at 12:28:36PM +0200, Mark Brown wrote:
> On Wed, Oct 12, 2016 at 12:00:03PM +0300, Mika Westerberg wrote:
> 
> > The _DEP method should be used for Operation Region dependencies but
> > here it is used for functional dependencies so that the audio machine
> > driver can find the corresponding codec. Why Microsoft used it like this
> > and not pushed it to ASWG to be added to the ACPI spec? Should we now
> > refuse to support it in Linux on the basis that it has not been
> > discussed with ASWG and it abuses _DEP?
> 
> The fact that the ACPI community hasn't been doing a good job of working
> together is essentially the issue that people are pushing back on here.  
> We appear to not even be trying to set a better standard for how to do
> things here.

Why using _DSD and the existing, well tested, DT properties (remote
endpoints) is not even considered making a better standard?

> Sitting externally to the group at Intel doing this it really looks like
> there's been a decision to mirror DT into ACPI en masse.

There has been no such decision as far as I can tell.

> If we really want to do that we should actually take that decision,
> preferrably with at least buy in from other ACPI users on Linux and
> with some review of existing ACPI standardization efforts to make sure
> we're not duplicating work there.  Ideally we'd also be pushing
> anything we do towards ASWG even if just as a fiat accomplait.

Agreed.

> > > By copying DT, but changing a few things, we're in effect creating a new
> > > ill-defined Linux-specific standard. If we're going to create a new standard,
> > > we should go through the ASWG, and make an actual standard. If we're not going
> > > to create a new standard, we should use DT directly, rather than trying to
> > > force DT into ACPI.
> 
> > These boards we are talking about ship with ACPI based firmware. We
> > should not expect users of those boards to be cabable of replacing the
> > existing firmware with DT one.
> 
> Since we're still at the point of defining bindings hopefully we're not
> shipping yet...

No, those systems are already out there. See Intel Joule for one example
(there are many others). You can connect your own camera sensor there.

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Wed, Oct 12, 2016 at 1:12 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Wed, Oct 12, 2016 at 12:28:36PM +0200, Mark Brown wrote:
>> On Wed, Oct 12, 2016 at 12:00:03PM +0300, Mika Westerberg wrote:
>>
>> > The _DEP method should be used for Operation Region dependencies but
>> > here it is used for functional dependencies so that the audio machine
>> > driver can find the corresponding codec. Why Microsoft used it like this
>> > and not pushed it to ASWG to be added to the ACPI spec? Should we now
>> > refuse to support it in Linux on the basis that it has not been
>> > discussed with ASWG and it abuses _DEP?
>>
>> The fact that the ACPI community hasn't been doing a good job of working
>> together is essentially the issue that people are pushing back on here.
>> We appear to not even be trying to set a better standard for how to do
>> things here.
>
> Why using _DSD and the existing, well tested, DT properties (remote
> endpoints) is not even considered making a better standard?
>
>> Sitting externally to the group at Intel doing this it really looks like
>> there's been a decision to mirror DT into ACPI en masse.
>
> There has been no such decision as far as I can tell.

And this is the whole point really.

I'm not aware of any decision of this kind either and (as we have
pointed out for several times already) there are technical obstacles
to doing that even if someone wanted to do that.  But again, nobody
wants AFAICS.

Moreover, I don't really like the direction it is going into.  We
should be collaborating instead of accusing our respective employers
of doing nasty things in this forum.

Let's focus on the technical side then or the discussion is over.

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 751 bytes --]

On Wed, Oct 12, 2016 at 01:25:32PM +0200, Rafael J. Wysocki wrote:

> Moreover, I don't really like the direction it is going into.  We
> should be collaborating instead of accusing our respective employers
> of doing nasty things in this forum.

> Let's focus on the technical side then or the discussion is over.

To be clear at least from my perspective I don't think there's any bad
intent on anyone's part, I think at most we've got a lack of
coordination about the direction Linux is taking it's ACPI usage in and
to what extent that's aligned with ACPI standards and what OSs like
Windows are doing.  I have no doubt that we all want to make things
better and I apologize that I've expressed myself in a way that led
people to think otherwise.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Wed, Oct 12, 2016 at 12:28 PM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Oct 12, 2016 at 12:00:03PM +0300, Mika Westerberg wrote:
>
>> The _DEP method should be used for Operation Region dependencies but
>> here it is used for functional dependencies so that the audio machine
>> driver can find the corresponding codec. Why Microsoft used it like this
>> and not pushed it to ASWG to be added to the ACPI spec? Should we now
>> refuse to support it in Linux on the basis that it has not been
>> discussed with ASWG and it abuses _DEP?
>
> The fact that the ACPI community hasn't been doing a good job of working
> together is essentially the issue that people are pushing back on here.

But this is not the right venue to talk about it which you should know. :-)

> We appear to not even be trying to set a better standard for how to do
> things here.

Sorry, who's "we"?

> Sitting externally to the group at Intel doing this it really looks like
> there's been a decision to mirror DT into ACPI en masse.  If we really
> want to do that we should actually take that decision, preferrably with
> at least buy in from other ACPI users on Linux and with some review of
> existing ACPI standardization efforts to make sure we're not duplicating
> work there.  Ideally we'd also be pushing anything we do towards ASWG
> even if just as a fiat accomplait.
>
>> > By copying DT, but changing a few things, we're in effect creating a new
>> > ill-defined Linux-specific standard. If we're going to create a new standard,
>> > we should go through the ASWG, and make an actual standard. If we're not going
>> > to create a new standard, we should use DT directly, rather than trying to
>> > force DT into ACPI.
>
>> These boards we are talking about ship with ACPI based firmware. We
>> should not expect users of those boards to be cabable of replacing the
>> existing firmware with DT one.
>
> Since we're still at the point of defining bindings hopefully we're not
> shipping yet...

It seems to me that the problem is escaping you.

The systems in question are shipping in this form or another, with
ACPI tables in the firmware.

In one case, alternative OSes can handle devices on them through
drivers that contain code dependent on specific device IDs in their
ACPI tables.  Linux has drivers for those devices too, but they are
based on the of_* interface and expect information from DT that is not
available from the ACPI tables (and it is not available, because the
drivers working with the alternative OS simply don't need it, as for
them the device ID is sufficient to convey all of the information on
the given device).

In the second case, devices can be added to them through extensions
that require information on those devices to be provided to the kernel
in some way (IOW, they aren't auto-enumerable).  Again, there are
drivers for at least some of those devices in Linux already, but they
are based on the of_* interface and expect information from DT that is
not available from the ACPI tables (and it is not available, because
the devices were not there when the firmware was developed, so there
is no way it can contain information on them).

But this isn't the point of this patch series even.

The point is that ports/endpoints appear to be a useful concept.  I
agree with that.

Moreover, *in* *principle* I don't see anything wrong with adding code
to the kernel to support that (generally useful concept) through _DSD
properties.  Again, if you see anything wrong with this in particular,
please let me know, but please be specific.

Finally, if ports/endpoints can be supported through _DSD properties,
I don't see any reason to not follow DT in that respect.

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]

On Wed, Oct 12, 2016 at 01:14:18PM +0200, Rafael J. Wysocki wrote:
> On Wed, Oct 12, 2016 at 12:28 PM, Mark Brown <broonie@kernel.org> wrote:

> > We appear to not even be trying to set a better standard for how to do
> > things here.

> Sorry, who's "we"?

Linux.

> In one case, alternative OSes can handle devices on them through
> drivers that contain code dependent on specific device IDs in their
> ACPI tables.

Right, this is currently the idiomatic way to handle these things in
ACPI unfortunately and it's what Windows (which I'm assuming is the
alternative OSs you're referencing here!) does.  Are people working on
trying to move this forwards?

>               Linux has drivers for those devices too, but they are
> based on the of_* interface and expect information from DT that is not

If the drivers have DT code that's not well partitioned in the
initialization code that sounds like a general problem :(

> available from the ACPI tables (and it is not available, because the
> drivers working with the alternative OS simply don't need it, as for
> them the device ID is sufficient to convey all of the information on
> the given device).

What we've been doing up until now to support these systems in Linux is
to have platform data in the driver that's matched via DMI information.
The Windows systems don't appear to be in the slightest bit interested
in using the Linux bindings or updating their firmware to accomodate us
so the sensible thing is to work in the same way as they do.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Wed, Oct 12, 2016 at 2:32 PM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Oct 12, 2016 at 01:14:18PM +0200, Rafael J. Wysocki wrote:
>> On Wed, Oct 12, 2016 at 12:28 PM, Mark Brown <broonie@kernel.org> wrote:
>
>> > We appear to not even be trying to set a better standard for how to do
>> > things here.
>
>> Sorry, who's "we"?
>
> Linux.
>
>> In one case, alternative OSes can handle devices on them through
>> drivers that contain code dependent on specific device IDs in their
>> ACPI tables.
>
> Right, this is currently the idiomatic way to handle these things in
> ACPI unfortunately and it's what Windows (which I'm assuming is the
> alternative OSs you're referencing here!) does.  Are people working on
> trying to move this forwards?

"Forward" in what way?  To convince the Windows people to change their
ecosystem?  No.

>>               Linux has drivers for those devices too, but they are
>> based on the of_* interface and expect information from DT that is not
>
> If the drivers have DT code that's not well partitioned in the
> initialization code that sounds like a general problem :(
>
>> available from the ACPI tables (and it is not available, because the
>> drivers working with the alternative OS simply don't need it, as for
>> them the device ID is sufficient to convey all of the information on
>> the given device).
>
> What we've been doing up until now to support these systems in Linux is
> to have platform data in the driver that's matched via DMI information.

That is becoming more and more problematic, though, and generally
leads to inflated DMI tables in drivers which is not perfect to put it
lightly.

> The Windows systems don't appear to be in the slightest bit interested
> in using the Linux bindings or updating their firmware to accomodate us
> so the sensible thing is to work in the same way as they do.

That would mean adding static data tables to drivers based on device
IDs somehow, but that pretty much would be what happened in the ARM
camp before DT, wouldn't it?

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1919 bytes --]

On Wed, Oct 12, 2016 at 02:42:25PM +0200, Rafael J. Wysocki wrote:
> On Wed, Oct 12, 2016 at 2:32 PM, Mark Brown <broonie@kernel.org> wrote:

> > Right, this is currently the idiomatic way to handle these things in
> > ACPI unfortunately and it's what Windows (which I'm assuming is the
> > alternative OSs you're referencing here!) does.  Are people working on
> > trying to move this forwards?

> "Forward" in what way?  To convince the Windows people to change their
> ecosystem?  No.

Well, at least trying to get standard ACPI able to express these ideas
better.

> > What we've been doing up until now to support these systems in Linux is
> > to have platform data in the driver that's matched via DMI information.

> That is becoming more and more problematic, though, and generally
> leads to inflated DMI tables in drivers which is not perfect to put it
> lightly.

Tell me about it - as far as I can see we're going to end up with a
kernel patch for most laptops or tablets anyone tries to run Linux on in
order to get audio working which is going to be a pretty miserable
experience for users and distros.

> > The Windows systems don't appear to be in the slightest bit interested
> > in using the Linux bindings or updating their firmware to accomodate us
> > so the sensible thing is to work in the same way as they do.

> That would mean adding static data tables to drivers based on device
> IDs somehow, but that pretty much would be what happened in the ARM
> camp before DT, wouldn't it?

Yes, that's what it means.  It's pretty much a step backwards from what
architectures doing board files have - no changes are needed in the
drivers, instead when you add a new board you add a file which describes
everything on the board including all the platform data for the device.
It at least keeps all the support for the board and the matching code in
one place rather than scattering it through drivers.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Wed, Oct 12, 2016 at 4:59 PM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Oct 12, 2016 at 02:42:25PM +0200, Rafael J. Wysocki wrote:
>> On Wed, Oct 12, 2016 at 2:32 PM, Mark Brown <broonie@kernel.org> wrote:
>
>> > Right, this is currently the idiomatic way to handle these things in
>> > ACPI unfortunately and it's what Windows (which I'm assuming is the
>> > alternative OSs you're referencing here!) does.  Are people working on
>> > trying to move this forwards?
>
>> "Forward" in what way?  To convince the Windows people to change their
>> ecosystem?  No.
>
> Well, at least trying to get standard ACPI able to express these ideas
> better.

There are things that could be covered by ACPI proper (clocks and
regulators support in particular), but some other things covered by DT
in Linux really go beyond that.

The "ports" and "endpoints" concept is a good example of that I think,
as the use case is not in the traditional ACPI space (configuration
and PM).  I'm not really sure if the ASWG is the right venue to talk
about that would-be standard even and if the ACPI spec is the right
place to put it into.

>> > What we've been doing up until now to support these systems in Linux is
>> > to have platform data in the driver that's matched via DMI information.
>
>> That is becoming more and more problematic, though, and generally
>> leads to inflated DMI tables in drivers which is not perfect to put it
>> lightly.
>
> Tell me about it - as far as I can see we're going to end up with a
> kernel patch for most laptops or tablets anyone tries to run Linux on in
> order to get audio working which is going to be a pretty miserable
> experience for users and distros.

Exactly.

>> > The Windows systems don't appear to be in the slightest bit interested
>> > in using the Linux bindings or updating their firmware to accomodate us
>> > so the sensible thing is to work in the same way as they do.
>
>> That would mean adding static data tables to drivers based on device
>> IDs somehow, but that pretty much would be what happened in the ARM
>> camp before DT, wouldn't it?
>
> Yes, that's what it means.  It's pretty much a step backwards from what
> architectures doing board files have - no changes are needed in the
> drivers, instead when you add a new board you add a file which describes
> everything on the board including all the platform data for the device.
> It at least keeps all the support for the board and the matching code in
> one place rather than scattering it through drivers.

Right.

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Sakari Ailus]

Hi Lorenzo,

Lorenzo Pieralisi wrote:
> FWIW I had a quick look at dts bindings with "compatible = nokia,smia"
> and related kernel driver.
>
> Those nodes require properties like clocks and power supplies, it
> seems to me that there are already ACPI PM methods to control those
> properties and therefore they should not be handled with PRP0001,
> I am happy to be corrected if I am wrong.

Those properties are there for DT only. ACPI already has native 
mechanisms for power resources but the driver right now still requires 
explicit regulator and clock, meaning that it does not work on an ACPI 
system out of the box.

The patch making the regulator optional is here:

<URL:http://www.spinics.net/lists/linux-media/msg106156.html>

I'll provide another patch to make the clock optional as well *once I 
can fully test it*. Right now I can't.

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

Re: [RFC 00/15] ACPI graph support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 474 bytes --]

On Wed, Oct 05, 2016 at 02:41:29PM +0300, Mika Westerberg wrote:

> The whole purpose of PRP0001 ID is to allow DT bindings to be reused in
> ACPI systems, so that the drivers can just call device_property_* and
> get the properties regardless of the underlying firmware interface.

> Are you saying that's not wanted?

The idea was to allow the reuse of simple leaf drivers, as soon as you
get into things where you need to refer to other nodes it becomes much
less clear.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC 00/15] ACPI graph support

[Sudeep Holla]

On 05/10/16 12:41, Mika Westerberg wrote:
> On Wed, Oct 05, 2016 at 10:22:15AM +0100, Lorenzo Pieralisi wrote:
>> [ +MarkR, MarkB, Rob, Al - I suspect they may want to have a say]
>>
>> On Wed, Oct 05, 2016 at 01:45:33AM +0300, Sakari Ailus wrote:
>>> Hello everyone,
>>>
>>> I've been working awhile with my collegue Mika Westerberg to bring
>>> firmware graph support to ACPI based systems. In practice 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.
>>
>> There is an ongoing effort to avoid wholesale import of DT bindings
>> into ACPI, I am not a V4L2 expert but it seems to me that with patches
>> like the one you have submitted we are getting closer and closer to
>> achieving it instead of avoiding it.
>
> The whole purpose of PRP0001 ID is to allow DT bindings to be reused in
> ACPI systems, so that the drivers can just call device_property_* and
> get the properties regardless of the underlying firmware interface.
>

Does this also mean if there's some new bindings added to DT which ACPI
specification still lacks, then instead of enhancing ACPI specification
adding that to it, we can take a shortcut method of PRP0001 and
completely ignore ACPI. People are trying to do that as it's simple and
faster.

And yes this has been raised multiple times in past, but worth raising
every-time we head in that direction. And it's increasing day-by-day
which is alarming.

Even though you may say no to that, it absolutely prevents no one
to do so unless we control what bindings can be support using DSD.

-- 
Regards,
Sudeep

Re: [RFC 00/15] ACPI graph support

[Mika Westerberg]

On Wed, Oct 05, 2016 at 04:30:18PM +0100, Sudeep Holla wrote:
> 
> 
> On 05/10/16 12:41, Mika Westerberg wrote:
> > On Wed, Oct 05, 2016 at 10:22:15AM +0100, Lorenzo Pieralisi wrote:
> > > [ +MarkR, MarkB, Rob, Al - I suspect they may want to have a say]
> > > 
> > > On Wed, Oct 05, 2016 at 01:45:33AM +0300, Sakari Ailus wrote:
> > > > Hello everyone,
> > > > 
> > > > I've been working awhile with my collegue Mika Westerberg to bring
> > > > firmware graph support to ACPI based systems. In practice 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.
> > > 
> > > There is an ongoing effort to avoid wholesale import of DT bindings
> > > into ACPI, I am not a V4L2 expert but it seems to me that with patches
> > > like the one you have submitted we are getting closer and closer to
> > > achieving it instead of avoiding it.
> > 
> > The whole purpose of PRP0001 ID is to allow DT bindings to be reused in
> > ACPI systems, so that the drivers can just call device_property_* and
> > get the properties regardless of the underlying firmware interface.
> > 
> 
> Does this also mean if there's some new bindings added to DT which ACPI
> specification still lacks, then instead of enhancing ACPI specification
> adding that to it, we can take a shortcut method of PRP0001 and
> completely ignore ACPI. People are trying to do that as it's simple and
> faster.

No and we are not ignoring ACPI either with this patch series.

> And yes this has been raised multiple times in past, but worth raising
> every-time we head in that direction. And it's increasing day-by-day
> which is alarming.
> 
> Even though you may say no to that, it absolutely prevents no one
> to do so unless we control what bindings can be support using DSD.

So how you propose we deal with this? Add it to the ACPI spec?

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Wed, Oct 5, 2016 at 5:30 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 05/10/16 12:41, Mika Westerberg wrote:
>>
>> On Wed, Oct 05, 2016 at 10:22:15AM +0100, Lorenzo Pieralisi wrote:
>>>
>>> [ +MarkR, MarkB, Rob, Al - I suspect they may want to have a say]
>>>
>>> On Wed, Oct 05, 2016 at 01:45:33AM +0300, Sakari Ailus wrote:
>>>>
>>>> Hello everyone,
>>>>
>>>> I've been working awhile with my collegue Mika Westerberg to bring
>>>> firmware graph support to ACPI based systems. In practice 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.
>>>
>>>
>>> There is an ongoing effort to avoid wholesale import of DT bindings
>>> into ACPI, I am not a V4L2 expert but it seems to me that with patches
>>> like the one you have submitted we are getting closer and closer to
>>> achieving it instead of avoiding it.
>>
>>
>> The whole purpose of PRP0001 ID is to allow DT bindings to be reused in
>> ACPI systems, so that the drivers can just call device_property_* and
>> get the properties regardless of the underlying firmware interface.
>>
>
> Does this also mean if there's some new bindings added to DT which ACPI
> specification still lacks, then instead of enhancing ACPI specification
> adding that to it, we can take a shortcut method of PRP0001 and
> completely ignore ACPI.

No.

> People are trying to do that as it's simple and faster.

The answer is really very simple: You need to build on top of what is
defined in ACPI already instead of trying to replace it.

So if your properties duplicate ACPI-defined ways of doing things or
would be in a direct conflict with them, they cannot be supported by
the mainline kernel as far as I'm concerned.

If, OTOH, they are defined such that they will extend the information
that can be provided by ACPI natively, I don't see a reason to reject
the concept as a matter of principle.

> And yes this has been raised multiple times in past, but worth raising
> every-time we head in that direction. And it's increasing day-by-day
> which is alarming.
>
> Even though you may say no to that, it absolutely prevents no one
> to do so unless we control what bindings can be support using DSD.

First of all, that's totally unrealistic.  You can't prevent anyone
from using whatever properties they want in practice.  You can only
(try to) block the mainline kernel from supporting those properties,
but the question here is why.

The whole exercise with _DSD properties is all about code reuse.

There is a metric ton of DT-centric code already in the kernel that is
well understood and tested and it would be plain unreasonable to try
to reinvent the wheel and (a) invent ACPI-specific ways of doing the
same thing for every use case that is distinct enough and (b) write
code using those ACPI-specific ways instead of already defined
properties just in order to handle the same hardware in the same OS.
[If you have to support different OSes, this is a different matter.]

If you are saying "no DT properties in _DSD", this implies that code
reuse is bad for some unspecified reason.  It is very difficult to me
to agree with that position.

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Sudeep Holla]

On 05/10/16 21:18, Rafael J. Wysocki wrote:
> On Wed, Oct 5, 2016 at 5:30 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>

[...]

>> Does this also mean if there's some new bindings added to DT which ACPI
>> specification still lacks, then instead of enhancing ACPI specification
>> adding that to it, we can take a shortcut method of PRP0001 and
>> completely ignore ACPI.
>
> No.
>

I know that the intentions of this series is not that, but my question
is more how can we prevent that misuse.

>> People are trying to do that as it's simple and faster.
>
> The answer is really very simple: You need to build on top of what is
> defined in ACPI already instead of trying to replace it.
>

No doubt about that, though controlling it is another question.
Especially ARM vendors who have some driver for DT might just use this
method without anyone noticing, ignoring the ACPI method as everything
just works out of box. That's the main worry. Also it also provides no
incentive for them to work with ASWG to enhance ACPI specification.

> So if your properties duplicate ACPI-defined ways of doing things or
> would be in a direct conflict with them, they cannot be supported by
> the mainline kernel as far as I'm concerned.
>

Agreed.

> If, OTOH, they are defined such that they will extend the information
> that can be provided by ACPI natively, I don't see a reason to reject
> the concept as a matter of principle.
>

While I agree conceptually, it confuses(again ARM vendors who are new to
ACPI) as when to use this method vs adding something to the ACPI
specification. They might just start to use this for all, again as it
just works with minimal or no kernel change.

>> And yes this has been raised multiple times in past, but worth raising
>> every-time we head in that direction. And it's increasing day-by-day
>> which is alarming.
>>
>> Even though you may say no to that, it absolutely prevents no one
>> to do so unless we control what bindings can be support using DSD.
>
> First of all, that's totally unrealistic.  You can't prevent anyone
> from using whatever properties they want in practice.  You can only
> (try to) block the mainline kernel from supporting those properties,
> but the question here is why.
>

Agreed, but just afraid that it may become a de-facto standard as long
as things can be made to work.

> The whole exercise with _DSD properties is all about code reuse.
>

Sure. On the side note, we should at-least have PRP0001 reserved
officially in the ACPI specification, I still see no mention about that
in the spec and both firmware and Linux are using it for a while now.

> There is a metric ton of DT-centric code already in the kernel that is
> well understood and tested and it would be plain unreasonable to try
> to reinvent the wheel and (a) invent ACPI-specific ways of doing the
> same thing for every use case that is distinct enough and (b) write
> code using those ACPI-specific ways instead of already defined
> properties just in order to handle the same hardware in the same OS.
> [If you have to support different OSes, this is a different matter.]
>

Agreed, but as you mentioned the question of different OS remains.

> If you are saying "no DT properties in _DSD", this implies that code
> reuse is bad for some unspecified reason.  It is very difficult to me
> to agree with that position.
>

No, I am not saying no. I am just worried where do we stop this and how
do we prevent ARM vendors exploiting this, and not rather contributing
to the ACPI specification enhancement.

-- 
Regards,
Sudeep

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Thu, Oct 6, 2016 at 12:29 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 05/10/16 21:18, Rafael J. Wysocki wrote:
>>
>> On Wed, Oct 5, 2016 at 5:30 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>
>>>
>>>
>
> [...]
>
>>> Does this also mean if there's some new bindings added to DT which ACPI
>>> specification still lacks, then instead of enhancing ACPI specification
>>> adding that to it, we can take a shortcut method of PRP0001 and
>>> completely ignore ACPI.
>>
>>
>> No.
>>
>
> I know that the intentions of this series is not that, but my question
> is more how can we prevent that misuse.
>
>>> People are trying to do that as it's simple and faster.
>>
>>
>> The answer is really very simple: You need to build on top of what is
>> defined in ACPI already instead of trying to replace it.
>>
>
> No doubt about that, though controlling it is another question.
> Especially ARM vendors who have some driver for DT might just use this
> method without anyone noticing, ignoring the ACPI method as everything
> just works out of box. That's the main worry. Also it also provides no
> incentive for them to work with ASWG to enhance ACPI specification.

They will need to hack the kernel to do that, because the mainline
will not support anything like regulator or pinctrl support based on
DT properties, at least as long as I have any influence on that.

And if they need to hack the kernel anyway, nothing prevents them from
hacking it to support whatever properties they like even if we don't
put any support for any generic _DSD properties into the mainline.
That's why the whole "oh but this allows ARM vendors to abuse things"
argument doesn't hold any water in my view.

Yes, it may make it a bit easier for them to abuse things (as they
don't have to hack the kernel to support properties already supported
by it), but OTOH it sort of avoids the problem that multiple vendors
could hack the kernel in different and possibly incompatible ways to
make it support the same set of properties they all need.

>> So if your properties duplicate ACPI-defined ways of doing things or
>> would be in a direct conflict with them, they cannot be supported by
>> the mainline kernel as far as I'm concerned.
>>
>
> Agreed.
>
>> If, OTOH, they are defined such that they will extend the information
>> that can be provided by ACPI natively, I don't see a reason to reject
>> the concept as a matter of principle.
>>
>
> While I agree conceptually, it confuses(again ARM vendors who are new to
> ACPI) as when to use this method vs adding something to the ACPI
> specification. They might just start to use this for all, again as it
> just works with minimal or no kernel change.

So let me repeat: Even if the mainline kernel had not supported any
generic _DSD properties (it does support GPIO properties today), those
vendors could have hacked it to support them anyway and there's
nothing to prevent them from doing that.

Making the mainline kernel support some additional generic _DSD
properties doesn't make the situation any worse in any way IMO.

>>> And yes this has been raised multiple times in past, but worth raising
>>> every-time we head in that direction. And it's increasing day-by-day
>>> which is alarming.
>>>
>>> Even though you may say no to that, it absolutely prevents no one
>>> to do so unless we control what bindings can be support using DSD.
>>
>>
>> First of all, that's totally unrealistic.  You can't prevent anyone
>> from using whatever properties they want in practice.  You can only
>> (try to) block the mainline kernel from supporting those properties,
>> but the question here is why.
>>
>
> Agreed, but just afraid that it may become a de-facto standard as long
> as things can be made to work.
>
>> The whole exercise with _DSD properties is all about code reuse.
>>
>
> Sure. On the side note, we should at-least have PRP0001 reserved
> officially in the ACPI specification, I still see no mention about that
> in the spec and both firmware and Linux are using it for a while now.

The whole PRPxxxx range of device IDs is reserved.

>> There is a metric ton of DT-centric code already in the kernel that is
>> well understood and tested and it would be plain unreasonable to try
>> to reinvent the wheel and (a) invent ACPI-specific ways of doing the
>> same thing for every use case that is distinct enough and (b) write
>> code using those ACPI-specific ways instead of already defined
>> properties just in order to handle the same hardware in the same OS.
>> [If you have to support different OSes, this is a different matter.]
>>
>
> Agreed, but as you mentioned the question of different OS remains.
>
>> If you are saying "no DT properties in _DSD", this implies that code
>> reuse is bad for some unspecified reason.  It is very difficult to me
>> to agree with that position.
>>
>
> No, I am not saying no. I am just worried where do we stop this and how
> do we prevent ARM vendors exploiting this, and not rather contributing
> to the ACPI specification enhancement.

The ACPI spec may not be the right place to define support for things
this series is about, though.  Ideally, they should be defined by
other standards using ASL as a form of expression or representation,
if you will.

And that still needs to happen for different OSes to be able to use
the ACPI tables in the same way (which doesn't even happen for
different versions of Windows, but that's a different matter).

However, adding support for additional _DSD properties to the mainline
kernel need not prevent a standardization process like this from
happening.  The point is that this process will take time and both the
HW and the code in the kernel supporting it are available today.  The
only problem is that the code is DT-centric and there are ACPI tables
in the boards in question.

If/when the new standard comes along, we will support it by default
and then fall back to _DSD properties if information defined by it is
not available.  Pretty much the same way as we fall back to
driver-supplied data if we can get information we need from the
platform firmware.

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Sudeep Holla]

On 06/10/16 14:04, Rafael J. Wysocki wrote:
> On Thu, Oct 6, 2016 at 12:29 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:

[...]

>> No doubt about that, though controlling it is another question.
>> Especially ARM vendors who have some driver for DT might just use this
>> method without anyone noticing, ignoring the ACPI method as everything
>> just works out of box. That's the main worry. Also it also provides no
>> incentive for them to work with ASWG to enhance ACPI specification.
>
> They will need to hack the kernel to do that, because the mainline
> will not support anything like regulator or pinctrl support based on
> DT properties, at least as long as I have any influence on that.
>

OK, that's good.

> And if they need to hack the kernel anyway, nothing prevents them from
> hacking it to support whatever properties they like even if we don't
> put any support for any generic _DSD properties into the mainline.
> That's why the whole "oh but this allows ARM vendors to abuse things"
> argument doesn't hold any water in my view.
>

While I agree with you, the argument which will be done and will win
most of the time to upstream something is that "we have shipped the
product with that table, we need to support it upstream...". I don't
think even you disagree with that ;)

> Yes, it may make it a bit easier for them to abuse things (as they
> don't have to hack the kernel to support properties already supported
> by it), but OTOH it sort of avoids the problem that multiple vendors
> could hack the kernel in different and possibly incompatible ways to
> make it support the same set of properties they all need.
>

Pity, but still better :)

>> Sure. On the side note, we should at-least have PRP0001 reserved
>> officially in the ACPI specification, I still see no mention about that
>> in the spec and both firmware and Linux are using it for a while now.
>
> The whole PRPxxxx range of device IDs is reserved.
>

Good, sorry I couldn't spot anything in ACPIv6.1 or uefi.org apart from
dsd mailing list.

>>> There is a metric ton of DT-centric code already in the kernel that is
>>> well understood and tested and it would be plain unreasonable to try
>>> to reinvent the wheel and (a) invent ACPI-specific ways of doing the
>>> same thing for every use case that is distinct enough and (b) write
>>> code using those ACPI-specific ways instead of already defined
>>> properties just in order to handle the same hardware in the same OS.
>>> [If you have to support different OSes, this is a different matter.]
>>>
>>
>> Agreed, but as you mentioned the question of different OS remains.
>>
>>> If you are saying "no DT properties in _DSD", this implies that code
>>> reuse is bad for some unspecified reason.  It is very difficult to me
>>> to agree with that position.
>>>
>>
>> No, I am not saying no. I am just worried where do we stop this and how
>> do we prevent ARM vendors exploiting this, and not rather contributing
>> to the ACPI specification enhancement.
>
> The ACPI spec may not be the right place to define support for things
> this series is about, though.  Ideally, they should be defined by
> other standards using ASL as a form of expression or representation,
> if you will.
>

Yes, that would be ideal.

> And that still needs to happen for different OSes to be able to use
> the ACPI tables in the same way (which doesn't even happen for
> different versions of Windows, but that's a different matter).
>
> However, adding support for additional _DSD properties to the mainline
> kernel need not prevent a standardization process like this from
> happening.  The point is that this process will take time and both the
> HW and the code in the kernel supporting it are available today.  The
> only problem is that the code is DT-centric and there are ACPI tables
> in the boards in question.
>
> If/when the new standard comes along, we will support it by default
> and then fall back to _DSD properties if information defined by it is
> not available.  Pretty much the same way as we fall back to
> driver-supplied data if we can get information we need from the
> platform firmware.
>

Yes that's much better.

-- 
Regards,
Sudeep

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Thu, Oct 6, 2016 at 4:20 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 06/10/16 14:04, Rafael J. Wysocki wrote:
>>
>> On Thu, Oct 6, 2016 at 12:29 PM, Sudeep Holla <sudeep.holla@arm.com>
>> wrote:
>
>
> [...]
>
>>> No doubt about that, though controlling it is another question.
>>> Especially ARM vendors who have some driver for DT might just use this
>>> method without anyone noticing, ignoring the ACPI method as everything
>>> just works out of box. That's the main worry. Also it also provides no
>>> incentive for them to work with ASWG to enhance ACPI specification.
>>
>>
>> They will need to hack the kernel to do that, because the mainline
>> will not support anything like regulator or pinctrl support based on
>> DT properties, at least as long as I have any influence on that.
>>
>
> OK, that's good.
>
>> And if they need to hack the kernel anyway, nothing prevents them from
>> hacking it to support whatever properties they like even if we don't
>> put any support for any generic _DSD properties into the mainline.
>> That's why the whole "oh but this allows ARM vendors to abuse things"
>> argument doesn't hold any water in my view.
>>
>
> While I agree with you, the argument which will be done and will win
> most of the time to upstream something is that "we have shipped the
> product with that table, we need to support it upstream...". I don't
> think even you disagree with that ;)

Well, maybe they should have talked to the upstream before shipping
the product with that table?  Failing to do so is like jumping to a
pool from a tower without checking how much water is there in it in
the first place ...

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Sudeep Holla]

On 06/10/16 18:05, Rafael J. Wysocki wrote:
> On Thu, Oct 6, 2016 at 4:20 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>

[..]

>>
>> While I agree with you, the argument which will be done and will win
>> most of the time to upstream something is that "we have shipped the
>> product with that table, we need to support it upstream...". I don't
>> think even you disagree with that ;)
>
> Well, maybe they should have talked to the upstream before shipping
> the product with that table?  Failing to do so is like jumping to a
> pool from a tower without checking how much water is there in it in
> the first place ...
>

Really ? I admit that I like this response and your stance here and I 
definitely support it but I have seen quite a few cases which tells me 
otherwise.

-- 
Regards,
Sudeep

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Thu, Oct 6, 2016 at 7:20 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 06/10/16 18:05, Rafael J. Wysocki wrote:
>>
>> On Thu, Oct 6, 2016 at 4:20 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>
>>>
>
> [..]
>
>>>
>>> While I agree with you, the argument which will be done and will win
>>> most of the time to upstream something is that "we have shipped the
>>> product with that table, we need to support it upstream...". I don't
>>> think even you disagree with that ;)
>>
>>
>> Well, maybe they should have talked to the upstream before shipping
>> the product with that table?  Failing to do so is like jumping to a
>> pool from a tower without checking how much water is there in it in
>> the first place ...
>>
>
> Really ? I admit that I like this response and your stance here and I
> definitely support it but I have seen quite a few cases which tells me
> otherwise.

Was I involved in any of those?

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Sudeep Holla]

On 11/10/16 01:05, Rafael J. Wysocki wrote:
> On Thu, Oct 6, 2016 at 7:20 PM, Sudeep Holla <sudeep.holla@arm.com>
> wrote:

[...]

>>> Well, maybe they should have talked to the upstream before
>>> shipping the product with that table?  Failing to do so is like
>>> jumping to a pool from a tower without checking how much water is
>>> there in it in the first place ...
>>>
>>
>> Really ? I admit that I like this response and your stance here and
>> I definitely support it but I have seen quite a few cases which
>> tells me otherwise.
>
> Was I involved in any of those?
>

No I wasn't referring to any particular case here. What I meant is we
have quite a few hacks/workarounds even on x86 for various reasons like:
other OS works, the product is shipped and firmware can't be fixed, and
so on. We might encounter similar situations even with DSD in future and
hence my worries.

-- 
Regards,
Sudeep

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Tue, Oct 11, 2016 at 10:57 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 11/10/16 01:05, Rafael J. Wysocki wrote:
>>
>> On Thu, Oct 6, 2016 at 7:20 PM, Sudeep Holla <sudeep.holla@arm.com>
>> wrote:
>
>
> [...]
>
>>>> Well, maybe they should have talked to the upstream before
>>>> shipping the product with that table?  Failing to do so is like
>>>> jumping to a pool from a tower without checking how much water is
>>>> there in it in the first place ...
>>>>
>>>
>>> Really ? I admit that I like this response and your stance here and
>>> I definitely support it but I have seen quite a few cases which
>>> tells me otherwise.
>>
>>
>> Was I involved in any of those?
>>
>
> No I wasn't referring to any particular case here. What I meant is we
> have quite a few hacks/workarounds even on x86 for various reasons like:
> other OS works, the product is shipped and firmware can't be fixed, and
> so on. We might encounter similar situations even with DSD in future and
> hence my worries.

There surely will be cases when people mess up their _DSD and
validation is insufficient and so on.  That seems to be the nature of
the business today.

What I'm talking about is not implementation mistakes, but cases when
people have a problem to address and decide to do that in a way that
cannot be done by the mainline (for technical reasons), and then
expect the mainline to accept this.  When in doubt about which way to
go, they should talk to the mainline in the first place or the
solution they choose may stay out of the tree forever (and become a
major pain to maintain going forward).

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

On Tue, Oct 11, 2016 at 01:59:14PM +0200, Rafael J. Wysocki wrote:

> expect the mainline to accept this.  When in doubt about which way to
> go, they should talk to the mainline in the first place or the
> solution they choose may stay out of the tree forever (and become a
> major pain to maintain going forward).

Realistically if people ship something in sufficient volume for people
to care we're going to end up supporting them.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

[-- Attachment #1: Type: text/plain, Size: 623 bytes --]

On Tuesday, October 11, 2016 03:15:58 PM Mark Brown wrote:
> On Tue, Oct 11, 2016 at 01:59:14PM +0200, Rafael J. Wysocki wrote:
> 
> > expect the mainline to accept this.  When in doubt about which way to
> > go, they should talk to the mainline in the first place or the
> > solution they choose may stay out of the tree forever (and become a
> > major pain to maintain going forward).
> 
> Realistically if people ship something in sufficient volume for people
> to care we're going to end up supporting them.

I wonder why the mainline doesn't boot on any flagship Android phones I'm
aware of, then. :-)

Thanks,
Rafael

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC 00/15] ACPI graph support

[Al Stone]

On 10/06/2016 08:20 AM, Sudeep Holla wrote:
> 
> 
> On 06/10/16 14:04, Rafael J. Wysocki wrote:
>> On Thu, Oct 6, 2016 at 12:29 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> 
> [...]
> 
>>> No doubt about that, though controlling it is another question.
>>> Especially ARM vendors who have some driver for DT might just use this
>>> method without anyone noticing, ignoring the ACPI method as everything
>>> just works out of box. That's the main worry. Also it also provides no
>>> incentive for them to work with ASWG to enhance ACPI specification.
>>
>> They will need to hack the kernel to do that, because the mainline
>> will not support anything like regulator or pinctrl support based on
>> DT properties, at least as long as I have any influence on that.
>>
> 
> OK, that's good.
> 
>> And if they need to hack the kernel anyway, nothing prevents them from
>> hacking it to support whatever properties they like even if we don't
>> put any support for any generic _DSD properties into the mainline.
>> That's why the whole "oh but this allows ARM vendors to abuse things"
>> argument doesn't hold any water in my view.
>>
> 
> While I agree with you, the argument which will be done and will win
> most of the time to upstream something is that "we have shipped the
> product with that table, we need to support it upstream...". I don't
> think even you disagree with that ;)
> 
>> Yes, it may make it a bit easier for them to abuse things (as they
>> don't have to hack the kernel to support properties already supported
>> by it), but OTOH it sort of avoids the problem that multiple vendors
>> could hack the kernel in different and possibly incompatible ways to
>> make it support the same set of properties they all need.
>>
> 
> Pity, but still better :)
> 
>>> Sure. On the side note, we should at-least have PRP0001 reserved
>>> officially in the ACPI specification, I still see no mention about that
>>> in the spec and both firmware and Linux are using it for a while now.
>>
>> The whole PRPxxxx range of device IDs is reserved.
>>
> 
> Good, sorry I couldn't spot anything in ACPIv6.1 or uefi.org apart from
> dsd mailing list.
> 
>>>> There is a metric ton of DT-centric code already in the kernel that is
>>>> well understood and tested and it would be plain unreasonable to try
>>>> to reinvent the wheel and (a) invent ACPI-specific ways of doing the
>>>> same thing for every use case that is distinct enough and (b) write
>>>> code using those ACPI-specific ways instead of already defined
>>>> properties just in order to handle the same hardware in the same OS.
>>>> [If you have to support different OSes, this is a different matter.]
>>>>
>>>
>>> Agreed, but as you mentioned the question of different OS remains.
>>>
>>>> If you are saying "no DT properties in _DSD", this implies that code
>>>> reuse is bad for some unspecified reason.  It is very difficult to me
>>>> to agree with that position.
>>>>
>>>
>>> No, I am not saying no. I am just worried where do we stop this and how
>>> do we prevent ARM vendors exploiting this, and not rather contributing
>>> to the ACPI specification enhancement.
>>
>> The ACPI spec may not be the right place to define support for things
>> this series is about, though.  Ideally, they should be defined by
>> other standards using ASL as a form of expression or representation,
>> if you will.
>>
> 
> Yes, that would be ideal.
> 
>> And that still needs to happen for different OSes to be able to use
>> the ACPI tables in the same way (which doesn't even happen for
>> different versions of Windows, but that's a different matter).
>>
>> However, adding support for additional _DSD properties to the mainline
>> kernel need not prevent a standardization process like this from
>> happening.  The point is that this process will take time and both the
>> HW and the code in the kernel supporting it are available today.  The
>> only problem is that the code is DT-centric and there are ACPI tables
>> in the boards in question.
>>
>> If/when the new standard comes along, we will support it by default
>> and then fall back to _DSD properties if information defined by it is
>> not available.  Pretty much the same way as we fall back to
>> driver-supplied data if we can get information we need from the
>> platform firmware.
>>
> 
> Yes that's much better.
> 

So where does this leave us?  What I take away from the discussion is this:

   a) each use of _DSD device properties in the kernel is to be evaluated on a
      case-by-case basis.  Therefore, Rafael and/or driver maintainers have the
      final say on acceptance in the Linux kernel.

   b) if it makes sense to generalize a property and update the ACPI spec
      instead of using _DSD device properties, that's the preferred path.

   c) if a _DSD device property is doing something that can and should be
      described in ACPI, do it in ACPI, not with a device property.

   d) re-use of DT bindings is an acceptable use for _DSD device properties,
      unless they contradict (b) or (c).

   e) Windows and Linux are already diverging in their use of _DSD.

Did I misunderstand anything?

If I didn't, then it seems a mechanism external to the Linux kernel to document
device properties is completely redundant, especially given item (e) -- they
will either be documented in DT, or documented by the driver.  And if that's the
case, then the dsd@acpica.org mailing list is irrelevant, or what's worse, makes
for duplicated work, and should just go away.

I'm fine with that, if that's what we're saying (it's less for me to do :), but
let's say it explicitly instead of re-hashing it every time _DSD is used.  The
above list is nice and simple and personally I'd rather have simple than a full
blown registration process.

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

Re: [RFC 00/15] ACPI graph support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1679 bytes --]

On Thu, Oct 06, 2016 at 11:20:49AM -0600, Al Stone wrote:

> So where does this leave us?  What I take away from the discussion is this:

>    a) each use of _DSD device properties in the kernel is to be evaluated on a
>       case-by-case basis.  Therefore, Rafael and/or driver maintainers have the
>       final say on acceptance in the Linux kernel.

Are we talking about pure _DSD here or are we not talking about
something definitely ACPI specific?  I was under the impression from the
thread title that there's something involving inter-node links going on
here since I thought it wasn't possible to represent that in _DSD alone.
I might be missing something here, I was copied in part way through the
thread.

>    e) Windows and Linux are already diverging in their use of _DSD.

I'm concerned we're seeing this outside of just _DSD.

> If I didn't, then it seems a mechanism external to the Linux kernel to document
> device properties is completely redundant, especially given item (e) -- they
> will either be documented in DT, or documented by the driver.  And if that's the
> case, then the dsd@acpica.org mailing list is irrelevant, or what's worse, makes
> for duplicated work, and should just go away.

> I'm fine with that, if that's what we're saying (it's less for me to do :), but
> let's say it explicitly instead of re-hashing it every time _DSD is used.  The
> above list is nice and simple and personally I'd rather have simple than a full
> blown registration process.

Like I've said before having some effort to try to pull the ACPI
community together so they're talking to each other and trying to come
up with best practices seems like a good idea.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [RFC 00/15] ACPI graph support

[Al Stone]

On 10/06/2016 02:14 PM, Mark Brown wrote:
> On Thu, Oct 06, 2016 at 11:20:49AM -0600, Al Stone wrote:
> 
>> So where does this leave us?  What I take away from the discussion is
>> this:
> 
>> a) each use of _DSD device properties in the kernel is to be evaluated on
>> a case-by-case basis.  Therefore, Rafael and/or driver maintainers have
>> the final say on acceptance in the Linux kernel.
> 
> Are we talking about pure _DSD here or are we not talking about something
> definitely ACPI specific?  I was under the impression from the thread title
> that there's something involving inter-node links going on here since I
> thought it wasn't possible to represent that in _DSD alone. I might be
> missing something here, I was copied in part way through the thread.

My intent was pure _DSD.  I have to wonder if inter-node links shouldn't be
handled more directly in ASL; that may be harder than using device properties,
but it might be the right long term solution.

>> e) Windows and Linux are already diverging in their use of _DSD.
> 
> I'm concerned we're seeing this outside of just _DSD.

Well, since I have not been appointed Emperor, I'm not sure it can be prevented.

>> If I didn't, then it seems a mechanism external to the Linux kernel to
>> document device properties is completely redundant, especially given item
>> (e) -- they will either be documented in DT, or documented by the driver.
>> And if that's the case, then the dsd@acpica.org mailing list is
>> irrelevant, or what's worse, makes for duplicated work, and should just
>> go away.
> 
>> I'm fine with that, if that's what we're saying (it's less for me to do
>> :), but let's say it explicitly instead of re-hashing it every time _DSD
>> is used.  The above list is nice and simple and personally I'd rather
>> have simple than a full blown registration process.
> 
> Like I've said before having some effort to try to pull the ACPI community
> together so they're talking to each other and trying to come up with best
> practices seems like a good idea.
> 

I agree it seems like a good idea.  As a practical matter, however, if it is
just going to be ignored, there's no real point, is there?

That also begs the question, though: if the idea is to re-use DT bindings as is,
why not just use DT directly and not bother with ACPI?  Wouldn't that be easier
on everyone?  If arm64 can use ACPI *or* DT, surely any other architecture can.

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

Re: [RFC 00/15] ACPI graph support

[Mark Rutland]

On Wed, Oct 05, 2016 at 02:41:29PM +0300, Mika Westerberg wrote:
> The whole purpose of PRP0001 ID is to allow DT bindings to be reused in
> ACPI systems, so that the drivers can just call device_property_* and
> get the properties regardless of the underlying firmware interface.
> 
> Are you saying that's not wanted?

Yes; at least from my PoV.

I have argued that this is the wrong level of abstraction multiple times,
across many threads, and in person at conferences.

To be clear, I have no issue with the general concept of _DSD; describing
intra-device properties with key-value pairs make sense. I am more than happy
for _DSD bindings to be inspired by DT bindings, but I am less than happy with
artificially tying the two together, and pretending that they are the same
thing when they aren't.

> > It is an RFC and my comment is that I do not like the direction
> > this ACPI->_DSD->DT is taking, I would like to understand where
> > this is intended to stop because I am getting worried.
> 
> I understand that if there is already an existing native ACPI way of
> doing things, that should be used. However, we do not have such thing
> for remote endpoints used between camera components,

As I have said before, this kind of thing should be handled by the ASWG. This
is a larger matter than a single device binding, there are related concerns to
be addressed (e.g. power management), and there are others who deal in ACPI who
need visibility of the issue, and need to have input.

> and on the other hand there is an exiting DT binding which only requires
> small changes to the v4l2 framework (convert it to use fwnodes) to get the
> thing supported on both DT and ACPI systems.

So we're blindly copying the DT binding, outside of the view of the ASWG and
other ACPI users, in a manner that's only going to work with Linux.

At this point, why bother with ACPI at all?

Mark.

Re: [RFC 00/15] ACPI graph support

[Rafael J. Wysocki]

On Tuesday, October 11, 2016 01:28:15 PM Mark Rutland wrote:
> On Wed, Oct 05, 2016 at 02:41:29PM +0300, Mika Westerberg wrote:
> > The whole purpose of PRP0001 ID is to allow DT bindings to be reused in
> > ACPI systems, so that the drivers can just call device_property_* and
> > get the properties regardless of the underlying firmware interface.
> > 
> > Are you saying that's not wanted?
> 
> Yes; at least from my PoV.
> 
> I have argued that this is the wrong level of abstraction multiple times,
> across many threads, and in person at conferences.
> 
> To be clear, I have no issue with the general concept of _DSD; describing
> intra-device properties with key-value pairs make sense. I am more than happy
> for _DSD bindings to be inspired by DT bindings, but I am less than happy with
> artificially tying the two together, and pretending that they are the same
> thing when they aren't.

Basically, we're trying to address a technical problem which is to avoid
having two different code paths, one for DT and one for ACPI, in every driver
(and in every piece of every subsystem that happens to take configuration
information from the outside of the kernel).

Today, Linux is the only OS really having this problem, so nobody else cares
realistically.  That may change in the future, but when exactly is rather hard
to say.

> > > It is an RFC and my comment is that I do not like the direction
> > > this ACPI->_DSD->DT is taking, I would like to understand where
> > > this is intended to stop because I am getting worried.
> > 
> > I understand that if there is already an existing native ACPI way of
> > doing things, that should be used. However, we do not have such thing
> > for remote endpoints used between camera components,
> 
> As I have said before, this kind of thing should be handled by the ASWG. This
> is a larger matter than a single device binding, there are related concerns to
> be addressed (e.g. power management), and there are others who deal in ACPI who
> need visibility of the issue, and need to have input.
> 
> > and on the other hand there is an exiting DT binding which only requires
> > small changes to the v4l2 framework (convert it to use fwnodes) to get the
> > thing supported on both DT and ACPI systems.
> 
> So we're blindly copying the DT binding, outside of the view of the ASWG and
> other ACPI users, in a manner that's only going to work with Linux.

I don't think that anyone is talking about copying DT bindings blindly.
At least I'm not FWIW.

> At this point, why bother with ACPI at all?

Because that's the only thing we can get from the firmware in many cases.

We are trying to make it possible to run Linux on systems that ship with
a different OS and with ACPI tables in the firmware without an option to
replace them with anything else.  In those cases, really, a lot of stuff
expected by the Linux kernel code using DT today is going to be missing,
because the other OSes expect to have drivers shipped along with the
platform with the specific knowledge on it based on the specific device
IDs used in it.  In such cases, it is not a problem if there are many
drivers handling the same piece of hardware in the ecosystem as long as
they match different ACPI/PNP device IDs.

That would be a problem in Linux, though, so we need to get the HW
configuration information to the drivers we have somehow.

Generally, there are two ways that can be done.  One is to put all of the
information needed and missing from the firmware-provided ACPI tables into the
kernel itself (which is rather not attractive, from a single-binary distro
kernel perspective, for example).  Another one is to be able to put that
*missing* information into something like an initrd matching the kernel that
will go with it.  It has to be on top of the firmware-provided ACPI stuff,
because usually we don't have enough information allowing us to replace it
with, say, a DT as a whole.

Another use case is when the board ships with ACPI tables in the firmware
and you want to extend it by adding some devices to it and you need to
describe those devices to the kernel somehow.  In that case, you only
really care about Linux and following DT is the most straightforward way.

It may not be possible to follow DT exactly for technical reasons (where
PM and similar are involved etc), but OTOH do we really have to reinvent
every piece of stuff already covered by DT just for the sake of it?

Thanks,
Rafael

Re: [RFC 00/15] ACPI graph support

[Sakari Ailus]

Hi Lorenzo,

On 10/05/16 12:22, Lorenzo Pieralisi wrote:
> [ +MarkR, MarkB, Rob, Al - I suspect they may want to have a say]
>
> On Wed, Oct 05, 2016 at 01:45:33AM +0300, Sakari Ailus wrote:
>> Hello everyone,
>>
>> I've been working awhile with my collegue Mika Westerberg to bring
>> firmware graph support to ACPI based systems. In practice 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.
>
> There is an ongoing effort to avoid wholesale import of DT bindings
> into ACPI, I am not a V4L2 expert but it seems to me that with patches
> like the one you have submitted we are getting closer and closer to
> achieving it instead of avoiding it.
>
> For your information, Al is working on a process to submit _DSD
> bindings and this patchset should at least be vetted within that
> context.
>
> It is an RFC and my comment is that I do not like the direction
> this ACPI->_DSD->DT is taking, I would like to understand where
> this is intended to stop because I am getting worried.

Thank you for your feedback.

The ACPI standard defines the syntax for _DSD property and data
extensions but it does not provide a solution for documenting these
properties. In other words, it does provide a mechanism but it does not
tell how and for which specific purposes that mechanism should or may be
used. The _DSD property extension specification strongly discourages the
use of _DSD for purposes that already are within the scope of the ACPI
standard. In this respect, the use of _DSD in this RFC patchset does
conform to the ACPI standard specifications.

That said, I do recognise that --- as we do have a single interface to
access the properties in drivers --- there must be uniform use of these
properties by firmware implementations or there will be problems. This
holds true independently of the firmware implementation, be it Device
tree, ACPI or both.

The Device tree binding documentation is part of the kernel and that's
what the FDT binaries are expected to use, too. The bindings are Linux
specific and, as far as I understand, cannot be expected to be used by
other operating systems even on same hardware.

ACPI should be more generic than that. Already, you can find ACPI BIOS
options for choosing the operating system you're running on. This is
hardly ideal.

AFAIU, the _DSD property database is intended for documenting the 
properties drivers use, and as long as the DT properties in the Linux 
kernel and in the _DSD property database match, a driver should be 
safely able to use those properties independently of the firmware 
implementation.

I do not agree with the proposition of making concepts and
implementation different on ACPI for the sake of it being ACPI. With
_DSD, the ACPI does support the same tree structure of nodes and 
properties as DT; I see no reason why "ACPI native" support for this 
class of constructs could not (or yet, even should not) use the same 
mechanisms as the Device tree does. There's simply no need to reinvent 
the wheel.

The patchset makes use of not only _DSD properties but also the _DSD
data extension. In order for that not to remain Linux specific, the next
logical step would be to extend the scope of the _DSD property database 
to include _DSD data extension in this respect and document the following:

1) how _DSD data extension graph nodes are referenced,

2) port and endpoint concepts and how they are to be used,

3) generic properties defined in
Documentation/devicetree/bindings/media/video-interfaces.txt to the _DSD 
database and

4) driver specific properties.

The ports and endpoints are not part (or have not been a part) of Device 
tree itself, but they are the established practice on Linux and they 
have worked well. Yet they do not have anything Linux specific as such: 
they simply are a mechanism to describe the hardware.

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

Re: [RFC 00/15] ACPI graph support

[Mark Rutland]

On Fri, Oct 07, 2016 at 12:10:54AM +0300, Sakari Ailus wrote:
> The ACPI standard defines the syntax for _DSD property and data
> extensions but it does not provide a solution for documenting these
> properties. In other words, it does provide a mechanism but it does not
> tell how and for which specific purposes that mechanism should or may be
> used. The _DSD property extension specification strongly discourages the
> use of _DSD for purposes that already are within the scope of the ACPI
> standard. In this respect, the use of _DSD in this RFC patchset does
> conform to the ACPI standard specifications.
> 
> That said, I do recognise that --- as we do have a single interface to
> access the properties in drivers --- there must be uniform use of these
> properties by firmware implementations or there will be problems. This
> holds true independently of the firmware implementation, be it Device
> tree, ACPI or both.

We only have that 'single interface to access the properties in drivers' (by
which I assume you mean the device_property_* accessors), because they were
specifically added with PRP in mind. So I don't follow.

> The Device tree binding documentation is part of the kernel and that's
> what the FDT binaries are expected to use, too. The bindings are Linux
> specific and, as far as I understand, cannot be expected to be used by
> other operating systems even on same hardware.

While it's true that Linux is the largest user, bindings are definitely not
intended to be OS specific, and there are other users today (e.g. FreeBSD).
There's an effort to properly factor out bindings into the devicetree.org spec,
where the very core DT bindings live today.

> I do not agree with the proposition of making concepts and implementation
> different on ACPI for the sake of it being ACPI.

No one is suggesting difference for difference's sake.

There are two large issues:

* PRP* circumvents ASWG, and its intended semantics are at best unclear. Due to
  this, it's a free-for-all.

* There *are* differences between ACPI and DT models which require
  consideration. For both componentised devices and pinctrl there are concerns
  w.r.t. how that interacts with the usual ACPI PM model, for example.

> With _DSD, the ACPI does support the same tree structure of nodes and
> properties as DT; I see no reason why "ACPI native" support for this class of
> constructs could not (or yet, even should not) use the same mechanisms as the
> Device tree does.There's simply no need to reinvent the wheel.

If the ASWG are fine with a DT-inspired (and perhaps identical) model, I have
no problem with that. I think it needs to be raised there first.

[...]

> The ports and endpoints are not part (or have not been a part) of Device
> tree itself, but they are the established practice on Linux and they have
> worked well. Yet they do not have anything Linux specific as such: they
> simply are a mechanism to describe the hardware.

This is true, in that it is a description of hardware, and certainly is not
intended to be Linux-specific.

It is not true, in that other communities have had no input. If other ACPI
users want to solve this problem in a different way, this will be
Linux-specific (and until it's in the ACPI spec, it must be assumed that is the
case).

Thanks,
Mark.