On Tue, Jan 03, 2023 at 11:33, Jeff LaBundy <jeff@xxxxxxxxxxx> wrote:
> Each call to device/fwnode_get_named_child_node() must be matched
> with a call to fwnode_handle_put() once the corresponding node is
> no longer in use. This ensures a reference count remains balanced
> in the case of dynamic device tree support.
>
> Currently, the driver never calls fwnode_handle_put(); this patch
> adds the missing calls. Because fwnode_handle_put() does not take
> a const *fwnode_handle, the const qualifier is removed across all
> corresponding *fwnode_handle instances.
>
> To facilitate this change, the logic that parses event thresholds
> is collapsed in order to limit the number of locations from which
> fwnode_handle_put() must be called.
>
> Fixes: f1d2809de97a ("Input: Add support for Azoteq IQS626A")
> Signed-off-by: Jeff LaBundy <jeff@xxxxxxxxxxx>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@xxxxxxxxxxxx>
> ---
> drivers/input/misc/iqs626a.c | 87 +++++++++++++++++-------------------
> 1 file changed, 42 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/input/misc/iqs626a.c b/drivers/input/misc/iqs626a.c
> index 23b5dd9552dc..060efb75f8cf 100644
> --- a/drivers/input/misc/iqs626a.c
> +++ b/drivers/input/misc/iqs626a.c
> @@ -458,12 +458,11 @@ struct iqs626_private {
>
> static noinline_for_stack int
> iqs626_parse_events(struct iqs626_private *iqs626,
> - const struct fwnode_handle *ch_node,
> - enum iqs626_ch_id ch_id)
> + struct fwnode_handle *ch_node, enum iqs626_ch_id ch_id)
> {
> struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
> struct i2c_client *client = iqs626->client;
> - const struct fwnode_handle *ev_node;
> + struct fwnode_handle *ev_node;
> const char *ev_name;
> u8 *thresh, *hyst;
> unsigned int thresh_tp[IQS626_NUM_CH_TP_3];
> @@ -509,7 +508,7 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> * Trackpad touch events are simply described under the
> * trackpad child node.
> */
> - ev_node = ch_node;
> + ev_node = fwnode_handle_get(ch_node);
> } else {
> ev_name = iqs626_events[i].name;
> ev_node = fwnode_get_named_child_node(ch_node, ev_name);
> @@ -533,6 +532,7 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> dev_err(&client->dev,
> "Invalid input type: %u\n",
> val);
> + fwnode_handle_put(ev_node);
> return -EINVAL;
> }
>
> @@ -547,6 +547,7 @@ 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;
> }
>
> @@ -561,29 +562,12 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> }
> }
>
> - if (ch_id != IQS626_CH_TP_2 && ch_id != IQS626_CH_TP_3 &&
> - !fwnode_property_read_u32(ev_node, "azoteq,thresh", &val)) {
> - if (val > IQS626_CHx_THRESH_MAX) {
> - dev_err(&client->dev,
> - "Invalid %s channel threshold: %u\n",
> - fwnode_get_name(ch_node), val);
> - return -EINVAL;
> - }
> -
> - if (ch_id == IQS626_CH_HALL)
> - *thresh = val;
> - else
> - *(thresh + iqs626_events[i].th_offs) = val;
> -
> - continue;
> - }
> -
> - if (!fwnode_property_present(ev_node, "azoteq,thresh"))
> - continue;
> -
> error = fwnode_property_read_u32_array(ev_node, "azoteq,thresh",
> thresh_tp, num_ch);
> - if (error) {
> + fwnode_handle_put(ev_node);
> + if (error == -EINVAL) {
> + continue;
> + } else if (error) {
> dev_err(&client->dev,
> "Failed to read %s channel thresholds: %d\n",
> fwnode_get_name(ch_node), error);
> @@ -598,6 +582,20 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> return -EINVAL;
> }
>
> + switch (ch_id) {
> + case IQS626_CH_TP_2:
> + case IQS626_CH_TP_3:
> + break;
> +
> + case IQS626_CH_HALL:
> + *thresh = thresh_tp[j];
> + continue;
> +
> + default:
> + thresh[iqs626_events[i].th_offs] = thresh_tp[j];
> + continue;
> + }
> +
> sys_reg->tp_grp_reg.ch_reg_tp[j].thresh = thresh_tp[j];
> }
> }
> @@ -607,8 +605,7 @@ iqs626_parse_events(struct iqs626_private *iqs626,
>
> static noinline_for_stack int
> iqs626_parse_ati_target(struct iqs626_private *iqs626,
> - const struct fwnode_handle *ch_node,
> - enum iqs626_ch_id ch_id)
> + struct fwnode_handle *ch_node, enum iqs626_ch_id ch_id)
> {
> struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
> struct i2c_client *client = iqs626->client;
> @@ -716,7 +713,7 @@ iqs626_parse_ati_target(struct iqs626_private *iqs626,
> }
>
> static int iqs626_parse_pins(struct iqs626_private *iqs626,
> - const struct fwnode_handle *ch_node,
> + struct fwnode_handle *ch_node,
> const char *propname, u8 *enable)
> {
> struct i2c_client *client = iqs626->client;
> @@ -764,7 +761,7 @@ static int iqs626_parse_pins(struct iqs626_private *iqs626,
> }
>
> static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
> - const struct fwnode_handle *ch_node)
> + struct fwnode_handle *ch_node)
> {
> struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
> struct i2c_client *client = iqs626->client;
> @@ -889,8 +886,7 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
>
> static noinline_for_stack int
> iqs626_parse_channel(struct iqs626_private *iqs626,
> - const struct fwnode_handle *ch_node,
> - enum iqs626_ch_id ch_id)
> + struct fwnode_handle *ch_node, enum iqs626_ch_id ch_id)
> {
> struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
> struct i2c_client *client = iqs626->client;
> @@ -924,6 +920,20 @@ iqs626_parse_channel(struct iqs626_private *iqs626,
> return -EINVAL;
> }
>
> + error = iqs626_parse_ati_target(iqs626, ch_node, ch_id);
> + if (error)
> + return error;
> +
> + error = iqs626_parse_events(iqs626, ch_node, ch_id);
> + if (error)
> + return error;
> +
> + if (!fwnode_property_present(ch_node, "azoteq,ati-exclude"))
> + sys_reg->redo_ati |= iqs626_channels[ch_id].active;
> +
> + if (!fwnode_property_present(ch_node, "azoteq,reseed-disable"))
> + sys_reg->reseed |= iqs626_channels[ch_id].active;
> +
> *engine |= IQS626_CHx_ENG_0_MEAS_CAP_SIZE;
> if (fwnode_property_present(ch_node, "azoteq,meas-cap-decrease"))
> *engine &= ~IQS626_CHx_ENG_0_MEAS_CAP_SIZE;
> @@ -1378,23 +1388,10 @@ static int iqs626_parse_prop(struct iqs626_private *iqs626)
> continue;
>
> error = iqs626_parse_channel(iqs626, ch_node, i);
> + fwnode_handle_put(ch_node);
> if (error)
> return error;
>
> - error = iqs626_parse_ati_target(iqs626, ch_node, i);
> - if (error)
> - return error;
> -
> - error = iqs626_parse_events(iqs626, ch_node, i);
> - if (error)
> - return error;
> -
> - if (!fwnode_property_present(ch_node, "azoteq,ati-exclude"))
> - sys_reg->redo_ati |= iqs626_channels[i].active;
> -
> - if (!fwnode_property_present(ch_node, "azoteq,reseed-disable"))
> - sys_reg->reseed |= iqs626_channels[i].active;
> -
> sys_reg->active |= iqs626_channels[i].active;
> }
>
> --
> 2.34.1
