[PATCH 0/3] Support OV5670 on IPU3 devices

[PATCH 0/3] Support OV5670 on IPU3 devices

[Daniel Scally]

Hello all

The OV5670 is found in the Dell 7212 tablet, along with an IPU3 ISP. This small
series adds support for the sensor by connecting it through the ipu-bridge and
adding board data for the PMIC so that it can be powered.

Regards
Dan

Daniel Scally (3):
  media: i2c: Defer ov5670_probe() if endpoint not found
  platform/x86: int3472: Add board data for Dell 7212
  media: ipu-bridge: Add _HID for OV5670

 drivers/media/i2c/ov5670.c                    |   9 +-
 drivers/media/pci/intel/ipu-bridge.c          |   2 +
 .../x86/intel/int3472/tps68470_board_data.c   | 128 ++++++++++++++++++
 3 files changed, 137 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH 1/3] media: i2c: Defer ov5670_probe() if endpoint not found

[Daniel Scally]

The OV5670 is found on a Dell 7212, coupled to Intel's CIO2 CSI-2
receiver. The ACPI tables don't describe the connection properly and
so this is one of the sensors that needs to wait for the ipu-bridge
to set up the graph with software nodes. Defer probe if an endpoint
isn't found to allow time for that.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 drivers/media/i2c/ov5670.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
index 2aee85965cf7..c02d42998fcb 100644
--- a/drivers/media/i2c/ov5670.c
+++ b/drivers/media/i2c/ov5670.c
@@ -2688,10 +2688,15 @@ static int ov5670_probe(struct i2c_client *client)
 	if (ret)
 		return dev_err_probe(&client->dev, ret, "GPIO probe failed\n");
 
-	/* Graph Endpoint */
+	/*
+	 * Graph Endpoint. If it's missing we defer rather than fail, as this
+	 * sensor is known to co-exist on systems with the IPU3 and so it might
+	 * be created by the ipu-bridge.
+	 */
 	handle = fwnode_graph_get_next_endpoint(dev_fwnode(&client->dev), NULL);
 	if (!handle)
-		return dev_err_probe(&client->dev, -ENXIO, "Endpoint for node get failed\n");
+		return dev_err_probe(&client->dev, -EPROBE_DEFER,
+				     "Endpoint for node get failed\n");
 
 	ov5670->endpoint.bus_type = V4L2_MBUS_CSI2_DPHY;
 	ov5670->endpoint.bus.mipi_csi2.num_data_lanes = 2;
-- 
2.34.1

[PATCH 2/3] platform/x86: int3472: Add board data for Dell 7212

[Daniel Scally]

The Dell 7212 Rugged Extreme Tablet pairs an OV5670 sensor with the
Intel IPU3 ISP. The sensor is powered by a TPS68470 PMIC, and so we
need some board data to describe how to configure the GPIOs and
regulators to run the sensor.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 .../x86/intel/int3472/tps68470_board_data.c   | 128 ++++++++++++++++++
 1 file changed, 128 insertions(+)

diff --git a/drivers/platform/x86/intel/int3472/tps68470_board_data.c b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
index 322237e056f3..d28053733bd2 100644
--- a/drivers/platform/x86/intel/int3472/tps68470_board_data.c
+++ b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
@@ -129,6 +129,109 @@ static const struct tps68470_regulator_platform_data surface_go_tps68470_pdata =
 	},
 };
 
+/* Settings for Dell 7212 Tablet */
+
+static struct regulator_consumer_supply int3479_vsio_consumer_supplies[] = {
+	REGULATOR_SUPPLY("avdd", "i2c-INT3479:00"),
+};
+
+static struct regulator_consumer_supply int3479_aux1_consumer_supplies[] = {
+	REGULATOR_SUPPLY("dvdd", "i2c-INT3479:00"),
+};
+
+static struct regulator_consumer_supply int3479_aux2_consumer_supplies[] = {
+	REGULATOR_SUPPLY("dovdd", "i2c-INT3479:00"),
+};
+
+static const struct regulator_init_data dell_7212_tps68470_core_reg_init_data = {
+	.constraints = {
+		.min_uV = 1200000,
+		.max_uV = 1200000,
+		.apply_uV = 1,
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies = 0,
+	.consumer_supplies = NULL
+};
+
+static const struct regulator_init_data dell_7212_tps68470_ana_reg_init_data = {
+	.constraints = {
+		.min_uV = 2815200,
+		.max_uV = 2815200,
+		.apply_uV = 1,
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies = 0,
+	.consumer_supplies = NULL
+};
+
+static const struct regulator_init_data dell_7212_tps68470_vcm_reg_init_data = {
+	.constraints = {
+		.min_uV = 2815200,
+		.max_uV = 2815200,
+		.apply_uV = 1,
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies = 0,
+	.consumer_supplies = NULL
+};
+
+static const struct regulator_init_data dell_7212_tps68470_vio_reg_init_data = {
+	.constraints = {
+		.min_uV = 1800600,
+		.max_uV = 1800600,
+		.apply_uV = 1,
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies = 0,
+	.consumer_supplies = NULL,
+};
+
+static const struct regulator_init_data dell_7212_tps68470_vsio_reg_init_data = {
+	.constraints = {
+		.min_uV = 1800600,
+		.max_uV = 1800600,
+		.apply_uV = 1,
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies = ARRAY_SIZE(int3479_vsio_consumer_supplies),
+	.consumer_supplies = int3479_vsio_consumer_supplies,
+};
+
+static const struct regulator_init_data dell_7212_tps68470_aux1_reg_init_data = {
+	.constraints = {
+		.min_uV = 1213200,
+		.max_uV = 1213200,
+		.apply_uV = 1,
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies = ARRAY_SIZE(int3479_aux1_consumer_supplies),
+	.consumer_supplies = int3479_aux1_consumer_supplies,
+};
+
+static const struct regulator_init_data dell_7212_tps68470_aux2_reg_init_data = {
+	.constraints = {
+		.min_uV = 1800600,
+		.max_uV = 1800600,
+		.apply_uV = 1,
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+	.num_consumer_supplies = ARRAY_SIZE(int3479_aux2_consumer_supplies),
+	.consumer_supplies = int3479_aux2_consumer_supplies,
+};
+
+static const struct tps68470_regulator_platform_data dell_7212_tps68470_pdata = {
+	.reg_init_data = {
+		[TPS68470_CORE] = &dell_7212_tps68470_core_reg_init_data,
+		[TPS68470_ANA]  = &dell_7212_tps68470_ana_reg_init_data,
+		[TPS68470_VCM]  = &dell_7212_tps68470_vcm_reg_init_data,
+		[TPS68470_VIO] = &dell_7212_tps68470_vio_reg_init_data,
+		[TPS68470_VSIO] = &dell_7212_tps68470_vsio_reg_init_data,
+		[TPS68470_AUX1] = &dell_7212_tps68470_aux1_reg_init_data,
+		[TPS68470_AUX2] = &dell_7212_tps68470_aux2_reg_init_data,
+	},
+};
+
 static struct gpiod_lookup_table surface_go_int347a_gpios = {
 	.dev_id = "i2c-INT347A:00",
 	.table = {
@@ -146,6 +249,15 @@ static struct gpiod_lookup_table surface_go_int347e_gpios = {
 	}
 };
 
+static struct gpiod_lookup_table dell_7212_int3479_gpios = {
+	.dev_id = "i2c-INT3479:00",
+	.table = {
+		GPIO_LOOKUP("tps68470-gpio", 3, "reset", GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP("tps68470-gpio", 4, "powerdown", GPIO_ACTIVE_LOW),
+		{ }
+	}
+};
+
 static const struct int3472_tps68470_board_data surface_go_tps68470_board_data = {
 	.dev_name = "i2c-INT3472:05",
 	.tps68470_regulator_pdata = &surface_go_tps68470_pdata,
@@ -166,6 +278,15 @@ static const struct int3472_tps68470_board_data surface_go3_tps68470_board_data
 	},
 };
 
+static const struct int3472_tps68470_board_data dell_7212_tps68470_board_data = {
+	.dev_name = "i2c-INT3472:05",
+	.tps68470_regulator_pdata = &dell_7212_tps68470_pdata,
+	.n_gpiod_lookups = 1,
+	.tps68470_gpio_lookup_tables = {
+		&dell_7212_int3479_gpios,
+	},
+};
+
 static const struct dmi_system_id int3472_tps68470_board_data_table[] = {
 	{
 		.matches = {
@@ -188,6 +309,13 @@ static const struct dmi_system_id int3472_tps68470_board_data_table[] = {
 		},
 		.driver_data = (void *)&surface_go3_tps68470_board_data,
 	},
+	{
+		.matches = {
+			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Latitude 7212 Rugged Extreme Tablet"),
+		},
+		.driver_data = (void *)&dell_7212_tps68470_board_data,
+	},
 	{ }
 };
 
-- 
2.34.1

[PATCH 3/3] media: ipu-bridge: Add _HID for OV5670

[Daniel Scally]

The OV5670 is found on Dell 7212 tablets paired with an IPU3 ISP
and needs to be connected by the ipu-bridge. Add it to the list
of supported devices.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 drivers/media/pci/intel/ipu-bridge.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/pci/intel/ipu-bridge.c b/drivers/media/pci/intel/ipu-bridge.c
index a0e9a71580b5..4e921c751629 100644
--- a/drivers/media/pci/intel/ipu-bridge.c
+++ b/drivers/media/pci/intel/ipu-bridge.c
@@ -59,6 +59,8 @@ static const struct ipu_sensor_config ipu_supported_sensors[] = {
 	IPU_SENSOR_CONFIG("INT33BE", 1, 419200000),
 	/* Omnivision OV2740 */
 	IPU_SENSOR_CONFIG("INT3474", 1, 180000000),
+	/* Omnivision OV5670 */
+	IPU_SENSOR_CONFIG("INT3479", 1, 422400000),
 	/* Omnivision OV8865 */
 	IPU_SENSOR_CONFIG("INT347A", 1, 360000000),
 	/* Omnivision OV7251 */
-- 
2.34.1

Re: [PATCH 2/3] platform/x86: int3472: Add board data for Dell 7212

[Ilpo Järvinen]

On Tue, 15 Oct 2024, Daniel Scally wrote:

> The Dell 7212 Rugged Extreme Tablet pairs an OV5670 sensor with the
> Intel IPU3 ISP. The sensor is powered by a TPS68470 PMIC, and so we
> need some board data to describe how to configure the GPIOs and
> regulators to run the sensor.
> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
>  .../x86/intel/int3472/tps68470_board_data.c   | 128 ++++++++++++++++++
>  1 file changed, 128 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel/int3472/tps68470_board_data.c b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> index 322237e056f3..d28053733bd2 100644
> --- a/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> +++ b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> @@ -129,6 +129,109 @@ static const struct tps68470_regulator_platform_data surface_go_tps68470_pdata =
>  	},
>  };
>  
> +/* Settings for Dell 7212 Tablet */
> +
> +static struct regulator_consumer_supply int3479_vsio_consumer_supplies[] = {
> +	REGULATOR_SUPPLY("avdd", "i2c-INT3479:00"),
> +};
> +
> +static struct regulator_consumer_supply int3479_aux1_consumer_supplies[] = {
> +	REGULATOR_SUPPLY("dvdd", "i2c-INT3479:00"),
> +};
> +
> +static struct regulator_consumer_supply int3479_aux2_consumer_supplies[] = {
> +	REGULATOR_SUPPLY("dovdd", "i2c-INT3479:00"),
> +};
> +
> +static const struct regulator_init_data dell_7212_tps68470_core_reg_init_data = {
> +	.constraints = {
> +		.min_uV = 1200000,
> +		.max_uV = 1200000,
> +		.apply_uV = 1,
> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> +	},
> +	.num_consumer_supplies = 0,
> +	.consumer_supplies = NULL

Add comma to any non-terminator entry.

> +};
> +
> +static const struct regulator_init_data dell_7212_tps68470_ana_reg_init_data = {
> +	.constraints = {
> +		.min_uV = 2815200,
> +		.max_uV = 2815200,
> +		.apply_uV = 1,
> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> +	},
> +	.num_consumer_supplies = 0,
> +	.consumer_supplies = NULL
> +};
> +
> +static const struct regulator_init_data dell_7212_tps68470_vcm_reg_init_data = {
> +	.constraints = {
> +		.min_uV = 2815200,
> +		.max_uV = 2815200,
> +		.apply_uV = 1,
> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> +	},
> +	.num_consumer_supplies = 0,
> +	.consumer_supplies = NULL
> +};

This looks exactly identical to dell_7212_tps68470_ana_reg_init_data.

> +static const struct regulator_init_data dell_7212_tps68470_vio_reg_init_data = {
> +	.constraints = {
> +		.min_uV = 1800600,
> +		.max_uV = 1800600,
> +		.apply_uV = 1,
> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> +	},
> +	.num_consumer_supplies = 0,
> +	.consumer_supplies = NULL,
> +};
> +
> +static const struct regulator_init_data dell_7212_tps68470_vsio_reg_init_data = {
> +	.constraints = {
> +		.min_uV = 1800600,
> +		.max_uV = 1800600,
> +		.apply_uV = 1,
> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> +	},
> +	.num_consumer_supplies = ARRAY_SIZE(int3479_vsio_consumer_supplies),
> +	.consumer_supplies = int3479_vsio_consumer_supplies,
> +};
> +
> +static const struct regulator_init_data dell_7212_tps68470_aux1_reg_init_data = {
> +	.constraints = {
> +		.min_uV = 1213200,
> +		.max_uV = 1213200,
> +		.apply_uV = 1,
> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> +	},
> +	.num_consumer_supplies = ARRAY_SIZE(int3479_aux1_consumer_supplies),
> +	.consumer_supplies = int3479_aux1_consumer_supplies,
> +};
> +
> +static const struct regulator_init_data dell_7212_tps68470_aux2_reg_init_data = {
> +	.constraints = {
> +		.min_uV = 1800600,
> +		.max_uV = 1800600,
> +		.apply_uV = 1,
> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> +	},
> +	.num_consumer_supplies = ARRAY_SIZE(int3479_aux2_consumer_supplies),
> +	.consumer_supplies = int3479_aux2_consumer_supplies,
> +};
> +
> +static const struct tps68470_regulator_platform_data dell_7212_tps68470_pdata = {
> +	.reg_init_data = {
> +		[TPS68470_CORE] = &dell_7212_tps68470_core_reg_init_data,
> +		[TPS68470_ANA]  = &dell_7212_tps68470_ana_reg_init_data,
> +		[TPS68470_VCM]  = &dell_7212_tps68470_vcm_reg_init_data,
> +		[TPS68470_VIO] = &dell_7212_tps68470_vio_reg_init_data,

Inconsistent spaces.

-- 
 i.

> +		[TPS68470_VSIO] = &dell_7212_tps68470_vsio_reg_init_data,
> +		[TPS68470_AUX1] = &dell_7212_tps68470_aux1_reg_init_data,
> +		[TPS68470_AUX2] = &dell_7212_tps68470_aux2_reg_init_data,
> +	},
> +};
> +
>  static struct gpiod_lookup_table surface_go_int347a_gpios = {
>  	.dev_id = "i2c-INT347A:00",
>  	.table = {
> @@ -146,6 +249,15 @@ static struct gpiod_lookup_table surface_go_int347e_gpios = {
>  	}
>  };
>  
> +static struct gpiod_lookup_table dell_7212_int3479_gpios = {
> +	.dev_id = "i2c-INT3479:00",
> +	.table = {
> +		GPIO_LOOKUP("tps68470-gpio", 3, "reset", GPIO_ACTIVE_LOW),
> +		GPIO_LOOKUP("tps68470-gpio", 4, "powerdown", GPIO_ACTIVE_LOW),
> +		{ }
> +	}
> +};
> +
>  static const struct int3472_tps68470_board_data surface_go_tps68470_board_data = {
>  	.dev_name = "i2c-INT3472:05",
>  	.tps68470_regulator_pdata = &surface_go_tps68470_pdata,
> @@ -166,6 +278,15 @@ static const struct int3472_tps68470_board_data surface_go3_tps68470_board_data
>  	},
>  };
>  
> +static const struct int3472_tps68470_board_data dell_7212_tps68470_board_data = {
> +	.dev_name = "i2c-INT3472:05",
> +	.tps68470_regulator_pdata = &dell_7212_tps68470_pdata,
> +	.n_gpiod_lookups = 1,
> +	.tps68470_gpio_lookup_tables = {
> +		&dell_7212_int3479_gpios,
> +	},
> +};
> +
>  static const struct dmi_system_id int3472_tps68470_board_data_table[] = {
>  	{
>  		.matches = {
> @@ -188,6 +309,13 @@ static const struct dmi_system_id int3472_tps68470_board_data_table[] = {
>  		},
>  		.driver_data = (void *)&surface_go3_tps68470_board_data,
>  	},
> +	{
> +		.matches = {
> +			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +			DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Latitude 7212 Rugged Extreme Tablet"),
> +		},
> +		.driver_data = (void *)&dell_7212_tps68470_board_data,
> +	},
>  	{ }
>  };
>  
> 

Re: [PATCH 2/3] platform/x86: int3472: Add board data for Dell 7212

[Dan Scally]

Afternoon - thanks for the review

On 16/10/2024 12:58, Ilpo Järvinen wrote:
> On Tue, 15 Oct 2024, Daniel Scally wrote:
>
>> The Dell 7212 Rugged Extreme Tablet pairs an OV5670 sensor with the
>> Intel IPU3 ISP. The sensor is powered by a TPS68470 PMIC, and so we
>> need some board data to describe how to configure the GPIOs and
>> regulators to run the sensor.
>>
>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
>> ---
>>   .../x86/intel/int3472/tps68470_board_data.c   | 128 ++++++++++++++++++
>>   1 file changed, 128 insertions(+)
>>
>> diff --git a/drivers/platform/x86/intel/int3472/tps68470_board_data.c b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
>> index 322237e056f3..d28053733bd2 100644
>> --- a/drivers/platform/x86/intel/int3472/tps68470_board_data.c
>> +++ b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
>> @@ -129,6 +129,109 @@ static const struct tps68470_regulator_platform_data surface_go_tps68470_pdata =
>>   	},
>>   };
>>   
>> +/* Settings for Dell 7212 Tablet */
>> +
>> +static struct regulator_consumer_supply int3479_vsio_consumer_supplies[] = {
>> +	REGULATOR_SUPPLY("avdd", "i2c-INT3479:00"),
>> +};
>> +
>> +static struct regulator_consumer_supply int3479_aux1_consumer_supplies[] = {
>> +	REGULATOR_SUPPLY("dvdd", "i2c-INT3479:00"),
>> +};
>> +
>> +static struct regulator_consumer_supply int3479_aux2_consumer_supplies[] = {
>> +	REGULATOR_SUPPLY("dovdd", "i2c-INT3479:00"),
>> +};
>> +
>> +static const struct regulator_init_data dell_7212_tps68470_core_reg_init_data = {
>> +	.constraints = {
>> +		.min_uV = 1200000,
>> +		.max_uV = 1200000,
>> +		.apply_uV = 1,
>> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
>> +	},
>> +	.num_consumer_supplies = 0,
>> +	.consumer_supplies = NULL
> Add comma to any non-terminator entry.
Ack
>
>> +};
>> +
>> +static const struct regulator_init_data dell_7212_tps68470_ana_reg_init_data = {
>> +	.constraints = {
>> +		.min_uV = 2815200,
>> +		.max_uV = 2815200,
>> +		.apply_uV = 1,
>> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
>> +	},
>> +	.num_consumer_supplies = 0,
>> +	.consumer_supplies = NULL
>> +};
>> +
>> +static const struct regulator_init_data dell_7212_tps68470_vcm_reg_init_data = {
>> +	.constraints = {
>> +		.min_uV = 2815200,
>> +		.max_uV = 2815200,
>> +		.apply_uV = 1,
>> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
>> +	},
>> +	.num_consumer_supplies = 0,
>> +	.consumer_supplies = NULL
>> +};
> This looks exactly identical to dell_7212_tps68470_ana_reg_init_data.

It is the same currently, but only because I've not added the consumers yet - largely because the 
sensor/vcm combination that will consume these lines needs additional driver work anyway. When 
they're done I'd plan to add consumer definitions for these regulators too.


Thanks

Dan

>
>> +static const struct regulator_init_data dell_7212_tps68470_vio_reg_init_data = {
>> +	.constraints = {
>> +		.min_uV = 1800600,
>> +		.max_uV = 1800600,
>> +		.apply_uV = 1,
>> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
>> +	},
>> +	.num_consumer_supplies = 0,
>> +	.consumer_supplies = NULL,
>> +};
>> +
>> +static const struct regulator_init_data dell_7212_tps68470_vsio_reg_init_data = {
>> +	.constraints = {
>> +		.min_uV = 1800600,
>> +		.max_uV = 1800600,
>> +		.apply_uV = 1,
>> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
>> +	},
>> +	.num_consumer_supplies = ARRAY_SIZE(int3479_vsio_consumer_supplies),
>> +	.consumer_supplies = int3479_vsio_consumer_supplies,
>> +};
>> +
>> +static const struct regulator_init_data dell_7212_tps68470_aux1_reg_init_data = {
>> +	.constraints = {
>> +		.min_uV = 1213200,
>> +		.max_uV = 1213200,
>> +		.apply_uV = 1,
>> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
>> +	},
>> +	.num_consumer_supplies = ARRAY_SIZE(int3479_aux1_consumer_supplies),
>> +	.consumer_supplies = int3479_aux1_consumer_supplies,
>> +};
>> +
>> +static const struct regulator_init_data dell_7212_tps68470_aux2_reg_init_data = {
>> +	.constraints = {
>> +		.min_uV = 1800600,
>> +		.max_uV = 1800600,
>> +		.apply_uV = 1,
>> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
>> +	},
>> +	.num_consumer_supplies = ARRAY_SIZE(int3479_aux2_consumer_supplies),
>> +	.consumer_supplies = int3479_aux2_consumer_supplies,
>> +};
>> +
>> +static const struct tps68470_regulator_platform_data dell_7212_tps68470_pdata = {
>> +	.reg_init_data = {
>> +		[TPS68470_CORE] = &dell_7212_tps68470_core_reg_init_data,
>> +		[TPS68470_ANA]  = &dell_7212_tps68470_ana_reg_init_data,
>> +		[TPS68470_VCM]  = &dell_7212_tps68470_vcm_reg_init_data,
>> +		[TPS68470_VIO] = &dell_7212_tps68470_vio_reg_init_data,
> Inconsistent spaces.
>

Re: [PATCH 2/3] platform/x86: int3472: Add board data for Dell 7212

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 5325 bytes --]

On Wed, 16 Oct 2024, Dan Scally wrote:

> Afternoon - thanks for the review
> 
> On 16/10/2024 12:58, Ilpo Järvinen wrote:
> > On Tue, 15 Oct 2024, Daniel Scally wrote:
> > 
> > > The Dell 7212 Rugged Extreme Tablet pairs an OV5670 sensor with the
> > > Intel IPU3 ISP. The sensor is powered by a TPS68470 PMIC, and so we
> > > need some board data to describe how to configure the GPIOs and
> > > regulators to run the sensor.
> > > 
> > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> > > ---
> > >   .../x86/intel/int3472/tps68470_board_data.c   | 128 ++++++++++++++++++
> > >   1 file changed, 128 insertions(+)
> > > 
> > > diff --git a/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> > > b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> > > index 322237e056f3..d28053733bd2 100644
> > > --- a/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> > > +++ b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
> > > @@ -129,6 +129,109 @@ static const struct tps68470_regulator_platform_data
> > > surface_go_tps68470_pdata =
> > >   	},
> > >   };
> > >   +/* Settings for Dell 7212 Tablet */
> > > +
> > > +static struct regulator_consumer_supply int3479_vsio_consumer_supplies[]
> > > = {
> > > +	REGULATOR_SUPPLY("avdd", "i2c-INT3479:00"),
> > > +};
> > > +
> > > +static struct regulator_consumer_supply int3479_aux1_consumer_supplies[]
> > > = {
> > > +	REGULATOR_SUPPLY("dvdd", "i2c-INT3479:00"),
> > > +};
> > > +
> > > +static struct regulator_consumer_supply int3479_aux2_consumer_supplies[]
> > > = {
> > > +	REGULATOR_SUPPLY("dovdd", "i2c-INT3479:00"),
> > > +};
> > > +
> > > +static const struct regulator_init_data
> > > dell_7212_tps68470_core_reg_init_data = {
> > > +	.constraints = {
> > > +		.min_uV = 1200000,
> > > +		.max_uV = 1200000,
> > > +		.apply_uV = 1,
> > > +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> > > +	},
> > > +	.num_consumer_supplies = 0,
> > > +	.consumer_supplies = NULL
> > Add comma to any non-terminator entry.
> Ack
> > 
> > > +};
> > > +
> > > +static const struct regulator_init_data
> > > dell_7212_tps68470_ana_reg_init_data = {
> > > +	.constraints = {
> > > +		.min_uV = 2815200,
> > > +		.max_uV = 2815200,
> > > +		.apply_uV = 1,
> > > +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> > > +	},
> > > +	.num_consumer_supplies = 0,
> > > +	.consumer_supplies = NULL
> > > +};
> > > +
> > > +static const struct regulator_init_data
> > > dell_7212_tps68470_vcm_reg_init_data = {
> > > +	.constraints = {
> > > +		.min_uV = 2815200,
> > > +		.max_uV = 2815200,
> > > +		.apply_uV = 1,
> > > +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> > > +	},
> > > +	.num_consumer_supplies = 0,
> > > +	.consumer_supplies = NULL
> > > +};
> > This looks exactly identical to dell_7212_tps68470_ana_reg_init_data.
> 
> It is the same currently, but only because I've not added the consumers yet -
> largely because the sensor/vcm combination that will consume these lines needs
> additional driver work anyway. When they're done I'd plan to add consumer
> definitions for these regulators too.

Fair enough. Thanks.

--
 i.

> Thanks
> 
> Dan
> 
> > 
> > > +static const struct regulator_init_data
> > > dell_7212_tps68470_vio_reg_init_data = {
> > > +	.constraints = {
> > > +		.min_uV = 1800600,
> > > +		.max_uV = 1800600,
> > > +		.apply_uV = 1,
> > > +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> > > +	},
> > > +	.num_consumer_supplies = 0,
> > > +	.consumer_supplies = NULL,
> > > +};
> > > +
> > > +static const struct regulator_init_data
> > > dell_7212_tps68470_vsio_reg_init_data = {
> > > +	.constraints = {
> > > +		.min_uV = 1800600,
> > > +		.max_uV = 1800600,
> > > +		.apply_uV = 1,
> > > +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> > > +	},
> > > +	.num_consumer_supplies = ARRAY_SIZE(int3479_vsio_consumer_supplies),
> > > +	.consumer_supplies = int3479_vsio_consumer_supplies,
> > > +};
> > > +
> > > +static const struct regulator_init_data
> > > dell_7212_tps68470_aux1_reg_init_data = {
> > > +	.constraints = {
> > > +		.min_uV = 1213200,
> > > +		.max_uV = 1213200,
> > > +		.apply_uV = 1,
> > > +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> > > +	},
> > > +	.num_consumer_supplies = ARRAY_SIZE(int3479_aux1_consumer_supplies),
> > > +	.consumer_supplies = int3479_aux1_consumer_supplies,
> > > +};
> > > +
> > > +static const struct regulator_init_data
> > > dell_7212_tps68470_aux2_reg_init_data = {
> > > +	.constraints = {
> > > +		.min_uV = 1800600,
> > > +		.max_uV = 1800600,
> > > +		.apply_uV = 1,
> > > +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> > > +	},
> > > +	.num_consumer_supplies = ARRAY_SIZE(int3479_aux2_consumer_supplies),
> > > +	.consumer_supplies = int3479_aux2_consumer_supplies,
> > > +};
> > > +
> > > +static const struct tps68470_regulator_platform_data
> > > dell_7212_tps68470_pdata = {
> > > +	.reg_init_data = {
> > > +		[TPS68470_CORE] = &dell_7212_tps68470_core_reg_init_data,
> > > +		[TPS68470_ANA]  = &dell_7212_tps68470_ana_reg_init_data,
> > > +		[TPS68470_VCM]  = &dell_7212_tps68470_vcm_reg_init_data,
> > > +		[TPS68470_VIO] = &dell_7212_tps68470_vio_reg_init_data,
> > Inconsistent spaces.
> > 
>