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: <CADvTj4qdoD-mo7CxNW8VitZf+wXTiZ7m28R4JPQ9kQJGhUH7bA@mail.gmail.com>
Date: Mon, 26 May 2025 17:22:48 -0600
From: James Hilliard <james.hilliard1@...il.com>
To: Andrew Lunn <andrew@...n.ch>
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 4:38 PM Andrew Lunn <andrew@...n.ch> wrote:
>
> 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 these may not actually be required when using the internal
EPHY's now that I think about it again.

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

Hmm, would that make sense even though the MAC driver also
supports external PHY's?

> 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.

Well the current patches I've been playing around with for the AC200
phy are pretty similar to those patches still.

Here's a branch that works on both AC200/AC300 but only if I do
the PHY initialization in u-boot:
https://github.com/jameshilliard/linux-h616/commits/acx00

> 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.

This is what I'm doing right? I mean I'm putting the phy ID in the
DT for both the AC200 and AC300. When doing the reset driver
for say the AC200 I would wire that up to only the AC200 phy
node and not the AC300 node(since the AC300 reset is MDIO
based while the AC200 is i2c based).

> 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.

Without a MFD or some other node that exposes a reset I'm a bit
confused about what driver would actually issue the reset.

Yeah, we need a phandle to the i2c_adapter, but since the resets
would be under the AC200 PHY node I assume there would need
to be some sort of intermediary driver implementing the i2c reset
right?