linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mtd: chips: AMD chip 0x2201 - write words not buffers
@ 2021-02-23 17:00 Mauri Sandberg
  2021-03-08  5:47 ` Vignesh Raghavendra
  2021-03-09 17:48 ` [PATCH v2] mtd: cfi_cmdset_0002: AMD chip 0x2201 - write words Mauri Sandberg
  0 siblings, 2 replies; 6+ messages in thread
From: Mauri Sandberg @ 2021-02-23 17:00 UTC (permalink / raw)
  To: linux-mtd; +Cc: Mauri Sandberg

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(+)

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;
+}
+
 /* 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 },
 	{ 0, 0, NULL }
 };
+
 static struct cfi_fixup jedec_fixup_table[] = {
 	{ CFI_MFR_SST, SST49LF004B, fixup_use_fwh_lock },
 	{ CFI_MFR_SST, SST49LF040B, fixup_use_fwh_lock },
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] mtd: chips: AMD chip 0x2201 - write words not buffers
  2021-02-23 17:00 [PATCH] mtd: chips: AMD chip 0x2201 - write words not buffers Mauri Sandberg
@ 2021-03-08  5:47 ` Vignesh Raghavendra
  2021-03-08 17:41   ` Mauri Sandberg
  2021-03-09 17:48 ` [PATCH v2] mtd: cfi_cmdset_0002: AMD chip 0x2201 - write words Mauri Sandberg
  1 sibling, 1 reply; 6+ messages in thread
From: Vignesh Raghavendra @ 2021-03-08  5:47 UTC (permalink / raw)
  To: Mauri Sandberg, linux-mtd

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/

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mtd: chips: AMD chip 0x2201 - write words not buffers
  2021-03-08  5:47 ` Vignesh Raghavendra
@ 2021-03-08 17:41   ` Mauri Sandberg
  0 siblings, 0 replies; 6+ messages in thread
From: Mauri Sandberg @ 2021-03-08 17:41 UTC (permalink / raw)
  To: Vignesh Raghavendra, linux-mtd

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

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] mtd: cfi_cmdset_0002: AMD chip 0x2201 - write words
  2021-02-23 17:00 [PATCH] mtd: chips: AMD chip 0x2201 - write words not buffers Mauri Sandberg
  2021-03-08  5:47 ` Vignesh Raghavendra
@ 2021-03-09 17:48 ` Mauri Sandberg
  2021-04-13 16:26   ` Vignesh Raghavendra
  1 sibling, 1 reply; 6+ messages in thread
From: Mauri Sandberg @ 2021-03-09 17:48 UTC (permalink / raw)
  To: linux-mtd; +Cc: vigneshr, richard, miquel.raynal, Mauri Sandberg

Buffer writes do not work with AMD chip 0x2201. The chip in question
is a AMD/Spansion/Cypress Semiconductor S29GL256N and datasheet [1]
talks about writing buffers being possible. While waiting for a neater
solution resort to writing word-sized chunks only.

Without the patch kernel logs will be flooded with entries like below:

jffs2_scan_eraseblock(): End of filesystem marker found at 0x0
jffs2_build_filesystem(): unlocking the mtd device...
done.
jffs2_build_filesystem(): erasing all blocks after the end marker...
MTD do_write_buffer_wait(): software timeout, address:0x01ec000a.
jffs2: Write clean marker to block at 0x01920000 failed: -5
MTD do_write_buffer_wait(): software timeout, address:0x01e2000a.
jffs2: Write clean marker to block at 0x01880000 failed: -5
MTD do_write_buffer_wait(): software timeout, address:0x01e0000a.
jffs2: Write clean marker to block at 0x01860000 failed: -5
MTD do_write_buffer_wait(): software timeout, address:0x01dc000a.
jffs2: Write clean marker to block at 0x01820000 failed: -5
MTD do_write_buffer_wait(): software timeout, address:0x01da000a.
jffs2: Write clean marker to block at 0x01800000 failed: -5
...

Tested on a Buffalo wzr-hp-g300nh running kernel 5.10.16.

[1] https://www.cypress.com/file/219941/download
or  https://datasheetspdf.com/pdf-file/565708/SPANSION/S29GL256N/1

Signed-off-by: Mauri Sandberg <sandberg@mailfence.com>
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index a1f3e1031c3d..28b6f3583f8a 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -272,6 +272,10 @@ static void fixup_use_write_buffers(struct mtd_info *mtd)
 {
 	struct map_info *map = mtd->priv;
 	struct cfi_private *cfi = map->fldrv_priv;
+
+	if ((cfi->mfr == CFI_MFR_AMD) && (cfi->id == 0x2201))
+		return;
+
 	if (cfi->cfiq->BufWriteTimeoutTyp) {
 		pr_debug("Using buffer write method\n");
 		mtd->_write = cfi_amdstd_write_buffers;

base-commit: 5de15b610f785f0e188fefb707f0b19de156968a
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] mtd: cfi_cmdset_0002: AMD chip 0x2201 - write words
  2021-03-09 17:48 ` [PATCH v2] mtd: cfi_cmdset_0002: AMD chip 0x2201 - write words Mauri Sandberg
@ 2021-04-13 16:26   ` Vignesh Raghavendra
  2021-04-16  7:45     ` Mauri Sandberg
  0 siblings, 1 reply; 6+ messages in thread
From: Vignesh Raghavendra @ 2021-04-13 16:26 UTC (permalink / raw)
  To: linux-mtd, Mauri Sandberg; +Cc: Vignesh Raghavendra, richard, miquel.raynal

Hi Mauri Sandberg,

On Tue, 9 Mar 2021 19:48:59 +0200, Mauri Sandberg wrote:
> Buffer writes do not work with AMD chip 0x2201. The chip in question
> is a AMD/Spansion/Cypress Semiconductor S29GL256N and datasheet [1]
> talks about writing buffers being possible. While waiting for a neater
> solution resort to writing word-sized chunks only.
> 
> Without the patch kernel logs will be flooded with entries like below:
> 
> [...]

Reworded the $subject to:

	mtd: cfi_cmdset_0002: Disable buffered writes for AMD chip 0x2201

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git cfi/next, thanks!
[1/1] mtd: cfi_cmdset_0002: AMD chip 0x2201 - write words
      https://git.kernel.org/mtd/c/7e44041136

--
Regards
Vignesh


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] mtd: cfi_cmdset_0002: AMD chip 0x2201 - write words
  2021-04-13 16:26   ` Vignesh Raghavendra
@ 2021-04-16  7:45     ` Mauri Sandberg
  0 siblings, 0 replies; 6+ messages in thread
From: Mauri Sandberg @ 2021-04-16  7:45 UTC (permalink / raw)
  To: Vignesh Raghavendra, linux-mtd
  Cc: miquel.raynal, Vignesh Raghavendra, richard

> ----------------------------------------
> From: Vignesh Raghavendra <vigneshr@ti.com>
> Reworded the $subject to:
> 
> 	mtd: cfi_cmdset_0002: Disable buffered writes for AMD chip 0x2201
> 
> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git cfi/next, thanks!

Thanks for the review!

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-04-16  8:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-23 17:00 [PATCH] mtd: chips: AMD chip 0x2201 - write words not buffers Mauri Sandberg
2021-03-08  5:47 ` Vignesh Raghavendra
2021-03-08 17:41   ` Mauri Sandberg
2021-03-09 17:48 ` [PATCH v2] mtd: cfi_cmdset_0002: AMD chip 0x2201 - write words Mauri Sandberg
2021-04-13 16:26   ` Vignesh Raghavendra
2021-04-16  7:45     ` Mauri Sandberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).