On 26 October 2015 03:29:46 GMT+00:00, Alison Schofield <amsfield22@xxxxxxxxx> wrote:
>On Sun, Oct 25, 2015 at 10:54:29AM +0000, Jonathan Cameron wrote:
>> On 23/10/15 17:54, Alison Schofield wrote:
>> > Cleanups related to the usage of dummy_scan_elements in the
>> > iio dummy driver:
>> > - change enum names to upper case INDEX_*
>> > - use the enum for IIO_CHAN_SOFT_TIMESTAMP (rm hard coded value)
>> > - comment edited to match changes & follow preferred kernel style
>> >
>> As Greg observed this would be better as three separate patches.
>> Comment inline as well.
>>
>> Jonathan
>
>Greg, Jonathan,
>
>Thanks for the review. I understand it needs to be a set.
>
>I only see 2 patches:
>1) change enum names to be upper case.
>2) replace hardcoded scan_index in IIO_CHAN_SOFT_TIMESTAMP
>
>I don't see the comments as a 3rd, they go with each change.
>ie. when I change the enum names, I update the comments
>right above it to match. Same with Patch 2.
>
>OK?
There are a few other comment syntax changes that should really be in a third patch, trivial though they are.
>
>alison
>
>> > Signed-off-by: Alison Schofield <amsfield22@xxxxxxxxx>
>> > ---
>> > drivers/staging/iio/iio_simple_dummy.c | 22
>++++++++++++----------
>> > drivers/staging/iio/iio_simple_dummy.h | 20
>+++++++++++---------
>> > drivers/staging/iio/iio_simple_dummy_buffer.c | 9 +++++----
>> > 3 files changed, 28 insertions(+), 23 deletions(-)
>> >
>> > diff --git a/drivers/staging/iio/iio_simple_dummy.c
>b/drivers/staging/iio/iio_simple_dummy.c
>> > index 381f90f..5e19068 100644
>> > --- a/drivers/staging/iio/iio_simple_dummy.c
>> > +++ b/drivers/staging/iio/iio_simple_dummy.c
>> > @@ -1,4 +1,4 @@
>> > -/**
>> > +/*
>> > * Copyright (c) 2011 Jonathan Cameron
>> > *
>> > * This program is free software; you can redistribute it and/or
>modify it
>> > @@ -137,7 +137,7 @@ static const struct iio_chan_spec
>iio_dummy_channels[] = {
>> > */
>> > .info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_SAMP_FREQ),
>> > /* The ordering of elements in the buffer via an enum */
>> > - .scan_index = voltage0,
>> > + .scan_index = INDEX_VOLTAGE_0,
>> > .scan_type = { /* Description of storage in buffer */
>> > .sign = 'u', /* unsigned */
>> > .realbits = 13, /* 13 bits */
>> > @@ -176,7 +176,7 @@ static const struct iio_chan_spec
>iio_dummy_channels[] = {
>> > * sampling_frequency
>> > * The frequency in Hz at which the channels are sampled
>> > */
>> > - .scan_index = diffvoltage1m2,
>> > + .scan_index = INDEX_DIFFVOLTAGE_1M2,
>> > .scan_type = { /* Description of storage in buffer */
>> > .sign = 's', /* signed */
>> > .realbits = 12, /* 12 bits */
>> > @@ -194,7 +194,7 @@ static const struct iio_chan_spec
>iio_dummy_channels[] = {
>> > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
>> > .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
>> > .info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_SAMP_FREQ),
>> > - .scan_index = diffvoltage3m4,
>> > + .scan_index = INDEX_DIFFVOLTAGE_3M4,
>> > .scan_type = {
>> > .sign = 's',
>> > .realbits = 11,
>> > @@ -221,7 +221,7 @@ static const struct iio_chan_spec
>iio_dummy_channels[] = {
>> > BIT(IIO_CHAN_INFO_CALIBSCALE) |
>> > BIT(IIO_CHAN_INFO_CALIBBIAS),
>> > .info_mask_shared_by_dir = BIT(IIO_CHAN_INFO_SAMP_FREQ),
>> > - .scan_index = accelx,
>> > + .scan_index = INDEX_ACCELX,
>> > .scan_type = { /* Description of storage in buffer */
>> > .sign = 's', /* signed */
>> > .realbits = 16, /* 16 bits */
>> > @@ -230,10 +230,10 @@ static const struct iio_chan_spec
>iio_dummy_channels[] = {
>> > },
>> > },
>> > /*
>> > - * Convenience macro for timestamps. 4 is the index in
>> > - * the buffer.
>> > + * Convenience macro for timestamps.
>> > + * INDEX_TIMESTAMP is the index in the buffer.
>> > */
>> > - IIO_CHAN_SOFT_TIMESTAMP(4),
>> > + IIO_CHAN_SOFT_TIMESTAMP(INDEX_TIMESTAMP),
>> > /* DAC channel out_voltage0_raw */
>> > {
>> > .type = IIO_VOLTAGE,
>> > @@ -364,8 +364,10 @@ static int iio_dummy_read_raw(struct iio_dev
>*indio_dev,
>> > ret = IIO_VAL_INT_PLUS_MICRO;
>> > break;
>> > case 1:
>> > - /* all differential adc channels ->
>> > - * 0.000001344 */
>> > + /*
>> > + * all differential adc
>> > + * channels -> 0.000001344
>> > + */
>> > *val = 0;
>> > *val2 = 1344;
>> > ret = IIO_VAL_INT_PLUS_NANO;
>> > diff --git a/drivers/staging/iio/iio_simple_dummy.h
>b/drivers/staging/iio/iio_simple_dummy.h
>> > index 5c2f4d0..31b62ad 100644
>> > --- a/drivers/staging/iio/iio_simple_dummy.h
>> > +++ b/drivers/staging/iio/iio_simple_dummy.h
>> > @@ -96,20 +96,22 @@ iio_simple_dummy_events_unregister(struct
>iio_dev *indio_dev)
>> >
>> > #endif /* CONFIG_IIO_SIMPLE_DUMMY_EVENTS*/
>> >
>> > -/**
>> > +/*
>> > * enum iio_simple_dummy_scan_elements - scan index enum
>> > - * @voltage0: the single ended voltage channel
>> > - * @diffvoltage1m2: first differential channel
>> > - * @diffvoltage3m4: second differenial channel
>> > - * @accelx: acceleration channel
>> > + * @INDEX_VOLTAGE_0: the single ended voltage channel
>> > + * @INDEX_DIFFVOLTAGE_1M2: first differential channel
>> > + * @INDEX_DIFFVOLTAGE_3M4: second differential channel
>> > + * @INDEX_ACCELX: acceleration channel
>> > + * @INDEX_TIMESTAMP: timestamp channel
>> > *
>> > * Enum provides convenient numbering for the scan index.
>> > */
>> > enum iio_simple_dummy_scan_elements {
>> > - voltage0,
>> > - diffvoltage1m2,
>> > - diffvoltage3m4,
>> > - accelx,
>> > + INDEX_VOLTAGE_0,
>> Even better would be to prefix these to make it clear they
>> are driver specific. With such generic names one might
>> suspect they come from somewhere in the core.
>>
>> Perhaps
>> DUMMY_INDEX_...
>> ?
>> > + INDEX_DIFFVOLTAGE_1M2,
>> > + INDEX_DIFFVOLTAGE_3M4,
>> > + INDEX_ACCELX,
>> > + INDEX_TIMESTAMP,
>> > };
>> >
>> > #ifdef CONFIG_IIO_SIMPLE_DUMMY_BUFFER
>> > diff --git a/drivers/staging/iio/iio_simple_dummy_buffer.c
>b/drivers/staging/iio/iio_simple_dummy_buffer.c
>> > index 00ed774..7516835 100644
>> > --- a/drivers/staging/iio/iio_simple_dummy_buffer.c
>> > +++ b/drivers/staging/iio/iio_simple_dummy_buffer.c
>> > @@ -27,10 +27,11 @@
>> > /* Some fake data */
>> >
>> > static const s16 fakedata[] = {
>> > - [voltage0] = 7,
>> > - [diffvoltage1m2] = -33,
>> > - [diffvoltage3m4] = -2,
>> > - [accelx] = 344,
>> > + [INDEX_VOLTAGE_0] = 7,
>> > + [INDEX_DIFFVOLTAGE_1M2] = -33,
>> > + [INDEX_DIFFVOLTAGE_3M4] = -2,
>> > + [INDEX_ACCELX] = 344,
>> > + [INDEX_TIMESTAMP] = 50,
>> > };
>> >
>> > /**
>> >
>>
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
