From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759417AbcJZCfX (ORCPT ); Tue, 25 Oct 2016 22:35:23 -0400 Received: from bh-25.webhostbox.net ([208.91.199.152]:40098 "EHLO bh-25.webhostbox.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750749AbcJZCfQ (ORCPT ); Tue, 25 Oct 2016 22:35:16 -0400 Subject: Re: [PATCH] ARM: imx6: Fix GPC probe error path To: Shawn Guo References: <1477416878-30353-1-git-send-email-linux@roeck-us.net> Cc: Sascha Hauer , Fabio Estevam , Russell King , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Philipp Zabel , Arnd Bergmann , Jon Hunter , Ulf Hansson , "Rafael J. Wysocki" From: Guenter Roeck Message-ID: <39c26009-8e50-d182-dbfd-49f44cb9402a@roeck-us.net> Date: Tue, 25 Oct 2016 19:35:11 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <1477416878-30353-1-git-send-email-linux@roeck-us.net> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Authenticated_sender: linux@roeck-us.net X-OutGoing-Spam-Status: No, score=-1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - bh-25.webhostbox.net X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - roeck-us.net X-Get-Message-Sender-Via: bh-25.webhostbox.net: authenticated_id: linux@roeck-us.net X-Authenticated-Sender: bh-25.webhostbox.net: linux@roeck-us.net X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 10/25/2016 10:34 AM, Guenter Roeck wrote: > GPC may fail to instantiate with > > imx-gpc: probe of 20dc000.gpc failed with error -22 > > which is returned from of_genpd_add_provider_onecell(). The error path > does not call pm_genpd_remove(). This results in the following crash > later on. > > Unhandled fault: page domain fault (0x01b) at 0x00000040 > pgd = c0204000 > [00000040] *pgd=00000000 > Internal error: : 1b [#1] SMP ARM > Modules linked in: > CPU: 0 PID: 108 Comm: kworker/0:3 Not tainted 4.9.0-rc2 #8 > Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) > Workqueue: pm genpd_power_off_work_fn > task: c759ea00 task.stack: c766a000 > PC is at mutex_lock+0xc/0x4c > LR is at regulator_disable+0x28/0x64 > ... > [] (mutex_lock) from [] (regulator_disable+0x28/0x64) > [] (regulator_disable) from [] (imx6q_pm_pu_power_off+0x90/0x98) > [] (imx6q_pm_pu_power_off) from [] (genpd_poweroff+0x114/0x1d4) > [] (genpd_poweroff) from [] (genpd_power_off_work_fn+0x20/0x2c) > [] (genpd_power_off_work_fn) from [] (process_one_work+0x138/0x34c) > [] (process_one_work) from [] (worker_thread+0x38/0x510) > [] (worker_thread) from [] (kthread+0xdc/0xf4) > [] (kthread) from [] (ret_from_fork+0x14/0x3c) > > This is seen with multi_v7_defconfig and imx6dl-sabrelite.dtb running in > qemu (v2.7 patched to fix a qemu related problem). The error return from > of_genpd_add_provider_onecell() is not seen in v4.8 and may be caused by > a devicetree change (this is a wild guess only), but that is a different > problem. > > Fixes: 00eb60a8b4f7 ("ARM: imx6: gpc: Add PU power domain for GPU/VPU") > Cc: Philipp Zabel > Cc: Arnd Bergmann > Signed-off-by: Guenter Roeck > --- > Several bisect attempts trying to track down "imx-gpc: probe ... failed > with error -22" point to commit 00e729c93395 ("Merge tag 'armsoc-dt' of > git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc"). I have not > been able to track down the real culprit. Part of the problem is that > CONFIG_REGULATOR_ANATOP must be enabled for the problem to be seen, and > CONFIG_ARCH_AT91 causes compile errors for some sequence of commits between > v4.8 and v4.9-rc1. But even after taking this into account, the bisect > results always point to 00e729c93395. If anyone has an idea how to track > down that problem, or what might be causing it, please let me know. > Looking into this some more, it turns out that of_genpd_add_provider_onecell() now returns an error if one of the provided power domains does not exist. In this case, the "ARM" power domain does not exist. I don't see where it is created, so it may well be that this now fails for all imx6 boards with multi_v7_defconfig. Looking into kernelci.org test results, this is confirmed for at least imx6dl-riotboard. Overall I think it is quite safe to assume that all imx6 boards crash with mainline kernels and multi_v7_defconfig. The change can be tracked down to commit 0159ec67076 ("PM / Domains: Verify the PM domain is present when adding a provider"). Adding everyone in the commit log for feedback. Guenter > arch/arm/mach-imx/gpc.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c > index 0df062d8b2c9..f3f40045b4c9 100644 > --- a/arch/arm/mach-imx/gpc.c > +++ b/arch/arm/mach-imx/gpc.c > @@ -409,6 +409,7 @@ static int imx_gpc_genpd_init(struct device *dev, struct regulator *pu_reg) > { > struct clk *clk; > int i; > + int ret; > > imx6q_pu_domain.reg = pu_reg; > > @@ -431,9 +432,14 @@ static int imx_gpc_genpd_init(struct device *dev, struct regulator *pu_reg) > return 0; > > pm_genpd_init(&imx6q_pu_domain.base, NULL, false); > - return of_genpd_add_provider_onecell(dev->of_node, > - &imx_gpc_onecell_data); > + ret = of_genpd_add_provider_onecell(dev->of_node, > + &imx_gpc_onecell_data); > + if (ret) > + goto genpd_remove; > + return 0; > > +genpd_remove: > + pm_genpd_remove(&imx6q_pu_domain.base); > clk_err: > while (i--) > clk_put(imx6q_pu_domain.clk[i]); >