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:   Wed, 27 Oct 2021 16:44:15 +0900
From:   William Breathitt Gray <vilhelm.gray@gmail.com>
To:     David Lechner <david@lechnology.com>
Cc:     linux-iio@vger.kernel.org, Robert Nelson <robertcnelson@gmail.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/8] counter/ti-eqep: add support for latched position
Message-ID: <YXkDT2gaFfdIsgTQ@shinobu>
References: <20211017013343.3385923-1-david@lechnology.com>
 <20211017013343.3385923-6-david@lechnology.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="oo+GcfNM/KI+dHM6"
Content-Disposition: inline
In-Reply-To: <20211017013343.3385923-6-david@lechnology.com>
Precedence: bulk


--oo+GcfNM/KI+dHM6
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sat, Oct 16, 2021 at 08:33:40PM -0500, David Lechner wrote:
> This adds support to the TI eQEP counter driver for a latched position.
> This is a new extension that gets the counter count that was recorded
> when an event was triggered. A new device-level latch_mode attribute is
> added to select the trigger. Edge capture unit support will be needed
> to make full use of this, but "Unit timeout" mode can already be used
> to calculate high speeds.
>=20
> The unit timer could also have attributes for latched_time and
> latched_period that use the same trigger. However this is not a use
> case at this time, so they can be added later if needed.

I see that "latched_count" holds the captured counter count; would this
"latched_time" hold the captured unit timer time? If so, does that mean
setting the latch mode to "Unit timeout" always results in a
"latched_time" equal to 0 (assuming that's when the timeout event
triggers)?

>=20
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
>  drivers/counter/ti-eqep.c | 50 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
>=20
> diff --git a/drivers/counter/ti-eqep.c b/drivers/counter/ti-eqep.c
> index 1ba7f3c7cb7e..ef899655ad1d 100644
> --- a/drivers/counter/ti-eqep.c
> +++ b/drivers/counter/ti-eqep.c
> @@ -405,12 +405,28 @@ static int ti_eqep_direction_read(struct counter_de=
vice *counter,
>  	return 0;
>  }
> =20
> +static int ti_eqep_position_latched_count_read(struct counter_device *co=
unter,
> +					       struct counter_count *count,
> +					       u64 *value)
> +{
> +	struct ti_eqep_cnt *priv =3D counter->priv;
> +	u32 qposlat;
> +
> +	regmap_read(priv->regmap32, QPOSLAT, &qposlat);
> +
> +	*value =3D qposlat;
> +
> +	return 0;
> +}
> +
>  static struct counter_comp ti_eqep_position_ext[] =3D {
>  	COUNTER_COMP_CEILING(ti_eqep_position_ceiling_read,
>  			     ti_eqep_position_ceiling_write),
>  	COUNTER_COMP_ENABLE(ti_eqep_position_enable_read,
>  			    ti_eqep_position_enable_write),
>  	COUNTER_COMP_DIRECTION(ti_eqep_direction_read),
> +	COUNTER_COMP_COUNT_U64("latched_count",
> +			       ti_eqep_position_latched_count_read, NULL),
>  };
> =20
>  static struct counter_signal ti_eqep_signals[] =3D {
> @@ -463,6 +479,38 @@ static struct counter_count ti_eqep_counts[] =3D {
>  	},
>  };
> =20
> +static int ti_eqep_latch_mode_read(struct counter_device *counter,
> +					    u32 *value)
> +{
> +	struct ti_eqep_cnt *priv =3D counter->priv;
> +	u32 qepctl;
> +
> +	regmap_read(priv->regmap16, QEPCTL, &qepctl);
> +	*value =3D !!(qepctl & QEPCTL_QCLM);
> +
> +	return 0;
> +}
> +
> +static int ti_eqep_latch_mode_write(struct counter_device *counter,
> +					     u32 value)
> +{
> +	struct ti_eqep_cnt *priv =3D counter->priv;
> +
> +	if (value)
> +		regmap_set_bits(priv->regmap16, QEPCTL, QEPCTL_QCLM);
> +	else
> +		regmap_clear_bits(priv->regmap16, QEPCTL, QEPCTL_QCLM);
> +
> +	return 0;
> +}
> +
> +static const char *const ti_eqep_latch_mode_names[] =3D {
> +	"Read count",
> +	"Unit timeout",
> +};
> +
> +static DEFINE_COUNTER_ENUM(ti_eqep_latch_modes, ti_eqep_latch_mode_names=
);
> +
>  static int ti_eqep_unit_timer_time_read(struct counter_device *counter,
>  				       u64 *value)
>  {
> @@ -553,6 +601,8 @@ static int ti_eqep_unit_timer_enable_write(struct cou=
nter_device *counter,
>  }
> =20
>  static struct counter_comp ti_eqep_device_ext[] =3D {
> +	COUNTER_COMP_DEVICE_ENUM("latch_mode", ti_eqep_latch_mode_read,
> +				ti_eqep_latch_mode_write, ti_eqep_latch_modes),

It seems more appropriate to move this alongside "latched_count" as
Count extension because this is setting the trigger mode to latch the
respective Count's count. Or does this particular extension also affect
the "latched_time" capture for the unit timer?

William Breathitt Gray

>  	COUNTER_COMP_DEVICE_U64("unit_timer_time", ti_eqep_unit_timer_time_read,
>  				ti_eqep_unit_timer_time_write),
>  	COUNTER_COMP_DEVICE_U64("unit_timer_period",
> --=20
> 2.25.1
>=20

--oo+GcfNM/KI+dHM6
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEk5I4PDJ2w1cDf/bghvpINdm7VJIFAmF5AzoACgkQhvpINdm7
VJLgGQ//T/av+naaP5b2gfHOqXf8cEcfqKkcjA+rzE8Jr/JOOULRM6C/8mzjnJ4M
d3JQfY3QbmcC5E14+zjEAgIfR48vrj9MlC53/vaD8uc6Z0JYBo55BVsA4oWGF4pz
joPNrEgN6RSGxuvgDZKXXWDi7DznLfzUhxOevGxJdAgCxHFFUNcwGTChrR4rGnje
OlEvf2D4AVbccAOzxkKwfmcEYaOwO0/LtfoMbDwmskCU8EUxo/Mpl1JklNN90QTB
8BBd1ntPEhYxnJ1PDYVgjBoOdXXG1WY0bsmANRafHEfm9viG1szOmnMmBPVY9tG8
SRpZctjB1kR3novzC9LQEB0Nxor41bVOxvkV2rV8TJ1DzO5poLQFHc1cmX5atXew
ZhMZg/JQCRUKaz6t22XziMMQIq4Vnc4BMPr+TCogW8LC7dOi573U73lfvATpcD57
BA8LRvA/R4GheuQuNPYA5ADRQP/PJtGq/htIILoKAJi6Icfl+fF9mNvN2Pxw9wZK
5+Y17c6l6wdzsoITKhmtZNr9HUIAm3TSS/JdFJZwxnI69XL3wpxThT0KE/NnX8aZ
REEB43sRWWu92xgN5Bi5YktA6uMdjC9IHfwITmLyLnOtDbViv04AHNuZefpwVFwj
fjoWfFnjwXlgg3g25rcOkUD+twNthvtEibL8sjy0XYXlcWorskw=
=u0gb
-----END PGP SIGNATURE-----

--oo+GcfNM/KI+dHM6--