[U-Boot] [PATCH] Loop block device for sandbox
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
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]); + int fd; + + if ((dev < 0) || (dev >= max_devs)) { + 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 */ + 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; + 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); + 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; + memcpy(pdev->vendor, vendor, sizeof(vendor)); + memcpy(pdev->revision, revision, sizeof(revision)); + namelen = sizeof(filenames[dev]); + if (namelen > 20) + namelen = 20; + memcpy(pdev->product, filenames[dev], namelen); + pdev->product[20] = 0; + + if (fd != -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 + #endif
More CC
On Wednesday 29 August 2012 17:46:43 Pavel Herrmann wrote:
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
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]);
- int fd;
- if ((dev < 0) || (dev >= max_devs)) {
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 */
- 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;
- 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);
- 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;
- memcpy(pdev->vendor, vendor, sizeof(vendor));
- memcpy(pdev->revision, revision, sizeof(revision));
- namelen = sizeof(filenames[dev]);
- if (namelen > 20)
namelen = 20;- memcpy(pdev->product, filenames[dev], namelen);
- pdev->product[20] = 0;
- if (fd != -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
#endif
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
On Thursday 30 of August 2012 00:18:18 Marek Vasut wrote: ...snip...
+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.
Straight copy from dwc_ahsata.c, makes it more readable thought, as the order of operation is not very intuitive IMHO.
+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
well, it isn't, at least not from the command - thats why you define the number of ports in advance, you get "dev" already range-checked
- 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);
Besides, lseek can fail, can it not?
If you open a pipe (or nothing), yes in the first case, you shouldn't, in the second, the I/O op will harmlessly fail as well
- if (namelen > 20)
namelen = 20;Why do you trim down the string, won't simple strdup() work?
nah, the destination is char[21], as it is the exact length of corresponding field in ATA identify response (one more for a 0 at the end)
- memcpy(pdev->product, filenames[dev], namelen);
- pdev->product[20] = 0;
- if (fd != -1) {
And if "fd" is -1 ?
then all defaults to an invalid device, because you failed to open the file, for whatever the reason.
"agreed" to the other comments
Best Regards Pavel Herrmann
Dear Pavel Herrmann,
On Thursday 30 of August 2012 00:18:18 Marek Vasut wrote: ...snip...
+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.
Straight copy from dwc_ahsata.c, makes it more readable thought, as the order of operation is not very intuitive IMHO.
sata_dev_desc + dev ?
+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
well, it isn't, at least not from the command - thats why you define the number of ports in advance, you get "dev" already range-checked
Range check is fine, but will pdev be inited? It's a pointer from some array.
- 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);
Besides, lseek can fail, can it not?
If you open a pipe (or nothing), yes in the first case, you shouldn't
Shouldn't ... what? Sorry, I cannot parse this.
in the second, the I/O op will harmlessly fail as well
How so?
- if (namelen > 20)
namelen = 20;Why do you trim down the string, won't simple strdup() work?
nah, the destination is char[21], as it is the exact length of corresponding field in ATA identify response (one more for a 0 at the end)
I see, is it a full path ? If so, it might be a better idea to use the filename itself instead of the whole path. So you'd prevent names like "~/../foo/../.././bar.img" .
- memcpy(pdev->product, filenames[dev], namelen);
- pdev->product[20] = 0;
- if (fd != -1) {
And if "fd" is -1 ?
then all defaults to an invalid device, because you failed to open the file, for whatever the reason.
At least the printf below will choke, since pdev->lba is uninited
"agreed" to the other comments
Best Regards Pavel Herrmann _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Best regards, Marek Vasut
On Thursday 30 of August 2012 20:45:13 Marek Vasut wrote:
Dear Pavel Herrmann,
On Thursday 30 of August 2012 00:18:18 Marek Vasut wrote: ...snip...
+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.
Straight copy from dwc_ahsata.c, makes it more readable thought, as the order of operation is not very intuitive IMHO.
sata_dev_desc + dev ?
even less intuitive
+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
well, it isn't, at least not from the command - thats why you define the number of ports in advance, you get "dev" already range-checked
Range check is fine, but will pdev be inited? It's a pointer from some array.
init_sata is called first, so pdev is inited (see cmd_sata.c)
- 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);
Besides, lseek can fail, can it not?
If you open a pipe (or nothing), yes in the first case, you shouldn't
Shouldn't ... what? Sorry, I cannot parse this.
shouldn't do that - means i agree there should be a check in case you are actively trying to break things, and use pipes/sockets as loop blocks
in the second, the I/O op will harmlessly fail as well
How so?
because then the fd is -1, and read/write will do the right thing there (nothing, return -1 and set errno to EBADF)
- if (namelen > 20)
namelen = 20;Why do you trim down the string, won't simple strdup() work?
nah, the destination is char[21], as it is the exact length of corresponding field in ATA identify response (one more for a 0 at the end)
I see, is it a full path ? If so, it might be a better idea to use the filename itself instead of the whole path. So you'd prevent names like "~/../foo/../.././bar.img" .
yes, i was thinking about "...${last 17 bytes of the name}" if the name was longer, but this proved significantly simpler for demonstrating the general idea.
- memcpy(pdev->product, filenames[dev], namelen);
- pdev->product[20] = 0;
- if (fd != -1) {
And if "fd" is -1 ?
then all defaults to an invalid device, because you failed to open the file, for whatever the reason.
At least the printf below will choke, since pdev->lba is uninited
not the case. sata_dev_desc is inited in cmd_sata.c, and therefore by not doing anything we get an empty device
Best Regards Pavel Herrmann
Dear Pavel Herrmann,
On Thursday 30 of August 2012 20:45:13 Marek Vasut wrote:
Dear Pavel Herrmann,
On Thursday 30 of August 2012 00:18:18 Marek Vasut wrote: ...snip...
+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.
Straight copy from dwc_ahsata.c, makes it more readable thought, as the order of operation is not very intuitive IMHO.
sata_dev_desc + dev ?
even less intuitive
Why so?
+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
well, it isn't, at least not from the command - thats why you define the number of ports in advance, you get "dev" already range-checked
Range check is fine, but will pdev be inited? It's a pointer from some array.
init_sata is called first, so pdev is inited (see cmd_sata.c)
Unless it fails. Then what ?
- 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);
Besides, lseek can fail, can it not?
If you open a pipe (or nothing), yes in the first case, you shouldn't
Shouldn't ... what? Sorry, I cannot parse this.
shouldn't do that - means i agree there should be a check in case you are actively trying to break things, and use pipes/sockets as loop blocks
Good
in the second, the I/O op will harmlessly fail as well
How so?
because then the fd is -1, and read/write will do the right thing there (nothing, return -1 and set errno to EBADF)
From write(2)
-->8--
RETURN VALUE On success, the number of bytes written is returned (zero indicates nothing was written). On error, -1 is returned, and errno is set appropriately.
If count is zero and fd refers to a regular file, then write() may return a failure status if one of the errors below is detected. If no errors are detected, 0 will be returned without causing any other effect. If count is zero and fd refers to a file other than a regular file, the results are not specified.
--8<--
I don't see the case where fd = -1 handled there at all. The last sentence resembles it, but in that case, the behavior is undefined. Can you elaborate please?
- if (namelen > 20)
namelen = 20;Why do you trim down the string, won't simple strdup() work?
nah, the destination is char[21], as it is the exact length of corresponding field in ATA identify response (one more for a 0 at the end)
I see, is it a full path ? If so, it might be a better idea to use the filename itself instead of the whole path. So you'd prevent names like "~/../foo/../.././bar.img" .
yes, i was thinking about "...${last 17 bytes of the name}" if the name was longer, but this proved significantly simpler for demonstrating the general idea.
I think the FS code might contain some function to fixup the path and get filename from path.
- memcpy(pdev->product, filenames[dev], namelen);
- pdev->product[20] = 0;
- if (fd != -1) {
And if "fd" is -1 ?
then all defaults to an invalid device, because you failed to open the file, for whatever the reason.
At least the printf below will choke, since pdev->lba is uninited
not the case. sata_dev_desc is inited in cmd_sata.c, and therefore by not doing anything we get an empty device
I see ... shall we also move all these memcpy() calls in to if (fd != -1) then?
Best Regards Pavel Herrmann
Best regards, Marek Vasut
On Thursday 30 August 2012 23:53:58 Marek Vasut wrote:
Dear Pavel Herrmann,
On Thursday 30 of August 2012 20:45:13 Marek Vasut wrote:
Dear Pavel Herrmann,
On Thursday 30 of August 2012 00:18:18 Marek Vasut wrote: ...snip...
+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.
Straight copy from dwc_ahsata.c, makes it more readable thought, as the order of operation is not very intuitive IMHO.
sata_dev_desc + dev ?
even less intuitive
Why so?
because of the silent "*sizeof(sata_dev_desc)". I know this is standardized in C (so is the order of operands), but doing "+" on non-numbers is a little too C++ for me. I know that generated code will be eactly the same in all cases.
+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
well, it isn't, at least not from the command - thats why you define the number of ports in advance, you get "dev" already range-checked
Range check is fine, but will pdev be inited? It's a pointer from some array.
init_sata is called first, so pdev is inited (see cmd_sata.c)
Unless it fails. Then what ?
the only way init can "fail" is if it gets a wrong device number (which should not happen), or if it cannot open the file, in which case it still sets pdev as -1.
in the second, the I/O op will harmlessly fail as well
How so?
because then the fd is -1, and read/write will do the right thing there (nothing, return -1 and set errno to EBADF)
From write(2)
-->8--
RETURN VALUE On success, the number of bytes written is returned (zero indicates nothing was written). On error, -1 is returned, and errno is set appropriately.
If count is zero and fd refers to a regular file, then write() mayreturn a failure status if one of the errors below is detected. If no errors are detected, 0 will be returned without causing any other effect. If count is zero and fd refers to a file other than a regular file, the results are not specified.
--8<--
I don't see the case where fd = -1 handled there at all. The last sentence resembles it, but in that case, the behavior is undefined. Can you elaborate please?
RETURN VALUE ... On error, -1 is returned, and errno is set appropriately. ... ERRORS ... EBADF fd is not a valid file descriptor or is not open for writing. ... -1 is definitely not a valid file descriptor. this point is moot, as checking success of lseek (because of pipes/sockets) will filter out invalid fd as well
- if (namelen > 20)
namelen = 20;Why do you trim down the string, won't simple strdup() work?
nah, the destination is char[21], as it is the exact length of corresponding field in ATA identify response (one more for a 0 at the end)
I see, is it a full path ? If so, it might be a better idea to use the filename itself instead of the whole path. So you'd prevent names like "~/../foo/../.././bar.img" .
yes, i was thinking about "...${last 17 bytes of the name}" if the name was longer, but this proved significantly simpler for demonstrating the general idea.
I think the FS code might contain some function to fixup the path and get filename from path.
that still wouldn't solve the problem, flename can still be over 20 bytes long
- memcpy(pdev->product, filenames[dev], namelen);
- pdev->product[20] = 0;
- if (fd != -1) {
And if "fd" is -1 ?
then all defaults to an invalid device, because you failed to open the file, for whatever the reason.
At least the printf below will choke, since pdev->lba is uninited
not the case. sata_dev_desc is inited in cmd_sata.c, and therefore by not doing anything we get an empty device
I see ... shall we also move all these memcpy() calls in to if (fd != -1) then?
I'd like to know that the device is a loopback, and what filename, not just that it failed to init
Pavel Herrmann
Dear Pavel Herrmann,
On Thursday 30 August 2012 23:53:58 Marek Vasut wrote:
Dear Pavel Herrmann,
On Thursday 30 of August 2012 20:45:13 Marek Vasut wrote:
Dear Pavel Herrmann,
On Thursday 30 of August 2012 00:18:18 Marek Vasut wrote: ...snip...
> +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.
Straight copy from dwc_ahsata.c, makes it more readable thought, as the order of operation is not very intuitive IMHO.
sata_dev_desc + dev ?
even less intuitive
Why so?
because of the silent "*sizeof(sata_dev_desc)". I know this is standardized in C (so is the order of operands), but doing "+" on non-numbers is a little too C++ for me. I know that generated code will be eactly the same in all cases.
It's simple increment of a pointer, normal thing.
> +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
well, it isn't, at least not from the command - thats why you define the number of ports in advance, you get "dev" already range-checked
Range check is fine, but will pdev be inited? It's a pointer from some array.
init_sata is called first, so pdev is inited (see cmd_sata.c)
Unless it fails. Then what ?
the only way init can "fail" is if it gets a wrong device number (which should not happen), or if it cannot open the file, in which case it still sets pdev as -1.
If pdev is -1, then this explodes for certain, on unaligned access and on wrong ptr deref.
in the second, the I/O op will harmlessly fail as well
How so?
because then the fd is -1, and read/write will do the right thing there (nothing, return -1 and set errno to EBADF)
From write(2)
-->8--
RETURN VALUE
On success, the number of bytes written is returned (zero indicatesnothing was written). On error, -1 is returned,
and errno is set appropriately. If count is zero and fd refers to a regular file, then write() mayreturn a failure status if one of the errors below
is detected. If no errors are detected, 0 will be returned withoutcausing any other effect. If count is zero and fd
refers to a file other than a regular file, the results are notspecified.
--8<--
I don't see the case where fd = -1 handled there at all. The last sentence resembles it, but in that case, the behavior is undefined. Can you elaborate please?
RETURN VALUE ... On error, -1 is returned, and errno is set appropriately. ... ERRORS ... EBADF fd is not a valid file descriptor or is not open for writing. ... -1 is definitely not a valid file descriptor.
I see, good catch.
this point is moot, as checking success of lseek (because of pipes/sockets) will filter out invalid fd as well
> + if (namelen > 20) > + namelen = 20;
Why do you trim down the string, won't simple strdup() work?
nah, the destination is char[21], as it is the exact length of corresponding field in ATA identify response (one more for a 0 at the end)
I see, is it a full path ? If so, it might be a better idea to use the filename itself instead of the whole path. So you'd prevent names like "~/../foo/../.././bar.img" .
yes, i was thinking about "...${last 17 bytes of the name}" if the name was longer, but this proved significantly simpler for demonstrating the general idea.
I think the FS code might contain some function to fixup the path and get filename from path.
that still wouldn't solve the problem, flename can still be over 20 bytes long
Then pick the first 20 ... but this is really discutable
> + memcpy(pdev->product, filenames[dev], namelen); > + pdev->product[20] = 0; > + > + if (fd != -1) {
And if "fd" is -1 ?
then all defaults to an invalid device, because you failed to open the file, for whatever the reason.
At least the printf below will choke, since pdev->lba is uninited
not the case. sata_dev_desc is inited in cmd_sata.c, and therefore by not doing anything we get an empty device
I see ... shall we also move all these memcpy() calls in to if (fd != -1) then?
I'd like to know that the device is a loopback, and what filename, not just that it failed to init
But are such data used somewhere further down the road?
Pavel Herrmann
Best regards, Marek Vasut
On Friday 31 of August 2012 14:57:41 Marek Vasut wrote:
> > +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
well, it isn't, at least not from the command - thats why you define the number of ports in advance, you get "dev" already range-checked
Range check is fine, but will pdev be inited? It's a pointer from some array.
init_sata is called first, so pdev is inited (see cmd_sata.c)
Unless it fails. Then what ?
the only way init can "fail" is if it gets a wrong device number (which should not happen), or if it cannot open the file, in which case it still sets pdev as -1.
If pdev is -1, then this explodes for certain, on unaligned access and on wrong ptr deref.
again, no
there is no pointer in this, pdev->priv is actually an int stored as void*, pdev itself is pointer to a global array. which one does explode in your case?
> > + memcpy(pdev->product, filenames[dev], namelen); > > + pdev->product[20] = 0; > > + > > + if (fd != -1) { > > And if "fd" is -1 ?
then all defaults to an invalid device, because you failed to open the file, for whatever the reason.
At least the printf below will choke, since pdev->lba is uninited
not the case. sata_dev_desc is inited in cmd_sata.c, and therefore by not doing anything we get an empty device
I see ... shall we also move all these memcpy() calls in to if (fd != -1) then?
I'd like to know that the device is a loopback, and what filename, not just that it failed to init
But are such data used somewhere further down the road?
yes, "sata info" for example
Pavel Herrmann
Dear Pavel Herrmann,
On Friday 31 of August 2012 14:57:41 Marek Vasut wrote:
> > > +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 > > well, it isn't, at least not from the command - thats why you > define the number of ports in advance, you get "dev" already > range-checked
Range check is fine, but will pdev be inited? It's a pointer from some array.
init_sata is called first, so pdev is inited (see cmd_sata.c)
Unless it fails. Then what ?
the only way init can "fail" is if it gets a wrong device number (which should not happen), or if it cannot open the file, in which case it still sets pdev as -1.
If pdev is -1, then this explodes for certain, on unaligned access and on wrong ptr deref.
again, no
there is no pointer in this, pdev->priv is actually an int stored as void*, pdev itself is pointer to a global array. which one does explode in your case?
pdev, but given that the pointer is inited, I see the issue at hand now.
> > > + memcpy(pdev->product, filenames[dev], namelen); > > > + pdev->product[20] = 0; > > > + > > > + if (fd != -1) { > > > > And if "fd" is -1 ? > > then all defaults to an invalid device, because you failed to > open the file, for whatever the reason.
At least the printf below will choke, since pdev->lba is uninited
not the case. sata_dev_desc is inited in cmd_sata.c, and therefore by not doing anything we get an empty device
I see ... shall we also move all these memcpy() calls in to if (fd != -1) then?
I'd like to know that the device is a loopback, and what filename, not just that it failed to init
But are such data used somewhere further down the road?
yes, "sata info" for example
And how does it determine that the init failed?
I think we're almost clear on these things.
Pavel Herrmann
Best regards, Marek Vasut
On Friday 31 of August 2012 18:02:22 Marek Vasut wrote:
> > > > + memcpy(pdev->product, filenames[dev], namelen); > > > > + pdev->product[20] = 0; > > > > + > > > > + if (fd != -1) { > > > > > > And if "fd" is -1 ? > > > > then all defaults to an invalid device, because you failed to > > open the file, for whatever the reason. > > At least the printf below will choke, since pdev->lba is > uninited
not the case. sata_dev_desc is inited in cmd_sata.c, and therefore by not doing anything we get an empty device
I see ... shall we also move all these memcpy() calls in to if (fd != -1) then?
I'd like to know that the device is a loopback, and what filename, not just that it failed to init
But are such data used somewhere further down the road?
yes, "sata info" for example
And how does it determine that the init failed?
given that the only thing the init does is open a file and put the decriptor to ->priv, you can tell whether it failed by looking at the descriptor (-1 means failed). the other fail-path in init is if it is given a wrong index, but that should never happen (at least not when called from cmd_sata)
on a disk with 0 size you cant have a partition table, so any FS code will refuse to work, any direct operation will fail because fd is -1
or did you mean a different "it"?
I think we're almost clear on these things.
thats a relief
Pavel Herrmann
Dear Pavel Herrmann,
On Friday 31 of August 2012 18:02:22 Marek Vasut wrote:
> > > > > + memcpy(pdev->product, filenames[dev], namelen); > > > > > + pdev->product[20] = 0; > > > > > + > > > > > + if (fd != -1) { > > > > > > > > And if "fd" is -1 ? > > > > > > then all defaults to an invalid device, because you failed > > > to open the file, for whatever the reason. > > > > At least the printf below will choke, since pdev->lba is > > uninited > > not the case. sata_dev_desc is inited in cmd_sata.c, and > therefore by not doing anything we get an empty device
I see ... shall we also move all these memcpy() calls in to if (fd != -1) then?
I'd like to know that the device is a loopback, and what filename, not just that it failed to init
But are such data used somewhere further down the road?
yes, "sata info" for example
And how does it determine that the init failed?
given that the only thing the init does is open a file and put the decriptor to ->priv, you can tell whether it failed by looking at the descriptor (-1 means failed). the other fail-path in init is if it is given a wrong index, but that should never happen (at least not when called from cmd_sata)
on a disk with 0 size you cant have a partition table, so any FS code will refuse to work, any direct operation will fail because fd is -1
or did you mean a different "it"?
Ok, I'd like to get it reviewed by someone else and then I think it can be applied.
Mike?
I think we're almost clear on these things.
thats a relief
Pavel Herrmann
Best regards, Marek Vasut
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 --- 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 | 122 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 123 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..7f25a4f --- /dev/null +++ b/drivers/block/sata_loopback.c @@ -0,0 +1,122 @@ +/* + * (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[] = "SATA 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]; + int fd; + + if ((dev < 0) || (dev >= max_devs)) { + 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. */ + 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; + 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; + + 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 info:\nfilename: %s\nsize: %lu\nblock count: %lu\n", + filenames[dev], bytes, pdev->lba); + + return 0; +}
Signed-off-by: Pavel Herrmann morpheus.ibis@gmail.com --- include/configs/sandbox.h | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index 0220386..3126542 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", "obscenelylongfilename", "disk3"} +#define CONFIG_SYS_SATA_MAX_DEVICE 3 +#define CONFIG_DOS_PARTITION +#define CONFIG_CMD_FAT +#define CONFIG_CMD_EXT2 + #endif
Dear Pavel Herrmann,
I don't understand what this patch does from the lacking description. Please add proper description to the patch. ALWAYS!
Signed-off-by: Pavel Herrmann morpheus.ibis@gmail.com
include/configs/sandbox.h | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index 0220386..3126542 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", "obscenelylongfilename", "disk3"} +#define CONFIG_SYS_SATA_MAX_DEVICE 3 +#define CONFIG_DOS_PARTITION +#define CONFIG_CMD_FAT +#define CONFIG_CMD_EXT2
#endif
Best regards, Marek Vasut
Hi
On Saturday 01 of September 2012 16:20:12 Marek Vasut wrote:
Dear Pavel Herrmann,
I don't understand what this patch does from the lacking description. Please add proper description to the patch. ALWAYS!
see $title perhaps? there is not much more to say.
v2 on the way
Pavel Herrmann
Enable SATA_LOOP and a few disk-related commands for sandbox
Signed-off-by: Pavel Herrmann morpheus.ibis@gmail.com --- changes for v2: add a few words of description
include/configs/sandbox.h | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index 0220386..3126542 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", "obscenelylongfilename", "disk3"} +#define CONFIG_SYS_SATA_MAX_DEVICE 3 +#define CONFIG_DOS_PARTITION +#define CONFIG_CMD_FAT +#define CONFIG_CMD_EXT2 + #endif
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
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 | 122 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 123 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..7f25a4f --- /dev/null +++ b/drivers/block/sata_loopback.c @@ -0,0 +1,122 @@ +/*
- (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[] = "SATA loopback";
+static const char * const filenames[] = CONFIG_SATA_LOOP_DISKS; +static int max_devs = CONFIG_SYS_SATA_MAX_DEVICE;
One more thing is missing -- documentation for these (add to doc/ ). Alternatively (which would be much better), let sandbox uboot accept params and supply these as params.
+extern block_dev_desc_t sata_dev_desc[];
+int init_sata(int dev) +{
- block_dev_desc_t *pdev = &sata_dev_desc[dev];
- int fd;
- if ((dev < 0) || (dev >= max_devs)) {
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. */
- 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;
- 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;
- 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 info:\nfilename: %s\nsize: %lu\nblock count: %lu\n",
filenames[dev], bytes, pdev->lba);- return 0;
+}
Best regards, Marek Vasut
On Monday 03 of September 2012 18:49:00 Marek Vasut wrote:
+static const char revision[] = "0.0"; +static const char vendor[] = "SATA loopback";
+static const char * const filenames[] = CONFIG_SATA_LOOP_DISKS; +static int max_devs = CONFIG_SYS_SATA_MAX_DEVICE;
One more thing is missing -- documentation for these (add to doc/ ).
which file shoud that be exactly? README.sata has nothing about configs, i cannot find any README.sandbox or README.configs
Alternatively (which would be much better), let sandbox uboot accept params and supply these as params.
or even make a command that would allow you to specify a filename in runtime, possibly with a dynamic number of ports.
Pavel Herrmann
Dear Pavel Herrmann,
On Monday 03 of September 2012 18:49:00 Marek Vasut wrote:
+static const char revision[] = "0.0"; +static const char vendor[] = "SATA loopback";
+static const char * const filenames[] = CONFIG_SATA_LOOP_DISKS; +static int max_devs = CONFIG_SYS_SATA_MAX_DEVICE;
One more thing is missing -- documentation for these (add to doc/ ).
which file shoud that be exactly? README.sata has nothing about configs, i cannot find any README.sandbox or README.configs
Create one ... especially for the newly added feature -- eg. README.sata_loopback or something,
Alternatively (which would be much better), let sandbox uboot accept params and supply these as params.
or even make a command that would allow you to specify a filename in runtime, possibly with a dynamic number of ports.
Good idea, indeed.
Pavel Herrmann
Best regards, Marek Vasut
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
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" +);
Enable SATA_LOOP and a few disk-related commands for sandbox
Signed-off-by: Pavel Herrmann morpheus.ibis@gmail.com --- Changes for v3: drop static names for loop devices
Changes for v2: add a few words of description
include/configs/sandbox.h | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index 0220386..c238feb 100644 --- a/include/configs/sandbox.h +++ b/include/configs/sandbox.h @@ -93,4 +93,12 @@ "stdout=serial\0" \ "stderr=serial\0"
+/* SATA loopback device */ +#define CONFIG_CMD_SATA +#define CONFIG_SATA_LOOP +#define CONFIG_SYS_SATA_MAX_DEVICE 3 +#define CONFIG_DOS_PARTITION +#define CONFIG_CMD_FAT +#define CONFIG_CMD_EXT2 + #endif
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.
+);
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.
+);
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
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,
[...]
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
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 v4: checkpatch fixes use NULLs instead of "" extend sata_loop command
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 | 212 ++++++++++++++++++++++++++++++++++++++++++ include/configs/sandbox.h | 8 ++ 3 files changed, 221 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..600fbdc --- /dev/null +++ b/drivers/block/sata_loopback.c @@ -0,0 +1,212 @@ +/* + * (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"; + +/* this will be auto-initialized to NULLs */ +static char *filenames[CONFIG_SYS_SATA_MAX_DEVICE]; + +extern block_dev_desc_t sata_dev_desc[]; + +static inline int range_check(int dev) +{ + return (dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE); +} + +int init_sata(int dev) +{ + block_dev_desc_t *pdev = &sata_dev_desc[dev]; + int fd, old_fd; + + if (range_check(dev)) { + printf("File index %d is out of range.\n", dev); + return -EINVAL; + } + + if (filenames[dev]) + fd = os_open(filenames[dev], OS_O_RDWR); + else + fd = -1; + + old_fd = (long) pdev->priv; + /* This is ugly, but saves allocation for 1 int. */ + pdev->priv = (void *) (long) fd; + + 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; + char *filename; + lbaint_t bytes = 0; + + if (range_check(dev)) { + printf("File index %d is out of range.\n", dev); + return -EINVAL; + } + + if (filenames[dev]) + filename = filenames[dev]; + else + filename = ""; + + memcpy(pdev->vendor, vendor, sizeof(vendor)); + memcpy(pdev->revision, revision, sizeof(revision)); + namelen = strlen(filename); + + if (namelen > 20) { + /* take the last 17 chars, prepend them with "..." */ + memcpy(pdev->product, "...", 3); + memcpy(pdev->product+3, filename + (namelen - 17), 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, filename, 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, filename); + } + + return 0; +} + + +int do_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + int dev = 0; + + switch (argc) { + case 0: + case 1: + return CMD_RET_USAGE; + case 2: + if (!strncmp(argv[1], "inf", 3)) { + for (dev = 0; dev < CONFIG_SYS_SATA_MAX_DEVICE; dev++) + scan_sata(dev); + return 0; + } + return CMD_RET_USAGE; + 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); + /* + * init_sata() and scan_sata() do their own range + * check, however we need to explicitly do it here + * as well. + */ + if (range_check(dev)) { + printf("File index %d is out of range.\n", dev); + return -EINVAL; + } + free(filenames[dev]); + filenames[dev] = strdup(argv[3]); + init_sata(dev); + scan_sata(dev); + /* + * Scan the partition table if we succeeded in loading + * the new loop file. + */ + if (sata_dev_desc[dev].lba > 0) + init_part(&sata_dev_desc[dev]); + + return 0; + } + return CMD_RET_USAGE; + } + return CMD_RET_USAGE; +} + +U_BOOT_CMD( + sata_loop, 4, 1, do_loop, + "SATA loopback", + "info - show info about all loop devices\n" + "sata_loop info devnum - show info about loop devnum\n" + "sata_loop load devnum file - load file from host FS into loop devnum" +); diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index 0220386..a713430 100644 --- a/include/configs/sandbox.h +++ b/include/configs/sandbox.h @@ -93,4 +93,12 @@ "stdout=serial\0" \ "stderr=serial\0"
+/* SATA loopback device */ +#define CONFIG_CMD_SATA +#define CONFIG_SATA_LOOP +#define CONFIG_SYS_SATA_MAX_DEVICE 2 +#define CONFIG_DOS_PARTITION +#define CONFIG_CMD_FAT +#define CONFIG_CMD_EXT2 + #endif
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.
WARNING: externs should be avoided in .c files #141: FILE: drivers/block/sata_loopback.c:39: +extern block_dev_desc_t sata_dev_desc[];
total: 0 errors, 1 warnings, 231 lines checked
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE
Signed-off-by: Pavel Herrmann morpheus.ibis@gmail.com CC: Marek Vasut marex@denx.de CC: Mike Frysinger vapier@gentoo.org
Changes for v4: checkpatch fixes use NULLs instead of "" extend sata_loop command
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 | 212 ++++++++++++++++++++++++++++++++++++++++++ include/configs/sandbox.h | 8 ++ 3 files changed, 221 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..600fbdc --- /dev/null +++ b/drivers/block/sata_loopback.c @@ -0,0 +1,212 @@ +/*
- (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";
+/* this will be auto-initialized to NULLs */ +static char *filenames[CONFIG_SYS_SATA_MAX_DEVICE];
+extern block_dev_desc_t sata_dev_desc[];
+static inline int range_check(int dev) +{
- return (dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE);
+}
+int init_sata(int dev) +{
- block_dev_desc_t *pdev = &sata_dev_desc[dev];
- int fd, old_fd;
- if (range_check(dev)) {
printf("File index %d is out of range.\n", dev);return -EINVAL;- }
- if (filenames[dev])
fd = os_open(filenames[dev], OS_O_RDWR);- else
fd = -1;- old_fd = (long) pdev->priv;
- /* This is ugly, but saves allocation for 1 int. */
- pdev->priv = (void *) (long) fd;
- 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;
- char *filename;
- lbaint_t bytes = 0;
- if (range_check(dev)) {
printf("File index %d is out of range.\n", dev);return -EINVAL;- }
- if (filenames[dev])
filename = filenames[dev];- else
filename = "";- memcpy(pdev->vendor, vendor, sizeof(vendor));
- memcpy(pdev->revision, revision, sizeof(revision));
- namelen = strlen(filename);
- if (namelen > 20) {
/* take the last 17 chars, prepend them with "..." */memcpy(pdev->product, "...", 3);memcpy(pdev->product+3, filename + (namelen - 17), 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, filename, 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, filename);- }
- return 0;
+}
+int do_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{
- int dev = 0;
- switch (argc) {
- case 0:
- case 1:
return CMD_RET_USAGE;- case 2:
if (!strncmp(argv[1], "inf", 3)) {for (dev = 0; dev < CONFIG_SYS_SATA_MAX_DEVICE; dev++)scan_sata(dev);return 0;}return CMD_RET_USAGE;- 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);/** init_sata() and scan_sata() do their own range* check, however we need to explicitly do it here* as well.*/if (range_check(dev)) {printf("File index %d is out of range.\n", dev);return -EINVAL;}free(filenames[dev]);filenames[dev] = strdup(argv[3]);init_sata(dev);scan_sata(dev);/** Scan the partition table if we succeeded in loading* the new loop file.*/if (sata_dev_desc[dev].lba > 0)init_part(&sata_dev_desc[dev]);return 0;}return CMD_RET_USAGE;- }
- return CMD_RET_USAGE;
+}
+U_BOOT_CMD(
- sata_loop, 4, 1, do_loop,
- "SATA loopback",
- "info - show info about all loop devices\n"
- "sata_loop info devnum - show info about loop devnum\n"
- "sata_loop load devnum file - load file from host FS into loop devnum"
+); diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index 0220386..a713430 100644 --- a/include/configs/sandbox.h +++ b/include/configs/sandbox.h @@ -93,4 +93,12 @@ "stdout=serial\0" \ "stderr=serial\0"
+/* SATA loopback device */ +#define CONFIG_CMD_SATA +#define CONFIG_SATA_LOOP +#define CONFIG_SYS_SATA_MAX_DEVICE 2 +#define CONFIG_DOS_PARTITION +#define CONFIG_CMD_FAT +#define CONFIG_CMD_EXT2
#endif
On Friday 07 of September 2012 01:29:55 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.
WARNING: externs should be avoided in .c files #141: FILE: drivers/block/sata_loopback.c:39: +extern block_dev_desc_t sata_dev_desc[];
total: 0 errors, 1 warnings, 231 lines checked
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE
Yes, i know about that, and chose to ignore it. i will not create a header file for a single line that checkpatch doesnt like (with just a warning), especially when other drivers have such a line in them as well.
Signed-off-by: Pavel Herrmann morpheus.ibis@gmail.com CC: Marek Vasut marex@denx.de CC: Mike Frysinger vapier@gentoo.org
Changes for v4: checkpatch fixes use NULLs instead of "" extend sata_loop command
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 | 212
++++++++++++++++++++++++++++++++++++++++++ include/configs/sandbox.h |
8 ++
3 files changed, 221 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..600fbdc --- /dev/null +++ b/drivers/block/sata_loopback.c @@ -0,0 +1,212 @@ +/*
- (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";
+/* this will be auto-initialized to NULLs */ +static char *filenames[CONFIG_SYS_SATA_MAX_DEVICE];
+extern block_dev_desc_t sata_dev_desc[];
+static inline int range_check(int dev) +{
- return (dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE);
+}
+int init_sata(int dev) +{
- block_dev_desc_t *pdev = &sata_dev_desc[dev];
- int fd, old_fd;
- if (range_check(dev)) {
printf("File index %d is out of range.\n", dev);return -EINVAL;- }
- if (filenames[dev])
fd = os_open(filenames[dev], OS_O_RDWR);- else
fd = -1;- old_fd = (long) pdev->priv;
- /* This is ugly, but saves allocation for 1 int. */
- pdev->priv = (void *) (long) fd;
- 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;
- char *filename;
- lbaint_t bytes = 0;
- if (range_check(dev)) {
printf("File index %d is out of range.\n", dev);return -EINVAL;- }
- if (filenames[dev])
filename = filenames[dev];- else
filename = "";- memcpy(pdev->vendor, vendor, sizeof(vendor));
- memcpy(pdev->revision, revision, sizeof(revision));
- namelen = strlen(filename);
- if (namelen > 20) {
/* take the last 17 chars, prepend them with "..." */memcpy(pdev->product, "...", 3);memcpy(pdev->product+3, filename + (namelen - 17), 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, filename, 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, filename);- }
- return 0;
+}
+int do_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{
- int dev = 0;
- switch (argc) {
- case 0:
- case 1:
return CMD_RET_USAGE;- case 2:
if (!strncmp(argv[1], "inf", 3)) {for (dev = 0; dev < CONFIG_SYS_SATA_MAX_DEVICE; dev++)scan_sata(dev);return 0;}return CMD_RET_USAGE;- 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);/** init_sata() and scan_sata() do their own range* check, however we need to explicitly do it here* as well.*/if (range_check(dev)) {printf("File index %d is out of range.\n", dev);return -EINVAL;}free(filenames[dev]);filenames[dev] = strdup(argv[3]);init_sata(dev);scan_sata(dev);/** Scan the partition table if we succeeded in loading* the new loop file.*/if (sata_dev_desc[dev].lba > 0)init_part(&sata_dev_desc[dev]);return 0;}return CMD_RET_USAGE;- }
- return CMD_RET_USAGE;
+}
+U_BOOT_CMD(
- sata_loop, 4, 1, do_loop,
- "SATA loopback",
- "info - show info about all loop devices\n"
- "sata_loop info devnum - show info about loop devnum\n"
- "sata_loop load devnum file - load file from host FS into loop devnum"
+); diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index 0220386..a713430 100644 --- a/include/configs/sandbox.h +++ b/include/configs/sandbox.h @@ -93,4 +93,12 @@
"stdout=serial\0" \ "stderr=serial\0"+/* SATA loopback device */ +#define CONFIG_CMD_SATA +#define CONFIG_SATA_LOOP +#define CONFIG_SYS_SATA_MAX_DEVICE 2 +#define CONFIG_DOS_PARTITION +#define CONFIG_CMD_FAT +#define CONFIG_CMD_EXT2
#endif
Dear Pavel Herrmann,
On Friday 07 of September 2012 01:29:55 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.
WARNING: externs should be avoided in .c files #141: FILE: drivers/block/sata_loopback.c:39: +extern block_dev_desc_t sata_dev_desc[];
total: 0 errors, 1 warnings, 231 lines checked
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE
Yes, i know about that, and chose to ignore it.
Sorry, this is not how it works.
i will not create a header file for a single line that checkpatch doesnt like (with just a warning), especially when other drivers have such a line in them as well.
So, implement proper accessor for this list? Or export the structure through some sata-related header? How does other do it?
Signed-off-by: Pavel Herrmann morpheus.ibis@gmail.com CC: Marek Vasut marex@denx.de CC: Mike Frysinger vapier@gentoo.org
Changes for v4: checkpatch fixes use NULLs instead of "" extend sata_loop command
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 | 212
++++++++++++++++++++++++++++++++++++++++++ include/configs/sandbox.h |
8 ++
3 files changed, 221 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..600fbdc --- /dev/null +++ b/drivers/block/sata_loopback.c @@ -0,0 +1,212 @@ +/*
- (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";
+/* this will be auto-initialized to NULLs */ +static char *filenames[CONFIG_SYS_SATA_MAX_DEVICE];
+extern block_dev_desc_t sata_dev_desc[];
+static inline int range_check(int dev) +{
- return (dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE);
+}
+int init_sata(int dev) +{
- block_dev_desc_t *pdev = &sata_dev_desc[dev];
- int fd, old_fd;
- if (range_check(dev)) {
printf("File index %d is out of range.\n", dev);return -EINVAL;- }
- if (filenames[dev])
fd = os_open(filenames[dev], OS_O_RDWR);- else
fd = -1;- old_fd = (long) pdev->priv;
- /* This is ugly, but saves allocation for 1 int. */
- pdev->priv = (void *) (long) fd;
- 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;
- char *filename;
- lbaint_t bytes = 0;
- if (range_check(dev)) {
printf("File index %d is out of range.\n", dev);return -EINVAL;- }
- if (filenames[dev])
filename = filenames[dev];- else
filename = "";- memcpy(pdev->vendor, vendor, sizeof(vendor));
- memcpy(pdev->revision, revision, sizeof(revision));
- namelen = strlen(filename);
- if (namelen > 20) {
/* take the last 17 chars, prepend them with "..." */memcpy(pdev->product, "...", 3);memcpy(pdev->product+3, filename + (namelen - 17), 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, filename, 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, filename);- }
- return 0;
+}
+int do_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{
- int dev = 0;
- switch (argc) {
- case 0:
- case 1:
return CMD_RET_USAGE;- case 2:
if (!strncmp(argv[1], "inf", 3)) {for (dev = 0; dev < CONFIG_SYS_SATA_MAX_DEVICE; dev++)scan_sata(dev);return 0;}return CMD_RET_USAGE;- 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);/** init_sata() and scan_sata() do their own range* check, however we need to explicitly do it here* as well.*/if (range_check(dev)) {printf("File index %d is out of range.\n", dev);return -EINVAL;}free(filenames[dev]);filenames[dev] = strdup(argv[3]);init_sata(dev);scan_sata(dev);/** Scan the partition table if we succeeded in loading* the new loop file.*/if (sata_dev_desc[dev].lba > 0)init_part(&sata_dev_desc[dev]);return 0;}return CMD_RET_USAGE;- }
- return CMD_RET_USAGE;
+}
+U_BOOT_CMD(
- sata_loop, 4, 1, do_loop,
- "SATA loopback",
- "info - show info about all loop devices\n"
- "sata_loop info devnum - show info about loop devnum\n"
- "sata_loop load devnum file - load file from host FS into loop
devnum" +); diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h index 0220386..a713430 100644 --- a/include/configs/sandbox.h +++ b/include/configs/sandbox.h @@ -93,4 +93,12 @@
"stdout=serial\0" \ "stderr=serial\0"+/* SATA loopback device */ +#define CONFIG_CMD_SATA +#define CONFIG_SATA_LOOP +#define CONFIG_SYS_SATA_MAX_DEVICE 2 +#define CONFIG_DOS_PARTITION +#define CONFIG_CMD_FAT +#define CONFIG_CMD_EXT2
#endif
On Friday 07 of September 2012 11:26:48 Marek Vasut wrote:
Dear Pavel Herrmann,
On Friday 07 of September 2012 01:29:55 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.
WARNING: externs should be avoided in .c files #141: FILE: drivers/block/sata_loopback.c:39: +extern block_dev_desc_t sata_dev_desc[];
total: 0 errors, 1 warnings, 231 lines checked
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE
Yes, i know about that, and chose to ignore it.
Sorry, this is not how it works.
i will not create a header file for a single line that checkpatch doesnt like (with just a warning), especially when other drivers have such a line in them as well.
So, implement proper accessor for this list? Or export the structure through some sata-related header? How does other do it?
other drivers have the same line that i do, therefore ignore the checkpatch warning, same as i do
see sata_dwc.c, ata_piix.c, fsl_sata.c, sata_sil3114.c
Pavel Herrmann
Dear Pavel Herrmann,
On Friday 07 of September 2012 11:26:48 Marek Vasut wrote:
Dear Pavel Herrmann,
On Friday 07 of September 2012 01:29:55 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.
WARNING: externs should be avoided in .c files #141: FILE: drivers/block/sata_loopback.c:39: +extern block_dev_desc_t sata_dev_desc[];
total: 0 errors, 1 warnings, 231 lines checked
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE
Yes, i know about that, and chose to ignore it.
Sorry, this is not how it works.
i will not create a header file for a single line that checkpatch doesnt like (with just a warning), especially when other drivers have such a line in them as well.
So, implement proper accessor for this list? Or export the structure through some sata-related header? How does other do it?
other drivers have the same line that i do, therefore ignore the checkpatch warning, same as i do
Oh god damned! This is definitelly not the way to go!
NAK! NAK! NAK!! NAK!!! NAK!!!!! NAK!!!!!!!! ... you get the idea ;-)
see sata_dwc.c, ata_piix.c, fsl_sata.c, sata_sil3114.c
Sad state of codebase, thanks for pointing out the obvious :-( You know, now that you pointed it out, you also know what to do :-)
Besides, you'd have to do it anyway, so let's do it now.
Pavel Herrmann
Best regards, Marek Vasut
On Fri, Sep 07, 2012 at 11:19:03AM +0200, Pavel Herrmann wrote:
On Friday 07 of September 2012 01:29:55 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.
WARNING: externs should be avoided in .c files #141: FILE: drivers/block/sata_loopback.c:39: +extern block_dev_desc_t sata_dev_desc[];
total: 0 errors, 1 warnings, 231 lines checked
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE
Yes, i know about that, and chose to ignore it. i will not create a header file for a single line that checkpatch doesnt like (with just a warning), especially when other drivers have such a line in them as well.
Please move your extern into <sata.h>, as the starting point before merge. If you could also go and fix: $ git grep -l sata_dev_desc common/cmd_sata.c drivers/block/ata_piix.c drivers/block/dwc_ahsata.c drivers/block/dwc_ahsata.h drivers/block/fsl_sata.c drivers/block/pata_bfin.c drivers/block/pata_bfin.h drivers/block/sata_dwc.c drivers/block/sata_sil.c drivers/block/sata_sil.h drivers/block/sata_sil3114.c
To use <sata.h> (some already do) instead of their own private extern I'd greatly appreciate it.
Hi
On Thursday 13 September 2012 15:31:39 Tom Rini wrote:
On Fri, Sep 07, 2012 at 11:19:03AM +0200, Pavel Herrmann wrote:
On Friday 07 of September 2012 01:29:55 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.
WARNING: externs should be avoided in .c files #141: FILE: drivers/block/sata_loopback.c:39: +extern block_dev_desc_t sata_dev_desc[];
total: 0 errors, 1 warnings, 231 lines checked
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE
Yes, i know about that, and chose to ignore it. i will not create a header file for a single line that checkpatch doesnt like (with just a warning), especially when other drivers have such a line in them as well.
Please move your extern into <sata.h>, as the starting point before merge. If you could also go and fix: $ git grep -l sata_dev_desc common/cmd_sata.c drivers/block/ata_piix.c drivers/block/dwc_ahsata.c drivers/block/dwc_ahsata.h drivers/block/fsl_sata.c drivers/block/pata_bfin.c drivers/block/pata_bfin.h drivers/block/sata_dwc.c drivers/block/sata_sil.c drivers/block/sata_sil.h drivers/block/sata_sil3114.c
To use <sata.h> (some already do) instead of their own private extern I'd greatly appreciate it.
I dont think <sata.h> is the place to put this, as it is a "consumer-facing" header. rather create some common sata header in drivers/block
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 --- Changes for v5: use common sata extern header - this requires "[PATCH] Fix checkpatch warnings about externs in *.c" to be merged first make sure we have valid callbacks before scanning partitions
Changes for v4: checkpatch fixes use NULLs instead of "" extend sata_loop command
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 | 213 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 214 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..eb3103b --- /dev/null +++ b/drivers/block/sata_loopback.c @@ -0,0 +1,213 @@ +/* + * (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> +#include "sata_extern.h" + +static const char revision[] = "0.0"; +static const char vendor[] = "SATA loopback"; + +/* this will be auto-initialized to NULLs */ +static char *filenames[CONFIG_SYS_SATA_MAX_DEVICE]; + +static inline int range_check(int dev) +{ + return (dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE); +} + +int init_sata(int dev) +{ + block_dev_desc_t *pdev = &sata_dev_desc[dev]; + int fd, old_fd; + + if (range_check(dev)) { + printf("File index %d is out of range.\n", dev); + return -EINVAL; + } + + if (filenames[dev]) + fd = os_open(filenames[dev], OS_O_RDWR); + else + fd = -1; + + old_fd = (long) pdev->priv; + /* This is ugly, but saves allocation for 1 int. */ + pdev->priv = (void *) (long) fd; + + if ((old_fd > 2) || (old_fd < 0)) + os_close(old_fd); + + return 0; +} + +ulong sata_read(int dev, ulong 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; +} + +ulong sata_write(int dev, ulong start, lbaint_t blkcnt, const 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; + char *filename; + lbaint_t bytes = 0; + + if (range_check(dev)) { + printf("File index %d is out of range.\n", dev); + return -EINVAL; + } + + if (filenames[dev]) + filename = filenames[dev]; + else + filename = ""; + + memcpy(pdev->vendor, vendor, sizeof(vendor)); + memcpy(pdev->revision, revision, sizeof(revision)); + namelen = strlen(filename); + + if (namelen > 20) { + /* take the last 17 chars, prepend them with "..." */ + memcpy(pdev->product, "...", 3); + memcpy(pdev->product+3, filename + (namelen - 17), 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; + pdev->block_read = sata_read; + pdev->block_write = sata_write; + printf("SATA loop %d:\nfilename: %s\nsize: %lu\nblock count:" + " %lu\n", dev, filename, 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, filename); + } + + return 0; +} + + +int do_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + int dev = 0; + + switch (argc) { + case 0: + case 1: + return CMD_RET_USAGE; + case 2: + if (!strncmp(argv[1], "inf", 3)) { + for (dev = 0; dev < CONFIG_SYS_SATA_MAX_DEVICE; dev++) + scan_sata(dev); + return 0; + } + return CMD_RET_USAGE; + 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); + /* + * init_sata() and scan_sata() do their own range + * check, however we need to explicitly do it here + * as well. + */ + if (range_check(dev)) { + printf("File index %d is out of range.\n", dev); + return -EINVAL; + } + free(filenames[dev]); + filenames[dev] = strdup(argv[3]); + init_sata(dev); + scan_sata(dev); + /* + * Scan the partition table if we succeeded in loading + * the new loop file. + */ + if (sata_dev_desc[dev].lba > 0) + init_part(&sata_dev_desc[dev]); + + return 0; + } + return CMD_RET_USAGE; + } + return CMD_RET_USAGE; +} + +U_BOOT_CMD( + sata_loop, 4, 1, do_loop, + "SATA loopback", + "info - show info about all loop devices\n" + "sata_loop 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 --- Changes for v6: sync with new version of "Fix checkpatch warnings about externs in *.c" set all constant disk properties even on not connected drivers
Changes for v5: use common sata extern header - this requires "[PATCH] Fix checkpatch warnings about externs in *.c" to be merged first make sure we have valid callbacks before scanning partitions
Changes for v4: checkpatch fixes use NULLs instead of "" extend sata_loop command
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/sata_loopback.c | 209 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 209 insertions(+) create mode 100644 drivers/block/sata_loopback.c
diff --git a/drivers/block/sata_loopback.c b/drivers/block/sata_loopback.c new file mode 100644 index 0000000..b55b270 --- /dev/null +++ b/drivers/block/sata_loopback.c @@ -0,0 +1,209 @@ +/* + * (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 <sata.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"; + +/* this will be auto-initialized to NULLs */ +static char *filenames[CONFIG_SYS_SATA_MAX_DEVICE]; + +static inline int range_check(int dev) +{ + return (dev < 0) || (dev >= CONFIG_SYS_SATA_MAX_DEVICE); +} + +int init_sata(int dev) +{ + block_dev_desc_t *pdev = &sata_dev_desc[dev]; + int fd, old_fd; + + if (range_check(dev)) { + printf("File index %d is out of range.\n", dev); + return -EINVAL; + } + + if (filenames[dev]) + fd = os_open(filenames[dev], OS_O_RDWR); + else + fd = -1; + + old_fd = (long) pdev->priv; + /* This is ugly, but saves allocation for 1 int. */ + pdev->priv = (void *) (long) fd; + + if ((old_fd > 2) || (old_fd < 0)) + os_close(old_fd); + + return 0; +} + +ulong sata_read(int dev, ulong 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; +} + +ulong sata_write(int dev, ulong start, lbaint_t blkcnt, const 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; + char *filename; + lbaint_t bytes = 0; + + if (range_check(dev)) { + printf("File index %d is out of range.\n", dev); + return -EINVAL; + } + + if (filenames[dev]) + filename = filenames[dev]; + else + filename = ""; + + memcpy(pdev->vendor, vendor, sizeof(vendor)); + memcpy(pdev->revision, revision, sizeof(revision)); + namelen = strlen(filename); + + if (namelen > 20) { + /* take the last 17 chars, prepend them with "..." */ + memcpy(pdev->product, "...", 3); + memcpy(pdev->product+3, filename + (namelen - 17), 17); + } else + memcpy(pdev->product, filename, namelen); + + pdev->product[20] = 0; + pdev->type = DEV_TYPE_HARDDISK; + pdev->blksz = ATA_SECT_SIZE; + pdev->lun = 0; + pdev->block_read = sata_read; + pdev->block_write = sata_write; + + bytes = os_lseek(fd, 0, OS_SEEK_END); + if (bytes != -1) { + pdev->lba = bytes/ATA_SECT_SIZE; + printf("SATA loop %d:\nfilename: %s\nsize: %lu\nblock count:" + " %lu\n", dev, filename, bytes, pdev->lba); + } else { + pdev->lba = 0; + printf("SATA loop %d:\nfilename: %s\nFAILED TO OPEN\n", + dev, filename); + } + + return 0; +} + + +int do_loop(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + int dev = 0; + + switch (argc) { + case 0: + case 1: + return CMD_RET_USAGE; + case 2: + if (!strncmp(argv[1], "inf", 3)) { + for (dev = 0; dev < CONFIG_SYS_SATA_MAX_DEVICE; dev++) + scan_sata(dev); + return 0; + } + return CMD_RET_USAGE; + 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); + /* + * init_sata() and scan_sata() do their own range + * check, however we need to explicitly do it here + * as well. + */ + if (range_check(dev)) { + printf("File index %d is out of range.\n", dev); + return -EINVAL; + } + free(filenames[dev]); + filenames[dev] = strdup(argv[3]); + init_sata(dev); + scan_sata(dev); + /* + * Scan the partition table if we succeeded in loading + * the new loop file. + */ + if (sata_dev_desc[dev].lba > 0) + init_part(&sata_dev_desc[dev]); + + return 0; + } + return CMD_RET_USAGE; + } + return CMD_RET_USAGE; +} + +U_BOOT_CMD( + sata_loop, 4, 1, do_loop, + "SATA loopback", + "info - show info about all loop devices\n" + "sata_loop info devnum - show info about loop devnum\n" + "sata_loop load devnum file - load file from host FS into loop devnum" +);
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.
The description might use a little bit of caressing here :)
[...]
- if (os_lseek(fd, start_byte, OS_SEEK_SET) != start_byte)
return -1;
Use errno consistently ... so -EINVAL ? Or something else?
- retval = os_read(fd, buffer, length_byte);
- return retval / ATA_SECT_SIZE;
+}
[...]
if (!strncmp(argv[1], "load", 4)) {
if (strncmp()) return CMD_USAGE;
one indent less here ;-)
dev = simple_strtoul(argv[2], NULL, 10);/** init_sata() and scan_sata() do their own range* check, however we need to explicitly do it here* as well.*/if (range_check(dev)) {printf("File index %d is out of range.\n", dev);return -EINVAL;}free(filenames[dev]);filenames[dev] = strdup(argv[3]);init_sata(dev);scan_sata(dev);/** Scan the partition table if we succeeded in loading* the new loop file.*/if (sata_dev_desc[dev].lba > 0)init_part(&sata_dev_desc[dev]);return 0;}return CMD_RET_USAGE;- }
- return CMD_RET_USAGE;
+}
+U_BOOT_CMD(
- sata_loop, 4, 1, do_loop,
- "SATA loopback",
- "info - show info about all loop devices\n"
- "sata_loop info devnum - show info about loop devnum\n"
- "sata_loop load devnum file - load file from host FS into loop devnum"
+);
participants (4)
-
Marek Vasut -
Marek Vasut -
Pavel Herrmann -
Tom Rini