| Message ID | 20250503162440.2954-7-quic_nitirawa@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Refactor ufs phy powerup sequence | expand |
On Sat, May 03, 2025 at 09:54:35PM +0530, Nitin Rawat wrote: > Rename qmp_ufs_disable to qmp_ufs_power_off to better represent its > functionality. Additionally, move the qmp_ufs_exit() call inside > qmp_ufs_power_off to preserve the functionality of .power_off. > > There is no functional change. > > Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com> > --- > drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 30 +++++++++---------------- > 1 file changed, 11 insertions(+), 19 deletions(-) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > index 94095393148c..c501223fc5f9 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > @@ -1835,6 +1835,15 @@ static int qmp_ufs_phy_calibrate(struct phy *phy) > return 0; > } > > +static int qmp_ufs_exit(struct phy *phy) > +{ > + struct qmp_ufs *qmp = phy_get_drvdata(phy); > + > + qmp_ufs_com_exit(qmp); Just inline it, unless you have any other plans. > + > + return 0; > +} > + > static int qmp_ufs_power_off(struct phy *phy) > { > struct qmp_ufs *qmp = phy_get_drvdata(phy); > @@ -1851,28 +1860,11 @@ static int qmp_ufs_power_off(struct phy *phy) > qphy_clrbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], > SW_PWRDN); > > - return 0; > -} > - > -static int qmp_ufs_exit(struct phy *phy) > -{ > - struct qmp_ufs *qmp = phy_get_drvdata(phy); > - > - qmp_ufs_com_exit(qmp); > + qmp_ufs_exit(phy); > > return 0; > } > > -static int qmp_ufs_disable(struct phy *phy) > -{ > - int ret; > - > - ret = qmp_ufs_power_off(phy); > - if (ret) > - return ret; > - return qmp_ufs_exit(phy); > -} > - > static int qmp_ufs_set_mode(struct phy *phy, enum phy_mode mode, int submode) > { > struct qmp_ufs *qmp = phy_get_drvdata(phy); > @@ -1921,7 +1913,7 @@ static int qmp_ufs_phy_init(struct phy *phy) > static const struct phy_ops qcom_qmp_ufs_phy_ops = { > .init = qmp_ufs_phy_init, > .power_on = qmp_ufs_power_on, > - .power_off = qmp_ufs_disable, > + .power_off = qmp_ufs_power_off, > .calibrate = qmp_ufs_phy_calibrate, > .set_mode = qmp_ufs_set_mode, > .owner = THIS_MODULE, > -- > 2.48.1 >
On 5/4/2025 9:07 PM, Dmitry Baryshkov wrote: > On Sat, May 03, 2025 at 09:54:35PM +0530, Nitin Rawat wrote: >> Rename qmp_ufs_disable to qmp_ufs_power_off to better represent its >> functionality. Additionally, move the qmp_ufs_exit() call inside >> qmp_ufs_power_off to preserve the functionality of .power_off. >> >> There is no functional change. >> >> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com> >> --- >> drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 30 +++++++++---------------- >> 1 file changed, 11 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c >> index 94095393148c..c501223fc5f9 100644 >> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c >> @@ -1835,6 +1835,15 @@ static int qmp_ufs_phy_calibrate(struct phy *phy) >> return 0; >> } >> >> +static int qmp_ufs_exit(struct phy *phy) >> +{ >> + struct qmp_ufs *qmp = phy_get_drvdata(phy); >> + >> + qmp_ufs_com_exit(qmp); > > Just inline it, unless you have any other plans. Hi Dmitry, I have inlined qcom_ufs_com_exit in patch #7 of the same series. I separated it into a different patch to keep each patch simpler. Could you please review patch #7 and share your thoughts. [PATCH V4 07/11] phy: qcom-qmp-ufs: Remove qmp_ufs_exit() and Inline qmp_ufs_com_exit(). Regards, Nitin > >> + >> + return 0; >> +} >> + >> static int qmp_ufs_power_off(struct phy *phy) >> { >> struct qmp_ufs *qmp = phy_get_drvdata(phy); >> @@ -1851,28 +1860,11 @@ static int qmp_ufs_power_off(struct phy *phy) >> qphy_clrbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], >> SW_PWRDN); >> >> - return 0; >> -} >> - >> -static int qmp_ufs_exit(struct phy *phy) >> -{ >> - struct qmp_ufs *qmp = phy_get_drvdata(phy); >> - >> - qmp_ufs_com_exit(qmp); >> + qmp_ufs_exit(phy); >> >> return 0; >> } >> >> -static int qmp_ufs_disable(struct phy *phy) >> -{ >> - int ret; >> - >> - ret = qmp_ufs_power_off(phy); >> - if (ret) >> - return ret; >> - return qmp_ufs_exit(phy); >> -} >> - >> static int qmp_ufs_set_mode(struct phy *phy, enum phy_mode mode, int submode) >> { >> struct qmp_ufs *qmp = phy_get_drvdata(phy); >> @@ -1921,7 +1913,7 @@ static int qmp_ufs_phy_init(struct phy *phy) >> static const struct phy_ops qcom_qmp_ufs_phy_ops = { >> .init = qmp_ufs_phy_init, >> .power_on = qmp_ufs_power_on, >> - .power_off = qmp_ufs_disable, >> + .power_off = qmp_ufs_power_off, >> .calibrate = qmp_ufs_phy_calibrate, >> .set_mode = qmp_ufs_set_mode, >> .owner = THIS_MODULE, >> -- >> 2.48.1 >> >
On Sun, May 04, 2025 at 06:37:41PM +0300, Dmitry Baryshkov wrote: > On Sat, May 03, 2025 at 09:54:35PM +0530, Nitin Rawat wrote: > > Rename qmp_ufs_disable to qmp_ufs_power_off to better represent its > > functionality. Additionally, move the qmp_ufs_exit() call inside > > qmp_ufs_power_off to preserve the functionality of .power_off. > > > > There is no functional change. > > > > Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com> > > --- > > drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 30 +++++++++---------------- > > 1 file changed, 11 insertions(+), 19 deletions(-) > > > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > > index 94095393148c..c501223fc5f9 100644 > > --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > > @@ -1835,6 +1835,15 @@ static int qmp_ufs_phy_calibrate(struct phy *phy) > > return 0; > > } > > > > +static int qmp_ufs_exit(struct phy *phy) > > +{ > > + struct qmp_ufs *qmp = phy_get_drvdata(phy); > > + > > + qmp_ufs_com_exit(qmp); > > Just inline it, unless you have any other plans. I mean, inline a call to qmp_ufs_com_exit() into qmp_ufs_power_off(). > > > + > > + return 0; > > +} > > + > > static int qmp_ufs_power_off(struct phy *phy) > > { > > struct qmp_ufs *qmp = phy_get_drvdata(phy); > > @@ -1851,28 +1860,11 @@ static int qmp_ufs_power_off(struct phy *phy) > > qphy_clrbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], > > SW_PWRDN); > > > > - return 0; > > -} > > - > > -static int qmp_ufs_exit(struct phy *phy) > > -{ > > - struct qmp_ufs *qmp = phy_get_drvdata(phy); > > - > > - qmp_ufs_com_exit(qmp); > > + qmp_ufs_exit(phy); > > > > return 0; > > } > > > > -static int qmp_ufs_disable(struct phy *phy) > > -{ > > - int ret; > > - > > - ret = qmp_ufs_power_off(phy); > > - if (ret) > > - return ret; > > - return qmp_ufs_exit(phy); > > -} > > - > > static int qmp_ufs_set_mode(struct phy *phy, enum phy_mode mode, int submode) > > { > > struct qmp_ufs *qmp = phy_get_drvdata(phy); > > @@ -1921,7 +1913,7 @@ static int qmp_ufs_phy_init(struct phy *phy) > > static const struct phy_ops qcom_qmp_ufs_phy_ops = { > > .init = qmp_ufs_phy_init, > > .power_on = qmp_ufs_power_on, > > - .power_off = qmp_ufs_disable, > > + .power_off = qmp_ufs_power_off, > > .calibrate = qmp_ufs_phy_calibrate, > > .set_mode = qmp_ufs_set_mode, > > .owner = THIS_MODULE, > > -- > > 2.48.1 > > > > -- > With best wishes > Dmitry > > -- > linux-phy mailing list > linux-phy@lists.infradead.org > https://lists.infradead.org/mailman/listinfo/linux-phy
On Sun, May 04, 2025 at 09:22:06PM +0530, Nitin Rawat wrote: > > > On 5/4/2025 9:07 PM, Dmitry Baryshkov wrote: > > On Sat, May 03, 2025 at 09:54:35PM +0530, Nitin Rawat wrote: > > > Rename qmp_ufs_disable to qmp_ufs_power_off to better represent its > > > functionality. Additionally, move the qmp_ufs_exit() call inside > > > qmp_ufs_power_off to preserve the functionality of .power_off. > > > > > > There is no functional change. > > > > > > Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com> > > > --- > > > drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 30 +++++++++---------------- > > > 1 file changed, 11 insertions(+), 19 deletions(-) > > > > > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > > > index 94095393148c..c501223fc5f9 100644 > > > --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > > > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c > > > @@ -1835,6 +1835,15 @@ static int qmp_ufs_phy_calibrate(struct phy *phy) > > > return 0; > > > } > > > > > > +static int qmp_ufs_exit(struct phy *phy) > > > +{ > > > + struct qmp_ufs *qmp = phy_get_drvdata(phy); > > > + > > > + qmp_ufs_com_exit(qmp); > > > > Just inline it, unless you have any other plans. > > Hi Dmitry, > > I have inlined qcom_ufs_com_exit in patch #7 of the same series. I separated > it into a different patch to keep each patch simpler. You have inlined qmp_ufs_com_exit() contents. Here I've asked you to inline qmp_ufs_exit(), keeping qmp_ufs_com_exit() as is. > > Could you please review patch #7 and share your thoughts. > > [PATCH V4 07/11] phy: qcom-qmp-ufs: Remove qmp_ufs_exit() and Inline > qmp_ufs_com_exit(). > > > Regards, > Nitin > > > > > > > + > > > + return 0; > > > +} > > > + > > > static int qmp_ufs_power_off(struct phy *phy) > > > { > > > struct qmp_ufs *qmp = phy_get_drvdata(phy); > > > @@ -1851,28 +1860,11 @@ static int qmp_ufs_power_off(struct phy *phy) > > > qphy_clrbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], > > > SW_PWRDN); > > > > > > - return 0; > > > -} > > > - > > > -static int qmp_ufs_exit(struct phy *phy) > > > -{ > > > - struct qmp_ufs *qmp = phy_get_drvdata(phy); > > > - > > > - qmp_ufs_com_exit(qmp); > > > + qmp_ufs_exit(phy); > > > > > > return 0; > > > } > > > > > > -static int qmp_ufs_disable(struct phy *phy) > > > -{ > > > - int ret; > > > - > > > - ret = qmp_ufs_power_off(phy); > > > - if (ret) > > > - return ret; > > > - return qmp_ufs_exit(phy); > > > -} > > > - > > > static int qmp_ufs_set_mode(struct phy *phy, enum phy_mode mode, int submode) > > > { > > > struct qmp_ufs *qmp = phy_get_drvdata(phy); > > > @@ -1921,7 +1913,7 @@ static int qmp_ufs_phy_init(struct phy *phy) > > > static const struct phy_ops qcom_qmp_ufs_phy_ops = { > > > .init = qmp_ufs_phy_init, > > > .power_on = qmp_ufs_power_on, > > > - .power_off = qmp_ufs_disable, > > > + .power_off = qmp_ufs_power_off, > > > .calibrate = qmp_ufs_phy_calibrate, > > > .set_mode = qmp_ufs_set_mode, > > > .owner = THIS_MODULE, > > > -- > > > 2.48.1 > > > > > > > > -- > linux-phy mailing list > linux-phy@lists.infradead.org > https://lists.infradead.org/mailman/listinfo/linux-phy
On 5/6/2025 5:23 PM, Dmitry Baryshkov wrote: > On Sun, May 04, 2025 at 09:22:06PM +0530, Nitin Rawat wrote: >> >> >> On 5/4/2025 9:07 PM, Dmitry Baryshkov wrote: >>> On Sat, May 03, 2025 at 09:54:35PM +0530, Nitin Rawat wrote: >>>> Rename qmp_ufs_disable to qmp_ufs_power_off to better represent its >>>> functionality. Additionally, move the qmp_ufs_exit() call inside >>>> qmp_ufs_power_off to preserve the functionality of .power_off. >>>> >>>> There is no functional change. >>>> >>>> Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com> >>>> --- >>>> drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 30 +++++++++---------------- >>>> 1 file changed, 11 insertions(+), 19 deletions(-) >>>> >>>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c >>>> index 94095393148c..c501223fc5f9 100644 >>>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c >>>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c >>>> @@ -1835,6 +1835,15 @@ static int qmp_ufs_phy_calibrate(struct phy *phy) >>>> return 0; >>>> } >>>> >>>> +static int qmp_ufs_exit(struct phy *phy) >>>> +{ >>>> + struct qmp_ufs *qmp = phy_get_drvdata(phy); >>>> + >>>> + qmp_ufs_com_exit(qmp); >>> >>> Just inline it, unless you have any other plans. >> >> Hi Dmitry, >> >> I have inlined qcom_ufs_com_exit in patch #7 of the same series. I separated >> it into a different patch to keep each patch simpler. > > You have inlined qmp_ufs_com_exit() contents. Here I've asked you to > inline qmp_ufs_exit(), keeping qmp_ufs_com_exit() as is. Sure Dmitry. I'll update this in next patchset. Thanks > >> >> Could you please review patch #7 and share your thoughts. >> >> [PATCH V4 07/11] phy: qcom-qmp-ufs: Remove qmp_ufs_exit() and Inline >> qmp_ufs_com_exit(). >> >> >> Regards, >> Nitin >> >> >>> >>>> + >>>> + return 0; >>>> +} >>>> + >>>> static int qmp_ufs_power_off(struct phy *phy) >>>> { >>>> struct qmp_ufs *qmp = phy_get_drvdata(phy); >>>> @@ -1851,28 +1860,11 @@ static int qmp_ufs_power_off(struct phy *phy) >>>> qphy_clrbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], >>>> SW_PWRDN); >>>> >>>> - return 0; >>>> -} >>>> - >>>> -static int qmp_ufs_exit(struct phy *phy) >>>> -{ >>>> - struct qmp_ufs *qmp = phy_get_drvdata(phy); >>>> - >>>> - qmp_ufs_com_exit(qmp); >>>> + qmp_ufs_exit(phy); >>>> >>>> return 0; >>>> } >>>> >>>> -static int qmp_ufs_disable(struct phy *phy) >>>> -{ >>>> - int ret; >>>> - >>>> - ret = qmp_ufs_power_off(phy); >>>> - if (ret) >>>> - return ret; >>>> - return qmp_ufs_exit(phy); >>>> -} >>>> - >>>> static int qmp_ufs_set_mode(struct phy *phy, enum phy_mode mode, int submode) >>>> { >>>> struct qmp_ufs *qmp = phy_get_drvdata(phy); >>>> @@ -1921,7 +1913,7 @@ static int qmp_ufs_phy_init(struct phy *phy) >>>> static const struct phy_ops qcom_qmp_ufs_phy_ops = { >>>> .init = qmp_ufs_phy_init, >>>> .power_on = qmp_ufs_power_on, >>>> - .power_off = qmp_ufs_disable, >>>> + .power_off = qmp_ufs_power_off, >>>> .calibrate = qmp_ufs_phy_calibrate, >>>> .set_mode = qmp_ufs_set_mode, >>>> .owner = THIS_MODULE, >>>> -- >>>> 2.48.1 >>>> >>> >> >> >> -- >> linux-phy mailing list >> linux-phy@lists.infradead.org >> https://lists.infradead.org/mailman/listinfo/linux-phy >
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c index 94095393148c..c501223fc5f9 100644 --- a/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c +++ b/drivers/phy/qualcomm/phy-qcom-qmp-ufs.c @@ -1835,6 +1835,15 @@ static int qmp_ufs_phy_calibrate(struct phy *phy) return 0; } +static int qmp_ufs_exit(struct phy *phy) +{ + struct qmp_ufs *qmp = phy_get_drvdata(phy); + + qmp_ufs_com_exit(qmp); + + return 0; +} + static int qmp_ufs_power_off(struct phy *phy) { struct qmp_ufs *qmp = phy_get_drvdata(phy); @@ -1851,28 +1860,11 @@ static int qmp_ufs_power_off(struct phy *phy) qphy_clrbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL], SW_PWRDN); - return 0; -} - -static int qmp_ufs_exit(struct phy *phy) -{ - struct qmp_ufs *qmp = phy_get_drvdata(phy); - - qmp_ufs_com_exit(qmp); + qmp_ufs_exit(phy); return 0; } -static int qmp_ufs_disable(struct phy *phy) -{ - int ret; - - ret = qmp_ufs_power_off(phy); - if (ret) - return ret; - return qmp_ufs_exit(phy); -} - static int qmp_ufs_set_mode(struct phy *phy, enum phy_mode mode, int submode) { struct qmp_ufs *qmp = phy_get_drvdata(phy); @@ -1921,7 +1913,7 @@ static int qmp_ufs_phy_init(struct phy *phy) static const struct phy_ops qcom_qmp_ufs_phy_ops = { .init = qmp_ufs_phy_init, .power_on = qmp_ufs_power_on, - .power_off = qmp_ufs_disable, + .power_off = qmp_ufs_power_off, .calibrate = qmp_ufs_phy_calibrate, .set_mode = qmp_ufs_set_mode, .owner = THIS_MODULE,
Rename qmp_ufs_disable to qmp_ufs_power_off to better represent its functionality. Additionally, move the qmp_ufs_exit() call inside qmp_ufs_power_off to preserve the functionality of .power_off. There is no functional change. Signed-off-by: Nitin Rawat <quic_nitirawa@quicinc.com> --- drivers/phy/qualcomm/phy-qcom-qmp-ufs.c | 30 +++++++++---------------- 1 file changed, 11 insertions(+), 19 deletions(-) -- 2.48.1