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: <d9d0881b-12cd-40af-bb22-d84236d2e04d@lunn.ch>
Date: Tue, 27 May 2025 01:44:54 +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 05:22:48PM -0600, James Hilliard wrote:
> 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?

If an external PHY is being used, i would not expect a phy-mode of
internal.
 
> > 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

I personally don't like depending on the bootloader. I often replace
the bootloader, because it is a crippled version that does not let me
TFTP boot, does not have flash enabled for saving configuration, and i
want to use barebox etc. I think it is much better when Linux drives
the hardware, not the bootloader.

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

You need an reset controller, yes, but is that an MFD? Or just a reset
controller? Where does this reset controller live?

Question like this show we are missing the big picture. What are all
the different parts, how are they interconnected? Once we see that, we
can give you better advice.

	Andrew