On Wed, May 17, 2023 at 1:54 PM Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> wrote: > > Hi Rafael, > > Thanks for the review. > > On Wed, May 17, 2023 at 12:53:43PM +0200, Rafael J. Wysocki wrote: > > > + list_for_each_entry(csi2, &ctx->crs_csi2_head, list) { > > > + struct acpi_device_software_nodes *local_swnodes; > > > + struct crs_csi2_instance *inst; > > > + > > > + local_swnodes = crs_csi2_swnode_get(csi2->handle); > > > + if (WARN_ON_ONCE(!local_swnodes)) > > > + continue; > > > + > > > + list_for_each_entry(inst, &csi2->buses, list) { > > > + struct acpi_device_software_nodes *remote_swnodes; > > > + struct acpi_device_software_node_port *local_port; > > > + struct acpi_device_software_node_port *remote_port; > > > + struct software_node *local_node, *remote_node; > > > + unsigned int local_index, remote_index; > > > + unsigned int bus_type; > > > + > > > + remote_swnodes = crs_csi2_swnode_get(inst->remote_handle); > > > + if (WARN_ON_ONCE(!remote_swnodes)) > > > + continue; > > > + > > > + local_index = next_csi2_port_index(local_swnodes, inst->csi2.local_port_instance); > > > + remote_index = next_csi2_port_index(remote_swnodes, inst->csi2.resource_source.index); > > > + > > > + if (WARN_ON_ONCE(local_index >= local_swnodes->num_ports) || > > > + WARN_ON_ONCE(remote_index >= remote_swnodes->num_ports)) > > > + goto out_free; > > > + > > > + switch (inst->csi2.phy_type) { > > > + case ACPI_CRS_CSI2_PHY_TYPE_C: > > > + bus_type = V4L2_FWNODE_BUS_TYPE_CSI2_CPHY; > > > + break; > > > + case ACPI_CRS_CSI2_PHY_TYPE_D: > > > + bus_type = V4L2_FWNODE_BUS_TYPE_CSI2_DPHY; > > > + break; > > > + default: > > > + acpi_handle_info(csi2->handle, > > > + "ignoring CSI-2 PHY type %u\n", > > > + inst->csi2.phy_type); > > > + continue; > > > + } > > > + > > > + local_port = &local_swnodes->ports[local_index]; > > > + local_node = &local_swnodes->nodes[ACPI_DEVICE_SWNODE_EP(local_index)]; > > > + local_port->remote_ep_ref[0] = SOFTWARE_NODE_REFERENCE(local_node); > > > > This looks odd. Is local_port pointing to its own node as a remote > > endpont, or am I confused? > > This is a reference to a software node that will be, in turn, referenced by > the "remote-endpoint" property entry in the remote node. Look for > ACPI_DEVICE_SWNODE_EP_REMOTE_EP a few lines below these. To be precise, IIUC, it is going to be the "remote-endpoint" value for the remote node. OK, thanks for the explanation. This isn't exactly straightforward TBH.