Optimized nand_read_buf for kirkwood

PATCH Superseded Phil Sutter 3 participants 8 messages Aug 26, 2013 – Nov 14, 2013

Subsystems (2)

Memory technology devices (MTD) NAND flash

lore

Share & tools

lore

Phil Sutter

author

2013-08-26 12:10 UTC #0 lore raw

PATCH v4 1/1 Optimized nand_read_buf for kirkwood

From: Nico Erfurth <[email protected]>

The basic idea is taken from the linux-kernel, but further optimized.

First align the buffer to 8 bytes, then use ldrd/strd to read and store in 8 byte quantities, then do the final bytes.

Tested using: 'date ; nand read.raw 0xE00000 0x0 0x10000 ; date'. Without this patch, NAND read of 132MB took 49s (~2.69MB/s). With this patch in place, reading the same amount of data was done in 27s (~4.89MB/s). So read performance is increased by ~80%!

Signed-off-by Nico Erfurth <[email protected]>

Tested-by Phil Sutter <[email protected]>

Cc Prafulla Wadaskar <[email protected]>

---
Changed since V3:
- fixed author

---
 drivers/mtd/nand/kirkwood_nand.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

drivers/mtd/nand/kirkwood_nand.c +32 -0

index 0a99a10..85ea5d2 100644
@@ -38,6 +38,37 @@ struct kwnandf_registers {
 static struct kwnandf_registers *nf_reg =
 	(struct kwnandf_registers *)KW_NANDF_BASE;
 
+
+/*
+ * The basic idea is stolen from the linux kernel, but the inner loop is
+ * optimized a bit more.
+ */
+static void kw_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int len)
+{
+	struct nand_chip *chip = mtd->priv;
+
+	while (len && (unsigned long)buf & 7) {
+		*buf++ = readb(chip->IO_ADDR_R);
+		len--;
+	};
+
+	/* This loop reads and writes 64bit per round. */
+	asm volatile (
+		"1:\n"
+		"  subs   %0, #8\n"
+		"  ldrpld r2, [%2]\n"
+		"  strpld r2, [%1], #8\n"
+		"  bhi    1b\n"
+		"  addne  %0, #8\n"
+		: "+&r" (len), "+&r" (buf)
+		: "r" (chip->IO_ADDR_R)
+		: "r2", "r3", "memory", "cc"
+	);
+
+	while (len--)
+		*buf++ = readb(chip->IO_ADDR_R);
+}
+
 /*
  * hardware specific access to control-lines/bits
  */

@@ -80,6 +111,7 @@ int board_nand_init(struct nand_chip *nand)
 	nand->ecc.mode = NAND_ECC_SOFT;
 #endif
 	nand->cmd_ctrl = kw_nand_hwcontrol;
+	nand->read_buf = kw_nand_read_buf;
 	nand->chip_delay = 40;
 	nand->select_chip = kw_nand_select_chip;
 	return 0;

Signature · 1 line

1.8.1.5

Scott Wood

2013-08-26 15:43 UTC #1 lore raw

PATCH v4 1/1 Optimized nand_read_buf for kirkwood

Quoted 12 lines · Phil Sutter

From: Nico Erfurth <[email protected]>
The basic idea is taken from the linux-kernel, but further optimized.
First align the buffer to 8 bytes, then use ldrd/strd to read and store in 8 byte quantities, then do the final bytes.
Tested using: 'date ; nand read.raw 0xE00000 0x0 0x10000 ; date'. Without this patch, NAND read of 132MB took 49s (~2.69MB/s). With this patch in place, reading the same amount of data was done in 27s (~4.89MB/s). So read performance is increased by ~80%!

Quoted 6 lines

Signed-off-by Nico Erfurth <[email protected]>
Tested-by Phil Sutter <[email protected]>
Cc Prafulla Wadaskar <[email protected]>
---
Changed since V3:
- fixed author

It needs your Signed-off-by: as well -- can I add that when applying?

-Scott

Phil Sutter

author

2013-08-26 16:00 UTC #2 lore raw

PATCH v4 1/1 Optimized nand_read_buf for kirkwood

Quoted 13 lines · Scott Wood

Quoted 12 lines · Phil Sutter
From: Nico Erfurth <[email protected]>
The basic idea is taken from the linux-kernel, but further optimized.
First align the buffer to 8 bytes, then use ldrd/strd to read and store in 8 byte quantities, then do the final bytes.
Tested using: 'date ; nand read.raw 0xE00000 0x0 0x10000 ; date'. Without this patch, NAND read of 132MB took 49s (~2.69MB/s). With this patch in place, reading the same amount of data was done in 27s (~4.89MB/s). So read performance is increased by ~80%!

Quoted 8 lines

Quoted 6 lines
Signed-off-by Nico Erfurth <[email protected]>
Tested-by Phil Sutter <[email protected]>
Cc Prafulla Wadaskar <[email protected]>
---
Changed since V3:
- fixed author
It needs your Signed-off-by: as well -- can I add that when applying?

Yes, that's fine by me. So if I submit others' patches, I need to sign them off as well?

Best wishes,

Phil Sutter
Software Engineer

Signature · 15 lines

Viprinet Europe GmbH
Mainzer Str. 43
55411 Bingen am Rhein
Germany

Phone/Zentrale:               +49 6721 49030-0
Direct line/Durchwahl:        +49 6721 49030-134
Fax:                          +49 6721 49030-109

phil.sutter at viprinet.com
http://www.viprinet.com

Registered office/Sitz der Gesellschaft: Bingen am Rhein, Germany
Commercial register/Handelsregister: Amtsgericht Mainz HRB44090
CEO/Gesch?ftsf?hrer: Simon Kissel

Phil Sutter

author

2013-08-26 16:06 UTC #3 lore raw

PATCH v4 1/1 Optimized nand_read_buf for kirkwood

Quoted 21 lines · Phil Sutter

Quoted 20 lines · Scott Wood
Quoted 18 lines · Phil Sutter
From: Nico Erfurth <[email protected]>
The basic idea is taken from the linux-kernel, but further optimized.
First align the buffer to 8 bytes, then use ldrd/strd to read and store in 8 byte quantities, then do the final bytes.
Tested using: 'date ; nand read.raw 0xE00000 0x0 0x10000 ; date'. Without this patch, NAND read of 132MB took 49s (~2.69MB/s). With this patch in place, reading the same amount of data was done in 27s (~4.89MB/s). So read performance is increased by ~80%!
Signed-off-by Nico Erfurth <[email protected]>
Tested-by Phil Sutter <[email protected]>
Cc Prafulla Wadaskar <[email protected]>
---
Changed since V3:
- fixed author

Quoted 4 lines

Quoted 1 line
It needs your Signed-off-by: as well -- can I add that when applying?
Yes, that's fine by me. So if I submit others' patches, I need to sign them off as well?

Ah, nevermind. Reading 1.12 of Documentation/SubmittingPatches helps.

Best wishes,

Phil Sutter
Software Engineer

Signature · 15 lines

Viprinet Europe GmbH
Mainzer Str. 43
55411 Bingen am Rhein
Germany

Phone/Zentrale:               +49 6721 49030-0
Direct line/Durchwahl:        +49 6721 49030-134
Fax:                          +49 6721 49030-109

phil.sutter at viprinet.com
http://www.viprinet.com

Registered office/Sitz der Gesellschaft: Bingen am Rhein, Germany
Commercial register/Handelsregister: Amtsgericht Mainz HRB44090
CEO/Gesch?ftsf?hrer: Simon Kissel

Scott Wood

2013-11-14 01:31 UTC #4 lore raw

PATCH v4 1/1 Optimized nand_read_buf for kirkwood

In reply to Phil Sutter

Quoted 16 lines · Phil Sutter

From: Nico Erfurth <[email protected]>
The basic idea is taken from the linux-kernel, but further optimized.
First align the buffer to 8 bytes, then use ldrd/strd to read and store in 8 byte quantities, then do the final bytes.
Tested using: 'date ; nand read.raw 0xE00000 0x0 0x10000 ; date'. Without this patch, NAND read of 132MB took 49s (~2.69MB/s). With this patch in place, reading the same amount of data was done in 27s (~4.89MB/s). So read performance is increased by ~80%!
Signed-off-by Nico Erfurth <[email protected]>
Tested-by Phil Sutter <[email protected]>
Cc Prafulla Wadaskar <[email protected]>

Quoted 6 lines

---
Changed since V3:
- fixed author

---
 drivers/mtd/nand/kirkwood_nand.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

I tried to build-test this, and I couldn't find any board that defines CONFIG_NAND_KIRKWOOD.

The patch that removed it was commit b5befd8211b54ae2d2fca3fbed061c879951ceaa ("arm/km: fix u-boot.kwb build breakage"), over two years ago. It's not clear whether the removal was intentional.

What target did you use to test this?

-Scott

Holger Brunck

2013-11-14 08:18 UTC #5 lore raw

PATCH v4 1/1 Optimized nand_read_buf for kirkwood

Hi Scott,

Quoted 24 lines · Scott Wood

Quoted 16 lines · Phil Sutter
From: Nico Erfurth <[email protected]>
The basic idea is taken from the linux-kernel, but further optimized.
First align the buffer to 8 bytes, then use ldrd/strd to read and store in 8 byte quantities, then do the final bytes.
Tested using: 'date ; nand read.raw 0xE00000 0x0 0x10000 ; date'. Without this patch, NAND read of 132MB took 49s (~2.69MB/s). With this patch in place, reading the same amount of data was done in 27s (~4.89MB/s). So read performance is increased by ~80%!
Signed-off-by Nico Erfurth <[email protected]>
Tested-by Phil Sutter <[email protected]>
Cc Prafulla Wadaskar <[email protected]>
Quoted 6 lines
---
Changed since V3:
- fixed author
---
 drivers/mtd/nand/kirkwood_nand.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Quoted 3 lines

I tried to build-test this, and I couldn't find any board that defines CONFIG_NAND_KIRKWOOD.

it's not in board specific code defined it's defined in a common kirkwood header:

arch/arm/include/asm/arch-kirkwood/config.h:58:#define CONFIG_NAND_KIRKWOOD

Quoted 5 lines

The patch that removed it was commit b5befd8211b54ae2d2fca3fbed061c879951ceaa ("arm/km: fix u-boot.kwb build breakage"), over two years ago. It's not clear whether the removal was intentional.

yes it was. We include this common header and therefore we don't need to redefine it in our board setup.

Regards
Holger

Scott Wood

2013-11-14 23:59 UTC #6 lore raw

PATCH v4 1/1 Optimized nand_read_buf for kirkwood

Quoted 27 lines · Holger Brunck

Hi Scott,
Quoted 24 lines · Scott Wood
Quoted 22 lines · Phil Sutter
From: Nico Erfurth <[email protected]>
The basic idea is taken from the linux-kernel, but further optimized.
First align the buffer to 8 bytes, then use ldrd/strd to read and store in 8 byte quantities, then do the final bytes.
Tested using: 'date ; nand read.raw 0xE00000 0x0 0x10000 ; date'. Without this patch, NAND read of 132MB took 49s (~2.69MB/s). With this patch in place, reading the same amount of data was done in 27s (~4.89MB/s). So read performance is increased by ~80%!
Signed-off-by Nico Erfurth <[email protected]>
Tested-by Phil Sutter <[email protected]>
Cc Prafulla Wadaskar <[email protected]>
---
Changed since V3:
- fixed author
---
 drivers/mtd/nand/kirkwood_nand.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Quoted 7 lines

Quoted 3 lines
I tried to build-test this, and I couldn't find any board that defines CONFIG_NAND_KIRKWOOD.
it's not in board specific code defined it's defined in a common kirkwood header:
arch/arm/include/asm/arch-kirkwood/config.h:58:#define CONFIG_NAND_KIRKWOOD

Oops. I thought I grepped the whole tree rather than just include/, but apparently not.

-Scott

Phil Sutter

author

2013-11-14 12:12 UTC #7 lore raw

PATCH v4 1/1 Optimized nand_read_buf for kirkwood

In reply to Scott Wood

Scott,

Quoted 27 lines · Scott Wood

Quoted 16 lines · Phil Sutter
From: Nico Erfurth <[email protected]>
The basic idea is taken from the linux-kernel, but further optimized.
First align the buffer to 8 bytes, then use ldrd/strd to read and store in 8 byte quantities, then do the final bytes.
Tested using: 'date ; nand read.raw 0xE00000 0x0 0x10000 ; date'. Without this patch, NAND read of 132MB took 49s (~2.69MB/s). With this patch in place, reading the same amount of data was done in 27s (~4.89MB/s). So read performance is increased by ~80%!
Signed-off-by Nico Erfurth <[email protected]>
Tested-by Phil Sutter <[email protected]>
Cc Prafulla Wadaskar <[email protected]>
Quoted 6 lines
---
Changed since V3:
- fixed author
---
 drivers/mtd/nand/kirkwood_nand.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
I tried to build-test this, and I couldn't find any board that defines CONFIG_NAND_KIRKWOOD.

Quoted 6 lines

The patch that removed it was commit b5befd8211b54ae2d2fca3fbed061c879951ceaa ("arm/km: fix u-boot.kwb build breakage"), over two years ago. It's not clear whether the removal was intentional.
What target did you use to test this?

I tested using a custom board with Marvell Kirkwood SoC, but e.g. the Marvell OpenRD Ultimate should be fine.

Best wishes,

Phil Sutter
Software Engineer

Signature · 15 lines

Viprinet Europe GmbH
Mainzer Str. 43
55411 Bingen am Rhein
Germany

Phone/Zentrale:               +49 6721 49030-0
Direct line/Durchwahl:        +49 6721 49030-134
Fax:                          +49 6721 49030-109

phil.sutter at viprinet.com
http://www.viprinet.com

Registered office/Sitz der Gesellschaft: Bingen am Rhein, Germany
Commercial register/Handelsregister: Amtsgericht Mainz HRB44090
CEO/Gesch?ftsf?hrer: Simon Kissel