Skip to contentSkip to conversation

omap3 beagle: nand (mostly) cleanups

PATCH In review David Brownell 4 participants 7 messages Aug 29, 2008 – Sep 5, 2008

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 omap3 beagle: nand (mostly) cleanups

From: David Brownell <[email protected]>

Clean up the Beagle NAND config a bit: use SZ_128K instead of an arithmetic expression for the NAND block size. Set the NAND_BUSWIDTH_16 flag. Make its init routine static.

(Unrelated: make omap3_beagle_i2c_init be part of beagle_init. It shouldn't be called on non-beagle hardware; and beagle_init already runs with other arch_initcall code.)

---
 arch/arm/mach-omap2/board-omap3beagle.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/board-omap3beagle.c +7 -7
@@ -50,24 +50,24 @@ static struct mtd_partition omap3beagle_
 	{
 		.name		= "X-Loader",
 		.offset		= 0,
-		.size		= 4*(64 * 2048),
+		.size		= 4 * SZ_128K,
 		.mask_flags	= MTD_WRITEABLE,	/* force read-only */
 	},
 	{
 		.name		= "U-Boot",
 		.offset		= MTDPART_OFS_APPEND,	/* Offset = 0x80000 */
-		.size		= 15*(64 * 2048),
+		.size		= 15 * SZ_128K,
 		.mask_flags	= MTD_WRITEABLE,	/* force read-only */
 	},
 	{
 		.name		= "U-Boot Env",
 		.offset		= MTDPART_OFS_APPEND,	/* Offset = 0x260000 */
-		.size		= 1*(64 * 2048),
+		.size		= 1 * SZ_128K,
 	},
 	{
 		.name		= "Kernel",
 		.offset		= MTDPART_OFS_APPEND,	/* Offset = 0x280000 */
-		.size		= 32*(64 * 2048),
+		.size		= 32 * SZ_128K,
 	},
 	{
 		.name		= "File System",
@@ -77,6 +77,7 @@ static struct mtd_partition omap3beagle_
 };
 
 static struct omap_nand_platform_data omap3beagle_nand_data = {
+	.options	= NAND_BUSWIDTH_16,
 	.parts		= omap3beagle_nand_partitions,
 	.nr_parts	= ARRAY_SIZE(omap3beagle_nand_partitions),
 	.dma_channel	= -1,		/* disable DMA in OMAP NAND driver */
@@ -205,7 +206,7 @@ static struct platform_device *omap3_bea
 	&keys_gpio,
 };
 
-void __init omap3beagle_flash_init(void)
+static void __init omap3beagle_flash_init(void)
 {
 	u8 cs = 0;
 	u8 nandcs = GPMC_CS_NUM + 1;
@@ -245,6 +246,7 @@ void __init omap3beagle_flash_init(void)
 
 static void __init omap3_beagle_init(void)
 {
+	omap3_beagle_i2c_init();
 	platform_add_devices(omap3_beagle_devices, ARRAY_SIZE(omap3_beagle_devices));
 	omap_board_config = omap3_beagle_config;
 	omap_board_config_size = ARRAY_SIZE(omap3_beagle_config);
@@ -255,8 +257,6 @@ static void __init omap3_beagle_init(voi
 	omap3beagle_flash_init();
 }
 
-arch_initcall(omap3_beagle_i2c_init);
-
 static void __init omap3_beagle_map_io(void)
 {
 	omap2_set_globals_343x();
#1 lore raw
Quoted 5 lines

From: David Brownell <[email protected]>

Clean up the Beagle NAND config a bit: use SZ_128K instead of an arithmetic expression for the NAND block size. Set the NAND_BUSWIDTH_16 flag. Make its init routine static.

Actually, this was talked about before a bit and the SZ_xyz flags were left out.

The point of telling the geometry in the addition was to be explicit about the number of sectors being used.

For x-loader you should use 4. The size doesn't matter so much. The ROM code will search for the 1 non-bad block sector in the first four sectors.

A porter of some of these things might try and use size thinking that was the requirement when that is not the whole story. The geometry does matter here.

Regards,
Richard W.

#2 lore raw
Quoted 9 lines · Richard
Quoted 3 lines

Clean up the Beagle NAND config a bit:  use SZ_128K instead of an arithmetic expression for the NAND block size.  Set the NAND_BUSWIDTH_16 flag.  Make its init routine static.

Actually, this was talked about before a bit and the SZ_xyz flags were left out.

The point of telling the geometry in the addition was to be explicit about the number of sectors being used.

So #define NAND_BLOCK_SIZE SZ_128K and use that symbol instead?

Frankly N*(64 * 2048) isn't very meaningful. The comment at the top already says everything is in units of blocks, but one is left to guess that "block" is "(64 * 2048)"...

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
In reply to Woodruff, Richard
Quoted 5 lines · Richard

Actually, this was talked about before a bit and the SZ_xyz flags were left out.

The point of telling the geometry in the addition was to be explicit about the number of sectors being used.

So ... this instead?

=== CUT HERE
From: David Brownell <[email protected]>

Clean up the Beagle NAND config a bit: use SZ_128K instead of an arithmetic expression for the NAND block size. Set the NAND_BUSWIDTH_16 flag. Make its init routine static.

(Unrelated: make omap3_beagle_i2c_init be part of beagle_init. It shouldn't be called on non-beagle hardware; and beagle_init already runs with other arch_initcall code.)

---
 arch/arm/mach-omap2/board-omap3beagle.c |   17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)
arch/arm/mach-omap2/board-omap3beagle.c +10 -7
@@ -42,32 +42,35 @@
 #include <mach/gpmc.h>
 #include <mach/nand.h>
 
+
 #define GPMC_CS0_BASE  0x60
 #define GPMC_CS_SIZE   0x30
 
+#define NAND_BLOCK_SIZE		SZ_128K
+
 static struct mtd_partition omap3beagle_nand_partitions[] = {
 	/* All the partition sizes are listed in terms of NAND block size */
 	{
 		.name		= "X-Loader",
 		.offset		= 0,
-		.size		= 4*(64 * 2048),
+		.size		= 4 * NAND_BLOCK_SIZE,
 		.mask_flags	= MTD_WRITEABLE,	/* force read-only */
 	},
 	{
 		.name		= "U-Boot",
 		.offset		= MTDPART_OFS_APPEND,	/* Offset = 0x80000 */
-		.size		= 15*(64 * 2048),
+		.size		= 15 * NAND_BLOCK_SIZE,
 		.mask_flags	= MTD_WRITEABLE,	/* force read-only */
 	},
 	{
 		.name		= "U-Boot Env",
 		.offset		= MTDPART_OFS_APPEND,	/* Offset = 0x260000 */
-		.size		= 1*(64 * 2048),
+		.size		= 1 * NAND_BLOCK_SIZE,
 	},
 	{
 		.name		= "Kernel",
 		.offset		= MTDPART_OFS_APPEND,	/* Offset = 0x280000 */
-		.size		= 32*(64 * 2048),
+		.size		= 32 * NAND_BLOCK_SIZE,
 	},
 	{
 		.name		= "File System",
@@ -77,6 +80,7 @@ static struct mtd_partition omap3beagle_
 };
 
 static struct omap_nand_platform_data omap3beagle_nand_data = {
+	.options	= NAND_BUSWIDTH_16,
 	.parts		= omap3beagle_nand_partitions,
 	.nr_parts	= ARRAY_SIZE(omap3beagle_nand_partitions),
 	.dma_channel	= -1,		/* disable DMA in OMAP NAND driver */
@@ -205,7 +209,7 @@ static struct platform_device *omap3_bea
 	&keys_gpio,
 };
 
-void __init omap3beagle_flash_init(void)
+static void __init omap3beagle_flash_init(void)
 {
 	u8 cs = 0;
 	u8 nandcs = GPMC_CS_NUM + 1;
@@ -245,6 +249,7 @@ void __init omap3beagle_flash_init(void)
 
 static void __init omap3_beagle_init(void)
 {
+	omap3_beagle_i2c_init();
 	platform_add_devices(omap3_beagle_devices, ARRAY_SIZE(omap3_beagle_devices));
 	omap_board_config = omap3_beagle_config;
 	omap_board_config_size = ARRAY_SIZE(omap3_beagle_config);
@@ -255,8 +260,6 @@ static void __init omap3_beagle_init(voi
 	omap3beagle_flash_init();
 }
 
-arch_initcall(omap3_beagle_i2c_init);
-
 static void __init omap3_beagle_map_io(void)
 {
 	omap2_set_globals_343x();
#4 lore raw

Hi,

Quoted 8 lines

From: David Brownell [mailto:[email protected]]

Quoted 5 lines

Actually, this was talked about before a bit and the SZ_xyz flags were left out.

The point of telling the geometry in the addition was to be explicit about the number of sectors being used.

So ... this instead?

Yes that looks good thanks.

Quoted 1 line
Quoted 88 lines
---
 arch/arm/mach-omap2/board-omap3beagle.c |   17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)
arch/arm/mach-omap2/board-omap3beagle.c +4 -1
@@ -42,32 +42,35 @@
 #include <mach/gpmc.h>
 #include <mach/nand.h>
 
+
 #define GPMC_CS0_BASE  0x60
 #define GPMC_CS_SIZE   0x30
 
+#define NAND_BLOCK_SIZE              SZ_128K
+
 static struct mtd_partition omap3beagle_nand_partitions[] = {
      /* All the partition sizes are listed in terms of NAND block size
 */
      {
              .name           = "X-Loader",
              .offset         = 0,
-             .size           = 4*(64 * 2048),
+             .size           = 4 * NAND_BLOCK_SIZE,
              .mask_flags     = MTD_WRITEABLE,        /* force read-only */
      },
      {
              .name           = "U-Boot",
              .offset         = MTDPART_OFS_APPEND,   /* Offset =

0x80000 */

arch/arm/mach-omap2/board-omap3beagle.c +1 -1
-             .size           = 15*(64 * 2048),
+             .size           = 15 * NAND_BLOCK_SIZE,
              .mask_flags     = MTD_WRITEABLE,        /* force read-only */
      },
      {
              .name           = "U-Boot Env",
              .offset         = MTDPART_OFS_APPEND,   /* Offset =

0x260000 */

arch/arm/mach-omap2/board-omap3beagle.c +1 -1
-             .size           = 1*(64 * 2048),
+             .size           = 1 * NAND_BLOCK_SIZE,
      },
      {
              .name           = "Kernel",
              .offset         = MTDPART_OFS_APPEND,   /* Offset =

0x280000 */

arch/arm/mach-omap2/board-omap3beagle.c +2 -1
-             .size           = 32*(64 * 2048),
+             .size           = 32 * NAND_BLOCK_SIZE,
      },
      {
              .name           = "File System",
@@ -77,6 +80,7 @@ static struct mtd_partition omap3beagle_
 };
 
 static struct omap_nand_platform_data omap3beagle_nand_data = {
+     .options        = NAND_BUSWIDTH_16,
      .parts          = omap3beagle_nand_partitions,
      .nr_parts       = ARRAY_SIZE(omap3beagle_nand_partitions),
      .dma_channel    = -1,           /* disable DMA in OMAP NAND driver

*/

arch/arm/mach-omap2/board-omap3beagle.c +2 -1
@@ -205,7 +209,7 @@ static struct platform_device *omap3_bea
      &keys_gpio,
 };
 
-void __init omap3beagle_flash_init(void)
+static void __init omap3beagle_flash_init(void)
 {
      u8 cs = 0;
      u8 nandcs = GPMC_CS_NUM + 1;
@@ -245,6 +249,7 @@ void __init omap3beagle_flash_init(void)
 
 static void __init omap3_beagle_init(void)
 {
+     omap3_beagle_i2c_init();
      platform_add_devices(omap3_beagle_devices,
 ARRAY_SIZE(omap3_beagle_devices));
      omap_board_config = omap3_beagle_config;
      omap_board_config_size = ARRAY_SIZE(omap3_beagle_config);
arch/arm/mach-omap2/board-omap3beagle.c +0 -2
@@ -255,8 +260,6 @@ static void __init omap3_beagle_init(voi
      omap3beagle_flash_init();
 }
 
-arch_initcall(omap3_beagle_i2c_init);
-
 static void __init omap3_beagle_map_io(void)
 {
      omap2_set_globals_343x();
maintainer
#5 lore raw
Quoted 6 lines · Woodruff, Richard

Hi,

Quoted 4 lines

From: David Brownell [mailto:[email protected]]

Quoted 3 lines

Actually, this was talked about before a bit and the SZ_xyz flags were left out.

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

The point of telling the geometry in the addition was to be explicit about the number of sectors being used.

So ... this instead?

Yes that looks good thanks.

Pushing these three beagle patches today.

Tony

Quoted 98 lines
Quoted 2 lines
Quoted 80 lines
---
 arch/arm/mach-omap2/board-omap3beagle.c |   17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)
arch/arm/mach-omap2/board-omap3beagle.c +4 -1
@@ -42,32 +42,35 @@
 #include <mach/gpmc.h>
 #include <mach/nand.h>
 
+
 #define GPMC_CS0_BASE  0x60
 #define GPMC_CS_SIZE   0x30
 
+#define NAND_BLOCK_SIZE              SZ_128K
+
 static struct mtd_partition omap3beagle_nand_partitions[] = {
      /* All the partition sizes are listed in terms of NAND block size
 */
      {
              .name           = "X-Loader",
              .offset         = 0,
-             .size           = 4*(64 * 2048),
+             .size           = 4 * NAND_BLOCK_SIZE,
              .mask_flags     = MTD_WRITEABLE,        /* force read-only */
      },
      {
              .name           = "U-Boot",
              .offset         = MTDPART_OFS_APPEND,   /* Offset =

0x80000 */

arch/arm/mach-omap2/board-omap3beagle.c +1 -1
-             .size           = 15*(64 * 2048),
+             .size           = 15 * NAND_BLOCK_SIZE,
              .mask_flags     = MTD_WRITEABLE,        /* force read-only */
      },
      {
              .name           = "U-Boot Env",
              .offset         = MTDPART_OFS_APPEND,   /* Offset =

0x260000 */

arch/arm/mach-omap2/board-omap3beagle.c +1 -1
-             .size           = 1*(64 * 2048),
+             .size           = 1 * NAND_BLOCK_SIZE,
      },
      {
              .name           = "Kernel",
              .offset         = MTDPART_OFS_APPEND,   /* Offset =

0x280000 */

arch/arm/mach-omap2/board-omap3beagle.c +2 -1
-             .size           = 32*(64 * 2048),
+             .size           = 32 * NAND_BLOCK_SIZE,
      },
      {
              .name           = "File System",
@@ -77,6 +80,7 @@ static struct mtd_partition omap3beagle_
 };
 
 static struct omap_nand_platform_data omap3beagle_nand_data = {
+     .options        = NAND_BUSWIDTH_16,
      .parts          = omap3beagle_nand_partitions,
      .nr_parts       = ARRAY_SIZE(omap3beagle_nand_partitions),
      .dma_channel    = -1,           /* disable DMA in OMAP NAND driver

*/

arch/arm/mach-omap2/board-omap3beagle.c +2 -1
@@ -205,7 +209,7 @@ static struct platform_device *omap3_bea
      &keys_gpio,
 };
 
-void __init omap3beagle_flash_init(void)
+static void __init omap3beagle_flash_init(void)
 {
      u8 cs = 0;
      u8 nandcs = GPMC_CS_NUM + 1;
@@ -245,6 +249,7 @@ void __init omap3beagle_flash_init(void)
 
 static void __init omap3_beagle_init(void)
 {
+     omap3_beagle_i2c_init();
      platform_add_devices(omap3_beagle_devices,
 ARRAY_SIZE(omap3_beagle_devices));
      omap_board_config = omap3_beagle_config;
      omap_board_config_size = ARRAY_SIZE(omap3_beagle_config);

arch/arm/mach-omap2/board-omap3beagle.c static void __init omap3_beagle_init(voi · line 265

Quoted 8 lines
arch/arm/mach-omap2/board-omap3beagle.c +0 -2
@@ -255,8 +260,6 @@ static void __init omap3_beagle_init(voi
      omap3beagle_flash_init();
 }
 
-arch_initcall(omap3_beagle_i2c_init);
-
 static void __init omap3_beagle_map_io(void)
 {
      omap2_set_globals_343x();

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#6 lore raw
In reply to David Brownell
Quoted 3 lines · David Brownell

(Unrelated: make omap3_beagle_i2c_init be part of beagle_init. It shouldn't be called on non-beagle hardware; and beagle_init already runs with other arch_initcall code.)

Good catch, I will do the same on the Overo board file.

Steve