From: Stefan Roese <sr@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] net: phy: do not read configuration register on reset
Date: Tue, 9 Feb 2016 19:42:25 +0100 [thread overview]
Message-ID: <56BA3311.30407@denx.de> (raw)
In-Reply-To: <1449688885-17037-1-git-send-email-stefan@agner.ch>
On 09.12.2015 20:21, Stefan Agner wrote:
> When doing a software reset, the reset flag should be written without
> other bits set. Writing the current state will lead to restoring the
> state of the PHY (e.g. Powerdown), which is not what is expected from
> a software reset.
>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
> This lead to the PHY staying powered down on a reboot when using a
> Micrel PHY and not using hardware reset...
>
> It also aligns with the behavior of Linux:
> http://lxr.free-electrons.com/source/drivers/net/phy/phy_device.c#L1122
>
> --
> Stefan
>
> drivers/net/phy/phy.c | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 51b5746..ef9f13b 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -717,15 +717,7 @@ int phy_reset(struct phy_device *phydev)
> }
> #endif
>
> - reg = phy_read(phydev, devad, MII_BMCR);
> - if (reg < 0) {
> - debug("PHY status read failed\n");
> - return -1;
> - }
> -
> - reg |= BMCR_RESET;
> -
> - if (phy_write(phydev, devad, MII_BMCR, reg) < 0) {
> + if (phy_write(phydev, devad, MII_BMCR, BMCR_RESET) < 0) {
> debug("PHY reset failed\n");
> return -1;
> }
> @@ -738,6 +730,7 @@ int phy_reset(struct phy_device *phydev)
> * auto-clearing). This should happen within 0.5 seconds per the
> * IEEE spec.
> */
> + reg = phy_read(phydev, devad, MII_BMCR);
> while ((reg & BMCR_RESET) && timeout--) {
> reg = phy_read(phydev, devad, MII_BMCR);
This breaks PHY link auto negotiation support on Marvell Armada
388 ClearFog for me. As with this patch, bit 12 (A/N enable) will
get cleared. And the link is not established correctly.
The problem here seems to be, that the Marvell PHY driver calls
phy_reset() again at the end of m88e1111s_config(). Resulting
in the loss of the configuration of the MII_BMCR register. I'm
currently thinking if this patchbelow would be the correct fix
for this problem?
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index eab1558..8ede927 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -270,8 +270,7 @@ static int m88e1111s_config(struct phy_device *phydev)
printf("%s: phy soft reset timeout\n", __func__);
genphy_config_aneg(phydev);
-
- phy_reset(phydev);
+ genphy_restart_aneg(phydev);
Thanks,
Stefan
prev parent reply other threads:[~2016-02-09 18:42 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-09 19:21 [U-Boot] [PATCH] net: phy: do not read configuration register on reset Stefan Agner
2015-12-09 19:32 ` Michael Welling
2015-12-10 0:52 ` Bin Meng
2015-12-10 20:28 ` Joe Hershberger
2016-01-29 21:25 ` [U-Boot] " Joe Hershberger
2016-01-29 21:26 ` Joe Hershberger
2016-02-09 18:42 ` Stefan Roese [this message]
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=56BA3311.30407@denx.de \
--to=sr@denx.de \
--cc=u-boot@lists.denx.de \
/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.