[PATCH net-next v2 0/8] phy: net: meson-gxl: clean-up and improvements

[PATCH net-next v2 0/8] phy: net: meson-gxl: clean-up and improvements

[Jerome Brunet]

The patchset is a v2 of the previous single clean-up patch [0] which was
part of larger series. I initially to send these patches separately but
adding helper function without using them did not make much sense after
all. So, here is the complete patchset.

This patchset add defines for the control registers and helpers to access
the banked registers. The goal being to make it easier to understand what
the driver actually does.

Then there is fix for the incorrect sampling of the MII LPA register which
is often breaking the auto-negotiation with this PHY. More details on this
in the related patch

CONFIG_A6 settings is removed since this statement was without effect

Finally interrupt support is added, speeding things up a little

This series has been tested on the libretech-cc and khadas VIM

Jerome Brunet (8):
  net: phy: meson-gxl: check phy_write return value
  net: phy: meson-gxl: define control registers
  net: phy: meson-gxl: add read and write helpers for bank registers
  net: phy: meson-gxl: use genphy_config_init
  net: phy: meson-gxl: detect LPA corruption
  net: phy: meson-gxl: leave CONFIG_A6 untouched
  net: phy: meson-gxl: add interrupt support
  net: phy: meson-gxl: join the authors

 drivers/net/phy/meson-gxl.c | 215 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 197 insertions(+), 18 deletions(-)

-- 
2.14.3

[PATCH net-next v2 0/8] phy: net: meson-gxl: clean-up and improvements

[Jerome Brunet]

The patchset is a v2 of the previous single clean-up patch [0] which was
part of larger series. I initially to send these patches separately but
adding helper function without using them did not make much sense after
all. So, here is the complete patchset.

This patchset add defines for the control registers and helpers to access
the banked registers. The goal being to make it easier to understand what
the driver actually does.

Then there is fix for the incorrect sampling of the MII LPA register which
is often breaking the auto-negotiation with this PHY. More details on this
in the related patch

CONFIG_A6 settings is removed since this statement was without effect

Finally interrupt support is added, speeding things up a little

This series has been tested on the libretech-cc and khadas VIM

Jerome Brunet (8):
  net: phy: meson-gxl: check phy_write return value
  net: phy: meson-gxl: define control registers
  net: phy: meson-gxl: add read and write helpers for bank registers
  net: phy: meson-gxl: use genphy_config_init
  net: phy: meson-gxl: detect LPA corruption
  net: phy: meson-gxl: leave CONFIG_A6 untouched
  net: phy: meson-gxl: add interrupt support
  net: phy: meson-gxl: join the authors

 drivers/net/phy/meson-gxl.c | 215 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 197 insertions(+), 18 deletions(-)

-- 
2.14.3

[PATCH net-next v2 0/8] phy: net: meson-gxl: clean-up and improvements

[Jerome Brunet]

The patchset is a v2 of the previous single clean-up patch [0] which was
part of larger series. I initially to send these patches separately but
adding helper function without using them did not make much sense after
all. So, here is the complete patchset.

This patchset add defines for the control registers and helpers to access
the banked registers. The goal being to make it easier to understand what
the driver actually does.

Then there is fix for the incorrect sampling of the MII LPA register which
is often breaking the auto-negotiation with this PHY. More details on this
in the related patch

CONFIG_A6 settings is removed since this statement was without effect

Finally interrupt support is added, speeding things up a little

This series has been tested on the libretech-cc and khadas VIM

Jerome Brunet (8):
  net: phy: meson-gxl: check phy_write return value
  net: phy: meson-gxl: define control registers
  net: phy: meson-gxl: add read and write helpers for bank registers
  net: phy: meson-gxl: use genphy_config_init
  net: phy: meson-gxl: detect LPA corruption
  net: phy: meson-gxl: leave CONFIG_A6 untouched
  net: phy: meson-gxl: add interrupt support
  net: phy: meson-gxl: join the authors

 drivers/net/phy/meson-gxl.c | 215 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 197 insertions(+), 18 deletions(-)

-- 
2.14.3

[PATCH net-next v2 0/8] phy: net: meson-gxl: clean-up and improvements

[Jerome Brunet]

The patchset is a v2 of the previous single clean-up patch [0] which was
part of larger series. I initially to send these patches separately but
adding helper function without using them did not make much sense after
all. So, here is the complete patchset.

This patchset add defines for the control registers and helpers to access
the banked registers. The goal being to make it easier to understand what
the driver actually does.

Then there is fix for the incorrect sampling of the MII LPA register which
is often breaking the auto-negotiation with this PHY. More details on this
in the related patch

CONFIG_A6 settings is removed since this statement was without effect

Finally interrupt support is added, speeding things up a little

This series has been tested on the libretech-cc and khadas VIM

Jerome Brunet (8):
  net: phy: meson-gxl: check phy_write return value
  net: phy: meson-gxl: define control registers
  net: phy: meson-gxl: add read and write helpers for bank registers
  net: phy: meson-gxl: use genphy_config_init
  net: phy: meson-gxl: detect LPA corruption
  net: phy: meson-gxl: leave CONFIG_A6 untouched
  net: phy: meson-gxl: add interrupt support
  net: phy: meson-gxl: join the authors

 drivers/net/phy/meson-gxl.c | 215 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 197 insertions(+), 18 deletions(-)

-- 
2.14.3

[PATCH net-next v2 1/8] net: phy: meson-gxl: check phy_write return value

[Jerome Brunet]

Always check phy_write return values. Better to be safe than sorry

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 50 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 38 insertions(+), 12 deletions(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 1ea69b7585d9..7ddb709f69fc 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -25,27 +25,53 @@
 
 static int meson_gxl_config_init(struct phy_device *phydev)
 {
+	int ret;
+
 	/* Enable Analog and DSP register Bank access by */
-	phy_write(phydev, 0x14, 0x0000);
-	phy_write(phydev, 0x14, 0x0400);
-	phy_write(phydev, 0x14, 0x0000);
-	phy_write(phydev, 0x14, 0x0400);
+	ret = phy_write(phydev, 0x14, 0x0000);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x0400);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x0000);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x0400);
+	if (ret)
+		return ret;
 
 	/* Write Analog register 23 */
-	phy_write(phydev, 0x17, 0x8E0D);
-	phy_write(phydev, 0x14, 0x4417);
+	ret = phy_write(phydev, 0x17, 0x8E0D);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x4417);
+	if (ret)
+		return ret;
 
 	/* Enable fractional PLL */
-	phy_write(phydev, 0x17, 0x0005);
-	phy_write(phydev, 0x14, 0x5C1B);
+	ret = phy_write(phydev, 0x17, 0x0005);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x5C1B);
+	if (ret)
+		return ret;
 
 	/* Program fraction FR_PLL_DIV1 */
-	phy_write(phydev, 0x17, 0x029A);
-	phy_write(phydev, 0x14, 0x5C1D);
+	ret = phy_write(phydev, 0x17, 0x029A);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x5C1D);
+	if (ret)
+		return ret;
 
 	/* Program fraction FR_PLL_DIV1 */
-	phy_write(phydev, 0x17, 0xAAAA);
-	phy_write(phydev, 0x14, 0x5C1C);
+	ret = phy_write(phydev, 0x17, 0xAAAA);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x5C1C);
+	if (ret)
+		return ret;
 
 	return 0;
 }
-- 
2.14.3

[PATCH net-next v2 1/8] net: phy: meson-gxl: check phy_write return value

[Jerome Brunet]

Always check phy_write return values. Better to be safe than sorry

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 50 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 38 insertions(+), 12 deletions(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 1ea69b7585d9..7ddb709f69fc 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -25,27 +25,53 @@
 
 static int meson_gxl_config_init(struct phy_device *phydev)
 {
+	int ret;
+
 	/* Enable Analog and DSP register Bank access by */
-	phy_write(phydev, 0x14, 0x0000);
-	phy_write(phydev, 0x14, 0x0400);
-	phy_write(phydev, 0x14, 0x0000);
-	phy_write(phydev, 0x14, 0x0400);
+	ret = phy_write(phydev, 0x14, 0x0000);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x0400);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x0000);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x0400);
+	if (ret)
+		return ret;
 
 	/* Write Analog register 23 */
-	phy_write(phydev, 0x17, 0x8E0D);
-	phy_write(phydev, 0x14, 0x4417);
+	ret = phy_write(phydev, 0x17, 0x8E0D);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x4417);
+	if (ret)
+		return ret;
 
 	/* Enable fractional PLL */
-	phy_write(phydev, 0x17, 0x0005);
-	phy_write(phydev, 0x14, 0x5C1B);
+	ret = phy_write(phydev, 0x17, 0x0005);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x5C1B);
+	if (ret)
+		return ret;
 
 	/* Program fraction FR_PLL_DIV1 */
-	phy_write(phydev, 0x17, 0x029A);
-	phy_write(phydev, 0x14, 0x5C1D);
+	ret = phy_write(phydev, 0x17, 0x029A);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x5C1D);
+	if (ret)
+		return ret;
 
 	/* Program fraction FR_PLL_DIV1 */
-	phy_write(phydev, 0x17, 0xAAAA);
-	phy_write(phydev, 0x14, 0x5C1C);
+	ret = phy_write(phydev, 0x17, 0xAAAA);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x5C1C);
+	if (ret)
+		return ret;
 
 	return 0;
 }
-- 
2.14.3

[PATCH net-next v2 1/8] net: phy: meson-gxl: check phy_write return value

[Jerome Brunet]

Always check phy_write return values. Better to be safe than sorry

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 50 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 38 insertions(+), 12 deletions(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 1ea69b7585d9..7ddb709f69fc 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -25,27 +25,53 @@
 
 static int meson_gxl_config_init(struct phy_device *phydev)
 {
+	int ret;
+
 	/* Enable Analog and DSP register Bank access by */
-	phy_write(phydev, 0x14, 0x0000);
-	phy_write(phydev, 0x14, 0x0400);
-	phy_write(phydev, 0x14, 0x0000);
-	phy_write(phydev, 0x14, 0x0400);
+	ret = phy_write(phydev, 0x14, 0x0000);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x0400);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x0000);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x0400);
+	if (ret)
+		return ret;
 
 	/* Write Analog register 23 */
-	phy_write(phydev, 0x17, 0x8E0D);
-	phy_write(phydev, 0x14, 0x4417);
+	ret = phy_write(phydev, 0x17, 0x8E0D);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x4417);
+	if (ret)
+		return ret;
 
 	/* Enable fractional PLL */
-	phy_write(phydev, 0x17, 0x0005);
-	phy_write(phydev, 0x14, 0x5C1B);
+	ret = phy_write(phydev, 0x17, 0x0005);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x5C1B);
+	if (ret)
+		return ret;
 
 	/* Program fraction FR_PLL_DIV1 */
-	phy_write(phydev, 0x17, 0x029A);
-	phy_write(phydev, 0x14, 0x5C1D);
+	ret = phy_write(phydev, 0x17, 0x029A);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x5C1D);
+	if (ret)
+		return ret;
 
 	/* Program fraction FR_PLL_DIV1 */
-	phy_write(phydev, 0x17, 0xAAAA);
-	phy_write(phydev, 0x14, 0x5C1C);
+	ret = phy_write(phydev, 0x17, 0xAAAA);
+	if (ret)
+		return ret;
+	ret = phy_write(phydev, 0x14, 0x5C1C);
+	if (ret)
+		return ret;
 
 	return 0;
 }
-- 
2.14.3

[PATCH net-next v2 2/8] net: phy: meson-gxl: define control registers

[Jerome Brunet]

Define registers and bits in meson-gxl PHY driver to make a bit
more human friendly. No functional change.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 64 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 51 insertions(+), 13 deletions(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 7ddb709f69fc..d82aa8cea401 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -22,54 +22,92 @@
 #include <linux/ethtool.h>
 #include <linux/phy.h>
 #include <linux/netdevice.h>
+#include <linux/bitfield.h>
+
+#define TSTCNTL		20
+#define  TSTCNTL_READ		BIT(15)
+#define  TSTCNTL_WRITE		BIT(14)
+#define  TSTCNTL_REG_BANK_SEL	GENMASK(12, 11)
+#define  TSTCNTL_TEST_MODE	BIT(10)
+#define  TSTCNTL_READ_ADDRESS	GENMASK(9, 5)
+#define  TSTCNTL_WRITE_ADDRESS	GENMASK(4, 0)
+#define TSTREAD1	21
+#define TSTWRITE	23
+
+#define BANK_ANALOG_DSP		0
+#define BANK_BIST		3
+
+/* Analog/DSP Registers */
+#define A6_CONFIG_REG	0x17
+
+/* BIST Registers */
+#define FR_PLL_CONTROL	0x1b
+#define FR_PLL_DIV0	0x1c
+#define FR_PLL_DIV1	0x1d
 
 static int meson_gxl_config_init(struct phy_device *phydev)
 {
 	int ret;
 
 	/* Enable Analog and DSP register Bank access by */
-	ret = phy_write(phydev, 0x14, 0x0000);
+	ret = phy_write(phydev, TSTCNTL, 0);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x0400);
+	ret = phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x0000);
+	ret = phy_write(phydev, TSTCNTL, 0);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x0400);
+	ret = phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
 	if (ret)
 		return ret;
 
-	/* Write Analog register 23 */
-	ret = phy_write(phydev, 0x17, 0x8E0D);
+	/* Write CONFIG_A6*/
+	ret = phy_write(phydev, TSTWRITE, 0x8e0d)
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x4417);
+	ret = phy_write(phydev, TSTCNTL,
+			TSTCNTL_WRITE
+			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_ANALOG_DSP)
+			| TSTCNTL_TEST_MODE
+			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, A6_CONFIG_REG));
 	if (ret)
 		return ret;
 
 	/* Enable fractional PLL */
-	ret = phy_write(phydev, 0x17, 0x0005);
+	ret = phy_write(phydev, TSTWRITE, 0x0005);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x5C1B);
+	ret = phy_write(phydev, TSTCNTL,
+			TSTCNTL_WRITE
+			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
+			| TSTCNTL_TEST_MODE
+			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_CONTROL));
 	if (ret)
 		return ret;
 
 	/* Program fraction FR_PLL_DIV1 */
-	ret = phy_write(phydev, 0x17, 0x029A);
+	ret = phy_write(phydev, TSTWRITE, 0x029a);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x5C1D);
+	ret = phy_write(phydev, TSTCNTL,
+			TSTCNTL_WRITE
+			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
+			| TSTCNTL_TEST_MODE
+			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_DIV1));
 	if (ret)
 		return ret;
 
 	/* Program fraction FR_PLL_DIV1 */
-	ret = phy_write(phydev, 0x17, 0xAAAA);
+	ret = phy_write(phydev, TSTWRITE, 0xaaaa);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x5C1C);
+	ret = phy_write(phydev, TSTCNTL,
+			TSTCNTL_WRITE
+			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
+			| TSTCNTL_TEST_MODE
+			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_DIV0));
 	if (ret)
 		return ret;
 
-- 
2.14.3

[PATCH net-next v2 2/8] net: phy: meson-gxl: define control registers

[Jerome Brunet]

Define registers and bits in meson-gxl PHY driver to make a bit
more human friendly. No functional change.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 64 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 51 insertions(+), 13 deletions(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 7ddb709f69fc..d82aa8cea401 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -22,54 +22,92 @@
 #include <linux/ethtool.h>
 #include <linux/phy.h>
 #include <linux/netdevice.h>
+#include <linux/bitfield.h>
+
+#define TSTCNTL		20
+#define  TSTCNTL_READ		BIT(15)
+#define  TSTCNTL_WRITE		BIT(14)
+#define  TSTCNTL_REG_BANK_SEL	GENMASK(12, 11)
+#define  TSTCNTL_TEST_MODE	BIT(10)
+#define  TSTCNTL_READ_ADDRESS	GENMASK(9, 5)
+#define  TSTCNTL_WRITE_ADDRESS	GENMASK(4, 0)
+#define TSTREAD1	21
+#define TSTWRITE	23
+
+#define BANK_ANALOG_DSP		0
+#define BANK_BIST		3
+
+/* Analog/DSP Registers */
+#define A6_CONFIG_REG	0x17
+
+/* BIST Registers */
+#define FR_PLL_CONTROL	0x1b
+#define FR_PLL_DIV0	0x1c
+#define FR_PLL_DIV1	0x1d
 
 static int meson_gxl_config_init(struct phy_device *phydev)
 {
 	int ret;
 
 	/* Enable Analog and DSP register Bank access by */
-	ret = phy_write(phydev, 0x14, 0x0000);
+	ret = phy_write(phydev, TSTCNTL, 0);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x0400);
+	ret = phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x0000);
+	ret = phy_write(phydev, TSTCNTL, 0);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x0400);
+	ret = phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
 	if (ret)
 		return ret;
 
-	/* Write Analog register 23 */
-	ret = phy_write(phydev, 0x17, 0x8E0D);
+	/* Write CONFIG_A6*/
+	ret = phy_write(phydev, TSTWRITE, 0x8e0d)
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x4417);
+	ret = phy_write(phydev, TSTCNTL,
+			TSTCNTL_WRITE
+			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_ANALOG_DSP)
+			| TSTCNTL_TEST_MODE
+			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, A6_CONFIG_REG));
 	if (ret)
 		return ret;
 
 	/* Enable fractional PLL */
-	ret = phy_write(phydev, 0x17, 0x0005);
+	ret = phy_write(phydev, TSTWRITE, 0x0005);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x5C1B);
+	ret = phy_write(phydev, TSTCNTL,
+			TSTCNTL_WRITE
+			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
+			| TSTCNTL_TEST_MODE
+			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_CONTROL));
 	if (ret)
 		return ret;
 
 	/* Program fraction FR_PLL_DIV1 */
-	ret = phy_write(phydev, 0x17, 0x029A);
+	ret = phy_write(phydev, TSTWRITE, 0x029a);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x5C1D);
+	ret = phy_write(phydev, TSTCNTL,
+			TSTCNTL_WRITE
+			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
+			| TSTCNTL_TEST_MODE
+			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_DIV1));
 	if (ret)
 		return ret;
 
 	/* Program fraction FR_PLL_DIV1 */
-	ret = phy_write(phydev, 0x17, 0xAAAA);
+	ret = phy_write(phydev, TSTWRITE, 0xaaaa);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x5C1C);
+	ret = phy_write(phydev, TSTCNTL,
+			TSTCNTL_WRITE
+			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
+			| TSTCNTL_TEST_MODE
+			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_DIV0));
 	if (ret)
 		return ret;
 
-- 
2.14.3

[PATCH net-next v2 2/8] net: phy: meson-gxl: define control registers

[Jerome Brunet]

Define registers and bits in meson-gxl PHY driver to make a bit
more human friendly. No functional change.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 64 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 51 insertions(+), 13 deletions(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 7ddb709f69fc..d82aa8cea401 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -22,54 +22,92 @@
 #include <linux/ethtool.h>
 #include <linux/phy.h>
 #include <linux/netdevice.h>
+#include <linux/bitfield.h>
+
+#define TSTCNTL		20
+#define  TSTCNTL_READ		BIT(15)
+#define  TSTCNTL_WRITE		BIT(14)
+#define  TSTCNTL_REG_BANK_SEL	GENMASK(12, 11)
+#define  TSTCNTL_TEST_MODE	BIT(10)
+#define  TSTCNTL_READ_ADDRESS	GENMASK(9, 5)
+#define  TSTCNTL_WRITE_ADDRESS	GENMASK(4, 0)
+#define TSTREAD1	21
+#define TSTWRITE	23
+
+#define BANK_ANALOG_DSP		0
+#define BANK_BIST		3
+
+/* Analog/DSP Registers */
+#define A6_CONFIG_REG	0x17
+
+/* BIST Registers */
+#define FR_PLL_CONTROL	0x1b
+#define FR_PLL_DIV0	0x1c
+#define FR_PLL_DIV1	0x1d
 
 static int meson_gxl_config_init(struct phy_device *phydev)
 {
 	int ret;
 
 	/* Enable Analog and DSP register Bank access by */
-	ret = phy_write(phydev, 0x14, 0x0000);
+	ret = phy_write(phydev, TSTCNTL, 0);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x0400);
+	ret = phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x0000);
+	ret = phy_write(phydev, TSTCNTL, 0);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x0400);
+	ret = phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
 	if (ret)
 		return ret;
 
-	/* Write Analog register 23 */
-	ret = phy_write(phydev, 0x17, 0x8E0D);
+	/* Write CONFIG_A6*/
+	ret = phy_write(phydev, TSTWRITE, 0x8e0d)
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x4417);
+	ret = phy_write(phydev, TSTCNTL,
+			TSTCNTL_WRITE
+			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_ANALOG_DSP)
+			| TSTCNTL_TEST_MODE
+			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, A6_CONFIG_REG));
 	if (ret)
 		return ret;
 
 	/* Enable fractional PLL */
-	ret = phy_write(phydev, 0x17, 0x0005);
+	ret = phy_write(phydev, TSTWRITE, 0x0005);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x5C1B);
+	ret = phy_write(phydev, TSTCNTL,
+			TSTCNTL_WRITE
+			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
+			| TSTCNTL_TEST_MODE
+			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_CONTROL));
 	if (ret)
 		return ret;
 
 	/* Program fraction FR_PLL_DIV1 */
-	ret = phy_write(phydev, 0x17, 0x029A);
+	ret = phy_write(phydev, TSTWRITE, 0x029a);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x5C1D);
+	ret = phy_write(phydev, TSTCNTL,
+			TSTCNTL_WRITE
+			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
+			| TSTCNTL_TEST_MODE
+			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_DIV1));
 	if (ret)
 		return ret;
 
 	/* Program fraction FR_PLL_DIV1 */
-	ret = phy_write(phydev, 0x17, 0xAAAA);
+	ret = phy_write(phydev, TSTWRITE, 0xaaaa);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, 0x14, 0x5C1C);
+	ret = phy_write(phydev, TSTCNTL,
+			TSTCNTL_WRITE
+			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
+			| TSTCNTL_TEST_MODE
+			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_DIV0));
 	if (ret)
 		return ret;
 
-- 
2.14.3

[PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Jerome Brunet]

Add read and write helpers to manipulate banked registers on this PHY
This helps clarify the settings applied to these registers in the init
function and upcoming changes.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 103 ++++++++++++++++++++++++++++----------------
 1 file changed, 67 insertions(+), 36 deletions(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index d82aa8cea401..05054770aefb 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -45,11 +45,13 @@
 #define FR_PLL_DIV0	0x1c
 #define FR_PLL_DIV1	0x1d
 
-static int meson_gxl_config_init(struct phy_device *phydev)
+static int meson_gxl_open_banks(struct phy_device *phydev)
 {
 	int ret;
 
-	/* Enable Analog and DSP register Bank access by */
+	/* Enable Analog and DSP register Bank access by
+	 * toggling TSTCNTL_TEST_MODE bit in the TSTCNTL register
+	 */
 	ret = phy_write(phydev, TSTCNTL, 0);
 	if (ret)
 		return ret;
@@ -59,55 +61,84 @@ static int meson_gxl_config_init(struct phy_device *phydev)
 	ret = phy_write(phydev, TSTCNTL, 0);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
-	if (ret)
-		return ret;
+	return phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
+}
 
-	/* Write CONFIG_A6*/
-	ret = phy_write(phydev, TSTWRITE, 0x8e0d)
+static void meson_gxl_close_banks(struct phy_device *phydev)
+{
+	phy_write(phydev, TSTCNTL, 0);
+}
+
+static int meson_gxl_read_reg(struct phy_device *phydev,
+			      unsigned int bank, unsigned int reg)
+{
+	int ret;
+
+	ret = meson_gxl_open_banks(phydev);
 	if (ret)
-		return ret;
-	ret = phy_write(phydev, TSTCNTL,
-			TSTCNTL_WRITE
-			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_ANALOG_DSP)
-			| TSTCNTL_TEST_MODE
-			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, A6_CONFIG_REG));
+		goto out;
+
+	ret = phy_write(phydev, TSTCNTL, TSTCNTL_READ |
+			FIELD_PREP(TSTCNTL_REG_BANK_SEL, bank) |
+			TSTCNTL_TEST_MODE |
+			FIELD_PREP(TSTCNTL_READ_ADDRESS, reg));
 	if (ret)
-		return ret;
+		goto out;
 
-	/* Enable fractional PLL */
-	ret = phy_write(phydev, TSTWRITE, 0x0005);
+	ret = phy_read(phydev, TSTREAD1);
+out:
+	/* Close the bank access on our way out */
+	meson_gxl_close_banks(phydev);
+	return ret;
+}
+
+static int meson_gxl_write_reg(struct phy_device *phydev,
+			       unsigned int bank, unsigned int reg,
+			       uint16_t value)
+{
+	int ret;
+
+	ret = meson_gxl_open_banks(phydev);
 	if (ret)
-		return ret;
-	ret = phy_write(phydev, TSTCNTL,
-			TSTCNTL_WRITE
-			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
-			| TSTCNTL_TEST_MODE
-			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_CONTROL));
+		goto out;
+
+	ret = phy_write(phydev, TSTWRITE, value);
 	if (ret)
-		return ret;
+		goto out;
 
-	/* Program fraction FR_PLL_DIV1 */
-	ret = phy_write(phydev, TSTWRITE, 0x029a);
+	ret = phy_write(phydev, TSTCNTL, TSTCNTL_WRITE |
+			FIELD_PREP(TSTCNTL_REG_BANK_SEL, bank) |
+			TSTCNTL_TEST_MODE |
+			FIELD_PREP(TSTCNTL_WRITE_ADDRESS, reg));
+
+out:
+	/* Close the bank access on our way out */
+	meson_gxl_close_banks(phydev);
+	return ret;
+}
+
+static int meson_gxl_config_init(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Write CONFIG_A6*/
+	ret = meson_gxl_write_reg(phydev, BANK_ANALOG_DSP, A6_CONFIG_REG,
+				  0x8e0d);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, TSTCNTL,
-			TSTCNTL_WRITE
-			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
-			| TSTCNTL_TEST_MODE
-			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_DIV1));
+
+	/* Enable fractional PLL */
+	ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_CONTROL, 0x5);
 	if (ret)
 		return ret;
 
 	/* Program fraction FR_PLL_DIV1 */
-	ret = phy_write(phydev, TSTWRITE, 0xaaaa);
+	ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_DIV1, 0x029a);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, TSTCNTL,
-			TSTCNTL_WRITE
-			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
-			| TSTCNTL_TEST_MODE
-			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_DIV0));
+
+	/* Program fraction FR_PLL_DIV1 */
+	ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_DIV0, 0xaaaa);
 	if (ret)
 		return ret;
 
-- 
2.14.3

[PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Jerome Brunet]

Add read and write helpers to manipulate banked registers on this PHY
This helps clarify the settings applied to these registers in the init
function and upcoming changes.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 103 ++++++++++++++++++++++++++++----------------
 1 file changed, 67 insertions(+), 36 deletions(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index d82aa8cea401..05054770aefb 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -45,11 +45,13 @@
 #define FR_PLL_DIV0	0x1c
 #define FR_PLL_DIV1	0x1d
 
-static int meson_gxl_config_init(struct phy_device *phydev)
+static int meson_gxl_open_banks(struct phy_device *phydev)
 {
 	int ret;
 
-	/* Enable Analog and DSP register Bank access by */
+	/* Enable Analog and DSP register Bank access by
+	 * toggling TSTCNTL_TEST_MODE bit in the TSTCNTL register
+	 */
 	ret = phy_write(phydev, TSTCNTL, 0);
 	if (ret)
 		return ret;
@@ -59,55 +61,84 @@ static int meson_gxl_config_init(struct phy_device *phydev)
 	ret = phy_write(phydev, TSTCNTL, 0);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
-	if (ret)
-		return ret;
+	return phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
+}
 
-	/* Write CONFIG_A6*/
-	ret = phy_write(phydev, TSTWRITE, 0x8e0d)
+static void meson_gxl_close_banks(struct phy_device *phydev)
+{
+	phy_write(phydev, TSTCNTL, 0);
+}
+
+static int meson_gxl_read_reg(struct phy_device *phydev,
+			      unsigned int bank, unsigned int reg)
+{
+	int ret;
+
+	ret = meson_gxl_open_banks(phydev);
 	if (ret)
-		return ret;
-	ret = phy_write(phydev, TSTCNTL,
-			TSTCNTL_WRITE
-			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_ANALOG_DSP)
-			| TSTCNTL_TEST_MODE
-			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, A6_CONFIG_REG));
+		goto out;
+
+	ret = phy_write(phydev, TSTCNTL, TSTCNTL_READ |
+			FIELD_PREP(TSTCNTL_REG_BANK_SEL, bank) |
+			TSTCNTL_TEST_MODE |
+			FIELD_PREP(TSTCNTL_READ_ADDRESS, reg));
 	if (ret)
-		return ret;
+		goto out;
 
-	/* Enable fractional PLL */
-	ret = phy_write(phydev, TSTWRITE, 0x0005);
+	ret = phy_read(phydev, TSTREAD1);
+out:
+	/* Close the bank access on our way out */
+	meson_gxl_close_banks(phydev);
+	return ret;
+}
+
+static int meson_gxl_write_reg(struct phy_device *phydev,
+			       unsigned int bank, unsigned int reg,
+			       uint16_t value)
+{
+	int ret;
+
+	ret = meson_gxl_open_banks(phydev);
 	if (ret)
-		return ret;
-	ret = phy_write(phydev, TSTCNTL,
-			TSTCNTL_WRITE
-			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
-			| TSTCNTL_TEST_MODE
-			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_CONTROL));
+		goto out;
+
+	ret = phy_write(phydev, TSTWRITE, value);
 	if (ret)
-		return ret;
+		goto out;
 
-	/* Program fraction FR_PLL_DIV1 */
-	ret = phy_write(phydev, TSTWRITE, 0x029a);
+	ret = phy_write(phydev, TSTCNTL, TSTCNTL_WRITE |
+			FIELD_PREP(TSTCNTL_REG_BANK_SEL, bank) |
+			TSTCNTL_TEST_MODE |
+			FIELD_PREP(TSTCNTL_WRITE_ADDRESS, reg));
+
+out:
+	/* Close the bank access on our way out */
+	meson_gxl_close_banks(phydev);
+	return ret;
+}
+
+static int meson_gxl_config_init(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Write CONFIG_A6*/
+	ret = meson_gxl_write_reg(phydev, BANK_ANALOG_DSP, A6_CONFIG_REG,
+				  0x8e0d);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, TSTCNTL,
-			TSTCNTL_WRITE
-			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
-			| TSTCNTL_TEST_MODE
-			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_DIV1));
+
+	/* Enable fractional PLL */
+	ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_CONTROL, 0x5);
 	if (ret)
 		return ret;
 
 	/* Program fraction FR_PLL_DIV1 */
-	ret = phy_write(phydev, TSTWRITE, 0xaaaa);
+	ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_DIV1, 0x029a);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, TSTCNTL,
-			TSTCNTL_WRITE
-			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
-			| TSTCNTL_TEST_MODE
-			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_DIV0));
+
+	/* Program fraction FR_PLL_DIV1 */
+	ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_DIV0, 0xaaaa);
 	if (ret)
 		return ret;
 
-- 
2.14.3

[PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Jerome Brunet]

Add read and write helpers to manipulate banked registers on this PHY
This helps clarify the settings applied to these registers in the init
function and upcoming changes.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 103 ++++++++++++++++++++++++++++----------------
 1 file changed, 67 insertions(+), 36 deletions(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index d82aa8cea401..05054770aefb 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -45,11 +45,13 @@
 #define FR_PLL_DIV0	0x1c
 #define FR_PLL_DIV1	0x1d
 
-static int meson_gxl_config_init(struct phy_device *phydev)
+static int meson_gxl_open_banks(struct phy_device *phydev)
 {
 	int ret;
 
-	/* Enable Analog and DSP register Bank access by */
+	/* Enable Analog and DSP register Bank access by
+	 * toggling TSTCNTL_TEST_MODE bit in the TSTCNTL register
+	 */
 	ret = phy_write(phydev, TSTCNTL, 0);
 	if (ret)
 		return ret;
@@ -59,55 +61,84 @@ static int meson_gxl_config_init(struct phy_device *phydev)
 	ret = phy_write(phydev, TSTCNTL, 0);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
-	if (ret)
-		return ret;
+	return phy_write(phydev, TSTCNTL, TSTCNTL_TEST_MODE);
+}
 
-	/* Write CONFIG_A6*/
-	ret = phy_write(phydev, TSTWRITE, 0x8e0d)
+static void meson_gxl_close_banks(struct phy_device *phydev)
+{
+	phy_write(phydev, TSTCNTL, 0);
+}
+
+static int meson_gxl_read_reg(struct phy_device *phydev,
+			      unsigned int bank, unsigned int reg)
+{
+	int ret;
+
+	ret = meson_gxl_open_banks(phydev);
 	if (ret)
-		return ret;
-	ret = phy_write(phydev, TSTCNTL,
-			TSTCNTL_WRITE
-			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_ANALOG_DSP)
-			| TSTCNTL_TEST_MODE
-			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, A6_CONFIG_REG));
+		goto out;
+
+	ret = phy_write(phydev, TSTCNTL, TSTCNTL_READ |
+			FIELD_PREP(TSTCNTL_REG_BANK_SEL, bank) |
+			TSTCNTL_TEST_MODE |
+			FIELD_PREP(TSTCNTL_READ_ADDRESS, reg));
 	if (ret)
-		return ret;
+		goto out;
 
-	/* Enable fractional PLL */
-	ret = phy_write(phydev, TSTWRITE, 0x0005);
+	ret = phy_read(phydev, TSTREAD1);
+out:
+	/* Close the bank access on our way out */
+	meson_gxl_close_banks(phydev);
+	return ret;
+}
+
+static int meson_gxl_write_reg(struct phy_device *phydev,
+			       unsigned int bank, unsigned int reg,
+			       uint16_t value)
+{
+	int ret;
+
+	ret = meson_gxl_open_banks(phydev);
 	if (ret)
-		return ret;
-	ret = phy_write(phydev, TSTCNTL,
-			TSTCNTL_WRITE
-			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
-			| TSTCNTL_TEST_MODE
-			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_CONTROL));
+		goto out;
+
+	ret = phy_write(phydev, TSTWRITE, value);
 	if (ret)
-		return ret;
+		goto out;
 
-	/* Program fraction FR_PLL_DIV1 */
-	ret = phy_write(phydev, TSTWRITE, 0x029a);
+	ret = phy_write(phydev, TSTCNTL, TSTCNTL_WRITE |
+			FIELD_PREP(TSTCNTL_REG_BANK_SEL, bank) |
+			TSTCNTL_TEST_MODE |
+			FIELD_PREP(TSTCNTL_WRITE_ADDRESS, reg));
+
+out:
+	/* Close the bank access on our way out */
+	meson_gxl_close_banks(phydev);
+	return ret;
+}
+
+static int meson_gxl_config_init(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Write CONFIG_A6*/
+	ret = meson_gxl_write_reg(phydev, BANK_ANALOG_DSP, A6_CONFIG_REG,
+				  0x8e0d);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, TSTCNTL,
-			TSTCNTL_WRITE
-			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
-			| TSTCNTL_TEST_MODE
-			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_DIV1));
+
+	/* Enable fractional PLL */
+	ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_CONTROL, 0x5);
 	if (ret)
 		return ret;
 
 	/* Program fraction FR_PLL_DIV1 */
-	ret = phy_write(phydev, TSTWRITE, 0xaaaa);
+	ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_DIV1, 0x029a);
 	if (ret)
 		return ret;
-	ret = phy_write(phydev, TSTCNTL,
-			TSTCNTL_WRITE
-			| FIELD_PREP(TSTCNTL_REG_BANK_SEL, BANK_BIST)
-			| TSTCNTL_TEST_MODE
-			| FIELD_PREP(TSTCNTL_WRITE_ADDRESS, FR_PLL_DIV0));
+
+	/* Program fraction FR_PLL_DIV1 */
+	ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_DIV0, 0xaaaa);
 	if (ret)
 		return ret;
 
-- 
2.14.3

[PATCH net-next v2 4/8] net: phy: meson-gxl: use genphy_config_init

[Jerome Brunet]

Use the generic init function to populate some of the phydev
structure fields

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 05054770aefb..2e8c40df33c2 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -142,7 +142,7 @@ static int meson_gxl_config_init(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
-	return 0;
+	return genphy_config_init(phydev);
 }
 
 static struct phy_driver meson_gxl_phy[] = {
-- 
2.14.3

[PATCH net-next v2 4/8] net: phy: meson-gxl: use genphy_config_init

[Jerome Brunet]

Use the generic init function to populate some of the phydev
structure fields

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 05054770aefb..2e8c40df33c2 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -142,7 +142,7 @@ static int meson_gxl_config_init(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
-	return 0;
+	return genphy_config_init(phydev);
 }
 
 static struct phy_driver meson_gxl_phy[] = {
-- 
2.14.3

[PATCH net-next v2 4/8] net: phy: meson-gxl: use genphy_config_init

[Jerome Brunet]

Use the generic init function to populate some of the phydev
structure fields

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 05054770aefb..2e8c40df33c2 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -142,7 +142,7 @@ static int meson_gxl_config_init(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
-	return 0;
+	return genphy_config_init(phydev);
 }
 
 static struct phy_driver meson_gxl_phy[] = {
-- 
2.14.3

[PATCH net-next v2 4/8] net: phy: meson-gxl: use genphy_config_init

[Jerome Brunet]

Use the generic init function to populate some of the phydev
structure fields

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 05054770aefb..2e8c40df33c2 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -142,7 +142,7 @@ static int meson_gxl_config_init(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
-	return 0;
+	return genphy_config_init(phydev);
 }
 
 static struct phy_driver meson_gxl_phy[] = {
-- 
2.14.3

[PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption

[Jerome Brunet]

The purpose of this change is to fix the incorrect detection of the link
partner (LP) advertised capabilities which sometimes happens with this PHY
(roughly 1 time in a dozen)

This issue may cause the link to be negotiated at 10Mbps/Full or
10Mbps/Half when 100MBps/Full is actually possible. In some case, the link
is even completely broken and no communication is possible.

To detect the corruption, we must look for a magic undocumented bit in the
WOL bank (hint given by the SoC vendor kernel) but this is not enough to
cover all cases. We also have to look at the LPA ack. If the LP supports
Aneg but did not ack our base code when aneg is completed, we assume
something went wrong.

The detection of a corrupted LPA triggers a restart of the aneg process.
This solves the problem but may take up to 6 retries to complete.

Fixes: 7334b3e47aee ("net: phy: Add Meson GXL Internal PHY driver")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---

I suppose this patch probably seems a bit hacky, especially the part
about the link partner acknowledge. I'm trying to figure out if the
value in MII_LPA makes sense but I don't have such a deep knowledge
of the ethernet spec.

To me, it does not makes sense for the LP to support ANEG (Bit 1 in
MII_EXPENSION), the aneg to have successfully complete and, at the
same time, LP does not ACK our base code word, which we should have
sent during this aneg.

If you think this may have unintended consequences or if you have
an idea to this differently, feel free to let me know.

 drivers/net/phy/meson-gxl.c | 59 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 2e8c40df33c2..726e0eeed475 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -35,11 +35,16 @@
 #define TSTWRITE	23
 
 #define BANK_ANALOG_DSP		0
+#define BANK_WOL		1
 #define BANK_BIST		3
 
 /* Analog/DSP Registers */
 #define A6_CONFIG_REG	0x17
 
+/* WOL Registers */
+#define LPI_STATUS	0xc
+#define  LPI_STATUS_RSV12	BIT(12)
+
 /* BIST Registers */
 #define FR_PLL_CONTROL	0x1b
 #define FR_PLL_DIV0	0x1c
@@ -145,6 +150,58 @@ static int meson_gxl_config_init(struct phy_device *phydev)
 	return genphy_config_init(phydev);
 }
 
+/* This specific function is provided to cope with the possible failures of
+ * this phy during aneg process. When aneg fails, the PHY reports that aneg
+ * is done but the value found in MII_LPA is wrong:
+ *  - Early failures: MII_LPA is just 0x0001. if MII_EXPANSION reports that
+ *    the link partner (LP) supports aneg but the LP never acked our base
+ *    code word, it is likely that we never sent it to begin with.
+ *  - Late failures: MII_LPA is filled with a value which seems to make sense
+ *    but it actually is not what the LP is advertising. It seems that we
+ *    can detect this using a magic bit in the WOL bank (reg 12 - bit 12).
+ *    If this particular bit is not set when aneg is reported being done,
+ *    it means MII_LPA is likely to be wrong.
+ *
+ * In both case, forcing a restart of the aneg process solve the problem.
+ * When this failure happens, the first retry is usually successful but,
+ * in some cases, it may take up to 6 retries to get a decent result
+ */
+int meson_gxl_read_status(struct phy_device *phydev)
+{
+	int ret, wol, lpa, exp;
+
+	if (phydev->autoneg == AUTONEG_ENABLE) {
+		ret = genphy_aneg_done(phydev);
+		if (ret < 0)
+			return ret;
+		else if (!ret)
+			goto read_status_continue;
+
+		/* Aneg is done, let's check everything is fine */
+		wol = meson_gxl_read_reg(phydev, BANK_WOL, LPI_STATUS);
+		if (wol < 0)
+			return wol;
+
+		lpa = phy_read(phydev, MII_LPA);
+		if (lpa < 0)
+			return lpa;
+
+		exp = phy_read(phydev, MII_EXPANSION);
+		if (exp < 0)
+			return exp;
+
+		if (!(wol & LPI_STATUS_RSV12) ||
+		    ((exp & EXPANSION_NWAY) && !(lpa & LPA_LPACK))) {
+			/* Looks like aneg failed after all */
+			phydev_dbg(phydev, "LPA corruption - aneg restart\n");
+			return genphy_restart_aneg(phydev);
+		}
+	}
+
+read_status_continue:
+	return genphy_read_status(phydev);
+}
+
 static struct phy_driver meson_gxl_phy[] = {
 	{
 		.phy_id		= 0x01814400,
@@ -155,7 +212,7 @@ static struct phy_driver meson_gxl_phy[] = {
 		.config_init	= meson_gxl_config_init,
 		.config_aneg	= genphy_config_aneg,
 		.aneg_done      = genphy_aneg_done,
-		.read_status	= genphy_read_status,
+		.read_status	= meson_gxl_read_status,
 		.suspend        = genphy_suspend,
 		.resume         = genphy_resume,
 	},
-- 
2.14.3

[PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption

[Jerome Brunet]

The purpose of this change is to fix the incorrect detection of the link
partner (LP) advertised capabilities which sometimes happens with this PHY
(roughly 1 time in a dozen)

This issue may cause the link to be negotiated at 10Mbps/Full or
10Mbps/Half when 100MBps/Full is actually possible. In some case, the link
is even completely broken and no communication is possible.

To detect the corruption, we must look for a magic undocumented bit in the
WOL bank (hint given by the SoC vendor kernel) but this is not enough to
cover all cases. We also have to look at the LPA ack. If the LP supports
Aneg but did not ack our base code when aneg is completed, we assume
something went wrong.

The detection of a corrupted LPA triggers a restart of the aneg process.
This solves the problem but may take up to 6 retries to complete.

Fixes: 7334b3e47aee ("net: phy: Add Meson GXL Internal PHY driver")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---

I suppose this patch probably seems a bit hacky, especially the part
about the link partner acknowledge. I'm trying to figure out if the
value in MII_LPA makes sense but I don't have such a deep knowledge
of the ethernet spec.

To me, it does not makes sense for the LP to support ANEG (Bit 1 in
MII_EXPENSION), the aneg to have successfully complete and, at the
same time, LP does not ACK our base code word, which we should have
sent during this aneg.

If you think this may have unintended consequences or if you have
an idea to this differently, feel free to let me know.

 drivers/net/phy/meson-gxl.c | 59 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 2e8c40df33c2..726e0eeed475 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -35,11 +35,16 @@
 #define TSTWRITE	23
 
 #define BANK_ANALOG_DSP		0
+#define BANK_WOL		1
 #define BANK_BIST		3
 
 /* Analog/DSP Registers */
 #define A6_CONFIG_REG	0x17
 
+/* WOL Registers */
+#define LPI_STATUS	0xc
+#define  LPI_STATUS_RSV12	BIT(12)
+
 /* BIST Registers */
 #define FR_PLL_CONTROL	0x1b
 #define FR_PLL_DIV0	0x1c
@@ -145,6 +150,58 @@ static int meson_gxl_config_init(struct phy_device *phydev)
 	return genphy_config_init(phydev);
 }
 
+/* This specific function is provided to cope with the possible failures of
+ * this phy during aneg process. When aneg fails, the PHY reports that aneg
+ * is done but the value found in MII_LPA is wrong:
+ *  - Early failures: MII_LPA is just 0x0001. if MII_EXPANSION reports that
+ *    the link partner (LP) supports aneg but the LP never acked our base
+ *    code word, it is likely that we never sent it to begin with.
+ *  - Late failures: MII_LPA is filled with a value which seems to make sense
+ *    but it actually is not what the LP is advertising. It seems that we
+ *    can detect this using a magic bit in the WOL bank (reg 12 - bit 12).
+ *    If this particular bit is not set when aneg is reported being done,
+ *    it means MII_LPA is likely to be wrong.
+ *
+ * In both case, forcing a restart of the aneg process solve the problem.
+ * When this failure happens, the first retry is usually successful but,
+ * in some cases, it may take up to 6 retries to get a decent result
+ */
+int meson_gxl_read_status(struct phy_device *phydev)
+{
+	int ret, wol, lpa, exp;
+
+	if (phydev->autoneg == AUTONEG_ENABLE) {
+		ret = genphy_aneg_done(phydev);
+		if (ret < 0)
+			return ret;
+		else if (!ret)
+			goto read_status_continue;
+
+		/* Aneg is done, let's check everything is fine */
+		wol = meson_gxl_read_reg(phydev, BANK_WOL, LPI_STATUS);
+		if (wol < 0)
+			return wol;
+
+		lpa = phy_read(phydev, MII_LPA);
+		if (lpa < 0)
+			return lpa;
+
+		exp = phy_read(phydev, MII_EXPANSION);
+		if (exp < 0)
+			return exp;
+
+		if (!(wol & LPI_STATUS_RSV12) ||
+		    ((exp & EXPANSION_NWAY) && !(lpa & LPA_LPACK))) {
+			/* Looks like aneg failed after all */
+			phydev_dbg(phydev, "LPA corruption - aneg restart\n");
+			return genphy_restart_aneg(phydev);
+		}
+	}
+
+read_status_continue:
+	return genphy_read_status(phydev);
+}
+
 static struct phy_driver meson_gxl_phy[] = {
 	{
 		.phy_id		= 0x01814400,
@@ -155,7 +212,7 @@ static struct phy_driver meson_gxl_phy[] = {
 		.config_init	= meson_gxl_config_init,
 		.config_aneg	= genphy_config_aneg,
 		.aneg_done      = genphy_aneg_done,
-		.read_status	= genphy_read_status,
+		.read_status	= meson_gxl_read_status,
 		.suspend        = genphy_suspend,
 		.resume         = genphy_resume,
 	},
-- 
2.14.3

[PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption

[Jerome Brunet]

The purpose of this change is to fix the incorrect detection of the link
partner (LP) advertised capabilities which sometimes happens with this PHY
(roughly 1 time in a dozen)

This issue may cause the link to be negotiated at 10Mbps/Full or
10Mbps/Half when 100MBps/Full is actually possible. In some case, the link
is even completely broken and no communication is possible.

To detect the corruption, we must look for a magic undocumented bit in the
WOL bank (hint given by the SoC vendor kernel) but this is not enough to
cover all cases. We also have to look at the LPA ack. If the LP supports
Aneg but did not ack our base code when aneg is completed, we assume
something went wrong.

The detection of a corrupted LPA triggers a restart of the aneg process.
This solves the problem but may take up to 6 retries to complete.

Fixes: 7334b3e47aee ("net: phy: Add Meson GXL Internal PHY driver")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---

I suppose this patch probably seems a bit hacky, especially the part
about the link partner acknowledge. I'm trying to figure out if the
value in MII_LPA makes sense but I don't have such a deep knowledge
of the ethernet spec.

To me, it does not makes sense for the LP to support ANEG (Bit 1 in
MII_EXPENSION), the aneg to have successfully complete and, at the
same time, LP does not ACK our base code word, which we should have
sent during this aneg.

If you think this may have unintended consequences or if you have
an idea to this differently, feel free to let me know.

 drivers/net/phy/meson-gxl.c | 59 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 2e8c40df33c2..726e0eeed475 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -35,11 +35,16 @@
 #define TSTWRITE	23
 
 #define BANK_ANALOG_DSP		0
+#define BANK_WOL		1
 #define BANK_BIST		3
 
 /* Analog/DSP Registers */
 #define A6_CONFIG_REG	0x17
 
+/* WOL Registers */
+#define LPI_STATUS	0xc
+#define  LPI_STATUS_RSV12	BIT(12)
+
 /* BIST Registers */
 #define FR_PLL_CONTROL	0x1b
 #define FR_PLL_DIV0	0x1c
@@ -145,6 +150,58 @@ static int meson_gxl_config_init(struct phy_device *phydev)
 	return genphy_config_init(phydev);
 }
 
+/* This specific function is provided to cope with the possible failures of
+ * this phy during aneg process. When aneg fails, the PHY reports that aneg
+ * is done but the value found in MII_LPA is wrong:
+ *  - Early failures: MII_LPA is just 0x0001. if MII_EXPANSION reports that
+ *    the link partner (LP) supports aneg but the LP never acked our base
+ *    code word, it is likely that we never sent it to begin with.
+ *  - Late failures: MII_LPA is filled with a value which seems to make sense
+ *    but it actually is not what the LP is advertising. It seems that we
+ *    can detect this using a magic bit in the WOL bank (reg 12 - bit 12).
+ *    If this particular bit is not set when aneg is reported being done,
+ *    it means MII_LPA is likely to be wrong.
+ *
+ * In both case, forcing a restart of the aneg process solve the problem.
+ * When this failure happens, the first retry is usually successful but,
+ * in some cases, it may take up to 6 retries to get a decent result
+ */
+int meson_gxl_read_status(struct phy_device *phydev)
+{
+	int ret, wol, lpa, exp;
+
+	if (phydev->autoneg == AUTONEG_ENABLE) {
+		ret = genphy_aneg_done(phydev);
+		if (ret < 0)
+			return ret;
+		else if (!ret)
+			goto read_status_continue;
+
+		/* Aneg is done, let's check everything is fine */
+		wol = meson_gxl_read_reg(phydev, BANK_WOL, LPI_STATUS);
+		if (wol < 0)
+			return wol;
+
+		lpa = phy_read(phydev, MII_LPA);
+		if (lpa < 0)
+			return lpa;
+
+		exp = phy_read(phydev, MII_EXPANSION);
+		if (exp < 0)
+			return exp;
+
+		if (!(wol & LPI_STATUS_RSV12) ||
+		    ((exp & EXPANSION_NWAY) && !(lpa & LPA_LPACK))) {
+			/* Looks like aneg failed after all */
+			phydev_dbg(phydev, "LPA corruption - aneg restart\n");
+			return genphy_restart_aneg(phydev);
+		}
+	}
+
+read_status_continue:
+	return genphy_read_status(phydev);
+}
+
 static struct phy_driver meson_gxl_phy[] = {
 	{
 		.phy_id		= 0x01814400,
@@ -155,7 +212,7 @@ static struct phy_driver meson_gxl_phy[] = {
 		.config_init	= meson_gxl_config_init,
 		.config_aneg	= genphy_config_aneg,
 		.aneg_done      = genphy_aneg_done,
-		.read_status	= genphy_read_status,
+		.read_status	= meson_gxl_read_status,
 		.suspend        = genphy_suspend,
 		.resume         = genphy_resume,
 	},
-- 
2.14.3

[PATCH net-next v2 6/8] net: phy: meson-gxl: leave CONFIG_A6 untouched

[Jerome Brunet]

The PHY performs just as well when left in its default configuration and
it makes senses because this poke gets reset just after init.

According to the documentation, all registers in the Analog/DSP bank are
reset when there is a mode switch from 10BT to 100BT.

In the end, we have used the default configuration so far and there is no
reason to change now. Remove CONFIG_A6 poke to make this clear.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---

Out of curiosity, I tried to re-apply the ANALOG/DSP settings on speed
changes (patch available here [0] if someone wants to try) but I did
not notice any change as a result. In the end, I thought it was safer
to keep on using the ANALOG settings we have been actually using so far,
everybody seems to be happy with them

[0]: https://github.com/jeromebrunet/linux/commit/b594288e629a61574e76112497474fd3cf46c781

 drivers/net/phy/meson-gxl.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 726e0eeed475..5325940fe899 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -38,9 +38,6 @@
 #define BANK_WOL		1
 #define BANK_BIST		3
 
-/* Analog/DSP Registers */
-#define A6_CONFIG_REG	0x17
-
 /* WOL Registers */
 #define LPI_STATUS	0xc
 #define  LPI_STATUS_RSV12	BIT(12)
@@ -126,12 +123,6 @@ static int meson_gxl_config_init(struct phy_device *phydev)
 {
 	int ret;
 
-	/* Write CONFIG_A6*/
-	ret = meson_gxl_write_reg(phydev, BANK_ANALOG_DSP, A6_CONFIG_REG,
-				  0x8e0d);
-	if (ret)
-		return ret;
-
 	/* Enable fractional PLL */
 	ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_CONTROL, 0x5);
 	if (ret)
-- 
2.14.3

[PATCH net-next v2 6/8] net: phy: meson-gxl: leave CONFIG_A6 untouched

[Jerome Brunet]

The PHY performs just as well when left in its default configuration and
it makes senses because this poke gets reset just after init.

According to the documentation, all registers in the Analog/DSP bank are
reset when there is a mode switch from 10BT to 100BT.

In the end, we have used the default configuration so far and there is no
reason to change now. Remove CONFIG_A6 poke to make this clear.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---

Out of curiosity, I tried to re-apply the ANALOG/DSP settings on speed
changes (patch available here [0] if someone wants to try) but I did
not notice any change as a result. In the end, I thought it was safer
to keep on using the ANALOG settings we have been actually using so far,
everybody seems to be happy with them

[0]: https://github.com/jeromebrunet/linux/commit/b594288e629a61574e76112497474fd3cf46c781

 drivers/net/phy/meson-gxl.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 726e0eeed475..5325940fe899 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -38,9 +38,6 @@
 #define BANK_WOL		1
 #define BANK_BIST		3
 
-/* Analog/DSP Registers */
-#define A6_CONFIG_REG	0x17
-
 /* WOL Registers */
 #define LPI_STATUS	0xc
 #define  LPI_STATUS_RSV12	BIT(12)
@@ -126,12 +123,6 @@ static int meson_gxl_config_init(struct phy_device *phydev)
 {
 	int ret;
 
-	/* Write CONFIG_A6*/
-	ret = meson_gxl_write_reg(phydev, BANK_ANALOG_DSP, A6_CONFIG_REG,
-				  0x8e0d);
-	if (ret)
-		return ret;
-
 	/* Enable fractional PLL */
 	ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_CONTROL, 0x5);
 	if (ret)
-- 
2.14.3

[PATCH net-next v2 6/8] net: phy: meson-gxl: leave CONFIG_A6 untouched

[Jerome Brunet]

The PHY performs just as well when left in its default configuration and
it makes senses because this poke gets reset just after init.

According to the documentation, all registers in the Analog/DSP bank are
reset when there is a mode switch from 10BT to 100BT.

In the end, we have used the default configuration so far and there is no
reason to change now. Remove CONFIG_A6 poke to make this clear.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---

Out of curiosity, I tried to re-apply the ANALOG/DSP settings on speed
changes (patch available here [0] if someone wants to try) but I did
not notice any change as a result. In the end, I thought it was safer
to keep on using the ANALOG settings we have been actually using so far,
everybody seems to be happy with them

[0]: https://github.com/jeromebrunet/linux/commit/b594288e629a61574e76112497474fd3cf46c781

 drivers/net/phy/meson-gxl.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 726e0eeed475..5325940fe899 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -38,9 +38,6 @@
 #define BANK_WOL		1
 #define BANK_BIST		3
 
-/* Analog/DSP Registers */
-#define A6_CONFIG_REG	0x17
-
 /* WOL Registers */
 #define LPI_STATUS	0xc
 #define  LPI_STATUS_RSV12	BIT(12)
@@ -126,12 +123,6 @@ static int meson_gxl_config_init(struct phy_device *phydev)
 {
 	int ret;
 
-	/* Write CONFIG_A6*/
-	ret = meson_gxl_write_reg(phydev, BANK_ANALOG_DSP, A6_CONFIG_REG,
-				  0x8e0d);
-	if (ret)
-		return ret;
-
 	/* Enable fractional PLL */
 	ret = meson_gxl_write_reg(phydev, BANK_BIST, FR_PLL_CONTROL, 0x5);
 	if (ret)
-- 
2.14.3

[PATCH net-next v2 7/8] net: phy: meson-gxl: add interrupt support

[Jerome Brunet]

Enable interrupt support in meson-gxl PHY driver

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 5325940fe899..861b021b9758 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -33,6 +33,14 @@
 #define  TSTCNTL_WRITE_ADDRESS	GENMASK(4, 0)
 #define TSTREAD1	21
 #define TSTWRITE	23
+#define INTSRC_FLAG	29
+#define  INTSRC_ANEG_PR		BIT(1)
+#define  INTSRC_PARALLEL_FAULT	BIT(2)
+#define  INTSRC_ANEG_LP_ACK	BIT(3)
+#define  INTSRC_LINK_DOWN	BIT(4)
+#define  INTSRC_REMOTE_FAULT	BIT(5)
+#define  INTSRC_ANEG_COMPLETE	BIT(6)
+#define INTSRC_MASK	30
 
 #define BANK_ANALOG_DSP		0
 #define BANK_WOL		1
@@ -193,17 +201,44 @@ int meson_gxl_read_status(struct phy_device *phydev)
 	return genphy_read_status(phydev);
 }
 
+static int meson_gxl_ack_interrupt(struct phy_device *phydev)
+{
+	int ret = phy_read(phydev, INTSRC_FLAG);
+
+	return ret < 0 ? ret : 0;
+}
+
+static int meson_gxl_config_intr(struct phy_device *phydev)
+{
+	u16 val;
+
+	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+		val = INTSRC_ANEG_PR
+			| INTSRC_PARALLEL_FAULT
+			| INTSRC_ANEG_LP_ACK
+			| INTSRC_LINK_DOWN
+			| INTSRC_REMOTE_FAULT
+			| INTSRC_ANEG_COMPLETE;
+	} else {
+		val = 0;
+	}
+
+	return phy_write(phydev, INTSRC_MASK, val);
+}
+
 static struct phy_driver meson_gxl_phy[] = {
 	{
 		.phy_id		= 0x01814400,
 		.phy_id_mask	= 0xfffffff0,
 		.name		= "Meson GXL Internal PHY",
 		.features	= PHY_BASIC_FEATURES,
-		.flags		= PHY_IS_INTERNAL,
+		.flags		= PHY_IS_INTERNAL | PHY_HAS_INTERRUPT,
 		.config_init	= meson_gxl_config_init,
 		.config_aneg	= genphy_config_aneg,
 		.aneg_done      = genphy_aneg_done,
 		.read_status	= meson_gxl_read_status,
+		.ack_interrupt	= meson_gxl_ack_interrupt,
+		.config_intr	= meson_gxl_config_intr,
 		.suspend        = genphy_suspend,
 		.resume         = genphy_resume,
 	},
-- 
2.14.3

[PATCH net-next v2 7/8] net: phy: meson-gxl: add interrupt support

[Jerome Brunet]

Enable interrupt support in meson-gxl PHY driver

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 5325940fe899..861b021b9758 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -33,6 +33,14 @@
 #define  TSTCNTL_WRITE_ADDRESS	GENMASK(4, 0)
 #define TSTREAD1	21
 #define TSTWRITE	23
+#define INTSRC_FLAG	29
+#define  INTSRC_ANEG_PR		BIT(1)
+#define  INTSRC_PARALLEL_FAULT	BIT(2)
+#define  INTSRC_ANEG_LP_ACK	BIT(3)
+#define  INTSRC_LINK_DOWN	BIT(4)
+#define  INTSRC_REMOTE_FAULT	BIT(5)
+#define  INTSRC_ANEG_COMPLETE	BIT(6)
+#define INTSRC_MASK	30
 
 #define BANK_ANALOG_DSP		0
 #define BANK_WOL		1
@@ -193,17 +201,44 @@ int meson_gxl_read_status(struct phy_device *phydev)
 	return genphy_read_status(phydev);
 }
 
+static int meson_gxl_ack_interrupt(struct phy_device *phydev)
+{
+	int ret = phy_read(phydev, INTSRC_FLAG);
+
+	return ret < 0 ? ret : 0;
+}
+
+static int meson_gxl_config_intr(struct phy_device *phydev)
+{
+	u16 val;
+
+	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+		val = INTSRC_ANEG_PR
+			| INTSRC_PARALLEL_FAULT
+			| INTSRC_ANEG_LP_ACK
+			| INTSRC_LINK_DOWN
+			| INTSRC_REMOTE_FAULT
+			| INTSRC_ANEG_COMPLETE;
+	} else {
+		val = 0;
+	}
+
+	return phy_write(phydev, INTSRC_MASK, val);
+}
+
 static struct phy_driver meson_gxl_phy[] = {
 	{
 		.phy_id		= 0x01814400,
 		.phy_id_mask	= 0xfffffff0,
 		.name		= "Meson GXL Internal PHY",
 		.features	= PHY_BASIC_FEATURES,
-		.flags		= PHY_IS_INTERNAL,
+		.flags		= PHY_IS_INTERNAL | PHY_HAS_INTERRUPT,
 		.config_init	= meson_gxl_config_init,
 		.config_aneg	= genphy_config_aneg,
 		.aneg_done      = genphy_aneg_done,
 		.read_status	= meson_gxl_read_status,
+		.ack_interrupt	= meson_gxl_ack_interrupt,
+		.config_intr	= meson_gxl_config_intr,
 		.suspend        = genphy_suspend,
 		.resume         = genphy_resume,
 	},
-- 
2.14.3

[PATCH net-next v2 7/8] net: phy: meson-gxl: add interrupt support

[Jerome Brunet]

Enable interrupt support in meson-gxl PHY driver

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 5325940fe899..861b021b9758 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -33,6 +33,14 @@
 #define  TSTCNTL_WRITE_ADDRESS	GENMASK(4, 0)
 #define TSTREAD1	21
 #define TSTWRITE	23
+#define INTSRC_FLAG	29
+#define  INTSRC_ANEG_PR		BIT(1)
+#define  INTSRC_PARALLEL_FAULT	BIT(2)
+#define  INTSRC_ANEG_LP_ACK	BIT(3)
+#define  INTSRC_LINK_DOWN	BIT(4)
+#define  INTSRC_REMOTE_FAULT	BIT(5)
+#define  INTSRC_ANEG_COMPLETE	BIT(6)
+#define INTSRC_MASK	30
 
 #define BANK_ANALOG_DSP		0
 #define BANK_WOL		1
@@ -193,17 +201,44 @@ int meson_gxl_read_status(struct phy_device *phydev)
 	return genphy_read_status(phydev);
 }
 
+static int meson_gxl_ack_interrupt(struct phy_device *phydev)
+{
+	int ret = phy_read(phydev, INTSRC_FLAG);
+
+	return ret < 0 ? ret : 0;
+}
+
+static int meson_gxl_config_intr(struct phy_device *phydev)
+{
+	u16 val;
+
+	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+		val = INTSRC_ANEG_PR
+			| INTSRC_PARALLEL_FAULT
+			| INTSRC_ANEG_LP_ACK
+			| INTSRC_LINK_DOWN
+			| INTSRC_REMOTE_FAULT
+			| INTSRC_ANEG_COMPLETE;
+	} else {
+		val = 0;
+	}
+
+	return phy_write(phydev, INTSRC_MASK, val);
+}
+
 static struct phy_driver meson_gxl_phy[] = {
 	{
 		.phy_id		= 0x01814400,
 		.phy_id_mask	= 0xfffffff0,
 		.name		= "Meson GXL Internal PHY",
 		.features	= PHY_BASIC_FEATURES,
-		.flags		= PHY_IS_INTERNAL,
+		.flags		= PHY_IS_INTERNAL | PHY_HAS_INTERRUPT,
 		.config_init	= meson_gxl_config_init,
 		.config_aneg	= genphy_config_aneg,
 		.aneg_done      = genphy_aneg_done,
 		.read_status	= meson_gxl_read_status,
+		.ack_interrupt	= meson_gxl_ack_interrupt,
+		.config_intr	= meson_gxl_config_intr,
 		.suspend        = genphy_suspend,
 		.resume         = genphy_resume,
 	},
-- 
2.14.3

[PATCH net-next v2 8/8] net: phy: meson-gxl: join the authors

[Jerome Brunet]

Following previous changes, join the other authors of this driver and
take the blame with them

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 861b021b9758..4cd5b2622ae1 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -256,4 +256,5 @@ MODULE_DEVICE_TABLE(mdio, meson_gxl_tbl);
 MODULE_DESCRIPTION("Amlogic Meson GXL Internal PHY driver");
 MODULE_AUTHOR("Baoqi wang");
 MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
+MODULE_AUTHOR("Jerome Brunet <jbrunet@baylibre.com>");
 MODULE_LICENSE("GPL");
-- 
2.14.3

[PATCH net-next v2 8/8] net: phy: meson-gxl: join the authors

[Jerome Brunet]

Following previous changes, join the other authors of this driver and
take the blame with them

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 861b021b9758..4cd5b2622ae1 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -256,4 +256,5 @@ MODULE_DEVICE_TABLE(mdio, meson_gxl_tbl);
 MODULE_DESCRIPTION("Amlogic Meson GXL Internal PHY driver");
 MODULE_AUTHOR("Baoqi wang");
 MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
+MODULE_AUTHOR("Jerome Brunet <jbrunet@baylibre.com>");
 MODULE_LICENSE("GPL");
-- 
2.14.3

[PATCH net-next v2 8/8] net: phy: meson-gxl: join the authors

[Jerome Brunet]

Following previous changes, join the other authors of this driver and
take the blame with them

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/net/phy/meson-gxl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
index 861b021b9758..4cd5b2622ae1 100644
--- a/drivers/net/phy/meson-gxl.c
+++ b/drivers/net/phy/meson-gxl.c
@@ -256,4 +256,5 @@ MODULE_DEVICE_TABLE(mdio, meson_gxl_tbl);
 MODULE_DESCRIPTION("Amlogic Meson GXL Internal PHY driver");
 MODULE_AUTHOR("Baoqi wang");
 MODULE_AUTHOR("Neil Armstrong <narmstrong@baylibre.com>");
+MODULE_AUTHOR("Jerome Brunet <jbrunet@baylibre.com>");
 MODULE_LICENSE("GPL");
-- 
2.14.3

Re: [PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:12PM +0100, Jerome Brunet wrote:
> The purpose of this change is to fix the incorrect detection of the link
> partner (LP) advertised capabilities which sometimes happens with this PHY
> (roughly 1 time in a dozen)

Hi Jerome

Since this is a real fix, please send it alone. Base it on net, not
net-next.  David will then get it applied to stable, so it will be
backported.

Thanks
	Andrew

[PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:12PM +0100, Jerome Brunet wrote:
> The purpose of this change is to fix the incorrect detection of the link
> partner (LP) advertised capabilities which sometimes happens with this PHY
> (roughly 1 time in a dozen)

Hi Jerome

Since this is a real fix, please send it alone. Base it on net, not
net-next.  David will then get it applied to stable, so it will be
backported.

Thanks
	Andrew

[PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:12PM +0100, Jerome Brunet wrote:
> The purpose of this change is to fix the incorrect detection of the link
> partner (LP) advertised capabilities which sometimes happens with this PHY
> (roughly 1 time in a dozen)

Hi Jerome

Since this is a real fix, please send it alone. Base it on net, not
net-next.  David will then get it applied to stable, so it will be
backported.

Thanks
	Andrew

Re: [PATCH net-next v2 1/8] net: phy: meson-gxl: check phy_write return value

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:08PM +0100, Jerome Brunet wrote:
> Always check phy_write return values. Better to be safe than sorry
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next v2 1/8] net: phy: meson-gxl: check phy_write return value

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:08PM +0100, Jerome Brunet wrote:
> Always check phy_write return values. Better to be safe than sorry
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next v2 1/8] net: phy: meson-gxl: check phy_write return value

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:08PM +0100, Jerome Brunet wrote:
> Always check phy_write return values. Better to be safe than sorry
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption

[Jerome Brunet]

On Thu, 2017-12-07 at 16:34 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:12PM +0100, Jerome Brunet wrote:
> > The purpose of this change is to fix the incorrect detection of the link
> > partner (LP) advertised capabilities which sometimes happens with this PHY
> > (roughly 1 time in a dozen)
> 
> Hi Jerome
> 
> Since this is a real fix, please send it alone. Base it on net, not
> net-next.  David will then get it applied to stable, so it will be
> backported.

I hesitated on this. For this fix to be "not too ugly" I need at least the
helper functions (patch 1 to 3) ... which are not really fixes.

Would it be Ok if send patches 1 to 5 to net ?
and 6 to 8 separately on net-next ?

> 
> Thanks
> 	Andrew

[PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption

[Jerome Brunet]

On Thu, 2017-12-07 at 16:34 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:12PM +0100, Jerome Brunet wrote:
> > The purpose of this change is to fix the incorrect detection of the link
> > partner (LP) advertised capabilities which sometimes happens with this PHY
> > (roughly 1 time in a dozen)
> 
> Hi Jerome
> 
> Since this is a real fix, please send it alone. Base it on net, not
> net-next.  David will then get it applied to stable, so it will be
> backported.

I hesitated on this. For this fix to be "not too ugly" I need at least the
helper functions (patch 1 to 3) ... which are not really fixes.

Would it be Ok if send patches 1 to 5 to net ?
and 6 to 8 separately on net-next ?

> 
> Thanks
> 	Andrew

[PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption

[Jerome Brunet]

On Thu, 2017-12-07 at 16:34 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:12PM +0100, Jerome Brunet wrote:
> > The purpose of this change is to fix the incorrect detection of the link
> > partner (LP) advertised capabilities which sometimes happens with this PHY
> > (roughly 1 time in a dozen)
> 
> Hi Jerome
> 
> Since this is a real fix, please send it alone. Base it on net, not
> net-next.  David will then get it applied to stable, so it will be
> backported.

I hesitated on this. For this fix to be "not too ugly" I need at least the
helper functions (patch 1 to 3) ... which are not really fixes.

Would it be Ok if send patches 1 to 5 to net ?
and 6 to 8 separately on net-next ?

> 
> Thanks
> 	Andrew

Re: [PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:10PM +0100, Jerome Brunet wrote:
> Add read and write helpers to manipulate banked registers on this PHY
> This helps clarify the settings applied to these registers in the init
> function and upcoming changes.
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  drivers/net/phy/meson-gxl.c | 103 ++++++++++++++++++++++++++++----------------
>  1 file changed, 67 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
> index d82aa8cea401..05054770aefb 100644
> --- a/drivers/net/phy/meson-gxl.c
> +++ b/drivers/net/phy/meson-gxl.c
> @@ -45,11 +45,13 @@
>  #define FR_PLL_DIV0	0x1c
>  #define FR_PLL_DIV1	0x1d
>  
> -static int meson_gxl_config_init(struct phy_device *phydev)
> +static int meson_gxl_open_banks(struct phy_device *phydev)

Hi Jerome

Does the word bank come from the datasheet? Most of the phy drives use
page instead.

Also, we have discovered a race condition which affects drivers using
pages, which can lead to corruption of registers. At some point, i
expect we will be adding helpers to access paged registers, which do
the right thing with respect to locks.

So it would be nice if you used the work page, not bank.

   Thanks

      Andrew

[PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:10PM +0100, Jerome Brunet wrote:
> Add read and write helpers to manipulate banked registers on this PHY
> This helps clarify the settings applied to these registers in the init
> function and upcoming changes.
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  drivers/net/phy/meson-gxl.c | 103 ++++++++++++++++++++++++++++----------------
>  1 file changed, 67 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
> index d82aa8cea401..05054770aefb 100644
> --- a/drivers/net/phy/meson-gxl.c
> +++ b/drivers/net/phy/meson-gxl.c
> @@ -45,11 +45,13 @@
>  #define FR_PLL_DIV0	0x1c
>  #define FR_PLL_DIV1	0x1d
>  
> -static int meson_gxl_config_init(struct phy_device *phydev)
> +static int meson_gxl_open_banks(struct phy_device *phydev)

Hi Jerome

Does the word bank come from the datasheet? Most of the phy drives use
page instead.

Also, we have discovered a race condition which affects drivers using
pages, which can lead to corruption of registers. At some point, i
expect we will be adding helpers to access paged registers, which do
the right thing with respect to locks.

So it would be nice if you used the work page, not bank.

   Thanks

      Andrew

[PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:10PM +0100, Jerome Brunet wrote:
> Add read and write helpers to manipulate banked registers on this PHY
> This helps clarify the settings applied to these registers in the init
> function and upcoming changes.
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  drivers/net/phy/meson-gxl.c | 103 ++++++++++++++++++++++++++++----------------
>  1 file changed, 67 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
> index d82aa8cea401..05054770aefb 100644
> --- a/drivers/net/phy/meson-gxl.c
> +++ b/drivers/net/phy/meson-gxl.c
> @@ -45,11 +45,13 @@
>  #define FR_PLL_DIV0	0x1c
>  #define FR_PLL_DIV1	0x1d
>  
> -static int meson_gxl_config_init(struct phy_device *phydev)
> +static int meson_gxl_open_banks(struct phy_device *phydev)

Hi Jerome

Does the word bank come from the datasheet? Most of the phy drives use
page instead.

Also, we have discovered a race condition which affects drivers using
pages, which can lead to corruption of registers. At some point, i
expect we will be adding helpers to access paged registers, which do
the right thing with respect to locks.

So it would be nice if you used the work page, not bank.

   Thanks

      Andrew

Re: [PATCH net-next v2 4/8] net: phy: meson-gxl: use genphy_config_init

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:11PM +0100, Jerome Brunet wrote:
> Use the generic init function to populate some of the phydev
> structure fields
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v2 4/8] net: phy: meson-gxl: use genphy_config_init

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:11PM +0100, Jerome Brunet wrote:
> Use the generic init function to populate some of the phydev
> structure fields
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next v2 4/8] net: phy: meson-gxl: use genphy_config_init

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:11PM +0100, Jerome Brunet wrote:
> Use the generic init function to populate some of the phydev
> structure fields
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next v2 4/8] net: phy: meson-gxl: use genphy_config_init

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:11PM +0100, Jerome Brunet wrote:
> Use the generic init function to populate some of the phydev
> structure fields
> 
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Neil Armstrong]

On 07/12/2017 16:46, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:10PM +0100, Jerome Brunet wrote:
>> Add read and write helpers to manipulate banked registers on this PHY
>> This helps clarify the settings applied to these registers in the init
>> function and upcoming changes.
>>
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>>  drivers/net/phy/meson-gxl.c | 103 ++++++++++++++++++++++++++++----------------
>>  1 file changed, 67 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
>> index d82aa8cea401..05054770aefb 100644
>> --- a/drivers/net/phy/meson-gxl.c
>> +++ b/drivers/net/phy/meson-gxl.c
>> @@ -45,11 +45,13 @@
>>  #define FR_PLL_DIV0	0x1c
>>  #define FR_PLL_DIV1	0x1d
>>  
>> -static int meson_gxl_config_init(struct phy_device *phydev)
>> +static int meson_gxl_open_banks(struct phy_device *phydev)
> 
> Hi Jerome
> 
> Does the word bank come from the datasheet? Most of the phy drives use
> page instead.

Yes, it's explicitly described as banks in the datasheet.

Neil

> 
> Also, we have discovered a race condition which affects drivers using
> pages, which can lead to corruption of registers. At some point, i
> expect we will be adding helpers to access paged registers, which do
> the right thing with respect to locks.
> 
> So it would be nice if you used the work page, not bank.
> 
>    Thanks
> 
>       Andrew
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Neil Armstrong]

On 07/12/2017 16:46, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:10PM +0100, Jerome Brunet wrote:
>> Add read and write helpers to manipulate banked registers on this PHY
>> This helps clarify the settings applied to these registers in the init
>> function and upcoming changes.
>>
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>>  drivers/net/phy/meson-gxl.c | 103 ++++++++++++++++++++++++++++----------------
>>  1 file changed, 67 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
>> index d82aa8cea401..05054770aefb 100644
>> --- a/drivers/net/phy/meson-gxl.c
>> +++ b/drivers/net/phy/meson-gxl.c
>> @@ -45,11 +45,13 @@
>>  #define FR_PLL_DIV0	0x1c
>>  #define FR_PLL_DIV1	0x1d
>>  
>> -static int meson_gxl_config_init(struct phy_device *phydev)
>> +static int meson_gxl_open_banks(struct phy_device *phydev)
> 
> Hi Jerome
> 
> Does the word bank come from the datasheet? Most of the phy drives use
> page instead.

Yes, it's explicitly described as banks in the datasheet.

Neil

> 
> Also, we have discovered a race condition which affects drivers using
> pages, which can lead to corruption of registers. At some point, i
> expect we will be adding helpers to access paged registers, which do
> the right thing with respect to locks.
> 
> So it would be nice if you used the work page, not bank.
> 
>    Thanks
> 
>       Andrew
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Neil Armstrong]

On 07/12/2017 16:46, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:10PM +0100, Jerome Brunet wrote:
>> Add read and write helpers to manipulate banked registers on this PHY
>> This helps clarify the settings applied to these registers in the init
>> function and upcoming changes.
>>
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>> ---
>>  drivers/net/phy/meson-gxl.c | 103 ++++++++++++++++++++++++++++----------------
>>  1 file changed, 67 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
>> index d82aa8cea401..05054770aefb 100644
>> --- a/drivers/net/phy/meson-gxl.c
>> +++ b/drivers/net/phy/meson-gxl.c
>> @@ -45,11 +45,13 @@
>>  #define FR_PLL_DIV0	0x1c
>>  #define FR_PLL_DIV1	0x1d
>>  
>> -static int meson_gxl_config_init(struct phy_device *phydev)
>> +static int meson_gxl_open_banks(struct phy_device *phydev)
> 
> Hi Jerome
> 
> Does the word bank come from the datasheet? Most of the phy drives use
> page instead.

Yes, it's explicitly described as banks in the datasheet.

Neil

> 
> Also, we have discovered a race condition which affects drivers using
> pages, which can lead to corruption of registers. At some point, i
> expect we will be adding helpers to access paged registers, which do
> the right thing with respect to locks.
> 
> So it would be nice if you used the work page, not bank.
> 
>    Thanks
> 
>       Andrew
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [PATCH net-next v2 6/8] net: phy: meson-gxl: leave CONFIG_A6 untouched

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:13PM +0100, Jerome Brunet wrote:
> The PHY performs just as well when left in its default configuration and
> it makes senses because this poke gets reset just after init.

The only thing which might speak against this, is some bootloader
which sets something other than the default, and here we put it back
to the value it should have. But if you say a reset will put it back
to the default value anyway, this seems save.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v2 6/8] net: phy: meson-gxl: leave CONFIG_A6 untouched

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:13PM +0100, Jerome Brunet wrote:
> The PHY performs just as well when left in its default configuration and
> it makes senses because this poke gets reset just after init.

The only thing which might speak against this, is some bootloader
which sets something other than the default, and here we put it back
to the value it should have. But if you say a reset will put it back
to the default value anyway, this seems save.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next v2 6/8] net: phy: meson-gxl: leave CONFIG_A6 untouched

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:13PM +0100, Jerome Brunet wrote:
> The PHY performs just as well when left in its default configuration and
> it makes senses because this poke gets reset just after init.

The only thing which might speak against this, is some bootloader
which sets something other than the default, and here we put it back
to the value it should have. But if you say a reset will put it back
to the default value anyway, this seems save.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next v2 6/8] net: phy: meson-gxl: leave CONFIG_A6 untouched

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:13PM +0100, Jerome Brunet wrote:
> The PHY performs just as well when left in its default configuration and
> it makes senses because this poke gets reset just after init.

The only thing which might speak against this, is some bootloader
which sets something other than the default, and here we put it back
to the value it should have. But if you say a reset will put it back
to the default value anyway, this seems save.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Jerome Brunet]

On Thu, 2017-12-07 at 16:46 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:10PM +0100, Jerome Brunet wrote:
> > Add read and write helpers to manipulate banked registers on this PHY
> > This helps clarify the settings applied to these registers in the init
> > function and upcoming changes.
> > 
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > ---
> >  drivers/net/phy/meson-gxl.c | 103 ++++++++++++++++++++++++++++-------------
> > ---
> >  1 file changed, 67 insertions(+), 36 deletions(-)
> > 
> > diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
> > index d82aa8cea401..05054770aefb 100644
> > --- a/drivers/net/phy/meson-gxl.c
> > +++ b/drivers/net/phy/meson-gxl.c
> > @@ -45,11 +45,13 @@
> >  #define FR_PLL_DIV0	0x1c
> >  #define FR_PLL_DIV1	0x1d
> >  
> > -static int meson_gxl_config_init(struct phy_device *phydev)
> > +static int meson_gxl_open_banks(struct phy_device *phydev)
> 
> Hi Jerome
> 
> Does the word bank come from the datasheet? Most of the phy drives use
> page instead.
> 
> Also, we have discovered a race condition which affects drivers using
> pages, which can lead to corruption of registers. At some point, i
> expect we will be adding helpers to access paged registers, which do
> the right thing with respect to locks.
> 
> So it would be nice if you used the work page, not bank.

Banks actually comes from the datasheet, Yes.
I don't mind renaming it but I would be making things up. As you wish ?

Does the usual pages comes with this weird toggle thing to open the access ?
Would we able to use these generic helpers with our this kind of quirks ?

> 
>    Thanks
> 
>       Andrew

[PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Jerome Brunet]

On Thu, 2017-12-07 at 16:46 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:10PM +0100, Jerome Brunet wrote:
> > Add read and write helpers to manipulate banked registers on this PHY
> > This helps clarify the settings applied to these registers in the init
> > function and upcoming changes.
> > 
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > ---
> >  drivers/net/phy/meson-gxl.c | 103 ++++++++++++++++++++++++++++-------------
> > ---
> >  1 file changed, 67 insertions(+), 36 deletions(-)
> > 
> > diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
> > index d82aa8cea401..05054770aefb 100644
> > --- a/drivers/net/phy/meson-gxl.c
> > +++ b/drivers/net/phy/meson-gxl.c
> > @@ -45,11 +45,13 @@
> >  #define FR_PLL_DIV0	0x1c
> >  #define FR_PLL_DIV1	0x1d
> >  
> > -static int meson_gxl_config_init(struct phy_device *phydev)
> > +static int meson_gxl_open_banks(struct phy_device *phydev)
> 
> Hi Jerome
> 
> Does the word bank come from the datasheet? Most of the phy drives use
> page instead.
> 
> Also, we have discovered a race condition which affects drivers using
> pages, which can lead to corruption of registers. At some point, i
> expect we will be adding helpers to access paged registers, which do
> the right thing with respect to locks.
> 
> So it would be nice if you used the work page, not bank.

Banks actually comes from the datasheet, Yes.
I don't mind renaming it but I would be making things up. As you wish ?

Does the usual pages comes with this weird toggle thing to open the access ?
Would we able to use these generic helpers with our this kind of quirks ?

> 
>    Thanks
> 
>       Andrew

[PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Jerome Brunet]

On Thu, 2017-12-07 at 16:46 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:10PM +0100, Jerome Brunet wrote:
> > Add read and write helpers to manipulate banked registers on this PHY
> > This helps clarify the settings applied to these registers in the init
> > function and upcoming changes.
> > 
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > ---
> >  drivers/net/phy/meson-gxl.c | 103 ++++++++++++++++++++++++++++-------------
> > ---
> >  1 file changed, 67 insertions(+), 36 deletions(-)
> > 
> > diff --git a/drivers/net/phy/meson-gxl.c b/drivers/net/phy/meson-gxl.c
> > index d82aa8cea401..05054770aefb 100644
> > --- a/drivers/net/phy/meson-gxl.c
> > +++ b/drivers/net/phy/meson-gxl.c
> > @@ -45,11 +45,13 @@
> >  #define FR_PLL_DIV0	0x1c
> >  #define FR_PLL_DIV1	0x1d
> >  
> > -static int meson_gxl_config_init(struct phy_device *phydev)
> > +static int meson_gxl_open_banks(struct phy_device *phydev)
> 
> Hi Jerome
> 
> Does the word bank come from the datasheet? Most of the phy drives use
> page instead.
> 
> Also, we have discovered a race condition which affects drivers using
> pages, which can lead to corruption of registers. At some point, i
> expect we will be adding helpers to access paged registers, which do
> the right thing with respect to locks.
> 
> So it would be nice if you used the work page, not bank.

Banks actually comes from the datasheet, Yes.
I don't mind renaming it but I would be making things up. As you wish ?

Does the usual pages comes with this weird toggle thing to open the access ?
Would we able to use these generic helpers with our this kind of quirks ?

> 
>    Thanks
> 
>       Andrew

Re: [PATCH net-next v2 7/8] net: phy: meson-gxl: add interrupt support

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:14PM +0100, Jerome Brunet wrote:
> Enable interrupt support in meson-gxl PHY driver

Hi Jerome

Is it possible to implement did_interrupt()? That allows for shared
interrupts. It does however work fine without it, so long as the
interrupt is not shared.

	  Thanks
		Andrew

[PATCH net-next v2 7/8] net: phy: meson-gxl: add interrupt support

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:14PM +0100, Jerome Brunet wrote:
> Enable interrupt support in meson-gxl PHY driver

Hi Jerome

Is it possible to implement did_interrupt()? That allows for shared
interrupts. It does however work fine without it, so long as the
interrupt is not shared.

	  Thanks
		Andrew

[PATCH net-next v2 7/8] net: phy: meson-gxl: add interrupt support

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:14PM +0100, Jerome Brunet wrote:
> Enable interrupt support in meson-gxl PHY driver

Hi Jerome

Is it possible to implement did_interrupt()? That allows for shared
interrupts. It does however work fine without it, so long as the
interrupt is not shared.

	  Thanks
		Andrew

Re: [PATCH net-next v2 8/8] net: phy: meson-gxl: join the authors

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:15PM +0100, Jerome Brunet wrote:
> Following previous changes, join the other authors of this driver and
> take the blame with them

:-)

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next v2 8/8] net: phy: meson-gxl: join the authors

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:15PM +0100, Jerome Brunet wrote:
> Following previous changes, join the other authors of this driver and
> take the blame with them

:-)

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next v2 8/8] net: phy: meson-gxl: join the authors

[Andrew Lunn]

On Thu, Dec 07, 2017 at 03:27:15PM +0100, Jerome Brunet wrote:
> Following previous changes, join the other authors of this driver and
> take the blame with them

:-)

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next v2 6/8] net: phy: meson-gxl: leave CONFIG_A6 untouched

[Jerome Brunet]

On Thu, 2017-12-07 at 16:49 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:13PM +0100, Jerome Brunet wrote:
> > The PHY performs just as well when left in its default configuration and
> > it makes senses because this poke gets reset just after init.
> 
> The only thing which might speak against this, is some bootloader
> which sets something other than the default, and here we put it back
> to the value it should have. But if you say a reset will put it back
> to the default value anyway, this seems save.

I was worried about this too bu the bank also gets reset on power down and soft
reset, so we won't get bootloader value at this point 

> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
>     Andrew

Re: [PATCH net-next v2 6/8] net: phy: meson-gxl: leave CONFIG_A6 untouched

[Jerome Brunet]

On Thu, 2017-12-07 at 16:49 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:13PM +0100, Jerome Brunet wrote:
> > The PHY performs just as well when left in its default configuration and
> > it makes senses because this poke gets reset just after init.
> 
> The only thing which might speak against this, is some bootloader
> which sets something other than the default, and here we put it back
> to the value it should have. But if you say a reset will put it back
> to the default value anyway, this seems save.

I was worried about this too bu the bank also gets reset on power down and soft
reset, so we won't get bootloader value at this point 

> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
>     Andrew

[PATCH net-next v2 6/8] net: phy: meson-gxl: leave CONFIG_A6 untouched

[Jerome Brunet]

On Thu, 2017-12-07 at 16:49 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:13PM +0100, Jerome Brunet wrote:
> > The PHY performs just as well when left in its default configuration and
> > it makes senses because this poke gets reset just after init.
> 
> The only thing which might speak against this, is some bootloader
> which sets something other than the default, and here we put it back
> to the value it should have. But if you say a reset will put it back
> to the default value anyway, this seems save.

I was worried about this too bu the bank also gets reset on power down and soft
reset, so we won't get bootloader value at this point 

> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
>     Andrew

[PATCH net-next v2 6/8] net: phy: meson-gxl: leave CONFIG_A6 untouched

[Jerome Brunet]

On Thu, 2017-12-07 at 16:49 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:13PM +0100, Jerome Brunet wrote:
> > The PHY performs just as well when left in its default configuration and
> > it makes senses because this poke gets reset just after init.
> 
> The only thing which might speak against this, is some bootloader
> which sets something other than the default, and here we put it back
> to the value it should have. But if you say a reset will put it back
> to the default value anyway, this seems save.

I was worried about this too bu the bank also gets reset on power down and soft
reset, so we won't get bootloader value at this point 

> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
>     Andrew

Re: [PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Andrew Lunn]

> Banks actually comes from the datasheet, Yes.
> I don't mind renaming it but I would be making things up. As you wish ?

Keep it as is for the moment.
 
> Does the usual pages comes with this weird toggle thing to open the access ?
> Would we able to use these generic helpers with our this kind of quirks ?

I don't think the API has been defined yet. But what has been
discussed is adding functions to struct phy_driver. The driver can
then implement whatever is needed to select a given page. There will
then be helpers which take the lock, select the page, do the
read/write, select page 0, and unlock.

Supporting this funny toggle should not be a problem.

	   Andrew

[PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Andrew Lunn]

> Banks actually comes from the datasheet, Yes.
> I don't mind renaming it but I would be making things up. As you wish ?

Keep it as is for the moment.
 
> Does the usual pages comes with this weird toggle thing to open the access ?
> Would we able to use these generic helpers with our this kind of quirks ?

I don't think the API has been defined yet. But what has been
discussed is adding functions to struct phy_driver. The driver can
then implement whatever is needed to select a given page. There will
then be helpers which take the lock, select the page, do the
read/write, select page 0, and unlock.

Supporting this funny toggle should not be a problem.

	   Andrew

[PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Andrew Lunn]

> Banks actually comes from the datasheet, Yes.
> I don't mind renaming it but I would be making things up. As you wish ?

Keep it as is for the moment.
 
> Does the usual pages comes with this weird toggle thing to open the access ?
> Would we able to use these generic helpers with our this kind of quirks ?

I don't think the API has been defined yet. But what has been
discussed is adding functions to struct phy_driver. The driver can
then implement whatever is needed to select a given page. There will
then be helpers which take the lock, select the page, do the
read/write, select page 0, and unlock.

Supporting this funny toggle should not be a problem.

	   Andrew

Re: [PATCH net-next v2 7/8] net: phy: meson-gxl: add interrupt support

[Jerome Brunet]

On Thu, 2017-12-07 at 16:54 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:14PM +0100, Jerome Brunet wrote:
> > Enable interrupt support in meson-gxl PHY driver
> 
> Hi Jerome
> 
> Is it possible to implement did_interrupt()? That allows for shared
> interrupts. It does however work fine without it, so long as the
> interrupt is not shared.

Hi Andrew,

It is always possible ;). The irq status registers gets reset on read.

In such case, ack_interrupt() and did_interrupt() would be more or less the same
function, except for the returned value, I guess ?

The phy being internal, I think it is unlikely to ever share its interrupt
though. If you prefer I implement this callback, I can certainly re-spin with it
?

Thanks for the lightning fast review by the way !

> 
> 	  Thanks
> 		Andrew

[PATCH net-next v2 7/8] net: phy: meson-gxl: add interrupt support

[Jerome Brunet]

On Thu, 2017-12-07 at 16:54 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:14PM +0100, Jerome Brunet wrote:
> > Enable interrupt support in meson-gxl PHY driver
> 
> Hi Jerome
> 
> Is it possible to implement did_interrupt()? That allows for shared
> interrupts. It does however work fine without it, so long as the
> interrupt is not shared.

Hi Andrew,

It is always possible ;). The irq status registers gets reset on read.

In such case, ack_interrupt() and did_interrupt() would be more or less the same
function, except for the returned value, I guess ?

The phy being internal, I think it is unlikely to ever share its interrupt
though. If you prefer I implement this callback, I can certainly re-spin with it
?

Thanks for the lightning fast review by the way !

> 
> 	  Thanks
> 		Andrew

[PATCH net-next v2 7/8] net: phy: meson-gxl: add interrupt support

[Jerome Brunet]

On Thu, 2017-12-07 at 16:54 +0100, Andrew Lunn wrote:
> On Thu, Dec 07, 2017 at 03:27:14PM +0100, Jerome Brunet wrote:
> > Enable interrupt support in meson-gxl PHY driver
> 
> Hi Jerome
> 
> Is it possible to implement did_interrupt()? That allows for shared
> interrupts. It does however work fine without it, so long as the
> interrupt is not shared.

Hi Andrew,

It is always possible ;). The irq status registers gets reset on read.

In such case, ack_interrupt() and did_interrupt() would be more or less the same
function, except for the returned value, I guess ?

The phy being internal, I think it is unlikely to ever share its interrupt
though. If you prefer I implement this callback, I can certainly re-spin with it
?

Thanks for the lightning fast review by the way !

> 
> 	  Thanks
> 		Andrew

Re: [PATCH net-next v2 7/8] net: phy: meson-gxl: add interrupt support

[Andrew Lunn]

> The phy being internal, I think it is unlikely to ever share its interrupt
> though.

O.K, don't bother.
 
> Thanks for the lightning fast review by the way !

You are welcome.

    Andrew

[PATCH net-next v2 7/8] net: phy: meson-gxl: add interrupt support

[Andrew Lunn]

> The phy being internal, I think it is unlikely to ever share its interrupt
> though.

O.K, don't bother.
 
> Thanks for the lightning fast review by the way !

You are welcome.

    Andrew

[PATCH net-next v2 7/8] net: phy: meson-gxl: add interrupt support

[Andrew Lunn]

> The phy being internal, I think it is unlikely to ever share its interrupt
> though.

O.K, don't bother.
 
> Thanks for the lightning fast review by the way !

You are welcome.

    Andrew

Re: [PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption

[Andrew Lunn]

> Would it be Ok if send patches 1 to 5 to net ?
> and 6 to 8 separately on net-next ?

No. The rules for stable is that a patch must really fix something and
be minimal.

Documentation/process/stable-kernel-rules.rst 

What might be best is to develop a minimal, but ugly patch for stable.
Get it applied. Around a week later, net will be merged into
net-next. You can then have a 'revert' patch, followed by this series
making it nice and clean.

       Andrew

[PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption

[Andrew Lunn]

> Would it be Ok if send patches 1 to 5 to net ?
> and 6 to 8 separately on net-next ?

No. The rules for stable is that a patch must really fix something and
be minimal.

Documentation/process/stable-kernel-rules.rst 

What might be best is to develop a minimal, but ugly patch for stable.
Get it applied. Around a week later, net will be merged into
net-next. You can then have a 'revert' patch, followed by this series
making it nice and clean.

       Andrew

[PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption

[Andrew Lunn]

> Would it be Ok if send patches 1 to 5 to net ?
> and 6 to 8 separately on net-next ?

No. The rules for stable is that a patch must really fix something and
be minimal.

Documentation/process/stable-kernel-rules.rst 

What might be best is to develop a minimal, but ugly patch for stable.
Get it applied. Around a week later, net will be merged into
net-next. You can then have a 'revert' patch, followed by this series
making it nice and clean.

       Andrew

Re: [PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption

[Jerome Brunet]

On Thu, 2017-12-07 at 17:12 +0100, Andrew Lunn wrote:
> > Would it be Ok if send patches 1 to 5 to net ?
> > and 6 to 8 separately on net-next ?
> 
> No. The rules for stable is that a patch must really fix something and
> be minimal.
> 
> Documentation/process/stable-kernel-rules.rst 
> 
> What might be best is to develop a minimal, but ugly patch for stable.
> Get it applied. Around a week later, net will be merged into
> net-next. You can then have a 'revert' patch, followed by this series
> making it nice and clean.

Looks like a plan. Will do.
Thanks Andrew.

> 
>        Andrew

[PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption

[Jerome Brunet]

On Thu, 2017-12-07 at 17:12 +0100, Andrew Lunn wrote:
> > Would it be Ok if send patches 1 to 5 to net ?
> > and 6 to 8 separately on net-next ?
> 
> No. The rules for stable is that a patch must really fix something and
> be minimal.
> 
> Documentation/process/stable-kernel-rules.rst 
> 
> What might be best is to develop a minimal, but ugly patch for stable.
> Get it applied. Around a week later, net will be merged into
> net-next. You can then have a 'revert' patch, followed by this series
> making it nice and clean.

Looks like a plan. Will do.
Thanks Andrew.

> 
>        Andrew

[PATCH net-next v2 5/8] net: phy: meson-gxl: detect LPA corruption

[Jerome Brunet]

On Thu, 2017-12-07 at 17:12 +0100, Andrew Lunn wrote:
> > Would it be Ok if send patches 1 to 5 to net ?
> > and 6 to 8 separately on net-next ?
> 
> No. The rules for stable is that a patch must really fix something and
> be minimal.
> 
> Documentation/process/stable-kernel-rules.rst 
> 
> What might be best is to develop a minimal, but ugly patch for stable.
> Get it applied. Around a week later, net will be merged into
> net-next. You can then have a 'revert' patch, followed by this series
> making it nice and clean.

Looks like a plan. Will do.
Thanks Andrew.

> 
>        Andrew

Re: [PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Russell King - ARM Linux]

On Thu, Dec 07, 2017 at 05:02:35PM +0100, Andrew Lunn wrote:
> > Banks actually comes from the datasheet, Yes.
> > I don't mind renaming it but I would be making things up. As you wish ?
> 
> Keep it as is for the moment.
>  
> > Does the usual pages comes with this weird toggle thing to open the access ?
> > Would we able to use these generic helpers with our this kind of quirks ?
> 
> I don't think the API has been defined yet. But what has been
> discussed is adding functions to struct phy_driver. The driver can
> then implement whatever is needed to select a given page. There will
> then be helpers which take the lock, select the page, do the
> read/write, select page 0, and unlock.

I'm not sure adding generic helpers really works, because the indirection
through phy_driver just makes the code more complex.  For Marvell, I
ended up with:

http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=9ca46084228bb1b8851e7d24276d85c4ec6e13ae

and the preceding three commits.

The "generic" versions I came up with were basically lifting
marvell_read_paged(), marvell_write_paged() and marvell_modify_paged()
to phylib, along with the lower leve marvell_save_page(),
marvell_select_page() and marvell_restore_page().  The result is a
fair amount of out-of-line code and an assumption that it's a single
register.  As soon as a PHY has other requirements, these generic
implementations don't work.

Also, unfortunately, we can't get help from sparse for statically
checking the locking - the __acquire()/__release() annotations don't
work for mutexes.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Russell King - ARM Linux]

On Thu, Dec 07, 2017 at 05:02:35PM +0100, Andrew Lunn wrote:
> > Banks actually comes from the datasheet, Yes.
> > I don't mind renaming it but I would be making things up. As you wish ?
> 
> Keep it as is for the moment.
>  
> > Does the usual pages comes with this weird toggle thing to open the access ?
> > Would we able to use these generic helpers with our this kind of quirks ?
> 
> I don't think the API has been defined yet. But what has been
> discussed is adding functions to struct phy_driver. The driver can
> then implement whatever is needed to select a given page. There will
> then be helpers which take the lock, select the page, do the
> read/write, select page 0, and unlock.

I'm not sure adding generic helpers really works, because the indirection
through phy_driver just makes the code more complex.  For Marvell, I
ended up with:

http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=9ca46084228bb1b8851e7d24276d85c4ec6e13ae

and the preceding three commits.

The "generic" versions I came up with were basically lifting
marvell_read_paged(), marvell_write_paged() and marvell_modify_paged()
to phylib, along with the lower leve marvell_save_page(),
marvell_select_page() and marvell_restore_page().  The result is a
fair amount of out-of-line code and an assumption that it's a single
register.  As soon as a PHY has other requirements, these generic
implementations don't work.

Also, unfortunately, we can't get help from sparse for statically
checking the locking - the __acquire()/__release() annotations don't
work for mutexes.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH net-next v2 3/8] net: phy: meson-gxl: add read and write helpers for bank registers

[Russell King - ARM Linux]

On Thu, Dec 07, 2017 at 05:02:35PM +0100, Andrew Lunn wrote:
> > Banks actually comes from the datasheet, Yes.
> > I don't mind renaming it but I would be making things up. As you wish ?
> 
> Keep it as is for the moment.
>  
> > Does the usual pages comes with this weird toggle thing to open the access ?
> > Would we able to use these generic helpers with our this kind of quirks ?
> 
> I don't think the API has been defined yet. But what has been
> discussed is adding functions to struct phy_driver. The driver can
> then implement whatever is needed to select a given page. There will
> then be helpers which take the lock, select the page, do the
> read/write, select page 0, and unlock.

I'm not sure adding generic helpers really works, because the indirection
through phy_driver just makes the code more complex.  For Marvell, I
ended up with:

http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=phy&id=9ca46084228bb1b8851e7d24276d85c4ec6e13ae

and the preceding three commits.

The "generic" versions I came up with were basically lifting
marvell_read_paged(), marvell_write_paged() and marvell_modify_paged()
to phylib, along with the lower leve marvell_save_page(),
marvell_select_page() and marvell_restore_page().  The result is a
fair amount of out-of-line code and an assumption that it's a single
register.  As soon as a PHY has other requirements, these generic
implementations don't work.

Also, unfortunately, we can't get help from sparse for statically
checking the locking - the __acquire()/__release() annotations don't
work for mutexes.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up