Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v5 0/5] Introduce the Counter character device interface
To:     William Breathitt Gray <vilhelm.gray@gmail.com>, jic23@kernel.org
Cc:     kamel.bouhara@bootlin.com, gwendal@chromium.org,
        alexandre.belloni@bootlin.com, linux-iio@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        linux-stm32@st-md-mailman.stormreply.com,
        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
References: <cover.1601170670.git.vilhelm.gray@gmail.com>
From:   David Lechner <david@lechnology.com>
Message-ID: <caeeb0b2-6b66-b623-98e3-acdc261ec20e@lechnology.com>
Date:   Mon, 12 Oct 2020 19:35:11 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <cover.1601170670.git.vilhelm.gray@gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 9/26/20 9:18 PM, William Breathitt Gray wrote:
> The following are some questions I have about this patchset:
> 
> 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 implicit cast these values via the driver
>     callback parameters; userspace would still use u32 with no issue.
> 
>     In theory this can work because GCC will treat enums are having a
>     32-bit size; but I worry about the possibility of build targets that
>     have -fshort-enums enabled, resulting in enums having a size less
>     than 32 bits. Would this be a problem?

We shouldn't have to worry about userspace programs using -fshort-enums
since that would break all kernel interfaces that use enums, not just
these - so no one should be using that compiler flag.

So I am in favor of using strongly typed enums with u32 as the
"generic" enum member type.

> 
> 2. Should I have reserved members in the userspace structures?
> 
>     The structures in include/uapi/linux/counter.h are available to
>     userspace applications. Should I reserve space in these structures
>     for future additions and usage? Will endianess and packing be a
>     concern here?
> 
Since there doesn't seem to be a large number of counter devices
this probably isn't critical. Are there any aspects of counter
devices in general that couldn't be described with the proposed
structures? For example, could there be components more than two
levels deep (i.e. it would need id, parent id and grandparent id
to describe fully)?