-
Notifications
You must be signed in to change notification settings - Fork 89
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.
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
Use 32 bit sector id (IEC-140) #356
Use 32 bit sector id (IEC-140) #356
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, just one suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me!
It would be great if you could clean up the commit history in this branch. (If not, no issue, I can do it for you before merging.)
|
||
if (capacity > 0xFFFF) { | ||
ESP_LOGE(TAG, "sector size too large"); | ||
return RES_PARERR; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@igrr
Do you think this is the appropriate error to return when overflowing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems fine, it should prevent FATFS from initializing further.
Do you mean to rebase? Or squash? |
If doing an interactive rebase ( |
d234913
to
794abb0
Compare
794abb0
to
892d994
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me. @UnTraDe thank you for the PR.
@igrr Anything else before merging? |
Checklist
url
field definedChange description
The
dhara
library uses thedhara_sector_t
(typedef uint32_t dhara_sector_t
) to address sectors internally. This PR changes the component's interface to acceptuint32_t
sector id where applicable.This change will allow to utilize the entire capacity of NAND chips with more than 2^16 sectors
As discussed in #327 (comment)