| Message ID | 1521785487-29866-7-git-send-email-mgautam@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thu, Mar 22, 2018 at 11:12 PM Manu Gautam <mgautam@codeaurora.org> wrote: > There are two QUSB2 PHYs present on sdm845. Update PHY > registers programming for both the PHYs related to > electrical parameters to improve eye diagram. > Signed-off-by: Manu Gautam <mgautam@codeaurora.org> > --- > drivers/phy/qualcomm/phy-qcom-qusb2.c | 39 +++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) Reviewed-by: Evan Green <evgreen@chromium.org> -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Thu, Mar 22, 2018 at 11:11 PM, Manu Gautam <mgautam@codeaurora.org> wrote: > There are two QUSB2 PHYs present on sdm845. Update PHY > registers programming for both the PHYs related to > electrical parameters to improve eye diagram. This tuning difference is truly associated with the SoC itself? Are you sure? Are the two different PHYs in the SoC somehow using different silicon processes? ...or is one close to another IP block that is noisy? ...or something else to account for this difference? It seems more likely that this tuning difference is associated with the board. If you're _certain_ this is really due to internal SoC differences you'll have to come up with some darn good evidence to convince me... If the tuning is truly associated with the board then: 1. You should have a single device tree compatible string. IMHO it should contain the name of the SoC in it, so "qcom,sdm845-qusb2-phy". It's generally OK to name something in Linux using the name of the first thing that happened to support it in Linux (even if later processors use the exact same component). Leaving it as just "qcom,qusb2-v2-phy" is OK with me too if that's what everyone wants. 2. You should figure out how to describe the needed board-to-board tuning in device tree. The only two differences you have right now are: QUSB2PHY_IMP_CTRL1: 0 => 0x8 QUSB2PHY_PORT_TUNE1: 0x30 => 0x48 I'm not sure I found all the correct documentation for the PHY (the docs I have say that "TUNE1" bit 3 is "reserved") so I can't come up with all of these for you. But I think I found the difference accounting for the upper nybble of TUNE1 changing from 0x3 to 0x4. For this, I think you'd want a device tree property like: qcom,hstx_trim_mv ...and the values of that property would be the values from 800 to 950 in 8 steps, or [800, 821, 842, 864, 885, 907, 928, 950]. You'd want to do similar things for the other differences. You don't need to encode every possible difference right now. When you come up with something that needs to be different you add a new optional device tree property (defaulting to whatever the driver used to do) to describe your new property. -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 3/28/2018 4:22 AM, Doug Anderson wrote: > Hi, > > On Thu, Mar 22, 2018 at 11:11 PM, Manu Gautam <mgautam@codeaurora.org> wrote: >> There are two QUSB2 PHYs present on sdm845. Update PHY >> registers programming for both the PHYs related to >> electrical parameters to improve eye diagram. > This tuning difference is truly associated with the SoC itself? Are > you sure? Are the two different PHYs in the SoC somehow using > different silicon processes? ...or is one close to another IP block > that is noisy? ...or something else to account for this difference? > > It seems more likely that this tuning difference is associated with > the board. If you're _certain_ this is really due to internal SoC > differences you'll have to come up with some darn good evidence to > convince me... This difference must be due to board only. > > If the tuning is truly associated with the board then: > > 1. You should have a single device tree compatible string. IMHO it > should contain the name of the SoC in it, so "qcom,sdm845-qusb2-phy". > It's generally OK to name something in Linux using the name of the > first thing that happened to support it in Linux (even if later > processors use the exact same component). Leaving it as just > "qcom,qusb2-v2-phy" is OK with me too if that's what everyone wants. I will remove "qcom,qusb2-v2-phy" as I don't expect any users of that. > > > 2. You should figure out how to describe the needed board-to-board > tuning in device tree. > > The only two differences you have right now are: > > QUSB2PHY_IMP_CTRL1: 0 => 0x8 > QUSB2PHY_PORT_TUNE1: 0x30 => 0x48 > > I'm not sure I found all the correct documentation for the PHY (the > docs I have say that "TUNE1" bit 3 is "reserved") so I can't come up > with all of these for you. But I think I found the difference > accounting for the upper nybble of TUNE1 changing from 0x3 to 0x4. > For this, I think you'd want a device tree property like: > > qcom,hstx_trim_mv > > ...and the values of that property would be the values from 800 to 950 > in 8 steps, or [800, 821, 842, 864, 885, 907, 928, 950]. > > You'd want to do similar things for the other differences. > > You don't need to encode every possible difference right now. When > you come up with something that needs to be different you add a new > optional device tree property (defaulting to whatever the driver used > to do) to describe your new property. Sure. I will come up with separate device tree properties to specify board-to-board differences in PHY tuning. > > -Doug
diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c b/drivers/phy/qualcomm/phy-qcom-qusb2.c index 40fdef8..020cbb2 100644 --- a/drivers/phy/qualcomm/phy-qcom-qusb2.c +++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c @@ -174,6 +174,27 @@ enum qusb2phy_reg_layout { QUSB2_PHY_INIT_CFG(QUSB2PHY_CHG_CTRL2, 0x0), }; +static const struct qusb2_phy_init_tbl sdm845_init_tbl_1[] = { + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_ANALOG_CONTROLS_TWO, 0x03), + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_CLOCK_INVERTERS, 0x7c), + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_CMODE, 0x80), + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_LOCK_DELAY, 0x0a), + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_DIGITAL_TIMERS_TWO, 0x19), + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_BIAS_CONTROL_1, 0x40), + QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_BIAS_CONTROL_2, 0x20), + QUSB2_PHY_INIT_CFG(QUSB2PHY_PWR_CTRL2, 0x21), + QUSB2_PHY_INIT_CFG(QUSB2PHY_IMP_CTRL1, 0x8), + QUSB2_PHY_INIT_CFG(QUSB2PHY_IMP_CTRL2, 0x58), + + QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE1, 0x45), + QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE2, 0x29), + QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE3, 0xca), + QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE4, 0x04), + QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE5, 0x03), + + QUSB2_PHY_INIT_CFG(QUSB2PHY_CHG_CTRL2, 0x0), +}; + struct qusb2_phy_cfg { const struct qusb2_phy_init_tbl *tbl; /* number of entries in the table */ @@ -220,6 +241,18 @@ struct qusb2_phy_cfg { .autoresume_en = BIT(0), }; +static const struct qusb2_phy_cfg sdm845_phy_cfg_1 = { + .tbl = sdm845_init_tbl_1, + .tbl_num = ARRAY_SIZE(sdm845_init_tbl_1), + .regs = qusb2_v2_regs_layout, + + .disable_ctrl = (PWR_CTRL1_VREF_SUPPLY_TRIM | PWR_CTRL1_CLAMP_N_EN | + POWER_DOWN), + .mask_core_ready = CORE_READY_STATUS, + .has_pll_override = true, + .autoresume_en = BIT(0), +}; + static const char * const qusb2_phy_vreg_names[] = { "vdda-pll", "vdda-phy-dpdm", }; @@ -649,6 +682,12 @@ static int qusb2_phy_exit(struct phy *phy) }, { .compatible = "qcom,qusb2-v2-phy", .data = &qusb2_v2_phy_cfg, + }, { + .compatible = "qcom,sdm845-qusb2-phy-1", + .data = &sdm845_phy_cfg_1, + }, { + .compatible = "qcom,sdm845-qusb2-phy-2", + .data = &qusb2_v2_phy_cfg, }, { }, };
There are two QUSB2 PHYs present on sdm845. Update PHY registers programming for both the PHYs related to electrical parameters to improve eye diagram. Signed-off-by: Manu Gautam <mgautam@codeaurora.org> --- drivers/phy/qualcomm/phy-qcom-qusb2.c | 39 +++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)