Re: [PATCH v2 4/6] pds_fwctl: initial driver framework

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, Mar 03, 2025 at 09:45:52AM -0700, Dave Jiang wrote:
> > +static int pdsfc_probe(struct auxiliary_device *adev,
> > +		       const struct auxiliary_device_id *id)
> > +{
> > +	struct pds_auxiliary_dev *padev =
> > +			container_of(adev, struct pds_auxiliary_dev, aux_dev);
> > +	struct pdsfc_dev *pdsfc __free(pdsfc_dev) =
> > +			fwctl_alloc_device(&padev->vf_pdev->dev, &pdsfc_ops,
> > +					   struct pdsfc_dev, fwctl);
> > +	struct device *dev = &adev->dev;
> > +	int err;
> > +
> 
> It's ok to move the pdsfc declaration inline right before this check
> below. That would help prevent any accidental usages of pdsfc before
> the check. This is an exception to the coding style guidelines with
> the introduction of cleanup mechanisms.

Yeah.. I'm starting to feel negative about cleanup.h - there are too
many special style notes that seem to only be known by hearsay :\

> > +static void pdsfc_remove(struct auxiliary_device *adev)
> > +{
> > +	struct pdsfc_dev *pdsfc __free(pdsfc_dev) = auxiliary_get_drvdata(adev);
> > +
> > +	fwctl_unregister(&pdsfc->fwctl);
> 
> Missing fwctl_put(). See fwctl_unregister() header comments. Caller
> must still call fwctl_put() to free the fwctl after calling
> fwctl_unregister().

The code is correct, the put is hidden in the __free

However I think we decided not to use __free in this context and open
code the put as a style choice

Thanks,
Jason