Re: [PATCH 10/21] net: phy: add X-Powers AC200 EPHY control driver
James Hilliard <[email protected]>
| Newsgroups | gmane.linux.drivers.devicetree,gmane.linux.network,gmane.linux.ports.arm.kernel,gmane.linux.kernel,gmane.linux.ports.arm.rockchip |
|---|---|
| Message-ID | <CADvTj4o0JdjoO+FC4pO=qnjEQ8MRyzQdKp2Hxy9NsPU-gym5OQ@mail.gmail.com> |
On Mon, Aug 3, 2026 at 7:51 AM Andrew Lunn <[email protected]> wrote: > > > +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. Therefore, implementing a GPIO controller would expose capabilities the hardware does not have, and a pinmux driver would only provide a single fixed function. The callbacks exist because the LED function belongs to the PHY, while the pad-enable and polarity registers are reached through the AC200 I2C regmap rather than the PHY’s MDIO registers. > > > +static int ac200_ephy_ctl_power_off_locked(struct ac200_ephy_ctl *priv) > > +{ > > +} > > + > > +static int ac200_ephy_ctl_power_off(struct acx00_ephy_control *control) > > +{ > > > +} > > + > > +static int ac200_ephy_ctl_power_on(struct acx00_ephy_control *control, > > + unsigned int phy_addr) > > +{ > > +} > > These look like a regulator? The phylib core has support for > regulators. The external supplies are already represented as regulators. These callbacks do more than enable a supply: they sequence the package clock, reset and shutdown controls, then program the PHY address, calibration and interface state required before the normal Clause 22 endpoint becomes usable. > Maybe take a step back and look at the overall architecture, and how > Linux can represent these blocks. > > Andrew