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 Sat, Jan 11, 2014 at 02:16:05PM -0800, Insop Song wrote:
> On Sat, Jan 11, 2014 at 12:51 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> 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.
> >
> 
> 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
> real    5m33.652s
> user    14m33.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
> ...
> real    0m13.252s
> user    0m17.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
> ..
> real    5m23.871s
> user    15m41.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
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