Re: [PATCH RFC v5 00/12] ZTE zx297520v3 clock bindings and driver

[Conor Dooley <conor@kernel.org>]

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

On Wed, Jul 01, 2026 at 08:22:21PM +0300, Stefan Dösinger wrote:
> Am Montag, 29. Juni 2026, 18:49:08 Ostafrikanische Zeit schrieb Conor Dooley:
> 
> > Bindings seem fine to me, I'll be happy to give you some r-b tags when
> > you go non-RFC. To be frank, I think you should drop them as you've got
> > no significant questions here I think and you'll be taken a bt more
> > seriously.
> 
> Thanks for all the advice so far!
> 
> Here's one more binding related question: Philipp's request to give the PHY 
> reset its own reset ID means I need a node and driver to consume that reset. 
> My question is if it should be another MFD subdevice of topcrm or not. I am 
> leaning towards not:
> 
> usb_phy: phy@2 {
> 	compatible = "zte,zx29-usb2-phy";
> 	interrupts = <GIC_SPI 42 IRQ_TYPE_EDGE_RISING>, <GIC_SPI 43 
> IRQ_TYPE_EDGE_RISING>;
> 	interrupt-names = "powerup", "powerdown";
> 	syscon = <&topcrm 0x84 0x2>;
> 	resets = <&topcrm ZX297520V3_USB_PHY_RESET>;
> 	reset-names = "phy";
> 	#phy-cells = <0>;

Where is this node's reg property?
What bus is this node on?

It looks like the answer to both is that it should be a child of the
topcrm and this "syscon" property should be replaced by "reg".

> };
> 
> usb0: usb@1500000 {
> 	compatible = "snps,dwc2";
> 	reg = <0x01500000 0x1000>;
> 	...
> 	phys = <&usb_phy>;
> 	phy-names = "usb2-phy";
> };
> 
> I am not aware of any IO region to configure the PHY, although one may exist. 
> topcrm + 0x84 has two status bits reporting if USB and HSIC are powered and 
> out of reset. Nevertheless, the PHY feels distinct enough from topcrm that it 
> should have its own binding. The phy driver would merely deassert the reset 

This is the sort of thing that should be a child node of the syscon,
rather than integrated into the parent, given it consumes resources from
another feature of the syscon.

> and wait for the ready bit and maybe in the future do something useful with 
> the connect/disconnect IRQs.
> 
> Interestingly the USB IO region is actually downstream of the AHB bus and 
> matrix controller, but it has its clocks and resets in topcrm. I suspect the 
> purpose of this setup is to allow wake-by-USB IRQs while shutting down the 
> main data path.
> 
> Cheers,
> Stefan



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