Messages in this thread | | | From | David Brownell <> | Subject | Re: [PATCH] SPI FLASH naming conflicts solving | Date | Fri, 1 Jun 2007 10:01:05 -0700 |
| |
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
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |