-
Notifications
You must be signed in to change notification settings - Fork 84
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
feat(MiscDrivers): Add API for Setting SDHC Data Width #1045
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.
Don't delete this
mxc_version.h
Outdated
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.
or this
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.
Some feedback below - take it or leave it. This looks good, just some thoughts on a potentially cleaner diff for this.
On the version number files - See #937. I still need to get around to an automatic workflow for updating them, but don't delete them in the meantime
Libraries/SDHC/Source/sdhc_lib.c
Outdated
@@ -236,6 +237,8 @@ int MXC_SDHC_Lib_SetBusWidth(mxc_sdhc_data_width bus_width) | |||
uint32_t card_status; | |||
int result; | |||
|
|||
bus_width = bus_width == MXC_SDHC_LIB_DEFAULT_DATA ? MXC_SDHC_Get_Default_DataWidth() : bus_width; |
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.
I think it might be cleaner to come in "over the top" and avoid adding the extra enum value. A direct getter/setter to the state variable would work, so your call from diskio.c
would look like:
MXC_SDHC_Lib_Write(sector, (void *)buff, count, MXC_SDHC_Get_DataWidth())
Usually you're not gonna change your data width more than once at run-time, so a mechanism for setting the default value might be better implemented as a build-time option. Users can use the setter API you've exposed on initialization.
This would let us avoid these modifications to sdhc_lib.c
, since the data width is already an arg to the SDHC functions
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.
That is what is set as in diskio.c
. The point of doing this extra step was to not break the API if someone uses it directly. Since Get/Set default data width doesn't really do more than allow us to get it at the higher level. It is a little Awkard that you set it and then have to pass it anyway. I am fine deleting those modifications though.
9dd470b
to
c6eb053
Compare
Signed-off-by: Sadik Ozer <sadik.ozer@analog.com>
This reverts commit e6474de.
Description
Added API to set default SPI mode (single/quad) for SDHC. Compile time default set to single mode to maintain backwards compatibility.