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 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.

>> 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.
>

Understood, however the use case for this driver is to load firmware, only that.
The other way to do could have been use a user space program read file
and access gpio (or other means) which could end up taking lot longer
firmware programming time.

>> I still needed "device" to use request_firmware() even though I don't
>> use it. I've looked at other drivers which uses request_firmware()
>> after your suggestion of removing char device node, and I've found
>> platform_device can be a simple one to use.
>
> Please base your device on a real one in the system, like your FPGA
> device, surely there is some way it can be detected to be present or
> not?
>

Do you still think a fake platform device doesn't cut? then what do you suggest?

Thank you,

ISS
_______________________________________________
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