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 <[email protected]>
Cc: Thara Gopinath <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Bhupesh Sharma <[email protected]>
---
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
+ * 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__ */
--
2.35.3
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 <[email protected]>
> Cc: Thara Gopinath <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> 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 <[email protected]>
>> Cc: Thara Gopinath <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Bhupesh Sharma <[email protected]>
>> ---
>> 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
>>