Skip to contentSkip to conversation

Fill in remaining MTD driver data for OneNAND (take #2)

PATCH New Fathi BOUDRA 3 participants 4 messages Aug 6, 2008 – Aug 11, 2008 Memory technology devices (MTD)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fill in remaining MTD driver data for OneNAND (take #2)

Fill in remaining MTD driver data for OneNAND.
Review onenand_print_device_info():
- Return device info to fill mtd device name.
- Remove verbose parameter as it become useless.

Since last comments:
- Include malloc.h
- Initialize dev_info pointer.

---

common/cmd_onenand.c +0 -0
index ce99a38..c9a0627 100644
@@ -38,7 +38,7 @@ int do_onenand(cmd_tbl_t * cmdtp, int flag, int argc, char 

*argv[])
onenand_init();
return 0;
}
- onenand_print_device_info(onenand_chip.device_id, 1);
+ printf("%s\n", onenand_mtd.name);
return 0;

default:

(quoted diff) +0 -0

b/drivers/mtd/onenand/onenand_base.c
index d32e382..8d76724 100644

drivers/mtd/onenand/onenand_base.c +1 -0
@@ -19,6 +19,7 @@
 
 #include <asm/io.h>
 #include <asm/errno.h>
+#include <malloc.h>
 
 /* It should access 16-bit instead of 8-bit */
 static inline void *memcpy_16(void *dst, const void *src, unsigned int len)
@@ -1112,21 +1113,21 @@ int onenand_unlock(struct mtd_info *mtd, loff_t ofs, 

size_t len)
*
* Print device ID
*/
-void onenand_print_device_info(int device, int verbose) +char * onenand_print_device_info(int device)
{
int vcc, demuxed, ddp, density;
-
- if (!verbose)
- return;
+ char *dev_info = (char *)malloc(80);

vcc = device & ONENAND_DEVICE_VCC_MASK;
demuxed = device & ONENAND_DEVICE_IS_DEMUX;
ddp = device & ONENAND_DEVICE_IS_DDP;
density = device >> ONENAND_DEVICE_DENSITY_SHIFT;
- printk(KERN_INFO "%sOneNAND%s %dMB %sV 16-bit (0x%02x)\n",
+ sprintf(dev_info, "%sOneNAND%s %dMB %sV 16-bit (0x%02x)",
demuxed ? "" : "Muxed ",
ddp ? "(DDP)" : "",
(16 << density), vcc ? "2.65/3.3" : "1.8", device);
+
+ return dev_info;
}

static const struct onenand_manufacturers onenand_manuf_ids[] = {
@@ -1205,7 +1206,7 @@ static int onenand_probe(struct mtd_info *mtd)
}

/* Flash device information */
- onenand_print_device_info(dev_id, 0);
+ mtd->name = onenand_print_device_info(dev_id);
this->device_id = dev_id;

density = dev_id >> ONENAND_DEVICE_DENSITY_SHIFT;
@@ -1241,6 +1242,18 @@ static int onenand_probe(struct mtd_info *mtd)
this->options |= ONENAND_CONT_LOCK;
}

+ /* Fill in remaining MTD driver data */
+ mtd->erase = onenand_erase;
+ mtd->read = onenand_read;
+ mtd->write = onenand_write;
+ mtd->read_ecc = onenand_read_ecc;
+ mtd->write_ecc = onenand_write_ecc;
+ mtd->read_oob = onenand_read_oob;
+ mtd->write_oob = onenand_write_oob;
+ mtd->sync = onenand_sync;
+ mtd->block_isbad = onenand_block_isbad;
+ mtd->block_markbad = onenand_block_markbad;
+
return 0;
}

include/onenand_uboot.h +0 -0
index 4449f98..cb7914d 100644
@@ -39,6 +39,6 @@ extern int onenand_erase(struct mtd_info *mtd, struct 

erase_info *instr);

extern int onenand_unlock(struct mtd_info *mtd, loff_t ofs, size_t len);

-extern void onenand_print_device_info(int device, int verbose); +extern char * onenand_print_device_info(int device);

#endif /* __UBOOT_ONENAND_H */

#1 lore raw

PATCH 1/1 Fill in remaining MTD driver data for OneNAND (take #2)

Review

Quoted 2 lines · Fathi BOUDRA
(quoted diff) +1 -1
-void onenand_print_device_info(int device, int verbose)
+char * onenand_print_device_info(int device)

No space after unary '*' (here and elsewhere).

Review

Quoted 6 lines

{

(quoted diff) +1 -3
 	int vcc, demuxed, ddp, density;
-
-	if (!verbose)
-		return;
+	char *dev_info = (char *)malloc(80);

Don't cast the return of malloc.

Why not just declare a static array?

Review

Quoted 5 lines
(quoted diff) +1 -1
-	printk(KERN_INFO "%sOneNAND%s %dMB %sV 16-bit (0x%02x)\n",
+	sprintf(dev_info, "%sOneNAND%s %dMB %sV 16-bit (0x%02x)",
 	       demuxed ? "" : "Muxed ",
 	       ddp ? "(DDP)" : "",
 	       (16 << density), vcc ? "2.65/3.3" : "1.8", device);

It'd be better to use snprintf, even if you're pretty sure it won't overflow.

-Scott

#2 lore raw

PATCH 1/1 Fill in remaining MTD driver data for OneNAND (take #2)

Quoted 9 lines · In message <[email protected]> you wrote:

Review

Quoted 6 lines
(quoted diff) +1 -1
-	printk(KERN_INFO "%sOneNAND%s %dMB %sV 16-bit (0x%02x)\n",
+	sprintf(dev_info, "%sOneNAND%s %dMB %sV 16-bit (0x%02x)",
 	       demuxed ? "" : "Muxed ",
 	       ddp ? "(DDP)" : "",
 	       (16 << density), vcc ? "2.65/3.3" : "1.8", device);

It'd be better to use snprintf, even if you're pretty sure it won't overflow.

This in turn requires that you test the return code for overflowing the buffer - otherwise there is the risk of using an unterminated string.

Best regards,

Wolfgang Denk

Signature · 7 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A good aphorism is too hard for the tooth of time, and  is  not  worn
away  by  all  the  centuries,  although  it serves as food for every
epoch.                                  - Friedrich Wilhelm Nietzsche
                          _Miscellaneous Maxims and Opinions_ no. 168
#3 lore raw

PATCH 1/1 Fill in remaining MTD driver data for OneNAND (take #2)

In reply to Fathi BOUDRA

[resent with the proper list address, sorry for the duplicate]

Quoted 10 lines · Fathi BOUDRA

Fill in remaining MTD driver data for OneNAND.
Review onenand_print_device_info():
- Return device info to fill mtd device name.
- Remove verbose parameter as it become useless.

Since last comments:
- Include malloc.h
- Initialize dev_info pointer.

Applied to nand-flash/testing. Next time, please avoid line-wrapped patches, and put any comments that you don't want to be part of the permanent changelog below the "---" line.

-Scott