| Message ID | 1346843795-4744-1-git-send-email-morpheus.ibis@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
Dear Pavel Herrmann, > This driver uses files as block devices, can be used for testing disk > operations on sandbox. > A new command "sata_loop" is introduced to load files in runtime. > > Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com> > CC: Marek Vasut <marex@denx.de> > CC: Mike Frysinger <vapier@gentoo.org> ERROR: "foo * bar" should be "foo *bar" #136: FILE: drivers/block/sata_loopback.c:36: +static char * filenames[CONFIG_SYS_SATA_MAX_DEVICE]; WARNING: externs should be avoided in .c files #138: FILE: drivers/block/sata_loopback.c:38: +extern block_dev_desc_t sata_dev_desc[]; ERROR: do not initialise statics to 0 or NULL #142: FILE: drivers/block/sata_loopback.c:42: + static int zeroed = 0; ERROR: spaces required around that '=' (ctx:VxV) #150: FILE: drivers/block/sata_loopback.c:50: + filenames[i]=strdup(""); ^ ERROR: space prohibited after that open parenthesis '(' #181: FILE: drivers/block/sata_loopback.c:81: + if ( os_lseek(fd, start_byte, OS_SEEK_SET) != start_byte) ERROR: space prohibited after that open parenthesis '(' #197: FILE: drivers/block/sata_loopback.c:97: + if ( os_lseek(fd, start_byte, OS_SEEK_SET) != start_byte) ERROR: do not initialise statics to 0 or NULL #256: FILE: drivers/block/sata_loopback.c:156: + static int zeroed = 0; total: 6 errors, 1 warnings, 207 lines checked Checkpatch and I never sleep, don't even try ;-) > --- > Changes for v3: > introduce sata_loop command > > Changes for v2: > split sandbox config off into separate patch (2/2) > rename file to signify exported API > style fixes > show end of long filenames rather than beginning > check for lseek errors to indicate non-regular file > > drivers/block/Makefile | 1 + > drivers/block/sata_loopback.c | 200 > ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 201 > insertions(+) > create mode 100644 drivers/block/sata_loopback.c > > diff --git a/drivers/block/Makefile b/drivers/block/Makefile > index f1ebdcc..c95651a 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} += sata_loopback.o > > COBJS := $(COBJS-y) > SRCS := $(COBJS:.o=.c) > diff --git a/drivers/block/sata_loopback.c b/drivers/block/sata_loopback.c > new file mode 100644 > index 0000000..0e6923b > --- /dev/null > +++ b/drivers/block/sata_loopback.c > @@ -0,0 +1,200 @@ > +/* > + * (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> > +#include <command.h> > +#include <malloc.h> > + > +static const char revision[] = "0.0"; > +static const char vendor[] = "SATA loopback"; > + > +static char * filenames[CONFIG_SYS_SATA_MAX_DEVICE]; > + > +extern block_dev_desc_t sata_dev_desc[]; > + > +int init_sata(int dev) > +{ > + static int zeroed = 0; Is this really needed here and below? Pull this crap out. > + block_dev_desc_t *pdev = &sata_dev_desc[dev]; > + int fd, old_fd; > + > + Redundant newline > + if (!zeroed) { > + int i; > + for (i = 0; i < CONFIG_SYS_SATA_MAX_DEVICE; i++) > + filenames[i]=strdup(""); > + zeroed = 1; > + } > + > + if ((dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE)) { > + printf("File index %d is out of range.\n", dev); > + return -EINVAL; > + } > + > + fd = os_open(filenames[dev], OS_O_RDWR); > + /* This is ugly, but saves allocation for 1 int. */ > + old_fd = (long) pdev->priv; > + pdev->priv = (void *) (long) fd; > + /* > + * sadly we cannot set -1 to all as above, because "sata init" will zero > + * this value before calling sata_init. Sorry, I can't parse this. > + */ > + if ((old_fd > 2) || (old_fd < 0)) > + os_close(old_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; > + lbaint_t start_byte = ATA_SECT_SIZE * start; > + lbaint_t length_byte = ATA_SECT_SIZE * blkcnt; > + lbaint_t retval; > + > + if ( os_lseek(fd, start_byte, OS_SEEK_SET) != start_byte) > + return -1; > + > + retval = os_read(fd, buffer, length_byte); > + > + 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 start_byte = ATA_SECT_SIZE * start; > + lbaint_t length_byte = ATA_SECT_SIZE * blkcnt; > + lbaint_t retval; > + > + if ( os_lseek(fd, start_byte, OS_SEEK_SET) != start_byte) > + return -1; > + > + retval = os_write(fd, buffer, length_byte); > + > + return retval/ATA_SECT_SIZE; a[space]operator[space]b ... did you ignore all my previous comments? > +} > + > +int scan_sata(int dev) > +{ > + block_dev_desc_t *pdev = &sata_dev_desc[dev]; > + int fd = (long) pdev->priv; > + int namelen; > + const char *filename = filenames[dev]; > + lbaint_t bytes = 0; > + > + if ((dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE)) { > + printf("File index %d is out of range.\n", dev); > + return -EINVAL; > + } > + > + memcpy(pdev->vendor, vendor, sizeof(vendor)); > + memcpy(pdev->revision, revision, sizeof(revision)); > + namelen = strlen(filenames[dev]); > + > + if (namelen > 20) { > + /* take the last 17 chars, prepend them with "..." */ > + filename += (namelen - 17); > + memcpy(pdev->product, "...", 3); > + memcpy(pdev->product+3, filename, 17); > + } else > + memcpy(pdev->product, filename, namelen); > + > + pdev->product[20] = 0; > + > + bytes = os_lseek(fd, 0, OS_SEEK_END); > + if (bytes != -1) { > + pdev->type = DEV_TYPE_HARDDISK; > + pdev->blksz = ATA_SECT_SIZE; > + pdev->lun = 0; > + pdev->lba = bytes/ATA_SECT_SIZE; > + printf("SATA loop %d:\nfilename: %s\nsize: %lu\nblock count:" > + " %lu\n", dev, filenames[dev], bytes, pdev->lba); > + } else { > + pdev->type = DEV_TYPE_HARDDISK; > + pdev->blksz = ATA_SECT_SIZE; > + pdev->lun = 0; > + pdev->lba = 0; > + printf("SATA loop %d:\nfilename: %s\nFAILED TO OPEN\n", > + dev, filenames[dev]); > + } > + > + return 0; > +} > + > + > +int do_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > +{ > + int dev = 0; > + static int zeroed = 0; move this out. besides, I think it'd be much systematic to just scream at user to call "sata rescan" and bail out instead of doing it for him. > + /* make sure we have valid filenames */ > + if (!zeroed) { > + init_sata(0); > + zeroed = 1; > + } > + > + switch (argc) { > + case 0: > + case 1: > + return CMD_RET_USAGE; > + case 2: > + dev = simple_strtoul(argv[1], NULL, 10); Ok, so if I run this command and ask for device 0xb00bf33d ... will this survive? Hint: it won't, rangecheck missing. > + return scan_sata(dev); > + > + case 3: > + if (!strncmp(argv[1], "inf", 3)) { > + dev = simple_strtoul(argv[2], NULL, 10); Same here > + return scan_sata(dev); > + } > + return CMD_RET_USAGE; > + case 4: > + if (!strncmp(argv[1], "load", 4)) { > + dev = simple_strtoul(argv[2], NULL, 10); > + if ((dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE)) { And here you have it ? Uh oh, I see, sata_scan() does it for you ... I'd say, abstract it out into lightweight static inline function. > + printf("File index %d is out of range.\n", dev); > + return -EINVAL; > + } > + free(filenames[dev]); > + filenames[dev] = strdup(argv[3]); > + init_sata(dev); > + return scan_sata(dev); > + } > + return CMD_RET_USAGE; > + } > + return CMD_RET_USAGE; > +} > + > +U_BOOT_CMD( > + sata_loop, 4, 1, do_loop, > + "SATA loopback", > + "[info] devnum - show info about loop devnum\n" Make this "info" part mandatory. Than you can cut the whole argc loop into simple "if argc != 2 ; then fail" . And do simple checking for the first letter of the argument being either i or d . > + "sata_loop load devnum file - load file from host FS into loop devnum" sata_loop is redundant above. > +);
On Wednesday 05 September 2012 13:33:13 Marek Vasut wrote: > Dear Pavel Herrmann, > > > This driver uses files as block devices, can be used for testing disk > > operations on sandbox. > > A new command "sata_loop" is introduced to load files in runtime. > > > > Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com> > > CC: Marek Vasut <marex@denx.de> > > CC: Mike Frysinger <vapier@gentoo.org> > > ERROR: "foo * bar" should be "foo *bar" > #136: FILE: drivers/block/sata_loopback.c:36: > +static char * filenames[CONFIG_SYS_SATA_MAX_DEVICE]; > > WARNING: externs should be avoided in .c files > #138: FILE: drivers/block/sata_loopback.c:38: > +extern block_dev_desc_t sata_dev_desc[]; > > ERROR: do not initialise statics to 0 or NULL > #142: FILE: drivers/block/sata_loopback.c:42: > + static int zeroed = 0; > > ERROR: spaces required around that '=' (ctx:VxV) > #150: FILE: drivers/block/sata_loopback.c:50: > + filenames[i]=strdup(""); > ^ > > ERROR: space prohibited after that open parenthesis '(' > #181: FILE: drivers/block/sata_loopback.c:81: > + if ( os_lseek(fd, start_byte, OS_SEEK_SET) != start_byte) > > ERROR: space prohibited after that open parenthesis '(' > #197: FILE: drivers/block/sata_loopback.c:97: > + if ( os_lseek(fd, start_byte, OS_SEEK_SET) != start_byte) > > ERROR: do not initialise statics to 0 or NULL > #256: FILE: drivers/block/sata_loopback.c:156: > + static int zeroed = 0; > > total: 6 errors, 1 warnings, 207 lines checked > > Checkpatch and I never sleep, don't even try ;-) > sorry, my bad, i thought i had it on pre-commit > > --- > > > > Changes for v3: > > introduce sata_loop command > > > > Changes for v2: > > split sandbox config off into separate patch (2/2) > > rename file to signify exported API > > style fixes > > show end of long filenames rather than beginning > > check for lseek errors to indicate non-regular file > > > > drivers/block/Makefile | 1 + > > drivers/block/sata_loopback.c | 200 > > > > ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 201 > > insertions(+) > > > > create mode 100644 drivers/block/sata_loopback.c > > > > diff --git a/drivers/block/Makefile b/drivers/block/Makefile > > index f1ebdcc..c95651a 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} += sata_loopback.o > > > > COBJS := $(COBJS-y) > > SRCS := $(COBJS:.o=.c) > > > > diff --git a/drivers/block/sata_loopback.c b/drivers/block/sata_loopback.c > > new file mode 100644 > > index 0000000..0e6923b > > --- /dev/null > > +++ b/drivers/block/sata_loopback.c > > @@ -0,0 +1,200 @@ > > +/* > > + * (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> > > +#include <command.h> > > +#include <malloc.h> > > + > > +static const char revision[] = "0.0"; > > +static const char vendor[] = "SATA loopback"; > > + > > +static char * filenames[CONFIG_SYS_SATA_MAX_DEVICE]; > > + > > +extern block_dev_desc_t sata_dev_desc[]; > > + > > +int init_sata(int dev) > > +{ > > + static int zeroed = 0; > > Is this really needed here and below? Pull this crap out. yes, because you dont know whetehr this gets called first from sata_loop or from "sata init" > > + block_dev_desc_t *pdev = &sata_dev_desc[dev]; > > + int fd, old_fd; > > + > > + > > Redundant newline > > > + if (!zeroed) { > > + int i; > > + for (i = 0; i < CONFIG_SYS_SATA_MAX_DEVICE; i++) > > + filenames[i]=strdup(""); > > + zeroed = 1; > > + } > > + > > + if ((dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE)) { > > + printf("File index %d is out of range.\n", dev); > > + return -EINVAL; > > + } > > + > > + fd = os_open(filenames[dev], OS_O_RDWR); > > + /* This is ugly, but saves allocation for 1 int. */ > > + old_fd = (long) pdev->priv; > > + pdev->priv = (void *) (long) fd; > > + /* > > + * sadly we cannot set -1 to all as above, because "sata init" will zero > > + * this value before calling sata_init. > > Sorry, I can't parse this. > > > + */ > > + if ((old_fd > 2) || (old_fd < 0)) > > + os_close(old_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; > > + lbaint_t start_byte = ATA_SECT_SIZE * start; > > + lbaint_t length_byte = ATA_SECT_SIZE * blkcnt; > > + lbaint_t retval; > > + > > + if ( os_lseek(fd, start_byte, OS_SEEK_SET) != start_byte) > > + return -1; > > + > > + retval = os_read(fd, buffer, length_byte); > > + > > + 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 start_byte = ATA_SECT_SIZE * start; > > + lbaint_t length_byte = ATA_SECT_SIZE * blkcnt; > > + lbaint_t retval; > > + > > + if ( os_lseek(fd, start_byte, OS_SEEK_SET) != start_byte) > > + return -1; > > + > > + retval = os_write(fd, buffer, length_byte); > > + > > + return retval/ATA_SECT_SIZE; > > a[space]operator[space]b ... did you ignore all my previous comments? > > > +} > > + > > +int scan_sata(int dev) > > +{ > > + block_dev_desc_t *pdev = &sata_dev_desc[dev]; > > + int fd = (long) pdev->priv; > > + int namelen; > > + const char *filename = filenames[dev]; > > + lbaint_t bytes = 0; > > + > > + if ((dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE)) { > > + printf("File index %d is out of range.\n", dev); > > + return -EINVAL; > > + } > > + > > + memcpy(pdev->vendor, vendor, sizeof(vendor)); > > + memcpy(pdev->revision, revision, sizeof(revision)); > > + namelen = strlen(filenames[dev]); > > + > > + if (namelen > 20) { > > + /* take the last 17 chars, prepend them with "..." */ > > + filename += (namelen - 17); > > + memcpy(pdev->product, "...", 3); > > + memcpy(pdev->product+3, filename, 17); > > + } else > > + memcpy(pdev->product, filename, namelen); > > + > > + pdev->product[20] = 0; > > + > > + bytes = os_lseek(fd, 0, OS_SEEK_END); > > + if (bytes != -1) { > > + pdev->type = DEV_TYPE_HARDDISK; > > + pdev->blksz = ATA_SECT_SIZE; > > + pdev->lun = 0; > > + pdev->lba = bytes/ATA_SECT_SIZE; > > + printf("SATA loop %d:\nfilename: %s\nsize: %lu\nblock count:" > > + " %lu\n", dev, filenames[dev], bytes, pdev->lba); > > + } else { > > + pdev->type = DEV_TYPE_HARDDISK; > > + pdev->blksz = ATA_SECT_SIZE; > > + pdev->lun = 0; > > + pdev->lba = 0; > > + printf("SATA loop %d:\nfilename: %s\nFAILED TO OPEN\n", > > + dev, filenames[dev]); > > + } > > + > > + return 0; > > +} > > + > > + > > +int do_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > > +{ > > + int dev = 0; > > + static int zeroed = 0; > > move this out. > > besides, I think it'd be much systematic to just scream at user to call > "sata rescan" and bail out instead of doing it for him. i dont actually need a sata rescan, i just need to make sure i have dynamically allocated names, so i can safely call free() later, otherwise this segfaults when called before sata scan > > + /* make sure we have valid filenames */ > > + if (!zeroed) { > > + init_sata(0); > > + zeroed = 1; > > + } > > + > > + switch (argc) { > > + case 0: > > + case 1: > > + return CMD_RET_USAGE; > > + case 2: > > + dev = simple_strtoul(argv[1], NULL, 10); > > Ok, so if I run this command and ask for device 0xb00bf33d ... will this > survive? Hint: it won't, rangecheck missing. > > > + return scan_sata(dev); > > + > > + case 3: > > + if (!strncmp(argv[1], "inf", 3)) { > > + dev = simple_strtoul(argv[2], NULL, 10); > > Same here > > > + return scan_sata(dev); > > + } > > + return CMD_RET_USAGE; > > + case 4: > > + if (!strncmp(argv[1], "load", 4)) { > > + dev = simple_strtoul(argv[2], NULL, 10); > > + if ((dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE)) { > > And here you have it ? > > Uh oh, I see, sata_scan() does it for you ... I'd say, abstract it out into > lightweight static inline function. > > > + printf("File index %d is out of range.\n", dev); > > + return -EINVAL; > > + } > > + free(filenames[dev]); > > + filenames[dev] = strdup(argv[3]); > > + init_sata(dev); > > + return scan_sata(dev); > > + } > > + return CMD_RET_USAGE; > > + } > > + return CMD_RET_USAGE; > > +} > > + > > +U_BOOT_CMD( > > + sata_loop, 4, 1, do_loop, > > + "SATA loopback", > > + "[info] devnum - show info about loop devnum\n" > > Make this "info" part mandatory. Than you can cut the whole argc loop into > simple "if argc != 2 ; then fail" . And do simple checking for the first > letter of the argument being either i or d . > > > + "sata_loop load devnum file - load file from host FS into loop devnum" > > sata_loop is redundant above. > > > +);
second try... On Wednesday 05 September 2012 13:33:13 Marek Vasut wrote: > Dear Pavel Herrmann, > > > This driver uses files as block devices, can be used for testing disk > > operations on sandbox. > > A new command "sata_loop" is introduced to load files in runtime. > > > > Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com> > > CC: Marek Vasut <marex@denx.de> > > CC: Mike Frysinger <vapier@gentoo.org> > > ERROR: "foo * bar" should be "foo *bar" > #136: FILE: drivers/block/sata_loopback.c:36: > +static char * filenames[CONFIG_SYS_SATA_MAX_DEVICE]; > > WARNING: externs should be avoided in .c files > #138: FILE: drivers/block/sata_loopback.c:38: > +extern block_dev_desc_t sata_dev_desc[]; > > ERROR: do not initialise statics to 0 or NULL > #142: FILE: drivers/block/sata_loopback.c:42: > + static int zeroed = 0; > > ERROR: spaces required around that '=' (ctx:VxV) > #150: FILE: drivers/block/sata_loopback.c:50: > + filenames[i]=strdup(""); > ^ > > ERROR: space prohibited after that open parenthesis '(' > #181: FILE: drivers/block/sata_loopback.c:81: > + if ( os_lseek(fd, start_byte, OS_SEEK_SET) != start_byte) > > ERROR: space prohibited after that open parenthesis '(' > #197: FILE: drivers/block/sata_loopback.c:97: > + if ( os_lseek(fd, start_byte, OS_SEEK_SET) != start_byte) > > ERROR: do not initialise statics to 0 or NULL > #256: FILE: drivers/block/sata_loopback.c:156: > + static int zeroed = 0; > > total: 6 errors, 1 warnings, 207 lines checked > > Checkpatch and I never sleep, don't even try ;-) > sorry, my bad, i thought i had it on pre-commit > > --- > > > > Changes for v3: > > introduce sata_loop command > > > > Changes for v2: > > split sandbox config off into separate patch (2/2) > > rename file to signify exported API > > style fixes > > show end of long filenames rather than beginning > > check for lseek errors to indicate non-regular file > > > > drivers/block/Makefile | 1 + > > drivers/block/sata_loopback.c | 200 > > > > ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 201 > > insertions(+) > > > > create mode 100644 drivers/block/sata_loopback.c > > > > diff --git a/drivers/block/Makefile b/drivers/block/Makefile > > index f1ebdcc..c95651a 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} += sata_loopback.o > > > > COBJS := $(COBJS-y) > > SRCS := $(COBJS:.o=.c) > > > > diff --git a/drivers/block/sata_loopback.c b/drivers/block/sata_loopback.c > > new file mode 100644 > > index 0000000..0e6923b > > --- /dev/null > > +++ b/drivers/block/sata_loopback.c > > @@ -0,0 +1,200 @@ > > +/* > > + * (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> > > +#include <command.h> > > +#include <malloc.h> > > + > > +static const char revision[] = "0.0"; > > +static const char vendor[] = "SATA loopback"; > > + > > +static char * filenames[CONFIG_SYS_SATA_MAX_DEVICE]; > > + > > +extern block_dev_desc_t sata_dev_desc[]; > > + > > +int init_sata(int dev) > > +{ > > + static int zeroed = 0; > > Is this really needed here and below? Pull this crap out. yes, because you dont know whether this gets called first from sata_loop or from "sata init" > > +int scan_sata(int dev) > > +{ > > + block_dev_desc_t *pdev = &sata_dev_desc[dev]; > > + int fd = (long) pdev->priv; > > + int namelen; > > + const char *filename = filenames[dev]; > > + lbaint_t bytes = 0; > > + > > + if ((dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE)) { > > + printf("File index %d is out of range.\n", dev); > > + return -EINVAL; > > + } > > + > > + memcpy(pdev->vendor, vendor, sizeof(vendor)); > > + memcpy(pdev->revision, revision, sizeof(revision)); > > + namelen = strlen(filenames[dev]); > > + > > + if (namelen > 20) { > > + /* take the last 17 chars, prepend them with "..." */ > > + filename += (namelen - 17); > > + memcpy(pdev->product, "...", 3); > > + memcpy(pdev->product+3, filename, 17); > > + } else > > + memcpy(pdev->product, filename, namelen); > > + > > + pdev->product[20] = 0; > > + > > + bytes = os_lseek(fd, 0, OS_SEEK_END); > > + if (bytes != -1) { > > + pdev->type = DEV_TYPE_HARDDISK; > > + pdev->blksz = ATA_SECT_SIZE; > > + pdev->lun = 0; > > + pdev->lba = bytes/ATA_SECT_SIZE; > > + printf("SATA loop %d:\nfilename: %s\nsize: %lu\nblock count:" > > + " %lu\n", dev, filenames[dev], bytes, pdev->lba); > > + } else { > > + pdev->type = DEV_TYPE_HARDDISK; > > + pdev->blksz = ATA_SECT_SIZE; > > + pdev->lun = 0; > > + pdev->lba = 0; > > + printf("SATA loop %d:\nfilename: %s\nFAILED TO OPEN\n", > > + dev, filenames[dev]); > > + } > > + > > + return 0; > > +} > > + > > + > > +int do_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > > +{ > > + int dev = 0; > > + static int zeroed = 0; > > move this out. > > besides, I think it'd be much systematic to just scream at user to call > "sata rescan" and bail out instead of doing it for him. i dont actually need a sata rescan, i just need to make sure i have dynamically allocated names, so i can safely call free() later, otherwise this segfaults when called before sata scan > > + /* make sure we have valid filenames */ > > + if (!zeroed) { > > + init_sata(0); > > + zeroed = 1; > > + } > > + > > + switch (argc) { > > + case 0: > > + case 1: > > + return CMD_RET_USAGE; > > + case 2: > > + dev = simple_strtoul(argv[1], NULL, 10); > > Ok, so if I run this command and ask for device 0xb00bf33d ... will this > survive? Hint: it won't, rangecheck missing. hint - scan_sata does the range check in this codepath > > + return scan_sata(dev); > > + > > + case 3: > > + if (!strncmp(argv[1], "inf", 3)) { > > + dev = simple_strtoul(argv[2], NULL, 10); > > Same here see above > > + return scan_sata(dev); > > + } > > + return CMD_RET_USAGE; > > + case 4: > > + if (!strncmp(argv[1], "load", 4)) { > > + dev = simple_strtoul(argv[2], NULL, 10); > > + if ((dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE)) { > > And here you have it ? > > Uh oh, I see, sata_scan() does it for you ... I'd say, abstract it out into > lightweight static inline function. > > > + printf("File index %d is out of range.\n", dev); > > + return -EINVAL; > > + } > > + free(filenames[dev]); > > + filenames[dev] = strdup(argv[3]); > > + init_sata(dev); > > + return scan_sata(dev); > > + } > > + return CMD_RET_USAGE; > > + } > > + return CMD_RET_USAGE; > > +} > > + > > +U_BOOT_CMD( > > + sata_loop, 4, 1, do_loop, > > + "SATA loopback", > > + "[info] devnum - show info about loop devnum\n" > > Make this "info" part mandatory. Than you can cut the whole argc loop into > simple "if argc != 2 ; then fail" . And do simple checking for the first > letter of the argument being either i or d . wont help, still need argc 3 or 4 > > + "sata_loop load devnum file - load file from host FS into loop devnum" > > sata_loop is redundant above. really? how do you figure? Pavel Herrmann
Dear Pavel Herrmann, [...] > > besides, I think it'd be much systematic to just scream at user to call > > "sata rescan" and bail out instead of doing it for him. > > i dont actually need a sata rescan, i just need to make sure i have > dynamically allocated names Sorry, I can't parse this ... but ... > , so i can safely call free() later, otherwise > this segfaults when called before sata scan The free() function frees the memory space pointed to by ptr, which must have been returned by a previous call to malloc(), calloc() or realloc(). Otherwise, or if free(ptr) has already been called before, undefined behavior occurs. If ptr is NULL, no operation is performed. So if you call free() on null pointer, nothing happens. Where's the real problem? > > > + /* make sure we have valid filenames */ > > > + if (!zeroed) { > > > + init_sata(0); > > > + zeroed = 1; > > > + } > > > + > > > + switch (argc) { > > > + case 0: > > > + case 1: > > > + return CMD_RET_USAGE; > > > + case 2: > > > + dev = simple_strtoul(argv[1], NULL, 10); > > > > Ok, so if I run this command and ask for device 0xb00bf33d ... will this > > survive? Hint: it won't, rangecheck missing. > > > > > + return scan_sata(dev); > > > + > > > + case 3: > > > + if (!strncmp(argv[1], "inf", 3)) { > > > + dev = simple_strtoul(argv[2], NULL, 10); > > > > Same here > > > > > + return scan_sata(dev); > > > + } > > > + return CMD_RET_USAGE; > > > + case 4: > > > + if (!strncmp(argv[1], "load", 4)) { > > > + dev = simple_strtoul(argv[2], NULL, 10); > > > + if ((dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE)) { > > > > And here you have it ? > > > > Uh oh, I see, sata_scan() does it for you ... I'd say, abstract it out > > into lightweight static inline function. > > > > > + printf("File index %d is out of range.\n", dev); > > > + return -EINVAL; > > > + } > > > + free(filenames[dev]); > > > + filenames[dev] = strdup(argv[3]); > > > + init_sata(dev); > > > + return scan_sata(dev); > > > + } > > > + return CMD_RET_USAGE; > > > + } > > > + return CMD_RET_USAGE; > > > +} > > > + > > > +U_BOOT_CMD( > > > + sata_loop, 4, 1, do_loop, > > > + "SATA loopback", > > > + "[info] devnum - show info about loop devnum\n" > > > > Make this "info" part mandatory. Than you can cut the whole argc loop > > into simple "if argc != 2 ; then fail" . And do simple checking for the > > first letter of the argument being either i or d . > > > > > + "sata_loop load devnum file - load file from host FS into loop > > > devnum" > > > > sata_loop is redundant above. > > > > > +);
Dear Pavel Herrmann, [...] > > > > move this out. > > > > besides, I think it'd be much systematic to just scream at user to call > > "sata rescan" and bail out instead of doing it for him. > > i dont actually need a sata rescan, i just need to make sure i have > dynamically allocated names, so i can safely call free() later, otherwise > this segfaults when called before sata scan See my previous email > > > + /* make sure we have valid filenames */ > > > + if (!zeroed) { > > > + init_sata(0); > > > + zeroed = 1; > > > + } > > > + > > > + switch (argc) { > > > + case 0: > > > + case 1: > > > + return CMD_RET_USAGE; > > > + case 2: > > > + dev = simple_strtoul(argv[1], NULL, 10); > > > > Ok, so if I run this command and ask for device 0xb00bf33d ... will this > > survive? Hint: it won't, rangecheck missing. > > hint - scan_sata does the range check in this codepath saw it, see below. Duplication of code is not good and is error prone. > > > + return scan_sata(dev); > > > + > > > + case 3: > > > + if (!strncmp(argv[1], "inf", 3)) { > > > + dev = simple_strtoul(argv[2], NULL, 10); > > > > Same here > > see above > > > > + return scan_sata(dev); > > > + } > > > + return CMD_RET_USAGE; > > > + case 4: > > > + if (!strncmp(argv[1], "load", 4)) { > > > + dev = simple_strtoul(argv[2], NULL, 10); > > > + if ((dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE)) { > > > > And here you have it ? > > > > Uh oh, I see, sata_scan() does it for you ... I'd say, abstract it out > > into lightweight static inline function. > > > > > + printf("File index %d is out of range.\n", dev); > > > + return -EINVAL; > > > + } > > > + free(filenames[dev]); > > > + filenames[dev] = strdup(argv[3]); > > > + init_sata(dev); > > > + return scan_sata(dev); > > > + } > > > + return CMD_RET_USAGE; > > > + } > > > + return CMD_RET_USAGE; > > > +} > > > + > > > +U_BOOT_CMD( > > > + sata_loop, 4, 1, do_loop, > > > + "SATA loopback", > > > + "[info] devnum - show info about loop devnum\n" > > > > Make this "info" part mandatory. Than you can cut the whole argc loop > > into simple "if argc != 2 ; then fail" . And do simple checking for the > > first letter of the argument being either i or d . > > wont help, still need argc 3 or 4 Makes is simpler, you can bail out if it's not 3 or 4 > > > + "sata_loop load devnum file - load file from host FS into loop > > > devnum" > > > > sata_loop is redundant above. > > really? how do you figure? Run "help sata_loop" and see the result ... > > Pavel Herrmann
On Wednesday 05 of September 2012 14:48:40 Marek Vasut wrote: > Dear Pavel Herrmann, > > [...] > > > > besides, I think it'd be much systematic to just scream at user to call > > > "sata rescan" and bail out instead of doing it for him. > > > > i dont actually need a sata rescan, i just need to make sure i have > > dynamically allocated names > > Sorry, I can't parse this ... but ... > > > , so i can safely call free() later, otherwise > > this segfaults when called before sata scan > > The free() function frees the memory space pointed to by ptr, which must > have been returned by a previous call to malloc(), calloc() or realloc(). > Otherwise, or if free(ptr) has already been called before, undefined > behavior occurs. If ptr is NULL, no operation is performed. > > So if you call free() on null pointer, nothing happens. Where's the real > problem? if you called "sata init" before setting all the loops you would get to open(NULL) and a strlen(NULL). i think its easier to supply valid empty string then check for NULL at every access. > > > > > > Make this "info" part mandatory. Than you can cut the whole argc loop > > > into simple "if argc != 2 ; then fail" . And do simple checking for the > > > first letter of the argument being either i or d . > > > > wont help, still need argc 3 or 4 > > Makes is simpler, you can bail out if it's not 3 or 4 still, i should have a "sata_loop info" work on all files, so theres a valid command with argc 2 > > > > + "sata_loop load devnum file - load file from host FS into loop > > > > devnum" > > > > > > sata_loop is redundant above. > > > > really? how do you figure? > > Run "help sata_loop" and see the result ... =>help sata_loop sata_loop - SATA loopback Usage: sata_loop [info] devnum - show info about loop devnum sata_loop load devnum file - load file from host FS into loop devnum i dont see your problem Pavel Herrmann
Dear Pavel Herrmann, > On Wednesday 05 of September 2012 14:48:40 Marek Vasut wrote: > > Dear Pavel Herrmann, > > > > [...] > > > > > > besides, I think it'd be much systematic to just scream at user to > > > > call "sata rescan" and bail out instead of doing it for him. > > > > > > i dont actually need a sata rescan, i just need to make sure i have > > > dynamically allocated names > > > > Sorry, I can't parse this ... but ... > > > > > , so i can safely call free() later, otherwise > > > this segfaults when called before sata scan > > > > The free() function frees the memory space pointed to by ptr, which must > > have been returned by a previous call to malloc(), calloc() or realloc(). > > Otherwise, or if free(ptr) has already been called before, undefined > > behavior occurs. If ptr is NULL, no operation is performed. > > > > So if you call free() on null pointer, nothing happens. Where's the real > > problem? > > if you called "sata init" before setting all the loops you would get to > open(NULL) and a strlen(NULL). i think its easier to supply valid empty > string then check for NULL at every access. I'd say check for null on every access ... you'd lower memory requirements and the increase in code requirement should really be very minor. Also, it'd remove the "zeroed" static variable altogether, correct? > > > > Make this "info" part mandatory. Than you can cut the whole argc loop > > > > into simple "if argc != 2 ; then fail" . And do simple checking for > > > > the first letter of the argument being either i or d . > > > > > > wont help, still need argc 3 or 4 > > > > Makes is simpler, you can bail out if it's not 3 or 4 > > still, i should have a "sata_loop info" work on all files, so theres a > valid command with argc 2 Understood. > > > > > + "sata_loop load devnum file - load file from host FS into loop > > > > > devnum" > > > > > > > > sata_loop is redundant above. > > > > > > really? how do you figure? > > > > Run "help sata_loop" and see the result ... > > =>help sata_loop > sata_loop - SATA loopback > > Usage: > sata_loop [info] devnum - show info about loop devnum > sata_loop load devnum file - load file from host FS into loop devnum > > i dont see your problem Ewww ... nevermind ... I just detected yet another stupid issue in the codebase. I'd say this "sata_loop" should be either omitted and filled by the macro or something. But that's for other time to solve ... so I'm fine with this now. > Pavel Herrmann Best regards, Marek Vasut
On Thursday 06 of September 2012 03:08:42 Marek Vasut wrote: > Dear Pavel Herrmann, > > > On Wednesday 05 of September 2012 14:48:40 Marek Vasut wrote: > > > Dear Pavel Herrmann, > > > > > > [...] > > > > > > > > besides, I think it'd be much systematic to just scream at user to > > > > > call "sata rescan" and bail out instead of doing it for him. > > > > > > > > i dont actually need a sata rescan, i just need to make sure i have > > > > dynamically allocated names > > > > > > Sorry, I can't parse this ... but ... > > > > > > > , so i can safely call free() later, otherwise > > > > this segfaults when called before sata scan > > > > > > The free() function frees the memory space pointed to by ptr, which must > > > have been returned by a previous call to malloc(), calloc() or > > > realloc(). > > > Otherwise, or if free(ptr) has already been called before, undefined > > > behavior occurs. If ptr is NULL, no operation is performed. > > > > > > So if you call free() on null pointer, nothing happens. Where's the real > > > problem? > > > > if you called "sata init" before setting all the loops you would get to > > open(NULL) and a strlen(NULL). i think its easier to supply valid empty > > string then check for NULL at every access. > > I'd say check for null on every access ... you'd lower memory requirements > and the increase in code requirement should really be very minor. Also, > it'd remove the "zeroed" static variable altogether, correct? OK, fine by me > > > > > Make this "info" part mandatory. Than you can cut the whole argc > > > > > loop > > > > > into simple "if argc != 2 ; then fail" . And do simple checking for > > > > > the first letter of the argument being either i or d . > > > > > > > > wont help, still need argc 3 or 4 > > > > > > Makes is simpler, you can bail out if it's not 3 or 4 > > > > still, i should have a "sata_loop info" work on all files, so theres a > > valid command with argc 2 > > Understood. > > > > > > > + "sata_loop load devnum file - load file from host FS into loop > > > > > > devnum" > > > > > > > > > > sata_loop is redundant above. > > > > > > > > really? how do you figure? > > > > > > Run "help sata_loop" and see the result ... > > > > =>help sata_loop > > sata_loop - SATA loopback > > > > Usage: > > sata_loop [info] devnum - show info about loop devnum > > sata_loop load devnum file - load file from host FS into loop devnum > > > > i dont see your problem > > Ewww ... nevermind ... I just detected yet another stupid issue in the > codebase. I'd say this "sata_loop" should be either omitted and filled by > the macro or something. But that's for other time to solve ... so I'm fine > with this now. it actually is, but only to the first line (as both lines are in one string). In my opinion it should either add this for each line (with some magic string modification) or none at all, to keep it consistent Pavel Herrmann
Dear Pavel Herrmann, > On Thursday 06 of September 2012 03:08:42 Marek Vasut wrote: > > Dear Pavel Herrmann, > > > > > On Wednesday 05 of September 2012 14:48:40 Marek Vasut wrote: > > > > Dear Pavel Herrmann, > > > > > > > > [...] > > > > > > > > > > besides, I think it'd be much systematic to just scream at user > > > > > > to call "sata rescan" and bail out instead of doing it for him. > > > > > > > > > > i dont actually need a sata rescan, i just need to make sure i have > > > > > dynamically allocated names > > > > > > > > Sorry, I can't parse this ... but ... > > > > > > > > > , so i can safely call free() later, otherwise > > > > > this segfaults when called before sata scan > > > > > > > > The free() function frees the memory space pointed to by ptr, which > > > > must have been returned by a previous call to malloc(), calloc() or > > > > realloc(). > > > > Otherwise, or if free(ptr) has already been called before, undefined > > > > behavior occurs. If ptr is NULL, no operation is performed. > > > > > > > > So if you call free() on null pointer, nothing happens. Where's the > > > > real problem? > > > > > > if you called "sata init" before setting all the loops you would get to > > > open(NULL) and a strlen(NULL). i think its easier to supply valid empty > > > string then check for NULL at every access. > > > > I'd say check for null on every access ... you'd lower memory > > requirements and the increase in code requirement should really be very > > minor. Also, it'd remove the "zeroed" static variable altogether, > > correct? > > OK, fine by me > > > > > > > Make this "info" part mandatory. Than you can cut the whole argc > > > > > > loop > > > > > > into simple "if argc != 2 ; then fail" . And do simple checking > > > > > > for the first letter of the argument being either i or d . > > > > > > > > > > wont help, still need argc 3 or 4 > > > > > > > > Makes is simpler, you can bail out if it's not 3 or 4 > > > > > > still, i should have a "sata_loop info" work on all files, so theres a > > > valid command with argc 2 > > > > Understood. > > > > > > > > > + "sata_loop load devnum file - load file from host FS into > > > > > > > loop devnum" > > > > > > > > > > > > sata_loop is redundant above. > > > > > > > > > > really? how do you figure? > > > > > > > > Run "help sata_loop" and see the result ... > > > > > > =>help sata_loop > > > sata_loop - SATA loopback > > > > > > Usage: > > > sata_loop [info] devnum - show info about loop devnum > > > sata_loop load devnum file - load file from host FS into loop devnum > > > > > > i dont see your problem > > > > Ewww ... nevermind ... I just detected yet another stupid issue in the > > codebase. I'd say this "sata_loop" should be either omitted and filled by > > the macro or something. But that's for other time to solve ... so I'm > > fine with this now. > > it actually is, but only to the first line (as both lines are in one > string). In my opinion it should either add this for each line (with some > magic string modification) or none at all, to keep it consistent Keep it as is now, but it's something that should eventually be resolved. > Pavel Herrmann
diff --git a/drivers/block/Makefile b/drivers/block/Makefile index f1ebdcc..c95651a 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} += sata_loopback.o COBJS := $(COBJS-y) SRCS := $(COBJS:.o=.c) diff --git a/drivers/block/sata_loopback.c b/drivers/block/sata_loopback.c new file mode 100644 index 0000000..0e6923b --- /dev/null +++ b/drivers/block/sata_loopback.c @@ -0,0 +1,200 @@ +/* + * (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> +#include <command.h> +#include <malloc.h> + +static const char revision[] = "0.0"; +static const char vendor[] = "SATA loopback"; + +static char * filenames[CONFIG_SYS_SATA_MAX_DEVICE]; + +extern block_dev_desc_t sata_dev_desc[]; + +int init_sata(int dev) +{ + static int zeroed = 0; + block_dev_desc_t *pdev = &sata_dev_desc[dev]; + int fd, old_fd; + + + if (!zeroed) { + int i; + for (i = 0; i < CONFIG_SYS_SATA_MAX_DEVICE; i++) + filenames[i]=strdup(""); + zeroed = 1; + } + + if ((dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE)) { + printf("File index %d is out of range.\n", dev); + return -EINVAL; + } + + fd = os_open(filenames[dev], OS_O_RDWR); + /* This is ugly, but saves allocation for 1 int. */ + old_fd = (long) pdev->priv; + pdev->priv = (void *) (long) fd; + /* + * sadly we cannot set -1 to all as above, because "sata init" will zero + * this value before calling sata_init. + */ + if ((old_fd > 2) || (old_fd < 0)) + os_close(old_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; + lbaint_t start_byte = ATA_SECT_SIZE * start; + lbaint_t length_byte = ATA_SECT_SIZE * blkcnt; + lbaint_t retval; + + if ( os_lseek(fd, start_byte, OS_SEEK_SET) != start_byte) + return -1; + + retval = os_read(fd, buffer, length_byte); + + 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 start_byte = ATA_SECT_SIZE * start; + lbaint_t length_byte = ATA_SECT_SIZE * blkcnt; + lbaint_t retval; + + if ( os_lseek(fd, start_byte, OS_SEEK_SET) != start_byte) + return -1; + + retval = os_write(fd, buffer, length_byte); + + 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; + const char *filename = filenames[dev]; + lbaint_t bytes = 0; + + if ((dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE)) { + printf("File index %d is out of range.\n", dev); + return -EINVAL; + } + + memcpy(pdev->vendor, vendor, sizeof(vendor)); + memcpy(pdev->revision, revision, sizeof(revision)); + namelen = strlen(filenames[dev]); + + if (namelen > 20) { + /* take the last 17 chars, prepend them with "..." */ + filename += (namelen - 17); + memcpy(pdev->product, "...", 3); + memcpy(pdev->product+3, filename, 17); + } else + memcpy(pdev->product, filename, namelen); + + pdev->product[20] = 0; + + bytes = os_lseek(fd, 0, OS_SEEK_END); + if (bytes != -1) { + pdev->type = DEV_TYPE_HARDDISK; + pdev->blksz = ATA_SECT_SIZE; + pdev->lun = 0; + pdev->lba = bytes/ATA_SECT_SIZE; + printf("SATA loop %d:\nfilename: %s\nsize: %lu\nblock count:" + " %lu\n", dev, filenames[dev], bytes, pdev->lba); + } else { + pdev->type = DEV_TYPE_HARDDISK; + pdev->blksz = ATA_SECT_SIZE; + pdev->lun = 0; + pdev->lba = 0; + printf("SATA loop %d:\nfilename: %s\nFAILED TO OPEN\n", + dev, filenames[dev]); + } + + return 0; +} + + +int do_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + int dev = 0; + static int zeroed = 0; + + /* make sure we have valid filenames */ + if (!zeroed) { + init_sata(0); + zeroed = 1; + } + + switch (argc) { + case 0: + case 1: + return CMD_RET_USAGE; + case 2: + dev = simple_strtoul(argv[1], NULL, 10); + return scan_sata(dev); + + case 3: + if (!strncmp(argv[1], "inf", 3)) { + dev = simple_strtoul(argv[2], NULL, 10); + return scan_sata(dev); + } + return CMD_RET_USAGE; + case 4: + if (!strncmp(argv[1], "load", 4)) { + dev = simple_strtoul(argv[2], NULL, 10); + if ((dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE)) { + printf("File index %d is out of range.\n", dev); + return -EINVAL; + } + free(filenames[dev]); + filenames[dev] = strdup(argv[3]); + init_sata(dev); + return scan_sata(dev); + } + return CMD_RET_USAGE; + } + return CMD_RET_USAGE; +} + +U_BOOT_CMD( + sata_loop, 4, 1, do_loop, + "SATA loopback", + "[info] devnum - show info about loop devnum\n" + "sata_loop load devnum file - load file from host FS into loop devnum" +);
This driver uses files as block devices, can be used for testing disk operations on sandbox. A new command "sata_loop" is introduced to load files in runtime. Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com> CC: Marek Vasut <marex@denx.de> CC: Mike Frysinger <vapier@gentoo.org> --- Changes for v3: introduce sata_loop command Changes for v2: split sandbox config off into separate patch (2/2) rename file to signify exported API style fixes show end of long filenames rather than beginning check for lseek errors to indicate non-regular file drivers/block/Makefile | 1 + drivers/block/sata_loopback.c | 200 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 201 insertions(+) create mode 100644 drivers/block/sata_loopback.c