From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Goldschmidt Date: Mon, 16 Mar 2020 20:09:16 +0100 Subject: [U-Boot] [PATCH] ARM: socfpga: Remove socfpga_sdram_apply_static_cfg() In-Reply-To: References: <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> <093bd6c18dc96cd2d2cbf7d19c20edac98f2a7ec.camel@linux.intel.com> <2c711e5d-bad7-9c72-043b-ec2abc9f0f4d@gmail.com> 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 Am 16.03.2020 um 20:06 schrieb Marek Vasut: > On 3/16/20 8:04 PM, Simon Goldschmidt wrote: >> Am 16.03.2020 um 19:04 schrieb Dalon L Westergreen: >>> >>> >>> On Thu, 2020-03-12 at 16:57 +0100, Marek Vasut wrote: >>>> On 3/12/20 4:54 PM, Dalon L Westergreen wrote: >>>> [...] >>>> >>>> (thanks for fixing your mailer :)) >>>> >>>>>>>>>> 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? >>>>>>>> >>>>> >>>>> Unfortunately the sdram cfg apply must occur AFTER the fpga is >>>>> configured. This >>>>> is because the FPGA drives configuration bits, around the interfaces >>>>> datawidth >>>>> for example, that are used in setting up the dram interface. I believe the >>>>> intent is for the command to only run when the ridge enable function is >>>>> called. >>>> >>>> So that's one part of the fix -- only run this apply_static_cfg if the >>>> bitstream uses the F2S bridge. >>> >>> actually, the restriction is to apply this only if the FPGA is configured, >>> whether the bridge is used is irrelevant. you can further restrict this >>> to only when the bridge is used, but to me that means devicetree entries >>> or something to determine whether it is used. >> >> In my opinion, we need an FPGA-specific devicetree (or something >> similar) to describe the FPGA image, anyway. > > Like a DTO ? > >> Today, too much >> configuration is applied at compile time (or when programming SPL to >> flash at latest) - there's currently no way to switch peripherals to >> FPGA for one image but not for another, for example. > > Yes > >> Worse: if you enable bridges but there's no slave attached, the CPU can >> be stuck. That would need to be described with the FPGA image as well. > > Can you propose a patch ? In corona times and with kids now at home, I don't know if I can soon :-( Regards, Simon