| Message ID | 20161017160941.4205-3-atull@opensource.altera.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Alan, couple of nits inline and some comments on ordering the patches ;-) On Mon, Oct 17, 2016 at 6:09 PM, Alan Tull <atull@opensource.altera.com> wrote: > This patch adds a minor change in the FPGA Mangager API s/Mangager/Manager/ > to hold information that is specific to an FPGA image > file. This change is expected to bring little, if any, > pain. > > An FPGA image file will have particulars that affect how the > image is programmed to the FPGA. One example is that > current 'flags' currently has one bit which shows whether the > FPGA image was built for full reconfiguration or partial > reconfiguration. Another example is timeout values for > enabling or disabling the bridges in the FPGA. As the > complexity of the FPGA design increases, the bridges in the > FPGA may take longer times to enable or disable. According for the current ordering bridges are not yet defined if we merge patches in this order? Not terrible imho, but I thought I'd point it out. Would swapping the order make sense? I also think [5/10] should be squashed together with this commit to make it an atomic change. Apart from my comments above feel free to add my Acked-by Thanks for keeping this going, Moritz
On Mon, 17 Oct 2016, Moritz Fischer wrote: > Hi Alan, > > couple of nits inline and some comments on ordering the patches ;-) > > On Mon, Oct 17, 2016 at 6:09 PM, Alan Tull <atull@opensource.altera.com> wrote: > > This patch adds a minor change in the FPGA Mangager API > > s/Mangager/Manager/ Yup! > > > to hold information that is specific to an FPGA image > > file. This change is expected to bring little, if any, > > pain. > > > > An FPGA image file will have particulars that affect how the > > image is programmed to the FPGA. One example is that > > current 'flags' currently has one bit which shows whether the > > FPGA image was built for full reconfiguration or partial > > reconfiguration. Another example is timeout values for > > enabling or disabling the bridges in the FPGA. As the > > complexity of the FPGA design increases, the bridges in the > > FPGA may take longer times to enable or disable. > > According for the current ordering bridges are not yet defined if we > merge patches in this order? > Not terrible imho, but I thought I'd point it out. Would swapping the > order make sense? Probably, yes. > > I also think [5/10] should be squashed together with this commit to > make it an atomic change. So far my bindings and code have gone in separately. Bindings through Rob and code through Greg KH or Dinh. > > Apart from my comments above feel free to add my Acked-by > > Thanks for keeping this going, > > Moritz > Thanks for all the code reviews! Alan
diff --git a/Documentation/fpga/fpga-mgr.txt b/Documentation/fpga/fpga-mgr.txt index ce3e84f..9227e3f 100644 --- a/Documentation/fpga/fpga-mgr.txt +++ b/Documentation/fpga/fpga-mgr.txt @@ -18,21 +18,25 @@ API Functions: To program the FPGA from a file or from a buffer: ------------------------------------------------- - int fpga_mgr_buf_load(struct fpga_manager *mgr, u32 flags, + 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 buffer in memory. - int fpga_mgr_firmware_load(struct fpga_manager *mgr, u32 flags, + 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). - -For both these functions, flags == 0 for normal full reconfiguration or -FPGA_MGR_PARTIAL_RECONFIG for partial reconfiguration. If successful, the FPGA -ends up in operating mode. Return 0 on success or a negative error code. +the firmware search path (see the firmware class documentation). 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 get/put a reference to a FPGA manager: ----------------------------------------- @@ -70,8 +74,11 @@ struct device_node *mgr_node = ... char *buf = ... int count = ... +/* struct with information about the FPGA image to program. */ +struct fpga_image_info info; + /* flags indicates whether to do full or partial reconfiguration */ -int flags = 0; +info.flags = 0; int ret; @@ -79,7 +86,7 @@ int ret; struct fpga_manager *mgr = of_fpga_mgr_get(mgr_node); /* Load the buffer to the FPGA */ -ret = fpga_mgr_buf_load(mgr, flags, buf, count); +ret = fpga_mgr_buf_load(mgr, &info, buf, count); /* Release the FPGA manager */ fpga_mgr_put(mgr); @@ -96,8 +103,11 @@ struct device_node *mgr_node = ... /* FPGA image is in this file which is in the firmware search path */ const char *path = "fpga-image-9.rbf" +/* struct with information about the FPGA image to program. */ +struct fpga_image_info info; + /* flags indicates whether to do full or partial reconfiguration */ -int flags = 0; +info.flags = 0; int ret; @@ -105,7 +115,7 @@ int ret; struct fpga_manager *mgr = of_fpga_mgr_get(mgr_node); /* Get the firmware image (path) and load it to the FPGA */ -ret = fpga_mgr_firmware_load(mgr, flags, path); +ret = fpga_mgr_firmware_load(mgr, &info, path); /* Release the FPGA manager */ fpga_mgr_put(mgr);
This patch adds a minor change in the FPGA Mangager API to hold information that is specific to an FPGA image file. This change is expected to bring little, if any, pain. An FPGA image file will have particulars that affect how the image is programmed to the FPGA. One example is that current 'flags' currently has one bit which shows whether the FPGA image was built for full reconfiguration or partial reconfiguration. Another example is timeout values for enabling or disabling the bridges in the FPGA. As the complexity of the FPGA design increases, the bridges in the FPGA may take longer times to enable or disable. This patch documents the change in the FPGA Manager API functions, replacing the 'u32 flag' parameter with a pointer to struct fpga_image_info. Signed-off-by: Alan Tull <atull@opensource.altera.com> --- v19: Added in v19 of this patchset v20: No change for this patch in v20 of patchset --- Documentation/fpga/fpga-mgr.txt | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-)