Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 26 Jul 2023 10:36:28 +0530
From:   Pavan Kondeti <quic_pkondeti@quicinc.com>
To:     Elliot Berman <quic_eberman@quicinc.com>
CC:     Mark Rutland <mark.rutland@arm.com>,
        Lorenzo Pieralisi <lpieralisi@kernel.org>,
        Sebastian Reichel <sre@kernel.org>,
        "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        <linux-arm-kernel@lists.infradead.org>, <linux-pm@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-arm-msm@vger.kernel.org>, <kernel@quicinc.com>,
        "Satya Durga Srinivasu Prabhala" <quic_satyap@quicinc.com>,
        Melody Olvera <quic_molvera@quicinc.com>,
        Prasad Sodagudi <quic_psodagud@quicinc.com>
Subject: Re: [RFC PATCH 1/4] power: reset: reboot-mode: Allow magic to be 0
Message-ID: <1241d1bd-aa0f-4e27-9e40-1624bbc4c2e6@quicinc.com>
References: <20230724223057.1208122-1-quic_eberman@quicinc.com>
 <20230724223057.1208122-2-quic_eberman@quicinc.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230724223057.1208122-2-quic_eberman@quicinc.com>
Precedence: bulk

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