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 2021/05/31 13:13:51 UTC
[GitHub] [incubator-nuttx] gustavonihei opened a new pull request #3809: drivers/spi: Change prefix to a more intuitive "spislave"
gustavonihei opened a new pull request #3809:
URL: https://github.com/apache/incubator-nuttx/pull/3809
## Summary
This PR intends to rename the prefix initially adopted for the SPI Slave module on the recently added character driver.
**spislave** seems more intuitive then **spislv**, and the price for this improvement is just 2 chars.
## Impact
No impact, just renaming of the module prefix.
## Testing
CI build success
--
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] gustavonihei commented on a change in pull request #3809: drivers/spi: Change prefix to a more intuitive "spislave"
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3809:
URL: https://github.com/apache/incubator-nuttx/pull/3809#discussion_r644687137
##########
File path: drivers/spi/spi_slave_driver.c
##########
@@ -57,7 +57,7 @@
* Private Types
****************************************************************************/
-struct spislv_driver_s
+struct spislave_driver_s
Review comment:
I wasn't aware of the I2C Slave interface, thanks for letting me know.
This proposal is nice, I agree with the change.
I can submit this change tomorrow, or feel free to submit 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] xiaoxiang781216 commented on a change in pull request #3809: drivers/spi: Change prefix to a more intuitive "spislave"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #3809:
URL: https://github.com/apache/incubator-nuttx/pull/3809#discussion_r644445001
##########
File path: drivers/spi/spi_slave_driver.c
##########
@@ -57,7 +57,7 @@
* Private Types
****************************************************************************/
-struct spislv_driver_s
+struct spislave_driver_s
Review comment:
Here is the convention used by i2c slave:
i2c_slave_s/i2c_slaveops_s/I2CS_XXX
How about we use the similar convention for spi slave:
spi_slave_dev_s/spi_slave_devops_s/spi_slave_ctrlr_s/spi_slave_ctrlrops_s/SPIS_DEV_XXX/SPIS_CTRLR_XXX
--
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] gustavonihei commented on a change in pull request #3809: drivers/spi: Change prefix to a more intuitive "spislave"
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3809:
URL: https://github.com/apache/incubator-nuttx/pull/3809#discussion_r643397626
##########
File path: drivers/spi/spi_slave_driver.c
##########
@@ -57,7 +57,7 @@
* Private Types
****************************************************************************/
-struct spislv_driver_s
+struct spislave_driver_s
Review comment:
The `spislave_driver_s` follows the same pattern from the `spi_driver_s` naming, considering `spislave_` as the single prefix.
I agree that `spi_sdev_` would be in sync with the prefix adopted in `slave.h`, but it is difficult to infer that `sdev` refers to Slave Device. The same with `spi_sctrlr`. On the other hand, `spislave_dev` and `spislave_ctrlr` are self explanatory, don't you think?
--
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] xiaoxiang781216 commented on a change in pull request #3809: drivers/spi: Change prefix to a more intuitive "spislave"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #3809:
URL: https://github.com/apache/incubator-nuttx/pull/3809#discussion_r644445001
##########
File path: drivers/spi/spi_slave_driver.c
##########
@@ -57,7 +57,7 @@
* Private Types
****************************************************************************/
-struct spislv_driver_s
+struct spislave_driver_s
Review comment:
Here is the convention used by i2c slave:
i2c_slave_s/i2c_slaveops_s/I2CS_XXX
How about we use the similar convention for spi slave:
spi_slave_dev_s/spi_slave_devops_s/spi_slave_ctrlr_s/spi_slave_ctrlrops_s/SPIS_DEV_XXX/SPIS_CTRLR_XXX
##########
File path: drivers/spi/spi_slave_driver.c
##########
@@ -57,7 +57,7 @@
* Private Types
****************************************************************************/
-struct spislv_driver_s
+struct spislave_driver_s
Review comment:
Sure, let's wait your PR.
--
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] gustavonihei commented on a change in pull request #3809: drivers/spi: Change prefix to a more intuitive "spislave"
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3809:
URL: https://github.com/apache/incubator-nuttx/pull/3809#discussion_r644687137
##########
File path: drivers/spi/spi_slave_driver.c
##########
@@ -57,7 +57,7 @@
* Private Types
****************************************************************************/
-struct spislv_driver_s
+struct spislave_driver_s
Review comment:
I wasn't aware of the I2C Slave interface, thanks for letting me know.
This proposal is nice, I agree with the change.
I can submit this change tomorrow, or feel free to submit 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] xiaoxiang781216 commented on a change in pull request #3809: drivers/spi: Change prefix to a more intuitive "spislave"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #3809:
URL: https://github.com/apache/incubator-nuttx/pull/3809#discussion_r642615051
##########
File path: include/nuttx/spi/slave.h
##########
@@ -561,7 +561,7 @@ struct spi_sdev_s
****************************************************************************/
#ifdef CONFIG_SPI_SLAVE_DRIVER
-int spislv_register(FAR struct spi_sctrlr_s *sctrlr, int bus);
+int spislave_register(FAR struct spi_sctrlr_s *sctrlr, int bus);
Review comment:
should we use spi_sdev_ prefix instead just like spi_sdev_s/spi_sdevops_s?
##########
File path: drivers/spi/spi_slave_driver.c
##########
@@ -57,7 +57,7 @@
* Private Types
****************************************************************************/
-struct spislv_driver_s
+struct spislave_driver_s
Review comment:
should we give the driver a special name? since it is most used for simple echo testing.
--
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] xiaoxiang781216 commented on a change in pull request #3809: drivers/spi: Change prefix to a more intuitive "spislave"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #3809:
URL: https://github.com/apache/incubator-nuttx/pull/3809#discussion_r644736464
##########
File path: drivers/spi/spi_slave_driver.c
##########
@@ -57,7 +57,7 @@
* Private Types
****************************************************************************/
-struct spislv_driver_s
+struct spislave_driver_s
Review comment:
Sure, let's wait your PR.
--
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] xiaoxiang781216 commented on a change in pull request #3809: drivers/spi: Change prefix to a more intuitive "spislave"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #3809:
URL: https://github.com/apache/incubator-nuttx/pull/3809#discussion_r642741035
##########
File path: drivers/spi/spi_slave_driver.c
##########
@@ -57,7 +57,7 @@
* Private Types
****************************************************************************/
-struct spislv_driver_s
+struct spislave_driver_s
Review comment:
Ok. how about the prefix(spislave_->spi_sdev_)?
--
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] gustavonihei commented on a change in pull request #3809: drivers/spi: Change prefix to a more intuitive "spislave"
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3809:
URL: https://github.com/apache/incubator-nuttx/pull/3809#discussion_r642622674
##########
File path: drivers/spi/spi_slave_driver.c
##########
@@ -57,7 +57,7 @@
* Private Types
****************************************************************************/
-struct spislv_driver_s
+struct spislave_driver_s
Review comment:
At first I thought of calling it `spi_echo`, but then I changed my mind because it would be too restrictive.
It may be extended for further than just echo testing.
One may create a test application that gives real responses to commands from Master.
The driver idea is to serve just a bridge to the userspace.
--
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 #3809: drivers/spi: Change prefix to a more intuitive "spislave"
Posted by GitBox <gi...@apache.org>.
davids5 merged pull request #3809:
URL: https://github.com/apache/incubator-nuttx/pull/3809
--
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