[PATCH 0/6] input: use device_for_each_child_node_scoped()

[PATCH 0/6] input: use device_for_each_child_node_scoped()

[Javier Carrasco]

Switch to the _scoped() version introduced in commit 365130fd47af
("device property: Introduce device_for_each_child_node_scoped()")
to remove the need for manual calling of fwnode_handle_put() in the
paths where the code exits the loop early. This modification simplifies
the code and eliminates the risk of leaking memory if any early exit is
added without de-allocating the child node.

There are six users of the non-scoped version in the input subsystem:

- iqs269a
- qt1050
- gpio_keys
- gpio_keys_polled
- adc-keys
- adc-joystick

This series is based on the master branch of linux-next (next-20240412)
to have access to the scoped version of device_for_each_child_node().

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
Javier Carrasco (6):
      input: iqs269a: use device_for_each_child_node_scoped()
      input: qt1050: use device_for_each_child_node_scoped()
      input: gpio_keys: use device_for_each_child_node_scoped()
      input: gpio_keys_polled: use device_for_each_child_node_scoped()
      input: adc-keys: use device_for_each_child_node_scoped()
      input: adc-joystick: use device_for_each_child_node_scoped()

 drivers/input/joystick/adc-joystick.c     | 16 +++++-----------
 drivers/input/keyboard/adc-keys.c         |  5 +----
 drivers/input/keyboard/gpio_keys.c        |  4 +---
 drivers/input/keyboard/gpio_keys_polled.c |  4 +---
 drivers/input/keyboard/qt1050.c           | 12 ++++--------
 drivers/input/misc/iqs269a.c              |  7 ++-----
 6 files changed, 14 insertions(+), 34 deletions(-)
---
base-commit: 9ed46da14b9b9b2ad4edb3b0c545b6dbe5c00d39
change-id: 20240404-input_device_for_each_child_node_scoped-0a55a76ad7ee

Best regards,
-- 
Javier Carrasco <javier.carrasco.cruz@gmail.com>

[PATCH 1/6] input: iqs269a: use device_for_each_child_node_scoped()

[Javier Carrasco]

