Re: [PATCH 4/9] iio: mma8452: Basic support for transient events.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/05/15 15:58, Jonathan Cameron wrote:
On 29/04/15 08:52, Martin Fuzzey wrote:
On 25/02/15 13:25, Jonathan Cameron wrote:
On 19/02/15 14:16, Martin Fuzzey wrote:
The event is triggered when the highpass filtered absolute acceleration
exceeds the threshold.

Signed-off-by: Martin Fuzzey <mfuzzey@xxxxxxxxxxx>
you in_accel_transient_scale isn't documented.
Why do we need this naming at all?
Ah. It's not clear this is just the event rather than the underlying channel.
Sorry, not understanding you here.

Are you saying it's not clear in the documentation (which is the next
patch "iio: doc: Describe scale attributes for event thresholds" that
you've already applied) or in the code?
oops, I suspect I didn't like the naming of the attribute being somewhat
ambiguous.

Also, whilst the next patch is documentation, it doesn't seem to include
this particular attribute...

Ah, I think you are confusing the attribute name and the local variable name.
in_accel_transient_scale is just the local variable name.

The attribute name is events/in_accel_scale and that is documented in the patch you have already merged.


$ git show d1bd4867b0959d5221dc528ccf60c8534aae865d
commit d1bd4867b0959d5221dc528ccf60c8534aae865d
Author: Martin Fuzzey <mfuzzey@xxxxxxxxxxx>
Date:   Thu Feb 19 15:16:04 2015 +0100

    iio: doc: Describe scale attributes for event thresholds

    Signed-off-by: Martin Fuzzey <mfuzzey@xxxxxxxxxxx>
    Signed-off-by: Jonathan Cameron <jic23@xxxxxxxxxx>

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 9a70c31..9230709 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -661,6 +661,24 @@ Description:
                value is in raw device units or in processed units (as _raw
                and _input do on sysfs direct channel read attributes).

+What:          /sys/.../events/in_accel_scale
...


The code is:
/*
  * Threshold is configured in fixed 8G/127 steps regardless of
  * currently selected scale for measurement.
  */
static IIO_CONST_ATTR_NAMED(accel_transient_scale, in_accel_scale, "0.617742");

static struct attribute *mma8452_event_attributes[] = {
     &iio_const_attr_accel_transient_scale.dev_attr.attr,
     NULL,
};

static struct attribute_group mma8452_event_attribute_group = {
     .attrs = mma8452_event_attributes,
     .name = "events",
};

#define MMA8452_CHANNEL(axis, idx) { \
     .type = IIO_ACCEL, \
...
     .num_event_specs = ARRAY_SIZE(mma8452_transient_event), \
}


That seems fairly clear to me.

Perhaps, make it an event parameter instead... e.g. add scale to the ev_info
array like we do for hysteresis etc.
But doing that would create a read/write attribute in sysfs whereas the above code creates a read only attribute and makes it clear that the attribute is a constant through the use of IIO_CONST_ATTR_NAMED

Once this is sorted out I'll send another (hopefully last) respin with the remaining remarks fixed.

Regards,

Martin

Otherwise, looks good to me.

Again, Peter's input would be good.



--
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




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux