| Message ID | 1548262809-19361-2-git-send-email-vladimir.oltean@nxp.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Joe Hershberger |
| Headers | show |
| Series | net: phy: allow disabling SmartEEE for Atheros PHYs | expand |
On Wed, Jan 23, 2019 at 5:46 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > --- > Changes in v2: > * Patch added in this version. > > drivers/net/phy/atheros.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c > index 3783d15..b4066e3 100644 > --- a/drivers/net/phy/atheros.c > +++ b/drivers/net/phy/atheros.c > @@ -57,11 +57,9 @@ static int ar8035_config(struct phy_device *phydev) > { > int regval; > > - phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x0007); > - phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x8016); > - phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x4007); > - regval = phy_read(phydev, MDIO_DEVAD_NONE, 0xe); > - phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018)); > + /* CLK_25M output clock select: 125 MHz */ > + regval = phy_read_mmd(phydev, MDIO_MMD_AN, 0x8016); > + phy_write_mmd(phydev, MDIO_MMD_AN, 0x8016, regval | 0x0018); I think I see how the old code accomplished the same thing. It was woefully undocumented. Can you improve on the magic 0x18 number? What about 0x8016? Is that a register whose name you know and can assign a constant? > > phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05); > regval = phy_read(phydev, MDIO_DEVAD_NONE, 0x1e); > -- > 2.7.4 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
On 1/24/19 8:16 AM, Joe Hershberger wrote: > On Wed, Jan 23, 2019 at 5:46 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: >> >> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> >> --- >> Changes in v2: >> * Patch added in this version. >> >> drivers/net/phy/atheros.c | 8 +++----- >> 1 file changed, 3 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c >> index 3783d15..b4066e3 100644 >> --- a/drivers/net/phy/atheros.c >> +++ b/drivers/net/phy/atheros.c >> @@ -57,11 +57,9 @@ static int ar8035_config(struct phy_device *phydev) >> { >> int regval; >> >> - phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x0007); >> - phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x8016); >> - phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x4007); >> - regval = phy_read(phydev, MDIO_DEVAD_NONE, 0xe); >> - phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018)); >> + /* CLK_25M output clock select: 125 MHz */ >> + regval = phy_read_mmd(phydev, MDIO_MMD_AN, 0x8016); >> + phy_write_mmd(phydev, MDIO_MMD_AN, 0x8016, regval | 0x0018); > > I think I see how the old code accomplished the same thing. It was > woefully undocumented. Can you improve on the magic 0x18 number? What > about 0x8016? Is that a register whose name you know and can assign a > constant? > Register MMD7 8016 has no name in the documentation that I can see. It is simply referred to as that. I'd rather keep it like this rather than create confusion by making up a name for it. -Vladimir
On Thu, Jan 24, 2019 at 6:35 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > > On 1/24/19 8:16 AM, Joe Hershberger wrote: > > On Wed, Jan 23, 2019 at 5:46 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote: > >> > >> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > >> --- > >> Changes in v2: > >> * Patch added in this version. > >> > >> drivers/net/phy/atheros.c | 8 +++----- > >> 1 file changed, 3 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c > >> index 3783d15..b4066e3 100644 > >> --- a/drivers/net/phy/atheros.c > >> +++ b/drivers/net/phy/atheros.c > >> @@ -57,11 +57,9 @@ static int ar8035_config(struct phy_device *phydev) > >> { > >> int regval; > >> > >> - phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x0007); > >> - phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x8016); > >> - phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x4007); > >> - regval = phy_read(phydev, MDIO_DEVAD_NONE, 0xe); > >> - phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018)); > >> + /* CLK_25M output clock select: 125 MHz */ > >> + regval = phy_read_mmd(phydev, MDIO_MMD_AN, 0x8016); > >> + phy_write_mmd(phydev, MDIO_MMD_AN, 0x8016, regval | 0x0018); > > > > I think I see how the old code accomplished the same thing. It was > > woefully undocumented. Can you improve on the magic 0x18 number? What > > about 0x8016? Is that a register whose name you know and can assign a > > constant? > > > > Register MMD7 8016 has no name in the documentation that I can see. It > is simply referred to as that. I'd rather keep it like this rather than > create confusion by making up a name for it. OK > > -Vladimir > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c index 3783d15..b4066e3 100644 --- a/drivers/net/phy/atheros.c +++ b/drivers/net/phy/atheros.c @@ -57,11 +57,9 @@ static int ar8035_config(struct phy_device *phydev) { int regval; - phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x0007); - phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x8016); - phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x4007); - regval = phy_read(phydev, MDIO_DEVAD_NONE, 0xe); - phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018)); + /* CLK_25M output clock select: 125 MHz */ + regval = phy_read_mmd(phydev, MDIO_MMD_AN, 0x8016); + phy_write_mmd(phydev, MDIO_MMD_AN, 0x8016, regval | 0x0018); phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05); regval = phy_read(phydev, MDIO_DEVAD_NONE, 0x1e);
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- Changes in v2: * Patch added in this version. drivers/net/phy/atheros.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)