From 4d4d2d4346857bf778fafaa97d6f76bb1663e3c9 Mon Sep 17 00:00:00 2001 From: Marco Pagani Date: Tue, 5 Mar 2024 20:29:26 +0100 Subject: fpga: manager: add owner module and take its refcount The current implementation of the fpga manager assumes that the low-level module registers a driver for the parent device and uses its owner pointer to take the module's refcount. This approach is problematic since it can lead to a null pointer dereference while attempting to get the manager if the parent device does not have a driver. To address this problem, add a module owner pointer to the fpga_manager struct and use it to take the module's refcount. Modify the functions for registering the manager to take an additional owner module parameter and rename them to avoid conflicts. Use the old function names for helper macros that automatically set the module that registers the manager as the owner. This ensures compatibility with existing low-level control modules and reduces the chances of registering a manager without setting the owner. Also, update the documentation to keep it consistent with the new interface for registering an fpga manager. Other changes: opportunistically move put_device() from __fpga_mgr_get() to fpga_mgr_get() and of_fpga_mgr_get() to improve code clarity since the manager device is taken in these functions. Fixes: 654ba4cc0f3e ("fpga manager: ensure lifetime with of_fpga_mgr_get") Suggested-by: Greg Kroah-Hartman Suggested-by: Xu Yilun Signed-off-by: Marco Pagani Acked-by: Xu Yilun Link: https://lore.kernel.org/r/20240305192926.84886-1-marpagan@redhat.com Signed-off-by: Xu Yilun --- Documentation/driver-api/fpga/fpga-mgr.rst | 34 ++++++++++++++++++------------ 1 file changed, 20 insertions(+), 14 deletions(-) (limited to 'Documentation/driver-api') diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst b/Documentation/driver-api/fpga/fpga-mgr.rst index 49c0a9512653..8d2b79f696c1 100644 --- a/Documentation/driver-api/fpga/fpga-mgr.rst +++ b/Documentation/driver-api/fpga/fpga-mgr.rst @@ -24,7 +24,8 @@ How to support a new FPGA device -------------------------------- To add another FPGA manager, write a driver that implements a set of ops. The -probe function calls fpga_mgr_register() or fpga_mgr_register_full(), such as:: +probe function calls ``fpga_mgr_register()`` or ``fpga_mgr_register_full()``, +such as:: static const struct fpga_manager_ops socfpga_fpga_ops = { .write_init = socfpga_fpga_ops_configure_init, @@ -69,10 +70,11 @@ probe function calls fpga_mgr_register() or fpga_mgr_register_full(), such as:: } Alternatively, the probe function could call one of the resource managed -register functions, devm_fpga_mgr_register() or devm_fpga_mgr_register_full(). -When these functions are used, the parameter syntax is the same, but the call -to fpga_mgr_unregister() should be removed. In the above example, the -socfpga_fpga_remove() function would not be required. +register functions, ``devm_fpga_mgr_register()`` or +``devm_fpga_mgr_register_full()``. When these functions are used, the +parameter syntax is the same, but the call to ``fpga_mgr_unregister()`` should be +removed. In the above example, the ``socfpga_fpga_remove()`` function would not be +required. The ops will implement whatever device specific register writes are needed to do the programming sequence for this particular FPGA. These ops return 0 for @@ -125,15 +127,19 @@ API for implementing a new FPGA Manager driver * struct fpga_manager - the FPGA manager struct * struct fpga_manager_ops - Low level FPGA manager driver ops * struct fpga_manager_info - Parameter structure for fpga_mgr_register_full() -* fpga_mgr_register_full() - Create and register an FPGA manager using the +* __fpga_mgr_register_full() - Create and register an FPGA manager using the fpga_mgr_info structure to provide the full flexibility of options -* fpga_mgr_register() - Create and register an FPGA manager using standard +* __fpga_mgr_register() - Create and register an FPGA manager using standard arguments -* devm_fpga_mgr_register_full() - Resource managed version of - fpga_mgr_register_full() -* devm_fpga_mgr_register() - Resource managed version of fpga_mgr_register() +* __devm_fpga_mgr_register_full() - Resource managed version of + __fpga_mgr_register_full() +* __devm_fpga_mgr_register() - Resource managed version of __fpga_mgr_register() * fpga_mgr_unregister() - Unregister an FPGA manager +Helper macros ``fpga_mgr_register_full()``, ``fpga_mgr_register()``, +``devm_fpga_mgr_register_full()``, and ``devm_fpga_mgr_register()`` are available +to ease the registration. + .. kernel-doc:: include/linux/fpga/fpga-mgr.h :functions: fpga_mgr_states @@ -147,16 +153,16 @@ API for implementing a new FPGA Manager driver :functions: fpga_manager_info .. kernel-doc:: drivers/fpga/fpga-mgr.c - :functions: fpga_mgr_register_full + :functions: __fpga_mgr_register_full .. kernel-doc:: drivers/fpga/fpga-mgr.c - :functions: fpga_mgr_register + :functions: __fpga_mgr_register .. kernel-doc:: drivers/fpga/fpga-mgr.c - :functions: devm_fpga_mgr_register_full + :functions: __devm_fpga_mgr_register_full .. kernel-doc:: drivers/fpga/fpga-mgr.c - :functions: devm_fpga_mgr_register + :functions: __devm_fpga_mgr_register .. kernel-doc:: drivers/fpga/fpga-mgr.c :functions: fpga_mgr_unregister -- cgit v1.2.3 From 1da11f822042eb6ef4b6064dc048f157a7852529 Mon Sep 17 00:00:00 2001 From: Marco Pagani Date: Fri, 22 Mar 2024 18:18:37 +0100 Subject: fpga: bridge: add owner module and take its refcount The current implementation of the fpga bridge assumes that the low-level module registers a driver for the parent device and uses its owner pointer to take the module's refcount. This approach is problematic since it can lead to a null pointer dereference while attempting to get the bridge if the parent device does not have a driver. To address this problem, add a module owner pointer to the fpga_bridge struct and use it to take the module's refcount. Modify the function for registering a bridge to take an additional owner module parameter and rename it to avoid conflicts. Use the old function name for a helper macro that automatically sets the module that registers the bridge as the owner. This ensures compatibility with existing low-level control modules and reduces the chances of registering a bridge without setting the owner. Also, update the documentation to keep it consistent with the new interface for registering an fpga bridge. Other changes: opportunistically move put_device() from __fpga_bridge_get() to fpga_bridge_get() and of_fpga_bridge_get() to improve code clarity since the bridge device is taken in these functions. Fixes: 21aeda950c5f ("fpga: add fpga bridge framework") Suggested-by: Greg Kroah-Hartman Suggested-by: Xu Yilun Reviewed-by: Russ Weight Signed-off-by: Marco Pagani Acked-by: Xu Yilun Link: https://lore.kernel.org/r/20240322171839.233864-1-marpagan@redhat.com Signed-off-by: Xu Yilun --- Documentation/driver-api/fpga/fpga-bridge.rst | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'Documentation/driver-api') diff --git a/Documentation/driver-api/fpga/fpga-bridge.rst b/Documentation/driver-api/fpga/fpga-bridge.rst index 604208534095..833f68fb0700 100644 --- a/Documentation/driver-api/fpga/fpga-bridge.rst +++ b/Documentation/driver-api/fpga/fpga-bridge.rst @@ -6,9 +6,12 @@ API to implement a new FPGA bridge * struct fpga_bridge - The FPGA Bridge structure * struct fpga_bridge_ops - Low level Bridge driver ops -* fpga_bridge_register() - Create and register a bridge +* __fpga_bridge_register() - Create and register a bridge * fpga_bridge_unregister() - Unregister a bridge +The helper macro ``fpga_bridge_register()`` automatically sets +the module that registers the FPGA bridge as the owner. + .. kernel-doc:: include/linux/fpga/fpga-bridge.h :functions: fpga_bridge @@ -16,7 +19,7 @@ API to implement a new FPGA bridge :functions: fpga_bridge_ops .. kernel-doc:: drivers/fpga/fpga-bridge.c - :functions: fpga_bridge_register + :functions: __fpga_bridge_register .. kernel-doc:: drivers/fpga/fpga-bridge.c :functions: fpga_bridge_unregister -- cgit v1.2.3 From b7c0e1ecee403a43abc89eb3e75672b01ff2ece9 Mon Sep 17 00:00:00 2001 From: Marco Pagani Date: Fri, 19 Apr 2024 10:35:59 +0200 Subject: fpga: region: add owner module and take its refcount The current implementation of the fpga region assumes that the low-level module registers a driver for the parent device and uses its owner pointer to take the module's refcount. This approach is problematic since it can lead to a null pointer dereference while attempting to get the region during programming if the parent device does not have a driver. To address this problem, add a module owner pointer to the fpga_region struct and use it to take the module's refcount. Modify the functions for registering a region to take an additional owner module parameter and rename them to avoid conflicts. Use the old function names for helper macros that automatically set the module that registers the region as the owner. This ensures compatibility with existing low-level control modules and reduces the chances of registering a region without setting the owner. Also, update the documentation to keep it consistent with the new interface for registering an fpga region. Fixes: 0fa20cdfcc1f ("fpga: fpga-region: device tree control for FPGA") Suggested-by: Greg Kroah-Hartman Suggested-by: Xu Yilun Reviewed-by: Russ Weight Signed-off-by: Marco Pagani Acked-by: Xu Yilun Link: https://lore.kernel.org/r/20240419083601.77403-1-marpagan@redhat.com Signed-off-by: Xu Yilun --- Documentation/driver-api/fpga/fpga-region.rst | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'Documentation/driver-api') diff --git a/Documentation/driver-api/fpga/fpga-region.rst b/Documentation/driver-api/fpga/fpga-region.rst index dc55d60a0b4a..2d03b5fb7657 100644 --- a/Documentation/driver-api/fpga/fpga-region.rst +++ b/Documentation/driver-api/fpga/fpga-region.rst @@ -46,13 +46,16 @@ API to add a new FPGA region ---------------------------- * struct fpga_region - The FPGA region struct -* struct fpga_region_info - Parameter structure for fpga_region_register_full() -* fpga_region_register_full() - Create and register an FPGA region using the +* struct fpga_region_info - Parameter structure for __fpga_region_register_full() +* __fpga_region_register_full() - Create and register an FPGA region using the fpga_region_info structure to provide the full flexibility of options -* fpga_region_register() - Create and register an FPGA region using standard +* __fpga_region_register() - Create and register an FPGA region using standard arguments * fpga_region_unregister() - Unregister an FPGA region +Helper macros ``fpga_region_register()`` and ``fpga_region_register_full()`` +automatically set the module that registers the FPGA region as the owner. + The FPGA region's probe function will need to get a reference to the FPGA Manager it will be using to do the programming. This usually would happen during the region's probe function. @@ -82,10 +85,10 @@ following APIs to handle building or tearing down that list. :functions: fpga_region_info .. kernel-doc:: drivers/fpga/fpga-region.c - :functions: fpga_region_register_full + :functions: __fpga_region_register_full .. kernel-doc:: drivers/fpga/fpga-region.c - :functions: fpga_region_register + :functions: __fpga_region_register .. kernel-doc:: drivers/fpga/fpga-region.c :functions: fpga_region_unregister -- cgit v1.2.3