Linux 2.4.x MTD CFI P30/P33 support

Hello Willy,

The following patch adds support of P30 and P33 NOR FLASH support in Linux 2.4
This flash is substitution of J3 flash which is widely used it on Linux 2.4 kernels. Currently many customers wishing to substitute J3 for P3x face issues on Linux 2.4. This patch resolves issues with P3x on all generic Linux kernels versions since 2.4.21.

The patch just allows using minor version "4" in CFI driver. Since differences between CFI minor version "3" and minor version "4" are small the CFI driver is not affected by this. Patch has been verified on Mainstone (PXA27x based) platform.

==================================================
--- a/drivers/mtd/chips/cfi_cmdset_0001.c 2003-06-13
18:51:34.000000000 +0400
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c 2007-02-16
21:39:50.000000000 +0300
@@ -152,7 +152,7 @@
}

 		if (extp->MajorVersion != '1' || 
-		    (extp->MinorVersion < '0' || extp->MinorVersion >

'3')) {
+ (extp->MinorVersion < '0' || extp->MinorVersion >
'4')) {
printk(KERN_WARNING " Unknown IntelExt Extended
Query "
"version %c.%c.\n", extp->MajorVersion, extp->MinorVersion);
===================================================

Thanks,
Alexey

Hi Alexey,

OK, it seems trivial enough to me. I have no problem merging this. Your mailer has wrapped lines, but I'll fix this by hand.

David, are you OK too ?

Thanks,
Willy

Hi Willy,

Oh thank you very much. Sorry for wrapped text. I've corrected it.

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

@@ -152,7 +152,7 @@

		}
		
		if (extp->MajorVersion != '1' || 
-		    (extp->MinorVersion < '0' || extp->MinorVersion > '3')) {
+		    (extp->MinorVersion < '0' || extp->MinorVersion > '4')) {
			printk(KERN_WARNING "  Unknown IntelExt Extended Query "
			       "version %c.%c.\n",  extp->MajorVersion,
			       extp->MinorVersion);

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

Thanks,
Alexey

Hello Willy,

What version of linux 2.4.x kernel do you expect P30/P33 support may be included?
May be something else I should do in addition. What should I do to get the patch included into linux 2.4.x?

Thanks,
Alexey

Hello Alexey,

Oh I'm sorry. I asked David and left the patch pending in the "waiting confirmation" queue. Since he did not respond, I will merge it now.

Thanks again for having pinged me again. It's not always easy to deal with people who don't have time to spend on 2.4 anymore, so I certainly can imagine it's not easy on your side to wait for my responses :-/

Regards,
Willy

Hi Dawid

I tried to enable SPI flash device in our development platform (Mainstone).
Since the platform driver uses NOR flash device - it includes "include/asm-arm/mach/flash.h" file where the structure "flhash_platform _data" is defined. The structure with the same name "flhash_platform _data" is also defined in "include/linux/spi/flash.h".
These structures are different - so they cause compile errors.

I attached the fix for this issue. IMHO it's more preferable to resolve issue by minor modification of SPI driver than touching all arm platform drivers.

=============================

diff -aur orig/drivers/mtd/devices/m25p80.c new/drivers/mtd/devices/m25p80.c
@@ -423,7 +449,7 @@
 */

static int __devinit m25p_probe(struct spi_device *spi) {
- struct flash_platform_data *data;

+	struct spi_flash_platform_data	*data;
	struct m25p			*flash;
	struct flash_info		*info;
	unsigned			i;

diff -aur orig/drivers/mtd/devices/mtd_dataflash.c new/drivers/mtd/devices/mtd_dataflash.c
@@ -457,7 +457,7 @@

{
	struct dataflash		*priv;
	struct mtd_info			*device;

- struct flash_platform_data *pdata = spi->dev.platform_data;
+ struct spi_flash_platform_data *pdata = spi->dev.platform_data;

	priv = kzalloc(sizeof *priv, GFP_KERNEL);

=============================
Could you please take a look at this. The fix is simple. If you don't complain could you please include it.

Thanks,
Alexey

P/S Also I have a question:
I faced rather strange problem with sending read and write commands to SPI flash. It works Ok if I reverse 32bit SPI FLASH command I mean - read/write/erase works if to substitute
flash->command[0] = OPCODE_SE;
flash->command[1] = offset >> 16;
flash->command[2] = offset >> 8;
flash->command[3] = offset;
for
flash->command[3] = OPCODE_SE;
flash->command[2] = offset >> 16;
flash->command[1] = offset >> 8;
flash->command[0] = offset;
in file drivers/mtd/devices/m25p80.c

It seems pretty strange and may require more investigation.
I wonder If you have any ideas why it could happen? If anybody tested m25p80.c driver on LittleEndian platforms?

That name seems a bit generic for either driver to use!

Except I don't understand why <asm/mach/flash.h> is used there instead of the more current <linux/mtd/physmap.h> ... certainly mainstone's current code doesn't need more than physmap. After all, the reason for the physmap updates was to help move away from the platform-specific drivers (like those ARM ones).

And for mainstone, switching looks trivial. Not to mention overdue, since the "pxa2xx-flash" driver which would use the old ARM-specific "flash_platform_data" isn't available in the kernel.org tree ... so arch/arm/mach-pxa/mainstone.c seems to be buggy in even using that struct.

Not "all" of them ... and in any case, as I said, I thought the idea was to start getting rid of all those platform specific mapping drivers.

In your case, it seems like fixing mainstone.c should come first; and that would eliminate the problem you're seeing. As folk have noted, the PXA code hasn't been well maintained; this would seem to be a symptom of that.

But otherwise, if that ARM-specific stuff should still be used ... maybe. You omitted the header file change, as well as some of its other users. (A surprising number of Blackfin boards use m25p80 with platform data to specify partitions.) A shorter name, maybe "spi_flash_data", would be better too.

I think that'd be for the MTD and platform maintainers to handle.

I'd guess this is just a bug in your spi controller driver or in its configuration. It looks like you're using a 32 bits_per_word setting -- with an effective cpu_to_be32() in the I/O path, wrong since the data is *already* in the right byte order -- instead of the 8 bits specified by the driver (no byte order changes).

- Dave