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: 2788 bytes --]

On Wed, Jun 17, 2026 at 10:23:56PM +0100, Conor Dooley wrote:
> 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?

Although, the /naming/ is more natural in the second case. "topcrm"
sounds like the actual name of this register region. And syscon@ for the
node name.

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