[PATCH v3 0/2] rohm-bdi718x7/71828: Use software nodes for gpio-keys

[PATCH v3 0/2] rohm-bdi718x7/71828: Use software nodes for gpio-keys

[Dmitry Torokhov]

Now that gpio-keys can use platform resources to identify interrupts
assigned to buttons we can convert ROHM power buttons to use software
nodes and device properties for configuration, removing the need to use
platform data.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
Changes in v3:
- Stopped mixing code and variable declarations, use more temps
- Moved assignment to irq_domain closer to where is is being used
- Added missing SOB
- Link to v2: https://patch.msgid.link/20260322-rohm-software-nodes-v2-0-3c7d21336d37@gmail.com

v2:
- dropped patch to gpio-keys as it is in the mainline now
- reworked the both drivers to dynamically allocate per-device software
  nodes

v1:
https://lore.kernel.org/r/20250817224731.1911207-1-dmitry.torokhov@gmail.com/

---
Dmitry Torokhov (2):
      mfd: rohm-bd71828: Use software nodes for gpio-keys
      mfd: rohm-bd718x7: Use software nodes for gpio-keys

 drivers/mfd/rohm-bd71828.c | 120 +++++++++++++++++++++++++++++++++------------
 drivers/mfd/rohm-bd718x7.c | 118 +++++++++++++++++++++++++++++++-------------
 2 files changed, 173 insertions(+), 65 deletions(-)
---
base-commit: 7109a2155340cc7b21f27e832ece6df03592f2e8
change-id: 20260313-rohm-software-nodes-0b4a3d36128c

Thanks.

-- 
Dmitry

[PATCH v3 1/2] mfd: rohm-bd71828: Use software nodes for gpio-keys

[Dmitry Torokhov]

Refactor the rohm-bd71828 MFD driver to use software nodes for
instantiating the gpio-keys child device, replacing the old
platform_data mechanism.

The power key's properties are now defined using software nodes and
property entries. The IRQ is passed as a resource attached to the
platform device.

This will allow dropping support for using platform data for configuring
gpio-keys in the future.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/mfd/rohm-bd71828.c | 120 +++++++++++++++++++++++++++++++++------------
 1 file changed, 88 insertions(+), 32 deletions(-)

diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
index a79f354bf5cb..e78af75a2115 100644
--- a/drivers/mfd/rohm-bd71828.c
+++ b/drivers/mfd/rohm-bd71828.c
@@ -5,7 +5,8 @@
  * ROHM BD718[15/28/79] and BD72720 PMIC driver
  */
 
-#include <linux/gpio_keys.h>
+#include <linux/device/devres.h>
+#include <linux/gfp_types.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
 #include <linux/interrupt.h>
@@ -18,6 +19,7 @@
 #include <linux/mfd/rohm-generic.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/types.h>
 
@@ -37,19 +39,6 @@
 		},							   \
 	}
 
-static struct gpio_keys_button button = {
-	.code = KEY_POWER,
-	.gpio = -1,
-	.type = EV_KEY,
-	.wakeup = 1,
-};
-
-static const struct gpio_keys_platform_data bd71828_powerkey_data = {
-	.buttons = &button,
-	.nbuttons = 1,
-	.name = "bd71828-pwrkey",
-};
-
 static const struct resource bd71815_rtc_irqs[] = {
 	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC0, "bd70528-rtc-alm-0"),
 	DEFINE_RES_IRQ_NAMED(BD71815_INT_RTC1, "bd70528-rtc-alm-1"),
@@ -174,11 +163,8 @@ static struct mfd_cell bd71828_mfd_cells[] = {
 		.name = "bd71828-rtc",
 		.resources = bd71828_rtc_irqs,
 		.num_resources = ARRAY_SIZE(bd71828_rtc_irqs),
-	}, {
-		.name = "gpio-keys",
-		.platform_data = &bd71828_powerkey_data,
-		.pdata_size = sizeof(bd71828_powerkey_data),
 	},
+	/* Power button is registered separately */
 };
 
 static const struct resource bd72720_power_irqs[] = {
@@ -242,11 +228,8 @@ static const struct mfd_cell bd72720_mfd_cells[] = {
 		.name = "bd72720-rtc",
 		.resources = bd72720_rtc_irqs,
 		.num_resources = ARRAY_SIZE(bd72720_rtc_irqs),
-	}, {
-		.name = "gpio-keys",
-		.platform_data = &bd71828_powerkey_data,
-		.pdata_size = sizeof(bd71828_powerkey_data),
 	},
+	/* Power button is registered separately */
 };
 
 static const struct regmap_range bd71815_volatile_ranges[] = {
@@ -877,6 +860,78 @@ static int set_clk_mode(struct device *dev, struct regmap *regmap,
 				  OUT32K_MODE_CMOS);
 }
 
