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

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

Hi James,

On 8/3/26 07:14, James Hilliard wrote:
> AC200 and AC300 contain compatible Fast Ethernet link-PHY implementations
> which report the same ID, but their identification registers are not
> accessible until the companion-specific control interface completes
> its power-up sequence. Match the ID supplied by firmware and require
> an X-Powers control reference because the Rockchip RK630 PHY reports
> the same ID.
> 
> On fixed hardware, reference one already enabled control device. A fixed
> AC300 system may optionally read the configuration field for calibration-
> range tuning; its absence selects the normal range. On systems which may
> contain either companion, require a logical two-bit NVMEM field to select
> the control interface and the AC300 calibration range. Enable a selected
> fail-needs-probe node when CONFIG_OF_DYNAMIC is available; otherwise the
> same topology works when both candidate providers are already enabled.
> Validate that each phandle references the expected control-device type and
> restore the selector's status changeset if dynamic device creation fails.
> 
> Keep a device link to the selected provider and defer until its driver
> binds. Verify that the MAC interface agrees with the provider's phy-mode
> before resetting the hardware. Power the control block before each PHY
> reset, shut it down during PHY suspend, and restore the vendor
> configuration after resume. If control power-off fails during suspend,
> attempt to power the PHY back on and restore its link configuration before
> returning the error. Pass the link address to the AC200 provider for
> EPHY_CTL.PHY_ADDR programming; the AC300 provider instead validates its
> package-strapped address relationship.
> 
> Apply the common vendor analog initialization and SID-derived tuning while
> preserving unrelated register fields. Support a board-selected xMII
> receive-clock inversion and report and configure forced or automatic
> MDI/MDI-X state.
> 
> Expose UAPS through the ethtool energy-detect power-down tunable as its
> no-transmit mode. Accumulate the hardware receive-byte, receive-packet and
> receive-CRC-error counters across PHY resets, expose all three as driver
> statistics and map CRC errors to the standardized PHY receive-error
> counter. Treat counter snapshots as best-effort telemetry so a read failure
> cannot prevent PHY recovery.
> 
> Start both standard EEE advertisement and the PHY-autonomous Intelligent
> EEE transmitter disabled, matching the vendor configuration. Preserve the
> supported modes so ethtool can enable and configure both at runtime. The
> PHY has no documented programmable LPI timer, so reject a nonzero timer
> when enabling Tx LPI while accepting any timer value when disabling it.
> Clear BMCR_PDOWN before soft reset because ACx00 can acknowledge reset
> while powered down without restarting its link logic.
> 
> Use the shared PHY interrupt for link changes when firmware provides one;
> otherwise retain the normal phylib polling fallback. On wakeup-source
> systems with a routed interrupt, expose magic-packet and link-change
> Wake-on-LAN, program the detector MAC address for magic wake, and arm the
> nested IRQ as a wake source. Keep the PHY powered while either PHY or MAC
> Wake-on-LAN is active. During suspend, leave only requested PHY wake events
> unmasked, then restore normal link monitoring on resume. Do not advertise
> either wake mode in polling mode because the CPU cannot poll while
> suspended.
> 
> Register standard PHY LEDs for the described package outputs and expose
> their fixed link/activity, 100 Mbps speed and duplex rules to the netdev
> trigger. Use the vendor fallback of all three outputs active-low when no
> LED topology is provided; otherwise apply the described output set and
> shared polarity. Quiesce the provider if PHY probing or removal tears down
> the link device.

That's a very verbose commit message, which gives it an 'LLM generated' vibe.

Can you submit that driver separately and split down into multiple pieces ?

1457 lines is quite big for a PHY driver, especially one that supports
just a pair of PHYs. That makes it hard to review.

Can you plit it into more logical chunks, one bringing basic features,
one for LEDs, one for WoL, one for stats, etc.

Thanks,

Maxime



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip