linux-kernel - Re: [PATCH 2/2] mfd: madera: Wait for BOOT

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

Message-ID: <20181002085252.GQ1653@imbe.wolfsonmicro.main>
Date:   Tue, 2 Oct 2018 09:52:52 +0100
From:   Charles Keepax <ckeepax@...nsource.cirrus.com>
To:     Richard Fitzgerald <rf@...nsource.cirrus.com>
CC:     <lee.jones@...aro.org>, <patches@...nsource.cirrus.com>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] mfd: madera: Wait for BOOT_DONE before reading
 device ID

On Mon, Oct 01, 2018 at 03:33:57PM +0100, Richard Fitzgerald wrote:
> It isn't safe to read the device ID in SOFTWARE_RESET register
> until the silicon boot sequence has completed. This patch
> rearranges the code to wait for BOOT_DONE first.
> 
> If we don't have a GPIO to control hard reset we must issue
> a soft reset to get the silicon into a known-good booted state
> before reading the device ID.
> 
> Signed-off-by: Stuart Henderson <stuarth@...nsource.cirrus.com>
> Signed-off-by: Richard Fitzgerald <rf@...nsource.cirrus.com>
> ---
>  drivers/mfd/madera-core.c | 33 +++++++++++++++------------------
>  1 file changed, 15 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mfd/madera-core.c b/drivers/mfd/madera-core.c
> index 3ff9a0615658..b9d42edf96c9 100644
> --- a/drivers/mfd/madera-core.c
> +++ b/drivers/mfd/madera-core.c
> @@ -454,10 +454,23 @@ int madera_dev_init(struct madera *madera)
>  	regcache_cache_only(madera->regmap, false);
>  	regcache_cache_only(madera->regmap_32bit, false);
>  
> +	/* If we don't have a reset GPIO use a soft reset */
> +	if (!madera->pdata.reset) {
> +		ret = madera_soft_reset(madera);
> +		if (ret)
> +			goto err_reset;
> +	}
> +

I think we should leave this soft reset lower down its better
to only write to the chip once we have identified it. It will
mean we need to wait for boot done twice (in the soft reset
case) but its only on probe and that is probably better than
doing some writes to an unknown chip.

>  	/*
> -	 * Now we can power up and verify that this is a chip we know about
> -	 * before we start doing any writes to its registers.
> +	 * Must wait for internal boot sequence to complete before
> +	 * reading the device ID
>  	 */
> +	ret = madera_wait_for_boot(madera);
> +	if (ret) {
> +		dev_err(madera->dev, "Device failed initial boot: %d\n", ret);
> +		goto err_reset;
> +	}
> +
>  	ret = regmap_read(madera->regmap, MADERA_SOFTWARE_RESET, &hwid);
>  	if (ret) {
>  		dev_err(dev, "Failed to read ID register: %d\n", ret);
> @@ -519,22 +532,6 @@ int madera_dev_init(struct madera *madera)
>  		goto err_reset;
>  	}
>  
> -	/*
> -	 * It looks like a device we support. If we don't have a hard reset
> -	 * we can now attempt a soft reset.
> -	 */
> -	if (!madera->pdata.reset) {
> -		ret = madera_soft_reset(madera);
> -		if (ret)
> -			goto err_reset;
> -	}
> -
> -	ret = madera_wait_for_boot(madera);
> -	if (ret) {
> -		dev_err(madera->dev, "Device failed initial boot: %d\n", ret);
> -		goto err_reset;
> -	}

And this guy would then only need done in the case of a soft
reset.

Thanks,
Charles

> -
>  	ret = regmap_read(madera->regmap, MADERA_HARDWARE_REVISION,
>  			  &madera->rev);
>  	if (ret) {
> -- 
> 2.11.0