[PATCH net-next v2 0/4] net: phy: micrel: Add support for lan8842

[PATCH net-next v2 0/4] net: phy: micrel: Add support for lan8842

[Horatiu Vultur]

Add support for LAN8842 which supports industry-standard SGMII.
While add this the first 3 patches in the series cleans more the
driver, they should not introduce any functional changes.

v1->v2:
- add the first 3 patches to clean the driver
- drop fast link failure support
- implement reading the statistics in the new way

Horatiu Vultur (4):
  net: phy: micrel: Start using PHY_ID_MATCH_MODEL
  net: phy: micrel: Introduce lanphy_modify_page_reg
  net: phy: micrel: Replace hardcoded pages with defines
  net: phy: micrel: Add support for lan8842

 drivers/net/phy/micrel.c   | 727 +++++++++++++++++++++++++------------
 include/linux/micrel_phy.h |   1 +
 2 files changed, 497 insertions(+), 231 deletions(-)

-- 
2.34.1

[PATCH net-next v2 1/4] net: phy: micrel: Start using PHY_ID_MATCH_MODEL

[Horatiu Vultur]

Start using PHY_ID_MATCH_MODEL for all the drivers.
While at this add also PHY_ID_KSZ8041RNLI to micrel_tbl.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 drivers/net/phy/micrel.c | 66 ++++++++++++++++------------------------
 1 file changed, 27 insertions(+), 39 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index f678c1bdacdf0..c6aacf7feb7b0 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -5643,8 +5643,7 @@ static int ksz9131_resume(struct phy_device *phydev)
 
 static struct phy_driver ksphy_driver[] = {
 {
-	.phy_id		= PHY_ID_KS8737,
-	.phy_id_mask	= MICREL_PHY_ID_MASK,
+	PHY_ID_MATCH_MODEL(PHY_ID_KS8737),
 	.name		= "Micrel KS8737",
 	/* PHY_BASIC_FEATURES */
 	.driver_data	= &ks8737_type,
@@ -5685,8 +5684,7 @@ static struct phy_driver ksphy_driver[] = {
 	.suspend	= kszphy_suspend,
 	.resume		= kszphy_resume,
 }, {
-	.phy_id		= PHY_ID_KSZ8041,
-	.phy_id_mask	= MICREL_PHY_ID_MASK,
+	PHY_ID_MATCH_MODEL(PHY_ID_KSZ8041),
 	.name		= "Micrel KSZ8041",
 	/* PHY_BASIC_FEATURES */
 	.driver_data	= &ksz8041_type,
@@ -5701,8 +5699,7 @@ static struct phy_driver ksphy_driver[] = {
 	.suspend	= ksz8041_suspend,
 	.resume		= ksz8041_resume,
 }, {
-	.phy_id		= PHY_ID_KSZ8041RNLI,
-	.phy_id_mask	= MICREL_PHY_ID_MASK,
+	PHY_ID_MATCH_MODEL(PHY_ID_KSZ8041RNLI),
 	.name		= "Micrel KSZ8041RNLI",
 	/* PHY_BASIC_FEATURES */
 	.driver_data	= &ksz8041_type,
@@ -5745,9 +5742,8 @@ static struct phy_driver ksphy_driver[] = {
 	.suspend	= kszphy_suspend,
 	.resume		= kszphy_resume,
 }, {
-	.phy_id		= PHY_ID_KSZ8081,
+	PHY_ID_MATCH_MODEL(PHY_ID_KSZ8081),
 	.name		= "Micrel KSZ8081 or KSZ8091",
-	.phy_id_mask	= MICREL_PHY_ID_MASK,
 	.flags		= PHY_POLL_CABLE_TEST,
 	/* PHY_BASIC_FEATURES */
 	.driver_data	= &ksz8081_type,
@@ -5766,9 +5762,8 @@ static struct phy_driver ksphy_driver[] = {
 	.cable_test_start	= ksz886x_cable_test_start,
 	.cable_test_get_status	= ksz886x_cable_test_get_status,
 }, {
-	.phy_id		= PHY_ID_KSZ8061,
+	PHY_ID_MATCH_MODEL(PHY_ID_KSZ8061),
 	.name		= "Micrel KSZ8061",
-	.phy_id_mask	= MICREL_PHY_ID_MASK,
 	/* PHY_BASIC_FEATURES */
 	.probe		= kszphy_probe,
 	.config_init	= ksz8061_config_init,
@@ -5796,8 +5791,7 @@ static struct phy_driver ksphy_driver[] = {
 	.read_mmd	= genphy_read_mmd_unsupported,
 	.write_mmd	= genphy_write_mmd_unsupported,
 }, {
-	.phy_id		= PHY_ID_KSZ9031,
-	.phy_id_mask	= MICREL_PHY_ID_MASK,
+	PHY_ID_MATCH_MODEL(PHY_ID_KSZ9031),
 	.name		= "Micrel KSZ9031 Gigabit PHY",
 	.flags		= PHY_POLL_CABLE_TEST,
 	.driver_data	= &ksz9021_type,
@@ -5817,8 +5811,7 @@ static struct phy_driver ksphy_driver[] = {
 	.cable_test_get_status	= ksz9x31_cable_test_get_status,
 	.set_loopback	= ksz9031_set_loopback,
 }, {
-	.phy_id		= PHY_ID_LAN8814,
-	.phy_id_mask	= MICREL_PHY_ID_MASK,
+	PHY_ID_MATCH_MODEL(PHY_ID_LAN8814),
 	.name		= "Microchip INDY Gigabit Quad PHY",
 	.flags          = PHY_POLL_CABLE_TEST,
 	.config_init	= lan8814_config_init,
@@ -5836,8 +5829,7 @@ static struct phy_driver ksphy_driver[] = {
 	.cable_test_start	= lan8814_cable_test_start,
 	.cable_test_get_status	= ksz886x_cable_test_get_status,
 }, {
-	.phy_id		= PHY_ID_LAN8804,
-	.phy_id_mask	= MICREL_PHY_ID_MASK,
+	PHY_ID_MATCH_MODEL(PHY_ID_LAN8804),
 	.name		= "Microchip LAN966X Gigabit PHY",
 	.config_init	= lan8804_config_init,
 	.driver_data	= &ksz9021_type,
@@ -5852,8 +5844,7 @@ static struct phy_driver ksphy_driver[] = {
 	.config_intr	= lan8804_config_intr,
 	.handle_interrupt = lan8804_handle_interrupt,
 }, {
-	.phy_id		= PHY_ID_LAN8841,
-	.phy_id_mask	= MICREL_PHY_ID_MASK,
+	PHY_ID_MATCH_MODEL(PHY_ID_LAN8841),
 	.name		= "Microchip LAN8841 Gigabit PHY",
 	.flags		= PHY_POLL_CABLE_TEST,
 	.driver_data	= &lan8841_type,
@@ -5870,8 +5861,7 @@ static struct phy_driver ksphy_driver[] = {
 	.cable_test_start	= lan8814_cable_test_start,
 	.cable_test_get_status	= ksz886x_cable_test_get_status,
 }, {
-	.phy_id		= PHY_ID_KSZ9131,
-	.phy_id_mask	= MICREL_PHY_ID_MASK,
+	PHY_ID_MATCH_MODEL(PHY_ID_KSZ9131),
 	.name		= "Microchip KSZ9131 Gigabit PHY",
 	/* PHY_GBIT_FEATURES */
 	.flags		= PHY_POLL_CABLE_TEST,
@@ -5892,8 +5882,7 @@ static struct phy_driver ksphy_driver[] = {
 	.cable_test_get_status	= ksz9x31_cable_test_get_status,
 	.get_features	= ksz9477_get_features,
 }, {
-	.phy_id		= PHY_ID_KSZ8873MLL,
-	.phy_id_mask	= MICREL_PHY_ID_MASK,
+	PHY_ID_MATCH_MODEL(PHY_ID_KSZ8873MLL),
 	.name		= "Micrel KSZ8873MLL Switch",
 	/* PHY_BASIC_FEATURES */
 	.config_init	= kszphy_config_init,
@@ -5902,8 +5891,7 @@ static struct phy_driver ksphy_driver[] = {
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 }, {
-	.phy_id		= PHY_ID_KSZ886X,
-	.phy_id_mask	= MICREL_PHY_ID_MASK,
+	PHY_ID_MATCH_MODEL(PHY_ID_KSZ886X),
 	.name		= "Micrel KSZ8851 Ethernet MAC or KSZ886X Switch",
 	.driver_data	= &ksz886x_type,
 	/* PHY_BASIC_FEATURES */
@@ -5923,8 +5911,7 @@ static struct phy_driver ksphy_driver[] = {
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 }, {
-	.phy_id		= PHY_ID_KSZ9477,
-	.phy_id_mask	= MICREL_PHY_ID_MASK,
+	PHY_ID_MATCH_MODEL(PHY_ID_KSZ9477),
 	.name		= "Microchip KSZ9477",
 	.probe		= kszphy_probe,
 	/* PHY_GBIT_FEATURES */
@@ -5951,22 +5938,23 @@ MODULE_LICENSE("GPL");
 
 static const struct mdio_device_id __maybe_unused micrel_tbl[] = {
 	{ PHY_ID_KSZ9021, 0x000ffffe },
-	{ PHY_ID_KSZ9031, MICREL_PHY_ID_MASK },
-	{ PHY_ID_KSZ9131, MICREL_PHY_ID_MASK },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_KSZ9031) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_KSZ9131) },
 	{ PHY_ID_KSZ8001, 0x00fffffc },
-	{ PHY_ID_KS8737, MICREL_PHY_ID_MASK },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_KS8737) },
 	{ PHY_ID_KSZ8021, 0x00ffffff },
 	{ PHY_ID_KSZ8031, 0x00ffffff },
-	{ PHY_ID_KSZ8041, MICREL_PHY_ID_MASK },
-	{ PHY_ID_KSZ8051, MICREL_PHY_ID_MASK },
-	{ PHY_ID_KSZ8061, MICREL_PHY_ID_MASK },
-	{ PHY_ID_KSZ8081, MICREL_PHY_ID_MASK },
-	{ PHY_ID_KSZ8873MLL, MICREL_PHY_ID_MASK },
-	{ PHY_ID_KSZ886X, MICREL_PHY_ID_MASK },
-	{ PHY_ID_KSZ9477, MICREL_PHY_ID_MASK },
-	{ PHY_ID_LAN8814, MICREL_PHY_ID_MASK },
-	{ PHY_ID_LAN8804, MICREL_PHY_ID_MASK },
-	{ PHY_ID_LAN8841, MICREL_PHY_ID_MASK },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_KSZ8041) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_KSZ8041RNLI) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_KSZ8051) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_KSZ8061) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_KSZ8081) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_KSZ8873MLL) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_KSZ886X) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_KSZ9477) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_LAN8814) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_LAN8804) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_LAN8841) },
 	{ }
 };
 
-- 
2.34.1

[PATCH net-next v2 2/4] net: phy: micrel: Introduce lanphy_modify_page_reg

[Horatiu Vultur]

As the name suggests this function modifies the register in an
extended page. It has the same parameters as phy_modify_mmd.
This function was introduce because there are many places in the
code where the registers was read then the value was modified and
written back. So replace all this code with this function to make
it clear.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 drivers/net/phy/micrel.c | 228 +++++++++++++++++++--------------------
 1 file changed, 113 insertions(+), 115 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index c6aacf7feb7b0..b04c471c11a4a 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -2838,6 +2838,24 @@ static int lanphy_write_page_reg(struct phy_device *phydev, int page, u16 addr,
 	return val;
 }
 
+static int lanphy_modify_page_reg(struct phy_device *phydev, int page, u16 addr,
+				  u16 mask, u16 set)
+{
+	int new, ret;
+
+	ret = lanphy_read_page_reg(phydev, page, addr);
+	if (ret < 0)
+		return ret;
+
+	new = (ret & ~mask) | set;
+	if (new == ret)
+		return 0;
+
+	ret = lanphy_write_page_reg(phydev, page, addr, new);
+
+	return ret < 0 ? ret : 1;
+}
+
 static int lan8814_config_ts_intr(struct phy_device *phydev, bool enable)
 {
 	u16 val = 0;
@@ -2926,7 +2944,6 @@ static int lan8814_hwtstamp(struct mii_timestamper *mii_ts,
 	struct lan8814_ptp_rx_ts *rx_ts, *tmp;
 	int txcfg = 0, rxcfg = 0;
 	int pkt_ts_enable;
-	int tx_mod;
 
 	ptp_priv->hwts_tx_type = config->tx_type;
 	ptp_priv->rx_filter = config->rx_filter;
@@ -2973,13 +2990,14 @@ static int lan8814_hwtstamp(struct mii_timestamper *mii_ts,
 	lanphy_write_page_reg(ptp_priv->phydev, 5, PTP_RX_TIMESTAMP_EN, pkt_ts_enable);
 	lanphy_write_page_reg(ptp_priv->phydev, 5, PTP_TX_TIMESTAMP_EN, pkt_ts_enable);
 
-	tx_mod = lanphy_read_page_reg(ptp_priv->phydev, 5, PTP_TX_MOD);
 	if (ptp_priv->hwts_tx_type == HWTSTAMP_TX_ONESTEP_SYNC) {
-		lanphy_write_page_reg(ptp_priv->phydev, 5, PTP_TX_MOD,
-				      tx_mod | PTP_TX_MOD_TX_PTP_SYNC_TS_INSERT_);
+		lanphy_modify_page_reg(ptp_priv->phydev, 5, PTP_TX_MOD,
+				       PTP_TX_MOD_TX_PTP_SYNC_TS_INSERT_,
+				       PTP_TX_MOD_TX_PTP_SYNC_TS_INSERT_);
 	} else if (ptp_priv->hwts_tx_type == HWTSTAMP_TX_ON) {
-		lanphy_write_page_reg(ptp_priv->phydev, 5, PTP_TX_MOD,
-				      tx_mod & ~PTP_TX_MOD_TX_PTP_SYNC_TS_INSERT_);
+		lanphy_modify_page_reg(ptp_priv->phydev, 5, PTP_TX_MOD,
+				       PTP_TX_MOD_TX_PTP_SYNC_TS_INSERT_,
+				       0);
 	}
 
 	if (config->rx_filter != HWTSTAMP_FILTER_NONE)
@@ -3382,73 +3400,66 @@ static void lan8814_ptp_set_reload(struct phy_device *phydev, int event,
 static void lan8814_ptp_enable_event(struct phy_device *phydev, int event,
 				     int pulse_width)
 {
-	u16 val;
-
-	val = lanphy_read_page_reg(phydev, 4, LAN8814_PTP_GENERAL_CONFIG);
-	/* Set the pulse width of the event */
-	val &= ~(LAN8814_PTP_GENERAL_CONFIG_LTC_EVENT_MASK(event));
-	/* Make sure that the target clock will be incremented each time when
+	/* Set the pulse width of the event,
+	 * Make sure that the target clock will be incremented each time when
 	 * local time reaches or pass it
+	 * Set the polarity high
 	 */
-	val |= LAN8814_PTP_GENERAL_CONFIG_LTC_EVENT_SET(event, pulse_width);
-	val &= ~(LAN8814_PTP_GENERAL_CONFIG_RELOAD_ADD_X(event));
-	/* Set the polarity high */
-	val |= LAN8814_PTP_GENERAL_CONFIG_POLARITY_X(event);
-	lanphy_write_page_reg(phydev, 4, LAN8814_PTP_GENERAL_CONFIG, val);
+	lanphy_modify_page_reg(phydev, 4, LAN8814_PTP_GENERAL_CONFIG,
+			       LAN8814_PTP_GENERAL_CONFIG_LTC_EVENT_MASK(event) |
+			       LAN8814_PTP_GENERAL_CONFIG_LTC_EVENT_SET(event, pulse_width) |
+			       LAN8814_PTP_GENERAL_CONFIG_RELOAD_ADD_X(event) |
+			       LAN8814_PTP_GENERAL_CONFIG_POLARITY_X(event),
+			       LAN8814_PTP_GENERAL_CONFIG_LTC_EVENT_SET(event, pulse_width) |
+			       LAN8814_PTP_GENERAL_CONFIG_POLARITY_X(event));
 }
 
 static void lan8814_ptp_disable_event(struct phy_device *phydev, int event)
 {
-	u16 val;
-
 	/* Set target to too far in the future, effectively disabling it */
 	lan8814_ptp_set_target(phydev, event, 0xFFFFFFFF, 0);
 
 	/* And then reload once it recheas the target */
-	val = lanphy_read_page_reg(phydev, 4, LAN8814_PTP_GENERAL_CONFIG);
-	val |= LAN8814_PTP_GENERAL_CONFIG_RELOAD_ADD_X(event);
-	lanphy_write_page_reg(phydev, 4, LAN8814_PTP_GENERAL_CONFIG, val);
+	lanphy_modify_page_reg(phydev, 4, LAN8814_PTP_GENERAL_CONFIG,
+			       LAN8814_PTP_GENERAL_CONFIG_RELOAD_ADD_X(event),
+			       LAN8814_PTP_GENERAL_CONFIG_RELOAD_ADD_X(event));
 }
 
 static void lan8814_ptp_perout_off(struct phy_device *phydev, int pin)
 {
-	u16 val;
-
 	/* Disable gpio alternate function,
 	 * 1: select as gpio,
 	 * 0: select alt func
 	 */
-	val = lanphy_read_page_reg(phydev, 4, LAN8814_GPIO_EN_ADDR(pin));
-	val |= LAN8814_GPIO_EN_BIT(pin);
-	lanphy_write_page_reg(phydev, 4, LAN8814_GPIO_EN_ADDR(pin), val);
+	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_EN_ADDR(pin),
+			       LAN8814_GPIO_EN_BIT(pin),
+			       LAN8814_GPIO_EN_BIT(pin));
 
-	val = lanphy_read_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin));
-	val &= ~LAN8814_GPIO_DIR_BIT(pin);
-	lanphy_write_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin), val);
+	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin),
+			       LAN8814_GPIO_DIR_BIT(pin),
+			       0);
 
-	val = lanphy_read_page_reg(phydev, 4, LAN8814_GPIO_BUF_ADDR(pin));
-	val &= ~LAN8814_GPIO_BUF_BIT(pin);
-	lanphy_write_page_reg(phydev, 4, LAN8814_GPIO_BUF_ADDR(pin), val);
+	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_BUF_ADDR(pin),
+			       LAN8814_GPIO_BUF_BIT(pin),
+			       0);
 }
 
 static void lan8814_ptp_perout_on(struct phy_device *phydev, int pin)
 {
-	int val;
-
 	/* Set as gpio output */
-	val = lanphy_read_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin));
-	val |= LAN8814_GPIO_DIR_BIT(pin);
-	lanphy_write_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin), val);
+	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin),
+			       LAN8814_GPIO_DIR_BIT(pin),
+			       LAN8814_GPIO_DIR_BIT(pin));
 
 	/* Enable gpio 0:for alternate function, 1:gpio */
-	val = lanphy_read_page_reg(phydev, 4, LAN8814_GPIO_EN_ADDR(pin));
-	val &= ~LAN8814_GPIO_EN_BIT(pin);
-	lanphy_write_page_reg(phydev, 4, LAN8814_GPIO_EN_ADDR(pin), val);
+	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_EN_ADDR(pin),
+			       LAN8814_GPIO_EN_BIT(pin),
+			       0);
 
 	/* Set buffer type to push pull */
-	val = lanphy_read_page_reg(phydev, 4, LAN8814_GPIO_BUF_ADDR(pin));
-	val |= LAN8814_GPIO_BUF_BIT(pin);
-	lanphy_write_page_reg(phydev, 4, LAN8814_GPIO_BUF_ADDR(pin), val);
+	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_BUF_ADDR(pin),
+			       LAN8814_GPIO_BUF_BIT(pin),
+			       LAN8814_GPIO_BUF_BIT(pin));
 }
 
 static int lan8814_ptp_perout(struct ptp_clock_info *ptpci,
@@ -3563,61 +3574,59 @@ static int lan8814_ptp_perout(struct ptp_clock_info *ptpci,
 
 static void lan8814_ptp_extts_on(struct phy_device *phydev, int pin, u32 flags)
 {
-	u16 tmp;
-
 	/* Set as gpio input */
-	tmp = lanphy_read_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin));
-	tmp &= ~LAN8814_GPIO_DIR_BIT(pin);
-	lanphy_write_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin), tmp);
+	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin),
+			       LAN8814_GPIO_DIR_BIT(pin),
+			       0);
 
 	/* Map the pin to ltc pin 0 of the capture map registers */
-	tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_CAP_MAP_LO);
-	tmp |= pin;
-	lanphy_write_page_reg(phydev, 4, PTP_GPIO_CAP_MAP_LO, tmp);
+	lanphy_modify_page_reg(phydev, 4, PTP_GPIO_CAP_MAP_LO,
+			       pin,
+			       pin);
 
 	/* Enable capture on the edges of the ltc pin */
-	tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_CAP_EN);
 	if (flags & PTP_RISING_EDGE)
-		tmp |= PTP_GPIO_CAP_EN_GPIO_RE_CAPTURE_ENABLE(0);
+		lanphy_modify_page_reg(phydev, 4, PTP_GPIO_CAP_EN,
+				       PTP_GPIO_CAP_EN_GPIO_RE_CAPTURE_ENABLE(0),
+				       PTP_GPIO_CAP_EN_GPIO_RE_CAPTURE_ENABLE(0));
 	if (flags & PTP_FALLING_EDGE)
-		tmp |= PTP_GPIO_CAP_EN_GPIO_FE_CAPTURE_ENABLE(0);
-	lanphy_write_page_reg(phydev, 4, PTP_GPIO_CAP_EN, tmp);
+		lanphy_modify_page_reg(phydev, 4, PTP_GPIO_CAP_EN,
+				       PTP_GPIO_CAP_EN_GPIO_FE_CAPTURE_ENABLE(0),
+				       PTP_GPIO_CAP_EN_GPIO_FE_CAPTURE_ENABLE(0));
 
 	/* Enable interrupt top interrupt */
-	tmp = lanphy_read_page_reg(phydev, 4, PTP_COMMON_INT_ENA);
-	tmp |= PTP_COMMON_INT_ENA_GPIO_CAP_EN;
-	lanphy_write_page_reg(phydev, 4, PTP_COMMON_INT_ENA, tmp);
+	lanphy_modify_page_reg(phydev, 4, PTP_COMMON_INT_ENA,
+			       PTP_COMMON_INT_ENA_GPIO_CAP_EN,
+			       PTP_COMMON_INT_ENA_GPIO_CAP_EN);
 }
 
 static void lan8814_ptp_extts_off(struct phy_device *phydev, int pin)
 {
-	u16 tmp;
-
 	/* Set as gpio out */
-	tmp = lanphy_read_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin));
-	tmp |= LAN8814_GPIO_DIR_BIT(pin);
-	lanphy_write_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin), tmp);
+	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin),
+			       LAN8814_GPIO_DIR_BIT(pin),
+			       LAN8814_GPIO_DIR_BIT(pin));
 
 	/* Enable alternate, 0:for alternate function, 1:gpio */
-	tmp = lanphy_read_page_reg(phydev, 4, LAN8814_GPIO_EN_ADDR(pin));
-	tmp &= ~LAN8814_GPIO_EN_BIT(pin);
-	lanphy_write_page_reg(phydev, 4, LAN8814_GPIO_EN_ADDR(pin), tmp);
+	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_EN_ADDR(pin),
+			       LAN8814_GPIO_EN_BIT(pin),
+			       0);
 
 	/* Clear the mapping of pin to registers 0 of the capture registers */
-	tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_CAP_MAP_LO);
-	tmp &= ~GENMASK(3, 0);
-	lanphy_write_page_reg(phydev, 4, PTP_GPIO_CAP_MAP_LO, tmp);
+	lanphy_modify_page_reg(phydev, 4, PTP_GPIO_CAP_MAP_LO,
+			       GENMASK(3, 0),
+			       0);
 
 	/* Disable capture on both of the edges */
-	tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_CAP_EN);
-	tmp &= ~PTP_GPIO_CAP_EN_GPIO_RE_CAPTURE_ENABLE(pin);
-	tmp &= ~PTP_GPIO_CAP_EN_GPIO_FE_CAPTURE_ENABLE(pin);
-	lanphy_write_page_reg(phydev, 4, PTP_GPIO_CAP_EN, tmp);
+	lanphy_modify_page_reg(phydev, 4, PTP_GPIO_CAP_EN,
+			       PTP_GPIO_CAP_EN_GPIO_RE_CAPTURE_ENABLE(pin) |
+			       PTP_GPIO_CAP_EN_GPIO_FE_CAPTURE_ENABLE(pin),
+			       0);
 
 	/* Disable interrupt top interrupt */
-	tmp = lanphy_read_page_reg(phydev, 4, PTP_COMMON_INT_ENA);
-	tmp &= ~PTP_COMMON_INT_ENA_GPIO_CAP_EN;
-	lanphy_write_page_reg(phydev, 4, PTP_COMMON_INT_ENA, tmp);
+	lanphy_modify_page_reg(phydev, 4, PTP_COMMON_INT_ENA,
+			       PTP_COMMON_INT_ENA_GPIO_CAP_EN,
+			       0);
 }
 
 static int lan8814_ptp_extts(struct ptp_clock_info *ptpci,
@@ -3857,9 +3866,9 @@ static int lan8814_gpio_process_cap(struct lan8814_shared_priv *shared)
 	/* This is 0 because whatever was the input pin it was mapped it to
 	 * ltc gpio pin 0
 	 */
-	tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_SEL);
-	tmp |= PTP_GPIO_SEL_GPIO_SEL(0);
-	lanphy_write_page_reg(phydev, 4, PTP_GPIO_SEL, tmp);
+	lanphy_modify_page_reg(phydev, 4, PTP_GPIO_SEL,
+			       PTP_GPIO_SEL_GPIO_SEL(0),
+			       PTP_GPIO_SEL_GPIO_SEL(0));
 
 	tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_CAP_STS);
 	if (!(tmp & PTP_GPIO_CAP_STS_PTP_GPIO_RE_STS(0)) &&
@@ -3906,13 +3915,10 @@ static int lan8814_handle_gpio_interrupt(struct phy_device *phydev, u16 status)
 
 static int lan8804_config_init(struct phy_device *phydev)
 {
-	int val;
-
 	/* MDI-X setting for swap A,B transmit */
-	val = lanphy_read_page_reg(phydev, 2, LAN8804_ALIGN_SWAP);
-	val &= ~LAN8804_ALIGN_TX_A_B_SWAP_MASK;
-	val |= LAN8804_ALIGN_TX_A_B_SWAP;
-	lanphy_write_page_reg(phydev, 2, LAN8804_ALIGN_SWAP, val);
+	lanphy_modify_page_reg(phydev, 2, LAN8804_ALIGN_SWAP,
+			       LAN8804_ALIGN_TX_A_B_SWAP_MASK,
+			       LAN8804_ALIGN_TX_A_B_SWAP);
 
 	/* Make sure that the PHY will not stop generating the clock when the
 	 * link partner goes down
@@ -4054,7 +4060,6 @@ static void lan8814_ptp_init(struct phy_device *phydev)
 {
 	struct kszphy_priv *priv = phydev->priv;
 	struct kszphy_ptp_priv *ptp_priv = &priv->ptp_priv;
-	u32 temp;
 
 	if (!IS_ENABLED(CONFIG_PTP_1588_CLOCK) ||
 	    !IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING))
@@ -4062,13 +4067,13 @@ static void lan8814_ptp_init(struct phy_device *phydev)
 
 	lanphy_write_page_reg(phydev, 5, TSU_HARD_RESET, TSU_HARD_RESET_);
 
-	temp = lanphy_read_page_reg(phydev, 5, PTP_TX_MOD);
-	temp |= PTP_TX_MOD_BAD_UDPV4_CHKSUM_FORCE_FCS_DIS_;
-	lanphy_write_page_reg(phydev, 5, PTP_TX_MOD, temp);
+	lanphy_modify_page_reg(phydev, 5, PTP_TX_MOD,
+			       PTP_TX_MOD_BAD_UDPV4_CHKSUM_FORCE_FCS_DIS_,
+			       PTP_TX_MOD_BAD_UDPV4_CHKSUM_FORCE_FCS_DIS_);
 
-	temp = lanphy_read_page_reg(phydev, 5, PTP_RX_MOD);
-	temp |= PTP_RX_MOD_BAD_UDPV4_CHKSUM_FORCE_FCS_DIS_;
-	lanphy_write_page_reg(phydev, 5, PTP_RX_MOD, temp);
+	lanphy_modify_page_reg(phydev, 5, PTP_RX_MOD,
+			       PTP_RX_MOD_BAD_UDPV4_CHKSUM_FORCE_FCS_DIS_,
+			       PTP_RX_MOD_BAD_UDPV4_CHKSUM_FORCE_FCS_DIS_);
 
 	lanphy_write_page_reg(phydev, 5, PTP_RX_PARSE_CONFIG, 0);
 	lanphy_write_page_reg(phydev, 5, PTP_TX_PARSE_CONFIG, 0);
@@ -4194,23 +4199,21 @@ static void lan8814_setup_led(struct phy_device *phydev, int val)
 static int lan8814_config_init(struct phy_device *phydev)
 {
 	struct kszphy_priv *lan8814 = phydev->priv;
-	int val;
 
 	/* Reset the PHY */
-	val = lanphy_read_page_reg(phydev, 4, LAN8814_QSGMII_SOFT_RESET);
-	val |= LAN8814_QSGMII_SOFT_RESET_BIT;
-	lanphy_write_page_reg(phydev, 4, LAN8814_QSGMII_SOFT_RESET, val);
+	lanphy_modify_page_reg(phydev, 4, LAN8814_QSGMII_SOFT_RESET,
+			       LAN8814_QSGMII_SOFT_RESET_BIT,
+			       LAN8814_QSGMII_SOFT_RESET_BIT);
 
 	/* Disable ANEG with QSGMII PCS Host side */
-	val = lanphy_read_page_reg(phydev, 5, LAN8814_QSGMII_PCS1G_ANEG_CONFIG);
-	val &= ~LAN8814_QSGMII_PCS1G_ANEG_CONFIG_ANEG_ENA;
-	lanphy_write_page_reg(phydev, 5, LAN8814_QSGMII_PCS1G_ANEG_CONFIG, val);
+	lanphy_modify_page_reg(phydev, 4, LAN8814_QSGMII_PCS1G_ANEG_CONFIG,
+			       LAN8814_QSGMII_PCS1G_ANEG_CONFIG_ANEG_ENA,
+			       0);
 
 	/* MDI-X setting for swap A,B transmit */
-	val = lanphy_read_page_reg(phydev, 2, LAN8814_ALIGN_SWAP);
-	val &= ~LAN8814_ALIGN_TX_A_B_SWAP_MASK;
-	val |= LAN8814_ALIGN_TX_A_B_SWAP;
-	lanphy_write_page_reg(phydev, 2, LAN8814_ALIGN_SWAP, val);
+	lanphy_modify_page_reg(phydev, 2, LAN8814_ALIGN_SWAP,
+			       LAN8814_ALIGN_TX_A_B_SWAP_MASK,
+			       LAN8814_ALIGN_TX_A_B_SWAP);
 
 	if (lan8814->led_mode >= 0)
 		lan8814_setup_led(phydev, lan8814->led_mode);
@@ -4241,29 +4244,24 @@ static int lan8814_release_coma_mode(struct phy_device *phydev)
 
 static void lan8814_clear_2psp_bit(struct phy_device *phydev)
 {
-	u16 val;
-
 	/* It was noticed that when traffic is passing through the PHY and the
 	 * cable is removed then the LED was still one even though there is no
 	 * link
 	 */
-	val = lanphy_read_page_reg(phydev, 2, LAN8814_EEE_STATE);
-	val &= ~LAN8814_EEE_STATE_MASK2P5P;
-	lanphy_write_page_reg(phydev, 2, LAN8814_EEE_STATE, val);
+	lanphy_modify_page_reg(phydev, 2, LAN8814_EEE_STATE,
+			       LAN8814_EEE_STATE_MASK2P5P,
+			       0);
 }
 
 static void lan8814_update_meas_time(struct phy_device *phydev)
 {
-	u16 val;
-
 	/* By setting the measure time to a value of 0xb this will allow cables
 	 * longer than 100m to be used. This configuration can be used
 	 * regardless of the mode of operation of the PHY
 	 */
-	val = lanphy_read_page_reg(phydev, 1, LAN8814_PD_CONTROLS);
-	val &= ~LAN8814_PD_CONTROLS_PD_MEAS_TIME_MASK;
-	val |= LAN8814_PD_CONTROLS_PD_MEAS_TIME_VAL;
-	lanphy_write_page_reg(phydev, 1, LAN8814_PD_CONTROLS, val);
+	lanphy_modify_page_reg(phydev, 1, LAN8814_PD_CONTROLS,
+			       LAN8814_PD_CONTROLS_PD_MEAS_TIME_MASK,
+			       LAN8814_PD_CONTROLS_PD_MEAS_TIME_VAL);
 }
 
 static int lan8814_probe(struct phy_device *phydev)
-- 
2.34.1

[PATCH net-next v2 3/4] net: phy: micrel: Replace hardcoded pages with defines

[Horatiu Vultur]

The functions lan_*_page_reg gets as a second parameter the page
where the register is. In all the functions the page was hardcoded.
Replace the hardcoded values with defines to make it more clear
what are those parameters.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 drivers/net/phy/micrel.c | 291 +++++++++++++++++++++++++--------------
 1 file changed, 185 insertions(+), 106 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index b04c471c11a4a..d20f028106b7d 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -2788,6 +2788,13 @@ static int ksz886x_cable_test_get_status(struct phy_device *phydev,
 	return ret;
 }
 
+#define LAN_EXT_PAGE_0					0
+#define LAN_EXT_PAGE_1					1
+#define LAN_EXT_PAGE_2					2
+#define LAN_EXT_PAGE_4					4
+#define LAN_EXT_PAGE_5					5
+#define LAN_EXT_PAGE_31					31
+
 #define LAN_EXT_PAGE_ACCESS_CONTROL			0x16
 #define LAN_EXT_PAGE_ACCESS_ADDRESS_DATA		0x17
 #define LAN_EXT_PAGE_ACCESS_CTRL_EP_FUNC		0x4000
@@ -2866,35 +2873,46 @@ static int lan8814_config_ts_intr(struct phy_device *phydev, bool enable)
 		      PTP_TSU_INT_EN_PTP_RX_TS_EN_ |
 		      PTP_TSU_INT_EN_PTP_RX_TS_OVRFL_EN_;
 
-	return lanphy_write_page_reg(phydev, 5, PTP_TSU_INT_EN, val);
+	return lanphy_write_page_reg(phydev, LAN_EXT_PAGE_5, PTP_TSU_INT_EN,
+				     val);
 }
 
 static void lan8814_ptp_rx_ts_get(struct phy_device *phydev,
 				  u32 *seconds, u32 *nano_seconds, u16 *seq_id)
 {
-	*seconds = lanphy_read_page_reg(phydev, 5, PTP_RX_INGRESS_SEC_HI);
+	*seconds = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_5,
+					PTP_RX_INGRESS_SEC_HI);
 	*seconds = (*seconds << 16) |
-		   lanphy_read_page_reg(phydev, 5, PTP_RX_INGRESS_SEC_LO);
+		   lanphy_read_page_reg(phydev, LAN_EXT_PAGE_5,
+					PTP_RX_INGRESS_SEC_LO);
 
-	*nano_seconds = lanphy_read_page_reg(phydev, 5, PTP_RX_INGRESS_NS_HI);
+	*nano_seconds = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_5
+					     , PTP_RX_INGRESS_NS_HI);
 	*nano_seconds = ((*nano_seconds & 0x3fff) << 16) |
-			lanphy_read_page_reg(phydev, 5, PTP_RX_INGRESS_NS_LO);
+			lanphy_read_page_reg(phydev, LAN_EXT_PAGE_5,
+					     PTP_RX_INGRESS_NS_LO);
 
-	*seq_id = lanphy_read_page_reg(phydev, 5, PTP_RX_MSG_HEADER2);
+	*seq_id = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_5,
+				       PTP_RX_MSG_HEADER2);
 }
 
 static void lan8814_ptp_tx_ts_get(struct phy_device *phydev,
 				  u32 *seconds, u32 *nano_seconds, u16 *seq_id)
 {
-	*seconds = lanphy_read_page_reg(phydev, 5, PTP_TX_EGRESS_SEC_HI);
+	*seconds = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_5,
+					PTP_TX_EGRESS_SEC_HI);
 	*seconds = *seconds << 16 |
-		   lanphy_read_page_reg(phydev, 5, PTP_TX_EGRESS_SEC_LO);
+		   lanphy_read_page_reg(phydev, LAN_EXT_PAGE_5,
+					PTP_TX_EGRESS_SEC_LO);
 
-	*nano_seconds = lanphy_read_page_reg(phydev, 5, PTP_TX_EGRESS_NS_HI);
+	*nano_seconds = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_5,
+					     PTP_TX_EGRESS_NS_HI);
 	*nano_seconds = ((*nano_seconds & 0x3fff) << 16) |
-			lanphy_read_page_reg(phydev, 5, PTP_TX_EGRESS_NS_LO);
+			lanphy_read_page_reg(phydev, LAN_EXT_PAGE_5,
+					     PTP_TX_EGRESS_NS_LO);
 
