[PATCH v2 1/3] mfd: timberdale: set up a software node for the GPIO cell

Bartosz Golaszewski posted 3 patches 3 weeks ago
There is a newer version of this series
[PATCH v2 1/3] mfd: timberdale: set up a software node for the GPIO cell
Posted by Bartosz Golaszewski 3 weeks ago
Create a software node with device properties for the GPIO cell and
attach it to all the GPIO cells.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/mfd/timberdale.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/mfd/timberdale.c b/drivers/mfd/timberdale.c
index a4d9c070d481a182890a58e4b8c850c4c29f7f17..8d7dfeaa76258d02e50528e39c4f7125d9201388 100644
--- a/drivers/mfd/timberdale.c
+++ b/drivers/mfd/timberdale.c
@@ -181,6 +181,18 @@ static struct timbgpio_platform_data
 	.irq_base = 200,
 };
 
+static const struct property_entry timberdale_gpio_properties[] = {
+	PROPERTY_ENTRY_U32("ngpios", GPIO_NR_PINS),
+	PROPERTY_ENTRY_U32("gpio-base", 0),
+	PROPERTY_ENTRY_U32("irq-base", 200),
+	{ }
+};
+
+static const struct software_node timberdale_gpio_swnode = {
+	.name = "timb-gpio",
+	.properties = timberdale_gpio_properties,
+};
+
 static const struct resource timberdale_gpio_resources[] = {
 	{
 		.start	= GPIOOFFSET,
@@ -392,6 +404,7 @@ static const struct mfd_cell timberdale_cells_bar0_cfg0[] = {
 		.resources = timberdale_gpio_resources,
 		.platform_data = &timberdale_gpio_platform_data,
 		.pdata_size = sizeof(timberdale_gpio_platform_data),
+		.swnode = &timberdale_gpio_swnode,
 	},
 	{
 		.name = "timb-video",
@@ -454,6 +467,7 @@ static const struct mfd_cell timberdale_cells_bar0_cfg1[] = {
 		.resources = timberdale_gpio_resources,
 		.platform_data = &timberdale_gpio_platform_data,
 		.pdata_size = sizeof(timberdale_gpio_platform_data),
+		.swnode = &timberdale_gpio_swnode,
 	},
 	{
 		.name = "timb-mlogicore",
@@ -516,6 +530,7 @@ static const struct mfd_cell timberdale_cells_bar0_cfg2[] = {
 		.resources = timberdale_gpio_resources,
 		.platform_data = &timberdale_gpio_platform_data,
 		.pdata_size = sizeof(timberdale_gpio_platform_data),
+		.swnode = &timberdale_gpio_swnode,
 	},
 	{
 		.name = "timb-video",
@@ -566,6 +581,7 @@ static const struct mfd_cell timberdale_cells_bar0_cfg3[] = {
 		.resources = timberdale_gpio_resources,
 		.platform_data = &timberdale_gpio_platform_data,
 		.pdata_size = sizeof(timberdale_gpio_platform_data),
+		.swnode = &timberdale_gpio_swnode,
 	},
 	{
 		.name = "timb-video",

-- 
2.47.3
Re: [PATCH v2 1/3] mfd: timberdale: set up a software node for the GPIO cell
Posted by Lee Jones 1 week, 4 days ago
On Mon, 16 Mar 2026, Bartosz Golaszewski wrote:

> Create a software node with device properties for the GPIO cell and
> attach it to all the GPIO cells.

Could you elaborate a little on the motivation for this change, please?
I assume this is the first step in a larger plan to convert the GPIO
child driver to use device properties and remove the legacy 'platform_data'.
If so, it would be good to state that in the commit message or a cover
letter for the series.

> 
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
>  drivers/mfd/timberdale.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/mfd/timberdale.c b/drivers/mfd/timberdale.c
> index a4d9c070d481a182890a58e4b8c850c4c29f7f17..8d7dfeaa76258d02e50528e39c4f7125d9201388 100644
> --- a/drivers/mfd/timberdale.c
> +++ b/drivers/mfd/timberdale.c
> @@ -181,6 +181,18 @@ static struct timbgpio_platform_data
>  	.irq_base = 200,
>  };
>  
> +static const struct property_entry timberdale_gpio_properties[] = {
> +	PROPERTY_ENTRY_U32("ngpios", GPIO_NR_PINS),
> +	PROPERTY_ENTRY_U32("gpio-base", 0),

This appears to introduce a functional change. The existing 'platform_data'
sets 'gpio_base' to -1, which results in dynamic allocation. Hard-coding
a base of 0 here is very likely to cause conflicts.

Should this property be removed to retain the dynamic allocation behaviour?

> +	PROPERTY_ENTRY_U32("irq-base", 200),

This magic number should be defined.

> +	{ }
> +};
> +
> +static const struct software_node timberdale_gpio_swnode = {
> +	.name = "timb-gpio",
> +	.properties = timberdale_gpio_properties,
> +};
> +
>  static const struct resource timberdale_gpio_resources[] = {
>  	{
>  		.start	= GPIOOFFSET,
> @@ -392,6 +404,7 @@ static const struct mfd_cell timberdale_cells_bar0_cfg0[] = {
>  		.resources = timberdale_gpio_resources,
>  		.platform_data = &timberdale_gpio_platform_data,
>  		.pdata_size = sizeof(timberdale_gpio_platform_data),
> +		.swnode = &timberdale_gpio_swnode,
>  	},
>  	{
>  		.name = "timb-video",
> @@ -454,6 +467,7 @@ static const struct mfd_cell timberdale_cells_bar0_cfg1[] = {
>  		.resources = timberdale_gpio_resources,
>  		.platform_data = &timberdale_gpio_platform_data,
>  		.pdata_size = sizeof(timberdale_gpio_platform_data),
> +		.swnode = &timberdale_gpio_swnode,
>  	},
>  	{
>  		.name = "timb-mlogicore",
> @@ -516,6 +530,7 @@ static const struct mfd_cell timberdale_cells_bar0_cfg2[] = {
>  		.resources = timberdale_gpio_resources,
>  		.platform_data = &timberdale_gpio_platform_data,
>  		.pdata_size = sizeof(timberdale_gpio_platform_data),
> +		.swnode = &timberdale_gpio_swnode,
>  	},
>  	{
>  		.name = "timb-video",
> @@ -566,6 +581,7 @@ static const struct mfd_cell timberdale_cells_bar0_cfg3[] = {
>  		.resources = timberdale_gpio_resources,
>  		.platform_data = &timberdale_gpio_platform_data,
>  		.pdata_size = sizeof(timberdale_gpio_platform_data),
> +		.swnode = &timberdale_gpio_swnode,
>  	},
>  	{
>  		.name = "timb-video",
> 
> -- 
> 2.47.3
> 

-- 
Lee Jones [李琼斯]
Re: [PATCH v2 1/3] mfd: timberdale: set up a software node for the GPIO cell
Posted by Bartosz Golaszewski 1 week, 4 days ago
On Thu, Mar 26, 2026 at 11:52 AM Lee Jones <lee@kernel.org> wrote:
>
> On Mon, 16 Mar 2026, Bartosz Golaszewski wrote:
>
> > Create a software node with device properties for the GPIO cell and
> > attach it to all the GPIO cells.
>
> Could you elaborate a little on the motivation for this change, please?
> I assume this is the first step in a larger plan to convert the GPIO
> child driver to use device properties and remove the legacy 'platform_data'.
> If so, it would be good to state that in the commit message or a cover
> letter for the series.
>

Sure, I'll remember next time. It's not really a larger plan, I'm just
trying to clean up various GPIO-related headers specific to individual
drivers. Most platform data structures can easily be converted to
device properties.

> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> > ---
> >  drivers/mfd/timberdale.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/drivers/mfd/timberdale.c b/drivers/mfd/timberdale.c
> > index a4d9c070d481a182890a58e4b8c850c4c29f7f17..8d7dfeaa76258d02e50528e39c4f7125d9201388 100644
> > --- a/drivers/mfd/timberdale.c
> > +++ b/drivers/mfd/timberdale.c
> > @@ -181,6 +181,18 @@ static struct timbgpio_platform_data
> >       .irq_base = 200,
> >  };
> >
> > +static const struct property_entry timberdale_gpio_properties[] = {
> > +     PROPERTY_ENTRY_U32("ngpios", GPIO_NR_PINS),
> > +     PROPERTY_ENTRY_U32("gpio-base", 0),
>
> This appears to introduce a functional change. The existing 'platform_data'
> sets 'gpio_base' to -1, which results in dynamic allocation. Hard-coding
> a base of 0 here is very likely to cause conflicts.
>

Please look at the definition of struct timbgpio_platform_data patch
3/3 removes. It also sets base to 0 so I'm actually avoiding a
functional change here.


> Should this property be removed to retain the dynamic allocation behaviour?
>
> > +     PROPERTY_ENTRY_U32("irq-base", 200),
>
> This magic number should be defined.
>

It was not defined in platform data either but sure, I can fix it in v2.

Bart