public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Priyansh Jain <priyansh.jain@oss.qualcomm.com>
To: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>,
	Amit Kucheria <amitk@kernel.org>,
	Thara Gopinath <thara.gopinath@gmail.com>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	Daniel Lezcano <daniel.lezcano@kernel.org>,
	Zhang Rui <rui.zhang@intel.com>,
	Lukasz Luba <lukasz.luba@arm.com>
Cc: linux-pm@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-kernel@vger.kernel.org, manaf.pallikunhi@oss.qualcomm.com
Subject: Re: [PATCH 1/2] thermal: qcom: tsens: atomic temperature read with hardware-guided retries
Date: Tue, 5 May 2026 15:09:08 +0530	[thread overview]
Message-ID: <c07fd488-a455-413f-b25f-9f9f1afda097@oss.qualcomm.com> (raw)
In-Reply-To: <b4972eaa-cfea-4fed-990d-2cd34177d045@oss.qualcomm.com>



On 05-05-2026 03:05 pm, Daniel Lezcano wrote:
> 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

Thanks , will go ahead with my approach and will add a comment before 
comparison code .

Thanks,
Priyansh

> 
>    -- Daniel


  reply	other threads:[~2026-05-05  9:39 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-30  5:44 [PATCH 0/2] thermal: qcom: tsens: fix temperature handling Priyansh Jain
2026-04-30  5:44 ` [PATCH 1/2] thermal: qcom: tsens: atomic temperature read with hardware-guided retries Priyansh Jain
2026-04-30 15:51   ` Konrad Dybcio
     [not found]     ` <10c07347-a0df-42d3-b216-5150817b9ed2@oss.qualcomm.com>
2026-05-04  9:59       ` Konrad Dybcio
2026-05-04 10:34         ` Priyansh Jain
2026-04-30 16:00   ` Konrad Dybcio
     [not found]     ` <fc027ab4-695b-4622-b30e-8a79ce6e1781@oss.qualcomm.com>
2026-05-04  9:46       ` Konrad Dybcio
2026-05-04 17:29   ` Daniel Lezcano
2026-05-05  6:11     ` Priyansh Jain
2026-05-05  7:43       ` Daniel Lezcano
2026-05-05  8:48         ` Priyansh Jain
2026-05-05  9:35           ` Daniel Lezcano
2026-05-05  9:39             ` Priyansh Jain [this message]
2026-04-30  5:44 ` [PATCH 2/2] thermal: qcom: tsens: widen temperature limits to match hardware range Priyansh Jain
2026-04-30 16:01   ` Konrad Dybcio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c07fd488-a455-413f-b25f-9f9f1afda097@oss.qualcomm.com \
    --to=priyansh.jain@oss.qualcomm.com \
    --cc=amitk@kernel.org \
    --cc=daniel.lezcano@kernel.org \
    --cc=daniel.lezcano@oss.qualcomm.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=lukasz.luba@arm.com \
    --cc=manaf.pallikunhi@oss.qualcomm.com \
    --cc=rafael@kernel.org \
    --cc=rui.zhang@intel.com \
    --cc=thara.gopinath@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox