[U-Boot] [PATCH] drivers: net: phy: atheros: apply the previous register setting for AR8031 to fix the voltage peak issue
Ken.Lin
ken.lin at advantech.com
Mon Feb 6 17:36:19 UTC 2017
> -----Original Message-----
> From: Sekhar Nori [mailto:nsekhar at ti.com]
> Sent: Monday, February 6, 2017 12:53 AM
> To: Ken.Lin; ken <Ken Lin>; sbabic at denx.de
> Cc: u-boot at lists.denx.de; martin.donnelly at ge.com; Peter.Chiang; Peter.Stretz;
> akshay.bhat at timesys.com; joe.hershberger at ni.com;
> albert.u.uboot at aribaud.net
> Subject: Re: [PATCH] drivers: net: phy: atheros: apply the previous register
> setting for AR8031 to fix the voltage peak issue
>
> On Saturday 04 February 2017 12:26 AM, Ken.Lin wrote:
> >
> >
> >> -----Original Message-----
> >> From: Sekhar Nori [mailto:nsekhar at ti.com]
> >> Sent: Thursday, February 2, 2017 9:44 PM
> >> To: ken <Ken Lin>; sbabic at denx.de
> >> Cc: u-boot at lists.denx.de; martin.donnelly at ge.com; Ken.Lin;
> >> Peter.Chiang; Peter.Stretz; akshay.bhat at timesys.com;
> >> joe.hershberger at ni.com; albert.u.uboot at aribaud.net
> >> Subject: Re: [PATCH] drivers: net: phy: atheros: apply the previous
> >> register setting for AR8031 to fix the voltage peak issue
> >>
> >> On Friday 03 February 2017 02:53 AM, ken <Ken Lin> wrote:
> >>> Apply the previous setting for the reserved bits in SetDes Test and
> >>> System Mode Control register to avoid the voltage peak issue while
> >>> we do the IEEE PHY comformance test
> >>>
> >>> Signed-off-by: ken Lin <ken.lin at advantech.com> Tested on Advantech
> >>> DMS-BA16 board
> >>> Tested-by: Ken Lin <ken.lin at advantech.com>
> >>> ---
> >>> drivers/net/phy/atheros.c | 6 ++++++
> >>> 1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
> >>> index b34cdd3d87..82fe228604 100644
> >>> --- a/drivers/net/phy/atheros.c
> >>> +++ b/drivers/net/phy/atheros.c
> >>> @@ -28,6 +28,8 @@ static int ar8021_config(struct phy_device
> >>> *phydev)
> >>>
> >>> static int ar8031_config(struct phy_device *phydev) {
> >>> + int regval;
> >>> +
> >>> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> >>> phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
> >>> phy_write(phydev, MDIO_DEVAD_NONE,
> >> AR803x_PHY_DEBUG_ADDR_REG, @@
> >>> -44,6 +46,10 @@ static int ar8031_config(struct phy_device *phydev)
> >>> AR803x_RGMII_RX_CLK_DLY);
> >>> }
> >>>
> >>> + phy_write(phydev, MDIO_DEVAD_NONE,
> >> AR803x_PHY_DEBUG_ADDR_REG, AR803x_DEBUG_REG_5);
> >>> + regval = phy_read(phydev, MDIO_DEVAD_NONE,
> >> AR803x_PHY_DEBUG_DATA_REG);
> >>> + phy_write(phydev, MDIO_DEVAD_NONE,
> >> AR803x_PHY_DEBUG_DATA_REG,
> >>> + regval | 0x3C47);
> >>
> >> I thought I had responded to this, but probably the mail did not go through.
> >>
> >> What you have defeats the purpose of reading back the register. The
> >> idea of reading back was to avoid using any hardcoded reset state of the
> register.
> >>
> >> Also, your patch does nothing to the existing code writing to
> >> AR803x_DEBUG_REG_5.
> >> So you have two unneeded writes now. Does the following work for you
> >> (not compiled or tested).
> >>
> >
> > The register setting would turn out to be 0x3D47 on our project boards and
> our signal measurement results show the patch (v2 version,
> https://patchwork.ozlabs.org/patch/723461/)) could fix the voltage peak issue.
> > The proposed solution is to follow the implementation in previous commits,
> which include the reserved bits settings in SerDes Test and System Mode Control
> register.
>
> So what does the register setting turn out to be with my patch below ?
>
> What are the "previous commits" you refer to ?
>
When I dig around the commit history, I could find the related config change info for AR8031.
The original setting for the reserved bits in SerDes Test and System Mode Control register are not 0 (0x3C47).
commit d584c68ce0f5bf2f430ccfb2ba00bd506206fb91
Author: Fabio Estevam <fabio.estevam at nxp.com>
Date: Tue Jan 5 17:02:52 2016 -0200
phy: atheros: Use ar8035_config for AR8031
Commit 08ad9b068afb88 (" ar8031: modify the config func of ar8031 to
ar8021_config") selected 'ar8021_config' as the configuration function
for AR8031.
The correct would be to use 'ar8035_config' instead as AR8031/AR8035
have the same programming model and even share the same phy driver
in the linux kernel: drivers/net/phy/at803x.c.
Tested on a mx6qsabresd and wandboard, which now can work without
any PHY setup code in the board files.
Signed-off-by: Fabio Estevam <fabio.estevam at nxp.com>
Acked-by: Joe Hershberger <joe.hershberger at ni.com>
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index d509e30..ba57b1a 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -51,7 +51,7 @@ static struct phy_driver AR8031_driver = {
.uid = 0x4dd074,
.mask = 0xffffffef,
.features = PHY_GBIT_FEATURES,
- .config = ar8021_config,
+ .config = ar8035_config,
.startup = genphy_startup,
.shutdown = genphy_shutdown,
};
commit ce412b79e72557702185443501478646a938b5fe
Author: Mugunthan V N <mugunthanvnm at ti.com>
Date: Thu Oct 13 19:33:36 2016 +0530
drivers: net: phy: atheros: add separate config for AR8031
In the current driver implementation, config() callback is common
for AR8035 and AR8031 phy. In config() callback, driver tries to
configure MMD Access Control Register and MMD Access Address Data
Register unconditionally for both phy versions which leads to
auto negotiation failure in AM335x EVMsk second port which uses
AR8031 Giga bit RGMII phy. Fixing this by adding separate config
for AR8031 phy.
Reviewed-by: Sekhar Nori <nsekhar at ti.com>
Signed-off-by: Mugunthan V N <mugunthanvnm at ti.com>
Acked-by: Joe Hershberger <joe.hershberger at ni.com>
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 694a338..b34cdd3 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -8,6 +8,15 @@
*/
#include <phy.h>
+#define AR803x_PHY_DEBUG_ADDR_REG 0x1d
+#define AR803x_PHY_DEBUG_DATA_REG 0x1e
+
+#define AR803x_DEBUG_REG_5 0x5
+#define AR803x_RGMII_TX_CLK_DLY 0x100
+
+#define AR803x_DEBUG_REG_0 0x0
+#define AR803x_RGMII_RX_CLK_DLY 0x8000
+
static int ar8021_config(struct phy_device *phydev)
{
phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);
@@ -17,6 +26,32 @@ static int ar8021_config(struct phy_device *phydev)
return 0;
}
+static int ar8031_config(struct phy_device *phydev)
+{
+ if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
+ phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
+ phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
+ AR803x_DEBUG_REG_5);
+ phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
+ AR803x_RGMII_TX_CLK_DLY);
+ }
+
+ if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
+ phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
+ phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
+ AR803x_DEBUG_REG_0);
+ phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
+ AR803x_RGMII_RX_CLK_DLY);
+ }
+
+ phydev->supported = phydev->drv->features;
+
+ genphy_config_aneg(phydev);
+ genphy_restart_aneg(phydev);
+
+ return 0;
+}
+
When I checked the NXP u-boot codebase (git://git.freescale.com/imx/uboot-imx.git, branch l5.1.1_2.1.0-ga), It seems NXP worked with the PHY vendors and set the reserved bits to 0x3D47.
commit 9082eeac5de1335d663016668c9b89c290f5c79b
Author: Andy Fleming <afleming at freescale.com>
Date: Thu Apr 7 21:56:05 2011 -0500
phylib: Add a bunch of PHY drivers from tsec
The tsec driver had a bunch of PHY drivers already written. This
converts them all into PHY Lib drivers, and serves as the first
set of PHY drivers for PHY Lib.
While doing that, cleaned up a number of magic numbers (though
not all of them, as PHY vendors like to keep their numbers as
magical as possible). Also, noticed that almost all of the
vitesse/cicada PHYs had the same config/parse/startup functions,
so those have been collapsed into one.
Signed-off-by: Andy Fleming <afleming at freescale.com>
Signed-off-by: Kumar Gala <galak at kernel.crashing.org>
Acked-by: Detlev Zundel <dzu at denx.de>
+/*
+ * Atheros PHY drivers
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ *
+ * Copyright 2011 Freescale Semiconductor, Inc.
+ * author Andy Fleming
+ *
+ */
+#include <phy.h>
+
+static int ar8021_config(struct phy_device *phydev)
+{
+ phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);
+ phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x3D47);
+
+ return 0;
+}
+
+struct phy_driver AR8021_driver = {
+ .name = "AR8021",
+ .uid = 0x4dd040,
+ .mask = 0xfffff0,
+ .features = PHY_GBIT_FEATURES,
+ .config = ar8021_config,
+ .startup = genphy_startup,
+ .shutdown = genphy_shutdown,
+};
+
+int phy_atheros_init(void)
+{
+ phy_register(&AR8021_driver);
+
+ return 0;
+}
commit 626ee1e32eeb4fc89e0f406d6067ed6e71d8302f
Author: Shengzhou Liu <Shengzhou.Liu at freescale.com>
Date: Thu Aug 8 16:33:35 2013 +0800
phylib: update atheros ar803x phy
As AR8031 and AR8033 have same PHY ID 0x4dd074, they use the
common driver. Currently AR8031_driver didn't work for AR8033,
hence updated it to have it work on AR8031/AR8033.
Signed-off-by: Shengzhou Liu <Shengzhou.Liu at freescale.com>
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 0f2dfd6..7a1453f 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -48,11 +48,11 @@ static struct phy_driver AR8021_driver = {
};
static struct phy_driver AR8031_driver = {
- .name = "AR8031",
+ .name = "AR8031/AR8033",
.uid = 0x4dd074,
.mask = 0xfffff0,
.features = PHY_GBIT_FEATURES,
- .config = genphy_config,
+ .config = ar8021_config,
.startup = genphy_startup,
.shutdown = genphy_shutdown,
};
> >> Thanks,
> >> Sekhar
> >>
> >> ---8<---
> >> diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
> >> index
> >> b34cdd3d87dc..aff9eb3d18c6 100644
> >> --- a/drivers/net/phy/atheros.c
> >> +++ b/drivers/net/phy/atheros.c
> >> @@ -28,12 +28,16 @@ static int ar8021_config(struct phy_device
> >> *phydev)
> >>
> >> static int ar8031_config(struct phy_device *phydev) {
> >> + int regval;
> >> +
> >> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
> >> phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
> >> phy_write(phydev, MDIO_DEVAD_NONE,
> AR803x_PHY_DEBUG_ADDR_REG,
> >> AR803x_DEBUG_REG_5);
> >> + regval = phy_read(phydev, MDIO_DEVAD_NONE,
> >> + AR803x_PHY_DEBUG_DATA_REG);
> >> phy_write(phydev, MDIO_DEVAD_NONE,
> AR803x_PHY_DEBUG_DATA_REG,
> >> - AR803x_RGMII_TX_CLK_DLY);
> >> + regval | AR803x_RGMII_TX_CLK_DLY);
> >> }
> >>
> >> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
>
> Thanks,
> Sekhar
>
>
> --
> This message has been scanned for viruses and dangerous content by
> MailScanner, and is believed to be clean.
--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.
More information about the U-Boot
mailing list