Re: [PATCH v5 03/18] platform/x86/intel/pmt: refactor rc with a return value

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Inverted logic in `enable_show` causes the sysfs attribute to output 1 when the device is disabled and 0 when it is enabled.
--

commit 861c9b2ba4669926e83219067272a187332bb551
Author: Michael J. Ruhl <michael.j.ruhl@intel.com>

platform/x86/intel/pmt: refactor rc with a return value

Upcoming changes will include possible failures from HW accesses.
Refactor pmt_crashlog_rw() with a return value.
Update all necessary usage to use the return code.

> diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
> index 831cff743803e..808f2b695a7f7 100644
> --- a/drivers/platform/x86/intel/pmt/crashlog.c
> +++ b/drivers/platform/x86/intel/pmt/crashlog.c

[ ... ]

> @@ -312,9 +332,14 @@ static ssize_t
>  enable_show(struct device *dev, struct device_attribute *attr, char *buf)
>  {
>  	struct crashlog_entry *crashlog = dev_get_drvdata(dev);
> -	bool enabled = !pmt_crashlog_disabled(crashlog);
> +	bool enabled;
> +	int ret;
> +
> +	ret = pmt_crashlog_disabled(crashlog, &enabled);
> +	if (ret)
> +		return ret;
>  
> -	return sprintf(buf, "%d\n", enabled);
> +	return sysfs_emit(buf, "%d\n", enabled);

[Severity: Medium]
Does this code invert the logic for the enable sysfs attribute?

Looking at enable_show() in drivers/platform/x86/intel/pmt/crashlog.c,
pmt_crashlog_disabled() populates the enabled variable with the hardware's
disabled state (true if disabled, false if enabled).

Because the value is no longer negated like the original code
(!pmt_crashlog_disabled(crashlog)), reading the enable sysfs file will
incorrectly output 1 when the device is disabled and 0 when it is enabled.

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903201002.1064889-20-michael.j.ruhl@intel.com?part=3