[U-Boot] [PATCH v8 4/7] arm: socfpga: Enable FPGA driver on SPL

[Chee, Tien Fong]

On Rab, 2017-06-07 at 14:31 +0200, Marek Vasut wrote:
> On 06/07/2017 01:26 PM, Chee, Tien Fong wrote:
> > 
> > On Rab, 2017-06-07 at 16:04 +0800, Chee, Tien Fong wrote:
> > > 
> > > On Rab, 2017-06-07 at 08:36 +0200, Marek Vasut wrote:
> > > > 
> > > > 
> > > > On 06/07/2017 05:06 AM, Chee, Tien Fong wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > On Sel, 2017-06-06 at 11:50 +0200, Marek Vasut wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On 06/06/2017 11:46 AM, Chee, Tien Fong wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > On Sel, 2017-06-06 at 11:41 +0200, Marek Vasut wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > On 06/06/2017 11:36 AM, Chee, Tien Fong wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > On Sel, 2017-06-06 at 10:35 +0200, Marek Vasut wrote:
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > On 06/06/2017 10:19 AM, Chee, Tien Fong wrote:
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > On Sel, 2017-06-06 at 10:03 +0200, Marek Vasut
> > > > > > > > > > > wrote:
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > On 06/06/2017 08:35 AM, tien.fong.chee at intel.co
> > > > > > > > > > > > m
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > From: Tien Fong Chee <tien.fong.chee at intel.co
> > > > > > > > > > > > > m>
> > > > > > > > > > > > > 
> > > > > > > > > > > > > This patch is for enabling FPGA driver
> > > > > > > > > > > > > support on
> > > > > > > > > > > > > SPL
> > > > > > > > > > > > Why would we want that on Gen5 ? I believe this
> > > > > > > > > > > > is
> > > > > > > > > > > > only
> > > > > > > > > > > > needed on
> > > > > > > > > > > > Gen10.
> > > > > > > > > > > > 
> > > > > > > > > > > I already moved the fpga_manager driver into
> > > > > > > > > > > drivers/fpga/
> > > > > > > > > > > on
> > > > > > > > > > > patch
> > > > > > > > > > > 6,
> > > > > > > > > > > and fpga_manager drivers are required on SPL.
> > > > > > > > > > > Actually
> > > > > > > > > > > fpga_manager
> > > > > > > > > > > driver should be part of the drivers/fpga.
> > > > > > > > > > I think I miss some fundamental piece of
> > > > > > > > > > information .
> > > > > > > > > > Why
> > > > > > > > > > would
> > > > > > > > > > I
> > > > > > > > > > need
> > > > > > > > > > anything from the FPGA framework in SPL on Gen5 ?
> > > > > > > > > > It is
> > > > > > > > > > not
> > > > > > > > > > needed
> > > > > > > > > > thus
> > > > > > > > > > far. Is it because you shuffled some of the code
> > > > > > > > > > around
> > > > > > > > > > or
> > > > > > > > > > what ?
> > > > > > > > > > 
> > > > > > > > > Because we need to know some status and mode type
> > > > > > > > > from
> > > > > > > > > FPGA
> > > > > > > > > even we
> > > > > > > > > did
> > > > > > > > > not program FPGA in SPL.
> > > > > > > > But we didn't have this option enabled before and
> > > > > > > > everything
> > > > > > > > worked
> > > > > > > > on
> > > > > > > > gen5, why do we need it now ?
> > > > > > > > 
> > > > > > > Because i already move them into fpga driver, because
> > > > > > > those
> > > > > > > functions
> > > > > > > should be part of fpga driver. So, this moving happen in
> > > > > > > patch
> > > > > > > 6.
> > > > > > I see. Does enabling the FPGA_SPL stuff pull in any of the
> > > > > > FPGA
> > > > > > framework ? Does the size of the Gen5 SPL change before and
> > > > > > after
> > > > > > this
> > > > > > patchset ? If you did not check, please do.
> > > > > Yeah, i confirm FPGA driver is availabled in SPL. A bit
> > > > > change in
> > > > > size,
> > > > > 1~2K more. I did the test on our devkits also.
> > > > OK, so that's not acceptable because we're already very close
> > > > to
> > > > the
> > > > size limit of the SPL.
> > > > 
> > > Any safety guideline?
> > > I checked the spl.map, we still have 10K left after calculation
> > > including bss size.
> > > 
> > I compiled all Intel fpga related defconfigs, and we have 9K free
> > based
> > on total 64K memory size. SO building PFGA driver would contribute
> > around 1~2K only to SPL size. Do you have concern with that?
> Yes
> 
> > 
> > If you have concern, i would remove patch 6, keep fpga_manager
> > intact.
> Can't you rework things such that they don't add useless code into
> the
> SPL instead ?
> 
I checked the codes yesterday, mostly are bridge, and HPS-FPGA
interface configuration in SPL. So, i think we can try to remove them
as they are not required in SPL, but i need more time to test it out
and ensure this change doesn't break anything.
To avoid impact the progress of submitting the rest of patchset as they
are still pending until this patchset is accepted , so i suggest we can
do it in later. So, i will keep fpga_manager in there.

sounds good to you?
> > 
> > > 
> > > Thanks.
>