[PATCH v2 06/10] arm: mach-k3: am625_init: Probe AM65 CPSW NUSS

[Chintan Vankar]

On 20/05/24 17:42, Roger Quadros wrote:
> 
> 
> On 25/04/2024 15:59, Chintan Vankar wrote:
>>
>>
>> On 25/04/24 17:57, Roger Quadros wrote:
>>>
>>>
>>> On 25/04/2024 15:08, Chintan Vankar wrote:
>>>> From: Kishon Vijay Abraham I <kishon at ti.com>
>>>>
>>>> In order to support Ethernet boot on AM62x, probe AM65 CPSW NUSS
>>>> driver in board_init_f().
>>>>
>>>> Signed-off-by: Kishon Vijay Abraham I <kishon at ti.com>
>>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli at ti.com>
>>>> Signed-off-by: Chintan Vankar <c-vankar at ti.com>
>>>> ---
>>>>
>>>> Link to v1:
>>>> https://lore.kernel.org/r/20240112064759.1801600-8-s-vadapalli@ti.com/
>>>>
>>>> Changes from v1 to v2:
>>>> - No changes.
>>>>
>>>>    arch/arm/mach-k3/am625_init.c | 10 ++++++++++
>>>>    1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
>>>> index 668f9a51ef..9bf61b1e83 100644
>>>> --- a/arch/arm/mach-k3/am625_init.c
>>>> +++ b/arch/arm/mach-k3/am625_init.c
>>>> @@ -277,6 +277,16 @@ void board_init_f(ulong dummy)
>>>>            if (ret)
>>>>                panic("DRAM init failed: %d\n", ret);
>>>>        }
>>>> +
>>>> +    if (IS_ENABLED(CONFIG_SPL_ETH) && IS_ENABLED(CONFIG_TI_AM65_CPSW_NUSS) &&
>>>> +        spl_boot_device() == BOOT_DEVICE_ETHERNET) {
>>>> +        struct udevice *cpswdev;
>>>> +
>>>> +        if (uclass_get_device_by_driver(UCLASS_MISC, DM_DRIVER_GET(am65_cpsw_nuss),
>>>> +                        &cpswdev))
>>>> +            printf("Failed to probe am65_cpsw_nuss driver\n");
>>>> +    }
>>>> +
>>>
>>> This looks like a HACK.
>>> The network driver should be probed only when the networking feature is actually required.
>>>
>>
>> Driver is probed only when the condition above
>> "spl_boot_device() == BOOT_DEVICE_ETHERNET" is true, which says Boot
>> device is Ethernet, and here we are booting with Ethernet so driver is
>> getting probed.
> 
> I think you misunderstood what I was saying as am625_init.c is not using
> any Ethernet function it should not cause the Ethernet driver to probe.
> 
> Instead it should be probed by the networking use case.
> 
> What happens if you don't use this patch? Where is it failing?
> 

You are correct that it should be probed by the networking use case and
we are using networking functionality of "Booting via Ethernet".
Regarding its probing, I already had discussion that why do I have to
probe it explicitly at here:
https://lore.kernel.org/r/ee1d16fd-b99b-4b07-97bb-a896e179157a@ti.com/

Now if I don't use this patch then Ethernet will not get initialized at
SPL stage, and in "spl_net_load_image()" function, there will be an
error saying "No Ethernet Found", and since we selected booting via
Ethernet it will fail to boot.

>>
>>>>        spl_enable_cache();
>>>>          fixup_a53_cpu_freq_by_speed_grade();
>>>
>