* [PATCH v7 net-next 1/8] dpll: add generic DPLL type
2026-04-30 9:42 [PATCH v7 net-next 0/8] dpll/ice: Add generic DPLL type and full TX reference clock control for E825 Grzegorz Nitka
@ 2026-04-30 9:42 ` Grzegorz Nitka
2026-04-30 11:49 ` [Intel-wired-lan] " Loktionov, Aleksandr
2026-04-30 9:42 ` [PATCH v7 net-next 2/8] dpll: allow registering FW-identified pin with a different DPLL Grzegorz Nitka
` (6 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Grzegorz Nitka @ 2026-04-30 9:42 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, intel-wired-lan, poros, richardcochran,
andrew+netdev, przemyslaw.kitszel, anthony.l.nguyen,
Prathosh.Satish, ivecera, jiri, arkadiusz.kubalewski,
vadim.fedorenko, donald.hunter, horms, pabeni, kuba, davem,
edumazet, Grzegorz Nitka
Add DPLL_TYPE_GENERIC to represent DPLL devices which do not fit the
existing PPS or EEC classes.
The UAPI type is intentionally generic. During netdev discussion,
maintainers pointed out that introducing identifiers tied to a specific
placement or single design does not scale across ASICs and vendors.
The role of a DPLL is already inferable from the spawning driver,
bus device, and pin topology, without encoding additional
purpose-specific taxonomy in the type name.
Using a generic type keeps the UAPI extensible and avoids premature
naming that may become incorrect as new hardware topologies are
exposed through the DPLL subsystem.
Expose the new type through UAPI and netlink specification as "generic".
Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
---
Documentation/netlink/specs/dpll.yaml | 3 +++
drivers/dpll/dpll_nl.c | 2 +-
include/uapi/linux/dpll.h | 2 ++
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/Documentation/netlink/specs/dpll.yaml b/Documentation/netlink/specs/dpll.yaml
index 40465a3d7fc2..572cf7ae5f36 100644
--- a/Documentation/netlink/specs/dpll.yaml
+++ b/Documentation/netlink/specs/dpll.yaml
@@ -138,6 +138,9 @@ definitions:
-
name: eec
doc: dpll drives the Ethernet Equipment Clock
+ -
+ name: generic
+ doc: generic dpll type for devices outside PPS/EEC classes
render-max: true
-
type: enum
diff --git a/drivers/dpll/dpll_nl.c b/drivers/dpll/dpll_nl.c
index 1e652340a5d7..9a3b70ea3ae0 100644
--- a/drivers/dpll/dpll_nl.c
+++ b/drivers/dpll/dpll_nl.c
@@ -34,7 +34,7 @@ const struct nla_policy dpll_reference_sync_nl_policy[DPLL_A_PIN_STATE + 1] = {
static const struct nla_policy dpll_device_id_get_nl_policy[DPLL_A_TYPE + 1] = {
[DPLL_A_MODULE_NAME] = { .type = NLA_NUL_STRING, },
[DPLL_A_CLOCK_ID] = { .type = NLA_U64, },
- [DPLL_A_TYPE] = NLA_POLICY_RANGE(NLA_U32, 1, 2),
+ [DPLL_A_TYPE] = NLA_POLICY_RANGE(NLA_U32, 1, 3),
};
/* DPLL_CMD_DEVICE_GET - do */
diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h
index 871685f7c353..648553053cd8 100644
--- a/include/uapi/linux/dpll.h
+++ b/include/uapi/linux/dpll.h
@@ -109,10 +109,12 @@ enum dpll_clock_quality_level {
* enum dpll_type - type of dpll, valid values for DPLL_A_TYPE attribute
* @DPLL_TYPE_PPS: dpll produces Pulse-Per-Second signal
* @DPLL_TYPE_EEC: dpll drives the Ethernet Equipment Clock
+ * @DPLL_TYPE_GENERIC: generic dpll type for devices outside PPS/EEC classes
*/
enum dpll_type {
DPLL_TYPE_PPS = 1,
DPLL_TYPE_EEC,
+ DPLL_TYPE_GENERIC,
/* private: */
__DPLL_TYPE_MAX,
--
2.39.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* RE: [Intel-wired-lan] [PATCH v7 net-next 1/8] dpll: add generic DPLL type
2026-04-30 9:42 ` [PATCH v7 net-next 1/8] dpll: add generic DPLL type Grzegorz Nitka
@ 2026-04-30 11:49 ` Loktionov, Aleksandr
2026-05-05 7:43 ` Nitka, Grzegorz
0 siblings, 1 reply; 21+ messages in thread
From: Loktionov, Aleksandr @ 2026-04-30 11:49 UTC (permalink / raw)
To: Nitka, Grzegorz, netdev@vger.kernel.org
Cc: Vecera, Ivan, vadim.fedorenko@linux.dev, kuba@kernel.org,
jiri@resnulli.us, edumazet@google.com, Kitszel, Przemyslaw,
richardcochran@gmail.com, donald.hunter@gmail.com,
linux-kernel@vger.kernel.org, Kubalewski, Arkadiusz,
andrew+netdev@lunn.ch, intel-wired-lan@lists.osuosl.org,
horms@kernel.org, Prathosh.Satish@microchip.com,
Nguyen, Anthony L, pabeni@redhat.com, davem@davemloft.net
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Grzegorz Nitka
> Sent: Thursday, April 30, 2026 11:43 AM
> To: netdev@vger.kernel.org
> Cc: Vecera, Ivan <ivecera@redhat.com>; vadim.fedorenko@linux.dev;
> kuba@kernel.org; jiri@resnulli.us; edumazet@google.com; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; richardcochran@gmail.com;
> donald.hunter@gmail.com; linux-kernel@vger.kernel.org; Kubalewski,
> Arkadiusz <arkadiusz.kubalewski@intel.com>; andrew+netdev@lunn.ch;
> intel-wired-lan@lists.osuosl.org; horms@kernel.org;
> Prathosh.Satish@microchip.com; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; pabeni@redhat.com; davem@davemloft.net
> Subject: [Intel-wired-lan] [PATCH v7 net-next 1/8] dpll: add generic
> DPLL type
>
> Add DPLL_TYPE_GENERIC to represent DPLL devices which do not fit the
> existing PPS or EEC classes.
>
> The UAPI type is intentionally generic. During netdev discussion,
> maintainers pointed out that introducing identifiers tied to a
> specific placement or single design does not scale across ASICs and
> vendors.
> The role of a DPLL is already inferable from the spawning driver, bus
> device, and pin topology, without encoding additional purpose-specific
> taxonomy in the type name.
>
> Using a generic type keeps the UAPI extensible and avoids premature
> naming that may become incorrect as new hardware topologies are
> exposed through the DPLL subsystem.
>
> Expose the new type through UAPI and netlink specification as
> "generic".
>
> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> ---
> Documentation/netlink/specs/dpll.yaml | 3 +++
> drivers/dpll/dpll_nl.c | 2 +-
> include/uapi/linux/dpll.h | 2 ++
> 3 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/netlink/specs/dpll.yaml
> b/Documentation/netlink/specs/dpll.yaml
> index 40465a3d7fc2..572cf7ae5f36 100644
> --- a/Documentation/netlink/specs/dpll.yaml
> +++ b/Documentation/netlink/specs/dpll.yaml
> @@ -138,6 +138,9 @@ definitions:
> -
> name: eec
> doc: dpll drives the Ethernet Equipment Clock
> + -
> + name: generic
> + doc: generic dpll type for devices outside PPS/EEC classes
> render-max: true
> -
> type: enum
> diff --git a/drivers/dpll/dpll_nl.c b/drivers/dpll/dpll_nl.c index
> 1e652340a5d7..9a3b70ea3ae0 100644
> --- a/drivers/dpll/dpll_nl.c
> +++ b/drivers/dpll/dpll_nl.c
> @@ -34,7 +34,7 @@ const struct nla_policy
> dpll_reference_sync_nl_policy[DPLL_A_PIN_STATE + 1] = { static const
> struct nla_policy dpll_device_id_get_nl_policy[DPLL_A_TYPE + 1] = {
> [DPLL_A_MODULE_NAME] = { .type = NLA_NUL_STRING, },
> [DPLL_A_CLOCK_ID] = { .type = NLA_U64, },
> - [DPLL_A_TYPE] = NLA_POLICY_RANGE(NLA_U32, 1, 2),
> + [DPLL_A_TYPE] = NLA_POLICY_RANGE(NLA_U32, 1, 3),
I think you need especial note if you manually edit "do not edit directly" file.
Isn't it ?
> };
>
> /* DPLL_CMD_DEVICE_GET - do */
> diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h
> index 871685f7c353..648553053cd8 100644
> --- a/include/uapi/linux/dpll.h
> +++ b/include/uapi/linux/dpll.h
> @@ -109,10 +109,12 @@ enum dpll_clock_quality_level {
> * enum dpll_type - type of dpll, valid values for DPLL_A_TYPE
> attribute
> * @DPLL_TYPE_PPS: dpll produces Pulse-Per-Second signal
> * @DPLL_TYPE_EEC: dpll drives the Ethernet Equipment Clock
> + * @DPLL_TYPE_GENERIC: generic dpll type for devices outside PPS/EEC
> + classes
> */
> enum dpll_type {
> DPLL_TYPE_PPS = 1,
> DPLL_TYPE_EEC,
> + DPLL_TYPE_GENERIC,
>
> /* private: */
> __DPLL_TYPE_MAX,
> --
> 2.39.3
^ permalink raw reply [flat|nested] 21+ messages in thread* RE: [Intel-wired-lan] [PATCH v7 net-next 1/8] dpll: add generic DPLL type
2026-04-30 11:49 ` [Intel-wired-lan] " Loktionov, Aleksandr
@ 2026-05-05 7:43 ` Nitka, Grzegorz
0 siblings, 0 replies; 21+ messages in thread
From: Nitka, Grzegorz @ 2026-05-05 7:43 UTC (permalink / raw)
To: Loktionov, Aleksandr, netdev@vger.kernel.org
Cc: Vecera, Ivan, vadim.fedorenko@linux.dev, kuba@kernel.org,
jiri@resnulli.us, edumazet@google.com, Kitszel, Przemyslaw,
richardcochran@gmail.com, donald.hunter@gmail.com,
linux-kernel@vger.kernel.org, Kubalewski, Arkadiusz,
andrew+netdev@lunn.ch, intel-wired-lan@lists.osuosl.org,
horms@kernel.org, Prathosh.Satish@microchip.com,
Nguyen, Anthony L, pabeni@redhat.com, davem@davemloft.net
> -----Original Message-----
> From: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
> Sent: Thursday, April 30, 2026 1:49 PM
> To: Nitka, Grzegorz <grzegorz.nitka@intel.com>; netdev@vger.kernel.org
> Cc: Vecera, Ivan <ivecera@redhat.com>; vadim.fedorenko@linux.dev;
> kuba@kernel.org; jiri@resnulli.us; edumazet@google.com; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; richardcochran@gmail.com;
> donald.hunter@gmail.com; linux-kernel@vger.kernel.org; Kubalewski,
> Arkadiusz <arkadiusz.kubalewski@intel.com>; andrew+netdev@lunn.ch;
> intel-wired-lan@lists.osuosl.org; horms@kernel.org;
> Prathosh.Satish@microchip.com; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; pabeni@redhat.com;
> davem@davemloft.net
> Subject: RE: [Intel-wired-lan] [PATCH v7 net-next 1/8] dpll: add generic DPLL
> type
>
>
>
> > -----Original Message-----
> > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> > Of Grzegorz Nitka
> > Sent: Thursday, April 30, 2026 11:43 AM
> > To: netdev@vger.kernel.org
> > Cc: Vecera, Ivan <ivecera@redhat.com>; vadim.fedorenko@linux.dev;
> > kuba@kernel.org; jiri@resnulli.us; edumazet@google.com; Kitszel,
> > Przemyslaw <przemyslaw.kitszel@intel.com>; richardcochran@gmail.com;
> > donald.hunter@gmail.com; linux-kernel@vger.kernel.org; Kubalewski,
> > Arkadiusz <arkadiusz.kubalewski@intel.com>; andrew+netdev@lunn.ch;
> > intel-wired-lan@lists.osuosl.org; horms@kernel.org;
> > Prathosh.Satish@microchip.com; Nguyen, Anthony L
> > <anthony.l.nguyen@intel.com>; pabeni@redhat.com;
> davem@davemloft.net
> > Subject: [Intel-wired-lan] [PATCH v7 net-next 1/8] dpll: add generic
> > DPLL type
> >
> > Add DPLL_TYPE_GENERIC to represent DPLL devices which do not fit the
> > existing PPS or EEC classes.
> >
> > The UAPI type is intentionally generic. During netdev discussion,
> > maintainers pointed out that introducing identifiers tied to a
> > specific placement or single design does not scale across ASICs and
> > vendors.
> > The role of a DPLL is already inferable from the spawning driver, bus
> > device, and pin topology, without encoding additional purpose-specific
> > taxonomy in the type name.
> >
> > Using a generic type keeps the UAPI extensible and avoids premature
> > naming that may become incorrect as new hardware topologies are
> > exposed through the DPLL subsystem.
> >
> > Expose the new type through UAPI and netlink specification as
> > "generic".
> >
> > Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> > ---
> > Documentation/netlink/specs/dpll.yaml | 3 +++
> > drivers/dpll/dpll_nl.c | 2 +-
> > include/uapi/linux/dpll.h | 2 ++
> > 3 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/netlink/specs/dpll.yaml
> > b/Documentation/netlink/specs/dpll.yaml
> > index 40465a3d7fc2..572cf7ae5f36 100644
> > --- a/Documentation/netlink/specs/dpll.yaml
> > +++ b/Documentation/netlink/specs/dpll.yaml
> > @@ -138,6 +138,9 @@ definitions:
> > -
> > name: eec
> > doc: dpll drives the Ethernet Equipment Clock
> > + -
> > + name: generic
> > + doc: generic dpll type for devices outside PPS/EEC classes
> > render-max: true
> > -
> > type: enum
> > diff --git a/drivers/dpll/dpll_nl.c b/drivers/dpll/dpll_nl.c index
> > 1e652340a5d7..9a3b70ea3ae0 100644
> > --- a/drivers/dpll/dpll_nl.c
> > +++ b/drivers/dpll/dpll_nl.c
> > @@ -34,7 +34,7 @@ const struct nla_policy
> > dpll_reference_sync_nl_policy[DPLL_A_PIN_STATE + 1] = { static const
> > struct nla_policy dpll_device_id_get_nl_policy[DPLL_A_TYPE + 1] = {
> > [DPLL_A_MODULE_NAME] = { .type = NLA_NUL_STRING, },
> > [DPLL_A_CLOCK_ID] = { .type = NLA_U64, },
> > - [DPLL_A_TYPE] = NLA_POLICY_RANGE(NLA_U32, 1, 2),
> > + [DPLL_A_TYPE] = NLA_POLICY_RANGE(NLA_U32, 1, 3),
>
> I think you need especial note if you manually edit "do not edit directly" file.
> Isn't it ?
>
This is not manual edit. This file is generated by ynl-regen.sh script.
And attached to this commit.
I believe this is the way how it should be processed.
Thanks!
Grzegorz
> > };
> >
> > /* DPLL_CMD_DEVICE_GET - do */
> > diff --git a/include/uapi/linux/dpll.h b/include/uapi/linux/dpll.h
> > index 871685f7c353..648553053cd8 100644
> > --- a/include/uapi/linux/dpll.h
> > +++ b/include/uapi/linux/dpll.h
> > @@ -109,10 +109,12 @@ enum dpll_clock_quality_level {
> > * enum dpll_type - type of dpll, valid values for DPLL_A_TYPE
> > attribute
> > * @DPLL_TYPE_PPS: dpll produces Pulse-Per-Second signal
> > * @DPLL_TYPE_EEC: dpll drives the Ethernet Equipment Clock
> > + * @DPLL_TYPE_GENERIC: generic dpll type for devices outside PPS/EEC
> > + classes
> > */
> > enum dpll_type {
> > DPLL_TYPE_PPS = 1,
> > DPLL_TYPE_EEC,
> > + DPLL_TYPE_GENERIC,
> >
> > /* private: */
> > __DPLL_TYPE_MAX,
> > --
> > 2.39.3
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v7 net-next 2/8] dpll: allow registering FW-identified pin with a different DPLL
2026-04-30 9:42 [PATCH v7 net-next 0/8] dpll/ice: Add generic DPLL type and full TX reference clock control for E825 Grzegorz Nitka
2026-04-30 9:42 ` [PATCH v7 net-next 1/8] dpll: add generic DPLL type Grzegorz Nitka
@ 2026-04-30 9:42 ` Grzegorz Nitka
2026-05-02 17:27 ` Jakub Kicinski
2026-04-30 9:42 ` [PATCH v7 net-next 3/8] dpll: extend pin notifier and netlink events with notification source ID Grzegorz Nitka
` (5 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Grzegorz Nitka @ 2026-04-30 9:42 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, intel-wired-lan, poros, richardcochran,
andrew+netdev, przemyslaw.kitszel, anthony.l.nguyen,
Prathosh.Satish, ivecera, jiri, arkadiusz.kubalewski,
vadim.fedorenko, donald.hunter, horms, pabeni, kuba, davem,
edumazet, Grzegorz Nitka, Jiri Pirko, Aleksandr Loktionov
Relax the (module, clock_id) equality requirement when registering a
pin identified by firmware (pin->fwnode). Some platforms associate a
FW-described pin with a DPLL instance that differs from the pin's
(module, clock_id) tuple. For such pins, permit registration without
requiring the strict match. Non-FW pins still require equality.
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
---
drivers/dpll/dpll_core.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index cbb635db4321..afe4552bdcd4 100644
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -883,11 +883,21 @@ dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin,
return -EINVAL;
mutex_lock(&dpll_lock);
- if (WARN_ON(!(dpll->module == pin->module &&
- dpll->clock_id == pin->clock_id)))
+
+ /*
+ * For pins identified via firmware (pin->fwnode), allow registration
+ * even if the pin's (module, clock_id) differs from the target DPLL.
+ * For non-fwnode pins, require a strict (module, clock_id) match.
+ */
+ if (!pin->fwnode &&
+ WARN_ON_ONCE(dpll->module != pin->module ||
+ dpll->clock_id != pin->clock_id)) {
ret = -EINVAL;
- else
- ret = __dpll_pin_register(dpll, pin, ops, priv, NULL);
+ goto out_unlock;
+ }
+
+ ret = __dpll_pin_register(dpll, pin, ops, priv, NULL);
+out_unlock:
mutex_unlock(&dpll_lock);
return ret;
--
2.39.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v7 net-next 2/8] dpll: allow registering FW-identified pin with a different DPLL
2026-04-30 9:42 ` [PATCH v7 net-next 2/8] dpll: allow registering FW-identified pin with a different DPLL Grzegorz Nitka
@ 2026-05-02 17:27 ` Jakub Kicinski
0 siblings, 0 replies; 21+ messages in thread
From: Jakub Kicinski @ 2026-05-02 17:27 UTC (permalink / raw)
To: Grzegorz Nitka
Cc: netdev, linux-kernel, intel-wired-lan, poros, richardcochran,
andrew+netdev, przemyslaw.kitszel, anthony.l.nguyen,
Prathosh.Satish, ivecera, jiri, arkadiusz.kubalewski,
vadim.fedorenko, donald.hunter, horms, pabeni, davem, edumazet,
Jiri Pirko, Aleksandr Loktionov
On Thu, 30 Apr 2026 11:42:32 +0200 Grzegorz Nitka wrote:
> Relax the (module, clock_id) equality requirement when registering a
> pin identified by firmware (pin->fwnode). Some platforms associate a
> FW-described pin with a DPLL instance that differs from the pin's
> (module, clock_id) tuple. For such pins, permit registration without
> requiring the strict match. Non-FW pins still require equality.
AI asks what prevents the modules from disappearing:
Does this relaxed check expose pin->module to a use-after-free during
netlink queries?
If module A registers a firmware-described pin allocated by module B,
they will have different module pointers.
Because fwnode_dpll_pin_find() increases the pin's refcount but does
not take a reference to module B via try_module_get(), it appears module B
could be unloaded while module A still holds an active reference to the pin.
When module B unloads, its struct module memory is freed, leaving
pin->module as a dangling pointer.
A subsequent user-space Netlink query using DPLL_CMD_PIN_GET iterates over
the registered pins and calls nla_put_string() with module_name(pin->module),
which would dereference the freed module memory.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v7 net-next 3/8] dpll: extend pin notifier and netlink events with notification source ID
2026-04-30 9:42 [PATCH v7 net-next 0/8] dpll/ice: Add generic DPLL type and full TX reference clock control for E825 Grzegorz Nitka
2026-04-30 9:42 ` [PATCH v7 net-next 1/8] dpll: add generic DPLL type Grzegorz Nitka
2026-04-30 9:42 ` [PATCH v7 net-next 2/8] dpll: allow registering FW-identified pin with a different DPLL Grzegorz Nitka
@ 2026-04-30 9:42 ` Grzegorz Nitka
2026-05-02 17:29 ` Jakub Kicinski
2026-05-02 17:31 ` Jakub Kicinski
2026-04-30 9:42 ` [PATCH v7 net-next 4/8] dpll: zl3073x: allow SyncE_Ref pin state change Grzegorz Nitka
` (4 subsequent siblings)
7 siblings, 2 replies; 21+ messages in thread
From: Grzegorz Nitka @ 2026-04-30 9:42 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, intel-wired-lan, poros, richardcochran,
andrew+netdev, przemyslaw.kitszel, anthony.l.nguyen,
Prathosh.Satish, ivecera, jiri, arkadiusz.kubalewski,
vadim.fedorenko, donald.hunter, horms, pabeni, kuba, davem,
edumazet, Grzegorz Nitka, Aleksandr Loktionov
Extend the DPLL pin notification API to include a source identifier
indicating where the notification originates. This allows notifier
consumers and netlink listeners to distinguish between notifications
coming from an associated DPLL instance, a parent pin, or the pin
itself.
A new field, src_id, is added to struct dpll_pin_notifier_info and is
passed through all pin-related notification paths. Callers of
dpll_pin_notify() are updated to provide a meaningful source identifier
based on their context:
- pin registration/unregistration uses the DPLL's clock_id,
- pin-on-pin operations use the parent pin's clock_id,
- pin changes use the pin's own clock_id.
As introduced in the commit ("dpll: allow registering FW-identified pin
with a different DPLL"), it is possible to share the same physical pin
via firmware description (fwnode) with DPLL objects from different
kernel modules. This means that a given pin can be registered multiple
times.
Driver such as ICE (E825 devices) rely on this mechanism when listening
for the event where a shared-fwnode pin appears, while avoiding reacting
to events triggered by their own registration logic.
This change only extends the notification metadata and does not alter
existing semantics for drivers that do not use the new field.
Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
---
drivers/dpll/dpll_core.c | 14 ++++++++------
drivers/dpll/dpll_core.h | 3 ++-
drivers/dpll/dpll_netlink.c | 10 +++++-----
drivers/dpll/dpll_netlink.h | 4 ++--
include/linux/dpll.h | 1 +
5 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
index afe4552bdcd4..857bd02da7ba 100644
--- a/drivers/dpll/dpll_core.c
+++ b/drivers/dpll/dpll_core.c
@@ -71,7 +71,8 @@ void dpll_device_notify(struct dpll_device *dpll, unsigned long action)
call_dpll_notifiers(action, &info);
}
-void dpll_pin_notify(struct dpll_pin *pin, unsigned long action)
+void dpll_pin_notify(struct dpll_pin *pin, u64 src_clock_id,
+ unsigned long action)
{
struct dpll_pin_notifier_info info = {
.pin = pin,
@@ -80,6 +81,7 @@ void dpll_pin_notify(struct dpll_pin *pin, unsigned long action)
.clock_id = pin->clock_id,
.fwnode = pin->fwnode,
.prop = &pin->prop,
+ .src_clock_id = src_clock_id,
};
call_dpll_notifiers(action, &info);
@@ -847,7 +849,7 @@ __dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin,
if (ret)
goto ref_pin_del;
xa_set_mark(&dpll_pin_xa, pin->id, DPLL_REGISTERED);
- dpll_pin_create_ntf(pin);
+ dpll_pin_create_ntf(pin, dpll->clock_id);
return ret;
@@ -949,7 +951,7 @@ void dpll_pin_unregister(struct dpll_device *dpll, struct dpll_pin *pin,
return;
mutex_lock(&dpll_lock);
- dpll_pin_delete_ntf(pin);
+ dpll_pin_delete_ntf(pin, dpll->clock_id);
__dpll_pin_unregister(dpll, pin, ops, priv, NULL);
mutex_unlock(&dpll_lock);
}
@@ -995,7 +997,7 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent, struct dpll_pin *pin,
stop = i;
goto dpll_unregister;
}
- dpll_pin_create_ntf(pin);
+ dpll_pin_create_ntf(pin, parent->clock_id);
}
mutex_unlock(&dpll_lock);
@@ -1006,7 +1008,7 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent, struct dpll_pin *pin,
if (i < stop) {
__dpll_pin_unregister(ref->dpll, pin, ops, priv,
parent);
- dpll_pin_delete_ntf(pin);
+ dpll_pin_delete_ntf(pin, parent->clock_id);
}
dpll_xa_ref_pin_del(&pin->parent_refs, parent, ops, priv, pin);
unlock:
@@ -1032,7 +1034,7 @@ void dpll_pin_on_pin_unregister(struct dpll_pin *parent, struct dpll_pin *pin,
unsigned long i;
mutex_lock(&dpll_lock);
- dpll_pin_delete_ntf(pin);
+ dpll_pin_delete_ntf(pin, parent->clock_id);
dpll_xa_ref_pin_del(&pin->parent_refs, parent, ops, priv, pin);
xa_for_each(&pin->dpll_refs, i, ref)
__dpll_pin_unregister(ref->dpll, pin, ops, priv, parent);
diff --git a/drivers/dpll/dpll_core.h b/drivers/dpll/dpll_core.h
index 71ac88ef2017..92cb919317eb 100644
--- a/drivers/dpll/dpll_core.h
+++ b/drivers/dpll/dpll_core.h
@@ -98,6 +98,7 @@ extern struct xarray dpll_pin_xa;
extern struct mutex dpll_lock;
void dpll_device_notify(struct dpll_device *dpll, unsigned long action);
-void dpll_pin_notify(struct dpll_pin *pin, unsigned long action);
+void dpll_pin_notify(struct dpll_pin *pin, u64 src_clock_id,
+ unsigned long action);
#endif
diff --git a/drivers/dpll/dpll_netlink.c b/drivers/dpll/dpll_netlink.c
index af7ce62ec55c..aeae5afe71c8 100644
--- a/drivers/dpll/dpll_netlink.c
+++ b/drivers/dpll/dpll_netlink.c
@@ -888,21 +888,21 @@ dpll_pin_event_send(enum dpll_cmd event, struct dpll_pin *pin)
return ret;
}
-int dpll_pin_create_ntf(struct dpll_pin *pin)
+int dpll_pin_create_ntf(struct dpll_pin *pin, u64 src_clock_id)
{
- dpll_pin_notify(pin, DPLL_PIN_CREATED);
+ dpll_pin_notify(pin, src_clock_id, DPLL_PIN_CREATED);
return dpll_pin_event_send(DPLL_CMD_PIN_CREATE_NTF, pin);
}
-int dpll_pin_delete_ntf(struct dpll_pin *pin)
+int dpll_pin_delete_ntf(struct dpll_pin *pin, u64 src_clock_id)
{
- dpll_pin_notify(pin, DPLL_PIN_DELETED);
+ dpll_pin_notify(pin, src_clock_id, DPLL_PIN_DELETED);
return dpll_pin_event_send(DPLL_CMD_PIN_DELETE_NTF, pin);
}
int __dpll_pin_change_ntf(struct dpll_pin *pin)
{
- dpll_pin_notify(pin, DPLL_PIN_CHANGED);
+ dpll_pin_notify(pin, pin->clock_id, DPLL_PIN_CHANGED);
return dpll_pin_event_send(DPLL_CMD_PIN_CHANGE_NTF, pin);
}
diff --git a/drivers/dpll/dpll_netlink.h b/drivers/dpll/dpll_netlink.h
index dd28b56d27c5..89fef266392f 100644
--- a/drivers/dpll/dpll_netlink.h
+++ b/drivers/dpll/dpll_netlink.h
@@ -8,8 +8,8 @@ int dpll_device_create_ntf(struct dpll_device *dpll);
int dpll_device_delete_ntf(struct dpll_device *dpll);
-int dpll_pin_create_ntf(struct dpll_pin *pin);
+int dpll_pin_create_ntf(struct dpll_pin *pin, u64 src_clock_id);
-int dpll_pin_delete_ntf(struct dpll_pin *pin);
+int dpll_pin_delete_ntf(struct dpll_pin *pin, u64 src_clock_id);
int __dpll_pin_change_ntf(struct dpll_pin *pin);
diff --git a/include/linux/dpll.h b/include/linux/dpll.h
index b7277a8b484d..299cef38b657 100644
--- a/include/linux/dpll.h
+++ b/include/linux/dpll.h
@@ -212,6 +212,7 @@ struct dpll_pin_notifier_info {
u64 clock_id;
const struct fwnode_handle *fwnode;
const struct dpll_pin_properties *prop;
+ u64 src_clock_id;
};
#if IS_ENABLED(CONFIG_DPLL)
--
2.39.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v7 net-next 3/8] dpll: extend pin notifier and netlink events with notification source ID
2026-04-30 9:42 ` [PATCH v7 net-next 3/8] dpll: extend pin notifier and netlink events with notification source ID Grzegorz Nitka
@ 2026-05-02 17:29 ` Jakub Kicinski
2026-05-02 17:31 ` Jakub Kicinski
1 sibling, 0 replies; 21+ messages in thread
From: Jakub Kicinski @ 2026-05-02 17:29 UTC (permalink / raw)
To: Grzegorz Nitka
Cc: netdev, linux-kernel, intel-wired-lan, poros, richardcochran,
andrew+netdev, przemyslaw.kitszel, anthony.l.nguyen,
Prathosh.Satish, ivecera, jiri, arkadiusz.kubalewski,
vadim.fedorenko, donald.hunter, horms, pabeni, davem, edumazet,
Aleksandr Loktionov
On Thu, 30 Apr 2026 11:42:33 +0200 Grzegorz Nitka wrote:
>
> -int dpll_pin_delete_ntf(struct dpll_pin *pin)
> +int dpll_pin_delete_ntf(struct dpll_pin *pin, u64 src_clock_id)
double space
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v7 net-next 3/8] dpll: extend pin notifier and netlink events with notification source ID
2026-04-30 9:42 ` [PATCH v7 net-next 3/8] dpll: extend pin notifier and netlink events with notification source ID Grzegorz Nitka
2026-05-02 17:29 ` Jakub Kicinski
@ 2026-05-02 17:31 ` Jakub Kicinski
1 sibling, 0 replies; 21+ messages in thread
From: Jakub Kicinski @ 2026-05-02 17:31 UTC (permalink / raw)
To: Grzegorz Nitka
Cc: netdev, linux-kernel, intel-wired-lan, poros, richardcochran,
andrew+netdev, przemyslaw.kitszel, anthony.l.nguyen,
Prathosh.Satish, ivecera, jiri, arkadiusz.kubalewski,
vadim.fedorenko, donald.hunter, horms, pabeni, davem, edumazet,
Aleksandr Loktionov
On Thu, 30 Apr 2026 11:42:33 +0200 Grzegorz Nitka wrote:
> Subject: [PATCH v7 net-next 3/8] dpll: extend pin notifier and netlink events with notification source ID
As AI points out you're only adding this for the internal notification,
but the commit title mentions netlink too. If we don't need it in
netlink we can just drop the references in the commit msg?
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v7 net-next 4/8] dpll: zl3073x: allow SyncE_Ref pin state change
2026-04-30 9:42 [PATCH v7 net-next 0/8] dpll/ice: Add generic DPLL type and full TX reference clock control for E825 Grzegorz Nitka
` (2 preceding siblings ...)
2026-04-30 9:42 ` [PATCH v7 net-next 3/8] dpll: extend pin notifier and netlink events with notification source ID Grzegorz Nitka
@ 2026-04-30 9:42 ` Grzegorz Nitka
2026-05-02 17:33 ` Jakub Kicinski
2026-04-30 9:42 ` [PATCH v7 net-next 5/8] ice: introduce TXC DPLL device and TX ref clock pin framework for E825 Grzegorz Nitka
` (3 subsequent siblings)
7 siblings, 1 reply; 21+ messages in thread
From: Grzegorz Nitka @ 2026-04-30 9:42 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, intel-wired-lan, poros, richardcochran,
andrew+netdev, przemyslaw.kitszel, anthony.l.nguyen,
Prathosh.Satish, ivecera, jiri, arkadiusz.kubalewski,
vadim.fedorenko, donald.hunter, horms, pabeni, kuba, davem,
edumazet, Grzegorz Nitka, Aleksandr Loktionov
The SyncE_Ref pin may operate as either an active or inactive reference
depending on board design and system configuration. Some platforms need
to disable the SyncE reference dynamically (e.g., when selecting a
different recovered clock input). The hardware supports toggling this
pin, therefore advertise the STATE_CAN_CHANGE capability.
Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
---
drivers/dpll/zl3073x/prop.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/dpll/zl3073x/prop.c b/drivers/dpll/zl3073x/prop.c
index ac9d41d0f978..acd7061a741a 100644
--- a/drivers/dpll/zl3073x/prop.c
+++ b/drivers/dpll/zl3073x/prop.c
@@ -215,6 +215,15 @@ struct zl3073x_pin_props *zl3073x_pin_props_get(struct zl3073x_dev *zldev,
props->dpll_props.type = DPLL_PIN_TYPE_GNSS;
+ /*
+ * The SyncE_Ref pin supports enabling/disabling dynamically.
+ * Some platforms may choose to expose this through firmware
+ * configuration later. For now, advertise this capability
+ * universally since the hardware allows state toggling.
+ */
+ props->dpll_props.capabilities |=
+ DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE;
+
/* The output pin phase adjustment granularity equals half of
* the synth frequency count.
*/
--
2.39.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v7 net-next 4/8] dpll: zl3073x: allow SyncE_Ref pin state change
2026-04-30 9:42 ` [PATCH v7 net-next 4/8] dpll: zl3073x: allow SyncE_Ref pin state change Grzegorz Nitka
@ 2026-05-02 17:33 ` Jakub Kicinski
0 siblings, 0 replies; 21+ messages in thread
From: Jakub Kicinski @ 2026-05-02 17:33 UTC (permalink / raw)
To: grzegorz.nitka
Cc: Jakub Kicinski, netdev, ivecera, vadim.fedorenko, jiri, edumazet,
przemyslaw.kitszel, richardcochran, donald.hunter, linux-kernel,
arkadiusz.kubalewski, andrew+netdev, intel-wired-lan, horms,
Prathosh.Satish, anthony.l.nguyen, pabeni, davem
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
dpll: zl3073x: allow SyncE_Ref pin state change
This patch advertises DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE on the
output-direction branch of zl3073x_pin_props_get() with the stated intent
of allowing dynamic enable/disable of the SyncE_Ref pin.
> diff --git a/drivers/dpll/zl3073x/prop.c b/drivers/dpll/zl3073x/prop.c
> index ac9d41d0f978..acd7061a741a 100644
> --- a/drivers/dpll/zl3073x/prop.c
> +++ b/drivers/dpll/zl3073x/prop.c
> @@ -215,6 +215,15 @@ struct zl3073x_pin_props *zl3073x_pin_props_get(struct zl3073x_dev *zldev,
>
> props->dpll_props.type = DPLL_PIN_TYPE_GNSS;
>
> + /*
> + * The SyncE_Ref pin supports enabling/disabling dynamically.
> + * Some platforms may choose to expose this through firmware
> + * configuration later. For now, advertise this capability
> + * universally since the hardware allows state toggling.
> + */
> + props->dpll_props.capabilities |=
> + DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE;
Is this capability actually supported by the output pin ops?
Looking at drivers/dpll/zl3073x/dpll.c, the output pin ops vector does
not install a .state_on_dpll_set callback:
static const struct dpll_pin_ops zl3073x_dpll_output_pin_ops = {
.direction_get = zl3073x_dpll_pin_direction_get,
.esync_get = zl3073x_dpll_output_pin_esync_get,
.esync_set = zl3073x_dpll_output_pin_esync_set,
.frequency_get = zl3073x_dpll_output_pin_frequency_get,
.frequency_set = zl3073x_dpll_output_pin_frequency_set,
.phase_adjust_get = zl3073x_dpll_output_pin_phase_adjust_get,
.phase_adjust_set = zl3073x_dpll_output_pin_phase_adjust_set,
.state_on_dpll_get = zl3073x_dpll_output_pin_state_on_dpll_get,
};
and the corresponding getter hardcodes the state to CONNECTED with no
hardware read:
zl3073x_dpll_output_pin_state_on_dpll_get(...)
{
/* If the output pin is registered then it is always connected */
*state = DPLL_PIN_STATE_CONNECTED;
return 0;
}
On the netlink side, dpll_pin_state_set() first checks the capability
bit and then requires the setter to exist:
if (!(DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE & pin->prop.capabilities)) {
NL_SET_ERR_MSG(extack, "state changing is not allowed");
return -EOPNOTSUPP;
}
...
if (!ops->state_on_dpll_set)
return -EOPNOTSUPP;
With this patch, the capability check now passes for every output pin,
but the operation then fails with -EOPNOTSUPP because no setter is
provided, and the reported state will remain CONNECTED regardless of
what userspace requested.
Should this series also add a .state_on_dpll_set callback (and a
matching hardware path, for example via zl3073x_out_state_set or the
output-enable mailbox) before advertising the capability, so the commit
subject "allow SyncE_Ref pin state change" can actually be honored?
Separately, should the scope of this change match the commit message?
The subject and the first sentence of the new comment describe a change
scoped to the SyncE_Ref pin:
* The SyncE_Ref pin supports enabling/disabling dynamically.
but the assignment lives in the generic else-branch of
zl3073x_pin_props_get() that runs for every output pin (SMA outputs,
1PPS outputs, internal-oscillator outputs, MUX outputs, etc.), and it
executes before the fwnode "connection-type" parsing below can classify
the pin as ext/gnss/int/synce/mux. The last sentence of the same
comment acknowledges this:
* universally since the hardware allows state toggling.
Would it be clearer to either gate the flag on the SyncE_Ref pin
specifically (e.g., via firmware-node property, as the comment hints
at), or to update the subject and leading comment sentence to reflect
that the capability is being advertised for all output pins?
> + /*
> + * The SyncE_Ref pin supports enabling/disabling dynamically.
> + * Some platforms may choose to expose this through firmware
> + * configuration later. For now, advertise this capability
> + * universally since the hardware allows state toggling.
> + */
This isn't a bug, but the indentation of the comment block is
inconsistent. The opening "/*" is prefixed with one tab followed by
seven spaces, while the " *" continuation lines and the closing " */"
are prefixed with two tabs, so the opening "/" does not align with the
continuation asterisks. Adjacent comment blocks in the same function
(for example "The output pin phase adjustment granularity...") use pure
tab alignment.
> + props->dpll_props.capabilities |=
> + DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE;
This isn't a bug, but props was just obtained from kzalloc_obj() and
nothing in this else-branch writes to capabilities before this point.
The symmetric input branch immediately above uses plain "=":
props->dpll_props.capabilities =
DPLL_PIN_CAPABILITIES_PRIORITY_CAN_CHANGE |
DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE;
Would "=" be more consistent here, since there are no pre-existing bits
to merge with?
--
pw-bot: cr
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v7 net-next 5/8] ice: introduce TXC DPLL device and TX ref clock pin framework for E825
2026-04-30 9:42 [PATCH v7 net-next 0/8] dpll/ice: Add generic DPLL type and full TX reference clock control for E825 Grzegorz Nitka
` (3 preceding siblings ...)
2026-04-30 9:42 ` [PATCH v7 net-next 4/8] dpll: zl3073x: allow SyncE_Ref pin state change Grzegorz Nitka
@ 2026-04-30 9:42 ` Grzegorz Nitka
2026-04-30 11:46 ` [Intel-wired-lan] " Loktionov, Aleksandr
2026-05-02 17:33 ` Jakub Kicinski
2026-04-30 9:42 ` [PATCH v7 net-next 6/8] ice: implement CPI support for E825C Grzegorz Nitka
` (2 subsequent siblings)
7 siblings, 2 replies; 21+ messages in thread
From: Grzegorz Nitka @ 2026-04-30 9:42 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, intel-wired-lan, poros, richardcochran,
andrew+netdev, przemyslaw.kitszel, anthony.l.nguyen,
Prathosh.Satish, ivecera, jiri, arkadiusz.kubalewski,
vadim.fedorenko, donald.hunter, horms, pabeni, kuba, davem,
edumazet, Grzegorz Nitka
E825 devices provide a dedicated TX clock (TXC) domain which may be
driven by multiple reference clock sources, including external board
references and port-derived SyncE. To support future TX clock control
and observability through the Linux DPLL subsystem, introduce a
separate TXC DPLL device (of DPLL_TYPE_GENERIC) and a framework for
representing TX reference clock inputs.
This change adds a new internal DPLL pin type (TXCLK) and registers
TX reference clock pins for E825-based devices:
- EXT_EREF0: a board-level external electrical reference
- SYNCE: a port-derived SyncE reference described via firmware nodes
The TXC DPLL device is created and managed alongside the existing
PPS and EEC DPLL instances. TXCLK pins are registered directly or
deferred via a notifier when backed by fwnode-described pins.
A per-pin attribute encodes the TX reference source associated with
each TXCLK pin.
At this stage, TXCLK pin state callbacks and TXC DPLL lock status
reporting are implemented as placeholders. Pin state getters always
return DISCONNECTED, and the TXC DPLL is initialized in the UNLOCKED
state. No hardware configuration or TX reference switching is
performed yet.
This patch establishes the structural groundwork required for
hardware-backed TX reference selection, verification, and
synchronization status reporting, which will be implemented in
subsequent patches.
Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
---
drivers/net/ethernet/intel/ice/ice_dpll.c | 296 ++++++++++++++++++--
drivers/net/ethernet/intel/ice/ice_dpll.h | 6 +
drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 7 +
3 files changed, 286 insertions(+), 23 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
index 62f75701d652..d839b50187ba 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
@@ -19,6 +19,11 @@
#define ICE_DPLL_SW_PIN_INPUT_BASE_QSFP 6
#define ICE_DPLL_SW_PIN_OUTPUT_BASE 0
+#define E825_EXT_EREF_PIN_IDX 0
+#define E825_EXT_SYNCE_PIN_IDX 1
+#define E825_RCLK_PARENT_0_PIN_IDX 0
+#define E825_RCLK_PARENT_1_PIN_IDX 1
+
#define ICE_DPLL_PIN_SW_INPUT_ABS(in_idx) \
(ICE_DPLL_SW_PIN_INPUT_BASE_SFP + (in_idx))
@@ -57,6 +62,7 @@
* @ICE_DPLL_PIN_TYPE_OUTPUT: output pin
* @ICE_DPLL_PIN_TYPE_RCLK_INPUT: recovery clock input pin
* @ICE_DPLL_PIN_TYPE_SOFTWARE: software controlled SMA/U.FL pins
+ * @ICE_DPLL_PIN_TYPE_TXCLK: transmit clock reference input pin
*/
enum ice_dpll_pin_type {
ICE_DPLL_PIN_INVALID,
@@ -64,6 +70,7 @@ enum ice_dpll_pin_type {
ICE_DPLL_PIN_TYPE_OUTPUT,
ICE_DPLL_PIN_TYPE_RCLK_INPUT,
ICE_DPLL_PIN_TYPE_SOFTWARE,
+ ICE_DPLL_PIN_TYPE_TXCLK,
};
static const char * const pin_type_name[] = {
@@ -71,10 +78,13 @@ static const char * const pin_type_name[] = {
[ICE_DPLL_PIN_TYPE_OUTPUT] = "output",
[ICE_DPLL_PIN_TYPE_RCLK_INPUT] = "rclk-input",
[ICE_DPLL_PIN_TYPE_SOFTWARE] = "software",
+ [ICE_DPLL_PIN_TYPE_TXCLK] = "txclk-input",
};
static const char * const ice_dpll_sw_pin_sma[] = { "SMA1", "SMA2" };
static const char * const ice_dpll_sw_pin_ufl[] = { "U.FL1", "U.FL2" };
+static const char * const ice_dpll_ext_eref_pin = "EXT_EREF0";
+static const char * const ice_dpll_fwnode_ext_synce = "clk_ref_synce";
static const struct dpll_pin_frequency ice_esync_range[] = {
DPLL_PIN_FREQUENCY_RANGE(0, DPLL_PIN_FREQUENCY_1_HZ),
@@ -2517,12 +2527,75 @@ ice_dpll_rclk_state_on_pin_get(const struct dpll_pin *pin, void *pin_priv,
return ret;
}
+/**
+ * ice_dpll_txclk_state_on_dpll_set - set a state on TX clk pin
+ * @pin: pointer to a pin
+ * @pin_priv: private data pointer passed on pin registration
+ * @dpll: registered dpll pointer
+ * @dpll_priv: private data pointer passed on dpll registration
+ * @state: state to be set on pin
+ * @extack: error reporting
+ *
+ * Dpll subsystem callback, set a state of a Tx reference clock pin
+ *
+ * Return:
+ * * negative - failure
+ */
+static int
+ice_dpll_txclk_state_on_dpll_set(const struct dpll_pin *pin, void *pin_priv,
+ const struct dpll_device *dpll,
+ void *dpll_priv, enum dpll_pin_state state,
+ struct netlink_ext_ack *extack)
+{
+ /*
+ * TODO: set HW accordingly to selected TX reference clock.
+ * To be added in the follow up patches.
+ */
+ return -EOPNOTSUPP;
+}
+
+/**
+ * ice_dpll_txclk_state_on_dpll_get - get a state of Tx clk reference pin
+ * @pin: pointer to a pin
+ * @pin_priv: private data pointer passed on pin registration
+ * @dpll: registered dpll pointer
+ * @dpll_priv: private data pointer passed on dpll registration
+ * @state: on success holds pin state on parent pin
+ * @extack: error reporting
+ *
+ * dpll subsystem callback, get a state of a TX clock reference pin.
+ *
+ * Return:
+ * * 0 - success
+ */
+static int
+ice_dpll_txclk_state_on_dpll_get(const struct dpll_pin *pin, void *pin_priv,
+ const struct dpll_device *dpll,
+ void *dpll_priv,
+ enum dpll_pin_state *state,
+ struct netlink_ext_ack *extack)
+{
+ /*
+ * TODO: query HW status to determine if the TX reference is selected.
+ * To be added in the follow up patches.
+ */
+ *state = DPLL_PIN_STATE_DISCONNECTED;
+
+ return 0;
+}
+
static const struct dpll_pin_ops ice_dpll_rclk_ops = {
.state_on_pin_set = ice_dpll_rclk_state_on_pin_set,
.state_on_pin_get = ice_dpll_rclk_state_on_pin_get,
.direction_get = ice_dpll_input_direction,
};
+static const struct dpll_pin_ops ice_dpll_txclk_ops = {
+ .state_on_dpll_set = ice_dpll_txclk_state_on_dpll_set,
+ .state_on_dpll_get = ice_dpll_txclk_state_on_dpll_get,
+ .direction_get = ice_dpll_input_direction,
+};
+
static const struct dpll_pin_ops ice_dpll_pin_sma_ops = {
.state_on_dpll_set = ice_dpll_sma_pin_state_set,
.state_on_dpll_get = ice_dpll_sw_pin_state_get,
@@ -3023,9 +3096,13 @@ ice_dpll_unregister_pins(struct dpll_device *dpll, struct ice_dpll_pin *pins,
{
int i;
- for (i = 0; i < count; i++)
- if (!pins[i].hidden)
- dpll_pin_unregister(dpll, pins[i].pin, ops, &pins[i]);
+ for (i = 0; i < count; i++) {
+ if (pins[i].hidden)
+ continue;
+ if (IS_ERR_OR_NULL(pins[i].pin))
+ continue;
+ dpll_pin_unregister(dpll, pins[i].pin, ops, &pins[i]);
+ }
}
/**
@@ -3199,19 +3276,40 @@ static bool ice_dpll_is_fwnode_pin(struct ice_dpll_pin *pin)
return !IS_ERR_OR_NULL(pin->fwnode);
}
+static bool ice_dpll_fwnode_eq(const struct fwnode_handle *a,
+ const struct fwnode_handle *b)
+{
+ return a && b && a == b;
+}
+
static void ice_dpll_pin_notify_work(struct work_struct *work)
{
struct ice_dpll_pin_work *w = container_of(work,
struct ice_dpll_pin_work,
work);
struct ice_dpll_pin *pin, *parent = w->pin;
+ bool is_tx_synce_parent = false;
struct ice_pf *pf = parent->pf;
+ bool is_rclk_parent = false;
int ret;
wait_for_completion(&pf->dplls.dpll_init);
if (!test_bit(ICE_FLAG_DPLL, pf->flags))
goto out; /* DPLL initialization failed */
+ /* Decide which parent we are handling, defensively checking FWNs */
+ is_rclk_parent =
+ ice_dpll_fwnode_eq(parent->fwnode,
+ pf->dplls.inputs[E825_RCLK_PARENT_0_PIN_IDX].fwnode) ||
+ ice_dpll_fwnode_eq(parent->fwnode,
+ pf->dplls.inputs[E825_RCLK_PARENT_1_PIN_IDX].fwnode);
+
+ is_tx_synce_parent =
+ ice_dpll_fwnode_eq(parent->fwnode,
+ pf->dplls.txclks[E825_EXT_SYNCE_PIN_IDX].fwnode);
+ if (!is_rclk_parent && !is_tx_synce_parent)
+ goto out;
+
switch (w->action) {
case DPLL_PIN_CREATED:
if (!IS_ERR_OR_NULL(parent->pin)) {
@@ -3228,16 +3326,28 @@ static void ice_dpll_pin_notify_work(struct work_struct *work)
goto out;
}
- /* Register rclk pin */
- pin = &pf->dplls.rclk;
- ret = dpll_pin_on_pin_register(parent->pin, pin->pin,
- &ice_dpll_rclk_ops, pin);
- if (ret) {
- dev_err(ice_pf_to_dev(pf),
- "Failed to register pin: %pe\n", ERR_PTR(ret));
- dpll_pin_put(parent->pin, &parent->tracker);
- parent->pin = NULL;
- goto out;
+ if (is_rclk_parent) {
+ /* Register rclk pin via on-pin relationship */
+ pin = &pf->dplls.rclk;
+ ret = dpll_pin_on_pin_register(parent->pin, pin->pin,
+ &ice_dpll_rclk_ops, pin);
+ if (ret) {
+ dev_err(ice_pf_to_dev(pf),
+ "RCLK pin register failed: %pe\n",
+ ERR_PTR(ret));
+ goto drop_parent_ref;
+ }
+ } else if (is_tx_synce_parent) {
+ /* Register TX-CLK SYNCE pin directly to TXC DPLL */
+ pin = &pf->dplls.txclks[E825_EXT_SYNCE_PIN_IDX];
+ ret = dpll_pin_register(pf->dplls.txc.dpll, pin->pin,
+ &ice_dpll_txclk_ops, pin);
+ if (ret) {
+ dev_err(ice_pf_to_dev(pf),
+ "TX SYNCE pin register failed: %pe\n",
+ ERR_PTR(ret));
+ goto drop_parent_ref;
+ }
}
break;
case DPLL_PIN_DELETED:
@@ -3246,11 +3356,18 @@ static void ice_dpll_pin_notify_work(struct work_struct *work)
goto out;
}
- /* Unregister rclk pin */
- pin = &pf->dplls.rclk;
- dpll_pin_on_pin_unregister(parent->pin, pin->pin,
- &ice_dpll_rclk_ops, pin);
-
+ if (is_rclk_parent) {
+ /* Unregister rclk pin */
+ pin = &pf->dplls.rclk;
+ dpll_pin_on_pin_unregister(parent->pin, pin->pin,
+ &ice_dpll_rclk_ops, pin);
+ } else if (is_tx_synce_parent) {
+ /* Unregister TX-CLK SYNCE pin from TXC DPLL */
+ pin = &pf->dplls.txclks[E825_EXT_SYNCE_PIN_IDX];
+ dpll_pin_unregister(pf->dplls.txc.dpll, pin->pin,
+ &ice_dpll_txclk_ops, pin);
+ }
+drop_parent_ref:
/* Drop fwnode pin reference */
dpll_pin_put(parent->pin, &parent->tracker);
parent->pin = NULL;
@@ -3276,6 +3393,12 @@ static int ice_dpll_pin_notify(struct notifier_block *nb, unsigned long action,
if (pin->fwnode != info->fwnode)
return NOTIFY_DONE; /* Not this pin */
+ /* Ignore notification which are the outcome of internal pin
+ * registration/unregistration calls - synce pin case.
+ */
+ if (info->src_clock_id == pin->pf->dplls.clock_id)
+ return NOTIFY_DONE;
+
work = kzalloc_obj(*work);
if (!work)
return NOTIFY_DONE;
@@ -3401,6 +3524,19 @@ ice_dpll_deinit_fwnode_pins(struct ice_pf *pf, struct ice_dpll_pin *pins,
destroy_workqueue(pf->dplls.wq);
}
+static int ice_dpll_deinit_txclk_pins(struct ice_pf *pf)
+{
+ struct ice_dpll_pin *synce_pin = &pf->dplls.txclks[E825_EXT_SYNCE_PIN_IDX];
+ struct ice_dpll *dt = &pf->dplls.txc;
+
+ ice_dpll_unregister_pins(dt->dpll, pf->dplls.txclks,
+ &ice_dpll_txclk_ops,
+ ARRAY_SIZE(pf->dplls.txclks));
+ ice_dpll_release_pins(&pf->dplls.txclks[E825_EXT_EREF_PIN_IDX], 1);
+ ice_dpll_deinit_fwnode_pin(synce_pin);
+ return 0;
+}
+
/**
* ice_dpll_deinit_pins - deinitialize direct pins
* @pf: board private structure
@@ -3420,8 +3556,10 @@ static void ice_dpll_deinit_pins(struct ice_pf *pf, bool cgu)
struct ice_dpll *dp = &d->pps;
ice_dpll_deinit_rclk_pin(pf);
- if (pf->hw.mac_type == ICE_MAC_GENERIC_3K_E825)
+ if (pf->hw.mac_type == ICE_MAC_GENERIC_3K_E825) {
+ ice_dpll_deinit_txclk_pins(pf);
ice_dpll_deinit_fwnode_pins(pf, pf->dplls.inputs, 0);
+ }
if (cgu) {
ice_dpll_unregister_pins(dp->dpll, inputs, &ice_dpll_input_ops,
num_inputs);
@@ -3552,6 +3690,58 @@ ice_dpll_init_fwnode_pins(struct ice_pf *pf, struct ice_dpll_pin *pins,
return ret;
}
+static int ice_dpll_init_txclk_pins(struct ice_pf *pf, int start_idx)
+{
+ struct ice_dpll_pin *ref_pin = pf->dplls.txclks;
+ struct ice_dpll *txc = &pf->dplls.txc;
+ int ret;
+
+ /* Configure EXT_EREF0 pin */
+ ret = ice_dpll_get_pins(pf, ref_pin, start_idx, 1, pf->dplls.clock_id);
+ if (ret)
+ return ret;
+ ret = dpll_pin_register(txc->dpll, ref_pin->pin, &ice_dpll_txclk_ops,
+ ref_pin);
+ if (ret)
+ goto err_release_ext_eref;
+
+ /*
+ * Configure EXT_SYNCE pin (fwnode-backed).
+ * The pin may not yet be available; in that case registration
+ * will be deferred via the notifier path.
+ */
+ ref_pin++;
+ ret = ice_dpll_init_fwnode_pin(ref_pin, ice_dpll_fwnode_ext_synce);
+ if (ret)
+ goto err_unregister_ext_eref;
+
+ if (IS_ERR_OR_NULL(ref_pin->pin)) {
+ dev_dbg(ice_pf_to_dev(pf),
+ "Tx-clk SYNCE pin not registered yet\n");
+ return 0;
+ }
+
+ ret = dpll_pin_register(txc->dpll, ref_pin->pin, &ice_dpll_txclk_ops,
+ ref_pin);
+ if (ret)
+ goto err_deinit_synce;
+
+ return 0;
+
+err_deinit_synce:
+ ice_dpll_deinit_fwnode_pin(ref_pin);
+err_unregister_ext_eref:
+ dpll_pin_unregister(txc->dpll,
+ pf->dplls.txclks[E825_EXT_EREF_PIN_IDX].pin,
+ &ice_dpll_txclk_ops,
+ &pf->dplls.txclks[E825_EXT_EREF_PIN_IDX]);
+
+err_release_ext_eref:
+ ice_dpll_release_pins(&pf->dplls.txclks[E825_EXT_EREF_PIN_IDX], 1);
+
+ return ret;
+}
+
/**
* ice_dpll_init_pins_e825 - init pins and register pins with a dplls
* @pf: board private structure
@@ -3574,6 +3764,15 @@ static int ice_dpll_init_pins_e825(struct ice_pf *pf)
ret = ice_dpll_init_rclk_pin(pf, DPLL_PIN_IDX_UNSPEC,
&ice_dpll_rclk_ops);
+
+ if (ret)
+ goto unregister_pins;
+
+ ret = ice_dpll_init_txclk_pins(pf, 0);
+ if (ret)
+ ice_dpll_deinit_rclk_pin(pf);
+
+unregister_pins:
if (ret) {
/* Inform DPLL notifier works that DPLL init was finished
* unsuccessfully (ICE_DPLL_FLAG not set).
@@ -3692,7 +3891,7 @@ static int ice_dpll_init_pins(struct ice_pf *pf, bool cgu)
static void
ice_dpll_deinit_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu)
{
- if (cgu)
+ if (cgu || pf->hw.mac_type == ICE_MAC_GENERIC_3K_E825)
dpll_device_unregister(d->dpll, d->ops, d);
dpll_device_put(d->dpll, &d->tracker);
}
@@ -3727,12 +3926,13 @@ ice_dpll_init_dpll(struct ice_pf *pf, struct ice_dpll *d, bool cgu,
return ret;
}
d->pf = pf;
- if (cgu) {
+ if (cgu || pf->hw.mac_type == ICE_MAC_GENERIC_3K_E825) {
const struct dpll_device_ops *ops = &ice_dpll_ops;
if (type == DPLL_TYPE_PPS && ice_dpll_is_pps_phase_monitor(pf))
ops = &ice_dpll_pom_ops;
- ice_dpll_update_state(pf, d, true);
+ if (cgu)
+ ice_dpll_update_state(pf, d, true);
ret = dpll_device_register(d->dpll, type, ops, d);
if (ret) {
dpll_device_put(d->dpll, &d->tracker);
@@ -4081,6 +4281,36 @@ static int ice_dpll_init_info_sw_pins(struct ice_pf *pf)
return 0;
}
+/**
+ * ice_dpll_init_info_txclk_pins_e825c - initializes tx-clk pins information
+ * @pf: board private structure
+ *
+ * Init information for tx-clks pin, cache them in pf->dplls.txclks
+ *
+ * Return:
+ * * 0 - success
+ */
+static int ice_dpll_init_info_txclk_pins_e825c(struct ice_pf *pf)
+{
+ struct ice_dpll_pin *tx_pin;
+
+ for (int i = 0; i < ICE_DPLL_TXCLK_NUM_MAX; i++) {
+ tx_pin = &pf->dplls.txclks[i];
+ tx_pin->prop.type = DPLL_PIN_TYPE_EXT;
+ tx_pin->prop.capabilities |=
+ DPLL_PIN_CAPABILITIES_STATE_CAN_CHANGE;
+ tx_pin->pf = pf;
+ if (i == E825_EXT_EREF_PIN_IDX) {
+ tx_pin->prop.board_label = ice_dpll_ext_eref_pin;
+ tx_pin->tx_ref_src = ICE_REF_CLK_EREF0;
+ } else if (i == E825_EXT_SYNCE_PIN_IDX) {
+ tx_pin->tx_ref_src = ICE_REF_CLK_SYNCE;
+ }
+ }
+
+ return 0;
+}
+
/**
* ice_dpll_init_pins_info - init pins info wrapper
* @pf: board private structure
@@ -4106,6 +4336,9 @@ ice_dpll_init_pins_info(struct ice_pf *pf, enum ice_dpll_pin_type pin_type)
return ice_dpll_init_info_rclk_pin(pf);
case ICE_DPLL_PIN_TYPE_SOFTWARE:
return ice_dpll_init_info_sw_pins(pf);
+
+ case ICE_DPLL_PIN_TYPE_TXCLK:
+ return ice_dpll_init_info_txclk_pins_e825c(pf);
default:
return -EINVAL;
}
@@ -4139,11 +4372,15 @@ static void ice_dpll_deinit_info(struct ice_pf *pf)
static int ice_dpll_init_info_e825c(struct ice_pf *pf)
{
struct ice_dplls *d = &pf->dplls;
+ struct ice_dpll *dt = &d->txc;
int ret = 0;
int i;
d->clock_id = ice_generate_clock_id(pf);
d->num_inputs = ICE_SYNCE_CLK_NUM;
+ dt->dpll_state = DPLL_LOCK_STATUS_UNLOCKED;
+ dt->mode = DPLL_MODE_MANUAL;
+ dt->dpll_idx = pf->ptp.port.port_num;
d->inputs = kzalloc_objs(*d->inputs, d->num_inputs);
if (!d->inputs)
@@ -4160,6 +4397,11 @@ static int ice_dpll_init_info_e825c(struct ice_pf *pf)
ret = ice_dpll_init_pins_info(pf, ICE_DPLL_PIN_TYPE_RCLK_INPUT);
if (ret)
goto deinit_info;
+
+ ret = ice_dpll_init_pins_info(pf, ICE_DPLL_PIN_TYPE_TXCLK);
+ if (ret)
+ goto deinit_info;
+
dev_dbg(ice_pf_to_dev(pf),
"%s - success, inputs: %u, outputs: %u, rclk-parents: %u\n",
__func__, d->num_inputs, d->num_outputs, d->rclk.num_parents);
@@ -4292,6 +4534,9 @@ void ice_dpll_deinit(struct ice_pf *pf)
ice_dpll_deinit_dpll(pf, &pf->dplls.pps, cgu);
if (!IS_ERR_OR_NULL(pf->dplls.eec.dpll))
ice_dpll_deinit_dpll(pf, &pf->dplls.eec, cgu);
+ if (!IS_ERR_OR_NULL(pf->dplls.txc.dpll))
+ ice_dpll_deinit_dpll(pf, &pf->dplls.txc, false);
+
ice_dpll_deinit_info(pf);
mutex_destroy(&pf->dplls.lock);
}
@@ -4317,14 +4562,19 @@ static void ice_dpll_init_e825(struct ice_pf *pf)
err = ice_dpll_init_info_e825c(pf);
if (err)
goto err_exit;
- err = ice_dpll_init_pins_e825(pf);
+ err = ice_dpll_init_dpll(pf, &pf->dplls.txc, false, DPLL_TYPE_GENERIC);
if (err)
goto deinit_info;
+ err = ice_dpll_init_pins_e825(pf);
+ if (err)
+ goto deinit_txclk;
set_bit(ICE_FLAG_DPLL, pf->flags);
complete_all(&d->dpll_init);
return;
+deinit_txclk:
+ ice_dpll_deinit_dpll(pf, &pf->dplls.txc, false);
deinit_info:
ice_dpll_deinit_info(pf);
err_exit:
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h b/drivers/net/ethernet/intel/ice/ice_dpll.h
index ae42cdea0ee1..23f9d4da73c5 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.h
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
@@ -7,6 +7,7 @@
#include "ice.h"
#define ICE_DPLL_RCLK_NUM_MAX 4
+#define ICE_DPLL_TXCLK_NUM_MAX 2
/**
* enum ice_dpll_pin_sw - enumerate ice software pin indices:
@@ -63,6 +64,7 @@ struct ice_dpll_pin {
u8 ref_sync;
bool active;
bool hidden;
+ enum ice_e825c_ref_clk tx_ref_src;
};
/** ice_dpll - store info required for DPLL control
@@ -111,9 +113,11 @@ struct ice_dpll {
* @lock: locks access to configuration of a dpll
* @eec: pointer to EEC dpll dev
* @pps: pointer to PPS dpll dev
+ * @txc: pointer to TXC dpll dev
* @inputs: input pins pointer
* @outputs: output pins pointer
* @rclk: recovered pins pointer
+ * @txclks: TX clock reference pins pointer
* @num_inputs: number of input pins available on dpll
* @num_outputs: number of output pins available on dpll
* @cgu_state_acq_err_num: number of errors returned during periodic work
@@ -131,11 +135,13 @@ struct ice_dplls {
struct completion dpll_init;
struct ice_dpll eec;
struct ice_dpll pps;
+ struct ice_dpll txc;
struct ice_dpll_pin *inputs;
struct ice_dpll_pin *outputs;
struct ice_dpll_pin sma[ICE_DPLL_PIN_SW_NUM];
struct ice_dpll_pin ufl[ICE_DPLL_PIN_SW_NUM];
struct ice_dpll_pin rclk;
+ struct ice_dpll_pin txclks[ICE_DPLL_TXCLK_NUM_MAX];
u8 num_inputs;
u8 num_outputs;
u8 sma_data;
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
index 1c9e77dbc770..10795d023f44 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
@@ -265,6 +265,13 @@ struct ice_cgu_pin_desc {
struct dpll_pin_frequency *freq_supp;
};
+enum ice_e825c_ref_clk {
+ ICE_REF_CLK_ENET,
+ ICE_REF_CLK_SYNCE,
+ ICE_REF_CLK_EREF0,
+ ICE_REF_CLK_MAX,
+};
+
#define E810C_QSFP_C827_0_HANDLE 2
#define E810C_QSFP_C827_1_HANDLE 3
--
2.39.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* RE: [Intel-wired-lan] [PATCH v7 net-next 5/8] ice: introduce TXC DPLL device and TX ref clock pin framework for E825
2026-04-30 9:42 ` [PATCH v7 net-next 5/8] ice: introduce TXC DPLL device and TX ref clock pin framework for E825 Grzegorz Nitka
@ 2026-04-30 11:46 ` Loktionov, Aleksandr
2026-05-02 17:33 ` Jakub Kicinski
1 sibling, 0 replies; 21+ messages in thread
From: Loktionov, Aleksandr @ 2026-04-30 11:46 UTC (permalink / raw)
To: Nitka, Grzegorz, netdev@vger.kernel.org
Cc: Vecera, Ivan, vadim.fedorenko@linux.dev, kuba@kernel.org,
jiri@resnulli.us, edumazet@google.com, Kitszel, Przemyslaw,
richardcochran@gmail.com, donald.hunter@gmail.com,
linux-kernel@vger.kernel.org, Kubalewski, Arkadiusz,
andrew+netdev@lunn.ch, intel-wired-lan@lists.osuosl.org,
horms@kernel.org, Prathosh.Satish@microchip.com,
Nguyen, Anthony L, pabeni@redhat.com, davem@davemloft.net
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Grzegorz Nitka
> Sent: Thursday, April 30, 2026 11:43 AM
> To: netdev@vger.kernel.org
> Cc: Vecera, Ivan <ivecera@redhat.com>; vadim.fedorenko@linux.dev;
> kuba@kernel.org; jiri@resnulli.us; edumazet@google.com; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; richardcochran@gmail.com;
> donald.hunter@gmail.com; linux-kernel@vger.kernel.org; Kubalewski,
> Arkadiusz <arkadiusz.kubalewski@intel.com>; andrew+netdev@lunn.ch;
> intel-wired-lan@lists.osuosl.org; horms@kernel.org;
> Prathosh.Satish@microchip.com; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; pabeni@redhat.com; davem@davemloft.net
> Subject: [Intel-wired-lan] [PATCH v7 net-next 5/8] ice: introduce TXC
> DPLL device and TX ref clock pin framework for E825
>
> E825 devices provide a dedicated TX clock (TXC) domain which may be
> driven by multiple reference clock sources, including external board
> references and port-derived SyncE. To support future TX clock control
> and observability through the Linux DPLL subsystem, introduce a
> separate TXC DPLL device (of DPLL_TYPE_GENERIC) and a framework for
> representing TX reference clock inputs.
>
> This change adds a new internal DPLL pin type (TXCLK) and registers TX
> reference clock pins for E825-based devices:
> - EXT_EREF0: a board-level external electrical reference
> - SYNCE: a port-derived SyncE reference described via firmware nodes
>
> The TXC DPLL device is created and managed alongside the existing PPS
> and EEC DPLL instances. TXCLK pins are registered directly or deferred
> via a notifier when backed by fwnode-described pins.
> A per-pin attribute encodes the TX reference source associated with
> each TXCLK pin.
>
> At this stage, TXCLK pin state callbacks and TXC DPLL lock status
> reporting are implemented as placeholders. Pin state getters always
> return DISCONNECTED, and the TXC DPLL is initialized in the UNLOCKED
> state. No hardware configuration or TX reference switching is
> performed yet.
>
> This patch establishes the structural groundwork required for
> hardware-backed TX reference selection, verification, and
> synchronization status reporting, which will be implemented in
> subsequent patches.
>
> Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_dpll.c | 296 ++++++++++++++++++-
> -
> drivers/net/ethernet/intel/ice/ice_dpll.h | 6 +
> drivers/net/ethernet/intel/ice/ice_ptp_hw.h | 7 +
> 3 files changed, 286 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c
> b/drivers/net/ethernet/intel/ice/ice_dpll.c
> index 62f75701d652..d839b50187ba 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
> @@ -19,6 +19,11 @@
> #define ICE_DPLL_SW_PIN_INPUT_BASE_QSFP 6
> #define ICE_DPLL_SW_PIN_OUTPUT_BASE 0
>
> +#define E825_EXT_EREF_PIN_IDX 0
> +#define E825_EXT_SYNCE_PIN_IDX 1
> +#define E825_RCLK_PARENT_0_PIN_IDX 0
> +#define E825_RCLK_PARENT_1_PIN_IDX 1
> +
> #define ICE_DPLL_PIN_SW_INPUT_ABS(in_idx) \
> (ICE_DPLL_SW_PIN_INPUT_BASE_SFP + (in_idx))
>
> @@ -57,6 +62,7 @@
> * @ICE_DPLL_PIN_TYPE_OUTPUT: output pin
> * @ICE_DPLL_PIN_TYPE_RCLK_INPUT: recovery clock input pin
> * @ICE_DPLL_PIN_TYPE_SOFTWARE: software controlled SMA/U.FL pins
...
> /**
> @@ -3199,19 +3276,40 @@ static bool ice_dpll_is_fwnode_pin(struct
> ice_dpll_pin *pin)
> return !IS_ERR_OR_NULL(pin->fwnode);
> }
>
> +static bool ice_dpll_fwnode_eq(const struct fwnode_handle *a,
> + const struct fwnode_handle *b) {
> + return a && b && a == b;
I'm pretty sure that return a && a == b; is enough instead.
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> +}
> +
...
> #define E810C_QSFP_C827_0_HANDLE 2
> #define E810C_QSFP_C827_1_HANDLE 3
>
> --
> 2.39.3
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v7 net-next 5/8] ice: introduce TXC DPLL device and TX ref clock pin framework for E825
2026-04-30 9:42 ` [PATCH v7 net-next 5/8] ice: introduce TXC DPLL device and TX ref clock pin framework for E825 Grzegorz Nitka
2026-04-30 11:46 ` [Intel-wired-lan] " Loktionov, Aleksandr
@ 2026-05-02 17:33 ` Jakub Kicinski
1 sibling, 0 replies; 21+ messages in thread
From: Jakub Kicinski @ 2026-05-02 17:33 UTC (permalink / raw)
To: grzegorz.nitka
Cc: Jakub Kicinski, netdev, ivecera, vadim.fedorenko, jiri, edumazet,
przemyslaw.kitszel, richardcochran, donald.hunter, linux-kernel,
arkadiusz.kubalewski, andrew+netdev, intel-wired-lan, horms,
Prathosh.Satish, anthony.l.nguyen, pabeni, davem
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
ice: introduce TXC DPLL device and TX ref clock pin framework for E825
This patch adds a new TXC DPLL device of DPLL_TYPE_GENERIC for E825
devices, together with a TXCLK pin type and two TX reference pins
(EXT_EREF0 and a fwnode-described SYNCE pin). Pin state callbacks and
TXC lock status are placeholders at this stage.
> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
> index 62f75701d652..d839b50187ba 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
[ ... ]
> @@ -3199,19 +3276,40 @@ static bool ice_dpll_is_fwnode_pin(struct ice_dpll_pin *pin)
> return !IS_ERR_OR_NULL(pin->fwnode);
> }
>
> +static bool ice_dpll_fwnode_eq(const struct fwnode_handle *a,
> + const struct fwnode_handle *b)
> +{
> + return a && b && a == b;
> +}
> +
> static void ice_dpll_pin_notify_work(struct work_struct *work)
> {
> struct ice_dpll_pin_work *w = container_of(work,
> struct ice_dpll_pin_work,
> work);
> struct ice_dpll_pin *pin, *parent = w->pin;
> + bool is_tx_synce_parent = false;
> struct ice_pf *pf = parent->pf;
> + bool is_rclk_parent = false;
> int ret;
>
> wait_for_completion(&pf->dplls.dpll_init);
> if (!test_bit(ICE_FLAG_DPLL, pf->flags))
> goto out; /* DPLL initialization failed */
>
> + /* Decide which parent we are handling, defensively checking FWNs */
[ ... ]
> + } else if (is_tx_synce_parent) {
> + /* Register TX-CLK SYNCE pin directly to TXC DPLL */
> + pin = &pf->dplls.txclks[E825_EXT_SYNCE_PIN_IDX];
> + ret = dpll_pin_register(pf->dplls.txc.dpll, pin->pin,
> + &ice_dpll_txclk_ops, pin);
> + if (ret) {
> + dev_err(ice_pf_to_dev(pf),
> + "TX SYNCE pin register failed: %pe\n",
> + ERR_PTR(ret));
> + goto drop_parent_ref;
> + }
> }
> break;
[ ... ]
> @@ -3401,6 +3524,19 @@ ice_dpll_deinit_fwnode_pins(struct ice_pf *pf, struct ice_dpll_pin *pins,
> destroy_workqueue(pf->dplls.wq);
> }
>
> +static int ice_dpll_deinit_txclk_pins(struct ice_pf *pf)
> +{
> + struct ice_dpll_pin *synce_pin = &pf->dplls.txclks[E825_EXT_SYNCE_PIN_IDX];
> + struct ice_dpll *dt = &pf->dplls.txc;
> +
> + ice_dpll_unregister_pins(dt->dpll, pf->dplls.txclks,
> + &ice_dpll_txclk_ops,
> + ARRAY_SIZE(pf->dplls.txclks));
> + ice_dpll_release_pins(&pf->dplls.txclks[E825_EXT_EREF_PIN_IDX], 1);
> + ice_dpll_deinit_fwnode_pin(synce_pin);
> + return 0;
> +}
Is there a teardown race between this function and a concurrent
ice_dpll_pin_notify_work() that is about to register the SYNCE pin on
the TXC DPLL?
The work's registration path runs:
parent->pin = fwnode_dpll_pin_find(parent->fwnode,
&parent->tracker);
...
} else if (is_tx_synce_parent) {
pin = &pf->dplls.txclks[E825_EXT_SYNCE_PIN_IDX];
ret = dpll_pin_register(pf->dplls.txc.dpll, pin->pin,
&ice_dpll_txclk_ops, pin);
while ice_dpll_deinit_txclk_pins() first scans pf->dplls.txclks[] with
ice_dpll_unregister_pins(), which skips any slot where pins[i].pin is
still NULL:
for (i = 0; i < count; i++) {
if (pins[i].hidden)
continue;
if (IS_ERR_OR_NULL(pins[i].pin))
continue;
dpll_pin_unregister(dpll, pins[i].pin, ops, &pins[i]);
}
Only afterward does it call ice_dpll_deinit_fwnode_pin(synce_pin), which
unregisters the notifier and flushes pf->dplls.wq.
If an external fwnode DPLL provider emits a DPLL_PIN_CREATED for
clk_ref_synce between the two steps, can this sequence occur?
deinit thread notifier work
------------- -------------
ice_dpll_unregister_pins(txclks)
sees txclks[1].pin == NULL
skips SYNCE slot
parent->pin = fwnode_dpll_pin_find()
dpll_pin_register(txc->dpll,
pin->pin, &ops, pin)
-> adds to txc->dpll->pin_refs
and pin->dpll_refs
ice_dpll_release_pins(EREF0, 1)
ice_dpll_deinit_fwnode_pin(synce_pin)
unregister_dpll_notifier()
flush_workqueue() /* no work pending */
dpll_pin_put(synce_pin->pin, ...)
After this, dpll_pin_put() only drops the pin refcount; it does not call
dpll_pin_unregister(). So the TXC dpll's pin_refs still contains the
SYNCE entry when ice_dpll_deinit_dpll(pf, &pf->dplls.txc, false) later
runs dpll_device_unregister() and dpll_device_put(). Does this trip
WARN_ON_ONCE(!xa_empty(&dpll->pin_refs)) in __dpll_device_put() and then
free the dpll_device while pin->dpll_refs still references it, leaving a
use-after-free on any subsequent walk of pin->dpll_refs?
Would it be safer to call ice_dpll_deinit_fwnode_pin(synce_pin) first so
the notifier is torn down and pf->dplls.wq drained before txclks[] is
scanned, or to hold pf->dplls.lock across both the deinit scan and the
work's parent->pin = ...; dpll_pin_register(...) sequence?
A similar structural race exists for the rclk path via
dpll_pin_on_pin_register(), but this patch introduces a new instance on
the direct-on-TXC SYNCE registration. The ordering in
ice_dpll_deinit_txclk_pins() appears unchanged by later patches in the
series, so the same race seems to persist beyond this commit.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v7 net-next 6/8] ice: implement CPI support for E825C
2026-04-30 9:42 [PATCH v7 net-next 0/8] dpll/ice: Add generic DPLL type and full TX reference clock control for E825 Grzegorz Nitka
` (4 preceding siblings ...)
2026-04-30 9:42 ` [PATCH v7 net-next 5/8] ice: introduce TXC DPLL device and TX ref clock pin framework for E825 Grzegorz Nitka
@ 2026-04-30 9:42 ` Grzegorz Nitka
2026-05-02 17:33 ` Jakub Kicinski
2026-04-30 9:42 ` [PATCH v7 net-next 7/8] ice: add Tx reference clock index handling to AN restart command Grzegorz Nitka
2026-04-30 9:42 ` [PATCH v7 net-next 8/8] ice: implement E825 TX ref clock control and TXC hardware sync status Grzegorz Nitka
7 siblings, 1 reply; 21+ messages in thread
From: Grzegorz Nitka @ 2026-04-30 9:42 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, intel-wired-lan, poros, richardcochran,
andrew+netdev, przemyslaw.kitszel, anthony.l.nguyen,
Prathosh.Satish, ivecera, jiri, arkadiusz.kubalewski,
vadim.fedorenko, donald.hunter, horms, pabeni, kuba, davem,
edumazet, Grzegorz Nitka
Add full CPI (Converged PHY Interface) command handling required for
E825C devices. The CPI interface allows the driver to interact with
PHY-side control logic through the LM/PHY command registers, including
enabling/disabling/selection of PHY reference clock.
This patch introduces:
- a new CPI subsystem (ice_cpi.c / ice_cpi.h) implementing the CPI
request/acknowledge state machine, including REQ/ACK protocol,
command execution, and response handling
- helper functions for reading/writing PHY registers over Sideband
Queue
- CPI command execution API (ice_cpi_exec) and a helper for enabling or
disabling Tx reference clocks (CPI 0xF1 opcode 'Config PHY clocking')
- assurance of CPI transaction serialization into the CPI core.
CPI REQ/ACK is a multi-step handshake and must be executed
atomically per PHY. Centralize the lock in ice_cpi_exec() and
use adapter-scoped per-PHY mutexes, which match the hardware sharing
model across PFs.
- addition of the non-posted write opcode (wr_np) to SBQ
- Makefile integration to build CPI support together with the PTP stack
This provides the infrastructure necessary to support PHY-side
configuration flows on E825C and is required for advanced link control
and Tx reference clock management.
Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
---
drivers/net/ethernet/intel/ice/Makefile | 2 +-
drivers/net/ethernet/intel/ice/ice_adapter.c | 4 +
drivers/net/ethernet/intel/ice/ice_adapter.h | 7 +
drivers/net/ethernet/intel/ice/ice_cpi.c | 364 +++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_cpi.h | 61 ++++
drivers/net/ethernet/intel/ice/ice_sbq_cmd.h | 5 +-
drivers/net/ethernet/intel/ice/ice_type.h | 2 +
7 files changed, 442 insertions(+), 3 deletions(-)
create mode 100644 drivers/net/ethernet/intel/ice/ice_cpi.c
create mode 100644 drivers/net/ethernet/intel/ice/ice_cpi.h
diff --git a/drivers/net/ethernet/intel/ice/Makefile b/drivers/net/ethernet/intel/ice/Makefile
index 5b2c666496e7..38db476ab2ec 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-$(CONFIG_PTP_1588_CLOCK) += ice_ptp.o ice_ptp_hw.o ice_dpll.o ice_tspll.o ice_cpi.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_adapter.c b/drivers/net/ethernet/intel/ice/ice_adapter.c
index cbb57060bd56..2dc3629d6d0f 100644
--- a/drivers/net/ethernet/intel/ice/ice_adapter.c
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.c
@@ -62,6 +62,8 @@ static struct ice_adapter *ice_adapter_new(struct pci_dev *pdev)
adapter->index = ice_adapter_index(pdev);
spin_lock_init(&adapter->ptp_gltsyn_time_lock);
spin_lock_init(&adapter->txq_ctx_lock);
+ for (int i = 0; i < ARRAY_SIZE(adapter->cpi_phy_lock); i++)
+ mutex_init(&adapter->cpi_phy_lock[i]);
refcount_set(&adapter->refcount, 1);
mutex_init(&adapter->ports.lock);
@@ -73,6 +75,8 @@ static struct ice_adapter *ice_adapter_new(struct pci_dev *pdev)
static void ice_adapter_free(struct ice_adapter *adapter)
{
WARN_ON(!list_empty(&adapter->ports.ports));
+ for (int i = 0; i < ARRAY_SIZE(adapter->cpi_phy_lock); i++)
+ mutex_destroy(&adapter->cpi_phy_lock[i]);
mutex_destroy(&adapter->ports.lock);
kfree(adapter);
diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.h b/drivers/net/ethernet/intel/ice/ice_adapter.h
index e95266c7f20b..fa238a6a0e1a 100644
--- a/drivers/net/ethernet/intel/ice/ice_adapter.h
+++ b/drivers/net/ethernet/intel/ice/ice_adapter.h
@@ -5,9 +5,12 @@
#define _ICE_ADAPTER_H_
#include <linux/types.h>
+#include <linux/mutex.h>
#include <linux/spinlock_types.h>
#include <linux/refcount_types.h>
+#include "ice_type.h"
+
struct pci_dev;
struct ice_pf;
@@ -31,6 +34,8 @@ struct ice_port_list {
* @ptp_gltsyn_time_lock: Spinlock protecting access to the GLTSYN_TIME
* register of the PTP clock.
* @txq_ctx_lock: Spinlock protecting access to the GLCOMM_QTX_CNTX_CTL register
+ * @cpi_phy_lock: Per-PHY mutex serializing CPI REQ/ACK transactions.
+ * Index 0 = PHY0, index 1 = PHY1. Only used on E825C.
* @ctrl_pf: Control PF of the adapter
* @ports: Ports list
* @index: 64-bit index cached for collision detection on 32bit systems
@@ -41,6 +46,8 @@ struct ice_adapter {
spinlock_t ptp_gltsyn_time_lock;
/* For access to GLCOMM_QTX_CNTX_CTL register */
spinlock_t txq_ctx_lock;
+ /* Serialize CPI REQ/ACK transactions per PHY (E825C only) */
+ struct mutex cpi_phy_lock[ICE_E825_MAX_PHYS];
struct ice_pf *ctrl_pf;
struct ice_port_list ports;
diff --git a/drivers/net/ethernet/intel/ice/ice_cpi.c b/drivers/net/ethernet/intel/ice/ice_cpi.c
new file mode 100644
index 000000000000..d5be36c60697
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_cpi.c
@@ -0,0 +1,364 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2018-2026 Intel Corporation */
+
+#include "ice_type.h"
+#include "ice_common.h"
+#include "ice_ptp_hw.h"
+#include "ice.h"
+#include "ice_cpi.h"
+
+/**
+ * ice_cpi_get_dest_dev - get destination PHY for given phy index
+ * @hw: pointer to the HW struct
+ * @phy: phy index of port the CPI action is taken on
+ *
+ * Return: sideband queue destination PHY device.
+ */
+static enum ice_sbq_dev_id ice_cpi_get_dest_dev(struct ice_hw *hw, u8 phy)
+{
+ u8 curr_phy = hw->lane_num / hw->ptp.ports_per_phy;
+
+ /* In the driver, lanes 4..7 are in fact 0..3 on a second PHY.
+ * On a single complex E825C, PHY 0 is always destination device phy_0
+ * and PHY 1 is phy_0_peer.
+ * On dual complex E825C, device phy_0 points to PHY on a current
+ * complex and phy_0_peer to PHY on a different complex.
+ */
+ if ((!ice_is_dual(hw) && phy) ||
+ (ice_is_dual(hw) && phy != curr_phy))
+ return ice_sbq_dev_phy_0_peer;
+ else
+ return ice_sbq_dev_phy_0;
+}
+
+/**
+ * ice_cpi_write_phy - Write a CPI port register
+ * @hw: pointer to the HW struct
+ * @phy: phy index of port the CPI action is taken on
+ * @addr: PHY register address
+ * @val: Value to write
+ *
+ * Return:
+ * * 0 on success
+ * * other error codes when failed to write to PHY
+ */
+static int ice_cpi_write_phy(struct ice_hw *hw, u8 phy, u32 addr, u32 val)
+{
+ struct ice_sbq_msg_input msg = {
+ .dest_dev = ice_cpi_get_dest_dev(hw, phy),
+ .opcode = ice_sbq_msg_wr_np,
+ .msg_addr_low = lower_16_bits(addr),
+ .msg_addr_high = upper_16_bits(addr),
+ .data = val
+ };
+ int err;
+
+ err = ice_sbq_rw_reg(hw, &msg, LIBIE_AQ_FLAG_RD);
+ if (err)
+ ice_debug(hw, ICE_DBG_PTP,
+ "Failed to write CPI msg to phy %d, err: %d\n",
+ phy, err);
+
+ return err;
+}
+
+/**
+ * ice_cpi_read_phy - Read a CPI port register
+ * @hw: pointer to the HW struct
+ * @phy: phy index of port the CPI action is taken on
+ * @addr: PHY register address
+ * @val: storage for register value
+ *
+ * Return:
+ * * 0 on success
+ * * other error codes when failed to read from PHY
+ */
+static int ice_cpi_read_phy(struct ice_hw *hw, u8 phy, u32 addr, u32 *val)
+{
+ struct ice_sbq_msg_input msg = {
+ .dest_dev = ice_cpi_get_dest_dev(hw, phy),
+ .opcode = ice_sbq_msg_rd,
+ .msg_addr_low = lower_16_bits(addr),
+ .msg_addr_high = upper_16_bits(addr)
+ };
+ int err;
+
+ err = ice_sbq_rw_reg(hw, &msg, LIBIE_AQ_FLAG_RD);
+ if (err) {
+ ice_debug(hw, ICE_DBG_PTP,
+ "Failed to read CPI msg from phy %d, err: %d\n",
+ phy, err);
+ return err;
+ }
+
+ *val = msg.data;
+
+ return 0;
+}
+
+/**
+ * ice_cpi_wait_req0_ack0 - waits for CPI interface to be available
+ * @hw: pointer to the HW struct
+ * @phy: phy index of port the CPI action is taken on
+ *
+ * This function checks if CPI interface is ready to use by CPI client.
+ * It's done by assuring LM.CMD.REQ and PHY.CMD.ACK bit in CPI
+ * interface registers to be 0.
+ *
+ * Return: 0 on success, negative on error
+ */
+static int ice_cpi_wait_req0_ack0(struct ice_hw *hw, int phy)
+{
+ u32 phy_val;
+ u32 lm_val;
+
+ for (int i = 0; i < CPI_RETRIES_COUNT; i++) {
+ int err;
+
+ /* check if another CPI Client is also accessing CPI */
+ err = ice_cpi_read_phy(hw, phy, CPI0_LM1_CMD_DATA, &lm_val);
+ if (err)
+ return err;
+ if (FIELD_GET(CPI_LM_CMD_REQ_M, lm_val))
+ return -EBUSY;
+
+ /* check if PHY.ACK is deasserted */
+ err = ice_cpi_read_phy(hw, phy, CPI0_PHY1_CMD_DATA, &phy_val);
+ if (err)
+ return err;
+ if (FIELD_GET(CPI_PHY_CMD_ERROR_M, phy_val))
+ return -EFAULT;
+ if (!FIELD_GET(CPI_PHY_CMD_ACK_M, phy_val))
+ /* req0 and ack0 at this point - ready to go */
+ return 0;
+
+ msleep(CPI_RETRIES_CADENCE_MS);
+ }
+
+ return -ETIMEDOUT;
+}
+
+/**
+ * ice_cpi_wait_ack - Waits for the PHY.ACK bit to be asserted/deasserted
+ * @hw: pointer to the HW struct
+ * @phy: phy index of port the CPI action is taken on
+ * @asserted: desired state of PHY.ACK bit
+ * @data: pointer to the user data where PHY.data is stored
+ *
+ * This function checks if PHY.ACK bit is asserted or deasserted, depending
+ * on the phase of CPI handshake. If 'asserted' state is required, PHY command
+ * data is stored in the 'data' storage.
+ *
+ * Return: 0 on success, negative on error
+ */
+static int ice_cpi_wait_ack(struct ice_hw *hw, u8 phy, bool asserted,
+ u32 *data)
+{
+ u32 phy_val;
+
+ for (int i = 0; i < CPI_RETRIES_COUNT; i++) {
+ int err;
+
+ err = ice_cpi_read_phy(hw, phy, CPI0_PHY1_CMD_DATA, &phy_val);
+ if (err)
+ return err;
+ if (FIELD_GET(CPI_PHY_CMD_ERROR_M, phy_val))
+ return -EFAULT;
+ if (asserted && FIELD_GET(CPI_PHY_CMD_ACK_M, phy_val)) {
+ if (data)
+ *data = phy_val;
+ return 0;
+ }
+ if (!asserted && !FIELD_GET(CPI_PHY_CMD_ACK_M, phy_val))
+ return 0;
+
+ msleep(CPI_RETRIES_CADENCE_MS);
+ }
+
+ return -ETIMEDOUT;
+}
+
+#define ice_cpi_wait_ack0(hw, port) \
+ ice_cpi_wait_ack(hw, port, false, NULL)
+
+#define ice_cpi_wait_ack1(hw, port, data) \
+ ice_cpi_wait_ack(hw, port, true, data)
+
+/**
+ * ice_cpi_req0 - deasserts LM.REQ bit
+ * @hw: pointer to the HW struct
+ * @phy: phy index of port the CPI action is taken on
+ * @data: the command data
+ *
+ * Return: 0 on success, negative on CPI write error
+ */
+static int ice_cpi_req0(struct ice_hw *hw, u8 phy, u32 data)
+{
+ data &= ~CPI_LM_CMD_REQ_M;
+
+ return ice_cpi_write_phy(hw, phy, CPI0_LM1_CMD_DATA, data);
+}
+
+/**
+ * ice_cpi_exec_cmd - writes command data to CPI interface
+ * @hw: pointer to the HW struct
+ * @phy: phy index of port the CPI action is taken on
+ * @data: the command data
+ *
+ * Return: 0 on success, otherwise negative on error
+ */
+static int ice_cpi_exec_cmd(struct ice_hw *hw, int phy, u32 data)
+{
+ return ice_cpi_write_phy(hw, phy, CPI0_LM1_CMD_DATA, data);
+}
+
+/**
+ * ice_cpi_phy_lock - get per-PHY lock for CPI transaction serialization
+ * @hw: pointer to the HW struct
+ * @phy: PHY index
+ *
+ * Return: pointer to PHY mutex, or %NULL when context is unavailable.
+ */
+static struct mutex *ice_cpi_phy_lock(struct ice_hw *hw, u8 phy)
+{
+ struct ice_pf *pf = hw->back;
+
+ if (!pf || !pf->adapter || phy >= ICE_E825_MAX_PHYS)
+ return NULL;
+
+ return &pf->adapter->cpi_phy_lock[phy];
+}
+
+/**
+ * ice_cpi_exec - executes CPI command
+ * @hw: pointer to the HW struct
+ * @phy: phy index of port the CPI action is taken on
+ * @cmd: pointer to the command struct to execute
+ * @resp: pointer to user allocated CPI response struct
+ *
+ * This function executes CPI request with respect to CPI handshake
+ * mechanism.
+ *
+ * Return: 0 on success, otherwise negative on error
+ */
+int ice_cpi_exec(struct ice_hw *hw, u8 phy,
+ const struct ice_cpi_cmd *cmd,
+ struct ice_cpi_resp *resp)
+{
+ struct mutex *cpi_lock; /* serializes CPI transactions per PHY */
+ u32 phy_cmd, lm_cmd = 0;
+ int err, err1 = 0;
+
+ if (!cmd || !resp)
+ return -EINVAL;
+
+ cpi_lock = ice_cpi_phy_lock(hw, phy);
+ if (!cpi_lock)
+ return -EINVAL;
+
+ mutex_lock(cpi_lock);
+
+ lm_cmd =
+ FIELD_PREP(CPI_LM_CMD_REQ_M, CPI_LM_CMD_REQ) |
+ FIELD_PREP(CPI_LM_CMD_GET_SET_M, cmd->set) |
+ FIELD_PREP(CPI_LM_CMD_OPCODE_M, cmd->opcode) |
+ FIELD_PREP(CPI_LM_CMD_PORTLANE_M, cmd->port) |
+ FIELD_PREP(CPI_LM_CMD_DATA_M, cmd->data);
+
+ /* 1. Try to acquire the bus, PHY ACK should be low before we begin */
+ err = ice_cpi_wait_req0_ack0(hw, phy);
+ if (err)
+ goto cpi_exec_exit;
+
+ /* 2. We start the CPI request */
+ err = ice_cpi_exec_cmd(hw, phy, lm_cmd);
+ if (err)
+ goto cpi_exec_exit;
+
+ /*
+ * 3. Wait for CPI confirmation, PHY ACK should be asserted and opcode
+ * echoed in the response
+ */
+ err = ice_cpi_wait_ack1(hw, phy, &phy_cmd);
+ if (err)
+ goto cpi_deassert;
+
+ if (FIELD_GET(CPI_PHY_CMD_ACK_M, phy_cmd) &&
+ FIELD_GET(CPI_LM_CMD_OPCODE_M, lm_cmd) !=
+ FIELD_GET(CPI_PHY_CMD_OPCODE_M, phy_cmd)) {
+ err = -EFAULT;
+ goto cpi_deassert;
+ }
+
+ resp->opcode = FIELD_GET(CPI_PHY_CMD_OPCODE_M, phy_cmd);
+ resp->data = FIELD_GET(CPI_PHY_CMD_DATA_M, phy_cmd);
+ resp->port = FIELD_GET(CPI_PHY_CMD_PORTLANE_M, phy_cmd);
+
+cpi_deassert:
+ /* 4. We deassert REQ */
+ err1 = ice_cpi_req0(hw, phy, lm_cmd);
+ if (err1)
+ goto cpi_exec_exit;
+
+ /* 5. PHY ACK should be deasserted in response */
+ err1 = ice_cpi_wait_ack0(hw, phy);
+
+cpi_exec_exit:
+ if (!err)
+ err = err1;
+
+ mutex_unlock(cpi_lock);
+
+ return err;
+}
+
+/**
+ * ice_cpi_set_cmd - execute CPI SET command
+ * @hw: pointer to the HW struct
+ * @opcode: CPI command opcode
+ * @phy: phy index CPI command is applied for
+ * @port_lane: ephy index CPI command is applied for
+ * @data: CPI opcode context specific data
+ *
+ * Return: 0 on success.
+ */
+static int ice_cpi_set_cmd(struct ice_hw *hw, u16 opcode, u8 phy, u8 port_lane,
+ u16 data)
+{
+ struct ice_cpi_resp cpi_resp = {0};
+ struct ice_cpi_cmd cpi_cmd = {
+ .opcode = opcode,
+ .set = true,
+ .port = port_lane,
+ .data = data,
+ };
+
+ return ice_cpi_exec(hw, phy, &cpi_cmd, &cpi_resp);
+}
+
+/**
+ * ice_cpi_ena_dis_clk_ref - enables/disables Tx reference clock on port
+ * @hw: pointer to the HW struct
+ * @phy: phy index of port for which Tx reference clock is enabled/disabled
+ * @clk: Tx reference clock to enable or disable
+ * @enable: bool value to enable or disable Tx reference clock
+ *
+ * This function executes CPI request to enable or disable specific
+ * Tx reference clock on given PHY.
+ *
+ * Return: 0 on success.
+ */
+int ice_cpi_ena_dis_clk_ref(struct ice_hw *hw, u8 phy,
+ enum ice_e825c_ref_clk clk, bool enable)
+{
+ u16 val;
+
+ val = FIELD_PREP(CPI_OPCODE_PHY_CLK_PHY_SEL_M, phy) |
+ FIELD_PREP(CPI_OPCODE_PHY_CLK_REF_CTRL_M,
+ enable ? CPI_OPCODE_PHY_CLK_ENABLE :
+ CPI_OPCODE_PHY_CLK_DISABLE) |
+ FIELD_PREP(CPI_OPCODE_PHY_CLK_REF_SEL_M, clk);
+
+ return ice_cpi_set_cmd(hw, CPI_OPCODE_PHY_CLK, phy, 0, val);
+}
+
diff --git a/drivers/net/ethernet/intel/ice/ice_cpi.h b/drivers/net/ethernet/intel/ice/ice_cpi.h
new file mode 100644
index 000000000000..932fe0c0824a
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_cpi.h
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright (C) 2018-2025 Intel Corporation */
+
+#ifndef _ICE_CPI_H_
+#define _ICE_CPI_H_
+
+#define CPI0_PHY1_CMD_DATA 0x7FD028
+#define CPI0_LM1_CMD_DATA 0x7FD024
+#define CPI_RETRIES_COUNT 10
+#define CPI_RETRIES_CADENCE_MS 100
+
+/* CPI PHY CMD DATA register (CPI0_PHY1_CMD_DATA) */
+#define CPI_PHY_CMD_DATA_M GENMASK(15, 0)
+#define CPI_PHY_CMD_OPCODE_M GENMASK(23, 16)
+#define CPI_PHY_CMD_PORTLANE_M GENMASK(26, 24)
+#define CPI_PHY_CMD_RSVD_M GENMASK(29, 27)
+#define CPI_PHY_CMD_ERROR_M BIT(30)
+#define CPI_PHY_CMD_ACK_M BIT(31)
+
+/* CPI LM CMD DATA register (CPI0_LM1_CMD_DATA) */
+#define CPI_LM_CMD_DATA_M GENMASK(15, 0)
+#define CPI_LM_CMD_OPCODE_M GENMASK(23, 16)
+#define CPI_LM_CMD_PORTLANE_M GENMASK(26, 24)
+#define CPI_LM_CMD_RSVD_M GENMASK(28, 27)
+#define CPI_LM_CMD_GET_SET_M BIT(29)
+#define CPI_LM_CMD_RESET_M BIT(30)
+#define CPI_LM_CMD_REQ_M BIT(31)
+
+#define CPI_OPCODE_PHY_CLK 0xF1
+#define CPI_OPCODE_PHY_CLK_PHY_SEL_M GENMASK(9, 6)
+#define CPI_OPCODE_PHY_CLK_REF_CTRL_M GENMASK(5, 4)
+#define CPI_OPCODE_PHY_CLK_PORT_SEL 0
+#define CPI_OPCODE_PHY_CLK_DISABLE 1
+#define CPI_OPCODE_PHY_CLK_ENABLE 2
+#define CPI_OPCODE_PHY_CLK_REF_SEL_M GENMASK(3, 0)
+
+#define CPI_OPCODE_PHY_PCS_RESET 0xF0
+#define CPI_OPCODE_PHY_PCS_ONPI_RESET_VAL 0x3F
+
+#define CPI_LM_CMD_REQ 1
+#define CPI_LM_CMD_SET 1
+
+struct ice_cpi_cmd {
+ u8 port;
+ u8 opcode;
+ u16 data;
+ bool set;
+};
+
+struct ice_cpi_resp {
+ u8 port;
+ u8 opcode;
+ u16 data;
+};
+
+int ice_cpi_exec(struct ice_hw *hw, u8 phy,
+ const struct ice_cpi_cmd *cmd,
+ struct ice_cpi_resp *resp);
+int ice_cpi_ena_dis_clk_ref(struct ice_hw *hw, u8 port,
+ enum ice_e825c_ref_clk clk, bool enable);
+#endif /* _ICE_CPI_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h b/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
index 21bb861febbf..226243d32968 100644
--- a/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_sbq_cmd.h
@@ -54,8 +54,9 @@ enum ice_sbq_dev_id {
};
enum ice_sbq_msg_opcode {
- ice_sbq_msg_rd = 0x00,
- ice_sbq_msg_wr = 0x01
+ ice_sbq_msg_rd = 0x00,
+ ice_sbq_msg_wr = 0x01,
+ ice_sbq_msg_wr_np = 0x02
};
#define ICE_SBQ_MSG_FLAGS 0x40
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index 1e82f4c40b32..d9a5c1aae7c2 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -893,6 +893,8 @@ struct ice_ptp_hw {
u8 ports_per_phy;
};
+#define ICE_E825_MAX_PHYS 2
+
/* Port hardware description */
struct ice_hw {
u8 __iomem *hw_addr;
--
2.39.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v7 net-next 6/8] ice: implement CPI support for E825C
2026-04-30 9:42 ` [PATCH v7 net-next 6/8] ice: implement CPI support for E825C Grzegorz Nitka
@ 2026-05-02 17:33 ` Jakub Kicinski
0 siblings, 0 replies; 21+ messages in thread
From: Jakub Kicinski @ 2026-05-02 17:33 UTC (permalink / raw)
To: grzegorz.nitka
Cc: Jakub Kicinski, netdev, ivecera, vadim.fedorenko, jiri, edumazet,
przemyslaw.kitszel, richardcochran, donald.hunter, linux-kernel,
arkadiusz.kubalewski, andrew+netdev, intel-wired-lan, horms,
Prathosh.Satish, anthony.l.nguyen, pabeni, davem
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
ice: implement CPI support for E825C
This patch adds CPI (Converged PHY Interface) command handling for E825C
devices, introducing a new ice_cpi.c/ice_cpi.h subsystem that implements
the REQ/ACK handshake and a helper to enable or disable Tx reference
clocks. It also adds per-PHY mutexes on the adapter and a non-posted
write opcode (wr_np) to SBQ.
> diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.h b/drivers/net/ethernet/intel/ice/ice_adapter.h
> index e95266c7f20b..fa238a6a0e1a 100644
> --- a/drivers/net/ethernet/intel/ice/ice_adapter.h
> +++ b/drivers/net/ethernet/intel/ice/ice_adapter.h
> @@ -31,6 +34,8 @@ struct ice_port_list {
> * @ptp_gltsyn_time_lock: Spinlock protecting access to the GLTSYN_TIME
> * register of the PTP clock.
> * @txq_ctx_lock: Spinlock protecting access to the GLCOMM_QTX_CNTX_CTL register
> + * @cpi_phy_lock: Per-PHY mutex serializing CPI REQ/ACK transactions.
> + * Index 0 = PHY0, index 1 = PHY1. Only used on E825C.
> * @ctrl_pf: Control PF of the adapter
> * @ports: Ports list
> * @index: 64-bit index cached for collision detection on 32bit systems
The kernel-doc says cpi_phy_lock is "Only used on E825C", but in
ice_adapter_new() and ice_adapter_free() the mutexes are unconditionally
initialized and destroyed for every adapter:
for (int i = 0; i < ARRAY_SIZE(adapter->cpi_phy_lock); i++)
mutex_init(&adapter->cpi_phy_lock[i]);
Would it be better to either relax the comment or gate the
init/destroy on an E825C device check so that the behavior matches the
documentation?
> diff --git a/drivers/net/ethernet/intel/ice/ice_cpi.c b/drivers/net/ethernet/intel/ice/ice_cpi.c
> new file mode 100644
> index 000000000000..d5be36c60697
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/ice/ice_cpi.c
> @@ -0,0 +1,364 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2018-2026 Intel Corporation */
The sibling ice_cpi.h added in the same commit uses
"SPDX-License-Identifier: GPL-2.0-only" and "Copyright (C) 2018-2025".
Should the two newly added files use the same SPDX form (GPL-2.0 is the
deprecated alias of GPL-2.0-only) and the same copyright end year?
[ ... ]
> +/**
> + * ice_cpi_wait_req0_ack0 - waits for CPI interface to be available
> + * @hw: pointer to the HW struct
> + * @phy: phy index of port the CPI action is taken on
> + *
> + * This function checks if CPI interface is ready to use by CPI client.
> + * It's done by assuring LM.CMD.REQ and PHY.CMD.ACK bit in CPI
> + * interface registers to be 0.
> + *
> + * Return: 0 on success, negative on error
> + */
> +static int ice_cpi_wait_req0_ack0(struct ice_hw *hw, int phy)
> +{
> + u32 phy_val;
> + u32 lm_val;
> +
> + for (int i = 0; i < CPI_RETRIES_COUNT; i++) {
> + int err;
> +
> + /* check if another CPI Client is also accessing CPI */
> + err = ice_cpi_read_phy(hw, phy, CPI0_LM1_CMD_DATA, &lm_val);
> + if (err)
> + return err;
> + if (FIELD_GET(CPI_LM_CMD_REQ_M, lm_val))
> + return -EBUSY;
The function is shaped as a retry loop and the kernel-doc says it
"waits for CPI interface to be available", but when LM.REQ is observed
set it returns -EBUSY on the first iteration with no retry.
Only the ACK-deasserted check actually benefits from the loop and the
msleep(CPI_RETRIES_CADENCE_MS) cadence.
If another CPI client transiently holds REQ, should this path also be
retried (up to CPI_RETRIES_COUNT times) rather than failing
immediately?
[ ... ]
> +/**
> + * ice_cpi_exec - executes CPI command
> + * @hw: pointer to the HW struct
> + * @phy: phy index of port the CPI action is taken on
> + * @cmd: pointer to the command struct to execute
> + * @resp: pointer to user allocated CPI response struct
> + *
> + * This function executes CPI request with respect to CPI handshake
> + * mechanism.
> + *
> + * Return: 0 on success, otherwise negative on error
> + */
> +int ice_cpi_exec(struct ice_hw *hw, u8 phy,
> + const struct ice_cpi_cmd *cmd,
> + struct ice_cpi_resp *resp)
> +{
[ ... ]
> + /* 1. Try to acquire the bus, PHY ACK should be low before we begin */
> + err = ice_cpi_wait_req0_ack0(hw, phy);
> + if (err)
> + goto cpi_exec_exit;
> +
> + /* 2. We start the CPI request */
> + err = ice_cpi_exec_cmd(hw, phy, lm_cmd);
> + if (err)
> + goto cpi_exec_exit;
Can this leave LM.REQ stuck asserted on the hardware?
ice_cpi_exec_cmd() writes lm_cmd with CPI_LM_CMD_REQ_M set. If
ice_sbq_rw_reg() returns an error after the hardware has already latched
the LM.REQ=1 write (for example an admin-queue completion timeout after
the write was dispatched), control jumps to cpi_exec_exit and skips the
REQ deassert at cpi_deassert (steps 4 and 5).
Every subsequent ice_cpi_exec() on that PHY would then fail immediately
in ice_cpi_wait_req0_ack0() with -EBUSY, because that helper returns
-EBUSY on the first read that observes LM.REQ==1 and does not retry.
The step-3 error path already falls through to cpi_deassert; should the
step-2 error path do the same so REQ is always cleared? Deasserting REQ
when it was never latched is harmless.
> +
> + /*
> + * 3. Wait for CPI confirmation, PHY ACK should be asserted and opcode
> + * echoed in the response
> + */
> + err = ice_cpi_wait_ack1(hw, phy, &phy_cmd);
> + if (err)
> + goto cpi_deassert;
> +
> + if (FIELD_GET(CPI_PHY_CMD_ACK_M, phy_cmd) &&
> + FIELD_GET(CPI_LM_CMD_OPCODE_M, lm_cmd) !=
> + FIELD_GET(CPI_PHY_CMD_OPCODE_M, phy_cmd)) {
> + err = -EFAULT;
> + goto cpi_deassert;
> + }
Is the FIELD_GET(CPI_PHY_CMD_ACK_M, phy_cmd) conjunct here ever false?
ice_cpi_wait_ack1() only returns 0 with asserted=true when
CPI_PHY_CMD_ACK_M is set in the value it stored into phy_cmd:
if (asserted && FIELD_GET(CPI_PHY_CMD_ACK_M, phy_val)) {
if (data)
*data = phy_val;
return 0;
}
So the ACK sub-expression is always true on this path, and reading the
code suggests a NACK/ACK=0 case is handled here that isn't. Would it be
clearer to drop the redundant check?
[ ... ]
> +/**
> + * ice_cpi_set_cmd - execute CPI SET command
> + * @hw: pointer to the HW struct
> + * @opcode: CPI command opcode
> + * @phy: phy index CPI command is applied for
> + * @port_lane: ephy index CPI command is applied for
> + * @data: CPI opcode context specific data
> + *
> + * Return: 0 on success.
> + */
> +static int ice_cpi_set_cmd(struct ice_hw *hw, u16 opcode, u8 phy, u8 port_lane,
> + u16 data)
[ ... ]
> +/**
> + * ice_cpi_ena_dis_clk_ref - enables/disables Tx reference clock on port
> + * @hw: pointer to the HW struct
> + * @phy: phy index of port for which Tx reference clock is enabled/disabled
> + * @clk: Tx reference clock to enable or disable
> + * @enable: bool value to enable or disable Tx reference clock
> + *
> + * This function executes CPI request to enable or disable specific
> + * Tx reference clock on given PHY.
> + *
> + * Return: 0 on success.
> + */
> +int ice_cpi_ena_dis_clk_ref(struct ice_hw *hw, u8 phy,
> + enum ice_e825c_ref_clk clk, bool enable)
The kernel-doc here and on ice_cpi_set_cmd() says "Return: 0 on
success." but both functions forward any negative errno produced by
ice_cpi_exec() (such as -EINVAL, -EBUSY, -EFAULT, or -ETIMEDOUT).
Should the Return: documentation also cover the error path?
> diff --git a/drivers/net/ethernet/intel/ice/ice_cpi.h b/drivers/net/ethernet/intel/ice/ice_cpi.h
> new file mode 100644
> index 000000000000..932fe0c0824a
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/ice/ice_cpi.h
> @@ -0,0 +1,61 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/* Copyright (C) 2018-2025 Intel Corporation */
> +
> +#ifndef _ICE_CPI_H_
> +#define _ICE_CPI_H_
[ ... ]
> +/* CPI LM CMD DATA register (CPI0_LM1_CMD_DATA) */
> +#define CPI_LM_CMD_DATA_M GENMASK(15, 0)
> +#define CPI_LM_CMD_OPCODE_M GENMASK(23, 16)
> +#define CPI_LM_CMD_PORTLANE_M GENMASK(26, 24)
> +#define CPI_LM_CMD_RSVD_M GENMASK(28, 27)
> +#define CPI_LM_CMD_GET_SET_M BIT(29)
> +#define CPI_LM_CMD_RESET_M BIT(30)
> +#define CPI_LM_CMD_REQ_M BIT(31)
> +
> +#define CPI_OPCODE_PHY_CLK 0xF1
> +#define CPI_OPCODE_PHY_CLK_PHY_SEL_M GENMASK(9, 6)
> +#define CPI_OPCODE_PHY_CLK_REF_CTRL_M GENMASK(5, 4)
> +#define CPI_OPCODE_PHY_CLK_PORT_SEL 0
> +#define CPI_OPCODE_PHY_CLK_DISABLE 1
> +#define CPI_OPCODE_PHY_CLK_ENABLE 2
> +#define CPI_OPCODE_PHY_CLK_REF_SEL_M GENMASK(3, 0)
> +
> +#define CPI_OPCODE_PHY_PCS_RESET 0xF0
> +#define CPI_OPCODE_PHY_PCS_ONPI_RESET_VAL 0x3F
> +
> +#define CPI_LM_CMD_REQ 1
> +#define CPI_LM_CMD_SET 1
A few of these macros appear unused anywhere in this patch and through
the end of the series: CPI_LM_CMD_RESET_M, CPI_OPCODE_PHY_CLK_PORT_SEL,
CPI_OPCODE_PHY_PCS_RESET, CPI_OPCODE_PHY_PCS_ONPI_RESET_VAL, and
CPI_LM_CMD_SET. CPI_LM_CMD_SET (value 1) looks like it was intended to
be passed to FIELD_PREP(CPI_LM_CMD_GET_SET_M, ...), but ice_cpi_exec()
uses the bool cmd->set directly and leaves the named constant dead.
Would it make sense to drop the unused defines, or wire them in if
they're intended for a follow-up?
Also, the commit message mentions "enabling/disabling/selection of PHY
reference clock" and CPI_OPCODE_PHY_CLK_PORT_SEL is defined for the
selection variant, but no API in this patch uses it;
ice_cpi_ena_dis_clk_ref() only issues ENABLE or DISABLE. Should the
commit message be trimmed to match what the patch actually delivers?
> +
> +struct ice_cpi_cmd {
> + u8 port;
> + u8 opcode;
> + u16 data;
> + bool set;
> +};
> +
> +struct ice_cpi_resp {
> + u8 port;
> + u8 opcode;
> + u16 data;
> +};
> +
> +int ice_cpi_exec(struct ice_hw *hw, u8 phy,
> + const struct ice_cpi_cmd *cmd,
> + struct ice_cpi_resp *resp);
> +int ice_cpi_ena_dis_clk_ref(struct ice_hw *hw, u8 port,
> + enum ice_e825c_ref_clk clk, bool enable);
> +#endif /* _ICE_CPI_H_ */
Is ice_cpi.h intended to be self-contained?
The header declares ice_cpi_ena_dis_clk_ref() with an
enum ice_e825c_ref_clk parameter and uses struct ice_hw * in both
prototypes, but it does not include ice_ptp_hw.h (which defines
enum ice_e825c_ref_clk) and does not pull in anything that forward-
declares struct ice_hw. ice_cpi.c compiles because it includes
ice_type.h, ice_common.h and ice_ptp_hw.h before ice_cpi.h.
Since C does not allow a portable forward declaration of an enum type,
would a future consumer that includes ice_cpi.h without first including
ice_ptp_hw.h fail to compile? Should the header include what it needs?
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v7 net-next 7/8] ice: add Tx reference clock index handling to AN restart command
2026-04-30 9:42 [PATCH v7 net-next 0/8] dpll/ice: Add generic DPLL type and full TX reference clock control for E825 Grzegorz Nitka
` (5 preceding siblings ...)
2026-04-30 9:42 ` [PATCH v7 net-next 6/8] ice: implement CPI support for E825C Grzegorz Nitka
@ 2026-04-30 9:42 ` Grzegorz Nitka
2026-04-30 11:29 ` [Intel-wired-lan] " Loktionov, Aleksandr
2026-04-30 9:42 ` [PATCH v7 net-next 8/8] ice: implement E825 TX ref clock control and TXC hardware sync status Grzegorz Nitka
7 siblings, 1 reply; 21+ messages in thread
From: Grzegorz Nitka @ 2026-04-30 9:42 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, intel-wired-lan, poros, richardcochran,
andrew+netdev, przemyslaw.kitszel, anthony.l.nguyen,
Prathosh.Satish, ivecera, jiri, arkadiusz.kubalewski,
vadim.fedorenko, donald.hunter, horms, pabeni, kuba, davem,
edumazet, Grzegorz Nitka
Extend the Restart Auto-Negotiation (AN) AdminQ command with a new
parameter allowing software to specify the Tx reference clock index to
be used during link restart.
This patch:
- adds REFCLK field definitions to ice_aqc_restart_an
- updates ice_aq_set_link_restart_an() to take a new refclk parameter
and properly encode it into the command
- keeps legacy behavior by passing REFCLK_NOCHANGE where appropriate
This prepares the driver for configurations requiring dynamic selection
of the Tx reference clock as part of the AN flow.
Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
---
drivers/net/ethernet/intel/ice/ice_adminq_cmd.h | 2 ++
drivers/net/ethernet/intel/ice/ice_common.c | 5 ++++-
drivers/net/ethernet/intel/ice/ice_common.h | 2 +-
drivers/net/ethernet/intel/ice/ice_lib.c | 3 ++-
4 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index 3cbb1b0582e3..42878abac9eb 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -1169,6 +1169,8 @@ struct ice_aqc_restart_an {
u8 cmd_flags;
#define ICE_AQC_RESTART_AN_LINK_RESTART BIT(1)
#define ICE_AQC_RESTART_AN_LINK_ENABLE BIT(2)
+#define ICE_AQC_RESTART_AN_REFCLK_M GENMASK(4, 3)
+#define ICE_AQC_RESTART_AN_REFCLK_NOCHANGE 0
u8 reserved2[13];
};
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index ce11fea122d0..de88aec9137c 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -4126,12 +4126,13 @@ int ice_get_link_status(struct ice_port_info *pi, bool *link_up)
* @pi: pointer to the port information structure
* @ena_link: if true: enable link, if false: disable link
* @cd: pointer to command details structure or NULL
+ * @refclk: the new TX reference clock, 0 if no change
*
* Sets up the link and restarts the Auto-Negotiation over the link.
*/
int
ice_aq_set_link_restart_an(struct ice_port_info *pi, bool ena_link,
- struct ice_sq_cd *cd)
+ struct ice_sq_cd *cd, u8 refclk)
{
struct ice_aqc_restart_an *cmd;
struct libie_aq_desc desc;
@@ -4147,6 +4148,8 @@ ice_aq_set_link_restart_an(struct ice_port_info *pi, bool ena_link,
else
cmd->cmd_flags &= ~ICE_AQC_RESTART_AN_LINK_ENABLE;
+ cmd->cmd_flags |= FIELD_PREP(ICE_AQC_RESTART_AN_REFCLK_M, refclk);
+
return ice_aq_send_cmd(pi->hw, &desc, NULL, 0, cd);
}
diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h
index e700ac0dc347..9f5344212195 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.h
+++ b/drivers/net/ethernet/intel/ice/ice_common.h
@@ -215,7 +215,7 @@ ice_cfg_phy_fec(struct ice_port_info *pi, struct ice_aqc_set_phy_cfg_data *cfg,
enum ice_fec_mode fec);
int
ice_aq_set_link_restart_an(struct ice_port_info *pi, bool ena_link,
- struct ice_sq_cd *cd);
+ struct ice_sq_cd *cd, u8 refclk);
int
ice_aq_set_mac_cfg(struct ice_hw *hw, u16 max_frame_size, struct ice_sq_cd *cd);
int
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index 837b71b7b2b7..8cdc4fda89e9 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -3769,7 +3769,8 @@ int ice_set_link(struct ice_vsi *vsi, bool ena)
if (vsi->type != ICE_VSI_PF)
return -EINVAL;
- status = ice_aq_set_link_restart_an(pi, ena, NULL);
+ status = ice_aq_set_link_restart_an(pi, ena, NULL,
+ ICE_AQC_RESTART_AN_REFCLK_NOCHANGE);
/* if link is owned by manageability, FW will return LIBIE_AQ_RC_EMODE.
* this is not a fatal error, so print a warning message and return
--
2.39.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* RE: [Intel-wired-lan] [PATCH v7 net-next 7/8] ice: add Tx reference clock index handling to AN restart command
2026-04-30 9:42 ` [PATCH v7 net-next 7/8] ice: add Tx reference clock index handling to AN restart command Grzegorz Nitka
@ 2026-04-30 11:29 ` Loktionov, Aleksandr
0 siblings, 0 replies; 21+ messages in thread
From: Loktionov, Aleksandr @ 2026-04-30 11:29 UTC (permalink / raw)
To: Nitka, Grzegorz, netdev@vger.kernel.org
Cc: Vecera, Ivan, vadim.fedorenko@linux.dev, kuba@kernel.org,
jiri@resnulli.us, edumazet@google.com, Kitszel, Przemyslaw,
richardcochran@gmail.com, donald.hunter@gmail.com,
linux-kernel@vger.kernel.org, Kubalewski, Arkadiusz,
andrew+netdev@lunn.ch, intel-wired-lan@lists.osuosl.org,
horms@kernel.org, Prathosh.Satish@microchip.com,
Nguyen, Anthony L, pabeni@redhat.com, davem@davemloft.net
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Grzegorz Nitka
> Sent: Thursday, April 30, 2026 11:43 AM
> To: netdev@vger.kernel.org
> Cc: Vecera, Ivan <ivecera@redhat.com>; vadim.fedorenko@linux.dev;
> kuba@kernel.org; jiri@resnulli.us; edumazet@google.com; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; richardcochran@gmail.com;
> donald.hunter@gmail.com; linux-kernel@vger.kernel.org; Kubalewski,
> Arkadiusz <arkadiusz.kubalewski@intel.com>; andrew+netdev@lunn.ch;
> intel-wired-lan@lists.osuosl.org; horms@kernel.org;
> Prathosh.Satish@microchip.com; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; pabeni@redhat.com; davem@davemloft.net
> Subject: [Intel-wired-lan] [PATCH v7 net-next 7/8] ice: add Tx
> reference clock index handling to AN restart command
>
> Extend the Restart Auto-Negotiation (AN) AdminQ command with a new
> parameter allowing software to specify the Tx reference clock index to
> be used during link restart.
>
> This patch:
> - adds REFCLK field definitions to ice_aqc_restart_an
> - updates ice_aq_set_link_restart_an() to take a new refclk parameter
> and properly encode it into the command
> - keeps legacy behavior by passing REFCLK_NOCHANGE where appropriate
>
> This prepares the driver for configurations requiring dynamic
> selection of the Tx reference clock as part of the AN flow.
>
> Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_adminq_cmd.h | 2 ++
> drivers/net/ethernet/intel/ice/ice_common.c | 5 ++++-
> drivers/net/ethernet/intel/ice/ice_common.h | 2 +-
> drivers/net/ethernet/intel/ice/ice_lib.c | 3 ++-
> 4 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> index 3cbb1b0582e3..42878abac9eb 100644
> --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> @@ -1169,6 +1169,8 @@ struct ice_aqc_restart_an {
> u8 cmd_flags;
> #define ICE_AQC_RESTART_AN_LINK_RESTART BIT(1)
> #define ICE_AQC_RESTART_AN_LINK_ENABLE BIT(2)
> +#define ICE_AQC_RESTART_AN_REFCLK_M GENMASK(4, 3)
> +#define ICE_AQC_RESTART_AN_REFCLK_NOCHANGE 0
> u8 reserved2[13];
> };
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.c
> b/drivers/net/ethernet/intel/ice/ice_common.c
> index ce11fea122d0..de88aec9137c 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.c
> +++ b/drivers/net/ethernet/intel/ice/ice_common.c
> @@ -4126,12 +4126,13 @@ int ice_get_link_status(struct ice_port_info
> *pi, bool *link_up)
> * @pi: pointer to the port information structure
> * @ena_link: if true: enable link, if false: disable link
> * @cd: pointer to command details structure or NULL
> + * @refclk: the new TX reference clock, 0 if no change
> *
> * Sets up the link and restarts the Auto-Negotiation over the link.
> */
> int
> ice_aq_set_link_restart_an(struct ice_port_info *pi, bool ena_link,
> - struct ice_sq_cd *cd)
> + struct ice_sq_cd *cd, u8 refclk)
> {
> struct ice_aqc_restart_an *cmd;
> struct libie_aq_desc desc;
> @@ -4147,6 +4148,8 @@ ice_aq_set_link_restart_an(struct ice_port_info
> *pi, bool ena_link,
> else
> cmd->cmd_flags &= ~ICE_AQC_RESTART_AN_LINK_ENABLE;
>
> + cmd->cmd_flags |= FIELD_PREP(ICE_AQC_RESTART_AN_REFCLK_M,
> refclk);
> +
> return ice_aq_send_cmd(pi->hw, &desc, NULL, 0, cd); }
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_common.h
> b/drivers/net/ethernet/intel/ice/ice_common.h
> index e700ac0dc347..9f5344212195 100644
> --- a/drivers/net/ethernet/intel/ice/ice_common.h
> +++ b/drivers/net/ethernet/intel/ice/ice_common.h
> @@ -215,7 +215,7 @@ ice_cfg_phy_fec(struct ice_port_info *pi, struct
> ice_aqc_set_phy_cfg_data *cfg,
> enum ice_fec_mode fec);
> int
> ice_aq_set_link_restart_an(struct ice_port_info *pi, bool ena_link,
> - struct ice_sq_cd *cd);
> + struct ice_sq_cd *cd, u8 refclk);
> int
> ice_aq_set_mac_cfg(struct ice_hw *hw, u16 max_frame_size, struct
> ice_sq_cd *cd); int diff --git
> a/drivers/net/ethernet/intel/ice/ice_lib.c
> b/drivers/net/ethernet/intel/ice/ice_lib.c
> index 837b71b7b2b7..8cdc4fda89e9 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -3769,7 +3769,8 @@ int ice_set_link(struct ice_vsi *vsi, bool ena)
> if (vsi->type != ICE_VSI_PF)
> return -EINVAL;
>
> - status = ice_aq_set_link_restart_an(pi, ena, NULL);
> + status = ice_aq_set_link_restart_an(pi, ena, NULL,
> +
> ICE_AQC_RESTART_AN_REFCLK_NOCHANGE);
>
> /* if link is owned by manageability, FW will return
> LIBIE_AQ_RC_EMODE.
> * this is not a fatal error, so print a warning message and
> return
> --
> 2.39.3
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v7 net-next 8/8] ice: implement E825 TX ref clock control and TXC hardware sync status
2026-04-30 9:42 [PATCH v7 net-next 0/8] dpll/ice: Add generic DPLL type and full TX reference clock control for E825 Grzegorz Nitka
` (6 preceding siblings ...)
2026-04-30 9:42 ` [PATCH v7 net-next 7/8] ice: add Tx reference clock index handling to AN restart command Grzegorz Nitka
@ 2026-04-30 9:42 ` Grzegorz Nitka
2026-04-30 11:33 ` [Intel-wired-lan] " Loktionov, Aleksandr
2026-04-30 11:37 ` Loktionov, Aleksandr
7 siblings, 2 replies; 21+ messages in thread
From: Grzegorz Nitka @ 2026-04-30 9:42 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, intel-wired-lan, poros, richardcochran,
andrew+netdev, przemyslaw.kitszel, anthony.l.nguyen,
Prathosh.Satish, ivecera, jiri, arkadiusz.kubalewski,
vadim.fedorenko, donald.hunter, horms, pabeni, kuba, davem,
edumazet, Grzegorz Nitka
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 27346 bytes --]
Build on the previously introduced TXC DPLL framework and implement
full TX reference clock control and hardware-backed synchronization
status reporting for E825 devices.
E825 firmware may accept or override TX reference clock requests based
on device-wide routing constraints and link conditions. For this
reason, TX reference selection and synchronization status must be
observed from hardware rather than inferred from user intent.
This change implements TX reference switching using a deferred worker,
triggered by DPLL TXCLK pin operations. Pin set callbacks express
selection intent and schedule the operation asynchronously; firmware
commands and autonegotiation restarts are executed outside of DPLL
context.
After link-up, the effective TX reference clock is read back from
hardware and software state is reconciled accordingly. TXCLK pin state
reflects only the selected reference clock topology:
- External references (SYNCE, EREF0) are represented as TXCLK pins
- The internal ENET/TXCO clock has no pin representation; when selected,
all TXCLK pins are reported DISCONNECTED
Actual hardware synchronization result is reported exclusively via the
TXC DPLL lock status:
- LOCKED when an external TX reference is in use
- UNLOCKED when falling back to ENET/TXCO
This separation allows userspace to distinguish between TX reference
selection and successful synchronization, matching the DPLL subsystem
model where pin state describes topology and device lock status
describes signal quality.
With this change, TX reference clocks on E825 devices can be reliably
selected, verified against hardware state, and monitored for effective
synchronization via standard DPLL interfaces.
Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
---
drivers/net/ethernet/intel/ice/Makefile | 2 +-
drivers/net/ethernet/intel/ice/ice.h | 12 +
drivers/net/ethernet/intel/ice/ice_dpll.c | 110 ++++++++-
drivers/net/ethernet/intel/ice/ice_dpll.h | 4 +
drivers/net/ethernet/intel/ice/ice_ptp.c | 26 +-
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 | 255 ++++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_txclk.h | 38 +++
10 files changed, 499 insertions(+), 19 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.h b/drivers/net/ethernet/intel/ice/ice.h
index 725b130dd3a2..f72bb1aa4067 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -1155,4 +1155,16 @@ static inline struct ice_hw *ice_get_primary_hw(struct ice_pf *pf)
else
return &pf->adapter->ctrl_pf->hw;
}
+
+/**
+ * ice_get_ctrl_pf - Get pointer to Control PF of the adapter
+ * @pf: pointer to the current PF structure
+ *
+ * Return: A pointer to ice_pf structure which is Control PF,
+ * NULL if it's not initialized yet.
+ */
+static inline struct ice_pf *ice_get_ctrl_pf(struct ice_pf *pf)
+{
+ return !pf->adapter ? NULL : pf->adapter->ctrl_pf;
+}
#endif /* _ICE_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c b/drivers/net/ethernet/intel/ice/ice_dpll.c
index d839b50187ba..628ae59d95bd 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"
+#include "ice_txclk.h"
#include <linux/dpll.h>
#include <linux/property.h>
@@ -19,8 +20,6 @@
#define ICE_DPLL_SW_PIN_INPUT_BASE_QSFP 6
#define ICE_DPLL_SW_PIN_OUTPUT_BASE 0
-#define E825_EXT_EREF_PIN_IDX 0
-#define E825_EXT_SYNCE_PIN_IDX 1
#define E825_RCLK_PARENT_0_PIN_IDX 0
#define E825_RCLK_PARENT_1_PIN_IDX 1
@@ -2527,6 +2526,44 @@ ice_dpll_rclk_state_on_pin_get(const struct dpll_pin *pin, void *pin_priv,
return ret;
}
+/**
+ * ice_dpll_txclk_work - apply a pending TX reference clock change
+ * @work: work_struct embedded in struct ice_dplls
+ *
+ * This worker executes an outstanding TX reference clock switch request
+ * that was previously queued via the DPLL TXCLK pin set callback.
+ *
+ * The worker performs only the operational part of the switch, issuing
+ * the necessary firmware commands to request a new TX reference clock
+ * selection (e.g. triggering an AN restart). It does not verify whether
+ * the requested clock was ultimately accepted by the hardware.
+ *
+ * Hardware verification, software state reconciliation, pin state
+ * notification, and TXC DPLL lock-status updates are performed later,
+ * after link-up, by ice_txclk_update_and_notify().
+ *
+ * Context:
+ * - Runs in process context on pf->dplls.wq and may sleep.
+ * - Serializes access to shared TXCLK state using pf->dplls.lock.
+ */
+static void ice_dpll_txclk_work(struct work_struct *work)
+{
+ struct ice_dplls *dplls =
+ container_of(work, struct ice_dplls, txclk_work);
+ struct ice_pf *pf = container_of(dplls, struct ice_pf, dplls);
+ enum ice_e825c_ref_clk clk;
+ bool do_switch;
+
+ mutex_lock(&pf->dplls.lock);
+ do_switch = pf->dplls.txclk_switch_requested;
+ clk = pf->ptp.port.tx_clk_req;
+ pf->dplls.txclk_switch_requested = false;
+ mutex_unlock(&pf->dplls.lock);
+
+ if (do_switch)
+ ice_txclk_set_clk(pf, clk);
+}
+
/**
* ice_dpll_txclk_state_on_dpll_set - set a state on TX clk pin
* @pin: pointer to a pin
@@ -2538,7 +2575,9 @@ ice_dpll_rclk_state_on_pin_get(const struct dpll_pin *pin, void *pin_priv,
*
* Dpll subsystem callback, set a state of a Tx reference clock pin
*
+ * Context: Acquires and releases pf->dplls.lock
* Return:
+ * * 0 - success
* * negative - failure
*/
static int
@@ -2547,11 +2586,29 @@ ice_dpll_txclk_state_on_dpll_set(const struct dpll_pin *pin, void *pin_priv,
void *dpll_priv, enum dpll_pin_state state,
struct netlink_ext_ack *extack)
{
- /*
- * TODO: set HW accordingly to selected TX reference clock.
- * To be added in the follow up patches.
- */
- return -EOPNOTSUPP;
+ struct ice_dpll_pin *p = pin_priv;
+ struct ice_pf *pf = p->pf;
+ enum ice_e825c_ref_clk new_clk;
+
+ if (ice_dpll_is_reset(pf, extack))
+ return -EBUSY;
+
+ mutex_lock(&pf->dplls.lock);
+ new_clk = (state == DPLL_PIN_STATE_DISCONNECTED) ? ICE_REF_CLK_ENET :
+ p->tx_ref_src;
+ if (new_clk == pf->ptp.port.tx_clk_req) {
+ NL_SET_ERR_MSG_FMT(extack,
+ "pin:%u state:%u on parent device already set",
+ p->idx, state);
+ goto unlock;
+ }
+
+ pf->ptp.port.tx_clk_req = new_clk;
+ pf->dplls.txclk_switch_requested = true;
+ queue_work(pf->dplls.wq, &pf->dplls.txclk_work);
+unlock:
+ mutex_unlock(&pf->dplls.lock);
+ return 0;
}
/**
@@ -2563,10 +2620,21 @@ ice_dpll_txclk_state_on_dpll_set(const struct dpll_pin *pin, void *pin_priv,
* @state: on success holds pin state on parent pin
* @extack: error reporting
*
- * dpll subsystem callback, get a state of a TX clock reference pin.
+ * TXCLK DPLL pin state is derived and not stored explicitly.
+ *
+ * Only external TX reference clocks (SYNCE, EREF0) are modeled
+ * as DPLL pins. The internal ENET (TXCO) clock has no pin and,
+ * when selected, all TXCLK pins are reported DISCONNECTED.
+ *
+ * During a pending TXCLK switch, the requested pin may be
+ * reported as CONNECTED before hardware verification.
+ * Hardware acceptance and synchronization are reported
+ * exclusively via TXC DPLL lock-status.
*
+ * Context: Acquires and releases pf->dplls.lock
* Return:
* * 0 - success
+ * * negative - failure
*/
static int
ice_dpll_txclk_state_on_dpll_get(const struct dpll_pin *pin, void *pin_priv,
@@ -2575,11 +2643,21 @@ ice_dpll_txclk_state_on_dpll_get(const struct dpll_pin *pin, void *pin_priv,
enum dpll_pin_state *state,
struct netlink_ext_ack *extack)
{
- /*
- * TODO: query HW status to determine if the TX reference is selected.
- * To be added in the follow up patches.
- */
- *state = DPLL_PIN_STATE_DISCONNECTED;
+ struct ice_dpll_pin *p = pin_priv;
+ struct ice_pf *pf = p->pf;
+
+ if (ice_dpll_is_reset(pf, extack))
+ return -EBUSY;
+
+ mutex_lock(&pf->dplls.lock);
+ if (pf->ptp.port.tx_clk_req == p->tx_ref_src)
+ *state = DPLL_PIN_STATE_CONNECTED;
+ else if (pf->ptp.port.tx_clk == p->tx_ref_src &&
+ pf->ptp.port.tx_clk_req == pf->ptp.port.tx_clk)
+ *state = DPLL_PIN_STATE_CONNECTED;
+ else
+ *state = DPLL_PIN_STATE_DISCONNECTED;
+ mutex_unlock(&pf->dplls.lock);
return 0;
}
@@ -4402,6 +4480,8 @@ static int ice_dpll_init_info_e825c(struct ice_pf *pf)
if (ret)
goto deinit_info;
+ INIT_WORK(&pf->dplls.txclk_work, ice_dpll_txclk_work);
+
dev_dbg(ice_pf_to_dev(pf),
"%s - success, inputs: %u, outputs: %u, rclk-parents: %u\n",
__func__, d->num_inputs, d->num_outputs, d->rclk.num_parents);
@@ -4538,6 +4618,10 @@ void ice_dpll_deinit(struct ice_pf *pf)
ice_dpll_deinit_dpll(pf, &pf->dplls.txc, false);
ice_dpll_deinit_info(pf);
+
+ if (pf->hw.mac_type == ICE_MAC_GENERIC_3K_E825)
+ flush_work(&pf->dplls.txclk_work);
+
mutex_destroy(&pf->dplls.lock);
}
diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.h b/drivers/net/ethernet/intel/ice/ice_dpll.h
index 23f9d4da73c5..9a96b905141d 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.h
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.h
@@ -8,6 +8,8 @@
#define ICE_DPLL_RCLK_NUM_MAX 4
#define ICE_DPLL_TXCLK_NUM_MAX 2
+#define E825_EXT_EREF_PIN_IDX 0
+#define E825_EXT_SYNCE_PIN_IDX 1
/**
* enum ice_dpll_pin_sw - enumerate ice software pin indices:
@@ -152,6 +154,8 @@ struct ice_dplls {
s32 output_phase_adj_max;
u32 periodic_counter;
bool generic;
+ struct work_struct txclk_work;
+ bool txclk_switch_requested;
};
#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
index 36df742c326c..c1332b51559e 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
@@ -4,6 +4,7 @@
#include "ice.h"
#include "ice_lib.h"
#include "ice_trace.h"
+#include "ice_txclk.h"
static const char ice_pin_names[][64] = {
"SDP0",
@@ -54,11 +55,6 @@ static const struct ice_ptp_pin_desc ice_pin_desc_dpll[] = {
{ SDP3, { 3, -1 }, { 0, 0 }},
};
-static struct ice_pf *ice_get_ctrl_pf(struct ice_pf *pf)
-{
- return !pf->adapter ? NULL : pf->adapter->ctrl_pf;
-}
-
static struct ice_ptp *ice_get_ctrl_ptp(struct ice_pf *pf)
{
struct ice_pf *ctrl_pf = ice_get_ctrl_pf(pf);
@@ -1328,6 +1324,9 @@ void ice_ptp_link_change(struct ice_pf *pf, bool linkup)
}
}
mutex_unlock(&pf->dplls.lock);
+
+ if (linkup)
+ ice_txclk_update_and_notify(pf);
}
switch (hw->mac_type) {
@@ -3073,6 +3072,7 @@ static int ice_ptp_setup_pf(struct ice_pf *pf)
{
struct ice_ptp *ctrl_ptp = ice_get_ctrl_ptp(pf);
struct ice_ptp *ptp = &pf->ptp;
+ u8 port_num, phy;
if (!ctrl_ptp) {
dev_info(ice_pf_to_dev(pf),
@@ -3090,6 +3090,10 @@ static int ice_ptp_setup_pf(struct ice_pf *pf)
&pf->adapter->ports.ports);
mutex_unlock(&pf->adapter->ports.lock);
+ port_num = ptp->port.port_num;
+ phy = port_num / pf->hw.ptp.ports_per_phy;
+ set_bit(port_num, &ctrl_ptp->tx_refclks[phy][pf->ptp.port.tx_clk]);
+
return 0;
}
@@ -3290,6 +3294,7 @@ static void ice_ptp_init_tx_interrupt_mode(struct ice_pf *pf)
*/
void ice_ptp_init(struct ice_pf *pf)
{
+ enum ice_e825c_ref_clk tx_ref_clk;
struct ice_ptp *ptp = &pf->ptp;
struct ice_hw *hw = &pf->hw;
int err;
@@ -3318,6 +3323,17 @@ void ice_ptp_init(struct ice_pf *pf)
goto err_exit;
}
+ ptp->port.tx_clk = ICE_REF_CLK_ENET;
+ ptp->port.tx_clk_req = ICE_REF_CLK_ENET;
+ if (hw->mac_type == ICE_MAC_GENERIC_3K_E825) {
+ err = ice_get_serdes_ref_sel_e825c(hw, ptp->port.port_num,
+ &tx_ref_clk);
+ if (!err) {
+ ptp->port.tx_clk = tx_ref_clk;
+ ptp->port.tx_clk_req = tx_ref_clk;
+ }
+ }
+
err = ice_ptp_setup_pf(pf);
if (err)
goto err_exit;
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.h b/drivers/net/ethernet/intel/ice/ice_ptp.h
index 8c44bd758a4f..8b385271ab36 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp.h
@@ -144,6 +144,8 @@ struct ice_ptp_tx {
* @link_up: indicates whether the link is up
* @tx_fifo_busy_cnt: number of times the Tx FIFO was busy
* @port_num: the port number this structure represents
+ * @tx_clk: currently active Tx reference clock source
+ * @tx_clk_req: requested Tx reference clock source (new target)
*/
struct ice_ptp_port {
struct list_head list_node;
@@ -153,6 +155,8 @@ struct ice_ptp_port {
bool link_up;
u8 tx_fifo_busy_cnt;
u8 port_num;
+ enum ice_e825c_ref_clk tx_clk;
+ enum ice_e825c_ref_clk tx_clk_req;
};
enum ice_ptp_tx_interrupt {
@@ -236,6 +240,7 @@ struct ice_ptp_pin_desc {
* @info: structure defining PTP hardware capabilities
* @clock: pointer to registered PTP clock device
* @tstamp_config: hardware timestamping configuration
+ * @tx_refclks: bitmaps table to store the information about TX reference clocks
* @reset_time: kernel time after clock stop on reset
* @tx_hwtstamp_good: number of completed Tx timestamp requests
* @tx_hwtstamp_skipped: number of Tx time stamp requests skipped
@@ -261,6 +266,8 @@ struct ice_ptp {
struct ptp_clock_info info;
struct ptp_clock *clock;
struct kernel_hwtstamp_config tstamp_config;
+#define ICE_E825_MAX_PHYS 2
+ unsigned long tx_refclks[ICE_E825_MAX_PHYS][ICE_REF_CLK_MAX];
u64 reset_time;
u64 tx_hwtstamp_good;
u32 tx_hwtstamp_skipped;
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
index 24fb7a3e14d6..f7f82aef9f40 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.c
@@ -486,6 +486,43 @@ static int ice_read_phy_eth56g(struct ice_hw *hw, u8 port, u32 addr, u32 *val)
return err;
}
+/**
+ * ice_get_serdes_ref_sel_e825c - Read current Tx ref clock source
+ * @hw: pointer to the HW struct
+ * @port: port number for which Tx reference clock is read
+ * @clk: Tx reference clock value (output)
+ *
+ * Return: 0 on success, other error codes when failed to read from PHY
+ */
+int ice_get_serdes_ref_sel_e825c(struct ice_hw *hw, u8 port,
+ enum ice_e825c_ref_clk *clk)
+{
+ u8 lane = port % hw->ptp.ports_per_phy;
+ u32 serdes_rx_nt, serdes_tx_nt;
+ u32 val;
+ int ret;
+
+ ret = ice_read_phy_eth56g(hw, port,
+ SERDES_IP_IF_LN_FLXM_GENERAL(lane, 0),
+ &val);
+ if (ret)
+ return ret;
+
+ serdes_rx_nt = FIELD_GET(CFG_ICTL_PCS_REF_SEL_RX_NT, val);
+ serdes_tx_nt = FIELD_GET(CFG_ICTL_PCS_REF_SEL_TX_NT, val);
+
+ if (serdes_tx_nt == REF_SEL_NT_SYNCE &&
+ serdes_rx_nt == REF_SEL_NT_SYNCE)
+ *clk = ICE_REF_CLK_SYNCE;
+ else if (serdes_tx_nt == REF_SEL_NT_EREF0 &&
+ serdes_rx_nt == REF_SEL_NT_EREF0)
+ *clk = ICE_REF_CLK_EREF0;
+ else
+ *clk = ICE_REF_CLK_ENET;
+
+ return 0;
+}
+
/**
* ice_phy_res_address_eth56g - Calculate a PHY port register address
* @hw: pointer to the HW struct
diff --git a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
index 10795d023f44..31c44bc16b1b 100644
--- a/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
+++ b/drivers/net/ethernet/intel/ice/ice_ptp_hw.h
@@ -383,6 +383,8 @@ int ice_start_phy_timer_eth56g(struct ice_hw *hw, u8 port);
int ice_phy_cfg_intr_eth56g(struct ice_hw *hw, u8 port, bool ena, u8 threshold);
int ice_phy_cfg_ptp_1step_eth56g(struct ice_hw *hw, u8 port);
int ice_ptp_phy_soft_reset_eth56g(struct ice_hw *hw, u8 port);
+int ice_get_serdes_ref_sel_e825c(struct ice_hw *hw, u8 port,
+ enum ice_e825c_ref_clk *clk);
#define ICE_ETH56G_NOMINAL_INCVAL 0x140000000ULL
#define ICE_ETH56G_NOMINAL_PCS_REF_TUS 0x100000000ULL
@@ -795,4 +797,29 @@ static inline u64 ice_get_base_incval(struct ice_hw *hw)
#define PHY_PTP_1STEP_PD_DELAY_M GENMASK(30, 1)
#define PHY_PTP_1STEP_PD_DLY_V_M BIT(31)
+#define SERDES_IP_IF_LN_FLXM_GENERAL(n, m) \
+ (0x32B800 + (m) * 0x100000 + (n) * 0x8000)
+#define CFG_RESERVED0_1 GENMASK(1, 0)
+#define CFG_ICTL_PCS_MODE_NT BIT(2)
+#define CFG_ICTL_PCS_RCOMP_SLAVE_EN_NT BIT(3)
+#define CFG_ICTL_PCS_CMN_FORCE_PUP_A BIT(4)
+#define CFG_ICTL_PCS_RCOMP_SLAVE_VALID_A BIT(5)
+#define CFG_ICTL_PCS_REF_SEL_RX_NT GENMASK(9, 6)
+#define REF_SEL_NT_ENET 0
+#define REF_SEL_NT_EREF0 1
+#define REF_SEL_NT_SYNCE 2
+#define CFG_IDAT_DFX_OBS_DIG_ GENMASK(11, 10)
+#define CFG_IRST_APB_MEM_B BIT(12)
+#define CFG_ICTL_PCS_DISCONNECT_NT BIT(13)
+#define CFG_ICTL_PCS_ISOLATE_NT BIT(14)
+#define CFG_RESERVED15_15 BIT(15)
+#define CFG_IRST_PCS_TSTBUS_B_A BIT(16)
+#define CFG_ICTL_PCS_REF_TERM_HIZ_EN_NT BIT(17)
+#define CFG_RESERVED18_19 GENMASK(19, 18)
+#define CFG_ICTL_PCS_SYNTHLCSLOW_FORCE_PUP_A BIT(20)
+#define CFG_ICTL_PCS_SYNTHLCFAST_FORCE_PUP_A BIT(21)
+#define CFG_RESERVED22_24 GENMASK(24, 22)
+#define CFG_ICTL_PCS_REF_SEL_TX_NT GENMASK(28, 25)
+#define CFG_RESERVED29_31 GENMASK(31, 29)
+
#endif /* _ICE_PTP_HW_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_txclk.c b/drivers/net/ethernet/intel/ice/ice_txclk.c
new file mode 100644
index 000000000000..449248f1d968
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_txclk.c
@@ -0,0 +1,255 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2026 Intel Corporation */
+
+#include "ice.h"
+#include "ice_cpi.h"
+#include "ice_txclk.h"
+
+#define ICE_PHY0 0
+#define ICE_PHY1 1
+
+/**
+ * ice_txclk_get_pin - map TX reference clock to its DPLL pin
+ * @pf: pointer to the PF structure
+ * @ref_clk: TX reference clock selection
+ *
+ * Return the DPLL pin corresponding to a given external TX reference
+ * clock. Only external TX reference clocks (SYNCE and EREF0) are
+ * represented as DPLL pins. The internal ENET (TXCO) clock has no
+ * associated DPLL pin and therefore yields %NULL.
+ *
+ * This helper is used when emitting DPLL pin change notifications
+ * after TX reference clock transitions have been verified.
+ *
+ * Return: Pointer to the corresponding struct dpll_pin, or %NULL if
+ * the TX reference clock has no DPLL pin representation.
+ */
+static struct dpll_pin *
+ice_txclk_get_pin(struct ice_pf *pf, enum ice_e825c_ref_clk ref_clk)
+{
+ switch (ref_clk) {
+ case ICE_REF_CLK_SYNCE:
+ return pf->dplls.txclks[E825_EXT_SYNCE_PIN_IDX].pin;
+ case ICE_REF_CLK_EREF0:
+ return pf->dplls.txclks[E825_EXT_EREF_PIN_IDX].pin;
+ case ICE_REF_CLK_ENET:
+ default:
+ return NULL;
+ }
+}
+
+/**
+ * ice_txclk_enable_peer - Enable required TX reference clock on peer PHY
+ * @pf: pointer to the PF structure
+ * @clk: TX reference clock that must be enabled
+ *
+ * Some TX reference clocks on E825-class devices (SyncE and EREF0) must
+ * be enabled on both PHY complexes to allow proper routing:
+ *
+ * - SyncE must be enabled on both PHYs when used by PHY0
+ * - EREF0 must be enabled on both PHYs when used by PHY1
+ *
+ * If the requested clock is not yet enabled on the peer PHY, enable it.
+ * ENET does not require duplication and is ignored.
+ *
+ * Return: 0 on success or negative error code on failure.
+ */
+static int ice_txclk_enable_peer(struct ice_pf *pf, enum ice_e825c_ref_clk clk)
+{
+ struct ice_pf *ctrl_pf = ice_get_ctrl_pf(pf);
+ bool peer_clk_in_use;
+ u8 port_num, phy;
+ int err;
+
+ if (clk == ICE_REF_CLK_ENET)
+ return 0;
+
+ if (IS_ERR_OR_NULL(ctrl_pf)) {
+ dev_err(ice_pf_to_dev(pf),
+ "Can't enable tx-clk on peer: no controlling PF\n");
+ return -EINVAL;
+ }
+
+ port_num = pf->ptp.port.port_num;
+ phy = port_num / pf->hw.ptp.ports_per_phy;
+ peer_clk_in_use = true;
+ mutex_lock(&ctrl_pf->dplls.lock);
+ if (clk == ICE_REF_CLK_SYNCE && phy == ICE_PHY0)
+ peer_clk_in_use = ice_txclk_any_port_uses(ctrl_pf,
+ ICE_PHY1,
+ clk);
+ else if (clk == ICE_REF_CLK_EREF0 && phy == ICE_PHY1)
+ peer_clk_in_use = ice_txclk_any_port_uses(ctrl_pf,
+ ICE_PHY0,
+ clk);
+ mutex_unlock(&ctrl_pf->dplls.lock);
+
+ if ((clk == ICE_REF_CLK_SYNCE && phy == ICE_PHY0 && !peer_clk_in_use) ||
+ (clk == ICE_REF_CLK_EREF0 && phy == ICE_PHY1 && !peer_clk_in_use)) {
+ u8 peer_phy = phy ? ICE_PHY0 : ICE_PHY1;
+
+ err = ice_cpi_ena_dis_clk_ref(&pf->hw, peer_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, peer_phy);
+ return err;
+ }
+ }
+
+ return 0;
+}
+
+#define ICE_REFCLK_USER_TO_AQ_IDX(x) ((x) + 1)
+
+/**
+ * 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;
+ bool clk_in_use;
+ u8 port_num, phy;
+ int err;
+
+ if (pf->ptp.port.tx_clk == clk)
+ return 0;
+
+ if (IS_ERR_OR_NULL(ctrl_pf)) {
+ dev_err(ice_pf_to_dev(pf),
+ "Can't set tx-clk: no controlling PF\n");
+ return -EINVAL;
+ }
+
+ port_num = pf->ptp.port.port_num;
+ phy = port_num / pf->hw.ptp.ports_per_phy;
+ port_info = pf->hw.port_info;
+ mutex_lock(&ctrl_pf->dplls.lock);
+ clk_in_use = ice_txclk_any_port_uses(ctrl_pf, phy, clk);
+ mutex_unlock(&ctrl_pf->dplls.lock);
+
+ /* Check if the TX clk is enabled for this PHY, if not - enable it */
+ if (!clk_in_use) {
+ err = ice_cpi_ena_dis_clk_ref(&pf->hw, phy, clk, true);
+ if (err) {
+ dev_err(ice_pf_to_dev(pf), "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;
+ }
+
+ /* 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_pf_to_dev(pf),
+ "AN restart AQ command failed with err %d\n",
+ err);
+
+ return err;
+}
+
+/**
+ * ice_txclk_update_and_notify - Validate TX reference clock switching
+ * @pf: pointer to PF structure
+ *
+ * After a link-up event, verify whether the previously requested TX reference
+ * clock transition actually succeeded. The SERDES reference selector reflects
+ * the effective hardware choice, which may differ from the requested clock
+ * when Auto-Negotiation or firmware applies additional policy.
+ *
+ * If the hardware-selected clock differs from the requested one, update the
+ * software state accordingly and stop further processing.
+ *
+ * When the switch is successful, update the per‑PHY usage bitmaps so that the
+ * driver knows which reference clock is currently in use by this port.
+ *
+ * This function does not initiate a clock switch; it only validates the result
+ * of a previously triggered transition and performs cleanup of unused clocks.
+ */
+void ice_txclk_update_and_notify(struct ice_pf *pf)
+{
+ struct ice_ptp_port *ptp_port = &pf->ptp.port;
+ struct ice_pf *ctrl_pf = ice_get_ctrl_pf(pf);
+ struct dpll_pin *old_pin = NULL;
+ struct dpll_pin *new_pin = NULL;
+ struct ice_hw *hw = &pf->hw;
+ enum ice_e825c_ref_clk clk;
+ int err;
+ u8 phy;
+
+ phy = ptp_port->port_num / hw->ptp.ports_per_phy;
+
+ mutex_lock(&pf->dplls.lock);
+ /* no TX clock change requested */
+ if (pf->ptp.port.tx_clk == pf->ptp.port.tx_clk_req) {
+ mutex_unlock(&pf->dplls.lock);
+ return;
+ }
+ /* verify current Tx reference settings */
+ err = ice_get_serdes_ref_sel_e825c(hw,
+ ptp_port->port_num,
+ &clk);
+ if (err) {
+ mutex_unlock(&pf->dplls.lock);
+ return;
+ }
+
+ if (clk != pf->ptp.port.tx_clk_req) {
+ dev_warn(ice_pf_to_dev(pf),
+ "Failed to switch tx-clk for phy %d and clk %u (current: %u)\n",
+ phy, pf->ptp.port.tx_clk_req, clk);
+ old_pin = ice_txclk_get_pin(pf, pf->ptp.port.tx_clk_req);
+ new_pin = ice_txclk_get_pin(pf, clk);
+ pf->ptp.port.tx_clk = clk;
+ pf->ptp.port.tx_clk_req = clk;
+ goto err_notify;
+ }
+
+ old_pin = ice_txclk_get_pin(pf, pf->ptp.port.tx_clk);
+ pf->ptp.port.tx_clk = clk;
+ pf->ptp.port.tx_clk_req = clk;
+
+ if (IS_ERR_OR_NULL(ctrl_pf)) {
+ dev_err(ice_pf_to_dev(pf),
+ "Can't set tx-clk: no controlling PF\n");
+ goto err_notify;
+ }
+
+ /* update Tx reference clock usage map */
+ for (int i = 0; i < ICE_REF_CLK_MAX; i++)
+ (clk == i) ?
+ set_bit(ptp_port->port_num,
+ &ctrl_pf->ptp.tx_refclks[phy][i]) :
+ clear_bit(ptp_port->port_num,
+ &ctrl_pf->ptp.tx_refclks[phy][i]);
+
+err_notify:
+ mutex_unlock(&pf->dplls.lock);
+
+ /* Notify TX clk pins state transition */
+ if (old_pin)
+ dpll_pin_change_ntf(old_pin);
+ if (new_pin)
+ dpll_pin_change_ntf(new_pin);
+
+ /* Update TXC DPLL lock status based on effective TX clk */
+ if (!IS_ERR_OR_NULL(pf->dplls.txc.dpll)) {
+ enum dpll_lock_status new_lock;
+
+ new_lock = ice_txclk_lock_status(clk);
+
+ if (pf->dplls.txc.dpll_state != new_lock) {
+ pf->dplls.txc.dpll_state = new_lock;
+ dpll_device_change_ntf(pf->dplls.txc.dpll);
+ }
+ }
+}
diff --git a/drivers/net/ethernet/intel/ice/ice_txclk.h b/drivers/net/ethernet/intel/ice/ice_txclk.h
new file mode 100644
index 000000000000..bdf591ea8f14
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_txclk.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2026 Intel Corporation */
+
+#ifndef _ICE_TXCLK_H_
+#define _ICE_TXCLK_H_
+
+/**
+ * ice_txclk_any_port_uses - check if any port on a PHY uses this TX refclk
+ * @ctrl_pf: control PF (owner of the shared tx_refclks map)
+ * @phy: PHY index
+ * @clk: TX reference clock
+ *
+ * Return: true if any bit (port) is set for this clock on this PHY
+ */
+static inline bool
+ice_txclk_any_port_uses(const struct ice_pf *ctrl_pf, u8 phy,
+ enum ice_e825c_ref_clk clk)
+{
+ return find_first_bit(&ctrl_pf->ptp.tx_refclks[phy][clk],
+ BITS_PER_LONG) < BITS_PER_LONG;
+}
+
+static inline enum dpll_lock_status
+ice_txclk_lock_status(enum ice_e825c_ref_clk clk)
+{
+ switch (clk) {
+ case ICE_REF_CLK_SYNCE:
+ case ICE_REF_CLK_EREF0:
+ return DPLL_LOCK_STATUS_LOCKED;
+ case ICE_REF_CLK_ENET:
+ default:
+ return DPLL_LOCK_STATUS_UNLOCKED;
+ }
+}
+
+int ice_txclk_set_clk(struct ice_pf *pf, enum ice_e825c_ref_clk clk);
+void ice_txclk_update_and_notify(struct ice_pf *pf);
+#endif /* _ICE_TXCLK_H_ */
--
2.39.3
^ permalink raw reply related [flat|nested] 21+ messages in thread* RE: [Intel-wired-lan] [PATCH v7 net-next 8/8] ice: implement E825 TX ref clock control and TXC hardware sync status
2026-04-30 9:42 ` [PATCH v7 net-next 8/8] ice: implement E825 TX ref clock control and TXC hardware sync status Grzegorz Nitka
@ 2026-04-30 11:33 ` Loktionov, Aleksandr
2026-04-30 11:37 ` Loktionov, Aleksandr
1 sibling, 0 replies; 21+ messages in thread
From: Loktionov, Aleksandr @ 2026-04-30 11:33 UTC (permalink / raw)
To: Nitka, Grzegorz, netdev@vger.kernel.org
Cc: Vecera, Ivan, vadim.fedorenko@linux.dev, kuba@kernel.org,
jiri@resnulli.us, edumazet@google.com, Kitszel, Przemyslaw,
richardcochran@gmail.com, donald.hunter@gmail.com,
linux-kernel@vger.kernel.org, Kubalewski, Arkadiusz,
andrew+netdev@lunn.ch, intel-wired-lan@lists.osuosl.org,
horms@kernel.org, Prathosh.Satish@microchip.com,
Nguyen, Anthony L, pabeni@redhat.com, davem@davemloft.net
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Grzegorz Nitka
> Sent: Thursday, April 30, 2026 11:43 AM
> To: netdev@vger.kernel.org
> Cc: Vecera, Ivan <ivecera@redhat.com>; vadim.fedorenko@linux.dev;
> kuba@kernel.org; jiri@resnulli.us; edumazet@google.com; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; richardcochran@gmail.com;
> donald.hunter@gmail.com; linux-kernel@vger.kernel.org; Kubalewski,
> Arkadiusz <arkadiusz.kubalewski@intel.com>; andrew+netdev@lunn.ch;
> intel-wired-lan@lists.osuosl.org; horms@kernel.org;
> Prathosh.Satish@microchip.com; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; pabeni@redhat.com; davem@davemloft.net
> Subject: [Intel-wired-lan] [PATCH v7 net-next 8/8] ice: implement E825
> TX ref clock control and TXC hardware sync status
>
> Build on the previously introduced TXC DPLL framework and implement
> full TX reference clock control and hardware-backed synchronization
> status reporting for E825 devices.
>
> E825 firmware may accept or override TX reference clock requests based
> on device-wide routing constraints and link conditions. For this
> reason, TX reference selection and synchronization status must be
> observed from hardware rather than inferred from user intent.
>
> This change implements TX reference switching using a deferred worker,
> triggered by DPLL TXCLK pin operations. Pin set callbacks express
> selection intent and schedule the operation asynchronously; firmware
> commands and autonegotiation restarts are executed outside of DPLL
> context.
>
> After link-up, the effective TX reference clock is read back from
> hardware and software state is reconciled accordingly. TXCLK pin state
> reflects only the selected reference clock topology:
> - External references (SYNCE, EREF0) are represented as TXCLK pins
> - The internal ENET/TXCO clock has no pin representation; when
> selected,
> all TXCLK pins are reported DISCONNECTED
>
> Actual hardware synchronization result is reported exclusively via the
> TXC DPLL lock status:
> - LOCKED when an external TX reference is in use
> - UNLOCKED when falling back to ENET/TXCO
>
> This separation allows userspace to distinguish between TX reference
> selection and successful synchronization, matching the DPLL subsystem
> model where pin state describes topology and device lock status
> describes signal quality.
>
> With this change, TX reference clocks on E825 devices can be reliably
> selected, verified against hardware state, and monitored for effective
> synchronization via standard DPLL interfaces.
>
> Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> ---
> drivers/net/ethernet/intel/ice/Makefile | 2 +-
> drivers/net/ethernet/intel/ice/ice.h | 12 +
> drivers/net/ethernet/intel/ice/ice_dpll.c | 110 ++++++++-
> drivers/net/ethernet/intel/ice/ice_dpll.h | 4 +
> drivers/net/ethernet/intel/ice/ice_ptp.c | 26 +-
> 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 | 255 ++++++++++++++++++++
> drivers/net/ethernet/intel/ice/ice_txclk.h | 38 +++
> 10 files changed, 499 insertions(+), 19 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.h
> b/drivers/net/ethernet/intel/ice/ice.h
> index 725b130dd3a2..f72bb1aa4067 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -1155,4 +1155,16 @@ static inline struct ice_hw
> *ice_get_primary_hw(struct ice_pf *pf)
> else
> return &pf->adapter->ctrl_pf->hw;
> }
...
> * * negative - failure
> */
> static int
> @@ -2547,11 +2586,29 @@ ice_dpll_txclk_state_on_dpll_set(const struct
> dpll_pin *pin, void *pin_priv,
> void *dpll_priv, enum dpll_pin_state
> state,
> struct netlink_ext_ack *extack)
> {
> - /*
> - * TODO: set HW accordingly to selected TX reference clock.
> - * To be added in the follow up patches.
> - */
> - return -EOPNOTSUPP;
> + struct ice_dpll_pin *p = pin_priv;
> + struct ice_pf *pf = p->pf;
> + enum ice_e825c_ref_clk new_clk;
> +
> + if (ice_dpll_is_reset(pf, extack))
> + return -EBUSY;
> +
> + mutex_lock(&pf->dplls.lock);
> + new_clk = (state == DPLL_PIN_STATE_DISCONNECTED) ?
> ICE_REF_CLK_ENET :
> + p->tx_ref_src;
> + if (new_clk == pf->ptp.port.tx_clk_req) {
> + NL_SET_ERR_MSG_FMT(extack,
> + "pin:%u state:%u on parent device
> already set",
> + p->idx, state);
> + goto unlock;
extack message attached, but ...
> + }
> +
> + pf->ptp.port.tx_clk_req = new_clk;
> + pf->dplls.txclk_switch_requested = true;
> + queue_work(pf->dplls.wq, &pf->dplls.txclk_work);
> +unlock:
> + mutex_unlock(&pf->dplls.lock);
> + return 0;
... function returns success.
IMHO either drop the message or return -EALREADY/-EBUSY.
Or I missed something?
> }
>
> /**
> @@ -2563,10 +2620,21 @@ ice_dpll_txclk_state_on_dpll_set(const struct
> dpll_pin *pin, void *pin_priv,
> * @state: on success holds pin state on parent pin
> * @extack: error reporting
> *
...
> --
> 2.39.3
^ permalink raw reply [flat|nested] 21+ messages in thread* RE: [Intel-wired-lan] [PATCH v7 net-next 8/8] ice: implement E825 TX ref clock control and TXC hardware sync status
2026-04-30 9:42 ` [PATCH v7 net-next 8/8] ice: implement E825 TX ref clock control and TXC hardware sync status Grzegorz Nitka
2026-04-30 11:33 ` [Intel-wired-lan] " Loktionov, Aleksandr
@ 2026-04-30 11:37 ` Loktionov, Aleksandr
1 sibling, 0 replies; 21+ messages in thread
From: Loktionov, Aleksandr @ 2026-04-30 11:37 UTC (permalink / raw)
To: Nitka, Grzegorz, netdev@vger.kernel.org
Cc: Vecera, Ivan, vadim.fedorenko@linux.dev, kuba@kernel.org,
jiri@resnulli.us, edumazet@google.com, Kitszel, Przemyslaw,
richardcochran@gmail.com, donald.hunter@gmail.com,
linux-kernel@vger.kernel.org, Kubalewski, Arkadiusz,
andrew+netdev@lunn.ch, intel-wired-lan@lists.osuosl.org,
horms@kernel.org, Prathosh.Satish@microchip.com,
Nguyen, Anthony L, pabeni@redhat.com, davem@davemloft.net
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Grzegorz Nitka
> Sent: Thursday, April 30, 2026 11:43 AM
> To: netdev@vger.kernel.org
> Cc: Vecera, Ivan <ivecera@redhat.com>; vadim.fedorenko@linux.dev;
> kuba@kernel.org; jiri@resnulli.us; edumazet@google.com; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; richardcochran@gmail.com;
> donald.hunter@gmail.com; linux-kernel@vger.kernel.org; Kubalewski,
> Arkadiusz <arkadiusz.kubalewski@intel.com>; andrew+netdev@lunn.ch;
> intel-wired-lan@lists.osuosl.org; horms@kernel.org;
> Prathosh.Satish@microchip.com; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; pabeni@redhat.com; davem@davemloft.net
> Subject: [Intel-wired-lan] [PATCH v7 net-next 8/8] ice: implement E825
> TX ref clock control and TXC hardware sync status
>
> Build on the previously introduced TXC DPLL framework and implement
> full TX reference clock control and hardware-backed synchronization
> status reporting for E825 devices.
>
> E825 firmware may accept or override TX reference clock requests based
> on device-wide routing constraints and link conditions. For this
> reason, TX reference selection and synchronization status must be
> observed from hardware rather than inferred from user intent.
>
> This change implements TX reference switching using a deferred worker,
> triggered by DPLL TXCLK pin operations. Pin set callbacks express
> selection intent and schedule the operation asynchronously; firmware
> commands and autonegotiation restarts are executed outside of DPLL
> context.
>
> After link-up, the effective TX reference clock is read back from
> hardware and software state is reconciled accordingly. TXCLK pin state
> reflects only the selected reference clock topology:
> - External references (SYNCE, EREF0) are represented as TXCLK pins
> - The internal ENET/TXCO clock has no pin representation; when
> selected,
> all TXCLK pins are reported DISCONNECTED
>
> Actual hardware synchronization result is reported exclusively via the
> TXC DPLL lock status:
> - LOCKED when an external TX reference is in use
> - UNLOCKED when falling back to ENET/TXCO
>
> This separation allows userspace to distinguish between TX reference
> selection and successful synchronization, matching the DPLL subsystem
> model where pin state describes topology and device lock status
> describes signal quality.
>
> With this change, TX reference clocks on E825 devices can be reliably
> selected, verified against hardware state, and monitored for effective
> synchronization via standard DPLL interfaces.
>
> Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> ---
> drivers/net/ethernet/intel/ice/Makefile | 2 +-
> drivers/net/ethernet/intel/ice/ice.h | 12 +
> drivers/net/ethernet/intel/ice/ice_dpll.c | 110 ++++++++-
> drivers/net/ethernet/intel/ice/ice_dpll.h | 4 +
> drivers/net/ethernet/intel/ice/ice_ptp.c | 26 +-
> 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 | 255 ++++++++++++++++++++
> drivers/net/ethernet/intel/ice/ice_txclk.h | 38 +++
> 10 files changed, 499 insertions(+), 19 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_dpll_txclk_work - apply a pending TX reference clock change
> + * @work: work_struct embedded in struct ice_dplls
> + *
> + * This worker executes an outstanding TX reference clock switch
> +request
> + * that was previously queued via the DPLL TXCLK pin set callback.
> + *
> + * The worker performs only the operational part of the switch,
> issuing
> + * the necessary firmware commands to request a new TX reference
> clock
> + * selection (e.g. triggering an AN restart). It does not verify
> +whether
> + * the requested clock was ultimately accepted by the hardware.
> + *
> + * Hardware verification, software state reconciliation, pin state
> + * notification, and TXC DPLL lock-status updates are performed
> later,
> + * after link-up, by ice_txclk_update_and_notify().
> + *
> + * Context:
> + * - Runs in process context on pf->dplls.wq and may sleep.
> + * - Serializes access to shared TXCLK state using pf->dplls.lock.
> + */
> +static void ice_dpll_txclk_work(struct work_struct *work) {
> + struct ice_dplls *dplls =
> + container_of(work, struct ice_dplls, txclk_work);
> + struct ice_pf *pf = container_of(dplls, struct ice_pf, dplls);
> + enum ice_e825c_ref_clk clk;
> + bool do_switch;
> +
> + mutex_lock(&pf->dplls.lock);
> + do_switch = pf->dplls.txclk_switch_requested;
Two stray spaces.
> + clk = pf->ptp.port.tx_clk_req;
> + pf->dplls.txclk_switch_requested = false;
Two stray spaces.
> + mutex_unlock(&pf->dplls.lock);
> +
> + if (do_switch)
> + ice_txclk_set_clk(pf, clk);
> +}
> +
...
> --
> 2.39.3
^ permalink raw reply [flat|nested] 21+ messages in thread