Re: [PATCH v3 06/11] platform/x86/intel/pmt: re-order trigger logic

["David E. Box" <david.e.box@linux.intel.com>]

On Thu, 2025-06-05 at 14:44 -0400, 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.
> 
> 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 94858bfb52f8..aa3f57fbe018 100644
> --- a/drivers/platform/x86/intel/pmt/crashlog.c
> +++ b/drivers/platform/x86/intel/pmt/crashlog.c
> @@ -191,6 +191,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;
> @@ -200,10 +204,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;

LGTM

Reviewed-by: David E. Box <david.e.box@linux.intel.com>