[PATCH 00/11] Additional fixes for Azoteq IQS7222A/B/C

[PATCH 00/11] Additional fixes for Azoteq IQS7222A/B/C

[Jeff LaBundy]

This series comprises a second round of fixes that result from
continued testing and updated guidance from the vendor.

Jeff LaBundy (11):
  Input: iqs7222 - drop unused device node references
  Input: iqs7222 - report malformed properties
  dt-bindings: input: iqs7222: Correct minimum slider size
  Input: iqs7222 - protect against undefined slider size
  Input: iqs7222 - trim force communication command
  Input: iqs7222 - avoid sending empty SYN_REPORT events
  Input: iqs7222 - set all ULP entry masks by default
  Input: iqs7222 - allow 'linux,code' to be optional
  dt-bindings: input: iqs7222: Allow 'linux,code' to be optional
  dt-bindings: input: iqs7222: Add support for IQS7222A v1.13+
  Input: iqs7222 - add support for IQS7222A v1.13+

 .../bindings/input/azoteq,iqs7222.yaml        |  25 +-
 drivers/input/misc/iqs7222.c                  | 365 ++++++++++++++----
 2 files changed, 303 insertions(+), 87 deletions(-)

-- 
2.25.1

[PATCH 01/11] Input: iqs7222 - drop unused device node references

[Jeff LaBundy]

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.

Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C")
Signed-off-by: Jeff LaBundy <jeff@labundy.com>
---
 drivers/input/misc/iqs7222.c | 134 ++++++++++++++++++++++++-----------
 1 file changed, 91 insertions(+), 43 deletions(-)

diff --git a/drivers/input/misc/iqs7222.c b/drivers/input/misc/iqs7222.c
index b2e8097a2e6d..04c1050d845c 100644
--- a/drivers/input/misc/iqs7222.c
+++ b/drivers/input/misc/iqs7222.c
@@ -1703,7 +1703,7 @@ static int iqs7222_parse_cycle(struct iqs7222_private *iqs7222, int cycle_index)
 				    IQS7222_REG_GRP_CYCLE,
 				    IQS7222_REG_KEY_NONE);
 	if (error)
-		return error;
+		goto put_cycle_node;
 
 	if (!cycle_node)
 		return 0;
@@ -1714,17 +1714,19 @@ static int iqs7222_parse_cycle(struct iqs7222_private *iqs7222, int cycle_index)
 	 * CTx pins (CTx0-8).
 	 */
 	if (!fwnode_property_present(cycle_node, "azoteq,tx-enable"))
-		return 0;
+		goto put_cycle_node;
 
 	count = fwnode_property_count_u32(cycle_node, "azoteq,tx-enable");
 	if (count < 0) {
 		dev_err(&client->dev, "Failed to count %s CTx pins: %d\n",
 			fwnode_get_name(cycle_node), count);
-		return count;
+		error = count;
+		goto put_cycle_node;
 	} else if (count > ARRAY_SIZE(pins)) {
 		dev_err(&client->dev, "Invalid number of %s CTx pins\n",
 			fwnode_get_name(cycle_node));
-		return -EINVAL;
+		error = -EINVAL;
+		goto put_cycle_node;
 	}
 
 	error = fwnode_property_read_u32_array(cycle_node, "azoteq,tx-enable",
@@ -1732,7 +1734,7 @@ static int iqs7222_parse_cycle(struct iqs7222_private *iqs7222, int cycle_index)
 	if (error) {
 		dev_err(&client->dev, "Failed to read %s CTx pins: %d\n",
 			fwnode_get_name(cycle_node), error);
-		return error;
+		goto put_cycle_node;
 	}
 
 	cycle_setup[1] &= ~GENMASK(7 + ARRAY_SIZE(pins) - 1, 7);
@@ -1741,13 +1743,17 @@ static int iqs7222_parse_cycle(struct iqs7222_private *iqs7222, int cycle_index)
 		if (pins[i] > 8) {
 			dev_err(&client->dev, "Invalid %s CTx pin: %u\n",
 				fwnode_get_name(cycle_node), pins[i]);
-			return -EINVAL;
+			error = -EINVAL;
+			goto put_cycle_node;
 		}
 
 		cycle_setup[1] |= BIT(pins[i] + 7);
 	}
 
-	return 0;
+put_cycle_node:
+	fwnode_handle_put(cycle_node);
+
+	return error;
 }
 
 static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
@@ -1766,7 +1772,7 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 				    IQS7222_REG_GRP_CHAN,
 				    IQS7222_REG_KEY_NONE);
 	if (error)
-		return error;
+		goto put_chan_node;
 
 	if (!chan_node)
 		return 0;
@@ -1793,14 +1799,15 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 			dev_err(&client->dev,
 				"Failed to read %s reference channel: %d\n",
 				fwnode_get_name(chan_node), error);
-			return error;
+			goto put_chan_node;
 		}
 
 		if (val >= ext_chan) {
 			dev_err(&client->dev,
 				"Invalid %s reference channel: %u\n",
 				fwnode_get_name(chan_node), val);
-			return -EINVAL;
+			error = -EINVAL;
+			goto put_chan_node;
 		}
 
 		ref_setup = iqs7222->chan_setup[val];
@@ -1818,7 +1825,8 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 				dev_err(&client->dev,
 					"Invalid %s reference weight: %u\n",
 					fwnode_get_name(chan_node), val);
-				return -EINVAL;
+				error = -EINVAL;
+				goto put_chan_node;
 			}
 
 			chan_setup[5] = val;
@@ -1851,12 +1859,14 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 			dev_err(&client->dev,
 				"Failed to count %s CRx pins: %d\n",
 				fwnode_get_name(chan_node), count);
-			return count;
+			error = count;
+			goto put_chan_node;
 		} else if (count > ARRAY_SIZE(pins)) {
 			dev_err(&client->dev,
 				"Invalid number of %s CRx pins\n",
 				fwnode_get_name(chan_node));
-			return -EINVAL;
+			error = -EINVAL;
+			goto put_chan_node;
 		}
 
 		error = fwnode_property_read_u32_array(chan_node,
@@ -1866,7 +1876,7 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 			dev_err(&client->dev,
 				"Failed to read %s CRx pins: %d\n",
 				fwnode_get_name(chan_node), error);
-			return error;
+			goto put_chan_node;
 		}
 
 		chan_setup[0] &= ~GENMASK(4 + ARRAY_SIZE(pins) - 1, 4);
@@ -1878,7 +1888,8 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 				dev_err(&client->dev,
 					"Invalid %s CRx pin: %u\n",
 					fwnode_get_name(chan_node), pins[i]);
