[PATCH] MTD Maps driver for Sharp LH7a40x

[PATCH] MTD Maps driver for Sharp LH7a40x

[Marc Singer]

This is the MTD patch.


diff -Nru a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
--- a/drivers/mtd/maps/Kconfig	Sat Jun  5 10:30:48 2004
+++ b/drivers/mtd/maps/Kconfig	Sat Jun  5 10:30:48 2004
@@ -421,6 +421,14 @@
 	  Excalibur XA10 Development Board. If you are building a kernel
 	  for on of these boards then you should say 'Y' otherwise say 'N'.
 
+config MTD_LPD7A40X
+	tristate "CFI Flash device mapped on Logic PD LH7A40X-10 Card Engines"
+	depends on (MACH_LPD7A400 || MACH_LPD7A404) && MTD_CFI_INTELEXT && MTD_PARTITIONS
+	help
+	  This provides a driver for the on-board flash of the Logic PD
+	  LH7A40X-10 Card Engines.  Static partition mappings
+	  correspond to the BLOB loader's layout.
+
 config MTD_FORTUNET
 	tristate "CFI Flash device mapped on the FortuNet board"
 	depends on ARM && MTD_CFI && MTD_PARTITIONS && SA1100_FORTUNET
diff -Nru a/drivers/mtd/maps/Makefile b/drivers/mtd/maps/Makefile
--- a/drivers/mtd/maps/Makefile	Sat Jun  5 10:30:48 2004
+++ b/drivers/mtd/maps/Makefile	Sat Jun  5 10:30:48 2004
@@ -56,3 +56,4 @@
 obj-$(CONFIG_MTD_ARCTIC)	+= arctic-mtd.o
 obj-$(CONFIG_MTD_H720X)		+= h720x-flash.o
 obj-$(CONFIG_MTD_IXP4XX)	+= ixp4xx.o
+obj-$(CONFIG_MTD_LPD7A40X)	+= lpd7a400-flash.o
diff -Nru a/drivers/mtd/maps/lpd7a400-flash.c b/drivers/mtd/maps/lpd7a400-flash.c
--- /dev/null	Wed Dec 31 16:00:00 1969
+++ b/drivers/mtd/maps/lpd7a400-flash.c	Sat Jun  5 10:30:48 2004
@@ -0,0 +1,171 @@
+/* drivers/mtd/maps/lpd7a400-flash.c
+ *
+ *  Copyright (C) 2004 Coastal Environmental Systems
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  version 2 as published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <asm/io.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/map.h>
+#include <linux/mtd/partitions.h>
+#include <asm/hardware.h>
+
+
+#define FLASH_PHYS	0x00000000
+#define FLASH_SIZE	0x01000000 /* 16MB */
+#define FLASH_BUSWIDTH	32
+
+#define WINDOW_SIZE 	64*1024*1024 /* Flash mapping window */
+
+static struct map_info lpd7a400_maps[] = {
+	{
+		.name =		"LPD7A40x-10 Flash",
+		.phys =		FLASH_PHYS,
+		.size =		WINDOW_SIZE,
+		.buswidth =	FLASH_BUSWIDTH/8,
+	}
+};
+#define C_MAPS (ARRAY_SIZE (lpd7a400_maps))
+
+static struct mtd_partition lpd7a400_partitions[] = {
+	{
+		.name =		"BLOB boot loader",
+		.offset =	0x00000000,
+		.size =		0x00040000,
+		.mask_flags =	MTD_WRITEABLE /* r/o */
+	},
+	{
+		.name =		"BLOB parameters",
+		.size =		0x00040000,
+		.offset =	0x00040000,
+		.mask_flags =	MTD_WRITEABLE /* r/o */
+	},
+	{
+		.name =		"kernel",
+		.offset =	0x00080000,
+		.size =		0x00180000,
+	},
+	{
+		.name =		"user",
+		.offset =	0x00200000,
+		.size =		MTDPART_SIZ_FULL,
+	}
+};
+
+static struct mtd_info *lpd7a400_mtds[C_MAPS];
+static struct mtd_partition *parsed_parts[C_MAPS];
+static int nr_parsed_parts[C_MAPS];
+
+static const char *probes[] = { NULL };
+
+static int __init init_flash_lpd7a400(void)
+{
+	int ret = 0, i;
+	int cPartitions = 0;
+
+	for (i = 0; i < C_MAPS; ++i) {
+		lpd7a400_maps[i].virt
+			= (unsigned long) __ioremap (lpd7a400_maps[i].phys,
+						     lpd7a400_maps[i].size,
+						     0, 0);
+
+		if (!lpd7a400_maps[i].virt) {
+			printk (KERN_WARNING "Failedioremap %s\n",
+				lpd7a400_maps[i].name);
+			if (!ret)
+				ret = -ENOMEM;
+			continue;
+		}
+
+		simple_map_init (&lpd7a400_maps[i]);
+
+		printk (KERN_NOTICE "Probing \"%s\" "
+			"at physical address 0x%08lx\n",
+			lpd7a400_maps[i].name, lpd7a400_maps[i].phys);
+
+		lpd7a400_mtds[i]
+			= do_map_probe ("cfi_probe", &lpd7a400_maps[i]);
+		
+		if (!lpd7a400_mtds[i]) {
+			iounmap ((void*) lpd7a400_maps[i].virt);
+			if (!ret)
+				ret = -EIO;
+			continue;
+		}
+		++cPartitions;
+		lpd7a400_mtds[i]->owner = THIS_MODULE;
+
+		int ret = parse_mtd_partitions (lpd7a400_mtds[i], probes,
+						&parsed_parts[i], 0);
+
+		if (ret > 0)
+			nr_parsed_parts[i] = ret;
+	}
+
+	if (!cPartitions)
+		return ret;
+	
+	for (i = 0; i < C_MAPS; i++) {
+		if (!lpd7a400_mtds[i]) {
+			printk (KERN_WARNING "%s is absent. Skipping\n",
+				lpd7a400_maps[i].name);
+		}
+		else if (nr_parsed_parts[i]) {
+			add_mtd_partitions (lpd7a400_mtds[i],
+					    parsed_parts[i],
+					    nr_parsed_parts[i]);
+		}
+		else  {
+			printk("Using static partitions on \"%s\"\n",
+			       lpd7a400_maps[i].name);
+			add_mtd_partitions (lpd7a400_mtds[i],
+					    lpd7a400_partitions,
+					    ARRAY_SIZE (lpd7a400_partitions));
+		}
+#if 0
+		else {
+			printk ("Registering %s as whole device\n",
+				lpd7a400_maps[i].name);
+			add_mtd_device(lpd7a400_mtds[i]);
+		}
+#endif
+	}
+	return 0;
+}
+
+static void __exit cleanup_flash_lpd7a400(void)
+{
+	int i;
+	for (i = 0; i < C_MAPS; i++) {
+		if (!lpd7a400_mtds[i])
+			continue;
+
+/*		if (nr_parsed_parts[i] || !i) */
+			del_mtd_partitions (lpd7a400_mtds[i]);
+/*
+		else
+			del_mtd_device (lpd7a400_mtds[i]);
+*/
+
+		map_destroy (lpd7a400_mtds[i]);
+		iounmap ((void*) lpd7a400_maps[i].virt);
+
+		if (parsed_parts[i])
+			kfree (parsed_parts[i]);
+	}
+}
+
+module_init(init_flash_lpd7a400);
+module_exit(cleanup_flash_lpd7a400);
+
+MODULE_AUTHOR("Marc Singer");
+MODULE_DESCRIPTION("MTD map driver for Logic PD LPD7A40x-10 CardEngines");
+MODULE_LICENSE("GPL");

Re: [PATCH] MTD Maps driver for Sharp LH7a40x

[Thomas Gleixner]

Hello Marc,

On Saturday 12 June 2004 18:47, Marc Singer wrote:
> This is the MTD patch.

Please clean up the code from unused code inside #if 0 / #endifs and comments. 
Is there more than one chip on the board ? If not, then the whole for / array 
stuff is unneccecary and makes it just hard to understand.

-- 
Thomas
_____________________________________________________________________
From slash dot org
"When customers are visiting, engineers are not allowed to wear ties. 
That way the customer can tell who is the engineer and who is the 
salesman (and therefore whom to believe.). Ties cut off blood flow 
to the brain, making it easier for the salesmen to do their jobs." 
_____________________________________________________________________
linutronix - competence in embedded & realtime linux
http://www.linutronix.de
mail: tglx@linutronix.de

[PATCH] MTD Maps driver for Sharp LH7a40x

[Marc Singer]

I've removed the ununsed, if-def's and commented, code from the
driver.  

As for the C_MAPS loop, there doesn't seem to be a good way to remove
it.  Even though there is only one 'chip', we still the map_info
structure for the simple_map_init() and friends.  The number of code
code bytes to be saved is nomimal.  The change would be to remove the
array references.  How do they make the code 'confusing'?


# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/06/13 14:15:25-07:00 elf@florence.buici.com 
#   Removed unused code for whole-device mapping.
# 
# drivers/mtd/maps/lpd7a400-flash.c
#   2004/06/13 14:15:15-07:00 elf@florence.buici.com +2 -14
#   Removed unused code for whole-device mapping.
# 
# drivers/mtd/maps/lpd7a400-flash.c
#   2004/06/04 13:25:08-07:00 elf@florence.buici.com +171 -0
# 
# drivers/mtd/maps/lpd7a400-flash.c
#   2004/06/04 13:25:08-07:00 elf@florence.buici.com +0 -0
#   BitKeeper file /m/elf/home/z/embedded/linux/drivers/mtd/maps/lpd7a400-flash.c
# 
# drivers/mtd/maps/Makefile
#   2004/06/04 13:25:08-07:00 elf@florence.buici.com +1 -0
#   Makefile change to include LPD7a40x MTD/Maps driver.
# 
# drivers/mtd/maps/Kconfig
#   2004/06/04 13:25:08-07:00 elf@florence.buici.com +8 -0
#   New configuration option for MTD/Maps driver for Logic PD LPD7a40x boards.
# 
diff -Nru a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
--- a/drivers/mtd/maps/Kconfig	Sun Jun 13 14:22:06 2004
+++ b/drivers/mtd/maps/Kconfig	Sun Jun 13 14:22:06 2004
@@ -421,6 +421,14 @@
 	  Excalibur XA10 Development Board. If you are building a kernel
 	  for on of these boards then you should say 'Y' otherwise say 'N'.
 
+config MTD_LPD7A40X
+	tristate "CFI Flash device mapped on Logic PD LH7A40X-10 Card Engines"
+	depends on (MACH_LPD7A400 || MACH_LPD7A404) && MTD_CFI_INTELEXT && MTD_PARTITIONS
+	help
+	  This provides a driver for the on-board flash of the Logic PD
+	  LH7A40X-10 Card Engines.  Static partition mappings
+	  correspond to the BLOB loader's layout.
+
 config MTD_FORTUNET
 	tristate "CFI Flash device mapped on the FortuNet board"
 	depends on ARM && MTD_CFI && MTD_PARTITIONS && SA1100_FORTUNET
diff -Nru a/drivers/mtd/maps/Makefile b/drivers/mtd/maps/Makefile
--- a/drivers/mtd/maps/Makefile	Sun Jun 13 14:22:06 2004
+++ b/drivers/mtd/maps/Makefile	Sun Jun 13 14:22:06 2004
@@ -56,3 +56,4 @@
 obj-$(CONFIG_MTD_ARCTIC)	+= arctic-mtd.o
 obj-$(CONFIG_MTD_H720X)		+= h720x-flash.o
 obj-$(CONFIG_MTD_IXP4XX)	+= ixp4xx.o
+obj-$(CONFIG_MTD_LPD7A40X)	+= lpd7a400-flash.o
diff -Nru a/drivers/mtd/maps/lpd7a400-flash.c b/drivers/mtd/maps/lpd7a400-flash.c
--- /dev/null	Wed Dec 31 16:00:00 1969
+++ b/drivers/mtd/maps/lpd7a400-flash.c	Sun Jun 13 14:22:06 2004
@@ -0,0 +1,159 @@
+/* drivers/mtd/maps/lpd7a400-flash.c
+ *
+ *  Copyright (C) 2004 Coastal Environmental Systems
+ *
+ *  This program is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU General Public License
+ *  version 2 as published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <asm/io.h>
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/map.h>
+#include <linux/mtd/partitions.h>
+#include <asm/hardware.h>
+
+
+#define FLASH_PHYS	0x00000000
+#define FLASH_SIZE	0x01000000 /* 16MB */
+#define FLASH_BUSWIDTH	32
+
+#define WINDOW_SIZE 	64*1024*1024 /* Flash mapping window */
+
+static struct map_info lpd7a400_maps[] = {
+	{
+		.name =		"LPD7A40x-10 Flash",
+		.phys =		FLASH_PHYS,
+		.size =		WINDOW_SIZE,
+		.buswidth =	FLASH_BUSWIDTH/8,
+	}
+};
+#define C_MAPS (ARRAY_SIZE (lpd7a400_maps))
+
+static struct mtd_partition lpd7a400_partitions[] = {
+	{
+		.name =		"BLOB boot loader",
+		.offset =	0x00000000,
+		.size =		0x00040000,
+		.mask_flags =	MTD_WRITEABLE /* r/o */
+	},
+	{
+		.name =		"BLOB parameters",
+		.size =		0x00040000,
+		.offset =	0x00040000,
+		.mask_flags =	MTD_WRITEABLE /* r/o */
+	},
+	{
+		.name =		"kernel",
+		.offset =	0x00080000,
+		.size =		0x00180000,
+	},
+	{
+		.name =		"user",
+		.offset =	0x00200000,
+		.size =		MTDPART_SIZ_FULL,
+	}
+};
+
+static struct mtd_info *lpd7a400_mtds[C_MAPS];
+static struct mtd_partition *parsed_parts[C_MAPS];
+static int nr_parsed_parts[C_MAPS];
+
+static const char *probes[] = { NULL };
+
+static int __init init_flash_lpd7a400(void)
+{
+	int ret = 0, i;
+	int cPartitions = 0;
+
+	for (i = 0; i < C_MAPS; ++i) {
+		lpd7a400_maps[i].virt
+			= (unsigned long) __ioremap (lpd7a400_maps[i].phys,
+						     lpd7a400_maps[i].size,
+						     0, 0);
+
+		if (!lpd7a400_maps[i].virt) {
+			printk (KERN_WARNING "Failed ioremap %s\n",
+				lpd7a400_maps[i].name);
+			if (!ret)
+				ret = -ENOMEM;
+			continue;
+		}
+
+		simple_map_init (&lpd7a400_maps[i]);
+
+		printk (KERN_NOTICE "Probing \"%s\" "
+			"at physical address 0x%08lx\n",
+			lpd7a400_maps[i].name, lpd7a400_maps[i].phys);
+
+		lpd7a400_mtds[i]
+			= do_map_probe ("cfi_probe", &lpd7a400_maps[i]);
+		
+		if (!lpd7a400_mtds[i]) {
+			iounmap ((void*) lpd7a400_maps[i].virt);
+			if (!ret)
+				ret = -EIO;
+			continue;
+		}
+		++cPartitions;
+		lpd7a400_mtds[i]->owner = THIS_MODULE;
+
+		int ret = parse_mtd_partitions (lpd7a400_mtds[i], probes,
+						&parsed_parts[i], 0);
+
+		if (ret > 0)
+			nr_parsed_parts[i] = ret;
+	}
+
+	if (!cPartitions)
+		return ret;
+	
+	for (i = 0; i < C_MAPS; i++) {
+		if (!lpd7a400_mtds[i]) {
+			printk (KERN_WARNING "%s is absent. Skipping\n",
+				lpd7a400_maps[i].name);
+		}
+		else if (nr_parsed_parts[i]) {
+			add_mtd_partitions (lpd7a400_mtds[i],
+					    parsed_parts[i],
+					    nr_parsed_parts[i]);
+		}
+		else  {
+			printk("Using static partitions on \"%s\"\n",
+			       lpd7a400_maps[i].name);
+			add_mtd_partitions (lpd7a400_mtds[i],
+					    lpd7a400_partitions,
+					    ARRAY_SIZE (lpd7a400_partitions));
+		}
+	}
+	return 0;
+}
+
+static void __exit cleanup_flash_lpd7a400(void)
+{
+	int i;
+	for (i = 0; i < C_MAPS; i++) {
+		if (!lpd7a400_mtds[i])
+			continue;
+
+		del_mtd_partitions (lpd7a400_mtds[i]);
+
+		map_destroy (lpd7a400_mtds[i]);
+		iounmap ((void*) lpd7a400_maps[i].virt);
+
+		if (parsed_parts[i])
+			kfree (parsed_parts[i]);
+	}
+}
+
+module_init(init_flash_lpd7a400);
+module_exit(cleanup_flash_lpd7a400);
+
+MODULE_AUTHOR("Marc Singer");
+MODULE_DESCRIPTION("MTD map driver for Logic PD LPD7A40x-10 CardEngines");
+MODULE_LICENSE("GPL");

