RE: [PATCH net-next v5 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

[Biju Das <biju.das.jz@xxxxxxxxxxxxxx>]

Hi All,

> -----Original Message-----
> From: Lad, Prabhakar <prabhakar.csengg@xxxxxxxxx>
> Sent: 15 April 2025 13:33
> Subject: Re: [PATCH net-next v5 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH
> 
> Hi Russell,
> 
> On Mon, Apr 14, 2025 at 7:14 PM Lad, Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> >
> > Hi Russell,
> >
> > On Mon, Apr 14, 2025 at 5:57 PM Russell King (Oracle)
> > <linux@xxxxxxxxxxxxxxx> wrote:
> > >
> > > On Mon, Apr 07, 2025 at 01:03:17PM +0100, Prabhakar wrote:
> > > > +     gbeth->rstc = devm_reset_control_get_exclusive(dev, NULL);
> > > > +     if (IS_ERR(gbeth->rstc))
> > > > +             return PTR_ERR(gbeth->rstc);
> > > > +
> > > > +     gbeth->dev = dev;
> > > > +     gbeth->regs = stmmac_res.addr;
> > > > +     gbeth->plat_dat = plat_dat;
> > > > +     plat_dat->bsp_priv = gbeth;
> > > > +     plat_dat->set_clk_tx_rate = stmmac_set_clk_tx_rate;
> > > > +     plat_dat->clks_config = renesas_gbeth_clks_config;
> > > > +     plat_dat->flags |= STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY |
> > > > +                        STMMAC_FLAG_EN_TX_LPI_CLK_PHY_CAP |
> > > > +                        STMMAC_FLAG_SPH_DISABLE;
> > > > +
> > > > +     err = renesas_gbeth_clks_config(gbeth, true);
> > > > +     if (err)
> > > > +             return err;
> > > > +
> > > > +     err = stmmac_dvr_probe(dev, plat_dat, &stmmac_res);
> > > > +     if (err)
> > > > +             renesas_gbeth_clks_config(gbeth, false);
> > > > +
> > > > +     return err;
> > > > +}
> > > > +
> > > > +static void renesas_gbeth_remove(struct platform_device *pdev) {
> > > > +     stmmac_dvr_remove(&pdev->dev);
> > > > +
> > > > +     renesas_gbeth_clks_config(get_stmmac_bsp_priv(&pdev->dev),
> > > > +false); }
> > >
> > > Would calling renesas_gbeth_clks_config() in the suspend/resume
> > > paths cause problems?
> > >
> > > If not, please consider using plat_dat->init() and plat_dat->exit()
> > > to control these clocks, and then use devm_stmmac_pltfr_probe()
> > > which will call the ->init and ->exit functions around the probe as
> > > necessary and at removal time (and you won't need the remove
> > > method.)
> > >
> 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.

[1] https://lore.kernel.org/all/20250417084015.74154-4-prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx/

static int renesas_gbeth_init(struct platform_device *pdev, void *priv)
 {
+       struct net_device *ndev = platform_get_drvdata(pdev);
        struct plat_stmmacenet_data *plat_dat;
        struct renesas_gbeth *gbeth = priv;
        int ret;
@@ -50,6 +52,11 @@ static int renesas_gbeth_init(struct platform_device *pdev, void *priv)
        if (ret)
                reset_control_assert(gbeth->rstc);
 
+       if (gbeth->suspend) {
+               gbeth->suspend = false;
+               phy_init_hw(ndev->phydev);
+       }
+
        return ret;
 }
 
@@ -66,6 +73,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)
@@ -136,6 +145,7 @@ static struct platform_driver renesas_gbeth_driver = {
        .probe  = renesas_gbeth_probe,
        .driver = {
                .name           = "renesas-gbeth",
+               .pm             = &stmmac_pltfr_pm_ops,
                .of_match_table = renesas_gbeth_match,
        },
 };

Cheers,
Biju