Skip to contentSkip to conversation

Decreases code size of the nand_spl

PATCH New Alex Waterman 4 participants 12 messages May 4, 2011 – May 13, 2011

lore
Share & tools
lore
#1 lore raw

PATCH 1/1 Decreases code size of the nand_spl

Hi Alex,

Quoted 9 lines · On Wednesday 04 May 2011 15:10:15 Alex Waterman wrote:

From b59f1e5b0bc3684615756c12fd5c5f9fcaa4c812 Mon Sep 17 00:00:00 2001
From: Alex Waterman <[email protected]>
Date: Tue, 3 May 2011 15:00:23 -0400
Subject: [PATCH] Decreases code size of the nand_spl

The canyonland boards nand_spl size is just under the maximum 4KByte size. This patch decreases the size of the nand_spl to make a previous commit - commit 65a9db7be0868be91ba81b9b5bf821de82e6d9b0 - fit in the nand_spl.

Quoted 5 lines
Signed-off-by Alex Waterman <[email protected]>

---
This patch uses a function pointer declared as a local variable; checkpatch didn't mind but this seems like it could be (stylistically) a very bad idea. Any thoughts?

Please see my patches sent a few hours ago:

"nand_spl: nand_boot.c: Init nand_chip.options to 0" "nand_spl: nand_boot.c: Remove CONFIG_SYS_NAND_READ_DELAY"

The 2nd patch fixes the size problem as well. So no need for your patch any more.

But thanks anyway.

BTW: What platform/SoC are you using?

Cheers,
Stefan

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de

author
#2 lore raw

PATCH 1/1 Decreases code size of the nand_spl

Stefan,

Quoted 2 lines

The 2nd patch fixes the size problem as well. So no need for your patch any more.

Ahh, I saw the first but missed the second I guess.

Quoted 1 line

BTW: What platform/SoC are you using?

I am using a custom board based on the AMCC sequoia board. At some point I plan to try and mainline our port for that board if possible. But I have more work to go on that.

- Alex

#3 lore raw

PATCH 1/1 Decreases code size of the nand_spl

In reply to Stefan Roese
Quoted 17 lines · On Wed, 4 May 2011 15:47:27 +0200 Stefan Roese <[email protected]> wrote:

Hi Alex,

Quoted 14 lines · On Wednesday 04 May 2011 15:10:15 Alex Waterman wrote:

From b59f1e5b0bc3684615756c12fd5c5f9fcaa4c812 Mon Sep 17 00:00:00 2001
From: Alex Waterman <[email protected]>
Date: Tue, 3 May 2011 15:00:23 -0400
Subject: [PATCH] Decreases code size of the nand_spl

The canyonland boards nand_spl size is just under the maximum 4KByte size. This patch decreases the size of the nand_spl to make a previous commit - commit 65a9db7be0868be91ba81b9b5bf821de82e6d9b0 - fit in the nand_spl.

Signed-off-by Alex Waterman <[email protected]>

---
This patch uses a function pointer declared as a local variable; checkpatch didn't mind but this seems like it could be (stylistically) a very bad idea. Any thoughts?

I don't see what's wrong with a local function pointer.

Quoted 7 lines

Please see my patches sent a few hours ago:

"nand_spl: nand_boot.c: Init nand_chip.options to 0" "nand_spl: nand_boot.c: Remove CONFIG_SYS_NAND_READ_DELAY"

The 2nd patch fixes the size problem as well. So no need for your patch any more.

Or we could apply both and save even more space, delaying the next time we run into trouble. :-)

-Scott

author
#4 lore raw

PATCH 1/1 Decreases code size of the nand_spl

Scott,

Quoted 2 lines

Or we could apply both and save even more space, delaying the next time we run into trouble. :-)

Since the spl is so limited in space, would it be worth making every function in the spl use a local function pointer instead of lots of dereferences?

- Alex

#5 lore raw

PATCH 1/1 Decreases code size of the nand_spl

Quoted 8 lines · On Wed, 4 May 2011 14:24:15 -0400 Alex Waterman <[email protected]> wrote:

Scott,

Quoted 2 lines

Or we could apply both and save even more space, delaying the next time we run into trouble. :-)

Since the spl is so limited in space, would it be worth making every function in the spl use a local function pointer instead of lots of dereferences?

It'll only help when the same pointer is called more than once in the function, but I'd be fine with doing it in those cases.

-Scott

author
#6 lore raw

PATCH v2 1/1 Decreases code size of the nand_spl

This patch decreases the code size of the nand_spl by turning multiple function pointer dereferences in a single function into a single local function pointer.

Signed-off-by Alex Waterman <[email protected]>
Cc Scott Wood <[email protected]>
Cc Stefan Roese <[email protected]>

---
This works on my local board but I haven't tested it on anything else since I do not have access to any other hardware. I did make sure the canyonlands board still builds.

 nand_spl/nand_boot.c |   50 ++++++++++++++++++++++++++++----------------------
 1 files changed, 28 insertions(+), 22 deletions(-)
nand_spl/nand_boot.c +28 -22
index 4a96878..dd7f3b2 100644
@@ -35,34 +35,37 @@ static int nand_command(struct mtd_info *mtd, int block, int page, int offs, u8
 {
 	struct nand_chip *this = mtd->priv;
 	int page_addr = page + block * CONFIG_SYS_NAND_PAGE_COUNT;
+	void (*cmd_ctrl)(struct mtd_info *mtd, int cmd,
+			unsigned int ctrl) = this->cmd_ctrl;
+	int (*dev_ready)(struct mtd_info *mtd) = this->dev_ready;
 
-	if (this->dev_ready)
-		while (!this->dev_ready(mtd))
+	if (dev_ready != NULL)
+		while (!dev_ready(mtd))
 			;
 	else
 		CONFIG_SYS_NAND_READ_DELAY;
 
 	/* Begin command latch cycle */
-	this->cmd_ctrl(mtd, cmd, NAND_CTRL_CLE | NAND_CTRL_CHANGE);
+	cmd_ctrl(mtd, cmd, NAND_CTRL_CLE | NAND_CTRL_CHANGE);
 	/* Set ALE and clear CLE to start address cycle */
 	/* Column address */
-	this->cmd_ctrl(mtd, offs, NAND_CTRL_ALE | NAND_CTRL_CHANGE);
-	this->cmd_ctrl(mtd, page_addr & 0xff, NAND_CTRL_ALE); /* A[16:9] */
-	this->cmd_ctrl(mtd, (page_addr >> 8) & 0xff,
+	cmd_ctrl(mtd, offs, NAND_CTRL_ALE | NAND_CTRL_CHANGE);
+	cmd_ctrl(mtd, page_addr & 0xff, NAND_CTRL_ALE); /* A[16:9] */
+	cmd_ctrl(mtd, (page_addr >> 8) & 0xff,
 		       NAND_CTRL_ALE); /* A[24:17] */
 #ifdef CONFIG_SYS_NAND_4_ADDR_CYCLE
 	/* One more address cycle for devices > 32MiB */
-	this->cmd_ctrl(mtd, (page_addr >> 16) & 0x0f,
+	cmd_ctrl(mtd, (page_addr >> 16) & 0x0f,
 		       NAND_CTRL_ALE); /* A[28:25] */
 #endif
 	/* Latch in address */
-	this->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
+	cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
 
 	/*
 	 * Wait a while for the data to be ready
 	 */
-	if (this->dev_ready)
-		while (!this->dev_ready(mtd))
+	if (dev_ready != NULL)
+		while (!dev_ready(mtd))
 			;
 	else
 		CONFIG_SYS_NAND_READ_DELAY;

@@ -77,9 +80,12 @@ static int nand_command(struct mtd_info *mtd, int block, int page, int offs, u8
 {
 	struct nand_chip *this = mtd->priv;
 	int page_addr = page + block * CONFIG_SYS_NAND_PAGE_COUNT;
+	void (*cmd_ctrl)(struct mtd_info *mtd, int cmd,
+			unsigned int ctrl) = this->cmd_ctrl;
+	int (*dev_ready)(struct mtd_info *mtd) = this->dev_ready;
 
-	if (this->dev_ready)
-		while (!this->dev_ready(mtd))
+	if (dev_ready != NULL)
+		while (!dev_ready(mtd))
 			;
 	else
 		CONFIG_SYS_NAND_READ_DELAY;

@@ -95,31 +101,31 @@ static int nand_command(struct mtd_info *mtd, int block, int page, int offs, u8
 		offs >>= 1;
 
 	/* Begin command latch cycle */
-	this->cmd_ctrl(mtd, cmd, NAND_CTRL_CLE | NAND_CTRL_CHANGE);
+	cmd_ctrl(mtd, cmd, NAND_CTRL_CLE | NAND_CTRL_CHANGE);
 	/* Set ALE and clear CLE to start address cycle */
 	/* Column address */
-	this->cmd_ctrl(mtd, offs & 0xff,
+	cmd_ctrl(mtd, offs & 0xff,
 		       NAND_CTRL_ALE | NAND_CTRL_CHANGE); /* A[7:0] */
-	this->cmd_ctrl(mtd, (offs >> 8) & 0xff, NAND_CTRL_ALE); /* A[11:9] */
+	cmd_ctrl(mtd, (offs >> 8) & 0xff, NAND_CTRL_ALE); /* A[11:9] */
 	/* Row address */
-	this->cmd_ctrl(mtd, (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */
-	this->cmd_ctrl(mtd, ((page_addr >> 8) & 0xff),
+	cmd_ctrl(mtd, (page_addr & 0xff), NAND_CTRL_ALE); /* A[19:12] */
+	cmd_ctrl(mtd, ((page_addr >> 8) & 0xff),
 		       NAND_CTRL_ALE); /* A[27:20] */
 #ifdef CONFIG_SYS_NAND_5_ADDR_CYCLE
 	/* One more address cycle for devices > 128MiB */
-	this->cmd_ctrl(mtd, (page_addr >> 16) & 0x0f,
+	cmd_ctrl(mtd, (page_addr >> 16) & 0x0f,
 		       NAND_CTRL_ALE); /* A[31:28] */
 #endif
 	/* Latch in address */
-	this->cmd_ctrl(mtd, NAND_CMD_READSTART,
+	cmd_ctrl(mtd, NAND_CMD_READSTART,
 		       NAND_CTRL_CLE | NAND_CTRL_CHANGE);
-	this->cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
+	cmd_ctrl(mtd, NAND_CMD_NONE, NAND_NCE | NAND_CTRL_CHANGE);
 
 	/*
 	 * Wait a while for the data to be ready
 	 */
-	if (this->dev_ready)
-		while (!this->dev_ready(mtd))
+	if (dev_ready != NULL)
+		while (!dev_ready(mtd))
 			;
 	else
 		CONFIG_SYS_NAND_READ_DELAY;
Signature · 1 line
1.7.4.4
#7 lore raw

PATCH v2 1/1 Decreases code size of the nand_spl

Dear Scott,

Quoted 6 lines · In message <[email protected]> Alex Waterman wrote:

This patch decreases the code size of the nand_spl by turning multiple function pointer dereferences in a single function into a single local function pointer.

Signed-off-by Alex Waterman <[email protected]>
Cc Scott Wood <[email protected]>
Cc Stefan Roese <[email protected]>
Quoted 7 lines

---
This works on my local board but I haven't tested it on anything else since I do not have access to any other hardware. I did make sure the canyonlands board still builds.

 nand_spl/nand_boot.c |   50 ++++++++++++++++++++++++++++----------------------
 1 files changed, 28 insertions(+), 22 deletions(-)

Can this patch go in, please? I would like to see the build problems fixed.

Stefan, eventually you want to add your Tested-by: and/or Acked-by: ?

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I think it's a new feature. Don't tell anyone it was an accident. :-)
  -- Larry Wall on s/foo/bar/eieio in <[email protected]>
#8 lore raw

PATCH v2 1/1 Decreases code size of the nand_spl

Quoted 14 lines · On Thu, 12 May 2011 23:49:40 +0200 Wolfgang Denk <[email protected]> wrote:

Dear Scott,

Quoted 11 lines · In message <[email protected]> Alex Waterman wrote:

This patch decreases the code size of the nand_spl by turning multiple function pointer dereferences in a single function into a single local function pointer.

Signed-off-by Alex Waterman <[email protected]>
Cc Scott Wood <[email protected]>
Cc Stefan Roese <[email protected]>

---
This works on my local board but I haven't tested it on anything else since I do not have access to any other hardware. I did make sure the canyonlands board still builds.

Quoted 5 lines
Quoted 2 lines
 nand_spl/nand_boot.c |   50 ++++++++++++++++++++++++++++----------------------
 1 files changed, 28 insertions(+), 22 deletions(-)

Can this patch go in, please? I would like to see the build problems fixed.

Yes, sorry for the delay -- I'll try to get to it tomorrow.

-Scott

#9 lore raw

PATCH v2 1/1 Decreases code size of the nand_spl

Dear Scott,

Quoted 5 lines · In message <[email protected]> you wrote:
Quoted 3 lines

Can this patch go in, please? I would like to see the build problems fixed.

Yes, sorry for the delay -- I'll try to get to it tomorrow.

Thanks. Guess you guys are as busy as evryone else ;-)

Best regards,

Wolfgang Denk

Signature · 5 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The use of COBOL cripples the mind; its teaching  should,  therefore,
be regarded as a criminal offense.                   - E. W. Dijkstra
#10 lore raw

PATCH v2 1/1 Decreases code size of the nand_spl

In reply to Wolfgang Denk
Quoted 12 lines · On Thursday 12 May 2011 23:49:40 Wolfgang Denk wrote:
Quoted 12 lines

This patch decreases the code size of the nand_spl by turning multiple function pointer dereferences in a single function into a single local function pointer.

Signed-off-by Alex Waterman <[email protected]>
Cc Scott Wood <[email protected]>
Cc Stefan Roese <[email protected]>

---
This works on my local board but I haven't tested it on anything else since I do not have access to any other hardware. I did make sure the canyonlands board still builds.

Quoted 8 lines
Quoted 3 lines
 nand_spl/nand_boot.c |   50

++++++++++++++++++++++++++++---------------------- 1 files changed, 28 insertions(+), 22 deletions(-)

Can this patch go in, please? I would like to see the build problems fixed.

Stefan, eventually you want to add your Tested-by: and/or Acked-by: ?

Sure:

Acked-by Stefan Roese <[email protected]>

Thanks,
Stefan

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de

#11 lore raw

PATCH 1/1 Decreases code size of the nand_spl

In reply to Alex Waterman
Quoted 9 lines · Alex Waterman

From b59f1e5b0bc3684615756c12fd5c5f9fcaa4c812 Mon Sep 17 00:00:00 2001
From: Alex Waterman <[email protected]>
Date: Tue, 3 May 2011 15:00:23 -0400
Subject: [PATCH] Decreases code size of the nand_spl

The canyonland boards nand_spl size is just under the maximum 4KByte size. This patch decreases the size of the nand_spl to make a previous commit - commit 65a9db7be0868be91ba81b9b5bf821de82e6d9b0 - fit in the nand_spl.

Quoted 7 lines
Signed-off-by Alex Waterman <[email protected]>

---
This patch uses a function pointer declared as a local variable; checkpatch didn't mind but this seems like it could be (stylistically) a very bad idea. Any thoughts?

 nand_spl/nand_boot.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

Applied to u-boot-nand-flash

-Scott