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

Zhang Rui
2017-08-08T12:49:03Z

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.
>
>
>
>
>