| Message ID | 20200908064507.30774-4-stanley.chu@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | scsi: ufs-mediatek: Fixes for kernel v5.10 | expand |
> > Forcibly leave UniPro low-power mode if UIC commands is failed. > This makes hba_enable_delay_us as correct (default) value for > re-enabling the host. > > At the same time, change type of parameter "lpm" in function > ufs_mtk_unipro_set_pm() to "bool". Semantically, better leave it u32 as its eventually assigned to the arg3 of the uic command > > Signed-off-by: Stanley Chu <stanley.chu@mediatek.com> > --- > drivers/scsi/ufs/ufs-mediatek.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c > index 887c03e8bcc0..feba74a72309 100644 > --- a/drivers/scsi/ufs/ufs-mediatek.c > +++ b/drivers/scsi/ufs/ufs-mediatek.c > @@ -419,7 +419,7 @@ static int ufs_mtk_pwr_change_notify(struct ufs_hba > *hba, > return ret; > } > > -static int ufs_mtk_unipro_set_pm(struct ufs_hba *hba, u32 lpm) > +static int ufs_mtk_unipro_set_pm(struct ufs_hba *hba, bool lpm) > { > int ret; > struct ufs_mtk_host *host = ufshcd_get_variant(hba); > @@ -427,8 +427,14 @@ static int ufs_mtk_unipro_set_pm(struct ufs_hba > *hba, u32 lpm) > ret = ufshcd_dme_set(hba, > UIC_ARG_MIB_SEL(VS_UNIPROPOWERDOWNCONTROL, 0), > lpm); > - if (!ret) > + if (!ret || !lpm) { > + /* > + * Forcibly set as non-LPM mode if UIC commands is failed > + * to use default hba_enable_delay_us value for re-enabling > + * the host. > + */ > host->unipro_lpm = lpm; Maybe just host->unipro_lpm = false; instead
Hi Avri, On Sat, 2020-09-19 at 08:08 +0000, Avri Altman wrote: > > > > Forcibly leave UniPro low-power mode if UIC commands is failed. > > This makes hba_enable_delay_us as correct (default) value for > > re-enabling the host. > > > > At the same time, change type of parameter "lpm" in function > > ufs_mtk_unipro_set_pm() to "bool". > Semantically, better leave it u32 as its eventually assigned to the arg3 of the uic command > Thanks for reminding. I will use specific u32 values while sending arg3 to fix this. > > > > Signed-off-by: Stanley Chu <stanley.chu@mediatek.com> > > --- > > drivers/scsi/ufs/ufs-mediatek.c | 20 ++++++++++++++------ > > 1 file changed, 14 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c > > index 887c03e8bcc0..feba74a72309 100644 > > --- a/drivers/scsi/ufs/ufs-mediatek.c > > +++ b/drivers/scsi/ufs/ufs-mediatek.c > > @@ -419,7 +419,7 @@ static int ufs_mtk_pwr_change_notify(struct ufs_hba > > *hba, > > return ret; > > } > > > > -static int ufs_mtk_unipro_set_pm(struct ufs_hba *hba, u32 lpm) > > +static int ufs_mtk_unipro_set_pm(struct ufs_hba *hba, bool lpm) > > { > > int ret; > > struct ufs_mtk_host *host = ufshcd_get_variant(hba); > > @@ -427,8 +427,14 @@ static int ufs_mtk_unipro_set_pm(struct ufs_hba > > *hba, u32 lpm) > > ret = ufshcd_dme_set(hba, > > UIC_ARG_MIB_SEL(VS_UNIPROPOWERDOWNCONTROL, 0), > > lpm); > > - if (!ret) > > + if (!ret || !lpm) { > > + /* > > + * Forcibly set as non-LPM mode if UIC commands is failed > > + * to use default hba_enable_delay_us value for re-enabling > > + * the host. > > + */ > > host->unipro_lpm = lpm; > Maybe just host->unipro_lpm = false; instead This statement shall stay like this for case: "lpm = true" Thanks, Stanley Chu
diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c index 887c03e8bcc0..feba74a72309 100644 --- a/drivers/scsi/ufs/ufs-mediatek.c +++ b/drivers/scsi/ufs/ufs-mediatek.c @@ -419,7 +419,7 @@ static int ufs_mtk_pwr_change_notify(struct ufs_hba *hba, return ret; } -static int ufs_mtk_unipro_set_pm(struct ufs_hba *hba, u32 lpm) +static int ufs_mtk_unipro_set_pm(struct ufs_hba *hba, bool lpm) { int ret; struct ufs_mtk_host *host = ufshcd_get_variant(hba); @@ -427,8 +427,14 @@ static int ufs_mtk_unipro_set_pm(struct ufs_hba *hba, u32 lpm) ret = ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(VS_UNIPROPOWERDOWNCONTROL, 0), lpm); - if (!ret) + if (!ret || !lpm) { + /* + * Forcibly set as non-LPM mode if UIC commands is failed + * to use default hba_enable_delay_us value for re-enabling + * the host. + */ host->unipro_lpm = lpm; + } return ret; } @@ -438,7 +444,9 @@ static int ufs_mtk_pre_link(struct ufs_hba *hba) int ret; u32 tmp; - ufs_mtk_unipro_set_pm(hba, 0); + ret = ufs_mtk_unipro_set_pm(hba, false); + if (ret) + return ret; /* * Setting PA_Local_TX_LCC_Enable to 0 before link startup @@ -546,7 +554,7 @@ static int ufs_mtk_link_set_hpm(struct ufs_hba *hba) if (err) return err; - err = ufs_mtk_unipro_set_pm(hba, 0); + err = ufs_mtk_unipro_set_pm(hba, false); if (err) return err; @@ -567,10 +575,10 @@ static int ufs_mtk_link_set_lpm(struct ufs_hba *hba) { int err; - err = ufs_mtk_unipro_set_pm(hba, 1); + err = ufs_mtk_unipro_set_pm(hba, true); if (err) { /* Resume UniPro state for following error recovery */ - ufs_mtk_unipro_set_pm(hba, 0); + ufs_mtk_unipro_set_pm(hba, false); return err; }
Forcibly leave UniPro low-power mode if UIC commands is failed. This makes hba_enable_delay_us as correct (default) value for re-enabling the host. At the same time, change type of parameter "lpm" in function ufs_mtk_unipro_set_pm() to "bool". Signed-off-by: Stanley Chu <stanley.chu@mediatek.com> --- drivers/scsi/ufs/ufs-mediatek.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)