thermal/drivers/hisi: Remove confusing error message
diff mbox

Message ID 1499439833-32531-1-git-send-email-daniel.lezcano@linaro.org
State Changes Requested
Delegated to: Zhang Rui
Headers show

Commit Message

Daniel Lezcano July 7, 2017, 3:03 p.m. UTC
The sensor id is unknown at init time and we use all id in the authorized
MAX_SENSORS interval to register the sensor. On this SoC there is one
thermal-zone with one sensor on it. No need to spit on the console everytime we
failed to register thermal sensors, information which is deliberaly known as it
is part of the discovery process.

 hisi_thermal f7030700.tsensor: failed to register sensor id 0: -19
 hisi_thermal f7030700.tsensor: failed to register thermal sensor: -19
 hisi_thermal f7030700.tsensor: failed to register sensor id 1: -19
 hisi_thermal f7030700.tsensor: failed to register thermal sensor: -19
 hisi_thermal f7030700.tsensor: failed to register sensor id 3: -19
 hisi_thermal f7030700.tsensor: failed to register thermal sensor: -19

Remove the error messages.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/thermal/hisi_thermal.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Zhang Rui Aug. 8, 2017, 7:55 a.m. UTC | #1
On Fri, 2017-07-07 at 17:03 +0200, Daniel Lezcano wrote:
> The sensor id is unknown at init time and we use all id in the
> authorized
> MAX_SENSORS interval to register the sensor. On this SoC there is one
> thermal-zone with one sensor on it. No need to spit on the console
> everytime we
> failed to register thermal sensors, information which is deliberaly
> known as it
> is part of the discovery process.
> 
>  hisi_thermal f7030700.tsensor: failed to register sensor id 0: -19
>  hisi_thermal f7030700.tsensor: failed to register thermal sensor:
> -19
>  hisi_thermal f7030700.tsensor: failed to register sensor id 1: -19
>  hisi_thermal f7030700.tsensor: failed to register thermal sensor:
> -19
>  hisi_thermal f7030700.tsensor: failed to register sensor id 3: -19
>  hisi_thermal f7030700.tsensor: failed to register thermal sensor:
> -19
> 
> Remove the error messages.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/thermal/hisi_thermal.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/thermal/hisi_thermal.c
> b/drivers/thermal/hisi_thermal.c
> index f642966..2cc98c6 100644
> --- a/drivers/thermal/hisi_thermal.c
> +++ b/drivers/thermal/hisi_thermal.c
> @@ -187,6 +187,9 @@ static int hisi_thermal_get_temp(void *_sensor,
> int *temp)
>  
>  	dev_dbg(&data->pdev->dev, "id=%d, irq=%d, temp=%d,
> thres=%d\n",
>  		sensor->id, data->irq_enabled, *temp, sensor-
> >thres_temp);
> +
> +	printk("id=%d, irq=%d, temp=%d, thres=%d\n",
> +		sensor->id, data->irq_enabled, *temp, sensor-
> >thres_temp);

what's this printk for?

