linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/2] Introduce a vops for resetting host controller
@ 2019-10-23  4:13 Can Guo
  2019-10-23  4:13 ` [PATCH v1 1/2] scsi: ufs: " Can Guo
  2019-10-23  4:13 ` [PATCH v1 2/2] scsi: ufs-qcom: Add reset control support for " Can Guo
  0 siblings, 2 replies; 11+ messages in thread
From: Can Guo @ 2019-10-23  4:13 UTC (permalink / raw)
  To: asutoshd, nguyenb, rnayak, linux-scsi, kernel-team, saravanak,
	salyzyn, cang

Some UFS host controllers need their specific implementations of resetting
to get them into a good state. Provide a new vops to allow the platform
driver to implement this own reset operation.

Can Guo (2):
  scsi: ufs: Introduce a vops for resetting host controller
  scsi: ufs-qcom: Add reset control support for host controller

 drivers/scsi/ufs/ufs-qcom.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufs-qcom.h |  3 +++
 drivers/scsi/ufs/ufshcd.c   | 16 ++++++++++++++++
 drivers/scsi/ufs/ufshcd.h   | 10 ++++++++++
 4 files changed, 72 insertions(+)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v1 1/2] scsi: ufs: Introduce a vops for resetting host controller
  2019-10-23  4:13 [PATCH v1 0/2] Introduce a vops for resetting host controller Can Guo
@ 2019-10-23  4:13 ` Can Guo
  2019-10-23 10:39   ` [EXT] " Bean Huo (beanhuo)
                     ` (2 more replies)
  2019-10-23  4:13 ` [PATCH v1 2/2] scsi: ufs-qcom: Add reset control support for " Can Guo
  1 sibling, 3 replies; 11+ messages in thread
From: Can Guo @ 2019-10-23  4:13 UTC (permalink / raw)
  To: asutoshd, nguyenb, rnayak, linux-scsi, kernel-team, saravanak,
	salyzyn, cang
  Cc: Alim Akhtar, Avri Altman, Pedro Sousa, James E.J. Bottomley,
	Martin K. Petersen, Stanley Chu, Bean Huo, Tomas Winkler,
	Subhash Jadavani, Arnd Bergmann, Bjorn Andersson, open list

Some UFS host controllers need their specific implementations of resetting
to get them into a good state. Provide a new vops to allow the platform
driver to implement this own reset operation.

Signed-off-by: Can Guo <cang@codeaurora.org>
---
 drivers/scsi/ufs/ufshcd.c | 16 ++++++++++++++++
 drivers/scsi/ufs/ufshcd.h | 10 ++++++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c28c144..161e3c4 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -3859,6 +3859,14 @@ static int ufshcd_link_recovery(struct ufs_hba *hba)
 	ufshcd_set_eh_in_progress(hba);
 	spin_unlock_irqrestore(hba->host->host_lock, flags);
 
+	ret = ufshcd_vops_full_reset(hba);
+	if (ret)
+		dev_warn(hba->dev, "%s: full reset returned %d\n",
+				  __func__, ret);
+
+	/* Reset the attached device */
+	ufshcd_vops_device_reset(hba);
+
 	ret = ufshcd_host_reset_and_restore(hba);
 
 	spin_lock_irqsave(hba->host->host_lock, flags);
@@ -6241,6 +6249,11 @@ static int ufshcd_reset_and_restore(struct ufs_hba *hba)
 	int retries = MAX_HOST_RESET_RETRIES;
 
 	do {
+		err = ufshcd_vops_full_reset(hba);
+		if (err)
+			dev_warn(hba->dev, "%s: full reset returned %d\n",
+					__func__, err);
+
 		/* Reset the attached device */
 		ufshcd_vops_device_reset(hba);
 
@@ -8384,6 +8397,9 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 		goto exit_gating;
 	}
 
