From patchwork Mon Aug 14 08:55:35 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Shawn Lin X-Patchwork-Id: 9898377 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 6A49C6038C for ; Mon, 14 Aug 2017 08:57:15 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5E27E285A3 for ; Mon, 14 Aug 2017 08:57:15 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 52F35285AD; Mon, 14 Aug 2017 08:57:15 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RCVD_IN_SBL autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8AA93285A8 for ; Mon, 14 Aug 2017 08:57:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752381AbdHNI5F (ORCPT ); Mon, 14 Aug 2017 04:57:05 -0400 Received: from lucky1.263xmail.com ([211.157.147.132]:40729 "EHLO lucky1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751552AbdHNI5E (ORCPT ); Mon, 14 Aug 2017 04:57:04 -0400 Received: from shawn.lin?rock-chips.com (unknown [192.168.167.242]) by lucky1.263xmail.com (Postfix) with ESMTP id E647A64581; Mon, 14 Aug 2017 16:56:58 +0800 (CST) X-263anti-spam: KSV:0; X-MAIL-GRAY: 1 X-MAIL-DELIVERY: 0 X-KSVirus-check: 0 X-ABS-CHECKED: 4 Received: from localhost.localdomain (localhost [127.0.0.1]) by smtp.263.net (Postfix) with ESMTPA id 6021B3C4; Mon, 14 Aug 2017 16:56:56 +0800 (CST) X-RL-SENDER: shawn.lin@rock-chips.com X-FST-TO: ulf.hansson@linaro.org X-SENDER-IP: 58.22.7.114 X-LOGIN-NAME: shawn.lin@rock-chips.com X-UNIQUE-TAG: X-ATTACHMENT-NUM: 0 X-SENDER: lintao@rock-chips.com X-DNS-TYPE: 0 Received: from localhost.localdomain (unknown [58.22.7.114]) by smtp.263.net (Postfix) whith ESMTP id 27215USZ6HW; Mon, 14 Aug 2017 16:56:58 +0800 (CST) From: Shawn Lin To: Ulf Hansson , "Rafael J. Wysocki" Cc: Jaehoon Chung , linux-mmc@vger.kernel.org, Shawn Lin Subject: [RFC PATCH] mmc: dw_mmc: fix potential system abort if activating CONFIG_DEBUG_SHIRQ Date: Mon, 14 Aug 2017 16:55:35 +0800 Message-Id: <1502700935-82281-1-git-send-email-shawn.lin@rock-chips.com> X-Mailer: git-send-email 1.9.1 Sender: linux-mmc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-mmc@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP With CONFIG_DEBUG_SHIRQ enabled, the irq tear down routine would still access the irq handler registed as a shard irq. Per the comment within the function of __free_irq, it says "It's a shared IRQ -- the driver ought to be prepared for an IRQ event to happen even now it's being freed". However when failing to probe the driver, dw_mmc disables the clock and asserts the reset pin, even power off its genpd for accessing the registers and the following check for shared irq state would call the irq handler which accesses the register w/o all necessary resouce prepared. That will hang the system forever. With adding some dump_stack we could see how that happened. Synopsys Designware Multimedia Card Interface Driver dwmmc_rockchip fe320000.dwmmc: IDMAC supports 32-bit address mode. dwmmc_rockchip fe320000.dwmmc: Using internal DMA controller. dwmmc_rockchip fe320000.dwmmc: Version ID is 270a CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.13.0-rc3-next-20170807-00004-g93d3644-dirty #5 Hardware name: Firefly-RK3399 Board (DT) Call trace: [] dump_backtrace+0x0/0x300 [] show_stack+0x14/0x20 [] dump_stack+0xa4/0xc8 [] dw_mci_interrupt+0x3c/0x6a8 [] __free_irq+0x308/0x410 [] free_irq+0x54/0xb0 [] devm_irq_release+0x30/0x40 [] release_nodes+0x1e4/0x390 [] devres_release_all+0x50/0x78 [] driver_probe_device+0x128/0x3b8 [] __driver_attach+0xe4/0xe8 [] bus_for_each_dev+0xe0/0x138 [] driver_attach+0x30/0x40 [] bus_add_driver+0x1d0/0x328 [] driver_register+0xb4/0x198 [] __platform_driver_register+0x7c/0x88 [] dw_mci_rockchip_pltfm_driver_init+0x18/0x20 [] do_one_initcall+0x14c/0x1b8 [] kernel_init_freeable+0x238/0x2d8 [] kernel_init+0x10/0x110 [] ret_from_fork+0x10/0x50 Synchronous External Abort: synchronous external abort (0x96000010) at 0xffff20000aaa4040 Internal error: : 96000010 [#1] PREEMPT SMP Modules linked in: CPU: 0 PID: 1 Comm: swapper/0 Not tainted task: ffff80006ba28080 task.stack: ffff80006ba24000 PC is at dw_mci_interrupt+0x4c/0x6a8 LR is at dw_mci_interrupt+0x44/0x6a8 pc : [] lr : [] pstate: 600001c5 ... Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b SMP: stopping secondary CPUs Kernel Offset: disabled CPU features: 0x00200c Memory Limit: none ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b In order to fix this, we remove all the clock-disabling from the error handle path and driver's remove function. And replying on the devm_add_action_or_reset to fire the clock-disabling and reset signal at the appropriate time. *However*!!! However, it still can't stop the device's dismiss callback to fire the genpd_power_off_work_fn, which still would power off the genpd and break the system. More precisely, the genpd power off races with the devres's release as they are totally asynchronous, so sometimes the system is alive due to the race contition. See the following case: rockchip fe320000.dwmmc: IDMAC supports 32-bit address mode. dwmmc_rockchip fe320000.dwmmc: Using internal DMA controller. dwmmc_rockchip fe320000.dwmmc: Version ID is 270a CPU: 0 PID: 39 Comm: kworker/0:1 Not tainted 4.13.0-rc3-next-20170807-00004-g93d3644-dirty #20 dw_mci_interrupt enter //action from free_irq dw_mci_post_cleanup enter //dw_mmc's devm action callback dwmmc_rockchip: probe of fe320000.dwmmc failed with error -12 rockchip_pd_power: power_off //luckly power off genpd here Hardware name: Firefly-RK3399 Board (DT) Workqueue: pm genpd_power_off_work_fn Call trace: [] dump_backtrace+0x0/0x300 sdhci-pltfm: SDHCI platform and OF driver helper [] show_stack+0x14/0x20 [] dump_stack+0xa4/0xc8 [] rockchip_pd_power+0x640/0x648 [] rockchip_pd_power_off+0x10/0x18 [] genpd_power_off+0x19c/0x388 [] genpd_power_off_work_fn+0x4c/0x80 [] process_one_work+0x388/0x5e0 [] worker_thread+0x84/0x680 [] kthread+0x18c/0x1d0 [] ret_from_fork+0x10/0x50 Signed-off-by: Shawn Lin --- Dear Ulf and Rafael, I have poor knowledge about power_domain and haven't find the right solution to fix this problem for genpd part after reading the power domain code. Moreover, I don't think it's dw_mmc specific as the issue I found seems widely exists in all different kinds of drivers. So I would appreciate if you could kindly point me to the right direction. :) drivers/mmc/host/dw_mmc.c | 48 +++++++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index b9640c7..05b5acf 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -3012,6 +3012,18 @@ static void dw_mci_enable_cd(struct dw_mci *host) } } +static void dw_mci_post_cleanup(void *data) +{ + struct dw_mci *host = data; + + clk_disable_unprepare(host->ciu_clk); + clk_disable_unprepare(host->biu_clk); + + if (!IS_ERR(host->pdata->rstc)) + reset_control_assert(host->pdata->rstc); + +} + int dw_mci_probe(struct dw_mci *host) { const struct dw_mci_drv_data *drv_data = host->drv_data; @@ -3047,7 +3059,7 @@ int dw_mci_probe(struct dw_mci *host) ret = clk_prepare_enable(host->ciu_clk); if (ret) { dev_err(host->dev, "failed to enable ciu clock\n"); - goto err_clk_biu; + return ret; } if (host->pdata->bus_hz) { @@ -3060,11 +3072,16 @@ int dw_mci_probe(struct dw_mci *host) host->bus_hz = clk_get_rate(host->ciu_clk); } + ret = devm_add_action_or_reset(host->dev, dw_mci_post_cleanup, host); + if (ret) { + dev_err(host->dev, "unable to add action or reset\n"); + return ret; + } + if (!host->bus_hz) { dev_err(host->dev, "Platform data must supply bus speed\n"); - ret = -ENODEV; - goto err_clk_ciu; + return -ENODEV; } if (drv_data && drv_data->init) { @@ -3072,7 +3089,7 @@ int dw_mci_probe(struct dw_mci *host) if (ret) { dev_err(host->dev, "implementation specific init failed\n"); - goto err_clk_ciu; + return ret; } } @@ -3119,10 +3136,8 @@ int dw_mci_probe(struct dw_mci *host) } /* Reset all blocks */ - if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) { - ret = -ENODEV; - goto err_clk_ciu; - } + if (!dw_mci_ctrl_reset(host, SDMMC_CTRL_ALL_RESET_FLAGS)) + return -ENODEV; host->dma_ops = host->pdata->dma_ops; dw_mci_init_dma(host); @@ -3209,15 +3224,6 @@ int dw_mci_probe(struct dw_mci *host) if (host->use_dma && host->dma_ops->exit) host->dma_ops->exit(host); - if (!IS_ERR(host->pdata->rstc)) - reset_control_assert(host->pdata->rstc); - -err_clk_ciu: - clk_disable_unprepare(host->ciu_clk); - -err_clk_biu: - clk_disable_unprepare(host->biu_clk); - return ret; } EXPORT_SYMBOL(dw_mci_probe); @@ -3237,17 +3243,9 @@ void dw_mci_remove(struct dw_mci *host) if (host->use_dma && host->dma_ops->exit) host->dma_ops->exit(host); - - if (!IS_ERR(host->pdata->rstc)) - reset_control_assert(host->pdata->rstc); - - clk_disable_unprepare(host->ciu_clk); - clk_disable_unprepare(host->biu_clk); } EXPORT_SYMBOL(dw_mci_remove); - - #ifdef CONFIG_PM int dw_mci_runtime_suspend(struct device *dev) {