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
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 <[email protected]>
---
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
Fix dereference dev before null check.
Signed-off-by: Wei Ni <[email protected]>
---
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
Fix memory allocation to store the pointers to
thermal_zone_device.
Signed-off-by: Wei Ni <[email protected]>
---
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
Convert warnings to info as not all platforms may
have all the thresholds and sensors enabled.
Signed-off-by: Wei Ni <[email protected]>
---
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
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 <[email protected]>
> ---
> drivers/thermal/tegra/soctherm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Thierry Reding <[email protected]>
On Mon, Nov 05, 2018 at 05:32:34PM +0800, Wei Ni wrote:
> Fix dereference dev before null check.
>
> Signed-off-by: Wei Ni <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> 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
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 <[email protected]>
> ---
> drivers/thermal/tegra/soctherm.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
Acked-by: Thierry Reding <[email protected]>
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 <[email protected]>
>> ---
>> 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
>
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 <[email protected]>
>> ---
>> 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
>