From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751427AbcFXHIb (ORCPT ); Fri, 24 Jun 2016 03:08:31 -0400 Received: from lucky1.263xmail.com ([211.157.147.134]:41574 "EHLO lucky1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751082AbcFXHI3 (ORCPT ); Fri, 24 Jun 2016 03:08:29 -0400 X-263anti-spam: bpcheck:1;KSV:0; X-MAIL-GRAY: 1 X-MAIL-DELIVERY: 0 X-KSVirus-check: 0 X-ADDR-CHECKED: 0 X-RL-SENDER: shawn.lin@rock-chips.com X-FST-TO: shawn.lin@rock-chips.com X-SENDER-IP: 58.22.7.114 X-LOGIN-NAME: shawn.lin@rock-chips.com X-UNIQUE-TAG: X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Subject: Re: mmc: dw_mmc: warning with CONFIG_DMA_API_DEBUG To: Jaehoon Chung , Shawn Lin , Seung-Woo Kim , ulf.hansson@linaro.org, linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org References: <001501d1cace$9b8f2b60$d2ad8220$@samsung.com> <000801d1cb64$070dfb60$1529f220$@samsung.com> <5768D188.3080508@samsung.com> <576BCA56.90206@samsung.com> <576CB765.1060709@samsung.com> Cc: shawn.lin@rock-chips.com From: Shawn Lin Message-ID: Date: Fri, 24 Jun 2016 15:08:18 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <576CB765.1060709@samsung.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 在 2016/6/24 12:30, Jaehoon Chung 写道: > On 06/24/2016 10:25 AM, Shawn Lin wrote: >> Hi Jaehoon, >> >> On 2016/6/23 19:39, Jaehoon Chung wrote: >>> Hi Shawn, >>> >>> On 06/21/2016 04:39 PM, Shawn Lin wrote: >>>> 在 2016/6/21 13:32, Jaehoon Chung 写道: >>>>> 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.org; 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+0x7bc/0xb38 >>>>>>>>> dwmmc_exynos 12200000.mmc: DMA-API: device driver tries to free DMA memory it has not allocated [device >>>>>>>> address=0x000000006d9d2200] >>>>>>>> >>>>>>>> Thanks for this report and fix. >>>>>>>> >>>>>>>> DTO(the same as IDMAC-RI/TI) interrupts may or may not come together >>>>>>>> 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 but 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 cleared 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 tasklet >>>> but we may still fallback to the IDMAC interrupt case as the tasklet >>>> may come up a little late, namely right after the IDMAC interrupt checking. >>>> >>>> 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 Data 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. >> >> Which solution? This $SUBJECT or the one I sent? > > Yours. :) okay I will resend this pacth again for you to pick up. > >> >> >> >>> And then consider the HS200 tuning block with the below patch. >>> >>> https://patchwork.kernel.org/patch/8935791/ >> >> 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. :) >> >>> >>> 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 environment. >>>>>> >>>>>> 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 application >>>> 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_stack+0x74/0x94) >>>>> [ 2.469949] [] (dump_stack) from [] (__warn+0xd4/0x100) >>>>> [ 2.469961] [] (__warn) from [] (warn_slowpath_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 [] (dw_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 [] (vprintk_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 [] (hub_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 [] (kthread+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 irq, 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 = mci_readl(host, IDSTS); >>>>>>>> @@ -2482,7 +2483,8 @@ static irqreturn_t dw_mci_interrupt(int irq, void >>>>>>>> *dev_id) >>>>>>>> mci_writel(host, IDSTS, SDMMC_IDMAC_INT_TI | >>>>>>>> >>>>>>>> SDMMC_IDMAC_INT_RI); >>>>>>>> mci_writel(host, IDSTS, 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); >>>>>>>> } >>>>>>>> } >>>>>>>> >>>>>>>> >>>>>>>>> [size=128 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/0x94) >>>>>>>>> [] (dump_stack) from [] (__warn+0xf8/0x110) >>>>>>>>> [] (__warn) from [] (warn_slowpath_fmt+0x48/0x50) >>>>>>>>> [] (warn_slowpath_fmt) from [] (check_unmap+0x7bc/0xb38) >>>>>>>>> [] (check_unmap) from [] (debug_dma_unmap_sg+0x118/0x148) >>>>>>>>> [] (debug_dma_unmap_sg) from [] (dw_mci_dma_cleanup+0x7c/0xb8) >>>>>>>>> [] (dw_mci_dma_cleanup) from [] (dw_mci_stop_dma+0x40/0x50) >>>>>>>>> [] (dw_mci_stop_dma) from [] (dw_mci_tasklet_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/0x10c) >>>>>>>>> [] (irq_exit) from [] (__handle_domain_irq+0x90/0xfc) >>>>>>>>> [] (__handle_domain_irq) from [] (gic_handle_irq+0x64/0xa8) >>>>>>>>> [] (gic_handle_irq) from [] (__irq_svc+0x54/0x90) >>>>>>>>> Exception stack(0xc1101ef8 to 0xc1101f40) >>>>>>>>> 1ee0: 00000001 00000000 >>>>>>>>> 1f00: 00000000 c011b600 c1100000 c110753c 00000000 c11c3984 c11074d4 c1107548 >>>>>>>>> 1f20: 00000000 c1101f54 c1101f58 c1101f48 c010a1fc c010a200 60000013 ffffffff >>>>>>>>> [] (__irq_svc) from [] (arch_cpu_idle+0x48/0x4c) >>>>>>>>> [] (arch_cpu_idle) from [] (default_idle_call+0x30/0x3c) >>>>>>>>> [] (default_idle_call) from [] (cpu_startup_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() is called >>>>>>>>> again. So it causes dma_unmap_sg() is called twice for same sg. 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_mmc.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 = 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 = 0; >>>>>>>>> >>>>>>>>> /* >>>>>>>>> * If the card was removed, data will be NULL. No point in trying to >>>>>>>>> @@ -943,8 +945,6 @@ static int dw_mci_submit_data_dma(struct dw_mci *host, struct mmc_data *data) >>>>>>>>> int sg_len; >>>>>>>>> u32 temp; >>>>>>>>> >>>>>>>>> - host->using_dma = 0; >>>>>>>>> - >>>>>>>>> /* If we don't have a channel, we can't do DMA */ >>>>>>>>> if (!host->use_dma) >>>>>>>>> return -ENODEV; >>>>>>>>> --- >>>>>>>>> >>>>>>>>> Best Regards, >>>>>>>>> - Seung-Woo Kim >>>>>>>>> >>>>>>>>> -- >>>>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in >>>>>>>>> the body of a message to majordomo@vger.kernel.org >>>>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Best Regards >>>>>>>> Shawn Lin >>>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>>> >>>>> >>>> >>>> >>> >>> >>> >>> >> >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Best Regards Shawn Lin