Remove hardcoded number of CFI chips
PATCH 1/1 Remove hardcoded number of CFI chips
All,
The attached patch is an update of the patch I posted about a month ago [1] that removes the hardcoded CFI_MAX_CHIPS, which was originally written by Andrzej Mialkowski @ Intel. This patch fixes two issues:
- Get rid of extra structure probe_info that was just carrying around a long* variable.
- Fix bug in bitmap size on systems with 2-8 chips. The old patch had:
+ max_chips = map->size >> cfi.chipshift; + probe.chip_map = kmalloc((max_chips + 7) / 8, GFP_KERNEL);
This would cause systems with 2-8 chips to be allocated only one byte for the bitmap instead of the 2 needed. Fixed code does:
+ max_chips = map->size >> cfi.chipshift;
+ chip_map = kmalloc((max_chips / 8) + ((max_chip % 8) ? 1 : 0), GFP_KERNEL);
Probably a cleaner way to do this. We could just use a u32 of this kmalloc which would give us a limit of 32 chips, which _should_ be OK for most systems and when we need more, just move to a u64. If this is prefered, I'll update the patch.
Comments?
[1]: http://lists.infradead.org/pipermail/linux-mtd/2003-October/008608.html
Index: drivers/mtd/chips/cfi_probe.c
===================================================================
RCS file: /home/cvs/mtd/drivers/mtd/chips/cfi_probe.c,v retrieving revision 1.72
mtd/chips/cfi_probe.c +15 -15
@@ -26,7 +26,7 @@#endif static int cfi_probe_chip (struct map_info *map,__u32 base, -struct flchip *chips,struct cfi_private *cfi); +unsigned long *chip_map,struct cfi_private *cfi);static int cfi_chip_setup (struct map_info *map,struct cfi_private *cfi);struct mtd_info *cfi_probe (struct map_info *map);
@@ -49,7 +49,7 @@ }static int cfi_probe_chip (struct map_info *map,__u32 base, -struct flchip *chips,struct cfi_private *cfi) +unsigned long *chip_map,struct cfi_private *cfi) {int i;
@@ -78,18 +78,24 @@ }/* Check each previous chip to see if it's an alias */ -for (i=0 ; i<cfi->numchips; i++) { +for (i=0 ; i < (base >> cfi->chipshift); i++) { +unsigned long start; +if (!test_bit (i, chip_map)) { +/* Skip location; no valid chip at this address */ +continue ; + } + start = i << cfi->chipshift;/* This chip should be in read mode if it's one we've already touched. */ -if (qry_present (map,chips[i].start,cfi)) { +if (qry_present (map, start, cfi)) {/* Eep. This chip also had the QRY marker. * Is it an alias for the new one? */ -cfi_send_gen_cmd (0xF0 ,0 , chips[i].start, map, cfi, cfi->device_type,NULL ); +cfi_send_gen_cmd (0xF0 ,0 , start, map, cfi, cfi->device_type,NULL );/* If the QRY marker goes away, it's an alias */ -if (!qry_present (map, chips[i].start, cfi)) { +if (!qry_present (map, start, cfi)) {printk (KERN_DEBUG "%s: Found an alias at 0x%x for the chip at 0x%lx\n" , - map->name, base, chips[i].start); + map->name, base, start);return 0 ; }/* Yes, it's actually got QRY for data. Most
@@ -100,7 +106,7 @@if (qry_present (map, base, cfi)) {printk (KERN_DEBUG "%s: Found an alias at 0x%x for the chip at 0x%lx\n" , - map->name, base, chips[i].start); + map->name, base, start);return 0 ; } }
@@ -108,13 +114,7 @@/* OK, if we got to here, then none of the previous chips appear to be aliases for the current one. */ -if (cfi->numchips ==MAX_CFI_CHIPS ) { -printk (KERN_WARNING "%s: Too many flash chips detected. Increase MAX_CFI_CHIPS from %d.\n" , map->name,MAX_CFI_CHIPS ); -/* Doesn't matter about resetting it to Read Mode - we're not going to talk to it anyway */ -return -1 ; - } - chips[cfi->numchips].start = base; - chips[cfi->numchips].state =FL_READY ; +set_bit ((base >> cfi->chipshift), chip_map);/* Update chip map */ cfi->numchips++;/* Put it back into Read Mode */
Index: drivers/mtd/chips/gen_probe.c
===================================================================
RCS file: /home/cvs/mtd/drivers/mtd/chips/gen_probe.c,v retrieving revision 1.13
mtd/chips/gen_probe.c +33 -21
95 lines not on this page · raw message
Index: drivers/mtd/chips/jedec_probe.c
===================================================================
RCS file: /home/cvs/mtd/drivers/mtd/chips/jedec_probe.c,v retrieving revision 1.38
mtd/chips/jedec_probe.c +15 -18
@@ -1416,7 +1416,7 @@static int cfi_jedec_setup (struct cfi_private *p_cfi,int index);static int jedec_probe_chip (struct map_info *map,__u32 base, -struct flchip *chips,struct cfi_private *cfi); +unsigned long *chip_map,struct cfi_private *cfi);struct mtd_info *jedec_probe (struct map_info *map);
@@ -1644,7 +1644,7 @@static int jedec_probe_chip (struct map_info *map,__u32 base, -struct flchip *chips,struct cfi_private *cfi) +unsigned long *chip_map,struct cfi_private *cfi) {int i;enum uaddr uaddr_idx =MTD_UADDR_NOT_SUPPORTED ;
@@ -1731,21 +1731,24 @@ } } -/* Check each previous chip to see if it's an alias */ -for (i=0 ; i<cfi->numchips; i++) { -/* This chip should be in read mode if it's one -we've already touched. */ -if (jedec_read_mfr (map, chips[i].start, cfi) == cfi->mfr && -jedec_read_id (map, chips[i].start, cfi) == cfi->id) { +/* Check each previous chip locations to see if it's an alias */ +for (i=0 ; i < (base >> cfi->chipshift); i++) { +unsigned long start; +if (!test_bit (i, chip_map)) { +continue ;/* Skip location; no valid chip at this address */ + } + start = i << cfi->chipshift; +if (jedec_read_mfr (map, start, cfi) == cfi->mfr && +jedec_read_id (map, start, cfi) == cfi->id) {/* Eep. This chip also looks like it's in autoselect mode. Is it an alias for the new one? */ -jedec_reset (chips[i].start, map, cfi); +jedec_reset (start, map, cfi);/* If the device IDs go away, it's an alias */ if (jedec_read_mfr (map, base, cfi) != cfi->mfr ||jedec_read_id (map, base, cfi) != cfi->id) {printk (KERN_DEBUG "%s: Found an alias at 0x%x for the chip at 0x%lx\n" , - map->name, base, chips[i].start); + map->name, base, start);return 0 ; }
@@ -1757,7 +1760,7 @@if (jedec_read_mfr (map, base, cfi) == cfi->mfr &&jedec_read_id (map, base, cfi) == cfi->id) {printk (KERN_DEBUG "%s: Found an alias at 0x%x for the chip at 0x%lx\n" , - map->name, base, chips[i].start); + map->name, base, start);return 0 ; } }
@@ -1765,13 +1768,7 @@/* OK, if we got to here, then none of the previous chips appear to be aliases for the current one. */ -if (cfi->numchips ==MAX_CFI_CHIPS ) { -printk (KERN_WARNING "%s: Too many flash chips detected. Increase MAX_CFI_CHIPS from %d.\n" , map->name,MAX_CFI_CHIPS ); -/* Doesn't matter about resetting it to Read Mode - we're not going to talk to it anyway */ -return -1 ; - } - chips[cfi->numchips].start = base; - chips[cfi->numchips].state =FL_READY ; +set_bit ((base >> cfi->chipshift), chip_map);/* Update chip map */ cfi->numchips++; ok_out:
Index: include/linux/mtd/cfi.h
===================================================================
RCS file: /home/cvs/mtd/include/linux/mtd/cfi.h,v
retrieving revision 1.37
linux/mtd/cfi.h +0 -2
@@ -335,8 +335,6 @@struct flchip chips[0 ];/* per-chip data structure for each chip */ }; -#define MAX_CFI_CHIPS 8/* Entirely arbitrary to avoid realloc() */ -/* * Returns the command address according to the given geometry. */
Index: include/linux/mtd/gen_probe.h
===================================================================
RCS file: /home/cvs/mtd/include/linux/mtd/gen_probe.h,v retrieving revision 1.1
linux/mtd/gen_probe.h +2 -2
@@ -10,12 +10,12 @@#include <linux/mtd/flashchip.h> #include <linux/mtd/map.h> #include <linux/mtd/cfi.h> +#include <asm/bitops.h> struct chip_probe {char *name;int (*probe_chip)(struct map_info *map,__u32 base, -struct flchip *chips,struct cfi_private *cfi); - +unsigned long *chip_map,struct cfi_private *cfi); };struct mtd_info *mtd_do_chip_probe (struct map_info *map,struct chip_probe *cp);
Signature · 6 lines
Deepak Saxena - [email protected] "To eliminate the concept of waste means to design things - products, packaging, and systems - from the very beggining on the understanding that waste does not exist" - William McDonough & Michael Braungart, from Cradle to Cradle: Remaking the Way We Make Things
Cytowanie Deepak Saxena <[email protected]>:
Quoted 9 lines
All,
The attached patch is an update of the patch I posted about a month ago [1] that removes the hardcoded
CFI_MAX_CHIPS, which was originally written by Andrzej Mialkowski @ Intel. This patch fixes two issues:- Get rid of extra structure
probe_infothat was just carrying around a long* variable.
This 'stupid' single field structure was intentional to allow easier expansion if somebody fill need for additional fields.
Quoted 6 lines
- Fix bug in bitmap size on systems with 2-8 chips. The old patch had:
+ max_chips = map->size >> cfi.chipshift; + probe.chip_map = kmalloc((max_chips + 7) / 8, GFP_KERNEL);
Quoted 7 lines
This would cause systems with 2-8 chips to be allocated only one byte for the bitmap instead of the 2 needed. Fixed code does:
+
max_chips=map->size>> cfi.chipshift;
+chip_map= kmalloc((max_chips / 8) + ((max_chip% 8) ? 1 : 0),GFP_KERNEL);
Why I need two bytes to store up to 8 bits? By the way 2/8 + 2?1:0 = 1. Initially I thought to put higher alignment here for instance ((max_chips +
31) / 32)*4) but found no reason to do this. Maybe I was wrong...
Andrzej