From 685983f4decc5fa5700a0ab083859a2fe59acf10 Mon Sep 17 00:00:00 2001 From: Sricharan R Date: Fri, 10 Jun 2016 23:38:19 +0530 Subject: i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled With CONFIG_DEBUG_SG is enabled and when dma mode is used, below dump is seen, ------------[ cut here ]------------ kernel BUG at include/linux/scatterlist.h:140! Internal error: Oops - BUG: 0 [#1] PREEMPT SMP Modules linked in: CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.4.0-00459-g9f087b9-dirty #7 Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT) task: ffffffc036868000 ti: ffffffc036870000 task.ti: ffffffc036870000 PC is at qup_sg_set_buf.isra.13+0x138/0x154 LR is at qup_sg_set_buf.isra.13+0x50/0x154 pc : [] lr : [] pstate: 60000145 sp : ffffffc0368735c0 x29: ffffffc0368735c0 x28: ffffffc036873752 x27: ffffffc035233018 x26: ffffffc000c4e000 x25: 0000000000000000 x24: 0000000000000004 x23: 0000000000000000 x22: ffffffc035233668 x21: ffffff80004e3000 x20: ffffffc0352e0018 x19: 0000004000000000 x18: 0000000000000028 x17: 0000000000000004 x16: ffffffc0017a39c8 x15: 0000000000001cdf x14: ffffffc0019929d8 x13: ffffffc0352e0018 x12: 0000000000000000 x11: 0000000000000001 x10: 0000000000000001 x9 : ffffffc0012b2d70 x8 : ffffff80004e3000 x7 : 0000000000000018 x6 : 0000000030000000 x5 : ffffffc00199f018 x4 : ffffffc035233018 x3 : 0000000000000004 x2 : 00000000c0000000 x1 : 0000000000000003 x0 : 0000000000000000 Process swapper/0 (pid: 1, stack limit = 0xffffffc036870020) Stack: (0xffffffc0368735c0 to 0xffffffc036874000) sg_set_bug expects that the buf parameter passed in should be from lowmem and a valid pageframe. This is not true for pages from dma_alloc_coherent which can be carveouts, hence the check fails. Change allocation of sg buffers from dma_coherent memory to kzalloc to fix the issue. Note that now dma_map/unmap is used to make the kzalloc'ed buffers coherent before passing it to the dmaengine. Signed-off-by: Sricharan R Reviewed-by: Andy Gross Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-qup.c | 51 +++++++++++++------------------------------- 1 file changed, 15 insertions(+), 36 deletions(-) (limited to 'drivers/i2c/busses/i2c-qup.c') diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c index cc6439ab3f71..43adc2d8759b 100644 --- a/drivers/i2c/busses/i2c-qup.c +++ b/drivers/i2c/busses/i2c-qup.c @@ -585,8 +585,8 @@ static void qup_i2c_bam_cb(void *data) } static int qup_sg_set_buf(struct scatterlist *sg, void *buf, - struct qup_i2c_tag *tg, unsigned int buflen, - struct qup_i2c_dev *qup, int map, int dir) + unsigned int buflen, struct qup_i2c_dev *qup, + int dir) { int ret; @@ -595,9 +595,6 @@ static int qup_sg_set_buf(struct scatterlist *sg, void *buf, if (!ret) return -EINVAL; - if (!map) - sg_dma_address(sg) = tg->addr + ((u8 *)buf - tg->start); - return 0; } @@ -670,16 +667,15 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg, /* scratch buf to read the start and len tags */ ret = qup_sg_set_buf(&qup->brx.sg[rx_buf++], &qup->brx.tag.start[0], - &qup->brx.tag, - 2, qup, 0, 0); + 2, qup, DMA_FROM_DEVICE); if (ret) return ret; ret = qup_sg_set_buf(&qup->brx.sg[rx_buf++], &msg->buf[limit * i], - NULL, tlen, qup, - 1, DMA_FROM_DEVICE); + tlen, qup, + DMA_FROM_DEVICE); if (ret) return ret; @@ -688,7 +684,7 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg, } ret = qup_sg_set_buf(&qup->btx.sg[tx_buf++], &qup->start_tag.start[off], - &qup->start_tag, len, qup, 0, 0); + len, qup, DMA_TO_DEVICE); if (ret) return ret; @@ -696,8 +692,7 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg, /* scratch buf to read the BAM EOT and FLUSH tags */ ret = qup_sg_set_buf(&qup->brx.sg[rx_buf++], &qup->brx.tag.start[0], - &qup->brx.tag, 2, - qup, 0, 0); + 2, qup, DMA_FROM_DEVICE); if (ret) return ret; } else { @@ -709,17 +704,15 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg, len = qup_i2c_set_tags(tags, qup, msg, 1); ret = qup_sg_set_buf(&qup->btx.sg[tx_buf++], - tags, - &qup->start_tag, len, - qup, 0, 0); + tags, len, + qup, DMA_TO_DEVICE); if (ret) return ret; tx_len += len; ret = qup_sg_set_buf(&qup->btx.sg[tx_buf++], &msg->buf[limit * i], - NULL, tlen, qup, 1, - DMA_TO_DEVICE); + tlen, qup, DMA_TO_DEVICE); if (ret) return ret; i++; @@ -738,8 +731,7 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg, QUP_BAM_FLUSH_STOP; ret = qup_sg_set_buf(&qup->btx.sg[tx_buf++], &qup->btx.tag.start[0], - &qup->btx.tag, len, - qup, 0, 0); + len, qup, DMA_TO_DEVICE); if (ret) return ret; tx_nents += 1; @@ -1407,27 +1399,21 @@ static int qup_i2c_probe(struct platform_device *pdev) /* 2 tag bytes for each block + 5 for start, stop tags */ size = blocks * 2 + 5; - qup->dpool = dma_pool_create("qup_i2c-dma-pool", &pdev->dev, - size, 4, 0); - qup->start_tag.start = dma_pool_alloc(qup->dpool, GFP_KERNEL, - &qup->start_tag.addr); + qup->start_tag.start = devm_kzalloc(&pdev->dev, + size, GFP_KERNEL); if (!qup->start_tag.start) { ret = -ENOMEM; goto fail_dma; } - qup->brx.tag.start = dma_pool_alloc(qup->dpool, - GFP_KERNEL, - &qup->brx.tag.addr); + qup->brx.tag.start = devm_kzalloc(&pdev->dev, 2, GFP_KERNEL); if (!qup->brx.tag.start) { ret = -ENOMEM; goto fail_dma; } - qup->btx.tag.start = dma_pool_alloc(qup->dpool, - GFP_KERNEL, - &qup->btx.tag.addr); + qup->btx.tag.start = devm_kzalloc(&pdev->dev, 2, GFP_KERNEL); if (!qup->btx.tag.start) { ret = -ENOMEM; goto fail_dma; @@ -1566,13 +1552,6 @@ static int qup_i2c_remove(struct platform_device *pdev) struct qup_i2c_dev *qup = platform_get_drvdata(pdev); if (qup->is_dma) { - dma_pool_free(qup->dpool, qup->start_tag.start, - qup->start_tag.addr); - dma_pool_free(qup->dpool, qup->brx.tag.start, - qup->brx.tag.addr); - dma_pool_free(qup->dpool, qup->btx.tag.start, - qup->btx.tag.addr); - dma_pool_destroy(qup->dpool); dma_release_channel(qup->btx.dma); dma_release_channel(qup->brx.dma); } -- cgit v1.2.3 From fbf9921f8b35d9b241a1ee0008d310a3a5390273 Mon Sep 17 00:00:00 2001 From: Sricharan R Date: Fri, 10 Jun 2016 23:38:21 +0530 Subject: i2c: qup: Fix error handling Among the bus errors reported from the QUP_MASTER_STATUS register only NACK is considered and transfer gets suspended, while other errors are ignored. Correct this and suspend the transfer for other errors as well. This avoids unnecessary 'timeouts' which happens when waiting for events that would never happen when there is already an error condition on the bus. Also the error handling procedure should be the same for both NACK and other bus errors in case of dma mode. So correct that as well. Signed-off-by: Sricharan R Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-qup.c | 76 ++++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 35 deletions(-) (limited to 'drivers/i2c/busses/i2c-qup.c') diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c index 43adc2d8759b..3e5fac88376d 100644 --- a/drivers/i2c/busses/i2c-qup.c +++ b/drivers/i2c/busses/i2c-qup.c @@ -310,6 +310,7 @@ static int qup_i2c_wait_ready(struct qup_i2c_dev *qup, int op, bool val, u32 opflags; u32 status; u32 shift = __ffs(op); + int ret = 0; len *= qup->one_byte_t; /* timeout after a wait of twice the max time */ @@ -321,18 +322,28 @@ static int qup_i2c_wait_ready(struct qup_i2c_dev *qup, int op, bool val, if (((opflags & op) >> shift) == val) { if ((op == QUP_OUT_NOT_EMPTY) && qup->is_last) { - if (!(status & I2C_STATUS_BUS_ACTIVE)) - return 0; + if (!(status & I2C_STATUS_BUS_ACTIVE)) { + ret = 0; + goto done; + } } else { - return 0; + ret = 0; + goto done; } } - if (time_after(jiffies, timeout)) - return -ETIMEDOUT; - + if (time_after(jiffies, timeout)) { + ret = -ETIMEDOUT; + goto done; + } usleep_range(len, len * 2); } + +done: + if (qup->bus_err || qup->qup_err) + ret = (qup->bus_err & QUP_I2C_NACK_FLAG) ? -ENXIO : -EIO; + + return ret; } static void qup_i2c_set_write_mode_v2(struct qup_i2c_dev *qup, @@ -793,39 +804,35 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg, } if (ret || qup->bus_err || qup->qup_err) { - if (qup->bus_err & QUP_I2C_NACK_FLAG) { - msg--; - dev_err(qup->dev, "NACK from %x\n", msg->addr); - ret = -EIO; + if (qup_i2c_change_state(qup, QUP_RUN_STATE)) { + dev_err(qup->dev, "change to run state timed out"); + goto desc_err; + } - if (qup_i2c_change_state(qup, QUP_RUN_STATE)) { - dev_err(qup->dev, "change to run state timed out"); - return ret; - } + if (rx_nents) + writel(QUP_BAM_INPUT_EOT, + qup->base + QUP_OUT_FIFO_BASE); - if (rx_nents) - writel(QUP_BAM_INPUT_EOT, - qup->base + QUP_OUT_FIFO_BASE); + writel(QUP_BAM_FLUSH_STOP, qup->base + QUP_OUT_FIFO_BASE); - writel(QUP_BAM_FLUSH_STOP, - qup->base + QUP_OUT_FIFO_BASE); + qup_i2c_flush(qup); - qup_i2c_flush(qup); + /* wait for remaining interrupts to occur */ + if (!wait_for_completion_timeout(&qup->xfer, HZ)) + dev_err(qup->dev, "flush timed out\n"); - /* wait for remaining interrupts to occur */ - if (!wait_for_completion_timeout(&qup->xfer, HZ)) - dev_err(qup->dev, "flush timed out\n"); + qup_i2c_rel_dma(qup); - qup_i2c_rel_dma(qup); - } + ret = (qup->bus_err & QUP_I2C_NACK_FLAG) ? -ENXIO : -EIO; } +desc_err: dma_unmap_sg(qup->dev, qup->btx.sg, tx_nents, DMA_TO_DEVICE); if (rx_nents) dma_unmap_sg(qup->dev, qup->brx.sg, rx_nents, DMA_FROM_DEVICE); -desc_err: + return ret; } @@ -841,9 +848,6 @@ static int qup_i2c_bam_xfer(struct i2c_adapter *adap, struct i2c_msg *msg, if (ret) goto out; - qup->bus_err = 0; - qup->qup_err = 0; - writel(0, qup->base + QUP_MX_INPUT_CNT); writel(0, qup->base + QUP_MX_OUTPUT_CNT); @@ -881,12 +885,8 @@ static int qup_i2c_wait_for_complete(struct qup_i2c_dev *qup, ret = -ETIMEDOUT; } - if (qup->bus_err || qup->qup_err) { - if (qup->bus_err & QUP_I2C_NACK_FLAG) { - dev_err(qup->dev, "NACK from %x\n", msg->addr); - ret = -EIO; - } - } + if (qup->bus_err || qup->qup_err) + ret = (qup->bus_err & QUP_I2C_NACK_FLAG) ? -ENXIO : -EIO; return ret; } @@ -1178,6 +1178,9 @@ static int qup_i2c_xfer(struct i2c_adapter *adap, if (ret < 0) goto out; + qup->bus_err = 0; + qup->qup_err = 0; + writel(1, qup->base + QUP_SW_RESET); ret = qup_i2c_poll_state(qup, QUP_RESET_STATE); if (ret) @@ -1227,6 +1230,9 @@ static int qup_i2c_xfer_v2(struct i2c_adapter *adap, struct qup_i2c_dev *qup = i2c_get_adapdata(adap); int ret, len, idx = 0, use_dma = 0; + qup->bus_err = 0; + qup->qup_err = 0; + ret = pm_runtime_get_sync(qup->dev); if (ret < 0) goto out; -- cgit v1.2.3 From 0130944bc1d212ad4b33584424d56fe7c049eb65 Mon Sep 17 00:00:00 2001 From: Naveen Kaje Date: Thu, 5 May 2016 12:33:17 -0600 Subject: i2c: qup: use address helper function in read transfer qup_i2c_issue_read() derives the address from i2c_msg. This called in the read path when I2C_M_RD flag is set. Therefore, use the 8 bit address helper function. Signed-off-by: Naveen Kaje Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-qup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/i2c/busses/i2c-qup.c') diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c index 3e5fac88376d..69849a95d479 100644 --- a/drivers/i2c/busses/i2c-qup.c +++ b/drivers/i2c/busses/i2c-qup.c @@ -1012,7 +1012,7 @@ static void qup_i2c_issue_read(struct qup_i2c_dev *qup, struct i2c_msg *msg) { u32 addr, len, val; - addr = (msg->addr << 1) | 1; + addr = i2c_8bit_addr_from_msg(msg); /* 0 is used to specify a length 256 (QUP_READ_LIMIT) */ len = (msg->len == QUP_READ_LIMIT) ? 0 : msg->len; -- cgit v1.2.3 From 2b84a4dd4be0fd6d6fecdb14040cbbc38a3b525a Mon Sep 17 00:00:00 2001 From: Abhishek Sahu Date: Mon, 9 May 2016 18:14:30 +0530 Subject: i2c: qup: Cleared the error bits in ISR 1. Current QCOM I2C driver hangs when sending data to address 0x03-0x07 in some scenarios. The QUP controller generates invalid write in this case, since these addresses are reserved for different bus formats. 2. Also, the error handling is done by I2C QUP ISR in the case of DMA mode. The state need to be RESET in case of any error for clearing the available data in FIFO, which otherwise leaves the BAM DMA controller in hang state. This patch fixes the above two issues by clearing the error bits from I2C and QUP status in ISR in case of I2C error, QUP error and resets the QUP state to clear the FIFO data. Signed-off-by: Abhishek Sahu Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-qup.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'drivers/i2c/busses/i2c-qup.c') diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c index 69849a95d479..686f262e34f7 100644 --- a/drivers/i2c/busses/i2c-qup.c +++ b/drivers/i2c/busses/i2c-qup.c @@ -213,14 +213,16 @@ static irqreturn_t qup_i2c_interrupt(int irq, void *dev) bus_err &= I2C_STATUS_ERROR_MASK; qup_err &= QUP_STATUS_ERROR_FLAGS; - if (qup_err) { - /* Clear Error interrupt */ + /* Clear the error bits in QUP_ERROR_FLAGS */ + if (qup_err) writel(qup_err, qup->base + QUP_ERROR_FLAGS); - goto done; - } - if (bus_err) { - /* Clear Error interrupt */ + /* Clear the error bits in QUP_I2C_STATUS */ + if (bus_err) + writel(bus_err, qup->base + QUP_I2C_STATUS); + + /* Reset the QUP State in case of error */ + if (qup_err || bus_err) { writel(QUP_RESET_STATE, qup->base + QUP_STATE); goto done; } -- cgit v1.2.3 From 5c135e151aee5f7e7f2ffb336cdb3e4e041caef4 Mon Sep 17 00:00:00 2001 From: Abhishek Sahu Date: Mon, 9 May 2016 18:14:31 +0530 Subject: i2c: qup: Fixed the DMA segments length 1. The current QCOM I2C driver code is failing for transfer length greater than 255. This is happening due to improper segments length as the I2C DMA segments can be maximum of 256 bytes. 2. The transfer length tlen was being initialized with 0 for 256 bytes, which is being passed for DMA mappings resulting in improper DMA mapping length. This patch fixes the above said problems by initializing the block count with the values calculated in qup_i2c_set_blk_data and calculating the remaining length for last DMA segment. Also, the block data length need to be decremented after each transfer. Additionally, this patch corrects the tlen assignment for DMA mapping. Signed-off-by: Abhishek Sahu Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-qup.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'drivers/i2c/busses/i2c-qup.c') diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c index 686f262e34f7..29139bbe8f85 100644 --- a/drivers/i2c/busses/i2c-qup.c +++ b/drivers/i2c/busses/i2c-qup.c @@ -659,23 +659,24 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg, u8 *tags; while (idx < num) { - blocks = (msg->len + limit) / limit; - rem = msg->len % limit; tx_len = 0, len = 0, i = 0; qup->is_last = (idx == (num - 1)); qup_i2c_set_blk_data(qup, msg); + blocks = qup->blk.count; + rem = msg->len - (blocks - 1) * limit; + if (msg->flags & I2C_M_RD) { rx_nents += (blocks * 2) + 1; tx_nents += 1; while (qup->blk.pos < blocks) { - /* length set to '0' implies 256 bytes */ - tlen = (i == (blocks - 1)) ? rem : 0; + tlen = (i == (blocks - 1)) ? rem : limit; tags = &qup->start_tag.start[off + len]; len += qup_i2c_set_tags(tags, qup, msg, 1); + qup->blk.data_len -= tlen; /* scratch buf to read the start and len tags */ ret = qup_sg_set_buf(&qup->brx.sg[rx_buf++], @@ -712,9 +713,10 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg, tx_nents += (blocks * 2); while (qup->blk.pos < blocks) { - tlen = (i == (blocks - 1)) ? rem : 0; + tlen = (i == (blocks - 1)) ? rem : limit; tags = &qup->start_tag.start[off + tx_len]; len = qup_i2c_set_tags(tags, qup, msg, 1); + qup->blk.data_len -= tlen; ret = qup_sg_set_buf(&qup->btx.sg[tx_buf++], tags, len, -- cgit v1.2.3