From: Jonathan Cameron <jic23@kernel.org>
To: William Breathitt Gray <vilhelm.gray@gmail.com>
Cc: benjamin.gaignard@st.com, fabrice.gasnier@st.com,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v6 3/9] docs: Add Generic Counter interface documentation
Date: Tue, 22 May 2018 18:08:05 +0100 [thread overview]
Message-ID: <20180522180805.2b61f0ed@archlinux> (raw)
In-Reply-To: <20180521134729.GB5723@sophia>
...
> >> +
> >> +COUNT
> >> +-----
> >> +A Count represents the count data for a set of Signals. The Generic
> >> +Counter interface provides the following available count data types:
> >> +
> >> +* COUNT_POSITION_UNSIGNED:
> >> + Unsigned integer value representing position.
> >> +
> >> +* COUNT_POSITION_SIGNED:
> >> + Signed integer value representing position.
> >
> >Just a thought: As the '0' position is effectively arbitrary is there any
> >actual difference between signed and unsigned? If we defined all counters
> >to be unsigned and just offset any signed ones so the range still fitted
> >would we end up with a simpler interface - there would be no real loss
> >of meaning that I can see.. I suppose it might not be what people expect
> >though when they see their counters start at a large positive value...
>
> This is something I've been on the fence about for a while. I would
> actually prefer the interface to have simply a COUNT_POSITION data type
> to represent position and leave it as unsigned; distinguishing between
> signed and unsigned position seems ultimately arbitrary given that it's
> mathematically just an offset as you have pointed out.
>
> If we were to go down this route, then we'd have a count value that may
> always be represented using an unsigned data type, with an offset value
> that may always be represented using a signed data type -- the
> relationship being such: position = count + offset. Is that correct?
Given the positions are all arbitrary (such as limits etc) there is no
real need to have 0 as in anyway special. So you could just apply an
offset to everything then don't make them visible to userspace at all.
>
> My reason for giving the option for either signed or unsigned position
> was to help minimize the work userspace would have to do in order to get
> the value in which they're actually interested. I suppose it was a
> question of how abstract I want to make the interface -- although,
> making it simpler for userspace put more of a burden on the kernel side.
>
> However, the "offset" value route may actually be more robust in the end
> because userspace would have to know whether they want a signed or
> unsigned position regardless in order to parse, so with count and offet
> defined we know they will always be unsigned and signed respectively.
Hmm. It's not obvious to me which is the best option.
>
> >
> >
> >
> >
> >> +
> >> +A Count has a count function mode which represents the update behavior
> >> +for the count data. The Generic Counter interface provides the following
> >> +available count function modes:
> >> +
> >> +* Increase:
> >> + Accumulated count is incremented.
> >> +
> >> +* Decrease:
> >> + Accumulated count is decremented.
> >> +
> >> +* Pulse-Direction:
> >> + Rising edges on quadrature pair signal A updates the respective count.
> >> + The input level of quadrature pair signal B determines direction.
> >> +
> >Perhaps group the quadrature modes for the point of view of this document?
> >Might be slightly easier to read?
> >
> >* Quadrature Modes
> >
> > - x1 A: etc.
> >
> >> +* Quadrature x1 A:
> >> + If direction is forward, rising edges on quadrature pair signal A
> >> + updates the respective count; if the direction is backward, falling
> >> + edges on quadrature pair signal A updates the respective count.
> >> + Quadrature encoding determines the direction.
> >> +
> >> +* Quadrature x1 B:
> >> + If direction is forward, rising edges on quadrature pair signal B
> >> + updates the respective count; if the direction is backward, falling
> >> + edges on quadrature pair signal B updates the respective count.
> >> + Quadrature encoding determines the direction.
> >> +
> >> +* Quadrature x2 A:
> >> + Any state transition on quadrature pair signal A updates the
> >> + respective count. Quadrature encoding determines the direction.
> >> +
> >> +* Quadrature x2 B:
> >> + Any state transition on quadrature pair signal B updates the
> >> + respective count. Quadrature encoding determines the direction.
> >> +
> >> +* Quadrature x2 Rising:
> >> + Rising edges on either quadrature pair signals updates the respective
> >> + count. Quadrature encoding determines the direction.
> >
> >This one I've never met. Really? There are devices who do this form
> >of crazy? It gives really uneven counting and I'm failing to see when
> >it would ever make sense... References for these odd corner cases
> >would be good.
> >
> >
> >__|---|____|-----|____
> >____|----|____|-----|____
> >
> >001122222223334444444
>
> That's the same reaction I had when I discovered this -- in fact the
> STM32 LP Timer is the first time I've come across such a quadrature
> mode. I'm not sure of the use case for this mode, because positioning
> wouldn't be precise as you've pointed out. Perhaps Fabrice or Benjamin
> can probe the ST guys responsible for this design choice to figure out
> the rationale.
Hmm. My inclination would be to not support it unless someone can up
with a meaningful use. We are adding ABI (be it not much) for a case
that to us makes no sense.
Looks rather like the sort of thing that is a side effect of the
implementation rather than deliberate.
>
> I'm leaving in these modes for now, as they do exist in the STM32 LP
> Timer, but it does make me curious what the intentions for them were
> (perhaps use cases outside of traditional quadrature encoder
> positioning).
>
Sure if there is a usecase then fair enough.
Jonathan
next prev parent reply other threads:[~2018-05-22 17:08 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-16 17:50 [PATCH v6 0/9] Introduce the Counter subsystem William Breathitt Gray
2018-05-16 17:50 ` [PATCH v6 1/9] counter: Introduce the Generic Counter interface William Breathitt Gray
2018-05-20 15:06 ` Jonathan Cameron
2018-05-21 13:06 ` William Breathitt Gray
2018-05-16 17:50 ` [PATCH v6 2/9] counter: Documentation: Add Generic Counter sysfs documentation William Breathitt Gray
2018-05-20 15:12 ` Jonathan Cameron
2018-05-16 17:51 ` [PATCH v6 3/9] docs: Add Generic Counter interface documentation William Breathitt Gray
2018-05-19 15:30 ` kbuild test robot
2018-05-20 15:31 ` Jonathan Cameron
2018-05-21 13:47 ` William Breathitt Gray
2018-05-22 17:08 ` Jonathan Cameron [this message]
2018-05-25 9:26 ` Fabrice Gasnier
2018-05-25 13:00 ` William Breathitt Gray
2018-05-16 17:51 ` [PATCH v6 4/9] counter: 104-quad-8: Add Generic Counter interface support William Breathitt Gray
2018-05-19 13:16 ` kbuild test robot
2018-05-19 13:55 ` William Breathitt Gray
2018-05-19 13:16 ` [RFC PATCH] counter: 104-quad-8: quad8_ops can be static kbuild test robot
2018-05-20 15:42 ` [PATCH v6 4/9] counter: 104-quad-8: Add Generic Counter interface support Jonathan Cameron
2018-05-21 13:51 ` William Breathitt Gray
2018-05-16 17:51 ` [PATCH v6 5/9] counter: 104-quad-8: Documentation: Add Generic Counter sysfs documentation William Breathitt Gray
2018-05-16 17:51 ` [PATCH v6 6/9] dt-bindings: counter: Document stm32 quadrature encoder William Breathitt Gray
2018-05-17 16:23 ` Rob Herring
2018-05-17 18:07 ` William Breathitt Gray
2018-05-17 18:59 ` Benjamin Gaignard
2018-05-18 16:28 ` Rob Herring
2018-05-20 15:47 ` Jonathan Cameron
2018-06-04 13:03 ` Benjamin Gaignard
2018-05-16 17:52 ` [PATCH v6 7/9] counter: Add STM32 Timer " William Breathitt Gray
2018-05-16 17:52 ` [PATCH v6 8/9] counter: stm32-lptimer: add counter device William Breathitt Gray
2018-05-18 16:34 ` Rob Herring
2018-05-16 17:52 ` [PATCH v6 9/9] iio: counter: Remove IIO counter subdirectory William Breathitt Gray
2018-05-20 15:53 ` Jonathan Cameron
2018-05-21 13:58 ` William Breathitt Gray
2018-05-22 10:44 ` Jonathan Cameron
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=20180522180805.2b61f0ed@archlinux \
--to=jic23@kernel.org \
--cc=benjamin.gaignard@st.com \
--cc=devicetree@vger.kernel.org \
--cc=fabrice.gasnier@st.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=vilhelm.gray@gmail.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).