From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Mon, 9 Mar 2020 15:25:41 +0100 Subject: [U-Boot] [PATCH] ARM: socfpga: Remove socfpga_sdram_apply_static_cfg() In-Reply-To: References: <8961c1b4-94bc-fcd9-662d-1a4340469099@denx.de> <7107092b-4025-88a5-079b-9ea1002ca677@denx.de> <87a2193e-dfed-8c78-5e4a-01a4cb36545a@ic-automation.de> <5a684d24-0e60-bed3-a7c0-d53fcd54b7e4@denx.de> <7e87eb65-6fcc-d9a5-e36b-193f5bba9e3d@denx.de> <061dc254-934d-53d5-1446-cdcf16a85e5e@denx.de> <515836a3-d276-da15-616b-679ad87408ff@denx.de> <31d45968-9187-bbfd-7592-c45dd2a64e8b@denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 3/9/20 3:24 PM, Simon Goldschmidt wrote: > On Mon, Mar 9, 2020 at 3:15 PM Marek Vasut wrote: >> >> On 3/9/20 3:10 PM, Simon Goldschmidt wrote: >>> On Mon, Mar 9, 2020 at 1:57 PM Marek Vasut wrote: >>>> >>>> On 3/9/20 9:50 AM, Ley Foon Tan wrote: >>>>> On Thu, Feb 13, 2020 at 2:52 AM Dalon L Westergreen >>>>> 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 ? :)