From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934402AbcKVSXv (ORCPT ); Tue, 22 Nov 2016 13:23:51 -0500 Received: from vern.gendns.com ([206.190.152.46]:52190 "EHLO vern.gendns.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934164AbcKVSXt (ORCPT ); Tue, 22 Nov 2016 13:23:49 -0500 Subject: Re: [PATCH v2] iio: adc: New driver for TI ADS7950 chips To: Jonathan Cameron , Peter Meerwald-Stadler References: <1479666484-2582-1-git-send-email-david@lechnology.com> <04F737B2-EB34-47DB-A913-C7D0A987E9AA@jic23.retrosnub.co.uk> Cc: Jonathan Cameron , Hartmut Knaack , Lars-Peter Clausen , linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org From: David Lechner Message-ID: <748ee481-eb9a-28f2-2c8b-214eea63c2b0@lechnology.com> Date: Tue, 22 Nov 2016 12:23:46 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <04F737B2-EB34-47DB-A913-C7D0A987E9AA@jic23.retrosnub.co.uk> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - vern.gendns.com X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - lechnology.com X-Get-Message-Sender-Via: vern.gendns.com: authenticated_id: davidmain+lechnology.com/only user confirmed/virtual account not confirmed X-Authenticated-Sender: vern.gendns.com: davidmain@lechnology.com X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/22/2016 01:23 AM, Jonathan Cameron wrote: > > > On 21 November 2016 22:54:24 GMT+00:00, Peter Meerwald-Stadler wrote: >> >>>> +static int ti_ads7950_read_raw(struct iio_dev *indio_dev, >>>> + struct iio_chan_spec const *chan, >>>> + int *val, int *val2, long m) >>>> +{ >>>> + struct ti_ads7950_state *st = iio_priv(indio_dev); >>>> + int ret; >>>> + >>>> + switch (m) { >>>> + case IIO_CHAN_INFO_RAW: >>>> + >>>> + ret = iio_device_claim_direct_mode(indio_dev); >>>> + if (ret < 0) >>>> + return ret; >>>> + >>>> + ret = ti_ads7950_scan_direct(st, chan->address); >>>> + iio_device_release_direct_mode(indio_dev); >>>> + if (ret < 0) >>>> + return ret; >>>> + >>>> + if (chan->address != TI_ADS7950_EXTRACT(ret, 12, 4)) >>>> + return -EIO; >>>> + >>>> + *val = TI_ADS7950_EXTRACT(ret, 0, 12); >>> >>> I'm not sure if I am doing this right. There are 8- 10- and 12-bit >> versions of >>> this chip. The 8- and 10-bit versions still return a 12-bit number >> where the >>> last 4 or 2 bits are always 0. Should I be shifting the 12-bit value >> here >>> based on the chip being used so that *val is 0-255 for 8-bit and >> 0-1023 for >>> 10-bit? Or should this be *really* raw and not even use >> TI_ADS7950_EXTRACT() >>> to mask the channel address bits? >> >> I'd shift and adjust _SCALE so that *val * scale gives mV > It would also be fine to not do anything and let userspace deal with shifting and masking for > buffered data. Non buffered obviously still needs shifting and masking though! I have sent a v3 patch already that does exactly this. Buffered data is untouched and unbuffered data is now correct with shifting and masking. > > I'd slightly prefer the doing nothing route but don't really care as both are valid uses of the ABI. > > Jonathan >> >>>> + >>>> + return IIO_VAL_INT; >>>> + case IIO_CHAN_INFO_SCALE: >>>> + ret = ti_ads7950_get_range(st); >>>> + if (ret < 0) >>>> + return ret; >>>> + >>>> + *val = ret; >>>> + *val2 = chan->scan_type.realbits; >>>> + >>>> + return IIO_VAL_FRACTIONAL_LOG2; >>>> + } >>>> + >>>> + return -EINVAL; >>>> +} >