risc-v/espressif: Fix I2C polling wait timeout comparison.

[Aurora-QIU0]

Summary

clock_t is unsigned unless CONFIG_SYSTEM_TIME64 is set (see sys/types.h). The
comparison in esp_i2c_polling_waitdone():

while (current - timeout < 0 && priv->error == 0)

underflows to a large positive value, so the condition is always false and the
loop body never runs. status keeps its initial value of zero and the function
returns OK without ever waiting for the transfer.

Under CONFIG_I2C_POLLED every transfer therefore reports completion immediately:
no timeout is raised and register reads return whatever the RX FIFO happens to
contain. This is a silent "false success" - no error is reported.

Cast the difference to int32_t to restore the intended signed comparison. The
result stays correct across counter wrap since the timeout is much shorter than
the counter range.

Impact

• New feature? NO
• Impact on user? NO (only corrects polling wait timeout)
• Impact on build? NO
• Impact on hardware? YES (risc-v/espressif I2C, polling mode)
• Impact on documentation? NO
• Impact on security? NO
• Impact on compatibility? NO

Testing

Validated on real hardware during an ESP32-P4 board bring-up. Before the fix,
I2C reads returned garbage (0x2f) with ret=0 - a mathematically impossible
combination (status==0 yet success). After the fix, the GT911 touch controller
enumerated correctly and register reads returned real values.

[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

[Aurora-QIU0]

The issues reported by the check job are fixed in b6c06cf.

Because this PR touches esp_i2c.c, the pre-existing nxstyle violations in that
file were addressed as well, per CONTRIBUTING.md section 2.1 ("adapt all modified
files even if you did not introduce the problem yourself"):

• 1267 - statement over-indented inside its enclosing block (8 spaces where the block body is at 6)
• 1303, 1592 - missing blank line after declarations
• 1710-1725 - case/default labels inside switch (port) were aligned with the brace opening the switch body instead of one level further in

Verified locally with tools/nxstyle.c (gcc 13.3, Ubuntu 24.04): before the change
the file reports 14 violations; afterwards
nxstyle arch/risc-v/src/common/espressif/esp_i2c.c exits 0 with no output.
All other ESP32-P4 / risc-v espressif files were left untouched.

The workflow runs for the new commit show action_required — would you mind
re-approving them when convenient?

[Aurora-QIU0]

Testing update 鈥?build and runtime logs (ESP32-P4, real hardware)

Board: ESP32-P4 Function EV Board (esp32p4-function-ev-board), ESP32-P4 eco7.
Built and exercised on hardware during a board bring-up. This addresses the
build/runtime log requirement in CONTRIBUTING.md section 1.7.2.

Build

ninja: Building C object .../espressif/esp_i2c.c.o
       -> libarch.a -> nuttx -> nuttx.bin
nuttx.bin   797756 bytes   md5 0c4ac4e6c0df1ed695f9911a30e39c97
flash: "Wrote 797756 bytes" + "Hash of data verified" (rc=0)

Runtime before the change (polling mode, GT911 at 0x5d, I2C0)

Serial capture after reset 鈥?the I2C devices never come up and nsh is never
reached:

Failed to initialize ES8311: -19
Failed to initialize I2C driver: -12
ERROR: touch_register() failed: -12
ERROR: Failed to initialize GT911 touch: -12

Instrumented probe inside esp_i2c_polling_waitdone(), the decisive line:

I2CP[5] waitdone ret=0 err=0 laststatus=0x00000000

status == 0 together with ret == 0 is self-contradictory: had the loop run
with status stuck at zero it could only have timed out after SEC2TICK(10)
and returned -ETIMEDOUT. The only reading of the observation that is
internally consistent is that the loop body never ran at all 鈥?which is exactly
what the unsigned underflow in the comparison produces.

Runtime after the change (same board, same bus)

CHECK1 0x8140 raw = [39 31 31 00]         -> PASS   ("911\0", GT911 product id)
CHECK2 0x8047 raw = [59 00 04 58 02 05]   -> PASS   (real config block, not garbage)
GT911 touchscreen registered at /dev/input0 (polling mode)
nsh prompt reached; 0 panic / 0 assert / 0 ERROR

The read of 0x8140 returning the GT911 product id is only possible if the
polling wait actually waited for the transfer, i.e. the loop now runs.

[acassis]

@xiaoxiang781216 @raiden00pl the NTFC is failing for all PR:

Maybe we should convert NTFC errors to warning to avoid breaking the CI

[raiden00pl]

@acassis but that is exactly why NTFC is for: detecting regressions in code. You have a clear reason why it's failing: a crash in ostest.

[acassis]

@acassis but that is exactly why NTFC is for: detecting regressions in code. You have a clear reason why it's failing: a crash in ostest.

Yes, I was just afraid that it could start producing more false positive and make our CI worst.

I just merged apache/nuttx-apps#3790 let's see if it will fix everything. Now everybody needs to rebase

[xiaoxiang781216]

@acassis but that is exactly why NTFC is for: detecting regressions in code. You have a clear reason why it's failing: a crash in ostest.

@raiden00pl but why NTFC can't block the patch which introduce the break: #20112?

[raiden00pl]

@xiaoxiang781216 because #20112 was not a breaking change. Breaking change was in apache/nuttx-apps#3759
CI passed for apps 3759 before #20112 was merged, and apps 3759 was merged after #20112 was merged. It was just a coincidence that two not compatible PRs were introduced.

[acassis]

@xiaoxiang781216 @raiden00pl I think any change on sched/ needs to be well tested, because it affects all arch/boards and all applications. We are seeing many patches coming where the test is not properly done. The testings when modifying sched/ should include many real boards.

[raiden00pl]

@acassis but that’s not the problem in this case. Here, the issue is from the separation of apps and nuttx. There’s nothing we can do about it given the limited CI budget. Ideally, every CI test should be rerun with every update to the master branch: master update to nuttx retrigger apps CI, master update to apps retrigger nuttx CI

[acassis]

@acassis but that’s not the problem in this case. Here, the issue is from the separation of apps and nuttx. There’s nothing we can do about it given the limited CI budget. Ideally, every CI test should be rerun with every update to the master branch: master update to nuttx retrigger apps CI, master update to apps retrigger nuttx CI

Understood. I think at least the "Restart Job" button on CI interface should automatically update the master to avoid asking the contributor to do a rebase to let the CI update to the latest nuttx and apps

[xiaoxiang781216]

@xiaoxiang781216 @raiden00pl I think any change on sched/ needs to be well tested, because it affects all arch/boards and all applications. We are seeing many patches coming where the test is not properly done. The testings when modifying sched/ should include many real boards.

The better direction is improving ntfc and nxdart to run the test case on real hardware distributed. Requesting contributor to do the testing with many real boards will decrease the community activity.