diff mbox series

[v3] thermal/drivers/tsens: Add suspend to RAM support for tsens

Message ID 20240227160928.2671-1-quic_priyjain@quicinc.com (mailing list archive)
State Superseded
Headers show
Series [v3] thermal/drivers/tsens: Add suspend to RAM support for tsens | expand

Commit Message

Priyansh Jain Feb. 27, 2024, 4:09 p.m. UTC
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(+)

Comments

Amit Kucheria March 16, 2024, 8:07 p.m. UTC | #1
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.

>
>  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
>
Priyansh Jain March 19, 2024, 10:48 a.m. UTC | #2
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
>>
Amit Kucheria March 21, 2024, 12:51 p.m. UTC | #3
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)?

Regards,
Amit
Priyansh Jain March 26, 2024, 7:29 a.m. UTC | #4
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
diff mbox series

Patch

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,
 };
 
 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;