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

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

[Joao Paulo Menezes Linaris]

Use scoped_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 scoped_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>
---
 drivers/counter/intel-qep.c | 126 +++++++++++++++++-------------------
 1 file changed, 58 insertions(+), 68 deletions(-)

diff --git a/drivers/counter/intel-qep.c b/drivers/counter/intel-qep.c
index c49c17805..199a8d562 100644
--- a/drivers/counter/intel-qep.c
+++ b/drivers/counter/intel-qep.c
@@ -188,25 +188,22 @@ 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;
-	}
+	/* Lock mutex while execution in scoped_guard() scope */
+	scoped_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);
+		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,28 +223,28 @@ static int intel_qep_enable_write(struct counter_device *counter,
 	u32 reg;
 	bool changed;
 
-	mutex_lock(&qep->lock);
-	changed = val ^ qep->enabled;
-	if (!changed)
-		goto out;
-
-	pm_runtime_get_sync(qep->dev);
-	reg = intel_qep_readl(qep, INTEL_QEPCON);
-	if (val) {
-		/* Enable peripheral and keep runtime PM always on */
-		reg |= INTEL_QEPCON_EN;
-		pm_runtime_get_noresume(qep->dev);
-	} else {
-		/* Let runtime PM be idle and disable peripheral */
-		pm_runtime_put_noidle(qep->dev);
-		reg &= ~INTEL_QEPCON_EN;
+	/* Lock mutex while execution in scoped_guard() scope */
+	scoped_guard(mutex, &qep->lock){
+		changed = val ^ qep->enabled;
+		if (!changed) 
+			return 0;
+
+		pm_runtime_get_sync(qep->dev);
+		reg = intel_qep_readl(qep, INTEL_QEPCON);
+		if (val) {
+			/* Enable peripheral and keep runtime PM always on */
+			reg |= INTEL_QEPCON_EN;
+			pm_runtime_get_noresume(qep->dev);
+		} else {
+			/* Let runtime PM be idle and disable peripheral */
+			pm_runtime_put_noidle(qep->dev);
+			reg &= ~INTEL_QEPCON_EN;
+		}
+		intel_qep_writel(qep, INTEL_QEPCON, reg);
+		pm_runtime_put(qep->dev);
+		qep->enabled = val;
 	}
-	intel_qep_writel(qep, INTEL_QEPCON, reg);
-	pm_runtime_put(qep->dev);
-	qep->enabled = val;
 
-out:
-	mutex_unlock(&qep->lock);
 	return 0;
 }
 
@@ -279,7 +276,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,25 +296,23 @@ 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;
+	/* Lock mutex while execution in scoped_guard() scope */
+	scoped_guard(mutex, &qep->lock){
+		if (qep->enabled)
+			return -EBUSY;
+
+		pm_runtime_get_sync(qep->dev);
+		reg = intel_qep_readl(qep, INTEL_QEPCON);
+		if (enable)
+			reg |= INTEL_QEPCON_FLT_EN;
+		else
+			reg &= ~INTEL_QEPCON_FLT_EN;
+		intel_qep_writel(qep, INTEL_QEPFLT, length);
+		intel_qep_writel(qep, INTEL_QEPCON, reg);
+		pm_runtime_put(qep->dev);
 	}
 
-	pm_runtime_get_sync(qep->dev);
-	reg = intel_qep_readl(qep, INTEL_QEPCON);
-	if (enable)
-		reg |= INTEL_QEPCON_FLT_EN;
-	else
-		reg &= ~INTEL_QEPCON_FLT_EN;
-	intel_qep_writel(qep, INTEL_QEPFLT, length);
-	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,28 +336,24 @@ 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;
-	}
 
