Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH 01/22] Input: ad714x - use guard notation when acquiring mutex
From: Javier Carrasco @ 2024-09-04 18:47 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel
In-Reply-To: <20240904044244.1042174-2-dmitry.torokhov@gmail.com>

On 04/09/2024 06:42, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/ad714x.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/input/misc/ad714x.c b/drivers/input/misc/ad714x.c
> index 1acd8429c56c..d106f37df6bc 100644
> --- a/drivers/input/misc/ad714x.c
> +++ b/drivers/input/misc/ad714x.c
> @@ -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 */
>  
> @@ -1197,8 +1193,6 @@ static int ad714x_resume(struct device *dev)
>  
>  	ad714x->read(ad714x, STG_LOW_INT_STA_REG, &ad714x->l_state, 3);
>  
> -	mutex_unlock(&ad714x->mutex);
> -
>  	return 0;
>  }
>  

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH 02/22] Input: ati_remote2 - use guard notation when acquiring mutex
From: Javier Carrasco @ 2024-09-04 18:49 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel,
	Javier Carrasco Cruz
In-Reply-To: <20240904044244.1042174-3-dmitry.torokhov@gmail.com>

On 04/09/2024 06:42, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/ati_remote2.c | 57 +++++++++++---------------------
>  1 file changed, 19 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/input/misc/ati_remote2.c b/drivers/input/misc/ati_remote2.c
> index 795f69edb4b2..e84649af801d 100644
> --- a/drivers/input/misc/ati_remote2.c
> +++ b/drivers/input/misc/ati_remote2.c
> @@ -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);
> @@ -955,9 +939,6 @@ static int ati_remote2_reset_resume(struct usb_interface *interface)
>  	if (!r)
>  		ar2->flags &= ~ATI_REMOTE2_SUSPENDED;
>  
> - out:
> -	mutex_unlock(&ati_remote2_mutex);
> -
>  	return r;
>  }
>  

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH 04/22] Input: cma3000_d0x - use guard notation when acquiring mutex
From: Javier Carrasco @ 2024-09-04 18:51 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel,
	Javier Carrasco Cruz
In-Reply-To: <20240904044244.1042174-5-dmitry.torokhov@gmail.com>

On 04/09/2024 06:42, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/cma3000_d0x.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/input/misc/cma3000_d0x.c b/drivers/input/misc/cma3000_d0x.c
> index 0c68e924a1cc..cfc12332bee1 100644
> --- a/drivers/input/misc/cma3000_d0x.c
> +++ b/drivers/input/misc/cma3000_d0x.c
> @@ -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);
>  
>  	if (data->suspended && data->opened)
>  		cma3000_poweron(data);
>  
>  	data->suspended = false;
> -
> -	mutex_unlock(&data->mutex);
>  }
>  EXPORT_SYMBOL(cma3000_resume);
>  

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH 12/22] Input: iqs269a - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 18:53 UTC (permalink / raw)
  To: Javier Carrasco
  Cc: linux-input, Michael Hennerich, Ville Syrjala, Support Opensource,
	Eddie James, Andrey Moiseev, Hans de Goede, Jeff LaBundy,
	linux-kernel
In-Reply-To: <818a1972-2862-460c-89b7-476ac0680db7@gmail.com>

On Wed, Sep 04, 2024 at 08:41:30PM +0200, Javier Carrasco wrote:
> On 04/09/2024 20:21, Dmitry Torokhov wrote:
> > Hi Javier,
> > 
> > On Wed, Sep 04, 2024 at 03:53:40PM +0200, Javier Carrasco wrote:
> >> On 04/09/2024 06:47, Dmitry Torokhov wrote:
> >>> 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 <dmitry.torokhov@gmail.com>
> >>> ---
> >>>  drivers/input/misc/iqs269a.c | 46 +++++++++++++-----------------------
> >>>  1 file changed, 16 insertions(+), 30 deletions(-)
> >>>
> >>> diff --git a/drivers/input/misc/iqs269a.c b/drivers/input/misc/iqs269a.c
> >>> index 843f8a3f3410..c34d847fa4af 100644
> >>> --- a/drivers/input/misc/iqs269a.c
> >>> +++ b/drivers/input/misc/iqs269a.c
> >>
> >> ...
> >>
> >>> @@ -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);
> >>
> >> 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.
> > 
> >>
> >>> -	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);
> >>
> >> same here?
> >>
> >>>  
> >>> +	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.

Simply less indentation ;) and in this driver uniformity with for example
iqs269_ati_target_set() where critical section does indeed extend to the
whole function.

Not super strong arguments either.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 05/22] Input: da7280 - use guard notation when acquiring mutex and spinlock
From: Javier Carrasco @ 2024-09-04 19:02 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel
In-Reply-To: <20240904044244.1042174-6-dmitry.torokhov@gmail.com>

On 04/09/2024 06:42, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/da7280.c | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/input/misc/da7280.c b/drivers/input/misc/da7280.c
> index 1629b7ea4cbd..e4a605c6af15 100644
> --- a/drivers/input/misc/da7280.c
> +++ b/drivers/input/misc/da7280.c
> @@ -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) {
>  		haptics->suspended = false;
> -		spin_unlock_irq(&haptics->input_dev->event_lock);
>  	}
>  
> -	mutex_unlock(&haptics->input_dev->mutex);
> -	return retval;
> +	return 0;
>  }
>  
>  #ifdef CONFIG_OF

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH 06/22] Input: kxtj9 - use guard notation when acquiring mutex/disabling irq
From: Javier Carrasco @ 2024-09-04 19:03 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel,
	Javier Carrasco Cruz
In-Reply-To: <20240904044244.1042174-7-dmitry.torokhov@gmail.com>

On 04/09/2024 06:42, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/kxtj9.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/input/misc/kxtj9.c b/drivers/input/misc/kxtj9.c
> index 837682cb2a7d..c6146bcee9f9 100644
> --- a/drivers/input/misc/kxtj9.c
> +++ b/drivers/input/misc/kxtj9.c
> @@ -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;
>  
> -	mutex_lock(&input_dev->mutex);
> +	guard(mutex)(&input_dev->mutex);
>  
>  	if (input_device_enabled(input_dev))
>  		kxtj9_enable(tj9);
>  
> -	mutex_unlock(&input_dev->mutex);
>  	return 0;
>  }
>  

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH 07/22] Input: drv260x - use guard notation when acquiring mutex
From: Javier Carrasco @ 2024-09-04 19:05 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel
In-Reply-To: <20240904044244.1042174-8-dmitry.torokhov@gmail.com>

On 04/09/2024 06:42, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/drv260x.c | 50 +++++++++++++++++-------------------
>  1 file changed, 24 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
> index 61b503835aa6..96cd6a078c8a 100644
> --- a/drivers/input/misc/drv260x.c
> +++ b/drivers/input/misc/drv260x.c
> @@ -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;
>  		}
>  
>  		gpiod_set_value(haptics->enable_gpio, 1);
>  	}
>  
> -out:
> -	mutex_unlock(&haptics->input_dev->mutex);
> -	return ret;
> +	return 0;
>  }
>  
>  static DEFINE_SIMPLE_DEV_PM_OPS(drv260x_pm_ops, drv260x_suspend, drv260x_resume);

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH 08/22] Input: drv2665 - use guard notation when acquiring mutex
From: Javier Carrasco @ 2024-09-04 19:07 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel,
	Javier Carrasco Cruz
