[PATCH v5 00/15] Add functionality to ipu3-cio2 driver allowing software_node connections to sensors on platforms designed for Windows

[PATCH v5 00/15] Add functionality to ipu3-cio2 driver allowing software_node connections to sensors on platforms designed for Windows

[Daniel Scally]

Hello all

v4:
https://lore.kernel.org/linux-media/20210103231235.792999-1-djrscally@gmail.com/T/#m11b7cb977e1b73fba1e625c3d6a189e2943a7783
v3:
https://lore.kernel.org/linux-media/20201224010907.263125-1-djrscally@gmail.com/T/#m37b831bb2b406917d6db5da9acf9ed35df65d72d
v2:
https://lore.kernel.org/linux-media/20201217234337.1983732-1-djrscally@gmail.com/T/#md93fd090009b42a6a98aed892aff0d38cf07e0cd
v1:
https://lore.kernel.org/linux-media/20201130133129.1024662-1-djrscally@gmail.com/T/#m91934e12e3d033da2e768e952ea3b4a125ee3e67

This series is to start adding support for webcams on laptops with ACPI tables
designed for use with CIO2 on Windows. This series extends the ipu3-cio2
driver to allow for patching the firmware via software_nodes if endpoints
aren't defined by ACPI.

I'm hopeful that most or all of this series could get picked up for 5.12.
We touch a few different areas (listed below), but I think the easiest
approach would be to merge everything through media tree. Rafael, Greg,
Mauro and Sergey; are you ok with that plan, or would you prefer a
different approach? Mauro; if that plan is ok (and of course assuming that
the rest of the patches are acked by their respective maintainers) could
we get a dedicated feature branch just in case the following series ends
up being ready in time too?

lib
  lib/test_printf.c: Use helper function to unwind array of
    software_nodes

base
  software_node: Fix refcounts in software_node_get_next_child()
  property: Return true in fwnode_device_is_available for NULL ops
  property: Call fwnode_graph_get_endpoint_by_id() for fwnode->secondary
  software_node: Enforce parent before child ordering of nodes arrays
  software_node: unregister software_nodes in reverse order
  include: fwnode.h: Define format macros for ports and endpoints

acpi
  acpi: Add acpi_dev_get_next_match_dev() and helper macro

media
  media: v4l2-core: v4l2-async: Check sd->fwnode->secondary in
    match_fwnode()
  ipu3-cio2: Add T: entry to MAINTAINERS
  ipu3-cio2: Rename ipu3-cio2.c
  ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver
  include: media: v4l2-fwnode: Include v4l2_fwnode_bus_type

Series-level changelog:
	- Rebased onto 5.11-rc1

Thanks
Dan

Andy Shevchenko (1):
  media: ipu3-cio2: Add headers that ipu3-cio2.h is direct user of

Daniel Scally (13):
  software_node: Fix refcounts in software_node_get_next_child()
  device property: Return true in fwnode_device_is_available for NULL
    ops
  device property: Call fwnode_graph_get_endpoint_by_id() for
    fwnode->secondary
  software_node: Enforce parent before child ordering of nodes arrays
  software_node: unregister software_nodes in reverse order
  device property: Define format macros for ports and endpoints
  lib/test_printf.c: Use helper function to unwind array of
    software_nodes
  ipu3-cio2: Add T: entry to MAINTAINERS
  ipu3-cio2: Rename ipu3-cio2.c
  media: v4l2-core: v4l2-async: Check sd->fwnode->secondary in
    match_fwnode()
  ACPI / bus: Add acpi_dev_get_next_match_dev() and helper macro
  media: v4l2-fwnode: Include v4l2_fwnode_bus_type
  ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver

Heikki Krogerus (1):
  software_node: Add support for fwnode_graph*() family of functions

 MAINTAINERS                                   |   2 +
 drivers/acpi/utils.c                          |  30 +-
 drivers/base/property.c                       |  15 +-
 drivers/base/swnode.c                         | 180 ++++++++--
 drivers/media/pci/intel/ipu3/Kconfig          |  18 +
 drivers/media/pci/intel/ipu3/Makefile         |   3 +
 drivers/media/pci/intel/ipu3/cio2-bridge.c    | 311 ++++++++++++++++++
 drivers/media/pci/intel/ipu3/cio2-bridge.h    | 125 +++++++
 .../ipu3/{ipu3-cio2.c => ipu3-cio2-main.c}    |  34 ++
 drivers/media/pci/intel/ipu3/ipu3-cio2.h      |  24 ++
 drivers/media/v4l2-core/v4l2-async.c          |   8 +
 drivers/media/v4l2-core/v4l2-fwnode.c         |  11 -
 include/acpi/acpi_bus.h                       |   7 +
 include/linux/fwnode.h                        |   7 +
 include/media/v4l2-fwnode.h                   |  22 ++
 lib/test_printf.c                             |   4 +-
 16 files changed, 763 insertions(+), 38 deletions(-)
 create mode 100644 drivers/media/pci/intel/ipu3/cio2-bridge.c
 create mode 100644 drivers/media/pci/intel/ipu3/cio2-bridge.h
 rename drivers/media/pci/intel/ipu3/{ipu3-cio2.c => ipu3-cio2-main.c} (98%)

-- 
2.25.1

[PATCH v5 01/15] software_node: Fix refcounts in software_node_get_next_child()

[Daniel Scally]

The software_node_get_next_child() function currently does not hold
references to the child software_node that it finds or put the ref that
is held against the old child - fix that.

Fixes: 59abd83672f7 ("drivers: base: Introducing software nodes to the firmware node framework")
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v5:

	- None

 drivers/base/swnode.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 4a4b2008fbc2..4fcc1a6fb724 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -443,14 +443,18 @@ software_node_get_next_child(const struct fwnode_handle *fwnode,
 	struct swnode *c = to_swnode(child);
 
 	if (!p || list_empty(&p->children) ||
-	    (c && list_is_last(&c->entry, &p->children)))
+	    (c && list_is_last(&c->entry, &p->children))) {
+		fwnode_handle_put(child);
 		return NULL;
+	}
 
 	if (c)
 		c = list_next_entry(c, entry);
 	else
 		c = list_first_entry(&p->children, struct swnode, entry);
-	return &c->fwnode;
+
+	fwnode_handle_put(child);
+	return fwnode_handle_get(&c->fwnode);
 }
 
 static struct fwnode_handle *
-- 
2.25.1

Re: [PATCH v5 01/15] software_node: Fix refcounts in software_node_get_next_child()

[Heikki Krogerus]

On Thu, Jan 07, 2021 at 01:28:24PM +0000, Daniel Scally wrote:
> The software_node_get_next_child() function currently does not hold
> references to the child software_node that it finds or put the ref that
> is held against the old child - fix that.
> 
> Fixes: 59abd83672f7 ("drivers: base: Introducing software nodes to the firmware node framework")
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Changes in v5:
> 
> 	- None
> 
>  drivers/base/swnode.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 4a4b2008fbc2..4fcc1a6fb724 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -443,14 +443,18 @@ software_node_get_next_child(const struct fwnode_handle *fwnode,
>  	struct swnode *c = to_swnode(child);
>  
>  	if (!p || list_empty(&p->children) ||
> -	    (c && list_is_last(&c->entry, &p->children)))
> +	    (c && list_is_last(&c->entry, &p->children))) {
> +		fwnode_handle_put(child);
>  		return NULL;
> +	}
>  
>  	if (c)
>  		c = list_next_entry(c, entry);
>  	else
>  		c = list_first_entry(&p->children, struct swnode, entry);
> -	return &c->fwnode;
> +
> +	fwnode_handle_put(child);
> +	return fwnode_handle_get(&c->fwnode);
>  }
>  
>  static struct fwnode_handle *
> -- 
> 2.25.1

thanks,

-- 
heikki

Re: [PATCH v5 01/15] software_node: Fix refcounts in software_node_get_next_child()

[Greg KH]

On Thu, Jan 07, 2021 at 01:28:24PM +0000, Daniel Scally wrote:
> The software_node_get_next_child() function currently does not hold
> references to the child software_node that it finds or put the ref that
> is held against the old child - fix that.
> 
> Fixes: 59abd83672f7 ("drivers: base: Introducing software nodes to the firmware node framework")
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> Changes in v5:

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[PATCH v5 02/15] media: ipu3-cio2: Add headers that ipu3-cio2.h is direct user of

[Daniel Scally]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Add headers that ipu3-cio2.h is direct user of.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Daniel Scally <djrscally@gmail.com>
Tested-by: Daniel Scally <djrscally@gmail.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v5:

	- Added my Signed-off-by this time, apparently I need to do that.

 drivers/media/pci/intel/ipu3/ipu3-cio2.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.h b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
index ccf0b85ae36f..62187ab5ae43 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.h
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
@@ -4,8 +4,26 @@
 #ifndef __IPU3_CIO2_H
 #define __IPU3_CIO2_H
 
+#include <linux/bits.h>
+#include <linux/dma-mapping.h>
+#include <linux/kernel.h>
+#include <linux/mutex.h>
 #include <linux/types.h>
 
+#include <asm/page.h>
+
+#include <media/media-device.h>
+#include <media/media-entity.h>
+#include <media/v4l2-async.h>
+#include <media/v4l2-dev.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-subdev.h>
+#include <media/videobuf2-core.h>
+#include <media/videobuf2-v4l2.h>
+
+struct cio2_fbpt_entry;		/* defined here, after the first usage */
+struct pci_dev;
+
 #define CIO2_NAME					"ipu3-cio2"
 #define CIO2_DEVICE_NAME				"Intel IPU3 CIO2"
 #define CIO2_ENTITY_NAME				"ipu3-csi2"
-- 
2.25.1

[PATCH v5 03/15] device property: Return true in fwnode_device_is_available for NULL ops

[Daniel Scally]

Some types of fwnode_handle do not implement the device_is_available()
check, such as those created by software_nodes. There isn't really a
meaningful way to check for the availability of a device that doesn't
actually exist, so if the check isn't implemented just assume that the
"device" is present.

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v5:

	- Changed the commit subject

 drivers/base/property.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 35b95c6ac0c6..0bf5260f14c6 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -837,9 +837,15 @@ EXPORT_SYMBOL_GPL(fwnode_handle_put);
 /**
  * fwnode_device_is_available - check if a device is available for use
  * @fwnode: Pointer to the fwnode of the device.
+ *
+ * For fwnode node types that don't implement the .device_is_available()
+ * operation, this function returns true.
  */
 bool fwnode_device_is_available(const struct fwnode_handle *fwnode)
 {
+	if (!fwnode_has_op(fwnode, device_is_available))
+		return true;
+
 	return fwnode_call_bool_op(fwnode, device_is_available);
 }
 EXPORT_SYMBOL_GPL(fwnode_device_is_available);
-- 
2.25.1

Re: [PATCH v5 03/15] device property: Return true in fwnode_device_is_available for NULL ops

[Heikki Krogerus]

On Thu, Jan 07, 2021 at 01:28:26PM +0000, Daniel Scally wrote:
> Some types of fwnode_handle do not implement the device_is_available()
> check, such as those created by software_nodes. There isn't really a
> meaningful way to check for the availability of a device that doesn't
> actually exist, so if the check isn't implemented just assume that the
> "device" is present.
> 
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Changes in v5:
> 
> 	- Changed the commit subject
> 
>  drivers/base/property.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index 35b95c6ac0c6..0bf5260f14c6 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -837,9 +837,15 @@ EXPORT_SYMBOL_GPL(fwnode_handle_put);
>  /**
>   * fwnode_device_is_available - check if a device is available for use
>   * @fwnode: Pointer to the fwnode of the device.
> + *
> + * For fwnode node types that don't implement the .device_is_available()
> + * operation, this function returns true.
>   */
>  bool fwnode_device_is_available(const struct fwnode_handle *fwnode)
>  {
> +	if (!fwnode_has_op(fwnode, device_is_available))
> +		return true;
> +
>  	return fwnode_call_bool_op(fwnode, device_is_available);
>  }
>  EXPORT_SYMBOL_GPL(fwnode_device_is_available);
> -- 
> 2.25.1

thanks,

-- 
heikki

Re: [PATCH v5 03/15] device property: Return true in fwnode_device_is_available for NULL ops

[Greg KH]

On Thu, Jan 07, 2021 at 01:28:26PM +0000, Daniel Scally wrote:
> Some types of fwnode_handle do not implement the device_is_available()
> check, such as those created by software_nodes. There isn't really a
> meaningful way to check for the availability of a device that doesn't
> actually exist, so if the check isn't implemented just assume that the
> "device" is present.
> 
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> Changes in v5:
> 
> 	- Changed the commit subject
> 

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[PATCH v5 04/15] device property: Call fwnode_graph_get_endpoint_by_id() for fwnode->secondary

[Daniel Scally]

This function is used to find fwnode endpoints against a device. In
some instances those endpoints are software nodes which are children of
fwnode->secondary. Add support to fwnode_graph_get_endpoint_by_id() to
find those endpoints by recursively calling itself passing the ptr to
fwnode->secondary in the event no endpoint is found for the primary.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v5:

	- Changed the commit subject

 drivers/base/property.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/base/property.c b/drivers/base/property.c
index 0bf5260f14c6..1421e9548857 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1215,7 +1215,14 @@ fwnode_graph_get_endpoint_by_id(const struct fwnode_handle *fwnode,
 		best_ep_id = fwnode_ep.id;
 	}
 
-	return best_ep;
+	if (best_ep)
+		return best_ep;
+
+	if (fwnode && !IS_ERR_OR_NULL(fwnode->secondary))
+		return fwnode_graph_get_endpoint_by_id(fwnode->secondary, port,
+						       endpoint, flags);
+
+	return NULL;
 }
 EXPORT_SYMBOL_GPL(fwnode_graph_get_endpoint_by_id);
 
-- 
2.25.1

Re: [PATCH v5 04/15] device property: Call fwnode_graph_get_endpoint_by_id() for fwnode->secondary

[Greg KH]

On Thu, Jan 07, 2021 at 01:28:27PM +0000, Daniel Scally wrote:
> This function is used to find fwnode endpoints against a device. In
> some instances those endpoints are software nodes which are children of
> fwnode->secondary. Add support to fwnode_graph_get_endpoint_by_id() to
> find those endpoints by recursively calling itself passing the ptr to
> fwnode->secondary in the event no endpoint is found for the primary.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> Changes in v5:
> 
> 	- Changed the commit subject
> 
>  drivers/base/property.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[PATCH v5 05/15] software_node: Enforce parent before child ordering of nodes arrays

[Daniel Scally]

Registering software_nodes with the .parent member set to point to a
currently unregistered software_node has the potential for problems,
so enforce parent -> child ordering in arrays passed in to
software_node_register_nodes().

