Received-SPF: pass (google.com: domain of linux-kernel+bounces-118460-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <b0e7ce76-486a-727a-4ef4-d08f38be8408@quicinc.com>
Date: Tue, 26 Mar 2024 12:59:54 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.1
Subject: Re: [PATCH v3] thermal/drivers/tsens: Add suspend to RAM support for
 tsens
To: Amit Kucheria <amitk@kernel.org>
CC: Thara Gopinath <thara.gopinath@gmail.com>,
        Bjorn Andersson
	<andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        "Rafael J .
 Wysocki" <rafael@kernel.org>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Zhang Rui <rui.zhang@intel.com>, Lukasz Luba <lukasz.luba@arm.com>,
        <linux-pm@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <quic_manafm@quicinc.com>
References: <20240227160928.2671-1-quic_priyjain@quicinc.com>
 <CAHLCerModb=01WX=q6XU0XO8dr5EaSQ5RaBoFLFc_=vpOGAgaw@mail.gmail.com>
 <06ab4347-3ed0-432a-cc36-49837d8a28de@quicinc.com>
 <CAHLCerPnyT56WukNqX6_4gM7siczYBpSsb7XM_eW=vb5dBwceA@mail.gmail.com>
Content-Language: en-US
From: Priyansh Jain <quic_priyjain@quicinc.com>
In-Reply-To: <CAHLCerPnyT56WukNqX6_4gM7siczYBpSsb7XM_eW=vb5dBwceA@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit


On 3/21/2024 6:21 PM, Amit Kucheria wrote:
> On Tue, Mar 19, 2024 at 4:19 PM Priyansh Jain <quic_priyjain@quicinc.com> wrote:
>>
>>
>>
>> On 3/17/2024 1:37 AM, Amit Kucheria wrote:
>>> On Tue, Feb 27, 2024 at 9:40 PM Priyansh Jain <quic_priyjain@quicinc.com> wrote:
>>>>
>>>> As part of suspend to RAM, tsens hardware will be turned off.
>>>> While resume callback, re-initialize tsens hardware.
>>>>
>>>> Signed-off-by: Priyansh Jain <quic_priyjain@quicinc.com>
>>>> ---
>>>> V2 -> V3: Remove suspend callback & interrupt enablement part from
>>>> resume callback.
>>>> V1 -> V2: Update commit text to explain the necessity of this patch
>>>>
>>>>    drivers/thermal/qcom/tsens-v2.c |  1 +
>>>>    drivers/thermal/qcom/tsens.c    | 40 +++++++++++++++++++++++++++++++++
>>>>    drivers/thermal/qcom/tsens.h    |  6 +++++
>>>>    3 files changed, 47 insertions(+)
>>>>
>>>> diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
>>>> index 29a61d2d6ca3..0cb7301eca6e 100644
>>>> --- a/drivers/thermal/qcom/tsens-v2.c
>>>> +++ b/drivers/thermal/qcom/tsens-v2.c
>>>> @@ -107,6 +107,7 @@ static const struct reg_field tsens_v2_regfields[MAX_REGFIELDS] = {
>>>>    static const struct tsens_ops ops_generic_v2 = {
>>>>           .init           = init_common,
>>>>           .get_temp       = get_temp_tsens_valid,
>>>> +       .resume         = tsens_resume_common,
>>>>    };
>>>
>>> Please add resume callbacks for the other tsens hardware too and make
>>> sure that your reinit function handles them too.
>>>
>> We have discussed internally on this and we think that if someone wants
>> to extend the support (and do the validation) of one of those old
>> platforms they can add the resume ops for that platform. There are many
>> versions of tsens hardware so we are bit skeptical to add reinit support
>> for all these platforms with any validations(since S2R mode is not
>> enabled for all these older platforms so it is not possible to validate).
> 
> Then why does tsens_reinit refer to tsens_version(priv) >= VER_0_1
> when re-enabling the irq?
> 
> Perhaps we should explicitly disable platforms that are not validated
> for this functionality (.resume = NULL) and have the reinit function
> only work for validated platforms (tsens_version >= VER_2_X)?
> 
Sure i will update tsens_reinit to handle only tsens_version >= VER_2_X 
in next patch. Also will add (.resume = NULL ) for all the platforms for 
which this functionality is not validated in next patch.

Regards,
Priyansh

> Regards,
> Amit