| 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
| ||
|
Message-ID: <51E95BEC.5080703@digi.com>
Date: Fri, 19 Jul 2013 17:31:56 +0200
From: Hector Palacios <hector.palacios@...i.com>
To: Marek Vasut <marex@...x.de>
CC: "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devicetree-discuss@...ts.ozlabs.org"
<devicetree-discuss@...ts.ozlabs.org>,
"alexandre.belloni@...e-electrons.com"
<alexandre.belloni@...e-electrons.com>,
"jic23@...nel.org" <jic23@...nel.org>,
"lars@...afoo.de" <lars@...afoo.de>,
"fabio.estevam@...escale.com" <fabio.estevam@...escale.com>
Subject: Re: [PATCH v2 5/5] iio: mxs-lradc: add write_raw function to modify
scale
Dear Marek,
On 07/19/2013 04:39 PM, Marek Vasut wrote:
> Dear Hector Palacios,
>
>> Added write_raw function to manipulate the optional divider_by_two
>> through the scaling attribute out of the available scales.
>>
>> Signed-off-by: Hector Palacios <hector.palacios@...i.com>
>> ---
>> drivers/staging/iio/adc/mxs-lradc.c | 55
>> ++++++++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1
>> deletion(-)
>>
>> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
>> b/drivers/staging/iio/adc/mxs-lradc.c index c929733..286cde2 100644
>> --- a/drivers/staging/iio/adc/mxs-lradc.c
>> +++ b/drivers/staging/iio/adc/mxs-lradc.c
>> @@ -144,6 +144,7 @@ struct mxs_lradc {
>>
>> uint32_t vref_mv[LRADC_MAX_TOTAL_CHANS];
>> unsigned int scale_avail[LRADC_MAX_TOTAL_CHANS][2][2];
>> + unsigned int is_divided[LRADC_MAX_TOTAL_CHANS];
>
> Why not use bitfield ? ;-)
This is used in some math below and an unsigned int looked more appropriate:
case IIO_CHAN_INFO_SCALE:
*val = lradc->vref_mv[chan->channel];
*val2 = chan->scan_type.realbits -
lradc->is_divided[chan->channel];
ret = IIO_VAL_FRACTIONAL_LOG2;
break;
>
>> /*
>> * Touchscreen LRADC channels receives a private slot in the CTRL4
>> @@ -202,6 +203,7 @@ struct mxs_lradc {
>> #define LRADC_CTRL1_LRADC_IRQ_OFFSET 0
>>
>> #define LRADC_CTRL2 0x20
>> +#define LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET 24
>> #define LRADC_CTRL2_TEMPSENSE_PWD (1 << 15)
>>
>> #define LRADC_STATUS 0x40
>> @@ -310,7 +312,8 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>> break;
>> case IIO_CHAN_INFO_SCALE:
>> *val = lradc->vref_mv[chan->channel];
>> - *val2 = chan->scan_type.realbits;
>> + *val2 = chan->scan_type.realbits -
>> + lradc->is_divided[chan->channel];
>> ret = IIO_VAL_FRACTIONAL_LOG2;
>> break;
>> default:
>> @@ -323,6 +326,54 @@ static int mxs_lradc_read_raw(struct iio_dev *iio_dev,
>> return ret;
>> }
>>
>> +static int mxs_lradc_write_raw(struct iio_dev *iio_dev,
>> + const struct iio_chan_spec *chan,
>> + int val, int val2, long m)
>> +{
>> + struct mxs_lradc *lradc = iio_priv(iio_dev);
>> + int ret;
>> +
>> + ret = mutex_trylock(&lradc->lock);
>> + if (!ret)
>> + return -EBUSY;
>> +
>> + switch (m) {
>> + case IIO_CHAN_INFO_SCALE:
>> + ret = -EINVAL;
>> + if (val == lradc->scale_avail[chan->channel][0][0] &&
>> + val2 == lradc->scale_avail[chan->channel][0][1]) {
>> + /* [0] -> divider by two disabled */
>
> This comment is confusing, you use [0] in different dimensions of the array. So
> is the stuff below.
>
> Still, how does this even work, can you show me and example how to set the
> divider from userland ? Sorry, I'm a bit confused with this 3D-business here,
> even if the comment in previous patch made it a bit clearer. Actually you can
> use some #define to specify if you're accessing div/2 or div/1 portion of the
> array to make it more readable.
>
> Like ... scale_avail[chan->channel][LRADC_DIV_BY_2][LRADC_DECIMAL_PART] ...
> would by nice.
Agreed.
How it works:
# cd /sys/devices/80000000.apb/80040000.apbx/80050000.lradc/iio:device0
Here you have three entries per channel:
in_voltageX_raw -> the sample raw value
in_voltageX_scale -> the scale to multiply the raw value to get the voltage in mV
in_voltageX_scale_available -> lists the available scales of the channel
For example for channel 0:
# cat in_voltage0_scale_available
0.451660156 0.903320312 (two scales available, first with divider by two disabled,
second with divider by two enabled)
# cat in_voltage0_scale
0.451660156 (divider by two is currently disabled)
# cat in_voltage0_raw (shows measured value)
1000
If you multiply the value by the scale you get: 1000 * 0.451660156 = 451.6 mV
# echo 0.903320312 > in_voltage0_scale (enables the divider by two)
# cat in_voltage0_raw (shows measured value)
500
Voltage at channel is the same but now measured value is applying the scale so it
shows half the value than before. Now if you multiply: 500 * 0.903320312 = 451.6 mV
(the same voltage but you now have a bigger scale and can measure up to 3.7V).
Other channels (like 10 on the MX28) will show different scales because of fixed
predividers.
The multi-dimension array is needed to store the big decimal number.
>> + writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET,
>> + lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_CLR);
>> + lradc->is_divided[chan->channel] = 0;
>> + ret = 0;
>> + } else if (val == lradc->scale_avail[chan->channel][1][0] &&
>> + val2 == lradc->scale_avail[chan->channel][1][1]) {
>> + /* [1] -> divider by two enabled */
>> + writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET,
>> + lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_SET);
>> + lradc->is_divided[chan->channel] = 1;
>> + ret = 0;
>> + }
>> +
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + break;
>> + }
>> +
>> + mutex_unlock(&lradc->lock);
>> +
>> + return ret;
>> +}
>> +
>> +static int mxs_lradc_write_raw_get_fmt(struct iio_dev *iio_dev,
>> + const struct iio_chan_spec *chan,
>> + long m)
>> +{
>> + return IIO_VAL_INT_PLUS_NANO;
>> +}
>> +
>> static ssize_t mxs_lradc_show_scale_available_ch(struct device *dev,
>> struct device_attribute *attr,
>> char *buf,
>> @@ -400,6 +451,8 @@ static const struct attribute_group
>> mxs_lradc_attribute_group = { static const struct iio_info
>> mxs_lradc_iio_info = {
>> .driver_module = THIS_MODULE,
>> .read_raw = mxs_lradc_read_raw,
>> + .write_raw = mxs_lradc_write_raw,
>> + .write_raw_get_fmt = &mxs_lradc_write_raw_get_fmt,
>
> Is this & needed here ?
No it isn't.
Thanks.
Best regards,
--
Hector Palacios
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists