On Sat, Oct 16, 2021 at 08:33:37PM -0500, David Lechner wrote:
> This adds support for direction to the TI eQEP counter driver. It adds
> both a direction attribute to sysfs and a direction change event to
> the chrdev. The direction change event type is new public API.
>
> Signed-off-by: David Lechner <david@xxxxxxxxxxxxxx>
Just one minor comment below regarding the IRQ handler; the rest of the
patch is fine.
> ---
> drivers/counter/ti-eqep.c | 33 +++++++++++++++++++++++++++++++++
> include/uapi/linux/counter.h | 2 ++
> 2 files changed, 35 insertions(+)
>
> diff --git a/drivers/counter/ti-eqep.c b/drivers/counter/ti-eqep.c
> index b7c79435e127..9881e5115da6 100644
> --- a/drivers/counter/ti-eqep.c
> +++ b/drivers/counter/ti-eqep.c
> @@ -106,6 +106,15 @@
> #define QCLR_PCE BIT(1)
> #define QCLR_INT BIT(0)
>
> +#define QEPSTS_UPEVNT BIT(7)
> +#define QEPSTS_FDF BIT(6)
> +#define QEPSTS_QDF BIT(5)
> +#define QEPSTS_QDLF BIT(4)
> +#define QEPSTS_COEF BIT(3)
> +#define QEPSTS_CDEF BIT(2)
> +#define QEPSTS_FIMF BIT(1)
> +#define QEPSTS_PCEF BIT(0)
> +
> /* EQEP Inputs */
> enum {
> TI_EQEP_SIGNAL_QEPA, /* QEPA/XCLK */
> @@ -286,6 +295,9 @@ static int ti_eqep_events_configure(struct counter_device *counter)
> case COUNTER_EVENT_UNDERFLOW:
> qeint |= QEINT_PCU;
> break;
> + case COUNTER_EVENT_DIRECTION_CHANGE:
> + qeint |= QEINT_QDC;
> + break;
> }
> }
>
> @@ -298,6 +310,7 @@ static int ti_eqep_watch_validate(struct counter_device *counter,
> switch (watch->event) {
> case COUNTER_EVENT_OVERFLOW:
> case COUNTER_EVENT_UNDERFLOW:
> + case COUNTER_EVENT_DIRECTION_CHANGE:
> return 0;
> default:
> return -EINVAL;
> @@ -371,11 +384,27 @@ static int ti_eqep_position_enable_write(struct counter_device *counter,
> return 0;
> }
>
> +static int ti_eqep_direction_read(struct counter_device *counter,
> + struct counter_count *count,
> + enum counter_count_direction *direction)
> +{
> + struct ti_eqep_cnt *priv = counter->priv;
> + u32 qepsts;
> +
> + regmap_read(priv->regmap16, QEPSTS, &qepsts);
> +
> + *direction = (qepsts & QEPSTS_QDF) ? COUNTER_COUNT_DIRECTION_FORWARD
> + : COUNTER_COUNT_DIRECTION_BACKWARD;
> +
> + return 0;
> +}
> +
> static struct counter_comp ti_eqep_position_ext[] = {
> 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),
> };
>
> static struct counter_signal ti_eqep_signals[] = {
> @@ -442,6 +471,10 @@ static irqreturn_t ti_eqep_irq_handler(int irq, void *dev_id)
> if (qflg & QFLG_PCU)
> counter_push_event(counter, COUNTER_EVENT_UNDERFLOW, 0);
>
> + if (qflg & QFLG_QDC)
> + counter_push_event(counter, COUNTER_EVENT_DIRECTION_CHANGE, 0);
> +
> +
> regmap_set_bits(priv->regmap16, QCLR, ~0);
As mentioned in the previous patch comments, you should try if possible
to clear only the interrupt flags for the events that you're actually
handling here.
William Breathitt Gray
>
> return IRQ_HANDLED;
> diff --git a/include/uapi/linux/counter.h b/include/uapi/linux/counter.h
> index d0aa95aeff7b..36dd3b474d09 100644
> --- a/include/uapi/linux/counter.h
> +++ b/include/uapi/linux/counter.h
> @@ -61,6 +61,8 @@ enum counter_event_type {
> COUNTER_EVENT_THRESHOLD,
> /* Index signal detected */
> COUNTER_EVENT_INDEX,
> + /* Direction change detected */
> + COUNTER_EVENT_DIRECTION_CHANGE,
> };
>
> /**
> --
> 2.25.1
>
Attachment:
signature.asc
Description: PGP signature