In-Reply-To: <20240904044244.1042174-9-dmitry.torokhov@gmail.com>

On 04/09/2024 06:42, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/drv2665.c | 44 +++++++++++++++++-------------------
>  1 file changed, 21 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/input/misc/drv2665.c b/drivers/input/misc/drv2665.c
> index f98e4d765307..77ec96c7db76 100644
> --- a/drivers/input/misc/drv2665.c
> +++ b/drivers/input/misc/drv2665.c
> @@ -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;
>  		}
>  
>  	}
>  
> -out:
> -	mutex_unlock(&haptics->input_dev->mutex);
> -	return ret;
> +	return 0;
>  }
>  
>  static DEFINE_SIMPLE_DEV_PM_OPS(drv2665_pm_ops, drv2665_suspend, drv2665_resume);

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH 09/22] Input: drv2667 - use guard notation when acquiring mutex
From: Javier Carrasco @ 2024-09-04 19:08 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel,
	Javier Carrasco Cruz
In-Reply-To: <20240904044244.1042174-10-dmitry.torokhov@gmail.com>

On 04/09/2024 06:42, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/drv2667.c | 44 +++++++++++++++++-------------------
>  1 file changed, 21 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/input/misc/drv2667.c b/drivers/input/misc/drv2667.c
> index ad49845374b9..906292625f84 100644
> --- a/drivers/input/misc/drv2667.c
> +++ b/drivers/input/misc/drv2667.c
> @@ -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;
>  		}
>  
>  	}
>  
> -out:
> -	mutex_unlock(&haptics->input_dev->mutex);
> -	return ret;
> +	return 0;
>  }
>  
>  static DEFINE_SIMPLE_DEV_PM_OPS(drv2667_pm_ops, drv2667_suspend, drv2667_resume);


Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH 10/22] Input: ideapad_slidebar - use guard notation when acquiring spinlock
From: Javier Carrasco @ 2024-09-04 19:09 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel,
	Javier Carrasco Cruz
In-Reply-To: <20240904044244.1042174-11-dmitry.torokhov@gmail.com>

On 04/09/2024 06:42, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/ideapad_slidebar.c | 22 +++++-----------------
>  1 file changed, 5 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/input/misc/ideapad_slidebar.c b/drivers/input/misc/ideapad_slidebar.c
> index fa4e7f67d713..592bd159a194 100644
> --- a/drivers/input/misc/ideapad_slidebar.c
> +++ b/drivers/input/misc/ideapad_slidebar.c
> @@ -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;
> +	guard(spinlock_irqsave)(&io_lock);
>  
> -	spin_lock_irqsave(&io_lock, flags);
>  	outb(0xf7, 0xff29);
>  	outb(0x8b, 0xff2a);
>  	outb(mode, 0xff2b);
> -	spin_unlock_irqrestore(&io_lock, flags);
>  }
>  
>  static bool slidebar_i8042_filter(unsigned char data, unsigned char str,

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH 11/22] Input: ibm-panel - use guard notation when acquiring spinlock
From: Javier Carrasco @ 2024-09-04 19:11 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel,
	Javier Carrasco Cruz
In-Reply-To: <20240904044735.1047285-1-dmitry.torokhov@gmail.com>

On 04/09/2024 06:47, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/ibm-panel.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/input/misc/ibm-panel.c b/drivers/input/misc/ibm-panel.c
> index 867ac7aa10d2..aa48f62d7ea0 100644
> --- a/drivers/input/misc/ibm-panel.c
> +++ b/drivers/input/misc/ibm-panel.c
> @@ -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;
>  }
>  

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH 16/22] Input: max8997_haptic - use guard notation when acquiring mutex
From: Javier Carrasco @ 2024-09-04 19:12 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel,
	Javier Carrasco Cruz
In-Reply-To: <20240904044834.1048468-1-dmitry.torokhov@gmail.com>

On 04/09/2024 06:48, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/max8997_haptic.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/input/misc/max8997_haptic.c b/drivers/input/misc/max8997_haptic.c
> index 11cac4b7dddc..2853455daef2 100644
> --- a/drivers/input/misc/max8997_haptic.c
> +++ b/drivers/input/misc/max8997_haptic.c
> @@ -153,19 +153,19 @@ static void max8997_haptic_enable(struct max8997_haptic *chip)
>  {
>  	int error;
>  
> -	mutex_lock(&chip->mutex);
> +	guard(mutex)(&chip->mutex);
>  
>  	error = max8997_haptic_set_duty_cycle(chip);
>  	if (error) {
>  		dev_err(chip->dev, "set_pwm_cycle failed, error: %d\n", error);
> -		goto out;
> +		return;
>  	}
>  
>  	if (!chip->enabled) {
>  		error = regulator_enable(chip->regulator);
>  		if (error) {
>  			dev_err(chip->dev, "Failed to enable regulator\n");
> -			goto out;
> +			return;
>  		}
>  		max8997_haptic_configure(chip);
>  		if (chip->mode == MAX8997_EXTERNAL_MODE) {
> @@ -173,19 +173,16 @@ static void max8997_haptic_enable(struct max8997_haptic *chip)
>  			if (error) {
>  				dev_err(chip->dev, "Failed to enable PWM\n");
>  				regulator_disable(chip->regulator);
> -				goto out;
> +				return;
>  			}
>  		}
>  		chip->enabled = true;
>  	}
> -
> -out:
> -	mutex_unlock(&chip->mutex);
>  }
>  
>  static void max8997_haptic_disable(struct max8997_haptic *chip)
>  {
> -	mutex_lock(&chip->mutex);
> +	guard(mutex)(&chip->mutex);
>  
>  	if (chip->enabled) {
>  		chip->enabled = false;
> @@ -194,8 +191,6 @@ static void max8997_haptic_disable(struct max8997_haptic *chip)
>  			pwm_disable(chip->pwm);
>  		regulator_disable(chip->regulator);
>  	}
> -
> -	mutex_unlock(&chip->mutex);
>  }
>  
>  static void max8997_haptic_play_effect_work(struct work_struct *work)

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH 18/22] Input: powermate - use guard notation when acquiring spinlock
From: Javier Carrasco @ 2024-09-04 19:16 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel,
	Javier Carrasco Cruz
In-Reply-To: <20240904044902.1049017-1-dmitry.torokhov@gmail.com>

On 04/09/2024 06:49, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/powermate.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/input/misc/powermate.c b/drivers/input/misc/powermate.c
> index 4b039abffc4b..ecb92ee5ebbc 100644
> --- a/drivers/input/misc/powermate.c
> +++ b/drivers/input/misc/powermate.c
> @@ -194,22 +194,18 @@ static void powermate_sync_state(struct powermate_device *pm)
>  static void powermate_config_complete(struct urb *urb)
>  {
>  	struct powermate_device *pm = urb->context;
> -	unsigned long flags;
>  
>  	if (urb->status)
>  		printk(KERN_ERR "powermate: config urb returned %d\n", urb->status);
>  
> -	spin_lock_irqsave(&pm->lock, flags);
> +	guard(spinlock_irqsave)(&pm->lock);
>  	powermate_sync_state(pm);
> -	spin_unlock_irqrestore(&pm->lock, flags);
>  }
>  
>  /* Set the LED up as described and begin the sync with the hardware if required */
>  static void powermate_pulse_led(struct powermate_device *pm, int static_brightness, int pulse_speed,
>  				int pulse_table, int pulse_asleep, int pulse_awake)
>  {
> -	unsigned long flags;
> -
>  	if (pulse_speed < 0)
>  		pulse_speed = 0;
>  	if (pulse_table < 0)
> @@ -222,8 +218,7 @@ static void powermate_pulse_led(struct powermate_device *pm, int static_brightne
>  	pulse_asleep = !!pulse_asleep;
>  	pulse_awake = !!pulse_awake;
>  
> -
> -	spin_lock_irqsave(&pm->lock, flags);
> +	guard(spinlock_irqsave)(&pm->lock);
>  
>  	/* mark state updates which are required */
>  	if (static_brightness != pm->static_brightness) {
> @@ -245,8 +240,6 @@ static void powermate_pulse_led(struct powermate_device *pm, int static_brightne
>  	}
>  
>  	powermate_sync_state(pm);
> -
> -	spin_unlock_irqrestore(&pm->lock, flags);
>  }
>  
>  /* Callback from the Input layer when an event arrives from userspace to configure the LED */

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH 19/22] Input: pwm-beeper - use guard notation when acquiring spinlock
From: Javier Carrasco @ 2024-09-04 19:22 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel,
	Javier Carrasco Cruz
In-Reply-To: <20240904044914.1049280-1-dmitry.torokhov@gmail.com>

On 04/09/2024 06:49, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/pwm-beeper.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
> index 5b9aedf4362f..0e19e97d98ec 100644
> --- a/drivers/input/misc/pwm-beeper.c
> +++ b/drivers/input/misc/pwm-beeper.c
> @@ -203,9 +203,9 @@ static int pwm_beeper_suspend(struct device *dev)
>  	 * beeper->suspended, but to ensure that pwm_beeper_event
>  	 * does not re-submit work once flag is set.
>  	 */
> -	spin_lock_irq(&beeper->input->event_lock);
> -	beeper->suspended = true;
> -	spin_unlock_irq(&beeper->input->event_lock);

I assume you know that you don't need the braces for the scoped_guard()
in these cases. If you prefer doing so to clarify (you are leaving an
empty line afterwards anyway, but still), I am ok with it.

Note that other users of scoped_guard() tend to use them without braces
for single instructions.

> +	scoped_guard(spinlock_irq, &beeper->input->event_lock) {
> +		beeper->suspended = true;
> +	}
>  
>  	pwm_beeper_stop(beeper);
>  
> @@ -216,9 +216,9 @@ static int pwm_beeper_resume(struct device *dev)
>  {
>  	struct pwm_beeper *beeper = dev_get_drvdata(dev);
>  
> -	spin_lock_irq(&beeper->input->event_lock);
> -	beeper->suspended = false;
> -	spin_unlock_irq(&beeper->input->event_lock);
> +	scoped_guard(spinlock_irq, &beeper->input->event_lock) {
> +		beeper->suspended = false;
> +	}
>  
>  	/* Let worker figure out if we should resume beeping */
>  	schedule_work(&beeper->work);

With or without braces,

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH 20/22] Input: regulator-haptic - use guard notation when acquiring mutex
From: Javier Carrasco @ 2024-09-04 19:27 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel,
	Javier Carrasco Cruz
In-Reply-To: <20240904044922.1049488-1-dmitry.torokhov@gmail.com>

On 04/09/2024 06:49, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/regulator-haptic.c | 23 ++++++++---------------
>  1 file changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/input/misc/regulator-haptic.c b/drivers/input/misc/regulator-haptic.c
> index 02f73b7c0462..41af6aefaa07 100644
> --- a/drivers/input/misc/regulator-haptic.c
> +++ b/drivers/input/misc/regulator-haptic.c
> @@ -83,12 +83,10 @@ static void regulator_haptic_work(struct work_struct *work)
>  	struct regulator_haptic *haptic = container_of(work,
>  					struct regulator_haptic, work);
>  
> -	mutex_lock(&haptic->mutex);
> +	guard(mutex)(&haptic->mutex);
>  
>  	if (!haptic->suspended)
>  		regulator_haptic_set_voltage(haptic, haptic->magnitude);
> -
> -	mutex_unlock(&haptic->mutex);
>  }
>  
>  static int regulator_haptic_play_effect(struct input_dev *input, void *data,
> @@ -207,17 +205,14 @@ static int regulator_haptic_suspend(struct device *dev)
>  	struct regulator_haptic *haptic = platform_get_drvdata(pdev);

error became an unused variable and can be dropped.

>  	int error;
>  
> -	error = mutex_lock_interruptible(&haptic->mutex);
> -	if (error)
> -		return error;
> -
> -	regulator_haptic_set_voltage(haptic, 0);
> -
> -	haptic->suspended = true;
> +	scoped_guard(mutex_intr, &haptic->mutex) {
> +		regulator_haptic_set_voltage(haptic, 0);
> +		haptic->suspended = true;
>  
> -	mutex_unlock(&haptic->mutex);
> +		return 0;
> +	}
>  
> -	return 0;
> +	return -EINTR;
>  }
>  
>  static int regulator_haptic_resume(struct device *dev)
> @@ -226,7 +221,7 @@ static int regulator_haptic_resume(struct device *dev)
>  	struct regulator_haptic *haptic = platform_get_drvdata(pdev);
>  	unsigned int magnitude;
>  
> -	mutex_lock(&haptic->mutex);
> +	guard(mutex)(&haptic->mutex);
>  
>  	haptic->suspended = false;
>  
> @@ -234,8 +229,6 @@ static int regulator_haptic_resume(struct device *dev)
>  	if (magnitude)
>  		regulator_haptic_set_voltage(haptic, magnitude);
>  
> -	mutex_unlock(&haptic->mutex);
> -
>  	return 0;
>  }
>  

^ permalink raw reply

* Re: [PATCH 21/22] Input: rotary_encoder - use guard notation when acquiring mutex
From: Javier Carrasco @ 2024-09-04 19:32 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel,
	Javier Carrasco Cruz
In-Reply-To: <20240904044929.1049700-1-dmitry.torokhov@gmail.com>

