[U-Boot] [PATCH v8 4/8] ARM: socfpga: Add FPGA drivers for Arria 10 FPGA bitstream loading

[Chee, Tien Fong]

On Thu, 2019-02-14 at 16:15 +0100, Marek Vasut wrote:
> On 2/14/19 4:14 PM, Chee, Tien Fong wrote:
> > 
> > On Thu, 2019-02-14 at 13:29 +0100, Marek Vasut wrote:
> > > 
> > > On 2/14/19 1:14 PM, Chee, Tien Fong wrote:
> > > > 
> > > > 
> > > > On Thu, 2019-02-14 at 11:41 +0100, Marek Vasut wrote:
> > > > > 
> > > > > 
> > > > > On 2/14/19 7:44 AM, Chee, Tien Fong wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On Wed, 2019-02-13 at 17:20 +0100, Marek Vasut wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > On 2/13/19 3:18 PM, tien.fong.chee at intel.com wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > From: Tien Fong Chee <tien.fong.chee at intel.com>
> > > > > > > > 
> > > > > > > > Add FPGA driver to support program FPGA with FPGA
> > > > > > > > bitstream
> > > > > > > > loading
> > > > > > > > from
> > > > > > > > filesystem. The driver are designed based on generic
> > > > > > > > firmware
> > > > > > > > loader
> > > > > > > > framework. The driver can handle FPGA program operation
> > > > > > > > from
> > > > > > > > loading FPGA
> > > > > > > > bitstream in flash to memory and then to program FPGA.
> > > > > > > > 
> > > > > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.com
> > > > > > > > >
> > > > > > > > 
> > > > > > > > ---
> > > > > > > > 
> > > > > > > > changes for v8
> > > > > > > > - Added codes to discern bitstream type based on fpga
> > > > > > > > node
> > > > > > > > name.
> > > > > > > > 
> > > > > > > > changes for v7
> > > > > > > > - Restructure the FPGA driver to support both
> > > > > > > > peripheral
> > > > > > > > bitstream
> > > > > > > > and core
> > > > > > > >   bitstream bundled into FIT image.
> > > > > > > > - Support loadable property for core bitstream. User
> > > > > > > > can
> > > > > > > > set
> > > > > > > > loadable
> > > > > > > >   in DDR for better performance. This loading would be
> > > > > > > > done
> > > > > > > > in
> > > > > > > > one
> > > > > > > > large
> > > > > > > >   chunk instead of chunk by chunk loading with small
> > > > > > > > memory
> > > > > > > > buffer.
> > > > > > > > ---
> > > > > > > >  arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts       |  
> > > > > > > > 17 +
> > > > > > > >  .../include/mach/fpga_manager_arria10.h            |  
> > > > > > > > 39
> > > > > > > > +-
> > > > > > > >  drivers/fpga/socfpga_arria10.c                     |
> > > > > > > > 467
> > > > > > > > ++++++++++++++++++++-
> > > > > > > >  3 files changed, 500 insertions(+), 23 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git
> > > > > > > > a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
> > > > > > > > b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
> > > > > > > > index 998d811..14f1967 100644
> > > > > > > > --- a/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
> > > > > > > > +++ b/arch/arm/dts/socfpga_arria10_socdk_sdmmc.dts
> > > > [...]
> > > > 
> > > > > 
> > > > > 
> > > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > - * FPGA Manager to program the FPGA. This is the
> > > > > > > > interface
> > > > > > > > used by
> > > > > > > > FPGA driver.
> > > > > > > > - * Return 0 for sucess, non-zero for error.
> > > > > > > > - */
> > > > > > > > +char *get_fpga_filename(const void *fdt, int *len)
> > > > > > > > +{
> > > > > > > > +	char *fpga_filename = NULL;
> > > > > > > > +	int node_offset;
> > > > > > > > +
> > > > > > > > +	fdtdec_find_aliases_for_id(gd->fdt_blob,
> > > > > > > > "fpga_mgr",
> > > > > > > > +				COMPAT_ALTERA_SOCFPGA_
> > > > > > > > FPGA
> > > > > > > > 0,
> > > > > > > > +				&node_offset, 1);
> > > > > > > > +
> > > > > > > > +	ofnode fpgamgr_node =
> > > > > > > > offset_to_ofnode(node_offset);
> > > > > > > > +
> > > > > > > > +	if (ofnode_valid(fpgamgr_node))
> > > > > > > > +		fpga_filename = (char
> > > > > > > > *)ofnode_read_string(fpgamgr_node,
> > > > > > > > +						"altr,
> > > > > > > > bits
> > > > > > > > trea
> > > > > > > > m");
> > > > > > > > +
> > > > > > > > +
> > > > > > > Why is the cast needed ?
> > > > > > The return string would be eventually set to the char
> > > > > > *filename
> > > > > > in
> > > > > > common struct fpga_fsinfo. So, the cast here is to avoid
> > > > > > the
> > > > > > warning
> > > > > > from compiler.
> > > > > I presume if the compiler generates a warning, it's for a
> > > > > reason.
> > > > > What
> > > > > warning is that ?
> > > > drivers/fpga/socfpga_arria10.c: In function
> > > > 'get_fpga_filename':
> > > > drivers/fpga/socfpga_arria10.c:466:17: warning: assignment
> > > > discards
> > > > 'const' qualifier from pointer target type [-Wdiscarded-
> > > > qualifiers]
> > > >    fpga_filename = ofnode_read_string(fpgamgr_node,
> > > Add missing const then ?
> > Then this requires change on common struct fpga_fsinfo, this would
> > impact to other user using this. Why the cast is not allow as we
> > only
> > reading the filename?
> If the string isn't const, someone can write it. If someone writes
> this
> string, won't it corrupt the DT ?
Yes, but this would not happen in this driver, right?
I don't know also why this common struct declare without the const, may
be it supports the write?
>