Software nodes that are children of another software node should be
unregistered before their parent. To allow easy unregistering of an array
of software_nodes ordered parent to child, reverse the order in which
software_node_unregister_nodes() unregisters software_nodes.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v5:

	- None

 drivers/base/swnode.c | 42 ++++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 4fcc1a6fb724..166c5cc73f39 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -692,7 +692,11 @@ swnode_register(const struct software_node *node, struct swnode *parent,
  * software_node_register_nodes - Register an array of software nodes
  * @nodes: Zero terminated array of software nodes to be registered
  *
- * Register multiple software nodes at once.
+ * Register multiple software nodes at once. If any node in the array
+ * has its .parent pointer set (which can only be to another software_node),
+ * then its parent **must** have been registered before it is; either outside
+ * of this function or by ordering the array such that parent comes before
+ * child.
  */
 int software_node_register_nodes(const struct software_node *nodes)
 {
@@ -700,14 +704,23 @@ int software_node_register_nodes(const struct software_node *nodes)
 	int i;
 
 	for (i = 0; nodes[i].name; i++) {
-		ret = software_node_register(&nodes[i]);
-		if (ret) {
-			software_node_unregister_nodes(nodes);
-			return ret;
+		const struct software_node *parent = nodes[i].parent;
+
+		if (parent && !software_node_to_swnode(parent)) {
+			ret = -EINVAL;
+			goto err_unregister_nodes;
 		}
+
+		ret = software_node_register(&nodes[i]);
+		if (ret)
+			goto err_unregister_nodes;
 	}
 
 	return 0;
+
+err_unregister_nodes:
+	software_node_unregister_nodes(nodes);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(software_node_register_nodes);
 
@@ -715,18 +728,23 @@ EXPORT_SYMBOL_GPL(software_node_register_nodes);
  * software_node_unregister_nodes - Unregister an array of software nodes
  * @nodes: Zero terminated array of software nodes to be unregistered
  *
- * Unregister multiple software nodes at once.
+ * Unregister multiple software nodes at once. If parent pointers are set up
+ * in any of the software nodes then the array **must** be ordered such that
+ * parents come before their children.
  *
- * NOTE: Be careful using this call if the nodes had parent pointers set up in
- * them before registering.  If so, it is wiser to remove the nodes
- * individually, in the correct order (child before parent) instead of relying
- * on the sequential order of the list of nodes in the array.
+ * NOTE: If you are uncertain whether the array is ordered such that
+ * parents will be unregistered before their children, it is wiser to
+ * remove the nodes individually, in the correct order (child before
+ * parent).
  */
 void software_node_unregister_nodes(const struct software_node *nodes)
 {
-	int i;
+	unsigned int i = 0;
+
+	while (nodes[i].name)
+		i++;
 
-	for (i = 0; nodes[i].name; i++)
+	while (i--)
 		software_node_unregister(&nodes[i]);
 }
 EXPORT_SYMBOL_GPL(software_node_unregister_nodes);
-- 
2.25.1

Re: [PATCH v5 05/15] software_node: Enforce parent before child ordering of nodes arrays

[Heikki Krogerus]

On Thu, Jan 07, 2021 at 01:28:28PM +0000, Daniel Scally wrote:
> Registering software_nodes with the .parent member set to point to a
> currently unregistered software_node has the potential for problems,
> so enforce parent -> child ordering in arrays passed in to
> software_node_register_nodes().
> 
> Software nodes that are children of another software node should be
> unregistered before their parent. To allow easy unregistering of an array
> of software_nodes ordered parent to child, reverse the order in which
> software_node_unregister_nodes() unregisters software_nodes.
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Changes in v5:
> 
> 	- None
> 
>  drivers/base/swnode.c | 42 ++++++++++++++++++++++++++++++------------
>  1 file changed, 30 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 4fcc1a6fb724..166c5cc73f39 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -692,7 +692,11 @@ swnode_register(const struct software_node *node, struct swnode *parent,
>   * software_node_register_nodes - Register an array of software nodes
>   * @nodes: Zero terminated array of software nodes to be registered
>   *
> - * Register multiple software nodes at once.
> + * Register multiple software nodes at once. If any node in the array
> + * has its .parent pointer set (which can only be to another software_node),
> + * then its parent **must** have been registered before it is; either outside
> + * of this function or by ordering the array such that parent comes before
> + * child.
>   */
>  int software_node_register_nodes(const struct software_node *nodes)
>  {
> @@ -700,14 +704,23 @@ int software_node_register_nodes(const struct software_node *nodes)
>  	int i;
>  
>  	for (i = 0; nodes[i].name; i++) {
> -		ret = software_node_register(&nodes[i]);
> -		if (ret) {
> -			software_node_unregister_nodes(nodes);
> -			return ret;
> +		const struct software_node *parent = nodes[i].parent;
> +
> +		if (parent && !software_node_to_swnode(parent)) {
> +			ret = -EINVAL;
> +			goto err_unregister_nodes;
>  		}
> +
> +		ret = software_node_register(&nodes[i]);
> +		if (ret)
> +			goto err_unregister_nodes;
>  	}
>  
>  	return 0;
> +
> +err_unregister_nodes:
> +	software_node_unregister_nodes(nodes);
> +	return ret;
>  }
>  EXPORT_SYMBOL_GPL(software_node_register_nodes);
>  
> @@ -715,18 +728,23 @@ EXPORT_SYMBOL_GPL(software_node_register_nodes);
>   * software_node_unregister_nodes - Unregister an array of software nodes
>   * @nodes: Zero terminated array of software nodes to be unregistered
>   *
> - * Unregister multiple software nodes at once.
> + * Unregister multiple software nodes at once. If parent pointers are set up
> + * in any of the software nodes then the array **must** be ordered such that
> + * parents come before their children.
>   *
> - * NOTE: Be careful using this call if the nodes had parent pointers set up in
> - * them before registering.  If so, it is wiser to remove the nodes
> - * individually, in the correct order (child before parent) instead of relying
> - * on the sequential order of the list of nodes in the array.
> + * NOTE: If you are uncertain whether the array is ordered such that
> + * parents will be unregistered before their children, it is wiser to
> + * remove the nodes individually, in the correct order (child before
> + * parent).
>   */
>  void software_node_unregister_nodes(const struct software_node *nodes)
>  {
> -	int i;
> +	unsigned int i = 0;
> +
> +	while (nodes[i].name)
> +		i++;
>  
> -	for (i = 0; nodes[i].name; i++)
> +	while (i--)
>  		software_node_unregister(&nodes[i]);
>  }
>  EXPORT_SYMBOL_GPL(software_node_unregister_nodes);
> -- 
> 2.25.1

-- 
heikki

Re: [PATCH v5 05/15] software_node: Enforce parent before child ordering of nodes arrays

[Greg KH]

On Thu, Jan 07, 2021 at 01:28:28PM +0000, Daniel Scally wrote:
> Registering software_nodes with the .parent member set to point to a
> currently unregistered software_node has the potential for problems,
> so enforce parent -> child ordering in arrays passed in to
> software_node_register_nodes().
> 
> Software nodes that are children of another software node should be
> unregistered before their parent. To allow easy unregistering of an array
> of software_nodes ordered parent to child, reverse the order in which
> software_node_unregister_nodes() unregisters software_nodes.
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> Changes in v5:
> 
> 	- None

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[PATCH v5 06/15] software_node: unregister software_nodes in reverse order

[Daniel Scally]

To maintain consistency with software_node_unregister_nodes(), reverse
the order in which the software_node_unregister_node_group() function
unregisters nodes.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v5:

	- None

 drivers/base/swnode.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 166c5cc73f39..6f7443c6d3b5 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -779,16 +779,23 @@ EXPORT_SYMBOL_GPL(software_node_register_node_group);
  * software_node_unregister_node_group - Unregister a group of software nodes
  * @node_group: NULL terminated array of software node pointers to be unregistered
  *
- * Unregister multiple software nodes at once.
+ * Unregister multiple software nodes at once. The array will be unwound in
+ * reverse order (i.e. last entry first) and thus if any members of the array are
+ * children of another member then the children must appear later in the list such
+ * that they are unregistered first.
  */
-void software_node_unregister_node_group(const struct software_node **node_group)
+void software_node_unregister_node_group(
+		const struct software_node **node_group)
 {
-	unsigned int i;
+	unsigned int i = 0;
 
 	if (!node_group)
 		return;
 
-	for (i = 0; node_group[i]; i++)
+	while (node_group[i])
+		i++;
+
+	while (i--)
 		software_node_unregister(node_group[i]);
 }
 EXPORT_SYMBOL_GPL(software_node_unregister_node_group);
-- 
2.25.1

Re: [PATCH v5 06/15] software_node: unregister software_nodes in reverse order

[Heikki Krogerus]

On Thu, Jan 07, 2021 at 01:28:29PM +0000, Daniel Scally wrote:
> To maintain consistency with software_node_unregister_nodes(), reverse
> the order in which the software_node_unregister_node_group() function
> unregisters nodes.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Changes in v5:
> 
> 	- None
> 
>  drivers/base/swnode.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index 166c5cc73f39..6f7443c6d3b5 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -779,16 +779,23 @@ EXPORT_SYMBOL_GPL(software_node_register_node_group);
>   * software_node_unregister_node_group - Unregister a group of software nodes
>   * @node_group: NULL terminated array of software node pointers to be unregistered
>   *
> - * Unregister multiple software nodes at once.
> + * Unregister multiple software nodes at once. The array will be unwound in
> + * reverse order (i.e. last entry first) and thus if any members of the array are
> + * children of another member then the children must appear later in the list such
> + * that they are unregistered first.
>   */
> -void software_node_unregister_node_group(const struct software_node **node_group)
> +void software_node_unregister_node_group(
> +		const struct software_node **node_group)
>  {
> -	unsigned int i;
> +	unsigned int i = 0;
>  
>  	if (!node_group)
>  		return;
>  
> -	for (i = 0; node_group[i]; i++)
> +	while (node_group[i])
> +		i++;
> +
> +	while (i--)
>  		software_node_unregister(node_group[i]);
>  }
>  EXPORT_SYMBOL_GPL(software_node_unregister_node_group);
> -- 
> 2.25.1

-- 
heikki

Re: [PATCH v5 06/15] software_node: unregister software_nodes in reverse order

[Greg KH]

On Thu, Jan 07, 2021 at 01:28:29PM +0000, Daniel Scally wrote:
> To maintain consistency with software_node_unregister_nodes(), reverse
> the order in which the software_node_unregister_node_group() function
> unregisters nodes.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[PATCH v5 07/15] device property: Define format macros for ports and endpoints

[Daniel Scally]

OF, ACPI and software_nodes all implement graphs including nodes for ports
and endpoints. These are all intended to be named with a common schema,
as "port@n" and "endpoint@n" where n is an unsigned int representing the
index of the node. To ensure commonality across the subsystems, provide a
set of macros to define the format.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v5:

	- Changed commit subject

 include/linux/fwnode.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index fde4ad97564c..77414e431e89 100644
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -50,6 +50,13 @@ struct fwnode_endpoint {
 	const struct fwnode_handle *local_fwnode;
 };
 
+/*
+ * ports and endpoints defined as software_nodes should all follow a common
+ * naming scheme; use these macros to ensure commonality.
+ */
+#define SWNODE_GRAPH_PORT_NAME_FMT		"port@%u"
+#define SWNODE_GRAPH_ENDPOINT_NAME_FMT		"endpoint@%u"
+
 #define NR_FWNODE_REFERENCE_ARGS	8
 
 /**
-- 
2.25.1

Re: [PATCH v5 07/15] device property: Define format macros for ports and endpoints

[Heikki Krogerus]

On Thu, Jan 07, 2021 at 01:28:30PM +0000, Daniel Scally wrote:
> OF, ACPI and software_nodes all implement graphs including nodes for ports
> and endpoints. These are all intended to be named with a common schema,
> as "port@n" and "endpoint@n" where n is an unsigned int representing the
> index of the node. To ensure commonality across the subsystems, provide a
> set of macros to define the format.
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Changes in v5:
> 
> 	- Changed commit subject
> 
>  include/linux/fwnode.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index fde4ad97564c..77414e431e89 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -50,6 +50,13 @@ struct fwnode_endpoint {
>  	const struct fwnode_handle *local_fwnode;
>  };
>  
> +/*
> + * ports and endpoints defined as software_nodes should all follow a common
> + * naming scheme; use these macros to ensure commonality.
> + */
> +#define SWNODE_GRAPH_PORT_NAME_FMT		"port@%u"
> +#define SWNODE_GRAPH_ENDPOINT_NAME_FMT		"endpoint@%u"
> +
>  #define NR_FWNODE_REFERENCE_ARGS	8
>  
>  /**
> -- 
> 2.25.1

-- 
heikki

Re: [PATCH v5 07/15] device property: Define format macros for ports and endpoints

[Laurent Pinchart]

Hi Rafael,

Could you please let us know with an Acked-by if this patch can be
merged through the linux-media tree for v5.12 ? The cover letter
contains additional details (in a nutshell, this is a cross-tree series
and we would like to avoid topic branches).

On Thu, Jan 07, 2021 at 01:28:30PM +0000, Daniel Scally wrote:
> OF, ACPI and software_nodes all implement graphs including nodes for ports
> and endpoints. These are all intended to be named with a common schema,
> as "port@n" and "endpoint@n" where n is an unsigned int representing the
> index of the node. To ensure commonality across the subsystems, provide a
> set of macros to define the format.
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> Changes in v5:
> 
> 	- Changed commit subject
> 
>  include/linux/fwnode.h | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index fde4ad97564c..77414e431e89 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -50,6 +50,13 @@ struct fwnode_endpoint {
>  	const struct fwnode_handle *local_fwnode;
>  };
>  
> +/*
> + * ports and endpoints defined as software_nodes should all follow a common
> + * naming scheme; use these macros to ensure commonality.
> + */
> +#define SWNODE_GRAPH_PORT_NAME_FMT		"port@%u"
> +#define SWNODE_GRAPH_ENDPOINT_NAME_FMT		"endpoint@%u"
> +
>  #define NR_FWNODE_REFERENCE_ARGS	8
>  
>  /**

-- 
Regards,

Laurent Pinchart

[PATCH v5 08/15] software_node: Add support for fwnode_graph*() family of functions

[Daniel Scally]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

This implements the remaining .graph_*() callbacks in the fwnode
operations structure for the software nodes. That makes the
fwnode_graph_*() functions available in the drivers also when software
nodes are used.

The implementation tries to mimic the "OF graph" as much as possible, but
there is no support for the "reg" device property. The ports will need to
have the index in their  name which starts with "port@" (for example
"port@0", "port@1", ...) and endpoints will use the index of the software
node that is given to them during creation. The port nodes can also be
grouped under a specially named "ports" subnode, just like in DT, if
necessary.

The remote-endpoints are reference properties under the endpoint nodes
that are named "remote-endpoint".

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Co-developed-by: Daniel Scally <djrscally@gmail.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v5:

	- Cosmetic changes only

 drivers/base/swnode.c | 115 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 114 insertions(+), 1 deletion(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index 6f7443c6d3b5..9104a0abd531 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -540,6 +540,115 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode,
 	return 0;
 }
 
+static struct fwnode_handle *
+swnode_graph_find_next_port(const struct fwnode_handle *parent,
+			    struct fwnode_handle *port)
+{
+	struct fwnode_handle *old = port;
+
+	while ((port = software_node_get_next_child(parent, old))) {
+		/*
+		 * fwnode ports have naming style "port@", so we search for any
+		 * children that follow that convention.
+		 */
+		if (!strncmp(to_swnode(port)->node->name, "port@",
+			     strlen("port@")))
+			return port;
+		old = port;
+	}
+
+	return NULL;
+}
+
+static struct fwnode_handle *
+software_node_graph_get_next_endpoint(const struct fwnode_handle *fwnode,
+				      struct fwnode_handle *endpoint)
+{
+	struct swnode *swnode = to_swnode(fwnode);
+	struct fwnode_handle *parent;
+	struct fwnode_handle *port;
+
+	if (!swnode)
+		return NULL;
+
+	if (endpoint) {
+		port = software_node_get_parent(endpoint);
+		parent = software_node_get_parent(port);
+	} else {
+		parent = software_node_get_named_child_node(fwnode, "ports");
+		if (!parent)
+			parent = software_node_get(&swnode->fwnode);
+
+		port = swnode_graph_find_next_port(parent, NULL);
+	}
+
+	for (; port; port = swnode_graph_find_next_port(parent, port)) {
+		endpoint = software_node_get_next_child(port, endpoint);
+		if (endpoint) {
+			fwnode_handle_put(port);
+			break;
+		}
+	}
+
+	fwnode_handle_put(parent);
+
+	return endpoint;
+}
+
+static struct fwnode_handle *
+software_node_graph_get_remote_endpoint(const struct fwnode_handle *fwnode)
+{
+	struct swnode *swnode = to_swnode(fwnode);
+	const struct software_node_ref_args *ref;
+	const struct property_entry *prop;
+
+	if (!swnode)
+		return NULL;
+
+	prop = property_entry_get(swnode->node->properties, "remote-endpoint");
+	if (!prop || prop->type != DEV_PROP_REF || prop->is_inline)
+		return NULL;
+
+	ref = prop->pointer;
+
+	return software_node_get(software_node_fwnode(ref[0].node));
+}
+
+static struct fwnode_handle *
+software_node_graph_get_port_parent(struct fwnode_handle *fwnode)
+{
+	struct swnode *swnode = to_swnode(fwnode);
+
+	swnode = swnode->parent;
+	if (swnode && !strcmp(swnode->node->name, "ports"))
+		swnode = swnode->parent;
+
+	return swnode ? software_node_get(&swnode->fwnode) : NULL;
+}
+
+static int
+software_node_graph_parse_endpoint(const struct fwnode_handle *fwnode,
+				   struct fwnode_endpoint *endpoint)
+{
+	struct swnode *swnode = to_swnode(fwnode);
+	const char *parent_name = swnode->parent->node->name;
+	int ret;
+
+	if (strlen("port@") >= strlen(parent_name) ||
+	    strncmp(parent_name, "port@", strlen("port@")))
+		return -EINVAL;
+
+	/* Ports have naming style "port@n", we need to select the n */
+	ret = kstrtou32(parent_name + strlen("port@"), 10, &endpoint->port);
+	if (ret)
+		return ret;
+
+	endpoint->id = swnode->id;
+	endpoint->local_fwnode = fwnode;
+
+	return 0;
+}
+
 static const struct fwnode_operations software_node_ops = {
 	.get = software_node_get,
 	.put = software_node_put,
@@ -551,7 +660,11 @@ static const struct fwnode_operations software_node_ops = {
 	.get_parent = software_node_get_parent,
 	.get_next_child_node = software_node_get_next_child,
 	.get_named_child_node = software_node_get_named_child_node,
-	.get_reference_args = software_node_get_reference_args
+	.get_reference_args = software_node_get_reference_args,
+	.graph_get_next_endpoint = software_node_graph_get_next_endpoint,
+	.graph_get_remote_endpoint = software_node_graph_get_remote_endpoint,
+	.graph_get_port_parent = software_node_graph_get_port_parent,
+	.graph_parse_endpoint = software_node_graph_parse_endpoint,
 };
 
 /* -------------------------------------------------------------------------- */
-- 
2.25.1

Re: [PATCH v5 08/15] software_node: Add support for fwnode_graph*() family of functions

[Greg KH]

On Thu, Jan 07, 2021 at 01:28:31PM +0000, Daniel Scally wrote:
> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 
> This implements the remaining .graph_*() callbacks in the fwnode
> operations structure for the software nodes. That makes the
> fwnode_graph_*() functions available in the drivers also when software
> nodes are used.
> 
> The implementation tries to mimic the "OF graph" as much as possible, but
> there is no support for the "reg" device property. The ports will need to
> have the index in their  name which starts with "port@" (for example
> "port@0", "port@1", ...) and endpoints will use the index of the software
> node that is given to them during creation. The port nodes can also be
> grouped under a specially named "ports" subnode, just like in DT, if
> necessary.
> 
> The remote-endpoints are reference properties under the endpoint nodes
> that are named "remote-endpoint".
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Co-developed-by: Daniel Scally <djrscally@gmail.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> Changes in v5:
> 
> 	- Cosmetic changes only

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[PATCH v5 09/15] lib/test_printf.c: Use helper function to unwind array of software_nodes

[Daniel Scally]

Use the software_node_unregister_nodes() helper function to unwind this
array in a cleaner way.

Acked-by: Petr Mladek <pmladek@suse.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v5:

	- None

 lib/test_printf.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 7ac87f18a10f..7d60f24240a4 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -644,9 +644,7 @@ static void __init fwnode_pointer(void)
 	test(second_name, "%pfwP", software_node_fwnode(&softnodes[1]));
 	test(third_name, "%pfwP", software_node_fwnode(&softnodes[2]));
 
-	software_node_unregister(&softnodes[2]);
-	software_node_unregister(&softnodes[1]);
-	software_node_unregister(&softnodes[0]);
+	software_node_unregister_nodes(softnodes);
 }
 
 static void __init
-- 
2.25.1

Re: [PATCH v5 09/15] lib/test_printf.c: Use helper function to unwind array of software_nodes

[Laurent Pinchart]

Hi Peter, Steven and Sergey,

Could you please let us know if you're fine with this patch getting
merged in v5.12 through the linux-media tree ? The cover letter contains
additional details (in a nutshell, this is a cross-tree series and we
would like to avoid topic branches if possible).

On Thu, Jan 07, 2021 at 01:28:32PM +0000, Daniel Scally wrote:
> Use the software_node_unregister_nodes() helper function to unwind this
> array in a cleaner way.
> 
> Acked-by: Petr Mladek <pmladek@suse.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> Changes in v5:
> 
> 	- None
> 
>  lib/test_printf.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index 7ac87f18a10f..7d60f24240a4 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -644,9 +644,7 @@ static void __init fwnode_pointer(void)
>  	test(second_name, "%pfwP", software_node_fwnode(&softnodes[1]));
>  	test(third_name, "%pfwP", software_node_fwnode(&softnodes[2]));
>  
> -	software_node_unregister(&softnodes[2]);
> -	software_node_unregister(&softnodes[1]);
> -	software_node_unregister(&softnodes[0]);
> +	software_node_unregister_nodes(softnodes);
>  }
>  
>  static void __init

-- 
Regards,

Laurent Pinchart

[Devel] Re: [PATCH v5 09/15] lib/test_printf.c: Use helper function to unwind array of software_nodes

[Andy Shevchenko]

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

On Saturday, January 9, 2021, Laurent Pinchart <
laurent.pinchart(a)ideasonboard.com> wrote:

> Hi Peter, Steven and Sergey,
>
> Could you please let us know if you're fine with this patch getting
> merged in v5.12 through the linux-media tree ? The cover letter contains
> additional details (in a nutshell, this is a cross-tree series and we
> would like to avoid topic branches if possible).


There is already a tag by Petr.


>
> On Thu, Jan 07, 2021 at 01:28:32PM +0000, Daniel Scally wrote:
> > Use the software_node_unregister_nodes() helper function to unwind this
> > array in a cleaner way.
> >
> > Acked-by: Petr Mladek <pmladek(a)suse.com>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko(a)linux.intel.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart(a)ideasonboard.com>
> > Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com>
> > Suggested-by: Andy Shevchenko <andriy.shevchenko(a)linux.intel.com>
> > Signed-off-by: Daniel Scally <djrscally(a)gmail.com>
> > ---
> > Changes in v5:
> >
> >       - None
> >
> >  lib/test_printf.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/lib/test_printf.c b/lib/test_printf.c
> > index 7ac87f18a10f..7d60f24240a4 100644
> > --- a/lib/test_printf.c
> > +++ b/lib/test_printf.c
> > @@ -644,9 +644,7 @@ static void __init fwnode_pointer(void)
> >       test(second_name, "%pfwP", software_node_fwnode(&softnodes[1]));
> >       test(third_name, "%pfwP", software_node_fwnode(&softnodes[2]));
> >
> > -     software_node_unregister(&softnodes[2]);
> > -     software_node_unregister(&softnodes[1]);
> > -     software_node_unregister(&softnodes[0]);
> > +     software_node_unregister_nodes(softnodes);
> >  }
> >
> >  static void __init
>
> --
> Regards,
>
> Laurent Pinchart
>


-- 
With Best Regards,
Andy Shevchenko

[-- Attachment #2: attachment.htm --]
[-- Type: text/html, Size: 2820 bytes --]

Re: [PATCH v5 09/15] lib/test_printf.c: Use helper function to unwind array of software_nodes

[Laurent Pinchart]

Hi Andy,

On Sat, Jan 09, 2021 at 11:07:33AM +0200, Andy Shevchenko wrote:
> On Saturday, January 9, 2021, Laurent Pinchart wrote:
> 
> > Hi Peter, Steven and Sergey,
> >
> > Could you please let us know if you're fine with this patch getting
> > merged in v5.12 through the linux-media tree ? The cover letter contains
> > additional details (in a nutshell, this is a cross-tree series and we
> > would like to avoid topic branches if possible).
> 
> There is already a tag by Petr.

I saw that, but looking at the corresponding e-mail, there was no clear
acknowledgement that we could merge this patch through a different tree.

> > On Thu, Jan 07, 2021 at 01:28:32PM +0000, Daniel Scally wrote:
> > > Use the software_node_unregister_nodes() helper function to unwind this
> > > array in a cleaner way.
> > >
> > > Acked-by: Petr Mladek <pmladek@suse.com>
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Signed-off-by: Daniel Scally <djrscally@gmail.com>
> > > ---
> > > Changes in v5:
> > >
> > >       - None
> > >
> > >  lib/test_printf.c | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > >
> > > diff --git a/lib/test_printf.c b/lib/test_printf.c
> > > index 7ac87f18a10f..7d60f24240a4 100644
> > > --- a/lib/test_printf.c
> > > +++ b/lib/test_printf.c
> > > @@ -644,9 +644,7 @@ static void __init fwnode_pointer(void)
> > >       test(second_name, "%pfwP", software_node_fwnode(&softnodes[1]));
> > >       test(third_name, "%pfwP", software_node_fwnode(&softnodes[2]));
> > >
> > > -     software_node_unregister(&softnodes[2]);
> > > -     software_node_unregister(&softnodes[1]);
> > > -     software_node_unregister(&softnodes[0]);
> > > +     software_node_unregister_nodes(softnodes);
> > >  }
> > >
> > >  static void __init

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 09/15] lib/test_printf.c: Use helper function to unwind array of software_nodes