-	*seq_id = lanphy_read_page_reg(phydev, 5, PTP_TX_MSG_HEADER2);
+	*seq_id = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_5,
+				       PTP_TX_MSG_HEADER2);
 }
 
 static int lan8814_ts_info(struct mii_timestamper *mii_ts, struct kernel_ethtool_ts_info *info)
@@ -2928,11 +2946,11 @@ static void lan8814_flush_fifo(struct phy_device *phydev, bool egress)
 	int i;
 
 	for (i = 0; i < FIFO_SIZE; ++i)
-		lanphy_read_page_reg(phydev, 5,
+		lanphy_read_page_reg(phydev, LAN_EXT_PAGE_5,
 				     egress ? PTP_TX_MSG_HEADER2 : PTP_RX_MSG_HEADER2);
 
 	/* Read to clear overflow status bit */
-	lanphy_read_page_reg(phydev, 5, PTP_TSU_INT_STS);
+	lanphy_read_page_reg(phydev, LAN_EXT_PAGE_5, PTP_TSU_INT_STS);
 }
 
 static int lan8814_hwtstamp(struct mii_timestamper *mii_ts,
@@ -2982,20 +3000,26 @@ static int lan8814_hwtstamp(struct mii_timestamper *mii_ts,
 		rxcfg |= PTP_RX_PARSE_CONFIG_IPV4_EN_ | PTP_RX_PARSE_CONFIG_IPV6_EN_;
 		txcfg |= PTP_TX_PARSE_CONFIG_IPV4_EN_ | PTP_TX_PARSE_CONFIG_IPV6_EN_;
 	}
-	lanphy_write_page_reg(ptp_priv->phydev, 5, PTP_RX_PARSE_CONFIG, rxcfg);
-	lanphy_write_page_reg(ptp_priv->phydev, 5, PTP_TX_PARSE_CONFIG, txcfg);
+	lanphy_write_page_reg(ptp_priv->phydev, LAN_EXT_PAGE_5,
+			      PTP_RX_PARSE_CONFIG, rxcfg);
+	lanphy_write_page_reg(ptp_priv->phydev, LAN_EXT_PAGE_5,
+			      PTP_TX_PARSE_CONFIG, txcfg);
 
 	pkt_ts_enable = PTP_TIMESTAMP_EN_SYNC_ | PTP_TIMESTAMP_EN_DREQ_ |
 			PTP_TIMESTAMP_EN_PDREQ_ | PTP_TIMESTAMP_EN_PDRES_;
-	lanphy_write_page_reg(ptp_priv->phydev, 5, PTP_RX_TIMESTAMP_EN, pkt_ts_enable);
-	lanphy_write_page_reg(ptp_priv->phydev, 5, PTP_TX_TIMESTAMP_EN, pkt_ts_enable);
+	lanphy_write_page_reg(ptp_priv->phydev, LAN_EXT_PAGE_5,
+			      PTP_RX_TIMESTAMP_EN, pkt_ts_enable);
+	lanphy_write_page_reg(ptp_priv->phydev, LAN_EXT_PAGE_5,
+			      PTP_TX_TIMESTAMP_EN, pkt_ts_enable);
 
 	if (ptp_priv->hwts_tx_type == HWTSTAMP_TX_ONESTEP_SYNC) {
-		lanphy_modify_page_reg(ptp_priv->phydev, 5, PTP_TX_MOD,
+		lanphy_modify_page_reg(ptp_priv->phydev, LAN_EXT_PAGE_5,
+				       PTP_TX_MOD,
 				       PTP_TX_MOD_TX_PTP_SYNC_TS_INSERT_,
 				       PTP_TX_MOD_TX_PTP_SYNC_TS_INSERT_);
 	} else if (ptp_priv->hwts_tx_type == HWTSTAMP_TX_ON) {
-		lanphy_modify_page_reg(ptp_priv->phydev, 5, PTP_TX_MOD,
+		lanphy_modify_page_reg(ptp_priv->phydev, LAN_EXT_PAGE_5,
+				       PTP_TX_MOD,
 				       PTP_TX_MOD_TX_PTP_SYNC_TS_INSERT_,
 				       0);
 	}
@@ -3119,29 +3143,41 @@ static bool lan8814_rxtstamp(struct mii_timestamper *mii_ts, struct sk_buff *skb
 static void lan8814_ptp_clock_set(struct phy_device *phydev,
 				  time64_t sec, u32 nsec)
 {
-	lanphy_write_page_reg(phydev, 4, PTP_CLOCK_SET_SEC_LO, lower_16_bits(sec));
-	lanphy_write_page_reg(phydev, 4, PTP_CLOCK_SET_SEC_MID, upper_16_bits(sec));
-	lanphy_write_page_reg(phydev, 4, PTP_CLOCK_SET_SEC_HI, upper_32_bits(sec));
-	lanphy_write_page_reg(phydev, 4, PTP_CLOCK_SET_NS_LO, lower_16_bits(nsec));
-	lanphy_write_page_reg(phydev, 4, PTP_CLOCK_SET_NS_HI, upper_16_bits(nsec));
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4, PTP_CLOCK_SET_SEC_LO,
+			      lower_16_bits(sec));
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4, PTP_CLOCK_SET_SEC_MID,
+			      upper_16_bits(sec));
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4, PTP_CLOCK_SET_SEC_HI,
+			      upper_32_bits(sec));
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4, PTP_CLOCK_SET_NS_LO,
+			      lower_16_bits(nsec));
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4, PTP_CLOCK_SET_NS_HI,
+			      upper_16_bits(nsec));
 
-	lanphy_write_page_reg(phydev, 4, PTP_CMD_CTL, PTP_CMD_CTL_PTP_CLOCK_LOAD_);
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4, PTP_CMD_CTL,
+			      PTP_CMD_CTL_PTP_CLOCK_LOAD_);
 }
 
 static void lan8814_ptp_clock_get(struct phy_device *phydev,
 				  time64_t *sec, u32 *nsec)
 {
-	lanphy_write_page_reg(phydev, 4, PTP_CMD_CTL, PTP_CMD_CTL_PTP_CLOCK_READ_);
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4, PTP_CMD_CTL,
+			      PTP_CMD_CTL_PTP_CLOCK_READ_);
 
-	*sec = lanphy_read_page_reg(phydev, 4, PTP_CLOCK_READ_SEC_HI);
+	*sec = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_4,
+				    PTP_CLOCK_READ_SEC_HI);
 	*sec <<= 16;
-	*sec |= lanphy_read_page_reg(phydev, 4, PTP_CLOCK_READ_SEC_MID);
+	*sec |= lanphy_read_page_reg(phydev, LAN_EXT_PAGE_4,
+				     PTP_CLOCK_READ_SEC_MID);
 	*sec <<= 16;
-	*sec |= lanphy_read_page_reg(phydev, 4, PTP_CLOCK_READ_SEC_LO);
+	*sec |= lanphy_read_page_reg(phydev, LAN_EXT_PAGE_4,
+				     PTP_CLOCK_READ_SEC_LO);
 
-	*nsec = lanphy_read_page_reg(phydev, 4, PTP_CLOCK_READ_NS_HI);
+	*nsec = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_4,
+				     PTP_CLOCK_READ_NS_HI);
 	*nsec <<= 16;
-	*nsec |= lanphy_read_page_reg(phydev, 4, PTP_CLOCK_READ_NS_LO);
+	*nsec |= lanphy_read_page_reg(phydev, LAN_EXT_PAGE_4,
+				      PTP_CLOCK_READ_NS_LO);
 }
 
 static int lan8814_ptpci_gettime64(struct ptp_clock_info *ptpci,
@@ -3180,14 +3216,18 @@ static void lan8814_ptp_set_target(struct phy_device *phydev, int event,
 				   s64 start_sec, u32 start_nsec)
 {
 	/* Set the start time */
-	lanphy_write_page_reg(phydev, 4, LAN8814_PTP_CLOCK_TARGET_SEC_LO(event),
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
+			      LAN8814_PTP_CLOCK_TARGET_SEC_LO(event),
 			      lower_16_bits(start_sec));
-	lanphy_write_page_reg(phydev, 4, LAN8814_PTP_CLOCK_TARGET_SEC_HI(event),
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
+			      LAN8814_PTP_CLOCK_TARGET_SEC_HI(event),
 			      upper_16_bits(start_sec));
 
-	lanphy_write_page_reg(phydev, 4, LAN8814_PTP_CLOCK_TARGET_NS_LO(event),
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
+			      LAN8814_PTP_CLOCK_TARGET_NS_LO(event),
 			      lower_16_bits(start_nsec));
-	lanphy_write_page_reg(phydev, 4, LAN8814_PTP_CLOCK_TARGET_NS_HI(event),
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
+			      LAN8814_PTP_CLOCK_TARGET_NS_HI(event),
 			      upper_16_bits(start_nsec) & 0x3fff);
 }
 
@@ -3285,9 +3325,11 @@ static void lan8814_ptp_clock_step(struct phy_device *phydev,
 			adjustment_value_lo = adjustment_value & 0xffff;
 			adjustment_value_hi = (adjustment_value >> 16) & 0x3fff;
 
-			lanphy_write_page_reg(phydev, 4, PTP_LTC_STEP_ADJ_LO,
+			lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
+					      PTP_LTC_STEP_ADJ_LO,
 					      adjustment_value_lo);
-			lanphy_write_page_reg(phydev, 4, PTP_LTC_STEP_ADJ_HI,
+			lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
+					      PTP_LTC_STEP_ADJ_HI,
 					      PTP_LTC_STEP_ADJ_DIR_ |
 					      adjustment_value_hi);
 			seconds -= ((s32)adjustment_value);
@@ -3305,9 +3347,11 @@ static void lan8814_ptp_clock_step(struct phy_device *phydev,
 			adjustment_value_lo = adjustment_value & 0xffff;
 			adjustment_value_hi = (adjustment_value >> 16) & 0x3fff;
 
-			lanphy_write_page_reg(phydev, 4, PTP_LTC_STEP_ADJ_LO,
+			lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
+					      PTP_LTC_STEP_ADJ_LO,
 					      adjustment_value_lo);
-			lanphy_write_page_reg(phydev, 4, PTP_LTC_STEP_ADJ_HI,
+			lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
+					      PTP_LTC_STEP_ADJ_HI,
 					      adjustment_value_hi);
 			seconds += ((s32)adjustment_value);
 
@@ -3315,7 +3359,7 @@ static void lan8814_ptp_clock_step(struct phy_device *phydev,
 			set_seconds += adjustment_value;
 			lan8814_ptp_update_target(phydev, set_seconds);
 		}
-		lanphy_write_page_reg(phydev, 4, PTP_CMD_CTL,
+		lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4, PTP_CMD_CTL,
 				      PTP_CMD_CTL_PTP_LTC_STEP_SEC_);
 	}
 	if (nano_seconds) {
@@ -3325,12 +3369,14 @@ static void lan8814_ptp_clock_step(struct phy_device *phydev,
 		nano_seconds_lo = nano_seconds & 0xffff;
 		nano_seconds_hi = (nano_seconds >> 16) & 0x3fff;
 
-		lanphy_write_page_reg(phydev, 4, PTP_LTC_STEP_ADJ_LO,
+		lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
+				      PTP_LTC_STEP_ADJ_LO,
 				      nano_seconds_lo);
-		lanphy_write_page_reg(phydev, 4, PTP_LTC_STEP_ADJ_HI,
+		lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
+				      PTP_LTC_STEP_ADJ_HI,
 				      PTP_LTC_STEP_ADJ_DIR_ |
 				      nano_seconds_hi);
-		lanphy_write_page_reg(phydev, 4, PTP_CMD_CTL,
+		lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4, PTP_CMD_CTL,
 				      PTP_CMD_CTL_PTP_LTC_STEP_NSEC_);
 	}
 }
@@ -3372,8 +3418,10 @@ static int lan8814_ptpci_adjfine(struct ptp_clock_info *ptpci, long scaled_ppm)
 		kszphy_rate_adj_hi |= PTP_CLOCK_RATE_ADJ_DIR_;
 
 	mutex_lock(&shared->shared_lock);
-	lanphy_write_page_reg(phydev, 4, PTP_CLOCK_RATE_ADJ_HI, kszphy_rate_adj_hi);
-	lanphy_write_page_reg(phydev, 4, PTP_CLOCK_RATE_ADJ_LO, kszphy_rate_adj_lo);
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4, PTP_CLOCK_RATE_ADJ_HI,
+			      kszphy_rate_adj_hi);
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4, PTP_CLOCK_RATE_ADJ_LO,
+			      kszphy_rate_adj_lo);
 	mutex_unlock(&shared->shared_lock);
 
 	return 0;
