Re: [PATCH v2 06/10] arm: mach-k3: am625_init: Probe AM65 CPSW NUSS
On 21/05/2024 08:34, Chintan Vankar wrote:
>
>
> 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
>
> In order to support Ethernet boot on AM62x, probe AM65 CPSW NUSS
> driver in board_init_f().
>
> Signed-off-by: Kishon Vijay Abraham I
> Signed-off-by: Siddharth Vadapalli
> Signed-off-by: Chintan Vankar
> ---
>
> Link to v1:
> https://lore.kernel.org/r/20240112064759.1801600-8-s-vadapa...@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),
> + ))
> + 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-a896e1791...@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.
OK Now I see the problem.
in am65-cpsw-nuss.c the following is defined:
> U_BOOT_DRIVER(am65_cpsw_nuss) = {
> .name = "am65_cpsw_nuss",
> .id = UCLASS_MISC,
> .of_match = am65_cpsw_nuss_ids,
> .probe = am65_cpsw_probe_nuss,
> .priv_auto = sizeof(struct am65_cpsw_common),
> };
>
> U_BOOT_DRIVER(am65_cpsw_nuss_port) = {
> .name = "am65_cpsw_nuss_port",
> .id = UCLASS_ETH,
> .probe = am65_cpsw_port_probe,
> .ops= _cpsw_ops,
> .priv_auto = sizeof(struct am65_cpsw_priv),
> .plat_auto = sizeof(struct eth_pdata),
> .flags = DM_FLAG_ALLOC_PRIV_DMA | DM_FLAG_OS_PREPARE,
> };
Now I suppose spl_net_load_image() tries to probe all UCLASS_ETH devices
which should call am65_cpsw_port_probe() but it fails to probe
am65_cpsw_probe_nuss().
This limitation was introduced by commit
38922b1f4acc ("net: ti: am65-cpsw: Add support for multi port independent MAC
mode")
which says "Since top level driver is now UCLASS_MISC, board
files would need to instantiate this driver explicitly.
I wonder if there can be a better solution to this?
--
cheers,
-roger
Re: [PATCH v2 06/10] arm: mach-k3: am625_init: Probe AM65 CPSW NUSS
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
In order to support Ethernet boot on AM62x, probe AM65 CPSW NUSS
driver in board_init_f().
Signed-off-by: Kishon Vijay Abraham I
Signed-off-by: Siddharth Vadapalli
Signed-off-by: Chintan Vankar
---
Link to v1:
https://lore.kernel.org/r/20240112064759.1801600-8-s-vadapa...@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),
+ ))
+ 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-a896e1791...@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();
Re: [PATCH v2 06/10] arm: mach-k3: am625_init: Probe AM65 CPSW NUSS
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
>>>
>>> In order to support Ethernet boot on AM62x, probe AM65 CPSW NUSS
>>> driver in board_init_f().
>>>
>>> Signed-off-by: Kishon Vijay Abraham I
>>> Signed-off-by: Siddharth Vadapalli
>>> Signed-off-by: Chintan Vankar
>>> ---
>>>
>>> Link to v1:
>>> https://lore.kernel.org/r/20240112064759.1801600-8-s-vadapa...@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),
>>> + ))
>>> + 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?
>
>>> spl_enable_cache();
>>> fixup_a53_cpu_freq_by_speed_grade();
>>
--
cheers,
-roger
Re: [PATCH v2 06/10] arm: mach-k3: am625_init: Probe AM65 CPSW NUSS
On 25/04/24 17:57, Roger Quadros wrote:
On 25/04/2024 15:08, Chintan Vankar wrote:
From: Kishon Vijay Abraham I
In order to support Ethernet boot on AM62x, probe AM65 CPSW NUSS
driver in board_init_f().
Signed-off-by: Kishon Vijay Abraham I
Signed-off-by: Siddharth Vadapalli
Signed-off-by: Chintan Vankar
---
Link to v1:
https://lore.kernel.org/r/20240112064759.1801600-8-s-vadapa...@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),
+ ))
+ 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.
spl_enable_cache();
fixup_a53_cpu_freq_by_speed_grade();
Re: [PATCH v2 06/10] arm: mach-k3: am625_init: Probe AM65 CPSW NUSS
On 25/04/2024 15:08, Chintan Vankar wrote:
> From: Kishon Vijay Abraham I
>
> In order to support Ethernet boot on AM62x, probe AM65 CPSW NUSS
> driver in board_init_f().
>
> Signed-off-by: Kishon Vijay Abraham I
> Signed-off-by: Siddharth Vadapalli
> Signed-off-by: Chintan Vankar
> ---
>
> Link to v1:
> https://lore.kernel.org/r/20240112064759.1801600-8-s-vadapa...@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),
> + ))
> + 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.
> spl_enable_cache();
>
> fixup_a53_cpu_freq_by_speed_grade();
--
cheers,
-roger
[PATCH v2 06/10] arm: mach-k3: am625_init: Probe AM65 CPSW NUSS
From: Kishon Vijay Abraham I
In order to support Ethernet boot on AM62x, probe AM65 CPSW NUSS
driver in board_init_f().
Signed-off-by: Kishon Vijay Abraham I
Signed-off-by: Siddharth Vadapalli
Signed-off-by: Chintan Vankar
---
Link to v1:
https://lore.kernel.org/r/20240112064759.1801600-8-s-vadapa...@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),
+ ))
+ printf("Failed to probe am65_cpsw_nuss driver\n");
+ }
+
spl_enable_cache();
fixup_a53_cpu_freq_by_speed_grade();
--
2.34.1
