Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934572AbbHJKW7 (ORCPT ); Mon, 10 Aug 2015 06:22:59 -0400 Received: from regular1.263xmail.com ([211.150.99.130]:59487 "EHLO regular1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934342AbbHJKWy (ORCPT ); Mon, 10 Aug 2015 06:22:54 -0400 X-263anti-spam: KSV:0;BIG:0;ABS:1;DNS:0;ATT:0;SPF:S; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ABS-CHECKED: 1 X-SKE-CHECKED: 1 X-ADDR-CHECKED: 0 X-RL-SENDER: wxt@rock-chips.com X-FST-TO: linux-kernel@vger.kernel.org X-SENDER-IP: 58.22.7.114 X-LOGIN-NAME: wxt@rock-chips.com X-UNIQUE-TAG: <712641d08852fb40211d7d6f9250c929> X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Message-ID: <55C87B70.5050805@rock-chips.com> Date: Mon, 10 Aug 2015 18:22:40 +0800 From: Caesar Wang User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Dmitry Torokhov CC: Heiko Stuebner , Eduardo Valentin , Doug Anderson , "linux-pm@vger.kernel.org" , linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] thermal: rockchip: fix handling of invalid readings References: <20150807205923.GA33949@dtor-ws> <55C84521.5040007@rock-chips.com> In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5184 Lines: 154 在 2015年08月10日 15:59, Dmitry Torokhov 写道: > Hi Caesar, > > On Sun, Aug 9, 2015 at 11:30 PM, Caesar Wang wrote: >> Dear Dmitry, >> >> Thanks your patch. >> >> The code looks like fine,but I don't think the TS-ADC will work on rk3288 >> SoC. > What do you mean? It seems to work when I ran it... Making a mistake.:-( >> >> 在 2015年08月08日 04:59, Dmitry Torokhov 写道: >>> We attempted to signal invalid code by returning -EAGAIN from >>> rk_tsadcv2_code_to_temp(), unfortunately the return value was stuffed >>> directly into the temperature pointer, potentially confusing upper >>> layers with temperature of -EINVAL. >>> >>> Let's split temperature from error/success indicator to avoid such >>> confusion. >>> >>> Also change the way we scan the temperature table to start with the 2nd >>> element so that we do not need to worry that we may reference out of >>> bounds element while doing binary search and keep checking that we end >>> up with 'mid' equal to 0 (since we are looking for the temperature that >>> would fall into interval between the 'mid' and 'mid - 1') . >>> >>> Signed-off-by: Dmitry Torokhov >>> --- >>> drivers/thermal/rockchip_thermal.c | 28 +++++++++++++--------------- >>> 1 file changed, 13 insertions(+), 15 deletions(-) >>> >>> diff --git a/drivers/thermal/rockchip_thermal.c >>> b/drivers/thermal/rockchip_thermal.c >>> index c89ffb2..93ee307 100644 >>> --- a/drivers/thermal/rockchip_thermal.c >>> +++ b/drivers/thermal/rockchip_thermal.c >>> @@ -124,7 +124,7 @@ struct rockchip_thermal_data { >>> #define TSADCV2_AUTO_PERIOD_HT_TIME 50 /* msec */ >>> struct tsadc_table { >>> - unsigned long code; >>> + u32 code; >>> long temp; >>> }; >>> @@ -164,7 +164,6 @@ static const struct tsadc_table v2_code_table[] = { >>> {3452, 115000}, >>> {3437, 120000}, >>> {3421, 125000}, >>> - {0, 125000}, >>> }; >>> static u32 rk_tsadcv2_temp_to_code(long temp) >>> @@ -191,19 +190,21 @@ static u32 rk_tsadcv2_temp_to_code(long temp) >>> return 0; >>> } >>> -static int rk_tsadcv2_code_to_temp(u32 code) >>> +static int rk_tsadcv2_code_to_temp(u32 code, int *temp) It's wired that can't git-am this patch. I search the Eduardo'branch[0] for rockchip-thermal.c driver. The function is "static long rk_tsadcv2_code_to_temp(u32 code)" Maybe i'm missing something, I verified this patch on my board but this fixed. So you can free add it: Tested-by: Caesar Wang [0]: url = git://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git --- Thanks, Caesar >>> { >>> - unsigned int low = 0; >>> + unsigned int low = 1; >>> unsigned int high = ARRAY_SIZE(v2_code_table) - 1; >>> unsigned int mid = (low + high) / 2; >>> unsigned int num; >>> unsigned long denom; >>> - /* Invalid code, return -EAGAIN */ >>> - if (code > TSADCV2_DATA_MASK) >>> - return -EAGAIN; >>> + BUILD_BUG_ON(ARRAY_SIZE(v2_code_table) < 2); >>> - while (low <= high && mid) { >>> + code &= TSADCV2_DATA_MASK; >>> + if (code < v2_code_table[high].code) >>> + return -EAGAIN; /* Incorrect reading */ >>> + >>> + while (low <= high) { >>> if (code >= v2_code_table[mid].code && >>> code < v2_code_table[mid - 1].code) >>> break; >>> @@ -223,7 +224,9 @@ static int rk_tsadcv2_code_to_temp(u32 code) >>> num = v2_code_table[mid].temp - v2_code_table[mid - 1].temp; >>> num *= v2_code_table[mid - 1].code - code; >>> denom = v2_code_table[mid - 1].code - v2_code_table[mid].code; >>> - return v2_code_table[mid - 1].temp + (num / denom); >>> + *temp = v2_code_table[mid - 1].temp + (num / denom); >>> + >>> + return 0; >>> } >>> /** >>> @@ -281,14 +284,9 @@ static int rk_tsadcv2_get_temp(int chn, void __iomem >>> *regs, int *temp) >>> { >>> u32 val; >>> - /* the A/D value of the channel last conversion need some time */ >>> val = readl_relaxed(regs + TSADCV2_DATA(chn)); >>> - if (val == 0) >>> - return -EAGAIN; >>> - >> >> if we reserve the above code, that will get the ADC value. > But if we pass 0 into rk_tsadcv2_code_to_temp() it will trigger: > >>> + if (code < v2_code_table[high].code) >>> + return -EAGAIN; /* Incorrect reading */ > and still return -EAGAIN, as it was. There is no behavior change as > far as I can see. Yup, that's ok for me. >> >>> - *temp = rk_tsadcv2_code_to_temp(val); >>> - return 0; >>> + return rk_tsadcv2_code_to_temp(val, temp); >>> } >>> static void rk_tsadcv2_tshut_temp(int chn, void __iomem *regs, long >>> temp) >> > Thanks, > Dmitry > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/