On Mon, Jan 14, 2019 at 9:23 PM Marek Vasut <ma...@denx.de> 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... > > >>> 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? Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
