Hi Giuseppe, On Thu, Jun 16, 2016 at 6:37 AM, Giuseppe CAVALLARO <peppe.cavall...@st.com> wrote: > > Hi Vincent > > > On 6/15/2016 7:04 PM, Vincent Palatin wrote: >> >> On Sun, Jun 12, 2016 at 11:46 PM, Giuseppe CAVALLARO >> <peppe.cavall...@st.com> wrote: >>> >>> On 6/11/2016 3:00 AM, Vincent Palatin wrote: >>>> >>>> >>>> In order to support Wake-On-Lan when using the RK3288 integrated MAC >>>> (with an external RGMII PHY), we need to avoid shutting down the regulator >>>> of the external PHY when the MAC is suspended as it's currently done in >>>> the MAC >>>> platform code. >>>> As a first step, create independant callbacks for suspend/resume rather >>>> than >>>> re-using exit/init callbacks. So the dwmac platform driver can behave >>>> differently >>>> on suspend where it might skip shutting the PHY and at module unloading. >>>> Then update the dwmac-rk driver to switch off the PHY regulator only if we >>>> are >>>> not planning to wake up from the LAN. >>>> Finally add the PMT interrupt to the MAC device tree configuration, so we >>>> can >>>> wake up the core from it when the PHY has received the magic packet. >>> >>> >>> >>> IMO these could be sent for net-next and also other glue logic >>> files should be reworked in order to use the new API for coherence. >> >> >> Given they will have the same set of functions for exit/init and >> suspend/resume, you mean duplicating the callbacks like this : >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c >> @@ -359,6 +359,8 @@ static int sti_dwmac_probe(struct platform_device *pdev) >> plat_dat->bsp_priv = dwmac; >> plat_dat->init = sti_dwmac_init; >> plat_dat->exit = sti_dwmac_exit; >> + plat_dat->suspend = sti_dwmac_exit; >> + plat_dat->resume = sti_dwmac_init; >> plat_dat->fix_mac_speed = data->fix_retime_src; >> >> ret = sti_dwmac_init(pdev, plat_dat->bsp_priv); >> >> Is this anyhow useful ? > > > I think this is mandatory otherwise you are not guaranteeing the PM > stuff working on the rest of the glue-logics (not only sti); because > init/exit calls won't be called anymore.
As mentioned in the PATCH 1/3 description: "If the driver does not provide the suspend or resume callback, we fall back to the old behavior trying to use exit or init. [...]" ie. --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c @@ -411,7 +411,9 @@ static int stmmac_pltfr_suspend(struct device *dev) struct platform_device *pdev = to_platform_device(dev); ret = stmmac_suspend(dev); - if (priv->plat->exit) + if (priv->plat->suspend) + priv->plat->suspend(pdev, priv->plat->bsp_priv); + else if (priv->plat->exit) priv->plat->exit(pdev, priv->plat->bsp_priv); return ret; @@ -430,7 +432,9 @@ static int stmmac_pltfr_resume(struct device *dev) struct stmmac_priv *priv = netdev_priv(ndev); struct platform_device *pdev = to_platform_device(dev); - if (priv->plat->init) + if (priv->plat->resume) + priv->plat->resume(pdev, priv->plat->bsp_priv); + else if (priv->plat->init) priv->plat->init(pdev, priv->plat->bsp_priv); return stmmac_resume(dev); So I was under the impression that everything should continue working as before for drivers only providing init/exit, by falling back on calling ->exit() if there is no suspend() callback initialized for the PM calls. You think that won't work ? > > So I kindly ask you to > propagate the fix and send the V3. The implementation above is ok for > me. > > peppe >