Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <1b5613ad-6d0d-0979-ddd0-4677ade7beb9@linaro.org>
Date:   Mon, 16 Jan 2023 10:14:33 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH 2/2] ARM: dts: imx6qdl: support child mfd cells for the
 reset controller
Content-Language: en-US
To:     Ahmad Fatoum <a.fatoum@pengutronix.de>,
        Bastian Krause <bst@pengutronix.de>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Pengutronix Kernel Team <kernel@pengutronix.de>,
        Fabio Estevam <festevam@gmail.com>,
        NXP Linux Team <linux-imx@nxp.com>
Cc:     devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org
References: <20230113-syscon-child-mfd-v1-0-0dd31b7de373@pengutronix.de>
 <20230113-syscon-child-mfd-v1-2-0dd31b7de373@pengutronix.de>
 <392f6e9d-b7c2-37df-2067-f7d967a20f10@linaro.org>
 <12080bf5-2cc4-e215-555e-5438ed1bd851@pengutronix.de>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <12080bf5-2cc4-e215-555e-5438ed1bd851@pengutronix.de>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 16/01/2023 09:27, Ahmad Fatoum wrote:
> Hello Krzysztof,
> 
> On 16.01.23 09:20, Krzysztof Kozlowski wrote:
>> On 13/01/2023 18:32, Bastian Krause wrote:
>>> The actual syscon-reboot-mode child node can be added by a board
>>> device-tree or fixed up by the bootloader. For the child node to be
>>> probed, the compatible needs to include simple-mfd. The binding now
>>> specifies this, so have the SoC dtsi adhere to it.
>>>
>>> Suggested-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>> Signed-off-by: Bastian Krause <bst@pengutronix.de>
>>> ---
>>>  arch/arm/boot/dts/imx6qdl.dtsi | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
>>> index ff1e0173b39be..b16be39458aa6 100644
>>> --- a/arch/arm/boot/dts/imx6qdl.dtsi
>>> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
>>> @@ -865,7 +865,8 @@ epit2: epit@20d4000 { /* EPIT2 */
>>>  			};
>>>  
>>>  			src: reset-controller@20d8000 {
>>> -				compatible = "fsl,imx6q-src", "fsl,imx51-src";
>>> +				compatible = "fsl,imx6q-src", "fsl,imx51-src",
>>> +					     "syscon", "simple-mfd";
>>
>> You need children here. Otherwise simple-mfd does not make sense. If you
>> expect something else to add children (I don't understand why, usually
>> reboot capability is fixed per SoC and only sometimes extended with some
>> other means), then this "else" will also change compatible.
> 
> It's about syscon-reboot-mode, not syscon-reboot. Such modes are board-
> not soc-specific. 

syscon-reboot-mode is also mostly SoC specific. What exactly would
differ on different boards? Register offsets of SoC component? Register
values used by SoC power management unit?

Anyway, the binding is then not correct - it does not allow
syscon-reboot-mode child. I'll comment there.


Best regards,
Krzysztof