* [PATCH v4] lib: sparse: Make CHUNK_TYPE_RAW buffer aligned
@ 2021-11-16 1:35 qianfanguijin
2021-11-16 15:05 ` Sean Anderson
2022-01-15 12:37 ` Tom Rini
0 siblings, 2 replies; 4+ messages in thread
From: qianfanguijin @ 2021-11-16 1:35 UTC (permalink / raw)
To: u-boot; +Cc: sean.anderson, hs, qianfan Zhao
From: qianfan Zhao <qianfanguijin@163.com>
CHUNK_TYPE_RAW buffer is not aligned, and flash sparse images by
fastboot will report "Misaligned operation" if DCACHE is enabled.
Flashing Sparse Image
CACHE: Misaligned operation at range [84000028, 84001028]
CACHE: Misaligned operation at range [84001034, 84002034]
CACHE: Misaligned operation at range [8401104c, 8401304c]
Fix it
Signed-off-by: qianfan Zhao <qianfanguijin@163.com>
---
lib/image-sparse.c | 69 ++++++++++++++++++++++++++++++++++++++++------
1 file changed, 61 insertions(+), 8 deletions(-)
diff --git a/lib/image-sparse.c b/lib/image-sparse.c
index d80fdbbf58..5ec0f94ab3 100644
--- a/lib/image-sparse.c
+++ b/lib/image-sparse.c
@@ -46,9 +46,66 @@
#include <asm/cache.h>
#include <linux/math64.h>
+#include <linux/err.h>
static void default_log(const char *ignored, char *response) {}
+static lbaint_t write_sparse_chunk_raw(struct sparse_storage *info,
+ lbaint_t blk, lbaint_t blkcnt,
+ void *data,
+ char *response)
+{
+ lbaint_t n = blkcnt, write_blks, blks = 0, aligned_buf_blks = 100;
+ uint32_t *aligned_buf = NULL;
+
+ if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) {
+ write_blks = info->write(info, blk, n, data);
+ if (write_blks < n)
+ goto write_fail;
+
+ return write_blks;
+ }
+
+ aligned_buf = memalign(ARCH_DMA_MINALIGN, info->blksz * aligned_buf_blks);
+ if (!aligned_buf) {
+ info->mssg("Malloc failed for: CHUNK_TYPE_RAW", response);
+ return -ENOMEM;
+ }
+
+ while (blkcnt > 0) {
+ n = min(aligned_buf_blks, blkcnt);
+ memcpy(aligned_buf, data, n * info->blksz);
+
+ /* write_blks might be > n due to NAND bad-blocks */
+ write_blks = info->write(info, blk + blks, n, aligned_buf);
+ if (write_blks < n) {
+ free(aligned_buf);
+ goto write_fail;
+ }
+
+ blks += write_blks;
+ data += n * info->blksz;
+ blkcnt -= n;
+ }
+
+ free(aligned_buf);
+ return blks;
+
+write_fail:
+ if (IS_ERR_VALUE(write_blks)) {
+ printf("%s: Write failed, block #" LBAFU " [" LBAFU "] (%lld)\n",
+ __func__, blk + blks, n, (long long)write_blks);
+ info->mssg("flash write failure", response);
+ return write_blks;
+ }
+
+ /* write_blks < n */
+ printf("%s: Write failed, block #" LBAFU " [" LBAFU "]\n",
+ __func__, blk + blks, n);
+ info->mssg("flash write failure(incomplete)", response);
+ return -1;
+}
+
int write_sparse_image(struct sparse_storage *info,
const char *part_name, void *data, char *response)
{
@@ -152,15 +209,11 @@ int write_sparse_image(struct sparse_storage *info,
return -1;
}
- blks = info->write(info, blk, blkcnt, data);
- /* blks might be > blkcnt (eg. NAND bad-blocks) */
- if (blks < blkcnt) {
- printf("%s: %s" LBAFU " [" LBAFU "]\n",
- __func__, "Write failed, block #",
- blk, blks);
- info->mssg("flash write failure", response);
+ blks = write_sparse_chunk_raw(info, blk, blkcnt,
+ data, response);
+ if (blks < 0)
return -1;
- }
+
blk += blks;
bytes_written += ((u64)blkcnt) * info->blksz;
total_blocks += chunk_header->chunk_sz;
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v4] lib: sparse: Make CHUNK_TYPE_RAW buffer aligned
2021-11-16 1:35 [PATCH v4] lib: sparse: Make CHUNK_TYPE_RAW buffer aligned qianfanguijin
@ 2021-11-16 15:05 ` Sean Anderson
[not found] ` <tencent_2353D67A319ACDD88C7A564D884FA2FA4F05@qq.com>
2022-01-15 12:37 ` Tom Rini
1 sibling, 1 reply; 4+ messages in thread
From: Sean Anderson @ 2021-11-16 15:05 UTC (permalink / raw)
To: qianfanguijin, u-boot; +Cc: hs, qianfan Zhao
On 11/15/21 8:35 PM, qianfanguijin@qq.com wrote:
> From: qianfan Zhao <qianfanguijin@163.com>
>
> CHUNK_TYPE_RAW buffer is not aligned, and flash sparse images by
> fastboot will report "Misaligned operation" if DCACHE is enabled.
>
> Flashing Sparse Image
> CACHE: Misaligned operation at range [84000028, 84001028]
> CACHE: Misaligned operation at range [84001034, 84002034]
> CACHE: Misaligned operation at range [8401104c, 8401304c]
>
> Fix it
>
> Signed-off-by: qianfan Zhao <qianfanguijin@163.com>
> ---
> lib/image-sparse.c | 69 ++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 61 insertions(+), 8 deletions(-)
>
> diff --git a/lib/image-sparse.c b/lib/image-sparse.c
> index d80fdbbf58..5ec0f94ab3 100644
> --- a/lib/image-sparse.c
> +++ b/lib/image-sparse.c
> @@ -46,9 +46,66 @@
> #include <asm/cache.h>
>
> #include <linux/math64.h>
> +#include <linux/err.h>
>
> static void default_log(const char *ignored, char *response) {}
>
> +static lbaint_t write_sparse_chunk_raw(struct sparse_storage *info,
> + lbaint_t blk, lbaint_t blkcnt,
> + void *data,
> + char *response)
> +{
> + lbaint_t n = blkcnt, write_blks, blks = 0, aligned_buf_blks = 100;
> + uint32_t *aligned_buf = NULL;
> +
> + if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) {
> + write_blks = info->write(info, blk, n, data);
> + if (write_blks < n)
> + goto write_fail;
> +
> + return write_blks;
> + }
> +
> + aligned_buf = memalign(ARCH_DMA_MINALIGN, info->blksz * aligned_buf_blks);
> + if (!aligned_buf) {
> + info->mssg("Malloc failed for: CHUNK_TYPE_RAW", response);
> + return -ENOMEM;
> + }
> +
> + while (blkcnt > 0) {
> + n = min(aligned_buf_blks, blkcnt);
> + memcpy(aligned_buf, data, n * info->blksz);
> +
> + /* write_blks might be > n due to NAND bad-blocks */
nit: <
> + write_blks = info->write(info, blk + blks, n, aligned_buf);
> + if (write_blks < n) {
> + free(aligned_buf);
> + goto write_fail;
> + }
> +
> + blks += write_blks;
> + data += n * info->blksz;
> + blkcnt -= n;
> + }
> +
> + free(aligned_buf);
> + return blks;
> +
> +write_fail:
I think this label can be lower, but it does not affect correctness.
> + if (IS_ERR_VALUE(write_blks)) {
> + printf("%s: Write failed, block #" LBAFU " [" LBAFU "] (%lld)\n",
> + __func__, blk + blks, n, (long long)write_blks);
> + info->mssg("flash write failure", response);
> + return write_blks;
> + }
> +
> + /* write_blks < n */
> + printf("%s: Write failed, block #" LBAFU " [" LBAFU "]\n",
> + __func__, blk + blks, n);
> + info->mssg("flash write failure(incomplete)", response);
> + return -1;
-EIO?
> +}
> +
> int write_sparse_image(struct sparse_storage *info,
> const char *part_name, void *data, char *response)
> {
> @@ -152,15 +209,11 @@ int write_sparse_image(struct sparse_storage *info,
> return -1;
> }
>
> - blks = info->write(info, blk, blkcnt, data);
> - /* blks might be > blkcnt (eg. NAND bad-blocks) */
> - if (blks < blkcnt) {
> - printf("%s: %s" LBAFU " [" LBAFU "]\n",
> - __func__, "Write failed, block #",
> - blk, blks);
> - info->mssg("flash write failure", response);
> + blks = write_sparse_chunk_raw(info, blk, blkcnt,
> + data, response);
> + if (blks < 0)
> return -1;
ditto
> - }
> +
> blk += blks;
> bytes_written += ((u64)blkcnt) * info->blksz;
> total_blocks += chunk_header->chunk_sz;
>
Reviewed-by: Sean Anderson <sean.anderson@seco.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v4] lib: sparse: Make CHUNK_TYPE_RAW buffer aligned
[not found] ` <tencent_2353D67A319ACDD88C7A564D884FA2FA4F05@qq.com>
@ 2021-11-18 19:46 ` Sean Anderson
0 siblings, 0 replies; 4+ messages in thread
From: Sean Anderson @ 2021-11-18 19:46 UTC (permalink / raw)
To: qianfan; +Cc: u-boot Mailing List
On 11/17/21 3:41 AM, qianfan wrote:
>
> 在 2021/11/16 23:05, Sean Anderson 写道:
>>
>>
>> On 11/15/21 8:35 PM, qianfanguijin@qq.com wrote:
>>> From: qianfan Zhao <qianfanguijin@163.com>
>>>
>>> CHUNK_TYPE_RAW buffer is not aligned, and flash sparse images by
>>> fastboot will report "Misaligned operation" if DCACHE is enabled.
>>>
>>> Flashing Sparse Image
>>> CACHE: Misaligned operation at range [84000028, 84001028]
>>> CACHE: Misaligned operation at range [84001034, 84002034]
>>> CACHE: Misaligned operation at range [8401104c, 8401304c]
>>>
>>> Fix it
>>>
>>> Signed-off-by: qianfan Zhao <qianfanguijin@163.com>
>>> ---
>>> lib/image-sparse.c | 69 ++++++++++++++++++++++++++++++++++++++++------
>>> 1 file changed, 61 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/lib/image-sparse.c b/lib/image-sparse.c
>>> index d80fdbbf58..5ec0f94ab3 100644
>>> --- a/lib/image-sparse.c
>>> +++ b/lib/image-sparse.c
>>> @@ -46,9 +46,66 @@
>>> #include <asm/cache.h>
>>> #include <linux/math64.h>
>>> +#include <linux/err.h>
>>> static void default_log(const char *ignored, char *response) {}
>>> +static lbaint_t write_sparse_chunk_raw(struct sparse_storage *info,
>>> + lbaint_t blk, lbaint_t blkcnt,
>>> + void *data,
>>> + char *response)
>>> +{
>>> + lbaint_t n = blkcnt, write_blks, blks = 0, aligned_buf_blks = 100;
>>> + uint32_t *aligned_buf = NULL;
>>> +
>>> + if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) {
>>> + write_blks = info->write(info, blk, n, data);
>>> + if (write_blks < n)
>>> + goto write_fail;
>>> +
>>> + return write_blks;
>>> + }
>>> +
>>> + aligned_buf = memalign(ARCH_DMA_MINALIGN, info->blksz * aligned_buf_blks);
>>> + if (!aligned_buf) {
>>> + info->mssg("Malloc failed for: CHUNK_TYPE_RAW", response);
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + while (blkcnt > 0) {
>>> + n = min(aligned_buf_blks, blkcnt);
>>> + memcpy(aligned_buf, data, n * info->blksz);
>>> +
>>> + /* write_blks might be > n due to NAND bad-blocks */
>>
>> nit: <
>
> This comment is misleading here, but it is actually correct. It is correct to write more that the specified number when nand flash bad blocks occur, so (write_blks >= n) is write successful. maybe rewrite the judge conditions like this is better?
Ah, you are right.
> if (!(write_blks >= n))
>
>>
>>> + write_blks = info->write(info, blk + blks, n, aligned_buf);
>>> + if (write_blks < n) {
>>> + free(aligned_buf);
>>> + goto write_fail;
>>> + }
>>> +
>>> + blks += write_blks;
>>> + data += n * info->blksz;
>>> + blkcnt -= n;
>>> + }
>>> +
>>> + free(aligned_buf);
>>> + return blks;
>>> +
>>> +write_fail:
>>
>> I think this label can be lower, but it does not affect correctness.
> sorry, could you please explain this clear, I can't understand.
>>
>>> + if (IS_ERR_VALUE(write_blks)) {
>>> + printf("%s: Write failed, block #" LBAFU " [" LBAFU "] (%lld)\n",
>>> + __func__, blk + blks, n, (long long)write_blks);
>>> + info->mssg("flash write failure", response);
>>> + return write_blks;
>>> + }
The above label could be placed here, since you check the value of write_blks before jumping.
--Sean
>>> + /* write_blks < n */
>>> + printf("%s: Write failed, block #" LBAFU " [" LBAFU "]\n",
>>> + __func__, blk + blks, n);
>>> + info->mssg("flash write failure(incomplete)", response);
>>> + return -1;
>>
>> -EIO?
> It's OK
>>
>>> +}
>>> +
>>> int write_sparse_image(struct sparse_storage *info,
>>> const char *part_name, void *data, char *response)
>>> {
>>> @@ -152,15 +209,11 @@ int write_sparse_image(struct sparse_storage *info,
>>> return -1;
>>> }
>>> - blks = info->write(info, blk, blkcnt, data);
>>> - /* blks might be > blkcnt (eg. NAND bad-blocks) */
>>> - if (blks < blkcnt) {
>>> - printf("%s: %s" LBAFU " [" LBAFU "]\n",
>>> - __func__, "Write failed, block #",
>>> - blk, blks);
>>> - info->mssg("flash write failure", response);
>>> + blks = write_sparse_chunk_raw(info, blk, blkcnt,
>>> + data, response);
>>> + if (blks < 0)
>>> return -1;
>>
>> ditto
> It's OK
>>
>>> - }
>>> +
>>> blk += blks;
>>> bytes_written += ((u64)blkcnt) * info->blksz;
>>> total_blocks += chunk_header->chunk_sz;
>>>
>>
>> Reviewed-by: Sean Anderson <sean.anderson@seco.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v4] lib: sparse: Make CHUNK_TYPE_RAW buffer aligned
2021-11-16 1:35 [PATCH v4] lib: sparse: Make CHUNK_TYPE_RAW buffer aligned qianfanguijin
2021-11-16 15:05 ` Sean Anderson
@ 2022-01-15 12:37 ` Tom Rini
1 sibling, 0 replies; 4+ messages in thread
From: Tom Rini @ 2022-01-15 12:37 UTC (permalink / raw)
To: qianfanguijin; +Cc: u-boot, sean.anderson, hs, qianfan Zhao
[-- Attachment #1: Type: text/plain, Size: 649 bytes --]
On Tue, Nov 16, 2021 at 09:35:38AM +0800, qianfanguijin@qq.com wrote:
> From: qianfan Zhao <qianfanguijin@163.com>
>
> CHUNK_TYPE_RAW buffer is not aligned, and flash sparse images by
> fastboot will report "Misaligned operation" if DCACHE is enabled.
>
> Flashing Sparse Image
> CACHE: Misaligned operation at range [84000028, 84001028]
> CACHE: Misaligned operation at range [84001034, 84002034]
> CACHE: Misaligned operation at range [8401104c, 8401304c]
>
> Fix it
>
> Signed-off-by: qianfan Zhao <qianfanguijin@163.com>
> Reviewed-by: Sean Anderson <sean.anderson@seco.com>
Applied to u-boot/master, thanks!
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-15 12:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-16 1:35 [PATCH v4] lib: sparse: Make CHUNK_TYPE_RAW buffer aligned qianfanguijin
2021-11-16 15:05 ` Sean Anderson
[not found] ` <tencent_2353D67A319ACDD88C7A564D884FA2FA4F05@qq.com>
2021-11-18 19:46 ` Sean Anderson
2022-01-15 12:37 ` Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.