@@ -3382,17 +3430,17 @@ static int lan8814_ptpci_adjfine(struct ptp_clock_info *ptpci, long scaled_ppm)
 static void lan8814_ptp_set_reload(struct phy_device *phydev, int event,
 				   s64 period_sec, u32 period_nsec)
 {
-	lanphy_write_page_reg(phydev, 4,
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
 			      LAN8814_PTP_CLOCK_TARGET_RELOAD_SEC_LO(event),
 			      lower_16_bits(period_sec));
-	lanphy_write_page_reg(phydev, 4,
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
 			      LAN8814_PTP_CLOCK_TARGET_RELOAD_SEC_HI(event),
 			      upper_16_bits(period_sec));
 
-	lanphy_write_page_reg(phydev, 4,
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
 			      LAN8814_PTP_CLOCK_TARGET_RELOAD_NS_LO(event),
 			      lower_16_bits(period_nsec));
-	lanphy_write_page_reg(phydev, 4,
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
 			      LAN8814_PTP_CLOCK_TARGET_RELOAD_NS_HI(event),
 			      upper_16_bits(period_nsec) & 0x3fff);
 }
@@ -3405,7 +3453,7 @@ static void lan8814_ptp_enable_event(struct phy_device *phydev, int event,
 	 * local time reaches or pass it
 	 * Set the polarity high
 	 */
-	lanphy_modify_page_reg(phydev, 4, LAN8814_PTP_GENERAL_CONFIG,
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4, LAN8814_PTP_GENERAL_CONFIG,
 			       LAN8814_PTP_GENERAL_CONFIG_LTC_EVENT_MASK(event) |
 			       LAN8814_PTP_GENERAL_CONFIG_LTC_EVENT_SET(event, pulse_width) |
 			       LAN8814_PTP_GENERAL_CONFIG_RELOAD_ADD_X(event) |
@@ -3420,7 +3468,7 @@ static void lan8814_ptp_disable_event(struct phy_device *phydev, int event)
 	lan8814_ptp_set_target(phydev, event, 0xFFFFFFFF, 0);
 
 	/* And then reload once it recheas the target */
-	lanphy_modify_page_reg(phydev, 4, LAN8814_PTP_GENERAL_CONFIG,
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4, LAN8814_PTP_GENERAL_CONFIG,
 			       LAN8814_PTP_GENERAL_CONFIG_RELOAD_ADD_X(event),
 			       LAN8814_PTP_GENERAL_CONFIG_RELOAD_ADD_X(event));
 }
@@ -3431,15 +3479,18 @@ static void lan8814_ptp_perout_off(struct phy_device *phydev, int pin)
 	 * 1: select as gpio,
 	 * 0: select alt func
 	 */
-	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_EN_ADDR(pin),
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4,
+			       LAN8814_GPIO_EN_ADDR(pin),
 			       LAN8814_GPIO_EN_BIT(pin),
 			       LAN8814_GPIO_EN_BIT(pin));
 
-	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin),
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4,
+			       LAN8814_GPIO_DIR_ADDR(pin),
 			       LAN8814_GPIO_DIR_BIT(pin),
 			       0);
 
-	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_BUF_ADDR(pin),
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4,
+			       LAN8814_GPIO_BUF_ADDR(pin),
 			       LAN8814_GPIO_BUF_BIT(pin),
 			       0);
 }
@@ -3447,17 +3498,20 @@ static void lan8814_ptp_perout_off(struct phy_device *phydev, int pin)
 static void lan8814_ptp_perout_on(struct phy_device *phydev, int pin)
 {
 	/* Set as gpio output */
-	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin),
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4,
+			       LAN8814_GPIO_DIR_ADDR(pin),
 			       LAN8814_GPIO_DIR_BIT(pin),
 			       LAN8814_GPIO_DIR_BIT(pin));
 
 	/* Enable gpio 0:for alternate function, 1:gpio */
-	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_EN_ADDR(pin),
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4,
+			       LAN8814_GPIO_EN_ADDR(pin),
 			       LAN8814_GPIO_EN_BIT(pin),
 			       0);
 
 	/* Set buffer type to push pull */
-	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_BUF_ADDR(pin),
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4,
+			       LAN8814_GPIO_BUF_ADDR(pin),
 			       LAN8814_GPIO_BUF_BIT(pin),
 			       LAN8814_GPIO_BUF_BIT(pin));
 }
@@ -3575,27 +3629,28 @@ static int lan8814_ptp_perout(struct ptp_clock_info *ptpci,
 static void lan8814_ptp_extts_on(struct phy_device *phydev, int pin, u32 flags)
 {
 	/* Set as gpio input */
-	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin),
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4,
+			       LAN8814_GPIO_DIR_ADDR(pin),
 			       LAN8814_GPIO_DIR_BIT(pin),
 			       0);
 
 	/* Map the pin to ltc pin 0 of the capture map registers */
-	lanphy_modify_page_reg(phydev, 4, PTP_GPIO_CAP_MAP_LO,
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4, PTP_GPIO_CAP_MAP_LO,
 			       pin,
 			       pin);
 
 	/* Enable capture on the edges of the ltc pin */
 	if (flags & PTP_RISING_EDGE)
-		lanphy_modify_page_reg(phydev, 4, PTP_GPIO_CAP_EN,
+		lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4, PTP_GPIO_CAP_EN,
 				       PTP_GPIO_CAP_EN_GPIO_RE_CAPTURE_ENABLE(0),
 				       PTP_GPIO_CAP_EN_GPIO_RE_CAPTURE_ENABLE(0));
 	if (flags & PTP_FALLING_EDGE)
-		lanphy_modify_page_reg(phydev, 4, PTP_GPIO_CAP_EN,
+		lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4, PTP_GPIO_CAP_EN,
 				       PTP_GPIO_CAP_EN_GPIO_FE_CAPTURE_ENABLE(0),
 				       PTP_GPIO_CAP_EN_GPIO_FE_CAPTURE_ENABLE(0));
 
 	/* Enable interrupt top interrupt */
-	lanphy_modify_page_reg(phydev, 4, PTP_COMMON_INT_ENA,
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4, PTP_COMMON_INT_ENA,
 			       PTP_COMMON_INT_ENA_GPIO_CAP_EN,
 			       PTP_COMMON_INT_ENA_GPIO_CAP_EN);
 }
@@ -3603,28 +3658,31 @@ static void lan8814_ptp_extts_on(struct phy_device *phydev, int pin, u32 flags)
 static void lan8814_ptp_extts_off(struct phy_device *phydev, int pin)
 {
 	/* Set as gpio out */
-	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_DIR_ADDR(pin),
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4,
+			       LAN8814_GPIO_DIR_ADDR(pin),
 			       LAN8814_GPIO_DIR_BIT(pin),
 			       LAN8814_GPIO_DIR_BIT(pin));
 
 	/* Enable alternate, 0:for alternate function, 1:gpio */
-	lanphy_modify_page_reg(phydev, 4, LAN8814_GPIO_EN_ADDR(pin),
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4,
+			       LAN8814_GPIO_EN_ADDR(pin),
 			       LAN8814_GPIO_EN_BIT(pin),
 			       0);
 
 	/* Clear the mapping of pin to registers 0 of the capture registers */
-	lanphy_modify_page_reg(phydev, 4, PTP_GPIO_CAP_MAP_LO,
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4,
+			       PTP_GPIO_CAP_MAP_LO,
 			       GENMASK(3, 0),
 			       0);
 
 	/* Disable capture on both of the edges */
-	lanphy_modify_page_reg(phydev, 4, PTP_GPIO_CAP_EN,
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4, PTP_GPIO_CAP_EN,
 			       PTP_GPIO_CAP_EN_GPIO_RE_CAPTURE_ENABLE(pin) |
 			       PTP_GPIO_CAP_EN_GPIO_FE_CAPTURE_ENABLE(pin),
 			       0);
 
 	/* Disable interrupt top interrupt */
-	lanphy_modify_page_reg(phydev, 4, PTP_COMMON_INT_ENA,
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4, PTP_COMMON_INT_ENA,
 			       PTP_COMMON_INT_ENA_GPIO_CAP_EN,
 			       0);
 }
@@ -3756,7 +3814,8 @@ static void lan8814_get_tx_ts(struct kszphy_ptp_priv *ptp_priv)
 		/* If other timestamps are available in the FIFO,
 		 * process them.
 		 */
-		reg = lanphy_read_page_reg(phydev, 5, PTP_CAP_INFO);
+		reg = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_5,
+					   PTP_CAP_INFO);
 	} while (PTP_CAP_INFO_TX_TS_CNT_GET_(reg) > 0);
 }
 
@@ -3829,7 +3888,8 @@ static void lan8814_get_rx_ts(struct kszphy_ptp_priv *ptp_priv)
 		/* If other timestamps are available in the FIFO,
 		 * process them.
 		 */
-		reg = lanphy_read_page_reg(phydev, 5, PTP_CAP_INFO);
+		reg = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_5,
+					   PTP_CAP_INFO);
 	} while (PTP_CAP_INFO_RX_TS_CNT_GET_(reg) > 0);
 }
 
@@ -3866,31 +3926,39 @@ static int lan8814_gpio_process_cap(struct lan8814_shared_priv *shared)
 	/* This is 0 because whatever was the input pin it was mapped it to
 	 * ltc gpio pin 0
 	 */
-	lanphy_modify_page_reg(phydev, 4, PTP_GPIO_SEL,
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4, PTP_GPIO_SEL,
 			       PTP_GPIO_SEL_GPIO_SEL(0),
 			       PTP_GPIO_SEL_GPIO_SEL(0));
 
-	tmp = lanphy_read_page_reg(phydev, 4, PTP_GPIO_CAP_STS);
+	tmp = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_4, PTP_GPIO_CAP_STS);
 	if (!(tmp & PTP_GPIO_CAP_STS_PTP_GPIO_RE_STS(0)) &&
 	    !(tmp & PTP_GPIO_CAP_STS_PTP_GPIO_FE_STS(0)))
 		return -1;
 
 	if (tmp & BIT(0)) {
-		sec = lanphy_read_page_reg(phydev, 4, PTP_GPIO_RE_LTC_SEC_HI_CAP);
+		sec = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_4,
+					   PTP_GPIO_RE_LTC_SEC_HI_CAP);
 		sec <<= 16;
-		sec |= lanphy_read_page_reg(phydev, 4, PTP_GPIO_RE_LTC_SEC_LO_CAP);
+		sec |= lanphy_read_page_reg(phydev, LAN_EXT_PAGE_4,
+					    PTP_GPIO_RE_LTC_SEC_LO_CAP);
 
-		nsec = lanphy_read_page_reg(phydev, 4, PTP_GPIO_RE_LTC_NS_HI_CAP) & 0x3fff;
+		nsec = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_4,
+					    PTP_GPIO_RE_LTC_NS_HI_CAP) & 0x3fff;
 		nsec <<= 16;
-		nsec |= lanphy_read_page_reg(phydev, 4, PTP_GPIO_RE_LTC_NS_LO_CAP);
+		nsec |= lanphy_read_page_reg(phydev, LAN_EXT_PAGE_4,
+					     PTP_GPIO_RE_LTC_NS_LO_CAP);
 	} else {
-		sec = lanphy_read_page_reg(phydev, 4, PTP_GPIO_FE_LTC_SEC_HI_CAP);
+		sec = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_4,
+					   PTP_GPIO_FE_LTC_SEC_HI_CAP);
 		sec <<= 16;
-		sec |= lanphy_read_page_reg(phydev, 4, PTP_GPIO_FE_LTC_SEC_LO_CAP);
+		sec |= lanphy_read_page_reg(phydev, LAN_EXT_PAGE_4,
+					    PTP_GPIO_FE_LTC_SEC_LO_CAP);
 
-		nsec = lanphy_read_page_reg(phydev, 4, PTP_GPIO_FE_LTC_NS_HI_CAP) & 0x3fff;
+		nsec = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_4,
+					    PTP_GPIO_FE_LTC_NS_HI_CAP) & 0x3fff;
 		nsec <<= 16;
-		nsec |= lanphy_read_page_reg(phydev, 4, PTP_GPIO_RE_LTC_NS_LO_CAP);
+		nsec |= lanphy_read_page_reg(phydev, LAN_EXT_PAGE_4,
+					     PTP_GPIO_RE_LTC_NS_LO_CAP);
 	}
 
 	ptp_event.index = 0;
