[PATCH] regmap: define cleanup helper for regmap_field

Sander Vanheule posted 1 patch 1 month, 1 week ago
include/linux/regmap.h | 3 +++
1 file changed, 3 insertions(+)
[PATCH] regmap: define cleanup helper for regmap_field
Posted by Sander Vanheule 1 month, 1 week ago
For temporary field allocation, the user has to perform manual cleanup,
or rely on devm_regmap_field_alloc() to (eventually) clean up the
allocated resources when an error occurs.

Add a cleanup helper that takes care of freeing the allocated
regmap_field whenever it goes out of scope.

This can simplify this example:

    struct regmap_field *field = regmap_field_alloc(...);
    if (!field)
        return -ENOMEM;

    int err = regmap_field_write(...);

    regmap_field_free(field);

    return err;

into the shorter:

    struct regmap_field *field __free(regmap_field) = regmap_field_alloc(...);
    if (!field)
        return -ENOMEM;

    return regmap_field_write(...);

Signed-off-by: Sander Vanheule <sander@svanheule.net>
---
The included headers in regmap.h are not sorted, so I just included
cleanup.h at the bottom of the list. Let me know if the headers should
be sorted too, then I can make a preparatory patch.

Best,
Sander
---
 include/linux/regmap.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index caff2240bdab..b5508ef4a973 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -19,6 +19,7 @@
 #include <linux/lockdep.h>
 #include <linux/iopoll.h>
 #include <linux/fwnode.h>
+#include <linux/cleanup.h>
 
 struct module;
 struct clk;
@@ -1460,6 +1461,8 @@ struct regmap_field *regmap_field_alloc(struct regmap *regmap,
 		struct reg_field reg_field);
 void regmap_field_free(struct regmap_field *field);
 
+DEFINE_FREE(regmap_field, struct regmap_field *, if (_T) regmap_field_free(_T))
+
 struct regmap_field *devm_regmap_field_alloc(struct device *dev,
 		struct regmap *regmap, struct reg_field reg_field);
 void devm_regmap_field_free(struct device *dev,	struct regmap_field *field);
-- 
2.53.0
Re: [PATCH] regmap: define cleanup helper for regmap_field
Posted by Mark Brown 1 month, 1 week ago
On Fri, Feb 20, 2026 at 04:36:46PM +0100, Sander Vanheule wrote:

>     struct regmap_field *field = regmap_field_alloc(...);
>     if (!field)
>         return -ENOMEM;
> 
>     int err = regmap_field_write(...);
> 
>     regmap_field_free(field);
> 
>     return err;

That seems like a bit of an odd pattern, I'd expect the fields to live
for the lifetime of the driver rather than a function.  OTOH if people
are doing that then sure.

> The included headers in regmap.h are not sorted, so I just included
> cleanup.h at the bottom of the list. Let me know if the headers should
> be sorted too, then I can make a preparatory patch.

Yes please.
Re: [PATCH] regmap: define cleanup helper for regmap_field
Posted by Sander Vanheule 1 month, 1 week ago
On Fri, 2026-02-20 at 15:43 +0000, Mark Brown wrote:
> On Fri, Feb 20, 2026 at 04:36:46PM +0100, Sander Vanheule wrote:
> 
> >     struct regmap_field *field = regmap_field_alloc(...);
> >     if (!field)
> >         return -ENOMEM;
> > 
> >     int err = regmap_field_write(...);
> > 
> >     regmap_field_free(field);
> > 
> >     return err;
> 
> That seems like a bit of an odd pattern, I'd expect the fields to live
> for the lifetime of the driver rather than a function.  OTOH if people
> are doing that then sure.

That is usually the case yes. I did find myself using devm_regmap_field_alloc()
in a proposed driver [1] to avoid goto's in less trivial logic. I'll update the
example in v2 to show the benefit a bit more.

[1] https://lore.kernel.org/all/20251215175115.135294-5-sander@svanheule.net/

Best,
Sander
Re: [PATCH] regmap: define cleanup helper for regmap_field
Posted by Sander Vanheule 1 month, 1 week ago
Hi Mark,

On Fri, 2026-02-20 at 16:36 +0100, Sander Vanheule wrote:
> For temporary field allocation, the user has to perform manual cleanup,
> or rely on devm_regmap_field_alloc() to (eventually) clean up the
> allocated resources when an error occurs.
> 
> Add a cleanup helper that takes care of freeing the allocated
> regmap_field whenever it goes out of scope.
> 
> This can simplify this example:
> 
>     struct regmap_field *field = regmap_field_alloc(...);
>     if (!field)
>         return -ENOMEM;

The error handling should of course be:

        if (IS_ERR(field))
            return PTR_ERR(field);

I'll wait a bit for other feedback before I send a v2.

Best,
Sander