Miscellaneous fixes for Azoteq IQS626A
Subsystems (3)
v2 cover Miscellaneous fixes for Azoteq IQS626A
This series addresses a fwnode_handle reference count leak. Tightly coupled with this logic is that behind the discussion in [1], where it was pointed out that two properties in question have different types across bindings for the same vendor.
Addressing the latter in fact simplifies the former; therefore both changes are introduced in patch [1/2]. As there are no known users of this driver, it is safe to change the binding in this way.
Patch [2/2] makes the corresponding documentation change and carries a fixes tag so that it accompanies patch [1/2] into stable releases.
[1] https://patchwork.kernel.org/patch/13116327/
Jeff LaBundy (2):
Input: iqs626a - drop unused device node references
dt-bindings: input: iqs626a: Redefine trackpad property types
.../devicetree/bindings/input/iqs626a.yaml | 94 ++++++++--- drivers/input/misc/iqs626a.c | 156 ++++++++---------- 2 files changed, 140 insertions(+), 110 deletions(-)
Signature · 1 line
2.34.1
PATCH v2 1/2 Input: iqs626a - drop unused device node references
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.
As part of this change, trackpad channel touch thresholds and ATI base values are now specified under single trackpad channel child nodes. This enhancement moves both properties to scalar values as opposed to arrays, making their types consistent across bindings.
---
Changes in v2:
- Added support for trackpad channel child nodes and removed logic that parsed former trackpad touch threshold and ATI base arrays
drivers/input/misc/iqs626a.c | 156 ++++++++++++++++------------------- 1 file changed, 73 insertions(+), 83 deletions(-)
drivers/input/misc/iqs626a.c +73 -83
@@ -458,18 +458,15 @@ struct iqs626_private { static noinline_for_stack intiqs626_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 ];unsigned int val; -int num_ch = iqs626_channels[ch_id].num_ch; -int error, i, j; +int i;switch (ch_id) {case IQS626_CH_ULP_0 :
@@ -509,7 +506,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 +530,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 +545,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 ; }
@@ -567,6 +566,7 @@ 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 ; }
@@ -574,32 +574,9 @@ iqs626_parse_events(struct iqs626_private *iqs626, *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) { -dev_err (&client->dev, -"Failed to read %s channel thresholds: %d\n" , -fwnode_get_name (ch_node), error); -return error; } -for (j =0 ; j < num_ch; j++) { -if (thresh_tp[j] > IQS626_CHx_THRESH_MAX) { -dev_err (&client->dev, -"Invalid %s channel threshold: %u\n" , -fwnode_get_name (ch_node), thresh_tp[j]); -return -EINVAL ; - } - - sys_reg->tp_grp_reg.ch_reg_tp[j].thresh = thresh_tp[j]; - } +fwnode_handle_put (ev_node); }return 0 ;
@@ -607,16 +584,13 @@ iqs626_parse_events(struct iqs626_private *iqs626, static noinline_for_stack intiqs626_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; -unsigned int ati_base[IQS626_NUM_CH_TP_3 ];unsigned int val;u8 *ati_target; -int num_ch = iqs626_channels[ch_id].num_ch; -int error, i; +int i;switch (ch_id) {case IQS626_CH_ULP_0 :
@@ -683,40 +657,13 @@ iqs626_parse_ati_target(struct iqs626_private *iqs626, *ati_target &= ~IQS626_CHx_ATI_BASE_MASK; *ati_target |= val; - -return 0 ; - } - -if (!fwnode_property_present (ch_node,"azoteq,ati-base" )) -return 0 ; - - error =fwnode_property_read_u32_array (ch_node,"azoteq,ati-base" , - ati_base, num_ch); -if (error) { -dev_err (&client->dev, -"Failed to read %s channel ATI bases: %d\n" , -fwnode_get_name (ch_node), error); -return error; - } - -for (i =0 ; i < num_ch; i++) { -if (ati_base[i] < IQS626_TPx_ATI_BASE_MIN || - ati_base[i] > IQS626_TPx_ATI_BASE_MAX) { -dev_err (&client->dev, -"Invalid %s channel ATI base: %u\n" , -fwnode_get_name (ch_node), ati_base[i]); -return -EINVAL ; - } - - ati_base[i] -= IQS626_TPx_ATI_BASE_MIN; - sys_reg->tp_grp_reg.ch_reg_tp[i].ati_base = ati_base[i]; }return 0 ; }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,13 +711,14 @@ 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, +enum iqs626_ch_id ch_id) {struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;struct i2c_client *client = iqs626->client;u8 *hyst = &sys_reg->tp_grp_reg.hyst; +int error, count, i;unsigned int val; -int error, count;if (!fwnode_property_read_u32 (ch_node,"azoteq,lta-update" , &val)) {if (val > IQS626_MISC_A_TPx_LTA_UPDATE_MAX) {
@@ -823,6 +771,48 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626, *hyst |= (val << IQS626_FILT_STR_LP_TPx_SHIFT); } +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); +if (!tc_node) +continue ; + +if (!fwnode_property_read_u32 (tc_node,"azoteq,ati-base" , + &val)) { +if (val < IQS626_TPx_ATI_BASE_MIN || + val > IQS626_TPx_ATI_BASE_MAX) { +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 ; + } + + *ati_base = val - IQS626_TPx_ATI_BASE_MIN; + } + +if (!fwnode_property_read_u32 (tc_node,"azoteq,thresh" , + &val)) { +if (val > IQS626_CHx_THRESH_MAX) { +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" ))return 0 ;
@@ -889,8 +879,7 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626, static noinline_for_stack intiqs626_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 +913,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;
@@ -1057,7 +1060,7 @@ iqs626_parse_channel(struct iqs626_private *iqs626, *(engine +1 ) |= IQS626_CHx_ENG_1_ATI_BAND_TIGHTEN;if (ch_id ==IQS626_CH_TP_2 || ch_id ==IQS626_CH_TP_3 ) -return iqs626_parse_trackpad (iqs626, ch_node); +return iqs626_parse_trackpad (iqs626, ch_node, ch_id);if (ch_id ==IQS626_CH_ULP_0 ) { sys_reg->ch_reg_ulp.hyst &= ~IQS626_ULP_PROJ_ENABLE ;
@@ -1378,23 +1381,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; }
Signature · 1 line
2.34.1
Re: [PATCH v2 1/2] Input: iqs626a - drop unused device node references
Quoted 10 lines · 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. Becausefwnode_handle_put()does not take a const*fwnode_handle, the const qualifier is removed across all corresponding*fwnode_handleinstances.
Quoted 7 lines
As part of this change, trackpad channel touch thresholds and ATI base values are now specified under single trackpad channel child nodes. This enhancement moves both properties to scalar values as opposed to arrays, making their types consistent across bindings.
Fixes f1d2809de97a ("Input: Add support for Azoteq IQS626A")Signed-off-by Jeff LaBundy <[email protected]>
Applied, thank you.
Signature · 1 line
Dmitry
PATCH v2 2/2 dt-bindings: input: iqs626a: Redefine trackpad property types
In reply to Jeff LaBundyFollowing a recent refactor of the driver to properly drop unused device nodes, the driver expects trackpad channel touch thresholds and ATI base values to be specified under single trackpad channel child nodes.
This enhancement moves both properties to scalar values as opposed to arrays, making their types consistent across bindings.
---
Changes in v2:
- Added to series
.../devicetree/bindings/input/iqs626a.yaml | 94 +++++++++++++------ 1 file changed, 67 insertions(+), 27 deletions(-)
Documentation/devicetree/bindings/input/iqs626a.yaml +67 -27
@@ -564,16 +564,6 @@ patternProperties: 2: Partial 3: Full - azoteq,ati-base: - $ref: /schemas/types.yaml#/definitions/uint32-array - minItems: 6 - maxItems: 9 - items: - minimum: 45 - maximum: 300 - default: [45, 45, 45, 45, 45, 45, 45, 45, 45] - description: Specifies each individual trackpad channel's ATI base. - azoteq,ati-target: $ref: /schemas/types.yaml#/definitions/uint32 multipleOf: 32
@@ -620,17 +610,6 @@ patternProperties: description: Tightens the ATI band from 1/8 to 1/16 of the desired target. - azoteq,thresh: - $ref: /schemas/types.yaml#/definitions/uint32-array - minItems: 6 - maxItems: 9 - items: - minimum: 0 - maximum: 255 - default: [0, 0, 0, 0, 0, 0, 0, 0, 0] - description: - Specifies each individual trackpad channel's touch threshold. - azoteq,hyst: $ref: /schemas/types.yaml#/definitions/uint32 minimum: 0
@@ -720,6 +699,28 @@ patternProperties: Specifies the number of points across which an axial gesture must travel in order to be interpreted as a flick or swipe. + patternProperties: + "^channel-[0-8]$": + type: object + description: Represents a single trackpad channel. + + properties: + azoteq,thresh: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 255 + default: 0 + description: Specifies the threshold for the channel. + + azoteq,ati-base: + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 45 + maximum: 300 + default: 45 + description: Specifies the channel's ATI base. + + additionalProperties: false + dependencies: azoteq,gesture-swipe: ["linux,keycodes"] azoteq,timeout-tap-ms: ["linux,keycodes"]
@@ -780,14 +781,8 @@ examples: azoteq,filt-str-lp-cnt = <1>; azoteq,hyst = <4>; - azoteq,thresh = <35>, <40>, <40>, - <38>, <33>, <38>, - <35>, <35>, <35>; azoteq,ati-mode = <3>; - azoteq,ati-base = <195>, <195>, <195>, - <195>, <195>, <195>, - <195>, <195>, <195>; azoteq,ati-target = <512>; azoteq,proj-bias = <1>;
@@ -804,6 +799,51 @@ examples: azoteq,timeout-swipe-ms = <800>; azoteq,timeout-tap-ms = <400>; azoteq,thresh-swipe = <40>; + + channel-0 { + azoteq,thresh = <35>; + azoteq,ati-base = <195>; + }; + + channel-1 { + azoteq,thresh = <40>; + azoteq,ati-base = <195>; + }; + + channel-2 { + azoteq,thresh = <40>; + azoteq,ati-base = <195>; + }; + + channel-3 { + azoteq,thresh = <38>; + azoteq,ati-base = <195>; + }; + + channel-4 { + azoteq,thresh = <33>; + azoteq,ati-base = <195>; + }; + + channel-5 { + azoteq,thresh = <38>; + azoteq,ati-base = <195>; + }; + + channel-6 { + azoteq,thresh = <35>; + azoteq,ati-base = <195>; + }; + + channel-7 { + azoteq,thresh = <35>; + azoteq,ati-base = <195>; + }; + + channel-8 { + azoteq,thresh = <35>; + azoteq,ati-base = <195>; + }; }; /*
Signature · 1 line
2.34.1
Re: [PATCH v2 2/2] dt-bindings: input: iqs626a: Redefine trackpad property types
Quoted 8 lines · Jeff LaBundy
Following a recent refactor of the driver to properly drop unused device nodes, the driver expects trackpad channel touch thresholds and ATI base values to be specified under single trackpad channel child nodes.
This enhancement moves both properties to scalar values as opposed to arrays, making their types consistent across bindings.
Quoted 9 lines
Fixes a8f1f0dc865c ("dt-bindings: input: Add bindings for Azoteq IQS626A")Signed-off-by Jeff LaBundy <[email protected]>---
Changes in v2:
- Added to series.../devicetree/bindings/input/iqs626a.yaml | 94 +++++++++++++------ 1 file changed, 67 insertions(+), 27 deletions(-)
Re: [PATCH v2 2/2] dt-bindings: input: iqs626a: Redefine trackpad property types
In reply to Jeff LaBundyQuoted 10 lines · Jeff LaBundy
Following a recent refactor of the driver to properly drop unused device nodes, the driver expects trackpad channel touch thresholds and ATI base values to be specified under single trackpad channel child nodes.
This enhancement moves both properties to scalar values as opposed to arrays, making their types consistent across bindings.
Fixes a8f1f0dc865c ("dt-bindings: input: Add bindings for Azoteq IQS626A")Signed-off-by Jeff LaBundy <[email protected]>
Applied, thank you.
Signature · 1 line
Dmitry