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

2014-01-21 Thread Insop Song 
On Mon, Jan 20, 2014 at 10:06 AM, Greg KH gre...@linuxfoundation.org wrote:
 On Mon, Jan 20, 2014 at 09:16:08AM -0800, Insop Song wrote:
 
  On the FPGA side, there are dedicated pins for programming, and
  through these you cannot get meaningful information (again unless you
  are JTAG capable)
  Such as these on the FPGA side, PROGRAM_B, INIT_B, CCLK, D[0:7], and DONE.
  On a process side, we use gpio pin to connect to the above pins.
  It's GPIO pins that we do the bit banging as defined for programming
  guide from Xilinx.
 
  Yes, but where do you learn about how those pins are hooked up to the
  CPU so that the driver can control them?
 
 This is hard coded.

 Really?  Shouldn't they be in a board file or device tree attribute
 somewhere?  What happens in the next board that is created with this
 chip and the memory locations are in a different place?


I think the same, as I put in TODO file is alluding that.

+TODO:

+ - get bus width input instead of hardcoded bus width

I said bus width, but this includes fpga programming method (bus
width and pin configuration, gpio or any other types) can be either
insmod parameter or device tree (as you suggested).
Give me some time to think about this, meanwhile I might hear from
other people as well soon as this driver is parked in upstream staging
area.

Thank you again,

ISS
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

2014-01-21 Thread Greg KH 
On Tue, Jan 21, 2014 at 02:08:53AM -0800, Insop Song wrote:
 On Mon, Jan 20, 2014 at 10:06 AM, Greg KH gre...@linuxfoundation.org wrote:
  On Mon, Jan 20, 2014 at 09:16:08AM -0800, Insop Song wrote:
  
   On the FPGA side, there are dedicated pins for programming, and
   through these you cannot get meaningful information (again unless you
   are JTAG capable)
   Such as these on the FPGA side, PROGRAM_B, INIT_B, CCLK, D[0:7], and 
   DONE.
   On a process side, we use gpio pin to connect to the above pins.
   It's GPIO pins that we do the bit banging as defined for programming
   guide from Xilinx.
  
   Yes, but where do you learn about how those pins are hooked up to the
   CPU so that the driver can control them?
  
  This is hard coded.
 
  Really?  Shouldn't they be in a board file or device tree attribute
  somewhere?  What happens in the next board that is created with this
  chip and the memory locations are in a different place?
 
 
 I think the same, as I put in TODO file is alluding that.
 
 +TODO:
 
 + - get bus width input instead of hardcoded bus width
 
 I said bus width, but this includes fpga programming method (bus
 width and pin configuration, gpio or any other types) can be either
 insmod parameter or device tree (as you suggested).
 Give me some time to think about this, meanwhile I might hear from
 other people as well soon as this driver is parked in upstream staging
 area.

Fair enough, I'll queue it up after 3.14-rc1 is out.

thanks,

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

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

2014-01-20 Thread Insop Song 
On Thu, Jan 16, 2014 at 1:41 PM, Greg KH gre...@linuxfoundation.org wrote:
 On Thu, Jan 16, 2014 at 11:47:41AM -0800, Insop Song wrote:
  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?
 

 On the FPGA side, there are dedicated pins for programming, and
 through these you cannot get meaningful information (again unless you
 are JTAG capable)
 Such as these on the FPGA side, PROGRAM_B, INIT_B, CCLK, D[0:7], and DONE.
 On a process side, we use gpio pin to connect to the above pins.
 It's GPIO pins that we do the bit banging as defined for programming
 guide from Xilinx.

 Yes, but where do you learn about how those pins are hooked up to the
 CPU so that the driver can control them?

This is hard coded.

 Anyway, I have no objection to your driver, it does good things, and we
 want it, I think we are talking past each other at the moment, sorry.

 How about you repost the code, just using a platform device for the
 moment, and we can take it from there.  After all, code in the staging
 tree is meant to be cleaned up :)


Thank you for your suggestions and feedback.
I am sure I will have to clean up more :)
Will clean up and send a d new patch shortly.

Thank you,

ISS
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

2014-01-20 Thread Greg KH 
On Mon, Jan 20, 2014 at 09:16:08AM -0800, Insop Song wrote:
 On Thu, Jan 16, 2014 at 1:41 PM, Greg KH gre...@linuxfoundation.org wrote:
  On Thu, Jan 16, 2014 at 11:47:41AM -0800, Insop Song wrote:
   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?
  
 
  On the FPGA side, there are dedicated pins for programming, and
  through these you cannot get meaningful information (again unless you
  are JTAG capable)
  Such as these on the FPGA side, PROGRAM_B, INIT_B, CCLK, D[0:7], and DONE.
  On a process side, we use gpio pin to connect to the above pins.
  It's GPIO pins that we do the bit banging as defined for programming
  guide from Xilinx.
 
  Yes, but where do you learn about how those pins are hooked up to the
  CPU so that the driver can control them?
 
 This is hard coded.

Really?  Shouldn't they be in a board file or device tree attribute
somewhere?  What happens in the next board that is created with this
chip and the memory locations are in a different place?

thanks,

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

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

2014-01-20 Thread Hartley Sweeten 
On Monday, January 20, 2014 11:06 AM, Greg KH wrote:
 On Mon, Jan 20, 2014 at 09:16:08AM -0800, Insop Song wrote:
 On Thu, Jan 16, 2014 at 1:41 PM, Greg KH gre...@linuxfoundation.org wrote:
 On Thu, Jan 16, 2014 at 11:47:41AM -0800, Insop Song wrote:
 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?


 On the FPGA side, there are dedicated pins for programming, and
 through these you cannot get meaningful information (again unless you
 are JTAG capable)
 Such as these on the FPGA side, PROGRAM_B, INIT_B, CCLK, D[0:7], and DONE.
 On a process side, we use gpio pin to connect to the above pins.
 It's GPIO pins that we do the bit banging as defined for programming
 guide from Xilinx.

 Yes, but where do you learn about how those pins are hooked up to the
 CPU so that the driver can control them?

 This is hard coded.

 Really?  Shouldn't they be in a board file or device tree attribute
 somewhere?  What happens in the next board that is created with this
 chip and the memory locations are in a different place?

I have not looked at this driver but...

Couldn't this be done from user space using urjtag?

http://urjtag.org/

Regards,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

2014-01-20 Thread Insop Song 
On Mon, Jan 20, 2014 at 10:51 AM, Hartley Sweeten
hartl...@visionengravers.com wrote:

 I have not looked at this driver but...

 Couldn't this be done from user space using urjtag?

 http://urjtag.org/


Thank you for letting me know urjtag.
This can be useful to replace existing jtag dongles.

However, the use case for my fpgaboot driver and urjtag are different.

The fpgaboot driver downloads Xilinx FPGA firmware during run time on
the target without any h/w device support (i.e jtag dongle). This can
be used in development as well as deployment scenario. And it does
_one_ thing very well, this can download fpga firmware (10s of MByte)
in less then half a seconds within our system.

What I see urjtag does exactly what jtag devices are doing, which
means urjtag normally runs on development host (not on target), and
need a jatag dongle. It supports many different types of dongles and
devices, which itself is awesome in many ways, and could be cheaper.

Regards,

ISS
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

2014-01-16 Thread Insop Song 
On Tue, Jan 14, 2014 at 10:00 AM, Greg KH gre...@linuxfoundation.org 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.s...@gmail.com wrote:
  On Mon, Jan 13, 2014 at 10:44 AM, Greg KH gre...@linuxfoundation.org 
  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 gre...@linuxfoundation.org 
  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 gre...@linuxfoundation.org 
   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.s...@gainspeed.com
   
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

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

2014-01-16 Thread Greg KH 
On Thu, Jan 16, 2014 at 12:56:15AM -0800, Insop Song wrote:
 On Tue, Jan 14, 2014 at 10:00 AM, Greg KH gre...@linuxfoundation.org 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.s...@gmail.com wrote:
   On Mon, Jan 13, 2014 at 10:44 AM, Greg KH gre...@linuxfoundation.org 
   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 gre...@linuxfoundation.org 
   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 
gre...@linuxfoundation.org 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.s...@gainspeed.com

 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
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

2014-01-16 Thread Insop Song 
On Thu, Jan 16, 2014 at 8:09 AM, Greg KH gre...@linuxfoundation.org wrote:
 On Thu, Jan 16, 2014 at 12:56:15AM -0800, Insop Song wrote:
 On Tue, Jan 14, 2014 at 10:00 AM, Greg KH gre...@linuxfoundation.org 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.s...@gmail.com wrote:
   On Mon, Jan 13, 2014 at 10:44 AM, Greg KH gre...@linuxfoundation.org 
   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 gre...@linuxfoundation.org 
   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 
gre...@linuxfoundation.org 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.s...@gainspeed.com

 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?


On the FPGA side, there are dedicated pins for programming, and
through these you cannot get meaningful information (again unless you
are JTAG capable)
Such as these on the FPGA side, PROGRAM_B, INIT_B, CCLK, D[0:7], and DONE.
On a process side, we use gpio pin to connect to the above pins.
It's GPIO pins that we do the bit banging as defined for programming
guide from Xilinx.

Here are few use cases of using my driver:

1. As a FPGA development support tool,
During FPGA

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

2014-01-14 Thread Insop Song 
On Mon, Jan 13, 2014 at 10:44 AM, Greg KH gre...@linuxfoundation.org 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 gre...@linuxfoundation.org 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 gre...@linuxfoundation.org 
  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.s...@gainspeed.com
  
   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
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

2014-01-14 Thread Insop Song 
On Tue, Jan 14, 2014 at 1:18 AM, Insop Song insop.s...@gmail.com wrote:
 On Mon, Jan 13, 2014 at 10:44 AM, Greg KH gre...@linuxfoundation.org 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 gre...@linuxfoundation.org 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 gre...@linuxfoundation.org 
  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.s...@gainspeed.com
  
   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.



Greg, I think it's better to add more explanation for FPGA.

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.

FPGA firmware download happen ever time system boots, so this is
different from occasional upgrade.

To sum up, FPGA download occurs every boot, and before this download
is done, the device in question is not functional.

 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

Thank you,

ISS
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

2014-01-14 Thread Greg KH 
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.s...@gmail.com wrote:
  On Mon, Jan 13, 2014 at 10:44 AM, Greg KH gre...@linuxfoundation.org 
  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 gre...@linuxfoundation.org 
  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 gre...@linuxfoundation.org 
   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.s...@gainspeed.com
   
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?

 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 :)

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

  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

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

2014-01-13 Thread Greg KH 
On Sat, Jan 11, 2014 at 04:37:32PM -0800, Insop Song wrote:
 On Sat, Jan 11, 2014 at 4:10 PM, Greg KH gre...@linuxfoundation.org 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 gre...@linuxfoundation.org 
  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.s...@gainspeed.com
  
   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?

 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.

 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?

thanks,

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

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

2014-01-11 Thread Greg KH 
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.s...@gainspeed.com

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.

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

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

2014-01-11 Thread Greg KH 
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.s...@gainspeed.com
 
 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.

thanks,

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

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

2014-01-11 Thread Insop Song 
On Sat, Jan 11, 2014 at 12:51 PM, Greg KH gre...@linuxfoundation.org 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.s...@gainspeed.com

 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.


Before sending out the patch, I tested on powerpc config since I am
using powerpc target.

Here is the log:

BUILD TEST for PPC64, 1
---

- tested before the v1 patch sent

$ CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make ppc64e_defconfig
$ make nconfig
$ grep -i fpga .config
CONFIG_GS_FPGABOOT=y
$ yes  | \make oldconfig
$ time CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make -j4
..
  CC  drivers/staging/gs_fpgaboot/io.o
  CC  drivers/rtc/class.o
  CC [M]  fs/xfs/xfs_sb.o
  LD  drivers/staging/gs_fpgaboot/gs_fpga.o
  LD  drivers/staging/gs_fpgaboot/built-in.o
..  LD [M]  sound/soundcore.ko
real5m33.652s
user14m33.011s
sys 1m12.000s



BUILD TEST for PPC64,2
---

- tested before the v1 patch sent

$ grep -i GS_FPGA .config
CONFIG_GS_FPGABOOT=m

$ time CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make -j4
..
  LD  drivers/staging/gs_fpgaboot/built-in.o
  CC [M]  drivers/staging/gs_fpgaboot/gs_fpgaboot.o
  CC [M]  drivers/staging/gs_fpgaboot/io.o
  LD [M]  drivers/staging/gs_fpgaboot/gs_fpga.o
  LD  drivers/staging/built-in.o
...
real0m13.252s
user0m17.697s
sys 0m2.280s



BUILD TEST for PPC64, 3
---
- tested before the v1 patch sent

$ CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make distclean
$ CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make ppc64e_defconfig
$  grep -i GS_FPGA .config
CONFIG_GS_FPGABOOT=m

