[PATCH net 1/6] net: dsa: b53: serialize SerDes register transactions

[Florian Fainelli <florian.fainelli@broadcom.com>]

The lane and block selectors are shared by both SerDes lanes. Taking
reg_mutex separately for each register access allows another port to
change either selector before the data access. Hold the mutex across the
complete transaction and use the raw bus operations while it is held.

Propagate bus errors instead of returning uninitialized register contents,
only cache a successfully selected lane, and invalidate the cache after
failed accesses. Make the PCS callbacks fail configuration or report link
down on read errors, and reject out-of-range lane indices at discovery.

Fixes: 0e01491de646 ("net: dsa: b53: Add SerDes support")
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
 drivers/net/dsa/b53/b53_common.c |   1 +
 drivers/net/dsa/b53/b53_serdes.c | 148 ++++++++++++++++++++++++-------
 2 files changed, 115 insertions(+), 34 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 0880310c9ce3..3f8f956aab82 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -3203,6 +3203,7 @@ struct b53_device *b53_switch_alloc(struct device *base,
 	dev->ds = ds;
 	dev->priv = priv;
 	dev->ops = ops;
+	dev->serdes_lane = B53_INVALID_LANE;
 	ds->ops = &b53_switch_ops;
 	ds->phylink_mac_ops = &b53_phylink_mac_ops;
 	dev->vlan_enabled = true;
diff --git a/drivers/net/dsa/b53/b53_serdes.c b/drivers/net/dsa/b53/b53_serdes.c
index 7460122f6abc..c90894877626 100644
--- a/drivers/net/dsa/b53/b53_serdes.c
+++ b/drivers/net/dsa/b53/b53_serdes.c
@@ -22,47 +22,96 @@ static inline struct b53_pcs *pcs_to_b53_pcs(struct phylink_pcs *pcs)
 	return container_of(pcs, struct b53_pcs, pcs);
 }
 
-static void b53_serdes_write_blk(struct b53_device *dev, u8 offset, u16 block,
-				 u16 value)
+/* The caller holds reg_mutex across lane, block and data accesses. */
+static int b53_serdes_write_blk(struct b53_device *dev, u8 offset, u16 block,
+				u16 value)
 {
-	b53_write16(dev, B53_SERDES_PAGE, B53_SERDES_BLKADDR, block);
-	b53_write16(dev, B53_SERDES_PAGE, offset, value);
+	int ret;
+
+	lockdep_assert_held(&dev->reg_mutex);
+
+	ret = dev->ops->write16(dev, B53_SERDES_PAGE, B53_SERDES_BLKADDR,
+				block);
+	if (ret)
+		return ret;
+
+	return dev->ops->write16(dev, B53_SERDES_PAGE, offset, value);
 }
 
-static u16 b53_serdes_read_blk(struct b53_device *dev, u8 offset, u16 block)
+static int b53_serdes_read_blk(struct b53_device *dev, u8 offset, u16 block)
 {
 	u16 value;
+	int ret;
+
+	lockdep_assert_held(&dev->reg_mutex);
 
-	b53_write16(dev, B53_SERDES_PAGE, B53_SERDES_BLKADDR, block);
-	b53_read16(dev, B53_SERDES_PAGE, offset, &value);
+	ret = dev->ops->write16(dev, B53_SERDES_PAGE, B53_SERDES_BLKADDR,
+				block);
+	if (ret)
+		return ret;
 
-	return value;
+	ret = dev->ops->read16(dev, B53_SERDES_PAGE, offset, &value);
+	return ret ? ret : value;
 }
 
-static void b53_serdes_set_lane(struct b53_device *dev, u8 lane)
+static int b53_serdes_set_lane(struct b53_device *dev, u8 lane)
 {
+	int ret;
+
+	lockdep_assert_held(&dev->reg_mutex);
+
+	if (lane >= B53_N_PCS)
+		return -EINVAL;
+
 	if (dev->serdes_lane == lane)
-		return;
+		return 0;
 
-	WARN_ON(lane > 1);
+	ret = b53_serdes_write_blk(dev, B53_SERDES_LANE,
+				   SERDES_XGXSBLK0_BLOCKADDRESS, lane);
+	if (ret)
+		return ret;
 
-	b53_serdes_write_blk(dev, B53_SERDES_LANE,
-			     SERDES_XGXSBLK0_BLOCKADDRESS, lane);
 	dev->serdes_lane = lane;
+	return 0;
 }
 
-static void b53_serdes_write(struct b53_device *dev, u8 lane,
-			     u8 offset, u16 block, u16 value)
+static int b53_serdes_write(struct b53_device *dev, u8 lane,
+			    u8 offset, u16 block, u16 value)
 {
-	b53_serdes_set_lane(dev, lane);
-	b53_serdes_write_blk(dev, offset, block, value);
+	int ret;
+
+	mutex_lock(&dev->reg_mutex);
+	ret = b53_serdes_set_lane(dev, lane);
+	if (!ret)
+		ret = b53_serdes_write_blk(dev, offset, block, value);
+	if (ret)
+		dev->serdes_lane = B53_INVALID_LANE;
+	mutex_unlock(&dev->reg_mutex);
+
+	return ret;
 }
 
-static u16 b53_serdes_read(struct b53_device *dev, u8 lane,
+static int b53_serdes_read(struct b53_device *dev, u8 lane,
 			   u8 offset, u16 block)
 {
-	b53_serdes_set_lane(dev, lane);
-	return b53_serdes_read_blk(dev, offset, block);
+	int ret;
+
+	mutex_lock(&dev->reg_mutex);
+	ret = b53_serdes_set_lane(dev, lane);
+	if (!ret)
+		ret = b53_serdes_read_blk(dev, offset, block);
+	if (ret < 0)
+		dev->serdes_lane = B53_INVALID_LANE;
+	mutex_unlock(&dev->reg_mutex);
+
+	return ret;
+}
+
+static void b53_serdes_error(struct b53_device *dev, u8 lane, int ret)
+{
+	if (ret < 0)
+		dev_err_ratelimited(dev->dev, "SerDes lane %u access failed: %pe\n",
+				    lane, ERR_PTR(ret));
 }
 
 static int b53_serdes_config(struct phylink_pcs *pcs, unsigned int neg_mode,
@@ -72,31 +121,37 @@ static int b53_serdes_config(struct phylink_pcs *pcs, unsigned int neg_mode,
 {
 	struct b53_device *dev = pcs_to_b53_pcs(pcs)->dev;
 	u8 lane = pcs_to_b53_pcs(pcs)->lane;
-	u16 reg;
+	int reg;
 
 	reg = b53_serdes_read(dev, lane, B53_SERDES_DIGITAL_CONTROL(1),
 			      SERDES_DIGITAL_BLK);
+	if (reg < 0)
+		return reg;
+
 	if (interface == PHY_INTERFACE_MODE_1000BASEX)
 		reg |= FIBER_MODE_1000X;
 	else
 		reg &= ~FIBER_MODE_1000X;
-	b53_serdes_write(dev, lane, B53_SERDES_DIGITAL_CONTROL(1),
-			 SERDES_DIGITAL_BLK, reg);
-
-	return 0;
+	return b53_serdes_write(dev, lane, B53_SERDES_DIGITAL_CONTROL(1),
+				SERDES_DIGITAL_BLK, reg);
 }
 
 static void b53_serdes_an_restart(struct phylink_pcs *pcs)
 {
 	struct b53_device *dev = pcs_to_b53_pcs(pcs)->dev;
 	u8 lane = pcs_to_b53_pcs(pcs)->lane;
-	u16 reg;
+	int reg;
 
 	reg = b53_serdes_read(dev, lane, B53_SERDES_MII_REG(MII_BMCR),
 			      SERDES_MII_BLK);
+	if (reg < 0)
+		goto out;
+
 	reg |= BMCR_ANRESTART;
-	b53_serdes_write(dev, lane, B53_SERDES_MII_REG(MII_BMCR),
-			 SERDES_MII_BLK, reg);
+	reg = b53_serdes_write(dev, lane, B53_SERDES_MII_REG(MII_BMCR),
+			       SERDES_MII_BLK, reg);
+out:
+	b53_serdes_error(dev, lane, reg);
 }
 
 static void b53_serdes_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
@@ -104,12 +159,23 @@ static void b53_serdes_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
 {
 	struct b53_device *dev = pcs_to_b53_pcs(pcs)->dev;
 	u8 lane = pcs_to_b53_pcs(pcs)->lane;
-	u16 dig, bmsr;
+	int dig, bmsr;
 
 	dig = b53_serdes_read(dev, lane, B53_SERDES_DIGITAL_STATUS,
 			      SERDES_DIGITAL_BLK);
+	if (dig < 0) {
+		state->link = false;
+		b53_serdes_error(dev, lane, dig);
+		return;
+	}
+
 	bmsr = b53_serdes_read(dev, lane, B53_SERDES_MII_REG(MII_BMSR),
 			       SERDES_MII_BLK);
+	if (bmsr < 0) {
+		state->link = false;
+		b53_serdes_error(dev, lane, bmsr);
+		return;
+	}
 
 	switch ((dig >> SPEED_STATUS_SHIFT) & SPEED_STATUS_MASK) {
 	case SPEED_STATUS_10:
@@ -140,19 +206,24 @@ void b53_serdes_link_set(struct b53_device *dev, int port, unsigned int mode,
 			 phy_interface_t interface, bool link_up)
 {
 	u8 lane = b53_serdes_map_lane(dev, port);
-	u16 reg;
+	int reg;
 
 	if (lane == B53_INVALID_LANE)
 		return;
 
 	reg = b53_serdes_read(dev, lane, B53_SERDES_MII_REG(MII_BMCR),
 			      SERDES_MII_BLK);
+	if (reg < 0)
+		goto out;
+
 	if (link_up)
 		reg &= ~BMCR_PDOWN;
 	else
 		reg |= BMCR_PDOWN;
-	b53_serdes_write(dev, lane, B53_SERDES_MII_REG(MII_BMCR),
-			 SERDES_MII_BLK, reg);
+	reg = b53_serdes_write(dev, lane, B53_SERDES_MII_REG(MII_BMCR),
+			       SERDES_MII_BLK, reg);
+out:
+	b53_serdes_error(dev, lane, reg);
 }
 EXPORT_SYMBOL(b53_serdes_link_set);
 
@@ -213,16 +284,25 @@ int b53_serdes_init(struct b53_device *dev, int port)
 {
 	u8 lane = b53_serdes_map_lane(dev, port);
 	struct b53_pcs *pcs;
-	u16 id0, msb, lsb;
+	int id0, msb, lsb;
 
-	if (lane == B53_INVALID_LANE)
+	if (lane >= B53_N_PCS)
 		return -EINVAL;
 
 	id0 = b53_serdes_read(dev, lane, B53_SERDES_ID0, SERDES_ID0);
+	if (id0 < 0)
+		return id0;
+
 	msb = b53_serdes_read(dev, lane, B53_SERDES_MII_REG(MII_PHYSID1),
 			      SERDES_MII_BLK);
+	if (msb < 0)
+		return msb;
+
 	lsb = b53_serdes_read(dev, lane, B53_SERDES_MII_REG(MII_PHYSID2),
 			      SERDES_MII_BLK);
+	if (lsb < 0)
+		return lsb;
+
 	if (id0 == 0 || id0 == 0xffff) {
 		dev_err(dev->dev, "SerDes not initialized, check settings\n");
 		return -ENODEV;
-- 
2.53.0