Message-ID: <CADvTj4qZz5q7x3_+OB8FiSnkEehzzjikVcCaqdcwHDYesMzpWw@mail.gmail.com>
Date: Mon, 26 May 2025 18:16:22 -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 5:45 PM Andrew Lunn <andrew@...n.ch> wrote:
>
> 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.

Ok, I guess this is a bit of a separate issue vs phy selection right?

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

Yeah, I'm just using that for verifying the PHY selection logic at the
moment is functional and that Linux can handle the PHY's once in
an initialized state. The initialization sequence I'm using in u-boot
is pretty far from being suitable for upstream as well.

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

Well at least the AC200(the i2c one) appears to be a full MFD:
https://github.com/DeciHD/allwinner_docs/blob/main/mfd_xpowers/AC200_Datasheet_V1.1.pdf

The AC300 appears to only have EPHY related functionality:
https://github.com/DeciHD/allwinner_docs/blob/main/mfd_xpowers/AC300_User_Manual_V1.0.pdf

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

If you look at the block diagrams here it should hopefully give you
a better idea of how the AC200 and AC300 PHY's are connected
on the H616:
http://file.whycan.com/files/202304/V85x/Linux_EMAC_%e5%bc%80%e5%8f%91%e6%8c%87%e5%8d%97.pdf

Powered by blists - more mailing lists