-	pm_runtime_get_sync(qep->dev);
-	reg = intel_qep_readl(qep, INTEL_QEPCON);
-	if (val)
-		reg &= ~INTEL_QEPCON_COUNT_RST_MODE;
-	else
-		reg |= INTEL_QEPCON_COUNT_RST_MODE;
+	/* Lock mutex while execution in scoped_guard() scope */
+	scoped_guard(mutex, &qep->lock){
+		if (qep->enabled) 
+			return -EBUSY;
 
-	intel_qep_writel(qep, INTEL_QEPCON, reg);
-	pm_runtime_put(qep->dev);
+		pm_runtime_get_sync(qep->dev);
+		reg = intel_qep_readl(qep, INTEL_QEPCON);
+		if (val)
+			reg &= ~INTEL_QEPCON_COUNT_RST_MODE;
+		else
+			reg |= INTEL_QEPCON_COUNT_RST_MODE;
 
-out:
-	mutex_unlock(&qep->lock);
+		intel_qep_writel(qep, INTEL_QEPCON, reg);
+		pm_runtime_put(qep->dev);
+	}
 
-	return ret;
+	return 0;
 }
 
 static struct counter_comp intel_qep_count_ext[] = {
-- 
2.34.1

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

[Ilpo Järvinen]

On Wed, 22 Apr 2026, Joao Paulo Menezes Linaris wrote:

> Use scoped_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 scoped_guard() scope.

Please fold the changelog text properly.

> 
> 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>
> ---
>  drivers/counter/intel-qep.c | 126 +++++++++++++++++-------------------
>  1 file changed, 58 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/counter/intel-qep.c b/drivers/counter/intel-qep.c
> index c49c17805..199a8d562 100644
> --- a/drivers/counter/intel-qep.c
> +++ b/drivers/counter/intel-qep.c
> @@ -188,25 +188,22 @@ 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;
> -	}
> +	/* Lock mutex while execution in scoped_guard() scope */

Why add dead obvious comments like this. Please don't do it.

> +	scoped_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);
> +		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;

Why is using scoped variable of guard necessary in these cases??? Are 
you just making these changes using some tool without inspecting if the 
end result makes sense or not??

Also, runtime PM also has guards support added (admittedly changing them 
doesn't remove gotos). But one of the cases seems non-pairing so that 
seems not convertable to guard.

>  }
>  
>  static int intel_qep_enable_read(struct counter_device *counter,
> @@ -226,28 +223,28 @@ static int intel_qep_enable_write(struct counter_device *counter,
>  	u32 reg;
>  	bool changed;
>  
> -	mutex_lock(&qep->lock);
> -	changed = val ^ qep->enabled;
> -	if (!changed)
> -		goto out;
> -
> -	pm_runtime_get_sync(qep->dev);
> -	reg = intel_qep_readl(qep, INTEL_QEPCON);
> -	if (val) {
> -		/* Enable peripheral and keep runtime PM always on */
> -		reg |= INTEL_QEPCON_EN;
> -		pm_runtime_get_noresume(qep->dev);
> -	} else {
> -		/* Let runtime PM be idle and disable peripheral */
> -		pm_runtime_put_noidle(qep->dev);
> -		reg &= ~INTEL_QEPCON_EN;
> +	/* Lock mutex while execution in scoped_guard() scope */
> +	scoped_guard(mutex, &qep->lock){
> +		changed = val ^ qep->enabled;
> +		if (!changed) 
> +			return 0;
> +
> +		pm_runtime_get_sync(qep->dev);
> +		reg = intel_qep_readl(qep, INTEL_QEPCON);
> +		if (val) {
> +			/* Enable peripheral and keep runtime PM always on */
> +			reg |= INTEL_QEPCON_EN;
> +			pm_runtime_get_noresume(qep->dev);
> +		} else {
> +			/* Let runtime PM be idle and disable peripheral */
> +			pm_runtime_put_noidle(qep->dev);
> +			reg &= ~INTEL_QEPCON_EN;
> +		}
> +		intel_qep_writel(qep, INTEL_QEPCON, reg);
> +		pm_runtime_put(qep->dev);
> +		qep->enabled = val;
>  	}
> -	intel_qep_writel(qep, INTEL_QEPCON, reg);
> -	pm_runtime_put(qep->dev);
> -	qep->enabled = val;
>  
> -out:
> -	mutex_unlock(&qep->lock);
>  	return 0;
>  }
>  
> @@ -279,7 +276,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,25 +296,23 @@ 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;
> +	/* Lock mutex while execution in scoped_guard() scope */
> +	scoped_guard(mutex, &qep->lock){
> +		if (qep->enabled)
> +			return -EBUSY;
> +
> +		pm_runtime_get_sync(qep->dev);
> +		reg = intel_qep_readl(qep, INTEL_QEPCON);
> +		if (enable)
> +			reg |= INTEL_QEPCON_FLT_EN;
> +		else
> +			reg &= ~INTEL_QEPCON_FLT_EN;
> +		intel_qep_writel(qep, INTEL_QEPFLT, length);
> +		intel_qep_writel(qep, INTEL_QEPCON, reg);
> +		pm_runtime_put(qep->dev);
>  	}
>  
> -	pm_runtime_get_sync(qep->dev);
> -	reg = intel_qep_readl(qep, INTEL_QEPCON);
> -	if (enable)
> -		reg |= INTEL_QEPCON_FLT_EN;
> -	else
> -		reg &= ~INTEL_QEPCON_FLT_EN;
> -	intel_qep_writel(qep, INTEL_QEPFLT, length);
> -	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,28 +336,24 @@ 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;
> -	}
>  
> -	pm_runtime_get_sync(qep->dev);
> -	reg = intel_qep_readl(qep, INTEL_QEPCON);
> -	if (val)
> -		reg &= ~INTEL_QEPCON_COUNT_RST_MODE;
> -	else
> -		reg |= INTEL_QEPCON_COUNT_RST_MODE;
> +	/* Lock mutex while execution in scoped_guard() scope */
> +	scoped_guard(mutex, &qep->lock){
> +		if (qep->enabled) 
> +			return -EBUSY;
>  
> -	intel_qep_writel(qep, INTEL_QEPCON, reg);
> -	pm_runtime_put(qep->dev);
> +		pm_runtime_get_sync(qep->dev);
> +		reg = intel_qep_readl(qep, INTEL_QEPCON);
> +		if (val)
> +			reg &= ~INTEL_QEPCON_COUNT_RST_MODE;
> +		else
> +			reg |= INTEL_QEPCON_COUNT_RST_MODE;
>  
> -out:
> -	mutex_unlock(&qep->lock);
> +		intel_qep_writel(qep, INTEL_QEPCON, reg);
> +		pm_runtime_put(qep->dev);
> +	}
>  
> -	return ret;
> +	return 0;
>  }
>  
>  static struct counter_comp intel_qep_count_ext[] = {
> 

-- 
 i.

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

[Joao Paulo Menezes Linaris]

Hello. Thanks for the suggestions!

Em qui., 23 de abr. de 2026 às 08:42, Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> escreveu:
>
> On Wed, 22 Apr 2026, Joao Paulo Menezes Linaris wrote:
>
> > Use scoped_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 scoped_guard() scope.
>
> Please fold the changelog text properly.
>
> >
> > 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>
> > ---
> >  drivers/counter/intel-qep.c | 126 +++++++++++++++++-------------------
> >  1 file changed, 58 insertions(+), 68 deletions(-)
> >
> > diff --git a/drivers/counter/intel-qep.c b/drivers/counter/intel-qep.c
> > index c49c17805..199a8d562 100644
> > --- a/drivers/counter/intel-qep.c
> > +++ b/drivers/counter/intel-qep.c
> > @@ -188,25 +188,22 @@ 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;
> > -     }
> > +     /* Lock mutex while execution in scoped_guard() scope */
>
> Why add dead obvious comments like this. Please don't do it.
>
> > +     scoped_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);
> > +             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;
>
> Why is using scoped variable of guard necessary in these cases??? Are
> you just making these changes using some tool without inspecting if the
> end result makes sense or not??
>
> Also, runtime PM also has guards support added (admittedly changing them
> doesn't remove gotos). But one of the cases seems non-pairing so that
> seems not convertable to guard.
>
Just to be sure I'm on the right page regarding this suggestion,
according to pm_runtime documentation, pm_runtim_get_sync and
pm_runtime_put, in the functions I changed, would be the pairs we are
talking about. And, therefore, if we have something like:

ret = pm_runtime_get_sync(dev);
if (ret < 0)
    // returns before the decrement of device's usage counter
    return ret;

pm_runtime_put(dev);

This would be considered a non-pairing case, which seems not
convertible to guard. Is that right? In the functions I implemented
the lock guards, I couldn't find any case in which the function could
return in between get/put calls. In this case, would it be correct to
keep all the added guards?

> >  }
> >
> >  static int intel_qep_enable_read(struct counter_device *counter,
> > @@ -226,28 +223,28 @@ static int intel_qep_enable_write(struct counter_device *counter,
> >       u32 reg;
> >       bool changed;
> >
> > -     mutex_lock(&qep->lock);
> > -     changed = val ^ qep->enabled;
> > -     if (!changed)
> > -             goto out;
> > -
> > -     pm_runtime_get_sync(qep->dev);
> > -     reg = intel_qep_readl(qep, INTEL_QEPCON);
> > -     if (val) {
> > -             /* Enable peripheral and keep runtime PM always on */
> > -             reg |= INTEL_QEPCON_EN;
> > -             pm_runtime_get_noresume(qep->dev);
> > -     } else {
> > -             /* Let runtime PM be idle and disable peripheral */
> > -             pm_runtime_put_noidle(qep->dev);
> > -             reg &= ~INTEL_QEPCON_EN;
> > +     /* Lock mutex while execution in scoped_guard() scope */
> > +     scoped_guard(mutex, &qep->lock){
> > +             changed = val ^ qep->enabled;
> > +             if (!changed)
> > +                     return 0;
> > +
> > +             pm_runtime_get_sync(qep->dev);
> > +             reg = intel_qep_readl(qep, INTEL_QEPCON);
> > +             if (val) {
> > +                     /* Enable peripheral and keep runtime PM always on */
> > +                     reg |= INTEL_QEPCON_EN;
> > +                     pm_runtime_get_noresume(qep->dev);
> > +             } else {
> > +                     /* Let runtime PM be idle and disable peripheral */
> > +                     pm_runtime_put_noidle(qep->dev);
> > +                     reg &= ~INTEL_QEPCON_EN;
> > +             }
> > +             intel_qep_writel(qep, INTEL_QEPCON, reg);
> > +             pm_runtime_put(qep->dev);
> > +             qep->enabled = val;
> >       }
> > -     intel_qep_writel(qep, INTEL_QEPCON, reg);
> > -     pm_runtime_put(qep->dev);
> > -     qep->enabled = val;
> >
> > -out:
> > -     mutex_unlock(&qep->lock);
> >       return 0;
> >  }
> >
> > @@ -279,7 +276,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,25 +296,23 @@ 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;
> > +     /* Lock mutex while execution in scoped_guard() scope */
> > +     scoped_guard(mutex, &qep->lock){
> > +             if (qep->enabled)
> > +                     return -EBUSY;
> > +
> > +             pm_runtime_get_sync(qep->dev);
> > +             reg = intel_qep_readl(qep, INTEL_QEPCON);
> > +             if (enable)
> > +                     reg |= INTEL_QEPCON_FLT_EN;
> > +             else
> > +                     reg &= ~INTEL_QEPCON_FLT_EN;
> > +             intel_qep_writel(qep, INTEL_QEPFLT, length);
> > +             intel_qep_writel(qep, INTEL_QEPCON, reg);
> > +             pm_runtime_put(qep->dev);
> >       }
> >
> > -     pm_runtime_get_sync(qep->dev);
> > -     reg = intel_qep_readl(qep, INTEL_QEPCON);
> > -     if (enable)
> > -             reg |= INTEL_QEPCON_FLT_EN;
> > -     else
> > -             reg &= ~INTEL_QEPCON_FLT_EN;
> > -     intel_qep_writel(qep, INTEL_QEPFLT, length);
> > -     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,28 +336,24 @@ 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;
> > -     }
> >
> > -     pm_runtime_get_sync(qep->dev);
> > -     reg = intel_qep_readl(qep, INTEL_QEPCON);
> > -     if (val)
> > -             reg &= ~INTEL_QEPCON_COUNT_RST_MODE;
> > -     else
> > -             reg |= INTEL_QEPCON_COUNT_RST_MODE;
> > +     /* Lock mutex while execution in scoped_guard() scope */
> > +     scoped_guard(mutex, &qep->lock){
> > +             if (qep->enabled)
> > +                     return -EBUSY;
> >
> > -     intel_qep_writel(qep, INTEL_QEPCON, reg);
> > -     pm_runtime_put(qep->dev);
> > +             pm_runtime_get_sync(qep->dev);
> > +             reg = intel_qep_readl(qep, INTEL_QEPCON);
> > +             if (val)
> > +                     reg &= ~INTEL_QEPCON_COUNT_RST_MODE;
> > +             else
> > +                     reg |= INTEL_QEPCON_COUNT_RST_MODE;
> >
> > -out:
> > -     mutex_unlock(&qep->lock);
> > +             intel_qep_writel(qep, INTEL_QEPCON, reg);
> > +             pm_runtime_put(qep->dev);
> > +     }
> >
> > -     return ret;
> > +     return 0;
> >  }
> >
> >  static struct counter_comp intel_qep_count_ext[] = {
> >
>
> --
>  i.
>
--
Best regards,
Joao Paulo Menezes Linaris

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

[Joao Paulo Menezes Linaris]

Use scoped_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>
---
v1 -> v2:
 - fold the commit message properly
 - replace scoped_guard() with guard()
 - remove unnecessary comments

 drivers/counter/intel-qep.c | 47 +++++++++++--------------------------
 1 file changed, 14 insertions(+), 33 deletions(-)

diff --git a/drivers/counter/intel-qep.c b/drivers/counter/intel-qep.c
index c49c17805..ef6eb3249 100644
--- a/drivers/counter/intel-qep.c
+++ b/drivers/counter/intel-qep.c
@@ -188,25 +188,20 @@ 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 +221,10 @@ 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 +241,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 +272,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 +292,9 @@ 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 +306,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 +330,9 @@ 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 +344,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[] = {
-- 
2.34.1

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

[Maxwell Doose]

Hi Joao, comments inline.

On Thu May 7, 2026 at 10:40 PM CDT, Joao Paulo Menezes Linaris wrote:
> Use scoped_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>
> ---
> v1 -> v2:
>  - fold the commit message properly
>  - replace scoped_guard() with guard()
>  - remove unnecessary comments
>

If this is the v2 then the subject should be:

[PATCH v2] counter: ...rest of subject here...

You can do this by running git format-patch -1 -v2.

> +	guard(mutex)(&qep->lock);
> +	if (qep->enabled)
> +		return -EBUSY;
>  

Would recommend putting blank lines between guard(mutex)() and other
code, Andy usually treats the guard()()s as separate things.

>
>  	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 +221,10 @@ static int intel_qep_enable_write(struct counter_device *counter,
>  	u32 reg;
>  	bool changed;
>  
> -	mutex_lock(&qep->lock);
> +	guard(mutex)(&qep->lock);
>

Again (and so on and so forth).

best regards,
max