Re: [PATCH] MTD Maps driver for Sharp LH7a40x

[Thomas Gleixner]

On Sunday 13 June 2004 23:43, Marc Singer wrote:
> I've removed the ununsed, if-def's and commented, code from the
> driver.
>
> As for the C_MAPS loop, there doesn't seem to be a good way to remove
> it.  Even though there is only one 'chip', we still the map_info
> structure for the simple_map_init() and friends.  The number of code
> code bytes to be saved is nomimal.  The change would be to remove the
> array references.  How do they make the code 'confusing'?

That's not a question of code bytes. 

Why does this code need a for (i = 0; i < C_MAPS;...) loop, if there is only 
one chip which has to be detected, neglected or whatever and C_MAPS is 
therefor 1 ?

In fact the whole driver could be replaced by command line options.

-- 
Thomas

Re: [PATCH] MTD Maps driver for Sharp LH7a40x

[Marc Singer]

On Mon, Jun 14, 2004 at 02:07:48AM +0200, Thomas Gleixner wrote:
> On Sunday 13 June 2004 23:43, Marc Singer wrote:
> > I've removed the ununsed, if-def's and commented, code from the
> > driver.
> >
> > As for the C_MAPS loop, there doesn't seem to be a good way to remove
> > it.  Even though there is only one 'chip', we still the map_info
> > structure for the simple_map_init() and friends.  The number of code
> > code bytes to be saved is nomimal.  The change would be to remove the
> > array references.  How do they make the code 'confusing'?
> 
> That's not a question of code bytes. 
> 
> Why does this code need a for (i = 0; i < C_MAPS;...) loop, if there is only 
> one chip which has to be detected, neglected or whatever and C_MAPS is 
> therefor 1 ?

You originally stated that you thought it made the driver 'confusing'.
I'm not seeing how this makes it confusing.

> 
> In fact the whole driver could be replaced by command line options.

So can the memory map.  The presence of a command line option doesn't
make it a good idea to use it. 

Let me put this a different way.  Are you saying that you'll apply the
patch if I remove the loop?

Cheers.

Re: [PATCH] MTD Maps driver for Sharp LH7a40x

[Jun Sun]

On Sun, Jun 13, 2004 at 05:54:55PM -0700, Marc Singer wrote:
> On Mon, Jun 14, 2004 at 02:07:48AM +0200, Thomas Gleixner wrote:
> > On Sunday 13 June 2004 23:43, Marc Singer wrote:
> > > I've removed the ununsed, if-def's and commented, code from the
> > > driver.
> > >
> > > As for the C_MAPS loop, there doesn't seem to be a good way to remove
> > > it.  Even though there is only one 'chip', we still the map_info
> > > structure for the simple_map_init() and friends.  The number of code
> > > code bytes to be saved is nomimal.  The change would be to remove the
> > > array references.  How do they make the code 'confusing'?
> > 
> > That's not a question of code bytes. 
> > 
> > Why does this code need a for (i = 0; i < C_MAPS;...) loop, if there is only 
> > one chip which has to be detected, neglected or whatever and C_MAPS is 
> > therefor 1 ?
> 
> You originally stated that you thought it made the driver 'confusing'.
> I'm not seeing how this makes it confusing.
> 
> > 
> > In fact the whole driver could be replaced by command line options.
> 
> So can the memory map.  The presence of a command line option doesn't
> make it a good idea to use it. 
> 
> Let me put this a different way.  Are you saying that you'll apply the
> patch if I remove the loop?
> 

How about just using physmap mapping driver and keep all your
board specific stuff to your board directory?   See include/linux/mtd/physmap.h
for more details.

Jun

Re: [PATCH] MTD Maps driver for Sharp LH7a40x

[Thomas Gleixner]

Jun,

On Saturday 19 June 2004 02:05, Jun Sun wrote:
>
> How about just using physmap mapping driver and keep all your
> board specific stuff to your board directory?   See
> include/linux/mtd/physmap.h for more details.

Thanks for jumping in. That's what I was trying to point out. 

In fact most of the board drivers in drivers/mtd/maps could / should go away. 
The reason to have one is crappy hardware, unusual chip constellations or 
other anomalities.

The common case can just be solved by using the physmap driver and the command 
line partition parser. Maybe we should think about an extension to physmap so 
the board support code can supply an appropriate partitioning scheme along 
with the chip description. 

I was thinking about simliar support for the NAND driver too, but there it 
turns out that the anomalities are given per design, although it should not 
be an unsolvable problem.

-- 
Thomas
_____________________________________________________________________
From slash dot org
"When customers are visiting, engineers are not allowed to wear ties. 
That way the customer can tell who is the engineer and who is the 
salesman (and therefore whom to believe.). Ties cut off blood flow 
to the brain, making it easier for the salesmen to do their jobs." 
_____________________________________________________________________
linutronix - competence in embedded & realtime linux
http://www.linutronix.de
mail: tglx@linutronix.de

Re: [PATCH] MTD Maps driver for Sharp LH7a40x

[Marc Singer]

On Sat, Jun 19, 2004 at 02:16:29AM +0200, Thomas Gleixner wrote:
> Jun,
> 
> On Saturday 19 June 2004 02:05, Jun Sun wrote:
> >
> > How about just using physmap mapping driver and keep all your
> > board specific stuff to your board directory?   See
> > include/linux/mtd/physmap.h for more details.
> 
> Thanks for jumping in. That's what I was trying to point out. 
> 
> In fact most of the board drivers in drivers/mtd/maps could / should go away. 
> The reason to have one is crappy hardware, unusual chip constellations or 
> other anomalities.
> 
> The common case can just be solved by using the physmap driver and the command 
> line partition parser. Maybe we should think about an extension to physmap so 
> the board support code can supply an appropriate partitioning scheme along 
> with the chip description. 
> 
> I was thinking about simliar support for the NAND driver too, but there it 
> turns out that the anomalities are given per design, although it should not 
> be an unsolvable problem.

I've chatted with Russell King about this, too.  I think we agree that
board-specific code would be a bad idea.  The better solution appears
to be something on the command line.  I'm going to look into it.  For
now, consider this PATCH discarded.

Re: [PATCH] MTD Maps driver for Sharp LH7a40x

[Thomas Gleixner]

On Saturday 19 June 2004 02:38, Marc Singer wrote:
> > The common case can just be solved by using the physmap driver and the
> > command line partition parser. Maybe we should think about an extension
> > to physmap so the board support code can supply an appropriate
> > partitioning scheme along with the chip description.
> >
> > I was thinking about simliar support for the NAND driver too, but there
> > it turns out that the anomalities are given per design, although it
> > should not be an unsolvable problem.
>
> I've chatted with Russell King about this, too.  I think we agree that
> board-specific code would be a bad idea.  The better solution appears
> to be something on the command line.  I'm going to look into it.  For
> now, consider this PATCH discarded.

Yes, for the common case where we dont have special functions neccecary the 
commandline resp. ATAG (unfortunately only for ARM) is the right place.
This works if the only information which is neccecary is

base address, size and buswidth. 

Russell, can you please give define two ATAG numbers for this purpose ? 
ATAG_MTD_PHYSMAP and 
ATAG_MTD_PARTITION
I would be happy to provide the data structures and parsers to make this work.

BUT,

For chips which need Vpp setting for programming and erasing this is rather 
impossible. Same applies to NAND drivers, as the NAND interface can hardly be 
specified board independent on the commandline.

For general purpose boards (PCI, PC104 whatever) drivers/mtd/ is surely the 
right place. For embedded boards I'm not sure if it makes sense to have all 
this code in the mtd subsystem or if it would be better to have this in the 
board specific code in arch/xxx/boards/myboard.c which is there anyway.

Another possiblity would be to include a hardware related header file in the 
physmap driver and the to be written counterpart for NAND.

#define BOARD_SPECIFIC_MTD_FUNCTIONS
#include <asm/hardware.h>

asm/hardware.h includes board_hardware.h if available.

Inside board_hardware.h then we would have

#ifdef BOARD_SPECIFIC_MTD_FUNCTIONS
board_mtd_functionA(){}
board_mtd_functionB(){}
(nand)pyhsmap chip = {
	.address = xxxx;
	.....
	.functionA = board_mtd_functionA;
	.functionA = board_mtd_functionB;
}
#endif

Not all arch's provide a hardware.h file but this should be a solvable 
problem. For ARM this should work right out of the box.

I know it looks weird in the first place, but it could be one of the possible 
solutions to solve the board support mess. Russell uses a similar approach 
for the timer init and interrupt handler for the various subarchs of ARM 
since long.

Any thoughts ?

-- 
Thomas
_____________________________________________________________________
From slash dot org
"When customers are visiting, engineers are not allowed to wear ties. 
That way the customer can tell who is the engineer and who is the 
salesman (and therefore whom to believe.). Ties cut off blood flow 
to the brain, making it easier for the salesmen to do their jobs." 
_____________________________________________________________________
linutronix - competence in embedded & realtime linux
http://www.linutronix.de
mail: tglx@linutronix.de

Re: [PATCH] MTD Maps driver for Sharp LH7a40x

[Russell King - ARM Linux]

On Sat, Jun 19, 2004 at 10:19:09AM +0200, Thomas Gleixner wrote:
> On Saturday 19 June 2004 02:38, Marc Singer wrote:
> > > The common case can just be solved by using the physmap driver and the
> > > command line partition parser. Maybe we should think about an extension
> > > to physmap so the board support code can supply an appropriate
> > > partitioning scheme along with the chip description.
> > >
> > > I was thinking about simliar support for the NAND driver too, but there
> > > it turns out that the anomalities are given per design, although it
> > > should not be an unsolvable problem.
> >
> > I've chatted with Russell King about this, too.  I think we agree that
> > board-specific code would be a bad idea.  The better solution appears
> > to be something on the command line.  I'm going to look into it.  For
> > now, consider this PATCH discarded.
> 
> Yes, for the common case where we dont have special functions neccecary the 
> commandline resp. ATAG (unfortunately only for ARM) is the right place.

ATAG is only ARM because Linux doesn't like binary interfaces - and
that's one of the reasons I don't want to see it expanding more and
more.  See the "IDE support for lh7a40x" on the linux-arm-kernel list
for a discussion on this topic.

> This works if the only information which is neccecary is
> 
> base address, size and buswidth. 
> 
> Russell, can you please give define two ATAG numbers for this purpose ? 
> ATAG_MTD_PHYSMAP and 
> ATAG_MTD_PARTITION
> I would be happy to provide the data structures and parsers to make this work.

I've already thrown out passing partition information via ATAGs years
ago, and the solution which came from that was the mtdparts= command
line argument.  Now that we have that, there is no reason for
ATAG_MTD_PARTITION.

> For chips which need Vpp setting for programming and erasing this is rather 
> impossible. Same applies to NAND drivers, as the NAND interface can hardly be 
> specified board independent on the commandline.

You may want to take a look at how integrator-flash.c handles this
aspect.  The handling of VPP and write enables is platform specific
and requires some code to do it, so obviously we can't encode that
into an ATAG or command line argument.

Note how integrator-flash.c is used on the Integrator/AP, Integrator/CP
and Versatile/PB platforms, and notice the way the platform support code
is separate in arch/arm/mach-integrator/integrator_*.c and
arch/arm/mach-versatile/core.c.  I think this is a good model for
what you're trying to achieve.

Also note that the size of the flash doesn't have to exactly match the
size of flash fitted to the device - it just has to be of sufficient
size to cover the largest flash which may be fitted to your device.
(or, if you have a way to discover the fitted flash size without too
much hastle, then you could use that.)

> #define BOARD_SPECIFIC_MTD_FUNCTIONS
> #include <asm/hardware.h>
> 
> asm/hardware.h includes board_hardware.h if available.

Eww, no thanks.  We've recently killed that in the PXA tree.  hardware.h
is included by 99% of the kernel include files which means any modification
to it or included files causes the whole kernel to rebuild - which takes
long enough on x86 with gcc 3 already.

> I know it looks weird in the first place, but it could be one of the possible 
> solutions to solve the board support mess. Russell uses a similar approach 
> for the timer init and interrupt handler for the various subarchs of ARM 
> since long.

The timer init is a mess and Deepak's been cleaning that up, so please
don't use the current structure as a justification.