+	/* Reset controller to power on reset (POR) state */
+	ufshcd_vops_full_reset(hba);
+
 	/* Reset the attached device */
 	ufshcd_vops_device_reset(hba);
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index e0fe247..253b9ea 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -296,6 +296,8 @@ struct ufs_pwr_mode_info {
  * @apply_dev_quirks: called to apply device specific quirks
  * @suspend: called during host controller PM callback
  * @resume: called during host controller PM callback
+ * @full_reset: called for handling variant specific implementations of
+ *              resetting the hci
  * @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
@@ -325,6 +327,7 @@ struct ufs_hba_variant_ops {
 	int	(*apply_dev_quirks)(struct ufs_hba *);
 	int     (*suspend)(struct ufs_hba *, enum ufs_pm_op);
 	int     (*resume)(struct ufs_hba *, enum ufs_pm_op);
+	int	(*full_reset)(struct ufs_hba *hba);
 	void	(*dbg_register_dump)(struct ufs_hba *hba);
 	int	(*phy_initialization)(struct ufs_hba *);
 	void	(*device_reset)(struct ufs_hba *hba);
@@ -1076,6 +1079,13 @@ static inline int ufshcd_vops_resume(struct ufs_hba *hba, enum ufs_pm_op op)
 	return 0;
 }
 
+static inline int ufshcd_vops_full_reset(struct ufs_hba *hba)
+{
+	if (hba->vops && hba->vops->full_reset)
+		return hba->vops->full_reset(hba);
+	return 0;
+}
+
 static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba)
 {
 	if (hba->vops && hba->vops->dbg_register_dump)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v1 2/2] scsi: ufs-qcom: Add reset control support for host controller
  2019-10-23  4:13 [PATCH v1 0/2] Introduce a vops for resetting host controller Can Guo
  2019-10-23  4:13 ` [PATCH v1 1/2] scsi: ufs: " Can Guo
@ 2019-10-23  4:13 ` Can Guo
  1 sibling, 0 replies; 11+ messages in thread
From: Can Guo @ 2019-10-23  4:13 UTC (permalink / raw)
  To: asutoshd, nguyenb, rnayak, linux-scsi, kernel-team, saravanak,
	salyzyn, cang
  Cc: Andy Gross, Alim Akhtar, Avri Altman, Pedro Sousa,
	James E.J. Bottomley, Martin K. Petersen,
	open list:ARM/QUALCOMM SUPPORT, open list

Add reset control for host controller and provide it through vops to UFS
core driver.

Signed-off-by: Can Guo <cang@codeaurora.org>
---
 drivers/scsi/ufs/ufs-qcom.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufs-qcom.h |  3 +++
 2 files changed, 46 insertions(+)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index a5b7148..3dee906 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -576,6 +576,39 @@ static int ufs_qcom_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
 	return 0;
 }
 
+static int ufs_qcom_full_reset(struct ufs_hba *hba)
+{
+	int ret = -ENOTSUPP;
+	struct ufs_qcom_host *host = ufshcd_get_variant(hba);
+
+	if (!host->core_reset) {
+		dev_warn(hba->dev, "%s: failed, err = %d\n", __func__, ret);
+		goto out;
+	}
+
+	ret = reset_control_assert(host->core_reset);
+	if (ret) {
+		dev_err(hba->dev, "%s: core_reset assert failed, err = %d\n",
+				 __func__, ret);
+		goto out;
+	}
+
+	/*
+	 * The hardware requirement for delay between assert/deassert
+	 * is at least 3-4 sleep clock (32.7KHz) cycles, which comes to
+	 * ~125us (4/32768). To be on the safe side add 200us delay.
+	 */
+	usleep_range(200, 210);
+
+	ret = reset_control_deassert(host->core_reset);
+	if (ret)
+		dev_err(hba->dev, "%s: core_reset deassert failed, err = %d\n",
+				 __func__, ret);
+
+out:
+	return ret;
+}
+
 #ifdef CONFIG_MSM_BUS_SCALING
 static int ufs_qcom_get_bus_vote(struct ufs_qcom_host *host,
 		const char *speed_mode)
@@ -1101,6 +1134,15 @@ static int ufs_qcom_init(struct ufs_hba *hba)
 	host->hba = hba;
 	ufshcd_set_variant(hba, host);
 
+	/* Setup the reset control of HCI */
+	host->core_reset = devm_reset_control_get(hba->dev, "rst");
+	if (IS_ERR(host->core_reset)) {
+		err = PTR_ERR(host->core_reset);
+		dev_warn(dev, "Failed to get reset control %d\n", err);
+		host->core_reset = NULL;
+		err = 0;
+	}
+
 	/* Fire up the reset controller. Failure here is non-fatal. */
 	host->rcdev.of_node = dev->of_node;
 	host->rcdev.ops = &ufs_qcom_reset_ops;
