Skip to contentSkip to conversation

input: automate of_node_put() calls for device_node

PATCH Applied Javier Carrasco 3 participants 5 messages Oct 21, 2024 – Oct 22, 2024

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

v3 cover input: automate of_node_put() calls for device_node

The first patch of the series provides device managed memory, which simplifies bbc_bee_probe() and removes the need for the goto instructions That also prepares the function for the second patch where the cleanup attribute is used for 'dp'.

---
Changes in v3:
- Add patch to use device managed memory and simplify the code by
dropping labels in bbc_beep_probe().
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- rebase onto input/next, drop applied patches.
- sparcspkr: drop goto before node declaration and return -ENOMEM.
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Javier Carrasco (2):
Input: sparcspkr - use device managed memory for 'state'
Input: sparcspkr - use cleanup facility for device_node

 drivers/input/misc/sparcspkr.c | 45 ++++++++++++++----------------------------
 1 file changed, 15 insertions(+), 30 deletions(-)

---
base-commit: d0c3a7aa814c091843ccca467c02078db9da4e1e
change-id: 20241009-input_automate_of_node_put-1bae9f5c02d9

Best regards,

Signature · 1 line
Javier Carrasco <[email protected]>
#1 lore raw

PATCH v3 1/2 Input: sparcspkr - use device managed memory for 'state'

Use devm_kzalloc() in bbc_bee_probe() and grover_beep_probe() to automatically free 'state' when the device is removed. Drop the kfree(state) calls from the probe error paths and the remove functions accordingly.

---
 drivers/input/misc/sparcspkr.c | 41 ++++++++++++++---------------------------
 1 file changed, 14 insertions(+), 27 deletions(-)
drivers/input/misc/sparcspkr.c +14 -27
index ff7b6291894a..e484d79b5597 100644
@@ -183,46 +183,39 @@ static int bbc_beep_probe(struct platform_device *op)
 	struct sparcspkr_state *state;
 	struct bbc_beep_info *info;
 	struct device_node *dp;
-	int err = -ENOMEM;
+	int err;
 
-	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	state = devm_kzalloc(&op->dev, sizeof(*state), GFP_KERNEL);
 	if (!state)
-		goto out_err;
+		return -ENOMEM;
 
 	state->name = "Sparc BBC Speaker";
 	state->event = bbc_spkr_event;
 	spin_lock_init(&state->lock);
 
 	dp = of_find_node_by_path("/");
-	err = -ENODEV;
 	if (!dp)
-		goto out_free;
+		return -ENODEV;
 
 	info = &state->u.bbc;
 	info->clock_freq = of_getintprop_default(dp, "clock-frequency", 0);
 	of_node_put(dp);
 	if (!info->clock_freq)
-		goto out_free;
+		return -ENODEV;
 
 	info->regs = of_ioremap(&op->resource[0], 0, 6, "bbc beep");
 	if (!info->regs)
-		goto out_free;
+		return -ENODEV;
 
 	platform_set_drvdata(op, state);
 
 	err = sparcspkr_probe(&op->dev);
-	if (err)
-		goto out_clear_drvdata;
+	if (err) {
+		of_iounmap(&op->resource[0], info->regs, 6);
+		return err;
+	}
 
 	return 0;
-
-out_clear_drvdata:
-	of_iounmap(&op->resource[0], info->regs, 6);
-
-out_free:
-	kfree(state);
-out_err:
-	return err;
 }
 
 static void bbc_remove(struct platform_device *op)
@@ -237,8 +230,6 @@ static void bbc_remove(struct platform_device *op)
 	input_unregister_device(input_dev);
 
 	of_iounmap(&op->resource[0], info->regs, 6);
