On Sat, Jul 07, 2018 at 10:19:40PM -0400, Jacob Feder wrote:
> Hi all,
> I have developed this driver for a Xilinx-provided IP block for their Zynq
> SoC. I fixed all of the checkpatch.pl problems I knew how to. If someone
> could chime in on how to fix the remaining few it would be appreciated.
> 
> Also looking for feedback on general structure. It's my first driver (and
> patch submission) so I'm sure there are plenty of things to be improved on
> :).
> 
> Functionally everything works OK except the performance isn't as good as I
> was hoping for. I have been testing it by operating the FIFO in loopback
> mode (AXI-Stream TX interface feeding back into RX interface) running on
> the XC7Z020 (7000 series) Zynq device. I am getting anything between
> 3-16MB/s depending on the amount of data transferred. The performance
> claimed by the PG080 datasheet is ~65MB/s. The CPU isn't under significant
> load (~12%) as reported by top so I don't think that's the bottleneck.
> 
> Please +CC in responses as I'm not on the mailing list.
> 
> Cheers
> 
> 
> This IP core has read and write AXI-Stream FIFOs, the contents of which can
> be accessed from the AXI4 memory-mapped interface. This is useful for
> transferring data from a processor into the FPGA fabric. The driver creates
> a character device that can be read/written to with standard
> open/read/write/close.

Why not use the uio api, which allows userspace to mmap the memory of
the device and access it directly from userspace?  That should make
things a lot faster, right?

Or if that doesn't work, what about the fpga API the kernel now has?
Would that work for this hardware?


> 
> See Xilinx PG080 document for IP details.

Do you have a link to that?  If so, can you put it in here?

> 
> Currently supports only store-forward mode with a 32-bit
> AXI4-Lite interface. DOES NOT support:
>       - cut-through mode
>       - AXI4 (non-lite)
> 
> Signed-off-by: Jacob Feder <jacobsfe...@gmail.com>
> ---
>  drivers/staging/axisfifo/axis-fifo.c | 1296 
> ++++++++++++++++++++++++++++++++++
>  drivers/staging/axisfifo/axis-fifo.h |  119 ++++
>  2 files changed, 1415 insertions(+)
>  create mode 100644 drivers/staging/axisfifo/axis-fifo.c
>  create mode 100644 drivers/staging/axisfifo/axis-fifo.h

Why does a single .c file need a .h file?

I'll be glad to take this driver, as others can clean it up in-tree (I
think your locking is crazy and is probably causing a lot of performance
issues), but I need a TODO file for it listing what you think is needed
to do in order to get this out of the staging tree?

Or, if you thin it should use the fpga or uio interface instead, maybe
it's just easier to redo it based on that?

thanks,

greg k-h
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to