Re: [PATCH v9 1/7] staging: usage documentation for FPGA manager core

[Pavel Machek <pavel@denx.de>]

On Fri 2015-07-17 10:51:11, atull@opensource.altera.com wrote:
> From: Alan Tull <atull@opensource.altera.com>
> 
> Add a document on the new FPGA manager core.
> 

> --- /dev/null
> +++ b/drivers/staging/fpga/Documentation/fpga-mgr.txt
> @@ -0,0 +1,117 @@
> +  FPGA Manager Core
> +
> +  Alan Tull 2015
> +
> +  Overview
> +  --------

The formatting is quite funny here. Add a newline after ---'s? Or
better format it the way other documents are formatted?

> +The FPGA manager core exports a set of functions for programming an image to a

"into"?

> +FPGA.  All manufacturor specifics are hidden away in a low level driver.  The

manufacturer (more then one instance).

> +API is manufacturor agnostic.  Of course the FPGA image data itself is very
> +manufacturor specific but for our purposes it's just data in a buffer or file

, but

> +or something.  The FPGA manager core won't parse it or know anything about it.

kill "or know anything"?

> +  Files
> +  -----
> +drivers/staging/fpga/fpga-mgr.c
> +include/linux/fpga/fpga-mgr.h
> +

Kill this section, as it is going to change?

> +  The API Functions
> +  ----------------
> +The API that is exported is currently 6 functions:
> +
> +   int fpga_mgr_buf_load(struct fpga_manager *mgr,
> +                         u32 flags,
> +                         const char *buf,
> +                         size_t count);
> +
> +An FPGA image exists as a buffer in memory.  Load it into the FPGA.  The FPGA
> +ends up in operating mode or return a negative error code.

So 0 on success?

> +   int fpga_mgr_firmware_load(struct fpga_manager *mgr,
> +                              u32 flags,
> +                              const char *image_name);
> +
> +An FPGA image exists as a file that is on the firmware search path (see the

", that is in"

> +firmware class documentation).  Load as above.
> +
> +   struct fpga_manager *of_fpga_mgr_get(struct device_node *node);
> +
> +Given a DT node, get a reference to a fpga manager.

fpga->FPGA, fix globally??

> +   void fpga_mgr_put(struct fpga_manager *mgr);
> +
> +Release the reference to the fpga manager.
> +
> +   int fpga_mgr_register(struct device *dev,
> +                         const char *name,
> +                         const struct fpga_manager_ops *mops,
> +                         void *priv);
> +   void fpga_mgr_unregister(struct device *dev);
> +
> +Register/unregister the lower level device specific driver.

"low level".. And "device specific" -> "FPGA-specific" ?


> +To add another fpga manager, look at the bottom part of socfpga.c for an
> +example, starting with the declaration of socfpga_fpga_ops.

Umm. You have good documentation below. Maybe you don't need to send
people to read sources...?

> +static const struct fpga_manager_ops socfpga_fpga_ops = {
> +       .write_init = socfpga_fpga_ops_configure_init,
> +       .write = socfpga_fpga_ops_configure_write,
> +       .write_complete = socfpga_fpga_ops_configure_complete,
> +       .state = socfpga_fpga_ops_state,
> +};
> +
> +You will want to create a platform driver that has a set of ops like that
> +and then register it with fpga_mgr_register in your probe function.  Your
> +ops will implement whatever device specific register writes needed and
> +will return negative error codes if things don't go well.
> +
> +The programming seqence is:

sequence.

> + 1. .write_init
> + 2. .write (may be called once or multiple times)
> + 3. .write_complete
> +
> +The .write_init function will prepare the FPGA to receive the image data.
> +
> +The .write function receives an image buffer or a chunk of the image and
> +writes it the FPGA.  The buffer may arrive as one chunk or a bunck
> of

bunch.

> +small chunks through this function being called multiple times.
> +
> +The .write_complete function is called after all the image has been written
> +to put the FPGA into operating mode.
> +
> +The .state function will read your hardware and return a code of type
> +"enum fpga_mgr_states".  It doesn't result in a change in hardware state.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html