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

Jump to bottom

fix the bug: if LITTLEFS_SDMMC_SUPPORT turns on, some features are pr… #172

Merged
merged 1 commit into from
Jan 29, 2024
Merged

fix the bug: if LITTLEFS_SDMMC_SUPPORT turns on, some features are pr… #172

merged 1 commit into from
Jan 29, 2024

Conversation

wang33winner
Copy link
Contributor

…oblematic when using flash.
some macro for CONFIG_LITTLEFS_SDMMC_SUPPORT will drop the work for flash

BrianPugh
BrianPugh reviewed Jan 26, 2024
View reviewed changes
Copy link
Member

@BrianPugh BrianPugh left a comment

Choose a reason for hiding this comment

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

@huming2207 can you take a look at this? There appears to be 3 real changes among the formatting/linting changes.

src/esp_littlefs.c
@@ -253,13 +253,12 @@ esp_err_t format_from_efs(esp_littlefs_t *efs)
if (efs->sdcard) {
res = lfs_format(efs->fs, &efs->cfg);
} else
#else
#endif
Copy link
Member

Choose a reason for hiding this comment

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

real change

src/esp_littlefs.c
@@ -1072,11 +1071,10 @@ static esp_err_t esp_littlefs_init(const esp_vfs_littlefs_conf_t* conf)
if (conf->sdcard) {
err = esp_littlefs_format_sdmmc(conf->sdcard);
} else
#else
#endif
Copy link
Member

Choose a reason for hiding this comment

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

real change

src/esp_littlefs.c
#ifndef CONFIG_LITTLEFS_SDMMC_SUPPORT
res = lfs_fs_grow(efs->fs, efs->partition->size / efs->cfg.block_size);
#else
#ifdef CONFIG_LITTLEFS_SDMMC_SUPPORT
Copy link
Member

Choose a reason for hiding this comment

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

real change

wang33winner
wang33winner commented Jan 26, 2024
View reviewed changes
src/esp_littlefs.c
@@ -1093,15 +1091,14 @@ static esp_err_t esp_littlefs_init(const esp_vfs_littlefs_conf_t* conf)
efs->cache = esp_littlefs_calloc(sizeof(*efs->cache), efs->cache_size);

if(conf->grow_on_mount){
#ifndef CONFIG_LITTLEFS_SDMMC_SUPPORT
res = lfs_fs_grow(efs->fs, efs->partition->size / efs->cfg.block_size);
#else
Copy link
Contributor Author

Choose a reason for hiding this comment

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

need as well

wang33winner
wang33winner commented Jan 26, 2024
View reviewed changes
src/esp_littlefs.c
if (efs->sdcard) {
res = lfs_fs_grow(efs->fs, efs->sdcard->csd.capacity);
} else {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

need as well

wang33winner
wang33winner commented Jan 26, 2024
View reviewed changes
src/esp_littlefs.c
res = lfs_fs_grow(efs->fs, efs->partition->size / efs->cfg.block_size);
}
#endif
Copy link
Contributor Author

Choose a reason for hiding this comment

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

need as well

wang33winner
wang33winner commented Jan 26, 2024
View reviewed changes
src/esp_littlefs.c
{
efs->cfg.block_count = efs->partition->size / efs->cfg.block_size;
res = lfs_format(efs->fs, &efs->cfg);
efs->cfg.block_count = 0;
}
#endif
Copy link
Contributor Author

Choose a reason for hiding this comment

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

need as well

wang33winner
wang33winner commented Jan 26, 2024
View reviewed changes
src/esp_littlefs.c
{
err = esp_littlefs_format_partition(efs->partition);
}
#endif
Copy link
Contributor Author

Choose a reason for hiding this comment

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

need as well

@wang33winner
Copy link
Contributor Author

@huming2207 can you take a look at this? There appears to be 3 real changes among the formatting/linting changes.

yes, some "endif" modifications are also required, others are formatting/linting changes.

@BrianPugh
Copy link
Member

the code changes look good to me, I'll merge in a day or two (unless huming2207 provides back sooner).

@huming2207
Copy link
Collaborator

huming2207 commented Jan 26, 2024 •
edited
Loading

I've reviewed the code - frankly speaking I don't know what is this for. Instead, this PR will definitely break SD card support.

I really don't think the LITTLEFS_SDMMC_SUPPORT will affect any existing LittleFS functionalities on SPI NOR flash over esp_partition. My colleagues are also using v1.13 for production on SPI NOR flash and they have no complaints so far. I will double check with them on next Monday.

@huming2207
Copy link
Collaborator

Oh I'm really sorry, I misread the code on iPad 😅😅

I think it's fine. I will test it with our real devices and see if that actually cause any issues on Monday. On the other hand so far I didn't hear any report about breaking on esp_partition on SPI flash though, I will double check again with my colleagues...

@BrianPugh BrianPugh mentioned this pull request Jan 27, 2024
Merged
@huming2207
Copy link
Collaborator

Hi @BrianPugh we've tested it and we believe it's all good to go.

@BrianPugh BrianPugh merged commit 1e2bb0a into joltwallet:master Jan 29, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BrianPugh BrianPugh BrianPugh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wang33winner @BrianPugh @huming2207