Use __free(fwnode_handle) cleanup facility to ensure that references to
acquired fwnodes are dropped at appropriate times automatically.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/iqs626a.c | 22 ++++++----------------
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/drivers/input/misc/iqs626a.c b/drivers/input/misc/iqs626a.c
index 0dab54d3a060..7a6e6927f331 100644
--- a/drivers/input/misc/iqs626a.c
+++ b/drivers/input/misc/iqs626a.c
@@ -462,7 +462,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
{
struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
struct i2c_client *client = iqs626->client;
- struct fwnode_handle *ev_node;
const char *ev_name;
u8 *thresh, *hyst;
unsigned int val;
@@ -501,6 +500,7 @@ iqs626_parse_events(struct iqs626_private *iqs626,
if (!iqs626_channels[ch_id].events[i])
continue;
+ struct fwnode_handle *ev_node __free(fwnode_handle) = NULL;
if (ch_id == IQS626_CH_TP_2 || ch_id == IQS626_CH_TP_3) {
/*
* Trackpad touch events are simply described under the
@@ -530,7 +530,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
dev_err(&client->dev,
"Invalid input type: %u\n",
val);
- fwnode_handle_put(ev_node);
return -EINVAL;
}
@@ -545,7 +544,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
dev_err(&client->dev,
"Invalid %s channel hysteresis: %u\n",
fwnode_get_name(ch_node), val);
- fwnode_handle_put(ev_node);
return -EINVAL;
}
@@ -566,7 +564,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
dev_err(&client->dev,
"Invalid %s channel threshold: %u\n",
fwnode_get_name(ch_node), val);
- fwnode_handle_put(ev_node);
return -EINVAL;
}
@@ -575,8 +572,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
else
*(thresh + iqs626_events[i].th_offs) = val;
}
-
- fwnode_handle_put(ev_node);
}
return 0;
@@ -774,12 +769,12 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
for (i = 0; i < iqs626_channels[ch_id].num_ch; i++) {
u8 *ati_base = &sys_reg->tp_grp_reg.ch_reg_tp[i].ati_base;
u8 *thresh = &sys_reg->tp_grp_reg.ch_reg_tp[i].thresh;
- struct fwnode_handle *tc_node;
char tc_name[10];
snprintf(tc_name, sizeof(tc_name), "channel-%d", i);
- tc_node = fwnode_get_named_child_node(ch_node, tc_name);
+ struct fwnode_handle *tc_node __free(fwnode_handle) =
+ fwnode_get_named_child_node(ch_node, tc_name);
if (!tc_node)
continue;
@@ -790,7 +785,6 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
dev_err(&client->dev,
"Invalid %s %s ATI base: %u\n",
fwnode_get_name(ch_node), tc_name, val);
- fwnode_handle_put(tc_node);
return -EINVAL;
}
@@ -803,14 +797,11 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
dev_err(&client->dev,
"Invalid %s %s threshold: %u\n",
fwnode_get_name(ch_node), tc_name, val);
- fwnode_handle_put(tc_node);
return -EINVAL;
}
*thresh = val;
}
-
- fwnode_handle_put(tc_node);
}
if (!fwnode_property_present(ch_node, "linux,keycodes"))
@@ -1233,7 +1224,6 @@ static int iqs626_parse_prop(struct iqs626_private *iqs626)
{
struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
struct i2c_client *client = iqs626->client;
- struct fwnode_handle *ch_node;
unsigned int val;
int error, i;
u16 general;
@@ -1375,13 +1365,13 @@ static int iqs626_parse_prop(struct iqs626_private *iqs626)
sys_reg->active = 0;
for (i = 0; i < ARRAY_SIZE(iqs626_channels); i++) {
- ch_node = device_get_named_child_node(&client->dev,
- iqs626_channels[i].name);
+ struct fwnode_handle *ch_node __free(fwnode_handle) =
+ device_get_named_child_node(&client->dev,
+ iqs626_channels[i].name);
if (!ch_node)
continue;
error = iqs626_parse_channel(iqs626, ch_node, i);
- fwnode_handle_put(ch_node);
if (error)
return error;
--
2.46.0.469.g59c65b2a67-googHi Dmitry,
On Tue, Sep 03, 2024 at 09:48:13PM -0700, Dmitry Torokhov wrote:
> Use __free(fwnode_handle) cleanup facility to ensure that references to
> acquired fwnodes are dropped at appropriate times automatically.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/misc/iqs626a.c | 22 ++++++----------------
> 1 file changed, 6 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/input/misc/iqs626a.c b/drivers/input/misc/iqs626a.c
> index 0dab54d3a060..7a6e6927f331 100644
> --- a/drivers/input/misc/iqs626a.c
> +++ b/drivers/input/misc/iqs626a.c
> @@ -462,7 +462,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> {
> struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
> struct i2c_client *client = iqs626->client;
> - struct fwnode_handle *ev_node;
> const char *ev_name;
> u8 *thresh, *hyst;
> unsigned int val;
> @@ -501,6 +500,7 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> if (!iqs626_channels[ch_id].events[i])
> continue;
>
> + struct fwnode_handle *ev_node __free(fwnode_handle) = NULL;
This seems to deviate from what I understand to be a more conventional
style of declaring variables at the top of the scope, and separate from
actual code, like below:
for (i = 0; i < ARRAY_SIZE(iqs626_events); i++) {
struct fwnode_handle *ev_node __free(fwnode_handle);
if (!iqs626_channels[ch_id].events[i])
continue;
I also did not see any reason to explicitly declare the variable as NULL;
let me know in case I have misunderstood.
> if (ch_id == IQS626_CH_TP_2 || ch_id == IQS626_CH_TP_3) {
> /*
> * Trackpad touch events are simply described under the
> @@ -530,7 +530,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> dev_err(&client->dev,
> "Invalid input type: %u\n",
> val);
> - fwnode_handle_put(ev_node);
> return -EINVAL;
> }
>
> @@ -545,7 +544,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> dev_err(&client->dev,
> "Invalid %s channel hysteresis: %u\n",
> fwnode_get_name(ch_node), val);
> - fwnode_handle_put(ev_node);
> return -EINVAL;
> }
>
> @@ -566,7 +564,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> dev_err(&client->dev,
> "Invalid %s channel threshold: %u\n",
> fwnode_get_name(ch_node), val);
> - fwnode_handle_put(ev_node);
> return -EINVAL;
> }
>
> @@ -575,8 +572,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> else
> *(thresh + iqs626_events[i].th_offs) = val;
> }
> -
> - fwnode_handle_put(ev_node);
> }
>
> return 0;
> @@ -774,12 +769,12 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
> for (i = 0; i < iqs626_channels[ch_id].num_ch; i++) {
> u8 *ati_base = &sys_reg->tp_grp_reg.ch_reg_tp[i].ati_base;
> u8 *thresh = &sys_reg->tp_grp_reg.ch_reg_tp[i].thresh;
> - struct fwnode_handle *tc_node;
> char tc_name[10];
>
> snprintf(tc_name, sizeof(tc_name), "channel-%d", i);
>
> - tc_node = fwnode_get_named_child_node(ch_node, tc_name);
> + struct fwnode_handle *tc_node __free(fwnode_handle) =
> + fwnode_get_named_child_node(ch_node, tc_name);
Same here.
> if (!tc_node)
> continue;
>
> @@ -790,7 +785,6 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
> dev_err(&client->dev,
> "Invalid %s %s ATI base: %u\n",
> fwnode_get_name(ch_node), tc_name, val);
> - fwnode_handle_put(tc_node);
> return -EINVAL;
> }
>
> @@ -803,14 +797,11 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
> dev_err(&client->dev,
> "Invalid %s %s threshold: %u\n",
> fwnode_get_name(ch_node), tc_name, val);
> - fwnode_handle_put(tc_node);
> return -EINVAL;
> }
>
> *thresh = val;
> }
> -
> - fwnode_handle_put(tc_node);
> }
>
> if (!fwnode_property_present(ch_node, "linux,keycodes"))
> @@ -1233,7 +1224,6 @@ static int iqs626_parse_prop(struct iqs626_private *iqs626)
> {
> struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
> struct i2c_client *client = iqs626->client;
> - struct fwnode_handle *ch_node;
> unsigned int val;
> int error, i;
> u16 general;
> @@ -1375,13 +1365,13 @@ static int iqs626_parse_prop(struct iqs626_private *iqs626)
> sys_reg->active = 0;
>
> for (i = 0; i < ARRAY_SIZE(iqs626_channels); i++) {
> - ch_node = device_get_named_child_node(&client->dev,
> - iqs626_channels[i].name);
> + struct fwnode_handle *ch_node __free(fwnode_handle) =
> + device_get_named_child_node(&client->dev,
> + iqs626_channels[i].name);
> if (!ch_node)
> continue;
>
> error = iqs626_parse_channel(iqs626, ch_node, i);
> - fwnode_handle_put(ch_node);
> if (error)
> return error;
>
> --
> 2.46.0.469.g59c65b2a67-goog
>
Kind regards,
Jeff LaBundy
Hi Jeff,
On Sun, Sep 08, 2024 at 07:02:41PM -0500, Jeff LaBundy wrote:
> Hi Dmitry,
>
> On Tue, Sep 03, 2024 at 09:48:13PM -0700, Dmitry Torokhov wrote:
> > Use __free(fwnode_handle) cleanup facility to ensure that references to
> > acquired fwnodes are dropped at appropriate times automatically.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > drivers/input/misc/iqs626a.c | 22 ++++++----------------
> > 1 file changed, 6 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/input/misc/iqs626a.c b/drivers/input/misc/iqs626a.c
> > index 0dab54d3a060..7a6e6927f331 100644
> > --- a/drivers/input/misc/iqs626a.c
> > +++ b/drivers/input/misc/iqs626a.c
> > @@ -462,7 +462,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > {
> > struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
> > struct i2c_client *client = iqs626->client;
> > - struct fwnode_handle *ev_node;
> > const char *ev_name;
> > u8 *thresh, *hyst;
> > unsigned int val;
> > @@ -501,6 +500,7 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > if (!iqs626_channels[ch_id].events[i])
> > continue;
> >
> > + struct fwnode_handle *ev_node __free(fwnode_handle) = NULL;
>
> This seems to deviate from what I understand to be a more conventional
> style of declaring variables at the top of the scope, and separate from
> actual code, like below:
This is follows Linus' guidance on combining declaration and
initialization for variables using __free() cleanup annotations (where
possible). These annotations are the reason for dropping
-Wdeclaration-after-statement from our makefiles. See b5ec6fd286df
("kbuild: Drop -Wdeclaration-after-statement") and discussion in
https://lore.kernel.org/all/CAHk-=wi-RyoUhbChiVaJZoZXheAwnJ7OO=Gxe85BkPAd93TwDA@mail.gmail.com
>
>
> for (i = 0; i < ARRAY_SIZE(iqs626_events); i++) {
> struct fwnode_handle *ev_node __free(fwnode_handle);
>
> if (!iqs626_channels[ch_id].events[i])
> continue;
This will result in attempt to "put" a garbage pointer if we follow
"continue" code path. In general __free() pointers have to be
initialized, either to NULL or to a valid object (assuming that cleanup
function can deal with NULLs).
>
> I also did not see any reason to explicitly declare the variable as NULL;
> let me know in case I have misunderstood.
See the above. Yes, in this particular case it will get a value in both
branches, but I feel it is too fragile and may get messed up if someone
refactors code.
>
> > if (ch_id == IQS626_CH_TP_2 || ch_id == IQS626_CH_TP_3) {
> > /*
> > * Trackpad touch events are simply described under the
> > @@ -530,7 +530,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > dev_err(&client->dev,
> > "Invalid input type: %u\n",
> > val);
> > - fwnode_handle_put(ev_node);
> > return -EINVAL;
> > }
> >
> > @@ -545,7 +544,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > dev_err(&client->dev,
> > "Invalid %s channel hysteresis: %u\n",
> > fwnode_get_name(ch_node), val);
> > - fwnode_handle_put(ev_node);
> > return -EINVAL;
> > }
> >
> > @@ -566,7 +564,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > dev_err(&client->dev,
> > "Invalid %s channel threshold: %u\n",
> > fwnode_get_name(ch_node), val);
> > - fwnode_handle_put(ev_node);
> > return -EINVAL;
> > }
> >
> > @@ -575,8 +572,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > else
> > *(thresh + iqs626_events[i].th_offs) = val;
> > }
> > -
> > - fwnode_handle_put(ev_node);
> > }
> >
> > return 0;
> > @@ -774,12 +769,12 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
> > for (i = 0; i < iqs626_channels[ch_id].num_ch; i++) {
> > u8 *ati_base = &sys_reg->tp_grp_reg.ch_reg_tp[i].ati_base;
> > u8 *thresh = &sys_reg->tp_grp_reg.ch_reg_tp[i].thresh;
> > - struct fwnode_handle *tc_node;
> > char tc_name[10];
> >
> > snprintf(tc_name, sizeof(tc_name), "channel-%d", i);
> >
> > - tc_node = fwnode_get_named_child_node(ch_node, tc_name);
> > + struct fwnode_handle *tc_node __free(fwnode_handle) =
> > + fwnode_get_named_child_node(ch_node, tc_name);
>
> Same here.
Yes, combining declaration and initialization is preferred for such
pointers.
Thanks.
--
Dmitry
Hi Dmitry,
On Sun, Sep 08, 2024 at 06:31:26PM -0700, Dmitry Torokhov wrote:
> Hi Jeff,
>
> On Sun, Sep 08, 2024 at 07:02:41PM -0500, Jeff LaBundy wrote:
> > Hi Dmitry,
> >
> > On Tue, Sep 03, 2024 at 09:48:13PM -0700, Dmitry Torokhov wrote:
> > > Use __free(fwnode_handle) cleanup facility to ensure that references to
> > > acquired fwnodes are dropped at appropriate times automatically.
> > >
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > ---
> > > drivers/input/misc/iqs626a.c | 22 ++++++----------------
> > > 1 file changed, 6 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/input/misc/iqs626a.c b/drivers/input/misc/iqs626a.c
> > > index 0dab54d3a060..7a6e6927f331 100644
> > > --- a/drivers/input/misc/iqs626a.c
> > > +++ b/drivers/input/misc/iqs626a.c
> > > @@ -462,7 +462,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > > {
> > > struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
> > > struct i2c_client *client = iqs626->client;
> > > - struct fwnode_handle *ev_node;
> > > const char *ev_name;
> > > u8 *thresh, *hyst;
> > > unsigned int val;
> > > @@ -501,6 +500,7 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > > if (!iqs626_channels[ch_id].events[i])
> > > continue;
> > >
> > > + struct fwnode_handle *ev_node __free(fwnode_handle) = NULL;
> >
> > This seems to deviate from what I understand to be a more conventional
> > style of declaring variables at the top of the scope, and separate from
> > actual code, like below:
>
> This is follows Linus' guidance on combining declaration and
> initialization for variables using __free() cleanup annotations (where
> possible). These annotations are the reason for dropping
> -Wdeclaration-after-statement from our makefiles. See b5ec6fd286df
> ("kbuild: Drop -Wdeclaration-after-statement") and discussion in
> https://lore.kernel.org/all/CAHk-=wi-RyoUhbChiVaJZoZXheAwnJ7OO=Gxe85BkPAd93TwDA@mail.gmail.com
Understood; thank you for the reference.
>
> >
> >
> > for (i = 0; i < ARRAY_SIZE(iqs626_events); i++) {
> > struct fwnode_handle *ev_node __free(fwnode_handle);
> >
> > if (!iqs626_channels[ch_id].events[i])
> > continue;
>
> This will result in attempt to "put" a garbage pointer if we follow
> "continue" code path. In general __free() pointers have to be
> initialized, either to NULL or to a valid object (assuming that cleanup
> function can deal with NULLs).
Great catch; I missed the fact that fwnode_handle_put() is implicitly
being called in all exit paths now.
>
> >
> > I also did not see any reason to explicitly declare the variable as NULL;
> > let me know in case I have misunderstood.
>
> See the above. Yes, in this particular case it will get a value in both
> branches, but I feel it is too fragile and may get messed up if someone
> refactors code.
Based on the above point, I agree with your approach.
>
> >
> > > if (ch_id == IQS626_CH_TP_2 || ch_id == IQS626_CH_TP_3) {
> > > /*
> > > * Trackpad touch events are simply described under the
> > > @@ -530,7 +530,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > > dev_err(&client->dev,
> > > "Invalid input type: %u\n",
> > > val);
> > > - fwnode_handle_put(ev_node);
> > > return -EINVAL;
> > > }
> > >
> > > @@ -545,7 +544,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > > dev_err(&client->dev,
> > > "Invalid %s channel hysteresis: %u\n",
> > > fwnode_get_name(ch_node), val);
> > > - fwnode_handle_put(ev_node);
> > > return -EINVAL;
> > > }
> > >
> > > @@ -566,7 +564,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > > dev_err(&client->dev,
> > > "Invalid %s channel threshold: %u\n",
> > > fwnode_get_name(ch_node), val);
> > > - fwnode_handle_put(ev_node);
> > > return -EINVAL;
> > > }
> > >
> > > @@ -575,8 +572,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> > > else
> > > *(thresh + iqs626_events[i].th_offs) = val;
> > > }
> > > -
> > > - fwnode_handle_put(ev_node);
> > > }
> > >
> > > return 0;
> > > @@ -774,12 +769,12 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
> > > for (i = 0; i < iqs626_channels[ch_id].num_ch; i++) {
> > > u8 *ati_base = &sys_reg->tp_grp_reg.ch_reg_tp[i].ati_base;
> > > u8 *thresh = &sys_reg->tp_grp_reg.ch_reg_tp[i].thresh;
> > > - struct fwnode_handle *tc_node;
> > > char tc_name[10];
> > >
> > > snprintf(tc_name, sizeof(tc_name), "channel-%d", i);
> > >
> > > - tc_node = fwnode_get_named_child_node(ch_node, tc_name);
> > > + struct fwnode_handle *tc_node __free(fwnode_handle) =
> > > + fwnode_get_named_child_node(ch_node, tc_name);
> >
> > Same here.
>
> Yes, combining declaration and initialization is preferred for such
> pointers.
ACK. Please feel free to add:
Reviewed-by: Jeff LaBundy <jeff@labundy.com>
>
> Thanks.
>
> --
> Dmitry
Thank you for the discussion!
Kind regards,
Jeff LaBundy
On 04/09/2024 06:48, Dmitry Torokhov wrote:
> Use __free(fwnode_handle) cleanup facility to ensure that references to
> acquired fwnodes are dropped at appropriate times automatically.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/misc/iqs626a.c | 22 ++++++----------------
> 1 file changed, 6 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/input/misc/iqs626a.c b/drivers/input/misc/iqs626a.c
> index 0dab54d3a060..7a6e6927f331 100644
> --- a/drivers/input/misc/iqs626a.c
> +++ b/drivers/input/misc/iqs626a.c
> @@ -462,7 +462,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> {
> struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
> struct i2c_client *client = iqs626->client;
> - struct fwnode_handle *ev_node;
> const char *ev_name;
> u8 *thresh, *hyst;
> unsigned int val;
> @@ -501,6 +500,7 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> if (!iqs626_channels[ch_id].events[i])
> continue;
>
> + struct fwnode_handle *ev_node __free(fwnode_handle) = NULL;
> if (ch_id == IQS626_CH_TP_2 || ch_id == IQS626_CH_TP_3) {
> /*
> * Trackpad touch events are simply described under the
> @@ -530,7 +530,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> dev_err(&client->dev,
> "Invalid input type: %u\n",
> val);
> - fwnode_handle_put(ev_node);
> return -EINVAL;
> }
>
> @@ -545,7 +544,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> dev_err(&client->dev,
> "Invalid %s channel hysteresis: %u\n",
> fwnode_get_name(ch_node), val);
> - fwnode_handle_put(ev_node);
> return -EINVAL;
> }
>
> @@ -566,7 +564,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> dev_err(&client->dev,
> "Invalid %s channel threshold: %u\n",
> fwnode_get_name(ch_node), val);
> - fwnode_handle_put(ev_node);
> return -EINVAL;
> }
>
> @@ -575,8 +572,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> else
> *(thresh + iqs626_events[i].th_offs) = val;
> }
> -
> - fwnode_handle_put(ev_node);
> }
>
> return 0;
> @@ -774,12 +769,12 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
> for (i = 0; i < iqs626_channels[ch_id].num_ch; i++) {
> u8 *ati_base = &sys_reg->tp_grp_reg.ch_reg_tp[i].ati_base;
> u8 *thresh = &sys_reg->tp_grp_reg.ch_reg_tp[i].thresh;
> - struct fwnode_handle *tc_node;
> char tc_name[10];
>
> snprintf(tc_name, sizeof(tc_name), "channel-%d", i);
>
> - tc_node = fwnode_get_named_child_node(ch_node, tc_name);
> + struct fwnode_handle *tc_node __free(fwnode_handle) =
> + fwnode_get_named_child_node(ch_node, tc_name);
> if (!tc_node)
> continue;
>
> @@ -790,7 +785,6 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
> dev_err(&client->dev,
> "Invalid %s %s ATI base: %u\n",
> fwnode_get_name(ch_node), tc_name, val);
> - fwnode_handle_put(tc_node);
> return -EINVAL;
> }
>
> @@ -803,14 +797,11 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
> dev_err(&client->dev,
> "Invalid %s %s threshold: %u\n",
> fwnode_get_name(ch_node), tc_name, val);
> - fwnode_handle_put(tc_node);
> return -EINVAL;
> }
>
> *thresh = val;
> }
> -
> - fwnode_handle_put(tc_node);
> }
>
> if (!fwnode_property_present(ch_node, "linux,keycodes"))
> @@ -1233,7 +1224,6 @@ static int iqs626_parse_prop(struct iqs626_private *iqs626)
> {
> struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
> struct i2c_client *client = iqs626->client;
> - struct fwnode_handle *ch_node;
> unsigned int val;
> int error, i;
> u16 general;
> @@ -1375,13 +1365,13 @@ static int iqs626_parse_prop(struct iqs626_private *iqs626)
> sys_reg->active = 0;
>
> for (i = 0; i < ARRAY_SIZE(iqs626_channels); i++) {
> - ch_node = device_get_named_child_node(&client->dev,
> - iqs626_channels[i].name);
> + struct fwnode_handle *ch_node __free(fwnode_handle) =
> + device_get_named_child_node(&client->dev,
> + iqs626_channels[i].name);
> if (!ch_node)
> continue;
>
> error = iqs626_parse_channel(iqs626, ch_node, i);
> - fwnode_handle_put(ch_node);
> if (error)
> return error;
>
Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
© 2016 - 2025 Red Hat, Inc.