Switch to the _scoped() version introduced in commit 365130fd47af
("device property: Introduce device_for_each_child_node_scoped()")
to remove the need for manual calling of fwnode_handle_put() in the
paths where the code exits the loop early.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/input/misc/iqs269a.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/input/misc/iqs269a.c b/drivers/input/misc/iqs269a.c
index cd14ff9f57cf..843f8a3f3410 100644
--- a/drivers/input/misc/iqs269a.c
+++ b/drivers/input/misc/iqs269a.c
@@ -811,7 +811,6 @@ static int iqs269_parse_prop(struct iqs269_private *iqs269)
 {
 	struct iqs269_sys_reg *sys_reg = &iqs269->sys_reg;
 	struct i2c_client *client = iqs269->client;
-	struct fwnode_handle *ch_node;
 	u16 general, misc_a, misc_b;
 	unsigned int val;
 	int error;
@@ -1049,12 +1048,10 @@ static int iqs269_parse_prop(struct iqs269_private *iqs269)
 
 	sys_reg->event_mask = ~((u8)IQS269_EVENT_MASK_SYS);
 
-	device_for_each_child_node(&client->dev, ch_node) {
+	device_for_each_child_node_scoped(&client->dev, ch_node) {
 		error = iqs269_parse_chan(iqs269, ch_node);
-		if (error) {
-			fwnode_handle_put(ch_node);
+		if (error)
 			return error;
-		}
 	}
 
 	/*

-- 
2.40.1

[PATCH 2/6] input: qt1050: use device_for_each_child_node_scoped()

[Javier Carrasco]

Switch to the _scoped() version introduced in commit 365130fd47af
("device property: Introduce device_for_each_child_node_scoped()")
to remove the need for manual calling of fwnode_handle_put() in the
paths where the code exits the loop early.

In this case the err label was no longer necessary and EINVAL is
returned directly.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/input/keyboard/qt1050.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/input/keyboard/qt1050.c b/drivers/input/keyboard/qt1050.c
index b51dfcd76038..6ac2b9dbdb85 100644
--- a/drivers/input/keyboard/qt1050.c
+++ b/drivers/input/keyboard/qt1050.c
@@ -355,21 +355,21 @@ static int qt1050_parse_fw(struct qt1050_priv *ts)
 		if (fwnode_property_read_u32(child, "linux,code",
 					     &button.keycode)) {
 			dev_err(dev, "Button without keycode\n");
-			goto err;
+			return -EINVAL;
 		}
 		if (button.keycode >= KEY_MAX) {
 			dev_err(dev, "Invalid keycode 0x%x\n",
 				button.keycode);
-			goto err;
+			return -EINVAL;
 		}
 
 		if (fwnode_property_read_u32(child, "reg",
 					     &button.num)) {
 			dev_err(dev, "Button without pad number\n");
-			goto err;
+			return -EINVAL;
 		}
 		if (button.num < 0 || button.num > QT1050_MAX_KEYS - 1)
-			goto err;
+			return -EINVAL;
 
 		ts->reg_keys |= BIT(button.num);
 
@@ -419,10 +419,6 @@ static int qt1050_parse_fw(struct qt1050_priv *ts)
 	}
 
 	return 0;
-
-err:
-	fwnode_handle_put(child);
-	return -EINVAL;
 }
 
 static int qt1050_probe(struct i2c_client *client)

-- 
2.40.1

[PATCH 3/6] input: gpio_keys: use device_for_each_child_node_scoped()

[Javier Carrasco]

Switch to the _scoped() version introduced in commit 365130fd47af
("device property: Introduce device_for_each_child_node_scoped()")
to remove the need for manual calling of fwnode_handle_put() in the
paths where the code exits the loop early.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/input/keyboard/gpio_keys.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 9f3bcd41cf67..9fb0bdcfbf9e 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -768,7 +768,6 @@ gpio_keys_get_devtree_pdata(struct device *dev)
 {
 	struct gpio_keys_platform_data *pdata;
 	struct gpio_keys_button *button;
-	struct fwnode_handle *child;
 	int nbuttons, irq;
 
 	nbuttons = device_get_child_node_count(dev);
@@ -790,7 +789,7 @@ gpio_keys_get_devtree_pdata(struct device *dev)
 
 	device_property_read_string(dev, "label", &pdata->name);
 
-	device_for_each_child_node(dev, child) {
+	device_for_each_child_node_scoped(dev, child) {
 		if (is_of_node(child)) {
 			irq = of_irq_get_byname(to_of_node(child), "irq");
 			if (irq > 0)
@@ -808,7 +807,6 @@ gpio_keys_get_devtree_pdata(struct device *dev)
 		if (fwnode_property_read_u32(child, "linux,code",
 					     &button->code)) {
 			dev_err(dev, "Button without keycode\n");
-			fwnode_handle_put(child);
 			return ERR_PTR(-EINVAL);
 		}
 

-- 
2.40.1

[PATCH 4/6] input: gpio_keys_polled: use device_for_each_child_node_scoped()

[Javier Carrasco]

Switch to the _scoped() version introduced in commit 365130fd47af
("device property: Introduce device_for_each_child_node_scoped()")
to remove the need for manual calling of fwnode_handle_put() in the
paths where the code exits the loop early.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/input/keyboard/gpio_keys_polled.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index b41fd1240f43..41ca0d3c9098 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -144,7 +144,6 @@ gpio_keys_polled_get_devtree_pdata(struct device *dev)
 {
 	struct gpio_keys_platform_data *pdata;
 	struct gpio_keys_button *button;
-	struct fwnode_handle *child;
 	int nbuttons;
 
 	nbuttons = device_get_child_node_count(dev);
@@ -166,11 +165,10 @@ gpio_keys_polled_get_devtree_pdata(struct device *dev)
 
 	device_property_read_string(dev, "label", &pdata->name);
 
-	device_for_each_child_node(dev, child) {
+	device_for_each_child_node_scoped(dev, child) {
 		if (fwnode_property_read_u32(child, "linux,code",
 					     &button->code)) {
 			dev_err(dev, "button without keycode\n");
-			fwnode_handle_put(child);
 			return ERR_PTR(-EINVAL);
 		}
 

-- 
2.40.1

[PATCH 5/6] input: adc-keys: use device_for_each_child_node_scoped()

[Javier Carrasco]

Switch to the _scoped() version introduced in commit 365130fd47af
("device property: Introduce device_for_each_child_node_scoped()")
to remove the need for manual calling of fwnode_handle_put() in the
paths where the code exits the loop early.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/input/keyboard/adc-keys.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/adc-keys.c b/drivers/input/keyboard/adc-keys.c
index bf72ab8df817..f1753207429d 100644
--- a/drivers/input/keyboard/adc-keys.c
+++ b/drivers/input/keyboard/adc-keys.c
@@ -66,7 +66,6 @@ static void adc_keys_poll(struct input_dev *input)
 static int adc_keys_load_keymap(struct device *dev, struct adc_keys_state *st)
 {
 	struct adc_keys_button *map;
-	struct fwnode_handle *child;
 	int i;
 
 	st->num_keys = device_get_child_node_count(dev);
@@ -80,11 +79,10 @@ static int adc_keys_load_keymap(struct device *dev, struct adc_keys_state *st)
 		return -ENOMEM;
 
 	i = 0;
-	device_for_each_child_node(dev, child) {
+	device_for_each_child_node_scoped(dev, child) {
 		if (fwnode_property_read_u32(child, "press-threshold-microvolt",
 					     &map[i].voltage)) {
 			dev_err(dev, "Key with invalid or missing voltage\n");
-			fwnode_handle_put(child);
 			return -EINVAL;
 		}
 		map[i].voltage /= 1000;
@@ -92,7 +90,6 @@ static int adc_keys_load_keymap(struct device *dev, struct adc_keys_state *st)
 		if (fwnode_property_read_u32(child, "linux,code",
 					     &map[i].keycode)) {
 			dev_err(dev, "Key with invalid or missing linux,code\n");
-			fwnode_handle_put(child);
 			return -EINVAL;
 		}
 

-- 
2.40.1

[PATCH 6/6] input: adc-joystick: use device_for_each_child_node_scoped()

[Javier Carrasco]

Switch to the _scoped() version introduced in commit 365130fd47af
("device property: Introduce device_for_each_child_node_scoped()")
to remove the need for manual calling of fwnode_handle_put() in the
paths where the code exits the loop early.

In this case the err_fwnode_put label was no longer necessary and the
error code is returned directly.

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/input/joystick/adc-joystick.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/input/joystick/adc-joystick.c b/drivers/input/joystick/adc-joystick.c
index c0deff5d4282..c7c2edf908e6 100644
--- a/drivers/input/joystick/adc-joystick.c
+++ b/drivers/input/joystick/adc-joystick.c
@@ -122,7 +122,6 @@ static void adc_joystick_cleanup(void *data)
 static int adc_joystick_set_axes(struct device *dev, struct adc_joystick *joy)
 {
 	struct adc_joystick_axis *axes;
-	struct fwnode_handle *child;
 	int num_axes, error, i;
 
 	num_axes = device_get_child_node_count(dev);
@@ -141,31 +140,30 @@ static int adc_joystick_set_axes(struct device *dev, struct adc_joystick *joy)
 	if (!axes)
 		return -ENOMEM;
 
-	device_for_each_child_node(dev, child) {
+	device_for_each_child_node_scoped(dev, child) {
 		error = fwnode_property_read_u32(child, "reg", &i);
 		if (error) {
 			dev_err(dev, "reg invalid or missing\n");
-			goto err_fwnode_put;
+			return error;
 		}
 
 		if (i >= num_axes) {
-			error = -EINVAL;
 			dev_err(dev, "No matching axis for reg %d\n", i);
-			goto err_fwnode_put;
+			return -EINVAL;
 		}
 
 		error = fwnode_property_read_u32(child, "linux,code",
 						 &axes[i].code);
 		if (error) {
 			dev_err(dev, "linux,code invalid or missing\n");
-			goto err_fwnode_put;
+			return error;
 		}
 
 		error = fwnode_property_read_u32_array(child, "abs-range",
 						       axes[i].range, 2);
 		if (error) {
 			dev_err(dev, "abs-range invalid or missing\n");
-			goto err_fwnode_put;
+			return error;
 		}
 
 		fwnode_property_read_u32(child, "abs-fuzz", &axes[i].fuzz);
@@ -180,10 +178,6 @@ static int adc_joystick_set_axes(struct device *dev, struct adc_joystick *joy)
 	joy->axes = axes;
 
 	return 0;
-
-err_fwnode_put:
-	fwnode_handle_put(child);
-	return error;
 }
 
 static int adc_joystick_probe(struct platform_device *pdev)

-- 
2.40.1

Re: [PATCH 2/6] input: qt1050: use device_for_each_child_node_scoped()

[Dmitry Torokhov]

Hi Javier,

On Fri, Apr 12, 2024 at 10:57:31PM +0200, Javier Carrasco wrote:
> Switch to the _scoped() version introduced in commit 365130fd47af
> ("device property: Introduce device_for_each_child_node_scoped()")
> to remove the need for manual calling of fwnode_handle_put() in the
> paths where the code exits the loop early.
> 
> In this case the err label was no longer necessary and EINVAL is
> returned directly.
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> ---
>  drivers/input/keyboard/qt1050.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/input/keyboard/qt1050.c b/drivers/input/keyboard/qt1050.c
> index b51dfcd76038..6ac2b9dbdb85 100644
> --- a/drivers/input/keyboard/qt1050.c
> +++ b/drivers/input/keyboard/qt1050.c
> @@ -355,21 +355,21 @@ static int qt1050_parse_fw(struct qt1050_priv *ts)
>  		if (fwnode_property_read_u32(child, "linux,code",
>  					     &button.keycode)) {
>  			dev_err(dev, "Button without keycode\n");
> -			goto err;
> +			return -EINVAL;

It looks like the chunk actually switching to
device_for_each_child_node_scoped() is missing from the patch. I added
it and applied, thank you.

Thanks.

-- 
Dmitry

Re: [PATCH 0/6] input: use device_for_each_child_node_scoped()

[Dmitry Torokhov]

On Fri, Apr 12, 2024 at 10:57:29PM +0200, Javier Carrasco wrote:
> Switch to the _scoped() version introduced in commit 365130fd47af
> ("device property: Introduce device_for_each_child_node_scoped()")
> to remove the need for manual calling of fwnode_handle_put() in the
> paths where the code exits the loop early. This modification simplifies
> the code and eliminates the risk of leaking memory if any early exit is
> added without de-allocating the child node.
> 
> There are six users of the non-scoped version in the input subsystem:
> 
> - iqs269a
> - qt1050
> - gpio_keys
> - gpio_keys_polled
> - adc-keys
> - adc-joystick
> 
> This series is based on the master branch of linux-next (next-20240412)
> to have access to the scoped version of device_for_each_child_node().
> 
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

Applied the series (after adjusting qt1050 patch), thank you.

-- 
Dmitry

Re: [PATCH 2/6] input: qt1050: use device_for_each_child_node_scoped()

[Javier Carrasco]

On 16/07/2024 03:03, Dmitry Torokhov wrote:
> Hi Javier,
> 
> On Fri, Apr 12, 2024 at 10:57:31PM +0200, Javier Carrasco wrote:
>> Switch to the _scoped() version introduced in commit 365130fd47af
>> ("device property: Introduce device_for_each_child_node_scoped()")
>> to remove the need for manual calling of fwnode_handle_put() in the
>> paths where the code exits the loop early.
>>
>> In this case the err label was no longer necessary and EINVAL is
>> returned directly.
>>
>> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
>> ---
>>  drivers/input/keyboard/qt1050.c | 12 ++++--------
>>  1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/input/keyboard/qt1050.c b/drivers/input/keyboard/qt1050.c
>> index b51dfcd76038..6ac2b9dbdb85 100644
>> --- a/drivers/input/keyboard/qt1050.c
>> +++ b/drivers/input/keyboard/qt1050.c
>> @@ -355,21 +355,21 @@ static int qt1050_parse_fw(struct qt1050_priv *ts)
>>  		if (fwnode_property_read_u32(child, "linux,code",
>>  					     &button.keycode)) {
>>  			dev_err(dev, "Button without keycode\n");
>> -			goto err;
>> +			return -EINVAL;
> 
> It looks like the chunk actually switching to
> device_for_each_child_node_scoped() is missing from the patch. I added
> it and applied, thank you.
> 
> Thanks.
> 


Thank your for adding the missing bit. Did you remove the child variable
at the beginning of the function as well? It will be unused otherwise
(child is defined in the macro itself).

Thanks again and best regards,
Javier Carrasco

Re: [PATCH 2/6] input: qt1050: use device_for_each_child_node_scoped()

[Dmitry Torokhov]

On Tue, Jul 16, 2024 at 06:17:35AM +0200, Javier Carrasco wrote:
> On 16/07/2024 03:03, Dmitry Torokhov wrote:
> > Hi Javier,
> > 
> > On Fri, Apr 12, 2024 at 10:57:31PM +0200, Javier Carrasco wrote:
> >> Switch to the _scoped() version introduced in commit 365130fd47af
> >> ("device property: Introduce device_for_each_child_node_scoped()")
> >> to remove the need for manual calling of fwnode_handle_put() in the
> >> paths where the code exits the loop early.
> >>
> >> In this case the err label was no longer necessary and EINVAL is
> >> returned directly.
> >>
> >> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> >> ---
> >>  drivers/input/keyboard/qt1050.c | 12 ++++--------
> >>  1 file changed, 4 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/input/keyboard/qt1050.c b/drivers/input/keyboard/qt1050.c
> >> index b51dfcd76038..6ac2b9dbdb85 100644
> >> --- a/drivers/input/keyboard/qt1050.c
> >> +++ b/drivers/input/keyboard/qt1050.c
> >> @@ -355,21 +355,21 @@ static int qt1050_parse_fw(struct qt1050_priv *ts)
> >>  		if (fwnode_property_read_u32(child, "linux,code",
> >>  					     &button.keycode)) {
> >>  			dev_err(dev, "Button without keycode\n");
> >> -			goto err;
> >> +			return -EINVAL;
> > 
> > It looks like the chunk actually switching to
> > device_for_each_child_node_scoped() is missing from the patch. I added
> > it and applied, thank you.
> > 
> > Thanks.
> > 
> 
> 
> Thank your for adding the missing bit. Did you remove the child variable
> at the beginning of the function as well? It will be unused otherwise
> (child is defined in the macro itself).

Yep, I did.

Thanks.

-- 
Dmitry