Thanks Konrad for the review.
Sorry, I missed your comments on the v2 patch and ended up posting v3.
I'm happy to post a v4 incorporating your feedback once the discussion
concludes.
On 7/6/2026 5:11 PM, Konrad Dybcio wrote:
On 7/5/26 3:57 PM, Aniket Randive wrote:
The driver uses a static XFER_TIMEOUT of HZ (1 second) for all transfers
regardless of message length or bus frequency, causing unnecessary
delays on error paths.
Compute the timeout dynamically from message length and bus frequency
with a 10x safety margin over the theoretical wire time and a 300ms
floor. For GPI multi-descriptor transfers, use the maximum message
length across all queued messages as the per-completion timeout.
What's the reason for a 0.3 s floor?
The floor accounts for I2C clock stretching. The spec allows slaves to
hold SCL low indefinitely during internal processing. A dynamically
computed xfer time alone gives no time for that.
300ms value covers worst-case stretching while still detecting real
hangs 3x faster than the old 1s static timeout.
Thanks,
Aniket
Why a 10x safety margin specifically?
[...]
The multiplier covers the gap between theoretical xfer time and actual
completion time (DMA descriptor setup, interrupt latency, and scheduling
jitter on a loaded system)
Without it, short transfers would have almost no extra time before a
spurious timeout.
Thanks,
Aniket
+static unsigned long geni_i2c_xfer_timeout(struct geni_i2c_dev *gi2c, size_t
len)
+{
+ size_t bit_cnt = len * 9;
+ size_t bit_usec = (bit_cnt * USEC_PER_SEC) / gi2c->clk_freq_out;
mult_frac()
Konrad
Good catch. I'll switch the calculation to mult_frac() as suggested in
the next v4 patch.
Thanks,
Aniket
