Skip to content

drivers/spi/ice40.c: operator precedence issue in final clock cycles calculation #19367

Description

@Zepp-Hanzj

Description

In drivers/spi/ice40.c, line 272:

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"

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions