[PATCH 00/21] Qualcomm generic board support

[Caleb Connolly]

On 06/12/2023 10:44, Ilias Apalodimas wrote:
> Hi Tom,
> 
> On Wed, 22 Nov 2023 at 16:28, Tom Rini <trini at konsulko.com> wrote:
>>
>> On Wed, Nov 22, 2023 at 07:44:09PM +0530, Sumit Garg wrote:
>>> On Wed, 22 Nov 2023 at 19:31, Tom Rini <trini at konsulko.com> wrote:
>>>>
>>>> On Wed, Nov 22, 2023 at 11:51:29AM +0530, Sumit Garg wrote:
>>>>> Hi Caleb,
>>>>>
>>>>> On Tue, 21 Nov 2023 at 22:39, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>>>> [snip]
>>>>>> == DT loading ==
>>>>>>
>>>>>> Previously, boards used the FDT blob embedded into U-Boot (via
>>>>>> OF_SEPARATE). However, most Qualcomm boards run U-Boot as a secondary
>>>>>> bootloader, so we can instead rely on the first-stage bootloader to
>>>>>> populate some useful FDT properties for us (notably the /memory node and
>>>>>> KASLR seed) and fetch the DTB that it provides. Combined with the memory
>>>>>> map changes above, this let's us entirely avoid configuring the memory
>>>>>> map explicitly.
>>>>>
>>>>> Since with this change, we don't need to embed FDT blob in the u-boot
>>>>> binary, so I was thinking if we really need to import DTs from Linux
>>>>> for different platforms and then play a catchup game?
>>>>>
>>>>> IMO, the build command would look like following if we import
>>>>> pre-built FDT blob from Linux:
>>>>>
>>>>> - Build u-boot::
>>>>>
>>>>>        $ export CROSS_COMPILE=<aarch64 toolchain prefix>
>>>>>        $ make qcom_defconfig
>>>>>        $ make
>>>>>
>>>>> - gzip u-boot::
>>>>>
>>>>>        gzip u-boot-nodtb.bin
>>>>>
>>>>> - Append dtb to gzipped u-boot::
>>>>>
>>>>>         cat u-boot-nodtb.bin.gz
>>>>> <linux-tree>/arch/arm64/boot/dts/qcom/your-board.dtb >
>>>>> u-boot-nodtb.bin.gz-dtb
>>>>>
>>>>> This would avoid the maintenance burden to keep DT in sync with that
>>>>> of Linux. And since DT bindings in Linux are backwards compatible, we
>>>>> can say u-boot should work with DTB picked up from any Linux kernel
>>>>> stable release.
>>>>
>>>> I guess one question I have is, are we being passed the device tree
>>>> (since we're acting like the Linux Kernel)
>>>
>>> Yeah that is the case here, see patch #1 in this series regarding how
>>> FDT address is being retrieved from previous stage bootloader (ABL on
>>> sdm845 and qcs404 SoCs).
>>
>> That's what I thought.
>>
>>>> or knowing that we have the
>>>> dtb attached to the end of us and making use of the old kernel appended
>>>> dtb option? We're fine in for example the rpi_arm64 case of just being
>>>> given a device tree from the previous stage and not needing one in-tree.
>>>
>>> That's good to know and we can replicate that for Qcom platforms which
>>> are chainloaded and don't need an embedded DT.
>>
>> So yes, moving these towards the direction of rpi_arm64 and specifically
>> using imply OF_HAS_PRIOR_STAGE or select OF_HAS_PRIOR_STAGE (force that
>> the dtb must be provided to us) sounds like the right direction to take
>> these platforms.

This is implicitly enforced by the bootloader on Qualcomm platforms. It
will only even boot U-Boot if we gzip u-boot-nodtb.bin, then append some
valid DTB to it, and then shove it in an Android boot image. If there is
no DTB then the bootloader won't run U-Boot. If we somehow did run
U-Boot without a valid DTB then we wouldn't be able to print anything
anyway (unless debug UART is enabled).


>>
> 
> IMHO that option isn't that useful.  Grepping for OF_HAS_PRIOR_STAGE
> shows that we use it to print where the DT came from unless I am
> missing something...
> On top of that having implies in the Kconfig to control those prints
> makes little sense to me and it's a half-baked solution anyway,
> because may boards don't fill this properly.  This thing was cleaned
> up in 2e8d2f88439d, 2ea63271e522f, and d6f8ab30a2af46 but then got
> reintroduced in 275b4832f6bf91 without anyone acking or reviewing. I
> am fine with Caleb suggests here, but I think we can do way better.
> 
> Instead of having that imply we can get rid of it and only have
> OF_BOARD(and perhaps rename it). So a general cleanup I have in mind
> is
> 
> CONFIG_OF_SEPARATE -> rename it to CONFIG_OF_APPEND because it is
> actually appended at the end of the binary.
> CONFIG_OF_EMBED -> Leave it as is, it's there for debugging mostly.
> CONFIG_OF_BOARD perhaps rename it to something more useful, but the
> semantics are 'The DT comes from an *external* source. A bloblist,
> some EEPROM location, CPU registers etc'
> CONFIG_OF_HAS_PRIOR_STAGE -> Get rid of it again. gd already has
> gd->fdt_src, just add a FDTSRC_UNKNOWN as default and delegate the
> responsibility of filling this properly at fdtdec_setup(). The boards
> that use OF_BOARD can fill this in properly, instead of relying on
> Kconfig imply options and we can even be more explicit on where the DT
> came from.

This sounds good to me, although I guess I'm mostly unaffected anyways.
> 
> I can send an RFC if anyone thinks this is a cleaner approach>
> Thanks
> /Ilias
> 
> 
> 
>> --
>> Tom

-- 
// Caleb (they/them)