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

[Sakari Ailus]

Hi Thierry,

On Tue, Apr 21, 2026 at 03:52:14PM -0700, 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;
> 
> 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.
> 
> The board_data lookup is moved before the clock-pdata allocation so
> that it is available for both the static and dynamic paths.
> 
> Signed-off-by: Thierry Chatard <tchatard@xxxxxxxxx>
> ---
>  drivers/platform/x86/intel/int3472/tps68470.c | 36 +++++++++++++++----
>  drivers/platform/x86/intel/int3472/tps68470.h | 10 ++++++
>  2 files changed, 39 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
> index a496075c0..e121eb24a 100644
> --- a/drivers/platform/x86/intel/int3472/tps68470.c
> +++ b/drivers/platform/x86/intel/int3472/tps68470.c
> @@ -147,17 +147,40 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
>  	struct tps68470_clk_platform_data *clk_pdata;
>  	struct mfd_cell *cells;
>  	struct regmap *regmap;
> -	int n_consumers;
> +	unsigned int n_consumers;
>  	int device_type;
>  	int ret;
> -	int i;
> +	unsigned int i;
>  
>  	if (!adev)
>  		return -ENODEV;
>  
> -	n_consumers = skl_int3472_fill_clk_pdata(&client->dev, &clk_pdata);
> -	if (n_consumers < 0)
> -		return n_consumers;
> +	/*
> +	 * Look up board data before building clock platform data.  On
> +	 * platforms where a sensor's ACPI _DEP does not list the INT3472
> +	 * device, for_each_acpi_consumer_dev() misses that sensor and its
> +	 * clock consumer entry is never registered.  Board data can supply
> +	 * a static consumer list to use instead, bypassing the broken _DEP
> +	 * traversal.
> +	 */
> +	board_data = int3472_tps68470_get_board_data(dev_name(&client->dev));
> +
> +	if (board_data && board_data->n_clk_consumers) {
> +		clk_pdata = devm_kzalloc(&client->dev,
> +					 struct_size(clk_pdata, consumers,
> +						     board_data->n_clk_consumers),
> +					 GFP_KERNEL);
> +		if (!clk_pdata)
> +			return -ENOMEM;
> +		clk_pdata->n_consumers = board_data->n_clk_consumers;
> +		for (i = 0; i < board_data->n_clk_consumers; i++)
> +			clk_pdata->consumers[i] = board_data->clk_consumers[i];
> +		n_consumers = board_data->n_clk_consumers;
> +	} else {
> +		n_consumers = skl_int3472_fill_clk_pdata(&client->dev, &clk_pdata);
> +		if (n_consumers < 0)

n_consumers is now unsigned int so this check will always be false. I'd
ust make it int again.

Also, the code above is specific to systems shipped with Windows and is now
even more outsized compared to simply creating the MFD devices for ChromeOS
systems. Can you return from the ChromeOS and device type error checks in
the switch() below and move the Windows case out of the switch()?

> +			return n_consumers;
> +	}
>  
>  	regmap = devm_regmap_init_i2c(client, &tps68470_regmap_config);
>  	if (IS_ERR(regmap)) {
> @@ -176,7 +199,6 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
>  	device_type = skl_int3472_tps68470_calc_type(adev);
>  	switch (device_type) {
>  	case DESIGNED_FOR_WINDOWS:
> -		board_data = int3472_tps68470_get_board_data(dev_name(&client->dev));
>  		if (!board_data)
>  			return dev_err_probe(&client->dev, -ENODEV, "No board-data found for this model\n");
>  
> @@ -233,7 +255,7 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
>  static void skl_int3472_tps68470_remove(struct i2c_client *client)
>  {
>  	const struct int3472_tps68470_board_data *board_data;
> -	int i;
> +	unsigned int i;
>  
>  	board_data = int3472_tps68470_get_board_data(dev_name(&client->dev));
>  	if (board_data) {
> diff --git a/drivers/platform/x86/intel/int3472/tps68470.h b/drivers/platform/x86/intel/int3472/tps68470.h
> index 35915e701..1d3d67459 100644
> --- a/drivers/platform/x86/intel/int3472/tps68470.h
> +++ b/drivers/platform/x86/intel/int3472/tps68470.h
> @@ -12,11 +12,21 @@
>  #define _INTEL_SKL_INT3472_TPS68470_H
>  
>  struct gpiod_lookup_table;
> +struct tps68470_clk_consumer;
>  struct tps68470_regulator_platform_data;
>  
>  struct int3472_tps68470_board_data {
>  	const char *dev_name;
>  	const struct tps68470_regulator_platform_data *tps68470_regulator_pdata;
> +	/*
> +	 * Static clock consumers.  When n_clk_consumers is non-zero these
> +	 * are used in place of for_each_acpi_consumer_dev() to build the
> +	 * tps68470-clk platform data.  Needed on platforms where a sensor's
> +	 * ACPI _DEP does not list the INT3472 device, causing that sensor
> +	 * to be missed by the ACPI dependency traversal.
> +	 */
> +	unsigned int n_clk_consumers;
> +	const struct tps68470_clk_consumer *clk_consumers;
>  	unsigned int n_gpiod_lookups;
>  	struct gpiod_lookup_table *tps68470_gpio_lookup_tables[];
>  };

-- 
Kind regards,

Sakari Ailus