All of lore.kernel.org
 help / color / mirror / Atom feed
From: leonard.crestez@nxp.com (Leonard Crestez)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] net: phy: micrel: Restore led_mode and clk_sel on resume
Date: Wed, 31 May 2017 01:08:00 +0300	[thread overview]
Message-ID: <1496182080.19669.11.camel@nxp.com> (raw)
In-Reply-To: <44ddeaeb-3e23-3d88-cc5c-9ed290507648@gmail.com>

On Tue, 2017-05-30 at 11:05 -0700, Florian Fainelli wrote:
> On 05/30/2017 10:34 AM, Leonard Crestez wrote:
> > These bits seem to be lost after a suspend/resume cycle so just set them
> > again.
> > 
> > This patch fixes ethernet suspend/resume on imx6ul-14x14-evk boards.
> > 
> > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> > ---
> >  drivers/net/phy/micrel.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> > index 6a5fd18..c53ee17 100644
> > --- a/drivers/net/phy/micrel.c
> > +++ b/drivers/net/phy/micrel.c
> > @@ -700,6 +700,9 @@ static int kszphy_suspend(struct phy_device *phydev)
> >  
> >  static int kszphy_resume(struct phy_device *phydev)
> >  {
> > +	struct kszphy_priv *priv = phydev->priv;
> > +	int ret;
> > +
> >  	genphy_resume(phydev);
> >  
> >  	/* Enable PHY Interrupts */
> > @@ -709,6 +712,18 @@ static int kszphy_resume(struct phy_device *phydev)
> >  			phydev->drv->config_intr(phydev);
> >  	}
> >  
> > +	if (priv->rmii_ref_clk_sel) {
> > +		ret = kszphy_rmii_clk_sel(phydev, priv->rmii_ref_clk_sel_val);
> > +		if (ret) {
> > +			phydev_err(phydev,
> > +				   "failed to set rmii reference clock\n");
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	if (priv->led_mode >= 0)
> > +		kszphy_setup_led(phydev, priv->type->led_mode_reg, priv->led_mode);
> 
> Should not we actually call kszphy_config_init() in order to restore
> broadcast and nand disable bits as well?

I don't know. In my case the B_CAST_OFF bit doesn't seem to be lost and
NAND_TREE_ON is already off by the time it gets to linux.

The bit that get lost seem to disappear just as the phy is resumed. I
added some prints and they look like this:

PM: early resume of devices complete after 6.534 msecs
begin resume
0x1F=0x8190 0x16=0x202
after genphy_resume 0x1F=0x8100 0x16=0x202
end
resume 0x1F=0x8190 0x16=0x202

> If not, I would be more comfortable if we did create a specific function
> that takes care of setting the reference clock and LED mode.

Ok, I can add a function called kszphy_config_reset() with a comment
explaining it's for bits lost on reset/resume.

Or perhaps a better option would be to just save/restore the entire
0x1F register?

WARNING: multiple messages have this Message-ID (diff)
From: Leonard Crestez <leonard.crestez@nxp.com>
To: Florian Fainelli <f.fainelli@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>, Andy Duan <fugang.duan@nxp.com>
Cc: Johan Hovold <johan@kernel.org>,
	Alexandre Belloni <alexandre.belloni@free-electrons.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Fabio Estevam <fabio.estevam@nxp.com>, <netdev@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>, Shawn Guo <shawnguo@kernel.org>
Subject: Re: [PATCH 2/2] net: phy: micrel: Restore led_mode and clk_sel on resume
Date: Wed, 31 May 2017 01:08:00 +0300	[thread overview]
Message-ID: <1496182080.19669.11.camel@nxp.com> (raw)
In-Reply-To: <44ddeaeb-3e23-3d88-cc5c-9ed290507648@gmail.com>

On Tue, 2017-05-30 at 11:05 -0700, Florian Fainelli wrote:
> On 05/30/2017 10:34 AM, Leonard Crestez wrote:
> > These bits seem to be lost after a suspend/resume cycle so just set them
> > again.
> > 
> > This patch fixes ethernet suspend/resume on imx6ul-14x14-evk boards.
> > 
> > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> > ---
> >  drivers/net/phy/micrel.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> > index 6a5fd18..c53ee17 100644
> > --- a/drivers/net/phy/micrel.c
> > +++ b/drivers/net/phy/micrel.c
> > @@ -700,6 +700,9 @@ static int kszphy_suspend(struct phy_device *phydev)
> >  
> >  static int kszphy_resume(struct phy_device *phydev)
> >  {
> > +	struct kszphy_priv *priv = phydev->priv;
> > +	int ret;
> > +
> >  	genphy_resume(phydev);
> >  
> >  	/* Enable PHY Interrupts */
> > @@ -709,6 +712,18 @@ static int kszphy_resume(struct phy_device *phydev)
> >  			phydev->drv->config_intr(phydev);
> >  	}
> >  
> > +	if (priv->rmii_ref_clk_sel) {
> > +		ret = kszphy_rmii_clk_sel(phydev, priv->rmii_ref_clk_sel_val);
> > +		if (ret) {
> > +			phydev_err(phydev,
> > +				   "failed to set rmii reference clock\n");
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	if (priv->led_mode >= 0)
> > +		kszphy_setup_led(phydev, priv->type->led_mode_reg, priv->led_mode);
> 
> Should not we actually call kszphy_config_init() in order to restore
> broadcast and nand disable bits as well?

I don't know. In my case the B_CAST_OFF bit doesn't seem to be lost and
NAND_TREE_ON is already off by the time it gets to linux.

The bit that get lost seem to disappear just as the phy is resumed. I
added some prints and they look like this:

PM: early resume of devices complete after 6.534 msecs
begin resume
0x1F=0x8190 0x16=0x202
after genphy_resume 0x1F=0x8100 0x16=0x202
end
resume 0x1F=0x8190 0x16=0x202

> If not, I would be more comfortable if we did create a specific function
> that takes care of setting the reference clock and LED mode.

Ok, I can add a function called kszphy_config_reset() with a comment
explaining it's for bits lost on reset/resume.

Or perhaps a better option would be to just save/restore the entire
0x1F register?

  reply	other threads:[~2017-05-30 22:08 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-30 17:34 [PATCH 0/2] ARM: imx6ul-14x14-evk: Fix suspend over nfs by phy cleanup Leonard Crestez
2017-05-30 17:34 ` Leonard Crestez
2017-05-30 17:34 ` [PATCH 1/2] ARM: dts: imx6ul-14x14-evk: Add ksz8081 phy properties Leonard Crestez
2017-05-30 17:34   ` Leonard Crestez
2017-05-30 17:34   ` Leonard Crestez
2017-05-30 18:10   ` Florian Fainelli
2017-05-30 18:10     ` Florian Fainelli
2017-05-30 22:20     ` Leonard Crestez
2017-05-30 22:20       ` Leonard Crestez
2017-05-31  4:08   ` Fabio Estevam
2017-05-31  4:08     ` Fabio Estevam
2017-05-30 17:34 ` [PATCH 2/2] net: phy: micrel: Restore led_mode and clk_sel on resume Leonard Crestez
2017-05-30 17:34   ` Leonard Crestez
2017-05-30 18:05   ` Florian Fainelli
2017-05-30 18:05     ` Florian Fainelli
2017-05-30 22:08     ` Leonard Crestez [this message]
2017-05-30 22:08       ` Leonard Crestez
2017-05-30 22:19       ` Florian Fainelli
2017-05-30 22:19         ` Florian Fainelli
2017-05-30 23:14         ` Leonard Crestez
2017-05-30 23:14           ` Leonard Crestez
2017-05-30 18:12   ` Fabio Estevam
2017-05-30 18:12     ` Fabio Estevam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1496182080.19669.11.camel@nxp.com \
    --to=leonard.crestez@nxp.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.