Convert misc input drivers to use new cleanup facilities

Using guard notation makes the code more compact and error handling more robust by ensuring that mutexes are released in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/ad714x.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

drivers/input/misc/ad714x.c +3 -7

index 1acd8429c56c..d106f37df6bc 100644
@@ -941,7 +941,7 @@ static irqreturn_t ad714x_interrupt_thread(int irq, void *data)
 	struct ad714x_chip *ad714x = data;
 	int i;
 
-	mutex_lock(&ad714x->mutex);
+	guard(mutex)(&ad714x->mutex);
 
 	ad714x->read(ad714x, STG_LOW_INT_STA_REG, &ad714x->l_state, 3);
 

@@ -954,8 +954,6 @@ static irqreturn_t ad714x_interrupt_thread(int irq, void *data)
 	for (i = 0; i < ad714x->hw->touchpad_num; i++)
 		ad714x_touchpad_state_machine(ad714x, i);
 
-	mutex_unlock(&ad714x->mutex);
-
 	return IRQ_HANDLED;
 }
 

@@ -1169,13 +1167,11 @@ static int ad714x_suspend(struct device *dev)
 
 	dev_dbg(ad714x->dev, "%s enter\n", __func__);
 
-	mutex_lock(&ad714x->mutex);
+	guard(mutex)(&ad714x->mutex);
 
 	data = ad714x->hw->sys_cfg_reg[AD714X_PWR_CTRL] | 0x3;
 	ad714x->write(ad714x, AD714X_PWR_CTRL, data);
 
-	mutex_unlock(&ad714x->mutex);
-
 	return 0;
 }
 

@@ -1184,7 +1180,7 @@ static int ad714x_resume(struct device *dev)
 	struct ad714x_chip *ad714x = dev_get_drvdata(dev);
 	dev_dbg(ad714x->dev, "%s enter\n", __func__);
 
-	mutex_lock(&ad714x->mutex);
+	guard(mutex)(&ad714x->mutex);
 
 	/* resume to non-shutdown mode */
 

Using guard notation makes the code more compact and error handling more robust by ensuring that mutexes are released in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/ati_remote2.c | 57 +++++++++++---------------------
 1 file changed, 19 insertions(+), 38 deletions(-)

drivers/input/misc/ati_remote2.c +19 -35

index 795f69edb4b2..e84649af801d 100644
@@ -244,29 +244,21 @@ static int ati_remote2_open(struct input_dev *idev)
 	if (r) {
 		dev_err(&ar2->intf[0]->dev,
 			"%s(): usb_autopm_get_interface() = %d\n", __func__, r);
-		goto fail1;
+		return r;
 	}
 
-	mutex_lock(&ati_remote2_mutex);
+	scoped_guard(mutex, &ati_remote2_mutex) {
+		if (!(ar2->flags & ATI_REMOTE2_SUSPENDED)) {
+			r = ati_remote2_submit_urbs(ar2);
+			if (r)
+				break;
+		}
 
-	if (!(ar2->flags & ATI_REMOTE2_SUSPENDED)) {
-		r = ati_remote2_submit_urbs(ar2);
-		if (r)
-			goto fail2;
+		ar2->flags |= ATI_REMOTE2_OPENED;
 	}
 
-	ar2->flags |= ATI_REMOTE2_OPENED;
-
-	mutex_unlock(&ati_remote2_mutex);
-
 	usb_autopm_put_interface(ar2->intf[0]);
 
-	return 0;
-
- fail2:
-	mutex_unlock(&ati_remote2_mutex);
-	usb_autopm_put_interface(ar2->intf[0]);
- fail1:
 	return r;
 }
 

@@ -276,14 +268,12 @@ static void ati_remote2_close(struct input_dev *idev)
 
 	dev_dbg(&ar2->intf[0]->dev, "%s()\n", __func__);
 
-	mutex_lock(&ati_remote2_mutex);
+	guard(mutex)(&ati_remote2_mutex);
 
 	if (!(ar2->flags & ATI_REMOTE2_SUSPENDED))
 		ati_remote2_kill_urbs(ar2);
 
 	ar2->flags &= ~ATI_REMOTE2_OPENED;
-
-	mutex_unlock(&ati_remote2_mutex);
 }
 
 static void ati_remote2_input_mouse(struct ati_remote2 *ar2)

@@ -713,16 +703,14 @@ static ssize_t ati_remote2_store_channel_mask(struct device *dev,
 		return r;
 	}
 
-	mutex_lock(&ati_remote2_mutex);
-
-	if (mask != ar2->channel_mask) {
-		r = ati_remote2_setup(ar2, mask);
-		if (!r)
-			ar2->channel_mask = mask;
+	scoped_guard(mutex, &ati_remote2_mutex) {
+		if (mask != ar2->channel_mask) {
+			r = ati_remote2_setup(ar2, mask);
+			if (!r)
+				ar2->channel_mask = mask;
+		}
 	}
 
-	mutex_unlock(&ati_remote2_mutex);
-
 	usb_autopm_put_interface(ar2->intf[0]);
 
 	return r ? r : count;

@@ -892,15 +880,13 @@ static int ati_remote2_suspend(struct usb_interface *interface,
 
 	dev_dbg(&ar2->intf[0]->dev, "%s()\n", __func__);
 
-	mutex_lock(&ati_remote2_mutex);
+	guard(mutex)(&ati_remote2_mutex);
 
 	if (ar2->flags & ATI_REMOTE2_OPENED)
 		ati_remote2_kill_urbs(ar2);
 
 	ar2->flags |= ATI_REMOTE2_SUSPENDED;
 
-	mutex_unlock(&ati_remote2_mutex);
-
 	return 0;
 }
 

@@ -917,7 +903,7 @@ static int ati_remote2_resume(struct usb_interface *interface)
 
 	dev_dbg(&ar2->intf[0]->dev, "%s()\n", __func__);
 
-	mutex_lock(&ati_remote2_mutex);
+	guard(mutex)(&ati_remote2_mutex);
 
 	if (ar2->flags & ATI_REMOTE2_OPENED)
 		r = ati_remote2_submit_urbs(ar2);

@@ -925,8 +911,6 @@ static int ati_remote2_resume(struct usb_interface *interface)
 	if (!r)
 		ar2->flags &= ~ATI_REMOTE2_SUSPENDED;
 
-	mutex_unlock(&ati_remote2_mutex);
-
 	return r;
 }
 

@@ -943,11 +927,11 @@ static int ati_remote2_reset_resume(struct usb_interface *interface)
 
 	dev_dbg(&ar2->intf[0]->dev, "%s()\n", __func__);
 
-	mutex_lock(&ati_remote2_mutex);
+	guard(mutex)(&ati_remote2_mutex);
 
 	r = ati_remote2_setup(ar2, ar2->channel_mask);
 	if (r)
-		goto out;
+		return r;
 
 	if (ar2->flags & ATI_REMOTE2_OPENED)
 		r = ati_remote2_submit_urbs(ar2);

Using guard notation makes the code more compact and error handling more robust by ensuring that locks are released in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/cm109.c | 167 ++++++++++++++++++-------------------
 1 file changed, 79 insertions(+), 88 deletions(-)

drivers/input/misc/cm109.c +77 -86

index 728325a2d574..0cfe5d4a573c 100644
@@ -355,6 +355,35 @@ static void cm109_submit_buzz_toggle(struct cm109_dev *dev)
 			__func__, error);
 }
 
+static void cm109_submit_ctl(struct cm109_dev *dev)
+{
+	int error;
+
+	guard(spinlock_irqsave)(&dev->ctl_submit_lock);
+
+	dev->irq_urb_pending = 0;
+
+	if (unlikely(dev->shutdown))
+		return;
+
+	if (dev->buzzer_state)
+		dev->ctl_data->byte[HID_OR0] |= BUZZER_ON;
+	else
+		dev->ctl_data->byte[HID_OR0] &= ~BUZZER_ON;
+
+	dev->ctl_data->byte[HID_OR1] = dev->keybit;
+	dev->ctl_data->byte[HID_OR2] = dev->keybit;
+
+	dev->buzzer_pending = 0;
+	dev->ctl_urb_pending = 1;
+
+	error = usb_submit_urb(dev->urb_ctl, GFP_ATOMIC);
+	if (error)
+		dev_err(&dev->intf->dev,
+			"%s: usb_submit_urb (urb_ctl) failed %d\n",
+			__func__, error);
+}
+
 /*
  * IRQ handler
  */

@@ -362,8 +391,6 @@ static void cm109_urb_irq_callback(struct urb *urb)
 {
 	struct cm109_dev *dev = urb->context;
 	const int status = urb->status;
-	int error;
-	unsigned long flags;
 
 	dev_dbg(&dev->intf->dev, "### URB IRQ: [0x%02x 0x%02x 0x%02x 0x%02x] keybit=0x%02x\n",
 	     dev->irq_data->byte[0],

@@ -401,32 +428,7 @@ static void cm109_urb_irq_callback(struct urb *urb)
 	}
 
  out:
-
-	spin_lock_irqsave(&dev->ctl_submit_lock, flags);
-
-	dev->irq_urb_pending = 0;
-
-	if (likely(!dev->shutdown)) {
-
-		if (dev->buzzer_state)
-			dev->ctl_data->byte[HID_OR0] |= BUZZER_ON;
-		else
-			dev->ctl_data->byte[HID_OR0] &= ~BUZZER_ON;
-
-		dev->ctl_data->byte[HID_OR1] = dev->keybit;
-		dev->ctl_data->byte[HID_OR2] = dev->keybit;
-
-		dev->buzzer_pending = 0;
-		dev->ctl_urb_pending = 1;
-
-		error = usb_submit_urb(dev->urb_ctl, GFP_ATOMIC);
-		if (error)
-			dev_err(&dev->intf->dev,
-				"%s: usb_submit_urb (urb_ctl) failed %d\n",
-				__func__, error);
-	}
-
-	spin_unlock_irqrestore(&dev->ctl_submit_lock, flags);
+	cm109_submit_ctl(dev);
 }
 
 static void cm109_urb_ctl_callback(struct urb *urb)

@@ -434,7 +436,6 @@ static void cm109_urb_ctl_callback(struct urb *urb)
 	struct cm109_dev *dev = urb->context;
 	const int status = urb->status;
 	int error;
-	unsigned long flags;
 
 	dev_dbg(&dev->intf->dev, "### URB CTL: [0x%02x 0x%02x 0x%02x 0x%02x]\n",
 	     dev->ctl_data->byte[0],

@@ -449,35 +450,31 @@ static void cm109_urb_ctl_callback(struct urb *urb)
 				    __func__, status);
 	}
 
-	spin_lock_irqsave(&dev->ctl_submit_lock, flags);
+	guard(spinlock_irqsave)(&dev->ctl_submit_lock);
 
 	dev->ctl_urb_pending = 0;
 
-	if (likely(!dev->shutdown)) {
-
-		if (dev->buzzer_pending || status) {
-			dev->buzzer_pending = 0;
-			dev->ctl_urb_pending = 1;
-			cm109_submit_buzz_toggle(dev);
-		} else if (likely(!dev->irq_urb_pending)) {
-			/* ask for key data */
-			dev->irq_urb_pending = 1;
-			error = usb_submit_urb(dev->urb_irq, GFP_ATOMIC);
-			if (error)
-				dev_err(&dev->intf->dev,
-					"%s: usb_submit_urb (urb_irq) failed %d\n",
-					__func__, error);
-		}
-	}
+	if (unlikely(dev->shutdown))
+		return;
 
-	spin_unlock_irqrestore(&dev->ctl_submit_lock, flags);
+	if (dev->buzzer_pending || status) {
+		dev->buzzer_pending = 0;
+		dev->ctl_urb_pending = 1;
+		cm109_submit_buzz_toggle(dev);
+	} else if (likely(!dev->irq_urb_pending)) {
+		/* ask for key data */
+		dev->irq_urb_pending = 1;
+		error = usb_submit_urb(dev->urb_irq, GFP_ATOMIC);
+		if (error)
+			dev_err(&dev->intf->dev,
+				"%s: usb_submit_urb (urb_irq) failed %d\n",
+				__func__, error);
+	}
 }
 
 static void cm109_toggle_buzzer_async(struct cm109_dev *dev)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->ctl_submit_lock, flags);
+	guard(spinlock_irqsave)(&dev->ctl_submit_lock);
 
 	if (dev->ctl_urb_pending) {
 		/* URB completion will resubmit */

@@ -486,8 +483,6 @@ static void cm109_toggle_buzzer_async(struct cm109_dev *dev)
 		dev->ctl_urb_pending = 1;
 		cm109_submit_buzz_toggle(dev);
 	}
-
-	spin_unlock_irqrestore(&dev->ctl_submit_lock, flags);
 }
 
 static void cm109_toggle_buzzer_sync(struct cm109_dev *dev, int on)

@@ -556,32 +551,30 @@ static int cm109_input_open(struct input_dev *idev)
 		return error;
 	}
 
-	mutex_lock(&dev->pm_mutex);
-
-	dev->buzzer_state = 0;
-	dev->key_code = -1;	/* no keys pressed */
-	dev->keybit = 0xf;
+	scoped_guard(mutex, &dev->pm_mutex) {
+		dev->buzzer_state = 0;
+		dev->key_code = -1;	/* no keys pressed */
+		dev->keybit = 0xf;
 
-	/* issue INIT */
-	dev->ctl_data->byte[HID_OR0] = HID_OR_GPO_BUZ_SPDIF;
-	dev->ctl_data->byte[HID_OR1] = dev->keybit;
-	dev->ctl_data->byte[HID_OR2] = dev->keybit;
-	dev->ctl_data->byte[HID_OR3] = 0x00;
+		/* issue INIT */
+		dev->ctl_data->byte[HID_OR0] = HID_OR_GPO_BUZ_SPDIF;
+		dev->ctl_data->byte[HID_OR1] = dev->keybit;
+		dev->ctl_data->byte[HID_OR2] = dev->keybit;
+		dev->ctl_data->byte[HID_OR3] = 0x00;
 
-	dev->ctl_urb_pending = 1;
-	error = usb_submit_urb(dev->urb_ctl, GFP_KERNEL);
-	if (error) {
-		dev->ctl_urb_pending = 0;
-		dev_err(&dev->intf->dev, "%s: usb_submit_urb (urb_ctl) failed %d\n",
-			__func__, error);
-	} else {
-		dev->open = 1;
+		dev->ctl_urb_pending = 1;
+		error = usb_submit_urb(dev->urb_ctl, GFP_KERNEL);
+		if (!error) {
+			dev->open = 1;
+			return 0;
+		}
 	}
 
-	mutex_unlock(&dev->pm_mutex);
+	dev->ctl_urb_pending = 0;
+	usb_autopm_put_interface(dev->intf);
 
-	if (error)
-		usb_autopm_put_interface(dev->intf);
+	dev_err(&dev->intf->dev, "%s: usb_submit_urb (urb_ctl) failed %d\n",
+		__func__, error);
 
 	return error;
 }

@@ -590,17 +583,15 @@ static void cm109_input_close(struct input_dev *idev)
 {
 	struct cm109_dev *dev = input_get_drvdata(idev);
 
-	mutex_lock(&dev->pm_mutex);
-
-	/*
-	 * Once we are here event delivery is stopped so we
-	 * don't need to worry about someone starting buzzer
-	 * again
-	 */
-	cm109_stop_traffic(dev);
-	dev->open = 0;
-
-	mutex_unlock(&dev->pm_mutex);
+	scoped_guard(mutex, &dev->pm_mutex) {
+		/*
+		 * Once we are here event delivery is stopped so we
+		 * don't need to worry about someone starting buzzer
+		 * again
+		 */
+		cm109_stop_traffic(dev);
+		dev->open = 0;
+	}
 
 	usb_autopm_put_interface(dev->intf);
 }

@@ -823,9 +814,9 @@ static int cm109_usb_suspend(struct usb_interface *intf, pm_message_t message)
 
 	dev_info(&intf->dev, "cm109: usb_suspend (event=%d)\n", message.event);
 
-	mutex_lock(&dev->pm_mutex);
+	guard(mutex)(&dev->pm_mutex);
+
 	cm109_stop_traffic(dev);
-	mutex_unlock(&dev->pm_mutex);
 
 	return 0;
 }

@@ -836,9 +827,9 @@ static int cm109_usb_resume(struct usb_interface *intf)
 

Using guard notation makes the code more compact and error handling more robust by ensuring that mutexes are released in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/cma3000_d0x.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

drivers/input/misc/cma3000_d0x.c +4 -10

index 0c68e924a1cc..cfc12332bee1 100644
@@ -217,15 +217,13 @@ static int cma3000_open(struct input_dev *input_dev)
 {
 	struct cma3000_accl_data *data = input_get_drvdata(input_dev);
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 
 	if (!data->suspended)
 		cma3000_poweron(data);
 
 	data->opened = true;
 
-	mutex_unlock(&data->mutex);
-
 	return 0;
 }
 

@@ -233,40 +231,34 @@ static void cma3000_close(struct input_dev *input_dev)
 {
 	struct cma3000_accl_data *data = input_get_drvdata(input_dev);
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 
 	if (!data->suspended)
 		cma3000_poweroff(data);
 
 	data->opened = false;
-
-	mutex_unlock(&data->mutex);
 }
 
 void cma3000_suspend(struct cma3000_accl_data *data)
 {
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 
 	if (!data->suspended && data->opened)
 		cma3000_poweroff(data);
 
 	data->suspended = true;
-
-	mutex_unlock(&data->mutex);
 }
 EXPORT_SYMBOL(cma3000_suspend);
 
 
 void cma3000_resume(struct cma3000_accl_data *data)
 {
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);

Using guard notation makes the code more compact and error handling more robust by ensuring that locks are released in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/da7280.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

drivers/input/misc/da7280.c +11 -11

