Re: [PATCH v7 2/5] platform/x86: int3472: tps68470: fix clock consumer registration for Dell Latitude 5285

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Wed, Aug 19, 2026 at 04:01:04PM +0200, Thierry Chatard wrote:
> The BIOS on the Dell Latitude 5285 leaves GNVS field C0TP at zero.
> With C0TP=0 the ACPI _DEP method on INT3479 (OV5670, front camera)
> resolves to PCI0 instead of the INT3472 (TPS68470 PMIC) device.
> 
> Because for_each_acpi_consumer_dev() walks the _DEP reverse-mapping,
> INT3479 is invisible to it: the clock consumer lookup entry for the
> front camera is never registered with the tps68470-clk driver, and
> the OV5670 sensor driver cannot acquire its MCLK.

> Fix this without touching ACPI tables by adding optional static clock
> consumer fields to struct int3472_tps68470_board_data:
> 
>   unsigned int n_clk_consumers;
>   const struct tps68470_clk_consumer *clk_consumers;

No need to repeat what we may read in the code.
Is this message constructed with a help of AI?
The rule of thumb: use AI as a tool, do not blindly
copy'n'paste what it pukes.

> When board data is present and n_clk_consumers is non-zero, probe uses
> the static list instead of for_each_acpi_consumer_dev() to populate
> tps68470-clk platform data.  Platforms that do not set these fields
> continue to use the existing ACPI traversal path unchanged.

...

> +	struct gpiod_lookup_table * const *tables;

Okay, you want the outer pointer to be const, that's probably fine, but in any
case the GPIOLIB modifies the given tables.

>  	int n_consumers;
>  	int device_type;
>  	int ret;
> -	int i;
> +	unsigned int i;

Keep the reversed xmas tree ordering.

...

> +	/*
> +	 * The order of the cells matters here! The clk must be first
> +	 * because the regulator depends on it. The gpios must be last,
> +	 * acpi_gpiochip_add() calls acpi_dev_clear_dependencies() and
> +	 * the clk + regulators must be ready when this happens.
> +	 */
> +	cells[0].name = "tps68470-clk";
> +	cells[0].platform_data = clk_pdata;
> +	cells[0].pdata_size = struct_size(clk_pdata, consumers, n_consumers);
> +	cells[1].name = "tps68470-regulator";
> +	cells[1].platform_data = (void *)board_data->tps68470_regulator_pdata;
> +	cells[1].pdata_size = sizeof(struct tps68470_regulator_platform_data);
> +	cells[2].name = "tps68470-gpio";
> +
> +	tables = board_data->tps68470_gpio_lookup_tables;
> +	for (i = 0; i < board_data->n_gpiod_lookups; i++)
> +		gpiod_add_lookup_table(tables[i]);

I'm wondering if we can / want to use gpiod_add_lookup_tables().

> +	ret = devm_mfd_add_devices(&client->dev, PLATFORM_DEVID_NONE,
> +				   cells, TPS68470_WIN_MFD_CELL_COUNT,
> +				   NULL, 0, NULL);
> +	kfree(cells);
> +
> +	if (ret) {
> +		for (i = 0; i < board_data->n_gpiod_lookups; i++)
> +			gpiod_remove_lookup_table(tables[i]);
> +	}

...

>  	const struct int3472_tps68470_board_data *board_data;
> -	int i;
> +	unsigned int i;

This change along with the similar above deserves a separate patch.

-- 
With Best Regards,
Andy Shevchenko