Messages in this thread | | | Date | Mon, 9 Jan 2017 10:04:36 -0600 (CST) | From | Alan Tull <> | Subject | Re: [PATCH v3 fpga 3/4] fpga: Add scatterlist based programming |
| |
On Fri, 6 Jan 2017, Jason Gunthorpe wrote:
> Requiring contiguous kernel memory is not a good idea, this is a limited > resource and allocation can fail under normal work loads. > > This introduces a .write_sg op that supporting drivers can provide > to DMA directly from dis-contiguous memory and a new entry point > fpga_mgr_buf_load_sg that users can call to directly provide page > lists. > > The full matrix of compatibility is provided, either the linear or sg > interface can be used by the user with a driver supporting either > interface. > > A notable change for drivers is that the .write op can now be called > multiple times. > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > --- > Documentation/fpga/fpga-mgr.txt | 19 +++- > drivers/fpga/fpga-mgr.c | 238 ++++++++++++++++++++++++++++++++++------ > include/linux/fpga/fpga-mgr.h | 5 + > 3 files changed, 228 insertions(+), 34 deletions(-) > > diff --git a/Documentation/fpga/fpga-mgr.txt b/Documentation/fpga/fpga-mgr.txt > index 86ee5078fd034f..78f197fadfd1b6 100644 > --- a/Documentation/fpga/fpga-mgr.txt > +++ b/Documentation/fpga/fpga-mgr.txt > @@ -22,7 +22,16 @@ To program the FPGA from a file or from a buffer: > struct fpga_image_info *info, > const char *buf, size_t count); > > -Load the FPGA from an image which exists as a buffer in memory. > +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); > + > +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_firmware_load(struct fpga_manager *mgr, > struct fpga_image_info *info, > @@ -166,7 +175,7 @@ success or negative error codes otherwise. > > The programming sequence is: > 1. .write_init > - 2. .write (may be called once or multiple times) > + 2. .write or .write_sg (may be called once or multiple times) > 3. .write_complete > > The .write_init function will prepare the FPGA to receive the image data. The > @@ -176,7 +185,11 @@ buffer up at least this much before starting. > > The .write function writes a buffer to the FPGA. The buffer may be contain the > whole FPGA image or may be a smaller chunk of an FPGA image. In the latter > -case, this function is called multiple times for successive chunks. > +case, this function is called multiple times for successive chunks. This interface > +is suitable for drivers which use PIO. > + > +The .write_sg version behaves the same as .write except the input is a sg_table > +scatter list. This interface is suitable for drivers which use DMA. > > The .write_complete function is called after all the image has been written > to put the FPGA into operating mode. > diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c > index f0a69d3e60a584..30f9778d0632d2 100644 > --- a/drivers/fpga/fpga-mgr.c > +++ b/drivers/fpga/fpga-mgr.c > @@ -1,4 +1,4 @@ > -/* > + /*
Hi Jason,
Need to take these added 2 spaces out. Otherwise,
Acked-by: Alan Tull <atull@opensource.altera.com>
Thank you for adding this functionality.
Alan
> * FPGA Manager Core > * > * Copyright (C) 2013-2015 Altera Corporation > @@ -25,16 +25,106 @@ > #include <linux/of.h> > #include <linux/mutex.h> > #include <linux/slab.h> > +#include <linux/scatterlist.h> > +#include <linux/highmem.h> > > static DEFINE_IDA(fpga_mgr_ida); > static struct class *fpga_mgr_class; > > +/* > + * Call the low level driver's write_init function. This will do the > + * device-specific things to get the FPGA into the state where it is ready to > + * receive an FPGA image. The low level driver only gets to see the first > + * initial_header_size bytes in the buffer. > + */ > +static int fpga_mgr_write_init_buf(struct fpga_manager *mgr, > + struct fpga_image_info *info, > + const char *buf, size_t count) > +{ > + int ret; > + > + mgr->state = FPGA_MGR_STATE_WRITE_INIT; > + if (!mgr->mops->initial_header_size) > + ret = mgr->mops->write_init(mgr, info, NULL, 0); > + else > + ret = mgr->mops->write_init( > + mgr, info, buf, min(mgr->mops->initial_header_size, count)); > + > + if (ret) { > + dev_err(&mgr->dev, "Error preparing FPGA for writing\n"); > + mgr->state = FPGA_MGR_STATE_WRITE_INIT_ERR; > + return ret; > + } > + > + return 0; > +} > + > +static int fpga_mgr_write_init_sg(struct fpga_manager *mgr, > + struct fpga_image_info *info, > + struct sg_table *sgt) > +{ > + struct sg_mapping_iter miter; > + size_t len; > + char *buf; > + int ret; > + > + if (!mgr->mops->initial_header_size) > + return fpga_mgr_write_init_buf(mgr, info, NULL, 0); > + > + /* > + * First try to use miter to map the first fragment to access the > + * header, this is the typical path. > + */ > + sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG); > + if (sg_miter_next(&miter) && > + miter.length >= mgr->mops->initial_header_size) { > + ret = fpga_mgr_write_init_buf(mgr, info, miter.addr, > + miter.length); > + sg_miter_stop(&miter); > + return ret; > + } > + sg_miter_stop(&miter); > + > + /* Otherwise copy the fragments into temporary memory. */ > + buf = kmalloc(mgr->mops->initial_header_size, GFP_KERNEL); > + if (!buf) > + return -ENOMEM; > + > + len = sg_copy_to_buffer(sgt->sgl, sgt->nents, buf, > + mgr->mops->initial_header_size); > + ret = fpga_mgr_write_init_buf(mgr, info, buf, len); > + > + kfree(buf); > + > + return ret; > +} > + > +/* > + * After all the FPGA image has been written, do the device specific steps to > + * finish and set the FPGA into operating mode. > + */ > +static int fpga_mgr_write_complete(struct fpga_manager *mgr, > + struct fpga_image_info *info) > +{ > + int ret; > + > + mgr->state = FPGA_MGR_STATE_WRITE_COMPLETE; > + ret = mgr->mops->write_complete(mgr, info); > + if (ret) { > + dev_err(&mgr->dev, "Error after writing image data to FPGA\n"); > + mgr->state = FPGA_MGR_STATE_WRITE_COMPLETE_ERR; > + return ret; > + } > + mgr->state = FPGA_MGR_STATE_OPERATING; > + > + return 0; > +} > + > /** > - * fpga_mgr_buf_load - load fpga from image in buffer > + * fpga_mgr_buf_load_sg - load fpga from image in buffer from a scatter list > * @mgr: fpga manager > * @info: fpga image specific information > - * @buf: buffer contain fpga image > - * @count: byte count of buf > + * @sgt: scatterlist table > * > * Step the low level fpga manager through the device-specific steps of getting > * an FPGA ready to be configured, writing the image to it, then doing whatever > @@ -42,54 +132,139 @@ static struct class *fpga_mgr_class; > * mgr pointer from of_fpga_mgr_get() or fpga_mgr_get() and checked that it is > * not an error code. > * > + * This is the preferred entry point for FPGA programming, it does not require > + * any contiguous kernel memory. > + * > * Return: 0 on success, negative error code otherwise. > */ > -int fpga_mgr_buf_load(struct fpga_manager *mgr, struct fpga_image_info *info, > - const char *buf, size_t count) > +int fpga_mgr_buf_load_sg(struct fpga_manager *mgr, struct fpga_image_info *info, > + struct sg_table *sgt) > { > - struct device *dev = &mgr->dev; > int ret; > > - /* > - * Call the low level driver's write_init function. This will do the > - * device-specific things to get the FPGA into the state where it is > - * ready to receive an FPGA image. The low level driver only gets to > - * see the first initial_header_size bytes in the buffer. > - */ > - mgr->state = FPGA_MGR_STATE_WRITE_INIT; > - ret = mgr->mops->write_init(mgr, info, buf, > - min(mgr->mops->initial_header_size, count)); > + ret = fpga_mgr_write_init_sg(mgr, info, sgt); > + if (ret) > + return ret; > + > + /* Write the FPGA image to the FPGA. */ > + mgr->state = FPGA_MGR_STATE_WRITE; > + if (mgr->mops->write_sg) { > + ret = mgr->mops->write_sg(mgr, sgt); > + } else { > + struct sg_mapping_iter miter; > + > + sg_miter_start(&miter, sgt->sgl, sgt->nents, SG_MITER_FROM_SG); > + while (sg_miter_next(&miter)) { > + ret = mgr->mops->write(mgr, miter.addr, miter.length); > + if (ret) > + break; > + } > + sg_miter_stop(&miter); > + } > + > if (ret) { > - dev_err(dev, "Error preparing FPGA for writing\n"); > - mgr->state = FPGA_MGR_STATE_WRITE_INIT_ERR; > + dev_err(&mgr->dev, "Error while writing image data to FPGA\n"); > + mgr->state = FPGA_MGR_STATE_WRITE_ERR; > return ret; > } > > + return fpga_mgr_write_complete(mgr, info); > +} > +EXPORT_SYMBOL_GPL(fpga_mgr_buf_load_sg); > + > +static int fpga_mgr_buf_load_mapped(struct fpga_manager *mgr, > + struct fpga_image_info *info, > + const char *buf, size_t count) > +{ > + int ret; > + > + ret = fpga_mgr_write_init_buf(mgr, info, buf, count); > + if (ret) > + return ret; > + > /* > * Write the FPGA image to the FPGA. > */ > mgr->state = FPGA_MGR_STATE_WRITE; > ret = mgr->mops->write(mgr, buf, count); > if (ret) { > - dev_err(dev, "Error while writing image data to FPGA\n"); > + dev_err(&mgr->dev, "Error while writing image data to FPGA\n"); > mgr->state = FPGA_MGR_STATE_WRITE_ERR; > return ret; > } > > + return fpga_mgr_write_complete(mgr, info); > +} > + > +/** > + * fpga_mgr_buf_load - load fpga from image in buffer > + * @mgr: fpga manager > + * @flags: flags setting fpga confuration modes > + * @buf: buffer contain fpga image > + * @count: byte count of buf > + * > + * Step the low level fpga manager through the device-specific steps of getting > + * an FPGA ready to be configured, writing the image to it, then doing whatever > + * post-configuration steps necessary. This code assumes the caller got the > + * mgr pointer from of_fpga_mgr_get() and checked that it is not an error code. > + * > + * Return: 0 on success, negative error code otherwise. > + */ > +int fpga_mgr_buf_load(struct fpga_manager *mgr, struct fpga_image_info *info, > + const char *buf, size_t count) > +{ > + struct page **pages; > + struct sg_table sgt; > + const void *p; > + int nr_pages; > + int index; > + int rc; > + > /* > - * After all the FPGA image has been written, do the device specific > - * steps to finish and set the FPGA into operating mode. > + * This is just a fast path if the caller has already created a > + * contiguous kernel buffer and the driver doesn't require SG, non-SG > + * drivers will still work on the slow path. > */ > - mgr->state = FPGA_MGR_STATE_WRITE_COMPLETE; > - ret = mgr->mops->write_complete(mgr, info); > - if (ret) { > - dev_err(dev, "Error after writing image data to FPGA\n"); > - mgr->state = FPGA_MGR_STATE_WRITE_COMPLETE_ERR; > - return ret; > + if (mgr->mops->write) > + return fpga_mgr_buf_load_mapped(mgr, info, buf, count); > + > + /* > + * Convert the linear kernel pointer into a sg_table of pages for use > + * by the driver. > + */ > + nr_pages = DIV_ROUND_UP((unsigned long)buf + count, PAGE_SIZE) - > + (unsigned long)buf / PAGE_SIZE; > + pages = kmalloc_array(nr_pages, sizeof(struct page *), GFP_KERNEL); > + if (!pages) > + return -ENOMEM; > + > + p = buf - offset_in_page(buf); > + for (index = 0; index < nr_pages; index++) { > + if (is_vmalloc_addr(p)) > + pages[index] = vmalloc_to_page(p); > + else > + pages[index] = kmap_to_page((void *)p); > + if (!pages[index]) { > + kfree(pages); > + return -EFAULT; > + } > + p += PAGE_SIZE; > } > - mgr->state = FPGA_MGR_STATE_OPERATING; > > - return 0; > + /* > + * The temporary pages list is used to code share the merging algorithm > + * in sg_alloc_table_from_pages > + */ > + rc = sg_alloc_table_from_pages(&sgt, pages, index, offset_in_page(buf), > + count, GFP_KERNEL); > + kfree(pages); > + if (rc) > + return rc; > + > + rc = fpga_mgr_buf_load_sg(mgr, info, &sgt); > + sg_free_table(&sgt); > + > + return rc; > } > EXPORT_SYMBOL_GPL(fpga_mgr_buf_load); > > @@ -291,8 +466,9 @@ int fpga_mgr_register(struct device *dev, const char *name, > struct fpga_manager *mgr; > int id, ret; > > - if (!mops || !mops->write_init || !mops->write || > - !mops->write_complete || !mops->state) { > + if (!mops || !mops->write_complete || !mops->state || > + !mops->write_init || (!mops->write && !mops->write_sg) || > + (mops->write && mops->write_sg)) { > dev_err(dev, "Attempt to register without fpga_manager_ops\n"); > return -EINVAL; > } > diff --git a/include/linux/fpga/fpga-mgr.h b/include/linux/fpga/fpga-mgr.h > index 16551d5eac36a7..57beb5d09bfcb2 100644 > --- a/include/linux/fpga/fpga-mgr.h > +++ b/include/linux/fpga/fpga-mgr.h > @@ -22,6 +22,7 @@ > #define _LINUX_FPGA_MGR_H > > struct fpga_manager; > +struct sg_table; > > /** > * enum fpga_mgr_states - fpga framework states > @@ -88,6 +89,7 @@ struct fpga_image_info { > * @state: returns an enum value of the FPGA's state > * @write_init: prepare the FPGA to receive confuration data > * @write: write count bytes of configuration data to the FPGA > + * @write_sg: write the scatter list of configuration data to the FPGA > * @write_complete: set FPGA to operating state after writing is done > * @fpga_remove: optional: Set FPGA into a specific state during driver remove > * > @@ -102,6 +104,7 @@ struct fpga_manager_ops { > struct fpga_image_info *info, > const char *buf, size_t count); > int (*write)(struct fpga_manager *mgr, const char *buf, size_t count); > + int (*write_sg)(struct fpga_manager *mgr, struct sg_table *sgt); > int (*write_complete)(struct fpga_manager *mgr, > struct fpga_image_info *info); > void (*fpga_remove)(struct fpga_manager *mgr); > @@ -129,6 +132,8 @@ struct fpga_manager { > > int fpga_mgr_buf_load(struct fpga_manager *mgr, struct fpga_image_info *info, > const char *buf, size_t count); > +int fpga_mgr_buf_load_sg(struct fpga_manager *mgr, struct fpga_image_info *info, > + struct sg_table *sgt); > > int fpga_mgr_firmware_load(struct fpga_manager *mgr, > struct fpga_image_info *info, > -- > 2.7.4 > >
| |