linux-kernel - Re: [PATCH] mux: mmio: use reg property when parent device is not a syscon

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]

Message-ID: <7d73a37f-80b7-b623-1b71-fa19bf379713@linaro.org>
Date:   Tue, 16 May 2023 18:19:36 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Andrew Davis <afd@...com>, Peter Rosin <peda@...ntia.se>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Nishanth Menon <nm@...com>,
        Vignesh Raghavendra <vigneshr@...com>
Cc:     devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mux: mmio: use reg property when parent device is not a
 syscon

On 16/05/2023 17:18, Andrew Davis wrote:
> On 5/15/23 4:14 PM, Peter Rosin wrote:
>> Hi!
>>
>> 2023-05-15 at 21:19, Andrew Davis wrote:
>>> The DT binding for the reg-mux compatible states it can be used when the
>>> "parent device of mux controller is not syscon device". It also allows
>>> for a reg property. When the parent device is indeed not a syscon device,
>>> nor is it a regmap provider, we should fallback to using that reg
>>> property to identify the address space to use for this mux.
>>
>> We should? Says who?
>>
>> Don't get me wrong, I'm not saying the change is bad or wrong, I would just
>> like to see an example where it matters. Or, at least some rationale for why
>> the code needs to change other than covering some case that looks like it
>> could/should be possible based on the binding. I.e., why is it not better to
>> "close the hole" in the binding instead?
>>
> 
> Sure, so this all stated when I was building a checker to make sure that drivers
> are not mapping overlapping register spaces. I noticed syscon nodes are a source
> of that so I'm trying to look into their usage.
> 
> To start, IHMO there is only one valid use for syscon and that is when more than
> one driver needs to access shared bits in a single register. DT has no way to

It has... what about all existing efuse/nvmem devices?

> describe down to the bit granular level, so one must give that register to
> a "syscon node", then have the device node use a phandle to the syscon node:
> 
> common_reg: syscon@...00 {
> 	compatible = "syscon";
> 	reg = <0x10000 0x4>;
> };
> 
> consumer@1 {
> 	syscon-efuse = <&common_reg 0x1>;
> };
> 
> consumer@2 {
> 	syscon-efuse = <&common_reg 0x2>;
> };
> 
> Something like that, then regmap will take care of synchronizing access.

Syscon is not for this.

> 

...

> 
> Ideally DT nodes all describe their register space in a "reg"
> property and all the "large collection of devices" spaces become
> "simple-bus" nodes. "syscon" nodes can then be limited to only the
> rare case when multiple devices share bits in a single register.
> 
> If Rob and Krzysztof agree I can send a patch with the above
> guidance to the Devicetree Specification repo also.

Agree on what?


Best regards,
Krzysztof