netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/8] net: phy: mscc: multiple improvements
@ 2020-06-25 15:42 Antoine Tenart
  2020-06-25 15:42 ` [PATCH net-next 1/8] net: phy: mscc: macsec: fix sparse warnings Antoine Tenart
                   ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Antoine Tenart @ 2020-06-25 15:42 UTC (permalink / raw)
  To: davem, andrew, f.fainelli, hkallweit1
  Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni

Hello,

This series contains various improvements to the MSCC PHY driver, fixing
sparse and smatch warnings, using functions provided by the PHY core,
and improving the driver consistency and maintenance.

I don't think any of those improvements and fixes is worth backporting
to stable trees.

Thanks!
Antoine

Antoine Tenart (8):
  net: phy: mscc: macsec: fix sparse warnings
  net: phy: mscc: fix a possible double unlock
  net: phy: mscc: ptp: fix a smatch error
  net: phy: mscc: ptp: fix a typo in a comment
  net: phy: mscc: do not access the MDIO bus lock directly
  net: phy: mscc: restore the base page in vsc8514/8584_config_init
  net: phy: mscc: remove useless page configuration in the config init
  net: phy: mscc: improve vsc8514/8584_config_init consistency

 drivers/net/phy/mscc/mscc_macsec.c | 12 ++++---
 drivers/net/phy/mscc/mscc_main.c   | 54 +++++++++++++++++-------------
 drivers/net/phy/mscc/mscc_ptp.c    | 15 +++++----
 3 files changed, 45 insertions(+), 36 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH net-next 1/8] net: phy: mscc: macsec: fix sparse warnings
  2020-06-25 15:42 [PATCH net-next 0/8] net: phy: mscc: multiple improvements Antoine Tenart
@ 2020-06-25 15:42 ` Antoine Tenart
  2020-06-25 16:46   ` Andrew Lunn
  2020-06-25 15:42 ` [PATCH net-next 2/8] net: phy: mscc: fix a possible double unlock Antoine Tenart
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 12+ messages in thread
From: Antoine Tenart @ 2020-06-25 15:42 UTC (permalink / raw)
  To: davem, andrew, f.fainelli, hkallweit1
  Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni

This patch fixes the following sparse warnings when building MACsec
support in the MSCC PHY driver.

  mscc_macsec.c:393:42: warning: cast from restricted sci_t
  mscc_macsec.c:395:42: warning: restricted sci_t degrades to integer
  mscc_macsec.c:402:42: warning: restricted __be16 degrades to integer
  mscc_macsec.c:608:34: warning: cast from restricted sci_t
  mscc_macsec.c:610:34: warning: restricted sci_t degrades to integer

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/phy/mscc/mscc_macsec.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc_macsec.c b/drivers/net/phy/mscc/mscc_macsec.c
index 713c62b1d1f0..77c8c2fb28de 100644
--- a/drivers/net/phy/mscc/mscc_macsec.c
+++ b/drivers/net/phy/mscc/mscc_macsec.c
@@ -385,21 +385,23 @@ static void vsc8584_macsec_flow(struct phy_device *phydev,
 	}
 
 	if (bank == MACSEC_INGR && flow->match.sci && flow->rx_sa->sc->sci) {
+		u64 sci = (__force u64)flow->rx_sa->sc->sci;
+
 		match |= MSCC_MS_SAM_MISC_MATCH_TCI(BIT(3));
 		mask |= MSCC_MS_SAM_MASK_TCI_MASK(BIT(3)) |
 			MSCC_MS_SAM_MASK_SCI_MASK;
 
 		vsc8584_macsec_phy_write(phydev, bank, MSCC_MS_SAM_MATCH_SCI_LO(idx),
-					 lower_32_bits(flow->rx_sa->sc->sci));
+					 lower_32_bits(sci));
 		vsc8584_macsec_phy_write(phydev, bank, MSCC_MS_SAM_MATCH_SCI_HI(idx),
-					 upper_32_bits(flow->rx_sa->sc->sci));
+					 upper_32_bits(sci));
 	}
 
 	if (flow->match.etype) {
 		mask |= MSCC_MS_SAM_MASK_MAC_ETYPE_MASK;
 
 		vsc8584_macsec_phy_write(phydev, bank, MSCC_MS_SAM_MAC_SA_MATCH_HI(idx),
-					 MSCC_MS_SAM_MAC_SA_MATCH_HI_ETYPE(htons(flow->etype)));
+					 MSCC_MS_SAM_MAC_SA_MATCH_HI_ETYPE((__force u32)htons(flow->etype)));
 	}
 
 	match |= MSCC_MS_SAM_MISC_MATCH_PRIORITY(flow->priority);
