All of lore.kernel.org
 help / color / mirror / Atom feed
From: atull@opensource.altera.com (atull)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/4] fpga mgr: Introduce FPGA capabilities
Date: Mon, 14 Nov 2016 08:06:52 -0600	[thread overview]
Message-ID: <alpine.DEB.2.10.1611140801480.2786@atull-VirtualBox> (raw)
In-Reply-To: <20161107001326.7395-2-moritz.fischer@ettus.com>

On Mon, 7 Nov 2016, Moritz Fischer wrote:

> Add FPGA capabilities as a way to express the capabilities
> of a given FPGA manager.
> 
> Removes code duplication by comparing the low-level driver's
> capabilities at the framework level rather than having each driver
> check for supported operations in the write_init() callback.
> 
> This allows for extending with additional capabilities, similar
> to the the dmaengine framework's implementation.
> 
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> Cc: Alan Tull <atull@opensource.altera.com>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: S?ren Brinkmann <soren.brinkmann@xilinx.com>
> Cc: linux-kernel at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> ---
> 
> Changes from RFC:
> * in the RFC the caps weren't actually stored into the struct fpga_mgr
> 
> Note:
> 
> If people disagree on the typedef being a 'false positive' I can fix
> that in a future rev of the patchset.
> 
> Thanks,
> 
>     Moritz
> 
> ---
>  drivers/fpga/fpga-mgr.c       | 15 ++++++++++++++
>  drivers/fpga/socfpga.c        | 10 +++++-----
>  drivers/fpga/zynq-fpga.c      |  7 ++++++-
>  include/linux/fpga/fpga-mgr.h | 46 ++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 71 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index 953dc91..ed57c17 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -49,6 +49,18 @@ int fpga_mgr_buf_load(struct fpga_manager *mgr, u32 flags, const char *buf,
>  	struct device *dev = &mgr->dev;
>  	int ret;
>  
> +	if (flags & FPGA_MGR_PARTIAL_RECONFIG &&
> +	    !fpga_mgr_has_cap(FPGA_MGR_CAP_PARTIAL_RECONF, mgr->caps)) {
> +		dev_err(dev, "Partial reconfiguration not supported\n");
> +		return -ENOTSUPP;
> +	}
> +
> +	if (flags & FPGA_MGR_FULL_RECONFIG &&
> +	    !fpga_mgr_has_cap(FPGA_MGR_CAP_FULL_RECONF, mgr->caps)) {
> +		dev_err(dev, "Full reconfiguration not supported\n");
> +		return -ENOTSUPP;
> +	}
> +

Could you move the checks to their own function like
'fpga_mgr_check_caps()' or something?  I really like it if we can keep
the functions short, like a screen or so where it's practicle to do
so and I could see the number of caps growing here.  The only
counter argument I could think of is if a cap affects the sequence
in this function.  Hmmm...

Alan

WARNING: multiple messages have this Message-ID (diff)
From: atull <atull@opensource.altera.com>
To: Moritz Fischer <moritz.fischer@ettus.com>
Cc: <linux-kernel@vger.kernel.org>,
	<moritz.fischer.private@gmail.com>, <michal.simek@xilinx.com>,
	<soren.brinkmann@xilinx.com>,
	<linux-arm-kernel@lists.infradead.org>, <julia@ni.com>
Subject: Re: [PATCH 1/4] fpga mgr: Introduce FPGA capabilities
Date: Mon, 14 Nov 2016 08:06:52 -0600	[thread overview]
Message-ID: <alpine.DEB.2.10.1611140801480.2786@atull-VirtualBox> (raw)
In-Reply-To: <20161107001326.7395-2-moritz.fischer@ettus.com>

[-- Attachment #1: Type: text/plain, Size: 2357 bytes --]

On Mon, 7 Nov 2016, Moritz Fischer wrote:

> Add FPGA capabilities as a way to express the capabilities
> of a given FPGA manager.
> 
> Removes code duplication by comparing the low-level driver's
> capabilities at the framework level rather than having each driver
> check for supported operations in the write_init() callback.
> 
> This allows for extending with additional capabilities, similar
> to the the dmaengine framework's implementation.
> 
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> Cc: Alan Tull <atull@opensource.altera.com>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: Sören Brinkmann <soren.brinkmann@xilinx.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> ---
> 
> Changes from RFC:
> * in the RFC the caps weren't actually stored into the struct fpga_mgr
> 
> Note:
> 
> If people disagree on the typedef being a 'false positive' I can fix
> that in a future rev of the patchset.
> 
> Thanks,
> 
>     Moritz
> 
> ---
>  drivers/fpga/fpga-mgr.c       | 15 ++++++++++++++
>  drivers/fpga/socfpga.c        | 10 +++++-----
>  drivers/fpga/zynq-fpga.c      |  7 ++++++-
>  include/linux/fpga/fpga-mgr.h | 46 ++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 71 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index 953dc91..ed57c17 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -49,6 +49,18 @@ int fpga_mgr_buf_load(struct fpga_manager *mgr, u32 flags, const char *buf,
>  	struct device *dev = &mgr->dev;
>  	int ret;
>  
> +	if (flags & FPGA_MGR_PARTIAL_RECONFIG &&
> +	    !fpga_mgr_has_cap(FPGA_MGR_CAP_PARTIAL_RECONF, mgr->caps)) {
> +		dev_err(dev, "Partial reconfiguration not supported\n");
> +		return -ENOTSUPP;
> +	}
> +
> +	if (flags & FPGA_MGR_FULL_RECONFIG &&
> +	    !fpga_mgr_has_cap(FPGA_MGR_CAP_FULL_RECONF, mgr->caps)) {
> +		dev_err(dev, "Full reconfiguration not supported\n");
> +		return -ENOTSUPP;
> +	}
> +

Could you move the checks to their own function like
'fpga_mgr_check_caps()' or something?  I really like it if we can keep
the functions short, like a screen or so where it's practicle to do
so and I could see the number of caps growing here.  The only
counter argument I could think of is if a cap affects the sequence
in this function.  Hmmm...

Alan

  parent reply	other threads:[~2016-11-14 14:06 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-07  0:13 [PATCH 0/4] fpga mgr: Add support for capabilities & encrypted bistreams Moritz Fischer
2016-11-07  0:13 ` Moritz Fischer
2016-11-07  0:13 ` [PATCH 1/4] fpga mgr: Introduce FPGA capabilities Moritz Fischer
2016-11-07  0:13   ` Moritz Fischer
2016-11-14 14:01   ` atull
2016-11-14 14:01     ` atull
2016-11-14 14:06   ` atull [this message]
2016-11-14 14:06     ` atull
2016-11-14 17:26     ` Moritz Fischer
2016-11-14 17:26       ` Moritz Fischer
2016-11-14 23:23       ` atull
2016-11-14 23:23         ` atull
2016-11-07  0:13 ` [PATCH 2/4] fpga mgr: Expose FPGA capabilities to userland via sysfs Moritz Fischer
2016-11-07  0:13   ` Moritz Fischer
2016-11-14 14:33   ` atull
2016-11-14 14:33     ` atull
2016-11-07  0:13 ` [PATCH 3/4] fpga mgr: zynq: Add support for encrypted bitstreams Moritz Fischer
2016-11-07  0:13   ` Moritz Fischer
2016-11-08 18:32   ` Sören Brinkmann
2016-11-08 18:32     ` Sören Brinkmann
2016-11-08 18:59     ` Moritz Fischer
2016-11-08 18:59       ` Moritz Fischer
2016-11-15  2:42   ` atull
2016-11-15  2:42     ` atull
2016-11-15  3:25     ` Moritz Fischer
2016-11-15  3:25       ` Moritz Fischer
2016-11-07  0:13 ` [PATCH 4/4] fpga mgr: socfpga: Expose " Moritz Fischer
2016-11-07  0:13   ` Moritz Fischer
2016-11-13 22:37   ` atull
2016-11-13 22:37     ` atull

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.10.1611140801480.2786@atull-VirtualBox \
    --to=atull@opensource.altera.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.