Re: [PATCH 5/5] platform: int3472: Add MSI prestige board data

[Hans de Goede <hansg@kernel.org>]

Hi,

On 10-Mar-26 15:32, Dan Scally wrote:
> Hi Antti
> 
> On 10/03/2026 12:44, Antti Laakso wrote:
>> Define regulators and gpios for MSI Prestige 14 AI EVO+ laptop.
>>
>> Signed-off-by: Antti Laakso <antti.laakso@linux.intel.com>
>> ---
>>   .../x86/intel/int3472/tps68470_board_data.c   | 97 +++++++++++++++++++
>>   1 file changed, 97 insertions(+)
>>
>> diff --git a/drivers/platform/x86/intel/int3472/tps68470_board_data.c b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
>> index 71357a036292..fe7c23e72d66 100644
>> --- a/drivers/platform/x86/intel/int3472/tps68470_board_data.c
>> +++ b/drivers/platform/x86/intel/int3472/tps68470_board_data.c
>> @@ -232,6 +232,73 @@ static const struct tps68470_regulator_platform_data dell_7212_tps68470_pdata =
>>       },
>>   };
>>   +/* Settings for MSI Prestige 14 laptop. */
>> +
>> +static struct regulator_consumer_supply ovti5675_avdd_consumer_supplies[] = {
>> +    REGULATOR_SUPPLY("avdd", "i2c-OVTI5675:00"),
>> +};
>> +
>> +static struct regulator_consumer_supply ovti5675_dovdd_consumer_supplies[] = {
>> +    REGULATOR_SUPPLY("dovdd", "i2c-OVTI5675:00"),
>> +};
>> +
>> +static struct regulator_consumer_supply ovti5675_dvdd_consumer_supplies[] = {
>> +    REGULATOR_SUPPLY("dvdd", "i2c-OVTI5675:00"),
>> +};
>> +
>> +static const struct regulator_init_data msi_p14_ai_evo_tps68470_core_reg_init_data = {
>> +    .constraints = {
>> +        .min_uV = 1200000,
>> +        .max_uV = 1200000,
>> +        .apply_uV = 1,
>> +        .valid_ops_mask = REGULATOR_CHANGE_STATUS,
>> +    },
>> +    .num_consumer_supplies = ARRAY_SIZE(ovti5675_dvdd_consumer_supplies),
>> +    .consumer_supplies = ovti5675_dvdd_consumer_supplies,
>> +};
>> +
>> +static const struct regulator_init_data msi_p14_ai_evo_tps68470_ana_reg_init_data = {
>> +    .constraints = {
>> +        .min_uV = 2815200,
>> +        .max_uV = 2815200,
>> +        .apply_uV = 1,
>> +        .valid_ops_mask = REGULATOR_CHANGE_STATUS,
>> +    },
>> +    .num_consumer_supplies = ARRAY_SIZE(ovti5675_avdd_consumer_supplies),
>> +    .consumer_supplies = ovti5675_avdd_consumer_supplies,
>> +};
>> +
>> +static const struct regulator_init_data msi_p14_ai_evo_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 msi_p14_ai_evo_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(ovti5675_dovdd_consumer_supplies),
>> +    .consumer_supplies = ovti5675_dovdd_consumer_supplies,
>> +};
>> +
>> +static const struct tps68470_regulator_platform_data msi_p14_ai_evo_tps68470_pdata = {
>> +    .reg_init_data = {
>> +        [TPS68470_CORE] = &msi_p14_ai_evo_tps68470_core_reg_init_data,
>> +        [TPS68470_ANA]  = &msi_p14_ai_evo_tps68470_ana_reg_init_data,
>> +        [TPS68470_VIO]  = &msi_p14_ai_evo_tps68470_vio_reg_init_data,
>> +        [TPS68470_VSIO] = &msi_p14_ai_evo_tps68470_vsio_reg_init_data,
>> +    },
>> +};
>> +
>>   static struct gpiod_lookup_table surface_go_int347a_gpios = {
>>       .dev_id = "i2c-INT347A:00",
>>       .table = {
>> @@ -258,6 +325,19 @@ static struct gpiod_lookup_table dell_7212_int3479_gpios = {
>>       }
>>   };
>>   +static struct gpiod_lookup_table msi_p14_ai_evo_ovti5675_gpios = {
>> +    .dev_id = "i2c-OVTI5675:00",
>> +    .table = {
>> +        GPIO_LOOKUP_IDX("tps68470-gpio", 9, "reset", 0, GPIO_ACTIVE_LOW),
>> +        GPIO_LOOKUP_IDX("tps68470-gpio", 7, "reset", 1, GPIO_ACTIVE_LOW),
> 
> The ov5675 driver seems only to look for a single gpio, so I think the second entry would never be accessed here...should there be an accompanying driver change?

Ah I missed this, yes having 2 entries mapping to the same label is not
useful. The first entry (with index 0) will simply always be used in that
case.

Also this should use plain GPIO_LOOKUP() since the index part (the only
difference here) is never used in sensor drivers.

Regards,

Hans





> 
> Thanks
> Dan
> 
>> +        { }
>> +    }
>> +};
>> +
>> +static const struct tps68470_gpio_platform_data msi_p14_ai_evo_tps68470_gpio_pdata = {
>> +    .daisy_chain_enable = true,
>> +};
>> +
>>   static const struct int3472_tps68470_board_data surface_go_tps68470_board_data = {
>>       .dev_name = "i2c-INT3472:05",
>>       .tps68470_regulator_pdata = &surface_go_tps68470_pdata,
>> @@ -287,6 +367,16 @@ static const struct int3472_tps68470_board_data dell_7212_tps68470_board_data =
>>       },
>>   };
>>   +static const struct int3472_tps68470_board_data msi_p14_ai_evo_tps68470_board_data = {
>> +    .dev_name = "i2c-INT3472:06",
>> +    .tps68470_regulator_pdata = &msi_p14_ai_evo_tps68470_pdata,
>> +    .tps68470_gpio_pdata = &msi_p14_ai_evo_tps68470_gpio_pdata,
>> +    .n_gpiod_lookups = 1,
>> +    .tps68470_gpio_lookup_tables = {
>> +        &msi_p14_ai_evo_ovti5675_gpios,
>> +    },
>> +};
>> +
>>   static const struct dmi_system_id int3472_tps68470_board_data_table[] = {
>>       {
>>           .matches = {
>> @@ -316,6 +406,13 @@ static const struct dmi_system_id int3472_tps68470_board_data_table[] = {
>>           },
>>           .driver_data = (void *)&dell_7212_tps68470_board_data,
>>       },
>> +    {
>> +        .matches = {
>> +            DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Micro-Star International Co., Ltd."),
>> +            DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Prestige 14 AI+ Evo C2VMG"),
>> +        },
>> +        .driver_data = (void *)&msi_p14_ai_evo_tps68470_board_data,
>> +    },
>>       { }
>>   };
>>   
>