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;
Date:   Sun, 18 Oct 2020 10:14:01 -0400
From:   William Breathitt Gray <vilhelm.gray@gmail.com>
To:     David Lechner <david@lechnology.com>
Cc:     jic23@kernel.org, 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
Subject: Re: [PATCH v5 0/5] Introduce the Counter character device interface
Message-ID: <20201018141401.GA231549@shinobu>
References: <cover.1601170670.git.vilhelm.gray@gmail.com>
 <caeeb0b2-6b66-b623-98e3-acdc261ec20e@lechnology.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="GvXjxJ+pjyke8COw"
Content-Disposition: inline
In-Reply-To: <caeeb0b2-6b66-b623-98e3-acdc261ec20e@lechnology.com>
Precedence: bulk


--GvXjxJ+pjyke8COw
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Oct 12, 2020 at 07:35:11PM -0500, David Lechner wrote:
> On 9/26/20 9:18 PM, William Breathitt Gray wrote:
> > The following are some questions I have about this patchset:
> >=20
> > 1. Should standard Counter component data types be defined as u8 or u32?
> >=20
> >     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 t=
he
> >     Counter subsystem code as u8 data types.
> >=20
> >     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.
> >=20
> >     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?
>=20
> 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.
>=20
> So I am in favor of using strongly typed enums with u32 as the
> "generic" enum member type.

That reasoning pacifies my worries. I'll test out strongly typed enums
in the next revision of this patchset.

> >=20
> > 2. Should I have reserved members in the userspace structures?
> >=20
> >     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?
> >=20
> 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)?

I can't think of any devices that would require more depth, so we
probably don't need any additional members added to the userspace
structures. The current structure should be flexible enough for future
expansion, and any additional functionality we come across can be
handled by implementing new extension types as we have for the sysfs
attributes.

William Breathitt Gray

--GvXjxJ+pjyke8COw
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEk5I4PDJ2w1cDf/bghvpINdm7VJIFAl+MTZ8ACgkQhvpINdm7
VJK9LBAAge9m8K8GflWtmFR6Fs3MjZ4VRmuFs6SXmkK0zYWPpaw0vy5Is6poej9f
p2hXz0EBxEVSdD+/hMmnyrz3VeLrRrSKN4iekMxGpScZoiyjeh0wy9J0kcv01zTd
jYR8Jm4lCJShMW20HFHUZXR3gUGTzXXB6jot2jDk+UMAkISYK7msNt4nmb6WsaU0
4QGpNZE0uznJN1PZ8pzv4NZ7LzZpf/HDDU6XgCIT9lWrQIbVXiOXG9aeXA0nL8cJ
WmBwtSZ5KQJo9Ijjo1Rn6zhNUoKMzF1wvGNEHvpvJDZ8n9x4ljyVh9FsEMUV9MC4
jWgJMA2+7vRbrheBqAuLTB2rC46xZuSr3cS7mu9sY+DO+LSNVafLWU8vjzzK5tfX
ahPxfm7Bz/lX8zhddNPWL1t7aNFMCJJdLAmRIlHewf9mdoE0n35ifyvOpgAACtIn
qFgr0TkpZdwq7TG3FCNqXRE80cw0YWZUIXeZr0MFoETzHPs50VvFDBQB/xKU1/wo
qdV2OZAn5bdfbk/DMqg9S61+H3KbujUgQZ+k+ANB4UMKYdo3QyNgBNUpFQgUqlp2
rqFuAZTNVG1VY2mJcA+8QJe4eQkn3oEGzgvK29ruiysk6nk/GjEhtJ8qJ+NLqw7q
HpTWR26ANLXz6Iy7izaSHM3jLiAp675Ej6bDzkZRf74d+RoKf5Y=
=oiqw
-----END PGP SIGNATURE-----

--GvXjxJ+pjyke8COw--