linux-kernel - Re: [PATCH v1 3/3] dt-bindings: net: sun8i-emac: Add AC300 EMAC1 nvmem phy selection

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d4109cc5-83d5-4acd-b0fb-39a50043060b@lunn.ch>
Date: Tue, 27 May 2025 00:38:15 +0200
From: Andrew Lunn <andrew@...n.ch>
To: James Hilliard <james.hilliard1@...il.com>
Cc: netdev@...r.kernel.org, linux-sunxi@...ts.linux.dev,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, Chen-Yu Tsai <wens@...e.org>,
	Jernej Skrabec <jernej.skrabec@...il.com>,
	Samuel Holland <samuel@...lland.org>,
	Maxime Ripard <mripard@...nel.org>, devicetree@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 3/3] dt-bindings: net: sun8i-emac: Add AC300 EMAC1
 nvmem phy selection

On Mon, May 26, 2025 at 03:32:03PM -0600, James Hilliard wrote:
> On Mon, May 26, 2025 at 1:36 PM Andrew Lunn <andrew@...n.ch> wrote:
> >
> > > +        phy-mode = "rgmii";
> >
> > Does the PCB have extra long clock lines?
> 
> I'm not sure, it's a copackaged(maybe on-die is the wrong terminology)
> PHY I think so I assume the clock lines are internal, in the device specific
> dts we set something like this on the emac1 node:
> allwinner,rx-delay-ps = <3100>;
> allwinner,tx-delay-ps = <700>;

Those values are just weird. The RGMII delay should be 2000ps. 3100 is
way too big, and 700 is way too small.

I think phy-mode = "internal" would be better, and just hard code the
delays either in the MAC or PHY driver.

Thanks for the link to the old thread, which was 5 years
ago. Hopefully since then, a bit more has been learnt. Quickly reading
through that thread, i don't think an MFD is not the correct solution.
In the last 5 years we have had to deal with more chicken/egg problems
with PHYs. It has now become pretty much standard practice to put the
ID values in DT, to get the driver probed when the device does not
respond on the bus. The DT node can then use phandles to the reset and
clock controller to configure them as needed, the core will probably
do that. I2C is a bit messier, you probably want a phandle pointing to
the i2c_adapter, so you can use i2c_transfer() on it in the probe()
function.

	Andrew