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

[Marek Vasut]

On 1/8/19 9:52 PM, Simon Goldschmidt wrote:
> Am 08.01.2019 um 16:01 schrieb Simon Goldschmidt:
>> Am 08.01.2019 um 15:50 schrieb Marek Vasut:
>>> On 1/8/19 3:48 PM, Tom Rini wrote:
>>>> On Tue, Jan 08, 2019 at 01:49:14PM +0100, Marek Vasut wrote:
>>>>> On 1/8/19 1:46 PM, Simon Goldschmidt wrote:
>>>>>> On Tue, Jan 8, 2019 at 1:22 PM Marek Vasut <marex at denx.de> wrote:
>>>>>>>
>>>>>>> On 1/8/19 1:09 PM, Simon Goldschmidt wrote:
>>>>>>>> On Tue, Jan 8, 2019 at 1:05 PM Marek Vasut <marex at denx.de> wrote:
>>>>>>>>>
>>>>>>>>> On 1/8/19 7:24 AM, Simon Goldschmidt wrote:
>>>>>>>>>> On Mon, Jan 7, 2019 at 11:58 PM Marek Vasut <marex at denx.de>
>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> 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 ?
>>>>>>>>>>
>>>>>>>>>> Here again, socfpga seems to be another bad example. Taking
>>>>>>>>>> peripherals out of reset
>>>>>>>>>> is cluttered throughout the mach-socfpga code at least in SPL.
>>>>>>>>>> By now
>>>>>>>>>> I know socfpga is
>>>>>>>>>> lacking support for clock and reset management via devicetree.
>>>>>>>>>> And
>>>>>>>>>> this is bad, I know,
>>>>>>>>>> but can we keep this a seperate issue from OF_PLATDATA?
>>>>>>>>>>
>>>>>>>>>> That being said, drivers/reset/reset-uclass.c fails to compile
>>>>>>>>>> with
>>>>>>>>>> OF_PLATDATA, so I
>>>>>>>>>> guess this has not been used with OF_PLATDATA before. And
>>>>>>>>>> given that I
>>>>>>>>>> don't seem
>>>>>>>>>> to need it for socfpga either, I don't think this would be the
>>>>>>>>>> right
>>>>>>>>>> series to fix that.
>>>>>>>>>
>>>>>>>>> Don't you need it to unreset at least the DWMMC or CQSPI ?
>>>>>>>>
>>>>>>>> Reading the code, it seems like that's taken care of through
>>>>>>>> another hack in
>>>>>>>> spl_boot_device() ;-)
>>>>>>>
>>>>>>> Sigh.
>>>>>>>
>>>>>>>>> Anyway, I'd much prefer to start cleaning up the horrorshow that
>>>>>>>>> arch/arm/mach-socfpga is in terms of clock and reset, at least
>>>>>>>>> like A10.
>>>>>>>>> Would that be possible ?
>>>>>>>>
>>>>>>>> I would be best, yes. I don't know when I will find the time to
>>>>>>>> do that, though.
>>>>>>>> I don't know how much effort that would be, either. Is there
>>>>>>>> maybe a patch
>>>>>>>> where A10 got converted from "as bad as gen5" to its current
>>>>>>>> state? That
>>>>>>>> would help me to see if I can do it...
>>>>>>>
>>>>>>> A10 got switched to reset framework recently (in last 6 months or
>>>>>>> so),
>>>>>>> the reset driver is the same for Gen5 and A10 too, so it should
>>>>>>> be easy
>>>>>>> to recycle.
>>>>>>
>>>>>> Hmm, ok, let me check that... it would indeed be nice to port this
>>>>>> to gen5.
>>>>>>
>>>>>> Since you seem kidn of opposed to OF_PLATDATA, does it make any sense
>>>>>> to continue on this? I mean, I thought I heard people here saying
>>>>>> "use
>>>>>> OF_PLATDATA" if you're running out of space in SPL. After using
>>>>>> it, I'm not too
>>>>>> keen on using it, either, but it does seem to give me some code
>>>>>> space back...
>>>>>
>>>>> OF_PLATDATA is for platforms with really small SRAM, some 30k or
>>>>> below.
>>>>> This platform has a massive 60k of SRAM for SPL, so if we're
>>>>> running out
>>>>> of space, we're doing something wrong.
>>>>
>>>> It's not for "30k or below" but "needs more space to enable all desired
>>>> features inside of SPL".
>>>
>>> Which the SoCFPGA should have with 60k of SRAM. If U-Boot SPL became so
>>> bloated that even platform with so much space has issues, how can we
>>> even cater for the rest of platforms with much more limited SPL ? And if
>>> that is the case, we have a much bigger problem ...
>>
>> I have to add here: I'm not running out of space in SPL just yet. I'm at
>> 45k But I have tried and the remaining 15k don't seem to be enough to
>> add verified FIT loading code. I expect this to be something very rarely
>> used, if it is used at all (I saw presentations doing this to implement
>> verified boot, but I failed to find the code in SPL that does this)...
>>
>> I don't know yet how I'll continue here. I'll check again exactly how
>> much of the SRAM my current OF_PLATDATA patches save.
> 
> Ok, so when booting from mmc (with the tiny mmc framework), this series
> saves ~5.6 KiB of the SRAM for SPL (~3.7 KiB .text reduction, ~1.7 KiB
> for the DTB binary, other sections are slightly smaller).
> 
> It's not the world, but it's more than 10% (from ~40 KiB to 34.5 KiB).
> I don't insist on doing this, but I wanted to write down numbers in case
> someone wants to discuss this further.

I'm sure if you went all the way and removed even the DT support
altogether, the SPL would be even smaller too.

-- 
Best regards,
Marek Vasut