[PATCH net v4 0/4] Correcting switch hardware versions and reported speeds

[PATCH net v4 0/4] Correcting switch hardware versions and reported speeds

[Justin Lai]

This patch set mainly involves correcting switch hardware versions and
reported speeds.
Details are as follows:
1. Refactor the rtase_check_mac_version_valid() function.
2. Correct the speed for RTL907XD-V1
3. Corrects error handling of the rtase_check_mac_version_valid()
4. Add defines for hardware version id

v1 -> v2:
- Add Fixes: tag.
- Add defines for hardware version id.
- Modify the error message for an invalid hardware version ID.

v2 -> v3:
- Remove the patch "Add support for RTL907XD-VA PCIe port".

v3 -> v4:
- Modify commit message to describe the main reason for the fix.

Justin Lai (4):
  rtase: Refactor the rtase_check_mac_version_valid() function
  rtase: Correct the speed for RTL907XD-V1
  rtase: Corrects error handling of the rtase_check_mac_version_valid()
  rtase: Add defines for hardware version id

 drivers/net/ethernet/realtek/rtase/rtase.h    |  7 ++-
 .../net/ethernet/realtek/rtase/rtase_main.c   | 43 +++++++++++++------
 2 files changed, 36 insertions(+), 14 deletions(-)

-- 
2.34.1

[PATCH net v4 1/4] rtase: Refactor the rtase_check_mac_version_valid() function

[Justin Lai]

Different hardware requires different configurations, and this distinction
was not made previously. Additionally, the error message was not clear
enough. Therefore, in this patch, the following changes have been made to
address these issues:
1. Sets tp->hw_ver.
2. Changes the return type from bool to int.
3. Modify the error message for an invalid hardware version id.

Fixes: a36e9f5cfe9e ("rtase: Add support for a pci table in this module")
Signed-off-by: Justin Lai <justinlai0215@realtek.com>
---
 drivers/net/ethernet/realtek/rtase/rtase.h    |  2 ++
 .../net/ethernet/realtek/rtase/rtase_main.c   | 22 +++++++++++--------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/realtek/rtase/rtase.h b/drivers/net/ethernet/realtek/rtase/rtase.h
index 583c33930f88..547c71937b01 100644
--- a/drivers/net/ethernet/realtek/rtase/rtase.h
+++ b/drivers/net/ethernet/realtek/rtase/rtase.h
@@ -327,6 +327,8 @@ struct rtase_private {
 	u16 int_nums;
 	u16 tx_int_mit;
 	u16 rx_int_mit;
+
+	u32 hw_ver;
 };
 
 #define RTASE_LSO_64K 64000
diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c b/drivers/net/ethernet/realtek/rtase/rtase_main.c
index f8777b7663d3..0c19c5645d53 100644
--- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
+++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
@@ -1972,20 +1972,21 @@ static void rtase_init_software_variable(struct pci_dev *pdev,
 	tp->dev->max_mtu = RTASE_MAX_JUMBO_SIZE;
 }
 
-static bool rtase_check_mac_version_valid(struct rtase_private *tp)
+static int rtase_check_mac_version_valid(struct rtase_private *tp)
 {
-	u32 hw_ver = rtase_r32(tp, RTASE_TX_CONFIG_0) & RTASE_HW_VER_MASK;
-	bool known_ver = false;
+	int ret = -ENODEV;
 
-	switch (hw_ver) {
+	tp->hw_ver = rtase_r32(tp, RTASE_TX_CONFIG_0) & RTASE_HW_VER_MASK;
+
+	switch (tp->hw_ver) {
 	case 0x00800000:
 	case 0x04000000:
 	case 0x04800000:
-		known_ver = true;
+		ret = 0;
 		break;
 	}
 
-	return known_ver;
+	return ret;
 }
 
 static int rtase_init_board(struct pci_dev *pdev, struct net_device **dev_out,
@@ -2105,9 +2106,12 @@ static int rtase_init_one(struct pci_dev *pdev,
 	tp->pdev = pdev;
 
 	/* identify chip attached to board */
-	if (!rtase_check_mac_version_valid(tp))
-		return dev_err_probe(&pdev->dev, -ENODEV,
-				     "unknown chip version, contact rtase maintainers (see MAINTAINERS file)\n");
+	ret = rtase_check_mac_version_valid(tp);
+	if (ret != 0) {
+		dev_err(&pdev->dev,
+			"unknown chip version: 0x%08x, contact rtase maintainers (see MAINTAINERS file)\n",
+			tp->hw_ver);
+	}
 
 	rtase_init_software_variable(pdev, tp);
 	rtase_init_hardware(tp);
-- 
2.34.1

[PATCH net v4 2/4] rtase: Correct the speed for RTL907XD-V1

[Justin Lai]

Previously, the reported speed was uniformly set to SPEED_5000, but the
RTL907XD-V1 actually operates at a speed of SPEED_10000. Therefore, this
patch makes the necessary correction.

Fixes: dd7f17c40fd1 ("rtase: Implement ethtool function")
Signed-off-by: Justin Lai <justinlai0215@realtek.com>
---
 drivers/net/ethernet/realtek/rtase/rtase_main.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c b/drivers/net/ethernet/realtek/rtase/rtase_main.c
index 0c19c5645d53..5b8012987ea6 100644
--- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
+++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
@@ -1714,10 +1714,21 @@ static int rtase_get_settings(struct net_device *dev,
 			      struct ethtool_link_ksettings *cmd)
 {
 	u32 supported = SUPPORTED_MII | SUPPORTED_Pause | SUPPORTED_Asym_Pause;
+	const struct rtase_private *tp = netdev_priv(dev);
 
 	ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
 						supported);
-	cmd->base.speed = SPEED_5000;
+
+	switch (tp->hw_ver) {
+	case 0x00800000:
+	case 0x04000000:
+		cmd->base.speed = SPEED_5000;
+		break;
+	case 0x04800000:
+		cmd->base.speed = SPEED_10000;
+		break;
+	}
+
 	cmd->base.duplex = DUPLEX_FULL;
 	cmd->base.port = PORT_MII;
 	cmd->base.autoneg = AUTONEG_DISABLE;
-- 
2.34.1

[PATCH net v4 3/4] rtase: Corrects error handling of the rtase_check_mac_version_valid()

[Justin Lai]

Previously, when the hardware version ID was determined to be invalid,
only an error message was printed without any further handling. Therefore,
this patch makes the necessary corrections to address this.

Fixes: a36e9f5cfe9e ("rtase: Add support for a pci table in this module")
Signed-off-by: Justin Lai <justinlai0215@realtek.com>
---
 drivers/net/ethernet/realtek/rtase/rtase_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c b/drivers/net/ethernet/realtek/rtase/rtase_main.c
index 5b8012987ea6..26331a2b7b2d 100644
--- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
+++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
@@ -2122,6 +2122,7 @@ static int rtase_init_one(struct pci_dev *pdev,
 		dev_err(&pdev->dev,
 			"unknown chip version: 0x%08x, contact rtase maintainers (see MAINTAINERS file)\n",
 			tp->hw_ver);
+		goto err_out_release_board;
 	}
 
 	rtase_init_software_variable(pdev, tp);
@@ -2196,6 +2197,7 @@ static int rtase_init_one(struct pci_dev *pdev,
 		netif_napi_del(&ivec->napi);
 	}
 
+err_out_release_board:
 	rtase_release_board(pdev, dev, ioaddr);
 
 	return ret;
-- 
2.34.1

[PATCH net v4 4/4] rtase: Add defines for hardware version id

[Justin Lai]

Add defines for hardware version id.

Signed-off-by: Justin Lai <justinlai0215@realtek.com>
---
 drivers/net/ethernet/realtek/rtase/rtase.h      |  5 ++++-
 drivers/net/ethernet/realtek/rtase/rtase_main.c | 12 ++++++------
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/realtek/rtase/rtase.h b/drivers/net/ethernet/realtek/rtase/rtase.h
index 547c71937b01..4a4434869b10 100644
--- a/drivers/net/ethernet/realtek/rtase/rtase.h
+++ b/drivers/net/ethernet/realtek/rtase/rtase.h
@@ -9,7 +9,10 @@
 #ifndef RTASE_H
 #define RTASE_H
 
-#define RTASE_HW_VER_MASK 0x7C800000
+#define RTASE_HW_VER_MASK     0x7C800000
+#define RTASE_HW_VER_906X_7XA 0x00800000
+#define RTASE_HW_VER_906X_7XC 0x04000000
+#define RTASE_HW_VER_907XD_V1 0x04800000
 
 #define RTASE_RX_DMA_BURST_256       4
 #define RTASE_TX_DMA_BURST_UNLIMITED 7
diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c b/drivers/net/ethernet/realtek/rtase/rtase_main.c
index 26331a2b7b2d..1bfe5ef40c52 100644
--- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
+++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
@@ -1720,11 +1720,11 @@ static int rtase_get_settings(struct net_device *dev,
 						supported);
 
 	switch (tp->hw_ver) {
-	case 0x00800000:
-	case 0x04000000:
+	case RTASE_HW_VER_906X_7XA:
+	case RTASE_HW_VER_906X_7XC:
 		cmd->base.speed = SPEED_5000;
 		break;
-	case 0x04800000:
+	case RTASE_HW_VER_907XD_V1:
 		cmd->base.speed = SPEED_10000;
 		break;
 	}
@@ -1990,9 +1990,9 @@ static int rtase_check_mac_version_valid(struct rtase_private *tp)
 	tp->hw_ver = rtase_r32(tp, RTASE_TX_CONFIG_0) & RTASE_HW_VER_MASK;
 
 	switch (tp->hw_ver) {
-	case 0x00800000:
-	case 0x04000000:
-	case 0x04800000:
+	case RTASE_HW_VER_906X_7XA:
+	case RTASE_HW_VER_906X_7XC:
+	case RTASE_HW_VER_907XD_V1:
 		ret = 0;
 		break;
 	}
-- 
2.34.1

Re: [PATCH net v4 4/4] rtase: Add defines for hardware version id

[Michal Kubiak]

On Tue, Nov 19, 2024 at 05:57:06PM +0800, Justin Lai wrote:
> Add defines for hardware version id.
> 
> Signed-off-by: Justin Lai <justinlai0215@realtek.com>
> ---
>  drivers/net/ethernet/realtek/rtase/rtase.h      |  5 ++++-
>  drivers/net/ethernet/realtek/rtase/rtase_main.c | 12 ++++++------
>  2 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/rtase/rtase.h b/drivers/net/ethernet/realtek/rtase/rtase.h
> index 547c71937b01..4a4434869b10 100644
> --- a/drivers/net/ethernet/realtek/rtase/rtase.h
> +++ b/drivers/net/ethernet/realtek/rtase/rtase.h
> @@ -9,7 +9,10 @@
>  #ifndef RTASE_H
>  #define RTASE_H
>  
> -#define RTASE_HW_VER_MASK 0x7C800000
> +#define RTASE_HW_VER_MASK     0x7C800000
> +#define RTASE_HW_VER_906X_7XA 0x00800000
> +#define RTASE_HW_VER_906X_7XC 0x04000000
> +#define RTASE_HW_VER_907XD_V1 0x04800000
>  
>  #define RTASE_RX_DMA_BURST_256       4
>  #define RTASE_TX_DMA_BURST_UNLIMITED 7
> diff --git a/drivers/net/ethernet/realtek/rtase/rtase_main.c b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> index 26331a2b7b2d..1bfe5ef40c52 100644
> --- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> +++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> @@ -1720,11 +1720,11 @@ static int rtase_get_settings(struct net_device *dev,
>  						supported);
>  
>  	switch (tp->hw_ver) {
> -	case 0x00800000:
> -	case 0x04000000:
> +	case RTASE_HW_VER_906X_7XA:
> +	case RTASE_HW_VER_906X_7XC:
>  		cmd->base.speed = SPEED_5000;
>  		break;
> -	case 0x04800000:
> +	case RTASE_HW_VER_907XD_V1:
>  		cmd->base.speed = SPEED_10000;
>  		break;
>  	}


This is new code added in the patch #2.
I understand that you want to have those preprocessor definitions in a
separate patch, but why does this patch have to be the last one?
If you had included this change before the patch #2, you would be able
to send the final version of the above code (with no intermediate
changes).

Thanks,
Michal

RE: [PATCH net v4 4/4] rtase: Add defines for hardware version id

[Justin Lai]

> 
> On Tue, Nov 19, 2024 at 05:57:06PM +0800, Justin Lai wrote:
> > Add defines for hardware version id.
> >
> > Signed-off-by: Justin Lai <justinlai0215@realtek.com>
> > ---
> >  drivers/net/ethernet/realtek/rtase/rtase.h      |  5 ++++-
> >  drivers/net/ethernet/realtek/rtase/rtase_main.c | 12 ++++++------
> >  2 files changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/realtek/rtase/rtase.h
> > b/drivers/net/ethernet/realtek/rtase/rtase.h
> > index 547c71937b01..4a4434869b10 100644
> > --- a/drivers/net/ethernet/realtek/rtase/rtase.h
> > +++ b/drivers/net/ethernet/realtek/rtase/rtase.h
> > @@ -9,7 +9,10 @@
> >  #ifndef RTASE_H
> >  #define RTASE_H
> >
> > -#define RTASE_HW_VER_MASK 0x7C800000
> > +#define RTASE_HW_VER_MASK     0x7C800000
> > +#define RTASE_HW_VER_906X_7XA 0x00800000 #define
> > +RTASE_HW_VER_906X_7XC 0x04000000 #define
> RTASE_HW_VER_907XD_V1
> > +0x04800000
> >
> >  #define RTASE_RX_DMA_BURST_256       4
> >  #define RTASE_TX_DMA_BURST_UNLIMITED 7 diff --git
> > a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > index 26331a2b7b2d..1bfe5ef40c52 100644
> > --- a/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > +++ b/drivers/net/ethernet/realtek/rtase/rtase_main.c
> > @@ -1720,11 +1720,11 @@ static int rtase_get_settings(struct net_device
> *dev,
> >                                               supported);
> >
> >       switch (tp->hw_ver) {
> > -     case 0x00800000:
> > -     case 0x04000000:
> > +     case RTASE_HW_VER_906X_7XA:
> > +     case RTASE_HW_VER_906X_7XC:
> >               cmd->base.speed = SPEED_5000;
> >               break;
> > -     case 0x04800000:
> > +     case RTASE_HW_VER_907XD_V1:
> >               cmd->base.speed = SPEED_10000;
> >               break;
> >       }
> 
> 
> This is new code added in the patch #2.
> I understand that you want to have those preprocessor definitions in a separate
> patch, but why does this patch have to be the last one?
> If you had included this change before the patch #2, you would be able to send
> the final version of the above code (with no intermediate changes).
> 
> Thanks,
> Michal

Hi Michal,

Thank you for your suggestion. Upon further consideration, I also
agree that the addition of the hardware version ID definitions should
be included before patch #2.

Justin