Message ID | 1585597017-30683-5-git-send-email-wcheng@codeaurora.org (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Add SS/HS-USB changes for Qualcomm SM8150 chipset | expand |
On 3/31/2020 1:06 AM, Wesley Cheng wrote: > The register map for SM8150 QMP USB SSPHY has moved > QPHY_POWER_DOWN_CONTROL to a different offset. Allow for > an offset in the register table to override default value > if it is a DP capable PHY. > > Signed-off-by: Wesley Cheng <wcheng@codeaurora.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c > index cc04471..4c0517e 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c > @@ -164,6 +164,7 @@ enum qphy_reg_layout { > [QPHY_SW_RESET] = 0x00, > [QPHY_START_CTRL] = 0x44, > [QPHY_PCS_STATUS] = 0x14, > + [QPHY_COM_POWER_DOWN_CONTROL] = 0x40, Since this is in PCS block please rename it to - QPHY_PCS_POWER_DOWN_CONTROL > }; > > static const unsigned int sdm845_ufsphy_regs_layout[] = { > @@ -1627,6 +1628,9 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy) > if (cfg->has_phy_com_ctrl) > qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL], > SW_PWRDN); > + else if (cfg->has_phy_dp_com_ctrl && cfg->regs[QPHY_COM_POWER_DOWN_CONTROL]) > + qphy_setbits(pcs, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL], > + cfg->pwrdn_ctrl); > else > qphy_setbits(pcs, QPHY_POWER_DOWN_CONTROL, cfg->pwrdn_ctrl); Since, this register is in PCS block why check for dp_com_ctrl here? Something like: if (cfg->has_phy_com_ctrl) { qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL], SW_PWRDN); } else { if (cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL]) qphy_setbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], cfg->pwrdn_ctrl); else qphy_setbits(pcs, QPHY_POWER_DOWN_CONTROL, cfg->pwrdn_ctrl); } > > @@ -1671,10 +1675,12 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy) > return ret; > } > > -static int qcom_qmp_phy_com_exit(struct qcom_qmp *qmp) > +static int qcom_qmp_phy_com_exit(struct qmp_phy *qphy) > { > + struct qcom_qmp *qmp = qphy->qmp; > const struct qmp_phy_cfg *cfg = qmp->cfg; > void __iomem *serdes = qmp->serdes; > + void __iomem *pcs = qphy->pcs; > int i = cfg->num_resets; > > mutex_lock(&qmp->phy_mutex); > @@ -1691,6 +1697,9 @@ static int qcom_qmp_phy_com_exit(struct qcom_qmp *qmp) > SW_RESET); > qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL], > SW_PWRDN); > + } else if (cfg->has_phy_dp_com_ctrl && cfg->regs[QPHY_COM_POWER_DOWN_CONTROL]) { Can we add change similar to init() here ? > + cfg->pwrdn_ctrl); > } > > while (--i >= 0) > @@ -1829,7 +1838,7 @@ static int qcom_qmp_phy_enable(struct phy *phy) > if (cfg->has_lane_rst) > reset_control_assert(qphy->lane_rst); > err_lane_rst: > - qcom_qmp_phy_com_exit(qmp); > + qcom_qmp_phy_com_exit(qphy); > > return ret; > } > @@ -1855,7 +1864,7 @@ static int qcom_qmp_phy_disable(struct phy *phy) > if (cfg->has_lane_rst) > reset_control_assert(qphy->lane_rst); > > - qcom_qmp_phy_com_exit(qmp); > + qcom_qmp_phy_com_exit(qphy); > > qmp->phy_initialized = false; >
Hi Manu, Thanks for the feedback and review. On 4/2/2020 12:35 AM, Manu Gautam wrote: > > On 3/31/2020 1:06 AM, Wesley Cheng wrote: >> The register map for SM8150 QMP USB SSPHY has moved >> QPHY_POWER_DOWN_CONTROL to a different offset. Allow for >> an offset in the register table to override default value >> if it is a DP capable PHY. >> >> Signed-off-by: Wesley Cheng <wcheng@codeaurora.org> >> --- >> drivers/phy/qualcomm/phy-qcom-qmp.c | 15 ++++++++++++--- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c >> index cc04471..4c0517e 100644 >> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c >> @@ -164,6 +164,7 @@ enum qphy_reg_layout { >> [QPHY_SW_RESET] = 0x00, >> [QPHY_START_CTRL] = 0x44, >> [QPHY_PCS_STATUS] = 0x14, >> + [QPHY_COM_POWER_DOWN_CONTROL] = 0x40, > Since this is in PCS block please rename it to - > > QPHY_PCS_POWER_DOWN_CONTROL > Sure, will add another enum value to the register layout, and rename it where needed. >> }; >> >> static const unsigned int sdm845_ufsphy_regs_layout[] = { >> @@ -1627,6 +1628,9 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy) >> if (cfg->has_phy_com_ctrl) >> qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL], >> SW_PWRDN); >> + else if (cfg->has_phy_dp_com_ctrl && cfg->regs[QPHY_COM_POWER_DOWN_CONTROL]) >> + qphy_setbits(pcs, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL], >> + cfg->pwrdn_ctrl); >> else >> qphy_setbits(pcs, QPHY_POWER_DOWN_CONTROL, cfg->pwrdn_ctrl); > Since, this register is in PCS block why check for dp_com_ctrl here? > Something like: > > if (cfg->has_phy_com_ctrl) { > qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL], > SW_PWRDN); > } else { > if (cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL]) > qphy_setbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], > cfg->pwrdn_ctrl); > else > qphy_setbits(pcs, QPHY_POWER_DOWN_CONTROL, cfg->pwrdn_ctrl); > } > Agree with this. >> >> @@ -1671,10 +1675,12 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy) >> return ret; >> } >> >> -static int qcom_qmp_phy_com_exit(struct qcom_qmp *qmp) >> +static int qcom_qmp_phy_com_exit(struct qmp_phy *qphy) >> { >> + struct qcom_qmp *qmp = qphy->qmp; >> const struct qmp_phy_cfg *cfg = qmp->cfg; >> void __iomem *serdes = qmp->serdes; >> + void __iomem *pcs = qphy->pcs; >> int i = cfg->num_resets; >> >> mutex_lock(&qmp->phy_mutex); >> @@ -1691,6 +1697,9 @@ static int qcom_qmp_phy_com_exit(struct qcom_qmp *qmp) >> SW_RESET); >> qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL], >> SW_PWRDN); >> + } else if (cfg->has_phy_dp_com_ctrl && cfg->regs[QPHY_COM_POWER_DOWN_CONTROL]) { > > Can we add change similar to init() here ? > > Sure. I will move this check to where the current code writes to the PWR DOWN CONTROL in static int qcom_qmp_phy_disable(struct phy *phy) { ... qphy_clrbits(qphy->pcs, QPHY_POWER_DOWN_CONTROL, cfg->pwrdn_ctrl); We wouldn't want the SW to write to an incorrect register. >> + cfg->pwrdn_ctrl); >> } >> >> while (--i >= 0) >> @@ -1829,7 +1838,7 @@ static int qcom_qmp_phy_enable(struct phy *phy) >> if (cfg->has_lane_rst) >> reset_control_assert(qphy->lane_rst); >> err_lane_rst: >> - qcom_qmp_phy_com_exit(qmp); >> + qcom_qmp_phy_com_exit(qphy); >> >> return ret; >> } >> @@ -1855,7 +1864,7 @@ static int qcom_qmp_phy_disable(struct phy *phy) >> if (cfg->has_lane_rst) >> reset_control_assert(qphy->lane_rst); >> >> - qcom_qmp_phy_com_exit(qmp); >> + qcom_qmp_phy_com_exit(qphy); >> >> qmp->phy_initialized = false; >> >
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c index cc04471..4c0517e 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c @@ -164,6 +164,7 @@ enum qphy_reg_layout { [QPHY_SW_RESET] = 0x00, [QPHY_START_CTRL] = 0x44, [QPHY_PCS_STATUS] = 0x14, + [QPHY_COM_POWER_DOWN_CONTROL] = 0x40, }; static const unsigned int sdm845_ufsphy_regs_layout[] = { @@ -1627,6 +1628,9 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy) if (cfg->has_phy_com_ctrl) qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL], SW_PWRDN); + else if (cfg->has_phy_dp_com_ctrl && cfg->regs[QPHY_COM_POWER_DOWN_CONTROL]) + qphy_setbits(pcs, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL], + cfg->pwrdn_ctrl); else qphy_setbits(pcs, QPHY_POWER_DOWN_CONTROL, cfg->pwrdn_ctrl); @@ -1671,10 +1675,12 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy) return ret; } -static int qcom_qmp_phy_com_exit(struct qcom_qmp *qmp) +static int qcom_qmp_phy_com_exit(struct qmp_phy *qphy) { + struct qcom_qmp *qmp = qphy->qmp; const struct qmp_phy_cfg *cfg = qmp->cfg; void __iomem *serdes = qmp->serdes; + void __iomem *pcs = qphy->pcs; int i = cfg->num_resets; mutex_lock(&qmp->phy_mutex); @@ -1691,6 +1697,9 @@ static int qcom_qmp_phy_com_exit(struct qcom_qmp *qmp) SW_RESET); qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL], SW_PWRDN); + } else if (cfg->has_phy_dp_com_ctrl && cfg->regs[QPHY_COM_POWER_DOWN_CONTROL]) { + qphy_clrbits(pcs, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL], + cfg->pwrdn_ctrl); } while (--i >= 0) @@ -1829,7 +1838,7 @@ static int qcom_qmp_phy_enable(struct phy *phy) if (cfg->has_lane_rst) reset_control_assert(qphy->lane_rst); err_lane_rst: - qcom_qmp_phy_com_exit(qmp); + qcom_qmp_phy_com_exit(qphy); return ret; } @@ -1855,7 +1864,7 @@ static int qcom_qmp_phy_disable(struct phy *phy) if (cfg->has_lane_rst) reset_control_assert(qphy->lane_rst); - qcom_qmp_phy_com_exit(qmp); + qcom_qmp_phy_com_exit(qphy); qmp->phy_initialized = false;
The register map for SM8150 QMP USB SSPHY has moved QPHY_POWER_DOWN_CONTROL to a different offset. Allow for an offset in the register table to override default value if it is a DP capable PHY. Signed-off-by: Wesley Cheng <wcheng@codeaurora.org> --- drivers/phy/qualcomm/phy-qcom-qmp.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)