Re: [PATCH v4 05/10] platform/x86/intel/pmt: re-order trigger logic

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

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

On Tue, 10 Jun 2025, Michael J. Ruhl wrote:

> Setting the clear bit or checking the complete bit before checking to
> see if crashlog is disabled seems incorrect.
> 
> Check disable before accessing any other bits.
> 
> Reviewed-by: David E. Box <david.e.box@linux.intel.com>
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
>  drivers/platform/x86/intel/pmt/crashlog.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
> index 1525e702e993..3d3b72451c01 100644
> --- a/drivers/platform/x86/intel/pmt/crashlog.c
> +++ b/drivers/platform/x86/intel/pmt/crashlog.c
> @@ -192,6 +192,10 @@ trigger_store(struct device *dev, struct device_attribute *attr,
>  
>  	guard(mutex)(&entry->control_mutex);
>  
> +	/* if device is currently disabled, return busy */
> +	if (pmt_crashlog_disabled(&entry->entry))
> +		return -EBUSY;
> +
>  	if (!trigger) {
>  		pmt_crashlog_set_clear(&entry->entry);
>  		return count;
> @@ -201,10 +205,6 @@ trigger_store(struct device *dev, struct device_attribute *attr,
>  	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;
> 

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

-- 
 i.