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

Justin Lai posted 4 patches 3 days, 12 hours ago
There is a newer version of this series
[PATCH net v3 4/4] rtase: Add defines for hardware version id
Posted by Justin Lai 3 days, 12 hours ago
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 v3 4/4] rtase: Add defines for hardware version id
Posted by Michal Kubiak 3 days, 5 hours ago
On Mon, Nov 18, 2024 at 12:08:28PM +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(-)
> 

The patch is addressed to the "net" tree, but "Fixes" tag is missing.
Also, the patch does not look like a bugfix it's rather an improvement
of coding style with no functional changes. That's why I doubt it should go
to the "net" tree.

Thanks,
Michal
RE: [PATCH net v3 4/4] rtase: Add defines for hardware version id
Posted by Justin Lai 2 days, 9 hours ago
> 
> On Mon, Nov 18, 2024 at 12:08:28PM +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(-)
> >
> 
> The patch is addressed to the "net" tree, but "Fixes" tag is missing.
> Also, the patch does not look like a bugfix it's rather an improvement of coding
> style with no functional changes. That's why I doubt it should go to the "net"
> tree.
> 
> Thanks,
> Michal

Hi Michal,

This patch introduces multiple defines for hardware version IDs, rather
than addressing any issues related to the function logic, which is why it
does not include a "Fixes:" tag. The reason for isolating this change in a
separate patch is to maintain a "single patch, single purpose" approach.
Additionally, these defines will be used in other patches within the same
patch set, which is why they are included in this patch set as a whole.

Justin
Re: [PATCH net v3 4/4] rtase: Add defines for hardware version id
Posted by Michal Kubiak 2 days, 5 hours ago
On Tue, Nov 19, 2024 at 07:22:46AM +0000, Justin Lai wrote:
> > 
> > On Mon, Nov 18, 2024 at 12:08:28PM +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(-)
> > >
> > 
> > The patch is addressed to the "net" tree, but "Fixes" tag is missing.
> > Also, the patch does not look like a bugfix it's rather an improvement of coding
> > style with no functional changes. That's why I doubt it should go to the "net"
> > tree.
> > 
> > Thanks,
> > Michal
> 
> Hi Michal,
> 
> This patch introduces multiple defines for hardware version IDs, rather
> than addressing any issues related to the function logic, which is why it
> does not include a "Fixes:" tag. The reason for isolating this change in a
> separate patch is to maintain a "single patch, single purpose" approach.
> Additionally, these defines will be used in other patches within the same
> patch set, which is why they are included in this patch set as a whole.
> 
> Justin
> 

Hi Justin,

I understand the purpose of the patch, however the patch is addressed
to the "net" tree. Each patch sent to "net" tree should have "Fixes" tag,
because that tree is for fixes only.
You may consider sending the patch to the "net-next", (which is closed
now because of the merge window).

Thanks,
Michal
RE: [PATCH net v3 4/4] rtase: Add defines for hardware version id
Posted by Justin Lai 1 day, 10 hours ago
> 
> On Tue, Nov 19, 2024 at 07:22:46AM +0000, Justin Lai wrote:
> > >
> > > On Mon, Nov 18, 2024 at 12:08:28PM +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(-)
> > > >
> > >
> > > The patch is addressed to the "net" tree, but "Fixes" tag is missing.
> > > Also, the patch does not look like a bugfix it's rather an
> > > improvement of coding style with no functional changes. That's why I doubt
> it should go to the "net"
> > > tree.
> > >
> > > Thanks,
> > > Michal
> >
> > Hi Michal,
> >
> > This patch introduces multiple defines for hardware version IDs,
> > rather than addressing any issues related to the function logic, which
> > is why it does not include a "Fixes:" tag. The reason for isolating
> > this change in a separate patch is to maintain a "single patch, single
> purpose" approach.
> > Additionally, these defines will be used in other patches within the
> > same patch set, which is why they are included in this patch set as a whole.
> >
> > Justin
> >
> 
> Hi Justin,
> 
> I understand the purpose of the patch, however the patch is addressed to the
> "net" tree. Each patch sent to "net" tree should have "Fixes" tag, because that
> tree is for fixes only.
> You may consider sending the patch to the "net-next", (which is closed now
> because of the merge window).
> 
> Thanks,
> Michal

Hi Michal,

Thank you for your suggestion. Upon further consideration, I agree
that the addition of the hardware version ID definitions should have been
implemented directly when fixing the related issue. I will make the
necessary changes accordingly.

Justin