CM-x2xx NAND flash support
Subsystems (2)
PATCH 1/1 CM-x2xx NAND flash support
This patch provides MTD support for NAND flash devices on CM-x2xx modules.
Signature · 2 lines
Sincerely yours, Mike Rapoport
drivers/mtd/nand/Kconfig | 8 + drivers/mtd/nand/Makefile | 2 drivers/mtd/nand/cmx255-nand.c | 247 ++++++++++++++++++++++++++++++++++++ drivers/mtd/nand/cmx270-nand.c | 273 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 530 insertions(+), 0 deletions(-)
drivers/mtd/nand/Kconfig +8 -0
@@ -232,6 +232,14 @@ config MTD_NAND_CS553X If you say "m", the module will be called "cs553x_nand.ko". +config MTD_NAND_CM_X255 + tristate "Support for NAND Flash on CM-X255 modules" + depends on MTD_NAND && CM_X255 + +config MTD_NAND_CM_X270 + tristate "Support for NAND Flash on CM-X270 modules" + depends on MTD_NAND && CM_X270 + config MTD_NAND_NANDSIM tristate "Support for NAND Flash Simulator" depends on MTD_NAND && MTD_PARTITIONS
drivers/mtd/nand/Makefile +2 -0
@@ -22,5 +22,7 @@ obj-$(CONFIG_MTD_NAND_TS7250) += ts7250 obj-$(CONFIG_MTD_NAND_NANDSIM) += nandsim.o obj-$(CONFIG_MTD_NAND_CS553X) += cs553x_nand.o obj-$(CONFIG_MTD_NAND_NDFC) += ndfc.o +obj-$(CONFIG_MTD_NAND_CM_X255) += cmx255-nand.o +obj-$(CONFIG_MTD_NAND_CM_X270) += cmx270-nand.o nand-objs = nand_base.o nand_bbt.o
drivers/mtd/nand/cmx255-nand.c new file +247 -0
247 lines not on this page · raw message
drivers/mtd/nand/cmx270-nand.c new file +273 -0
273 lines not on this page · raw message
Quoted 5 lines · Mike Rapoport
This patch provides MTD support for NAND flash devices on CM-x2xx modules.
Signed-off-by Mike Rapoport <[email protected]>+static struct
mtd_info*cmx255_nand_mtd= NULL;
static variables are initialized to 0 by default
Quoted 3 lines
+/*
+ * Module stuff
+ */
Comment without context
drivers/mtd/nand/cmx255-nand.c line 64
Quoted 6 lines
drivers/mtd/nand/cmx255-nand.c new file +6
-0+#define DRAIN_WB () \ + do { \ + unsigned char dummy; \ + asm volatile ("mcr p15, 0, r0, c7, c10, 4":::"r0"); \ + dummy=*((unsigned char*)UNCACHED_ADDR); \ + } while(0);
stray semicolon ----^
I bet xscale has this functionality somewhere as a macro / inline already
drivers/mtd/nand/cmx255-nand.c line 105
Quoted 7 lines
drivers/mtd/nand/cmx255-nand.c new file +7
-0+/* +* hardware specific access to control-lines +*/ +static void cmx255_hwcontrol (struct mtd_info *mtd,int dat, +unsigned int ctrl) +{ +struct nand_chip * this = (struct nand_chip *) (mtd->priv);
no type cast for void* necessary
Quoted 1 line
+ unsigned int nandaddr = (unsigned int)this->IO_ADDR_W;
what the hell is this type cast for ?
void __iomem *nandaddr =
drivers/mtd/nand/cmx255-nand.c line 111
Quoted 4 lines
drivers/mtd/nand/cmx255-nand.c new file +4
-0+DRAIN_WB (); + +if ( ctrl &NAND_CTRL_CHANGE ) { +if ( ctrl &NAND_NCE )/* CE should be changed */
if (ctrl & N...) {
also the comment is wrong for all three if()s. The given value isstatic and does not necessarily change from one call to the next. e.g. nCE is kept low across a complete access cycle
drivers/mtd/nand/cmx255-nand.c line 124
Quoted 13 lines
drivers/mtd/nand/cmx255-nand.c new file +13
-0+nand_cs_on (); + else +nand_cs_off (); +if ( ctrl &NAND_ALE )/* ALE should be changed */ +nand_ale_on (); +else +nand_ale_off (); +if ( ctrl &NAND_CLE )/* CLE should be changed */ +nand_cle_on (); +else +nand_cle_off (); + } + this->IO_ADDR_W = (void *)nandaddr;
nandaddr = this->IO_ADDR_W; this->IO_ADDR_W = nandaddr; ?????
Quoted 12 lines
drivers/mtd/nand/cmx255-nand.c new file +12
-0+DRAIN_WB (); + +if ( dat !=NAND_CMD_NONE ) +writel (dat, this->IO_ADDR_W); +} + +/* +* read device ready pin +*/ +static int cmx255_device_ready (struct mtd_info *mtd) +{ +DRAIN_WB ();
drivers/mtd/nand/cmx255-nand.c line 147
Quoted 10 lines
drivers/mtd/nand/cmx255-nand.c new file +10
-0+return GPLR (GPIO_NAND_RB ) &GPIO_bit (GPIO_NAND_RB ); +} + +/* +* Main initialization routine +*/ +static int __init cmx255_init (void ) +{ +struct nand_chip *this; +const char *part_type =0 ;
= NULL
Review
Quoted 2 lines
(quoted diff) +2
-0+ int mtd_parts_nb = 0; + struct mtd_partition *mtd_parts = 0;
dito
Quoted 1 line
+ static unsigned int nandaddr = 0;
why static ? also: void __iomem *nandaddr;
Quoted 1 line
+ int retval = 0;
Please do not initialize local variables, when there is no need to.
drivers/mtd/nand/cmx255-nand.c line 156
Quoted 4 lines
drivers/mtd/nand/cmx255-nand.c new file +4
-0+/* Allocate memory for MTD device structure and private data */ + cmx255_nand_mtd =kmalloc (sizeof (struct mtd_info ) + +sizeof (struct nand_chip ), +GFP_KERNEL );
kzalloc
drivers/mtd/nand/cmx255-nand.c line 163
Quoted 8 lines
drivers/mtd/nand/cmx255-nand.c new file +7
-0+if (!cmx255_nand_mtd) { +printk ("Unable to allocate CM-X255 MTD device structure.\n" ); + retval = -ENOMEM ; + goto out; + } + + nandaddr = (unsigned int )ioremap (PXA_CS1_PHYS ,0x100 );
See above, also convert to a platform device !
Quoted 1 line
+ if ( !nandaddr ) {
if (!nandaddr)
drivers/mtd/nand/cmx255-nand.c line 175
Quoted 11 lines
drivers/mtd/nand/cmx255-nand.c new file +11
-0+printk ("Unable to ioremap NAND device" ); + retval = -EINVAL ; + goto err1; + } + +/* Get pointer to private data */ + this = (struct nand_chip *) (&cmx255_nand_mtd[1 ]); + +/* Initialize structures */ +memset ((char *) cmx255_nand_mtd,0 ,sizeof (struct mtd_info )); +memset ((char *) this,0 ,sizeof (struct nand_chip ));
See above (kzalloc)
drivers/mtd/nand/cmx255-nand.c line 183
Quoted 7 lines
drivers/mtd/nand/cmx255-nand.c new file +7
-0+/* Link the private data with the MTD structure */ + cmx255_nand_mtd->owner =THIS_MODULE ; + cmx255_nand_mtd->priv = this; + +/* insert callbacks */ + this->IO_ADDR_R = (void __iomem *)nandaddr; + this->IO_ADDR_W = (void __iomem *)nandaddr;
See above
drivers/mtd/nand/cmx255-nand.c line 192
Quoted 9 lines
drivers/mtd/nand/cmx255-nand.c new file +9
-0+ this->cmd_ctrl = cmx255_hwcontrol; + this->dev_ready = cmx255_device_ready; + +/* 20 us command delay time */ + this->chip_delay =20 ; + this->ecc.mode =NAND_ECC_SOFT ; + +/* Scan to find existence of the device */ +if (nand_scan (cmx255_nand_mtd,1 )) {
if (nand_scan(
Quoted 6 lines
drivers/mtd/nand/cmx255-nand.c new file +5
-0+printk (KERN_NOTICE "No NAND device\n" ); + retval = -ENXIO ; + goto err2; + } +
drivers/mtd/nand/cmx255-nand.c line 207
Quoted 10 lines
drivers/mtd/nand/cmx255-nand.c new file +10
-0+#ifdef CONFIG_MTD_CMDLINE_PARTS + mtd_parts_nb =parse_cmdline_partitions (cmx255_nand_mtd, &mtd_parts, +"cmx255" ); +if (mtd_parts_nb >0 ) + part_type ="command line" ; +else + mtd_parts_nb =0 ; +#endif +if (mtd_parts_nb ==0 ) + {
if (!mtd_parts_nb) {drivers/mtd/nand/cmx255-nand.c line 215
Quoted 8 lines
drivers/mtd/nand/cmx255-nand.c new file +8
-0+ mtd_parts = partition_info; + mtd_parts_nb =NUM_PARTITIONS ; + part_type ="static" ; + } + +/* Register the partitions */ +printk (KERN_NOTICE "Using %s partition definition\n" , part_type); +add_mtd_partitions (cmx255_nand_mtd, mtd_parts, mtd_parts_nb);
add_mtd_partitions can fail !
also, when you do not intend to use this driver w/o partitions, then please remove the #ifdef CONFIG_MTD_PARTITIONS around partition_info, else you have to do a add_mtd_device(mtd) when partitions are disabled
return 0;
otherwise you iounmap and free the mtd data structure
Please submit tested drivers. This one never ever worked !
drivers/mtd/nand/cmx255-nand.c line 218
Quoted 2 lines
drivers/mtd/nand/cmx255-nand.c new file +2
-0+ err2: +iounmap ((void *)nandaddr);
See above
Quoted 7 lines
drivers/mtd/nand/cmx255-nand.c new file +7
-0+ err1: +kfree (cmx255_nand_mtd); + out: +/* Return happy */ +return retval; +} +module_init (cmx255_init);
drivers/mtd/nand/cmx255-nand.c line 235
Quoted 10 lines
drivers/mtd/nand/cmx255-nand.c new file +10
-0+ +/* +* Clean up routine +*/ +static void __exit cmx255_cleanup (void) +{ +struct nand_chip *this; + + this = (struct nand_chip *) (&cmx255_nand_mtd[1 ]); +iounmap (this->IO_ADDR_R);
unmap after nand_release(). As long as the device is not released it can be accessed !!!!
drivers/mtd/nand/cmx255-nand.c line 238
Quoted 2 lines
drivers/mtd/nand/cmx255-nand.c new file +2
-0+/* Release resources, unregister device */ +nand_release (cmx255_nand_mtd);
nand_release(
drivers/mtd/nand/cmx255-nand.c line 241
Quoted 3 lines
drivers/mtd/nand/cmx255-nand.c new file +3
-0+ +/* Free the MTD device structure */ +kfree (cmx255_nand_mtd);
kfree(
Quoted 44 lines
drivers/mtd/nand/cmx255-nand.c new file +6
-0+} +module_exit (cmx255_cleanup); + +MODULE_LICENSE ("GPL" ); +MODULE_AUTHOR ("Mike Rapoport <mike at compulab dot co dot il>" ); +MODULE_DESCRIPTION ("NAND flash driver for Compulab CM-X255 Module" );drivers/mtd/nand/cmx270-nand.c new file +32
-0@@ -0,0 +1,273 @@ +/* +* linux/drivers/mtd/nand/cmx270-nand.c +* +* Copyright (C) 2006 Compulab, Ltd. +* Mike Rapoport <[email protected]> +* +* Derived from drivers/mtd/nand/h1910.c +* Copyright (C) 2002 Marius Grger ([email protected]) +* Copyright (c) 2001 Thomas Gleixner ([email protected]) +* +* +* 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. +* +* Overview: +* This is a device driver for the NAND flash device found on the +* CM-X270 board. +*/ + +#include <linux/slab.h> +#include <linux/init.h> +#include <linux/module.h> +#include <linux/mtd/mtd.h> +#include <linux/mtd/nand.h> +#include <linux/mtd/partitions.h> + +#include <asm/io.h> +#include <asm/irq.h> + +#include <asm/arch/hardware.h> +#include <asm/arch/pxa-regs.h>
drivers/mtd/nand/cmx270-nand.c line 42
Quoted 10 lines
drivers/mtd/nand/cmx270-nand.c new file +10
-0+ +#define GPIO_NAND_CS (11) +#define GPIO_NAND_RB (89) + +#define DRAIN_WB () \ + do { \ + unsigned char dummy; \ + asm volatile ("mcr p15, 0, r0, c7, c10, 4":::"r0"); \ + dummy=*((unsigned char*)UNCACHED_ADDR); \ + } while(0);
I bet xscale has this functionality somewhere as a macro / inline already
Quoted 16 lines
drivers/mtd/nand/cmx270-nand.c new file +16
-0+/* +* MTD structure for CM-X270 board +*/ +static struct mtd_info *cmx270_nand_mtd =NULL ; + +/* +* Module stuff +*/ + +#ifdef CONFIG_MTD_PARTITIONS +/* + * Define static partitions for flash device + */ +static struct mtd_partition partition_info[] = { + { + .name = "cmx270-0",
drivers/mtd/nand/cmx270-nand.c line 69
Quoted 10 lines
drivers/mtd/nand/cmx270-nand.c new file +10
-0+ .offset = 0, + .size =MTDPART_SIZ_FULL + } +}; +#define NUM_PARTITIONS (ARRAY_SIZE(partition_info)) + +#endif + + +static u_char cmx270_read_byte (struct mtd_info *mtd)
stray tab
drivers/mtd/nand/cmx270-nand.c line 70
Quoted 2 lines
drivers/mtd/nand/cmx270-nand.c new file +2
-0+{ +struct nand_chip *this = mtd->priv;
blank line between variable declaration and instructions
drivers/mtd/nand/cmx270-nand.c line 77
Quoted 6 lines
drivers/mtd/nand/cmx270-nand.c new file +6
-0+return (readl (this->IO_ADDR_R) >>16 ); +} + +static void cmx270_write_byte (struct mtd_info *mtd,u_char byte) +{ +struct nand_chip *this = mtd->priv;
dito
drivers/mtd/nand/cmx270-nand.c line 88
Quoted 11 lines
drivers/mtd/nand/cmx270-nand.c new file +11
-0+writel ((byte <<16 ), this->IO_ADDR_W); +} + +static void cmx270_write_buf (struct mtd_info *mtd,const u_char *buf,int len) +{ +int i; +struct nand_chip *this = mtd->priv; + +for (i=0 ; i<len; i++) { +writel ((*buf++ <<16 ), this->IO_ADDR_W); + }
No brackets for oneliners
drivers/mtd/nand/cmx270-nand.c line 98
Quoted 10 lines
drivers/mtd/nand/cmx270-nand.c new file +10
-0+} + +static void cmx270_read_buf (struct mtd_info *mtd,u_char *buf,int len) +{ +int i; +struct nand_chip *this = mtd->priv; + +for (i=0 ; i<len; i++) { + *buf++ =readl (this->IO_ADDR_R) >>16 ; + }
dito
drivers/mtd/nand/cmx270-nand.c line 108
Quoted 10 lines
drivers/mtd/nand/cmx270-nand.c new file +10
-0+} + +static int cmx270_verify_buf (struct mtd_info *mtd,const u_char *buf,int len) +{ +int i; +struct nand_chip *this = mtd->priv; + +for (i=0 ; i<len; i++) { +if ( buf[i] != (u_char)(readl (this->IO_ADDR_R) >>16 ) ) +return -EFAULT ;
no extra blanks after ( and before )
Quoted 14 lines
drivers/mtd/nand/cmx270-nand.c new file +14
-0+ } + +return 0 ; +} + +static inline void nand_cs_on (void ) +{ +GPCR (GPIO_NAND_CS ) =GPIO_bit (GPIO_NAND_CS ); +} + +static void nand_cs_off (void ) +{ +DRAIN_WB (); +
drivers/mtd/nand/cmx270-nand.c line 132
Quoted 10 lines
drivers/mtd/nand/cmx270-nand.c new file +10
-0+GPSR (GPIO_NAND_CS ) =GPIO_bit (GPIO_NAND_CS ); +} + +/* +* hardware specific access to control-lines +*/ +static void cmx270_hwcontrol (struct mtd_info *mtd,int dat, +unsigned int ctrl) +{ +struct nand_chip * this = (struct nand_chip *) (mtd->priv);
See above
Quoted 9 lines
drivers/mtd/nand/cmx270-nand.c new file +9
-0+unsigned int nandaddr = (unsigned int )this->IO_ADDR_W; + +DRAIN_WB (); + +if ( ctrl &NAND_CTRL_CHANGE ) { +if ( ctrl &NAND_ALE )/* ALE should be changed */ + nandaddr |= (1 <<3 ); +else + nandaddr &= ~(1 <<3 );
drivers/mtd/nand/cmx270-nand.c line 151
Quoted 10 lines
drivers/mtd/nand/cmx270-nand.c new file +10
-0+if ( ctrl &NAND_CLE )/* CLE should be changed */ + nandaddr |= (1 <<2 ); +else + nandaddr &= ~(1 <<2 ); +if ( ctrl &NAND_NCE )/* CE should be changed */ +nand_cs_on (); +else +nand_cs_off (); + } + this->IO_ADDR_W = (void *)nandaddr;
dito
Quoted 15 lines
drivers/mtd/nand/cmx270-nand.c new file +15
-0+DRAIN_WB (); +if ( dat !=NAND_CMD_NONE ) +cmx270_write_byte (mtd, dat); +DRAIN_WB (); +} + +/* +* read device ready pin +*/ +static int cmx270_device_ready (struct mtd_info *mtd) +{ +DRAIN_WB (); +return (GPLR (GPIO_NAND_RB ) &GPIO_bit (GPIO_NAND_RB ) ); +} +
drivers/mtd/nand/cmx270-nand.c line 176
Quoted 10 lines
drivers/mtd/nand/cmx270-nand.c new file +10
-0+/* +* Main initialization routine +*/ +static int __init cmx270_init (void ) +{ +struct nand_chip *this; +const char *part_type =0 ; +int mtd_parts_nb =0 ; +struct mtd_partition *mtd_parts =0 ; +unsigned int nandaddr =0 ;
See above
drivers/mtd/nand/cmx270-nand.c line 182
Quoted 5 lines
drivers/mtd/nand/cmx270-nand.c new file +5
-0+/* Allocate memory for MTD device structure and private data */ + cmx270_nand_mtd =kmalloc (sizeof (struct mtd_info ) + +sizeof (struct nand_chip ), +GFP_KERNEL ); +mb ();
Why do you beed a memory barrier here ?
Quoted 7 lines
drivers/mtd/nand/cmx270-nand.c new file +7
-0+if (!cmx270_nand_mtd) { +printk ("Unable to allocate CM-X270 NAND MTD device structure.\n" ); +return -ENOMEM ; + } + + nandaddr = (unsigned int )ioremap (PXA_CS1_PHYS ,12 ); +if ( !nandaddr ) {
drivers/mtd/nand/cmx270-nand.c line 199
Quoted 10 lines
drivers/mtd/nand/cmx270-nand.c new file +10
-0+printk ("Unable to ioremap NAND device" ); +return -EINVAL ; + } + +/* Get pointer to private data */ + this = (struct nand_chip *) (&cmx270_nand_mtd[1 ]); + +/* Initialize structures */ +memset ((char *) cmx270_nand_mtd,0 ,sizeof (struct mtd_info )); +memset ((char *) this,0 ,sizeof (struct nand_chip ));
See above
Quoted 36 lines
drivers/mtd/nand/cmx270-nand.c new file +36
-0+/* Link the private data with the MTD structure */ + cmx270_nand_mtd->owner =THIS_MODULE ; + cmx270_nand_mtd->priv = this; + +/* insert callbacks */ + this->IO_ADDR_R = (void __iomem *)nandaddr; + this->IO_ADDR_W = (void __iomem *)nandaddr; + this->cmd_ctrl = cmx270_hwcontrol; + this->dev_ready = cmx270_device_ready;/* unknown whether that was correct or not so we will just do it like this */ + +/* 15 us command delay time */ + this->chip_delay =20 ; + this->ecc.mode =NAND_ECC_SOFT ; + +/* read/write functions */ + this->read_byte = cmx270_read_byte; + this->read_buf = cmx270_read_buf; + this->write_buf = cmx270_write_buf; + this->verify_buf = cmx270_verify_buf; + +/* Scan to find existence of the device */ +if (nand_scan (cmx270_nand_mtd,1 )) { +printk (KERN_NOTICE "No NAND device\n" ); +iounmap ((void *)nandaddr); +kfree (cmx270_nand_mtd); +return -ENXIO ; + } + +#ifdef CONFIG_MTD_CMDLINE_PARTS + mtd_parts_nb =parse_cmdline_partitions (cmx270_nand_mtd, &mtd_parts, +"cmx270" ); +if (mtd_parts_nb >0 ) + part_type ="command line" ; +else + mtd_parts_nb =0 ; +#endif
drivers/mtd/nand/cmx270-nand.c line 246
Quoted 10 lines
drivers/mtd/nand/cmx270-nand.c new file +10
-0+if (mtd_parts_nb ==0 ) + { + mtd_parts = partition_info; + mtd_parts_nb =NUM_PARTITIONS ; + part_type ="static" ; + } + +/* Register the partitions */ +printk (KERN_NOTICE "Using %s partition definition\n" , part_type); +add_mtd_partitions (cmx270_nand_mtd, mtd_parts, mtd_parts_nb);
See above
drivers/mtd/nand/cmx270-nand.c line 256
Quoted 9 lines
drivers/mtd/nand/cmx270-nand.c new file +9
-0+/* Return happy */ +return 0 ; +} +module_init (cmx270_init); + +/* +* Clean up routine +*/ +static void __exit cmx270_cleanup (void)
_cleanup(
drivers/mtd/nand/cmx270-nand.c line 261
Quoted 5 lines
drivers/mtd/nand/cmx270-nand.c new file +5
-0+{ +struct nand_chip *this; + + this = (struct nand_chip *) (&cmx270_nand_mtd[1 ]); +iounmap (this->IO_ADDR_R);
See above
drivers/mtd/nand/cmx270-nand.c line 273
Quoted 11 lines
drivers/mtd/nand/cmx270-nand.c new file +11
-0+/* Release resources, unregister device */ +nand_release (cmx270_nand_mtd); + +/* Free the MTD device structure */ +kfree (cmx270_nand_mtd); +} +module_exit (cmx270_cleanup); + +MODULE_LICENSE ("GPL" ); +MODULE_AUTHOR ("Mike Rapoport <mike at compulab dot co dot il>" ); +MODULE_DESCRIPTION ("NAND flash driver for Compulab CM-X270 Module" );
Can you please combine both drivers into one and make it a platform device. There is no need to keep lots of duplicate functionality around.
tglx
Quoted 22 lines · Thomas Gleixner
Quoted 12 lines · Mike Rapoport
This patch provides MTD support for NAND flash devices on CM-x2xx modules.
Signed-off-by Mike Rapoport <[email protected]>drivers/mtd/nand/cmx255-nand.c line 64
Quoted 6 lines
drivers/mtd/nand/cmx255-nand.c new file +6
-0+#define DRAIN_WB () \ + do { \ + unsigned char dummy; \ + asm volatile ("mcr p15, 0, r0, c7, c10, 4":::"r0"); \ + dummy=*((unsigned char*)UNCACHED_ADDR); \ + } while(0);
Quoted 5 lines
stray semicolon ----^
I bet xscale has this functionality somewhere as a macro / inline already
I found none. Even md() on xscale will not drain write buffer.
Quoted 11 lines
Quoted 5 lines
+ unsigned int nandaddr = (unsigned int)this->IO_ADDR_W;
what the hell is this type cast for ?
void
__iomem*nandaddr =
you can't do |= and &= with void __iomem*
Quoted 5 lines
Can you please combine both drivers into one and make it a platform device. There is no need to keep lots of duplicate functionality around.
I'm dropping cm-x255 support (management decision) so there's only one platform that can use this driver. What's the point then?
Quoted 9 lines
tglx
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Signature · 2 lines
Sincerely yours, Mike Rapoport
Am Dienstag, 11. Juli 2006 09:48 schrieb Mike Rapoport:
Quoted 11 lines
Quoted 10 lines · Thomas Gleixner
Quoted 8 lines · Mike Rapoport
This patch provides MTD support for NAND flash devices on CM-x2xx modules.
drivers/mtd/nand/cmx255-nand.c new file +6
-0+#define DRAIN_WB () \ + do { \ + unsigned char dummy; \ + asm volatile ("mcr p15, 0, r0, c7, c10, 4":::"r0"); \ + dummy=*((unsigned char*)UNCACHED_ADDR); \ + } while(0);
Quoted 5 lines
Quoted 3 lines
stray semicolon ----^
I bet xscale has this functionality somewhere as a macro / inline already
I found none. Even
md()on xscale will not drain write buffer.
dmac_clean_range() from asm/cacheflush.h is what you want.
regards
Signature · 4 lines
Wolfgang Muees Vor den Grashoefen 1 Auerswald GmbH & Co. KG D-38162 Cremlingen Hardware Development Germany Tel +49 5306 9219 0 Fax +49 5306 9219 94
Quoted 21 lines · Wolfgang Mües
Am Dienstag, 11. Juli 2006 09:48 schrieb Mike Rapoport:
Quoted 18 lines
Quoted 15 lines · Thomas Gleixner
drivers/mtd/nand/cmx255-nand.c line 64
Quoted 10 lines · Mike Rapoport
This patch provides MTD support for NAND flash devices on CM-x2xx modules.
drivers/mtd/nand/cmx255-nand.c new file +6
-0+#define DRAIN_WB () \ + do { \ + unsigned char dummy; \ + asm volatile ("mcr p15, 0, r0, c7, c10, 4":::"r0"); \ + dummy=*((unsigned char*)UNCACHED_ADDR); \ + } while(0);stray semicolon ----^
Quoted 10 lines
Quoted 6 lines
Quoted 3 lines
I bet xscale has this functionality somewhere as a macro / inline already
I found none. Even
md()on xscale will not drain write buffer.
dmac_clean_range()fromasm/cacheflush.his what you want.
But I don't need cache cleanup. I need to drain WB to make sure GPIO level has changed. Besides, I cannot use dmac_clean_range() if I want the driver as module
Quoted 4 lines
regards
Signature · 2 lines
Sincerely yours, Mike Rapoport
Quoted 10 lines · Mike Rapoport
Quoted 9 lines
Quoted 3 lines
+ unsigned int nandaddr = (unsigned int)this->IO_ADDR_W;
what the hell is this type cast for ?
void
__iomem*nandaddr =you can't do |= and &= with void __iomem*
Err, why not ?
Quoted 5 lines
Quoted 3 lines
Can you please combine both drivers into one and make it a platform device. There is no need to keep lots of duplicate functionality around.
I'm dropping cm-x255 support (management decision) so there's only one platform that can use this driver. What's the point then?
Sorry man, you submitted _two_ drivers in the first place and I did the review on those. I'm not good at witchcrafting the decisions of your management.
tglx
Quoted 15 lines · Thomas Gleixner
Quoted 12 lines · Mike Rapoport
Quoted 12 lines
Quoted 5 lines
+ unsigned int nandaddr = (unsigned int)this->IO_ADDR_W;
what the hell is this type cast for ?
void
__iomem*nandaddr =
Quoted 7 lines
Quoted 3 lines
you can't do |= and &= with void __iomem*
Err, why not ?
I have this function:
static void cmx270_hwcontrol(struct mtd_info *mtd, int dat,
unsigned int ctrl)
{
struct nand_chip* this = (struct nand_chip *) (mtd->priv);
void __iomem *nandaddr = this->IO_ADDR_W;
if (ctrl & NAND_CTRL_CHANGE) {
if ( ctrl & NAND_ALE )
nandaddr |= (1 << 3);
else
nandaddr &= ~(1 << 3);
if ( ctrl & NAND_CLE )
nandaddr |= (1 << 2);
else
nandaddr &= ~(1 << 2);
if ( ctrl & NAND_NCE )
nand_cs_on();
else
nand_cs_off();
}
this->IO_ADDR_W = (void*)nandaddr;
if (dat != NAND_CMD_NONE)
cmx270_write_byte(mtd, dat);
}And these errors:drivers/mtd/nand/cmx270-nand.c: In function 'cmx270_hwcontrol':drivers/mtd/nand/cmx270-nand.c:137: error: invalid operands to binary | drivers/mtd/nand/cmx270-nand.c:139: error: invalid operands to binary & drivers/mtd/nand/cmx270-nand.c:141: error: invalid operands to binary | drivers/mtd/nand/cmx270-nand.c:143: error: invalid operands to binary &
Quoted 1 line
gcc -v
arm-xscale-linux-gnu-gcc -v
Using built-in specs.
Target: arm-xscale-linux-gnu
...
Thread model: posix
gcc version 4.1.0
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Can you please combine both drivers into one and make it a platform device. There is no need to keep lots of duplicate functionality around.
Quoted 9 lines
Quoted 3 lines
I'm dropping cm-x255 support (management decision) so there's only one platform that can use this driver. What's the point then?
Sorry man, you submitted _two_ drivers in the first place and I did the review on those. I'm not good at witchcrafting the decisions of your management.
You're right, and I agree that for two drivers it makes sence creating platform device and combining the drivers. But now I'm going to submit only _one_ driver. Do you still think platfrom device would be necessary?
Quoted 12 lines
tglx
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Signature · 2 lines
Sincerely yours, Mike Rapoport
Quoted 8 lines · Thomas Gleixner
Quoted 5 lines · Mike Rapoport
This patch provides MTD support for NAND flash devices on CM-x2xx modules.
Signed-off-by Mike Rapoport <[email protected]>
I've tried to fix all the coding style errors.
Quoted 10 lines
Review
Quoted 10 lines
(quoted diff) +6
-0+#define DRAIN_WB() \ + do { \ + unsigned char dummy; \ + asm volatile ("mcr p15, 0, r0, c7, c10, 4":::"r0"); \ + dummy=*((unsigned char*)UNCACHED_ADDR); \ + } while(0);
Quoted 4 lines
I bet xscale has this functionality somewhere as a macro / inline already
There's dma_clean_range but it's not exactly what I need, so I left this macro (without the last semi column)
Quoted 24 lines
Quoted 3 lines
+ unsigned int nandaddr = (unsigned int)this->IO_ADDR_W;
what the hell is this type cast for ?
void
__iomem*nandaddr =Quoted 14 lines
(quoted diff) +14
-0+ unsigned int nandaddr = (unsigned int)this->IO_ADDR_W; + + DRAIN_WB(); + + if ( ctrl & NAND_CTRL_CHANGE ) { + if ( ctrl & NAND_ALE ) /* ALE should be changed */ + nandaddr |= (1 << 3); + else + nandaddr &= ~(1 << 3); + if ( ctrl & NAND_CLE ) /* CLE should be changed */ + nandaddr |= (1 << 2); + else + nandaddr &= ~(1 << 2); + if ( ctrl & NAND_NCE ) /* CE should be changed */
Quoted 9 lines
Review
Quoted 7 lines
(quoted diff) +5
-0+ nand_cs_on(); + else + nand_cs_off(); + } + this->IO_ADDR_W = (void*)nandaddr;dito
As I wrote earlier, void* cannot be operand of bitwise operations. I have ALE and CLE connected to local bus address lines, that's why I'm using bitwise operations for address. I'll appreciate any other way to achieve the same functionality, but I've found none.
Quoted 4 lines
Can you please combine both drivers into one and make it a platform device. There is no need to keep lots of duplicate functionality around.
Sorry for the inconvenience, but I've dropped x255 support, so I don't it makes sense to make the driver platform device.
Quoted 9 lines
tglx
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
Signature · 2 lines
Sincerely yours, Mike Rapoport
drivers/mtd/nand/Kconfig | 4 + drivers/mtd/nand/Makefile | 1 drivers/mtd/nand/cmx270-nand.c | 271 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 276 insertions(+), 0 deletions(-)
drivers/mtd/nand/Kconfig +4 -0
@@ -232,6 +232,10 @@ config MTD_NAND_CS553X If you say "m", the module will be called "cs553x_nand.ko". +config MTD_NAND_CM_X270 + tristate "Support for NAND Flash on CM-X270 modules" + depends on MTD_NAND && MACH_CM_X270 + config MTD_NAND_NANDSIM tristate "Support for NAND Flash Simulator" depends on MTD_NAND && MTD_PARTITIONS
drivers/mtd/nand/Makefile +1 -0
@@ -22,5 +22,6 @@ obj-$(CONFIG_MTD_NAND_TS7250) += ts7250 obj-$(CONFIG_MTD_NAND_NANDSIM) += nandsim.o obj-$(CONFIG_MTD_NAND_CS553X) += cs553x_nand.o obj-$(CONFIG_MTD_NAND_NDFC) += ndfc.o +obj-$(CONFIG_MTD_NAND_CM_X270) += cmx270-nand.o nand-objs = nand_base.o nand_bbt.o
drivers/mtd/nand/cmx270-nand.c new file +271 -0
271 lines not on this page · raw message