Received-SPF: pass (google.com: domain of linux-kernel+bounces-1033-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Fri, 15 Dec 2023 13:42:38 +0000
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Jie Luo <quic_luoj@quicinc.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	hkallweit1@gmail.com, corbet@lwn.net, p.zabel@pengutronix.de,
	f.fainelli@gmail.com, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org
Subject: Re: [PATCH v8 14/14] dt-bindings: net: ar803x: add qca8084 PHY
 properties
Message-ID: <ZXxXzm8hP68KrXYs@shell.armlinux.org.uk>
References: <20231215074005.26976-1-quic_luoj@quicinc.com>
 <20231215074005.26976-15-quic_luoj@quicinc.com>
 <bdfba8a7-9197-4aae-a7f9-6075a375f60b@linaro.org>
 <c3391e33-e770-4c61-855e-d90e82b95f75@quicinc.com>
 <4cb2bd57-f3d3-49f9-9c02-a922fd270572@lunn.ch>
 <ed0dd288-be8a-4161-a19f-2d4d2d17b3ec@quicinc.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ed0dd288-be8a-4161-a19f-2d4d2d17b3ec@quicinc.com>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>

On Fri, Dec 15, 2023 at 08:16:53PM +0800, Jie Luo wrote:
> On 12/15/2023 7:25 PM, Andrew Lunn wrote:
> > > The "maxItems: 1" of the property resets is defined in ethernet-phy.yaml
> > > that is referenced by qca,ar803x.yaml, but i have 11 reset instances
> > > used for qca8084 PHY
> > 
> > 11!?!?? Really? Why?
> > 
> > I assume the order and timer matters, otherwise why would you need
> > 11? So the PHY driver needs to handle this, not phylib framework. So
> > you will be adding vendor properties to describe all 11 of them. So
> > ethernet-phy.yaml does not matter.
> > 
> > 	Andrew
> 
> Since these resets need to be configured in the special sequence, and
> these clocks need to be configured with different clock rate.
> 
> But the clock instance get, the property name is fixed to "clock-names"
> according to the function of_parse_clkspec, and the reset property name
> is also fixed to "reset-names" from function __of_reset_control_get.

I think you need to give more details about this.

Where are these 11 resets located? What is the sequence? Why does the
PHY driver need to deal with each individual reset?

IMHO, a PHY driver should _not_ be dealing with the resets outside of
the PHY device itself, and I find it hard to imagine that qca8084
would have 11 external resets.

If these are 11 internal resets (to qca8084) then why are you using the
reset subsystem, and why do you need to describe them in DT? Surely if
they are internal to the PHY, that can be encapsulated within the PHY
driver?

This is an example of why it is useful to have an _example_ of the use
of this binding, because it would answer some of the above questions.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!