RE: [Intel-wired-lan] [PATCH v2 net-next 8/8] ice: add TX reference clock (tx_clk) control for E825 devices

[Nitka, Grzegorz]

> -----Original Message-----
> From: Loktionov, Aleksandr <aleksandr.loktionov@xxxxxxxxx>
> Sent: Tuesday, March 24, 2026 9:20 AM
> To: Nitka, Grzegorz <grzegorz.nitka@xxxxxxxxx>; netdev@xxxxxxxxxxxxxxx
> Cc: Vecera, Ivan <ivecera@xxxxxxxxxx>; vadim.fedorenko@xxxxxxxxx;
> kuba@xxxxxxxxxx; jiri@xxxxxxxxxxx; edumazet@xxxxxxxxxx; Kitszel,
> Przemyslaw <przemyslaw.kitszel@xxxxxxxxx>; richardcochran@xxxxxxxxx;
> donald.hunter@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Kubalewski,
> Arkadiusz <arkadiusz.kubalewski@xxxxxxxxx>; andrew+netdev@xxxxxxx;
> intel-wired-lan@xxxxxxxxxxxxxxxx; horms@xxxxxxxxxx;
> Prathosh.Satish@xxxxxxxxxxxxx; Nguyen, Anthony L
> <anthony.l.nguyen@xxxxxxxxx>; pabeni@xxxxxxxxxx;
> davem@xxxxxxxxxxxxx
> Subject: RE: [Intel-wired-lan] [PATCH v2 net-next 8/8] ice: add TX reference
> clock (tx_clk) control for E825 devices
> 
> 
> 
> > -----Original Message-----
> > From: Intel-wired-lan <intel-wired-lan-bounces@xxxxxxxxxx> On Behalf
> > Of Grzegorz Nitka
> > Sent: Saturday, March 21, 2026 11:26 PM
> > To: netdev@xxxxxxxxxxxxxxx
> > Cc: Vecera, Ivan <ivecera@xxxxxxxxxx>; vadim.fedorenko@xxxxxxxxx;
> > kuba@xxxxxxxxxx; jiri@xxxxxxxxxxx; edumazet@xxxxxxxxxx; Kitszel,
> > Przemyslaw <przemyslaw.kitszel@xxxxxxxxx>; richardcochran@xxxxxxxxx;
> > donald.hunter@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Kubalewski,
> > Arkadiusz <arkadiusz.kubalewski@xxxxxxxxx>; andrew+netdev@xxxxxxx;
> > intel-wired-lan@xxxxxxxxxxxxxxxx; horms@xxxxxxxxxx;
> > Prathosh.Satish@xxxxxxxxxxxxx; Nguyen, Anthony L
> > <anthony.l.nguyen@xxxxxxxxx>; pabeni@xxxxxxxxxx;
> davem@xxxxxxxxxxxxx
> > Subject: [Intel-wired-lan] [PATCH v2 net-next 8/8] ice: add TX
> > reference clock (tx_clk) control for E825 devices
> >
> > Add full support for selecting and controlling the TX SERDES reference
> > clock on E825C hardware. E825C devicede supports selecting among
> > multiple SERDES transmit reference clock sources (ENET, SyncE, EREF0),
> > but imposes several routing constraints: on some paths a reference
> > must be enabled on both PHY complexes, and ports sharing a PHY must
> > coordinate usage so that a reference is not disabled while still in
> > active use. Until now the driver did not expose this domain through
> > the DPLL API, nor did it provide a coherent control layer for
> > enabling, switching, or tracking TX reference clocks.
> >
> > This patch implements full TX reference clock management for E825
> > devices. Compared to previous iterations, the logic is now separated
> > into a dedicated module (ice_txclk.c) which encapsulates all clock-
> > selection rules, cross‑PHY dependencies, and the bookkeeping needed to
> > ensure safe transitions. This allows the DPLL layer and the PTP code
> > to remain focused on their respective roles.
> >
> > Key additions:
> >
> >   * A new txclk control module (`ice_txclk.c`) implementing:
> >       - software usage tracking for each reference clock per PHY,
> >       - peer‑PHY enable rules (SyncE required on both PHYs when used
> > on
> >         PHY0, EREF0 required on both when used on PHY1),
> >       - safe disabling of unused reference clocks after switching,
> >       - a single, driver‑internal entry point for clock changes.
> >
> >   * Integration with the DPLL pin ops:
> >       - pin‑set now calls into `ice_txclk_set_clk()` to request a
> >         hardware switch,
> >       - pin‑get reports the current SERDES reference by reading back
> > the
> >         active selector (`ice_get_serdes_ref_sel_e825c()`).
> >
> >   * Wiring the requested reference clock into Auto‑Negotiation restart
> >     through the already‑extended `ice_aq_set_link_restart_an()`.
> >
> >   * After each link-up the driver verifies the effective hardware
> > state
> >     (`ice_txclk_verify()`) and updates its per‑PHY usage bitmaps,
> >     correcting the requested/active state if the FW or AN flow applied
> > a
> >     different reference.
> >
> >   * PTP PF initialization now seeds the ENET reference clock as
> > enabled
> >     by default for its port.
> >
> > All reference clock transitions are serialized through the DPLL lock,
> > and usage information is shared across all PFs belonging to the same
> > E825C controller PF. This ensures that concurrent changes are
> > coordinated and that shared PHYs never see an unexpected disable.
> >
> > With this patch, E825 devices gain full userspace‑driven TXC reference
> > clock selection via the DPLL subsystem, enabling complete SyncE
> > support, precise multi‑clock setups, and predictable clock routing
> > behavior.
> >
> > Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@xxxxxxxxx>
> > Signed-off-by: Grzegorz Nitka <grzegorz.nitka@xxxxxxxxx>
> > ---
> >  drivers/net/ethernet/intel/ice/Makefile     |   2 +-
> >  drivers/net/ethernet/intel/ice/ice_dpll.c   |  53 ++++-
> >  drivers/net/ethernet/intel/ice/ice_ptp.c    |  22 ++
> >  drivers/net/ethernet/intel/ice/ice_ptp.h    |   7 +
> >  drivers/net/ethernet/intel/ice/ice_ptp_hw.c |  37 +++
> > drivers/net/ethernet/intel/ice/ice_ptp_hw.h |  27 +++
> > drivers/net/ethernet/intel/ice/ice_txclk.c  | 237 ++++++++++++++++++++
> > drivers/net/ethernet/intel/ice/ice_txclk.h  |  41 ++++
> >  8 files changed, 414 insertions(+), 12 deletions(-)  create mode
> > 100644 drivers/net/ethernet/intel/ice/ice_txclk.c
> >  create mode 100644 drivers/net/ethernet/intel/ice/ice_txclk.h
> >
> > diff --git a/drivers/net/ethernet/intel/ice/Makefile
> > b/drivers/net/ethernet/intel/ice/Makefile
> > index 38db476ab2ec..95fd0c49800f 100644
> > --- a/drivers/net/ethernet/intel/ice/Makefile
> > +++ b/drivers/net/ethernet/intel/ice/Makefile
> > @@ -54,7 +54,7 @@ ice-$(CONFIG_PCI_IOV) +=	\
> >  	ice_vf_mbx.o		\
> >  	ice_vf_vsi_vlan_ops.o	\
> >  	ice_vf_lib.o
> > -ice-$(CONFIG_PTP_1588_CLOCK) += ice_ptp.o ice_ptp_hw.o ice_dpll.o
> > ice_tspll.o ice_cpi.o
> > +ice-$(CONFIG_PTP_1588_CLOCK) += ice_ptp.o ice_ptp_hw.o ice_dpll.o
> > +ice_tspll.o ice_cpi.o ice_txclk.o
> >  ice-$(CONFIG_DCB) += ice_dcb.o ice_dcb_nl.o ice_dcb_lib.o
> >  ice-$(CONFIG_RFS_ACCEL) += ice_arfs.o
> >  ice-$(CONFIG_XDP_SOCKETS) += ice_xsk.o
> > diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c
> > b/drivers/net/ethernet/intel/ice/ice_dpll.c
> > index 38a0bbb316d8..286146c6d4d2 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
> > @@ -4,6 +4,7 @@
> >  #include "ice.h"
> >  #include "ice_lib.h"
> >  #include "ice_trace.h"
> 
> ...
> 
> > +/**
> > + * ice_txclk_set_clk - Set Tx reference clock
> > + * @pf: pointer to pf structure
> > + * @clk: new Tx clock
> > + *
> > + * Return: 0 on success, negative value otherwise.
> > + */
> > +int ice_txclk_set_clk(struct ice_pf *pf, enum ice_e825c_ref_clk clk)
> > {
> > +	struct ice_pf *ctrl_pf = ice_get_ctrl_pf(pf);
> > +	struct ice_port_info *port_info;
> > +	u8 port_num, phy;
> > +	int err;
> > +
> > +	if (pf->ptp.port.tx_clk == clk)
> > +		return 0;
> > +
> > +	port_num = pf->ptp.port.port_num;
> > +	phy = port_num / pf->hw.ptp.ports_per_phy;
> > +	port_info = pf->hw.port_info;
> > +
> > +	/* Check if the TX clk is enabled for this PHY, if not - enable
> > it */
> > +	if (!ice_txclk_any_port_uses(ctrl_pf, phy, clk)) {
> > +		err = ice_cpi_ena_dis_clk_ref(&pf->hw, phy, clk, true);
> > +		if (err) {
> > +			dev_err(ice_hw_to_dev(&pf->hw), "Failed to enable
> > the %u TX clock for the %u PHY\n",
> > +				clk, phy);
> > +			return err;
> > +		}
> > +		err = ice_txclk_enable_peer(pf, clk);
> > +		if (err)
> > +			return err;
> > +	}
> > +
> > +	pf->ptp.port.tx_clk_req = clk;
> "requested clock" state variable is committed HERE, BEFORE the hardware
> command below.
> If the AQ command fails, this is never rolled back.
> 
> > +
> > +	/* We are ready to switch to the new TX clk. */
> > +	err = ice_aq_set_link_restart_an(port_info, true, NULL,
> > +					 ICE_REFCLK_USER_TO_AQ_IDX(clk));
> > +	if (err)
> > +		dev_err(ice_hw_to_dev(&pf->hw), "Failed to switch to %u
> > TX clock for the %u PHY\n",
> > +			clk, phy);
> Function returns error, but damage still persists.
> 

Thanks Aleks for your comment.

Hmm ... 'yes' and 'no'.
Setting different tx-clk is a multistage process.
Please note, in this implementation, the final verification whether switching
succeeded or not, is done in ice_txclk_verify function.
At this point what we know is that AN restart failed (it might be many reasons for that).
I think, what is worth to change, is the error message which should be more
about AN restart failure.

Regards

Grzegorz
> > +
> > +	return err;
> > +}
> 
> ...
> 
> > --
> > 2.39.3