[PATCH v3] mtd: cfi_cmdset_0002: Change write buffer to check correct value

[Joakim Tjernlund]

On Fri, 2018-05-11 at 07:58 +0000, IKEGAMI Tokunori wrote:
> 
> From: Tokunori Ikegami <ikegami at allied-telesis.co.jp>
> 
> For the word write it is checked if the chip has the correct value.
> But it is not checked for the write buffer as only checked if ready.
> To make sure for the write buffer change to check the value.
> 
> It is enough as this patch is only checking the last written word.
> Since it is described by data sheets to check the operation status.
> 
> Signed-off-by: Tokunori Ikegami <ikegami at allied-telesis.co.jp>
> Cc: Chris Packham <chris.packham at alliedtelesis.co.nz>
> Cc: Brian Norris <computersforpeace at gmail.com>
> Cc: David Woodhouse <dwmw2 at infradead.org>
> Cc: Boris Brezillon <boris.brezillon at free-electrons.com>
> Cc: Marek Vasut <marek.vasut at gmail.com>
> Cc: Richard Weinberger <richard at nod.at>
> Cc: Cyrille Pitchen <cyrille.pitchen at wedev4u.fr>
> Cc: linux-mtd at lists.infradead.org
> ---
> No difference for the change itself from the original patch v1.
> But just updated the patch subject to add version as v3.
> Form the original patch v1 it is also changed to add Brian as Cc and the patch is sent to Boris-san as To.
>   Note: This was changed from the second patch v2 and this v3 patch is not changed about this.
> 
> This fix is required to resolve the flash write error caused on our products.
> 
> This patch itself is not depended to other related patches to fix for the flash erase error.
> The flash erase error is not able to still reproduce at this moment.
> So I will do fix the flash erase error separately.
> 
> But the flash write error can be reproduced and this fix can resolve it.
> So please review the patch again and if any problem please let me know.
> 
> 
>  drivers/mtd/chips/cfi_cmdset_0002.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index 56aa6b75213d..5e9f2ca0a6c1 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -1879,7 +1879,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
>                 if (time_after(jiffies, timeo) && !chip_ready(map, adr))
>                         break;
> 
> -               if (chip_ready(map, adr)) {
> +               if (chip_ready(map, adr) && chip_good(map, adr, datum)) {
>                         xip_enable(map, chip, adr);
>                         goto op_done;
>                 }

What goes on here, I think, is that the Toggle bit is not reliable and adding chip_good()
basically tests DQ7 too. 
If that is the case, one should only need chip_good(), have you tested that?

In general I think toggle bit is awkward and moving to DQ7 would simply things once
tests for other status bits are added.

 Jocke