Re: [Intel-wired-lan] [PATCH RFC net-next v2 12/12] ice: dpll: Support E825-C SyncE and dynamic pin discovery

2025-12-16 Thread Ivan Vecera




On 12/16/25 2:28 PM, Loktionov, Aleksandr wrote:




-Original Message-
...
+   /* Register rclk pin */
+   pin = &pf->dplls.rclk;
+   dpll_pin_on_pin_unregister(parent->pin, pin->pin,
+  &ice_dpll_rclk_ops, pin);
+
+   /* Drop fwnode pin reference */
+   dpll_pin_put(parent->pin, &parent->tracker);
+   parent->pin = NULL;
+   break;
+   default:
+   break;
+   }
+out:
+   kfree(work);

It looks like you free the embedded work_struct pointer instead of the 
allocated ice_dpll_pin_work container @ice_dpll_pin_notify().
Isn't it?


You are right, will fix this in non-RFC submission.


+}
+
...
+static int ice_dpll_init_info_e825c(struct ice_pf *pf)
+{
+   struct ice_dplls *d = &pf->dplls;
+   int ret = 0;
+   int i;
+
+   d->clock_id = ice_generate_clock_id(pf);
+   d->num_inputs = ICE_SYNCE_CLK_NUM;
+
+   d->inputs = kcalloc(d->num_inputs, sizeof(*d->inputs),
GFP_KERNEL);

It looks like for E825-C the allocated pin info (d->inputs and related fields) 
is never freed:
error paths in ice_dpll_init_info_e825c() return after kcalloc() without 
cleanup, and ice_dpll_deinit() explicitly skips ice_dpll_deinit_info() for this 
MAC.


You are right, this is Arek's code part. I don't see any problem to call
ice_dpll_deinit_info() also for this MAC (.outputs, .pps.input_prio and
.eec.input_prio should be NULL for e825c so it is safe to kfree them).

Will add correct cleanup into ice_dpll_init_info_e825c() and call
ice_dpll_deinit_info() also for this MAC.


With the best regards
Alex


+   if (!d->inputs)
+   return -ENOMEM;
+
+   ret = ice_get_cgu_rclk_pin_info(&pf->hw, &d->base_rclk_idx,
+   &pf->dplls.rclk.num_parents);
+   if (ret)
+   return ret;
+
+   for (i = 0; i < pf->dplls.rclk.num_parents; i++)
+   pf->dplls.rclk.parent_idx[i] = d->base_rclk_idx + i;
+
+   if (ice_pf_src_tmr_owned(pf)) {
+   d->base_1588_idx = ICE_E825_1588_BASE_IDX;
+   pf->dplls.pin_1588.num_parents = pf-

dplls.rclk.num_parents;

+   for (i = 0; i < pf->dplls.pin_1588.num_parents; i++)
+   pf->dplls.pin_1588.parent_idx[i] = d-

base_1588_idx + i;

+   }
+   ret = ice_dpll_init_pins_info(pf,
ICE_DPLL_PIN_TYPE_RCLK_INPUT);
+   if (ret)
+   return ret;
+   dev_dbg(ice_pf_to_dev(pf),
+   "%s - success, inputs: %u, outputs: %u, rclk-parents:
%u, pin_1588-parents: %u\n",
+__func__, d->num_inputs, d->num_outputs, d-

rclk.num_parents,

+d->pin_1588.num_parents);
+   return 0;
+}
+
...
+int ice_tspll_bypass_mux_active_e825c(struct ice_hw *hw, u8 port,
bool *active,
+ enum ice_synce_clk output)
+{
+   u8 active_clk;
+   u32 val;
+
+   switch (output) {
+   case ICE_SYNCE_CLK0:
+   ice_read_cgu_reg(hw, ICE_CGU_R10, &val);
+   active_clk = FIELD_GET(ICE_CGU_R10_SYNCE_S_REF_CLK,
val);
+   break;
+   case ICE_SYNCE_CLK1:
+   ice_read_cgu_reg(hw, ICE_CGU_R11, &val);
+   active_clk = FIELD_GET(ICE_CGU_R11_SYNCE_S_BYP_CLK,
val);
+   break;
+   default:
+   return -EINVAL;
+   }

ice_read_cgu_reg() return codes are ignored, can you explain why?


Arek's code... will fix in the non-RFC submission.

Thanks a lot Alex for your sharp eyes. ;-)

Ivan



Re: [Intel-wired-lan] [PATCH RFC net-next v2 12/12] ice: dpll: Support E825-C SyncE and dynamic pin discovery

2025-12-16 Thread Loktionov, Aleksandr



> -Original Message-
> From: Intel-wired-lan  On Behalf
> Of Ivan Vecera
> Sent: Monday, December 15, 2025 9:31 PM
> To: [email protected]
> Cc: Eric Dumazet ; Nguyen, Anthony L
> ; Rob Herring ; Leon
> Romanovsky ; Lobakin, Aleksander
> ; [email protected]; Kitszel,
> Przemyslaw ; Kubalewski, Arkadiusz
> ; [email protected];
> Jakub Kicinski ; Paolo Abeni ;
> [email protected]; Conor Dooley ; Jiri
> Pirko ; Richard Cochran ;
> Prathosh Satish ; Willem de Bruijn
> ; Vadim Fedorenko ;
> Mark Bloch ; [email protected]; Tariq
> Toukan ; Andrew Lunn ;
> Stefan Wahren ; Simon Horman ;
> Jonathan Lemon ; Krzysztof Kozlowski
> ; Saeed Mahameed ; David S.
> Miller 
> Subject: [Intel-wired-lan] [PATCH RFC net-next v2 12/12] ice: dpll:
> Support E825-C SyncE and dynamic pin discovery
> 
> From: Arkadiusz Kubalewski 
> 
> Add DPLL support for the Intel E825-C Ethernet controller. Unlike
> previous
> generations (E810), the E825-C connects to the platform's DPLL
> subsystem
> via MUX pins defined in the system firmware (Device Tree/ACPI).
> 
> Implement the following mechanisms to support this architecture:
> 
> 1. Dynamic Pin Discovery: Use the fwnode_dpll_pin_find() helper to
>locate the parent MUX pins defined in the firmware.
> 
> 2. Asynchronous Registration: Since the platform DPLL driver may probe
>independently of the network driver, utilize the DPLL notifier
> chain
>(register_dpll_notifier). The driver listens for DPLL_PIN_CREATED
>events to detect when the parent MUX pins become available, then
>registers its own Recovered Clock (RCLK) and PTP (1588) pins as
> children
>of those parents.
> 
> 3. Hardware Configuration: Implement the specific register access
> logic
>for E825-C CGU (Clock Generation Unit) registers (R10, R11). This
>includes configuring the bypass MUXes and clock dividers required
> to
>drive SyncE and PTP signals.
> 
> 4. Split Initialization: Refactor `ice_dpll_init()` to separate the
>static initialization path of E810 from the dynamic, firmware-
> driven
>path required for E825-C.
> 
> Co-developed-by: Ivan Vecera 
> Signed-off-by: Ivan Vecera 
> Co-developed-by: Grzegorz Nitka 
> Signed-off-by: Grzegorz Nitka 
> Signed-off-by: Arkadiusz Kubalewski 
> ---
> Changes:
> RFC v2:
> * Fixed infinite loop in ice_dpll_pin_get_parent_num()
> ---
>  drivers/net/ethernet/intel/ice/ice_dpll.c   | 961 ++-
> -
>  drivers/net/ethernet/intel/ice/ice_dpll.h   |  29 +
>  drivers/net/ethernet/intel/ice/ice_lib.c|   3 +
>  drivers/net/ethernet/intel/ice/ice_ptp.c|  29 +
>  drivers/net/ethernet/intel/ice/ice_ptp_hw.c |   9 +-
>  drivers/net/ethernet/intel/ice/ice_ptp_hw.h |   1 +
>  drivers/net/ethernet/intel/ice/ice_tspll.c  | 223 +
>  drivers/net/ethernet/intel/ice/ice_tspll.h  |  14 +-
>  drivers/net/ethernet/intel/ice/ice_type.h   |   6 +
>  9 files changed, 1185 insertions(+), 90 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c
> b/drivers/net/ethernet/intel/ice/ice_dpll.c
> index 4eca62688d834..443294a9bd4b3 100644
> --- a/drivers/net/ethernet/intel/ice/ice_dpll.c
> +++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
> @@ -5,6 +5,7 @@
>  #include "ice_lib.h"
>  #include "ice_trace.h"
>  #include 
> +#include 
> 
>  #define ICE_CGU_STATE_ACQ_ERR_THRESHOLD  50
>  #define ICE_DPLL_PIN_IDX_INVALID 0xff
> @@ -74,6 +75,7 @@ static const char * const pin_type_name[] = {
> 
>  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 ice_dpll_pin_1588[] = "pin_1588";
> 
>  static const struct dpll_pin_frequency ice_esync_range[] = {
>   DPLL_PIN_FREQUENCY_RANGE(0, DPLL_PIN_FREQUENCY_1_HZ),
> @@ -528,6 +530,130 @@ ice_dpll_pin_disable(struct ice_hw *hw, struct
> ice_dpll_pin *pin,
>   return ret;
>  }
> 
> +/**
> + * ice_dpll_pin_store_state - updates the state of pin in SW
> bookkeeping
> + * @pin: pointer to a pin
> + * @parent: parent pin index
> + * @state: pin state (connected or disconnected)
> + */
> +static inline void
> +ice_dpll_pin_store_state(struct ice_dpll_pin *pin, int parent, bool
> state)
> +{
> + pin->state[parent] = state ? DPLL_PIN_STATE_CONNECTED :
> + DPLL_PIN_STATE_DISCONNECTED;
> +}
> +
> +/**
> + * ice_dpll_rclk_update_e825c - updates the state of rclk pin on
> e825c device
> + * @pf: private board str

[Intel-wired-lan] [PATCH RFC net-next v2 12/12] ice: dpll: Support E825-C SyncE and dynamic pin discovery

2025-12-15 Thread Ivan Vecera
From: Arkadiusz Kubalewski 

Add DPLL support for the Intel E825-C Ethernet controller. Unlike previous
generations (E810), the E825-C connects to the platform's DPLL subsystem
via MUX pins defined in the system firmware (Device Tree/ACPI).

Implement the following mechanisms to support this architecture:

1. Dynamic Pin Discovery: Use the fwnode_dpll_pin_find() helper to
   locate the parent MUX pins defined in the firmware.

2. Asynchronous Registration: Since the platform DPLL driver may probe
   independently of the network driver, utilize the DPLL notifier chain
   (register_dpll_notifier). The driver listens for DPLL_PIN_CREATED
   events to detect when the parent MUX pins become available, then
   registers its own Recovered Clock (RCLK) and PTP (1588) pins as children
   of those parents.

3. Hardware Configuration: Implement the specific register access logic
   for E825-C CGU (Clock Generation Unit) registers (R10, R11). This
   includes configuring the bypass MUXes and clock dividers required to
   drive SyncE and PTP signals.

4. Split Initialization: Refactor `ice_dpll_init()` to separate the
   static initialization path of E810 from the dynamic, firmware-driven
   path required for E825-C.

Co-developed-by: Ivan Vecera 
Signed-off-by: Ivan Vecera 
Co-developed-by: Grzegorz Nitka 
Signed-off-by: Grzegorz Nitka 
Signed-off-by: Arkadiusz Kubalewski 
---
Changes:
RFC v2:
* Fixed infinite loop in ice_dpll_pin_get_parent_num()
---
 drivers/net/ethernet/intel/ice/ice_dpll.c   | 961 ++--
 drivers/net/ethernet/intel/ice/ice_dpll.h   |  29 +
 drivers/net/ethernet/intel/ice/ice_lib.c|   3 +
 drivers/net/ethernet/intel/ice/ice_ptp.c|  29 +
 drivers/net/ethernet/intel/ice/ice_ptp_hw.c |   9 +-
 drivers/net/ethernet/intel/ice/ice_ptp_hw.h |   1 +
 drivers/net/ethernet/intel/ice/ice_tspll.c  | 223 +
 drivers/net/ethernet/intel/ice/ice_tspll.h  |  14 +-
 drivers/net/ethernet/intel/ice/ice_type.h   |   6 +
 9 files changed, 1185 insertions(+), 90 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_dpll.c 
b/drivers/net/ethernet/intel/ice/ice_dpll.c
index 4eca62688d834..443294a9bd4b3 100644
--- a/drivers/net/ethernet/intel/ice/ice_dpll.c
+++ b/drivers/net/ethernet/intel/ice/ice_dpll.c
@@ -5,6 +5,7 @@
 #include "ice_lib.h"
 #include "ice_trace.h"
 #include 
+#include 
 
 #define ICE_CGU_STATE_ACQ_ERR_THRESHOLD50
 #define ICE_DPLL_PIN_IDX_INVALID   0xff
@@ -74,6 +75,7 @@ static const char * const pin_type_name[] = {
 
 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 ice_dpll_pin_1588[] = "pin_1588";
 
 static const struct dpll_pin_frequency ice_esync_range[] = {
DPLL_PIN_FREQUENCY_RANGE(0, DPLL_PIN_FREQUENCY_1_HZ),
@@ -528,6 +530,130 @@ ice_dpll_pin_disable(struct ice_hw *hw, struct 
ice_dpll_pin *pin,
return ret;
 }
 
+/**
+ * ice_dpll_pin_store_state - updates the state of pin in SW bookkeeping
+ * @pin: pointer to a pin
+ * @parent: parent pin index
+ * @state: pin state (connected or disconnected)
+ */
+static inline void
+ice_dpll_pin_store_state(struct ice_dpll_pin *pin, int parent, bool state)
+{
+   pin->state[parent] = state ? DPLL_PIN_STATE_CONNECTED :
+   DPLL_PIN_STATE_DISCONNECTED;
+}
+
+/**
+ * ice_dpll_rclk_update_e825c - updates the state of rclk pin on e825c device
+ * @pf: private board struct
+ * @pin: pointer to a pin
+ *
+ * Update struct holding pin states info, states are separate for each parent
+ *
+ * Context: Called under pf->dplls.lock
+ * Return:
+ * * 0 - OK
+ * * negative - error
+ */
+static int ice_dpll_rclk_update_e825c(struct ice_pf *pf,
+ struct ice_dpll_pin *pin)
+{
+   u8 rclk_bits;
+   int err;
+   u32 reg;
+
+   if (pf->dplls.rclk.num_parents > ICE_SYNCE_CLK_NUM)
+   return -EINVAL;
+
+   err = ice_read_cgu_reg(&pf->hw, ICE_CGU_R10, ®);
+   if (err)
+   return err;
+   rclk_bits = FIELD_GET(ICE_CGU_R10_SYNCE_S_REF_CLK, reg);
+   ice_dpll_pin_store_state(pin, ICE_SYNCE_CLK0, rclk_bits ==
+   (pf->ptp.port.port_num + ICE_CGU_BYPASS_MUX_OFFSET_E825C));
+
+   err = ice_read_cgu_reg(&pf->hw, ICE_CGU_R11, ®);
+   if (err)
+   return err;
+   rclk_bits = FIELD_GET(ICE_CGU_R11_SYNCE_S_BYP_CLK, reg);
+   ice_dpll_pin_store_state(pin, ICE_SYNCE_CLK1, rclk_bits ==
+   (pf->ptp.port.port_num + ICE_CGU_BYPASS_MUX_OFFSET_E825C));
+   return 0;
+}
+
+/**
+ * ice_dpll_rclk_update - updates the state of rclk pin on a device
+ * @pf: private board struct
+ * @pin: pointer to a pin
+ * @port_num: port number
+ *
+ * Update struct holding pin states info, states are separate for each parent
+ *
+ * Context: Called under pf->dplls.lock
+ * Return:
+ * * 0 - OK
+ * * negative - error
+ */
+static int ice_dpll_rclk_up