linux-kernel - Re: [PATCH v2 2/2] power: reset: syscon-reboot: support different reset modes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <135bdfc38dad8994b94318174a970fd22dc87f45.camel@linaro.org>
Date: Wed, 26 Feb 2025 19:10:02 +0000
From: André Draszik <andre.draszik@...aro.org>
To: Sebastian Reichel <sre@...nel.org>, Rob Herring <robh@...nel.org>, 
 Krzysztof Kozlowski
	 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>
Cc: Peter Griffin <peter.griffin@...aro.org>, Tudor Ambarus	
 <tudor.ambarus@...aro.org>, Will McVicker <willmcvicker@...gle.com>, 
	kernel-team@...roid.com, linux-pm@...r.kernel.org,
 devicetree@...r.kernel.org, 	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] power: reset: syscon-reboot: support different
 reset modes

On Wed, 2025-02-26 at 16:44 +0000, André Draszik wrote:
> Linux supports a couple different reset modes, but this driver here
> doesn't distinguish between them and issues the same syscon register
> write irrespective of the reset mode requested by the kernel.
> 
> Update this driver to support most of Linux' reset modes: cold, hard,
> warm, and soft.
> 
> The actions to take for these are taken from DT, and are all new
> optional properties. The property names match the existing properties
> supported but should be prefixed with the reset mode.
> 
> This change is meant to be backwards compatible with existing DTs, and
> if Linux requests a reset mode that this driver doesn't support, or
> that the DT doesn't specify, the reset is triggered using the fallback
> / default entry.
> 
> As an example why this is useful, other than properly supporting the
> Linux reboot= kernel command line option or sysfs entry, this change
> allows platforms to e.g. default to a more secure cold-reset, but
> also to do a warm-reset in case RAM contents needs to be retained
> across the reset.
> 
> Signed-off-by: André Draszik <andre.draszik@...aro.org>
> ---
>  drivers/power/reset/syscon-reboot.c | 88 ++++++++++++++++++++++++++++++++-----
>  1 file changed, 77 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/power/reset/syscon-reboot.c b/drivers/power/reset/syscon-reboot.c
> index d623d77e657e4c233d8ae88bb099bee13c48a9ef..1d3d8a3265ae8005c685b42d3e554bd8bb0047ea 100644
> --- a/drivers/power/reset/syscon-reboot.c
> +++ b/drivers/power/reset/syscon-reboot.c
> @@ -14,11 +14,29 @@
>  #include <linux/reboot.h>
>  #include <linux/regmap.h>
>  
> -struct syscon_reboot_context {
> -	struct regmap *map;
> +/* REBOOT_GPIO doesn't make sense for syscon-reboot */
> +static const struct {
> +	enum reboot_mode mode;
> +	const char *prefix;
> +} prefix_map[] = {
> +	{ .mode = REBOOT_COLD, .prefix = "cold"  },
> +	{ .mode = REBOOT_WARM, .prefix = "warm"  },
> +	{ .mode = REBOOT_HARD, .prefix = "hard"  },
> +	{ .mode = REBOOT_SOFT, .prefix = "soft"  },
> +};
> +
> +struct reboot_mode_bits {
>  	u32 offset;
>  	u32 value;
>  	u32 mask;
> +	bool valid;
> +};
> +
> +struct syscon_reboot_context {
> +	struct regmap *map;
> +
> +	struct reboot_mode_bits mode_bits[REBOOT_SOFT + 1];
> +	struct reboot_mode_bits catchall;
>  	struct notifier_block restart_handler;
>  };
>  
> @@ -28,9 +46,16 @@ static int syscon_restart_handle(struct notifier_block *this,
>  	struct syscon_reboot_context *ctx =
>  			container_of(this, struct syscon_reboot_context,
>  					restart_handler);
> +	const struct reboot_mode_bits *mode_bits;
> +
> +	if (mode < ARRAY_SIZE(ctx->mode_bits) && ctx->mode_bits[mode].valid)
> +		mode_bits = &ctx->mode_bits[mode];
> +	else
> +		mode_bits = &ctx->catchall;
>  
>  	/* Issue the reboot */
> -	regmap_update_bits(ctx->map, ctx->offset, ctx->mask, ctx->value);
> +	regmap_update_bits(ctx->map, mode_bits->offset, mode_bits->mask,
> +			   mode_bits->value);
>  
>  	mdelay(1000);
>  
> @@ -45,6 +70,7 @@ static int syscon_reboot_probe(struct platform_device *pdev)
>  	int mask_err, value_err;
>  	int priority;
>  	int err;
> +	char prop[32];
>  
>  	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL);
>  	if (!ctx)
> @@ -60,12 +86,52 @@ static int syscon_reboot_probe(struct platform_device *pdev)
>  	if (of_property_read_s32(pdev->dev.of_node, "priority", &priority))
>  		priority = 192;
>  
> -	if (of_property_read_u32(pdev->dev.of_node, "offset", &ctx->offset))
> -		if (of_property_read_u32(pdev->dev.of_node, "reg", &ctx->offset))
> +	BUILD_BUG_ON(ARRAY_SIZE(prefix_map) != ARRAY_SIZE(ctx->mode_bits));
> +	BUILD_BUG_ON(ARRAY_SIZE(ctx->mode_bits) <= REBOOT_COLD);
> +	BUILD_BUG_ON(ARRAY_SIZE(ctx->mode_bits) <= REBOOT_WARM);
> +	BUILD_BUG_ON(ARRAY_SIZE(ctx->mode_bits) <= REBOOT_HARD);
> +	BUILD_BUG_ON(ARRAY_SIZE(ctx->mode_bits) <= REBOOT_SOFT);
> +
> +	for (int i = 0; i < ARRAY_SIZE(prefix_map); ++i) {
> +		const char * const prefix = prefix_map[i].prefix;
> +		struct reboot_mode_bits * const mode_bits =
> +			&ctx->mode_bits[prefix_map[i].mode];
> +
> +		snprintf(prop, sizeof(prop), "%s-offset", prefix);
> +		if (of_property_read_u32(pdev->dev.of_node, "offset",
> +					 &mode_bits->offset))
> +			continue;

It should also check the %-reg property, and it needs
to use prop instead of "offset" in of_property_read_u32().

A.