[U-Boot] [PATCH v1 1/4] arm: socfpga: imply SPL config instead of select

[Marek Vasut]

On 1/14/19 10:53 PM, Simon Goldschmidt wrote:
> 
> 
> Am Mo., 14. Jan. 2019, 22:50 hat Tom Rini <trini at konsulko.com
> <mailto:trini at konsulko.com>> geschrieben:
> 
>     On Mon, Jan 14, 2019 at 10:30:36PM +0100, Marek Vasut wrote:
>     > On 1/14/19 10:28 PM, Tom Rini wrote:
>     > > On Mon, Jan 14, 2019 at 07:31:26PM +0100, Marek Vasut wrote:
>     > >> 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 ?
>     > >
>     > > Yes, we care.  Especially since it sounds like we're talking about
>     > > something that's an LTS and not super-ancient vendor kernel. 
>     Off the
>     > > top of my head I can't recall if we ever fully removed support
>     in sunxi
>     > > for the vendor kernel in some cases, or just made it,
>     eventually, opt-in
>     > > as it was a fairly annoying incompatible behavior case.
>     > >
>     > > But yes, in general, we do care about old kernels and need to be
>     loud
>     > > and clear about when we're removing support for them on a given
>     SoC due
>     > > to it being a PITA to support both ways of doing X and people
>     have had Y
>     > > years to migrate or correct their kernel.
>     >
>     > Then we have to add some fallback mechanism, possibly the env variable
>     > to tell reset controller to unreset everything.
> 
>     Alright, so the prior art in question is:
>     commit accc9e446be6c3bd129315a0c5830bddccfa16da
>     Author: Hans de Goede <hdegoede at redhat.com <mailto:hdegoede at redhat.com>>
>     Date:   Wed Oct 22 14:56:36 2014 +0200
> 
>         sunxi: Add CONFIG_OLD_SUNXI_KERNEL_COMPAT Kconfig option
> 
>     And is about supporting the sunxi 3.4 vendor kernel tree, and something
>     we have still (and a single enabler of).  So, what's the range of broken
>     / not broken kernels here?
> 
> 
> I haven't yet tested but my idea is that it is still broken (some of the
> devices won't come out of reset in some scenarios).
> 
> So I guess such a Kconfig option would even need to be enabled by
> default for some time...

Well, if you had a variable to configure that at runtime, you won't need
another ad-hoc Kconfig option .

-- 
Best regards,
Marek Vasut