mbox series

[v3,0/8] fpga: Populate dev_release functions

Message ID 20210610193422.286835-1-russell.h.weight@intel.com (mailing list archive)
Headers show
Series fpga: Populate dev_release functions | expand

Message

Russ Weight June 10, 2021, 7:34 p.m. UTC
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(-)

Comments

Russ Weight June 10, 2021, 7:38 p.m. UTC | #1
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(-)
>
Moritz Fischer June 10, 2021, 7:44 p.m. UTC | #2
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