Message-ID: <CANk1AXTqNiR8Wy5K7RzOR9rn=URWf3gXsusmvN3JE2Z=e5wt+Q@mail.gmail.com>
Date:   Thu, 17 Aug 2017 14:52:17 -0500
From:   Alan Tull <atull@...nel.org>
To:     Moritz Fischer <moritz.fischer@...us.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-fpga@...r.kernel.org
Subject: Re: [PATCH v3 01/16] doc: fpga: update documents for the FPGA API

On Thu, Aug 17, 2017 at 2:42 PM, Moritz Fischer
<moritz.fischer@...us.com> wrote:
> Hi Alan,
>
> I think that should go together with [3/16] in one commit,
> to make API change go together with actual change?

Yes, the patches will be going in together anyway (not through
separate maintainers), so I could do that.  I hope to post the next
version in a couple weeks.

Alan

>
> On Thu, Jul 6, 2017 at 11:47 AM, Alan Tull <atull@...nel.org> wrote:
>> The FPGA manager has been simplified to have a single
>> fpga_mgr_load function which replaces the three
>> fpga_mgr_*load* functions.
>>
>> The parameters presenting the FPGA image have been
>> added to struct fpga_image_info.
>>
>> Additional functions have been added to alloc/free
>> fpga_image_info.
>>
>> Getting a FPGA manager has been separated from
>> locking it.  So an unlocked reference can be saved and
>> only locked when we're about to program.
>>
>> A document for fpga-region has been added as well.
>>
>> Signed-off-by: Alan Tull <atull@...nel.org>
>> ---
>>  Documentation/fpga/fpga-mgr.txt    | 133 +++++++++++++++++++------------------
>>  Documentation/fpga/fpga-region.txt |  95 ++++++++++++++++++++++++++
>>  Documentation/fpga/overview.txt    |  23 +++++++
>>  3 files changed, 188 insertions(+), 63 deletions(-)
>>  create mode 100644 Documentation/fpga/fpga-region.txt
>>  create mode 100644 Documentation/fpga/overview.txt
>>
>> diff --git a/Documentation/fpga/fpga-mgr.txt b/Documentation/fpga/fpga-mgr.txt
>> index 78f197f..0fc41eb 100644
>> --- a/Documentation/fpga/fpga-mgr.txt
>> +++ b/Documentation/fpga/fpga-mgr.txt
>> @@ -11,61 +11,66 @@ hidden away in a low level driver which registers a set of ops with the core.
>>  The FPGA image data itself is very manufacturer specific, but for our purposes
>>  it's just binary data.  The FPGA manager core won't parse it.
>>
>> +The FPGA image to be programmed can be in a scatter gather list, a single
>> +contiguous buffer, or a firmware file.  Because allocating contiguous kernel
>> +memory for the buffer should be avoided, users are encouraged to use a scatter
>> +gather list instead if possible.
>> +
>> +The particulars for programming the image are presented in a structure (struct
>> +fpga_image_info).  This struct contains parameters such as pointers to the
>> +FPGA image as well as image-specific particulars such as whether the image was
>> +built for full or partial reconfiguration.
>>
>>  API Functions:
>>  ==============
>>
>> -To program the FPGA from a file or from a buffer:
>> --------------------------------------------------
>> -
>> -       int fpga_mgr_buf_load(struct fpga_manager *mgr,
>> -                             struct fpga_image_info *info,
>> -                             const char *buf, size_t count);
>> -
>> -Load the FPGA from an image which exists as a contiguous buffer in
>> -memory. Allocating contiguous kernel memory for the buffer should be avoided,
>> -users are encouraged to use the _sg interface instead of this.
>> -
>> -        int fpga_mgr_buf_load_sg(struct fpga_manager *mgr,
>> -                                struct fpga_image_info *info,
>> -                                struct sg_table *sgt);
>> +To program the FPGA:
>> +--------------------
>>
>> -Load the FPGA from an image from non-contiguous in memory. Callers can
>> -construct a sg_table using alloc_page backed memory.
>> +       int fpga_mgr_load(struct fpga_manager *mgr,
>> +                         struct fpga_image_info *info);
>>
>> -       int fpga_mgr_firmware_load(struct fpga_manager *mgr,
>> -                                  struct fpga_image_info *info,
>> -                                  const char *image_name);
>> -
>> -Load the FPGA from an image which exists as a file.  The image file must be on
>> -the firmware search path (see the firmware class documentation).  If successful,
>> +Load the FPGA from an image which is indicated in the info.  If successful,
>>  the FPGA ends up in operating mode.  Return 0 on success or a negative error
>>  code.
>>
>> -A FPGA design contained in a FPGA image file will likely have particulars that
>> -affect how the image is programmed to the FPGA.  These are contained in struct
>> -fpga_image_info.  Currently the only such particular is a single flag bit
>> -indicating whether the image is for full or partial reconfiguration.
>> +To allocate or free a struct fpga_image_info:
>> +---------------------------------------------
>> +
>> +       struct fpga_image_info *fpga_image_info_alloc(struct device *dev);
>> +
>> +       void fpga_image_info_free(struct device *dev,
>> +                                 struct fpga_image_info *info);
>>
>>  To get/put a reference to a FPGA manager:
>>  -----------------------------------------
>>
>>         struct fpga_manager *of_fpga_mgr_get(struct device_node *node);
>>         struct fpga_manager *fpga_mgr_get(struct device *dev);
>> +       void fpga_mgr_put(struct fpga_manager *mgr);
>>
>> -Given a DT node or device, get an exclusive reference to a FPGA manager.
>> +Given a DT node or device, get a reference to a FPGA manager.  This pointer
>> +can be saved until you are ready to program the FPGA.  fpga_mgr_put
>> +releases the reference.
>>
>> -       void fpga_mgr_put(struct fpga_manager *mgr);
>>
>> -Release the reference.
>> +To get exclusive control of a FPGA manager:
>> +-------------------------------------------
>> +
>> +       int fpga_mgr_lock(struct fpga_magager *mgr);
>> +       void fpga_mgr_unlock(struct fpga_magager *mgr);
>> +
>> +The user should call fpga_mgr_lock and verify that it returns 0 before
>> +attempting to program the FPGA.  Likeqise, the user should call
>> +fpga_mgr_unlock when done programming the FPGA.
>>
>>
>>  To register or unregister the low level FPGA-specific driver:
>>  -------------------------------------------------------------
>>
>>         int fpga_mgr_register(struct device *dev, const char *name,
>> -                             const struct fpga_manager_ops *mops,
>> -                             void *priv);
>> +                             const struct fpga_manager_ops *mops,
>> +                             void *priv);
>>
>>         void fpga_mgr_unregister(struct device *dev);
>>
>> @@ -78,59 +83,61 @@ How to write an image buffer to a supported FPGA
>>  /* Include to get the API */
>>  #include <linux/fpga/fpga-mgr.h>
>>
>> -/* device node that specifies the FPGA manager to use */
>> -struct device_node *mgr_node = ...
>> +struct fpga_manager *mgr;
>> +struct fpga_image_info *info;
>> +int ret;
>>
>> -/* FPGA image is in this buffer.  count is size of the buffer. */
>> -char *buf = ...
>> -int count = ...
>> +/*
>> + * Get a reference to FPGA manager.  This example uses the  device node of the
>> + * manager.  You could use fpga_mgr_get() instead if you have the device instead
>> + * of the device node.
>> + */
>> +mgr = of_fpga_mgr_get(mgr_node);
>>
>>  /* struct with information about the FPGA image to program. */
>> -struct fpga_image_info info;
>> +info = fpga_image_info_alloc(dev);
>>
>>  /* flags indicates whether to do full or partial reconfiguration */
>> -info.flags = 0;
>> +info->flags = FPGA_MGR_PARTIAL_RECONFIG;
>>
>> -int ret;
>> +/*
>> + * At this point, indicate where the image is. This is pseudo-code; you're
>> + * probably going to use one of these three.
>> + */
>> +if (using scatter gather) {
>>
>> -/* Get exclusive control of FPGA manager */
>> -struct fpga_manager *mgr = of_fpga_mgr_get(mgr_node);
>> +       info->sgt = [your scatter gather table]
>>
>> -/* Load the buffer to the FPGA */
>> -ret = fpga_mgr_buf_load(mgr, &info, buf, count);
>> -
>> -/* Release the FPGA manager */
>> -fpga_mgr_put(mgr);
>> +} else if (using a buffer) {
>>
>> +       info->buf = [your image buffer]
>> +       info->count = [image buffer size]
>>
>> -How to write an image file to a supported FPGA
>> -==============================================
>> -/* Include to get the API */
>> -#include <linux/fpga/fpga-mgr.h>
>> -
>> -/* device node that specifies the FPGA manager to use */
>> -struct device_node *mgr_node = ...
>> +} else if (using a firmware file) {
>>
>> -/* FPGA image is in this file which is in the firmware search path */
>> -const char *path = "fpga-image-9.rbf"
>> +       info->firmware_name = devm_kstrdup(dev, firmware_name, GFP_KERNEL);
>>
>> -/* struct with information about the FPGA image to program. */
>> -struct fpga_image_info info;
>> +} else {
>>
>> -/* flags indicates whether to do full or partial reconfiguration */
>> -info.flags = 0;
>> +       not implemented!
>>
>> -int ret;
>> +}
>>
>>  /* Get exclusive control of FPGA manager */
>> -struct fpga_manager *mgr = of_fpga_mgr_get(mgr_node);
>> +ret = fpga_mgr_lock(mgr);
>>
>> -/* Get the firmware image (path) and load it to the FPGA */
>> -ret = fpga_mgr_firmware_load(mgr, &info, path);
>> +/* Load the buffer to the FPGA */
>> +ret = fpga_mgr_buf_load(mgr, &info, buf, count);
>>
>>  /* Release the FPGA manager */
>> +fpga_mgr_unlock(mgr);
>>  fpga_mgr_put(mgr);
>>
>> +/* Free your image storage in some appropriate way */
>> +...
>> +
>> +/* Deallocate the image info if you're done with it */
>> +fpga_image_info_free(dev, info);
>>
>>  How to support a new FPGA device
>>  ================================
>> diff --git a/Documentation/fpga/fpga-region.txt b/Documentation/fpga/fpga-region.txt
>> new file mode 100644
>> index 0000000..139a02b
>> --- /dev/null
>> +++ b/Documentation/fpga/fpga-region.txt
>> @@ -0,0 +1,95 @@
>> +FPGA Regions
>> +
>> +Alan Tull 2017
>> +
>> +CONTENTS
>> + - Introduction
>> + - The FPGA region API
>> + - Usage example
>> +
>> +Introduction
>> +============
>> +
>> +This document is meant to be an brief overview of the FPGA region API usage.  A
>> +more conceptual look at regions can be found in [1].
>> +
>> +For the purposes of this API document, let's just say that a region associates
>> +an FPGA Manager and a bridge (or bridges) with a reprogrammable region of an
>> +FPGA or the whole FPGA.  The API provides a way to register a region and to
>> +program a region.
>> +
>> +Currently the only layer above fpga-region.c in the kernel is the Device Tree
>> +support (of-fpga-region.c) described in [1].  The DT support layer uses regions
>> +to program the FPGA and then DT to handle enumeration.  The common region code
>> +is intended to be used by other schemes that have other ways of accomplishing
>> +enumeration after programming.
>> +
>> +An fpga-region can be set up to know the following things:
>> +* which FPGA manager to use to do the programming
>> +* which bridges to disable before programming and enable afterwards.
>> +
>> +Additional info needed to program the FPGA image is passed in the struct
>> +fpga_image_info [2] including:
>> +* pointers to the image as either a scatter-gather buffer, a contiguous
>> +  buffer, or the name of firmware file
>> +* flags indicating specifics such as whether the image if for partial
>> +  reconfiguration.
>> +
>> +===================
>> +The FPGA region API
>> +===================
>> +
>> +To register or unregister a region:
>> +-----------------------------------
>> +
>> +       int fpga_region_register(struct device *dev,
>> +                                struct fpga_region *region);
>> +       int fpga_region_unregister(struct fpga_region *region);
>> +
>> +An example of usage can be seen in the probe function of [3]
>> +
>> +To program an FPGA:
>> +-------------------
>> +       int fpga_region_program_fpga(struct fpga_region *region);
>> +
>> +This function operates on info passed in the fpga_image_info
>> +(region->info).
>> +
>> +This function will attempt to:
>> + * lock the region's mutex
>> + * lock the region's FPGA manager
>> + * build a list of FPGA bridges if a method has been specified to do so
>> + * disable the bridges
>> + * program the FPGA
>> + * re-enable the bridges
>> + * release the locks
>> +
>> +=============
>> +Usage example
>> +=============
>> +
>> +First, allocate the info struct:
>> +
>> +       info = fpga_image_info_alloc(dev);
>> +       if (!info)
>> +               return -ENOMEM;
>> +
>> +Set flags as needed, i.e.
>> +
>> +       info->flags |= FPGA_MGR_PARTIAL_RECONFIG;
>> +
>> +Point to your FPGA image, such as:
>> +
>> +       info->sgt = &sgt;
>> +
>> +Add info to region and do the programming:
>> +
>> +       region->info = info;
>> +       ret = fpga_region_program_fpga(region);
>> +
>> +Then enumerate whatever hardware has appeared in the FPGA.
>> +
>> +--
>> +[1] ../devicetree/bindings/fpga/fpga-region.txt
>> +[2] ./fpga-mgr.txt
>> +[3] ../../drivers/fpga/of-fpga-region.c
>> diff --git a/Documentation/fpga/overview.txt b/Documentation/fpga/overview.txt
>> new file mode 100644
>> index 0000000..149ac8a
>> --- /dev/null
>> +++ b/Documentation/fpga/overview.txt
>> @@ -0,0 +1,23 @@
>> +Linux kernel FPGA support
>> +
>> +Alan Tull 2017
>> +
>> +The main point of this project has been to separate the out the upper layers
>> +that know when to reprogram a FPGA from the lower layers that know how to
>> +reprogram a specific FPGA device.  The intention is to make this manufacturor
>> +agnostic, understanding that of course the FPGA images are very device specific
>> +themselves.
>> +
>> +At this point it time, the framework in the kernel includes:
>> +* low level FPGA manager drivers that know how to program a specific device
>> +* the fpga-mgr framework they are registered with
>> +* low level FPGA bridge drivers for hard/soft bridges which are intended to
>> +  be disable during FPGA programming
>> +* the fpga-bridge framework they are registered with
>> +* the fpga-region framework which associates and controls managers and bridges
>> +  as reconfigurable regions
>> +* the of-fpga-region support for reprogramming FPGAs when device tree overlays
>> +  are applied.
>> +
>> +I would encourage you the user to add code that creates fpga regions rather
>> +that trying to control managers and bridges separately.
>> --
>> 2.7.4
>>
>
> Cheers,
>
> Moritz

Powered by blists - more mailing lists