Re: [PATCH] mtd: chips: AMD chip 0x2201 - write words not buffers

[Vignesh Raghavendra <vigneshr@ti.com>]

Hi,

On 2/23/21 10:30 PM, Mauri Sandberg wrote:
> CFI flash memory driver cmdset_0002 uses buffers for write operations.
> That does not work with AMD chip with id 0x2201 and we must resort to
> writing word sized chunks only.
> 

Which flash is this? Do you have datasheet for the same.

Please add above information to the commit message for future reference.

> This patch creates fixup mechanism that renders a chip to use word sized
> write operation and adds that as the last item in a fixup table, shadowing
> the entry that causes use of buffer writes.
> 
> Without the patch kernel logs will be flooded with entries like below:
> 
> MTD do_erase_oneblock(): ERASE 0x01fa0000
> MTD do_write_buffer(): WRITE 0x01fa0000(0x00001985)
> MTD do_erase_oneblock(): ERASE 0x01f80000
> MTD do_write_buffer(): WRITE 0x01f80000(0x00001985)
> MTD do_write_buffer_wait(): software timeout, address:0x01f8000a.
> jffs2: Write clean marker to block at 0x01a60000 failed: -5
> MTD do_erase_oneblock(): ERASE 0x01f60000
> MTD do_write_buffer(): WRITE 0x01f60000(0x00001985)
> MTD do_write_buffer_wait(): software timeout, address:0x01f6000a.
> jffs2: Write clean marker to block at 0x01a40000 failed: -5
> 
> Signed-off-by: Mauri Sandberg <sandberg@mailfence.com>
> ---

1. Please explicitly CC maintainers for quicker response. Given the
amount of traffic on Mailing list, it'll take quite a while for me to
get to a patch that is not address directly to me.

Run ./scripts/get_maintainer.pl  on the patch to get list of maintainers
to be CC'd

2. $subject should start with appropriate prefix:

	"mtd: cfi_cmdset_0002: ...."

Just run git log --oneline on the file being patched to know the convention.

If you don't CC the maintainers and use the appropriate prefixes, it
will be a while until I get to such patches.


>  drivers/mtd/chips/cfi_cmdset_0002.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index a1f3e1031c3d..b8b564342062 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -279,6 +279,12 @@ static void fixup_use_write_buffers(struct mtd_info *mtd)
>  }
>  #endif /* !FORCE_WORD_WRITE */
>  
> +static void fixup_use_write_words(struct mtd_info *mtd)
> +{
> +	pr_debug("Using word write method\n");
> +	mtd->_write = cfi_amdstd_write_words;

Default is cfi_amdstd_write_words(). fixup_use_write_buffers() forces
this to cfi_amdstd_write_buffers() if 	cfi->cfiq->BufWriteTimeoutTyp is
non zero.

What does this field read in case of your flash? Should be 0 in case
buffered write is not supported.

> +}
> +
>  /* Atmel chips don't use the same PRI format as AMD chips */
>  static void fixup_convert_atmel_pri(struct mtd_info *mtd)
>  {
> @@ -470,8 +476,10 @@ static struct cfi_fixup cfi_fixup_table[] = {
>  #if !FORCE_WORD_WRITE
>  	{ CFI_MFR_ANY, CFI_ID_ANY, fixup_use_write_buffers },
>  #endif
> +	{ CFI_MFR_AMD, 0x2201, fixup_use_write_words },


Should be worked around in fixup_use_write_buffers() instead. No point
in overriding mtd->_write to cfi_amdstd_write_buffers() and then
reverting it again.

>  	{ 0, 0, NULL }
>  };
> +

Please don't do white space fixes in the same patch adding
code/feature/bug-fix

>  static struct cfi_fixup jedec_fixup_table[] = {
>  	{ CFI_MFR_SST, SST49LF004B, fixup_use_fwh_lock },
>  	{ CFI_MFR_SST, SST49LF040B, fixup_use_fwh_lock },
> 

Regards
Vignesh

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/