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

quoted

-----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.

quoted

+}
+
...
+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

quoted

+	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-

quoted

dplls.rclk.num_parents;

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

quoted

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-

quoted

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