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

Wolfgang Denk wd at denx.de
Thu Jan 31 07:48:55 CET 2013


Dear Lubomir Rintel,

In message <1359589584-19846-1-git-send-email-lkundrak at 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.

What exactly would be such cases?

> diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
> index 37b60b8..0d8052d 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)

This change appears to be redundant.  If mtd_type is null, then this
is already caught iun te test mtd_type != MTD_DATAFLASH, isn't it?

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

I don't understand this.  You talk about a MTD device here, but expect
that MEMGETINFO will not work on it?  Please explain in which exact
circumstances such a situation wouldhappen.

>  	if (mtdinfo.type != MTD_NORFLASH &&
>  	    mtdinfo.type != MTD_NANDFLASH &&
> -	    mtdinfo.type != MTD_DATAFLASH) {
> +	    mtdinfo.type != MTD_DATAFLASH &&
> +	    mtdinfo.type) {

Again, this last line appears to be redundant.

>  	}
>  
>  	DEVTYPE(dev_current) = mtdinfo.type;
> -
>  	rc = flash_read_buf(dev_current, fd, environment.image, CUR_ENVSIZE,

Please don't make such unrelated white space changes in this commit.

> 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

I don't see why one would use that.  Please elucidate.


Please also make sure to run your patch through checkpatch - it
catches a number of "space prohibited between function name and open
parenthesis" warnings and tells you that your Signed-off-by: line is
missing.

Best regards,

Wolfgang Denk

-- 
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
Conceptual integrity in turn dictates that the  design  must  proceed
from  one  mind,  or  from  a  very small number of agreeing resonant
minds.               - Frederick Brooks Jr., "The Mythical Man Month"


More information about the U-Boot mailing list