linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/8] ACPI _CRS CSI-2 and MIPI DisCo for Imaging support
@ 2023-02-08 21:27 Sakari Ailus
  2023-02-08 21:27 ` [PATCH v5 1/8] ACPI: property: Parse data node string references in properties Sakari Ailus
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Sakari Ailus @ 2023-02-08 21:27 UTC (permalink / raw)
  To: linux-acpi; +Cc: linux-media, rafael, andriy.shevchenko, heikki.krogerus

Hello all,

Here's an implementation of ACPI 6.4 _CRS CSI-2 resource descriptor and
MIPI DisCo for Imaging 1.0 [1]. What the two basically provide is an
officially sanctioned way to describe CSI-2 connected cameras to operating
system software, something DT based systems have enjoyed for quite some
time already.

The implementation digs the information from ACPI tables (_CRS descriptors
and data + property extensions) and constructs software nodes that are
compatible with Documentation/firmware-guide/acpi/dsd/graph.rst and
Documentation/devicetree/bindings/media/video-interface-devices.yaml . No
specific driver changes are needed.

These patches are on the testing branch of the linux-acpi tree where they
depend on the patch constifying the ACPI pathname argument for
acpi_get_handle() (commit 91fdb91ccca2b48572a1ccf1d382fd599e3e1237).

[1] https://www.mipi.org/specifications/mipi-disco-imaging

since v4:

- Add leading dots to comment sentences.

- Use UINT_MAX - 1 to denote an unallocated port instead of ~1U.

- Unwrap a line.

- Get ACPI handle into a local variable in acpi_init_swnodes() for easier
  use, use acpi_device_handle() to obtain it.

- Rework "rotation" property checking and _PLD object access.

- Also obtain return value of acpi_get_name() into a local variable before
  testing it.

- Use a local variable for the first element string pointer in
  acpi_properties_prepare_mipi().

since v3:

- Add comments to data structures and functions, code inside functions.

- Use ACPI_FAILURE() for testing ACPI framework function return values.

- Unwrap a few lines.

- Rename list heads as "head", some were called just "list".

- Count ACPI handles related to _CRS CSI2 resources during tree walk.

- Reshape testing for CSI-2 port allocation in next_csi2_port_index().

- Move allocation of software nodes into a new function,
  acpi_crs_csi2_alloc_fill_swnodes().

- Comments: acpi_bus_scan_crs_csi2() is to be called on the namespace
  root.

- Use size_t for this_count variable in acpi_bus_scan_crs_csi2().

- Revert the NEXT_PROPERTY() macro condition to original (the suggested
  one was different).

- Remove u union in init_port_csi2_common().

- Fix val array size in init_port_csi2_common(). This issue was masked by
  the presence of a u32 field in the union.

- Use "-" for copyright year range.

since v2:

- Unwrap a few lines.

- Copy CSI-2 resource source string using strscpy() instead of memcpy() in
  scan_check_crs_csi2_instance.

- Fix GRAPH_PORT_NAME() sanity check bug introduced in v2.

- Fix snprintf() return value check for port node name in
  get_mipi_port_handle().

- Fix mipi-img-lane-polarities reading.

- Cast bit value to bool instead of using ... ? 1U : 0U.

- Get primary fwnode using acpi_fwnode_handle().

- Don't use MIPI_IMG_PREFIX in the array of renamed properties.

- Use tabs for indenting drivers/acpi/property.c authors.

- Add a comment on assigning ACPI device's secondary fwnode and assign
  ACPI device's secondary fwnode straight to NULL when unassigning it.

since v1:

- Update copyright notices.

- Include linux/types.h instead of linux/kernel.h in drivers/acpi/mipi.c.

- Use SWNODE_GRAPH_PORT_NAME_FMT instead of plain "port@%u" in
  GRAPH_PORT_NAME macro.

- Make the condition in NEXT_PROPERTY() macro easier to read.

- Unwrap lines to make them moderately longer than 80 characters.

- Use * BITS_PER_TYPE(u8) instead of << 3 to convert bytes to bits in
  init_port_csi2_common().

- Test ACPI framework call success using ACPI_SUCCESS() instead of
  comparing with AE_OK. Likewise for ACPI_FAILURE and != AE_OK.

- Use newly added SOFTWARE_NODE() macro to construct the root software
  node.

- Use str_has_prefix() to test for a string prefix instead of memcmp().

- Add pr_fmt() macro to drivers/acpi/property.c.

- Move logical or operators to the end of the line in
  acpi_properties_prepare().

- Improve bad node type error in acpi_parse_string_ref().

Sakari Ailus (8):
  ACPI: property: Parse data node string references in properties
  ACPI: property: Parse _CRS CSI-2 descriptor
  device property: Add SOFTWARE_NODE() macro for defining software nodes
  ACPI: property: Generate camera swnodes for ACPI and DisCo for Imaging
  ACPI: property: Dig "rotation" property for devices with CSI2 _CRS
  ACPI: property: Rename parsed MIPI DisCo for Imaging properties
  ACPI: property: Skip MIPI property table without "mipi-img" prefix
  ACPI: property: Document _CRS CSI-2 and DisCo for Imaging support

 drivers/acpi/Makefile    |   2 +-
 drivers/acpi/internal.h  |   9 +
 drivers/acpi/mipi.c      | 806 +++++++++++++++++++++++++++++++++++++++
 drivers/acpi/property.c  | 132 +++++--
 drivers/acpi/scan.c      |  35 +-
 include/acpi/acpi_bus.h  |  82 ++++
 include/linux/property.h |   7 +
 7 files changed, 1042 insertions(+), 31 deletions(-)
 create mode 100644 drivers/acpi/mipi.c

-- 
2.30.2


Sakari Ailus (8):
  ACPI: property: Parse data node string references in properties
  ACPI: property: Parse _CRS CSI-2 descriptor
  device property: Add SOFTWARE_NODE() macro for defining software nodes
  ACPI: property: Generate camera swnodes for ACPI and DisCo for Imaging
  ACPI: property: Dig "rotation" property for devices with CSI2 _CRS
  ACPI: property: Rename parsed MIPI DisCo for Imaging properties
  ACPI: property: Skip MIPI property table without "mipi-img" prefix
  ACPI: property: Document _CRS CSI-2 and DisCo for Imaging support

 drivers/acpi/Makefile    |   2 +-
 drivers/acpi/internal.h  |   9 +
 drivers/acpi/mipi.c      | 809 +++++++++++++++++++++++++++++++++++++++
 drivers/acpi/property.c  | 132 +++++--
 drivers/acpi/scan.c      |  35 +-
 include/acpi/acpi_bus.h  |  82 ++++
 include/linux/property.h |   7 +
 7 files changed, 1045 insertions(+), 31 deletions(-)
 create mode 100644 drivers/acpi/mipi.c

-- 
2.30.2


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

* [PATCH v5 1/8] ACPI: property: Parse data node string references in properties
  2023-02-08 21:27 [PATCH v5 0/8] ACPI _CRS CSI-2 and MIPI DisCo for Imaging support Sakari Ailus
@ 2023-02-08 21:27 ` Sakari Ailus
  2023-03-07 13:30   ` Rafael J. Wysocki
  2023-02-08 21:27 ` [PATCH v5 2/8] ACPI: property: Parse _CRS CSI-2 descriptor Sakari Ailus
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Sakari Ailus @ 2023-02-08 21:27 UTC (permalink / raw)
  To: linux-acpi; +Cc: linux-media, rafael, andriy.shevchenko, heikki.krogerus

Add support for parsing property references using strings, besides
reference objects that were previously supported. This allows also
referencing data nodes which was not possible with reference objects.

Also add pr_fmt() macro to prefix printouts.

While at it, update copyright.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/property.c | 110 ++++++++++++++++++++++++++++++++++------
 1 file changed, 94 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index b8d9eb9a433ed..4db7701aeaf5c 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -2,14 +2,17 @@
 /*
  * ACPI device specific properties support.
  *
- * Copyright (C) 2014, Intel Corporation
+ * Copyright (C) 2014-2023, Intel Corporation
  * All rights reserved.
  *
  * Authors: Mika Westerberg <mika.westerberg@linux.intel.com>
- *          Darren Hart <dvhart@linux.intel.com>
- *          Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+ *	    Darren Hart <dvhart@linux.intel.com>
+ *	    Rafael J. Wysocki <rafael.j.wysocki@intel.com>
+ *	    Sakari Ailus <sakari.ailus@linux.intel.com>
  */
 
+#define pr_fmt(fmt) "ACPI: " fmt
+
 #include <linux/acpi.h>
 #include <linux/device.h>
 #include <linux/export.h>
@@ -795,7 +798,8 @@ acpi_fwnode_get_named_child_node(const struct fwnode_handle *fwnode,
 static int acpi_get_ref_args(struct fwnode_reference_args *args,
 			     struct fwnode_handle *ref_fwnode,
 			     const union acpi_object **element,
-			     const union acpi_object *end, size_t num_args)
+			     const union acpi_object *end, size_t num_args,
+			     bool subnode_string)
 {
 	u32 nargs = 0, i;
 
@@ -803,13 +807,16 @@ static int acpi_get_ref_args(struct fwnode_reference_args *args,
 	 * Find the referred data extension node under the
 	 * referred device node.
 	 */
-	for (; *element < end && (*element)->type == ACPI_TYPE_STRING;
-	     (*element)++) {
-		const char *child_name = (*element)->string.pointer;
-
-		ref_fwnode = acpi_fwnode_get_named_child_node(ref_fwnode, child_name);
-		if (!ref_fwnode)
-			return -EINVAL;
+	if (subnode_string) {
+		for (; *element < end && (*element)->type == ACPI_TYPE_STRING;
+		     (*element)++) {
+			const char *child_name = (*element)->string.pointer;
+
+			ref_fwnode = acpi_fwnode_get_named_child_node(ref_fwnode,
+								      child_name);
+			if (!ref_fwnode)
+				return -EINVAL;
+		}
 	}
 
 	/*
@@ -820,7 +827,8 @@ static int acpi_get_ref_args(struct fwnode_reference_args *args,
 	for (i = 0; (*element) + i < end && i < num_args; i++) {
 		acpi_object_type type = (*element)[i].type;
 
-		if (type == ACPI_TYPE_LOCAL_REFERENCE)
+		if (type == ACPI_TYPE_LOCAL_REFERENCE ||
+		    (!subnode_string && type == ACPI_TYPE_STRING))
 			break;
 
 		if (type == ACPI_TYPE_INTEGER)
@@ -844,6 +852,43 @@ static int acpi_get_ref_args(struct fwnode_reference_args *args,
 	return 0;
 }
 
+static struct fwnode_handle *
+acpi_parse_string_ref(const struct fwnode_handle *fwnode, const char *refstring)
+{
+	acpi_handle scope, handle;
+	struct acpi_data_node *dn;
+	struct acpi_device *device;
+	acpi_status status;
+
+	if (is_acpi_device_node(fwnode)) {
+		scope = to_acpi_device_node(fwnode)->handle;
+	} else if (is_acpi_data_node(fwnode)) {
+		scope = to_acpi_data_node(fwnode)->handle;
+	} else {
+		pr_err("bad node type for node %pfw\n", fwnode);
+		return ERR_PTR(-EINVAL);
+	}
+
+	status = acpi_get_handle(scope, refstring, &handle);
+	if (ACPI_FAILURE(status)) {
+		acpi_handle_debug(scope, "can't get handle for %s", refstring);
+		return ERR_PTR(-EINVAL);
+	}
+
+	device = acpi_fetch_acpi_dev(handle);
+	if (device)
+		return acpi_fwnode_handle(device);
+
+	status = acpi_get_data_full(handle, acpi_nondev_subnode_tag,
+				    (void **)&dn, NULL);
+	if (ACPI_FAILURE(status) || !dn) {
+		acpi_handle_debug(handle, "can't find subnode");
+		return ERR_PTR(-EINVAL);
+	}
+
+	return &dn->fwnode;
+}
+
 /**
  * __acpi_node_get_property_reference - returns handle to the referenced object
  * @fwnode: Firmware node to get the property from
@@ -886,6 +931,7 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 	const union acpi_object *element, *end;
 	const union acpi_object *obj;
 	const struct acpi_device_data *data;
+	struct fwnode_handle *ref_fwnode;
 	struct acpi_device *device;
 	int ret, idx = 0;
 
@@ -909,16 +955,29 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 
 		args->fwnode = acpi_fwnode_handle(device);
 		args->nargs = 0;
+		return 0;
+	case ACPI_TYPE_STRING:
+		if (index)
+			return -ENOENT;
+
+		ref_fwnode = acpi_parse_string_ref(fwnode, obj->string.pointer);
+		if (IS_ERR(ref_fwnode))
+			return PTR_ERR(ref_fwnode);
+
+		args->fwnode = ref_fwnode;
+		args->nargs = 0;
+
 		return 0;
 	case ACPI_TYPE_PACKAGE:
 		/*
 		 * If it is not a single reference, then it is a package of
-		 * references followed by number of ints as follows:
+		 * references, followed by number of ints as follows:
 		 *
 		 *  Package () { REF, INT, REF, INT, INT }
 		 *
-		 * The index argument is then used to determine which reference
-		 * the caller wants (along with the arguments).
+		 * Here, REF may be either a local reference or a string. The
+		 * index argument is then used to determine which reference the
+		 * caller wants (along with the arguments).
 		 */
 		break;
 	default:
@@ -942,7 +1001,26 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
 
 			ret = acpi_get_ref_args(idx == index ? args : NULL,
 						acpi_fwnode_handle(device),
-						&element, end, num_args);
+						&element, end, num_args, true);
+			if (ret < 0)
+				return ret;
+
+			if (idx == index)
+				return 0;
+
+			break;
+		case ACPI_TYPE_STRING:
+			ref_fwnode =
+				acpi_parse_string_ref(fwnode,
+						      element->string.pointer);
+			if (IS_ERR(ref_fwnode))
+				return PTR_ERR(ref_fwnode);
+
+			element++;
+
+			ret = acpi_get_ref_args(idx == index ? args : NULL,
+						ref_fwnode, &element, end,
+						num_args, false);
 			if (ret < 0)
 				return ret;
 
-- 
2.30.2


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

* [PATCH v5 2/8] ACPI: property: Parse _CRS CSI-2 descriptor
  2023-02-08 21:27 [PATCH v5 0/8] ACPI _CRS CSI-2 and MIPI DisCo for Imaging support Sakari Ailus
  2023-02-08 21:27 ` [PATCH v5 1/8] ACPI: property: Parse data node string references in properties Sakari Ailus
@ 2023-02-08 21:27 ` Sakari Ailus
  2023-02-09  9:47   ` Dan Carpenter
  2023-03-07 13:38   ` Rafael J. Wysocki
  2023-02-08 21:27 ` [PATCH v5 3/8] device property: Add SOFTWARE_NODE() macro for defining software nodes Sakari Ailus
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 16+ messages in thread
From: Sakari Ailus @ 2023-02-08 21:27 UTC (permalink / raw)
  To: linux-acpi; +Cc: linux-media, rafael, andriy.shevchenko, heikki.krogerus

Parse newly added ACPI _CRS CSI-2 descriptor for CSI-2 and camera
configuration. For now, only figure out where the descriptor is present in
order to allow adding information from it to related devices.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/Makefile   |   2 +-
 drivers/acpi/internal.h |   7 +
 drivers/acpi/mipi.c     | 358 ++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/scan.c     |  16 +-
 include/acpi/acpi_bus.h |  11 ++
 5 files changed, 389 insertions(+), 5 deletions(-)
 create mode 100644 drivers/acpi/mipi.c

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index a5b649e71ab1b..a98fa1bc15548 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -37,7 +37,7 @@ acpi-$(CONFIG_ACPI_SLEEP)	+= proc.o
 # ACPI Bus and Device Drivers
 #
 acpi-y				+= bus.o glue.o
-acpi-y				+= scan.o
+acpi-y				+= scan.o mipi.o
 acpi-y				+= resource.o
 acpi-y				+= acpi_processor.o
 acpi-y				+= processor_core.o
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index ec584442fb298..1ec4aa92bf17a 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -282,4 +282,11 @@ void acpi_init_lpit(void);
 static inline void acpi_init_lpit(void) { }
 #endif
 
+/*--------------------------------------------------------------------------
+				ACPI and MIPI DisCo for Imaging conversion
+  -------------------------------------------------------------------------- */
+
+void acpi_crs_csi2_swnodes_del_free(void);
+void acpi_bus_scan_crs_csi2(acpi_handle handle);
+
 #endif /* _ACPI_INTERNAL_H_ */
diff --git a/drivers/acpi/mipi.c b/drivers/acpi/mipi.c
new file mode 100644
index 0000000000000..33d50df831933
--- /dev/null
+++ b/drivers/acpi/mipi.c
@@ -0,0 +1,358 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MIPI DisCo for Imaging support.
+ *
+ * Copyright (C) 2023 Intel Corporation
+ */
+
+#include <linux/acpi.h>
+#include <linux/limits.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/overflow.h>
+#include <linux/types.h>
+#include <linux/slab.h>
+#include <linux/sort.h>
+#include <linux/string.h>
+
+#include "internal.h"
+
+/* Temporary ACPI device handle to software node data structure mapping */
+struct crs_csi2_swnodes {
+	struct list_head list;
+	acpi_handle handle;
+	struct acpi_device_software_nodes *ads;
+};
+
+static LIST_HEAD(crs_csi2_swnodes);
+
+static void crs_csi2_swnode_del_free(struct crs_csi2_swnodes *swnodes)
+{
+	list_del(&swnodes->list);
+	kfree(swnodes);
+}
+
+void acpi_crs_csi2_swnodes_del_free(void)
+{
+	struct crs_csi2_swnodes *swnodes, *swnodes_tmp;
+
+	list_for_each_entry_safe(swnodes, swnodes_tmp, &crs_csi2_swnodes, list)
+		crs_csi2_swnode_del_free(swnodes);
+}
+
+/*
+ * Description of a _CRS CSI2 resource descriptor before software node creation
+ */
+struct crs_csi2_instance {
+	struct list_head list;
+	struct acpi_resource_csi2_serialbus csi2;
+	acpi_handle remote_handle;
+	char remote_name[];
+};
+
+/* Temporary list of ACPI device nodes with _CRS CSI2 resource descriptors */
+struct crs_csi2 {
+	struct list_head list;
+	acpi_handle handle;
+	struct list_head buses;
+};
+
+/*
+ * Context for collecting _CRS CSI2 resource descriptors during ACPI namespace
+ * walk
+ */
+struct scan_check_crs_csi2_context {
+	struct list_head res_head;
+	acpi_handle handle;
+	size_t handle_count;
+};
+
+/* Context for scanning ACPI device nodes for _CRS CSI2 resource descriptors */
+struct crs_csi2_all {
+	struct list_head crs_csi2_head;
+	size_t handle_count;
+};
+
+/* Scan a single CSI2 resource descriptor in _CRS. */
+static acpi_status scan_check_crs_csi2_instance(struct acpi_resource *res,
+						void *context)
+{
+	struct scan_check_crs_csi2_context *inst_context = context;
+	struct acpi_resource_csi2_serialbus *csi2;
+	struct crs_csi2_instance *inst;
+	acpi_handle remote_handle;
+
+	if (res->type != ACPI_RESOURCE_TYPE_SERIAL_BUS)
+		return AE_OK;
+
+	csi2 = &res->data.csi2_serial_bus;
+
+	if (csi2->type != ACPI_RESOURCE_SERIAL_TYPE_CSI2)
+		return AE_OK;
+
+	if (!csi2->resource_source.string_length) {
+		acpi_handle_debug(inst_context->handle,
+				  "invalid resource source string length\n");
+		return AE_OK;
+	}
+
+	if (ACPI_FAILURE(acpi_get_handle(NULL, csi2->resource_source.string_ptr,
+					 &remote_handle))) {
+		acpi_handle_warn(inst_context->handle,
+				 "cannot get handle for %s\n",
+				 csi2->resource_source.string_ptr);
+		return AE_OK;
+	}
+
+	/*
+	 * Allocate space to store the _CRS CSI2 entry and its data and add it
+	 * to the list.
+	 */
+	inst = kmalloc(struct_size(inst, remote_name, csi2->resource_source.string_length),
+		       GFP_KERNEL);
+	if (!inst)
+		return AE_OK;
+
+	inst->csi2 = *csi2;
+	strscpy(inst->remote_name, csi2->resource_source.string_ptr,
+		csi2->resource_source.string_length);
+	inst->csi2.resource_source.string_ptr = inst->remote_name;
+	inst->remote_handle = remote_handle;
+
+	list_add(&inst->list, &inst_context->res_head);
+
+	inst_context->handle_count++;
+
+	return AE_OK;
+}
+
+/*
+ * Find all devices with _CRS CSI2 resource descriptors and collect them
+ * into a list for later use.
+ */
+static acpi_status scan_check_crs_csi2(acpi_handle handle, u32 nesting_level,
+				       void *context, void **ret)
+{
+	struct scan_check_crs_csi2_context inst_context = {
+		.handle = handle,
+		.res_head = LIST_HEAD_INIT(inst_context.res_head),
+	};
+	struct crs_csi2_all *csi2_all = context;
+	struct crs_csi2 *csi2;
+
+	acpi_walk_resources(handle, METHOD_NAME__CRS,
+			    scan_check_crs_csi2_instance, &inst_context);
+
+	if (list_empty(&inst_context.res_head))
+		return AE_OK;
+
+	/*
+	 * Found entry, so allocate memory for it, fill it and add it to the
+	 * list.
+	 */
+	csi2 = kmalloc(sizeof(*csi2), GFP_KERNEL);
+	if (!csi2)
+		return AE_OK;
+
+	csi2->handle = handle;
+	list_replace(&inst_context.res_head, &csi2->buses);
+	list_add(&csi2->list, &csi2_all->crs_csi2_head);
+
+	/* This handle plus remote handles in _CRS CSI2 resource descriptors */
+	csi2_all->handle_count += 1 + inst_context.handle_count;
+
+	return AE_OK;
+}
+
+struct acpi_handle_ref {
+	acpi_handle handle;
+	unsigned int count;
+};
+
+#define NO_CSI2_PORT (UINT_MAX - 1)
+
+static int crs_handle_cmp(const void *__a, const void *__b)
+{
+	const struct acpi_handle_ref *a = __a, *b = __b;
+
+	return a->handle < b->handle ? -1 : a->handle > b->handle;
+}
+
+/*
+ * Release entries in temporary storage of ACPI device nodes with _CRS CSI2
+ * resource descriptors.
+ */
+static void crs_csi2_release(struct list_head *crs_csi2_handles)
+{
+	struct crs_csi2 *csi2, *csi2_tmp;
+
+	list_for_each_entry_safe(csi2, csi2_tmp, crs_csi2_handles, list) {
+		struct crs_csi2_instance *inst, *inst_tmp;
+
+		list_for_each_entry_safe(inst, inst_tmp, &csi2->buses, list) {
+			list_del(&inst->list);
+			kfree(inst);
+		}
+
+		list_del(&csi2->list);
+		kfree(csi2);
+	}
+}
+
+/*
+ * Allocate memory and set up software nodes for an ACPI device with given
+ * number of CSI-2 ports.
+ */
+void acpi_crs_csi2_alloc_fill_swnodes(size_t ports_count, acpi_handle handle)
+{
+	struct acpi_device_software_nodes *ads;
+	struct crs_csi2_swnodes *swnodes;
+	size_t alloc_size;
+	unsigned int i;
+	bool overflow;
+	void *end;
+
+	/*
+	 * Allocate memory for ports, node pointers (number of nodes +
+	 * 1 (guardian), nodes (root + number of ports * 2 (for for
+	 * every port there is an endpoint)).
+	 */
+	overflow = check_mul_overflow(sizeof(*ads->ports) +
+				      sizeof(*ads->nodes) * 2 +
+				      sizeof(*ads->nodeptrs) * 2,
+				      ports_count, &alloc_size);
+	overflow = overflow ||
+		   check_add_overflow(sizeof(*ads) + sizeof(*ads->nodes) +
+				      sizeof(*ads->nodeptrs) * 2,
+				      alloc_size, &alloc_size);
+	if (overflow) {
+		acpi_handle_warn(handle,
+				 "too many _CRS CSI2 resource handles (%zu)",
+				 ports_count);
+		return;
+	}
+
+	swnodes = kzalloc(sizeof(*swnodes), GFP_KERNEL);
+	ads = kzalloc(alloc_size, GFP_KERNEL);
+	ads->ports = (void *)(ads + 1);
+	ads->nodes = (void *)(ads->ports + ports_count);
+	ads->nodeptrs = (void *)(ads->nodes + ports_count * 2 + 1);
+	end = ads->nodeptrs + ports_count * 2 + 2;
+	if (!swnodes || !ads || WARN_ON((void *)ads + alloc_size != end)) {
+		kfree(swnodes);
+		kfree(ads);
+		acpi_handle_debug(handle,
+				  "cannot allocate for %zu software nodes\n",
+				  ports_count);
+		return;
+	}
+
+	ads->num_ports = ports_count;
+	for (i = 0; i < ports_count * 2 + 1; i++)
+		ads->nodeptrs[i] = &ads->nodes[i];
+	ads->nodeptrs[i] = NULL;
+	for (i = 0; i < ports_count; i++)
+		ads->ports[i].port_nr = NO_CSI2_PORT;
+	swnodes->handle = handle;
+	swnodes->ads = ads;
+	list_add(&swnodes->list, &crs_csi2_swnodes);
+}
+
+/**
+ * acpi_bus_scan_crs_csi2 - Scan a device and its child devices for _CRS CSI-2
+ *
+ * @handle: Root of the ACPI Namespace branch to scan
+ *
+ * This function does a number of things:
+ *
+ * 1. Iteratively scan all ACPI device nodes starting from a handle for _CRS
+ *    CSI-2 instances. The instances are stored for later use.
+ *
+ * 2. Count how many references to other devices _CRS CSI-2 instances have in
+ *    total.
+ *
+ * 3. Count the number of references to other devices for each _CRS CSI-2
+ *    instance.
+ *
+ * 4. Allocate memory for swnodes each ACPI device requires later on, and
+ *    generate a list of such allocations.
+ *
+ * Note that struct acpi_device isn't available yet at this time.
+ *
+ * acpi_scan_lock in scan.c must be held when calling this function.
+ */
+void acpi_bus_scan_crs_csi2(acpi_handle handle)
+{
+	struct acpi_handle_ref *handle_refs;
+	struct acpi_handle_ref *this = NULL;
+	struct crs_csi2_all csi2_all = {
+		.crs_csi2_head = LIST_HEAD_INIT(csi2_all.crs_csi2_head),
+	};
+	size_t this_count;
+	unsigned int curr = 0;
+	struct crs_csi2 *csi2;
+
+	/*
+	 * Collect the devices that have a _CRS CSI-2 resource. Each CSI-2 TX
+	 * port has one.
+	 */
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, ACPI_UINT32_MAX,
+			    scan_check_crs_csi2, NULL, &csi2_all, NULL);
+
+	/* No handles? Bail out here. */
+	if (!csi2_all.handle_count)
+		return;
+
+	handle_refs = kcalloc(csi2_all.handle_count + 1, sizeof(*handle_refs),
+			      GFP_KERNEL);
+	if (!handle_refs) {
+		acpi_handle_debug(handle, "no memory for %zu handle refs\n",
+				  csi2_all.handle_count + 1);
+		return;
+	}
+
+	/* Associate handles to the number of references. */
+	list_for_each_entry(csi2, &csi2_all.crs_csi2_head, list) {
+		struct crs_csi2_instance *inst;
+		struct acpi_handle_ref *handle_ref;
+
+		handle_ref = &handle_refs[curr++];
+		handle_ref->handle = csi2->handle;
+
+		list_for_each_entry(inst, &csi2->buses, list) {
+			handle_refs[curr].handle = inst->remote_handle;
+			handle_refs[curr].count = 1;
+			handle_ref->count++;
+			curr++;
+		}
+	}
+
+	handle_refs[curr].handle = NULL;
+
+	/* Sort the handles by remote so duplicates canbe easily found. */
+	sort(handle_refs, csi2_all.handle_count, sizeof(*handle_refs), crs_handle_cmp, NULL);
+
+	/*
+	 * Finally count references in each handle, allocate space for device
+	 * specific ports, properties and fill the _CRS CSI2 descriptor
+	 * originated data.
+	 */
+	this = handle_refs;
+	this_count = this->count;
+	for (curr = 1; curr < csi2_all.handle_count + 1; curr++) {
+		/* Weed out duplicate receiver devices. */
+		if (this->handle == handle_refs[curr].handle) {
+			this_count += handle_refs[curr].count;
+			continue;
+		}
+
+		acpi_crs_csi2_alloc_fill_swnodes(this_count, this->handle);
+
+		this = &handle_refs[curr];
+		this_count = this->count;
+	}
+
+	kfree(handle_refs);
+
+	crs_csi2_release(&csi2_all.crs_csi2_head);
+}
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 0c6f06abe3f47..50de874b8f208 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2423,9 +2423,12 @@ EXPORT_SYMBOL_GPL(acpi_dev_get_next_consumer_dev);
 int acpi_bus_scan(acpi_handle handle)
 {
 	struct acpi_device *device = NULL;
+	int ret = 0;
 
 	acpi_bus_scan_second_pass = false;
 
+	acpi_bus_scan_crs_csi2(handle);
+
 	/* Pass 1: Avoid enumerating devices with missing dependencies. */
 
 	if (ACPI_SUCCESS(acpi_bus_check_add(handle, true, &device)))
@@ -2433,13 +2436,15 @@ int acpi_bus_scan(acpi_handle handle)
 				    acpi_bus_check_add_1, NULL, NULL,
 				    (void **)&device);
 
-	if (!device)
-		return -ENODEV;
+	if (!device) {
+		ret = -ENODEV;
+		goto out_release;
+	}
 
 	acpi_bus_attach(device, (void *)true);
 
 	if (!acpi_bus_scan_second_pass)
-		return 0;
+		goto out_release;
 
 	/* Pass 2: Enumerate all of the remaining devices. */
 
@@ -2452,7 +2457,10 @@ int acpi_bus_scan(acpi_handle handle)
 
 	acpi_bus_attach(device, NULL);
 
-	return 0;
+out_release:
+	acpi_crs_csi2_swnodes_del_free();
+
+	return ret;
 }
 EXPORT_SYMBOL(acpi_bus_scan);
 
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index e44be31115a67..a05fe22c1175c 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -360,6 +360,17 @@ struct acpi_device_data {
 
 struct acpi_gpio_mapping;
 
+struct acpi_device_software_node_port {
+	unsigned int port_nr;
+};
+
+struct acpi_device_software_nodes {
+	struct acpi_device_software_node_port *ports;
+	struct software_node *nodes;
+	const struct software_node **nodeptrs;
+	unsigned int num_ports;
+};
+
 /* Device */
 struct acpi_device {
 	u32 pld_crc;
-- 
2.30.2


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

* [PATCH v5 3/8] device property: Add SOFTWARE_NODE() macro for defining software nodes
  2023-02-08 21:27 [PATCH v5 0/8] ACPI _CRS CSI-2 and MIPI DisCo for Imaging support Sakari Ailus
  2023-02-08 21:27 ` [PATCH v5 1/8] ACPI: property: Parse data node string references in properties Sakari Ailus
  2023-02-08 21:27 ` [PATCH v5 2/8] ACPI: property: Parse _CRS CSI-2 descriptor Sakari Ailus
@ 2023-02-08 21:27 ` Sakari Ailus
  2023-02-08 21:27 ` [PATCH v5 4/8] ACPI: property: Generate camera swnodes for ACPI and DisCo for Imaging Sakari Ailus
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Sakari Ailus @ 2023-02-08 21:27 UTC (permalink / raw)
  To: linux-acpi; +Cc: linux-media, rafael, andriy.shevchenko, heikki.krogerus

Add SOFTWARE_NODE() macro in order to make defining software nodes look
nicer. This is analogous to different PROPERTY_ENTRY_*() macros for
defining properties.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 include/linux/property.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/property.h b/include/linux/property.h
index 37179e3abad5c..6745a86bc9b97 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -477,6 +477,13 @@ struct software_node {
 	const struct property_entry *properties;
 };
 
+#define SOFTWARE_NODE(_name_, _properties_, _parent_)	\
+	(struct software_node) {			\
+		.name = _name_,				\
+		.properties = _properties_,		\
+		.parent = _parent_,			\
+	}
+
 bool is_software_node(const struct fwnode_handle *fwnode);
 const struct software_node *
 to_software_node(const struct fwnode_handle *fwnode);
-- 
2.30.2


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

* [PATCH v5 4/8] ACPI: property: Generate camera swnodes for ACPI and DisCo for Imaging
  2023-02-08 21:27 [PATCH v5 0/8] ACPI _CRS CSI-2 and MIPI DisCo for Imaging support Sakari Ailus
                   ` (2 preceding siblings ...)
  2023-02-08 21:27 ` [PATCH v5 3/8] device property: Add SOFTWARE_NODE() macro for defining software nodes Sakari Ailus
@ 2023-02-08 21:27 ` Sakari Ailus
  2023-02-08 21:27 ` [PATCH v5 5/8] ACPI: property: Dig "rotation" property for devices with CSI2 _CRS Sakari Ailus
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Sakari Ailus @ 2023-02-08 21:27 UTC (permalink / raw)
  To: linux-acpi; +Cc: linux-media, rafael, andriy.shevchenko, heikki.krogerus

Generate software nodes for information parsed from ACPI _CRS for CSI-2 as
well as MIPI DisCo for Imaging spec. The software nodes are compliant with
existing ACPI or DT definitions and are parsed by relevant drivers without
changes.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/internal.h |   1 +
 drivers/acpi/mipi.c     | 356 ++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/scan.c     |  19 +++
 include/acpi/acpi_bus.h |  70 ++++++++
 4 files changed, 446 insertions(+)

diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 1ec4aa92bf17a..fac87404e294c 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -288,5 +288,6 @@ static inline void acpi_init_lpit(void) { }
 
 void acpi_crs_csi2_swnodes_del_free(void);
 void acpi_bus_scan_crs_csi2(acpi_handle handle);
+void acpi_init_swnodes(struct acpi_device *device);
 
 #endif /* _ACPI_INTERNAL_H_ */
diff --git a/drivers/acpi/mipi.c b/drivers/acpi/mipi.c
index 33d50df831933..f81a3bb860609 100644
--- a/drivers/acpi/mipi.c
+++ b/drivers/acpi/mipi.c
@@ -15,6 +15,8 @@
 #include <linux/sort.h>
 #include <linux/string.h>
 
+#include <media/v4l2-fwnode.h>
+
 #include "internal.h"
 
 /* Temporary ACPI device handle to software node data structure mapping */
@@ -26,6 +28,18 @@ struct crs_csi2_swnodes {
 
 static LIST_HEAD(crs_csi2_swnodes);
 
+/* Obtain pre-allocated software nodes for an ACPI device handle */
+static struct acpi_device_software_nodes *crs_csi2_swnode_get(acpi_handle handle)
+{
+	struct crs_csi2_swnodes *swnodes;
+
+	list_for_each_entry(swnodes, &crs_csi2_swnodes, list)
+		if (swnodes->handle == handle)
+			return swnodes->ads;
+
+	return NULL;
+}
+
 static void crs_csi2_swnode_del_free(struct crs_csi2_swnodes *swnodes)
 {
 	list_del(&swnodes->list);
@@ -171,6 +185,35 @@ struct acpi_handle_ref {
 
 #define NO_CSI2_PORT (UINT_MAX - 1)
 
+/*
+ * Return next free entry in ports array of a software nodes related to an ACPI
+ * device.
+ */
+static unsigned int next_csi2_port_index(struct acpi_device_software_nodes *ads,
+					 unsigned int port_nr)
+{
+	unsigned int i;
+
+	for (i = 0; i < ads->num_ports; i++) {
+		struct acpi_device_software_node_port *port = &ads->ports[i];
+
+		if (port->port_nr == port_nr)
+			return i;
+
+		if (port->port_nr == NO_CSI2_PORT) {
+			port->port_nr = port_nr;
+			return i;
+		}
+	}
+
+	return NO_CSI2_PORT;
+}
+
+/* Print graph port name into a buffer, return non-zero if failed. */
+#define GRAPH_PORT_NAME(var, num)					    \
+	(snprintf((var), sizeof(var), SWNODE_GRAPH_PORT_NAME_FMT, (num)) >= \
+	 sizeof(var))
+
 static int crs_handle_cmp(const void *__a, const void *__b)
 {
 	const struct acpi_handle_ref *a = __a, *b = __b;
@@ -258,6 +301,9 @@ void acpi_crs_csi2_alloc_fill_swnodes(size_t ports_count, acpi_handle handle)
 	list_add(&swnodes->list, &crs_csi2_swnodes);
 }
 
+#define ACPI_CRS_CSI2_PHY_TYPE_C	0
+#define ACPI_CRS_CSI2_PHY_TYPE_D	1
+
 /**
  * acpi_bus_scan_crs_csi2 - Scan a device and its child devices for _CRS CSI-2
  *
@@ -277,6 +323,8 @@ void acpi_crs_csi2_alloc_fill_swnodes(size_t ports_count, acpi_handle handle)
  * 4. Allocate memory for swnodes each ACPI device requires later on, and
  *    generate a list of such allocations.
  *
+ * 5. Set up properties for software nodes.
+ *
  * Note that struct acpi_device isn't available yet at this time.
  *
  * acpi_scan_lock in scan.c must be held when calling this function.
@@ -352,7 +400,315 @@ void acpi_bus_scan_crs_csi2(acpi_handle handle)
 		this_count = this->count;
 	}
 
+	/*
+	 * Allocate and set up necessary software nodes for each device and set
+	 * up properties from _CRS CSI2 descriptor.
+	 */
+	list_for_each_entry(csi2, &csi2_all.crs_csi2_head, list) {
+		struct acpi_device_software_nodes *local_swnodes;
+		struct crs_csi2_instance *inst;
+
+		local_swnodes = crs_csi2_swnode_get(csi2->handle);
+		if (WARN_ON_ONCE(!local_swnodes))
+			continue;
+
+		list_for_each_entry(inst, &csi2->buses, list) {
+			struct acpi_device_software_nodes *remote_swnodes;
+			struct acpi_device_software_node_port *local_port;
+			struct acpi_device_software_node_port *remote_port;
+			struct software_node *local_node, *remote_node;
+			unsigned int local_index, remote_index;
+			unsigned int bus_type;
+
+			remote_swnodes = crs_csi2_swnode_get(inst->remote_handle);
+			if (WARN_ON_ONCE(!remote_swnodes))
+				continue;
+
+			local_index = next_csi2_port_index(local_swnodes, inst->csi2.local_port_instance);
+			remote_index = next_csi2_port_index(remote_swnodes, inst->csi2.resource_source.index);
+
+			if (WARN_ON_ONCE(local_index >= local_swnodes->num_ports) ||
+			    WARN_ON_ONCE(remote_index >= remote_swnodes->num_ports))
+				goto out_free;
+
+			switch (inst->csi2.phy_type) {
+			case ACPI_CRS_CSI2_PHY_TYPE_C:
+				bus_type = V4L2_FWNODE_BUS_TYPE_CSI2_CPHY;
+				break;
+			case ACPI_CRS_CSI2_PHY_TYPE_D:
+				bus_type = V4L2_FWNODE_BUS_TYPE_CSI2_DPHY;
+				break;
+			default:
+				acpi_handle_info(handle,
+						 "ignoring CSI-2 PHY type %u\n",
+						 inst->csi2.phy_type);
+				continue;
+			}
+
+			local_port = &local_swnodes->ports[local_index];
+			local_node = &local_swnodes->nodes[ACPI_DEVICE_SWNODE_EP(local_index)];
+			local_port->remote_ep_ref[0] = SOFTWARE_NODE_REFERENCE(local_node);
+			local_port->crs_csi2_local = true;
+
+			remote_port = &remote_swnodes->ports[remote_index];
+			remote_node = &remote_swnodes->nodes[ACPI_DEVICE_SWNODE_EP(remote_index)];
+			remote_port->remote_ep_ref[0] = SOFTWARE_NODE_REFERENCE(remote_node);
+
+			local_port->ep_props[ACPI_DEVICE_SWNODE_EP_REMOTE_EP] =
+				PROPERTY_ENTRY_REF_ARRAY("remote-endpoint",
+							 remote_port->remote_ep_ref);
+			local_port->ep_props[ACPI_DEVICE_SWNODE_EP_BUS_TYPE] =
+				PROPERTY_ENTRY_U32("bus-type", bus_type);
+			local_port->ep_props[ACPI_DEVICE_SWNODE_EP_REG] =
+				PROPERTY_ENTRY_U32("reg", 0);
+			local_port->port_props[ACPI_DEVICE_SWNODE_PRT_REG] =
+				PROPERTY_ENTRY_U32("reg", inst->csi2.local_port_instance);
+			if (GRAPH_PORT_NAME(local_port->port_name,
+					    inst->csi2.local_port_instance))
+				acpi_handle_warn(handle,
+						 "name for local port %u too long",
+						 inst->csi2.local_port_instance);
+
+			remote_port->ep_props[ACPI_DEVICE_SWNODE_EP_REMOTE_EP] =
+				PROPERTY_ENTRY_REF_ARRAY("remote-endpoint", local_port->remote_ep_ref);
+			remote_port->ep_props[ACPI_DEVICE_SWNODE_EP_BUS_TYPE] =
+				PROPERTY_ENTRY_U32("bus-type", bus_type);
+			remote_port->ep_props[ACPI_DEVICE_SWNODE_EP_REG] =
+				PROPERTY_ENTRY_U32("reg", 0);
+			remote_port->port_props[ACPI_DEVICE_SWNODE_PRT_REG] =
+				PROPERTY_ENTRY_U32("reg",
+						   inst->csi2.resource_source.index);
+			if (GRAPH_PORT_NAME(remote_port->port_name,
+					    inst->csi2.resource_source.index))
+				acpi_handle_warn(handle,
+						 "name for remote port %u too long",
+						 inst->csi2.resource_source.index);
+		}
+	}
+
+out_free:
 	kfree(handle_refs);
 
 	crs_csi2_release(&csi2_all.crs_csi2_head);
 }
+
+/*
+ * Get the index of the next property in the property array, with a given
+ * maximum values.
+ */
+#define NEXT_PROPERTY(index, max)				\
+	(WARN_ON(++(index) > ACPI_DEVICE_SWNODE_##max + 1) ?	\
+	 ACPI_DEVICE_SWNODE_##max : (index) - 1)
+
+static struct fwnode_handle *get_mipi_port_handle(struct acpi_device *device,
+						  unsigned int port)
+{
+	static const char mipi_port_prefix[] = "mipi-img-port-";
+	char mipi_port_name[sizeof(mipi_port_prefix) + 2];
+
+	if (snprintf(mipi_port_name, sizeof(mipi_port_name), "%s%u",
+		     mipi_port_prefix, port) >= sizeof(mipi_port_name)) {
+		acpi_handle_info(acpi_device_handle(device),
+				 "mipi port name too long for port %u\n", port);
+		return NULL;
+	}
+
+	return fwnode_get_named_child_node(acpi_fwnode_handle(device),
+					   mipi_port_name);
+}
+
+static void init_port_csi2_common(struct acpi_device *device,
+				  struct fwnode_handle *mipi_port_fwnode,
+				  unsigned int *ep_prop_index,
+				  unsigned int port_nr)
+{
+	unsigned int port_index = next_csi2_port_index(device->swnodes, port_nr);
+	struct acpi_device_software_nodes *ads = device->swnodes;
+	struct acpi_device_software_node_port *port = &ads->ports[port_index];
+	unsigned int num_lanes = 0;
+	u8 val[ARRAY_SIZE(port->data_lanes)];
+	int ret;
+
+	*ep_prop_index = ACPI_DEVICE_SWNODE_EP_CLOCK_LANES;
+
+	if (GRAPH_PORT_NAME(port->port_name, port_nr))
+		return;
+
+	ads->nodes[ACPI_DEVICE_SWNODE_PRT(port_index)] =
+		SOFTWARE_NODE(port->port_name, port->port_props,
+			      &ads->nodes[ACPI_DEVICE_SWNODE_ROOT]);
+
+	ret = fwnode_property_read_u8(mipi_port_fwnode, "mipi-img-clock-lane", val);
+	if (!ret) {
+		port->ep_props[NEXT_PROPERTY(*ep_prop_index, EP_CLOCK_LANES)] =
+			PROPERTY_ENTRY_U32("clock-lanes", *val);
+	}
+	ret = fwnode_property_count_u8(mipi_port_fwnode, "mipi-img-data-lanes");
+	if (ret > 0) {
+		num_lanes = ret;
+
+		if (num_lanes > ARRAY_SIZE(port->data_lanes)) {
+			acpi_handle_warn(acpi_device_handle(device),
+					 "too many data lanes (%u)\n",
+					 num_lanes);
+			num_lanes = ARRAY_SIZE(port->data_lanes);
+		}
+
+		ret = fwnode_property_read_u8_array(mipi_port_fwnode, "mipi-img-data-lanes",
+						    val, num_lanes);
+		if (!ret) {
+			unsigned int i;
+
+			for (i = 0; i < num_lanes; i++)
+				port->data_lanes[i] = val[i];
+
+			port->ep_props[NEXT_PROPERTY(*ep_prop_index, EP_DATA_LANES)] =
+				PROPERTY_ENTRY_U32_ARRAY_LEN("data-lanes", port->data_lanes,
+							     num_lanes);
+		}
+	}
+
+	ret = fwnode_property_count_u8(mipi_port_fwnode, "mipi-img-lane-polarities");
+	if (ret > 0) {
+		unsigned int bytes = min_t(unsigned int, ret, sizeof(val));
+
+		fwnode_property_read_u8_array(mipi_port_fwnode,
+					      "mipi-img-lane-polarities",
+					      val, bytes);
+
+		/* Total number of lanes here is clock lane + data lanes */
+		if (bytes * BITS_PER_TYPE(u8) >= 1 + num_lanes) {
+			unsigned int i;
+
+			/* Move polarity bits to the lane polarity u32 array */
+			for (i = 0; i < 1 + num_lanes; i++)
+				port->lane_polarities[i] =
+					(bool)(val[i >> 3] & (1 << (i & 7)));
+
+			port->ep_props[NEXT_PROPERTY(*ep_prop_index, EP_LANE_POLARITIES)] =
+				PROPERTY_ENTRY_U32_ARRAY_LEN("lane-polarities",
+							     port->lane_polarities,
+							     1 + num_lanes);
+		} else {
+			acpi_handle_warn(acpi_device_handle(device),
+					 "too few lane polarity bytes (%u)\n",
+					 bytes);
+		}
+	}
+
+	ads->nodes[ACPI_DEVICE_SWNODE_EP(port_index)] =
+		SOFTWARE_NODE("endpoint@0", ads->ports[port_index].ep_props,
+			      &ads->nodes[ACPI_DEVICE_SWNODE_PRT(port_index)]);
+}
+
+static void init_port_csi2_local(struct acpi_device *device,
+				 unsigned int port_nr)
+{
+	unsigned int port_index = next_csi2_port_index(device->swnodes, port_nr);
+	struct fwnode_handle *mipi_port_fwnode =
+		get_mipi_port_handle(device, port_nr);
+	struct acpi_device_software_node_port *port =
+		&device->swnodes->ports[port_index];
+	unsigned int ep_prop_index;
+	int ret;
+
+	init_port_csi2_common(device, mipi_port_fwnode, &ep_prop_index, port_nr);
+
+	ret = fwnode_property_count_u64(mipi_port_fwnode, "mipi-img-link-frequencies");
+	if (ret > 0) {
+		unsigned int num_link_freqs = ret;
+
+		if (num_link_freqs > ARRAY_SIZE(port->link_frequencies)) {
+			acpi_handle_info(acpi_device_handle(device),
+					 "too many link frequencies %u\n",
+					 num_link_freqs);
+			num_link_freqs = ARRAY_SIZE(port->link_frequencies);
+		}
+
+		ret = fwnode_property_read_u64_array(mipi_port_fwnode,
+						     "mipi-img-link-frequencies",
+						     port->link_frequencies,
+						     num_link_freqs);
+		if (!ret)
+			port->ep_props[NEXT_PROPERTY(ep_prop_index, EP_LINK_FREQUENCIES)] =
+				PROPERTY_ENTRY_U64_ARRAY_LEN("link-frequencies",
+							     port->link_frequencies,
+							     num_link_freqs);
+		else
+			acpi_handle_info(acpi_device_handle(device),
+					 "can't get link frequencies (%d)\n",
+					 ret);
+	}
+
+	fwnode_handle_put(mipi_port_fwnode);
+}
+
+static void init_port_csi2_remote(struct acpi_device *device,
+				  unsigned int port_nr)
+{
+	struct fwnode_handle *mipi_port_fwnode = get_mipi_port_handle(device, port_nr);
+	unsigned int ep_prop_index;
+
+	init_port_csi2_common(device, mipi_port_fwnode, &ep_prop_index, port_nr);
+
+	fwnode_handle_put(mipi_port_fwnode);
+}
+
+/**
+ * acpi_init_swnodes - Set up software nodes for properties gathered elsewhere
+ *
+ * @device: ACPI device for which the software nodes are initialised
+ *
+ * Initialise and register software nodes for properties for which the data is
+ * gathered elsewhere, e.g. _CRS CSI-2 descriptors. The process itself takes
+ * place before this function is called.
+ *
+ * acpi_scan_lock in scan.c must be held when calling this function.
+ */
+void acpi_init_swnodes(struct acpi_device *device)
+{
+	struct acpi_device_software_nodes *ads;
+	struct acpi_buffer buffer = { .length = ACPI_ALLOCATE_BUFFER };
+	acpi_handle handle = acpi_device_handle(device);
+	struct fwnode_handle *primary;
+	acpi_status status;
+	unsigned int i;
+	int ret;
+
+	device->swnodes = ads = crs_csi2_swnode_get(handle);
+	if (!ads)
+		return;
+
+	status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
+	if (ACPI_FAILURE(status)) {
+		acpi_handle_warn(handle, "cannot get path name\n");
+		return;
+	}
+
+	ads->nodes[ACPI_DEVICE_SWNODE_ROOT] =
+		SOFTWARE_NODE(buffer.pointer, ads->dev_props, NULL);
+
+	for (i = 0; i < ads->num_ports; i++) {
+		struct acpi_device_software_node_port *port = &ads->ports[i];
+
+		if (port->crs_csi2_local)
+			init_port_csi2_local(device, port->port_nr);
+		else
+			init_port_csi2_remote(device, port->port_nr);
+	}
+
+	ret = software_node_register_node_group(ads->nodeptrs);
+	if (ret < 0) {
+		acpi_handle_warn(handle,
+				 "cannot register software nodes (%d)!\n", ret);
+		device->swnodes = NULL;
+		return;
+	}
+
+	/*
+	 * Note we can't use set_secondary_fwnode() here as the device's
+	 * primary fwnode hasn't been set yet.
+	 */
+	primary = acpi_fwnode_handle(device);
+	primary->secondary = software_node_fwnode(ads->nodes);
+}
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 50de874b8f208..9cc30c9c87080 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -449,10 +449,28 @@ static void acpi_free_power_resources_lists(struct acpi_device *device)
 	}
 }
 
+static void acpi_free_swnodes(struct acpi_device *device)
+{
+	struct acpi_device_software_nodes *ads = device->swnodes;
+	struct fwnode_handle *primary;
+
+	if (!ads)
+		return;
+
+	software_node_unregister_node_group(ads->nodeptrs);
+	primary = acpi_fwnode_handle(device);
+	primary->secondary = NULL;
+	kfree(ads->nodes[ACPI_DEVICE_SWNODE_ROOT].name);
+	kfree(ads);
+
+	device->swnodes = NULL;
+}
+
 static void acpi_device_release(struct device *dev)
 {
 	struct acpi_device *acpi_dev = to_acpi_device(dev);
 
+	acpi_free_swnodes(acpi_dev);
 	acpi_free_properties(acpi_dev);
 	acpi_free_pnp_ids(&acpi_dev->pnp);
 	acpi_free_power_resources_lists(acpi_dev);
@@ -1771,6 +1789,7 @@ void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
 	acpi_device_get_busid(device);
 	acpi_set_pnp_ids(handle, &device->pnp, type);
 	acpi_init_properties(device);
+	acpi_init_swnodes(device);
 	acpi_bus_get_flags(device);
 	device->flags.match_driver = false;
 	device->flags.initialized = true;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index a05fe22c1175c..e05d1c1f6ac23 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -360,15 +360,84 @@ struct acpi_device_data {
 
 struct acpi_gpio_mapping;
 
+enum acpi_device_swnode_dev_props {
+	ACPI_DEVICE_SWNODE_DEV_NUM_OF,
+	ACPI_DEVICE_SWNODE_DEV_NUM_ENTRIES
+};
+
+enum acpi_device_swnode_port_props {
+	ACPI_DEVICE_SWNODE_PRT_REG,
+	ACPI_DEVICE_SWNODE_PRT_NUM_OF,
+	ACPI_DEVICE_SWNODE_PRT_NUM_ENTRIES
+};
+
+enum acpi_device_swnode_ep_props {
+	ACPI_DEVICE_SWNODE_EP_REMOTE_EP,
+	ACPI_DEVICE_SWNODE_EP_BUS_TYPE,
+	ACPI_DEVICE_SWNODE_EP_REG,
+	ACPI_DEVICE_SWNODE_EP_CLOCK_LANES,
+	ACPI_DEVICE_SWNODE_EP_DATA_LANES,
+	ACPI_DEVICE_SWNODE_EP_LANE_POLARITIES,
+	/* TX only */
+	ACPI_DEVICE_SWNODE_EP_LINK_FREQUENCIES,
+	ACPI_DEVICE_SWNODE_EP_NUM_OF,
+	ACPI_DEVICE_SWNODE_EP_NUM_ENTRIES
+};
+
+#define ACPI_DEVICE_SWNODE_ROOT			0
+/*
+ * Each device has a root swnode plus two times as many nodes as the
+ * number of CSI-2 ports.
+ */
+#define ACPI_DEVICE_SWNODE_PRT(port)		(1 + 2 * (port))
+#define ACPI_DEVICE_SWNODE_EP(endpoint)	\
+	(ACPI_DEVICE_SWNODE_PRT(endpoint) + 1)
+
+#define ACPI_DEVICE_SWNODE_CSI2_DATA_LANES		4
+
+/**
+ * struct acpi_device_software_node_port: Software nodes for MIPI DisCo for
+ *					  Imaging support
+ * @port_name: the name of the port
+ * @data_lanes: "data-lanes" property values
+ * @lane_polarities: "lane-polarities" property values
+ * @link_frequencies: "link_frequencies" property values
+ * @port_nr: the number of the port
+ * @crs_crs2_local: whether the _CRS CSI2 record is local to the port (i.e. the
+ *		    port is a transmitter port)
+ * port_props: the port properties
+ * ep_props: the endpoint properties
+ * remote_ep_ref: reference to the remote endpoint
+ */
 struct acpi_device_software_node_port {
+	char port_name[8];
+	u32 data_lanes[ACPI_DEVICE_SWNODE_CSI2_DATA_LANES];
+	u32 lane_polarities[1 /* clock lane */ +
+			    ACPI_DEVICE_SWNODE_CSI2_DATA_LANES];
+	u64 link_frequencies[4];
 	unsigned int port_nr;
+	bool crs_csi2_local;
+
+	struct property_entry port_props[ACPI_DEVICE_SWNODE_PRT_NUM_ENTRIES];
+	struct property_entry ep_props[ACPI_DEVICE_SWNODE_EP_NUM_ENTRIES];
+
+	struct software_node_ref_args remote_ep_ref[1];
 };
 
+/**
+ * struct acpi_device_software_nodes - Software nodes for an ACPI device
+ * @ports: information related to each port and endpoint within a port
+ * @nodes: software nodes for root as well as ports and endpoints
+ * @nodeprts: array of software node pointers, for (un)registering them
+ * @num_ports: the number of ports
+ */
 struct acpi_device_software_nodes {
 	struct acpi_device_software_node_port *ports;
 	struct software_node *nodes;
 	const struct software_node **nodeptrs;
 	unsigned int num_ports;
+
+	struct property_entry dev_props[ACPI_DEVICE_SWNODE_DEV_NUM_ENTRIES];
 };
 
 /* Device */
@@ -377,6 +446,7 @@ struct acpi_device {
 	int device_type;
 	acpi_handle handle;		/* no handle for fixed hardware */
 	struct fwnode_handle fwnode;
+	struct acpi_device_software_nodes *swnodes;
 	struct list_head wakeup_list;
 	struct list_head del_list;
 	struct acpi_device_status status;
-- 
2.30.2


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

* [PATCH v5 5/8] ACPI: property: Dig "rotation" property for devices with CSI2 _CRS
  2023-02-08 21:27 [PATCH v5 0/8] ACPI _CRS CSI-2 and MIPI DisCo for Imaging support Sakari Ailus
                   ` (3 preceding siblings ...)
  2023-02-08 21:27 ` [PATCH v5 4/8] ACPI: property: Generate camera swnodes for ACPI and DisCo for Imaging Sakari Ailus
@ 2023-02-08 21:27 ` Sakari Ailus
  2023-02-08 21:27 ` [PATCH v5 6/8] ACPI: property: Rename parsed MIPI DisCo for Imaging properties Sakari Ailus
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Sakari Ailus @ 2023-02-08 21:27 UTC (permalink / raw)
  To: linux-acpi; +Cc: linux-media, rafael, andriy.shevchenko, heikki.krogerus

Dig "rotation" property value for devices with _CRS CSI2 resource
descriptor. The value comes from _PLD (physical location of device)
object, if it exists for the device.

This way camera sensor drivers that know the "rotation" property do not
need to care about _PLD on ACPI.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/mipi.c     | 17 +++++++++++++++++
 include/acpi/acpi_bus.h |  1 +
 2 files changed, 18 insertions(+)

diff --git a/drivers/acpi/mipi.c b/drivers/acpi/mipi.c
index f81a3bb860609..08e67163b2b80 100644
--- a/drivers/acpi/mipi.c
+++ b/drivers/acpi/mipi.c
@@ -667,10 +667,12 @@ static void init_port_csi2_remote(struct acpi_device *device,
  */
 void acpi_init_swnodes(struct acpi_device *device)
 {
+	struct fwnode_handle *fwnode = acpi_fwnode_handle(device);
 	struct acpi_device_software_nodes *ads;
 	struct acpi_buffer buffer = { .length = ACPI_ALLOCATE_BUFFER };
 	acpi_handle handle = acpi_device_handle(device);
 	struct fwnode_handle *primary;
+	unsigned int prop_index = 0;
 	acpi_status status;
 	unsigned int i;
 	int ret;
@@ -679,6 +681,21 @@ void acpi_init_swnodes(struct acpi_device *device)
 	if (!ads)
 		return;
 
+	/*
+	 * Check if "rotation" property exists and if it doesn't but there's a
+	 * _PLD object, then get the rotation value from there.
+	 */
+	if (!fwnode_property_present(fwnode, "rotation")) {
+		struct acpi_pld_info *pld;
+
+		status = acpi_get_physical_device_location(handle, &pld);
+		if (ACPI_SUCCESS(status)) {
+			ads->dev_props[NEXT_PROPERTY(prop_index, DEV_ROTATION)] =
+				PROPERTY_ENTRY_U32("rotation", pld->rotation * 45U);
+			kfree(pld);
+		}
+	}
+
 	status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
 	if (ACPI_FAILURE(status)) {
 		acpi_handle_warn(handle, "cannot get path name\n");
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index e05d1c1f6ac23..f73e65a21b894 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -361,6 +361,7 @@ struct acpi_device_data {
 struct acpi_gpio_mapping;
 
 enum acpi_device_swnode_dev_props {
+	ACPI_DEVICE_SWNODE_DEV_ROTATION,
 	ACPI_DEVICE_SWNODE_DEV_NUM_OF,
 	ACPI_DEVICE_SWNODE_DEV_NUM_ENTRIES
 };
-- 
2.30.2


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

* [PATCH v5 6/8] ACPI: property: Rename parsed MIPI DisCo for Imaging properties
  2023-02-08 21:27 [PATCH v5 0/8] ACPI _CRS CSI-2 and MIPI DisCo for Imaging support Sakari Ailus
                   ` (4 preceding siblings ...)
  2023-02-08 21:27 ` [PATCH v5 5/8] ACPI: property: Dig "rotation" property for devices with CSI2 _CRS Sakari Ailus
@ 2023-02-08 21:27 ` Sakari Ailus
  2023-02-08 21:27 ` [PATCH v5 7/8] ACPI: property: Skip MIPI property table without "mipi-img" prefix Sakari Ailus
  2023-02-08 21:27 ` [PATCH v5 8/8] ACPI: property: Document _CRS CSI-2 and DisCo for Imaging support Sakari Ailus
  7 siblings, 0 replies; 16+ messages in thread
From: Sakari Ailus @ 2023-02-08 21:27 UTC (permalink / raw)
  To: linux-acpi; +Cc: linux-media, rafael, andriy.shevchenko, heikki.krogerus

MIPI DisCo for Imaging defines properties for sensor-adjacent devices such
as EEPROM, LED flash or lens VCM as either device or sub-node references.
This is compliant with existing DT definitions apart from property names.

Rename parsed MIPI-defined properties so drivers will have a unified view
of them as defined in DT and already parsed by drivers. This can be done
in-place as the MIPI-defined property strings are always longer than the
DT one. This also results in loss of constness in parser function
arguments.

Individual bindings to devices could define the references differently
between MIPI DisCo for Imaging and DT, in terms of device or sub-node
references. This will still need to be handled in the drivers themselves.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/internal.h |  1 +
 drivers/acpi/mipi.c     | 36 ++++++++++++++++++++++++++++++++++++
 drivers/acpi/property.c | 22 ++++++++++++----------
 3 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index fac87404e294c..f107094bfe16f 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -289,5 +289,6 @@ static inline void acpi_init_lpit(void) { }
 void acpi_crs_csi2_swnodes_del_free(void);
 void acpi_bus_scan_crs_csi2(acpi_handle handle);
 void acpi_init_swnodes(struct acpi_device *device);
+void acpi_properties_prepare_mipi(union acpi_object *elements);
 
 #endif /* _ACPI_INTERNAL_H_ */
diff --git a/drivers/acpi/mipi.c b/drivers/acpi/mipi.c
index 08e67163b2b80..258a2b351dcf3 100644
--- a/drivers/acpi/mipi.c
+++ b/drivers/acpi/mipi.c
@@ -729,3 +729,39 @@ void acpi_init_swnodes(struct acpi_device *device)
 	primary = acpi_fwnode_handle(device);
 	primary->secondary = software_node_fwnode(ads->nodes);
 }
+
+static const struct mipi_disco_prop {
+	const char *mipi_prop;
+	const char *dt_prop;
+} mipi_disco_props[] = {
+	{ "mipi-img-lens-focus",	"lens-focus" },
+	{ "mipi-img-flash-leds",	"flash-leds" },
+	{ "mipi-img-clock-frequency",	"clock-frequency" },
+	{ "mipi-img-led-max-current",	"led-max-microamp" },
+	{ "mipi-img-flash-max-current",	"flash-max-microamp" },
+	{ "mipi-img-flash-max-timeout",	"flash-max-timeout-us" },
+};
+
+/**
+ * acpi_properties_prepare_mipi - Rename MIPI properties as commin DT ones
+ *
+ * @elements: ACPI object containing _DSD properties for a device node
+ *
+ * Renames MIPI-defined properties as common DT ones. The pre-requisite is that
+ * the names of all such MIPI properties are no longer than the corresponding DT
+ * ones.
+ */
+void acpi_properties_prepare_mipi(union acpi_object *elements)
+{
+	char *e0 = elements[0].string.pointer;
+	unsigned int i;
+
+	/* Replace MIPI DisCo for Imaging property names with DT equivalents. */
+	for (i = 0; i < ARRAY_SIZE(mipi_disco_props); i++) {
+		if (!strcmp(mipi_disco_props[i].mipi_prop, e0)) {
+			WARN_ON(strscpy(e0, mipi_disco_props[i].dt_prop,
+					elements[0].string.length) < 0);
+			break;
+		}
+	}
+}
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 4db7701aeaf5c..af9cb849a73ac 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -124,7 +124,7 @@ static bool acpi_nondev_subnode_extract(union acpi_object *desc,
 }
 
 static bool acpi_nondev_subnode_data_ok(acpi_handle handle,
-					const union acpi_object *link,
+					union acpi_object *link,
 					struct list_head *list,
 					struct fwnode_handle *parent)
 {
@@ -145,7 +145,7 @@ static bool acpi_nondev_subnode_data_ok(acpi_handle handle,
 }
 
 static bool acpi_nondev_subnode_ok(acpi_handle scope,
-				   const union acpi_object *link,
+				   union acpi_object *link,
 				   struct list_head *list,
 				   struct fwnode_handle *parent)
 {
@@ -276,22 +276,24 @@ static bool acpi_property_value_ok(const union acpi_object *value)
 	return false;
 }
 
-static bool acpi_properties_format_valid(const union acpi_object *properties)
+static bool acpi_properties_prepare(union acpi_object *properties)
 {
-	int i;
+	unsigned int i;
 
 	for (i = 0; i < properties->package.count; i++) {
-		const union acpi_object *property;
+		union acpi_object *property = &properties->package.elements[i];
+		union acpi_object *elements = property->package.elements;
 
-		property = &properties->package.elements[i];
 		/*
 		 * Only two elements allowed, the first one must be a string and
 		 * the second one has to satisfy certain conditions.
 		 */
-		if (property->package.count != 2
-		    || property->package.elements[0].type != ACPI_TYPE_STRING
-		    || !acpi_property_value_ok(&property->package.elements[1]))
+		if (property->package.count != 2 ||
+		    elements[0].type != ACPI_TYPE_STRING ||
+		    !acpi_property_value_ok(&elements[1]))
 			return false;
+
+		acpi_properties_prepare_mipi(elements);
 	}
 	return true;
 }
@@ -523,7 +525,7 @@ static bool acpi_extract_properties(acpi_handle scope, union acpi_object *desc,
 		 * We found the matching GUID. Now validate the format of the
 		 * package immediately following it.
 		 */
-		if (!acpi_properties_format_valid(properties))
+		if (!acpi_properties_prepare(properties))
 			continue;
 
 		acpi_data_add_props(data, (const guid_t *)guid->buffer.pointer,
-- 
2.30.2


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

* [PATCH v5 7/8] ACPI: property: Skip MIPI property table without "mipi-img" prefix
  2023-02-08 21:27 [PATCH v5 0/8] ACPI _CRS CSI-2 and MIPI DisCo for Imaging support Sakari Ailus
                   ` (5 preceding siblings ...)
  2023-02-08 21:27 ` [PATCH v5 6/8] ACPI: property: Rename parsed MIPI DisCo for Imaging properties Sakari Ailus
@ 2023-02-08 21:27 ` Sakari Ailus
  2023-02-08 21:27 ` [PATCH v5 8/8] ACPI: property: Document _CRS CSI-2 and DisCo for Imaging support Sakari Ailus
  7 siblings, 0 replies; 16+ messages in thread
From: Sakari Ailus @ 2023-02-08 21:27 UTC (permalink / raw)
  To: linux-acpi; +Cc: linux-media, rafael, andriy.shevchenko, heikki.krogerus

For all _DSD properties, skip going through the MIPI DisCo for Imaging
property name substitution table if the property doesn't have "mipi-img-"
prefix.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/mipi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/acpi/mipi.c b/drivers/acpi/mipi.c
index 258a2b351dcf3..62e8f3c6006d7 100644
--- a/drivers/acpi/mipi.c
+++ b/drivers/acpi/mipi.c
@@ -730,6 +730,8 @@ void acpi_init_swnodes(struct acpi_device *device)
 	primary->secondary = software_node_fwnode(ads->nodes);
 }
 
+#define MIPI_IMG_PREFIX "mipi-img-"
+
 static const struct mipi_disco_prop {
 	const char *mipi_prop;
 	const char *dt_prop;
@@ -756,6 +758,9 @@ void acpi_properties_prepare_mipi(union acpi_object *elements)
 	char *e0 = elements[0].string.pointer;
 	unsigned int i;
 
+	if (!str_has_prefix(elements[0].string.pointer, MIPI_IMG_PREFIX))
+		return;
+
 	/* Replace MIPI DisCo for Imaging property names with DT equivalents. */
 	for (i = 0; i < ARRAY_SIZE(mipi_disco_props); i++) {
 		if (!strcmp(mipi_disco_props[i].mipi_prop, e0)) {
-- 
2.30.2


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

* [PATCH v5 8/8] ACPI: property: Document _CRS CSI-2 and DisCo for Imaging support
  2023-02-08 21:27 [PATCH v5 0/8] ACPI _CRS CSI-2 and MIPI DisCo for Imaging support Sakari Ailus
                   ` (6 preceding siblings ...)
  2023-02-08 21:27 ` [PATCH v5 7/8] ACPI: property: Skip MIPI property table without "mipi-img" prefix Sakari Ailus
@ 2023-02-08 21:27 ` Sakari Ailus
  7 siblings, 0 replies; 16+ messages in thread
From: Sakari Ailus @ 2023-02-08 21:27 UTC (permalink / raw)
  To: linux-acpi; +Cc: linux-media, rafael, andriy.shevchenko, heikki.krogerus

Document how ACPI _CRS CSI-2 and DisCo for Imaging works. It's non-trivial
so such documentation can be useful.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/acpi/mipi.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/acpi/mipi.c b/drivers/acpi/mipi.c
index 62e8f3c6006d7..864754776900b 100644
--- a/drivers/acpi/mipi.c
+++ b/drivers/acpi/mipi.c
@@ -3,6 +3,43 @@
  * MIPI DisCo for Imaging support.
  *
  * Copyright (C) 2023 Intel Corporation
+ *
+ * _CRS CSI-2 descriptors, as defined starting from ACPI 6.4 [1], contain
+ * information on cross-device CSI-2 bus configuration. The descriptors are
+ * located under transmitter devices, and the receiver devices have no direct
+ * way to access them even if the information in these descriptors is equally
+ * important for receivers. This information is amended with MIPI DisCo for
+ * Imaging [2] specification that defines _DSD data nodes and properties.
+ *
+ * The support for these is based on two-fold approach, firstly renaming
+ * properties where semantics matches and secondly gathering information to
+ * generate properties using information gathered from various sources. The
+ * former is trivial (see acpi_properties_prepare_mipi() at the end of the
+ * file) whereas the latter requires a more elaborate explanation.
+ *
+ * acpi_bus_scan_crs_csi2() scans an ACPI bus for devices with _CRS CSI-2
+ * descriptors and stores them to a linked list. This is done as traversing just
+ * this list is much smaller task than the entire DSDT. This list is then used
+ * to figure out how much memory is needed for swnodes related to a given ACPI
+ * device (handle). Further on, the same function sets the property values for
+ * the properties the values of which are obtained from the _CRS CSI-2
+ * descriptor. The information is stored into another list where the information
+ * can be looked up based on device's acpi_handle as the struct acpi_device
+ * isn't available yet at this point (and could not, as cross-device references
+ * need to be set up before the devices are available for drivers to probe).
+ *
+ * For each struct acpi_device, acpi_init_swnodes() further obtains information
+ * required to find out the values for the rest of the properties needed by
+ * drivers. This includes all port and endpoint properties as the node
+ * structures used by DT graphs and DisCo for Imaging are different. Finally the
+ * function registers software nodes for the device and sets the secondary
+ * pointer for the ACPI device's fwnode.
+ *
+ * Access to data the structures is serialised using acpi_scan_lock in scan.c.
+ *
+ * [1] https://uefi.org/sites/default/files/resources/ACPI_Spec_6_4_Jan22.pdf
+ *
+ * [2] https://www.mipi.org/specifications/mipi-disco-imaging
  */
 
 #include <linux/acpi.h>
-- 
2.30.2


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

* Re: [PATCH v5 2/8] ACPI: property: Parse _CRS CSI-2 descriptor
  2023-02-08 21:27 ` [PATCH v5 2/8] ACPI: property: Parse _CRS CSI-2 descriptor Sakari Ailus
@ 2023-02-09  9:47   ` Dan Carpenter
  2023-03-07 13:38   ` Rafael J. Wysocki
  1 sibling, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2023-02-09  9:47 UTC (permalink / raw)
  To: oe-kbuild, Sakari Ailus, linux-acpi
  Cc: lkp, oe-kbuild-all, linux-media, rafael, andriy.shevchenko,
	heikki.krogerus

Hi Sakari,

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Sakari-Ailus/ACPI-property-Parse-data-node-string-references-in-properties/20230209-053017
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20230208212712.3184953-3-sakari.ailus%40linux.intel.com
patch subject: [PATCH v5 2/8] ACPI: property: Parse _CRS CSI-2 descriptor
config: i386-randconfig-m021 (https://download.01.org/0day-ci/archive/20230209/202302091722.YCT7d1kl-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202302091722.YCT7d1kl-lkp@intel.com

smatch warnings:
drivers/acpi/mipi.c:241 acpi_crs_csi2_alloc_fill_swnodes() warn: variable dereferenced before check 'ads' (see line 237)

vim +/ads +241 drivers/acpi/mipi.c

06167f6b99bad9 Sakari Ailus 2023-02-08  206  void acpi_crs_csi2_alloc_fill_swnodes(size_t ports_count, acpi_handle handle)
06167f6b99bad9 Sakari Ailus 2023-02-08  207  {
06167f6b99bad9 Sakari Ailus 2023-02-08  208  	struct acpi_device_software_nodes *ads;
06167f6b99bad9 Sakari Ailus 2023-02-08  209  	struct crs_csi2_swnodes *swnodes;
06167f6b99bad9 Sakari Ailus 2023-02-08  210  	size_t alloc_size;
06167f6b99bad9 Sakari Ailus 2023-02-08  211  	unsigned int i;
06167f6b99bad9 Sakari Ailus 2023-02-08  212  	bool overflow;
06167f6b99bad9 Sakari Ailus 2023-02-08  213  	void *end;
06167f6b99bad9 Sakari Ailus 2023-02-08  214  
06167f6b99bad9 Sakari Ailus 2023-02-08  215  	/*
06167f6b99bad9 Sakari Ailus 2023-02-08  216  	 * Allocate memory for ports, node pointers (number of nodes +
06167f6b99bad9 Sakari Ailus 2023-02-08  217  	 * 1 (guardian), nodes (root + number of ports * 2 (for for
06167f6b99bad9 Sakari Ailus 2023-02-08  218  	 * every port there is an endpoint)).
06167f6b99bad9 Sakari Ailus 2023-02-08  219  	 */
06167f6b99bad9 Sakari Ailus 2023-02-08  220  	overflow = check_mul_overflow(sizeof(*ads->ports) +
06167f6b99bad9 Sakari Ailus 2023-02-08  221  				      sizeof(*ads->nodes) * 2 +
06167f6b99bad9 Sakari Ailus 2023-02-08  222  				      sizeof(*ads->nodeptrs) * 2,
06167f6b99bad9 Sakari Ailus 2023-02-08  223  				      ports_count, &alloc_size);
06167f6b99bad9 Sakari Ailus 2023-02-08  224  	overflow = overflow ||
06167f6b99bad9 Sakari Ailus 2023-02-08  225  		   check_add_overflow(sizeof(*ads) + sizeof(*ads->nodes) +
06167f6b99bad9 Sakari Ailus 2023-02-08  226  				      sizeof(*ads->nodeptrs) * 2,
06167f6b99bad9 Sakari Ailus 2023-02-08  227  				      alloc_size, &alloc_size);
06167f6b99bad9 Sakari Ailus 2023-02-08  228  	if (overflow) {
06167f6b99bad9 Sakari Ailus 2023-02-08  229  		acpi_handle_warn(handle,
06167f6b99bad9 Sakari Ailus 2023-02-08  230  				 "too many _CRS CSI2 resource handles (%zu)",
06167f6b99bad9 Sakari Ailus 2023-02-08  231  				 ports_count);
06167f6b99bad9 Sakari Ailus 2023-02-08  232  		return;
06167f6b99bad9 Sakari Ailus 2023-02-08  233  	}
06167f6b99bad9 Sakari Ailus 2023-02-08  234  
06167f6b99bad9 Sakari Ailus 2023-02-08  235  	swnodes = kzalloc(sizeof(*swnodes), GFP_KERNEL);
06167f6b99bad9 Sakari Ailus 2023-02-08  236  	ads = kzalloc(alloc_size, GFP_KERNEL);
06167f6b99bad9 Sakari Ailus 2023-02-08 @237  	ads->ports = (void *)(ads + 1);
                                                ^^^^^^^^^^
Derference

06167f6b99bad9 Sakari Ailus 2023-02-08  238  	ads->nodes = (void *)(ads->ports + ports_count);
06167f6b99bad9 Sakari Ailus 2023-02-08  239  	ads->nodeptrs = (void *)(ads->nodes + ports_count * 2 + 1);
06167f6b99bad9 Sakari Ailus 2023-02-08  240  	end = ads->nodeptrs + ports_count * 2 + 2;
06167f6b99bad9 Sakari Ailus 2023-02-08 @241  	if (!swnodes || !ads || WARN_ON((void *)ads + alloc_size != end)) {
                                                                ^^^^
Checked to late.

06167f6b99bad9 Sakari Ailus 2023-02-08  242  		kfree(swnodes);
06167f6b99bad9 Sakari Ailus 2023-02-08  243  		kfree(ads);
06167f6b99bad9 Sakari Ailus 2023-02-08  244  		acpi_handle_debug(handle,
06167f6b99bad9 Sakari Ailus 2023-02-08  245  				  "cannot allocate for %zu software nodes\n",
06167f6b99bad9 Sakari Ailus 2023-02-08  246  				  ports_count);
06167f6b99bad9 Sakari Ailus 2023-02-08  247  		return;
06167f6b99bad9 Sakari Ailus 2023-02-08  248  	}
06167f6b99bad9 Sakari Ailus 2023-02-08  249  
06167f6b99bad9 Sakari Ailus 2023-02-08  250  	ads->num_ports = ports_count;
06167f6b99bad9 Sakari Ailus 2023-02-08  251  	for (i = 0; i < ports_count * 2 + 1; i++)
06167f6b99bad9 Sakari Ailus 2023-02-08  252  		ads->nodeptrs[i] = &ads->nodes[i];
06167f6b99bad9 Sakari Ailus 2023-02-08  253  	ads->nodeptrs[i] = NULL;
06167f6b99bad9 Sakari Ailus 2023-02-08  254  	for (i = 0; i < ports_count; i++)
06167f6b99bad9 Sakari Ailus 2023-02-08  255  		ads->ports[i].port_nr = NO_CSI2_PORT;
06167f6b99bad9 Sakari Ailus 2023-02-08  256  	swnodes->handle = handle;
06167f6b99bad9 Sakari Ailus 2023-02-08  257  	swnodes->ads = ads;
06167f6b99bad9 Sakari Ailus 2023-02-08  258  	list_add(&swnodes->list, &crs_csi2_swnodes);
06167f6b99bad9 Sakari Ailus 2023-02-08  259  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests


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

* Re: [PATCH v5 1/8] ACPI: property: Parse data node string references in properties
  2023-02-08 21:27 ` [PATCH v5 1/8] ACPI: property: Parse data node string references in properties Sakari Ailus
@ 2023-03-07 13:30   ` Rafael J. Wysocki
  2023-03-13 16:59     ` Sakari Ailus
  0 siblings, 1 reply; 16+ messages in thread
From: Rafael J. Wysocki @ 2023-03-07 13:30 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-acpi, linux-media, rafael, andriy.shevchenko, heikki.krogerus

On Wed, Feb 8, 2023 at 10:27 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Add support for parsing property references using strings, besides
> reference objects that were previously supported. This allows also
> referencing data nodes which was not possible with reference objects.
>
> Also add pr_fmt() macro to prefix printouts.
>
> While at it, update copyright.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/acpi/property.c | 110 ++++++++++++++++++++++++++++++++++------
>  1 file changed, 94 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index b8d9eb9a433ed..4db7701aeaf5c 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -2,14 +2,17 @@
>  /*
>   * ACPI device specific properties support.
>   *
> - * Copyright (C) 2014, Intel Corporation
> + * Copyright (C) 2014-2023, Intel Corporation
>   * All rights reserved.
>   *
>   * Authors: Mika Westerberg <mika.westerberg@linux.intel.com>
> - *          Darren Hart <dvhart@linux.intel.com>
> - *          Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> + *         Darren Hart <dvhart@linux.intel.com>
> + *         Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Is the above change really that useful?

> + *         Sakari Ailus <sakari.ailus@linux.intel.com>
>   */
>
> +#define pr_fmt(fmt) "ACPI: " fmt
> +
>  #include <linux/acpi.h>
>  #include <linux/device.h>
>  #include <linux/export.h>
> @@ -795,7 +798,8 @@ acpi_fwnode_get_named_child_node(const struct fwnode_handle *fwnode,
>  static int acpi_get_ref_args(struct fwnode_reference_args *args,
>                              struct fwnode_handle *ref_fwnode,
>                              const union acpi_object **element,
> -                            const union acpi_object *end, size_t num_args)
> +                            const union acpi_object *end, size_t num_args,
> +                            bool subnode_string)
>  {
>         u32 nargs = 0, i;
>
> @@ -803,13 +807,16 @@ static int acpi_get_ref_args(struct fwnode_reference_args *args,
>          * Find the referred data extension node under the
>          * referred device node.
>          */
> -       for (; *element < end && (*element)->type == ACPI_TYPE_STRING;
> -            (*element)++) {
> -               const char *child_name = (*element)->string.pointer;
> -
> -               ref_fwnode = acpi_fwnode_get_named_child_node(ref_fwnode, child_name);
> -               if (!ref_fwnode)
> -                       return -EINVAL;
> +       if (subnode_string) {
> +               for (; *element < end && (*element)->type == ACPI_TYPE_STRING;
> +                    (*element)++) {
> +                       const char *child_name = (*element)->string.pointer;
> +
> +                       ref_fwnode = acpi_fwnode_get_named_child_node(ref_fwnode,
> +                                                                     child_name);
> +                       if (!ref_fwnode)
> +                               return -EINVAL;
> +               }
>         }
>
>         /*
> @@ -820,7 +827,8 @@ static int acpi_get_ref_args(struct fwnode_reference_args *args,
>         for (i = 0; (*element) + i < end && i < num_args; i++) {
>                 acpi_object_type type = (*element)[i].type;
>
> -               if (type == ACPI_TYPE_LOCAL_REFERENCE)
> +               if (type == ACPI_TYPE_LOCAL_REFERENCE ||
> +                   (!subnode_string && type == ACPI_TYPE_STRING))
>                         break;
>
>                 if (type == ACPI_TYPE_INTEGER)
> @@ -844,6 +852,43 @@ static int acpi_get_ref_args(struct fwnode_reference_args *args,
>         return 0;
>  }
>
> +static struct fwnode_handle *
> +acpi_parse_string_ref(const struct fwnode_handle *fwnode, const char *refstring)
> +{
> +       acpi_handle scope, handle;
> +       struct acpi_data_node *dn;
> +       struct acpi_device *device;
> +       acpi_status status;
> +
> +       if (is_acpi_device_node(fwnode)) {
> +               scope = to_acpi_device_node(fwnode)->handle;
> +       } else if (is_acpi_data_node(fwnode)) {
> +               scope = to_acpi_data_node(fwnode)->handle;
> +       } else {
> +               pr_err("bad node type for node %pfw\n", fwnode);

Is the log level here adequate?

I mean, what is a sysadmin seeing this message in the log expected to
do about it?

> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       status = acpi_get_handle(scope, refstring, &handle);
> +       if (ACPI_FAILURE(status)) {
> +               acpi_handle_debug(scope, "can't get handle for %s", refstring);
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       device = acpi_fetch_acpi_dev(handle);
> +       if (device)
> +               return acpi_fwnode_handle(device);
> +
> +       status = acpi_get_data_full(handle, acpi_nondev_subnode_tag,
> +                                   (void **)&dn, NULL);
> +       if (ACPI_FAILURE(status) || !dn) {
> +               acpi_handle_debug(handle, "can't find subnode");
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       return &dn->fwnode;
> +}
> +
>  /**
>   * __acpi_node_get_property_reference - returns handle to the referenced object
>   * @fwnode: Firmware node to get the property from
> @@ -886,6 +931,7 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
>         const union acpi_object *element, *end;
>         const union acpi_object *obj;
>         const struct acpi_device_data *data;
> +       struct fwnode_handle *ref_fwnode;
>         struct acpi_device *device;
>         int ret, idx = 0;
>
> @@ -909,16 +955,29 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
>
>                 args->fwnode = acpi_fwnode_handle(device);
>                 args->nargs = 0;
> +               return 0;
> +       case ACPI_TYPE_STRING:
> +               if (index)
> +                       return -ENOENT;
> +
> +               ref_fwnode = acpi_parse_string_ref(fwnode, obj->string.pointer);
> +               if (IS_ERR(ref_fwnode))
> +                       return PTR_ERR(ref_fwnode);
> +
> +               args->fwnode = ref_fwnode;
> +               args->nargs = 0;
> +
>                 return 0;
>         case ACPI_TYPE_PACKAGE:
>                 /*
>                  * If it is not a single reference, then it is a package of
> -                * references followed by number of ints as follows:
> +                * references, followed by number of ints as follows:
>                  *
>                  *  Package () { REF, INT, REF, INT, INT }
>                  *
> -                * The index argument is then used to determine which reference
> -                * the caller wants (along with the arguments).
> +                * Here, REF may be either a local reference or a string. The
> +                * index argument is then used to determine which reference the
> +                * caller wants (along with the arguments).
>                  */
>                 break;
>         default:
> @@ -942,7 +1001,26 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
>
>                         ret = acpi_get_ref_args(idx == index ? args : NULL,
>                                                 acpi_fwnode_handle(device),
> -                                               &element, end, num_args);
> +                                               &element, end, num_args, true);
> +                       if (ret < 0)
> +                               return ret;
> +
> +                       if (idx == index)
> +                               return 0;
> +
> +                       break;
> +               case ACPI_TYPE_STRING:
> +                       ref_fwnode =
> +                               acpi_parse_string_ref(fwnode,
> +                                                     element->string.pointer);
> +                       if (IS_ERR(ref_fwnode))
> +                               return PTR_ERR(ref_fwnode);
> +
> +                       element++;
> +
> +                       ret = acpi_get_ref_args(idx == index ? args : NULL,
> +                                               ref_fwnode, &element, end,
> +                                               num_args, false);
>                         if (ret < 0)
>                                 return ret;
>
> --
> 2.30.2
>

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

* Re: [PATCH v5 2/8] ACPI: property: Parse _CRS CSI-2 descriptor
  2023-02-08 21:27 ` [PATCH v5 2/8] ACPI: property: Parse _CRS CSI-2 descriptor Sakari Ailus
  2023-02-09  9:47   ` Dan Carpenter
@ 2023-03-07 13:38   ` Rafael J. Wysocki
  2023-03-07 19:00     ` Sakari Ailus
  1 sibling, 1 reply; 16+ messages in thread
From: Rafael J. Wysocki @ 2023-03-07 13:38 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: linux-acpi, linux-media, rafael, andriy.shevchenko, heikki.krogerus

Note: The report from Dan Carpenter has not been addressed.

On Wed, Feb 8, 2023 at 10:27 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Parse newly added ACPI _CRS CSI-2 descriptor for CSI-2 and camera
> configuration. For now, only figure out where the descriptor is present in
> order to allow adding information from it to related devices.

This should contain (a) pointer(s) to the relevant specification material.

> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/acpi/Makefile   |   2 +-
>  drivers/acpi/internal.h |   7 +
>  drivers/acpi/mipi.c     | 358 ++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/scan.c     |  16 +-
>  include/acpi/acpi_bus.h |  11 ++
>  5 files changed, 389 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/acpi/mipi.c
>
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index a5b649e71ab1b..a98fa1bc15548 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -37,7 +37,7 @@ acpi-$(CONFIG_ACPI_SLEEP)     += proc.o
>  # ACPI Bus and Device Drivers
>  #
>  acpi-y                         += bus.o glue.o
> -acpi-y                         += scan.o
> +acpi-y                         += scan.o mipi.o
>  acpi-y                         += resource.o
>  acpi-y                         += acpi_processor.o
>  acpi-y                         += processor_core.o
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index ec584442fb298..1ec4aa92bf17a 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -282,4 +282,11 @@ void acpi_init_lpit(void);
>  static inline void acpi_init_lpit(void) { }
>  #endif
>
> +/*--------------------------------------------------------------------------
> +                               ACPI and MIPI DisCo for Imaging conversion
> +  -------------------------------------------------------------------------- */
> +
> +void acpi_crs_csi2_swnodes_del_free(void);
> +void acpi_bus_scan_crs_csi2(acpi_handle handle);
> +
>  #endif /* _ACPI_INTERNAL_H_ */
> diff --git a/drivers/acpi/mipi.c b/drivers/acpi/mipi.c
> new file mode 100644
> index 0000000000000..33d50df831933
> --- /dev/null
> +++ b/drivers/acpi/mipi.c
> @@ -0,0 +1,358 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * MIPI DisCo for Imaging support.

I do realize that the last patch in the series contains the
description of this code, but it would be really helpful to put at
least some of it here and update it in the following patches as the
code gets modified.

> + *
> + * Copyright (C) 2023 Intel Corporation
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/limits.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/overflow.h>
> +#include <linux/types.h>
> +#include <linux/slab.h>
> +#include <linux/sort.h>
> +#include <linux/string.h>
> +
> +#include "internal.h"
> +
> +/* Temporary ACPI device handle to software node data structure mapping */
> +struct crs_csi2_swnodes {
> +       struct list_head list;
> +       acpi_handle handle;
> +       struct acpi_device_software_nodes *ads;
> +};
> +
> +static LIST_HEAD(crs_csi2_swnodes);
> +
> +static void crs_csi2_swnode_del_free(struct crs_csi2_swnodes *swnodes)
> +{
> +       list_del(&swnodes->list);
> +       kfree(swnodes);
> +}
> +
> +void acpi_crs_csi2_swnodes_del_free(void)
> +{
> +       struct crs_csi2_swnodes *swnodes, *swnodes_tmp;
> +
> +       list_for_each_entry_safe(swnodes, swnodes_tmp, &crs_csi2_swnodes, list)
> +               crs_csi2_swnode_del_free(swnodes);
> +}
> +
> +/*
> + * Description of a _CRS CSI2 resource descriptor before software node creation
> + */
> +struct crs_csi2_instance {
> +       struct list_head list;
> +       struct acpi_resource_csi2_serialbus csi2;
> +       acpi_handle remote_handle;
> +       char remote_name[];
> +};
> +
> +/* Temporary list of ACPI device nodes with _CRS CSI2 resource descriptors */
> +struct crs_csi2 {
> +       struct list_head list;
> +       acpi_handle handle;
> +       struct list_head buses;
> +};
> +
> +/*
> + * Context for collecting _CRS CSI2 resource descriptors during ACPI namespace
> + * walk
> + */
> +struct scan_check_crs_csi2_context {
> +       struct list_head res_head;
> +       acpi_handle handle;
> +       size_t handle_count;
> +};
> +
> +/* Context for scanning ACPI device nodes for _CRS CSI2 resource descriptors */
> +struct crs_csi2_all {
> +       struct list_head crs_csi2_head;
> +       size_t handle_count;
> +};
> +
> +/* Scan a single CSI2 resource descriptor in _CRS. */
> +static acpi_status scan_check_crs_csi2_instance(struct acpi_resource *res,
> +                                               void *context)
> +{
> +       struct scan_check_crs_csi2_context *inst_context = context;
> +       struct acpi_resource_csi2_serialbus *csi2;
> +       struct crs_csi2_instance *inst;
> +       acpi_handle remote_handle;
> +
> +       if (res->type != ACPI_RESOURCE_TYPE_SERIAL_BUS)
> +               return AE_OK;
> +
> +       csi2 = &res->data.csi2_serial_bus;
> +
> +       if (csi2->type != ACPI_RESOURCE_SERIAL_TYPE_CSI2)
> +               return AE_OK;
> +
> +       if (!csi2->resource_source.string_length) {
> +               acpi_handle_debug(inst_context->handle,
> +                                 "invalid resource source string length\n");
> +               return AE_OK;
> +       }
> +
> +       if (ACPI_FAILURE(acpi_get_handle(NULL, csi2->resource_source.string_ptr,
> +                                        &remote_handle))) {
> +               acpi_handle_warn(inst_context->handle,
> +                                "cannot get handle for %s\n",
> +                                csi2->resource_source.string_ptr);
> +               return AE_OK;
> +       }
> +
> +       /*
> +        * Allocate space to store the _CRS CSI2 entry and its data and add it
> +        * to the list.
> +        */
> +       inst = kmalloc(struct_size(inst, remote_name, csi2->resource_source.string_length),
> +                      GFP_KERNEL);
> +       if (!inst)
> +               return AE_OK;
> +
> +       inst->csi2 = *csi2;
> +       strscpy(inst->remote_name, csi2->resource_source.string_ptr,
> +               csi2->resource_source.string_length);
> +       inst->csi2.resource_source.string_ptr = inst->remote_name;
> +       inst->remote_handle = remote_handle;
> +
> +       list_add(&inst->list, &inst_context->res_head);
> +
> +       inst_context->handle_count++;
> +
> +       return AE_OK;
> +}
> +
> +/*
> + * Find all devices with _CRS CSI2 resource descriptors and collect them
> + * into a list for later use.
> + */
> +static acpi_status scan_check_crs_csi2(acpi_handle handle, u32 nesting_level,
> +                                      void *context, void **ret)
> +{
> +       struct scan_check_crs_csi2_context inst_context = {
> +               .handle = handle,
> +               .res_head = LIST_HEAD_INIT(inst_context.res_head),
> +       };
> +       struct crs_csi2_all *csi2_all = context;
> +       struct crs_csi2 *csi2;
> +
> +       acpi_walk_resources(handle, METHOD_NAME__CRS,
> +                           scan_check_crs_csi2_instance, &inst_context);
> +
> +       if (list_empty(&inst_context.res_head))
> +               return AE_OK;
> +
> +       /*
> +        * Found entry, so allocate memory for it, fill it and add it to the
> +        * list.
> +        */
> +       csi2 = kmalloc(sizeof(*csi2), GFP_KERNEL);
> +       if (!csi2)
> +               return AE_OK;
> +
> +       csi2->handle = handle;
> +       list_replace(&inst_context.res_head, &csi2->buses);
> +       list_add(&csi2->list, &csi2_all->crs_csi2_head);
> +
> +       /* This handle plus remote handles in _CRS CSI2 resource descriptors */
> +       csi2_all->handle_count += 1 + inst_context.handle_count;
> +
> +       return AE_OK;
> +}
> +
> +struct acpi_handle_ref {
> +       acpi_handle handle;
> +       unsigned int count;
> +};
> +
> +#define NO_CSI2_PORT (UINT_MAX - 1)
> +
> +static int crs_handle_cmp(const void *__a, const void *__b)
> +{
> +       const struct acpi_handle_ref *a = __a, *b = __b;
> +
> +       return a->handle < b->handle ? -1 : a->handle > b->handle;
> +}
> +
> +/*
> + * Release entries in temporary storage of ACPI device nodes with _CRS CSI2
> + * resource descriptors.
> + */
> +static void crs_csi2_release(struct list_head *crs_csi2_handles)
> +{
> +       struct crs_csi2 *csi2, *csi2_tmp;
> +
> +       list_for_each_entry_safe(csi2, csi2_tmp, crs_csi2_handles, list) {
> +               struct crs_csi2_instance *inst, *inst_tmp;
> +
> +               list_for_each_entry_safe(inst, inst_tmp, &csi2->buses, list) {
> +                       list_del(&inst->list);
> +                       kfree(inst);
> +               }
> +
> +               list_del(&csi2->list);
> +               kfree(csi2);
> +       }
> +}
> +
> +/*
> + * Allocate memory and set up software nodes for an ACPI device with given
> + * number of CSI-2 ports.
> + */
> +void acpi_crs_csi2_alloc_fill_swnodes(size_t ports_count, acpi_handle handle)
> +{
> +       struct acpi_device_software_nodes *ads;
> +       struct crs_csi2_swnodes *swnodes;
> +       size_t alloc_size;
> +       unsigned int i;
> +       bool overflow;
> +       void *end;
> +
> +       /*
> +        * Allocate memory for ports, node pointers (number of nodes +
> +        * 1 (guardian), nodes (root + number of ports * 2 (for for
> +        * every port there is an endpoint)).
> +        */
> +       overflow = check_mul_overflow(sizeof(*ads->ports) +
> +                                     sizeof(*ads->nodes) * 2 +
> +                                     sizeof(*ads->nodeptrs) * 2,
> +                                     ports_count, &alloc_size);
> +       overflow = overflow ||
> +                  check_add_overflow(sizeof(*ads) + sizeof(*ads->nodes) +
> +                                     sizeof(*ads->nodeptrs) * 2,
> +                                     alloc_size, &alloc_size);
> +       if (overflow) {
> +               acpi_handle_warn(handle,
> +                                "too many _CRS CSI2 resource handles (%zu)",
> +                                ports_count);
> +               return;
> +       }
> +
> +       swnodes = kzalloc(sizeof(*swnodes), GFP_KERNEL);
> +       ads = kzalloc(alloc_size, GFP_KERNEL);
> +       ads->ports = (void *)(ads + 1);
> +       ads->nodes = (void *)(ads->ports + ports_count);
> +       ads->nodeptrs = (void *)(ads->nodes + ports_count * 2 + 1);
> +       end = ads->nodeptrs + ports_count * 2 + 2;
> +       if (!swnodes || !ads || WARN_ON((void *)ads + alloc_size != end)) {
> +               kfree(swnodes);
> +               kfree(ads);
> +               acpi_handle_debug(handle,
> +                                 "cannot allocate for %zu software nodes\n",
> +                                 ports_count);
> +               return;
> +       }
> +
> +       ads->num_ports = ports_count;
> +       for (i = 0; i < ports_count * 2 + 1; i++)
> +               ads->nodeptrs[i] = &ads->nodes[i];
> +       ads->nodeptrs[i] = NULL;
> +       for (i = 0; i < ports_count; i++)
> +               ads->ports[i].port_nr = NO_CSI2_PORT;
> +       swnodes->handle = handle;
> +       swnodes->ads = ads;
> +       list_add(&swnodes->list, &crs_csi2_swnodes);
> +}
> +
> +/**
> + * acpi_bus_scan_crs_csi2 - Scan a device and its child devices for _CRS CSI-2
> + *
> + * @handle: Root of the ACPI Namespace branch to scan
> + *
> + * This function does a number of things:
> + *
> + * 1. Iteratively scan all ACPI device nodes starting from a handle for _CRS
> + *    CSI-2 instances. The instances are stored for later use.
> + *
> + * 2. Count how many references to other devices _CRS CSI-2 instances have in
> + *    total.
> + *
> + * 3. Count the number of references to other devices for each _CRS CSI-2
> + *    instance.
> + *
> + * 4. Allocate memory for swnodes each ACPI device requires later on, and
> + *    generate a list of such allocations.
> + *
> + * Note that struct acpi_device isn't available yet at this time.
> + *
> + * acpi_scan_lock in scan.c must be held when calling this function.
> + */
> +void acpi_bus_scan_crs_csi2(acpi_handle handle)
> +{
> +       struct acpi_handle_ref *handle_refs;
> +       struct acpi_handle_ref *this = NULL;
> +       struct crs_csi2_all csi2_all = {
> +               .crs_csi2_head = LIST_HEAD_INIT(csi2_all.crs_csi2_head),
> +       };
> +       size_t this_count;
> +       unsigned int curr = 0;
> +       struct crs_csi2 *csi2;
> +
> +       /*
> +        * Collect the devices that have a _CRS CSI-2 resource. Each CSI-2 TX
> +        * port has one.
> +        */
> +       acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, ACPI_UINT32_MAX,
> +                           scan_check_crs_csi2, NULL, &csi2_all, NULL);

I still don't like this walk and I still think that it is avoidable.

At least I don't see why it isn't avoidable ATM.

> +
> +       /* No handles? Bail out here. */
> +       if (!csi2_all.handle_count)
> +               return;
> +
> +       handle_refs = kcalloc(csi2_all.handle_count + 1, sizeof(*handle_refs),
> +                             GFP_KERNEL);
> +       if (!handle_refs) {
> +               acpi_handle_debug(handle, "no memory for %zu handle refs\n",
> +                                 csi2_all.handle_count + 1);
> +               return;
> +       }
> +
> +       /* Associate handles to the number of references. */
> +       list_for_each_entry(csi2, &csi2_all.crs_csi2_head, list) {
> +               struct crs_csi2_instance *inst;
> +               struct acpi_handle_ref *handle_ref;
> +
> +               handle_ref = &handle_refs[curr++];
> +               handle_ref->handle = csi2->handle;
> +
> +               list_for_each_entry(inst, &csi2->buses, list) {
> +                       handle_refs[curr].handle = inst->remote_handle;
> +                       handle_refs[curr].count = 1;
> +                       handle_ref->count++;
> +                       curr++;
> +               }
> +       }
> +
> +       handle_refs[curr].handle = NULL;
> +
> +       /* Sort the handles by remote so duplicates canbe easily found. */
> +       sort(handle_refs, csi2_all.handle_count, sizeof(*handle_refs), crs_handle_cmp, NULL);
> +
> +       /*
> +        * Finally count references in each handle, allocate space for device
> +        * specific ports, properties and fill the _CRS CSI2 descriptor
> +        * originated data.
> +        */
> +       this = handle_refs;
> +       this_count = this->count;
> +       for (curr = 1; curr < csi2_all.handle_count + 1; curr++) {
> +               /* Weed out duplicate receiver devices. */
> +               if (this->handle == handle_refs[curr].handle) {
> +                       this_count += handle_refs[curr].count;
> +                       continue;
> +               }
> +
> +               acpi_crs_csi2_alloc_fill_swnodes(this_count, this->handle);
> +
> +               this = &handle_refs[curr];
> +               this_count = this->count;
> +       }
> +
> +       kfree(handle_refs);
> +
> +       crs_csi2_release(&csi2_all.crs_csi2_head);
> +}
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 0c6f06abe3f47..50de874b8f208 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2423,9 +2423,12 @@ EXPORT_SYMBOL_GPL(acpi_dev_get_next_consumer_dev);
>  int acpi_bus_scan(acpi_handle handle)
>  {
>         struct acpi_device *device = NULL;
> +       int ret = 0;
>
>         acpi_bus_scan_second_pass = false;
>
> +       acpi_bus_scan_crs_csi2(handle);
> +
>         /* Pass 1: Avoid enumerating devices with missing dependencies. */
>
>         if (ACPI_SUCCESS(acpi_bus_check_add(handle, true, &device)))
> @@ -2433,13 +2436,15 @@ int acpi_bus_scan(acpi_handle handle)
>                                     acpi_bus_check_add_1, NULL, NULL,
>                                     (void **)&device);
>
> -       if (!device)
> -               return -ENODEV;
> +       if (!device) {
> +               ret = -ENODEV;
> +               goto out_release;
> +       }
>
>         acpi_bus_attach(device, (void *)true);
>
>         if (!acpi_bus_scan_second_pass)
> -               return 0;
> +               goto out_release;
>
>         /* Pass 2: Enumerate all of the remaining devices. */
>
> @@ -2452,7 +2457,10 @@ int acpi_bus_scan(acpi_handle handle)
>
>         acpi_bus_attach(device, NULL);
>
> -       return 0;
> +out_release:
> +       acpi_crs_csi2_swnodes_del_free();
> +
> +       return ret;
>  }
>  EXPORT_SYMBOL(acpi_bus_scan);
>
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index e44be31115a67..a05fe22c1175c 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -360,6 +360,17 @@ struct acpi_device_data {
>
>  struct acpi_gpio_mapping;
>
> +struct acpi_device_software_node_port {
> +       unsigned int port_nr;
> +};
> +
> +struct acpi_device_software_nodes {
> +       struct acpi_device_software_node_port *ports;
> +       struct software_node *nodes;
> +       const struct software_node **nodeptrs;
> +       unsigned int num_ports;
> +};
> +
>  /* Device */
>  struct acpi_device {
>         u32 pld_crc;
> --
> 2.30.2
>

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

* Re: [PATCH v5 2/8] ACPI: property: Parse _CRS CSI-2 descriptor
  2023-03-07 13:38   ` Rafael J. Wysocki
@ 2023-03-07 19:00     ` Sakari Ailus
  2023-03-07 19:02       ` Sakari Ailus
  2023-03-07 19:33       ` Sakari Ailus
  0 siblings, 2 replies; 16+ messages in thread
From: Sakari Ailus @ 2023-03-07 19:00 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linux-acpi, linux-media, andriy.shevchenko, heikki.krogerus

Hi Rafael,

On Tue, Mar 07, 2023 at 02:38:43PM +0100, Rafael J. Wysocki wrote:
> Note: The report from Dan Carpenter has not been addressed.

I'll reply to Dan --- it's a false positive.

> 
> On Wed, Feb 8, 2023 at 10:27 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Parse newly added ACPI _CRS CSI-2 descriptor for CSI-2 and camera
> > configuration. For now, only figure out where the descriptor is present in
> > order to allow adding information from it to related devices.
> 
> This should contain (a) pointer(s) to the relevant specification material.

I can add that for v6.

> 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/acpi/Makefile   |   2 +-
> >  drivers/acpi/internal.h |   7 +
> >  drivers/acpi/mipi.c     | 358 ++++++++++++++++++++++++++++++++++++++++
> >  drivers/acpi/scan.c     |  16 +-
> >  include/acpi/acpi_bus.h |  11 ++
> >  5 files changed, 389 insertions(+), 5 deletions(-)
> >  create mode 100644 drivers/acpi/mipi.c
> >
> > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> > index a5b649e71ab1b..a98fa1bc15548 100644
> > --- a/drivers/acpi/Makefile
> > +++ b/drivers/acpi/Makefile
> > @@ -37,7 +37,7 @@ acpi-$(CONFIG_ACPI_SLEEP)     += proc.o
> >  # ACPI Bus and Device Drivers
> >  #
> >  acpi-y                         += bus.o glue.o
> > -acpi-y                         += scan.o
> > +acpi-y                         += scan.o mipi.o
> >  acpi-y                         += resource.o
> >  acpi-y                         += acpi_processor.o
> >  acpi-y                         += processor_core.o
> > diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> > index ec584442fb298..1ec4aa92bf17a 100644
> > --- a/drivers/acpi/internal.h
> > +++ b/drivers/acpi/internal.h
> > @@ -282,4 +282,11 @@ void acpi_init_lpit(void);
> >  static inline void acpi_init_lpit(void) { }
> >  #endif
> >
> > +/*--------------------------------------------------------------------------
> > +                               ACPI and MIPI DisCo for Imaging conversion
> > +  -------------------------------------------------------------------------- */
> > +
> > +void acpi_crs_csi2_swnodes_del_free(void);
> > +void acpi_bus_scan_crs_csi2(acpi_handle handle);
> > +
> >  #endif /* _ACPI_INTERNAL_H_ */
> > diff --git a/drivers/acpi/mipi.c b/drivers/acpi/mipi.c
> > new file mode 100644
> > index 0000000000000..33d50df831933
> > --- /dev/null
> > +++ b/drivers/acpi/mipi.c
> > @@ -0,0 +1,358 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * MIPI DisCo for Imaging support.
> 
> I do realize that the last patch in the series contains the
> description of this code, but it would be really helpful to put at
> least some of it here and update it in the following patches as the
> code gets modified.

I'll see if it could be meaningfully split, with some going to this patch.

> 
> > + *
> > + * Copyright (C) 2023 Intel Corporation
> > + */

...

> > +void acpi_bus_scan_crs_csi2(acpi_handle handle)
> > +{
> > +       struct acpi_handle_ref *handle_refs;
> > +       struct acpi_handle_ref *this = NULL;
> > +       struct crs_csi2_all csi2_all = {
> > +               .crs_csi2_head = LIST_HEAD_INIT(csi2_all.crs_csi2_head),
> > +       };
> > +       size_t this_count;
> > +       unsigned int curr = 0;
> > +       struct crs_csi2 *csi2;
> > +
> > +       /*
> > +        * Collect the devices that have a _CRS CSI-2 resource. Each CSI-2 TX
> > +        * port has one.
> > +        */
> > +       acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, ACPI_UINT32_MAX,
> > +                           scan_check_crs_csi2, NULL, &csi2_all, NULL);
> 
> I still don't like this walk and I still think that it is avoidable.
> 
> At least I don't see why it isn't avoidable ATM.

In acpi_scan_init(), the ACPI tables has been just obtained (via
acpi_get_table()), and the second table traversal in acpi_bus_scan(),
acpi_bus_check_add_1() will, in turn, create devices that further can be
probed.

I guess we could look at the "mipi-img-port-*" nodes right under the device
to see whether the device is expected to be a CSI-2 receiver. I'm not sure
if this would bring a performance improvement or further degradation.

Another option could be to opportunistically initialise a device, and once
encountering a "mipi-img-port-*" node in _DSD property parsing, bail out
and postpone initialisation of the device on first namespace traversal.
This would mean adding some extra error handling in e.g.
acpi_init_device_object() callers.

As the proportion of devices with CSI-2 connections compared to all devices
is expected to be rather small, the second option would likely bring a
performance improvement compared to the current implementation but it would
be somewhat ugly to involve property parsing with this IMO.

-- 
Kind regards,

Sakari Ailus

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

* Re: [PATCH v5 2/8] ACPI: property: Parse _CRS CSI-2 descriptor
  2023-03-07 19:00     ` Sakari Ailus
@ 2023-03-07 19:02       ` Sakari Ailus
  2023-03-07 19:33       ` Sakari Ailus
  1 sibling, 0 replies; 16+ messages in thread
From: Sakari Ailus @ 2023-03-07 19:02 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linux-acpi, linux-media, andriy.shevchenko, heikki.krogerus

On Tue, Mar 07, 2023 at 09:00:30PM +0200, Sakari Ailus wrote:
> Hi Rafael,
> 
> On Tue, Mar 07, 2023 at 02:38:43PM +0100, Rafael J. Wysocki wrote:
> > Note: The report from Dan Carpenter has not been addressed.
> 
> I'll reply to Dan --- it's a false positive.

Ah, it isn't. I'll address it for v6.

-- 
Sakari Ailus

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

* Re: [PATCH v5 2/8] ACPI: property: Parse _CRS CSI-2 descriptor
  2023-03-07 19:00     ` Sakari Ailus
  2023-03-07 19:02       ` Sakari Ailus
@ 2023-03-07 19:33       ` Sakari Ailus
  1 sibling, 0 replies; 16+ messages in thread
From: Sakari Ailus @ 2023-03-07 19:33 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linux-acpi, linux-media, andriy.shevchenko, heikki.krogerus

On Tue, Mar 07, 2023 at 09:00:30PM +0200, Sakari Ailus wrote:
> Another option could be to opportunistically initialise a device, and once
> encountering a "mipi-img-port-*" node in _DSD property parsing, bail out
> and postpone initialisation of the device on first namespace traversal.
> This would mean adding some extra error handling in e.g.
> acpi_init_device_object() callers.

What I think could be also done is to collect the handles with these nodes
to a list (requires struct list_head in struct acpi_device) in order to
create the devices after tree traversal.

This approach might be usable for even getting rid of the second pass
altogether, with the tradeoff of requiring a little bit of extra memory.

-- 
Sakari Ailus

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

* Re: [PATCH v5 1/8] ACPI: property: Parse data node string references in properties
  2023-03-07 13:30   ` Rafael J. Wysocki
@ 2023-03-13 16:59     ` Sakari Ailus
  0 siblings, 0 replies; 16+ messages in thread
From: Sakari Ailus @ 2023-03-13 16:59 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linux-acpi, linux-media, andriy.shevchenko, heikki.krogerus

Hi Rafael,

On Tue, Mar 07, 2023 at 02:30:45PM +0100, Rafael J. Wysocki wrote:
> On Wed, Feb 8, 2023 at 10:27 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Add support for parsing property references using strings, besides
> > reference objects that were previously supported. This allows also
> > referencing data nodes which was not possible with reference objects.
> >
> > Also add pr_fmt() macro to prefix printouts.
> >
> > While at it, update copyright.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/acpi/property.c | 110 ++++++++++++++++++++++++++++++++++------
> >  1 file changed, 94 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> > index b8d9eb9a433ed..4db7701aeaf5c 100644
> > --- a/drivers/acpi/property.c
> > +++ b/drivers/acpi/property.c
> > @@ -2,14 +2,17 @@
> >  /*
> >   * ACPI device specific properties support.
> >   *
> > - * Copyright (C) 2014, Intel Corporation
> > + * Copyright (C) 2014-2023, Intel Corporation
> >   * All rights reserved.
> >   *
> >   * Authors: Mika Westerberg <mika.westerberg@linux.intel.com>
> > - *          Darren Hart <dvhart@linux.intel.com>
> > - *          Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > + *         Darren Hart <dvhart@linux.intel.com>
> > + *         Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Is the above change really that useful?

Indentation should use tabs. I initially had the added line only do that
but Andy asked to adjust that.

> 
> > + *         Sakari Ailus <sakari.ailus@linux.intel.com>
> >   */
> >
> > +#define pr_fmt(fmt) "ACPI: " fmt
> > +
> >  #include <linux/acpi.h>
> >  #include <linux/device.h>
> >  #include <linux/export.h>
> > @@ -795,7 +798,8 @@ acpi_fwnode_get_named_child_node(const struct fwnode_handle *fwnode,
> >  static int acpi_get_ref_args(struct fwnode_reference_args *args,
> >                              struct fwnode_handle *ref_fwnode,
> >                              const union acpi_object **element,
> > -                            const union acpi_object *end, size_t num_args)
> > +                            const union acpi_object *end, size_t num_args,
> > +                            bool subnode_string)
> >  {
> >         u32 nargs = 0, i;
> >
> > @@ -803,13 +807,16 @@ static int acpi_get_ref_args(struct fwnode_reference_args *args,
> >          * Find the referred data extension node under the
> >          * referred device node.
> >          */
> > -       for (; *element < end && (*element)->type == ACPI_TYPE_STRING;
> > -            (*element)++) {
> > -               const char *child_name = (*element)->string.pointer;
> > -
> > -               ref_fwnode = acpi_fwnode_get_named_child_node(ref_fwnode, child_name);
> > -               if (!ref_fwnode)
> > -                       return -EINVAL;
> > +       if (subnode_string) {
> > +               for (; *element < end && (*element)->type == ACPI_TYPE_STRING;
> > +                    (*element)++) {
> > +                       const char *child_name = (*element)->string.pointer;
> > +
> > +                       ref_fwnode = acpi_fwnode_get_named_child_node(ref_fwnode,
> > +                                                                     child_name);
> > +                       if (!ref_fwnode)
> > +                               return -EINVAL;
> > +               }
> >         }
> >
> >         /*
> > @@ -820,7 +827,8 @@ static int acpi_get_ref_args(struct fwnode_reference_args *args,
> >         for (i = 0; (*element) + i < end && i < num_args; i++) {
> >                 acpi_object_type type = (*element)[i].type;
> >
> > -               if (type == ACPI_TYPE_LOCAL_REFERENCE)
> > +               if (type == ACPI_TYPE_LOCAL_REFERENCE ||
> > +                   (!subnode_string && type == ACPI_TYPE_STRING))
> >                         break;
> >
> >                 if (type == ACPI_TYPE_INTEGER)
> > @@ -844,6 +852,43 @@ static int acpi_get_ref_args(struct fwnode_reference_args *args,
> >         return 0;
> >  }
> >
> > +static struct fwnode_handle *
> > +acpi_parse_string_ref(const struct fwnode_handle *fwnode, const char *refstring)
> > +{
> > +       acpi_handle scope, handle;
> > +       struct acpi_data_node *dn;
> > +       struct acpi_device *device;
> > +       acpi_status status;
> > +
> > +       if (is_acpi_device_node(fwnode)) {
> > +               scope = to_acpi_device_node(fwnode)->handle;
> > +       } else if (is_acpi_data_node(fwnode)) {
> > +               scope = to_acpi_data_node(fwnode)->handle;
> > +       } else {
> > +               pr_err("bad node type for node %pfw\n", fwnode);
> 
> Is the log level here adequate?
> 
> I mean, what is a sysadmin seeing this message in the log expected to
> do about it?

Probably nothing else than to scream. I'll change this to pr_debug().

> 
> > +               return ERR_PTR(-EINVAL);
> > +       }
> > +
> > +       status = acpi_get_handle(scope, refstring, &handle);
> > +       if (ACPI_FAILURE(status)) {
> > +               acpi_handle_debug(scope, "can't get handle for %s", refstring);
> > +               return ERR_PTR(-EINVAL);
> > +       }
> > +
> > +       device = acpi_fetch_acpi_dev(handle);
> > +       if (device)
> > +               return acpi_fwnode_handle(device);
> > +
> > +       status = acpi_get_data_full(handle, acpi_nondev_subnode_tag,
> > +                                   (void **)&dn, NULL);
> > +       if (ACPI_FAILURE(status) || !dn) {
> > +               acpi_handle_debug(handle, "can't find subnode");
> > +               return ERR_PTR(-EINVAL);
> > +       }
> > +
> > +       return &dn->fwnode;
> > +}
> > +
> >  /**
> >   * __acpi_node_get_property_reference - returns handle to the referenced object
> >   * @fwnode: Firmware node to get the property from
> > @@ -886,6 +931,7 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
> >         const union acpi_object *element, *end;
> >         const union acpi_object *obj;
> >         const struct acpi_device_data *data;
> > +       struct fwnode_handle *ref_fwnode;
> >         struct acpi_device *device;
> >         int ret, idx = 0;
> >
> > @@ -909,16 +955,29 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
> >
> >                 args->fwnode = acpi_fwnode_handle(device);
> >                 args->nargs = 0;
> > +               return 0;
> > +       case ACPI_TYPE_STRING:
> > +               if (index)
> > +                       return -ENOENT;
> > +
> > +               ref_fwnode = acpi_parse_string_ref(fwnode, obj->string.pointer);
> > +               if (IS_ERR(ref_fwnode))
> > +                       return PTR_ERR(ref_fwnode);
> > +
> > +               args->fwnode = ref_fwnode;
> > +               args->nargs = 0;
> > +
> >                 return 0;
> >         case ACPI_TYPE_PACKAGE:
> >                 /*
> >                  * If it is not a single reference, then it is a package of
> > -                * references followed by number of ints as follows:
> > +                * references, followed by number of ints as follows:
> >                  *
> >                  *  Package () { REF, INT, REF, INT, INT }
> >                  *
> > -                * The index argument is then used to determine which reference
> > -                * the caller wants (along with the arguments).
> > +                * Here, REF may be either a local reference or a string. The
> > +                * index argument is then used to determine which reference the
> > +                * caller wants (along with the arguments).
> >                  */
> >                 break;
> >         default:
> > @@ -942,7 +1001,26 @@ int __acpi_node_get_property_reference(const struct fwnode_handle *fwnode,
> >
> >                         ret = acpi_get_ref_args(idx == index ? args : NULL,
> >                                                 acpi_fwnode_handle(device),
> > -                                               &element, end, num_args);
> > +                                               &element, end, num_args, true);
> > +                       if (ret < 0)
> > +                               return ret;
> > +
> > +                       if (idx == index)
> > +                               return 0;
> > +
> > +                       break;
> > +               case ACPI_TYPE_STRING:
> > +                       ref_fwnode =
> > +                               acpi_parse_string_ref(fwnode,
> > +                                                     element->string.pointer);
> > +                       if (IS_ERR(ref_fwnode))
> > +                               return PTR_ERR(ref_fwnode);
> > +
> > +                       element++;
> > +
> > +                       ret = acpi_get_ref_args(idx == index ? args : NULL,
> > +                                               ref_fwnode, &element, end,
> > +                                               num_args, false);
> >                         if (ret < 0)
> >                                 return ret;
> >

-- 
Kind regards,

Sakari Ailus

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

end of thread, other threads:[~2023-03-13 17:02 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-08 21:27 [PATCH v5 0/8] ACPI _CRS CSI-2 and MIPI DisCo for Imaging support Sakari Ailus
2023-02-08 21:27 ` [PATCH v5 1/8] ACPI: property: Parse data node string references in properties Sakari Ailus
2023-03-07 13:30   ` Rafael J. Wysocki
2023-03-13 16:59     ` Sakari Ailus
2023-02-08 21:27 ` [PATCH v5 2/8] ACPI: property: Parse _CRS CSI-2 descriptor Sakari Ailus
2023-02-09  9:47   ` Dan Carpenter
2023-03-07 13:38   ` Rafael J. Wysocki
2023-03-07 19:00     ` Sakari Ailus
2023-03-07 19:02       ` Sakari Ailus
2023-03-07 19:33       ` Sakari Ailus
2023-02-08 21:27 ` [PATCH v5 3/8] device property: Add SOFTWARE_NODE() macro for defining software nodes Sakari Ailus
2023-02-08 21:27 ` [PATCH v5 4/8] ACPI: property: Generate camera swnodes for ACPI and DisCo for Imaging Sakari Ailus
2023-02-08 21:27 ` [PATCH v5 5/8] ACPI: property: Dig "rotation" property for devices with CSI2 _CRS Sakari Ailus
2023-02-08 21:27 ` [PATCH v5 6/8] ACPI: property: Rename parsed MIPI DisCo for Imaging properties Sakari Ailus
2023-02-08 21:27 ` [PATCH v5 7/8] ACPI: property: Skip MIPI property table without "mipi-img" prefix Sakari Ailus
2023-02-08 21:27 ` [PATCH v5 8/8] ACPI: property: Document _CRS CSI-2 and DisCo for Imaging support Sakari Ailus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).