Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

phy: rockchip-typec: Avoid magic numbers + add delays in aux calib

NOTE: nothing is known to be fixed by this change, but it does enforce
some delays that are documented to be necessary. Possibly this could
fix some corner cases.

The function tcphy_dp_aux_calibration(), like most of the functions in
the type C PHY, is mostly undocumented and filled with mysterious,
hardcoded numbers.

Let's attempt to try to document some of these numbers and clean the
function up a little bit. Here's the actual cleanup that happened
here:

1. All magic numbers were replaced with bit definitions.

2. For registers that we modify multiple times I now keep track of the
value of the register rather than randomly doing a
read/modify/write or just hardcoding a new number based on knowing
what the old number was.

3. Delay 10 ms (vs 1 ms) after writing the calibration code. No idea
if this is important but it matches the example in the docs.

4. Whenever setting a "delayed" version of a signal always put an
explicit delay in the code. No known problems were seen without
this delay but it seems wise to have it. Whenever a delay of "at
least 100 ns" was specified I used a delay of 1 us.

5. Added comments to some of the bits of code.

6. Removed duplicate setting of TX_ANA_CTRL_REG_5 (to 0)

7. Moved setting of TX_ANA_CTRL_REG_3 to the same place it was in the
sample code. Note that TX_ANA_CTRL_REG_3 ought to be initted to 0
(and elsewhere we assume that we just got a reset), but it seems
fine to be explicit.

8. Treats the calibration code as a 7-bit two's complement number.
This isn't strictly required, but seems slightly cleaner. The docs
say "treat this as a two's complement number, but it should never
be negative". If we ever read the "adjustment" codes as documented
then perhaps the two's complement bit will matter more.

There are still a few weird / mysterious things around aux init and
this doesn't attempt to fix all of them. Mostly it's aimed at doing
changes that should be _very_ safe and add a lot of clarity. Things
specifically not done:

A) Resolve the fact that some registers are read/modify/write and
others are explicitly initted to a value. We always call
tcphy_dp_aux_calibration() right after resetting the PHY so it's
probably not critical, but it's a little weird that the code is
inconsistent.

B) Fully resolve the documented init sequence with the current one.
We still have a few mystery steps and we also leave out turning on
TXDA_DRV_LDO_BG_FB_EN and TXDA_DRV_LDO_BG_REF_EN, which is in the
sample code.

C) Clean things up to read all the bits of the calibration code. This
will hopefully come in a followup change.

This also doesn't attempt to document any of the other parts of the
PHY--just the aux init which is all I got docs for.

Reviewed-by: Chris Zhong <zyw@rock-chips.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>

authored by

Douglas Anderson and committed by
Kishon Vijay Abraham I
f85fd4c9 11a6e41c

+163 -39
+163 -39
drivers/phy/rockchip/phy-rockchip-typec.c
··· 102 102 #define CMN_PLL1_SS_CTRL1 (0xb8 << 2) 103 103 #define CMN_PLL1_SS_CTRL2 (0xb9 << 2) 104 104 #define CMN_RXCAL_OVRD (0xd1 << 2) 105 + 105 106 #define CMN_TXPUCAL_CTRL (0xe0 << 2) 106 107 #define CMN_TXPUCAL_OVRD (0xe1 << 2) 108 + #define CMN_TXPDCAL_CTRL (0xf0 << 2) 107 109 #define CMN_TXPDCAL_OVRD (0xf1 << 2) 110 + 111 + /* For CMN_TXPUCAL_CTRL, CMN_TXPDCAL_CTRL */ 112 + #define CMN_TXPXCAL_START BIT(15) 113 + #define CMN_TXPXCAL_DONE BIT(14) 114 + #define CMN_TXPXCAL_NO_RESPONSE BIT(13) 115 + #define CMN_TXPXCAL_CURRENT_RESPONSE BIT(12) 116 + 117 + #define CMN_TXPU_ADJ_CTRL (0x108 << 2) 118 + #define CMN_TXPD_ADJ_CTRL (0x10c << 2) 119 + 120 + /* 121 + * For CMN_TXPUCAL_CTRL, CMN_TXPDCAL_CTRL, 122 + * CMN_TXPU_ADJ_CTRL, CMN_TXPDCAL_CTRL 123 + * 124 + * NOTE: some of these registers are documented to be 2's complement 125 + * signed numbers, but then documented to be always positive. Weird. 126 + * In such a case, using CMN_CALIB_CODE_POS() avoids the unnecessary 127 + * sign extension. 128 + */ 129 + #define CMN_CALIB_CODE_WIDTH 7 130 + #define CMN_CALIB_CODE_OFFSET 0 131 + #define CMN_CALIB_CODE_MASK GENMASK(CMN_CALIB_CODE_WIDTH, 0) 132 + #define CMN_CALIB_CODE(x) \ 133 + sign_extend32((x) >> CMN_CALIB_CODE_OFFSET, CMN_CALIB_CODE_WIDTH) 134 + 135 + #define CMN_CALIB_CODE_POS_MASK GENMASK(CMN_CALIB_CODE_WIDTH - 1, 0) 136 + #define CMN_CALIB_CODE_POS(x) \ 137 + (((x) >> CMN_CALIB_CODE_OFFSET) & CMN_CALIB_CODE_POS_MASK) 138 + 108 139 #define CMN_DIAG_PLL0_FBH_OVRD (0x1c0 << 2) 109 140 #define CMN_DIAG_PLL0_FBL_OVRD (0x1c1 << 2) 110 141 #define CMN_DIAG_PLL0_OVRD (0x1c2 << 2) ··· 169 138 #define TX_TXCC_MGNFS_MULT_101(n) ((0x4055 | ((n) << 9)) << 2) 170 139 #define TX_TXCC_MGNFS_MULT_110(n) ((0x4056 | ((n) << 9)) << 2) 171 140 #define TX_TXCC_MGNFS_MULT_111(n) ((0x4057 | ((n) << 9)) << 2) 141 + #define TX_TXCC_MGNLS_MULT_000(n) ((0x4058 | ((n) << 9)) << 2) 142 + #define TX_TXCC_MGNLS_MULT_001(n) ((0x4059 | ((n) << 9)) << 2) 143 + #define TX_TXCC_MGNLS_MULT_010(n) ((0x405a | ((n) << 9)) << 2) 144 + #define TX_TXCC_MGNLS_MULT_011(n) ((0x405b | ((n) << 9)) << 2) 145 + #define TX_TXCC_MGNLS_MULT_100(n) ((0x405c | ((n) << 9)) << 2) 146 + #define TX_TXCC_MGNLS_MULT_101(n) ((0x405d | ((n) << 9)) << 2) 147 + #define TX_TXCC_MGNLS_MULT_110(n) ((0x405e | ((n) << 9)) << 2) 148 + #define TX_TXCC_MGNLS_MULT_111(n) ((0x405f | ((n) << 9)) << 2) 149 + 172 150 #define XCVR_DIAG_PLLDRC_CTRL(n) ((0x40e0 | ((n) << 9)) << 2) 173 151 #define XCVR_DIAG_BIDI_CTRL(n) ((0x40e8 | ((n) << 9)) << 2) 174 152 #define XCVR_DIAG_LANE_FCM_EN_MGN(n) ((0x40f2 | ((n) << 9)) << 2) ··· 190 150 #define TX_RCVDET_ST_TMR(n) ((0x4123 | ((n) << 9)) << 2) 191 151 #define TX_DIAG_TX_DRV(n) ((0x41e1 | ((n) << 9)) << 2) 192 152 #define TX_DIAG_BGREF_PREDRV_DELAY (0x41e7 << 2) 153 + 154 + /* Use this for "n" in macros like "_MULT_XXX" to target the aux channel */ 155 + #define AUX_CH_LANE 8 156 + 193 157 #define TX_ANA_CTRL_REG_1 (0x5020 << 2) 158 + 159 + #define TXDA_DP_AUX_EN BIT(15) 160 + #define AUXDA_SE_EN BIT(14) 161 + #define TXDA_CAL_LATCH_EN BIT(13) 162 + #define AUXDA_POLARITY BIT(12) 163 + #define TXDA_DRV_POWER_ISOLATION_EN BIT(11) 164 + #define TXDA_DRV_POWER_EN_PH_2_N BIT(10) 165 + #define TXDA_DRV_POWER_EN_PH_1_N BIT(9) 166 + #define TXDA_BGREF_EN BIT(8) 167 + #define TXDA_DRV_LDO_EN BIT(7) 168 + #define TXDA_DECAP_EN_DEL BIT(6) 169 + #define TXDA_DECAP_EN BIT(5) 170 + #define TXDA_UPHY_SUPPLY_EN_DEL BIT(4) 171 + #define TXDA_UPHY_SUPPLY_EN BIT(3) 172 + #define TXDA_LOW_LEAKAGE_EN BIT(2) 173 + #define TXDA_DRV_IDLE_LOWI_EN BIT(1) 174 + #define TXDA_DRV_CMN_MODE_EN BIT(0) 175 + 194 176 #define TX_ANA_CTRL_REG_2 (0x5021 << 2) 177 + 178 + #define AUXDA_DEBOUNCING_CLK BIT(15) 179 + #define TXDA_LPBK_RECOVERED_CLK_EN BIT(14) 180 + #define TXDA_LPBK_ISI_GEN_EN BIT(13) 181 + #define TXDA_LPBK_SERIAL_EN BIT(12) 182 + #define TXDA_LPBK_LINE_EN BIT(11) 183 + #define TXDA_DRV_LDO_REDC_SINKIQ BIT(10) 184 + #define XCVR_DECAP_EN_DEL BIT(9) 185 + #define XCVR_DECAP_EN BIT(8) 186 + #define TXDA_MPHY_ENABLE_HS_NT BIT(7) 187 + #define TXDA_MPHY_SA_MODE BIT(6) 188 + #define TXDA_DRV_LDO_RBYR_FB_EN BIT(5) 189 + #define TXDA_DRV_RST_PULL_DOWN BIT(4) 190 + #define TXDA_DRV_LDO_BG_FB_EN BIT(3) 191 + #define TXDA_DRV_LDO_BG_REF_EN BIT(2) 192 + #define TXDA_DRV_PREDRV_EN_DEL BIT(1) 193 + #define TXDA_DRV_PREDRV_EN BIT(0) 194 + 195 195 #define TXDA_COEFF_CALC_CTRL (0x5022 << 2) 196 + 197 + #define TX_HIGH_Z BIT(6) 198 + #define TX_VMARGIN_OFFSET 3 199 + #define TX_VMARGIN_MASK 0x7 200 + #define LOW_POWER_SWING_EN BIT(2) 201 + #define TX_FCM_DRV_MAIN_EN BIT(1) 202 + #define TX_FCM_FULL_MARGIN BIT(0) 203 + 196 204 #define TX_DIG_CTRL_REG_2 (0x5024 << 2) 205 + 206 + #define TX_HIGH_Z_TM_EN BIT(15) 207 + #define TX_RESCAL_CODE_OFFSET 0 208 + #define TX_RESCAL_CODE_MASK 0x3f 209 + 197 210 #define TXDA_CYA_AUXDA_CYA (0x5025 << 2) 198 211 #define TX_ANA_CTRL_REG_3 (0x5026 << 2) 199 212 #define TX_ANA_CTRL_REG_4 (0x5027 << 2) ··· 549 456 */ 550 457 tx_ana_ctrl_reg_1 = readl(tcphy->base + TX_ANA_CTRL_REG_1); 551 458 if (!tcphy->flip) 552 - tx_ana_ctrl_reg_1 |= BIT(12); 459 + tx_ana_ctrl_reg_1 |= AUXDA_POLARITY; 553 460 else 554 - tx_ana_ctrl_reg_1 &= ~BIT(12); 461 + tx_ana_ctrl_reg_1 &= ~AUXDA_POLARITY; 555 462 writel(tx_ana_ctrl_reg_1, tcphy->base + TX_ANA_CTRL_REG_1); 556 463 } 557 464 558 465 static void tcphy_dp_aux_calibration(struct rockchip_typec_phy *tcphy) 559 466 { 467 + u16 val; 560 468 u16 tx_ana_ctrl_reg_1; 561 - u16 rdata, rdata2, val; 469 + u16 tx_ana_ctrl_reg_2; 470 + s32 pu_calib_code; 562 471 563 472 /* disable txda_cal_latch_en for rewrite the calibration values */ 564 473 tx_ana_ctrl_reg_1 = readl(tcphy->base + TX_ANA_CTRL_REG_1); 565 - tx_ana_ctrl_reg_1 &= ~BIT(13); 474 + tx_ana_ctrl_reg_1 &= ~TXDA_CAL_LATCH_EN; 566 475 writel(tx_ana_ctrl_reg_1, tcphy->base + TX_ANA_CTRL_REG_1); 567 476 568 477 /* 569 - * read a resistor calibration code from CMN_TXPUCAL_CTRL[6:0] and 570 - * write it to TX_DIG_CTRL_REG_2[6:0], and delay 1ms to make sure it 571 - * works. 478 + * read a resistor calibration code from CMN_TXPUCAL_CTRL[5:0] and 479 + * write it to TX_DIG_CTRL_REG_2[5:0]. 572 480 */ 573 - rdata = readl(tcphy->base + TX_DIG_CTRL_REG_2); 574 - rdata = rdata & 0xffc0; 481 + val = readl(tcphy->base + CMN_TXPUCAL_CTRL); 482 + pu_calib_code = CMN_CALIB_CODE_POS(val); 575 483 576 - rdata2 = readl(tcphy->base + CMN_TXPUCAL_CTRL); 577 - rdata2 = rdata2 & 0x3f; 578 - 579 - val = rdata | rdata2; 484 + /* write the calibration, then delay 10 ms as sample in docs */ 485 + val = readl(tcphy->base + TX_DIG_CTRL_REG_2); 486 + val &= ~(TX_RESCAL_CODE_MASK << TX_RESCAL_CODE_OFFSET); 487 + val |= pu_calib_code << TX_RESCAL_CODE_OFFSET; 580 488 writel(val, tcphy->base + TX_DIG_CTRL_REG_2); 581 - usleep_range(1000, 1050); 489 + usleep_range(10000, 10050); 582 490 583 491 /* 584 492 * Enable signal for latch that sample and holds calibration values. 585 493 * Activate this signal for 1 clock cycle to sample new calibration 586 494 * values. 587 495 */ 588 - tx_ana_ctrl_reg_1 |= BIT(13); 496 + tx_ana_ctrl_reg_1 |= TXDA_CAL_LATCH_EN; 589 497 writel(tx_ana_ctrl_reg_1, tcphy->base + TX_ANA_CTRL_REG_1); 590 498 usleep_range(150, 200); 591 499 592 500 /* set TX Voltage Level and TX Deemphasis to 0 */ 593 501 writel(0, tcphy->base + PHY_DP_TX_CTL); 502 + 594 503 /* re-enable decap */ 595 - writel(0x100, tcphy->base + TX_ANA_CTRL_REG_2); 596 - writel(0x300, tcphy->base + TX_ANA_CTRL_REG_2); 597 - tx_ana_ctrl_reg_1 |= BIT(3); 504 + tx_ana_ctrl_reg_2 = XCVR_DECAP_EN; 505 + writel(tx_ana_ctrl_reg_2, tcphy->base + TX_ANA_CTRL_REG_2); 506 + udelay(1); 507 + tx_ana_ctrl_reg_2 |= XCVR_DECAP_EN_DEL; 508 + writel(tx_ana_ctrl_reg_2, tcphy->base + TX_ANA_CTRL_REG_2); 509 + 510 + writel(0, tcphy->base + TX_ANA_CTRL_REG_3); 511 + 512 + tx_ana_ctrl_reg_1 |= TXDA_UPHY_SUPPLY_EN; 598 513 writel(tx_ana_ctrl_reg_1, tcphy->base + TX_ANA_CTRL_REG_1); 599 - tx_ana_ctrl_reg_1 |= BIT(4); 514 + udelay(1); 515 + tx_ana_ctrl_reg_1 |= TXDA_UPHY_SUPPLY_EN_DEL; 600 516 writel(tx_ana_ctrl_reg_1, tcphy->base + TX_ANA_CTRL_REG_1); 601 517 602 518 writel(0, tcphy->base + TX_ANA_CTRL_REG_5); ··· 617 515 writel(0x1001, tcphy->base + TX_ANA_CTRL_REG_4); 618 516 619 517 /* re-enables Bandgap reference for LDO */ 620 - tx_ana_ctrl_reg_1 |= BIT(7); 518 + tx_ana_ctrl_reg_1 |= TXDA_DRV_LDO_EN; 621 519 writel(tx_ana_ctrl_reg_1, tcphy->base + TX_ANA_CTRL_REG_1); 622 - tx_ana_ctrl_reg_1 |= BIT(8); 520 + udelay(5); 521 + tx_ana_ctrl_reg_1 |= TXDA_BGREF_EN; 623 522 writel(tx_ana_ctrl_reg_1, tcphy->base + TX_ANA_CTRL_REG_1); 624 523 625 524 /* 626 525 * re-enables the transmitter pre-driver, driver data selection MUX, 627 526 * and receiver detect circuits. 628 527 */ 629 - writel(0x301, tcphy->base + TX_ANA_CTRL_REG_2); 630 - writel(0x303, tcphy->base + TX_ANA_CTRL_REG_2); 528 + tx_ana_ctrl_reg_2 |= TXDA_DRV_PREDRV_EN; 529 + writel(tx_ana_ctrl_reg_2, tcphy->base + TX_ANA_CTRL_REG_2); 530 + udelay(1); 531 + tx_ana_ctrl_reg_2 |= TXDA_DRV_PREDRV_EN_DEL; 532 + writel(tx_ana_ctrl_reg_2, tcphy->base + TX_ANA_CTRL_REG_2); 631 533 632 534 /* 633 - * Do some magic undocumented stuff, some of which appears to 634 - * undo the "re-enables Bandgap reference for LDO" above. 535 + * Do all the undocumented magic: 536 + * - Turn on TXDA_DP_AUX_EN, whatever that is, even though sample 537 + * never shows this going on. 538 + * - Turn on TXDA_DECAP_EN (and TXDA_DECAP_EN_DEL) even though 539 + * docs say for aux it's always 0. 540 + * - Turn off the LDO and BGREF, which we just spent time turning 541 + * on above (???). 542 + * 543 + * Without this magic, things seem worse. 635 544 */ 636 - tx_ana_ctrl_reg_1 |= BIT(15); 637 - tx_ana_ctrl_reg_1 &= ~BIT(8); 638 - tx_ana_ctrl_reg_1 &= ~BIT(7); 639 - tx_ana_ctrl_reg_1 |= BIT(6); 640 - tx_ana_ctrl_reg_1 |= BIT(5); 545 + tx_ana_ctrl_reg_1 |= TXDA_DP_AUX_EN; 546 + tx_ana_ctrl_reg_1 |= TXDA_DECAP_EN; 547 + tx_ana_ctrl_reg_1 &= ~TXDA_DRV_LDO_EN; 548 + tx_ana_ctrl_reg_1 &= ~TXDA_BGREF_EN; 549 + writel(tx_ana_ctrl_reg_1, tcphy->base + TX_ANA_CTRL_REG_1); 550 + udelay(1); 551 + tx_ana_ctrl_reg_1 |= TXDA_DECAP_EN_DEL; 641 552 writel(tx_ana_ctrl_reg_1, tcphy->base + TX_ANA_CTRL_REG_1); 642 553 643 - writel(0, tcphy->base + TX_ANA_CTRL_REG_3); 644 - writel(0, tcphy->base + TX_ANA_CTRL_REG_4); 645 - writel(0, tcphy->base + TX_ANA_CTRL_REG_5); 646 - 647 554 /* 648 - * Controls low_power_swing_en, don't set the voltage swing of the 649 - * driver to 400mv. The values below are peak to peak (differential) 650 - * values. 555 + * Undo the work we did to set the LDO voltage. 556 + * This doesn't seem to help nor hurt, but it kinda goes with the 557 + * undocumented magic above. 651 558 */ 559 + writel(0, tcphy->base + TX_ANA_CTRL_REG_4); 560 + 561 + /* Don't set voltage swing to 400 mV peak to peak (differential) */ 652 562 writel(0, tcphy->base + TXDA_COEFF_CALC_CTRL); 563 + 564 + /* Init TXDA_CYA_AUXDA_CYA for unknown magic reasons */ 653 565 writel(0, tcphy->base + TXDA_CYA_AUXDA_CYA); 654 566 655 - /* Controls tx_high_z_tm_en */ 567 + /* 568 + * More undocumented magic, presumably the goal of which is to 569 + * make the "auxda_source_aux_oen" be ignored and instead to decide 570 + * about "high impedance state" based on what software puts in the 571 + * register TXDA_COEFF_CALC_CTRL (see TX_HIGH_Z). Since we only 572 + * program that register once and we don't set the bit TX_HIGH_Z, 573 + * presumably the goal here is that we should never put the analog 574 + * driver in high impedance state. 575 + */ 656 576 val = readl(tcphy->base + TX_DIG_CTRL_REG_2); 657 - val |= BIT(15); 577 + val |= TX_HIGH_Z_TM_EN; 658 578 writel(val, tcphy->base + TX_DIG_CTRL_REG_2); 659 579 } 660 580