Re: [PATCH v4 1/1] staging: fpgaboot: Xilinx FPGA firmware download driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 16, 2014 at 12:56:15AM -0800, Insop Song wrote:
> On Tue, Jan 14, 2014 at 10:00 AM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > On Tue, Jan 14, 2014 at 01:37:50AM -0800, Insop Song wrote:
> >> On Tue, Jan 14, 2014 at 1:18 AM, Insop Song <insop.song@xxxxxxxxx> wrote:
> >> > On Mon, Jan 13, 2014 at 10:44 AM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> >> >> On Sat, Jan 11, 2014 at 04:37:32PM -0800, Insop Song wrote:
> >> >>> On Sat, Jan 11, 2014 at 4:10 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> >> >>> > On Sat, Jan 11, 2014 at 03:56:48PM -0800, Insop Song wrote:
> >> >>> >> On Sat, Jan 11, 2014 at 1:05 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> >> >>> >> > On Sat, Jan 11, 2014 at 12:51:28PM -0800, Greg KH wrote:
> >> >>> >> >> On Thu, Jan 09, 2014 at 11:48:04AM -0800, Insop Song wrote:
> >> >>> >> >> > This driver downloads Xilinx FPGA firmware using gpio pins.
> >> >>> >> >> > It loads Xilinx FPGA bitstream format firmware image and
> >> >>> >> >> > program the Xilinx FPGA using SelectMAP (parallel) mode.
> >> >>> >> >> >
> >> >>> >> >> > Signed-off-by: Insop Song <insop.song@xxxxxxxxxxxxx>
> >> >>> >> >>
> >> >>> >> >> This patch breaks the build on my machine, please be more careful:
> >> >>> >> >>
> >> >>> >> >> drivers/staging/gs_fpgaboot/gs_fpgaboot.c:30:28: fatal error: include/asm/io.h: No such file or directory
> >> >>> >> >>  #include <include/asm/io.h>
> >> >>> >> >>
> >> >>> >> >> Please fix this up, test it, and resend.
> >> >>> >> >
> >> >>> >> > Also, while you are fixing things up, please just remove the character
> >> >>> >> > device node from the driver, you aren't doing anything with it, so it's
> >> >>> >> > not needed.
> >> >>> >> >
> >> >>> >>
> >> >>> >> Thank you for the suggestion, makes a lot of sense and code cleaner.
> >> >>> >>
> >> >>> >> Could you take a quick look?
> >> >>> >> If it is good, will send out a new patch, v5, shortly.
> >> >>> >>
> >> >>> >> Main change is
> >> >>> >>
> >> >>> >> +/* fake device for request_firmware */
> >> >>> >> +static struct platform_device        *firmware_pdev;
> >> >>> >>
> >> >>> >> + firmware_pdev = platform_device_register_simple("fpgaboot", -1,
> >> >>> >> +                                                  NULL, 0);
> >> >>> >
> >> >>> > Why do you need a platform device?  Don't you have something on the
> >> >>> > hardware that describes the fact that this device is in the system, or
> >> >>> > not?  Like a PCI id?  DMI id?  Device Tree entry?
> >> >>> >
> >> >>> > You must have something that you can trigger off of.
> >> >>> >
> >> >>>
> >> >>> This driver is only to download fpga firmware, not intended to keep
> >> >>> managing the fpga device after the downloading. The device we use is
> >> >>> not part of the device tree or PCI.
> >> >>
> >> >> But how does the system know to load it or not, automatically?
> >> >>
> >> >>> It downloads the image and after that no need to be existing.
> >> >>
> >> >> Then why not have the module remove itself?
> >> >>
> >> >
> >> > The following (insmod to load firmware and rmmod remove the module)
> >> > commands are triggered from user space application. They are wrapped
> >> > as a script and triggered from an application.
> >> > Once the firmware is loaded, then this driver's job is done.
> >
> > Yes, but that's not the "normal" way kernel drivers interact with
> > userspace.  Please don't try to create something that is different than
> > the thousands of other drivers out there, and rely on custom userspace
> > scripts.  Let's use the infrastructure we already have in place for
> > doing this exact thing.
> >
> >> >>> Here is how we use
> >> >>>
> >> >>> $ insmod gs_fpga.ko file="xlinx_fpga_top_bitstream.bit"
> >> >>> $ rmmod gs_fpga
> >> >>
> >> >> Linux modules should be able to be autoloaded based on hardware present
> >> >> in the system, it's been that way for over a decade now.
> >> >>
> >> >
> >> >
> >> Greg, I think it's better to add more explanation for FPGA.
> >
> > I know what a FPGA is, thanks :)
> >
> >> FPGA (Field-programmable gate array) is not functional until the
> >> firmware is downloaded.
> >> That means that it is not easy, if not possible, to auto detect the
> >> device. This is why "insmod" is manually called.
> >
> > That's not true, there has to be some way to detect the device is in the
> > system, otherwise what's to prevent you from loading the module on a
> > system that doesn't have the hardware and having bad things happen?
> >
> > There has to be some way to detect this device is present, right?  What
> > is it?  Where is it located in the device tree description?
> >
> 
> 
> There is no way to detect FPGA until it is programmed.
> This is a reason and the only reason of this driver to download the
> program to the FPGA so that it can function.

So how do you get the memory locations of where the FPGA is in the
system in order to be able to send data to it?  Surely that's in the
device tree file somewhere, right?

thanks,

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




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux