Hi Avri, On Tue, 2020-11-03 at 07:12 +0000, Avri Altman wrote: > > > > In ufs_mtk_unipro_set_lpm(), use specific unsigned values > > as the argument to invoke ufshcd_dme_set(). > > > > In the same time, change the name of ufs_mtk_unipro_set_pm() > > to ufs_mtk_unipro_set_lpm() to align the naming convention > > in MediaTek UFS driver. > > > > Signed-off-by: Stanley Chu <stanley.chu@xxxxxxxxxxxx> > Looks like this patch is gilding the lily? Thanks for the review. Please see explanation below. > > > --- > > drivers/scsi/ufs/ufs-mediatek.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c > > index 8df73bc2f8cb..0196a89055b5 100644 > > --- a/drivers/scsi/ufs/ufs-mediatek.c > > +++ b/drivers/scsi/ufs/ufs-mediatek.c > > @@ -639,14 +639,14 @@ static int ufs_mtk_pwr_change_notify(struct > > ufs_hba *hba, > > return ret; > > } > > > > -static int ufs_mtk_unipro_set_pm(struct ufs_hba *hba, bool lpm) > > +static int ufs_mtk_unipro_set_lpm(struct ufs_hba *hba, bool lpm) > > { > > int ret; > > struct ufs_mtk_host *host = ufshcd_get_variant(hba); > > > > ret = ufshcd_dme_set(hba, > > UIC_ARG_MIB_SEL(VS_UNIPROPOWERDOWNCONTROL, 0), > > - lpm); > > + lpm ? 1 : 0); > bool is implicitly converted to int anyway? This change is the echo of your suggestion in https://patchwork.kernel.org/project/linux-scsi/patch/20200908064507.30774-4-stanley.chu@xxxxxxxxxxxx/ Actually I think your suggestion is constructive that the accepted values of VS_UNIPROPOWERDOWNCONTROL are better clearly defined so I use the casting here in this patch. > > > if (!ret || !lpm) { > > /* > > * Forcibly set as non-LPM mode if UIC commands is failed > > @@ -664,7 +664,7 @@ static int ufs_mtk_pre_link(struct ufs_hba *hba) > > int ret; > > u32 tmp; > > > > - ret = ufs_mtk_unipro_set_pm(hba, false); > > + ret = ufs_mtk_unipro_set_lpm(hba, false); > > if (ret) > > return ret; > > > > @@ -774,7 +774,7 @@ static int ufs_mtk_link_set_hpm(struct ufs_hba > > *hba) > > if (err) > > return err; > > > > - err = ufs_mtk_unipro_set_pm(hba, false); > > + err = ufs_mtk_unipro_set_lpm(hba, false); > > if (err) > > return err; > > > > @@ -795,10 +795,10 @@ static int ufs_mtk_link_set_lpm(struct ufs_hba > > *hba) > > { > > int err; > > > > - err = ufs_mtk_unipro_set_pm(hba, true); > > + err = ufs_mtk_unipro_set_lpm(hba, true); > > if (err) { > > /* Resume UniPro state for following error recovery */ > > - ufs_mtk_unipro_set_pm(hba, false); > > + ufs_mtk_unipro_set_lpm(hba, false); > > return err; > > } > > > > -- > > 2.18.0 Thanks, Stanley Chu