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:   Mon, 27 Sep 2021 20:33:00 +0900
From:   William Breathitt Gray <vilhelm.gray@gmail.com>
To:     Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Cc:     Jonathan Cameron <jic23@kernel.org>,
        linux-stm32@st-md-mailman.stormreply.com, kernel@pengutronix.de,
        a.fatoum@pengutronix.de, kamel.bouhara@bootlin.com,
        gwendal@chromium.org, alexandre.belloni@bootlin.com,
        david@lechnology.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, o.rempel@pengutronix.de,
        jarkko.nikula@linux.intel.com,
        Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: [PATCH v16 07/14] counter: Add character device interface
Message-ID: <YVGr7PQQThADPNSW@shinobu>
References: <cover.1630031207.git.vilhelm.gray@gmail.com>
 <422c765c91d060cdebc4f17f7aeb255d9c1a4e16.1630031207.git.vilhelm.gray@gmail.com>
 <20210912171821.54af145b@jic23-huawei>
 <YUhdyRdzuBtUxOzT@shinobu>
 <20210926161542.5cf99b58@jic23-huawei>
 <YVGbHQnpBTQYm/7/@shinobu>
 <20210927122000.00007d65@Huawei.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="qFJe+gs0QSTquwqB"
Content-Disposition: inline
In-Reply-To: <20210927122000.00007d65@Huawei.com>
Precedence: bulk


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

On Mon, Sep 27, 2021 at 12:20:00PM +0100, Jonathan Cameron wrote:
> On Mon, 27 Sep 2021 19:21:17 +0900
> William Breathitt Gray <vilhelm.gray@gmail.com> wrote:
>=20
> > On Sun, Sep 26, 2021 at 04:15:42PM +0100, Jonathan Cameron wrote:
> > > On Mon, 20 Sep 2021 19:09:13 +0900
> > > William Breathitt Gray <vilhelm.gray@gmail.com> wrote:
> > >  =20
> > > > On Sun, Sep 12, 2021 at 05:18:42PM +0100, Jonathan Cameron wrote: =
=20
> > > > > On Fri, 27 Aug 2021 12:47:51 +0900
> > > > > William Breathitt Gray <vilhelm.gray@gmail.com> wrote:
> > > > >    =20
> > > > > > This patch introduces a character device interface for the Coun=
ter
> > > > > > subsystem. Device data is exposed through standard character de=
vice read
> > > > > > operations. Device data is gathered when a Counter event is pus=
hed by
> > > > > > the respective Counter device driver. Configuration is handled =
via ioctl
> > > > > > operations on the respective Counter character device node.
> > > > > >=20
> > > > > > Cc: David Lechner <david@lechnology.com>
> > > > > > Cc: Gwendal Grignou <gwendal@chromium.org>
> > > > > > Cc: Dan Carpenter <dan.carpenter@oracle.com>
> > > > > > Cc: Oleksij Rempel <o.rempel@pengutronix.de>
> > > > > > Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> =
  =20
> > > > >=20
> > > > > Hi William,
> > > > >=20
> > > > > Why the bit based lock?  It feels like a mutex_trylock() type app=
roach or
> > > > > spinlock_trylock() would be a more common solution to this proble=
m.
> > > > > There is precedence for doing what you have here though so I'm no=
t that
> > > > > worried about it.   =20
> > > >=20
> > > > Hi Jonathan,
> > > >=20
> > > > We originally used a mutex for this, but Jarkko discovered that this
> > > > produced a warning because chrdev_lock would be held when returning=
 to
> > > > user space:
> > > > https://lore.kernel.org/linux-arm-kernel/YOq19zTsOzKA8v7c@shinobu/T=
/#m6072133d418d598a5f368bb942c945e46cfab9a5
> > > >=20
> > > > Following David Lechner's suggestion, I decided to reimplement
> > > > chrdev_lock as a bitmap using an atomic flag. =20
> > >=20
> > > Ok.  I'm not sure bit lock was quite what was intended (as there is o=
nly one of them)
> > > but I suppose it doesn't greatly matter. =20
> >=20
> > It didn't cross my mind before, but would declaring chrdev_lock as an
> > atomic_t be a more appropriate solution here because we have only one
> > flag?
> >=20
> > William Breathitt Gray
> >=20
>=20
> It would be less esoteric.  This was the first time I've ever come across=
 the bitlock stuff
> whereas atomics are an every day thing.
>=20
> Thanks,
>=20
> Jonathan

I agree. I'll try that out then and reimplement this using
atomic_inc_and_test() instead of test_and_set_bit_lock().

William Breathitt Gray

--qFJe+gs0QSTquwqB
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEk5I4PDJ2w1cDf/bghvpINdm7VJIFAmFRq84ACgkQhvpINdm7
VJKSxw/+K++TD+bpwz9akNC7gg8dm4YlOS7y97Z1JAkS+KcKMbTpdrFJJJbrtIIq
3JJi5iMjTtTsAPPl6Wa1qRCst3ryHR7UTEMJdAy2IR08If2piGn2frklSFH68ejU
0zrSBXnmGPmnVdLudetma4gd6MRY6VVNm8ncTbevE7kgo9QJsYvEFxRsrrf/zeHK
WRBTPNexTxSuclwr7WO612XFr7cUg3iIjdyDw2NMZplkLyFR/HSuw5pX62RFKAe3
z7NhkBsYF+FK4IraGP+2HlUHZ0v02iK86uGgUf4gmFM0tUHcEpN78Gd8lMm1XmE2
bIhnhm3vGrgcdINfcaKIHWTw6l8Xs7WjIlI8cHP8Mmw04euXtcAK2RFxxBdjn8kq
47yxGD2tReXUGSBuv7lXeja+kJ5CU1GHOVcetCQ7qjGXl8bxR/3W0vhetb1uxtLn
LEfXV18JF2eFuXA0vV0fX9BctKWPREOUFGoB+dE/LhlKUTFg/8E2aVFcD3pg7z9I
TBtpGA/mNREX9v4eTDvT5G+cYM44hqfYMMwktyk1pomFfU1L7GFBtNs1l5pU1HvR
UoTUkDJw12nIBwcOSslERLdsoikGTyrtoq3NXebrwI2CTL9sTuQ4SkLNp5m7GASn
4GCmOs7Gnc3vUL4E6jwgKclaJqCxtDSfN/PmYQ43qJT6p0bHrIU=
=xydv
-----END PGP SIGNATURE-----

--qFJe+gs0QSTquwqB--