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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <2a50d5a3-3116-43e6-ada7-d6c02c483708@redhat.com>
Date: Tue, 16 Dec 2025 17:49:52 +0100
From: Ivan Vecera <ivecera@...hat.com>
To: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>,
 "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Cc: Eric Dumazet <edumazet@...gle.com>,
 "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
 Rob Herring <robh@...nel.org>, Leon Romanovsky <leon@...nel.org>,
 "Lobakin, Aleksander" <aleksander.lobakin@...el.com>,
 "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
 "Kitszel, Przemyslaw" <przemyslaw.kitszel@...el.com>,
 "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>,
 "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
 Conor Dooley <conor+dt@...nel.org>, Jiri Pirko <jiri@...nulli.us>,
 Richard Cochran <richardcochran@...il.com>,
 Prathosh Satish <Prathosh.Satish@...rochip.com>,
 Willem de Bruijn <willemb@...gle.com>,
 Vadim Fedorenko <vadim.fedorenko@...ux.dev>, Mark Bloch <mbloch@...dia.com>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 Tariq Toukan <tariqt@...dia.com>, Andrew Lunn <andrew+netdev@...n.ch>,
 Stefan Wahren <wahrenst@....net>, Simon Horman <horms@...nel.org>,
 Jonathan Lemon <jonathan.lemon@...il.com>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Saeed Mahameed
 <saeedm@...dia.com>, "David S. Miller" <davem@...emloft.net>
Subject: Re: [Intel-wired-lan] [PATCH RFC net-next v2 12/12] ice: dpll:
 Support E825-C SyncE and dynamic pin discovery

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