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: 43+ 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-01 19:38 ` sashiko-bot
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:45 ` sashiko-bot
2026-09-01 19:27 ` [PATCH v4 05/18] platform/x86/intel/pmt: Add helpers for callback info Michael J. Ruhl
2026-09-01 19:40 ` sashiko-bot
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-01 19:51 ` sashiko-bot
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-02 19:06 ` Rodrigo Vivi
2026-09-02 20:47 ` Ruhl, Michael J
2026-09-01 19:27 ` [PATCH v4 08/18] drm/xe/vsec: Correct locking order Michael J. Ruhl
2026-09-02 19:08 ` Rodrigo Vivi
2026-09-02 19:11 ` Matthew Brost
2026-09-01 19:27 ` [PATCH v4 09/18] drm/xe/vsec: Use correct pm state get Michael J. Ruhl
2026-09-02 19:10 ` Rodrigo Vivi
2026-09-01 19:27 ` [PATCH v4 10/18] drm/xe/vsec: Support possible hotplug exit Michael J. Ruhl
2026-09-02 19:15 ` Rodrigo Vivi
2026-09-01 19:27 ` [PATCH v4 11/18] drm/xe/vsec: Support Crescent Island PMT Michael J. Ruhl
2026-09-02 19:16 ` Rodrigo Vivi
2026-09-01 19:27 ` [PATCH v4 12/18] drm/xe/vsec: Refactor BattleMage PMT defines Michael J. Ruhl
2026-09-02 19:18 ` Rodrigo Vivi
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 20:14 ` sashiko-bot
2026-09-01 19:27 ` [PATCH v4 16/18] drm/xe/vsec: Add PMT GUID internal access Michael J. Ruhl
2026-09-01 20:20 ` sashiko-bot
2026-09-01 19:27 ` [PATCH v4 17/18] drm/xe/vsec: Update PMT " Michael J. Ruhl
2026-09-01 20:21 ` sashiko-bot
2026-09-03 5:50 ` Poosa, Karthik
2026-09-01 19:27 ` [PATCH v4 18/18] drm/xe/vsec: Refactor platform check Michael J. Ruhl
2026-09-03 5:57 ` Poosa, Karthik
2026-09-01 19:34 ` ✗ CI.checkpatch: warning for Crescent Island PMT support (rev6) Patchwork
2026-09-01 19:35 ` ✗ CI.KUnit: failure " Patchwork
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.