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 DC4A3FA3743 for ; Mon, 31 Oct 2022 14:31:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231596AbiJaObM (ORCPT ); Mon, 31 Oct 2022 10:31:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49132 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231607AbiJaObL (ORCPT ); Mon, 31 Oct 2022 10:31:11 -0400 Received: from fllv0015.ext.ti.com (fllv0015.ext.ti.com [198.47.19.141]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 30A1D6587; Mon, 31 Oct 2022 07:31:09 -0700 (PDT) Received: from fllv0034.itg.ti.com ([10.64.40.246]) by fllv0015.ext.ti.com (8.15.2/8.15.2) with ESMTP id 29VEUkUL113944; Mon, 31 Oct 2022 09:30:46 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1667226646; bh=u2H/VwoOd2lyZ9o3nREa5OAy04W4A1/6vZHoNRcnkpw=; h=Date:From:Subject:To:CC:References:In-Reply-To; b=awrwz8BqEKksG3S91R62JGweD2XF2s1U/DNrmP0UILAr2jbHIkeIbDWnDC4c5PzZi OBWzWVL+IUbdd4cLK+hPbFUU/DSKEPHzldisE2xlqTtbJPz39SEpwnFOw9xgy/eqXG Wos5MihnfRPdZpEQoIDAWdvrYfr30bD0jpVWnsbk= Received: from DLEE104.ent.ti.com (dlee104.ent.ti.com [157.170.170.34]) by fllv0034.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 29VEUkhq120665 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Mon, 31 Oct 2022 09:30:46 -0500 Received: from DLEE102.ent.ti.com (157.170.170.32) by DLEE104.ent.ti.com (157.170.170.34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.6; Mon, 31 Oct 2022 09:30:46 -0500 Received: from fllv0040.itg.ti.com (10.64.41.20) by DLEE102.ent.ti.com (157.170.170.32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.6 via Frontend Transport; Mon, 31 Oct 2022 09:30:46 -0500 Received: from [10.250.35.234] (ileaxei01-snat.itg.ti.com [10.180.69.5]) by fllv0040.itg.ti.com (8.15.2/8.15.2) with ESMTP id 29VEUjHd094160; Mon, 31 Oct 2022 09:30:45 -0500 Message-ID: <0025ec36-0632-b79e-beba-cf838018a704@ti.com> Date: Mon, 31 Oct 2022 09:30:45 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.2 From: Andrew Davis Subject: Re: [PATCH v3 2/9] ARM: dts: nspire: Use syscon-reboot to handle restart To: Krzysztof Kozlowski , Lee Jones , Rob Herring , Krzysztof Kozlowski , Arnd Bergmann , Linus Walleij , Geert Uytterhoeven , Daniel Tang , Fabian Vogt CC: , , References: <20221027181337.8651-1-afd@ti.com> <20221027181337.8651-3-afd@ti.com> <050f3d65-5720-9c97-1930-bc458c4c2fb8@linaro.org> <4236ab07-6ad3-efcd-7d5e-c244581d2944@linaro.org> Content-Language: en-US In-Reply-To: <4236ab07-6ad3-efcd-7d5e-c244581d2944@linaro.org> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On 10/27/22 4:27 PM, Krzysztof Kozlowski wrote: > On 27/10/2022 17:07, Andrew Davis wrote: >> On 10/27/22 2:33 PM, Krzysztof Kozlowski wrote: >>> On 27/10/2022 14:13, Andrew Davis wrote: >>>> Writing this bit can be handled by the syscon-reboot driver. >>>> Add this node to DT. >>>> >>>> Signed-off-by: Andrew Davis >>>> Reviewed-by: Linus Walleij >>>> Tested-by: Fabian Vogt >>>> Reviewed-by: Fabian Vogt >>>> --- >>>> arch/arm/boot/dts/nspire.dtsi | 7 +++++++ >>>> 1 file changed, 7 insertions(+) >>>> >>>> diff --git a/arch/arm/boot/dts/nspire.dtsi b/arch/arm/boot/dts/nspire.dtsi >>>> index bb240e6a3a6f..48fbc9d533c3 100644 >>>> --- a/arch/arm/boot/dts/nspire.dtsi >>>> +++ b/arch/arm/boot/dts/nspire.dtsi >>>> @@ -172,7 +172,14 @@ rtc: rtc@90090000 { >>>> }; >>>> >>>> misc: misc@900a0000 { >>>> + compatible = "ti,nspire-misc", "syscon", "simple-mfd"; >>> >>> You have syscon and simple-mfd, but bindings in patch #1 say only syscon. >>> >> >> I'm not following, are you just saying my wording in the patch message just >> wasn't complete? > > Your binding patch adds nspire compatible to the list of two items, so > you have two items in total - nspire followed by syscon. > > What you implemented here is different. > Is there a list of three items I can add this compatible? If instead you mean I should go make a new binding, just say so :) >> >> Or are you saying something more about nodes that are both syscon and simple-mfd? >> In that case, having both syscon and simple-mfd seems rather common, looks like >> you added the rule for it[0]. >> >> Thinking on this, they almost represent the same thing. simple-mfd says "my child >> nodes should be considered devices", why do we need that? Couldn't we simply state >> that "syscon" node's children are always devices, I mean what else could they be, >> syscon is an MFD after all (and lives in drivers/mfd/). > > No, syscon is not an MFD. Syscon means system controller and alone it > does not have children. > The binding lives in devicetree/bindings/*mfd*/, it is mentioned as one in devicetree/bindings/mfd/mfd.txt. If it is not an MFD then the bindings are giving out mixed signals here.. >> >> "syscon" often just says, others can use the registers within this node, so as a >> different option, make "syscon" a property of "simple-mfd" nodes. I'm seeing all >> these examples of devices that should have been children of the "syscon" device, >> but instead use >> >> regmap = <&x>; >> syscon = <&x>; >> >> or similar and put the device node out somewhere random. And in those cases, >> wouldn't it have been more correct to use the normal "reg" and "regions" to >> define the registers belonging to the child node/device?.. > > Sorry, I do not follow. How this is even related to your patch? > > Your bindings say A, DTS say B. A != B. This needs fixing. > I said it was compatible with "syscon", not that it is incompatible with "simple-mfd" devices. What I've done here gives no dtbs_check warnings and "devicetree/bindings/mfd/mfd.txt" explicitly allows what I am doing. Unless we do not consider the old bindings valid? If so, would you like me to convert mfd.txt to yaml, just let me know. > Unless you are asking me what your device is in general. This I don't > really know, but if you want to use it as regmap provider for system > registers and as a parent of syscon-based reboot device, then your > device is syscon and simple-mfd. With a specific compatible. Was this > your question? > Yes, I would like to use it as a regmap provider, my question here is a much more general one: why do I need to specify that in device tree? That is not a hardware description, my hardware is not "regmap" hardware. This "syscon" stuff feels like a bodge to make the Linux drivers and bus frameworks interact the way we want. I know at this point this has little to do with this series, but I'd like to just think this out for a moment. The latest Devicetree Specification talks about "simple-bus" as a special compatible that communicates that child nodes with compatible strings need probed also. ("simple-mfd" seems to be used the same way but without needing a "ranges" property..) Both of these are properties of a node, not something a device is "compatible" with. "compatibles" are also supposed to be listed "from most specific to most general", so which is more specific, "simple-mfd" or "syscon", etc.. Seems like Rob might agree[0], these are not really compatibles. We cant fix history, but for new nodes, instead of growing the problem and forcing these to be overloaded compatibles, we allow these to become new standard node properties. For instance: main_conf: syscon@43000000 { compatible = "ti,j721e-system-controller"; reg = <0x0 0x43000000 0x0 0x20000>; simple-bus; syscon; ... }; Thoughts? Thanks, Andrew [0] https://lore.kernel.org/all/CAL_JsqKiUcO76bo1GoepWM1TusJWoty_BRy2hFSgtEVMqtrvvQ@mail.gmail.com/ > Best regards, > Krzysztof >