Re: [PATCH RFC v3 0/5] ZTE zx297520v3 clock bindings and driver

["Stefan Dösinger" <stefandoesinger@gmail.com>]

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

Hi Philipp,

I did some more reading and checked past clock driver submissions. This email 
is to check if I understood it right.

Am Donnerstag, 4. Juni 2026, 18:23:01 Ostafrikanische Zeit schrieb Philipp 
Zabel:
> > The register lock because all LSP and at least one TOP register contains
> > both clocks and resets.
> 
> That could be solved with regmap.

This will require me to change the clk component to use regmaps too. There's 
no regmap equivalent to clk-{div,gate,mux}.c, so I'll need my own. qcom and 
meson have similar drivers already, so I'd likely copy one of them. Is there a 
particular reason why there isn't a regmap equivalent of clk-{div,gate,mux}.c?

Another hypothetical solution is a custom regmap implementation that locks my 
clk driver's lock. I see that only in imx/clk-imx8ulp-sim-lpav.c.

However, the topclk region also has a stray register that controls if a 
watchdog timeout resets the board. So there's no good way past a syscon 
compatible and syscon generated regmap anyway.

Afaics syscon regmaps only support a single IO region, so I'd likely revert 
back to the topclk/matrixclk split with different bindings, bite the other 
bullet and list all 50 or so PLL outputs as clocks passed from top to matrix.

Which gives a device tree setup like this:

topcrm: something@13b000 {
	compatible = "zte,zx297520v3-topcrm", "syscon", "simple-mfd";
	reg = <0x0013b000 0x400>;
	#address-cells = <1>;
	#size-cells = <1>;
	ranges;

	topclk: clock-controller@0 {
		compatible = "zte,zx297520v3-topclk";
		reg = <0x0 0x400>;
		#clock-cells = <1>;
		#reset-cells = <1>;
		clocks = <&osc26m>, <&osc32k>;
		clock-names = "osc26m", "osc32k";
	};

	reboot {
		compatible = "syscon-reboot";
		offset = <0x0>;
		mask = <0x1>;
	};
};

watchdog_t18: watchdog@148000 {
	compatible = "zte,zx297520-wdt";
	reg = <0x00148000 0x20>;
	clocks = <&topclk ZX297520V3_WDT_T18_WCLK>, <&topclk 
ZX297520V3_WDT_T18_PCLK>;
	clock-names = "wclk", "pclk";
	resets = <&topclk ZX297520V3_WDT_T18_RESET>;
	zte,wdt-reset-sysctrl = <&topcrm 0x2c 0x3 0x3>;
};

(I did not attempt to build this, might have typos)

And the reset controller will be an aux-bus child of the clock controller. I 
could make the reset controller its own device with its own bindings, but that 
would misrepresent the hardware.

Did I understand this correctly?

For some reason in my dev tree the reset sysctrl works even though my clock 
driver does not use the syscon compatible nor manually create a regmap.

> > Shared register definition: in the case of the LSP clocks breaking up the
> > composite definition would sacrifice readability.
> 
> That is a matter of perspective.
> 
> I had a harder time validating that the resets[] array is properly
> initialized from the two different composite arrays because of the
> unordered reset_ids, and some remaining resets[] being filled via code.

I do have a sanity check loop for that.

> It would be much simpler if you split the reset definitions out into a
> single, separate, const array, indexed by reset id. In fact,
> I would suggest this even if you don't intend to move the reset code.

I had to collect the clocks and resets from all over ZTE's kernel and U-Boot 
sources plus manual testing, so maybe I am a bit too attached to seeing all 
controls for a given device in one place. If the reset controls move to a 
different file, the composite structure becomes less useful, so I'll probably 
split it up and just list single div, gates and regs.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 870 bytes --]