$ time CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make -j4
..
real5m23.871s
user15m41.403s
sys 1m14.009s



Sorry for not checking other platform, so I made this change to check
for ARCH and ran the test.

How about this? if it is okay, then I will send a new patch.

diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index 746af9b..2770de5 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -27,7 +27,10 @@
 #include linux/of.h
 #include linux/delay.h

+#if defined(CONFIG_PPC64)
 #include include/asm/io.h
+#endif
+
 #include linux/firmware.h

 #include gs_fpgaboot.h
diff --git a/drivers/staging/gs_fpgaboot/io.c b/drivers/staging/gs_fpgaboot/io.c
index ce0e248..2d33ff6 100644
--- a/drivers/staging/gs_fpgaboot/io.c
+++ b/drivers/staging/gs_fpgaboot/io.c
@@ -27,15 +27,14 @@
 #include linux/of.h
 #include linux/of_address.h

-#include include/asm/io.h
 #include linux/firmware.h

 #include io.h

-/*
- * this should come from board specific configuration
- */
-#define CONFIG_B4860G100
+#if defined(CONFIG_PPC64)
+#include include/asm/io.h
+#define CONFIG_B4860G100 /* board specific configuration */
+#endif

 #ifdef CONFIG_B4860G100
 static struct gpiobus gbus;



Here is the build output

BUILD TEST ARCH=x86_64
---
- tested with more change

$ make distclean
$ make defconfig
$ make -j4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

2014-01-11 Thread Insop Song 
On Sat, Jan 11, 2014 at 1:05 PM, Greg KH gre...@linuxfoundation.org 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.s...@gainspeed.com

 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);

Thank you,

ISS

-


diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
index 746af9b..eacc128 100644
--- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
+++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
@@ -27,7 +27,10 @@
 #include linux/of.h
 #include linux/delay.h

+#if defined(CONFIG_PPC64)
 #include include/asm/io.h
+#endif
+
 #include linux/firmware.h

 #include gs_fpgaboot.h
@@ -39,16 +42,13 @@
 static uint8_t bits_magic[] = {
0x0, 0x9, 0xf, 0xf0, 0xf, 0xf0,
0xf, 0xf0, 0xf, 0xf0, 0x0, 0x0, 0x1};
-/*
- * Device variables
- */
-static int fpga_major;
-static struct class *fpga_class;
-static struct device *fpga_device;

-static char*file = xlinx_fpga_top_bitstream.bit;
+/* fake device for request_firmware */
+static struct platform_device*firmware_pdev;
+
+static char  *file = xlinx_fpga_firmware.bit;
 module_param(file, charp, S_IRUGO);
-MODULE_PARM_DESC(file, FPGA image file name.);
+MODULE_PARM_DESC(file, Xilinx FPGA firmware file.);

 #ifdef DEBUG_FPGA
 static void datadump(char *msg, void *m, int n)
@@ -205,7 +205,7 @@ static int gs_load_image(struct fpgaimage *fimage,
char *file)

pr_info(load fpgaimage %s\n, file);

-   err = request_firmware(fimage-fw_entry, file, fpga_device);
+ err = request_firmware(fimage-fw_entry, file, firmware_pdev-dev);
if (err != 0) {
pr_err(firmware %s is missing, cannot continue.\n, file);
return err;
@@ -302,93 +302,19 @@ static int gs_set_download_method(struct
fpgaimage *fimage)
return 0;
 }

-static int gs_fpgaboot_release(struct inode *inode, struct file *file)
-{
-   pr_info(gs_fpgaboot_release\n);
-   return 0;
-}
-
-static int gs_fpgaboot_open(struct inode *inode, struct file *file)
-{
-   pr_info(gs_fpgaboot_open\n);
-   return 0;
-}
-
-static ssize_t gs_fpgaboot_write(struct file *f,
-   const char *c, size_t s, loff_t *lo)
-{
-   pr_info(gs_fpgaboot_write\n);
-   return 0;
-}
-
-const struct file_operations fpga_fops = {
-   .owner = THIS_MODULE,
-   .open = gs_fpgaboot_open,
-   .write = gs_fpgaboot_write,
-   .release = gs_fpgaboot_release,
-};
-
 static int init_driver(void)
 {
-   int retval;
-
-   /*
-* First, see if we can dynamically allocate
-* a major for our device
-*/
-   fpga_major = register_chrdev(0, DEVICE_NAME, fpga_fops);
-   if (fpga_major  0) {
-   pr_err(failed to register device: error %d\n,
-   fpga_major);
-   retval = fpga_major;
-   goto failed_chrdevreg;
-   }
-
-   /*
-* We can either tie our device to
-* a bus (existing, or one that we create)
-* or use a virtual device class.
-* For this example, we choose the latter
-*/
-   fpga_class = class_create(THIS_MODULE, CLASS_NAME);
-   if (IS_ERR(fpga_class)) {
-   pr_err(failed to register device class '%s'\n,
-   CLASS_NAME);
-   retval = PTR_ERR(fpga_class);
-   goto failed_classreg;
-   }
-
-   /*
-* With a class, the easiest way to instantiate
-* a device is to call device_create()
-*/
-   fpga_device = device_create(fpga_class, NULL,
-   MKDEV(fpga_major, 0), NULL,
-   CLASS_NAME _ DEVICE_NAME);
-   if (IS_ERR(fpga_device)) {
-   pr_err(failed to create device '%s_%s'\n,
-   CLASS_NAME, DEVICE_NAME);
-   retval = PTR_ERR(fpga_device);
-   goto failed_devreg;
-   }
+ firmware_pdev = platform_device_register_simple(fpgaboot, -1,
+  NULL, 0);
+ if (IS_ERR(firmware_pdev))
+ return PTR_ERR(firmware_pdev);

return 0;
-
-failed_devreg:
-

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

2014-01-11 Thread Greg KH 
On Sat, Jan 11, 2014 at 03:56:48PM -0800, Insop Song wrote:
 On Sat, Jan 11, 2014 at 1:05 PM, Greg KH gre...@linuxfoundation.org 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.s...@gainspeed.com
 
  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.

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

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

2014-01-11 Thread Greg KH 
On Sat, Jan 11, 2014 at 02:16:05PM -0800, Insop Song wrote:
 On Sat, Jan 11, 2014 at 12:51 PM, Greg KH gre...@linuxfoundation.org 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.s...@gainspeed.com
 
  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.
 
 
 Before sending out the patch, I tested on powerpc config since I am
 using powerpc target.
 
 Here is the log:
 
 BUILD TEST for PPC64, 1
 ---
 
 - tested before the v1 patch sent
 
 $ CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make ppc64e_defconfig
 $ make nconfig
 $ grep -i fpga .config
 CONFIG_GS_FPGABOOT=y
 $ yes  | \make oldconfig
 $ time CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make -j4
 ..
   CC  drivers/staging/gs_fpgaboot/io.o
   CC  drivers/rtc/class.o
   CC [M]  fs/xfs/xfs_sb.o
   LD  drivers/staging/gs_fpgaboot/gs_fpga.o
   LD  drivers/staging/gs_fpgaboot/built-in.o
 ..  LD [M]  sound/soundcore.ko
 real5m33.652s
 user14m33.011s
 sys 1m12.000s
 
 
 
 BUILD TEST for PPC64,2
 ---
 
 - tested before the v1 patch sent
 
 $ grep -i GS_FPGA .config
 CONFIG_GS_FPGABOOT=m
 
 $ time CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make -j4
 ..
   LD  drivers/staging/gs_fpgaboot/built-in.o
   CC [M]  drivers/staging/gs_fpgaboot/gs_fpgaboot.o
   CC [M]  drivers/staging/gs_fpgaboot/io.o
   LD [M]  drivers/staging/gs_fpgaboot/gs_fpga.o
   LD  drivers/staging/built-in.o
 ...
 real0m13.252s
 user0m17.697s
 sys 0m2.280s
 
 
 
 BUILD TEST for PPC64, 3
 ---
 - tested before the v1 patch sent
 
 $ CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make distclean
 $ CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make ppc64e_defconfig
 $  grep -i GS_FPGA .config
 CONFIG_GS_FPGABOOT=m
 
 $ time CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make -j4
 ..
 real5m23.871s
 user15m41.403s
 sys 1m14.009s
 
 
 
 Sorry for not checking other platform, so I made this change to check
 for ARCH and ran the test.
 
 How about this? if it is okay, then I will send a new patch.
 
 diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
 b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
 index 746af9b..2770de5 100644
 --- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
 +++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
 @@ -27,7 +27,10 @@
  #include linux/of.h
  #include linux/delay.h
 
 +#if defined(CONFIG_PPC64)
  #include include/asm/io.h
 +#endif

Ick, no, you should never need an #if statment in a driver at all.

What's wrong with a simple:
#include linux/io.h
instead?

 -/*
 - * this should come from board specific configuration
 - */
 -#define CONFIG_B4860G100

Isn't there a config option already for this platform?

thanks,

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

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

2014-01-11 Thread Insop Song 
On Sat, Jan 11, 2014 at 4:10 PM, Greg KH gre...@linuxfoundation.org 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 gre...@linuxfoundation.org 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.s...@gainspeed.com
 
  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.

It downloads the image and after that no need to be existing.
Here is how we use

$ insmod gs_fpga.ko file=xlinx_fpga_top_bitstream.bit
$ rmmod gs_fpga

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.

What do you think?

Thank you,

ISS
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

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

2014-01-11 Thread Insop Song 
On Sat, Jan 11, 2014 at 4:09 PM, Greg KH gre...@linuxfoundation.org wrote:
 On Sat, Jan 11, 2014 at 02:16:05PM -0800, Insop Song wrote:
 On Sat, Jan 11, 2014 at 12:51 PM, Greg KH gre...@linuxfoundation.org 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.s...@gainspeed.com
 
  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.
 

 Before sending out the patch, I tested on powerpc config since I am
 using powerpc target.

 Here is the log:

 BUILD TEST for PPC64, 1
 ---

 - tested before the v1 patch sent

 $ CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make ppc64e_defconfig
 $ make nconfig
 $ grep -i fpga .config
 CONFIG_GS_FPGABOOT=y
 $ yes  | \make oldconfig
 $ time CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make -j4
 ..
   CC  drivers/staging/gs_fpgaboot/io.o
   CC  drivers/rtc/class.o
   CC [M]  fs/xfs/xfs_sb.o
   LD  drivers/staging/gs_fpgaboot/gs_fpga.o
   LD  drivers/staging/gs_fpgaboot/built-in.o
 ..  LD [M]  sound/soundcore.ko
 real5m33.652s
 user14m33.011s
 sys 1m12.000s



 BUILD TEST for PPC64,2
 ---

 - tested before the v1 patch sent

 $ grep -i GS_FPGA .config
 CONFIG_GS_FPGABOOT=m

 $ time CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make -j4
 ..
   LD  drivers/staging/gs_fpgaboot/built-in.o
   CC [M]  drivers/staging/gs_fpgaboot/gs_fpgaboot.o
   CC [M]  drivers/staging/gs_fpgaboot/io.o
   LD [M]  drivers/staging/gs_fpgaboot/gs_fpga.o
   LD  drivers/staging/built-in.o
 ...
 real0m13.252s
 user0m17.697s
 sys 0m2.280s



 BUILD TEST for PPC64, 3
 ---
 - tested before the v1 patch sent

 $ CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make distclean
 $ CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make ppc64e_defconfig
 $  grep -i GS_FPGA .config
 CONFIG_GS_FPGABOOT=m

 $ time CROSS_COMPILE=powerpc64-fsl-linux- ARCH=powerpc  make -j4
 ..
 real5m23.871s
 user15m41.403s
 sys 1m14.009s



 Sorry for not checking other platform, so I made this change to check
 for ARCH and ran the test.

 How about this? if it is okay, then I will send a new patch.

 diff --git a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
 b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
 index 746af9b..2770de5 100644
 --- a/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
 +++ b/drivers/staging/gs_fpgaboot/gs_fpgaboot.c
 @@ -27,7 +27,10 @@
  #include linux/of.h
  #include linux/delay.h

 +#if defined(CONFIG_PPC64)
  #include include/asm/io.h
 +#endif

 Ick, no, you should never need an #if statment in a driver at all.

 What's wrong with a simple:
 #include linux/io.h
 instead?


Thank you, that works.

-#include include/asm/io.h
+#include linux/io.h


 -/*
 - * this should come from board specific configuration
 - */
 -#define CONFIG_B4860G100

 Isn't there a config option already for this platform?


CONFIG_B4860G100 is not in Kconfig of the kernel nor my patch, yet.

This defines the target I am using and defines how to actually program
FPGA firmware using gpio pins.

I put #define CONFIG_B4860G100 in the patch, so that we know build
goes through the code and make sure it builds.

I will remove this define.

-/*
- * this should come from board specific configuration
- */
-#define CONFIG_B4860G100
-


Thank you,

ISS,
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel