| Message ID | 20240906021719.37754-1-takamitz@amazon.co.jp (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2,net-next] e1000e: Remove duplicated writel() in e1000_configure_tx/rx() | expand |
On 06/09/2024 5:17, Takamitsu Iwai wrote: > Duplicated register initialization codes exist in e1000_configure_tx() > and e1000_configure_rx(). > > For example, writel(0, tx_ring->head) writes 0 to tx_ring->head, which > is adapter->hw.hw_addr + E1000_TDH(0). > > This initialization is already done in ew32(TDH(0), 0). > > ew32(TDH(0), 0) is equivalent to __ew32(hw, E1000_TDH(0), 0). It > executes writel(0, hw->hw_addr + E1000_TDH(0)). Since variable hw is > set to &adapter->hw, it is equal to writel(0, tx_ring->head). > > We can remove similar four writel() in e1000_configure_tx() and > e1000_configure_rx(). > > commit 0845d45e900c ("e1000e: Modify Tx/Rx configurations to avoid > null pointer dereferences in e1000_open") has introduced these > writel(). This commit moved register writing to > e1000_configure_tx/rx(), and as result, it caused duplication in > e1000_configure_tx/rx(). > > This patch modifies the sequence of register writing, but removing > these writes is safe because the same writes were already there before > the commit. > > I also have checked the datasheets [0] [1] and have not found any > description that we need to write RDH, RDT, TDH and TDT registers > twice at initialization. Furthermore, we have tested this patch on an > I219-V device physically. > > Link: https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/82577-gbe-phy-datasheet.pdf [0] > Link: https://www.intel.com/content/www/us/en/content-details/613460/intel-82583v-gbe-controller-datasheet.html [1] > Tested-by: Kohei Enju <enjuk@amazon.com> > Signed-off-by: Takamitsu Iwai <takamitz@amazon.co.jp> > --- > > v1->v2 > modify commit message to explain the reason why we can remove these writes safely. > > v1 link > https://lore.kernel.org/netdev/20240902061454.85744-1-takamitz@amazon.co.jp/ > > drivers/net/ethernet/intel/e1000e/netdev.c | 6 ------ > 1 file changed, 6 deletions(-) > Tested-by: Mor Bar-Gabay <morx.bar.gabay@intel.com>
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c index da5c59daf8ba..89c57be89c88 100644 --- a/drivers/net/ethernet/intel/e1000e/netdev.c +++ b/drivers/net/ethernet/intel/e1000e/netdev.c @@ -2928,11 +2928,8 @@ static void e1000_configure_tx(struct e1000_adapter *adapter) tx_ring->head = adapter->hw.hw_addr + E1000_TDH(0); tx_ring->tail = adapter->hw.hw_addr + E1000_TDT(0); - writel(0, tx_ring->head); if (adapter->flags2 & FLAG2_PCIM2PCI_ARBITER_WA) e1000e_update_tdt_wa(tx_ring, 0); - else - writel(0, tx_ring->tail); /* Set the Tx Interrupt Delay register */ ew32(TIDV, adapter->tx_int_delay); @@ -3253,11 +3250,8 @@ static void e1000_configure_rx(struct e1000_adapter *adapter) rx_ring->head = adapter->hw.hw_addr + E1000_RDH(0); rx_ring->tail = adapter->hw.hw_addr + E1000_RDT(0); - writel(0, rx_ring->head); if (adapter->flags2 & FLAG2_PCIM2PCI_ARBITER_WA) e1000e_update_rdt_wa(rx_ring, 0); - else - writel(0, rx_ring->tail); /* Enable Receive Checksum Offload for TCP and UDP */ rxcsum = er32(RXCSUM);
Duplicated register initialization codes exist in e1000_configure_tx() and e1000_configure_rx(). For example, writel(0, tx_ring->head) writes 0 to tx_ring->head, which is adapter->hw.hw_addr + E1000_TDH(0). This initialization is already done in ew32(TDH(0), 0). ew32(TDH(0), 0) is equivalent to __ew32(hw, E1000_TDH(0), 0). It executes writel(0, hw->hw_addr + E1000_TDH(0)). Since variable hw is set to &adapter->hw, it is equal to writel(0, tx_ring->head). We can remove similar four writel() in e1000_configure_tx() and e1000_configure_rx(). commit 0845d45e900c ("e1000e: Modify Tx/Rx configurations to avoid null pointer dereferences in e1000_open") has introduced these writel(). This commit moved register writing to e1000_configure_tx/rx(), and as result, it caused duplication in e1000_configure_tx/rx(). This patch modifies the sequence of register writing, but removing these writes is safe because the same writes were already there before the commit. I also have checked the datasheets [0] [1] and have not found any description that we need to write RDH, RDT, TDH and TDT registers twice at initialization. Furthermore, we have tested this patch on an I219-V device physically. Link: https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/82577-gbe-phy-datasheet.pdf [0] Link: https://www.intel.com/content/www/us/en/content-details/613460/intel-82583v-gbe-controller-datasheet.html [1] Tested-by: Kohei Enju <enjuk@amazon.com> Signed-off-by: Takamitsu Iwai <takamitz@amazon.co.jp> --- v1->v2 modify commit message to explain the reason why we can remove these writes safely. v1 link https://lore.kernel.org/netdev/20240902061454.85744-1-takamitz@amazon.co.jp/ drivers/net/ethernet/intel/e1000e/netdev.c | 6 ------ 1 file changed, 6 deletions(-)