Re: [PATCH v2 0/6] ACPI: scan: MIPI DiSco for Imaging support

Sakari Ailus
2023-10-31T08:45:37Z

Hi Rafael,

On Fri, Oct 20, 2023 at 04:33:56PM +0200, Rafael J. Wysocki wrote:
> Hi Folks,
>
> This is a new revision of
>
> https://lore.kernel.org/linux-acpi/13276375.uLZWGnKmhe@kreacher/
>
> which was reported to have issues and it took time to revisit it.
>
> > 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 v2 addresses at least 3 issues found in the v1 by code inspection:
>
> * A port_count field incrementation was missing in acpi_mipi_scan_crs_csi2(),
> so its value for all of the devices having CSI2 resources in _CRS was always
> 1 (and it should be equal to the number of valid CSI2 connection resources).
>
> * Some acpi_mipi_crs_csi2_list members could be freed prematurely, so they were
> inaccessible when extract_crs_csi2_conn_info() attempted to access them.
>
> * A check of remote_swnodes() against NULL was missing, which could result in
> a crash in a case when the swnodes memory could not be allocated for some
> acpi_mipi_crs_csi2_list entries.
>
> Apart from that, it rearranges the code somewhat to make it easier to follow
> and to avoid premature freeing of memory in it in general and the new file
> added by it is now called mipi-di.c (instead of mipi-disco-imaging.c) for
> compactness.
>
> The series is based on current linux-next.

Thanks for the update. I've tested this and I can confirm it works, to the
extent implemented in the set. The rest can be implemented on top
(mainly replicating properties).

I'll comment on a few patches in the set.

Do you prefer to make the changes or shall I? I presume them to be fairly
minor.

--
Regards,

Sakari Ailus