I think you're better off using integrator-flash.c as is, and just
supplying the relevent platform device structure and associated code.

Re: [PATCH] MTD Maps driver for Sharp LH7a40x

[Thomas Gleixner]

On Saturday 19 June 2004 11:23, Russell King - ARM Linux wrote:
>
> ATAG is only ARM because Linux doesn't like binary interfaces - and
> that's one of the reasons I don't want to see it expanding more and
> more.  See the "IDE support for lh7a40x" on the linux-arm-kernel list
> for a discussion on this topic.

Ok, sounds reasonable. I did not follow this thread, as the topic was out of 
my primary interest.

> > For chips which need Vpp setting for programming and erasing this is
> > rather impossible. Same applies to NAND drivers, as the NAND interface
> > can hardly be specified board independent on the commandline.
>
> You may want to take a look at how integrator-flash.c handles this
> aspect.  The handling of VPP and write enables is platform specific
> and requires some code to do it, so obviously we can't encode that
> into an ATAG or command line argument.
>
> Note how integrator-flash.c is used on the Integrator/AP, Integrator/CP
> and Versatile/PB platforms, and notice the way the platform support code
> is separate in arch/arm/mach-integrator/integrator_*.c and
> arch/arm/mach-versatile/core.c.  I think this is a good model for
> what you're trying to achieve.

Yep. I was not aware of this. But it would have been my first choice too to 
put this into the board support files / directories which are available / 
neccecary for embedded systems anyway.

> Also note that the size of the flash doesn't have to exactly match the
> size of flash fitted to the device - it just has to be of sufficient
> size to cover the largest flash which may be fitted to your device.
> (or, if you have a way to discover the fitted flash size without too
> much hastle, then you could use that.)

I know. For NAND we use the chip ID to do that anyway.

> Eww, no thanks.  We've recently killed that in the PXA tree.  hardware.h
> is included by 99% of the kernel include files which means any modification
> to it or included files causes the whole kernel to rebuild - which takes
> long enough on x86 with gcc 3 already.
> The timer init is a mess and Deepak's been cleaning that up, so please
> don't use the current structure as a justification.
> I think you're better off using integrator-flash.c as is, and just
> supplying the relevent platform device structure and associated code.

I just said that ist possible, but weird. You're definitly right to make this 
go away.

So a final conclusion would be:

1. Add commandline parsing for flashbase,size,buswidth which covers the case 
where no special code is neccecary to access the chips.

2. Use the already existing platform model, which can be retrieved from your 
work in arch/arm/mach-xxxx

3. Extend the generic NAND driver to provide a simple mechanism to make 2.) 
work for NAND too

4. Obsolete the board specific mapping drivers in drivers/mtd/maps and 
drivers/mtd/nand when this is done.

Did I miss(understand) anything ?

-- 
Thomas
_____________________________________________________________________
From slash dot org
"When customers are visiting, engineers are not allowed to wear ties. 
That way the customer can tell who is the engineer and who is the 
salesman (and therefore whom to believe.). Ties cut off blood flow 
to the brain, making it easier for the salesmen to do their jobs." 
_____________________________________________________________________
linutronix - competence in embedded & realtime linux
http://www.linutronix.de
mail: tglx@linutronix.de

Re: [PATCH] MTD Maps driver for Sharp LH7a40x

[Russell King - ARM Linux]

On Sat, Jun 19, 2004 at 11:51:53AM +0200, Thomas Gleixner wrote:
> So a final conclusion would be:
> 
> 1. Add commandline parsing for flashbase,size,buswidth which covers the case 
> where no special code is neccecary to access the chips.

I don't have strong views on this, but if the platform model is going
to be used (so that we get the VPP and WP enable code), there seems to
be little point in separating this out into the boot loader.

Other than that, it seems fine.

RE: [PATCH] MTD Maps driver for Sharp LH7a40x

[David Woodhouse]

On Sat, 2004-06-19 at 10:08 -0600, Derek Jones wrote:
> This makes a lot of sense.

Your mail, on the other hand, makes none. It's HTML, it's top-posting,
it lacks correct References: headers, and it lacks correct quoting --
some of what seems to be your own input I know is actually something
that Thomas said. 

Please get a proper mail client before ever trying to send mail to me or
my lists again.

-- 
dwmw2