Received-SPF: pass (google.com: domain of linux-kernel+bounces-45997-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <676cf135-ab3e-48d8-9fdf-83276502b58a@samsung.com>
Date: Wed, 31 Jan 2024 09:27:47 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] mmc: pwrseq: Use proper reboot notifier path
Content-Language: en-US
To: Andrew Davis <afd@ti.com>, Ulf Hansson <ulf.hansson@linaro.org>, Yangtao
	Li <frank.li@vivo.com>
Cc: linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org
From: Marek Szyprowski <m.szyprowski@samsung.com>
In-Reply-To: <20240126190110.148599-1-afd@ti.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <CGME20240126190119eucas1p28714f09cd2afe41087dcaceba9862a64@eucas1p2.samsung.com>
	<20240126190110.148599-1-afd@ti.com>

On 26.01.2024 20:01, Andrew Davis wrote:
> This driver registers itself as a reboot handler, which means it claims
> it can reboot the system. It does this so it is called during the system
> reboot sequence. The correct way to be notified during the reboot
> sequence is to register a notifier with register_reboot_notifier().
> Do this here.
>
> Note this will be called during normal reboots but not emergency reboots.
> This is the expected behavior, emergency reboot means emergency, not go
> do some cleanup with emmc pins.. The reboot notifiers are intentionally
> not called in the emergency path for a reason and working around that by
> pretending to be a reboot handler is a hack.


Well, I'm the author of this 'hack' and unfortunately there was no other 
way to make emergency reboot working on boards requiring the eMMC 
pwrseq. IIRC this has been already discussed and the conclusion was to 
accept the hack with the comments explaining the problem.


> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>   drivers/mmc/core/pwrseq_emmc.c | 8 +-------
>   1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/core/pwrseq_emmc.c b/drivers/mmc/core/pwrseq_emmc.c
> index 3b6d69cefb4eb..d5045fd1a02c1 100644
> --- a/drivers/mmc/core/pwrseq_emmc.c
> +++ b/drivers/mmc/core/pwrseq_emmc.c
> @@ -70,14 +70,8 @@ static int mmc_pwrseq_emmc_probe(struct platform_device *pdev)
>   		return PTR_ERR(pwrseq->reset_gpio);
>   
>   	if (!gpiod_cansleep(pwrseq->reset_gpio)) {
> -		/*
> -		 * register reset handler to ensure emmc reset also from
> -		 * emergency_reboot(), priority 255 is the highest priority
> -		 * so it will be executed before any system reboot handler.
> -		 */
>   		pwrseq->reset_nb.notifier_call = mmc_pwrseq_emmc_reset_nb;
> -		pwrseq->reset_nb.priority = 255;
> -		register_restart_handler(&pwrseq->reset_nb);
> +		register_reboot_notifier(&pwrseq->reset_nb);
>   	} else {
>   		dev_notice(dev, "EMMC reset pin tied to a sleepy GPIO driver; reset on emergency-reboot disabled\n");
>   	}

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland