mtd: chips: AMD chip 0x2201 - write words not buffers

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.

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>
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 8 ++++++++
 1 file changed, 8 insertions(+)

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

Apologies for the etiquette mistakes. It's the first patch I am submitting.
Answers to comments inline.

> ----------------------------------------
> From: Vignesh Raghavendra <vigneshr@ti.com>
> Sent: Mon Mar 08 06:47:57 CET 2021
> To: Mauri Sandberg <sandberg@mailfence.com>, <linux-mtd@lists.infradead.org>
> Subject: Re: [PATCH] mtd: chips: AMD chip 0x2201 - write words not buffers
> 
> 
> 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.
Appears to be a S29GL256N giving hits as Spansion and Cypress Semiconductor being the manufacturer.
Datasheet is available too. Will make a note of that info.

Here's the link: https://www.cypress.com/file/219941/download

> 
> 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.
It gives me '6'.  Datasheet does indicate that buffer writes are 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


-- 
Sent with https://mailfence.com
Secure and private email