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

On Mon, Aug 3, 2026 at 11:57=E2=80=AFAM Andrew Lunn <[email protected]> wrote:
>
> 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]> wro=
te:
> > >
> > > > +static int
> > > > +ac200_ephy_ctl_set_led_outputs(struct acx00_ephy_control *control,
> > > > +                            unsigned long outputs)
> > > > +{
> > > > +}
> > >
> > > > +static int
> > > > +ac200_ephy_ctl_set_led_polarity(struct acx00_ephy_control *control=
,
> > > > +                             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.

>
>        Andrew
>