On 04/09/2024 06:49, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/rotary_encoder.c | 23 ++++++++---------------
>  1 file changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
> index 6628fe540834..52761da9f999 100644
> --- a/drivers/input/misc/rotary_encoder.c
> +++ b/drivers/input/misc/rotary_encoder.c
> @@ -113,7 +113,7 @@ static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
>  	struct rotary_encoder *encoder = dev_id;
>  	unsigned int state;
>  
> -	mutex_lock(&encoder->access_mutex);
> +	guard(mutex)(&encoder->access_mutex);
>  
>  	state = rotary_encoder_get_state(encoder);
>  
> @@ -136,8 +136,6 @@ static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
>  		break;
>  	}
>  
> -	mutex_unlock(&encoder->access_mutex);
> -
>  	return IRQ_HANDLED;
>  }
>  
> @@ -146,7 +144,7 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
>  	struct rotary_encoder *encoder = dev_id;
>  	unsigned int state;
>  
> -	mutex_lock(&encoder->access_mutex);
> +	guard(mutex)(&encoder->access_mutex);
>  
>  	state = rotary_encoder_get_state(encoder);
>  
> @@ -159,8 +157,6 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
>  		}
>  	}
>  
> -	mutex_unlock(&encoder->access_mutex);
> -
>  	return IRQ_HANDLED;
>  }
>  
> @@ -169,22 +165,19 @@ static irqreturn_t rotary_encoder_quarter_period_irq(int irq, void *dev_id)
>  	struct rotary_encoder *encoder = dev_id;
>  	unsigned int state;
>  
> -	mutex_lock(&encoder->access_mutex);
> +	guard(mutex)(&encoder->access_mutex);
>  
>  	state = rotary_encoder_get_state(encoder);
>  
> -	if ((encoder->last_stable + 1) % 4 == state)
> +	if ((encoder->last_stable + 1) % 4 == state) {
>  		encoder->dir = 1;
> -	else if (encoder->last_stable == (state + 1) % 4)
> +		rotary_encoder_report_event(encoder);
> +	} else if (encoder->last_stable == (state + 1) % 4) {
>  		encoder->dir = -1;
> -	else
> -		goto out;
> -
> -	rotary_encoder_report_event(encoder);
> +		rotary_encoder_report_event(encoder);
> +	}
>  
> -out:
>  	encoder->last_stable = state;
> -	mutex_unlock(&encoder->access_mutex);
>  
>  	return IRQ_HANDLED;
>  }

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH 22/22] Input: sparcspkr - use guard notation when acquiring spinlock
From: Javier Carrasco @ 2024-09-04 19:33 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel,
	Javier Carrasco Cruz
In-Reply-To: <20240904044938.1049843-1-dmitry.torokhov@gmail.com>

On 04/09/2024 06:49, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/sparcspkr.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/input/misc/sparcspkr.c b/drivers/input/misc/sparcspkr.c
> index 20020cbc0752..5de59ae90c67 100644
> --- a/drivers/input/misc/sparcspkr.c
> +++ b/drivers/input/misc/sparcspkr.c
> @@ -69,7 +69,6 @@ static int bbc_spkr_event(struct input_dev *dev, unsigned int type, unsigned int
>  	struct sparcspkr_state *state = dev_get_drvdata(dev->dev.parent);
>  	struct bbc_beep_info *info = &state->u.bbc;
>  	unsigned int count = 0;
> -	unsigned long flags;
>  
>  	if (type != EV_SND)
>  		return -1;
> @@ -85,7 +84,7 @@ static int bbc_spkr_event(struct input_dev *dev, unsigned int type, unsigned int
>  
>  	count = bbc_count_to_reg(info, count);
>  
> -	spin_lock_irqsave(&state->lock, flags);
> +	guard(spinlock_irqsave)(&state->lock);
>  
>  	if (count) {
>  		sbus_writeb(0x01,                 info->regs + 0);
> @@ -97,8 +96,6 @@ static int bbc_spkr_event(struct input_dev *dev, unsigned int type, unsigned int
>  		sbus_writeb(0x00,                 info->regs + 0);
>  	}
>  
> -	spin_unlock_irqrestore(&state->lock, flags);
> -
>  	return 0;
>  }
>  
> @@ -107,7 +104,6 @@ static int grover_spkr_event(struct input_dev *dev, unsigned int type, unsigned
>  	struct sparcspkr_state *state = dev_get_drvdata(dev->dev.parent);
>  	struct grover_beep_info *info = &state->u.grover;
>  	unsigned int count = 0;
> -	unsigned long flags;
>  
>  	if (type != EV_SND)
>  		return -1;
> @@ -121,7 +117,7 @@ static int grover_spkr_event(struct input_dev *dev, unsigned int type, unsigned
>  	if (value > 20 && value < 32767)
>  		count = 1193182 / value;
>  
> -	spin_lock_irqsave(&state->lock, flags);
> +	guard(spinlock_irqsave)(&state->lock);
>  
>  	if (count) {
>  		/* enable counter 2 */
> @@ -136,8 +132,6 @@ static int grover_spkr_event(struct input_dev *dev, unsigned int type, unsigned
>  		sbus_writeb(sbus_readb(info->enable_reg) & 0xFC, info->enable_reg);
>  	}
>  
> -	spin_unlock_irqrestore(&state->lock, flags);
> -
>  	return 0;
>  }
>  

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH 03/22] Input: cm109 - use guard notation when acquiring mutex and spinlock
From: Javier Carrasco @ 2024-09-04 19:44 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel,
	Javier Carrasco Cruz
In-Reply-To: <20240904044244.1042174-4-dmitry.torokhov@gmail.com>

On 04/09/2024 06:42, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/misc/cm109.c | 167 ++++++++++++++++++-------------------
>  1 file changed, 79 insertions(+), 88 deletions(-)
> 
> diff --git a/drivers/input/misc/cm109.c b/drivers/input/misc/cm109.c
> index 728325a2d574..0cfe5d4a573c 100644
> --- a/drivers/input/misc/cm109.c
> +++ b/drivers/input/misc/cm109.c
> @@ -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)
>  
>  	dev_info(&intf->dev, "cm109: usb_resume\n");
>  
> -	mutex_lock(&dev->pm_mutex);
> +	guard(mutex)(&dev->pm_mutex);
> +
>  	cm109_restore_state(dev);
> -	mutex_unlock(&dev->pm_mutex);
>  
>  	return 0;
>  }

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH v26 30/33] ALSA: usb-audio: qcom: Use card and PCM index from QMI request
From: Wesley Cheng @ 2024-09-04 19:46 UTC (permalink / raw)
  To: Pierre-Louis Bossart, srinivas.kandagatla, mathias.nyman, perex,
	conor+dt, dmitry.torokhov, corbet, broonie, lgirdwood, tiwai,
	krzk+dt, Thinh.Nguyen, bgoswami, robh, gregkh
  Cc: linux-kernel, devicetree, linux-sound, linux-input, linux-usb,
	linux-arm-msm, linux-doc, alsa-devel
In-Reply-To: <f8090415-e0ae-4923-bdc8-58622623fc9d@linux.intel.com>

Hi Pierre,

