| Message ID | 20181105171457.28150-1-agust@denx.de (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | [v2] fpga: mgr: add notifier for manager register/unregister events | expand |
On Mon, Nov 5, 2018 at 11:15 AM Anatolij Gustschin <agust@denx.de> wrote: Hi Anatolij, > > Add API functions for registering and removing a notifier for FPGA > manager register/unregister events. Notify when a new FPGA manager > has been registered or when an existing manager is being removed. > This will help configuration interface drivers to get the notion > of low-level FPGA managers appearing or disappearing, when using > hotpluggable FPGA configuration devices (e.g. via USB-FPP or > USB-SPI adapters). Are you going to be adding any code upstream that uses this API? Alan
Hi Alan, On Mon, 5 Nov 2018 15:08:03 -0600 Alan Tull atull@kernel.org wrote: >On Mon, Nov 5, 2018 at 11:15 AM Anatolij Gustschin <agust@denx.de> wrote: > >Hi Anatolij, > >> >> Add API functions for registering and removing a notifier for FPGA >> manager register/unregister events. Notify when a new FPGA manager >> has been registered or when an existing manager is being removed. >> This will help configuration interface drivers to get the notion >> of low-level FPGA managers appearing or disappearing, when using >> hotpluggable FPGA configuration devices (e.g. via USB-FPP or >> USB-SPI adapters). > >Are you going to be adding any code upstream that uses this API? I'd like to add our fpga-cfg driver [1] in upstream which uses this API. Here [2] is a better readable README about it. Thanks, Anatolij [1] http://git.denx.de/?p=fpga-cfg.git;a=summary [2] https://github.com/vdsao/fpga-cfg/blob/master/README.md
On Tue, Nov 6, 2018 at 3:29 AM Anatolij Gustschin <agust@denx.de> wrote: > > Hi Alan, > > On Mon, 5 Nov 2018 15:08:03 -0600 > Alan Tull atull@kernel.org wrote: > > >On Mon, Nov 5, 2018 at 11:15 AM Anatolij Gustschin <agust@denx.de> wrote: > > > >Hi Anatolij, > > > >> > >> Add API functions for registering and removing a notifier for FPGA > >> manager register/unregister events. Notify when a new FPGA manager > >> has been registered or when an existing manager is being removed. > >> This will help configuration interface drivers to get the notion > >> of low-level FPGA managers appearing or disappearing, when using > >> hotpluggable FPGA configuration devices (e.g. via USB-FPP or > >> USB-SPI adapters). To be clear, this patch will need to wait to go upstream when there is a user for it. So you could resubmit it when you are submitting patches that use it. That way we won't be adding code and maintaining an API which has no upstream use. > > > >Are you going to be adding any code upstream that uses this API? > > I'd like to add our fpga-cfg driver [1] in upstream which uses this API. > Here [2] is a better readable README about it. This is adding a debugfs interface for FPGA manager. I've posted a patch for this recently and it got discussed [3]. My view is that FPGA manager debugfs is fine for debug/development work, but turning it on and leaving it on for production work is really wrong. I keep seeing it come up since there currently is not an accessible alternative if you are running without devicetree. It's more stable and secure if the kernel handles reprogramming, bridges (if they exist), and enumeration all together under kernel control rather than userspace handling them piecemeal. So any production FPGA interfaces need to be added on top of FPGA region (which coordinates programming and bridges) and if possible handle enumeration. The other benefit of adding the interface on top of FPGA regions is that supports a wider set of users, i.e. if you can control a region, you can handle users who need bridges as well as users who don't. > > Thanks, > > Anatolij > > > [1] http://git.denx.de/?p=fpga-cfg.git;a=summary > [2] https://github.com/vdsao/fpga-cfg/blob/master/README.md [3] https://lkml.org/lkml/2018/8/16/665
On Tue, Nov 6, 2018 at 11:22 AM Alan Tull <atull@kernel.org> wrote: > > On Tue, Nov 6, 2018 at 3:29 AM Anatolij Gustschin <agust@denx.de> wrote: > > > > Hi Alan, > > > > On Mon, 5 Nov 2018 15:08:03 -0600 > > Alan Tull atull@kernel.org wrote: > > > > >On Mon, Nov 5, 2018 at 11:15 AM Anatolij Gustschin <agust@denx.de> wrote: > > > > > >Hi Anatolij, > > > > > >> > > >> Add API functions for registering and removing a notifier for FPGA > > >> manager register/unregister events. Notify when a new FPGA manager > > >> has been registered or when an existing manager is being removed. > > >> This will help configuration interface drivers to get the notion > > >> of low-level FPGA managers appearing or disappearing, when using > > >> hotpluggable FPGA configuration devices (e.g. via USB-FPP or > > >> USB-SPI adapters). > > To be clear, this patch will need to wait to go upstream when there is > a user for it. So you could resubmit it when you are submitting > patches that use it. That way we won't be adding code and maintaining > an API which has no upstream use. > > > > > > >Are you going to be adding any code upstream that uses this API? > > > > I'd like to add our fpga-cfg driver [1] in upstream which uses this API. > > Here [2] is a better readable README about it. > > This is adding a debugfs interface for FPGA manager. I've posted a > patch for this recently and it got discussed [3]. > > My view is that FPGA manager debugfs is fine for debug/development > work, but turning it on and leaving it on for production work is > really wrong. I keep seeing it come up since there currently is not > an accessible alternative if you are running without devicetree. > > It's more stable and secure if the kernel handles reprogramming, > bridges (if they exist), and enumeration all together under kernel > control rather than userspace handling them piecemeal. So any > production FPGA interfaces need to be added on top of FPGA region > (which coordinates programming and bridges) and if possible handle > enumeration. The other benefit of adding the interface on top of FPGA > regions is that supports a wider set of users, i.e. if you can control > a region, you can handle users who need bridges as well as users who > don't. This interface also adds a way of adding a configuration description in a sysfs file that will be parsed in the kernel [2]. One thing that was discussed at length on the linux-fpga mailing list last year was adding a header to an FPGA image that could contain image specific information (Jason Gunthorpe's suggestion [4]). The desire was to keep the image specific information together with the image. There were a few proposals regarding the format of the header, with Moritz suggesting using FIT images [5] [6]. FIT has the advantage of being a standard that is already supported in the kernel (so we can avoid adding code that does string parsing to the kernel). Putting the image file together is easy to do using mkimage. I did a RFC [7]. > > > > > Thanks, > > > > Anatolij > > > > > > [1] http://git.denx.de/?p=fpga-cfg.git;a=summary > > [2] https://github.com/vdsao/fpga-cfg/blob/master/README.md > > [3] https://lkml.org/lkml/2018/8/16/665 [4] https://lkml.org/lkml/2017/2/15/469 [5] https://lkml.org/lkml/2017/2/15/544 [6] https://lkml.org/lkml/2017/2/15/641 [7] https://lkml.org/lkml/2017/7/24/649
diff --git a/Documentation/driver-api/fpga/fpga-mgr.rst b/Documentation/driver-api/fpga/fpga-mgr.rst index 576f1945eacd..fd736a22f5fc 100644 --- a/Documentation/driver-api/fpga/fpga-mgr.rst +++ b/Documentation/driver-api/fpga/fpga-mgr.rst @@ -125,3 +125,18 @@ API for implementing a new FPGA Manager driver .. kernel-doc:: drivers/fpga/fpga-mgr.c :functions: fpga_mgr_unregister + +Notification about added or removed FPGA managers +------------------------------------------------- + +To register or unregister the notifier callback for signalling +about the low level FPGA managers being added or removed, use + +* :c:func:`fpga_mgr_register_mgr_notifier` — Add notifier for manager add/remove event +* :c:func:`fpga_mgr_unregister_mgr_notifier` — Remove notifier for manager events + +.. kernel-doc:: drivers/fpga/fpga-mgr.c + :functions: fpga_mgr_register_mgr_notifier + +.. kernel-doc:: drivers/fpga/fpga-mgr.c + :functions: fpga_mgr_unregister_mgr_notifier diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c index c3866816456a..307717ad9c6d 100644 --- a/drivers/fpga/fpga-mgr.c +++ b/drivers/fpga/fpga-mgr.c @@ -21,6 +21,46 @@ static DEFINE_IDA(fpga_mgr_ida); static struct class *fpga_mgr_class; +static BLOCKING_NOTIFIER_HEAD(fpga_mgr_notifier_list); + +static int fpga_mgr_notify_registered(struct device *dev, void *data) +{ + struct fpga_manager *mgr = to_fpga_manager(dev); + + blocking_notifier_call_chain(&fpga_mgr_notifier_list, + FPGA_MGR_ADD, mgr); + return 0; +} + +/** + * fpga_mgr_register_mgr_notifier() - register fpga manager notifier callback + * @nb: pointer to the notifier block for the callback events. + * + * Add a notifier callback for FPGA manager changes. These changes are + * either FPGA manager being added or removed. + */ +void fpga_mgr_register_mgr_notifier(struct notifier_block *nb) +{ + blocking_notifier_chain_register(&fpga_mgr_notifier_list, nb); + + class_for_each_device(fpga_mgr_class, NULL, NULL, + fpga_mgr_notify_registered); +} +EXPORT_SYMBOL_GPL(fpga_mgr_register_mgr_notifier); + +/** + * fpga_mgr_unregister_mgr_notifier() - unregister a notifier callback + * @nb: pointer to the notifier block for the callback events. + * + * Remove a notifier callback. fpga_mgr_register_mgr_notifier() must have + * been previously called for this function to work properly. + */ +void fpga_mgr_unregister_mgr_notifier(struct notifier_block *nb) +{ + blocking_notifier_chain_unregister(&fpga_mgr_notifier_list, nb); +} +EXPORT_SYMBOL_GPL(fpga_mgr_unregister_mgr_notifier); + /** * fpga_image_info_alloc - Allocate a FPGA image info struct * @dev: owning device @@ -700,6 +740,8 @@ int fpga_mgr_register(struct fpga_manager *mgr) dev_info(&mgr->dev, "%s registered\n", mgr->name); + blocking_notifier_call_chain(&fpga_mgr_notifier_list, + FPGA_MGR_ADD, mgr); return 0; error_device: @@ -719,6 +761,9 @@ void fpga_mgr_unregister(struct fpga_manager *mgr) { dev_info(&mgr->dev, "%s %s\n", __func__, mgr->name); + blocking_notifier_call_chain(&fpga_mgr_notifier_list, + FPGA_MGR_REMOVE, mgr); + /* * If the low level driver provides a method for putting fpga into * a desired state upon unregister, do it. diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h index e8ca62b2cb5b..d6eaec80557b 100644 --- a/include/linux/fpga/fpga-mgr.h +++ b/include/linux/fpga/fpga-mgr.h @@ -10,6 +10,7 @@ #include <linux/mutex.h> #include <linux/platform_device.h> +#include <linux/notifier.h> struct fpga_manager; struct sg_table; @@ -202,4 +203,15 @@ struct fpga_manager *devm_fpga_mgr_create(struct device *dev, const char *name, const struct fpga_manager_ops *mops, void *priv); +/* + * FPGA Manager register notifier events + * FPGA_MGR_ADD: a new fpga manager has been registered + * FPGA_MGR_REMOVE: a registered fpga manager is being removed + */ +#define FPGA_MGR_ADD 1 +#define FPGA_MGR_REMOVE 2 + +void fpga_mgr_register_mgr_notifier(struct notifier_block *nb); +void fpga_mgr_unregister_mgr_notifier(struct notifier_block *nb); + #endif /*_LINUX_FPGA_MGR_H */
Add API functions for registering and removing a notifier for FPGA manager register/unregister events. Notify when a new FPGA manager has been registered or when an existing manager is being removed. This will help configuration interface drivers to get the notion of low-level FPGA managers appearing or disappearing, when using hotpluggable FPGA configuration devices (e.g. via USB-FPP or USB-SPI adapters). Signed-off-by: Anatolij Gustschin <agust@denx.de> --- Changes in v2: - rebased on v4.20-rc1, reworked documentation - also notify about all fpga managers that were registered before adding the manager register/unregister notifier Documentation/driver-api/fpga/fpga-mgr.rst | 15 ++++++++ drivers/fpga/fpga-mgr.c | 45 ++++++++++++++++++++++ include/linux/fpga/fpga-mgr.h | 12 ++++++ 3 files changed, 72 insertions(+)