@@ -3916,15 +3984,16 @@ static int lan8814_handle_gpio_interrupt(struct phy_device *phydev, u16 status)
 static int lan8804_config_init(struct phy_device *phydev)
 {
 	/* MDI-X setting for swap A,B transmit */
-	lanphy_modify_page_reg(phydev, 2, LAN8804_ALIGN_SWAP,
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_2, LAN8804_ALIGN_SWAP,
 			       LAN8804_ALIGN_TX_A_B_SWAP_MASK,
 			       LAN8804_ALIGN_TX_A_B_SWAP);
 
 	/* Make sure that the PHY will not stop generating the clock when the
 	 * link partner goes down
 	 */
-	lanphy_write_page_reg(phydev, 31, LAN8814_CLOCK_MANAGEMENT, 0x27e);
-	lanphy_read_page_reg(phydev, 1, LAN8814_LINK_QUALITY);
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_31,
+			      LAN8814_CLOCK_MANAGEMENT, 0x27e);
+	lanphy_read_page_reg(phydev, LAN_EXT_PAGE_1, LAN8814_LINK_QUALITY);
 
 	return 0;
 }
@@ -4006,7 +4075,8 @@ static irqreturn_t lan8814_handle_interrupt(struct phy_device *phydev)
 	}
 
 	while (true) {
-		irq_status = lanphy_read_page_reg(phydev, 5, PTP_TSU_INT_STS);
+		irq_status = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_5,
+						  PTP_TSU_INT_STS);
 		if (!irq_status)
 			break;
 
@@ -4034,7 +4104,7 @@ static int lan8814_config_intr(struct phy_device *phydev)
 {
 	int err;
 
-	lanphy_write_page_reg(phydev, 4, LAN8814_INTR_CTRL_REG,
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4, LAN8814_INTR_CTRL_REG,
 			      LAN8814_INTR_CTRL_REG_POLARITY |
 			      LAN8814_INTR_CTRL_REG_INTR_ENABLE);
 
@@ -4065,29 +4135,34 @@ static void lan8814_ptp_init(struct phy_device *phydev)
 	    !IS_ENABLED(CONFIG_NETWORK_PHY_TIMESTAMPING))
 		return;
 
-	lanphy_write_page_reg(phydev, 5, TSU_HARD_RESET, TSU_HARD_RESET_);
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_5,
+			      TSU_HARD_RESET, TSU_HARD_RESET_);
 
-	lanphy_modify_page_reg(phydev, 5, PTP_TX_MOD,
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_5, PTP_TX_MOD,
 			       PTP_TX_MOD_BAD_UDPV4_CHKSUM_FORCE_FCS_DIS_,
 			       PTP_TX_MOD_BAD_UDPV4_CHKSUM_FORCE_FCS_DIS_);
 
-	lanphy_modify_page_reg(phydev, 5, PTP_RX_MOD,
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_5, PTP_RX_MOD,
 			       PTP_RX_MOD_BAD_UDPV4_CHKSUM_FORCE_FCS_DIS_,
 			       PTP_RX_MOD_BAD_UDPV4_CHKSUM_FORCE_FCS_DIS_);
 
-	lanphy_write_page_reg(phydev, 5, PTP_RX_PARSE_CONFIG, 0);
-	lanphy_write_page_reg(phydev, 5, PTP_TX_PARSE_CONFIG, 0);
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_5, PTP_RX_PARSE_CONFIG, 0);
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_5, PTP_TX_PARSE_CONFIG, 0);
 
 	/* Removing default registers configs related to L2 and IP */
-	lanphy_write_page_reg(phydev, 5, PTP_TX_PARSE_L2_ADDR_EN, 0);
-	lanphy_write_page_reg(phydev, 5, PTP_RX_PARSE_L2_ADDR_EN, 0);
-	lanphy_write_page_reg(phydev, 5, PTP_TX_PARSE_IP_ADDR_EN, 0);
-	lanphy_write_page_reg(phydev, 5, PTP_RX_PARSE_IP_ADDR_EN, 0);
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_5,
+			      PTP_TX_PARSE_L2_ADDR_EN, 0);
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_5,
+			      PTP_RX_PARSE_L2_ADDR_EN, 0);
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_5,
+			      PTP_TX_PARSE_IP_ADDR_EN, 0);
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_5,
+			      PTP_RX_PARSE_IP_ADDR_EN, 0);
 
 	/* Disable checking for minorVersionPTP field */
-	lanphy_write_page_reg(phydev, 5, PTP_RX_VERSION,
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_5, PTP_RX_VERSION,
 			      PTP_MAX_VERSION(0xff) | PTP_MIN_VERSION(0x0));
-	lanphy_write_page_reg(phydev, 5, PTP_TX_VERSION,
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_5, PTP_TX_VERSION,
 			      PTP_MAX_VERSION(0xff) | PTP_MIN_VERSION(0x0));
 
 	skb_queue_head_init(&ptp_priv->tx_queue);
@@ -4172,12 +4247,14 @@ static int lan8814_ptp_probe_once(struct phy_device *phydev)
 	/* The EP.4 is shared between all the PHYs in the package and also it
 	 * can be accessed by any of the PHYs
 	 */
-	lanphy_write_page_reg(phydev, 4, LTC_HARD_RESET, LTC_HARD_RESET_);
-	lanphy_write_page_reg(phydev, 4, PTP_OPERATING_MODE,
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
+			      LTC_HARD_RESET, LTC_HARD_RESET_);
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4, PTP_OPERATING_MODE,
 			      PTP_OPERATING_MODE_STANDALONE_);
 
 	/* Enable ptp to run LTC clock for ptp and gpio 1PPS operation */
-	lanphy_write_page_reg(phydev, 4, PTP_CMD_CTL, PTP_CMD_CTL_PTP_ENABLE_);
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4, PTP_CMD_CTL,
+			      PTP_CMD_CTL_PTP_ENABLE_);
 
 	return 0;
 }
@@ -4186,14 +4263,14 @@ static void lan8814_setup_led(struct phy_device *phydev, int val)
 {
 	int temp;
 
-	temp = lanphy_read_page_reg(phydev, 5, LAN8814_LED_CTRL_1);
+	temp = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_5, LAN8814_LED_CTRL_1);
 
 	if (val)
 		temp |= LAN8814_LED_CTRL_1_KSZ9031_LED_MODE_;
 	else
 		temp &= ~LAN8814_LED_CTRL_1_KSZ9031_LED_MODE_;
 
-	lanphy_write_page_reg(phydev, 5, LAN8814_LED_CTRL_1, temp);
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_5, LAN8814_LED_CTRL_1, temp);
 }
 
 static int lan8814_config_init(struct phy_device *phydev)
@@ -4201,17 +4278,19 @@ static int lan8814_config_init(struct phy_device *phydev)
 	struct kszphy_priv *lan8814 = phydev->priv;
 
 	/* Reset the PHY */
-	lanphy_modify_page_reg(phydev, 4, LAN8814_QSGMII_SOFT_RESET,
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4,
+			       LAN8814_QSGMII_SOFT_RESET,
 			       LAN8814_QSGMII_SOFT_RESET_BIT,
 			       LAN8814_QSGMII_SOFT_RESET_BIT);
 
 	/* Disable ANEG with QSGMII PCS Host side */
-	lanphy_modify_page_reg(phydev, 4, LAN8814_QSGMII_PCS1G_ANEG_CONFIG,
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4,
+			       LAN8814_QSGMII_PCS1G_ANEG_CONFIG,
 			       LAN8814_QSGMII_PCS1G_ANEG_CONFIG_ANEG_ENA,
 			       0);
 
 	/* MDI-X setting for swap A,B transmit */
-	lanphy_modify_page_reg(phydev, 2, LAN8814_ALIGN_SWAP,
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_2, LAN8814_ALIGN_SWAP,
 			       LAN8814_ALIGN_TX_A_B_SWAP_MASK,
 			       LAN8814_ALIGN_TX_A_B_SWAP);
 
@@ -4248,7 +4327,7 @@ static void lan8814_clear_2psp_bit(struct phy_device *phydev)
 	 * cable is removed then the LED was still one even though there is no
 	 * link
 	 */
-	lanphy_modify_page_reg(phydev, 2, LAN8814_EEE_STATE,
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_2, LAN8814_EEE_STATE,
 			       LAN8814_EEE_STATE_MASK2P5P,
 			       0);
 }
@@ -4259,7 +4338,7 @@ static void lan8814_update_meas_time(struct phy_device *phydev)
 	 * longer than 100m to be used. This configuration can be used
 	 * regardless of the mode of operation of the PHY
 	 */
-	lanphy_modify_page_reg(phydev, 1, LAN8814_PD_CONTROLS,
+	lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_1, LAN8814_PD_CONTROLS,
 			       LAN8814_PD_CONTROLS_PD_MEAS_TIME_MASK,
 			       LAN8814_PD_CONTROLS_PD_MEAS_TIME_VAL);
 }
@@ -4284,7 +4363,7 @@ static int lan8814_probe(struct phy_device *phydev)
 	/* Strap-in value for PHY address, below register read gives starting
 	 * phy address value
 	 */
-	addr = lanphy_read_page_reg(phydev, 4, 0) & 0x1F;
+	addr = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_4, 0) & 0x1F;
 	devm_phy_package_join(&phydev->mdio.dev, phydev,
 			      addr, sizeof(struct lan8814_shared_priv));
 
-- 
2.34.1

[PATCH net-next v2 4/4] net: phy: micrel: Add support for lan8842

[Horatiu Vultur]

The LAN8842 is a low-power, single port triple-speed (10BASE-T/ 100BASE-TX/
1000BASE-T) ethernet physical layer transceiver (PHY) that supports
transmission and reception of data on standard CAT-5, as well as CAT-5e and
CAT-6, Unshielded Twisted Pair (UTP) cables.

The LAN8842 supports industry-standard SGMII (Serial Gigabit Media
Independent Interface) providing chip-to-chip connection to a Gigabit
Ethernet MAC using a single serialized link (differential pair) in each
direction.

There are 2 variants of the lan8842. The one that supports timestamping
(lan8842) and one that doesn't have timestamping (lan8832).

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 drivers/net/phy/micrel.c   | 200 +++++++++++++++++++++++++++++++++++++
 include/linux/micrel_phy.h |   1 +
 2 files changed, 201 insertions(+)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index d20f028106b7d..da39d9a1b251a 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -448,6 +448,17 @@ struct kszphy_priv {
 	struct kszphy_phy_stats phy_stats;
 };
 
+struct lan8842_phy_stats {
+	u64 rx_packets;
+	u64 rx_errors;
+	u64 tx_packets;
+	u64 tx_errors;
+};
+
+struct lan8842_priv {
+	struct lan8842_phy_stats phy_stats;
+};
+
 static const struct kszphy_type lan8814_type = {
 	.led_mode_reg		= ~LAN8814_LED_CTRL_1,
 	.cable_diag_reg		= LAN8814_CABLE_DIAG,
@@ -5718,6 +5729,181 @@ static int ksz9131_resume(struct phy_device *phydev)
 	return kszphy_resume(phydev);
 }
 
+#define LAN8842_SELF_TEST			14 /* 0x0e */
+#define LAN8842_SELF_TEST_RX_CNT_ENA		BIT(8)
+#define LAN8842_SELF_TEST_TX_CNT_ENA		BIT(4)
+
+static int lan8842_probe(struct phy_device *phydev)
+{
+	struct lan8842_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	phydev->priv = priv;
+
+	/* Similar to lan8814 this PHY has a pin which needs to be pulled down
+	 * to enable to pass any traffic through it. Therefore use the same
+	 * function as lan8814
+	 */
+	ret = lan8814_release_coma_mode(phydev);
+	if (ret)
+		return ret;
+
+	/* Enable to count the RX and TX packets */
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_2,
+			      LAN8842_SELF_TEST,
+			      LAN8842_SELF_TEST_RX_CNT_ENA |
+			      LAN8842_SELF_TEST_TX_CNT_ENA);
+
+	return 0;
+}
+
+#define LAN8842_SGMII_AUTO_ANEG_ENA		69 /* 0x45 */
+#define LAN8842_FLF				15 /* 0x0e */
+#define LAN8842_FLF_ENA				BIT(1)
+#define LAN8842_FLF_ENA_LINK_DOWN		BIT(0)
+
+static int lan8842_config_init(struct phy_device *phydev)
+{
+	int ret;
+
+	/* Reset the PHY */
+	ret = lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_4,
+				     LAN8814_QSGMII_SOFT_RESET,
+				     LAN8814_QSGMII_SOFT_RESET_BIT,
+				     LAN8814_QSGMII_SOFT_RESET_BIT);
+	if (ret < 0)
+		return ret;
+
+	/* Disable ANEG with QSGMII PCS Host side
+	 * It has the same address as lan8814
+	 */
+	ret = lanphy_modify_page_reg(phydev, LAN_EXT_PAGE_5,
+				     LAN8814_QSGMII_PCS1G_ANEG_CONFIG,
+				     LAN8814_QSGMII_PCS1G_ANEG_CONFIG_ANEG_ENA,
+				     0);
+	if (ret < 0)
+		return ret;
+
+	/* Disable also the SGMII_AUTO_ANEG_ENA, this will determine what is the
+	 * PHY autoneg with the other end and then will update the host side
+	 */
+	ret = lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
+				    LAN8842_SGMII_AUTO_ANEG_ENA, 0);
+	if (ret < 0)
+		return ret;
+
+	/* To allow the PHY to control the LEDs the GPIOs of the PHY should have
+	 * a function mode and not the GPIO. Apparently by default the value is
+	 * GPIO and not function even though the datasheet it says that it is
+	 * function. Therefore set this value.
+	 */
+	return lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
+				     LAN8814_GPIO_EN2, 0);
+}
+
+#define LAN8842_INTR_CTRL_REG			52 /* 0x34 */
+
+static int lan8842_config_intr(struct phy_device *phydev)
+{
+	int err;
+
+	lanphy_write_page_reg(phydev, LAN_EXT_PAGE_4,
+			      LAN8842_INTR_CTRL_REG,
+			      LAN8814_INTR_CTRL_REG_INTR_ENABLE);
+
+	/* enable / disable interrupts */
+	if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
+		err = lan8814_ack_interrupt(phydev);
+		if (err)
+			return err;
+
+		err = phy_write(phydev, LAN8814_INTC, LAN8814_INT_LINK);
+	} else {
+		err = phy_write(phydev, LAN8814_INTC, 0);
+		if (err)
+			return err;
+
+		err = lan8814_ack_interrupt(phydev);
+	}
+
+	return err;
+}
+
+static irqreturn_t lan8842_handle_interrupt(struct phy_device *phydev)
+{
+	int ret = IRQ_NONE;
+	int irq_status;
+
+	irq_status = phy_read(phydev, LAN8814_INTS);
+	if (irq_status < 0) {
+		phy_error(phydev);
+		return IRQ_NONE;
+	}
+
+	if (irq_status & LAN8814_INT_LINK) {
+		phy_trigger_machine(phydev);
+		ret = IRQ_HANDLED;
+	}
+
+	return ret;
+}
+
+static u64 lan8842_get_stat(struct phy_device *phydev, int count, int *regs)
+{
+	int val;
+	u64 ret = 0;
+
+	for (int j = 0; j < count; ++j) {
+		val = lanphy_read_page_reg(phydev, LAN_EXT_PAGE_2, regs[j]);
+		if (val < 0)
+			return U64_MAX;
+
+		ret <<= 16;
+		ret += val;
+	}
+	return ret;
+}
+
+static int lan8842_update_stats(struct phy_device *phydev)
+{
+	struct lan8842_priv *priv = phydev->priv;
+	int rx_packets_regs[] = {88, 61, 60};
+	int rx_errors_regs[] = {63, 62};
+	int tx_packets_regs[] = {89, 85, 84};
+	int tx_errors_regs[] = {87, 86};
+
+	priv->phy_stats.rx_packets = lan8842_get_stat(phydev,
+						      ARRAY_SIZE(rx_packets_regs),
+						      rx_packets_regs);
+	priv->phy_stats.rx_errors = lan8842_get_stat(phydev,
+						     ARRAY_SIZE(rx_errors_regs),
+						     rx_errors_regs);
+	priv->phy_stats.tx_packets = lan8842_get_stat(phydev,
+						      ARRAY_SIZE(tx_packets_regs),
+						      tx_packets_regs);
+	priv->phy_stats.tx_errors = lan8842_get_stat(phydev,
+						     ARRAY_SIZE(tx_errors_regs),
+						     tx_errors_regs);
+
+	return 0;
+}
+
+static void lan8842_get_phy_stats(struct phy_device *phydev,
+				  struct ethtool_eth_phy_stats *eth_stats,
+				  struct ethtool_phy_stats *stats)
+{
+	struct lan8842_priv *priv = phydev->priv;
+
+	stats->rx_packets = priv->phy_stats.rx_packets;
+	stats->rx_errors = priv->phy_stats.rx_errors;
+	stats->tx_packets = priv->phy_stats.tx_packets;
+	stats->tx_errors = priv->phy_stats.rx_errors;
+}
+
 static struct phy_driver ksphy_driver[] = {
 {
 	PHY_ID_MATCH_MODEL(PHY_ID_KS8737),
@@ -5937,6 +6123,19 @@ static struct phy_driver ksphy_driver[] = {
 	.resume		= lan8841_resume,
 	.cable_test_start	= lan8814_cable_test_start,
 	.cable_test_get_status	= ksz886x_cable_test_get_status,
+}, {
+	PHY_ID_MATCH_MODEL(PHY_ID_LAN8842),
+	.name		= "Microchip LAN8842 Gigabit PHY",
+	.flags		= PHY_POLL_CABLE_TEST,
+	.driver_data	= &lan8814_type,
+	.probe		= lan8842_probe,
+	.config_init	= lan8842_config_init,
+	.config_intr	= lan8842_config_intr,
+	.handle_interrupt = lan8842_handle_interrupt,
+	.get_phy_stats	= lan8842_get_phy_stats,
+	.update_stats	= lan8842_update_stats,
+	.cable_test_start	= lan8814_cable_test_start,
+	.cable_test_get_status	= ksz886x_cable_test_get_status,
 }, {
 	PHY_ID_MATCH_MODEL(PHY_ID_KSZ9131),
 	.name		= "Microchip KSZ9131 Gigabit PHY",
@@ -6032,6 +6231,7 @@ static const struct mdio_device_id __maybe_unused micrel_tbl[] = {
 	{ PHY_ID_MATCH_MODEL(PHY_ID_LAN8814) },
 	{ PHY_ID_MATCH_MODEL(PHY_ID_LAN8804) },
 	{ PHY_ID_MATCH_MODEL(PHY_ID_LAN8841) },
+	{ PHY_ID_MATCH_MODEL(PHY_ID_LAN8842) },
 	{ }
 };
 
diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h
index 9af01bdd86d26..ca691641788b8 100644
--- a/include/linux/micrel_phy.h
+++ b/include/linux/micrel_phy.h
@@ -32,6 +32,7 @@
 #define PHY_ID_LAN8814		0x00221660
 #define PHY_ID_LAN8804		0x00221670
 #define PHY_ID_LAN8841		0x00221650
+#define PHY_ID_LAN8842		0x002216C0
 
 #define PHY_ID_KSZ886X		0x00221430
 #define PHY_ID_KSZ8863		0x00221435
-- 
2.34.1

Re: [PATCH net-next v2 2/4] net: phy: micrel: Introduce lanphy_modify_page_reg

[Russell King (Oracle)]

On Thu, Jul 24, 2025 at 10:08:24PM +0200, Horatiu Vultur wrote:
> +static int lanphy_modify_page_reg(struct phy_device *phydev, int page, u16 addr,
> +				  u16 mask, u16 set)
> +{
> +	int new, ret;
> +
> +	ret = lanphy_read_page_reg(phydev, page, addr);
> +	if (ret < 0)
> +		return ret;
> +
> +	new = (ret & ~mask) | set;
> +	if (new == ret)
> +		return 0;
> +
> +	ret = lanphy_write_page_reg(phydev, page, addr, new);

Please implement this more safely. Another user could jump in between
the read and the write and change this same register.

	phy_lock_mdio_bus(phydev);
	__phy_write(phydev, LAN_EXT_PAGE_ACCESS_CONTROL, page);
	__phy_write(phydev, LAN_EXT_PAGE_ACCESS_ADDRESS_DATA, addr);
	__phy_write(phydev, LAN_EXT_PAGE_ACCESS_CONTROL,
		    (page | LAN_EXT_PAGE_ACCESS_CTRL_EP_FUNC));
	ret = __phy_modify_changed(phydev, LAN_EXT_PAGE_ACCESS_ADDRESS_DATA,
				   mask, set);
	if (ret < 0)
		phydev_err(phydev, "Error: phy_modify has returned error %d\n", 
			   ret);

unlock:
	phy_unlock_mdio_bus(phydev);

	return ret;

is all that it'll take (assuming the control/address register doesn't
need to be rewritten.)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 3/4] net: phy: micrel: Replace hardcoded pages with defines

[Russell King (Oracle)]

On Thu, Jul 24, 2025 at 10:08:25PM +0200, Horatiu Vultur wrote:
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index b04c471c11a4a..d20f028106b7d 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -2788,6 +2788,13 @@ static int ksz886x_cable_test_get_status(struct phy_device *phydev,
>  	return ret;
>  }
>  
> +#define LAN_EXT_PAGE_0					0
> +#define LAN_EXT_PAGE_1					1
> +#define LAN_EXT_PAGE_2					2
> +#define LAN_EXT_PAGE_4					4
> +#define LAN_EXT_PAGE_5					5
> +#define LAN_EXT_PAGE_31					31

I don't see the point of this change. This is almost as bad as:

#define ZERO 0
#define ONE 1
#define TWO 2
#define THREE 3
...
#define ONE_HUNDRED_AND_FIFTY_FIVE 155
etc

It doesn't give us any new information, and just adds extra clutter,
making the code less readable.

The point of using register definitions is to describe the purpose
of the number, giving the number a meaning, not to just hide the
number because we don't want to see such things in C code.

I'm sorry if you were asked to do this in v1, but I think if you
were asked to do it, it would've been assuming that the definitions
could be more meaningful.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net-next v2 2/4] net: phy: micrel: Introduce lanphy_modify_page_reg

[Horatiu Vultur]

The 07/24/2025 21:41, Russell King (Oracle) wrote:

Hi Russell,

> 
> On Thu, Jul 24, 2025 at 10:08:24PM +0200, Horatiu Vultur wrote:
> > +static int lanphy_modify_page_reg(struct phy_device *phydev, int page, u16 addr,
> > +                               u16 mask, u16 set)
> > +{
> > +     int new, ret;
> > +
> > +     ret = lanphy_read_page_reg(phydev, page, addr);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +     new = (ret & ~mask) | set;
> > +     if (new == ret)
> > +             return 0;
> > +
> > +     ret = lanphy_write_page_reg(phydev, page, addr, new);
> 
> Please implement this more safely. Another user could jump in between
> the read and the write and change this same register.
> 
>         phy_lock_mdio_bus(phydev);
>         __phy_write(phydev, LAN_EXT_PAGE_ACCESS_CONTROL, page);
>         __phy_write(phydev, LAN_EXT_PAGE_ACCESS_ADDRESS_DATA, addr);
>         __phy_write(phydev, LAN_EXT_PAGE_ACCESS_CONTROL,
>                     (page | LAN_EXT_PAGE_ACCESS_CTRL_EP_FUNC));
>         ret = __phy_modify_changed(phydev, LAN_EXT_PAGE_ACCESS_ADDRESS_DATA,
>                                    mask, set);
>         if (ret < 0)
>                 phydev_err(phydev, "Error: phy_modify has returned error %d\n",
>                            ret);
> 
> unlock:
>         phy_unlock_mdio_bus(phydev);
> 
>         return ret;
> 
> is all that it'll take (assuming the control/address register doesn't
> need to be rewritten.)

Thanks, I will look into this.

> 
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

-- 
/Horatiu

Re: [PATCH net-next v2 3/4] net: phy: micrel: Replace hardcoded pages with defines

[Horatiu Vultur]

The 07/24/2025 21:45, Russell King (Oracle) wrote:
> 
> On Thu, Jul 24, 2025 at 10:08:25PM +0200, Horatiu Vultur wrote:
> > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> > index b04c471c11a4a..d20f028106b7d 100644
> > --- a/drivers/net/phy/micrel.c
> > +++ b/drivers/net/phy/micrel.c
> > @@ -2788,6 +2788,13 @@ static int ksz886x_cable_test_get_status(struct phy_device *phydev,
> >       return ret;
> >  }
> >
> > +#define LAN_EXT_PAGE_0                                       0
> > +#define LAN_EXT_PAGE_1                                       1
> > +#define LAN_EXT_PAGE_2                                       2
> > +#define LAN_EXT_PAGE_4                                       4
> > +#define LAN_EXT_PAGE_5                                       5
> > +#define LAN_EXT_PAGE_31                                      31

Hi Russell,

> 
> I don't see the point of this change. This is almost as bad as:
> 
> #define ZERO 0
> #define ONE 1
> #define TWO 2
> #define THREE 3
> ...
> #define ONE_HUNDRED_AND_FIFTY_FIVE 155
> etc
> 
> It doesn't give us any new information, and just adds extra clutter,
> making the code less readable.
> 
> The point of using register definitions is to describe the purpose
> of the number, giving the number a meaning, not to just hide the
> number because we don't want to see such things in C code.
> 
> I'm sorry if you were asked to do this in v1, but I think if you
> were asked to do it, it would've been assuming that the definitions
> could be more meaningful.

You are right, I have been ask to change this in version 1:
https://lkml.org/lkml/2025/7/23/672

I have mentioned it that the extended pages don't have any meaningfull
names also in the register description document. But Oleksij says he
will be fine with xxxx_EXT_PAGE_0, so maybe I have missunderstood Oleksij
in what he proposed.

> 
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

-- 
/Horatiu

Re: [PATCH net-next v2 3/4] net: phy: micrel: Replace hardcoded pages with defines

[Oleksij Rempel]

On Fri, Jul 25, 2025 at 08:48:39AM +0200, Horatiu Vultur wrote:
> The 07/24/2025 21:45, Russell King (Oracle) wrote:
> > 
> > On Thu, Jul 24, 2025 at 10:08:25PM +0200, Horatiu Vultur wrote:
> > > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> > > index b04c471c11a4a..d20f028106b7d 100644
> > > --- a/drivers/net/phy/micrel.c
> > > +++ b/drivers/net/phy/micrel.c
> > > @@ -2788,6 +2788,13 @@ static int ksz886x_cable_test_get_status(struct phy_device *phydev,
> > >       return ret;
> e >  }
> > >
> > > +#define LAN_EXT_PAGE_0                                       0
> > > +#define LAN_EXT_PAGE_1                                       1
> > > +#define LAN_EXT_PAGE_2                                       2
> > > +#define LAN_EXT_PAGE_4                                       4
> > > +#define LAN_EXT_PAGE_5                                       5
> > > +#define LAN_EXT_PAGE_31                                      31
> 
> Hi Russell,
> 
> > 
> > I don't see the point of this change. This is almost as bad as:
> > 
> > #define ZERO 0
> > #define ONE 1
> > #define TWO 2
> > #define THREE 3
> > ...
> > #define ONE_HUNDRED_AND_FIFTY_FIVE 155
> > etc
> > 
> > It doesn't give us any new information, and just adds extra clutter,
> > making the code less readable.
> > 
> > The point of using register definitions is to describe the purpose
> > of the number, giving the number a meaning, not to just hide the
> > number because we don't want to see such things in C code.
> > 
> > I'm sorry if you were asked to do this in v1, but I think if you
> > were asked to do it, it would've been assuming that the definitions
> > could be more meaningful.
> 
> You are right, I have been ask to change this in version 1:
> https://lkml.org/lkml/2025/7/23/672
> 
> I have mentioned it that the extended pages don't have any meaningfull
> names also in the register description document. But Oleksij says he
> will be fine with xxxx_EXT_PAGE_0, so maybe I have missunderstood Oleksij

Hi,

I requested these defines because it's much easier to search for a specific
define than for a raw number - especially when debugging or comparing with
datasheets. Even if the names are generic, it helps track down usage when
documentation becomes available or evolves.

To improve the situation, I reviewed the LAN8814 documentation and observed how
the existing driver and patches (for LAN8842) use these extended pages.
Based on that, I suggest following names:

Documented Extended Pages:

These are described in the LAN8814 documentation:

/**
 * LAN8814_PAGE_COMMON_REGS - Selects Extended Page 4.
 *
 * This page contains device-common registers that affect the entire chip.
 * It includes controls for chip-level resets, strap status, GPIO,
 * QSGMII, the shared 1588 PTP block, and the PVT monitor.
 */
#define LAN8814_PAGE_COMMON_REGS 4

/**
 * LAN8814_PAGE_PORT_REGS - Selects Extended Page 5.
 *
 * This page contains port-specific registers that must be accessed
 * on a per-port basis. It includes controls for port LEDs, QSGMII PCS,
 * rate adaptation FIFOs, and the per-port 1588 TSU block.
 */
#define LAN8814_PAGE_PORT_REGS 5

Undocumented Pages (based on driver and patch analysis):

These pages are not officially documented, but their use is visible in the
driver and LAN8842 patch:

/**
 * LAN8814_PAGE_AFE_PMA - Selects Extended Page 1.
 *
 * This page appears to control the Analog Front-End (AFE) and Physical
 * Medium Attachment (PMA) layers. It is used to access registers like
 * LAN8814_PD_CONTROLS and LAN8814_LINK_QUALITY.
 */
#define LAN8814_PAGE_AFE_PMA 1

/**
 * LAN8814_PAGE_PCS_DIGITAL - Selects Extended Page 2.
 *
 * This page seems dedicated to the Physical Coding Sublayer (PCS) and other
 * digital logic. It is used for MDI-X alignment (LAN8814_ALIGN_SWAP) and EEE
 * state (LAN8814_EEE_STATE) in the LAN8814, and is repurposed for statistics
 * and self-test counters in the LAN8842.
 */
#define LAN8814_PAGE_PCS_DIGITAL 2

/**
 * LAN8814_PAGE_SYSTEM_CTRL - Selects Extended Page 31.
 *
 * This page appears to hold fundamental system or global controls. In the
 * driver, it is used by the related LAN8804 to access the
 * LAN8814_CLOCK_MANAGEMENT register.
 */
#define LAN8814_PAGE_SYSTEM_CTRL 31

While these names are not official, they still give useful hints and make the
code more readable. I doubt the LAN8842 has an identical layout, but it looks
similar enough to reuse these patterns for now.

Are there any plans to make the LAN8842 register documentation public? That
would help clarify this further and improve upstream support.

Best regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH net-next v2 4/4] net: phy: micrel: Add support for lan8842

[ALOK TIWARI]

On 7/25/2025 1:38 AM, Horatiu Vultur wrote:
> +static void lan8842_get_phy_stats(struct phy_device *phydev,
> +				  struct ethtool_eth_phy_stats *eth_stats,
> +				  struct ethtool_phy_stats *stats)
> +{
> +	struct lan8842_priv *priv = phydev->priv;
> +
> +	stats->rx_packets = priv->phy_stats.rx_packets;
> +	stats->rx_errors = priv->phy_stats.rx_errors;
> +	stats->tx_packets = priv->phy_stats.tx_packets;
> +	stats->tx_errors = priv->phy_stats.rx_errors;

looks like a copy-paste mistake
stats->tx_errors = priv->phy_stats.tx_errors;

> +}
> +


Thanks,
Alok

Re: [PATCH net-next v2 4/4] net: phy: micrel: Add support for lan8842

[Horatiu Vultur]

The 07/26/2025 17:23, ALOK TIWARI wrote:

Hi Alok,

> 
> On 7/25/2025 1:38 AM, Horatiu Vultur wrote:
> > +static void lan8842_get_phy_stats(struct phy_device *phydev,
> > +                               struct ethtool_eth_phy_stats *eth_stats,
> > +                               struct ethtool_phy_stats *stats)
> > +{
> > +     struct lan8842_priv *priv = phydev->priv;
> > +
> > +     stats->rx_packets = priv->phy_stats.rx_packets;
> > +     stats->rx_errors = priv->phy_stats.rx_errors;
> > +     stats->tx_packets = priv->phy_stats.tx_packets;
> > +     stats->tx_errors = priv->phy_stats.rx_errors;
> 
> looks like a copy-paste mistake
> stats->tx_errors = priv->phy_stats.tx_errors;

That is a good catch!
I will fix it in the next version.

> 
> > +}
> > +
> 
> 
> Thanks,
> Alok

-- 
/Horatiu

Re: [PATCH net-next v2 3/4] net: phy: micrel: Replace hardcoded pages with defines

[Horatiu Vultur]

The 07/26/2025 08:21, Oleksij Rempel wrote:

Hi,

> 
> On Fri, Jul 25, 2025 at 08:48:39AM +0200, Horatiu Vultur wrote:
> > The 07/24/2025 21:45, Russell King (Oracle) wrote:
> > >
> > > On Thu, Jul 24, 2025 at 10:08:25PM +0200, Horatiu Vultur wrote:
> > > > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> > > > index b04c471c11a4a..d20f028106b7d 100644
> > > > --- a/drivers/net/phy/micrel.c
> > > > +++ b/drivers/net/phy/micrel.c
> > > > @@ -2788,6 +2788,13 @@ static int ksz886x_cable_test_get_status(struct phy_device *phydev,
> > > >       return ret;
> > e >  }
> > > >
> > > > +#define LAN_EXT_PAGE_0                                       0
> > > > +#define LAN_EXT_PAGE_1                                       1
> > > > +#define LAN_EXT_PAGE_2                                       2
> > > > +#define LAN_EXT_PAGE_4                                       4
> > > > +#define LAN_EXT_PAGE_5                                       5
> > > > +#define LAN_EXT_PAGE_31                                      31
> >
> > Hi Russell,
> >
> > >
> > > I don't see the point of this change. This is almost as bad as:
> > >
> > > #define ZERO 0
> > > #define ONE 1
> > > #define TWO 2
> > > #define THREE 3
> > > ...
> > > #define ONE_HUNDRED_AND_FIFTY_FIVE 155
> > > etc
> > >
> > > It doesn't give us any new information, and just adds extra clutter,
> > > making the code less readable.
> > >
> > > The point of using register definitions is to describe the purpose
> > > of the number, giving the number a meaning, not to just hide the
> > > number because we don't want to see such things in C code.
> > >
> > > I'm sorry if you were asked to do this in v1, but I think if you
> > > were asked to do it, it would've been assuming that the definitions
> > > could be more meaningful.
> >
> > You are right, I have been ask to change this in version 1:
> > https://lkml.org/lkml/2025/7/23/672
> >
> > I have mentioned it that the extended pages don't have any meaningfull
> > names also in the register description document. But Oleksij says he
> > will be fine with xxxx_EXT_PAGE_0, so maybe I have missunderstood Oleksij
> 
> Hi,
> 
> I requested these defines because it's much easier to search for a specific
> define than for a raw number - especially when debugging or comparing with
> datasheets. Even if the names are generic, it helps track down usage when
> documentation becomes available or evolves.
> 
> To improve the situation, I reviewed the LAN8814 documentation and observed how
> the existing driver and patches (for LAN8842) use these extended pages.
> Based on that, I suggest following names:
> 
> Documented Extended Pages:
> 
> These are described in the LAN8814 documentation:
> 
> /**
>  * LAN8814_PAGE_COMMON_REGS - Selects Extended Page 4.
>  *
>  * This page contains device-common registers that affect the entire chip.
>  * It includes controls for chip-level resets, strap status, GPIO,
>  * QSGMII, the shared 1588 PTP block, and the PVT monitor.
>  */
> #define LAN8814_PAGE_COMMON_REGS 4
> 
> /**
>  * LAN8814_PAGE_PORT_REGS - Selects Extended Page 5.
>  *
>  * This page contains port-specific registers that must be accessed
>  * on a per-port basis. It includes controls for port LEDs, QSGMII PCS,
>  * rate adaptation FIFOs, and the per-port 1588 TSU block.
>  */
> #define LAN8814_PAGE_PORT_REGS 5
> 
> Undocumented Pages (based on driver and patch analysis):
> 
> These pages are not officially documented, but their use is visible in the
> driver and LAN8842 patch:
> 
> /**
>  * LAN8814_PAGE_AFE_PMA - Selects Extended Page 1.
>  *
>  * This page appears to control the Analog Front-End (AFE) and Physical
>  * Medium Attachment (PMA) layers. It is used to access registers like
>  * LAN8814_PD_CONTROLS and LAN8814_LINK_QUALITY.
>  */
> #define LAN8814_PAGE_AFE_PMA 1
> 
> /**
>  * LAN8814_PAGE_PCS_DIGITAL - Selects Extended Page 2.
>  *
>  * This page seems dedicated to the Physical Coding Sublayer (PCS) and other
>  * digital logic. It is used for MDI-X alignment (LAN8814_ALIGN_SWAP) and EEE
>  * state (LAN8814_EEE_STATE) in the LAN8814, and is repurposed for statistics
>  * and self-test counters in the LAN8842.
>  */
> #define LAN8814_PAGE_PCS_DIGITAL 2
> 
> /**
>  * LAN8814_PAGE_SYSTEM_CTRL - Selects Extended Page 31.
>  *
>  * This page appears to hold fundamental system or global controls. In the
>  * driver, it is used by the related LAN8804 to access the
>  * LAN8814_CLOCK_MANAGEMENT register.
>  */
> #define LAN8814_PAGE_SYSTEM_CTRL 31
> 
> While these names are not official, they still give useful hints and make the
> code more readable. I doubt the LAN8842 has an identical layout, but it looks
> similar enough to reuse these patterns for now.

I think the name of the defines matches pretty good with the
functionality of those pages.

> 
> Are there any plans to make the LAN8842 register documentation public? That
> would help clarify this further and improve upstream support.

I don't know exactly but I have started to ask around when and if they will
make the lan8842 register description public.

> 
> Best regards,
> Oleksij
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

-- 
/Horatiu