+static void bd71828_i2c_unregister_swnodes(void *data)
+{
+	const struct software_node *nodes = data;
+	const struct software_node *node_group[] = {
+		&nodes[0], &nodes[1], NULL
+	};
+
+	software_node_unregister_node_group(node_group);
+}
+
+static int bd71828_i2c_register_pwrbutton(struct device *dev, int button_irq,
+					  struct irq_domain *irq_domain)
+{
+	static const struct property_entry bd71828_powerkey_parent_props[] = {
+		PROPERTY_ENTRY_STRING("label", "bd71828-pwrkey"),
+		{ }
+	};
+	static const struct property_entry bd71828_powerkey_props[] = {
+		PROPERTY_ENTRY_U32("linux,code", KEY_POWER),
+		PROPERTY_ENTRY_BOOL("wakeup-source"),
+		{ }
+	};
+	const struct resource res[] = {
+		DEFINE_RES_IRQ_NAMED(button_irq, "bd71828-pwrkey"),
+	};
+	struct mfd_cell gpio_keys_cell = {
+		.name = "gpio-keys",
+		.resources = res,
+		.num_resources = ARRAY_SIZE(res),
+	};
+	const struct software_node * const *node_group;
+	struct software_node *nodes;
+	int error;
+
+	nodes = devm_kcalloc(dev, 2, sizeof(*nodes), GFP_KERNEL);
+	if (!nodes)
+		return -ENOMEM;
+
+	/* Node corresponding to gpio-keys device itself */
+	nodes[0].name = devm_kasprintf(dev, GFP_KERNEL, "%s-power-key", dev_name(dev));
+	if (!nodes[0].name)
+		return -ENOMEM;
+
+	nodes[0].properties = bd71828_powerkey_parent_props;
+
+	/* Node representing power button within gpio-keys device */
+	nodes[1].parent = &nodes[0];
+	nodes[1].properties = bd71828_powerkey_props;
+
+	node_group = (const struct software_node *[]){
+		&nodes[0],
+		&nodes[1],
+		NULL
+	};
+
+	error = software_node_register_node_group(node_group);
+	if (error)
+		return error;
+
+	error = devm_add_action_or_reset(dev, bd71828_i2c_unregister_swnodes, nodes);
+	if (error)
+		return error;
+
+	gpio_keys_cell.swnode = &nodes[0];
+	error = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, &gpio_keys_cell, 1,
+				     NULL, 0, irq_domain);
+	if (error)
+		return dev_err_probe(dev, error, "Failed to create power button subdevice");
+
+	return 0;
+}
+
 static struct i2c_client *bd71828_dev;
 static void bd71828_power_off(void)
 {
@@ -929,6 +984,7 @@ static struct regmap *bd72720_do_regmaps(struct i2c_client *i2c)
 static int bd71828_i2c_probe(struct i2c_client *i2c)
 {
 	struct regmap_irq_chip_data *irq_data;
+	struct irq_domain *irq_domain;
 	int ret;
 	struct regmap *regmap = NULL;
 	const struct regmap_config *regmap_config;
@@ -1022,23 +1078,23 @@ static int bd71828_i2c_probe(struct i2c_client *i2c)
 					"Failed to enable main level IRQs\n");
 		}
 	}
-	if (button_irq) {
-		ret = regmap_irq_get_virq(irq_data, button_irq);
-		if (ret < 0)
-			return dev_err_probe(&i2c->dev, ret,
-					     "Failed to get the power-key IRQ\n");
-
-		button.irq = ret;
-	}
 
 	ret = set_clk_mode(&i2c->dev, regmap, clkmode_reg);
 	if (ret)
 		return ret;
 
+	irq_domain = regmap_irq_get_domain(irq_data);
+
 	ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO, mfd, cells,
-				   NULL, 0, regmap_irq_get_domain(irq_data));
+				   NULL, 0, irq_domain);
 	if (ret)
-		return	dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n");
+		return dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n");
+
+	if (button_irq) {
+		ret = bd71828_i2c_register_pwrbutton(&i2c->dev, button_irq, irq_domain);
+		if (ret)
+			return ret;
+	}
 
 	if (of_device_is_system_power_controller(i2c->dev.of_node) &&
 	    chip_type == ROHM_CHIP_TYPE_BD71828) {

-- 
2.53.0.1018.g2bb0e51243-goog

Re: [PATCH v3 1/2] mfd: rohm-bd71828: Use software nodes for gpio-keys

[Matti Vaittinen]

On 25/03/2026 02:54, Dmitry Torokhov wrote:
> Refactor the rohm-bd71828 MFD driver to use software nodes for
> instantiating the gpio-keys child device, replacing the old
> platform_data mechanism.
> 
> The power key's properties are now defined using software nodes and
> property entries. The IRQ is passed as a resource attached to the
> platform device.
> 
> This will allow dropping support for using platform data for configuring
> gpio-keys in the future.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>   drivers/mfd/rohm-bd71828.c | 120 +++++++++++++++++++++++++++++++++------------
>   1 file changed, 88 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
> index a79f354bf5cb..e78af75a2115 100644
> --- a/drivers/mfd/rohm-bd71828.c
> +++ b/drivers/mfd/rohm-bd71828.c

// snip

> +static int bd71828_i2c_register_pwrbutton(struct device *dev, int button_irq,
> +					  struct irq_domain *irq_domain)
> +{
> +	static const struct property_entry bd71828_powerkey_parent_props[] = {
> +		PROPERTY_ENTRY_STRING("label", "bd71828-pwrkey"),
> +		{ }
> +	};
> +	static const struct property_entry bd71828_powerkey_props[] = {
> +		PROPERTY_ENTRY_U32("linux,code", KEY_POWER),
> +		PROPERTY_ENTRY_BOOL("wakeup-source"),
> +		{ }
> +	};
> +	const struct resource res[] = {
> +		DEFINE_RES_IRQ_NAMED(button_irq, "bd71828-pwrkey"),
> +	};
> +	struct mfd_cell gpio_keys_cell = {
> +		.name = "gpio-keys",
> +		.resources = res,
> +		.num_resources = ARRAY_SIZE(res),
> +	};
> +	const struct software_node * const *node_group;
> +	struct software_node *nodes;
> +	int error;
> +
> +	nodes = devm_kcalloc(dev, 2, sizeof(*nodes), GFP_KERNEL);
> +	if (!nodes)
> +		return -ENOMEM;
> +
> +	/* Node corresponding to gpio-keys device itself */
> +	nodes[0].name = devm_kasprintf(dev, GFP_KERNEL, "%s-power-key", dev_name(dev));
> +	if (!nodes[0].name)
> +		return -ENOMEM;
> +
> +	nodes[0].properties = bd71828_powerkey_parent_props;
> +
> +	/* Node representing power button within gpio-keys device */
> +	nodes[1].parent = &nodes[0];
> +	nodes[1].properties = bd71828_powerkey_props;
> +
> +	node_group = (const struct software_node *[]){
> +		&nodes[0],
> +		&nodes[1],
> +		NULL
> +	};

Here, same as with the 2/2. This construct seems a tad strange to me, 
but I suppose I can live with it if Lee thinks this is the way to go.

Yours,
-- Matti

-- 
---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

[PATCH v3 2/2] mfd: rohm-bd718x7: Use software nodes for gpio-keys

[Dmitry Torokhov]

Refactor the rohm-bd7182x7 MFD driver to use software nodes for
instantiating the gpio-keys child device, replacing the old
platform_data mechanism.

The power key's properties are now defined using software nodes and
property entries. The IRQ is passed as a resource attached to the
platform device.

This will allow dropping support for using platform data for configuring
gpio-keys in the future.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/mfd/rohm-bd718x7.c | 118 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 85 insertions(+), 33 deletions(-)

diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c
index ff714fd4f54d..39961177a17f 100644
--- a/drivers/mfd/rohm-bd718x7.c
+++ b/drivers/mfd/rohm-bd718x7.c
@@ -7,7 +7,8 @@
 // Datasheet for BD71837MWV available from
 // https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e
 
-#include <linux/gpio_keys.h>
+#include <linux/device/devres.h>
+#include <linux/gfp_types.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
 #include <linux/interrupt.h>
@@ -15,37 +16,16 @@
 #include <linux/mfd/core.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/types.h>
 
-static struct gpio_keys_button button = {
-	.code = KEY_POWER,
-	.gpio = -1,
-	.type = EV_KEY,
-};
-
-static struct gpio_keys_platform_data bd718xx_powerkey_data = {
-	.buttons = &button,
-	.nbuttons = 1,
-	.name = "bd718xx-pwrkey",
-};
-
 static struct mfd_cell bd71837_mfd_cells[] = {
-	{
-		.name = "gpio-keys",
-		.platform_data = &bd718xx_powerkey_data,
-		.pdata_size = sizeof(bd718xx_powerkey_data),
-	},
 	{ .name = "bd71837-clk", },
 	{ .name = "bd71837-pmic", },
 };
 
 static struct mfd_cell bd71847_mfd_cells[] = {
-	{
-		.name = "gpio-keys",
-		.platform_data = &bd718xx_powerkey_data,
-		.pdata_size = sizeof(bd718xx_powerkey_data),
-	},
 	{ .name = "bd71847-clk", },
 	{ .name = "bd71847-pmic", },
 };
@@ -125,10 +105,84 @@ static int bd718xx_init_press_duration(struct regmap *regmap,
 	return 0;
 }
 
+static void bd718xx_i2c_unregister_swnodes(void *data)
+{
+	const struct software_node *nodes = data;
+	const struct software_node *node_group[] = {
+		&nodes[0], &nodes[1], NULL
+	};
+
+	software_node_unregister_node_group(node_group);
+}
+
+static int bd718xx_i2c_register_pwrbutton(struct device *dev,
+					  struct irq_domain *irq_domain)
+{
+	static const struct property_entry bd718xx_powerkey_parent_props[] = {
+		PROPERTY_ENTRY_STRING("label", "bd718xx-pwrkey"),
+		{ }
+	};
+	static const struct property_entry bd718xx_powerkey_props[] = {
+		PROPERTY_ENTRY_U32("linux,code", KEY_POWER),
+		{ }
+	};
+	const struct resource res[] = {
+		DEFINE_RES_IRQ_NAMED(BD718XX_INT_PWRBTN_S, "bd718xx-pwrkey"),
+	};
+	struct mfd_cell gpio_keys_cell = {
+		.name = "gpio-keys",
+		.resources = res,
+		.num_resources = ARRAY_SIZE(res),
+	};
+	const struct software_node * const *node_group;
+	struct software_node *nodes;
+	int error;
+
+	nodes = devm_kcalloc(dev, 2, sizeof(*nodes), GFP_KERNEL);
+	if (!nodes)
+		return -ENOMEM;
+
+	/* Node corresponding to gpio-keys device itself */
+	nodes[0].name = devm_kasprintf(dev, GFP_KERNEL, "%s-power-key", dev_name(dev));
+	if (!nodes[0].name)
+		return -ENOMEM;
+
+	nodes[0].properties = bd718xx_powerkey_parent_props;
+
+	/* Node representing power button within gpio-keys device */
+	nodes[1].parent = &nodes[0];
+	nodes[1].properties = bd718xx_powerkey_props;
+
+	node_group = (const struct software_node *[]){
+		&nodes[0],
+		&nodes[1],
+		NULL
+	};
+
+	error = software_node_register_node_group(node_group);
+	if (error)
+		return error;
+
+	error = devm_add_action_or_reset(dev, bd718xx_i2c_unregister_swnodes,
+					 nodes);
+	if (error)
+		return error;
+
+	gpio_keys_cell.swnode = &nodes[0];
+	error = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
+				     &gpio_keys_cell, 1, NULL, 0, irq_domain);
+	if (error)
+		return dev_err_probe(dev, error,
+				     "Failed to create power button subdevice");
+
+	return 0;
+}
+
 static int bd718xx_i2c_probe(struct i2c_client *i2c)
 {
 	struct regmap *regmap;
 	struct regmap_irq_chip_data *irq_data;
+	struct irq_domain *irq_domain;
 	int ret;
 	unsigned int chip_type;
 	struct mfd_cell *mfd;
@@ -169,20 +223,18 @@ static int bd718xx_i2c_probe(struct i2c_client *i2c)
 	if (ret)
 		return ret;
 
-	ret = regmap_irq_get_virq(irq_data, BD718XX_INT_PWRBTN_S);
-
-	if (ret < 0)
-		return dev_err_probe(&i2c->dev, ret, "Failed to get the IRQ\n");
-
-	button.irq = ret;
+	irq_domain = regmap_irq_get_domain(irq_data);
 
 	ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO,
-				   mfd, cells, NULL, 0,
-				   regmap_irq_get_domain(irq_data));
+				   mfd, cells, NULL, 0, irq_domain);
 	if (ret)
