From patchwork Wed Oct 26 14:54:53 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthias Brugger X-Patchwork-Id: 9397405 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 18E4D60234 for ; Wed, 26 Oct 2016 14:55:40 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E9C3C29BE8 for ; Wed, 26 Oct 2016 14:55:39 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DC1F529BF0; Wed, 26 Oct 2016 14:55:39 +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=-4.1 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,FREEMAIL_FROM,RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id B699429BE8 for ; Wed, 26 Oct 2016 14:55:38 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bzPcA-00049q-0y; Wed, 26 Oct 2016 14:55:38 +0000 Received: from mail-wm0-x244.google.com ([2a00:1450:400c:c09::244]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1bzPbq-0002pe-VK; Wed, 26 Oct 2016 14:55:22 +0000 Received: by mail-wm0-x244.google.com with SMTP id z194so1940898wmd.5; Wed, 26 Oct 2016 07:54:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=xOYzF7Aa8/K1+vVUWsxmHkyXXsmX/wE2znF4vyvybxI=; b=zJkjJRpdbzdkY4mgAk/Ql+uH5i6YwyXMj8v12sijAcb4bUKIixaq11wiVKOCO+oPiE xD7e6uexDnrY8hdO+6tDK2e3QwNQWreQGWSg1rvhsXAEfD+W/A0mk7lujZ7kkSy6u+49 ee6dz6CuXjtjiEPxoRcDuI4n2AIsXMKj6Hvubv4UbQLDSnDtc35arwofUM+jOV0e4sz7 /SRjtWXKRuluEbG6wZloaAilwNKcACF07RsMmAqa3u7JhFl25UhOjxIM8bFcBh1P5nCJ dsTlb60knDU5syY0VHB1ePQUporR//lPgRp5KuF6kNJkbRP/e21j08ZMvhatUmA6H24d LR/w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=xOYzF7Aa8/K1+vVUWsxmHkyXXsmX/wE2znF4vyvybxI=; b=OLkDqVIiWANcXqkdwKwRu0+Jvv+UOkHuaJfeLtkeI3EaIu+1tGkLUrt1qKdw3tbSGT PfCTOhjsdRSHRxTagTUGgho3syD6vxJF9Gk9JOEczmJs+sYjhzRRnpO2fjjTtKyfZOOm 4hU2bYkuSk0ELSuB6i7zDv1hig6/0K65y70Qtj5GWw7cxRzwYpeSreoTIT5P+MdHJRpV 6esvEKAX0UNYvWerog2VTKzED6m0JKWHPD3cSPG4MAL3SBkMYhqX5z7p6T1K2njLP7sn 1uT8jfJwik5NJD9c2Fsik2AvT68LvixxQhcqwGUAscjBDZeGhykxXPZ7KykhYjUCmvuX joJg== X-Gm-Message-State: ABUngvcK5wiWnU5+s8IAxemQbU5/gZyBCp/Ztvmo/woBVG3xW1y6sjI7XFfN59wfXm3o0A== X-Received: by 10.28.8.11 with SMTP id 11mr3413624wmi.27.1477493696344; Wed, 26 Oct 2016 07:54:56 -0700 (PDT) Received: from [172.22.222.124] ([188.85.231.155]) by smtp.gmail.com with ESMTPSA id l130sm10017614wmb.18.2016.10.26.07.54.54 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 26 Oct 2016 07:54:55 -0700 (PDT) Subject: Re: [PATCH v7 2/4] soc: mediatek: Init MT8173 scpsys driver earlier To: Yong Wu References: <1463390894-32062-1-git-send-email-jamesjj.liao@mediatek.com> <1463390894-32062-3-git-send-email-jamesjj.liao@mediatek.com> <34025ec4-19d3-8b25-d669-50c6f19159cd@gmail.com> <1467782540.26485.11.camel@mtksdaap41> <577FA0D9.8070408@gmail.com> <1468314071.24705.4.camel@mhfsdcap03> From: Matthias Brugger Message-ID: <42e4a2ad-4349-6d64-b170-c31963d0416f@gmail.com> Date: Wed, 26 Oct 2016 16:54:53 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1468314071.24705.4.camel@mhfsdcap03> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20161026_075519_207003_9C5B7D20 X-CRM114-Status: GOOD ( 40.04 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: James Liao , srv_heupstream@mediatek.com, devicetree@vger.kernel.org, joro@8bytes.org, Kevin Hilman , linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, linux-mediatek@lists.infradead.org, Sascha Hauer , Rob Herring , linux-arm-kernel@lists.infradead.org Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+patchwork-linux-mediatek=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Yong, On 07/12/2016 11:01 AM, Yong Wu wrote: > Hi Matthias, > > On Fri, 2016-07-08 at 14:47 +0200, Matthias Brugger wrote: >> >> On 06/07/16 07:22, James Liao wrote: >>> On Sat, 2016-07-02 at 18:35 +0200, Matthias Brugger wrote: >>>> >>>> On 05/16/2016 11:28 AM, James Liao wrote: >>>>> Some power domain comsumers may init before module_init. >>>>> So the power domain provider (scpsys) need to be initialized >>>>> earlier too. >>>>> >>>>> Take an example for our IOMMU (M4U) and SMI. SMI is a bridge >>>>> between IOMMU and multimedia HW. SMI is responsible to >>>>> enable/disable iommu and help transfer data for each multimedia >>>>> HW. Both of them have to wait until the power and clocks are >>>>> enabled. >>>>> >>>>> So scpsys driver should be initialized before SMI, and SMI should >>>>> be initialized before IOMMU, and then init IOMMU consumers >>>>> (display/vdec/venc/camera etc.). >>>>> >>>>> IOMMU is subsys_init by default. So we need to init scpsys driver >>>>> before subsys_init. >>>>> >>>>> Signed-off-by: James Liao >>>>> Reviewed-by: Kevin Hilman >>>>> --- >>>>> drivers/soc/mediatek/mtk-scpsys.c | 19 ++++++++++++++++++- >>>>> 1 file changed, 18 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c >>>>> index 5870a24..00c0adb 100644 >>>>> --- a/drivers/soc/mediatek/mtk-scpsys.c >>>>> +++ b/drivers/soc/mediatek/mtk-scpsys.c >>>>> @@ -617,4 +617,21 @@ static struct platform_driver scpsys_drv = { >>>>> .of_match_table = of_match_ptr(of_scpsys_match_tbl), >>>>> }, >>>>> }; >>>>> -builtin_platform_driver(scpsys_drv); >>>>> + >>>>> +static int __init scpsys_drv_init(void) >>>>> +{ >>>>> + return platform_driver_register(&scpsys_drv); >>>>> +} >>>>> + >>>>> +/* >>>>> + * There are some Mediatek drivers which depend on the power domain driver need >>>>> + * to probe in earlier initcall levels. So scpsys driver also need to probe >>>>> + * earlier. >>>>> + * >>>>> + * IOMMU(M4U) and SMI drivers for example. SMI is a bridge between IOMMU and >>>>> + * multimedia HW. IOMMU depends on SMI, and SMI is a power domain consumer, >>>>> + * so the proper probe sequence should be scpsys -> SMI -> IOMMU driver. >>>>> + * IOMMU drivers are initialized during subsys_init by default, so we need to >>>>> + * move SMI and scpsys drivers to subsys_init or earlier init levels. >>>>> + */ >>>>> +subsys_initcall(scpsys_drv_init); >>>>> >>>> >>>> Can't we achieve this with probe deferring? I'm not really keen on >>>> coding the order of the different drivers like this. >>> >>> Hi Matthias, >>> >>> Some drivers such as IOMMU don't support probe deferring. So scpsys need >>> to init before them by changing init level. >>> >> >> SMI larbs return EPROBE_DEFER if it can't find the PM provider for it's >> domain, so this part should not be the problem. The larbs get added as >> components in the probe, when the PM provider is present. >> >> The iommu driver uses the larbs as components. As long as not all >> components are present the iommu does not bind them. So from what I see >> this should work without any problem. >> >> Can you please specify where the iommu dirver has a problem. Maybe we >> need to fix the driver rather then scpsys. > > We got a problem while bootup, the hang backtrace is like below(Sorry, I > can't find the full backtrace now): > > [ 7.832359] Call trace: > [ 7.834778] [] mtk_smi_larb_get+0x24/0xa8 > [ 7.840300] [] mtk_drm_crtc_enable+0x6c/0x450 > > The reason is that "larb->mmu" is NULL while DRM call mtk_smi_larb_get. > > DRM call iommu_present to wait for whether IOMMU is ready[1]. > But in the MTK-IOMMU, It will call > bus_set_iommu->mtk_iommu_add_device->mtk_iommu_attach_device, then it's > able to transfer "struct mtk_smi_iommu" to SMI whose probe maybe delayed > by power-domain, then SMI could finish its probe. > > So If DRM probe is called after the time of calling bus_set_iommu and > before the time of SMI probe finish, this problem can be reproduced. > > The root cause is that iommu_present cann't indicate that MTK-IOMMU and > SMI is ready actually. in other words, the DRM cann't detect > whether the SMI has probed done. > > If we move scpsys_init from module_initcall to subsys_initcall, the > IOMMU and SMI could be probe done during the subsys_initcall period. > this issue can be avoid. > And as we grep before, there are also some examples whose power-domain > is also not module_init[2]. > core_initcall(exynos4_pm_init_power_domain); > subsys_initcall(imx_pgc_init); > > If you think that this patch will hide the problem above, then I have to > add a new function, like below: > //================== > bool mtk_smi_larb_ready(struct device *larbdev) > { > struct mtk_smi_larb *larb = dev_get_drvdata(larbdev); > > return larb && !!larb->mmu; > } > //================= > > And in the probe of all the MTK-IOMMU consumer, we have to add this: > //==================== > if (!mtk_smi_larb_ready(&larb_pdev->dev)) > return -EPROBE_DEFER; > //==================== > > The sequence is a little complex, If I don't explain clearly, please > tell me. > Any other suggestion about this? Thanks. I'm still trying to find a different way to solve this. Unfortunately I'm not able to reproduce this on my mt8173-evb... ...but: iommu_present checkes if the bus->iommu_ops is set. If we set these option in the iommu after all components got initialized, we should be fine. Or did I miss something? Would you mind trying the patch below (against v4.9-rc1)? If you know of any way to reproduce this issue, I would be happy to know. I'm adding Joerg, the iommu maintainer, maybe he has an idea. Thanks a lot, Matthias } static int mtk_iommu_remove(struct platform_device *pdev) diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c index b12c12d..9249011 100644 --- a/drivers/iommu/mtk_iommu.c +++ b/drivers/iommu/mtk_iommu.c @@ -602,10 +602,12 @@ static int mtk_iommu_probe(struct platform_device *pdev) if (ret) return ret; - if (!iommu_present(&platform_bus_type)) + ret = component_master_add_with_match(dev, &mtk_iommu_com_ops, match); + + if (ret && !iommu_present(&platform_bus_type)) bus_set_iommu(&platform_bus_type, &mtk_iommu_ops); - return component_master_add_with_match(dev, &mtk_iommu_com_ops, match); + return ret;