linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/4] Fixes for Tegra soctherm
@ 2018-11-05  9:32 Wei Ni
  2018-11-05  9:32 ` [PATCH v1 1/4] thermal: tegra: continue if sensor register fails Wei Ni
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Wei Ni @ 2018-11-05  9:32 UTC (permalink / raw)
  To: thierry.reding, daniel.lezcano, linux-tegra
  Cc: rui.zhang, edubezval, linux-kernel, Wei Ni

This series fixed some issues for Tegra soctherm

Wei Ni (4):
  thermal: tegra: continue if sensor register fails
  thermal: tegra: remove unnecessary warnings
  thermal: tegra: fix memory allocation
  thermal: tegra: fix coverity defect

 drivers/thermal/tegra/soctherm.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v1 1/4] thermal: tegra: continue if sensor register fails
  2018-11-05  9:32 [PATCH v1 0/4] Fixes for Tegra soctherm Wei Ni
@ 2018-11-05  9:32 ` Wei Ni
  2018-11-08 12:48   ` Thierry Reding
  2018-11-05  9:32 ` [PATCH v1 2/4] thermal: tegra: remove unnecessary warnings Wei Ni
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Wei Ni @ 2018-11-05  9:32 UTC (permalink / raw)
  To: thierry.reding, daniel.lezcano, linux-tegra
  Cc: rui.zhang, edubezval, linux-kernel, Wei Ni

Don't bail when a sensor fails to register with the
thermal zone and allow other sensors to register.
This allows other sensors to register with thermal
framework even if one sensor fails registration.

Signed-off-by: Wei Ni <wni@nvidia.com>
---
 drivers/thermal/tegra/soctherm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
index ed28110a3535..b708300303ff 100644
--- a/drivers/thermal/tegra/soctherm.c
+++ b/drivers/thermal/tegra/soctherm.c
@@ -1370,9 +1370,9 @@ static int tegra_soctherm_probe(struct platform_device *pdev)
 							 &tegra_of_thermal_ops);
 		if (IS_ERR(z)) {
 			err = PTR_ERR(z);
-			dev_err(&pdev->dev, "failed to register sensor: %d\n",
+			dev_warn(&pdev->dev, "failed to register sensor: %d\n",
 				err);
-			goto disable_clocks;
+			continue;
 		}
 
 		zone->tz = z;
@@ -1434,6 +1434,8 @@ static int __maybe_unused soctherm_resume(struct device *dev)
 		struct thermal_zone_device *tz;
 
 		tz = tegra->thermctl_tzs[soc->ttgs[i]->id];
+		if (!tz)
+			continue;
 		err = tegra_soctherm_set_hwtrips(dev, soc->ttgs[i], tz);
 		if (err) {
 			dev_err(&pdev->dev,
-- 
2.7.4


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v1 2/4] thermal: tegra: remove unnecessary warnings
  2018-11-05  9:32 [PATCH v1 0/4] Fixes for Tegra soctherm Wei Ni
  2018-11-05  9:32 ` [PATCH v1 1/4] thermal: tegra: continue if sensor register fails Wei Ni
@ 2018-11-05  9:32 ` Wei Ni
  2018-11-08 12:47   ` Thierry Reding
  2018-11-05  9:32 ` [PATCH v1 3/4] thermal: tegra: fix memory allocation Wei Ni
  2018-11-05  9:32 ` [PATCH v1 4/4] thermal: tegra: fix coverity defect Wei Ni
  3 siblings, 1 reply; 11+ messages in thread
From: Wei Ni @ 2018-11-05  9:32 UTC (permalink / raw)
  To: thierry.reding, daniel.lezcano, linux-tegra
  Cc: rui.zhang, edubezval, linux-kernel, Wei Ni

Convert warnings to info as not all platforms may
have all the thresholds and sensors enabled.

Signed-off-by: Wei Ni <wni@nvidia.com>
---
 drivers/thermal/tegra/soctherm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
index b708300303ff..39a8bda07ac4 100644
--- a/drivers/thermal/tegra/soctherm.c
+++ b/drivers/thermal/tegra/soctherm.c
@@ -550,7 +550,7 @@ static int tegra_soctherm_set_hwtrips(struct device *dev,
 
 	ret = tz->ops->get_crit_temp(tz, &temperature);
 	if (ret) {
-		dev_warn(dev, "thermtrip: %s: missing critical temperature\n",
+		dev_info(dev, "thermtrip: %s: missing critical temperature\n",
 			 sg->name);
 		goto set_throttle;
 	}
@@ -569,7 +569,7 @@ static int tegra_soctherm_set_hwtrips(struct device *dev,
 set_throttle:
 	ret = get_hot_temp(tz, &trip, &temperature);
 	if (ret) {
-		dev_warn(dev, "throttrip: %s: missing hot temperature\n",
+		dev_info(dev, "throttrip: %s: missing hot temperature\n",
 			 sg->name);
 		return 0;
 	}
@@ -600,7 +600,7 @@ static int tegra_soctherm_set_hwtrips(struct device *dev,
 	}
 
 	if (i == THROTTLE_SIZE)
-		dev_warn(dev, "throttrip: %s: missing throttle cdev\n",
+		dev_info(dev, "throttrip: %s: missing throttle cdev\n",
 			 sg->name);
 
 	return 0;
-- 
2.7.4


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v1 3/4] thermal: tegra: fix memory allocation
  2018-11-05  9:32 [PATCH v1 0/4] Fixes for Tegra soctherm Wei Ni
  2018-11-05  9:32 ` [PATCH v1 1/4] thermal: tegra: continue if sensor register fails Wei Ni
  2018-11-05  9:32 ` [PATCH v1 2/4] thermal: tegra: remove unnecessary warnings Wei Ni
@ 2018-11-05  9:32 ` Wei Ni
  2018-11-08 12:37   ` Thierry Reding
  2018-11-05  9:32 ` [PATCH v1 4/4] thermal: tegra: fix coverity defect Wei Ni
  3 siblings, 1 reply; 11+ messages in thread
From: Wei Ni @ 2018-11-05  9:32 UTC (permalink / raw)
  To: thierry.reding, daniel.lezcano, linux-tegra
  Cc: rui.zhang, edubezval, linux-kernel, Wei Ni

Fix memory allocation to store the pointers to
thermal_zone_device.

Signed-off-by: Wei Ni <wni@nvidia.com>
---
 drivers/thermal/tegra/soctherm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
index 39a8bda07ac4..3042837364e8 100644
--- a/drivers/thermal/tegra/soctherm.c
+++ b/drivers/thermal/tegra/soctherm.c
@@ -1339,7 +1339,7 @@ static int tegra_soctherm_probe(struct platform_device *pdev)
 	}
 
 	tegra->thermctl_tzs = devm_kcalloc(&pdev->dev,
-					   soc->num_ttgs, sizeof(*z),
+					   soc->num_ttgs, sizeof(z),
 					   GFP_KERNEL);
 	if (!tegra->thermctl_tzs)
 		return -ENOMEM;
-- 
2.7.4


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v1 4/4] thermal: tegra: fix coverity defect
  2018-11-05  9:32 [PATCH v1 0/4] Fixes for Tegra soctherm Wei Ni
                   ` (2 preceding siblings ...)
  2018-11-05  9:32 ` [PATCH v1 3/4] thermal: tegra: fix memory allocation Wei Ni
