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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- 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.
> > 
>