| 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: <5470711C.5080707@kernel.org>
Date: Sat, 22 Nov 2014 11:18:52 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Daniel Baluta <daniel.baluta@...el.com>, knaack.h@....de
CC: linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
irina.tirdea@...el.com
Subject: Re: [PATCH v4 6/7] iio: dummy: Demonstrate the usage of new channel
types
On 22/11/14 11:05, Jonathan Cameron wrote:
> On 10/11/14 12:45, Daniel Baluta wrote:
>> Adds support for the new channel types in the dummy driver:
>> * a new channel IIO_ACTIVITY
>> * two state transition events (running and walking)
>> * a new channel IIO_STEPS and support for reading and writing
>> pedometer step counter
>> * step detect event
>> * a new IIO_CHAN_INFO_CALIBHEIGHT mask bit for reading and writing
>> user's height.
>>
>> Signed-off-by: Irina Tirdea <irina.tirdea@...el.com>
>> Signed-off-by: Daniel Baluta <daniel.baluta@...el.com>
> The only bit that makes me wonder in here is allowing writing to the steps
> and activity _input attributes.
>
> Now I can see that it makes sense from the point of view of testing the
> interface and any userspace code, but the question here is whether the
> 'normal IIO intefaces' of this dummy driver should correspond to real sensors
> where writing to input channels isn't ever valid!.
>
> The alternative would be to create some deliberately not standard ABI for
> these with a suitably obvious, 'I don't normally exist' naming - which
> would then need documenting. Perhaps what you have is the best option.
> Pitty there isn't an out_steps_* interface. Human interface control :)
> Or even better out_activity_run_* which would be fun...
>
> Upshot is - leave this be. It's our dummy driver afterall and we can
> change the ABI as much as we like without anyone having a valid argument
> that we broke their system ;)
Applied to the togreg branch of iio.git - initially pushed out as testing.
Nicely done with cc'ing a miss-spelt version of your own email address ;)
>> ---
>> drivers/staging/iio/iio_simple_dummy.c | 199 +++++++++++++++++++++++---
>> drivers/staging/iio/iio_simple_dummy.h | 5 +
>> drivers/staging/iio/iio_simple_dummy_events.c | 43 ++++++
>> 3 files changed, 229 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/staging/iio/iio_simple_dummy.c b/drivers/staging/iio/iio_simple_dummy.c
>> index bf78e6f..10a9e08 100644
>> --- a/drivers/staging/iio/iio_simple_dummy.c
>> +++ b/drivers/staging/iio/iio_simple_dummy.c
>> @@ -69,6 +69,34 @@ static const struct iio_event_spec iio_dummy_event = {
>> .mask_separate = BIT(IIO_EV_INFO_VALUE) | BIT(IIO_EV_INFO_ENABLE),
>> };
>>
>> +/*
>> + * simple step detect event - triggered when a step is detected
>> + */
>> +static const struct iio_event_spec step_detect_event = {
>> + .type = IIO_EV_TYPE_INSTANCE,
>> + .dir = IIO_EV_DIR_NONE,
>> + .mask_separate = BIT(IIO_EV_INFO_ENABLE),
>> +};
>> +
>> +/*
>> + * simple transition event - triggered when the reported running confidence
>> + * value rises above a threshold value
>> + */
>> +static const struct iio_event_spec iio_running_event = {
>> + .type = IIO_EV_TYPE_THRESH,
>> + .dir = IIO_EV_DIR_RISING,
>> + .mask_separate = BIT(IIO_EV_INFO_VALUE) | BIT(IIO_EV_INFO_ENABLE),
>> +};
>> +
>> +/*
>> + * simple transition event - triggered when the reported walking confidence
>> + * value falls under a threshold value
>> + */
>> +static const struct iio_event_spec iio_walking_event = {
>> + .type = IIO_EV_TYPE_THRESH,
>> + .dir = IIO_EV_DIR_FALLING,
>> + .mask_separate = BIT(IIO_EV_INFO_VALUE) | BIT(IIO_EV_INFO_ENABLE),
>> +};
>> #endif
>>
>> /*
>> @@ -215,6 +243,37 @@ static const struct iio_chan_spec iio_dummy_channels[] = {
>> .indexed = 1,
>> .channel = 0,
>> },
>> + {
>> + .type = IIO_STEPS,
>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_ENABLE) |
>> + BIT(IIO_CHAN_INFO_CALIBHEIGHT),
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>> + .scan_index = -1,
>> +#ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
>> + .event_spec = &step_detect_event,
>> + .num_event_specs = 1,
>> +#endif /* CONFIG_IIO_SIMPLE_DUMMY_EVENTS */
>> + },
>> + {
>> + .type = IIO_ACTIVITY,
>> + .modified = 1,
>> + .channel2 = IIO_MOD_RUNNING,
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>> +#ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
>> + .event_spec = &iio_running_event,
>> + .num_event_specs = 1,
>> +#endif /* CONFIG_IIO_SIMPLE_DUMMY_EVENTS */
>> + },
>> + {
>> + .type = IIO_ACTIVITY,
>> + .modified = 1,
>> + .channel2 = IIO_MOD_WALKING,
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>> +#ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
>> + .event_spec = &iio_walking_event,
>> + .num_event_specs = 1,
>> +#endif /* CONFIG_IIO_SIMPLE_DUMMY_EVENTS */
>> + },
>> };
>>
>> /**
>> @@ -263,24 +322,55 @@ static int iio_dummy_read_raw(struct iio_dev *indio_dev,
>> break;
>> }
>> break;
>> + case IIO_CHAN_INFO_PROCESSED:
>> + switch (chan->type) {
>> + case IIO_STEPS:
>> + *val = st->steps;
>> + ret = IIO_VAL_INT;
>> + break;
>> + case IIO_ACTIVITY:
>> + switch (chan->channel2) {
>> + case IIO_MOD_RUNNING:
>> + *val = st->activity_running;
>> + ret = IIO_VAL_INT;
>> + break;
>> + case IIO_MOD_WALKING:
>> + *val = st->activity_walking;
>> + ret = IIO_VAL_INT;
>> + break;
>> + default:
>> + break;
>> + }
>> + break;
>> + default:
>> + break;
>> + }
>> + break;
>> case IIO_CHAN_INFO_OFFSET:
>> /* only single ended adc -> 7 */
>> *val = 7;
>> ret = IIO_VAL_INT;
>> break;
>> case IIO_CHAN_INFO_SCALE:
>> - switch (chan->differential) {
>> - case 0:
>> - /* only single ended adc -> 0.001333 */
>> - *val = 0;
>> - *val2 = 1333;
>> - ret = IIO_VAL_INT_PLUS_MICRO;
>> + switch (chan->type) {
>> + case IIO_VOLTAGE:
>> + switch (chan->differential) {
>> + case 0:
>> + /* only single ended adc -> 0.001333 */
>> + *val = 0;
>> + *val2 = 1333;
>> + ret = IIO_VAL_INT_PLUS_MICRO;
>> + break;
>> + case 1:
>> + /* all differential adc channels ->
>> + * 0.000001344 */
>> + *val = 0;
>> + *val2 = 1344;
>> + ret = IIO_VAL_INT_PLUS_NANO;
>> + }
>> + break;
>> + default:
>> break;
>> - case 1:
>> - /* all differential adc channels -> 0.000001344 */
>> - *val = 0;
>> - *val2 = 1344;
>> - ret = IIO_VAL_INT_PLUS_NANO;
>> }
>> break;
>> case IIO_CHAN_INFO_CALIBBIAS:
>> @@ -298,6 +388,27 @@ static int iio_dummy_read_raw(struct iio_dev *indio_dev,
>> *val2 = 33;
>> ret = IIO_VAL_INT_PLUS_NANO;
>> break;
>> + case IIO_CHAN_INFO_ENABLE:
>> + switch (chan->type) {
>> + case IIO_STEPS:
>> + *val = st->steps_enabled;
>> + ret = IIO_VAL_INT;
>> + break;
>> + default:
>> + break;
>> + }
>> + break;
>> + case IIO_CHAN_INFO_CALIBHEIGHT:
>> + switch (chan->type) {
>> + case IIO_STEPS:
>> + *val = st->height;
>> + ret = IIO_VAL_INT;
>> + break;
>> + default:
>> + break;
>> + }
>> + break;
>> +
>> default:
>> break;
>> }
>> @@ -330,14 +441,45 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev,
>>
>> switch (mask) {
>> case IIO_CHAN_INFO_RAW:
>> - if (chan->output == 0)
>> + switch (chan->type) {
>> + case IIO_VOLTAGE:
>> + if (chan->output == 0)
>> + return -EINVAL;
>> +
>> + /* Locking not required as writing single value */
>> + mutex_lock(&st->lock);
>> + st->dac_val = val;
>> + mutex_unlock(&st->lock);
>> + return 0;
>> + default:
>> return -EINVAL;
>> -
>> - /* Locking not required as writing single value */
>> - mutex_lock(&st->lock);
>> - st->dac_val = val;
>> - mutex_unlock(&st->lock);
>> - return 0;
>> + }
>> + case IIO_CHAN_INFO_PROCESSED:
>> + switch (chan->type) {
>> + case IIO_STEPS:
>> + mutex_lock(&st->lock);
>> + st->steps = val;
>> + mutex_unlock(&st->lock);
>> + return 0;
>> + case IIO_ACTIVITY:
>> + if (val < 0)
>> + val = 0;
>> + if (val > 100)
>> + val = 100;
>> + switch (chan->channel2) {
>> + case IIO_MOD_RUNNING:
>> + st->activity_running = val;
>> + return 0;
>> + case IIO_MOD_WALKING:
>> + st->activity_walking = val;
>> + return 0;
>> + default:
>> + return -EINVAL;
>> + }
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> case IIO_CHAN_INFO_CALIBSCALE:
>> mutex_lock(&st->lock);
>> /* Compare against table - hard matching here */
>> @@ -356,6 +498,24 @@ static int iio_dummy_write_raw(struct iio_dev *indio_dev,
>> st->accel_calibbias = val;
>> mutex_unlock(&st->lock);
>> return 0;
>> + case IIO_CHAN_INFO_ENABLE:
>> + switch (chan->type) {
>> + case IIO_STEPS:
>> + mutex_lock(&st->lock);
>> + st->steps_enabled = val;
>> + mutex_unlock(&st->lock);
>> + return 0;
>> + default:
>> + return -EINVAL;
>> + }
>> + case IIO_CHAN_INFO_CALIBHEIGHT:
>> + switch (chan->type) {
>> + case IIO_STEPS:
>> + st->height = val;
>> + return 0;
>> + default:
>> + return -EINVAL;
>> + }
>>
>> default:
>> return -EINVAL;
>> @@ -395,6 +555,9 @@ static int iio_dummy_init_device(struct iio_dev *indio_dev)
>> st->accel_val = 34;
>> st->accel_calibbias = -7;
>> st->accel_calibscale = &dummy_scales[0];
>> + st->steps = 47;
>> + st->activity_running = 98;
>> + st->activity_walking = 4;
>>
>> return 0;
>> }
>> diff --git a/drivers/staging/iio/iio_simple_dummy.h b/drivers/staging/iio/iio_simple_dummy.h
>> index ad89842..3b714b4 100644
>> --- a/drivers/staging/iio/iio_simple_dummy.h
>> +++ b/drivers/staging/iio/iio_simple_dummy.h
>> @@ -34,9 +34,14 @@ struct iio_dummy_state {
>> int differential_adc_val[2];
>> int accel_val;
>> int accel_calibbias;
>> + int activity_running;
>> + int activity_walking;
>> const struct iio_dummy_accel_calibscale *accel_calibscale;
>> struct mutex lock;
>> struct iio_dummy_regs *regs;
>> + int steps_enabled;
>> + int steps;
>> + int height;
>> #ifdef CONFIG_IIO_SIMPLE_DUMMY_EVENTS
>> int event_irq;
>> int event_val;
>> diff --git a/drivers/staging/iio/iio_simple_dummy_events.c b/drivers/staging/iio/iio_simple_dummy_events.c
>> index 719dfa5..ac15a44 100644
>> --- a/drivers/staging/iio/iio_simple_dummy_events.c
>> +++ b/drivers/staging/iio/iio_simple_dummy_events.c
>> @@ -72,6 +72,22 @@ int iio_simple_dummy_write_event_config(struct iio_dev *indio_dev,
>> st->event_en = state;
>> else
>> return -EINVAL;
>> + default:
>> + return -EINVAL;
>> + }
>> + break;
>> + case IIO_ACTIVITY:
>> + switch (type) {
>> + case IIO_EV_TYPE_THRESH:
>> + st->event_en = state;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> + case IIO_STEPS:
>> + switch (type) {
>> + case IIO_EV_TYPE_INSTANCE:
>> + st->event_en = state;
>> break;
>> default:
>> return -EINVAL;
>> @@ -161,6 +177,33 @@ static irqreturn_t iio_simple_dummy_event_handler(int irq, void *private)
>> IIO_EV_TYPE_THRESH, 0, 0, 0),
>> iio_get_time_ns());
>> break;
>> + case 1:
>> + if (st->activity_running > st->event_val)
>> + iio_push_event(indio_dev,
>> + IIO_EVENT_CODE(IIO_ACTIVITY, 0,
>> + IIO_MOD_RUNNING,
>> + IIO_EV_DIR_RISING,
>> + IIO_EV_TYPE_THRESH,
>> + 0, 0, 0),
>> + iio_get_time_ns());
>> + break;
>> + case 2:
>> + if (st->activity_walking < st->event_val)
>> + iio_push_event(indio_dev,
>> + IIO_EVENT_CODE(IIO_ACTIVITY, 0,
>> + IIO_MOD_WALKING,
>> + IIO_EV_DIR_FALLING,
>> + IIO_EV_TYPE_THRESH,
>> + 0, 0, 0),
>> + iio_get_time_ns());
>> + break;
>> + case 3:
>> + iio_push_event(indio_dev,
>> + IIO_EVENT_CODE(IIO_STEPS, 0, IIO_NO_MOD,
>> + IIO_EV_DIR_NONE,
>> + IIO_EV_TYPE_INSTANCE, 0, 0, 0),
>> + iio_get_time_ns());
>> + break;
>> default:
>> break;
>> }
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
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