On 3/9/20 3:24 PM, Simon Goldschmidt wrote: > On Mon, Mar 9, 2020 at 3:15 PM Marek Vasut <ma...@denx.de> wrote: >> >> 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 ? > > From the CPU side, that should work, no? Of course you have to make sure no > other peripheraly (including FPGA!) are using the RAM. > > And if this would be an explicit command, people needing this could > experiment with it - and hopefully give better hints as to what's going wrong > if we *do* see problems again.
Maybe altera has something hidden somewhere in the bus tunables ? :)
