Re: [PATCH 03/10] platform/x86/intel/pmt: use guard(mutex)

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

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

On Fri, 30 May 2025, Michael J. Ruhl wrote:

> Update the mutex paths to use the new guard() mechanism.
> 
> With the removal of goto, do some minor cleanup of the current
> logic path.
> 
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
>  drivers/platform/x86/intel/pmt/crashlog.c | 32 +++++++++++------------
>  1 file changed, 15 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
> index d40c8e212733..c6d8a7a61d39 100644
> --- a/drivers/platform/x86/intel/pmt/crashlog.c
> +++ b/drivers/platform/x86/intel/pmt/crashlog.c
> @@ -155,9 +155,9 @@ enable_store(struct device *dev, struct device_attribute *attr,
>  	if (result)
>  		return result;
>  
> -	mutex_lock(&entry->control_mutex);
> +	guard(mutex)(&entry->control_mutex);
> +
>  	pmt_crashlog_set_disable(&entry->entry, !enabled);
> -	mutex_unlock(&entry->control_mutex);
>  
>  	return count;
>  }
> @@ -189,26 +189,24 @@ trigger_store(struct device *dev, struct device_attribute *attr,
>  	if (result)
>  		return result;
>  
> -	mutex_lock(&entry->control_mutex);
> +	guard(mutex)(&entry->control_mutex);
>  
>  	if (!trigger) {
>  		pmt_crashlog_set_clear(&entry->entry);
> -	} else if (pmt_crashlog_complete(&entry->entry)) {
> -		/* we cannot trigger a new crash if one is still pending */
> -		result = -EEXIST;
> -		goto err;
> -	} else if (pmt_crashlog_disabled(&entry->entry)) {
> -		/* if device is currently disabled, return busy */
> -		result = -EBUSY;
> -		goto err;
> -	} else {
> -		pmt_crashlog_set_execute(&entry->entry);
> +		return count;
>  	}
>  
> -	result = count;
> -err:
> -	mutex_unlock(&entry->control_mutex);
> -	return result;
> +	/* we cannot trigger a new crash if one is still pending */
> +	if (pmt_crashlog_complete(&entry->entry))
> +		return -EEXIST;
> +
> +	/* if device is currently disabled, return busy */
> +	if (pmt_crashlog_disabled(&entry->entry))
> +		return -EBUSY;
> +
> +	pmt_crashlog_set_execute(&entry->entry);
> +
> +	return count;
>  }
>  static DEVICE_ATTR_RW(trigger);

Thanks, the control flow is very straightforward after this change.

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


-- 
 i.