@ 2018-11-05  9:32 ` Wei Ni
  2018-11-08 12:37   ` Thierry Reding
  3 siblings, 1 reply; 11+ messages in thread
From: Wei Ni @ 2018-11-05  9:32 UTC (permalink / raw)
  To: thierry.reding, daniel.lezcano, linux-tegra
  Cc: rui.zhang, edubezval, linux-kernel, Wei Ni

Fix dereference dev before null check.

Signed-off-by: Wei Ni <wni@nvidia.com>
---
 drivers/thermal/tegra/soctherm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
index 3042837364e8..96527df91f2a 100644
--- a/drivers/thermal/tegra/soctherm.c
+++ b/drivers/thermal/tegra/soctherm.c
@@ -397,7 +397,7 @@ static int throttrip_program(struct device *dev,
 			     struct soctherm_throt_cfg *stc,
 			     int trip_temp)
 {
-	struct tegra_soctherm *ts = dev_get_drvdata(dev);
+	struct tegra_soctherm *ts;
 	int temp, cpu_throt, gpu_throt;
 	unsigned int throt;
 	u32 r, reg_off;
@@ -405,6 +405,8 @@ static int throttrip_program(struct device *dev,
 	if (!sg || !stc || !stc->init)
 		return -EINVAL;
 
+	ts = dev_get_drvdata(dev);
+
 	temp = enforce_temp_range(dev, trip_temp) / ts->soc->thresh_grain;
 
 	/* Hardcode LIGHT on LEVEL1 and HEAVY on LEVEL2 */
-- 
2.7.4


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 4/4] thermal: tegra: fix coverity defect
  2018-11-05  9:32 ` [PATCH v1 4/4] thermal: tegra: fix coverity defect Wei Ni
@ 2018-11-08 12:37   ` Thierry Reding
  2018-11-09  6:44     ` Wei Ni
  0 siblings, 1 reply; 11+ messages in thread
From: Thierry Reding @ 2018-11-08 12:37 UTC (permalink / raw)
  To: Wei Ni; +Cc: daniel.lezcano, linux-tegra, rui.zhang, edubezval, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]

On Mon, Nov 05, 2018 at 05:32:34PM +0800, Wei Ni wrote:
> Fix dereference dev before null check.
> 
> Signed-off-by: Wei Ni <wni@nvidia.com>
> ---
>  drivers/thermal/tegra/soctherm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
> index 3042837364e8..96527df91f2a 100644
> --- a/drivers/thermal/tegra/soctherm.c
> +++ b/drivers/thermal/tegra/soctherm.c
> @@ -397,7 +397,7 @@ static int throttrip_program(struct device *dev,
>  			     struct soctherm_throt_cfg *stc,
>  			     int trip_temp)
>  {
> -	struct tegra_soctherm *ts = dev_get_drvdata(dev);
> +	struct tegra_soctherm *ts;
>  	int temp, cpu_throt, gpu_throt;
>  	unsigned int throt;
>  	u32 r, reg_off;
> @@ -405,6 +405,8 @@ static int throttrip_program(struct device *dev,
>  	if (!sg || !stc || !stc->init)
>  		return -EINVAL;
>  
> +	ts = dev_get_drvdata(dev);

I think coverity is wrong. How is dev ever going to be NULL in this
case? We allocate all of these struct tegra_thermctl_zone structures in
tegra_soctherm_probe() and assign zone->dev = &pdev->dev, which can
never be NULL.

And even if it could, the code would've crashed earlier in
tegra_soctherm_probe() already.

Furthermore, I fail to see how your patch would fix the defect. None of
the checks in the conditional above actually check the dev value.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 3/4] thermal: tegra: fix memory allocation
  2018-11-05  9:32 ` [PATCH v1 3/4] thermal: tegra: fix memory allocation Wei Ni
@ 2018-11-08 12:37   ` Thierry Reding
  0 siblings, 0 replies; 11+ messages in thread
From: Thierry Reding @ 2018-11-08 12:37 UTC (permalink / raw)
  To: Wei Ni; +Cc: daniel.lezcano, linux-tegra, rui.zhang, edubezval, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 327 bytes --]

On Mon, Nov 05, 2018 at 05:32:33PM +0800, Wei Ni wrote:
> Fix memory allocation to store the pointers to
> thermal_zone_device.
> 
> Signed-off-by: Wei Ni <wni@nvidia.com>
> ---
>  drivers/thermal/tegra/soctherm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 2/4] thermal: tegra: remove unnecessary warnings
  2018-11-05  9:32 ` [PATCH v1 2/4] thermal: tegra: remove unnecessary warnings Wei Ni
@ 2018-11-08 12:47   ` Thierry Reding
  2018-11-09  7:21     ` Wei Ni
  0 siblings, 1 reply; 11+ messages in thread
From: Thierry Reding @ 2018-11-08 12:47 UTC (permalink / raw)
  To: Wei Ni; +Cc: daniel.lezcano, linux-tegra, rui.zhang, edubezval, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 599 bytes --]

On Mon, Nov 05, 2018 at 05:32:32PM +0800, Wei Ni wrote:
> Convert warnings to info as not all platforms may
> have all the thresholds and sensors enabled.
> 
> Signed-off-by: Wei Ni <wni@nvidia.com>
> ---
>  drivers/thermal/tegra/soctherm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

This seems overly generalized to me. Shouldn't we be checking in a more
fine-grained way for the absence of thresholds and/or sensors?

Otherwise, how are going to make the difference between the sensor not
being enabled or the device tree just missing the information?

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/4] thermal: tegra: continue if sensor register fails
  2018-11-05  9:32 ` [PATCH v1 1/4] thermal: tegra: continue if sensor register fails Wei Ni
@ 2018-11-08 12:48   ` Thierry Reding
  0 siblings, 0 replies; 11+ messages in thread
From: Thierry Reding @ 2018-11-08 12:48 UTC (permalink / raw)
  To: Wei Ni; +Cc: daniel.lezcano, linux-tegra, rui.zhang, edubezval, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 473 bytes --]

On Mon, Nov 05, 2018 at 05:32:31PM +0800, Wei Ni wrote:
> Don't bail when a sensor fails to register with the
> thermal zone and allow other sensors to register.
> This allows other sensors to register with thermal
> framework even if one sensor fails registration.
> 
> Signed-off-by: Wei Ni <wni@nvidia.com>
> ---
>  drivers/thermal/tegra/soctherm.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 4/4] thermal: tegra: fix coverity defect
  2018-11-08 12:37   ` Thierry Reding
@ 2018-11-09  6:44     ` Wei Ni
  0 siblings, 0 replies; 11+ messages in thread
From: Wei Ni @ 2018-11-09  6:44 UTC (permalink / raw)
  To: Thierry Reding
  Cc: daniel.lezcano, linux-tegra, rui.zhang, edubezval, linux-kernel



On 8/11/2018 8:37 PM, Thierry Reding wrote:
> On Mon, Nov 05, 2018 at 05:32:34PM +0800, Wei Ni wrote:
>> Fix dereference dev before null check.
>>
>> Signed-off-by: Wei Ni <wni@nvidia.com>
>> ---
>>  drivers/thermal/tegra/soctherm.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
>> index 3042837364e8..96527df91f2a 100644
>> --- a/drivers/thermal/tegra/soctherm.c
>> +++ b/drivers/thermal/tegra/soctherm.c
>> @@ -397,7 +397,7 @@ static int throttrip_program(struct device *dev,
>>  			     struct soctherm_throt_cfg *stc,
>>  			     int trip_temp)
>>  {
>> -	struct tegra_soctherm *ts = dev_get_drvdata(dev);
>> +	struct tegra_soctherm *ts;
>>  	int temp, cpu_throt, gpu_throt;
>>  	unsigned int throt;
>>  	u32 r, reg_off;
>> @@ -405,6 +405,8 @@ static int throttrip_program(struct device *dev,
>>  	if (!sg || !stc || !stc->init)
>>  		return -EINVAL;
>>  
>> +	ts = dev_get_drvdata(dev);
> 
> I think coverity is wrong. How is dev ever going to be NULL in this
> case? We allocate all of these struct tegra_thermctl_zone structures in
> tegra_soctherm_probe() and assign zone->dev = &pdev->dev, which can
> never be NULL.
> 
> And even if it could, the code would've crashed earlier in
> tegra_soctherm_probe() already.
> 
> Furthermore, I fail to see how your patch would fix the defect. None of
> the checks in the conditional above actually check the dev value.
> 
Yes, you are right, we doesn't need this change. The driver would not
pass null dev in any case.
And this driver already had a change "1fba81cc09bd thermal: tegra:
remove null check for dev pointer" which remove this "dev" checking.

Thank.
Wei.

> Thierry
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 2/4] thermal: tegra: remove unnecessary warnings
  2018-11-08 12:47   ` Thierry Reding
@ 2018-11-09  7:21     ` Wei Ni
  0 siblings, 0 replies; 11+ messages in thread
From: Wei Ni @ 2018-11-09  7:21 UTC (permalink / raw)
  To: Thierry Reding
  Cc: daniel.lezcano, linux-tegra, rui.zhang, edubezval, linux-kernel



On 8/11/2018 8:47 PM, Thierry Reding wrote:
> On Mon, Nov 05, 2018 at 05:32:32PM +0800, Wei Ni wrote:
>> Convert warnings to info as not all platforms may
>> have all the thresholds and sensors enabled.
>>
>> Signed-off-by: Wei Ni <wni@nvidia.com>
>> ---
>>  drivers/thermal/tegra/soctherm.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> This seems overly generalized to me. Shouldn't we be checking in a more
> fine-grained way for the absence of thresholds and/or sensors?
> 
> Otherwise, how are going to make the difference between the sensor not
> being enabled or the device tree just missing the information?
> 
The sensor being enabled or not is controlled by device tree, if the dts
have the corresponding nodes, then the sensors should be enabled. And
the thresholds for sensor are not necessary, so I think we just need to
print out them.
BTW, in my patch 1/4, I should print out the sensor name if the sensor
not enabled and register failed.
Will update it.

> Thierry
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-11-09  7:21 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-05  9:32 [PATCH v1 0/4] Fixes for Tegra soctherm Wei Ni
2018-11-05  9:32 ` [PATCH v1 1/4] thermal: tegra: continue if sensor register fails Wei Ni
2018-11-08 12:48   ` Thierry Reding
2018-11-05  9:32 ` [PATCH v1 2/4] thermal: tegra: remove unnecessary warnings Wei Ni
2018-11-08 12:47   ` Thierry Reding
2018-11-09  7:21     ` Wei Ni
2018-11-05  9:32 ` [PATCH v1 3/4] thermal: tegra: fix memory allocation Wei Ni
2018-11-08 12:37   ` Thierry Reding
2018-11-05  9:32 ` [PATCH v1 4/4] thermal: tegra: fix coverity defect Wei Ni
2018-11-08 12:37   ` Thierry Reding
2018-11-09  6:44     ` Wei Ni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).