From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jaehoon Chung Subject: Re: mmc: dw_mmc: warning with CONFIG_DMA_API_DEBUG Date: Fri, 24 Jun 2016 13:30:29 +0900 Message-ID: <576CB765.1060709@samsung.com> References: <001501d1cace$9b8f2b60$d2ad8220$@samsung.com> <000801d1cb64$070dfb60$1529f220$@samsung.com> <5768D188.3080508@samsung.com> <576BCA56.90206@samsung.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from mailout4.samsung.com ([203.254.224.34]:60773 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750833AbcFXEac (ORCPT ); Fri, 24 Jun 2016 00:30:32 -0400 In-reply-to: Sender: linux-mmc-owner@vger.kernel.org List-Id: linux-mmc@vger.kernel.org To: Shawn Lin , Seung-Woo Kim , ulf.hansson@linaro.org, linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org On 06/24/2016 10:25 AM, Shawn Lin wrote: > Hi Jaehoon, >=20 > On 2016/6/23 19:39, Jaehoon Chung wrote: >> Hi Shawn, >> >> On 06/21/2016 04:39 PM, Shawn Lin wrote: >>> =E5=9C=A8 2016/6/21 13:32, Jaehoon Chung =E5=86=99=E9=81=93: >>>> Hi guys, >>>> >>>> On 06/21/2016 11:31 AM, Shawn Lin wrote: >>>>> On 2016/6/21 10:24, Seung-Woo Kim wrote: >>>>>> Hello Shawn, >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: Shawn Lin [mailto:shawn.lin@rock-chips.com] >>>>>>> Sent: Tuesday, June 21, 2016 10:52 AM >>>>>>> To: Seung-Woo Kim; jh80.chung@samsung.com; ulf.hansson@linaro.o= rg; linux-mmc@vger.kernel.org; linux- >>>>>>> kernel@vger.kernel.org >>>>>>> Cc: shawn.lin@rock-chips.com >>>>>>> Subject: Re: mmc: dw_mmc: warning with CONFIG_DMA_API_DEBUG >>>>>>> >>>>>>> On 2016/6/20 16:34, Seung-Woo Kim wrote: >>>>>>>> Hi folks, >>>>>>>> >>>>>>>> During booting test on my Exynos5422 based Odroid-XU3, kernel = compiled >>>>>>>> with CONFIG_DMA_API_DEBUG reported following warning: >>>>>>>> >>>>>>>> ------------[ cut here ]------------ >>>>>>>> WARNING: CPU: 0 PID: 0 at lib/dma-debug.c:1096 check_unmap+0x7= bc/0xb38 >>>>>>>> dwmmc_exynos 12200000.mmc: DMA-API: device driver tries to fre= e DMA memory it has not allocated [device >>>>>>> address=3D0x000000006d9d2200] >>>>>>> >>>>>>> Thanks for this report and fix. >>>>>>> >>>>>>> DTO(the same as IDMAC-RI/TI) interrupts may or may not come tog= ether >>>>>>> with DATA_ERR. If DATA_ERR occur without geting DTO, we should = issue >>>>>>> CMD12 manually to generate DTO. It's a ugly deisgn for dwmmc bu= t from >>>>>>> the vendor's ask. >>>>>>> >>>>>>> So you should never think we complete the xfer without >>>>>>> checking DATA_ERR. This way you got the warning. >>>> >>>> Well, EVENT_DATA_ERR is already checked in tasklet_func..and clear= ed that flags. >>> >>> From my view, the reality is that when we got DATA_ERROR interrupts= , >>> we set EVENT_DATA_ERR to the pending_events and schedule the taskle= t >>> but we may still fallback to the IDMAC interrupt case as the taskle= t >>> may come up a little late, namely right after the IDMAC interrupt c= hecking. >>> >>> I'm trying to add some log there, and it well proves my guess. >> >> You're right..This is appeared because of "Data Over". >> If Data Over interrupt is occurred, SW needs to read the remaining D= ata in FIFO. >> At that time, it was set to DATA_COMPLETE. Because SW might read the= remaining data, but already set to ERROR_DATA. >> >> In this case, Your suggestion may prevent to free twice. >> >> There is other case..during tuning sequence.. :( >> I found that it also appeared during the tuning sequence. >> - Really..stupid design.. >> >> My suggestions are >> First, apply the below solution. >=20 > Which solution? This $SUBJECT or the one I sent? Yours. :) >=20 >=20 >=20 >> And then consider the HS200 tuning block with the below patch. >> >> https://patchwork.kernel.org/patch/8935791/ >=20 > I saw this patch long ago, but I still have not seen > this issue or got reports for it. From the code itself, it should > be ok to landed as I checked the databook. :) >=20 >> >> How about? >> Do you have any other opinion? >> >> Best Regards, >> Jaehoon Chung >> >>> >>>> >>>>>>> >>>>>>> So could you try this one: >>>>>> >>>>>> With your patch, there is no more the DMA API waring in my envir= onment. >>>>> >>>>> Nice to hear that. Thanks for testing, Seung-Woo. >>>> >>>> Really? It's not solution..When send tuning command, it should be = returned CRC error. >>>> Then it called the dw_mci_stop_dma() and also dma_ops->complete(). >>> >>> Hrmm.. I can't see the reason it will also call dma_ops->complete. >>> Could you explain a bit more here? :) >>> >>> From V2.70a Table 3-2 >>> For MMC CMD19, there may be no CRC status returned by the >>> card but EBE is generated. Hence, EBE is set for CMD19. The applica= tion >>> should not treat this as an error. >>> >>> >>>> >>>> When i applied you suggestion, also produced.. :) >>>> >>>> [ 2.469916] [] (unwind_backtrace) from [] (= show_stack+0x10/0x14) >>>> [ 2.469934] [] (show_stack) from [] (dump_s= tack+0x74/0x94) >>>> [ 2.469949] [] (dump_stack) from [] (__warn= +0xd4/0x100) >>>> [ 2.469961] [] (__warn) from [] (warn_slowp= ath_fmt+0x38/0x48) >>>> [ 2.469975] [] (warn_slowpath_fmt) from [] = (check_unmap+0x828/0x8a8) >>>> [ 2.469991] [] (check_unmap) from [] (debug= _dma_unmap_sg+0x5c/0x13c) >>>> [ 2.470012] [] (debug_dma_unmap_sg) from []= (dw_mci_dma_cleanup+0x68/0xa4) >>>> [ 2.470029] [] (dw_mci_dma_cleanup) from []= (dw_mci_stop_dma+0x30/0x40) >>>> [ 2.470045] [] (dw_mci_stop_dma) from [] (d= w_mci_tasklet_func+0x340/0x3b4) >>>> [ 2.470063] [] (dw_mci_tasklet_func) from [= ] (tasklet_action+0x84/0x12c) >>>> [ 2.470076] [] (tasklet_action) from [] (__= do_softirq+0xec/0x244) >>>> [ 2.470089] [] (__do_softirq) from [] (irq_= exit+0xb4/0xf8) >>>> [ 2.470109] [] (irq_exit) from [] (__handle= _domain_irq+0x70/0xe4) >>>> [ 2.470123] [] (__handle_domain_irq) from [= ] (gic_handle_irq+0x50/0x9c) >>>> [ 2.470135] [] (gic_handle_irq) from [] (__= irq_svc+0x54/0x90) >>>> [ 2.470141] Exception stack(0xee3d3c58 to 0xee3d3ca0) >>>> [ 2.470148] 3c40: = c0b1ef0c 0000000a >>>> [ 2.470159] 3c60: 00000001 0000005c 0000016d 00000000 c0b4f7a4 = 00000006 00000000 0000005c >>>> [ 2.470170] 3c80: 00000006 c0b671e8 60000013 ee3d3ca8 c0398e08 = c015b830 60000013 ffffffff >>>> [ 2.470183] [] (__irq_svc) from [] (console= _unlock+0x560/0x628) >>>> [ 2.470196] [] (console_unlock) from [] (vp= rintk_emit+0x1fc/0x508) >>>> [ 2.470211] [] (vprintk_emit) from [] (dev_= vprintk_emit+0xf8/0x198) >>>> [ 2.470224] [] (dev_vprintk_emit) from [] (= dev_printk_emit+0x1c/0x2c) >>>> [ 2.470235] [] (dev_printk_emit) from [] (_= _dev_printk+0x4c/0x70) >>>> [ 2.470246] [] (__dev_printk) from [] (_dev= _info+0x38/0x48) >>>> [ 2.470258] [] (_dev_info) from [] (usb_new= _device+0xe8/0x3d0) >>>> [ 2.470270] [] (usb_new_device) from [] (hu= b_event+0x760/0xff4) >>>> [ 2.470289] [] (hub_event) from [] (process= _one_work+0x120/0x328) >>>> [ 2.470304] [] (process_one_work) from [] (= worker_thread+0x2c/0x4ac) >>>> [ 2.470321] [] (worker_thread) from [] (kth= read+0xd8/0xf4) >>>> [ 2.470335] [] (kthread) from [>>> >>>> I'm checking this.. >>>> >>>> Best Regards, >>>> Jaehoon Chung >>>> >>>>> >>>>> Hi Jaehoon, >>>>> >>>>> How about this? >>>>> >>>>>> >>>>>> Best Regards, >>>>>> - Seung-Woo Kim >>>>>> >>>>>>> >>>>>>> --- a/drivers/mmc/host/dw_mmc.c >>>>>>> +++ b/drivers/mmc/host/dw_mmc.c >>>>>>> @@ -2474,7 +2474,8 @@ static irqreturn_t dw_mci_interrupt(int i= rq, void >>>>>>> *dev_id) >>>>>>> mci_writel(host, IDSTS64, SDMMC_IDMAC_= INT_TI | >>>>>>> >>>>>>> SDMMC_IDMAC_INT_RI); >>>>>>> mci_writel(host, IDSTS64, SDMMC_IDMAC_= INT_NI); >>>>>>> - host->dma_ops->complete((void *)host); >>>>>>> + if (!test_bit(EVENT_DATA_ERROR, >>>>>>> &host->pending_events)) >>>>>>> + host->dma_ops->complete((void *= )host); >>>>>>> } >>>>>>> } else { >>>>>>> pending =3D mci_readl(host, IDSTS); >>>>>>> @@ -2482,7 +2483,8 @@ static irqreturn_t dw_mci_interrupt(int i= rq, void >>>>>>> *dev_id) >>>>>>> mci_writel(host, IDSTS, SDMMC_IDMAC_IN= T_TI | >>>>>>> >>>>>>> SDMMC_IDMAC_INT_RI); >>>>>>> mci_writel(host, IDSTS, SDMMC_IDMAC_IN= T_NI); >>>>>>> - host->dma_ops->complete((void *)host); >>>>>>> + if (!test_bit(EVENT_DATA_ERROR, >>>>>>> &host->pending_events)) >>>>>>> + host->dma_ops->complete((void *= )host); >>>>>>> } >>>>>>> } >>>>>>> >>>>>>> >>>>>>>> [size=3D128 bytes] >>>>>>>> Modules linked in: >>>>>>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.7.0-rc4 #26 >>>>>>>> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) >>>>>>>> [] (unwind_backtrace) from [] (show_stack+= 0x20/0x24) >>>>>>>> [] (show_stack) from [] (dump_stack+0x80/0= x94) >>>>>>>> [] (dump_stack) from [] (__warn+0xf8/0x110= ) >>>>>>>> [] (__warn) from [] (warn_slowpath_fmt+0x4= 8/0x50) >>>>>>>> [] (warn_slowpath_fmt) from [] (check_unma= p+0x7bc/0xb38) >>>>>>>> [] (check_unmap) from [] (debug_dma_unmap_= sg+0x118/0x148) >>>>>>>> [] (debug_dma_unmap_sg) from [] (dw_mci_dm= a_cleanup+0x7c/0xb8) >>>>>>>> [] (dw_mci_dma_cleanup) from [] (dw_mci_st= op_dma+0x40/0x50) >>>>>>>> [] (dw_mci_stop_dma) from [] (dw_mci_taskl= et_func+0x130/0x3b4) >>>>>>>> [] (dw_mci_tasklet_func) from [] (tasklet_= action+0xb4/0x150) >>>>>>>> [] (tasklet_action) from [] (__do_softirq+= 0xe4/0x3cc) >>>>>>>> [] (__do_softirq) from [] (irq_exit+0xd0/0= x10c) >>>>>>>> [] (irq_exit) from [] (__handle_domain_irq= +0x90/0xfc) >>>>>>>> [] (__handle_domain_irq) from [] (gic_hand= le_irq+0x64/0xa8) >>>>>>>> [] (gic_handle_irq) from [] (__irq_svc+0x5= 4/0x90) >>>>>>>> Exception stack(0xc1101ef8 to 0xc1101f40) >>>>>>>> 1ee0: 00= 000001 00000000 >>>>>>>> 1f00: 00000000 c011b600 c1100000 c110753c 00000000 c11c3984 c1= 1074d4 c1107548 >>>>>>>> 1f20: 00000000 c1101f54 c1101f58 c1101f48 c010a1fc c010a200 60= 000013 ffffffff >>>>>>>> [] (__irq_svc) from [] (arch_cpu_idle+0x48= /0x4c) >>>>>>>> [] (arch_cpu_idle) from [] (default_idle_c= all+0x30/0x3c) >>>>>>>> [] (default_idle_call) from [] (cpu_startu= p_entry+0x358/0x3b4) >>>>>>>> [] (cpu_startup_entry) from [] (rest_init+= 0x94/0x98) >>>>>>>> [] (rest_init) from [] (start_kernel+0x3a4= /0x3b0) >>>>>>>> [] (start_kernel) from [<4000807c>] (0x4000807c) >>>>>>>> ---[ end trace 256f83eed365daf0 ]--- >>>>>>>> >>>>>>>> The warning occurs because after complete callback function, >>>>>>>> dw_mci_dmac_complete_dma() is called, then dw_mci_stop_dma() i= s called >>>>>>>> again. So it causes dma_unmap_sg() is called twice for same sg= =2E It >>>>>>>> occurs during clock setting at booting time. >>>>>>>> >>>>>>>> Simply, clearing host->using_dma flag on dw_mci_dmac_complete_= dma() and >>>>>>>> dw_mci_stop_dma() like following fixes the issue, but I am not= sure >>>>>>>> this approach is proper. >>>>>>>> --- >>>>>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_m= mc.c >>>>>>>> index 2cc6123..a71c94b 100644 >>>>>>>> --- a/drivers/mmc/host/dw_mmc.c >>>>>>>> +++ b/drivers/mmc/host/dw_mmc.c >>>>>>>> @@ -388,6 +388,7 @@ static void dw_mci_stop_dma(struct dw_mci = *host) >>>>>>>> if (host->using_dma) { >>>>>>>> host->dma_ops->stop(host); >>>>>>>> host->dma_ops->cleanup(host); >>>>>>>> + host->using_dma =3D 0; >>>>>>>> } >>>>>>>> >>>>>>>> /* Data transfer was stopped by the interrupt handler */ >>>>>>>> @@ -455,6 +456,7 @@ static void dw_mci_dmac_complete_dma(void = *arg) >>>>>>>> DMA_FROM_DEVICE); >>>>>>>> >>>>>>>> host->dma_ops->cleanup(host); >>>>>>>> + host->using_dma =3D 0; >>>>>>>> >>>>>>>> /* >>>>>>>> * If the card was removed, data will be NULL. No point i= n trying to >>>>>>>> @@ -943,8 +945,6 @@ static int dw_mci_submit_data_dma(struct d= w_mci *host, struct mmc_data *data) >>>>>>>> int sg_len; >>>>>>>> u32 temp; >>>>>>>> >>>>>>>> - host->using_dma =3D 0; >>>>>>>> - >>>>>>>> /* If we don't have a channel, we can't do DMA */ >>>>>>>> if (!host->use_dma) >>>>>>>> return -ENODEV; >>>>>>>> --- >>>>>>>> >>>>>>>> Best Regards, >>>>>>>> - Seung-Woo Kim >>>>>>>> >>>>>>>> --=20 >>>>>>>> To unsubscribe from this list: send the line "unsubscribe linu= x-mmc" in >>>>>>>> the body of a message to majordomo@vger.kernel.org >>>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.= html >>>>>>>> >>>>>>> >>>>>>> >>>>>>> --=20 >>>>>>> Best Regards >>>>>>> Shawn Lin >>>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>> >>>> >>>> >>>> >>> >>> >> >> >> >> >=20 >=20