@@ -1596,6 +1638,7 @@ static void ufs_qcom_device_reset(struct ufs_hba *hba)
 	.apply_dev_quirks	= ufs_qcom_apply_dev_quirks,
 	.suspend		= ufs_qcom_suspend,
 	.resume			= ufs_qcom_resume,
+	.full_reset		= ufs_qcom_full_reset,
 	.dbg_register_dump	= ufs_qcom_dump_dbg_regs,
 	.device_reset		= ufs_qcom_device_reset,
 };
diff --git a/drivers/scsi/ufs/ufs-qcom.h b/drivers/scsi/ufs/ufs-qcom.h
index d401f17..2d95e7c 100644
--- a/drivers/scsi/ufs/ufs-qcom.h
+++ b/drivers/scsi/ufs/ufs-qcom.h
@@ -6,6 +6,7 @@
 #define UFS_QCOM_H_
 
 #include <linux/reset-controller.h>
+#include <linux/reset.h>
 
 #define MAX_UFS_QCOM_HOSTS	1
 #define MAX_U32                 (~(u32)0)
@@ -233,6 +234,8 @@ struct ufs_qcom_host {
 	u32 dbg_print_en;
 	struct ufs_qcom_testbus testbus;
 
+	/* Reset control of HCI */
+	struct reset_control *core_reset;
 	struct reset_controller_dev rcdev;
 
 	struct gpio_desc *device_reset;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* RE: [EXT] [PATCH v1 1/2] scsi: ufs: Introduce a vops for resetting host controller
  2019-10-23  4:13 ` [PATCH v1 1/2] scsi: ufs: " Can Guo
@ 2019-10-23 10:39   ` Bean Huo (beanhuo)
  2019-10-29  2:11     ` cang
  2019-10-31 14:44   ` Mark Salyzyn
  2019-11-04 14:28   ` Avri Altman
  2 siblings, 1 reply; 11+ messages in thread
From: Bean Huo (beanhuo) @ 2019-10-23 10:39 UTC (permalink / raw)
  To: Can Guo, asutoshd, nguyenb, rnayak, linux-scsi, kernel-team,
	saravanak, salyzyn
  Cc: Alim Akhtar, Avri Altman, Pedro Sousa, James E.J. Bottomley,
	Martin K. Petersen, Stanley Chu, Tomas Winkler, Subhash Jadavani,
	Arnd Bergmann, Bjorn Andersson, open list

Hi, Can Guo
Actually, we already have DME_RESET,  this is not enough for Qualcomm host? 
Thanks,

//Bean

> 
> Some UFS host controllers need their specific implementations of resetting to
> get them into a good state. Provide a new vops to allow the platform driver to
> implement this own reset operation.
> 
> Signed-off-by: Can Guo <cang@codeaurora.org>
> ---
>  drivers/scsi/ufs/ufshcd.c | 16 ++++++++++++++++  drivers/scsi/ufs/ufshcd.h | 10
> ++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index
> c28c144..161e3c4 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -3859,6 +3859,14 @@ static int ufshcd_link_recovery(struct ufs_hba *hba)
>  	ufshcd_set_eh_in_progress(hba);
>  	spin_unlock_irqrestore(hba->host->host_lock, flags);
> 
> +	ret = ufshcd_vops_full_reset(hba);
> +	if (ret)
> +		dev_warn(hba->dev, "%s: full reset returned %d\n",
> +				  __func__, ret);
> +
> +	/* Reset the attached device */
> +	ufshcd_vops_device_reset(hba);
> +
>  	ret = ufshcd_host_reset_and_restore(hba);
> 
>  	spin_lock_irqsave(hba->host->host_lock, flags); @@ -6241,6 +6249,11
> @@ static int ufshcd_reset_and_restore(struct ufs_hba *hba)
>  	int retries = MAX_HOST_RESET_RETRIES;
> 
>  	do {
> +		err = ufshcd_vops_full_reset(hba);
> +		if (err)
> +			dev_warn(hba->dev, "%s: full reset returned %d\n",
> +					__func__, err);
> +
>  		/* Reset the attached device */
>  		ufshcd_vops_device_reset(hba);
> 
> @@ -8384,6 +8397,9 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem
> *mmio_base, unsigned int irq)
>  		goto exit_gating;
>  	}
> 
> +	/* Reset controller to power on reset (POR) state */
> +	ufshcd_vops_full_reset(hba);
> +
>  	/* Reset the attached device */
>  	ufshcd_vops_device_reset(hba);
> 
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index
> e0fe247..253b9ea 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -296,6 +296,8 @@ struct ufs_pwr_mode_info {
>   * @apply_dev_quirks: called to apply device specific quirks
>   * @suspend: called during host controller PM callback
>   * @resume: called during host controller PM callback
> + * @full_reset: called for handling variant specific implementations of
> + *              resetting the hci
>   * @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 @@ -325,6
> +327,7 @@ struct ufs_hba_variant_ops {
>  	int	(*apply_dev_quirks)(struct ufs_hba *);
>  	int     (*suspend)(struct ufs_hba *, enum ufs_pm_op);
>  	int     (*resume)(struct ufs_hba *, enum ufs_pm_op);
> +	int	(*full_reset)(struct ufs_hba *hba);
>  	void	(*dbg_register_dump)(struct ufs_hba *hba);
>  	int	(*phy_initialization)(struct ufs_hba *);
>  	void	(*device_reset)(struct ufs_hba *hba);
> @@ -1076,6 +1079,13 @@ static inline int ufshcd_vops_resume(struct ufs_hba
> *hba, enum ufs_pm_op op)
>  	return 0;
>  }
> 
> +static inline int ufshcd_vops_full_reset(struct ufs_hba *hba) {
> +	if (hba->vops && hba->vops->full_reset)
> +		return hba->vops->full_reset(hba);
> +	return 0;
> +}
> +
>  static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba)  {
>  	if (hba->vops && hba->vops->dbg_register_dump)
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [EXT] [PATCH v1 1/2] scsi: ufs: Introduce a vops for resetting host controller
  2019-10-23 10:39   ` [EXT] " Bean Huo (beanhuo)
@ 2019-10-29  2:11     ` cang
  0 siblings, 0 replies; 11+ messages in thread
From: cang @ 2019-10-29  2:11 UTC (permalink / raw)
  To: Bean Huo (beanhuo)
  Cc: asutoshd, nguyenb, rnayak, linux-scsi, kernel-team, saravanak,
	salyzyn, Alim Akhtar, Avri Altman, Pedro Sousa,
	James E.J. Bottomley, Martin K. Petersen, Stanley Chu,
	Tomas Winkler, Subhash Jadavani, Arnd Bergmann, Bjorn Andersson,
	open list

On 2019-10-23 18:39, Bean Huo (beanhuo) wrote:
> Hi, Can Guo
> Actually, we already have DME_RESET,  this is not enough for Qualcomm 
> host?
> Thanks,
> 
> //Bean
> 

Hi Bean,

Yes, for Qualcomm hosts, we need this to fully reset the whole UFS 
controller block

Can Guo

>> 
>> Some UFS host controllers need their specific implementations of 
>> resetting to
>> get them into a good state. Provide a new vops to allow the platform 
>> driver to
>> implement this own reset operation.
>> 
>> Signed-off-by: Can Guo <cang@codeaurora.org>
>> ---
>>  drivers/scsi/ufs/ufshcd.c | 16 ++++++++++++++++  
>> drivers/scsi/ufs/ufshcd.h | 10
>> ++++++++++
>>  2 files changed, 26 insertions(+)
>> 
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c 
>> index
>> c28c144..161e3c4 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -3859,6 +3859,14 @@ static int ufshcd_link_recovery(struct ufs_hba 
>> *hba)
>>  	ufshcd_set_eh_in_progress(hba);
>>  	spin_unlock_irqrestore(hba->host->host_lock, flags);
>> 
>> +	ret = ufshcd_vops_full_reset(hba);
>> +	if (ret)
>> +		dev_warn(hba->dev, "%s: full reset returned %d\n",
>> +				  __func__, ret);
>> +
>> +	/* Reset the attached device */
>> +	ufshcd_vops_device_reset(hba);
>> +
>>  	ret = ufshcd_host_reset_and_restore(hba);
>> 
>>  	spin_lock_irqsave(hba->host->host_lock, flags); @@ -6241,6 +6249,11
>> @@ static int ufshcd_reset_and_restore(struct ufs_hba *hba)
>>  	int retries = MAX_HOST_RESET_RETRIES;
>> 
>>  	do {
>> +		err = ufshcd_vops_full_reset(hba);
>> +		if (err)
>> +			dev_warn(hba->dev, "%s: full reset returned %d\n",
>> +					__func__, err);
>> +
>>  		/* Reset the attached device */
>>  		ufshcd_vops_device_reset(hba);
>> 
>> @@ -8384,6 +8397,9 @@ int ufshcd_init(struct ufs_hba *hba, void 
>> __iomem
>> *mmio_base, unsigned int irq)
>>  		goto exit_gating;
>>  	}
>> 
>> +	/* Reset controller to power on reset (POR) state */
>> +	ufshcd_vops_full_reset(hba);
>> +
>>  	/* Reset the attached device */
>>  	ufshcd_vops_device_reset(hba);
>> 
>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h 
>> index
>> e0fe247..253b9ea 100644
>> --- a/drivers/scsi/ufs/ufshcd.h
>> +++ b/drivers/scsi/ufs/ufshcd.h
>> @@ -296,6 +296,8 @@ struct ufs_pwr_mode_info {
>>   * @apply_dev_quirks: called to apply device specific quirks
>>   * @suspend: called during host controller PM callback
>>   * @resume: called during host controller PM callback
>> + * @full_reset: called for handling variant specific implementations 
>> of
>> + *              resetting the hci
>>   * @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 @@ 
>> -325,6
>> +327,7 @@ struct ufs_hba_variant_ops {
>>  	int	(*apply_dev_quirks)(struct ufs_hba *);
>>  	int     (*suspend)(struct ufs_hba *, enum ufs_pm_op);
>>  	int     (*resume)(struct ufs_hba *, enum ufs_pm_op);
>> +	int	(*full_reset)(struct ufs_hba *hba);
>>  	void	(*dbg_register_dump)(struct ufs_hba *hba);
>>  	int	(*phy_initialization)(struct ufs_hba *);
>>  	void	(*device_reset)(struct ufs_hba *hba);
>> @@ -1076,6 +1079,13 @@ static inline int ufshcd_vops_resume(struct 
>> ufs_hba
>> *hba, enum ufs_pm_op op)
>>  	return 0;
>>  }
>> 
>> +static inline int ufshcd_vops_full_reset(struct ufs_hba *hba) {
>> +	if (hba->vops && hba->vops->full_reset)
>> +		return hba->vops->full_reset(hba);
>> +	return 0;
>> +}
>> +
>>  static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba) 
>>  {
>>  	if (hba->vops && hba->vops->dbg_register_dump)
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> Forum,
>> a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/2] scsi: ufs: Introduce a vops for resetting host controller
  2019-10-23  4:13 ` [PATCH v1 1/2] scsi: ufs: " Can Guo
  2019-10-23 10:39   ` [EXT] " Bean Huo (beanhuo)
@ 2019-10-31 14:44   ` Mark Salyzyn
  2019-11-01  1:18     ` cang
  2019-11-04 14:28   ` Avri Altman
  2 siblings, 1 reply; 11+ messages in thread
From: Mark Salyzyn @ 2019-10-31 14:44 UTC (permalink / raw)
  To: Can Guo, asutoshd, nguyenb, rnayak, linux-scsi, kernel-team,
	saravanak, salyzyn
  Cc: Alim Akhtar, Avri Altman, Pedro Sousa, James E.J. Bottomley,
	Martin K. Petersen, Stanley Chu, Bean Huo, Tomas Winkler,
	Subhash Jadavani, Arnd Bergmann, Bjorn Andersson, open list

On 10/22/19 9:13 PM, Can Guo wrote:
> Some UFS host controllers need their specific implementations of resetting
> to get them into a good state. Provide a new vops to allow the platform
> driver to implement this own reset operation.
>
> Signed-off-by: Can Guo <cang@codeaurora.org>
> ---
>   drivers/scsi/ufs/ufshcd.c | 16 ++++++++++++++++
>   drivers/scsi/ufs/ufshcd.h | 10 ++++++++++
>   2 files changed, 26 insertions(+)
>
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index c28c144..161e3c4 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -3859,6 +3859,14 @@ static int ufshcd_link_recovery(struct ufs_hba *hba)
>   	ufshcd_set_eh_in_progress(hba);
>   	spin_unlock_irqrestore(hba->host->host_lock, flags);
>   
> +	ret = ufshcd_vops_full_reset(hba);
> +	if (ret)
> +		dev_warn(hba->dev, "%s: full reset returned %d\n",
> +				  __func__, ret);
> +
> +	/* Reset the attached device */
> +	ufshcd_vops_device_reset(hba);
> +
>   	ret = ufshcd_host_reset_and_restore(hba);
>   
>   	spin_lock_irqsave(hba->host->host_lock, flags);

In all your cases, especially after this adjustment, 
ufshcd_vops_full_reset is called blindly (+error checking message) 
before ufshcd_vops_device_reset. What about dropping the .full_reset 
(should really have been called .hw_reset or .host_reset) addition to 
the vops, just adding ufshcd_vops_device_reset call here before 
ufshcd_host_reset_and_restore, and in the driver folding the 
ufshcd_vops_full_reset code into the .device_reset handler?

Would that be workable? It would be simpler if so.

I can see a desire for the heads up 
(ufshcd_vops_full_reset+)ufshcd_vops_device_reset calls before 
ufshcd_host_reset_and_restore because that function will spin 10 seconds 
waiting for a response from a standardized register, that itself could 
be hardware locked up requiring product specific reset procedures. But 
if that is the case, then what about all the other calls to 
ufshcd_host_reset_and_restore in this file that are not provided the 
heads up? My guess is that the host device only demonstrated issues in 
the ufshcd_link_recovery handling path? Are you sure this is the only 
path that tickles the controller into a hardware lockup state?

Sincerely -- Mark Salyzyn


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/2] scsi: ufs: Introduce a vops for resetting host controller
  2019-10-31 14:44   ` Mark Salyzyn
@ 2019-11-01  1:18     ` cang
  0 siblings, 0 replies; 11+ messages in thread