-		dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n");
+		return dev_err_probe(&i2c->dev, ret, "Failed to create subdevices\n");
 
-	return ret;
+	ret = bd718xx_i2c_register_pwrbutton(&i2c->dev, irq_domain);
+	if (ret)
+		return ret;
+
+	return 0;
 }
 
 static const struct of_device_id bd718xx_of_match[] = {

-- 
2.53.0.1018.g2bb0e51243-goog

Re: [PATCH v3 2/2] mfd: rohm-bd718x7: Use software nodes for gpio-keys

[Matti Vaittinen]

On 25/03/2026 02:54, Dmitry Torokhov wrote:
> Refactor the rohm-bd7182x7 MFD driver to use software nodes for
> instantiating the gpio-keys child device, replacing the old
> platform_data mechanism.
> 
> The power key's properties are now defined using software nodes and
> property entries. The IRQ is passed as a resource attached to the
> platform device.
> 
> This will allow dropping support for using platform data for configuring
> gpio-keys in the future.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>   drivers/mfd/rohm-bd718x7.c | 118 ++++++++++++++++++++++++++++++++-------------
>   1 file changed, 85 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c
> index ff714fd4f54d..39961177a17f 100644
> --- a/drivers/mfd/rohm-bd718x7.c
> +++ b/drivers/mfd/rohm-bd718x7.c
> @@ -7,7 +7,8 @@
>   // Datasheet for BD71837MWV available from
>   // https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e
>   
> -#include <linux/gpio_keys.h>
> +#include <linux/device/devres.h>
> +#include <linux/gfp_types.h>
>   #include <linux/i2c.h>
>   #include <linux/input.h>
>   #include <linux/interrupt.h>
> @@ -15,37 +16,16 @@
>   #include <linux/mfd/core.h>
>   #include <linux/module.h>
>   #include <linux/of.h>
> +#include <linux/property.h>
>   #include <linux/regmap.h>
>   #include <linux/types.h>
>   
> -static struct gpio_keys_button button = {
> -	.code = KEY_POWER,
> -	.gpio = -1,
> -	.type = EV_KEY,
> -};
> -
> -static struct gpio_keys_platform_data bd718xx_powerkey_data = {
> -	.buttons = &button,
> -	.nbuttons = 1,
> -	.name = "bd718xx-pwrkey",
> -};
> -
>   static struct mfd_cell bd71837_mfd_cells[] = {
> -	{
> -		.name = "gpio-keys",
> -		.platform_data = &bd718xx_powerkey_data,
> -		.pdata_size = sizeof(bd718xx_powerkey_data),
> -	},
>   	{ .name = "bd71837-clk", },
>   	{ .name = "bd71837-pmic", },
>   };
>   
>   static struct mfd_cell bd71847_mfd_cells[] = {
> -	{
> -		.name = "gpio-keys",
> -		.platform_data = &bd718xx_powerkey_data,
> -		.pdata_size = sizeof(bd718xx_powerkey_data),
> -	},
>   	{ .name = "bd71847-clk", },
>   	{ .name = "bd71847-pmic", },
>   };
> @@ -125,10 +105,84 @@ static int bd718xx_init_press_duration(struct regmap *regmap,
>   	return 0;
>   }
>   
> +static void bd718xx_i2c_unregister_swnodes(void *data)
> +{
> +	const struct software_node *nodes = data;
> +	const struct software_node *node_group[] = {
> +		&nodes[0], &nodes[1], NULL
> +	};
> +
> +	software_node_unregister_node_group(node_group);
> +}
> +
> +static int bd718xx_i2c_register_pwrbutton(struct device *dev,
> +					  struct irq_domain *irq_domain)
> +{
> +	static const struct property_entry bd718xx_powerkey_parent_props[] = {
> +		PROPERTY_ENTRY_STRING("label", "bd718xx-pwrkey"),
> +		{ }
> +	};
> +	static const struct property_entry bd718xx_powerkey_props[] = {
> +		PROPERTY_ENTRY_U32("linux,code", KEY_POWER),
> +		{ }
> +	};
> +	const struct resource res[] = {
> +		DEFINE_RES_IRQ_NAMED(BD718XX_INT_PWRBTN_S, "bd718xx-pwrkey"),
> +	};
> +	struct mfd_cell gpio_keys_cell = {
> +		.name = "gpio-keys",
> +		.resources = res,
> +		.num_resources = ARRAY_SIZE(res),
> +	};
> +	const struct software_node * const *node_group;
> +	struct software_node *nodes;
> +	int error;
> +
> +	nodes = devm_kcalloc(dev, 2, sizeof(*nodes), GFP_KERNEL);
> +	if (!nodes)
> +		return -ENOMEM;
> +
> +	/* Node corresponding to gpio-keys device itself */
> +	nodes[0].name = devm_kasprintf(dev, GFP_KERNEL, "%s-power-key", dev_name(dev));
> +	if (!nodes[0].name)
> +		return -ENOMEM;
> +
> +	nodes[0].properties = bd718xx_powerkey_parent_props;
> +
> +	/* Node representing power button within gpio-keys device */
> +	nodes[1].parent = &nodes[0];
> +	nodes[1].properties = bd718xx_powerkey_props;
> +
> +	node_group = (const struct software_node *[]){
> +		&nodes[0],
> +		&nodes[1],
> +		NULL
> +	};

Hmm. I suppose I was not explaining myself well. When I asked for a 
temporary variable, I was hoping to get rid of this syntax. Something like:
	const struct software_node *node_group[3];

         node_group[0] = &nodes[0];
         node_group[1] = &nodes[1];
         node_group[2] = NULL;

would look more familiar to me. Well, I suppose I can live with this if 
it is Ok to Lee though. Let's see if he has an opinion.

Other than this the change looks very good to me! Thanks.

Yours,
-- Matti

-- 
---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

Re: [PATCH v3 2/2] mfd: rohm-bd718x7: Use software nodes for gpio-keys

[Dmitry Torokhov]

On Fri, Mar 27, 2026 at 12:55:55PM +0200, Matti Vaittinen wrote:
> On 25/03/2026 02:54, Dmitry Torokhov wrote:
> > +
> > +	node_group = (const struct software_node *[]){
> > +		&nodes[0],
> > +		&nodes[1],
> > +		NULL
> > +	};
> 
> Hmm. I suppose I was not explaining myself well. When I asked for a
> temporary variable, I was hoping to get rid of this syntax. Something like:
> 	const struct software_node *node_group[3];
> 
>         node_group[0] = &nodes[0];
>         node_group[1] = &nodes[1];
>         node_group[2] = NULL;
> 
> would look more familiar to me. Well, I suppose I can live with this if it
> is Ok to Lee though. Let's see if he has an opinion.

This is simply a compound literal, part of the C standard since C99. It
allows skip explicitly declaring the dimensions of the node_group[]
array (which is "far" away from where we initialize it and it
potentially may get out of sync).

We have quite a few in the kernel, DEFINE_RES_IRQ() and others for
example are compound literals under the hood.  

Thanks.

-- 
Dmitry

Re: [PATCH v3 2/2] mfd: rohm-bd718x7: Use software nodes for gpio-keys

[Matti Vaittinen]

On 27/03/2026 18:10, Dmitry Torokhov wrote:
> On Fri, Mar 27, 2026 at 12:55:55PM +0200, Matti Vaittinen wrote:
>> On 25/03/2026 02:54, Dmitry Torokhov wrote:
>>> +
>>> +	node_group = (const struct software_node *[]){
>>> +		&nodes[0],
>>> +		&nodes[1],
>>> +		NULL
>>> +	};
>>
>> Hmm. I suppose I was not explaining myself well. When I asked for a
>> temporary variable, I was hoping to get rid of this syntax. Something like:
>> 	const struct software_node *node_group[3];
>>
>>          node_group[0] = &nodes[0];
>>          node_group[1] = &nodes[1];
>>          node_group[2] = NULL;
>>
>> would look more familiar to me. Well, I suppose I can live with this if it
>> is Ok to Lee though. Let's see if he has an opinion.
> 
> This is simply a compound literal, part of the C standard since C99. It
> allows skip explicitly declaring the dimensions of the node_group[]
> array (which is "far" away from where we initialize it and it
> potentially may get out of sync).
> 
> We have quite a few in the kernel, DEFINE_RES_IRQ() and others for
> example are compound literals under the hood.

Yes. But ones I've seen have been wrapped in macros. I don't think I've 
seen open-coded one written directly to a call-site (although that's 
what the macros end up). Hence, this is not something I see typically 
when reading drivers.

But as I said, if this is fine with Lee, I can live with this too :)

-- 
---
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

Re: [PATCH v3 2/2] mfd: rohm-bd718x7: Use software nodes for gpio-keys

[Lee Jones]

On Wed, 01 Apr 2026, Matti Vaittinen wrote:

> On 27/03/2026 18:10, Dmitry Torokhov wrote:
> > On Fri, Mar 27, 2026 at 12:55:55PM +0200, Matti Vaittinen wrote:
> > > On 25/03/2026 02:54, Dmitry Torokhov wrote:
> > > > +
> > > > +	node_group = (const struct software_node *[]){
> > > > +		&nodes[0],
> > > > +		&nodes[1],
> > > > +		NULL
> > > > +	};
> > > 
> > > Hmm. I suppose I was not explaining myself well. When I asked for a
> > > temporary variable, I was hoping to get rid of this syntax. Something like:
> > > 	const struct software_node *node_group[3];
> > > 
> > >          node_group[0] = &nodes[0];
> > >          node_group[1] = &nodes[1];
> > >          node_group[2] = NULL;
> > > 
> > > would look more familiar to me. Well, I suppose I can live with this if it
> > > is Ok to Lee though. Let's see if he has an opinion.
> > 
> > This is simply a compound literal, part of the C standard since C99. It
> > allows skip explicitly declaring the dimensions of the node_group[]
> > array (which is "far" away from where we initialize it and it
> > potentially may get out of sync).
> > 
> > We have quite a few in the kernel, DEFINE_RES_IRQ() and others for
> > example are compound literals under the hood.
> 
> Yes. But ones I've seen have been wrapped in macros. I don't think I've seen
> open-coded one written directly to a call-site (although that's what the
> macros end up). Hence, this is not something I see typically when reading
> drivers.
> 
> But as I said, if this is fine with Lee, I can live with this too :)

