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, 22 Feb 2021 11:06:49 +0900
From:   William Breathitt Gray <vilhelm.gray@gmail.com>
To:     David Lechner <david@lechnology.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, o.rempel@pengutronix.de
Subject: Re: [PATCH v8 03/22] counter: 104-quad-8: Return error when invalid
 mode during ceiling_write
Message-ID: <YDMRuTfazBev7fon@shinobu>
References: <cover.1613131238.git.vilhelm.gray@gmail.com>
 <2be0c071e7730ea16b8faa1efcd3d0d908e7f9db.1613131238.git.vilhelm.gray@gmail.com>
 <8e7daaad-e824-a823-1465-327cbc90ab23@lechnology.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="/YbOQ52TSX5QvpA5"
Content-Disposition: inline
In-Reply-To: <8e7daaad-e824-a823-1465-327cbc90ab23@lechnology.com>
Precedence: bulk


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

On Sat, Feb 20, 2021 at 10:30:03AM -0600, David Lechner wrote:
> On 2/12/21 6:13 AM, William Breathitt Gray wrote:
> > The 104-QUAD-8 only has two count modes where a ceiling value makes
> > sense: Range Limit and Modulo-N. Outside of these two modes, setting a
> > ceiling value is an invalid operation -- so let's report it as such by
> > returning -EINVAL.
> >=20
> > Fixes: fc069262261c ("counter: 104-quad-8: Add lock guards - generic in=
terface")
> > Cc: Syed Nayyar Waris <syednwaris@gmail.com>
> > Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> > ---
> >   drivers/counter/104-quad-8.c | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> >=20
> > diff --git a/drivers/counter/104-quad-8.c b/drivers/counter/104-quad-8.c
> > index 9691f8612be8..f0608b21196a 100644
> > --- a/drivers/counter/104-quad-8.c
> > +++ b/drivers/counter/104-quad-8.c
> > @@ -714,13 +714,14 @@ static ssize_t quad8_count_ceiling_write(struct c=
ounter_device *counter,
> >   	switch (priv->count_mode[count->id]) {
> >   	case 1:
> >   	case 3:
> > +		mutex_unlock(&priv->lock);
> >   		quad8_preset_register_set(priv, count->id, ceiling);
> > -		break;
> > +		return len;
> >   	}
> >  =20
> >   	mutex_unlock(&priv->lock);
> >  =20
> > -	return len;
> > +	return -EINVAL;
> >   }
> >  =20
> >   static ssize_t quad8_count_preset_enable_read(struct counter_device *=
counter,
> >=20
>=20
> What happens if the register is written to when in the wrong mode?
>=20
> If it doesn't hurt anything, I would suggest always writing the register
> instead so that users don't have to know worry about the fact that the mo=
de
> has to be set first.

Unfortunately this register is used for two different functions: as a
ceiling limit and as a preset value. When we're in the preset mode, the
value of this register will be used to set the counter value when an
interrupt occurs; whereas in ceiling mode, this value is used as the
upper counting limit for the counter. We should only set this value when
in the correct mode, lest we end clobbering the value for the other mode
(e.g. user sets ceiling value, but driver unexpectantly changes the
preset value).

William Breathitt Gray

--/YbOQ52TSX5QvpA5
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEk5I4PDJ2w1cDf/bghvpINdm7VJIFAmAzEbkACgkQhvpINdm7
VJL1CxAA42q+QCnTGNHPlJqhKTHAXIJHaKfmrLkcN2wDB/ceG4KPrs0Gr+Cpuw5B
wF9WlOmZCM8IAS1oC/flSZ0Uaziurx573EsBHCOYeHn1Tu0ITD2UyaqOYdFAmRKJ
PR5p6BdZqiZUeBa21fdMNg30vvkRyS94JcML889YF7KkDcsOHAld8virOD3dabhB
S5Kw/8eXSR05ajgnlLnlgy6BzjzgKWXVNqRWZBQHbDN0JeI6m7vxRMhcIWe4V5QQ
EK0pt1Hl6AsyxRqShZxKcknVRJ1Q2rbhnDIc8N7cxn+HolEBtEtxibh8GDMr4Vyq
xfCtZCnX6fUrp99L/boKOnnXTyPeJBhzpSXsD8+L0vWlHoPM7fxXxJrlj+X4JB0/
VpX/Uc4ub+EGQmidRim6znDBRiHWHcbk8Mirn2fU53ZYf5fwnGb9gxw+5z6WzEFA
RxrQCW8OySzNReQQNUELfxqKFpK2QIl+ox/R0yasDgAry3d55wcMD9kziKIiyi+W
QIbQfSLgI8IjteB8lIRmESQmBVs/FybUCULwElDI5/cta1gYRVBRm506b4Adwcx0
abgc8VK2A/0EySUKYpme61aQikIwyFJxBl7I1zpeY+ZleHqVTQhxsDWFieCMHvvH
GY0CUSAR58XdUskJjxj/N0PqmLHHbaxALhuCsL1nZET2to+WSQ8=
=3uyq
-----END PGP SIGNATURE-----

--/YbOQ52TSX5QvpA5--