[PATCH v3 0/7] ACPI: scan: MIPI DisCo for Imaging support

[PATCH v3 0/7] ACPI: scan: MIPI DisCo for Imaging support

[Rafael J. Wysocki]

Hi Folks,

This is a new revision of

https://lore.kernel.org/linux-acpi/2187487.irdbgypaU6@kreacher/

addressing comments from Sakari and adding one more patch (also from
Sakari).

The main points from the original cover letter are still valid:

> The general idea is the same - CSI-2 resource descriptors, introduced in
> ACPI 6.4 and defined by
> 
> https://uefi.org/specs/ACPI/6.5/06_Device_Configuration.html#camera-serial-i
> nterface-csi-2-connection-resource-descriptor
> 
> are found and used for creating a set of software nodes that represent the
> CSI-2 connection graph.
> 
> These software nodes need to be available before any scan handlers or ACPI
> drivers are bound to any struct acpi_device objects, so all of that is done
> at the early stage of ACPI device enumeration, but unnecessary ACPI
> namespace walks are avoided.
> 
> The CSI-2 software nodes are populated with data extracted from the CSI-2
> resource descriptors themselves and from device properties defined by the
> MIPI DisCo for Imaging specification (see
> https://www.mipi.org/specifications/mipi-disco-imaging).
> 
> Patches [4,6/6] come from the original series directly, but the other
> patches have been changes substantially, so I've decided to re-start patch
> series versioning from scratch.

The series is based on the current mainline.

Thanks!

[PATCH v3 1/7] ACPI: property: Support using strings in reference properties

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

In order to allow referencing data nodes directly, which is not possible
currently, add support for representing references in device properties
as strings (relative or absolute name paths).  For example, after this
change, the "mipi-img-flash-leds" property in the ASL snippet below will
be treated as a proper reference to the LED0 object under LEDD.

 Package ()
 {
     "mipi-img-flash-leds",  "\\_SB.PCI0.I2C2.LEDD.LED0",
 }

 Device (LEDD)
 {
     Name (_DSD, Package ()  // _DSD: Device-Specific Data
     {
         ToUUID ("dbb8e3e6-5886-4ba6-8795-1319f52a966b"), /* Hierarchical Data Extension */,
         Package ()
         {
             Package ()
             {
                 "mipi-img-flash-led-0", 
                 "LED0",
             }
         },
     })
     Name (LED0, Package ()  // _DSD: Device-Specific Data
     {
         ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties */,
         Package ()
         {
             Package ()
             {
                 "mipi-img-max-current", 
                 1000000,
             }
         }
     })
 }

Also remove the mechanism allowing data nodes to be referenced
indirectly, with the help of an object reference pointing to the
"ancestor" device and a path relative to it (this mechanism is not
expected to be in use in any production platform firmware in the field).

Note that this change allows also using strings for referencing device
objects, in addition to object references that have been supported
already.

While at it, add pr_fmt() macro to prefix printouts and update
copyright.

Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v2 -> v3: No changes

---
 drivers/acpi/property.c |  102 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 82 insertions(+), 20 deletions(-)

Index: linux-pm/drivers/acpi/property.c
===================================================================
--- linux-pm.orig/drivers/acpi/property.c
+++ linux-pm/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>
+ *          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>
@@ -801,27 +804,15 @@ static int acpi_get_ref_args(struct fwno
 	u32 nargs = 0, i;
 
 	/*
-	 * 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;
-	}
-
-	/*
 	 * Assume the following integer elements are all args. Stop counting on
-	 * the first reference or end of the package arguments. In case of
-	 * neither reference, nor integer, return an error, we can't parse it.
+	 * the first reference (possibly represented as a string) or end of the
+	 * package arguments. In case of neither reference, nor integer, return
+	 * an error, we can't parse it.
 	 */
 	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 || type == ACPI_TYPE_STRING)
 			break;
 
 		if (type == ACPI_TYPE_INTEGER)
@@ -845,6 +836,44 @@ static int acpi_get_ref_args(struct fwno
 	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_debug("Bad node type for node %pfw\n", fwnode);
+		return NULL;
+	}
+
+	status = acpi_get_handle(scope, refstring, &handle);
+	if (ACPI_FAILURE(status)) {
+		acpi_handle_debug(scope, "Unable to get an ACPI handle for %s\n",
+				  refstring);
+		return NULL;
+	}
+
+	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, "Subnode not found\n");
+		return NULL;
+	}
+
+	return &dn->fwnode;
+}
+
 /**
  * __acpi_node_get_property_reference - returns handle to the referenced object
  * @fwnode: Firmware node to get the property from
@@ -887,6 +916,7 @@ int __acpi_node_get_property_reference(c
 	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;
 
@@ -910,16 +940,30 @@ int __acpi_node_get_property_reference(c
 
 		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 (!ref_fwnode)
+			return -EINVAL;
+
+		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:
@@ -947,6 +991,24 @@ int __acpi_node_get_property_reference(c
 			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 (!ref_fwnode)
+				return -EINVAL;
+
+			element++;
+
+			ret = acpi_get_ref_args(idx == index ? args : NULL,
+						ref_fwnode, &element, end,
+						num_args);
+			if (ret < 0)
+				return ret;
+
 			if (idx == index)
 				return 0;
 

[PATCH v3 2/7] ACPI: scan: Extract CSI-2 connection graph from _CRS

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Find ACPI CSI-2 resource descriptors defined since ACPI 6.4 (for
CSI-2 and camera configuration) in _CRS for all device objects in
the given scope of the ACPI namespace that have them, identify the
corresponding "remote endpoint" device objects for them and
allocate memory for software nodes needed to create a DT-like data
structure representing the CSI-2 connection graph for drivers.

The code needed to populate these software nodes will be added by
subsequent change sets.

Link: https://uefi.org/specs/ACPI/6.5/06_Device_Configuration.html#camera-serial-interface-csi-2-connection-resource-descriptor
Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v2 -> v3:
   * Change the name of the new file to mipi-disco-img.c
   * Combine check_*_overflow() computations in alloc_crs_csi2_swnodes()

---
 drivers/acpi/Makefile         |    2 
 drivers/acpi/internal.h       |    8 +
 drivers/acpi/mipi-disco-img.c |  292 ++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/scan.c           |   48 +++++-
 include/acpi/acpi_bus.h       |   18 ++
 5 files changed, 358 insertions(+), 10 deletions(-)
 create mode 100644 drivers/acpi/mipi-disco-img.c

Index: linux-pm/drivers/acpi/Makefile
===================================================================
--- linux-pm.orig/drivers/acpi/Makefile
+++ linux-pm/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-disco-img.o
 acpi-y				+= resource.o
 acpi-y				+= acpi_processor.o
 acpi-y				+= processor_core.o
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -281,4 +281,12 @@ void acpi_init_lpit(void);
 static inline void acpi_init_lpit(void) { }
 #endif
 
+/*--------------------------------------------------------------------------
+		ACPI _CRS CSI-2 and MIPI DisCo for Imaging
+  -------------------------------------------------------------------------- */
+
+void acpi_mipi_check_crs_csi2(acpi_handle handle);
+void acpi_mipi_scan_crs_csi2(void);
+void acpi_mipi_crs_csi2_cleanup(void);
+
 #endif /* _ACPI_INTERNAL_H_ */
Index: linux-pm/drivers/acpi/mipi-disco-img.c
===================================================================
--- /dev/null
+++ linux-pm/drivers/acpi/mipi-disco-img.c
@@ -0,0 +1,292 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MIPI DisCo for Imaging support.
+ *
+ * Copyright (C) 2023 Intel Corporation
+ *
+ * Support MIPI DisCo for Imaging by parsing ACPI _CRS CSI-2 records defined in
+ * Section 6.4.3.8.2.4 "Camera Serial Interface (CSI-2) Connection Resource
+ * Descriptor" of ACPI 6.5.
+ *
+ * The implementation looks for the information in the ACPI namespace (CSI-2
+ * resource descriptors in _CRS) and constructs software nodes compatible with
+ * Documentation/firmware-guide/acpi/dsd/graph.rst to represent the CSI-2
+ * connection graph.
+ */
+
+#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/string.h>
+
+#include "internal.h"
+
+static LIST_HEAD(acpi_mipi_crs_csi2_list);
+
+static void acpi_mipi_data_tag(acpi_handle handle, void *context)
+{
+}
+
+/* Connection data extracted from one _CRS CSI-2 resource descriptor. */
+struct crs_csi2_connection {
+	struct list_head entry;
+	struct acpi_resource_csi2_serialbus csi2_data;
+	acpi_handle remote_handle;
+	char remote_name[];
+};
+
+/* Data extracted from _CRS CSI-2 resource descriptors for one device. */
+struct crs_csi2 {
+	struct list_head entry;
+	acpi_handle handle;
+	struct acpi_device_software_nodes *swnodes;
+	struct list_head connections;
+	u32 port_count;
+};
+
+struct csi2_resources_walk_data {
+	acpi_handle handle;
+	struct list_head connections;
+};
+
+static acpi_status parse_csi2_resource(struct acpi_resource *res, void *context)
+{
+	struct csi2_resources_walk_data *crwd = context;
+	struct acpi_resource_csi2_serialbus *csi2_res;
+	struct acpi_resource_source *csi2_res_src;
+	u16 csi2_res_src_length;
+	struct crs_csi2_connection *conn;
+	acpi_handle remote_handle;
+
+	if (res->type != ACPI_RESOURCE_TYPE_SERIAL_BUS)
+		return AE_OK;
+
+	csi2_res = &res->data.csi2_serial_bus;
+
+	if (csi2_res->type != ACPI_RESOURCE_SERIAL_TYPE_CSI2)
+		return AE_OK;
+
+	csi2_res_src = &csi2_res->resource_source;
+	if (ACPI_FAILURE(acpi_get_handle(NULL, csi2_res_src->string_ptr,
+					 &remote_handle))) {
+		acpi_handle_debug(crwd->handle,
+				  "unable to find resource source\n");
+		return AE_OK;
+	}
+	csi2_res_src_length = csi2_res_src->string_length;
+	if (!csi2_res_src_length) {
+		acpi_handle_debug(crwd->handle,
+				  "invalid resource source string length\n");
+		return AE_OK;
+	}
+
+	conn = kmalloc(struct_size(conn, remote_name, csi2_res_src_length + 1),
+		       GFP_KERNEL);
+	if (!conn)
+		return AE_OK;
+
+	conn->csi2_data = *csi2_res;
+	strscpy(conn->remote_name, csi2_res_src->string_ptr, csi2_res_src_length);
+	conn->csi2_data.resource_source.string_ptr = conn->remote_name;
+	conn->remote_handle = remote_handle;
+
+	list_add(&conn->entry, &crwd->connections);
+
+	return AE_OK;
+}
+
+static struct crs_csi2 *acpi_mipi_add_crs_csi2(acpi_handle handle,
+					       struct list_head *list)
+{
+	struct crs_csi2 *csi2;
+
+	csi2 = kzalloc(sizeof(*csi2), GFP_KERNEL);
+	if (!csi2)
+		return NULL;
+
+	csi2->handle = handle;
+	INIT_LIST_HEAD(&csi2->connections);
+	csi2->port_count = 1;
+
+	if (ACPI_FAILURE(acpi_attach_data(handle, acpi_mipi_data_tag, csi2))) {
+		kfree(csi2);
+		return NULL;
+	}
+
+	list_add(&csi2->entry, list);
+
+	return csi2;
+}
+
+static struct crs_csi2 *acpi_mipi_get_crs_csi2(acpi_handle handle)
+{
+	struct crs_csi2 *csi2;
+
+	if (ACPI_FAILURE(acpi_get_data_full(handle, acpi_mipi_data_tag,
+					    (void **)&csi2, NULL)))
+		return NULL;
+
+	return csi2;
+}
+
+static void csi_csr2_release_connections(struct list_head *list)
+{
+	struct crs_csi2_connection *conn, *conn_tmp;
+
+	list_for_each_entry_safe(conn, conn_tmp, list, entry) {
+		list_del(&conn->entry);
+		kfree(conn);
+	}
+}
+
+static void acpi_mipi_del_crs_csi2(struct crs_csi2 *csi2)
+{
+	list_del(&csi2->entry);
+	acpi_detach_data(csi2->handle, acpi_mipi_data_tag);
+	kfree(csi2->swnodes);
+	csi_csr2_release_connections(&csi2->connections);
+	kfree(csi2);
+}
+
+/**
+ * acpi_mipi_check_crs_csi2 - Look for devices with _CRS CSI-2 resources
+ * @handle: ACPI namespace walk starting point.
+ *
+ * Find all devices with _CRS CSI-2 resource descriptors in the ACPI namespace
+ * branch starting at @handle and collect them into a list.
+ */
+void acpi_mipi_check_crs_csi2(acpi_handle handle)
+{
+	struct csi2_resources_walk_data crwd = {
+		.handle = handle,
+		.connections = LIST_HEAD_INIT(crwd.connections),
+	};
+	struct crs_csi2 *csi2;
+
+	/*
+	 * Avoid allocating _CRS CSI-2 objects for devices without any CSI-2
+	 * resource descriptions in _CRS to reduce overhead.
+	 */
+	acpi_walk_resources(handle, METHOD_NAME__CRS, parse_csi2_resource, &crwd);
+	if (list_empty(&crwd.connections))
+		return;
+
+	/*
+	 * Create a _CRS CSI-2 entry to store the extracted connection
+	 * information and add it to the global list.
+	 */
+	csi2 = acpi_mipi_add_crs_csi2(handle, &acpi_mipi_crs_csi2_list);
+	if (!csi2) {
+		csi_csr2_release_connections(&crwd.connections);
+		return; /* Nothing really can be done about this. */
+	}
+
+	list_replace(&crwd.connections, &csi2->connections);
+}
+
+#define NO_CSI2_PORT (UINT_MAX - 1)
+
+static void alloc_crs_csi2_swnodes(struct crs_csi2 *csi2)
+{
+	size_t port_count = csi2->port_count;
+	struct acpi_device_software_nodes *swnodes;
+	size_t alloc_size;
+	unsigned int i;
+
+	/*
+	 * Allocate memory for ports, node pointers (number of nodes +
+	 * 1 (guardian), nodes (root + number of ports * 2 (because for
+	 * every port there is an endpoint)).
+	 */
+	if (check_mul_overflow(sizeof(*swnodes->ports) +
+			       sizeof(*swnodes->nodes) * 2 +
+			       sizeof(*swnodes->nodeptrs) * 2,
+			       port_count, &alloc_size) ||
+	    check_add_overflow(sizeof(*swnodes) +
+			       sizeof(*swnodes->nodes) +
+			       sizeof(*swnodes->nodeptrs) * 2,
+			       alloc_size, &alloc_size)) {
+		acpi_handle_info(csi2->handle,
+				 "too many _CRS CSI-2 resource handles (%zu)",
+				 port_count);
+		return;
+	}
+
+	swnodes = kmalloc(alloc_size, GFP_KERNEL);
+	if (!swnodes)
+		return;
+
+	swnodes->ports = (struct acpi_device_software_node_port *)(swnodes + 1);
+	swnodes->nodes = (struct software_node *)(swnodes->ports + port_count);
+	swnodes->nodeptrs = (const struct software_node **)(swnodes->nodes + 1 +
+				2 * port_count);
+	swnodes->num_ports = port_count;
+
+	for (i = 0; i < 2 * port_count + 1; i++)
+		swnodes->nodeptrs[i] = &swnodes->nodes[i];
+
+	swnodes->nodeptrs[i] = NULL;
+
+	for (i = 0; i < port_count; i++)
+		swnodes->ports[i].port_nr = NO_CSI2_PORT;
+
+	csi2->swnodes = swnodes;
+}
+
+/**
+ * acpi_mipi_scan_crs_csi2 - Create ACPI _CRS CSI-2 software nodes
+ *
+ * Note that this function must be called before any struct acpi_device objects
+ * are bound to any ACPI drivers or scan handlers, so it cannot assume the
+ * existence of struct acpi_device objects for every device present in the ACPI
+ * namespace.
+ *
+ * acpi_scan_lock in scan.c must be held when calling this function.
+ */
+void acpi_mipi_scan_crs_csi2(void)
+{
+	struct crs_csi2 *csi2;
+	LIST_HEAD(aux_list);
+
+	/* Count references to each ACPI handle in the CSI-2 connection graph. */
+	list_for_each_entry(csi2, &acpi_mipi_crs_csi2_list, entry) {
+		struct crs_csi2_connection *conn;
+
+		list_for_each_entry(conn, &csi2->connections, entry) {
+			struct crs_csi2 *remote_csi2;
+
+			csi2->port_count++;
+
+			remote_csi2 = acpi_mipi_get_crs_csi2(conn->remote_handle);
+			if (remote_csi2) {
+				remote_csi2->port_count++;
+				continue;
+			}
+			/*
+			 * The remote endpoint has no _CRS CSI-2 list entry yet,
+			 * so create one for it and add it to the list.
+			 */
+			acpi_mipi_add_crs_csi2(conn->remote_handle, &aux_list);
+		}
+	}
+	list_splice(&aux_list, &acpi_mipi_crs_csi2_list);
+
+	/* Allocate softwware nodes for representing the CSI-2 information. */
+	list_for_each_entry(csi2, &acpi_mipi_crs_csi2_list, entry)
+		alloc_crs_csi2_swnodes(csi2);
+}
+
+/**
+ * acpi_mipi_crs_csi2_cleanup - Free _CRS CSI-2 temporary data
+ */
+void acpi_mipi_crs_csi2_cleanup(void)
+{
+	struct crs_csi2 *csi2, *csi2_tmp;
+
+	list_for_each_entry_safe(csi2, csi2_tmp, &acpi_mipi_crs_csi2_list, entry)
+		acpi_mipi_del_crs_csi2(csi2);
+}
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -366,6 +366,24 @@ struct acpi_device_data {
 
 struct acpi_gpio_mapping;
 
+struct acpi_device_software_node_port {
+	unsigned int port_nr;
+};
+
+/**
+ * struct acpi_device_software_nodes - Software nodes for an ACPI device
+ * @nodes: Software nodes for root as well as ports and endpoints.
+ * @nodeprts: Array of software node pointers, for (un)registering them.
+ * @ports: Information related to each port and endpoint within a port.
+ * @num_ports: The number of ports.
+ */
+struct acpi_device_software_nodes {
+	struct software_node *nodes;
+	const struct software_node **nodeptrs;
+	struct acpi_device_software_node_port *ports;
+	unsigned int num_ports;
+};
+
 /* Device */
 struct acpi_device {
 	u32 pld_crc;
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -1976,7 +1976,7 @@ static void acpi_scan_init_hotplug(struc
 	}
 }
 
-static u32 acpi_scan_check_dep(acpi_handle handle, bool check_dep)
+static u32 acpi_scan_check_dep(acpi_handle handle)
 {
 	struct acpi_handle_list dep_devices;
 	acpi_status status;
@@ -1989,8 +1989,7 @@ static u32 acpi_scan_check_dep(acpi_hand
 	 * 2. ACPI nodes describing USB ports.
 	 * Still, checking for _HID catches more then just these cases ...
 	 */
-	if (!check_dep || !acpi_has_method(handle, "_DEP") ||
-	    !acpi_has_method(handle, "_HID"))
+	if (!acpi_has_method(handle, "_DEP") || !acpi_has_method(handle, "_HID"))
 		return 0;
 
 	status = acpi_evaluate_reference(handle, "_DEP", NULL, &dep_devices);
@@ -2036,7 +2035,13 @@ static u32 acpi_scan_check_dep(acpi_hand
 	return count;
 }
 
-static acpi_status acpi_bus_check_add(acpi_handle handle, bool check_dep,
+static acpi_status acpi_scan_check_crs_csi2_cb(acpi_handle handle, u32 a, void *b, void **c)
+{
+	acpi_mipi_check_crs_csi2(handle);
+	return AE_OK;
+}
+
+static acpi_status acpi_bus_check_add(acpi_handle handle, bool first_pass,
 				      struct acpi_device **adev_p)
 {
 	struct acpi_device *device = acpi_fetch_acpi_dev(handle);
@@ -2054,9 +2059,25 @@ static acpi_status acpi_bus_check_add(ac
 		if (acpi_device_should_be_hidden(handle))
 			return AE_OK;
 
-		/* Bail out if there are dependencies. */
-		if (acpi_scan_check_dep(handle, check_dep) > 0)
-			return AE_CTRL_DEPTH;
+		if (first_pass) {
+			acpi_mipi_check_crs_csi2(handle);
+
+			/* Bail out if there are dependencies. */
+			if (acpi_scan_check_dep(handle) > 0) {
+				/*
+				 * The entire CSI-2 connection graph needs to be
+				 * extracted before any drivers or scan handlers
+				 * are bound to struct device objects, so scan
+				 * _CRS CSI-2 resource descriptors for all
+				 * devices below the current handle.
+				 */
+				acpi_walk_namespace(ACPI_TYPE_DEVICE, handle,
+						    ACPI_UINT32_MAX,
+						    acpi_scan_check_crs_csi2_cb,
+						    NULL, NULL, NULL);
+				return AE_CTRL_DEPTH;
+			}
+		}
 
 		fallthrough;
 	case ACPI_TYPE_ANY:	/* for ACPI_ROOT_OBJECT */
@@ -2079,10 +2100,10 @@ static acpi_status acpi_bus_check_add(ac
 	}
 
 	/*
-	 * If check_dep is true at this point, the device has no dependencies,
+	 * If first_pass is true at this point, the device has no dependencies,
 	 * or the creation of the device object would have been postponed above.
 	 */
-	acpi_add_single_object(&device, handle, type, !check_dep);
+	acpi_add_single_object(&device, handle, type, !first_pass);
 	if (!device)
 		return AE_CTRL_DEPTH;
 
@@ -2494,12 +2515,21 @@ int acpi_bus_scan(acpi_handle handle)
 	if (!device)
 		return -ENODEV;
 
+	/*
+	 * Allocate ACPI _CRS CSI-2 software nodes using information extracted
+	 * from the _CRS CSI-2 resource descriptors during the ACPI namespace
+	 * walk above.
+	 */
+	acpi_mipi_scan_crs_csi2();
+
 	acpi_bus_attach(device, (void *)true);
 
 	/* Pass 2: Enumerate all of the remaining devices. */
 
 	acpi_scan_postponed();
 
+	acpi_mipi_crs_csi2_cleanup();
+
 	return 0;
 }
 EXPORT_SYMBOL(acpi_bus_scan);

Re: [PATCH v3 2/7] ACPI: scan: Extract CSI-2 connection graph from _CRS

[Miao Wang]

2023年11月7日 00:09，Rafael J. Wysocki <rjw@rjwysocki.net> 写道：
> 
>  +static acpi_status acpi_bus_check_add(acpi_handle handle, bool first_pass,
>   				      struct acpi_device **adev_p)
>   {
>   	struct acpi_device *device = acpi_fetch_acpi_dev(handle);
>  @@ -2054,9 +2059,25 @@ static acpi_status acpi_bus_check_add(ac
>   		if (acpi_device_should_be_hidden(handle))
>   			return AE_OK;
>   
>  -		/* Bail out if there are dependencies. */
>  -		if (acpi_scan_check_dep(handle, check_dep) > 0)
>  -			return AE_CTRL_DEPTH;
>  +		if (first_pass) {
>  +			acpi_mipi_check_crs_csi2(handle);
>  +
>  +			/* Bail out if there are dependencies. */
>  +			if (acpi_scan_check_dep(handle) > 0) {
>  +				/*
>  +				 * The entire CSI-2 connection graph needs to be
>  +				 * extracted before any drivers or scan handlers
>  +				 * are bound to struct device objects, so scan
>  +				 * _CRS CSI-2 resource descriptors for all
>  +				 * devices below the current handle.
>  +				 */
>  +				acpi_walk_namespace(ACPI_TYPE_DEVICE, handle,
>  +						    ACPI_UINT32_MAX,
>  +						    acpi_scan_check_crs_csi2_cb,
>  +						    NULL, NULL, NULL);
>  +				return AE_CTRL_DEPTH;
>  +			}
>  +		}
>   
>   		fallthrough;
>   	case ACPI_TYPE_ANY:	/* for ACPI_ROOT_OBJECT */
> 

Hi, I'd like to report some issues caused by this patch. Correct me if I'm wrong
since I'm not expert on ACPI. Before this patch, properties of ACPI devices with
_DEP relationship declared were evaluated after the initialization of the
depending devices, i.e. the execution of handler->attach(). With this patch,
_CRS of all the ACPI devices are evaluated to check if the device contains a
CSI2 resource, regardless of the declaration of _DEP relationship. The
evaluation of _CRS is even before _STA and other methods indicating the status
of the device.

This has caused some issues in certain scenarios, where the DSDT contains legacy
devices, which requires reading IO ports to form the result of its _CRS. An
example of such a legacy device is declared in the DSDT is as below:

    Device (LPT)
    {
        Name (_HID, EisaId ("PNP0400") /* Standard LPT Parallel Port */)
        Name (_UID, 0x02)  // _UID: Unique ID
        Name (_DDN, "LPT1")  // _DDN: DOS Device Name
        Name (_DEP, Package (0x01)  // _DEP: Dependencies
        {
            \_SB.PCI0
        })
        OperationRegion (ITE1, SystemIO, 0x2E, 0x02)
        Field (ITE1, ByteAcc, NoLock, Preserve) {INDX, 8,   DATA, 8}
        IndexField (INDX, DATA, ByteAcc, NoLock, Preserve)
        {
            // Omitting some declarations of fields
            IOAH,   8,
            IOAL,   8,
        }
        Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
        {
            Name (BUF0, ResourceTemplate ()
            {
                IO (Decode16,
                    0x0378,             // Range Minimum
                    0x0378,             // Range Maximum
                    0x01,               // Alignment
                    0x08,               // Length
                    _Y01)
                IRQ (Level, ActiveLow, Exclusive, _Y02)
                    {7}
            })
            CreateByteField (BUF0, \LPT._CRS._Y01._MIN, IOLO)
            CreateByteField (BUF0, 0x03, IOHI)
            CreateByteField (BUF0, \LPT._CRS._Y01._MAX, IORL)
            CreateByteField (BUF0, 0x05, IORH)
            CreateByteField (BUF0, \LPT._CRS._Y01._LEN, LEN1)
            CreateByteField (BUF0, \LPT._CRS._Y02._INT, IRQL) 
            
            IOLO = IOAL /* \LPT_.IOAL */
            IORL = IOAL /* \LPT_.IOAL */
            IOHI = IOAH /* \LPT_.IOAH */
            IORH = IOAH /* \LPT_.IOAH */
            // Omitting some assignments and calculations
            
            Return (BUF0)
        }
    }

On non-x86 platforms, IO ports are implemented using MMIO. The memory is
remapped in the initialization of the PCI root, using its QWordIO resource
declared in its _CRS, in acpi_pci_root_remap_iospace(). Before the
initialization of the PCI root, reads and writes of the IO ports will result in
accessing memory region which is not mapped.

Before this patch, adding a _DEP of Package(){PCI0} to these legacy devices will
postpone the initialization of these devices after the PCI root, solving the
above problem. After this patch, however, the evaluation of _CRS regardless of
_DEP declarations causes accessing IO ports before they are ready.

I've checked the ACPI spec, and it states that ``OSPM must guarantee that the
following operation regions are always accessible: SystemIO operation regions"
and ``Since the region types above are permanently available, no _REG methods
are required, nor will OSPM evaluate any _REG methods that appear in the same
scope as the operation region declaration(s) of these types". It seems that
from the view of the AML codes in the DSDT, it can never know when the IO ports
are ready, and it is impossible for Linux on non-x86 platforms to ensure IO
ports are always accessible.

I wonder if there would be a better solution to this problem.

Cheers,

Miao Wang

Re: [PATCH v3 2/7] ACPI: scan: Extract CSI-2 connection graph from _CRS

[Rafael J. Wysocki]

Hi,

Sorry for the delay.

On Thu, Nov 21, 2024 at 10:57 PM Miao Wang <shankerwangmiao@gmail.com> wrote:
>
> 2023年11月7日 00:09，Rafael J. Wysocki <rjw@rjwysocki.net> 写道：
> >
> >  +static acpi_status acpi_bus_check_add(acpi_handle handle, bool first_pass,
> >                                     struct acpi_device **adev_p)
> >   {
> >       struct acpi_device *device = acpi_fetch_acpi_dev(handle);
> >  @@ -2054,9 +2059,25 @@ static acpi_status acpi_bus_check_add(ac
> >               if (acpi_device_should_be_hidden(handle))
> >                       return AE_OK;
> >
> >  -            /* Bail out if there are dependencies. */
> >  -            if (acpi_scan_check_dep(handle, check_dep) > 0)
> >  -                    return AE_CTRL_DEPTH;
> >  +            if (first_pass) {
> >  +                    acpi_mipi_check_crs_csi2(handle);
> >  +
> >  +                    /* Bail out if there are dependencies. */
> >  +                    if (acpi_scan_check_dep(handle) > 0) {
> >  +                            /*
> >  +                             * The entire CSI-2 connection graph needs to be
> >  +                             * extracted before any drivers or scan handlers
> >  +                             * are bound to struct device objects, so scan
> >  +                             * _CRS CSI-2 resource descriptors for all
> >  +                             * devices below the current handle.
> >  +                             */
> >  +                            acpi_walk_namespace(ACPI_TYPE_DEVICE, handle,
> >  +                                                ACPI_UINT32_MAX,
> >  +                                                acpi_scan_check_crs_csi2_cb,
> >  +                                                NULL, NULL, NULL);
> >  +                            return AE_CTRL_DEPTH;
> >  +                    }
> >  +            }
> >
> >               fallthrough;
> >       case ACPI_TYPE_ANY:     /* for ACPI_ROOT_OBJECT */
> >
>
> Hi, I'd like to report some issues caused by this patch. Correct me if I'm wrong
> since I'm not expert on ACPI. Before this patch, properties of ACPI devices with
> _DEP relationship declared were evaluated after the initialization of the
> depending devices, i.e. the execution of handler->attach(). With this patch,
> _CRS of all the ACPI devices are evaluated to check if the device contains a
> CSI2 resource, regardless of the declaration of _DEP relationship.

Yes, but the _DEP information is not relevant for whether or not _CRS
may be evaluated at all.

> The evaluation of _CRS is even before _STA

Not really.  _STA has already been evaluated by
acpi_ns_init_one_device() for all devices at this point.

> and other methods indicating the status of the device.

No, but it doesn't matter, at least by the spec.  Had the device been
disabled, _CRS would have been expected to work anyway:

"If a device is disabled, then _CRS returns a valid resource template
for the device, but the actual resource assignments in the return byte
stream are ignored." (ACPI 6.5, Section 6.2.2. _CRS (Current Resource
Settings)).

That said, adding a device status check before the _CRS evaluation in
acpi_bus_check_add() is not inconceivable.

> This has caused some issues in certain scenarios, where the DSDT contains legacy
> devices, which requires reading IO ports to form the result of its _CRS. An
> example of such a legacy device is declared in the DSDT is as below:
>
>     Device (LPT)
>     {
>         Name (_HID, EisaId ("PNP0400") /* Standard LPT Parallel Port */)
>         Name (_UID, 0x02)  // _UID: Unique ID
>         Name (_DDN, "LPT1")  // _DDN: DOS Device Name
>         Name (_DEP, Package (0x01)  // _DEP: Dependencies
>         {
>             \_SB.PCI0
>         })
>         OperationRegion (ITE1, SystemIO, 0x2E, 0x02)
>         Field (ITE1, ByteAcc, NoLock, Preserve) {INDX, 8,   DATA, 8}
>         IndexField (INDX, DATA, ByteAcc, NoLock, Preserve)
>         {
>             // Omitting some declarations of fields
>             IOAH,   8,
>             IOAL,   8,
>         }
>         Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
>         {
>             Name (BUF0, ResourceTemplate ()
>             {
>                 IO (Decode16,
>                     0x0378,             // Range Minimum
>                     0x0378,             // Range Maximum
>                     0x01,               // Alignment
>                     0x08,               // Length
>                     _Y01)
>                 IRQ (Level, ActiveLow, Exclusive, _Y02)
>                     {7}
>             })
>             CreateByteField (BUF0, \LPT._CRS._Y01._MIN, IOLO)
>             CreateByteField (BUF0, 0x03, IOHI)
>             CreateByteField (BUF0, \LPT._CRS._Y01._MAX, IORL)
>             CreateByteField (BUF0, 0x05, IORH)
>             CreateByteField (BUF0, \LPT._CRS._Y01._LEN, LEN1)
>             CreateByteField (BUF0, \LPT._CRS._Y02._INT, IRQL)
>
>             IOLO = IOAL /* \LPT_.IOAL */
>             IORL = IOAL /* \LPT_.IOAL */
>             IOHI = IOAH /* \LPT_.IOAH */
>             IORH = IOAH /* \LPT_.IOAH */
>             // Omitting some assignments and calculations
>
>             Return (BUF0)
>         }
>     }
>
> On non-x86 platforms, IO ports are implemented using MMIO. The memory is
> remapped in the initialization of the PCI root, using its QWordIO resource
> declared in its _CRS, in acpi_pci_root_remap_iospace(). Before the
> initialization of the PCI root, reads and writes of the IO ports will result in
> accessing memory region which is not mapped.

That's not really straightforward.

> Before this patch, adding a _DEP of Package(){PCI0} to these legacy devices will
> postpone the initialization of these devices after the PCI root, solving the
> above problem. After this patch, however, the evaluation of _CRS regardless of
> _DEP declarations causes accessing IO ports before they are ready.

Well, before this patch, Linux simply did not have to evaluate _CRS
during device discovery.

Nowhere in the spec it is stated that _CRS cannot be evaluated for a
given device before enumerating all devices listed by its _DEP object.
As it is currently defined, _DEP is only about operation region
dependencies, not even about general device enumeration ordering (even
though it is used for enforcing specific enumeration ordering in the
field because OSes tend to enumerate devices in the order following
from _DEP as a matter of fact).

> I've checked the ACPI spec, and it states that ``OSPM must guarantee that the
> following operation regions are always accessible: SystemIO operation regions"

Which to me is clearly at odds with the SystemIO implementation
description above.

> and ``Since the region types above are permanently available, no _REG methods
> are required, nor will OSPM evaluate any _REG methods that appear in the same
> scope as the operation region declaration(s) of these types". It seems that
> from the view of the AML codes in the DSDT, it can never know when the IO ports
> are ready, and it is impossible for Linux on non-x86 platforms to ensure IO
> ports are always accessible.

They aren't always accessible anyhow.  They become accessible after
enumerating the PCI host bridge and they aren't accessible earlier.

> I wonder if there would be a better solution to this problem.

Well, it is a bit unfortunate that it took 6 kernel release cycles to
realize that there was a problem.  Had this been reported earlier,
there would have been more options on the table.

At this point, the functionality related to CSI-2 connection graphs
simply needs to be there, the only option for avoiding _CRS evaluation
in acpi_bus_check_add() would be to somehow defer the enumeration of
all devices using CSI-2 connections until the host bridge is
enumerated.  Alternatively, the enumeration of the PCI host bridge
might be pushed forward, but that would require addressing at least a
few enumeration ordering issues.  None of these would be a small
change and backporting any of that would have been a considerable
effort.

One of the things worth considering is whether or not CSI-2 is
relevant for the architectures that rely on the old behavior related
to _DEP.  If not at all, a Kconfig option could be added to disable
CSI-2 graphs enumeration on those architectures and that would make
the problem go away.

Also I'm wondering if the firmware could be updated to survive a mere
evaluation of _CRS if the hardware is not ready.

Thanks!

Re: [PATCH v3 2/7] ACPI: scan: Extract CSI-2 connection graph from _CRS

[Miao Wang]

> 2024年12月4日 03:44，Rafael J. Wysocki <rafael@kernel.org> 写道：
> 
> Hi,
> 
> Sorry for the delay.
> 
> On Thu, Nov 21, 2024 at 10:57 PM Miao Wang <shankerwangmiao@gmail.com> wrote:
>> 
>> 2023年11月7日 00:09，Rafael J. Wysocki <rjw@rjwysocki.net> 写道：
>>> 
>>> +static acpi_status acpi_bus_check_add(acpi_handle handle, bool first_pass,
>>>                                    struct acpi_device **adev_p)
>>>  {
>>>      struct acpi_device *device = acpi_fetch_acpi_dev(handle);
>>> @@ -2054,9 +2059,25 @@ static acpi_status acpi_bus_check_add(ac
>>>              if (acpi_device_should_be_hidden(handle))
>>>                      return AE_OK;
>>> 
>>> -            /* Bail out if there are dependencies. */
>>> -            if (acpi_scan_check_dep(handle, check_dep) > 0)
>>> -                    return AE_CTRL_DEPTH;
>>> +            if (first_pass) {
>>> +                    acpi_mipi_check_crs_csi2(handle);
>>> +
>>> +                    /* Bail out if there are dependencies. */
>>> +                    if (acpi_scan_check_dep(handle) > 0) {
>>> +                            /*
>>> +                             * The entire CSI-2 connection graph needs to be
>>> +                             * extracted before any drivers or scan handlers
>>> +                             * are bound to struct device objects, so scan
>>> +                             * _CRS CSI-2 resource descriptors for all
>>> +                             * devices below the current handle.
>>> +                             */
>>> +                            acpi_walk_namespace(ACPI_TYPE_DEVICE, handle,
>>> +                                                ACPI_UINT32_MAX,
>>> +                                                acpi_scan_check_crs_csi2_cb,
>>> +                                                NULL, NULL, NULL);
>>> +                            return AE_CTRL_DEPTH;
>>> +                    }
>>> +            }
>>> 
>>>              fallthrough;
>>>      case ACPI_TYPE_ANY:     /* for ACPI_ROOT_OBJECT */
>>> 
>> 
>> Hi, I'd like to report some issues caused by this patch. Correct me if I'm wrong
>> since I'm not expert on ACPI. Before this patch, properties of ACPI devices with
>> _DEP relationship declared were evaluated after the initialization of the
>> depending devices, i.e. the execution of handler->attach(). With this patch,
>> _CRS of all the ACPI devices are evaluated to check if the device contains a
>> CSI2 resource, regardless of the declaration of _DEP relationship.
> 
> Yes, but the _DEP information is not relevant for whether or not _CRS
> may be evaluated at all.
> 
>> The evaluation of _CRS is even before _STA
> 
> Not really.  _STA has already been evaluated by
> acpi_ns_init_one_device() for all devices at this point.

I quickly scanned the code in acpi_ns_init_one_device(), and it seems
that it an object does not have _INI method, then the _STA will not be
evaluated here.

> 
>> and other methods indicating the status of the device.
> 
> No, but it doesn't matter, at least by the spec.  Had the device been
> disabled, _CRS would have been expected to work anyway:
> 
> "If a device is disabled, then _CRS returns a valid resource template
> for the device, but the actual resource assignments in the return byte
> stream are ignored." (ACPI 6.5, Section 6.2.2. _CRS (Current Resource
> Settings)).
> 
> That said, adding a device status check before the _CRS evaluation in
> acpi_bus_check_add() is not inconceivable.
> 
>> This has caused some issues in certain scenarios, where the DSDT contains legacy
>> devices, which requires reading IO ports to form the result of its _CRS. An
>> example of such a legacy device is declared in the DSDT is as below:
>> 
>>    Device (LPT)
>>    {
>>        Name (_HID, EisaId ("PNP0400") /* Standard LPT Parallel Port */)
>>        Name (_UID, 0x02)  // _UID: Unique ID
>>        Name (_DDN, "LPT1")  // _DDN: DOS Device Name
>>        Name (_DEP, Package (0x01)  // _DEP: Dependencies
>>        {
>>            \_SB.PCI0
>>        })
>>        OperationRegion (ITE1, SystemIO, 0x2E, 0x02)
>>        Field (ITE1, ByteAcc, NoLock, Preserve) {INDX, 8,   DATA, 8}
>>        IndexField (INDX, DATA, ByteAcc, NoLock, Preserve)
>>        {
>>            // Omitting some declarations of fields
>>            IOAH,   8,
>>            IOAL,   8,
>>        }
>>        Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
>>        {
>>            Name (BUF0, ResourceTemplate ()
>>            {
>>                IO (Decode16,
>>                    0x0378,             // Range Minimum
>>                    0x0378,             // Range Maximum
>>                    0x01,               // Alignment
>>                    0x08,               // Length
>>                    _Y01)
>>                IRQ (Level, ActiveLow, Exclusive, _Y02)
>>                    {7}
>>            })
>>            CreateByteField (BUF0, \LPT._CRS._Y01._MIN, IOLO)
>>            CreateByteField (BUF0, 0x03, IOHI)
>>            CreateByteField (BUF0, \LPT._CRS._Y01._MAX, IORL)
>>            CreateByteField (BUF0, 0x05, IORH)
>>            CreateByteField (BUF0, \LPT._CRS._Y01._LEN, LEN1)
>>            CreateByteField (BUF0, \LPT._CRS._Y02._INT, IRQL)
>> 
>>            IOLO = IOAL /* \LPT_.IOAL */
>>            IORL = IOAL /* \LPT_.IOAL */
>>            IOHI = IOAH /* \LPT_.IOAH */
>>            IORH = IOAH /* \LPT_.IOAH */
>>            // Omitting some assignments and calculations
>> 
>>            Return (BUF0)
>>        }
>>    }
>> 
>> On non-x86 platforms, IO ports are implemented using MMIO. The memory is
>> remapped in the initialization of the PCI root, using its QWordIO resource
>> declared in its _CRS, in acpi_pci_root_remap_iospace(). Before the
>> initialization of the PCI root, reads and writes of the IO ports will result in
>> accessing memory region which is not mapped.
> 
> That's not really straightforward.
> 
>> Before this patch, adding a _DEP of Package(){PCI0} to these legacy devices will
>> postpone the initialization of these devices after the PCI root, solving the
>> above problem. After this patch, however, the evaluation of _CRS regardless of
>> _DEP declarations causes accessing IO ports before they are ready.
> 
> Well, before this patch, Linux simply did not have to evaluate _CRS
> during device discovery.
> 
> Nowhere in the spec it is stated that _CRS cannot be evaluated for a
> given device before enumerating all devices listed by its _DEP object.
> As it is currently defined, _DEP is only about operation region
> dependencies, not even about general device enumeration ordering (even
> though it is used for enforcing specific enumeration ordering in the
> field because OSes tend to enumerate devices in the order following
> from _DEP as a matter of fact).
> 
>> I've checked the ACPI spec, and it states that ``OSPM must guarantee that the
>> following operation regions are always accessible: SystemIO operation regions"
> 
> Which to me is clearly at odds with the SystemIO implementation
> description above.
> 
>> and ``Since the region types above are permanently available, no _REG methods
>> are required, nor will OSPM evaluate any _REG methods that appear in the same
>> scope as the operation region declaration(s) of these types". It seems that
>> from the view of the AML codes in the DSDT, it can never know when the IO ports
>> are ready, and it is impossible for Linux on non-x86 platforms to ensure IO
>> ports are always accessible.
> 
> They aren't always accessible anyhow.  They become accessible after
> enumerating the PCI host bridge and they aren't accessible earlier.
> 
>> I wonder if there would be a better solution to this problem.
> 
> Well, it is a bit unfortunate that it took 6 kernel release cycles to
> realize that there was a problem.  Had this been reported earlier,
> there would have been more options on the table.

Yeah, since it is rare to keep a legacy device on non-X86 platforms,
and as a result, this issue is not discovered until now.

> 
> At this point, the functionality related to CSI-2 connection graphs
> simply needs to be there, the only option for avoiding _CRS evaluation
> in acpi_bus_check_add() would be to somehow defer the enumeration of
> all devices using CSI-2 connections until the host bridge is
> enumerated.  Alternatively, the enumeration of the PCI host bridge
> might be pushed forward, but that would require addressing at least a
> few enumeration ordering issues.  None of these would be a small
> change and backporting any of that would have been a considerable
> effort.
> 
> One of the things worth considering is whether or not CSI-2 is
> relevant for the architectures that rely on the old behavior related
> to _DEP.  If not at all, a Kconfig option could be added to disable
> CSI-2 graphs enumeration on those architectures and that would make
> the problem go away.

I'm not familiar with ACPI. However, in my personal point of view, it
seems that the root cause of the issue is in the ACPI standard, which
assumes SystemIO operation regions are always available, which is not
the case on non-X86 platforms. Also, there is no mechanism for the DSDT
code to know whether SystemIO operation regions are available, and it
is thus impossible to return ``a valid resource template" in the _CRS
method when SystemIO operation regions are unavailable. So it is not
the architectures that want to rely on the old behavior related to
_DEP, but all non-x86 architectures with ACPI support may have no
choice but to rely on this to postpone possible IO port operations after
the initialization of the PCI host bridge. However, on non-x86
architectures, there might be limited number of machines with devices
using legacy IO operations, so this issue is currently not so significant.

The documents for CSI-2 give me an impression that it is related to a
certain kind of camera, so I don't think it is good to remove this for
specific architectures.

> Also I'm wondering if the firmware could be updated to survive a mere
> evaluation of _CRS if the hardware is not ready.

If we only consider the current ACPI standard, I cannot come up with
a solution on the firmware side, as discussed before. But utilizing
(or abusing?) the current Linux implementation details, I managed to
work this around in this way:

Device (LPT)
{
    //...

    Name (STAD, Zero)
    Name (_DEP, Package (0x01)
    {
        \_SB.PCI0
    })

    Method (_STA, 0, NotSerialized)
    {
        STAD = One
        // other codes
    }
    
    Method (_CRS, 0, NotSerialized)
    {
        Name (BUF0,/*....*/)

        // Initialize BUF0

        If (STAD)
        {
            // do actual IO operations and fill BUF0 with actual
            // numbers
        }
        
        Return (BUF0)
    }
}

This work around is because I noticed the _STA method is called after
the depending PCI0 is initialized, so set a flag in the _STA method to
mark the availability of the IO port operations.

Maybe we can prioritize the initialization of the PCI host bridge to
fully eliminate this issue?

Cheers,

Miao Wang

Re: [PATCH v3 2/7] ACPI: scan: Extract CSI-2 connection graph from _CRS

[Rafael J. Wysocki]

On Thu, Dec 5, 2024 at 8:24 AM Miao Wang <shankerwangmiao@gmail.com> wrote:
>
>
> > 2024年12月4日 03:44，Rafael J. Wysocki <rafael@kernel.org> 写道：
> >
> > Hi,
> >
> > Sorry for the delay.
> >
> > On Thu, Nov 21, 2024 at 10:57 PM Miao Wang <shankerwangmiao@gmail.com> wrote:
> >>
> >> 2023年11月7日 00:09，Rafael J. Wysocki <rjw@rjwysocki.net> 写道：
> >>>
> >>> +static acpi_status acpi_bus_check_add(acpi_handle handle, bool first_pass,
> >>>                                    struct acpi_device **adev_p)
> >>>  {
> >>>      struct acpi_device *device = acpi_fetch_acpi_dev(handle);
> >>> @@ -2054,9 +2059,25 @@ static acpi_status acpi_bus_check_add(ac
> >>>              if (acpi_device_should_be_hidden(handle))
> >>>                      return AE_OK;
> >>>
> >>> -            /* Bail out if there are dependencies. */
> >>> -            if (acpi_scan_check_dep(handle, check_dep) > 0)
> >>> -                    return AE_CTRL_DEPTH;
> >>> +            if (first_pass) {
> >>> +                    acpi_mipi_check_crs_csi2(handle);
> >>> +
> >>> +                    /* Bail out if there are dependencies. */
> >>> +                    if (acpi_scan_check_dep(handle) > 0) {
> >>> +                            /*
> >>> +                             * The entire CSI-2 connection graph needs to be
> >>> +                             * extracted before any drivers or scan handlers
> >>> +                             * are bound to struct device objects, so scan
> >>> +                             * _CRS CSI-2 resource descriptors for all
> >>> +                             * devices below the current handle.
> >>> +                             */
> >>> +                            acpi_walk_namespace(ACPI_TYPE_DEVICE, handle,
> >>> +                                                ACPI_UINT32_MAX,
> >>> +                                                acpi_scan_check_crs_csi2_cb,
> >>> +                                                NULL, NULL, NULL);
> >>> +                            return AE_CTRL_DEPTH;
> >>> +                    }
> >>> +            }
> >>>
> >>>              fallthrough;
> >>>      case ACPI_TYPE_ANY:     /* for ACPI_ROOT_OBJECT */
> >>>
> >>
> >> Hi, I'd like to report some issues caused by this patch. Correct me if I'm wrong
> >> since I'm not expert on ACPI. Before this patch, properties of ACPI devices with
> >> _DEP relationship declared were evaluated after the initialization of the
> >> depending devices, i.e. the execution of handler->attach(). With this patch,
> >> _CRS of all the ACPI devices are evaluated to check if the device contains a
> >> CSI2 resource, regardless of the declaration of _DEP relationship.

[cut]

>
> Maybe we can prioritize the initialization of the PCI host bridge to
> fully eliminate this issue?

The problem with this is that the current code requires struct
acpi_device objects to be present for all PCI devices that have
corresponding objects in the ACPI Namespace at the time when the host
bridge is initialized because that causes the PCI bus to be scanned
for devices and struct acpi_device objects are looked up from there.

To make this work, the "ACPI companion lookup" code needs to be
changed and that would be kind of a heavy lifting and it may introduce
some unexpected enumeration ordering issues.

Alternatively, the PCI host bridge could be initialized early, but
without scanning the PCI bus which would be scanned at the time when
all of the struct acpi_device objects are present.  It looks like this
could be made work, but it would require some investigation and code
refactoring.

Re: [PATCH v3 2/7] ACPI: scan: Extract CSI-2 connection graph from _CRS

[Miao Wang]

> 2024年12月5日 18:54，Rafael J. Wysocki <rafael@kernel.org> 写道：
> 
>> 
>> Maybe we can prioritize the initialization of the PCI host bridge to
>> fully eliminate this issue?
> 
> The problem with this is that the current code requires struct
> acpi_device objects to be present for all PCI devices that have
> corresponding objects in the ACPI Namespace at the time when the host
> bridge is initialized because that causes the PCI bus to be scanned
> for devices and struct acpi_device objects are looked up from there.
> 
> To make this work, the "ACPI companion lookup" code needs to be
> changed and that would be kind of a heavy lifting and it may introduce
> some unexpected enumeration ordering issues.
> 
> Alternatively, the PCI host bridge could be initialized early, but
> without scanning the PCI bus which would be scanned at the time when
> all of the struct acpi_device objects are present.  It looks like this
> could be made work, but it would require some investigation and code
> refactoring.

Thanks again for your explanation on this. I understand that it may need
lots of work. I think it would be better if the fix would become as minor
as possible, because it would be easier to backport and more importantly,
the case where legacy IO device is used on non-x86 ACPI-enabled
architectures is really rare.

I'm currently running out of better ideas, due to my limited experience
on this part of code. I suppose maybe we can scan the ACPI device tree
in three passes, adding one in the beginning to pick out the PCI Host.
But we should deal with the case where evaluating the _CRS of the PCI
Host itself requires reading IO ports, since it seems that the
specification does not forbid this...

If you are familiar with someone in the UEFI forum, maybe this problem
can be raised to them and a clarification can be requested.

Cheers,

Miao Wang

[PATCH v3 3/7] ACPI: scan: Extract _CRS CSI-2 connection information into swnodes

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Use the connection information extracted from the _CRS CSI-2 resource
descriptors for all devices that have them to populate port names and the
"reg", "bus-type" and "remote-endpoint" properties in the software nodes
representing the CSI-2 connection graph.

Link: https://uefi.org/specs/ACPI/6.5/06_Device_Configuration.html#camera-serial-interface-csi-2-connection-resource-descriptor
Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v2 -> v3:
   * Change the name of the new file to mipi-disco-img.c
   * Change the ACPI_DEVICE_CSI2_DATA_LANES value to 8

---
 drivers/acpi/mipi-disco-img.c |  153 +++++++++++++++++++++++++++++++++++++++++-
 include/acpi/acpi_bus.h       |   53 ++++++++++++++
 2 files changed, 205 insertions(+), 1 deletion(-)

Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -366,8 +366,61 @@ struct acpi_device_data {
 
 struct acpi_gpio_mapping;
 
+#define ACPI_DEVICE_CSI2_DATA_LANES		8
+
+#define ACPI_DEVICE_SWNODE_PORT_NAME_LENGTH	8
+
+enum acpi_device_swnode_port_props {
+	ACPI_DEVICE_SWNODE_PORT_REG,
+	ACPI_DEVICE_SWNODE_PORT_NUM_OF,
+	ACPI_DEVICE_SWNODE_PORT_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
+};
+
+/*
+ * Each device has a root software node plus two times as many nodes as the
+ * number of CSI-2 ports.
+ */
+#define ACPI_DEVICE_SWNODE_PORT(port)	(2 * (port) + 1)
+#define ACPI_DEVICE_SWNODE_EP(endpoint)	\
+		(ACPI_DEVICE_SWNODE_PORT(endpoint) + 1)
+
+/**
+ * struct acpi_device_software_node_port - MIPI DisCo for Imaging CSI-2 port
+ * @port_name: Port name.
+ * @data_lanes: "data-lanes" property values.
+ * @lane_polarities: "lane-polarities" property values.
+ * @link_frequencies: "link_frequencies" property values.
+ * @port_nr: Port number.
+ * @crs_crs2_local: _CRS CSI2 record present (i.e. this is a transmitter one).
+ * @port_props: Port properties.
+ * @ep_props: Endpoint properties.
+ * @remote_ep: Reference to the remote endpoint.
+ */
 struct acpi_device_software_node_port {
+	char port_name[ACPI_DEVICE_SWNODE_PORT_NAME_LENGTH + 1];
+	u32 data_lanes[ACPI_DEVICE_CSI2_DATA_LANES];
+	u32 lane_polarities[ACPI_DEVICE_CSI2_DATA_LANES + 1 /* clock lane */];
+	u64 link_frequencies[ACPI_DEVICE_CSI2_DATA_LANES];
 	unsigned int port_nr;
+	bool crs_csi2_local;
+
+	struct property_entry port_props[ACPI_DEVICE_SWNODE_PORT_NUM_ENTRIES];
+	struct property_entry ep_props[ACPI_DEVICE_SWNODE_EP_NUM_ENTRIES];
+
+	struct software_node_ref_args remote_ep[1];
 };
 
 /**
Index: linux-pm/drivers/acpi/mipi-disco-img.c
===================================================================
--- linux-pm.orig/drivers/acpi/mipi-disco-img.c
+++ linux-pm/drivers/acpi/mipi-disco-img.c
@@ -23,6 +23,8 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 
+#include <media/v4l2-fwnode.h>
+
 #include "internal.h"
 
 static LIST_HEAD(acpi_mipi_crs_csi2_list);
@@ -237,6 +239,142 @@ static void alloc_crs_csi2_swnodes(struc
 	csi2->swnodes = swnodes;
 }
 
+#define ACPI_CRS_CSI2_PHY_TYPE_C	0
+#define ACPI_CRS_CSI2_PHY_TYPE_D	1
+
+static unsigned int next_csi2_port_index(struct acpi_device_software_nodes *swnodes,
+					 unsigned int port_nr)
+{
+	unsigned int i;
+
+	for (i = 0; i < swnodes->num_ports; i++) {
+		struct acpi_device_software_node_port *port = &swnodes->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 on failure. */
+#define GRAPH_PORT_NAME(var, num)					    \
+	(snprintf((var), sizeof(var), SWNODE_GRAPH_PORT_NAME_FMT, (num)) >= \
+	 sizeof(var))
+
+static void extract_crs_csi2_conn_info(acpi_handle local_handle,
+				       struct acpi_device_software_nodes *local_swnodes,
+				       struct crs_csi2_connection *conn)
+{
+	struct crs_csi2 *remote_csi2 = acpi_mipi_get_crs_csi2(conn->remote_handle);
+	struct acpi_device_software_nodes *remote_swnodes;
+	struct acpi_device_software_node_port *local_port, *remote_port;
+	struct software_node *local_node, *remote_node;
+	unsigned int local_index, remote_index;
+	unsigned int bus_type;
+
+	/*
+	 * If the previous steps have failed to make room for a _CRS CSI-2
+	 * representation for the remote end of the given connection, skip it.
+	 */
+	if (!remote_csi2)
+		return;
+
+	remote_swnodes = remote_csi2->swnodes;
+	if (!remote_swnodes)
+		return;
+
+	switch (conn->csi2_data.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(local_handle, "unknown CSI-2 PHY type %u\n",
+				 conn->csi2_data.phy_type);
+		return;
+	}
+
+	local_index = next_csi2_port_index(local_swnodes,
+					   conn->csi2_data.local_port_instance);
+	if (WARN_ON_ONCE(local_index >= local_swnodes->num_ports))
+		return;
+
+	remote_index = next_csi2_port_index(remote_swnodes,
+					    conn->csi2_data.resource_source.index);
+	if (WARN_ON_ONCE(remote_index >= remote_swnodes->num_ports))
+		return;
+
+	local_port = &local_swnodes->ports[local_index];
+	local_node = &local_swnodes->nodes[ACPI_DEVICE_SWNODE_EP(local_index)];
+	local_port->crs_csi2_local = true;
+
+	remote_port = &remote_swnodes->ports[remote_index];
+	remote_node = &remote_swnodes->nodes[ACPI_DEVICE_SWNODE_EP(remote_index)];
+
+	local_port->remote_ep[0] = SOFTWARE_NODE_REFERENCE(remote_node);
+	remote_port->remote_ep[0] = SOFTWARE_NODE_REFERENCE(local_node);
+
+	local_port->ep_props[ACPI_DEVICE_SWNODE_EP_REMOTE_EP] =
+			PROPERTY_ENTRY_REF_ARRAY("remote-endpoint",
+						 local_port->remote_ep);
+
+	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_PORT_REG] =
+			PROPERTY_ENTRY_U32("reg", conn->csi2_data.local_port_instance);
+
+	if (GRAPH_PORT_NAME(local_port->port_name,
+			    conn->csi2_data.local_port_instance))
+		acpi_handle_info(local_handle, "local port %u name too long",
+				 conn->csi2_data.local_port_instance);
+
+	remote_port->ep_props[ACPI_DEVICE_SWNODE_EP_REMOTE_EP] =
+			PROPERTY_ENTRY_REF_ARRAY("remote-endpoint",
+						 remote_port->remote_ep);
+
+	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_PORT_REG] =
+			PROPERTY_ENTRY_U32("reg", conn->csi2_data.resource_source.index);
+
+	if (GRAPH_PORT_NAME(remote_port->port_name,
+			    conn->csi2_data.resource_source.index))
+		acpi_handle_info(local_handle, "remote port %u name too long",
+				 conn->csi2_data.resource_source.index);
+}
+
+static void prepare_crs_csi2_swnodes(struct crs_csi2 *csi2)
+{
+	struct acpi_device_software_nodes *local_swnodes = csi2->swnodes;
+	acpi_handle local_handle = csi2->handle;
+	struct crs_csi2_connection *conn;
+
+	/* Bail out if the allocation of swnodes has failed. */
+	if (!local_swnodes)
+		return;
+
+	list_for_each_entry(conn, &csi2->connections, entry)
+		extract_crs_csi2_conn_info(local_handle, local_swnodes, conn);
+}
+
 /**
  * acpi_mipi_scan_crs_csi2 - Create ACPI _CRS CSI-2 software nodes
  *
@@ -275,9 +413,22 @@ void acpi_mipi_scan_crs_csi2(void)
 	}
 	list_splice(&aux_list, &acpi_mipi_crs_csi2_list);
 
-	/* Allocate softwware nodes for representing the CSI-2 information. */
+	/*
+	 * Allocate softwware nodes for representing the CSI-2 information.
+	 *
+	 * This needs to be done for all of the list entries in one go, because
+	 * they may point to each other without restrictions and the next step
+	 * relies on the availability of swnodes memory for each list entry.
+	 */
 	list_for_each_entry(csi2, &acpi_mipi_crs_csi2_list, entry)
 		alloc_crs_csi2_swnodes(csi2);
+
+	/*
+	 * Set up software node properties using data from _CRS CSI-2 resource
+	 * descriptors.
+	 */
+	list_for_each_entry(csi2, &acpi_mipi_crs_csi2_list, entry)
+		prepare_crs_csi2_swnodes(csi2);
 }
 
 /**

[PATCH v3 4/7] device property: Add SOFTWARE_NODE() macro for defining software nodes

[Rafael J. Wysocki]

From: Sakari Ailus <sakari.ailus@linux.intel.com>

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

v2 -> v3: No changes

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

Index: linux-pm/include/linux/property.h
===================================================================
--- linux-pm.orig/include/linux/property.h
+++ linux-pm/include/linux/property.h
@@ -488,6 +488,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);

[PATCH v3 5/7] ACPI: scan: Extract MIPI DisCo for Imaging data into swnodes

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: [PATCH] ACPI: scan: Extract MIPI DisCo for Imaging data into swnodes

Add information extracted from the MIPI DisCo for Imaging device
properties to software nodes created during the CSI-2 connection graph
discovery.

Link: https://www.mipi.org/specifications/mipi-disco-imaging
Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v2 -> v3:
   * Change the name of the new file to mipi-disco-img.c
   * "DiSco" -> "DisCo" in multiple places
   * Fix the link in the Link: tag
   * Change the number of data lanes limit and add a comment regarding it
   * Use ACPI_DEVICE_CSI2_DATA_LANES directly in several places instead
     of array sizes equal to it

---
 drivers/acpi/internal.h       |    1 
 drivers/acpi/mipi-disco-img.c |  249 +++++++++++++++++++++++++++++++++++++++++-
 drivers/acpi/scan.c           |   12 +-
 include/acpi/acpi_bus.h       |   17 ++
 4 files changed, 275 insertions(+), 4 deletions(-)

Index: linux-pm/drivers/acpi/mipi-disco-img.c
===================================================================
--- linux-pm.orig/drivers/acpi/mipi-disco-img.c
+++ linux-pm/drivers/acpi/mipi-disco-img.c
@@ -6,12 +6,16 @@
  *
  * Support MIPI DisCo for Imaging by parsing ACPI _CRS CSI-2 records defined in
  * Section 6.4.3.8.2.4 "Camera Serial Interface (CSI-2) Connection Resource
- * Descriptor" of ACPI 6.5.
+ * Descriptor" of ACPI 6.5 and using device properties defined by the MIPI DisCo
+ * for Imaging specification.
  *
  * The implementation looks for the information in the ACPI namespace (CSI-2
  * resource descriptors in _CRS) and constructs software nodes compatible with
  * Documentation/firmware-guide/acpi/dsd/graph.rst to represent the CSI-2
- * connection graph.
+ * connection graph.  The software nodes are then populated with the data
+ * extracted from the _CRS CSI-2 resource descriptors and the MIPI DisCo
+ * for Imaging device properties present in _DSD for the ACPI device objects
+ * with CSI-2 connections.
  */
 
 #include <linux/acpi.h>
@@ -431,6 +435,247 @@ void acpi_mipi_scan_crs_csi2(void)
 		prepare_crs_csi2_swnodes(csi2);
 }
 
+/*
+ * Get the index of the next property in the property array, with a given
+ * maximum value.
+ */
+#define NEXT_PROPERTY(index, max)			\
+	(WARN_ON((index) > ACPI_DEVICE_SWNODE_##max) ?	\
+	 ACPI_DEVICE_SWNODE_##max : (index)++)
+
+static void init_csi2_port_local(struct acpi_device *adev,
+				 struct acpi_device_software_node_port *port,
+				 struct fwnode_handle *port_fwnode,
+				 unsigned int index)
+{
+	acpi_handle handle = acpi_device_handle(adev);
+	unsigned int num_link_freqs;
+	int ret;
+
+	ret = fwnode_property_count_u64(port_fwnode, "mipi-img-link-frequencies");
+	if (ret <= 0)
+		return;
+
+	num_link_freqs = ret;
+	if (num_link_freqs > ACPI_DEVICE_CSI2_DATA_LANES) {
+		acpi_handle_info(handle, "Too many link frequencies: %u\n",
+				 num_link_freqs);
+		num_link_freqs = ACPI_DEVICE_CSI2_DATA_LANES;
+	}
+
+	ret = fwnode_property_read_u64_array(port_fwnode,
+					     "mipi-img-link-frequencies",
+					     port->link_frequencies,
+					     num_link_freqs);
+	if (ret) {
+		acpi_handle_info(handle, "Unable to get link frequencies (%d)\n",
+				 ret);
+		return;
+	}
+
+	port->ep_props[NEXT_PROPERTY(index, EP_LINK_FREQUENCIES)] =
+				PROPERTY_ENTRY_U64_ARRAY_LEN("link-frequencies",
+							     port->link_frequencies,
+							     num_link_freqs);
+}
+
+static void init_csi2_port(struct acpi_device *adev,
+			   struct acpi_device_software_nodes *swnodes,
+			   struct acpi_device_software_node_port *port,
+			   struct fwnode_handle *port_fwnode,
+			   unsigned int port_index)
+{
+	unsigned int ep_prop_index = ACPI_DEVICE_SWNODE_EP_CLOCK_LANES;
+	acpi_handle handle = acpi_device_handle(adev);
+	u8 val[ACPI_DEVICE_CSI2_DATA_LANES];
+	int num_lanes = 0;
+	int ret;
+
+	if (GRAPH_PORT_NAME(port->port_name, port->port_nr))
+		return;
+
+	swnodes->nodes[ACPI_DEVICE_SWNODE_PORT(port_index)] =
+			SOFTWARE_NODE(port->port_name, port->port_props,
+				      &swnodes->nodes[ACPI_DEVICE_SWNODE_ROOT]);
+
+	ret = fwnode_property_read_u8(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[0]);
+
+	ret = fwnode_property_count_u8(port_fwnode, "mipi-img-data-lanes");
+	if (ret > 0) {
+		num_lanes = ret;
+
+		if (num_lanes > ACPI_DEVICE_CSI2_DATA_LANES) {
+			acpi_handle_info(handle, "Too many data lanes: %u\n",
+					 num_lanes);
+			num_lanes = ACPI_DEVICE_CSI2_DATA_LANES;
+		}
+
+		ret = fwnode_property_read_u8_array(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(port_fwnode, "mipi-img-lane-polarities");
+	if (ret * BITS_PER_TYPE(u8) >= num_lanes + 1) {
+		unsigned long mask = 0;
+		int byte_count = ret;
+		unsigned int i;
+
+		/*
+		 * The total number of lanes is ACPI_DEVICE_CSI2_DATA_LANES + 1
+		 * (data lanes + clock lane).  It is not expected to ever be
+		 * greater than the number of bits in an unsigned long
+		 * variable, but ensure that this is the case.
+		 */
+		BUILD_BUG_ON(BITS_PER_TYPE(unsigned long) <= ACPI_DEVICE_CSI2_DATA_LANES);
+
+		if (byte_count > ACPI_DEVICE_CSI2_DATA_LANES) {
+			acpi_handle_info(handle, "Too many lane polarities: %d\n",
+					 byte_count);
+			byte_count = ACPI_DEVICE_CSI2_DATA_LANES;
+		}
+		fwnode_property_read_u8_array(port_fwnode, "mipi-img-lane-polarities",
+					      val, byte_count);
+
+		for (i = 0; BITS_PER_TYPE(u8) * i <= num_lanes; i++)
+			mask |= (unsigned long)val[i] << BITS_PER_TYPE(u8) * i;
+
+		for (i = 0; i <= num_lanes; i++)
+			port->lane_polarities[i] = test_bit(i, &mask);
+
+		port->ep_props[NEXT_PROPERTY(ep_prop_index, EP_LANE_POLARITIES)] =
+				PROPERTY_ENTRY_U32_ARRAY_LEN("lane-polarities",
+							     port->lane_polarities,
+							     num_lanes + 1);
+	} else {
+		acpi_handle_info(handle, "Lane polarity bytes missing\n");
+	}
+
+	swnodes->nodes[ACPI_DEVICE_SWNODE_EP(port_index)] =
+		SOFTWARE_NODE("endpoint@0", swnodes->ports[port_index].ep_props,
+			      &swnodes->nodes[ACPI_DEVICE_SWNODE_PORT(port_index)]);
+
+	if (port->crs_csi2_local)
+		init_csi2_port_local(adev, port, port_fwnode, ep_prop_index);
+}
+
+#define MIPI_IMG_PORT_PREFIX "mipi-img-port-"
+
+static struct fwnode_handle *get_mipi_port_handle(struct fwnode_handle *adev_fwnode,
+						  unsigned int port_nr)
+{
+	char port_name[sizeof(MIPI_IMG_PORT_PREFIX) + 2];
+
+	if (snprintf(port_name, sizeof(port_name), "%s%u",
+		     MIPI_IMG_PORT_PREFIX, port_nr) >= sizeof(port_name))
+		return NULL;
+
+	return fwnode_get_named_child_node(adev_fwnode, port_name);
+}
+
+static void init_crs_csi2_swnodes(struct crs_csi2 *csi2)
+{
+	struct acpi_buffer buffer = { .length = ACPI_ALLOCATE_BUFFER };
+	struct acpi_device_software_nodes *swnodes = csi2->swnodes;
+	acpi_handle handle = csi2->handle;
+	struct fwnode_handle *adev_fwnode;
+	struct acpi_device *adev;
+	acpi_status status;
+	unsigned int i;
+	int ret;
+
+	/*
+	 * Bail out if the swnodes are not available (either they have not been
+	 * allocated or they have been assigned to the device already).
+	 */
+	if (!swnodes)
+		return;
+
+	adev = acpi_fetch_acpi_dev(handle);
+	if (!adev)
+		return;
+
+	adev_fwnode = acpi_fwnode_handle(adev);
+
+	status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
+	if (ACPI_FAILURE(status)) {
+		acpi_handle_info(handle, "Unable to get the path name\n");
+		return;
+	}
+
+	swnodes->nodes[ACPI_DEVICE_SWNODE_ROOT] =
+			SOFTWARE_NODE(buffer.pointer, swnodes->dev_props, NULL);
+
+	for (i = 0; i < swnodes->num_ports; i++) {
+		struct acpi_device_software_node_port *port = &swnodes->ports[i];
+		struct fwnode_handle *port_fwnode;
+
+		/*
+		 * The MIPI DisCo for Imaging specification defines _DSD device
+		 * properties for providing CSI-2 port parameters that can be
+		 * accessed through the generic device properties framework.  To
+		 * access them, it is first necessary to find the data node
+		 * representing the port under the given ACPI device object.
+		 */
+		port_fwnode = get_mipi_port_handle(adev_fwnode, port->port_nr);
+		if (!port_fwnode) {
+			acpi_handle_info(handle,
+					 "MIPI port name too long for port %u\n",
+					 port->port_nr);
+			continue;
+		}
+
+		init_csi2_port(adev, swnodes, port, port_fwnode, i);
+
+		fwnode_handle_put(port_fwnode);
+	}
+
+	ret = software_node_register_node_group(swnodes->nodeptrs);
+	if (ret < 0) {
+		acpi_handle_info(handle,
+				 "Unable to register software nodes (%d)\n", ret);
+		return;
+	}
+
+	adev->swnodes = swnodes;
+	adev_fwnode->secondary = software_node_fwnode(swnodes->nodes);
+
+	/*
+	 * Prevents the swnodes from this csi2 entry from being assigned again
+	 * or freed prematurely.
+	 */
+	csi2->swnodes = NULL;
+}
+
+/**
+ * acpi_mipi_init_crs_csi2_swnodes - Initialize _CRS CSI-2 software nodes
+ *
+ * Use MIPI DisCo for Imaging device properties to finalize the initialization
+ * of CSI-2 software nodes for all ACPI device objects that have been already
+ * enumerated.
+ */
+void acpi_mipi_init_crs_csi2_swnodes(void)
+{
+	struct crs_csi2 *csi2, *csi2_tmp;
+
+	list_for_each_entry_safe(csi2, csi2_tmp, &acpi_mipi_crs_csi2_list, entry)
+		init_crs_csi2_swnodes(csi2);
+}
+
 /**
  * acpi_mipi_crs_csi2_cleanup - Free _CRS CSI-2 temporary data
  */
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -287,6 +287,7 @@ static inline void acpi_init_lpit(void)
 
 void acpi_mipi_check_crs_csi2(acpi_handle handle);
 void acpi_mipi_scan_crs_csi2(void);
+void acpi_mipi_init_crs_csi2_swnodes(void);
 void acpi_mipi_crs_csi2_cleanup(void);
 
 #endif /* _ACPI_INTERNAL_H_ */
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -2447,6 +2447,13 @@ static void acpi_scan_postponed_branch(a
 
 	acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
 			    acpi_bus_check_add_2, NULL, NULL, (void **)&adev);
+
+	/*
+	 * Populate the ACPI _CRS CSI-2 software nodes for the ACPI devices that
+	 * have been added above.
+	 */
+	acpi_mipi_init_crs_csi2_swnodes();
+
 	acpi_bus_attach(adev, NULL);
 }
 
@@ -2516,11 +2523,12 @@ int acpi_bus_scan(acpi_handle handle)
 		return -ENODEV;
 
 	/*
-	 * Allocate ACPI _CRS CSI-2 software nodes using information extracted
+	 * Set up ACPI _CRS CSI-2 software nodes using information extracted
 	 * from the _CRS CSI-2 resource descriptors during the ACPI namespace
-	 * walk above.
+	 * walk above and MIPI DisCo for Imaging device properties.
 	 */
 	acpi_mipi_scan_crs_csi2();
+	acpi_mipi_init_crs_csi2_swnodes();
 
 	acpi_bus_attach(device, (void *)true);
 
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -366,10 +366,24 @@ struct acpi_device_data {
 
 struct acpi_gpio_mapping;
 
+#define ACPI_DEVICE_SWNODE_ROOT			0
+
+/*
+ * The maximum expected number of CSI-2 data lanes.
+ *
+ * This number is not expected to ever have to be equal to or greater than the
+ * number of bits in an unsigned long variable, but if it needs to be increased
+ * above that limit, code will need to be adjusted accordingly.
+ */
 #define ACPI_DEVICE_CSI2_DATA_LANES		8
 
 #define ACPI_DEVICE_SWNODE_PORT_NAME_LENGTH	8
 
+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_PORT_REG,
 	ACPI_DEVICE_SWNODE_PORT_NUM_OF,
@@ -425,12 +439,14 @@ struct acpi_device_software_node_port {
 
 /**
  * struct acpi_device_software_nodes - Software nodes for an ACPI device
+ * @dev_props: Device properties.
  * @nodes: Software nodes for root as well as ports and endpoints.
  * @nodeprts: Array of software node pointers, for (un)registering them.
  * @ports: Information related to each port and endpoint within a port.
  * @num_ports: The number of ports.
  */
 struct acpi_device_software_nodes {
+	struct property_entry dev_props[ACPI_DEVICE_SWNODE_DEV_NUM_ENTRIES];
 	struct software_node *nodes;
 	const struct software_node **nodeptrs;
 	struct acpi_device_software_node_port *ports;
@@ -455,6 +471,7 @@ struct acpi_device {
 	struct acpi_device_data data;
 	struct acpi_scan_handler *handler;
 	struct acpi_hotplug_context *hp;
+	struct acpi_device_software_nodes *swnodes;
 	const struct acpi_gpio_mapping *driver_gpios;
 	void *driver_data;
 	struct device dev;

Re: [PATCH v3 5/7] ACPI: scan: Extract MIPI DisCo for Imaging data into swnodes

[Sakari Ailus]

Hi Rafael,

On Mon, Nov 06, 2023 at 05:27:26PM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Subject: [PATCH] ACPI: scan: Extract MIPI DisCo for Imaging data into swnodes
> 
> Add information extracted from the MIPI DisCo for Imaging device
> properties to software nodes created during the CSI-2 connection graph
> discovery.
> 
> Link: https://www.mipi.org/specifications/mipi-disco-imaging
> Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> v2 -> v3:
>    * Change the name of the new file to mipi-disco-img.c
>    * "DiSco" -> "DisCo" in multiple places
>    * Fix the link in the Link: tag
>    * Change the number of data lanes limit and add a comment regarding it
>    * Use ACPI_DEVICE_CSI2_DATA_LANES directly in several places instead
>      of array sizes equal to it

Thanks for the update. I've tested the set, so you can add:

Tested-by: Sakari Ailus <sakari.ailus@linux.intel.com>

with the following diff fixing mipi-img-lane-polarities parsing:

diff --git a/drivers/acpi/mipi-disco-img.c b/drivers/acpi/mipi-disco-img.c
index 3123180d9b54..92b45e792a07 100644
--- a/drivers/acpi/mipi-disco-img.c
+++ b/drivers/acpi/mipi-disco-img.c
@@ -530,7 +530,12 @@ static void init_csi2_port(struct acpi_device *adev,
 	}
 
 	ret = fwnode_property_count_u8(port_fwnode, "mipi-img-lane-polarities");
-	if (ret * BITS_PER_TYPE(u8) >= num_lanes + 1) {
+	if (ret < 0) {
+		acpi_handle_debug(handle, "Lane polarity bytes missing\n");
+	} else if (ret * BITS_PER_TYPE(u8) < num_lanes + 1) {
+		acpi_handle_info(handle, "Too few lane polarity bytes (%lu vs. %d)\n",
+				 ret * BITS_PER_TYPE(u8), num_lanes + 1);
+	} else {
 		unsigned long mask = 0;
 		int byte_count = ret;
 		unsigned int i;
@@ -543,15 +548,15 @@ static void init_csi2_port(struct acpi_device *adev,
 		 */
 		BUILD_BUG_ON(BITS_PER_TYPE(unsigned long) <= ACPI_DEVICE_CSI2_DATA_LANES);
 
-		if (byte_count > ACPI_DEVICE_CSI2_DATA_LANES) {
+		if (byte_count > sizeof(mask)) {
 			acpi_handle_info(handle, "Too many lane polarities: %d\n",
 					 byte_count);
-			byte_count = ACPI_DEVICE_CSI2_DATA_LANES;
+			byte_count = sizeof(mask);
 		}
 		fwnode_property_read_u8_array(port_fwnode, "mipi-img-lane-polarities",
 					      val, byte_count);
 
-		for (i = 0; BITS_PER_TYPE(u8) * i <= num_lanes; i++)
+		for (i = 0; i < byte_count; i++)
 			mask |= (unsigned long)val[i] << BITS_PER_TYPE(u8) * i;
 
 		for (i = 0; i <= num_lanes; i++)
@@ -561,8 +566,6 @@ static void init_csi2_port(struct acpi_device *adev,
 				PROPERTY_ENTRY_U32_ARRAY_LEN("lane-polarities",
 							     port->lane_polarities,
 							     num_lanes + 1);
-	} else {
-		acpi_handle_info(handle, "Lane polarity bytes missing\n");
 	}
 
 	swnodes->nodes[ACPI_DEVICE_SWNODE_EP(port_index)] =

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v3 5/7] ACPI: scan: Extract MIPI DisCo for Imaging data into swnodes

[Rafael J. Wysocki]

Hi Sakari,

On Mon, Nov 6, 2023 at 11:33 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Rafael,
>
> On Mon, Nov 06, 2023 at 05:27:26PM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Subject: [PATCH] ACPI: scan: Extract MIPI DisCo for Imaging data into swnodes
> >
> > Add information extracted from the MIPI DisCo for Imaging device
> > properties to software nodes created during the CSI-2 connection graph
> > discovery.
> >
> > Link: https://www.mipi.org/specifications/mipi-disco-imaging
> > Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >
> > v2 -> v3:
> >    * Change the name of the new file to mipi-disco-img.c
> >    * "DiSco" -> "DisCo" in multiple places
> >    * Fix the link in the Link: tag
> >    * Change the number of data lanes limit and add a comment regarding it
> >    * Use ACPI_DEVICE_CSI2_DATA_LANES directly in several places instead
> >      of array sizes equal to it
>
> Thanks for the update. I've tested the set, so you can add:
>
> Tested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>
> with the following diff fixing mipi-img-lane-polarities parsing:
>
> diff --git a/drivers/acpi/mipi-disco-img.c b/drivers/acpi/mipi-disco-img.c
> index 3123180d9b54..92b45e792a07 100644
> --- a/drivers/acpi/mipi-disco-img.c
> +++ b/drivers/acpi/mipi-disco-img.c
> @@ -530,7 +530,12 @@ static void init_csi2_port(struct acpi_device *adev,
>         }
>
>         ret = fwnode_property_count_u8(port_fwnode, "mipi-img-lane-polarities");
> -       if (ret * BITS_PER_TYPE(u8) >= num_lanes + 1) {
> +       if (ret < 0) {
> +               acpi_handle_debug(handle, "Lane polarity bytes missing\n");
> +       } else if (ret * BITS_PER_TYPE(u8) < num_lanes + 1) {
> +               acpi_handle_info(handle, "Too few lane polarity bytes (%lu vs. %d)\n",
> +                                ret * BITS_PER_TYPE(u8), num_lanes + 1);
> +       } else {
>                 unsigned long mask = 0;
>                 int byte_count = ret;
>                 unsigned int i;
> @@ -543,15 +548,15 @@ static void init_csi2_port(struct acpi_device *adev,
>                  */
>                 BUILD_BUG_ON(BITS_PER_TYPE(unsigned long) <= ACPI_DEVICE_CSI2_DATA_LANES);
>
> -               if (byte_count > ACPI_DEVICE_CSI2_DATA_LANES) {
> +               if (byte_count > sizeof(mask)) {
>                         acpi_handle_info(handle, "Too many lane polarities: %d\n",
>                                          byte_count);
> -                       byte_count = ACPI_DEVICE_CSI2_DATA_LANES;
> +                       byte_count = sizeof(mask);
>                 }
>                 fwnode_property_read_u8_array(port_fwnode, "mipi-img-lane-polarities",
>                                               val, byte_count);
>
> -               for (i = 0; BITS_PER_TYPE(u8) * i <= num_lanes; i++)
> +               for (i = 0; i < byte_count; i++)
>                         mask |= (unsigned long)val[i] << BITS_PER_TYPE(u8) * i;
>
>                 for (i = 0; i <= num_lanes; i++)
> @@ -561,8 +566,6 @@ static void init_csi2_port(struct acpi_device *adev,
>                                 PROPERTY_ENTRY_U32_ARRAY_LEN("lane-polarities",
>                                                              port->lane_polarities,
>                                                              num_lanes + 1);
> -       } else {
> -               acpi_handle_info(handle, "Lane polarity bytes missing\n");
>         }
>
>         swnodes->nodes[ACPI_DEVICE_SWNODE_EP(port_index)] =
>
> --

Thanks for testing and the fix!

I have folded it into the original patch and sent an update.

Also I have updated the git branch by replacing the original commit
with the updated one and I've added the Tested-by: tag to the commits
in it.

Thank you!

[PATCH v3 6/7] ACPI: property: Dig "rotation" property for devices with CSI2 _CRS

[Rafael J. Wysocki]

From: Sakari Ailus <sakari.ailus@linux.intel.com>

Find the "rotation" property value for devices with _CRS CSI-2 resource
descriptors and use it to add the "rotation" property to the software
nodes representing the CSI-2 connection graph.  That value typically
comes from the _PLD (Physical Location of Device) object if it is
present for the given device.

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

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
[ rjw: Changelog edits, file rename ]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v2 -> v3:
   * Change the name of the new file to mipi-disco-img.c

---
 drivers/acpi/mipi-disco-img.c |   17 +++++++++++++++++
 include/acpi/acpi_bus.h       |    1 +
 2 files changed, 18 insertions(+)

Index: linux-pm/drivers/acpi/mipi-disco-img.c
===================================================================
--- linux-pm.orig/drivers/acpi/mipi-disco-img.c
+++ linux-pm/drivers/acpi/mipi-disco-img.c
@@ -592,6 +592,7 @@ static void init_crs_csi2_swnodes(struct
 	struct acpi_buffer buffer = { .length = ACPI_ALLOCATE_BUFFER };
 	struct acpi_device_software_nodes *swnodes = csi2->swnodes;
 	acpi_handle handle = csi2->handle;
+	unsigned int prop_index = 0;
 	struct fwnode_handle *adev_fwnode;
 	struct acpi_device *adev;
 	acpi_status status;
@@ -611,6 +612,22 @@ static void init_crs_csi2_swnodes(struct
 
 	adev_fwnode = acpi_fwnode_handle(adev);
 
+	/*
+	 * If the "rotation" property is not present, but _PLD is there,
+	 * evaluate it to get the "rotation" value.
+	 */
+	if (!fwnode_property_present(adev_fwnode, "rotation")) {
+		struct acpi_pld_info *pld;
+
+		status = acpi_get_physical_device_location(handle, &pld);
+		if (ACPI_SUCCESS(status)) {
+			swnodes->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_info(handle, "Unable to get the path name\n");
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -380,6 +380,7 @@ struct acpi_gpio_mapping;
 #define ACPI_DEVICE_SWNODE_PORT_NAME_LENGTH	8
 
 enum acpi_device_swnode_dev_props {
+	ACPI_DEVICE_SWNODE_DEV_ROTATION,
 	ACPI_DEVICE_SWNODE_DEV_NUM_OF,
 	ACPI_DEVICE_SWNODE_DEV_NUM_ENTRIES
 };

[PATCH v3 7/7] ACPI: property: Replicate DT-aligned u32 properties from DisCo for Imaging

[Rafael J. Wysocki]

From: Sakari Ailus <sakari.ailus@linux.intel.com>

MIPI DisCo for Imaging defines properties for camera sensors that
functionally align with DT equivalents.

Replicate these properties in the ACPI device swnodes so the code
using the corresponding DT properties already does not need to be
updated to deal with their MIPI counterparts directly.

The replicated properties are:

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

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
[ rjw: Changelog edits, removal of redundant braces ]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/mipi-disco-img.c |   17 +++++++++++++++++
 include/acpi/acpi_bus.h       |    4 ++++
 2 files changed, 21 insertions(+)

Index: linux-pm/drivers/acpi/mipi-disco-img.c
===================================================================
--- linux-pm.orig/drivers/acpi/mipi-disco-img.c
+++ linux-pm/drivers/acpi/mipi-disco-img.c
@@ -597,6 +597,7 @@ static void init_crs_csi2_swnodes(struct
 	struct acpi_device *adev;
 	acpi_status status;
 	unsigned int i;
+	u32 val;
 	int ret;
 
 	/*
@@ -628,6 +629,22 @@ static void init_crs_csi2_swnodes(struct
 		}
 	}
 
+	if (!fwnode_property_read_u32(adev_fwnode, "mipi-img-clock-frequency", &val))
+		swnodes->dev_props[NEXT_PROPERTY(prop_index, DEV_CLOCK_FREQUENCY)] =
+			PROPERTY_ENTRY_U32("clock-frequency", val);
+
+	if (!fwnode_property_read_u32(adev_fwnode, "mipi-img-led-max-current", &val))
+		swnodes->dev_props[NEXT_PROPERTY(prop_index, DEV_LED_MAX_MICROAMP)] =
+			PROPERTY_ENTRY_U32("led-max-microamp", val);
+
+	if (!fwnode_property_read_u32(adev_fwnode, "mipi-img-flash-max-current", &val))
+		swnodes->dev_props[NEXT_PROPERTY(prop_index, DEV_FLASH_MAX_MICROAMP)] =
+			PROPERTY_ENTRY_U32("flash-max-microamp", val);
+
+	if (!fwnode_property_read_u32(adev_fwnode, "mipi-img-flash-max-timeout-us", &val))
+		swnodes->dev_props[NEXT_PROPERTY(prop_index, DEV_FLASH_MAX_TIMEOUT_US)] =
+			PROPERTY_ENTRY_U32("flash-max-timeout-us", val);
+
 	status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
 	if (ACPI_FAILURE(status)) {
 		acpi_handle_info(handle, "Unable to get the path name\n");
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -381,6 +381,10 @@ struct acpi_gpio_mapping;
 
 enum acpi_device_swnode_dev_props {
 	ACPI_DEVICE_SWNODE_DEV_ROTATION,
+	ACPI_DEVICE_SWNODE_DEV_CLOCK_FREQUENCY,
+	ACPI_DEVICE_SWNODE_DEV_LED_MAX_MICROAMP,
+	ACPI_DEVICE_SWNODE_DEV_FLASH_MAX_MICROAMP,
+	ACPI_DEVICE_SWNODE_DEV_FLASH_MAX_TIMEOUT_US,
 	ACPI_DEVICE_SWNODE_DEV_NUM_OF,
 	ACPI_DEVICE_SWNODE_DEV_NUM_ENTRIES
 };

Re: [PATCH v3 0/7] ACPI: scan: MIPI DisCo for Imaging support

[Rafael J. Wysocki]

On Mon, Nov 6, 2023 at 5:34 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>
> Hi Folks,
>
> This is a new revision of
>
> https://lore.kernel.org/linux-acpi/2187487.irdbgypaU6@kreacher/
>
> addressing comments from Sakari and adding one more patch (also from
> Sakari).
>
> The main points from the original cover letter are still valid:
>
> > The general idea is the same - CSI-2 resource descriptors, introduced in
> > ACPI 6.4 and defined by
> >
> > https://uefi.org/specs/ACPI/6.5/06_Device_Configuration.html#camera-serial-i
> > nterface-csi-2-connection-resource-descriptor
> >
> > are found and used for creating a set of software nodes that represent the
> > CSI-2 connection graph.
> >
> > These software nodes need to be available before any scan handlers or ACPI
> > drivers are bound to any struct acpi_device objects, so all of that is done
> > at the early stage of ACPI device enumeration, but unnecessary ACPI
> > namespace walks are avoided.
> >
> > The CSI-2 software nodes are populated with data extracted from the CSI-2
> > resource descriptors themselves and from device properties defined by the
> > MIPI DisCo for Imaging specification (see
> > https://www.mipi.org/specifications/mipi-disco-imaging).
> >
> > Patches [4,6/6] come from the original series directly, but the other
> > patches have been changes substantially, so I've decided to re-start patch
> > series versioning from scratch.
>
> The series is based on the current mainline.

And now it is available in the acpi-mipi-disco-img branch of
linux-pm.git, for easier access.

Thanks!

[PATCH v3.1 5/7] ACPI: scan: Extract MIPI DisCo for Imaging data into swnodes

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Add information extracted from the MIPI DisCo for Imaging device
properties to software nodes created during the CSI-2 connection graph
discovery.

Link: https://www.mipi.org/specifications/mipi-disco-imaging
Co-developed-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Tested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---

v3 -> v3.1:
   * Merge lane-polarities property parsing fix from Sakari
   * Add Tested-by: tag from Sakari

v2 -> v3:
   * Change the name of the new file to mipi-disco-img.c
   * "DiSco" -> "DisCo" in multiple places
   * Fix the link in the Link: tag
   * Change the number of data lanes limit and add a comment regarding it
   * Use ACPI_DEVICE_CSI2_DATA_LANES directly in several places instead
     of array sizes equal to it

---
 drivers/acpi/internal.h       |    1 
 drivers/acpi/mipi-disco-img.c |  252 +++++++++++++++++++++++++++++++++++++++++-
 drivers/acpi/scan.c           |   12 +-
 include/acpi/acpi_bus.h       |   17 ++
 4 files changed, 278 insertions(+), 4 deletions(-)

Index: linux-pm/drivers/acpi/mipi-disco-img.c
===================================================================
--- linux-pm.orig/drivers/acpi/mipi-disco-img.c
+++ linux-pm/drivers/acpi/mipi-disco-img.c
@@ -6,12 +6,16 @@
  *
  * Support MIPI DisCo for Imaging by parsing ACPI _CRS CSI-2 records defined in
  * Section 6.4.3.8.2.4 "Camera Serial Interface (CSI-2) Connection Resource
- * Descriptor" of ACPI 6.5.
+ * Descriptor" of ACPI 6.5 and using device properties defined by the MIPI DisCo
+ * for Imaging specification.
  *
  * The implementation looks for the information in the ACPI namespace (CSI-2
  * resource descriptors in _CRS) and constructs software nodes compatible with
  * Documentation/firmware-guide/acpi/dsd/graph.rst to represent the CSI-2
- * connection graph.
+ * connection graph.  The software nodes are then populated with the data
+ * extracted from the _CRS CSI-2 resource descriptors and the MIPI DisCo
+ * for Imaging device properties present in _DSD for the ACPI device objects
+ * with CSI-2 connections.
  */
 
 #include <linux/acpi.h>
@@ -431,6 +435,250 @@ void acpi_mipi_scan_crs_csi2(void)
 		prepare_crs_csi2_swnodes(csi2);
 }
 
+/*
+ * Get the index of the next property in the property array, with a given
+ * maximum value.
+ */
+#define NEXT_PROPERTY(index, max)			\
+	(WARN_ON((index) > ACPI_DEVICE_SWNODE_##max) ?	\
+	 ACPI_DEVICE_SWNODE_##max : (index)++)
+
+static void init_csi2_port_local(struct acpi_device *adev,
+				 struct acpi_device_software_node_port *port,
+				 struct fwnode_handle *port_fwnode,
+				 unsigned int index)
+{
+	acpi_handle handle = acpi_device_handle(adev);
+	unsigned int num_link_freqs;
+	int ret;
+
+	ret = fwnode_property_count_u64(port_fwnode, "mipi-img-link-frequencies");
+	if (ret <= 0)
+		return;
+
+	num_link_freqs = ret;
+	if (num_link_freqs > ACPI_DEVICE_CSI2_DATA_LANES) {
+		acpi_handle_info(handle, "Too many link frequencies: %u\n",
+				 num_link_freqs);
+		num_link_freqs = ACPI_DEVICE_CSI2_DATA_LANES;
+	}
+
+	ret = fwnode_property_read_u64_array(port_fwnode,
+					     "mipi-img-link-frequencies",
+					     port->link_frequencies,
+					     num_link_freqs);
+	if (ret) {
+		acpi_handle_info(handle, "Unable to get link frequencies (%d)\n",
+				 ret);
+		return;
+	}
+
+	port->ep_props[NEXT_PROPERTY(index, EP_LINK_FREQUENCIES)] =
+				PROPERTY_ENTRY_U64_ARRAY_LEN("link-frequencies",
+							     port->link_frequencies,
+							     num_link_freqs);
+}
+
+static void init_csi2_port(struct acpi_device *adev,
+			   struct acpi_device_software_nodes *swnodes,
+			   struct acpi_device_software_node_port *port,
+			   struct fwnode_handle *port_fwnode,
+			   unsigned int port_index)
+{
+	unsigned int ep_prop_index = ACPI_DEVICE_SWNODE_EP_CLOCK_LANES;
+	acpi_handle handle = acpi_device_handle(adev);
+	u8 val[ACPI_DEVICE_CSI2_DATA_LANES];
+	int num_lanes = 0;
+	int ret;
+
+	if (GRAPH_PORT_NAME(port->port_name, port->port_nr))
+		return;
+
+	swnodes->nodes[ACPI_DEVICE_SWNODE_PORT(port_index)] =
+			SOFTWARE_NODE(port->port_name, port->port_props,
+				      &swnodes->nodes[ACPI_DEVICE_SWNODE_ROOT]);
+
+	ret = fwnode_property_read_u8(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[0]);
+
+	ret = fwnode_property_count_u8(port_fwnode, "mipi-img-data-lanes");
+	if (ret > 0) {
+		num_lanes = ret;
+
+		if (num_lanes > ACPI_DEVICE_CSI2_DATA_LANES) {
+			acpi_handle_info(handle, "Too many data lanes: %u\n",
+					 num_lanes);
+			num_lanes = ACPI_DEVICE_CSI2_DATA_LANES;
+		}
+
+		ret = fwnode_property_read_u8_array(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(port_fwnode, "mipi-img-lane-polarities");
+	if (ret < 0) {
+		acpi_handle_debug(handle, "Lane polarity bytes missing\n");
+	} else if (ret * BITS_PER_TYPE(u8) < num_lanes + 1) {
+		acpi_handle_info(handle, "Too few lane polarity bytes (%lu vs. %d)\n",
+				 ret * BITS_PER_TYPE(u8), num_lanes + 1);
+	} else {
+		unsigned long mask = 0;
+		int byte_count = ret;
+		unsigned int i;
+
+		/*
+		 * The total number of lanes is ACPI_DEVICE_CSI2_DATA_LANES + 1
+		 * (data lanes + clock lane).  It is not expected to ever be
+		 * greater than the number of bits in an unsigned long
+		 * variable, but ensure that this is the case.
+		 */
+		BUILD_BUG_ON(BITS_PER_TYPE(unsigned long) <= ACPI_DEVICE_CSI2_DATA_LANES);
+
+		if (byte_count > sizeof(mask)) {
+			acpi_handle_info(handle, "Too many lane polarities: %d\n",
+					 byte_count);
+			byte_count = sizeof(mask);
+		}
+		fwnode_property_read_u8_array(port_fwnode, "mipi-img-lane-polarities",
+					      val, byte_count);
+
+		for (i = 0; i < byte_count; i++)
+			mask |= (unsigned long)val[i] << BITS_PER_TYPE(u8) * i;
+
+		for (i = 0; i <= num_lanes; i++)
+			port->lane_polarities[i] = test_bit(i, &mask);
+
+		port->ep_props[NEXT_PROPERTY(ep_prop_index, EP_LANE_POLARITIES)] =
+				PROPERTY_ENTRY_U32_ARRAY_LEN("lane-polarities",
+							     port->lane_polarities,
+							     num_lanes + 1);
+	}
+
+	swnodes->nodes[ACPI_DEVICE_SWNODE_EP(port_index)] =
+		SOFTWARE_NODE("endpoint@0", swnodes->ports[port_index].ep_props,
+			      &swnodes->nodes[ACPI_DEVICE_SWNODE_PORT(port_index)]);
+
+	if (port->crs_csi2_local)
+		init_csi2_port_local(adev, port, port_fwnode, ep_prop_index);
+}
+
+#define MIPI_IMG_PORT_PREFIX "mipi-img-port-"
+
+static struct fwnode_handle *get_mipi_port_handle(struct fwnode_handle *adev_fwnode,
+						  unsigned int port_nr)
+{
+	char port_name[sizeof(MIPI_IMG_PORT_PREFIX) + 2];
+
+	if (snprintf(port_name, sizeof(port_name), "%s%u",
+		     MIPI_IMG_PORT_PREFIX, port_nr) >= sizeof(port_name))
+		return NULL;
+
+	return fwnode_get_named_child_node(adev_fwnode, port_name);
+}
+
+static void init_crs_csi2_swnodes(struct crs_csi2 *csi2)
+{
+	struct acpi_buffer buffer = { .length = ACPI_ALLOCATE_BUFFER };
+	struct acpi_device_software_nodes *swnodes = csi2->swnodes;
+	acpi_handle handle = csi2->handle;
+	struct fwnode_handle *adev_fwnode;
+	struct acpi_device *adev;
+	acpi_status status;
+	unsigned int i;
+	int ret;
+
+	/*
+	 * Bail out if the swnodes are not available (either they have not been
+	 * allocated or they have been assigned to the device already).
+	 */
+	if (!swnodes)
+		return;
+
+	adev = acpi_fetch_acpi_dev(handle);
+	if (!adev)
+		return;
+
+	adev_fwnode = acpi_fwnode_handle(adev);
+
+	status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
+	if (ACPI_FAILURE(status)) {
+		acpi_handle_info(handle, "Unable to get the path name\n");
+		return;
+	}
+
+	swnodes->nodes[ACPI_DEVICE_SWNODE_ROOT] =
+			SOFTWARE_NODE(buffer.pointer, swnodes->dev_props, NULL);
+
+	for (i = 0; i < swnodes->num_ports; i++) {
+		struct acpi_device_software_node_port *port = &swnodes->ports[i];
+		struct fwnode_handle *port_fwnode;
+
+		/*
+		 * The MIPI DisCo for Imaging specification defines _DSD device
+		 * properties for providing CSI-2 port parameters that can be
+		 * accessed through the generic device properties framework.  To
+		 * access them, it is first necessary to find the data node
+		 * representing the port under the given ACPI device object.
+		 */
+		port_fwnode = get_mipi_port_handle(adev_fwnode, port->port_nr);
+		if (!port_fwnode) {
+			acpi_handle_info(handle,
+					 "MIPI port name too long for port %u\n",
+					 port->port_nr);
+			continue;
+		}
+
+		init_csi2_port(adev, swnodes, port, port_fwnode, i);
+
+		fwnode_handle_put(port_fwnode);
+	}
+
+	ret = software_node_register_node_group(swnodes->nodeptrs);
+	if (ret < 0) {
+		acpi_handle_info(handle,
+				 "Unable to register software nodes (%d)\n", ret);
+		return;
+	}
+
+	adev->swnodes = swnodes;
+	adev_fwnode->secondary = software_node_fwnode(swnodes->nodes);
+
+	/*
+	 * Prevents the swnodes from this csi2 entry from being assigned again
+	 * or freed prematurely.
+	 */
+	csi2->swnodes = NULL;
+}
+
+/**
+ * acpi_mipi_init_crs_csi2_swnodes - Initialize _CRS CSI-2 software nodes
+ *
+ * Use MIPI DisCo for Imaging device properties to finalize the initialization
+ * of CSI-2 software nodes for all ACPI device objects that have been already
+ * enumerated.
+ */
+void acpi_mipi_init_crs_csi2_swnodes(void)
+{
+	struct crs_csi2 *csi2, *csi2_tmp;
+
+	list_for_each_entry_safe(csi2, csi2_tmp, &acpi_mipi_crs_csi2_list, entry)
+		init_crs_csi2_swnodes(csi2);
+}
+
 /**
  * acpi_mipi_crs_csi2_cleanup - Free _CRS CSI-2 temporary data
  */
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -287,6 +287,7 @@ static inline void acpi_init_lpit(void)
 
 void acpi_mipi_check_crs_csi2(acpi_handle handle);
 void acpi_mipi_scan_crs_csi2(void);
+void acpi_mipi_init_crs_csi2_swnodes(void);
 void acpi_mipi_crs_csi2_cleanup(void);
 
 #endif /* _ACPI_INTERNAL_H_ */
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -2447,6 +2447,13 @@ static void acpi_scan_postponed_branch(a
 
 	acpi_walk_namespace(ACPI_TYPE_ANY, handle, ACPI_UINT32_MAX,
 			    acpi_bus_check_add_2, NULL, NULL, (void **)&adev);
+
+	/*
+	 * Populate the ACPI _CRS CSI-2 software nodes for the ACPI devices that
+	 * have been added above.
+	 */
+	acpi_mipi_init_crs_csi2_swnodes();
+
 	acpi_bus_attach(adev, NULL);
 }
 
@@ -2516,11 +2523,12 @@ int acpi_bus_scan(acpi_handle handle)
 		return -ENODEV;
 
 	/*
-	 * Allocate ACPI _CRS CSI-2 software nodes using information extracted
+	 * Set up ACPI _CRS CSI-2 software nodes using information extracted
 	 * from the _CRS CSI-2 resource descriptors during the ACPI namespace
-	 * walk above.
+	 * walk above and MIPI DisCo for Imaging device properties.
 	 */
 	acpi_mipi_scan_crs_csi2();
+	acpi_mipi_init_crs_csi2_swnodes();
 
 	acpi_bus_attach(device, (void *)true);
 
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -366,10 +366,24 @@ struct acpi_device_data {
 
 struct acpi_gpio_mapping;
 
+#define ACPI_DEVICE_SWNODE_ROOT			0
+
+/*
+ * The maximum expected number of CSI-2 data lanes.
+ *
+ * This number is not expected to ever have to be equal to or greater than the
+ * number of bits in an unsigned long variable, but if it needs to be increased
+ * above that limit, code will need to be adjusted accordingly.
+ */
 #define ACPI_DEVICE_CSI2_DATA_LANES		8
 
 #define ACPI_DEVICE_SWNODE_PORT_NAME_LENGTH	8
 
+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_PORT_REG,
 	ACPI_DEVICE_SWNODE_PORT_NUM_OF,
@@ -425,12 +439,14 @@ struct acpi_device_software_node_port {
 
 /**
  * struct acpi_device_software_nodes - Software nodes for an ACPI device
+ * @dev_props: Device properties.
  * @nodes: Software nodes for root as well as ports and endpoints.
  * @nodeprts: Array of software node pointers, for (un)registering them.
  * @ports: Information related to each port and endpoint within a port.
  * @num_ports: The number of ports.
  */
 struct acpi_device_software_nodes {
+	struct property_entry dev_props[ACPI_DEVICE_SWNODE_DEV_NUM_ENTRIES];
 	struct software_node *nodes;
 	const struct software_node **nodeptrs;
 	struct acpi_device_software_node_port *ports;
@@ -455,6 +471,7 @@ struct acpi_device {
 	struct acpi_device_data data;
 	struct acpi_scan_handler *handler;
 	struct acpi_hotplug_context *hp;
+	struct acpi_device_software_nodes *swnodes;
 	const struct acpi_gpio_mapping *driver_gpios;
 	void *driver_data;
 	struct device dev;