[PATCH] Linux 2.4.x MTD CFI P30/P33 support

[PATCH] Linux 2.4.x MTD CFI P30/P33 support

[Korolev, Alexey]

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.

Signed-off-by: Alexey Korolev <alexey.korolev@intel.com>
==================================================
--- 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

Re: [PATCH] Linux 2.4.x MTD CFI P30/P33 support

[Willy Tarreau]

Hi Alexey,

On Sat, Mar 17, 2007 at 02:45:13AM +0300, Korolev, Alexey wrote:
> 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.

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

> Signed-off-by: Alexey Korolev <alexey.korolev@intel.com>
> ==================================================
> --- 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

Re: [PATCH] Linux 2.4.x MTD CFI P30/P33 support

[akorolev]

Hi Willy,


> 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.
>   
Oh thank you very much. Sorry for wrapped text. I've corrected it.

Signed-off-by: Alexey Korolev <alexey.korolev@intel.com
------------------------
--- 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

Re: [PATCH] Linux 2.4.x MTD CFI P30/P33 support

[akorolev]

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
> Hi Willy,
>
>
>> 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.
>>   
> Oh thank you very much. Sorry for wrapped text. I've corrected it.
>
> Signed-off-by: Alexey Korolev <alexey.korolev@intel.com
> ------------------------
> --- 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
>
>

Re: [PATCH] Linux 2.4.x MTD CFI P30/P33 support

[Willy Tarreau]

Hello Alexey,

On Tue, Apr 10, 2007 at 09:45:12PM +0400, akorolev wrote:
> 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?

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

[PATCH] SPI FLASH naming conflicts solving

[akorolev]

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
--- orig/drivers/mtd/devices/m25p80.c	2007-06-01 19:06:52.000000000 +0400
+++ new/drivers/mtd/devices/m25p80.c	2007-05-10 16:56:15.000000000 +0400
@@ -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
--- orig/drivers/mtd/devices/mtd_dataflash.c	2007-06-01 19:06:48.000000000 +0400
+++ new/drivers/mtd/devices/mtd_dataflash.c	2007-05-10 14:41:39.000000000 +0400
@@ -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?

Re: [PATCH] SPI FLASH naming conflicts solving

[David Brownell]

On Friday 01 June 2007, akorolev wrote:
> 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.

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.


> 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.

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.


> --- orig/drivers/mtd/devices/m25p80.c	2007-06-01 19:06:52.000000000 +0400
> +++ new/drivers/mtd/devices/m25p80.c	2007-05-10 16:56:15.000000000 +0400
> @@ -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
> --- orig/drivers/mtd/devices/mtd_dataflash.c	2007-06-01 19:06:48.000000000 +0400
> +++ new/drivers/mtd/devices/mtd_dataflash.c	2007-05-10 14:41:39.000000000 +0400
> @@ -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.  

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


> 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?

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