My personal preference is to avoid the use of compound literals for
structs inside functions.

-- 
Lee Jones [李琼斯]

Re: [PATCH v3 2/2] mfd: rohm-bd718x7: Use software nodes for gpio-keys

[Dmitry Torokhov]

On Wed, Apr 01, 2026 at 11:59:30AM +0100, Lee Jones wrote:
> On Wed, 01 Apr 2026, Matti Vaittinen wrote:
> 
> > On 27/03/2026 18:10, Dmitry Torokhov wrote:
> > > On Fri, Mar 27, 2026 at 12:55:55PM +0200, Matti Vaittinen wrote:
> > > > On 25/03/2026 02:54, Dmitry Torokhov wrote:
> > > > > +
> > > > > +	node_group = (const struct software_node *[]){
> > > > > +		&nodes[0],
> > > > > +		&nodes[1],
> > > > > +		NULL
> > > > > +	};
> > > > 
> > > > Hmm. I suppose I was not explaining myself well. When I asked for a
> > > > temporary variable, I was hoping to get rid of this syntax. Something like:
> > > > 	const struct software_node *node_group[3];
> > > > 
> > > >          node_group[0] = &nodes[0];
> > > >          node_group[1] = &nodes[1];
> > > >          node_group[2] = NULL;
> > > > 
> > > > would look more familiar to me. Well, I suppose I can live with this if it
> > > > is Ok to Lee though. Let's see if he has an opinion.
> > > 
> > > This is simply a compound literal, part of the C standard since C99. It
> > > allows skip explicitly declaring the dimensions of the node_group[]
> > > array (which is "far" away from where we initialize it and it
> > > potentially may get out of sync).
> > > 
> > > We have quite a few in the kernel, DEFINE_RES_IRQ() and others for
> > > example are compound literals under the hood.
> > 
> > Yes. But ones I've seen have been wrapped in macros. I don't think I've seen
> > open-coded one written directly to a call-site (although that's what the
> > macros end up). Hence, this is not something I see typically when reading
> > drivers.
> > 
> > But as I said, if this is fine with Lee, I can live with this too :)
> 
> My personal preference is to avoid the use of compound literals for
> structs inside functions.
> 

I assume use of macros like DEFINE_RES_IRQ() is still OK?

Thanks.

-- 
Dmitry