| Message ID | 1620813706-25331-1-git-send-email-peter.wang@mediatek.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4] scsi: ufs-mediatek: fix ufs power down specs violation | expand |
Hi Peter, On Wed, 2021-05-12 at 18:01 +0800, peter.wang@mediatek.com wrote: > From: Peter Wang <peter.wang@mediatek.com> > > As per specs, e.g, JESD220E chapter 7.2, while powering off > the ufs device, RST_N signal should be between VSS(Ground) > and VCCQ/VCCQ2. The power down sequence after fixing as below: > > Power down: > 1. Assert RST_N low > 2. Turn-off VCC > 3. Turn-off VCCQ/VCCQ2 > > Signed-off-by: Peter Wang <peter.wang@mediatek.com> > --- > drivers/scsi/ufs/ufs-mediatek.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c > index c55202b..47b4066 100644 > --- a/drivers/scsi/ufs/ufs-mediatek.c > +++ b/drivers/scsi/ufs/ufs-mediatek.c > @@ -922,6 +922,7 @@ static void ufs_mtk_vreg_set_lpm(struct ufs_hba *hba, bool lpm) > static int ufs_mtk_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) > { > int err; > + struct arm_smccc_res res; > > if (ufshcd_is_link_hibern8(hba)) { > err = ufs_mtk_link_set_lpm(hba); > @@ -941,6 +942,9 @@ static int ufs_mtk_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) > goto fail; > } > > + if (ufshcd_is_link_off(hba)) > + ufs_mtk_device_reset_ctrl(0, res); > + For the case "linkoff @ suspend", HW reset pin can be de-asserted via ufshcd_resume() -> ufshcd_reset_and_restore() -> ufshcd_vops_device_reset(), so this looks good to me. Reviewed-by: Stanley Chu <stanley.chu@mediatek.com>
> From: Peter Wang <peter.wang@mediatek.com> > > As per specs, e.g, JESD220E chapter 7.2, while powering off > the ufs device, RST_N signal should be between VSS(Ground) > and VCCQ/VCCQ2. The power down sequence after fixing as below: > > Power down: > 1. Assert RST_N low > 2. Turn-off VCC > 3. Turn-off VCCQ/VCCQ2 > > Signed-off-by: Peter Wang <peter.wang@mediatek.com> > --- > drivers/scsi/ufs/ufs-mediatek.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c > index c55202b..47b4066 100644 > --- a/drivers/scsi/ufs/ufs-mediatek.c > +++ b/drivers/scsi/ufs/ufs-mediatek.c > @@ -922,6 +922,7 @@ static void ufs_mtk_vreg_set_lpm(struct ufs_hba > *hba, bool lpm) > static int ufs_mtk_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) > { > int err; > + struct arm_smccc_res res; > > if (ufshcd_is_link_hibern8(hba)) { > err = ufs_mtk_link_set_lpm(hba); > @@ -941,6 +942,9 @@ static int ufs_mtk_suspend(struct ufs_hba *hba, > enum ufs_pm_op pm_op) > goto fail; > } > > + if (ufshcd_is_link_off(hba)) > + ufs_mtk_device_reset_ctrl(0, res); According to your commit log, you should call reset just before ufs_mtk_vreg_set_lpm, or turn phy off, whichever turn off vcc - Few lines above. Thanks, Avri > + > return 0; > fail: > /* > -- > 1.7.9.5
On Thu, 2021-05-13 at 06:45 +0000, Avri Altman wrote: > > From: Peter Wang <peter.wang@mediatek.com> > > > > As per specs, e.g, JESD220E chapter 7.2, while powering off > > the ufs device, RST_N signal should be between VSS(Ground) > > and VCCQ/VCCQ2. The power down sequence after fixing as below: > > > > Power down: > > 1. Assert RST_N low > > 2. Turn-off VCC > > 3. Turn-off VCCQ/VCCQ2 > > > > Signed-off-by: Peter Wang <peter.wang@mediatek.com> > > --- > > drivers/scsi/ufs/ufs-mediatek.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c > > index c55202b..47b4066 100644 > > --- a/drivers/scsi/ufs/ufs-mediatek.c > > +++ b/drivers/scsi/ufs/ufs-mediatek.c > > @@ -922,6 +922,7 @@ static void ufs_mtk_vreg_set_lpm(struct ufs_hba > > *hba, bool lpm) > > static int ufs_mtk_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) > > { > > int err; > > + struct arm_smccc_res res; > > > > if (ufshcd_is_link_hibern8(hba)) { > > err = ufs_mtk_link_set_lpm(hba); > > @@ -941,6 +942,9 @@ static int ufs_mtk_suspend(struct ufs_hba *hba, > > enum ufs_pm_op pm_op) > > goto fail; > > } > > > > + if (ufshcd_is_link_off(hba)) > > + ufs_mtk_device_reset_ctrl(0, res); > According to your commit log, you should call reset just before > ufs_mtk_vreg_set_lpm, or turn phy off, whichever turn off vcc - > Few lines above. > > Thanks, > Avri ufs_mtk_vreg_set_lpm only set vccq2 power mode to lpm, dose't trun off vccq2.And turn phy off is also turn off vcc, dose't turn off vccq2. RST_N keep high is no problem when we trun off vcc and vccq2 keep on. But RST_N will got problem if we trun off vccq2. (RST_N signal should be between VSS(Ground) and VCCQ/VCCQ2) Here set RST_N to low is after shut down pm set link off, and before shutdown pm turn off vccq2. Thanks Peter > > > + > > return 0; > > fail: > > /* > > -- > > 1.7.9.5 >
> > > + if (ufshcd_is_link_off(hba)) > > > + ufs_mtk_device_reset_ctrl(0, res); > > According to your commit log, you should call reset just before > > ufs_mtk_vreg_set_lpm, or turn phy off, whichever turn off vcc - > > Few lines above. > > > > Thanks, > > Avri > > ufs_mtk_vreg_set_lpm only set vccq2 power mode to lpm, dose't trun off > vccq2.And turn phy off is also turn off vcc, dose't turn off vccq2. > RST_N keep high is no problem when we trun off vcc and vccq2 keep on. > But RST_N will got problem if we trun off vccq2. > (RST_N signal should be between VSS(Ground) and VCCQ/VCCQ2) > Here set RST_N to low is after shut down pm set link off, and before > shutdown pm turn off vccq2. OK. Thanks for clarifying this. Thanks, Avri > > Thanks > Peter > > > > > > + > > > return 0; > > > fail: > > > /* > > > -- > > > 1.7.9.5 > >
On Wed, 12 May 2021 18:01:45 +0800, peter.wang@mediatek.com wrote: > As per specs, e.g, JESD220E chapter 7.2, while powering off > the ufs device, RST_N signal should be between VSS(Ground) > and VCCQ/VCCQ2. The power down sequence after fixing as below: > > Power down: > 1. Assert RST_N low > 2. Turn-off VCC > 3. Turn-off VCCQ/VCCQ2 Applied to 5.13/scsi-fixes, thanks! [1/1] scsi: ufs-mediatek: fix ufs power down specs violation https://git.kernel.org/mkp/scsi/c/c625b80b9d00
diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c index c55202b..47b4066 100644 --- a/drivers/scsi/ufs/ufs-mediatek.c +++ b/drivers/scsi/ufs/ufs-mediatek.c @@ -922,6 +922,7 @@ static void ufs_mtk_vreg_set_lpm(struct ufs_hba *hba, bool lpm) static int ufs_mtk_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) { int err; + struct arm_smccc_res res; if (ufshcd_is_link_hibern8(hba)) { err = ufs_mtk_link_set_lpm(hba); @@ -941,6 +942,9 @@ static int ufs_mtk_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) goto fail; } + if (ufshcd_is_link_off(hba)) + ufs_mtk_device_reset_ctrl(0, res); + return 0; fail: /*