-
-	kfree(state);
 }
 
 static const struct of_device_id bbc_beep_match[] = {
@@ -266,9 +257,9 @@ static int grover_beep_probe(struct platform_device *op)
 	struct grover_beep_info *info;
 	int err = -ENOMEM;
 
-	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	state = devm_kzalloc(&op->dev, sizeof(*state), GFP_KERNEL);
 	if (!state)
-		goto out_err;
+		return err;
 
 	state->name = "Sparc Grover Speaker";
 	state->event = grover_spkr_event;
@@ -277,7 +268,7 @@ static int grover_beep_probe(struct platform_device *op)
 	info = &state->u.grover;
 	info->freq_regs = of_ioremap(&op->resource[2], 0, 2, "grover beep freq");
 	if (!info->freq_regs)
-		goto out_free;
+		return err;
 
 	info->enable_reg = of_ioremap(&op->resource[3], 0, 1, "grover beep enable");
 	if (!info->enable_reg)
@@ -296,9 +287,7 @@ static int grover_beep_probe(struct platform_device *op)
 
 out_unmap_freq_regs:
 	of_iounmap(&op->resource[2], info->freq_regs, 2);
-out_free:
-	kfree(state);
-out_err:
+
 	return err;
 }
 
@@ -315,8 +304,6 @@ static void grover_remove(struct platform_device *op)
 
 	of_iounmap(&op->resource[3], info->enable_reg, 1);
 	of_iounmap(&op->resource[2], info->freq_regs, 2);
-
-	kfree(state);
 }
 
 static const struct of_device_id grover_beep_match[] = {
Signature · 1 line
2.43.0
#2 lore raw

PATCH v3 2/2 Input: sparcspkr - use cleanup facility for device_node

In reply to Javier Carrasco

Use the 'free(device_node)' macro to simplify the code by automatically freeing the device node, which removes the need for explicit calls to 'of_node_put()'.

---
 drivers/input/misc/sparcspkr.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/input/misc/sparcspkr.c +1 -3
index e484d79b5597..8d7303fc13bc 100644
@@ -182,7 +182,6 @@ static int bbc_beep_probe(struct platform_device *op)
 {
 	struct sparcspkr_state *state;
 	struct bbc_beep_info *info;
-	struct device_node *dp;
 	int err;
 
 	state = devm_kzalloc(&op->dev, sizeof(*state), GFP_KERNEL);
@@ -193,13 +192,12 @@ static int bbc_beep_probe(struct platform_device *op)
 	state->event = bbc_spkr_event;
 	spin_lock_init(&state->lock);
 
-	dp = of_find_node_by_path("/");
+	struct device_node *dp __free(device_node) = of_find_node_by_path("/");
 	if (!dp)
 		return -ENODEV;
 
 	info = &state->u.bbc;
 	info->clock_freq = of_getintprop_default(dp, "clock-frequency", 0);
-	of_node_put(dp);
 	if (!info->clock_freq)
 		return -ENODEV;
 
Signature · 1 line
2.43.0
Reviewed-by on v3 2/2
#3 lore raw

Re: [PATCH v3 2/2] Input: sparcspkr - use cleanup facility for device_node

Quoted 6 lines · Javier Carrasco

Use the 'free(device_node)' macro to simplify the code by automatically freeing the device node, which removes the need for explicit calls to 'of_node_put()'.

Not sure why I received this patch, but given my recent experience with the cleanup stuff,

Quoted 35 lines
---
 drivers/input/misc/sparcspkr.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/input/misc/sparcspkr.c +1 -4
index e484d79b5597..8d7303fc13bc 100644
@@ -182,7 +182,6 @@ static int bbc_beep_probe(struct platform_device *op)
 {
        struct sparcspkr_state *state;
        struct bbc_beep_info *info;
-       struct device_node *dp;
        int err;
 
        state = devm_kzalloc(&op->dev, sizeof(*state), GFP_KERNEL);
@@ -193,13 +192,12 @@ static int bbc_beep_probe(struct platform_device *op)
        state->event = bbc_spkr_event;
        spin_lock_init(&state->lock);
 
-       dp = of_find_node_by_path("/");
+       struct device_node *dp __free(device_node) = of_find_node_by_path("/");
        if (!dp)
                return -ENODEV;
 
        info = &state->u.bbc;
        info->clock_freq = of_getintprop_default(dp, "clock-frequency", 0);
-       of_node_put(dp);
        if (!info->clock_freq)
                return -ENODEV;
 
 
--

2.43.0

maintainer Applied
#4 lore raw
In reply to Javier Carrasco
Quoted 6 lines · Javier Carrasco

The first patch of the series provides device managed memory, which simplifies bbc_bee_probe() and removes the need for the goto instructions That also prepares the function for the second patch where the cleanup attribute is used for 'dp'.

Applied the series, thank you.

Signature · 1 line
Dmitry