[U-Boot] [PATCH] Loop block device for sandbox

[Marek Vasut <marex@denx.de>]

Dear Pavel Herrmann,

> This driver uses files as block devices, can be used for testing disk
> operations on sandbox. Port count and filenames are set in board config.
> 
> Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
> CC: Marek Vasut <marex@denx.de>
> ---
>  drivers/block/Makefile    |   1 +
>  drivers/block/loop.c      | 107
> ++++++++++++++++++++++++++++++++++++++++++++++ include/configs/sandbox.h |
>   9 ++++
>  3 files changed, 117 insertions(+)
>  create mode 100644 drivers/block/loop.c
> 
> diff --git a/drivers/block/Makefile b/drivers/block/Makefile
> index f1ebdcc..5eecf37 100644
> --- a/drivers/block/Makefile
> +++ b/drivers/block/Makefile
> @@ -40,6 +40,7 @@ COBJS-$(CONFIG_SATA_SIL) += sata_sil.o
>  COBJS-$(CONFIG_IDE_SIL680) += sil680.o
>  COBJS-$(CONFIG_SCSI_SYM53C8XX) += sym53c8xx.o
>  COBJS-$(CONFIG_SYSTEMACE) += systemace.o
> +COBJS-${CONFIG_SATA_LOOP} += loop.o

Move this to a more descriptive filename, maybe sata_loopback.c ?

> 
>  COBJS	:= $(COBJS-y)
>  SRCS	:= $(COBJS:.o=.c)
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> new file mode 100644
> index 0000000..c9edfc3
> --- /dev/null
> +++ b/drivers/block/loop.c
> @@ -0,0 +1,107 @@
> +/*
> + * (C) Copyright 2012
> + * Pavel Herrmann <morpheus.ibis@gmail.com>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <common.h>
> +#include <part.h>
> +#include <ata.h>
> +#include <libata.h>
> +#include <errno.h>
> +#include <os.h>
> +
> +static const char revision[] = "0.0";
> +static const char vendor[] = "loopback";
> +
> +static const char * const filenames[] = CONFIG_SATA_LOOP_DISKS;
> +static int max_devs = CONFIG_SYS_SATA_MAX_DEVICE;
> +
> +extern block_dev_desc_t sata_dev_desc[];
> +
> +int init_sata(int dev)
> +{
> +	block_dev_desc_t *pdev = &(sata_dev_desc[dev]);

Superfluous braces ... Actually, I think sata_dev_desc as it would work very 
well too.

> +	int fd;
> +
> +	if ((dev < 0) || (dev >= max_devs)) {
> +		printf("file index %d is out of range\n", dev);

Capital "F" and period at the end of a sentence.

> +		return -EINVAL;
> +	}
> +
> +	fd = os_open(filenames[dev], OS_O_RDWR);
> +	/* this is ugly, but saves allocation for 1 int */

Same here.

> +	pdev->priv = (void *) (long) fd;
> +
> +	return 0;
> +}
> +
> +lbaint_t sata_read(int dev, lbaint_t start, lbaint_t blkcnt, void *buffer)
> +{
> +	block_dev_desc_t *pdev = &(sata_dev_desc[dev]);
> +	int fd = (long) pdev->priv;

If pdev is NULL, this will crash

> +	lbaint_t retval;
> +
> +	os_lseek(fd, start*ATA_SECT_SIZE, OS_SEEK_SET);
> +	retval = os_read(fd, buffer, ATA_SECT_SIZE * blkcnt);
> +
> +	return retval/ATA_SECT_SIZE;
> +}
> +
> +lbaint_t sata_write(int dev, lbaint_t start, lbaint_t blkcnt, void
> *buffer) +{
> +	block_dev_desc_t *pdev = &(sata_dev_desc[dev]);
> +	int fd = (long) pdev->priv;
> +	lbaint_t retval;
> +
> +	os_lseek(fd, start*ATA_SECT_SIZE, OS_SEEK_SET);

Please do the multiplication in a consistent manner, same for division etc. ... 
like you do on the following line (x1 [space] oper [space] x2).

Besides, lseek can fail, can it not?

> +	retval = os_write(fd, buffer, ATA_SECT_SIZE * blkcnt);
> +
> +	return retval/ATA_SECT_SIZE;
> +}
> +
> +int scan_sata(int dev)
> +{
> +	block_dev_desc_t *pdev = &(sata_dev_desc[dev]);
> +	int fd = (long) pdev->priv;
> +	int namelen;
> +	lbaint_t bytes = 0;

newline here

> +	memcpy(pdev->vendor, vendor, sizeof(vendor));
> +	memcpy(pdev->revision, revision, sizeof(revision));
> +	namelen = sizeof(filenames[dev]);

newline here

> +	if (namelen > 20)
> +		namelen = 20;

Why do you trim down the string, won't simple strdup() work?

newline here

> +	memcpy(pdev->product, filenames[dev], namelen);
> +	pdev->product[20] = 0;
> +
> +	if (fd != -1) {

And if "fd" is -1 ?

> +		pdev->type = DEV_TYPE_HARDDISK;
> +		pdev->blksz = ATA_SECT_SIZE;
> +		pdev->lun = 0;
> +
> +		bytes = os_lseek(fd, 0, OS_SEEK_END);
> +		pdev->lba = bytes/ATA_SECT_SIZE;
> +	}
> +
> +	printf("SATA loop info:\nfilename: %s\nsize: %lu\nblock count: %lu\n",
> +		filenames[dev], bytes, pdev->lba);
> +
> +	return 0;
> +}
> diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
> index 0220386..412341f 100644
> --- a/include/configs/sandbox.h
> +++ b/include/configs/sandbox.h
> @@ -93,4 +93,13 @@
>  					"stdout=serial\0" \
>  					"stderr=serial\0"
> 
> +/* SATA loopback device */
> +#define CONFIG_CMD_SATA
> +#define CONFIG_SATA_LOOP
> +#define CONFIG_SATA_LOOP_DISKS {"disk1", "disk2"}
> +#define CONFIG_SYS_SATA_MAX_DEVICE 2
> +#define CONFIG_DOS_PARTITION
> +#define CONFIG_CMD_FAT
> +#define CONFIG_CMD_EXT2

Move this to a separate patch.

>  #endif

Best regards,
Marek Vasut