From: David Lechner <david@lechnology.com>
To: William Breathitt Gray <vilhelm.gray@gmail.com>, David.Laight@ACULAB.COM
Cc: jic23@kernel.org, kernel@pengutronix.de,
linux-stm32@st-md-mailman.stormreply.com,
a.fatoum@pengutronix.de, kamel.bouhara@bootlin.com,
gwendal@chromium.org, alexandre.belloni@bootlin.com,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, syednwaris@gmail.com,
patrick.havelange@essensium.com, fabrice.gasnier@st.com,
mcoquelin.stm32@gmail.com, alexandre.torgue@st.com
Subject: Re: [PATCH v6 0/5] Introduce the Counter character device interface
Date: Mon, 21 Dec 2020 09:19:17 -0600 [thread overview]
Message-ID: <b10c5ba6-e8c7-96d5-db9f-3828018330f6@lechnology.com> (raw)
In-Reply-To: <X9/Foc6wGl5dR1yK@shinobu>
On 12/20/20 3:44 PM, William Breathitt Gray wrote:
> On Sun, Dec 13, 2020 at 05:15:14PM -0600, David Lechner wrote:
>> On 11/22/20 2:29 PM, William Breathitt Gray wrote:
>>>
>>> 1. Should standard Counter component data types be defined as u8 or u32?
>>>
>>> Many standard Counter component types such COUNTER_COMP_SIGNAL_LEVEL
>>> have standard values defined (e.g. COUNTER_SIGNAL_LEVEL_LOW and
>>> COUNTER_SIGNAL_LEVEL_HIGH). These values are currently handled by the
>>> Counter subsystem code as u8 data types.
>>>
>>> If u32 is used for these values instead, C enum structures could be
>>> used by driver authors to implicitly cast these values via the driver
>>> callback parameters.
>>>
>>> This question is primarily addressed to David Lechner. I'm somewhat
>>> confused about how this setup would look in device drivers. I've gone
>>> ahead and refactored the code to support u32 enums, and pushed it to
>>> a separate branch on my repository called counter_chrdev_v6_u32_enum:
>>> https://gitlab.com/vilhelmgray/iio/-/tree/counter_chrdev_v6_u32_enum
>>>
>>> Please check it out and let me know what you think. Is this the
>>> support you had in mind? I'm curious to see an example of how would
>>> your driver callback functions would look in this case. If everything
>>> works out fine, then I'll submit this branch as v7 of this patchset.
>>
>> I haven't had time to look at this in depth, but just superficially looking
>> at it, it is mostly there. The driver callback would just use the enum type
>> in place of u32. For example:
>>
>> static int ti_eqep_function_write(struct counter_device *counter,
>> struct counter_count *count,
>> enum counter_function function)
>>
>> and the COUNTER_FUNCTION_* constants would be defined as:
>>
>> enum counter_function {
>> COUNTER_FUNCTION_INCREASE,
>> ...
>> };
>>
>> instead of using #define macros.
>>
>> One advantage I see to using u8, at least in the user API data structures,
>> is that it increases the number of events that fit in the kfifo buffer by
>> a significant factor.
>>
>> And that is not to say that we couldn't do both: have the user API structs
>> use u8 for enum values and still use u32/strong enum types internally in
>> the callback functions.
>
> I'm including David Laight because he initially opposed enums in favor
> of fixed size types when we discussed this in an earlier revision:
> https://lkml.org/lkml/2020/5/3/159
>
> However, there have been significant changes to this patchset so the
> context now is different than those earlier discussions (i.e. we're no
> longer discussing ioctl calls).
>
> I think reimplementing these constants as enums as described could work.
> If we do so, should the enum constants be given specific values? For
> example:
>
> enum counter_function {
> COUNTER_FUNCTION_INCREASE = 0,
> COUNTER_FUNCTION_DECREASE = 1,
> ...
> };
I would say no on the explicit values since they don't have
any significant meaning.
prev parent reply other threads:[~2020-12-21 15:20 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-22 20:29 [PATCH v6 0/5] Introduce the Counter character device interface William Breathitt Gray
2020-11-22 20:29 ` [PATCH v6 2/5] docs: counter: Update to reflect sysfs internalization William Breathitt Gray
2020-11-22 20:29 ` [PATCH v6 3/5] counter: Add character device interface William Breathitt Gray
2020-12-13 23:58 ` David Lechner
2020-12-25 17:30 ` William Breathitt Gray
2021-01-19 9:20 ` Oleksij Rempel
2021-01-21 8:03 ` William Breathitt Gray
2021-01-21 18:26 ` Jonathan Cameron
2020-11-22 20:29 ` [PATCH v6 4/5] docs: counter: Document " William Breathitt Gray
2020-11-22 20:29 ` [PATCH v6 5/5] counter: 104-quad-8: Add IRQ support for the ACCES 104-QUAD-8 William Breathitt Gray
[not found] ` <950660d49af7d12b09bc9d3b1db6f8ff74209c26.1606075915.git.vilhelm.gray@gmail.com>
2020-11-25 13:07 ` [PATCH v6 1/5] counter: Internalize sysfs interface code William Breathitt Gray
2020-12-13 23:15 ` David Lechner
2020-12-20 22:11 ` William Breathitt Gray
2020-12-21 15:26 ` David Lechner
2020-12-13 23:15 ` [PATCH v6 0/5] Introduce the Counter character device interface David Lechner
2020-12-20 21:44 ` William Breathitt Gray
2020-12-21 15:19 ` David Lechner [this message]
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=b10c5ba6-e8c7-96d5-db9f-3828018330f6@lechnology.com \
--to=david@lechnology.com \
--cc=David.Laight@ACULAB.COM \
--cc=a.fatoum@pengutronix.de \
--cc=alexandre.belloni@bootlin.com \
--cc=alexandre.torgue@st.com \
--cc=fabrice.gasnier@st.com \
--cc=gwendal@chromium.org \
--cc=jic23@kernel.org \
--cc=kamel.bouhara@bootlin.com \
--cc=kernel@pengutronix.de \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=mcoquelin.stm32@gmail.com \
--cc=patrick.havelange@essensium.com \
--cc=syednwaris@gmail.com \
--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).