Hi Rafael, On Mon, May 15, 2023 at 06:45:10PM +0200, Rafael J. Wysocki wrote: > On Wednesday, March 29, 2023 12:09:44 PM CEST Sakari Ailus wrote: > > Parse newly added ACPI _CRS CSI-2 descriptor for CSI-2 and camera > > configuration, associate it with appropriate devices and allocate memory for > > software nodes needed to create a DT-like data structure for drivers. > > It occurred to me, that there would be so many things I would like to change > in this patch, so it would be better to create my own version of it, which > is appended. > > It is based on > > https://patchwork.kernel.org/project/linux-acpi/patch/2694293.mvXUDI8C0e@kreacher/ > > that has just been posted. > > IIUC, the idea is to extract the ACPI handle for each resource source in every > _CRS CSI-2 resource descriptor and count how many times each handle appears in > a CSI-2 context, either because it is referenced from a _CRS CSI-2 resource > descriptor (as a "resource source"), or because its device object has CSI-2 > resource descriptors in _CRS. Correct. > > This allows a set of software nodes to be allocated for each of these handles. > > If I got that totally wrong, please let me know. Otherwise, I will rework the > remaining patches in the series to match this one. It seems about right. I mostly see renames, moving the code around, using the existing dependency list and then parsing sub-tree for _CRS CSI-2 objects right from the bus scan callback. It seems you've also moved the structs from internal.h to what is now called mipi-disco-imaging.c. They'll be later needed in e.g. scan.c. At least I'd use names that indicate they're related to scanning the bus: they're not needed after this is done. I don't have objections to you reworking the rest, but given the number of non-trivial changes, will it work after this? :-) I can also do this, although I would un-do some of the changes in this patch in order to prepare for the rest (such as moving the structs from internal.h). See e.g. "ACPI: scan: Generate software nodes based on MIPI DisCo for Imaging", I think it's the 6th patch. ... > +/** > + * acpi_mipi_scan_crs_csi2 - Allocate 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; > + acpi_status status; > + > + status = acpi_get_data_full(conn->remote_handle, > + acpi_mipi_data_tag, > + (void **)&remote_csi2, > + NULL); > + if (ACPI_SUCCESS(status) && remote_csi2) { > + remote_csi2->port_count++; > + continue; > + } > + > + /* > + * The "remote" device has no _CRS CSI-2 list entry yet, > + * so create one for it and add it to the list. > + */ > + remote_csi2 = create_crs_csi2_entry(conn->remote_handle); > + if (!remote_csi2) > + continue; > + > + list_add(&remote_csi2->entry, &aux_list); > + } > + } > + list_splice(&aux_list, &acpi_mipi_crs_csi2_list); > + > + /* Allocate softwware nodes for representing the CSI-2 information. */ "software" > + list_for_each_entry(csi2, &acpi_mipi_crs_csi2_list, entry) > + alloc_fill_crs_csi2_swnodes(csi2->handle, csi2->port_count); > + > + acpi_mipi_crs_csi2_release(); > +} -- Kind regards, Sakari Ailus