From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 19FC6C433EF for ; Tue, 12 Jul 2022 12:58:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229683AbiGLM6u (ORCPT ); Tue, 12 Jul 2022 08:58:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45940 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229605AbiGLM6t (ORCPT ); Tue, 12 Jul 2022 08:58:49 -0400 Received: from mail-lj1-x22a.google.com (mail-lj1-x22a.google.com [IPv6:2a00:1450:4864:20::22a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3460DDEB3 for ; Tue, 12 Jul 2022 05:58:47 -0700 (PDT) Received: by mail-lj1-x22a.google.com with SMTP id bx13so9784137ljb.1 for ; Tue, 12 Jul 2022 05:58:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=EGT/5rR75zdMwDOLE513Y0sGyM5McQ5xj8+VzcoomHc=; b=x5Myz/Puv22xNJAXrjPJuxPyw/U2U2hxyuBqBUFFYjHVX1N0FHv7sQnEbXeuKbh8Dc Y4jEovGFbfEASy1txuvK4uExzQEUGP2Pu6YK4ZkkbU7/AH/3km/AGzqkC3GGszhV4HZa 1ijAxPma5gSbh6bcGDsp6rc2rsmkHBmnZyfECrIfMLCmDyeYNLZmZKkVeJv0ux8aam2S OYkQnwB2J2velsBalSoRT7/nggrbKlXyZzETTZ5CYBIsPz/2HCTKaaiSou/9bx63vRZr 6f2Azarr3yz0zR7P9TNSHkB4m7zEQVUDD1EIyQlCzAX7Cs+SNnPc0fchpnNbsUhvV0dJ 3+hw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=EGT/5rR75zdMwDOLE513Y0sGyM5McQ5xj8+VzcoomHc=; b=G71djqI2QI3rTYLVZoH61cxhkzQUYNy5Kev5SFPennLw5R4XLkFU9TZKYHbENq67yR m/UAcnC4HMxBiMg9nGEim/nPpENWQIm4zlEgYlR701LToO6qQ6ZuhDYP8Nzj8j/eRq93 9vKXBN+fzIsd1UqPcxK/63TL/51EAp7GMg85vcEYk85tjwQwKbKxhc0jnqN0NeLElQ7w EWWZOB6D1k3jwjwcyGN4lIaeU4K3hmNUCnXCccjruH4l7RFiWanGwRf6lQVkleF9Hz2L oUJSSK8h36IJSEUlw2j6hCgGWVhKNAWTz6sqHsQBSoLmf+MC1gVldHelIxXXoI+HYOnE f3BA== X-Gm-Message-State: AJIora+e39ceUIqN/pBi7aWrBkphHCBkdQyhsW8NFYhVH0EpVCu9Epuh S+8iThGTsTXzwclcg32mvtCXUA== X-Google-Smtp-Source: AGRyM1tOUFmMvb7s4feimVleVi8I888W6gmqG6Ll2R6vFTMNI4O190e6r4pHr1mrlMHGffiLIwdaCw== X-Received: by 2002:a2e:3016:0:b0:25d:702c:a923 with SMTP id w22-20020a2e3016000000b0025d702ca923mr4473194ljw.288.1657630725347; Tue, 12 Jul 2022 05:58:45 -0700 (PDT) Received: from [10.0.0.8] (fwa5da9-171.bb.online.no. [88.93.169.171]) by smtp.gmail.com with ESMTPSA id q5-20020a056512210500b00489ed49d243sm508636lfr.260.2022.07.12.05.58.43 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 12 Jul 2022 05:58:44 -0700 (PDT) Message-ID: <4795d9a8-3a57-ffe5-c0e5-9877860f5107@linaro.org> Date: Tue, 12 Jul 2022 14:58:36 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH v1 08/16] arm64: dts: mt8195: Add power domains controller Content-Language: en-US To: AngeloGioacchino Del Regno , Tinghan Shen , Yong Wu , Joerg Roedel , Will Deacon , Rob Herring , Krzysztof Kozlowski , Matthias Brugger , Chun-Jie Chen , Weiyi Lu Cc: iommu@lists.linux-foundation.org, linux-mediatek@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Project_Global_Chrome_Upstream_Group@mediatek.com References: <20220704100028.19932-1-tinghan.shen@mediatek.com> <20220704100028.19932-9-tinghan.shen@mediatek.com> <3b65405d-167f-a0c7-d15e-5da6f08d99b3@linaro.org> <0301ebc6-1222-e813-f237-f14ad8444940@linaro.org> <1eb212ea-c5a9-b06f-606f-1271ac52adf9@linaro.org> <83162e4f-a31f-79cf-ba01-58b45fd4f62e@collabora.com> <410cf9aa-471b-644c-9540-9bc0b89b8fd9@linaro.org> <0ca0e46d-0685-8228-4d26-c6cf20d7a9fc@collabora.com> From: Krzysztof Kozlowski In-Reply-To: <0ca0e46d-0685-8228-4d26-c6cf20d7a9fc@collabora.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On 12/07/2022 14:54, AngeloGioacchino Del Regno wrote: > Il 12/07/22 14:47, Krzysztof Kozlowski ha scritto: >> On 12/07/2022 12:33, AngeloGioacchino Del Regno wrote: >>> Il 12/07/22 11:03, Krzysztof Kozlowski ha scritto: >>>> On 12/07/2022 10:53, AngeloGioacchino Del Regno wrote: >>>>> Il 12/07/22 10:37, Krzysztof Kozlowski ha scritto: >>>>>> On 12/07/2022 10:17, AngeloGioacchino Del Regno wrote: >>>>>>> Il 06/07/22 17:18, Krzysztof Kozlowski ha scritto: >>>>>>>> On 06/07/2022 14:00, Tinghan Shen wrote: >>>>>>>>> Hi Krzysztof, >>>>>>>>> >>>>>>>>> After discussing your message with our power team, >>>>>>>>> we realized that we need your help to ensure we fully understand you. >>>>>>>>> >>>>>>>>> On Mon, 2022-07-04 at 14:38 +0200, Krzysztof Kozlowski wrote: >>>>>>>>>> On 04/07/2022 12:00, Tinghan Shen wrote: >>>>>>>>>>> Add power domains controller node for mt8195. >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: Weiyi Lu >>>>>>>>>>> Signed-off-by: Tinghan Shen >>>>>>>>>>> --- >>>>>>>>>>> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 327 +++++++++++++++++++++++ >>>>>>>>>>> 1 file changed, 327 insertions(+) >>>>>>>>>>> >>>>>>>>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi >>>>>>>>>>> index 8d59a7da3271..d52e140d9271 100644 >>>>>>>>>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi >>>>>>>>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi >>>>>>>>>>> @@ -10,6 +10,7 @@ >>>>>>>>>>> #include >>>>>>>>>>> #include >>>>>>>>>>> #include >>>>>>>>>>> +#include >>>>>>>>>>> >>>>>>>>>>> / { >>>>>>>>>>> compatible = "mediatek,mt8195"; >>>>>>>>>>> @@ -338,6 +339,332 @@ >>>>>>>>>>> #interrupt-cells = <2>; >>>>>>>>>>> }; >>>>>>>>>>> >>>>>>>>>>> + scpsys: syscon@10006000 { >>>>>>>>>>> + compatible = "syscon", "simple-mfd"; >>>>>>>>>> >>>>>>>>>> These compatibles cannot be alone. >>>>>>>>> >>>>>>>>> the scpsys sub node has the compatible of the power domain driver. >>>>>>>>> do you suggest that the compatible in the sub node should move to here? >>>>>>>> >>>>>>>> Not necessarily, depends. You have here device node representing system >>>>>>>> registers. They need they own compatibles, just like everywhere in the >>>>>>>> kernel (except the broken cases...). >>>>>>>> >>>>>>>> Whether this should be compatible of power-domain driver, it depends >>>>>>>> what this device node is. I don't know, I don't have your datasheets or >>>>>>>> your architecture diagrams... >>>>>>>> >>>>>>>>> >>>>>>>>>>> + reg = <0 0x10006000 0 0x1000>; >>>>>>>>>>> + #power-domain-cells = <1>; >>>>>>>>>> >>>>>>>>>> If it is simple MFD, then probably it is not a power domain provider. >>>>>>>>>> Decide. >>>>>>>>> >>>>>>>>> this MFD device is the power controller on mt8195. >>>>>>>> >>>>>>>> Then it is not a simple MFD but a power controller. Do not use >>>>>>>> "simple-mfd" compatible. >>>>>>>> >>>>>>>>> Some features need >>>>>>>>> to do some operations on registers in this node. We think that implement >>>>>>>>> the operation of these registers as the MFD device can provide flexibility >>>>>>>>> for future use. We want to clarify if you're saying that an MFD device >>>>>>>>> cannot be a power domain provider. >>>>>>>> >>>>>>>> MFD device is Linuxism, so it has nothing to do here. I am talking only >>>>>>>> about simple-mfd. simple-mfd is a simple device only instantiating >>>>>>>> children and not providing anything to anyone. Neither to children. This >>>>>>>> the most important part. The children do not depend on anything from >>>>>>>> simple-mfd device. For example simple-mfd device can be shut down >>>>>>>> (gated) and children should still operate. Being a power domain >>>>>>>> controller, contradicts this usually. >>>>>>>> >>>>>>> >>>>>>> If my interpretation of this issue is right, I have pushed a solution for it. >>>>>>> Krzysztof, Matthias, can you please check [1] and give feedback, so that >>>>>>> Tinghan can rewrite this commit ASAP? >>>>>>> >>>>>>> Reason is - I need the MT8195 devicetree to be complete to push the remaining >>>>>>> pieces for Tomato Chromebooks, of course. >>>>>>> >>>>>>> [1]: https://patchwork.kernel.org/project/linux-mediatek/list/?series=658527 >>>>>> >>>>>> I have two or three similar discussions, so maybe I lost the context, >>>>>> but I don't understand how your fix is matching real hardware. >>>>>> >>>>>> In the patchset here, Tinghan claimed that power domain controller is a >>>>>> child of 10006000. 10006000 is also a power domain controller. This was >>>>>> explicitly described by the DTS code. >>>>>> >>>>>> Now you abandon this hierarchy in favor of syscon. If the hierarchy was >>>>>> correct, your patchset does not match the hardware, so it's a no-go. >>>>>> Describe the hardware. >>>>>> >>>>>> However maybe this patch did not make any sense and there is no >>>>>> relationship parent-child... so what do you guys send here? Bunch of >>>>>> hacks and work-arounds? >>>>>> >>>>> >>>>> For how I get it, hardware side, the SPM (System Power Manager) resides inside >>>>> of the SCPSYS block (consequently, in that iospace). >>>>> >>>>> As Matthias pointed out earlier, SCPSYS provides more functionality, but the >>>>> only one that's currently implemented upstream is the System Power Manager, >>>>> responsible for managing the MTCMOS (power domains). >>>>> >>>>> In any case, now I'm a little confused on how to proceed, can you please give >>>>> some suggestion? >>>> >>>> You should make SCPSYS (0x10006000, AFAIU) a proper driver with its own >>>> compatible (followed by syscon if needed), even if now it is almost >>>> empty stub. The driver should populate OF children and then you can >>>> embed SPM inside and reference to parent's regmap. No need for >>>> simple-mfd. Later the SCPSYS can grow, if you ever need it. >>>> >>>> >>> >>> I see an issue with such approach: the SCPSYS doesn't have a mailbox, doesn't >>> need power management from the Linux side, doesn't have any register to check >>> HW revision, it's always online (hence it doesn't need Linux to boot it), it >>> doesn't need any root clock, nor regulator, nor mmu context, and there's no >>> retrievable "boot log" of any sort. >> >> No problems, there are other drivers who do not need any resources, >> except address space. >> >>> >>> Hence, a driver with its own compatible would be an empty stub forever: it's >>> not going to get any "scpsys root handling" at all, because there's none to do. >> >> But it is a power domain provider, so you need to embed it in some >> dirver, don't you? >> >> >>> Digging through some downstream kernels, the only other functionality that I >>> can find in the SCPSYS is a MODULE_RESET (which is used to reset the SCP System) >>> and a INFRA_IRQ_SET, used to set "wake locks" on the AP and CONNSYS (modem). >> >> So why was power domain provider added to SCPSYS? Guys, I don't know >> your architecture, so I deduct it based on pieces of DTS code I see. >> >>> >>> Both of these may only be used in the SCP mailbox driver (which is *not* SCPSYS) >>> to perform an ipi_send operation (but currently we simply en/disable the clock >>> and that's enough), or to perform a reset and firmware reload of the SCP (but >>> currently we're simply powering off and back on: this may change in the future). >>> >>> So, at the end of the day, we would end up having a copy of simple-pm-bus and >>> nothing else, which doesn't look like being optimal at all. >> >> No, because you need that power domain driver, don't you? If you don't >> need power domain provider/driver, why the heck this is there: >> >> + scpsys: syscon@10006000 { >> + compatible = "syscon", "simple-mfd"; >> + reg = <0 0x10006000 0 0x1000>; >> + #power-domain-cells = <1>; >> ^^^^^^^^^^^^^^^^^ >> Entire discussion started from this. >> > > Is this all a huge misunderstanding? It probably is, at least partially. > > That node shouldn't have any #power-domain-cells, the only PD is the SPM node > (mediatek,mt8195-power-controller), not the scpsys parent! Ugh... My comment was quite clear I think: > + #power-domain-cells = <1>; If it is simple MFD, then probably it is not a power domain provider. Decide. and you keep telling me that it is a power domain provider and MFD and something more. Anyway neither syscon nor simple-mfd can be without specific compatible. Best regards, Krzysztof