Hi Andrew,
> -----Original Message-----
> From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Sent: 21 April 2025 18:50
> Subject: RE: [PATCH net-next v5 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH
>
> Hi Andrew,
>
> > -----Original Message-----
> > From: Biju Das
> > Sent: 21 April 2025 18:26
> > Subject: RE: [PATCH net-next v5 3/3] net: stmmac: Add DWMAC glue layer
> > for Renesas GBETH
> >
> > Hi Andrew,
> >
> > > -----Original Message-----
> > > From: Andrew Lunn <andrew@xxxxxxx>
> > > Sent: 21 April 2025 16:12
> > > Subject: Re: [PATCH net-next v5 3/3] net: stmmac: Add DWMAC glue
> > > layer for Renesas GBETH
> > >
> > > On Mon, Apr 21, 2025 at 02:22:01PM +0000, Biju Das wrote:
> > > > Hi Andrew,
> > > >
> > > > > -----Original Message-----
> > > > > From: Andrew Lunn <andrew@xxxxxxx>
> > > > > Sent: 21 April 2025 15:02
> > > > > Subject: Re: [PATCH net-next v5 3/3] net: stmmac: Add DWMAC glue
> > > > > layer for Renesas GBETH
> > > > >
> > > > > > > On the RZ/G3E, the upstream support for testing S2R is not
> > > > > > > yet in a usable state. So for now, I'll switch to using
> > > > > > > init/exit callbacks and drop the PM
> > > callback.
> > > > > >
> > > > > > FYI, On RZ/G3E, for STR to work with mainline, we need to reinitialize the PHY.
> > > > > > I have done below changes on top of [1] to make STR working.
> > > > >
> > > > > Can you explain why you need to reinitialise the PHY? The MAC
> > > > > driver should not need to do this, so something is wrong
> > > > > somewhere. If we understand the 'Why?' we can probably tell you a better way to do this.
> > > >
> > > > Without this change bind/unbind works. But for the STR case,
> > > > without reinitializing the PHY, even though the IP link is UP, I
> > > > am not able to talk the NFS server or ping
> > > the host properly.
> > > >
> > > > I checked clock/reset before and after reset everything set as expected.
> > > >
> > > > Only change during STR is, on wakeup we need to restore direction
> > > > (MII/RGMII) of IO block for ET0/1_TXC_TXCLK (IO attribute) in the
> > > > pin control driver. After that looks like PHY init is required to talk to server.
> > >
> > > When talking about suspend/resume, is this with or without WoL?
> >
> > Without WoL.
> >
> > >
> > > If WoL is enabled for the PHY, the PHY is left running while the
> > > system is suspended, and so all its clocks and reset lines also need
> > > to be left enabled etc. On resume, there should not be any need to touch the PHY.
> >
> > OK.
> >
> > >
> > > If WoL is not enabled in the PHY, it should get powered off. On
> > > resume, phylib should be reinitializing the PHY.
> >
> > OK.
> >
> > >
> > > Which of these two cases need the reinitialisation?
> > >
> > > The reasons i can think of for the PHY needing a reinitialization are:
> > >
> > > 1) It lost power when it did not expect to loose power
> > > 2) Got reset when it did not expect to be reset
> > > 3) Clock not ticking when it should of been ticking.
> >
> > OK.
> >
> > >
> > > So you cannot just check clock/reset before and after, you need to
> > > look at the order of events. The loss of power, or a reset after
> > > phylib resumed the PHY would not be good. Similarly, if the needed
> > > clocks are not ticking while
> > phylib resumes it would also not be good.
> > >
> > > I would also suggest you check the datasheet for the PHY, and does
> > > it document anything about the clock input TXC_TXCLK is connected to?
> >
> > It is connected to TX_CTL on micrel phy.
> >
> > > Does it need to be ticking while configuring the PHY? Any action
> > > which needs to be taken after this starts ticking? Is the pinctrl
> > > resume being called before the PHY
> > resume?
> >
> > Pinctrl resume is called before PHY resume
> >
> > Previously STR did not work, because of not restoring direction
> > (MII/RGMII) of IO block for ET0/1_TXC_TXCLK (IO attribute) .Because of
> > this, the direction of IO block is set to IN (input) MII mode instead of OUT(output) RGMII mode.
> >
> > Now everything works. Thanks for your detailed pointers.
>
> I need to debug this issue further as without reinitializing the PHY, the STR wakeup is not stable
> (Linkup, but not able to contact the server).
Looks like the issue is timing related. By comparing the working and non-working logs
On Working case, PHY resume happens after proper initialization of gbeth.
whereas on non-working case, 2 times PHY resume is called, one before proper initialization
and other after that.
Working case logs: code change, see[1]
--------------------------------------
[ 33.928696] CPU3: Booted secondary processor 0x0000000300 [0x412fd050]
[ 33.928696] CPU3 is up
[ 33.928696] ########rzg2l_pinctrl_resume_noirq 3216
[ 33.928696] ########renesas_gbeth_init 55
[ 33.928696] dwmac4: Master AXI performs fixed burst length
[ 33.928696] renesas-gbeth 15c30000.ethernet eth0: No Safety Features support found
[ 33.928696] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008 Advanced Timestamp supported
[ 33.928696] renesas-gbeth 15c30000.ethernet eth0: configuring for phy/rgmii-id link mode
[ 33.928696] ########kszphy_resume 2181
[ 33.928696] OOM killer enabled.
[ 33.928696] Restarting tasks ... done.
root@smarc-rzg3e[ 33.928696] random: crng reseeded on system resumption
:~# [ 33.928696] PM: suspend exit
root@smarc-rzg3e:~# [ 33.928696] renesas-gbeth 15c30000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
root@smarc-rzg3e:~# [ 33.928696] mmc2: Skipping voltage switch
root@smarc-rzg3e:~# ping 192.168.10.1
PING 192.168.10.1 (192.168.10.1) 56(84) bytes of data.
64 bytes from 192.168.10.1: icmp_seq=1 ttl=64 time=0.381 ms
^C
--- 192.168.10.1 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 0.381/0.381/0.381/0.000 ms
root@smarc-rzg3e:~#
No working case logs: code change, see[2]
-----------------------------------------
[ 42.485978] CPU3: Booted secondary processor 0x0000000300 [0x412fd050]
[ 42.485978] CPU3 is up
[ 42.485978] ########rzg2l_pinctrl_resume_noirq 3216
[ 42.485978] ########renesas_gbeth_init 55
[ 42.485978] ########kszphy_resume 2181
[ 42.485978] dwmac4: Master AXI performs fixed burst length
[ 42.485978] renesas-gbeth 15c30000.ethernet eth0: No Safety Features support found
[ 42.485978] renesas-gbeth 15c30000.ethernet eth0: IEEE 1588-2008 Advanced Timestamp supported
[ 42.485978] renesas-gbeth 15c30000.ethernet eth0: configuring for phy/rgmii-id link mode
[ 42.485978] ########kszphy_resume 2181
[ 42.485978] OOM killer enabled.
[ 42.485978] Restarting tasks ... done.
[ 42.485978] random: crng reseeded on system resumption
[ 42.485978] PM: suspend exit
root@smarc-rzg3e:~# [ 42.485978] renesas-gbeth 15c30000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
root@smarc-rzg3e:~# [ 42.485978] mmc2: Skipping voltage switch
ping 192.168.10.1
[ 42.485978] nfs: server 192.168.10.1 not responding, still trying
[1]
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c
index df4ca897a60c..021f34116a4f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c
@@ -25,6 +25,8 @@ struct renesas_gbeth {
struct plat_stmmacenet_data *plat_dat;
struct reset_control *rstc;
struct device *dev;
+
+ bool suspend;
};
static const char *const renesas_gbeth_clks[] = {
@@ -49,6 +51,14 @@ static int renesas_gbeth_init(struct platform_device *pdev, void *priv)
plat_dat->clks);
if (ret)
reset_control_assert(gbeth->rstc);
+
+ pr_err("########%s %d",__func__,__LINE__);
+ if (gbeth->suspend) {
+ struct net_device *ndev = platform_get_drvdata(pdev);
+
+ gbeth->suspend = false;
+ phy_init_hw(ndev->phydev);
+ }
return ret;
}
@@ -66,6 +76,8 @@ static void renesas_gbeth_exit(struct platform_device *pdev, void *priv)
ret = reset_control_assert(gbeth->rstc);
if (ret)
dev_err(gbeth->dev, "Reset assert failed\n");
+
+ gbeth->suspend = true;
}
static int renesas_gbeth_probe(struct platform_device *pdev)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 71fb4410c31b..741b5a7f7b93 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -2178,6 +2178,8 @@ static int kszphy_resume(struct phy_device *phydev)
phydev->drv->config_intr(phydev);
}
+ pr_err("########%s %d",__func__,__LINE__);
+
return 0;
}
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 4ea798cf78cf..e94f3fa7bb64 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -3213,6 +3213,7 @@ static int rzg2l_pinctrl_resume_noirq(struct device *dev)
int i;
int ret;
+ pr_err("########%s %d",__func__,__LINE__);
if (!atomic_read(&pctrl->wakeup_path)) {
ret = clk_prepare_enable(pctrl->clk);
if (ret)
[2]
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c
index df4ca897a60c..c31c6c715818 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-renesas-gbeth.c
@@ -25,6 +25,8 @@ struct renesas_gbeth {
struct plat_stmmacenet_data *plat_dat;
struct reset_control *rstc;
struct device *dev;
+
+ bool suspend;
};
static const char *const renesas_gbeth_clks[] = {
@@ -49,6 +51,14 @@ static int renesas_gbeth_init(struct platform_device *pdev, void *priv)
plat_dat->clks);
if (ret)
reset_control_assert(gbeth->rstc);
+
+ pr_err("########%s %d",__func__,__LINE__);
+ //if (gbeth->suspend) {
+ // struct net_device *ndev = platform_get_drvdata(pdev);
+
+ // gbeth->suspend = false;
+ // phy_init_hw(ndev->phydev);
+ //}
return ret;
}
@@ -66,6 +76,8 @@ static void renesas_gbeth_exit(struct platform_device *pdev, void *priv)
ret = reset_control_assert(gbeth->rstc);
if (ret)
dev_err(gbeth->dev, "Reset assert failed\n");
+
+ gbeth->suspend = true;
}
static int renesas_gbeth_probe(struct platform_device *pdev)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 71fb4410c31b..741b5a7f7b93 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -2178,6 +2178,8 @@ static int kszphy_resume(struct phy_device *phydev)
phydev->drv->config_intr(phydev);
}
+ pr_err("########%s %d",__func__,__LINE__);
+
return 0;
}
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index 4ea798cf78cf..e94f3fa7bb64 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -3213,6 +3213,7 @@ static int rzg2l_pinctrl_resume_noirq(struct device *dev)
int i;
int ret;
+ pr_err("########%s %d",__func__,__LINE__);
if (!atomic_read(&pctrl->wakeup_path)) {
ret = clk_prepare_enable(pctrl->clk);
if (ret)
