2022-11-14 05:15:39

by Xiaolei Wang

Subject: [PATCH] net: fec: Create device link between fec0 and fec1

On imx6sx, there are two fec interfaces, but the external
phys can only be configured by fec0 mii_bus. That means
the fec1 can't work independently, it only work when the
fec0 is active. It is alright in the normal boot since the
fec0 will be probed first. But then the fec0 maybe moved
behind of fec1 in the dpm_list due to various device link.
So in system suspend and resume, we would get the following
warning when configuring the external phy of fec1 via the
fec0 mii_bus due to the inactive of fec0. In order to fix
this issue, we create a device link between fec0 and fec1.
This will make sure that fec0 is always active when fec1
is in active mode.

WARNING: CPU: 0 PID: 24 at drivers/net/phy/phy.c:983 phy_error+0x20/0x68
Modules linked in:
CPU: 0 PID: 24 Comm: kworker/0:2 Not tainted 6.1.0-rc3-00011-g5aaef24b5c6d-dirty #34
Hardware name: Freescale i.MX6 SoloX (Device Tree)
Workqueue: events_power_efficient phy_state_machine
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x68/0x90
dump_stack_lvl from __warn+0xb4/0x24c
__warn from warn_slowpath_fmt+0x5c/0xd8
warn_slowpath_fmt from phy_error+0x20/0x68
phy_error from phy_state_machine+0x22c/0x23c
phy_state_machine from process_one_work+0x288/0x744
process_one_work from worker_thread+0x3c/0x500
worker_thread from kthread+0xf0/0x114
kthread from ret_from_fork+0x14/0x28
Exception stack(0xf0951fb0 to 0xf0951ff8)

Fixes: 2f664823a470 ("net: phy: at803x: add device tree binding")
Signed-off-by: Xiaolei Wang <[email protected]>
---
drivers/net/ethernet/freescale/fec_main.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index f623c12eaf95..aecbda9aca65 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3963,7 +3963,19 @@ fec_probe(struct platform_device *pdev)
goto failed_stop_mode;

phy_node = of_parse_phandle(np, "phy-handle", 0);
- if (!phy_node && of_phy_is_fixed_link(np)) {
+ if (phy_node) {
+ struct phy_device *phy_dev = of_phy_find_device(phy_node);
+ struct device *fec_dev = phy_dev ? phy_dev->mdio.bus->parent : NULL;
+ /* The external phy used by current fec interface is managed
+ * by another fec interface, so we should create a device link
+ * between them.
+ */
+ if (fec_dev && &pdev->dev != fec_dev)
+ device_link_add(&pdev->dev, fec_dev,
+ DL_FLAG_PM_RUNTIME);
+ if (phy_dev)
+ put_device(&phy_dev->mdio.dev);
+ } else if (of_phy_is_fixed_link(np)) {
ret = of_phy_register_fixed_link(np);
if (ret < 0) {
dev_err(&pdev->dev,
--
2.25.1

2022-11-14 13:32:59

by Andrew Lunn

Subject: Re: [PATCH] net: fec: Create device link between fec0 and fec1

On Mon, Nov 14, 2022 at 12:11:43PM +0800, Xiaolei Wang wrote:
> On imx6sx, there are two fec interfaces, but the external
> phys can only be configured by fec0 mii_bus. That means
> the fec1 can't work independently, it only work when the
> fec0 is active. It is alright in the normal boot since the
> fec0 will be probed first. But then the fec0 maybe moved
> behind of fec1 in the dpm_list due to various device link.
> So in system suspend and resume, we would get the following
> warning when configuring the external phy of fec1 via the
> fec0 mii_bus due to the inactive of fec0. In order to fix
> this issue, we create a device link between fec0 and fec1.
> This will make sure that fec0 is always active when fec1
> is in active mode.

I'm wondering if this should be more generic? I have seen this setup
more frequently on the FEC, but there are other dual MAC SoCs which
can have a similar setup, two PHYs sharing one MDIO bus.

Can this be pushed into phylib?

Andrew