Re: [U-Boot] [PATCH v2 03/40] dm: blk: Add a generic function for block device commands
Hi Simon,
On Thu, Aug 31, 2017 at 8:51 PM, Simon Glass wrote:
> Hi Bin,
>
> On 1 August 2017 at 12:56, Bin Meng wrote:
>> Hi Simon,
>>
>> On Sun, Jul 30, 2017 at 1:34 AM, Simon Glass wrote:
>>> Most block devices provide a command (e.g. 'sata', 'scsi', 'ide') and
>>> these commands generally do the same thing. This makes it harder to
>>> maintain this code and keep it consistent.
>>>
>>> We now have a block device interface which is either implemented by driver
>>> model (when CONFIG_BLK is enabled) or with a legacy interface. Therefore
>>> it is possible to handle most of what these commands do with generic code.
>>>
>>> Add a new generic function to process block-device commands using the
>>> interface type and the current device number for that type.
>>>
>>> Signed-off-by: Simon Glass
>>> ---
>>>
>>> Changes in v2: None
>>>
>>> cmd/Makefile | 1 +
>>> cmd/blk_common.c | 104
>>> +++
>>> include/blk.h| 12 +++
>>> 3 files changed, 117 insertions(+)
>>> create mode 100644 cmd/blk_common.c
>>>
>>> diff --git a/cmd/Makefile b/cmd/Makefile
>>> index bd231f24d8..e2b3a9cc71 100644
>>> --- a/cmd/Makefile
>>> +++ b/cmd/Makefile
>>> @@ -15,6 +15,7 @@ obj-y += version.o
>>> # command
>>> obj-$(CONFIG_CMD_AES) += aes.o
>>> obj-$(CONFIG_CMD_ARMFLASH) += armflash.o
>>> +obj-y += blk_common.o
>>> obj-$(CONFIG_SOURCE) += source.o
>>> obj-$(CONFIG_CMD_SOURCE) += source.o
>>> obj-$(CONFIG_CMD_BDI) += bdinfo.o
>>> diff --git a/cmd/blk_common.c b/cmd/blk_common.c
>>> new file mode 100644
>>> index 00..86c75e78d8
>>> --- /dev/null
>>> +++ b/cmd/blk_common.c
>>> @@ -0,0 +1,104 @@
>>> +/*
>>> + * Handling of common block commands
>>> + *
>>> + * Copyright (c) 2017 Google, Inc
>>> + *
>>> + * (C) Copyright 2000-2011
>>> + * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
>>> + *
>>> + * SPDX-License-Identifier:GPL-2.0+
>>> + */
>>> +
>>> +#include
>>> +#include
>>> +
>>> +#ifdef HAVE_BLOCK_DEVICE
>>> +int blk_common_cmd(int argc, char * const argv[], enum if_type if_type,
>>> + int *cur_devnump)
>>> +{
>>> + const char *if_name = blk_get_if_type_name(if_type);
>>> +
>>> + switch (argc) {
>>> + case 0:
>>> + case 1:
>>> + return CMD_RET_USAGE;
>>> + case 2:
>>> + if (strncmp(argv[1], "inf", 3) == 0) {
>>> + blk_list_devices(if_type);
>>> + return 0;
>>> + } else if (strncmp(argv[1], "dev", 3) == 0) {
>>> + if (blk_print_device_num(if_type, *cur_devnump)) {
>>> + printf("\nno %s devices available\n",
>>> if_name);
>>> + return CMD_RET_FAILURE;
>>> + }
>>> + return 0;
>>> + } else if (strncmp(argv[1], "part", 4) == 0) {
>>> + if (blk_list_part(if_type))
>>> + printf("\nno %s devices available\n",
>>> if_name);
>>> + return 0;
>>> + }
>>> + return CMD_RET_USAGE;
>>> + case 3:
>>> + if (strncmp(argv[1], "dev", 3) == 0) {
>>> + int dev = (int)simple_strtoul(argv[2], NULL, 10);
>>> +
>>> + if (!blk_show_device(if_type, dev)) {
>>> + *cur_devnump = dev;
>>> + printf("... is now current device\n");
>>> + } else {
>>> + return CMD_RET_FAILURE;
>>> + }
>>> + return 0;
>>> + } else if (strncmp(argv[1], "part", 4) == 0) {
>>> + int dev = (int)simple_strtoul(argv[2], NULL, 10);
>>> +
>>> + if (blk_print_part_devnum(if_type, dev)) {
>>> + printf("\n%s device %d not available\n",
>>> + if_name, dev);
>>> + return CMD_RET_FAILURE;
>>> + }
>>> + return 0;
>>> + }
>>> + return CMD_RET_USAGE;
>>> +
>>> + default: /* at least 4 args */
>>> + if (strcmp(argv[1], "read") == 0) {
>>> + ulong addr = simple_strtoul(argv[2], NULL, 16);
>>> + lbaint_t blk = simple_strtoul(argv[3], NULL, 16);
>>> + ulong cnt = simple_strtoul(argv[4], NULL, 16);
>>> + ulong n;
>>> +
>>> + printf("\n%s read: device %d block # %lld, count
>>> %ld ... ",
>>> + if_name, *cur_devnump, (unsigned long
>>> long)blk,
>>> + cnt);
>>
>> Please use LBAFU for parameter blk, to handle both 32-bit and 64-bit LBA.
>
> It looks like you have kindly done a patch for that?
>
Yes, http://patchwor
Re: [U-Boot] [PATCH v2 03/40] dm: blk: Add a generic function for block device commands
Hi Bin,
On 1 August 2017 at 12:56, Bin Meng wrote:
> Hi Simon,
>
> On Sun, Jul 30, 2017 at 1:34 AM, Simon Glass wrote:
>> Most block devices provide a command (e.g. 'sata', 'scsi', 'ide') and
>> these commands generally do the same thing. This makes it harder to
>> maintain this code and keep it consistent.
>>
>> We now have a block device interface which is either implemented by driver
>> model (when CONFIG_BLK is enabled) or with a legacy interface. Therefore
>> it is possible to handle most of what these commands do with generic code.
>>
>> Add a new generic function to process block-device commands using the
>> interface type and the current device number for that type.
>>
>> Signed-off-by: Simon Glass
>> ---
>>
>> Changes in v2: None
>>
>> cmd/Makefile | 1 +
>> cmd/blk_common.c | 104
>> +++
>> include/blk.h| 12 +++
>> 3 files changed, 117 insertions(+)
>> create mode 100644 cmd/blk_common.c
>>
>> diff --git a/cmd/Makefile b/cmd/Makefile
>> index bd231f24d8..e2b3a9cc71 100644
>> --- a/cmd/Makefile
>> +++ b/cmd/Makefile
>> @@ -15,6 +15,7 @@ obj-y += version.o
>> # command
>> obj-$(CONFIG_CMD_AES) += aes.o
>> obj-$(CONFIG_CMD_ARMFLASH) += armflash.o
>> +obj-y += blk_common.o
>> obj-$(CONFIG_SOURCE) += source.o
>> obj-$(CONFIG_CMD_SOURCE) += source.o
>> obj-$(CONFIG_CMD_BDI) += bdinfo.o
>> diff --git a/cmd/blk_common.c b/cmd/blk_common.c
>> new file mode 100644
>> index 00..86c75e78d8
>> --- /dev/null
>> +++ b/cmd/blk_common.c
>> @@ -0,0 +1,104 @@
>> +/*
>> + * Handling of common block commands
>> + *
>> + * Copyright (c) 2017 Google, Inc
>> + *
>> + * (C) Copyright 2000-2011
>> + * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
>> + *
>> + * SPDX-License-Identifier:GPL-2.0+
>> + */
>> +
>> +#include
>> +#include
>> +
>> +#ifdef HAVE_BLOCK_DEVICE
>> +int blk_common_cmd(int argc, char * const argv[], enum if_type if_type,
>> + int *cur_devnump)
>> +{
>> + const char *if_name = blk_get_if_type_name(if_type);
>> +
>> + switch (argc) {
>> + case 0:
>> + case 1:
>> + return CMD_RET_USAGE;
>> + case 2:
>> + if (strncmp(argv[1], "inf", 3) == 0) {
>> + blk_list_devices(if_type);
>> + return 0;
>> + } else if (strncmp(argv[1], "dev", 3) == 0) {
>> + if (blk_print_device_num(if_type, *cur_devnump)) {
>> + printf("\nno %s devices available\n",
>> if_name);
>> + return CMD_RET_FAILURE;
>> + }
>> + return 0;
>> + } else if (strncmp(argv[1], "part", 4) == 0) {
>> + if (blk_list_part(if_type))
>> + printf("\nno %s devices available\n",
>> if_name);
>> + return 0;
>> + }
>> + return CMD_RET_USAGE;
>> + case 3:
>> + if (strncmp(argv[1], "dev", 3) == 0) {
>> + int dev = (int)simple_strtoul(argv[2], NULL, 10);
>> +
>> + if (!blk_show_device(if_type, dev)) {
>> + *cur_devnump = dev;
>> + printf("... is now current device\n");
>> + } else {
>> + return CMD_RET_FAILURE;
>> + }
>> + return 0;
>> + } else if (strncmp(argv[1], "part", 4) == 0) {
>> + int dev = (int)simple_strtoul(argv[2], NULL, 10);
>> +
>> + if (blk_print_part_devnum(if_type, dev)) {
>> + printf("\n%s device %d not available\n",
>> + if_name, dev);
>> + return CMD_RET_FAILURE;
>> + }
>> + return 0;
>> + }
>> + return CMD_RET_USAGE;
>> +
>> + default: /* at least 4 args */
>> + if (strcmp(argv[1], "read") == 0) {
>> + ulong addr = simple_strtoul(argv[2], NULL, 16);
>> + lbaint_t blk = simple_strtoul(argv[3], NULL, 16);
>> + ulong cnt = simple_strtoul(argv[4], NULL, 16);
>> + ulong n;
>> +
>> + printf("\n%s read: device %d block # %lld, count %ld
>> ... ",
>> + if_name, *cur_devnump, (unsigned long
>> long)blk,
>> + cnt);
>
> Please use LBAFU for parameter blk, to handle both 32-bit and 64-bit LBA.
It looks like you have kindly done a patch for that?
Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2 03/40] dm: blk: Add a generic function for block device commands
Hi Simon,
On Sun, Jul 30, 2017 at 1:34 AM, Simon Glass wrote:
> Most block devices provide a command (e.g. 'sata', 'scsi', 'ide') and
> these commands generally do the same thing. This makes it harder to
> maintain this code and keep it consistent.
>
> We now have a block device interface which is either implemented by driver
> model (when CONFIG_BLK is enabled) or with a legacy interface. Therefore
> it is possible to handle most of what these commands do with generic code.
>
> Add a new generic function to process block-device commands using the
> interface type and the current device number for that type.
>
> Signed-off-by: Simon Glass
> ---
>
> Changes in v2: None
>
> cmd/Makefile | 1 +
> cmd/blk_common.c | 104
> +++
> include/blk.h| 12 +++
> 3 files changed, 117 insertions(+)
> create mode 100644 cmd/blk_common.c
>
> diff --git a/cmd/Makefile b/cmd/Makefile
> index bd231f24d8..e2b3a9cc71 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -15,6 +15,7 @@ obj-y += version.o
> # command
> obj-$(CONFIG_CMD_AES) += aes.o
> obj-$(CONFIG_CMD_ARMFLASH) += armflash.o
> +obj-y += blk_common.o
> obj-$(CONFIG_SOURCE) += source.o
> obj-$(CONFIG_CMD_SOURCE) += source.o
> obj-$(CONFIG_CMD_BDI) += bdinfo.o
> diff --git a/cmd/blk_common.c b/cmd/blk_common.c
> new file mode 100644
> index 00..86c75e78d8
> --- /dev/null
> +++ b/cmd/blk_common.c
> @@ -0,0 +1,104 @@
> +/*
> + * Handling of common block commands
> + *
> + * Copyright (c) 2017 Google, Inc
> + *
> + * (C) Copyright 2000-2011
> + * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
> + *
> + * SPDX-License-Identifier:GPL-2.0+
> + */
> +
> +#include
> +#include
> +
> +#ifdef HAVE_BLOCK_DEVICE
> +int blk_common_cmd(int argc, char * const argv[], enum if_type if_type,
> + int *cur_devnump)
> +{
> + const char *if_name = blk_get_if_type_name(if_type);
> +
> + switch (argc) {
> + case 0:
> + case 1:
> + return CMD_RET_USAGE;
> + case 2:
> + if (strncmp(argv[1], "inf", 3) == 0) {
> + blk_list_devices(if_type);
> + return 0;
> + } else if (strncmp(argv[1], "dev", 3) == 0) {
> + if (blk_print_device_num(if_type, *cur_devnump)) {
> + printf("\nno %s devices available\n",
> if_name);
> + return CMD_RET_FAILURE;
> + }
> + return 0;
> + } else if (strncmp(argv[1], "part", 4) == 0) {
> + if (blk_list_part(if_type))
> + printf("\nno %s devices available\n",
> if_name);
> + return 0;
> + }
> + return CMD_RET_USAGE;
> + case 3:
> + if (strncmp(argv[1], "dev", 3) == 0) {
> + int dev = (int)simple_strtoul(argv[2], NULL, 10);
> +
> + if (!blk_show_device(if_type, dev)) {
> + *cur_devnump = dev;
> + printf("... is now current device\n");
> + } else {
> + return CMD_RET_FAILURE;
> + }
> + return 0;
> + } else if (strncmp(argv[1], "part", 4) == 0) {
> + int dev = (int)simple_strtoul(argv[2], NULL, 10);
> +
> + if (blk_print_part_devnum(if_type, dev)) {
> + printf("\n%s device %d not available\n",
> + if_name, dev);
> + return CMD_RET_FAILURE;
> + }
> + return 0;
> + }
> + return CMD_RET_USAGE;
> +
> + default: /* at least 4 args */
> + if (strcmp(argv[1], "read") == 0) {
> + ulong addr = simple_strtoul(argv[2], NULL, 16);
> + lbaint_t blk = simple_strtoul(argv[3], NULL, 16);
> + ulong cnt = simple_strtoul(argv[4], NULL, 16);
> + ulong n;
> +
> + printf("\n%s read: device %d block # %lld, count %ld
> ... ",
> + if_name, *cur_devnump, (unsigned long long)blk,
> + cnt);
Please use LBAFU for parameter blk, to handle both 32-bit and 64-bit LBA.
> +
> + n = blk_read_devnum(if_type, *cur_devnump, blk, cnt,
> + (ulong *)addr);
> +
> + printf("%ld blocks read: %s\n", n,
> + n == cnt ? "OK" : "ERROR");
> + return n == cnt ? 0 : 1;
> + } else if (strcmp(argv[1], "write") == 0) {
> + ulong addr = simple_strtoul(argv[2], NULL,
[U-Boot] [PATCH v2 03/40] dm: blk: Add a generic function for block device commands
Most block devices provide a command (e.g. 'sata', 'scsi', 'ide') and
these commands generally do the same thing. This makes it harder to
maintain this code and keep it consistent.
We now have a block device interface which is either implemented by driver
model (when CONFIG_BLK is enabled) or with a legacy interface. Therefore
it is possible to handle most of what these commands do with generic code.
Add a new generic function to process block-device commands using the
interface type and the current device number for that type.
Signed-off-by: Simon Glass
---
Changes in v2: None
cmd/Makefile | 1 +
cmd/blk_common.c | 104 +++
include/blk.h| 12 +++
3 files changed, 117 insertions(+)
create mode 100644 cmd/blk_common.c
diff --git a/cmd/Makefile b/cmd/Makefile
index bd231f24d8..e2b3a9cc71 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -15,6 +15,7 @@ obj-y += version.o
# command
obj-$(CONFIG_CMD_AES) += aes.o
obj-$(CONFIG_CMD_ARMFLASH) += armflash.o
+obj-y += blk_common.o
obj-$(CONFIG_SOURCE) += source.o
obj-$(CONFIG_CMD_SOURCE) += source.o
obj-$(CONFIG_CMD_BDI) += bdinfo.o
diff --git a/cmd/blk_common.c b/cmd/blk_common.c
new file mode 100644
index 00..86c75e78d8
--- /dev/null
+++ b/cmd/blk_common.c
@@ -0,0 +1,104 @@
+/*
+ * Handling of common block commands
+ *
+ * Copyright (c) 2017 Google, Inc
+ *
+ * (C) Copyright 2000-2011
+ * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+ *
+ * SPDX-License-Identifier:GPL-2.0+
+ */
+
+#include
+#include
+
+#ifdef HAVE_BLOCK_DEVICE
+int blk_common_cmd(int argc, char * const argv[], enum if_type if_type,
+ int *cur_devnump)
+{
+ const char *if_name = blk_get_if_type_name(if_type);
+
+ switch (argc) {
+ case 0:
+ case 1:
+ return CMD_RET_USAGE;
+ case 2:
+ if (strncmp(argv[1], "inf", 3) == 0) {
+ blk_list_devices(if_type);
+ return 0;
+ } else if (strncmp(argv[1], "dev", 3) == 0) {
+ if (blk_print_device_num(if_type, *cur_devnump)) {
+ printf("\nno %s devices available\n", if_name);
+ return CMD_RET_FAILURE;
+ }
+ return 0;
+ } else if (strncmp(argv[1], "part", 4) == 0) {
+ if (blk_list_part(if_type))
+ printf("\nno %s devices available\n", if_name);
+ return 0;
+ }
+ return CMD_RET_USAGE;
+ case 3:
+ if (strncmp(argv[1], "dev", 3) == 0) {
+ int dev = (int)simple_strtoul(argv[2], NULL, 10);
+
+ if (!blk_show_device(if_type, dev)) {
+ *cur_devnump = dev;
+ printf("... is now current device\n");
+ } else {
+ return CMD_RET_FAILURE;
+ }
+ return 0;
+ } else if (strncmp(argv[1], "part", 4) == 0) {
+ int dev = (int)simple_strtoul(argv[2], NULL, 10);
+
+ if (blk_print_part_devnum(if_type, dev)) {
+ printf("\n%s device %d not available\n",
+ if_name, dev);
+ return CMD_RET_FAILURE;
+ }
+ return 0;
+ }
+ return CMD_RET_USAGE;
+
+ default: /* at least 4 args */
+ if (strcmp(argv[1], "read") == 0) {
+ ulong addr = simple_strtoul(argv[2], NULL, 16);
+ lbaint_t blk = simple_strtoul(argv[3], NULL, 16);
+ ulong cnt = simple_strtoul(argv[4], NULL, 16);
+ ulong n;
+
+ printf("\n%s read: device %d block # %lld, count %ld
... ",
+ if_name, *cur_devnump, (unsigned long long)blk,
+ cnt);
+
+ n = blk_read_devnum(if_type, *cur_devnump, blk, cnt,
+ (ulong *)addr);
+
+ printf("%ld blocks read: %s\n", n,
+ n == cnt ? "OK" : "ERROR");
+ return n == cnt ? 0 : 1;
+ } else if (strcmp(argv[1], "write") == 0) {
+ ulong addr = simple_strtoul(argv[2], NULL, 16);
+ lbaint_t blk = simple_strtoul(argv[3], NULL, 16);
+ ulong cnt = simple_strtoul(argv[4], NULL, 16);
+ ulong n;
+
+ printf("\n%s write: device %d block # %lld, count %ld
... ",
+ if_name, *cur_devnump, (unsigned long long)blk,
+ cnt);
+
