linux-kernel - Re: [PATCH v3 2/9] ARM: dts: nspire: Use syscon-reboot to handle restart

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 27 Oct 2022 16:07:14 -0500
From:   Andrew Davis <afd@...com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Lee Jones <lee@...nel.org>, Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Arnd Bergmann <arnd@...db.de>,
        Linus Walleij <linus.walleij@...aro.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Daniel Tang <dt.tangr@...il.com>,
        Fabian Vogt <fabian@...ter-vogt.de>
CC:     <devicetree@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/9] ARM: dts: nspire: Use syscon-reboot to handle
 restart

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 <afd@...com>
>> Reviewed-by: Linus Walleij <linus.walleij@...aro.org>
>> Tested-by: Fabian Vogt <fabian@...ter-vogt.de>
>> Reviewed-by: Fabian Vogt <fabian@...ter-vogt.de>
>> ---
>>   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@...90000 {
>>   			};
>>   
>>   			misc: misc@...a0000 {
>> +				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?

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/).

"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?..

Thanks,
Andrew

[0] https://lore.kernel.org/all/20220817142246.828762-5-krzysztof.kozlowski@linaro.org/

> 
> Best regards,
> Krzysztof
>