diff mbox series

[3/4] phy: qcom-qmp: Add optional SW reset

Message ID 20191219150433.2785427-4-vkoul@kernel.org
State New
Headers show
Series phy: qcom-qmp: Fixes and updates for sm8150 | expand

Commit Message

Vinod Koul Dec. 19, 2019, 3:04 p.m. UTC
For V4 QMP UFS Phy, we need to assert reset bits, configure the phy and
then deassert it, so add optional has_sw_reset flag and use that to
configure the QPHY_SW_RESET register.

Signed-off-by: Vinod Koul <vkoul@kernel.org>

---
 drivers/phy/qualcomm/phy-qcom-qmp.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

-- 
2.23.0

Comments

Jeffrey Hugo Dec. 19, 2019, 3:31 p.m. UTC | #1
On Thu, Dec 19, 2019 at 8:05 AM Vinod Koul <vkoul@kernel.org> wrote:
>

> For V4 QMP UFS Phy, we need to assert reset bits, configure the phy and

> then deassert it, so add optional has_sw_reset flag and use that to

> configure the QPHY_SW_RESET register.

>

> Signed-off-by: Vinod Koul <vkoul@kernel.org>


Reviewed-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>


> ---

>  drivers/phy/qualcomm/phy-qcom-qmp.c | 10 ++++++++++

>  1 file changed, 10 insertions(+)

>

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c

> index 06f971ca518e..80304b7cd895 100644

> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c

> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c

> @@ -1023,6 +1023,9 @@ struct qmp_phy_cfg {

>

>         /* true, if PCS block has no separate SW_RESET register */

>         bool no_pcs_sw_reset;

> +

> +       /* true if sw reset needs to be invoked */

> +       bool has_sw_reset;

>  };

>

>  /**

> @@ -1391,6 +1394,7 @@ static const struct qmp_phy_cfg sm8150_ufsphy_cfg = {

>

>         .is_dual_lane_phy       = true,

>         .no_pcs_sw_reset        = true,

> +       .has_sw_reset           = true,

>  };

>

>  static void qcom_qmp_phy_configure(void __iomem *base,

> @@ -1475,6 +1479,9 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy)

>                              SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);

>         }

>

> +       if (cfg->has_sw_reset)

> +               qphy_setbits(serdes, cfg->regs[QPHY_SW_RESET], SW_RESET);

> +

>         if (cfg->has_phy_com_ctrl)

>                 qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL],

>                              SW_PWRDN);

> @@ -1651,6 +1658,9 @@ static int qcom_qmp_phy_enable(struct phy *phy)

>         if (cfg->has_phy_dp_com_ctrl)

>                 qphy_clrbits(dp_com, QPHY_V3_DP_COM_SW_RESET, SW_RESET);

>

> +       if (cfg->has_sw_reset)

> +               qphy_clrbits(pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);

> +

>         /* start SerDes and Phy-Coding-Sublayer */

>         qphy_setbits(pcs, cfg->regs[QPHY_START_CTRL], cfg->start_ctrl);

>

> --

> 2.23.0

>
Can Guo Dec. 20, 2019, 12:22 a.m. UTC | #2
On 2019-12-19 23:04, Vinod Koul wrote:
> For V4 QMP UFS Phy, we need to assert reset bits, configure the phy and

> then deassert it, so add optional has_sw_reset flag and use that to

> configure the QPHY_SW_RESET register.

> 

> Signed-off-by: Vinod Koul <vkoul@kernel.org>

> ---

>  drivers/phy/qualcomm/phy-qcom-qmp.c | 10 ++++++++++

>  1 file changed, 10 insertions(+)

> 

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c

> b/drivers/phy/qualcomm/phy-qcom-qmp.c

> index 06f971ca518e..80304b7cd895 100644

> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c

> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c

> @@ -1023,6 +1023,9 @@ struct qmp_phy_cfg {

> 

>  	/* true, if PCS block has no separate SW_RESET register */

>  	bool no_pcs_sw_reset;

> +

> +	/* true if sw reset needs to be invoked */

> +	bool has_sw_reset;

>  };

> 

>  /**

> @@ -1391,6 +1394,7 @@ static const struct qmp_phy_cfg sm8150_ufsphy_cfg 

> = {

> 

>  	.is_dual_lane_phy	= true,

>  	.no_pcs_sw_reset	= true,

> +	.has_sw_reset		= true,

>  };

> 

>  static void qcom_qmp_phy_configure(void __iomem *base,

> @@ -1475,6 +1479,9 @@ static int qcom_qmp_phy_com_init(struct qmp_phy 

> *qphy)

>  			     SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);

>  	}

> 

> +	if (cfg->has_sw_reset)

> +		qphy_setbits(serdes, cfg->regs[QPHY_SW_RESET], SW_RESET);

> +


Are you sure you want to set this in the serdes register? QPHY_SW_RESET
is in its pcs register.

>  	if (cfg->has_phy_com_ctrl)

>  		qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL],

>  			     SW_PWRDN);

> @@ -1651,6 +1658,9 @@ static int qcom_qmp_phy_enable(struct phy *phy)

>  	if (cfg->has_phy_dp_com_ctrl)

>  		qphy_clrbits(dp_com, QPHY_V3_DP_COM_SW_RESET, SW_RESET);

> 

> +	if (cfg->has_sw_reset)

> +		qphy_clrbits(pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);

> +


Yet you are clearing it from pcs register.

Regards,
Can Guo

>  	/* start SerDes and Phy-Coding-Sublayer */

>  	qphy_setbits(pcs, cfg->regs[QPHY_START_CTRL], cfg->start_ctrl);
Can Guo Dec. 20, 2019, 12:49 a.m. UTC | #3
On 2019-12-20 08:22, cang@codeaurora.org wrote:
> On 2019-12-19 23:04, Vinod Koul wrote:

>> For V4 QMP UFS Phy, we need to assert reset bits, configure the phy 

>> and

>> then deassert it, so add optional has_sw_reset flag and use that to

>> configure the QPHY_SW_RESET register.

>> 

>> Signed-off-by: Vinod Koul <vkoul@kernel.org>

>> ---

>>  drivers/phy/qualcomm/phy-qcom-qmp.c | 10 ++++++++++

>>  1 file changed, 10 insertions(+)

>> 

>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c

>> b/drivers/phy/qualcomm/phy-qcom-qmp.c

>> index 06f971ca518e..80304b7cd895 100644

>> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c

>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c

>> @@ -1023,6 +1023,9 @@ struct qmp_phy_cfg {

>> 

>>  	/* true, if PCS block has no separate SW_RESET register */

>>  	bool no_pcs_sw_reset;

>> +

>> +	/* true if sw reset needs to be invoked */

>> +	bool has_sw_reset;

>>  };

>> 

>>  /**

>> @@ -1391,6 +1394,7 @@ static const struct qmp_phy_cfg 

>> sm8150_ufsphy_cfg = {

>> 

>>  	.is_dual_lane_phy	= true,

>>  	.no_pcs_sw_reset	= true,

>> +	.has_sw_reset		= true,

>>  };

>> 

>>  static void qcom_qmp_phy_configure(void __iomem *base,

>> @@ -1475,6 +1479,9 @@ static int qcom_qmp_phy_com_init(struct qmp_phy 

>> *qphy)

>>  			     SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);

>>  	}

>> 

>> +	if (cfg->has_sw_reset)

>> +		qphy_setbits(serdes, cfg->regs[QPHY_SW_RESET], SW_RESET);

>> +

> 

> Are you sure you want to set this in the serdes register? QPHY_SW_RESET

> is in its pcs register.

> 

>>  	if (cfg->has_phy_com_ctrl)

>>  		qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL],

>>  			     SW_PWRDN);

>> @@ -1651,6 +1658,9 @@ static int qcom_qmp_phy_enable(struct phy *phy)

>>  	if (cfg->has_phy_dp_com_ctrl)

>>  		qphy_clrbits(dp_com, QPHY_V3_DP_COM_SW_RESET, SW_RESET);

>> 

>> +	if (cfg->has_sw_reset)

>> +		qphy_clrbits(pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);

>> +

> 

> Yet you are clearing it from pcs register.

> 

> Regards,

> Can Guo

> 

>>  	/* 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);
+
         if (cfg->has_phy_com_ctrl)
                 qphy_setbits(serdes, 
cfg->regs[QPHY_COM_POWER_DOWN_CONTROL],
                              SW_PWRDN);

Regards,
Can Guo.
Vinod Koul Dec. 20, 2019, 4:24 a.m. UTC | #4
On 20-12-19, 08:49, cang@codeaurora.org wrote:
> On 2019-12-20 08:22, cang@codeaurora.org wrote:

> > On 2019-12-19 23:04, Vinod Koul wrote:

> > > For V4 QMP UFS Phy, we need to assert reset bits, configure the phy

> > > and

> > > then deassert it, so add optional has_sw_reset flag and use that to

> > > configure the QPHY_SW_RESET register.

> > > 

> > > Signed-off-by: Vinod Koul <vkoul@kernel.org>

> > > ---

> > >  drivers/phy/qualcomm/phy-qcom-qmp.c | 10 ++++++++++

> > >  1 file changed, 10 insertions(+)

> > > 

> > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c

> > > b/drivers/phy/qualcomm/phy-qcom-qmp.c

> > > index 06f971ca518e..80304b7cd895 100644

> > > --- a/drivers/phy/qualcomm/phy-qcom-qmp.c

> > > +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c

> > > @@ -1023,6 +1023,9 @@ struct qmp_phy_cfg {

> > > 

> > >  	/* true, if PCS block has no separate SW_RESET register */

> > >  	bool no_pcs_sw_reset;

> > > +

> > > +	/* true if sw reset needs to be invoked */

> > > +	bool has_sw_reset;

> > >  };

> > > 

> > >  /**

> > > @@ -1391,6 +1394,7 @@ static const struct qmp_phy_cfg

> > > sm8150_ufsphy_cfg = {

> > > 

> > >  	.is_dual_lane_phy	= true,

> > >  	.no_pcs_sw_reset	= true,

> > > +	.has_sw_reset		= true,

> > >  };

> > > 

> > >  static void qcom_qmp_phy_configure(void __iomem *base,

> > > @@ -1475,6 +1479,9 @@ static int qcom_qmp_phy_com_init(struct

> > > qmp_phy *qphy)

> > >  			     SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);

> > >  	}

> > > 

> > > +	if (cfg->has_sw_reset)

> > > +		qphy_setbits(serdes, cfg->regs[QPHY_SW_RESET], SW_RESET);

> > > +

> > 

> > Are you sure you want to set this in the serdes register? QPHY_SW_RESET

> > is in its pcs register.

> > 

> > >  	if (cfg->has_phy_com_ctrl)

> > >  		qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL],

> > >  			     SW_PWRDN);

> > > @@ -1651,6 +1658,9 @@ static int qcom_qmp_phy_enable(struct phy *phy)

> > >  	if (cfg->has_phy_dp_com_ctrl)

> > >  		qphy_clrbits(dp_com, QPHY_V3_DP_COM_SW_RESET, SW_RESET);

> > > 

> > > +	if (cfg->has_sw_reset)

> > > +		qphy_clrbits(pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);

> > > +

> > 

> > Yet you are clearing it from pcs register.


updated now, will send v2

> > 

> > Regards,

> > Can Guo

> > 

> > >  	/* 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
-- 
~Vinod
Vinod Koul Dec. 20, 2019, 7:10 a.m. UTC | #5
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@codeaurora.org wrote:

> > > On 2019-12-20 08:22, cang@codeaurora.org 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
-- 
~Vinod
Vinod Koul Dec. 20, 2019, 7:53 a.m. UTC | #6
On 20-12-19, 15:41, Can Guo wrote:
> On 2019-12-20 15:10, Vinod Koul wrote:

> > On 20-12-19, 14:00, Can Guo wrote:

> 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.


The argument here is that we are making this UFS specific and we do not
know if this will be true in future as QMP is a common phy, so adding a
separate flag helps to keep it independent and to be used in other
situations.

Thanks
-- 
~Vinod
Manu Gautam Dec. 23, 2019, 9:02 a.m. UTC | #7
On 12/20/2019 6:19 AM, cang@codeaurora.org wrote:
> On 2019-12-20 08:22, cang@codeaurora.org wrote:

>> On 2019-12-19 23:04, Vinod Koul wrote:

>>> For V4 QMP UFS Phy, we need to assert reset bits, configure the phy and

>>> then deassert it, so add optional has_sw_reset flag and use that to

>>> configure the QPHY_SW_RESET register.

>>>

>>> Signed-off-by: Vinod Koul <vkoul@kernel.org>

>>> ---

>>>  drivers/phy/qualcomm/phy-qcom-qmp.c | 10 ++++++++++

>>>  1 file changed, 10 insertions(+)

>>>

>>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c

>>> b/drivers/phy/qualcomm/phy-qcom-qmp.c

>>> index 06f971ca518e..80304b7cd895 100644

>>> --- a/drivers/phy/qualcomm/phy-qcom-qmp.c

>>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp.c

>>> @@ -1023,6 +1023,9 @@ struct qmp_phy_cfg {

>>>

>>>      /* true, if PCS block has no separate SW_RESET register */

>>>      bool no_pcs_sw_reset;

>>> +

>>> +    /* true if sw reset needs to be invoked */

>>> +    bool has_sw_reset;

>>>  };

>>>

>>>  /**

>>> @@ -1391,6 +1394,7 @@ static const struct qmp_phy_cfg sm8150_ufsphy_cfg = {

>>>

>>>      .is_dual_lane_phy    = true,

>>>      .no_pcs_sw_reset    = true,

>>> +    .has_sw_reset        = true,

>>>  };

>>>

>>>  static void qcom_qmp_phy_configure(void __iomem *base,

>>> @@ -1475,6 +1479,9 @@ static int qcom_qmp_phy_com_init(struct qmp_phy *qphy)

>>>                   SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);

>>>      }

>>>

>>> +    if (cfg->has_sw_reset)

>>> +        qphy_setbits(serdes, cfg->regs[QPHY_SW_RESET], SW_RESET);

>>> +

>>

>> Are you sure you want to set this in the serdes register? QPHY_SW_RESET

>> is in its pcs register.

>>

>>>      if (cfg->has_phy_com_ctrl)

>>>          qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL],

>>>                   SW_PWRDN);

>>> @@ -1651,6 +1658,9 @@ static int qcom_qmp_phy_enable(struct phy *phy)

>>>      if (cfg->has_phy_dp_com_ctrl)

>>>          qphy_clrbits(dp_com, QPHY_V3_DP_COM_SW_RESET, SW_RESET);

>>>

>>> +    if (cfg->has_sw_reset)

>>> +        qphy_clrbits(pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);

>>> +

>>

>> Yet you are clearing it from pcs register.

>>

>> Regards,

>> Can Guo

>>

>>>      /* 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,



This makes sense to me.

>  };

>

>  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);

> +



This change is not needed as POR value of SW_RESET bit is '1' which will be
set as part of GCC or clk_reset.
We just need to clear this bit which code already takes care of.


>         if (cfg->has_phy_com_ctrl)

>                 qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL],

>                              SW_PWRDN);

>

> Regards,

> Can Guo.


-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
diff mbox series

Patch

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
index 06f971ca518e..80304b7cd895 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -1023,6 +1023,9 @@  struct qmp_phy_cfg {
 
 	/* true, if PCS block has no separate SW_RESET register */
 	bool no_pcs_sw_reset;
+
+	/* true if sw reset needs to be invoked */
+	bool has_sw_reset;
 };
 
 /**
@@ -1391,6 +1394,7 @@  static const struct qmp_phy_cfg sm8150_ufsphy_cfg = {
 
 	.is_dual_lane_phy	= true,
 	.no_pcs_sw_reset	= true,
+	.has_sw_reset		= true,
 };
 
 static void qcom_qmp_phy_configure(void __iomem *base,
@@ -1475,6 +1479,9 @@  static int qcom_qmp_phy_com_init(struct qmp_phy *qphy)
 			     SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
 	}
 
+	if (cfg->has_sw_reset)
+		qphy_setbits(serdes, cfg->regs[QPHY_SW_RESET], SW_RESET);
+
 	if (cfg->has_phy_com_ctrl)
 		qphy_setbits(serdes, cfg->regs[QPHY_COM_POWER_DOWN_CONTROL],
 			     SW_PWRDN);
@@ -1651,6 +1658,9 @@  static int qcom_qmp_phy_enable(struct phy *phy)
 	if (cfg->has_phy_dp_com_ctrl)
 		qphy_clrbits(dp_com, QPHY_V3_DP_COM_SW_RESET, SW_RESET);
 
+	if (cfg->has_sw_reset)
+		qphy_clrbits(pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);
+
 	/* start SerDes and Phy-Coding-Sublayer */
 	qphy_setbits(pcs, cfg->regs[QPHY_START_CTRL], cfg->start_ctrl);