Re: [PATCH v3 0/8] fpga: Populate dev_release functions

From: Moritz Fischer <mdf@kernel.org>
To: Russ Weight <russell.h.weight@intel.com>
Cc: mdf@kernel.org, linux-fpga@vger.kernel.org, trix@redhat.com,
	lgoncalv@redhat.com, yilun.xu@intel.com, hao.wu@intel.com,
	matthew.gerlach@intel.com, richard.gong@intel.com
Subject: Re: [PATCH v3 0/8] fpga: Populate dev_release functions
Date: Thu, 10 Jun 2021 12:44:03 -0700	[thread overview]
Message-ID: <YMJrgzoEzJ00YOsR@epycbox.lan> (raw)
In-Reply-To: <f0e83bd2-4cb2-9fc8-2c6e-86a522f3afbe@intel.com>

On Thu, Jun 10, 2021 at 12:38:17PM -0700, Russ Weight wrote:
> I think this version of the patch is ready to go. If you agree, please
> provide a Reviewed-by or Tested-by tag.
> 
> Thanks,
> - Russ
> 
> On 6/10/21 12:34 PM, Russ Weight wrote:
> > The FPGA framework has a convention of using managed resource functions
> > to allow parent drivers to manage the data structures allocated by the
> > class drivers. They use an empty *_dev_release() function to satisfy the
> > class driver.
> >
> > This is inconsistent with linux driver model.
> >
> > These changes populate the class dev_release callback functions while
> > maintaining the current API.  Additional changes are made to maintain
> > consistency with the driver model.
> >
> > For more context on these changes, refer to this email thread:
> >
> > https://marc.info/?l=linux-fpga&m=162127412218557&w=2
> >
> > Changelog v2 -> v3:
> >   - Added Reviewed-by tags
> >   - Moved a "dev" to "parent" rename in the comment header for
> >     devm_fpga_region_create() from patch 8 to patch 5.
> >
> > Changelog v1 -> v2:
> >   - Moved the renaming of "dev" to "parent" into a separate patch each for
> >     fpga-mgr, fpga-bridge, fpga-region.
> >   - Restored the call to fpga_mgr_free() in devm_*_mgr_release() instead of 
> >     changing it to put_device().
> >   - Replaced patch "fpga: altera-pr-ip: Remove function alt_pr_unregister"
> >     with "fpga: altera-pr-ip: Remove function alt_pr_unregister". This patch
> >     removes the alt_pr_unregister() function altogether, instead of just
> >     removing portions of it.
> >
> > Russ Weight (8):
> >   fpga: altera-pr-ip: Remove function alt_pr_unregister
> >   fpga: stratix10-soc: Add missing fpga_mgr_free() call
> >   fpga: mgr: Rename dev to parent for parent device
> >   fpga: bridge: Rename dev to parent for parent device
> >   fpga: region: Rename dev to parent for parent device
> >   fpga: mgr: Use standard dev_release for class driver
> >   fpga: bridge: Use standard dev_release for class driver
> >   fpga: region: Use standard dev_release for class driver
> >
> >  drivers/fpga/altera-pr-ip-core.c       | 10 -----
> >  drivers/fpga/fpga-bridge.c             | 46 ++++++++++-----------
> >  drivers/fpga/fpga-mgr.c                | 55 ++++++++++++--------------
> >  drivers/fpga/fpga-region.c             | 44 ++++++++++-----------
> >  drivers/fpga/stratix10-soc.c           |  1 +
> >  include/linux/fpga/altera-pr-ip-core.h |  1 -
> >  6 files changed, 71 insertions(+), 86 deletions(-)
> >
> 

Series looks good to me now, I'll wait for others to chime in and pick
it up tomorrow if nobody speaks up.

Thanks for doing the work!

- Moritz