linux-kernel - Re: [PATCH] thermal/drivers/hisi: Remove confusing error message

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 8 Aug 2017 21:29:55 +0800
From:   Leo Yan <leo.yan@...aro.org>
To:     Zhang Rui <rui.zhang@...el.com>
Cc:     Daniel Lezcano <daniel.lezcano@...aro.org>, edubezval@...il.com,
        "open list:THERMAL" <linux-pm@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>,
        kong.kongxinwei@...ilicon.com
Subject: Re: [PATCH] thermal/drivers/hisi: Remove confusing error message

On Tue, Aug 08, 2017 at 08:48:51PM +0800, Zhang Rui wrote:

[...]

> > > > @@ -352,10 +353,9 @@ static int hisi_thermal_probe(struct
> > > > platform_device *pdev)
> > > >  		ret = hisi_thermal_register_sensor(pdev, data,
> > > >  						   &data-
> > > > > 
> > > > > sensors[i], i);
> > > >  		if (ret)
> > > > -			dev_err(&pdev->dev,
> > > > -				"failed to register thermal
> > > > sensor:
> > > > %d\n", ret);
> > > > -		else
> > > > -			hisi_thermal_toggle_sensor(&data-
> > > > > 
> > > > > sensors[i], true);
> > > > +			continue;
> > > > +
> > > > +		hisi_thermal_toggle_sensor(&data->sensors[i],
> > > > true);
> > > >  	}
> > > >  
> > > >  	return 0;
> > > With these removed, is there any other information in dmesg that
> > > suggests this failure?
> > The problem is there are always failures showed in dmesg. The init
> > function is based on the assumption there is HISI_MAX_SENSORS sensors
> > which is not true for the hi6220 and that raises at boot time errors.
> > 
> > Why HISI_MAX_SENSORS(=4) while there is only one on hi6220 AFAIK? and
> > this driver is only used for hi6220 (now).
> > 
> right, I think we should remove one error log, and then change the
> HISI_MAX_SENSORS to reflect the reality instead.
> 
> XinWei and Leo,
> can you please help check this?

Sure.

Here I am a bit confusion and I think this is a common question for
SoC thermal driver.

Hi6220 does has 4 thermal sensors, but we now only use one sensor of
them (thermal sensor id 2) to bind with thermal zone and other three
sensors are not bound to any thermal zone. So this is the reason the
booting reports the failure.

I think changing HISI_MAX_SENSORS value cannot resolve this issue, due
we are using thermal id 2. How about below change? We change to use
warning for sensors without binding, and remove redundant log.

diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
index 9c3ce34..6d34980 100644
--- a/drivers/thermal/hisi_thermal.c
+++ b/drivers/thermal/hisi_thermal.c
@@ -260,8 +260,6 @@ static int hisi_thermal_register_sensor(struct platform_device *pdev,
        if (IS_ERR(sensor->tzd)) {
                ret = PTR_ERR(sensor->tzd);
                sensor->tzd = NULL;
-               dev_err(&pdev->dev, "failed to register sensor id %d: %d\n",
-                       sensor->id, ret);
                return ret;
        }
 
@@ -351,7 +349,10 @@ static int hisi_thermal_probe(struct platform_device *pdev)
        for (i = 0; i < HISI_MAX_SENSORS; ++i) {
                ret = hisi_thermal_register_sensor(pdev, data,
                                                   &data->sensors[i], i);
-               if (ret)
+               if (ret == -ENODEV)
+                       dev_warn(&pdev->dev,
+                                "thermal sensor %d has not bound\n", i);
+               else if (ret)
                        dev_err(&pdev->dev,
                                "failed to register thermal sensor: %d\n", ret);
                else

Thanks,
Leo Yan