Re: [RFC PATCH 1/4] power: reset: reboot-mode: Allow magic to be 0

[Pavan Kondeti <quic_pkondeti@quicinc.com>]

On Mon, Jul 24, 2023 at 03:30:51PM -0700, Elliot Berman wrote:
> Allow magic from the reboot-mode driver to be defined, but 0. This is
> useful when the register/command to trigger reboot needs 0 to be
> written. This is the case when the "default" reboot mode is to enter a
> crash dump collection mode (e.g. when triggered by a watchdog) and Linux
> doing a normal reboot requires the setting to be explicitly reset to
> zero.

Sorry, it is not clear to me. The current code does not treat 0 as a
valid value for magic. Since our platform has a meaning for 0 as a magic
value, we are doing this correct?

Basically we are allowing

reboot-mode {
	mode-normal = <0x0>;
	...
}

What is "default" reboot mode you are referring here? are you referring
to

enum reboot_mode reboot_mode DEFAULT_REBOOT_MODE

defined in kernel/reboot.c?

> 
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
>  drivers/power/reset/reboot-mode.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> index b4076b10b893..a646aefa041b 100644
> --- a/drivers/power/reset/reboot-mode.c
> +++ b/drivers/power/reset/reboot-mode.c
> @@ -19,11 +19,10 @@ struct mode_info {
>  	struct list_head list;
>  };
>  
> -static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> -					  const char *cmd)
> +static bool get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> +					  const char *cmd, unsigned int *magic)
>  {
>  	const char *normal = "normal";
> -	int magic = 0;
>  	struct mode_info *info;
>  
>  	if (!cmd)
> @@ -31,12 +30,12 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
>  
>  	list_for_each_entry(info, &reboot->head, list) {
>  		if (!strcmp(info->mode, cmd)) {
> -			magic = info->magic;
> -			break;
> +			*magic = info->magic;
> +			return true;
>  		}
>  	}
>  
> -	return magic;
> +	return false;
>  }
>  
>  static int reboot_mode_notify(struct notifier_block *this,
> @@ -46,8 +45,7 @@ static int reboot_mode_notify(struct notifier_block *this,
>  	unsigned int magic;
>  
>  	reboot = container_of(this, struct reboot_mode_driver, reboot_notifier);
> -	magic = get_reboot_mode_magic(reboot, cmd);
> -	if (magic)
> +	if (get_reboot_mode_magic(reboot, cmd, &magic))
>  		reboot->write(reboot, magic);
>  
>  	return NOTIFY_DONE;
> -- 
> 2.41.0
> 

Thanks,
Pavan