@@ -545,7 +547,7 @@ static int vsc8584_macsec_transformation(struct phy_device *phydev,
 	int i, ret, index = flow->index;
 	u32 rec = 0, control = 0;
 	u8 hkey[16];
-	sci_t sci;
+	u64 sci;
 
 	ret = vsc8584_macsec_derive_key(flow->key, priv->secy->key_len, hkey);
 	if (ret)
@@ -603,7 +605,7 @@ static int vsc8584_macsec_transformation(struct phy_device *phydev,
 					 priv->secy->replay_window);
 
 	/* Set the input vectors */
-	sci = bank == MACSEC_INGR ? flow->rx_sa->sc->sci : priv->secy->sci;
+	sci = (__force u64)(bank == MACSEC_INGR ? flow->rx_sa->sc->sci : priv->secy->sci);
 	vsc8584_macsec_phy_write(phydev, bank, MSCC_MS_XFORM_REC(index, rec++),
 				 lower_32_bits(sci));
 	vsc8584_macsec_phy_write(phydev, bank, MSCC_MS_XFORM_REC(index, rec++),
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 2/8] net: phy: mscc: fix a possible double unlock
  2020-06-25 15:42 [PATCH net-next 0/8] net: phy: mscc: multiple improvements Antoine Tenart
  2020-06-25 15:42 ` [PATCH net-next 1/8] net: phy: mscc: macsec: fix sparse warnings Antoine Tenart
@ 2020-06-25 15:42 ` Antoine Tenart
  2020-06-25 15:42 ` [PATCH net-next 3/8] net: phy: mscc: ptp: fix a smatch error Antoine Tenart
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Antoine Tenart @ 2020-06-25 15:42 UTC (permalink / raw)
  To: davem, andrew, f.fainelli, hkallweit1
  Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni,
	kernel test robot, Dan Carpenter

On vsc8584_ptp_init failure we jump to the 'err' label, which unlocks
the MDIO bus lock. But vsc8584_ptp_init isn't called with the MDIO bus
lock taken, which could result in a double unlock. Fix this.

Fixes: ab2bf9339357 ("net: phy: mscc: 1588 block initialization")
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/phy/mscc/mscc_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index 2a7082983c09..cb4e15d6e2db 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -1436,7 +1436,7 @@ static int vsc8584_config_init(struct phy_device *phydev)
 
 	ret = vsc8584_ptp_init(phydev);
 	if (ret)
-		goto err;
+		return ret;
 
 	phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD);
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 3/8] net: phy: mscc: ptp: fix a smatch error
  2020-06-25 15:42 [PATCH net-next 0/8] net: phy: mscc: multiple improvements Antoine Tenart
  2020-06-25 15:42 ` [PATCH net-next 1/8] net: phy: mscc: macsec: fix sparse warnings Antoine Tenart
  2020-06-25 15:42 ` [PATCH net-next 2/8] net: phy: mscc: fix a possible double unlock Antoine Tenart
@ 2020-06-25 15:42 ` Antoine Tenart
  2020-06-25 15:42 ` [PATCH net-next 4/8] net: phy: mscc: ptp: fix a typo in a comment Antoine Tenart
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Antoine Tenart @ 2020-06-25 15:42 UTC (permalink / raw)
  To: davem, andrew, f.fainelli, hkallweit1
  Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni,
	kernel test robot, Dan Carpenter

The following error was reported by smatch:
vsc85xx_ts_read_csr() error: uninitialized symbol 'blk_hw'.

In practice this is very unlikely, as all the block identifiers given to
this functions are handled and described in an enum. The smatch error is
fixed by doing what is already done in vsc85xx_ts_write_csr: using the
"PROCESSOR" block by default.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/phy/mscc/mscc_ptp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c
index 17256d85cedf..030a56c9a06d 100644
--- a/drivers/net/phy/mscc/mscc_ptp.c
+++ b/drivers/net/phy/mscc/mscc_ptp.c
@@ -75,6 +75,7 @@ static u32 vsc85xx_ts_read_csr(struct phy_device *phydev, enum ts_blk blk,
 		blk_hw = base_port ? EGRESS_ENGINE_0 : EGRESS_ENGINE_1;
 		break;
 	case PROCESSOR:
+	default:
 		blk_hw = base_port ? PROCESSOR_0 : PROCESSOR_1;
 		break;
 	}
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 4/8] net: phy: mscc: ptp: fix a typo in a comment
  2020-06-25 15:42 [PATCH net-next 0/8] net: phy: mscc: multiple improvements Antoine Tenart
                   ` (2 preceding siblings ...)
  2020-06-25 15:42 ` [PATCH net-next 3/8] net: phy: mscc: ptp: fix a smatch error Antoine Tenart
@ 2020-06-25 15:42 ` Antoine Tenart
  2020-06-25 15:42 ` [PATCH net-next 5/8] net: phy: mscc: do not access the MDIO bus lock directly Antoine Tenart
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Antoine Tenart @ 2020-06-25 15:42 UTC (permalink / raw)
  To: davem, andrew, f.fainelli, hkallweit1
  Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni

This patch fixes a typo in a comment, s/Ths/This/. The patch is cosmetic
only.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/phy/mscc/mscc_ptp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c
index 030a56c9a06d..d4266911efc5 100644
--- a/drivers/net/phy/mscc/mscc_ptp.c
+++ b/drivers/net/phy/mscc/mscc_ptp.c
@@ -1564,7 +1564,7 @@ int vsc8584_ptp_probe(struct phy_device *phydev)
 
 	/* Retrieve the shared load/save GPIO. Request it as non exclusive as
 	 * the same GPIO can be requested by all the PHYs of the same package.
-	 * Ths GPIO must be used with the gpio_lock taken (the lock is shared
+	 * This GPIO must be used with the gpio_lock taken (the lock is shared
 	 * between all PHYs).
 	 */
 	vsc8531->load_save = devm_gpiod_get_optional(&phydev->mdio.dev, "load-save",
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 5/8] net: phy: mscc: do not access the MDIO bus lock directly
  2020-06-25 15:42 [PATCH net-next 0/8] net: phy: mscc: multiple improvements Antoine Tenart
                   ` (3 preceding siblings ...)
  2020-06-25 15:42 ` [PATCH net-next 4/8] net: phy: mscc: ptp: fix a typo in a comment Antoine Tenart
@ 2020-06-25 15:42 ` Antoine Tenart
  2020-06-25 16:47   ` Andrew Lunn
  2020-06-25 15:42 ` [PATCH net-next 6/8] net: phy: mscc: restore the base page in vsc8514/8584_config_init Antoine Tenart
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 12+ messages in thread
From: Antoine Tenart @ 2020-06-25 15:42 UTC (permalink / raw)
  To: davem, andrew, f.fainelli, hkallweit1
  Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni

This patch improves the MSCC driver by using the provided
phy_lock_mdio_bus and phy_unlock_mdio_bus helpers instead of locking and
unlocking the MDIO bus lock directly. The patch is only cosmetic but
should improve maintenance and consistency.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/phy/mscc/mscc_main.c | 24 ++++++++++++------------
 drivers/net/phy/mscc/mscc_ptp.c  | 12 ++++++------
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index cb4e15d6e2db..03680933f530 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -1288,7 +1288,7 @@ static void vsc8584_get_base_addr(struct phy_device *phydev)
 	struct vsc8531_private *vsc8531 = phydev->priv;
 	u16 val, addr;
 
-	mutex_lock(&phydev->mdio.bus->mdio_lock);
+	phy_lock_mdio_bus(phydev);
 	__phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_EXTENDED);
 
 	addr = __phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_4);
