| Message ID | 20220701145815.2037993-3-bhupesh.sharma@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for tsens controller reinit via trustzone | expand |
On Fri 01 Jul 09:58 CDT 2022, Bhupesh Sharma wrote: > QCoM sm8150 tsens controller might require re-initialization Please spell out Qualcomm. > via trustzone [via scm call(s)] when it enters a 'bad state' > causing sensor temperatures/interrupts status to be in an > 'invalid' state. > > Add hooks for the same in the qcom tsens driver which > can be used by followup patch(es). > This patch enables needs_reinit_wa, which is actually implemented in patch 3, wouldn't it make more sense to flip them around; to first implement the feature and then enable it in this patch? > Cc: Amit Kucheria <amitk@kernel.org> > Cc: Thara Gopinath <thara.gopinath@gmail.com> > Cc: linux-pm@vger.kernel.org > Cc: linux-arm-msm@vger.kernel.org > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > drivers/thermal/qcom/tsens-v2.c | 11 +++++++++++ > drivers/thermal/qcom/tsens.c | 4 ++++ > drivers/thermal/qcom/tsens.h | 6 +++++- > 3 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c > index b293ed32174b..61d38a56d29a 100644 > --- a/drivers/thermal/qcom/tsens-v2.c > +++ b/drivers/thermal/qcom/tsens-v2.c > @@ -101,6 +101,17 @@ struct tsens_plat_data data_tsens_v2 = { > .fields = tsens_v2_regfields, > }; > > +/* For sm8150 tsens, its suggested to monitor the controller health /* * Outside the network stack, the first line should be left empty in * multiline comments. */ > + * periodically and in case an issue is detected to reinit tsens > + * controller via trustzone. > + */ > +struct tsens_plat_data data_tsens_sm8150 = { I doubt this is sm8150-specific, so the first question is if this should be attempted on all data_tsens_v2 platforms. Otherwise, how about naming this data_tsens_v2_reinit? Regards, Bjorn > + .ops = &ops_generic_v2, > + .feat = &tsens_v2_feat, > + .needs_reinit_wa = true, > + .fields = tsens_v2_regfields, > +}; > + > /* Kept around for backward compatibility with old msm8996.dtsi */ > struct tsens_plat_data data_8996 = { > .num_sensors = 13, > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c > index 7963ee33bf75..97f4d4454f20 100644 > --- a/drivers/thermal/qcom/tsens.c > +++ b/drivers/thermal/qcom/tsens.c > @@ -991,6 +991,9 @@ static const struct of_device_id tsens_table[] = { > }, { > .compatible = "qcom,msm8996-tsens", > .data = &data_8996, > + }, { > + .compatible = "qcom,sm8150-tsens", > + .data = &data_tsens_sm8150, > }, { > .compatible = "qcom,tsens-v1", > .data = &data_tsens_v1, > @@ -1135,6 +1138,7 @@ static int tsens_probe(struct platform_device *pdev) > > priv->dev = dev; > priv->num_sensors = num_sensors; > + priv->needs_reinit_wa = data->needs_reinit_wa; > priv->ops = data->ops; > for (i = 0; i < priv->num_sensors; i++) { > if (data->hw_ids) > diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h > index 1471a2c00f15..48a7bda902c1 100644 > --- a/drivers/thermal/qcom/tsens.h > +++ b/drivers/thermal/qcom/tsens.h > @@ -515,6 +515,7 @@ struct tsens_features { > * @num_sensors: Number of sensors supported by platform > * @ops: operations the tsens instance supports > * @hw_ids: Subset of sensors ids supported by platform, if not the first n > + * @needs_reinit_wa: tsens controller might need reinit via trustzone > * @feat: features of the IP > * @fields: bitfield locations > */ > @@ -522,6 +523,7 @@ struct tsens_plat_data { > const u32 num_sensors; > const struct tsens_ops *ops; > unsigned int *hw_ids; > + bool needs_reinit_wa; > struct tsens_features *feat; > const struct reg_field *fields; > }; > @@ -544,6 +546,7 @@ struct tsens_context { > * @srot_map: pointer to SROT register address space > * @tm_offset: deal with old device trees that don't address TM and SROT > * address space separately > + * @needs_reinit_wa: tsens controller might need reinit via trustzone > * @ul_lock: lock while processing upper/lower threshold interrupts > * @crit_lock: lock while processing critical threshold interrupts > * @rf: array of regmap_fields used to store value of the field > @@ -561,6 +564,7 @@ struct tsens_priv { > struct regmap *tm_map; > struct regmap *srot_map; > u32 tm_offset; > + bool needs_reinit_wa; > > /* lock for upper/lower threshold interrupts */ > spinlock_t ul_lock; > @@ -593,6 +597,6 @@ extern struct tsens_plat_data data_8916, data_8939, data_8974, data_9607; > extern struct tsens_plat_data data_tsens_v1, data_8976; > > /* TSENS v2 targets */ > -extern struct tsens_plat_data data_8996, data_tsens_v2; > +extern struct tsens_plat_data data_8996, data_tsens_sm8150, data_tsens_v2; > > #endif /* __QCOM_TSENS_H__ */ > -- > 2.35.3 >
Hi Bjorn, Thanks for your review. On 7/19/22 8:25 AM, Bjorn Andersson wrote: > On Fri 01 Jul 09:58 CDT 2022, Bhupesh Sharma wrote: > >> QCoM sm8150 tsens controller might require re-initialization > > Please spell out Qualcomm. Ok. >> via trustzone [via scm call(s)] when it enters a 'bad state' >> causing sensor temperatures/interrupts status to be in an >> 'invalid' state. >> >> Add hooks for the same in the qcom tsens driver which >> can be used by followup patch(es). >> > > This patch enables needs_reinit_wa, which is actually implemented in > patch 3, wouldn't it make more sense to flip them around; to first > implement the feature and then enable it in this patch? Yes, this was reported by the kernel test bot as well. I will address this in v2. >> Cc: Amit Kucheria <amitk@kernel.org> >> Cc: Thara Gopinath <thara.gopinath@gmail.com> >> Cc: linux-pm@vger.kernel.org >> Cc: linux-arm-msm@vger.kernel.org >> Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> >> --- >> drivers/thermal/qcom/tsens-v2.c | 11 +++++++++++ >> drivers/thermal/qcom/tsens.c | 4 ++++ >> drivers/thermal/qcom/tsens.h | 6 +++++- >> 3 files changed, 20 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c >> index b293ed32174b..61d38a56d29a 100644 >> --- a/drivers/thermal/qcom/tsens-v2.c >> +++ b/drivers/thermal/qcom/tsens-v2.c >> @@ -101,6 +101,17 @@ struct tsens_plat_data data_tsens_v2 = { >> .fields = tsens_v2_regfields, >> }; >> >> +/* For sm8150 tsens, its suggested to monitor the controller health > > /* > * Outside the network stack, the first line should be left empty in > * multiline comments. > */ Ok. >> + * periodically and in case an issue is detected to reinit tsens >> + * controller via trustzone. >> + */ >> +struct tsens_plat_data data_tsens_sm8150 = { > > I doubt this is sm8150-specific, so the first question is if this should > be attempted on all data_tsens_v2 platforms. Otherwise, how about naming > this data_tsens_v2_reinit? I agree. Konrad reported one more use case of the same. So, I will take care of the same in v2. 'data_tsens_v2_reinit' makes sense to me. Regards, Bhupesh >> + .ops = &ops_generic_v2, >> + .feat = &tsens_v2_feat, >> + .needs_reinit_wa = true, >> + .fields = tsens_v2_regfields, >> +}; >> + >> /* Kept around for backward compatibility with old msm8996.dtsi */ >> struct tsens_plat_data data_8996 = { >> .num_sensors = 13, >> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c >> index 7963ee33bf75..97f4d4454f20 100644 >> --- a/drivers/thermal/qcom/tsens.c >> +++ b/drivers/thermal/qcom/tsens.c >> @@ -991,6 +991,9 @@ static const struct of_device_id tsens_table[] = { >> }, { >> .compatible = "qcom,msm8996-tsens", >> .data = &data_8996, >> + }, { >> + .compatible = "qcom,sm8150-tsens", >> + .data = &data_tsens_sm8150, >> }, { >> .compatible = "qcom,tsens-v1", >> .data = &data_tsens_v1, >> @@ -1135,6 +1138,7 @@ static int tsens_probe(struct platform_device *pdev) >> >> priv->dev = dev; >> priv->num_sensors = num_sensors; >> + priv->needs_reinit_wa = data->needs_reinit_wa; >> priv->ops = data->ops; >> for (i = 0; i < priv->num_sensors; i++) { >> if (data->hw_ids) >> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h >> index 1471a2c00f15..48a7bda902c1 100644 >> --- a/drivers/thermal/qcom/tsens.h >> +++ b/drivers/thermal/qcom/tsens.h >> @@ -515,6 +515,7 @@ struct tsens_features { >> * @num_sensors: Number of sensors supported by platform >> * @ops: operations the tsens instance supports >> * @hw_ids: Subset of sensors ids supported by platform, if not the first n >> + * @needs_reinit_wa: tsens controller might need reinit via trustzone >> * @feat: features of the IP >> * @fields: bitfield locations >> */ >> @@ -522,6 +523,7 @@ struct tsens_plat_data { >> const u32 num_sensors; >> const struct tsens_ops *ops; >> unsigned int *hw_ids; >> + bool needs_reinit_wa; >> struct tsens_features *feat; >> const struct reg_field *fields; >> }; >> @@ -544,6 +546,7 @@ struct tsens_context { >> * @srot_map: pointer to SROT register address space >> * @tm_offset: deal with old device trees that don't address TM and SROT >> * address space separately >> + * @needs_reinit_wa: tsens controller might need reinit via trustzone >> * @ul_lock: lock while processing upper/lower threshold interrupts >> * @crit_lock: lock while processing critical threshold interrupts >> * @rf: array of regmap_fields used to store value of the field >> @@ -561,6 +564,7 @@ struct tsens_priv { >> struct regmap *tm_map; >> struct regmap *srot_map; >> u32 tm_offset; >> + bool needs_reinit_wa; >> >> /* lock for upper/lower threshold interrupts */ >> spinlock_t ul_lock; >> @@ -593,6 +597,6 @@ extern struct tsens_plat_data data_8916, data_8939, data_8974, data_9607; >> extern struct tsens_plat_data data_tsens_v1, data_8976; >> >> /* TSENS v2 targets */ >> -extern struct tsens_plat_data data_8996, data_tsens_v2; >> +extern struct tsens_plat_data data_8996, data_tsens_sm8150, data_tsens_v2; >> >> #endif /* __QCOM_TSENS_H__ */ >> -- >> 2.35.3 >>
diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c index b293ed32174b..61d38a56d29a 100644 --- a/drivers/thermal/qcom/tsens-v2.c +++ b/drivers/thermal/qcom/tsens-v2.c @@ -101,6 +101,17 @@ struct tsens_plat_data data_tsens_v2 = { .fields = tsens_v2_regfields, }; +/* For sm8150 tsens, its suggested to monitor the controller health + * periodically and in case an issue is detected to reinit tsens + * controller via trustzone. + */ +struct tsens_plat_data data_tsens_sm8150 = { + .ops = &ops_generic_v2, + .feat = &tsens_v2_feat, + .needs_reinit_wa = true, + .fields = tsens_v2_regfields, +}; + /* Kept around for backward compatibility with old msm8996.dtsi */ struct tsens_plat_data data_8996 = { .num_sensors = 13, diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c index 7963ee33bf75..97f4d4454f20 100644 --- a/drivers/thermal/qcom/tsens.c +++ b/drivers/thermal/qcom/tsens.c @@ -991,6 +991,9 @@ static const struct of_device_id tsens_table[] = { }, { .compatible = "qcom,msm8996-tsens", .data = &data_8996, + }, { + .compatible = "qcom,sm8150-tsens", + .data = &data_tsens_sm8150, }, { .compatible = "qcom,tsens-v1", .data = &data_tsens_v1, @@ -1135,6 +1138,7 @@ static int tsens_probe(struct platform_device *pdev) priv->dev = dev; priv->num_sensors = num_sensors; + priv->needs_reinit_wa = data->needs_reinit_wa; priv->ops = data->ops; for (i = 0; i < priv->num_sensors; i++) { if (data->hw_ids) diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h index 1471a2c00f15..48a7bda902c1 100644 --- a/drivers/thermal/qcom/tsens.h +++ b/drivers/thermal/qcom/tsens.h @@ -515,6 +515,7 @@ struct tsens_features { * @num_sensors: Number of sensors supported by platform * @ops: operations the tsens instance supports * @hw_ids: Subset of sensors ids supported by platform, if not the first n + * @needs_reinit_wa: tsens controller might need reinit via trustzone * @feat: features of the IP * @fields: bitfield locations */ @@ -522,6 +523,7 @@ struct tsens_plat_data { const u32 num_sensors; const struct tsens_ops *ops; unsigned int *hw_ids; + bool needs_reinit_wa; struct tsens_features *feat; const struct reg_field *fields; }; @@ -544,6 +546,7 @@ struct tsens_context { * @srot_map: pointer to SROT register address space * @tm_offset: deal with old device trees that don't address TM and SROT * address space separately + * @needs_reinit_wa: tsens controller might need reinit via trustzone * @ul_lock: lock while processing upper/lower threshold interrupts * @crit_lock: lock while processing critical threshold interrupts * @rf: array of regmap_fields used to store value of the field @@ -561,6 +564,7 @@ struct tsens_priv { struct regmap *tm_map; struct regmap *srot_map; u32 tm_offset; + bool needs_reinit_wa; /* lock for upper/lower threshold interrupts */ spinlock_t ul_lock; @@ -593,6 +597,6 @@ extern struct tsens_plat_data data_8916, data_8939, data_8974, data_9607; extern struct tsens_plat_data data_tsens_v1, data_8976; /* TSENS v2 targets */ -extern struct tsens_plat_data data_8996, data_tsens_v2; +extern struct tsens_plat_data data_8996, data_tsens_sm8150, data_tsens_v2; #endif /* __QCOM_TSENS_H__ */
QCoM sm8150 tsens controller might require re-initialization via trustzone [via scm call(s)] when it enters a 'bad state' causing sensor temperatures/interrupts status to be in an 'invalid' state. Add hooks for the same in the qcom tsens driver which can be used by followup patch(es). Cc: Amit Kucheria <amitk@kernel.org> Cc: Thara Gopinath <thara.gopinath@gmail.com> Cc: linux-pm@vger.kernel.org Cc: linux-arm-msm@vger.kernel.org Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> --- drivers/thermal/qcom/tsens-v2.c | 11 +++++++++++ drivers/thermal/qcom/tsens.c | 4 ++++ drivers/thermal/qcom/tsens.h | 6 +++++- 3 files changed, 20 insertions(+), 1 deletion(-)