diff mbox series

[net,3/6] net: dsa: vsc73xx: use defined values in phy operations

Message ID 20240802080403.739509-4-paweldembicki@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: dsa: vsc73xx: fix MDIO bus access and PHY operations | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 29 this patch: 29
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 29 this patch: 29
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 29 this patch: 29
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 84 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Pawel Dembicki Aug. 2, 2024, 8:04 a.m. UTC
This commit changes magic numbers in phy operations.
Some shifted registers was replaced with bitfield macros.

No functional changes done.

Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
---
This patch came from net-next series[0].
Changes since net-next:
  - rebased to netdev/main only

[0] https://patchwork.kernel.org/project/netdevbpf/patch/20240729210615.279952-6-paweldembicki@gmail.com/
---
 drivers/net/dsa/vitesse-vsc73xx-core.c | 45 ++++++++++++++++++++------
 1 file changed, 35 insertions(+), 10 deletions(-)

Comments

Linus Walleij Aug. 2, 2024, 9:03 p.m. UTC | #1
On Fri, Aug 2, 2024 at 10:04 AM Pawel Dembicki <paweldembicki@gmail.com> wrote:

> This commit changes magic numbers in phy operations.
> Some shifted registers was replaced with bitfield macros.
>
> No functional changes done.
>
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Florian Fainelli Aug. 2, 2024, 9:57 p.m. UTC | #2
On 8/2/24 01:04, Pawel Dembicki wrote:
> This commit changes magic numbers in phy operations.
> Some shifted registers was replaced with bitfield macros.
> 
> No functional changes done.
> 
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Vladimir Oltean Aug. 2, 2024, 10:15 p.m. UTC | #3
On Fri, Aug 02, 2024 at 10:04:00AM +0200, Pawel Dembicki wrote:
> This commit changes magic numbers in phy operations.
> Some shifted registers was replaced with bitfield macros.
> 
> No functional changes done.
> 
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> ---

Your patch helps. It makes it clearer that the hardware could be driven
by the drivers/net/mdio/mdio-mscc-miim.c driver. No?

Otherwise, I wonder if the triage you've done between bug fixes for
'net' and cleanup for 'net-next' is enough.
Pawel Dembicki Aug. 4, 2024, 11:08 p.m. UTC | #4
sob., 3 sie 2024 o 00:15 Vladimir Oltean <olteanv@gmail.com> napisał(a):
>
> On Fri, Aug 02, 2024 at 10:04:00AM +0200, Pawel Dembicki wrote:
> > This commit changes magic numbers in phy operations.
> > Some shifted registers was replaced with bitfield macros.
> >
> > No functional changes done.
> >
> > Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> > ---
>
> Your patch helps. It makes it clearer that the hardware could be driven
> by the drivers/net/mdio/mdio-mscc-miim.c driver. No?

Older VItesse hardware implementation is similar. vsc73xx has slightly
different registers but it could be resolved.
However I'm not sure if it is possible to have one implementation for
platform and spi driver. I can't prepare support for spi connection
without a device for tests.

To be honest, I would prefer to merge the fixes to the existing
implementation at this point.

> Otherwise, I wonder if the triage you've done between bug fixes for
> 'net' and cleanup for 'net-next' is enough.

This patch isn't a fix. It helps to reduce magic numbers in the next
patch. I could send it to net-next and add missing definitions to the
"busy_check" patch.
diff mbox series

Patch

diff --git a/drivers/net/dsa/vitesse-vsc73xx-core.c b/drivers/net/dsa/vitesse-vsc73xx-core.c
index 4b300c293dec..b6c46a3da9a5 100644
--- a/drivers/net/dsa/vitesse-vsc73xx-core.c
+++ b/drivers/net/dsa/vitesse-vsc73xx-core.c
@@ -21,6 +21,7 @@ 
 #include <linux/of.h>
 #include <linux/of_mdio.h>
 #include <linux/bitops.h>
+#include <linux/bitfield.h>
 #include <linux/if_bridge.h>
 #include <linux/if_vlan.h>
 #include <linux/etherdevice.h>
@@ -40,6 +41,10 @@ 
 #define VSC73XX_BLOCK_ARBITER	0x5 /* Only subblock 0 */
 #define VSC73XX_BLOCK_SYSTEM	0x7 /* Only subblock 0 */
 
+/* MII Block subblock */
+#define VSC73XX_BLOCK_MII_INTERNAL	0x0 /* Internal MDIO subblock */
+#define VSC73XX_BLOCK_MII_EXTERNAL	0x1 /* External MDIO subblock */
+
 #define CPU_PORT	6 /* CPU port */
 
 /* MAC Block registers */
@@ -221,9 +226,22 @@ 
 #define VSC73XX_VLANACCESS_VLAN_TBL_CMD_CLEAR_TABLE	3
 
 /* MII block 3 registers */
-#define VSC73XX_MII_STAT	0x0
-#define VSC73XX_MII_CMD		0x1
-#define VSC73XX_MII_DATA	0x2
+#define VSC73XX_MII_STAT		0x0
+#define VSC73XX_MII_CMD			0x1
+#define VSC73XX_MII_DATA		0x2
+
+#define VSC73XX_MII_STAT_BUSY		BIT(3)
+#define VSC73XX_MII_STAT_READ		BIT(2)
+#define VSC73XX_MII_STAT_WRITE		BIT(1)
+
+#define VSC73XX_MII_CMD_SCAN		BIT(27)
+#define VSC73XX_MII_CMD_OPERATION	BIT(26)
+#define VSC73XX_MII_CMD_PHY_ADDR	GENMASK(25, 21)
+#define VSC73XX_MII_CMD_PHY_REG		GENMASK(20, 16)
+#define VSC73XX_MII_CMD_WRITE_DATA	GENMASK(15, 0)
+
+#define VSC73XX_MII_DATA_FAILURE	BIT(16)
+#define VSC73XX_MII_DATA_READ_DATA	GENMASK(15, 0)
 
 /* Arbiter block 5 registers */
 #define VSC73XX_ARBEMPTY		0x0c
@@ -535,20 +553,24 @@  static int vsc73xx_phy_read(struct dsa_switch *ds, int phy, int regnum)
 	int ret;
 
 	/* Setting bit 26 means "read" */
-	cmd = BIT(26) | (phy << 21) | (regnum << 16);
-	ret = vsc73xx_write(vsc, VSC73XX_BLOCK_MII, 0, 1, cmd);
+	cmd = VSC73XX_MII_CMD_OPERATION |
+	      FIELD_PREP(VSC73XX_MII_CMD_PHY_ADDR, phy) |
+	      FIELD_PREP(VSC73XX_MII_CMD_PHY_REG, regnum);
+	ret = vsc73xx_write(vsc, VSC73XX_BLOCK_MII, VSC73XX_BLOCK_MII_INTERNAL,
+			    VSC73XX_MII_CMD, cmd);
 	if (ret)
 		return ret;
 	msleep(2);
-	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_MII, 0, 2, &val);
+	ret = vsc73xx_read(vsc, VSC73XX_BLOCK_MII, VSC73XX_BLOCK_MII_INTERNAL,
+			   VSC73XX_MII_DATA, &val);
 	if (ret)
 		return ret;
-	if (val & BIT(16)) {
+	if (val & VSC73XX_MII_DATA_FAILURE) {
 		dev_err(vsc->dev, "reading reg %02x from phy%d failed\n",
 			regnum, phy);
 		return -EIO;
 	}
-	val &= 0xFFFFU;
+	val &= VSC73XX_MII_DATA_READ_DATA;
 
 	dev_dbg(vsc->dev, "read reg %02x from phy%d = %04x\n",
 		regnum, phy, val);
@@ -574,8 +596,11 @@  static int vsc73xx_phy_write(struct dsa_switch *ds, int phy, int regnum,
 		return 0;
 	}
 
-	cmd = (phy << 21) | (regnum << 16) | val;
-	ret = vsc73xx_write(vsc, VSC73XX_BLOCK_MII, 0, 1, cmd);
+	cmd = FIELD_PREP(VSC73XX_MII_CMD_PHY_ADDR, phy) |
+	      FIELD_PREP(VSC73XX_MII_CMD_PHY_REG, regnum) |
+	      FIELD_PREP(VSC73XX_MII_CMD_WRITE_DATA, val);
+	ret = vsc73xx_write(vsc, VSC73XX_BLOCK_MII, VSC73XX_BLOCK_MII_INTERNAL,
+			    VSC73XX_MII_CMD, cmd);
 	if (ret)
 		return ret;