scsi: ufs: Fix ufs power down/on specs violation
Subsystems (3)
RFC PATCH v4 1/1 scsi: ufs: Fix ufs power down/on specs violation
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device, RST_N signal and REF_CLK signal should be between VSS(Ground) and VCCQ/VCCQ2.
To flexibly control device reset line, refactor the function ufschd_vops_device_reset(sturct ufs_hba *hba) to ufshcd_ vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.
--- drivers/scsi/ufs/ufs-mediatek.c | 32 ++++++++++++++++---------------- drivers/scsi/ufs/ufs-qcom.c | 24 +++++++++++++++--------- drivers/scsi/ufs/ufshcd.c | 36 +++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 8 ++++---- 4 files changed, 60 insertions(+), 40 deletions(-)
drivers/scsi/ufs/ufs-mediatek.c +16 -16
@@ -841,27 +841,27 @@ static int ufs_mtk_link_startup_notify(struct ufs_hba *hba,return ret; } -static int ufs_mtk_device_reset (struct ufs_hba *hba) +static int ufs_mtk_device_reset (struct ufs_hba *hba,bool asserted) {struct arm_smccc_res res; -ufs_mtk_device_reset_ctrl (0 , res); +if (asserted) { +ufs_mtk_device_reset_ctrl (0 , res); -/* -* The reset signal is active low. UFS devices shall detect -* more than or equal to 1us of positive or negative RST_n -* pulse width. -* -* To be on safe side, keep the reset low for at least 10us. -*/ -usleep_range (10 ,15 ); - -ufs_mtk_device_reset_ctrl (1 , res); - -/* Some devices may need time to respond to rst_n */ -usleep_range (10000 ,15000 ); +/* +* The reset signal is active low. UFS devices shall detect +* more than or equal to 1us of positive or negative RST_n +* pulse width. +* +* To be on safe side, keep the reset low for at least 10us. +*/ +usleep_range (10 ,15 ); + }else { +ufs_mtk_device_reset_ctrl (1 , res); -dev_info (hba->dev,"device reset done\n" ); +/* Some devices may need time to respond to rst_n */ +usleep_range (10000 ,15000 ); + }return 0 ; }
drivers/scsi/ufs/ufs-qcom.c +15 -9
@@ -1406,10 +1406,11 @@ static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba)/** * ufs_qcom_device_reset() - toggle the (optional) device reset line * @hba: per-adapter instance +* @asserted: assert or deassert device reset line * * Toggles the (optional) reset line to reset the attached device. */ -static int ufs_qcom_device_reset (struct ufs_hba *hba) +static int ufs_qcom_device_reset (struct ufs_hba *hba,bool asserted) {struct ufs_qcom_host *host =ufshcd_get_variant (hba);
@@ -1417,15 +1418,20 @@ static int ufs_qcom_device_reset(struct ufs_hba *hba)if (!host->device_reset)return -EOPNOTSUPP ; -/* -* The UFS device shall detect reset pulses of 1us, sleep for 10us to -* be on the safe side. -*/ -gpiod_set_value_cansleep (host->device_reset,1 ); -usleep_range (10 ,15 ); +if (asserted) { +gpiod_set_value_cansleep (host->device_reset,1 ); -gpiod_set_value_cansleep (host->device_reset,0 ); -usleep_range (10 ,15 ); +/* +* The UFS device shall detect reset pulses of 1us, sleep for 10us to +* be on the safe side. +*/ +usleep_range (10 ,15 ); + }else { +gpiod_set_value_cansleep (host->device_reset,0 ); + +/* Some devices may need time to respond to rst_n */ +usleep_range (10 ,15 ); + }return 0 ; }
drivers/scsi/ufs/ufshcd.c +25 -11
@@ -585,7 +585,13 @@ static void ufshcd_device_reset(struct ufs_hba *hba) {int err; - err =ufshcd_vops_device_reset (hba); + err =ufshcd_vops_device_reset (hba, true); +if (err) { +dev_err (hba->dev,"asserting device reset failed: %d\n" , err); +return ; + } + + err =ufshcd_vops_device_reset (hba, false);if (!err) {ufshcd_set_ufs_dev_active (hba);
@@ -593,7 +599,11 @@ static void ufshcd_device_reset(struct ufs_hba *hba) hba->wb_enabled = false; hba->wb_buf_flush_enabled = false; } +dev_dbg (hba->dev,"device reset done\n" ); + }else { +dev_err (hba->dev,"deasserting device reset failed: %d\n" , err); } +if (err != -EOPNOTSUPP )ufshcd_update_evt_hist (hba,UFS_EVT_DEV_RESET , err); }
@@ -8686,8 +8696,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)if (ret) goto set_dev_active; -ufshcd_vreg_set_lpm (hba); - disable_clks:/* * Call vendor specific suspend callback. As these callbacks may access
@@ -8703,6 +8711,9 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)*/ ufshcd_disable_irq (hba); +if (ufshcd_is_link_off (hba)) +ufshcd_vops_device_reset (hba, true); +ufshcd_setup_clocks (hba, false);if (ufshcd_is_clkgating_allowed (hba)) {
@@ -8711,6 +8722,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) hba->clk_gating.state); } +ufshcd_vreg_set_lpm (hba); +/* Put the host controller in low power mode if possible */ ufshcd_hba_vreg_set_lpm (hba); goto out;
@@ -8778,18 +8791,19 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) old_link_state = hba->uic_link_state;ufshcd_hba_vreg_set_hpm (hba); + + ret =ufshcd_vreg_set_hpm (hba); +if (ret) + goto out; +/* Make sure clocks are enabled before accessing controller */ ret =ufshcd_setup_clocks (hba, true);if (ret) - goto out; + goto disable_vreg;/* enable the host irq as host controller would be active soon */ ufshcd_enable_irq (hba); - ret =ufshcd_vreg_set_hpm (hba); -if (ret) - goto disable_irq_and_vops_clks; -/* * Call vendor specific resume callback. As these callbacks may access * vendor specific host controller register space call them when the
@@ -8797,7 +8811,7 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)*/ ret =ufshcd_vops_resume (hba, pm_op);if (ret) - goto disable_vreg; + goto disable_irq_and_vops_clks;/* For DeepSleep, the only supported option is to have the link off */ WARN_ON (ufshcd_is_ufs_dev_deepsleep (hba) && !ufshcd_is_link_off (hba));
@@ -8864,8 +8878,6 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)ufshcd_link_state_transition (hba, old_link_state,0 ); vendor_suspend:ufshcd_vops_suspend (hba, pm_op); -disable_vreg: -ufshcd_vreg_set_lpm (hba); disable_irq_and_vops_clks:ufshcd_disable_irq (hba);if (hba->clk_scaling.is_allowed)
@@ -8876,6 +8888,8 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)trace_ufshcd_clk_gating (dev_name (hba->dev), hba->clk_gating.state); } +disable_vreg: +ufshcd_vreg_set_lpm (hba); out: hba->pm_op_in_progress =0 ;if (ret)
drivers/scsi/ufs/ufshcd.h +4 -4
@@ -319,7 +319,7 @@ struct ufs_pwr_mode_info {* @resume: called during host controller PM callback * @dbg_register_dump: used to dump controller debug information * @phy_initialization: used to initialize phys -* @device_reset: called to issue a reset pulse on the UFS device +* @device_reset: called to assert or deassert device reset line * @program_key: program or evict an inline encryption key * @event_notify: called to notify important events */
@@ -350,7 +350,7 @@ struct ufs_hba_variant_ops {int (*resume)(struct ufs_hba *,enum ufs_pm_op );void (*dbg_register_dump)(struct ufs_hba *hba);int (*phy_initialization)(struct ufs_hba *); -int (*device_reset)(struct ufs_hba *hba); +int (*device_reset)(struct ufs_hba *hba,bool asserted);void (*config_scaling_param)(struct ufs_hba *hba,struct devfreq_dev_profile *profile,void *data);
@@ -1216,10 +1216,10 @@ static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba) hba->vops->dbg_register_dump (hba); } -static inline int ufshcd_vops_device_reset (struct ufs_hba *hba) +static inline int ufshcd_vops_device_reset (struct ufs_hba *hba,bool asserted) {if (hba->vops && hba->vops->device_reset) -return hba->vops->device_reset (hba); +return hba->vops->device_reset (hba, asserted);return -EOPNOTSUPP ; }
Signature · 8 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ Linux-mediatek mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-mediatek
Quoted 96 lines
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.Cc Kiwoong Kim <[email protected]>Cc Stanley Chu <[email protected]>Signed-off-by Ziqi Chen <[email protected]>--- drivers/scsi/ufs/ufs-mediatek.c | 32 ++++++++++++++++---------------- drivers/scsi/ufs/ufs-qcom.c | 24 +++++++++++++++--------- drivers/scsi/ufs/ufshcd.c | 36 +++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 8 ++++---- 4 files changed, 60 insertions(+), 40 deletions(-)drivers/scsi/ufs/ufs- renamed from drivers/scsi/ufs/ufs-mediatek.c +0
-0
mediatek.cindex 80618af..072f4db 100644drivers/scsi/ufs/ufs-mediatek.c +16
-16@@ -841,27 +841,27 @@ static int ufs_mtk_link_startup_notify(structufs_hba *hba, return ret; } -static int ufs_mtk_device_reset (struct ufs_hba *hba) +static int ufs_mtk_device_reset (struct ufs_hba *hba,bool asserted) {struct arm_smccc_res res; -ufs_mtk_device_reset_ctrl (0 , res); +if (asserted) { +ufs_mtk_device_reset_ctrl (0 , res); -/* -* The reset signal is active low. UFS devices shall detect -* more than or equal to 1us of positive or negative RST_n -* pulse width. -* -* To be on safe side, keep the reset low for at least 10us. -*/ -usleep_range (10 ,15 ); - -ufs_mtk_device_reset_ctrl (1 , res); - -/* Some devices may need time to respond to rst_n */ -usleep_range (10000 ,15000 ); +/* +* The reset signal is active low. UFS devices shall detect +* more than or equal to 1us of positive or negative RST_n +* pulse width. +* +* To be on safe side, keep the reset low for at least 10us. +*/ +usleep_range (10 ,15 ); + }else { +ufs_mtk_device_reset_ctrl (1 , res); -dev_info (hba->dev,"device reset done\n" ); +/* Some devices may need time to respond to rst_n */ +usleep_range (10000 ,15000 ); + }return 0 ;}
drivers/scsi/ufs/ufs-qcom.c +2
-3@@ -1406,10 +1406,11 @@ static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba)/** * ufs_qcom_device_reset() - toggle the (optional) device reset line * @hba: per-adapter instance +* @asserted: assert or deassert device reset line * * Toggles the (optional) reset line to reset the attached device. */ -static int ufs_qcom_device_reset (struct ufs_hba *hba) +static int ufs_qcom_device_reset (struct ufs_hba *hba,bool asserted) {struct ufs_qcom_host *host =ufshcd_get_variant (hba);@@ -1417,15 +1418,20 @@ static int ufs_qcom_device_reset(struct ufs_hba *hba)if (!host->device_reset)return -EOPNOTSUPP ; -/* -* The UFS device shall detect reset pulses of 1us, sleep for 10us to
drivers/scsi/ufs/ufs-qcom.c +0
-1-* be on the safe side.
drivers/scsi/ufs/ufs-qcom.c static int ufs_qcom_device_reset(struct ufs_hba *hba)
Quoted 10 lines
drivers/scsi/ufs/ufs-qcom.c +4
-5-*/ -gpiod_set_value_cansleep (host->device_reset,1 ); -usleep_range (10 ,15 ); +if (asserted) { +gpiod_set_value_cansleep (host->device_reset,1 ); -gpiod_set_value_cansleep (host->device_reset,0 ); -usleep_range (10 ,15 ); +/* +* The UFS device shall detect reset pulses of 1us, sleep
for
Quoted 55 lines
10us to
drivers/scsi/ufs/ufs-qcom.c +9
-0+* be on the safe side. +*/ +usleep_range (10 ,15 ); + }else { +gpiod_set_value_cansleep (host->device_reset,0 ); + +/* Some devices may need time to respond to rst_n */ +usleep_range (10 ,15 ); + }return 0 ; }drivers/scsi/ufs/ufshcd.c index renamed from drivers/scsi/ufs/ufshcd.c +0
-0e221add..f2daac2 100644
drivers/scsi/ufs/ufshcd.c +7
-1@@ -585,7 +585,13 @@ static void ufshcd_device_reset(struct ufs_hba *hba) {int err; - err =ufshcd_vops_device_reset (hba); + err =ufshcd_vops_device_reset (hba, true); +if (err) { +dev_err (hba->dev,"asserting device reset failed: %d\n" , err); +return ; + } + + err =ufshcd_vops_device_reset (hba, false);if (!err) {drivers/scsi/ufs/ufshcd.c +4
-0ufshcd_set_ufs_dev_active (hba);@@ -593,7 +599,11 @@ static void ufshcd_device_reset(struct ufs_hba *hba) hba->wb_enabled = false; hba->wb_buf_flush_enabled = false; } +dev_dbg (hba->dev,"device reset done\n" ); + }else { +dev_err (hba->dev,"deasserting device reset failed: %d\n" , err); } +if (err != -EOPNOTSUPP ) ufshcd_update_evt_hist(hba, UFS_EVT_DEV_RESET, err); } @@ -8686,8 +8696,6 @@ static int
ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)drivers/scsi/ufs/ufshcd.c +0
-2if (ret) goto set_dev_active; -ufshcd_vreg_set_lpm (hba); - disable_clks:/* * Call vendor specific suspend callback. As these callbacks may
Quoted 1 line
access @@ -8703,6 +8711,9 @@ static int ufshcd_suspend(struct
ufs_hba
*hba,
Quoted 115 lines
enum
ufs_pm_oppm_op)drivers/scsi/ufs/ufshcd.c +3
-0*/ ufshcd_disable_irq (hba); +if (ufshcd_is_link_off (hba)) +ufshcd_vops_device_reset (hba, true); +ufshcd_setup_clocks (hba, false); if (ufshcd_is_clkgating_allowed(hba)) { @@ -8711,6 +8722,8 @@static int
ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)drivers/scsi/ufs/ufshcd.c +8
-5hba->clk_gating.state); } +ufshcd_vreg_set_lpm (hba); +/* Put the host controller in low power mode if possible */ ufshcd_hba_vreg_set_lpm (hba); goto out;@@ -8778,18 +8791,19 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) old_link_state = hba->uic_link_state;ufshcd_hba_vreg_set_hpm (hba); + + ret =ufshcd_vreg_set_hpm (hba); +if (ret) + goto out; +/* Make sure clocks are enabled before accessing controller */ ret =ufshcd_setup_clocks (hba, true);if (ret) - goto out; + goto disable_vreg;/* enable the host irq as host controller would be active soon */ ufshcd_enable_irq (hba); - ret =ufshcd_vreg_set_hpm (hba); -if (ret) - goto disable_irq_and_vops_clks; -/* * Call vendor specific resume callback. As these callbacks may access
* vendor specific host controller register space call them when
the @@ -8797,7 +8811,7 @@ static intufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)drivers/scsi/ufs/ufshcd.c +1
-1*/ ret =ufshcd_vops_resume (hba, pm_op);if (ret) - goto disable_vreg; + goto disable_irq_and_vops_clks;/* For DeepSleep, the only supported option is to have the link off */
WARN_ON(ufshcd_is_ufs_dev_deepsleep(hba)
&& !ufshcd_is_link_off(hba)); @@ -8864,8 +8878,6 @@ static intufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)drivers/scsi/ufs/ufshcd.c +2
-2ufshcd_link_state_transition (hba, old_link_state,0 ); vendor_suspend:ufshcd_vops_suspend (hba, pm_op); -disable_vreg: -ufshcd_vreg_set_lpm (hba); disable_irq_and_vops_clks:ufshcd_disable_irq (hba);if (hba->clk_scaling.is_allowed)@@ -8876,6 +8888,8 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) trace_ufshcd_clk_gating(dev_name(hba->dev), hba->clk_gating.state); } +disable_vreg: +ufshcd_vreg_set_lpm (hba); out: hba->pm_op_in_progress =0 ;drivers/scsi/ufs/ufshcd.c +0
-0if (ret)drivers/scsi/ufs/ufshcd.h index renamed from drivers/scsi/ufs/ufshcd.h +0
-09bb5f0e..d5fbaba 100644
drivers/scsi/ufs/ufshcd.h +5
-5@@ -319,7 +319,7 @@ struct ufs_pwr_mode_info {* @resume: called during host controller PM callback * @dbg_register_dump: used to dump controller debug information * @phy_initialization: used to initialize phys -* @device_reset: called to issue a reset pulse on the UFS device +* @device_reset: called to assert or deassert device reset line * @program_key: program or evict an inline encryption key * @event_notify: called to notify important events */ @@ -350,7 +350,7 @@ struct ufs_hba_variant_ops {int (*resume)(struct ufs_hba *,enum ufs_pm_op );void (*dbg_register_dump)(struct ufs_hba *hba);int (*phy_initialization)(struct ufs_hba *); -int (*device_reset)(struct ufs_hba *hba); +int (*device_reset)(struct ufs_hba *hba,bool asserted);void (*config_scaling_param)(struct ufs_hba *hba,struct devfreq_dev_profile *profile,void *data);@@ -1216,10 +1216,10 @@ static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba) hba->vops->dbg_register_dump(hba); } -static inline int ufshcd_vops_device_reset (struct ufs_hba *hba) +static inline int ufshcd_vops_device_reset (struct ufs_hba *hba,bool +asserted) {if (hba->vops && hba->vops->device_reset) -return hba->vops->device_reset (hba); +return hba->vops->device_reset (hba, asserted);return -EOPNOTSUPP ; } --
Quoted 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Quoted 10 lines · Ziqi Chen
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.
Quoted 3 lines
Cc Kiwoong Kim <[email protected]>Cc Stanley Chu <[email protected]>Signed-off-by Ziqi Chen <[email protected]>
Thanks for this fix including ufs-mediatek change as well.
_______________________________________________
Linux-mediatek mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Quoted 278 lines · Ziqi Chen
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.Cc Kiwoong Kim <[email protected]>Cc Stanley Chu <[email protected]>Signed-off-by Ziqi Chen <[email protected]>--- drivers/scsi/ufs/ufs-mediatek.c | 32 ++++++++++++++++---------------- drivers/scsi/ufs/ufs-qcom.c | 24 +++++++++++++++--------- drivers/scsi/ufs/ufshcd.c | 36 +++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 8 ++++---- 4 files changed, 60 insertions(+), 40 deletions(-)drivers/scsi/ufs/ufs-mediatek.c +16
-16@@ -841,27 +841,27 @@ static int ufs_mtk_link_startup_notify(structufs_hba *hba, return ret; } -static int ufs_mtk_device_reset (struct ufs_hba *hba) +static int ufs_mtk_device_reset (struct ufs_hba *hba,bool asserted) {struct arm_smccc_res res; -ufs_mtk_device_reset_ctrl (0 , res); +if (asserted) { +ufs_mtk_device_reset_ctrl (0 , res); -/* -* The reset signal is active low. UFS devices shall detect -* more than or equal to 1us of positive or negative RST_n -* pulse width. -* -* To be on safe side, keep the reset low for at least 10us. -*/ -usleep_range (10 ,15 ); - -ufs_mtk_device_reset_ctrl (1 , res); - -/* Some devices may need time to respond to rst_n */ -usleep_range (10000 ,15000 ); +/* +* The reset signal is active low. UFS devices shall detect +* more than or equal to 1us of positive or negative RST_n +* pulse width. +* +* To be on safe side, keep the reset low for at least 10us. +*/ +usleep_range (10 ,15 ); + }else { +ufs_mtk_device_reset_ctrl (1 , res); -dev_info (hba->dev,"device reset done\n" ); +/* Some devices may need time to respond to rst_n */ +usleep_range (10000 ,15000 ); + }return 0 ;}
drivers/scsi/ufs/ufs-qcom.c +6
-9@@ -1406,10 +1406,11 @@ static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba)/** * ufs_qcom_device_reset() - toggle the (optional) device reset line * @hba: per-adapter instance +* @asserted: assert or deassert device reset line * * Toggles the (optional) reset line to reset the attached device. */ -static int ufs_qcom_device_reset (struct ufs_hba *hba) +static int ufs_qcom_device_reset (struct ufs_hba *hba,bool asserted) {struct ufs_qcom_host *host =ufshcd_get_variant (hba);@@ -1417,15 +1418,20 @@ static int ufs_qcom_device_reset(struct ufs_hba *hba)if (!host->device_reset)return -EOPNOTSUPP ; -/* -* The UFS device shall detect reset pulses of 1us, sleep for 10us to -* be on the safe side. -*/ -gpiod_set_value_cansleep (host->device_reset,1 ); -usleep_range (10 ,15 ); +if (asserted) { +gpiod_set_value_cansleep (host->device_reset,1 ); -gpiod_set_value_cansleep (host->device_reset,0 ); -usleep_range (10 ,15 ); +/* +* The UFS device shall detect reset pulses of 1us, sleep for 10us to
drivers/scsi/ufs/ufs-qcom.c +9
-0+* be on the safe side. +*/ +usleep_range (10 ,15 ); + }else { +gpiod_set_value_cansleep (host->device_reset,0 ); + +/* Some devices may need time to respond to rst_n */ +usleep_range (10 ,15 ); + }return 0 ; }drivers/scsi/ufs/ufshcd.c +7
-1@@ -585,7 +585,13 @@ static void ufshcd_device_reset(struct ufs_hba *hba) {int err; - err =ufshcd_vops_device_reset (hba); + err =ufshcd_vops_device_reset (hba, true); +if (err) { +dev_err (hba->dev,"asserting device reset failed: %d\n" , err); +return ; + } + + err =ufshcd_vops_device_reset (hba, false);if (!err) {drivers/scsi/ufs/ufshcd.c +4
-0ufshcd_set_ufs_dev_active (hba);@@ -593,7 +599,11 @@ static void ufshcd_device_reset(struct ufs_hba *hba) hba->wb_enabled = false; hba->wb_buf_flush_enabled = false; } +dev_dbg (hba->dev,"device reset done\n" ); + }else { +dev_err (hba->dev,"deasserting device reset failed: %d\n" , err); } +if (err != -EOPNOTSUPP )ufshcd_update_evt_hist (hba,UFS_EVT_DEV_RESET , err);}
drivers/scsi/ufs/ufshcd.c +0
-2@@ -8686,8 +8696,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)if (ret) goto set_dev_active; - ufshcd_vreg_set_lpm(hba); - disable_clks:/* * Call vendor specific suspend callback. As these callbacks may
accessdrivers/scsi/ufs/ufshcd.c +3
-0@@ -8703,6 +8711,9 @@ static int ufshcd_suspend(struct ufs_hba *hba,enum ufs_pm_op pm_op) */ ufshcd_disable_irq (hba); +if (ufshcd_is_link_off (hba)) +ufshcd_vops_device_reset (hba, true); +ufshcd_setup_clocks (hba, false);drivers/scsi/ufs/ufshcd.c +2
-0if (ufshcd_is_clkgating_allowed (hba)) {@@ -8711,6 +8722,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) hba->clk_gating.state); } +ufshcd_vreg_set_lpm (hba); +/* Put the host controller in low power mode if possible */ ufshcd_hba_vreg_set_lpm (hba);drivers/scsi/ufs/ufshcd.c +6
-5goto out;@@ -8778,18 +8791,19 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) old_link_state = hba->uic_link_state;ufshcd_hba_vreg_set_hpm (hba); + + ret =ufshcd_vreg_set_hpm (hba); +if (ret) + goto out; +/* Make sure clocks are enabled before accessing controller */ ret =ufshcd_setup_clocks (hba, true);if (ret) - goto out; + goto disable_vreg;/* enable the host irq as host controller would be active soon */ ufshcd_enable_irq (hba); - ret =ufshcd_vreg_set_hpm (hba); -if (ret) - goto disable_irq_and_vops_clks; -/* * Call vendor specific resume callback. As these callbacks may access
drivers/scsi/ufs/ufshcd.c +1
-1* vendor specific host controller register space call them when the @@ -8797,7 +8811,7 @@ static int ufshcd_resume(struct ufs_hba *hba,enum ufs_pm_op pm_op) */ ret =ufshcd_vops_resume (hba, pm_op);if (ret) - goto disable_vreg; + goto disable_irq_and_vops_clks;/* For DeepSleep, the only supported option is to have the link off */
WARN_ON(ufshcd_is_ufs_dev_deepsleep(hba) &&
!ufshcd_is_link_off(hba));drivers/scsi/ufs/ufshcd.c +0
-2@@ -8864,8 +8878,6 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) ufshcd_link_state_transition(hba, old_link_state, 0); vendor_suspend: ufshcd_vops_suspend(hba, pm_op); -disable_vreg: -ufshcd_vreg_set_lpm (hba); disable_irq_and_vops_clks:ufshcd_disable_irq (hba);drivers/scsi/ufs/ufshcd.c +2
-0if (hba->clk_scaling.is_allowed)@@ -8876,6 +8888,8 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) trace_ufshcd_clk_gating(dev_name(hba->dev), hba->clk_gating.state); } +disable_vreg: +ufshcd_vreg_set_lpm (hba); out: hba->pm_op_in_progress =0 ;drivers/scsi/ufs/ufshcd.c +0
-0if (ret)drivers/scsi/ufs/ufshcd.h +4
-4@@ -319,7 +319,7 @@ struct ufs_pwr_mode_info {* @resume: called during host controller PM callback * @dbg_register_dump: used to dump controller debug information * @phy_initialization: used to initialize phys -* @device_reset: called to issue a reset pulse on the UFS device +* @device_reset: called to assert or deassert device reset line * @program_key: program or evict an inline encryption key * @event_notify: called to notify important events */ @@ -350,7 +350,7 @@ struct ufs_hba_variant_ops {int (*resume)(struct ufs_hba *,enum ufs_pm_op );void (*dbg_register_dump)(struct ufs_hba *hba);int (*phy_initialization)(struct ufs_hba *); -int (*device_reset)(struct ufs_hba *hba); +int (*device_reset)(struct ufs_hba *hba,bool asserted);void (*config_scaling_param)(struct ufs_hba *hba,struct devfreq_dev_profile *profile,void *data);@@ -1216,10 +1216,10 @@ static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba) hba->vops->dbg_register_dump(hba); } -static inline int ufshcd_vops_device_reset (struct ufs_hba *hba) +static inline int ufshcd_vops_device_reset (struct ufs_hba *hba,bool asserted) {if (hba->vops && hba->vops->device_reset) -return hba->vops->device_reset (hba); +return hba->vops->device_reset (hba, asserted);
Quoted 2 lines
return
-EOPNOTSUPP;
}
_______________________________________________
Linux-mediatek mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Hi,
Quoted 10 lines
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.
Sorry for my late response.
Please allow few more days to consult internally about this.
Quoted 27 lines
Cc Kiwoong Kim <[email protected]>Cc Stanley Chu <[email protected]>Signed-off-by Ziqi Chen <[email protected]>drivers/scsi/ufs/ufs-qcom.c +3
-9-static int ufs_qcom_device_reset (struct ufs_hba *hba) +static int ufs_qcom_device_reset (struct ufs_hba *hba,bool asserted) {struct ufs_qcom_host *host =ufshcd_get_variant (hba);@@ -1417,15 +1418,20 @@ static int ufs_qcom_device_reset(struct ufs_hba *hba)if (!host->device_reset)return -EOPNOTSUPP ; -/* -* The UFS device shall detect reset pulses of 1us, sleep for 10us to -* be on the safe side. -*/ -gpiod_set_value_cansleep (host->device_reset,1 ); -usleep_range (10 ,15 ); +if (asserted) { +gpiod_set_value_cansleep (host->device_reset,1 ); -gpiod_set_value_cansleep (host->device_reset,0 ); -usleep_range (10 ,15 );
drivers/scsi/ufs/ufs-qcom.c static int ufs_qcom_device_reset(struct ufs_hba · line 1434
Quoted 10 lines
drivers/scsi/ufs/ufs-qcom.c +10
-0+/* +* The UFS device shall detect reset pulses of 1us, sleep for 10us to +* be on the safe side. +*/ +usleep_range (10 ,15 ); + }else { +gpiod_set_value_cansleep (host->device_reset,0 ); + +/* Some devices may need time to respond to rst_n */ +usleep_range (10 ,15 );
Since sleep the same on assert/de-assert can move it outside the if-else clause?
Quoted 4 lines
drivers/scsi/ufs/ufs-mediatek.c +1
-0+ }return 0 ;}
All the below changes, in suspend/resume, deserves some references in your commit log, And probably a separate patch.
Thanks,
Avri
Quoted 124 lines
drivers/scsi/ufs/ufshcd.c +0
-2@@ -8686,8 +8696,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)if (ret) goto set_dev_active; -ufshcd_vreg_set_lpm (hba); - disable_clks:/* * Call vendor specific suspend callback. As these callbacks may access
drivers/scsi/ufs/ufshcd.c +3
-0@@ -8703,6 +8711,9 @@ static int ufshcd_suspend(struct ufs_hba *hba,enum ufs_pm_op pm_op) */ ufshcd_disable_irq (hba); +if (ufshcd_is_link_off (hba)) +ufshcd_vops_device_reset (hba, true); +ufshcd_setup_clocks (hba, false);if (ufshcd_is_clkgating_allowed(hba)) {drivers/scsi/ufs/ufshcd.c +2
-0@@ -8711,6 +8722,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) hba->clk_gating.state); } +ufshcd_vreg_set_lpm (hba); +/* Put the host controller in low power mode if possible */ ufshcd_hba_vreg_set_lpm (hba);goto out;drivers/scsi/ufs/ufshcd.c +6
-5@@ -8778,18 +8791,19 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) old_link_state = hba->uic_link_state;ufshcd_hba_vreg_set_hpm (hba); + + ret =ufshcd_vreg_set_hpm (hba); +if (ret) + goto out; +/* Make sure clocks are enabled before accessing controller */ ret =ufshcd_setup_clocks (hba, true);if (ret) - goto out; + goto disable_vreg;/* enable the host irq as host controller would be active soon */ ufshcd_enable_irq (hba); - ret =ufshcd_vreg_set_hpm (hba); -if (ret) - goto disable_irq_and_vops_clks; -/* * Call vendor specific resume callback. As these callbacks may access * vendor specific host controller register space call them when the
drivers/scsi/ufs/ufshcd.c +1
-1@@ -8797,7 +8811,7 @@ static int ufshcd_resume(struct ufs_hba *hba,enum ufs_pm_op pm_op) */ ret =ufshcd_vops_resume (hba, pm_op);if (ret) - goto disable_vreg; + goto disable_irq_and_vops_clks;/* For DeepSleep, the only supported option is to have the link off */ WARN_ON(ufshcd_is_ufs_dev_deepsleep(hba) &&!ufshcd_is_link_off(hba));
drivers/scsi/ufs/ufshcd.c +0
-2@@ -8864,8 +8878,6 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) ufshcd_link_state_transition(hba, old_link_state, 0); vendor_suspend: ufshcd_vops_suspend(hba, pm_op); -disable_vreg: -ufshcd_vreg_set_lpm (hba); disable_irq_and_vops_clks:ufshcd_disable_irq (hba);if (hba->clk_scaling.is_allowed)drivers/scsi/ufs/ufshcd.c +2
-0@@ -8876,6 +8888,8 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) trace_ufshcd_clk_gating(dev_name(hba->dev), hba->clk_gating.state); } +disable_vreg: +ufshcd_vreg_set_lpm (hba); out: hba->pm_op_in_progress =0 ;if (ret)
drivers/scsi/ufs/ufshcd.h +4
-4@@ -319,7 +319,7 @@ struct ufs_pwr_mode_info {* @resume: called during host controller PM callback * @dbg_register_dump: used to dump controller debug information * @phy_initialization: used to initialize phys -* @device_reset: called to issue a reset pulse on the UFS device +* @device_reset: called to assert or deassert device reset line * @program_key: program or evict an inline encryption key * @event_notify: called to notify important events */ @@ -350,7 +350,7 @@ struct ufs_hba_variant_ops {int (*resume)(struct ufs_hba *,enum ufs_pm_op );void (*dbg_register_dump)(struct ufs_hba *hba);int (*phy_initialization)(struct ufs_hba *); -int (*device_reset)(struct ufs_hba *hba); +int (*device_reset)(struct ufs_hba *hba,bool asserted);void (*config_scaling_param)(struct ufs_hba *hba,struct devfreq_dev_profile *profile,void *data);@@ -1216,10 +1216,10 @@ static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba) hba->vops->dbg_register_dump(hba); } -static inline int ufshcd_vops_device_reset (struct ufs_hba *hba) +static inline int ufshcd_vops_device_reset (struct ufs_hba *hba,bool asserted) {if (hba->vops && hba->vops->device_reset) -return hba->vops->device_reset (hba); +return hba->vops->device_reset (hba, asserted);return -EOPNOTSUPP;
Quoted 5 lines
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Linux-mediatek mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Quoted 41 lines · Avri Altman
Hi,
Quoted 10 lines
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.Sorry for my late response.
Please allow few more days to consult internally about this.Quoted 27 lines
Cc Kiwoong Kim <[email protected]>Cc Stanley Chu <[email protected]>Signed-off-by Ziqi Chen <[email protected]>drivers/scsi/ufs/ufs-qcom.c +1
-1-static int ufs_qcom_device_reset (struct ufs_hba *hba) +static int ufs_qcom_device_reset (struct ufs_hba *hba,bool asserted) {struct ufs_qcom_host *host =ufshcd_get_variant (hba);@@ -1417,15 +1418,20 @@ static int ufs_qcom_device_reset(struct
ufs_hba
*hba)if (!host->device_reset) return -EOPNOTSUPP;(quoted diff) +0
-2- /* - * The UFS device shall detect reset pulses of 1us, sleep for10us to
drivers/scsi/ufs/ufs-qcom.c +2
-4-* be on the safe side. -*/ -gpiod_set_value_cansleep (host->device_reset,1 ); -usleep_range (10 ,15 ); +if (asserted) { +gpiod_set_value_cansleep (host->device_reset,1 );
Quoted 15 lines
drivers/scsi/ufs/ufs-qcom.c static int ufs_qcom_device_reset(struct ufs_hba *hba) · line 1433
Quoted 13 lines
drivers/scsi/ufs/ufs-qcom.c +2
-2-gpiod_set_value_cansleep (host->device_reset,0 ); -usleep_range (10 ,15 ); +/* +* The UFS device shall detect reset pulses of 1us, sleep for 10us to
drivers/scsi/ufs/ufs-qcom.c +8
-0+* be on the safe side. +*/ +usleep_range (10 ,15 ); + }else { +gpiod_set_value_cansleep (host->device_reset,0 ); + +/* Some devices may need time to respond to rst_n */ +usleep_range (10 ,15 );Since sleep the same on assert/de-assert can move it outside the if-else clause?
Hi Avri,
Even though there is same delay on assert/de-assert, they have different purposes. The delay on assert is for JEDEC requirement while the delay on de-assert is for some devices requirement. The latter is just a empirical value and is still controversial among SoC vendors. This value may be changed in the further. So I don't think we should move it outside the if-else clause.
Quoted 9 lines
Quoted 5 lines
+ }
return 0;}
All the below changes, in suspend/resume, deserves some references in your commit log,
And probably a separate patch.
Below changes adjusted the sequence of CLK, Rst_n, VCC and VCCQ/VCCQ2, These requirements are referred in JEDEC and already quoted in my commit log. We don't need more descriptions.
Below changes have also modified common callback interface, we'd better keep implementation and caller changes in one patch.
Best Regards,
Ziqi Chen
Quoted 133 lines
Thanks,
AvriQuoted 129 lines
drivers/scsi/ufs/ufshcd.c +0
-2@@ -8686,8 +8696,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)if (ret) goto set_dev_active; -ufshcd_vreg_set_lpm (hba); - disable_clks:/* * Call vendor specific suspend callback. As these callbacks
may accessdrivers/scsi/ufs/ufshcd.c +3
-0@@ -8703,6 +8711,9 @@ static int ufshcd_suspend(struct ufs_hba *hba,enum ufs_pm_op pm_op) */ ufshcd_disable_irq (hba); +if (ufshcd_is_link_off (hba)) +ufshcd_vops_device_reset (hba, true); +ufshcd_setup_clocks (hba, false);if (ufshcd_is_clkgating_allowed(hba)) {drivers/scsi/ufs/ufshcd.c +2
-0@@ -8711,6 +8722,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) hba->clk_gating.state); } +ufshcd_vreg_set_lpm (hba); +/* Put the host controller in low power mode if possible */ ufshcd_hba_vreg_set_lpm (hba);goto out;drivers/scsi/ufs/ufshcd.c +6
-5@@ -8778,18 +8791,19 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) old_link_state = hba->uic_link_state;ufshcd_hba_vreg_set_hpm (hba); + + ret =ufshcd_vreg_set_hpm (hba); +if (ret) + goto out; +/* Make sure clocks are enabled before accessing controller */ ret =ufshcd_setup_clocks (hba, true);if (ret) - goto out; + goto disable_vreg;/* enable the host irq as host controller would be active soon */ ufshcd_enable_irq (hba); - ret =ufshcd_vreg_set_hpm (hba); -if (ret) - goto disable_irq_and_vops_clks; -/* * Call vendor specific resume callback. As these callbacks
may access
* vendor specific host controller register space call them
when thedrivers/scsi/ufs/ufshcd.c +1
-1@@ -8797,7 +8811,7 @@ static int ufshcd_resume(struct ufs_hba *hba,enum ufs_pm_op pm_op) */ ret =ufshcd_vops_resume (hba, pm_op);if (ret) - goto disable_vreg; + goto disable_irq_and_vops_clks;/* For DeepSleep, the only supported option is to have the link off */
WARN_ON(ufshcd_is_ufs_dev_deepsleep(hba) &&!ufshcd_is_link_off(hba));
drivers/scsi/ufs/ufshcd.c +0
-2@@ -8864,8 +8878,6 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) ufshcd_link_state_transition(hba, old_link_state, 0); vendor_suspend: ufshcd_vops_suspend(hba, pm_op); -disable_vreg: -ufshcd_vreg_set_lpm (hba); disable_irq_and_vops_clks:ufshcd_disable_irq (hba);if (hba->clk_scaling.is_allowed)drivers/scsi/ufs/ufshcd.c +2
-0@@ -8876,6 +8888,8 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) trace_ufshcd_clk_gating(dev_name(hba->dev), hba->clk_gating.state); } +disable_vreg: +ufshcd_vreg_set_lpm (hba); out: hba->pm_op_in_progress =0 ;if (ret)
drivers/scsi/ufs/ufshcd.h +2
-2@@ -319,7 +319,7 @@ struct ufs_pwr_mode_info {* @resume: called during host controller PM callback * @dbg_register_dump: used to dump controller debug information * @phy_initialization: used to initialize phys -* @device_reset: called to issue a reset pulse on the UFS device +* @device_reset: called to assert or deassert device reset line * @program_key: program or evict an inline encryption key * @event_notify: called to notify important events */ @@ -350,7 +350,7 @@ struct ufs_hba_variant_ops {int (*resume)(struct ufs_hba *,enum ufs_pm_op );void (*dbg_register_dump)(struct ufs_hba *hba);int (*phy_initialization)(struct ufs_hba *); -int (*device_reset)(struct ufs_hba *hba); +int (*device_reset)(struct ufs_hba *hba,bool asserted); void (*config_scaling_param)(struct ufs_hba *hba, struct devfreq_dev_profile *profile,void *data);drivers/scsi/ufs/ufshcd.h +2
-2@@ -1216,10 +1216,10 @@ static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba) hba->vops->dbg_register_dump(hba); } -static inline int ufshcd_vops_device_reset (struct ufs_hba *hba) +static inline int ufshcd_vops_device_reset (struct ufs_hba *hba,bool asserted) {if (hba->vops && hba->vops->device_reset) -return hba->vops->device_reset (hba); +return hba->vops->device_reset (hba, asserted);
Quoted 6 lines
Quoted 6 lines
return -EOPNOTSUPP;}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Linux-mediatek mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Quoted 48 lines · Ziqi Chen
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.Cc Kiwoong Kim <[email protected]>Cc Stanley Chu <[email protected]>Signed-off-by Ziqi Chen <[email protected]>--- drivers/scsi/ufs/ufs-mediatek.c | 32 ++++++++++++++++---------------- drivers/scsi/ufs/ufs-qcom.c | 24 +++++++++++++++--------- drivers/scsi/ufs/ufshcd.c | 36 +++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 8 ++++---- 4 files changed, 60 insertions(+), 40 deletions(-)drivers/scsi/ufs/ufs-mediatek.c +3
-13@@ -841,27 +841,27 @@ static int ufs_mtk_link_startup_notify(struct ufs_hba *hba,return ret; } -static int ufs_mtk_device_reset (struct ufs_hba *hba) +static int ufs_mtk_device_reset (struct ufs_hba *hba,bool asserted) {struct arm_smccc_res res; -ufs_mtk_device_reset_ctrl (0 , res); +if (asserted) { +ufs_mtk_device_reset_ctrl (0 , res); -/* -* The reset signal is active low. UFS devices shall detect -* more than or equal to 1us of positive or negative RST_n -* pulse width. -* -* To be on safe side, keep the reset low for at least 10us. -*/ -usleep_range (10 ,15 ); - -ufs_mtk_device_reset_ctrl (1 , res); -
drivers/scsi/ufs/ufs-mediatek.c static int ufs_mtk_link_startup_notify(struct ufs_hba *hba, · line 858
Quoted 10 lines
drivers/scsi/ufs/ufs-mediatek.c +8
-2-/* Some devices may need time to respond to rst_n */ -usleep_range (10000 ,15000 ); +/* +* The reset signal is active low. UFS devices shall detect +* more than or equal to 1us of positive or negative RST_n +* pulse width. +* +* To be on safe side, keep the reset low for at least 10us. +*/ +usleep_range (10 ,15 );
I see no point in allowing vendors to "tweak" the 1us->10us adjustment. The specification says 1us and we all agree that 10us gives us good enough slack. I.e. this is common code.
drivers/scsi/ufs/ufs-mediatek.c static int ufs_mtk_link_startup_notify(struct ufs_hba *hba, · line 863
Quoted 6 lines
drivers/scsi/ufs/ufs-mediatek.c +4
-1+ }else { +ufs_mtk_device_reset_ctrl (1 , res); -dev_info (hba->dev,"device reset done\n" ); +/* Some devices may need time to respond to rst_n */ +usleep_range (10000 ,15000 );
The comment in both the Qualcomm and Mediatek drivers claim that this is sleep relates to the UFS device (not host), so why should it be different?
What happens if I take the device that Mediatek see a need for a 10ms delay and hook that up to a Qualcomm host? This really should go in the common code.
As such I really would prefer to see these delays in the common code! You really shouldn't write code based on a speculation that one day there might come someone who need other values - when that day come we can just change the code, and if it never comes we're better off with the cleaner implementation.
Regards,
Bjorn
Quoted 200 lines
drivers/scsi/ufs/ufs-mediatek.c +1
-0+ }return 0 ; }drivers/scsi/ufs/ufs-qcom.c +15
-9@@ -1406,10 +1406,11 @@ static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba)/** * ufs_qcom_device_reset() - toggle the (optional) device reset line * @hba: per-adapter instance +* @asserted: assert or deassert device reset line * * Toggles the (optional) reset line to reset the attached device. */ -static int ufs_qcom_device_reset (struct ufs_hba *hba) +static int ufs_qcom_device_reset (struct ufs_hba *hba,bool asserted) {struct ufs_qcom_host *host =ufshcd_get_variant (hba);@@ -1417,15 +1418,20 @@ static int ufs_qcom_device_reset(struct ufs_hba *hba)if (!host->device_reset)return -EOPNOTSUPP ; -/* -* The UFS device shall detect reset pulses of 1us, sleep for 10us to -* be on the safe side. -*/ -gpiod_set_value_cansleep (host->device_reset,1 ); -usleep_range (10 ,15 ); +if (asserted) { +gpiod_set_value_cansleep (host->device_reset,1 ); -gpiod_set_value_cansleep (host->device_reset,0 ); -usleep_range (10 ,15 ); +/* +* The UFS device shall detect reset pulses of 1us, sleep for 10us to +* be on the safe side. +*/ +usleep_range (10 ,15 ); + }else { +gpiod_set_value_cansleep (host->device_reset,0 ); + +/* Some devices may need time to respond to rst_n */ +usleep_range (10 ,15 ); + }return 0 ; }drivers/scsi/ufs/ufshcd.c +25
-11@@ -585,7 +585,13 @@ static void ufshcd_device_reset(struct ufs_hba *hba) {int err; - err =ufshcd_vops_device_reset (hba); + err =ufshcd_vops_device_reset (hba, true); +if (err) { +dev_err (hba->dev,"asserting device reset failed: %d\n" , err); +return ; + } + + err =ufshcd_vops_device_reset (hba, false);if (!err) {ufshcd_set_ufs_dev_active (hba);@@ -593,7 +599,11 @@ static void ufshcd_device_reset(struct ufs_hba *hba) hba->wb_enabled = false; hba->wb_buf_flush_enabled = false; } +dev_dbg (hba->dev,"device reset done\n" ); + }else { +dev_err (hba->dev,"deasserting device reset failed: %d\n" , err); } +if (err != -EOPNOTSUPP )ufshcd_update_evt_hist (hba,UFS_EVT_DEV_RESET , err); }@@ -8686,8 +8696,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)if (ret) goto set_dev_active; -ufshcd_vreg_set_lpm (hba); - disable_clks:/* * Call vendor specific suspend callback. As these callbacks may access @@ -8703,6 +8711,9 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)*/ ufshcd_disable_irq (hba); +if (ufshcd_is_link_off (hba)) +ufshcd_vops_device_reset (hba, true); +ufshcd_setup_clocks (hba, false);if (ufshcd_is_clkgating_allowed (hba)) {@@ -8711,6 +8722,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) hba->clk_gating.state); } +ufshcd_vreg_set_lpm (hba); +/* Put the host controller in low power mode if possible */ ufshcd_hba_vreg_set_lpm (hba); goto out;@@ -8778,18 +8791,19 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) old_link_state = hba->uic_link_state;ufshcd_hba_vreg_set_hpm (hba); + + ret =ufshcd_vreg_set_hpm (hba); +if (ret) + goto out; +/* Make sure clocks are enabled before accessing controller */ ret =ufshcd_setup_clocks (hba, true);if (ret) - goto out; + goto disable_vreg;/* enable the host irq as host controller would be active soon */ ufshcd_enable_irq (hba); - ret =ufshcd_vreg_set_hpm (hba); -if (ret) - goto disable_irq_and_vops_clks; -/* * Call vendor specific resume callback. As these callbacks may access * vendor specific host controller register space call them when the @@ -8797,7 +8811,7 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)*/ ret =ufshcd_vops_resume (hba, pm_op);if (ret) - goto disable_vreg; + goto disable_irq_and_vops_clks;/* For DeepSleep, the only supported option is to have the link off */ WARN_ON (ufshcd_is_ufs_dev_deepsleep (hba) && !ufshcd_is_link_off (hba));@@ -8864,8 +8878,6 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)ufshcd_link_state_transition (hba, old_link_state,0 ); vendor_suspend:ufshcd_vops_suspend (hba, pm_op); -disable_vreg: -ufshcd_vreg_set_lpm (hba); disable_irq_and_vops_clks:ufshcd_disable_irq (hba);if (hba->clk_scaling.is_allowed)@@ -8876,6 +8888,8 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)trace_ufshcd_clk_gating (dev_name (hba->dev), hba->clk_gating.state); } +disable_vreg: +ufshcd_vreg_set_lpm (hba); out: hba->pm_op_in_progress =0 ;if (ret)drivers/scsi/ufs/ufshcd.h +4
-4@@ -319,7 +319,7 @@ struct ufs_pwr_mode_info {* @resume: called during host controller PM callback * @dbg_register_dump: used to dump controller debug information * @phy_initialization: used to initialize phys -* @device_reset: called to issue a reset pulse on the UFS device +* @device_reset: called to assert or deassert device reset line * @program_key: program or evict an inline encryption key * @event_notify: called to notify important events */ @@ -350,7 +350,7 @@ struct ufs_hba_variant_ops {int (*resume)(struct ufs_hba *,enum ufs_pm_op );void (*dbg_register_dump)(struct ufs_hba *hba);int (*phy_initialization)(struct ufs_hba *); -int (*device_reset)(struct ufs_hba *hba); +int (*device_reset)(struct ufs_hba *hba,bool asserted);void (*config_scaling_param)(struct ufs_hba *hba,struct devfreq_dev_profile *profile,void *data);@@ -1216,10 +1216,10 @@ static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba) hba->vops->dbg_register_dump (hba); } -static inline int ufshcd_vops_device_reset (struct ufs_hba *hba) +static inline int ufshcd_vops_device_reset (struct ufs_hba *hba,bool asserted) {if (hba->vops && hba->vops->device_reset) -return hba->vops->device_reset (hba); +return hba->vops->device_reset (hba, asserted);return -EOPNOTSUPP ; }Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
_______________________________________________
Linux-mediatek mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Quoted 58 lines · Bjorn Andersson
Quoted 56 lines · Ziqi Chen
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.Cc Kiwoong Kim <[email protected]>Cc Stanley Chu <[email protected]>Signed-off-by Ziqi Chen <[email protected]>--- drivers/scsi/ufs/ufs-mediatek.c | 32 ++++++++++++++++---------------- drivers/scsi/ufs/ufs-qcom.c | 24 +++++++++++++++--------- drivers/scsi/ufs/ufshcd.c | 36 +++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 8 ++++---- 4 files changed, 60 insertions(+), 40 deletions(-)drivers/scsi/ufs/ufs-mediatek.c +7
-15@@ -841,27 +841,27 @@ static int ufs_mtk_link_startup_notify(structufs_hba *hba, return ret; } -static int ufs_mtk_device_reset (struct ufs_hba *hba) +static int ufs_mtk_device_reset (struct ufs_hba *hba,bool asserted) {struct arm_smccc_res res; -ufs_mtk_device_reset_ctrl (0 , res); +if (asserted) { +ufs_mtk_device_reset_ctrl (0 , res); -/* -* The reset signal is active low. UFS devices shall detect -* more than or equal to 1us of positive or negative RST_n -* pulse width. -* -* To be on safe side, keep the reset low for at least 10us. -*/ -usleep_range (10 ,15 ); - -ufs_mtk_device_reset_ctrl (1 , res); - -/* Some devices may need time to respond to rst_n */ -usleep_range (10000 ,15000 ); +/* +* The reset signal is active low. UFS devices shall detect +* more than or equal to 1us of positive or negative RST_n +* pulse width.
Quoted 8 lines
drivers/scsi/ufs/ufs-mediatek.c static int ufs_mtk_link_startup_notify(struct ufs_hba *hba, · line 858
Quoted 4 lines
drivers/scsi/ufs/ufs-mediatek.c +4
-0+* +* To be on safe side, keep the reset low for at least 10us. +*/ +usleep_range (10 ,15 );I see no point in allowing vendors to "tweak" the 1us->10us adjustment. The specification says 1us and we all agree that 10us gives us good enough slack. I.e. this is common code.
Hi Bjron,
We tried, but Samsung fellows wanted 5us. We couldn't get a agreement on this delay in short term, so we chose to leave it in vops.
Quoted 12 lines
drivers/scsi/ufs/ufs-mediatek.c static int ufs_mtk_link_startup_notify(struct ufs_hba *hba, · line 863
Quoted 7 lines
drivers/scsi/ufs/ufs-mediatek.c +4
-1+ }else { +ufs_mtk_device_reset_ctrl (1 , res); -dev_info (hba->dev,"device reset done\n" ); +/* Some devices may need time to respond to rst_n */ +usleep_range (10000 ,15000 );The comment in both the Qualcomm and Mediatek drivers claim that this is sleep relates to the UFS device (not host), so why should it be different?
Quoted 4 lines
What happens if I take the device that Mediatek see a need for a 10ms delay and hook that up to a Qualcomm host? This really should go in the common code.
Agree, but Qualcomm host didn't have any problems with 10us yet, so if we put the 10ms delay to common code, Qualcomm host would suffer longer delay when device reset happens - both bootup and resume(xpm_lvl = 5/6) latency would be increased.
Regards,
Can Guo.
Quoted 224 lines
As such I really would prefer to see these delays in the common code! You really shouldn't write code based on a speculation that one day there might come someone who need other values - when that day come we can just change the code, and if it never comes we're better off with the cleaner implementation.
Regards,
BjornQuoted 213 lines
drivers/scsi/ufs/ufs-mediatek.c +1
-0+ }return 0 ;}
drivers/scsi/ufs/ufs-qcom.c +2
-3@@ -1406,10 +1406,11 @@ static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba)/** * ufs_qcom_device_reset() - toggle the (optional) device reset line * @hba: per-adapter instance +* @asserted: assert or deassert device reset line * * Toggles the (optional) reset line to reset the attached device. */ -static int ufs_qcom_device_reset (struct ufs_hba *hba) +static int ufs_qcom_device_reset (struct ufs_hba *hba,bool asserted) {struct ufs_qcom_host *host =ufshcd_get_variant (hba);@@ -1417,15 +1418,20 @@ static int ufs_qcom_device_reset(struct ufs_hba *hba)if (!host->device_reset)return -EOPNOTSUPP ; -/* -* The UFS device shall detect reset pulses of 1us, sleep for 10us to
drivers/scsi/ufs/ufs-qcom.c +4
-6-* be on the safe side. -*/ -gpiod_set_value_cansleep (host->device_reset,1 ); -usleep_range (10 ,15 ); +if (asserted) { +gpiod_set_value_cansleep (host->device_reset,1 ); -gpiod_set_value_cansleep (host->device_reset,0 ); -usleep_range (10 ,15 ); +/* +* The UFS device shall detect reset pulses of 1us, sleep for 10us to
drivers/scsi/ufs/ufs-qcom.c +9
-0+* be on the safe side. +*/ +usleep_range (10 ,15 ); + }else { +gpiod_set_value_cansleep (host->device_reset,0 ); + +/* Some devices may need time to respond to rst_n */ +usleep_range (10 ,15 ); + }return 0 ; }drivers/scsi/ufs/ufshcd.c +7
-1@@ -585,7 +585,13 @@ static void ufshcd_device_reset(struct ufs_hba *hba) {int err; - err =ufshcd_vops_device_reset (hba); + err =ufshcd_vops_device_reset (hba, true); +if (err) { +dev_err (hba->dev,"asserting device reset failed: %d\n" , err); +return ; + } + + err =ufshcd_vops_device_reset (hba, false);if (!err) {drivers/scsi/ufs/ufshcd.c +4
-0ufshcd_set_ufs_dev_active (hba);@@ -593,7 +599,11 @@ static void ufshcd_device_reset(struct ufs_hba *hba) hba->wb_enabled = false; hba->wb_buf_flush_enabled = false; } +dev_dbg (hba->dev,"device reset done\n" ); + }else { +dev_err (hba->dev,"deasserting device reset failed: %d\n" , err); } +if (err != -EOPNOTSUPP )ufshcd_update_evt_hist (hba,UFS_EVT_DEV_RESET , err);}
drivers/scsi/ufs/ufshcd.c +0
-2@@ -8686,8 +8696,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)if (ret) goto set_dev_active; - ufshcd_vreg_set_lpm(hba); - disable_clks:/* * Call vendor specific suspend callback. As these callbacks may
accessdrivers/scsi/ufs/ufshcd.c +3
-0@@ -8703,6 +8711,9 @@ static int ufshcd_suspend(struct ufs_hba *hba,enum ufs_pm_op pm_op) */ ufshcd_disable_irq (hba); +if (ufshcd_is_link_off (hba)) +ufshcd_vops_device_reset (hba, true); +ufshcd_setup_clocks (hba, false);drivers/scsi/ufs/ufshcd.c +2
-0if (ufshcd_is_clkgating_allowed (hba)) {@@ -8711,6 +8722,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) hba->clk_gating.state); } +ufshcd_vreg_set_lpm (hba); +/* Put the host controller in low power mode if possible */ ufshcd_hba_vreg_set_lpm (hba);drivers/scsi/ufs/ufshcd.c +6
-5goto out;@@ -8778,18 +8791,19 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) old_link_state = hba->uic_link_state;ufshcd_hba_vreg_set_hpm (hba); + + ret =ufshcd_vreg_set_hpm (hba); +if (ret) + goto out; +/* Make sure clocks are enabled before accessing controller */ ret =ufshcd_setup_clocks (hba, true);if (ret) - goto out; + goto disable_vreg;/* enable the host irq as host controller would be active soon */ ufshcd_enable_irq (hba); - ret =ufshcd_vreg_set_hpm (hba); -if (ret) - goto disable_irq_and_vops_clks; -/* * Call vendor specific resume callback. As these callbacks may access
drivers/scsi/ufs/ufshcd.c +1
-1* vendor specific host controller register space call them when the @@ -8797,7 +8811,7 @@ static int ufshcd_resume(struct ufs_hba *hba,enum ufs_pm_op pm_op) */ ret =ufshcd_vops_resume (hba, pm_op);if (ret) - goto disable_vreg; + goto disable_irq_and_vops_clks;/* For DeepSleep, the only supported option is to have the link off */
WARN_ON(ufshcd_is_ufs_dev_deepsleep(hba) &&
!ufshcd_is_link_off(hba));drivers/scsi/ufs/ufshcd.c +0
-2@@ -8864,8 +8878,6 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) ufshcd_link_state_transition(hba, old_link_state, 0); vendor_suspend: ufshcd_vops_suspend(hba, pm_op); -disable_vreg: -ufshcd_vreg_set_lpm (hba); disable_irq_and_vops_clks:ufshcd_disable_irq (hba);drivers/scsi/ufs/ufshcd.c +2
-0if (hba->clk_scaling.is_allowed)@@ -8876,6 +8888,8 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) trace_ufshcd_clk_gating(dev_name(hba->dev), hba->clk_gating.state); } +disable_vreg: +ufshcd_vreg_set_lpm (hba); out: hba->pm_op_in_progress =0 ;drivers/scsi/ufs/ufshcd.c +0
-0if (ret)drivers/scsi/ufs/ufshcd.h +4
-4@@ -319,7 +319,7 @@ struct ufs_pwr_mode_info {* @resume: called during host controller PM callback * @dbg_register_dump: used to dump controller debug information * @phy_initialization: used to initialize phys -* @device_reset: called to issue a reset pulse on the UFS device +* @device_reset: called to assert or deassert device reset line * @program_key: program or evict an inline encryption key * @event_notify: called to notify important events */ @@ -350,7 +350,7 @@ struct ufs_hba_variant_ops {int (*resume)(struct ufs_hba *,enum ufs_pm_op );void (*dbg_register_dump)(struct ufs_hba *hba);int (*phy_initialization)(struct ufs_hba *); -int (*device_reset)(struct ufs_hba *hba); +int (*device_reset)(struct ufs_hba *hba,bool asserted);void (*config_scaling_param)(struct ufs_hba *hba,struct devfreq_dev_profile *profile,void *data);@@ -1216,10 +1216,10 @@ static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba) hba->vops->dbg_register_dump(hba); } -static inline int ufshcd_vops_device_reset (struct ufs_hba *hba) +static inline int ufshcd_vops_device_reset (struct ufs_hba *hba,bool asserted) {if (hba->vops && hba->vops->device_reset) -return hba->vops->device_reset (hba); +return hba->vops->device_reset (hba, asserted);
Quoted 6 lines
Quoted 6 lines
return
-EOPNOTSUPP;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
_______________________________________________
Linux-mediatek mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Quoted 91 lines · Can Guo
Quoted 59 lines · Bjorn Andersson
Quoted 57 lines · Ziqi Chen
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.Cc Kiwoong Kim <[email protected]>Cc Stanley Chu <[email protected]>Signed-off-by Ziqi Chen <[email protected]>--- drivers/scsi/ufs/ufs-mediatek.c | 32 ++++++++++++++++---------------- drivers/scsi/ufs/ufs-qcom.c | 24 +++++++++++++++--------- drivers/scsi/ufs/ufshcd.c | 36 +++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 8 ++++---- 4 files changed, 60 insertions(+), 40 deletions(-)drivers/scsi/ufs/ufs-mediatek.c +7
-15@@ -841,27 +841,27 @@ static int ufs_mtk_link_startup_notify(structufs_hba *hba, return ret; } -static int ufs_mtk_device_reset (struct ufs_hba *hba) +static int ufs_mtk_device_reset (struct ufs_hba *hba,bool asserted) {struct arm_smccc_res res; -ufs_mtk_device_reset_ctrl (0 , res); +if (asserted) { +ufs_mtk_device_reset_ctrl (0 , res); -/* -* The reset signal is active low. UFS devices shall detect -* more than or equal to 1us of positive or negative RST_n -* pulse width. -* -* To be on safe side, keep the reset low for at least 10us. -*/ -usleep_range (10 ,15 ); - -ufs_mtk_device_reset_ctrl (1 , res); - -/* Some devices may need time to respond to rst_n */ -usleep_range (10000 ,15000 ); +/* +* The reset signal is active low. UFS devices shall detect +* more than or equal to 1us of positive or negative RST_n +* pulse width. Quoted 8 lines
drivers/scsi/ufs/ufs-mediatek.c static int ufs_mtk_link_startup_notify(struct ufs_hba *hba, · line 858
Quoted 4 lines
drivers/scsi/ufs/ufs-mediatek.c +4
-0+* +* To be on safe side, keep the reset low for at least 10us. +*/ +usleep_range (10 ,15 );I see no point in allowing vendors to "tweak" the 1us->10us adjustment. The specification says 1us and we all agree that 10us gives us good enough slack. I.e. this is common code.
Hi Bjron,
We tried, but Samsung fellows wanted 5us. We couldn't get a agreement on this delay in short term, so we chose to leave it in vops.
Quoted 12 lines
drivers/scsi/ufs/ufs-mediatek.c static int ufs_mtk_link_startup_notify(struct ufs_hba *hba, · line 863
Quoted 7 lines
drivers/scsi/ufs/ufs-mediatek.c +4
-1+ }else { +ufs_mtk_device_reset_ctrl (1 , res); -dev_info (hba->dev,"device reset done\n" ); +/* Some devices may need time to respond to rst_n */ +usleep_range (10000 ,15000 );The comment in both the Qualcomm and Mediatek drivers claim that this is sleep relates to the UFS device (not host), so why should it be different?
Quoted 4 lines
What happens if I take the device that Mediatek see a need for a 10ms delay and hook that up to a Qualcomm host? This really should go in the common code.
Quoted 8 lines
Agree, but Qualcomm host didn't have any problems with 10us yet, so if we put the 10ms delay to common code, Qualcomm host would suffer longer delay when device reset happens - both bootup and
resume(xpm_lvl = 5/6)latency would be increased.Regards,
Can Guo.
Besides, currently this device reset vops is only registered by ufs-qcom.c and ufs-mediatek.c, meaning any delays that we put in the common code are not necessary for those who do not have this vops registered, i.e ufs-exynos.c, ufs-hisi.c.
Regards,
Can Guo.
Quoted 225 lines
Quoted 225 lines
As such I really would prefer to see these delays in the common code! You really shouldn't write code based on a speculation that one day there might come someone who need other values - when that day come we can just change the code, and if it never comes we're better off with the cleaner implementation.
Regards,
BjornQuoted 214 lines
drivers/scsi/ufs/ufs-mediatek.c +1
-0+ }return 0 ;}
drivers/scsi/ufs/ufs-qcom.c +2
-3@@ -1406,10 +1406,11 @@ static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba)/** * ufs_qcom_device_reset() - toggle the (optional) device reset line * @hba: per-adapter instance +* @asserted: assert or deassert device reset line * * Toggles the (optional) reset line to reset the attached device. */ -static int ufs_qcom_device_reset (struct ufs_hba *hba) +static int ufs_qcom_device_reset (struct ufs_hba *hba,bool asserted) {struct ufs_qcom_host *host =ufshcd_get_variant (hba);@@ -1417,15 +1418,20 @@ static int ufs_qcom_device_reset(struct ufs_hba *hba)if (!host->device_reset)return -EOPNOTSUPP ; -/* -* The UFS device shall detect reset pulses of 1us, sleep for 10us to
drivers/scsi/ufs/ufs-qcom.c +4
-6-* be on the safe side. -*/ -gpiod_set_value_cansleep (host->device_reset,1 ); -usleep_range (10 ,15 ); +if (asserted) { +gpiod_set_value_cansleep (host->device_reset,1 ); -gpiod_set_value_cansleep (host->device_reset,0 ); -usleep_range (10 ,15 ); +/* +* The UFS device shall detect reset pulses of 1us, sleep for 10us to
drivers/scsi/ufs/ufs-qcom.c +9
-0+* be on the safe side. +*/ +usleep_range (10 ,15 ); + }else { +gpiod_set_value_cansleep (host->device_reset,0 ); + +/* Some devices may need time to respond to rst_n */ +usleep_range (10 ,15 ); + }return 0 ; }drivers/scsi/ufs/ufshcd.c +7
-1@@ -585,7 +585,13 @@ static void ufshcd_device_reset(struct ufs_hba *hba) {int err; - err =ufshcd_vops_device_reset (hba); + err =ufshcd_vops_device_reset (hba, true); +if (err) { +dev_err (hba->dev,"asserting device reset failed: %d\n" , err); +return ; + } + + err =ufshcd_vops_device_reset (hba, false);if (!err) {drivers/scsi/ufs/ufshcd.c +4
-0ufshcd_set_ufs_dev_active (hba);@@ -593,7 +599,11 @@ static void ufshcd_device_reset(struct ufs_hba *hba) hba->wb_enabled = false; hba->wb_buf_flush_enabled = false; } +dev_dbg (hba->dev,"device reset done\n" ); + }else { +dev_err (hba->dev,"deasserting device reset failed: %d\n" , err); } +if (err != -EOPNOTSUPP )ufshcd_update_evt_hist (hba,UFS_EVT_DEV_RESET , err);}
drivers/scsi/ufs/ufshcd.c +0
-2@@ -8686,8 +8696,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)if (ret) goto set_dev_active; - ufshcd_vreg_set_lpm(hba); - disable_clks:/* * Call vendor specific suspend callback. As these callbacks may
accessdrivers/scsi/ufs/ufshcd.c +3
-0@@ -8703,6 +8711,9 @@ static int ufshcd_suspend(struct ufs_hba *hba,enum ufs_pm_op pm_op) */ ufshcd_disable_irq (hba); +if (ufshcd_is_link_off (hba)) +ufshcd_vops_device_reset (hba, true); +ufshcd_setup_clocks (hba, false);drivers/scsi/ufs/ufshcd.c +2
-0if (ufshcd_is_clkgating_allowed (hba)) {@@ -8711,6 +8722,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) hba->clk_gating.state); } +ufshcd_vreg_set_lpm (hba); +/* Put the host controller in low power mode if possible */ ufshcd_hba_vreg_set_lpm (hba);drivers/scsi/ufs/ufshcd.c +6
-5goto out;@@ -8778,18 +8791,19 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) old_link_state = hba->uic_link_state;ufshcd_hba_vreg_set_hpm (hba); + + ret =ufshcd_vreg_set_hpm (hba); +if (ret) + goto out; +/* Make sure clocks are enabled before accessing controller */ ret =ufshcd_setup_clocks (hba, true);if (ret) - goto out; + goto disable_vreg;/* enable the host irq as host controller would be active soon */ ufshcd_enable_irq (hba); - ret =ufshcd_vreg_set_hpm (hba); -if (ret) - goto disable_irq_and_vops_clks; -/* * Call vendor specific resume callback. As these callbacks may access
* vendor specific host controller register space call them when
thedrivers/scsi/ufs/ufshcd.c +1
-1@@ -8797,7 +8811,7 @@ static int ufshcd_resume(struct ufs_hba *hba,enum ufs_pm_op pm_op) */ ret =ufshcd_vops_resume (hba, pm_op);if (ret) - goto disable_vreg; + goto disable_irq_and_vops_clks;/* For DeepSleep, the only supported option is to have the link off */
WARN_ON(ufshcd_is_ufs_dev_deepsleep(hba) &&
!ufshcd_is_link_off(hba));drivers/scsi/ufs/ufshcd.c +0
-2@@ -8864,8 +8878,6 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) ufshcd_link_state_transition(hba, old_link_state, 0); vendor_suspend: ufshcd_vops_suspend(hba, pm_op); -disable_vreg: -ufshcd_vreg_set_lpm (hba); disable_irq_and_vops_clks:ufshcd_disable_irq (hba);drivers/scsi/ufs/ufshcd.c +2
-0if (hba->clk_scaling.is_allowed)@@ -8876,6 +8888,8 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) trace_ufshcd_clk_gating(dev_name(hba->dev), hba->clk_gating.state); } +disable_vreg: +ufshcd_vreg_set_lpm (hba); out: hba->pm_op_in_progress =0 ;drivers/scsi/ufs/ufshcd.c +0
-0if (ret)drivers/scsi/ufs/ufshcd.h +4
-4@@ -319,7 +319,7 @@ struct ufs_pwr_mode_info {* @resume: called during host controller PM callback * @dbg_register_dump: used to dump controller debug information * @phy_initialization: used to initialize phys -* @device_reset: called to issue a reset pulse on the UFS device +* @device_reset: called to assert or deassert device reset line * @program_key: program or evict an inline encryption key * @event_notify: called to notify important events */ @@ -350,7 +350,7 @@ struct ufs_hba_variant_ops {int (*resume)(struct ufs_hba *,enum ufs_pm_op );void (*dbg_register_dump)(struct ufs_hba *hba);int (*phy_initialization)(struct ufs_hba *); -int (*device_reset)(struct ufs_hba *hba); +int (*device_reset)(struct ufs_hba *hba,bool asserted);void (*config_scaling_param)(struct ufs_hba *hba,struct devfreq_dev_profile *profile,void *data);@@ -1216,10 +1216,10 @@ static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba) hba->vops->dbg_register_dump(hba); } -static inline int ufshcd_vops_device_reset (struct ufs_hba *hba) +static inline int ufshcd_vops_device_reset (struct ufs_hba *hba,bool asserted) {if (hba->vops && hba->vops->device_reset) -return hba->vops->device_reset (hba); +return hba->vops->device_reset (hba, asserted);
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
return
-EOPNOTSUPP;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
_______________________________________________
Linux-mediatek mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Quoted 108 lines · Can Guo
Quoted 103 lines · Can Guo
Quoted 61 lines · Bjorn Andersson
Quoted 59 lines · Ziqi Chen
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.Cc Kiwoong Kim <[email protected]>Cc Stanley Chu <[email protected]>Signed-off-by Ziqi Chen <[email protected]>--- drivers/scsi/ufs/ufs-mediatek.c | 32 ++++++++++++++++---------------- drivers/scsi/ufs/ufs-qcom.c | 24 +++++++++++++++--------- drivers/scsi/ufs/ufshcd.c | 36 +++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 8 ++++---- 4 files changed, 60 insertions(+), 40 deletions(-)drivers/scsi/ufs/ufs-mediatek.c +8
-15@@ -841,27 +841,27 @@ static int ufs_mtk_link_startup_notify(struct ufs_hba *hba, return ret; } -static int ufs_mtk_device_reset (struct ufs_hba *hba) +static int ufs_mtk_device_reset (struct ufs_hba *hba,bool asserted) {struct arm_smccc_res res; -ufs_mtk_device_reset_ctrl (0 , res); +if (asserted) { +ufs_mtk_device_reset_ctrl (0 , res); -/* -* The reset signal is active low. UFS devices shall detect -* more than or equal to 1us of positive or negative RST_n -* pulse width. -* -* To be on safe side, keep the reset low for at least 10us. -*/ -usleep_range (10 ,15 ); - -ufs_mtk_device_reset_ctrl (1 , res); - -/* Some devices may need time to respond to rst_n */ -usleep_range (10000 ,15000 ); +/* +* The reset signal is active low. UFS devices shall detect +* more than or equal to 1us of positive or negative RST_n +* pulse width. +* Quoted 8 lines
drivers/scsi/ufs/ufs-mediatek.c static int ufs_mtk_link_startup_notify(struct ufs_hba *hba, · line 858
Quoted 3 lines
drivers/scsi/ufs/ufs-mediatek.c +3
-0+* To be on safe side, keep the reset low for at least 10us. +*/ +usleep_range (10 ,15 );I see no point in allowing vendors to "tweak" the 1us->10us adjustment.
The specification says 1us and we all agree that 10us gives us good enough slack. I.e. this is common code.Hi Bjron,
We tried, but Samsung fellows wanted 5us. We couldn't get a agreement on this delay in short term, so we chose to leave it in vops.
Quoted 13 lines
drivers/scsi/ufs/ufs-mediatek.c static int ufs_mtk_link_startup_notify(struct ufs_hba *hba, · line 863
Quoted 7 lines
drivers/scsi/ufs/ufs-mediatek.c +4
-1+ }else { +ufs_mtk_device_reset_ctrl (1 , res); -dev_info (hba->dev,"device reset done\n" ); +/* Some devices may need time to respond to rst_n */ +usleep_range (10000 ,15000 );The comment in both the Qualcomm and Mediatek drivers claim that this is
sleep relates to the UFS device (not host), so why should it be different?Quoted 5 lines
What happens if I take the device that Mediatek see a need for a 10ms delay and hook that up to a Qualcomm host? This really should go in the
common code.Agree, but Qualcomm host didn't have any problems with 10us yet, so if we put
the 10ms delay to common code, Qualcomm host would suffer longer delay when
device reset happens - both bootup andresume(xpm_lvl = 5/6)latency would
be increased.Quoted 3 lines
Regards,
Can Guo.
Quoted 6 lines
Besides, currently this device reset vops is only registered by
ufs-qcom.candufs-mediatek.c, meaning any delays that we put in the common code are not
necessary for those who do not have this vops registered, i.eufs-exynos.c,ufs-hisi.c.
Surely we can detect this in the common code and only sleep if the vops is implemented - and successfully deasserted the reset.
Regards,
Bjorn
_______________________________________________
Linux-mediatek mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Quoted 69 lines · Can Guo
Quoted 59 lines · Bjorn Andersson
Quoted 57 lines · Ziqi Chen
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.Cc Kiwoong Kim <[email protected]>Cc Stanley Chu <[email protected]>Signed-off-by Ziqi Chen <[email protected]>--- drivers/scsi/ufs/ufs-mediatek.c | 32 ++++++++++++++++---------------- drivers/scsi/ufs/ufs-qcom.c | 24 +++++++++++++++--------- drivers/scsi/ufs/ufshcd.c | 36 +++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 8 ++++---- 4 files changed, 60 insertions(+), 40 deletions(-)drivers/scsi/ufs/ufs-mediatek.c +7
-15@@ -841,27 +841,27 @@ static int ufs_mtk_link_startup_notify(structufs_hba *hba, return ret; } -static int ufs_mtk_device_reset (struct ufs_hba *hba) +static int ufs_mtk_device_reset (struct ufs_hba *hba,bool asserted) {struct arm_smccc_res res; -ufs_mtk_device_reset_ctrl (0 , res); +if (asserted) { +ufs_mtk_device_reset_ctrl (0 , res); -/* -* The reset signal is active low. UFS devices shall detect -* more than or equal to 1us of positive or negative RST_n -* pulse width. -* -* To be on safe side, keep the reset low for at least 10us. -*/ -usleep_range (10 ,15 ); - -ufs_mtk_device_reset_ctrl (1 , res); - -/* Some devices may need time to respond to rst_n */ -usleep_range (10000 ,15000 ); +/* +* The reset signal is active low. UFS devices shall detect +* more than or equal to 1us of positive or negative RST_n +* pulse width. Quoted 8 lines
drivers/scsi/ufs/ufs-mediatek.c static int ufs_mtk_link_startup_notify(struct ufs_hba *hba, · line 858
Quoted 4 lines
drivers/scsi/ufs/ufs-mediatek.c +4
-0+* +* To be on safe side, keep the reset low for at least 10us. +*/ +usleep_range (10 ,15 );I see no point in allowing vendors to "tweak" the 1us->10us adjustment. The specification says 1us and we all agree that 10us gives us good enough slack. I.e. this is common code.
Quoted 5 lines
Hi Bjron,
We tried, but Samsung fellows wanted 5us. We couldn't get a agreement on this delay in short term, so we chose to leave it in vops.
I'm not able to find the code you're referring to.
Quoted 17 lines
Quoted 12 lines
drivers/scsi/ufs/ufs-mediatek.c static int ufs_mtk_link_startup_notify(struct ufs_hba *hba, · line 863
Quoted 7 lines
drivers/scsi/ufs/ufs-mediatek.c +4
-1+ }else { +ufs_mtk_device_reset_ctrl (1 , res); -dev_info (hba->dev,"device reset done\n" ); +/* Some devices may need time to respond to rst_n */ +usleep_range (10000 ,15000 );The comment in both the Qualcomm and Mediatek drivers claim that this is sleep relates to the UFS device (not host), so why should it be different?
Quoted 4 lines
What happens if I take the device that Mediatek see a need for a 10ms delay and hook that up to a Qualcomm host? This really should go in the common code.
Quoted 6 lines
Agree, but Qualcomm host didn't have any problems with 10us yet, so if we put
the 10ms delay to common code, Qualcomm host would suffer longer delay when device reset happens - both bootup andresume(xpm_lvl = 5/6)latency would be increased.
Okay, for the resume case I accept that this is a measurable difference. I still believe this is a property of the device and not the platform though.
Regards,
Bjorn
Quoted 227 lines
Regards,
Can Guo.Quoted 224 lines
As such I really would prefer to see these delays in the common code! You really shouldn't write code based on a speculation that one day there might come someone who need other values - when that day come we can just change the code, and if it never comes we're better off with the cleaner implementation.
Regards,
BjornQuoted 213 lines
drivers/scsi/ufs/ufs-mediatek.c +1
-0+ }return 0 ;}
drivers/scsi/ufs/ufs-qcom.c +2
-3@@ -1406,10 +1406,11 @@ static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba)/** * ufs_qcom_device_reset() - toggle the (optional) device reset line * @hba: per-adapter instance +* @asserted: assert or deassert device reset line * * Toggles the (optional) reset line to reset the attached device. */ -static int ufs_qcom_device_reset (struct ufs_hba *hba) +static int ufs_qcom_device_reset (struct ufs_hba *hba,bool asserted) {struct ufs_qcom_host *host =ufshcd_get_variant (hba);@@ -1417,15 +1418,20 @@ static int ufs_qcom_device_reset(struct ufs_hba *hba)if (!host->device_reset)return -EOPNOTSUPP ; -/* -* The UFS device shall detect reset pulses of 1us, sleep for 10us to
drivers/scsi/ufs/ufs-qcom.c +4
-6-* be on the safe side. -*/ -gpiod_set_value_cansleep (host->device_reset,1 ); -usleep_range (10 ,15 ); +if (asserted) { +gpiod_set_value_cansleep (host->device_reset,1 ); -gpiod_set_value_cansleep (host->device_reset,0 ); -usleep_range (10 ,15 ); +/* +* The UFS device shall detect reset pulses of 1us, sleep for 10us to
drivers/scsi/ufs/ufs-qcom.c +9
-0+* be on the safe side. +*/ +usleep_range (10 ,15 ); + }else { +gpiod_set_value_cansleep (host->device_reset,0 ); + +/* Some devices may need time to respond to rst_n */ +usleep_range (10 ,15 ); + }return 0 ; }drivers/scsi/ufs/ufshcd.c +7
-1@@ -585,7 +585,13 @@ static void ufshcd_device_reset(struct ufs_hba *hba) {int err; - err =ufshcd_vops_device_reset (hba); + err =ufshcd_vops_device_reset (hba, true); +if (err) { +dev_err (hba->dev,"asserting device reset failed: %d\n" , err); +return ; + } + + err =ufshcd_vops_device_reset (hba, false);if (!err) {drivers/scsi/ufs/ufshcd.c +4
-0ufshcd_set_ufs_dev_active (hba);@@ -593,7 +599,11 @@ static void ufshcd_device_reset(struct ufs_hba *hba) hba->wb_enabled = false; hba->wb_buf_flush_enabled = false; } +dev_dbg (hba->dev,"device reset done\n" ); + }else { +dev_err (hba->dev,"deasserting device reset failed: %d\n" , err); } +if (err != -EOPNOTSUPP )ufshcd_update_evt_hist (hba,UFS_EVT_DEV_RESET , err);}
drivers/scsi/ufs/ufshcd.c +0
-2@@ -8686,8 +8696,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)if (ret) goto set_dev_active; - ufshcd_vreg_set_lpm(hba); - disable_clks:/* * Call vendor specific suspend callback. As these callbacks may
accessdrivers/scsi/ufs/ufshcd.c +3
-0@@ -8703,6 +8711,9 @@ static int ufshcd_suspend(struct ufs_hba *hba,enum ufs_pm_op pm_op) */ ufshcd_disable_irq (hba); +if (ufshcd_is_link_off (hba)) +ufshcd_vops_device_reset (hba, true); +ufshcd_setup_clocks (hba, false);drivers/scsi/ufs/ufshcd.c +2
-0if (ufshcd_is_clkgating_allowed (hba)) {@@ -8711,6 +8722,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) hba->clk_gating.state); } +ufshcd_vreg_set_lpm (hba); +/* Put the host controller in low power mode if possible */ ufshcd_hba_vreg_set_lpm (hba);drivers/scsi/ufs/ufshcd.c +6
-5goto out;@@ -8778,18 +8791,19 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) old_link_state = hba->uic_link_state;ufshcd_hba_vreg_set_hpm (hba); + + ret =ufshcd_vreg_set_hpm (hba); +if (ret) + goto out; +/* Make sure clocks are enabled before accessing controller */ ret =ufshcd_setup_clocks (hba, true);if (ret) - goto out; + goto disable_vreg;/* enable the host irq as host controller would be active soon */ ufshcd_enable_irq (hba); - ret =ufshcd_vreg_set_hpm (hba); -if (ret) - goto disable_irq_and_vops_clks; -/* * Call vendor specific resume callback. As these callbacks may access
drivers/scsi/ufs/ufshcd.c +1
-1* vendor specific host controller register space call them when the @@ -8797,7 +8811,7 @@ static int ufshcd_resume(struct ufs_hba *hba,enum ufs_pm_op pm_op) */ ret =ufshcd_vops_resume (hba, pm_op);if (ret) - goto disable_vreg; + goto disable_irq_and_vops_clks;/* For DeepSleep, the only supported option is to have the link off */
WARN_ON(ufshcd_is_ufs_dev_deepsleep(hba) &&
!ufshcd_is_link_off(hba));drivers/scsi/ufs/ufshcd.c +0
-2@@ -8864,8 +8878,6 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) ufshcd_link_state_transition(hba, old_link_state, 0); vendor_suspend: ufshcd_vops_suspend(hba, pm_op); -disable_vreg: -ufshcd_vreg_set_lpm (hba); disable_irq_and_vops_clks:ufshcd_disable_irq (hba);drivers/scsi/ufs/ufshcd.c +2
-0if (hba->clk_scaling.is_allowed)@@ -8876,6 +8888,8 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) trace_ufshcd_clk_gating(dev_name(hba->dev), hba->clk_gating.state); } +disable_vreg: +ufshcd_vreg_set_lpm (hba); out: hba->pm_op_in_progress =0 ;drivers/scsi/ufs/ufshcd.c +0
-0if (ret)drivers/scsi/ufs/ufshcd.h +4
-4@@ -319,7 +319,7 @@ struct ufs_pwr_mode_info {* @resume: called during host controller PM callback * @dbg_register_dump: used to dump controller debug information * @phy_initialization: used to initialize phys -* @device_reset: called to issue a reset pulse on the UFS device +* @device_reset: called to assert or deassert device reset line * @program_key: program or evict an inline encryption key * @event_notify: called to notify important events */ @@ -350,7 +350,7 @@ struct ufs_hba_variant_ops {int (*resume)(struct ufs_hba *,enum ufs_pm_op );void (*dbg_register_dump)(struct ufs_hba *hba);int (*phy_initialization)(struct ufs_hba *); -int (*device_reset)(struct ufs_hba *hba); +int (*device_reset)(struct ufs_hba *hba,bool asserted);void (*config_scaling_param)(struct ufs_hba *hba,struct devfreq_dev_profile *profile,void *data);@@ -1216,10 +1216,10 @@ static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba) hba->vops->dbg_register_dump(hba); } -static inline int ufshcd_vops_device_reset (struct ufs_hba *hba) +static inline int ufshcd_vops_device_reset (struct ufs_hba *hba,bool asserted) {if (hba->vops && hba->vops->device_reset) -return hba->vops->device_reset (hba); +return hba->vops->device_reset (hba, asserted);
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
return
-EOPNOTSUPP;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Linux-mediatek mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Quoted 71 lines · Bjorn Andersson
Quoted 69 lines · Can Guo
Quoted 67 lines · Bjorn Andersson
Quoted 51 lines · Ziqi Chen
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.Cc Kiwoong Kim <[email protected]>Cc Stanley Chu <[email protected]>Signed-off-by Ziqi Chen <[email protected]>--- drivers/scsi/ufs/ufs-mediatek.c | 32 ++++++++++++++++---------------- drivers/scsi/ufs/ufs-qcom.c | 24 +++++++++++++++--------- drivers/scsi/ufs/ufshcd.c | 36 +++++++++++++++++++++++++----------- drivers/scsi/ufs/ufshcd.h | 8 ++++---- 4 files changed, 60 insertions(+), 40 deletions(-)drivers/scsi/ufs/ufs-mediatek.c +3
-13@@ -841,27 +841,27 @@ static int ufs_mtk_link_startup_notify(structufs_hba *hba, return ret; } -static int ufs_mtk_device_reset (struct ufs_hba *hba) +static int ufs_mtk_device_reset (struct ufs_hba *hba,bool asserted) {struct arm_smccc_res res; -ufs_mtk_device_reset_ctrl (0 , res); +if (asserted) { +ufs_mtk_device_reset_ctrl (0 , res); -/* -* The reset signal is active low. UFS devices shall detect -* more than or equal to 1us of positive or negative RST_n -* pulse width. -* -* To be on safe side, keep the reset low for at least 10us. -*/ -usleep_range (10 ,15 ); - -ufs_mtk_device_reset_ctrl (1 , res); -drivers/scsi/ufs/ufs-mediatek.c static int ufs_mtk_link_startup_notify(struct · line 859
Quoted 10 lines
drivers/scsi/ufs/ufs-mediatek.c +8
-2-/* Some devices may need time to respond to rst_n */ -usleep_range (10000 ,15000 ); +/* +* The reset signal is active low. UFS devices shall detect +* more than or equal to 1us of positive or negative RST_n +* pulse width. +* +* To be on safe side, keep the reset low for at least 10us. +*/ +usleep_range (10 ,15 );I see no point in allowing vendors to "tweak" the 1us->10us adjustment. The specification says 1us and we all agree that 10us gives us good enough slack. I.e. this is common code.
Quoted 7 lines
Quoted 5 lines
Hi Bjron,
We tried, but Samsung fellows wanted 5us. We couldn't get a agreement on this delay in short term, so we chose to leave it in vops.
I'm not able to find the code you're referring to.
static void exynos_ufs_dev_hw_reset(struct ufs_hba *hba)
{
struct exynos_ufs *ufs = ufshcd_get_variant(hba);
hci_writel(ufs, 0 << 0, HCI_GPIO_OUT);
udelay(5);
hci_writel(ufs, 1 << 0, HCI_GPIO_OUT);
}Quoted 25 lines
Quoted 18 lines
Quoted 13 lines
drivers/scsi/ufs/ufs-mediatek.c static int ufs_mtk_link_startup_notify(struct ufs_hba *hba, · line 863
Quoted 8 lines
drivers/scsi/ufs/ufs-mediatek.c +4
-1+ }else { +ufs_mtk_device_reset_ctrl (1 , res); -dev_info (hba->dev,"device reset done\n" ); +/* Some devices may need time to respond to rst_n */ +usleep_range (10000 ,15000 );The comment in both the Qualcomm and Mediatek drivers claim that this is sleep relates to the UFS device (not host), so why should it be different?
Quoted 4 lines
What happens if I take the device that Mediatek see a need for a 10ms delay and hook that up to a Qualcomm host? This really should go in the common code.
Quoted 6 lines
Agree, but Qualcomm host didn't have any problems with 10us yet, so if we put
the 10ms delay to common code, Qualcomm host would suffer longer delay when device reset happens - both bootup andresume(xpm_lvl = 5/6)latency would be increased.
Quoted 4 lines
Okay, for the resume case I accept that this is a measurable difference. I still believe this is a property of the device and not the platform though.
True, I also wanted to make the codes simpler and universal for everyone...
Quoted 230 lines
Regards,
BjornQuoted 227 lines
Regards,
Can Guo.Quoted 224 lines
As such I really would prefer to see these delays in the common code! You really shouldn't write code based on a speculation that one day there might come someone who need other values - when that day come we can just change the code, and if it never comes we're better off with the cleaner implementation.
Regards,
BjornQuoted 213 lines
drivers/scsi/ufs/ufs-mediatek.c +1
-0+ }return 0 ;}
drivers/scsi/ufs/ufs-qcom.c +2
-3@@ -1406,10 +1406,11 @@ static void ufs_qcom_dump_dbg_regs(struct ufs_hba *hba)/** * ufs_qcom_device_reset() - toggle the (optional) device reset line * @hba: per-adapter instance +* @asserted: assert or deassert device reset line * * Toggles the (optional) reset line to reset the attached device. */ -static int ufs_qcom_device_reset (struct ufs_hba *hba) +static int ufs_qcom_device_reset (struct ufs_hba *hba,bool asserted) {struct ufs_qcom_host *host =ufshcd_get_variant (hba);@@ -1417,15 +1418,20 @@ static int ufs_qcom_device_reset(struct ufs_hba *hba)if (!host->device_reset)return -EOPNOTSUPP ; -/* -* The UFS device shall detect reset pulses of 1us, sleep for 10us to
drivers/scsi/ufs/ufs-qcom.c +4
-6-* be on the safe side. -*/ -gpiod_set_value_cansleep (host->device_reset,1 ); -usleep_range (10 ,15 ); +if (asserted) { +gpiod_set_value_cansleep (host->device_reset,1 ); -gpiod_set_value_cansleep (host->device_reset,0 ); -usleep_range (10 ,15 ); +/* +* The UFS device shall detect reset pulses of 1us, sleep for 10us to
drivers/scsi/ufs/ufs-qcom.c +9
-0+* be on the safe side. +*/ +usleep_range (10 ,15 ); + }else { +gpiod_set_value_cansleep (host->device_reset,0 ); + +/* Some devices may need time to respond to rst_n */ +usleep_range (10 ,15 ); + }return 0 ; }drivers/scsi/ufs/ufshcd.c +7
-1@@ -585,7 +585,13 @@ static void ufshcd_device_reset(struct ufs_hba *hba) {int err; - err =ufshcd_vops_device_reset (hba); + err =ufshcd_vops_device_reset (hba, true); +if (err) { +dev_err (hba->dev,"asserting device reset failed: %d\n" , err); +return ; + } + + err =ufshcd_vops_device_reset (hba, false);if (!err) {drivers/scsi/ufs/ufshcd.c +4
-0ufshcd_set_ufs_dev_active (hba);@@ -593,7 +599,11 @@ static void ufshcd_device_reset(struct ufs_hba *hba) hba->wb_enabled = false; hba->wb_buf_flush_enabled = false; } +dev_dbg (hba->dev,"device reset done\n" ); + }else { +dev_err (hba->dev,"deasserting device reset failed: %d\n" , err); } +if (err != -EOPNOTSUPP )ufshcd_update_evt_hist (hba,UFS_EVT_DEV_RESET , err);}
drivers/scsi/ufs/ufshcd.c +0
-2@@ -8686,8 +8696,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)if (ret) goto set_dev_active; - ufshcd_vreg_set_lpm(hba); - disable_clks:/* * Call vendor specific suspend callback. As these callbacks may
accessdrivers/scsi/ufs/ufshcd.c +3
-0@@ -8703,6 +8711,9 @@ static int ufshcd_suspend(struct ufs_hba *hba,enum ufs_pm_op pm_op) */ ufshcd_disable_irq (hba); +if (ufshcd_is_link_off (hba)) +ufshcd_vops_device_reset (hba, true); +ufshcd_setup_clocks (hba, false);drivers/scsi/ufs/ufshcd.c +2
-0if (ufshcd_is_clkgating_allowed (hba)) {@@ -8711,6 +8722,8 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) hba->clk_gating.state); } +ufshcd_vreg_set_lpm (hba); +/* Put the host controller in low power mode if possible */ ufshcd_hba_vreg_set_lpm (hba);drivers/scsi/ufs/ufshcd.c +6
-5goto out;@@ -8778,18 +8791,19 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) old_link_state = hba->uic_link_state;ufshcd_hba_vreg_set_hpm (hba); + + ret =ufshcd_vreg_set_hpm (hba); +if (ret) + goto out; +/* Make sure clocks are enabled before accessing controller */ ret =ufshcd_setup_clocks (hba, true);if (ret) - goto out; + goto disable_vreg;/* enable the host irq as host controller would be active soon */ ufshcd_enable_irq (hba); - ret =ufshcd_vreg_set_hpm (hba); -if (ret) - goto disable_irq_and_vops_clks; -/* * Call vendor specific resume callback. As these callbacks may access
drivers/scsi/ufs/ufshcd.c +1
-1* vendor specific host controller register space call them when the @@ -8797,7 +8811,7 @@ static int ufshcd_resume(struct ufs_hba *hba,enum ufs_pm_op pm_op) */ ret =ufshcd_vops_resume (hba, pm_op);if (ret) - goto disable_vreg; + goto disable_irq_and_vops_clks;/* For DeepSleep, the only supported option is to have the link off */
WARN_ON(ufshcd_is_ufs_dev_deepsleep(hba) &&
!ufshcd_is_link_off(hba));drivers/scsi/ufs/ufshcd.c +0
-2@@ -8864,8 +8878,6 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) ufshcd_link_state_transition(hba, old_link_state, 0); vendor_suspend: ufshcd_vops_suspend(hba, pm_op); -disable_vreg: -ufshcd_vreg_set_lpm (hba); disable_irq_and_vops_clks:ufshcd_disable_irq (hba);drivers/scsi/ufs/ufshcd.c +2
-0if (hba->clk_scaling.is_allowed)@@ -8876,6 +8888,8 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) trace_ufshcd_clk_gating(dev_name(hba->dev), hba->clk_gating.state); } +disable_vreg: +ufshcd_vreg_set_lpm (hba); out: hba->pm_op_in_progress =0 ;drivers/scsi/ufs/ufshcd.c +0
-0if (ret)drivers/scsi/ufs/ufshcd.h +4
-4@@ -319,7 +319,7 @@ struct ufs_pwr_mode_info {* @resume: called during host controller PM callback * @dbg_register_dump: used to dump controller debug information * @phy_initialization: used to initialize phys -* @device_reset: called to issue a reset pulse on the UFS device +* @device_reset: called to assert or deassert device reset line * @program_key: program or evict an inline encryption key * @event_notify: called to notify important events */ @@ -350,7 +350,7 @@ struct ufs_hba_variant_ops {int (*resume)(struct ufs_hba *,enum ufs_pm_op );void (*dbg_register_dump)(struct ufs_hba *hba);int (*phy_initialization)(struct ufs_hba *); -int (*device_reset)(struct ufs_hba *hba); +int (*device_reset)(struct ufs_hba *hba,bool asserted);void (*config_scaling_param)(struct ufs_hba *hba,struct devfreq_dev_profile *profile,void *data);@@ -1216,10 +1216,10 @@ static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba) hba->vops->dbg_register_dump(hba); } -static inline int ufshcd_vops_device_reset (struct ufs_hba *hba) +static inline int ufshcd_vops_device_reset (struct ufs_hba *hba,bool asserted) {if (hba->vops && hba->vops->device_reset) -return hba->vops->device_reset (hba); +return hba->vops->device_reset (hba, asserted);
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
return
-EOPNOTSUPP;
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Linux-mediatek mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Quoted 9 lines · Ziqi Chen
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.
This patch assumes the power is controlled by voltage regulators, but for us it is controlled by firmware (ACPI), so it is not correct to change RST_n for all host controllers as you are doing.
Also we might need to use a firmware interface for device reset, in which case the 'asserted' value doe not make sense.
Can we leave the device reset callback alone, and instead introduce a new variant operation for setting RST_n to match voltage regulator power changes?
_______________________________________________
Linux-mediatek mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Quoted 11 lines · Adrian Hunter
Quoted 9 lines · Ziqi Chen
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.
Quoted 7 lines
This patch assumes the power is controlled by voltage regulators, but for us it is controlled by firmware (ACPI), so it is not correct to change
RST_nfor all host controllers as you are doing.Also we might need to use a firmware interface for device reset, in which case the 'asserted' value doe not make sense.
Are you saying that the entire flip-flop-the-reset is a single firmware operation in your case? If you look at the Mediatek driver, the implementation of ufs_mtk_device_reset_ctrl() is a jump to firmware.
But perhaps "asserted" isn't the appropriate English word for saying "the reset is in the resetting state"?
I just wanted to avoid the use of "high"/"lo" as if you look at the Mediatek code they pass the expected line-level to the firmware, while in the Qualcomm code we pass the logical state to the GPIO code which is setup up as "active low" and thereby flip the meaning before hitting the pad.
Quoted 2 lines
Can we leave the device reset callback alone, and instead introduce a new variant operation for setting
RST_nto match voltage regulator power changes?
Wouldn't this new function just have to look like the proposed patches? In which case for existing platforms we'd have both?
How would you implement this, or would you simply skip implementing this?
Regards,
Bjorn
_______________________________________________
Linux-mediatek mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Quoted 17 lines · Bjorn Andersson
Quoted 15 lines · Adrian Hunter
Quoted 9 lines · Ziqi Chen
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.This patch assumes the power is controlled by voltage regulators, but for us it is controlled by firmware (ACPI), so it is not correct to change
RST_nfor all host controllers as you are doing.
Quoted 6 lines
Quoted 3 lines
Also we might need to use a firmware interface for device reset, in which case the 'asserted' value doe not make sense.
Are you saying that the entire flip-flop-the-reset is a single firmware operation in your case?
Yes
Quoted 13 lines
If you look at the Mediatek driver, the
implementation ofufs_mtk_device_reset_ctrl()is a jump to firmware.But perhaps "asserted" isn't the appropriate English word for saying "the reset is in the resetting state"?
I just wanted to avoid the use of "high"/"lo" as if you look at the Mediatek code they pass the expected line-level to the firmware, while in the Qualcomm code we pass the logical state to the GPIO code which is setup up as "active low" and thereby flip the meaning before hitting the pad.
Quoted 8 lines
Quoted 2 lines
Can we leave the device reset callback alone, and instead introduce a new variant operation for setting
RST_nto match voltage regulator power changes?Wouldn't this new function just have to look like the proposed patches? In which case for existing platforms we'd have both?
How would you implement this, or would you simply skip implementing this?
Functionally, doing a device reset is not the same as adjusting signal levels to meet power up/off ramp requirements. However, the issue is that we do not use regulators, so the power is not necessarily being changed at those points, and we definitely do not want to reset instead of entering DeepSleep for example.
Off the top of my head, I imagine something like a callback called ufshcd_vops_prepare_power_ramp(hba, bool on) which is called only if hba->vreg_info->vcc is not NULL.
_______________________________________________
Linux-mediatek mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Quoted 55 lines · Adrian Hunter
Quoted 17 lines · Bjorn Andersson
Quoted 15 lines · Adrian Hunter
Quoted 9 lines · Ziqi Chen
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.This patch assumes the power is controlled by voltage regulators, but for us it is controlled by firmware (ACPI), so it is not correct to change
RST_nfor all host controllers as you are doing.Quoted 6 lines
Quoted 3 lines
Also we might need to use a firmware interface for device reset, in which case the 'asserted' value doe not make sense.
Are you saying that the entire flip-flop-the-reset is a single firmware operation in your case?
Yes
Quoted 13 lines
If you look at the Mediatek driver, the
implementation ofufs_mtk_device_reset_ctrl()is a jump to firmware.But perhaps "asserted" isn't the appropriate English word for saying "the reset is in the resetting state"?
I just wanted to avoid the use of "high"/"lo" as if you look at the Mediatek code they pass the expected line-level to the firmware, while in the Qualcomm code we pass the logical state to the GPIO code which is setup up as "active low" and thereby flip the meaning before hitting the pad.
Quoted 8 lines
Quoted 2 lines
Can we leave the device reset callback alone, and instead introduce a new variant operation for setting
RST_nto match voltage regulator power changes?Wouldn't this new function just have to look like the proposed patches? In which case for existing platforms we'd have both?
How would you implement this, or would you simply skip implementing this?
Functionally, doing a device reset is not the same as adjusting signal levels to meet power up/off ramp requirements. However, the issue is that we do not use regulators, so the power is not necessarily being changed at those points, and we definitely do not want to reset instead of entering DeepSleep for example.
Quoted 3 lines
Off the top of my head, I imagine something like a callback called
ufshcd_vops_prepare_power_ramp(hba, bool on)which is called only ifhba->vreg_info->vccis not NULL.
Hi Adrian,
I don't see you have the vops device_reset() implemented anywhere in current code base, how is this change impacting you? Do I miss anything or are you planning to push a change which implements device_reset() soon?
Thanks,
Can Guo.
_______________________________________________
Linux-mediatek mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Quoted 62 lines · Can Guo
Quoted 57 lines · Adrian Hunter
Quoted 18 lines · Bjorn Andersson
Quoted 16 lines · Adrian Hunter
Quoted 9 lines · Ziqi Chen
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.This patch assumes the power is controlled by voltage regulators, but for us
it is controlled by firmware (ACPI), so it is not correct to changeRST_nfor all host controllers as you are doing.Quoted 6 lines
Quoted 3 lines
Also we might need to use a firmware interface for device reset, in which case the 'asserted' value doe not make sense.
Are you saying that the entire flip-flop-the-reset is a single firmware operation in your case?
Yes
Quoted 17 lines
If you look at the Mediatek driver, the implementation of
ufs_mtk_device_reset_ctrl()is a jump to firmware.But perhaps "asserted" isn't the appropriate English word for saying "the reset is in the resetting state"?
I just wanted to avoid the use of "high"/"lo" as if you look at the Mediatek code they pass the expected line-level to the firmware, while in the Qualcomm code we pass the logical state to the GPIO code which is setup up as "active low" and thereby flip the meaning before hitting the pad.
Quoted 3 lines
Can we leave the device reset callback alone, and instead introduce a new variant operation for setting
RST_nto match voltage regulator power changes?Quoted 5 lines
Wouldn't this new function just have to look like the proposed patches? In which case for existing platforms we'd have both?
How would you implement this, or would you simply skip implementing this?
Functionally, doing a device reset is not the same as adjusting signal levels to meet power up/off ramp requirements. However, the issue is that we do not use regulators, so the power is not necessarily being changed at those points, and we definitely do not want to reset instead of entering DeepSleep for example.
Quoted 3 lines
Off the top of my head, I imagine something like a callback called
ufshcd_vops_prepare_power_ramp(hba, bool on)which is called only ifhba->vreg_info->vccis not NULL.
Quoted 5 lines
Hi Adrian,
I don't see you have the vops
device_reset()implemented anywhere in current code base, how is this change impacting you? Do I miss anything or are you planning to push a change which implementsdevice_reset()soon?
At some point, yes.
_______________________________________________
Linux-mediatek mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Quoted 63 lines · Adrian Hunter
Quoted 62 lines · Can Guo
Quoted 60 lines · Adrian Hunter
Quoted 18 lines · Bjorn Andersson
Quoted 16 lines · Adrian Hunter
Quoted 9 lines · Ziqi Chen
As per specs, e.g, JESD220E chapter 7.2, while powering off/on the ufs device,
RST_Nsignal andREF_CLKsignal should be between VSS(Ground) and VCCQ/VCCQ2.To flexibly control device reset line, refactor the function
ufschd_vops_device_reset(sturct ufs_hba *hba)toufshcd_vops_device_reset(sturct ufs_hba *hba, bool asserted). The new parameter "bool asserted" is used to separate device reset line pulling down from pulling up.This patch assumes the power is controlled by voltage regulators, but for us
it is controlled by firmware (ACPI), so it is not correct to changeRST_nfor all host controllers as you are doing.Quoted 6 lines
Quoted 3 lines
Also we might need to use a firmware interface for device reset, in which case the 'asserted' value doe not make sense.
Are you saying that the entire flip-flop-the-reset is a single firmware operation in your case?
Yes
Quoted 17 lines
If you look at the Mediatek driver, the implementation of
ufs_mtk_device_reset_ctrl()is a jump to firmware.But perhaps "asserted" isn't the appropriate English word for saying "the reset is in the resetting state"?
I just wanted to avoid the use of "high"/"lo" as if you look at the Mediatek code they pass the expected line-level to the firmware, while in the Qualcomm code we pass the logical state to the GPIO code which is setup up as "active low" and thereby flip the meaning before hitting the pad.
Quoted 3 lines
Can we leave the device reset callback alone, and instead introduce a new variant operation for setting
RST_nto match voltage regulator power changes?Quoted 5 lines
Wouldn't this new function just have to look like the proposed patches? In which case for existing platforms we'd have both?
How would you implement this, or would you simply skip implementing this?
Functionally, doing a device reset is not the same as adjusting signal levels to meet power up/off ramp requirements. However, the issue is that we do not use regulators, so the power is not necessarily being changed at those points, and we definitely do not want to reset instead of entering DeepSleep for example.
Off the top of my head, I imagine something like a callback called
ufshcd_vops_prepare_power_ramp(hba, bool on)which is called only ifhba->vreg_info->vccis not NULL.
Quoted 7 lines
Quoted 5 lines
Hi Adrian,
I don't see you have the vops
device_reset()implemented anywhere in current code base, how is this change impacting you? Do I miss anything or are you planning to push a change which implementsdevice_reset()soon?At some point, yes.
OK, then we don't even have to add a new vops, just go back to version #1 to use ufshcd_vops_suspend() to control the device_reset. We took the hard way because we wanted to fix it for all users.
_______________________________________________
Linux-mediatek mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-mediatek