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=-5.5 required=3.0 tests=BAYES_00,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_2 autolearn=no 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 46457C433E0 for ; Tue, 29 Dec 2020 16:54:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 121CE207C9 for ; Tue, 29 Dec 2020 16:54:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726168AbgL2Qyn (ORCPT ); Tue, 29 Dec 2020 11:54:43 -0500 Received: from mail.kernel.org ([198.145.29.99]:37740 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726178AbgL2Qyn (ORCPT ); Tue, 29 Dec 2020 11:54:43 -0500 Received: from archlinux (cpc108967-cmbg20-2-0-cust86.5-4.cable.virginm.net [81.101.6.87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 99A4A207C9; Tue, 29 Dec 2020 16:54:01 +0000 (UTC) Date: Tue, 29 Dec 2020 16:53:57 +0000 From: Jonathan Cameron To: Guenter Roeck Cc: Chris Lesiak , Linus Walleij , Jean Delvare , "linux-hwmon@vger.kernel.org" , Peter Rosin , Jonathan Cameron , "linux-iio@vger.kernel.org" Subject: Re: [PATCH v2] hwmon: (ntc_thermistor): try reading processed Message-ID: <20201229165357.3b833682@archlinux> In-Reply-To: <20201229163346.GA57378@roeck-us.net> References: <20201224011607.1059534-1-linus.walleij@linaro.org> <20201229142531.3a4ceedc@archlinux> <20201229163346.GA57378@roeck-us.net> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-hwmon@vger.kernel.org On Tue, 29 Dec 2020 08:33:46 -0800 Guenter Roeck wrote: > On Tue, Dec 29, 2020 at 02:25:31PM +0000, Jonathan Cameron wrote: > > On Sun, 27 Dec 2020 22:08:24 +0000 > > Chris Lesiak wrote: > > > > > Linus Walleij wrote: > > > > Aha you mean that iio_read_channel_processed() loses > > > > precision when converting raw to scaled? > > > > > > Yes. For example, take a 16-bit ADC with 4.096 V reference. > > > The native resolution is 62.5 microVolts. > > > Yet iio_read_channel_processed() can only give integer milliVolts. > > > iio_read_channel_raw() followed by iio_convert_raw_to_processed() > > > with a scale of 1000 will preserve more of the native resolution. > > > User space can do this by using floating point numbers when > > > converting to processed. > > > > > > You are likely to lose precision for all ADCs greater than about 12-bit. > > > > > > Chris Lesiak wrote: > > > >> I'd prefer a solution similar to the existing implementation of > > > >> iio_read_channel_processed. > > > > > > > That seems like the wrong way to work around a problem in > > > > the core. > > > > > > > If iio_read_channel_processed() loses precision we should > > > > fix iio_read_channel_processed() and not try to work around > > > > the problem in the consumers. > > > > > > > It's fine to fix all the consumers in the kernel. > > > > > > > What about changing the signature of: > > > > > > > int iio_read_channel_processed(struct iio_channel *chan, int *val) > > > > > > > to: > > > > > > > int iio_read_channel_processed(struct iio_channel *chan, int *val, > > > > unsigned int scale) > > > > > > > And just augment all calls to pass 1 except the ntc driver > > > > which then passes 1000 in the last argument? > > > > > > > If Jonathan agrees I can fix a patch to alter all the ~50 > > > > call sites like this and include the change to this NTC > > > > driver. > > > > > > That would meet my needs and does address what I think is a > > > shortcoming in the existing iio_read_channel_processed interface. > > I'm fine with this proposal as well. Makes a lot of sense given > > there is no particular reason why another subsystem should want to > > convert to IIO base units (here milivolts). > > > > The only other way I could think of doing it would be to > > have iio_read_channel_processed 'return' a pair of integers and > > type etc IIO_VAL_INT_PLUS_MICRO much like read_raw etc does inside > > the actual drivers. > > > > It would be a bit clunky to implement however and potentially require > > some messy maths in the consumers. > > > > May want to think about whether we need additional sanity checks for > > overflow etc. Seems unlikely we'd hit hit them for voltage, but > > we might for some other types of sensor where the base unit is much > > smaller (wrt to real world values). > > > > All this makes me wonder if it would be better to add a separate > API function (iio_read_channel_processed_scale ?) instead of replacing > the existing one. Changing 50+ callers at the same time sounds messy. Agreed - definitely makes more sense to do it that way. Jonathan > > Guenter