Re: [PATCH v3 2/8] ACPI: property: Parse _CRS CSI-2 descriptor

Andy Shevchenko
2023-01-27T10:08:46Z

On Thu, Jan 26, 2023 at 12:40:55AM +0200, Sakari Ailus wrote:
> Parse newly added ACPI _CRS CSI-2 descriptor for CSI-2 and camera
> configuration. For now, only figure out where the descriptor is present in
> order to allow adding information from it to related devices.

...

> + if (status != AE_OK) {

ACPI_FAILURE()

> + acpi_handle_warn(inst_context->handle,
> + "cannot get handle for %s\n",
> + csi2->resource_source.string_ptr);
> + return AE_OK;
> + }

...

> + struct scan_check_crs_csi2_context inst_context = {
> + .handle = handle,
> + .res_list = LIST_HEAD_INIT(inst_context.res_list),
> + };
> + struct list_head *list = context;
> + struct crs_csi2 *csi2;

> + INIT_LIST_HEAD(&inst_context.res_list);

Do you still need this?

...

> + acpi_walk_resources(handle, METHOD_NAME__CRS,
> + scan_check_crs_csi2_instance, &inst_context);
> +
> + if (list_empty(&inst_context.res_list))
> + return AE_OK;

I'm wondering if you can utilize acpi_dev_get_resources().

...

> + /* Collect the devices that have a _CRS CSI-2 resource */
> + acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, ACPI_UINT32_MAX,

Other serial buses limit the DEPTH by 32, why do we need more here?

> + scan_check_crs_csi2, NULL, &crs_csi2_handles, NULL);

...

> + sort(handle_refs, handle_count, sizeof(*handle_refs), crs_handle_cmp,
> + NULL);

A single line?

...

> + if (check_mul_overflow(sizeof(*ads->ports) +
> + sizeof(*ads->nodes) * 2 +
> + sizeof(*ads->nodeptrs) * 2,
> + (size_t)this_count, &alloc_size) ||

So, now you know why this_count can't be type of size_t?

> + check_add_overflow(sizeof(*ads) + sizeof(*ads->nodes) +
> + sizeof(*ads->nodeptrs) * 2,
> + alloc_size, &alloc_size)) {
> + acpi_handle_warn(handle, "too many handles (%u)",
> + this_count);
> + continue;
> + }

--
With Best Regards,
Andy Shevchenko