From: cang @ 2019-11-01  1:18 UTC (permalink / raw)
  To: Mark Salyzyn
  Cc: asutoshd, nguyenb, rnayak, linux-scsi, kernel-team, saravanak,
	salyzyn, Alim Akhtar, Avri Altman, Pedro Sousa,
	James E.J. Bottomley, Martin K. Petersen, Stanley Chu, Bean Huo,
	Tomas Winkler, Subhash Jadavani, Arnd Bergmann, Bjorn Andersson,
	open list

On 2019-10-31 22:44, Mark Salyzyn wrote:
> On 10/22/19 9:13 PM, Can Guo wrote:
>> Some UFS host controllers need their specific implementations of 
>> resetting
>> to get them into a good state. Provide a new vops to allow the 
>> platform
>> driver to implement this own reset operation.
>> 
>> Signed-off-by: Can Guo <cang@codeaurora.org>
>> ---
>>   drivers/scsi/ufs/ufshcd.c | 16 ++++++++++++++++
>>   drivers/scsi/ufs/ufshcd.h | 10 ++++++++++
>>   2 files changed, 26 insertions(+)
>> 
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index c28c144..161e3c4 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -3859,6 +3859,14 @@ static int ufshcd_link_recovery(struct ufs_hba 
>> *hba)
>>   	ufshcd_set_eh_in_progress(hba);
>>   	spin_unlock_irqrestore(hba->host->host_lock, flags);
>>   +	ret = ufshcd_vops_full_reset(hba);
>> +	if (ret)
>> +		dev_warn(hba->dev, "%s: full reset returned %d\n",
>> +				  __func__, ret);
>> +
>> +	/* Reset the attached device */
>> +	ufshcd_vops_device_reset(hba);
>> +
>>   	ret = ufshcd_host_reset_and_restore(hba);
>>     	spin_lock_irqsave(hba->host->host_lock, flags);
> 
> In all your cases, especially after this adjustment,
> ufshcd_vops_full_reset is called blindly (+error checking message)
> before ufshcd_vops_device_reset. What about dropping the .full_reset
> (should really have been called .hw_reset or .host_reset) addition to
> the vops, just adding ufshcd_vops_device_reset call here before
> ufshcd_host_reset_and_restore, and in the driver folding the
> ufshcd_vops_full_reset code into the .device_reset handler?
> 
> Would that be workable? It would be simpler if so.
> 
> I can see a desire for the heads up
> (ufshcd_vops_full_reset+)ufshcd_vops_device_reset calls before
> ufshcd_host_reset_and_restore because that function will spin 10
> seconds waiting for a response from a standardized register, that
> itself could be hardware locked up requiring product specific reset
> procedures. But if that is the case, then what about all the other
> calls to ufshcd_host_reset_and_restore in this file that are not
> provided the heads up? My guess is that the host device only
> demonstrated issues in the ufshcd_link_recovery handling path? Are you
> sure this is the only path that tickles the controller into a hardware
> lockup state?
> 
> Sincerely -- Mark Salyzyn

