* [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
@ 2016-09-21 16:11 ` Mugunthan V N
0 siblings, 0 replies; 60+ messages in thread
From: Mugunthan V N @ 2016-09-21 16:11 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds the required pieces to ti_am335x_adc driver for
DMA support
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
include/linux/mfd/ti_am335x_tscadc.h | 7 ++
2 files changed, 164 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index c3cfacca..89d0b07 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -30,10 +30,32 @@
#include <linux/iio/buffer.h>
#include <linux/iio/kfifo_buf.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+
+#define DMA_BUFFER_SIZE SZ_2K
+
+struct tiadc_dma {
+ /* Filter function */
+ dma_filter_fn fn;
+ /* Parameter to the filter function */
+ void *param;
+ struct dma_slave_config conf;
+ struct dma_chan *chan;
+ dma_addr_t addr;
+ dma_cookie_t cookie;
+ u8 *buf;
+ bool valid_buf_seg;
+ int buf_offset;
+ u8 fifo_thresh;
+};
+
struct tiadc_device {
struct ti_tscadc_dev *mfd_tscadc;
+ struct tiadc_dma dma;
struct mutex fifo1_lock; /* to protect fifo access */
int channels;
+ int total_ch_enabled;
u8 channel_line[8];
u8 channel_step[8];
int buffer_en_ch_steps;
@@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
u16 *data = adc_dev->data;
fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+
for (k = 0; k < fifo1count; k = k + i) {
for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
read = tiadc_readl(adc_dev, REG_FIFO1);
@@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
return IRQ_HANDLED;
}
+static void tiadc_dma_rx_complete(void *param)
+{
+ struct iio_dev *indio_dev = param;
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ struct tiadc_dma *dma = &adc_dev->dma;
+ u8 *data;
+ int i;
+
+ data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
+ dma->valid_buf_seg = !dma->valid_buf_seg;
+
+ for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
+ iio_push_to_buffers(indio_dev, data);
+ data += indio_dev->scan_bytes;
+ }
+}
+
+static int tiadc_start_dma(struct iio_dev *indio_dev)
+{
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ struct tiadc_dma *dma = &adc_dev->dma;
+ struct dma_async_tx_descriptor *desc;
+
+ dma->valid_buf_seg = false;
+ dma->fifo_thresh = FIFO1_THRESHOLD;
+ /*
+ * Make the fifo thresh as the multiple of total number of
+ * channels enabled, so make sure that that cyclic DMA period
+ * length is also a multiple of total number of channels
+ * enabled. This ensures that no invalid data is reported
+ * to the stack via iio_push_to_buffers().
+ */
+ dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
+ dma->buf_offset = DMA_BUFFER_SIZE / 2;
+ /* Make sure that period length is multiple of fifo thresh level */
+ dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
+ sizeof(u16));
+
+ dma->conf.src_maxburst = dma->fifo_thresh + 1;
+ dmaengine_slave_config(dma->chan, &dma->conf);
+
+ desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
+ dma->buf_offset * 2,
+ dma->buf_offset, DMA_DEV_TO_MEM,
+ DMA_PREP_INTERRUPT);
+ if (!desc)
+ return -EBUSY;
+
+ desc->callback = tiadc_dma_rx_complete;
+ desc->callback_param = indio_dev;
+
+ dma->cookie = dmaengine_submit(desc);
+
+ dma_async_issue_pending(dma->chan);
+
+ tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
+ tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
+ tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
+
+ return 0;
+}
+
static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
{
struct tiadc_device *adc_dev = iio_priv(indio_dev);
@@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
{
struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ struct tiadc_dma *dma = &adc_dev->dma;
unsigned int enb = 0;
u8 bit;
+ u32 irq_enable;
tiadc_step_config(indio_dev);
- for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
+ for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
enb |= (get_adc_step_bit(adc_dev, bit) << 1);
+ adc_dev->total_ch_enabled++;
+ }
adc_dev->buffer_en_ch_steps = enb;
+ if (dma->chan)
+ tiadc_start_dma(indio_dev);
+
am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
| IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
- tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
- | IRQENB_FIFO1OVRRUN);
+
+ irq_enable = IRQENB_FIFO1OVRRUN;
+ if (!dma->chan)
+ irq_enable |= IRQENB_FIFO1THRES;
+ tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
return 0;
}
@@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
{
struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ struct tiadc_dma *dma = &adc_dev->dma;
int fifo1count, i, read;
tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
adc_dev->buffer_en_ch_steps = 0;
+ adc_dev->total_ch_enabled = 0;
+ if (dma->chan) {
+ tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
+ dmaengine_terminate_async(dma->chan);
+ }
/* Flush FIFO of leftover data in the time it takes to disable adc */
fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
@@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
.driver_module = THIS_MODULE,
};
+static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
+{
+ return false;
+}
+
+static int tiadc_request_dma(struct platform_device *pdev,
+ struct tiadc_device *adc_dev)
+{
+ struct tiadc_dma *dma = &adc_dev->dma;
+ dma_cap_mask_t mask;
+
+ dma->fn = the_no_dma_filter_fn;
+
+ /* Default slave configuration parameters */
+ dma->conf.direction = DMA_DEV_TO_MEM;
+ dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+ dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
+
+ dma_cap_zero(mask);
+ dma_cap_set(DMA_CYCLIC, mask);
+
+ /* Get a channel for RX */
+ dma->chan = dma_request_slave_channel_compat(mask,
+ dma->fn, dma->param,
+ adc_dev->mfd_tscadc->dev,
+ "fifo1");
+ if (!dma->chan)
+ return -ENODEV;
+
+ /* RX buffer */
+ dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
+ &dma->addr, GFP_KERNEL);
+ if (!dma->buf)
+ goto err;
+
+ dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
+
+ return 0;
+err:
+ dma_release_channel(dma->chan);
+
+ return -ENOMEM;
+}
+
static int tiadc_parse_dt(struct platform_device *pdev,
struct tiadc_device *adc_dev)
{
@@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, indio_dev);
+ err = tiadc_request_dma(pdev, adc_dev);
+ if (err && err != -ENODEV)
+ goto err_dma;
+
return 0;
+err_dma:
+ iio_device_unregister(indio_dev);
err_buffer_unregister:
tiadc_iio_buffered_hardware_remove(indio_dev);
err_free_channels:
@@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
{
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ struct tiadc_dma *dma = &adc_dev->dma;
u32 step_en;
+ if (dma->chan)
+ dma_release_channel(dma->chan);
iio_device_unregister(indio_dev);
tiadc_iio_buffered_hardware_remove(indio_dev);
tiadc_channels_remove(indio_dev);
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index e45a208..fb9dc99 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -23,6 +23,8 @@
#define REG_IRQENABLE 0x02C
#define REG_IRQCLR 0x030
#define REG_IRQWAKEUP 0x034
+#define REG_DMAENABLE_SET 0x038
+#define REG_DMAENABLE_CLEAR 0x038
#define REG_CTRL 0x040
#define REG_ADCFSM 0x044
#define REG_CLKDIV 0x04C
@@ -36,6 +38,7 @@
#define REG_FIFO0THR 0xE8
#define REG_FIFO1CNT 0xF0
#define REG_FIFO1THR 0xF4
+#define REG_DMA1REQ 0xF8
#define REG_FIFO0 0x100
#define REG_FIFO1 0x200
@@ -126,6 +129,10 @@
#define FIFOREAD_DATA_MASK (0xfff << 0)
#define FIFOREAD_CHNLID_MASK (0xf << 16)
+/* DMA ENABLE/CLEAR Register */
+#define DMA_FIFO0 BIT(0)
+#define DMA_FIFO1 BIT(1)
+
/* Sequencer Status */
#define SEQ_STATUS BIT(5)
#define CHARGE_STEP 0x11
--
2.10.0.129.g35f6318
^ permalink raw reply related [flat|nested] 60+ messages in thread* [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
@ 2016-09-21 16:11 ` Mugunthan V N
0 siblings, 0 replies; 60+ messages in thread
From: Mugunthan V N @ 2016-09-21 16:11 UTC (permalink / raw)
To: linux-iio
Cc: Tony Lindgren, Rob Herring, Mark Rutland, Russell King,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Lee Jones, Vignesh R, Andrew F . Davis,
linux-omap, devicetree, linux-arm-kernel, linux-kernel,
Sekhar Nori, Peter Ujfalusi, Mugunthan V N
This patch adds the required pieces to ti_am335x_adc driver for
DMA support
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
include/linux/mfd/ti_am335x_tscadc.h | 7 ++
2 files changed, 164 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index c3cfacca..89d0b07 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -30,10 +30,32 @@
#include <linux/iio/buffer.h>
#include <linux/iio/kfifo_buf.h>
+#include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
+
+#define DMA_BUFFER_SIZE SZ_2K
+
+struct tiadc_dma {
+ /* Filter function */
+ dma_filter_fn fn;
+ /* Parameter to the filter function */
+ void *param;
+ struct dma_slave_config conf;
+ struct dma_chan *chan;
+ dma_addr_t addr;
+ dma_cookie_t cookie;
+ u8 *buf;
+ bool valid_buf_seg;
+ int buf_offset;
+ u8 fifo_thresh;
+};
+
struct tiadc_device {
struct ti_tscadc_dev *mfd_tscadc;
+ struct tiadc_dma dma;
struct mutex fifo1_lock; /* to protect fifo access */
int channels;
+ int total_ch_enabled;
u8 channel_line[8];
u8 channel_step[8];
int buffer_en_ch_steps;
@@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
u16 *data = adc_dev->data;
fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
+
for (k = 0; k < fifo1count; k = k + i) {
for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
read = tiadc_readl(adc_dev, REG_FIFO1);
@@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
return IRQ_HANDLED;
}
+static void tiadc_dma_rx_complete(void *param)
+{
+ struct iio_dev *indio_dev = param;
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ struct tiadc_dma *dma = &adc_dev->dma;
+ u8 *data;
+ int i;
+
+ data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
+ dma->valid_buf_seg = !dma->valid_buf_seg;
+
+ for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
+ iio_push_to_buffers(indio_dev, data);
+ data += indio_dev->scan_bytes;
+ }
+}
+
+static int tiadc_start_dma(struct iio_dev *indio_dev)
+{
+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ struct tiadc_dma *dma = &adc_dev->dma;
+ struct dma_async_tx_descriptor *desc;
+
+ dma->valid_buf_seg = false;
+ dma->fifo_thresh = FIFO1_THRESHOLD;
+ /*
+ * Make the fifo thresh as the multiple of total number of
+ * channels enabled, so make sure that that cyclic DMA period
+ * length is also a multiple of total number of channels
+ * enabled. This ensures that no invalid data is reported
+ * to the stack via iio_push_to_buffers().
+ */
+ dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
+ dma->buf_offset = DMA_BUFFER_SIZE / 2;
+ /* Make sure that period length is multiple of fifo thresh level */
+ dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
+ sizeof(u16));
+
+ dma->conf.src_maxburst = dma->fifo_thresh + 1;
+ dmaengine_slave_config(dma->chan, &dma->conf);
+
+ desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
+ dma->buf_offset * 2,
+ dma->buf_offset, DMA_DEV_TO_MEM,
+ DMA_PREP_INTERRUPT);
+ if (!desc)
+ return -EBUSY;
+
+ desc->callback = tiadc_dma_rx_complete;
+ desc->callback_param = indio_dev;
+
+ dma->cookie = dmaengine_submit(desc);
+
+ dma_async_issue_pending(dma->chan);
+
+ tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
+ tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
+ tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
+
+ return 0;
+}
+
static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
{
struct tiadc_device *adc_dev = iio_priv(indio_dev);
@@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
{
struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ struct tiadc_dma *dma = &adc_dev->dma;
unsigned int enb = 0;
u8 bit;
+ u32 irq_enable;
tiadc_step_config(indio_dev);
- for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
+ for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
enb |= (get_adc_step_bit(adc_dev, bit) << 1);
+ adc_dev->total_ch_enabled++;
+ }
adc_dev->buffer_en_ch_steps = enb;
+ if (dma->chan)
+ tiadc_start_dma(indio_dev);
+
am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
| IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
- tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
- | IRQENB_FIFO1OVRRUN);
+
+ irq_enable = IRQENB_FIFO1OVRRUN;
+ if (!dma->chan)
+ irq_enable |= IRQENB_FIFO1THRES;
+ tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
return 0;
}
@@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
{
struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ struct tiadc_dma *dma = &adc_dev->dma;
int fifo1count, i, read;
tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
adc_dev->buffer_en_ch_steps = 0;
+ adc_dev->total_ch_enabled = 0;
+ if (dma->chan) {
+ tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
+ dmaengine_terminate_async(dma->chan);
+ }
/* Flush FIFO of leftover data in the time it takes to disable adc */
fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
@@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
.driver_module = THIS_MODULE,
};
+static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
+{
+ return false;
+}
+
+static int tiadc_request_dma(struct platform_device *pdev,
+ struct tiadc_device *adc_dev)
+{
+ struct tiadc_dma *dma = &adc_dev->dma;
+ dma_cap_mask_t mask;
+
+ dma->fn = the_no_dma_filter_fn;
+
+ /* Default slave configuration parameters */
+ dma->conf.direction = DMA_DEV_TO_MEM;
+ dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
+ dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
+
+ dma_cap_zero(mask);
+ dma_cap_set(DMA_CYCLIC, mask);
+
+ /* Get a channel for RX */
+ dma->chan = dma_request_slave_channel_compat(mask,
+ dma->fn, dma->param,
+ adc_dev->mfd_tscadc->dev,
+ "fifo1");
+ if (!dma->chan)
+ return -ENODEV;
+
+ /* RX buffer */
+ dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
+ &dma->addr, GFP_KERNEL);
+ if (!dma->buf)
+ goto err;
+
+ dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
+
+ return 0;
+err:
+ dma_release_channel(dma->chan);
+
+ return -ENOMEM;
+}
+
static int tiadc_parse_dt(struct platform_device *pdev,
struct tiadc_device *adc_dev)
{
@@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, indio_dev);
+ err = tiadc_request_dma(pdev, adc_dev);
+ if (err && err != -ENODEV)
+ goto err_dma;
+
return 0;
+err_dma:
+ iio_device_unregister(indio_dev);
err_buffer_unregister:
tiadc_iio_buffered_hardware_remove(indio_dev);
err_free_channels:
@@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
{
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct tiadc_device *adc_dev = iio_priv(indio_dev);
+ struct tiadc_dma *dma = &adc_dev->dma;
u32 step_en;
+ if (dma->chan)
+ dma_release_channel(dma->chan);
iio_device_unregister(indio_dev);
tiadc_iio_buffered_hardware_remove(indio_dev);
tiadc_channels_remove(indio_dev);
diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
index e45a208..fb9dc99 100644
--- a/include/linux/mfd/ti_am335x_tscadc.h
+++ b/include/linux/mfd/ti_am335x_tscadc.h
@@ -23,6 +23,8 @@
#define REG_IRQENABLE 0x02C
#define REG_IRQCLR 0x030
#define REG_IRQWAKEUP 0x034
+#define REG_DMAENABLE_SET 0x038
+#define REG_DMAENABLE_CLEAR 0x038
#define REG_CTRL 0x040
#define REG_ADCFSM 0x044
#define REG_CLKDIV 0x04C
@@ -36,6 +38,7 @@
#define REG_FIFO0THR 0xE8
#define REG_FIFO1CNT 0xF0
#define REG_FIFO1THR 0xF4
+#define REG_DMA1REQ 0xF8
#define REG_FIFO0 0x100
#define REG_FIFO1 0x200
@@ -126,6 +129,10 @@
#define FIFOREAD_DATA_MASK (0xfff << 0)
#define FIFOREAD_CHNLID_MASK (0xf << 16)
+/* DMA ENABLE/CLEAR Register */
+#define DMA_FIFO0 BIT(0)
+#define DMA_FIFO1 BIT(1)
+
/* Sequencer Status */
#define SEQ_STATUS BIT(5)
#define CHARGE_STEP 0x11
--
2.10.0.129.g35f6318
^ permalink raw reply related [flat|nested] 60+ messages in thread[parent not found: <20160921161134.6951-3-mugunthanvnm-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
2016-09-21 16:11 ` Mugunthan V N
@ 2016-09-21 17:44 ` Peter Meerwald-Stadler
-1 siblings, 0 replies; 60+ messages in thread
From: Peter Meerwald-Stadler @ 2016-09-21 17:44 UTC (permalink / raw)
To: Mugunthan V N
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron,
Hartmut Knaack, Lars-Peter Clausen, Lee Jones, Vignesh R,
Andrew F . Davis, linux-omap-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori,
Peter Ujfalusi
> This patch adds the required pieces to ti_am335x_adc driver for
> DMA support
nitpicking below
> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
> ---
> drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
> 2 files changed, 164 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index c3cfacca..89d0b07 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -30,10 +30,32 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/kfifo_buf.h>
>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +
> +#define DMA_BUFFER_SIZE SZ_2K
> +
> +struct tiadc_dma {
> + /* Filter function */
> + dma_filter_fn fn;
> + /* Parameter to the filter function */
> + void *param;
> + struct dma_slave_config conf;
> + struct dma_chan *chan;
> + dma_addr_t addr;
> + dma_cookie_t cookie;
> + u8 *buf;
> + bool valid_buf_seg;
> + int buf_offset;
> + u8 fifo_thresh;
> +};
> +
> struct tiadc_device {
> struct ti_tscadc_dev *mfd_tscadc;
> + struct tiadc_dma dma;
> struct mutex fifo1_lock; /* to protect fifo access */
> int channels;
> + int total_ch_enabled;
> u8 channel_line[8];
> u8 channel_step[8];
> int buffer_en_ch_steps;
> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
> u16 *data = adc_dev->data;
>
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> +
extra newline added
> for (k = 0; k < fifo1count; k = k + i) {
> for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
> read = tiadc_readl(adc_dev, REG_FIFO1);
> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
> return IRQ_HANDLED;
> }
>
> +static void tiadc_dma_rx_complete(void *param)
> +{
> + struct iio_dev *indio_dev = param;
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> + u8 *data;
> + int i;
> +
> + data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
> + dma->valid_buf_seg = !dma->valid_buf_seg;
> +
> + for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
> + iio_push_to_buffers(indio_dev, data);
> + data += indio_dev->scan_bytes;
> + }
> +}
> +
> +static int tiadc_start_dma(struct iio_dev *indio_dev)
> +{
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> + struct dma_async_tx_descriptor *desc;
> +
> + dma->valid_buf_seg = false;
> + dma->fifo_thresh = FIFO1_THRESHOLD;
> + /*
> + * Make the fifo thresh as the multiple of total number of
> + * channels enabled, so make sure that that cyclic DMA period
that that
> + * length is also a multiple of total number of channels
> + * enabled. This ensures that no invalid data is reported
> + * to the stack via iio_push_to_buffers().
> + */
> + dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
> + dma->buf_offset = DMA_BUFFER_SIZE / 2;
> + /* Make sure that period length is multiple of fifo thresh level */
> + dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
> + sizeof(u16));
> +
> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
> + dmaengine_slave_config(dma->chan, &dma->conf);
> +
> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
> + dma->buf_offset * 2,
> + dma->buf_offset, DMA_DEV_TO_MEM,
> + DMA_PREP_INTERRUPT);
> + if (!desc)
> + return -EBUSY;
> +
> + desc->callback = tiadc_dma_rx_complete;
> + desc->callback_param = indio_dev;
> +
> + dma->cookie = dmaengine_submit(desc);
> +
> + dma_async_issue_pending(dma->chan);
> +
> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
> +
> + return 0;
> +}
> +
> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> unsigned int enb = 0;
> u8 bit;
> + u32 irq_enable;
why u32 and not unsigned int?
>
> tiadc_step_config(indio_dev);
> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
> + adc_dev->total_ch_enabled++;
> + }
> adc_dev->buffer_en_ch_steps = enb;
>
> + if (dma->chan)
> + tiadc_start_dma(indio_dev);
> +
> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>
> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
> - | IRQENB_FIFO1OVRRUN);
> +
> + irq_enable = IRQENB_FIFO1OVRRUN;
> + if (!dma->chan)
> + irq_enable |= IRQENB_FIFO1THRES;
> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>
> return 0;
> }
> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> int fifo1count, i, read;
>
> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
> adc_dev->buffer_en_ch_steps = 0;
> + adc_dev->total_ch_enabled = 0;
> + if (dma->chan) {
> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
> + dmaengine_terminate_async(dma->chan);
> + }
>
> /* Flush FIFO of leftover data in the time it takes to disable adc */
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
> .driver_module = THIS_MODULE,
> };
>
> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
> +{
> + return false;
> +}
> +
> +static int tiadc_request_dma(struct platform_device *pdev,
> + struct tiadc_device *adc_dev)
> +{
> + struct tiadc_dma *dma = &adc_dev->dma;
> + dma_cap_mask_t mask;
> +
> + dma->fn = the_no_dma_filter_fn;
> +
> + /* Default slave configuration parameters */
> + dma->conf.direction = DMA_DEV_TO_MEM;
> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
> +
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_CYCLIC, mask);
> +
> + /* Get a channel for RX */
> + dma->chan = dma_request_slave_channel_compat(mask,
> + dma->fn, dma->param,
> + adc_dev->mfd_tscadc->dev,
> + "fifo1");
> + if (!dma->chan)
> + return -ENODEV;
> +
> + /* RX buffer */
> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
> + &dma->addr, GFP_KERNEL);
> + if (!dma->buf)
> + goto err;
> +
> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
> +
> + return 0;
> +err:
> + dma_release_channel(dma->chan);
> +
> + return -ENOMEM;
> +}
> +
> static int tiadc_parse_dt(struct platform_device *pdev,
> struct tiadc_device *adc_dev)
> {
> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, indio_dev);
>
> + err = tiadc_request_dma(pdev, adc_dev);
> + if (err && err != -ENODEV)
> + goto err_dma;
> +
> return 0;
>
> +err_dma:
> + iio_device_unregister(indio_dev);
> err_buffer_unregister:
> tiadc_iio_buffered_hardware_remove(indio_dev);
> err_free_channels:
> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> u32 step_en;
>
> + if (dma->chan)
> + dma_release_channel(dma->chan);
> iio_device_unregister(indio_dev);
> tiadc_iio_buffered_hardware_remove(indio_dev);
> tiadc_channels_remove(indio_dev);
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index e45a208..fb9dc99 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -23,6 +23,8 @@
> #define REG_IRQENABLE 0x02C
> #define REG_IRQCLR 0x030
> #define REG_IRQWAKEUP 0x034
> +#define REG_DMAENABLE_SET 0x038
> +#define REG_DMAENABLE_CLEAR 0x038
same reg for set and clear?
> #define REG_CTRL 0x040
> #define REG_ADCFSM 0x044
> #define REG_CLKDIV 0x04C
> @@ -36,6 +38,7 @@
> #define REG_FIFO0THR 0xE8
> #define REG_FIFO1CNT 0xF0
> #define REG_FIFO1THR 0xF4
> +#define REG_DMA1REQ 0xF8
> #define REG_FIFO0 0x100
> #define REG_FIFO1 0x200
>
> @@ -126,6 +129,10 @@
> #define FIFOREAD_DATA_MASK (0xfff << 0)
> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>
> +/* DMA ENABLE/CLEAR Register */
> +#define DMA_FIFO0 BIT(0)
> +#define DMA_FIFO1 BIT(1)
> +
> /* Sequencer Status */
> #define SEQ_STATUS BIT(5)
> #define CHARGE_STEP 0x11
>
--
Peter Meerwald-Stadler
+43-664-2444418 (mobile)
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
@ 2016-09-21 17:44 ` Peter Meerwald-Stadler
0 siblings, 0 replies; 60+ messages in thread
From: Peter Meerwald-Stadler @ 2016-09-21 17:44 UTC (permalink / raw)
To: Mugunthan V N
Cc: linux-iio, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Lee Jones, Vignesh R, Andrew F . Davis, linux-omap, Sekhar Nori,
Peter Ujfalusi
> This patch adds the required pieces to ti_am335x_adc driver for
> DMA support
nitpicking below
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
> drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
> 2 files changed, 164 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index c3cfacca..89d0b07 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -30,10 +30,32 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/kfifo_buf.h>
>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +
> +#define DMA_BUFFER_SIZE SZ_2K
> +
> +struct tiadc_dma {
> + /* Filter function */
> + dma_filter_fn fn;
> + /* Parameter to the filter function */
> + void *param;
> + struct dma_slave_config conf;
> + struct dma_chan *chan;
> + dma_addr_t addr;
> + dma_cookie_t cookie;
> + u8 *buf;
> + bool valid_buf_seg;
> + int buf_offset;
> + u8 fifo_thresh;
> +};
> +
> struct tiadc_device {
> struct ti_tscadc_dev *mfd_tscadc;
> + struct tiadc_dma dma;
> struct mutex fifo1_lock; /* to protect fifo access */
> int channels;
> + int total_ch_enabled;
> u8 channel_line[8];
> u8 channel_step[8];
> int buffer_en_ch_steps;
> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
> u16 *data = adc_dev->data;
>
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> +
extra newline added
> for (k = 0; k < fifo1count; k = k + i) {
> for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
> read = tiadc_readl(adc_dev, REG_FIFO1);
> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
> return IRQ_HANDLED;
> }
>
> +static void tiadc_dma_rx_complete(void *param)
> +{
> + struct iio_dev *indio_dev = param;
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> + u8 *data;
> + int i;
> +
> + data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
> + dma->valid_buf_seg = !dma->valid_buf_seg;
> +
> + for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
> + iio_push_to_buffers(indio_dev, data);
> + data += indio_dev->scan_bytes;
> + }
> +}
> +
> +static int tiadc_start_dma(struct iio_dev *indio_dev)
> +{
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> + struct dma_async_tx_descriptor *desc;
> +
> + dma->valid_buf_seg = false;
> + dma->fifo_thresh = FIFO1_THRESHOLD;
> + /*
> + * Make the fifo thresh as the multiple of total number of
> + * channels enabled, so make sure that that cyclic DMA period
that that
> + * length is also a multiple of total number of channels
> + * enabled. This ensures that no invalid data is reported
> + * to the stack via iio_push_to_buffers().
> + */
> + dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
> + dma->buf_offset = DMA_BUFFER_SIZE / 2;
> + /* Make sure that period length is multiple of fifo thresh level */
> + dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
> + sizeof(u16));
> +
> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
> + dmaengine_slave_config(dma->chan, &dma->conf);
> +
> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
> + dma->buf_offset * 2,
> + dma->buf_offset, DMA_DEV_TO_MEM,
> + DMA_PREP_INTERRUPT);
> + if (!desc)
> + return -EBUSY;
> +
> + desc->callback = tiadc_dma_rx_complete;
> + desc->callback_param = indio_dev;
> +
> + dma->cookie = dmaengine_submit(desc);
> +
> + dma_async_issue_pending(dma->chan);
> +
> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
> +
> + return 0;
> +}
> +
> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> unsigned int enb = 0;
> u8 bit;
> + u32 irq_enable;
why u32 and not unsigned int?
>
> tiadc_step_config(indio_dev);
> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
> + adc_dev->total_ch_enabled++;
> + }
> adc_dev->buffer_en_ch_steps = enb;
>
> + if (dma->chan)
> + tiadc_start_dma(indio_dev);
> +
> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>
> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
> - | IRQENB_FIFO1OVRRUN);
> +
> + irq_enable = IRQENB_FIFO1OVRRUN;
> + if (!dma->chan)
> + irq_enable |= IRQENB_FIFO1THRES;
> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>
> return 0;
> }
> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> int fifo1count, i, read;
>
> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
> adc_dev->buffer_en_ch_steps = 0;
> + adc_dev->total_ch_enabled = 0;
> + if (dma->chan) {
> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
> + dmaengine_terminate_async(dma->chan);
> + }
>
> /* Flush FIFO of leftover data in the time it takes to disable adc */
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
> .driver_module = THIS_MODULE,
> };
>
> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
> +{
> + return false;
> +}
> +
> +static int tiadc_request_dma(struct platform_device *pdev,
> + struct tiadc_device *adc_dev)
> +{
> + struct tiadc_dma *dma = &adc_dev->dma;
> + dma_cap_mask_t mask;
> +
> + dma->fn = the_no_dma_filter_fn;
> +
> + /* Default slave configuration parameters */
> + dma->conf.direction = DMA_DEV_TO_MEM;
> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
> +
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_CYCLIC, mask);
> +
> + /* Get a channel for RX */
> + dma->chan = dma_request_slave_channel_compat(mask,
> + dma->fn, dma->param,
> + adc_dev->mfd_tscadc->dev,
> + "fifo1");
> + if (!dma->chan)
> + return -ENODEV;
> +
> + /* RX buffer */
> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
> + &dma->addr, GFP_KERNEL);
> + if (!dma->buf)
> + goto err;
> +
> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
> +
> + return 0;
> +err:
> + dma_release_channel(dma->chan);
> +
> + return -ENOMEM;
> +}
> +
> static int tiadc_parse_dt(struct platform_device *pdev,
> struct tiadc_device *adc_dev)
> {
> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, indio_dev);
>
> + err = tiadc_request_dma(pdev, adc_dev);
> + if (err && err != -ENODEV)
> + goto err_dma;
> +
> return 0;
>
> +err_dma:
> + iio_device_unregister(indio_dev);
> err_buffer_unregister:
> tiadc_iio_buffered_hardware_remove(indio_dev);
> err_free_channels:
> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> u32 step_en;
>
> + if (dma->chan)
> + dma_release_channel(dma->chan);
> iio_device_unregister(indio_dev);
> tiadc_iio_buffered_hardware_remove(indio_dev);
> tiadc_channels_remove(indio_dev);
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index e45a208..fb9dc99 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -23,6 +23,8 @@
> #define REG_IRQENABLE 0x02C
> #define REG_IRQCLR 0x030
> #define REG_IRQWAKEUP 0x034
> +#define REG_DMAENABLE_SET 0x038
> +#define REG_DMAENABLE_CLEAR 0x038
same reg for set and clear?
> #define REG_CTRL 0x040
> #define REG_ADCFSM 0x044
> #define REG_CLKDIV 0x04C
> @@ -36,6 +38,7 @@
> #define REG_FIFO0THR 0xE8
> #define REG_FIFO1CNT 0xF0
> #define REG_FIFO1THR 0xF4
> +#define REG_DMA1REQ 0xF8
> #define REG_FIFO0 0x100
> #define REG_FIFO1 0x200
>
> @@ -126,6 +129,10 @@
> #define FIFOREAD_DATA_MASK (0xfff << 0)
> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>
> +/* DMA ENABLE/CLEAR Register */
> +#define DMA_FIFO0 BIT(0)
> +#define DMA_FIFO1 BIT(1)
> +
> /* Sequencer Status */
> #define SEQ_STATUS BIT(5)
> #define CHARGE_STEP 0x11
>
--
Peter Meerwald-Stadler
+43-664-2444418 (mobile)
^ permalink raw reply [flat|nested] 60+ messages in thread[parent not found: <alpine.DEB.2.02.1609211936540.12662-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>]
* Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
2016-09-21 17:44 ` Peter Meerwald-Stadler
@ 2016-09-22 10:37 ` Mugunthan V N
-1 siblings, 0 replies; 60+ messages in thread
From: Mugunthan V N @ 2016-09-22 10:37 UTC (permalink / raw)
To: Peter Meerwald-Stadler
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron,
Hartmut Knaack, Lars-Peter Clausen, Lee Jones, Vignesh R,
Andrew F . Davis, linux-omap-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori,
Peter Ujfalusi
On Wednesday 21 September 2016 11:14 PM, Peter Meerwald-Stadler wrote:
>
>> This patch adds the required pieces to ti_am335x_adc driver for
>> DMA support
>
> nitpicking below
>
>> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
>> ---
>> drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
>> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
>> 2 files changed, 164 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
>> index c3cfacca..89d0b07 100644
>> --- a/drivers/iio/adc/ti_am335x_adc.c
>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>> @@ -30,10 +30,32 @@
>> #include <linux/iio/buffer.h>
>> #include <linux/iio/kfifo_buf.h>
>>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#define DMA_BUFFER_SIZE SZ_2K
>> +
>> +struct tiadc_dma {
>> + /* Filter function */
>> + dma_filter_fn fn;
>> + /* Parameter to the filter function */
>> + void *param;
>> + struct dma_slave_config conf;
>> + struct dma_chan *chan;
>> + dma_addr_t addr;
>> + dma_cookie_t cookie;
>> + u8 *buf;
>> + bool valid_buf_seg;
>> + int buf_offset;
>> + u8 fifo_thresh;
>> +};
>> +
>> struct tiadc_device {
>> struct ti_tscadc_dev *mfd_tscadc;
>> + struct tiadc_dma dma;
>> struct mutex fifo1_lock; /* to protect fifo access */
>> int channels;
>> + int total_ch_enabled;
>> u8 channel_line[8];
>> u8 channel_step[8];
>> int buffer_en_ch_steps;
>> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> u16 *data = adc_dev->data;
>>
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> +
>
> extra newline added
>
>> for (k = 0; k < fifo1count; k = k + i) {
>> for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
>> read = tiadc_readl(adc_dev, REG_FIFO1);
>> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> return IRQ_HANDLED;
>> }
>>
>> +static void tiadc_dma_rx_complete(void *param)
>> +{
>> + struct iio_dev *indio_dev = param;
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + u8 *data;
>> + int i;
>> +
>> + data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
>> + dma->valid_buf_seg = !dma->valid_buf_seg;
>> +
>> + for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
>> + iio_push_to_buffers(indio_dev, data);
>> + data += indio_dev->scan_bytes;
>> + }
>> +}
>> +
>> +static int tiadc_start_dma(struct iio_dev *indio_dev)
>> +{
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + struct dma_async_tx_descriptor *desc;
>> +
>> + dma->valid_buf_seg = false;
>> + dma->fifo_thresh = FIFO1_THRESHOLD;
>> + /*
>> + * Make the fifo thresh as the multiple of total number of
>> + * channels enabled, so make sure that that cyclic DMA period
>
> that that
>
>> + * length is also a multiple of total number of channels
>> + * enabled. This ensures that no invalid data is reported
>> + * to the stack via iio_push_to_buffers().
>> + */
>> + dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
>> + dma->buf_offset = DMA_BUFFER_SIZE / 2;
>> + /* Make sure that period length is multiple of fifo thresh level */
>> + dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
>> + sizeof(u16));
>> +
>> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
>> + dmaengine_slave_config(dma->chan, &dma->conf);
>> +
>> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
>> + dma->buf_offset * 2,
>> + dma->buf_offset, DMA_DEV_TO_MEM,
>> + DMA_PREP_INTERRUPT);
>> + if (!desc)
>> + return -EBUSY;
>> +
>> + desc->callback = tiadc_dma_rx_complete;
>> + desc->callback_param = indio_dev;
>> +
>> + dma->cookie = dmaengine_submit(desc);
>> +
>> + dma_async_issue_pending(dma->chan);
>> +
>> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
>> +
>> + return 0;
>> +}
>> +
>> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> unsigned int enb = 0;
>> u8 bit;
>> + u32 irq_enable;
>
> why u32 and not unsigned int?
>
>>
>> tiadc_step_config(indio_dev);
>> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
>> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
>> + adc_dev->total_ch_enabled++;
>> + }
>> adc_dev->buffer_en_ch_steps = enb;
>>
>> + if (dma->chan)
>> + tiadc_start_dma(indio_dev);
>> +
>> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>>
>> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
>> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
>> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
>> - | IRQENB_FIFO1OVRRUN);
>> +
>> + irq_enable = IRQENB_FIFO1OVRRUN;
>> + if (!dma->chan)
>> + irq_enable |= IRQENB_FIFO1THRES;
>> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>>
>> return 0;
>> }
>> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> int fifo1count, i, read;
>>
>> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>> adc_dev->buffer_en_ch_steps = 0;
>> + adc_dev->total_ch_enabled = 0;
>> + if (dma->chan) {
>> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
>> + dmaengine_terminate_async(dma->chan);
>> + }
>>
>> /* Flush FIFO of leftover data in the time it takes to disable adc */
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
>> .driver_module = THIS_MODULE,
>> };
>>
>> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
>> +{
>> + return false;
>> +}
>> +
>> +static int tiadc_request_dma(struct platform_device *pdev,
>> + struct tiadc_device *adc_dev)
>> +{
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + dma_cap_mask_t mask;
>> +
>> + dma->fn = the_no_dma_filter_fn;
>> +
>> + /* Default slave configuration parameters */
>> + dma->conf.direction = DMA_DEV_TO_MEM;
>> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
>> +
>> + dma_cap_zero(mask);
>> + dma_cap_set(DMA_CYCLIC, mask);
>> +
>> + /* Get a channel for RX */
>> + dma->chan = dma_request_slave_channel_compat(mask,
>> + dma->fn, dma->param,
>> + adc_dev->mfd_tscadc->dev,
>> + "fifo1");
>> + if (!dma->chan)
>> + return -ENODEV;
>> +
>> + /* RX buffer */
>> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>> + &dma->addr, GFP_KERNEL);
>> + if (!dma->buf)
>> + goto err;
>> +
>> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
>> +
>> + return 0;
>> +err:
>> + dma_release_channel(dma->chan);
>> +
>> + return -ENOMEM;
>> +}
>> +
>> static int tiadc_parse_dt(struct platform_device *pdev,
>> struct tiadc_device *adc_dev)
>> {
>> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>>
>> platform_set_drvdata(pdev, indio_dev);
>>
>> + err = tiadc_request_dma(pdev, adc_dev);
>> + if (err && err != -ENODEV)
>> + goto err_dma;
>> +
>> return 0;
>>
>> +err_dma:
>> + iio_device_unregister(indio_dev);
>> err_buffer_unregister:
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> err_free_channels:
>> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
>> {
>> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> u32 step_en;
>>
>> + if (dma->chan)
>> + dma_release_channel(dma->chan);
>> iio_device_unregister(indio_dev);
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> tiadc_channels_remove(indio_dev);
>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> index e45a208..fb9dc99 100644
>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> @@ -23,6 +23,8 @@
>> #define REG_IRQENABLE 0x02C
>> #define REG_IRQCLR 0x030
>> #define REG_IRQWAKEUP 0x034
>> +#define REG_DMAENABLE_SET 0x038
>> +#define REG_DMAENABLE_CLEAR 0x038
>
> same reg for set and clear?
Oops, will fix this and all above.
Regards
Mugunthan V N
>
>> #define REG_CTRL 0x040
>> #define REG_ADCFSM 0x044
>> #define REG_CLKDIV 0x04C
>> @@ -36,6 +38,7 @@
>> #define REG_FIFO0THR 0xE8
>> #define REG_FIFO1CNT 0xF0
>> #define REG_FIFO1THR 0xF4
>> +#define REG_DMA1REQ 0xF8
>> #define REG_FIFO0 0x100
>> #define REG_FIFO1 0x200
>>
>> @@ -126,6 +129,10 @@
>> #define FIFOREAD_DATA_MASK (0xfff << 0)
>> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>>
>> +/* DMA ENABLE/CLEAR Register */
>> +#define DMA_FIFO0 BIT(0)
>> +#define DMA_FIFO1 BIT(1)
>> +
>> /* Sequencer Status */
>> #define SEQ_STATUS BIT(5)
>> #define CHARGE_STEP 0x11
>>
>
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
@ 2016-09-22 10:37 ` Mugunthan V N
0 siblings, 0 replies; 60+ messages in thread
From: Mugunthan V N @ 2016-09-22 10:37 UTC (permalink / raw)
To: Peter Meerwald-Stadler
Cc: linux-iio, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Lee Jones, Vignesh R, Andrew F . Davis, linux-omap, Sekhar Nori,
Peter Ujfalusi
On Wednesday 21 September 2016 11:14 PM, Peter Meerwald-Stadler wrote:
>
>> This patch adds the required pieces to ti_am335x_adc driver for
>> DMA support
>
> nitpicking below
>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> ---
>> drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
>> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
>> 2 files changed, 164 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
>> index c3cfacca..89d0b07 100644
>> --- a/drivers/iio/adc/ti_am335x_adc.c
>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>> @@ -30,10 +30,32 @@
>> #include <linux/iio/buffer.h>
>> #include <linux/iio/kfifo_buf.h>
>>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#define DMA_BUFFER_SIZE SZ_2K
>> +
>> +struct tiadc_dma {
>> + /* Filter function */
>> + dma_filter_fn fn;
>> + /* Parameter to the filter function */
>> + void *param;
>> + struct dma_slave_config conf;
>> + struct dma_chan *chan;
>> + dma_addr_t addr;
>> + dma_cookie_t cookie;
>> + u8 *buf;
>> + bool valid_buf_seg;
>> + int buf_offset;
>> + u8 fifo_thresh;
>> +};
>> +
>> struct tiadc_device {
>> struct ti_tscadc_dev *mfd_tscadc;
>> + struct tiadc_dma dma;
>> struct mutex fifo1_lock; /* to protect fifo access */
>> int channels;
>> + int total_ch_enabled;
>> u8 channel_line[8];
>> u8 channel_step[8];
>> int buffer_en_ch_steps;
>> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> u16 *data = adc_dev->data;
>>
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> +
>
> extra newline added
>
>> for (k = 0; k < fifo1count; k = k + i) {
>> for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
>> read = tiadc_readl(adc_dev, REG_FIFO1);
>> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> return IRQ_HANDLED;
>> }
>>
>> +static void tiadc_dma_rx_complete(void *param)
>> +{
>> + struct iio_dev *indio_dev = param;
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + u8 *data;
>> + int i;
>> +
>> + data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
>> + dma->valid_buf_seg = !dma->valid_buf_seg;
>> +
>> + for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
>> + iio_push_to_buffers(indio_dev, data);
>> + data += indio_dev->scan_bytes;
>> + }
>> +}
>> +
>> +static int tiadc_start_dma(struct iio_dev *indio_dev)
>> +{
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + struct dma_async_tx_descriptor *desc;
>> +
>> + dma->valid_buf_seg = false;
>> + dma->fifo_thresh = FIFO1_THRESHOLD;
>> + /*
>> + * Make the fifo thresh as the multiple of total number of
>> + * channels enabled, so make sure that that cyclic DMA period
>
> that that
>
>> + * length is also a multiple of total number of channels
>> + * enabled. This ensures that no invalid data is reported
>> + * to the stack via iio_push_to_buffers().
>> + */
>> + dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
>> + dma->buf_offset = DMA_BUFFER_SIZE / 2;
>> + /* Make sure that period length is multiple of fifo thresh level */
>> + dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
>> + sizeof(u16));
>> +
>> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
>> + dmaengine_slave_config(dma->chan, &dma->conf);
>> +
>> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
>> + dma->buf_offset * 2,
>> + dma->buf_offset, DMA_DEV_TO_MEM,
>> + DMA_PREP_INTERRUPT);
>> + if (!desc)
>> + return -EBUSY;
>> +
>> + desc->callback = tiadc_dma_rx_complete;
>> + desc->callback_param = indio_dev;
>> +
>> + dma->cookie = dmaengine_submit(desc);
>> +
>> + dma_async_issue_pending(dma->chan);
>> +
>> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
>> +
>> + return 0;
>> +}
>> +
>> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> unsigned int enb = 0;
>> u8 bit;
>> + u32 irq_enable;
>
> why u32 and not unsigned int?
>
>>
>> tiadc_step_config(indio_dev);
>> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
>> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
>> + adc_dev->total_ch_enabled++;
>> + }
>> adc_dev->buffer_en_ch_steps = enb;
>>
>> + if (dma->chan)
>> + tiadc_start_dma(indio_dev);
>> +
>> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>>
>> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
>> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
>> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
>> - | IRQENB_FIFO1OVRRUN);
>> +
>> + irq_enable = IRQENB_FIFO1OVRRUN;
>> + if (!dma->chan)
>> + irq_enable |= IRQENB_FIFO1THRES;
>> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>>
>> return 0;
>> }
>> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> int fifo1count, i, read;
>>
>> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>> adc_dev->buffer_en_ch_steps = 0;
>> + adc_dev->total_ch_enabled = 0;
>> + if (dma->chan) {
>> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
>> + dmaengine_terminate_async(dma->chan);
>> + }
>>
>> /* Flush FIFO of leftover data in the time it takes to disable adc */
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
>> .driver_module = THIS_MODULE,
>> };
>>
>> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
>> +{
>> + return false;
>> +}
>> +
>> +static int tiadc_request_dma(struct platform_device *pdev,
>> + struct tiadc_device *adc_dev)
>> +{
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + dma_cap_mask_t mask;
>> +
>> + dma->fn = the_no_dma_filter_fn;
>> +
>> + /* Default slave configuration parameters */
>> + dma->conf.direction = DMA_DEV_TO_MEM;
>> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
>> +
>> + dma_cap_zero(mask);
>> + dma_cap_set(DMA_CYCLIC, mask);
>> +
>> + /* Get a channel for RX */
>> + dma->chan = dma_request_slave_channel_compat(mask,
>> + dma->fn, dma->param,
>> + adc_dev->mfd_tscadc->dev,
>> + "fifo1");
>> + if (!dma->chan)
>> + return -ENODEV;
>> +
>> + /* RX buffer */
>> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>> + &dma->addr, GFP_KERNEL);
>> + if (!dma->buf)
>> + goto err;
>> +
>> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
>> +
>> + return 0;
>> +err:
>> + dma_release_channel(dma->chan);
>> +
>> + return -ENOMEM;
>> +}
>> +
>> static int tiadc_parse_dt(struct platform_device *pdev,
>> struct tiadc_device *adc_dev)
>> {
>> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>>
>> platform_set_drvdata(pdev, indio_dev);
>>
>> + err = tiadc_request_dma(pdev, adc_dev);
>> + if (err && err != -ENODEV)
>> + goto err_dma;
>> +
>> return 0;
>>
>> +err_dma:
>> + iio_device_unregister(indio_dev);
>> err_buffer_unregister:
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> err_free_channels:
>> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
>> {
>> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> u32 step_en;
>>
>> + if (dma->chan)
>> + dma_release_channel(dma->chan);
>> iio_device_unregister(indio_dev);
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> tiadc_channels_remove(indio_dev);
>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> index e45a208..fb9dc99 100644
>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> @@ -23,6 +23,8 @@
>> #define REG_IRQENABLE 0x02C
>> #define REG_IRQCLR 0x030
>> #define REG_IRQWAKEUP 0x034
>> +#define REG_DMAENABLE_SET 0x038
>> +#define REG_DMAENABLE_CLEAR 0x038
>
> same reg for set and clear?
Oops, will fix this and all above.
Regards
Mugunthan V N
>
>> #define REG_CTRL 0x040
>> #define REG_ADCFSM 0x044
>> #define REG_CLKDIV 0x04C
>> @@ -36,6 +38,7 @@
>> #define REG_FIFO0THR 0xE8
>> #define REG_FIFO1CNT 0xF0
>> #define REG_FIFO1THR 0xF4
>> +#define REG_DMA1REQ 0xF8
>> #define REG_FIFO0 0x100
>> #define REG_FIFO1 0x200
>>
>> @@ -126,6 +129,10 @@
>> #define FIFOREAD_DATA_MASK (0xfff << 0)
>> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>>
>> +/* DMA ENABLE/CLEAR Register */
>> +#define DMA_FIFO0 BIT(0)
>> +#define DMA_FIFO1 BIT(1)
>> +
>> /* Sequencer Status */
>> #define SEQ_STATUS BIT(5)
>> #define CHARGE_STEP 0x11
>>
>
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
[not found] ` <20160921161134.6951-3-mugunthanvnm-l0cyMroinI0@public.gmane.org>
2016-09-21 17:44 ` Peter Meerwald-Stadler
@ 2016-09-22 6:18 ` Vignesh R
0 siblings, 0 replies; 60+ messages in thread
From: Vignesh R @ 2016-09-22 6:18 UTC (permalink / raw)
To: Mugunthan V N, linux-iio
Cc: Tony Lindgren, Rob Herring, Mark Rutland, Russell King,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Lee Jones, Andrew F . Davis, linux-omap,
devicetree, linux-arm-kernel, linux-kernel, Sekhar Nori,
Peter Ujfalusi
On Wednesday 21 September 2016 09:41 PM, Mugunthan V N wrote:
> This patch adds the required pieces to ti_am335x_adc driver for
> DMA support
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
> drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
> 2 files changed, 164 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index c3cfacca..89d0b07 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -30,10 +30,32 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/kfifo_buf.h>
>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +
> +#define DMA_BUFFER_SIZE SZ_2K
> +
> +struct tiadc_dma {
> + /* Filter function */
> + dma_filter_fn fn;
> + /* Parameter to the filter function */
> + void *param;
These will not be needed with newer APIs (see below)
> + struct dma_slave_config conf;
> + struct dma_chan *chan;
> + dma_addr_t addr;
> + dma_cookie_t cookie;
> + u8 *buf;
> + bool valid_buf_seg;
> + int buf_offset;
> + u8 fifo_thresh;
> +};
> +
> struct tiadc_device {
> struct ti_tscadc_dev *mfd_tscadc;
> + struct tiadc_dma dma;
> struct mutex fifo1_lock; /* to protect fifo access */
> int channels;
> + int total_ch_enabled;
> u8 channel_line[8];
> u8 channel_step[8];
> int buffer_en_ch_steps;
> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
> u16 *data = adc_dev->data;
>
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> +
> for (k = 0; k < fifo1count; k = k + i) {
> for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
> read = tiadc_readl(adc_dev, REG_FIFO1);
> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
> return IRQ_HANDLED;
> }
>
> +static void tiadc_dma_rx_complete(void *param)
> +{
> + struct iio_dev *indio_dev = param;
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> + u8 *data;
> + int i;
> +
> + data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
> + dma->valid_buf_seg = !dma->valid_buf_seg;
> +
> + for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
> + iio_push_to_buffers(indio_dev, data);
> + data += indio_dev->scan_bytes;
> + }
> +}
> +
> +static int tiadc_start_dma(struct iio_dev *indio_dev)
> +{
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> + struct dma_async_tx_descriptor *desc;
> +
> + dma->valid_buf_seg = false;
> + dma->fifo_thresh = FIFO1_THRESHOLD;
> + /*
> + * Make the fifo thresh as the multiple of total number of
> + * channels enabled, so make sure that that cyclic DMA period
> + * length is also a multiple of total number of channels
> + * enabled. This ensures that no invalid data is reported
> + * to the stack via iio_push_to_buffers().
> + */
> + dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
Can we use rounddown(FIFO1_THRESHOLD + 1, adc_dev->total_ch_enabled)?
> + dma->buf_offset = DMA_BUFFER_SIZE / 2;
> + /* Make sure that period length is multiple of fifo thresh level */
> + dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
> + sizeof(u16));
> +
Can we use rounddown()?
> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
> + dmaengine_slave_config(dma->chan, &dma->conf);
> +
> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
> + dma->buf_offset * 2,
> + dma->buf_offset, DMA_DEV_TO_MEM,
> + DMA_PREP_INTERRUPT);
> + if (!desc)
> + return -EBUSY;
> +
> + desc->callback = tiadc_dma_rx_complete;
> + desc->callback_param = indio_dev;
> +
> + dma->cookie = dmaengine_submit(desc);
> +
> + dma_async_issue_pending(dma->chan);
> +
> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
> +
> + return 0;
> +}
> +
> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> unsigned int enb = 0;
> u8 bit;
> + u32 irq_enable;
>
> tiadc_step_config(indio_dev);
> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
> + adc_dev->total_ch_enabled++;
> + }
> adc_dev->buffer_en_ch_steps = enb;
>
> + if (dma->chan)
> + tiadc_start_dma(indio_dev);
> +
> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>
> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
> - | IRQENB_FIFO1OVRRUN);
> +
> + irq_enable = IRQENB_FIFO1OVRRUN;
> + if (!dma->chan)
> + irq_enable |= IRQENB_FIFO1THRES;
> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>
> return 0;
> }
> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> int fifo1count, i, read;
>
> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
> adc_dev->buffer_en_ch_steps = 0;
> + adc_dev->total_ch_enabled = 0;
> + if (dma->chan) {
> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
> + dmaengine_terminate_async(dma->chan);
> + }
>
> /* Flush FIFO of leftover data in the time it takes to disable adc */
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
> .driver_module = THIS_MODULE,
> };
>
> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
> +{
> + return false;
> +}
> +
> +static int tiadc_request_dma(struct platform_device *pdev,
> + struct tiadc_device *adc_dev)
> +{
> + struct tiadc_dma *dma = &adc_dev->dma;
> + dma_cap_mask_t mask;
> +
> + dma->fn = the_no_dma_filter_fn;
> +
> + /* Default slave configuration parameters */
> + dma->conf.direction = DMA_DEV_TO_MEM;
> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
> +
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_CYCLIC, mask);
> +
> + /* Get a channel for RX */
> + dma->chan = dma_request_slave_channel_compat(mask,
> + dma->fn, dma->param,
> + adc_dev->mfd_tscadc->dev,
> + "fifo1");
Please use dma_request_chan() API instead, this does not need
dma_filter_fn and probe defer can be handled.
> + if (!dma->chan)
> + return -ENODEV;
> +
> + /* RX buffer */
> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
> + &dma->addr, GFP_KERNEL);
> + if (!dma->buf)
> + goto err;
> +
> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
Do we need _ratelimited? AFAICS, this print is called only once.
> +
> + return 0;
> +err:
> + dma_release_channel(dma->chan);
> +
> + return -ENOMEM;
> +}
> +
> static int tiadc_parse_dt(struct platform_device *pdev,
> struct tiadc_device *adc_dev)
> {
> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, indio_dev);
>
> + err = tiadc_request_dma(pdev, adc_dev);
> + if (err && err != -ENODEV)
> + goto err_dma;
> +
> return 0;
>
> +err_dma:
> + iio_device_unregister(indio_dev);
> err_buffer_unregister:
> tiadc_iio_buffered_hardware_remove(indio_dev);
> err_free_channels:
> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> u32 step_en;
>
> + if (dma->chan)
> + dma_release_channel(dma->chan);
dma_free_coherent() for dma->buf?
> iio_device_unregister(indio_dev);
> tiadc_iio_buffered_hardware_remove(indio_dev);
> tiadc_channels_remove(indio_dev);
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index e45a208..fb9dc99 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -23,6 +23,8 @@
> #define REG_IRQENABLE 0x02C
> #define REG_IRQCLR 0x030
> #define REG_IRQWAKEUP 0x034
> +#define REG_DMAENABLE_SET 0x038
> +#define REG_DMAENABLE_CLEAR 0x038
> #define REG_CTRL 0x040
> #define REG_ADCFSM 0x044
> #define REG_CLKDIV 0x04C
> @@ -36,6 +38,7 @@
> #define REG_FIFO0THR 0xE8
> #define REG_FIFO1CNT 0xF0
> #define REG_FIFO1THR 0xF4
> +#define REG_DMA1REQ 0xF8
> #define REG_FIFO0 0x100
> #define REG_FIFO1 0x200
>
> @@ -126,6 +129,10 @@
> #define FIFOREAD_DATA_MASK (0xfff << 0)
> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>
> +/* DMA ENABLE/CLEAR Register */
> +#define DMA_FIFO0 BIT(0)
> +#define DMA_FIFO1 BIT(1)
> +
> /* Sequencer Status */
> #define SEQ_STATUS BIT(5)
> #define CHARGE_STEP 0x11
>
--
Regards
Vignesh
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
@ 2016-09-22 6:18 ` Vignesh R
0 siblings, 0 replies; 60+ messages in thread
From: Vignesh R @ 2016-09-22 6:18 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 21 September 2016 09:41 PM, Mugunthan V N wrote:
> This patch adds the required pieces to ti_am335x_adc driver for
> DMA support
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
> drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
> 2 files changed, 164 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index c3cfacca..89d0b07 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -30,10 +30,32 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/kfifo_buf.h>
>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +
> +#define DMA_BUFFER_SIZE SZ_2K
> +
> +struct tiadc_dma {
> + /* Filter function */
> + dma_filter_fn fn;
> + /* Parameter to the filter function */
> + void *param;
These will not be needed with newer APIs (see below)
> + struct dma_slave_config conf;
> + struct dma_chan *chan;
> + dma_addr_t addr;
> + dma_cookie_t cookie;
> + u8 *buf;
> + bool valid_buf_seg;
> + int buf_offset;
> + u8 fifo_thresh;
> +};
> +
> struct tiadc_device {
> struct ti_tscadc_dev *mfd_tscadc;
> + struct tiadc_dma dma;
> struct mutex fifo1_lock; /* to protect fifo access */
> int channels;
> + int total_ch_enabled;
> u8 channel_line[8];
> u8 channel_step[8];
> int buffer_en_ch_steps;
> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
> u16 *data = adc_dev->data;
>
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> +
> for (k = 0; k < fifo1count; k = k + i) {
> for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
> read = tiadc_readl(adc_dev, REG_FIFO1);
> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
> return IRQ_HANDLED;
> }
>
> +static void tiadc_dma_rx_complete(void *param)
> +{
> + struct iio_dev *indio_dev = param;
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> + u8 *data;
> + int i;
> +
> + data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
> + dma->valid_buf_seg = !dma->valid_buf_seg;
> +
> + for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
> + iio_push_to_buffers(indio_dev, data);
> + data += indio_dev->scan_bytes;
> + }
> +}
> +
> +static int tiadc_start_dma(struct iio_dev *indio_dev)
> +{
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> + struct dma_async_tx_descriptor *desc;
> +
> + dma->valid_buf_seg = false;
> + dma->fifo_thresh = FIFO1_THRESHOLD;
> + /*
> + * Make the fifo thresh as the multiple of total number of
> + * channels enabled, so make sure that that cyclic DMA period
> + * length is also a multiple of total number of channels
> + * enabled. This ensures that no invalid data is reported
> + * to the stack via iio_push_to_buffers().
> + */
> + dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
Can we use rounddown(FIFO1_THRESHOLD + 1, adc_dev->total_ch_enabled)?
> + dma->buf_offset = DMA_BUFFER_SIZE / 2;
> + /* Make sure that period length is multiple of fifo thresh level */
> + dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
> + sizeof(u16));
> +
Can we use rounddown()?
> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
> + dmaengine_slave_config(dma->chan, &dma->conf);
> +
> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
> + dma->buf_offset * 2,
> + dma->buf_offset, DMA_DEV_TO_MEM,
> + DMA_PREP_INTERRUPT);
> + if (!desc)
> + return -EBUSY;
> +
> + desc->callback = tiadc_dma_rx_complete;
> + desc->callback_param = indio_dev;
> +
> + dma->cookie = dmaengine_submit(desc);
> +
> + dma_async_issue_pending(dma->chan);
> +
> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
> +
> + return 0;
> +}
> +
> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> unsigned int enb = 0;
> u8 bit;
> + u32 irq_enable;
>
> tiadc_step_config(indio_dev);
> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
> + adc_dev->total_ch_enabled++;
> + }
> adc_dev->buffer_en_ch_steps = enb;
>
> + if (dma->chan)
> + tiadc_start_dma(indio_dev);
> +
> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>
> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
> - | IRQENB_FIFO1OVRRUN);
> +
> + irq_enable = IRQENB_FIFO1OVRRUN;
> + if (!dma->chan)
> + irq_enable |= IRQENB_FIFO1THRES;
> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>
> return 0;
> }
> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> int fifo1count, i, read;
>
> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
> adc_dev->buffer_en_ch_steps = 0;
> + adc_dev->total_ch_enabled = 0;
> + if (dma->chan) {
> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
> + dmaengine_terminate_async(dma->chan);
> + }
>
> /* Flush FIFO of leftover data in the time it takes to disable adc */
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
> .driver_module = THIS_MODULE,
> };
>
> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
> +{
> + return false;
> +}
> +
> +static int tiadc_request_dma(struct platform_device *pdev,
> + struct tiadc_device *adc_dev)
> +{
> + struct tiadc_dma *dma = &adc_dev->dma;
> + dma_cap_mask_t mask;
> +
> + dma->fn = the_no_dma_filter_fn;
> +
> + /* Default slave configuration parameters */
> + dma->conf.direction = DMA_DEV_TO_MEM;
> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
> +
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_CYCLIC, mask);
> +
> + /* Get a channel for RX */
> + dma->chan = dma_request_slave_channel_compat(mask,
> + dma->fn, dma->param,
> + adc_dev->mfd_tscadc->dev,
> + "fifo1");
Please use dma_request_chan() API instead, this does not need
dma_filter_fn and probe defer can be handled.
> + if (!dma->chan)
> + return -ENODEV;
> +
> + /* RX buffer */
> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
> + &dma->addr, GFP_KERNEL);
> + if (!dma->buf)
> + goto err;
> +
> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
Do we need _ratelimited? AFAICS, this print is called only once.
> +
> + return 0;
> +err:
> + dma_release_channel(dma->chan);
> +
> + return -ENOMEM;
> +}
> +
> static int tiadc_parse_dt(struct platform_device *pdev,
> struct tiadc_device *adc_dev)
> {
> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, indio_dev);
>
> + err = tiadc_request_dma(pdev, adc_dev);
> + if (err && err != -ENODEV)
> + goto err_dma;
> +
> return 0;
>
> +err_dma:
> + iio_device_unregister(indio_dev);
> err_buffer_unregister:
> tiadc_iio_buffered_hardware_remove(indio_dev);
> err_free_channels:
> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> u32 step_en;
>
> + if (dma->chan)
> + dma_release_channel(dma->chan);
dma_free_coherent() for dma->buf?
> iio_device_unregister(indio_dev);
> tiadc_iio_buffered_hardware_remove(indio_dev);
> tiadc_channels_remove(indio_dev);
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index e45a208..fb9dc99 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -23,6 +23,8 @@
> #define REG_IRQENABLE 0x02C
> #define REG_IRQCLR 0x030
> #define REG_IRQWAKEUP 0x034
> +#define REG_DMAENABLE_SET 0x038
> +#define REG_DMAENABLE_CLEAR 0x038
> #define REG_CTRL 0x040
> #define REG_ADCFSM 0x044
> #define REG_CLKDIV 0x04C
> @@ -36,6 +38,7 @@
> #define REG_FIFO0THR 0xE8
> #define REG_FIFO1CNT 0xF0
> #define REG_FIFO1THR 0xF4
> +#define REG_DMA1REQ 0xF8
> #define REG_FIFO0 0x100
> #define REG_FIFO1 0x200
>
> @@ -126,6 +129,10 @@
> #define FIFOREAD_DATA_MASK (0xfff << 0)
> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>
> +/* DMA ENABLE/CLEAR Register */
> +#define DMA_FIFO0 BIT(0)
> +#define DMA_FIFO1 BIT(1)
> +
> /* Sequencer Status */
> #define SEQ_STATUS BIT(5)
> #define CHARGE_STEP 0x11
>
--
Regards
Vignesh
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
@ 2016-09-22 6:18 ` Vignesh R
0 siblings, 0 replies; 60+ messages in thread
From: Vignesh R @ 2016-09-22 6:18 UTC (permalink / raw)
To: Mugunthan V N, linux-iio-u79uwXL29TY76Z2rM5mHXA
Cc: Tony Lindgren, Rob Herring, Mark Rutland, Russell King,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Lee Jones, Andrew F . Davis,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori, Peter Ujfalusi
On Wednesday 21 September 2016 09:41 PM, Mugunthan V N wrote:
> This patch adds the required pieces to ti_am335x_adc driver for
> DMA support
>
> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
> ---
> drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
> 2 files changed, 164 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index c3cfacca..89d0b07 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -30,10 +30,32 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/kfifo_buf.h>
>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +
> +#define DMA_BUFFER_SIZE SZ_2K
> +
> +struct tiadc_dma {
> + /* Filter function */
> + dma_filter_fn fn;
> + /* Parameter to the filter function */
> + void *param;
These will not be needed with newer APIs (see below)
> + struct dma_slave_config conf;
> + struct dma_chan *chan;
> + dma_addr_t addr;
> + dma_cookie_t cookie;
> + u8 *buf;
> + bool valid_buf_seg;
> + int buf_offset;
> + u8 fifo_thresh;
> +};
> +
> struct tiadc_device {
> struct ti_tscadc_dev *mfd_tscadc;
> + struct tiadc_dma dma;
> struct mutex fifo1_lock; /* to protect fifo access */
> int channels;
> + int total_ch_enabled;
> u8 channel_line[8];
> u8 channel_step[8];
> int buffer_en_ch_steps;
> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
> u16 *data = adc_dev->data;
>
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> +
> for (k = 0; k < fifo1count; k = k + i) {
> for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
> read = tiadc_readl(adc_dev, REG_FIFO1);
> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
> return IRQ_HANDLED;
> }
>
> +static void tiadc_dma_rx_complete(void *param)
> +{
> + struct iio_dev *indio_dev = param;
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> + u8 *data;
> + int i;
> +
> + data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
> + dma->valid_buf_seg = !dma->valid_buf_seg;
> +
> + for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
> + iio_push_to_buffers(indio_dev, data);
> + data += indio_dev->scan_bytes;
> + }
> +}
> +
> +static int tiadc_start_dma(struct iio_dev *indio_dev)
> +{
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> + struct dma_async_tx_descriptor *desc;
> +
> + dma->valid_buf_seg = false;
> + dma->fifo_thresh = FIFO1_THRESHOLD;
> + /*
> + * Make the fifo thresh as the multiple of total number of
> + * channels enabled, so make sure that that cyclic DMA period
> + * length is also a multiple of total number of channels
> + * enabled. This ensures that no invalid data is reported
> + * to the stack via iio_push_to_buffers().
> + */
> + dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
Can we use rounddown(FIFO1_THRESHOLD + 1, adc_dev->total_ch_enabled)?
> + dma->buf_offset = DMA_BUFFER_SIZE / 2;
> + /* Make sure that period length is multiple of fifo thresh level */
> + dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
> + sizeof(u16));
> +
Can we use rounddown()?
> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
> + dmaengine_slave_config(dma->chan, &dma->conf);
> +
> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
> + dma->buf_offset * 2,
> + dma->buf_offset, DMA_DEV_TO_MEM,
> + DMA_PREP_INTERRUPT);
> + if (!desc)
> + return -EBUSY;
> +
> + desc->callback = tiadc_dma_rx_complete;
> + desc->callback_param = indio_dev;
> +
> + dma->cookie = dmaengine_submit(desc);
> +
> + dma_async_issue_pending(dma->chan);
> +
> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
> +
> + return 0;
> +}
> +
> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> unsigned int enb = 0;
> u8 bit;
> + u32 irq_enable;
>
> tiadc_step_config(indio_dev);
> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
> + adc_dev->total_ch_enabled++;
> + }
> adc_dev->buffer_en_ch_steps = enb;
>
> + if (dma->chan)
> + tiadc_start_dma(indio_dev);
> +
> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>
> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
> - | IRQENB_FIFO1OVRRUN);
> +
> + irq_enable = IRQENB_FIFO1OVRRUN;
> + if (!dma->chan)
> + irq_enable |= IRQENB_FIFO1THRES;
> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>
> return 0;
> }
> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> int fifo1count, i, read;
>
> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
> adc_dev->buffer_en_ch_steps = 0;
> + adc_dev->total_ch_enabled = 0;
> + if (dma->chan) {
> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
> + dmaengine_terminate_async(dma->chan);
> + }
>
> /* Flush FIFO of leftover data in the time it takes to disable adc */
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
> .driver_module = THIS_MODULE,
> };
>
> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
> +{
> + return false;
> +}
> +
> +static int tiadc_request_dma(struct platform_device *pdev,
> + struct tiadc_device *adc_dev)
> +{
> + struct tiadc_dma *dma = &adc_dev->dma;
> + dma_cap_mask_t mask;
> +
> + dma->fn = the_no_dma_filter_fn;
> +
> + /* Default slave configuration parameters */
> + dma->conf.direction = DMA_DEV_TO_MEM;
> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
> +
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_CYCLIC, mask);
> +
> + /* Get a channel for RX */
> + dma->chan = dma_request_slave_channel_compat(mask,
> + dma->fn, dma->param,
> + adc_dev->mfd_tscadc->dev,
> + "fifo1");
Please use dma_request_chan() API instead, this does not need
dma_filter_fn and probe defer can be handled.
> + if (!dma->chan)
> + return -ENODEV;
> +
> + /* RX buffer */
> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
> + &dma->addr, GFP_KERNEL);
> + if (!dma->buf)
> + goto err;
> +
> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
Do we need _ratelimited? AFAICS, this print is called only once.
> +
> + return 0;
> +err:
> + dma_release_channel(dma->chan);
> +
> + return -ENOMEM;
> +}
> +
> static int tiadc_parse_dt(struct platform_device *pdev,
> struct tiadc_device *adc_dev)
> {
> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, indio_dev);
>
> + err = tiadc_request_dma(pdev, adc_dev);
> + if (err && err != -ENODEV)
> + goto err_dma;
> +
> return 0;
>
> +err_dma:
> + iio_device_unregister(indio_dev);
> err_buffer_unregister:
> tiadc_iio_buffered_hardware_remove(indio_dev);
> err_free_channels:
> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> u32 step_en;
>
> + if (dma->chan)
> + dma_release_channel(dma->chan);
dma_free_coherent() for dma->buf?
> iio_device_unregister(indio_dev);
> tiadc_iio_buffered_hardware_remove(indio_dev);
> tiadc_channels_remove(indio_dev);
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index e45a208..fb9dc99 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -23,6 +23,8 @@
> #define REG_IRQENABLE 0x02C
> #define REG_IRQCLR 0x030
> #define REG_IRQWAKEUP 0x034
> +#define REG_DMAENABLE_SET 0x038
> +#define REG_DMAENABLE_CLEAR 0x038
> #define REG_CTRL 0x040
> #define REG_ADCFSM 0x044
> #define REG_CLKDIV 0x04C
> @@ -36,6 +38,7 @@
> #define REG_FIFO0THR 0xE8
> #define REG_FIFO1CNT 0xF0
> #define REG_FIFO1THR 0xF4
> +#define REG_DMA1REQ 0xF8
> #define REG_FIFO0 0x100
> #define REG_FIFO1 0x200
>
> @@ -126,6 +129,10 @@
> #define FIFOREAD_DATA_MASK (0xfff << 0)
> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>
> +/* DMA ENABLE/CLEAR Register */
> +#define DMA_FIFO0 BIT(0)
> +#define DMA_FIFO1 BIT(1)
> +
> /* Sequencer Status */
> #define SEQ_STATUS BIT(5)
> #define CHARGE_STEP 0x11
>
--
Regards
Vignesh
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
2016-09-22 6:18 ` Vignesh R
(?)
@ 2016-09-22 10:45 ` Mugunthan V N
-1 siblings, 0 replies; 60+ messages in thread
From: Mugunthan V N @ 2016-09-22 10:45 UTC (permalink / raw)
To: Vignesh R, linux-iio
Cc: Tony Lindgren, Rob Herring, Mark Rutland, Russell King,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Lee Jones, Andrew F . Davis, linux-omap,
devicetree, linux-arm-kernel, linux-kernel, Sekhar Nori,
Peter Ujfalusi
On Thursday 22 September 2016 11:48 AM, Vignesh R wrote:
>
>
> On Wednesday 21 September 2016 09:41 PM, Mugunthan V N wrote:
>> This patch adds the required pieces to ti_am335x_adc driver for
>> DMA support
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> ---
>> drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
>> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
>> 2 files changed, 164 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
>> index c3cfacca..89d0b07 100644
>> --- a/drivers/iio/adc/ti_am335x_adc.c
>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>> @@ -30,10 +30,32 @@
>> #include <linux/iio/buffer.h>
>> #include <linux/iio/kfifo_buf.h>
>>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#define DMA_BUFFER_SIZE SZ_2K
>> +
>> +struct tiadc_dma {
>> + /* Filter function */
>> + dma_filter_fn fn;
>> + /* Parameter to the filter function */
>> + void *param;
>
> These will not be needed with newer APIs (see below)
>
>> + struct dma_slave_config conf;
>> + struct dma_chan *chan;
>> + dma_addr_t addr;
>> + dma_cookie_t cookie;
>> + u8 *buf;
>> + bool valid_buf_seg;
>> + int buf_offset;
>> + u8 fifo_thresh;
>> +};
>> +
>> struct tiadc_device {
>> struct ti_tscadc_dev *mfd_tscadc;
>> + struct tiadc_dma dma;
>> struct mutex fifo1_lock; /* to protect fifo access */
>> int channels;
>> + int total_ch_enabled;
>> u8 channel_line[8];
>> u8 channel_step[8];
>> int buffer_en_ch_steps;
>> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> u16 *data = adc_dev->data;
>>
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> +
>> for (k = 0; k < fifo1count; k = k + i) {
>> for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
>> read = tiadc_readl(adc_dev, REG_FIFO1);
>> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> return IRQ_HANDLED;
>> }
>>
>> +static void tiadc_dma_rx_complete(void *param)
>> +{
>> + struct iio_dev *indio_dev = param;
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + u8 *data;
>> + int i;
>> +
>> + data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
>> + dma->valid_buf_seg = !dma->valid_buf_seg;
>> +
>> + for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
>> + iio_push_to_buffers(indio_dev, data);
>> + data += indio_dev->scan_bytes;
>> + }
>> +}
>> +
>> +static int tiadc_start_dma(struct iio_dev *indio_dev)
>> +{
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + struct dma_async_tx_descriptor *desc;
>> +
>> + dma->valid_buf_seg = false;
>> + dma->fifo_thresh = FIFO1_THRESHOLD;
>> + /*
>> + * Make the fifo thresh as the multiple of total number of
>> + * channels enabled, so make sure that that cyclic DMA period
>> + * length is also a multiple of total number of channels
>> + * enabled. This ensures that no invalid data is reported
>> + * to the stack via iio_push_to_buffers().
>> + */
>> + dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
>
> Can we use rounddown(FIFO1_THRESHOLD + 1, adc_dev->total_ch_enabled)?
>
>> + dma->buf_offset = DMA_BUFFER_SIZE / 2;
>> + /* Make sure that period length is multiple of fifo thresh level */
>> + dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
>> + sizeof(u16));
>> +
>
> Can we use rounddown()?
Will change this in next version.
>
>> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
>> + dmaengine_slave_config(dma->chan, &dma->conf);
>> +
>> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
>> + dma->buf_offset * 2,
>> + dma->buf_offset, DMA_DEV_TO_MEM,
>> + DMA_PREP_INTERRUPT);
>> + if (!desc)
>> + return -EBUSY;
>> +
>> + desc->callback = tiadc_dma_rx_complete;
>> + desc->callback_param = indio_dev;
>> +
>> + dma->cookie = dmaengine_submit(desc);
>> +
>> + dma_async_issue_pending(dma->chan);
>> +
>> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
>> +
>> + return 0;
>> +}
>> +
>> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> unsigned int enb = 0;
>> u8 bit;
>> + u32 irq_enable;
>>
>> tiadc_step_config(indio_dev);
>> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
>> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
>> + adc_dev->total_ch_enabled++;
>> + }
>> adc_dev->buffer_en_ch_steps = enb;
>>
>> + if (dma->chan)
>> + tiadc_start_dma(indio_dev);
>> +
>> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>>
>> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
>> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
>> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
>> - | IRQENB_FIFO1OVRRUN);
>> +
>> + irq_enable = IRQENB_FIFO1OVRRUN;
>> + if (!dma->chan)
>> + irq_enable |= IRQENB_FIFO1THRES;
>> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>>
>> return 0;
>> }
>> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> int fifo1count, i, read;
>>
>> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>> adc_dev->buffer_en_ch_steps = 0;
>> + adc_dev->total_ch_enabled = 0;
>> + if (dma->chan) {
>> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
>> + dmaengine_terminate_async(dma->chan);
>> + }
>>
>> /* Flush FIFO of leftover data in the time it takes to disable adc */
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
>> .driver_module = THIS_MODULE,
>> };
>>
>> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
>> +{
>> + return false;
>> +}
>> +
>> +static int tiadc_request_dma(struct platform_device *pdev,
>> + struct tiadc_device *adc_dev)
>> +{
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + dma_cap_mask_t mask;
>> +
>> + dma->fn = the_no_dma_filter_fn;
>> +
>> + /* Default slave configuration parameters */
>> + dma->conf.direction = DMA_DEV_TO_MEM;
>> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
>> +
>> + dma_cap_zero(mask);
>> + dma_cap_set(DMA_CYCLIC, mask);
>> +
>> + /* Get a channel for RX */
>> + dma->chan = dma_request_slave_channel_compat(mask,
>> + dma->fn, dma->param,
>> + adc_dev->mfd_tscadc->dev,
>> + "fifo1");
>
> Please use dma_request_chan() API instead, this does not need
> dma_filter_fn and probe defer can be handled.
Okay
>
>> + if (!dma->chan)
>> + return -ENODEV;
>> +
>> + /* RX buffer */
>> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>> + &dma->addr, GFP_KERNEL);
>> + if (!dma->buf)
>> + goto err;
>> +
>> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
>
> Do we need _ratelimited? AFAICS, this print is called only once.
True, will change to dev_dbg.
>
>> +
>> + return 0;
>> +err:
>> + dma_release_channel(dma->chan);
>> +
>> + return -ENOMEM;
>> +}
>> +
>> static int tiadc_parse_dt(struct platform_device *pdev,
>> struct tiadc_device *adc_dev)
>> {
>> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>>
>> platform_set_drvdata(pdev, indio_dev);
>>
>> + err = tiadc_request_dma(pdev, adc_dev);
>> + if (err && err != -ENODEV)
>> + goto err_dma;
>> +
>> return 0;
>>
>> +err_dma:
>> + iio_device_unregister(indio_dev);
>> err_buffer_unregister:
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> err_free_channels:
>> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
>> {
>> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> u32 step_en;
>>
>> + if (dma->chan)
>> + dma_release_channel(dma->chan);
>
> dma_free_coherent() for dma->buf?
Oops missed, will fix in next version.
Regards
Mugunthan V N
>
>> iio_device_unregister(indio_dev);
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> tiadc_channels_remove(indio_dev);
>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> index e45a208..fb9dc99 100644
>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> @@ -23,6 +23,8 @@
>> #define REG_IRQENABLE 0x02C
>> #define REG_IRQCLR 0x030
>> #define REG_IRQWAKEUP 0x034
>> +#define REG_DMAENABLE_SET 0x038
>> +#define REG_DMAENABLE_CLEAR 0x038
>> #define REG_CTRL 0x040
>> #define REG_ADCFSM 0x044
>> #define REG_CLKDIV 0x04C
>> @@ -36,6 +38,7 @@
>> #define REG_FIFO0THR 0xE8
>> #define REG_FIFO1CNT 0xF0
>> #define REG_FIFO1THR 0xF4
>> +#define REG_DMA1REQ 0xF8
>> #define REG_FIFO0 0x100
>> #define REG_FIFO1 0x200
>>
>> @@ -126,6 +129,10 @@
>> #define FIFOREAD_DATA_MASK (0xfff << 0)
>> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>>
>> +/* DMA ENABLE/CLEAR Register */
>> +#define DMA_FIFO0 BIT(0)
>> +#define DMA_FIFO1 BIT(1)
>> +
>> /* Sequencer Status */
>> #define SEQ_STATUS BIT(5)
>> #define CHARGE_STEP 0x11
>>
>
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
@ 2016-09-22 10:45 ` Mugunthan V N
0 siblings, 0 replies; 60+ messages in thread
From: Mugunthan V N @ 2016-09-22 10:45 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 22 September 2016 11:48 AM, Vignesh R wrote:
>
>
> On Wednesday 21 September 2016 09:41 PM, Mugunthan V N wrote:
>> This patch adds the required pieces to ti_am335x_adc driver for
>> DMA support
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> ---
>> drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
>> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
>> 2 files changed, 164 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
>> index c3cfacca..89d0b07 100644
>> --- a/drivers/iio/adc/ti_am335x_adc.c
>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>> @@ -30,10 +30,32 @@
>> #include <linux/iio/buffer.h>
>> #include <linux/iio/kfifo_buf.h>
>>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#define DMA_BUFFER_SIZE SZ_2K
>> +
>> +struct tiadc_dma {
>> + /* Filter function */
>> + dma_filter_fn fn;
>> + /* Parameter to the filter function */
>> + void *param;
>
> These will not be needed with newer APIs (see below)
>
>> + struct dma_slave_config conf;
>> + struct dma_chan *chan;
>> + dma_addr_t addr;
>> + dma_cookie_t cookie;
>> + u8 *buf;
>> + bool valid_buf_seg;
>> + int buf_offset;
>> + u8 fifo_thresh;
>> +};
>> +
>> struct tiadc_device {
>> struct ti_tscadc_dev *mfd_tscadc;
>> + struct tiadc_dma dma;
>> struct mutex fifo1_lock; /* to protect fifo access */
>> int channels;
>> + int total_ch_enabled;
>> u8 channel_line[8];
>> u8 channel_step[8];
>> int buffer_en_ch_steps;
>> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> u16 *data = adc_dev->data;
>>
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> +
>> for (k = 0; k < fifo1count; k = k + i) {
>> for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
>> read = tiadc_readl(adc_dev, REG_FIFO1);
>> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> return IRQ_HANDLED;
>> }
>>
>> +static void tiadc_dma_rx_complete(void *param)
>> +{
>> + struct iio_dev *indio_dev = param;
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + u8 *data;
>> + int i;
>> +
>> + data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
>> + dma->valid_buf_seg = !dma->valid_buf_seg;
>> +
>> + for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
>> + iio_push_to_buffers(indio_dev, data);
>> + data += indio_dev->scan_bytes;
>> + }
>> +}
>> +
>> +static int tiadc_start_dma(struct iio_dev *indio_dev)
>> +{
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + struct dma_async_tx_descriptor *desc;
>> +
>> + dma->valid_buf_seg = false;
>> + dma->fifo_thresh = FIFO1_THRESHOLD;
>> + /*
>> + * Make the fifo thresh as the multiple of total number of
>> + * channels enabled, so make sure that that cyclic DMA period
>> + * length is also a multiple of total number of channels
>> + * enabled. This ensures that no invalid data is reported
>> + * to the stack via iio_push_to_buffers().
>> + */
>> + dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
>
> Can we use rounddown(FIFO1_THRESHOLD + 1, adc_dev->total_ch_enabled)?
>
>> + dma->buf_offset = DMA_BUFFER_SIZE / 2;
>> + /* Make sure that period length is multiple of fifo thresh level */
>> + dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
>> + sizeof(u16));
>> +
>
> Can we use rounddown()?
Will change this in next version.
>
>> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
>> + dmaengine_slave_config(dma->chan, &dma->conf);
>> +
>> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
>> + dma->buf_offset * 2,
>> + dma->buf_offset, DMA_DEV_TO_MEM,
>> + DMA_PREP_INTERRUPT);
>> + if (!desc)
>> + return -EBUSY;
>> +
>> + desc->callback = tiadc_dma_rx_complete;
>> + desc->callback_param = indio_dev;
>> +
>> + dma->cookie = dmaengine_submit(desc);
>> +
>> + dma_async_issue_pending(dma->chan);
>> +
>> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
>> +
>> + return 0;
>> +}
>> +
>> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> unsigned int enb = 0;
>> u8 bit;
>> + u32 irq_enable;
>>
>> tiadc_step_config(indio_dev);
>> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
>> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
>> + adc_dev->total_ch_enabled++;
>> + }
>> adc_dev->buffer_en_ch_steps = enb;
>>
>> + if (dma->chan)
>> + tiadc_start_dma(indio_dev);
>> +
>> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>>
>> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
>> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
>> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
>> - | IRQENB_FIFO1OVRRUN);
>> +
>> + irq_enable = IRQENB_FIFO1OVRRUN;
>> + if (!dma->chan)
>> + irq_enable |= IRQENB_FIFO1THRES;
>> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>>
>> return 0;
>> }
>> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> int fifo1count, i, read;
>>
>> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>> adc_dev->buffer_en_ch_steps = 0;
>> + adc_dev->total_ch_enabled = 0;
>> + if (dma->chan) {
>> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
>> + dmaengine_terminate_async(dma->chan);
>> + }
>>
>> /* Flush FIFO of leftover data in the time it takes to disable adc */
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
>> .driver_module = THIS_MODULE,
>> };
>>
>> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
>> +{
>> + return false;
>> +}
>> +
>> +static int tiadc_request_dma(struct platform_device *pdev,
>> + struct tiadc_device *adc_dev)
>> +{
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + dma_cap_mask_t mask;
>> +
>> + dma->fn = the_no_dma_filter_fn;
>> +
>> + /* Default slave configuration parameters */
>> + dma->conf.direction = DMA_DEV_TO_MEM;
>> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
>> +
>> + dma_cap_zero(mask);
>> + dma_cap_set(DMA_CYCLIC, mask);
>> +
>> + /* Get a channel for RX */
>> + dma->chan = dma_request_slave_channel_compat(mask,
>> + dma->fn, dma->param,
>> + adc_dev->mfd_tscadc->dev,
>> + "fifo1");
>
> Please use dma_request_chan() API instead, this does not need
> dma_filter_fn and probe defer can be handled.
Okay
>
>> + if (!dma->chan)
>> + return -ENODEV;
>> +
>> + /* RX buffer */
>> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>> + &dma->addr, GFP_KERNEL);
>> + if (!dma->buf)
>> + goto err;
>> +
>> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
>
> Do we need _ratelimited? AFAICS, this print is called only once.
True, will change to dev_dbg.
>
>> +
>> + return 0;
>> +err:
>> + dma_release_channel(dma->chan);
>> +
>> + return -ENOMEM;
>> +}
>> +
>> static int tiadc_parse_dt(struct platform_device *pdev,
>> struct tiadc_device *adc_dev)
>> {
>> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>>
>> platform_set_drvdata(pdev, indio_dev);
>>
>> + err = tiadc_request_dma(pdev, adc_dev);
>> + if (err && err != -ENODEV)
>> + goto err_dma;
>> +
>> return 0;
>>
>> +err_dma:
>> + iio_device_unregister(indio_dev);
>> err_buffer_unregister:
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> err_free_channels:
>> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
>> {
>> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> u32 step_en;
>>
>> + if (dma->chan)
>> + dma_release_channel(dma->chan);
>
> dma_free_coherent() for dma->buf?
Oops missed, will fix in next version.
Regards
Mugunthan V N
>
>> iio_device_unregister(indio_dev);
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> tiadc_channels_remove(indio_dev);
>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> index e45a208..fb9dc99 100644
>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> @@ -23,6 +23,8 @@
>> #define REG_IRQENABLE 0x02C
>> #define REG_IRQCLR 0x030
>> #define REG_IRQWAKEUP 0x034
>> +#define REG_DMAENABLE_SET 0x038
>> +#define REG_DMAENABLE_CLEAR 0x038
>> #define REG_CTRL 0x040
>> #define REG_ADCFSM 0x044
>> #define REG_CLKDIV 0x04C
>> @@ -36,6 +38,7 @@
>> #define REG_FIFO0THR 0xE8
>> #define REG_FIFO1CNT 0xF0
>> #define REG_FIFO1THR 0xF4
>> +#define REG_DMA1REQ 0xF8
>> #define REG_FIFO0 0x100
>> #define REG_FIFO1 0x200
>>
>> @@ -126,6 +129,10 @@
>> #define FIFOREAD_DATA_MASK (0xfff << 0)
>> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>>
>> +/* DMA ENABLE/CLEAR Register */
>> +#define DMA_FIFO0 BIT(0)
>> +#define DMA_FIFO1 BIT(1)
>> +
>> /* Sequencer Status */
>> #define SEQ_STATUS BIT(5)
>> #define CHARGE_STEP 0x11
>>
>
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
@ 2016-09-22 10:45 ` Mugunthan V N
0 siblings, 0 replies; 60+ messages in thread
From: Mugunthan V N @ 2016-09-22 10:45 UTC (permalink / raw)
To: Vignesh R, linux-iio
Cc: Mark Rutland, devicetree, Lars-Peter Clausen, Tony Lindgren,
linux-kernel, Sekhar Nori, Russell King, Andrew F . Davis,
Rob Herring, linux-arm-kernel, Peter Meerwald-Stadler,
Hartmut Knaack, linux-omap, Lee Jones, Jonathan Cameron
On Thursday 22 September 2016 11:48 AM, Vignesh R wrote:
>
>
> On Wednesday 21 September 2016 09:41 PM, Mugunthan V N wrote:
>> This patch adds the required pieces to ti_am335x_adc driver for
>> DMA support
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> ---
>> drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
>> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
>> 2 files changed, 164 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
>> index c3cfacca..89d0b07 100644
>> --- a/drivers/iio/adc/ti_am335x_adc.c
>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>> @@ -30,10 +30,32 @@
>> #include <linux/iio/buffer.h>
>> #include <linux/iio/kfifo_buf.h>
>>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#define DMA_BUFFER_SIZE SZ_2K
>> +
>> +struct tiadc_dma {
>> + /* Filter function */
>> + dma_filter_fn fn;
>> + /* Parameter to the filter function */
>> + void *param;
>
> These will not be needed with newer APIs (see below)
>
>> + struct dma_slave_config conf;
>> + struct dma_chan *chan;
>> + dma_addr_t addr;
>> + dma_cookie_t cookie;
>> + u8 *buf;
>> + bool valid_buf_seg;
>> + int buf_offset;
>> + u8 fifo_thresh;
>> +};
>> +
>> struct tiadc_device {
>> struct ti_tscadc_dev *mfd_tscadc;
>> + struct tiadc_dma dma;
>> struct mutex fifo1_lock; /* to protect fifo access */
>> int channels;
>> + int total_ch_enabled;
>> u8 channel_line[8];
>> u8 channel_step[8];
>> int buffer_en_ch_steps;
>> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> u16 *data = adc_dev->data;
>>
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> +
>> for (k = 0; k < fifo1count; k = k + i) {
>> for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
>> read = tiadc_readl(adc_dev, REG_FIFO1);
>> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> return IRQ_HANDLED;
>> }
>>
>> +static void tiadc_dma_rx_complete(void *param)
>> +{
>> + struct iio_dev *indio_dev = param;
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + u8 *data;
>> + int i;
>> +
>> + data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
>> + dma->valid_buf_seg = !dma->valid_buf_seg;
>> +
>> + for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
>> + iio_push_to_buffers(indio_dev, data);
>> + data += indio_dev->scan_bytes;
>> + }
>> +}
>> +
>> +static int tiadc_start_dma(struct iio_dev *indio_dev)
>> +{
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + struct dma_async_tx_descriptor *desc;
>> +
>> + dma->valid_buf_seg = false;
>> + dma->fifo_thresh = FIFO1_THRESHOLD;
>> + /*
>> + * Make the fifo thresh as the multiple of total number of
>> + * channels enabled, so make sure that that cyclic DMA period
>> + * length is also a multiple of total number of channels
>> + * enabled. This ensures that no invalid data is reported
>> + * to the stack via iio_push_to_buffers().
>> + */
>> + dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
>
> Can we use rounddown(FIFO1_THRESHOLD + 1, adc_dev->total_ch_enabled)?
>
>> + dma->buf_offset = DMA_BUFFER_SIZE / 2;
>> + /* Make sure that period length is multiple of fifo thresh level */
>> + dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
>> + sizeof(u16));
>> +
>
> Can we use rounddown()?
Will change this in next version.
>
>> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
>> + dmaengine_slave_config(dma->chan, &dma->conf);
>> +
>> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
>> + dma->buf_offset * 2,
>> + dma->buf_offset, DMA_DEV_TO_MEM,
>> + DMA_PREP_INTERRUPT);
>> + if (!desc)
>> + return -EBUSY;
>> +
>> + desc->callback = tiadc_dma_rx_complete;
>> + desc->callback_param = indio_dev;
>> +
>> + dma->cookie = dmaengine_submit(desc);
>> +
>> + dma_async_issue_pending(dma->chan);
>> +
>> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
>> +
>> + return 0;
>> +}
>> +
>> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> unsigned int enb = 0;
>> u8 bit;
>> + u32 irq_enable;
>>
>> tiadc_step_config(indio_dev);
>> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
>> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
>> + adc_dev->total_ch_enabled++;
>> + }
>> adc_dev->buffer_en_ch_steps = enb;
>>
>> + if (dma->chan)
>> + tiadc_start_dma(indio_dev);
>> +
>> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>>
>> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
>> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
>> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
>> - | IRQENB_FIFO1OVRRUN);
>> +
>> + irq_enable = IRQENB_FIFO1OVRRUN;
>> + if (!dma->chan)
>> + irq_enable |= IRQENB_FIFO1THRES;
>> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>>
>> return 0;
>> }
>> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> int fifo1count, i, read;
>>
>> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>> adc_dev->buffer_en_ch_steps = 0;
>> + adc_dev->total_ch_enabled = 0;
>> + if (dma->chan) {
>> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
>> + dmaengine_terminate_async(dma->chan);
>> + }
>>
>> /* Flush FIFO of leftover data in the time it takes to disable adc */
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
>> .driver_module = THIS_MODULE,
>> };
>>
>> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
>> +{
>> + return false;
>> +}
>> +
>> +static int tiadc_request_dma(struct platform_device *pdev,
>> + struct tiadc_device *adc_dev)
>> +{
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + dma_cap_mask_t mask;
>> +
>> + dma->fn = the_no_dma_filter_fn;
>> +
>> + /* Default slave configuration parameters */
>> + dma->conf.direction = DMA_DEV_TO_MEM;
>> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
>> +
>> + dma_cap_zero(mask);
>> + dma_cap_set(DMA_CYCLIC, mask);
>> +
>> + /* Get a channel for RX */
>> + dma->chan = dma_request_slave_channel_compat(mask,
>> + dma->fn, dma->param,
>> + adc_dev->mfd_tscadc->dev,
>> + "fifo1");
>
> Please use dma_request_chan() API instead, this does not need
> dma_filter_fn and probe defer can be handled.
Okay
>
>> + if (!dma->chan)
>> + return -ENODEV;
>> +
>> + /* RX buffer */
>> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>> + &dma->addr, GFP_KERNEL);
>> + if (!dma->buf)
>> + goto err;
>> +
>> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
>
> Do we need _ratelimited? AFAICS, this print is called only once.
True, will change to dev_dbg.
>
>> +
>> + return 0;
>> +err:
>> + dma_release_channel(dma->chan);
>> +
>> + return -ENOMEM;
>> +}
>> +
>> static int tiadc_parse_dt(struct platform_device *pdev,
>> struct tiadc_device *adc_dev)
>> {
>> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>>
>> platform_set_drvdata(pdev, indio_dev);
>>
>> + err = tiadc_request_dma(pdev, adc_dev);
>> + if (err && err != -ENODEV)
>> + goto err_dma;
>> +
>> return 0;
>>
>> +err_dma:
>> + iio_device_unregister(indio_dev);
>> err_buffer_unregister:
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> err_free_channels:
>> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
>> {
>> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> u32 step_en;
>>
>> + if (dma->chan)
>> + dma_release_channel(dma->chan);
>
> dma_free_coherent() for dma->buf?
Oops missed, will fix in next version.
Regards
Mugunthan V N
>
>> iio_device_unregister(indio_dev);
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> tiadc_channels_remove(indio_dev);
>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> index e45a208..fb9dc99 100644
>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> @@ -23,6 +23,8 @@
>> #define REG_IRQENABLE 0x02C
>> #define REG_IRQCLR 0x030
>> #define REG_IRQWAKEUP 0x034
>> +#define REG_DMAENABLE_SET 0x038
>> +#define REG_DMAENABLE_CLEAR 0x038
>> #define REG_CTRL 0x040
>> #define REG_ADCFSM 0x044
>> #define REG_CLKDIV 0x04C
>> @@ -36,6 +38,7 @@
>> #define REG_FIFO0THR 0xE8
>> #define REG_FIFO1CNT 0xF0
>> #define REG_FIFO1THR 0xF4
>> +#define REG_DMA1REQ 0xF8
>> #define REG_FIFO0 0x100
>> #define REG_FIFO1 0x200
>>
>> @@ -126,6 +129,10 @@
>> #define FIFOREAD_DATA_MASK (0xfff << 0)
>> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>>
>> +/* DMA ENABLE/CLEAR Register */
>> +#define DMA_FIFO0 BIT(0)
>> +#define DMA_FIFO1 BIT(1)
>> +
>> /* Sequencer Status */
>> #define SEQ_STATUS BIT(5)
>> #define CHARGE_STEP 0x11
>>
>
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
2016-09-22 10:45 ` Mugunthan V N
(?)
@ 2016-09-22 11:16 ` Peter Ujfalusi
-1 siblings, 0 replies; 60+ messages in thread
From: Peter Ujfalusi @ 2016-09-22 11:16 UTC (permalink / raw)
To: Mugunthan V N, Vignesh R, linux-iio
Cc: Tony Lindgren, Rob Herring, Mark Rutland, Russell King,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Lee Jones, Andrew F . Davis, linux-omap,
devicetree, linux-arm-kernel, linux-kernel, Sekhar Nori
On 09/22/16 13:45, Mugunthan V N wrote:
>>> + if (!dma->chan)
>>> + return -ENODEV;
>>> +
>>> + /* RX buffer */
>>> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>>> + &dma->addr, GFP_KERNEL);
>>> + if (!dma->buf)
>>> + goto err;
>>> +
>>> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
>>
>> Do we need _ratelimited? AFAICS, this print is called only once.
>
> True, will change to dev_dbg.
It would be better to remove it. It gives no useful debuggin information apart
from the fact that the driver did not failed to probe.
--
Péter
^ permalink raw reply [flat|nested] 60+ messages in thread
* [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
@ 2016-09-22 11:16 ` Peter Ujfalusi
0 siblings, 0 replies; 60+ messages in thread
From: Peter Ujfalusi @ 2016-09-22 11:16 UTC (permalink / raw)
To: linux-arm-kernel
On 09/22/16 13:45, Mugunthan V N wrote:
>>> + if (!dma->chan)
>>> + return -ENODEV;
>>> +
>>> + /* RX buffer */
>>> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>>> + &dma->addr, GFP_KERNEL);
>>> + if (!dma->buf)
>>> + goto err;
>>> +
>>> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
>>
>> Do we need _ratelimited? AFAICS, this print is called only once.
>
> True, will change to dev_dbg.
It would be better to remove it. It gives no useful debuggin information apart
from the fact that the driver did not failed to probe.
--
P?ter
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
@ 2016-09-22 11:16 ` Peter Ujfalusi
0 siblings, 0 replies; 60+ messages in thread
From: Peter Ujfalusi @ 2016-09-22 11:16 UTC (permalink / raw)
To: Mugunthan V N, Vignesh R, linux-iio
Cc: Tony Lindgren, Rob Herring, Mark Rutland, Russell King,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Lee Jones, Andrew F . Davis, linux-omap,
devicetree, linux-arm-kernel, linux-kernel, Sekhar Nori
On 09/22/16 13:45, Mugunthan V N wrote:
>>> + if (!dma->chan)
>>> + return -ENODEV;
>>> +
>>> + /* RX buffer */
>>> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>>> + &dma->addr, GFP_KERNEL);
>>> + if (!dma->buf)
>>> + goto err;
>>> +
>>> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
>>
>> Do we need _ratelimited? AFAICS, this print is called only once.
>
> True, will change to dev_dbg.
It would be better to remove it. It gives no useful debuggin information apart
from the fact that the driver did not failed to probe.
--
Péter
^ permalink raw reply [flat|nested] 60+ messages in thread
* Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
[not found] ` <20160921161134.6951-3-mugunthanvnm-l0cyMroinI0@public.gmane.org>
2016-09-21 17:44 ` Peter Meerwald-Stadler
@ 2016-09-22 7:20 ` Peter Ujfalusi
0 siblings, 0 replies; 60+ messages in thread
From: Peter Ujfalusi @ 2016-09-22 7:20 UTC (permalink / raw)
To: Mugunthan V N, linux-iio
Cc: Tony Lindgren, Rob Herring, Mark Rutland, Russell King,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Lee Jones, Vignesh R, Andrew F . Davis,
linux-omap, devicetree, linux-arm-kernel, linux-kernel,
Sekhar Nori
On 09/21/16 19:11, Mugunthan V N wrote:
> This patch adds the required pieces to ti_am335x_adc driver for
> DMA support
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
> drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
> 2 files changed, 164 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index c3cfacca..89d0b07 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -30,10 +30,32 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/kfifo_buf.h>
>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +
> +#define DMA_BUFFER_SIZE SZ_2K
> +
> +struct tiadc_dma {
> + /* Filter function */
> + dma_filter_fn fn;
> + /* Parameter to the filter function */
> + void *param;
> + struct dma_slave_config conf;
> + struct dma_chan *chan;
> + dma_addr_t addr;
> + dma_cookie_t cookie;
> + u8 *buf;
> + bool valid_buf_seg;
> + int buf_offset;
> + u8 fifo_thresh;
> +};
> +
> struct tiadc_device {
> struct ti_tscadc_dev *mfd_tscadc;
> + struct tiadc_dma dma;
> struct mutex fifo1_lock; /* to protect fifo access */
> int channels;
> + int total_ch_enabled;
> u8 channel_line[8];
> u8 channel_step[8];
> int buffer_en_ch_steps;
> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
> u16 *data = adc_dev->data;
>
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> +
> for (k = 0; k < fifo1count; k = k + i) {
> for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
> read = tiadc_readl(adc_dev, REG_FIFO1);
> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
> return IRQ_HANDLED;
> }
>
> +static void tiadc_dma_rx_complete(void *param)
> +{
> + struct iio_dev *indio_dev = param;
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> + u8 *data;
> + int i;
> +
> + data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
bool valid_buf_seg ? The buffer segment is valid or not valid? Which buffer
segment is valid when valid_buf_seg is true?
I know what this is doing, but it is logically not correct.
Instead you could have:
int current_period; /* The period the DMA is working on */
When you start the DMA:
dma->current_period = 0; /* We start to fill period 0 */
In here:
u8 *data = dma->buf + (dma->current_period * dma_period_size);
> + dma->valid_buf_seg = !dma->valid_buf_seg;
/* Currently we have only two periods, so we can just */
dma->current_period = !dma->current_period;
/* or */
dma->current_period = 1 - dma->current_period; /* swap the buffer ID */
I think this would make it better to follow and if later you figure that you
want three or more periods instead of the two, it is going to be easier to add
that.
> +
> + for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
> + iio_push_to_buffers(indio_dev, data);
> + data += indio_dev->scan_bytes;
> + }
> +}
> +
> +static int tiadc_start_dma(struct iio_dev *indio_dev)
> +{
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> + struct dma_async_tx_descriptor *desc;
> +
> + dma->valid_buf_seg = false;
> + dma->fifo_thresh = FIFO1_THRESHOLD;
FIFO1_THRESHOLD is defined as 19 in the global header file, so
if you have 2 channels enabled the trigger comes when the FIFO have 10
samples/ch, in case of all 8 channels enabled you will round it down the
trigger comes when you have 2 samples/ch.
Would not be better to try to use high threshold for the FIFO to reduce the
number of DMA requests needed runtime?
It is OK as it is at the moment, but I think it would make sense to try to
maximize the FIFO usage.
> + /*
> + * Make the fifo thresh as the multiple of total number of
> + * channels enabled, so make sure that that cyclic DMA period
> + * length is also a multiple of total number of channels
> + * enabled. This ensures that no invalid data is reported
> + * to the stack via iio_push_to_buffers().
> + */
> + dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
> + dma->buf_offset = DMA_BUFFER_SIZE / 2;
> + /* Make sure that period length is multiple of fifo thresh level */
> + dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
> + sizeof(u16));
> +
> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
> + dmaengine_slave_config(dma->chan, &dma->conf);
> +
> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
> + dma->buf_offset * 2,
> + dma->buf_offset, DMA_DEV_TO_MEM,
I find the buf_offset name and it's use really confusing. It should be called
period_size, really.
> + DMA_PREP_INTERRUPT);
> + if (!desc)
> + return -EBUSY;
> +
> + desc->callback = tiadc_dma_rx_complete;
> + desc->callback_param = indio_dev;
> +
> + dma->cookie = dmaengine_submit(desc);
> +
> + dma_async_issue_pending(dma->chan);
> +
> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
> +
> + return 0;
> +}
> +
> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> unsigned int enb = 0;
> u8 bit;
> + u32 irq_enable;
>
> tiadc_step_config(indio_dev);
> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
> + adc_dev->total_ch_enabled++;
> + }
> adc_dev->buffer_en_ch_steps = enb;
>
> + if (dma->chan)
> + tiadc_start_dma(indio_dev);
> +
> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>
> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
> - | IRQENB_FIFO1OVRRUN);
> +
> + irq_enable = IRQENB_FIFO1OVRRUN;
> + if (!dma->chan)
> + irq_enable |= IRQENB_FIFO1THRES;
> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>
> return 0;
> }
> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> int fifo1count, i, read;
>
> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
> adc_dev->buffer_en_ch_steps = 0;
> + adc_dev->total_ch_enabled = 0;
> + if (dma->chan) {
> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
> + dmaengine_terminate_async(dma->chan);
> + }
>
> /* Flush FIFO of leftover data in the time it takes to disable adc */
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
> .driver_module = THIS_MODULE,
> };
>
> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
> +{
> + return false;
> +}
> +
> +static int tiadc_request_dma(struct platform_device *pdev,
> + struct tiadc_device *adc_dev)
> +{
> + struct tiadc_dma *dma = &adc_dev->dma;
> + dma_cap_mask_t mask;
> +
> + dma->fn = the_no_dma_filter_fn;
> +
> + /* Default slave configuration parameters */
> + dma->conf.direction = DMA_DEV_TO_MEM;
> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
> +
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_CYCLIC, mask);
> +
> + /* Get a channel for RX */
> + dma->chan = dma_request_slave_channel_compat(mask,
> + dma->fn, dma->param,
> + adc_dev->mfd_tscadc->dev,
> + "fifo1");
dma_request_chan()
> + if (!dma->chan)
> + return -ENODEV;
> +
> + /* RX buffer */
> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
> + &dma->addr, GFP_KERNEL);
> + if (!dma->buf)
> + goto err;
> +
> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
> +
> + return 0;
> +err:
> + dma_release_channel(dma->chan);
> +
> + return -ENOMEM;
> +}
> +
> static int tiadc_parse_dt(struct platform_device *pdev,
> struct tiadc_device *adc_dev)
> {
> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, indio_dev);
>
> + err = tiadc_request_dma(pdev, adc_dev);
> + if (err && err != -ENODEV)
> + goto err_dma;
> +
> return 0;
>
> +err_dma:
> + iio_device_unregister(indio_dev);
> err_buffer_unregister:
> tiadc_iio_buffered_hardware_remove(indio_dev);
> err_free_channels:
> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> u32 step_en;
>
> + if (dma->chan)
> + dma_release_channel(dma->chan);
> iio_device_unregister(indio_dev);
> tiadc_iio_buffered_hardware_remove(indio_dev);
> tiadc_channels_remove(indio_dev);
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index e45a208..fb9dc99 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -23,6 +23,8 @@
> #define REG_IRQENABLE 0x02C
> #define REG_IRQCLR 0x030
> #define REG_IRQWAKEUP 0x034
> +#define REG_DMAENABLE_SET 0x038
> +#define REG_DMAENABLE_CLEAR 0x038
> #define REG_CTRL 0x040
> #define REG_ADCFSM 0x044
> #define REG_CLKDIV 0x04C
> @@ -36,6 +38,7 @@
> #define REG_FIFO0THR 0xE8
> #define REG_FIFO1CNT 0xF0
> #define REG_FIFO1THR 0xF4
> +#define REG_DMA1REQ 0xF8
> #define REG_FIFO0 0x100
> #define REG_FIFO1 0x200
>
> @@ -126,6 +129,10 @@
> #define FIFOREAD_DATA_MASK (0xfff << 0)
> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>
> +/* DMA ENABLE/CLEAR Register */
> +#define DMA_FIFO0 BIT(0)
> +#define DMA_FIFO1 BIT(1)
> +
> /* Sequencer Status */
> #define SEQ_STATUS BIT(5)
> #define CHARGE_STEP 0x11
>
--
Péter
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
@ 2016-09-22 7:20 ` Peter Ujfalusi
0 siblings, 0 replies; 60+ messages in thread
From: Peter Ujfalusi @ 2016-09-22 7:20 UTC (permalink / raw)
To: linux-arm-kernel
On 09/21/16 19:11, Mugunthan V N wrote:
> This patch adds the required pieces to ti_am335x_adc driver for
> DMA support
>
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
> ---
> drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
> 2 files changed, 164 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index c3cfacca..89d0b07 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -30,10 +30,32 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/kfifo_buf.h>
>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +
> +#define DMA_BUFFER_SIZE SZ_2K
> +
> +struct tiadc_dma {
> + /* Filter function */
> + dma_filter_fn fn;
> + /* Parameter to the filter function */
> + void *param;
> + struct dma_slave_config conf;
> + struct dma_chan *chan;
> + dma_addr_t addr;
> + dma_cookie_t cookie;
> + u8 *buf;
> + bool valid_buf_seg;
> + int buf_offset;
> + u8 fifo_thresh;
> +};
> +
> struct tiadc_device {
> struct ti_tscadc_dev *mfd_tscadc;
> + struct tiadc_dma dma;
> struct mutex fifo1_lock; /* to protect fifo access */
> int channels;
> + int total_ch_enabled;
> u8 channel_line[8];
> u8 channel_step[8];
> int buffer_en_ch_steps;
> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
> u16 *data = adc_dev->data;
>
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> +
> for (k = 0; k < fifo1count; k = k + i) {
> for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
> read = tiadc_readl(adc_dev, REG_FIFO1);
> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
> return IRQ_HANDLED;
> }
>
> +static void tiadc_dma_rx_complete(void *param)
> +{
> + struct iio_dev *indio_dev = param;
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> + u8 *data;
> + int i;
> +
> + data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
bool valid_buf_seg ? The buffer segment is valid or not valid? Which buffer
segment is valid when valid_buf_seg is true?
I know what this is doing, but it is logically not correct.
Instead you could have:
int current_period; /* The period the DMA is working on */
When you start the DMA:
dma->current_period = 0; /* We start to fill period 0 */
In here:
u8 *data = dma->buf + (dma->current_period * dma_period_size);
> + dma->valid_buf_seg = !dma->valid_buf_seg;
/* Currently we have only two periods, so we can just */
dma->current_period = !dma->current_period;
/* or */
dma->current_period = 1 - dma->current_period; /* swap the buffer ID */
I think this would make it better to follow and if later you figure that you
want three or more periods instead of the two, it is going to be easier to add
that.
> +
> + for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
> + iio_push_to_buffers(indio_dev, data);
> + data += indio_dev->scan_bytes;
> + }
> +}
> +
> +static int tiadc_start_dma(struct iio_dev *indio_dev)
> +{
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> + struct dma_async_tx_descriptor *desc;
> +
> + dma->valid_buf_seg = false;
> + dma->fifo_thresh = FIFO1_THRESHOLD;
FIFO1_THRESHOLD is defined as 19 in the global header file, so
if you have 2 channels enabled the trigger comes when the FIFO have 10
samples/ch, in case of all 8 channels enabled you will round it down the
trigger comes when you have 2 samples/ch.
Would not be better to try to use high threshold for the FIFO to reduce the
number of DMA requests needed runtime?
It is OK as it is at the moment, but I think it would make sense to try to
maximize the FIFO usage.
> + /*
> + * Make the fifo thresh as the multiple of total number of
> + * channels enabled, so make sure that that cyclic DMA period
> + * length is also a multiple of total number of channels
> + * enabled. This ensures that no invalid data is reported
> + * to the stack via iio_push_to_buffers().
> + */
> + dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
> + dma->buf_offset = DMA_BUFFER_SIZE / 2;
> + /* Make sure that period length is multiple of fifo thresh level */
> + dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
> + sizeof(u16));
> +
> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
> + dmaengine_slave_config(dma->chan, &dma->conf);
> +
> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
> + dma->buf_offset * 2,
> + dma->buf_offset, DMA_DEV_TO_MEM,
I find the buf_offset name and it's use really confusing. It should be called
period_size, really.
> + DMA_PREP_INTERRUPT);
> + if (!desc)
> + return -EBUSY;
> +
> + desc->callback = tiadc_dma_rx_complete;
> + desc->callback_param = indio_dev;
> +
> + dma->cookie = dmaengine_submit(desc);
> +
> + dma_async_issue_pending(dma->chan);
> +
> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
> +
> + return 0;
> +}
> +
> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> unsigned int enb = 0;
> u8 bit;
> + u32 irq_enable;
>
> tiadc_step_config(indio_dev);
> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
> + adc_dev->total_ch_enabled++;
> + }
> adc_dev->buffer_en_ch_steps = enb;
>
> + if (dma->chan)
> + tiadc_start_dma(indio_dev);
> +
> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>
> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
> - | IRQENB_FIFO1OVRRUN);
> +
> + irq_enable = IRQENB_FIFO1OVRRUN;
> + if (!dma->chan)
> + irq_enable |= IRQENB_FIFO1THRES;
> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>
> return 0;
> }
> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> int fifo1count, i, read;
>
> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
> adc_dev->buffer_en_ch_steps = 0;
> + adc_dev->total_ch_enabled = 0;
> + if (dma->chan) {
> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
> + dmaengine_terminate_async(dma->chan);
> + }
>
> /* Flush FIFO of leftover data in the time it takes to disable adc */
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
> .driver_module = THIS_MODULE,
> };
>
> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
> +{
> + return false;
> +}
> +
> +static int tiadc_request_dma(struct platform_device *pdev,
> + struct tiadc_device *adc_dev)
> +{
> + struct tiadc_dma *dma = &adc_dev->dma;
> + dma_cap_mask_t mask;
> +
> + dma->fn = the_no_dma_filter_fn;
> +
> + /* Default slave configuration parameters */
> + dma->conf.direction = DMA_DEV_TO_MEM;
> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
> +
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_CYCLIC, mask);
> +
> + /* Get a channel for RX */
> + dma->chan = dma_request_slave_channel_compat(mask,
> + dma->fn, dma->param,
> + adc_dev->mfd_tscadc->dev,
> + "fifo1");
dma_request_chan()
> + if (!dma->chan)
> + return -ENODEV;
> +
> + /* RX buffer */
> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
> + &dma->addr, GFP_KERNEL);
> + if (!dma->buf)
> + goto err;
> +
> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
> +
> + return 0;
> +err:
> + dma_release_channel(dma->chan);
> +
> + return -ENOMEM;
> +}
> +
> static int tiadc_parse_dt(struct platform_device *pdev,
> struct tiadc_device *adc_dev)
> {
> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, indio_dev);
>
> + err = tiadc_request_dma(pdev, adc_dev);
> + if (err && err != -ENODEV)
> + goto err_dma;
> +
> return 0;
>
> +err_dma:
> + iio_device_unregister(indio_dev);
> err_buffer_unregister:
> tiadc_iio_buffered_hardware_remove(indio_dev);
> err_free_channels:
> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> u32 step_en;
>
> + if (dma->chan)
> + dma_release_channel(dma->chan);
> iio_device_unregister(indio_dev);
> tiadc_iio_buffered_hardware_remove(indio_dev);
> tiadc_channels_remove(indio_dev);
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index e45a208..fb9dc99 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -23,6 +23,8 @@
> #define REG_IRQENABLE 0x02C
> #define REG_IRQCLR 0x030
> #define REG_IRQWAKEUP 0x034
> +#define REG_DMAENABLE_SET 0x038
> +#define REG_DMAENABLE_CLEAR 0x038
> #define REG_CTRL 0x040
> #define REG_ADCFSM 0x044
> #define REG_CLKDIV 0x04C
> @@ -36,6 +38,7 @@
> #define REG_FIFO0THR 0xE8
> #define REG_FIFO1CNT 0xF0
> #define REG_FIFO1THR 0xF4
> +#define REG_DMA1REQ 0xF8
> #define REG_FIFO0 0x100
> #define REG_FIFO1 0x200
>
> @@ -126,6 +129,10 @@
> #define FIFOREAD_DATA_MASK (0xfff << 0)
> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>
> +/* DMA ENABLE/CLEAR Register */
> +#define DMA_FIFO0 BIT(0)
> +#define DMA_FIFO1 BIT(1)
> +
> /* Sequencer Status */
> #define SEQ_STATUS BIT(5)
> #define CHARGE_STEP 0x11
>
--
P?ter
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
@ 2016-09-22 7:20 ` Peter Ujfalusi
0 siblings, 0 replies; 60+ messages in thread
From: Peter Ujfalusi @ 2016-09-22 7:20 UTC (permalink / raw)
To: Mugunthan V N, linux-iio-u79uwXL29TY76Z2rM5mHXA
Cc: Tony Lindgren, Rob Herring, Mark Rutland, Russell King,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Lee Jones, Vignesh R, Andrew F . Davis,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori
On 09/21/16 19:11, Mugunthan V N wrote:
> This patch adds the required pieces to ti_am335x_adc driver for
> DMA support
>
> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
> ---
> drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
> 2 files changed, 164 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index c3cfacca..89d0b07 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -30,10 +30,32 @@
> #include <linux/iio/buffer.h>
> #include <linux/iio/kfifo_buf.h>
>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +
> +#define DMA_BUFFER_SIZE SZ_2K
> +
> +struct tiadc_dma {
> + /* Filter function */
> + dma_filter_fn fn;
> + /* Parameter to the filter function */
> + void *param;
> + struct dma_slave_config conf;
> + struct dma_chan *chan;
> + dma_addr_t addr;
> + dma_cookie_t cookie;
> + u8 *buf;
> + bool valid_buf_seg;
> + int buf_offset;
> + u8 fifo_thresh;
> +};
> +
> struct tiadc_device {
> struct ti_tscadc_dev *mfd_tscadc;
> + struct tiadc_dma dma;
> struct mutex fifo1_lock; /* to protect fifo access */
> int channels;
> + int total_ch_enabled;
> u8 channel_line[8];
> u8 channel_step[8];
> int buffer_en_ch_steps;
> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
> u16 *data = adc_dev->data;
>
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> +
> for (k = 0; k < fifo1count; k = k + i) {
> for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
> read = tiadc_readl(adc_dev, REG_FIFO1);
> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
> return IRQ_HANDLED;
> }
>
> +static void tiadc_dma_rx_complete(void *param)
> +{
> + struct iio_dev *indio_dev = param;
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> + u8 *data;
> + int i;
> +
> + data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
bool valid_buf_seg ? The buffer segment is valid or not valid? Which buffer
segment is valid when valid_buf_seg is true?
I know what this is doing, but it is logically not correct.
Instead you could have:
int current_period; /* The period the DMA is working on */
When you start the DMA:
dma->current_period = 0; /* We start to fill period 0 */
In here:
u8 *data = dma->buf + (dma->current_period * dma_period_size);
> + dma->valid_buf_seg = !dma->valid_buf_seg;
/* Currently we have only two periods, so we can just */
dma->current_period = !dma->current_period;
/* or */
dma->current_period = 1 - dma->current_period; /* swap the buffer ID */
I think this would make it better to follow and if later you figure that you
want three or more periods instead of the two, it is going to be easier to add
that.
> +
> + for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
> + iio_push_to_buffers(indio_dev, data);
> + data += indio_dev->scan_bytes;
> + }
> +}
> +
> +static int tiadc_start_dma(struct iio_dev *indio_dev)
> +{
> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> + struct dma_async_tx_descriptor *desc;
> +
> + dma->valid_buf_seg = false;
> + dma->fifo_thresh = FIFO1_THRESHOLD;
FIFO1_THRESHOLD is defined as 19 in the global header file, so
if you have 2 channels enabled the trigger comes when the FIFO have 10
samples/ch, in case of all 8 channels enabled you will round it down the
trigger comes when you have 2 samples/ch.
Would not be better to try to use high threshold for the FIFO to reduce the
number of DMA requests needed runtime?
It is OK as it is at the moment, but I think it would make sense to try to
maximize the FIFO usage.
> + /*
> + * Make the fifo thresh as the multiple of total number of
> + * channels enabled, so make sure that that cyclic DMA period
> + * length is also a multiple of total number of channels
> + * enabled. This ensures that no invalid data is reported
> + * to the stack via iio_push_to_buffers().
> + */
> + dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
> + dma->buf_offset = DMA_BUFFER_SIZE / 2;
> + /* Make sure that period length is multiple of fifo thresh level */
> + dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
> + sizeof(u16));
> +
> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
> + dmaengine_slave_config(dma->chan, &dma->conf);
> +
> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
> + dma->buf_offset * 2,
> + dma->buf_offset, DMA_DEV_TO_MEM,
I find the buf_offset name and it's use really confusing. It should be called
period_size, really.
> + DMA_PREP_INTERRUPT);
> + if (!desc)
> + return -EBUSY;
> +
> + desc->callback = tiadc_dma_rx_complete;
> + desc->callback_param = indio_dev;
> +
> + dma->cookie = dmaengine_submit(desc);
> +
> + dma_async_issue_pending(dma->chan);
> +
> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
> +
> + return 0;
> +}
> +
> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> unsigned int enb = 0;
> u8 bit;
> + u32 irq_enable;
>
> tiadc_step_config(indio_dev);
> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
> + adc_dev->total_ch_enabled++;
> + }
> adc_dev->buffer_en_ch_steps = enb;
>
> + if (dma->chan)
> + tiadc_start_dma(indio_dev);
> +
> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>
> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
> - | IRQENB_FIFO1OVRRUN);
> +
> + irq_enable = IRQENB_FIFO1OVRRUN;
> + if (!dma->chan)
> + irq_enable |= IRQENB_FIFO1THRES;
> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>
> return 0;
> }
> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
> {
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> int fifo1count, i, read;
>
> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
> adc_dev->buffer_en_ch_steps = 0;
> + adc_dev->total_ch_enabled = 0;
> + if (dma->chan) {
> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
> + dmaengine_terminate_async(dma->chan);
> + }
>
> /* Flush FIFO of leftover data in the time it takes to disable adc */
> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
> .driver_module = THIS_MODULE,
> };
>
> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
> +{
> + return false;
> +}
> +
> +static int tiadc_request_dma(struct platform_device *pdev,
> + struct tiadc_device *adc_dev)
> +{
> + struct tiadc_dma *dma = &adc_dev->dma;
> + dma_cap_mask_t mask;
> +
> + dma->fn = the_no_dma_filter_fn;
> +
> + /* Default slave configuration parameters */
> + dma->conf.direction = DMA_DEV_TO_MEM;
> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
> +
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_CYCLIC, mask);
> +
> + /* Get a channel for RX */
> + dma->chan = dma_request_slave_channel_compat(mask,
> + dma->fn, dma->param,
> + adc_dev->mfd_tscadc->dev,
> + "fifo1");
dma_request_chan()
> + if (!dma->chan)
> + return -ENODEV;
> +
> + /* RX buffer */
> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
> + &dma->addr, GFP_KERNEL);
> + if (!dma->buf)
> + goto err;
> +
> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
> +
> + return 0;
> +err:
> + dma_release_channel(dma->chan);
> +
> + return -ENOMEM;
> +}
> +
> static int tiadc_parse_dt(struct platform_device *pdev,
> struct tiadc_device *adc_dev)
> {
> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, indio_dev);
>
> + err = tiadc_request_dma(pdev, adc_dev);
> + if (err && err != -ENODEV)
> + goto err_dma;
> +
> return 0;
>
> +err_dma:
> + iio_device_unregister(indio_dev);
> err_buffer_unregister:
> tiadc_iio_buffered_hardware_remove(indio_dev);
> err_free_channels:
> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct tiadc_device *adc_dev = iio_priv(indio_dev);
> + struct tiadc_dma *dma = &adc_dev->dma;
> u32 step_en;
>
> + if (dma->chan)
> + dma_release_channel(dma->chan);
> iio_device_unregister(indio_dev);
> tiadc_iio_buffered_hardware_remove(indio_dev);
> tiadc_channels_remove(indio_dev);
> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
> index e45a208..fb9dc99 100644
> --- a/include/linux/mfd/ti_am335x_tscadc.h
> +++ b/include/linux/mfd/ti_am335x_tscadc.h
> @@ -23,6 +23,8 @@
> #define REG_IRQENABLE 0x02C
> #define REG_IRQCLR 0x030
> #define REG_IRQWAKEUP 0x034
> +#define REG_DMAENABLE_SET 0x038
> +#define REG_DMAENABLE_CLEAR 0x038
> #define REG_CTRL 0x040
> #define REG_ADCFSM 0x044
> #define REG_CLKDIV 0x04C
> @@ -36,6 +38,7 @@
> #define REG_FIFO0THR 0xE8
> #define REG_FIFO1CNT 0xF0
> #define REG_FIFO1THR 0xF4
> +#define REG_DMA1REQ 0xF8
> #define REG_FIFO0 0x100
> #define REG_FIFO1 0x200
>
> @@ -126,6 +129,10 @@
> #define FIFOREAD_DATA_MASK (0xfff << 0)
> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>
> +/* DMA ENABLE/CLEAR Register */
> +#define DMA_FIFO0 BIT(0)
> +#define DMA_FIFO1 BIT(1)
> +
> /* Sequencer Status */
> #define SEQ_STATUS BIT(5)
> #define CHARGE_STEP 0x11
>
--
Péter
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
@ 2016-09-22 10:56 ` Mugunthan V N
0 siblings, 0 replies; 60+ messages in thread
From: Mugunthan V N @ 2016-09-22 10:56 UTC (permalink / raw)
To: Peter Ujfalusi, linux-iio
Cc: Tony Lindgren, Rob Herring, Mark Rutland, Russell King,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Lee Jones, Vignesh R, Andrew F . Davis,
linux-omap, devicetree, linux-arm-kernel, linux-kernel,
Sekhar Nori
On Thursday 22 September 2016 12:50 PM, Peter Ujfalusi wrote:
> On 09/21/16 19:11, Mugunthan V N wrote:
>> This patch adds the required pieces to ti_am335x_adc driver for
>> DMA support
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> ---
>> drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
>> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
>> 2 files changed, 164 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
>> index c3cfacca..89d0b07 100644
>> --- a/drivers/iio/adc/ti_am335x_adc.c
>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>> @@ -30,10 +30,32 @@
>> #include <linux/iio/buffer.h>
>> #include <linux/iio/kfifo_buf.h>
>>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#define DMA_BUFFER_SIZE SZ_2K
>> +
>> +struct tiadc_dma {
>> + /* Filter function */
>> + dma_filter_fn fn;
>> + /* Parameter to the filter function */
>> + void *param;
>> + struct dma_slave_config conf;
>> + struct dma_chan *chan;
>> + dma_addr_t addr;
>> + dma_cookie_t cookie;
>> + u8 *buf;
>> + bool valid_buf_seg;
>> + int buf_offset;
>> + u8 fifo_thresh;
>> +};
>> +
>> struct tiadc_device {
>> struct ti_tscadc_dev *mfd_tscadc;
>> + struct tiadc_dma dma;
>> struct mutex fifo1_lock; /* to protect fifo access */
>> int channels;
>> + int total_ch_enabled;
>> u8 channel_line[8];
>> u8 channel_step[8];
>> int buffer_en_ch_steps;
>> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> u16 *data = adc_dev->data;
>>
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> +
>> for (k = 0; k < fifo1count; k = k + i) {
>> for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
>> read = tiadc_readl(adc_dev, REG_FIFO1);
>> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> return IRQ_HANDLED;
>> }
>>
>> +static void tiadc_dma_rx_complete(void *param)
>> +{
>> + struct iio_dev *indio_dev = param;
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + u8 *data;
>> + int i;
>> +
>> + data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
>
> bool valid_buf_seg ? The buffer segment is valid or not valid? Which buffer
> segment is valid when valid_buf_seg is true?
>
> I know what this is doing, but it is logically not correct.
>
> Instead you could have:
>
> int current_period; /* The period the DMA is working on */
>
> When you start the DMA:
> dma->current_period = 0; /* We start to fill period 0 */
>
> In here:
>
> u8 *data = dma->buf + (dma->current_period * dma_period_size);
>
>> + dma->valid_buf_seg = !dma->valid_buf_seg;
>
> /* Currently we have only two periods, so we can just */
> dma->current_period = !dma->current_period;
> /* or */
> dma->current_period = 1 - dma->current_period; /* swap the buffer ID */
>
> I think this would make it better to follow and if later you figure that you
> want three or more periods instead of the two, it is going to be easier to add
> that.
Okay, will change this in next version patch.
>
>> +
>> + for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
>> + iio_push_to_buffers(indio_dev, data);
>> + data += indio_dev->scan_bytes;
>> + }
>> +}
>> +
>> +static int tiadc_start_dma(struct iio_dev *indio_dev)
>> +{
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + struct dma_async_tx_descriptor *desc;
>> +
>> + dma->valid_buf_seg = false;
>> + dma->fifo_thresh = FIFO1_THRESHOLD;
>
> FIFO1_THRESHOLD is defined as 19 in the global header file, so
> if you have 2 channels enabled the trigger comes when the FIFO have 10
> samples/ch, in case of all 8 channels enabled you will round it down the
> trigger comes when you have 2 samples/ch.
>
> Would not be better to try to use high threshold for the FIFO to reduce the
> number of DMA requests needed runtime?
>
> It is OK as it is at the moment, but I think it would make sense to try to
> maximize the FIFO usage.
I had already tried with higher values, It works fine. Just don't want
to change the non-DMA implementation as I am not aware of any
limitations why at the first place they have used fifo threshold as 19.
>
>> + /*
>> + * Make the fifo thresh as the multiple of total number of
>> + * channels enabled, so make sure that that cyclic DMA period
>> + * length is also a multiple of total number of channels
>> + * enabled. This ensures that no invalid data is reported
>> + * to the stack via iio_push_to_buffers().
>> + */
>> + dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
>> + dma->buf_offset = DMA_BUFFER_SIZE / 2;
>> + /* Make sure that period length is multiple of fifo thresh level */
>> + dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
>> + sizeof(u16));
>> +
>> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
>> + dmaengine_slave_config(dma->chan, &dma->conf);
>> +
>> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
>> + dma->buf_offset * 2,
>> + dma->buf_offset, DMA_DEV_TO_MEM,
>
> I find the buf_offset name and it's use really confusing. It should be called
> period_size, really.
Yes, will fix this.
>
>> + DMA_PREP_INTERRUPT);
>> + if (!desc)
>> + return -EBUSY;
>> +
>> + desc->callback = tiadc_dma_rx_complete;
>> + desc->callback_param = indio_dev;
>> +
>> + dma->cookie = dmaengine_submit(desc);
>> +
>> + dma_async_issue_pending(dma->chan);
>> +
>> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
>> +
>> + return 0;
>> +}
>> +
>> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> unsigned int enb = 0;
>> u8 bit;
>> + u32 irq_enable;
>>
>> tiadc_step_config(indio_dev);
>> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
>> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
>> + adc_dev->total_ch_enabled++;
>> + }
>> adc_dev->buffer_en_ch_steps = enb;
>>
>> + if (dma->chan)
>> + tiadc_start_dma(indio_dev);
>> +
>> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>>
>> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
>> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
>> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
>> - | IRQENB_FIFO1OVRRUN);
>> +
>> + irq_enable = IRQENB_FIFO1OVRRUN;
>> + if (!dma->chan)
>> + irq_enable |= IRQENB_FIFO1THRES;
>> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>>
>> return 0;
>> }
>> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> int fifo1count, i, read;
>>
>> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>> adc_dev->buffer_en_ch_steps = 0;
>> + adc_dev->total_ch_enabled = 0;
>> + if (dma->chan) {
>> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
>> + dmaengine_terminate_async(dma->chan);
>> + }
>>
>> /* Flush FIFO of leftover data in the time it takes to disable adc */
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
>> .driver_module = THIS_MODULE,
>> };
>>
>> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
>> +{
>> + return false;
>> +}
>> +
>> +static int tiadc_request_dma(struct platform_device *pdev,
>> + struct tiadc_device *adc_dev)
>> +{
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + dma_cap_mask_t mask;
>> +
>> + dma->fn = the_no_dma_filter_fn;
>> +
>> + /* Default slave configuration parameters */
>> + dma->conf.direction = DMA_DEV_TO_MEM;
>> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
>> +
>> + dma_cap_zero(mask);
>> + dma_cap_set(DMA_CYCLIC, mask);
>> +
>> + /* Get a channel for RX */
>> + dma->chan = dma_request_slave_channel_compat(mask,
>> + dma->fn, dma->param,
>> + adc_dev->mfd_tscadc->dev,
>> + "fifo1");
>
> dma_request_chan()
Will fix this.
>
>> + if (!dma->chan)
>> + return -ENODEV;
>> +
>> + /* RX buffer */
>> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>> + &dma->addr, GFP_KERNEL);
>> + if (!dma->buf)
>> + goto err;
>> +
>> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
>> +
>> + return 0;
>> +err:
>> + dma_release_channel(dma->chan);
>> +
>> + return -ENOMEM;
>> +}
>> +
>> static int tiadc_parse_dt(struct platform_device *pdev,
>> struct tiadc_device *adc_dev)
>> {
>> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>>
>> platform_set_drvdata(pdev, indio_dev);
>>
>> + err = tiadc_request_dma(pdev, adc_dev);
>> + if (err && err != -ENODEV)
>> + goto err_dma;
>> +
>> return 0;
>>
>> +err_dma:
>> + iio_device_unregister(indio_dev);
>> err_buffer_unregister:
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> err_free_channels:
>> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
>> {
>> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> u32 step_en;
>>
>> + if (dma->chan)
>> + dma_release_channel(dma->chan);
>> iio_device_unregister(indio_dev);
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> tiadc_channels_remove(indio_dev);
>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> index e45a208..fb9dc99 100644
>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> @@ -23,6 +23,8 @@
>> #define REG_IRQENABLE 0x02C
>> #define REG_IRQCLR 0x030
>> #define REG_IRQWAKEUP 0x034
>> +#define REG_DMAENABLE_SET 0x038
>> +#define REG_DMAENABLE_CLEAR 0x038
>> #define REG_CTRL 0x040
>> #define REG_ADCFSM 0x044
>> #define REG_CLKDIV 0x04C
>> @@ -36,6 +38,7 @@
>> #define REG_FIFO0THR 0xE8
>> #define REG_FIFO1CNT 0xF0
>> #define REG_FIFO1THR 0xF4
>> +#define REG_DMA1REQ 0xF8
>> #define REG_FIFO0 0x100
>> #define REG_FIFO1 0x200
>>
>> @@ -126,6 +129,10 @@
>> #define FIFOREAD_DATA_MASK (0xfff << 0)
>> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>>
>> +/* DMA ENABLE/CLEAR Register */
>> +#define DMA_FIFO0 BIT(0)
>> +#define DMA_FIFO1 BIT(1)
>> +
>> /* Sequencer Status */
>> #define SEQ_STATUS BIT(5)
>> #define CHARGE_STEP 0x11
>>
>
>
--
Regards
Mugunthan V N
^ permalink raw reply [flat|nested] 60+ messages in thread* [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
@ 2016-09-22 10:56 ` Mugunthan V N
0 siblings, 0 replies; 60+ messages in thread
From: Mugunthan V N @ 2016-09-22 10:56 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 22 September 2016 12:50 PM, Peter Ujfalusi wrote:
> On 09/21/16 19:11, Mugunthan V N wrote:
>> This patch adds the required pieces to ti_am335x_adc driver for
>> DMA support
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> ---
>> drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
>> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
>> 2 files changed, 164 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
>> index c3cfacca..89d0b07 100644
>> --- a/drivers/iio/adc/ti_am335x_adc.c
>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>> @@ -30,10 +30,32 @@
>> #include <linux/iio/buffer.h>
>> #include <linux/iio/kfifo_buf.h>
>>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#define DMA_BUFFER_SIZE SZ_2K
>> +
>> +struct tiadc_dma {
>> + /* Filter function */
>> + dma_filter_fn fn;
>> + /* Parameter to the filter function */
>> + void *param;
>> + struct dma_slave_config conf;
>> + struct dma_chan *chan;
>> + dma_addr_t addr;
>> + dma_cookie_t cookie;
>> + u8 *buf;
>> + bool valid_buf_seg;
>> + int buf_offset;
>> + u8 fifo_thresh;
>> +};
>> +
>> struct tiadc_device {
>> struct ti_tscadc_dev *mfd_tscadc;
>> + struct tiadc_dma dma;
>> struct mutex fifo1_lock; /* to protect fifo access */
>> int channels;
>> + int total_ch_enabled;
>> u8 channel_line[8];
>> u8 channel_step[8];
>> int buffer_en_ch_steps;
>> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> u16 *data = adc_dev->data;
>>
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> +
>> for (k = 0; k < fifo1count; k = k + i) {
>> for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
>> read = tiadc_readl(adc_dev, REG_FIFO1);
>> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> return IRQ_HANDLED;
>> }
>>
>> +static void tiadc_dma_rx_complete(void *param)
>> +{
>> + struct iio_dev *indio_dev = param;
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + u8 *data;
>> + int i;
>> +
>> + data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
>
> bool valid_buf_seg ? The buffer segment is valid or not valid? Which buffer
> segment is valid when valid_buf_seg is true?
>
> I know what this is doing, but it is logically not correct.
>
> Instead you could have:
>
> int current_period; /* The period the DMA is working on */
>
> When you start the DMA:
> dma->current_period = 0; /* We start to fill period 0 */
>
> In here:
>
> u8 *data = dma->buf + (dma->current_period * dma_period_size);
>
>> + dma->valid_buf_seg = !dma->valid_buf_seg;
>
> /* Currently we have only two periods, so we can just */
> dma->current_period = !dma->current_period;
> /* or */
> dma->current_period = 1 - dma->current_period; /* swap the buffer ID */
>
> I think this would make it better to follow and if later you figure that you
> want three or more periods instead of the two, it is going to be easier to add
> that.
Okay, will change this in next version patch.
>
>> +
>> + for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
>> + iio_push_to_buffers(indio_dev, data);
>> + data += indio_dev->scan_bytes;
>> + }
>> +}
>> +
>> +static int tiadc_start_dma(struct iio_dev *indio_dev)
>> +{
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + struct dma_async_tx_descriptor *desc;
>> +
>> + dma->valid_buf_seg = false;
>> + dma->fifo_thresh = FIFO1_THRESHOLD;
>
> FIFO1_THRESHOLD is defined as 19 in the global header file, so
> if you have 2 channels enabled the trigger comes when the FIFO have 10
> samples/ch, in case of all 8 channels enabled you will round it down the
> trigger comes when you have 2 samples/ch.
>
> Would not be better to try to use high threshold for the FIFO to reduce the
> number of DMA requests needed runtime?
>
> It is OK as it is at the moment, but I think it would make sense to try to
> maximize the FIFO usage.
I had already tried with higher values, It works fine. Just don't want
to change the non-DMA implementation as I am not aware of any
limitations why at the first place they have used fifo threshold as 19.
>
>> + /*
>> + * Make the fifo thresh as the multiple of total number of
>> + * channels enabled, so make sure that that cyclic DMA period
>> + * length is also a multiple of total number of channels
>> + * enabled. This ensures that no invalid data is reported
>> + * to the stack via iio_push_to_buffers().
>> + */
>> + dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
>> + dma->buf_offset = DMA_BUFFER_SIZE / 2;
>> + /* Make sure that period length is multiple of fifo thresh level */
>> + dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
>> + sizeof(u16));
>> +
>> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
>> + dmaengine_slave_config(dma->chan, &dma->conf);
>> +
>> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
>> + dma->buf_offset * 2,
>> + dma->buf_offset, DMA_DEV_TO_MEM,
>
> I find the buf_offset name and it's use really confusing. It should be called
> period_size, really.
Yes, will fix this.
>
>> + DMA_PREP_INTERRUPT);
>> + if (!desc)
>> + return -EBUSY;
>> +
>> + desc->callback = tiadc_dma_rx_complete;
>> + desc->callback_param = indio_dev;
>> +
>> + dma->cookie = dmaengine_submit(desc);
>> +
>> + dma_async_issue_pending(dma->chan);
>> +
>> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
>> +
>> + return 0;
>> +}
>> +
>> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> unsigned int enb = 0;
>> u8 bit;
>> + u32 irq_enable;
>>
>> tiadc_step_config(indio_dev);
>> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
>> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
>> + adc_dev->total_ch_enabled++;
>> + }
>> adc_dev->buffer_en_ch_steps = enb;
>>
>> + if (dma->chan)
>> + tiadc_start_dma(indio_dev);
>> +
>> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>>
>> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
>> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
>> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
>> - | IRQENB_FIFO1OVRRUN);
>> +
>> + irq_enable = IRQENB_FIFO1OVRRUN;
>> + if (!dma->chan)
>> + irq_enable |= IRQENB_FIFO1THRES;
>> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>>
>> return 0;
>> }
>> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> int fifo1count, i, read;
>>
>> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>> adc_dev->buffer_en_ch_steps = 0;
>> + adc_dev->total_ch_enabled = 0;
>> + if (dma->chan) {
>> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
>> + dmaengine_terminate_async(dma->chan);
>> + }
>>
>> /* Flush FIFO of leftover data in the time it takes to disable adc */
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
>> .driver_module = THIS_MODULE,
>> };
>>
>> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
>> +{
>> + return false;
>> +}
>> +
>> +static int tiadc_request_dma(struct platform_device *pdev,
>> + struct tiadc_device *adc_dev)
>> +{
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + dma_cap_mask_t mask;
>> +
>> + dma->fn = the_no_dma_filter_fn;
>> +
>> + /* Default slave configuration parameters */
>> + dma->conf.direction = DMA_DEV_TO_MEM;
>> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
>> +
>> + dma_cap_zero(mask);
>> + dma_cap_set(DMA_CYCLIC, mask);
>> +
>> + /* Get a channel for RX */
>> + dma->chan = dma_request_slave_channel_compat(mask,
>> + dma->fn, dma->param,
>> + adc_dev->mfd_tscadc->dev,
>> + "fifo1");
>
> dma_request_chan()
Will fix this.
>
>> + if (!dma->chan)
>> + return -ENODEV;
>> +
>> + /* RX buffer */
>> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>> + &dma->addr, GFP_KERNEL);
>> + if (!dma->buf)
>> + goto err;
>> +
>> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
>> +
>> + return 0;
>> +err:
>> + dma_release_channel(dma->chan);
>> +
>> + return -ENOMEM;
>> +}
>> +
>> static int tiadc_parse_dt(struct platform_device *pdev,
>> struct tiadc_device *adc_dev)
>> {
>> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>>
>> platform_set_drvdata(pdev, indio_dev);
>>
>> + err = tiadc_request_dma(pdev, adc_dev);
>> + if (err && err != -ENODEV)
>> + goto err_dma;
>> +
>> return 0;
>>
>> +err_dma:
>> + iio_device_unregister(indio_dev);
>> err_buffer_unregister:
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> err_free_channels:
>> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
>> {
>> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> u32 step_en;
>>
>> + if (dma->chan)
>> + dma_release_channel(dma->chan);
>> iio_device_unregister(indio_dev);
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> tiadc_channels_remove(indio_dev);
>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> index e45a208..fb9dc99 100644
>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> @@ -23,6 +23,8 @@
>> #define REG_IRQENABLE 0x02C
>> #define REG_IRQCLR 0x030
>> #define REG_IRQWAKEUP 0x034
>> +#define REG_DMAENABLE_SET 0x038
>> +#define REG_DMAENABLE_CLEAR 0x038
>> #define REG_CTRL 0x040
>> #define REG_ADCFSM 0x044
>> #define REG_CLKDIV 0x04C
>> @@ -36,6 +38,7 @@
>> #define REG_FIFO0THR 0xE8
>> #define REG_FIFO1CNT 0xF0
>> #define REG_FIFO1THR 0xF4
>> +#define REG_DMA1REQ 0xF8
>> #define REG_FIFO0 0x100
>> #define REG_FIFO1 0x200
>>
>> @@ -126,6 +129,10 @@
>> #define FIFOREAD_DATA_MASK (0xfff << 0)
>> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>>
>> +/* DMA ENABLE/CLEAR Register */
>> +#define DMA_FIFO0 BIT(0)
>> +#define DMA_FIFO1 BIT(1)
>> +
>> /* Sequencer Status */
>> #define SEQ_STATUS BIT(5)
>> #define CHARGE_STEP 0x11
>>
>
>
--
Regards
Mugunthan V N
^ permalink raw reply [flat|nested] 60+ messages in thread* Re: [PATCH 2/4] drivers: iio: ti_am335x_adc: add dma support
@ 2016-09-22 10:56 ` Mugunthan V N
0 siblings, 0 replies; 60+ messages in thread
From: Mugunthan V N @ 2016-09-22 10:56 UTC (permalink / raw)
To: Peter Ujfalusi, linux-iio-u79uwXL29TY76Z2rM5mHXA
Cc: Tony Lindgren, Rob Herring, Mark Rutland, Russell King,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Lee Jones, Vignesh R, Andrew F . Davis,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Sekhar Nori
On Thursday 22 September 2016 12:50 PM, Peter Ujfalusi wrote:
> On 09/21/16 19:11, Mugunthan V N wrote:
>> This patch adds the required pieces to ti_am335x_adc driver for
>> DMA support
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm-l0cyMroinI0@public.gmane.org>
>> ---
>> drivers/iio/adc/ti_am335x_adc.c | 160 ++++++++++++++++++++++++++++++++++-
>> include/linux/mfd/ti_am335x_tscadc.h | 7 ++
>> 2 files changed, 164 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
>> index c3cfacca..89d0b07 100644
>> --- a/drivers/iio/adc/ti_am335x_adc.c
>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>> @@ -30,10 +30,32 @@
>> #include <linux/iio/buffer.h>
>> #include <linux/iio/kfifo_buf.h>
>>
>> +#include <linux/dmaengine.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#define DMA_BUFFER_SIZE SZ_2K
>> +
>> +struct tiadc_dma {
>> + /* Filter function */
>> + dma_filter_fn fn;
>> + /* Parameter to the filter function */
>> + void *param;
>> + struct dma_slave_config conf;
>> + struct dma_chan *chan;
>> + dma_addr_t addr;
>> + dma_cookie_t cookie;
>> + u8 *buf;
>> + bool valid_buf_seg;
>> + int buf_offset;
>> + u8 fifo_thresh;
>> +};
>> +
>> struct tiadc_device {
>> struct ti_tscadc_dev *mfd_tscadc;
>> + struct tiadc_dma dma;
>> struct mutex fifo1_lock; /* to protect fifo access */
>> int channels;
>> + int total_ch_enabled;
>> u8 channel_line[8];
>> u8 channel_step[8];
>> int buffer_en_ch_steps;
>> @@ -184,6 +206,7 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> u16 *data = adc_dev->data;
>>
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> +
>> for (k = 0; k < fifo1count; k = k + i) {
>> for (i = 0; i < (indio_dev->scan_bytes)/2; i++) {
>> read = tiadc_readl(adc_dev, REG_FIFO1);
>> @@ -198,6 +221,68 @@ static irqreturn_t tiadc_worker_h(int irq, void *private)
>> return IRQ_HANDLED;
>> }
>>
>> +static void tiadc_dma_rx_complete(void *param)
>> +{
>> + struct iio_dev *indio_dev = param;
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + u8 *data;
>> + int i;
>> +
>> + data = dma->valid_buf_seg ? dma->buf + dma->buf_offset : dma->buf;
>
> bool valid_buf_seg ? The buffer segment is valid or not valid? Which buffer
> segment is valid when valid_buf_seg is true?
>
> I know what this is doing, but it is logically not correct.
>
> Instead you could have:
>
> int current_period; /* The period the DMA is working on */
>
> When you start the DMA:
> dma->current_period = 0; /* We start to fill period 0 */
>
> In here:
>
> u8 *data = dma->buf + (dma->current_period * dma_period_size);
>
>> + dma->valid_buf_seg = !dma->valid_buf_seg;
>
> /* Currently we have only two periods, so we can just */
> dma->current_period = !dma->current_period;
> /* or */
> dma->current_period = 1 - dma->current_period; /* swap the buffer ID */
>
> I think this would make it better to follow and if later you figure that you
> want three or more periods instead of the two, it is going to be easier to add
> that.
Okay, will change this in next version patch.
>
>> +
>> + for (i = 0; i < dma->buf_offset; i += indio_dev->scan_bytes) {
>> + iio_push_to_buffers(indio_dev, data);
>> + data += indio_dev->scan_bytes;
>> + }
>> +}
>> +
>> +static int tiadc_start_dma(struct iio_dev *indio_dev)
>> +{
>> + struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + struct dma_async_tx_descriptor *desc;
>> +
>> + dma->valid_buf_seg = false;
>> + dma->fifo_thresh = FIFO1_THRESHOLD;
>
> FIFO1_THRESHOLD is defined as 19 in the global header file, so
> if you have 2 channels enabled the trigger comes when the FIFO have 10
> samples/ch, in case of all 8 channels enabled you will round it down the
> trigger comes when you have 2 samples/ch.
>
> Would not be better to try to use high threshold for the FIFO to reduce the
> number of DMA requests needed runtime?
>
> It is OK as it is at the moment, but I think it would make sense to try to
> maximize the FIFO usage.
I had already tried with higher values, It works fine. Just don't want
to change the non-DMA implementation as I am not aware of any
limitations why at the first place they have used fifo threshold as 19.
>
>> + /*
>> + * Make the fifo thresh as the multiple of total number of
>> + * channels enabled, so make sure that that cyclic DMA period
>> + * length is also a multiple of total number of channels
>> + * enabled. This ensures that no invalid data is reported
>> + * to the stack via iio_push_to_buffers().
>> + */
>> + dma->fifo_thresh -= (dma->fifo_thresh + 1) % adc_dev->total_ch_enabled;
>> + dma->buf_offset = DMA_BUFFER_SIZE / 2;
>> + /* Make sure that period length is multiple of fifo thresh level */
>> + dma->buf_offset -= dma->buf_offset % ((dma->fifo_thresh + 1) *
>> + sizeof(u16));
>> +
>> + dma->conf.src_maxburst = dma->fifo_thresh + 1;
>> + dmaengine_slave_config(dma->chan, &dma->conf);
>> +
>> + desc = dmaengine_prep_dma_cyclic(dma->chan, dma->addr,
>> + dma->buf_offset * 2,
>> + dma->buf_offset, DMA_DEV_TO_MEM,
>
> I find the buf_offset name and it's use really confusing. It should be called
> period_size, really.
Yes, will fix this.
>
>> + DMA_PREP_INTERRUPT);
>> + if (!desc)
>> + return -EBUSY;
>> +
>> + desc->callback = tiadc_dma_rx_complete;
>> + desc->callback_param = indio_dev;
>> +
>> + dma->cookie = dmaengine_submit(desc);
>> +
>> + dma_async_issue_pending(dma->chan);
>> +
>> + tiadc_writel(adc_dev, REG_FIFO1THR, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMA1REQ, dma->fifo_thresh);
>> + tiadc_writel(adc_dev, REG_DMAENABLE_SET, DMA_FIFO1);
>> +
>> + return 0;
>> +}
>> +
>> static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> @@ -218,20 +303,30 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> unsigned int enb = 0;
>> u8 bit;
>> + u32 irq_enable;
>>
>> tiadc_step_config(indio_dev);
>> - for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels)
>> + for_each_set_bit(bit, indio_dev->active_scan_mask, adc_dev->channels) {
>> enb |= (get_adc_step_bit(adc_dev, bit) << 1);
>> + adc_dev->total_ch_enabled++;
>> + }
>> adc_dev->buffer_en_ch_steps = enb;
>>
>> + if (dma->chan)
>> + tiadc_start_dma(indio_dev);
>> +
>> am335x_tsc_se_set_cache(adc_dev->mfd_tscadc, enb);
>>
>> tiadc_writel(adc_dev, REG_IRQSTATUS, IRQENB_FIFO1THRES
>> | IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW);
>> - tiadc_writel(adc_dev, REG_IRQENABLE, IRQENB_FIFO1THRES
>> - | IRQENB_FIFO1OVRRUN);
>> +
>> + irq_enable = IRQENB_FIFO1OVRRUN;
>> + if (!dma->chan)
>> + irq_enable |= IRQENB_FIFO1THRES;
>> + tiadc_writel(adc_dev, REG_IRQENABLE, irq_enable);
>>
>> return 0;
>> }
>> @@ -239,12 +334,18 @@ static int tiadc_buffer_postenable(struct iio_dev *indio_dev)
>> static int tiadc_buffer_predisable(struct iio_dev *indio_dev)
>> {
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> int fifo1count, i, read;
>>
>> tiadc_writel(adc_dev, REG_IRQCLR, (IRQENB_FIFO1THRES |
>> IRQENB_FIFO1OVRRUN | IRQENB_FIFO1UNDRFLW));
>> am335x_tsc_se_clr(adc_dev->mfd_tscadc, adc_dev->buffer_en_ch_steps);
>> adc_dev->buffer_en_ch_steps = 0;
>> + adc_dev->total_ch_enabled = 0;
>> + if (dma->chan) {
>> + tiadc_writel(adc_dev, REG_DMAENABLE_CLEAR, 0x2);
>> + dmaengine_terminate_async(dma->chan);
>> + }
>>
>> /* Flush FIFO of leftover data in the time it takes to disable adc */
>> fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
>> @@ -430,6 +531,50 @@ static const struct iio_info tiadc_info = {
>> .driver_module = THIS_MODULE,
>> };
>>
>> +static bool the_no_dma_filter_fn(struct dma_chan *chan, void *param)
>> +{
>> + return false;
>> +}
>> +
>> +static int tiadc_request_dma(struct platform_device *pdev,
>> + struct tiadc_device *adc_dev)
>> +{
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> + dma_cap_mask_t mask;
>> +
>> + dma->fn = the_no_dma_filter_fn;
>> +
>> + /* Default slave configuration parameters */
>> + dma->conf.direction = DMA_DEV_TO_MEM;
>> + dma->conf.src_addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> + dma->conf.src_addr = adc_dev->mfd_tscadc->tscadc_phys_base + REG_FIFO1;
>> +
>> + dma_cap_zero(mask);
>> + dma_cap_set(DMA_CYCLIC, mask);
>> +
>> + /* Get a channel for RX */
>> + dma->chan = dma_request_slave_channel_compat(mask,
>> + dma->fn, dma->param,
>> + adc_dev->mfd_tscadc->dev,
>> + "fifo1");
>
> dma_request_chan()
Will fix this.
>
>> + if (!dma->chan)
>> + return -ENODEV;
>> +
>> + /* RX buffer */
>> + dma->buf = dma_alloc_coherent(dma->chan->device->dev, DMA_BUFFER_SIZE,
>> + &dma->addr, GFP_KERNEL);
>> + if (!dma->buf)
>> + goto err;
>> +
>> + dev_dbg_ratelimited(adc_dev->mfd_tscadc->dev, "got dma channel\n");
>> +
>> + return 0;
>> +err:
>> + dma_release_channel(dma->chan);
>> +
>> + return -ENOMEM;
>> +}
>> +
>> static int tiadc_parse_dt(struct platform_device *pdev,
>> struct tiadc_device *adc_dev)
>> {
>> @@ -512,8 +657,14 @@ static int tiadc_probe(struct platform_device *pdev)
>>
>> platform_set_drvdata(pdev, indio_dev);
>>
>> + err = tiadc_request_dma(pdev, adc_dev);
>> + if (err && err != -ENODEV)
>> + goto err_dma;
>> +
>> return 0;
>>
>> +err_dma:
>> + iio_device_unregister(indio_dev);
>> err_buffer_unregister:
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> err_free_channels:
>> @@ -525,8 +676,11 @@ static int tiadc_remove(struct platform_device *pdev)
>> {
>> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>> struct tiadc_device *adc_dev = iio_priv(indio_dev);
>> + struct tiadc_dma *dma = &adc_dev->dma;
>> u32 step_en;
>>
>> + if (dma->chan)
>> + dma_release_channel(dma->chan);
>> iio_device_unregister(indio_dev);
>> tiadc_iio_buffered_hardware_remove(indio_dev);
>> tiadc_channels_remove(indio_dev);
>> diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h
>> index e45a208..fb9dc99 100644
>> --- a/include/linux/mfd/ti_am335x_tscadc.h
>> +++ b/include/linux/mfd/ti_am335x_tscadc.h
>> @@ -23,6 +23,8 @@
>> #define REG_IRQENABLE 0x02C
>> #define REG_IRQCLR 0x030
>> #define REG_IRQWAKEUP 0x034
>> +#define REG_DMAENABLE_SET 0x038
>> +#define REG_DMAENABLE_CLEAR 0x038
>> #define REG_CTRL 0x040
>> #define REG_ADCFSM 0x044
>> #define REG_CLKDIV 0x04C
>> @@ -36,6 +38,7 @@
>> #define REG_FIFO0THR 0xE8
>> #define REG_FIFO1CNT 0xF0
>> #define REG_FIFO1THR 0xF4
>> +#define REG_DMA1REQ 0xF8
>> #define REG_FIFO0 0x100
>> #define REG_FIFO1 0x200
>>
>> @@ -126,6 +129,10 @@
>> #define FIFOREAD_DATA_MASK (0xfff << 0)
>> #define FIFOREAD_CHNLID_MASK (0xf << 16)
>>
>> +/* DMA ENABLE/CLEAR Register */
>> +#define DMA_FIFO0 BIT(0)
>> +#define DMA_FIFO1 BIT(1)
>> +
>> /* Sequencer Status */
>> #define SEQ_STATUS BIT(5)
>> #define CHARGE_STEP 0x11
>>
>
>
--
Regards
Mugunthan V N
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 60+ messages in thread