[PATCH 1/3] Input: gpio_keys - fall back to platform_get_irq() for interrupt-only keys

[PATCH 1/3] Input: gpio_keys - fall back to platform_get_irq() for interrupt-only keys

[Dmitry Torokhov]

To allow transitioning away from gpio-keys platform data attempt to
retrieve IRQ for interrupt-only keys using platform_get_irq_optional()
if interrupt is not specified in platform data.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/gpio_keys.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index f9db86da0818..f56e92f7d631 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -616,12 +616,19 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 			break;
 		}
 	} else {
-		if (!button->irq) {
-			dev_err(dev, "Found button without gpio or irq\n");
-			return -EINVAL;
-		}
+		if (button->irq) {
+			bdata->irq = button->irq;
+		} else {
+			irq = platform_get_irq_optional(pdev, idx);
+			if (irq < 0) {
+				error = irq;
+				return dev_err_probe(dev, error,
+						     "Unable to determine IRQ# for button #%d",
+						     idx);
+			}
 
-		bdata->irq = button->irq;
+			bdata->irq = irq;
+		}
 
 		if (button->type && button->type != EV_KEY) {
 			dev_err(dev, "Only EV_KEY allowed for IRQ buttons.\n");
-- 
2.51.0.rc1.163.g2494970778-goog

[PATCH 2/3] 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 | 81 +++++++++++++++++++++++++++-----------
 1 file changed, 58 insertions(+), 23 deletions(-)

diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
index a14b7aa69c3c..c29dde9996b7 100644
--- a/drivers/mfd/rohm-bd71828.c
+++ b/drivers/mfd/rohm-bd71828.c
@@ -4,7 +4,6 @@
 //
 // ROHM BD71828/BD71815 PMIC driver
 
-#include <linux/gpio_keys.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
 #include <linux/interrupt.h>
@@ -16,21 +15,32 @@
 #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>
 
-static struct gpio_keys_button button = {
-	.code = KEY_POWER,
-	.gpio = -1,
-	.type = EV_KEY,
+static const struct software_node bd71828_pwrkey_node = {
+	.name = "bd71828-power-key",
 };
 
-static const struct gpio_keys_platform_data bd71828_powerkey_data = {
-	.buttons = &button,
-	.nbuttons = 1,
-	.name = "bd71828-pwrkey",
+static const struct property_entry bd71828_powerkey_props[] = {
+	PROPERTY_ENTRY_U32("linux,code", KEY_POWER),
+	PROPERTY_ENTRY_STRING("label", "bd71828-pwrkey"),
+	{ }
 };
 
+static const struct software_node bd71828_powerkey_key_node = {
+	.properties = bd71828_powerkey_props,
+	.parent = &bd71828_pwrkey_node,
+};
+
+static const struct software_node *bd71828_swnodes[] = {
+	&bd71828_pwrkey_node,
+	&bd71828_powerkey_key_node,
+	NULL,
+};
+
+
 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"),
@@ -93,6 +103,10 @@ static const struct resource bd71815_power_irqs[] = {
 	DEFINE_RES_IRQ_NAMED(BD71815_INT_TEMP_BAT_HI_DET, "bd71815-bat-hi-det"),
 };
 
+static const struct resource bd71828_powerkey_irq_resources[] = {
+	DEFINE_RES_IRQ_NAMED(BD71828_INT_SHORTPUSH, "bd71828-pwrkey"),
+};
+
 static const struct mfd_cell bd71815_mfd_cells[] = {
 	{ .name = "bd71815-pmic", },
 	{ .name = "bd71815-clk", },
@@ -125,8 +139,9 @@ static const struct mfd_cell bd71828_mfd_cells[] = {
 		.num_resources = ARRAY_SIZE(bd71828_rtc_irqs),
 	}, {
 		.name = "gpio-keys",
-		.platform_data = &bd71828_powerkey_data,
-		.pdata_size = sizeof(bd71828_powerkey_data),
+		.swnode = &bd71828_pwrkey_node,
+		.resources = bd71828_powerkey_irq_resources,
+		.num_resources = ARRAY_SIZE(bd71828_powerkey_irq_resources),
 	},
 };
 
@@ -464,6 +479,30 @@ static int set_clk_mode(struct device *dev, struct regmap *regmap,
 				  OUT32K_MODE_CMOS);
 }
 
+static int bd71828_reg_cnt;
+
+static int bd71828_i2c_register_swnodes(void)
+{
+	int error;
+
+	if (bd71828_reg_cnt == 0) {
+		error = software_node_register_node_group(bd71828_swnodes);
+		if (error)
+			return error;
+	}
+
+	bd71828_reg_cnt++;
+	return 0;
+}
+
+static void bd71828_i2c_unregister_swnodes(void *dummy)
+{
+	if (bd71828_reg_cnt != 0) {
+		software_node_unregister_node_group(bd71828_swnodes);
+		bd71828_reg_cnt--;
+	}
+}
+
 static struct i2c_client *bd71828_dev;
 static void bd71828_power_off(void)
 {
@@ -495,7 +534,6 @@ static int bd71828_i2c_probe(struct i2c_client *i2c)
 	unsigned int chip_type;
 	const struct mfd_cell *mfd;
 	int cells;
-	int button_irq;
 	int clkmode_reg;
 
 	if (!i2c->irq) {
@@ -512,7 +550,14 @@ static int bd71828_i2c_probe(struct i2c_client *i2c)
 		regmap_config = &bd71828_regmap;
 		irqchip = &bd71828_irq_chip;
 		clkmode_reg = BD71828_REG_OUT32K;
-		button_irq = BD71828_INT_SHORTPUSH;
+		ret = bd71828_i2c_register_swnodes();
+		if (ret)
+			return dev_err_probe(&i2c->dev, ret, "Failed to register swnodes\n");
+
+		ret = devm_add_action_or_reset(&i2c->dev, bd71828_i2c_unregister_swnodes, NULL);
+		if (ret)
+			return ret;
+
 		break;
 	case ROHM_CHIP_TYPE_BD71815:
 		mfd = bd71815_mfd_cells;
@@ -526,7 +571,6 @@ static int bd71828_i2c_probe(struct i2c_client *i2c)
 		 * BD71815 data-sheet does not list the power-button IRQ so we
 		 * don't use it.
 		 */
-		button_irq = 0;
 		break;
 	default:
 		dev_err(&i2c->dev, "Unknown device type");
@@ -547,15 +591,6 @@ static int bd71828_i2c_probe(struct i2c_client *i2c)
 	dev_dbg(&i2c->dev, "Registered %d IRQs for chip\n",
 		irqchip->num_irqs);
 
-	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;
-- 
2.51.0.rc1.163.g2494970778-goog

[PATCH 3/3] 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.
---
 drivers/mfd/rohm-bd718x7.c | 76 ++++++++++++++++++++++++++++++--------
 1 file changed, 60 insertions(+), 16 deletions(-)

diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c
index 25e494a93d48..20150656ac9c 100644
--- a/drivers/mfd/rohm-bd718x7.c
+++ b/drivers/mfd/rohm-bd718x7.c
@@ -7,7 +7,6 @@
 // Datasheet for BD71837MWV available from
 // https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e
 
-#include <linux/gpio_keys.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
 #include <linux/interrupt.h>
@@ -15,26 +14,41 @@
 #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 const struct software_node bd718xx_pwrkey_node = {
+	.name = "bd718xx-power-key",
 };
 
-static struct gpio_keys_platform_data bd718xx_powerkey_data = {
-	.buttons = &button,
-	.nbuttons = 1,
-	.name = "bd718xx-pwrkey",
+static const struct property_entry bd718xx_powerkey_props[] = {
+	PROPERTY_ENTRY_U32("linux,code", KEY_POWER),
+	PROPERTY_ENTRY_STRING("label", "bd718xx-pwrkey"),
+	{ }
+};
+
+static const struct software_node bd718xx_powerkey_key_node = {
+	.properties = bd718xx_powerkey_props,
+	.parent = &bd718xx_pwrkey_node,
+};
+
+static const struct software_node *bd718xx_swnodes[] = {
+	&bd718xx_pwrkey_node,
+	&bd718xx_powerkey_key_node,
+	NULL,
+};
+
+static struct resource bd718xx_powerkey_irq_resources[] = {
+	DEFINE_RES_IRQ_NAMED(BD718XX_INT_PWRBTN_S, "bd718xx-pwrkey"),
 };
 
 static struct mfd_cell bd71837_mfd_cells[] = {
 	{
 		.name = "gpio-keys",
-		.platform_data = &bd718xx_powerkey_data,
-		.pdata_size = sizeof(bd718xx_powerkey_data),
+		.swnode = &bd718xx_pwrkey_node,
+		.resources = bd718xx_powerkey_irq_resources,
+		.num_resources = ARRAY_SIZE(bd718xx_powerkey_irq_resources),
 	},
 	{ .name = "bd71837-clk", },
 	{ .name = "bd71837-pmic", },
@@ -43,8 +57,9 @@ static struct mfd_cell bd71837_mfd_cells[] = {
 static struct mfd_cell bd71847_mfd_cells[] = {
 	{
 		.name = "gpio-keys",
-		.platform_data = &bd718xx_powerkey_data,
-		.pdata_size = sizeof(bd718xx_powerkey_data),
+		.swnode = &bd718xx_pwrkey_node,
+		.resources = bd718xx_powerkey_irq_resources,
+		.num_resources = ARRAY_SIZE(bd718xx_powerkey_irq_resources),
 	},
 	{ .name = "bd71847-clk", },
 	{ .name = "bd71847-pmic", },
@@ -126,6 +141,30 @@ static int bd718xx_init_press_duration(struct regmap *regmap,
 	return 0;
 }
 
+static int bd718xx_reg_cnt;
+
+static int bd718xx_i2c_register_swnodes(void)
+{
+	int error;
+
+	if (bd718xx_reg_cnt == 0) {
+		error = software_node_register_node_group(bd718xx_swnodes);
+		if (error)
+			return error;
+	}
+
+	bd718xx_reg_cnt++;
+	return 0;
+}
+
+static void bd718xx_i2c_unregister_swnodes(void *dummy)
+{
+	if (bd718xx_reg_cnt != 0) {
+		software_node_unregister_node_group(bd718xx_swnodes);
+		bd718xx_reg_cnt--;
+	}
+}
+
 static int bd718xx_i2c_probe(struct i2c_client *i2c)
 {
 	struct regmap *regmap;
@@ -170,13 +209,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);
+	ret = bd718xx_i2c_register_swnodes();
+	if (ret)
+		return dev_err_probe(&i2c->dev, ret, "Failed to register swnodes\n");
+
+	ret = devm_add_action_or_reset(&i2c->dev, bd718xx_i2c_unregister_swnodes, NULL);
+	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;
-
 	ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO,
 				   mfd, cells, NULL, 0,
 				   regmap_irq_get_domain(irq_data));
-- 
2.51.0.rc1.163.g2494970778-goog

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

[Matti Vaittinen]

On 18/08/2025 01:47, 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.

Thanks for doing this Dmitry! I believe I didn't understand how 
providing the IRQs via swnode works... :)

If I visit the ROHM office this week, then I will try to test this using 
the PMIC HW. (Next week I'll be in ELCE, and after it I have probably 
already forgotten this...)

> 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 | 81 +++++++++++++++++++++++++++-----------
>   1 file changed, 58 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
> index a14b7aa69c3c..c29dde9996b7 100644
> --- a/drivers/mfd/rohm-bd71828.c
> +++ b/drivers/mfd/rohm-bd71828.c
> @@ -4,7 +4,6 @@

// ...snip

>   
> +static int bd71828_reg_cnt;
> +
> +static int bd71828_i2c_register_swnodes(void)
> +{
> +	int error;
> +
> +	if (bd71828_reg_cnt == 0) {

Isn't this check racy...

> +		error = software_node_register_node_group(bd71828_swnodes);
> +		if (error)
> +			return error;
> +	}
> +
> +	bd71828_reg_cnt++;

... with this...

> +	return 0;
> +}
> +
> +static void bd71828_i2c_unregister_swnodes(void *dummy)
> +{
> +	if (bd71828_reg_cnt != 0) {

...this...

> +		software_node_unregister_node_group(bd71828_swnodes);
> +		bd71828_reg_cnt--;

...and this? Perhaps add a mutex or use atomics?

Also, shouldn't the software_node_unregister_node_group() be only called 
for the last instance to exit (Eg, "if (bd71828_reg_cnt == 0)" instead 
of the "if (bd71828_reg_cnt != 0) {")?

> +	}
> +}
> +

Other than that - I like this idea :)

Thanks!

Yours,
	-- Matti

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

[Matti Vaittinen]

On 18/08/2025 09:54, Matti Vaittinen wrote:
> On 18/08/2025 01:47, 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.
> 
> Thanks for doing this Dmitry! I believe I didn't understand how 
> providing the IRQs via swnode works... :)
> 
> If I visit the ROHM office this week, then I will try to test this using 
> the PMIC HW. (Next week I'll be in ELCE, and after it I have probably 
> already forgotten this...)
> 
>> 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 | 81 +++++++++++++++++++++++++++-----------
>>   1 file changed, 58 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
>> index a14b7aa69c3c..c29dde9996b7 100644
>> --- a/drivers/mfd/rohm-bd71828.c
>> +++ b/drivers/mfd/rohm-bd71828.c
>> @@ -4,7 +4,6 @@
> 
> // ...snip
> 
>> +static int bd71828_reg_cnt;
>> +
>> +static int bd71828_i2c_register_swnodes(void)
>> +{
>> +    int error;
>> +
>> +    if (bd71828_reg_cnt == 0) {
> 
> Isn't this check racy...
> 
>> +        error = software_node_register_node_group(bd71828_swnodes);
>> +        if (error)
>> +            return error;
>> +    }
>> +
>> +    bd71828_reg_cnt++;
> 
> ... with this...
> 
>> +    return 0;
>> +}
>> +
>> +static void bd71828_i2c_unregister_swnodes(void *dummy)
>> +{
>> +    if (bd71828_reg_cnt != 0) {
> 
> ...this...
> 
>> +        software_node_unregister_node_group(bd71828_swnodes);
>> +        bd71828_reg_cnt--;
> 
> ...and this? Perhaps add a mutex or use atomics?
> 
> Also, shouldn't the software_node_unregister_node_group() be only called 
> for the last instance to exit (Eg, "if (bd71828_reg_cnt == 0)" instead 
> of the "if (bd71828_reg_cnt != 0) {")?

Oh. Probably "if (bd71828_reg_cnt == 1)".

> 
>> +    }
>> +}
>> +
> 
> Other than that - I like this idea :)
> 
> Thanks!
> 
> Yours,
>      -- Matti

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

[Matti Vaittinen]

On 18/08/2025 01:47, 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.
> ---
>   drivers/mfd/rohm-bd718x7.c | 76 ++++++++++++++++++++++++++++++--------
>   1 file changed, 60 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/mfd/rohm-bd718x7.c b/drivers/mfd/rohm-bd718x7.c
> index 25e494a93d48..20150656ac9c 100644
> --- a/drivers/mfd/rohm-bd718x7.c
> +++ b/drivers/mfd/rohm-bd718x7.c
> @@ -7,7 +7,6 @@
>   // Datasheet for BD71837MWV available from
>   // https://www.rohm.com/datasheet/BD71837MWV/bd71837mwv-e

...

>   
> +static int bd718xx_reg_cnt;
> +
> +static int bd718xx_i2c_register_swnodes(void)
> +{
> +	int error;
> +
> +	if (bd718xx_reg_cnt == 0) {
> +		error = software_node_register_node_group(bd718xx_swnodes);
> +		if (error)
> +			return error;
> +	}
> +
> +	bd718xx_reg_cnt++;
> +	return 0;
> +}
> +
> +static void bd718xx_i2c_unregister_swnodes(void *dummy)
> +{
> +	if (bd718xx_reg_cnt != 0) {
> +		software_node_unregister_node_group(bd718xx_swnodes);
> +		bd718xx_reg_cnt--;
> +	}
> +}

Same questions here as with the patch 2/3.


Yours,
	-- Matti

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

[Dmitry Torokhov]

On Mon, Aug 18, 2025 at 09:56:07AM +0300, Matti Vaittinen wrote:
> On 18/08/2025 09:54, Matti Vaittinen wrote:
> > On 18/08/2025 01:47, 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.
> > 
> > Thanks for doing this Dmitry! I believe I didn't understand how
> > providing the IRQs via swnode works... :)
> > 
> > If I visit the ROHM office this week, then I will try to test this using
> > the PMIC HW. (Next week I'll be in ELCE, and after it I have probably
> > already forgotten this...)
> > 
> > > 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 | 81 +++++++++++++++++++++++++++-----------
> > >   1 file changed, 58 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
> > > index a14b7aa69c3c..c29dde9996b7 100644
> > > --- a/drivers/mfd/rohm-bd71828.c
> > > +++ b/drivers/mfd/rohm-bd71828.c
> > > @@ -4,7 +4,6 @@
> > 
> > // ...snip
> > 
> > > +static int bd71828_reg_cnt;
> > > +
> > > +static int bd71828_i2c_register_swnodes(void)
> > > +{
> > > +    int error;
> > > +
> > > +    if (bd71828_reg_cnt == 0) {
> > 
> > Isn't this check racy...
> > 
> > > +        error = software_node_register_node_group(bd71828_swnodes);
> > > +        if (error)
> > > +            return error;
> > > +    }
> > > +
> > > +    bd71828_reg_cnt++;
> > 
> > ... with this...
> > 
> > > +    return 0;
> > > +}
> > > +
> > > +static void bd71828_i2c_unregister_swnodes(void *dummy)
> > > +{
> > > +    if (bd71828_reg_cnt != 0) {
> > 
> > ...this...
> > 
> > > +        software_node_unregister_node_group(bd71828_swnodes);
> > > +        bd71828_reg_cnt--;
> > 
> > ...and this? Perhaps add a mutex or use atomics?
> > 
> > Also, shouldn't the software_node_unregister_node_group() be only called
> > for the last instance to exit (Eg, "if (bd71828_reg_cnt == 0)" instead
> > of the "if (bd71828_reg_cnt != 0) {")?
> 
> Oh. Probably "if (bd71828_reg_cnt == 1)".

You are right, I am not sure what I was thinking when I wrote this.

I actually doubt that sharing of nodes between devices would work well.
But I believe these devices are singletons, it should not be possible to
have several of them in a single system, right? So maybe the best way is
to simply instantiate them in probe and bail out if they are already
registered.

Thanks.

-- 
Dmitry

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

[Matti Vaittinen]

On 18/08/2025 20:11, Dmitry Torokhov wrote:
> On Mon, Aug 18, 2025 at 09:56:07AM +0300, Matti Vaittinen wrote:
>> On 18/08/2025 09:54, Matti Vaittinen wrote:
>>> On 18/08/2025 01:47, 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.
>>>
>>> Thanks for doing this Dmitry! I believe I didn't understand how
>>> providing the IRQs via swnode works... :)
>>>
>>> If I visit the ROHM office this week, then I will try to test this using
>>> the PMIC HW. (Next week I'll be in ELCE, and after it I have probably
>>> already forgotten this...)
>>>
>>>> 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 | 81 +++++++++++++++++++++++++++-----------
>>>>    1 file changed, 58 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/drivers/mfd/rohm-bd71828.c b/drivers/mfd/rohm-bd71828.c
>>>> index a14b7aa69c3c..c29dde9996b7 100644
>>>> --- a/drivers/mfd/rohm-bd71828.c
>>>> +++ b/drivers/mfd/rohm-bd71828.c
>>>> @@ -4,7 +4,6 @@
>>>
>>> // ...snip
>>>
>>>> +static int bd71828_reg_cnt;
>>>> +
>>>> +static int bd71828_i2c_register_swnodes(void)
>>>> +{
>>>> +    int error;
>>>> +
>>>> +    if (bd71828_reg_cnt == 0) {
>>>
>>> Isn't this check racy...
>>>
>>>> +        error = software_node_register_node_group(bd71828_swnodes);
>>>> +        if (error)
>>>> +            return error;
>>>> +    }
>>>> +
>>>> +    bd71828_reg_cnt++;
>>>
>>> ... with this...
>>>
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static void bd71828_i2c_unregister_swnodes(void *dummy)
>>>> +{
>>>> +    if (bd71828_reg_cnt != 0) {
>>>
>>> ...this...
>>>
>>>> +        software_node_unregister_node_group(bd71828_swnodes);
>>>> +        bd71828_reg_cnt--;
>>>
>>> ...and this? Perhaps add a mutex or use atomics?
>>>
>>> Also, shouldn't the software_node_unregister_node_group() be only called
>>> for the last instance to exit (Eg, "if (bd71828_reg_cnt == 0)" instead
>>> of the "if (bd71828_reg_cnt != 0) {")?
>>
>> Oh. Probably "if (bd71828_reg_cnt == 1)".
> 
> You are right, I am not sure what I was thinking when I wrote this.
> 
> I actually doubt that sharing of nodes between devices would work well.
> But I believe these devices are singletons, it should not be possible to
> have several of them in a single system, right?

I can't say for sure. I have seen more and more setups where more than 
one PMIC is used to power-up a system. Thus I nowadays try to use 
solutions which don't limit the amount of instances.

The BD718[37,47,50] regulator driver seems to be written in a way it 
doesn't properly support multiple driver instances. (It uses global 
data, with a comment that if multiple instances need to be supported the 
data should be copied):
https://elixir.bootlin.com/linux/v6.11-rc2/source/drivers/regulator/bd718x7-regulator.c#L1558

For BD71828 and BD71815 I don't see existing limitations on how many 
instances there can be...

...except that I do :)

The current MFD driver uses single static global for the gpio_keys 
platform data. I assume that wouldn't be race-free if we had multiple 
instances.

So, I am unsure what to say. I know that for example the BD9680x PMIC 
series is intended to be used with multi-PMIC configurations, and I 
believe these setups are getting more common. Hence I would like to see 
the bd718XX code to work on multi-PMIC systems too, so the gpio_keys 
swnode example could be copied over to new PMICs ;)

But yeah, I am not insisting on it. The existing solution does not 
support multiple instances, so if you think it gets too cumbersome to 
add such support, then I am happy with supporting just one chip/system.

> So maybe the best way is
> to simply instantiate them in probe and bail out if they are already
> registered.

Well, I wouldn't say best (as explained above), but yes, sufficient for 
these PMICs AFAICS.

Thanks for doing this!

Yours,
	-- Matti

Re: [PATCH 1/3] Input: gpio_keys - fall back to platform_get_irq() for interrupt-only keys

[Andy Shevchenko]

On Sun, Aug 17, 2025 at 03:47:26PM -0700, Dmitry Torokhov wrote:
> To allow transitioning away from gpio-keys platform data attempt to
> retrieve IRQ for interrupt-only keys using platform_get_irq_optional()
> if interrupt is not specified in platform data.

...

> +			irq = platform_get_irq_optional(pdev, idx);
> +			if (irq < 0) {

> +				error = irq;
> +				return dev_err_probe(dev, error,

Assigning error is redundant.

> +						     "Unable to determine IRQ# for button #%d",
> +						     idx);
> +			}

-- 
With Best Regards,
Andy Shevchenko