| Message ID | 20241118040828.454861-5-justinlai0215@realtek.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Correcting switch hardware versions and reported speeds | expand |
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
> > 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
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
> > 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
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; }
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(-)