Re: [RFT PATCH v2] ARM: omap1: enable real software node lookup of GPIOs on Nokia 770

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Wed, Feb 11, 2026 at 01:40:44PM -0800, Dmitry Torokhov wrote:
> On Wed, Feb 11, 2026 at 05:31:52PM +0100, Arnd Bergmann wrote:
> > On Wed, Feb 11, 2026, at 14:13, Bartosz Golaszewski wrote:
> > > Currently the board file for Nokia 770 creates dummy software nodes not
> > > attached in any way to the actual GPIO controller devices and uses the
> > > fact that GPIOLIB matching swnode's name to the GPIO chip's label during
> > > software node lookup. This behavior is wrong and we want to remove it.
> > > To that end, we need to first convert all existing users to creating
> > > actual fwnode links.
> > >
> > > Create real software nodes for GPIO controllers on OMAP16xx and
> > > reference them from the software nodes in the nokia board file.
> > >
> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> > > ---
> > 
> > I don't see mistakes here, and I don't want to throw a wrench in
> > this patch, but I wonder if there is a way to take this one step further:
> > 
> > > @@ -244,6 +263,14 @@ static int __init omap16xx_gpio_init(void)
> > >  		iounmap(base);
> > > 
> > >  		platform_device_register(omap16xx_gpio_dev[i]);
> > > +
> > > +		ret = device_add_software_node(&omap16xx_gpio_dev[i]->dev,
> > > +					       omap16xx_gpio_swnodes[i]);
> > > +
> > > +		if (ret) {
> > > +			dev_err(&pdev->dev, "Failed to add software node.\n");
> > > +			return ret;
> > > +		}
> > 
> > I was planning to go through the remaining 'static struct platform_device'
> > definitions in arch/arm/ after the planned board file removal and
> > try to convert these to 'platform_device_info' or similar, using
> > platform_device_register_full(). Since that function already contains
> > code to dynamically allocate the software_node, I had hoped that
> > a lot of this would just go away.
> > 
> > However, I see that your patch creates pointers to those software_node
> > instances, so  think that would become a bit harder, but I have not
> > actually tried it.
> > 
> > Do you know if there is a good way to do this without using static
> > platform devices?
> 
> I wonder if something like below will help reducing boilerplate...

And then we can change the gpio16xx like this (on top of Bartosz
changes):

diff --git a/arch/arm/mach-omap1/gpio16xx.c b/arch/arm/mach-omap1/gpio16xx.c
index 177621cb4784..a42f13890b94 100644
--- a/arch/arm/mach-omap1/gpio16xx.c
+++ b/arch/arm/mach-omap1/gpio16xx.c
@@ -26,7 +26,7 @@
 #define SYSCONFIG_WORD			0x14
 
 /* mpu gpio */
-static struct resource omap16xx_mpu_gpio_resources[] = {
+static const struct resource omap16xx_mpu_gpio_resources[] __initconst = {
 	{
 		.start	= OMAP1_MPUIO_VBASE,
 		.end	= OMAP1_MPUIO_VBASE + SZ_2K - 1,
@@ -38,7 +38,7 @@ static struct resource omap16xx_mpu_gpio_resources[] = {
 	},
 };
 
-static struct omap_gpio_reg_offs omap16xx_mpuio_regs = {
+static const struct omap_gpio_reg_offs omap16xx_mpuio_regs = {
 	.revision       = USHRT_MAX,
 	.direction	= OMAP_MPUIO_IO_CNTL,
 	.datain		= OMAP_MPUIO_INPUT_LATCH,
@@ -49,27 +49,27 @@ static struct omap_gpio_reg_offs omap16xx_mpuio_regs = {
 	.irqctrl	= OMAP_MPUIO_GPIO_INT_EDGE,
 };
 
-static struct omap_gpio_platform_data omap16xx_mpu_gpio_config = {
+static const struct omap_gpio_platform_data omap16xx_mpu_gpio_config __initconst = {
 	.is_mpuio		= true,
 	.bank_width		= 16,
 	.bank_stride		= 1,
 	.regs                   = &omap16xx_mpuio_regs,
 };
 
-const struct software_node omap16xx_mpu_gpio_swnode = { };
+static const struct software_node omap16xx_mpu_gpio_swnode;
 
-static struct platform_device omap16xx_mpu_gpio = {
-	.name           = "omap_gpio",
-	.id             = 0,
-	.dev            = {
-		.platform_data = &omap16xx_mpu_gpio_config,
-	},
-	.num_resources = ARRAY_SIZE(omap16xx_mpu_gpio_resources),
-	.resource = omap16xx_mpu_gpio_resources,
+static const struct platform_device_info omap16xx_mpu_gpio_info __initconst = {
+	.name = "omap_gpio",
+	.id = 0,
+	.data = &omap16xx_mpu_gpio_config,
+	.size_data = sizeof(omap16xx_mpu_gpio_config),
+	.res = omap16xx_mpu_gpio_resources,
+	.num_res = ARRAY_SIZE(omap16xx_mpu_gpio_resources),
+	.swnode = &omap16xx_mpu_gpio_swnode,
 };
 
 /* gpio1 */
-static struct resource omap16xx_gpio1_resources[] = {
+static const struct resource omap16xx_gpio1_resources[] __initconst = {
 	{
 		.start	= OMAP1610_GPIO1_BASE,
 		.end	= OMAP1610_GPIO1_BASE + SZ_2K - 1,
@@ -81,7 +81,7 @@ static struct resource omap16xx_gpio1_resources[] = {
 	},
 };
 
-static struct omap_gpio_reg_offs omap16xx_gpio_regs = {
+static const struct omap_gpio_reg_offs omap16xx_gpio_regs = {
 	.revision       = OMAP1610_GPIO_REVISION,
 	.direction	= OMAP1610_GPIO_DIRECTION,
 	.set_dataout	= OMAP1610_GPIO_SET_DATAOUT,
@@ -97,25 +97,25 @@ static struct omap_gpio_reg_offs omap16xx_gpio_regs = {
 	.edgectrl2	= OMAP1610_GPIO_EDGE_CTRL2,
 };
 
-static struct omap_gpio_platform_data omap16xx_gpio1_config = {
+static const struct omap_gpio_platform_data omap16xx_gpio1_config __initconst = {
 	.bank_width		= 16,
 	.regs                   = &omap16xx_gpio_regs,
 };
 
-const struct software_node omap16xx_gpio1_swnode = { };
+static const struct software_node omap16xx_gpio1_swnode;
 
-static struct platform_device omap16xx_gpio1 = {
-	.name           = "omap_gpio",
-	.id             = 1,
-	.dev            = {
-		.platform_data = &omap16xx_gpio1_config,
-	},
-	.num_resources = ARRAY_SIZE(omap16xx_gpio1_resources),
-	.resource = omap16xx_gpio1_resources,
+static const struct platform_device_info omap16xx_gpio1_info __initconst = {
+	.name = "omap_gpio",
+	.id = 1,
+	.data = &omap16xx_gpio1_config,
+	.size_data = sizeof(omap16xx_gpio1_config),
+	.res = omap16xx_gpio1_resources,
+	.num_res = ARRAY_SIZE(omap16xx_gpio1_resources),
+	.swnode = &omap16xx_gpio1_swnode,
 };
 
 /* gpio2 */
-static struct resource omap16xx_gpio2_resources[] = {
+static const struct resource omap16xx_gpio2_resources[] __initconst = {
 	{
 		.start	= OMAP1610_GPIO2_BASE,
 		.end	= OMAP1610_GPIO2_BASE + SZ_2K - 1,
@@ -127,25 +127,25 @@ static struct resource omap16xx_gpio2_resources[] = {
 	},
 };
 
-static const struct software_node omap16xx_gpio2_swnode = { };
+static const struct software_node omap16xx_gpio2_swnode;
 
-static struct omap_gpio_platform_data omap16xx_gpio2_config = {
+static const struct omap_gpio_platform_data omap16xx_gpio2_config __initconst = {
 	.bank_width		= 16,
 	.regs                   = &omap16xx_gpio_regs,
 };
 
-static struct platform_device omap16xx_gpio2 = {
-	.name           = "omap_gpio",
-	.id             = 2,
-	.dev            = {
-		.platform_data = &omap16xx_gpio2_config,
-	},
-	.num_resources = ARRAY_SIZE(omap16xx_gpio2_resources),
-	.resource = omap16xx_gpio2_resources,
+static const struct platform_device_info omap16xx_gpio2_info __initconst = {
+	.name = "omap_gpio",
+	.id = 2,
+	.data = &omap16xx_gpio2_config,
+	.size_data = sizeof(omap16xx_gpio2_config),
+	.res = omap16xx_gpio2_resources,
+	.num_res = ARRAY_SIZE(omap16xx_gpio2_resources),
+	.swnode = &omap16xx_gpio2_swnode,
 };
 
 /* gpio3 */
-static struct resource omap16xx_gpio3_resources[] = {
+static const struct resource omap16xx_gpio3_resources[] __initconst = {
 	{
 		.start	= OMAP1610_GPIO3_BASE,
 		.end	= OMAP1610_GPIO3_BASE + SZ_2K - 1,
@@ -157,25 +157,25 @@ static struct resource omap16xx_gpio3_resources[] = {
 	},
 };
 
-static struct omap_gpio_platform_data omap16xx_gpio3_config = {
+static const struct omap_gpio_platform_data omap16xx_gpio3_config __initconst = {
 	.bank_width		= 16,
 	.regs                   = &omap16xx_gpio_regs,
 };
 
-static const struct software_node omap16xx_gpio3_swnode = { };
+static const struct software_node omap16xx_gpio3_swnode;
 
-static struct platform_device omap16xx_gpio3 = {
-	.name           = "omap_gpio",
-	.id             = 3,
-	.dev            = {
-		.platform_data = &omap16xx_gpio3_config,
-	},
-	.num_resources = ARRAY_SIZE(omap16xx_gpio3_resources),
-	.resource = omap16xx_gpio3_resources,
+static const struct platform_device_info omap16xx_gpio3_info __initconst = {
+	.name = "omap_gpio",
+	.id = 3,
+	.data = &omap16xx_gpio3_config,
+	.size_data = sizeof(omap16xx_gpio3_config),
+	.res = omap16xx_gpio3_resources,
+	.num_res = ARRAY_SIZE(omap16xx_gpio3_resources),
+	.swnode = &omap16xx_gpio3_swnode,
 };
 
 /* gpio4 */
-static struct resource omap16xx_gpio4_resources[] = {
+static const struct resource omap16xx_gpio4_resources[] __initconst = {
 	{
 		.start	= OMAP1610_GPIO4_BASE,
 		.end	= OMAP1610_GPIO4_BASE + SZ_2K - 1,
@@ -187,37 +187,29 @@ static struct resource omap16xx_gpio4_resources[] = {
 	},
 };
 
-static struct omap_gpio_platform_data omap16xx_gpio4_config = {
+static const struct omap_gpio_platform_data omap16xx_gpio4_config __initconst = {
 	.bank_width		= 16,
 	.regs                   = &omap16xx_gpio_regs,
 };
 
-static const struct software_node omap16xx_gpio4_swnode = { };
-
-static struct platform_device omap16xx_gpio4 = {
-	.name           = "omap_gpio",
-	.id             = 4,
-	.dev            = {
-		.platform_data = &omap16xx_gpio4_config,
-	},
-	.num_resources = ARRAY_SIZE(omap16xx_gpio4_resources),
-	.resource = omap16xx_gpio4_resources,
-};
+static const struct software_node omap16xx_gpio4_swnode;
 
-static struct platform_device *omap16xx_gpio_dev[] __initdata = {
-	&omap16xx_mpu_gpio,
-	&omap16xx_gpio1,
-	&omap16xx_gpio2,
-	&omap16xx_gpio3,
-	&omap16xx_gpio4,
+static const struct platform_device_info omap16xx_gpio4_info __initconst = {
+	.name = "omap_gpio",
+	.id = 4,
+	.data = &omap16xx_gpio4_config,
+	.size_data = sizeof(omap16xx_gpio4_config),
+	.res = omap16xx_gpio4_resources,
+	.num_res = ARRAY_SIZE(omap16xx_gpio4_resources),
+	.swnode = &omap16xx_gpio4_swnode,
 };
 
-static const struct software_node *omap16xx_gpio_swnodes[] __initconst = {
-	&omap16xx_mpu_gpio_swnode,
-	&omap16xx_gpio1_swnode,
-	&omap16xx_gpio2_swnode,
-	&omap16xx_gpio3_swnode,
-	&omap16xx_gpio4_swnode,
+static const struct platform_device_info * const omap16xx_gpio_dev[] __initconst = {
+	&omap16xx_mpu_gpio_info,
+	&omap16xx_gpio1_info,
+	&omap16xx_gpio2_info,
+	&omap16xx_gpio3_info,
+	&omap16xx_gpio4_info,
 };
 
 /*
@@ -227,11 +219,7 @@ static const struct software_node *omap16xx_gpio_swnodes[] __initconst = {
  */
 static int __init omap16xx_gpio_init(void)
 {
-	int i, ret;
-	void __iomem *base;
-	struct resource *res;
-	struct platform_device *pdev;
-	struct omap_gpio_platform_data *pdata;
+	int i;
 
 	if (!cpu_is_omap16xx())
 		return -EINVAL;
@@ -240,36 +228,41 @@ static int __init omap16xx_gpio_init(void)
 	 * Enable system clock for GPIO module.
 	 * The CAM_CLK_CTRL *is* really the right place.
 	 */
-	omap_writel(omap_readl(ULPD_CAM_CLK_CTRL) | 0x04,
-					ULPD_CAM_CLK_CTRL);
+	omap_writel(omap_readl(ULPD_CAM_CLK_CTRL) | 0x04, ULPD_CAM_CLK_CTRL);
 
 	for (i = 0; i < ARRAY_SIZE(omap16xx_gpio_dev); i++) {
-		pdev = omap16xx_gpio_dev[i];
-		pdata = pdev->dev.platform_data;
+		const struct platform_device_info *pdev_info = omap16xx_gpio_dev[i];
+		const struct resource *res = NULL;
+		struct platform_device *pdev;
+		void __iomem *base;
+		int j;
+
+		for (j = 0; j < pdev_info->num_res; j++) {
+			if (resource_type(&pdev_info->res[j]) == IORESOURCE_MEM) {
+				res = &pdev_info->res[j];
+				break;
+			}
+		}
 
-		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 		if (unlikely(!res)) {
-			dev_err(&pdev->dev, "Invalid mem resource.\n");
+			pr_err("gpio%d: Invalid mem resource.\n", i);
 			return -ENODEV;
 		}
 
 		base = ioremap(res->start, resource_size(res));
 		if (unlikely(!base)) {
-			dev_err(&pdev->dev, "ioremap failed.\n");
+			pr_err("gpio%d: ioremap failed.\n", i);
 			return -ENOMEM;
 		}
 
 		__raw_writel(SYSCONFIG_WORD, base + OMAP1610_GPIO_SYSCONFIG);
 		iounmap(base);
 
-		platform_device_register(omap16xx_gpio_dev[i]);
-
-		ret = device_add_software_node(&omap16xx_gpio_dev[i]->dev,
-					       omap16xx_gpio_swnodes[i]);
-
-		if (ret) {
-			dev_err(&pdev->dev, "Failed to add software node.\n");
-			return ret;
+		pdev = platform_device_register_full(pdev_info);
+		if (IS_ERR(pdev)) {
+			pr_err("gpio%d: Failed to register device: %ld\n",
+			       i, PTR_ERR(pdev));
+			return PTR_ERR(pdev);
 		}
 	}
 
> 
> Thanks.
> 

-- 
Dmitry