Messages in this thread | | | Date | Tue, 11 Oct 2022 16:46:53 +0300 | Subject | Re: [PATCH 08/13] phy: qcom-qmp-pcie: drop power-down delay config | From | Dmitry Baryshkov <> |
| |
On 11/10/2022 16:14, Johan Hovold wrote: > The power-down delay was included in the first version of the QMP driver > as an optional delay after powering on the PHY (using > POWER_DOWN_CONTROL) and just before starting it. Later changes modified > this sequence by powering on before initialising the PHY, but the > optional delay stayed where it was (i.e. before starting the PHY). > > The vendor driver does not use a delay before starting the PHY and this > is likely not needed on any platform unless there is a corresponding > delay in the vendor kernel init sequence tables (i.e. in devicetree). > > Let's keep the delay for now, but drop the redundant delay period > configuration while increasing the unnecessarily low timer slack > somewhat.
Actually, the vendor driver does this 995..1005 sleep. But contrary to our driver it does that after programming whole PHY init sequence, which includes SW_RESET / START_CTL, but before programming the pipe clocks.
I think we can either drop this delay completely, or move it before read_poll_timeout().
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > --- > drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 27 +----------------------- > 1 file changed, 1 insertion(+), 26 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > index fa8bc6aeedf1..315de484f875 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c > @@ -1362,9 +1362,6 @@ struct qmp_phy_cfg { > > /* true, if PHY needs delay after POWER_DOWN */ > bool has_pwrdn_delay; > - /* power_down delay in usec */ > - int pwrdn_delay_min; > - int pwrdn_delay_max; > > /* QMP PHY pipe clock interface rate */ > unsigned long pipe_clock_rate; > @@ -1500,8 +1497,6 @@ static const struct qmp_phy_cfg ipq8074_pciephy_cfg = { > .phy_status = PHYSTATUS, > > .has_pwrdn_delay = true, > - .pwrdn_delay_min = 995, /* us */ > - .pwrdn_delay_max = 1005, /* us */ > }; > > static const struct qmp_phy_cfg ipq8074_pciephy_gen3_cfg = { > @@ -1529,8 +1524,6 @@ static const struct qmp_phy_cfg ipq8074_pciephy_gen3_cfg = { > .pwrdn_ctrl = SW_PWRDN | REFCLK_DRV_DSBL, > > .has_pwrdn_delay = true, > - .pwrdn_delay_min = 995, /* us */ > - .pwrdn_delay_max = 1005, /* us */ > > .pipe_clock_rate = 250000000, > }; > @@ -1562,8 +1555,6 @@ static const struct qmp_phy_cfg ipq6018_pciephy_cfg = { > .pwrdn_ctrl = SW_PWRDN | REFCLK_DRV_DSBL, > > .has_pwrdn_delay = true, > - .pwrdn_delay_min = 995, /* us */ > - .pwrdn_delay_max = 1005, /* us */ > }; > > static const struct qmp_phy_cfg sdm845_qmp_pciephy_cfg = { > @@ -1594,8 +1585,6 @@ static const struct qmp_phy_cfg sdm845_qmp_pciephy_cfg = { > .phy_status = PHYSTATUS, > > .has_pwrdn_delay = true, > - .pwrdn_delay_min = 995, /* us */ > - .pwrdn_delay_max = 1005, /* us */ > }; > > static const struct qmp_phy_cfg sdm845_qhp_pciephy_cfg = { > @@ -1624,8 +1613,6 @@ static const struct qmp_phy_cfg sdm845_qhp_pciephy_cfg = { > .phy_status = PHYSTATUS, > > .has_pwrdn_delay = true, > - .pwrdn_delay_min = 995, /* us */ > - .pwrdn_delay_max = 1005, /* us */ > }; > > static const struct qmp_phy_cfg sm8250_qmp_gen3x1_pciephy_cfg = { > @@ -1666,8 +1653,6 @@ static const struct qmp_phy_cfg sm8250_qmp_gen3x1_pciephy_cfg = { > .phy_status = PHYSTATUS, > > .has_pwrdn_delay = true, > - .pwrdn_delay_min = 995, /* us */ > - .pwrdn_delay_max = 1005, /* us */ > }; > > static const struct qmp_phy_cfg sm8250_qmp_gen3x2_pciephy_cfg = { > @@ -1708,8 +1693,6 @@ static const struct qmp_phy_cfg sm8250_qmp_gen3x2_pciephy_cfg = { > .phy_status = PHYSTATUS, > > .has_pwrdn_delay = true, > - .pwrdn_delay_min = 995, /* us */ > - .pwrdn_delay_max = 1005, /* us */ > }; > > static const struct qmp_phy_cfg msm8998_pciephy_cfg = { > @@ -1765,8 +1748,6 @@ static const struct qmp_phy_cfg sc8180x_pciephy_cfg = { > .pwrdn_ctrl = SW_PWRDN | REFCLK_DRV_DSBL, > > .has_pwrdn_delay = true, > - .pwrdn_delay_min = 995, /* us */ > - .pwrdn_delay_max = 1005, /* us */ > }; > > static const struct qmp_phy_cfg sdx55_qmp_pciephy_cfg = { > @@ -1797,8 +1778,6 @@ static const struct qmp_phy_cfg sdx55_qmp_pciephy_cfg = { > .phy_status = PHYSTATUS_4_20, > > .has_pwrdn_delay = true, > - .pwrdn_delay_min = 995, /* us */ > - .pwrdn_delay_max = 1005, /* us */ > }; > > static const struct qmp_phy_cfg sm8450_qmp_gen3x1_pciephy_cfg = { > @@ -1829,8 +1808,6 @@ static const struct qmp_phy_cfg sm8450_qmp_gen3x1_pciephy_cfg = { > .phy_status = PHYSTATUS, > > .has_pwrdn_delay = true, > - .pwrdn_delay_min = 995, /* us */ > - .pwrdn_delay_max = 1005, /* us */ > }; > > static const struct qmp_phy_cfg sm8450_qmp_gen4x2_pciephy_cfg = { > @@ -1876,8 +1853,6 @@ static const struct qmp_phy_cfg sm8450_qmp_gen4x2_pciephy_cfg = { > .phy_status = PHYSTATUS_4_20, > > .has_pwrdn_delay = true, > - .pwrdn_delay_min = 995, /* us */ > - .pwrdn_delay_max = 1005, /* us */ > }; > > static void qmp_pcie_configure_lane(void __iomem *base, > @@ -2037,7 +2012,7 @@ static int qmp_pcie_power_on(struct phy *phy) > qmp_pcie_pcs_init(qphy, mode_tables); > > if (cfg->has_pwrdn_delay) > - usleep_range(cfg->pwrdn_delay_min, cfg->pwrdn_delay_max); > + usleep_range(1000, 1200); > > /* Pull PHY out of reset state */ > qphy_clrbits(pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);
-- With best wishes Dmitry
| |