diff mbox series

[net-next,1/3] net: phy: marvell-88q2xxx: align defines

Message ID 20250214-marvell-88q2xxx-cleanup-v1-1-71d67c20f308@gmail.com (mailing list archive)
State Accepted
Commit 8dcaed624f6af020023a09f7d14decc7ec730355
Delegated to: Netdev Maintainers
Headers show
Series net: phy: marvell-88q2xxx: cleanup | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
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: 0 this patch: 0
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: 0 this patch: 0
netdev/checkpatch warning WARNING: line length of 86 exceeds 80 columns
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
netdev/contest success net-next-2025-02-15--03-00 (tests: 891)

Commit Message

Dimitri Fedrau Feb. 14, 2025, 4:32 p.m. UTC 
Align some defines.

Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>
---
 drivers/net/phy/marvell-88q2xxx.c | 62 +++++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

Comments

Niklas Söderlund Feb. 14, 2025, 5:52 p.m. UTC | #1 
Hi Dimitri,

Thanks for your work.

On 2025-02-14 17:32:03 +0100, Dimitri Fedrau wrote:
> Align some defines.
> 
> Signed-off-by: Dimitri Fedrau <dima.fedrau@gmail.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/net/phy/marvell-88q2xxx.c | 62 +++++++++++++++++++--------------------
>  1 file changed, 31 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
> index bad5e7b2357da067bfd1ec6bd1307c42f5dc5c91..6e95de080bc65e8e8543d4effb9846fdd823a9d4 100644
> --- a/drivers/net/phy/marvell-88q2xxx.c
> +++ b/drivers/net/phy/marvell-88q2xxx.c
> @@ -12,29 +12,29 @@
>  #include <linux/phy.h>
>  #include <linux/hwmon.h>
>  
> -#define PHY_ID_88Q2220_REVB0	(MARVELL_PHY_ID_88Q2220 | 0x1)
> -#define PHY_ID_88Q2220_REVB1	(MARVELL_PHY_ID_88Q2220 | 0x2)
> -#define PHY_ID_88Q2220_REVB2	(MARVELL_PHY_ID_88Q2220 | 0x3)
> -
> -#define MDIO_MMD_AN_MV_STAT			32769
> -#define MDIO_MMD_AN_MV_STAT_ANEG		0x0100
> -#define MDIO_MMD_AN_MV_STAT_LOCAL_RX		0x1000
> -#define MDIO_MMD_AN_MV_STAT_REMOTE_RX		0x2000
> -#define MDIO_MMD_AN_MV_STAT_LOCAL_MASTER	0x4000
> -#define MDIO_MMD_AN_MV_STAT_MS_CONF_FAULT	0x8000
> -
> -#define MDIO_MMD_AN_MV_STAT2			32794
> -#define MDIO_MMD_AN_MV_STAT2_AN_RESOLVED	0x0800
> -#define MDIO_MMD_AN_MV_STAT2_100BT1		0x2000
> -#define MDIO_MMD_AN_MV_STAT2_1000BT1		0x4000
> -
> -#define MDIO_MMD_PCS_MV_RESET_CTRL		32768
> -#define MDIO_MMD_PCS_MV_RESET_CTRL_TX_DISABLE	0x8
> -
> -#define MDIO_MMD_PCS_MV_INT_EN			32784
> -#define MDIO_MMD_PCS_MV_INT_EN_LINK_UP		0x0040
> -#define MDIO_MMD_PCS_MV_INT_EN_LINK_DOWN	0x0080
> -#define MDIO_MMD_PCS_MV_INT_EN_100BT1		0x1000
> +#define PHY_ID_88Q2220_REVB0				(MARVELL_PHY_ID_88Q2220 | 0x1)
> +#define PHY_ID_88Q2220_REVB1				(MARVELL_PHY_ID_88Q2220 | 0x2)
> +#define PHY_ID_88Q2220_REVB2				(MARVELL_PHY_ID_88Q2220 | 0x3)
> +
> +#define MDIO_MMD_AN_MV_STAT				32769
> +#define MDIO_MMD_AN_MV_STAT_ANEG			0x0100
> +#define MDIO_MMD_AN_MV_STAT_LOCAL_RX			0x1000
> +#define MDIO_MMD_AN_MV_STAT_REMOTE_RX			0x2000
> +#define MDIO_MMD_AN_MV_STAT_LOCAL_MASTER		0x4000
> +#define MDIO_MMD_AN_MV_STAT_MS_CONF_FAULT		0x8000
> +
> +#define MDIO_MMD_AN_MV_STAT2				32794
> +#define MDIO_MMD_AN_MV_STAT2_AN_RESOLVED		0x0800
> +#define MDIO_MMD_AN_MV_STAT2_100BT1			0x2000
> +#define MDIO_MMD_AN_MV_STAT2_1000BT1			0x4000
> +
> +#define MDIO_MMD_PCS_MV_RESET_CTRL			32768
> +#define MDIO_MMD_PCS_MV_RESET_CTRL_TX_DISABLE		0x8
> +
> +#define MDIO_MMD_PCS_MV_INT_EN				32784
> +#define MDIO_MMD_PCS_MV_INT_EN_LINK_UP			0x0040
> +#define MDIO_MMD_PCS_MV_INT_EN_LINK_DOWN		0x0080
> +#define MDIO_MMD_PCS_MV_INT_EN_100BT1			0x1000
>  
>  #define MDIO_MMD_PCS_MV_GPIO_INT_STAT			32785
>  #define MDIO_MMD_PCS_MV_GPIO_INT_STAT_LINK_UP		0x0040
> @@ -80,11 +80,11 @@
>  #define MDIO_MMD_PCS_MV_100BT1_STAT1_REMOTE_RX		0x2000
>  #define MDIO_MMD_PCS_MV_100BT1_STAT1_LOCAL_MASTER	0x4000
>  
> -#define MDIO_MMD_PCS_MV_100BT1_STAT2		33033
> -#define MDIO_MMD_PCS_MV_100BT1_STAT2_JABBER	0x0001
> -#define MDIO_MMD_PCS_MV_100BT1_STAT2_POL	0x0002
> -#define MDIO_MMD_PCS_MV_100BT1_STAT2_LINK	0x0004
> -#define MDIO_MMD_PCS_MV_100BT1_STAT2_ANGE	0x0008
> +#define MDIO_MMD_PCS_MV_100BT1_STAT2			33033
> +#define MDIO_MMD_PCS_MV_100BT1_STAT2_JABBER		0x0001
> +#define MDIO_MMD_PCS_MV_100BT1_STAT2_POL		0x0002
> +#define MDIO_MMD_PCS_MV_100BT1_STAT2_LINK		0x0004
> +#define MDIO_MMD_PCS_MV_100BT1_STAT2_ANGE		0x0008
>  
>  #define MDIO_MMD_PCS_MV_100BT1_INT_EN			33042
>  #define MDIO_MMD_PCS_MV_100BT1_INT_EN_LINKEVENT		0x0400
> @@ -92,7 +92,7 @@
>  #define MDIO_MMD_PCS_MV_COPPER_INT_STAT			33043
>  #define MDIO_MMD_PCS_MV_COPPER_INT_STAT_LINKEVENT	0x0400
>  
> -#define MDIO_MMD_PCS_MV_RX_STAT			33328
> +#define MDIO_MMD_PCS_MV_RX_STAT				33328
>  
>  #define MDIO_MMD_PCS_MV_TDR_RESET			65226
>  #define MDIO_MMD_PCS_MV_TDR_RESET_TDR_RST		0x1000
> @@ -115,8 +115,8 @@
>  
>  #define MDIO_MMD_PCS_MV_TDR_OFF_CUTOFF			65246
>  
> -#define MV88Q2XXX_LED_INDEX_TX_ENABLE	0
> -#define MV88Q2XXX_LED_INDEX_GPIO	1
> +#define MV88Q2XXX_LED_INDEX_TX_ENABLE			0
> +#define MV88Q2XXX_LED_INDEX_GPIO			1
>  
>  struct mv88q2xxx_priv {
>  	bool enable_temp;
> 
> -- 
> 2.39.5
>
Marek Behún Feb. 18, 2025, 11:54 a.m. UTC | #2 
> +#define MDIO_MMD_AN_MV_STAT				32769

Why the hell are register addresses in this driver in decimal?
Andrew Lunn Feb. 18, 2025, 2:12 p.m. UTC | #3 
On Tue, Feb 18, 2025 at 12:54:29PM +0100, Marek Behún wrote:
> > +#define MDIO_MMD_AN_MV_STAT				32769
> 
> Why the hell are register addresses in this driver in decimal?

Maybe because 802.3 uses decimal? Take a look at Table 45–3—PMA/PMD
registers for example.

	Andrew
Marek Behún Feb. 18, 2025, 2:31 p.m. UTC | #4 
On Tue, Feb 18, 2025 at 03:12:26PM +0100, Andrew Lunn wrote:
> On Tue, Feb 18, 2025 at 12:54:29PM +0100, Marek Behún wrote:
> > > +#define MDIO_MMD_AN_MV_STAT				32769
> > 
> > Why the hell are register addresses in this driver in decimal?
> 
> Maybe because 802.3 uses decimal? Take a look at Table 45–3—PMA/PMD
> registers for example.

Oh. Sorry about that, then :)
Russell King (Oracle) Feb. 18, 2025, 4 p.m. UTC | #5 
On Tue, Feb 18, 2025 at 12:54:29PM +0100, Marek Behún wrote:
> > +#define MDIO_MMD_AN_MV_STAT				32769
> 
> Why the hell are register addresses in this driver in decimal?

Shocker - some documentation gives driver addresses for PHYs in
decimal.

It then becomes a pain to have to manually translate back and
forth between hex and decimal if one is reading the driver code and
referring back to the documentation.
Dimitri Fedrau Feb. 18, 2025, 6:13 p.m. UTC | #6 
Am Tue, Feb 18, 2025 at 04:00:42PM +0000 schrieb Russell King (Oracle):
> On Tue, Feb 18, 2025 at 12:54:29PM +0100, Marek Behún wrote:
> > > +#define MDIO_MMD_AN_MV_STAT				32769
> > 
> > Why the hell are register addresses in this driver in decimal?
> 
> Shocker - some documentation gives driver addresses for PHYs in
> decimal.
> 
> It then becomes a pain to have to manually translate back and
> forth between hex and decimal if one is reading the driver code and
> referring back to the documentation.
>
Agreed. Is it worth to change addresses to hexadecimal numbers and use
BIT and GENMASK for the bits ?

Best regards,
Dimitri Fedrau
diff mbox series

Patch

diff --git a/drivers/net/phy/marvell-88q2xxx.c b/drivers/net/phy/marvell-88q2xxx.c
index bad5e7b2357da067bfd1ec6bd1307c42f5dc5c91..6e95de080bc65e8e8543d4effb9846fdd823a9d4 100644
--- a/drivers/net/phy/marvell-88q2xxx.c
+++ b/drivers/net/phy/marvell-88q2xxx.c
@@ -12,29 +12,29 @@ 
 #include <linux/phy.h>
 #include <linux/hwmon.h>
 
-#define PHY_ID_88Q2220_REVB0	(MARVELL_PHY_ID_88Q2220 | 0x1)
-#define PHY_ID_88Q2220_REVB1	(MARVELL_PHY_ID_88Q2220 | 0x2)
-#define PHY_ID_88Q2220_REVB2	(MARVELL_PHY_ID_88Q2220 | 0x3)
-
-#define MDIO_MMD_AN_MV_STAT			32769
-#define MDIO_MMD_AN_MV_STAT_ANEG		0x0100
-#define MDIO_MMD_AN_MV_STAT_LOCAL_RX		0x1000
-#define MDIO_MMD_AN_MV_STAT_REMOTE_RX		0x2000
-#define MDIO_MMD_AN_MV_STAT_LOCAL_MASTER	0x4000
-#define MDIO_MMD_AN_MV_STAT_MS_CONF_FAULT	0x8000
-
-#define MDIO_MMD_AN_MV_STAT2			32794
-#define MDIO_MMD_AN_MV_STAT2_AN_RESOLVED	0x0800
-#define MDIO_MMD_AN_MV_STAT2_100BT1		0x2000
-#define MDIO_MMD_AN_MV_STAT2_1000BT1		0x4000
-
-#define MDIO_MMD_PCS_MV_RESET_CTRL		32768
-#define MDIO_MMD_PCS_MV_RESET_CTRL_TX_DISABLE	0x8
-
-#define MDIO_MMD_PCS_MV_INT_EN			32784
-#define MDIO_MMD_PCS_MV_INT_EN_LINK_UP		0x0040
-#define MDIO_MMD_PCS_MV_INT_EN_LINK_DOWN	0x0080
-#define MDIO_MMD_PCS_MV_INT_EN_100BT1		0x1000
+#define PHY_ID_88Q2220_REVB0				(MARVELL_PHY_ID_88Q2220 | 0x1)
+#define PHY_ID_88Q2220_REVB1				(MARVELL_PHY_ID_88Q2220 | 0x2)
+#define PHY_ID_88Q2220_REVB2				(MARVELL_PHY_ID_88Q2220 | 0x3)
+
+#define MDIO_MMD_AN_MV_STAT				32769
+#define MDIO_MMD_AN_MV_STAT_ANEG			0x0100
+#define MDIO_MMD_AN_MV_STAT_LOCAL_RX			0x1000
+#define MDIO_MMD_AN_MV_STAT_REMOTE_RX			0x2000
+#define MDIO_MMD_AN_MV_STAT_LOCAL_MASTER		0x4000
+#define MDIO_MMD_AN_MV_STAT_MS_CONF_FAULT		0x8000
+
+#define MDIO_MMD_AN_MV_STAT2				32794
+#define MDIO_MMD_AN_MV_STAT2_AN_RESOLVED		0x0800
+#define MDIO_MMD_AN_MV_STAT2_100BT1			0x2000
+#define MDIO_MMD_AN_MV_STAT2_1000BT1			0x4000
+
+#define MDIO_MMD_PCS_MV_RESET_CTRL			32768
+#define MDIO_MMD_PCS_MV_RESET_CTRL_TX_DISABLE		0x8
+
+#define MDIO_MMD_PCS_MV_INT_EN				32784
+#define MDIO_MMD_PCS_MV_INT_EN_LINK_UP			0x0040
+#define MDIO_MMD_PCS_MV_INT_EN_LINK_DOWN		0x0080
+#define MDIO_MMD_PCS_MV_INT_EN_100BT1			0x1000
 
 #define MDIO_MMD_PCS_MV_GPIO_INT_STAT			32785
 #define MDIO_MMD_PCS_MV_GPIO_INT_STAT_LINK_UP		0x0040
@@ -80,11 +80,11 @@ 
 #define MDIO_MMD_PCS_MV_100BT1_STAT1_REMOTE_RX		0x2000
 #define MDIO_MMD_PCS_MV_100BT1_STAT1_LOCAL_MASTER	0x4000
 
-#define MDIO_MMD_PCS_MV_100BT1_STAT2		33033
-#define MDIO_MMD_PCS_MV_100BT1_STAT2_JABBER	0x0001
-#define MDIO_MMD_PCS_MV_100BT1_STAT2_POL	0x0002
-#define MDIO_MMD_PCS_MV_100BT1_STAT2_LINK	0x0004
-#define MDIO_MMD_PCS_MV_100BT1_STAT2_ANGE	0x0008
+#define MDIO_MMD_PCS_MV_100BT1_STAT2			33033
+#define MDIO_MMD_PCS_MV_100BT1_STAT2_JABBER		0x0001
+#define MDIO_MMD_PCS_MV_100BT1_STAT2_POL		0x0002
+#define MDIO_MMD_PCS_MV_100BT1_STAT2_LINK		0x0004
+#define MDIO_MMD_PCS_MV_100BT1_STAT2_ANGE		0x0008
 
 #define MDIO_MMD_PCS_MV_100BT1_INT_EN			33042
 #define MDIO_MMD_PCS_MV_100BT1_INT_EN_LINKEVENT		0x0400
@@ -92,7 +92,7 @@ 
 #define MDIO_MMD_PCS_MV_COPPER_INT_STAT			33043
 #define MDIO_MMD_PCS_MV_COPPER_INT_STAT_LINKEVENT	0x0400
 
-#define MDIO_MMD_PCS_MV_RX_STAT			33328
+#define MDIO_MMD_PCS_MV_RX_STAT				33328
 
 #define MDIO_MMD_PCS_MV_TDR_RESET			65226
 #define MDIO_MMD_PCS_MV_TDR_RESET_TDR_RST		0x1000
@@ -115,8 +115,8 @@ 
 
 #define MDIO_MMD_PCS_MV_TDR_OFF_CUTOFF			65246
 
-#define MV88Q2XXX_LED_INDEX_TX_ENABLE	0
-#define MV88Q2XXX_LED_INDEX_GPIO	1
+#define MV88Q2XXX_LED_INDEX_TX_ENABLE			0
+#define MV88Q2XXX_LED_INDEX_GPIO			1
 
 struct mv88q2xxx_priv {
 	bool enable_temp;