| Message ID | 20240326074033.17002-1-quic_priyjain@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4] thermal/drivers/tsens: Add suspend to RAM support for tsens | expand |
On 26/03/2024 08:40, Priyansh Jain 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> > --- > V3 -> V4: Make tsens_reinit function specific to tsens v2. Add > NULL resume callback support for platform whose versions < ver_2_x > in tsens ops. > 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-v0_1.c | 6 +++++ > drivers/thermal/qcom/tsens-v1.c | 3 +++ > drivers/thermal/qcom/tsens-v2.c | 1 + > drivers/thermal/qcom/tsens.c | 37 +++++++++++++++++++++++++++++++ > drivers/thermal/qcom/tsens.h | 5 +++++ > 5 files changed, 52 insertions(+) > > diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c > index 32d2d3e33287..7ed85379247b 100644 > --- a/drivers/thermal/qcom/tsens-v0_1.c > +++ b/drivers/thermal/qcom/tsens-v0_1.c > @@ -329,6 +329,7 @@ static const struct tsens_ops ops_8226 = { > .init = init_8226, > .calibrate = tsens_calibrate_common, > .get_temp = get_temp_common, > + .resume = NULL, As a static variable it is already set to NULL. Why do you need to explicitly set them everywhere ? [ ... ] > +#ifdef CONFIG_SUSPEND > +static int tsens_reinit(struct tsens_priv *priv) > +{ > + unsigned long flags; > + > + spin_lock_irqsave(&priv->ul_lock, flags); What this lock is protecting ? > + if (tsens_version(priv) >= VER_2_X) { May be move this test before the lock ? > + /* > + * 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 */ > + 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 */
On 3/26/2024 4:30 PM, Daniel Lezcano wrote: > On 26/03/2024 08:40, Priyansh Jain 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> >> --- >> V3 -> V4: Make tsens_reinit function specific to tsens v2. Add >> NULL resume callback support for platform whose versions < ver_2_x >> in tsens ops. >> 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-v0_1.c | 6 +++++ >> drivers/thermal/qcom/tsens-v1.c | 3 +++ >> drivers/thermal/qcom/tsens-v2.c | 1 + >> drivers/thermal/qcom/tsens.c | 37 +++++++++++++++++++++++++++++++ >> drivers/thermal/qcom/tsens.h | 5 +++++ >> 5 files changed, 52 insertions(+) >> >> diff --git a/drivers/thermal/qcom/tsens-v0_1.c >> b/drivers/thermal/qcom/tsens-v0_1.c >> index 32d2d3e33287..7ed85379247b 100644 >> --- a/drivers/thermal/qcom/tsens-v0_1.c >> +++ b/drivers/thermal/qcom/tsens-v0_1.c >> @@ -329,6 +329,7 @@ static const struct tsens_ops ops_8226 = { >> .init = init_8226, >> .calibrate = tsens_calibrate_common, >> .get_temp = get_temp_common, >> + .resume = NULL, > > As a static variable it is already set to NULL. Why do you need to > explicitly set them everywhere ? > It was asked in last version to explicitly add (.resume = NULL). So added this for all the tsens platforms for which resume callback is not validated. > [ ... ] > >> +#ifdef CONFIG_SUSPEND >> +static int tsens_reinit(struct tsens_priv *priv) >> +{ >> + unsigned long flags; >> + >> + spin_lock_irqsave(&priv->ul_lock, flags); > > What this lock is protecting ? > Yes this lock can be removed, will remove this in next patch. >> + if (tsens_version(priv) >= VER_2_X) { > > May be move this test before the lock ? > >> + /* >> + * 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 */ >> + 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 */ > >
On Wed, Mar 27, 2024 at 3:12 PM Priyansh Jain <quic_priyjain@quicinc.com> wrote: > On 3/26/2024 4:30 PM, Daniel Lezcano wrote: > > On 26/03/2024 08:40, Priyansh Jain 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> > >> --- > >> V3 -> V4: Make tsens_reinit function specific to tsens v2. Add > >> NULL resume callback support for platform whose versions < ver_2_x > >> in tsens ops. > >> 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-v0_1.c | 6 +++++ > >> drivers/thermal/qcom/tsens-v1.c | 3 +++ > >> drivers/thermal/qcom/tsens-v2.c | 1 + > >> drivers/thermal/qcom/tsens.c | 37 +++++++++++++++++++++++++++++++ > >> drivers/thermal/qcom/tsens.h | 5 +++++ > >> 5 files changed, 52 insertions(+) > >> > >> diff --git a/drivers/thermal/qcom/tsens-v0_1.c > >> b/drivers/thermal/qcom/tsens-v0_1.c > >> index 32d2d3e33287..7ed85379247b 100644 > >> --- a/drivers/thermal/qcom/tsens-v0_1.c > >> +++ b/drivers/thermal/qcom/tsens-v0_1.c > >> @@ -329,6 +329,7 @@ static const struct tsens_ops ops_8226 = { > >> .init = init_8226, > >> .calibrate = tsens_calibrate_common, > >> .get_temp = get_temp_common, > >> + .resume = NULL, > > > > As a static variable it is already set to NULL. Why do you need to > > explicitly set them everywhere ? > > > It was asked in last version to explicitly add (.resume = NULL). So > added this for all the tsens platforms for which resume callback is not > validated. Daniel's right, you can lose this bit now that your reinit function will only work for 2.x.
diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c index 32d2d3e33287..7ed85379247b 100644 --- a/drivers/thermal/qcom/tsens-v0_1.c +++ b/drivers/thermal/qcom/tsens-v0_1.c @@ -329,6 +329,7 @@ static const struct tsens_ops ops_8226 = { .init = init_8226, .calibrate = tsens_calibrate_common, .get_temp = get_temp_common, + .resume = NULL, }; struct tsens_plat_data data_8226 = { @@ -342,6 +343,7 @@ static const struct tsens_ops ops_8909 = { .init = init_8909, .calibrate = tsens_calibrate_common, .get_temp = get_temp_common, + .resume = NULL, }; struct tsens_plat_data data_8909 = { @@ -355,6 +357,7 @@ static const struct tsens_ops ops_8916 = { .init = init_common, .calibrate = calibrate_8916, .get_temp = get_temp_common, + .resume = NULL, }; struct tsens_plat_data data_8916 = { @@ -370,6 +373,7 @@ static const struct tsens_ops ops_8939 = { .init = init_8939, .calibrate = tsens_calibrate_common, .get_temp = get_temp_common, + .resume = NULL, }; struct tsens_plat_data data_8939 = { @@ -385,6 +389,7 @@ static const struct tsens_ops ops_8974 = { .init = init_common, .calibrate = calibrate_8974, .get_temp = get_temp_common, + .resume = NULL, }; struct tsens_plat_data data_8974 = { @@ -398,6 +403,7 @@ static const struct tsens_ops ops_9607 = { .init = init_9607, .calibrate = tsens_calibrate_common, .get_temp = get_temp_common, + .resume = NULL, }; struct tsens_plat_data data_9607 = { diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c index dc1c4ae2d8b0..770bf0917026 100644 --- a/drivers/thermal/qcom/tsens-v1.c +++ b/drivers/thermal/qcom/tsens-v1.c @@ -154,6 +154,7 @@ static const struct tsens_ops ops_generic_v1 = { .init = init_common, .calibrate = calibrate_v1, .get_temp = get_temp_tsens_valid, + .resume = NULL, }; struct tsens_plat_data data_tsens_v1 = { @@ -166,6 +167,7 @@ static const struct tsens_ops ops_8956 = { .init = init_8956, .calibrate = tsens_calibrate_common, .get_temp = get_temp_tsens_valid, + .resume = NULL, }; struct tsens_plat_data data_8956 = { @@ -179,6 +181,7 @@ static const struct tsens_ops ops_8976 = { .init = init_common, .calibrate = tsens_calibrate_common, .get_temp = get_temp_tsens_valid, + .resume = NULL, }; struct tsens_plat_data data_8976 = { 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..0ff588cb53b2 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,42 @@ 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); + + if (tsens_version(priv) >= VER_2_X) { + /* + * 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 */ + 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..cab39de045b1 100644 --- a/drivers/thermal/qcom/tsens.h +++ b/drivers/thermal/qcom/tsens.h @@ -634,6 +634,11 @@ void compute_intercept_slope(struct tsens_priv *priv, u32 *pt1, u32 *pt2, u32 mo 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> --- V3 -> V4: Make tsens_reinit function specific to tsens v2. Add NULL resume callback support for platform whose versions < ver_2_x in tsens ops. 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-v0_1.c | 6 +++++ drivers/thermal/qcom/tsens-v1.c | 3 +++ drivers/thermal/qcom/tsens-v2.c | 1 + drivers/thermal/qcom/tsens.c | 37 +++++++++++++++++++++++++++++++ drivers/thermal/qcom/tsens.h | 5 +++++ 5 files changed, 52 insertions(+)