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

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions