Re: [PATCH v14 3/6] reboot: add parsable tokens for power state change reasons

[Matti Vaittinen <mazziesaccount@gmail.com>]

On 21/09/2026 20:44, Faruque Ansari wrote:
> psc_reason_to_str() returns human-readable labels that contain spaces
> (e.g. "over temperature"). Those are fine for logs but unusable as
> values in a space-separated sysfs list or as a write target.
> 
> Extend the single reason descriptor table with a stable, space-free
> token next to the existing label, and add psc_reason_to_token() and
> psc_reason_from_token() so consumers can emit and parse reasons without
> inventing their own string table.
> 
> Co-developed-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> Signed-off-by: Faruque Ansari <faruque.ansari@oss.qualcomm.com>
> 
> changes v12:
> - new patch
> ---
>   include/linux/reboot.h |  2 ++
>   kernel/reboot.c        | 77 +++++++++++++++++++++++++++++++++++++++-----------
>   2 files changed, 63 insertions(+), 16 deletions(-)
> 
> diff --git a/include/linux/reboot.h b/include/linux/reboot.h
> index 08a7549bbc40..4c5327dd7645 100644
> --- a/include/linux/reboot.h
> +++ b/include/linux/reboot.h
> @@ -244,6 +244,8 @@ enum psc_reason {
>   #define PSCR_MAX_REASON	(PSCR_REASON_COUNT - 1)
>   
>   const char *psc_reason_to_str(enum psc_reason reason);
> +const char *psc_reason_to_token(enum psc_reason reason);
> +int psc_reason_from_token(const char *token, enum psc_reason *reason);
>   
>   /**
>    * enum hw_protection_action - Hardware protection action
> diff --git a/kernel/reboot.c b/kernel/reboot.c
> index d5a45db82c3d..e53fb31bad06 100644
> --- a/kernel/reboot.c
> +++ b/kernel/reboot.c
> @@ -1084,34 +1084,79 @@ void set_psc_reason(enum psc_reason reason)
>   }
>   EXPORT_SYMBOL_GPL(set_psc_reason);
>   
> -static const char * const pscr_reason_strs[] = {
> -	[PSCR_UNKNOWN]            = POWER_ON_REASON_UNKNOWN,
> -	[PSCR_UNDER_VOLTAGE]      = POWER_ON_REASON_BROWN_OUT,
> -	[PSCR_OVER_CURRENT]       = POWER_ON_REASON_OVER_CURRENT,
> -	[PSCR_REGULATOR_FAILURE]  = POWER_ON_REASON_REGULATOR_FAILURE,
> -	[PSCR_OVER_TEMPERATURE]   = POWER_ON_REASON_OVER_TEMPERATURE,
> -	[PSCR_EC_PANIC]           = POWER_ON_REASON_EC_PANIC,
> +/**
> + * struct psc_reason_desc - Descriptor for a power state change reason.
> + * @token: Stable, parsable identifier without spaces (e.g. "over-temperature").
> + *	   Suitable for use in sysfs values and as a user/kernel contract.
> + * @label: Human-readable description (e.g. "over temperature"), for logs.
> + */
> +struct psc_reason_desc {
> +	const char *token;
> +	const char *label;
> +};
> +
> +static const struct psc_reason_desc psc_reason_descs[] = {
> +	[PSCR_UNKNOWN]		 = { "unknown",		  POWER_ON_REASON_UNKNOWN },
> +	[PSCR_UNDER_VOLTAGE]	 = { "under-voltage",	  POWER_ON_REASON_BROWN_OUT },
> +	[PSCR_OVER_CURRENT]	 = { "over-current",	  POWER_ON_REASON_OVER_CURRENT },
> +	[PSCR_REGULATOR_FAILURE] = { "regulator-failure", POWER_ON_REASON_REGULATOR_FAILURE },
> +	[PSCR_OVER_TEMPERATURE]	 = { "over-temperature",  POWER_ON_REASON_OVER_TEMPERATURE },
> +	[PSCR_EC_PANIC]		 = { "ec-panic",	  POWER_ON_REASON_EC_PANIC },
>   };

nit:
Is there a reason for having the label as a define while token is a 
plain string? It may be just me, but I would have expected the userland 
contract to be defined in a header. (It may be just my false expectation 
though!).

>   
>   /**
> - * psc_reason_to_str - Converts a power state change reason enum to a string.
> - * @reason: The `psc_reason` enum value to be converted.
> - *
> - * This function provides a human-readable string representation of the power
> - * state change reason, making it easier to interpret logs and debug messages.
> + * psc_reason_to_str - Human-readable label for a power state change reason.
> + * @reason: The `psc_reason` value to convert.
>    *
> - * Return:
> - * - A string corresponding to the given `psc_reason` value.
> - * - `"Invalid"` if the value is not recognized.
> + * Return: The label string, or "Invalid" if @reason is out of range. For a
> + * stable, parsable form use psc_reason_to_token() instead.
>    */
>   const char *psc_reason_to_str(enum psc_reason reason)
>   {
>   	if (reason < 0 || reason >= PSCR_REASON_COUNT)
>   		return "Invalid";
> -	return pscr_reason_strs[reason];
> +	return psc_reason_descs[reason].label;

nit:
I would feel a tad safer if the psc_reason_descs[] was always guaranteed 
to be initialized up-to PSCR_REASON_COUNT. Having the enum and 
PSCR_REASON_COUNT defined in one place, and psc_reason_descs[] 
initialized here, adds a window for: "I added new enum value, forgot to 
update the psc_reason_descs[]". Can we add a NULL check or are we on a 
very performance critical path here?

>   }
>   EXPORT_SYMBOL_GPL(psc_reason_to_str);
>   
> +/**
> + * psc_reason_to_token - Stable, parsable token for a power state change reason.
> + * @reason: The `psc_reason` value to convert.
> + *
> + * Return: The token string (no spaces), or "invalid" if @reason is out of
> + * range. Round-trips with psc_reason_from_token().
> + */
> +const char *psc_reason_to_token(enum psc_reason reason)
> +{
> +	if (reason < 0 || reason >= PSCR_REASON_COUNT)
> +		return "invalid";
> +	return psc_reason_descs[reason].token;

Please, see my comment above.

> +}
> +EXPORT_SYMBOL_GPL(psc_reason_to_token);
> +
> +/**
> + * psc_reason_from_token - Parse a reason token into a `psc_reason` value.
> + * @token: A token as returned by psc_reason_to_token(). A trailing newline is
> + *	   tolerated.
> + * @reason: Output; set on success.
> + *
> + * Return: 0 on success or -EINVAL if @token matches no known reason.
> + */
> +int psc_reason_from_token(const char *token, enum psc_reason *reason)
> +{
> +	int i;
> +
> +	for (i = 0; i < PSCR_REASON_COUNT; i++) {
> +		if (sysfs_streq(token, psc_reason_descs[i].token)) {

I suppose my comment applies here as well.

> +			*reason = i;
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(psc_reason_from_token);
> +
>   /**
>    * __hw_protection_trigger - Trigger an emergency system shutdown or reboot
>    *

After all these years in the business - it's hard to be as confident as 
I used to. ;) So, I won't say [aloud] that my preferred way is the only 
correct way. Please weigh my comments and decide as you see best, this 
is "good enough" for me if you don't agree with my suggestions.

Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>

Yours,
   -- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~