[Andy Shevchenko]

On Sun, Jan 10, 2021 at 1:16 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Sat, Jan 09, 2021 at 11:07:33AM +0200, Andy Shevchenko wrote:
> > On Saturday, January 9, 2021, Laurent Pinchart wrote:
> > > Could you please let us know if you're fine with this patch getting
> > > merged in v5.12 through the linux-media tree ? The cover letter contains
> > > additional details (in a nutshell, this is a cross-tree series and we
> > > would like to avoid topic branches if possible).
> >
> > There is already a tag by Petr.
>
> I saw that, but looking at the corresponding e-mail, there was no clear
> acknowledgement that we could merge this patch through a different tree.

Fair point.

One question though, what so wrong with topic branches.

-- 
With Best Regards,
Andy Shevchenko

[Devel] Re: [PATCH v5 09/15] lib/test_printf.c: Use helper function to unwind array of software_nodes

[Andy Shevchenko]

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

On Sun, Jan 10, 2021 at 1:16 PM Laurent Pinchart
<laurent.pinchart(a)ideasonboard.com> wrote:
> On Sat, Jan 09, 2021 at 11:07:33AM +0200, Andy Shevchenko wrote:
> > On Saturday, January 9, 2021, Laurent Pinchart wrote:
> > > Could you please let us know if you're fine with this patch getting
> > > merged in v5.12 through the linux-media tree ? The cover letter contains
> > > additional details (in a nutshell, this is a cross-tree series and we
> > > would like to avoid topic branches if possible).
> >
> > There is already a tag by Petr.
>
> I saw that, but looking at the corresponding e-mail, there was no clear
> acknowledgement that we could merge this patch through a different tree.

Fair point.

One question though, what so wrong with topic branches.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 09/15] lib/test_printf.c: Use helper function to unwind array of software_nodes

[Laurent Pinchart]

On Sun, Jan 10, 2021 at 05:38:03PM +0200, Andy Shevchenko wrote:
> On Sun, Jan 10, 2021 at 1:16 PM Laurent Pinchart wrote:
> > On Sat, Jan 09, 2021 at 11:07:33AM +0200, Andy Shevchenko wrote:
> > > On Saturday, January 9, 2021, Laurent Pinchart wrote:
> > > > Could you please let us know if you're fine with this patch getting
> > > > merged in v5.12 through the linux-media tree ? The cover letter contains
> > > > additional details (in a nutshell, this is a cross-tree series and we
> > > > would like to avoid topic branches if possible).
> > >
> > > There is already a tag by Petr.
> >
> > I saw that, but looking at the corresponding e-mail, there was no clear
> > acknowledgement that we could merge this patch through a different tree.
> 
> Fair point.
> 
> One question though, what so wrong with topic branches.

They're not wrong, they just add more complexity as all maintainers
involved would need to create a topic branch based on v5.11-rc1 and
merge it in their respective tree. It's certainly doable, but when
there's no risk of conflict, merging the whole series through a single
tree is just easier.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 09/15] lib/test_printf.c: Use helper function to unwind array of software_nodes

[Sakari Ailus]

Hello all,

On Sat, Jan 09, 2021 at 04:01:05AM +0200, Laurent Pinchart wrote:
> Hi Peter, Steven and Sergey,
> 
> Could you please let us know if you're fine with this patch getting
> merged in v5.12 through the linux-media tree ? The cover letter contains
> additional details (in a nutshell, this is a cross-tree series and we
> would like to avoid topic branches if possible).

I'll proceed to merge this patch through the media tree.

Thanks.

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v5 09/15] lib/test_printf.c: Use helper function to unwind array of software_nodes

[Geert Uytterhoeven]

Hi Sakari, Mauro,

On Wed, Jan 13, 2021 at 2:26 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
> On Sat, Jan 09, 2021 at 04:01:05AM +0200, Laurent Pinchart wrote:
> > Could you please let us know if you're fine with this patch getting
> > merged in v5.12 through the linux-media tree ? The cover letter contains
> > additional details (in a nutshell, this is a cross-tree series and we
> > would like to avoid topic branches if possible).
>
> I'll proceed to merge this patch through the media tree.

Prefixing this patch with "media:"[1] when committing is confusing.

[1] commit f0328be57568c795 ("media: lib/test_printf.c: Use helper
function to unwind array of software_nodes").

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v5 10/15] ipu3-cio2: Add T: entry to MAINTAINERS

[Daniel Scally]

Development for the ipu3-cio2 driver is taking place in media_tree, but
there's no T: entry in MAINTAINERS to denote that - rectify that oversight

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v5:

	- None

 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 471561d9d55f..92228e8dd868 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9017,6 +9017,7 @@ M:	Bingbu Cao <bingbu.cao@intel.com>
 R:	Tianshu Qiu <tian.shu.qiu@intel.com>
 L:	linux-media@vger.kernel.org
 S:	Maintained
+T:	git git://linuxtv.org/media_tree.git
 F:	Documentation/userspace-api/media/v4l/pixfmt-srggb10-ipu3.rst
 F:	drivers/media/pci/intel/ipu3/
 
-- 
2.25.1

[PATCH v5 11/15] ipu3-cio2: Rename ipu3-cio2.c

[Daniel Scally]

ipu3-cio2 driver needs extending with multiple files; rename the main
source file and specify the renamed file in Makefile to accommodate that.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v5:

	- None

 drivers/media/pci/intel/ipu3/Makefile                          | 2 ++
 drivers/media/pci/intel/ipu3/{ipu3-cio2.c => ipu3-cio2-main.c} | 0
 2 files changed, 2 insertions(+)
 rename drivers/media/pci/intel/ipu3/{ipu3-cio2.c => ipu3-cio2-main.c} (100%)

diff --git a/drivers/media/pci/intel/ipu3/Makefile b/drivers/media/pci/intel/ipu3/Makefile
index 98ddd5beafe0..429d516452e4 100644
--- a/drivers/media/pci/intel/ipu3/Makefile
+++ b/drivers/media/pci/intel/ipu3/Makefile
@@ -1,2 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
+
+ipu3-cio2-y += ipu3-cio2-main.o
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
similarity index 100%
rename from drivers/media/pci/intel/ipu3/ipu3-cio2.c
rename to drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
-- 
2.25.1

[PATCH v5 12/15] media: v4l2-core: v4l2-async: Check sd->fwnode->secondary in match_fwnode()

[Daniel Scally]

Where the fwnode graph is comprised of software_nodes, these will be
assigned as the secondary to dev->fwnode. Check the v4l2_subdev's fwnode
for a secondary and attempt to match against it during match_fwnode() to
accommodate that possibility.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v5:

	- None

 drivers/media/v4l2-core/v4l2-async.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-async.c b/drivers/media/v4l2-core/v4l2-async.c
index e3ab003a6c85..9dd896d085ec 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -87,6 +87,14 @@ static bool match_fwnode(struct v4l2_async_notifier *notifier,
 	if (sd->fwnode == asd->match.fwnode)
 		return true;
 
+	/*
+	 * Check the same situation for any possible secondary assigned to the
+	 * subdev's fwnode
+	 */
+	if (!IS_ERR_OR_NULL(sd->fwnode->secondary) &&
+	    sd->fwnode->secondary == asd->match.fwnode)
+		return true;
+
 	/*
 	 * Otherwise, check if the sd fwnode and the asd fwnode refer to an
 	 * endpoint or a device. If they're of the same type, there's no match.
-- 
2.25.1

[PATCH v5 13/15] ACPI / bus: Add acpi_dev_get_next_match_dev() and helper macro

[Daniel Scally]

To ensure we handle situations in which multiple sensors of the same
model (and therefore _HID) are present in a system, we need to be able
to iterate over devices matching a known _HID but unknown _UID and _HRV
 - add acpi_dev_get_next_match_dev() to accommodate that possibility and
change acpi_dev_get_first_match_dev() to simply call the new function
with a NULL starting point. Add an iterator macro for convenience.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v5:

	- Changed commit subject

 drivers/acpi/utils.c    | 30 ++++++++++++++++++++++++++----
 include/acpi/acpi_bus.h |  7 +++++++
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index d5411a166685..ddca1550cce6 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -843,12 +843,13 @@ bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
 EXPORT_SYMBOL(acpi_dev_present);
 
 /**
- * acpi_dev_get_first_match_dev - Return the first match of ACPI device
+ * acpi_dev_get_next_match_dev - Return the next match of ACPI device
+ * @adev: Pointer to the previous acpi_device matching this @hid, @uid and @hrv
  * @hid: Hardware ID of the device.
  * @uid: Unique ID of the device, pass NULL to not check _UID
  * @hrv: Hardware Revision of the device, pass -1 to not check _HRV
  *
- * Return the first match of ACPI device if a matching device was present
+ * Return the next match of ACPI device if another matching device was present
  * at the moment of invocation, or NULL otherwise.
  *
  * The caller is responsible to call put_device() on the returned device.
@@ -856,8 +857,9 @@ EXPORT_SYMBOL(acpi_dev_present);
  * See additional information in acpi_dev_present() as well.
  */
 struct acpi_device *
-acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
+acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv)
 {
+	struct device *start = adev ? &adev->dev : NULL;
 	struct acpi_dev_match_info match = {};
 	struct device *dev;
 
@@ -865,9 +867,29 @@ acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
 	match.uid = uid;
 	match.hrv = hrv;
 
-	dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb);
+	dev = bus_find_device(&acpi_bus_type, start, &match, acpi_dev_match_cb);
 	return dev ? to_acpi_device(dev) : NULL;
 }
+EXPORT_SYMBOL(acpi_dev_get_next_match_dev);
+
+/**
+ * acpi_dev_get_first_match_dev - Return the first match of ACPI device
+ * @hid: Hardware ID of the device.
+ * @uid: Unique ID of the device, pass NULL to not check _UID
+ * @hrv: Hardware Revision of the device, pass -1 to not check _HRV
+ *
+ * Return the first match of ACPI device if a matching device was present
+ * at the moment of invocation, or NULL otherwise.
+ *
+ * The caller is responsible to call put_device() on the returned device.
+ *
+ * See additional information in acpi_dev_present() as well.
+ */
+struct acpi_device *
+acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
+{
+	return acpi_dev_get_next_match_dev(NULL, hid, uid, hrv);
+}
 EXPORT_SYMBOL(acpi_dev_get_first_match_dev);
 
 /*
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 6d1879bf9440..02a716a0af5d 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -683,9 +683,16 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
 
 bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2);
 
+struct acpi_device *
+acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv);
 struct acpi_device *
 acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv);
 
+#define for_each_acpi_dev_match(adev, hid, uid, hrv)			\
+	for (adev = acpi_dev_get_first_match_dev(hid, uid, hrv);	\
+	     adev;							\
+	     adev = acpi_dev_get_next_match_dev(adev, hid, uid, hrv))
+
 static inline void acpi_dev_put(struct acpi_device *adev)
 {
 	put_device(&adev->dev);
-- 
2.25.1

Re: [PATCH v5 13/15] ACPI / bus: Add acpi_dev_get_next_match_dev() and helper macro

[Laurent Pinchart]

Hi Rafael,

Could you please review this patch, and let us know (see question in the
cover letter) if it can be merged through the linux-media tree for v5.12
?

On Thu, Jan 07, 2021 at 01:28:36PM +0000, Daniel Scally wrote:
> To ensure we handle situations in which multiple sensors of the same
> model (and therefore _HID) are present in a system, we need to be able
> to iterate over devices matching a known _HID but unknown _UID and _HRV
>  - add acpi_dev_get_next_match_dev() to accommodate that possibility and
> change acpi_dev_get_first_match_dev() to simply call the new function
> with a NULL starting point. Add an iterator macro for convenience.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---
> Changes in v5:
> 
> 	- Changed commit subject
> 
>  drivers/acpi/utils.c    | 30 ++++++++++++++++++++++++++----
>  include/acpi/acpi_bus.h |  7 +++++++
>  2 files changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
> index d5411a166685..ddca1550cce6 100644
> --- a/drivers/acpi/utils.c
> +++ b/drivers/acpi/utils.c
> @@ -843,12 +843,13 @@ bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
>  EXPORT_SYMBOL(acpi_dev_present);
>  
>  /**
> - * acpi_dev_get_first_match_dev - Return the first match of ACPI device
> + * acpi_dev_get_next_match_dev - Return the next match of ACPI device
> + * @adev: Pointer to the previous acpi_device matching this @hid, @uid and @hrv
>   * @hid: Hardware ID of the device.
>   * @uid: Unique ID of the device, pass NULL to not check _UID
>   * @hrv: Hardware Revision of the device, pass -1 to not check _HRV
>   *
> - * Return the first match of ACPI device if a matching device was present
> + * Return the next match of ACPI device if another matching device was present
>   * at the moment of invocation, or NULL otherwise.
>   *
>   * The caller is responsible to call put_device() on the returned device.
> @@ -856,8 +857,9 @@ EXPORT_SYMBOL(acpi_dev_present);
>   * See additional information in acpi_dev_present() as well.
>   */
>  struct acpi_device *
> -acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
> +acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv)
>  {
> +	struct device *start = adev ? &adev->dev : NULL;
>  	struct acpi_dev_match_info match = {};
>  	struct device *dev;
>  
> @@ -865,9 +867,29 @@ acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
>  	match.uid = uid;
>  	match.hrv = hrv;
>  
> -	dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb);
> +	dev = bus_find_device(&acpi_bus_type, start, &match, acpi_dev_match_cb);
>  	return dev ? to_acpi_device(dev) : NULL;
>  }
> +EXPORT_SYMBOL(acpi_dev_get_next_match_dev);
> +
> +/**
> + * acpi_dev_get_first_match_dev - Return the first match of ACPI device
> + * @hid: Hardware ID of the device.
> + * @uid: Unique ID of the device, pass NULL to not check _UID
> + * @hrv: Hardware Revision of the device, pass -1 to not check _HRV
> + *
> + * Return the first match of ACPI device if a matching device was present
> + * at the moment of invocation, or NULL otherwise.
> + *
> + * The caller is responsible to call put_device() on the returned device.
> + *
> + * See additional information in acpi_dev_present() as well.
> + */
> +struct acpi_device *
> +acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
> +{
> +	return acpi_dev_get_next_match_dev(NULL, hid, uid, hrv);
> +}
>  EXPORT_SYMBOL(acpi_dev_get_first_match_dev);
>  
>  /*
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 6d1879bf9440..02a716a0af5d 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -683,9 +683,16 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
>  
>  bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2);
>  
> +struct acpi_device *
> +acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv);
>  struct acpi_device *
>  acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv);
>  
> +#define for_each_acpi_dev_match(adev, hid, uid, hrv)			\
> +	for (adev = acpi_dev_get_first_match_dev(hid, uid, hrv);	\
> +	     adev;							\
> +	     adev = acpi_dev_get_next_match_dev(adev, hid, uid, hrv))
> +
>  static inline void acpi_dev_put(struct acpi_device *adev)
>  {
>  	put_device(&adev->dev);

-- 
Regards,

Laurent Pinchart

[PATCH v5 14/15] media: v4l2-fwnode: Include v4l2_fwnode_bus_type

[Daniel Scally]

V4L2 fwnode bus types are enumerated in v4l2-fwnode.c, meaning they aren't
available to the rest of the kernel. Move the enum to the corresponding
header so that I can use the label to refer to those values.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
Changes in v5:

	- Changed commit subject

 drivers/media/v4l2-core/v4l2-fwnode.c | 11 -----------
 include/media/v4l2-fwnode.h           | 22 ++++++++++++++++++++++
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 5353e37eb950..c1c2b3060532 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -28,17 +28,6 @@
 #include <media/v4l2-fwnode.h>
 #include <media/v4l2-subdev.h>
 
-enum v4l2_fwnode_bus_type {
-	V4L2_FWNODE_BUS_TYPE_GUESS = 0,
-	V4L2_FWNODE_BUS_TYPE_CSI2_CPHY,
-	V4L2_FWNODE_BUS_TYPE_CSI1,
-	V4L2_FWNODE_BUS_TYPE_CCP2,
-	V4L2_FWNODE_BUS_TYPE_CSI2_DPHY,
-	V4L2_FWNODE_BUS_TYPE_PARALLEL,
-	V4L2_FWNODE_BUS_TYPE_BT656,
-	NR_OF_V4L2_FWNODE_BUS_TYPE,
-};
-
 static const struct v4l2_fwnode_bus_conv {
 	enum v4l2_fwnode_bus_type fwnode_bus_type;
 	enum v4l2_mbus_type mbus_type;
diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index 6d026dadbf98..e7ad95365a40 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -213,6 +213,28 @@ struct v4l2_fwnode_connector {
 	} connector;
 };
 
+/**
+ * enum v4l2_fwnode_bus_type - Video bus types defined by firmware properties
+ * @V4L2_FWNODE_BUS_TYPE_GUESS: Default value if no bus-type fwnode property
+ * @V4L2_FWNODE_BUS_TYPE_CSI2_CPHY: MIPI CSI-2 bus, C-PHY physical layer
+ * @V4L2_FWNODE_BUS_TYPE_CSI1: MIPI CSI-1 bus
+ * @V4L2_FWNODE_BUS_TYPE_CCP2: SMIA Compact Camera Port 2 bus
+ * @V4L2_FWNODE_BUS_TYPE_CSI2_DPHY: MIPI CSI-2 bus, D-PHY physical layer
+ * @V4L2_FWNODE_BUS_TYPE_PARALLEL: Camera Parallel Interface bus
+ * @V4L2_FWNODE_BUS_TYPE_BT656: BT.656 video format bus-type
+ * @NR_OF_V4L2_FWNODE_BUS_TYPE: Number of bus-types
+ */
+enum v4l2_fwnode_bus_type {
+	V4L2_FWNODE_BUS_TYPE_GUESS = 0,
+	V4L2_FWNODE_BUS_TYPE_CSI2_CPHY,
+	V4L2_FWNODE_BUS_TYPE_CSI1,
+	V4L2_FWNODE_BUS_TYPE_CCP2,
+	V4L2_FWNODE_BUS_TYPE_CSI2_DPHY,
+	V4L2_FWNODE_BUS_TYPE_PARALLEL,
+	V4L2_FWNODE_BUS_TYPE_BT656,
+	NR_OF_V4L2_FWNODE_BUS_TYPE
+};
+
 /**
  * v4l2_fwnode_endpoint_parse() - parse all fwnode node properties
  * @fwnode: pointer to the endpoint's fwnode handle
-- 
2.25.1

[PATCH v5 15/15] ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver

[Daniel Scally]

Currently on platforms designed for Windows, connections between CIO2 and
sensors are not properly defined in DSDT. This patch extends the ipu3-cio2
driver to compensate by building software_node connections, parsing the
connection properties from the sensor's SSDB buffer.

Suggested-by: Jordan Hand <jorhand@linux.microsoft.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
 MAINTAINERS                                   |   1 +
 drivers/media/pci/intel/ipu3/Kconfig          |  18 +
 drivers/media/pci/intel/ipu3/Makefile         |   1 +
 drivers/media/pci/intel/ipu3/cio2-bridge.c    | 311 ++++++++++++++++++
 drivers/media/pci/intel/ipu3/cio2-bridge.h    | 125 +++++++
 drivers/media/pci/intel/ipu3/ipu3-cio2-main.c |  34 ++
 drivers/media/pci/intel/ipu3/ipu3-cio2.h      |   6 +
 7 files changed, 496 insertions(+)
 create mode 100644 drivers/media/pci/intel/ipu3/cio2-bridge.c
 create mode 100644 drivers/media/pci/intel/ipu3/cio2-bridge.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 92228e8dd868..a091b496fdd8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9014,6 +9014,7 @@ INTEL IPU3 CSI-2 CIO2 DRIVER
 M:	Yong Zhi <yong.zhi@intel.com>
 M:	Sakari Ailus <sakari.ailus@linux.intel.com>
 M:	Bingbu Cao <bingbu.cao@intel.com>
+M:	Dan Scally <djrscally@gmail.com>
 R:	Tianshu Qiu <tian.shu.qiu@intel.com>
 L:	linux-media@vger.kernel.org
 S:	Maintained
diff --git a/drivers/media/pci/intel/ipu3/Kconfig b/drivers/media/pci/intel/ipu3/Kconfig
index 82d7f17e6a02..96a2231b16ad 100644
--- a/drivers/media/pci/intel/ipu3/Kconfig
+++ b/drivers/media/pci/intel/ipu3/Kconfig
@@ -16,3 +16,21 @@ config VIDEO_IPU3_CIO2
 	  Say Y or M here if you have a Skylake/Kaby Lake SoC with MIPI CSI-2
 	  connected camera.
 	  The module will be called ipu3-cio2.
+
+config CIO2_BRIDGE
+	bool "IPU3 CIO2 Sensors Bridge"
+	depends on VIDEO_IPU3_CIO2
+	help
+	  This extension provides an API for the ipu3-cio2 driver to create
+	  connections to cameras that are hidden in the SSDB buffer in ACPI.
+	  It can be used to enable support for cameras in detachable / hybrid
+	  devices that ship with Windows.
+
+	  Say Y here if your device is a detachable / hybrid laptop that comes
+	  with Windows installed by the OEM, for example:
+
+		- Microsoft Surface models (except Surface Pro 3)
+		- The Lenovo Miix line (for example the 510, 520, 710 and 720)
+		- Dell 7285
+
+	  If in doubt, say N here.
diff --git a/drivers/media/pci/intel/ipu3/Makefile b/drivers/media/pci/intel/ipu3/Makefile
index 429d516452e4..933777e6ea8a 100644
--- a/drivers/media/pci/intel/ipu3/Makefile
+++ b/drivers/media/pci/intel/ipu3/Makefile
@@ -2,3 +2,4 @@
 obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
 
 ipu3-cio2-y += ipu3-cio2-main.o
+ipu3-cio2-$(CONFIG_CIO2_BRIDGE) += cio2-bridge.o
diff --git a/drivers/media/pci/intel/ipu3/cio2-bridge.c b/drivers/media/pci/intel/ipu3/cio2-bridge.c
new file mode 100644
index 000000000000..143f3c0f445e
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/cio2-bridge.c
@@ -0,0 +1,311 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Author: Dan Scally <djrscally@gmail.com> */
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/pci.h>
+#include <linux/property.h>
+#include <media/v4l2-fwnode.h>
+
+#include "cio2-bridge.h"
+
+/*
+ * Extend this array with ACPI Hardware IDs of devices known to be working
+ * plus the number of link-frequencies expected by their drivers, along with
+ * the frequency values in hertz. This is somewhat opportunistic way of adding
+ * support for this for now in the hopes of a better source for the information
+ * (possibly some encoded value in the SSDB buffer that we're unaware of)
+ * becoming apparent in the future.
+ *
+ * Do not add an entry for a sensor that is not actually supported.
+ */
+static const struct cio2_sensor_config cio2_supported_sensors[] = {
+	/* Omnivision OV5693 */
+	CIO2_SENSOR_CONFIG("INT33BE", 0),
+	/* Omnivision OV2680 */
+	CIO2_SENSOR_CONFIG("OVTI2680", 0),
+};
+
+static const struct cio2_property_names prop_names = {
+	.clock_frequency = "clock-frequency",
+	.rotation = "rotation",
+	.bus_type = "bus-type",
+	.data_lanes = "data-lanes",
+	.remote_endpoint = "remote-endpoint",
+	.link_frequencies = "link-frequencies",
+};
+
+static int cio2_bridge_read_acpi_buffer(struct acpi_device *adev, char *id,
+					void *data, u32 size)
+{
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *obj;
+	acpi_status status;
+	int ret = 0;
+
+	status = acpi_evaluate_object(adev->handle, id, NULL, &buffer);
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	obj = buffer.pointer;
+	if (!obj) {
+		dev_err(&adev->dev, "Couldn't locate ACPI buffer\n");
+		return -ENODEV;
+	}
+
+	if (obj->type != ACPI_TYPE_BUFFER) {
+		dev_err(&adev->dev, "Not an ACPI buffer\n");
+		ret = -ENODEV;
+		goto out_free_buff;
+	}
+
+	if (obj->buffer.length > size) {
+		dev_err(&adev->dev, "Given buffer is too small\n");
+		ret = -EINVAL;
+		goto out_free_buff;
+	}
+
+	memcpy(data, obj->buffer.pointer, obj->buffer.length);
+
+out_free_buff:
+	kfree(buffer.pointer);
+	return ret;
+}
+
+static void cio2_bridge_create_fwnode_properties(
+	struct cio2_sensor *sensor,
+	struct cio2_bridge *bridge,
+	const struct cio2_sensor_config *cfg)
+{
+	sensor->prop_names = prop_names;
+
+	sensor->local_ref[0].node = &sensor->swnodes[SWNODE_CIO2_ENDPOINT];
+	sensor->remote_ref[0].node = &sensor->swnodes[SWNODE_SENSOR_ENDPOINT];
+
+	sensor->dev_properties[0] = PROPERTY_ENTRY_U32(
+					sensor->prop_names.clock_frequency,
+					sensor->ssdb.mclkspeed);
+	sensor->dev_properties[1] = PROPERTY_ENTRY_U8(
+					sensor->prop_names.rotation,
+					sensor->ssdb.degree);
+
+	sensor->ep_properties[0] = PROPERTY_ENTRY_U32(
+					sensor->prop_names.bus_type,
+					V4L2_FWNODE_BUS_TYPE_CSI2_DPHY);
+	sensor->ep_properties[1] = PROPERTY_ENTRY_U32_ARRAY_LEN(
+					sensor->prop_names.data_lanes,
+					bridge->data_lanes,
+					sensor->ssdb.lanes);
+	sensor->ep_properties[2] = PROPERTY_ENTRY_REF_ARRAY(
+					sensor->prop_names.remote_endpoint,
+					sensor->local_ref);
+
+	if (cfg->nr_link_freqs > 0)
+		sensor->ep_properties[3] = PROPERTY_ENTRY_U64_ARRAY_LEN(
+						sensor->prop_names.link_frequencies,
+						cfg->link_freqs,
+						cfg->nr_link_freqs);
+
+	sensor->cio2_properties[0] = PROPERTY_ENTRY_U32_ARRAY_LEN(
+					sensor->prop_names.data_lanes,
+					bridge->data_lanes,
+					sensor->ssdb.lanes);
+	sensor->cio2_properties[1] = PROPERTY_ENTRY_REF_ARRAY(
+					sensor->prop_names.remote_endpoint,
+					sensor->remote_ref);
+}
+
+static void cio2_bridge_init_swnode_names(struct cio2_sensor *sensor)
+{
+	snprintf(sensor->node_names.remote_port,
+		 sizeof(sensor->node_names.remote_port),
+		 SWNODE_GRAPH_PORT_NAME_FMT, sensor->ssdb.link);
+	snprintf(sensor->node_names.port,
+		 sizeof(sensor->node_names.port),
+		 SWNODE_GRAPH_PORT_NAME_FMT, 0); /* Always port 0 */
+	snprintf(sensor->node_names.endpoint,
+		 sizeof(sensor->node_names.endpoint),
+		 SWNODE_GRAPH_ENDPOINT_NAME_FMT, 0); /* And endpoint 0 */
+}
+
+static void cio2_bridge_create_connection_swnodes(struct cio2_bridge *bridge,
+						  struct cio2_sensor *sensor)
+{
+	struct software_node *nodes = sensor->swnodes;
+
+	cio2_bridge_init_swnode_names(sensor);
+
+	nodes[SWNODE_SENSOR_HID] = NODE_SENSOR(sensor->name,
+					       sensor->dev_properties);
+	nodes[SWNODE_SENSOR_PORT] = NODE_PORT(sensor->node_names.port,
+					      &nodes[SWNODE_SENSOR_HID]);
+	nodes[SWNODE_SENSOR_ENDPOINT] = NODE_ENDPOINT(
+						sensor->node_names.endpoint,
+						&nodes[SWNODE_SENSOR_PORT],
+						sensor->ep_properties);
+	nodes[SWNODE_CIO2_PORT] = NODE_PORT(sensor->node_names.remote_port,
+					    &bridge->cio2_hid_node);
+	nodes[SWNODE_CIO2_ENDPOINT] = NODE_ENDPOINT(
+						sensor->node_names.endpoint,
+						&nodes[SWNODE_CIO2_PORT],
+						sensor->cio2_properties);
+}
+
+static void cio2_bridge_unregister_sensors(struct cio2_bridge *bridge)
+{
+	struct cio2_sensor *sensor;
+	unsigned int i;
+
+	for (i = 0; i < bridge->n_sensors; i++) {
+		sensor = &bridge->sensors[i];
+		software_node_unregister_nodes(sensor->swnodes);
+		acpi_dev_put(sensor->adev);
+	}
+}
+
+static int cio2_bridge_connect_sensor(const struct cio2_sensor_config *cfg,
+				      struct cio2_bridge *bridge,
+				      struct pci_dev *cio2)
+{
+	struct fwnode_handle *fwnode;
+	struct cio2_sensor *sensor;
+	struct acpi_device *adev;
+	int ret;
+
+	for_each_acpi_dev_match(adev, cfg->hid, NULL, -1) {
+		if (!adev->status.enabled)
+			continue;
+
+		if (bridge->n_sensors >= CIO2_NUM_PORTS) {
+			dev_err(&cio2->dev, "Exceeded available CIO2 ports\n");
+			cio2_bridge_unregister_sensors(bridge);
+			ret = -EINVAL;
+			goto err_out;
+		}
+
+		sensor = &bridge->sensors[bridge->n_sensors];
+		sensor->adev = adev;
+		strscpy(sensor->name, cfg->hid, sizeof(sensor->name));
+
+		ret = cio2_bridge_read_acpi_buffer(adev, "SSDB",
+						   &sensor->ssdb,
+						   sizeof(sensor->ssdb));
+		if (ret)
+			goto err_put_adev;
+
+		if (sensor->ssdb.lanes > CIO2_MAX_LANES) {
+			dev_err(&adev->dev,
+				"Number of lanes in SSDB is invalid\n");
+			ret = -EINVAL;
+			goto err_put_adev;
+		}
+
+		cio2_bridge_create_fwnode_properties(sensor, bridge, cfg);
+		cio2_bridge_create_connection_swnodes(bridge, sensor);
+
+		ret = software_node_register_nodes(sensor->swnodes);
+		if (ret)
+			goto err_put_adev;
+
+		fwnode = software_node_fwnode(&sensor->swnodes[SWNODE_SENSOR_HID]);
+		if (!fwnode) {
+			ret = -ENODEV;
+			goto err_free_swnodes;
+		}
+
+		adev->fwnode.secondary = fwnode;
+
+		dev_info(&cio2->dev, "Found supported sensor %s\n",
+			 acpi_dev_name(adev));
+
+		bridge->n_sensors++;
+	}
+
+	return 0;
+
+err_free_swnodes:
+	software_node_unregister_nodes(sensor->swnodes);
+err_put_adev:
+	acpi_dev_put(sensor->adev);
+err_out:
+	return ret;
+}
+
+static int cio2_bridge_connect_sensors(struct cio2_bridge *bridge,
+				       struct pci_dev *cio2)
+{
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < ARRAY_SIZE(cio2_supported_sensors); i++) {
+		const struct cio2_sensor_config *cfg = &cio2_supported_sensors[i];
+
+		ret = cio2_bridge_connect_sensor(cfg, bridge, cio2);
+		if (ret)
+			goto err_unregister_sensors;
+	}
+
+	return 0;
+
+err_unregister_sensors:
+	cio2_bridge_unregister_sensors(bridge);
+	return ret;
+}
+
+int cio2_bridge_init(struct pci_dev *cio2)
+{
+	struct device *dev = &cio2->dev;
+	struct fwnode_handle *fwnode;
+	struct cio2_bridge *bridge;
+	unsigned int i;
+	int ret;
+
+	bridge = kzalloc(sizeof(*bridge), GFP_KERNEL);
+	if (!bridge)
+		return -ENOMEM;
+
+	strscpy(bridge->cio2_node_name, CIO2_HID, sizeof(bridge->cio2_node_name));
+	bridge->cio2_hid_node.name = bridge->cio2_node_name;
+
+	ret = software_node_register(&bridge->cio2_hid_node);
+	if (ret < 0) {
+		dev_err(dev, "Failed to register the CIO2 HID node\n");
+		goto err_free_bridge;
+	}
+
+	/*
+	 * Map the lane arrangement, which is fixed for the IPU3 (meaning we
+	 * only need one, rather than one per sensor). We include it as a
+	 * member of the struct cio2_bridge rather than a global variable so
+	 * that it survives if the module is unloaded along with the rest of
+	 * the struct.
+	 */
+	for (i = 0; i < CIO2_MAX_LANES; i++)
+		bridge->data_lanes[i] = i + 1;
+
+	ret = cio2_bridge_connect_sensors(bridge, cio2);
+	if (ret || bridge->n_sensors == 0)
+		goto err_unregister_cio2;
+
+	dev_info(dev, "Connected %d cameras\n", bridge->n_sensors);
+
+	fwnode = software_node_fwnode(&bridge->cio2_hid_node);
+	if (!fwnode) {
+		dev_err(dev, "Error getting fwnode from cio2 software_node\n");
+		ret = -ENODEV;
+		goto err_unregister_sensors;
+	}
+
+	set_secondary_fwnode(dev, fwnode);
+
+	return 0;
+
+err_unregister_sensors:
+	cio2_bridge_unregister_sensors(bridge);
+err_unregister_cio2:
+	software_node_unregister(&bridge->cio2_hid_node);
+err_free_bridge:
+	kfree(bridge);
+
+	return ret;
+}
diff --git a/drivers/media/pci/intel/ipu3/cio2-bridge.h b/drivers/media/pci/intel/ipu3/cio2-bridge.h
new file mode 100644
index 000000000000..dd0ffcafa489
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/cio2-bridge.h
@@ -0,0 +1,125 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Author: Dan Scally <djrscally@gmail.com> */
+#ifndef __CIO2_BRIDGE_H
+#define __CIO2_BRIDGE_H
+
+#include <linux/property.h>
+#include <linux/types.h>
+
+#include "ipu3-cio2.h"
+
+#define CIO2_HID				"INT343E"
+#define CIO2_MAX_LANES				4
+#define MAX_NUM_LINK_FREQS			3
+
+#define CIO2_SENSOR_CONFIG(_HID, _NR, ...)	\
+	(const struct cio2_sensor_config) {	\
+		.hid = _HID,			\
+		.nr_link_freqs = _NR,		\
+		.link_freqs = { __VA_ARGS__ }	\
+	}
+
+#define NODE_SENSOR(_HID, _PROPS)		\
+	(const struct software_node) {		\
+		.name = _HID,			\
+		.properties = _PROPS,		\
+	}
+
+#define NODE_PORT(_PORT, _SENSOR_NODE)		\
+	(const struct software_node) {		\
+		.name = _PORT,			\
+		.parent = _SENSOR_NODE,		\
+	}
+
+#define NODE_ENDPOINT(_EP, _PORT, _PROPS)	\
+	(const struct software_node) {		\
+		.name = _EP,			\
+		.parent = _PORT,		\
+		.properties = _PROPS,		\
+	}
+
+enum cio2_sensor_swnodes {
+	SWNODE_SENSOR_HID,
+	SWNODE_SENSOR_PORT,
+	SWNODE_SENSOR_ENDPOINT,
+	SWNODE_CIO2_PORT,
+	SWNODE_CIO2_ENDPOINT,
+	SWNODE_COUNT
+};
+
+/* Data representation as it is in ACPI SSDB buffer */
+struct cio2_sensor_ssdb {
+	u8 version;
+	u8 sku;
+	u8 guid_csi2[16];
+	u8 devfunction;
+	u8 bus;
+	u32 dphylinkenfuses;
+	u32 clockdiv;
+	u8 link;
+	u8 lanes;
+	u32 csiparams[10];
+	u32 maxlanespeed;
+	u8 sensorcalibfileidx;
+	u8 sensorcalibfileidxInMBZ[3];
+	u8 romtype;
+	u8 vcmtype;
+	u8 platforminfo;
+	u8 platformsubinfo;
+	u8 flash;
+	u8 privacyled;
+	u8 degree;
+	u8 mipilinkdefined;
+	u32 mclkspeed;
+	u8 controllogicid;
+	u8 reserved1[3];
+	u8 mclkport;
+	u8 reserved2[13];
+} __packed;
+
+struct cio2_property_names {
+	char clock_frequency[16];
+	char rotation[9];
+	char bus_type[9];
+	char data_lanes[11];
+	char remote_endpoint[16];
+	char link_frequencies[17];
+};
+
+struct cio2_node_names {
+	char port[7];
+	char endpoint[11];
+	char remote_port[7];
+};
+
+struct cio2_sensor_config {
+	const char *hid;
+	const u8 nr_link_freqs;
+	const u64 link_freqs[MAX_NUM_LINK_FREQS];
+};
+
+struct cio2_sensor {
+	char name[ACPI_ID_LEN];
+	struct acpi_device *adev;
+
+	struct software_node swnodes[6];
+	struct cio2_node_names node_names;
+
+	struct cio2_sensor_ssdb ssdb;
+	struct cio2_property_names prop_names;
+	struct property_entry ep_properties[5];
+	struct property_entry dev_properties[3];
+	struct property_entry cio2_properties[3];
+	struct software_node_ref_args local_ref[1];
+	struct software_node_ref_args remote_ref[1];
+};
+
+struct cio2_bridge {
+	char cio2_node_name[ACPI_ID_LEN];
+	struct software_node cio2_hid_node;
+	u32 data_lanes[4];
+	unsigned int n_sensors;
+	struct cio2_sensor sensors[CIO2_NUM_PORTS];
+};
+
+#endif
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
index 36e354ecf71e..db59b19a6236 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
@@ -1702,11 +1702,28 @@ static void cio2_queues_exit(struct cio2_device *cio2)
 		cio2_queue_exit(cio2, &cio2->queue[i]);
 }
 
