diff mbox series

[2/3] thermal: qcom: tsens: Add support for 'needs_reinit_wa' for sm8150

Message ID 20220701145815.2037993-3-bhupesh.sharma@linaro.org
State New
Headers show
Series Add support for tsens controller reinit via trustzone | expand

Commit Message

Bhupesh Sharma July 1, 2022, 2:58 p.m. UTC
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(-)

Comments

Bjorn Andersson July 19, 2022, 2:55 a.m. UTC | #1
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
>
Bhupesh Sharma July 20, 2022, 8:09 a.m. UTC | #2
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 mbox series

Patch

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__ */