Re: [PATCH v2 2/2] ACPI: property: Ignore bad graph port nodes on Dell XPS 9315

[andy.shevchenko@gmail.com]

Tue, Feb 13, 2024 at 03:46:06PM +0200, Sakari Ailus kirjoitti:
> Some systems were shipped with both Windows and Linux camera descriptions.
> In general, if Linux description exist, they will be used and Windows
> description ignored.
> 
> In this case the Linux descriptions were buggy so use Windows definition
> instead. This patch ignores the bad graph port nodes on Dell XPS 9315 and
> there are likely other such systems, too. The corresponding information
> has been added to ipu-bridge to cover the missing bits.

...

>  void acpi_mipi_scan_crs_csi2(void);
>  void acpi_mipi_init_crs_csi2_swnodes(void);
>  void acpi_mipi_crs_csi2_cleanup(void);

+ blank line?

> +bool acpi_graph_ignore_port(acpi_handle handle);
>  
>  #endif /* _ACPI_INTERNAL_H_ */

...

> +static const struct dmi_system_id dmi_ignore_port_nodes[] = {
> +	{
> +		.matches = {
> +			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "XPS 9315"),
> +		},
> +	},
> +	{ 0 }

0 is not needed. Moreover, it's a bit unusual.

> +};

...

> +static const char *strnext(const char *s1, const char *s2)
> +{
> +	s1 = strstr(s1, s2);
> +
> +	if (!s1)
> +		return NULL;
> +
> +	return s1 + strlen(s2);
> +}

NIH str_has_prefix() ?

...

> +/**
> + * acpi_graph_ignore_port - Tell whether a port node should be ignored
> + * @handle: The ACPI handle of the node (which may be a port node)
> + *
> + * Returns true if a port node should be ignored and the data to that should

Please, run kernel-doc validation and fix the warnings.

> + * come from other sources instead (Windows ACPI definitions and
> + * ipu-bridge). This is currently used to ignore bad port nodes related to IPU6
> + * ("IPU?") and camera sensor devices ("LNK?") in certain Dell systems with
> + * Intel VSC.
> + */
> +bool acpi_graph_ignore_port(acpi_handle handle)
> +{
> +	const char *path = NULL, *orig_path;
> +	static bool dmi_tested, ignore_port;
> +
> +	if (!dmi_tested) {
> +		ignore_port = dmi_first_match(dmi_ignore_port_nodes);
> +		dmi_tested = true;
> +	}
> +
> +	if (!ignore_port)
> +		return false;
> +
> +	/* Check if the device is either "IPU" or "LNK" (sensor). */
> +	orig_path = acpi_handle_path(handle);
> +	if (!orig_path)
> +		return false;
> +	path = strnext(orig_path, "IPU");

IIUC this can be rewritten as

	path += str_has_prefix();
	if (path == orig_path)
		...

> +	if (!path)
> +		path = strnext(orig_path, "LNK");
> +	if (!path)
> +		goto out_free;
> +
> +	if (!(path[0] >= '0' && path[0] <= '9' && path[1] == '.'))

isdigit() ?

> +		goto out_free;
> +
> +	/* Check if the node has a "PRT" prefix. */
> +	path = strnext(path, "PRT");
> +	if (path && path[0] >= '0' && path[0] <= '9' && !path[1]) {

Ditto.

> +		acpi_handle_debug(handle, "ignoring data node\n");
> +
> +		kfree(orig_path);
> +		return true;
> +	}
> +
> +out_free:
> +	kfree(orig_path);
> +	return false;
> +}

-- 
With Best Regards,
Andy Shevchenko