From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ang, Chee Hong <chee.hong.ang@intel.com>
Date: Mon, 8 Oct 2018 15:10:31 +0000
Subject: [U-Boot] [PATCH v2 3/4] arm: socfpga: stratix10: Add Stratix10
 FPGA into FPGA device table
In-Reply-To: <cd0915ac-95e3-48bf-5795-4c0763f080fd@denx.de>
References: <1538992107-37855-1-git-send-email-chee.hong.ang@intel.com>
 <1538992107-37855-4-git-send-email-chee.hong.ang@intel.com>
 <cd0915ac-95e3-48bf-5795-4c0763f080fd@denx.de>
Message-ID: <1539011430.38820.9.camel@intel.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
To: u-boot@lists.denx.de

On Mon, 2018-10-08 at 11:57 +0200, Marek Vasut wrote:
> On 10/08/2018 11:48 AM, chee.hong.ang at intel.com wrote:
> > 
> > From: "Ang, Chee Hong" <chee.hong.ang@intel.com>
> > 
> > Enable 'fpga' command in u-boot. User will be able to use the fpga
> > command to program the FPGA on Stratix10 SoC.
> > 
> > Signed-off-by: Ang, Chee Hong <chee.hong.ang@intel.com>
> > ---
> >  arch/arm/mach-socfpga/misc.c     | 29
> > +++++++++++++++++++++++++++++
> >  arch/arm/mach-socfpga/misc_s10.c |  2 ++
> >  drivers/fpga/altera.c            |  6 ++++++
> >  include/altera.h                 |  4 ++++
> >  4 files changed, 41 insertions(+)
> > 
> > diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-
> > socfpga/misc.c
> > index a4f6d5c..7986b58 100644
> > --- a/arch/arm/mach-socfpga/misc.c
> > +++ b/arch/arm/mach-socfpga/misc.c
> > @@ -88,6 +88,27 @@ int overwrite_console(void)
> >  #endif
> >  
> >  #ifdef CONFIG_FPGA
> > +#ifdef CONFIG_FPGA_STRATIX10
> > +/*
> > + * FPGA programming support for SoC FPGA Stratix 10
> > + */
> > +static Altera_desc altera_fpga[] = {
> > +	{
> > +		/* Family */
> > +		Intel_FPGA_Stratix10,
> > +		/* Interface type */
> > +		secure_device_manager_mailbox,
> > +		/* No limitation as additional data will be
> > ignored */
> > +		-1,
> > +		/* No device function table */
> > +		NULL,
> > +		/* Base interface address specified in driver */
> > +		NULL,
> > +		/* No cookie implementation */
> > +		0
> > +	},
> > +};
> > +#else
> >  /*
> >   * FPGA programming support for SoC FPGA Cyclone V
> >   */
> > @@ -107,6 +128,7 @@ static Altera_desc altera_fpga[] = {
> >  		0
> >  	},
> >  };
> > +#endif
> >  
> >  /* add device descriptor to FPGA device table */
> >  void socfpga_fpga_add(void)
> > @@ -116,6 +138,13 @@ void socfpga_fpga_add(void)
> >  	for (i = 0; i < ARRAY_SIZE(altera_fpga); i++)
> >  		fpga_add(fpga_altera, &altera_fpga[i]);
> >  }
> > +
> > +#else
> > +
> > +__weak void socfpga_fpga_add(void)
> > +{
> > +}
> Why is a __weak function defined only in else-statement ?
> 
> It should be defined always, with a sane default implementation.

I will remove the empty function in #else-statement and define the
default function like this :

/* add device descriptor to FPGA device table */
void socfpga_fpga_add(void)
{
#ifdef CONFIG_FPGA
	int i;
	fpga_init();
	for (i = 0; i < ARRAY_SIZE(altera_fpga); i++)
		fpga_add(fpga_altera, &altera_fpga[i]);
#endif
}

Is that OK?