Hi Mark Salyzyn,

Folding the "full_reset" vops inito "device_reset" vops is one choice 
for now. Shall do that.
Your guess is correct. the head up is needed in ufshcd_link_recovery() 
path because
link is already in bad state when we are here, expeically after hibern8 
exit fails.
So we need a full reset to PHY and host controller here before 
host_reset_and_restore.
But other calls to host_reset_and_restore are under good conditions.

Regards,
Can Guo.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v1 1/2] scsi: ufs: Introduce a vops for resetting host controller
  2019-10-23  4:13 ` [PATCH v1 1/2] scsi: ufs: " Can Guo
  2019-10-23 10:39   ` [EXT] " Bean Huo (beanhuo)
  2019-10-31 14:44   ` Mark Salyzyn
@ 2019-11-04 14:28   ` Avri Altman
  2019-11-04 14:34     ` [EXT] " Bean Huo (beanhuo)
  2019-11-04 23:44     ` cang
  2 siblings, 2 replies; 11+ messages in thread
From: Avri Altman @ 2019-11-04 14:28 UTC (permalink / raw)
  To: Can Guo, asutoshd, nguyenb, rnayak, linux-scsi, kernel-team,
	saravanak, salyzyn
  Cc: Alim Akhtar, Pedro Sousa, James E.J. Bottomley,
	Martin K. Petersen, Stanley Chu, Bean Huo, Tomas Winkler,
	Subhash Jadavani, Arnd Bergmann, Bjorn Andersson, open list

Hi, 
> 
> Some UFS host controllers need their specific implementations of resetting to
> get them into a good state. Provide a new vops to allow the platform driver to
> implement this own reset operation.
> 
> Signed-off-by: Can Guo <cang@codeaurora.org>
Did you withdraw from this patches and insert them to one of your fix bundle?
I couldn't tell.
As this is a vop, in what way its functionality can't be included in the device reset that was recently added?

Thanks,
Avri

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [EXT] RE: [PATCH v1 1/2] scsi: ufs: Introduce a vops for resetting host controller
  2019-11-04 14:28   ` Avri Altman
@ 2019-11-04 14:34     ` Bean Huo (beanhuo)
  2019-11-04 23:46       ` cang
  2019-11-04 23:44     ` cang
  1 sibling, 1 reply; 11+ messages in thread
From: Bean Huo (beanhuo) @ 2019-11-04 14:34 UTC (permalink / raw)
  To: Avri Altman, Can Guo, asutoshd, nguyenb, rnayak, linux-scsi,
	kernel-team, saravanak, salyzyn
  Cc: Alim Akhtar, Pedro Sousa, James E.J. Bottomley,
	Martin K. Petersen, Stanley Chu, Tomas Winkler, Subhash Jadavani,
	Arnd Bergmann, Bjorn Andersson, open list

Hi, Can

I agree with Avri, you can add this series patches to your bundle, and that is also easy to review for us.
Otherwise, we are confused by somehow crossing different series patches. 
Thanks,

//Bean
> 
> Hi,
> >
> > Some UFS host controllers need their specific implementations of
> > resetting to get them into a good state. Provide a new vops to allow
> > the platform driver to implement this own reset operation.
> >
> > Signed-off-by: Can Guo <cang@codeaurora.org>
> Did you withdraw from this patches and insert them to one of your fix bundle?
> I couldn't tell.
> As this is a vop, in what way its functionality can't be included in the device reset
> that was recently added?
> 
> Thanks,
> Avri

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v1 1/2] scsi: ufs: Introduce a vops for resetting host controller
  2019-11-04 14:28   ` Avri Altman
  2019-11-04 14:34     ` [EXT] " Bean Huo (beanhuo)
@ 2019-11-04 23:44     ` cang
  1 sibling, 0 replies; 11+ messages in thread
From: cang @ 2019-11-04 23:44 UTC (permalink / raw)
  To: Avri Altman
  Cc: asutoshd, nguyenb, rnayak, linux-scsi, kernel-team, saravanak,
	salyzyn, Alim Akhtar, Pedro Sousa, James E.J. Bottomley,
	Martin K. Petersen, Stanley Chu, Bean Huo, Tomas Winkler,
	Subhash Jadavani, Arnd Bergmann, Bjorn Andersson, open list

On 2019-11-04 22:28, Avri Altman wrote:
> Hi,
>> 
>> Some UFS host controllers need their specific implementations of 
>> resetting to
>> get them into a good state. Provide a new vops to allow the platform 
>> driver to
>> implement this own reset operation.
>> 
>> Signed-off-by: Can Guo <cang@codeaurora.org>
> Did you withdraw from this patches and insert them to one of your fix 
> bundle?
> I couldn't tell.
> As this is a vop, in what way its functionality can't be included in
> the device reset that was recently added?
> 
> Thanks,
> Avri

Hi Avri,

Sorry for making you confused.
Yes, I dropped this series because it cannot fulfil its purpose anymore. 
I come up with a way which puts the reset in the right place in UFS QCOM 
platfrom driver without an extra vops, so I inserted the two changes in 
fix bundle 3.

Thanks,
Can Guo.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [EXT] RE: [PATCH v1 1/2] scsi: ufs: Introduce a vops for resetting host controller
  2019-11-04 14:34     ` [EXT] " Bean Huo (beanhuo)
