Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

storeage/stream_flash: Cache write_block_size to ctx on init #68658

Merged
merged 1 commit into from
May 29, 2024
Merged

storeage/stream_flash: Cache write_block_size to ctx on init #68658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions include/zephyr/storage/stream_flash.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ struct stream_flash_ctx {
#ifdef CONFIG_STREAM_FLASH_ERASE
off_t last_erased_page_start_offset; /* Last erased offset */
#endif
uint8_t erase_value;
uint8_t write_block_size; /* Offset/size device write alignment */
Copy link
Contributor

@guicnDell guicnDell Jul 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@de-nordic I have now updated to Zephyr 3.7 and this is a bug for me.
The block size on the device I'm using, NXP's LPC55S59, is 512 and of course, we have an issue since this was defined as 1 byte (max size is 255).
This needs to be size_t.

How should we proceed here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi. I have provided fix here #76053.
Thanks for reporting the issue.
I just want to mention that you are always free to provide a fix that will go under review, so any doubts you can have, regarding technical solution, can be resolved together in the context of your contribution; so next time you find something like this I would like to encourage you to log an issue and, if you have time, follow it with a fix.

};

/**
Expand Down
14 changes: 10 additions & 4 deletions subsys/storage/stream/stream_flash.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,10 @@ static int flash_sync(struct stream_flash_ctx *ctx)
}
}

fill_length = flash_get_write_block_size(ctx->fdev);
fill_length = ctx->write_block_size;
if (ctx->buf_bytes % fill_length) {
fill_length -= ctx->buf_bytes % fill_length;
filler = flash_get_parameters(ctx->fdev)->erase_value;
filler = ctx->erase_value;

memset(ctx->buf + ctx->buf_bytes, filler, fill_length);
} else {
Expand Down Expand Up @@ -249,6 +249,7 @@ int stream_flash_init(struct stream_flash_ctx *ctx, const struct device *fdev,
uint8_t *buf, size_t buf_len, size_t offset, size_t size,
stream_flash_callback_t cb)
{
const struct flash_parameters *params;
if (!ctx || !fdev || !buf) {
return -EFAULT;
}
Expand All @@ -267,7 +268,10 @@ int stream_flash_init(struct stream_flash_ctx *ctx, const struct device *fdev,
.total_size = 0
};

if (buf_len % flash_get_write_block_size(fdev)) {
params = flash_get_parameters(fdev);
ctx->write_block_size = params->write_block_size;

if (buf_len % ctx->write_block_size) {
LOG_ERR("Buffer size is not aligned to minimal write-block-size");
return -EFAULT;
}
Expand All @@ -281,11 +285,12 @@ int stream_flash_init(struct stream_flash_ctx *ctx, const struct device *fdev,
}

if ((offset + size) > inspect_flash_ctx.total_size ||
offset % flash_get_write_block_size(fdev)) {
offset % ctx->write_block_size) {
LOG_ERR("Incorrect parameter");
return -EFAULT;
}


ctx->fdev = fdev;
ctx->buf = buf;
ctx->buf_len = buf_len;
Expand All @@ -299,6 +304,7 @@ int stream_flash_init(struct stream_flash_ctx *ctx, const struct device *fdev,
#ifdef CONFIG_STREAM_FLASH_ERASE
ctx->last_erased_page_start_offset = -1;
#endif
ctx->erase_value = params->erase_value;

return 0;
}
Expand Down
Loading