[U-Boot] [PATCH 1/3] arm: socfpga: Remove unused FPGA feature from gen5 SPL
Chee, Tien Fong
tien.fong.chee at intel.com
Fri Jul 28 05:16:28 UTC 2017
On Kha, 2017-07-27 at 11:37 +0200, Marek Vasut wrote:
> On 07/27/2017 11:21 AM, Chee, Tien Fong wrote:
> >
> > On Kha, 2017-07-27 at 10:21 +0200, Marek Vasut wrote:
> > >
> > > On 07/27/2017 06:36 AM, tien.fong.chee at intel.com wrote:
> > > >
> > > >
> > > > From: Tien Fong Chee <tien.fong.chee at intel.com>
> > > >
> > > > Remove unused FPGA feature for saving some space in gen5 SPL.
> > > I really dislike the ifdefery. Why is this patch even needed?
> > > I thought we had no space problems in the Gen5 SPL?
> > >
> > I can remove those codes within ifdefery because they are no longer
> > required.
> Why ?
>
because those functions are testing on FPGA when the bridge is enabled
in SPL. But, i will keep minimal ifdefery on socfpga_bridges_reset to
indicate the fpga bridges should not be released in SPL.
> >
> > I keep them here just for one day we can use if we need to.
> > Do you remember we have consent to clean up those useless codes for
> > SPL, all fpga related drivers will be moved into one driver
> > location.
> No, sorry.
>
Are you disagree on keeping the ifdefery codes, or disagree on moving
all FPGA related functions into drivers/fpga/... ?
> >
> > >
> > > >
> > > >
> > > > Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.com>
> > > > ---
> > > > arch/arm/mach-socfpga/reset_manager_gen5.c | 9 +++++++++
> > > > arch/arm/mach-socfpga/system_manager_gen5.c | 6 ------
> > > > drivers/ddr/altera/sdram.c | 8 +++++++-
> > > > 3 files changed, 16 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/arch/arm/mach-socfpga/reset_manager_gen5.c
> > > > b/arch/arm/mach-socfpga/reset_manager_gen5.c
> > > > index aa88adb..c954063 100644
> > > > --- a/arch/arm/mach-socfpga/reset_manager_gen5.c
> > > > +++ b/arch/arm/mach-socfpga/reset_manager_gen5.c
> > > > @@ -97,6 +97,14 @@ void socfpga_bridges_reset(int enable)
> > > > writel(0, &sysmgr_regs->iswgrp_handoff[0]);
> > > > writel(l3mask, &sysmgr_regs-
> > > > >iswgrp_handoff[1]);
> > > >
> > > > +/*
> > > > + * FPGA feature is not required in SPL, so FPGA bridges
> > > > release
> > > > from reset
> > > > + * should not be supported in SPL, but some FPGA releated
> > > > setting
> > > > can be stored
> > > > + * in handoff registers as SPL to U-boot/OS handoff
> > > > information.
> > > > These
> > > > + * handoff information can be used in later phase such as
> > > > feeding
> > > > handoff
> > > > + * information to bridge command when user want to enable FPGA
> > > > feature in U-boot
> > > > + */
> > > > +#ifndef CONFIG_SPL_BUILD
> > > > /* Check signal from FPGA. */
> > > > if (!fpgamgr_test_fpga_ready()) {
> > > > /* FPGA not ready, do nothing. We
> > > > allow
> > > > system to boot
> > > > @@ -110,6 +118,7 @@ void socfpga_bridges_reset(int enable)
> > > >
> > > > /* Remap the bridges into memory map */
> > > > writel(l3mask, SOCFPGA_L3REGS_ADDRESS);
> > > I believe the L3REGS needs to be programmed on Gen5.
> > >
> > Yes, this is done when calling command "bridge
> > enable". iswgrp_handoff[1] contains l3mask, which will be written
> > into SOCFPGA_L3REGS_ADDRESS.
> I think this needs to be done earlier, otherwise the first 1 MiB or
> so
> of DRAM isn't accessible.
>
Yeah, you are right....this is what i missing, the OCRAM should be
remap to lowest memory region 1 MiB. So i propose just to remap OCRAM,
and FPGA related bridges can be remaped in U-boot by calling the
"enable bridge" command.
> >
> > Snippet from misc_gen5.c:
> > int do_bridge(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> > argv[])
> > {
> > if (argc != 2)
> > return CMD_RET_USAGE;
> >
> > argv++;
> >
> > switch (*argv[0]) {
> > case 'e': /* Enable */
> > writel(iswgrp_handoff[2], &sysmgr_regs-
> > >
> > > fpgaintfgrp_module);
> > socfpga_sdram_apply_static_cfg();
> > writel(iswgrp_handoff[3], &sdr_ctrl->fpgaport_rst);
> > writel(iswgrp_handoff[0], &reset_manager_base-
> > >
> > > brg_mod_reset);
> > writel(iswgrp_handoff[1], &nic301_regs->remap);
> > break;
> >
> > >
> > > >
> > > >
> > > > +#endif
> > > > }
> > > > return;
> > > > }
> > > > diff --git a/arch/arm/mach-socfpga/system_manager_gen5.c
> > > > b/arch/arm/mach-socfpga/system_manager_gen5.c
> > > > index 3588a57..ee25496 100644
> > > > --- a/arch/arm/mach-socfpga/system_manager_gen5.c
> > > > +++ b/arch/arm/mach-socfpga/system_manager_gen5.c
> > > > @@ -43,12 +43,6 @@ static void
> > > > populate_sysmgr_fpgaintf_module(void)
> > > > /* populate (not writing) the value for
> > > > SYSMGR.FPGAINTF.MODULE
> > > > based on pinmux setting */
> > > > setbits_le32(&sysmgr_regs->iswgrp_handoff[2],
> > > > handoff_val);
> > > > -
> > > > - handoff_val = readl(&sysmgr_regs->iswgrp_handoff[2]);
> > > > - if (fpgamgr_test_fpga_ready()) {
> > > > - /* Enable the required signals only */
> > > > - writel(handoff_val, &sysmgr_regs-
> > > > >
> > > > > fpgaintfgrp_module);
> > > > - }
> > > > }
> > > >
> > > > /*
> > > > diff --git a/drivers/ddr/altera/sdram.c
> > > > b/drivers/ddr/altera/sdram.c
> > > > index e74c5b0..df16102 100644
> > > > --- a/drivers/ddr/altera/sdram.c
> > > > +++ b/drivers/ddr/altera/sdram.c
> > > > @@ -233,6 +233,7 @@ static void
> > > > sdram_dump_protection_config(void)
> > > > }
> > > > }
> > > >
> > > > +#ifndef CONFIG_SPL_BUILD
> > > > /**
> > > > * sdram_write_verify() - write to register and verify the
> > > > write.
> > > > * @addr: Register address
> > > > @@ -259,6 +260,7 @@ static unsigned sdram_write_verify(const
> > > > u32
> > > > *addr, const u32 val)
> > > > debug("correct!\n");
> > > > return 0;
> > > > }
> > > > +#endif
> > > >
> > > > /**
> > > > * sdr_get_ctrlcfg() - Get the value of DRAM CTRLCFG register
> > > > @@ -435,7 +437,6 @@ int sdram_mmr_init_full(unsigned int
> > > > sdr_phy_reg)
> > > > const unsigned int rows =
> > > > (cfg->dram_addrw &
> > > > SDR_CTRLGRP_DRAMADDRW_ROWBITS_MASK) >>
> > > > SDR_CTRLGRP_DRAMADDRW_ROWBITS_LSB;
> > > > - int ret;
> > > >
> > > > writel(rows, &sysmgr_regs->iswgrp_handoff[4]);
> > > >
> > > > @@ -444,13 +445,18 @@ int sdram_mmr_init_full(unsigned int
> > > > sdr_phy_reg)
> > > > /* saving this value to SYSMGR.ISWGRP.HANDOFF.FPGA2SDR
> > > > */
> > > > writel(cfg->fpgaport_rst, &sysmgr_regs-
> > > > >
> > > > > iswgrp_handoff[3]);
> > > >
> > > > + /* FPGA feature is not required in SPL, so no test on
> > > > FPGA
> > > > reset in SPL */
> > > > +#ifndef CONFIG_SPL_BUILD
> > > > /* only enable if the FPGA is programmed */
> > > > + int ret;
> > > > +
> > > > if (fpgamgr_test_fpga_ready()) {
> > > > ret = sdram_write_verify(&sdr_ctrl-
> > > > >fpgaport_rst,
> > > > cfg->fpgaport_rst);
> > > > if (ret)
> > > > return ret;
> > > > }
> > > > +#endif
> > > >
> > > > /* Restore the SDR PHY Register if valid */
> > > > if (sdr_phy_reg != 0xffffffff)
> > > >
>
More information about the U-Boot
mailing list