| Message ID | 1499439833-32531-1-git-send-email-daniel.lezcano@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Zhang Rui |
| Headers | show |
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
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.
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. > > > > >
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 ++++++------
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
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;
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(-)