[PATCH]: cmd: part: add part block command

The Intel Edison OTA process requires a conversion of data size from bytes to number of blocks. The following functions are used:
# function ota_conv_sizes # Convert a bytes size to a block size # input bytesize : size in bytes to convert # input blksize : size of a block in bytes # output num_blk : converted size in blocks
# function ota_mmc_write # Write a memory buffer to mmc drive # input floadaddr : address of buffer to write # input u_part_start : block start in mmc # input num_blk : number of block to write
This patch adds the cmd part sub-command 'block' which returns the partition block size in bytes.

From: Razvan Becheriu razvan.becheriu@gmail.com
Add part block sub-command which returns block size.
e.g.: part block mmc $mmcdev system_a system_a_index
Signed-off-by: Razvan Becheriu razvan.becheriu@gmail.com --- cmd/part.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/cmd/part.c b/cmd/part.c index 5e4e45ca6d..d78d914e7a 100644 --- a/cmd/part.c +++ b/cmd/part.c @@ -25,6 +25,7 @@ enum cmd_part_info { CMD_PART_INFO_START = 0, CMD_PART_INFO_SIZE, + CMD_PART_INFO_BLOCK, CMD_PART_INFO_NUMBER };
@@ -151,6 +152,9 @@ static int do_part_info(int argc, char * const argv[], enum cmd_part_info param) case CMD_PART_INFO_SIZE: snprintf(buf, sizeof(buf), LBAF, info.size); break; + case CMD_PART_INFO_BLOCK: + snprintf(buf, sizeof(buf), LBAF, info.blksz); + break; case CMD_PART_INFO_NUMBER: snprintf(buf, sizeof(buf), "0x%x", part); break; @@ -177,6 +181,11 @@ static int do_part_size(int argc, char * const argv[]) return do_part_info(argc, argv, CMD_PART_INFO_SIZE); }
+static int do_part_block(int argc, char * const argv[]) +{ + return do_part_info(argc, argv, CMD_PART_INFO_BLOCK); +} + static int do_part_number(int argc, char * const argv[]) { return do_part_info(argc, argv, CMD_PART_INFO_NUMBER); @@ -195,6 +204,8 @@ static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return do_part_start(argc - 2, argv + 2); else if (!strcmp(argv[1], "size")) return do_part_size(argc - 2, argv + 2); + else if (!strcmp(argv[1], "block")) + return do_part_block(argc - 2, argv + 2); else if (!strcmp(argv[1], "number")) return do_part_number(argc - 2, argv + 2);
@@ -219,6 +230,9 @@ U_BOOT_CMD( "part size <interface> <dev> <part> <varname>\n" " - set environment variable to the size of the partition (in blocks)\n" " part can be either partition number or partition name\n" + "part block <interface> <dev> <part> <varname>\n" + " - set environment variable to the size of the partition block\n" + " part can be either partition number or partition name\n" "part number <interface> <dev> <part> <varname>\n" " - set environment variable to the partition number using the partition name\n" " part must be specified as partition name"

On Mon, Jun 01, 2020 at 01:20:26PM +0300, razvan.becheriu@gmail.com wrote:
From: Razvan Becheriu razvan.becheriu@gmail.com
Add part block sub-command which returns block size. e.g.: part block mmc $mmcdev system_a system_a_index
Signed-off-by: Razvan Becheriu razvan.becheriu@gmail.com
cmd/part.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/cmd/part.c b/cmd/part.c index 5e4e45ca6d..d78d914e7a 100644 --- a/cmd/part.c +++ b/cmd/part.c @@ -25,6 +25,7 @@ enum cmd_part_info { CMD_PART_INFO_START = 0, CMD_PART_INFO_SIZE,
- CMD_PART_INFO_BLOCK, CMD_PART_INFO_NUMBER
};
@@ -151,6 +152,9 @@ static int do_part_info(int argc, char * const argv[], enum cmd_part_info param) case CMD_PART_INFO_SIZE: snprintf(buf, sizeof(buf), LBAF, info.size); break;
- case CMD_PART_INFO_BLOCK:
snprintf(buf, sizeof(buf), LBAF, info.blksz);
case CMD_PART_INFO_NUMBER: snprintf(buf, sizeof(buf), "0x%x", part); break;break;
@@ -177,6 +181,11 @@ static int do_part_size(int argc, char * const argv[]) return do_part_info(argc, argv, CMD_PART_INFO_SIZE); }
+static int do_part_block(int argc, char * const argv[]) +{
- return do_part_info(argc, argv, CMD_PART_INFO_BLOCK);
+}
static int do_part_number(int argc, char * const argv[]) { return do_part_info(argc, argv, CMD_PART_INFO_NUMBER); @@ -195,6 +204,8 @@ static int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return do_part_start(argc - 2, argv + 2); else if (!strcmp(argv[1], "size")) return do_part_size(argc - 2, argv + 2);
- else if (!strcmp(argv[1], "block"))
else if (!strcmp(argv[1], "number")) return do_part_number(argc - 2, argv + 2);return do_part_block(argc - 2, argv + 2);
@@ -219,6 +230,9 @@ U_BOOT_CMD( "part size <interface> <dev> <part> <varname>\n" " - set environment variable to the size of the partition (in blocks)\n" " part can be either partition number or partition name\n"
- "part block <interface> <dev> <part> <varname>\n"
- " - set environment variable to the size of the partition block\n"
- " part can be either partition number or partition name\n" "part number <interface> <dev> <part> <varname>\n" " - set environment variable to the partition number using the partition name\n" " part must be specified as partition name"
I believe you can solve the problems mentioned in the cover letter with setexpr to do the conversion so nak on this patch, thanks.

On Mon, Jun 01, 2020 at 01:20:25PM +0300, razvan.becheriu@gmail.com wrote:
The Intel Edison OTA process requires a conversion of data size from bytes to number of blocks. The following functions are used: # function ota_conv_sizes # Convert a bytes size to a block size # input bytesize : size in bytes to convert # input blksize : size of a block in bytes # output num_blk : converted size in blocks # function ota_mmc_write # Write a memory buffer to mmc drive # input floadaddr : address of buffer to write # input u_part_start : block start in mmc # input num_blk : number of block to write This patch adds the cmd part sub-command 'block' which returns the partition block size in bytes.
This is usually done with the setexpr command today, thanks!

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
setexpr can compute the divide/multiply part, but we still need to get the partition block size somehow.
I know that this is 0x200 by default, but we can not hardcode that in the scripts. we should read that from the partition info.
On 2020-06-02 at 17:55, trini@konsulko.com wrote:
On Mon, Jun 01, 2020 at 01:20:25PM +0300, razvan.becheriu@gmail.com
wrote:
The Intel Edison OTA process requires a conversion of data size from bytes to number of blocks. The following functions are used: # function ota_conv_sizes # Convert a bytes size to a block size # input bytesize : size in bytes to convert # input blksize : size of a block in bytes # output num_blk : converted size in blocks # function ota_mmc_write # Write a memory buffer to mmc drive # input floadaddr : address of buffer to write # input u_part_start : block start in mmc # input num_blk : number of block to write This patch adds the cmd part sub-command 'block' which returns the partition block size in bytes.
This is usually done with the setexpr command today, thanks!
-- Tom

On Tue, Jun 02, 2020 at 11:08:19AM -0700, razvan becheriu wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
setexpr can compute the divide/multiply part, but we still need to get the partition block size somehow.
I know that this is 0x200 by default, but we can not hardcode that in the scripts. we should read that from the partition info.
So there's plenty of examples today of hard-coding that value. Are you worried about a potential future change or an exists today problem you can't otherwise determine the hardware revision of at run-time? Thanks!

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
here is the implementation of the functions:
# function ota_conv_sizes # Convert a bytes size to a block size # input bytesize : size in bytes to convert # input blksize : size of a block in bytes # output num_blk : converted size in blocks setenv ota_conv_sizes 'setexpr num_blk $bytesize / $blksize ; setexpr mod_blk $bytesize % $blksize ; if itest $mod_blk > 0 ; then setexpr num_blk $num_blk + 1; fi;'
# function ota_mmc_write # Write a memory buffer to mmc drive # input floadaddr : address of buffer to write # input u_part_start : block start in mmc # input num_blk : number of block to write setenv ota_mmc_write 'if itest $ota_verbose == 1 ; then echo "mmc write ${floadaddr} ${u_part_start} ${num_blk};"; fi; mmc write $floadaddr $u_part_start $num_blk; ret=$?; if itest $ret != 0 ; then setenv ota_abort_reason "mmc write ${floadaddr} ${u_part_start} ${num_blk} failed"; setenv ota_abort 1; fi;'
the old u-boot version supported 'part info mmc 0:${u_part_num} u_part_start u_part_sz u_part_blksz;' to get the block size.
# function ota_get_partition_attributes # Retrieve partition attribute # input u_part_num : partition number # output u_part_start : partition start block number # output u_part_sz : partition size in blocks # output u_part_blksz : partition block size in bytes setenv ota_get_partition_attributes 'if itest $ota_verbose == 1 ; then echo "part info mmc 0:${u_part_num} u_part_start u_part_sz u_part_blksz;"; fi; part info mmc 0:${u_part_num} u_part_start u_part_sz u_part_blksz;ret=$?; if itest $ret != 0 ; then setenv ota_abort_reason "part info mmc 0:${u_part_num} u_part_start u_part_sz u_part_blksz failed: ${ret}"; setenv ota_abort 1; fi;'
the new u-boot does not, so we need an explicit command 'part block mmc 0 ${u_part_lbl} u_part_blksz;'
# function ota_get_partition_attributes_alternative # Retrieve partition attribute # input u_part_lbl : partition label # output u_part_start : partition start block number # output u_part_sz : partition size in blocks # output u_part_blksz : partition block size in bytes setenv ota_get_partition_attributes_alternative 'if itest $ota_verbose == 1 ; then echo "part start mmc 0 ${u_part_lbl} u_part_start; part size mmc 0 ${u_part_lbl} u_part_sz; part block mmc 0 ${u_part_lbl} u_part_blksz;"; fi; part start mmc 0 ${u_part_lbl} u_part_start;ret=$?; if itest $ret != 0 ; then setenv ota_abort_reason "part start mmc 0 ${u_part_lbl} u_part_start failed: ${ret}"; setenv ota_abort 1; fi; part size mmc 0 ${u_part_lbl} u_part_sz;ret=$?; if itest $ret != 0 ; then setenv ota_abort_reason "part size mmc 0 ${u_part_lbl} u_part_sz failed: ${ret}"; setenv ota_abort 1; fi; part block mmc 0 ${u_part_lbl} u_part_blksz;ret=$?; if itest $ret != 0 ; then setenv ota_abort_reason "part block mmc 0 ${u_part_lbl} u_part_blksz failed: ${ret}"; setenv ota_abort 1; fi;'
On 2020-06-02 at 18:08, razvan.becheriu@gmail.com wrote:
setexpr can compute the divide/multiply part, but we still need to get
the
partition block size somehow.
I know that this is 0x200 by default, but we can not hardcode that in the scripts. we should read that from the partition info.
On 2020-06-02 at 17:55, trini@konsulko.com wrote:
On Mon, Jun 01, 2020 at 01:20:25PM +0300, razvan.becheriu@gmail.com
wrote:
The Intel Edison OTA process requires a conversion of data size from bytes to number of blocks. The following functions are used: # function ota_conv_sizes # Convert a bytes size to a block size # input bytesize : size in bytes to convert # input blksize : size of a block in bytes # output num_blk : converted size in blocks # function ota_mmc_write # Write a memory buffer to mmc drive # input floadaddr : address of buffer to write # input u_part_start : block start in mmc # input num_blk : number of block to write This patch adds the cmd part sub-command 'block' which returns the partition block size in bytes.
This is usually done with the setexpr command today, thanks!
-- Tom

On Tue, Jun 02, 2020 at 11:36:48AM -0700, razvan becheriu wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
here is the implementation of the functions:
# function ota_conv_sizes # Convert a bytes size to a block size # input bytesize : size in bytes to convert # input blksize : size of a block in bytes # output num_blk : converted size in blocks setenv ota_conv_sizes 'setexpr num_blk $bytesize / $blksize ; setexpr mod_blk $bytesize % $blksize ; if itest $mod_blk > 0 ; then setexpr num_blk $num_blk + 1; fi;'
# function ota_mmc_write # Write a memory buffer to mmc drive # input floadaddr : address of buffer to write # input u_part_start : block start in mmc # input num_blk : number of block to write setenv ota_mmc_write 'if itest $ota_verbose == 1 ; then echo "mmc write ${floadaddr} ${u_part_start} ${num_blk};"; fi; mmc write $floadaddr $u_part_start $num_blk; ret=$?; if itest $ret != 0 ; then setenv ota_abort_reason "mmc write ${floadaddr} ${u_part_start} ${num_blk} failed"; setenv ota_abort 1; fi;'
the old u-boot version supported 'part info mmc 0:${u_part_num} u_part_start u_part_sz u_part_blksz;' to get the block size.
Old upstream U-Boot?

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
hm...
It seems that the changes never reached upstream:
git://github.com/01org/edison-u-boot.git branch edison-v2014.04
this branch was implementing 'part info' sub-command which was useful to retrieve partition info.
the new repo:
git://github.com/edison-fw/u-boot.git branch acpi-v2020.04 does implement 'part number' but does not implement neither 'info' or 'block'.
I do not insist for you to take this patch, but I think it is useful (it is the simplest version which supports all needed functionality).
If you think the same, let me know. I can change name of the sub-command or implement this in a different way, if needed.
Thank you, Razvan
On 2020-06-02 at 18:51, trini@konsulko.com wrote:
On Tue, Jun 02, 2020 at 11:36:48AM -0700, razvan becheriu wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
here is the implementation of the functions:
# function ota_conv_sizes # Convert a bytes size to a block size # input bytesize : size in bytes to convert # input blksize : size of a block in bytes # output num_blk : converted size in blocks setenv ota_conv_sizes 'setexpr num_blk $bytesize / $blksize ; setexpr mod_blk $bytesize % $blksize ; if itest $mod_blk > 0 ; then setexpr
num_blk
$num_blk + 1; fi;'
# function ota_mmc_write # Write a memory buffer to mmc drive # input floadaddr : address of buffer to write # input u_part_start : block start in mmc # input num_blk : number of block to write setenv ota_mmc_write 'if itest $ota_verbose == 1 ; then echo "mmc write ${floadaddr} ${u_part_start} ${num_blk};"; fi; mmc write $floadaddr $u_part_start $num_blk; ret=$?; if itest $ret != 0 ; then setenv ota_abort_reason "mmc write ${floadaddr} ${u_part_start} ${num_blk} failed"; setenv ota_abort 1; fi;'
the old u-boot version supported 'part info mmc 0:${u_part_num} u_part_start u_part_sz u_part_blksz;' to get the block size.
Old upstream U-Boot?
-- Tom

On Tue, Jun 02, 2020 at 12:23:52PM -0700, razvan becheriu wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
hm...
It seems that the changes never reached upstream:
git://github.com/01org/edison-u-boot.git branch edison-v2014.04
this branch was implementing 'part info' sub-command which was useful to retrieve partition info.
Ah, I thought it might be something like this, thanks for digging in to it.
the new repo:
git://github.com/edison-fw/u-boot.git branch acpi-v2020.04 does implement 'part number' but does not implement neither 'info' or 'block'.
I do not insist for you to take this patch, but I think it is useful (it is the simplest version which supports all needed functionality).
If you think the same, let me know. I can change name of the sub-command or implement this in a different way, if needed.
I think for now I just want to defer on this. We just aren't at the point where we can't solve this by knowing ahead of time what the blocksize is. I'm sure it will be a problem at some point, but the other part of the problem is figuring out how to add this functionality in an opt-in way that still looks clean when written. If you have some ideas on that however, I'd be happy to review. Thanks!
participants (3)
-
razvan becheriu
-
razvan.becheriu@gmail.com
-
Tom Rini