>  	/*
>  	 * Bind irq to sensor for two cases:
>  	 *   Reenable alarm IRQ if temperature below threshold;
> @@ -260,8 +263,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;
>  	}
>  
> @@ -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?

thanks,
rui
Daniel Lezcano Aug. 8, 2017, 10:15 a.m. UTC | #2
On 08/08/2017 09:55, Zhang Rui wrote:
> On Fri, 2017-07-07 at 17:03 +0200, Daniel Lezcano wrote:
>> The sensor id is unknown at init time and we use all id in the
>> authorized
>> MAX_SENSORS interval to register the sensor. On this SoC there is one
>> thermal-zone with one sensor on it. No need to spit on the console
>> everytime we
>> failed to register thermal sensors, information which is deliberaly
>> known as it
>> is part of the discovery process.
>>
>>  hisi_thermal f7030700.tsensor: failed to register sensor id 0: -19
>>  hisi_thermal f7030700.tsensor: failed to register thermal sensor:
>> -19
>>  hisi_thermal f7030700.tsensor: failed to register sensor id 1: -19
>>  hisi_thermal f7030700.tsensor: failed to register thermal sensor:
>> -19
>>  hisi_thermal f7030700.tsensor: failed to register sensor id 3: -19
>>  hisi_thermal f7030700.tsensor: failed to register thermal sensor:
>> -19
>>
>> Remove the error messages.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>  drivers/thermal/hisi_thermal.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/thermal/hisi_thermal.c
>> b/drivers/thermal/hisi_thermal.c
>> index f642966..2cc98c6 100644
>> --- a/drivers/thermal/hisi_thermal.c
>> +++ b/drivers/thermal/hisi_thermal.c
>> @@ -187,6 +187,9 @@ static int hisi_thermal_get_temp(void *_sensor,
>> int *temp)
>>  
>>  	dev_dbg(&data->pdev->dev, "id=%d, irq=%d, temp=%d,
>> thres=%d\n",
>>  		sensor->id, data->irq_enabled, *temp, sensor-
>>> thres_temp);
>> +
>> +	printk("id=%d, irq=%d, temp=%d, thres=%d\n",
>> +		sensor->id, data->irq_enabled, *temp, sensor-
>>> thres_temp);
> 
> what's this printk for?

Argh. It shouldn't be there.

>>  	/*
>>  	 * Bind irq to sensor for two cases:
>>  	 *   Reenable alarm IRQ if temperature below threshold;
>> @@ -260,8 +263,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;
>>  	}
>>  
>> @@ -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).

That ends up with 3 errors in dmesg for nothing.
Zhang Rui Aug. 8, 2017, 12:48 p.m. UTC | #3
On Tue, 2017-08-08 at 12:15 +0200, Daniel Lezcano wrote:
> On 08/08/2017 09:55, Zhang Rui wrote:
> > 
> > On Fri, 2017-07-07 at 17:03 +0200, Daniel Lezcano wrote:
> > > 
> > > The sensor id is unknown at init time and we use all id in the
> > > authorized
> > > MAX_SENSORS interval to register the sensor. On this SoC there is
> > > one
> > > thermal-zone with one sensor on it. No need to spit on the
> > > console
> > > everytime we
> > > failed to register thermal sensors, information which is
> > > deliberaly
> > > known as it
> > > is part of the discovery process.
> > > 
> > >  hisi_thermal f7030700.tsensor: failed to register sensor id 0:
> > > -19
> > >  hisi_thermal f7030700.tsensor: failed to register thermal
> > > sensor:
> > > -19
> > >  hisi_thermal f7030700.tsensor: failed to register sensor id 1:
> > > -19
> > >  hisi_thermal f7030700.tsensor: failed to register thermal
> > > sensor:
> > > -19
> > >  hisi_thermal f7030700.tsensor: failed to register sensor id 3:
> > > -19
> > >  hisi_thermal f7030700.tsensor: failed to register thermal
> > > sensor:
> > > -19
> > > 
> > > Remove the error messages.
> > > 
> > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > > ---
> > >  drivers/thermal/hisi_thermal.c | 12 ++++++------
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/thermal/hisi_thermal.c
> > > b/drivers/thermal/hisi_thermal.c
> > > index f642966..2cc98c6 100644
> > > --- a/drivers/thermal/hisi_thermal.c
> > > +++ b/drivers/thermal/hisi_thermal.c
> > > @@ -187,6 +187,9 @@ static int hisi_thermal_get_temp(void
> > > *_sensor,
> > > int *temp)
> > >  
> > >  	dev_dbg(&data->pdev->dev, "id=%d, irq=%d, temp=%d,
> > > thres=%d\n",
> > >  		sensor->id, data->irq_enabled, *temp, sensor-
> > > > 
> > > > thres_temp);
> > > +
> > > +	printk("id=%d, irq=%d, temp=%d, thres=%d\n",
> > > +		sensor->id, data->irq_enabled, *temp, sensor-
> > > > 
> > > > thres_temp);
> > what's this printk for?
> Argh. It shouldn't be there.
> 
> > 
> > > 
> > >  	/*
> > >  	 * Bind irq to sensor for two cases:
> > >  	 *   Reenable alarm IRQ if temperature below threshold;
> > > @@ -260,8 +263,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;
> > >  	}
> > >  
> > > @@ -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?

thanks,
rui
> That ends up with 3 errors in dmesg for nothing.
> 
> 
> 
> 
>
Eduardo Valentin Dec. 5, 2017, 1:52 a.m. UTC | #4
Hello,

Catching up on old patches.
On Fri, Jul 07, 2017 at 05:03:52PM +0200, Daniel Lezcano wrote:
> The sensor id is unknown at init time and we use all id in the authorized
> MAX_SENSORS interval to register the sensor. On this SoC there is one
> thermal-zone with one sensor on it. No need to spit on the console everytime we
> failed to register thermal sensors, information which is deliberaly known as it
> is part of the discovery process.
> 
>  hisi_thermal f7030700.tsensor: failed to register sensor id 0: -19
>  hisi_thermal f7030700.tsensor: failed to register thermal sensor: -19
>  hisi_thermal f7030700.tsensor: failed to register sensor id 1: -19
>  hisi_thermal f7030700.tsensor: failed to register thermal sensor: -19
>  hisi_thermal f7030700.tsensor: failed to register sensor id 3: -19
>  hisi_thermal f7030700.tsensor: failed to register thermal sensor: -19
> 
> Remove the error messages

Is this still needed? I am assuming no.
.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/thermal/hisi_thermal.c | 12 ++++++------
Daniel Lezcano Dec. 5, 2017, 6:48 a.m. UTC | #5
On 05/12/2017 02:52, Eduardo Valentin wrote:
> Hello,
> 
> Catching up on old patches.
> On Fri, Jul 07, 2017 at 05:03:52PM +0200, Daniel Lezcano wrote:
>> The sensor id is unknown at init time and we use all id in the authorized
>> MAX_SENSORS interval to register the sensor. On this SoC there is one
>> thermal-zone with one sensor on it. No need to spit on the console everytime we
>> failed to register thermal sensors, information which is deliberaly known as it
>> is part of the discovery process.
>>
>>  hisi_thermal f7030700.tsensor: failed to register sensor id 0: -19
>>  hisi_thermal f7030700.tsensor: failed to register thermal sensor: -19
>>  hisi_thermal f7030700.tsensor: failed to register sensor id 1: -19
>>  hisi_thermal f7030700.tsensor: failed to register thermal sensor: -19
>>  hisi_thermal f7030700.tsensor: failed to register sensor id 3: -19
>>  hisi_thermal f7030700.tsensor: failed to register thermal sensor: -19
>>
>> Remove the error messages
> 
> Is this still needed? I am assuming no.

Right, no longer needed.

  -- Daniel

Patch
diff mbox

diff --git a/drivers/thermal/hisi_thermal.c b/drivers/thermal/hisi_thermal.c
index f642966..2cc98c6 100644
--- a/drivers/thermal/hisi_thermal.c
+++ b/drivers/thermal/hisi_thermal.c
@@ -187,6 +187,9 @@  static int hisi_thermal_get_temp(void *_sensor, int *temp)
 
 	dev_dbg(&data->pdev->dev, "id=%d, irq=%d, temp=%d, thres=%d\n",
 		sensor->id, data->irq_enabled, *temp, sensor->thres_temp);
+
+	printk("id=%d, irq=%d, temp=%d, thres=%d\n",
+		sensor->id, data->irq_enabled, *temp, sensor->thres_temp);
 	/*
 	 * Bind irq to sensor for two cases:
 	 *   Reenable alarm IRQ if temperature below threshold;
@@ -260,8 +263,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;
 	}
 
@@ -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;