Re: [PATCH RFC v4 01/12] dt-bindings: clk: zte: Add zx297520v3 top clock and reset bindings

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2420 bytes --]

On Wed, Jun 17, 2026 at 08:47:53PM +0300, Stefan Dösinger wrote:
> Am Mittwoch, 17. Juni 2026, 19:08:02 Ostafrikanische Zeit schrieb Conor 
> Dooley:
> > On Tue, Jun 16, 2026 at 11:26:21PM +0300, Stefan Dösinger wrote:
> 
> > > +F:	Documentation/devicetree/zte,zx297520v3-*
> > 
> > Sashiko complaint here looks valid.
> 
> Yes, it is valid.
> 
> It also brought up another concern in patch 12 that has an impact on this 
> binding:
> 
> Am Dienstag, 16. Juni 2026, 23:42:19 Ostafrikanische Zeit schrieb sashiko-
> bot@kernel.org:
> > issue(s) to consider: - [Low] The `syscon-reboot` node is incorrectly
> > defined at the root level using the explicitly deprecated `regmap`
> > property. --
> 
> And indeed "regmap" is deprecated and I missed it somehow. As far as I 
> understand, to put syscon-reboot as a child of the clock, I need to add the 
> simple-mfd .compatible:
> 
> Either
> 
> topclk: clock-controller@13b000 {
> 	compatible = "zte,zx297520v3-topclk", "syscon", "simple-mfd";
> 	reg = <0x0013b000 0x400>;
> 	...
> 	syscon-reboot {
> 		compatible = "syscon-reboot";
> 		regmap = <&topclk>;
> 		offset = <0x0>;
> 		mask = <0x1>;
> 	};
> };
> 
> --- or ---
> 
> something@13b000 {
> 	compatible = "zte,zx297520v3-topcrm", "syscon", "simple-mfd";
> 	reg = <0x0013b000 0x400>;
> 	ranges;
> 
> 	topclk: clock-controller@0 {
> 		compatible = "zte,zx297520v3-topclk";
> 		...
> 	}
> 
> 	syscon-reboot {
> 		compatible = "syscon-reboot";
> 		offset = <0x0>;
> 		mask = <0x1>;
> 	};
> };
> 
> I see both ways in existing DTS files (e.g. imx6sl.dtsi for the fomer, 
> uniphier-pro4.dtsi for the latter). Is there a preferred way? I have a mild 
> preference for the first, as it would keep all 3 clocks in the same way. If I 
> go for the second, the clock driver would have to query its own node and the 
> parent node for the regmap.

The first one is more natural...

> AFAIU unrelated to syscon-reboot option 2 would give me the opportunity to 
> have separate clock and reset nodes and bindings and skip the aux bus, but 
> this would not be a correct representation of how the hardware works.

...for this reason!
Do you actually need an aux bus here though? Since you have to add
simple-mfd for your the syscon-reboot and simple-mfd is a real bus, can you
set the reset controller up with an mfd_cell + devm_mfd_add_devices()
instead?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]