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

["Rafael J. Wysocki" <rafael@xxxxxxxxxx>]

Hi Sakari,

On Mon, Nov 6, 2023 at 11:33 PM Sakari Ailus
<sakari.ailus@xxxxxxxxxxxxxxx> wrote:
>
> Hi Rafael,
>
> On Mon, Nov 06, 2023 at 05:27:26PM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > 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@xxxxxxxxxxxxxxx>
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > ---
> >
> > 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@xxxxxxxxxxxxxxx>
>
> 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!