Commit 5d54cb17 authored Feb 14, 2023 by Corinna Vinschen Committed by Jakub Kicinski Feb 15, 2023

igb: conditionalize I2C bit banging on external thermal sensor support



Commit a97f8783 ("igb: unbreak I2C bit-banging on i350") introduced
code to change I2C settings to bit banging unconditionally.

However, this patch introduced a regression:  On an Intel S2600CWR
Server Board with three NICs:

- 1x dual-port copper
  Intel I350 Gigabit Network Connection [8086:1521] (rev 01)
  fw 1.63, 0x80000dda

- 2x quad-port SFP+ with copper SFP Avago ABCU-5700RZ
  Intel I350 Gigabit Fiber Network Connection [8086:1522] (rev 01)
  fw 1.52.0

the SFP NICs no longer get link at all.  Reverting commit a97f8783
or switching to the Intel out-of-tree driver both fix the problem.

Per the igb out-of-tree driver, I2C bit banging on i350 depends on
support for an external thermal sensor (ETS).  However, commit
a97f8783 added bit banging unconditionally.  Additionally, the
out-of-tree driver always calls init_thermal_sensor_thresh on probe,
while our driver only calls init_thermal_sensor_thresh only in
igb_reset(), and only if an ETS is present, ignoring the internal
thermal sensor.  The affected SFPs don't provide an ETS.  Per Intel,
the behaviour is a result of i350 firmware requirements.

This patch fixes the problem by aligning the behaviour to the
out-of-tree driver:

- split igb_init_i2c() into two functions:
  - igb_init_i2c() only performs the basic I2C initialization.
  - igb_set_i2c_bb() makes sure that E1000_CTRL_I2C_ENA is set
    and enables bit-banging.

- igb_probe() only calls igb_set_i2c_bb() if an ETS is present.

- igb_probe() calls init_thermal_sensor_thresh() unconditionally.

- igb_reset() aligns its behaviour to igb_probe(), i. e., call
  igb_set_i2c_bb() if an ETS is present and call
  init_thermal_sensor_thresh() unconditionally.

Fixes: a97f8783 ("igb: unbreak I2C bit-banging on i350")
Tested-by: Mateusz Palczewski <mateusz.palczewski@intel.com>
Co-developed-by: Jamie Bainbridge <jbainbri@redhat.com>
Signed-off-by: Jamie Bainbridge <jbainbri@redhat.com>
Signed-off-by: Corinna Vinschen <vinschen@redhat.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
Link: https://lore.kernel.org/r/20230214185549.1306522-1-anthony.l.nguyen@intel.com


Signed-off-by: Jakub Kicinski <kuba@kernel.org>

parent dee4bf71

drivers/net/ethernet/intel/igb/igb_main.c

+32 −10

Original line number	Diff line number	Diff line
		@@ -2256,6 +2256,30 @@ static void igb_enable_mas(struct igb_adapter *adapter)
		}
		}

		#ifdef CONFIG_IGB_HWMON
		/**
		* igb_set_i2c_bb - Init I2C interface
		* @hw: pointer to hardware structure
		**/
		static void igb_set_i2c_bb(struct e1000_hw *hw)
		{
		u32 ctrl_ext;
		s32 i2cctl;

		ctrl_ext = rd32(E1000_CTRL_EXT);
		ctrl_ext \|= E1000_CTRL_I2C_ENA;
		wr32(E1000_CTRL_EXT, ctrl_ext);
		wrfl();

		i2cctl = rd32(E1000_I2CPARAMS);
		i2cctl \|= E1000_I2CBB_EN
		\| E1000_I2C_CLK_OE_N
		\| E1000_I2C_DATA_OE_N;
		wr32(E1000_I2CPARAMS, i2cctl);
		wrfl();
		}
		#endif

		void igb_reset(struct igb_adapter *adapter)
		{
		struct pci_dev *pdev = adapter->pdev;
		@@ -2400,6 +2424,7 @@ void igb_reset(struct igb_adapter *adapter)
		* interface.
		*/
		if (adapter->ets)
		igb_set_i2c_bb(hw);
		mac->ops.init_thermal_sensor_thresh(hw);
		}
		}
		@@ -3117,21 +3142,12 @@ static void igb_init_mas(struct igb_adapter *adapter)
		**/
		static s32 igb_init_i2c(struct igb_adapter *adapter)
		{
		struct e1000_hw *hw = &adapter->hw;
		s32 status = 0;
		s32 i2cctl;

		/* I2C interface supported on i350 devices */
		if (adapter->hw.mac.type != e1000_i350)
		return 0;

		i2cctl = rd32(E1000_I2CPARAMS);
		i2cctl \|= E1000_I2CBB_EN
		\| E1000_I2C_CLK_OUT \| E1000_I2C_CLK_OE_N
		\| E1000_I2C_DATA_OUT \| E1000_I2C_DATA_OE_N;
		wr32(E1000_I2CPARAMS, i2cctl);
		wrfl();

		/* Initialize the i2c bus which is controlled by the registers.
		* This bus will use the i2c_algo_bit structure that implements
		* the protocol through toggling of the 4 bits in the register.
		@@ -3521,6 +3537,12 @@ static int igb_probe(struct pci_dev pdev, const struct pci_device_id ent)
		adapter->ets = true;
		else
		adapter->ets = false;
		/* Only enable I2C bit banging if an external thermal
		* sensor is supported.
		*/
		if (adapter->ets)
		igb_set_i2c_bb(hw);
		hw->mac.ops.init_thermal_sensor_thresh(hw);
		if (igb_sysfs_init(adapter))
		dev_err(&pdev->dev,
		"failed to allocate sysfs resources\n");