[PATCH v1 1/1] rockchip: rock5b-rk3588: Enable automatic PCI enumeration

[Jonas Karlman]

On 2024-10-31 22:52, Jonas Karlman wrote:
> Hi Sebastian,
> 
> On 2024-10-31 22:16, Sebastian Reichel wrote:
>> Hi,
>>
>> On Thu, Oct 31, 2024 at 08:17:32PM +0100, Jonas Karlman wrote:
>>> On 2024-10-31 19:15, Eugen Hristev wrote:
>>>> On 10/31/24 20:01, Sebastian Reichel wrote:
>>>>> PCI enumeration is required to detect the onboard Ethernet device.
>>>>>
>>>>> Signed-off-by: Sebastian Reichel <sebastian.reichel at collabora.com>
>>>>> ---
>>>>>   configs/rock5b-rk3588_defconfig | 1 +
>>>>>   1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig
>>>>> index c54e13e8732c..e5b9f7d326b4 100644
>>>>> --- a/configs/rock5b-rk3588_defconfig
>>>>> +++ b/configs/rock5b-rk3588_defconfig
>>>>> @@ -17,6 +17,7 @@ CONFIG_DEBUG_UART_CLOCK=24000000
>>>>>   CONFIG_SPL_SPI_FLASH_SUPPORT=y
>>>>>   CONFIG_SPL_SPI=y
>>>>>   CONFIG_PCI=y
>>>>> +CONFIG_PCI_INIT_R=y
>>>>>   CONFIG_DEBUG_UART=y
>>>>>   CONFIG_AHCI=y
>>>>>   CONFIG_FIT=y
>>>>
>>>> This has been added and removed before, please see
>>>>
>>>> https://lore.kernel.org/all/20231104130451.111020-1-martin.roukala@mupuf.org/T/
>>>>
>>>> this commit adds it
>>>> 5b269ed404dc ("configs: rockchip: rock5b-rk3588: Enable CONFIG_PCI_INIT_R")
>>>>
>>>> this commit removes it again
>>>> 191ece249a96 ("rockchip: rk3588-rock-5b: Enable support for PCIe SATA 
>>>> cards")
>>
>> Ah, I should have checked the history. Sorry.
>>
>>> What use-case are you trying to solve with a forced pci enumeration?
>>>
>>> With standard boot Ethernet should be enabled when it is needed.
>>>
>>> However for EFI boot Ethernet maybe is not detected? Please see [1] for
>>> a possible alternative patch that I created when someone reported an
>>> issue with iPXE boot some time ago.
>>>
>>> I did not get confirmation it fully solved the issue, from what I could
>>> test it should help get Ethernet detected during EFI boot on a ROCK 5B.
>>>
>>> [1] https://github.com/Kwiboo/u-boot-rockchip/commit/90b6d787de0b3e6ed863440be576b01d83ba5789
>>
>> LAVA by default has a U-Boot script doing the following (IIUIC across all
>> boards), which fails with the current setup:
> 
> If you run a boot script you can easily add 'pci enum' before 'dhcp' and
> Ethernet will work for you LAVA script.

Look like something like following in device-types/rk3588-rock-5b.jinja2
could work for LAVA:

{% set uboot_ipaddr_cmd = 'pci enum; dhcp' %}

Regards,
Jonas

> 
>>
>> setenv autoload no
>> setenv initrd_high 0xffffffff
>> setenv fdt_high 0xffffffff
>> dhcp
>> setenv serverip 192.168.201.1
>> tftp 0x0400000 16212093/tftp-deploy-or30pzbe/kernel/vmlinuz
>> tftp 0xa200000 16212093/tftp-deploy-or30pzbe/ramdisk/ramdisk.cpio.gz.uboot
>> setenv initrd_size ${filesize}
>> tftp 0xa100000 16212093/tftp-deploy-or30pzbe/dtb/rk3588-rock-5b.dtb
>> setenv bootargs 'console=ttyS2,1500000n8 root=/dev/nfs rw nfsroot=192.168.201.1:/var/lib/lava/dispatcher/tmp/16212093/extract-nfsrootfs-e0aw1qlf,tcp,hard,v3 rootwait ip=dhcp'
>> booti 0x0400000 0xa200000 0xa100000
>>
>> Basically it comes down to missing network support when breaking
>> into the U-Boot shell. Also while it is obvious one needs to
>> enumerate the PCI bus for U-Boot developers with knowledge about
>> the Rock 5B, it's unexpected for people with less knowledge. So I
>> think it's sensible to have a default configuration supporting
>> network in the U-Boot shell without extra steps.
> 
> Unfortunately forcing a pci enumeration slow down booting from faster
> boot media, adding at least one second pci timeout delay and an error
> being logged for each unoccupied M.2 slot.
> 
> For normal use, standard boot automatically enumerate pci and Ethernet
> once needed, if that is not working then that is an issue to be solved.
> 
> PCI_INIT_R was rejected earlier because it forces unnecessary slower
> boot times on any user not booting from Ethernet.
> 
> With use of a boot script, the script should be capable/responsible to
> load any resources that is needed for its custom boot flow.
> 
> Regards,
> Jonas
> 
>>
>> -- Sebastian
>