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=-10.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_2 autolearn=ham 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 BEBBFC4361B for ; Mon, 14 Dec 2020 15:09:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7E88422795 for ; Mon, 14 Dec 2020 15:09:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730138AbgLNPIp (ORCPT ); Mon, 14 Dec 2020 10:08:45 -0500 Received: from frasgout.his.huawei.com ([185.176.79.56]:2255 "EHLO frasgout.his.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729007AbgLNPIi (ORCPT ); Mon, 14 Dec 2020 10:08:38 -0500 Received: from fraeml734-chm.china.huawei.com (unknown [172.18.147.226]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4Cvl783gv6z67Pyf; Mon, 14 Dec 2020 23:05:08 +0800 (CST) Received: from lhreml710-chm.china.huawei.com (10.201.108.61) by fraeml734-chm.china.huawei.com (10.206.15.215) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2106.2; Mon, 14 Dec 2020 16:07:56 +0100 Received: from localhost (10.47.77.193) by lhreml710-chm.china.huawei.com (10.201.108.61) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2106.2; Mon, 14 Dec 2020 15:07:55 +0000 Date: Mon, 14 Dec 2020 15:07:28 +0000 From: Jonathan Cameron To: Peter Rosin CC: Jonathan Cameron , Linus Walleij , linux-iio , "Hartmut Knaack" , Lars-Peter Clausen , "Peter Meerwald-Stadler" Subject: Re: [PATCH] iio: afe: iio-rescale: Support processed channels Message-ID: <20201214150728.00001fa7@Huawei.com> In-Reply-To: References: <20201101232211.1194304-1-linus.walleij@linaro.org> <435ebb1b-431c-fdeb-023e-39c6f6102e22@axentia.se> <20201213121615.55a86f77@archlinux> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 3.17.4 (GTK+ 2.24.32; i686-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.47.77.193] X-ClientProxiedBy: lhreml718-chm.china.huawei.com (10.201.108.69) To lhreml710-chm.china.huawei.com (10.201.108.61) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-iio@vger.kernel.org On Mon, 14 Dec 2020 09:34:40 +0100 Peter Rosin wrote: > On 2020-12-13 13:16, Jonathan Cameron wrote: > > On Sun, 13 Dec 2020 00:22:17 +0100 > > Peter Rosin wrote: > > > >> On 2020-12-12 13:26, Linus Walleij wrote: > >>> On Mon, Nov 2, 2020 at 12:22 AM Linus Walleij wrote: > >>> > >>>> It happens that an ADC will only provide raw or processed > >>>> voltage conversion channels. (adc/ab8500-gpadc.c). > >>>> On the Samsung GT-I9070 this is used for a light sensor > >>>> and current sense amplifier so we need to think of something. > >>>> > >>>> The idea is to allow processed channels and scale them > >>>> with 1/1 and then the rescaler can modify the result > >>>> on top. > >>>> > >>>> Cc: Peter Rosin > >>>> Signed-off-by: Linus Walleij > >>> > >>> Did we reach any conclusion on this? I really need to use > >>> the rescaler on an ADC that only handles processed channels... > >>> > >>> I'm sorry that I can't make this ADC disappear :D > >> > >> Hi! > >> > >> My conclusion was that the patch is buggy since it presents inconsistent > >> information. That needs to be fixed one way or the other. If the offending > >> information cannot be filtered out for some reason, I don't know what to > >> do. Details in my previous comment [1]. BTW, I still do not know the answer > >> to the .read_avail question at the end of that message, and I don't have > >> time to dig into it. Sorry. > > > > Unless I'm missing something, I think it presents no information unless > > we strangely have a driver providing read_avail for _RAW but only > > _PROCESSED channels which is a bug. I'm not that bothered about > > missing information in this particular, somewhat obscure, corner case. > > > > So I think we should take the patch as it stands. It's missed the > > merge window now anyway unfortunately. So Peter, I would suggest we > > take this and perhaps revisit to tidy up loose corners when we all have > > more time. > > My concern was a driver with a raw channel, including read_avail, providing > raw sample values but that no easy conversion existed to get from that to > the processed values. One option for the driver in that case would be to > provide these raw values, but then have no scaling info. Generally I resist this a lot. The reason is that it is impossible to write generic userspace software against it. The one time we did let this happen was with some of the heart rate sensors (pulse oximeters) where the algorithm to derive the eventual value is both complex - based on published literature, and proprietary (what was actually readily usable). What the measurement being provided to userspace was is well documented, but not how on earth you get from that to something useable for what the sensor is designed to measure. > I.e. the way I see > it, it is perfectly reasonable for a driver to provide raw with read_avail, > no scaling but also processed values. Why? What use would the raw values actually be? There are a couple of historical drivers where they evolved to this state, but it is not one we would normally accept. We go to a lot of effort to try and avoid this. > And that gets transformed by the > rescaler into the processed values being presented as raw, with rescaling > added on top, but with the read_avail info for this new raw channel being > completely wrong. > > For the intended driver (ab8500-gpadc) this is not the case (it has no > read_avail for its raw channel). But it does have a raw channel, so adding > read_avail seems easy and I can easily see other drivers already doing it. > Haven't checked that though... Drat. I'd failed to register this is one of those corner cases. > > But if you say that this never happens, fine. Otherwise, since it's too > late for the merge window anyway, the patch might as well be updated such > that the rescaler blocks the read_avail channel in this situation, if it > exists. That's fair enough. A sanity check and then suitable warning message to explain why it is blocked makes sense. Jonathan > > Cheers, > Peter