| Message ID | 20240227160928.2671-1-quic_priyjain@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Daniel Lezcano |
| Headers | show |
| Series | [v3] thermal/drivers/tsens: Add suspend to RAM support for tsens | expand |
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 >
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 >>
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
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 --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;
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(+)