Received-SPF: pass (google.com: domain of linux-kernel+bounces-107417-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <06ab4347-3ed0-432a-cc36-49837d8a28de@quicinc.com>
Date: Tue, 19 Mar 2024 16:18:32 +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>
Content-Language: en-US
From: Priyansh Jain <quic_priyjain@quicinc.com>
In-Reply-To: <CAHLCerModb=01WX=q6XU0XO8dr5EaSQ5RaBoFLFc_=vpOGAgaw@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit


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).

Regards,
Priyansh
>>
>>   struct tsens_plat_data data_tsens_v2 = {
>> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
>> index 6d7c16ccb44d..396c1cd71351 100644
>> --- a/drivers/thermal/qcom/tsens.c
>> +++ b/drivers/thermal/qcom/tsens.c
>> @@ -17,6 +17,7 @@
>>   #include <linux/pm.h>
>>   #include <linux/regmap.h>
>>   #include <linux/slab.h>
>> +#include <linux/suspend.h>
>>   #include <linux/thermal.h>
>>   #include "../thermal_hwmon.h"
>>   #include "tsens.h"
>> @@ -1193,6 +1194,45 @@ static int tsens_register_irq(struct tsens_priv *priv, char *irqname,
>>          return ret;
>>   }
>>
>> +#ifdef CONFIG_SUSPEND
>> +static int tsens_reinit(struct tsens_priv *priv)
>> +{
>> +       unsigned long flags;
>> +
>> +       spin_lock_irqsave(&priv->ul_lock, flags);
>> +
>> +       /* in VER_0 TSENS need to be explicitly enabled */
>> +       if (tsens_version(priv) == VER_0)
>> +               regmap_field_write(priv->rf[TSENS_EN], 1);
>> +
>> +       /*
>> +        * Re-enable the watchdog, unmask the bark.
>> +        * Disable cycle completion monitoring
>> +        */
>> +       if (priv->feat->has_watchdog) {
>> +               regmap_field_write(priv->rf[WDOG_BARK_MASK], 0);
>> +               regmap_field_write(priv->rf[CC_MON_MASK], 1);
>> +       }
>> +
>> +       /* Re-enable interrupts */
>> +       if (tsens_version(priv) >= VER_0_1)
>> +               tsens_enable_irq(priv);
>> +
>> +       spin_unlock_irqrestore(&priv->ul_lock, flags);
>> +
>> +       return 0;
>> +}
>> +
>> +int tsens_resume_common(struct tsens_priv *priv)
>> +{
>> +       if (pm_suspend_target_state == PM_SUSPEND_MEM)
>> +               tsens_reinit(priv);
>> +
>> +       return 0;
>> +}
>> +
>> +#endif /* !CONFIG_SUSPEND */
>> +
>>   static int tsens_register(struct tsens_priv *priv)
>>   {
>>          int i, ret;
>> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
>> index cb637fa289ca..7a147d9d8544 100644
>> --- a/drivers/thermal/qcom/tsens.h
>> +++ b/drivers/thermal/qcom/tsens.h
>> @@ -635,6 +635,12 @@ int init_common(struct tsens_priv *priv);
>>   int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp);
>>   int get_temp_common(const struct tsens_sensor *s, int *temp);
>>
>> +#ifdef CONFIG_SUSPEND
>> +int tsens_resume_common(struct tsens_priv *priv);
>> +#else
>> +#define tsens_resume_common            NULL
>> +#endif
>> +
>>   /* TSENS target */
>>   extern struct tsens_plat_data data_8960;
>>
>> --
>> 2.17.1
>>