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 043CFC433EF for ; Tue, 12 Jul 2022 12:54:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233335AbiGLMyT (ORCPT ); Tue, 12 Jul 2022 08:54:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41894 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233321AbiGLMyS (ORCPT ); Tue, 12 Jul 2022 08:54:18 -0400 Received: from madras.collabora.co.uk (madras.collabora.co.uk [46.235.227.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B3E056F7D6; Tue, 12 Jul 2022 05:54:16 -0700 (PDT) Received: from [192.168.1.100] (2-237-20-237.ip236.fastwebnet.it [2.237.20.237]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: kholk11) by madras.collabora.co.uk (Postfix) with ESMTPSA id 0E1AD6601A1F; Tue, 12 Jul 2022 13:54:14 +0100 (BST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1657630455; bh=1jh1BmtPv4h6P239R7DPwcXJAmN7HEZvgqe22MrYjT4=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=J3drevdajexd3AegCVExE5ZNIDtbNLqcPWGf9K74QLiYBGY9DuWDjTnMyae5WqajT ItbR5/MXBaaRKs1gC7E0WjV7J7P4x4fdwOfJWpcY+ajanvnwAwwuCPPHcC5t8vs264 AOfG4VN5x5+5SDyrEQYo0OEM8OO9aIXmygD2QnXGcPhcxhJiPwklY0ZZ595YS4mRTN BT0CAa9mjeGGSOZAXnCIZT0DRjirG+m0g9189IIEx+FUVoTnFUlCS+VsTAENkuZHdV FaZH+xg+szUsazBO2GbjCfWkLeKn2p89tmQP9aw6rKmLvVQu0a2Bpf0rVjFi8JpTIq HTRfppwWiXPcg== Message-ID: <0ca0e46d-0685-8228-4d26-c6cf20d7a9fc@collabora.com> Date: Tue, 12 Jul 2022 14:54:11 +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: Krzysztof Kozlowski , 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> From: AngeloGioacchino Del Regno In-Reply-To: <410cf9aa-471b-644c-9540-9bc0b89b8fd9@linaro.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 own vision is that either using syscon (as shown in the series that you've >> checked), keeping "simple-mfd", or changing it to "simple-bus" (whatever) is >> the cleanest (and best approach) - please otherwise explain why having such > > Again, simple-mfd is just MFD, not a power domain provider. > > simple-bus should not have it's own address space, so combining it with > syscon is rather wrong. > > https://lore.kernel.org/linux-devicetree/Ynq52E93mcTXcw9H@robh.at.kernel.org/ > >> a practically forever-stub driver (practically, a copy of simple-pm-bus.c) >> would be beneficial in any way. >> > > Of course not, but your DTS is saying it is not a stub. > > Best regards, > Krzysztof 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 637A7CCA47C for ; Tue, 12 Jul 2022 12:55:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=TwX5MZ376GufEw28BDIOLmAljtH3cbKGrUX9IqP25nY=; b=HdaT4nzxOH4yuO Lc4dkLf3VhZmOL17VhhveVFj7fIc5ZyIX8aEYhBoCBoKtWfmdx/XXRWLVL/X5mEXswLX6ohHIjWgz 6VZWdCTut9i9RUlt/3usd8F5M/2rM4uge1oZ0Ek7eHgq/Bi0fx21Kgluz+4a77dUqbCQDqFO4LpcX VR/Ij0kVwzSJa5d4H0a6YKWN1ega4SKLJJLdexuWHbP8Xc1TRfcRJAKacG2/SysUNofGAtz/A4b5L kTDNYEdEIXDEu4on3CtA2oMXBh101EExpNLHp+22NbLgWCbpDHRXtZNpfySPv1QbTBhu0dEH2ZUQs sO2lxY1+NjRarVA0/IPg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oBFPE-00B9RK-B5; Tue, 12 Jul 2022 12:54:24 +0000 Received: from madras.collabora.co.uk ([46.235.227.172]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oBFP8-00B9La-RI; Tue, 12 Jul 2022 12:54:22 +0000 Received: from [192.168.1.100] (2-237-20-237.ip236.fastwebnet.it [2.237.20.237]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: kholk11) by madras.collabora.co.uk (Postfix) with ESMTPSA id 0E1AD6601A1F; Tue, 12 Jul 2022 13:54:14 +0100 (BST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1657630455; bh=1jh1BmtPv4h6P239R7DPwcXJAmN7HEZvgqe22MrYjT4=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=J3drevdajexd3AegCVExE5ZNIDtbNLqcPWGf9K74QLiYBGY9DuWDjTnMyae5WqajT ItbR5/MXBaaRKs1gC7E0WjV7J7P4x4fdwOfJWpcY+ajanvnwAwwuCPPHcC5t8vs264 AOfG4VN5x5+5SDyrEQYo0OEM8OO9aIXmygD2QnXGcPhcxhJiPwklY0ZZ595YS4mRTN BT0CAa9mjeGGSOZAXnCIZT0DRjirG+m0g9189IIEx+FUVoTnFUlCS+VsTAENkuZHdV FaZH+xg+szUsazBO2GbjCfWkLeKn2p89tmQP9aw6rKmLvVQu0a2Bpf0rVjFi8JpTIq HTRfppwWiXPcg== Message-ID: <0ca0e46d-0685-8228-4d26-c6cf20d7a9fc@collabora.com> Date: Tue, 12 Jul 2022 14:54:11 +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: Krzysztof Kozlowski , 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> From: AngeloGioacchino Del Regno In-Reply-To: <410cf9aa-471b-644c-9540-9bc0b89b8fd9@linaro.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220712_055419_271395_A113A66B X-CRM114-Status: GOOD ( 47.32 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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 own vision is that either using syscon (as shown in the series that you've >> checked), keeping "simple-mfd", or changing it to "simple-bus" (whatever) is >> the cleanest (and best approach) - please otherwise explain why having such > > Again, simple-mfd is just MFD, not a power domain provider. > > simple-bus should not have it's own address space, so combining it with > syscon is rather wrong. > > https://lore.kernel.org/linux-devicetree/Ynq52E93mcTXcw9H@robh.at.kernel.org/ > >> a practically forever-stub driver (practically, a copy of simple-pm-bus.c) >> would be beneficial in any way. >> > > Of course not, but your DTS is saying it is not a stub. > > Best regards, > Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel