You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/09/15 23:43:43 UTC
[GitHub] [incubator-nuttx] btashton opened a new pull request #1798: mmcsd: Allow setting ID Mode SPI frequency vi Kconfig
btashton opened a new pull request #1798:
URL: https://github.com/apache/incubator-nuttx/pull/1798
## Summary
The SPI frequency of `IDMODE_CLOCK` clock for SPI was hard coded. This change keeps the default of 400KHz, but allows for it to be overwritten via Kconfig.
## Impact
Allow setting alternative clock rates.
## Testing
Verified on nRF52 platform at a rate of 250kHZ.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #1798: mmcsd: Allow setting ID Mode SPI frequency vi Kconfig
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #1798:
URL: https://github.com/apache/incubator-nuttx/pull/1798#discussion_r489099021
##########
File path: drivers/mmcsd/mmcsd_spi.c
##########
@@ -61,7 +61,9 @@
# define CONFIG_MMCSD_NSLOTS 1
#endif
-#define MMCSD_IDMODE_CLOCK (400000)
+#ifndef CONFIG_MMCSD_IDMODE_CLOCK
+# define CONFIG_MMCSD_IDMODE_CLOCK 400000
+#endif
Review comment:
Yeah, I would say it is best to remove it
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #1798: mmcsd: Allow setting ID Mode SPI frequency vi Kconfig
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #1798:
URL: https://github.com/apache/incubator-nuttx/pull/1798#discussion_r489099771
##########
File path: drivers/mmcsd/mmcsd_spi.c
##########
@@ -61,7 +61,9 @@
# define CONFIG_MMCSD_NSLOTS 1
#endif
-#define MMCSD_IDMODE_CLOCK (400000)
+#ifndef CONFIG_MMCSD_IDMODE_CLOCK
+# define CONFIG_MMCSD_IDMODE_CLOCK 400000
+#endif
Review comment:
>
>
> This is a pattern I see in a lot of places, for example right above with `CONFIG_MMCSD_NSLOTS` so I keeping it. Should I remove that as well?
It is a holdover from the days before kconfig-frontends when .config files were edited by hand. In that case, extensive verification of configuration settings was needed at compile time. But it is probably no longer necessary; we depend on the Kconfigs to assure that everything is defined and valid.
There is a lot of old stuff in config.h generated in mkconfig.c that could be removed too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 merged pull request #1798: mmcsd: Allow setting ID Mode SPI frequency vi Kconfig
Posted by GitBox <gi...@apache.org>.
davids5 merged pull request #1798:
URL: https://github.com/apache/incubator-nuttx/pull/1798
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] btashton commented on a change in pull request #1798: mmcsd: Allow setting ID Mode SPI frequency vi Kconfig
Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #1798:
URL: https://github.com/apache/incubator-nuttx/pull/1798#discussion_r489106419
##########
File path: drivers/mmcsd/mmcsd_spi.c
##########
@@ -61,7 +61,9 @@
# define CONFIG_MMCSD_NSLOTS 1
#endif
-#define MMCSD_IDMODE_CLOCK (400000)
+#ifndef CONFIG_MMCSD_IDMODE_CLOCK
+# define CONFIG_MMCSD_IDMODE_CLOCK 400000
+#endif
Review comment:
@v01d I removed the config defaults in that file that are defined in `Kconfig`. There is one more config `CONFIG_MMCSD_SECTOR512` but it does not look to be in Kconfig so I don't want to touch it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on a change in pull request #1798: mmcsd: Allow setting ID Mode SPI frequency vi Kconfig
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #1798:
URL: https://github.com/apache/incubator-nuttx/pull/1798#discussion_r489094836
##########
File path: drivers/mmcsd/mmcsd_spi.c
##########
@@ -61,7 +61,9 @@
# define CONFIG_MMCSD_NSLOTS 1
#endif
-#define MMCSD_IDMODE_CLOCK (400000)
+#ifndef CONFIG_MMCSD_IDMODE_CLOCK
+# define CONFIG_MMCSD_IDMODE_CLOCK 400000
+#endif
Review comment:
I think there's no need for this, Kconfig will always define it if you have the default
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] btashton commented on a change in pull request #1798: mmcsd: Allow setting ID Mode SPI frequency vi Kconfig
Posted by GitBox <gi...@apache.org>.
btashton commented on a change in pull request #1798:
URL: https://github.com/apache/incubator-nuttx/pull/1798#discussion_r489097150
##########
File path: drivers/mmcsd/mmcsd_spi.c
##########
@@ -61,7 +61,9 @@
# define CONFIG_MMCSD_NSLOTS 1
#endif
-#define MMCSD_IDMODE_CLOCK (400000)
+#ifndef CONFIG_MMCSD_IDMODE_CLOCK
+# define CONFIG_MMCSD_IDMODE_CLOCK 400000
+#endif
Review comment:
This is a pattern I see in a lot of places, for example right above with `CONFIG_MMCSD_NSLOTS` so I keeping it. Should I remove that as well?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org