Re: [PATCH v3] counter: intel-qep: Replace manual mutex logic with lock guards

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 4214 bytes --]

On Tue, 12 May 2026, Joao Paulo Menezes Linaris wrote:

> Use guard() for handling mutex lock instead of locking and unlocking
> mutex explicitly. This improves readability by eliminating the need for
> gotos and by clearly indicating mutex will be locked only when
> execution is in guard scope.
> 
> Signed-off-by: Joao Paulo Menezes Linaris <jplinaris@usp.br>
> Co-developed-by: Guilherme Dias <guilhermeabreu200105@usp.br>
> Signed-off-by: Guilherme Dias <guilhermeabreu200105@usp.br>

Thanks.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

> ---
> v2 -> v3:
>  - add blank line between guard(mutex)() and other code
> 
>  drivers/counter/intel-qep.c | 50 +++++++++++++------------------------
>  1 file changed, 18 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/counter/intel-qep.c b/drivers/counter/intel-qep.c
> index c49c17805..9c6536f75 100644
> --- a/drivers/counter/intel-qep.c
> +++ b/drivers/counter/intel-qep.c
> @@ -188,25 +188,21 @@ static int intel_qep_ceiling_write(struct counter_device *counter,
>  				   struct counter_count *count, u64 max)
>  {
>  	struct intel_qep *qep = counter_priv(counter);
> -	int ret = 0;
>  
>  	/* Intel QEP ceiling configuration only supports 32-bit values */
>  	if (max != (u32)max)
>  		return -ERANGE;
>  
> -	mutex_lock(&qep->lock);
> -	if (qep->enabled) {
> -		ret = -EBUSY;
> -		goto out;
> -	}
> +	guard(mutex)(&qep->lock);
> +
> +	if (qep->enabled)
> +		return -EBUSY;
>  
>  	pm_runtime_get_sync(qep->dev);
>  	intel_qep_writel(qep, INTEL_QEPMAX, max);
>  	pm_runtime_put(qep->dev);
>  
> -out:
> -	mutex_unlock(&qep->lock);
> -	return ret;
> +	return 0;
>  }
>  
>  static int intel_qep_enable_read(struct counter_device *counter,
> @@ -226,10 +222,11 @@ static int intel_qep_enable_write(struct counter_device *counter,
>  	u32 reg;
>  	bool changed;
>  
> -	mutex_lock(&qep->lock);
> +	guard(mutex)(&qep->lock);
> +
>  	changed = val ^ qep->enabled;
>  	if (!changed)
> -		goto out;
> +		return 0;
>  
>  	pm_runtime_get_sync(qep->dev);
>  	reg = intel_qep_readl(qep, INTEL_QEPCON);
> @@ -246,8 +243,6 @@ static int intel_qep_enable_write(struct counter_device *counter,
>  	pm_runtime_put(qep->dev);
>  	qep->enabled = val;
>  
> -out:
> -	mutex_unlock(&qep->lock);
>  	return 0;
>  }
>  
> @@ -279,7 +274,6 @@ static int intel_qep_spike_filter_ns_write(struct counter_device *counter,
>  	struct intel_qep *qep = counter_priv(counter);
>  	u32 reg;
>  	bool enable;
> -	int ret = 0;
>  
>  	/*
>  	 * Spike filter length is (MAX_COUNT + 2) clock periods.
> @@ -300,11 +294,10 @@ static int intel_qep_spike_filter_ns_write(struct counter_device *counter,
>  	if (length > INTEL_QEPFLT_MAX_COUNT(length))
>  		return -ERANGE;
>  
> -	mutex_lock(&qep->lock);
> -	if (qep->enabled) {
> -		ret = -EBUSY;
> -		goto out;
> -	}
> +	guard(mutex)(&qep->lock);
> +
> +	if (qep->enabled)
> +		return -EBUSY;
>  
>  	pm_runtime_get_sync(qep->dev);
>  	reg = intel_qep_readl(qep, INTEL_QEPCON);
> @@ -316,9 +309,7 @@ static int intel_qep_spike_filter_ns_write(struct counter_device *counter,
>  	intel_qep_writel(qep, INTEL_QEPCON, reg);
>  	pm_runtime_put(qep->dev);
>  
> -out:
> -	mutex_unlock(&qep->lock);
> -	return ret;
> +	return 0;
>  }
>  
>  static int intel_qep_preset_enable_read(struct counter_device *counter,
> @@ -342,13 +333,11 @@ static int intel_qep_preset_enable_write(struct counter_device *counter,
>  {
>  	struct intel_qep *qep = counter_priv(counter);
>  	u32 reg;
> -	int ret = 0;
>  
> -	mutex_lock(&qep->lock);
> -	if (qep->enabled) {
> -		ret = -EBUSY;
> -		goto out;
> -	}
> +	guard(mutex)(&qep->lock);
> +
> +	if (qep->enabled)
> +		return -EBUSY;
>  
>  	pm_runtime_get_sync(qep->dev);
>  	reg = intel_qep_readl(qep, INTEL_QEPCON);
> @@ -360,10 +349,7 @@ static int intel_qep_preset_enable_write(struct counter_device *counter,
>  	intel_qep_writel(qep, INTEL_QEPCON, reg);
>  	pm_runtime_put(qep->dev);
>  
> -out:
> -	mutex_unlock(&qep->lock);
> -
> -	return ret;
> +	return 0;
>  }
>  
>  static struct counter_comp intel_qep_count_ext[] = {
>