| 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: <201307191817.27102.marex@denx.de>
Date: Fri, 19 Jul 2013 18:17:26 +0200
From: Marek Vasut <marex@...x.de>
To: Hector Palacios <hector.palacios@...i.com>
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 Hector Palacios,
> 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;
Oh ok.
> >> /*
> >>
> >> * 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)
Ok, so I have to remember this value : '0.903320312' in case I want to enable
divide-by-two functionality? Hmmmm ... why would this interface not work:
echo 2 > in_voltage0_scale
or
echo 1 > in_voltage0_scale
?
> # 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.
Yes, understood. Thanks for the explanation, it really helped!
--
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