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@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)] =

-- 
Kind regards,

Sakari Ailus