On 2019-12-20 15:10, Vinod Koul wrote:
On 20-12-19, 14:00, Can Guo wrote:
On 2019-12-20 12:24, Vinod Koul wrote:
> On 20-12-19, 08:49, cang@xxxxxxxxxxxxxx wrote:
> > On 2019-12-20 08:22, cang@xxxxxxxxxxxxxx wrote:
> > > On 2019-12-19 23:04, Vinod Koul wrote:
> > >
> > > > /* start SerDes and Phy-Coding-Sublayer */
> > > > qphy_setbits(pcs, cfg->regs[QPHY_START_CTRL], cfg->start_ctrl);
> >
> > I thought your change would be like this
> >
> > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c
> > b/drivers/phy/qualcomm/phy-qcom-qmp.c
> > index 8e642a6..a4ab4b7 100755
> > --- a/drivers/phy/qualcomm/phy-qcom-qmp.c
> > +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
> > @@ -166,6 +166,7 @@ static const unsigned int
> > sdm845_ufsphy_regs_layout[] =
> > {
> > };
> >
> > static const unsigned int sm8150_ufsphy_regs_layout[] = {
> > + [QPHY_SW_RESET] = 0x08,
> > [QPHY_START_CTRL] = 0x00,
> > [QPHY_PCS_READY_STATUS] = 0x180,
> > };
> > @@ -1390,7 +1391,6 @@ static const struct qmp_phy_cfg
> > sm8150_ufsphy_cfg = {
> > .pwrdn_ctrl = SW_PWRDN,
> >
> > .is_dual_lane_phy = true,
> > - .no_pcs_sw_reset = true,
> > };
> >
> > static void qcom_qmp_phy_configure(void __iomem *base,
> > @@ -1475,6 +1475,9 @@ static int qcom_qmp_phy_com_init(struct
> > qmp_phy *qphy)
> > SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
> > }
> >
> > + if ((cfg->type == PHY_TYPE_UFS) && (!cfg->no_pcs_sw_reset))
> > + qphy_setbits(pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);
>
> Well am not sure if no_pcs_sw_reset would do this and side effect on
> other phys (somehow older ones dont seem to need this). That was the
> reason for a new flag and to be used for specific instances
>
> Thanks
Hi Vinod,
That is why I added the check as cfg->type == PHY_TYPE_UFS, meaning
this
change will only apply to UFS.
FYI, start from 8150(include 8150), QPHY_SW_RESET is present in PHY's
PCS register. no_pcs_sw_reset = TRUE should only be given to 845 and
older
targets, like 8998, because they don't have this QPHY_SW_RESET in
PHY's
register per their design, that's why they leverage the reset control
provided by UFS controller.
I have removed no_pcs_sw_reset and tested.
Well as you said even with UFS we have variations between various
chips,
so I thought leaving it separate might be better than creating a chance
of regression on older platforms!
Moreover, are we sure that the reset wont be there for other qmp phy's
in future other than UFS...
Thanks
Hi Vinod
We are just removing the no_pcs_sw_reset for 8150, right? Why is it
possibly impacting 845 or older paltforms?
In future, we will no longer need no_pcs_sw_reset for any newer QCOM UFS
PHY designs, as it is only for 845 and older platforms.
I am sure QPHY_SW_RESET will be within PHY's address space since 8150.
Otherwise, it will be a regression in UFS PHY design. We had a lot of
discussion about this on 845 years ago, then design team decided to add
it on later platforms, so I don't see a reason to remove it again. :)
I am not sure about the other qmp phys, but so long as UFS PHY needs the
reset, we need to keep it, as phy-qcom-qmp.c is a common driver. I am
not sure if I get your point here. Please correct me I am wrong.
Thanks,
Can Guo.
