[PATCH] acpi: pmic: Replace mutex_lock/unlock() with guard()/scoped_guard()

[PATCH] acpi: pmic: Replace mutex_lock/unlock() with guard()/scoped_guard()

[Maxwell Doose]

Replace mutex_lock() and unlock() macros with the newer guard() and
scoped_guard() macros. This will help modernize and clean the code.

In intel_soc_pmic_exec_mipi_pmic_seq_element(): While at it, remove
now redundant "ret" variable.

Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
 drivers/acpi/pmic/intel_pmic.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/pmic/intel_pmic.c b/drivers/acpi/pmic/intel_pmic.c
index 134e9ca8eaa2..36a43328c550 100644
--- a/drivers/acpi/pmic/intel_pmic.c
+++ b/drivers/acpi/pmic/intel_pmic.c
@@ -67,14 +67,12 @@ static acpi_status intel_pmic_power_handler(u32 function,
 	if (result == -ENOENT)
 		return AE_BAD_PARAMETER;
 
-	mutex_lock(&opregion->lock);
+	guard(&opregion->lock);
 
 	result = function == ACPI_READ ?
 		d->get_power(regmap, reg, bit, value64) :
 		d->update_power(regmap, reg, bit, *value64 == 1);
 
-	mutex_unlock(&opregion->lock);
-
 	return result ? AE_ERROR : AE_OK;
 }
 
@@ -182,7 +180,7 @@ static acpi_status intel_pmic_thermal_handler(u32 function,
 	if (result == -ENOENT)
 		return AE_BAD_PARAMETER;
 
-	mutex_lock(&opregion->lock);
+	scoped_guard(&opregion->lock) {
 
 	if (pmic_thermal_is_temp(address))
 		result = pmic_thermal_temp(opregion, reg, function, value64);
@@ -194,7 +192,7 @@ static acpi_status intel_pmic_thermal_handler(u32 function,
 	else
 		result = -EINVAL;
 
-	mutex_unlock(&opregion->lock);
+	}
 
 	if (result < 0) {
 		if (result == -EINVAL)
@@ -345,7 +343,6 @@ int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
 					      u32 value, u32 mask)
 {
 	const struct intel_pmic_opregion_data *d;
-	int ret;
 
 	if (!intel_pmic_opregion) {
 		pr_warn("%s: No PMIC registered\n", __func__);
@@ -354,30 +351,26 @@ int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,
 
 	d = intel_pmic_opregion->data;
 
-	mutex_lock(&intel_pmic_opregion->lock);
+	guard(&intel_pmic_opregion->lock);
 
 	if (d->exec_mipi_pmic_seq_element) {
-		ret = d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap,
+		return d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap,
 						    i2c_address, reg_address,
 						    value, mask);
 	} else if (d->pmic_i2c_address) {
 		if (i2c_address == d->pmic_i2c_address) {
-			ret = regmap_update_bits(intel_pmic_opregion->regmap,
+			return regmap_update_bits(intel_pmic_opregion->regmap,
 						 reg_address, mask, value);
 		} else {
 			pr_err("%s: Unexpected i2c-addr: 0x%02x (reg-addr 0x%x value 0x%x mask 0x%x)\n",
 			       __func__, i2c_address, reg_address, value, mask);
-			ret = -ENXIO;
+			return -ENXIO;
 		}
 	} else {
 		pr_warn("%s: Not implemented\n", __func__);
 		pr_warn("%s: i2c-addr: 0x%x reg-addr 0x%x value 0x%x mask 0x%x\n",
 			__func__, i2c_address, reg_address, value, mask);
-		ret = -EOPNOTSUPP;
+		return -EOPNOTSUPP;
 	}
-
-	mutex_unlock(&intel_pmic_opregion->lock);
-
-	return ret;
 }
 EXPORT_SYMBOL_GPL(intel_soc_pmic_exec_mipi_pmic_seq_element);
-- 
2.53.0

Re: [PATCH] acpi: pmic: Replace mutex_lock/unlock() with guard()/scoped_guard()

[Andy Shevchenko]

On Thu, Apr 23, 2026 at 10:55:01PM -0500, Maxwell Doose wrote:
> Replace mutex_lock() and unlock() macros with the newer guard() and
> scoped_guard() macros. This will help modernize and clean the code.
> 
> In intel_soc_pmic_exec_mipi_pmic_seq_element(): While at it, remove
> now redundant "ret" variable.

...

> static acpi_status intel_pmic_thermal_handler(u32 function,

> -	mutex_lock(&opregion->lock);
> +	scoped_guard(&opregion->lock) {
>  
>  	if (pmic_thermal_is_temp(address))
>  		result = pmic_thermal_temp(opregion, reg, function, value64);

>  	else
>  		result = -EINVAL;
>  
> -	mutex_unlock(&opregion->lock);
> +	}


No. You need to either split a helper and use guard()() there, or indent the
body of scoped_guard() accordingly.

...

> int intel_soc_pmic_exec_mipi_pmic_seq_element(u16 i2c_address, u32 reg_address,

> -	mutex_lock(&intel_pmic_opregion->lock);
> +	guard(&intel_pmic_opregion->lock);
>  
>  	if (d->exec_mipi_pmic_seq_element) {
> -		ret = d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap,
> +		return d->exec_mipi_pmic_seq_element(intel_pmic_opregion->regmap,
>  						    i2c_address, reg_address,
>  						    value, mask);
>  	} else if (d->pmic_i2c_address) {
>  		if (i2c_address == d->pmic_i2c_address) {
> -			ret = regmap_update_bits(intel_pmic_opregion->regmap,
> +			return regmap_update_bits(intel_pmic_opregion->regmap,
>  						 reg_address, mask, value);
>  		} else {
>  			pr_err("%s: Unexpected i2c-addr: 0x%02x (reg-addr 0x%x value 0x%x mask 0x%x)\n",
>  			       __func__, i2c_address, reg_address, value, mask);
> -			ret = -ENXIO;
> +			return -ENXIO;
>  		}
>  	} else {
>  		pr_warn("%s: Not implemented\n", __func__);
>  		pr_warn("%s: i2c-addr: 0x%x reg-addr 0x%x value 0x%x mask 0x%x\n",
>  			__func__, i2c_address, reg_address, value, mask);
> -		ret = -EOPNOTSUPP;
> +		return -EOPNOTSUPP;
>  	}
> -
> -	mutex_unlock(&intel_pmic_opregion->lock);
> -
> -	return ret;

This is not a switch-case and direct returns in if-else-if makes it harder to
follow. What you need is to drop now redundant 'else':s and leave the last one
unconditionally as the last code in the function.

>  }

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] acpi: pmic: Replace mutex_lock/unlock() with guard()/scoped_guard()

[Maxwell Doose]

On Fri, Apr 24, 2026 at 4:13 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> No. You need to either split a helper and use guard()() there, or indent the
> body of scoped_guard() accordingly.

Ah, I missed that. Thanks for pointing that out.

> This is not a switch-case and direct returns in if-else-if makes it harder to
> follow. What you need is to drop now redundant 'else':s and leave the last one
> unconditionally as the last code in the function.

Sounds good, I'll start adding those changes for a v2 for this at some point.

best regards,
maxwell