* [PATCH V2 0/2] drivers: i2c: qup: Some misc fixes
@ 2016-05-06 11:31 Sricharan R
2016-05-06 11:31 ` [PATCH V2 1/2] drivers: i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled Sricharan R
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Sricharan R @ 2016-05-06 11:31 UTC (permalink / raw)
To: devicetree, architt, linux-arm-msm, ntelkar, galak, linux-kernel,
andy.gross, linux-i2c, iivanov, agross, dmaengine,
linux-arm-kernel, nkaje
Cc: sricharan
One for fixing the bug with CONFIG_DEBUG_SG enabled and another
to suspend the transfer for all errors instead of just for nack.
Sricharan R (2):
drivers: i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled.
drivers: i2c: qup: Fix error handling
drivers/i2c/busses/i2c-qup.c | 87 +++++++++++++++++++++-----------------------
1 file changed, 42 insertions(+), 45 deletions(-)
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH V2 1/2] drivers: i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled 2016-05-06 11:31 [PATCH V2 0/2] drivers: i2c: qup: Some misc fixes Sricharan R @ 2016-05-06 11:31 ` Sricharan R 2016-05-06 17:36 ` [V2, " Naveen Kaje 2016-05-13 11:47 ` [PATCH V2 " Wolfram Sang 2016-05-06 11:31 ` [PATCH V2 2/2] drivers: i2c: qup: Fix error handling Sricharan R 2016-05-13 11:44 ` [PATCH V2 0/2] drivers: i2c: qup: Some misc fixes Wolfram Sang 2 siblings, 2 replies; 12+ messages in thread From: Sricharan R @ 2016-05-06 11:31 UTC (permalink / raw) To: devicetree, architt, linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross, linux-i2c, iivanov, agross, dmaengine, linux-arm-kernel, nkaje Cc: sricharan 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 : [<ffffffc0005a0ed8>] lr : [<ffffffc0005a0df0>] 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) Change allocation of sg buffers from dma_coherent memory to kzalloc to fix the issue. Signed-off-by: Sricharan R <sricharan@codeaurora.org> Reviewed-by: Andy Gross <andy.gross@linaro.org> --- drivers/i2c/busses/i2c-qup.c | 53 ++++++++++++++------------------------------ 1 file changed, 17 insertions(+), 36 deletions(-) diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c index 23eaabb..8620e99 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; @@ -1268,6 +1260,8 @@ static int qup_i2c_xfer_v2(struct i2c_adapter *adap, } } + idx = 0; + do { if (msgs[idx].len == 0) { ret = -EINVAL; @@ -1407,27 +1401,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 +1554,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); } -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [V2, 1/2] drivers: i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled 2016-05-06 11:31 ` [PATCH V2 1/2] drivers: i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled Sricharan R @ 2016-05-06 17:36 ` Naveen Kaje 2016-05-13 11:47 ` [PATCH V2 " Wolfram Sang 1 sibling, 0 replies; 12+ messages in thread From: Naveen Kaje @ 2016-05-06 17:36 UTC (permalink / raw) To: Sricharan R, devicetree, architt, linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross, linux-i2c, iivanov, agross, dmaengine, linux-arm-kernel On 5/6/2016 5:31 AM, Sricharan R wrote: > 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 : [<ffffffc0005a0ed8>] lr : [<ffffffc0005a0df0>] 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) > > Change allocation of sg buffers from dma_coherent memory to kzalloc > to fix the issue. > > Signed-off-by: Sricharan R <sricharan@codeaurora.org> > Reviewed-by: Andy Gross <andy.gross@linaro.org> Tested on QDF2432 with ACPI and SMBus support patches for regressions. Note that this platform does not use DMA mode. Tested-by: Naveen Kaje <nkaje@codeaurora.org> > --- > drivers/i2c/busses/i2c-qup.c | 53 ++++++++++++++------------------------------ > 1 file changed, 17 insertions(+), 36 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c > index 23eaabb..8620e99 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; > @@ -1268,6 +1260,8 @@ static int qup_i2c_xfer_v2(struct i2c_adapter *adap, > } > } > > + idx = 0; > + > do { > if (msgs[idx].len == 0) { > ret = -EINVAL; > @@ -1407,27 +1401,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 +1554,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); > } ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 1/2] drivers: i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled 2016-05-06 11:31 ` [PATCH V2 1/2] drivers: i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled Sricharan R 2016-05-06 17:36 ` [V2, " Naveen Kaje @ 2016-05-13 11:47 ` Wolfram Sang 2016-05-16 5:19 ` Sricharan 1 sibling, 1 reply; 12+ messages in thread From: Wolfram Sang @ 2016-05-13 11:47 UTC (permalink / raw) To: Sricharan R Cc: devicetree, architt, linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross, linux-i2c, iivanov, agross, dmaengine, linux-arm-kernel, nkaje [-- Attachment #1: Type: text/plain, Size: 1942 bytes --] On Fri, May 06, 2016 at 05:01:13PM +0530, Sricharan R wrote: > 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 : [<ffffffc0005a0ed8>] lr : [<ffffffc0005a0df0>] 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) > > Change allocation of sg buffers from dma_coherent memory to kzalloc > to fix the issue. This description describes what you do. But not why it is the correct solution to the OOPS. The OOPS neither describes it. Please add some more explanation. > > Signed-off-by: Sricharan R <sricharan@codeaurora.org> > Reviewed-by: Andy Gross <andy.gross@linaro.org> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH V2 1/2] drivers: i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled 2016-05-13 11:47 ` [PATCH V2 " Wolfram Sang @ 2016-05-16 5:19 ` Sricharan 0 siblings, 0 replies; 12+ messages in thread From: Sricharan @ 2016-05-16 5:19 UTC (permalink / raw) To: 'Wolfram Sang' Cc: devicetree, architt, linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross, linux-i2c, iivanov, agross, dmaengine, linux-arm-kernel, nkaje Hi, > > 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 : [<ffffffc0005a0ed8>] > > lr : [<ffffffc0005a0df0>] 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) > > > > Change allocation of sg buffers from dma_coherent memory to kzalloc to > > fix the issue. > > This description describes what you do. But not why it is the correct solution > to the OOPS. The OOPS neither describes it. Please add some more > explanation. > Ok,will describe it more. The reason it oops is sg_set_bug expects that the buf parameter passed in should be a from the lowmem and a valid pageframe. This is not true for pages from dma_alloc_coherent which can be carveouts, hence the check fails. Allocating buffers using kzalloc fixes the issue. Regards, Sricharan ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V2 2/2] drivers: i2c: qup: Fix error handling 2016-05-06 11:31 [PATCH V2 0/2] drivers: i2c: qup: Some misc fixes Sricharan R 2016-05-06 11:31 ` [PATCH V2 1/2] drivers: i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled Sricharan R @ 2016-05-06 11:31 ` Sricharan R 2016-05-06 17:53 ` [V2,2/2] " Naveen Kaje [not found] ` <1462534274-28925-3-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 2016-05-13 11:44 ` [PATCH V2 0/2] drivers: i2c: qup: Some misc fixes Wolfram Sang 2 siblings, 2 replies; 12+ messages in thread From: Sricharan R @ 2016-05-06 11:31 UTC (permalink / raw) To: devicetree, architt, linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross, linux-i2c, iivanov, agross, dmaengine, linux-arm-kernel, nkaje Cc: sricharan 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 unnessecary 'timeouts' which happens when waiting for events that would never happen when there is already an error condition on the bus. Signed-off-by: Sricharan R <sricharan@codeaurora.org> Reviewed-by: Andy Gross <andy.gross@linaro.org> --- drivers/i2c/busses/i2c-qup.c | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c index 8620e99..af52a47 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,31 @@ 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) { + if (qup->bus_err & QUP_I2C_NACK_FLAG) + dev_err(qup->dev, "NACK from %x\n", qup->msg->addr); + ret = -EIO; + } + + return ret; } static void qup_i2c_set_write_mode_v2(struct qup_i2c_dev *qup, @@ -796,7 +810,6 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg, 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"); @@ -818,6 +831,7 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg, qup_i2c_rel_dma(qup); } + ret = -EIO; } dma_unmap_sg(qup->dev, qup->btx.sg, tx_nents, DMA_TO_DEVICE); @@ -841,9 +855,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); @@ -882,10 +893,9 @@ static int qup_i2c_wait_for_complete(struct qup_i2c_dev *qup, } if (qup->bus_err || qup->qup_err) { - if (qup->bus_err & QUP_I2C_NACK_FLAG) { + if (qup->bus_err & QUP_I2C_NACK_FLAG) dev_err(qup->dev, "NACK from %x\n", msg->addr); - ret = -EIO; - } + ret = -EIO; } return ret; @@ -1178,6 +1188,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 +1240,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; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [V2,2/2] drivers: i2c: qup: Fix error handling 2016-05-06 11:31 ` [PATCH V2 2/2] drivers: i2c: qup: Fix error handling Sricharan R @ 2016-05-06 17:53 ` Naveen Kaje [not found] ` <1462534274-28925-3-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> 1 sibling, 0 replies; 12+ messages in thread From: Naveen Kaje @ 2016-05-06 17:53 UTC (permalink / raw) To: Sricharan R, devicetree, architt, linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross, linux-i2c, iivanov, agross, dmaengine, linux-arm-kernel On 5/6/2016 5:31 AM, Sricharan R wrote: > 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 unnessecary 'timeouts' which > happens when waiting for events that would never happen when there > is already an error condition on the bus. > > Signed-off-by: Sricharan R <sricharan@codeaurora.org> > Reviewed-by: Andy Gross <andy.gross@linaro.org> Hi Sricharan, Looks good to me. Tested on QDF2432 platform. Reviewed-by: Naveen Kaje <nkaje@codeaurora.org> Tested-by: Naveen Kaje <nkaje@codeaurora.org> > --- > drivers/i2c/busses/i2c-qup.c | 42 +++++++++++++++++++++++++++++------------- > 1 file changed, 29 insertions(+), 13 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c > index 8620e99..af52a47 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,31 @@ 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) { > + if (qup->bus_err & QUP_I2C_NACK_FLAG) > + dev_err(qup->dev, "NACK from %x\n", qup->msg->addr); > + ret = -EIO; > + } > + > + return ret; > } > > static void qup_i2c_set_write_mode_v2(struct qup_i2c_dev *qup, > @@ -796,7 +810,6 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg, > 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"); > @@ -818,6 +831,7 @@ static int qup_i2c_bam_do_xfer(struct qup_i2c_dev *qup, struct i2c_msg *msg, > > qup_i2c_rel_dma(qup); > } > + ret = -EIO; > } > > dma_unmap_sg(qup->dev, qup->btx.sg, tx_nents, DMA_TO_DEVICE); > @@ -841,9 +855,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); > > @@ -882,10 +893,9 @@ static int qup_i2c_wait_for_complete(struct qup_i2c_dev *qup, > } > > if (qup->bus_err || qup->qup_err) { > - if (qup->bus_err & QUP_I2C_NACK_FLAG) { > + if (qup->bus_err & QUP_I2C_NACK_FLAG) > dev_err(qup->dev, "NACK from %x\n", msg->addr); > - ret = -EIO; > - } > + ret = -EIO; > } > > return ret; > @@ -1178,6 +1188,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 +1240,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; ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <1462534274-28925-3-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* Re: [PATCH V2 2/2] drivers: i2c: qup: Fix error handling [not found] ` <1462534274-28925-3-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> @ 2016-05-13 11:48 ` Wolfram Sang 2016-05-13 11:50 ` Wolfram Sang 0 siblings, 1 reply; 12+ messages in thread From: Wolfram Sang @ 2016-05-13 11:48 UTC (permalink / raw) To: Sricharan R Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, architt-sgV2jX0FEOL9JmXXK+q4OQ, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, ntelkar-sgV2jX0FEOL9JmXXK+q4OQ, galak-sgV2jX0FEOL9JmXXK+q4OQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA, andy.gross-QSEj5FYQhm4dnm+yROfE0A, linux-i2c-u79uwXL29TY76Z2rM5mHXA, iivanov-NEYub+7Iv8PQT0dZR+AlfA, agross-sgV2jX0FEOL9JmXXK+q4OQ, dmaengine-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, nkaje-sgV2jX0FEOL9JmXXK+q4OQ [-- Attachment #1: Type: text/plain, Size: 683 bytes --] On Fri, May 06, 2016 at 05:01:14PM +0530, Sricharan R wrote: > 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 unnessecary 'timeouts' which > happens when waiting for events that would never happen when there > is already an error condition on the bus. > > Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> > Reviewed-by: Andy Gross <andy.gross-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Please check Documentation/i2c/fault-codes for proper fault codes. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 2/2] drivers: i2c: qup: Fix error handling 2016-05-13 11:48 ` [PATCH V2 2/2] " Wolfram Sang @ 2016-05-13 11:50 ` Wolfram Sang 2016-05-16 5:59 ` Sricharan 0 siblings, 1 reply; 12+ messages in thread From: Wolfram Sang @ 2016-05-13 11:50 UTC (permalink / raw) To: Sricharan R Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, architt-sgV2jX0FEOL9JmXXK+q4OQ, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, ntelkar-sgV2jX0FEOL9JmXXK+q4OQ, galak-sgV2jX0FEOL9JmXXK+q4OQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA, andy.gross-QSEj5FYQhm4dnm+yROfE0A, linux-i2c-u79uwXL29TY76Z2rM5mHXA, iivanov-NEYub+7Iv8PQT0dZR+AlfA, agross-sgV2jX0FEOL9JmXXK+q4OQ, dmaengine-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, nkaje-sgV2jX0FEOL9JmXXK+q4OQ [-- Attachment #1: Type: text/plain, Size: 946 bytes --] On Fri, May 13, 2016 at 01:48:23PM +0200, Wolfram Sang wrote: > On Fri, May 06, 2016 at 05:01:14PM +0530, Sricharan R wrote: > > 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 unnessecary 'timeouts' which > > happens when waiting for events that would never happen when there > > is already an error condition on the bus. > > > > Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> > > Reviewed-by: Andy Gross <andy.gross-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > > Please check Documentation/i2c/fault-codes for proper fault codes. And while we are here: NACK is not an error but a valid response. Can you remove the dev_err related to that? Otherwise tools like i2cdetect will probably flood your log. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH V2 2/2] drivers: i2c: qup: Fix error handling 2016-05-13 11:50 ` Wolfram Sang @ 2016-05-16 5:59 ` Sricharan 0 siblings, 0 replies; 12+ messages in thread From: Sricharan @ 2016-05-16 5:59 UTC (permalink / raw) To: 'Wolfram Sang' Cc: devicetree, architt, linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross, linux-i2c, iivanov, agross, dmaengine, linux-arm-kernel, nkaje Hi, > > > 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 unnessecary 'timeouts' which happens > > > when waiting for events that would never happen when there is > > > already an error condition on the bus. > > > > > > Signed-off-by: Sricharan R <sricharan@codeaurora.org> > > > Reviewed-by: Andy Gross <andy.gross@linaro.org> > > > > Please check Documentation/i2c/fault-codes for proper fault codes. > Ok, Thanks for pointing this out. So, for NACK I think it is more appropriate to return -ENXIO and -EIO for other errors. Will correct this. > And while we are here: NACK is not an error but a valid response. Can you > remove the dev_err related to that? Otherwise tools like i2cdetect will > probably flood your log. Ok, will correct this. Regards, Sricharan ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V2 0/2] drivers: i2c: qup: Some misc fixes 2016-05-06 11:31 [PATCH V2 0/2] drivers: i2c: qup: Some misc fixes Sricharan R 2016-05-06 11:31 ` [PATCH V2 1/2] drivers: i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled Sricharan R 2016-05-06 11:31 ` [PATCH V2 2/2] drivers: i2c: qup: Fix error handling Sricharan R @ 2016-05-13 11:44 ` Wolfram Sang 2016-05-16 5:13 ` Sricharan 2 siblings, 1 reply; 12+ messages in thread From: Wolfram Sang @ 2016-05-13 11:44 UTC (permalink / raw) To: Sricharan R Cc: devicetree, architt, linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross, linux-i2c, iivanov, agross, dmaengine, linux-arm-kernel, nkaje [-- Attachment #1: Type: text/plain, Size: 239 bytes --] On Fri, May 06, 2016 at 05:01:12PM +0530, Sricharan R wrote: > One for fixing the bug with CONFIG_DEBUG_SG enabled and another > to suspend the transfer for all errors instead of just for nack. You haven't stated what was changed in V2. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH V2 0/2] drivers: i2c: qup: Some misc fixes 2016-05-13 11:44 ` [PATCH V2 0/2] drivers: i2c: qup: Some misc fixes Wolfram Sang @ 2016-05-16 5:13 ` Sricharan 0 siblings, 0 replies; 12+ messages in thread From: Sricharan @ 2016-05-16 5:13 UTC (permalink / raw) To: 'Wolfram Sang' Cc: devicetree, architt, linux-arm-msm, ntelkar, galak, linux-kernel, andy.gross, linux-i2c, iivanov, agross, dmaengine, linux-arm-kernel, nkaje Hi, > > One for fixing the bug with CONFIG_DEBUG_SG enabled and another to > > suspend the transfer for all errors instead of just for nack. > > You haven't stated what was changed in V2. ah, sorry, will resend.. Regards, Sricharan ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-05-16 5:59 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-06 11:31 [PATCH V2 0/2] drivers: i2c: qup: Some misc fixes Sricharan R
2016-05-06 11:31 ` [PATCH V2 1/2] drivers: i2c: qup: Fix broken dma when CONFIG_DEBUG_SG is enabled Sricharan R
2016-05-06 17:36 ` [V2, " Naveen Kaje
2016-05-13 11:47 ` [PATCH V2 " Wolfram Sang
2016-05-16 5:19 ` Sricharan
2016-05-06 11:31 ` [PATCH V2 2/2] drivers: i2c: qup: Fix error handling Sricharan R
2016-05-06 17:53 ` [V2,2/2] " Naveen Kaje
[not found] ` <1462534274-28925-3-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2016-05-13 11:48 ` [PATCH V2 2/2] " Wolfram Sang
2016-05-13 11:50 ` Wolfram Sang
2016-05-16 5:59 ` Sricharan
2016-05-13 11:44 ` [PATCH V2 0/2] drivers: i2c: qup: Some misc fixes Wolfram Sang
2016-05-16 5:13 ` Sricharan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).