[PATCH net-next 00/10] net: mdio: Start separating C22 and C45

[PATCH net-next 00/10] net: mdio: Start separating C22 and C45

[Andrew Lunn]

This patch set starts the separation of C22 and C45 MDIO bus
transactions at the API level to the MDIO Bus drivers. C45 read and
write ops are added to the MDIO bus driver structure, and the MDIO
core will try to use these ops if requested to perform a C45
transfer. If not available a fallback to the older API is made, to
allow backwards compatibility until all drivers are converted.

A few drivers are then converted to this new API,

Andrew Lunn (10):
  net: mdio: Add dedicated C45 API to MDIO bus drivers
  net: mdio: mdiobus_register: Update validation test
  net: mdio: C22 is now optional, EOPNOTSUPP if not provided
  net: mdio: Move mdiobus_c45_addr() next to users
  net: mdio: mdio-bitbang: Separate C22 and C45 transactions
  net: mdio: mvmdio: Convert XSMI bus to new API
  net: ethernet: freescale: xgmac: Separate C22 and C45 transactions for
    xgmac
  net: ethernet: freescale: fec: Separate C22 and C45 transactions for
    xgmac
  net: dsa: Separate C22 and C45 MDIO bus transaction methods
  net: dsa: mv88e6xxx: Separate C22 and C45 transactions

 drivers/net/dsa/mt7530.c                    |  92 +++++-----
 drivers/net/dsa/mt7530.h                    |  15 +-
 drivers/net/dsa/mv88e6xxx/chip.c            | 175 +++++++++++++-----
 drivers/net/dsa/mv88e6xxx/chip.h            |   7 +
 drivers/net/dsa/mv88e6xxx/global2.c         |  66 ++++---
 drivers/net/dsa/mv88e6xxx/global2.h         |  18 +-
 drivers/net/dsa/mv88e6xxx/phy.c             |  32 ++++
 drivers/net/dsa/mv88e6xxx/phy.h             |   4 +
 drivers/net/dsa/mv88e6xxx/serdes.c          |   8 +-
 drivers/net/ethernet/freescale/fec_main.c   | 149 ++++++++++-----
 drivers/net/ethernet/freescale/xgmac_mdio.c | 154 ++++++++++++----
 drivers/net/ethernet/marvell/mvmdio.c       |  24 +--
 drivers/net/ethernet/renesas/sh_eth.c       |  37 +++-
 drivers/net/mdio/mdio-bitbang.c             |  77 +++++---
 drivers/net/phy/mdio_bus.c                  | 193 +++++++++++++++++++-
 include/linux/mdio-bitbang.h                |   6 +-
 include/linux/mdio.h                        |  38 +---
 include/linux/phy.h                         |   5 +
 include/net/dsa.h                           |   4 +
 net/dsa/slave.c                             |  35 +++-
 20 files changed, 836 insertions(+), 303 deletions(-)

-- 
2.36.0

[PATCH net-next 01/10] net: mdio: Add dedicated C45 API to MDIO bus drivers

[Andrew Lunn]

Currently C22 and C45 transactions are mixed over a combined API calls
which make use of a special bit in the reg address to indicate if a
C45 transaction should be performed. This makes it impossible to know
if the bus driver actually supports C45. Additionally, many C22 only
drivers don't return -EOPNOTSUPP when asked to perform a C45
transaction, they mistaking perform a C22 transaction.

This is the first step to cleanly separate C22 from C45. To maintain
backwards compatibility until all drivers which are capable of
performing C45 are converted to this new API, the helper functions
will fall back to the older API if the new API is not
supported. Eventually this fallback will be removed.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/mdio_bus.c | 166 +++++++++++++++++++++++++++++++++++++
 include/linux/mdio.h       |  33 ++------
 include/linux/phy.h        |   5 ++
 3 files changed, 179 insertions(+), 25 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 58d602985877..46a03c0b45e3 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -826,6 +826,100 @@ int __mdiobus_modify_changed(struct mii_bus *bus, int addr, u32 regnum,
 }
 EXPORT_SYMBOL_GPL(__mdiobus_modify_changed);
 
+/**
+ * __mdiobus_c45_read - Unlocked version of the mdiobus_c45_read function
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @devad: device address to read
+ * @regnum: register number to read
+ *
+ * Read a MDIO bus register. Caller must hold the mdio bus lock.
+ *
+ * NOTE: MUST NOT be called from interrupt context.
+ */
+int __mdiobus_c45_read(struct mii_bus *bus, int addr, int devad, u32 regnum)
+{
+	int retval;
+
+	lockdep_assert_held_once(&bus->mdio_lock);
+
+	if (bus->read_c45)
+		retval = bus->read_c45(bus, addr, devad, regnum);
+	else
+		retval = bus->read(bus, addr, mdiobus_c45_addr(devad, regnum));
+
+	trace_mdio_access(bus, 1, addr, regnum, retval, retval);
+	mdiobus_stats_acct(&bus->stats[addr], true, retval);
+
+	return retval;
+}
+EXPORT_SYMBOL(__mdiobus_c45_read);
+
+/**
+ * __mdiobus_c45_write - Unlocked version of the mdiobus_write function
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @devad: device address to read
+ * @regnum: register number to write
+ * @val: value to write to @regnum
+ *
+ * Write a MDIO bus register. Caller must hold the mdio bus lock.
+ *
+ * NOTE: MUST NOT be called from interrupt context.
+ */
+int __mdiobus_c45_write(struct mii_bus *bus, int addr, int devad, u32 regnum,
+			u16 val)
+{
+	int err;
+
+	lockdep_assert_held_once(&bus->mdio_lock);
+
+	if (bus->write_c45)
+		err = bus->write_c45(bus, addr, devad, regnum, val);
+	else
+		err = bus->write(bus, addr, mdiobus_c45_addr(devad, regnum),
+				 val);
+
+	trace_mdio_access(bus, 0, addr, regnum, val, err);
+	mdiobus_stats_acct(&bus->stats[addr], false, err);
+
+	return err;
+}
+EXPORT_SYMBOL(__mdiobus_c45_write);
+
+/**
+ * __mdiobus_c45_modify_changed - Unlocked version of the mdiobus_modify function
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @devad: device address to read
+ * @regnum: register number to modify
+ * @mask: bit mask of bits to clear
+ * @set: bit mask of bits to set
+ *
+ * Read, modify, and if any change, write the register value back to the
+ * device. Any error returns a negative number.
+ *
+ * NOTE: MUST NOT be called from interrupt context.
+ */
+static int __mdiobus_c45_modify_changed(struct mii_bus *bus, int addr,
+					int devad, u32 regnum, u16 mask,
+					u16 set)
+{
+	int new, ret;
+
+	ret = __mdiobus_c45_read(bus, addr, devad, regnum);
+	if (ret < 0)
+		return ret;
+
+	new = (ret & ~mask) | set;
+	if (new == ret)
+		return 0;
+
+	ret = __mdiobus_c45_write(bus, addr, devad, regnum, new);
+
+	return ret < 0 ? ret : 1;
+}
+
 /**
  * mdiobus_read_nested - Nested version of the mdiobus_read function
  * @bus: the mii_bus struct
@@ -873,6 +967,29 @@ int mdiobus_read(struct mii_bus *bus, int addr, u32 regnum)
 }
 EXPORT_SYMBOL(mdiobus_read);
 
+/**
+ * mdiobus_c45_read - Convenience function for reading a given MII mgmt register
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @devad: device address to read
+ * @regnum: register number to read
+ *
+ * NOTE: MUST NOT be called from interrupt context,
+ * because the bus read/write functions may wait for an interrupt
+ * to conclude the operation.
+ */
+int mdiobus_c45_read(struct mii_bus *bus, int addr, int devad, u32 regnum)
+{
+	int retval;
+
+	mutex_lock(&bus->mdio_lock);
+	retval = __mdiobus_c45_read(bus, addr, devad, regnum);
+	mutex_unlock(&bus->mdio_lock);
+
+	return retval;
+}
+EXPORT_SYMBOL(mdiobus_c45_read);
+
 /**
  * mdiobus_write_nested - Nested version of the mdiobus_write function
  * @bus: the mii_bus struct
@@ -922,6 +1039,31 @@ int mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
 }
 EXPORT_SYMBOL(mdiobus_write);
 
+/**
+ * mdiobus_c45_write - Convenience function for writing a given MII mgmt register
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @devad: device address to read
+ * @regnum: register number to write
+ * @val: value to write to @regnum
+ *
+ * NOTE: MUST NOT be called from interrupt context,
+ * because the bus read/write functions may wait for an interrupt
+ * to conclude the operation.
+ */
+int mdiobus_c45_write(struct mii_bus *bus, int addr, int devad, u32 regnum,
+		      u16 val)
+{
+	int err;
+
+	mutex_lock(&bus->mdio_lock);
+	err = __mdiobus_c45_write(bus, addr, devad, regnum, val);
+	mutex_unlock(&bus->mdio_lock);
+
+	return err;
+}
+EXPORT_SYMBOL(mdiobus_c45_write);
+
 /**
  * mdiobus_modify - Convenience function for modifying a given mdio device
  *	register
@@ -943,6 +1085,30 @@ int mdiobus_modify(struct mii_bus *bus, int addr, u32 regnum, u16 mask, u16 set)
 }
 EXPORT_SYMBOL_GPL(mdiobus_modify);
 
+/**
+ * mdiobus_c45_modify - Convenience function for modifying a given mdio device
+ *	register
+ * @bus: the mii_bus struct
+ * @addr: the phy address
+ * @devad: device address to read
+ * @regnum: register number to write
+ * @mask: bit mask of bits to clear
+ * @set: bit mask of bits to set
+ */
+int mdiobus_c45_modify(struct mii_bus *bus, int addr, int devad, u32 regnum,
+		       u16 mask, u16 set)
+{
+	int err;
+
+	mutex_lock(&bus->mdio_lock);
+	err = __mdiobus_c45_modify_changed(bus, addr, devad, regnum,
+					   mask, set);
+	mutex_unlock(&bus->mdio_lock);
+
+	return err < 0 ? err : 0;
+}
+EXPORT_SYMBOL_GPL(mdiobus_c45_modify);
+
 /**
  * mdiobus_modify_changed - Convenience function for modifying a given mdio
  *	device register and returning if it changed
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index 00177567cfef..d89b0879692e 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -423,6 +423,14 @@ int mdiobus_modify(struct mii_bus *bus, int addr, u32 regnum, u16 mask,
 		   u16 set);
 int mdiobus_modify_changed(struct mii_bus *bus, int addr, u32 regnum,
 			   u16 mask, u16 set);
+int __mdiobus_c45_read(struct mii_bus *bus, int addr, int devad, u32 regnum);
+int mdiobus_c45_read(struct mii_bus *bus, int addr, int devad, u32 regnum);
+int __mdiobus_c45_write(struct mii_bus *bus, int addr,  int devad, u32 regnum,
+			u16 val);
+int mdiobus_c45_write(struct mii_bus *bus, int addr,  int devad, u32 regnum,
+		      u16 val);
+int mdiobus_c45_modify(struct mii_bus *bus, int addr, int devad, u32 regnum,
+		       u16 mask, u16 set);
 
 static inline int mdiodev_read(struct mdio_device *mdiodev, u32 regnum)
 {
@@ -463,31 +471,6 @@ static inline u16 mdiobus_c45_devad(u32 regnum)
 	return FIELD_GET(MII_DEVADDR_C45_MASK, regnum);
 }
 
-static inline int __mdiobus_c45_read(struct mii_bus *bus, int prtad, int devad,
-				     u16 regnum)
-{
-	return __mdiobus_read(bus, prtad, mdiobus_c45_addr(devad, regnum));
-}
-
-static inline int __mdiobus_c45_write(struct mii_bus *bus, int prtad, int devad,
-				      u16 regnum, u16 val)
-{
-	return __mdiobus_write(bus, prtad, mdiobus_c45_addr(devad, regnum),
-			       val);
-}
-
-static inline int mdiobus_c45_read(struct mii_bus *bus, int prtad, int devad,
-				   u16 regnum)
-{
-	return mdiobus_read(bus, prtad, mdiobus_c45_addr(devad, regnum));
-}
-
-static inline int mdiobus_c45_write(struct mii_bus *bus, int prtad, int devad,
-				    u16 regnum, u16 val)
-{
-	return mdiobus_write(bus, prtad, mdiobus_c45_addr(devad, regnum), val);
-}
-
 int mdiobus_register_device(struct mdio_device *mdiodev);
 int mdiobus_unregister_device(struct mdio_device *mdiodev);
 bool mdiobus_is_registered_device(struct mii_bus *bus, int addr);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 2d12054932ba..895b2bfae688 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -357,6 +357,11 @@ struct mii_bus {
 	int (*read)(struct mii_bus *bus, int addr, int regnum);
 	/** @write: Perform a write transfer on the bus */
 	int (*write)(struct mii_bus *bus, int addr, int regnum, u16 val);
+	/** @read: Perform a C45 read transfer on the bus */
+	int (*read_c45)(struct mii_bus *bus, int addr, int devnum, int regnum);
+	/** @write: Perform a C45 write transfer on the bus */
+	int (*write_c45)(struct mii_bus *bus, int addr, int devnum,
+			 int regnum, u16 val);
 	/** @reset: Perform a reset of the bus */
 	int (*reset)(struct mii_bus *bus);
 
-- 
2.36.0

[PATCH net-next 02/10] net: mdio: mdiobus_register: Update validation test

[Andrew Lunn]

Now that C45 uses its own read/write methods, the validation performed
when a bus is registers needs updating. All combinations of C22 and
C45 are supported, but both read and write methods must be provided,
read only busses are not supported etc.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/mdio_bus.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 46a03c0b45e3..818d22fb3cb5 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -526,8 +526,16 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
 	int i, err;
 	struct gpio_desc *gpiod;
 
-	if (NULL == bus || NULL == bus->name ||
-	    NULL == bus->read || NULL == bus->write)
+	if (NULL == bus || NULL == bus->name)
+		return -EINVAL;
+
+	if (!bus->read != !bus->write)
+		return -EINVAL;
+
+	if (!bus->read_c45 != !bus->write_c45)
+		return -EINVAL;
+
+	if (!bus->read && !bus->read_c45)
 		return -EINVAL;
 
 	if (bus->parent && bus->parent->of_node)
-- 
2.36.0

Re: [PATCH net-next 02/10] net: mdio: mdiobus_register: Update validation test

[Sergey Shtylyov]

Hello!

On 5/8/22 6:30 PM, Andrew Lunn wrote:

> Now that C45 uses its own read/write methods, the validation performed
> when a bus is registers needs updating. All combinations of C22 and
> C45 are supported, but both read and write methods must be provided,
> read only busses are not supported etc.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/phy/mdio_bus.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 46a03c0b45e3..818d22fb3cb5 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -526,8 +526,16 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
>  	int i, err;
>  	struct gpio_desc *gpiod;
>  
> -	if (NULL == bus || NULL == bus->name ||
> -	    NULL == bus->read || NULL == bus->write)
> +	if (NULL == bus || NULL == bus->name)

   I suggest (!bus || !bus->name) to be consistent with the code below.
   BTW, doesn't checkpatch.pl complain about NULL == bus?

> +		return -EINVAL;
> +
> +	if (!bus->read != !bus->write)
> +		return -EINVAL;
> +
> +	if (!bus->read_c45 != !bus->write_c45)
> +		return -EINVAL;

   Hm, that's complicated! :-)

> +
> +	if (!bus->read && !bus->read_c45)
>  		return -EINVAL;
>  
>  	if (bus->parent && bus->parent->of_node)

MBR, Sergey

[PATCH net-next 03/10] net: mdio: C22 is now optional, EOPNOTSUPP if not provided

[Andrew Lunn]

When performing a C22 operation, check that the bus driver actually
provides the methods, and return -EOPNOTSUPP if not. C45 only busses
do exist, and in future their C22 methods will be NULL.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/mdio_bus.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 818d22fb3cb5..4638ae3e13e3 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -767,7 +767,10 @@ int __mdiobus_read(struct mii_bus *bus, int addr, u32 regnum)
 
 	lockdep_assert_held_once(&bus->mdio_lock);
 
-	retval = bus->read(bus, addr, regnum);
+	if (bus->read)
+		retval = bus->read(bus, addr, regnum);
+	else
+		retval = -EOPNOTSUPP;
 
 	trace_mdio_access(bus, 1, addr, regnum, retval, retval);
 	mdiobus_stats_acct(&bus->stats[addr], true, retval);
@@ -793,7 +796,10 @@ int __mdiobus_write(struct mii_bus *bus, int addr, u32 regnum, u16 val)
 
 	lockdep_assert_held_once(&bus->mdio_lock);
 
-	err = bus->write(bus, addr, regnum, val);
+	if (bus->write)
+		err = bus->write(bus, addr, regnum, val);
+	else
+		err = -EOPNOTSUPP;
 
 	trace_mdio_access(bus, 0, addr, regnum, val, err);
 	mdiobus_stats_acct(&bus->stats[addr], false, err);
-- 
2.36.0

[PATCH net-next 04/10] net: mdio: Move mdiobus_c45_addr() next to users

[Andrew Lunn]

Now that mdiobus_c45_addr() is only used within the MDIO code during
fallback, move the function next to its only users. This function
should not be used any more in drivers, the c45 helpers should be used
in its place, so hiding it away will prevent any new users from being
added.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/phy/mdio_bus.c | 5 +++++
 include/linux/mdio.h       | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 4638ae3e13e3..46cc7b77620e 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -840,6 +840,11 @@ int __mdiobus_modify_changed(struct mii_bus *bus, int addr, u32 regnum,
 }
 EXPORT_SYMBOL_GPL(__mdiobus_modify_changed);
 
+static u32 mdiobus_c45_addr(int devad, u16 regnum)
+{
+	return MII_ADDR_C45 | devad << MII_DEVADDR_C45_SHIFT | regnum;
+}
+
 /**
  * __mdiobus_c45_read - Unlocked version of the mdiobus_c45_read function
  * @bus: the mii_bus struct
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index d89b0879692e..ed91e2329df7 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -456,11 +456,6 @@ static inline int mdiodev_modify_changed(struct mdio_device *mdiodev,
 				      mask, set);
 }
 
-static inline u32 mdiobus_c45_addr(int devad, u16 regnum)
-{
-	return MII_ADDR_C45 | devad << MII_DEVADDR_C45_SHIFT | regnum;
-}
-
 static inline u16 mdiobus_c45_regad(u32 regnum)
 {
 	return FIELD_GET(MII_REGADDR_C45_MASK, regnum);
-- 
2.36.0

[PATCH net-next 05/10] net: mdio: mdio-bitbang: Separate C22 and C45 transactions

[Andrew Lunn]

The bitbbanging bus driver can perform both C22 and C45 transfers.
Create separate functions for each and register the C45 versions using
the new driver API calls.

The SH Ethernet driver places wrappers around these functions. In
order to not break boards which might be using C45, add similar
wrappers for C45 operations.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/renesas/sh_eth.c | 37 ++++++++++---
 drivers/net/mdio/mdio-bitbang.c       | 77 ++++++++++++++++++---------
 include/linux/mdio-bitbang.h          |  6 ++-
 3 files changed, 87 insertions(+), 33 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 67ade78fb767..3d0b80144dc5 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -3042,23 +3042,46 @@ static int sh_mdio_release(struct sh_eth_private *mdp)
 	return 0;
 }
 
-static int sh_mdiobb_read(struct mii_bus *bus, int phy, int reg)
+static int sh_mdiobb_read_c22(struct mii_bus *bus, int phy, int reg)
 {
 	int res;
 
 	pm_runtime_get_sync(bus->parent);
-	res = mdiobb_read(bus, phy, reg);
+	res = mdiobb_read_c22(bus, phy, reg);
 	pm_runtime_put(bus->parent);
 
 	return res;
 }
 
-static int sh_mdiobb_write(struct mii_bus *bus, int phy, int reg, u16 val)
+static int sh_mdiobb_write_c22(struct mii_bus *bus, int phy, int reg, u16 val)
 {
 	int res;
 
 	pm_runtime_get_sync(bus->parent);
-	res = mdiobb_write(bus, phy, reg, val);
+	res = mdiobb_write_c22(bus, phy, reg, val);
+	pm_runtime_put(bus->parent);
+
+	return res;
+}
+
+static int sh_mdiobb_read_c45(struct mii_bus *bus, int phy, int devad, int reg)
+{
+	int res;
+
+	pm_runtime_get_sync(bus->parent);
+	res = mdiobb_read_c45(bus, phy, devad, reg);
+	pm_runtime_put(bus->parent);
+
+	return res;
+}
+
+static int sh_mdiobb_write_c45(struct mii_bus *bus, int phy, int devad,
+			       int reg, u16 val)
+{
+	int res;
+
+	pm_runtime_get_sync(bus->parent);
+	res = mdiobb_write_c45(bus, phy, devad, reg, val);
 	pm_runtime_put(bus->parent);
 
 	return res;
@@ -3089,8 +3112,10 @@ static int sh_mdio_init(struct sh_eth_private *mdp,
 		return -ENOMEM;
 
 	/* Wrap accessors with Runtime PM-aware ops */
-	mdp->mii_bus->read = sh_mdiobb_read;
-	mdp->mii_bus->write = sh_mdiobb_write;
+	mdp->mii_bus->read = sh_mdiobb_read_c22;
+	mdp->mii_bus->write = sh_mdiobb_write_c22;
+	mdp->mii_bus->read_c45 = sh_mdiobb_read_c45;
+	mdp->mii_bus->write_c45 = sh_mdiobb_write_c45;
 
 	/* Hook up MII support for ethtool */
 	mdp->mii_bus->name = "sh_mii";
diff --git a/drivers/net/mdio/mdio-bitbang.c b/drivers/net/mdio/mdio-bitbang.c
index 07609114a26b..b83932562be2 100644
--- a/drivers/net/mdio/mdio-bitbang.c
+++ b/drivers/net/mdio/mdio-bitbang.c
@@ -127,14 +127,12 @@ static void mdiobb_cmd(struct mdiobb_ctrl *ctrl, int op, u8 phy, u8 reg)
 
 /* In clause 45 mode all commands are prefixed by MDIO_ADDR to specify the
    lower 16 bits of the 21 bit address. This transfer is done identically to a
-   MDIO_WRITE except for a different code. To enable clause 45 mode or
-   MII_ADDR_C45 into the address. Theoretically clause 45 and normal devices
-   can exist on the same bus. Normal devices should ignore the MDIO_ADDR
+   MDIO_WRITE except for a different code. Theoretically clause 45 and normal
+   devices can exist on the same bus. Normal devices should ignore the MDIO_ADDR
    phase. */
-static int mdiobb_cmd_addr(struct mdiobb_ctrl *ctrl, int phy, u32 addr)
+static void mdiobb_cmd_addr(struct mdiobb_ctrl *ctrl, int phy, int dev_addr,
+			    int reg)
 {
-	unsigned int dev_addr = (addr >> 16) & 0x1F;
-	unsigned int reg = addr & 0xFFFF;
 	mdiobb_cmd(ctrl, MDIO_C45_ADDR, phy, dev_addr);
 
 	/* send the turnaround (10) */
@@ -145,21 +143,13 @@ static int mdiobb_cmd_addr(struct mdiobb_ctrl *ctrl, int phy, u32 addr)
 
 	ctrl->ops->set_mdio_dir(ctrl, 0);
 	mdiobb_get_bit(ctrl);
-
-	return dev_addr;
 }
 
-int mdiobb_read(struct mii_bus *bus, int phy, int reg)
+static int mdiobb_read_common(struct mii_bus *bus, int phy)
 {
 	struct mdiobb_ctrl *ctrl = bus->priv;
 	int ret, i;
 
-	if (reg & MII_ADDR_C45) {
-		reg = mdiobb_cmd_addr(ctrl, phy, reg);
-		mdiobb_cmd(ctrl, MDIO_C45_READ, phy, reg);
-	} else
-		mdiobb_cmd(ctrl, ctrl->op_c22_read, phy, reg);
-
 	ctrl->ops->set_mdio_dir(ctrl, 0);
 
 	/* check the turnaround bit: the PHY should be driving it to zero, if this
@@ -180,17 +170,31 @@ int mdiobb_read(struct mii_bus *bus, int phy, int reg)
 	mdiobb_get_bit(ctrl);
 	return ret;
 }
-EXPORT_SYMBOL(mdiobb_read);
 
-int mdiobb_write(struct mii_bus *bus, int phy, int reg, u16 val)
+int mdiobb_read_c22(struct mii_bus *bus, int phy, int reg)
 {
 	struct mdiobb_ctrl *ctrl = bus->priv;
 
-	if (reg & MII_ADDR_C45) {
-		reg = mdiobb_cmd_addr(ctrl, phy, reg);
-		mdiobb_cmd(ctrl, MDIO_C45_WRITE, phy, reg);
-	} else
-		mdiobb_cmd(ctrl, ctrl->op_c22_write, phy, reg);
+	mdiobb_cmd(ctrl, ctrl->op_c22_read, phy, reg);
+
+	return mdiobb_read_common(bus, phy);
+}
+EXPORT_SYMBOL(mdiobb_read_c22);
+
+int mdiobb_read_c45(struct mii_bus *bus, int phy, int devad, int reg)
+{
+	struct mdiobb_ctrl *ctrl = bus->priv;
+
+	mdiobb_cmd_addr(ctrl, phy, devad, reg);
+	mdiobb_cmd(ctrl, MDIO_C45_READ, phy, reg);
+
+	return mdiobb_read_common(bus, phy);
+}
+EXPORT_SYMBOL(mdiobb_read_c45);
+
+static int mdiobb_write_common(struct mii_bus *bus, u16 val)
+{
+	struct mdiobb_ctrl *ctrl = bus->priv;
 
 	/* send the turnaround (10) */
 	mdiobb_send_bit(ctrl, 1);
@@ -202,7 +206,27 @@ int mdiobb_write(struct mii_bus *bus, int phy, int reg, u16 val)
 	mdiobb_get_bit(ctrl);
 	return 0;
 }
-EXPORT_SYMBOL(mdiobb_write);
+
+int mdiobb_write_c22(struct mii_bus *bus, int phy, int reg, u16 val)
+{
+	struct mdiobb_ctrl *ctrl = bus->priv;
+
+	mdiobb_cmd(ctrl, ctrl->op_c22_write, phy, reg);
+
+	return mdiobb_write_common(bus, val);
+}
+EXPORT_SYMBOL(mdiobb_write_c22);
+
+int mdiobb_write_c45(struct mii_bus *bus, int phy, int devad, int reg, u16 val)
+{
+	struct mdiobb_ctrl *ctrl = bus->priv;
+
+	mdiobb_cmd_addr(ctrl, phy, devad, reg);
+	mdiobb_cmd(ctrl, MDIO_C45_WRITE, phy, reg);
+
+	return mdiobb_write_common(bus, val);
+}
+EXPORT_SYMBOL(mdiobb_write_c45);
 
 struct mii_bus *alloc_mdio_bitbang(struct mdiobb_ctrl *ctrl)
 {
@@ -214,8 +238,11 @@ struct mii_bus *alloc_mdio_bitbang(struct mdiobb_ctrl *ctrl)
 
 	__module_get(ctrl->ops->owner);
 
-	bus->read = mdiobb_read;
-	bus->write = mdiobb_write;
+	bus->read = mdiobb_read_c22;
+	bus->write = mdiobb_write_c22;
+	bus->read_c45 = mdiobb_read_c45;
+	bus->write_c45 = mdiobb_write_c45;
+
 	bus->priv = ctrl;
 	if (!ctrl->override_op_c22) {
 		ctrl->op_c22_read = MDIO_READ;
diff --git a/include/linux/mdio-bitbang.h b/include/linux/mdio-bitbang.h
index 373630fe5c28..cffabdbce075 100644
--- a/include/linux/mdio-bitbang.h
+++ b/include/linux/mdio-bitbang.h
@@ -38,8 +38,10 @@ struct mdiobb_ctrl {
 	u8 op_c22_write;
 };
 
-int mdiobb_read(struct mii_bus *bus, int phy, int reg);
-int mdiobb_write(struct mii_bus *bus, int phy, int reg, u16 val);
+int mdiobb_read_c22(struct mii_bus *bus, int phy, int reg);
+int mdiobb_write_c22(struct mii_bus *bus, int phy, int reg, u16 val);
+int mdiobb_read_c45(struct mii_bus *bus, int devad, int phy, int reg);
+int mdiobb_write_c45(struct mii_bus *bus, int devad, int phy, int reg, u16 val);
 
 /* The returned bus is not yet registered with the phy layer. */
 struct mii_bus *alloc_mdio_bitbang(struct mdiobb_ctrl *ctrl);
-- 
2.36.0

Re: [PATCH net-next 05/10] net: mdio: mdio-bitbang: Separate C22 and C45 transactions

[Geert Uytterhoeven]

On Sun, May 8, 2022 at 5:31 PM Andrew Lunn <andrew@lunn.ch> wrote:
> The bitbbanging bus driver can perform both C22 and C45 transfers.
> Create separate functions for each and register the C45 versions using
> the new driver API calls.
>
> The SH Ethernet driver places wrappers around these functions. In
> order to not break boards which might be using C45, add similar
> wrappers for C45 operations.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/ethernet/renesas/sh_eth.c | 37 ++++++++++---

This part LGTM, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH net-next 06/10] net: mdio: mvmdio: Convert XSMI bus to new API

[Andrew Lunn]

The marvell MDIO driver supports two different hardware blocks. The
XSMI block is C45 only. Convert this block to the new API, and only
populate the c45 calls in the bus structure.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/marvell/mvmdio.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index ef878973b859..2d654a40af13 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -204,21 +204,17 @@ static const struct orion_mdio_ops orion_mdio_xsmi_ops = {
 	.poll_interval_max = MVMDIO_XSMI_POLL_INTERVAL_MAX,
 };
 
-static int orion_mdio_xsmi_read(struct mii_bus *bus, int mii_id,
-				int regnum)
+static int orion_mdio_xsmi_read_c45(struct mii_bus *bus, int mii_id,
+				    int dev_addr, int regnum)
 {
 	struct orion_mdio_dev *dev = bus->priv;
-	u16 dev_addr = (regnum >> 16) & GENMASK(4, 0);
 	int ret;
 
-	if (!(regnum & MII_ADDR_C45))
-		return -EOPNOTSUPP;
-
 	ret = orion_mdio_wait_ready(&orion_mdio_xsmi_ops, bus);
 	if (ret < 0)
 		return ret;
 
-	writel(regnum & GENMASK(15, 0), dev->regs + MVMDIO_XSMI_ADDR_REG);
+	writel(regnum, dev->regs + MVMDIO_XSMI_ADDR_REG);
 	writel((mii_id << MVMDIO_XSMI_PHYADDR_SHIFT) |
 	       (dev_addr << MVMDIO_XSMI_DEVADDR_SHIFT) |
 	       MVMDIO_XSMI_READ_OPERATION,
@@ -237,21 +233,17 @@ static int orion_mdio_xsmi_read(struct mii_bus *bus, int mii_id,
 	return readl(dev->regs + MVMDIO_XSMI_MGNT_REG) & GENMASK(15, 0);
 }
 
-static int orion_mdio_xsmi_write(struct mii_bus *bus, int mii_id,
-				int regnum, u16 value)
+static int orion_mdio_xsmi_write_c45(struct mii_bus *bus, int mii_id,
+				     int dev_addr, int regnum, u16 value)
 {
 	struct orion_mdio_dev *dev = bus->priv;
-	u16 dev_addr = (regnum >> 16) & GENMASK(4, 0);
 	int ret;
 
-	if (!(regnum & MII_ADDR_C45))
-		return -EOPNOTSUPP;
-
 	ret = orion_mdio_wait_ready(&orion_mdio_xsmi_ops, bus);
 	if (ret < 0)
 		return ret;
 
-	writel(regnum & GENMASK(15, 0), dev->regs + MVMDIO_XSMI_ADDR_REG);
+	writel(regnum, dev->regs + MVMDIO_XSMI_ADDR_REG);
 	writel((mii_id << MVMDIO_XSMI_PHYADDR_SHIFT) |
 	       (dev_addr << MVMDIO_XSMI_DEVADDR_SHIFT) |
 	       MVMDIO_XSMI_WRITE_OPERATION | value,
@@ -302,8 +294,8 @@ static int orion_mdio_probe(struct platform_device *pdev)
 		bus->write = orion_mdio_smi_write;
 		break;
 	case BUS_TYPE_XSMI:
-		bus->read = orion_mdio_xsmi_read;
-		bus->write = orion_mdio_xsmi_write;
+		bus->read_c45 = orion_mdio_xsmi_read_c45;
+		bus->write_c45 = orion_mdio_xsmi_write_c45;
 		break;
 	}
 
-- 
2.36.0

[PATCH net-next 07/10] net: ethernet: freescale: xgmac: Separate C22 and C45 transactions for xgmac

[Andrew Lunn]

The xgmac MDIO bus driver can perform both C22 and C45 transfers.
Create separate functions for each and register the C45 versions using
the new API calls where appropriate.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/freescale/xgmac_mdio.c | 154 +++++++++++++++-----
 1 file changed, 117 insertions(+), 37 deletions(-)

diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
index ec90da1de030..ddfe6bf1f231 100644
--- a/drivers/net/ethernet/freescale/xgmac_mdio.c
+++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
@@ -128,30 +128,59 @@ static int xgmac_wait_until_done(struct device *dev,
 	return 0;
 }
 
-/*
- * Write value to the PHY for this device to the register at regnum,waiting
+/* Write value to the PHY for this device to the register at regnum,waiting
  * until the write is done before it returns.  All PHY configuration has to be
  * done through the TSEC1 MIIM regs.
  */
-static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 value)
+static int xgmac_mdio_write_c22(struct mii_bus *bus, int phy_id, int regnum,
+				u16 value)
 {
 	struct mdio_fsl_priv *priv = (struct mdio_fsl_priv *)bus->priv;
 	struct tgec_mdio_controller __iomem *regs = priv->mdio_base;
-	uint16_t dev_addr;
+	bool endian = priv->is_little_endian;
 	u32 mdio_ctl, mdio_stat;
+	u16 dev_addr;
 	int ret;
+
+	mdio_stat = xgmac_read32(&regs->mdio_stat, endian);
+	dev_addr = regnum & 0x1f;
+	mdio_stat &= ~MDIO_STAT_ENC;
+
+	xgmac_write32(mdio_stat, &regs->mdio_stat, endian);
+
+	ret = xgmac_wait_until_free(&bus->dev, regs, endian);
+	if (ret)
+		return ret;
+
+	/* Set the port and dev addr */
+	mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
+	xgmac_write32(mdio_ctl, &regs->mdio_ctl, endian);
+
+	/* Write the value to the register */
+	xgmac_write32(MDIO_DATA(value), &regs->mdio_data, endian);
+
+	ret = xgmac_wait_until_done(&bus->dev, regs, endian);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/* Write value to the PHY for this device to the register at regnum,waiting
+ * until the write is done before it returns.  All PHY configuration has to be
+ * done through the TSEC1 MIIM regs.
+ */
+static int xgmac_mdio_write_c45(struct mii_bus *bus, int phy_id, int dev_addr,
+				int regnum, u16 value)
+{
+	struct mdio_fsl_priv *priv = (struct mdio_fsl_priv *)bus->priv;
+	struct tgec_mdio_controller __iomem *regs = priv->mdio_base;
 	bool endian = priv->is_little_endian;
+	u32 mdio_ctl, mdio_stat;
+	int ret;
 
 	mdio_stat = xgmac_read32(&regs->mdio_stat, endian);
-	if (regnum & MII_ADDR_C45) {
-		/* Clause 45 (ie 10G) */
-		dev_addr = (regnum >> 16) & 0x1f;
-		mdio_stat |= MDIO_STAT_ENC;
-	} else {
-		/* Clause 22 (ie 1G) */
-		dev_addr = regnum & 0x1f;
-		mdio_stat &= ~MDIO_STAT_ENC;
-	}
+	mdio_stat |= MDIO_STAT_ENC;
 
 	xgmac_write32(mdio_stat, &regs->mdio_stat, endian);
 
@@ -164,13 +193,11 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
 	xgmac_write32(mdio_ctl, &regs->mdio_ctl, endian);
 
 	/* Set the register address */
-	if (regnum & MII_ADDR_C45) {
-		xgmac_write32(regnum & 0xffff, &regs->mdio_addr, endian);
+	xgmac_write32(regnum & 0xffff, &regs->mdio_addr, endian);
 
-		ret = xgmac_wait_until_free(&bus->dev, regs, endian);
-		if (ret)
-			return ret;
-	}
+	ret = xgmac_wait_until_free(&bus->dev, regs, endian);
+	if (ret)
+		return ret;
 
 	/* Write the value to the register */
 	xgmac_write32(MDIO_DATA(value), &regs->mdio_data, endian);
@@ -182,31 +209,84 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
 	return 0;
 }
 
-/*
- * Reads from register regnum in the PHY for device dev, returning the value.
+/* Reads from register regnum in the PHY for device dev, returning the value.
  * Clears miimcom first.  All PHY configuration has to be done through the
  * TSEC1 MIIM regs.
  */
-static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
+static int xgmac_mdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
 {
 	struct mdio_fsl_priv *priv = (struct mdio_fsl_priv *)bus->priv;
 	struct tgec_mdio_controller __iomem *regs = priv->mdio_base;
+	bool endian = priv->is_little_endian;
 	unsigned long flags;
-	uint16_t dev_addr;
 	uint32_t mdio_stat;
 	uint32_t mdio_ctl;
+	u16 dev_addr;
 	int ret;
-	bool endian = priv->is_little_endian;
 
 	mdio_stat = xgmac_read32(&regs->mdio_stat, endian);
-	if (regnum & MII_ADDR_C45) {
-		dev_addr = (regnum >> 16) & 0x1f;
-		mdio_stat |= MDIO_STAT_ENC;
+	dev_addr = regnum & 0x1f;
+	mdio_stat &= ~MDIO_STAT_ENC;
+
+	xgmac_write32(mdio_stat, &regs->mdio_stat, endian);
+
+	ret = xgmac_wait_until_free(&bus->dev, regs, endian);
+	if (ret)
+		return ret;
+
+	/* Set the Port and Device Addrs */
+	mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
+	xgmac_write32(mdio_ctl, &regs->mdio_ctl, endian);
+
+	if (priv->has_a009885)
+		/* Once the operation completes, i.e. MDIO_STAT_BSY clears, we
+		 * must read back the data register within 16 MDC cycles.
+		 */
+		local_irq_save(flags);
+
+	/* Initiate the read */
+	xgmac_write32(mdio_ctl | MDIO_CTL_READ, &regs->mdio_ctl, endian);
+
+	ret = xgmac_wait_until_done(&bus->dev, regs, endian);
+	if (ret)
+		goto irq_restore;
+
+	/* Return all Fs if nothing was there */
+	if ((xgmac_read32(&regs->mdio_stat, endian) & MDIO_STAT_RD_ER) &&
+	    !priv->has_a011043) {
+		dev_dbg(&bus->dev,
+			"Error while reading PHY%d reg at %d.%d\n",
+			phy_id, dev_addr, regnum);
+		ret = 0xffff;
 	} else {
-		dev_addr = regnum & 0x1f;
-		mdio_stat &= ~MDIO_STAT_ENC;
+		ret = xgmac_read32(&regs->mdio_data, endian) & 0xffff;
+		dev_dbg(&bus->dev, "read %04x\n", ret);
 	}
 
+irq_restore:
+	if (priv->has_a009885)
+		local_irq_restore(flags);
+
+	return ret;
+}
+
+/* Reads from register regnum in the PHY for device dev, returning the value.
+ * Clears miimcom first.  All PHY configuration has to be done through the
+ * TSEC1 MIIM regs.
+ */
+static int xgmac_mdio_read_c45(struct mii_bus *bus, int phy_id, int dev_addr,
+			       int regnum)
+{
+	struct mdio_fsl_priv *priv = (struct mdio_fsl_priv *)bus->priv;
+	struct tgec_mdio_controller __iomem *regs = priv->mdio_base;
+	bool endian = priv->is_little_endian;
+	u32 mdio_stat, mdio_ctl;
+	unsigned long flags;
+	int ret;
+
+	mdio_stat = xgmac_read32(&regs->mdio_stat, endian);
+	mdio_stat |= MDIO_STAT_ENC;
+
 	xgmac_write32(mdio_stat, &regs->mdio_stat, endian);
 
 	ret = xgmac_wait_until_free(&bus->dev, regs, endian);
@@ -218,13 +298,11 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
 	xgmac_write32(mdio_ctl, &regs->mdio_ctl, endian);
 
 	/* Set the register address */
-	if (regnum & MII_ADDR_C45) {
-		xgmac_write32(regnum & 0xffff, &regs->mdio_addr, endian);
+	xgmac_write32(regnum & 0xffff, &regs->mdio_addr, endian);
 
-		ret = xgmac_wait_until_free(&bus->dev, regs, endian);
-		if (ret)
-			return ret;
-	}
+	ret = xgmac_wait_until_free(&bus->dev, regs, endian);
+	if (ret)
+		return ret;
 
 	if (priv->has_a009885)
 		/* Once the operation completes, i.e. MDIO_STAT_BSY clears, we
@@ -326,8 +404,10 @@ static int xgmac_mdio_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	bus->name = "Freescale XGMAC MDIO Bus";
-	bus->read = xgmac_mdio_read;
-	bus->write = xgmac_mdio_write;
+	bus->read = xgmac_mdio_read_c22;
+	bus->write = xgmac_mdio_write_c22;
+	bus->read_c45 = xgmac_mdio_read_c45;
+	bus->write_c45 = xgmac_mdio_write_c45;
 	bus->parent = &pdev->dev;
 	bus->probe_capabilities = MDIOBUS_C22_C45;
 	snprintf(bus->id, MII_BUS_ID_SIZE, "%pa", &res->start);
-- 
2.36.0

Re: [PATCH net-next 07/10] net: ethernet: freescale: xgmac: Separate C22 and C45 transactions for xgmac

[Vladimir Oltean]

On Sun, May 08, 2022 at 05:30:46PM +0200, Andrew Lunn wrote:
> The xgmac MDIO bus driver can perform both C22 and C45 transfers.
> Create separate functions for each and register the C45 versions using
> the new API calls where appropriate.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/ethernet/freescale/xgmac_mdio.c | 154 +++++++++++++++-----
>  1 file changed, 117 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
> index ec90da1de030..ddfe6bf1f231 100644
> --- a/drivers/net/ethernet/freescale/xgmac_mdio.c
> +++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
> @@ -128,30 +128,59 @@ static int xgmac_wait_until_done(struct device *dev,
>  	return 0;
>  }
>  
> -/*
> - * Write value to the PHY for this device to the register at regnum,waiting
> +/* Write value to the PHY for this device to the register at regnum,waiting
>   * until the write is done before it returns.  All PHY configuration has to be
>   * done through the TSEC1 MIIM regs.
>   */
> -static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 value)
> +static int xgmac_mdio_write_c22(struct mii_bus *bus, int phy_id, int regnum,
> +				u16 value)
>  {
>  	struct mdio_fsl_priv *priv = (struct mdio_fsl_priv *)bus->priv;
>  	struct tgec_mdio_controller __iomem *regs = priv->mdio_base;
> -	uint16_t dev_addr;
> +	bool endian = priv->is_little_endian;
>  	u32 mdio_ctl, mdio_stat;
> +	u16 dev_addr;
>  	int ret;
> +
> +	mdio_stat = xgmac_read32(&regs->mdio_stat, endian);
> +	dev_addr = regnum & 0x1f;

Please move this either to the variable declaration, or near the mdio_ctl write,
or just integrate it into the macro argument.

> +	mdio_stat &= ~MDIO_STAT_ENC;
> +

You can remove this empty line during read-modify-write patterns.

> +	xgmac_write32(mdio_stat, &regs->mdio_stat, endian);
> +
> +	ret = xgmac_wait_until_free(&bus->dev, regs, endian);
> +	if (ret)
> +		return ret;
> +
> +	/* Set the port and dev addr */
> +	mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
> +	xgmac_write32(mdio_ctl, &regs->mdio_ctl, endian);
> +
> +	/* Write the value to the register */
> +	xgmac_write32(MDIO_DATA(value), &regs->mdio_data, endian);
> +
> +	ret = xgmac_wait_until_done(&bus->dev, regs, endian);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +/* Write value to the PHY for this device to the register at regnum,waiting
> + * until the write is done before it returns.  All PHY configuration has to be
> + * done through the TSEC1 MIIM regs.
> + */
> +static int xgmac_mdio_write_c45(struct mii_bus *bus, int phy_id, int dev_addr,
> +				int regnum, u16 value)
> +{
> +	struct mdio_fsl_priv *priv = (struct mdio_fsl_priv *)bus->priv;
> +	struct tgec_mdio_controller __iomem *regs = priv->mdio_base;
>  	bool endian = priv->is_little_endian;
> +	u32 mdio_ctl, mdio_stat;
> +	int ret;
>  
>  	mdio_stat = xgmac_read32(&regs->mdio_stat, endian);
> -	if (regnum & MII_ADDR_C45) {
> -		/* Clause 45 (ie 10G) */
> -		dev_addr = (regnum >> 16) & 0x1f;
> -		mdio_stat |= MDIO_STAT_ENC;
> -	} else {
> -		/* Clause 22 (ie 1G) */
> -		dev_addr = regnum & 0x1f;
> -		mdio_stat &= ~MDIO_STAT_ENC;
> -	}
> +	mdio_stat |= MDIO_STAT_ENC;
>  
>  	xgmac_write32(mdio_stat, &regs->mdio_stat, endian);
>  
> @@ -164,13 +193,11 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
>  	xgmac_write32(mdio_ctl, &regs->mdio_ctl, endian);
>  
>  	/* Set the register address */
> -	if (regnum & MII_ADDR_C45) {
> -		xgmac_write32(regnum & 0xffff, &regs->mdio_addr, endian);
> +	xgmac_write32(regnum & 0xffff, &regs->mdio_addr, endian);

Does regnum ever exceed 0xffff now that the MMD is no longer encoded into it?

>  
> -		ret = xgmac_wait_until_free(&bus->dev, regs, endian);
> -		if (ret)
> -			return ret;
> -	}
> +	ret = xgmac_wait_until_free(&bus->dev, regs, endian);
> +	if (ret)
> +		return ret;
>  
>  	/* Write the value to the register */
>  	xgmac_write32(MDIO_DATA(value), &regs->mdio_data, endian);
> @@ -182,31 +209,84 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
>  	return 0;
>  }
>  
> -/*
> - * Reads from register regnum in the PHY for device dev, returning the value.
> +/* Reads from register regnum in the PHY for device dev, returning the value.
>   * Clears miimcom first.  All PHY configuration has to be done through the
>   * TSEC1 MIIM regs.
>   */
> -static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
> +static int xgmac_mdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
>  {
>  	struct mdio_fsl_priv *priv = (struct mdio_fsl_priv *)bus->priv;
>  	struct tgec_mdio_controller __iomem *regs = priv->mdio_base;
> +	bool endian = priv->is_little_endian;
>  	unsigned long flags;
> -	uint16_t dev_addr;
>  	uint32_t mdio_stat;
>  	uint32_t mdio_ctl;
> +	u16 dev_addr;
>  	int ret;
> -	bool endian = priv->is_little_endian;
>  
>  	mdio_stat = xgmac_read32(&regs->mdio_stat, endian);
> -	if (regnum & MII_ADDR_C45) {
> -		dev_addr = (regnum >> 16) & 0x1f;
> -		mdio_stat |= MDIO_STAT_ENC;
> +	dev_addr = regnum & 0x1f;

I'm thinking we could just pass "regnum & 0x1f" (or just regnum) to
MDIO_CTL_DEV_ADDR() for the c22 functions.

> +	mdio_stat &= ~MDIO_STAT_ENC;
> +
> +	xgmac_write32(mdio_stat, &regs->mdio_stat, endian);
> +
> +	ret = xgmac_wait_until_free(&bus->dev, regs, endian);
> +	if (ret)
> +		return ret;
> +
> +	/* Set the Port and Device Addrs */
> +	mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
> +	xgmac_write32(mdio_ctl, &regs->mdio_ctl, endian);
> +
> +	if (priv->has_a009885)
> +		/* Once the operation completes, i.e. MDIO_STAT_BSY clears, we
> +		 * must read back the data register within 16 MDC cycles.
> +		 */
> +		local_irq_save(flags);
> +
> +	/* Initiate the read */
> +	xgmac_write32(mdio_ctl | MDIO_CTL_READ, &regs->mdio_ctl, endian);
> +
> +	ret = xgmac_wait_until_done(&bus->dev, regs, endian);
> +	if (ret)
> +		goto irq_restore;
> +
> +	/* Return all Fs if nothing was there */
> +	if ((xgmac_read32(&regs->mdio_stat, endian) & MDIO_STAT_RD_ER) &&
> +	    !priv->has_a011043) {
> +		dev_dbg(&bus->dev,
> +			"Error while reading PHY%d reg at %d.%d\n",
> +			phy_id, dev_addr, regnum);
> +		ret = 0xffff;
>  	} else {
> -		dev_addr = regnum & 0x1f;
> -		mdio_stat &= ~MDIO_STAT_ENC;
> +		ret = xgmac_read32(&regs->mdio_data, endian) & 0xffff;
> +		dev_dbg(&bus->dev, "read %04x\n", ret);
>  	}
>  
> +irq_restore:
> +	if (priv->has_a009885)
> +		local_irq_restore(flags);
> +
> +	return ret;
> +}
> +
> +/* Reads from register regnum in the PHY for device dev, returning the value.
> + * Clears miimcom first.  All PHY configuration has to be done through the
> + * TSEC1 MIIM regs.
> + */
> +static int xgmac_mdio_read_c45(struct mii_bus *bus, int phy_id, int dev_addr,
> +			       int regnum)
> +{
> +	struct mdio_fsl_priv *priv = (struct mdio_fsl_priv *)bus->priv;
> +	struct tgec_mdio_controller __iomem *regs = priv->mdio_base;
> +	bool endian = priv->is_little_endian;
> +	u32 mdio_stat, mdio_ctl;
> +	unsigned long flags;
> +	int ret;
> +
> +	mdio_stat = xgmac_read32(&regs->mdio_stat, endian);
> +	mdio_stat |= MDIO_STAT_ENC;
> +
>  	xgmac_write32(mdio_stat, &regs->mdio_stat, endian);
>  
>  	ret = xgmac_wait_until_free(&bus->dev, regs, endian);
> @@ -218,13 +298,11 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
>  	xgmac_write32(mdio_ctl, &regs->mdio_ctl, endian);
>  
>  	/* Set the register address */
> -	if (regnum & MII_ADDR_C45) {
> -		xgmac_write32(regnum & 0xffff, &regs->mdio_addr, endian);
> +	xgmac_write32(regnum & 0xffff, &regs->mdio_addr, endian);
>  
> -		ret = xgmac_wait_until_free(&bus->dev, regs, endian);
> -		if (ret)
> -			return ret;
> -	}
> +	ret = xgmac_wait_until_free(&bus->dev, regs, endian);
> +	if (ret)
> +		return ret;
>  
>  	if (priv->has_a009885)
>  		/* Once the operation completes, i.e. MDIO_STAT_BSY clears, we
> @@ -326,8 +404,10 @@ static int xgmac_mdio_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	bus->name = "Freescale XGMAC MDIO Bus";
> -	bus->read = xgmac_mdio_read;
> -	bus->write = xgmac_mdio_write;
> +	bus->read = xgmac_mdio_read_c22;
> +	bus->write = xgmac_mdio_write_c22;
> +	bus->read_c45 = xgmac_mdio_read_c45;
> +	bus->write_c45 = xgmac_mdio_write_c45;
>  	bus->parent = &pdev->dev;
>  	bus->probe_capabilities = MDIOBUS_C22_C45;
>  	snprintf(bus->id, MII_BUS_ID_SIZE, "%pa", &res->start);
> -- 
> 2.36.0
> 

Re: [PATCH net-next 07/10] net: ethernet: freescale: xgmac: Separate C22 and C45 transactions for xgmac

[Andrew Lunn]

On Tue, May 10, 2022 at 09:28:18PM +0300, Vladimir Oltean wrote:
> On Sun, May 08, 2022 at 05:30:46PM +0200, Andrew Lunn wrote:
> > The xgmac MDIO bus driver can perform both C22 and C45 transfers.
> > Create separate functions for each and register the C45 versions using
> > the new API calls where appropriate.
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >  drivers/net/ethernet/freescale/xgmac_mdio.c | 154 +++++++++++++++-----
> >  1 file changed, 117 insertions(+), 37 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
> > index ec90da1de030..ddfe6bf1f231 100644
> > --- a/drivers/net/ethernet/freescale/xgmac_mdio.c
> > +++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
> > @@ -128,30 +128,59 @@ static int xgmac_wait_until_done(struct device *dev,
> >  	return 0;
> >  }
> >  
> > -/*
> > - * Write value to the PHY for this device to the register at regnum,waiting
> > +/* Write value to the PHY for this device to the register at regnum,waiting
> >   * until the write is done before it returns.  All PHY configuration has to be
> >   * done through the TSEC1 MIIM regs.
> >   */
> > -static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 value)
> > +static int xgmac_mdio_write_c22(struct mii_bus *bus, int phy_id, int regnum,
> > +				u16 value)
> >  {
> >  	struct mdio_fsl_priv *priv = (struct mdio_fsl_priv *)bus->priv;
> >  	struct tgec_mdio_controller __iomem *regs = priv->mdio_base;
> > -	uint16_t dev_addr;
> > +	bool endian = priv->is_little_endian;
> >  	u32 mdio_ctl, mdio_stat;
> > +	u16 dev_addr;
> >  	int ret;
> > +
> > +	mdio_stat = xgmac_read32(&regs->mdio_stat, endian);
> > +	dev_addr = regnum & 0x1f;
> 
> Please move this either to the variable declaration, or near the mdio_ctl write,
> or just integrate it into the macro argument.

There are masks like this in some drivers, others don't. Since for the
majority of the MDIO bus drivers i don't have the hardware i was
trying to keep my changes to a minimum, so i'm less likely to break
it.

Once we have all the bus drivers converted, we can validate all the
requests in the core to guarantee no users are passing invalid values
to the drivers. And then all these masks can be removed.

> 
> > +	mdio_stat &= ~MDIO_STAT_ENC;
> > +
> 
> You can remove this empty line during read-modify-write patterns.

Sure, but just an FYI: the old code probably did it that way. My aim
is to split C22 from C45, not re-write/clean up every driver. I have
over 40 patches in total, without doing cleanups.

   Andrew

Re: [PATCH net-next 07/10] net: ethernet: freescale: xgmac: Separate C22 and C45 transactions for xgmac

[Vladimir Oltean]

On Tue, May 10, 2022 at 09:01:58PM +0200, Andrew Lunn wrote:
> On Tue, May 10, 2022 at 09:28:18PM +0300, Vladimir Oltean wrote:
> > On Sun, May 08, 2022 at 05:30:46PM +0200, Andrew Lunn wrote:
> > > The xgmac MDIO bus driver can perform both C22 and C45 transfers.
> > > Create separate functions for each and register the C45 versions using
> > > the new API calls where appropriate.
> > > 
> > > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > > ---
> > >  drivers/net/ethernet/freescale/xgmac_mdio.c | 154 +++++++++++++++-----
> > >  1 file changed, 117 insertions(+), 37 deletions(-)
> > > 
> > > diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
> > > index ec90da1de030..ddfe6bf1f231 100644
> > > --- a/drivers/net/ethernet/freescale/xgmac_mdio.c
> > > +++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
> > > @@ -128,30 +128,59 @@ static int xgmac_wait_until_done(struct device *dev,
> > >  	return 0;
> > >  }
> > >  
> > > -/*
> > > - * Write value to the PHY for this device to the register at regnum,waiting
> > > +/* Write value to the PHY for this device to the register at regnum,waiting
> > >   * until the write is done before it returns.  All PHY configuration has to be
> > >   * done through the TSEC1 MIIM regs.
> > >   */
> > > -static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 value)
> > > +static int xgmac_mdio_write_c22(struct mii_bus *bus, int phy_id, int regnum,
> > > +				u16 value)
> > >  {
> > >  	struct mdio_fsl_priv *priv = (struct mdio_fsl_priv *)bus->priv;
> > >  	struct tgec_mdio_controller __iomem *regs = priv->mdio_base;
> > > -	uint16_t dev_addr;
> > > +	bool endian = priv->is_little_endian;
> > >  	u32 mdio_ctl, mdio_stat;
> > > +	u16 dev_addr;
> > >  	int ret;
> > > +
> > > +	mdio_stat = xgmac_read32(&regs->mdio_stat, endian);
> > > +	dev_addr = regnum & 0x1f;
> > 
> > Please move this either to the variable declaration, or near the mdio_ctl write,
> > or just integrate it into the macro argument.
> 
> There are masks like this in some drivers, others don't. Since for the
> majority of the MDIO bus drivers i don't have the hardware i was
> trying to keep my changes to a minimum, so i'm less likely to break
> it.
> 
> Once we have all the bus drivers converted, we can validate all the
> requests in the core to guarantee no users are passing invalid values
> to the drivers. And then all these masks can be removed.

Sure, I was going to revisit this comment, keep the masking with 0x1f,
I remembered in the meanwhile that it's supposed to represent
MII_MMD_CTRL_DEVAD_MASK, and that there is other stuff potentially
encoded in the devad, like post-increment stuff.

> > 
> > > +	mdio_stat &= ~MDIO_STAT_ENC;
> > > +
> > 
> > You can remove this empty line during read-modify-write patterns.
> 
> Sure, but just an FYI: the old code probably did it that way. My aim
> is to split C22 from C45, not re-write/clean up every driver. I have
> over 40 patches in total, without doing cleanups.

You are modifying this part of the code anyway.

[PATCH net-next 08/10] net: ethernet: freescale: fec: Separate C22 and C45 transactions for xgmac

[Andrew Lunn]

The fec MDIO bus driver can perform both C22 and C45 transfers.
Create separate functions for each and register the C45 versions using
the new API calls where appropriate.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/ethernet/freescale/fec_main.c | 149 +++++++++++++++-------
 1 file changed, 101 insertions(+), 48 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 9f33ec838b52..6f749387b5c5 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1860,47 +1860,74 @@ static int fec_enet_mdio_wait(struct fec_enet_private *fep)
 	return ret;
 }
 
-static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
+static int fec_enet_mdio_read_c22(struct mii_bus *bus, int mii_id, int regnum)
 {
 	struct fec_enet_private *fep = bus->priv;
 	struct device *dev = &fep->pdev->dev;
 	int ret = 0, frame_start, frame_addr, frame_op;
-	bool is_c45 = !!(regnum & MII_ADDR_C45);
 
 	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
 		return ret;
 
-	if (is_c45) {
-		frame_start = FEC_MMFR_ST_C45;
+	/* C22 read */
+	frame_op = FEC_MMFR_OP_READ;
+	frame_start = FEC_MMFR_ST;
+	frame_addr = regnum;
 
-		/* write address */
-		frame_addr = (regnum >> 16);
-		writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
-		       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
-		       FEC_MMFR_TA | (regnum & 0xFFFF),
-		       fep->hwp + FEC_MII_DATA);
+	/* start a read op */
+	writel(frame_start | frame_op |
+		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
+		FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);
 
-		/* wait for end of transfer */
-		ret = fec_enet_mdio_wait(fep);
-		if (ret) {
-			netdev_err(fep->netdev, "MDIO address write timeout\n");
-			goto out;
-		}
+	/* wait for end of transfer */
+	ret = fec_enet_mdio_wait(fep);
+	if (ret) {
+		netdev_err(fep->netdev, "MDIO read timeout\n");
+		goto out;
+	}
 
-		frame_op = FEC_MMFR_OP_READ_C45;
+	ret = FEC_MMFR_DATA(readl(fep->hwp + FEC_MII_DATA));
 
-	} else {
-		/* C22 read */
-		frame_op = FEC_MMFR_OP_READ;
-		frame_start = FEC_MMFR_ST;
-		frame_addr = regnum;
+out:
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	return ret;
+}
+
+static int fec_enet_mdio_read_c45(struct mii_bus *bus, int mii_id,
+				  int devad, int regnum)
+{
+	struct fec_enet_private *fep = bus->priv;
+	struct device *dev = &fep->pdev->dev;
+	int ret = 0, frame_start, frame_op;
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0)
+		return ret;
+
+	frame_start = FEC_MMFR_ST_C45;
+
+	/* write address */
+	writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
+	       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) |
+	       FEC_MMFR_TA | (regnum & 0xFFFF),
+	       fep->hwp + FEC_MII_DATA);
+
+	/* wait for end of transfer */
+	ret = fec_enet_mdio_wait(fep);
+	if (ret) {
+		netdev_err(fep->netdev, "MDIO address write timeout\n");
+		goto out;
 	}
 
+	frame_op = FEC_MMFR_OP_READ_C45;
+
 	/* start a read op */
 	writel(frame_start | frame_op |
-		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
-		FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);
+	       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) |
+	       FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);
 
 	/* wait for end of transfer */
 	ret = fec_enet_mdio_wait(fep);
@@ -1918,43 +1945,67 @@ static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
 	return ret;
 }
 
-static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
-			   u16 value)
+static int fec_enet_mdio_write_c22(struct mii_bus *bus, int mii_id, int regnum,
+				   u16 value)
 {
 	struct fec_enet_private *fep = bus->priv;
 	struct device *dev = &fep->pdev->dev;
 	int ret, frame_start, frame_addr;
-	bool is_c45 = !!(regnum & MII_ADDR_C45);
 
 	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
 		return ret;
 
-	if (is_c45) {
-		frame_start = FEC_MMFR_ST_C45;
+	/* C22 write */
+	frame_start = FEC_MMFR_ST;
+	frame_addr = regnum;
 
-		/* write address */
-		frame_addr = (regnum >> 16);
-		writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
-		       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
-		       FEC_MMFR_TA | (regnum & 0xFFFF),
-		       fep->hwp + FEC_MII_DATA);
+	/* start a write op */
+	writel(frame_start | FEC_MMFR_OP_WRITE |
+		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
+		FEC_MMFR_TA | FEC_MMFR_DATA(value),
+		fep->hwp + FEC_MII_DATA);
 
-		/* wait for end of transfer */
-		ret = fec_enet_mdio_wait(fep);
-		if (ret) {
-			netdev_err(fep->netdev, "MDIO address write timeout\n");
-			goto out;
-		}
-	} else {
-		/* C22 write */
-		frame_start = FEC_MMFR_ST;
-		frame_addr = regnum;
+	/* wait for end of transfer */
+	ret = fec_enet_mdio_wait(fep);
+	if (ret)
+		netdev_err(fep->netdev, "MDIO write timeout\n");
+
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
+
+	return ret;
+}
+
+static int fec_enet_mdio_write_c45(struct mii_bus *bus, int mii_id,
+				   int devad, int regnum, u16 value)
+{
+	struct fec_enet_private *fep = bus->priv;
+	struct device *dev = &fep->pdev->dev;
+	int ret, frame_start;
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0)
+		return ret;
+
+	frame_start = FEC_MMFR_ST_C45;
+
+	/* write address */
+	writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
+	       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) |
+	       FEC_MMFR_TA | (regnum & 0xFFFF),
+	       fep->hwp + FEC_MII_DATA);
+
+	/* wait for end of transfer */
+	ret = fec_enet_mdio_wait(fep);
+	if (ret) {
+		netdev_err(fep->netdev, "MDIO address write timeout\n");
+		goto out;
 	}
 
 	/* start a write op */
 	writel(frame_start | FEC_MMFR_OP_WRITE |
-		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
+		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) |
 		FEC_MMFR_TA | FEC_MMFR_DATA(value),
 		fep->hwp + FEC_MII_DATA);
 
