Zepp-Hanzj opened a new issue, #19367:
URL: https://github.com/apache/nuttx/issues/19367

   ## Description
   
   In `drivers/spi/ice40.c`, line 272:
   
   ```c
   for (size_t i = 0; i < ICE40_SPI_FINAL_CLK_CYCLES + 7 / 8; i++)
   ```
   
   Due to C operator precedence, `7 / 8` is evaluated first (integer division → 
0), making the `+ 7 / 8` a no-op. The expression evaluates to 
`ICE40_SPI_FINAL_CLK_CYCLES + 0 = 160`.
   
   This looks like it was intended to be the classic ceiling-division pattern 
`(n + 7) / 8` (convert bits to bytes), which would give `(160 + 7) / 8 = 20`.
   
   **Current:** sends 160 bytes (1280 clock cycles)
   **If `(160 + 7) / 8`:** sends 20 bytes (160 clock cycles)
   
   Since the macro `ICE40_SPI_FINAL_CLK_CYCLES` is defined as 160 in 
`include/nuttx/spi/ice40.h:54`, the name suggests it represents a **clock cycle 
count**, not a byte count. Sending 160 dummy bytes (1280 cycles) may be 
functionally harmless on real hardware — just excessive — which might be why it 
passed testing.
   
   ## Questions for maintainers
   
   1. Is the intent to send `ICE40_SPI_FINAL_CLK_CYCLES` clock cycles (160, 
i.e. 20 bytes), or 160 bytes (1280 cycles)?
   2. The original commit (i558b03) message says "Tested on ICE-V-Wireless 
board" — has this been tested with the fix?
   
   ## Reference
   
   - File: `drivers/spi/ice40.c:272`
   - Macro: `include/nuttx/spi/ice40.h:54` — `#define 
ICE40_SPI_FINAL_CLK_CYCLES 160`
   - Commit: `i558b032a12` — "Added Cypress ICE40 FPGA support"


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to