[PATCH] hw/ssi: aspeed: Deselect chip to reset state in command mode

[Cédric Le Goater]

On 12/20/19 4:34 PM, Andrew Jeffery wrote:
> This is probably a bug elsewhere, maybe in the kernel driver. However,
> this workaround/fix appears to resolve the squashfs/UBI corruption we've
> been seeing for quite some time in QEMU with e.g. witherspoon-bmc and
> associated images.

Nice one ! This has been bugging us for years. 
 
> The problem is the chip is left in e.g. read mode 

Which is the default mode we usually want for reads. Bizarre.

> and the deselect is
> necessary to reset it back to idle for the current command to issue
> correctly. 

The driver should have done it. Bizarre. Bizarre. 

> Failure to deselect leads to command-mode access of data at
> unexpected addresses (the state mismatch means the command/address are
> potentially ignored) and the result presents as data corruption.

How are you tracking this ? It is really really complex to corner
such cases. 

We could test the CE_STOP_ACTIVE bit when doing the load in read 
mode and put an error.  With a busy loop, we could stop execution 
to try to catch which instruction is doing the read.


> Something I don't yet understand is the symptom (squashfs corruption)
> wasn't overly deterministic. I would have expected more widespread
> symptoms given the change. Having said that, UBI hammers the flash with
> its wear-leveling (somewhat ironically). Its approach appears to be
> somewhat complex, so maybe it's not surprising that the results are so
> variable.

It feels like a race between a read, using the default read mode on the 
AHB flash window, and an update, using the slow path in the driver : 
select, write command, write data, unselect.

> Cc: Andrew Geissler <geissonator at gmail.com>
> Signed-off-by: Andrew Jeffery <andrew at aj.id.au>

I have pushed the patch on my tree as it seems to give better results
than before and it's not breaking the other platforms. 

But I am still seeing ubi corruption after a reboot with the latest 
witherspoon image (20191214002407) :/



Thanks, 

C.

> 
> ---
> 
> An early Christmas present for Andrew G.
> ---
>  hw/ssi/aspeed_smc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 7755eca34976..c8713f3e3347 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -739,6 +739,7 @@ static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size)
>          break;
>      case CTRL_READMODE:
>      case CTRL_FREADMODE:
> +        aspeed_smc_flash_unselect(fl);
>          aspeed_smc_flash_select(fl);
>          aspeed_smc_flash_setup(fl, addr);
>  
>