net: dsa: mt7530: prevent possible incorrect XTAL frequency selection

From: Arınç ÜNAL <[email protected]>

On MT7530, the HT_XTAL_FSEL field of the HWTRAP register stores a 2-bit value that represents the frequency of the crystal oscillator connected to the switch IC. The field is populated by the state of the ESW_P4_LED_0 and ESW_P4_LED_0 pins, which is done right after reset is deasserted.

ESW_P4_LED_0 ESW_P3_LED_0 Frequency
-----------------------------------------

  0               0               Reserved
  0               1               20MHz
  1               0               40MHz
  1               1               25MHz

On MT7531, the XTAL25 bit of the STRAP register stores this. The LAN0LED0 pin is used to populate the bit. 25MHz when the pin is high, 40MHz when it's low.

These pins are also used with LEDs, therefore, their state can be set to something other than the bootstrapping configuration. For example, a link may be established on port 3 before the DSA subdriver takes control of the switch which would set ESW_P3_LED_0 to high.

Currently on mt7530_setup() and mt7531_setup(), 1000 - 1100 usec delay is described between reset assertion and deassertion. Some switch ICs in real life conditions cannot always have these pins set back to the bootstrapping configuration before reset deassertion in this amount of delay. This causes wrong crystal frequency to be selected which puts the switch in a nonfunctional state after reset deassertion.

The tests below are conducted on an MT7530 with a 40MHz crystal oscillator by Justin Swartz.

With a cable from an active peer connected to port 3 before reset, an incorrect crystal frequency (0b11 = 25MHz) is selected:

                      [1]                  [3]     [5]
                      :                    :       :
              _____________________________         __________________
ESW_P4_LED_0                               |_______|
              _____________________________

ESW_P3_LED_0 |__________________________

                       :                  : :     :
                       :                  : [4]...:
                       :                  :
                       [2]................:

[1] Reset is asserted.
[2] Period of 1000 - 1100 usec.
[3] Reset is deasserted.
[4] Period of 315 usec. HWTRAP register is populated with incorrect XTAL frequency.
[5] Signals reflect the bootstrapped configuration.

Increase the delay between reset_control_assert() and reset_control_deassert(), and gpiod_set_value_cansleep(priv->reset, 0) and gpiod_set_value_cansleep(priv->reset, 1) to 5000 - 5100 usec. This amount ensures a higher possibility that the switch IC will have these pins back to the bootstrapping configuration before reset deassertion.

With a cable from an active peer connected to port 3 before reset, the correct crystal frequency (0b10 = 40MHz) is selected:

                      [1]        [2-1]     [3]     [5]
                      :          :         :       :
              _____________________________         __________________
ESW_P4_LED_0                               |_______|
              ___________________           _______

ESW_P3_LED_0 |_________| |__________________

                       :          :       : :     :
                       :          [2-2]...: [4]...:
                       [2]................:

[1] Reset is asserted.
[2] Period of 5000 - 5100 usec.
[2-1] ESW_P3_LED_0 goes low.
[2-2] Remaining period of 5000 - 5100 usec.
[3] Reset is deasserted.
[4] Period of 310 usec. HWTRAP register is populated with bootstrapped XTAL frequency.
[5] Signals reflect the bootstrapped configuration.

ESW_P3_LED_0 low period before reset deassertion:

5000 usec
- 5100 usec
TEST RESET HOLD
# (usec)
---------------------

       1           5410
       2           5440
       3           4375
       4           5490
       5           5475
       6           4335
       7           4370
       8           5435
       9           4205
      10           4335
      11           3750
      12           3170
      13           4395
      14           4375
      15           3515
      16           4335
      17           4220
      18           4175
      19           4175
      20           4350

Min 3170
Max 5490

Median 4342.500

     Avg       4466.500

Revert commit 2920dd92b980 ("net: dsa: mt7530: disable LEDs before reset"). Changing the state of pins via reset assertion is simpler and more efficient than doing so by setting the LED controller off.

---
 drivers/net/dsa/mt7530.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

index 678b51f9cea6..6986f538a4d0 100644
@@ -2192,22 +2192,16 @@ mt7530_setup(struct dsa_switch *ds)
 		}
 	}
 
-	/* Disable LEDs before reset to prevent the MT7530 sampling a
-	 * potentially incorrect HT_XTAL_FSEL value.
-	 */
-	mt7530_write(priv, MT7530_LED_EN, 0);
-	usleep_range(1000, 1100);
-
 	/* Reset whole chip through gpio pin or memory-mapped registers for
 	 * different type of hardware
 	 */
 	if (priv->mcm) {
 		reset_control_assert(priv->rstc);
-		usleep_range(1000, 1100);
+		usleep_range(5000, 5100);
 		reset_control_deassert(priv->rstc);
 	} else {
 		gpiod_set_value_cansleep(priv->reset, 0);
-		usleep_range(1000, 1100);
+		usleep_range(5000, 5100);
 		gpiod_set_value_cansleep(priv->reset, 1);
 	}
 

@@ -2420,11 +2414,11 @@ mt7531_setup(struct dsa_switch *ds)
 	 */
 	if (priv->mcm) {
 		reset_control_assert(priv->rstc);
-		usleep_range(1000, 1100);
+		usleep_range(5000, 5100);
 		reset_control_deassert(priv->rstc);
 	} else {
 		gpiod_set_value_cansleep(priv->reset, 0);
-		usleep_range(1000, 1100);
+		usleep_range(5000, 5100);
 		gpiod_set_value_cansleep(priv->reset, 1);
 	}
 

---
base-commit: d7d75124965aee23e5e4421d78376545cf070b0a
change-id: 20240312-for-netnext-mt7530-better-fix-xtal-frequency-31e6f599317f

Best regards,

Arınç ÜNAL <[email protected]>