+static int cio2_check_fwnode_graph(struct fwnode_handle *fwnode)
+{
+	struct fwnode_handle *endpoint;
+
+	if (IS_ERR_OR_NULL(fwnode))
+		return -EINVAL;
+
+	endpoint = fwnode_graph_get_next_endpoint(fwnode, NULL);
+	if (endpoint) {
+		fwnode_handle_put(endpoint);
+		return 0;
+	}
+
+	return cio2_check_fwnode_graph(fwnode->secondary);
+}
+
 /**************** PCI interface ****************/
 
 static int cio2_pci_probe(struct pci_dev *pci_dev,
 			  const struct pci_device_id *id)
 {
+	struct fwnode_handle *fwnode = dev_fwnode(&pci_dev->dev);
 	struct cio2_device *cio2;
 	int r;
 
@@ -1715,6 +1732,23 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
 		return -ENOMEM;
 	cio2->pci_dev = pci_dev;
 
+	/*
+	 * On some platforms no connections to sensors are defined in firmware,
+	 * if the device has no endpoints then we can try to build those as
+	 * software_nodes parsed from SSDB.
+	 */
+	r = cio2_check_fwnode_graph(fwnode);
+	if (r) {
+		if (fwnode && !IS_ERR_OR_NULL(fwnode->secondary)) {
+			dev_err(&pci_dev->dev, "fwnode graph has no endpoints connected\n");
+			return -EINVAL;
+		}
+
+		r = cio2_bridge_init(pci_dev);
+		if (r)
+			return r;
+	}
+
 	r = pcim_enable_device(pci_dev);
 	if (r) {
 		dev_err(&pci_dev->dev, "failed to enable device (%d)\n", r);
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.h b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
index 62187ab5ae43..dc3e343a37fb 100644
--- a/drivers/media/pci/intel/ipu3/ipu3-cio2.h
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
@@ -455,4 +455,10 @@ static inline struct cio2_queue *vb2q_to_cio2_queue(struct vb2_queue *vq)
 	return container_of(vq, struct cio2_queue, vbq);
 }
 
+#if IS_ENABLED(CONFIG_CIO2_BRIDGE)
+int cio2_bridge_init(struct pci_dev *cio2);
+#else
+int cio2_bridge_init(struct pci_dev *cio2) { return 0; }
+#endif
+
 #endif
-- 
2.25.1

Re: [PATCH v5 15/15] ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver

[Andy Shevchenko]

On Thu, Jan 07, 2021 at 01:28:38PM +0000, Daniel Scally wrote:
> Currently on platforms designed for Windows, connections between CIO2 and
> sensors are not properly defined in DSDT. This patch extends the ipu3-cio2
> driver to compensate by building software_node connections, parsing the
> connection properties from the sensor's SSDB buffer.
> 
> Suggested-by: Jordan Hand <jorhand@linux.microsoft.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---

Seems like missed changelog (no need to resend, just send a follow up).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 15/15] ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver

[Andy Shevchenko]

On Thu, Jan 07, 2021 at 04:05:05PM +0200, Andy Shevchenko wrote:
> On Thu, Jan 07, 2021 at 01:28:38PM +0000, Daniel Scally wrote:

> Seems like missed changelog (no need to resend, just send a follow up).

I meant the follow up with only changelog text, thanks!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 15/15] ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver

[Daniel Scally]

On 07/01/2021 13:28, Daniel Scally wrote:
> Currently on platforms designed for Windows, connections between CIO2 and
> sensors are not properly defined in DSDT. This patch extends the ipu3-cio2
> driver to compensate by building software_node connections, parsing the
> connection properties from the sensor's SSDB buffer.
> 
> Suggested-by: Jordan Hand <jorhand@linux.microsoft.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
> ---

Sorry all, missed the changelog on this one:

---
Changes in v5:

	- Changed some macros to declare compound literals
	- Switched cio2_check_fwnode_graph() to return int instead of bool
	- Added some comments to clarify parts of the code that might look
	  weird initially
	- Some cosmetic changes