On 8/30/2024 2:58 AM, Pierre-Louis Bossart wrote:
>
> On 8/29/24 21:41, Wesley Cheng wrote:
>> Utilize the card and PCM index coming from the USB QMI stream request.
>> This field follows what is set by the ASoC USB backend, and could
>> potentially carry information about a specific device selected through the
>> ASoC USB backend.  The backend also has information about the last USB
>> sound device plugged in, so it can choose to select the last device plugged
>> in, accordingly.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>> ---
>>  sound/usb/qcom/qc_audio_offload.c | 8 ++------
>>  1 file changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
>> index 0bd533f539e4..a7ad15404fd1 100644
>> --- a/sound/usb/qcom/qc_audio_offload.c
>> +++ b/sound/usb/qcom/qc_audio_offload.c
>> @@ -106,8 +106,6 @@ struct uaudio_qmi_dev {
>>  	bool er_mapped;
>>  	/* reference count to number of possible consumers */
>>  	atomic_t qdev_in_use;
>> -	/* idx to last udev card number plugged in */
>> -	unsigned int last_card_num;
>>  };
>>  
>>  struct uaudio_dev {
>> @@ -1261,7 +1259,7 @@ static int prepare_qmi_response(struct snd_usb_substream *subs,
>>  
>>  	pcm_dev_num = (req_msg->usb_token & QMI_STREAM_REQ_DEV_NUM_MASK) >> 8;
>>  	xfer_buf_len = req_msg->xfer_buff_size;
>> -	card_num = uaudio_qdev->last_card_num;
>> +	card_num = (req_msg->usb_token & QMI_STREAM_REQ_CARD_NUM_MASK) >> 16;
>>  
>>  	if (!uadev[card_num].ctrl_intf) {
>>  		dev_err(&subs->dev->dev, "audio ctrl intf info not cached\n");
>> @@ -1455,8 +1453,7 @@ static void handle_uaudio_stream_req(struct qmi_handle *handle,
>>  
>>  	direction = (req_msg->usb_token & QMI_STREAM_REQ_DIRECTION);
>>  	pcm_dev_num = (req_msg->usb_token & QMI_STREAM_REQ_DEV_NUM_MASK) >> 8;
>> -	pcm_card_num = req_msg->enable ? uaudio_qdev->last_card_num :
>> -				ffs(uaudio_qdev->card_slot) - 1;
>> +	pcm_card_num = (req_msg->usb_token & QMI_STREAM_REQ_CARD_NUM_MASK) >> 16;
>>  	if (pcm_card_num >= SNDRV_CARDS) {
>>  		ret = -EINVAL;
>>  		goto response;
>> @@ -1706,7 +1703,6 @@ static void qc_usb_audio_offload_probe(struct snd_usb_audio *chip)
>>  		sdev->card_idx = chip->card->number;
>>  		sdev->chip_idx = chip->index;
>>  
>> -		uaudio_qdev->last_card_num = chip->card->number;
>>  		snd_soc_usb_connect(usb_get_usb_backend(udev), sdev);
>>  	}
> This entire path seems like a bad split/merge, it removes stuff that was
> done earlier. Also it's not clear what this has to do with 'QMI', card
> and PCM device management is usually done at a higher level.
>
> not following, please be mindful of reviewer fatigue when adding such
> changes in patch 30/33....

I'll just add this as part of patch#28.  I think before I did the reordering of the series, this made a bit more sense to have as a patch on its own.  Now that the entire framework for the audio dsp to know about the card and pcm index is already done in previous patches, the plumbing is done for the qc_audio_offload to utilize the fields coming from the audio DSP, as they will carry valid values.

Thanks

Wesley Cheng


^ permalink raw reply

* Re: [PATCH 17/22] Input: pegasus_notetaker - use guard notation when acquiring mutex
From: Javier Carrasco @ 2024-09-04 19:52 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
	Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel,
	Javier Carrasco Cruz
In-Reply-To: <20240904044842.1048638-1-dmitry.torokhov@gmail.com>

On 04/09/2024 06:48, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
>  drivers/input/tablet/pegasus_notetaker.c | 86 +++++++++++++-----------
>  1 file changed, 48 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/input/tablet/pegasus_notetaker.c b/drivers/input/tablet/pegasus_notetaker.c
> index a68da2988f9c..e1dc8365bfe9 100644
> --- a/drivers/input/tablet/pegasus_notetaker.c
> +++ b/drivers/input/tablet/pegasus_notetaker.c
> @@ -214,6 +214,28 @@ static void pegasus_init(struct work_struct *work)
>  			error);
>  }
>  
> +static int __pegasus_open(struct pegasus *pegasus)
> +{
> +	int error;
> +
> +	guard(mutex)(&pegasus->pm_mutex);
> +
> +	pegasus->irq->dev = pegasus->usbdev;
> +	if (usb_submit_urb(pegasus->irq, GFP_KERNEL))
> +		return -EIO;
> +
> +	error = pegasus_set_mode(pegasus, PEN_MODE_XY, NOTETAKER_LED_MOUSE);
> +	if (error) {
> +		usb_kill_urb(pegasus->irq);
> +		cancel_work_sync(&pegasus->init);
> +		return error;
> +	}
> +
> +	pegasus->is_open = true;

Nit: blank line before return.

> +	return 0;
> +}

Nit: multiple blank lines.

> +
> +
>  static int pegasus_open(struct input_dev *dev)
>  {
>  	struct pegasus *pegasus = input_get_drvdata(dev);
> @@ -223,39 +245,25 @@ static int pegasus_open(struct input_dev *dev)
>  	if (error)
>  		return error;
>  
> -	mutex_lock(&pegasus->pm_mutex);
> -	pegasus->irq->dev = pegasus->usbdev;
> -	if (usb_submit_urb(pegasus->irq, GFP_KERNEL)) {
> -		error = -EIO;
> -		goto err_autopm_put;
> +	error = __pegasus_open(pegasus);
> +	if (error) {
> +		usb_autopm_put_interface(pegasus->intf);
> +		return error;
>  	}
>  
> -	error = pegasus_set_mode(pegasus, PEN_MODE_XY, NOTETAKER_LED_MOUSE);
> -	if (error)
> -		goto err_kill_urb;
> -
> -	pegasus->is_open = true;
> -	mutex_unlock(&pegasus->pm_mutex);
>  	return 0;
> -
> -err_kill_urb:
> -	usb_kill_urb(pegasus->irq);
> -	cancel_work_sync(&pegasus->init);
> -err_autopm_put:
> -	mutex_unlock(&pegasus->pm_mutex);
> -	usb_autopm_put_interface(pegasus->intf);
> -	return error;
>  }
>  
>  static void pegasus_close(struct input_dev *dev)
>  {
>  	struct pegasus *pegasus = input_get_drvdata(dev);
>  
> -	mutex_lock(&pegasus->pm_mutex);
> -	usb_kill_urb(pegasus->irq);
> -	cancel_work_sync(&pegasus->init);
> -	pegasus->is_open = false;
> -	mutex_unlock(&pegasus->pm_mutex);
> +	scoped_guard(mutex, &pegasus->pm_mutex) {
> +		usb_kill_urb(pegasus->irq);
> +		cancel_work_sync(&pegasus->init);
> +
> +		pegasus->is_open = false;
> +	}
>  
>  	usb_autopm_put_interface(pegasus->intf);
>  }
> @@ -411,10 +419,10 @@ static int pegasus_suspend(struct usb_interface *intf, pm_message_t message)
>  {
>  	struct pegasus *pegasus = usb_get_intfdata(intf);
>  
> -	mutex_lock(&pegasus->pm_mutex);
> +	guard(mutex)(&pegasus->pm_mutex);
> +
>  	usb_kill_urb(pegasus->irq);
>  	cancel_work_sync(&pegasus->init);
> -	mutex_unlock(&pegasus->pm_mutex);
>  
>  	return 0;
>  }
> @@ -422,31 +430,33 @@ static int pegasus_suspend(struct usb_interface *intf, pm_message_t message)
>  static int pegasus_resume(struct usb_interface *intf)
>  {
>  	struct pegasus *pegasus = usb_get_intfdata(intf);
> -	int retval = 0;
>  
> -	mutex_lock(&pegasus->pm_mutex);
> +	guard(mutex)(&pegasus->pm_mutex);
> +
>  	if (pegasus->is_open && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
> -		retval = -EIO;
> -	mutex_unlock(&pegasus->pm_mutex);
> +		return -EIO;
>  
> -	return retval;
> +	return 0;
>  }
>  
>  static int pegasus_reset_resume(struct usb_interface *intf)
>  {
>  	struct pegasus *pegasus = usb_get_intfdata(intf);
> -	int retval = 0;
> +	int error;
> +
> +	guard(mutex)(&pegasus->pm_mutex);
>  
> -	mutex_lock(&pegasus->pm_mutex);
>  	if (pegasus->is_open) {
> -		retval = pegasus_set_mode(pegasus, PEN_MODE_XY,
> +		error = pegasus_set_mode(pegasus, PEN_MODE_XY,
>  					  NOTETAKER_LED_MOUSE);
> -		if (!retval && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
> -			retval = -EIO;
> +		if (error)
> +			return error;
> +
> +		if (usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
> +			return -EIO;
>  	}
> -	mutex_unlock(&pegasus->pm_mutex);
>  
> -	return retval;
> +	return 0;
>  }
>  
>  static const struct usb_device_id pegasus_ids[] = {

Apart from the minor nitpicks,

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* [PATCH v2 20/22] Input: regulator-haptic - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 20:55 UTC (permalink / raw)
  To: Javier Carrasco
  Cc: linux-input, Michael Hennerich, Ville Syrjala, Support Opensource,
	Eddie James, Andrey Moiseev, Hans de Goede, Jeff LaBundy,
	linux-kernel
In-Reply-To: <3ff97fb3-27e0-496e-a8b0-0c2d69deeff2@gmail.com>

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 <dmitry.torokhov@gmail.com>
---

v2: drop no linger used "error" variable in regulator_haptic_suspend()

 drivers/input/misc/regulator-haptic.c |   24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/input/misc/regulator-haptic.c b/drivers/input/misc/regulator-haptic.c
index 02f73b7c0462..3666ba6d1f30 100644
--- a/drivers/input/misc/regulator-haptic.c
+++ b/drivers/input/misc/regulator-haptic.c
@@ -83,12 +83,10 @@ static void regulator_haptic_work(struct work_struct *work)
 	struct regulator_haptic *haptic = container_of(work,
 					struct regulator_haptic, work);
 
-	mutex_lock(&haptic->mutex);
+	guard(mutex)(&haptic->mutex);
 
 	if (!haptic->suspended)
 		regulator_haptic_set_voltage(haptic, haptic->magnitude);
-
-	mutex_unlock(&haptic->mutex);
 }
 
 static int regulator_haptic_play_effect(struct input_dev *input, void *data,
@@ -205,19 +203,15 @@ static int regulator_haptic_suspend(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct regulator_haptic *haptic = platform_get_drvdata(pdev);
-	int error;
 
-	error = mutex_lock_interruptible(&haptic->mutex);
-	if (error)
-		return error;
-
-	regulator_haptic_set_voltage(haptic, 0);
+	scoped_guard(mutex_intr, &haptic->mutex) {
+		regulator_haptic_set_voltage(haptic, 0);
+		haptic->suspended = true;
 
-	haptic->suspended = true;
-
-	mutex_unlock(&haptic->mutex);
+		return 0;
+	}
 
-	return 0;
+	return -EINTR;
 }
 
 static int regulator_haptic_resume(struct device *dev)
@@ -226,7 +220,7 @@ static int regulator_haptic_resume(struct device *dev)
 	struct regulator_haptic *haptic = platform_get_drvdata(pdev);
 	unsigned int magnitude;
 
-	mutex_lock(&haptic->mutex);
+	guard(mutex)(&haptic->mutex);
 
 	haptic->suspended = false;
 
@@ -234,8 +228,6 @@ static int regulator_haptic_resume(struct device *dev)
 	if (magnitude)
 		regulator_haptic_set_voltage(haptic, magnitude);
 
-	mutex_unlock(&haptic->mutex);
-
 	return 0;
 }
 

^ permalink raw reply related

* [PATCH v2 17/22] Input: pegasus_notetaker - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 20:59 UTC (permalink / raw)
  To: Javier Carrasco
  Cc: linux-input, Michael Hennerich, Ville Syrjala, Support Opensource,
	Eddie James, Andrey Moiseev, Hans de Goede, Jeff LaBundy,
	linux-kernel
In-Reply-To: <a41bb88a-b624-4b5b-a2ea-b49761c45a93@gmail.com>

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.

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

V2: fixed whitespace issues, added Reviewed-by from Javier

 drivers/input/tablet/pegasus_notetaker.c |   86 +++++++++++++++++-------------
 1 file changed, 48 insertions(+), 38 deletions(-)

diff --git a/drivers/input/tablet/pegasus_notetaker.c b/drivers/input/tablet/pegasus_notetaker.c
index a68da2988f9c..8d6b71d59793 100644
--- a/drivers/input/tablet/pegasus_notetaker.c
+++ b/drivers/input/tablet/pegasus_notetaker.c
@@ -214,6 +214,28 @@ static void pegasus_init(struct work_struct *work)
 			error);
 }
 
+static int __pegasus_open(struct pegasus *pegasus)
+{
+	int error;
+
+	guard(mutex)(&pegasus->pm_mutex);
+
+	pegasus->irq->dev = pegasus->usbdev;
+	if (usb_submit_urb(pegasus->irq, GFP_KERNEL))
+		return -EIO;
+
+	error = pegasus_set_mode(pegasus, PEN_MODE_XY, NOTETAKER_LED_MOUSE);
+	if (error) {
+		usb_kill_urb(pegasus->irq);
+		cancel_work_sync(&pegasus->init);
+		return error;
+	}
+
+	pegasus->is_open = true;
+
+	return 0;
+}
+
 static int pegasus_open(struct input_dev *dev)
 {
 	struct pegasus *pegasus = input_get_drvdata(dev);
@@ -223,39 +245,25 @@ static int pegasus_open(struct input_dev *dev)
 	if (error)
 		return error;
 
-	mutex_lock(&pegasus->pm_mutex);
-	pegasus->irq->dev = pegasus->usbdev;
-	if (usb_submit_urb(pegasus->irq, GFP_KERNEL)) {
-		error = -EIO;
-		goto err_autopm_put;
+	error = __pegasus_open(pegasus);
+	if (error) {
+		usb_autopm_put_interface(pegasus->intf);
+		return error;
 	}
 
-	error = pegasus_set_mode(pegasus, PEN_MODE_XY, NOTETAKER_LED_MOUSE);
-	if (error)
-		goto err_kill_urb;
-
-	pegasus->is_open = true;
-	mutex_unlock(&pegasus->pm_mutex);
 	return 0;
-
-err_kill_urb:
-	usb_kill_urb(pegasus->irq);
-	cancel_work_sync(&pegasus->init);
-err_autopm_put:
-	mutex_unlock(&pegasus->pm_mutex);
-	usb_autopm_put_interface(pegasus->intf);
-	return error;
 }
 
 static void pegasus_close(struct input_dev *dev)
 {
 	struct pegasus *pegasus = input_get_drvdata(dev);
 
-	mutex_lock(&pegasus->pm_mutex);
-	usb_kill_urb(pegasus->irq);
-	cancel_work_sync(&pegasus->init);
-	pegasus->is_open = false;
-	mutex_unlock(&pegasus->pm_mutex);
+	scoped_guard(mutex, &pegasus->pm_mutex) {
+		usb_kill_urb(pegasus->irq);
+		cancel_work_sync(&pegasus->init);
+
+		pegasus->is_open = false;
+	}
 
 	usb_autopm_put_interface(pegasus->intf);
 }
@@ -411,10 +419,10 @@ static int pegasus_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	struct pegasus *pegasus = usb_get_intfdata(intf);
 
-	mutex_lock(&pegasus->pm_mutex);
+	guard(mutex)(&pegasus->pm_mutex);
+
 	usb_kill_urb(pegasus->irq);
 	cancel_work_sync(&pegasus->init);
-	mutex_unlock(&pegasus->pm_mutex);
 
 	return 0;
 }
@@ -422,31 +430,33 @@ static int pegasus_suspend(struct usb_interface *intf, pm_message_t message)
 static int pegasus_resume(struct usb_interface *intf)
 {
 	struct pegasus *pegasus = usb_get_intfdata(intf);
-	int retval = 0;
 
-	mutex_lock(&pegasus->pm_mutex);
+	guard(mutex)(&pegasus->pm_mutex);
+
 	if (pegasus->is_open && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
-		retval = -EIO;
-	mutex_unlock(&pegasus->pm_mutex);
+		return -EIO;
 
-	return retval;
+	return 0;
 }
 
 static int pegasus_reset_resume(struct usb_interface *intf)
 {
 	struct pegasus *pegasus = usb_get_intfdata(intf);
-	int retval = 0;
+	int error;
+
+	guard(mutex)(&pegasus->pm_mutex);
 
-	mutex_lock(&pegasus->pm_mutex);
 	if (pegasus->is_open) {
-		retval = pegasus_set_mode(pegasus, PEN_MODE_XY,
+		error = pegasus_set_mode(pegasus, PEN_MODE_XY,
 					  NOTETAKER_LED_MOUSE);
-		if (!retval && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
-			retval = -EIO;
+		if (error)
+			return error;
+
+		if (usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
+			return -EIO;
 	}
-	mutex_unlock(&pegasus->pm_mutex);
 
-	return retval;
+	return 0;
 }
 
 static const struct usb_device_id pegasus_ids[] = {

^ permalink raw reply related

* Re: [PATCH v2 20/22] Input: regulator-haptic - use guard notation when acquiring mutex
From: Javier Carrasco @ 2024-09-04 21:41 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input, Michael Hennerich, Ville Syrjala, Support Opensource,
	Eddie James, Andrey Moiseev, Hans de Goede, Jeff LaBundy,
	linux-kernel, Javier Carrasco Cruz
In-Reply-To: <ZtjJKxQRRzJE0aWZ@google.com>

On 04/09/2024 22:55, Dmitry Torokhov wrote:
> 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 <dmitry.torokhov@gmail.com>
> ---
> 
> v2: drop no linger used "error" variable in regulator_haptic_suspend()
> 
>  drivers/input/misc/regulator-haptic.c |   24 ++++++++----------------
>  1 file changed, 8 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/input/misc/regulator-haptic.c b/drivers/input/misc/regulator-haptic.c
> index 02f73b7c0462..3666ba6d1f30 100644
> --- a/drivers/input/misc/regulator-haptic.c
> +++ b/drivers/input/misc/regulator-haptic.c
> @@ -83,12 +83,10 @@ static void regulator_haptic_work(struct work_struct *work)
>  	struct regulator_haptic *haptic = container_of(work,
>  					struct regulator_haptic, work);
>  
> -	mutex_lock(&haptic->mutex);
> +	guard(mutex)(&haptic->mutex);
>  
>  	if (!haptic->suspended)
>  		regulator_haptic_set_voltage(haptic, haptic->magnitude);
> -
> -	mutex_unlock(&haptic->mutex);
>  }
>  
>  static int regulator_haptic_play_effect(struct input_dev *input, void *data,
> @@ -205,19 +203,15 @@ static int regulator_haptic_suspend(struct device *dev)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
>  	struct regulator_haptic *haptic = platform_get_drvdata(pdev);
> -	int error;
>  
> -	error = mutex_lock_interruptible(&haptic->mutex);
> -	if (error)
> -		return error;
> -
> -	regulator_haptic_set_voltage(haptic, 0);
> +	scoped_guard(mutex_intr, &haptic->mutex) {
> +		regulator_haptic_set_voltage(haptic, 0);
> +		haptic->suspended = true;
>  
> -	haptic->suspended = true;
> -
> -	mutex_unlock(&haptic->mutex);
> +		return 0;
> +	}
>  
> -	return 0;
> +	return -EINTR;
>  }
>  
>  static int regulator_haptic_resume(struct device *dev)
> @@ -226,7 +220,7 @@ static int regulator_haptic_resume(struct device *dev)
>  	struct regulator_haptic *haptic = platform_get_drvdata(pdev);
>  	unsigned int magnitude;
>  
> -	mutex_lock(&haptic->mutex);
> +	guard(mutex)(&haptic->mutex);
>  
>  	haptic->suspended = false;
>  
> @@ -234,8 +228,6 @@ static int regulator_haptic_resume(struct device *dev)
>  	if (magnitude)
>  		regulator_haptic_set_voltage(haptic, magnitude);
>  
> -	mutex_unlock(&haptic->mutex);
> -
>  	return 0;
>  }
>  

Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

^ permalink raw reply

* Re: [PATCH 11/22] Input: ibm-panel - use guard notation when acquiring spinlock
From: Eddie James @ 2024-09-04 21:56 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input
  Cc: Michael Hennerich, Ville Syrjala, Support Opensource,
	Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
	linux-kernel
In-Reply-To: <20240904044735.1047285-1-dmitry.torokhov@gmail.com>


On 9/3/24 23:47, Dmitry Torokhov wrote:
> 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.


Reviewed-by: Eddie James <eajames@linux.ibm.com>


>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>   drivers/input/misc/ibm-panel.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/input/misc/ibm-panel.c b/drivers/input/misc/ibm-panel.c
> index 867ac7aa10d2..aa48f62d7ea0 100644
> --- a/drivers/input/misc/ibm-panel.c
> +++ b/drivers/input/misc/ibm-panel.c
> @@ -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;
>   }
>   

^ permalink raw reply

* [dtor-input:next] BUILD SUCCESS a790df272a20dcc88ffebe20eca34c54f528fcaa
From: kernel test robot @ 2024-09-04 23:02 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input

tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
branch HEAD: a790df272a20dcc88ffebe20eca34c54f528fcaa  Input: synaptics-rmi4 - fix crash when DPM query is not supported

elapsed time: 1445m

configs tested: 112
configs skipped: 2

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alpha                             allnoconfig   gcc-14.1.0
alpha                               defconfig   gcc-14.1.0
arc                              allmodconfig   clang-20
arc                               allnoconfig   gcc-14.1.0
arc                              allyesconfig   clang-20
arc                                 defconfig   gcc-14.1.0
arm                              allmodconfig   clang-20
arm                               allnoconfig   gcc-14.1.0
arm                              allyesconfig   clang-20
arm                          collie_defconfig   gcc-14.1.0
arm                                 defconfig   gcc-14.1.0
arm                          gemini_defconfig   gcc-14.1.0
arm                   milbeaut_m10v_defconfig   gcc-14.1.0
arm                           omap1_defconfig   gcc-14.1.0
arm                        spear3xx_defconfig   gcc-14.1.0
arm                    vt8500_v6_v7_defconfig   gcc-14.1.0
arm64                            allmodconfig   clang-20
arm64                             allnoconfig   gcc-14.1.0
arm64                               defconfig   gcc-14.1.0
csky                              allnoconfig   gcc-14.1.0
csky                                defconfig   gcc-14.1.0
hexagon                           allnoconfig   gcc-14.1.0
hexagon                             defconfig   gcc-14.1.0
i386                             allmodconfig   clang-18
i386                              allnoconfig   clang-18
i386                             allyesconfig   clang-18
i386                                defconfig   clang-18
loongarch                        allmodconfig   gcc-14.1.0
loongarch                         allnoconfig   gcc-14.1.0
loongarch                           defconfig   gcc-14.1.0
m68k                             allmodconfig   gcc-14.1.0
m68k                              allnoconfig   gcc-14.1.0
m68k                             allyesconfig   gcc-14.1.0
m68k                                defconfig   gcc-14.1.0
microblaze                       allmodconfig   gcc-14.1.0
microblaze                        allnoconfig   gcc-14.1.0
microblaze                       allyesconfig   gcc-14.1.0
microblaze                          defconfig   gcc-14.1.0
mips                              allnoconfig   gcc-14.1.0
mips                     loongson1c_defconfig   gcc-14.1.0
mips                      loongson3_defconfig   gcc-14.1.0
nios2                             allnoconfig   gcc-14.1.0
nios2                               defconfig   gcc-14.1.0
openrisc                          allnoconfig   clang-20
openrisc                         allyesconfig   gcc-14.1.0
openrisc                            defconfig   gcc-12
openrisc                    or1ksim_defconfig   gcc-14.1.0
parisc                           allmodconfig   gcc-14.1.0
parisc                            allnoconfig   clang-20
parisc                           allyesconfig   gcc-14.1.0
parisc                              defconfig   gcc-12
parisc64                            defconfig   gcc-14.1.0
powerpc                          allmodconfig   gcc-14.1.0
powerpc                           allnoconfig   clang-20
powerpc                          allyesconfig   gcc-14.1.0
powerpc                      ep88xc_defconfig   gcc-14.1.0
powerpc                 mpc836x_rdk_defconfig   gcc-14.1.0
powerpc                         ps3_defconfig   gcc-14.1.0
riscv                            allmodconfig   gcc-14.1.0
riscv                             allnoconfig   clang-20
riscv                            allyesconfig   gcc-14.1.0
riscv                               defconfig   gcc-12
s390                             allmodconfig   gcc-14.1.0
s390                              allnoconfig   clang-20
s390                             allyesconfig   gcc-14.1.0
s390                                defconfig   gcc-12
sh                               allmodconfig   gcc-14.1.0
sh                                allnoconfig   gcc-14.1.0
sh                               allyesconfig   gcc-14.1.0
sh                                  defconfig   gcc-12
sh                          polaris_defconfig   gcc-14.1.0
sh                          r7785rp_defconfig   gcc-14.1.0
sh                           se7750_defconfig   gcc-14.1.0
sparc                            allmodconfig   gcc-14.1.0
sparc64                             defconfig   gcc-12
um                                allnoconfig   clang-20
um                                  defconfig   gcc-12
um                             i386_defconfig   gcc-12
um                           x86_64_defconfig   gcc-12
x86_64                            allnoconfig   clang-18
x86_64                           allyesconfig   clang-18
x86_64       buildonly-randconfig-001-20240904   clang-18
x86_64       buildonly-randconfig-002-20240904   clang-18
x86_64       buildonly-randconfig-003-20240904   clang-18
x86_64       buildonly-randconfig-004-20240904   clang-18
x86_64       buildonly-randconfig-005-20240904   clang-18
x86_64       buildonly-randconfig-006-20240904   clang-18
x86_64                              defconfig   clang-18
x86_64                                  kexec   gcc-12
x86_64                randconfig-001-20240904   clang-18
x86_64                randconfig-002-20240904   clang-18
x86_64                randconfig-003-20240904   clang-18
x86_64                randconfig-004-20240904   clang-18
x86_64                randconfig-005-20240904   clang-18
x86_64                randconfig-006-20240904   clang-18
x86_64                randconfig-011-20240904   clang-18
x86_64                randconfig-012-20240904   clang-18
x86_64                randconfig-013-20240904   clang-18
x86_64                randconfig-014-20240904   clang-18
x86_64                randconfig-015-20240904   clang-18
x86_64                randconfig-016-20240904   clang-18
x86_64                randconfig-071-20240904   clang-18
x86_64                randconfig-072-20240904   clang-18
x86_64                randconfig-073-20240904   clang-18
x86_64                randconfig-074-20240904   clang-18
x86_64                randconfig-075-20240904   clang-18
x86_64                randconfig-076-20240904   clang-18
x86_64                          rhel-8.3-rust   clang-18
x86_64                               rhel-8.3   gcc-12
xtensa                            allnoconfig   gcc-14.1.0
xtensa                  audio_kc705_defconfig   gcc-14.1.0
xtensa                         virt_defconfig   gcc-14.1.0

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox