[PATCH] extend physmap.c to support run-time adding partitions

[PATCH] extend physmap.c to support run-time adding partitions

[Jun Sun]

[-- Attachment #1: Type: text/plain, Size: 707 bytes --]


While looking at drvers/mtd/maps directories, it becomes
obvious to me that many files are derived from physmap.c,
in order to add partition support or add their own
set_vpp() pointer, etc..

Such a need can be easily satisfied by extending physmap.c
a little.

In this patch, I like to introduce phsmap_add_partition()
function, which allows boards to add flash partitions
at run-time.  (Note that you can still override this
hardcoded partition by using kernel command line)

In the patch I also showed how a board can make use of this
extension.  Hopefully we can get rid of lots of files under 
the maps directory. :)

Boards which need to set set_vpp pointer can do so easily
too.

Any comments?

Jun

[-- Attachment #2: mtd-physmap-support-partitions.patch --]
[-- Type: text/plain, Size: 5231 bytes --]

diff -Nru linux/arch/mips/mips-boards/malta/malta_setup.c.orig linux/arch/mips/mips-boards/malta/malta_setup.c
--- linux/arch/mips/mips-boards/malta/malta_setup.c.orig	Fri Aug  1 10:50:18 2003
+++ linux/arch/mips/mips-boards/malta/malta_setup.c	Wed Oct 22 17:26:24 2003
@@ -24,6 +24,7 @@
 #ifdef CONFIG_BLK_DEV_IDE
 #include <linux/ide.h>
 #endif
+#include <linux/mtd/physmap.h>
 
 #include <asm/cpu.h>
 #include <asm/bootinfo.h>
@@ -166,6 +167,12 @@
         conswitchp = &dummy_con;
 #endif
 #endif
+
+#if defined(CONFIG_MTD_PHYSMAP) && defined(CONFIG_MTD_PARTITIONS)
+	physmap_add_partition("YAMON", 0x100000, 0x0, MTD_WRITEABLE);
+	physmap_add_partition("User FS", 0x300000, 0x100000, 0);
+#endif
+
 	mips_reboot_setup();
 
 	board_time_init = mips_time_init;
diff -Nru linux/drivers/mtd/maps/physmap.c.orig linux/drivers/mtd/maps/physmap.c
--- linux/drivers/mtd/maps/physmap.c.orig	Wed Jul 23 15:17:52 2003
+++ linux/drivers/mtd/maps/physmap.c	Wed Oct 22 17:42:50 2003
@@ -2,6 +2,11 @@
  * $Id: physmap.c,v 1.28 2003/05/28 15:53:43 dwmw2 Exp $
  *
  * Normal mappings of chips in physical memory
+ *
+ * Copyright (C) 2003 MontaVista Software Inc.
+ * Author: Jun Sun, jsun@mvista.com or jsun@junsun.net
+ *
+ * 031022 - [jsun] add physmap_add_partition()
  */
 
 #include <linux/module.h>
@@ -23,7 +28,7 @@
 
 
 struct map_info physmap_map = {
-	.name = "Physically mapped flash",
+	.name = "phys_mapped_flash",
 	.size = WINDOW_SIZE,
 	.buswidth = BUSWIDTH,
 	.phys = WINDOW_ADDR,
@@ -33,35 +38,31 @@
 static struct mtd_partition *mtd_parts;
 static int                   mtd_parts_nb;
 
-static struct mtd_partition physmap_partitions[] = {
-#if 0
-/* Put your own partition definitions here */
-	{
-		.name =		"bootROM",
-		.size =		0x80000,
-		.offset =	0,
-		.mask_flags =	MTD_WRITEABLE,  /* force read-only */
-	}, {
-		.name =		"zImage",
-		.size =		0x100000,
-		.offset =	MTDPART_OFS_APPEND,
-		.mask_flags =	MTD_WRITEABLE,  /* force read-only */
-	}, {
-		.name =		"ramdisk.gz",
-		.size =		0x300000,
-		.offset =	MTDPART_OFS_APPEND,
-		.mask_flags =	MTD_WRITEABLE,  /* force read-only */
-	}, {
-		.name =		"User FS",
-		.size =		MTDPART_SIZ_FULL,
-		.offset =	MTDPART_OFS_APPEND,
+#define NUM_PARTITIONS	4	/* random num which should be good enough */
+static struct mtd_partition physmap_partitions[NUM_PARTITIONS] __initdata = {};
+
+char *part_probes[] __initdata = {"cmdlinepart", "RedBoot", NULL};
+
+/*
+ * See include/linux/mtd/physmap.h file.
+ */
+static int part_index __initdata = 0;
+int __init physmap_add_partition(char *name, u32 size, u32 offset, u32 flags)
+{
+	if (part_index >= NUM_PARTITIONS) {
+		printk(KERN_ERR "physmap_add_partition() exceeds max partition table size (%d)\n", NUM_PARTITIONS);
+		return -1;
 	}
-#endif
-};
 
-#define NUM_PARTITIONS	(sizeof(physmap_partitions)/sizeof(struct mtd_partition))
-const char *part_probes[] = {"cmdlinepart", "RedBoot", NULL};
+	physmap_partitions[part_index].name = name;
+	physmap_partitions[part_index].size = size;
+	physmap_partitions[part_index].offset = offset;
+	physmap_partitions[part_index].mask_flags = flags;
+
+	part_index++;
 
+	return 0;
+}
 #endif /* CONFIG_MTD_PARTITIONS */
 
 int __init init_physmap(void)
@@ -97,11 +98,11 @@
 			return 0;
 		}
 
-		if (NUM_PARTITIONS != 0) 
+		if (part_index != 0) 
 		{
 			printk(KERN_NOTICE 
 			       "Using physmap partition definition\n");
-			add_mtd_partitions (mymtd, physmap_partitions, NUM_PARTITIONS);
+			add_mtd_partitions (mymtd, physmap_partitions, part_index);
 			return 0;
 		}
 
diff -Nru linux/include/linux/mtd/physmap.h.orig linux/include/linux/mtd/physmap.h
--- linux/include/linux/mtd/physmap.h.orig	Wed Oct 22 17:05:22 2003
+++ linux/include/linux/mtd/physmap.h	Wed Oct 22 18:20:34 2003
@@ -0,0 +1,48 @@
+/*
+ * For boards with physically mapped flash and using 
+ * drivers/mtd/maps/physmap.c mapping driver.
+ *
+ * Copyright (C) 2003 MontaVista Software Inc.
+ * Author: Jun Sun, jsun@mvista.com or jsun@junsun.net
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ */
+
+#ifndef __LINUX_MTD_PHYSMAP__
+
+#include <linux/config.h>
+
+#if defined(CONFIG_MTD_PHYSMAP) 
+
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/map.h>
+
+/*
+ * The map_info for physmap.  Board can override size, buswidth, phys,
+ * (*set_vpp)(), etc in their initial setup routine.
+ */
+extern struct map_info physmap_map;
+
+#if defined(CONFIG_MTD_PARTITIONS)
+
+/*
+ * Machines that wish to do flash partition may want to call this function in 
+ * their setup routine.  Examples:
+ *
+ *	physmap_add_partition("bootROM", 0x100000, 0, MTD_WRITEABLE);
+ *	physmap_add_partition("User FS", 0x300000, 0x100000, 0)
+ *
+ * Note that one can always override this hard-coded partition with 
+ * command line partition (you need to enable CONFIG_MTD_CMDLINE_PARTS).
+ */
+int physmap_add_partition(char *name, u32 size, u32 offset, u32 mask_flags);
+
+#endif /* defined(CONFIG_MTD_PARTITIONS) */
+#endif /* defined(CONFIG_MTD) */
+
+#endif /* __LINUX_MTD_PHYSMAP__ */
+

Re: [PATCH] extend physmap.c to support run-time adding partitions

[Jun Sun]

On Thu, Oct 23, 2003 at 05:33:07PM +0200, Jörn Engel wrote:
> On Wed, 22 October 2003 18:25:58 -0700, Jun Sun wrote:
> > 
> > While looking at drvers/mtd/maps directories, it becomes
> > obvious to me that many files are derived from physmap.c,
> > in order to add partition support or add their own
> > set_vpp() pointer, etc..
> > 
> > Such a need can be easily satisfied by extending physmap.c
> > a little.
> > 
> > In this patch, I like to introduce phsmap_add_partition()
> > function, which allows boards to add flash partitions
> > at run-time.  (Note that you can still override this
> > hardcoded partition by using kernel command line)
> > 
> > In the patch I also showed how a board can make use of this
> > extension.  Hopefully we can get rid of lots of files under 
> > the maps directory. :)
> 
> For most people, you saved source code size and wasted binary size.
> The very same people are far more concerned about binary size. :)
>

