netdev - Re: [PATCH net-next v10 09/14] dpll: zl3073x: Register DPLL devices and pins

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <9a0443f1-e3c0-443d-9120-636be25e6794@redhat.com>
Date: Mon, 16 Jun 2025 18:40:44 +0200
From: Ivan Vecera <ivecera@...hat.com>
To: Simon Horman <horms@...nel.org>
Cc: netdev@...r.kernel.org, Prathosh Satish <Prathosh.Satish@...rochip.com>,
 Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
 Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
 Jiri Pirko <jiri@...nulli.us>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Jonathan Corbet <corbet@....net>,
 Jason Gunthorpe <jgg@...pe.ca>, Shannon Nelson <shannon.nelson@....com>,
 Dave Jiang <dave.jiang@...el.com>,
 Jonathan Cameron <Jonathan.Cameron@...wei.com>, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
 Michal Schmidt <mschmidt@...hat.com>, Petr Oros <poros@...hat.com>
Subject: Re: [PATCH net-next v10 09/14] dpll: zl3073x: Register DPLL devices
 and pins



On 16. 06. 25 6:00 odp., Simon Horman wrote:
> On Sun, Jun 15, 2025 at 10:12:18PM +0200, Ivan Vecera wrote:
>> Enumerate all available DPLL channels and registers a DPLL device for
>> each of them. Check all input references and outputs and register
>> DPLL pins for them.
>>
>> Number of registered DPLL pins depends on configuration of references
>> and outputs. If the reference or output is configured as differential
>> one then only one DPLL pin is registered. Both references and outputs
>> can be also disabled from firmware configuration and in this case
>> no DPLL pins are registered.
>>
>> All registrable references are registered to all available DPLL devices
>> with exception of DPLLs that are configured in NCO (numerically
>> controlled oscillator) mode. In this mode DPLL channel acts as PHC and
>> cannot be locked to any reference.
>>
>> Device outputs are connected to one of synthesizers and each synthesizer
>> is driven by some DPLL channel. So output pins belonging to given output
>> are registered to DPLL device that drives associated synthesizer.
>>
>> Finally add kworker task to monitor async changes on all DPLL channels
>> and input pins and to notify about them DPLL core. Output pins are not
>> monitored as their parameters are not changed asynchronously by the
>> device.
>>
>> Co-developed-by: Prathosh Satish <Prathosh.Satish@...rochip.com>
>> Signed-off-by: Prathosh Satish <Prathosh.Satish@...rochip.com>
>> Signed-off-by: Ivan Vecera <ivecera@...hat.com>
> 
> ...
> 
>> diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
> 
> ...
> 
>> +static int
>> +zl3073x_devm_dpll_init(struct zl3073x_dev *zldev, u8 num_dplls)
>> +{
>> +	struct kthread_worker *kworker;
>> +	struct zl3073x_dpll *zldpll;
>> +	unsigned int i;
>> +	int rc;
>> +
>> +	INIT_LIST_HEAD(&zldev->dplls);
>> +
>> +	/* Initialize all DPLLs */
>> +	for (i = 0; i < num_dplls; i++) {
>> +		zldpll = zl3073x_dpll_alloc(zldev, i);
>> +		if (IS_ERR(zldpll)) {
>> +			dev_err_probe(zldev->dev, PTR_ERR(zldpll),
>> +				      "Failed to alloc DPLL%u\n", i);
> 
> Hi Ivan,
> 
> Jumping to the error label will return rc.
> But rc may not be initialised here.
> 
> Flagged by Smatch.

Hi Simon,
good catch... thanks.
Will fix this in v11 later today (after 24h).

Ivan

> 
>> +			goto error;
>> +		}
>> +
>> +		rc = zl3073x_dpll_register(zldpll);
>> +		if (rc) {
>> +			dev_err_probe(zldev->dev, rc,
>> +				      "Failed to register DPLL%u\n", i);
>> +			zl3073x_dpll_free(zldpll);
>> +			goto error;
>> +		}
>> +
>> +		list_add(&zldpll->list, &zldev->dplls);
>> +	}
>> +
>> +	/* Perform initial firmware fine phase correction */
>> +	rc = zl3073x_dpll_init_fine_phase_adjust(zldev);
>> +	if (rc) {
>> +		dev_err_probe(zldev->dev, rc,
>> +			      "Failed to init fine phase correction\n");
>> +		goto error;
>> +	}
>> +
>> +	/* Initialize monitoring thread */
>> +	kthread_init_delayed_work(&zldev->work, zl3073x_dev_periodic_work);
>> +	kworker = kthread_run_worker(0, "zl3073x-%s", dev_name(zldev->dev));
>> +	if (IS_ERR(kworker)) {
>> +		rc = PTR_ERR(kworker);
>> +		goto error;
>> +	}
>> +
>> +	zldev->kworker = kworker;
>> +	kthread_queue_delayed_work(zldev->kworker, &zldev->work, 0);
>> +
>> +	/* Add devres action to release DPLL related resources */
>> +	rc = devm_add_action_or_reset(zldev->dev, zl3073x_dev_dpll_fini, zldev);
>> +	if (rc)
>> +		goto error;
>> +
>> +	return 0;
>> +
>> +error:
>> +	zl3073x_dev_dpll_fini(zldev);
>> +
>> +	return rc;
>> +}
>> +
> 
> ...
>