From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4DA1CC17447 for ; Wed, 13 Nov 2019 09:56:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 2D6E020818 for ; Wed, 13 Nov 2019 09:56:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727667AbfKMJ4n (ORCPT ); Wed, 13 Nov 2019 04:56:43 -0500 Received: from relay2-d.mail.gandi.net ([217.70.183.194]:37319 "EHLO relay2-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727626AbfKMJ4m (ORCPT ); Wed, 13 Nov 2019 04:56:42 -0500 X-Originating-IP: 93.34.114.233 Received: from uno.localdomain (93-34-114-233.ip49.fastwebnet.it [93.34.114.233]) (Authenticated sender: jacopo@jmondi.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id E79A24000B; Wed, 13 Nov 2019 09:56:36 +0000 (UTC) Date: Wed, 13 Nov 2019 10:58:31 +0100 From: Jacopo Mondi To: Geert Uytterhoeven Cc: Geert Uytterhoeven , Jacopo Mondi , Jonathan Cameron , Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald-Stadler , Wolfram Sang , linux-iio@vger.kernel.org, Linux-Renesas , Linux Kernel Mailing List Subject: Re: [PATCH] iio: adc: max9611: Fix too short conversion time delay Message-ID: <20191113095831.ipezy2ganb7tk73i@uno.localdomain> References: <20191113092133.23723-1-geert+renesas@glider.be> <20191113093828.vk5qqtlr7bs5z5fb@uno.localdomain> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="4nrix7m2kjdzc3cg" Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20180716 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --4nrix7m2kjdzc3cg Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Geert, On Wed, Nov 13, 2019 at 10:46:21AM +0100, Geert Uytterhoeven wrote: > Hi Jacopo, > > On Wed, Nov 13, 2019 at 10:36 AM Jacopo Mondi wrote: > > On Wed, Nov 13, 2019 at 10:21:33AM +0100, Geert Uytterhoeven wrote: > > > As of commit b9ddd5091160793e ("iio: adc: max9611: Fix temperature > > > reading in probe"), max9611 initialization sometimes fails on the > > > Salvator-X(S) development board with: > > > > > > max9611 4-007f: Invalid value received from ADC 0x8000: aborting > > > max9611: probe of 4-007f failed with error -5 > > > > > > The max9611 driver tests communications with the chip by reading the = die > > > temperature during the probe function, which returns an invalid value. > > > > > > According to the datasheet, the typical ADC conversion time is 2 ms, = but > > > no minimum or maximum values are provided. However, the driver assum= es > > > a 1 ms conversion time. Usually the usleep_range() call returns after > > > more than 1.8 ms, hence it succeeds. When it returns earlier, the da= ta > > > register may be read too early, and the previous measurement value wi= ll > > > be returned. After boot, this is the temperature POR (power-on reset) > > > value, causing the failure above. > > > > > > Fix this by increasing the delay from 1000-2000 =C2=B5s to 2000-2200 = =C2=B5s. > > > > > > Note that this issue has always been present, but it was exposed by t= he > > > aformentioned commit. > > > > > > Fixes: 69780a3bbc0b1e7e ("iio: adc: Add Maxim max9611 ADC driver") > > > Signed-off-by: Geert Uytterhoeven > > > --- > > > This problem was exposed in v5.3. > > > > > > After this patch, probing of the two max9611 sensors succeeded during > > > ca. 3000 boot cycles on Salvator-X(S) boards, equipped with various > > > R-Car H3/M3-W/M3-N SoCs. > > > --- > > > drivers/iio/adc/max9611.c | 11 ++++++++--- > > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/iio/adc/max9611.c b/drivers/iio/adc/max9611.c > > > index da073d72f649f829..b0755f25356d700d 100644 > > > --- a/drivers/iio/adc/max9611.c > > > +++ b/drivers/iio/adc/max9611.c > > > @@ -89,6 +89,11 @@ > > > #define MAX9611_TEMP_SCALE_NUM 1000000 > > > #define MAX9611_TEMP_SCALE_DIV 2083 > > > > > > +/* > > > + * Conversion time is 2 ms (typically) > > > + */ > > > +#define MAX9611_CONV_TIME_US_RANGE 2000, 2200 > > > + > > > > Is a 20% sleep range enough or should it be slightly lengthen ? > > 10%? Ehrm... yes :/ > > This only impacts the variation, so what really happens depends on the > rate of the hrtimer (if present). > On R-Car Gen3, I think that uses the ARM Architectured Timer (cp15), > which has a period of 120 ns. > I'm not questioning the hrtimer rate, I'm questioning what would be an ideal interval to coalesce this with as much other delays as possible, but I think we're good and this is really a minor thing mostly for my personal education, as I've seen mentioned in other reviews a 20% range is usually suggested (found no mention of that in timers-howto.rst though) Thanks again for fixing this j > > Apart from this, thanks a lot for finding the issue root cause! > > > > Reviewed-by: Jacopo Mondi > > Thanks! > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m6= 8k.org > > In personal conversations with technical people, I call myself a hacker. = But > when I'm talking to journalists I just say "programmer" or something like= that. > -- Linus Torvalds --4nrix7m2kjdzc3cg Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEtcQ9SICaIIqPWDjAcjQGjxahVjwFAl3L08cACgkQcjQGjxah Vjz2VBAAmVgNax3Q2EqlMa191fXpyMMKE1rVyVhEmTpY6CjHQjCAIeTsnb/POqZU 6BM6jZ2WNkpb3zHxRlRjURKkJUsupTgCZS+ifYNTraIoqXuY3PYpXPo6/lOHMGb5 /vSDaLPF5sAiy4z670Gh3trgi3JZzWl0cLKpPCrUGa+TiSO3ifD0yAqAjQN4//0M fKoWokKRvVLsXX1ZkuryKZdDkKq9JDIjui4xDQHGo+2P4oexg7iF735msbXuyH/w VJ43+qQYktd9+lST8JYMf/c8aMDh8NGMs3m+DKxkvBiIzVySY9k2eGMcS4VUXjy+ r4SnFpmYoxJOpMRIdOOCn3ZkCEQRmfkuLBIZUgLd8gu3yKsvfD+hSr5V89wUSvVl QBnSXCIRKUefcUWSR9P8O+CmyF//n/EipO9+Dm4l3SPuZPPJx/3UdG6UwsqaCxdI Yo5RXVFPP70Xae29ufeqMCN0vP8tMuqMYuBdQhbXqy0hDSCZVI2KOEnnhhy/GITr EiDoEKcBGQuGv43zG8Dv9W/ISpVmlH/0Yvt0Y47iShVE6szWFGyWswK3sRjLuuF+ xDGxWMsAtCfeCCfM0CB2cJjTqq4TAtseep6Vprj2FPZkfUSQsjLeJChVlvtMMBDl idDM6QYmNMJEKCtw2RdCKFJncu5hhPwAvfiFyNjOa7CWkjsI8rY= =evql -----END PGP SIGNATURE----- --4nrix7m2kjdzc3cg--