From: William Breathitt Gray <vilhelm.gray@gmail.com>
To: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: linux-iio@vger.kernel.org
Subject: Re: [PATCH v3 2/2] counter: introduce support for Intel QEP Encoder
Date: Wed, 2 Oct 2019 14:11:05 -0400 [thread overview]
Message-ID: <20191002181105.GB100041@icarus> (raw)
In-Reply-To: <20191001093237.775608-2-felipe.balbi@linux.intel.com>
On Tue, Oct 01, 2019 at 12:32:37PM +0300, Felipe Balbi wrote:
> Add support for Intel PSE Quadrature Encoder
>
> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> ---
> Changes since v1:
> - Many more private sysfs files converted over to counter interface
>
> Changes since v2:
> - Completed conversion to counter framework
> - Removed Capture Compare for now
> - Removed RFC from subject
>
> drivers/counter/Kconfig | 6 +
> drivers/counter/Makefile | 1 +
> drivers/counter/intel-qep.c | 689 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 696 insertions(+)
> create mode 100644 drivers/counter/intel-qep.c
Hi Felipe,
Make sure to add an entry for this driver in the top-level MAINTAINERS
file so that we have a way to contact the maintainer in the future.
> diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
> index 2967d0a9ff91..f280cd721350 100644
> --- a/drivers/counter/Kconfig
> +++ b/drivers/counter/Kconfig
> @@ -59,4 +59,10 @@ config FTM_QUADDEC
> To compile this driver as a module, choose M here: the
> module will be called ftm-quaddec.
>
> +config INTEL_QEP
> + tristate "Intel Quadrature Encoder"
> + depends on PCI
> + help
> + Support for Intel Quadrature Encoder Devices
This help text seems rather short -- are you able to expand it a bit
further? The current description here gives me the impression that this
is a generic driver for all Intel quadrature encoder devices in
existence, but I suspect the Intel QEP counter driver has a much
narrower list of devices it supports; perhaps you can list those support
device models here.
> +
> endif # COUNTER
> diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile
> index 40d35522937d..cf291cfd8cf0 100644
> --- a/drivers/counter/Makefile
> +++ b/drivers/counter/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_104_QUAD_8) += 104-quad-8.o
> obj-$(CONFIG_STM32_TIMER_CNT) += stm32-timer-cnt.o
> obj-$(CONFIG_STM32_LPTIMER_CNT) += stm32-lptimer-cnt.o
> obj-$(CONFIG_FTM_QUADDEC) += ftm-quaddec.o
> +obj-$(CONFIG_INTEL_QEP) += intel-qep.o
> diff --git a/drivers/counter/intel-qep.c b/drivers/counter/intel-qep.c
> new file mode 100644
> index 000000000000..fa410a333b05
> --- /dev/null
> +++ b/drivers/counter/intel-qep.c
> @@ -0,0 +1,689 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * intel-qep.c - Intel Quadrature Encoder Driver
> + *
> + * Copyright (C) 2019 Intel Corporation - https://www.intel.com
> + *
> + * Author: Felipe Balbi <felipe.balbi@linux.intel.com>
> + */
> +#include <linux/bitops.h>
> +#include <linux/counter.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pci.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/sysfs.h>
Is the sysfs.h file include needed anymore?
> +
> +#define INTEL_QEPCON 0x00
> +#define INTEL_QEPFLT 0x04
> +#define INTEL_QEPCOUNT 0x08
> +#define INTEL_QEPMAX 0x0c
> +#define INTEL_QEPWDT 0x10
> +#define INTEL_QEPCAPDIV 0x14
> +#define INTEL_QEPCNTR 0x18
> +#define INTEL_QEPCAPBUF 0x1c
> +#define INTEL_QEPINT_STAT 0x20
> +#define INTEL_QEPINT_MASK 0x24
> +
> +/* QEPCON */
> +#define INTEL_QEPCON_EN BIT(0)
> +#define INTEL_QEPCON_FLT_EN BIT(1)
> +#define INTEL_QEPCON_EDGE_A BIT(2)
> +#define INTEL_QEPCON_EDGE_B BIT(3)
> +#define INTEL_QEPCON_EDGE_INDX BIT(4)
> +#define INTEL_QEPCON_SWPAB BIT(5)
> +#define INTEL_QEPCON_OP_MODE BIT(6)
> +#define INTEL_QEPCON_PH_ERR BIT(7)
> +#define INTEL_QEPCON_COUNT_RST_MODE BIT(8)
> +#define INTEL_QEPCON_INDX_GATING_MASK GENMASK(10, 9)
> +#define INTEL_QEPCON_INDX_GATING(n) (((n) & 3) << 9)
> +#define INTEL_QEPCON_INDX_PAL_PBL INTEL_QEPCON_INDX_GATING(0)
> +#define INTEL_QEPCON_INDX_PAL_PBH INTEL_QEPCON_INDX_GATING(1)
> +#define INTEL_QEPCON_INDX_PAH_PBL INTEL_QEPCON_INDX_GATING(2)
> +#define INTEL_QEPCON_INDX_PAH_PBH INTEL_QEPCON_INDX_GATING(3)
> +#define INTEL_QEPCON_CAP_MODE BIT(11)
> +#define INTEL_QEPCON_FIFO_THRE_MASK GENMASK(14, 12)
> +#define INTEL_QEPCON_FIFO_THRE(n) ((((n) - 1) & 7) << 12)
> +#define INTEL_QEPCON_FIFO_EMPTY BIT(15)
> +
> +/* QEPFLT */
> +#define INTEL_QEPFLT_MAX_COUNT(n) ((n) & 0x1fffff)
> +
> +/* QEPINT */
> +#define INTEL_QEPINT_FIFOCRIT BIT(5)
> +#define INTEL_QEPINT_FIFOENTRY BIT(4)
> +#define INTEL_QEPINT_QEPDIR BIT(3)
> +#define INTEL_QEPINT_QEPRST_UP BIT(2)
> +#define INTEL_QEPINT_QEPRST_DOWN BIT(1)
> +#define INTEL_QEPINT_WDT BIT(0)
> +
> +#define INTEL_QEP_DIRECTION_FORWARD 1
> +#define INTEL_QEP_DIRECTION_BACKWARD !INTEL_QEP_DIRECTION_FORWARD
> +
> +#define INTEL_QEP_COUNTER_EXT_RW(_name) \
> +{ \
> + .name = #_name, \
> + .read = _name##_read, \
> + .write = _name##_write, \
> +}
> +
> +#define INTEL_QEP_COUNTER_EXT_RO(_name) \
> +{ \
> + .name = #_name, \
> + .read = _name##_read, \
> +}
> +
> +#define INTEL_QEP_COUNTER_COUNT_EXT_RW(_name) \
> +{ \
> + .name = #_name, \
> + .read = _name##_read, \
> + .write = _name##_write, \
> +}
> +
> +#define INTEL_QEP_COUNTER_COUNT_EXT_RO(_name) \
> +{ \
> + .name = #_name, \
> + .read = _name##_read, \
> +}
You're using the same naming convention for all your extensions, so we
can reduce some complexity here by removing one pair of defines such as
INTEL_QEP_COUNTER_COUNT_EXT_RW/INTEL_QEP_COUNTER_COUNT_EXT_RO and just
use INTEL_QEP_COUNTER_EXT_RW/INTEL_QEP_COUNTER_EXT_RO instead for all
extensions. If you start using a different naming convention for Count
extensions versus device extensions, then you can separate them out with
dedicated defines.
> +
> +struct intel_qep {
> + struct counter_device counter;
> + struct mutex lock;
> + struct pci_dev *pci;
> + struct device *dev;
> + void __iomem *regs;
> + u32 interrupt;
> + int direction;
> + bool enabled;
> +};
> +
> +#define counter_to_qep(c) (container_of((c), struct intel_qep, counter))
This macro isn't needed since you set the counter_device priv member to
your intel_qep structure in the intel_qep_probe. What you can do instead
is simply define a pointer directly:
struct intel_qep *const qep = counter->priv;
You do so already in several of your main callbacks, so it seems like
the counter_to_qep macro use in the extension callbacks are just
leftovers from when you converted the extensions over from direct sysfs
callbacks.
> +
> +static inline u32 intel_qep_readl(void __iomem *base, u32 offset)
> +{
> + return readl(base + offset);
> +}
> +
> +static inline void intel_qep_writel(void __iomem *base, u32 offset, u32 value)
> +{
> + writel(value, base + offset);
> +}
> +
> +static const struct pci_device_id intel_qep_id_table[] = {
> + /* EHL */
> + { PCI_VDEVICE(INTEL, 0x4bc3), },
> + { PCI_VDEVICE(INTEL, 0x4b81), },
> + { PCI_VDEVICE(INTEL, 0x4b82), },
> + { PCI_VDEVICE(INTEL, 0x4b83), },
> + { } /* Terminating Entry */
> +};
> +MODULE_DEVICE_TABLE(pci, intel_qep_id_table);
> +
> +static void intel_qep_init(struct intel_qep *qep, bool reset)
> +{
> + u32 reg;
> +
> + reg = intel_qep_readl(qep->regs, INTEL_QEPCON);
> + reg &= ~INTEL_QEPCON_EN;
> + intel_qep_writel(qep->regs, INTEL_QEPCON, reg);
> +
> + /* make sure periperal is disabled by reading one more time */
> + reg = intel_qep_readl(qep->regs, INTEL_QEPCON);
> +
> + if (reset) {
> + reg &= ~(INTEL_QEPCON_OP_MODE | INTEL_QEPCON_FLT_EN);
> + reg |= INTEL_QEPCON_EDGE_A | INTEL_QEPCON_EDGE_B |
> + INTEL_QEPCON_EDGE_INDX | INTEL_QEPCON_COUNT_RST_MODE;
> + }
> +
> + intel_qep_writel(qep->regs, INTEL_QEPCON, reg);
> +
> + intel_qep_writel(qep->regs, INTEL_QEPWDT, 0x1000);
> + intel_qep_writel(qep->regs, INTEL_QEPINT_MASK, 0x0);
> +
> + qep->direction = INTEL_QEP_DIRECTION_FORWARD;
> +}
> +
> +static irqreturn_t intel_qep_irq_thread(int irq, void *_qep)
> +{
> + struct intel_qep *qep = _qep;
> + u32 stat;
> +
> + mutex_lock(&qep->lock);
> +
> + stat = qep->interrupt;
> + if (stat & INTEL_QEPINT_FIFOCRIT)
> + dev_dbg(qep->dev, "Fifo Critical\n");
> +
> + if (stat & INTEL_QEPINT_FIFOENTRY)
> + dev_dbg(qep->dev, "Fifo Entry\n");
> +
> + if (stat & INTEL_QEPINT_QEPDIR)
> + qep->direction = !qep->direction;
> +
> + if (stat & INTEL_QEPINT_QEPRST_UP)
> + qep->direction = INTEL_QEP_DIRECTION_FORWARD;
> +
> + if (stat & INTEL_QEPINT_QEPRST_DOWN)
> + qep->direction = INTEL_QEP_DIRECTION_BACKWARD;
> +
> + if (stat & INTEL_QEPINT_WDT)
> + dev_dbg(qep->dev, "Watchdog\n");
> +
> + intel_qep_writel(qep->regs, INTEL_QEPINT_MASK, 0x00);
> + mutex_unlock(&qep->lock);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t intel_qep_irq(int irq, void *_qep)
> +{
> + struct intel_qep *qep = _qep;
> + u32 stat;
> +
> + stat = intel_qep_readl(qep->regs, INTEL_QEPINT_STAT);
> + if (stat) {
> + qep->interrupt = stat;
> + intel_qep_writel(qep->regs, INTEL_QEPINT_MASK, 0xffffffff);
> + intel_qep_writel(qep->regs, INTEL_QEPINT_STAT, stat);
> + return IRQ_WAKE_THREAD;
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +enum intel_qep_synapse_action {
> + INTEL_QEP_SYNAPSE_ACTION_RISING_EDGE,
> + INTEL_QEP_SYNAPSE_ACTION_FALLING_EDGE,
> +};
> +
> +static enum counter_synapse_action intel_qep_synapse_actions[] = {
> + [INTEL_QEP_SYNAPSE_ACTION_RISING_EDGE] =
> + COUNTER_SYNAPSE_ACTION_RISING_EDGE,
> +
Trim the trailing whitespace here.
> + [INTEL_QEP_SYNAPSE_ACTION_FALLING_EDGE] =
> + COUNTER_SYNAPSE_ACTION_FALLING_EDGE,
> +};
> +
> +enum intel_qep_count_function {
> + INTEL_QEP_ENCODER_MODE_NORMAL,
> + INTEL_QEP_ENCODER_MODE_SWAPPED,
> +};
> +
> +static const enum counter_count_function intel_qep_count_functions[] = {
> + [INTEL_QEP_ENCODER_MODE_NORMAL] =
> + COUNTER_COUNT_FUNCTION_QUADRATURE_X4,
> +
> + [INTEL_QEP_ENCODER_MODE_SWAPPED] =
> + COUNTER_COUNT_FUNCTION_QUADRATURE_X4_SWAPPED,
> +};
> +
> +static int intel_qep_count_read(struct counter_device *counter,
> + struct counter_count *count,
> + struct counter_count_read_value *val)
> +{
> + struct intel_qep *const qep = counter->priv;
> + uint32_t cntval;
> +
> + cntval = intel_qep_readl(qep, INTEL_QEPCOUNT);
> + counter_count_read_value_set(val, COUNTER_COUNT_POSITION, &cntval);
> +
> + return 0;
> +}
> +
> +static int intel_qep_count_write(struct counter_device *counter,
> + struct counter_count *count,
> + struct counter_count_write_value *val)
> +{
> + struct intel_qep *const qep = counter->priv;
> + u32 cnt;
> + int err;
> +
> + err = counter_count_write_value_get(&cnt, COUNTER_COUNT_POSITION, val);
> + if (err)
> + return err;
> +
> + intel_qep_writel(qep->regs, INTEL_QEPMAX, cnt);
> +
> + return 0;
> +}
> +
> +static int intel_qep_function_get(struct counter_device *counter,
> + struct counter_count *count, size_t *function)
> +{
> + struct intel_qep *qep = counter_to_qep(counter);
> + u32 reg;
> +
> + reg = intel_qep_readl(qep->regs, INTEL_QEPCON);
> + if (reg & INTEL_QEPCON_SWPAB)
> + *function = INTEL_QEP_ENCODER_MODE_SWAPPED;
> + else
> + *function = INTEL_QEP_ENCODER_MODE_NORMAL;
> +
> + return 0;
> +}
> +
> +static int intel_qep_function_set(struct counter_device *counter,
> + struct counter_count *count, size_t function)
> +{
> + struct intel_qep *qep = counter_to_qep(counter);
> + u32 reg;
> +
> + reg = intel_qep_readl(qep->regs, INTEL_QEPCON);
> + if (function == INTEL_QEP_ENCODER_MODE_SWAPPED)
> + reg |= INTEL_QEPCON_SWPAB;
> + else
> + reg &= ~INTEL_QEPCON_SWPAB;
> + intel_qep_writel(qep->regs, INTEL_QEPCON, reg);
> +
> + return 0;
> +}
> +
> +static int intel_qep_action_get(struct counter_device *counter,
> + struct counter_count *count, struct counter_synapse *synapse,
> + size_t *action)
> +{
> + struct intel_qep *qep = counter_to_qep(counter);
> + u32 reg;
> +
> + reg = intel_qep_readl(qep->regs, INTEL_QEPCON);
> +
> + *action = reg & synapse->signal->id ?
> + INTEL_QEP_SYNAPSE_ACTION_RISING_EDGE :
> + INTEL_QEP_SYNAPSE_ACTION_FALLING_EDGE;
> +
> + return 0;
> +}
> +
> +static int intel_qep_action_set(struct counter_device *counter,
> + struct counter_count *count,
> + struct counter_synapse *synapse, size_t action)
> +{
> + struct intel_qep *qep = counter_to_qep(counter);
> + u32 reg;
> +
> + reg = intel_qep_readl(qep->regs, INTEL_QEPCON);
> +
> + if (action == INTEL_QEP_SYNAPSE_ACTION_RISING_EDGE)
> + reg |= synapse->signal->id;
> + else
> + reg &= ~synapse->signal->id;
> +
> + intel_qep_writel(qep->regs, INTEL_QEPCON, reg);
> +
> + return 0;
> +}
> +
> +static const struct counter_ops intel_qep_counter_ops = {
> + .count_read = intel_qep_count_read,
> + .count_write = intel_qep_count_write,
> +
> + .function_get = intel_qep_function_get,
> + .function_set = intel_qep_function_set,
> +
> + .action_get = intel_qep_action_get,
> + .action_set = intel_qep_action_set,
> +};
> +
> +static struct counter_signal intel_qep_signals[] = {
> + {
> + .id = INTEL_QEPCON_EDGE_A,
> + .name = "Phase A",
> + },
> + {
> + .id = INTEL_QEPCON_EDGE_B,
> + .name = "Phase B",
> + },
> + {
> + .id = INTEL_QEPCON_EDGE_INDX,
> + .name = "Index",
> + },
> +};
> +
> +static struct counter_synapse intel_qep_count_synapses[] = {
> + {
> + .actions_list = intel_qep_synapse_actions,
> + .num_actions = ARRAY_SIZE(intel_qep_synapse_actions),
> + .signal = &intel_qep_signals[0],
> + },
> + {
> + .actions_list = intel_qep_synapse_actions,
> + .num_actions = ARRAY_SIZE(intel_qep_synapse_actions),
> + .signal = &intel_qep_signals[1],
> + },
> + {
> + .actions_list = intel_qep_synapse_actions,
> + .num_actions = ARRAY_SIZE(intel_qep_synapse_actions),
> + .signal = &intel_qep_signals[2],
> + },
> +};
> +
> +static ssize_t ceiling_read(struct counter_device *counter,
> + struct counter_count *count, void *priv, char *buf)
> +{
> + struct intel_qep *qep = counter_to_qep(counter);
> + u32 reg;
> +
> + reg = intel_qep_readl(qep->regs, INTEL_QEPMAX);
> +
> + return snprintf(buf, PAGE_SIZE, "%d\n", reg);
Normally it's good to make sure you don't write over the PAGE_SIZE
limit, but this check isn't necessary here in the Counter extension
callbacks; the PAGE_SIZE check for buf will happen later on internally
so you don't have to worry about it as a Counter driver author.
Using sprintf for all the extension callbacks in your driver is fine:
return sprintf(buf, "%d\n", reg)
> +}
> +
> +static ssize_t ceiling_write(struct counter_device *counter,
> + struct counter_count *count, void *priv, const char *buf,
> + size_t len)
> +{
> + struct intel_qep *qep = counter_to_qep(counter);
> + u32 max;
> + int ret;
> +
> + ret = kstrtou32(buf, 0, &max);
> + if (ret < 0)
> + return ret;
> +
> + intel_qep_writel(qep->regs, INTEL_QEPMAX, max);
> +
> + return len;
> +}
> +
> +static ssize_t enable_read(struct counter_device *counter,
> + struct counter_count *count, void *priv, char *buf)
> +{
> + struct intel_qep *qep = counter_to_qep(counter);
> + u32 reg;
> +
> + reg = intel_qep_readl(qep->regs, INTEL_QEPCON);
> +
> + return snprintf(buf, PAGE_SIZE, "%d\n", !!(reg & INTEL_QEPCON_EN));
> +}
> +
> +static ssize_t enable_write(struct counter_device *counter,
> + struct counter_count *count, void *priv, const char *buf,
> + size_t len)
> +{
> + struct intel_qep *qep = counter_to_qep(counter);
> + u32 reg;
> + u32 val;
> + int ret;
> +
> + ret = kstrtou32(buf, 0, &val);
Use kstrtobool here instead so that values such as "y" and "n" can be
interpreted correctly as well.
> + if (ret < 0)
> + return ret;
> +
> + reg = intel_qep_readl(qep->regs, INTEL_QEPCON);
> +
> + if (val)
> + reg |= INTEL_QEPCON_EN;
> + else
> + reg &= ~INTEL_QEPCON_EN;
> +
> + intel_qep_writel(qep->regs, INTEL_QEPCON, reg);
> +
> + return len;
> +}
> +
> +static ssize_t direction_read(struct counter_device *counter,
> + struct counter_count *count, void *priv, char *buf)
> +{
> + struct intel_qep *qep = counter_to_qep(counter);
> +
> + return snprintf(buf, PAGE_SIZE, "%s\n", qep->direction ?
> + "forward" : "backward");
We standardized the direction extension values, so instead define a
enum counter_count_direction dir and set it to the direction you want.
When you're done, you can return like this:
return sprintf(buf, "%s\n", counter_count_direction_str[dir])
The counter_count_direction enum constants are provided in the counter.h
header file.
> +}
> +
> +static const struct counter_count_ext intel_qep_count_ext[] = {
> + INTEL_QEP_COUNTER_COUNT_EXT_RW(ceiling),
> + INTEL_QEP_COUNTER_COUNT_EXT_RW(enable),
> + INTEL_QEP_COUNTER_COUNT_EXT_RO(direction),
> +};
> +
> +static struct counter_count intel_qep_here counter_count[] = {
> + {
> + .id = 0,
> + .name = "Channel 1 Count",
> + .functions_list = intel_qep_count_functions,
> + .num_functions = ARRAY_SIZE(intel_qep_count_functions),
> + .synapses = intel_qep_count_synapses,
> + .num_synapses = ARRAY_SIZE(intel_qep_count_synapses),
> + .ext = intel_qep_count_ext,
> + .num_ext = ARRAY_SIZE(intel_qep_count_ext),
> + },
> +};
> +
> +static ssize_t noise_read(struct counter_device *counter, void *priv, char *buf)
> +{
> + struct intel_qep *qep = counter_to_qep(counter);
> + u32 reg;
> +
> + reg = intel_qep_readl(qep->regs, INTEL_QEPCON);
> +
> + if (!(reg & INTEL_QEPCON_FLT_EN))
> + return snprintf(buf, PAGE_SIZE, "0\n");
> +
> + reg = intel_qep_readl(qep->regs, INTEL_QEPFLT);
> +
> + return snprintf(buf, PAGE_SIZE, "%d\n", INTEL_QEPFLT_MAX_COUNT(reg));
> +}
> +
> +static ssize_t noise_write(struct counter_device *counter, void *priv,
> + const char *buf, size_t len)
> +{
> + struct intel_qep *qep = counter_to_qep(counter);
> + u32 reg;
> + u32 max;
> + int ret;
> +
> + ret = kstrtou32(buf, 0, &max);
> + if (ret < 0)
> + return ret;
> +
> + if (max > 0x1fffff)
> + max = 0x1ffff;
> +
> + reg = intel_qep_readl(qep->regs, INTEL_QEPCON);
> +
> + if (max == 0) {
> + reg &= ~INTEL_QEPCON_FLT_EN;
> + } else {
> + reg |= INTEL_QEPCON_FLT_EN;
> + intel_qep_writel(qep->regs, INTEL_QEPFLT, max);
> + }
> +
> + intel_qep_writel(qep->regs, INTEL_QEPCON, reg);
> +
Trim the trailing whitespace here.
> + return len;
> +}
Your "noise" extension attribute will need documentation, so create a
Documentation/ABI/testing/sysfs-bus-counter-intel-qep file with all the
information for it inside.
> +
> +static ssize_t preset_read(struct counter_device *counter, void *priv, char *buf)
> +{
> + return snprintf(buf, PAGE_SIZE, "0\n");
> +}
> +
> +static ssize_t preset_enable_read(struct counter_device *counter, void *priv,
> + char *buf)
> +{
> + struct intel_qep *qep = counter_to_qep(counter);
> + u32 reg;
> +
> + reg = intel_qep_readl(qep->regs, INTEL_QEPCON);
> + return snprintf(buf, PAGE_SIZE, "%d\n",
> + !(reg & INTEL_QEPCON_COUNT_RST_MODE));
> +}
> +
> +static ssize_t preset_enable_write(struct counter_device *counter, void *priv,
> + const char *buf, size_t len)
> +{
> + struct intel_qep *qep = counter_to_qep(counter);
> + u32 reg;
> + u32 val;
> + int ret;
> +
> + ret = kstrtou32(buf, 0, &val);
Use kstrtobool here for the same reason as the "enable" extension.
> + if (ret < 0)
> + return ret;
> +
> + reg = intel_qep_readl(qep->regs, INTEL_QEPCON);
> +
> + if (val)
> + reg &= ~INTEL_QEPCON_COUNT_RST_MODE;
> + else
> + reg |= INTEL_QEPCON_COUNT_RST_MODE;
> +
> + intel_qep_writel(qep->regs, INTEL_QEPCON, reg);
> +
Trim the trailing whitespace here.
William Breathitt Gray
> + return len;
> +}
> +
> +static const struct counter_device_ext intel_qep_ext[] = {
> + INTEL_QEP_COUNTER_EXT_RW(noise),
> + INTEL_QEP_COUNTER_EXT_RO(preset),
> + INTEL_QEP_COUNTER_EXT_RW(preset_enable)
> +};
> +
> +static int intel_qep_probe(struct pci_dev *pci, const struct pci_device_id *id)
> +{
> + struct intel_qep *qep;
> + struct device *dev = &pci->dev;
> + void __iomem *regs;
> + int ret;
> + int irq;
> +
> + qep = devm_kzalloc(dev, sizeof(*qep), GFP_KERNEL);
> + if (!qep)
> + return -ENOMEM;
> +
> + ret = pcim_enable_device(pci);
> + if (ret)
> + return ret;
> +
> + pci_set_master(pci);
> +
> + ret = pcim_iomap_regions(pci, BIT(0), pci_name(pci));
> + if (ret)
> + return ret;
> +
> + regs = pcim_iomap_table(pci)[0];
> + if (!regs)
> + return -ENOMEM;
> +
> + qep->pci = pci;
> + qep->dev = dev;
> + qep->regs = regs;
> + mutex_init(&qep->lock);
> +
> + intel_qep_init(qep, true);
> + pci_set_drvdata(pci, qep);
> +
> + qep->counter.name = pci_name(pci);
> + qep->counter.parent = dev;
> + qep->counter.ops = &intel_qep_counter_ops;
> + qep->counter.counts = intel_qep_counter_count;
> + qep->counter.num_counts = ARRAY_SIZE(intel_qep_counter_count);
> + qep->counter.signals = intel_qep_signals;
> + qep->counter.num_signals = ARRAY_SIZE(intel_qep_signals);
> + qep->counter.ext = intel_qep_ext;
> + qep->counter.num_ext = ARRAY_SIZE(intel_qep_ext);
> + qep->counter.priv = qep;
> +
> + ret = counter_register(&qep->counter);
> + if (ret)
> + return ret;
> +
> + ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
> + if (ret < 0)
> + goto err_irq_vectors;
> +
> + irq = pci_irq_vector(pci, 0);
> + ret = devm_request_threaded_irq(&pci->dev, irq, intel_qep_irq,
> + intel_qep_irq_thread, IRQF_SHARED | IRQF_TRIGGER_RISING,
> + "intel-qep", qep);
> + if (ret)
> + goto err_irq;
> +
> + pm_runtime_set_autosuspend_delay(dev, 1000);
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_put_noidle(dev);
> + pm_runtime_allow(dev);
> +
> + return 0;
> +
> +err_irq:
> + pci_free_irq_vectors(pci);
> +
> +err_irq_vectors:
> + counter_unregister(&qep->counter);
> +
> + return ret;
> +}
> +
> +static void intel_qep_remove(struct pci_dev *pci)
> +{
> + struct intel_qep *qep = pci_get_drvdata(pci);
> + struct device *dev = &pci->dev;
> +
> + pm_runtime_forbid(dev);
> + pm_runtime_get_noresume(dev);
> +
> + intel_qep_writel(qep->regs, INTEL_QEPCON, 0);
> + pci_free_irq_vectors(pci);
> + counter_unregister(&qep->counter);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int intel_qep_suspend(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static int intel_qep_resume(struct device *dev)
> +{
> + struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
> + struct intel_qep *qep = pci_get_drvdata(pdev);
> +
> + intel_qep_init(qep, false);
> +
> + return 0;
> +}
> +
> +static int intel_qep_runtime_suspend(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static int intel_qep_runtime_resume(struct device *dev)
> +{
> + struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
> + struct intel_qep *qep = pci_get_drvdata(pdev);
> +
> + intel_qep_init(qep, false);
> +
> + return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops intel_qep_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(intel_qep_suspend,
> + intel_qep_resume)
> + SET_RUNTIME_PM_OPS(intel_qep_runtime_suspend, intel_qep_runtime_resume,
> + NULL)
> +};
> +
> +static struct pci_driver intel_qep_driver = {
> + .name = "intel-qep",
> + .id_table = intel_qep_id_table,
> + .probe = intel_qep_probe,
> + .remove = intel_qep_remove,
> + .driver = {
> + .pm = &intel_qep_pm_ops,
> + }
> +};
> +
> +module_pci_driver(intel_qep_driver);
> +
> +MODULE_AUTHOR("Felipe Balbi <felipe.balbi@linux.intel.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Intel Quadrature Encoder Driver");
> --
> 2.23.0
>
next prev parent reply other threads:[~2019-10-02 18:11 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-16 9:34 [RFC/PATCH] counter: introduce support for Intel QEP Encoder Felipe Balbi
2019-09-17 11:46 ` William Breathitt Gray
2019-09-17 12:07 ` Felipe Balbi
2019-09-17 13:02 ` William Breathitt Gray
2019-09-19 8:03 ` [RFC/PATCH 1/2] counter: add support for Quadrature x4 with swapped inputs Felipe Balbi
2019-09-19 8:03 ` [RFC/PATCHv2 2/2] counter: introduce support for Intel QEP Encoder Felipe Balbi
2019-09-22 23:35 ` William Breathitt Gray
2019-09-24 9:46 ` Felipe Balbi
2019-09-24 11:32 ` William Breathitt Gray
2019-09-24 11:35 ` Felipe Balbi
2019-10-01 9:32 ` [PATCH v3 1/2] counter: add support for Quadrature x4 with swapped inputs Felipe Balbi
2019-10-01 9:32 ` [PATCH v3 2/2] counter: introduce support for Intel QEP Encoder Felipe Balbi
2019-10-02 18:11 ` William Breathitt Gray [this message]
2019-10-02 16:37 ` [PATCH v3 1/2] counter: add support for Quadrature x4 with swapped inputs William Breathitt Gray
2019-10-03 12:38 ` [RFC/PATCHv2 2/2] counter: introduce support for Intel QEP Encoder Jonathan Cameron
2019-09-24 21:46 ` David Lechner
2019-09-28 21:33 ` William Breathitt Gray
2019-09-30 5:22 ` Felipe Balbi
2019-10-02 0:34 ` William Breathitt Gray
2019-10-03 13:14 ` Jonathan Cameron
2019-10-16 20:20 ` William Breathitt Gray
2019-10-17 12:29 ` Jonathan Cameron
2019-09-24 11:37 ` [RFC/PATCH 1/2] counter: add support for Quadrature x4 with swapped inputs William Breathitt Gray
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191002181105.GB100041@icarus \
--to=vilhelm.gray@gmail.com \
--cc=felipe.balbi@linux.intel.com \
--cc=linux-iio@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).