On Tue, 2017-08-08 at 21:29 +0800, Leo Yan wrote: > 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. > Now we will get three "thermal sensor %d has not bound" messages for every normal probe, and an extra "failed to register thermal sensor:" for a real failure probe?
If that's the case, as we are not using the sensors on purpose, why not keep silence for -ENODEV? thanks, rui > 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