@@ -1297,7 +1297,7 @@ static void vsc8584_get_base_addr(struct phy_device *phydev)
 	val = __phy_read(phydev, MSCC_PHY_ACTIPHY_CNTL);
 
 	__phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD);
-	mutex_unlock(&phydev->mdio.bus->mdio_lock);
+	phy_unlock_mdio_bus(phydev);
 
 	/* In the package, there are two pairs of PHYs (PHY0 + PHY2 and
 	 * PHY1 + PHY3). The first PHY of each pair (PHY0 and PHY1) is
@@ -1331,7 +1331,7 @@ static int vsc8584_config_init(struct phy_device *phydev)
 
 	phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
 
-	mutex_lock(&phydev->mdio.bus->mdio_lock);
+	phy_lock_mdio_bus(phydev);
 
 	/* Some parts of the init sequence are identical for every PHY in the
 	 * package. Some parts are modifying the GPIO register bank which is a
@@ -1428,7 +1428,7 @@ static int vsc8584_config_init(struct phy_device *phydev)
 	if (ret)
 		goto err;
 
-	mutex_unlock(&phydev->mdio.bus->mdio_lock);
+	phy_unlock_mdio_bus(phydev);
 
 	ret = vsc8584_macsec_init(phydev);
 	if (ret)
@@ -1469,7 +1469,7 @@ static int vsc8584_config_init(struct phy_device *phydev)
 	return 0;
 
 err:
-	mutex_unlock(&phydev->mdio.bus->mdio_lock);
+	phy_unlock_mdio_bus(phydev);
 	return ret;
 }
 
@@ -1755,7 +1755,7 @@ static int vsc8514_config_init(struct phy_device *phydev)
 
 	phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
 
-	mutex_lock(&phydev->mdio.bus->mdio_lock);
+	phy_lock_mdio_bus(phydev);
 
 	/* Some parts of the init sequence are identical for every PHY in the
 	 * package. Some parts are modifying the GPIO register bank which is a
@@ -1843,14 +1843,14 @@ static int vsc8514_config_init(struct phy_device *phydev)
 		reg = vsc85xx_csr_ctrl_phy_read(phydev, PHY_MCB_TARGET,
 						PHY_S6G_PLL_STATUS);
 		if (reg == 0xffffffff) {
-			mutex_unlock(&phydev->mdio.bus->mdio_lock);
+			phy_unlock_mdio_bus(phydev);
 			return -EIO;
 		}
 
 	} while (time_before(jiffies, deadline) && (reg & BIT(12)));
 
 	if (reg & BIT(12)) {
-		mutex_unlock(&phydev->mdio.bus->mdio_lock);
+		phy_unlock_mdio_bus(phydev);
 		return -ETIMEDOUT;
 	}
 
@@ -1870,18 +1870,18 @@ static int vsc8514_config_init(struct phy_device *phydev)
 		reg = vsc85xx_csr_ctrl_phy_read(phydev, PHY_MCB_TARGET,
 						PHY_S6G_IB_STATUS0);
 		if (reg == 0xffffffff) {
-			mutex_unlock(&phydev->mdio.bus->mdio_lock);
+			phy_unlock_mdio_bus(phydev);
 			return -EIO;
 		}
 
 	} while (time_before(jiffies, deadline) && !(reg & BIT(8)));
 
 	if (!(reg & BIT(8))) {
-		mutex_unlock(&phydev->mdio.bus->mdio_lock);
+		phy_unlock_mdio_bus(phydev);
 		return -ETIMEDOUT;
 	}
 
-	mutex_unlock(&phydev->mdio.bus->mdio_lock);
+	phy_unlock_mdio_bus(phydev);
 
 	ret = phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD);
 
@@ -1908,7 +1908,7 @@ static int vsc8514_config_init(struct phy_device *phydev)
 	return ret;
 
 err:
-	mutex_unlock(&phydev->mdio.bus->mdio_lock);
+	phy_unlock_mdio_bus(phydev);
 	return ret;
 }
 
diff --git a/drivers/net/phy/mscc/mscc_ptp.c b/drivers/net/phy/mscc/mscc_ptp.c
index d4266911efc5..ef3441747348 100644
--- a/drivers/net/phy/mscc/mscc_ptp.c
+++ b/drivers/net/phy/mscc/mscc_ptp.c
@@ -80,7 +80,7 @@ static u32 vsc85xx_ts_read_csr(struct phy_device *phydev, enum ts_blk blk,
 		break;
 	}
 
-	mutex_lock(&phydev->mdio.bus->mdio_lock);
+	phy_lock_mdio_bus(phydev);
 
 	phy_ts_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_1588);
 
@@ -98,7 +98,7 @@ static u32 vsc85xx_ts_read_csr(struct phy_device *phydev, enum ts_blk blk,
 
 	phy_ts_base_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD);
 
-	mutex_unlock(&phydev->mdio.bus->mdio_lock);
+	phy_unlock_mdio_bus(phydev);
 
 	return val;
 }
@@ -130,7 +130,7 @@ static void vsc85xx_ts_write_csr(struct phy_device *phydev, enum ts_blk blk,
 		break;
 	}
 
-	mutex_lock(&phydev->mdio.bus->mdio_lock);
+	phy_lock_mdio_bus(phydev);
 
 	bypass = phy_ts_base_read(phydev, MSCC_PHY_BYPASS_CONTROL);
 
@@ -154,7 +154,7 @@ static void vsc85xx_ts_write_csr(struct phy_device *phydev, enum ts_blk blk,
 	if (cond && upper)
 		phy_ts_base_write(phydev, MSCC_PHY_BYPASS_CONTROL, bypass);
 
-	mutex_unlock(&phydev->mdio.bus->mdio_lock);
+	phy_unlock_mdio_bus(phydev);
 }
 
 /* Pick bytes from PTP header */
@@ -1273,7 +1273,7 @@ static int __vsc8584_init_ptp(struct phy_device *phydev)
 	u32 val;
 
 	if (!vsc8584_is_1588_input_clk_configured(phydev)) {
-		mutex_lock(&phydev->mdio.bus->mdio_lock);
+		phy_lock_mdio_bus(phydev);
 
 		/* 1588_DIFF_INPUT_CLK configuration: Use an external clock for
 		 * the LTC, as per 3.13.29 in the VSC8584 datasheet.
@@ -1285,7 +1285,7 @@ static int __vsc8584_init_ptp(struct phy_device *phydev)
 		phy_ts_base_write(phydev, MSCC_EXT_PAGE_ACCESS,
 				  MSCC_PHY_PAGE_STANDARD);
 
-		mutex_unlock(&phydev->mdio.bus->mdio_lock);
+		phy_unlock_mdio_bus(phydev);
 
 		vsc8584_set_input_clk_configured(phydev);
 	}
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 6/8] net: phy: mscc: restore the base page in vsc8514/8584_config_init
  2020-06-25 15:42 [PATCH net-next 0/8] net: phy: mscc: multiple improvements Antoine Tenart
                   ` (4 preceding siblings ...)
  2020-06-25 15:42 ` [PATCH net-next 5/8] net: phy: mscc: do not access the MDIO bus lock directly Antoine Tenart
@ 2020-06-25 15:42 ` Antoine Tenart
  2020-06-25 15:42 ` [PATCH net-next 7/8] net: phy: mscc: remove useless page configuration in the config init Antoine Tenart
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Antoine Tenart @ 2020-06-25 15:42 UTC (permalink / raw)
  To: davem, andrew, f.fainelli, hkallweit1
  Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni

In the vsc8584_config_init and vsc8514_config_init, the base page is set
to 'GPIO', configuration is done, and the page is never explicitly
restored to the standard page. No bug was triggered as it turns out
helpers called in those config_init functions do modify the base page,
and set it back to standard. But that is dangerous and any modification
to those functions would introduce bugs. This patch fixes this, to
improve maintenance, by restoring the base page to 'standard' once
'GPIO' accesses are completed.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/phy/mscc/mscc_main.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index 03680933f530..f625109df00a 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -1395,6 +1395,11 @@ static int vsc8584_config_init(struct phy_device *phydev)
 	if (ret)
 		goto err;
 
+	ret = phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS,
+			     MSCC_PHY_PAGE_STANDARD);
+	if (ret)
+		goto err;
+
 	if (!phy_interface_is_rgmii(phydev)) {
 		val = PROC_CMD_MCB_ACCESS_MAC_CONF | PROC_CMD_RST_CONF_PORT |
 			PROC_CMD_READ_MOD_WRITE_PORT;
@@ -1779,7 +1784,11 @@ static int vsc8514_config_init(struct phy_device *phydev)
 	val &= ~MAC_CFG_MASK;
 	val |= MAC_CFG_QSGMII;
 	ret = phy_base_write(phydev, MSCC_PHY_MAC_CFG_FASTLINK, val);
+	if (ret)
+		goto err;
 
+	ret = phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS,
+			     MSCC_PHY_PAGE_STANDARD);
 	if (ret)
 		goto err;
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 7/8] net: phy: mscc: remove useless page configuration in the config init
  2020-06-25 15:42 [PATCH net-next 0/8] net: phy: mscc: multiple improvements Antoine Tenart
                   ` (5 preceding siblings ...)
  2020-06-25 15:42 ` [PATCH net-next 6/8] net: phy: mscc: restore the base page in vsc8514/8584_config_init Antoine Tenart
@ 2020-06-25 15:42 ` Antoine Tenart
  2020-06-25 15:42 ` [PATCH net-next 8/8] net: phy: mscc: improve vsc8514/8584_config_init consistency Antoine Tenart
  2020-06-25 23:22 ` [PATCH net-next 0/8] net: phy: mscc: multiple improvements David Miller
  8 siblings, 0 replies; 12+ messages in thread
From: Antoine Tenart @ 2020-06-25 15:42 UTC (permalink / raw)
  To: davem, andrew, f.fainelli, hkallweit1
  Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni

In the middle of vsc8584_config_init and vsc8514_config_init, the page
is set to 'standard'. This is the default value, and the page isn't set
to another value before. Those pages configuration can be safely
removed.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/phy/mscc/mscc_main.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index f625109df00a..04e1ef791cec 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -1443,8 +1443,6 @@ static int vsc8584_config_init(struct phy_device *phydev)
 	if (ret)
 		return ret;
 
-	phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD);
-
 	val = phy_read(phydev, MSCC_PHY_EXT_PHY_CNTL_1);
 	val &= ~(MEDIA_OP_MODE_MASK | VSC8584_MAC_IF_SELECTION_MASK);
 	val |= (MEDIA_OP_MODE_COPPER << MEDIA_OP_MODE_POS) |
@@ -1892,11 +1890,6 @@ static int vsc8514_config_init(struct phy_device *phydev)
 
 	phy_unlock_mdio_bus(phydev);
 
-	ret = phy_write(phydev, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD);
-
-	if (ret)
-		return ret;
-
 	ret = phy_modify(phydev, MSCC_PHY_EXT_PHY_CNTL_1, MEDIA_OP_MODE_MASK,
 			 MEDIA_OP_MODE_COPPER << MEDIA_OP_MODE_POS);
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 8/8] net: phy: mscc: improve vsc8514/8584_config_init consistency
  2020-06-25 15:42 [PATCH net-next 0/8] net: phy: mscc: multiple improvements Antoine Tenart
                   ` (6 preceding siblings ...)
  2020-06-25 15:42 ` [PATCH net-next 7/8] net: phy: mscc: remove useless page configuration in the config init Antoine Tenart
@ 2020-06-25 15:42 ` Antoine Tenart
  2020-06-25 23:22 ` [PATCH net-next 0/8] net: phy: mscc: multiple improvements David Miller
  8 siblings, 0 replies; 12+ messages in thread
From: Antoine Tenart @ 2020-06-25 15:42 UTC (permalink / raw)
  To: davem, andrew, f.fainelli, hkallweit1
  Cc: Antoine Tenart, netdev, linux-kernel, thomas.petazzoni

All PHY read and write return values are checked for errors in
vsc8514_config_init and vsc8584_config_init, except for one. Fix this.

Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
---
 drivers/net/phy/mscc/mscc_main.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c
index 04e1ef791cec..a4fbf3a4fa97 100644
--- a/drivers/net/phy/mscc/mscc_main.c
+++ b/drivers/net/phy/mscc/mscc_main.c
@@ -1375,8 +1375,10 @@ static int vsc8584_config_init(struct phy_device *phydev)
 			goto err;
 	}
 
-	phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS,
-		       MSCC_PHY_PAGE_EXTENDED_GPIO);
+	ret = phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS,
+			     MSCC_PHY_PAGE_EXTENDED_GPIO);
+	if (ret)
+		goto err;
 
 	val = phy_base_read(phydev, MSCC_PHY_MAC_CFG_FASTLINK);
 	val &= ~MAC_CFG_MASK;
@@ -1774,8 +1776,10 @@ static int vsc8514_config_init(struct phy_device *phydev)
 	if (phy_package_init_once(phydev))
 		vsc8514_config_pre_init(phydev);
 
-	phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS,
-		       MSCC_PHY_PAGE_EXTENDED_GPIO);
+	ret = phy_base_write(phydev, MSCC_EXT_PAGE_ACCESS,
+			     MSCC_PHY_PAGE_EXTENDED_GPIO);
+	if (ret)
+		goto err;
 
 	val = phy_base_read(phydev, MSCC_PHY_MAC_CFG_FASTLINK);
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 1/8] net: phy: mscc: macsec: fix sparse warnings
  2020-06-25 15:42 ` [PATCH net-next 1/8] net: phy: mscc: macsec: fix sparse warnings Antoine Tenart
@ 2020-06-25 16:46   ` Andrew Lunn
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Lunn @ 2020-06-25 16:46 UTC (permalink / raw)
  To: Antoine Tenart
  Cc: davem, f.fainelli, hkallweit1, netdev, linux-kernel, thomas.petazzoni

On Thu, Jun 25, 2020 at 05:42:04PM +0200, Antoine Tenart wrote:
> This patch fixes the following sparse warnings when building MACsec
> support in the MSCC PHY driver.
> 
>   mscc_macsec.c:393:42: warning: cast from restricted sci_t
>   mscc_macsec.c:395:42: warning: restricted sci_t degrades to integer
>   mscc_macsec.c:402:42: warning: restricted __be16 degrades to integer
>   mscc_macsec.c:608:34: warning: cast from restricted sci_t
>   mscc_macsec.c:610:34: warning: restricted sci_t degrades to integer
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>

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

    Andrew

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 5/8] net: phy: mscc: do not access the MDIO bus lock directly
  2020-06-25 15:42 ` [PATCH net-next 5/8] net: phy: mscc: do not access the MDIO bus lock directly Antoine Tenart
@ 2020-06-25 16:47   ` Andrew Lunn
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Lunn @ 2020-06-25 16:47 UTC (permalink / raw)
  To: Antoine Tenart
  Cc: davem, f.fainelli, hkallweit1, netdev, linux-kernel, thomas.petazzoni

On Thu, Jun 25, 2020 at 05:42:08PM +0200, Antoine Tenart wrote:
> This patch improves the MSCC driver by using the provided
> phy_lock_mdio_bus and phy_unlock_mdio_bus helpers instead of locking and
> unlocking the MDIO bus lock directly. The patch is only cosmetic but
> should improve maintenance and consistency.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>

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

    Andrew

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 0/8] net: phy: mscc: multiple improvements
  2020-06-25 15:42 [PATCH net-next 0/8] net: phy: mscc: multiple improvements Antoine Tenart
                   ` (7 preceding siblings ...)
  2020-06-25 15:42 ` [PATCH net-next 8/8] net: phy: mscc: improve vsc8514/8584_config_init consistency Antoine Tenart
@ 2020-06-25 23:22 ` David Miller
  8 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2020-06-25 23:22 UTC (permalink / raw)
  To: antoine.tenart
  Cc: andrew, f.fainelli, hkallweit1, netdev, linux-kernel, thomas.petazzoni

From: Antoine Tenart <antoine.tenart@bootlin.com>
Date: Thu, 25 Jun 2020 17:42:03 +0200

> This series contains various improvements to the MSCC PHY driver, fixing
> sparse and smatch warnings, using functions provided by the PHY core,
> and improving the driver consistency and maintenance.
> 
> I don't think any of those improvements and fixes is worth backporting
> to stable trees.

Series applied, thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-06-25 23:22 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-25 15:42 [PATCH net-next 0/8] net: phy: mscc: multiple improvements Antoine Tenart
2020-06-25 15:42 ` [PATCH net-next 1/8] net: phy: mscc: macsec: fix sparse warnings Antoine Tenart
2020-06-25 16:46   ` Andrew Lunn
2020-06-25 15:42 ` [PATCH net-next 2/8] net: phy: mscc: fix a possible double unlock Antoine Tenart
2020-06-25 15:42 ` [PATCH net-next 3/8] net: phy: mscc: ptp: fix a smatch error Antoine Tenart
2020-06-25 15:42 ` [PATCH net-next 4/8] net: phy: mscc: ptp: fix a typo in a comment Antoine Tenart
2020-06-25 15:42 ` [PATCH net-next 5/8] net: phy: mscc: do not access the MDIO bus lock directly Antoine Tenart
2020-06-25 16:47   ` Andrew Lunn
2020-06-25 15:42 ` [PATCH net-next 6/8] net: phy: mscc: restore the base page in vsc8514/8584_config_init Antoine Tenart
2020-06-25 15:42 ` [PATCH net-next 7/8] net: phy: mscc: remove useless page configuration in the config init Antoine Tenart
2020-06-25 15:42 ` [PATCH net-next 8/8] net: phy: mscc: improve vsc8514/8584_config_init consistency Antoine Tenart
2020-06-25 23:22 ` [PATCH net-next 0/8] net: phy: mscc: multiple improvements David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).