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

[Ivan Vecera]

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@xxxxxxxxxxxxx>
> > Signed-off-by: Prathosh Satish <Prathosh.Satish@xxxxxxxxxxxxx>
> > Signed-off-by: Ivan Vecera <ivecera@xxxxxxxxxx>
>
> ...
>
> > 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;
> > +}
> > +
>
> ...