Re: [PATCH v5 3/7] fpga: dfl: introduce interrupt trigger setting API

2020-05-25 Thread Marcelo Tosatti
On Mon, Apr 20, 2020 at 04:11:39PM +0800, Xu Yilun wrote:
> FPGA user applications may be interested in interrupts generated by
> DFL features. For example, users can implement their own FPGA
> logics with interrupts enabled in AFU (Accelerated Function Unit,
> dynamic region of DFL based FPGA). So user applications need to be
> notified to handle these interrupts.
> 
> In order to allow userspace applications to monitor interrupts,
> driver requires userspace to provide eventfds as interrupt
> notification channels. Applications then poll/select on the eventfds
> to get notified.
> 
> This patch introduces a generic helper functions to do eventfds binding
> with given interrupts.
> 
> Sub feature drivers are expected to use XXX_GET_IRQ_NUM to query irq
> info, and XXX_SET_IRQ to set eventfds for interrupts. This patch also
> introduces helper functions for these 2 ioctls.
> 
> Signed-off-by: Luwei Kang 
> Signed-off-by: Wu Hao 
> Signed-off-by: Xu Yilun 
> Acked-by: Wu Hao 
> 
> v2: use unsigned int instead of int for irq array indexes in
> dfl_fpga_set_irq_triggers()
> Improves comments for NULL fds param in dfl_fpga_set_irq_triggers()
> v3: Improve comments of dfl_fpga_set_irq_triggers()
> refines code for dfl_fpga_set_irq_triggers, delete local variable j
> v4: Introduce 2 helper functions to help handle the XXX_GET_IRQ_NUM &
> XXX_SET_IRQ ioctls for sub feature drivers.
> v5: Some minor fix for Hao's comments
> ---
>  drivers/fpga/dfl.c| 156 
> ++
>  drivers/fpga/dfl.h|  17 +
>  include/uapi/linux/fpga-dfl.h |  13 
>  3 files changed, 186 insertions(+)
> 
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index b49fbed..208d8f0 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -10,7 +10,9 @@
>   *   Wu Hao 
>   *   Xiao Guangrong 
>   */
> +#include 
>  #include 
> +#include 
>  
>  #include "dfl.h"
>  
> @@ -534,6 +536,7 @@ static int build_info_commit_dev(struct 
> build_feature_devs_info *binfo)
>   unsigned int i;
>  
>   /* save resource information for each feature */
> + feature->dev = fdev;
>   feature->id = finfo->fid;
>   feature->resource_index = index;
>   feature->ioaddr = finfo->ioaddr;
> @@ -1395,6 +1398,159 @@ int dfl_fpga_cdev_config_ports_vf(struct 
> dfl_fpga_cdev *cdev, int num_vfs)
>  }
>  EXPORT_SYMBOL_GPL(dfl_fpga_cdev_config_ports_vf);
>  
> +static irqreturn_t dfl_irq_handler(int irq, void *arg)
> +{
> + struct eventfd_ctx *trigger = arg;
> +
> + eventfd_signal(trigger, 1);
> + return IRQ_HANDLED;
> +}
> +
> +static int do_set_irq_trigger(struct dfl_feature *feature, unsigned int idx,
> +   int fd)
> +{
> + struct platform_device *pdev = feature->dev;
> + struct eventfd_ctx *trigger;
> + int irq, ret;
> +
> + if (idx >= feature->nr_irqs)
> + return -EINVAL;
> +
> + irq = feature->irq_ctx[idx].irq;
> +
> + if (feature->irq_ctx[idx].trigger) {
> + free_irq(irq, feature->irq_ctx[idx].trigger);
> + kfree(feature->irq_ctx[idx].name);
> + eventfd_ctx_put(feature->irq_ctx[idx].trigger);
> + feature->irq_ctx[idx].trigger = NULL;
> + }
> +
> + if (fd < 0)
> + return 0;
> +
> + feature->irq_ctx[idx].name =
> + kasprintf(GFP_KERNEL, "fpga-irq[%u](%s-%llx)", idx,
> +   dev_name(>dev),
> +   (unsigned long long)feature->id);
> + if (!feature->irq_ctx[idx].name)
> + return -ENOMEM;
> +
> + trigger = eventfd_ctx_fdget(fd);
> + if (IS_ERR(trigger)) {
> + ret = PTR_ERR(trigger);
> + goto free_name;
> + }
> +
> + ret = request_irq(irq, dfl_irq_handler, 0,
> +   feature->irq_ctx[idx].name, trigger);
> + if (!ret) {
> + feature->irq_ctx[idx].trigger = trigger;
> + return ret;
> + }
> +
> + eventfd_ctx_put(trigger);
> +free_name:
> + kfree(feature->irq_ctx[idx].name);
> +
> + return ret;
> +}
> +
> +/**
> + * dfl_fpga_set_irq_triggers - set eventfd triggers for dfl feature 
> interrupts
> + *
> + * @feature: dfl sub feature.
> + * @start: start of irq index in this dfl sub feature.
> + * @count: number of irqs.
> + * @fds: eventfds to bind with irqs. unbind related irq if fds[n] is 
> negative.
> + *unbind "count" specified number of irqs if fds ptr is NULL.
> + *
> + * Bind given eventfds with irqs in this dfl sub feature. Unbind related irq 
> if
> + * fds[n] is negative. Unbind "count" specified number of irqs if fds ptr is
> + * NULL.
> + *
> + * Return: 0 on success, negative error code otherwise.
> + */
> +int dfl_fpga_set_irq_triggers(struct dfl_feature *feature, unsigned int 
> start,
> +   unsigned int count, int32_t *fds)
> +{
> + unsigned int i;
> + int ret 

Re: [PATCH v5 3/7] fpga: dfl: introduce interrupt trigger setting API

2020-05-13 Thread Xu Yilun
On Mon, May 11, 2020 at 09:16:34PM -0700, Moritz Fischer wrote:
> On Mon, Apr 20, 2020 at 04:11:39PM +0800, Xu Yilun wrote:
> > FPGA user applications may be interested in interrupts generated by
> > DFL features. For example, users can implement their own FPGA
> > logics with interrupts enabled in AFU (Accelerated Function Unit,
> > dynamic region of DFL based FPGA). So user applications need to be
> > notified to handle these interrupts.
> > 
> > In order to allow userspace applications to monitor interrupts,
> > driver requires userspace to provide eventfds as interrupt
> > notification channels. Applications then poll/select on the eventfds
> > to get notified.
> > 
> > This patch introduces a generic helper functions to do eventfds binding
> > with given interrupts.
> > 
> > Sub feature drivers are expected to use XXX_GET_IRQ_NUM to query irq
> > info, and XXX_SET_IRQ to set eventfds for interrupts. This patch also
> > introduces helper functions for these 2 ioctls.
> > 
> > Signed-off-by: Luwei Kang 
> > Signed-off-by: Wu Hao 
> > Signed-off-by: Xu Yilun 
> > Acked-by: Wu Hao 
> > 
> > v2: use unsigned int instead of int for irq array indexes in
> > dfl_fpga_set_irq_triggers()
> > Improves comments for NULL fds param in dfl_fpga_set_irq_triggers()
> > v3: Improve comments of dfl_fpga_set_irq_triggers()
> > refines code for dfl_fpga_set_irq_triggers, delete local variable j
> > v4: Introduce 2 helper functions to help handle the XXX_GET_IRQ_NUM &
> > XXX_SET_IRQ ioctls for sub feature drivers.
> > v5: Some minor fix for Hao's comments
> > ---
> >  drivers/fpga/dfl.c| 156 
> > ++
> >  drivers/fpga/dfl.h|  17 +
> >  include/uapi/linux/fpga-dfl.h |  13 
> >  3 files changed, 186 insertions(+)
> > 
> > diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> > index b49fbed..208d8f0 100644
> > --- a/drivers/fpga/dfl.c
> > +++ b/drivers/fpga/dfl.c
> > @@ -10,7 +10,9 @@
> >   *   Wu Hao 
> >   *   Xiao Guangrong 
> >   */
> > +#include 
> >  #include 
> > +#include 
> >  
> >  #include "dfl.h"
> >  
> > @@ -534,6 +536,7 @@ static int build_info_commit_dev(struct 
> > build_feature_devs_info *binfo)
> > unsigned int i;
> >  
> > /* save resource information for each feature */
> > +   feature->dev = fdev;
> > feature->id = finfo->fid;
> > feature->resource_index = index;
> > feature->ioaddr = finfo->ioaddr;
> > @@ -1395,6 +1398,159 @@ int dfl_fpga_cdev_config_ports_vf(struct 
> > dfl_fpga_cdev *cdev, int num_vfs)
> >  }
> >  EXPORT_SYMBOL_GPL(dfl_fpga_cdev_config_ports_vf);
> >  
> > +static irqreturn_t dfl_irq_handler(int irq, void *arg)
> > +{
> > +   struct eventfd_ctx *trigger = arg;
> > +
> > +   eventfd_signal(trigger, 1);
> > +   return IRQ_HANDLED;
> > +}
> > +
> > +static int do_set_irq_trigger(struct dfl_feature *feature, unsigned int 
> > idx,
> > + int fd)
> > +{
> > +   struct platform_device *pdev = feature->dev;
> > +   struct eventfd_ctx *trigger;
> > +   int irq, ret;
> > +
> > +   if (idx >= feature->nr_irqs)
> > +   return -EINVAL;
> > +
> > +   irq = feature->irq_ctx[idx].irq;
> > +
> > +   if (feature->irq_ctx[idx].trigger) {
> > +   free_irq(irq, feature->irq_ctx[idx].trigger);
> > +   kfree(feature->irq_ctx[idx].name);
> > +   eventfd_ctx_put(feature->irq_ctx[idx].trigger);
> > +   feature->irq_ctx[idx].trigger = NULL;
> > +   }
> > +
> > +   if (fd < 0)
> > +   return 0;
> > +
> > +   feature->irq_ctx[idx].name =
> > +   kasprintf(GFP_KERNEL, "fpga-irq[%u](%s-%llx)", idx,
> > + dev_name(>dev),
> > + (unsigned long long)feature->id);
> > +   if (!feature->irq_ctx[idx].name)
> > +   return -ENOMEM;
> > +
> > +   trigger = eventfd_ctx_fdget(fd);
> > +   if (IS_ERR(trigger)) {
> > +   ret = PTR_ERR(trigger);
> > +   goto free_name;
> > +   }
> > +
> > +   ret = request_irq(irq, dfl_irq_handler, 0,
> > + feature->irq_ctx[idx].name, trigger);
> > +   if (!ret) {
> > +   feature->irq_ctx[idx].trigger = trigger;
> > +   return ret;
> > +   }
> > +
> > +   eventfd_ctx_put(trigger);
> > +free_name:
> > +   kfree(feature->irq_ctx[idx].name);
> > +
> > +   return ret;
> > +}
> > +
> > +/**
> > + * dfl_fpga_set_irq_triggers - set eventfd triggers for dfl feature 
> > interrupts
> > + *
> > + * @feature: dfl sub feature.
> > + * @start: start of irq index in this dfl sub feature.
> > + * @count: number of irqs.
> > + * @fds: eventfds to bind with irqs. unbind related irq if fds[n] is 
> > negative.
> > + *  unbind "count" specified number of irqs if fds ptr is NULL.
> > + *
> > + * Bind given eventfds with irqs in this dfl sub feature. Unbind related 
> > irq if
> > + * fds[n] is negative. Unbind "count" specified number of irqs if fds ptr 
> > is
> > + * NULL.
> > 

Re: [PATCH v5 3/7] fpga: dfl: introduce interrupt trigger setting API

2020-05-11 Thread Moritz Fischer
On Mon, Apr 20, 2020 at 04:11:39PM +0800, Xu Yilun wrote:
> FPGA user applications may be interested in interrupts generated by
> DFL features. For example, users can implement their own FPGA
> logics with interrupts enabled in AFU (Accelerated Function Unit,
> dynamic region of DFL based FPGA). So user applications need to be
> notified to handle these interrupts.
> 
> In order to allow userspace applications to monitor interrupts,
> driver requires userspace to provide eventfds as interrupt
> notification channels. Applications then poll/select on the eventfds
> to get notified.
> 
> This patch introduces a generic helper functions to do eventfds binding
> with given interrupts.
> 
> Sub feature drivers are expected to use XXX_GET_IRQ_NUM to query irq
> info, and XXX_SET_IRQ to set eventfds for interrupts. This patch also
> introduces helper functions for these 2 ioctls.
> 
> Signed-off-by: Luwei Kang 
> Signed-off-by: Wu Hao 
> Signed-off-by: Xu Yilun 
> Acked-by: Wu Hao 
> 
> v2: use unsigned int instead of int for irq array indexes in
> dfl_fpga_set_irq_triggers()
> Improves comments for NULL fds param in dfl_fpga_set_irq_triggers()
> v3: Improve comments of dfl_fpga_set_irq_triggers()
> refines code for dfl_fpga_set_irq_triggers, delete local variable j
> v4: Introduce 2 helper functions to help handle the XXX_GET_IRQ_NUM &
> XXX_SET_IRQ ioctls for sub feature drivers.
> v5: Some minor fix for Hao's comments
> ---
>  drivers/fpga/dfl.c| 156 
> ++
>  drivers/fpga/dfl.h|  17 +
>  include/uapi/linux/fpga-dfl.h |  13 
>  3 files changed, 186 insertions(+)
> 
> diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> index b49fbed..208d8f0 100644
> --- a/drivers/fpga/dfl.c
> +++ b/drivers/fpga/dfl.c
> @@ -10,7 +10,9 @@
>   *   Wu Hao 
>   *   Xiao Guangrong 
>   */
> +#include 
>  #include 
> +#include 
>  
>  #include "dfl.h"
>  
> @@ -534,6 +536,7 @@ static int build_info_commit_dev(struct 
> build_feature_devs_info *binfo)
>   unsigned int i;
>  
>   /* save resource information for each feature */
> + feature->dev = fdev;
>   feature->id = finfo->fid;
>   feature->resource_index = index;
>   feature->ioaddr = finfo->ioaddr;
> @@ -1395,6 +1398,159 @@ int dfl_fpga_cdev_config_ports_vf(struct 
> dfl_fpga_cdev *cdev, int num_vfs)
>  }
>  EXPORT_SYMBOL_GPL(dfl_fpga_cdev_config_ports_vf);
>  
> +static irqreturn_t dfl_irq_handler(int irq, void *arg)
> +{
> + struct eventfd_ctx *trigger = arg;
> +
> + eventfd_signal(trigger, 1);
> + return IRQ_HANDLED;
> +}
> +
> +static int do_set_irq_trigger(struct dfl_feature *feature, unsigned int idx,
> +   int fd)
> +{
> + struct platform_device *pdev = feature->dev;
> + struct eventfd_ctx *trigger;
> + int irq, ret;
> +
> + if (idx >= feature->nr_irqs)
> + return -EINVAL;
> +
> + irq = feature->irq_ctx[idx].irq;
> +
> + if (feature->irq_ctx[idx].trigger) {
> + free_irq(irq, feature->irq_ctx[idx].trigger);
> + kfree(feature->irq_ctx[idx].name);
> + eventfd_ctx_put(feature->irq_ctx[idx].trigger);
> + feature->irq_ctx[idx].trigger = NULL;
> + }
> +
> + if (fd < 0)
> + return 0;
> +
> + feature->irq_ctx[idx].name =
> + kasprintf(GFP_KERNEL, "fpga-irq[%u](%s-%llx)", idx,
> +   dev_name(>dev),
> +   (unsigned long long)feature->id);
> + if (!feature->irq_ctx[idx].name)
> + return -ENOMEM;
> +
> + trigger = eventfd_ctx_fdget(fd);
> + if (IS_ERR(trigger)) {
> + ret = PTR_ERR(trigger);
> + goto free_name;
> + }
> +
> + ret = request_irq(irq, dfl_irq_handler, 0,
> +   feature->irq_ctx[idx].name, trigger);
> + if (!ret) {
> + feature->irq_ctx[idx].trigger = trigger;
> + return ret;
> + }
> +
> + eventfd_ctx_put(trigger);
> +free_name:
> + kfree(feature->irq_ctx[idx].name);
> +
> + return ret;
> +}
> +
> +/**
> + * dfl_fpga_set_irq_triggers - set eventfd triggers for dfl feature 
> interrupts
> + *
> + * @feature: dfl sub feature.
> + * @start: start of irq index in this dfl sub feature.
> + * @count: number of irqs.
> + * @fds: eventfds to bind with irqs. unbind related irq if fds[n] is 
> negative.
> + *unbind "count" specified number of irqs if fds ptr is NULL.
> + *
> + * Bind given eventfds with irqs in this dfl sub feature. Unbind related irq 
> if
> + * fds[n] is negative. Unbind "count" specified number of irqs if fds ptr is
> + * NULL.
> + *
> + * Return: 0 on success, negative error code otherwise.
> + */
> +int dfl_fpga_set_irq_triggers(struct dfl_feature *feature, unsigned int 
> start,
> +   unsigned int count, int32_t *fds)
> +{
> + unsigned int i;
> + int ret