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

[Andrew Davis]

On 5/16/23 11:19 AM, Krzysztof Kozlowski wrote:
> 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@10000 {
> > 	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.

That is how it is used today, and in 5 other ways too and there is
no guidance on it. Let me know what syscon is for then.

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

That we should provide the above guidance on when and how to use syscon
nodes. Right now it is a free for all and it is causing issues.

Andrew

> Best regards,
> Krzysztof