xiaoxiang781216 commented on a change in pull request #1115:
URL: https://github.com/apache/incubator-nuttx/pull/1115#discussion_r430149389
##########
File path: include/nuttx/spi/slave.h
##########
@@ -109,18 +111,20 @@
*
* Input Parameters:
* sctrlr - SPI slave controller interface instance
- * data - Command/data mode data value to be shifted out. The width of
- * the data must be the same as the nbits parameter previously
- * provided to the bind() methods.
+ * data - Pointer to the command/data mode data to be shifted out.
Review comment:
data -> value
##########
File path: include/nuttx/spi/slave.h
##########
@@ -109,18 +111,20 @@
*
* Input Parameters:
* sctrlr - SPI slave controller interface instance
- * data - Command/data mode data value to be shifted out. The width of
- * the data must be the same as the nbits parameter previously
- * provided to the bind() methods.
+ * data - Pointer to the command/data mode data to be shifted out.
+ * The alignment of the data must be the same as the nbits
Review comment:
alignment to width?
##########
File path: include/nuttx/spi/slave.h
##########
@@ -475,8 +503,10 @@ struct spi_sdevops_s
{
CODE void (*select)(FAR struct spi_sdev_s *sdev, bool selected);
CODE void (*cmddata)(FAR struct spi_sdev_s *sdev, bool data);
- CODE uint16_t (*getdata)(FAR struct spi_sdev_s *sdev);
- CODE void (*receive)(FAR struct spi_sdev_s *sdev, uint16_t cmd);
+ CODE uint16_t (*getdata)(FAR struct spi_sdev_s *sdev,
+ const uint16_t **data);
+ CODE uint16_t (*receive)(FAR struct spi_sdev_s *sdev,
Review comment:
CODE size_t (*getdata)(FAR struct spi_sdev_s *sdev,
FAR const void **data);
CODE size_t (*receive)(FAR struct spi_sdev_s *sdev,
FAR const void *data, size_t nwords);
##########
File path: include/nuttx/spi/slave.h
##########
@@ -157,6 +161,23 @@
#define SPI_SCTRLR_QFLUSH(c) ((c)->ops->qflush(c))
+/****************************************************************************
+ * Name: SPI_SCTRLR_RXQPOLL
+ *
+ * Description:
+ * Tell the controller to output all the receive queue data.
+ *
+ * Input Parameters:
+ * sctrlr - SPI slave controller interface instance
+ *
+ * Returned Value:
+ * Number of bytes left in the rx queue. If the device accepted all the
+ * data, the return value will be 0
+ *
+ ****************************************************************************/
+
+#define SPI_SCTRLR_RXQPOLL(c) ((c)->ops->rxqpoll(c))
Review comment:
Will this call block until the queue become empty?
##########
File path: include/nuttx/spi/slave.h
##########
@@ -245,10 +268,12 @@
*
* Input Parameters:
* sdev - SPI device interface instance
- * data - The last command/data value that was shifted in
+ * data - Pointer to the new data that has been shifted in
+ * len - Length of the new data in bytes
*
* Returned Value:
- * None
+ * Number of bytes accepted by the device. In other words,
Review comment:
Should we change bytes to unit? SPI is transfered by nbits which mayn't
equal 8bits.
##########
File path: include/nuttx/spi/slave.h
##########
@@ -157,6 +161,23 @@
#define SPI_SCTRLR_QFLUSH(c) ((c)->ops->qflush(c))
+/****************************************************************************
+ * Name: SPI_SCTRLR_RXQPOLL
Review comment:
Should we change to
SPI_SCTRLR_QEMPTY and qempty?
##########
File path: include/nuttx/spi/slave.h
##########
@@ -451,9 +477,11 @@ struct spi_sctrlrops_s
FAR struct spi_sdev_s *sdev, enum spi_smode_e mode,
int nbits);
CODE void (*unbind)(FAR struct spi_sctrlr_s *sctrlr);
- CODE int (*enqueue)(FAR struct spi_sctrlr_s *sctrlr, uint16_t data);
+ CODE int (*enqueue)(FAR struct spi_sctrlr_s *sctrlr,
+ const uint16_t *data, uint16_t len);
Review comment:
Change to:
FAR const void *data, size_t nwords
It's better to align with spi.h
##########
File path: include/nuttx/spi/slave.h
##########
@@ -157,6 +161,23 @@
#define SPI_SCTRLR_QFLUSH(c) ((c)->ops->qflush(c))
+/****************************************************************************
+ * Name: SPI_SCTRLR_RXQPOLL
+ *
+ * Description:
+ * Tell the controller to output all the receive queue data.
+ *
+ * Input Parameters:
+ * sctrlr - SPI slave controller interface instance
+ *
+ * Returned Value:
+ * Number of bytes left in the rx queue. If the device accepted all the
+ * data, the return value will be 0
+ *
+ ****************************************************************************/
+
+#define SPI_SCTRLR_RXQPOLL(c) ((c)->ops->rxqpoll(c))
Review comment:
How about we put the above description to header file and reword line
266 to 434?
##########
File path: include/nuttx/spi/slave.h
##########
@@ -157,6 +161,23 @@
#define SPI_SCTRLR_QFLUSH(c) ((c)->ops->qflush(c))
+/****************************************************************************
+ * Name: SPI_SCTRLR_RXQPOLL
Review comment:
From your description, qpoll is also a good candiate, but RX make the
confusion.
----------------------------------------------------------------
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:
[email protected]