>  MAINTAINERS                                   |   1 +
>  drivers/media/pci/intel/ipu3/Kconfig          |  18 +
>  drivers/media/pci/intel/ipu3/Makefile         |   1 +
>  drivers/media/pci/intel/ipu3/cio2-bridge.c    | 311 ++++++++++++++++++
>  drivers/media/pci/intel/ipu3/cio2-bridge.h    | 125 +++++++
>  drivers/media/pci/intel/ipu3/ipu3-cio2-main.c |  34 ++
>  drivers/media/pci/intel/ipu3/ipu3-cio2.h      |   6 +
>  7 files changed, 496 insertions(+)
>  create mode 100644 drivers/media/pci/intel/ipu3/cio2-bridge.c
>  create mode 100644 drivers/media/pci/intel/ipu3/cio2-bridge.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 92228e8dd868..a091b496fdd8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9014,6 +9014,7 @@ INTEL IPU3 CSI-2 CIO2 DRIVER
>  M:	Yong Zhi <yong.zhi@intel.com>
>  M:	Sakari Ailus <sakari.ailus@linux.intel.com>
>  M:	Bingbu Cao <bingbu.cao@intel.com>
> +M:	Dan Scally <djrscally@gmail.com>
>  R:	Tianshu Qiu <tian.shu.qiu@intel.com>
>  L:	linux-media@vger.kernel.org
>  S:	Maintained
> diff --git a/drivers/media/pci/intel/ipu3/Kconfig b/drivers/media/pci/intel/ipu3/Kconfig
> index 82d7f17e6a02..96a2231b16ad 100644
> --- a/drivers/media/pci/intel/ipu3/Kconfig
> +++ b/drivers/media/pci/intel/ipu3/Kconfig
> @@ -16,3 +16,21 @@ config VIDEO_IPU3_CIO2
>  	  Say Y or M here if you have a Skylake/Kaby Lake SoC with MIPI CSI-2
>  	  connected camera.
>  	  The module will be called ipu3-cio2.
> +
> +config CIO2_BRIDGE
> +	bool "IPU3 CIO2 Sensors Bridge"
> +	depends on VIDEO_IPU3_CIO2
> +	help
> +	  This extension provides an API for the ipu3-cio2 driver to create
> +	  connections to cameras that are hidden in the SSDB buffer in ACPI.
> +	  It can be used to enable support for cameras in detachable / hybrid
> +	  devices that ship with Windows.
> +
> +	  Say Y here if your device is a detachable / hybrid laptop that comes
> +	  with Windows installed by the OEM, for example:
> +
> +		- Microsoft Surface models (except Surface Pro 3)
> +		- The Lenovo Miix line (for example the 510, 520, 710 and 720)
> +		- Dell 7285
> +
> +	  If in doubt, say N here.
> diff --git a/drivers/media/pci/intel/ipu3/Makefile b/drivers/media/pci/intel/ipu3/Makefile
> index 429d516452e4..933777e6ea8a 100644
> --- a/drivers/media/pci/intel/ipu3/Makefile
> +++ b/drivers/media/pci/intel/ipu3/Makefile
> @@ -2,3 +2,4 @@
>  obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
>  
>  ipu3-cio2-y += ipu3-cio2-main.o
> +ipu3-cio2-$(CONFIG_CIO2_BRIDGE) += cio2-bridge.o
> diff --git a/drivers/media/pci/intel/ipu3/cio2-bridge.c b/drivers/media/pci/intel/ipu3/cio2-bridge.c
> new file mode 100644
> index 000000000000..143f3c0f445e
> --- /dev/null
> +++ b/drivers/media/pci/intel/ipu3/cio2-bridge.c
> @@ -0,0 +1,311 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Author: Dan Scally <djrscally@gmail.com> */
> +
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/pci.h>
> +#include <linux/property.h>
> +#include <media/v4l2-fwnode.h>
> +
> +#include "cio2-bridge.h"
> +
> +/*
> + * Extend this array with ACPI Hardware IDs of devices known to be working
> + * plus the number of link-frequencies expected by their drivers, along with
> + * the frequency values in hertz. This is somewhat opportunistic way of adding
> + * support for this for now in the hopes of a better source for the information
> + * (possibly some encoded value in the SSDB buffer that we're unaware of)
> + * becoming apparent in the future.
> + *
> + * Do not add an entry for a sensor that is not actually supported.
> + */
> +static const struct cio2_sensor_config cio2_supported_sensors[] = {
> +	/* Omnivision OV5693 */
> +	CIO2_SENSOR_CONFIG("INT33BE", 0),
> +	/* Omnivision OV2680 */
> +	CIO2_SENSOR_CONFIG("OVTI2680", 0),
> +};
> +
> +static const struct cio2_property_names prop_names = {
> +	.clock_frequency = "clock-frequency",
> +	.rotation = "rotation",
> +	.bus_type = "bus-type",
> +	.data_lanes = "data-lanes",
> +	.remote_endpoint = "remote-endpoint",
> +	.link_frequencies = "link-frequencies",
> +};
> +
> +static int cio2_bridge_read_acpi_buffer(struct acpi_device *adev, char *id,
> +					void *data, u32 size)
> +{
> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +	union acpi_object *obj;
> +	acpi_status status;
> +	int ret = 0;
> +
> +	status = acpi_evaluate_object(adev->handle, id, NULL, &buffer);
> +	if (ACPI_FAILURE(status))
> +		return -ENODEV;
> +
> +	obj = buffer.pointer;
> +	if (!obj) {
> +		dev_err(&adev->dev, "Couldn't locate ACPI buffer\n");
> +		return -ENODEV;
> +	}
> +
> +	if (obj->type != ACPI_TYPE_BUFFER) {
> +		dev_err(&adev->dev, "Not an ACPI buffer\n");
> +		ret = -ENODEV;
> +		goto out_free_buff;
> +	}
> +
> +	if (obj->buffer.length > size) {
> +		dev_err(&adev->dev, "Given buffer is too small\n");
> +		ret = -EINVAL;
> +		goto out_free_buff;
> +	}
> +
> +	memcpy(data, obj->buffer.pointer, obj->buffer.length);
> +
> +out_free_buff:
> +	kfree(buffer.pointer);
> +	return ret;
> +}
> +
> +static void cio2_bridge_create_fwnode_properties(
> +	struct cio2_sensor *sensor,
> +	struct cio2_bridge *bridge,
> +	const struct cio2_sensor_config *cfg)
> +{
> +	sensor->prop_names = prop_names;
> +
> +	sensor->local_ref[0].node = &sensor->swnodes[SWNODE_CIO2_ENDPOINT];
> +	sensor->remote_ref[0].node = &sensor->swnodes[SWNODE_SENSOR_ENDPOINT];
> +
> +	sensor->dev_properties[0] = PROPERTY_ENTRY_U32(
> +					sensor->prop_names.clock_frequency,
> +					sensor->ssdb.mclkspeed);
> +	sensor->dev_properties[1] = PROPERTY_ENTRY_U8(
> +					sensor->prop_names.rotation,
> +					sensor->ssdb.degree);
> +
> +	sensor->ep_properties[0] = PROPERTY_ENTRY_U32(
> +					sensor->prop_names.bus_type,
> +					V4L2_FWNODE_BUS_TYPE_CSI2_DPHY);
> +	sensor->ep_properties[1] = PROPERTY_ENTRY_U32_ARRAY_LEN(
> +					sensor->prop_names.data_lanes,
> +					bridge->data_lanes,
> +					sensor->ssdb.lanes);
> +	sensor->ep_properties[2] = PROPERTY_ENTRY_REF_ARRAY(
> +					sensor->prop_names.remote_endpoint,
> +					sensor->local_ref);
> +
> +	if (cfg->nr_link_freqs > 0)
> +		sensor->ep_properties[3] = PROPERTY_ENTRY_U64_ARRAY_LEN(
> +						sensor->prop_names.link_frequencies,
> +						cfg->link_freqs,
> +						cfg->nr_link_freqs);
> +
> +	sensor->cio2_properties[0] = PROPERTY_ENTRY_U32_ARRAY_LEN(
> +					sensor->prop_names.data_lanes,
> +					bridge->data_lanes,
> +					sensor->ssdb.lanes);
> +	sensor->cio2_properties[1] = PROPERTY_ENTRY_REF_ARRAY(
> +					sensor->prop_names.remote_endpoint,
> +					sensor->remote_ref);
> +}
> +
> +static void cio2_bridge_init_swnode_names(struct cio2_sensor *sensor)
> +{
> +	snprintf(sensor->node_names.remote_port,
> +		 sizeof(sensor->node_names.remote_port),
> +		 SWNODE_GRAPH_PORT_NAME_FMT, sensor->ssdb.link);
> +	snprintf(sensor->node_names.port,
> +		 sizeof(sensor->node_names.port),
> +		 SWNODE_GRAPH_PORT_NAME_FMT, 0); /* Always port 0 */
> +	snprintf(sensor->node_names.endpoint,
> +		 sizeof(sensor->node_names.endpoint),
> +		 SWNODE_GRAPH_ENDPOINT_NAME_FMT, 0); /* And endpoint 0 */
> +}
> +
> +static void cio2_bridge_create_connection_swnodes(struct cio2_bridge *bridge,
> +						  struct cio2_sensor *sensor)
> +{
> +	struct software_node *nodes = sensor->swnodes;
> +
> +	cio2_bridge_init_swnode_names(sensor);
> +
> +	nodes[SWNODE_SENSOR_HID] = NODE_SENSOR(sensor->name,
> +					       sensor->dev_properties);
> +	nodes[SWNODE_SENSOR_PORT] = NODE_PORT(sensor->node_names.port,
> +					      &nodes[SWNODE_SENSOR_HID]);
> +	nodes[SWNODE_SENSOR_ENDPOINT] = NODE_ENDPOINT(
> +						sensor->node_names.endpoint,
> +						&nodes[SWNODE_SENSOR_PORT],
> +						sensor->ep_properties);
> +	nodes[SWNODE_CIO2_PORT] = NODE_PORT(sensor->node_names.remote_port,
> +					    &bridge->cio2_hid_node);
> +	nodes[SWNODE_CIO2_ENDPOINT] = NODE_ENDPOINT(
> +						sensor->node_names.endpoint,
> +						&nodes[SWNODE_CIO2_PORT],
> +						sensor->cio2_properties);
> +}
> +
> +static void cio2_bridge_unregister_sensors(struct cio2_bridge *bridge)
> +{
> +	struct cio2_sensor *sensor;
> +	unsigned int i;
> +
> +	for (i = 0; i < bridge->n_sensors; i++) {
> +		sensor = &bridge->sensors[i];
> +		software_node_unregister_nodes(sensor->swnodes);
> +		acpi_dev_put(sensor->adev);
> +	}
> +}
> +
> +static int cio2_bridge_connect_sensor(const struct cio2_sensor_config *cfg,
> +				      struct cio2_bridge *bridge,
> +				      struct pci_dev *cio2)
> +{
> +	struct fwnode_handle *fwnode;
> +	struct cio2_sensor *sensor;
> +	struct acpi_device *adev;
> +	int ret;
> +
> +	for_each_acpi_dev_match(adev, cfg->hid, NULL, -1) {
> +		if (!adev->status.enabled)
> +			continue;
> +
> +		if (bridge->n_sensors >= CIO2_NUM_PORTS) {
> +			dev_err(&cio2->dev, "Exceeded available CIO2 ports\n");
> +			cio2_bridge_unregister_sensors(bridge);
> +			ret = -EINVAL;
> +			goto err_out;
> +		}
> +
> +		sensor = &bridge->sensors[bridge->n_sensors];
> +		sensor->adev = adev;
> +		strscpy(sensor->name, cfg->hid, sizeof(sensor->name));
> +
> +		ret = cio2_bridge_read_acpi_buffer(adev, "SSDB",
> +						   &sensor->ssdb,
> +						   sizeof(sensor->ssdb));
> +		if (ret)
> +			goto err_put_adev;
> +
> +		if (sensor->ssdb.lanes > CIO2_MAX_LANES) {
> +			dev_err(&adev->dev,
> +				"Number of lanes in SSDB is invalid\n");
> +			ret = -EINVAL;
> +			goto err_put_adev;
> +		}
> +
> +		cio2_bridge_create_fwnode_properties(sensor, bridge, cfg);
> +		cio2_bridge_create_connection_swnodes(bridge, sensor);
> +
> +		ret = software_node_register_nodes(sensor->swnodes);
> +		if (ret)
> +			goto err_put_adev;
> +
> +		fwnode = software_node_fwnode(&sensor->swnodes[SWNODE_SENSOR_HID]);
> +		if (!fwnode) {
> +			ret = -ENODEV;
> +			goto err_free_swnodes;
> +		}
> +
> +		adev->fwnode.secondary = fwnode;
> +
> +		dev_info(&cio2->dev, "Found supported sensor %s\n",
> +			 acpi_dev_name(adev));
> +
> +		bridge->n_sensors++;
> +	}
> +
> +	return 0;
> +
> +err_free_swnodes:
> +	software_node_unregister_nodes(sensor->swnodes);
> +err_put_adev:
> +	acpi_dev_put(sensor->adev);
> +err_out:
> +	return ret;
> +}
> +
> +static int cio2_bridge_connect_sensors(struct cio2_bridge *bridge,
> +				       struct pci_dev *cio2)
> +{
> +	unsigned int i;
> +	int ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(cio2_supported_sensors); i++) {
> +		const struct cio2_sensor_config *cfg = &cio2_supported_sensors[i];
> +
> +		ret = cio2_bridge_connect_sensor(cfg, bridge, cio2);
> +		if (ret)
> +			goto err_unregister_sensors;
> +	}
> +
> +	return 0;
> +
> +err_unregister_sensors:
> +	cio2_bridge_unregister_sensors(bridge);
> +	return ret;
> +}
> +
> +int cio2_bridge_init(struct pci_dev *cio2)
> +{
> +	struct device *dev = &cio2->dev;
> +	struct fwnode_handle *fwnode;
> +	struct cio2_bridge *bridge;
> +	unsigned int i;
> +	int ret;
> +
> +	bridge = kzalloc(sizeof(*bridge), GFP_KERNEL);
> +	if (!bridge)
> +		return -ENOMEM;
> +
> +	strscpy(bridge->cio2_node_name, CIO2_HID, sizeof(bridge->cio2_node_name));
> +	bridge->cio2_hid_node.name = bridge->cio2_node_name;
> +
> +	ret = software_node_register(&bridge->cio2_hid_node);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to register the CIO2 HID node\n");
> +		goto err_free_bridge;
> +	}
> +
> +	/*
> +	 * Map the lane arrangement, which is fixed for the IPU3 (meaning we
> +	 * only need one, rather than one per sensor). We include it as a
> +	 * member of the struct cio2_bridge rather than a global variable so
> +	 * that it survives if the module is unloaded along with the rest of
> +	 * the struct.
> +	 */
> +	for (i = 0; i < CIO2_MAX_LANES; i++)
> +		bridge->data_lanes[i] = i + 1;
> +
> +	ret = cio2_bridge_connect_sensors(bridge, cio2);
> +	if (ret || bridge->n_sensors == 0)
> +		goto err_unregister_cio2;
> +
> +	dev_info(dev, "Connected %d cameras\n", bridge->n_sensors);
> +
> +	fwnode = software_node_fwnode(&bridge->cio2_hid_node);
> +	if (!fwnode) {
> +		dev_err(dev, "Error getting fwnode from cio2 software_node\n");
> +		ret = -ENODEV;
> +		goto err_unregister_sensors;
> +	}
> +
> +	set_secondary_fwnode(dev, fwnode);
> +
> +	return 0;
> +
> +err_unregister_sensors:
> +	cio2_bridge_unregister_sensors(bridge);
> +err_unregister_cio2:
> +	software_node_unregister(&bridge->cio2_hid_node);
> +err_free_bridge:
> +	kfree(bridge);
> +
> +	return ret;
> +}
> diff --git a/drivers/media/pci/intel/ipu3/cio2-bridge.h b/drivers/media/pci/intel/ipu3/cio2-bridge.h
> new file mode 100644
> index 000000000000..dd0ffcafa489
> --- /dev/null
> +++ b/drivers/media/pci/intel/ipu3/cio2-bridge.h
> @@ -0,0 +1,125 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Author: Dan Scally <djrscally@gmail.com> */
> +#ifndef __CIO2_BRIDGE_H
> +#define __CIO2_BRIDGE_H
> +
> +#include <linux/property.h>
> +#include <linux/types.h>
> +
> +#include "ipu3-cio2.h"
> +
> +#define CIO2_HID				"INT343E"
> +#define CIO2_MAX_LANES				4
> +#define MAX_NUM_LINK_FREQS			3
> +
> +#define CIO2_SENSOR_CONFIG(_HID, _NR, ...)	\
> +	(const struct cio2_sensor_config) {	\
> +		.hid = _HID,			\
> +		.nr_link_freqs = _NR,		\
> +		.link_freqs = { __VA_ARGS__ }	\
> +	}
> +
> +#define NODE_SENSOR(_HID, _PROPS)		\
> +	(const struct software_node) {		\
> +		.name = _HID,			\
> +		.properties = _PROPS,		\
> +	}
> +
> +#define NODE_PORT(_PORT, _SENSOR_NODE)		\
> +	(const struct software_node) {		\
> +		.name = _PORT,			\
> +		.parent = _SENSOR_NODE,		\
> +	}
> +
> +#define NODE_ENDPOINT(_EP, _PORT, _PROPS)	\
> +	(const struct software_node) {		\
> +		.name = _EP,			\
> +		.parent = _PORT,		\
> +		.properties = _PROPS,		\
> +	}
> +
> +enum cio2_sensor_swnodes {
> +	SWNODE_SENSOR_HID,
> +	SWNODE_SENSOR_PORT,
> +	SWNODE_SENSOR_ENDPOINT,
> +	SWNODE_CIO2_PORT,
> +	SWNODE_CIO2_ENDPOINT,
> +	SWNODE_COUNT
> +};
> +
> +/* Data representation as it is in ACPI SSDB buffer */
> +struct cio2_sensor_ssdb {
> +	u8 version;
> +	u8 sku;
> +	u8 guid_csi2[16];
> +	u8 devfunction;
> +	u8 bus;
> +	u32 dphylinkenfuses;
> +	u32 clockdiv;
> +	u8 link;
> +	u8 lanes;
> +	u32 csiparams[10];
> +	u32 maxlanespeed;
> +	u8 sensorcalibfileidx;
> +	u8 sensorcalibfileidxInMBZ[3];
> +	u8 romtype;
> +	u8 vcmtype;
> +	u8 platforminfo;
> +	u8 platformsubinfo;
> +	u8 flash;
> +	u8 privacyled;
> +	u8 degree;
> +	u8 mipilinkdefined;
> +	u32 mclkspeed;
> +	u8 controllogicid;
> +	u8 reserved1[3];
> +	u8 mclkport;
> +	u8 reserved2[13];
> +} __packed;
> +
> +struct cio2_property_names {
> +	char clock_frequency[16];
> +	char rotation[9];
> +	char bus_type[9];
> +	char data_lanes[11];
> +	char remote_endpoint[16];
> +	char link_frequencies[17];
> +};
> +
> +struct cio2_node_names {
> +	char port[7];
> +	char endpoint[11];
> +	char remote_port[7];
> +};
> +
> +struct cio2_sensor_config {
> +	const char *hid;
> +	const u8 nr_link_freqs;
> +	const u64 link_freqs[MAX_NUM_LINK_FREQS];
> +};
> +
> +struct cio2_sensor {
> +	char name[ACPI_ID_LEN];
> +	struct acpi_device *adev;
> +
> +	struct software_node swnodes[6];
> +	struct cio2_node_names node_names;
> +
> +	struct cio2_sensor_ssdb ssdb;
> +	struct cio2_property_names prop_names;
> +	struct property_entry ep_properties[5];
> +	struct property_entry dev_properties[3];
> +	struct property_entry cio2_properties[3];
> +	struct software_node_ref_args local_ref[1];
> +	struct software_node_ref_args remote_ref[1];
> +};
> +
> +struct cio2_bridge {
> +	char cio2_node_name[ACPI_ID_LEN];
> +	struct software_node cio2_hid_node;
> +	u32 data_lanes[4];
> +	unsigned int n_sensors;
> +	struct cio2_sensor sensors[CIO2_NUM_PORTS];
> +};
> +
> +#endif
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> index 36e354ecf71e..db59b19a6236 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2-main.c
> @@ -1702,11 +1702,28 @@ static void cio2_queues_exit(struct cio2_device *cio2)
>  		cio2_queue_exit(cio2, &cio2->queue[i]);
>  }
>  
> +static int cio2_check_fwnode_graph(struct fwnode_handle *fwnode)
> +{
> +	struct fwnode_handle *endpoint;
> +
> +	if (IS_ERR_OR_NULL(fwnode))
> +		return -EINVAL;
> +
> +	endpoint = fwnode_graph_get_next_endpoint(fwnode, NULL);
> +	if (endpoint) {
> +		fwnode_handle_put(endpoint);
> +		return 0;
> +	}
> +
> +	return cio2_check_fwnode_graph(fwnode->secondary);
> +}
> +
>  /**************** PCI interface ****************/
>  
>  static int cio2_pci_probe(struct pci_dev *pci_dev,
>  			  const struct pci_device_id *id)
>  {
> +	struct fwnode_handle *fwnode = dev_fwnode(&pci_dev->dev);
>  	struct cio2_device *cio2;
>  	int r;
>  
> @@ -1715,6 +1732,23 @@ static int cio2_pci_probe(struct pci_dev *pci_dev,
>  		return -ENOMEM;
>  	cio2->pci_dev = pci_dev;
>  
> +	/*
> +	 * On some platforms no connections to sensors are defined in firmware,
> +	 * if the device has no endpoints then we can try to build those as
> +	 * software_nodes parsed from SSDB.
> +	 */
> +	r = cio2_check_fwnode_graph(fwnode);
> +	if (r) {
> +		if (fwnode && !IS_ERR_OR_NULL(fwnode->secondary)) {
> +			dev_err(&pci_dev->dev, "fwnode graph has no endpoints connected\n");
> +			return -EINVAL;
> +		}
> +
> +		r = cio2_bridge_init(pci_dev);
> +		if (r)
> +			return r;
> +	}
> +
>  	r = pcim_enable_device(pci_dev);
>  	if (r) {
>  		dev_err(&pci_dev->dev, "failed to enable device (%d)\n", r);
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.h b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
> index 62187ab5ae43..dc3e343a37fb 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.h
> +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.h
> @@ -455,4 +455,10 @@ static inline struct cio2_queue *vb2q_to_cio2_queue(struct vb2_queue *vq)
>  	return container_of(vq, struct cio2_queue, vbq);
>  }
>  
> +#if IS_ENABLED(CONFIG_CIO2_BRIDGE)
> +int cio2_bridge_init(struct pci_dev *cio2);
> +#else
> +int cio2_bridge_init(struct pci_dev *cio2) { return 0; }
> +#endif
> +
>  #endif
> 

Re: [PATCH v5 00/15] Add functionality to ipu3-cio2 driver allowing software_node connections to sensors on platforms designed for Windows

[Laurent Pinchart]

Hi Greg,

On Thu, Jan 07, 2021 at 01:28:23PM +0000, Daniel Scally wrote:
> 
> Hello all
> 
> v4:
> https://lore.kernel.org/linux-media/20210103231235.792999-1-djrscally@gmail.com/T/#m11b7cb977e1b73fba1e625c3d6a189e2943a7783
> v3:
> https://lore.kernel.org/linux-media/20201224010907.263125-1-djrscally@gmail.com/T/#m37b831bb2b406917d6db5da9acf9ed35df65d72d
> v2:
> https://lore.kernel.org/linux-media/20201217234337.1983732-1-djrscally@gmail.com/T/#md93fd090009b42a6a98aed892aff0d38cf07e0cd
> v1:
> https://lore.kernel.org/linux-media/20201130133129.1024662-1-djrscally@gmail.com/T/#m91934e12e3d033da2e768e952ea3b4a125ee3e67
> 
> This series is to start adding support for webcams on laptops with ACPI tables
> designed for use with CIO2 on Windows. This series extends the ipu3-cio2
> driver to allow for patching the firmware via software_nodes if endpoints
> aren't defined by ACPI.
> 
> I'm hopeful that most or all of this series could get picked up for 5.12.
> We touch a few different areas (listed below), but I think the easiest
> approach would be to merge everything through media tree. Rafael, Greg,
> Mauro and Sergey; are you ok with that plan, or would you prefer a
> different approach? Mauro; if that plan is ok (and of course assuming that
> the rest of the patches are acked by their respective maintainers) could
> we get a dedicated feature branch just in case the following series ends
> up being ready in time too?
> 
> lib
>   lib/test_printf.c: Use helper function to unwind array of
>     software_nodes
> 
> base
>   software_node: Fix refcounts in software_node_get_next_child()
>   property: Return true in fwnode_device_is_available for NULL ops
>   property: Call fwnode_graph_get_endpoint_by_id() for fwnode->secondary
>   software_node: Enforce parent before child ordering of nodes arrays
>   software_node: unregister software_nodes in reverse order

Could you please let us know with an Acked-by if these patches can be
merged through the linux-media tree for v5.12 ? This is a cross-tree
series and we would like to avoid topic branches if possible.

>   include: fwnode.h: Define format macros for ports and endpoints
> 
> acpi
>   acpi: Add acpi_dev_get_next_match_dev() and helper macro
> 
> media
>   media: v4l2-core: v4l2-async: Check sd->fwnode->secondary in
>     match_fwnode()
>   ipu3-cio2: Add T: entry to MAINTAINERS
>   ipu3-cio2: Rename ipu3-cio2.c
>   ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver
>   include: media: v4l2-fwnode: Include v4l2_fwnode_bus_type
> 
> Series-level changelog:
> 	- Rebased onto 5.11-rc1
> 
> Thanks
> Dan
> 
> Andy Shevchenko (1):
>   media: ipu3-cio2: Add headers that ipu3-cio2.h is direct user of
> 
> Daniel Scally (13):
>   software_node: Fix refcounts in software_node_get_next_child()
>   device property: Return true in fwnode_device_is_available for NULL
>     ops
>   device property: Call fwnode_graph_get_endpoint_by_id() for
>     fwnode->secondary
>   software_node: Enforce parent before child ordering of nodes arrays
>   software_node: unregister software_nodes in reverse order
>   device property: Define format macros for ports and endpoints
>   lib/test_printf.c: Use helper function to unwind array of
>     software_nodes
>   ipu3-cio2: Add T: entry to MAINTAINERS
>   ipu3-cio2: Rename ipu3-cio2.c
>   media: v4l2-core: v4l2-async: Check sd->fwnode->secondary in
>     match_fwnode()
>   ACPI / bus: Add acpi_dev_get_next_match_dev() and helper macro
>   media: v4l2-fwnode: Include v4l2_fwnode_bus_type
>   ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver
> 
> Heikki Krogerus (1):
>   software_node: Add support for fwnode_graph*() family of functions
> 
>  MAINTAINERS                                   |   2 +
>  drivers/acpi/utils.c                          |  30 +-
>  drivers/base/property.c                       |  15 +-
>  drivers/base/swnode.c                         | 180 ++++++++--
>  drivers/media/pci/intel/ipu3/Kconfig          |  18 +
>  drivers/media/pci/intel/ipu3/Makefile         |   3 +
>  drivers/media/pci/intel/ipu3/cio2-bridge.c    | 311 ++++++++++++++++++
>  drivers/media/pci/intel/ipu3/cio2-bridge.h    | 125 +++++++
>  .../ipu3/{ipu3-cio2.c => ipu3-cio2-main.c}    |  34 ++
>  drivers/media/pci/intel/ipu3/ipu3-cio2.h      |  24 ++
>  drivers/media/v4l2-core/v4l2-async.c          |   8 +
>  drivers/media/v4l2-core/v4l2-fwnode.c         |  11 -
>  include/acpi/acpi_bus.h                       |   7 +
>  include/linux/fwnode.h                        |   7 +
>  include/media/v4l2-fwnode.h                   |  22 ++
>  lib/test_printf.c                             |   4 +-
>  16 files changed, 763 insertions(+), 38 deletions(-)
>  create mode 100644 drivers/media/pci/intel/ipu3/cio2-bridge.c
>  create mode 100644 drivers/media/pci/intel/ipu3/cio2-bridge.h
>  rename drivers/media/pci/intel/ipu3/{ipu3-cio2.c => ipu3-cio2-main.c} (98%)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 00/15] Add functionality to ipu3-cio2 driver allowing software_node connections to sensors on platforms designed for Windows

[Greg Kroah-Hartman]

On Sat, Jan 09, 2021 at 04:08:04AM +0200, Laurent Pinchart wrote:
> Hi Greg,
> 
> On Thu, Jan 07, 2021 at 01:28:23PM +0000, Daniel Scally wrote:
> > 
> > Hello all
> > 
> > v4:
> > https://lore.kernel.org/linux-media/20210103231235.792999-1-djrscally@gmail.com/T/#m11b7cb977e1b73fba1e625c3d6a189e2943a7783
> > v3:
> > https://lore.kernel.org/linux-media/20201224010907.263125-1-djrscally@gmail.com/T/#m37b831bb2b406917d6db5da9acf9ed35df65d72d
> > v2:
> > https://lore.kernel.org/linux-media/20201217234337.1983732-1-djrscally@gmail.com/T/#md93fd090009b42a6a98aed892aff0d38cf07e0cd
> > v1:
> > https://lore.kernel.org/linux-media/20201130133129.1024662-1-djrscally@gmail.com/T/#m91934e12e3d033da2e768e952ea3b4a125ee3e67
> > 
> > This series is to start adding support for webcams on laptops with ACPI tables
> > designed for use with CIO2 on Windows. This series extends the ipu3-cio2
> > driver to allow for patching the firmware via software_nodes if endpoints
> > aren't defined by ACPI.
> > 
> > I'm hopeful that most or all of this series could get picked up for 5.12.
> > We touch a few different areas (listed below), but I think the easiest
> > approach would be to merge everything through media tree. Rafael, Greg,
> > Mauro and Sergey; are you ok with that plan, or would you prefer a
> > different approach? Mauro; if that plan is ok (and of course assuming that
> > the rest of the patches are acked by their respective maintainers) could
> > we get a dedicated feature branch just in case the following series ends
> > up being ready in time too?
> > 
> > lib
> >   lib/test_printf.c: Use helper function to unwind array of
> >     software_nodes
> > 
> > base
> >   software_node: Fix refcounts in software_node_get_next_child()
> >   property: Return true in fwnode_device_is_available for NULL ops
> >   property: Call fwnode_graph_get_endpoint_by_id() for fwnode->secondary
> >   software_node: Enforce parent before child ordering of nodes arrays
> >   software_node: unregister software_nodes in reverse order
> 
> Could you please let us know with an Acked-by if these patches can be
> merged through the linux-media tree for v5.12 ? This is a cross-tree
> series and we would like to avoid topic branches if possible.

Yes, they are all fine with me, will go provide my ack, sorry for the
delay.

greg k-h

[Devel] Re: [PATCH v5 00/15] Add functionality to ipu3-cio2 driver allowing software_node connections to sensors on platforms designed for Windows

[Rafael J. Wysocki]

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

On Thu, Jan 7, 2021 at 2:30 PM Daniel Scally <djrscally(a)gmail.com> wrote:
>
>
> Hello all
>
> v4:
> https://lore.kernel.org/linux-media/20210103231235.792999-1-djrscally(a)gmail.com/T/#m11b7cb977e1b73fba1e625c3d6a189e2943a7783
> v3:
> https://lore.kernel.org/linux-media/20201224010907.263125-1-djrscally(a)gmail.com/T/#m37b831bb2b406917d6db5da9acf9ed35df65d72d
> v2:
> https://lore.kernel.org/linux-media/20201217234337.1983732-1-djrscally(a)gmail.com/T/#md93fd090009b42a6a98aed892aff0d38cf07e0cd
> v1:
> https://lore.kernel.org/linux-media/20201130133129.1024662-1-djrscally(a)gmail.com/T/#m91934e12e3d033da2e768e952ea3b4a125ee3e67
>
> This series is to start adding support for webcams on laptops with ACPI tables
> designed for use with CIO2 on Windows. This series extends the ipu3-cio2
> driver to allow for patching the firmware via software_nodes if endpoints
> aren't defined by ACPI.
>
> I'm hopeful that most or all of this series could get picked up for 5.12.
> We touch a few different areas (listed below), but I think the easiest
> approach would be to merge everything through media tree. Rafael, Greg,
> Mauro and Sergey; are you ok with that plan, or would you prefer a
> different approach? Mauro; if that plan is ok (and of course assuming that
> the rest of the patches are acked by their respective maintainers) could
> we get a dedicated feature branch just in case the following series ends
> up being ready in time too?
>
> lib
>   lib/test_printf.c: Use helper function to unwind array of
>     software_nodes
>
> base
>   software_node: Fix refcounts in software_node_get_next_child()
>   property: Return true in fwnode_device_is_available for NULL ops
>   property: Call fwnode_graph_get_endpoint_by_id() for fwnode->secondary
>   software_node: Enforce parent before child ordering of nodes arrays
>   software_node: unregister software_nodes in reverse order
>   include: fwnode.h: Define format macros for ports and endpoints
>
> acpi
>   acpi: Add acpi_dev_get_next_match_dev() and helper macro
>
> media
>   media: v4l2-core: v4l2-async: Check sd->fwnode->secondary in
>     match_fwnode()
>   ipu3-cio2: Add T: entry to MAINTAINERS
>   ipu3-cio2: Rename ipu3-cio2.c
>   ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver
>   include: media: v4l2-fwnode: Include v4l2_fwnode_bus_type
>
> Series-level changelog:
>         - Rebased onto 5.11-rc1
>
> Thanks
> Dan
>
> Andy Shevchenko (1):
>   media: ipu3-cio2: Add headers that ipu3-cio2.h is direct user of
>
> Daniel Scally (13):
>   software_node: Fix refcounts in software_node_get_next_child()
>   device property: Return true in fwnode_device_is_available for NULL
>     ops
>   device property: Call fwnode_graph_get_endpoint_by_id() for
>     fwnode->secondary
>   software_node: Enforce parent before child ordering of nodes arrays
>   software_node: unregister software_nodes in reverse order
>   device property: Define format macros for ports and endpoints
>   lib/test_printf.c: Use helper function to unwind array of
>     software_nodes
>   ipu3-cio2: Add T: entry to MAINTAINERS
>   ipu3-cio2: Rename ipu3-cio2.c
>   media: v4l2-core: v4l2-async: Check sd->fwnode->secondary in
>     match_fwnode()
>   ACPI / bus: Add acpi_dev_get_next_match_dev() and helper macro
>   media: v4l2-fwnode: Include v4l2_fwnode_bus_type
>   ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver
>
> Heikki Krogerus (1):
>   software_node: Add support for fwnode_graph*() family of functions

Please feel free to add

Acked-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>

to all of the device properties patches in this series if that helps.

Thanks!

Re: [PATCH v5 00/15] Add functionality to ipu3-cio2 driver allowing software_node connections to sensors on platforms designed for Windows

[Rafael J. Wysocki]

On Thu, Jan 7, 2021 at 2:30 PM Daniel Scally <djrscally@gmail.com> wrote:
>
>
> Hello all
>
> v4:
> https://lore.kernel.org/linux-media/20210103231235.792999-1-djrscally@gmail.com/T/#m11b7cb977e1b73fba1e625c3d6a189e2943a7783
> v3:
> https://lore.kernel.org/linux-media/20201224010907.263125-1-djrscally@gmail.com/T/#m37b831bb2b406917d6db5da9acf9ed35df65d72d
> v2:
> https://lore.kernel.org/linux-media/20201217234337.1983732-1-djrscally@gmail.com/T/#md93fd090009b42a6a98aed892aff0d38cf07e0cd
> v1:
> https://lore.kernel.org/linux-media/20201130133129.1024662-1-djrscally@gmail.com/T/#m91934e12e3d033da2e768e952ea3b4a125ee3e67
>
> This series is to start adding support for webcams on laptops with ACPI tables
> designed for use with CIO2 on Windows. This series extends the ipu3-cio2
> driver to allow for patching the firmware via software_nodes if endpoints
> aren't defined by ACPI.
>
> I'm hopeful that most or all of this series could get picked up for 5.12.
> We touch a few different areas (listed below), but I think the easiest
> approach would be to merge everything through media tree. Rafael, Greg,
> Mauro and Sergey; are you ok with that plan, or would you prefer a
> different approach? Mauro; if that plan is ok (and of course assuming that
> the rest of the patches are acked by their respective maintainers) could
> we get a dedicated feature branch just in case the following series ends
> up being ready in time too?
>
> lib
>   lib/test_printf.c: Use helper function to unwind array of
>     software_nodes
>
> base
>   software_node: Fix refcounts in software_node_get_next_child()
>   property: Return true in fwnode_device_is_available for NULL ops
>   property: Call fwnode_graph_get_endpoint_by_id() for fwnode->secondary
>   software_node: Enforce parent before child ordering of nodes arrays
>   software_node: unregister software_nodes in reverse order
>   include: fwnode.h: Define format macros for ports and endpoints
>
> acpi
>   acpi: Add acpi_dev_get_next_match_dev() and helper macro
>
> media
>   media: v4l2-core: v4l2-async: Check sd->fwnode->secondary in
>     match_fwnode()
>   ipu3-cio2: Add T: entry to MAINTAINERS
>   ipu3-cio2: Rename ipu3-cio2.c
>   ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver
>   include: media: v4l2-fwnode: Include v4l2_fwnode_bus_type
>
> Series-level changelog:
>         - Rebased onto 5.11-rc1
>
> Thanks
> Dan
>
> Andy Shevchenko (1):
>   media: ipu3-cio2: Add headers that ipu3-cio2.h is direct user of
>
> Daniel Scally (13):
>   software_node: Fix refcounts in software_node_get_next_child()
>   device property: Return true in fwnode_device_is_available for NULL
>     ops
>   device property: Call fwnode_graph_get_endpoint_by_id() for
>     fwnode->secondary
>   software_node: Enforce parent before child ordering of nodes arrays
>   software_node: unregister software_nodes in reverse order
>   device property: Define format macros for ports and endpoints
>   lib/test_printf.c: Use helper function to unwind array of
>     software_nodes
>   ipu3-cio2: Add T: entry to MAINTAINERS
>   ipu3-cio2: Rename ipu3-cio2.c
>   media: v4l2-core: v4l2-async: Check sd->fwnode->secondary in
>     match_fwnode()
>   ACPI / bus: Add acpi_dev_get_next_match_dev() and helper macro
>   media: v4l2-fwnode: Include v4l2_fwnode_bus_type
>   ipu3-cio2: Add cio2-bridge to ipu3-cio2 driver
>
> Heikki Krogerus (1):
>   software_node: Add support for fwnode_graph*() family of functions

Please feel free to add

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

to all of the device properties patches in this series if that helps.

Thanks!

Re: [PATCH v5 00/15] Add functionality to ipu3-cio2 driver allowing software_node connections to sensors on platforms designed for Windows

[Daniel Scally]

Hi Rafael, Sakari

On 12/01/2021 19:34, Rafael J. Wysocki wrote:
> <snip>
>> I'm hopeful that most or all of this series could get picked up for 5.12.
>> We touch a few different areas (listed below), but I think the easiest
>> approach would be to merge everything through media tree. Rafael, Greg,
>> Mauro and Sergey; are you ok with that plan, or would you prefer a
>> different approach? Mauro; if that plan is ok (and of course assuming that
>> the rest of the patches are acked by their respective maintainers) could
>> we get a dedicated feature branch just in case the following series ends
>> up being ready in time too?
>>
>> <snip>
> Please feel free to add
>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> to all of the device properties patches in this series if that helps.
>
> Thanks!

Thanks very much (and Greg too).


Sakari; unless I'm misunderstanding something, I think this series could
be picked up now, right? Would it be ok to do that through your tree? I
think the idea of a dedicated feature branch can be dropped, I won't
have the second series ready in time for this round anyway.


First time doing this, so if I've missed something please let me know!

Re: [PATCH v5 00/15] Add functionality to ipu3-cio2 driver allowing software_node connections to sensors on platforms designed for Windows

[Sakari Ailus]

Hi Daniel,

On Tue, Jan 12, 2021 at 11:35:37PM +0000, Daniel Scally wrote:
> Hi Rafael, Sakari
> 
> On 12/01/2021 19:34, Rafael J. Wysocki wrote:
> > <snip>
> >> I'm hopeful that most or all of this series could get picked up for 5.12.
> >> We touch a few different areas (listed below), but I think the easiest
> >> approach would be to merge everything through media tree. Rafael, Greg,
> >> Mauro and Sergey; are you ok with that plan, or would you prefer a
> >> different approach? Mauro; if that plan is ok (and of course assuming that
> >> the rest of the patches are acked by their respective maintainers) could
> >> we get a dedicated feature branch just in case the following series ends
> >> up being ready in time too?
> >>
> >> <snip>
> > Please feel free to add
> >
> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > to all of the device properties patches in this series if that helps.
> >
> > Thanks!
> 
> Thanks very much (and Greg too).
> 
> 
> Sakari; unless I'm misunderstanding something, I think this series could
> be picked up now, right? Would it be ok to do that through your tree? I
> think the idea of a dedicated feature branch can be dropped, I won't
> have the second series ready in time for this round anyway.
> 
> 
> First time doing this, so if I've missed something please let me know!

I think it's ready, indeed. I'll let you know if there are any issues.

-- 
Kind regards,

Sakari Ailus

[Devel] Re: [PATCH v5 13/15] ACPI / bus: Add acpi_dev_get_next_match_dev() and helper macro

[Rafael J. Wysocki]

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

On Thu, Jan 7, 2021 at 2:31 PM Daniel Scally <djrscally(a)gmail.com> wrote:
>
> To ensure we handle situations in which multiple sensors of the same
> model (and therefore _HID) are present in a system, we need to be able
> to iterate over devices matching a known _HID but unknown _UID and _HRV
>  - add acpi_dev_get_next_match_dev() to accommodate that possibility and
> change acpi_dev_get_first_match_dev() to simply call the new function
> with a NULL starting point. Add an iterator macro for convenience.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko(a)linux.intel.com>
> Reviewed-by: Sakari Ailus <sakari.ailus(a)linux.intel.com>
> Suggested-by: Andy Shevchenko <andriy.shevchenko(a)linux.intel.com>
> Signed-off-by: Daniel Scally <djrscally(a)gmail.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>

> ---
> Changes in v5:
>
>         - Changed commit subject
>
>  drivers/acpi/utils.c    | 30 ++++++++++++++++++++++++++----
>  include/acpi/acpi_bus.h |  7 +++++++
>  2 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
> index d5411a166685..ddca1550cce6 100644
> --- a/drivers/acpi/utils.c
> +++ b/drivers/acpi/utils.c
> @@ -843,12 +843,13 @@ bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
>  EXPORT_SYMBOL(acpi_dev_present);
>
>  /**
> - * acpi_dev_get_first_match_dev - Return the first match of ACPI device
> + * acpi_dev_get_next_match_dev - Return the next match of ACPI device
> + * @adev: Pointer to the previous acpi_device matching this @hid, @uid and @hrv
>   * @hid: Hardware ID of the device.
>   * @uid: Unique ID of the device, pass NULL to not check _UID
>   * @hrv: Hardware Revision of the device, pass -1 to not check _HRV
>   *
> - * Return the first match of ACPI device if a matching device was present
> + * Return the next match of ACPI device if another matching device was present
>   * at the moment of invocation, or NULL otherwise.
>   *
>   * The caller is responsible to call put_device() on the returned device.
> @@ -856,8 +857,9 @@ EXPORT_SYMBOL(acpi_dev_present);
>   * See additional information in acpi_dev_present() as well.
>   */
>  struct acpi_device *
> -acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
> +acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv)
>  {
> +       struct device *start = adev ? &adev->dev : NULL;
>         struct acpi_dev_match_info match = {};
>         struct device *dev;
>
> @@ -865,9 +867,29 @@ acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
>         match.uid = uid;
>         match.hrv = hrv;
>
> -       dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb);
> +       dev = bus_find_device(&acpi_bus_type, start, &match, acpi_dev_match_cb);
>         return dev ? to_acpi_device(dev) : NULL;
>  }
> +EXPORT_SYMBOL(acpi_dev_get_next_match_dev);
> +
> +/**
> + * acpi_dev_get_first_match_dev - Return the first match of ACPI device
> + * @hid: Hardware ID of the device.
> + * @uid: Unique ID of the device, pass NULL to not check _UID
> + * @hrv: Hardware Revision of the device, pass -1 to not check _HRV
> + *
> + * Return the first match of ACPI device if a matching device was present
> + * at the moment of invocation, or NULL otherwise.
> + *
> + * The caller is responsible to call put_device() on the returned device.
> + *
> + * See additional information in acpi_dev_present() as well.
> + */
> +struct acpi_device *
> +acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
> +{
> +       return acpi_dev_get_next_match_dev(NULL, hid, uid, hrv);
> +}
>  EXPORT_SYMBOL(acpi_dev_get_first_match_dev);
>
>  /*
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 6d1879bf9440..02a716a0af5d 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -683,9 +683,16 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
>
>  bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2);
>
> +struct acpi_device *
> +acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv);
>  struct acpi_device *
>  acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv);
>
> +#define for_each_acpi_dev_match(adev, hid, uid, hrv)                   \
> +       for (adev = acpi_dev_get_first_match_dev(hid, uid, hrv);        \
> +            adev;                                                      \
> +            adev = acpi_dev_get_next_match_dev(adev, hid, uid, hrv))
> +
>  static inline void acpi_dev_put(struct acpi_device *adev)
>  {
>         put_device(&adev->dev);
> --
> 2.25.1
>

Re: [PATCH v5 13/15] ACPI / bus: Add acpi_dev_get_next_match_dev() and helper macro

[Rafael J. Wysocki]

On Thu, Jan 7, 2021 at 2:31 PM Daniel Scally <djrscally@gmail.com> wrote:
>
> To ensure we handle situations in which multiple sensors of the same
> model (and therefore _HID) are present in a system, we need to be able
> to iterate over devices matching a known _HID but unknown _UID and _HRV
>  - add acpi_dev_get_next_match_dev() to accommodate that possibility and
> change acpi_dev_get_first_match_dev() to simply call the new function
> with a NULL starting point. Add an iterator macro for convenience.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
> Changes in v5:
>
>         - Changed commit subject
>
>  drivers/acpi/utils.c    | 30 ++++++++++++++++++++++++++----
>  include/acpi/acpi_bus.h |  7 +++++++
>  2 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
> index d5411a166685..ddca1550cce6 100644
> --- a/drivers/acpi/utils.c
> +++ b/drivers/acpi/utils.c
> @@ -843,12 +843,13 @@ bool acpi_dev_present(const char *hid, const char *uid, s64 hrv)
>  EXPORT_SYMBOL(acpi_dev_present);
>
>  /**
> - * acpi_dev_get_first_match_dev - Return the first match of ACPI device
> + * acpi_dev_get_next_match_dev - Return the next match of ACPI device
> + * @adev: Pointer to the previous acpi_device matching this @hid, @uid and @hrv
>   * @hid: Hardware ID of the device.
>   * @uid: Unique ID of the device, pass NULL to not check _UID
>   * @hrv: Hardware Revision of the device, pass -1 to not check _HRV
>   *
> - * Return the first match of ACPI device if a matching device was present
> + * Return the next match of ACPI device if another matching device was present
>   * at the moment of invocation, or NULL otherwise.
>   *
>   * The caller is responsible to call put_device() on the returned device.
> @@ -856,8 +857,9 @@ EXPORT_SYMBOL(acpi_dev_present);
>   * See additional information in acpi_dev_present() as well.
>   */
>  struct acpi_device *
> -acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
> +acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv)
>  {
> +       struct device *start = adev ? &adev->dev : NULL;
>         struct acpi_dev_match_info match = {};
>         struct device *dev;
>
> @@ -865,9 +867,29 @@ acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
>         match.uid = uid;
>         match.hrv = hrv;
>
> -       dev = bus_find_device(&acpi_bus_type, NULL, &match, acpi_dev_match_cb);
> +       dev = bus_find_device(&acpi_bus_type, start, &match, acpi_dev_match_cb);
>         return dev ? to_acpi_device(dev) : NULL;
>  }
> +EXPORT_SYMBOL(acpi_dev_get_next_match_dev);
> +
> +/**
> + * acpi_dev_get_first_match_dev - Return the first match of ACPI device
> + * @hid: Hardware ID of the device.
> + * @uid: Unique ID of the device, pass NULL to not check _UID
> + * @hrv: Hardware Revision of the device, pass -1 to not check _HRV
> + *
> + * Return the first match of ACPI device if a matching device was present
> + * at the moment of invocation, or NULL otherwise.
> + *
> + * The caller is responsible to call put_device() on the returned device.
> + *
> + * See additional information in acpi_dev_present() as well.
> + */
> +struct acpi_device *
> +acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv)
> +{
> +       return acpi_dev_get_next_match_dev(NULL, hid, uid, hrv);
> +}
>  EXPORT_SYMBOL(acpi_dev_get_first_match_dev);
>
>  /*
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 6d1879bf9440..02a716a0af5d 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -683,9 +683,16 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
>
>  bool acpi_dev_hid_uid_match(struct acpi_device *adev, const char *hid2, const char *uid2);
>
> +struct acpi_device *
> +acpi_dev_get_next_match_dev(struct acpi_device *adev, const char *hid, const char *uid, s64 hrv);
>  struct acpi_device *
>  acpi_dev_get_first_match_dev(const char *hid, const char *uid, s64 hrv);
>
> +#define for_each_acpi_dev_match(adev, hid, uid, hrv)                   \
> +       for (adev = acpi_dev_get_first_match_dev(hid, uid, hrv);        \
> +            adev;                                                      \
> +            adev = acpi_dev_get_next_match_dev(adev, hid, uid, hrv))
> +
>  static inline void acpi_dev_put(struct acpi_device *adev)
>  {
>         put_device(&adev->dev);
> --
> 2.25.1
>

[Devel] Re: [PATCH v5 13/15] ACPI / bus: Add acpi_dev_get_next_match_dev() and helper macro

[Rafael J. Wysocki]

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

Hi,

On Sat, Jan 9, 2021 at 2:55 AM Laurent Pinchart
<laurent.pinchart(a)ideasonboard.com> wrote:
>
> Hi Rafael,
>
> Could you please review this patch, and let us know (see question in the
> cover letter)

Done, sorry for the delay.

> if it can be merged through the linux-media tree for v5.12
> ?

Yes, it can.

Thanks!

Re: [PATCH v5 13/15] ACPI / bus: Add acpi_dev_get_next_match_dev() and helper macro

[Rafael J. Wysocki]

Hi,

On Sat, Jan 9, 2021 at 2:55 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Rafael,
>
> Could you please review this patch, and let us know (see question in the
> cover letter)

Done, sorry for the delay.

> if it can be merged through the linux-media tree for v5.12
> ?

Yes, it can.

Thanks!

[Devel] Re: [PATCH v5 07/15] device property: Define format macros for ports and endpoints

[Rafael J. Wysocki]

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

On Thu, Jan 7, 2021 at 2:31 PM Daniel Scally <djrscally(a)gmail.com> wrote:
>
> OF, ACPI and software_nodes all implement graphs including nodes for ports
> and endpoints. These are all intended to be named with a common schema,
> as "port(a)n" and "endpoint(a)n" where n is an unsigned int representing the
> index of the node. To ensure commonality across the subsystems, provide a
> set of macros to define the format.
>
> Suggested-by: Andy Shevchenko <andriy.shevchenko(a)linux.intel.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko(a)gmail.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart(a)ideasonboard.com>
> Signed-off-by: Daniel Scally <djrscally(a)gmail.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com>

> ---
> Changes in v5:
>
>         - Changed commit subject
>
>  include/linux/fwnode.h | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index fde4ad97564c..77414e431e89 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -50,6 +50,13 @@ struct fwnode_endpoint {
>         const struct fwnode_handle *local_fwnode;
>  };
>
> +/*
> + * ports and endpoints defined as software_nodes should all follow a common
> + * naming scheme; use these macros to ensure commonality.
> + */
> +#define SWNODE_GRAPH_PORT_NAME_FMT             "port@%u"
> +#define SWNODE_GRAPH_ENDPOINT_NAME_FMT         "endpoint@%u"
> +
>  #define NR_FWNODE_REFERENCE_ARGS       8
>
>  /**
> --
> 2.25.1
>

Re: [PATCH v5 07/15] device property: Define format macros for ports and endpoints

[Rafael J. Wysocki]

On Thu, Jan 7, 2021 at 2:31 PM Daniel Scally <djrscally@gmail.com> wrote:
>
> OF, ACPI and software_nodes all implement graphs including nodes for ports
> and endpoints. These are all intended to be named with a common schema,
> as "port@n" and "endpoint@n" where n is an unsigned int representing the
> index of the node. To ensure commonality across the subsystems, provide a
> set of macros to define the format.
>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Daniel Scally <djrscally@gmail.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
> Changes in v5:
>
>         - Changed commit subject
>
>  include/linux/fwnode.h | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
> index fde4ad97564c..77414e431e89 100644
> --- a/include/linux/fwnode.h
> +++ b/include/linux/fwnode.h
> @@ -50,6 +50,13 @@ struct fwnode_endpoint {
>         const struct fwnode_handle *local_fwnode;
>  };
>
> +/*
> + * ports and endpoints defined as software_nodes should all follow a common
> + * naming scheme; use these macros to ensure commonality.
> + */
> +#define SWNODE_GRAPH_PORT_NAME_FMT             "port@%u"
> +#define SWNODE_GRAPH_ENDPOINT_NAME_FMT         "endpoint@%u"
> +
>  #define NR_FWNODE_REFERENCE_ARGS       8
>
>  /**
> --
> 2.25.1
>

[Devel] Re: [PATCH v5 07/15] device property: Define format macros for ports and endpoints

[Rafael J. Wysocki]

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

On Sat, Jan 9, 2021 at 3:06 AM Laurent Pinchart
<laurent.pinchart(a)ideasonboard.com> wrote:
>
> Hi Rafael,
>
> Could you please let us know with an Acked-by

Done, sorry for the delay.

> if this patch can be merged through the linux-media tree for v5.12 ?

Yes, it can.

Thanks!

Re: [PATCH v5 07/15] device property: Define format macros for ports and endpoints

[Rafael J. Wysocki]

On Sat, Jan 9, 2021 at 3:06 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Rafael,
>
> Could you please let us know with an Acked-by

Done, sorry for the delay.

> if this patch can be merged through the linux-media tree for v5.12 ?

Yes, it can.

Thanks!