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

[Simon Goldschmidt]

Am 14.01.2019 um 22:30 schrieb Marek Vasut:
> 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.

So is an env variable good enough as opt-in or opt-out? How was the 
sunxi thing handled? My feeling is that it would be better to have a 
Kconfig option for such things, to make it just boot after compiling it, 
without relying on the manual act of changing the environment.

But I'm just some user, I'll leave it up to you two to decide ;-)

Regards,
Simon