Re: [PATCH 3/5] platform: int3472: Add gpio platform data

[Antti Laakso <antti.laakso@linux.intel.com>]

Hi Bartosz,
On Tue, Mar 10, 2026 at 06:32:31AM -0700, Bartosz Golaszewski wrote:
> On Tue, 10 Mar 2026 13:44:25 +0100, Antti Laakso
> <antti.laakso@linux.intel.com> said:
> > The tps68470 supports i2c daisy chain, which need to be configured by
> > gpio-tps68470 driver. Add daisy chain information to platform data.
> >
> > Signed-off-by: Antti Laakso <antti.laakso@linux.intel.com>
> > ---
> >  drivers/platform/x86/intel/int3472/tps68470.c | 2 ++
> >  drivers/platform/x86/intel/int3472/tps68470.h | 1 +
> >  include/linux/platform_data/tps68470.h        | 4 ++++
> >  3 files changed, 7 insertions(+)
> >
> > diff --git a/drivers/platform/x86/intel/int3472/tps68470.c b/drivers/platform/x86/intel/int3472/tps68470.c
> > index a496075c0d2a..b02bc675cabe 100644
> > --- a/drivers/platform/x86/intel/int3472/tps68470.c
> > +++ b/drivers/platform/x86/intel/int3472/tps68470.c
> > @@ -197,6 +197,8 @@ static int skl_int3472_tps68470_probe(struct i2c_client *client)
> >  		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";
> > +		cells[2].platform_data = (void *)board_data->tps68470_gpio_pdata;
> > +		cells[2].pdata_size = sizeof(*board_data->tps68470_gpio_pdata);
> >
> >  		for (i = 0; i < board_data->n_gpiod_lookups; i++)
> >  			gpiod_add_lookup_table(board_data->tps68470_gpio_lookup_tables[i]);
> > diff --git a/drivers/platform/x86/intel/int3472/tps68470.h b/drivers/platform/x86/intel/int3472/tps68470.h
> > index 35915e701593..c1c4290eb6d5 100644
> > --- a/drivers/platform/x86/intel/int3472/tps68470.h
> > +++ b/drivers/platform/x86/intel/int3472/tps68470.h
> > @@ -17,6 +17,7 @@ struct tps68470_regulator_platform_data;
> >  struct int3472_tps68470_board_data {
> >  	const char *dev_name;
> >  	const struct tps68470_regulator_platform_data *tps68470_regulator_pdata;
> > +	const struct tps68470_gpio_platform_data *tps68470_gpio_pdata;
> >  	unsigned int n_gpiod_lookups;
> >  	struct gpiod_lookup_table *tps68470_gpio_lookup_tables[];
> >  };
> > diff --git a/include/linux/platform_data/tps68470.h b/include/linux/platform_data/tps68470.h
> > index e605a2cab07f..7330dab7a711 100644
> > --- a/include/linux/platform_data/tps68470.h
> > +++ b/include/linux/platform_data/tps68470.h
> > @@ -27,6 +27,10 @@ struct tps68470_regulator_platform_data {
> >  	const struct regulator_init_data *reg_init_data[TPS68470_NUM_REGULATORS];
> >  };
> >
> > +struct tps68470_gpio_platform_data {
> > +	const bool daisy_chain_enable;
> > +};
> > +
> >  struct tps68470_clk_consumer {
> >  	const char *consumer_dev_name;
> >  	const char *consumer_con_id;
> > --
> > 2.53.0
> >
> >
> 
> I know this is the pattern used in this driver but why not start converting
> it gradually to more modern APIs?
> 
> You could do:
> 
> static const struct property_entry tps68470_gpio_props[] = {
> 	PROPERTY_ENTRY_BOOL("daisy-chain-enable"),
> 	{ }
> };
> 
> static const struct software_node tps68470_gpio_swnode = {
> 	.properties = tps68470_gpio_props,
> };
> 
> static int skl_int3472_tps68470_probe(struct i2c_client *client)
> {
> 	...
> 	cells[2].swnode = &tps68470_gpio_swnode;
> 	...
> }
> 
> And avoid both modifying the public header as well as using ugly platform
> data. In the GPIO driver you'd just need:
> 
> 	if (device_property_present(&pdev->dev, "daisy-chain-enable"))
> 
> Bartosz

Thanks for suggestion, I'll use software node in v2.

Antti