@@ -2254,8 +2305,10 @@ static int fec_enet_mii_init(struct platform_device *pdev)
 	}
 
 	fep->mii_bus->name = "fec_enet_mii_bus";
-	fep->mii_bus->read = fec_enet_mdio_read;
-	fep->mii_bus->write = fec_enet_mdio_write;
+	fep->mii_bus->read = fec_enet_mdio_read_c22;
+	fep->mii_bus->write = fec_enet_mdio_write_c22;
+	fep->mii_bus->read_c45 = fec_enet_mdio_read_c45;
+	fep->mii_bus->write_c45 = fec_enet_mdio_write_c45;
 	snprintf(fep->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
 		pdev->name, fep->dev_id + 1);
 	fep->mii_bus->priv = fep;
-- 
2.36.0

Re: [PATCH net-next 08/10] net: ethernet: freescale: fec: Separate C22 and C45 transactions for xgmac

[Vladimir Oltean]

On Sun, May 08, 2022 at 05:30:47PM +0200, Andrew Lunn wrote:
> The fec MDIO bus driver can perform both C22 and C45 transfers.
> Create separate functions for each and register the C45 versions using
> the new API calls where appropriate.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 149 +++++++++++++++-------
>  1 file changed, 101 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 9f33ec838b52..6f749387b5c5 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -1860,47 +1860,74 @@ static int fec_enet_mdio_wait(struct fec_enet_private *fep)
>  	return ret;
>  }
>  
> -static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
> +static int fec_enet_mdio_read_c22(struct mii_bus *bus, int mii_id, int regnum)
>  {
>  	struct fec_enet_private *fep = bus->priv;
>  	struct device *dev = &fep->pdev->dev;
>  	int ret = 0, frame_start, frame_addr, frame_op;
> -	bool is_c45 = !!(regnum & MII_ADDR_C45);
>  
>  	ret = pm_runtime_resume_and_get(dev);
>  	if (ret < 0)
>  		return ret;
>  
> -	if (is_c45) {
> -		frame_start = FEC_MMFR_ST_C45;
> +	/* C22 read */
> +	frame_op = FEC_MMFR_OP_READ;
> +	frame_start = FEC_MMFR_ST;
> +	frame_addr = regnum;
>  
> -		/* write address */
> -		frame_addr = (regnum >> 16);
> -		writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
> -		       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
> -		       FEC_MMFR_TA | (regnum & 0xFFFF),
> -		       fep->hwp + FEC_MII_DATA);
> +	/* start a read op */
> +	writel(frame_start | frame_op |
> +		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
> +		FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);

You lost the argument alignment to the open parenthesis.

>  
> -		/* wait for end of transfer */
> -		ret = fec_enet_mdio_wait(fep);
> -		if (ret) {
> -			netdev_err(fep->netdev, "MDIO address write timeout\n");
> -			goto out;
> -		}
> +	/* wait for end of transfer */
> +	ret = fec_enet_mdio_wait(fep);
> +	if (ret) {
> +		netdev_err(fep->netdev, "MDIO read timeout\n");
> +		goto out;
> +	}
>  
> -		frame_op = FEC_MMFR_OP_READ_C45;
> +	ret = FEC_MMFR_DATA(readl(fep->hwp + FEC_MII_DATA));
>  
> -	} else {
> -		/* C22 read */
> -		frame_op = FEC_MMFR_OP_READ;
> -		frame_start = FEC_MMFR_ST;
> -		frame_addr = regnum;
> +out:
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_put_autosuspend(dev);
> +
> +	return ret;
> +}
> +
> +static int fec_enet_mdio_read_c45(struct mii_bus *bus, int mii_id,
> +				  int devad, int regnum)
> +{
> +	struct fec_enet_private *fep = bus->priv;
> +	struct device *dev = &fep->pdev->dev;
> +	int ret = 0, frame_start, frame_op;
> +
> +	ret = pm_runtime_resume_and_get(dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	frame_start = FEC_MMFR_ST_C45;
> +
> +	/* write address */
> +	writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
> +	       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) |
> +	       FEC_MMFR_TA | (regnum & 0xFFFF),
> +	       fep->hwp + FEC_MII_DATA);
> +
> +	/* wait for end of transfer */
> +	ret = fec_enet_mdio_wait(fep);
> +	if (ret) {
> +		netdev_err(fep->netdev, "MDIO address write timeout\n");
> +		goto out;
>  	}
>  
> +	frame_op = FEC_MMFR_OP_READ_C45;
> +
>  	/* start a read op */
>  	writel(frame_start | frame_op |
> -		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
> -		FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);
> +	       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) |
> +	       FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);
>  
>  	/* wait for end of transfer */
>  	ret = fec_enet_mdio_wait(fep);
> @@ -1918,43 +1945,67 @@ static int fec_enet_mdio_read(struct mii_bus *bus, int mii_id, int regnum)
>  	return ret;
>  }
>  
> -static int fec_enet_mdio_write(struct mii_bus *bus, int mii_id, int regnum,
> -			   u16 value)
> +static int fec_enet_mdio_write_c22(struct mii_bus *bus, int mii_id, int regnum,
> +				   u16 value)
>  {
>  	struct fec_enet_private *fep = bus->priv;
>  	struct device *dev = &fep->pdev->dev;
>  	int ret, frame_start, frame_addr;
> -	bool is_c45 = !!(regnum & MII_ADDR_C45);
>  
>  	ret = pm_runtime_resume_and_get(dev);
>  	if (ret < 0)
>  		return ret;
>  
> -	if (is_c45) {
> -		frame_start = FEC_MMFR_ST_C45;
> +	/* C22 write */
> +	frame_start = FEC_MMFR_ST;
> +	frame_addr = regnum;
>  
> -		/* write address */
> -		frame_addr = (regnum >> 16);
> -		writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
> -		       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
> -		       FEC_MMFR_TA | (regnum & 0xFFFF),
> -		       fep->hwp + FEC_MII_DATA);
> +	/* start a write op */
> +	writel(frame_start | FEC_MMFR_OP_WRITE |
> +		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
> +		FEC_MMFR_TA | FEC_MMFR_DATA(value),
> +		fep->hwp + FEC_MII_DATA);

Here as well.

>  
> -		/* wait for end of transfer */
> -		ret = fec_enet_mdio_wait(fep);
> -		if (ret) {
> -			netdev_err(fep->netdev, "MDIO address write timeout\n");
> -			goto out;
> -		}
> -	} else {
> -		/* C22 write */
> -		frame_start = FEC_MMFR_ST;
> -		frame_addr = regnum;
> +	/* wait for end of transfer */
> +	ret = fec_enet_mdio_wait(fep);
> +	if (ret)
> +		netdev_err(fep->netdev, "MDIO write timeout\n");
> +
> +	pm_runtime_mark_last_busy(dev);
> +	pm_runtime_put_autosuspend(dev);
> +
> +	return ret;
> +}
> +
> +static int fec_enet_mdio_write_c45(struct mii_bus *bus, int mii_id,
> +				   int devad, int regnum, u16 value)
> +{
> +	struct fec_enet_private *fep = bus->priv;
> +	struct device *dev = &fep->pdev->dev;
> +	int ret, frame_start;
> +
> +	ret = pm_runtime_resume_and_get(dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	frame_start = FEC_MMFR_ST_C45;
> +
> +	/* write address */
> +	writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
> +	       FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) |
> +	       FEC_MMFR_TA | (regnum & 0xFFFF),
> +	       fep->hwp + FEC_MII_DATA);
> +
> +	/* wait for end of transfer */
> +	ret = fec_enet_mdio_wait(fep);
> +	if (ret) {
> +		netdev_err(fep->netdev, "MDIO address write timeout\n");
> +		goto out;
>  	}
>  
>  	/* start a write op */
>  	writel(frame_start | FEC_MMFR_OP_WRITE |
> -		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
> +		FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) |
>  		FEC_MMFR_TA | FEC_MMFR_DATA(value),
>  		fep->hwp + FEC_MII_DATA);
>  
> @@ -2254,8 +2305,10 @@ static int fec_enet_mii_init(struct platform_device *pdev)
>  	}
>  
>  	fep->mii_bus->name = "fec_enet_mii_bus";
> -	fep->mii_bus->read = fec_enet_mdio_read;
> -	fep->mii_bus->write = fec_enet_mdio_write;
> +	fep->mii_bus->read = fec_enet_mdio_read_c22;
> +	fep->mii_bus->write = fec_enet_mdio_write_c22;
> +	fep->mii_bus->read_c45 = fec_enet_mdio_read_c45;
> +	fep->mii_bus->write_c45 = fec_enet_mdio_write_c45;
>  	snprintf(fep->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
>  		pdev->name, fep->dev_id + 1);
>  	fep->mii_bus->priv = fep;
> -- 
> 2.36.0
> 

[PATCH net-next 09/10] net: dsa: Separate C22 and C45 MDIO bus transaction methods

[Andrew Lunn]

By adding _c45 function pointers to the dsa_switch_op structure, the
dsa core can register an MDIO bus with C45 accessors.

The dsa-loop driver could in theory provide such accessors, since it
just passed requests to the MDIO bus it is on, but it seems unlikely
to be useful at the moment. It can however be added later.

mt7530 does support C45, but its uses a mix of registering its MDIO
bus and using the DSA core provided bus. This makes the change a bit
more complex.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mt7530.c | 92 ++++++++++++++++++++++------------------
 drivers/net/dsa/mt7530.h | 15 +++++--
 include/net/dsa.h        |  4 ++
 net/dsa/slave.c          | 35 +++++++++++++--
 4 files changed, 96 insertions(+), 50 deletions(-)

diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 2b02d823d497..8121cb6342d3 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -605,17 +605,29 @@ mt7530_mib_reset(struct dsa_switch *ds)
 	mt7530_write(priv, MT7530_MIB_CCR, CCR_MIB_ACTIVATE);
 }
 
-static int mt7530_phy_read(struct mt7530_priv *priv, int port, int regnum)
+static int mt7530_phy_read_c22(struct mt7530_priv *priv, int port, int regnum)
 {
 	return mdiobus_read_nested(priv->bus, port, regnum);
 }
 
-static int mt7530_phy_write(struct mt7530_priv *priv, int port, int regnum,
-			    u16 val)
+static int mt7530_phy_write_c22(struct mt7530_priv *priv, int port, int regnum,
+				u16 val)
 {
 	return mdiobus_write_nested(priv->bus, port, regnum, val);
 }
 
+static int mt7530_phy_read_c45(struct mt7530_priv *priv, int port,
+			       int devad, int regnum)
+{
+	return mdiobus_c45_read_nested(priv->bus, port, devad, regnum);
+}
+
+static int mt7530_phy_write_c45(struct mt7530_priv *priv, int port, int devad,
+				int regnum, u16 val)
+{
+	return mdiobus_c45_write_nested(priv->bus, port, devad, regnum, val);
+}
+
 static int
 mt7531_ind_c45_phy_read(struct mt7530_priv *priv, int port, int devad,
 			int regnum)
@@ -667,7 +679,7 @@ mt7531_ind_c45_phy_read(struct mt7530_priv *priv, int port, int devad,
 
 static int
 mt7531_ind_c45_phy_write(struct mt7530_priv *priv, int port, int devad,
-			 int regnum, u32 data)
+			 int regnum, u16 data)
 {
 	struct mii_bus *bus = priv->bus;
 	struct mt7530_dummy_poll p;
@@ -790,55 +802,43 @@ mt7531_ind_c22_phy_write(struct mt7530_priv *priv, int port, int regnum,
 }
 
 static int
-mt7531_ind_phy_read(struct mt7530_priv *priv, int port, int regnum)
+mt753x_phy_read_c22(struct mii_bus *bus, int port, int regnum)
 {
-	int devad;
-	int ret;
-
-	if (regnum & MII_ADDR_C45) {
-		devad = (regnum >> MII_DEVADDR_C45_SHIFT) & 0x1f;
-		ret = mt7531_ind_c45_phy_read(priv, port, devad,
-					      regnum & MII_REGADDR_C45_MASK);
-	} else {
-		ret = mt7531_ind_c22_phy_read(priv, port, regnum);
-	}
+	struct mt7530_priv *priv = bus->priv;
 
-	return ret;
+	return priv->info->phy_read_c22(priv, port, regnum);
 }
 
 static int
-mt7531_ind_phy_write(struct mt7530_priv *priv, int port, int regnum,
-		     u16 data)
+mt753x_phy_read_c45(struct mii_bus *bus, int port, int devad, int regnum)
 {
-	int devad;
-	int ret;
+	struct mt7530_priv *priv = bus->priv;
 
-	if (regnum & MII_ADDR_C45) {
-		devad = (regnum >> MII_DEVADDR_C45_SHIFT) & 0x1f;
-		ret = mt7531_ind_c45_phy_write(priv, port, devad,
-					       regnum & MII_REGADDR_C45_MASK,
-					       data);
-	} else {
-		ret = mt7531_ind_c22_phy_write(priv, port, regnum, data);
-	}
+	if (priv->info->phy_read_c45)
+		return priv->info->phy_read_c45(priv, port, devad, regnum);
 
-	return ret;
+	return -EOPNOTSUPP;
 }
 
 static int
-mt753x_phy_read(struct mii_bus *bus, int port, int regnum)
+mt753x_phy_write_c22(struct mii_bus *bus, int port, int regnum, u16 val)
 {
 	struct mt7530_priv *priv = bus->priv;
 
-	return priv->info->phy_read(priv, port, regnum);
+	return priv->info->phy_write_c22(priv, port, regnum, val);
 }
 
 static int
-mt753x_phy_write(struct mii_bus *bus, int port, int regnum, u16 val)
+mt753x_phy_write_c45(struct mii_bus *bus, int port, int devad, int regnum,
+		     u16 val)
 {
 	struct mt7530_priv *priv = bus->priv;
 
-	return priv->info->phy_write(priv, port, regnum, val);
+	if (priv->info->phy_write_c45)
+		return priv->info->phy_write_c45(priv, port, devad, regnum,
+						 val);
+
+	return -EOPNOTSUPP;
 }
 
 static void
@@ -2076,8 +2076,10 @@ mt7530_setup_mdio(struct mt7530_priv *priv)
 	bus->priv = priv;
 	bus->name = KBUILD_MODNAME "-mii";
 	snprintf(bus->id, MII_BUS_ID_SIZE, KBUILD_MODNAME "-%d", idx++);
-	bus->read = mt753x_phy_read;
-	bus->write = mt753x_phy_write;
+	bus->read = mt753x_phy_read_c22;
+	bus->write = mt753x_phy_write_c22;
+	bus->read_c45 = mt753x_phy_read_c45;
+	bus->write_c45 = mt753x_phy_write_c45;
 	bus->parent = dev;
 	bus->phy_mask = ~ds->phys_mii_mask;
 
@@ -3130,8 +3132,10 @@ static const struct mt753x_info mt753x_table[] = {
 		.id = ID_MT7621,
 		.pcs_ops = &mt7530_pcs_ops,
 		.sw_setup = mt7530_setup,
-		.phy_read = mt7530_phy_read,
-		.phy_write = mt7530_phy_write,
+		.phy_read_c22 = mt7530_phy_read_c22,
+		.phy_write_c22 = mt7530_phy_write_c22,
+		.phy_read_c45 = mt7530_phy_read_c45,
+		.phy_write_c45 = mt7530_phy_write_c45,
 		.pad_setup = mt7530_pad_clk_setup,
 		.mac_port_get_caps = mt7530_mac_port_get_caps,
 		.mac_port_config = mt7530_mac_config,
@@ -3140,8 +3144,10 @@ static const struct mt753x_info mt753x_table[] = {
 		.id = ID_MT7530,
 		.pcs_ops = &mt7530_pcs_ops,
 		.sw_setup = mt7530_setup,
-		.phy_read = mt7530_phy_read,
-		.phy_write = mt7530_phy_write,
+		.phy_read_c22 = mt7530_phy_read_c22,
+		.phy_write_c22 = mt7530_phy_write_c22,
+		.phy_read_c45 = mt7530_phy_read_c45,
+		.phy_write_c45 = mt7530_phy_write_c45,
 		.pad_setup = mt7530_pad_clk_setup,
 		.mac_port_get_caps = mt7530_mac_port_get_caps,
 		.mac_port_config = mt7530_mac_config,
@@ -3150,8 +3156,10 @@ static const struct mt753x_info mt753x_table[] = {
 		.id = ID_MT7531,
 		.pcs_ops = &mt7531_pcs_ops,
 		.sw_setup = mt7531_setup,
-		.phy_read = mt7531_ind_phy_read,
-		.phy_write = mt7531_ind_phy_write,
+		.phy_read_c22 = mt7531_ind_c22_phy_read,
+		.phy_write_c22 = mt7531_ind_c22_phy_write,
+		.phy_read_c45 = mt7531_ind_c45_phy_read,
+		.phy_write_c45 = mt7531_ind_c45_phy_write,
 		.pad_setup = mt7531_pad_setup,
 		.cpu_port_config = mt7531_cpu_port_config,
 		.mac_port_get_caps = mt7531_mac_port_get_caps,
@@ -3211,7 +3219,7 @@ mt7530_probe(struct mdio_device *mdiodev)
 	 * properly.
 	 */
 	if (!priv->info->sw_setup || !priv->info->pad_setup ||
-	    !priv->info->phy_read || !priv->info->phy_write ||
+	    !priv->info->phy_read_c22 || !priv->info->phy_write_c22 ||
 	    !priv->info->mac_port_get_caps ||
 	    !priv->info->mac_port_config)
 		return -EINVAL;
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 71e36b69b96d..1b14146a1f08 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -750,8 +750,10 @@ struct mt753x_pcs {
 /* struct mt753x_info -	This is the main data structure for holding the specific
  *			part for each supported device
  * @sw_setup:		Holding the handler to a device initialization
- * @phy_read:		Holding the way reading PHY port
- * @phy_write:		Holding the way writing PHY port
+ * @phy_read_c22:	Holding the way reading PHY port using C22
+ * @phy_write_c22:	Holding the way writing PHY port using C22
+ * @phy_read_c45:	Holding the way reading PHY port using C45
+ * @phy_write_c45:	Holding the way writing PHY port using C45
  * @pad_setup:		Holding the way setting up the bus pad for a certain
  *			MAC port
  * @phy_mode_supported:	Check if the PHY type is being supported on a certain
@@ -767,8 +769,13 @@ struct mt753x_info {
 	const struct phylink_pcs_ops *pcs_ops;
 
 	int (*sw_setup)(struct dsa_switch *ds);
-	int (*phy_read)(struct mt7530_priv *priv, int port, int regnum);
-	int (*phy_write)(struct mt7530_priv *priv, int port, int regnum, u16 val);
+	int (*phy_read_c22)(struct mt7530_priv *priv, int port, int regnum);
+	int (*phy_write_c22)(struct mt7530_priv *priv, int port, int regnum,
+			     u16 val);
+	int (*phy_read_c45)(struct mt7530_priv *priv, int port, int devad,
+			    int regnum);
+	int (*phy_write_c45)(struct mt7530_priv *priv, int port, int devad,
+			     int regnum, u16 val);
 	int (*pad_setup)(struct dsa_switch *ds, phy_interface_t interface);
 	int (*cpu_port_config)(struct dsa_switch *ds, int port);
 	void (*mac_port_get_caps)(struct dsa_switch *ds, int port,
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 934958fda962..64e36eb33879 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -820,6 +820,10 @@ struct dsa_switch_ops {
 	int	(*phy_read)(struct dsa_switch *ds, int port, int regnum);
 	int	(*phy_write)(struct dsa_switch *ds, int port,
 			     int regnum, u16 val);
+	int	(*phy_read_c45)(struct dsa_switch *ds, int port, int devad,
+				int regnum);
+	int	(*phy_write_c45)(struct dsa_switch *ds, int port, int devad,
+				 int regnum, u16 val);
 
 	/*
 	 * Link state adjustment (called from libphy)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 5ee0aced9410..a8976a67a8c0 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -165,7 +165,7 @@ static int dsa_slave_unsync_mc(struct net_device *dev,
 }
 
 /* slave mii_bus handling ***************************************************/
-static int dsa_slave_phy_read(struct mii_bus *bus, int addr, int reg)
+static int dsa_slave_phy_read_c22(struct mii_bus *bus, int addr, int reg)
 {
 	struct dsa_switch *ds = bus->priv;
 
@@ -175,7 +175,19 @@ static int dsa_slave_phy_read(struct mii_bus *bus, int addr, int reg)
 	return 0xffff;
 }
 
-static int dsa_slave_phy_write(struct mii_bus *bus, int addr, int reg, u16 val)
+static int dsa_slave_phy_read_c45(struct mii_bus *bus, int addr, int devad,
+				  int reg)
+{
+	struct dsa_switch *ds = bus->priv;
+
+	if (ds->phys_mii_mask & (1 << addr))
+		return ds->ops->phy_read_c45(ds, addr, devad, reg);
+
+	return 0xffff;
+}
+
+static int dsa_slave_phy_write_c22(struct mii_bus *bus, int addr, int reg,
+				   u16 val)
 {
 	struct dsa_switch *ds = bus->priv;
 
@@ -185,12 +197,27 @@ static int dsa_slave_phy_write(struct mii_bus *bus, int addr, int reg, u16 val)
 	return 0;
 }
 
+static int dsa_slave_phy_write_c45(struct mii_bus *bus, int addr, int reg,
+				   int devad, u16 val)
+{
+	struct dsa_switch *ds = bus->priv;
+
+	if (ds->phys_mii_mask & (1 << addr))
+		return ds->ops->phy_write_c45(ds, addr, devad, reg, val);
+
+	return 0;
+}
+
 void dsa_slave_mii_bus_init(struct dsa_switch *ds)
 {
 	ds->slave_mii_bus->priv = (void *)ds;
 	ds->slave_mii_bus->name = "dsa slave smi";
-	ds->slave_mii_bus->read = dsa_slave_phy_read;
-	ds->slave_mii_bus->write = dsa_slave_phy_write;
+	ds->slave_mii_bus->read = dsa_slave_phy_read_c22;
+	ds->slave_mii_bus->write = dsa_slave_phy_write_c22;
+	if (ds->ops->phy_read_c45 && ds->ops->phy_write_c45) {
+		ds->slave_mii_bus->read_c45 = dsa_slave_phy_read_c45;
+		ds->slave_mii_bus->write_c45 = dsa_slave_phy_write_c45;
+	}
 	snprintf(ds->slave_mii_bus->id, MII_BUS_ID_SIZE, "dsa-%d.%d",
 		 ds->dst->index, ds->index);
 	ds->slave_mii_bus->parent = ds->dev;
-- 
2.36.0

Re: [PATCH net-next 09/10] net: dsa: Separate C22 and C45 MDIO bus transaction methods

[kernel test robot]

Hi Andrew,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Andrew-Lunn/net-mdio-Start-separating-C22-and-C45/20220508-233302
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8fc0b6992a06998404321f26a57ea54522659b64
config: parisc-randconfig-r023-20220508 (https://download.01.org/0day-ci/archive/20220509/202205090111.Nnl7VN9a-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/643c2f1541edcf0c2cb0f91e3e1981a8691894a0
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Andrew-Lunn/net-mdio-Start-separating-C22-and-C45/20220508-233302
        git checkout 643c2f1541edcf0c2cb0f91e3e1981a8691894a0
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=parisc SHELL=/bin/bash drivers/net/dsa/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/net/dsa/mt7530.c: In function 'mt7530_phy_read_c45':
>> drivers/net/dsa/mt7530.c:622:16: error: implicit declaration of function 'mdiobus_c45_read_nested'; did you mean 'mdiobus_read_nested'? [-Werror=implicit-function-declaration]
     622 |         return mdiobus_c45_read_nested(priv->bus, port, devad, regnum);
         |                ^~~~~~~~~~~~~~~~~~~~~~~
         |                mdiobus_read_nested
   drivers/net/dsa/mt7530.c: In function 'mt7530_phy_write_c45':
>> drivers/net/dsa/mt7530.c:628:16: error: implicit declaration of function 'mdiobus_c45_write_nested'; did you mean 'mdiobus_write_nested'? [-Werror=implicit-function-declaration]
     628 |         return mdiobus_c45_write_nested(priv->bus, port, devad, regnum, val);
         |                ^~~~~~~~~~~~~~~~~~~~~~~~
         |                mdiobus_write_nested
   cc1: some warnings being treated as errors


vim +622 drivers/net/dsa/mt7530.c

   618	
   619	static int mt7530_phy_read_c45(struct mt7530_priv *priv, int port,
   620				       int devad, int regnum)
   621	{
 > 622		return mdiobus_c45_read_nested(priv->bus, port, devad, regnum);
   623	}
   624	
   625	static int mt7530_phy_write_c45(struct mt7530_priv *priv, int port, int devad,
   626					int regnum, u16 val)
   627	{
 > 628		return mdiobus_c45_write_nested(priv->bus, port, devad, regnum, val);
   629	}
   630	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH net-next 09/10] net: dsa: Separate C22 and C45 MDIO bus transaction methods

[kernel test robot]

Hi Andrew,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Andrew-Lunn/net-mdio-Start-separating-C22-and-C45/20220508-233302
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8fc0b6992a06998404321f26a57ea54522659b64
config: hexagon-randconfig-r041-20220509 (https://download.01.org/0day-ci/archive/20220509/202205090359.Jw8AQHKW-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a385645b470e2d3a1534aae618ea56b31177639f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/643c2f1541edcf0c2cb0f91e3e1981a8691894a0
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Andrew-Lunn/net-mdio-Start-separating-C22-and-C45/20220508-233302
        git checkout 643c2f1541edcf0c2cb0f91e3e1981a8691894a0
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/net/dsa/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/net/dsa/mt7530.c:622:9: error: call to undeclared function 'mdiobus_c45_read_nested'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
           return mdiobus_c45_read_nested(priv->bus, port, devad, regnum);
                  ^
   drivers/net/dsa/mt7530.c:622:9: note: did you mean 'mdiobus_read_nested'?
   include/linux/mdio.h:419:5: note: 'mdiobus_read_nested' declared here
   int mdiobus_read_nested(struct mii_bus *bus, int addr, u32 regnum);
       ^
>> drivers/net/dsa/mt7530.c:628:9: error: call to undeclared function 'mdiobus_c45_write_nested'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
           return mdiobus_c45_write_nested(priv->bus, port, devad, regnum, val);
                  ^
   drivers/net/dsa/mt7530.c:628:9: note: did you mean 'mdiobus_write_nested'?
   include/linux/mdio.h:421:5: note: 'mdiobus_write_nested' declared here
   int mdiobus_write_nested(struct mii_bus *bus, int addr, u32 regnum, u16 val);
       ^
   2 errors generated.


vim +/mdiobus_c45_read_nested +622 drivers/net/dsa/mt7530.c

   618	
   619	static int mt7530_phy_read_c45(struct mt7530_priv *priv, int port,
   620				       int devad, int regnum)
   621	{
 > 622		return mdiobus_c45_read_nested(priv->bus, port, devad, regnum);
   623	}
   624	
   625	static int mt7530_phy_write_c45(struct mt7530_priv *priv, int port, int devad,
   626					int regnum, u16 val)
   627	{
 > 628		return mdiobus_c45_write_nested(priv->bus, port, devad, regnum, val);
   629	}
   630	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH net-next 09/10] net: dsa: Separate C22 and C45 MDIO bus transaction methods

[Vladimir Oltean]

On Sun, May 08, 2022 at 05:30:48PM +0200, Andrew Lunn wrote:
> By adding _c45 function pointers to the dsa_switch_op structure, the
> dsa core can register an MDIO bus with C45 accessors.
> 
> The dsa-loop driver could in theory provide such accessors, since it
> just passed requests to the MDIO bus it is on, but it seems unlikely
> to be useful at the moment. It can however be added later.
> 
> mt7530 does support C45, but its uses a mix of registering its MDIO
> bus and using the DSA core provided bus. This makes the change a bit
> more complex.

mt7530/mt7531 don't request the DSA core to provide a bus (they don't
populate dsa_switch_ops :: phy_read). They just populate ds->slave_mii_bus
in order to have the non-OF based phy_connect.

> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  drivers/net/dsa/mt7530.c | 92 ++++++++++++++++++++++------------------
>  drivers/net/dsa/mt7530.h | 15 +++++--
>  include/net/dsa.h        |  4 ++
>  net/dsa/slave.c          | 35 +++++++++++++--
>  4 files changed, 96 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 2b02d823d497..8121cb6342d3 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -605,17 +605,29 @@ mt7530_mib_reset(struct dsa_switch *ds)
>  	mt7530_write(priv, MT7530_MIB_CCR, CCR_MIB_ACTIVATE);
>  }
>  
> -static int mt7530_phy_read(struct mt7530_priv *priv, int port, int regnum)
> +static int mt7530_phy_read_c22(struct mt7530_priv *priv, int port, int regnum)
>  {
>  	return mdiobus_read_nested(priv->bus, port, regnum);
>  }
>  
> -static int mt7530_phy_write(struct mt7530_priv *priv, int port, int regnum,
> -			    u16 val)
> +static int mt7530_phy_write_c22(struct mt7530_priv *priv, int port, int regnum,
> +				u16 val)
>  {
>  	return mdiobus_write_nested(priv->bus, port, regnum, val);
>  }
>  
> +static int mt7530_phy_read_c45(struct mt7530_priv *priv, int port,
> +			       int devad, int regnum)
> +{
> +	return mdiobus_c45_read_nested(priv->bus, port, devad, regnum);
> +}
> +
> +static int mt7530_phy_write_c45(struct mt7530_priv *priv, int port, int devad,
> +				int regnum, u16 val)
> +{
> +	return mdiobus_c45_write_nested(priv->bus, port, devad, regnum, val);
> +}
> +
>  static int
>  mt7531_ind_c45_phy_read(struct mt7530_priv *priv, int port, int devad,
>  			int regnum)
> @@ -667,7 +679,7 @@ mt7531_ind_c45_phy_read(struct mt7530_priv *priv, int port, int devad,
>  
>  static int
>  mt7531_ind_c45_phy_write(struct mt7530_priv *priv, int port, int devad,
> -			 int regnum, u32 data)
> +			 int regnum, u16 data)
>  {
>  	struct mii_bus *bus = priv->bus;
>  	struct mt7530_dummy_poll p;
> @@ -790,55 +802,43 @@ mt7531_ind_c22_phy_write(struct mt7530_priv *priv, int port, int regnum,
>  }
>  
>  static int
> -mt7531_ind_phy_read(struct mt7530_priv *priv, int port, int regnum)
> +mt753x_phy_read_c22(struct mii_bus *bus, int port, int regnum)
>  {
> -	int devad;
> -	int ret;
> -
> -	if (regnum & MII_ADDR_C45) {
> -		devad = (regnum >> MII_DEVADDR_C45_SHIFT) & 0x1f;
> -		ret = mt7531_ind_c45_phy_read(priv, port, devad,
> -					      regnum & MII_REGADDR_C45_MASK);
> -	} else {
> -		ret = mt7531_ind_c22_phy_read(priv, port, regnum);
> -	}
> +	struct mt7530_priv *priv = bus->priv;
>  
> -	return ret;
> +	return priv->info->phy_read_c22(priv, port, regnum);
>  }
>  
>  static int
> -mt7531_ind_phy_write(struct mt7530_priv *priv, int port, int regnum,
> -		     u16 data)
> +mt753x_phy_read_c45(struct mii_bus *bus, int port, int devad, int regnum)
>  {
> -	int devad;
> -	int ret;
> +	struct mt7530_priv *priv = bus->priv;
>  
> -	if (regnum & MII_ADDR_C45) {
> -		devad = (regnum >> MII_DEVADDR_C45_SHIFT) & 0x1f;
> -		ret = mt7531_ind_c45_phy_write(priv, port, devad,
> -					       regnum & MII_REGADDR_C45_MASK,
> -					       data);
> -	} else {
> -		ret = mt7531_ind_c22_phy_write(priv, port, regnum, data);
> -	}
> +	if (priv->info->phy_read_c45)
> +		return priv->info->phy_read_c45(priv, port, devad, regnum);

All switches provide phy_read_c45 and phy_write_c45, so the conditional
can be removed.

>  
> -	return ret;
> +	return -EOPNOTSUPP;
>  }
>  
>  static int
> -mt753x_phy_read(struct mii_bus *bus, int port, int regnum)
> +mt753x_phy_write_c22(struct mii_bus *bus, int port, int regnum, u16 val)
>  {
>  	struct mt7530_priv *priv = bus->priv;
>  
> -	return priv->info->phy_read(priv, port, regnum);
> +	return priv->info->phy_write_c22(priv, port, regnum, val);
>  }
>  
>  static int
> -mt753x_phy_write(struct mii_bus *bus, int port, int regnum, u16 val)
> +mt753x_phy_write_c45(struct mii_bus *bus, int port, int devad, int regnum,
> +		     u16 val)
>  {
>  	struct mt7530_priv *priv = bus->priv;
>  
> -	return priv->info->phy_write(priv, port, regnum, val);
> +	if (priv->info->phy_write_c45)
> +		return priv->info->phy_write_c45(priv, port, devad, regnum,
> +						 val);
> +
> +	return -EOPNOTSUPP;
>  }
>  
>  static void
> @@ -2076,8 +2076,10 @@ mt7530_setup_mdio(struct mt7530_priv *priv)
>  	bus->priv = priv;
>  	bus->name = KBUILD_MODNAME "-mii";
>  	snprintf(bus->id, MII_BUS_ID_SIZE, KBUILD_MODNAME "-%d", idx++);
> -	bus->read = mt753x_phy_read;
> -	bus->write = mt753x_phy_write;
> +	bus->read = mt753x_phy_read_c22;
> +	bus->write = mt753x_phy_write_c22;
> +	bus->read_c45 = mt753x_phy_read_c45;
> +	bus->write_c45 = mt753x_phy_write_c45;
>  	bus->parent = dev;
>  	bus->phy_mask = ~ds->phys_mii_mask;
>  
> @@ -3130,8 +3132,10 @@ static const struct mt753x_info mt753x_table[] = {
>  		.id = ID_MT7621,
>  		.pcs_ops = &mt7530_pcs_ops,
>  		.sw_setup = mt7530_setup,
> -		.phy_read = mt7530_phy_read,
> -		.phy_write = mt7530_phy_write,
> +		.phy_read_c22 = mt7530_phy_read_c22,
> +		.phy_write_c22 = mt7530_phy_write_c22,
> +		.phy_read_c45 = mt7530_phy_read_c45,
> +		.phy_write_c45 = mt7530_phy_write_c45,
>  		.pad_setup = mt7530_pad_clk_setup,
>  		.mac_port_get_caps = mt7530_mac_port_get_caps,
>  		.mac_port_config = mt7530_mac_config,
> @@ -3140,8 +3144,10 @@ static const struct mt753x_info mt753x_table[] = {
>  		.id = ID_MT7530,
>  		.pcs_ops = &mt7530_pcs_ops,
>  		.sw_setup = mt7530_setup,
> -		.phy_read = mt7530_phy_read,
> -		.phy_write = mt7530_phy_write,
> +		.phy_read_c22 = mt7530_phy_read_c22,
> +		.phy_write_c22 = mt7530_phy_write_c22,
> +		.phy_read_c45 = mt7530_phy_read_c45,
> +		.phy_write_c45 = mt7530_phy_write_c45,
>  		.pad_setup = mt7530_pad_clk_setup,
>  		.mac_port_get_caps = mt7530_mac_port_get_caps,
>  		.mac_port_config = mt7530_mac_config,
> @@ -3150,8 +3156,10 @@ static const struct mt753x_info mt753x_table[] = {
>  		.id = ID_MT7531,
>  		.pcs_ops = &mt7531_pcs_ops,
>  		.sw_setup = mt7531_setup,
> -		.phy_read = mt7531_ind_phy_read,
> -		.phy_write = mt7531_ind_phy_write,
> +		.phy_read_c22 = mt7531_ind_c22_phy_read,
> +		.phy_write_c22 = mt7531_ind_c22_phy_write,
> +		.phy_read_c45 = mt7531_ind_c45_phy_read,
> +		.phy_write_c45 = mt7531_ind_c45_phy_write,
>  		.pad_setup = mt7531_pad_setup,
>  		.cpu_port_config = mt7531_cpu_port_config,
>  		.mac_port_get_caps = mt7531_mac_port_get_caps,
> @@ -3211,7 +3219,7 @@ mt7530_probe(struct mdio_device *mdiodev)
>  	 * properly.
>  	 */
>  	if (!priv->info->sw_setup || !priv->info->pad_setup ||
> -	    !priv->info->phy_read || !priv->info->phy_write ||
> +	    !priv->info->phy_read_c22 || !priv->info->phy_write_c22 ||
>  	    !priv->info->mac_port_get_caps ||
>  	    !priv->info->mac_port_config)
>  		return -EINVAL;
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index 71e36b69b96d..1b14146a1f08 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -750,8 +750,10 @@ struct mt753x_pcs {
>  /* struct mt753x_info -	This is the main data structure for holding the specific
>   *			part for each supported device
>   * @sw_setup:		Holding the handler to a device initialization
> - * @phy_read:		Holding the way reading PHY port
> - * @phy_write:		Holding the way writing PHY port
> + * @phy_read_c22:	Holding the way reading PHY port using C22
> + * @phy_write_c22:	Holding the way writing PHY port using C22
> + * @phy_read_c45:	Holding the way reading PHY port using C45
> + * @phy_write_c45:	Holding the way writing PHY port using C45
>   * @pad_setup:		Holding the way setting up the bus pad for a certain
>   *			MAC port
>   * @phy_mode_supported:	Check if the PHY type is being supported on a certain
> @@ -767,8 +769,13 @@ struct mt753x_info {
>  	const struct phylink_pcs_ops *pcs_ops;
>  
>  	int (*sw_setup)(struct dsa_switch *ds);
> -	int (*phy_read)(struct mt7530_priv *priv, int port, int regnum);
> -	int (*phy_write)(struct mt7530_priv *priv, int port, int regnum, u16 val);
> +	int (*phy_read_c22)(struct mt7530_priv *priv, int port, int regnum);
> +	int (*phy_write_c22)(struct mt7530_priv *priv, int port, int regnum,
> +			     u16 val);
> +	int (*phy_read_c45)(struct mt7530_priv *priv, int port, int devad,
> +			    int regnum);
> +	int (*phy_write_c45)(struct mt7530_priv *priv, int port, int devad,
> +			     int regnum, u16 val);
>  	int (*pad_setup)(struct dsa_switch *ds, phy_interface_t interface);
>  	int (*cpu_port_config)(struct dsa_switch *ds, int port);
>  	void (*mac_port_get_caps)(struct dsa_switch *ds, int port,
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 934958fda962..64e36eb33879 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -820,6 +820,10 @@ struct dsa_switch_ops {
>  	int	(*phy_read)(struct dsa_switch *ds, int port, int regnum);
>  	int	(*phy_write)(struct dsa_switch *ds, int port,
>  			     int regnum, u16 val);
> +	int	(*phy_read_c45)(struct dsa_switch *ds, int port, int devad,
> +				int regnum);
> +	int	(*phy_write_c45)(struct dsa_switch *ds, int port, int devad,
> +				 int regnum, u16 val);

Therefore it's unlikely that this is useful at this stage.
mt7530 does not go through dsa_slave_phy_write_c45, but directly through
mt753x_phy_write().

I think this patch could avoid touching the DSA core.

>  
>  	/*
>  	 * Link state adjustment (called from libphy)
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 5ee0aced9410..a8976a67a8c0 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -165,7 +165,7 @@ static int dsa_slave_unsync_mc(struct net_device *dev,
>  }
>  
>  /* slave mii_bus handling ***************************************************/
> -static int dsa_slave_phy_read(struct mii_bus *bus, int addr, int reg)
> +static int dsa_slave_phy_read_c22(struct mii_bus *bus, int addr, int reg)
>  {
>  	struct dsa_switch *ds = bus->priv;
>  
> @@ -175,7 +175,19 @@ static int dsa_slave_phy_read(struct mii_bus *bus, int addr, int reg)
>  	return 0xffff;
>  }
>  
> -static int dsa_slave_phy_write(struct mii_bus *bus, int addr, int reg, u16 val)
> +static int dsa_slave_phy_read_c45(struct mii_bus *bus, int addr, int devad,
> +				  int reg)
> +{
> +	struct dsa_switch *ds = bus->priv;
> +
> +	if (ds->phys_mii_mask & (1 << addr))
> +		return ds->ops->phy_read_c45(ds, addr, devad, reg);
> +
> +	return 0xffff;
> +}
> +
> +static int dsa_slave_phy_write_c22(struct mii_bus *bus, int addr, int reg,
> +				   u16 val)
>  {
>  	struct dsa_switch *ds = bus->priv;
>  
> @@ -185,12 +197,27 @@ static int dsa_slave_phy_write(struct mii_bus *bus, int addr, int reg, u16 val)
>  	return 0;
>  }
>  
> +static int dsa_slave_phy_write_c45(struct mii_bus *bus, int addr, int reg,
> +				   int devad, u16 val)
> +{
> +	struct dsa_switch *ds = bus->priv;
> +
> +	if (ds->phys_mii_mask & (1 << addr))
> +		return ds->ops->phy_write_c45(ds, addr, devad, reg, val);
> +
> +	return 0;
> +}
> +
>  void dsa_slave_mii_bus_init(struct dsa_switch *ds)
>  {
>  	ds->slave_mii_bus->priv = (void *)ds;
>  	ds->slave_mii_bus->name = "dsa slave smi";
> -	ds->slave_mii_bus->read = dsa_slave_phy_read;
> -	ds->slave_mii_bus->write = dsa_slave_phy_write;
> +	ds->slave_mii_bus->read = dsa_slave_phy_read_c22;
> +	ds->slave_mii_bus->write = dsa_slave_phy_write_c22;
> +	if (ds->ops->phy_read_c45 && ds->ops->phy_write_c45) {
> +		ds->slave_mii_bus->read_c45 = dsa_slave_phy_read_c45;
> +		ds->slave_mii_bus->write_c45 = dsa_slave_phy_write_c45;
> +	}
>  	snprintf(ds->slave_mii_bus->id, MII_BUS_ID_SIZE, "dsa-%d.%d",
>  		 ds->dst->index, ds->index);
>  	ds->slave_mii_bus->parent = ds->dev;
> -- 
> 2.36.0
> 

[PATCH net-next 10/10] net: dsa: mv88e6xxx: Separate C22 and C45 transactions

[Andrew Lunn]

The global2 SMI MDIO bus driver can perform both C22 and C45
transfers. Create separate functions for each and register the C45
versions using the new API calls where appropriate. Update the SERDES
code to make use of these new accessors.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/dsa/mv88e6xxx/chip.c    | 175 ++++++++++++++++++++--------
 drivers/net/dsa/mv88e6xxx/chip.h    |   7 ++
 drivers/net/dsa/mv88e6xxx/global2.c |  66 ++++++-----
 drivers/net/dsa/mv88e6xxx/global2.h |  18 ++-
 drivers/net/dsa/mv88e6xxx/phy.c     |  32 +++++
 drivers/net/dsa/mv88e6xxx/phy.h     |   4 +
 drivers/net/dsa/mv88e6xxx/serdes.c  |   8 +-
 7 files changed, 225 insertions(+), 85 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 53fd12e7a21c..bc6d3b891fb8 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3850,6 +3850,24 @@ static int mv88e6xxx_mdio_read(struct mii_bus *bus, int phy, int reg)
 	return err ? err : val;
 }
 
+static int mv88e6xxx_mdio_read_c45(struct mii_bus *bus, int phy, int devad,
+				   int reg)
+{
+	struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv;
+	struct mv88e6xxx_chip *chip = mdio_bus->chip;
+	u16 val;
+	int err;
+
+	if (!chip->info->ops->phy_read_c45)
+		return -EOPNOTSUPP;
+
+	mv88e6xxx_reg_lock(chip);
+	err = chip->info->ops->phy_read_c45(chip, bus, phy, devad, reg, &val);
+	mv88e6xxx_reg_unlock(chip);
+
+	return err ? err : val;
+}
+
 static int mv88e6xxx_mdio_write(struct mii_bus *bus, int phy, int reg, u16 val)
 {
 	struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv;
@@ -3866,6 +3884,23 @@ static int mv88e6xxx_mdio_write(struct mii_bus *bus, int phy, int reg, u16 val)
 	return err;
 }
 
+static int mv88e6xxx_mdio_write_c45(struct mii_bus *bus, int phy, int devad,
+				    int reg, u16 val)
+{
+	struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv;
+	struct mv88e6xxx_chip *chip = mdio_bus->chip;
+	int err;
+
+	if (!chip->info->ops->phy_write_c45)
+		return -EOPNOTSUPP;
+
+	mv88e6xxx_reg_lock(chip);
+	err = chip->info->ops->phy_write_c45(chip, bus, phy, devad, reg, val);
+	mv88e6xxx_reg_unlock(chip);
+
+	return err;
+}
+
 static int mv88e6xxx_mdio_register(struct mv88e6xxx_chip *chip,
 				   struct device_node *np,
 				   bool external)
@@ -3904,6 +3939,8 @@ static int mv88e6xxx_mdio_register(struct mv88e6xxx_chip *chip,
 
 	bus->read = mv88e6xxx_mdio_read;
 	bus->write = mv88e6xxx_mdio_write;
+	bus->read_c45 = mv88e6xxx_mdio_read_c45;
+	bus->write_c45 = mv88e6xxx_mdio_write_c45;
 	bus->parent = chip->dev;
 
 	if (!external) {
@@ -4113,8 +4150,10 @@ static const struct mv88e6xxx_ops mv88e6097_ops = {
 	.ip_pri_map = mv88e6085_g1_ip_pri_map,
 	.irl_init_all = mv88e6352_g2_irl_init_all,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6185_port_sync_link,
 	.port_set_speed_duplex = mv88e6185_port_set_speed_duplex,
@@ -4162,8 +4201,10 @@ static const struct mv88e6xxx_ops mv88e6123_ops = {
 	.ip_pri_map = mv88e6085_g1_ip_pri_map,
 	.irl_init_all = mv88e6352_g2_irl_init_all,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_speed_duplex = mv88e6185_port_set_speed_duplex,
@@ -4243,8 +4284,10 @@ static const struct mv88e6xxx_ops mv88e6141_ops = {
 	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom8,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
@@ -4307,8 +4350,10 @@ static const struct mv88e6xxx_ops mv88e6161_ops = {
 	.ip_pri_map = mv88e6085_g1_ip_pri_map,
 	.irl_init_all = mv88e6352_g2_irl_init_all,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_speed_duplex = mv88e6185_port_set_speed_duplex,
@@ -4389,8 +4434,10 @@ static const struct mv88e6xxx_ops mv88e6171_ops = {
 	.ip_pri_map = mv88e6085_g1_ip_pri_map,
 	.irl_init_all = mv88e6352_g2_irl_init_all,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
@@ -4435,8 +4482,10 @@ static const struct mv88e6xxx_ops mv88e6172_ops = {
 	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom16,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
@@ -4490,8 +4539,10 @@ static const struct mv88e6xxx_ops mv88e6175_ops = {
 	.ip_pri_map = mv88e6085_g1_ip_pri_map,
 	.irl_init_all = mv88e6352_g2_irl_init_all,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
@@ -4536,8 +4587,10 @@ static const struct mv88e6xxx_ops mv88e6176_ops = {
 	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom16,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
@@ -4636,8 +4689,10 @@ static const struct mv88e6xxx_ops mv88e6190_ops = {
 	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom8,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
@@ -4699,8 +4754,10 @@ static const struct mv88e6xxx_ops mv88e6190x_ops = {
 	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom8,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
@@ -4762,8 +4819,10 @@ static const struct mv88e6xxx_ops mv88e6191_ops = {
 	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom8,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
@@ -4825,8 +4884,10 @@ static const struct mv88e6xxx_ops mv88e6240_ops = {
 	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom16,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
@@ -4888,8 +4949,10 @@ static const struct mv88e6xxx_ops mv88e6250_ops = {
 	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom16,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
@@ -4927,8 +4990,10 @@ static const struct mv88e6xxx_ops mv88e6290_ops = {
 	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom8,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
@@ -4992,8 +5057,10 @@ static const struct mv88e6xxx_ops mv88e6320_ops = {
 	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom16,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_speed_duplex = mv88e6185_port_set_speed_duplex,
@@ -5036,8 +5103,10 @@ static const struct mv88e6xxx_ops mv88e6321_ops = {
 	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom16,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_speed_duplex = mv88e6185_port_set_speed_duplex,
@@ -5078,8 +5147,10 @@ static const struct mv88e6xxx_ops mv88e6341_ops = {
 	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom8,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
@@ -5144,8 +5215,10 @@ static const struct mv88e6xxx_ops mv88e6350_ops = {
 	.ip_pri_map = mv88e6085_g1_ip_pri_map,
 	.irl_init_all = mv88e6352_g2_irl_init_all,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
@@ -5188,8 +5261,10 @@ static const struct mv88e6xxx_ops mv88e6351_ops = {
 	.ip_pri_map = mv88e6085_g1_ip_pri_map,
 	.irl_init_all = mv88e6352_g2_irl_init_all,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
@@ -5236,8 +5311,10 @@ static const struct mv88e6xxx_ops mv88e6352_ops = {
 	.get_eeprom = mv88e6xxx_g2_get_eeprom16,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom16,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6352_port_set_rgmii_delay,
@@ -5301,8 +5378,10 @@ static const struct mv88e6xxx_ops mv88e6390_ops = {
 	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom8,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
@@ -5368,8 +5447,10 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = {
 	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom8,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
@@ -5434,8 +5515,10 @@ static const struct mv88e6xxx_ops mv88e6393x_ops = {
 	.get_eeprom = mv88e6xxx_g2_get_eeprom8,
 	.set_eeprom = mv88e6xxx_g2_set_eeprom8,
 	.set_switch_mac = mv88e6xxx_g2_set_switch_mac,
-	.phy_read = mv88e6xxx_g2_smi_phy_read,
-	.phy_write = mv88e6xxx_g2_smi_phy_write,
+	.phy_read = mv88e6xxx_g2_smi_phy_read_c22,
+	.phy_write = mv88e6xxx_g2_smi_phy_write_c22,
+	.phy_read_c45 = mv88e6xxx_g2_smi_phy_read_c45,
+	.phy_write_c45 = mv88e6xxx_g2_smi_phy_write_c45,
 	.port_set_link = mv88e6xxx_port_set_link,
 	.port_sync_link = mv88e6xxx_port_sync_link,
 	.port_set_rgmii_delay = mv88e6390_port_set_rgmii_delay,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 5e03cfe50156..c8122765e930 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -451,6 +451,13 @@ struct mv88e6xxx_ops {
 			 struct mii_bus *bus,
 			 int addr, int reg, u16 val);
 
+	int (*phy_read_c45)(struct mv88e6xxx_chip *chip,
+			    struct mii_bus *bus,
+			    int addr, int devad, int reg, u16 *val);
+	int (*phy_write_c45)(struct mv88e6xxx_chip *chip,
+			     struct mii_bus *bus,
+			     int addr, int devad, int reg, u16 val);
+
 	/* Priority Override Table operations */
 	int (*pot_clear)(struct mv88e6xxx_chip *chip);
 
diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c
index fa65ecd9cb85..ed3b2f88e783 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.c
+++ b/drivers/net/dsa/mv88e6xxx/global2.c
@@ -739,20 +739,18 @@ static int mv88e6xxx_g2_smi_phy_read_data_c45(struct mv88e6xxx_chip *chip,
 	return mv88e6xxx_g2_read(chip, MV88E6XXX_G2_SMI_PHY_DATA, data);
 }
 
-static int mv88e6xxx_g2_smi_phy_read_c45(struct mv88e6xxx_chip *chip,
-					 bool external, int port, int reg,
-					 u16 *data)
+static int _mv88e6xxx_g2_smi_phy_read_c45(struct mv88e6xxx_chip *chip,
+					  bool external, int port, int devad,
+					  int reg, u16 *data)
 {
-	int dev = (reg >> 16) & 0x1f;
-	int addr = reg & 0xffff;
 	int err;
 
-	err = mv88e6xxx_g2_smi_phy_write_addr_c45(chip, external, port, dev,
-						  addr);
+	err = mv88e6xxx_g2_smi_phy_write_addr_c45(chip, external, port, devad,
+						  reg);
 	if (err)
 		return err;
 
-	return mv88e6xxx_g2_smi_phy_read_data_c45(chip, external, port, dev,
+	return mv88e6xxx_g2_smi_phy_read_data_c45(chip, external, port, devad,
 						  data);
 }
 
@@ -771,51 +769,65 @@ static int mv88e6xxx_g2_smi_phy_write_data_c45(struct mv88e6xxx_chip *chip,
 	return mv88e6xxx_g2_smi_phy_access_c45(chip, external, op, port, dev);
 }
 
-static int mv88e6xxx_g2_smi_phy_write_c45(struct mv88e6xxx_chip *chip,
-					  bool external, int port, int reg,
-					  u16 data)
+static int _mv88e6xxx_g2_smi_phy_write_c45(struct mv88e6xxx_chip *chip,
+					   bool external, int port, int devad,
+					   int reg, u16 data)
 {
-	int dev = (reg >> 16) & 0x1f;
-	int addr = reg & 0xffff;
 	int err;
 
-	err = mv88e6xxx_g2_smi_phy_write_addr_c45(chip, external, port, dev,
-						  addr);
+	err = mv88e6xxx_g2_smi_phy_write_addr_c45(chip, external, port, devad,
+						  reg);
 	if (err)
 		return err;
 
-	return mv88e6xxx_g2_smi_phy_write_data_c45(chip, external, port, dev,
+	return mv88e6xxx_g2_smi_phy_write_data_c45(chip, external, port, devad,
 						   data);
 }
 
-int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip, struct mii_bus *bus,
-			      int addr, int reg, u16 *val)
+int mv88e6xxx_g2_smi_phy_read_c22(struct mv88e6xxx_chip *chip,
+				  struct mii_bus *bus,
+				  int addr, int reg, u16 *val)
 {
 	struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv;
 	bool external = mdio_bus->external;
 
-	if (reg & MII_ADDR_C45)
-		return mv88e6xxx_g2_smi_phy_read_c45(chip, external, addr, reg,
-						     val);
-
 	return mv88e6xxx_g2_smi_phy_read_data_c22(chip, external, addr, reg,
 						  val);
 }
 
-int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip, struct mii_bus *bus,
-			       int addr, int reg, u16 val)
+int mv88e6xxx_g2_smi_phy_read_c45(struct mv88e6xxx_chip *chip,
+				  struct mii_bus *bus, int addr, int devad,
+				  int reg, u16 *val)
 {
 	struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv;
 	bool external = mdio_bus->external;
 
-	if (reg & MII_ADDR_C45)
-		return mv88e6xxx_g2_smi_phy_write_c45(chip, external, addr, reg,
-						      val);
+	return _mv88e6xxx_g2_smi_phy_read_c45(chip, external, addr, devad, reg,
+					      val);
+}
+
+int mv88e6xxx_g2_smi_phy_write_c22(struct mv88e6xxx_chip *chip,
+				   struct mii_bus *bus, int addr, int reg,
+				   u16 val)
+{
+	struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv;
+	bool external = mdio_bus->external;
 
 	return mv88e6xxx_g2_smi_phy_write_data_c22(chip, external, addr, reg,
 						   val);
 }
 
+int mv88e6xxx_g2_smi_phy_write_c45(struct mv88e6xxx_chip *chip,
+				   struct mii_bus *bus, int addr, int devad,
+				   int reg, u16 val)
+{
+	struct mv88e6xxx_mdio_bus *mdio_bus = bus->priv;
+	bool external = mdio_bus->external;
+
+	return _mv88e6xxx_g2_smi_phy_write_c45(chip, external, addr, devad, reg,
+					       val);
+}
+
 /* Offset 0x1B: Watchdog Control */
 static int mv88e6097_watchdog_action(struct mv88e6xxx_chip *chip, int irq)
 {
diff --git a/drivers/net/dsa/mv88e6xxx/global2.h b/drivers/net/dsa/mv88e6xxx/global2.h
index 807aeaad9830..ee44b35500bd 100644
--- a/drivers/net/dsa/mv88e6xxx/global2.h
+++ b/drivers/net/dsa/mv88e6xxx/global2.h
@@ -314,12 +314,18 @@ int mv88e6xxx_g2_wait_bit(struct mv88e6xxx_chip *chip, int reg,
 int mv88e6352_g2_irl_init_all(struct mv88e6xxx_chip *chip, int port);
 int mv88e6390_g2_irl_init_all(struct mv88e6xxx_chip *chip, int port);
 
-int mv88e6xxx_g2_smi_phy_read(struct mv88e6xxx_chip *chip,
-			      struct mii_bus *bus,
-			      int addr, int reg, u16 *val);
-int mv88e6xxx_g2_smi_phy_write(struct mv88e6xxx_chip *chip,
-			       struct mii_bus *bus,
-			       int addr, int reg, u16 val);
+int mv88e6xxx_g2_smi_phy_read_c22(struct mv88e6xxx_chip *chip,
+				  struct mii_bus *bus,
+				  int addr, int reg, u16 *val);
+int mv88e6xxx_g2_smi_phy_write_c22(struct mv88e6xxx_chip *chip,
+				   struct mii_bus *bus,
+				   int addr, int reg, u16 val);
+int mv88e6xxx_g2_smi_phy_read_c45(struct mv88e6xxx_chip *chip,
+				  struct mii_bus *bus,
+				  int addr, int devad, int reg, u16 *val);
+int mv88e6xxx_g2_smi_phy_write_c45(struct mv88e6xxx_chip *chip,
+				   struct mii_bus *bus,
+				   int addr, int devad, int reg, u16 val);
 int mv88e6xxx_g2_set_switch_mac(struct mv88e6xxx_chip *chip, u8 *addr);
 
 int mv88e6xxx_g2_get_eeprom8(struct mv88e6xxx_chip *chip,
diff --git a/drivers/net/dsa/mv88e6xxx/phy.c b/drivers/net/dsa/mv88e6xxx/phy.c
index 252b5b3a3efe..8bb88b3d900d 100644
--- a/drivers/net/dsa/mv88e6xxx/phy.c
+++ b/drivers/net/dsa/mv88e6xxx/phy.c
@@ -55,6 +55,38 @@ int mv88e6xxx_phy_write(struct mv88e6xxx_chip *chip, int phy, int reg, u16 val)
 	return chip->info->ops->phy_write(chip, bus, addr, reg, val);
 }
 
+int mv88e6xxx_phy_read_c45(struct mv88e6xxx_chip *chip, int phy, int devad,
+			   int reg, u16 *val)
+{
+	int addr = phy; /* PHY devices addresses start at 0x0 */
+	struct mii_bus *bus;
+
+	bus = mv88e6xxx_default_mdio_bus(chip);
+	if (!bus)
+		return -EOPNOTSUPP;
+
+	if (!chip->info->ops->phy_read_c45)
+		return -EOPNOTSUPP;
+
+	return chip->info->ops->phy_read_c45(chip, bus, addr, devad, reg, val);
+}
+
+int mv88e6xxx_phy_write_c45(struct mv88e6xxx_chip *chip, int phy, int devad,
+			    int reg, u16 val)
+{
+	int addr = phy; /* PHY devices addresses start at 0x0 */
+	struct mii_bus *bus;
+
+	bus = mv88e6xxx_default_mdio_bus(chip);
+	if (!bus)
+		return -EOPNOTSUPP;
+
+	if (!chip->info->ops->phy_write_c45)
+		return -EOPNOTSUPP;
+
+	return chip->info->ops->phy_write_c45(chip, bus, addr, devad, reg, val);
+}
+
 static int mv88e6xxx_phy_page_get(struct mv88e6xxx_chip *chip, int phy, u8 page)
 {
 	return mv88e6xxx_phy_write(chip, phy, MV88E6XXX_PHY_PAGE, page);
diff --git a/drivers/net/dsa/mv88e6xxx/phy.h b/drivers/net/dsa/mv88e6xxx/phy.h
index 05ea0d546969..5f47722364cc 100644
--- a/drivers/net/dsa/mv88e6xxx/phy.h
+++ b/drivers/net/dsa/mv88e6xxx/phy.h
@@ -28,6 +28,10 @@ int mv88e6xxx_phy_read(struct mv88e6xxx_chip *chip, int phy,
 		       int reg, u16 *val);
 int mv88e6xxx_phy_write(struct mv88e6xxx_chip *chip, int phy,
 			int reg, u16 val);
+int mv88e6xxx_phy_read_c45(struct mv88e6xxx_chip *chip, int phy, int devad,
+			   int reg, u16 *val);
+int mv88e6xxx_phy_write_c45(struct mv88e6xxx_chip *chip, int phy, int devad,
+			    int reg, u16 val);
 int mv88e6xxx_phy_page_read(struct mv88e6xxx_chip *chip, int phy,
 			    u8 page, int reg, u16 *val);
 int mv88e6xxx_phy_page_write(struct mv88e6xxx_chip *chip, int phy,
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index 7b37d45bc9fb..efb3bf898d84 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -36,17 +36,13 @@ static int mv88e6352_serdes_write(struct mv88e6xxx_chip *chip, int reg,
 static int mv88e6390_serdes_read(struct mv88e6xxx_chip *chip,
 				 int lane, int device, int reg, u16 *val)
 {
-	int reg_c45 = MII_ADDR_C45 | device << 16 | reg;
-
-	return mv88e6xxx_phy_read(chip, lane, reg_c45, val);
+	return mv88e6xxx_phy_read_c45(chip, lane, device, reg, val);
 }
 
 static int mv88e6390_serdes_write(struct mv88e6xxx_chip *chip,
 				  int lane, int device, int reg, u16 val)
 {
-	int reg_c45 = MII_ADDR_C45 | device << 16 | reg;
-
-	return mv88e6xxx_phy_write(chip, lane, reg_c45, val);
+	return mv88e6xxx_phy_write_c45(chip, lane, device, reg, val);
 }
 
 static int mv88e6xxx_serdes_pcs_get_state(struct mv88e6xxx_chip *chip,
-- 
2.36.0