From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Mon, 14 Jan 2019 21:49:36 +0100 Subject: [U-Boot] [PATCH v1 1/4] arm: socfpga: imply SPL config instead of select In-Reply-To: References: <20190107211423.10151-1-simon.k.r.goldschmidt@gmail.com> <20190107211423.10151-2-simon.k.r.goldschmidt@gmail.com> <033b301a-612d-3b80-7ecc-04530851c1ec@denx.de> <1dd165cd-2cad-013b-877c-78fe5780f9cf@gmail.com> <00f186a1-7aea-8d46-770d-b5e08f1f92c2@denx.de> <8d7142cb-5674-57d2-dce2-a4595b8a5538@gmail.com> <9bcf5990-df16-b10f-4f61-4f40bdcd5eb0@kernel.org> <28940ce1-aba0-6fcc-7dcb-8d6f4bc34ea1@gmail.com> <1c639b12-e963-f5b4-9136-83ebe0cd1a9b@denx.de> <60997006-4808-2931-0612-5a32a0db601c@gmail.com> <8569b306-2984-11e3-1d10-7e906749b5b2@gmail.com> <0984c8c9-f80a-8eef-4bbd-a5723fd0d013@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 1/14/19 9:30 PM, Simon Goldschmidt wrote: > On Mon, Jan 14, 2019 at 9:23 PM Marek Vasut wrote: >> >> On 1/14/19 9:12 PM, Simon Goldschmidt wrote: >>> Am 14.01.2019 um 21:01 schrieb Marek Vasut: >>>> On 1/14/19 8:43 PM, Simon Goldschmidt wrote: >>>>> Am 14.01.2019 um 20:33 schrieb Marek Vasut: >>>>>> On 1/14/19 7:58 PM, Simon Goldschmidt wrote: >>>>>>> Am 14.01.2019 um 19:31 schrieb Marek Vasut: >>>>>>>> On 1/14/19 5:05 PM, Simon Goldschmidt wrote: >>>>>>>>> Hi Dinh, >>>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>>> Am 14.01.2019 um 16:58 schrieb Dinh Nguyen: >>>>>>>>>> Hi Simon, >>>>>>>>>> >>>>>>>>>> On 1/14/19 9:50 AM, Simon Goldschmidt wrote: >>>>>>>>>>> Am 11.01.2019 um 23:02 schrieb Marek Vasut: >>>>>>>>>>>> On 1/11/19 9:39 PM, Simon Goldschmidt wrote: >>>>>>>>>>>>> Am 07.01.2019 um 23:53 schrieb Marek Vasut: >>>>>>>>>>>>>> On 1/7/19 10:14 PM, Simon Goldschmidt wrote: >>>>>>>>>>>>>>> In order to build a smaller SPL, let's imply SPL_DM_RESET and >>>>>>>>>>>>>>> SPL_WATCHDOG_SUPPORT instead of selecting them, so they can be >>>>>>>>>>>>>>> disabled >>>>>>>>>>>>>>> via defconfig. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> This also seems to be required to use OF_PLATDATA, as the >>>>>>>>>>>>>>> reset >>>>>>>>>>>>>>> drivers >>>>>>>>>>>>>>> don't seem to work with it. >>>>>>>>>>>>>> >>>>>>>>>>>>>> How do you un-reset IP blocks if you disable the reset >>>>>>>>>>>>>> controller ? >>>>>>>>>>>>> >>>>>>>>>>>>> I found that out just now: there's the function >>>>>>>>>>>>> 'reset_deassert_peripherals_handoff()' in spl_gen5.c that should >>>>>>>>>>>>> "De-assert reset for peripherals and bridges based on handoff". >>>>>>>>>>>>> However, >>>>>>>>>>>>> at least for Gen5, it just writes a 0 to rstmgr->permodrst. By >>>>>>>>>>>>> doing >>>>>>>>>>>>> that, it enables *ALL* peripherals on the SoC (except for >>>>>>>>>>>>> some DMA >>>>>>>>>>>>> channels that aren't really used) :-) >>>>>>>>>>>>> >>>>>>>>>>>>> I guess that needs some cleaning up as well ;-) >>>>>>>>>>>> >>>>>>>>>>>> Yes >>>>>>>>>>>> >>>>>>>>>>>>> I think the proper thing to do here would be to remove this >>>>>>>>>>>>> function and >>>>>>>>>>>>> convert all drivers to provide appropriate 'resets' >>>>>>>>>>>>> properties in >>>>>>>>>>>>> the >>>>>>>>>>>>> dts? >>>>>>>>>>>> >>>>>>>>>>>> Indeed >>>>>>>>>>> >>>>>>>>>>> So I just did that and it works nice for SPL and U-Boot: By adding >>>>>>>>>>> some >>>>>>>>>>> "resets" properties the the main dtsi and adding reset bulk >>>>>>>>>>> code to >>>>>>>>>>> the >>>>>>>>>>> cadence_qspi, denali_dt nand and drivers, I can nearly remove the >>>>>>>>>>> reset >>>>>>>>>>> code from arch/mach_socfpga. >>>>>>>>>>> >>>>>>>>>>> The problem would be that now Linux cannot use peripherals that >>>>>>>>>>> aren't >>>>>>>>>>> enabled by U-Boot because it relies on them being enabled. How are >>>>>>>>>>> such >>>>>>>>>>> dependencies solved? Because even if I would add reset support in >>>>>>>>>>> the >>>>>>>>>>> corresponding Linux drivers, we probably could not bootolder >>>>>>>>>>> Kernels >>>>>>>>>>> (e.g. the Debian 9 kernel - v4.9.x) with a new U-Boot... >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> I added an early reset driver for SoCFPGA that should take care of >>>>>>>>>> this. >>>>>>>>>> The patch is in v5.0-rc2[1]. >>>>>>>>> >>>>>>>>> OK, it's good to know that this work is already done, I haven't >>>>>>>>> monitored this close enough. >>>>>>>> >>>>>>>> We had the same problem with A10, indeed. >>>>>>>> >>>>>>>>> But am I correct that my above problem remains even in v5.0 as >>>>>>>>> not all >>>>>>>>> peripherals in socfpga.dtsi have a "resets" property set (e.g. >>>>>>>>> mmc and >>>>>>>>> qspi) and would thuse not be taken out of reset by Linux? >>>>>>>>> >>>>>>>>> Plus: should U-Boot work with older Linux kernels? Because if so, we >>>>>>>>> need fallback code in U-Boot to unreset peripherals when running >>>>>>>>> with an >>>>>>>>> older kernel... >>>>>>>> >>>>>>>> Yes, it'd break old broken kernels . The real question is, do we >>>>>>>> care ? >>>>>>> >>>>>>> Ok, so that at leat shows me I'm going into the right direction :-) >>>>>>> >>>>>>> There are some problems though: >>>>>>> - I do care (we're running 4.9 currently) *g* >>>>>>> - people running an RT kernel will care for a while (until the next >>>>>>> stable RT after fixing this will be released) >>>>>>> - we would currently be breaking *all* kernels, since no kernel should >>>>>>> yet be able to deassert reset for mmc and qspi (unless this is already >>>>>>> done by U-Boot)... >>>>>>> >>>>>>> So would it be OK to add a Kconfig option to U-Boot to keep the >>>>>>> current >>>>>>> behaviour (for old broken kernels like you said) until that code is >>>>>>> spread widely enough? Or is that a no-go? >>>>>> >>>>>> Would be nice to be able to tweak the reset driver behavior at runtime, >>>>>> to unreset things before booting the kernel if the user desires so. >>>>> >>>>> Instead of tweaking the reset driver, we could just add a command that >>>>> does that 'rstmgr->permodrst = 0;' thing my patch would remove. >>>> >>>> I don't want a new custom command. >>>> >>>>> Since noone has complained so far, I think writing 0 should be OK here. >>>>> I don't think it would make too much sense to use the reset handoff >>>>> defines from Quartus output for such a command. I think the way Quartus >>>>> does this is strange anyway... >>>>> >>>>> The question is if defconfigs should be able to use this to >>>>> automatically build a U-Boot config for older kernels. If so, we'd still >>>>> need a Kconfig option? >>>> >>>> I'd much rather have this runtime configurable. >>> >>> Then I'm afraid I don't know what you mean by "runtime configurable". >>> What should be the configuration source that is evaluated at runtime? >> >> I wonder ... maybe an environment variable with a U_BOOT_ENV_CALLBACK() >> hook ? > > But when doing it like that, I'd still have to modify the U-Boot > sources to build a version of U-Boot that could successfully boot a > 4.9 kernel. > I wanted to be able to do that without changing the sources, that's > why I would have added a Kconfig option... If you have an environment variable which allows you to indicate that your kernel is broken/old, you can set it and boot. >>>>> Thinking further about cleanup: I guess the clock driver is not that >>>>> hard to implement, either. The only thing that's driving me mad is >>>>> pinmux. Is there any chance to get more info from Intel to write this >>>>> properly so we can get rid of that iocsr scanchain defines? >>>> >>>> Clock driver should be easy, yes. Pinmux, I don't know, maybe project >>>> chibi has some information (the cyclone I documentation project). >>> >>> Interesing, I didn't know that project. The only thing I found is a repo >>> on github. But it seems like that one only contains FPGA-related stuff, >>> nothing about the HPS side... >> >> My understanding is that the IOCSR ring is on the FPGA side and it's >> exactly the same as pinmux configuration for the FPGA IO, but I might be >> wrong. > > Hmm, I really don't know. Has anyone ever tried asking Intel for this? > Is it a known fact that they won't give away such specs? Dinh is on CC :) -- Best regards, Marek Vasut