[PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Chaoyi Chen]

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

For external phy, clk_phy should be optional, and some external phy
need the clock input from clk_phy. This patch adds support for setting
clk_phy for external phy.

Signed-off-by: David Wu <david.wu@rock-chips.com>
Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
---

Changes in v3:
- Link to V2: https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
- Rebase to net-next/main

Changes in v2:
- Link to V1: https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
- Remove get clock frequency from DT prop

 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index ac8288301994..5d921e62c2f5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct plat_stmmacenet_data *plat)
 		clk_set_rate(plat->stmmac_clk, 50000000);
 	}
 
-	if (plat->phy_node && bsp_priv->integrated_phy) {
+	if (plat->phy_node) {
 		bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
 		ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
-		if (ret)
-			return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
-		clk_set_rate(bsp_priv->clk_phy, 50000000);
+		/* If it is not integrated_phy, clk_phy is optional */
+		if (bsp_priv->integrated_phy) {
+			if (ret)
+				return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
+			clk_set_rate(bsp_priv->clk_phy, 50000000);
+		}
 	}
 
 	return 0;
-- 
2.49.0

[PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Chaoyi Chen]

From: Chaoyi Chen <chaoyi.chen@rock-chips.com>

For external phy, clk_phy should be optional, and some external phy
need the clock input from clk_phy. This patch adds support for setting
clk_phy for external phy.

Signed-off-by: David Wu <david.wu@rock-chips.com>
Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
---

Changes in v3:
- Link to V2: https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
- Rebase to net-next/main

Changes in v2:
- Link to V1: https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
- Remove get clock frequency from DT prop

 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index ac8288301994..5d921e62c2f5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct plat_stmmacenet_data *plat)
 		clk_set_rate(plat->stmmac_clk, 50000000);
 	}
 
-	if (plat->phy_node && bsp_priv->integrated_phy) {
+	if (plat->phy_node) {
 		bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
 		ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
-		if (ret)
-			return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
-		clk_set_rate(bsp_priv->clk_phy, 50000000);
+		/* If it is not integrated_phy, clk_phy is optional */
+		if (bsp_priv->integrated_phy) {
+			if (ret)
+				return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
+			clk_set_rate(bsp_priv->clk_phy, 50000000);
+		}
 	}
 
 	return 0;
-- 
2.49.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri, 15 Aug 2025 10:35:15 +0800 you wrote:
> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> 
> For external phy, clk_phy should be optional, and some external phy
> need the clock input from clk_phy. This patch adds support for setting
> clk_phy for external phy.
> 
> Signed-off-by: David Wu <david.wu@rock-chips.com>
> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> 
> [...]

Here is the summary with links:
  - [net-next,v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy
    https://git.kernel.org/netdev/net-next/c/da114122b831

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri, 15 Aug 2025 10:35:15 +0800 you wrote:
> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> 
> For external phy, clk_phy should be optional, and some external phy
> need the clock input from clk_phy. This patch adds support for setting
> clk_phy for external phy.
> 
> Signed-off-by: David Wu <david.wu@rock-chips.com>
> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> 
> [...]

Here is the summary with links:
  - [net-next,v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy
    https://git.kernel.org/netdev/net-next/c/da114122b831

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Marek Szyprowski]

On 15.08.2025 04:35, Chaoyi Chen wrote:
> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>
> For external phy, clk_phy should be optional, and some external phy
> need the clock input from clk_phy. This patch adds support for setting
> clk_phy for external phy.
>
> Signed-off-by: David Wu <david.wu@rock-chips.com>
> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> ---
>
> Changes in v3:
> - Link to V2: https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
> - Rebase to net-next/main
>
> Changes in v2:
> - Link to V1: https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
> - Remove get clock frequency from DT prop
>
>   drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> index ac8288301994..5d921e62c2f5 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct plat_stmmacenet_data *plat)
>   		clk_set_rate(plat->stmmac_clk, 50000000);
>   	}
>   
> -	if (plat->phy_node && bsp_priv->integrated_phy) {
> +	if (plat->phy_node) {
>   		bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>   		ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
> -		if (ret)
> -			return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
> -		clk_set_rate(bsp_priv->clk_phy, 50000000);
> +		/* If it is not integrated_phy, clk_phy is optional */
> +		if (bsp_priv->integrated_phy) {
> +			if (ret)
> +				return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
> +			clk_set_rate(bsp_priv->clk_phy, 50000000);
> +		}
>   	}
>   
>   	return 0;

The above change lacks the following check ingmac_clk_enable(): if (!bsp_priv->integrated_phy) return 0;

Otherwise it blows on machines with integrated PHY, like Hardkernel's Odroid-M1 (RK3568 based):

rk_gmac-dwmac fe2a0000.ethernet: IRQ eth_lpi not found
rk_gmac-dwmac fe2a0000.ethernet: IRQ sfty not found
rk_gmac-dwmac fe2a0000.ethernet: clock input or output? (output).
rk_gmac-dwmac fe2a0000.ethernet: TX delay(0x4f).
rk_gmac-dwmac fe2a0000.ethernet: RX delay(0x2d).
rk_gmac-dwmac fe2a0000.ethernet: integrated PHY? (no).
Unable to handle kernel paging request at virtual address fffffffffffffffe
Mem abort info:
   ESR = 0x0000000096000006
   EC = 0x25: DABT (current EL), IL = 32 bits
   SET = 0, FnV = 0
   EA = 0, S1PTW = 0
   FSC = 0x06: level 2 translation fault
Data abort info:
   ISV = 0, ISS = 0x00000006, ISS2 = 0x00000000
   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
swapper pgtable: 4k pages, 48-bit VAs, pgdp=000000000249e000
[fffffffffffffffe] pgd=0000000000000000, p4d=000000000376f403, pud=0000000003770403, pmd=0000000000000000
Internal error: Oops: 0000000096000006 [#1]  SMP
Modules linked in: snd_soc_rockchip_i2s_tdm snd_soc_rk817 snd_soc_core snd_compress snd_pcm_dmaengine rockchip_thermal snd_pcm dwmac_rk(+) hantro_vpu rockchip_saradc industrialio_triggered_buffer kfifo_buf stmmac_platform rockchip_rga v4l2_vp9 stmmac spi_rockchip_sfc(+) v4l2_h264 snd_timer pcs_xpcs rockchipdrm(+) videobuf2_dma_sg v4l2_jpeg rk805_pwrkey v4l2_mem2mem videobuf2_dma_contig dw_hdmi_qp rockchip_dfi rk817_charger videobuf2_memops analogix_dp videobuf2_v4l2 dw_mipi_dsi panfrost snd rtc_rk808 drm_dp_aux_bus videodev drm_shmem_helper soundcore dw_hdmi videobuf2_common drm_display_helper mc gpu_sched ahci_dwc ipv6
CPU: 3 UID: 0 PID: 154 Comm: systemd-udevd Not tainted 6.17.0-rc1+ #10875 PREEMPT
Hardware name: Hardkernel ODROID-M1 (DT)
pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : clk_prepare+0x18/0x44
lr : gmac_clk_enable+0xf8/0x188 [dwmac_rk]
..
Call trace:
  clk_prepare+0x18/0x44 (P)
  gmac_clk_enable+0xf8/0x188 [dwmac_rk]
  rk_gmac_powerup+0x4c/0x1f0 [dwmac_rk]
  rk_gmac_probe+0x3b4/0x5c8 [dwmac_rk]
  platform_probe+0x5c/0xac
  really_probe+0xbc/0x298
  __driver_probe_device+0x78/0x12c
  driver_probe_device+0x40/0x164
  __driver_attach+0x9c/0x1ac
  bus_for_each_dev+0x74/0xd0
  driver_attach+0x24/0x30
  bus_add_driver+0xe4/0x208
  driver_register+0x60/0x128
  __platform_driver_register+0x24/0x30
  rk_gmac_dwmac_driver_init+0x20/0x1000 [dwmac_rk]
  do_one_initcall+0x64/0x308
  do_init_module+0x58/0x23c
  load_module+0x1b48/0x1dc4
  init_module_from_file+0x84/0xc4
  idempotent_init_module+0x188/0x280
  __arm64_sys_finit_module+0x68/0xac
  invoke_syscall+0x48/0x110
  el0_svc_common.constprop.0+0xc8/0xe8
  do_el0_svc+0x20/0x2c
  el0_svc+0x4c/0x160
  el0t_64_sync_handler+0xa0/0xe4
  el0t_64_sync+0x198/0x19c
Code: 910003fd f9000bf3 52800013 b40000e0 (f9400013)
---[ end trace 0000000000000000 ]---

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Marek Szyprowski]

On 15.08.2025 04:35, Chaoyi Chen wrote:
> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>
> For external phy, clk_phy should be optional, and some external phy
> need the clock input from clk_phy. This patch adds support for setting
> clk_phy for external phy.
>
> Signed-off-by: David Wu <david.wu@rock-chips.com>
> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> ---
>
> Changes in v3:
> - Link to V2: https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
> - Rebase to net-next/main
>
> Changes in v2:
> - Link to V1: https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
> - Remove get clock frequency from DT prop
>
>   drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> index ac8288301994..5d921e62c2f5 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct plat_stmmacenet_data *plat)
>   		clk_set_rate(plat->stmmac_clk, 50000000);
>   	}
>   
> -	if (plat->phy_node && bsp_priv->integrated_phy) {
> +	if (plat->phy_node) {
>   		bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>   		ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
> -		if (ret)
> -			return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
> -		clk_set_rate(bsp_priv->clk_phy, 50000000);
> +		/* If it is not integrated_phy, clk_phy is optional */
> +		if (bsp_priv->integrated_phy) {
> +			if (ret)
> +				return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
> +			clk_set_rate(bsp_priv->clk_phy, 50000000);
> +		}
>   	}
>   
>   	return 0;

The above change lacks the following check ingmac_clk_enable(): if (!bsp_priv->integrated_phy) return 0;

Otherwise it blows on machines with integrated PHY, like Hardkernel's Odroid-M1 (RK3568 based):

rk_gmac-dwmac fe2a0000.ethernet: IRQ eth_lpi not found
rk_gmac-dwmac fe2a0000.ethernet: IRQ sfty not found
rk_gmac-dwmac fe2a0000.ethernet: clock input or output? (output).
rk_gmac-dwmac fe2a0000.ethernet: TX delay(0x4f).
rk_gmac-dwmac fe2a0000.ethernet: RX delay(0x2d).
rk_gmac-dwmac fe2a0000.ethernet: integrated PHY? (no).
Unable to handle kernel paging request at virtual address fffffffffffffffe
Mem abort info:
   ESR = 0x0000000096000006
   EC = 0x25: DABT (current EL), IL = 32 bits
   SET = 0, FnV = 0
   EA = 0, S1PTW = 0
   FSC = 0x06: level 2 translation fault
Data abort info:
   ISV = 0, ISS = 0x00000006, ISS2 = 0x00000000
   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
swapper pgtable: 4k pages, 48-bit VAs, pgdp=000000000249e000
[fffffffffffffffe] pgd=0000000000000000, p4d=000000000376f403, pud=0000000003770403, pmd=0000000000000000
Internal error: Oops: 0000000096000006 [#1]  SMP
Modules linked in: snd_soc_rockchip_i2s_tdm snd_soc_rk817 snd_soc_core snd_compress snd_pcm_dmaengine rockchip_thermal snd_pcm dwmac_rk(+) hantro_vpu rockchip_saradc industrialio_triggered_buffer kfifo_buf stmmac_platform rockchip_rga v4l2_vp9 stmmac spi_rockchip_sfc(+) v4l2_h264 snd_timer pcs_xpcs rockchipdrm(+) videobuf2_dma_sg v4l2_jpeg rk805_pwrkey v4l2_mem2mem videobuf2_dma_contig dw_hdmi_qp rockchip_dfi rk817_charger videobuf2_memops analogix_dp videobuf2_v4l2 dw_mipi_dsi panfrost snd rtc_rk808 drm_dp_aux_bus videodev drm_shmem_helper soundcore dw_hdmi videobuf2_common drm_display_helper mc gpu_sched ahci_dwc ipv6
CPU: 3 UID: 0 PID: 154 Comm: systemd-udevd Not tainted 6.17.0-rc1+ #10875 PREEMPT
Hardware name: Hardkernel ODROID-M1 (DT)
pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : clk_prepare+0x18/0x44
lr : gmac_clk_enable+0xf8/0x188 [dwmac_rk]
..
Call trace:
  clk_prepare+0x18/0x44 (P)
  gmac_clk_enable+0xf8/0x188 [dwmac_rk]
  rk_gmac_powerup+0x4c/0x1f0 [dwmac_rk]
  rk_gmac_probe+0x3b4/0x5c8 [dwmac_rk]
  platform_probe+0x5c/0xac
  really_probe+0xbc/0x298
  __driver_probe_device+0x78/0x12c
  driver_probe_device+0x40/0x164
  __driver_attach+0x9c/0x1ac
  bus_for_each_dev+0x74/0xd0
  driver_attach+0x24/0x30
  bus_add_driver+0xe4/0x208
  driver_register+0x60/0x128
  __platform_driver_register+0x24/0x30
  rk_gmac_dwmac_driver_init+0x20/0x1000 [dwmac_rk]
  do_one_initcall+0x64/0x308
  do_init_module+0x58/0x23c
  load_module+0x1b48/0x1dc4
  init_module_from_file+0x84/0xc4
  idempotent_init_module+0x188/0x280
  __arm64_sys_finit_module+0x68/0xac
  invoke_syscall+0x48/0x110
  el0_svc_common.constprop.0+0xc8/0xe8
  do_el0_svc+0x20/0x2c
  el0_svc+0x4c/0x160
  el0t_64_sync_handler+0xa0/0xe4
  el0t_64_sync+0x198/0x19c
Code: 910003fd f9000bf3 52800013 b40000e0 (f9400013)
---[ end trace 0000000000000000 ]---

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Chaoyi Chen]

Hi Marek,

On 8/25/2025 3:23 PM, Marek Szyprowski wrote:
> On 15.08.2025 04:35, Chaoyi Chen wrote:
>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>
>> For external phy, clk_phy should be optional, and some external phy
>> need the clock input from clk_phy. This patch adds support for setting
>> clk_phy for external phy.
>>
>> Signed-off-by: David Wu <david.wu@rock-chips.com>
>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>> ---
>>
>> Changes in v3:
>> - Link to V2: https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
>> - Rebase to net-next/main
>>
>> Changes in v2:
>> - Link to V1: https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
>> - Remove get clock frequency from DT prop
>>
>>    drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
>>    1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> index ac8288301994..5d921e62c2f5 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct plat_stmmacenet_data *plat)
>>    		clk_set_rate(plat->stmmac_clk, 50000000);
>>    	}
>>    
>> -	if (plat->phy_node && bsp_priv->integrated_phy) {
>> +	if (plat->phy_node) {
>>    		bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>>    		ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
>> -		if (ret)
>> -			return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
>> -		clk_set_rate(bsp_priv->clk_phy, 50000000);
>> +		/* If it is not integrated_phy, clk_phy is optional */
>> +		if (bsp_priv->integrated_phy) {
>> +			if (ret)
>> +				return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
>> +			clk_set_rate(bsp_priv->clk_phy, 50000000);
>> +		}

I think  we should set bsp_priv->clk_phy to NULL here if we failed to get the clock.

Could you try this on your board? Thank you.

>>    	}
>>    
>>    	return 0;
> The above change lacks the following check ingmac_clk_enable(): if (!bsp_priv->integrated_phy) return 0;
>
> Otherwise it blows on machines with integrated PHY, like Hardkernel's Odroid-M1 (RK3568 based):
>
> rk_gmac-dwmac fe2a0000.ethernet: IRQ eth_lpi not found
> rk_gmac-dwmac fe2a0000.ethernet: IRQ sfty not found
> rk_gmac-dwmac fe2a0000.ethernet: clock input or output? (output).
> rk_gmac-dwmac fe2a0000.ethernet: TX delay(0x4f).
> rk_gmac-dwmac fe2a0000.ethernet: RX delay(0x2d).
> rk_gmac-dwmac fe2a0000.ethernet: integrated PHY? (no).
> Unable to handle kernel paging request at virtual address fffffffffffffffe
> Mem abort info:
>     ESR = 0x0000000096000006
>     EC = 0x25: DABT (current EL), IL = 32 bits
>     SET = 0, FnV = 0
>     EA = 0, S1PTW = 0
>     FSC = 0x06: level 2 translation fault
> Data abort info:
>     ISV = 0, ISS = 0x00000006, ISS2 = 0x00000000
>     CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>     GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> swapper pgtable: 4k pages, 48-bit VAs, pgdp=000000000249e000
> [fffffffffffffffe] pgd=0000000000000000, p4d=000000000376f403, pud=0000000003770403, pmd=0000000000000000
> Internal error: Oops: 0000000096000006 [#1]  SMP
> Modules linked in: snd_soc_rockchip_i2s_tdm snd_soc_rk817 snd_soc_core snd_compress snd_pcm_dmaengine rockchip_thermal snd_pcm dwmac_rk(+) hantro_vpu rockchip_saradc industrialio_triggered_buffer kfifo_buf stmmac_platform rockchip_rga v4l2_vp9 stmmac spi_rockchip_sfc(+) v4l2_h264 snd_timer pcs_xpcs rockchipdrm(+) videobuf2_dma_sg v4l2_jpeg rk805_pwrkey v4l2_mem2mem videobuf2_dma_contig dw_hdmi_qp rockchip_dfi rk817_charger videobuf2_memops analogix_dp videobuf2_v4l2 dw_mipi_dsi panfrost snd rtc_rk808 drm_dp_aux_bus videodev drm_shmem_helper soundcore dw_hdmi videobuf2_common drm_display_helper mc gpu_sched ahci_dwc ipv6
> CPU: 3 UID: 0 PID: 154 Comm: systemd-udevd Not tainted 6.17.0-rc1+ #10875 PREEMPT
> Hardware name: Hardkernel ODROID-M1 (DT)
> pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : clk_prepare+0x18/0x44
> lr : gmac_clk_enable+0xf8/0x188 [dwmac_rk]
> ..
> Call trace:
>    clk_prepare+0x18/0x44 (P)
>    gmac_clk_enable+0xf8/0x188 [dwmac_rk]
>    rk_gmac_powerup+0x4c/0x1f0 [dwmac_rk]
>    rk_gmac_probe+0x3b4/0x5c8 [dwmac_rk]
>    platform_probe+0x5c/0xac
>    really_probe+0xbc/0x298
>    __driver_probe_device+0x78/0x12c
>    driver_probe_device+0x40/0x164
>    __driver_attach+0x9c/0x1ac
>    bus_for_each_dev+0x74/0xd0
>    driver_attach+0x24/0x30
>    bus_add_driver+0xe4/0x208
>    driver_register+0x60/0x128
>    __platform_driver_register+0x24/0x30
>    rk_gmac_dwmac_driver_init+0x20/0x1000 [dwmac_rk]
>    do_one_initcall+0x64/0x308
>    do_init_module+0x58/0x23c
>    load_module+0x1b48/0x1dc4
>    init_module_from_file+0x84/0xc4
>    idempotent_init_module+0x188/0x280
>    __arm64_sys_finit_module+0x68/0xac
>    invoke_syscall+0x48/0x110
>    el0_svc_common.constprop.0+0xc8/0xe8
>    do_el0_svc+0x20/0x2c
>    el0_svc+0x4c/0x160
>    el0t_64_sync_handler+0xa0/0xe4
>    el0t_64_sync+0x198/0x19c
> Code: 910003fd f9000bf3 52800013 b40000e0 (f9400013)
> ---[ end trace 0000000000000000 ]---
>
> Best regards

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Chaoyi Chen]

Hi Marek,

On 8/25/2025 3:23 PM, Marek Szyprowski wrote:
> On 15.08.2025 04:35, Chaoyi Chen wrote:
>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>
>> For external phy, clk_phy should be optional, and some external phy
>> need the clock input from clk_phy. This patch adds support for setting
>> clk_phy for external phy.
>>
>> Signed-off-by: David Wu <david.wu@rock-chips.com>
>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>> ---
>>
>> Changes in v3:
>> - Link to V2: https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
>> - Rebase to net-next/main
>>
>> Changes in v2:
>> - Link to V1: https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
>> - Remove get clock frequency from DT prop
>>
>>    drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
>>    1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> index ac8288301994..5d921e62c2f5 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct plat_stmmacenet_data *plat)
>>    		clk_set_rate(plat->stmmac_clk, 50000000);
>>    	}
>>    
>> -	if (plat->phy_node && bsp_priv->integrated_phy) {
>> +	if (plat->phy_node) {
>>    		bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>>    		ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
>> -		if (ret)
>> -			return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
>> -		clk_set_rate(bsp_priv->clk_phy, 50000000);
>> +		/* If it is not integrated_phy, clk_phy is optional */
>> +		if (bsp_priv->integrated_phy) {
>> +			if (ret)
>> +				return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
>> +			clk_set_rate(bsp_priv->clk_phy, 50000000);
>> +		}

I think  we should set bsp_priv->clk_phy to NULL here if we failed to get the clock.

Could you try this on your board? Thank you.

>>    	}
>>    
>>    	return 0;
> The above change lacks the following check ingmac_clk_enable(): if (!bsp_priv->integrated_phy) return 0;
>
> Otherwise it blows on machines with integrated PHY, like Hardkernel's Odroid-M1 (RK3568 based):
>
> rk_gmac-dwmac fe2a0000.ethernet: IRQ eth_lpi not found
> rk_gmac-dwmac fe2a0000.ethernet: IRQ sfty not found
> rk_gmac-dwmac fe2a0000.ethernet: clock input or output? (output).
> rk_gmac-dwmac fe2a0000.ethernet: TX delay(0x4f).
> rk_gmac-dwmac fe2a0000.ethernet: RX delay(0x2d).
> rk_gmac-dwmac fe2a0000.ethernet: integrated PHY? (no).
> Unable to handle kernel paging request at virtual address fffffffffffffffe
> Mem abort info:
>     ESR = 0x0000000096000006
>     EC = 0x25: DABT (current EL), IL = 32 bits
>     SET = 0, FnV = 0
>     EA = 0, S1PTW = 0
>     FSC = 0x06: level 2 translation fault
> Data abort info:
>     ISV = 0, ISS = 0x00000006, ISS2 = 0x00000000
>     CM = 0, WnR = 0, TnD = 0, TagAccess = 0
>     GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> swapper pgtable: 4k pages, 48-bit VAs, pgdp=000000000249e000
> [fffffffffffffffe] pgd=0000000000000000, p4d=000000000376f403, pud=0000000003770403, pmd=0000000000000000
> Internal error: Oops: 0000000096000006 [#1]  SMP
> Modules linked in: snd_soc_rockchip_i2s_tdm snd_soc_rk817 snd_soc_core snd_compress snd_pcm_dmaengine rockchip_thermal snd_pcm dwmac_rk(+) hantro_vpu rockchip_saradc industrialio_triggered_buffer kfifo_buf stmmac_platform rockchip_rga v4l2_vp9 stmmac spi_rockchip_sfc(+) v4l2_h264 snd_timer pcs_xpcs rockchipdrm(+) videobuf2_dma_sg v4l2_jpeg rk805_pwrkey v4l2_mem2mem videobuf2_dma_contig dw_hdmi_qp rockchip_dfi rk817_charger videobuf2_memops analogix_dp videobuf2_v4l2 dw_mipi_dsi panfrost snd rtc_rk808 drm_dp_aux_bus videodev drm_shmem_helper soundcore dw_hdmi videobuf2_common drm_display_helper mc gpu_sched ahci_dwc ipv6
> CPU: 3 UID: 0 PID: 154 Comm: systemd-udevd Not tainted 6.17.0-rc1+ #10875 PREEMPT
> Hardware name: Hardkernel ODROID-M1 (DT)
> pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : clk_prepare+0x18/0x44
> lr : gmac_clk_enable+0xf8/0x188 [dwmac_rk]
> ..
> Call trace:
>    clk_prepare+0x18/0x44 (P)
>    gmac_clk_enable+0xf8/0x188 [dwmac_rk]
>    rk_gmac_powerup+0x4c/0x1f0 [dwmac_rk]
>    rk_gmac_probe+0x3b4/0x5c8 [dwmac_rk]
>    platform_probe+0x5c/0xac
>    really_probe+0xbc/0x298
>    __driver_probe_device+0x78/0x12c
>    driver_probe_device+0x40/0x164
>    __driver_attach+0x9c/0x1ac
>    bus_for_each_dev+0x74/0xd0
>    driver_attach+0x24/0x30
>    bus_add_driver+0xe4/0x208
>    driver_register+0x60/0x128
>    __platform_driver_register+0x24/0x30
>    rk_gmac_dwmac_driver_init+0x20/0x1000 [dwmac_rk]
>    do_one_initcall+0x64/0x308
>    do_init_module+0x58/0x23c
>    load_module+0x1b48/0x1dc4
>    init_module_from_file+0x84/0xc4
>    idempotent_init_module+0x188/0x280
>    __arm64_sys_finit_module+0x68/0xac
>    invoke_syscall+0x48/0x110
>    el0_svc_common.constprop.0+0xc8/0xe8
>    do_el0_svc+0x20/0x2c
>    el0_svc+0x4c/0x160
>    el0t_64_sync_handler+0xa0/0xe4
>    el0t_64_sync+0x198/0x19c
> Code: 910003fd f9000bf3 52800013 b40000e0 (f9400013)
> ---[ end trace 0000000000000000 ]---
>
> Best regards

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Marek Szyprowski]

On 25.08.2025 11:57, Chaoyi Chen wrote:
> On 8/25/2025 3:23 PM, Marek Szyprowski wrote:
>> On 15.08.2025 04:35, Chaoyi Chen wrote:
>>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>
>>> For external phy, clk_phy should be optional, and some external phy
>>> need the clock input from clk_phy. This patch adds support for setting
>>> clk_phy for external phy.
>>>
>>> Signed-off-by: David Wu <david.wu@rock-chips.com>
>>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>> ---
>>>
>>> Changes in v3:
>>> - Link to V2: 
>>> https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
>>> - Rebase to net-next/main
>>>
>>> Changes in v2:
>>> - Link to V1: 
>>> https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
>>> - Remove get clock frequency from DT prop
>>>
>>>    drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
>>>    1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c 
>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>> index ac8288301994..5d921e62c2f5 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>> @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct 
>>> plat_stmmacenet_data *plat)
>>>            clk_set_rate(plat->stmmac_clk, 50000000);
>>>        }
>>>    -    if (plat->phy_node && bsp_priv->integrated_phy) {
>>> +    if (plat->phy_node) {
>>>            bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>>>            ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
>>> -        if (ret)
>>> -            return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
>>> -        clk_set_rate(bsp_priv->clk_phy, 50000000);
>>> +        /* If it is not integrated_phy, clk_phy is optional */
>>> +        if (bsp_priv->integrated_phy) {
>>> +            if (ret)
>>> +                return dev_err_probe(dev, ret, "Cannot get PHY 
>>> clock\n");
>>> +            clk_set_rate(bsp_priv->clk_phy, 50000000);
>>> +        }
>
> I think  we should set bsp_priv->clk_phy to NULL here if we failed to 
> get the clock.
>
> Could you try this on your board? Thank you.

Right, the following change also fixes this issue:

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 9fc41207cc45..2d19d48be01f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -1415,6 +1415,8 @@ static int rk_gmac_clk_init(struct 
plat_stmmacenet_data *plat)
         if (plat->phy_node) {
                 bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
                 ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
+               if (ret)
+                       bsp_priv->clk_phy = NULL;
                 /* If it is not integrated_phy, clk_phy is optional */
                 if (bsp_priv->integrated_phy) {
                         if (ret)


 > ...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Marek Szyprowski]

On 25.08.2025 11:57, Chaoyi Chen wrote:
> On 8/25/2025 3:23 PM, Marek Szyprowski wrote:
>> On 15.08.2025 04:35, Chaoyi Chen wrote:
>>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>
>>> For external phy, clk_phy should be optional, and some external phy
>>> need the clock input from clk_phy. This patch adds support for setting
>>> clk_phy for external phy.
>>>
>>> Signed-off-by: David Wu <david.wu@rock-chips.com>
>>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>> ---
>>>
>>> Changes in v3:
>>> - Link to V2: 
>>> https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
>>> - Rebase to net-next/main
>>>
>>> Changes in v2:
>>> - Link to V1: 
>>> https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
>>> - Remove get clock frequency from DT prop
>>>
>>>    drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
>>>    1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c 
>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>> index ac8288301994..5d921e62c2f5 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>> @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct 
>>> plat_stmmacenet_data *plat)
>>>            clk_set_rate(plat->stmmac_clk, 50000000);
>>>        }
>>>    -    if (plat->phy_node && bsp_priv->integrated_phy) {
>>> +    if (plat->phy_node) {
>>>            bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>>>            ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
>>> -        if (ret)
>>> -            return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
>>> -        clk_set_rate(bsp_priv->clk_phy, 50000000);
>>> +        /* If it is not integrated_phy, clk_phy is optional */
>>> +        if (bsp_priv->integrated_phy) {
>>> +            if (ret)
>>> +                return dev_err_probe(dev, ret, "Cannot get PHY 
>>> clock\n");
>>> +            clk_set_rate(bsp_priv->clk_phy, 50000000);
>>> +        }
>
> I think  we should set bsp_priv->clk_phy to NULL here if we failed to 
> get the clock.
>
> Could you try this on your board? Thank you.

Right, the following change also fixes this issue:

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index 9fc41207cc45..2d19d48be01f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -1415,6 +1415,8 @@ static int rk_gmac_clk_init(struct 
plat_stmmacenet_data *plat)
         if (plat->phy_node) {
                 bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
                 ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
+               if (ret)
+                       bsp_priv->clk_phy = NULL;
                 /* If it is not integrated_phy, clk_phy is optional */
                 if (bsp_priv->integrated_phy) {
                         if (ret)


 > ...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Russell King (Oracle)]

On Mon, Aug 25, 2025 at 12:53:37PM +0200, Marek Szyprowski wrote:
> On 25.08.2025 11:57, Chaoyi Chen wrote:
> > On 8/25/2025 3:23 PM, Marek Szyprowski wrote:
> >> On 15.08.2025 04:35, Chaoyi Chen wrote:
> >>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> >>>
> >>> For external phy, clk_phy should be optional, and some external phy
> >>> need the clock input from clk_phy. This patch adds support for setting
> >>> clk_phy for external phy.
> >>>
> >>> Signed-off-by: David Wu <david.wu@rock-chips.com>
> >>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> >>> ---
> >>>
> >>> Changes in v3:
> >>> - Link to V2: 
> >>> https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
> >>> - Rebase to net-next/main
> >>>
> >>> Changes in v2:
> >>> - Link to V1: 
> >>> https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
> >>> - Remove get clock frequency from DT prop
> >>>
> >>>    drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
> >>>    1 file changed, 7 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c 
> >>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> >>> index ac8288301994..5d921e62c2f5 100644
> >>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> >>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> >>> @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct 
> >>> plat_stmmacenet_data *plat)
> >>>            clk_set_rate(plat->stmmac_clk, 50000000);
> >>>        }
> >>>    -    if (plat->phy_node && bsp_priv->integrated_phy) {
> >>> +    if (plat->phy_node) {
> >>>            bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
> >>>            ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
> >>> -        if (ret)
> >>> -            return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
> >>> -        clk_set_rate(bsp_priv->clk_phy, 50000000);
> >>> +        /* If it is not integrated_phy, clk_phy is optional */
> >>> +        if (bsp_priv->integrated_phy) {
> >>> +            if (ret)
> >>> +                return dev_err_probe(dev, ret, "Cannot get PHY 
> >>> clock\n");
> >>> +            clk_set_rate(bsp_priv->clk_phy, 50000000);
> >>> +        }
> >
> > I think  we should set bsp_priv->clk_phy to NULL here if we failed to 
> > get the clock.
> >
> > Could you try this on your board? Thank you.
> 
> Right, the following change also fixes this issue:
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c 
> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> index 9fc41207cc45..2d19d48be01f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> @@ -1415,6 +1415,8 @@ static int rk_gmac_clk_init(struct 
> plat_stmmacenet_data *plat)
>          if (plat->phy_node) {
>                  bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>                  ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
> +               if (ret)
> +                       bsp_priv->clk_phy = NULL;

Or just:

		clk = of_clk_get(plat->phy_node, 0);
		if (clk == ERR_PTR(-EPROBE_DEFER))
			...
		else if (!IS_ERR)
			bsp_priv->clk_phy = clk;

I don't have a free terminal to work out what "..." should be.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Russell King (Oracle)]

On Mon, Aug 25, 2025 at 12:53:37PM +0200, Marek Szyprowski wrote:
> On 25.08.2025 11:57, Chaoyi Chen wrote:
> > On 8/25/2025 3:23 PM, Marek Szyprowski wrote:
> >> On 15.08.2025 04:35, Chaoyi Chen wrote:
> >>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> >>>
> >>> For external phy, clk_phy should be optional, and some external phy
> >>> need the clock input from clk_phy. This patch adds support for setting
> >>> clk_phy for external phy.
> >>>
> >>> Signed-off-by: David Wu <david.wu@rock-chips.com>
> >>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> >>> ---
> >>>
> >>> Changes in v3:
> >>> - Link to V2: 
> >>> https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
> >>> - Rebase to net-next/main
> >>>
> >>> Changes in v2:
> >>> - Link to V1: 
> >>> https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
> >>> - Remove get clock frequency from DT prop
> >>>
> >>>    drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
> >>>    1 file changed, 7 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c 
> >>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> >>> index ac8288301994..5d921e62c2f5 100644
> >>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> >>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> >>> @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct 
> >>> plat_stmmacenet_data *plat)
> >>>            clk_set_rate(plat->stmmac_clk, 50000000);
> >>>        }
> >>>    -    if (plat->phy_node && bsp_priv->integrated_phy) {
> >>> +    if (plat->phy_node) {
> >>>            bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
> >>>            ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
> >>> -        if (ret)
> >>> -            return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
> >>> -        clk_set_rate(bsp_priv->clk_phy, 50000000);
> >>> +        /* If it is not integrated_phy, clk_phy is optional */
> >>> +        if (bsp_priv->integrated_phy) {
> >>> +            if (ret)
> >>> +                return dev_err_probe(dev, ret, "Cannot get PHY 
> >>> clock\n");
> >>> +            clk_set_rate(bsp_priv->clk_phy, 50000000);
> >>> +        }
> >
> > I think  we should set bsp_priv->clk_phy to NULL here if we failed to 
> > get the clock.
> >
> > Could you try this on your board? Thank you.
> 
> Right, the following change also fixes this issue:
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c 
> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> index 9fc41207cc45..2d19d48be01f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> @@ -1415,6 +1415,8 @@ static int rk_gmac_clk_init(struct 
> plat_stmmacenet_data *plat)
>          if (plat->phy_node) {
>                  bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>                  ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
> +               if (ret)
> +                       bsp_priv->clk_phy = NULL;

Or just:

		clk = of_clk_get(plat->phy_node, 0);
		if (clk == ERR_PTR(-EPROBE_DEFER))
			...
		else if (!IS_ERR)
			bsp_priv->clk_phy = clk;

I don't have a free terminal to work out what "..." should be.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Chaoyi Chen]

Hi Russell,

On 8/25/2025 9:37 PM, Russell King (Oracle) wrote:
> On Mon, Aug 25, 2025 at 12:53:37PM +0200, Marek Szyprowski wrote:
>> On 25.08.2025 11:57, Chaoyi Chen wrote:
>>> On 8/25/2025 3:23 PM, Marek Szyprowski wrote:
>>>> On 15.08.2025 04:35, Chaoyi Chen wrote:
>>>>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>>
>>>>> For external phy, clk_phy should be optional, and some external phy
>>>>> need the clock input from clk_phy. This patch adds support for setting
>>>>> clk_phy for external phy.
>>>>>
>>>>> Signed-off-by: David Wu <david.wu@rock-chips.com>
>>>>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>> ---
>>>>>
>>>>> Changes in v3:
>>>>> - Link to V2:
>>>>> https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
>>>>> - Rebase to net-next/main
>>>>>
>>>>> Changes in v2:
>>>>> - Link to V1:
>>>>> https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
>>>>> - Remove get clock frequency from DT prop
>>>>>
>>>>>     drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
>>>>>     1 file changed, 7 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>> index ac8288301994..5d921e62c2f5 100644
>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>> @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct
>>>>> plat_stmmacenet_data *plat)
>>>>>             clk_set_rate(plat->stmmac_clk, 50000000);
>>>>>         }
>>>>>     -    if (plat->phy_node && bsp_priv->integrated_phy) {
>>>>> +    if (plat->phy_node) {
>>>>>             bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>>>>>             ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
>>>>> -        if (ret)
>>>>> -            return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
>>>>> -        clk_set_rate(bsp_priv->clk_phy, 50000000);
>>>>> +        /* If it is not integrated_phy, clk_phy is optional */
>>>>> +        if (bsp_priv->integrated_phy) {
>>>>> +            if (ret)
>>>>> +                return dev_err_probe(dev, ret, "Cannot get PHY
>>>>> clock\n");
>>>>> +            clk_set_rate(bsp_priv->clk_phy, 50000000);
>>>>> +        }
>>> I think  we should set bsp_priv->clk_phy to NULL here if we failed to
>>> get the clock.
>>>
>>> Could you try this on your board? Thank you.
>> Right, the following change also fixes this issue:
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> index 9fc41207cc45..2d19d48be01f 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> @@ -1415,6 +1415,8 @@ static int rk_gmac_clk_init(struct
>> plat_stmmacenet_data *plat)
>>           if (plat->phy_node) {
>>                   bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>>                   ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
>> +               if (ret)
>> +                       bsp_priv->clk_phy = NULL;
> Or just:
>
> 		clk = of_clk_get(plat->phy_node, 0);
> 		if (clk == ERR_PTR(-EPROBE_DEFER))

Do we actually need this? Maybe other devm_clk_get() before it would fail in advance.


> 			...
> 		else if (!IS_ERR)
> 			bsp_priv->clk_phy = clk;
>
> I don't have a free terminal to work out what "..." should be.
>

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Chaoyi Chen]

Hi Russell,

On 8/25/2025 9:37 PM, Russell King (Oracle) wrote:
> On Mon, Aug 25, 2025 at 12:53:37PM +0200, Marek Szyprowski wrote:
>> On 25.08.2025 11:57, Chaoyi Chen wrote:
>>> On 8/25/2025 3:23 PM, Marek Szyprowski wrote:
>>>> On 15.08.2025 04:35, Chaoyi Chen wrote:
>>>>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>>
>>>>> For external phy, clk_phy should be optional, and some external phy
>>>>> need the clock input from clk_phy. This patch adds support for setting
>>>>> clk_phy for external phy.
>>>>>
>>>>> Signed-off-by: David Wu <david.wu@rock-chips.com>
>>>>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>> ---
>>>>>
>>>>> Changes in v3:
>>>>> - Link to V2:
>>>>> https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
>>>>> - Rebase to net-next/main
>>>>>
>>>>> Changes in v2:
>>>>> - Link to V1:
>>>>> https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
>>>>> - Remove get clock frequency from DT prop
>>>>>
>>>>>     drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
>>>>>     1 file changed, 7 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>> index ac8288301994..5d921e62c2f5 100644
>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>> @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct
>>>>> plat_stmmacenet_data *plat)
>>>>>             clk_set_rate(plat->stmmac_clk, 50000000);
>>>>>         }
>>>>>     -    if (plat->phy_node && bsp_priv->integrated_phy) {
>>>>> +    if (plat->phy_node) {
>>>>>             bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>>>>>             ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
>>>>> -        if (ret)
>>>>> -            return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
>>>>> -        clk_set_rate(bsp_priv->clk_phy, 50000000);
>>>>> +        /* If it is not integrated_phy, clk_phy is optional */
>>>>> +        if (bsp_priv->integrated_phy) {
>>>>> +            if (ret)
>>>>> +                return dev_err_probe(dev, ret, "Cannot get PHY
>>>>> clock\n");
>>>>> +            clk_set_rate(bsp_priv->clk_phy, 50000000);
>>>>> +        }
>>> I think  we should set bsp_priv->clk_phy to NULL here if we failed to
>>> get the clock.
>>>
>>> Could you try this on your board? Thank you.
>> Right, the following change also fixes this issue:
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> index 9fc41207cc45..2d19d48be01f 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>> @@ -1415,6 +1415,8 @@ static int rk_gmac_clk_init(struct
>> plat_stmmacenet_data *plat)
>>           if (plat->phy_node) {
>>                   bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>>                   ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
>> +               if (ret)
>> +                       bsp_priv->clk_phy = NULL;
> Or just:
>
> 		clk = of_clk_get(plat->phy_node, 0);
> 		if (clk == ERR_PTR(-EPROBE_DEFER))

Do we actually need this? Maybe other devm_clk_get() before it would fail in advance.


> 			...
> 		else if (!IS_ERR)
> 			bsp_priv->clk_phy = clk;
>
> I don't have a free terminal to work out what "..." should be.
>

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Russell King (Oracle)]

On Tue, Aug 26, 2025 at 04:08:40PM +0800, Chaoyi Chen wrote:
> Hi Russell,
> 
> On 8/25/2025 9:37 PM, Russell King (Oracle) wrote:
> > On Mon, Aug 25, 2025 at 12:53:37PM +0200, Marek Szyprowski wrote:
> > > On 25.08.2025 11:57, Chaoyi Chen wrote:
> > > > On 8/25/2025 3:23 PM, Marek Szyprowski wrote:
> > > > > On 15.08.2025 04:35, Chaoyi Chen wrote:
> > > > > > From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > > 
> > > > > > For external phy, clk_phy should be optional, and some external phy
> > > > > > need the clock input from clk_phy. This patch adds support for setting
> > > > > > clk_phy for external phy.
> > > > > > 
> > > > > > Signed-off-by: David Wu <david.wu@rock-chips.com>
> > > > > > Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > > ---
> > > > > > 
> > > > > > Changes in v3:
> > > > > > - Link to V2:
> > > > > > https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
> > > > > > - Rebase to net-next/main
> > > > > > 
> > > > > > Changes in v2:
> > > > > > - Link to V1:
> > > > > > https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
> > > > > > - Remove get clock frequency from DT prop
> > > > > > 
> > > > > >     drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
> > > > > >     1 file changed, 7 insertions(+), 4 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > > b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > > index ac8288301994..5d921e62c2f5 100644
> > > > > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > > @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct
> > > > > > plat_stmmacenet_data *plat)
> > > > > >             clk_set_rate(plat->stmmac_clk, 50000000);
> > > > > >         }
> > > > > >     -    if (plat->phy_node && bsp_priv->integrated_phy) {
> > > > > > +    if (plat->phy_node) {
> > > > > >             bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
> > > > > >             ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
> > > > > > -        if (ret)
> > > > > > -            return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
> > > > > > -        clk_set_rate(bsp_priv->clk_phy, 50000000);
> > > > > > +        /* If it is not integrated_phy, clk_phy is optional */
> > > > > > +        if (bsp_priv->integrated_phy) {
> > > > > > +            if (ret)
> > > > > > +                return dev_err_probe(dev, ret, "Cannot get PHY
> > > > > > clock\n");
> > > > > > +            clk_set_rate(bsp_priv->clk_phy, 50000000);
> > > > > > +        }
> > > > I think  we should set bsp_priv->clk_phy to NULL here if we failed to
> > > > get the clock.
> > > > 
> > > > Could you try this on your board? Thank you.
> > > Right, the following change also fixes this issue:
> > > 
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > index 9fc41207cc45..2d19d48be01f 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > @@ -1415,6 +1415,8 @@ static int rk_gmac_clk_init(struct
> > > plat_stmmacenet_data *plat)
> > >           if (plat->phy_node) {
> > >                   bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
> > >                   ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
> > > +               if (ret)
> > > +                       bsp_priv->clk_phy = NULL;
> > Or just:
> > 
> > 		clk = of_clk_get(plat->phy_node, 0);
> > 		if (clk == ERR_PTR(-EPROBE_DEFER))
> 
> Do we actually need this? Maybe other devm_clk_get() before it would fail in advance.

Is it the same clock as devm_clk_get()? If it is, what's the point of
getting it a second time. If it isn't, then it could be a different
clock which may be yet to probe.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Russell King (Oracle)]

On Tue, Aug 26, 2025 at 04:08:40PM +0800, Chaoyi Chen wrote:
> Hi Russell,
> 
> On 8/25/2025 9:37 PM, Russell King (Oracle) wrote:
> > On Mon, Aug 25, 2025 at 12:53:37PM +0200, Marek Szyprowski wrote:
> > > On 25.08.2025 11:57, Chaoyi Chen wrote:
> > > > On 8/25/2025 3:23 PM, Marek Szyprowski wrote:
> > > > > On 15.08.2025 04:35, Chaoyi Chen wrote:
> > > > > > From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > > 
> > > > > > For external phy, clk_phy should be optional, and some external phy
> > > > > > need the clock input from clk_phy. This patch adds support for setting
> > > > > > clk_phy for external phy.
> > > > > > 
> > > > > > Signed-off-by: David Wu <david.wu@rock-chips.com>
> > > > > > Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > > ---
> > > > > > 
> > > > > > Changes in v3:
> > > > > > - Link to V2:
> > > > > > https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
> > > > > > - Rebase to net-next/main
> > > > > > 
> > > > > > Changes in v2:
> > > > > > - Link to V1:
> > > > > > https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
> > > > > > - Remove get clock frequency from DT prop
> > > > > > 
> > > > > >     drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
> > > > > >     1 file changed, 7 insertions(+), 4 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > > b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > > index ac8288301994..5d921e62c2f5 100644
> > > > > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > > @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct
> > > > > > plat_stmmacenet_data *plat)
> > > > > >             clk_set_rate(plat->stmmac_clk, 50000000);
> > > > > >         }
> > > > > >     -    if (plat->phy_node && bsp_priv->integrated_phy) {
> > > > > > +    if (plat->phy_node) {
> > > > > >             bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
> > > > > >             ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
> > > > > > -        if (ret)
> > > > > > -            return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
> > > > > > -        clk_set_rate(bsp_priv->clk_phy, 50000000);
> > > > > > +        /* If it is not integrated_phy, clk_phy is optional */
> > > > > > +        if (bsp_priv->integrated_phy) {
> > > > > > +            if (ret)
> > > > > > +                return dev_err_probe(dev, ret, "Cannot get PHY
> > > > > > clock\n");
> > > > > > +            clk_set_rate(bsp_priv->clk_phy, 50000000);
> > > > > > +        }
> > > > I think  we should set bsp_priv->clk_phy to NULL here if we failed to
> > > > get the clock.
> > > > 
> > > > Could you try this on your board? Thank you.
> > > Right, the following change also fixes this issue:
> > > 
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > index 9fc41207cc45..2d19d48be01f 100644
> > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > @@ -1415,6 +1415,8 @@ static int rk_gmac_clk_init(struct
> > > plat_stmmacenet_data *plat)
> > >           if (plat->phy_node) {
> > >                   bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
> > >                   ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
> > > +               if (ret)
> > > +                       bsp_priv->clk_phy = NULL;
> > Or just:
> > 
> > 		clk = of_clk_get(plat->phy_node, 0);
> > 		if (clk == ERR_PTR(-EPROBE_DEFER))
> 
> Do we actually need this? Maybe other devm_clk_get() before it would fail in advance.

Is it the same clock as devm_clk_get()? If it is, what's the point of
getting it a second time. If it isn't, then it could be a different
clock which may be yet to probe.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Chaoyi Chen]

On 8/26/2025 5:25 PM, Russell King (Oracle) wrote:

> On Tue, Aug 26, 2025 at 04:08:40PM +0800, Chaoyi Chen wrote:
>> Hi Russell,
>>
>> On 8/25/2025 9:37 PM, Russell King (Oracle) wrote:
>>> On Mon, Aug 25, 2025 at 12:53:37PM +0200, Marek Szyprowski wrote:
>>>> On 25.08.2025 11:57, Chaoyi Chen wrote:
>>>>> On 8/25/2025 3:23 PM, Marek Szyprowski wrote:
>>>>>> On 15.08.2025 04:35, Chaoyi Chen wrote:
>>>>>>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>>>>
>>>>>>> For external phy, clk_phy should be optional, and some external phy
>>>>>>> need the clock input from clk_phy. This patch adds support for setting
>>>>>>> clk_phy for external phy.
>>>>>>>
>>>>>>> Signed-off-by: David Wu <david.wu@rock-chips.com>
>>>>>>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>>>> ---
>>>>>>>
>>>>>>> Changes in v3:
>>>>>>> - Link to V2:
>>>>>>> https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
>>>>>>> - Rebase to net-next/main
>>>>>>>
>>>>>>> Changes in v2:
>>>>>>> - Link to V1:
>>>>>>> https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
>>>>>>> - Remove get clock frequency from DT prop
>>>>>>>
>>>>>>>      drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
>>>>>>>      1 file changed, 7 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>> index ac8288301994..5d921e62c2f5 100644
>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>> @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct
>>>>>>> plat_stmmacenet_data *plat)
>>>>>>>              clk_set_rate(plat->stmmac_clk, 50000000);
>>>>>>>          }
>>>>>>>      -    if (plat->phy_node && bsp_priv->integrated_phy) {
>>>>>>> +    if (plat->phy_node) {
>>>>>>>              bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>>>>>>>              ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
>>>>>>> -        if (ret)
>>>>>>> -            return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
>>>>>>> -        clk_set_rate(bsp_priv->clk_phy, 50000000);
>>>>>>> +        /* If it is not integrated_phy, clk_phy is optional */
>>>>>>> +        if (bsp_priv->integrated_phy) {
>>>>>>> +            if (ret)
>>>>>>> +                return dev_err_probe(dev, ret, "Cannot get PHY
>>>>>>> clock\n");
>>>>>>> +            clk_set_rate(bsp_priv->clk_phy, 50000000);
>>>>>>> +        }
>>>>> I think  we should set bsp_priv->clk_phy to NULL here if we failed to
>>>>> get the clock.
>>>>>
>>>>> Could you try this on your board? Thank you.
>>>> Right, the following change also fixes this issue:
>>>>
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>> index 9fc41207cc45..2d19d48be01f 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>> @@ -1415,6 +1415,8 @@ static int rk_gmac_clk_init(struct
>>>> plat_stmmacenet_data *plat)
>>>>            if (plat->phy_node) {
>>>>                    bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>>>>                    ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
>>>> +               if (ret)
>>>> +                       bsp_priv->clk_phy = NULL;
>>> Or just:
>>>
>>> 		clk = of_clk_get(plat->phy_node, 0);
>>> 		if (clk == ERR_PTR(-EPROBE_DEFER))
>> Do we actually need this? Maybe other devm_clk_get() before it would fail in advance.
> Is it the same clock as devm_clk_get()? If it is, what's the point of
> getting it a second time. If it isn't, then it could be a different
> clock which may be yet to probe.

It's not the same clock, but it should be use the same clock controller driver, which is the CRU on the Rockchip platform.

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Chaoyi Chen]

On 8/26/2025 5:25 PM, Russell King (Oracle) wrote:

> On Tue, Aug 26, 2025 at 04:08:40PM +0800, Chaoyi Chen wrote:
>> Hi Russell,
>>
>> On 8/25/2025 9:37 PM, Russell King (Oracle) wrote:
>>> On Mon, Aug 25, 2025 at 12:53:37PM +0200, Marek Szyprowski wrote:
>>>> On 25.08.2025 11:57, Chaoyi Chen wrote:
>>>>> On 8/25/2025 3:23 PM, Marek Szyprowski wrote:
>>>>>> On 15.08.2025 04:35, Chaoyi Chen wrote:
>>>>>>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>>>>
>>>>>>> For external phy, clk_phy should be optional, and some external phy
>>>>>>> need the clock input from clk_phy. This patch adds support for setting
>>>>>>> clk_phy for external phy.
>>>>>>>
>>>>>>> Signed-off-by: David Wu <david.wu@rock-chips.com>
>>>>>>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>>>> ---
>>>>>>>
>>>>>>> Changes in v3:
>>>>>>> - Link to V2:
>>>>>>> https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
>>>>>>> - Rebase to net-next/main
>>>>>>>
>>>>>>> Changes in v2:
>>>>>>> - Link to V1:
>>>>>>> https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
>>>>>>> - Remove get clock frequency from DT prop
>>>>>>>
>>>>>>>      drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
>>>>>>>      1 file changed, 7 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>> index ac8288301994..5d921e62c2f5 100644
>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>> @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct
>>>>>>> plat_stmmacenet_data *plat)
>>>>>>>              clk_set_rate(plat->stmmac_clk, 50000000);
>>>>>>>          }
>>>>>>>      -    if (plat->phy_node && bsp_priv->integrated_phy) {
>>>>>>> +    if (plat->phy_node) {
>>>>>>>              bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>>>>>>>              ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
>>>>>>> -        if (ret)
>>>>>>> -            return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
>>>>>>> -        clk_set_rate(bsp_priv->clk_phy, 50000000);
>>>>>>> +        /* If it is not integrated_phy, clk_phy is optional */
>>>>>>> +        if (bsp_priv->integrated_phy) {
>>>>>>> +            if (ret)
>>>>>>> +                return dev_err_probe(dev, ret, "Cannot get PHY
>>>>>>> clock\n");
>>>>>>> +            clk_set_rate(bsp_priv->clk_phy, 50000000);
>>>>>>> +        }
>>>>> I think  we should set bsp_priv->clk_phy to NULL here if we failed to
>>>>> get the clock.
>>>>>
>>>>> Could you try this on your board? Thank you.
>>>> Right, the following change also fixes this issue:
>>>>
>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>> index 9fc41207cc45..2d19d48be01f 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>> @@ -1415,6 +1415,8 @@ static int rk_gmac_clk_init(struct
>>>> plat_stmmacenet_data *plat)
>>>>            if (plat->phy_node) {
>>>>                    bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>>>>                    ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
>>>> +               if (ret)
>>>> +                       bsp_priv->clk_phy = NULL;
>>> Or just:
>>>
>>> 		clk = of_clk_get(plat->phy_node, 0);
>>> 		if (clk == ERR_PTR(-EPROBE_DEFER))
>> Do we actually need this? Maybe other devm_clk_get() before it would fail in advance.
> Is it the same clock as devm_clk_get()? If it is, what's the point of
> getting it a second time. If it isn't, then it could be a different
> clock which may be yet to probe.

It's not the same clock, but it should be use the same clock controller driver, which is the CRU on the Rockchip platform.



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Russell King (Oracle)]

On Tue, Aug 26, 2025 at 05:29:04PM +0800, Chaoyi Chen wrote:
> On 8/26/2025 5:25 PM, Russell King (Oracle) wrote:
> 
> > On Tue, Aug 26, 2025 at 04:08:40PM +0800, Chaoyi Chen wrote:
> > > Hi Russell,
> > > 
> > > On 8/25/2025 9:37 PM, Russell King (Oracle) wrote:
> > > > On Mon, Aug 25, 2025 at 12:53:37PM +0200, Marek Szyprowski wrote:
> > > > > On 25.08.2025 11:57, Chaoyi Chen wrote:
> > > > > > On 8/25/2025 3:23 PM, Marek Szyprowski wrote:
> > > > > > > On 15.08.2025 04:35, Chaoyi Chen wrote:
> > > > > > > > From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > > > > 
> > > > > > > > For external phy, clk_phy should be optional, and some external phy
> > > > > > > > need the clock input from clk_phy. This patch adds support for setting
> > > > > > > > clk_phy for external phy.
> > > > > > > > 
> > > > > > > > Signed-off-by: David Wu <david.wu@rock-chips.com>
> > > > > > > > Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > > > > ---
> > > > > > > > 
> > > > > > > > Changes in v3:
> > > > > > > > - Link to V2:
> > > > > > > > https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
> > > > > > > > - Rebase to net-next/main
> > > > > > > > 
> > > > > > > > Changes in v2:
> > > > > > > > - Link to V1:
> > > > > > > > https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
> > > > > > > > - Remove get clock frequency from DT prop
> > > > > > > > 
> > > > > > > >      drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
> > > > > > > >      1 file changed, 7 insertions(+), 4 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > > > > b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > > > > index ac8288301994..5d921e62c2f5 100644
> > > > > > > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > > > > @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct
> > > > > > > > plat_stmmacenet_data *plat)
> > > > > > > >              clk_set_rate(plat->stmmac_clk, 50000000);
> > > > > > > >          }
> > > > > > > >      -    if (plat->phy_node && bsp_priv->integrated_phy) {
> > > > > > > > +    if (plat->phy_node) {
> > > > > > > >              bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
> > > > > > > >              ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
> > > > > > > > -        if (ret)
> > > > > > > > -            return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
> > > > > > > > -        clk_set_rate(bsp_priv->clk_phy, 50000000);
> > > > > > > > +        /* If it is not integrated_phy, clk_phy is optional */
> > > > > > > > +        if (bsp_priv->integrated_phy) {
> > > > > > > > +            if (ret)
> > > > > > > > +                return dev_err_probe(dev, ret, "Cannot get PHY
> > > > > > > > clock\n");
> > > > > > > > +            clk_set_rate(bsp_priv->clk_phy, 50000000);
> > > > > > > > +        }
> > > > > > I think  we should set bsp_priv->clk_phy to NULL here if we failed to
> > > > > > get the clock.
> > > > > > 
> > > > > > Could you try this on your board? Thank you.
> > > > > Right, the following change also fixes this issue:
> > > > > 
> > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > index 9fc41207cc45..2d19d48be01f 100644
> > > > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > @@ -1415,6 +1415,8 @@ static int rk_gmac_clk_init(struct
> > > > > plat_stmmacenet_data *plat)
> > > > >            if (plat->phy_node) {
> > > > >                    bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
> > > > >                    ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
> > > > > +               if (ret)
> > > > > +                       bsp_priv->clk_phy = NULL;
> > > > Or just:
> > > > 
> > > > 		clk = of_clk_get(plat->phy_node, 0);
> > > > 		if (clk == ERR_PTR(-EPROBE_DEFER))
> > > Do we actually need this? Maybe other devm_clk_get() before it would fail in advance.
> > Is it the same clock as devm_clk_get()? If it is, what's the point of
> > getting it a second time. If it isn't, then it could be a different
> > clock which may be yet to probe.
> 
> It's not the same clock, but it should be use the same clock controller driver, which is the CRU on the Rockchip platform.

Will it always be the same clock controller, including into the future
Rockchip devices?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Russell King (Oracle)]

On Tue, Aug 26, 2025 at 05:29:04PM +0800, Chaoyi Chen wrote:
> On 8/26/2025 5:25 PM, Russell King (Oracle) wrote:
> 
> > On Tue, Aug 26, 2025 at 04:08:40PM +0800, Chaoyi Chen wrote:
> > > Hi Russell,
> > > 
> > > On 8/25/2025 9:37 PM, Russell King (Oracle) wrote:
> > > > On Mon, Aug 25, 2025 at 12:53:37PM +0200, Marek Szyprowski wrote:
> > > > > On 25.08.2025 11:57, Chaoyi Chen wrote:
> > > > > > On 8/25/2025 3:23 PM, Marek Szyprowski wrote:
> > > > > > > On 15.08.2025 04:35, Chaoyi Chen wrote:
> > > > > > > > From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > > > > 
> > > > > > > > For external phy, clk_phy should be optional, and some external phy
> > > > > > > > need the clock input from clk_phy. This patch adds support for setting
> > > > > > > > clk_phy for external phy.
> > > > > > > > 
> > > > > > > > Signed-off-by: David Wu <david.wu@rock-chips.com>
> > > > > > > > Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> > > > > > > > ---
> > > > > > > > 
> > > > > > > > Changes in v3:
> > > > > > > > - Link to V2:
> > > > > > > > https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
> > > > > > > > - Rebase to net-next/main
> > > > > > > > 
> > > > > > > > Changes in v2:
> > > > > > > > - Link to V1:
> > > > > > > > https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
> > > > > > > > - Remove get clock frequency from DT prop
> > > > > > > > 
> > > > > > > >      drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
> > > > > > > >      1 file changed, 7 insertions(+), 4 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > > > > b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > > > > index ac8288301994..5d921e62c2f5 100644
> > > > > > > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > > > > @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct
> > > > > > > > plat_stmmacenet_data *plat)
> > > > > > > >              clk_set_rate(plat->stmmac_clk, 50000000);
> > > > > > > >          }
> > > > > > > >      -    if (plat->phy_node && bsp_priv->integrated_phy) {
> > > > > > > > +    if (plat->phy_node) {
> > > > > > > >              bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
> > > > > > > >              ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
> > > > > > > > -        if (ret)
> > > > > > > > -            return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
> > > > > > > > -        clk_set_rate(bsp_priv->clk_phy, 50000000);
> > > > > > > > +        /* If it is not integrated_phy, clk_phy is optional */
> > > > > > > > +        if (bsp_priv->integrated_phy) {
> > > > > > > > +            if (ret)
> > > > > > > > +                return dev_err_probe(dev, ret, "Cannot get PHY
> > > > > > > > clock\n");
> > > > > > > > +            clk_set_rate(bsp_priv->clk_phy, 50000000);
> > > > > > > > +        }
> > > > > > I think  we should set bsp_priv->clk_phy to NULL here if we failed to
> > > > > > get the clock.
> > > > > > 
> > > > > > Could you try this on your board? Thank you.
> > > > > Right, the following change also fixes this issue:
> > > > > 
> > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > index 9fc41207cc45..2d19d48be01f 100644
> > > > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > > > > @@ -1415,6 +1415,8 @@ static int rk_gmac_clk_init(struct
> > > > > plat_stmmacenet_data *plat)
> > > > >            if (plat->phy_node) {
> > > > >                    bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
> > > > >                    ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
> > > > > +               if (ret)
> > > > > +                       bsp_priv->clk_phy = NULL;
> > > > Or just:
> > > > 
> > > > 		clk = of_clk_get(plat->phy_node, 0);
> > > > 		if (clk == ERR_PTR(-EPROBE_DEFER))
> > > Do we actually need this? Maybe other devm_clk_get() before it would fail in advance.
> > Is it the same clock as devm_clk_get()? If it is, what's the point of
> > getting it a second time. If it isn't, then it could be a different
> > clock which may be yet to probe.
> 
> It's not the same clock, but it should be use the same clock controller driver, which is the CRU on the Rockchip platform.

Will it always be the same clock controller, including into the future
Rockchip devices?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Chaoyi Chen]

On 8/26/2025 5:34 PM, Russell King (Oracle) wrote:

> On Tue, Aug 26, 2025 at 05:29:04PM +0800, Chaoyi Chen wrote:
>> On 8/26/2025 5:25 PM, Russell King (Oracle) wrote:
>>
>>> On Tue, Aug 26, 2025 at 04:08:40PM +0800, Chaoyi Chen wrote:
>>>> Hi Russell,
>>>>
>>>> On 8/25/2025 9:37 PM, Russell King (Oracle) wrote:
>>>>> On Mon, Aug 25, 2025 at 12:53:37PM +0200, Marek Szyprowski wrote:
>>>>>> On 25.08.2025 11:57, Chaoyi Chen wrote:
>>>>>>> On 8/25/2025 3:23 PM, Marek Szyprowski wrote:
>>>>>>>> On 15.08.2025 04:35, Chaoyi Chen wrote:
>>>>>>>>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>>>>>>
>>>>>>>>> For external phy, clk_phy should be optional, and some external phy
>>>>>>>>> need the clock input from clk_phy. This patch adds support for setting
>>>>>>>>> clk_phy for external phy.
>>>>>>>>>
>>>>>>>>> Signed-off-by: David Wu <david.wu@rock-chips.com>
>>>>>>>>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> Changes in v3:
>>>>>>>>> - Link to V2:
>>>>>>>>> https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
>>>>>>>>> - Rebase to net-next/main
>>>>>>>>>
>>>>>>>>> Changes in v2:
>>>>>>>>> - Link to V1:
>>>>>>>>> https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
>>>>>>>>> - Remove get clock frequency from DT prop
>>>>>>>>>
>>>>>>>>>       drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
>>>>>>>>>       1 file changed, 7 insertions(+), 4 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>>>> index ac8288301994..5d921e62c2f5 100644
>>>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>>>> @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct
>>>>>>>>> plat_stmmacenet_data *plat)
>>>>>>>>>               clk_set_rate(plat->stmmac_clk, 50000000);
>>>>>>>>>           }
>>>>>>>>>       -    if (plat->phy_node && bsp_priv->integrated_phy) {
>>>>>>>>> +    if (plat->phy_node) {
>>>>>>>>>               bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>>>>>>>>>               ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
>>>>>>>>> -        if (ret)
>>>>>>>>> -            return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
>>>>>>>>> -        clk_set_rate(bsp_priv->clk_phy, 50000000);
>>>>>>>>> +        /* If it is not integrated_phy, clk_phy is optional */
>>>>>>>>> +        if (bsp_priv->integrated_phy) {
>>>>>>>>> +            if (ret)
>>>>>>>>> +                return dev_err_probe(dev, ret, "Cannot get PHY
>>>>>>>>> clock\n");
>>>>>>>>> +            clk_set_rate(bsp_priv->clk_phy, 50000000);
>>>>>>>>> +        }
>>>>>>> I think  we should set bsp_priv->clk_phy to NULL here if we failed to
>>>>>>> get the clock.
>>>>>>>
>>>>>>> Could you try this on your board? Thank you.
>>>>>> Right, the following change also fixes this issue:
>>>>>>
>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>> index 9fc41207cc45..2d19d48be01f 100644
>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>> @@ -1415,6 +1415,8 @@ static int rk_gmac_clk_init(struct
>>>>>> plat_stmmacenet_data *plat)
>>>>>>             if (plat->phy_node) {
>>>>>>                     bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>>>>>>                     ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
>>>>>> +               if (ret)
>>>>>> +                       bsp_priv->clk_phy = NULL;
>>>>> Or just:
>>>>>
>>>>> 		clk = of_clk_get(plat->phy_node, 0);
>>>>> 		if (clk == ERR_PTR(-EPROBE_DEFER))
>>>> Do we actually need this? Maybe other devm_clk_get() before it would fail in advance.
>>> Is it the same clock as devm_clk_get()? If it is, what's the point of
>>> getting it a second time. If it isn't, then it could be a different
>>> clock which may be yet to probe.
>> It's not the same clock, but it should be use the same clock controller driver, which is the CRU on the Rockchip platform.
> Will it always be the same clock controller, including into the future
> Rockchip devices?

Hmm, I'm not sure if it will be the same for the devices that come later. I'll try adding those checks.

Re: [PATCH net-next v3] net: ethernet: stmmac: dwmac-rk: Make the clk_phy could be used for external phy

[Chaoyi Chen]

On 8/26/2025 5:34 PM, Russell King (Oracle) wrote:

> On Tue, Aug 26, 2025 at 05:29:04PM +0800, Chaoyi Chen wrote:
>> On 8/26/2025 5:25 PM, Russell King (Oracle) wrote:
>>
>>> On Tue, Aug 26, 2025 at 04:08:40PM +0800, Chaoyi Chen wrote:
>>>> Hi Russell,
>>>>
>>>> On 8/25/2025 9:37 PM, Russell King (Oracle) wrote:
>>>>> On Mon, Aug 25, 2025 at 12:53:37PM +0200, Marek Szyprowski wrote:
>>>>>> On 25.08.2025 11:57, Chaoyi Chen wrote:
>>>>>>> On 8/25/2025 3:23 PM, Marek Szyprowski wrote:
>>>>>>>> On 15.08.2025 04:35, Chaoyi Chen wrote:
>>>>>>>>> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>>>>>>
>>>>>>>>> For external phy, clk_phy should be optional, and some external phy
>>>>>>>>> need the clock input from clk_phy. This patch adds support for setting
>>>>>>>>> clk_phy for external phy.
>>>>>>>>>
>>>>>>>>> Signed-off-by: David Wu <david.wu@rock-chips.com>
>>>>>>>>> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>> Changes in v3:
>>>>>>>>> - Link to V2:
>>>>>>>>> https://lore.kernel.org/netdev/20250812012127.197-1-kernel@airkyi.com/
>>>>>>>>> - Rebase to net-next/main
>>>>>>>>>
>>>>>>>>> Changes in v2:
>>>>>>>>> - Link to V1:
>>>>>>>>> https://lore.kernel.org/netdev/20250806011405.115-1-kernel@airkyi.com/
>>>>>>>>> - Remove get clock frequency from DT prop
>>>>>>>>>
>>>>>>>>>       drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 11 +++++++----
>>>>>>>>>       1 file changed, 7 insertions(+), 4 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>>>> index ac8288301994..5d921e62c2f5 100644
>>>>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>>>>> @@ -1412,12 +1412,15 @@ static int rk_gmac_clk_init(struct
>>>>>>>>> plat_stmmacenet_data *plat)
>>>>>>>>>               clk_set_rate(plat->stmmac_clk, 50000000);
>>>>>>>>>           }
>>>>>>>>>       -    if (plat->phy_node && bsp_priv->integrated_phy) {
>>>>>>>>> +    if (plat->phy_node) {
>>>>>>>>>               bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>>>>>>>>>               ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
>>>>>>>>> -        if (ret)
>>>>>>>>> -            return dev_err_probe(dev, ret, "Cannot get PHY clock\n");
>>>>>>>>> -        clk_set_rate(bsp_priv->clk_phy, 50000000);
>>>>>>>>> +        /* If it is not integrated_phy, clk_phy is optional */
>>>>>>>>> +        if (bsp_priv->integrated_phy) {
>>>>>>>>> +            if (ret)
>>>>>>>>> +                return dev_err_probe(dev, ret, "Cannot get PHY
>>>>>>>>> clock\n");
>>>>>>>>> +            clk_set_rate(bsp_priv->clk_phy, 50000000);
>>>>>>>>> +        }
>>>>>>> I think  we should set bsp_priv->clk_phy to NULL here if we failed to
>>>>>>> get the clock.
>>>>>>>
>>>>>>> Could you try this on your board? Thank you.
>>>>>> Right, the following change also fixes this issue:
>>>>>>
>>>>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>> index 9fc41207cc45..2d19d48be01f 100644
>>>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
>>>>>> @@ -1415,6 +1415,8 @@ static int rk_gmac_clk_init(struct
>>>>>> plat_stmmacenet_data *plat)
>>>>>>             if (plat->phy_node) {
>>>>>>                     bsp_priv->clk_phy = of_clk_get(plat->phy_node, 0);
>>>>>>                     ret = PTR_ERR_OR_ZERO(bsp_priv->clk_phy);
>>>>>> +               if (ret)
>>>>>> +                       bsp_priv->clk_phy = NULL;
>>>>> Or just:
>>>>>
>>>>> 		clk = of_clk_get(plat->phy_node, 0);
>>>>> 		if (clk == ERR_PTR(-EPROBE_DEFER))
>>>> Do we actually need this? Maybe other devm_clk_get() before it would fail in advance.
>>> Is it the same clock as devm_clk_get()? If it is, what's the point of
>>> getting it a second time. If it isn't, then it could be a different
>>> clock which may be yet to probe.
>> It's not the same clock, but it should be use the same clock controller driver, which is the CRU on the Rockchip platform.
> Will it always be the same clock controller, including into the future
> Rockchip devices?

Hmm, I'm not sure if it will be the same for the devices that come later. I'll try adding those checks.


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip