Patchwork [U-Boot] env: Allow accessing non-mtd devices

login
register
mail settings
Submitter Lubomir Rintel
Date Feb. 6, 2013, 11:05 p.m.
Message ID <1360191923-4688-1-git-send-email-lkundrak@v3.sk>
Download mbox | patch
Permalink /patch/218794/
State Changes Requested
Delegated to: Wolfgang Denk
Headers show

Comments

Lubomir Rintel - Feb. 6, 2013, 11:05 p.m.
In certain cases, memory device is present as flat file or block device (via
mmc or mtdblock layer). Do not attempt MTD operations against it.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 tools/env/fw_env.c      |   20 ++++++++++++++++----
 tools/env/fw_env.config |    3 +++
 2 files changed, 19 insertions(+), 4 deletions(-)
Wolfgang Denk - Feb. 6, 2013, 11:27 p.m.
Dear Lubomir Rintel,

In message <1360191923-4688-1-git-send-email-lkundrak@v3.sk> you wrote:
> In certain cases, memory device is present as flat file or block device (via
> mmc or mtdblock layer). Do not attempt MTD operations against it.
> 
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
>  tools/env/fw_env.c      |   20 ++++++++++++++++----
>  tools/env/fw_env.config |    3 +++
>  2 files changed, 19 insertions(+), 4 deletions(-)

Arghhh!  NAK.

There is no patch version, no history of changes, nothing.

Please read
http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions
and follow the rules.

Please also see my previous review comments.


Also:

> -	rc = ioctl (fd, MEMGETINFO, &mtdinfo);
> +	rc = fstat(fd, &st);
>  	if (rc < 0) {
> -		perror ("Cannot get MTD information");
> +		perror("Cannot access the device file");
>  		return -1;
>  	}

This error message is still misleading (as you did not use any
access(2) system call in your code); also, the use of perror() is
- let's say - a bit unusual (not your fault in the first place)
and should be fixed; it would be more helpful to print the actual
file name here.



Wolfgang Denk

Patch

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 37b60b8..72d77a9 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -838,7 +838,7 @@  static int flash_write_buf (int dev, int fd, void *buf, size_t count,
 		ioctl (fd, MEMUNLOCK, &erase);
 
 		/* Dataflash does not need an explicit erase cycle */
-		if (mtd_type != MTD_DATAFLASH)
+		if (mtd_type && mtd_type != MTD_DATAFLASH)
 			if (ioctl (fd, MEMERASE, &erase) != 0) {
 				fprintf (stderr, "MTD erase error on %s: %s\n",
 					 DEVNAME (dev),
@@ -949,17 +949,29 @@  static int flash_write (int fd_current, int fd_target, int dev_target)
 static int flash_read (int fd)
 {
 	struct mtd_info_user mtdinfo;
+	struct stat st;
 	int rc;
 
-	rc = ioctl (fd, MEMGETINFO, &mtdinfo);
+	rc = fstat(fd, &st);
 	if (rc < 0) {
-		perror ("Cannot get MTD information");
+		perror("Cannot access the device file");
 		return -1;
 	}
 
+	if (S_ISCHR(st.st_mode)) {
+		rc = ioctl(fd, MEMGETINFO, &mtdinfo);
+		if (rc < 0) {
+			perror("Cannot get MTD information");
+			return -1;
+		}
+	} else {
+		memset(&mtdinfo, 0, sizeof(mtdinfo));
+	}
+
 	if (mtdinfo.type != MTD_NORFLASH &&
 	    mtdinfo.type != MTD_NANDFLASH &&
-	    mtdinfo.type != MTD_DATAFLASH) {
+	    mtdinfo.type != MTD_DATAFLASH &&
+	    mtdinfo.type) {
 		fprintf (stderr, "Unsupported flash type %u\n", mtdinfo.type);
 		return -1;
 	}
diff --git a/tools/env/fw_env.config b/tools/env/fw_env.config
index 8e21d5a..c086512 100644
--- a/tools/env/fw_env.config
+++ b/tools/env/fw_env.config
@@ -17,3 +17,6 @@ 
 
 # NAND example
 #/dev/mtd0		0x4000		0x4000		0x20000			2
+
+# Block device example
+#/dev/mmcblk0		0xc0000		0x20000