Re: [PATCH 13/23] e1000: gather hardware bit tweaks.
Jeff Garzik wrote:
Kok, Auke wrote:
Several hardware bits were set all over the driver and have been
consolidated into a single function.
diff --git a/drivers/net/e1000/e1000_hw.c b/drivers/net/e1000/e1000_hw.c
index 9422864..a143b49 100644
Overall this is a positive change. However, NAK'd for two superficial
reasons:
1) the comments are completely useless. We can see from reading the
code what the comments tell us. What the comments DON'T tell us are (a)
a description of the bit being set, and (b) why that bit is being set.
2) the easy-for-humans-to-read notation for bit setting is
(1 << n)
not a hex constant.
reworked and added a whole bunch of comments. Here's what goes into the queue
instead:
the only part changed relative to my original submission is the new
e1000_initialize_hardware_bits function.
Auke
---
e1000: gather hardware bit tweaks.
From: Jeff Kirsher <[EMAIL PROTECTED]>
Several hardware bits were set all over the driver and have been
consolidated into a single function.
Signed-off-by: Jeff Kirsher <[EMAIL PROTECTED]>
Signed-off-by: Auke Kok <[EMAIL PROTECTED]>
---
drivers/net/e1000/e1000_hw.c | 155 ++--
drivers/net/e1000/e1000_hw.h |1
drivers/net/e1000/e1000_main.c | 24 +++---
3 files changed, 142 insertions(+), 38 deletions(-)
diff --git a/drivers/net/e1000/e1000_hw.c b/drivers/net/e1000/e1000_hw.c
index 6ec5cdd..dceaf5b 100644
--- a/drivers/net/e1000/e1000_hw.c
+++ b/drivers/net/e1000/e1000_hw.c
@@ -61,6 +61,7 @@ static int32_t e1000_id_led_init(struct
static int32_t e1000_init_lcd_from_nvm_config_region(struct e1000_hw *hw, uint32_t
cnf_base_addr, uint32_t cnf_size);
static int32_t e1000_init_lcd_from_nvm(struct e1000_hw *hw);
static void e1000_init_rx_addrs(struct e1000_hw *hw);
+static void e1000_initialize_hardware_bits(struct e1000_hw *hw);
static boolean_t e1000_is_onboard_nvm_eeprom(struct e1000_hw *hw);
static int32_t e1000_kumeran_lock_loss_workaround(struct e1000_hw *hw);
static int32_t e1000_mng_enable_host_if(struct e1000_hw *hw);
@@ -716,6 +717,123 @@ e1000_reset_hw(struct e1000_hw *hw)
}
/**
+ *
+ * Initialize a number of hardware-dependent bits
+ *
+ * hw: Struct containing variables accessed by shared code
+ *
+ * This function contains hardware limitation workarounds for PCI-E adapters
+ *
+ */
+static void
+e1000_initialize_hardware_bits(struct e1000_hw *hw)
+{
+if ((hw->mac_type >= e1000_82571) && (!hw->initialize_hw_bits_disable)) {
+/* Settings common to all PCI-express silicon */
+uint32_t reg_ctrl, reg_ctrl_ext;
+uint32_t reg_tarc0, reg_tarc1;
+uint32_t reg_tctl;
+uint32_t reg_txdctl, reg_txdctl1;
+
+/* link autonegotiation/sync workarounds */
+reg_tarc0 = E1000_READ_REG(hw, TARC0);
+reg_tarc0 &= ~((1 << 30)|(1 << 29)|(1 << 28)|(1 << 27));
+
+/* Enable not-done TX descriptor counting */
+reg_txdctl = E1000_READ_REG(hw, TXDCTL);
+reg_txdctl |= E1000_TXDCTL_COUNT_DESC;
+E1000_WRITE_REG(hw, TXDCTL, reg_txdctl);
+reg_txdctl1 = E1000_READ_REG(hw, TXDCTL1);
+reg_txdctl1 |= E1000_TXDCTL_COUNT_DESC;
+E1000_WRITE_REG(hw, TXDCTL1, reg_txdctl1);
+
+switch (hw->mac_type) {
+case e1000_82571:
+case e1000_82572:
+/* Clear PHY TX compatible mode bits */
+reg_tarc1 = E1000_READ_REG(hw, TARC1);
+reg_tarc1 &= ~((1 << 30)|(1 << 29));
+
+/* link autonegotiation/sync workarounds */
+reg_tarc0 |= ((1 << 26)|(1 << 25)|(1 << 24)|(1 << 23));
+
+/* TX ring control fixes */
+reg_tarc1 |= ((1 << 26)|(1 << 25)|(1 << 24));
+
+/* Multiple read bit is reversed polarity */
+reg_tctl = E1000_READ_REG(hw, TCTL);
+if (reg_tctl & E1000_TCTL_MULR)
+reg_tarc1 &= ~(1 << 28);
+else
+reg_tarc1 |= (1 << 28);
+
+E1000_WRITE_REG(hw, TARC1, reg_tarc1);
+break;
+case e1000_82573:
+reg_ctrl_ext = E1000_READ_REG(hw, CTRL_EXT);
+reg_ctrl_ext &= ~(1 << 23);
+reg_ctrl_ext |= (1 << 22);
+
+/* TX byte count fix */
+reg_ctrl = E1000_READ_REG(hw, CTRL);
+reg_ctrl &= ~(1 << 29);
+
+E1000_WRITE_REG(hw, CTRL_EXT, reg_ctrl_ext);
+E1000_WRITE_REG(hw, CTRL, reg_ctrl);
+break;
+case e1000_80003es2lan:
+/* improve small packet performace for fiber/serdes */
+if ((hw->media_type == e1000_media_type_fiber) ||
+(hw->media_type == e1000_media_type_internal_serdes)) {
+
Re: [PATCH 13/23] e1000: gather hardware bit tweaks.
Kok, Auke wrote:
Several hardware bits were set all over the driver and have been
consolidated into a single function.
diff --git a/drivers/net/e1000/e1000_hw.c b/drivers/net/e1000/e1000_hw.c
index 9422864..a143b49 100644
--- a/drivers/net/e1000/e1000_hw.c
+++ b/drivers/net/e1000/e1000_hw.c
@@ -61,6 +61,7 @@ static int32_t e1000_id_led_init(struct
static int32_t e1000_init_lcd_from_nvm_config_region(struct e1000_hw *hw, uint32_t cnf_base_addr, uint32_t cnf_size);
static int32_t e1000_init_lcd_from_nvm(struct e1000_hw *hw);
static void e1000_init_rx_addrs(struct e1000_hw *hw);
+static void e1000_initialize_hardware_bits(struct e1000_hw *hw);
static boolean_t e1000_is_onboard_nvm_eeprom(struct e1000_hw *hw);
static int32_t e1000_kumeran_lock_loss_workaround(struct e1000_hw *hw);
static int32_t e1000_mng_enable_host_if(struct e1000_hw *hw);
@@ -716,6 +717,110 @@ e1000_reset_hw(struct e1000_hw *hw)
}
/**
+ *
+ * Initialize a number of hardware-dependent bits
+ *
+ * hw: Struct containing variables accessed by shared code
+ *
+ */
+static void
+e1000_initialize_hardware_bits(struct e1000_hw *hw)
+{
+if ((hw->mac_type >= e1000_82571) && (!hw->initialize_hw_bits_disable)) {
+/* Settings common to all silicon */
+uint32_t reg_ctrl, reg_ctrl_ext;
+uint32_t reg_tarc0, reg_tarc1;
+uint32_t reg_tctl;
+uint32_t reg_txdctl, reg_txdctl1;
+
+reg_tarc0 = E1000_READ_REG(hw, TARC0);
+reg_tarc0 &= ~0x7800; /* Clear bits 30, 29, 28, and 27 */
+
+reg_txdctl = E1000_READ_REG(hw, TXDCTL);
+reg_txdctl |= E1000_TXDCTL_COUNT_DESC; /* Set bit 22 */
+E1000_WRITE_REG(hw, TXDCTL, reg_txdctl);
+
+reg_txdctl1 = E1000_READ_REG(hw, TXDCTL1);
+reg_txdctl1 |= E1000_TXDCTL_COUNT_DESC; /* Set bit 22 */
+E1000_WRITE_REG(hw, TXDCTL1, reg_txdctl1);
+
+switch (hw->mac_type) {
+case e1000_82571:
+case e1000_82572:
+reg_tarc1 = E1000_READ_REG(hw, TARC1);
+reg_tctl = E1000_READ_REG(hw, TCTL);
+
+/* Set the phy Tx compatible mode bits */
+reg_tarc1 &= ~0x6000; /* Clear bits 30 and 29 */
+
+reg_tarc0 |= 0x0780;/* Set TARC0 bits 23-26 */
+reg_tarc1 |= 0x0700;/* Set TARC1 bits 24-26 */
+
+if (reg_tctl & E1000_TCTL_MULR)
+reg_tarc1 &= ~0x1000; /* Clear bit 28 if MULR is 1b
*/
+else
+reg_tarc1 |= 0x1000;/* Set bit 28 if MULR is 0b */
+
+E1000_WRITE_REG(hw, TARC1, reg_tarc1);
+break;
+case e1000_82573:
+reg_ctrl_ext = E1000_READ_REG(hw, CTRL_EXT);
+reg_ctrl = E1000_READ_REG(hw, CTRL);
+
+reg_ctrl_ext &= ~0x0080;/* Clear bit 23 */
+reg_ctrl_ext |= 0x0040; /* Set bit 22 */
+reg_ctrl &= ~0x2000;/* Clear bit 29 */
+
+E1000_WRITE_REG(hw, CTRL_EXT, reg_ctrl_ext);
+E1000_WRITE_REG(hw, CTRL, reg_ctrl);
+break;
+case e1000_80003es2lan:
+if ((hw->media_type == e1000_media_type_fiber) ||
+(hw->media_type == e1000_media_type_internal_serdes)) {
+reg_tarc0 &= ~0x0010; /* Clear bit 20 */
+}
+
+reg_tctl = E1000_READ_REG(hw, TCTL);
+reg_tarc1 = E1000_READ_REG(hw, TARC1);
+if (reg_tctl & E1000_TCTL_MULR)
+reg_tarc1 &= ~0x1000; /* Clear bit 28 if MULR is 1b
*/
+else
+reg_tarc1 |= 0x1000;/* Set bit 28 if MULR is 0b */
+
+E1000_WRITE_REG(hw, TARC1, reg_tarc1);
+break;
+case e1000_ich8lan:
+if ((hw->revision_id < 3) ||
+((hw->device_id != E1000_DEV_ID_ICH8_IGP_M_AMT) &&
+ (hw->device_id != E1000_DEV_ID_ICH8_IGP_M)))
+reg_tarc0 |= 0x3000;/* Set TARC0 bits 29 and 28 */
+reg_ctrl_ext = E1000_READ_REG(hw, CTRL_EXT);
+reg_ctrl_ext |= 0x0040; /* Set bit 22 */
+E1000_WRITE_REG(hw, CTRL_EXT, reg_ctrl_ext);
+
+reg_tarc0 |= 0x0d80;/* Set TARC0 bits 23, 24, 26, 27 */
+
+reg_tarc1 = E1000_READ_REG(hw, TARC1);
+reg_tctl = E1000_READ_REG(hw, TCTL);
+
+if (reg_tctl & E1000_TCTL_MULR)
+reg_tarc1 &= ~0x1000; /* Clear bit 28 if MULR is 1b
*/
+else
+reg_tarc1 |= 0x1000;/* Set bit 28 if MULR is 0b */
+
+reg_tarc1 |= 0x4500;
[PATCH 13/23] e1000: gather hardware bit tweaks.
Several hardware bits were set all over the driver and have been
consolidated into a single function.
Signed-off-by: Jeff Kirsher <[EMAIL PROTECTED]>
Signed-off-by: Auke Kok <[EMAIL PROTECTED]>
---
drivers/net/e1000/e1000_hw.c | 142 +---
drivers/net/e1000/e1000_hw.h |1
drivers/net/e1000/e1000_main.c | 24 +++
3 files changed, 129 insertions(+), 38 deletions(-)
diff --git a/drivers/net/e1000/e1000_hw.c b/drivers/net/e1000/e1000_hw.c
index 9422864..a143b49 100644
--- a/drivers/net/e1000/e1000_hw.c
+++ b/drivers/net/e1000/e1000_hw.c
@@ -61,6 +61,7 @@ static int32_t e1000_id_led_init(struct
static int32_t e1000_init_lcd_from_nvm_config_region(struct e1000_hw *hw,
uint32_t cnf_base_addr, uint32_t cnf_size);
static int32_t e1000_init_lcd_from_nvm(struct e1000_hw *hw);
static void e1000_init_rx_addrs(struct e1000_hw *hw);
+static void e1000_initialize_hardware_bits(struct e1000_hw *hw);
static boolean_t e1000_is_onboard_nvm_eeprom(struct e1000_hw *hw);
static int32_t e1000_kumeran_lock_loss_workaround(struct e1000_hw *hw);
static int32_t e1000_mng_enable_host_if(struct e1000_hw *hw);
@@ -716,6 +717,110 @@ e1000_reset_hw(struct e1000_hw *hw)
}
/**
+ *
+ * Initialize a number of hardware-dependent bits
+ *
+ * hw: Struct containing variables accessed by shared code
+ *
+ */
+static void
+e1000_initialize_hardware_bits(struct e1000_hw *hw)
+{
+if ((hw->mac_type >= e1000_82571) && (!hw->initialize_hw_bits_disable)) {
+/* Settings common to all silicon */
+uint32_t reg_ctrl, reg_ctrl_ext;
+uint32_t reg_tarc0, reg_tarc1;
+uint32_t reg_tctl;
+uint32_t reg_txdctl, reg_txdctl1;
+
+reg_tarc0 = E1000_READ_REG(hw, TARC0);
+reg_tarc0 &= ~0x7800; /* Clear bits 30, 29, 28, and 27 */
+
+reg_txdctl = E1000_READ_REG(hw, TXDCTL);
+reg_txdctl |= E1000_TXDCTL_COUNT_DESC; /* Set bit 22 */
+E1000_WRITE_REG(hw, TXDCTL, reg_txdctl);
+
+reg_txdctl1 = E1000_READ_REG(hw, TXDCTL1);
+reg_txdctl1 |= E1000_TXDCTL_COUNT_DESC; /* Set bit 22 */
+E1000_WRITE_REG(hw, TXDCTL1, reg_txdctl1);
+
+switch (hw->mac_type) {
+case e1000_82571:
+case e1000_82572:
+reg_tarc1 = E1000_READ_REG(hw, TARC1);
+reg_tctl = E1000_READ_REG(hw, TCTL);
+
+/* Set the phy Tx compatible mode bits */
+reg_tarc1 &= ~0x6000; /* Clear bits 30 and 29 */
+
+reg_tarc0 |= 0x0780;/* Set TARC0 bits 23-26 */
+reg_tarc1 |= 0x0700;/* Set TARC1 bits 24-26 */
+
+if (reg_tctl & E1000_TCTL_MULR)
+reg_tarc1 &= ~0x1000; /* Clear bit 28 if MULR is 1b
*/
+else
+reg_tarc1 |= 0x1000;/* Set bit 28 if MULR is 0b */
+
+E1000_WRITE_REG(hw, TARC1, reg_tarc1);
+break;
+case e1000_82573:
+reg_ctrl_ext = E1000_READ_REG(hw, CTRL_EXT);
+reg_ctrl = E1000_READ_REG(hw, CTRL);
+
+reg_ctrl_ext &= ~0x0080;/* Clear bit 23 */
+reg_ctrl_ext |= 0x0040; /* Set bit 22 */
+reg_ctrl &= ~0x2000;/* Clear bit 29 */
+
+E1000_WRITE_REG(hw, CTRL_EXT, reg_ctrl_ext);
+E1000_WRITE_REG(hw, CTRL, reg_ctrl);
+break;
+case e1000_80003es2lan:
+if ((hw->media_type == e1000_media_type_fiber) ||
+(hw->media_type == e1000_media_type_internal_serdes)) {
+reg_tarc0 &= ~0x0010; /* Clear bit 20 */
+}
+
+reg_tctl = E1000_READ_REG(hw, TCTL);
+reg_tarc1 = E1000_READ_REG(hw, TARC1);
+if (reg_tctl & E1000_TCTL_MULR)
+reg_tarc1 &= ~0x1000; /* Clear bit 28 if MULR is 1b
*/
+else
+reg_tarc1 |= 0x1000;/* Set bit 28 if MULR is 0b */
+
+E1000_WRITE_REG(hw, TARC1, reg_tarc1);
+break;
+case e1000_ich8lan:
+if ((hw->revision_id < 3) ||
+((hw->device_id != E1000_DEV_ID_ICH8_IGP_M_AMT) &&
+ (hw->device_id != E1000_DEV_ID_ICH8_IGP_M)))
+reg_tarc0 |= 0x3000;/* Set TARC0 bits 29 and 28 */
+reg_ctrl_ext = E1000_READ_REG(hw, CTRL_EXT);
+reg_ctrl_ext |= 0x0040; /* Set bit 22 */
+E1000_WRITE_REG(hw, CTRL_EXT, reg_ctrl_ext);
+
+reg_tarc0 |= 0x0d80;/* Set TARC0 bits 23, 24, 26, 27 */
+
+reg_tarc1 = E1000_READ_REG(hw, TARC1);
+reg_tctl = E1000_READ_R
