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 03/18] platform/x86/intel/pmt: refactor rc with a return value
Date: Wed, 2 Sep 2026 12:08:38 +0300 (EEST) [thread overview]
Message-ID: <4251f034-6955-8bc8-6408-269d8bbd238b@linux.intel.com> (raw)
In-Reply-To: <20260901192736.626777-23-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_rw 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 | 109 +++++++++++++++-------
> 1 file changed, 77 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
> index ebc7880d95bd..83323b1ce4a9 100644
> --- a/drivers/platform/x86/intel/pmt/crashlog.c
> +++ b/drivers/platform/x86/intel/pmt/crashlog.c
> @@ -144,27 +144,29 @@ static int pmt_crashlog_rmw(struct crashlog_entry *crashlog, u32 bit, bool set)
> }
>
> /* Read the status register and see if the specified @bit is set */
> -static bool pmt_crashlog_rc(struct crashlog_entry *crashlog, u32 bit)
> +static int pmt_crashlog_rc(struct crashlog_entry *crashlog, u32 bit, bool *state)
> {
> const struct crashlog_status *status = &crashlog->info->status;
> u32 reg = readl(crashlog->entry.disc_table + status->offset);
>
> - return !!(reg & bit);
> + *state = !!(reg & bit);
> +
> + return 0;
> }
>
> -static bool pmt_crashlog_complete(struct crashlog_entry *crashlog)
> +static int pmt_crashlog_complete(struct crashlog_entry *crashlog, bool *state)
> {
> /* return current value of the crashlog complete flag */
> - return pmt_crashlog_rc(crashlog, crashlog->info->status.complete);
> + return pmt_crashlog_rc(crashlog, crashlog->info->status.complete, state);
> }
>
> -static bool pmt_crashlog_disabled(struct crashlog_entry *crashlog)
> +static int pmt_crashlog_disabled(struct crashlog_entry *crashlog, bool *state)
> {
> /* return current value of the crashlog disabled flag */
> - return pmt_crashlog_rc(crashlog, crashlog->info->status.disabled);
> + return pmt_crashlog_rc(crashlog, crashlog->info->status.disabled, state);
> }
>
> -static bool pmt_crashlog_supported(struct intel_pmt_entry *entry, u32 *crash_type, u32 *version)
> +static int pmt_crashlog_supported(struct intel_pmt_entry *entry, u32 *crash_type, u32 *version)
> {
> u32 discovery_header = readl(entry->disc_table + CONTROL_OFFSET);
>
> @@ -184,7 +186,7 @@ static bool pmt_crashlog_supported(struct intel_pmt_entry *entry, u32 *crash_typ
> }
>
> static int pmt_crashlog_set_disable(struct crashlog_entry *crashlog,
> - bool disable)
> + bool disable)
> {
> return pmt_crashlog_rmw(crashlog, crashlog->info->control.disable, disable);
> }
> @@ -199,14 +201,14 @@ static int pmt_crashlog_set_execute(struct crashlog_entry *crashlog)
> return pmt_crashlog_rmw(crashlog, crashlog->info->control.manual, true);
> }
>
> -static bool pmt_crashlog_cleared(struct crashlog_entry *crashlog)
> +static int pmt_crashlog_cleared(struct crashlog_entry *crashlog, bool *state)
> {
> - return pmt_crashlog_rc(crashlog, crashlog->info->status.cleared);
> + return pmt_crashlog_rc(crashlog, crashlog->info->status.cleared, state);
> }
>
> -static bool pmt_crashlog_consumed(struct crashlog_entry *crashlog)
> +static int pmt_crashlog_consumed(struct crashlog_entry *crashlog, bool *state)
> {
> - return pmt_crashlog_rc(crashlog, crashlog->info->status.consumed);
> + return pmt_crashlog_rc(crashlog, crashlog->info->status.consumed, state);
> }
>
> static int pmt_crashlog_set_consumed(struct crashlog_entry *crashlog)
> @@ -214,14 +216,14 @@ static int pmt_crashlog_set_consumed(struct crashlog_entry *crashlog)
> return pmt_crashlog_rmw(crashlog, crashlog->info->control.consume, true);
> }
>
> -static bool pmt_crashlog_error(struct crashlog_entry *crashlog)
> +static int pmt_crashlog_error(struct crashlog_entry *crashlog, bool *state)
> {
> - return pmt_crashlog_rc(crashlog, crashlog->info->status.error);
> + return pmt_crashlog_rc(crashlog, crashlog->info->status.error, state);
> }
>
> -static bool pmt_crashlog_rearm(struct crashlog_entry *crashlog)
> +static int pmt_crashlog_rearm(struct crashlog_entry *crashlog, bool *state)
> {
> - return pmt_crashlog_rc(crashlog, crashlog->info->status.rearmed);
> + return pmt_crashlog_rc(crashlog, crashlog->info->status.rearmed, state);
> }
>
> static int pmt_crashlog_set_rearm(struct crashlog_entry *crashlog)
> @@ -236,9 +238,14 @@ static ssize_t
> clear_show(struct device *dev, struct device_attribute *attr, char *buf)
> {
> struct crashlog_entry *crashlog = dev_get_drvdata(dev);
> - bool cleared = pmt_crashlog_cleared(crashlog);
> + bool cleared;
> + int ret;
>
> - return sysfs_emit(buf, "%d\n", cleared);
> + ret = pmt_crashlog_cleared(crashlog, &cleared);
> + if (ret)
> + return sysfs_emit(buf, "read error: %d\n", ret);
Never emit errors like this but just return the error code from a _show()
function.
--
i.
> + else
> + return sysfs_emit(buf, "%d\n", cleared);
> }
>
> static ssize_t
> @@ -271,9 +278,14 @@ static ssize_t
> consumed_show(struct device *dev, struct device_attribute *attr, char *buf)
> {
> struct crashlog_entry *crashlog = dev_get_drvdata(dev);
> - bool consumed = pmt_crashlog_consumed(crashlog);
> + bool consumed;
> + int ret;
>
> - return sysfs_emit(buf, "%d\n", consumed);
> + ret = pmt_crashlog_consumed(crashlog, &consumed);
> + if (ret)
> + return sysfs_emit(buf, "read error: %d\n", ret);
> + else
> + return sysfs_emit(buf, "%d\n", consumed);
> }
>
> static ssize_t
> @@ -281,7 +293,9 @@ consumed_store(struct device *dev, struct device_attribute *attr, const char *bu
> size_t count)
> {
> struct crashlog_entry *crashlog;
> + bool complete;
> bool consumed;
> + bool disabled;
> int ret;
>
> crashlog = dev_get_drvdata(dev);
> @@ -296,10 +310,16 @@ consumed_store(struct device *dev, struct device_attribute *attr, const char *bu
>
> guard(mutex)(&crashlog->control_mutex);
>
> - if (pmt_crashlog_disabled(crashlog))
> + ret = pmt_crashlog_disabled(crashlog, &disabled);
> + if (ret)
> + return ret;
> + if (disabled)
> return -EBUSY;
>
> - if (!pmt_crashlog_complete(crashlog))
> + ret = pmt_crashlog_complete(crashlog, &complete);
> + if (ret)
> + return ret;
> + if (!complete)
> return -EEXIST;
>
> ret = pmt_crashlog_set_consumed(crashlog);
> @@ -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;
>
> - return sprintf(buf, "%d\n", enabled);
> + ret = pmt_crashlog_disabled(crashlog, &enabled);
> + if (ret)
> + return sysfs_emit(buf, "error: %d\n", ret);
> + else
> + return sysfs_emit(buf, "%d\n", enabled);
> }
>
> static ssize_t
> @@ -343,9 +368,14 @@ static ssize_t
> error_show(struct device *dev, struct device_attribute *attr, char *buf)
> {
> struct crashlog_entry *crashlog = dev_get_drvdata(dev);
> - bool error = pmt_crashlog_error(crashlog);
> + bool error;
> + int ret;
>
> - return sysfs_emit(buf, "%d\n", error);
> + ret = pmt_crashlog_error(crashlog, &error);
> + if (ret)
> + return sysfs_emit(buf, "read error: %d\n", ret);
> + else
> + return sysfs_emit(buf, "%d\n", error);
> }
> static DEVICE_ATTR_RO(error);
>
> @@ -353,9 +383,14 @@ static ssize_t
> rearm_show(struct device *dev, struct device_attribute *attr, char *buf)
> {
> struct crashlog_entry *crashlog = dev_get_drvdata(dev);
> - int rearmed = pmt_crashlog_rearm(crashlog);
> + bool rearmed;
> + int ret;
>
> - return sysfs_emit(buf, "%d\n", rearmed);
> + ret = pmt_crashlog_rearm(crashlog, &rearmed);
> + if (ret)
> + return sysfs_emit(buf, "read error: %d\n", ret);
> + else
> + return sysfs_emit(buf, "%d\n", rearmed);
> }
>
> static ssize_t
> @@ -389,11 +424,15 @@ trigger_show(struct device *dev, struct device_attribute *attr, char *buf)
> {
> struct crashlog_entry *crashlog;
> bool trigger;
> + int ret;
>
> crashlog = dev_get_drvdata(dev);
> - trigger = pmt_crashlog_complete(crashlog);
>
> - return sprintf(buf, "%d\n", trigger);
> + ret = pmt_crashlog_complete(crashlog, &trigger);
> + if (ret)
> + return sysfs_emit(buf, "read error: %d\n", ret);
> + else
> + return sysfs_emit(buf, "%d\n", trigger);
> }
>
> static ssize_t
> @@ -401,6 +440,8 @@ trigger_store(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> {
> struct crashlog_entry *crashlog;
> + bool complete;
> + bool disabled;
> bool trigger;
> int ret;
>
> @@ -413,7 +454,10 @@ trigger_store(struct device *dev, struct device_attribute *attr,
> guard(mutex)(&crashlog->control_mutex);
>
> /* if device is currently disabled, return busy */
> - if (pmt_crashlog_disabled(crashlog))
> + ret = pmt_crashlog_disabled(crashlog, &disabled);
> + if (ret)
> + return ret;
> + if (disabled)
> return -EBUSY;
>
> if (!trigger) {
> @@ -422,7 +466,8 @@ trigger_store(struct device *dev, struct device_attribute *attr,
> }
>
> /* we cannot trigger a new crash if one is still pending */
> - if (pmt_crashlog_complete(crashlog))
> + ret = pmt_crashlog_complete(crashlog, &complete);
> + if (complete)
> return -EEXIST;
>
> ret = pmt_crashlog_set_execute(crashlog);
>
next prev parent reply other threads:[~2026-09-02 9:08 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
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 [this message]
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=4251f034-6955-8bc8-6408-269d8bbd238b@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.