index 1629b7ea4cbd..e4a605c6af15 100644
@@ -1263,39 +1263,37 @@ static int da7280_suspend(struct device *dev)
 {
 	struct da7280_haptic *haptics = dev_get_drvdata(dev);
 
-	mutex_lock(&haptics->input_dev->mutex);
+	guard(mutex)(&haptics->input_dev->mutex);
 
 	/*
 	 * Make sure no new requests will be submitted while device is
 	 * suspended.
 	 */
-	spin_lock_irq(&haptics->input_dev->event_lock);
-	haptics->suspended = true;
-	spin_unlock_irq(&haptics->input_dev->event_lock);
+	scoped_guard(spinlock_irq, &haptics->input_dev->event_lock) {
+		haptics->suspended = true;
+	}
 
 	da7280_haptic_stop(haptics);
 
-	mutex_unlock(&haptics->input_dev->mutex);
-
 	return 0;
 }
 
 static int da7280_resume(struct device *dev)
 {
 	struct da7280_haptic *haptics = dev_get_drvdata(dev);
-	int retval;
+	int error;
 
-	mutex_lock(&haptics->input_dev->mutex);
+	guard(mutex)(&haptics->input_dev->mutex);
 
-	retval = da7280_haptic_start(haptics);
-	if (!retval) {
-		spin_lock_irq(&haptics->input_dev->event_lock);
+	error = da7280_haptic_start(haptics);
+	if (error)
+		return error;
+
+	scoped_guard(spinlock_irq, &haptics->input_dev->event_lock) {

Using guard notation makes the code more compact and error handling more robust by ensuring that mutexes are released and interrupts are re-enabled in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/kxtj9.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

drivers/input/misc/kxtj9.c +3 -8

index 837682cb2a7d..c6146bcee9f9 100644
@@ -314,9 +314,8 @@ static ssize_t kxtj9_set_poll(struct device *dev, struct device_attribute *attr,
 		return error;
 
 	/* Lock the device to prevent races with open/close (and itself) */
-	mutex_lock(&input_dev->mutex);
-
-	disable_irq(client->irq);
+	guard(mutex)(&input_dev->mutex);
+	guard(disable_irq)(&client->irq);
 
 	/*
 	 * Set current interval to the greater of the minimum interval or

@@ -326,9 +325,6 @@ static ssize_t kxtj9_set_poll(struct device *dev, struct device_attribute *attr,
 
 	kxtj9_update_odr(tj9, tj9->last_poll_interval);
 
-	enable_irq(client->irq);
-	mutex_unlock(&input_dev->mutex);
-
 	return count;
 }
 

@@ -504,12 +500,11 @@ static int kxtj9_suspend(struct device *dev)
 	struct kxtj9_data *tj9 = i2c_get_clientdata(client);
 	struct input_dev *input_dev = tj9->input_dev;
 
-	mutex_lock(&input_dev->mutex);
+	guard(mutex)(&input_dev->mutex);
 
 	if (input_device_enabled(input_dev))
 		kxtj9_disable(tj9);
 
-	mutex_unlock(&input_dev->mutex);
 	return 0;
 }
 

@@ -519,12 +514,11 @@ static int kxtj9_resume(struct device *dev)
 	struct kxtj9_data *tj9 = i2c_get_clientdata(client);
 	struct input_dev *input_dev = tj9->input_dev;
 

Using guard notation makes the code more compact and error handling more robust by ensuring that mutexes are released in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/drv260x.c | 50 +++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 26 deletions(-)

drivers/input/misc/drv260x.c +23 -23

index 61b503835aa6..96cd6a078c8a 100644
@@ -537,64 +537,62 @@ static int drv260x_probe(struct i2c_client *client)
 static int drv260x_suspend(struct device *dev)
 {
 	struct drv260x_data *haptics = dev_get_drvdata(dev);
-	int ret = 0;
+	int error;
 
-	mutex_lock(&haptics->input_dev->mutex);
+	guard(mutex)(&haptics->input_dev->mutex);
 
 	if (input_device_enabled(haptics->input_dev)) {
-		ret = regmap_update_bits(haptics->regmap,
-					 DRV260X_MODE,
-					 DRV260X_STANDBY_MASK,
-					 DRV260X_STANDBY);
-		if (ret) {
+		error = regmap_update_bits(haptics->regmap,
+					   DRV260X_MODE,
+					   DRV260X_STANDBY_MASK,
+					   DRV260X_STANDBY);
+		if (error) {
 			dev_err(dev, "Failed to set standby mode\n");
-			goto out;
+			return error;
 		}
 
 		gpiod_set_value(haptics->enable_gpio, 0);
 
-		ret = regulator_disable(haptics->regulator);
-		if (ret) {
+		error = regulator_disable(haptics->regulator);
+		if (error) {
 			dev_err(dev, "Failed to disable regulator\n");
 			regmap_update_bits(haptics->regmap,
 					   DRV260X_MODE,
 					   DRV260X_STANDBY_MASK, 0);
+			return error;
 		}
 	}
-out:
-	mutex_unlock(&haptics->input_dev->mutex);
-	return ret;
+
+	return 0;
 }
 
 static int drv260x_resume(struct device *dev)
 {
 	struct drv260x_data *haptics = dev_get_drvdata(dev);
-	int ret = 0;
+	int error;
 
-	mutex_lock(&haptics->input_dev->mutex);
+	guard(mutex)(&haptics->input_dev->mutex);
 
 	if (input_device_enabled(haptics->input_dev)) {
-		ret = regulator_enable(haptics->regulator);
-		if (ret) {
+		error = regulator_enable(haptics->regulator);
+		if (error) {
 			dev_err(dev, "Failed to enable regulator\n");
-			goto out;
+			return error;
 		}
 
-		ret = regmap_update_bits(haptics->regmap,
-					 DRV260X_MODE,
-					 DRV260X_STANDBY_MASK, 0);
-		if (ret) {
+		error = regmap_update_bits(haptics->regmap,
+					   DRV260X_MODE,
+					   DRV260X_STANDBY_MASK, 0);
+		if (error) {
 			dev_err(dev, "Failed to unset standby mode\n");
 			regulator_disable(haptics->regulator);
-			goto out;
+			return error;
 		}
 

Using guard notation makes the code more compact and error handling more robust by ensuring that mutexes are released in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/drv2665.c | 44 +++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 23 deletions(-)

drivers/input/misc/drv2665.c +20 -20

index f98e4d765307..77ec96c7db76 100644
@@ -225,59 +225,57 @@ static int drv2665_probe(struct i2c_client *client)
 static int drv2665_suspend(struct device *dev)
 {
 	struct drv2665_data *haptics = dev_get_drvdata(dev);
-	int ret = 0;
+	int error;
 
-	mutex_lock(&haptics->input_dev->mutex);
+	guard(mutex)(&haptics->input_dev->mutex);
 
 	if (input_device_enabled(haptics->input_dev)) {
-		ret = regmap_update_bits(haptics->regmap, DRV2665_CTRL_2,
-					 DRV2665_STANDBY, DRV2665_STANDBY);
-		if (ret) {
+		error = regmap_update_bits(haptics->regmap, DRV2665_CTRL_2,
+					   DRV2665_STANDBY, DRV2665_STANDBY);
+		if (error) {
 			dev_err(dev, "Failed to set standby mode\n");
 			regulator_disable(haptics->regulator);
-			goto out;
+			return error;
 		}
 
-		ret = regulator_disable(haptics->regulator);
-		if (ret) {
+		error = regulator_disable(haptics->regulator);
+		if (error) {
 			dev_err(dev, "Failed to disable regulator\n");
 			regmap_update_bits(haptics->regmap,
 					   DRV2665_CTRL_2,
 					   DRV2665_STANDBY, 0);
+			return error;
 		}
 	}
-out:
-	mutex_unlock(&haptics->input_dev->mutex);
-	return ret;
+
+	return 0;
 }
 
 static int drv2665_resume(struct device *dev)
 {
 	struct drv2665_data *haptics = dev_get_drvdata(dev);
-	int ret = 0;
+	int error;
 
-	mutex_lock(&haptics->input_dev->mutex);
+	guard(mutex)(&haptics->input_dev->mutex);
 
 	if (input_device_enabled(haptics->input_dev)) {
-		ret = regulator_enable(haptics->regulator);
-		if (ret) {
+		error = regulator_enable(haptics->regulator);
+		if (error) {
 			dev_err(dev, "Failed to enable regulator\n");
-			goto out;
+			return error;
 		}
 
-		ret = regmap_update_bits(haptics->regmap, DRV2665_CTRL_2,
-					 DRV2665_STANDBY, 0);
-		if (ret) {
+		error = regmap_update_bits(haptics->regmap, DRV2665_CTRL_2,
+					   DRV2665_STANDBY, 0);
+		if (error) {
 			dev_err(dev, "Failed to unset standby mode\n");
 			regulator_disable(haptics->regulator);
-			goto out;
+			return error;
 		}

Using guard notation makes the code more compact and error handling more robust by ensuring that mutexes are released in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/drv2667.c | 44 +++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 23 deletions(-)

drivers/input/misc/drv2667.c +20 -20

index ad49845374b9..906292625f84 100644
@@ -402,59 +402,57 @@ static int drv2667_probe(struct i2c_client *client)
 static int drv2667_suspend(struct device *dev)
 {
 	struct drv2667_data *haptics = dev_get_drvdata(dev);
-	int ret = 0;
+	int error;
 
-	mutex_lock(&haptics->input_dev->mutex);
+	guard(mutex)(&haptics->input_dev->mutex);
 
 	if (input_device_enabled(haptics->input_dev)) {
-		ret = regmap_update_bits(haptics->regmap, DRV2667_CTRL_2,
-					 DRV2667_STANDBY, DRV2667_STANDBY);
-		if (ret) {
+		error = regmap_update_bits(haptics->regmap, DRV2667_CTRL_2,
+					   DRV2667_STANDBY, DRV2667_STANDBY);
+		if (error) {
 			dev_err(dev, "Failed to set standby mode\n");
 			regulator_disable(haptics->regulator);
-			goto out;
+			return error;
 		}
 
-		ret = regulator_disable(haptics->regulator);
-		if (ret) {
+		error = regulator_disable(haptics->regulator);
+		if (error) {
 			dev_err(dev, "Failed to disable regulator\n");
 			regmap_update_bits(haptics->regmap,
 					   DRV2667_CTRL_2,
 					   DRV2667_STANDBY, 0);
+			return error;
 		}
 	}
-out:
-	mutex_unlock(&haptics->input_dev->mutex);
-	return ret;
+
+	return 0;
 }
 
 static int drv2667_resume(struct device *dev)
 {
 	struct drv2667_data *haptics = dev_get_drvdata(dev);
-	int ret = 0;
+	int error;
 
-	mutex_lock(&haptics->input_dev->mutex);
+	guard(mutex)(&haptics->input_dev->mutex);
 
 	if (input_device_enabled(haptics->input_dev)) {
-		ret = regulator_enable(haptics->regulator);
-		if (ret) {
+		error = regulator_enable(haptics->regulator);
+		if (error) {
 			dev_err(dev, "Failed to enable regulator\n");
-			goto out;
+			return error;
 		}
 
-		ret = regmap_update_bits(haptics->regmap, DRV2667_CTRL_2,
-					 DRV2667_STANDBY, 0);
-		if (ret) {
+		error = regmap_update_bits(haptics->regmap, DRV2667_CTRL_2,
+					   DRV2667_STANDBY, 0);
+		if (error) {
 			dev_err(dev, "Failed to unset standby mode\n");
 			regulator_disable(haptics->regulator);
-			goto out;
+			return error;
 		}

Using guard notation makes the code more compact and error handling more robust by ensuring that locks are released in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/ideapad_slidebar.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

drivers/input/misc/ideapad_slidebar.c +4 -15

index fa4e7f67d713..592bd159a194 100644
@@ -95,41 +95,29 @@ static struct platform_device *slidebar_platform_dev;
 
 static u8 slidebar_pos_get(void)
 {
-	u8 res;
-	unsigned long flags;
+	guard(spinlock_irqsave)(&io_lock);
 
-	spin_lock_irqsave(&io_lock, flags);
 	outb(0xf4, 0xff29);
 	outb(0xbf, 0xff2a);
-	res = inb(0xff2b);
-	spin_unlock_irqrestore(&io_lock, flags);
-
-	return res;
+	return inb(0xff2b);
 }
 
 static u8 slidebar_mode_get(void)
 {
-	u8 res;
-	unsigned long flags;
+	guard(spinlock_irqsave)(&io_lock);
 
-	spin_lock_irqsave(&io_lock, flags);
 	outb(0xf7, 0xff29);
 	outb(0x8b, 0xff2a);
-	res = inb(0xff2b);
-	spin_unlock_irqrestore(&io_lock, flags);
-
-	return res;
+	return inb(0xff2b);
 }
 
 static void slidebar_mode_set(u8 mode)
 {
-	unsigned long flags;

Using guard notation makes the code more compact and error handling more robust by ensuring that locks are released in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/ibm-panel.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

drivers/input/misc/ibm-panel.c +1 -2

index 867ac7aa10d2..aa48f62d7ea0 100644
@@ -77,12 +77,11 @@ static void ibm_panel_process_command(struct ibm_panel *panel)
 static int ibm_panel_i2c_slave_cb(struct i2c_client *client,
 				  enum i2c_slave_event event, u8 *val)
 {
-	unsigned long flags;
 	struct ibm_panel *panel = i2c_get_clientdata(client);
 
 	dev_dbg(&panel->input->dev, "event: %u data: %02x\n", event, *val);
 
-	spin_lock_irqsave(&panel->lock, flags);
+	guard(spinlock_irqsave)(&panel->lock);
 
 	switch (event) {
 	case I2C_SLAVE_STOP:

Using guard notation makes the code more compact and error handling more robust by ensuring that locks are released in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
  drivers/input/misc/ibm-panel.c | 5 +----
  1 file changed, 1 insertion(+), 4 deletions(-)

drivers/input/misc/ibm-panel.c +1 -4

index 867ac7aa10d2..aa48f62d7ea0 100644
@@ -77,12 +77,11 @@ static void ibm_panel_process_command(struct ibm_panel *panel)
  static int ibm_panel_i2c_slave_cb(struct i2c_client *client,
  				  enum i2c_slave_event event, u8 *val)
  {
-	unsigned long flags;
  	struct ibm_panel *panel = i2c_get_clientdata(client);
 
  	dev_dbg(&panel->input->dev, "event: %u data: %02x\n", event, *val);
 
-	spin_lock_irqsave(&panel->lock, flags);
+	guard(spinlock_irqsave)(&panel->lock);
 
  	switch (event) {
  	case I2C_SLAVE_STOP:

@@ -114,8 +113,6 @@ static int ibm_panel_i2c_slave_cb(struct i2c_client *client,
  		break;
  	}
 
-	spin_unlock_irqrestore(&panel->lock, flags);
-
  	return 0;
  }
 

Using guard notation makes the code more compact and error handling more robust by ensuring that mutexes are released in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/iqs269a.c | 46 +++++++++++++-----------------------
 1 file changed, 16 insertions(+), 30 deletions(-)

drivers/input/misc/iqs269a.c +0 -0

index 843f8a3f3410..c34d847fa4af 100644

drivers/input/misc/iqs269a.c +1 -4

-	mutex_unlock(&iqs269->lock);
 
 	switch (engine_b & IQS269_CHx_ENG_B_ATI_BASE_MASK) {
 	case IQS269_CHx_ENG_B_ATI_BASE_75:

@@ -491,7 +487,7 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
 	if (target > IQS269_CHx_ENG_B_ATI_TARGET_MAX)
 		return -EINVAL;
 
-	mutex_lock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);
 
 	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
 

@@ -501,8 +497,6 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
 	ch_reg[ch_num].engine_b = cpu_to_be16(engine_b);
 	iqs269->ati_current = false;
 
-	mutex_unlock(&iqs269->lock);
-
 	return 0;
 }
 

Quoted 13 lines · Dmitry Torokhov

Using guard notation makes the code more compact and error handling more robust by ensuring that mutexes are released in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/iqs269a.c | 46 +++++++++++++-----------------------
 1 file changed, 16 insertions(+), 30 deletions(-)

drivers/input/misc/iqs269a.c +0 -0

index 843f8a3f3410..c34d847fa4af 100644

...

Quoted 4 lines

drivers/input/misc/iqs269a.c +0 -0

@@ -453,9 +449,9 @@ static int iqs269_ati_base_get(struct iqs269_private *iqs269,
 	if (ch_num >= IQS269_NUM_CH)
 		return -EINVAL;
 

drivers/input/misc/iqs269a.c static int iqs269_ati_base_get(struct iqs269_private *iqs269, · line 454

Quoted 4 lines

drivers/input/misc/iqs269a.c +2 -1

-	mutex_lock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);
+
 	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);

maybe scoped_guard() to keep the scope of the mutex as it used to be?

Quoted 24 lines

drivers/input/misc/iqs269a.c +1 -4

-	mutex_unlock(&iqs269->lock);
 
 	switch (engine_b & IQS269_CHx_ENG_B_ATI_BASE_MASK) {
 	case IQS269_CHx_ENG_B_ATI_BASE_75:

@@ -491,7 +487,7 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
 	if (target > IQS269_CHx_ENG_B_ATI_TARGET_MAX)
 		return -EINVAL;
 
-	mutex_lock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);
 
 	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
 

@@ -501,8 +497,6 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
 	ch_reg[ch_num].engine_b = cpu_to_be16(engine_b);
 	iqs269->ati_current = false;
 
-	mutex_unlock(&iqs269->lock);
-
 	return 0;
 }
 

drivers/input/misc/iqs269a.c static int iqs269_ati_target_get(struct iqs269_private *iqs269, · line 512

Quoted 7 lines

drivers/input/misc/iqs269a.c +1 -3

@@ -515,10 +509,9 @@ static int iqs269_ati_target_get(struct iqs269_private *iqs269,
 	if (ch_num >= IQS269_NUM_CH)
 		return -EINVAL;
 
-	mutex_lock(&iqs269->lock);
-	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
-	mutex_unlock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);

same here?

Quoted 1 line

drivers/input/misc/iqs269a.c static int iqs269_ati_target_get(struct iqs269_private *iqs269, · line 515

Quoted 2 lines

drivers/input/misc/iqs269a.c +1 -0

+	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
 	*target = (engine_b & IQS269_CHx_ENG_B_ATI_TARGET_MASK) * 32;

Hi Javier,

Quoted 21 lines · Javier Carrasco

Quoted 13 lines · Dmitry Torokhov

Using guard notation makes the code more compact and error handling more robust by ensuring that mutexes are released in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/iqs269a.c | 46 +++++++++++++-----------------------
 1 file changed, 16 insertions(+), 30 deletions(-)

drivers/input/misc/iqs269a.c +0 -0

index 843f8a3f3410..c34d847fa4af 100644

...

Quoted 4 lines

drivers/input/misc/iqs269a.c +0 -0

@@ -453,9 +449,9 @@ static int iqs269_ati_base_get(struct iqs269_private *iqs269,
 	if (ch_num >= IQS269_NUM_CH)
 		return -EINVAL;
 

Quoted 6 lines

drivers/input/misc/iqs269a.c static int iqs269_ati_base_get(struct iqs269_private *iqs269, · line 454

Quoted 4 lines

drivers/input/misc/iqs269a.c +2 -1

-	mutex_lock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);
+
 	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);

maybe scoped_guard() to keep the scope of the mutex as it used to be?

Thank you for looking over patches.

It is just a few computations extra, so I decided not to use scoped_guard(). Note that original code was forced to release mutex early to avoid having to unlock it in all switch branches.

Quoted 35 lines

Quoted 24 lines

drivers/input/misc/iqs269a.c +1 -4

-	mutex_unlock(&iqs269->lock);
 
 	switch (engine_b & IQS269_CHx_ENG_B_ATI_BASE_MASK) {
 	case IQS269_CHx_ENG_B_ATI_BASE_75:

@@ -491,7 +487,7 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
 	if (target > IQS269_CHx_ENG_B_ATI_TARGET_MAX)
 		return -EINVAL;
 
-	mutex_lock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);
 
 	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
 

@@ -501,8 +497,6 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
 	ch_reg[ch_num].engine_b = cpu_to_be16(engine_b);
 	iqs269->ati_current = false;
 
-	mutex_unlock(&iqs269->lock);
-
 	return 0;
 }
 

drivers/input/misc/iqs269a.c static int iqs269_ati_target_get(struct iqs269_private *iqs269, · line 512

Quoted 7 lines

drivers/input/misc/iqs269a.c +1 -3

@@ -515,10 +509,9 @@ static int iqs269_ati_target_get(struct iqs269_private *iqs269,
 	if (ch_num >= IQS269_NUM_CH)
 		return -EINVAL;
 
-	mutex_lock(&iqs269->lock);
-	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
-	mutex_unlock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);

same here?

Quoted 1 line

Quoted 2 lines

drivers/input/misc/iqs269a.c static int iqs269_ati_target_get(struct iqs269_private *iqs269, · line 515

Quoted 2 lines

drivers/input/misc/iqs269a.c +1 -0

+	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
 	*target = (engine_b & IQS269_CHx_ENG_B_ATI_TARGET_MASK) * 32;

Same here, calculating the line above will take no time at all...

Thanks.

Quoted 72 lines · Dmitry Torokhov

Hi Javier,

Quoted 21 lines · Javier Carrasco

Quoted 13 lines · Dmitry Torokhov

Using guard notation makes the code more compact and error handling more robust by ensuring that mutexes are released in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/iqs269a.c | 46 +++++++++++++-----------------------
 1 file changed, 16 insertions(+), 30 deletions(-)

drivers/input/misc/iqs269a.c +0 -0

index 843f8a3f3410..c34d847fa4af 100644

...

Quoted 4 lines

drivers/input/misc/iqs269a.c +0 -0

@@ -453,9 +449,9 @@ static int iqs269_ati_base_get(struct iqs269_private *iqs269,
 	if (ch_num >= IQS269_NUM_CH)
 		return -EINVAL;
 

Quoted 6 lines

drivers/input/misc/iqs269a.c static int iqs269_ati_base_get(struct iqs269_private *iqs269, · line 454

Quoted 4 lines

drivers/input/misc/iqs269a.c +2 -1

-	mutex_lock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);
+
 	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);

maybe scoped_guard() to keep the scope of the mutex as it used to be?

Thank you for looking over patches.

It is just a few computations extra, so I decided not to use scoped_guard(). Note that original code was forced to release mutex early to avoid having to unlock it in all switch branches.

Quoted 35 lines

Quoted 24 lines

drivers/input/misc/iqs269a.c +1 -4

-	mutex_unlock(&iqs269->lock);
 
 	switch (engine_b & IQS269_CHx_ENG_B_ATI_BASE_MASK) {
 	case IQS269_CHx_ENG_B_ATI_BASE_75:

@@ -491,7 +487,7 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
 	if (target > IQS269_CHx_ENG_B_ATI_TARGET_MAX)
 		return -EINVAL;
 
-	mutex_lock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);
 
 	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
 

@@ -501,8 +497,6 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
 	ch_reg[ch_num].engine_b = cpu_to_be16(engine_b);
 	iqs269->ati_current = false;
 
-	mutex_unlock(&iqs269->lock);
-
 	return 0;
 }
 

drivers/input/misc/iqs269a.c static int iqs269_ati_target_get(struct iqs269_private *iqs269, · line 512

Quoted 7 lines

drivers/input/misc/iqs269a.c +1 -3

@@ -515,10 +509,9 @@ static int iqs269_ati_target_get(struct iqs269_private *iqs269,
 	if (ch_num >= IQS269_NUM_CH)
 		return -EINVAL;
 
-	mutex_lock(&iqs269->lock);
-	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
-	mutex_unlock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);

same here?

Quoted 1 line

Quoted 7 lines

Quoted 2 lines

drivers/input/misc/iqs269a.c static int iqs269_ati_target_get(struct iqs269_private *iqs269, · line 515

Quoted 2 lines

drivers/input/misc/iqs269a.c +1 -0

+	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
 	*target = (engine_b & IQS269_CHx_ENG_B_ATI_TARGET_MASK) * 32;

Same here, calculating the line above will take no time at all...

Thanks.

As you pointed out, in reality the extra locked instructions will not make any difference. But as the conversion added instructions to be locked by the mutex without mentioning it, I thought it should be either left as it used to be with scoped_guard(), or explicitly mentioned in the description.

No strong feelings against it, but out of curiosity, why would you rather use guard()? I think scoped_guard() is a better way to self-document what has to be accessed via mutex, and what not.

Using guard notation makes the code more compact and error handling more robust by ensuring that mutexes are released in all code paths when control leaves critical section.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/iqs269a.c | 46 +++++++++++++-----------------------
 1 file changed, 16 insertions(+), 30 deletions(-)

drivers/input/misc/iqs269a.c +16 -30

index 843f8a3f3410..c34d847fa4af 100644
@@ -365,7 +365,7 @@ static int iqs269_ati_mode_set(struct iqs269_private *iqs269,
 	if (mode > IQS269_CHx_ENG_A_ATI_MODE_MAX)
 		return -EINVAL;
 
-	mutex_lock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);
 
 	engine_a = be16_to_cpu(ch_reg[ch_num].engine_a);
 

@@ -375,8 +375,6 @@ static int iqs269_ati_mode_set(struct iqs269_private *iqs269,
 	ch_reg[ch_num].engine_a = cpu_to_be16(engine_a);
 	iqs269->ati_current = false;
 
-	mutex_unlock(&iqs269->lock);
-
 	return 0;
 }
 

@@ -389,9 +387,9 @@ static int iqs269_ati_mode_get(struct iqs269_private *iqs269,
 	if (ch_num >= IQS269_NUM_CH)
 		return -EINVAL;
 
-	mutex_lock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);
+
 	engine_a = be16_to_cpu(ch_reg[ch_num].engine_a);
-	mutex_unlock(&iqs269->lock);
 
 	engine_a &= IQS269_CHx_ENG_A_ATI_MODE_MASK;
 	*mode = (engine_a >> IQS269_CHx_ENG_A_ATI_MODE_SHIFT);

@@ -429,7 +427,7 @@ static int iqs269_ati_base_set(struct iqs269_private *iqs269,
 		return -EINVAL;
 	}
 
-	mutex_lock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);
 
 	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
 

@@ -439,8 +437,6 @@ static int iqs269_ati_base_set(struct iqs269_private *iqs269,
 	ch_reg[ch_num].engine_b = cpu_to_be16(engine_b);
 	iqs269->ati_current = false;
 
-	mutex_unlock(&iqs269->lock);
-
 	return 0;
 }
 

@@ -453,9 +449,9 @@ static int iqs269_ati_base_get(struct iqs269_private *iqs269,
 	if (ch_num >= IQS269_NUM_CH)
 		return -EINVAL;
 
-	mutex_lock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);
+
 	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
-	mutex_unlock(&iqs269->lock);
 
 	switch (engine_b & IQS269_CHx_ENG_B_ATI_BASE_MASK) {
 	case IQS269_CHx_ENG_B_ATI_BASE_75:

@@ -491,7 +487,7 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
 	if (target > IQS269_CHx_ENG_B_ATI_TARGET_MAX)
 		return -EINVAL;
 
-	mutex_lock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);
 
 	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
 

@@ -501,8 +497,6 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
 	ch_reg[ch_num].engine_b = cpu_to_be16(engine_b);
 	iqs269->ati_current = false;
 
-	mutex_unlock(&iqs269->lock);
-
 	return 0;
 }
 

@@ -515,10 +509,9 @@ static int iqs269_ati_target_get(struct iqs269_private *iqs269,
 	if (ch_num >= IQS269_NUM_CH)
 		return -EINVAL;
 
-	mutex_lock(&iqs269->lock);
-	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
-	mutex_unlock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);
 
+	engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
 	*target = (engine_b & IQS269_CHx_ENG_B_ATI_TARGET_MASK) * 32;
 
 	return 0;

@@ -1199,7 +1192,7 @@ static int iqs269_dev_init(struct iqs269_private *iqs269)
 {
 	int error;
 
-	mutex_lock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);
 
 	/*
 	 * Early revisions of silicon require the following workaround in order

@@ -1210,19 +1203,19 @@ static int iqs269_dev_init(struct iqs269_private *iqs269)
 		error = regmap_multi_reg_write(iqs269->regmap, iqs269_tws_init,
 					       ARRAY_SIZE(iqs269_tws_init));
 		if (error)
-			goto err_mutex;
+			return error;
 	}
 
 	error = regmap_update_bits(iqs269->regmap, IQS269_HALL_UI,
 				   IQS269_HALL_UI_ENABLE,
 				   iqs269->hall_enable ? ~0 : 0);
 	if (error)
-		goto err_mutex;
+		return error;
 
 	error = regmap_raw_write(iqs269->regmap, IQS269_SYS_SETTINGS,
 				 &iqs269->sys_reg, sizeof(iqs269->sys_reg));
 	if (error)
-		goto err_mutex;
+		return error;
 
 	/*
 	 * The following delay gives the device time to deassert its RDY output

@@ -1232,10 +1225,7 @@ static int iqs269_dev_init(struct iqs269_private *iqs269)
 
 	iqs269->ati_current = true;
 
-err_mutex:
-	mutex_unlock(&iqs269->lock);
-
-	return error;
+	return 0;
 }
 
 static int iqs269_input_init(struct iqs269_private *iqs269)

@@ -1580,13 +1570,11 @@ static ssize_t hall_enable_store(struct device *dev,
 	if (error)
 		return error;
 
-	mutex_lock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);
 
 	iqs269->hall_enable = val;
 	iqs269->ati_current = false;
 
-	mutex_unlock(&iqs269->lock);
-
 	return count;
 }
 

@@ -1643,13 +1631,11 @@ static ssize_t rx_enable_store(struct device *dev,
 	if (val > 0xFF)
 		return -EINVAL;
 
-	mutex_lock(&iqs269->lock);
+	guard(mutex)(&iqs269->lock);
 
 	ch_reg[iqs269->ch_num].rx_enable = val;
 	iqs269->ati_current = false;
 
-	mutex_unlock(&iqs269->lock);
-
 	return count;
 }
 

Signature · 1 line

2.46.0.469.g59c65b2a67-goog

Use __free(fwnode_handle) cleanup facility to ensure that references to acquired fwnodes are dropped at appropriate times automatically.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/iqs269a.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

drivers/input/misc/iqs269a.c +3 -5

index c34d847fa4af..1851848e2cd3 100644
@@ -550,7 +550,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;

@@ -727,8 +726,9 @@ 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);
+		struct fwnode_handle *ev_node __free(fwnode_handle) =
+			fwnode_get_named_child_node(ch_node,
+						    iqs269_events[i].name);
 		if (!ev_node)
 			continue;
 

@@ -737,7 +737,6 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269,
 				dev_err(&client->dev,
 					"Invalid channel %u threshold: %u\n",
 					reg, val);
-				fwnode_handle_put(ev_node);
 				return -EINVAL;
 			}
 

@@ -751,7 +750,6 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269,
 				dev_err(&client->dev,
 					"Invalid channel %u hysteresis: %u\n",
 					reg, val);
-				fwnode_handle_put(ev_node);
 				return -EINVAL;
 			}
 

Use __free(fwnode_handle) cleanup facility to ensure that references to acquired fwnodes are dropped at appropriate times automatically.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/iqs269a.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

drivers/input/misc/iqs269a.c +3 -6

index c34d847fa4af..1851848e2cd3 100644
@@ -550,7 +550,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;

@@ -727,8 +726,9 @@ 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);
+		struct fwnode_handle *ev_node __free(fwnode_handle) =
+			fwnode_get_named_child_node(ch_node,
+						    iqs269_events[i].name);
 		if (!ev_node)
 			continue;
 

@@ -737,7 +737,6 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269,
 				dev_err(&client->dev,
 					"Invalid channel %u threshold: %u\n",
 					reg, val);
-				fwnode_handle_put(ev_node);
 				return -EINVAL;
 			}
 

@@ -751,7 +750,6 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269,
 				dev_err(&client->dev,
 					"Invalid channel %u hysteresis: %u\n",
 					reg, val);
-				fwnode_handle_put(ev_node);
 				return -EINVAL;
 			}
 

@@ -767,7 +765,6 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269,
 		}
 
 		error = fwnode_property_read_u32(ev_node, "linux,code", &val);
-		fwnode_handle_put(ev_node);
 		if (error == -EINVAL) {
 			continue;
 		} else if (error) {

Signature · 1 line

2.46.0.469.g59c65b2a67-goog

Use __free(fwnode_handle) cleanup facility to ensure that references to acquired fwnodes are dropped at appropriate times automatically.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/iqs626a.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

drivers/input/misc/iqs626a.c +4 -15

index 0dab54d3a060..7a6e6927f331 100644
@@ -462,7 +462,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
 {
 	struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
 	struct i2c_client *client = iqs626->client;
-	struct fwnode_handle *ev_node;
 	const char *ev_name;
 	u8 *thresh, *hyst;
 	unsigned int val;

@@ -501,6 +500,7 @@ iqs626_parse_events(struct iqs626_private *iqs626,
 		if (!iqs626_channels[ch_id].events[i])
 			continue;
 
+		struct fwnode_handle *ev_node __free(fwnode_handle) = NULL;
 		if (ch_id == IQS626_CH_TP_2 || ch_id == IQS626_CH_TP_3) {
 			/*
 			 * Trackpad touch events are simply described under the

@@ -530,7 +530,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
 					dev_err(&client->dev,
 						"Invalid input type: %u\n",
 						val);
-					fwnode_handle_put(ev_node);
 					return -EINVAL;
 				}
 

@@ -545,7 +544,6 @@ 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;
 			}
 

@@ -566,7 +564,6 @@ 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;
 			}
 

@@ -575,8 +572,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
 			else
 				*(thresh + iqs626_events[i].th_offs) = val;
 		}
-
-		fwnode_handle_put(ev_node);
 	}
 
 	return 0;

@@ -774,12 +769,12 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
 	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);
+		struct fwnode_handle *tc_node __free(fwnode_handle) =
+				fwnode_get_named_child_node(ch_node, tc_name);
 		if (!tc_node)
 			continue;
 

@@ -790,7 +785,6 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
 				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;
 			}
 

@@ -803,14 +797,11 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
 				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"))

@@ -1233,7 +1224,6 @@ static int iqs626_parse_prop(struct iqs626_private *iqs626)
 {
 	struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
 	struct i2c_client *client = iqs626->client;
-	struct fwnode_handle *ch_node;
 	unsigned int val;
 	int error, i;
 	u16 general;

@@ -1375,13 +1365,13 @@ static int iqs626_parse_prop(struct iqs626_private *iqs626)
 	sys_reg->active = 0;
 
 	for (i = 0; i < ARRAY_SIZE(iqs626_channels); i++) {
-		ch_node = device_get_named_child_node(&client->dev,
-						      iqs626_channels[i].name);
+		struct fwnode_handle *ch_node __free(fwnode_handle) =

Use __free(fwnode_handle) cleanup facility to ensure that references to acquired fwnodes are dropped at appropriate times automatically.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/iqs626a.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

drivers/input/misc/iqs626a.c +0 -1

index 0dab54d3a060..7a6e6927f331 100644
@@ -462,7 +462,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
 {
 	struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
 	struct i2c_client *client = iqs626->client;
-	struct fwnode_handle *ev_node;
 	const char *ev_name;
 	u8 *thresh, *hyst;
 	unsigned int val;

drivers/input/misc/iqs626a.c +0 -5

 		if (ch_id == IQS626_CH_TP_2 || ch_id == IQS626_CH_TP_3) {
 			/*
 			 * Trackpad touch events are simply described under the

@@ -530,7 +530,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
 					dev_err(&client->dev,
 						"Invalid input type: %u\n",
 						val);
-					fwnode_handle_put(ev_node);
 					return -EINVAL;
 				}
 

@@ -545,7 +544,6 @@ 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;
 			}
 

@@ -566,7 +564,6 @@ 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;
 			}
 

@@ -575,8 +572,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
 			else
 				*(thresh + iqs626_events[i].th_offs) = val;
 		}
-
-		fwnode_handle_put(ev_node);
 	}
 
 	return 0;

@@ -774,12 +769,12 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
 	for (i = 0; i < iqs626_channels[ch_id].num_ch; i++) {

drivers/input/misc/iqs626a.c +3 -8

 		if (!tc_node)
 			continue;
 

@@ -790,7 +785,6 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
 				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;
 			}
 

@@ -803,14 +797,11 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
 				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"))

@@ -1233,7 +1224,6 @@ static int iqs626_parse_prop(struct iqs626_private *iqs626)
 {
 	struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
 	struct i2c_client *client = iqs626->client;
-	struct fwnode_handle *ch_node;
 	unsigned int val;
 	int error, i;
 	u16 general;

@@ -1375,13 +1365,13 @@ static int iqs626_parse_prop(struct iqs626_private *iqs626)
 	sys_reg->active = 0;
 
 	for (i = 0; i < ARRAY_SIZE(iqs626_channels); i++) {
-		ch_node = device_get_named_child_node(&client->dev,
-						      iqs626_channels[i].name);
+		struct fwnode_handle *ch_node __free(fwnode_handle) =
+			device_get_named_child_node(&client->dev,
+						    iqs626_channels[i].name);
 		if (!ch_node)
 			continue;
 
 		error = iqs626_parse_channel(iqs626, ch_node, i);
-		fwnode_handle_put(ch_node);
 		if (error)
 			return error;
 

Signature · 1 line

2.46.0.469.g59c65b2a67-goog

Hi Dmitry,

Quoted 24 lines · Dmitry Torokhov

Use __free(fwnode_handle) cleanup facility to ensure that references to acquired fwnodes are dropped at appropriate times automatically.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/iqs626a.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

drivers/input/misc/iqs626a.c +0 -1

index 0dab54d3a060..7a6e6927f331 100644
@@ -462,7 +462,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
 {
 	struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
 	struct i2c_client *client = iqs626->client;
-	struct fwnode_handle *ev_node;
 	const char *ev_name;
 	u8 *thresh, *hyst;
 	unsigned int val;

@@ -501,6 +500,7 @@ iqs626_parse_events(struct iqs626_private *iqs626,
 		if (!iqs626_channels[ch_id].events[i])
 			continue;
 

drivers/input/misc/iqs626a.c iqs626_parse_events(struct iqs626_private *iqs626, · line 503

Quoted 1 line

drivers/input/misc/iqs626a.c +1 -0

+		struct fwnode_handle *ev_node __free(fwnode_handle) = NULL;

This seems to deviate from what I understand to be a more conventional style of declaring variables at the top of the scope, and separate from actual code, like below:

	for (i = 0; i < ARRAY_SIZE(iqs626_events); i++) {
		struct fwnode_handle *ev_node __free(fwnode_handle);

		if (!iqs626_channels[ch_id].events[i])
			continue;

I also did not see any reason to explicitly declare the variable as NULL; let me know in case I have misunderstood.

Quoted 44 lines

drivers/input/misc/iqs626a.c +0 -6

 		if (ch_id == IQS626_CH_TP_2 || ch_id == IQS626_CH_TP_3) {
 			/*
 			 * Trackpad touch events are simply described under the

@@ -530,7 +530,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
 					dev_err(&client->dev,
 						"Invalid input type: %u\n",
 						val);
-					fwnode_handle_put(ev_node);
 					return -EINVAL;
 				}
 

@@ -545,7 +544,6 @@ 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;
 			}
 

@@ -566,7 +564,6 @@ 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;
 			}
 

@@ -575,8 +572,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
 			else
 				*(thresh + iqs626_events[i].th_offs) = val;
 		}
-
-		fwnode_handle_put(ev_node);
 	}
 
 	return 0;

@@ -774,12 +769,12 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
 	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];
 

drivers/input/misc/iqs626a.c static int iqs626_parse_trackpad(struct iqs626_private *iqs626, · line 777

Quoted 5 lines

drivers/input/misc/iqs626a.c +2 -1

 		snprintf(tc_name, sizeof(tc_name), "channel-%d", i);
 
-		tc_node = fwnode_get_named_child_node(ch_node, tc_name);
+		struct fwnode_handle *tc_node __free(fwnode_handle) =
+				fwnode_get_named_child_node(ch_node, tc_name);

Same here.

Hi Jeff,

Quoted 27 lines · Jeff LaBundy

Hi Dmitry,

Quoted 24 lines · Dmitry Torokhov

Use __free(fwnode_handle) cleanup facility to ensure that references to acquired fwnodes are dropped at appropriate times automatically.

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/iqs626a.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

drivers/input/misc/iqs626a.c +0 -1

index 0dab54d3a060..7a6e6927f331 100644
@@ -462,7 +462,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
 {
 	struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
 	struct i2c_client *client = iqs626->client;
-	struct fwnode_handle *ev_node;
 	const char *ev_name;
 	u8 *thresh, *hyst;
 	unsigned int val;

@@ -501,6 +500,7 @@ iqs626_parse_events(struct iqs626_private *iqs626,
 		if (!iqs626_channels[ch_id].events[i])
 			continue;
 

Quoted 5 lines

drivers/input/misc/iqs626a.c iqs626_parse_events(struct iqs626_private *iqs626, · line 503

Quoted 1 line

drivers/input/misc/iqs626a.c +1 -0

+		struct fwnode_handle *ev_node __free(fwnode_handle) = NULL;

This seems to deviate from what I understand to be a more conventional style of declaring variables at the top of the scope, and separate from actual code, like below:

This is follows Linus' guidance on combining declaration and initialization for variables using __free() cleanup annotations (where possible). These annotations are the reason for dropping -Wdeclaration-after-statement from our makefiles. See b5ec6fd286df ("kbuild: Drop -Wdeclaration-after-statement") and discussion in
https://lore.kernel.org/all/CAHk-=wi-RyoUhbChiVaJZoZXheAwnJ7OO=Gxe85BkPAd93TwDA@mail.gmail.com

Quoted 6 lines

	for (i = 0; i < ARRAY_SIZE(iqs626_events); i++) {
		struct fwnode_handle *ev_node __free(fwnode_handle);

Quoted 2 lines

		if (!iqs626_channels[ch_id].events[i])
			continue;

This will result in attempt to "put" a garbage pointer if we follow "continue" code path. In general __free() pointers have to be initialized, either to NULL or to a valid object (assuming that cleanup function can deal with NULLs).

Quoted 4 lines

I also did not see any reason to explicitly declare the variable as NULL; let me know in case I have misunderstood.

See the above. Yes, in this particular case it will get a value in both branches, but I feel it is too fragile and may get messed up if someone refactors code.

Quoted 47 lines

Quoted 47 lines

drivers/input/misc/iqs626a.c +0 -6

 		if (ch_id == IQS626_CH_TP_2 || ch_id == IQS626_CH_TP_3) {
 			/*
 			 * Trackpad touch events are simply described under the

@@ -530,7 +530,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
 					dev_err(&client->dev,
 						"Invalid input type: %u\n",
 						val);
-					fwnode_handle_put(ev_node);
 					return -EINVAL;
 				}
 

@@ -545,7 +544,6 @@ 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;
 			}
 

@@ -566,7 +564,6 @@ 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;
 			}
 

@@ -575,8 +572,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
 			else
 				*(thresh + iqs626_events[i].th_offs) = val;
 		}
-
-		fwnode_handle_put(ev_node);
 	}
 
 	return 0;

@@ -774,12 +769,12 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
 	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);
 

Quoted 5 lines

drivers/input/misc/iqs626a.c static int iqs626_parse_trackpad(struct iqs626_private *iqs626, · line 777

Quoted 3 lines

drivers/input/misc/iqs626a.c +2 -1

-		tc_node = fwnode_get_named_child_node(ch_node, tc_name);
+		struct fwnode_handle *tc_node __free(fwnode_handle) =
+				fwnode_get_named_child_node(ch_node, tc_name);

Same here.

Yes, combining declaration and initialization is preferred for such pointers.

Thanks.

Signature · 1 line

Dmitry