linux-kernel - Re: [PATCH v2] iio: adc: New driver for TI ADS7950 chips

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <04F737B2-EB34-47DB-A913-C7D0A987E9AA@jic23.retrosnub.co.uk>
Date:   Tue, 22 Nov 2016 07:23:13 +0000
From:   Jonathan Cameron <jic23@...23.retrosnub.co.uk>
To:     Peter Meerwald-Stadler <pmeerw@...erw.net>,
        David Lechner <david@...hnology.com>
CC:     Jonathan Cameron <jic23@...nel.org>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org
Subject: Re: [PATCH v2] iio: adc: New driver for TI ADS7950 chips



On 21 November 2016 22:54:24 GMT+00:00, Peter Meerwald-Stadler <pmeerw@...erw.net> 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'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;
>> > +}

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.