On Thu, Sep 08, 2022 at 09:08:09PM -0500, Jeff LaBundy wrote: > Hi Dmitry, > > On Thu, Sep 08, 2022 at 02:21:13PM -0700, Dmitry Torokhov wrote: > > On Thu, Sep 08, 2022 at 08:15:39AM -0500, Jeff LaBundy wrote: > > > Nonzero return values of several calls to fwnode_property_read_u32() > > > are silenty ignored, leaving no way to know that the properties were > > > not applied in the event of an error. > > > > > > To solve this problem, follow the design pattern used throughout the > > > rest of the driver by first checking if the property is present, and > > > then evaluating the return value of fwnode_property_read_u32(). > > > > > > Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C") > > > Signed-off-by: Jeff LaBundy <jeff@xxxxxxxxxxx> > > > --- > > > drivers/input/misc/iqs7222.c | 65 ++++++++++++++++++++++++++++++------ > > > 1 file changed, 55 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/input/misc/iqs7222.c b/drivers/input/misc/iqs7222.c > > > index 04c1050d845c..fdade24caa8a 100644 > > > --- a/drivers/input/misc/iqs7222.c > > > +++ b/drivers/input/misc/iqs7222.c > > > @@ -1819,8 +1819,17 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index) > > > chan_setup[0] |= IQS7222_CHAN_SETUP_0_REF_MODE_FOLLOW; > > > chan_setup[4] = val * 42 + 1048; > > > > > > - if (!fwnode_property_read_u32(chan_node, "azoteq,ref-weight", > > > - &val)) { > > > + if (fwnode_property_present(chan_node, "azoteq,ref-weight")) { > > > + error = fwnode_property_read_u32(chan_node, > > > + "azoteq,ref-weight", > > > + &val); > > > > fwnode_property_read_u32() returns EINVAL if property is missing, so > > maybe have: > > > > error = fwnode_property_read_u32(chan_node, > > "azoteq,ref-weight", &val); > > if (!error) { > > ... > > } else { > > if (error != -EINVAL) { > > dev_err(&client->dev, > > "Failed to read %s reference weight: %d\n", > > fwnode_get_name(chan_node), error); > > goto put_chan_node; > > } > > } > > > > to avoid double calls into property handling code? > > That's a better idea; I can fold this into the helper functions proposed > for the previous patch too. We might be talking about different helpers. I had in mind: static int __iqs7222_parse_cycle(...) { ... } static int iqs7222_parse_cycle(...) { ... int retval = 0; error = iqs7222_parse_props(iqs7222, &cycle_node, cycle_index, IQS7222_REG_GRP_CYCLE, IQS7222_REG_KEY_NONE); if (error) return error; if (cycle_node) { retval = __iqs7222_parse_cycle(...); fwnode_put(cycle_node); } return retval; } so that we drop the node from one place. Thanks. -- Dmitry