From: Jonathan Cameron <jic23@kernel.org>
To: "Tirdea, Irina" <irina.tirdea@intel.com>
Cc: "Baluta, Daniel" <daniel.baluta@intel.com>,
"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [RFC PATCH 4/8] iio: core: Introduce pedometer STEP counter modifier
Date: Mon, 06 Oct 2014 17:31:21 +0100 [thread overview]
Message-ID: <888C517D-6823-4FB5-870E-484DADAF5230@kernel.org> (raw)
In-Reply-To: <1F3AC3675D538145B1661F571FE1805F199AD91D@IRSMSX105.ger.corp.intel.com>
On October 6, 2014 2:50:13 PM GMT+01:00, "Tirdea, Irina" <irina.tirdea@intel.com> wrote:
>> From: Jonathan Cameron [mailto:jic23@kernel.org]
>> On 02/10/14 14:43, Daniel Baluta wrote:
>> > From: Irina Tirdea <irina.tirdea@intel.com>
>> >
>> > One of the functionalities of a pedometer is a step counter.
>> > The step counter needs to be enabled and then it will count the
>steps
>> > in its hardware register. Whenever the applications need to check
>> > the step count, they will read the step counter register.
>> >
>> > To support this functionality we need a steps attribute that
>> > will export the number of steps.
>> >
>> I'm not keen on multiplexing different types of data onto a single
>activity type.
>> Steps is well enough defined on it's own to have it's own channel
>type.
>>
>> in_steps_input would be fine by me. I suppose steps might mean
>something else
>> though...
>>
>
>Hi Jonathan,
>
>Thanks for the review.
>
>Moving the pedometer part to a new type sounds good to me.
>However, I would prefer to add a new type called pedometer and keep
>steps as a modifier, generating names like in_ped_steps_input for the
>attribute and in_ped_steps_instance_en for the event.
>The reason for this is that for supporting Freescale's MMA9553L we will
>need additional attributes (distance, speed, calories, height, weight)
>that we can add as modifiers to this pedometer type. To keep things
>simple, I did not add these additional attributes to the RFC series,
>but I could do that if you think it would be useful. For this device,
>the motion events (walking, running, jogging, still) also depend on the
>height attribute being set, but we intend to deal with this dependency
>in the driver (using the pedometer's height attribute).
>
>What do you think?
I think I would rather each of these was included as a type rather than a modifier.
There are lots of other ways to measure speed for starters and often user space
won't care where it comes from...
In_speed_raw etc...
Trick where possible is to think about what is measured rather than how. Tends to give
more consistent interfaces.
>
>Thanks,
>Irina
>
>
>> > --- a/drivers/iio/industrialio-core.c
>> > +++ b/drivers/iio/industrialio-core.c
>> > @@ -96,6 +96,7 @@ static const char * const iio_modifier_names[] =
>{
>> > [IIO_MOD_JOGGING] = "jogging",
>> > [IIO_MOD_WALKING] = "walking",
>> > [IIO_MOD_STILL] = "still",
>> > + [IIO_MOD_PED_STEPS] = "steps",
>> > };
>> >
>> > /* relies on pairs of these shared then separate */
>> > diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
>> > index 003638d..ae51780 100644
>> > --- a/include/linux/iio/types.h
>> > +++ b/include/linux/iio/types.h
>> > @@ -65,6 +65,7 @@ enum iio_modifier {
>> > IIO_MOD_JOGGING,
>> > IIO_MOD_WALKING,
>> > IIO_MOD_STILL,
>> > + IIO_MOD_PED_STEPS,
>> > };
>> >
>> > enum iio_event_type {
>> >
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
next prev parent reply other threads:[~2014-10-06 16:31 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-02 13:43 [RFC PATCH] iio: Introduce activity channel Daniel Baluta
2014-10-02 13:43 ` [RFC PATCH 1/8] iio: dummy: Introduce virtual registers for dummy device Daniel Baluta
2014-10-04 12:48 ` Jonathan Cameron
2014-10-06 11:17 ` Daniel Baluta
2014-10-09 19:28 ` Jonathan Cameron
2014-10-19 20:30 ` Hartmut Knaack
2014-10-19 20:39 ` Daniel Baluta
2014-10-02 13:43 ` [RFC PATCH 2/8] iio: core: Introduce IIO_ACTIVITY channel Daniel Baluta
2014-10-04 13:00 ` Jonathan Cameron
2014-10-02 13:43 ` [RFC PATCH 3/8] iio: core: Introduce new MOTION event Daniel Baluta
2014-10-04 13:12 ` Jonathan Cameron
2014-10-06 14:17 ` Daniel Baluta
2014-10-09 19:31 ` Jonathan Cameron
2014-10-11 9:47 ` Daniel Baluta
2014-10-13 9:46 ` Karol Wrona
2014-10-07 10:48 ` Daniel Baluta
2014-10-09 19:37 ` Jonathan Cameron
2014-10-02 13:43 ` [RFC PATCH 4/8] iio: core: Introduce pedometer STEP counter modifier Daniel Baluta
2014-10-04 12:53 ` Jonathan Cameron
2014-10-06 13:50 ` Tirdea, Irina
2014-10-06 16:31 ` Jonathan Cameron [this message]
2014-10-07 13:54 ` Tirdea, Irina
2014-10-02 13:43 ` [RFC PATCH 5/8] iio: core: Introduce ENABLE channel info mask Daniel Baluta
2014-10-02 13:43 ` [RFC PATCH 6/8] iio: core: Introduce new STEP_DETECT event Daniel Baluta
2014-10-04 12:56 ` Jonathan Cameron
2014-10-02 13:43 ` [RFC PATCH 7/8] iio: dummy: Demonstrate the usage of activity channel Daniel Baluta
2014-10-02 13:43 ` [RFC PATCH 8/8] iio: event_monitor: Add support for " Daniel Baluta
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=888C517D-6823-4FB5-870E-484DADAF5230@kernel.org \
--to=jic23@kernel.org \
--cc=daniel.baluta@intel.com \
--cc=irina.tirdea@intel.com \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/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).