Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753229AbdHUKGX (ORCPT ); Mon, 21 Aug 2017 06:06:23 -0400 Received: from mail-wr0-f170.google.com ([209.85.128.170]:33429 "EHLO mail-wr0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751894AbdHUKGV (ORCPT ); Mon, 21 Aug 2017 06:06:21 -0400 Subject: Re: [PATCH] thermal/drivers/hisi: Remove confusing error message To: Leo Yan , Zhang Rui Cc: edubezval@gmail.com, "open list:THERMAL" , open list , kong.kongxinwei@hisilicon.com References: <1499439833-32531-1-git-send-email-daniel.lezcano@linaro.org> <1502178956.4296.7.camel@intel.com> <46604862-fbb8-8ed1-8b0d-7a51543b3398@linaro.org> <1502196531.4296.41.camel@intel.com> <20170808132955.GB32732@leoy-ThinkPad-T440> From: Daniel Lezcano Message-ID: <8f58bd6d-0561-7383-6be4-04ee3704099a@linaro.org> Date: Mon, 21 Aug 2017 12:06:17 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170808132955.GB32732@leoy-ThinkPad-T440> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3533 Lines: 107 On 08/08/2017 15:29, 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. Hi Leo, a cleanest solution would be either: - add the 3 missing thermal sensors in the DT and default to the id 2 or - remove all the code assuming 4 sensors and deal with the one unique sensor No ? > 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 > -- Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog