[PATCH v3] doc: improve e1000 driver documentation

[Bruce Richardson]

On Fri, Feb 20, 2026 at 09:19:45AM -0800, Stephen Hemminger wrote:
> On Fri, 20 Feb 2026 21:15:13 +0000
> Talluri Chaitanyababu <chaitanyababux.talluri at intel.com> wrote:
> 
> > Update e1000 driver docs to cover all Intel 1G devices,
> > including emulated e1000 and real hardware.
> > 
> > v3: Resolved apply patch failure.
> > 
> > Signed-off-by: Talluri Chaitanyababu <chaitanyababux.talluri at intel.com>
> 
> Pointed AI review at this patch, and the overall e1000 driver documentation.
> 

I think the review is overly negative, but it does highlight some stuff
that should be fixed either in this patch or a different one.

> Here's my review of the patch against the existing documentation:
> 
> ---
> 
> **Patch:** `[PATCH v3] doc: improve e1000 driver documentation`
> **Author:** Talluri Chaitanyababu <chaitanyababux.talluri at intel.com>
> 
> **Summary:** The patch prepends a new top-level section "E1000 Poll Mode Driver" with an "Overview" subsection to `e1000em.rst`, but there are several issues.
> 
> ---
> 
> **Structural / RST Issues**
> 
> The patch inserts a new top-level title ("E1000 Poll Mode Driver", underlined with `=`) before the existing top-level title ("Driver for VM Emulated Devices", also underlined with `=`). RST doesn't allow two sections at the same heading level in a document without one being nested under the other. This creates a document with two competing top-level headings, which is likely to confuse Sphinx and render incorrectly. The patch should either replace the existing title or restructure so "Driver for VM Emulated Devices" becomes a subsection beneath the new top-level heading.
> 

This needs to be fixed in this patch.

> **Content Accuracy**
> 
> The note in the new section states: *"The `igb` and `igc` PMDs are built from the same driver binary but are documented in separate sections for clarity."* This is misleading. `igb` is a Linux kernel driver, not a DPDK PMD, and `igc` similarly refers to a kernel driver (for I225/I226). DPDK's 1G PMD for real hardware uses the `e1000` driver. The note should be removed or corrected — as written it introduces inaccurate information.
> 

I disagree with this. I think the content here is correct so far as it
goes, but maybe we need to expand and clarify a little?

> **Scope vs. Commit Message**
> 
> The commit message says "Update e1000 driver docs to cover all Intel 1G devices, including emulated e1000 and real hardware," but the patch only adds 16 lines of overview text and doesn't actually document real hardware (PCI IDs, configuration, feature support, etc.). The commit message overclaims what the patch delivers.
> 
> **Content of the Overview**
> 
> The bullet point for `e1000e` lists an extensive range of adapters (82563/6/7, 82571/2/3/4/7/8/9, 82583, I217/I218/I219) but the existing file is entirely focused on emulated/VM devices. Mixing in details about `e1000e` physical hardware here, without corresponding documentation for those devices, leaves the overview incomplete and potentially confusing to readers.
> 

Yes, this needs to be fixed, but not necessarily in this patch.

> **Minor Style Issues**
> 
> The note uses a Unicode non-breaking hyphen (`‑`) in "PCI‑Express" rather than a regular hyphen or the conventional "PCIe" abbreviation. This can cause rendering or copy-paste issues and should use standard ASCII.
> 

Needs fixing.

> **Existing Document Issues (pre-patch)**
> 
> Since you asked for a documentation review of the file as well, a few pre-existing issues worth noting:
> 
> - The document is severely outdated — it references Fedora 14/18, QEMU 0.14.0/0.15.1, KVM setup instructions from around 2011–2012, and kernel version 2.6.25. These sections should either be updated or removed.
> - Line 60 has a broken code block: `tar xzf qemu-kvm-release.tar.gz cd qemu-kvm-release` — the `cd` command is on the same line as `tar`, missing a newline.
> - The hyperlinks on lines 53, 81, 95–99 use the old `http://` scheme and some point to domains that are likely stale (sourceforge KVM project, `qemu.weilnetz.de`).
> - Line 90: "The Qemu" should be "QEMU" (it's an acronym, always uppercase in DPDK docs).
> - Lines 113, 114, 120: em-dashes used as list bullets (`—`) are non-standard RST; they should use standard `*` bullets or proper RST definition lists.
> 
> ---

These issue do need to be fixed, but again, not necessarily in this patch.
It would be good after this patch is respun to have a follow-up patch or
patchset cleaning up the rest of the doc.

Marking patch as "changes requested" in patchwork, because of the things I
highlight above that need fixing.

/Bruce