On 3/9/20 3:10 PM, Simon Goldschmidt wrote: > On Mon, Mar 9, 2020 at 1:57 PM Marek Vasut <ma...@denx.de> wrote: >> >> On 3/9/20 9:50 AM, Ley Foon Tan wrote: >>> On Thu, Feb 13, 2020 at 2:52 AM Dalon L Westergreen >>> <dalon.westergr...@linux.intel.com> wrote: >>>> >>>> I am reading through this thread, and want to point out that it is not >>>> that the >>>> FPGA bridge need be actively used in the fpga, but >>>> rather that this port be configured in the FPGA configuration. This is an >>>> important distinction, ecery FPGA design that >>>> instantiates the HPS does configure the F2S Bridge. it drives select pins, >>>> control pins, data pins, etc, on the interface to known values, >>>> and so the apply can always be done as all SoC FPGA designs have the soc >>>> instantiated. If someone boots the arm, and uses an >>>> FPGA design without having the soc instantiated, it may then cause issues, >>>> but i >>>> would argue that is not a supported use >>>> in the first place. >>>> >>>> --dalon >>>> >>> >>> I can reproduce the issue if without setting applycfg bit. Access to >>> F2sdram interface will cause system hang. >>> >>> From the Cyclone 5 Soc datasheet: >>> applycfg - Write with this bit set to apply all the settings loaded in >>> SDR registers to the memory interface. This bit is write-only and >>> always returns 0 if read. >>> >>> Software should set applycfg bit if change any register under SDR >>> register range. SW is allowed to set this bit multiple times, the only >>> condition is SDRAM need to be idle. >>> My suggestion is we add back socfpga_sdram_apply_static_cfg(), but >>> change the sequence to below: >>> writel(iswgrp_handoff[3], &sdr_ctrl->fpgaport_rst); >>> socfpga_sdram_apply_static_cfg(); >>> >>> Marek and Simon, are you okay with this? If yes, I will submit patch for >>> this. >> >> The problem was that this was causing weird sporadic hangs on Gen5, >> which is why it was removed. So until there is an explanation for those >> hangs, I'm not really OK with this. > > These sporadic hangs you saw were on devices without an FPGA image directly > accessing the SDRAM ports, right?
Yes >> Maybe the application of static config should happen in SPL, before the >> DRAM is running, or something like that ? > > Thinking this further, limiting it to applying in SPL is not a good idea since > that prevents us from implementing different FPGA images/configs by loading a > config later in the boot (i.e. in U-Boot from a FIT-image). Well, but later we have SDRAM running and we cannot make it go idle, can we? > Would it work to make setting this optional, i.e. only write the bit if an > FPGA > config actually uses these ports? Then it couldn't lead to problems on other > hardware... Can you make SDRAM bus really idle ?