Hi Markus, On Mon, Mar 04, 2024 at 10:55:11AM +0100, Markus Elfring wrote: > From: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx> > Date: Mon, 4 Mar 2024 10:30:52 +0100 > > Scope-based resource management became supported also for this software > area by contributions of Jonathan Cameron on 2024-02-17. > > device property: Add cleanup.h based fwnode_handle_put() scope based cleanup. > https://lore.kernel.org/r/20240217164249.921878-3-jic23@xxxxxxxxxx > > > * Thus use the attribute “__free(fwnode_handle)”. > > * Reduce the scope for the local variable “ev_node” into a for loop. > > Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx> > --- > > v2: > An other cleanup technique was applied as requested by Dmitry Torokhov. > > > drivers/input/misc/iqs269a.c | 73 ++++++++++++++++++------------------ > 1 file changed, 37 insertions(+), 36 deletions(-) > > diff --git a/drivers/input/misc/iqs269a.c b/drivers/input/misc/iqs269a.c > index cd14ff9f57cf..9caee936927b 100644 > --- a/drivers/input/misc/iqs269a.c > +++ b/drivers/input/misc/iqs269a.c > @@ -557,7 +557,6 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269, > const struct fwnode_handle *ch_node) > { > struct i2c_client *client = iqs269->client; > - struct fwnode_handle *ev_node; > struct iqs269_ch_reg *ch_reg; > u16 engine_a, engine_b; > unsigned int reg, val; > @@ -734,47 +733,49 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269, > } > > for (i = 0; i < ARRAY_SIZE(iqs269_events); i++) { > - ev_node = fwnode_get_named_child_node(ch_node, > - iqs269_events[i].name); > - if (!ev_node) > - continue; > - > - if (!fwnode_property_read_u32(ev_node, "azoteq,thresh", &val)) { > - if (val > IQS269_CHx_THRESH_MAX) { > - dev_err(&client->dev, > - "Invalid channel %u threshold: %u\n", > - reg, val); > - fwnode_handle_put(ev_node); > - return -EINVAL; > + { > + struct fwnode_handle *ev_node __free(fwnode_handle) > + = fwnode_get_named_child_node(ch_node, > + iqs269_events[i].name); > + > + if (!ev_node) > + continue; > + > + if (!fwnode_property_read_u32(ev_node, "azoteq,thresh", &val)) { > + if (val > IQS269_CHx_THRESH_MAX) { > + dev_err(&client->dev, > + "Invalid channel %u threshold: %u\n", > + reg, val); > + return -EINVAL; > + } > + > + ch_reg->thresh[iqs269_events[i].th_offs] = val; I may just be a curmudgeon, but this is another NAK for me. The dummy curly braces and extra indentation make the code difficult to understand, and this simply does not seem like a natural way to write a driver. Just to remove 2-3 calls to fwnode_handle_put()? > } > > - ch_reg->thresh[iqs269_events[i].th_offs] = val; > - } > - > - if (!fwnode_property_read_u32(ev_node, "azoteq,hyst", &val)) { > - u8 *hyst = &ch_reg->hyst; > - > - if (val > IQS269_CHx_HYST_MAX) { > - dev_err(&client->dev, > - "Invalid channel %u hysteresis: %u\n", > - reg, val); > - fwnode_handle_put(ev_node); > - return -EINVAL; > + if (!fwnode_property_read_u32(ev_node, "azoteq,hyst", &val)) { > + u8 *hyst = &ch_reg->hyst; > + > + if (val > IQS269_CHx_HYST_MAX) { > + dev_err(&client->dev, > + "Invalid channel %u hysteresis: %u\n", > + reg, val); > + return -EINVAL; > + } > + > + if (i == IQS269_EVENT_DEEP_DN || > + i == IQS269_EVENT_DEEP_UP) { > + *hyst &= ~IQS269_CHx_HYST_DEEP_MASK; > + *hyst |= (val << IQS269_CHx_HYST_DEEP_SHIFT); > + } else if (i == IQS269_EVENT_TOUCH_DN || > + i == IQS269_EVENT_TOUCH_UP) { > + *hyst &= ~IQS269_CHx_HYST_TOUCH_MASK; > + *hyst |= val; > + } > } > > - if (i == IQS269_EVENT_DEEP_DN || > - i == IQS269_EVENT_DEEP_UP) { > - *hyst &= ~IQS269_CHx_HYST_DEEP_MASK; > - *hyst |= (val << IQS269_CHx_HYST_DEEP_SHIFT); > - } else if (i == IQS269_EVENT_TOUCH_DN || > - i == IQS269_EVENT_TOUCH_UP) { > - *hyst &= ~IQS269_CHx_HYST_TOUCH_MASK; > - *hyst |= val; > - } > + error = fwnode_property_read_u32(ev_node, "linux,code", &val); > } > > - error = fwnode_property_read_u32(ev_node, "linux,code", &val); > - fwnode_handle_put(ev_node); > if (error == -EINVAL) { > continue; > } else if (error) { > -- > 2.44.0 > Kind regards, Jeff LaBundy
