1. Patchew
  2. linux
  3. [PATCH net-next 0/3] net: phy: marvell-88q2xxx: cleanup
  4. View patch

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

Dimitri Fedrau posted 3 patches 10 months, 1 week ago
[PATCH net-next 1/3] net: phy: marvell-88q2xxx: align defines
Posted by Dimitri Fedrau 10 months, 1 week ago 
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(-)

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
Re: [PATCH net-next 1/3] net: phy: marvell-88q2xxx: align defines
Posted by Marek Behún 10 months ago 
> +#define MDIO_MMD_AN_MV_STAT				32769

Why the hell are register addresses in this driver in decimal?
Re: [PATCH net-next 1/3] net: phy: marvell-88q2xxx: align defines
Posted by Russell King (Oracle) 10 months ago 
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.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH net-next 1/3] net: phy: marvell-88q2xxx: align defines
Posted by Dimitri Fedrau 10 months ago 
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
Re: [PATCH net-next 1/3] net: phy: marvell-88q2xxx: align defines
Posted by Andrew Lunn 10 months ago 
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
Re: [PATCH net-next 1/3] net: phy: marvell-88q2xxx: align defines
Posted by Marek Behún 10 months ago 
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 :)
Re: [PATCH net-next 1/3] net: phy: marvell-88q2xxx: align defines
Posted by Niklas Söderlund 10 months, 1 week ago 
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
> 

-- 
Kind Regards,
Niklas Söderlund

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.