Ahh, note that I turned a couple of functions and data into
__init kind.  That should make space-conservative people happy.
 
> I've tried to clean this up before and failed, but maybe you can do a
> better job.  Good luck!
>

What was the main objection?  I can list quite a few benefits:

1. remove _lots_ of duplicated mapping files, which also means
   a cleaner CONFIG_xxx space and a simpler Makefile.
2. the board-specific partition setup is now done in arch/<xxx>/<board>/setup.c
   which means when a board is deleted, its flash support is automatically
   removed.  (Not so with current arrangement)
3. futher _common_ improvement is possible.  For example, for most MIPS boards
   we don't need ioremap to access the flash area - CPU has a fixed uncached
   mapping to physical space.  Now we can just modify one file and benefts
   about almost 20 boards.

Jun

Re: [PATCH] extend physmap.c to support run-time adding partitions

[Jörn Engel]

On Thu, 23 October 2003 10:03:04 -0700, Jun Sun wrote:
> On Thu, Oct 23, 2003 at 05:33:07PM +0200, Jörn Engel wrote:
> > On Wed, 22 October 2003 18:25:58 -0700, Jun Sun wrote:
> > > 
> > > While looking at drvers/mtd/maps directories, it becomes
> > > obvious to me that many files are derived from physmap.c,
> > > in order to add partition support or add their own
> > > set_vpp() pointer, etc..
> > > 
> > > Such a need can be easily satisfied by extending physmap.c
> > > a little.
> > > 
> > > In this patch, I like to introduce phsmap_add_partition()
> > > function, which allows boards to add flash partitions
> > > at run-time.  (Note that you can still override this
> > > hardcoded partition by using kernel command line)
> > > 
> > > In the patch I also showed how a board can make use of this
> > > extension.  Hopefully we can get rid of lots of files under 
> > > the maps directory. :)
> > 
> > For most people, you saved source code size and wasted binary size.
> > The very same people are far more concerned about binary size. :)
> >
> 
> Ahh, note that I turned a couple of functions and data into
> __init kind.  That should make space-conservative people happy.

__init still shows up in flash, though.

> > I've tried to clean this up before and failed, but maybe you can do a
> > better job.  Good luck!
> 
> What was the main objection?  I can list quite a few benefits:
> 
> 1. remove _lots_ of duplicated mapping files, which also means
>    a cleaner CONFIG_xxx space and a simpler Makefile.
> 2. the board-specific partition setup is now done in arch/<xxx>/<board>/setup.c
>    which means when a board is deleted, its flash support is automatically
>    removed.  (Not so with current arrangement)
> 3. futher _common_ improvement is possible.  For example, for most MIPS boards
>    we don't need ioremap to access the flash area - CPU has a fixed uncached
>    mapping to physical space.  Now we can just modify one file and benefts
>    about almost 20 boards.

o All those translate to improvements in the source code.  How about the
binary?  Compile with and without patch and post the kernel image
size.  And remember that noone will use two map files at the same time
in the real world.

o Copy and paste is simple.  So simple in fact, that everyone does it,
as you have observed.  Why make it more complicated, unless you have
clear advantages.


Yes, I like the basic idea, tried to do it myself.  But what's the use
if all your users care about binary size and that increases?

Jörn

-- 
"Translations are and will always be problematic. They inflict violence 
upon two languages." (translation from German)

Re: [PATCH] extend physmap.c to support run-time adding partitions

[Jun Sun]

On Thu, Oct 23, 2003 at 07:31:48PM +0200, Jörn Engel wrote:
> On Thu, 23 October 2003 10:03:04 -0700, Jun Sun wrote:
> > On Thu, Oct 23, 2003 at 05:33:07PM +0200, Jörn Engel wrote:
> > > On Wed, 22 October 2003 18:25:58 -0700, Jun Sun wrote:
> > > > 
> > > > While looking at drvers/mtd/maps directories, it becomes
> > > > obvious to me that many files are derived from physmap.c,
> > > > in order to add partition support or add their own
> > > > set_vpp() pointer, etc..
> > > > 
> > > > Such a need can be easily satisfied by extending physmap.c
> > > > a little.
> > > > 
> > > > In this patch, I like to introduce phsmap_add_partition()
> > > > function, which allows boards to add flash partitions
> > > > at run-time.  (Note that you can still override this
> > > > hardcoded partition by using kernel command line)
> > > > 
> > > > In the patch I also showed how a board can make use of this
> > > > extension.  Hopefully we can get rid of lots of files under 
> > > > the maps directory. :)
> > > 
> > > For most people, you saved source code size and wasted binary size.
> > > The very same people are far more concerned about binary size. :)
> > >
> > 
> > Ahh, note that I turned a couple of functions and data into
> > __init kind.  That should make space-conservative people happy.
> 
> __init still shows up in flash, though.
> 
> > > I've tried to clean this up before and failed, but maybe you can do a
> > > better job.  Good luck!
> > 
> > What was the main objection?  I can list quite a few benefits:
> > 
> > 1. remove _lots_ of duplicated mapping files, which also means
> >    a cleaner CONFIG_xxx space and a simpler Makefile.
> > 2. the board-specific partition setup is now done in arch/<xxx>/<board>/setup.c
> >    which means when a board is deleted, its flash support is automatically
> >    removed.  (Not so with current arrangement)
> > 3. futher _common_ improvement is possible.  For example, for most MIPS boards
> >    we don't need ioremap to access the flash area - CPU has a fixed uncached
> >    mapping to physical space.  Now we can just modify one file and benefts
> >    about almost 20 boards.
> 
> o All those translate to improvements in the source code.  How about the
> binary?  Compile with and without patch and post the kernel image
> size.  And remember that noone will use two map files at the same time
> in the real world.
> 
> o Copy and paste is simple.  So simple in fact, that everyone does it,
> as you have observed.  Why make it more complicated, unless you have
> clear advantages.
> 

... as if my previous listings are not advantages. :)

> 
> Yes, I like the basic idea, tried to do it myself.  But what's the use
> if all your users care about binary size and that increases?
>

I find it hard to belive this patch would increase kernel size.
Can someone using existing propriatary mapping driver apply this 
patch, switch to use physmap.c, and let us know the size increase?

How much increase would you start to really care in a typical .5M to 2M
kernel?  1K or 10K or 100K?  I think the increase should be minimum if any.

Actually I don't see any increase at all if the proprietary file is a strict copy
of physmap.c.

Jun

Re: [PATCH] extend physmap.c to support run-time adding partitions

[Jörn Engel]

On Thu, 23 October 2003 10:43:20 -0700, Jun Sun wrote:
> > 
> > o All those translate to improvements in the source code.  How about the
> > binary?  Compile with and without patch and post the kernel image
> > size.  And remember that noone will use two map files at the same time
> > in the real world.
> > 
> > o Copy and paste is simple.  So simple in fact, that everyone does it,
> > as you have observed.  Why make it more complicated, unless you have
> > clear advantages.
> 
> ... as if my previous listings are not advantages. :)

They are, no doubt.  But there are disadvantages as well.

> > Yes, I like the basic idea, tried to do it myself.  But what's the use
> > if all your users care about binary size and that increases?
> 
> I find it hard to belive this patch would increase kernel size.
> Can someone using existing propriatary mapping driver apply this 
> patch, switch to use physmap.c, and let us know the size increase?
> 
> How much increase would you start to really care in a typical .5M to 2M
> kernel?  1K or 10K or 100K?  I think the increase should be minimum if any.

I don't know and I don't care.  You want the patch in, you show the
numbers or convince David otherwise.

Jörn

-- 
All art is but imitation of nature.
-- Lucius Annaeus Seneca

Re: [PATCH] extend physmap.c to support run-time adding partitions

[Jun Sun]

On Thu, Oct 23, 2003 at 08:15:41PM +0200, Jörn Engel wrote:
> On Thu, 23 October 2003 10:43:20 -0700, Jun Sun wrote:
> > > 
> > > o All those translate to improvements in the source code.  How about the
> > > binary?  Compile with and without patch and post the kernel image
> > > size.  And remember that noone will use two map files at the same time
> > > in the real world.
> > > 
> > > o Copy and paste is simple.  So simple in fact, that everyone does it,
> > > as you have observed.  Why make it more complicated, unless you have
> > > clear advantages.
> > 
> > ... as if my previous listings are not advantages. :)
> 
> They are, no doubt.  But there are disadvantages as well.
> 
> > > Yes, I like the basic idea, tried to do it myself.  But what's the use
> > > if all your users care about binary size and that increases?
> > 
> > I find it hard to belive this patch would increase kernel size.
> > Can someone using existing propriatary mapping driver apply this 
> > patch, switch to use physmap.c, and let us know the size increase?
> > 
> > How much increase would you start to really care in a typical .5M to 2M
> > kernel?  1K or 10K or 100K?  I think the increase should be minimum if any.
> 
> I don't know and I don't care.  You want the patch in, you show the
> numbers or convince David otherwise.
>

I will do some numbers, but I don't really buy your logic.  I said
"This patch is great" and you are one who said it increases the size.
It seems to me you need to prove your claim. :)

Jun

Re: [PATCH] extend physmap.c to support run-time adding partitions

[Jun Sun]

On Thu, Oct 23, 2003 at 01:04:06PM -0700, Jun Sun wrote:
> On Thu, Oct 23, 2003 at 08:15:41PM +0200, Jörn Engel wrote:
> > On Thu, 23 October 2003 10:43:20 -0700, Jun Sun wrote:
> > > > 
> > > > o All those translate to improvements in the source code.  How about the
> > > > binary?  Compile with and without patch and post the kernel image
> > > > size.  And remember that noone will use two map files at the same time
> > > > in the real world.
> > > > 
> > > > o Copy and paste is simple.  So simple in fact, that everyone does it,
> > > > as you have observed.  Why make it more complicated, unless you have
> > > > clear advantages.
> > > 
> > > ... as if my previous listings are not advantages. :)
> > 
> > They are, no doubt.  But there are disadvantages as well.
> > 
> > > > Yes, I like the basic idea, tried to do it myself.  But what's the use
> > > > if all your users care about binary size and that increases?
> > > 
> > > I find it hard to belive this patch would increase kernel size.
> > > Can someone using existing propriatary mapping driver apply this 
> > > patch, switch to use physmap.c, and let us know the size increase?
> > > 
> > > How much increase would you start to really care in a typical .5M to 2M
> > > kernel?  1K or 10K or 100K?  I think the increase should be minimum if any.
> > 
> > I don't know and I don't care.  You want the patch in, you show the
> > numbers or convince David otherwise.
> >
> 
> I will do some numbers, 
<snip>

OK, I converted jmr3927 flash driver to use physmap and kernel size increases
by whopping 96 bytes!  See the ELF header dump below. :)

Meanwhile, we sadly announce the loss of drivers/mtd/maps/jmr3927-flash.c
file and the death of CONFIG_MTD_JMR3927 (not in MTD tree yet).  :0


Jun

-------------------------------------------

original vmlinux with no modification:

vmlinux.orig:     file format elf32-tradlittlemips

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 .text         001aa080  80050000  80050000  00001000  2**5
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  1 .fixup        00001250  801fa080  801fa080  001ab080  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  2 .kstrtab      00005000  801fb2d0  801fb2d0  001ac2d0  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  3 __ex_table    000016d8  802002d0  802002d0  001b12d0  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  4 __dbe_table   00000000  802019a8  802019a8  001b29a8  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  5 __ksymtab     000025a8  802019a8  802019a8  001b29a8  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  6 .data.init_task 00002000  80204000  80204000  001b5000  2**3
                  CONTENTS, ALLOC, LOAD, DATA
  7 .text.init    0001757c  80206000  80206000  001b7000  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  8 .data.init    00005070  8021d57c  8021d57c  001ce57c  2**2
                  CONTENTS, ALLOC, LOAD, DATA
  9 .setup.init   000000e8  802225f0  802225f0  001d35f0  2**2
                  CONTENTS, ALLOC, LOAD, DATA
 10 .initcall.init 000000ac  802226d8  802226d8  001d36d8  2**2
                  CONTENTS, ALLOC, LOAD, DATA
 11 .data.cacheline_aligned 00001470  80223000  80223000  001d4000  2**4
                  CONTENTS, ALLOC, LOAD, DATA
 12 .reginfo      00000018  80224470  80224470  001d5470  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA, LINK_ONCE_SAME_SIZE
 13 .data         0001a000  80225000  80225000  001d6000  2**12
                  CONTENTS, ALLOC, LOAD, DATA
 14 .bss          0002ce90  8023f000  8023f000  001f0000  2**4
                  ALLOC
 15 .comment      00002cce  8026be90  8026be90  001f0000  2**0
                  CONTENTS, READONLY
 16 .pdr          00027ae0  00000000  00000000  001f2cd0  2**2
                  CONTENTS, READONLY
 17 .mdebug.abi32 00000000  00000000  00000000  0021a7b0  2**0
                  CONTENTS, READONLY
 
-------------------------------------------

new vmlinux with patching:

vmlinux:     file format elf32-tradlittlemips

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 .text         001aa000  80050000  80050000  00001000  2**5
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  1 .fixup        00001250  801fa000  801fa000  001ab000  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  2 .kstrtab      00005000  801fb250  801fb250  001ac250  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  3 __ex_table    000016d8  80200250  80200250  001b1250  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  4 __dbe_table   00000000  80201928  80201928  001b2928  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  5 __ksymtab     000025a8  80201928  80201928  001b2928  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  6 .data.init_task 00002000  80204000  80204000  001b5000  2**3
                  CONTENTS, ALLOC, LOAD, DATA
  7 .text.init    0001763c  80206000  80206000  001b7000  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  8 .data.init    000050e0  8021d63c  8021d63c  001ce63c  2**2
                  CONTENTS, ALLOC, LOAD, DATA
  9 .setup.init   000000e8  80222720  80222720  001d3720  2**2
                  CONTENTS, ALLOC, LOAD, DATA
 10 .initcall.init 000000ac  80222808  80222808  001d3808  2**2
                  CONTENTS, ALLOC, LOAD, DATA
 11 .data.cacheline_aligned 00001470  80223000  80223000  001d4000  2**4
                  CONTENTS, ALLOC, LOAD, DATA
 12 .reginfo      00000018  80224470  80224470  001d5470  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA, LINK_ONCE_SAME_SIZE
 13 .data         0001a000  80225000  80225000  001d6000  2**12
                  CONTENTS, ALLOC, LOAD, DATA
 14 .bss          0002ce40  8023f000  8023f000  001f0000  2**4
                  ALLOC
 15 .comment      00002cce  8026be40  8026be40  001f0000  2**0
                  CONTENTS, READONLY
 16 .pdr          00027a00  00000000  00000000  001f2cd0  2**2
                  CONTENTS, READONLY
 17 .mdebug.abi32 00000000  00000000  00000000  0021a6d0  2**0
                  CONTENTS, READONLY

Re: [PATCH] extend physmap.c to support run-time adding partitions

[Jun Sun]

On Wed, Oct 22, 2003 at 06:25:58PM -0700, Jun Sun wrote:
> 
> While looking at drvers/mtd/maps directories, it becomes
> obvious to me that many files are derived from physmap.c,
> in order to add partition support or add their own
> set_vpp() pointer, etc..
> 
> Such a need can be easily satisfied by extending physmap.c
> a little.
> 
> In this patch, I like to introduce phsmap_add_partition()
> function, which allows boards to add flash partitions
> at run-time.  (Note that you can still override this
> hardcoded partition by using kernel command line)
> 
> In the patch I also showed how a board can make use of this
> extension.  Hopefully we can get rid of lots of files under 
> the maps directory. :)
> 
> Boards which need to set set_vpp pointer can do so easily
> too.
> 
> Any comments?
>

Any objections or comments?  David?

Jun
 

> diff -Nru linux/arch/mips/mips-boards/malta/malta_setup.c.orig linux/arch/mips/mips-boards/malta/malta_setup.c
> --- linux/arch/mips/mips-boards/malta/malta_setup.c.orig	Fri Aug  1 10:50:18 2003
> +++ linux/arch/mips/mips-boards/malta/malta_setup.c	Wed Oct 22 17:26:24 2003
> @@ -24,6 +24,7 @@
>  #ifdef CONFIG_BLK_DEV_IDE
>  #include <linux/ide.h>
>  #endif
> +#include <linux/mtd/physmap.h>
>  
>  #include <asm/cpu.h>
>  #include <asm/bootinfo.h>
> @@ -166,6 +167,12 @@
>          conswitchp = &dummy_con;
>  #endif
>  #endif
> +
> +#if defined(CONFIG_MTD_PHYSMAP) && defined(CONFIG_MTD_PARTITIONS)
> +	physmap_add_partition("YAMON", 0x100000, 0x0, MTD_WRITEABLE);
> +	physmap_add_partition("User FS", 0x300000, 0x100000, 0);
> +#endif
> +
>  	mips_reboot_setup();
>  
>  	board_time_init = mips_time_init;
> diff -Nru linux/drivers/mtd/maps/physmap.c.orig linux/drivers/mtd/maps/physmap.c
> --- linux/drivers/mtd/maps/physmap.c.orig	Wed Jul 23 15:17:52 2003
> +++ linux/drivers/mtd/maps/physmap.c	Wed Oct 22 17:42:50 2003
> @@ -2,6 +2,11 @@
>   * $Id: physmap.c,v 1.28 2003/05/28 15:53:43 dwmw2 Exp $
>   *
>   * Normal mappings of chips in physical memory
> + *
> + * Copyright (C) 2003 MontaVista Software Inc.
> + * Author: Jun Sun, jsun@mvista.com or jsun@junsun.net
> + *
> + * 031022 - [jsun] add physmap_add_partition()
>   */
>  
>  #include <linux/module.h>
> @@ -23,7 +28,7 @@
>  
>  
>  struct map_info physmap_map = {
> -	.name = "Physically mapped flash",
> +	.name = "phys_mapped_flash",
>  	.size = WINDOW_SIZE,
>  	.buswidth = BUSWIDTH,
>  	.phys = WINDOW_ADDR,
> @@ -33,35 +38,31 @@
>  static struct mtd_partition *mtd_parts;
>  static int                   mtd_parts_nb;
>  
> -static struct mtd_partition physmap_partitions[] = {
> -#if 0
> -/* Put your own partition definitions here */
> -	{
> -		.name =		"bootROM",
> -		.size =		0x80000,
> -		.offset =	0,
> -		.mask_flags =	MTD_WRITEABLE,  /* force read-only */
> -	}, {
> -		.name =		"zImage",
> -		.size =		0x100000,
> -		.offset =	MTDPART_OFS_APPEND,
> -		.mask_flags =	MTD_WRITEABLE,  /* force read-only */
> -	}, {
> -		.name =		"ramdisk.gz",
> -		.size =		0x300000,
> -		.offset =	MTDPART_OFS_APPEND,
> -		.mask_flags =	MTD_WRITEABLE,  /* force read-only */
> -	}, {
> -		.name =		"User FS",
> -		.size =		MTDPART_SIZ_FULL,
> -		.offset =	MTDPART_OFS_APPEND,
> +#define NUM_PARTITIONS	4	/* random num which should be good enough */
> +static struct mtd_partition physmap_partitions[NUM_PARTITIONS] __initdata = {};
> +
> +char *part_probes[] __initdata = {"cmdlinepart", "RedBoot", NULL};
> +
> +/*
> + * See include/linux/mtd/physmap.h file.
> + */
> +static int part_index __initdata = 0;
> +int __init physmap_add_partition(char *name, u32 size, u32 offset, u32 flags)
> +{
> +	if (part_index >= NUM_PARTITIONS) {
> +		printk(KERN_ERR "physmap_add_partition() exceeds max partition table size (%d)\n", NUM_PARTITIONS);
> +		return -1;
>  	}
> -#endif
> -};
>  
> -#define NUM_PARTITIONS	(sizeof(physmap_partitions)/sizeof(struct mtd_partition))
> -const char *part_probes[] = {"cmdlinepart", "RedBoot", NULL};
> +	physmap_partitions[part_index].name = name;
> +	physmap_partitions[part_index].size = size;
> +	physmap_partitions[part_index].offset = offset;
> +	physmap_partitions[part_index].mask_flags = flags;
> +
> +	part_index++;
>  
> +	return 0;
> +}
>  #endif /* CONFIG_MTD_PARTITIONS */
>  
>  int __init init_physmap(void)
> @@ -97,11 +98,11 @@
>  			return 0;
>  		}
>  
> -		if (NUM_PARTITIONS != 0) 
> +		if (part_index != 0) 
>  		{
>  			printk(KERN_NOTICE 
>  			       "Using physmap partition definition\n");
> -			add_mtd_partitions (mymtd, physmap_partitions, NUM_PARTITIONS);
> +			add_mtd_partitions (mymtd, physmap_partitions, part_index);
>  			return 0;
>  		}
>  
> diff -Nru linux/include/linux/mtd/physmap.h.orig linux/include/linux/mtd/physmap.h
> --- linux/include/linux/mtd/physmap.h.orig	Wed Oct 22 17:05:22 2003
> +++ linux/include/linux/mtd/physmap.h	Wed Oct 22 18:20:34 2003
> @@ -0,0 +1,48 @@
> +/*
> + * For boards with physically mapped flash and using 
> + * drivers/mtd/maps/physmap.c mapping driver.
> + *
> + * Copyright (C) 2003 MontaVista Software Inc.
> + * Author: Jun Sun, jsun@mvista.com or jsun@junsun.net
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + *
> + */
> +
> +#ifndef __LINUX_MTD_PHYSMAP__
> +
> +#include <linux/config.h>
> +
> +#if defined(CONFIG_MTD_PHYSMAP) 
> +
> +#include <linux/mtd/mtd.h>
> +#include <linux/mtd/map.h>
> +
> +/*
> + * The map_info for physmap.  Board can override size, buswidth, phys,
> + * (*set_vpp)(), etc in their initial setup routine.
> + */
> +extern struct map_info physmap_map;
> +
> +#if defined(CONFIG_MTD_PARTITIONS)
> +
> +/*
> + * Machines that wish to do flash partition may want to call this function in 
> + * their setup routine.  Examples:
> + *
> + *	physmap_add_partition("bootROM", 0x100000, 0, MTD_WRITEABLE);
> + *	physmap_add_partition("User FS", 0x300000, 0x100000, 0)
> + *
> + * Note that one can always override this hard-coded partition with 
> + * command line partition (you need to enable CONFIG_MTD_CMDLINE_PARTS).
> + */
> +int physmap_add_partition(char *name, u32 size, u32 offset, u32 mask_flags);
> +
> +#endif /* defined(CONFIG_MTD_PARTITIONS) */
> +#endif /* defined(CONFIG_MTD) */
> +
> +#endif /* __LINUX_MTD_PHYSMAP__ */
> +

Re: [PATCH] extend physmap.c to support run-time adding partitions

[Holger Schurig]

> Meanwhile, we sadly announce the loss of drivers/mtd/maps/jmr3927-flash.c
> file and the death of CONFIG_MTD_JMR3927 (not in MTD tree yet).  :0

First: I like a unified approach. I think that, when it makes sense, board
specifica should be in board specific files at one place. This doesn't
prevent copy&past, e.g. I can copy arch/arm/mach-pxa/accelent.c and paste
stuff into arch/arm/mach-pxa/ramses.c. It's actually easier now, because
there is only one place to look for board specifics.

For example, on linux-2.6 people are trying to have an almost #ifdef-less
drivers/video/pxafb.c and have all in arm/arm/mach-pxa/*.c files.


But just one note: you don't need CONFIG_MTD_JMR3927 if JMR3927 is your
board. At least not in arm/xscale kernels (not sure about other platforms).
You already have CONFIG_ARCH_<board>, e.g. I have CONFIG_ARCH_RAMSES. And I
can re-use this config var in drivers/mtd/maps.

-- 
Try Linux 2.6 from BitKeeper for PXA2x0 CPUs at
http://www.mn-logistik.de/unsupported/linux-2.6/

Re: [PATCH] extend physmap.c to support run-time adding partitions

[David Woodhouse]

On Mon, 2003-10-27 at 10:17 -0800, Jun Sun wrote:
> Any objections or comments?  David?

Sorry for delayed response. I'm not really sure about this. Take a look
at drivers/mtd/maps/rpxlite.c -- that's what we're able to eliminate,
and we could even stick half of _that_ into a library function. 

I'm all for cleaning up all the duplication in the map drivers, but I'm
not sure we need to do it by making users enter correct numbers for
CONFIG_MTD_PHYSMAP_*...

-- 
dwmw2

Re: [PATCH] extend physmap.c to support run-time adding partitions

[Jun Sun]

[-- Attachment #1: Type: text/plain, Size: 1354 bytes --]

On Tue, Oct 28, 2003 at 10:50:15AM +0000, David Woodhouse wrote:
> On Mon, 2003-10-27 at 10:17 -0800, Jun Sun wrote:
> > Any objections or comments?  David?
> 
> Sorry for delayed response. I'm not really sure about this. Take a look
> at drivers/mtd/maps/rpxlite.c -- that's what we're able to eliminate,

I think a lot of more can be eliminated, including db1x00-flash.c
which I did take a look.

What concerns me more is that I have at least four more new boards
that cry for this extension.  Without it, I would have to introduce
more junk files into linux-mtd.

We need to have this extension or something like this in place.

> and we could even stick half of _that_ into a library function. 
> 

I don't quite follow here.

> I'm all for cleaning up all the duplication in the map drivers, but I'm
> not sure we need to do it by making users enter correct numbers for
> CONFIG_MTD_PHYSMAP_*...
> 

I didn't like that aspect either.  I thought I should keep the
patch minimum so that it is easier for people to swallow. :)

Actually the new attached version is probably what I like more.
It gets rid of the config options for physmap.

Since physmap_map is external, one can easily setup the mapping
at the run time instead of configuration time.  The new patch
simply adds some syntactic sugar by introducing an inline function
to do this.  

Jun

[-- Attachment #2: patch2 --]
[-- Type: text/plain, Size: 7445 bytes --]

diff -Nru linux/arch/mips/mips-boards/malta/malta_setup.c.orig linux/arch/mips/mips-boards/malta/malta_setup.c
--- linux/arch/mips/mips-boards/malta/malta_setup.c.orig	Fri Aug  1 10:50:18 2003
+++ linux/arch/mips/mips-boards/malta/malta_setup.c	Tue Oct 28 17:02:01 2003
@@ -24,6 +24,7 @@
 #ifdef CONFIG_BLK_DEV_IDE
 #include <linux/ide.h>
 #endif
+#include <linux/mtd/physmap.h>
 
 #include <asm/cpu.h>
 #include <asm/bootinfo.h>
@@ -166,6 +167,15 @@
         conswitchp = &dummy_con;
 #endif
 #endif
+
+#if defined(CONFIG_MTD)
+	/* we use generic physmap mapping driver */
+	physmap_set_map(0x400000, 4, 0x1e000000);
+
+	physmap_add_partition("YAMON", 0x100000, 0x0, MTD_WRITEABLE);
+	physmap_add_partition("User FS", 0x300000, 0x100000, 0);
+#endif
+
 	mips_reboot_setup();
 
 	board_time_init = mips_time_init;
diff -Nru linux/drivers/mtd/maps/physmap.c.orig linux/drivers/mtd/maps/physmap.c
--- linux/drivers/mtd/maps/physmap.c.orig	Wed Jul 23 15:17:52 2003
+++ linux/drivers/mtd/maps/physmap.c	Tue Oct 28 17:05:12 2003
@@ -2,6 +2,11 @@
  * $Id: physmap.c,v 1.28 2003/05/28 15:53:43 dwmw2 Exp $
  *
  * Normal mappings of chips in physical memory
+ *
+ * Copyright (C) 2003 MontaVista Software Inc.
+ * Author: Jun Sun, jsun@mvista.com or jsun@junsun.net
+ *
+ * 031022 - [jsun] add physmap_add_partition()
  */
 
 #include <linux/module.h>
@@ -15,53 +20,39 @@
 #include <linux/config.h>
 #include <linux/mtd/partitions.h>
 
-#define WINDOW_ADDR CONFIG_MTD_PHYSMAP_START
-#define WINDOW_SIZE CONFIG_MTD_PHYSMAP_LEN
-#define BUSWIDTH CONFIG_MTD_PHYSMAP_BUSWIDTH
-
 static struct mtd_info *mymtd;
 
-
-struct map_info physmap_map = {
-	.name = "Physically mapped flash",
-	.size = WINDOW_SIZE,
-	.buswidth = BUSWIDTH,
-	.phys = WINDOW_ADDR,
-};
+struct map_info physmap_map = {.name = "phys_mapped_flash"};
 
 #ifdef CONFIG_MTD_PARTITIONS
 static struct mtd_partition *mtd_parts;
 static int                   mtd_parts_nb;
 
-static struct mtd_partition physmap_partitions[] = {
-#if 0
-/* Put your own partition definitions here */
-	{
-		.name =		"bootROM",
-		.size =		0x80000,
-		.offset =	0,
-		.mask_flags =	MTD_WRITEABLE,  /* force read-only */
-	}, {
-		.name =		"zImage",
-		.size =		0x100000,
-		.offset =	MTDPART_OFS_APPEND,
-		.mask_flags =	MTD_WRITEABLE,  /* force read-only */
-	}, {
-		.name =		"ramdisk.gz",
-		.size =		0x300000,
-		.offset =	MTDPART_OFS_APPEND,
-		.mask_flags =	MTD_WRITEABLE,  /* force read-only */
-	}, {
-		.name =		"User FS",
-		.size =		MTDPART_SIZ_FULL,
-		.offset =	MTDPART_OFS_APPEND,
+#define NUM_PARTITIONS	4	/* random num which should be good enough */
+static struct mtd_partition physmap_partitions[NUM_PARTITIONS] __initdata = {};
+
+char *part_probes[] __initdata = {"cmdlinepart", "RedBoot", NULL};
+
+/*
+ * See include/linux/mtd/physmap.h file.
+ */
+static int part_index __initdata = 0;
+int __init physmap_add_partition(char *name, u32 size, u32 offset, u32 flags)
+{
+	if (part_index >= NUM_PARTITIONS) {
+		printk(KERN_ERR "physmap_add_partition() exceeds max partition table size (%d)\n", NUM_PARTITIONS);
+		return -1;
 	}
-#endif
-};
 
-#define NUM_PARTITIONS	(sizeof(physmap_partitions)/sizeof(struct mtd_partition))
-const char *part_probes[] = {"cmdlinepart", "RedBoot", NULL};
+	physmap_partitions[part_index].name = name;
+	physmap_partitions[part_index].size = size;
+	physmap_partitions[part_index].offset = offset;
+	physmap_partitions[part_index].mask_flags = flags;
+
+	part_index++;
 
+	return 0;
+}
 #endif /* CONFIG_MTD_PARTITIONS */
 
 int __init init_physmap(void)
@@ -69,8 +60,8 @@
 	static const char *rom_probe_types[] = { "cfi_probe", "jedec_probe", "map_rom", 0 };
 	const char **type;
 
-       	printk(KERN_NOTICE "physmap flash device: %x at %x\n", WINDOW_SIZE, WINDOW_ADDR);
-	physmap_map.virt = (unsigned long)ioremap(WINDOW_ADDR, WINDOW_SIZE);
+       	printk(KERN_NOTICE "physmap flash device: %lx at %lx\n", physmap_map.size, physmap_map.phys);
+	physmap_map.virt = (unsigned long)ioremap(physmap_map.phys, physmap_map.size);
 
 	if (!physmap_map.virt) {
 		printk("Failed to ioremap\n");
@@ -97,11 +88,11 @@
 			return 0;
 		}
 
-		if (NUM_PARTITIONS != 0) 
+		if (part_index != 0) 
 		{
 			printk(KERN_NOTICE 
 			       "Using physmap partition definition\n");
-			add_mtd_partitions (mymtd, physmap_partitions, NUM_PARTITIONS);
+			add_mtd_partitions (mymtd, physmap_partitions, part_index);
 			return 0;
 		}
 
diff -Nru linux/drivers/mtd/maps/Config.in.orig linux/drivers/mtd/maps/Config.in
--- linux/drivers/mtd/maps/Config.in.orig	Tue Oct 21 18:07:35 2003
+++ linux/drivers/mtd/maps/Config.in	Tue Oct 28 16:59:07 2003
@@ -8,12 +8,7 @@
 
 bool '  Support for non-linear mappings of flash chips' CONFIG_MTD_COMPLEX_MAPPINGS
 
-dep_tristate '  CFI Flash device in physical memory map' CONFIG_MTD_PHYSMAP $CONFIG_MTD_GEN_PROBE
-if [ "$CONFIG_MTD_PHYSMAP" = "y" -o "$CONFIG_MTD_PHYSMAP" = "m" ]; then
-   hex '    Physical start address of flash mapping' CONFIG_MTD_PHYSMAP_START 0x8000000
-   hex '    Physical length of flash mapping' CONFIG_MTD_PHYSMAP_LEN 0x4000000
-   int '    Bus width in octets' CONFIG_MTD_PHYSMAP_BUSWIDTH 2
-fi
+bool '  CFI Flash device in physical memory map' CONFIG_MTD_PHYSMAP $CONFIG_MTD_GEN_PROBE
 
 if [ "$CONFIG_SPARC" = "y" -o "$CONFIG_SPARC64" = "y" ]; then
    dep_tristate '  Sun Microsystems userflash support' CONFIG_MTD_SUN_UFLASH $CONFIG_MTD_CFI
diff -Nru linux/drivers/mtd/maps/Makefile.orig linux/drivers/mtd/maps/Makefile
diff -Nru linux/include/linux/mtd/physmap.h.orig linux/include/linux/mtd/physmap.h
--- linux/include/linux/mtd/physmap.h.orig	Wed Oct 22 17:05:22 2003
+++ linux/include/linux/mtd/physmap.h	Tue Oct 28 17:11:29 2003
@@ -0,0 +1,58 @@
+/*
+ * For boards with physically mapped flash and using 
+ * drivers/mtd/maps/physmap.c mapping driver.
+ *
+ * Copyright (C) 2003 MontaVista Software Inc.
+ * Author: Jun Sun, jsun@mvista.com or jsun@junsun.net
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ *
+ */
+
+#ifndef __LINUX_MTD_PHYSMAP__
+
+#include <linux/config.h>
+
+#if defined(CONFIG_MTD_PHYSMAP) 
+
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/map.h>
+
+/*
+ * The map_info for physmap.  Board can override size, buswidth, phys,
+ * (*set_vpp)(), etc in their initial setup routine.
+ */
+extern struct map_info physmap_map;
+
+/*
+ * Board needs to specify the exact mapping during their setup time.
+ */
+static inline void physmap_set_map(unsigned long size, int buswidth, unsigned long phys_addr)
+{
+	physmap_map.size = size;
+	physmap_map.buswidth = buswidth;
+	physmap_map.phys = phys_addr;
+}
+
+#if defined(CONFIG_MTD_PARTITIONS)
+
+/*
+ * Machines that wish to do flash partition may want to call this function in 
+ * their setup routine.  Examples:
+ *
+ *	physmap_add_partition("bootROM", 0x100000, 0, MTD_WRITEABLE);	// RO
+ *	physmap_add_partition("User FS", 0x300000, 0x100000, 0); 	// RW
+ *
+ * Note that one can always override this hard-coded partition with 
+ * command line partition (you need to enable CONFIG_MTD_CMDLINE_PARTS).
+ */
+int physmap_add_partition(char *name, u32 size, u32 offset, u32 mask_flags);
+
+#endif /* defined(CONFIG_MTD_PARTITIONS) */
+#endif /* defined(CONFIG_MTD) */
+
+#endif /* __LINUX_MTD_PHYSMAP__ */
+

Re: [PATCH] extend physmap.c to support run-time adding partitions

[Jun Sun]

On Tue, Oct 28, 2003 at 09:22:36AM +0100, Holger Schurig wrote:
> > Meanwhile, we sadly announce the loss of drivers/mtd/maps/jmr3927-flash.c
> > file and the death of CONFIG_MTD_JMR3927 (not in MTD tree yet).  :0
> 
> First: I like a unified approach. I think that, when it makes sense, board
> specifica should be in board specific files at one place. This doesn't
> prevent copy&past, e.g. I can copy arch/arm/mach-pxa/accelent.c and paste
> stuff into arch/arm/mach-pxa/ramses.c. It's actually easier now, because
> there is only one place to look for board specifics.
> 

Exactly!  This is another unspoken benefit of this patch (it had too many
benefits. :0).  Now a lot of board-specific mtd code is moved
into board directory instead of mtd directory.  One would appreciate it
even more when the support for a board is dropped - guess what? No
mtd code change.  Not the case today.

> For example, on linux-2.6 people are trying to have an almost #ifdef-less
> drivers/video/pxafb.c and have all in arm/arm/mach-pxa/*.c files.
> 
> 
> But just one note: you don't need CONFIG_MTD_JMR3927 if JMR3927 is your
> board. At least not in arm/xscale kernels (not sure about other platforms).
> You already have CONFIG_ARCH_<board>, e.g. I have CONFIG_ARCH_RAMSES. And I
> can re-use this config var in drivers/mtd/maps.
>

That is true.  It just happened that JMR3927 chose to create a new
config for MTD.

Jun

Re: [PATCH] extend physmap.c to support run-time adding partitions

[David Woodhouse]

On Tue, 2003-10-28 at 18:28 -0800, Jun Sun wrote:
+#if defined(CONFIG_MTD)
+       /* we use generic physmap mapping driver */
+       physmap_set_map(0x400000, 4, 0x1e000000);
+
+       physmap_add_partition("YAMON", 0x100000, 0x0, MTD_WRITEABLE);
+       physmap_add_partition("User FS", 0x300000, 0x100000, 0);
+#endif

That's nicer. Why multiple physmap_add_partition() calls rather than
just a single array passed to physmap_set_map() though?

	mtd = physmap_set_map(adr, len, width, set_vpp, partitions);

	physmap_unset_map(mtd);

-- 
dwmw2

Re: [PATCH] extend physmap.c to support run-time adding partitions

[Jun Sun]

On Wed, Oct 29, 2003 at 11:13:26AM +0000, David Woodhouse wrote:
> On Tue, 2003-10-28 at 18:28 -0800, Jun Sun wrote:
> +#if defined(CONFIG_MTD)
> +       /* we use generic physmap mapping driver */
> +       physmap_set_map(0x400000, 4, 0x1e000000);
> +
> +       physmap_add_partition("YAMON", 0x100000, 0x0, MTD_WRITEABLE);
> +       physmap_add_partition("User FS", 0x300000, 0x100000, 0);
> +#endif
> 
> That's nicer. Why multiple physmap_add_partition() calls rather than
> just a single array passed to physmap_set_map() though?
> 

Hmm, currently partition only matters when CONFIG_MTD_PARTITIONS
is enabled.  I assume this mean sometimes people want to use
physmap without partitions.  True?

The board code here really takes advantage of the board-specific
knowledge, where it knows it has support for partitions.  (Maybe
I should add #ifdef CONFIG_MTD_PARTITIONS to surround adding partition
part)

> 	mtd = physmap_set_map(adr, len, width, set_vpp, partitions);
> 
> 	physmap_unset_map(mtd);
> 

phsmap currently only supports mapping for one physical flash unit.
I assume physmap_unset_map() is pretty much null op here?  

And the "physmap_set_map" really means something like "physmap_configure_map". 
So I am not sure if something "unconfigure" is necessary.

Jun

Re: [PATCH] extend physmap.c to support run-time adding partitions

[Jörn Engel]

On Wed, 29 October 2003 10:45:46 -0800, Jun Sun wrote:
> 
> phsmap currently only supports mapping for one physical flash unit.

I've created a physmap variant for multiply physical mappings once.
Google for mphysmap.c and you can still find it.  No reaction
whatsoever to this day, so you can guess how many people need it. ;)

Jörn

-- 
"Error protection by error detection and correction."
-- from a university class

Re: [PATCH] extend physmap.c to support run-time adding partitions

[Cam Mayor]

On Wednesday 29 October 2003 12:45, Jun Sun wrote:
> On Wed, Oct 29, 2003 at 11:13:26AM +0000, David Woodhouse wrote:
> >
> > That's nicer. Why multiple physmap_add_partition() calls rather than
> > just a single array passed to physmap_set_map() though?
>
> Hmm, currently partition only matters when CONFIG_MTD_PARTITIONS
> is enabled.  I assume this mean sometimes people want to use
> physmap without partitions.  True?

Yes.  We have a product that partitions physmap (from the kernel command 
line), but also uses the unpartitioned physmap base device. 

ie. physmap on /dev/mtd0
    physmap partitions on /dev/mtd1, /dev/mtd2, etc.   sometimes none.

cam

-- 
Cameron Mayor
Iders Incorporated

Re: [PATCH] extend physmap.c to support run-time adding partitions

[Jun Sun]

On Wed, Oct 29, 2003 at 04:57:27PM -0600, Cam Mayor wrote:
> On Wednesday 29 October 2003 12:45, Jun Sun wrote:
> > On Wed, Oct 29, 2003 at 11:13:26AM +0000, David Woodhouse wrote:
> > >
> > > That's nicer. Why multiple physmap_add_partition() calls rather than
> > > just a single array passed to physmap_set_map() though?
> >
> > Hmm, currently partition only matters when CONFIG_MTD_PARTITIONS
> > is enabled.  I assume this mean sometimes people want to use
> > physmap without partitions.  True?
> 
> Yes.  We have a product that partitions physmap (from the kernel command 
> line), but also uses the unpartitioned physmap base device. 
> 
> ie. physmap on /dev/mtd0
>     physmap partitions on /dev/mtd1, /dev/mtd2, etc.   sometimes none.
> 

Thanks for the data point.  

My original comment is really a question to David: do we really
want "partition" to be an argument in phsmap_set_map() even when
CONFIG_MTD_PARTITIONS is _not_ configured?

If David does not like multiple add partition calls, we can
easily introduce another interface function:

 	physmap_set_partitions(partitions)

So it seems like we have three possibilities in terms
how a board tells physmap driver about the partitions:

1) lumped in physmap_set_map() (as David suggested)

2) calling adding parition multiple times (as in my original
   proposal)

3) calling adding partitions in one shot with an array.

I don't like 1) because of CONFIG_MTD_PARTITION issue.  But if
David insists, I can modify patch as such.

Jun

Re: [PATCH] extend physmap.c to support run-time adding partitions

[Jun Sun]

On Wed, Oct 29, 2003 at 08:32:09PM +0100, Jörn Engel wrote:
> On Wed, 29 October 2003 10:45:46 -0800, Jun Sun wrote:
> > 
> > phsmap currently only supports mapping for one physical flash unit.
> 
> I've created a physmap variant for multiply physical mappings once.
> Google for mphysmap.c and you can still find it.  No reaction
> whatsoever to this day, so you can guess how many people need it. ;)
> 

Cool!  I think once we extend phsymap driver and get more boards
converted to using it good stuff like this will be in much more demand.

Jun