You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "Kevin-Zhou11 (via GitHub)" <gi...@apache.org> on 2024/09/03 07:05:30 UTC
[PR] xtensa/esp32s3: Add timing delay set interface for QSPI [nuttx]
Kevin-Zhou11 opened a new pull request, #13277:
URL: https://github.com/apache/nuttx/pull/13277
## Summary
When esp32s3 read from slave device through qspi, the data input signal has timing delay compared to clock signal, the higher the SPI frequency, the more impact to SPI sampling data, cause to receive wrong data.
esp32s3 has SPI timing compensation for this Scenario, add interface qspi driver to set timing compensation for the delay.
## Impact
No impact
## Testing
Build with esp32s3-devkit, test with project board.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] xtensa/esp32s3: Add timing delay set interface for QSPI [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on PR #13277:
URL: https://github.com/apache/nuttx/pull/13277#issuecomment-2326464977
Hi @Kevin-Zhou11 , thanks for the submission!
I will test it on our internal CI. Please wait to merge 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] xtensa/esp32s3: Add timing delay set interface for QSPI [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #13277:
URL: https://github.com/apache/nuttx/pull/13277#discussion_r1742245725
##########
arch/xtensa/src/esp32s3/esp32s3_qspi.c:
##########
@@ -1529,6 +1529,56 @@ int esp32s3_qspibus_set_attr(struct qspi_dev_s *dev,
return OK;
}
+/****************************************************************************
+ * Name: esp32s3_qspibus_set_delay
+ *
+ * Description:
+ * Set timing delay for QSPI input and output data.
+ *
+ * Input Parameters:
+ * dev - Device-specific state data
+ * din_mode - Input mode(0~3) to delay input data
+ * din_num - The delay number(0~3) to input data
+ * dout_mode - Output mode(0,1) to delay output data
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success. Otherwise -1 (ERROR).
+ *
+ ****************************************************************************/
+
+int esp32s3_qspibus_set_delay(struct qspi_dev_s *dev,
+ uint8_t din_mode,
+ uint8_t din_num,
+ uint8_t dout_mode)
+{
+ struct esp32s3_qspi_priv_s *priv = (struct esp32s3_qspi_priv_s *)dev;
+ uint8_t id = priv->config->id;
+
+ if ((din_mode > 3) || (din_num > 3) || (dout_mode > 1))
+ {
+ return -1;
+ }
+
+ uint32_t din_mode_reg = (din_mode << SPI_DIN0_MODE_S) |
+ (din_mode << SPI_DIN1_MODE_S) |
+ (din_mode << SPI_DIN2_MODE_S) |
+ (din_mode << SPI_DIN3_MODE_S);
+ uint32_t din_num_reg = (din_num << SPI_DIN0_NUM_S) |
+ (din_num << SPI_DIN1_NUM_S) |
+ (din_num << SPI_DIN2_NUM_S) |
+ (din_num << SPI_DIN3_NUM_S);
+ uint32_t dout_mode_reg = (dout_mode << SPI_DOUT0_MODE_S) |
+ (dout_mode << SPI_DOUT1_MODE_S) |
+ (dout_mode << SPI_DOUT2_MODE_S) |
+ (dout_mode << SPI_DOUT3_MODE_S);
Review Comment:
Ditto
##########
arch/xtensa/src/esp32s3/esp32s3_qspi.h:
##########
@@ -84,6 +84,28 @@ int esp32s3_qspibus_set_attr(struct qspi_dev_s *dev,
uint8_t addr_lines,
uint8_t data_lines);
+/****************************************************************************
+ * Name: esp32s3_qspibus_set_delay
+ *
+ * Description:
+ * Set timing delay for QSPI input and output data.
+ *
+ * Input Parameters:
+ * dev - Device-specific state data
+ * din_mode - Input mode(0~3) to delay input data
+ * din_num - The delay number(0~3) to input data
+ * dout_mode - Output mode(0,1) to delay output data
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success. Otherwise -1 (ERROR).
+ *
+ ****************************************************************************/
+
+int esp32s3_qspibus_set_delay(struct qspi_dev_s *dev,
+ uint8_t din_mode,
+ uint8_t din_num,
+ uint8_t dout_mode);
Review Comment:
Ditto
##########
arch/xtensa/src/esp32s3/esp32s3_qspi.c:
##########
@@ -1529,6 +1529,56 @@ int esp32s3_qspibus_set_attr(struct qspi_dev_s *dev,
return OK;
}
+/****************************************************************************
+ * Name: esp32s3_qspibus_set_delay
+ *
+ * Description:
+ * Set timing delay for QSPI input and output data.
+ *
+ * Input Parameters:
+ * dev - Device-specific state data
+ * din_mode - Input mode(0~3) to delay input data
+ * din_num - The delay number(0~3) to input data
+ * dout_mode - Output mode(0,1) to delay output data
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success. Otherwise -1 (ERROR).
+ *
+ ****************************************************************************/
+
+int esp32s3_qspibus_set_delay(struct qspi_dev_s *dev,
+ uint8_t din_mode,
+ uint8_t din_num,
+ uint8_t dout_mode)
+{
+ struct esp32s3_qspi_priv_s *priv = (struct esp32s3_qspi_priv_s *)dev;
+ uint8_t id = priv->config->id;
+
+ if ((din_mode > 3) || (din_num > 3) || (dout_mode > 1))
+ {
+ return -1;
+ }
+
+ uint32_t din_mode_reg = (din_mode << SPI_DIN0_MODE_S) |
+ (din_mode << SPI_DIN1_MODE_S) |
+ (din_mode << SPI_DIN2_MODE_S) |
+ (din_mode << SPI_DIN3_MODE_S);
+ uint32_t din_num_reg = (din_num << SPI_DIN0_NUM_S) |
+ (din_num << SPI_DIN1_NUM_S) |
+ (din_num << SPI_DIN2_NUM_S) |
+ (din_num << SPI_DIN3_NUM_S);
Review Comment:
Ditto
##########
arch/xtensa/src/esp32s3/esp32s3_qspi.c:
##########
@@ -1529,6 +1529,56 @@ int esp32s3_qspibus_set_attr(struct qspi_dev_s *dev,
return OK;
}
+/****************************************************************************
+ * Name: esp32s3_qspibus_set_delay
+ *
+ * Description:
+ * Set timing delay for QSPI input and output data.
+ *
+ * Input Parameters:
+ * dev - Device-specific state data
+ * din_mode - Input mode(0~3) to delay input data
+ * din_num - The delay number(0~3) to input data
+ * dout_mode - Output mode(0,1) to delay output data
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success. Otherwise -1 (ERROR).
+ *
+ ****************************************************************************/
+
+int esp32s3_qspibus_set_delay(struct qspi_dev_s *dev,
+ uint8_t din_mode,
+ uint8_t din_num,
+ uint8_t dout_mode)
Review Comment:
Please fix alignment: all uint8_t should be left aligned with struct name from previous line
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] xtensa/esp32s3: Add timing delay set interface for QSPI [nuttx]
Posted by "Kevin-Zhou11 (via GitHub)" <gi...@apache.org>.
Kevin-Zhou11 commented on PR #13277:
URL: https://github.com/apache/nuttx/pull/13277#issuecomment-2329173807
@tmedicci, thanks for your review.
> Internal CI testing passed successfully. Where are these interfaces are being used?
Our project has timing delayed issue, I call this interface in slave driver (such lan9250) after qspi initialize, below is an example.
```
g_dev = esp32s3_qspibus_initialize(port);
if (!g_dev)
{
nerr("ERROR: Failed to initialize QSPI port %d\n", port);
return -ENODEV;
}
ret = esp32s3_qspibus_set_delay(g_dev, 1, 0, 0);
if (ret < 0)
{
nerr("ERROR: Failed to set delay timing, ret=%d\n", ret);
}
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] xtensa/esp32s3: Add timing delay set interface for QSPI [nuttx]
Posted by "Kevin-Zhou11 (via GitHub)" <gi...@apache.org>.
Kevin-Zhou11 commented on PR #13277:
URL: https://github.com/apache/nuttx/pull/13277#issuecomment-2329196685
@acassis, thanks for your review.
I have fixed the alignment issue, please check.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] xtensa/esp32s3: Add timing delay set interface for QSPI [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #13277:
URL: https://github.com/apache/nuttx/pull/13277
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org