| Message ID | 20250428060225.1306986-2-faizal.abdul.rahim@linux.intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | igc: harmonize queue priority and add preemptible queue support | expand |
On 28/04/2025 9:02, Faizal Rahim wrote: > Consolidate TXDCTL-related macros for better organization and readability. > > Signed-off-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com> > --- > drivers/net/ethernet/intel/igc/igc.h | 6 ++++++ > drivers/net/ethernet/intel/igc/igc_base.h | 4 ---- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h > index 859a15e4ccba..e9d180eac015 100644 > --- a/drivers/net/ethernet/intel/igc/igc.h > +++ b/drivers/net/ethernet/intel/igc/igc.h > @@ -492,6 +492,12 @@ static inline u32 igc_rss_type(const union igc_adv_rx_desc *rx_desc) > #define IGC_RX_WTHRESH 4 > #define IGC_TX_WTHRESH 16 > > +/* Additional Transmit Descriptor Control definitions */ > +/* Ena specific Tx Queue */ > +#define IGC_TXDCTL_QUEUE_ENABLE 0x02000000 > +/* Transmit Software Flush */ > +#define IGC_TXDCTL_SWFLUSH 0x04000000 > + > #define IGC_RX_DMA_ATTR \ > (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING) > > diff --git a/drivers/net/ethernet/intel/igc/igc_base.h b/drivers/net/ethernet/intel/igc/igc_base.h > index 6320eabb72fe..4a56c634977b 100644 > --- a/drivers/net/ethernet/intel/igc/igc_base.h > +++ b/drivers/net/ethernet/intel/igc/igc_base.h > @@ -86,10 +86,6 @@ union igc_adv_rx_desc { > } wb; /* writeback */ > }; > > -/* Additional Transmit Descriptor Control definitions */ > -#define IGC_TXDCTL_QUEUE_ENABLE 0x02000000 /* Ena specific Tx Queue */ > -#define IGC_TXDCTL_SWFLUSH 0x04000000 /* Transmit Software Flush */ > - > /* Additional Receive Descriptor Control definitions */ > #define IGC_RXDCTL_QUEUE_ENABLE 0x02000000 /* Ena specific Rx Queue */ > #define IGC_RXDCTL_SWFLUSH 0x04000000 /* Receive Software Flush */ Is there an intrinsic value for moving these definitions from one H file to another? And if so, why move the Tx defs and leave the Rx defs where they are?
On 28/4/2025 3:01 pm, Ruinskiy, Dima wrote: > On 28/04/2025 9:02, Faizal Rahim wrote: >> Consolidate TXDCTL-related macros for better organization and readability. >> >> Signed-off-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com> >> --- >> drivers/net/ethernet/intel/igc/igc.h | 6 ++++++ >> drivers/net/ethernet/intel/igc/igc_base.h | 4 ---- >> 2 files changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/ >> intel/igc/igc.h >> index 859a15e4ccba..e9d180eac015 100644 >> --- a/drivers/net/ethernet/intel/igc/igc.h >> +++ b/drivers/net/ethernet/intel/igc/igc.h >> @@ -492,6 +492,12 @@ static inline u32 igc_rss_type(const union >> igc_adv_rx_desc *rx_desc) >> #define IGC_RX_WTHRESH 4 >> #define IGC_TX_WTHRESH 16 >> +/* Additional Transmit Descriptor Control definitions */ >> +/* Ena specific Tx Queue */ >> +#define IGC_TXDCTL_QUEUE_ENABLE 0x02000000 >> +/* Transmit Software Flush */ >> +#define IGC_TXDCTL_SWFLUSH 0x04000000 >> + >> #define IGC_RX_DMA_ATTR \ >> (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING) >> diff --git a/drivers/net/ethernet/intel/igc/igc_base.h b/drivers/net/ >> ethernet/intel/igc/igc_base.h >> index 6320eabb72fe..4a56c634977b 100644 >> --- a/drivers/net/ethernet/intel/igc/igc_base.h >> +++ b/drivers/net/ethernet/intel/igc/igc_base.h >> @@ -86,10 +86,6 @@ union igc_adv_rx_desc { >> } wb; /* writeback */ >> }; >> -/* Additional Transmit Descriptor Control definitions */ >> -#define IGC_TXDCTL_QUEUE_ENABLE 0x02000000 /* Ena specific Tx Queue */ >> -#define IGC_TXDCTL_SWFLUSH 0x04000000 /* Transmit Software Flush */ >> - >> /* Additional Receive Descriptor Control definitions */ >> #define IGC_RXDCTL_QUEUE_ENABLE 0x02000000 /* Ena specific Rx Queue */ >> #define IGC_RXDCTL_SWFLUSH 0x04000000 /* Receive Software Flush */ > > Is there an intrinsic value for moving these definitions from one H file to > another? And if so, why move the Tx defs and leave the Rx defs where they are? Hi Dima, I moved and refactored the TXDCTL macros because this patch series modifies the TXDCTL register, specifically setting new bitfields. Consolidating `IGC_TXDCTL_QUEUE_ENABLE` and `IGC_TXDCTL_SWFLUSH` alongside the existing TXDCTL macros improves readability and makes it easier to cross-reference the TXDCTL bitfield mapping, as documented in Section 8.11.16 of the i226 Software User Manual. The grouping now matches that layout directly: #define IGC_TXDCTL_PTHRESH_MASK GENMASK(4, 0) #define IGC_TXDCTL_HTHRESH_MASK GENMASK(12, 8) #define IGC_TXDCTL_WTHRESH_MASK GENMASK(20, 16) #define IGC_TXDCTL_QUEUE_ENABLE_MASK GENMASK(25, 25) #define IGC_TXDCTL_SWFLUSH_MASK GENMASK(26, 26) #define IGC_TXDCTL_PRIORITY_MASK GENMASK(27, 27) Since RXDCTL is a separate register with its own bitfield mapping section in the i226 Software User Manual, and this patch series does not modify it, I left the RXDCTL macros untouched. They are used independently in separate functions within the driver. That said, I can move the RXDCTL macros as well for consistency.
diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h index 859a15e4ccba..e9d180eac015 100644 --- a/drivers/net/ethernet/intel/igc/igc.h +++ b/drivers/net/ethernet/intel/igc/igc.h @@ -492,6 +492,12 @@ static inline u32 igc_rss_type(const union igc_adv_rx_desc *rx_desc) #define IGC_RX_WTHRESH 4 #define IGC_TX_WTHRESH 16 +/* Additional Transmit Descriptor Control definitions */ +/* Ena specific Tx Queue */ +#define IGC_TXDCTL_QUEUE_ENABLE 0x02000000 +/* Transmit Software Flush */ +#define IGC_TXDCTL_SWFLUSH 0x04000000 + #define IGC_RX_DMA_ATTR \ (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING) diff --git a/drivers/net/ethernet/intel/igc/igc_base.h b/drivers/net/ethernet/intel/igc/igc_base.h index 6320eabb72fe..4a56c634977b 100644 --- a/drivers/net/ethernet/intel/igc/igc_base.h +++ b/drivers/net/ethernet/intel/igc/igc_base.h @@ -86,10 +86,6 @@ union igc_adv_rx_desc { } wb; /* writeback */ }; -/* Additional Transmit Descriptor Control definitions */ -#define IGC_TXDCTL_QUEUE_ENABLE 0x02000000 /* Ena specific Tx Queue */ -#define IGC_TXDCTL_SWFLUSH 0x04000000 /* Transmit Software Flush */ - /* Additional Receive Descriptor Control definitions */ #define IGC_RXDCTL_QUEUE_ENABLE 0x02000000 /* Ena specific Rx Queue */ #define IGC_RXDCTL_SWFLUSH 0x04000000 /* Receive Software Flush */
Consolidate TXDCTL-related macros for better organization and readability. Signed-off-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com> --- drivers/net/ethernet/intel/igc/igc.h | 6 ++++++ drivers/net/ethernet/intel/igc/igc_base.h | 4 ---- 2 files changed, 6 insertions(+), 4 deletions(-)