Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753685AbbHJH7L (ORCPT ); Mon, 10 Aug 2015 03:59:11 -0400 Received: from mail-ig0-f178.google.com ([209.85.213.178]:36690 "EHLO mail-ig0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753283AbbHJH7I convert rfc822-to-8bit (ORCPT ); Mon, 10 Aug 2015 03:59:08 -0400 MIME-Version: 1.0 In-Reply-To: <55C84521.5040007@rock-chips.com> References: <20150807205923.GA33949@dtor-ws> <55C84521.5040007@rock-chips.com> Date: Mon, 10 Aug 2015 00:59:04 -0700 Message-ID: Subject: Re: [PATCH] thermal: rockchip: fix handling of invalid readings From: Dmitry Torokhov To: Caesar Wang Cc: Dmitry Torokhov , 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" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4499 Lines: 128 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... > > > 在 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) >> { >> - 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. > > >> - *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/