linux-kernel - Re: [PATCH] mmc: pwrseq: Use highest priority for eMMC restart handler

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] [thread-next>] [day] [month] [year] [list]

Message-id: <56282F71.2070408@samsung.com>
Date:	Thu, 22 Oct 2015 09:36:01 +0900
From:	Krzysztof Kozlowski <k.kozlowski@...sung.com>
To:	Javier Martinez Canillas <javier@....samsung.com>,
	linux-kernel@...r.kernel.org
Cc:	Markus Reichl <m.reichl@...etechno.de>,
	Anand Moon <linux.amoon@...il.com>,
	linux-samsung-soc@...r.kernel.org,
	Alim Akhtar <alim.akhtar@...sung.com>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	linux-mmc@...r.kernel.org, Alexandre Courbot <acourbot@...dia.com>,
	Ulf Hansson <ulf.hansson@...aro.org>
Subject: Re: [PATCH] mmc: pwrseq: Use highest priority for eMMC restart handler

On 22.10.2015 00:15, Javier Martinez Canillas wrote:
> The pwrseq_emmc driver does a eMMC card reset before a system reboot to
> allow broken or limited ROM boot-loaders (that don't have an eMMC reset
> logic) to be able to read the second stage from the eMMC.
> 
> But this has to be called before a system reboot handler and while most
> of them use the priority 128, there are other restart handlers (such as
> the syscon-reboot one) that use a higher priority. So, use the highest
> priority to make sure that the eMMC hw is reset before a system reboot.
> 
> Signed-off-by: Javier Martinez Canillas <javier@....samsung.com>
> Tested-by: Markus Reichl <m.reichl@...etechno.de>
> Tested-by: Anand Moon <linux.amoon@...il.com>
> Reviewed-by: Alim Akhtar <alim.akhtar@...sung.com>
> 
> ---
> Hello,
> 
> This patch was needed since a recent series from Alim [0] added
> syscon reboot and poweroff support to Exynos SoCs and removed
> the reset handler in the Exynos Power Management Unit (PMU) code.
> 
> But the PMU and syscon-reboot restart handler have a different
> priority so [0] breaks restart when eMMC is used on these boards.
> 
> [0]: http://www.spinics.net/lists/arm-kernel/msg454396.html
> 
> So this patch must be merged before [0] to avoid regressions.
> 
> Best regards,
> Javier
> 
>  drivers/mmc/core/pwrseq_emmc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/core/pwrseq_emmc.c b/drivers/mmc/core/pwrseq_emmc.c
> index 137c97fb7aa8..ad4f94ec7e8d 100644
> --- a/drivers/mmc/core/pwrseq_emmc.c
> +++ b/drivers/mmc/core/pwrseq_emmc.c
> @@ -84,11 +84,11 @@ struct mmc_pwrseq *mmc_pwrseq_emmc_alloc(struct mmc_host *host,
>  
>  	/*
>  	 * register reset handler to ensure emmc reset also from
> -	 * emergency_reboot(), priority 129 schedules it just before
> -	 * system reboot
> +	 * 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 = 129;
> +	pwrseq->reset_nb.priority = 255;

I see the problem which you are trying to solve but this may be tricker
then just kicking the number. Some of restart handlers are registered
with priority 192. I found few of such, like: at91_restart_nb,
zynq_slcr_restart_nb, rmobile_reset_nb (maybe more, I did not grep too
much).

I guess they chose the "192" priority on purpose.

Effectively, now the emmc handler will be executed before their
handlers... is it an issue? Maybe some testing on these platforms is
necessary?

Best regards,
Krzysztof

>  	register_restart_handler(&pwrseq->reset_nb);
>  
>  	pwrseq->pwrseq.ops = &mmc_pwrseq_emmc_ops;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/