diff mbox

[U-Boot,v2,03/40] dm: blk: Add a generic function for block device commands

Message ID 20170729173531.90977-4-sjg@chromium.org
State Accepted
Delegated to: Jaehoon Chung
Headers show

Commit Message

Simon Glass July 29, 2017, 5:34 p.m. UTC 
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 <sjg@chromium.org>
---

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

Comments

Bin Meng Aug. 1, 2017, 4:56 a.m. UTC | #1 
Hi Simon,

On Sun, Jul 30, 2017 at 1:34 AM, Simon Glass <sjg@chromium.org> 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 <sjg@chromium.org>
> ---
>
> 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 0000000000..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, wd@denx.de.
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <blk.h>
> +
> +#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, 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);

ditto.

> +
> +                       n = blk_write_devnum(if_type, *cur_devnump, blk, cnt,
> +                                            (ulong *)addr);
> +
> +                       printf("%ld blocks written: %s\n", n,
> +                              n == cnt ? "OK" : "ERROR");
> +                       return n == cnt ? 0 : 1;
> +               } else {
> +                       return CMD_RET_USAGE;
> +               }
> +
> +               return 0;
> +       }
> +}
> +#endif
> diff --git a/include/blk.h b/include/blk.h
> index 69076d3683..5e29c6a21f 100644
> --- a/include/blk.h
> +++ b/include/blk.h
> @@ -631,4 +631,16 @@ int blk_select_hwpart_devnum(enum if_type if_type, int devnum, int hwpart);
>   */
>  const char *blk_get_if_type_name(enum if_type if_type);
>
> +/**
> + * blk_common_cmd() - handle common commands with block devices
> + *
> + * @args: Number of arguments to the command (argv[0] is the command itself)
> + * @argv: Command arguments
> + * @if_type: Interface type
> + * @cur_devnump: Current device number for this interface type
> + * @return 0 if OK, CMD_RET_ERROR on error
> + */
> +int blk_common_cmd(int argc, char * const argv[], enum if_type if_type,
> +                  int *cur_devnump);
> +
>  #endif
> --

Regards,
Bin
Simon Glass Aug. 31, 2017, 12:51 p.m. UTC | #2 
Hi Bin,

On 1 August 2017 at 12:56, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Sun, Jul 30, 2017 at 1:34 AM, Simon Glass <sjg@chromium.org> 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 <sjg@chromium.org>
>> ---
>>
>> 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 0000000000..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, wd@denx.de.
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <blk.h>
>> +
>> +#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
Bin Meng Aug. 31, 2017, 12:53 p.m. UTC | #3 
Hi Simon,

On Thu, Aug 31, 2017 at 8:51 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 1 August 2017 at 12:56, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Simon,
>>
>> On Sun, Jul 30, 2017 at 1:34 AM, Simon Glass <sjg@chromium.org> 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 <sjg@chromium.org>
>>> ---
>>>
>>> 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 0000000000..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, wd@denx.de.
>>> + *
>>> + * SPDX-License-Identifier:    GPL-2.0+
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <blk.h>
>>> +
>>> +#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://patchwork.ozlabs.org/patch/804298/, but not applied yet.

Regards,
Bin
diff mbox

Patch

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 0000000000..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, wd@denx.de.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <blk.h>
+
+#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);
+
+			n = blk_write_devnum(if_type, *cur_devnump, blk, cnt,
+					     (ulong *)addr);
+
+			printf("%ld blocks written: %s\n", n,
+			       n == cnt ? "OK" : "ERROR");
+			return n == cnt ? 0 : 1;
+		} else {
+			return CMD_RET_USAGE;
+		}
+
+		return 0;
+	}
+}
+#endif
diff --git a/include/blk.h b/include/blk.h
index 69076d3683..5e29c6a21f 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -631,4 +631,16 @@  int blk_select_hwpart_devnum(enum if_type if_type, int devnum, int hwpart);
  */
 const char *blk_get_if_type_name(enum if_type if_type);
 
+/**
+ * blk_common_cmd() - handle common commands with block devices
+ *
+ * @args: Number of arguments to the command (argv[0] is the command itself)
+ * @argv: Command arguments
+ * @if_type: Interface type
+ * @cur_devnump: Current device number for this interface type
+ * @return 0 if OK, CMD_RET_ERROR on error
+ */
+int blk_common_cmd(int argc, char * const argv[], enum if_type if_type,
+		   int *cur_devnump);
+
 #endif