Re: [PATCH 1/2] thermal: qcom: tsens: atomic temperature read with hardware-guided retries

From: Daniel Lezcano

Date: Tue May 05 2026 - 05:39:05 EST


On 5/5/26 10:48, Priyansh Jain wrote:

[ ... ]


int prev = INTMAX;

/*
  * An explanation ...
  */

for (i = 0; i < max_retry; i++) {

     int value, valid;

     ret = regmap_field_read(priv->rf[field], &status);
     if (ret)
         return ret;

     value = FIELD_GET(priv->feat->last_temp_mask, status);

     valid = FIELD_GET(priv->feat->valid_bit, status)
     if (valid)
         return value;

     if (value == prev)
         return value;

     prev = value;
}

return -EAGAIN;

(Not tested)
This approach has some misalignment with the HW recommendations.
As per the HW guidelines, 3 back‑to‑back reads must be performed until a valid read is observed.
b or c should be returned only if none of the three reads(a,b,c) report the valid bit not set.

Right I missed the point the HW recommendations is to read 3 times in any case. Maybe replace if (value == prev) continue; ?

We need to store all three readings because, if all of them are invalid, we must compare the first, second, and third reads using the following logic:

if a == b, return b
else if b == c, return c
else return -EAGAIN

Given this requirement, comparing (value == prev) inside the read loop would not be correct, as it does not preserve all three samples for the final comparison.

I tried the different combinations and comparing inside the loop should work. But the optimization introduces an implicit inference not helping for the clarity of the code and probably prone to errors in case of changes. So probably simpler to keep your approach. Please add a comment above the if a == b return b else ...

Thanks

-- Daniel