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.
If you look at this not from xilinx, which I refer,
http://www.xilinx.com/support/documentation/application_notes/xapp583-fpga-configuration.pdf,
You cannot get any device id or data from the fpga until it is programmed.
>> FPGA firmware download happen ever time system boots, so this is
>> different from occasional "upgrade".
>
> Just like all other modules that use the firmware interface for their
> hardware :)
>
Greg, I am not arguing about the way it is now.
I just want to raise a point that FPGA is different until it is
programmed, my driver is exist only for the programming. :)
>> To sum up, FPGA download occurs every boot, and before this download
>> is done, the device in question is not functional.
>
> It's functional enough to get the data to it, so it must be operating
> somehow...
>
Again, no. Unless you program your gpio behave like JTAG, so no.
>> > 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?
>
> Use a "real" platform device that is created based on the device tree
> information that describes the FPGA device in the system. You have to
> have some way to describe the memory location to be able to talk to the
> FPGA, right? Just use that.
>
I'd have to wait your response before I answer this point.
Thank you,
Insop
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
