[PATCH v2 0/1] Add more support for NXP's mfgtool
NXP's mfgtool needs to query the block size for sparse image upload
Changes since v1:
* Dropped the "all" parition type patch * Added more detail to the patch commit message * Only define getvar_logical_blocksize when FASTBOOT_FLASH_MMC is defined * Combined fb_get_block_size into getvar_logical_blocksize * Fixed fastboot returned error message
Angus Ainslie (1): fastboot: fb_getvar: Add getvar_logical_blocksize for NXP mfgtool
drivers/fastboot/fb_getvar.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
NXP's mfgtool queies the mmc blocksize and splits a sparse image into blocksize size pieces for the upload.
Signed-off-by: Angus Ainslie angus@akkea.ca --- drivers/fastboot/fb_getvar.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c index d43f2cfee6..3cb72b8a54 100644 --- a/drivers/fastboot/fb_getvar.c +++ b/drivers/fastboot/fb_getvar.c @@ -26,6 +26,7 @@ static void getvar_has_slot(char *var_parameter, char *response); #endif #if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC) static void getvar_partition_type(char *part_name, char *response); +static void getvar_logical_blocksize(char *var_parameter, char *response); #endif #if CONFIG_IS_ENABLED(FASTBOOT_FLASH) static void getvar_partition_size(char *part_name, char *response); @@ -72,6 +73,9 @@ static const struct { }, { .variable = "partition-type", .dispatch = getvar_partition_type + }, { + .variable = "logical-block-size", + .dispatch = getvar_logical_blocksize #endif #if CONFIG_IS_ENABLED(FASTBOOT_FLASH) }, { @@ -232,6 +236,23 @@ static void getvar_partition_type(char *part_name, char *response) fastboot_okay(fs_get_type_name(), response); } } + +static void getvar_logical_blocksize(char *var_parameter, char *response) +{ + u32 blksz; + struct blk_desc *dev_desc; + + dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV); + + if (!dev_desc) { + pr_err("** Block device mmc %d not supported\n", + CONFIG_FASTBOOT_FLASH_MMC_DEV); + fastboot_fail("Get logical_blocksize failed", response); + return; + } + + fastboot_response("OKAY", response, "0x%08x", dev_desc->blksz); +} #endif
#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
Hi Angus,
NXP's mfgtool queies the mmc blocksize and splits a sparse image into blocksize size pieces for the upload.
It's still not clear to me why this is necessary. fastboot (for example) transfers in blocks of max-download-size. Using the actual block size seems like it would result in unnecessary overhead.
--Sean
Signed-off-by: Angus Ainslie angus@akkea.ca
drivers/fastboot/fb_getvar.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c index d43f2cfee6..3cb72b8a54 100644 --- a/drivers/fastboot/fb_getvar.c +++ b/drivers/fastboot/fb_getvar.c @@ -26,6 +26,7 @@ static void getvar_has_slot(char *var_parameter, char *response); #endif #if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC) static void getvar_partition_type(char *part_name, char *response); +static void getvar_logical_blocksize(char *var_parameter, char *response); #endif #if CONFIG_IS_ENABLED(FASTBOOT_FLASH) static void getvar_partition_size(char *part_name, char *response); @@ -72,6 +73,9 @@ static const struct { }, { .variable = "partition-type", .dispatch = getvar_partition_type
- }, {
.variable = "logical-block-size",.dispatch = getvar_logical_blocksize@@ -232,6 +236,23 @@ static void getvar_partition_type(char *part_name, char *response) fastboot_okay(fs_get_type_name(), response); } }
+static void getvar_logical_blocksize(char *var_parameter, char *response) +{
- u32 blksz;
- struct blk_desc *dev_desc;
- dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
- if (!dev_desc) {
pr_err("** Block device mmc %d not supported\n",CONFIG_FASTBOOT_FLASH_MMC_DEV);fastboot_fail("Get logical_blocksize failed", response);return;- }
- fastboot_response("OKAY", response, "0x%08x", dev_desc->blksz);
+} #endif
#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
Hi Sean,
On 2021-12-28 08:59, Sean Anderson wrote:
Hi Angus,
NXP's mfgtool queies the mmc blocksize and splits a sparse image into blocksize size pieces for the upload.
It's still not clear to me why this is necessary. fastboot (for example) transfers in blocks of max-download-size. Using the actual block size seems like it would result in unnecessary overhead.
The version of uuu that we are using requires the block-size for the sparse upload
https://source.puri.sm/Librem5/mfgtools/-/blob/pureos/amber/libuuu/fastboot....
It looks like the upstream version will default to 4096 if the block-size is not provided
https://github.com/NXPmicro/mfgtools/blob/5397913ad97db422c1d70f314dedff4cb7...
Instead of making assumptions about the block size wouldn't it be better to provide one if requested ?
We could also remove that being a required parameter from the uuu that we are using but that still leaves versions out there that won't work with a mainline u-boot.
Thanks Angus
--Sean
Signed-off-by: Angus Ainslie angus@akkea.ca
drivers/fastboot/fb_getvar.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c index d43f2cfee6..3cb72b8a54 100644 --- a/drivers/fastboot/fb_getvar.c +++ b/drivers/fastboot/fb_getvar.c @@ -26,6 +26,7 @@ static void getvar_has_slot(char *var_parameter, char *response); #endif #if CONFIG_IS_ENABLED(FASTBOOT_FLASH_MMC) static void getvar_partition_type(char *part_name, char *response); +static void getvar_logical_blocksize(char *var_parameter, char *response); #endif #if CONFIG_IS_ENABLED(FASTBOOT_FLASH) static void getvar_partition_size(char *part_name, char *response); @@ -72,6 +73,9 @@ static const struct { }, { .variable = "partition-type", .dispatch = getvar_partition_type
- }, {
.variable = "logical-block-size",.dispatch = getvar_logical_blocksize@@ -232,6 +236,23 @@ static void getvar_partition_type(char *part_name, char *response) fastboot_okay(fs_get_type_name(), response); } }
+static void getvar_logical_blocksize(char *var_parameter, char *response) +{
- u32 blksz;
- struct blk_desc *dev_desc;
- dev_desc = blk_get_dev("mmc", CONFIG_FASTBOOT_FLASH_MMC_DEV);
- if (!dev_desc) {
pr_err("** Block device mmc %d not supported\n",CONFIG_FASTBOOT_FLASH_MMC_DEV);fastboot_fail("Get logical_blocksize failed", response);return;- }
- fastboot_response("OKAY", response, "0x%08x", dev_desc->blksz);
+} #endif
#if CONFIG_IS_ENABLED(FASTBOOT_FLASH)
Hello Angus,
29.12.21 15:35, Angus Ainslie пише:
The version of uuu that we are using requires the block-size for the sparse upload
https://source.puri.sm/Librem5/mfgtools/-/blob/pureos/amber/libuuu/fastboot....
This version is outdated (2 years old). Maybe it is time to upgrade to the latest one?
It looks like the upstream version will default to 4096 if the block-size is not provided
https://github.com/NXPmicro/mfgtools/blob/5397913ad97db422c1d70f314dedff4cb7...
Instead of making assumptions about the block size wouldn't it be better to provide one if requested ?
+1 if it works faster ^_^
We could also remove that being a required parameter from the uuu that we are using but that still leaves versions out there that won't work with a mainline u-boot.
Why does mainline use fixed block size 4096?
Excuse me for direct questions I'm just curious.
Hi Oleh,
On 2021-12-29 07:07, Oleh Kravchenko wrote:
Hello Angus,
29.12.21 15:35, Angus Ainslie пише:
The version of uuu that we are using requires the block-size for the sparse upload
https://source.puri.sm/Librem5/mfgtools/-/blob/pureos/amber/libuuu/fastboot....
This version is outdated (2 years old). Maybe it is time to upgrade to the latest one?
That was one of my suggested solutions but it would still leave older uuu implementations out there that won't work with mainline u-boot.
It looks like the upstream version will default to 4096 if the block-size is not provided
https://github.com/NXPmicro/mfgtools/blob/5397913ad97db422c1d70f314dedff4cb7...
Instead of making assumptions about the block size wouldn't it be better to provide one if requested ?
+1 if it works faster ^_^
I haven't done any speed tests so I can't really comment about the speed.
We could also remove that being a required parameter from the uuu that we are using but that still leaves versions out there that won't work with a mainline u-boot.
Why does mainline use fixed block size 4096?
That's a fall back if the fastboot implementation doesn't provide a block-size , probably so that it can work with mainline u-boot.
Excuse me for direct questions I'm just curious.
NP that's what review is for.
Angus
On 12/29/21 8:35 AM, Angus Ainslie wrote:
Hi Sean,
On 2021-12-28 08:59, Sean Anderson wrote:
Hi Angus,
NXP's mfgtool queies the mmc blocksize and splits a sparse image into blocksize size pieces for the upload.
It's still not clear to me why this is necessary. fastboot (for example) transfers in blocks of max-download-size. Using the actual block size seems like it would result in unnecessary overhead.
The version of uuu that we are using requires the block-size for the sparse upload
https://source.puri.sm/Librem5/mfgtools/-/blob/pureos/amber/libuuu/fastboot....
It looks like the upstream version will default to 4096 if the block-size is not provided
https://github.com/NXPmicro/mfgtools/blob/5397913ad97db422c1d70f314dedff4cb7...
Instead of making assumptions about the block size wouldn't it be better to provide one if requested ?
The block size is for the sparse image. This determines the granularity of the sections of the image. For example, if the block size is 1K, then all sizes will be multiples of 1K. So if you have a block with 1 byte of data and 1023 bytes of 0 then the whole block will be transferred instead of being replaced with a fill block.
However, the sparse file block size size completely orthogonal to the block size of the device being written to. The only thing it affects is the efficiency of the sparse image. For example, I generate my sparse files with a block size of 1M, because it is a nice convenient number.
--Sean
Hi Sean,
On 2021-12-30 08:21, Sean Anderson wrote:
On 12/29/21 8:35 AM, Angus Ainslie wrote:
Hi Sean,
On 2021-12-28 08:59, Sean Anderson wrote:
Hi Angus,
NXP's mfgtool queies the mmc blocksize and splits a sparse image into blocksize size pieces for the upload.
It's still not clear to me why this is necessary. fastboot (for example) transfers in blocks of max-download-size. Using the actual block size seems like it would result in unnecessary overhead.
The version of uuu that we are using requires the block-size for the sparse upload
https://source.puri.sm/Librem5/mfgtools/-/blob/pureos/amber/libuuu/fastboot....
It looks like the upstream version will default to 4096 if the block-size is not provided
https://github.com/NXPmicro/mfgtools/blob/5397913ad97db422c1d70f314dedff4cb7...
Instead of making assumptions about the block size wouldn't it be better to provide one if requested ?
The block size is for the sparse image. This determines the granularity of the sections of the image. For example, if the block size is 1K, then all sizes will be multiples of 1K. So if you have a block with 1 byte of data and 1023 bytes of 0 then the whole block will be transferred instead of being replaced with a fill block.
Thanks for the explanation on how it works.
However, the sparse file block size size completely orthogonal to the block size of the device being written to. The only thing it affects is the efficiency of the sparse image. For example, I generate my sparse files with a block size of 1M, because it is a nice convenient number.
Ok so the way the NXP's uuu uses the blocksize is not correct. However the tool is already out there in the wild.
Can we add the block-size response to support that tool or will it be required that uuu needs to be upgraded to work with mainline u-boot ?
Thanks Angus
--Sean
On 1/3/22 8:27 AM, Angus Ainslie wrote:
Hi Sean,
On 2021-12-30 08:21, Sean Anderson wrote:
On 12/29/21 8:35 AM, Angus Ainslie wrote:
Hi Sean,
On 2021-12-28 08:59, Sean Anderson wrote:
Hi Angus,
NXP's mfgtool queies the mmc blocksize and splits a sparse image into blocksize size pieces for the upload.
It's still not clear to me why this is necessary. fastboot (for example) transfers in blocks of max-download-size. Using the actual block size seems like it would result in unnecessary overhead.
The version of uuu that we are using requires the block-size for the sparse upload
https://source.puri.sm/Librem5/mfgtools/-/blob/pureos/amber/libuuu/fastboot....
It looks like the upstream version will default to 4096 if the block-size is not provided
https://github.com/NXPmicro/mfgtools/blob/5397913ad97db422c1d70f314dedff4cb7...
Instead of making assumptions about the block size wouldn't it be better to provide one if requested ?
The block size is for the sparse image. This determines the granularity of the sections of the image. For example, if the block size is 1K, then all sizes will be multiples of 1K. So if you have a block with 1 byte of data and 1023 bytes of 0 then the whole block will be transferred instead of being replaced with a fill block.
Thanks for the explanation on how it works.
However, the sparse file block size size completely orthogonal to the block size of the device being written to. The only thing it affects is the efficiency of the sparse image. For example, I generate my sparse files with a block size of 1M, because it is a nice convenient number.
Ok so the way the NXP's uuu uses the blocksize is not correct. However the tool is already out there in the wild.
Can we add the block-size response to support that tool or will it be required that uuu needs to be upgraded to work with mainline u-boot ?
IMO this should be fixed in uuu. And as I understand it, this is only an issue when you transfer a raw image with uuu. Perhaps you can generate a fastboot sparse image (with img2simg) and transfer that instead.
--Sean
participants (3)
-
Angus Ainslie -
Oleh Kravchenko -
Sean Anderson