@ 2019-11-04 23:46       ` cang
  0 siblings, 0 replies; 11+ messages in thread
From: cang @ 2019-11-04 23:46 UTC (permalink / raw)
  To: Bean Huo (beanhuo)
  Cc: Avri Altman, asutoshd, nguyenb, rnayak, linux-scsi, kernel-team,
	saravanak, salyzyn, Alim Akhtar, Pedro Sousa,
	James E.J. Bottomley, Martin K. Petersen, Stanley Chu,
	Tomas Winkler, Subhash Jadavani, Arnd Bergmann, Bjorn Andersson,
	open list

On 2019-11-04 22:34, Bean Huo (beanhuo) wrote:
> Hi, Can
> 
> I agree with Avri, you can add this series patches to your bundle, and
> that is also easy to review for us.
> Otherwise, we are confused by somehow crossing different series 
> patches.
> Thanks,
> 
> //Bean

Hi Bean,

I moved the two changes to fix bundle 3. The vops is not needed anymore.
Meanwhile I find it inappropriate to put the host controller reset 
inside
the device reset vops. So you can check the new changes in fix bundle 3.

Regards,
Can Guo.

>> 
>> Hi,
>> >
>> > Some UFS host controllers need their specific implementations of
>> > resetting to get them into a good state. Provide a new vops to allow
>> > the platform driver to implement this own reset operation.
>> >
>> > Signed-off-by: Can Guo <cang@codeaurora.org>
>> Did you withdraw from this patches and insert them to one of your fix 
>> bundle?
>> I couldn't tell.
>> As this is a vop, in what way its functionality can't be included in 
>> the device reset
>> that was recently added?
>> 
>> Thanks,
>> Avri

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-11-04 23:47 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-23  4:13 [PATCH v1 0/2] Introduce a vops for resetting host controller Can Guo
2019-10-23  4:13 ` [PATCH v1 1/2] scsi: ufs: " Can Guo
2019-10-23 10:39   ` [EXT] " Bean Huo (beanhuo)
2019-10-29  2:11     ` cang
2019-10-31 14:44   ` Mark Salyzyn
2019-11-01  1:18     ` cang
2019-11-04 14:28   ` Avri Altman
2019-11-04 14:34     ` [EXT] " Bean Huo (beanhuo)
2019-11-04 23:46       ` cang
2019-11-04 23:44     ` cang
2019-10-23  4:13 ` [PATCH v1 2/2] scsi: ufs-qcom: Add reset control support for " Can Guo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).