Re: [PATCH 10/21] net: phy: add X-Powers AC200 EPHY control driver

On Tue, Aug 4, 2026 at 2:45=E2=80=AFAM James Hilliard <james.hilliard1@gmai=
l.com> wrote:
>
> On Mon, Aug 3, 2026 at 11:57=E2=80=AFAM Andrew Lunn <[email protected]> wrot=
e:
> >
> > On Mon, Aug 03, 2026 at 10:09:01AM -0600, James Hilliard wrote:
> > > On Mon, Aug 3, 2026 at 7:51=E2=80=AFAM Andrew Lunn <[email protected]> w=
rote:
> > > >
> > > > > +static int
> > > > > +ac200_ephy_ctl_set_led_outputs(struct acx00_ephy_control *contro=
l,
> > > > > +                            unsigned long outputs)
> > > > > +{
> > > > > +}
> > > >
> > > > > +static int
> > > > > +ac200_ephy_ctl_set_led_polarity(struct acx00_ephy_control *contr=
ol,
> > > > > +                             bool active_low)
> > > > > +{
> > > > > +}
> > > >
> > > > I've not looked into the details, but these look like pinmux. Does
> > > > this I2C device have a GPIO controller? Can these pins be used for
> > > > other things than LEDs? Maybe you should be implementing a GPIO and
> > > > pinmux driver. The Ethernet PHY driver can then just select the
> > > > correct pinmux configuration.
> > >
> > > These do not appear to be GPIO or muxable pins. The AC200
> > > documentation describes them as three dedicated EPHY LED
> > > outputs: link/activity, speed, and duplex. The bits in SYS_EPHY_CTL1
> > > enable those output pads, while EPHY_CTL provides their shared
> > > polarity setting. I have not found any documented GPIO data
> > > registers or alternative functions for these pins.
> >
> > So what i'm trying to do with both of these is make it look more like
> > standard Linux building blocks.
> >
> > If that is not possible, i would actually put all this code into the
> > PHY driver. You then have a regmap property to gain access to the I2C
> > registers.
>
> I understand the goal of using standard Linux building blocks where they
> fit, but I do not think folding these control paths into the common PHY
> driver gives the right device model here.
>
> The AC200 and AC300 expose the same Clause 22 PHY interface, but their
> control functions are separate devices with different transports and
> lifetimes. The AC200 control function is an MFD child accessed through
> the parent I2C regmap. The AC300 control function is a separately
> addressed non-PHY MDIO device. A regmap reference only describes the
> AC200 transport; it does not represent the AC300 control device.
>
> Folding both implementations into the common PHY driver would make that
> driver manage AC200 MFD registers and resources as well as an AC300 MDIO
> control endpoint. That would mix the package-specific control transports
> with the otherwise common Clause 22 implementation.
>
> The current split uses standard subsystems wherever they provide a
> matching abstraction: MFD, regmap, MDIO devices, clocks, regulators,
> NVMEM, device links, and the PHY LED framework. The narrow private
> interface covers only the compound operation which those frameworks
> cannot express: calibrating, addressing, configuring, and sequencing the
> package control block before the Clause 22 interface becomes usable.
>
> The link address is an important example. It is defined once by
> phydev->mdio.addr. AC200 must program that value through its I2C control
> interface, while AC300 must validate it against the address of its MDIO
> control endpoint. Generic PHY operations do not carry a consumer MDIO
> address. Encoding it in a phys specifier or control-node property would
> duplicate the PHY node's reg value. Giving the common PHY driver a raw
> regmap avoids passing the address only by moving the provider-specific
> implementation across the driver boundary.
>
> The LED pad-enable and polarity bits are similarly part of the package
> control function. They control dedicated EPHY outputs rather than GPIOs
> or muxable pins. The common PHY driver still owns the PHY LED semantics,
> while the selected control driver performs the package-specific register
> access.
>
> For those reasons, I think keeping the transport-specific control drivers
> and the common Clause 22 PHY driver separate is the cleaner division.

I think what Andrew is saying is that you can expose the regmap from the
MFD node, which you then reference from the PHY node with a custom phandle.

In the PHY driver, the first thing to do is retrieve the syscon, and if it
is not available yet, return -EPROBE_DEFER. Then you program all the bits
you currently do, just without going through a layer of redirection.
You can fetch the PHY address to program from the device tree. The downside
to doing all the setup in the PHY driver is that the PHY node has to have
the PHY ID in the compatible, as it is no longer generic. But you already
have that.


ChenYu