All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] igb: add missing fields to TXDCTL-register
@ 2016-10-19 12:37 ` Henrik Austad
  0 siblings, 0 replies; 6+ messages in thread
From: Henrik Austad @ 2016-10-19 12:37 UTC (permalink / raw)
  To: linux-kernel; +Cc: Henrik Austad, Jeff Kirsher, intel-wired-lan

The current list of E1000_TXDCTL-registers is incomplete. This adds the
missing parts for the Transmit Descriptor Control (TXDCTL) register.

The rest of these values (threshold for descriptor read/write) for
TXDCTL seems to be defined in igb/igb.h, not sure why this is split
though.

It seems that this was left out in the commit that added support for
82575 Gigabit Ethernet driver 9d5c8243 (igb: PCI-Express 82575 Gigabit
Ethernet driver).

Signed-off-by: Henrik Austad <henrik@austad.us>
Cc: linux-kernel@vger.kernel.org
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: intel-wired-lan@lists.osuosl.org
Signed-off-by: Henrik Austad <henrik@austad.us>
---
 drivers/net/ethernet/intel/igb/e1000_82575.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.h b/drivers/net/ethernet/intel/igb/e1000_82575.h
index 199ff98..212dbb8 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.h
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.h
@@ -158,7 +158,11 @@ struct e1000_adv_tx_context_desc {

 /* Additional Transmit Descriptor Control definitions */
 #define E1000_TXDCTL_QUEUE_ENABLE  0x02000000 /* Enable specific Tx Queue */
+
+/* Transmit Software Flush, sw-triggered desc writeback */
+#define E1000_TXDCTL_SWFLSH        0x04000000
 /* Tx Queue Arbitration Priority 0=low, 1=high */
+#define E1000_TXDCTL_PRIORITY      0x08000000

 /* Additional Receive Descriptor Control definitions */
 #define E1000_RXDCTL_QUEUE_ENABLE  0x02000000 /* Enable specific Rx Queue */
--
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Intel-wired-lan] [PATCH] igb: add missing fields to TXDCTL-register
@ 2016-10-19 12:37 ` Henrik Austad
  0 siblings, 0 replies; 6+ messages in thread
From: Henrik Austad @ 2016-10-19 12:37 UTC (permalink / raw)
  To: intel-wired-lan

The current list of E1000_TXDCTL-registers is incomplete. This adds the
missing parts for the Transmit Descriptor Control (TXDCTL) register.

The rest of these values (threshold for descriptor read/write) for
TXDCTL seems to be defined in igb/igb.h, not sure why this is split
though.

It seems that this was left out in the commit that added support for
82575 Gigabit Ethernet driver 9d5c8243 (igb: PCI-Express 82575 Gigabit
Ethernet driver).

Signed-off-by: Henrik Austad <henrik@austad.us>
Cc: linux-kernel at vger.kernel.org
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: intel-wired-lan at lists.osuosl.org
Signed-off-by: Henrik Austad <henrik@austad.us>
---
 drivers/net/ethernet/intel/igb/e1000_82575.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.h b/drivers/net/ethernet/intel/igb/e1000_82575.h
index 199ff98..212dbb8 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.h
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.h
@@ -158,7 +158,11 @@ struct e1000_adv_tx_context_desc {

 /* Additional Transmit Descriptor Control definitions */
 #define E1000_TXDCTL_QUEUE_ENABLE  0x02000000 /* Enable specific Tx Queue */
+
+/* Transmit Software Flush, sw-triggered desc writeback */
+#define E1000_TXDCTL_SWFLSH        0x04000000
 /* Tx Queue Arbitration Priority 0=low, 1=high */
+#define E1000_TXDCTL_PRIORITY      0x08000000

 /* Additional Receive Descriptor Control definitions */
 #define E1000_RXDCTL_QUEUE_ENABLE  0x02000000 /* Enable specific Rx Queue */
--
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Intel-wired-lan] [PATCH] igb: add missing fields to TXDCTL-register
  2016-10-19 12:37 ` [Intel-wired-lan] " Henrik Austad
@ 2016-10-19 14:25   ` Jesse Brandeburg
  -1 siblings, 0 replies; 6+ messages in thread
From: Jesse Brandeburg @ 2016-10-19 14:25 UTC (permalink / raw)
  To: Henrik Austad; +Cc: linux-kernel, intel-wired-lan

On Wed, 19 Oct 2016 14:37:59 +0200
Henrik Austad <henrik@austad.us> wrote:

> The current list of E1000_TXDCTL-registers is incomplete. This adds
> the missing parts for the Transmit Descriptor Control (TXDCTL)
> register.
> 
> The rest of these values (threshold for descriptor read/write) for
> TXDCTL seems to be defined in igb/igb.h, not sure why this is split
> though.

Hi Henrik, thanks for helping with our code.

While totally correct, having defines added to the kernel that are not
being used anywhere in the code isn't really very useful.  Often the
upstream maintainers/reviewers will reject a patch like this that just
adds to a .h file, because there are no actual users of the defines.

If the transmit or ethtool code were to use these (via the same patch)
or something like that, then the patch would be more likely to be
accepted.

Jesse

PS In the future no need to copy linux-kernel for patches going to our
submaintainer list.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Intel-wired-lan] [PATCH] igb: add missing fields to TXDCTL-register
@ 2016-10-19 14:25   ` Jesse Brandeburg
  0 siblings, 0 replies; 6+ messages in thread
From: Jesse Brandeburg @ 2016-10-19 14:25 UTC (permalink / raw)
  To: intel-wired-lan

On Wed, 19 Oct 2016 14:37:59 +0200
Henrik Austad <henrik@austad.us> wrote:

> The current list of E1000_TXDCTL-registers is incomplete. This adds
> the missing parts for the Transmit Descriptor Control (TXDCTL)
> register.
> 
> The rest of these values (threshold for descriptor read/write) for
> TXDCTL seems to be defined in igb/igb.h, not sure why this is split
> though.

Hi Henrik, thanks for helping with our code.

While totally correct, having defines added to the kernel that are not
being used anywhere in the code isn't really very useful.  Often the
upstream maintainers/reviewers will reject a patch like this that just
adds to a .h file, because there are no actual users of the defines.

If the transmit or ethtool code were to use these (via the same patch)
or something like that, then the patch would be more likely to be
accepted.

Jesse

PS In the future no need to copy linux-kernel for patches going to our
submaintainer list.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Intel-wired-lan] [PATCH] igb: add missing fields to TXDCTL-register
  2016-10-19 14:25   ` Jesse Brandeburg
@ 2016-10-19 15:08     ` Henrik Austad
  -1 siblings, 0 replies; 6+ messages in thread
From: Henrik Austad @ 2016-10-19 15:08 UTC (permalink / raw)
  To: Jesse Brandeburg; +Cc: linux-kernel, intel-wired-lan

[-- Attachment #1: Type: text/plain, Size: 1681 bytes --]

On Wed, Oct 19, 2016 at 07:25:10AM -0700, Jesse Brandeburg wrote:
> On Wed, 19 Oct 2016 14:37:59 +0200
> Henrik Austad <henrik@austad.us> wrote:
> 
> > The current list of E1000_TXDCTL-registers is incomplete. This adds
> > the missing parts for the Transmit Descriptor Control (TXDCTL)
> > register.
> > 
> > The rest of these values (threshold for descriptor read/write) for
> > TXDCTL seems to be defined in igb/igb.h, not sure why this is split
> > though.
> 
> Hi Henrik, thanks for helping with our code.
> 
> While totally correct, having defines added to the kernel that are not
> being used anywhere in the code isn't really very useful.  Often the
> upstream maintainers/reviewers will reject a patch like this that just
> adds to a .h file, because there are no actual users of the defines.

Yes, I agree, best to avoid bloat whenever possible.

> If the transmit or ethtool code were to use these (via the same patch)
> or something like that, then the patch would be more likely to be
> accepted.

Ah, good to know. I am in the process of spinning out a new set of 
TSN-patches (previous version: see [1]) and setting the priority-bit for 
the Tx-queues is required. This means that I'm hacking more at igb_main.c.

So this was more about laying the groundwork for the series.

I'll leave this patch in the tsn-series then, and resend once I'm ready and 
hope you can provide some feedback on the rest of the series then :)

> Jesse
> 
> PS In the future no need to copy linux-kernel for patches going to our
> submaintainer list.

Ok, I'll remember that, thanks!

1) https://lkml.org/lkml/2016/6/11/187

-- 
Henrik Austad

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Intel-wired-lan] [PATCH] igb: add missing fields to TXDCTL-register
@ 2016-10-19 15:08     ` Henrik Austad
  0 siblings, 0 replies; 6+ messages in thread
From: Henrik Austad @ 2016-10-19 15:08 UTC (permalink / raw)
  To: intel-wired-lan

On Wed, Oct 19, 2016 at 07:25:10AM -0700, Jesse Brandeburg wrote:
> On Wed, 19 Oct 2016 14:37:59 +0200
> Henrik Austad <henrik@austad.us> wrote:
> 
> > The current list of E1000_TXDCTL-registers is incomplete. This adds
> > the missing parts for the Transmit Descriptor Control (TXDCTL)
> > register.
> > 
> > The rest of these values (threshold for descriptor read/write) for
> > TXDCTL seems to be defined in igb/igb.h, not sure why this is split
> > though.
> 
> Hi Henrik, thanks for helping with our code.
> 
> While totally correct, having defines added to the kernel that are not
> being used anywhere in the code isn't really very useful.  Often the
> upstream maintainers/reviewers will reject a patch like this that just
> adds to a .h file, because there are no actual users of the defines.

Yes, I agree, best to avoid bloat whenever possible.

> If the transmit or ethtool code were to use these (via the same patch)
> or something like that, then the patch would be more likely to be
> accepted.

Ah, good to know. I am in the process of spinning out a new set of 
TSN-patches (previous version: see [1]) and setting the priority-bit for 
the Tx-queues is required. This means that I'm hacking more at igb_main.c.

So this was more about laying the groundwork for the series.

I'll leave this patch in the tsn-series then, and resend once I'm ready and 
hope you can provide some feedback on the rest of the series then :)

> Jesse
> 
> PS In the future no need to copy linux-kernel for patches going to our
> submaintainer list.

Ok, I'll remember that, thanks!

1) https://lkml.org/lkml/2016/6/11/187

-- 
Henrik Austad
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20161019/8eb0f082/attachment.asc>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-10-19 15:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-19 12:37 [PATCH] igb: add missing fields to TXDCTL-register Henrik Austad
2016-10-19 12:37 ` [Intel-wired-lan] " Henrik Austad
2016-10-19 14:25 ` Jesse Brandeburg
2016-10-19 14:25   ` Jesse Brandeburg
2016-10-19 15:08   ` Henrik Austad
2016-10-19 15:08     ` Henrik Austad

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.