* [U-Boot] [PATCH] net: phy: Improve the Marvell 151x constants
@ 2016-12-01 17:08 Joe Hershberger
2016-12-02 5:50 ` Stefan Roese
0 siblings, 1 reply; 3+ messages in thread
From: Joe Hershberger @ 2016-12-01 17:08 UTC (permalink / raw)
To: u-boot
Use some constants for the phy configuration instead of so many magic
numbers.
Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
---
drivers/net/phy/marvell.c | 47 ++++++++++++++++++++++++++++++++++++-----------
1 file changed, 36 insertions(+), 11 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index 4eeb0f6..e06fdac 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -82,6 +82,21 @@
#define MIIM_88E1310_PHY_RGMII_CTRL 21
#define MIIM_88E1310_PHY_PAGE 22
+/* 88E151x PHY defines */
+/* Page 3 registers */
+#define MIIM_88E151x_LED_FUNC_CTRL 16
+#define MIIM_88E151x_LED_FLD_SZ 4
+#define MIIM_88E151x_LED0_OFFS (0 * MIIM_88E151x_LED_FLD_SIZE)
+#define MIIM_88E151x_LED1_OFFS (1 * MIIM_88E151x_LED_FLD_SIZE)
+#define MIIM_88E151x_LED0_ACT 3
+#define MIIM_88E151x_LED1_100_1000_LINK 6
+#define MIIM_88E151x_LED_TIMER_CTRL 18
+#define MIIM_88E151x_INT_EN_OFFS 7
+/* Page 18 registers */
+#define MIIM_88E151x_GENERAL_CTRL 20
+#define MIIM_88E151x_MODE_SGMII 1
+#define MIIM_88E151x_RESET_OFFS 15
+
/* Marvell 88E1011S */
static int m88e1011s_config(struct phy_device *phydev)
{
@@ -289,7 +304,7 @@ static int m88e1518_config(struct phy_device *phydev)
*/
/* EEE initialization */
- phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x00ff);
+ phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 0x00ff);
phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x214B);
phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2144);
phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x0C28);
@@ -298,21 +313,23 @@ static int m88e1518_config(struct phy_device *phydev)
phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x214D);
phy_write(phydev, MDIO_DEVAD_NONE, 17, 0xCC0C);
phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2159);
- phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x0000);
+ phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 0x0000);
/* SGMII-to-Copper mode initialization */
if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
/* Select page 18 */
- phy_write(phydev, MDIO_DEVAD_NONE, 22, 18);
+ phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 18);
/* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
- m88e1518_phy_writebits(phydev, 20, 0, 3, 1);
+ m88e1518_phy_writebits(phydev, MIIM_88E151x_GENERAL_CTRL,
+ 0, 3, MIIM_88E151x_MODE_SGMII);
/* PHY reset is necessary after changing MODE[2:0] */
- m88e1518_phy_writebits(phydev, 20, 15, 1, 1);
+ m88e1518_phy_writebits(phydev, MIIM_88E151x_GENERAL_CTRL,
+ MIIM_88E151x_RESET_OFFS, 1, 1);
/* Reset page selection */
- phy_write(phydev, MDIO_DEVAD_NONE, 22, 0);
+ phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 0);
udelay(100);
}
@@ -324,17 +341,25 @@ static int m88e1518_config(struct phy_device *phydev)
static int m88e1510_config(struct phy_device *phydev)
{
/* Select page 3 */
- phy_write(phydev, MDIO_DEVAD_NONE, 22, 3);
+ phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE,
+ MIIM_88E1118_PHY_LED_PAGE);
/* Enable INTn output on LED[2] */
- m88e1518_phy_writebits(phydev, 18, 7, 1, 1);
+ m88e1518_phy_writebits(phydev, MIIM_88E151x_LED_TIMER_CTRL,
+ MIIM_88E151x_INT_EN_OFFS, 1, 1);
/* Configure LEDs */
- m88e1518_phy_writebits(phydev, 16, 0, 4, 3); /* LED[0]:0011 (ACT) */
- m88e1518_phy_writebits(phydev, 16, 4, 4, 6); /* LED[1]:0110 (LINK) */
+ /* LED[0]:0011 (ACT) */
+ m88e1518_phy_writebits(phydev, MIIM_88E151x_LED_FUNC_CTRL,
+ MIIM_88E151x_LED0_OFFS, MIIM_88E151x_LED_FLD_SZ,
+ MIIM_88E151x_LED0_ACT);
+ /* LED[1]:0110 (LINK 100/1000 Mbps) */
+ m88e1518_phy_writebits(phydev, MIIM_88E151x_LED_FUNC_CTRL,
+ MIIM_88E151x_LED1_OFFS, MIIM_88E151x_LED_FLD_SZ,
+ MIIM_88E151x_LED1_100_1000_LINK);
/* Reset page selection */
- phy_write(phydev, MDIO_DEVAD_NONE, 22, 0);
+ phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 0);
return m88e1518_config(phydev);
}
--
1.7.11.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [U-Boot] [PATCH] net: phy: Improve the Marvell 151x constants
2016-12-01 17:08 [U-Boot] [PATCH] net: phy: Improve the Marvell 151x constants Joe Hershberger
@ 2016-12-02 5:50 ` Stefan Roese
2016-12-09 13:16 ` Phil Edworthy
0 siblings, 1 reply; 3+ messages in thread
From: Stefan Roese @ 2016-12-02 5:50 UTC (permalink / raw)
To: u-boot
On 01.12.2016 18:08, Joe Hershberger wrote:
> Use some constants for the phy configuration instead of so many magic
> numbers.
>
> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
> ---
>
> drivers/net/phy/marvell.c | 47 ++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 36 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> index 4eeb0f6..e06fdac 100644
> --- a/drivers/net/phy/marvell.c
> +++ b/drivers/net/phy/marvell.c
> @@ -82,6 +82,21 @@
> #define MIIM_88E1310_PHY_RGMII_CTRL 21
> #define MIIM_88E1310_PHY_PAGE 22
>
> +/* 88E151x PHY defines */
> +/* Page 3 registers */
> +#define MIIM_88E151x_LED_FUNC_CTRL 16
> +#define MIIM_88E151x_LED_FLD_SZ 4
> +#define MIIM_88E151x_LED0_OFFS (0 * MIIM_88E151x_LED_FLD_SIZE)
> +#define MIIM_88E151x_LED1_OFFS (1 * MIIM_88E151x_LED_FLD_SIZE)
> +#define MIIM_88E151x_LED0_ACT 3
> +#define MIIM_88E151x_LED1_100_1000_LINK 6
> +#define MIIM_88E151x_LED_TIMER_CTRL 18
> +#define MIIM_88E151x_INT_EN_OFFS 7
> +/* Page 18 registers */
> +#define MIIM_88E151x_GENERAL_CTRL 20
> +#define MIIM_88E151x_MODE_SGMII 1
> +#define MIIM_88E151x_RESET_OFFS 15
> +
> /* Marvell 88E1011S */
> static int m88e1011s_config(struct phy_device *phydev)
> {
> @@ -289,7 +304,7 @@ static int m88e1518_config(struct phy_device *phydev)
> */
>
> /* EEE initialization */
> - phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x00ff);
> + phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 0x00ff);
> phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x214B);
> phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2144);
> phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x0C28);
> @@ -298,21 +313,23 @@ static int m88e1518_config(struct phy_device *phydev)
> phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x214D);
> phy_write(phydev, MDIO_DEVAD_NONE, 17, 0xCC0C);
> phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2159);
> - phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x0000);
> + phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 0x0000);
>
> /* SGMII-to-Copper mode initialization */
> if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
> /* Select page 18 */
> - phy_write(phydev, MDIO_DEVAD_NONE, 22, 18);
> + phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 18);
>
> /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
> - m88e1518_phy_writebits(phydev, 20, 0, 3, 1);
> + m88e1518_phy_writebits(phydev, MIIM_88E151x_GENERAL_CTRL,
> + 0, 3, MIIM_88E151x_MODE_SGMII);
>
> /* PHY reset is necessary after changing MODE[2:0] */
> - m88e1518_phy_writebits(phydev, 20, 15, 1, 1);
> + m88e1518_phy_writebits(phydev, MIIM_88E151x_GENERAL_CTRL,
> + MIIM_88E151x_RESET_OFFS, 1, 1);
>
> /* Reset page selection */
> - phy_write(phydev, MDIO_DEVAD_NONE, 22, 0);
> + phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 0);
>
> udelay(100);
> }
> @@ -324,17 +341,25 @@ static int m88e1518_config(struct phy_device *phydev)
> static int m88e1510_config(struct phy_device *phydev)
> {
> /* Select page 3 */
> - phy_write(phydev, MDIO_DEVAD_NONE, 22, 3);
> + phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE,
> + MIIM_88E1118_PHY_LED_PAGE);
>
> /* Enable INTn output on LED[2] */
> - m88e1518_phy_writebits(phydev, 18, 7, 1, 1);
> + m88e1518_phy_writebits(phydev, MIIM_88E151x_LED_TIMER_CTRL,
> + MIIM_88E151x_INT_EN_OFFS, 1, 1);
>
> /* Configure LEDs */
> - m88e1518_phy_writebits(phydev, 16, 0, 4, 3); /* LED[0]:0011 (ACT) */
> - m88e1518_phy_writebits(phydev, 16, 4, 4, 6); /* LED[1]:0110 (LINK) */
> + /* LED[0]:0011 (ACT) */
> + m88e1518_phy_writebits(phydev, MIIM_88E151x_LED_FUNC_CTRL,
> + MIIM_88E151x_LED0_OFFS, MIIM_88E151x_LED_FLD_SZ,
> + MIIM_88E151x_LED0_ACT);
> + /* LED[1]:0110 (LINK 100/1000 Mbps) */
> + m88e1518_phy_writebits(phydev, MIIM_88E151x_LED_FUNC_CTRL,
> + MIIM_88E151x_LED1_OFFS, MIIM_88E151x_LED_FLD_SZ,
> + MIIM_88E151x_LED1_100_1000_LINK);
>
> /* Reset page selection */
> - phy_write(phydev, MDIO_DEVAD_NONE, 22, 0);
> + phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 0);
>
> return m88e1518_config(phydev);
> }
>
Joe, thanks for working on this:
Reviewed-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
^ permalink raw reply [flat|nested] 3+ messages in thread
* [U-Boot] [PATCH] net: phy: Improve the Marvell 151x constants
2016-12-02 5:50 ` Stefan Roese
@ 2016-12-09 13:16 ` Phil Edworthy
0 siblings, 0 replies; 3+ messages in thread
From: Phil Edworthy @ 2016-12-09 13:16 UTC (permalink / raw)
To: u-boot
Hi Joe,
On 02 December 2016 05:50, Joe Hershberger wrote:
> On 01.12.2016 18:08, Joe Hershberger wrote:
> > Use some constants for the phy configuration instead of so many magic
> > numbers.
> >
> > Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
> > ---
> >
> > drivers/net/phy/marvell.c | 47 ++++++++++++++++++++++++++++++++++++-
> ----------
> > 1 file changed, 36 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> > index 4eeb0f6..e06fdac 100644
> > --- a/drivers/net/phy/marvell.c
> > +++ b/drivers/net/phy/marvell.c
> > @@ -82,6 +82,21 @@
> > #define MIIM_88E1310_PHY_RGMII_CTRL 21
> > #define MIIM_88E1310_PHY_PAGE 22
> >
> > +/* 88E151x PHY defines */
> > +/* Page 3 registers */
> > +#define MIIM_88E151x_LED_FUNC_CTRL 16
> > +#define MIIM_88E151x_LED_FLD_SZ 4
> > +#define MIIM_88E151x_LED0_OFFS (0 *
> MIIM_88E151x_LED_FLD_SIZE)
> > +#define MIIM_88E151x_LED1_OFFS (1 *
> MIIM_88E151x_LED_FLD_SIZE)
Build error:
marvell.c:89:38: error: 'MIIM_88E151x_LED_FLD_SIZE' undeclared
I guess it should use 'MIIM_88E151x_LED_FLD_SZ' instead!
> > +#define MIIM_88E151x_LED0_ACT 3
> > +#define MIIM_88E151x_LED1_100_1000_LINK 6
> > +#define MIIM_88E151x_LED_TIMER_CTRL 18
> > +#define MIIM_88E151x_INT_EN_OFFS 7
> > +/* Page 18 registers */
> > +#define MIIM_88E151x_GENERAL_CTRL 20
> > +#define MIIM_88E151x_MODE_SGMII 1
> > +#define MIIM_88E151x_RESET_OFFS 15
> > +
> > /* Marvell 88E1011S */
> > static int m88e1011s_config(struct phy_device *phydev)
> > {
> > @@ -289,7 +304,7 @@ static int m88e1518_config(struct phy_device *phydev)
> > */
> >
> > /* EEE initialization */
> > - phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x00ff);
> > + phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE,
> 0x00ff);
> > phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x214B);
> > phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2144);
> > phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x0C28);
> > @@ -298,21 +313,23 @@ static int m88e1518_config(struct phy_device
> *phydev)
> > phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x214D);
> > phy_write(phydev, MDIO_DEVAD_NONE, 17, 0xCC0C);
> > phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2159);
> > - phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x0000);
> > + phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE,
> 0x0000);
> >
> > /* SGMII-to-Copper mode initialization */
> > if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
> > /* Select page 18 */
> > - phy_write(phydev, MDIO_DEVAD_NONE, 22, 18);
> > + phy_write(phydev, MDIO_DEVAD_NONE,
> MIIM_88E1118_PHY_PAGE, 18);
> >
> > /* In reg 20, write MODE[2:0] = 0x1 (SGMII to Copper) */
> > - m88e1518_phy_writebits(phydev, 20, 0, 3, 1);
> > + m88e1518_phy_writebits(phydev,
> MIIM_88E151x_GENERAL_CTRL,
> > + 0, 3, MIIM_88E151x_MODE_SGMII);
> >
> > /* PHY reset is necessary after changing MODE[2:0] */
> > - m88e1518_phy_writebits(phydev, 20, 15, 1, 1);
> > + m88e1518_phy_writebits(phydev,
> MIIM_88E151x_GENERAL_CTRL,
> > + MIIM_88E151x_RESET_OFFS, 1, 1);
> >
> > /* Reset page selection */
> > - phy_write(phydev, MDIO_DEVAD_NONE, 22, 0);
> > + phy_write(phydev, MDIO_DEVAD_NONE,
> MIIM_88E1118_PHY_PAGE, 0);
> >
> > udelay(100);
> > }
> > @@ -324,17 +341,25 @@ static int m88e1518_config(struct phy_device
> *phydev)
> > static int m88e1510_config(struct phy_device *phydev)
> > {
> > /* Select page 3 */
> > - phy_write(phydev, MDIO_DEVAD_NONE, 22, 3);
> > + phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE,
> > + MIIM_88E1118_PHY_LED_PAGE);
> >
> > /* Enable INTn output on LED[2] */
> > - m88e1518_phy_writebits(phydev, 18, 7, 1, 1);
> > + m88e1518_phy_writebits(phydev, MIIM_88E151x_LED_TIMER_CTRL,
> > + MIIM_88E151x_INT_EN_OFFS, 1, 1);
> >
> > /* Configure LEDs */
> > - m88e1518_phy_writebits(phydev, 16, 0, 4, 3); /* LED[0]:0011 (ACT) */
> > - m88e1518_phy_writebits(phydev, 16, 4, 4, 6); /* LED[1]:0110 (LINK) */
> > + /* LED[0]:0011 (ACT) */
> > + m88e1518_phy_writebits(phydev, MIIM_88E151x_LED_FUNC_CTRL,
> > + MIIM_88E151x_LED0_OFFS,
> MIIM_88E151x_LED_FLD_SZ,
> > + MIIM_88E151x_LED0_ACT);
> > + /* LED[1]:0110 (LINK 100/1000 Mbps) */
> > + m88e1518_phy_writebits(phydev, MIIM_88E151x_LED_FUNC_CTRL,
> > + MIIM_88E151x_LED1_OFFS,
> MIIM_88E151x_LED_FLD_SZ,
> > + MIIM_88E151x_LED1_100_1000_LINK);
> >
> > /* Reset page selection */
> > - phy_write(phydev, MDIO_DEVAD_NONE, 22, 0);
> > + phy_write(phydev, MDIO_DEVAD_NONE, MIIM_88E1118_PHY_PAGE, 0);
> >
> > return m88e1518_config(phydev);
> > }
> >
>
> Joe, thanks for working on this:
>
> Reviewed-by: Stefan Roese <sr@denx.de>
>
With the above fixed, I have tested this on 88E1512 (patches sent earlier to
support the 88E1512 device.
Thanks
Phil
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-12-09 13:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-01 17:08 [U-Boot] [PATCH] net: phy: Improve the Marvell 151x constants Joe Hershberger
2016-12-02 5:50 ` Stefan Roese
2016-12-09 13:16 ` Phil Edworthy
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.