From: sathyanarayanan kuppuswamy <sathyanarayanan.kuppuswamy@linux.intel.com>
To: Jonathan Cameron <jic23@kernel.org>, pmeerw@pmeerw.net
Cc: linux-iio@vger.kernel.org, srinivas.pandruvada@linux.intel.com
Subject: Re: [PATCH v3 2/6] iio: documentation: Add ABI info for iio event persistence filter
Date: Thu, 09 Apr 2015 16:30:53 -0700 [thread overview]
Message-ID: <55270BAD.7030509@linux.intel.com> (raw)
In-Reply-To: <5526556A.1080609@kernel.org>
On 04/09/2015 03:33 AM, Jonathan Cameron wrote:
> On 09/04/15 01:06, Kuppuswamy Sathyanarayanan wrote:
>> Add ABI info for iio event persistence filter.
>>
>> Setting <n> to persistence filter would need atleast
>> <n> data values outside the upper/lower threshold
>> limits before generating an interrupt.
>>
>> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> ---
>> Documentation/ABI/testing/sysfs-bus-iio | 82 +++++++++++++++++++++++++++++++++
>> 1 file changed, 82 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
>> index 9a70c31..590b1d4 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>> @@ -856,6 +856,88 @@ Description:
>> met before an event is generated. If direction is not
>> specified then this period applies to both directions.
>>
> Please don't mass document cases unless they are actually in use. These should
> get added as and when they become so. Often a particular type of event
> characteristic is actually only implemented on a small subset of channel types
> (where it makes sense).
IMO, persistence filter can be applied for all these cases. But if you
think its better to
add ABI for what we currently use then I will leave only ALS/proxmity
related ABI's here.
+What: /sys/.../events/in_intensity0_thresh_persistence
+What: /sys/.../events/in_proximity0_thresh_persistence
>> +What: /sys/.../events/in_accel_x_thresh_rising_persistence
>> +What: /sys/.../events/in_accel_x_thresh_falling_persistence
>> +hat: /sys/.../events/in_accel_x_roc_rising_persistence
>> +What: /sys/.../events/in_accel_x_roc_falling_persistence
>> +What: /sys/.../events/in_accel_y_thresh_rising_persistence
>> +What: /sys/.../events/in_accel_y_thresh_falling_persistence
>> +What: /sys/.../events/in_accel_y_roc_rising_persistence
>> +What: /sys/.../events/in_accel_y_roc_falling_persistence
>> +What: /sys/.../events/in_accel_z_thresh_rising_persistence
>> +What: /sys/.../events/in_accel_z_thresh_falling_persistence
>> +What: /sys/.../events/in_accel_z_roc_rising_persistence
>> +What: /sys/.../events/in_accel_z_roc_falling_persistence
>> +What: /sys/.../events/in_anglvel_x_thresh_rising_persistence
>> +What: /sys/.../events/in_anglvel_x_thresh_falling_persistence
>> +What: /sys/.../events/in_anglvel_x_roc_rising_persistence
>> +What: /sys/.../events/in_anglvel_x_roc_falling_persistence
>> +What: /sys/.../events/in_anglvel_y_thresh_rising_persistence
>> +What: /sys/.../events/in_anglvel_y_thresh_falling_persistence
>> +What: /sys/.../events/in_anglvel_y_roc_rising_persistence
>> +What: /sys/.../events/in_anglvel_y_roc_falling_persistence
>> +What: /sys/.../events/in_anglvel_z_thresh_rising_persistence
>> +What: /sys/.../events/in_anglvel_z_thresh_falling_persistence
>> +What: /sys/.../events/in_anglvel_z_roc_rising_persistence
>> +What: /sys/.../events/in_anglvel_z_roc_falling_persistence
>> +What: /sys/.../events/in_magn_x_thresh_rising_persistence
>> +What: /sys/.../events/in_magn_x_thresh_falling_persistence
>> +What: /sys/.../events/in_magn_x_roc_rising_persistence
>> +What: /sys/.../events/in_magn_x_roc_falling_persistence
>> +What: /sys/.../events/in_magn_y_thresh_rising_persistence
>> +What: /sys/.../events/in_magn_y_thresh_falling_persistence
>> +What: /sys/.../events/in_magn_y_roc_rising_persistence
>> +What: /sys/.../events/in_magn_y_roc_falling_persistence
>> +What: /sys/.../events/in_magn_z_thresh_rising_persistence
>> +What: /sys/.../events/in_magn_z_thresh_falling_persistence
>> +What: /sys/.../events/in_magn_z_roc_rising_persistence
>> +What: /sys/.../events/in_magn_z_roc_falling_persistence
>> +What: /sys/.../events/in_rot_from_north_magnetic_thresh_rising_persistence
>> +What: /sys/.../events/in_rot_from_north_magnetic_thresh_falling_persistence
>> +What: /sys/.../events/in_rot_from_north_magnetic_roc_rising_persistence
>> +What: /sys/.../events/in_rot_from_north_magnetic_roc_falling_persistence
>> +What: /sys/.../events/in_rot_from_north_true_thresh_rising_persistence
>> +What: /sys/.../events/in_rot_from_north_true_thresh_falling_persistence
>> +What: /sys/.../events/in_rot_from_north_true_roc_rising_persistence
>> +What: /sys/.../events/in_rot_from_north_true_roc_falling_persistence
>> +What: /sys/.../events/in_rot_from_north_magnetic_tilt_comp_thresh_rising_persistence
>> +What: /sys/.../events/in_rot_from_north_magnetic_tilt_comp_thresh_falling_persistence
>> +What: /sys/.../events/in_rot_from_north_magnetic_tilt_comp_roc_rising_persistence
>> +What: /sys/.../events/in_rot_from_north_magnetic_tilt_comp_roc_falling_persistence
>> +What: /sys/.../events/in_rot_from_north_true_tilt_comp_thresh_rising_persistence
>> +What: /sys/.../events/in_rot_from_north_true_tilt_comp_thresh_falling_persistence
>> +What: /sys/.../events/in_rot_from_north_true_tilt_comp_roc_rising_persistence
>> +What: /sys/.../events/in_rot_from_north_true_tilt_comp_roc_falling_persistence
>> +What: /sys/.../events/in_voltageY_supply_thresh_rising_persistence
>> +What: /sys/.../events/in_voltageY_supply_thresh_falling_persistence
>> +What: /sys/.../events/in_voltageY_supply_roc_rising_persistence
>> +What: /sys/.../events/in_voltageY_supply_roc_falling_persistence
>> +What: /sys/.../events/in_voltageY_thresh_rising_persistence
>> +What: /sys/.../events/in_voltageY_thresh_falling_persistence
>> +What: /sys/.../events/in_voltageY_roc_rising_persistence
>> +What: /sys/.../events/in_voltageY_roc_falling_persistence
>> +What: /sys/.../events/in_tempY_thresh_rising_persistence
>> +What: /sys/.../events/in_tempY_thresh_falling_persistence
>> +What: /sys/.../events/in_tempY_roc_rising_persistence
>> +What: /sys/.../events/in_tempY_roc_falling_persistence
>> +What: /sys/.../events/in_accel_x&y&z_mag_falling_persistence
>> +What: /sys/.../events/in_intensity0_thresh_persistence
>> +What: /sys/.../events/in_proximity0_thresh_persistence
>> +What: /sys/.../events/in_activity_still_thresh_rising_persistence
>> +What: /sys/.../events/in_activity_still_thresh_falling_persistence
>> +What: /sys/.../events/in_activity_walking_thresh_rising_persistence
>> +What: /sys/.../events/in_activity_walking_thresh_falling_persistence
>> +What: /sys/.../events/in_activity_jogging_thresh_rising_persistence
>> +What: /sys/.../events/in_activity_jogging_thresh_falling_persistence
>> +What: /sys/.../events/in_activity_running_thresh_rising_persistence
>> +What: /sys/.../events/in_activity_running_thresh_falling_persistence
>> +KernelVersion: 4.0
>> +Contact: linux-iio@vger.kernel.org
>> +Description:
>> + Number of times an event should occur before generating an
>> + interrupt. Persistence filter value can be applied for both
>> + rising/falling threshold based interrupts.
> This still needs clarification as reading the discussion I'm still a little unsure
> what the condition is. I don't think Lars' original query ever got cleanly answered
Sorry for being unclear. Even in this case, the use case is "number of
consecutive" measurement data
outside the range.
>
> Taking just the upper threshold...
> The interpretations I can think of are:
>
> 1) tsl2591 use case (maps directly to period with the sampling frequency taken into
> account).
> - Too much light for N ALS cycling samples. If it drops below the threshold
> the count is reset.
> the tcs part that Daniel referenced is also this use case.
I agree that it can be mapped to _period if you take the sampling
frequency into account. But if you do that then it becomes dependent on
integration time ( for ALS case). Since _period is calculated based on
frequency, each time you change _integration_time then internally you
need to change the _period value to keep the ratio constant. Also you
need to consider what happens if your frequency > period. I think this
makes is bit complicated.
Don't you think its better to add another representation instead of
trying to fit it in existing ABI's ?
Following are the persistence filter descriptions from data sheets of
tsl2591 and ltr501.
LTR501: " The INTERRUPT PERSIST register controls the N number of times
the measurement data is outside the range defined by the upper and lower
threshold limits before asserting the INT".
TSL2591: " The Interrupt persistence filter sets the number of
consecutive out-of-range ALS cycles necessary to generate an interrupt"
>
> 2) A new option which doesn't take account of the signal dropping below the threshold.
> So a count of number of times above the threshold.
> i.e. On each cycle, if over threshold, increment N.
> Evaluate if N is greater than 'persistence' then trigger an event.
> Why this would ever make sense on a light sensor is beyond me ;) but there
> we are. We have a related abi for in_steps_change_value, which fires
> an event, only every N steps. It doesn't really adapt to this case though.
>
> The name persistence is definitely misleading if this is what the liteon parts
> do as that definitely implies case 1).
>
> Welcome to my life, bludgeoning what appears to new ABI into what we already have!
> (not a lot of point in standardized ABI otherwise!)
>
> Jonathan
>
>
>> +
>> What: /sys/.../events/in_activity_still_thresh_rising_en
>> What: /sys/.../events/in_activity_still_thresh_falling_en
>> What: /sys/.../events/in_activity_walking_thresh_rising_en
>>
>
--
Sathyanarayanan Kuppuswamy
Android kernel developer
next prev parent reply other threads:[~2015-04-09 23:31 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-09 0:06 [PATCH v3 0/6] Added LTR501 Interrupt support Kuppuswamy Sathyanarayanan
2015-04-09 0:06 ` [PATCH v3 1/6] iio: core : events ABI for specifying interrupt persistence Kuppuswamy Sathyanarayanan
2015-04-09 0:06 ` [PATCH v3 2/6] iio: documentation: Add ABI info for iio event persistence filter Kuppuswamy Sathyanarayanan
2015-04-09 10:33 ` Jonathan Cameron
2015-04-09 23:30 ` sathyanarayanan kuppuswamy [this message]
2015-04-10 5:58 ` Jonathan Cameron
2015-04-10 18:52 ` sathyanarayanan kuppuswamy
2015-04-11 18:38 ` Jonathan Cameron
2015-04-10 6:13 ` Jonathan Cameron
2015-04-09 0:06 ` [PATCH v3 3/6] iio: light: ltr501: Fix alignment to match open parenthesis Kuppuswamy Sathyanarayanan
2015-04-09 0:06 ` [PATCH v3 4/6] iio: ltr501: Add interrupt support Kuppuswamy Sathyanarayanan
2015-04-09 11:02 ` Jonathan Cameron
2015-04-09 0:06 ` [PATCH v3 5/6] iio: ltr501: Add interrupt rate control support Kuppuswamy Sathyanarayanan
2015-04-09 11:08 ` Jonathan Cameron
2015-04-09 0:06 ` [PATCH v3 6/6] iio: ltr501: Add ACPI enumeration support Kuppuswamy Sathyanarayanan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55270BAD.7030509@linux.intel.com \
--to=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=jic23@kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=pmeerw@pmeerw.net \
--cc=srinivas.pandruvada@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.