From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: "Michael J. Ruhl" <michael.j.ruhl@intel.com>
Cc: platform-driver-x86@vger.kernel.org,
intel-xe@lists.freedesktop.org, Hans de Goede <hansg@kernel.org>,
matthew.brost@intel.com, rodrigo.vivi@intel.com,
thomas.hellstrom@linux.intel.com, airlied@gmail.com,
simona@ffwll.ch, david.e.box@linux.intel.com,
anoop.c.vijay@intel.com, badal.nilawar@intel.com,
matthew.d.roper@intel.com, james.ausmus@intel.com,
karthik.poosa@intel.com
Subject: Re: [PATCH v4 02/18] platform/x86/intel/pmt: refactor rmw with a return value
Date: Wed, 2 Sep 2026 12:05:37 +0300 (EEST) [thread overview]
Message-ID: <82808204-b8a3-9c94-779d-1b1851146d4d@linux.intel.com> (raw)
In-Reply-To: <20260901192736.626777-22-michael.j.ruhl@intel.com>
On Tue, 1 Sep 2026, Michael J. Ruhl wrote:
> Upcoming changes will include possible failures from HW
> accesses.
>
> Refactor pmt_crashlog_rwm with a return value.
>
> Update all necessary usage to use the return code.
>
> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> ---
> drivers/platform/x86/intel/pmt/crashlog.c | 88 ++++++++++++-----------
> 1 file changed, 45 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
> index f936daf99e4d..ebc7880d95bd 100644
> --- a/drivers/platform/x86/intel/pmt/crashlog.c
> +++ b/drivers/platform/x86/intel/pmt/crashlog.c
> @@ -125,7 +125,7 @@ struct pmt_crashlog_priv {
> */
>
> /* Read, modify, write the control register, setting or clearing @bit based on @set */
> -static void pmt_crashlog_rmw(struct crashlog_entry *crashlog, u32 bit, bool set)
> +static int pmt_crashlog_rmw(struct crashlog_entry *crashlog, u32 bit, bool set)
> {
> const struct crashlog_control *control = &crashlog->info->control;
> struct intel_pmt_entry *entry = &crashlog->entry;
> @@ -139,6 +139,8 @@ static void pmt_crashlog_rmw(struct crashlog_entry *crashlog, u32 bit, bool set)
> reg &= ~bit;
>
> writel(reg, entry->disc_table + control->offset);
> +
> + return 0;
> }
>
> /* Read the status register and see if the specified @bit is set */
> @@ -181,20 +183,20 @@ static bool pmt_crashlog_supported(struct intel_pmt_entry *entry, u32 *crash_typ
> return false;
> }
>
> -static void pmt_crashlog_set_disable(struct crashlog_entry *crashlog,
> +static int pmt_crashlog_set_disable(struct crashlog_entry *crashlog,
> bool disable)
> {
> - pmt_crashlog_rmw(crashlog, crashlog->info->control.disable, disable);
> + return pmt_crashlog_rmw(crashlog, crashlog->info->control.disable, disable);
> }
>
> -static void pmt_crashlog_set_clear(struct crashlog_entry *crashlog)
> +static int pmt_crashlog_set_clear(struct crashlog_entry *crashlog)
> {
> - pmt_crashlog_rmw(crashlog, crashlog->info->control.clear, true);
> + return pmt_crashlog_rmw(crashlog, crashlog->info->control.clear, true);
> }
>
> -static void pmt_crashlog_set_execute(struct crashlog_entry *crashlog)
> +static int pmt_crashlog_set_execute(struct crashlog_entry *crashlog)
> {
> - pmt_crashlog_rmw(crashlog, crashlog->info->control.manual, true);
> + return pmt_crashlog_rmw(crashlog, crashlog->info->control.manual, true);
> }
>
> static bool pmt_crashlog_cleared(struct crashlog_entry *crashlog)
> @@ -207,9 +209,9 @@ static bool pmt_crashlog_consumed(struct crashlog_entry *crashlog)
> return pmt_crashlog_rc(crashlog, crashlog->info->status.consumed);
> }
>
> -static void pmt_crashlog_set_consumed(struct crashlog_entry *crashlog)
> +static int pmt_crashlog_set_consumed(struct crashlog_entry *crashlog)
> {
> - pmt_crashlog_rmw(crashlog, crashlog->info->control.consume, true);
> + return pmt_crashlog_rmw(crashlog, crashlog->info->control.consume, true);
> }
>
> static bool pmt_crashlog_error(struct crashlog_entry *crashlog)
> @@ -222,9 +224,9 @@ static bool pmt_crashlog_rearm(struct crashlog_entry *crashlog)
> return pmt_crashlog_rc(crashlog, crashlog->info->status.rearmed);
> }
>
> -static void pmt_crashlog_set_rearm(struct crashlog_entry *crashlog)
> +static int pmt_crashlog_set_rearm(struct crashlog_entry *crashlog)
> {
> - pmt_crashlog_rmw(crashlog, crashlog->info->control.rearm, true);
> + return pmt_crashlog_rmw(crashlog, crashlog->info->control.rearm, true);
> }
>
> /*
> @@ -245,13 +247,13 @@ clear_store(struct device *dev, struct device_attribute *attr,
> {
> struct crashlog_entry *crashlog;
> bool clear;
> - int result;
> + int ret;
>
> crashlog = dev_get_drvdata(dev);
>
> - result = kstrtobool(buf, &clear);
> - if (result)
> - return result;
> + ret = kstrtobool(buf, &clear);
> + if (ret)
> + return ret;
>
> /* set bit only */
> if (!clear)
> @@ -259,9 +261,9 @@ clear_store(struct device *dev, struct device_attribute *attr,
>
> guard(mutex)(&crashlog->control_mutex);
>
> - pmt_crashlog_set_clear(crashlog);
> + ret = pmt_crashlog_set_clear(crashlog);
>
> - return count;
> + return ret ? ret : count;
> }
> static DEVICE_ATTR_RW(clear);
>
> @@ -280,13 +282,13 @@ consumed_store(struct device *dev, struct device_attribute *attr, const char *bu
> {
> struct crashlog_entry *crashlog;
> bool consumed;
> - int result;
> + int ret;
>
> crashlog = dev_get_drvdata(dev);
>
> - result = kstrtobool(buf, &consumed);
> - if (result)
> - return result;
> + ret = kstrtobool(buf, &consumed);
> + if (ret)
> + return ret;
>
> /* set bit only */
> if (!consumed)
> @@ -300,9 +302,9 @@ consumed_store(struct device *dev, struct device_attribute *attr, const char *bu
> if (!pmt_crashlog_complete(crashlog))
> return -EEXIST;
>
> - pmt_crashlog_set_consumed(crashlog);
> + ret = pmt_crashlog_set_consumed(crashlog);
>
> - return count;
> + return ret ? ret : count;
You can use the more concise form for these:
return ret ?: count;
--
i.
> }
> static DEVICE_ATTR_RW(consumed);
>
> @@ -321,19 +323,19 @@ enable_store(struct device *dev, struct device_attribute *attr,
> {
> struct crashlog_entry *crashlog;
> bool enabled;
> - int result;
> + int ret;
>
> crashlog = dev_get_drvdata(dev);
>
> - result = kstrtobool(buf, &enabled);
> - if (result)
> - return result;
> + ret = kstrtobool(buf, &enabled);
> + if (ret)
> + return ret;
>
> guard(mutex)(&crashlog->control_mutex);
>
> - pmt_crashlog_set_disable(crashlog, !enabled);
> + ret = pmt_crashlog_set_disable(crashlog, !enabled);
>
> - return count;
> + return ret ? ret : count;
> }
> static DEVICE_ATTR_RW(enable);
>
> @@ -362,13 +364,13 @@ rearm_store(struct device *dev, struct device_attribute *attr, const char *buf,
> {
> struct crashlog_entry *crashlog;
> bool rearm;
> - int result;
> + int ret;
>
> crashlog = dev_get_drvdata(dev);
>
> - result = kstrtobool(buf, &rearm);
> - if (result)
> - return result;
> + ret = kstrtobool(buf, &rearm);
> + if (ret)
> + return ret;
>
> /* set only */
> if (!rearm)
> @@ -376,9 +378,9 @@ rearm_store(struct device *dev, struct device_attribute *attr, const char *buf,
>
> guard(mutex)(&crashlog->control_mutex);
>
> - pmt_crashlog_set_rearm(crashlog);
> + ret = pmt_crashlog_set_rearm(crashlog);
>
> - return count;
> + return ret ? ret : count;
> }
> static DEVICE_ATTR_RW(rearm);
>
> @@ -400,13 +402,13 @@ trigger_store(struct device *dev, struct device_attribute *attr,
> {
> struct crashlog_entry *crashlog;
> bool trigger;
> - int result;
> + int ret;
>
> crashlog = dev_get_drvdata(dev);
>
> - result = kstrtobool(buf, &trigger);
> - if (result)
> - return result;
> + ret = kstrtobool(buf, &trigger);
> + if (ret)
> + return ret;
>
> guard(mutex)(&crashlog->control_mutex);
>
> @@ -415,17 +417,17 @@ trigger_store(struct device *dev, struct device_attribute *attr,
> return -EBUSY;
>
> if (!trigger) {
> - pmt_crashlog_set_clear(crashlog);
> - return count;
> + ret = pmt_crashlog_set_clear(crashlog);
> + return ret ? ret : count;
> }
>
> /* we cannot trigger a new crash if one is still pending */
> if (pmt_crashlog_complete(crashlog))
> return -EEXIST;
>
> - pmt_crashlog_set_execute(crashlog);
> + ret = pmt_crashlog_set_execute(crashlog);
>
> - return count;
> + return ret ? ret : count;
> }
> static DEVICE_ATTR_RW(trigger);
>
>
next prev parent reply other threads:[~2026-09-02 9:05 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-01 19:27 [PATCH v4 00/18] Crescent Island PMT support Michael J. Ruhl
2026-09-01 19:27 ` [PATCH v4 01/18] platform/x86/intel/pmt: complete pcidev to device update Michael J. Ruhl
2026-09-01 19:27 ` [PATCH v4 02/18] platform/x86/intel/pmt: refactor rmw with a return value Michael J. Ruhl
2026-09-02 9:05 ` Ilpo Järvinen [this message]
2026-09-02 9:07 ` Ilpo Järvinen
2026-09-01 19:27 ` [PATCH v4 03/18] platform/x86/intel/pmt: refactor rc " Michael J. Ruhl
2026-09-02 9:08 ` Ilpo Järvinen
2026-09-01 19:27 ` [PATCH v4 04/18] platform/x86/intel/pmt: Add register access callbacks Michael J. Ruhl
2026-09-01 19:27 ` [PATCH v4 05/18] platform/x86/intel/pmt: Add helpers for callback info Michael J. Ruhl
2026-09-02 9:10 ` Ilpo Järvinen
2026-09-01 19:27 ` [PATCH v4 06/18] platform/x86/intel/pmt: Do not remap when using callbacks Michael J. Ruhl
2026-09-02 9:18 ` Ilpo Järvinen
2026-09-01 19:27 ` [PATCH v4 07/18] drm/xe/vsec: Do not register BMG PMT for VF Michael J. Ruhl
2026-09-01 19:27 ` [PATCH v4 08/18] drm/xe/vsec: Correct locking order Michael J. Ruhl
2026-09-01 19:27 ` [PATCH v4 09/18] drm/xe/vsec: Use correct pm state get Michael J. Ruhl
2026-09-01 19:27 ` [PATCH v4 10/18] drm/xe/vsec: Support possible hotplug exit Michael J. Ruhl
2026-09-01 19:27 ` [PATCH v4 11/18] drm/xe/vsec: Support Crescent Island PMT Michael J. Ruhl
2026-09-01 19:27 ` [PATCH v4 12/18] drm/xe/vsec: Refactor BattleMage PMT defines Michael J. Ruhl
2026-09-01 19:27 ` [PATCH v4 13/18] drm/xe/vsec: Crescent Island PMT decode Michael J. Ruhl
2026-09-01 19:27 ` [PATCH v4 14/18] drm/xe/vsec: Crescent Island PMT callbacks Michael J. Ruhl
2026-09-01 19:27 ` [PATCH v4 15/18] drm/xe/vsec: Support late bind fw information Michael J. Ruhl
2026-09-01 19:27 ` [PATCH v4 16/18] drm/xe/vsec: Add PMT GUID internal access Michael J. Ruhl
2026-09-01 19:27 ` [PATCH v4 17/18] drm/xe/vsec: Update PMT " Michael J. Ruhl
2026-09-01 19:27 ` [PATCH v4 18/18] drm/xe/vsec: Refactor platform check Michael J. Ruhl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=82808204-b8a3-9c94-779d-1b1851146d4d@linux.intel.com \
--to=ilpo.jarvinen@linux.intel.com \
--cc=airlied@gmail.com \
--cc=anoop.c.vijay@intel.com \
--cc=badal.nilawar@intel.com \
--cc=david.e.box@linux.intel.com \
--cc=hansg@kernel.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=james.ausmus@intel.com \
--cc=karthik.poosa@intel.com \
--cc=matthew.brost@intel.com \
--cc=matthew.d.roper@intel.com \
--cc=michael.j.ruhl@intel.com \
--cc=platform-driver-x86@vger.kernel.org \
--cc=rodrigo.vivi@intel.com \
--cc=simona@ffwll.ch \
--cc=thomas.hellstrom@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox