You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "dalmemail (via GitHub)" <gi...@apache.org> on 2025/03/06 17:57:00 UTC
[PR] arch/arm/src/stm32f0l0g0: Fix SPI reads when nbits is 8 [nuttx]
dalmemail opened a new pull request, #15949:
URL: https://github.com/apache/nuttx/pull/15949
## Summary
I found an issue when using SPI with nbits=8: SPI reads return unreliable results because SPI ignores nbits value and always performs 16-bit reads. The problem is fixed by adding specific logic to spi_readword() to handle the 8-bit case similar to the one already present on spi_writeword(). See also: http://efton.sk/STM32/gotcha/g22.html
## Impact
This allows drivers using SPI with nbits=8 to perform reads properly.
## Testing
One of our drivers, which uses spi with nbits=8, works on the stm32u5 but didn't on stm32f0l0g0 since many spi reads return 0 (when they shouldn't) or other unexpected results. After quite some testing we found this issue and after fixing it (applying this patch) it started working flawlessly.
--
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] arch/arm/src/stm32f0l0g0: Fix SPI reads when nbits is 8 [nuttx]
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao merged PR #15949:
URL: https://github.com/apache/nuttx/pull/15949
--
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] arch/arm/src/stm32f0l0g0: Fix SPI reads when nbits is 8 [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #15949:
URL: https://github.com/apache/nuttx/pull/15949#discussion_r1983892440
##########
arch/arm/src/stm32f0l0g0/stm32_spi.c:
##########
@@ -614,7 +637,16 @@ static inline uint16_t spi_readword(struct stm32_spidev_s *priv)
/* Then return the received byte */
- return spi_getreg(priv, STM32_SPI_DR_OFFSET);
+#ifdef HAVE_IP_SPI_V2
+ if (priv->nbits < 9)
+ {
+ return spi_getreg8(priv, STM32_SPI_DR_OFFSET);
Review Comment:
Look arch/arm/src/stm32/stm32_spi.c line 862 for reference
##########
arch/arm/src/stm32f0l0g0/stm32_spi.c:
##########
@@ -614,7 +637,16 @@ static inline uint16_t spi_readword(struct stm32_spidev_s *priv)
/* Then return the received byte */
- return spi_getreg(priv, STM32_SPI_DR_OFFSET);
+#ifdef HAVE_IP_SPI_V2
+ if (priv->nbits < 9)
+ {
+ return spi_getreg8(priv, STM32_SPI_DR_OFFSET);
Review Comment:
```suggestion
return (uint16_t) spi_getreg8(priv, STM32_SPI_DR_OFFSET);
--
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] arch/arm/src/stm32f0l0g0: Fix SPI reads when nbits is 8 [nuttx]
Posted by "nuttxpr (via GitHub)" <gi...@apache.org>.
nuttxpr commented on PR #15949:
URL: https://github.com/apache/nuttx/pull/15949#issuecomment-2704578396
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
Yes, this PR appears to meet the NuttX requirements, although some sections could be expanded for clarity and completeness.
**Strengths:**
* **Clear problem description:** The summary clearly explains the bug and the proposed solution.
* **External reference provided:** Linking to the STM32 gotcha page is helpful for understanding the underlying hardware issue.
* **Testing demonstrates impact:** The testing section shows a real-world improvement in driver functionality.
**Areas for improvement:**
* **Summary:** While the summary describes the "what" and "how," it doesn't explicitly state *why* this change is necessary beyond the immediate bug fix. Consider adding a sentence about the broader impact of incorrect SPI reads (e.g., "This change is necessary to ensure reliable data transfer when using SPI peripherals with 8-bit data width, preventing potential data corruption or device malfunction."). Also, specify the affected functional part of the code (e.g., "The `spi_readword()` function in the SPI driver").
* **Impact:** While the impact on the user is mentioned, the other impact categories are left blank. Even if the answer is "NO," explicitly stating it is good practice. For example:
* Impact on build: NO
* Impact on hardware: YES (stm32f0l0g0 and potentially other STM32 platforms)
* Impact on documentation: Possibly YES (if the SPI driver documentation doesn't already address 8-bit reads, it might need updating)
* Impact on security: NO (unless incorrect reads could have security implications, which is unlikely but worth considering)
* Impact on compatibility: NO (should improve compatibility with devices requiring 8-bit SPI transfers)
* **Testing:** The testing section is good but could be stronger. Provide more details about the build host and target environments. For example:
* Build Host: Linux (Ubuntu 20.04), x86_64, GCC 9.4.0
* Target: stm32f0l0g0 and stm32u5 (mention the board names and configurations used)
While the qualitative description of the test results is helpful, including snippets of the actual logs (e.g., showing incorrect vs. correct data reads) would be even better. If the logs are extensive, consider attaching them as a file to the PR.
By addressing these points, the PR will be even stronger and easier for reviewers to assess.
--
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