drivers/spi/ice40: fix operator precedence in final clock cycle count

[94xhn]

drivers/spi/ice40: fix operator precedence in final clock cycle count

Summary

• Why change is necessary (fix, update, new feature)? Bug fix: an operator-precedence error causes ice40_endwrite() to clock out 8x more finalization SPI bytes than intended.

• What functional part of the code is being changed? ice40_endwrite() in drivers/spi/ice40.c, the loop that clocks out dummy bytes after the FPGA bitstream to finish iCE40 configuration.

• How does the change exactly work (what will change and how)? ICE40_SPI_FINAL_CLK_CYCLES + 7 / 8 is parenthesized to (ICE40_SPI_FINAL_CLK_CYCLES + 7) / 8.

  In C, / binds tighter than +, so the unmodified expression parses as ICE40_SPI_FINAL_CLK_CYCLES + (7 / 8) = 160 + 0 = 160; the + 7 / 8 part is a silent no-op. ICE40_SPI_FINAL_CLK_CYCLES is defined as 160 in include/nuttx/spi/ice40.h:54. Each SPI_SEND() call sends one byte = 8 SPI clock cycles (word size is configured to 8 bits via SPI_SETBITS(spi, 8) in ice40_configspi()), so the unmodified loop runs 160 times and clocks out 160 × 8 = 1280 SPI clock cycles.

  The macro name (_CLK_CYCLES, not _BYTES) together with the classic (n + 7) / 8 ceiling-division idiom used elsewhere in embedded code to convert a bit/cycle count into a byte count make it clear the intent was bytes_to_send = ceil(ICE40_SPI_FINAL_CLK_CYCLES / 8) = (160 + 7) / 8 = 20 bytes, i.e. exactly 160 clock cycles, matching the macro name. Adding the missing parentheses restores that intended value (20 bytes / 160 cycles).

• Related NuttX Issue reference if applicable. Fixes drivers/spi/ice40.c: operator precedence issue in final clock cycles calculation #19367, reported by @Zepp-Hanzj on 2026-07-08. I independently verified the analysis in the issue against the current master source before implementing this fix.

Impact

• Is new feature added? Is existing feature changed? NO new feature. Existing behavior of ice40_endwrite()'s configuration-finalization step changes: it now clocks out 160 SPI clock cycles instead of 1280.
• Impact on user (will user need to adapt to change)? NO. Per Lattice's iCE40 Programming and Configuration technical note (FPGA-TN-02001), only >= 49 extra SPI_SCK clock cycles after CDONE goes high are required to complete configuration. Both the buggy (1280) and fixed (160) cycle counts are well above that minimum, so this is not expected to change observable behavior on hardware — it corrects a value that visibly contradicts both the macro's name and the intent documented by the code shape, without being a functional regression risk.
• Impact on build (will build process change)? NO.
• Impact on hardware (will arch(s) / board(s) / driver(s) change)? YES, drivers/spi/ice40.c (iCE40 FPGA bitstream-loading SPI driver, used e.g. by boards/risc-v/esp32c3-legacy iCE40 board support). Only the loop trip count in the finalization step changes; no other logic, no ABI/API/Kconfig change.
• Impact on documentation (is update required / provided)? NO, no user-visible behavior/documentation to update.
• Impact on security (any sort of implications)? NO.
• Impact on compatibility (backward/forward/interoperability)? NO.
• Anything else to consider or add? See "Testing" section below for an important disclosure: I do not have access to real iCE-V-Wireless/iCE40 hardware, so this could not be verified on a physical target. Verification was done with an isolated, faithful reproduction of the arithmetic/loop logic only.

Testing

I do not have access to real iCE40/iCE-V-Wireless hardware, so I could not produce a real-hardware build+runtime log for this change, which I understand is normally expected for driver changes. Given that, here is exactly what I did verify, so maintainers can judge whether it's sufficient or whether hardware confirmation from someone with the board is needed before merge:

• Build Host(s): Windows 11, gcc (MinGW-w64) via Git Bash, gcc -std=c11 -Wall -Wextra -Werror.
• Target(s): N/A — no physical iCE40 board available. Instead of a NuttX board build, I wrote a standalone, non-NuttX host program that copies the exact unmodified loop expression from ice40_endwrite() (before and after the fix), the exact macro definition from include/nuttx/spi/ice40.h, and a mock SPI_SEND() that counts bytes/clock cycles the same way the real one would (1 byte = 8 clock cycles, per SPI_SETBITS(spi, 8) in ice40_configspi()). This isolates and proves the arithmetic/loop-trip-count discrepancy without needing real SPI hardware, but it is not a substitute for an end-to-end hardware test of ice40_endwrite() itself.
• I also confirmed the diff is a single line, that the changed line is well under the 80-column limit, and that this is the only place ICE40_SPI_FINAL_CLK_CYCLES is used in the tree (grep -r ICE40_SPI_FINAL_CLK_CYCLES), so the fix cannot affect any other code path.

Reproduction steps (repository-independent, does not require NuttX build system):

$ gcc -std=c11 -Wall -Wextra -Werror -o repro repro_ice40_clk_cycles.c
$ ./repro

Testing logs (this is the reproduction program's output, not a NuttX board build/run log):

ICE40_SPI_FINAL_CLK_CYCLES = 160

Expression evaluation (matches C operator precedence):
  buggy : ICE40_SPI_FINAL_CLK_CYCLES + 7 / 8   = 160 (parses as 160 + (7/8) = 160 + 0)
  fixed : (ICE40_SPI_FINAL_CLK_CYCLES + 7) / 8 = 20 (parses as (160+7)/8 = 167/8)

Simulated ice40_endwrite() finalization loop:
  BEFORE FIX : 160 SPI_SEND() calls -> 1280 SPI clock cycles (intended: 160)
  AFTER FIX  : 20 SPI_SEND() calls -> 160 SPI clock cycles (intended: 160)

PASS: confirmed 8x discrepancy (160 vs 20 bytes sent) caused by C operator precedence in the unmodified driver code, and confirmed the parenthesized ceiling-division fix produces exactly ICE40_SPI_FINAL_CLK_CYCLES (160) clock cycles as the macro name promises.

(exit code 0, compiled with -Werror and no warnings.)

Full source of the reproduction program (click to expand)

/*
 * repro_ice40_clk_cycles.c
 *
 * Standalone reproduction for apache/nuttx issue #19367 / drivers/spi/ice40.c
 * operator-precedence bug in ice40_endwrite().
 *
 * Real (unmodified) source under test, apache/nuttx @ master,
 * drivers/spi/ice40.c line 272:
 *
 *     for (size_t i = 0; i < ICE40_SPI_FINAL_CLK_CYCLES + 7 / 8; i++)
 *       {
 *         SPI_SEND(dev->spi, 0xff);
 *       }
 *
 * ICE40_SPI_FINAL_CLK_CYCLES is defined in include/nuttx/spi/ice40.h:54 as:
 *
 *     #define ICE40_SPI_FINAL_CLK_CYCLES 160
 *
 * Each call to SPI_SEND() transmits exactly one byte == 8 SPI clock cycles
 * (this is how every other loop in the same file computes byte counts from
 * a clock/bit budget, e.g. ice40_write() and the ICE_SPI_MAX_XFER chunking
 * just above ice40_endwrite() in the same file operate on byte units).
 *
 * Build:  gcc -std=c11 -Wall -Wextra -o repro repro_ice40_clk_cycles.c
 * Run:    ./repro
 */

#include <stdio.h>
#include <stddef.h>
#include <assert.h>

/* Verbatim from include/nuttx/spi/ice40.h:54 */
#define ICE40_SPI_FINAL_CLK_CYCLES 160

static long g_spi_send_calls;
static long g_spi_clock_cycles; /* 8 clock cycles per SPI_SEND() byte */

/* Mock of SPI_SEND(dev->spi, 0xff) from include/nuttx/spi/spi.h.
 * Each invocation clocks out exactly one byte == 8 SPI clock cycles.
 */
static void mock_spi_send(unsigned char byte)
{
  (void)byte;
  g_spi_send_calls++;
  g_spi_clock_cycles += 8;
}

/* Faithful copy of the loop body in ice40_endwrite(), drivers/spi/ice40.c,
 * BEFORE the fix (line 272 verbatim, only SPI_SEND() replaced by the mock).
 */
static void ice40_endwrite_loop_buggy(void)
{
  for (size_t i = 0; i < ICE40_SPI_FINAL_CLK_CYCLES + 7 / 8; i++)
    {
      mock_spi_send(0xff);
    }
}

/* Same loop AFTER the fix: parenthesize the ceiling-division. */
static void ice40_endwrite_loop_fixed(void)
{
  for (size_t i = 0; i < (ICE40_SPI_FINAL_CLK_CYCLES + 7) / 8; i++)
    {
      mock_spi_send(0xff);
    }
}

int main(void)
{
  int status = 0;

  size_t buggy_bound = ICE40_SPI_FINAL_CLK_CYCLES + 7 / 8;
  size_t fixed_bound  = (ICE40_SPI_FINAL_CLK_CYCLES + 7) / 8;

  printf("ICE40_SPI_FINAL_CLK_CYCLES = %d\n\n", ICE40_SPI_FINAL_CLK_CYCLES);

  printf("Expression evaluation (matches C operator precedence):\n");
  printf("  buggy : ICE40_SPI_FINAL_CLK_CYCLES + 7 / 8   = %lu "
         "(parses as %d + (7/8) = %d + 0)\n",
         (unsigned long)buggy_bound, ICE40_SPI_FINAL_CLK_CYCLES,
         ICE40_SPI_FINAL_CLK_CYCLES);
  printf("  fixed : (ICE40_SPI_FINAL_CLK_CYCLES + 7) / 8 = %lu "
         "(parses as (%d+7)/8 = 167/8)\n\n",
         (unsigned long)fixed_bound, ICE40_SPI_FINAL_CLK_CYCLES);

  assert(buggy_bound == 160 && "unexpected buggy bound (repro invalid)");
  assert(fixed_bound == 20 && "unexpected fixed bound (repro invalid)");

  g_spi_send_calls = 0;
  g_spi_clock_cycles = 0;
  ice40_endwrite_loop_buggy();
  long buggy_calls = g_spi_send_calls;
  long buggy_cycles = g_spi_clock_cycles;

  g_spi_send_calls = 0;
  g_spi_clock_cycles = 0;
  ice40_endwrite_loop_fixed();
  long fixed_calls = g_spi_send_calls;
  long fixed_cycles = g_spi_clock_cycles;

  printf("Simulated ice40_endwrite() finalization loop:\n");
  printf("  BEFORE FIX : %ld SPI_SEND() calls -> %ld SPI clock cycles"
         " (intended: %d)\n",
         buggy_calls, buggy_cycles, ICE40_SPI_FINAL_CLK_CYCLES);
  printf("  AFTER FIX  : %ld SPI_SEND() calls -> %ld SPI clock cycles"
         " (intended: %d)\n\n",
         fixed_calls, fixed_cycles, ICE40_SPI_FINAL_CLK_CYCLES);

  if (buggy_calls != 160 || buggy_cycles != 1280)
    {
      printf("FAIL: expected the unmodified driver expression to produce "
             "160 SPI_SEND calls / 1280 cycles\n");
      status = 1;
    }

  if (fixed_calls != 20 || fixed_cycles != 160)
    {
      printf("FAIL: expected the fixed expression to produce 20 SPI_SEND "
             "calls / 160 cycles (== ICE40_SPI_FINAL_CLK_CYCLES)\n");
      status = 1;
    }

  if (status == 0)
    {
      printf("PASS: confirmed 8x discrepancy (160 vs 20 bytes sent) caused "
             "by C operator precedence in the unmodified driver code, and "
             "confirmed the parenthesized ceiling-division fix produces "
             "exactly ICE40_SPI_FINAL_CLK_CYCLES (160) clock cycles as "
             "the macro name promises.\n");
    }

  return status;
}

I was not able to cross-compile or run tools/nxstyle cleanly in my environment against this file (it reports a path-resolution error on Windows/Git-Bash that also occurs on unmodified files, e.g. drivers/spi/spi.c, so it is an environment/tooling limitation unrelated to this change, not a style issue introduced by it). The changed line is 67 columns, within the 80-column limit, and matches surrounding indentation/style.

PR verification Self-Check

• This PR introduces only one functional change.
• I have updated all required description fields above.
• My PR adheres to Contributing Guidelines and Documentation (git commit title and message, coding standard, etc).
• My PR is still work in progress (not ready for review).
• My PR is ready for review and can be safely merged into a codebase.

Disclosure

I used Claude (Anthropic, model claude-sonnet-5, via Claude Code) to help investigate this issue, write the fix, write the reproduction program, and draft this PR description. I (the human author, credited in the Signed-off-by trailer) reviewed the diff, the reasoning, and the reproduction output before submitting. Per this repository's CONTRIBUTING.md §1.5, the commit carries an Assisted-by: Claude Code:claude-sonnet-5 trailer. I do not have physical iCE40/iCE-V-Wireless hardware, so — as stated above in Testing — this fix has not been verified on real hardware, only via an isolated logic reproduction; I'm flagging this explicitly rather than implying hardware verification took place.

[github-actions]

MemBrowse Memory Report

No memory changes detected for:

• arduino-mega2560
• esp32-devkitc
• hifive1-revb
• mirtoo
• qemu-armv8a
• qemu-intel64
• rx65n-rsk2mb
• s698pm-dkit
• stm32-nucleo-f103rb

[acassis]

Hi @janouja since it is your baby, could you please test? to confirm everything is working as expected.

[linguini1]

You should spend some time setting up the build environment so you can test your PRs/apply the style checks. There are some helpful docs here: https://nuttx.apache.org/docs/latest/contributing/index.html