-				return -EINVAL;
+				error = -EINVAL;
+				goto put_chan_node;
 			}
 
 			chan_setup[0] |= BIT(pins[i] + 4 - min_crx);
@@ -1897,14 +1908,18 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 		error = iqs7222_parse_props(iqs7222, &event_node, chan_index,
 					    IQS7222_REG_GRP_BTN,
 					    iqs7222_kp_events[i].reg_key);
-		if (error)
-			return error;
+		if (error) {
+			fwnode_handle_put(event_node);
+			goto put_chan_node;
+		}
 
 		error = iqs7222_gpio_select(iqs7222, event_node,
 					    BIT(chan_index),
 					    dev_desc->touch_link - (i ? 0 : 2));
-		if (error)
-			return error;
+		if (error) {
+			fwnode_handle_put(event_node);
+			goto put_chan_node;
+		}
 
 		if (!fwnode_property_read_u32(event_node,
 					      "azoteq,timeout-press-ms",
@@ -1922,7 +1937,9 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 				dev_err(&client->dev,
 					"Invalid %s press timeout: %u\n",
 					fwnode_get_name(chan_node), val);
-				return -EINVAL;
+				error = -EINVAL;
+				fwnode_handle_put(event_node);
+				goto put_chan_node;
 			}
 
 			*setup &= ~(U8_MAX << i * 8);
@@ -1934,7 +1951,8 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 		if (error) {
 			dev_err(&client->dev, "Failed to read %s code: %d\n",
 				fwnode_get_name(chan_node), error);
-			return error;
+			fwnode_handle_put(event_node);
+			goto put_chan_node;
 		}
 
 		iqs7222->kp_code[chan_index][i] = val;
@@ -1948,19 +1966,24 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 				dev_err(&client->dev,
 					"Failed to read %s input type: %d\n",
 					fwnode_get_name(chan_node), error);
-				return error;
+				fwnode_handle_put(event_node);
+				goto put_chan_node;
 			}
 
 			if (val != EV_KEY && val != EV_SW) {
 				dev_err(&client->dev,
 					"Invalid %s input type: %u\n",
 					fwnode_get_name(chan_node), val);
-				return -EINVAL;
+				error = -EINVAL;
+				fwnode_handle_put(event_node);
+				goto put_chan_node;
 			}
 
 			iqs7222->kp_type[chan_index][i] = val;
 		}
 
+		fwnode_handle_put(event_node);
+
 		/*
 		 * Reference channels can opt out of event reporting by using
 		 * KEY_RESERVED in place of a true key or switch code.
@@ -1983,9 +2006,14 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 	 * The following call handles a special pair of properties that apply
 	 * to a channel node, but reside within the button (event) group.
 	 */
-	return iqs7222_parse_props(iqs7222, &chan_node, chan_index,
-				   IQS7222_REG_GRP_BTN,
-				   IQS7222_REG_KEY_DEBOUNCE);
+	error = iqs7222_parse_props(iqs7222, &chan_node, chan_index,
+				    IQS7222_REG_GRP_BTN,
+				    IQS7222_REG_KEY_DEBOUNCE);
+
+put_chan_node:
+	fwnode_handle_put(chan_node);
+
+	return error;
 }
 
 static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
@@ -2004,7 +2032,7 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 				    IQS7222_REG_GRP_SLDR,
 				    IQS7222_REG_KEY_NONE);
 	if (error)
-		return error;
+		goto put_sldr_node;
 
 	if (!sldr_node)
 		return 0;
@@ -2018,11 +2046,13 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 	if (count < 0) {
 		dev_err(&client->dev, "Failed to count %s channels: %d\n",
 			fwnode_get_name(sldr_node), count);
-		return count;
+		error = count;
+		goto put_sldr_node;
 	} else if (count < 3 || count > ARRAY_SIZE(chan_sel)) {
 		dev_err(&client->dev, "Invalid number of %s channels\n",
 			fwnode_get_name(sldr_node));
-		return -EINVAL;
+		error = -EINVAL;
+		goto put_sldr_node;
 	}
 
 	error = fwnode_property_read_u32_array(sldr_node,
@@ -2031,7 +2061,7 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 	if (error) {
 		dev_err(&client->dev, "Failed to read %s channels: %d\n",
 			fwnode_get_name(sldr_node), error);
-		return error;
+		goto put_sldr_node;
 	}
 
 	/*
@@ -2052,7 +2082,8 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 		if (chan_sel[i] >= ext_chan) {
 			dev_err(&client->dev, "Invalid %s channel: %u\n",
 				fwnode_get_name(sldr_node), chan_sel[i]);
-			return -EINVAL;
+			error = -EINVAL;
+			goto put_sldr_node;
 		}
 
 		/*
@@ -2071,7 +2102,8 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 		if (!val || val > dev_desc->sldr_res) {
 			dev_err(&client->dev, "Invalid %s size: %u\n",
 				fwnode_get_name(sldr_node), val);
-			return -EINVAL;
+			error = -EINVAL;
+			goto put_sldr_node;
 		}
 
 		if (reg_offset) {
@@ -2087,7 +2119,8 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 		if (val > (reg_offset ? U16_MAX : U8_MAX * 4)) {
 			dev_err(&client->dev, "Invalid %s top speed: %u\n",
 				fwnode_get_name(sldr_node), val);
-			return -EINVAL;
+			error = -EINVAL;
+			goto put_sldr_node;
 		}
 
 		if (reg_offset) {
@@ -2142,8 +2175,10 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 					    reg_offset ?
 					    IQS7222_REG_KEY_RESERVED :
 					    iqs7222_sl_events[i].reg_key);
-		if (error)
-			return error;
+		if (error) {
+			fwnode_handle_put(event_node);
+			goto put_sldr_node;
+		}
 
 		/*
 		 * The press/release event does not expose a direct GPIO link,
@@ -2155,8 +2190,10 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 					      : sldr_setup[3 + reg_offset],
 					    i ? 1568 + sldr_index * 30
 					      : sldr_setup[4 + reg_offset]);
-		if (error)
-			return error;
+		if (error) {
+			fwnode_handle_put(event_node);
+			goto put_sldr_node;
+		}
 
 		if (!reg_offset)
 			sldr_setup[9] |= iqs7222_sl_events[i].enable;
@@ -2166,12 +2203,15 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 		if (error) {
 			dev_err(&client->dev, "Failed to read %s code: %d\n",
 				fwnode_get_name(sldr_node), error);
-			return error;
+			fwnode_handle_put(event_node);
+			goto put_sldr_node;
 		}
 
 		iqs7222->sl_code[sldr_index][i] = val;
 		input_set_capability(iqs7222->keypad, EV_KEY, val);
 
+		fwnode_handle_put(event_node);
+
 		if (!dev_desc->event_offset)
 			continue;
 
@@ -2192,11 +2232,16 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 	 * The following call handles a special pair of properties that shift
 	 * to make room for a wheel enable control in the case of IQS7222C.
 	 */
-	return iqs7222_parse_props(iqs7222, &sldr_node, sldr_index,
-				   IQS7222_REG_GRP_SLDR,
-				   dev_desc->wheel_enable ?
-				   IQS7222_REG_KEY_WHEEL :
-				   IQS7222_REG_KEY_NO_WHEEL);
+	error = iqs7222_parse_props(iqs7222, &sldr_node, sldr_index,
+				    IQS7222_REG_GRP_SLDR,
+				    dev_desc->wheel_enable ?
+				    IQS7222_REG_KEY_WHEEL :
+				    IQS7222_REG_KEY_NO_WHEEL);
+
+put_sldr_node:
+	fwnode_handle_put(sldr_node);
+
+	return error;
 }
 
 static int iqs7222_parse_all(struct iqs7222_private *iqs7222)
@@ -2232,6 +2277,9 @@ static int iqs7222_parse_all(struct iqs7222_private *iqs7222)
 		error = iqs7222_parse_props(iqs7222, &gpio_node, i,
 					    IQS7222_REG_GRP_GPIO,
 					    IQS7222_REG_KEY_NONE);
+		if (gpio_node)
+			fwnode_handle_put(gpio_node);
+
 		if (error)
 			return error;
 
-- 
2.25.1

Re: [PATCH 01/11] Input: iqs7222 - drop unused device node references

[Dmitry Torokhov]

On Thu, Sep 08, 2022 at 08:15:38AM -0500, Jeff LaBundy 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.

Hmm, dev_fwnode() however does not do that, which means that
iqs7222_parse_props() has different refounting, depending on what is
being fetched. I think we need to start there.

Also, maybe we could avoid sprinkling gotos if we moved property reading
code into helpers?

Thanks.

-- 
Dmitry

Re: [PATCH 01/11] Input: iqs7222 - drop unused device node references

[Jeff LaBundy]

Hi Dmitry,

Thank you for taking a look.

On Thu, Sep 08, 2022 at 02:17:21PM -0700, Dmitry Torokhov wrote:
> On Thu, Sep 08, 2022 at 08:15:38AM -0500, Jeff LaBundy 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.
> 
> Hmm, dev_fwnode() however does not do that, which means that
> iqs7222_parse_props() has different refounting, depending on what is
> being fetched. I think we need to start there.

Right, but none of the callers that prompt iqs7222_parse_props() to
use dev_fwnode() follow with fwnode_handle_put().

> 
> Also, maybe we could avoid sprinkling gotos if we moved property reading
> code into helpers?

I like this idea; I will give it a try.

> 
> Thanks.
> 
> -- 
> Dmitry

Kind regards,
Jeff LaBundy

Re: [PATCH 01/11] Input: iqs7222 - drop unused device node references

[Dmitry Torokhov]

On Thu, Sep 08, 2022 at 09:04:06PM -0500, Jeff LaBundy wrote:
> Hi Dmitry,
> 
> Thank you for taking a look.
> 
> On Thu, Sep 08, 2022 at 02:17:21PM -0700, Dmitry Torokhov wrote:
> > On Thu, Sep 08, 2022 at 08:15:38AM -0500, Jeff LaBundy 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.
> > 
> > Hmm, dev_fwnode() however does not do that, which means that
> > iqs7222_parse_props() has different refounting, depending on what is
> > being fetched. I think we need to start there.
> 
> Right, but none of the callers that prompt iqs7222_parse_props() to
> use dev_fwnode() follow with fwnode_handle_put().

I think this is a problem that code has to be aware of that and behave
differently. I'd recommend bumping up refcount in dev_fwnode() path so
that all callers would behave uniformly. 

Thanks.

-- 
Dmitry

Re: [PATCH 01/11] Input: iqs7222 - drop unused device node references

[Jeff LaBundy]

Hi Dmitry,

On Thu, Sep 08, 2022 at 09:37:57PM -0700, Dmitry Torokhov wrote:
> On Thu, Sep 08, 2022 at 09:04:06PM -0500, Jeff LaBundy wrote:
> > Hi Dmitry,
> > 
> > Thank you for taking a look.
> > 
> > On Thu, Sep 08, 2022 at 02:17:21PM -0700, Dmitry Torokhov wrote:
> > > On Thu, Sep 08, 2022 at 08:15:38AM -0500, Jeff LaBundy 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.
> > > 
> > > Hmm, dev_fwnode() however does not do that, which means that
> > > iqs7222_parse_props() has different refounting, depending on what is
> > > being fetched. I think we need to start there.
> > 
> > Right, but none of the callers that prompt iqs7222_parse_props() to
> > use dev_fwnode() follow with fwnode_handle_put().
> 
> I think this is a problem that code has to be aware of that and behave
> differently. I'd recommend bumping up refcount in dev_fwnode() path so
> that all callers would behave uniformly. 

Agreed, right now the problem is that not all callers have a node to
put. So, I think the solution is to more thoughtfully encapsulate all
of this such that the caller is always responsible for passing a node,
iqs7222_parse_props() bumps the refcount of whatever it is fetching,
and the caller is always responsible for dropping the node.

This way, callers of iqs7222_parse_props() need not be burdened with
what it chooses to do under the hood.

> 
> Thanks.
> 
> -- 
> Dmitry

Kind regards,
Jeff LaBundy

[PATCH 02/11] Input: iqs7222 - report malformed properties

[Jeff LaBundy]

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@labundy.com>
---
 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);
+			if (error) {
+				dev_err(&client->dev,
+					"Failed to read %s reference weight: %d\n",
+					fwnode_get_name(chan_node), error);
+				goto put_chan_node;
+			}
+
 			if (val > U16_MAX) {
 				dev_err(&client->dev,
 					"Invalid %s reference weight: %u\n",
@@ -1921,9 +1930,8 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 			goto put_chan_node;
 		}
 
-		if (!fwnode_property_read_u32(event_node,
-					      "azoteq,timeout-press-ms",
-					      &val)) {
+		if (fwnode_property_present(event_node,
+					    "azoteq,timeout-press-ms")) {
 			/*
 			 * The IQS7222B employs a global pair of press timeout
 			 * registers as opposed to channel-specific registers.
@@ -1933,6 +1941,17 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 				     &iqs7222->btn_setup[chan_index][2] :
 				     &sys_setup[9];
 
+			error = fwnode_property_read_u32(event_node,
+							 "azoteq,timeout-press-ms",
+							 &val);
+			if (error) {
+				dev_err(&client->dev,
+					"Failed to read %s press timeout: %d\n",
+					fwnode_get_name(chan_node), error);
+				fwnode_handle_put(event_node);
+				goto put_chan_node;
+			}
+
 			if (val > U8_MAX * 500) {
 				dev_err(&client->dev,
 					"Invalid %s press timeout: %u\n",
@@ -2098,7 +2117,15 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 	if (fwnode_property_present(sldr_node, "azoteq,use-prox"))
 		sldr_setup[4 + reg_offset] -= 2;
 
-	if (!fwnode_property_read_u32(sldr_node, "azoteq,slider-size", &val)) {
+	if (fwnode_property_present(sldr_node, "azoteq,slider-size")) {
+		error = fwnode_property_read_u32(sldr_node,
+						 "azoteq,slider-size", &val);
+		if (error) {
+			dev_err(&client->dev, "Failed to read %s size: %d\n",
+				fwnode_get_name(sldr_node), error);
+			goto put_sldr_node;
+		}
+
 		if (!val || val > dev_desc->sldr_res) {
 			dev_err(&client->dev, "Invalid %s size: %u\n",
 				fwnode_get_name(sldr_node), val);
@@ -2115,7 +2142,16 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 		}
 	}
 
-	if (!fwnode_property_read_u32(sldr_node, "azoteq,top-speed", &val)) {
+	if (fwnode_property_present(sldr_node, "azoteq,top-speed")) {
+		error = fwnode_property_read_u32(sldr_node,
+						 "azoteq,top-speed", &val);
+		if (error) {
+			dev_err(&client->dev,
+				"Failed to read %s top speed: %d\n",
+				fwnode_get_name(sldr_node), error);
+			goto put_sldr_node;
+		}
+
 		if (val > (reg_offset ? U16_MAX : U8_MAX * 4)) {
 			dev_err(&client->dev, "Invalid %s top speed: %u\n",
 				fwnode_get_name(sldr_node), val);
@@ -2131,10 +2167,19 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 		}
 	}
 
-	if (!fwnode_property_read_u32(sldr_node, "linux,axis", &val)) {
-		u16 sldr_max = sldr_setup[3] - 1;
+	if (fwnode_property_present(sldr_node, "linux,axis")) {
+		u16 sldr_max;
+
+		error = fwnode_property_read_u32(sldr_node, "linux,axis", &val);
+		if (error) {
+			dev_err(&client->dev, "Failed to read %s axis: %d\n",
+				fwnode_get_name(sldr_node), error);
+			goto put_sldr_node;
+		}
 
-		if (!reg_offset) {
+		if (reg_offset) {
+			sldr_max = sldr_setup[3] - 1;
+		} else {
 			sldr_max = sldr_setup[2];
 
 			sldr_max &= IQS7222_SLDR_SETUP_2_RES_MASK;
-- 
2.25.1

Re: [PATCH 02/11] Input: iqs7222 - report malformed properties

[Dmitry Torokhov]

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@labundy.com>
> ---
>  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?

-- 
Dmitry

Re: [PATCH 02/11] Input: iqs7222 - report malformed properties

[Jeff LaBundy]

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@labundy.com>
> > ---
> >  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.

> 
> -- 
> Dmitry

Kind regards,
Jeff LaBundy

Re: [PATCH 02/11] Input: iqs7222 - report malformed properties

[Dmitry Torokhov]

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@labundy.com>
> > > ---
> > >  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

Re: [PATCH 02/11] Input: iqs7222 - report malformed properties

[Jeff LaBundy]

Hi Dmitry,

On Thu, Sep 08, 2022 at 09:42:47PM -0700, Dmitry Torokhov wrote:
> 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@labundy.com>
> > > > ---
> > > >  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.

Right, originally I had imagined a wrapper around fwnode_property_read_u32()
that calls fwnode_handle_put() in the error path. However, your proposal is
even better.

Thanks for the fruitful discussion; I'll clean all of this up for v2.

> 
> Thanks.
> 
> -- 
> Dmitry

Kind regards,
Jeff LaBundy

[PATCH 03/11] dt-bindings: input: iqs7222: Correct minimum slider size

[Jeff LaBundy]

The minimum slider size enforced by the driver is 1 or 16 for the
IQS7222C or IQS7222A, respectively.

Fixes: 44dc42d254bf ("dt-bindings: input: Add bindings for Azoteq IQS7222A/B/C")
Signed-off-by: Jeff LaBundy <jeff@labundy.com>
---
 Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml b/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
index 02e605fac408..785fb9e83354 100644
--- a/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
+++ b/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
@@ -501,7 +501,7 @@ patternProperties:
 
       azoteq,slider-size:
         $ref: /schemas/types.yaml#/definitions/uint32
-        minimum: 0
+        minimum: 1
         maximum: 65535
         description:
           Specifies the slider's one-dimensional resolution, equal to the
@@ -693,6 +693,7 @@ allOf:
           properties:
             azoteq,slider-size:
               multipleOf: 16
+              minimum: 16
               maximum: 4080
 
             azoteq,top-speed:
-- 
2.25.1

Re: [PATCH 03/11] dt-bindings: input: iqs7222: Correct minimum slider size

[Rob Herring]

On Thu, 08 Sep 2022 08:15:40 -0500, Jeff LaBundy wrote:
> The minimum slider size enforced by the driver is 1 or 16 for the
> IQS7222C or IQS7222A, respectively.
> 
> Fixes: 44dc42d254bf ("dt-bindings: input: Add bindings for Azoteq IQS7222A/B/C")
> Signed-off-by: Jeff LaBundy <jeff@labundy.com>
> ---
>  Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 04/11] Input: iqs7222 - protect against undefined slider size

[Jeff LaBundy]

Select variants of silicon do not define a default slider size, in
which case the size must be specified in the device tree. If it is
not, the axis's maximum value is reported as 65535 due to unsigned
integer overflow.

To solve this problem, move the existing zero-check outside of the
conditional block that checks whether the property is present.

Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C")
Signed-off-by: Jeff LaBundy <jeff@labundy.com>
---
 drivers/input/misc/iqs7222.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/input/misc/iqs7222.c b/drivers/input/misc/iqs7222.c
index fdade24caa8a..ba12c49e972c 100644
--- a/drivers/input/misc/iqs7222.c
+++ b/drivers/input/misc/iqs7222.c
@@ -2126,7 +2126,7 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 			goto put_sldr_node;
 		}
 
-		if (!val || val > dev_desc->sldr_res) {
+		if (val > dev_desc->sldr_res) {
 			dev_err(&client->dev, "Invalid %s size: %u\n",
 				fwnode_get_name(sldr_node), val);
 			error = -EINVAL;
@@ -2142,6 +2142,14 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 		}
 	}
 
+	if (!(reg_offset ? sldr_setup[3]
+			 : sldr_setup[2] & IQS7222_SLDR_SETUP_2_RES_MASK)) {
+		dev_err(&client->dev, "Undefined %s size\n",
+			fwnode_get_name(sldr_node));
+		error = -EINVAL;
+		goto put_sldr_node;
+	}
+
 	if (fwnode_property_present(sldr_node, "azoteq,top-speed")) {
 		error = fwnode_property_read_u32(sldr_node,
 						 "azoteq,top-speed", &val);
-- 
2.25.1

[PATCH 05/11] Input: iqs7222 - trim force communication command

[Jeff LaBundy]

According to the datasheets, writing only 0xFF is sufficient to
elicit a communication window. Remove the superfluous 0x00 from
the force communication command.

Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C")
Signed-off-by: Jeff LaBundy <jeff@labundy.com>
---
 drivers/input/misc/iqs7222.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/misc/iqs7222.c b/drivers/input/misc/iqs7222.c
index ba12c49e972c..365e59f78f1a 100644
--- a/drivers/input/misc/iqs7222.c
+++ b/drivers/input/misc/iqs7222.c
@@ -1077,7 +1077,7 @@ static int iqs7222_hard_reset(struct iqs7222_private *iqs7222)
 
 static int iqs7222_force_comms(struct iqs7222_private *iqs7222)
 {
-	u8 msg_buf[] = { 0xFF, 0x00, };
+	u8 msg_buf[] = { 0xFF, };
 	int ret;
 
 	/*
-- 
2.25.1

Re: [PATCH 05/11] Input: iqs7222 - trim force communication command

[Dmitry Torokhov]

On Thu, Sep 08, 2022 at 08:15:42AM -0500, Jeff LaBundy wrote:
> According to the datasheets, writing only 0xFF is sufficient to
> elicit a communication window. Remove the superfluous 0x00 from
> the force communication command.
> 
> Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C")
> Signed-off-by: Jeff LaBundy <jeff@labundy.com>

Applied, thank you.

-- 
Dmitry

Re: [PATCH 05/11] Input: iqs7222 - trim force communication command

[Jeff LaBundy]

Hi Dmitry,

On Thu, Sep 08, 2022 at 02:24:10PM -0700, Dmitry Torokhov wrote:
> On Thu, Sep 08, 2022 at 08:15:42AM -0500, Jeff LaBundy wrote:
> > According to the datasheets, writing only 0xFF is sufficient to
> > elicit a communication window. Remove the superfluous 0x00 from
> > the force communication command.
> > 
> > Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C")
> > Signed-off-by: Jeff LaBundy <jeff@labundy.com>
> 
> Applied, thank you.

I didn't happen to see this one hit your tree, so I can simply include
it in v2 coming soon. In case I have misunderstood, please let me know.

> 
> -- 
> Dmitry

Kind regards,
Jeff LaBundy

Re: [PATCH 05/11] Input: iqs7222 - trim force communication command

[Dmitry Torokhov]

On Tue, Sep 13, 2022 at 04:24:54PM -0500, Jeff LaBundy wrote:
> Hi Dmitry,
> 
> On Thu, Sep 08, 2022 at 02:24:10PM -0700, Dmitry Torokhov wrote:
> > On Thu, Sep 08, 2022 at 08:15:42AM -0500, Jeff LaBundy wrote:
> > > According to the datasheets, writing only 0xFF is sufficient to
> > > elicit a communication window. Remove the superfluous 0x00 from
> > > the force communication command.
> > > 
> > > Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C")
> > > Signed-off-by: Jeff LaBundy <jeff@labundy.com>
> > 
> > Applied, thank you.
> 
> I didn't happen to see this one hit your tree, so I can simply include
> it in v2 coming soon. In case I have misunderstood, please let me know.

Should be there now.

Thanks.

-- 
Dmitry

[PATCH 06/11] Input: iqs7222 - avoid sending empty SYN_REPORT events

[Jeff LaBundy]

Add a check to prevent sending undefined events, which ultimately
map to SYN_REPORT.

Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C")
Signed-off-by: Jeff LaBundy <jeff@labundy.com>
---
 drivers/input/misc/iqs7222.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/input/misc/iqs7222.c b/drivers/input/misc/iqs7222.c
index 365e59f78f1a..00c73a920ab2 100644
--- a/drivers/input/misc/iqs7222.c
+++ b/drivers/input/misc/iqs7222.c
@@ -2427,6 +2427,9 @@ static int iqs7222_report(struct iqs7222_private *iqs7222)
 			int k = 2 + j * (num_chan > 16 ? 2 : 1);
 			u16 state = le16_to_cpu(status[k + i / 16]);
 
+			if (!iqs7222->kp_type[i][j])
+				continue;
+
 			input_event(iqs7222->keypad,
 				    iqs7222->kp_type[i][j],
 				    iqs7222->kp_code[i][j],
-- 
2.25.1

Re: [PATCH 06/11] Input: iqs7222 - avoid sending empty SYN_REPORT events

[Dmitry Torokhov]

On Thu, Sep 08, 2022 at 08:15:43AM -0500, Jeff LaBundy wrote:
> Add a check to prevent sending undefined events, which ultimately
> map to SYN_REPORT.
> 
> Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C")
> Signed-off-by: Jeff LaBundy <jeff@labundy.com>

Applied, thank you.

-- 
Dmitry

[PATCH 07/11] Input: iqs7222 - set all ULP entry masks by default

[Jeff LaBundy]

Some devices expose an ultra-low-power (ULP) mode entry mask for
each channel. If the mask is set, the device cannot enter ULP so
long as the corresponding channel remains in an active state.

The vendor has advised setting the mask for any disabled channel.
To accommodate this suggestion, initially set all masks and then
clear them only if specified in the device tree.

Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C")
Signed-off-by: Jeff LaBundy <jeff@labundy.com>
---
 drivers/input/misc/iqs7222.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/input/misc/iqs7222.c b/drivers/input/misc/iqs7222.c
index 00c73a920ab2..d1a4ab3c95d3 100644
--- a/drivers/input/misc/iqs7222.c
+++ b/drivers/input/misc/iqs7222.c
@@ -1777,11 +1777,9 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 	if (!chan_node)
 		return 0;
 
-	if (dev_desc->allow_offset) {
-		sys_setup[dev_desc->allow_offset] |= BIT(chan_index);
-		if (fwnode_property_present(chan_node, "azoteq,ulp-allow"))
-			sys_setup[dev_desc->allow_offset] &= ~BIT(chan_index);
-	}
+	if (dev_desc->allow_offset &&
+	    fwnode_property_present(chan_node, "azoteq,ulp-allow"))
+		sys_setup[dev_desc->allow_offset] &= ~BIT(chan_index);
 
 	chan_setup[0] |= IQS7222_CHAN_SETUP_0_CHAN_EN;
 
@@ -2304,6 +2302,9 @@ static int iqs7222_parse_all(struct iqs7222_private *iqs7222)
 	u16 *sys_setup = iqs7222->sys_setup;
 	int error, i;
 
+	if (dev_desc->allow_offset)
+		sys_setup[dev_desc->allow_offset] = U16_MAX;
+
 	if (dev_desc->event_offset)
 		sys_setup[dev_desc->event_offset] = IQS7222_EVENT_MASK_ATI;
 
-- 
2.25.1

Re: [PATCH 07/11] Input: iqs7222 - set all ULP entry masks by default

[Dmitry Torokhov]

On Thu, Sep 08, 2022 at 08:15:44AM -0500, Jeff LaBundy wrote:
> Some devices expose an ultra-low-power (ULP) mode entry mask for
> each channel. If the mask is set, the device cannot enter ULP so
> long as the corresponding channel remains in an active state.
> 
> The vendor has advised setting the mask for any disabled channel.
> To accommodate this suggestion, initially set all masks and then
> clear them only if specified in the device tree.
> 
> Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C")
> Signed-off-by: Jeff LaBundy <jeff@labundy.com>

Applied, thank you.

-- 
Dmitry

[PATCH 08/11] Input: iqs7222 - allow 'linux,code' to be optional

[Jeff LaBundy]

Event nodes might be defined solely for the purpose of mapping a
GPIO, without needing to report the event to the input core. Add
support for this use-case by making 'linux,code' optional.

This change relieves the burden for reference channels having to
specify KEY_RESERVED for their corresponding key code. The check
that skips events specified with KEY_RESERVED can be dropped, as
input_register_device() already blocks this event.

Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C")
Signed-off-by: Jeff LaBundy <jeff@labundy.com>
---
 drivers/input/misc/iqs7222.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/input/misc/iqs7222.c b/drivers/input/misc/iqs7222.c
index d1a4ab3c95d3..ac810b8ab69f 100644
--- a/drivers/input/misc/iqs7222.c
+++ b/drivers/input/misc/iqs7222.c
@@ -1963,6 +1963,11 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 			*setup |= (val / 500 << i * 8);
 		}
 
+		if (!fwnode_property_present(event_node, "linux,code")) {
+			fwnode_handle_put(event_node);
+			continue;
+		}
+
 		error = fwnode_property_read_u32(event_node, "linux,code",
 						 &val);
 		if (error) {
@@ -1999,20 +2004,12 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222, int chan_index)
 			iqs7222->kp_type[chan_index][i] = val;
 		}
 
-		fwnode_handle_put(event_node);
-
-		/*
-		 * Reference channels can opt out of event reporting by using
-		 * KEY_RESERVED in place of a true key or switch code.
-		 */
-		if (iqs7222->kp_type[chan_index][i] == EV_KEY &&
-		    iqs7222->kp_code[chan_index][i] == KEY_RESERVED)
-			continue;
-
 		input_set_capability(iqs7222->keypad,
 				     iqs7222->kp_type[chan_index][i],
 				     iqs7222->kp_code[chan_index][i]);
 
+		fwnode_handle_put(event_node);
+
 		if (!dev_desc->event_offset)
 			continue;
 
@@ -2249,6 +2246,11 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 		if (!reg_offset)
 			sldr_setup[9] |= iqs7222_sl_events[i].enable;
 
+		if (!fwnode_property_present(event_node, "linux,code")) {
+			fwnode_handle_put(event_node);
+			continue;
+		}
+
 		error = fwnode_property_read_u32(event_node, "linux,code",
 						 &val);
 		if (error) {
-- 
2.25.1

[PATCH 09/11] dt-bindings: input: iqs7222: Allow 'linux,code' to be optional

[Jeff LaBundy]

The 'linux,code' property has been made optional in the driver;
update the binding to reflect this change.

Fixes: 44dc42d254bf ("dt-bindings: input: Add bindings for Azoteq IQS7222A/B/C")
Signed-off-by: Jeff LaBundy <jeff@labundy.com>
---
 Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml b/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
index 785fb9e83354..913fd2da9862 100644
--- a/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
+++ b/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
@@ -473,9 +473,6 @@ patternProperties:
               Specifies whether the event is to be interpreted as a key (1)
               or a switch (5).
 
-        required:
-          - linux,code
-
         additionalProperties: false
 
     dependencies:
@@ -620,9 +617,6 @@ patternProperties:
               GPIO, they must all be of the same type (proximity, touch or
               slider gesture).
 
-        required:
-          - linux,code
-
         additionalProperties: false
 
     required:
-- 
2.25.1

Re: [PATCH 09/11] dt-bindings: input: iqs7222: Allow 'linux,code' to be optional

[Rob Herring]

On Thu, Sep 08, 2022 at 08:15:46AM -0500, Jeff LaBundy wrote:
> The 'linux,code' property has been made optional in the driver;
> update the binding to reflect this change.

But with an old kernel, it would still be required. So still required in 
the DT.

> 
> Fixes: 44dc42d254bf ("dt-bindings: input: Add bindings for Azoteq IQS7222A/B/C")
> Signed-off-by: Jeff LaBundy <jeff@labundy.com>
> ---
>  Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml b/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
> index 785fb9e83354..913fd2da9862 100644
> --- a/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
> +++ b/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
> @@ -473,9 +473,6 @@ patternProperties:
>                Specifies whether the event is to be interpreted as a key (1)
>                or a switch (5).
>  
> -        required:
> -          - linux,code
> -
>          additionalProperties: false
>  
>      dependencies:
> @@ -620,9 +617,6 @@ patternProperties:
>                GPIO, they must all be of the same type (proximity, touch or
>                slider gesture).
>  
> -        required:
> -          - linux,code
> -
>          additionalProperties: false
>  
>      required:
> -- 
> 2.25.1
> 
> 

Re: [PATCH 09/11] dt-bindings: input: iqs7222: Allow 'linux,code' to be optional

[Jeff LaBundy]

Hi Rob,

Thank you for taking a look.

On Tue, Sep 13, 2022 at 06:42:49AM -0500, Rob Herring wrote:
> On Thu, Sep 08, 2022 at 08:15:46AM -0500, Jeff LaBundy wrote:
> > The 'linux,code' property has been made optional in the driver;
> > update the binding to reflect this change.
> 
> But with an old kernel, it would still be required. So still required in 
> the DT.

For v2 of this series, the corresponding driver change will be
absorbed in a patch intended for -stable; the refactoring just
worked out that way.

In that case, even older kernels would drop this requirement,
so I will keep the patch. In case I misunderstood your comment,
please let me know.

> 
> > 
> > Fixes: 44dc42d254bf ("dt-bindings: input: Add bindings for Azoteq IQS7222A/B/C")
> > Signed-off-by: Jeff LaBundy <jeff@labundy.com>
> > ---
> >  Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml | 6 ------
> >  1 file changed, 6 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml b/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
> > index 785fb9e83354..913fd2da9862 100644
> > --- a/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
> > +++ b/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
> > @@ -473,9 +473,6 @@ patternProperties:
> >                Specifies whether the event is to be interpreted as a key (1)
> >                or a switch (5).
> >  
> > -        required:
> > -          - linux,code
> > -
> >          additionalProperties: false
> >  
> >      dependencies:
> > @@ -620,9 +617,6 @@ patternProperties:
> >                GPIO, they must all be of the same type (proximity, touch or
> >                slider gesture).
> >  
> > -        required:
> > -          - linux,code
> > -
> >          additionalProperties: false
> >  
> >      required:
> > -- 
> > 2.25.1
> > 
> > 

Kind regards,
Jeff LaBundy

[PATCH 10/11] dt-bindings: input: iqs7222: Add support for IQS7222A v1.13+

[Jeff LaBundy]

IQS7222A revisions 1.13 and later widen the gesture multiplier from
x4 ms to x16 ms; update the binding accordingly.

As part of this change, refresh the corresponding properties in the
example as well.

Fixes: 44dc42d254bf ("dt-bindings: input: Add bindings for Azoteq IQS7222A/B/C")
Signed-off-by: Jeff LaBundy <jeff@labundy.com>
---
 .../bindings/input/azoteq,iqs7222.yaml           | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml b/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
index 913fd2da9862..9ddba7f2e7aa 100644
--- a/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
+++ b/Documentation/devicetree/bindings/input/azoteq,iqs7222.yaml
@@ -572,9 +572,9 @@ patternProperties:
           linux,code: true
 
           azoteq,gesture-max-ms:
-            multipleOf: 4
+            multipleOf: 16
             minimum: 0
-            maximum: 1020
+            maximum: 4080
             description:
               Specifies the length of time (in ms) within which a tap, swipe
               or flick gesture must be completed in order to be acknowledged
@@ -582,9 +582,9 @@ patternProperties:
               gesture applies to all remaining swipe or flick gestures.
 
           azoteq,gesture-min-ms:
-            multipleOf: 4
+            multipleOf: 16
             minimum: 0
-            maximum: 124
+            maximum: 496
             description:
               Specifies the length of time (in ms) for which a tap gesture must
               be held in order to be acknowledged by the device.
@@ -930,14 +930,14 @@ examples:
 
                             event-tap {
                                     linux,code = <KEY_PLAYPAUSE>;
-                                    azoteq,gesture-max-ms = <600>;
-                                    azoteq,gesture-min-ms = <24>;
+                                    azoteq,gesture-max-ms = <400>;
+                                    azoteq,gesture-min-ms = <32>;
                             };
 
                             event-flick-pos {
                                     linux,code = <KEY_NEXTSONG>;
-                                    azoteq,gesture-max-ms = <600>;
-                                    azoteq,gesture-dist = <816>;
+                                    azoteq,gesture-max-ms = <800>;
+                                    azoteq,gesture-dist = <800>;
                             };
 
                             event-flick-neg {
-- 
2.25.1

Re: [PATCH 10/11] dt-bindings: input: iqs7222: Add support for IQS7222A v1.13+

[Rob Herring]

On Thu, 08 Sep 2022 08:15:47 -0500, Jeff LaBundy wrote:
> IQS7222A revisions 1.13 and later widen the gesture multiplier from
> x4 ms to x16 ms; update the binding accordingly.
> 
> As part of this change, refresh the corresponding properties in the
> example as well.
> 
> Fixes: 44dc42d254bf ("dt-bindings: input: Add bindings for Azoteq IQS7222A/B/C")
> Signed-off-by: Jeff LaBundy <jeff@labundy.com>
> ---
>  .../bindings/input/azoteq,iqs7222.yaml           | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH 11/11] Input: iqs7222 - add support for IQS7222A v1.13+

[Jeff LaBundy]

IQS7222A revisions 1.13 and later widen the gesture multiplier from
x4 ms to x16 ms. Add a means to scale the gesture timings specified
in the device tree based on the revision of the device.

Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C")
Signed-off-by: Jeff LaBundy <jeff@labundy.com>
---
 drivers/input/misc/iqs7222.c | 122 +++++++++++++++++++++++++++++++++--
 1 file changed, 118 insertions(+), 4 deletions(-)

diff --git a/drivers/input/misc/iqs7222.c b/drivers/input/misc/iqs7222.c
index ac810b8ab69f..15f0bfa899dd 100644
--- a/drivers/input/misc/iqs7222.c
+++ b/drivers/input/misc/iqs7222.c
@@ -86,7 +86,9 @@ enum iqs7222_reg_key_id {
 	IQS7222_REG_KEY_TOUCH,
 	IQS7222_REG_KEY_DEBOUNCE,
 	IQS7222_REG_KEY_TAP,
+	IQS7222_REG_KEY_TAP_LEGACY,
 	IQS7222_REG_KEY_AXIAL,
+	IQS7222_REG_KEY_AXIAL_LEGACY,
 	IQS7222_REG_KEY_WHEEL,
 	IQS7222_REG_KEY_NO_WHEEL,
 	IQS7222_REG_KEY_RESERVED
@@ -202,10 +204,68 @@ struct iqs7222_dev_desc {
 	int allow_offset;
 	int event_offset;
 	int comms_offset;
+	bool legacy_gesture;
 	struct iqs7222_reg_grp_desc reg_grps[IQS7222_NUM_REG_GRPS];
 };
 
 static const struct iqs7222_dev_desc iqs7222_devs[] = {
+	{
+		.prod_num = IQS7222_PROD_NUM_A,
+		.fw_major = 1,
+		.fw_minor = 13,
+		.sldr_res = U8_MAX * 16,
+		.touch_link = 1768,
+		.allow_offset = 9,
+		.event_offset = 10,
+		.comms_offset = 12,
+		.reg_grps = {
+			[IQS7222_REG_GRP_STAT] = {
+				.base = IQS7222_SYS_STATUS,
+				.num_row = 1,
+				.num_col = 8,
+			},
+			[IQS7222_REG_GRP_CYCLE] = {
+				.base = 0x8000,
+				.num_row = 7,
+				.num_col = 3,
+			},
+			[IQS7222_REG_GRP_GLBL] = {
+				.base = 0x8700,
+				.num_row = 1,
+				.num_col = 3,
+			},
+			[IQS7222_REG_GRP_BTN] = {
+				.base = 0x9000,
+				.num_row = 12,
+				.num_col = 3,
+			},
+			[IQS7222_REG_GRP_CHAN] = {
+				.base = 0xA000,
+				.num_row = 12,
+				.num_col = 6,
+			},
+			[IQS7222_REG_GRP_FILT] = {
+				.base = 0xAC00,
+				.num_row = 1,
+				.num_col = 2,
+			},
+			[IQS7222_REG_GRP_SLDR] = {
+				.base = 0xB000,
+				.num_row = 2,
+				.num_col = 11,
+			},
+			[IQS7222_REG_GRP_GPIO] = {
+				.base = 0xC000,
+				.num_row = 1,
+				.num_col = 3,
+			},
+			[IQS7222_REG_GRP_SYS] = {
+				.base = IQS7222_SYS_SETUP,
+				.num_row = 1,
+				.num_col = 13,
+			},
+		},
+	},
 	{
 		.prod_num = IQS7222_PROD_NUM_A,
 		.fw_major = 1,
@@ -215,6 +275,7 @@ static const struct iqs7222_dev_desc iqs7222_devs[] = {
 		.allow_offset = 9,
 		.event_offset = 10,
 		.comms_offset = 12,
+		.legacy_gesture = true,
 		.reg_grps = {
 			[IQS7222_REG_GRP_STAT] = {
 				.base = IQS7222_SYS_STATUS,
@@ -874,6 +935,16 @@ static const struct iqs7222_prop_desc iqs7222_props[] = {
 		.reg_offset = 9,
 		.reg_shift = 8,
 		.reg_width = 8,
+		.val_pitch = 16,
+		.label = "maximum gesture time",
+	},
+	{
+		.name = "azoteq,gesture-max-ms",
+		.reg_grp = IQS7222_REG_GRP_SLDR,
+		.reg_key = IQS7222_REG_KEY_TAP_LEGACY,
+		.reg_offset = 9,
+		.reg_shift = 8,
+		.reg_width = 8,
 		.val_pitch = 4,
 		.label = "maximum gesture time",
 	},
@@ -884,6 +955,16 @@ static const struct iqs7222_prop_desc iqs7222_props[] = {
 		.reg_offset = 9,
 		.reg_shift = 3,
 		.reg_width = 5,
+		.val_pitch = 16,
+		.label = "minimum gesture time",
+	},
+	{
+		.name = "azoteq,gesture-min-ms",
+		.reg_grp = IQS7222_REG_GRP_SLDR,
+		.reg_key = IQS7222_REG_KEY_TAP_LEGACY,
+		.reg_offset = 9,
+		.reg_shift = 3,
+		.reg_width = 5,
 		.val_pitch = 4,
 		.label = "minimum gesture time",
 	},
@@ -897,6 +978,16 @@ static const struct iqs7222_prop_desc iqs7222_props[] = {
 		.val_pitch = 16,
 		.label = "gesture distance",
 	},
+	{
+		.name = "azoteq,gesture-dist",
+		.reg_grp = IQS7222_REG_GRP_SLDR,
+		.reg_key = IQS7222_REG_KEY_AXIAL_LEGACY,
+		.reg_offset = 10,
+		.reg_shift = 8,
+		.reg_width = 8,
+		.val_pitch = 16,
+		.label = "gesture distance",
+	},
 	{
 		.name = "azoteq,gesture-max-ms",
 		.reg_grp = IQS7222_REG_GRP_SLDR,
@@ -904,6 +995,16 @@ static const struct iqs7222_prop_desc iqs7222_props[] = {
 		.reg_offset = 10,
 		.reg_shift = 0,
 		.reg_width = 8,
+		.val_pitch = 16,
+		.label = "maximum gesture time",
+	},
+	{
+		.name = "azoteq,gesture-max-ms",
+		.reg_grp = IQS7222_REG_GRP_SLDR,
+		.reg_key = IQS7222_REG_KEY_AXIAL_LEGACY,
+		.reg_offset = 10,
+		.reg_shift = 0,
+		.reg_width = 8,
 		.val_pitch = 4,
 		.label = "maximum gesture time",
 	},
@@ -2213,16 +2314,29 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 	for (i = 0; i < ARRAY_SIZE(iqs7222_sl_events); i++) {
 		const char *event_name = iqs7222_sl_events[i].name;
 		struct fwnode_handle *event_node;
+		enum iqs7222_reg_key_id reg_key;
 
 		event_node = fwnode_get_named_child_node(sldr_node, event_name);
 		if (!event_node)
 			continue;
 
+		/*
+		 * Depending on the device, gestures are either offered using
+		 * one of two timing resolutions, or are not supported at all.
+		 */
+		if (reg_offset)
+			reg_key = IQS7222_REG_KEY_RESERVED;
+		else if (dev_desc->legacy_gesture &&
+			 iqs7222_sl_events[i].reg_key == IQS7222_REG_KEY_TAP)
+			reg_key = IQS7222_REG_KEY_TAP_LEGACY;
+		else if (dev_desc->legacy_gesture &&
+			 iqs7222_sl_events[i].reg_key == IQS7222_REG_KEY_AXIAL)
+			reg_key = IQS7222_REG_KEY_AXIAL_LEGACY;
+		else
+			reg_key = iqs7222_sl_events[i].reg_key;
+
 		error = iqs7222_parse_props(iqs7222, &event_node, sldr_index,
-					    IQS7222_REG_GRP_SLDR,
-					    reg_offset ?
-					    IQS7222_REG_KEY_RESERVED :
-					    iqs7222_sl_events[i].reg_key);
+					    IQS7222_REG_GRP_SLDR, reg_key);
 		if (error) {
 			fwnode_handle_put(event_node);
 			goto put_sldr_node;
-- 
2.25.1