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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).