Re: [PATCH 2/4] iio: hi8435: avoid garbage event at first enable

[Nikita Yushchenko <nikita.yoush@cogentembedded.com>]

>> +    int ret;
>> +    u32 tmp;
>> +
>> +    if (state) {
>> +        ret = hi8435_readl(priv, HI8435_SO31_0_REG, &tmp);
>> +        if (ret < 0)
>> +            return ret;
>> +        if (tmp & BIT(chan->channel))
>> +            priv->event_prev_val |= BIT(chan->channel);
>> +        else
>> +            priv->event_prev_val &= ~BIT(chan->channel);
>>   -    priv->event_scan_mask &= ~BIT(chan->channel);
>> -    if (state)
>>           priv->event_scan_mask |= BIT(chan->channel);
>> +    } else
>> +        priv->event_scan_mask &= ~BIT(chan->channel);
>>   
> 
> This will race with hi8435_iio_push_event for priv->event_scan_mask.

I was under impression that mutual-exclusion is provided by core. Now I
see it is not. Will submit a fix soon.

> Since you adding raw access then it is reasonable to initialize
> priv->event_prev_val in hi8435_read_raw.

This will cause complex interdependency between events and raw access.
E.g. should we generate event if change was discovered while processing
raw access from user space?  If we do, then we break semantics "events
generated only under trigger".  If we don't, we need complex state handling.

> Then use the following sequence for your application:
> 1. read raw value
> 2. enable events

So what if:
- one application calls read raw,
- then, far later, signal changes,
- then, far later, other application enables events?

Should second application get notification of change that happened long
before in enabled events?  I think no.

There is fundamental race between start-of-monitoring and
change-of-signal-near-start-of-monitoring. To avoid it, reading signal
must be either atomic with start of monitoring (which is not possible
with IIO API), or done after start of monitoring (which moves handling
of this race to user side).

Nikita