linux-kernel - Re: [PATCH] clk: at91: avoid sleeping early

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: <20190924122147.fojcu5u44letrele@pengutronix.de>
Date:   Tue, 24 Sep 2019 14:21:47 +0200
From:   Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
To:     Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc:     Stephen Boyd <sboyd@...nel.org>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Claudiu Beznea <claudiu.beznea@...rochip.com>,
        Michael Turquette <mturquette@...libre.com>,
        linux-clk@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] clk: at91: avoid sleeping early

On Fri, Sep 20, 2019 at 05:39:06PM +0200, Alexandre Belloni wrote:
> It is not allowed to sleep to early in the boot process and this may lead
> to kernel issues if the bootloader didn't prepare the slow clock and main
> clock.
> 
> This results in the following error and dump stack on the AriettaG25:
>    bad: scheduling from the idle thread!
> 
> Ensure it is possible to sleep, else simply have a delay.
> 
> Reported-by: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@...tlin.com>

Tested-by: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>

See below for a comment.

> Note that this was already discussed a while ago and Arnd said this approach was
> reasonable:
>   https://lore.kernel.org/lkml/6120818.MyeJZ74hYa@wuerfel/
> 
>  drivers/clk/at91/clk-main.c |  5 ++++-
>  drivers/clk/at91/sckc.c     | 20 ++++++++++++++++----
>  2 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/clk/at91/clk-main.c b/drivers/clk/at91/clk-main.c
> index f607ee702c83..ccd48e7a3d74 100644
> --- a/drivers/clk/at91/clk-main.c
> +++ b/drivers/clk/at91/clk-main.c
> @@ -293,7 +293,10 @@ static int clk_main_probe_frequency(struct regmap *regmap)
>  		regmap_read(regmap, AT91_CKGR_MCFR, &mcfr);
>  		if (mcfr & AT91_PMC_MAINRDY)
>  			return 0;
> -		usleep_range(MAINF_LOOP_MIN_WAIT, MAINF_LOOP_MAX_WAIT);
> +		if (system_state < SYSTEM_RUNNING)
> +			udelay(MAINF_LOOP_MIN_WAIT);
> +		else
> +			usleep_range(MAINF_LOOP_MIN_WAIT, MAINF_LOOP_MAX_WAIT);

Given that this construct is introduced several times, I wonder if we
want something like:

	static inline void early_usleep_range(unsigned long min, unsigned long max)
	{
		if (system_state < SYSTEM_RUNNING)
			udelay(min);
		else
			usleep_range(min, max);
	}

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |