| Message ID | 1581485910-8307-2-git-send-email-cang@codeaurora.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Select ADAPT type when do gear scaling | expand |
Hi, > > When scale gear, use ufshcd_config_pwr_mode() instead of > ufshcd_change_power_mode() so that > vops_pwr_change_notify(PRE_CHANGE) > can be utilized to allow vendors use customized settings before change > the power mode. > > Signed-off-by: Can Guo <cang@codeaurora.org> > --- > drivers/scsi/ufs/ufshcd.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index adcce41..67bd4f2 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -1059,8 +1059,7 @@ static int ufshcd_scale_gear(struct ufs_hba *hba, > bool scale_up) > } > > /* check if the power mode needs to be changed or not? */ > - ret = ufshcd_change_power_mode(hba, &new_pwr_info); > - > + ret = ufshcd_config_pwr_mode(hba, &new_pwr_info); You might want to inform ufshcd_config_pwr_mode() of the caller, As now it will be called much more frequently, and you want/don't want To call your vops on probe but not on scale_gear? Also, Alim exported ufshcd_config_pwr_mode a while ago, In commit 0d846e703dc8 "scsi: ufs: make ufshcd_config_pwr_mode of non-static func"), But nobody uses it outside ufshcd - so maybe revert this commit as part of this series? > if (ret) > dev_err(hba->dev, "%s: failed err %d, old gear: (tx %d rx %d), new > gear: (tx %d rx %d)", > __func__, ret, > @@ -4126,8 +4125,6 @@ int ufshcd_config_pwr_mode(struct ufs_hba *hba, > memcpy(&final_params, desired_pwr_mode, sizeof(final_params)); > > ret = ufshcd_change_power_mode(hba, &final_params); > - if (!ret) > - ufshcd_print_pwr_info(hba); > > return ret; > } > @@ -7157,6 +7154,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, > bool async) > __func__, ret); > goto out; > } > + ufshcd_print_pwr_info(hba); > } > > /* set the state as operational after switching to desired gear */ > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora > Forum, > a Linux Foundation Collaborative Project
On 2020-02-12 20:21, Avri Altman wrote: > Hi, > >> >> When scale gear, use ufshcd_config_pwr_mode() instead of >> ufshcd_change_power_mode() so that >> vops_pwr_change_notify(PRE_CHANGE) >> can be utilized to allow vendors use customized settings before change >> the power mode. >> >> Signed-off-by: Can Guo <cang@codeaurora.org> >> --- >> drivers/scsi/ufs/ufshcd.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c >> index adcce41..67bd4f2 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -1059,8 +1059,7 @@ static int ufshcd_scale_gear(struct ufs_hba >> *hba, >> bool scale_up) >> } >> >> /* check if the power mode needs to be changed or not? */ >> - ret = ufshcd_change_power_mode(hba, &new_pwr_info); >> - >> + ret = ufshcd_config_pwr_mode(hba, &new_pwr_info); > > You might want to inform ufshcd_config_pwr_mode() of the caller, > As now it will be called much more frequently, and you want/don't want > To call your vops on probe but not on scale_gear? > > Also, Alim exported ufshcd_config_pwr_mode a while ago, > In commit 0d846e703dc8 "scsi: ufs: make ufshcd_config_pwr_mode of > non-static func"), > But nobody uses it outside ufshcd - so maybe revert this commit as > part of this series? > > > Hi Avri, Thanks for your review and suggestion. What I get from your suggestion is that add one more parameter to ufshcd_config_pwr_mode(say: bool from_probe), and pass the param to ufshcd_vops_pwr_change_notify() so that vendor drivers know where is the call comes from? If I get it correctly, then yes, we can do that. However, in ufs-qcom.c, ufs_qcom_pwr_change_notify(PRE_CHANGE) is fine being called every time whenever there is a power mode change, we have no problem with that. And as we are the only user of clock scaling in the latest code base, so this change does not impact any other vendors. If later other vendors need to use clock scaling, they can do this change as you suggested above. As for the commit from Alim, I think I should leave it to Alim's call on it, maybe he has changes that use this func outside ufshcd.c but not uploaded yet. Thanks, Can Guo. >> if (ret) >> dev_err(hba->dev, "%s: failed err %d, old gear: (tx %d >> rx %d), new >> gear: (tx %d rx %d)", >> __func__, ret, >> @@ -4126,8 +4125,6 @@ int ufshcd_config_pwr_mode(struct ufs_hba *hba, >> memcpy(&final_params, desired_pwr_mode, >> sizeof(final_params)); >> >> ret = ufshcd_change_power_mode(hba, &final_params); >> - if (!ret) >> - ufshcd_print_pwr_info(hba); >> >> return ret; >> } >> @@ -7157,6 +7154,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, >> bool async) >> __func__, ret); >> goto out; >> } >> + ufshcd_print_pwr_info(hba); >> } >> >> /* set the state as operational after switching to desired >> gear */ >> -- >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora >> Forum, >> a Linux Foundation Collaborative Project
> > On 2020-02-12 20:21, Avri Altman wrote: > > Hi, > > > >> > >> When scale gear, use ufshcd_config_pwr_mode() instead of > >> ufshcd_change_power_mode() so that > >> vops_pwr_change_notify(PRE_CHANGE) > >> can be utilized to allow vendors use customized settings before change > >> the power mode. > >> > >> Signed-off-by: Can Guo <cang@codeaurora.org> Reviewed-by: Avri Altman <avri.altman@wdc.com>
On Tue, 2020-02-11 at 21:38 -0800, Can Guo wrote: > When scale gear, use ufshcd_config_pwr_mode() instead of > ufshcd_change_power_mode() so that vops_pwr_change_notify(PRE_CHANGE) > can be utilized to allow vendors use customized settings before change > the power mode. > > Signed-off-by: Can Guo <cang@codeaurora.org> Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index adcce41..67bd4f2 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -1059,8 +1059,7 @@ static int ufshcd_scale_gear(struct ufs_hba *hba, bool scale_up) } /* check if the power mode needs to be changed or not? */ - ret = ufshcd_change_power_mode(hba, &new_pwr_info); - + ret = ufshcd_config_pwr_mode(hba, &new_pwr_info); if (ret) dev_err(hba->dev, "%s: failed err %d, old gear: (tx %d rx %d), new gear: (tx %d rx %d)", __func__, ret, @@ -4126,8 +4125,6 @@ int ufshcd_config_pwr_mode(struct ufs_hba *hba, memcpy(&final_params, desired_pwr_mode, sizeof(final_params)); ret = ufshcd_change_power_mode(hba, &final_params); - if (!ret) - ufshcd_print_pwr_info(hba); return ret; } @@ -7157,6 +7154,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool async) __func__, ret); goto out; } + ufshcd_print_pwr_info(hba); } /* set the state as operational after switching to desired gear */
When scale gear, use ufshcd_config_pwr_mode() instead of ufshcd_change_power_mode() so that vops_pwr_change_notify(PRE_CHANGE) can be utilized to allow vendors use customized settings before change the power mode. Signed-off-by: Can Guo <cang@codeaurora.org> --- drivers/scsi/ufs/ufshcd.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)