| Message ID | 20211017013343.3385923-3-david@lechnology.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | counter: ti-eqep: implement features for speed measurement | expand |
On Sat, 16 Oct 2021 20:33:37 -0500 David Lechner <david@lechnology.com> 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@lechnology.com> Trivial comment inline. > --- > drivers/counter/ti-eqep.c | 33 +++++++++++++++++++++++++++++++++ > include/uapi/linux/counter.h | 2 ++ > 2 files changed, 35 insertions(+) > ... > 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); > + Nitpick. One blank line is enough here. > + > regmap_set_bits(priv->regmap16, QCLR, ~0); > > 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, > }; > > /**
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@lechnology.com> 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 >
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); 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, }; /**
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@lechnology.com> --- drivers/counter/ti-eqep.c | 33 +++++++++++++++++++++++++++++++++ include/uapi/linux/counter.h | 2 ++ 2 files changed, 35 insertions(+)