Re: [PATCH 2/3] scsi: ufs: Allow resetting the UFS device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[ Shuffling the recipients list ]

On 04/06/2019 09:20, Bjorn Andersson wrote:

> Acquire the device-reset GPIO and toggle this to reset the UFS device
> during initialization and host reset.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
> ---
>  drivers/scsi/ufs/ufshcd.c | 44 +++++++++++++++++++++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd.h |  4 ++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 8c1c551f2b42..951a0efee536 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -42,6 +42,7 @@
>  #include <linux/nls.h>
>  #include <linux/of.h>
>  #include <linux/bitfield.h>
> +#include <linux/gpio/consumer.h>
>  #include "ufshcd.h"
>  #include "ufs_quirks.h"
>  #include "unipro.h"
> @@ -6104,6 +6105,25 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
>  	return err;
>  }
>  
> +/**
> + ufshcd_device_reset() - toggle the (optional) device reset line
> + * @hba: per-adapter instance
> + *
> + * Toggles the (optional) reset line to reset the attached device.
> + */
> +static void ufshcd_device_reset(struct ufs_hba *hba)
> +{
> +	/*
> +	 * The USB device shall detect reset pulses of 1us, sleep for 10us to
> +	 * be on the safe side.
> +	 */
> +	gpiod_set_value_cansleep(hba->device_reset, 1);
> +	usleep_range(10, 15);
> +
> +	gpiod_set_value_cansleep(hba->device_reset, 0);
> +	usleep_range(10, 15);
> +}
> +
>  /**
>   * ufshcd_host_reset_and_restore - reset and restore host controller
>   * @hba: per-adapter instance
> @@ -6159,6 +6179,9 @@ static int ufshcd_reset_and_restore(struct ufs_hba *hba)
>  	int retries = MAX_HOST_RESET_RETRIES;
>  
>  	do {
> +		/* Reset the attached device */
> +		ufshcd_device_reset(hba);
> +
>  		err = ufshcd_host_reset_and_restore(hba);
>  	} while (err && --retries);
>  
> @@ -7355,6 +7378,18 @@ static void ufshcd_variant_hba_exit(struct ufs_hba *hba)
>  	ufshcd_vops_exit(hba);
>  }
>  
> +static int ufshcd_init_device_reset(struct ufs_hba *hba)
> +{
> +	hba->device_reset = devm_gpiod_get_optional(hba->dev, "device-reset",
> +						    GPIOD_OUT_HIGH);
> +	if (IS_ERR(hba->device_reset)) {
> +		dev_err(hba->dev, "failed to acquire reset gpio: %ld\n",
> +			PTR_ERR(hba->device_reset));
> +	}
> +
> +	return PTR_ERR_OR_ZERO(hba->device_reset);
> +}
> +
>  static int ufshcd_hba_init(struct ufs_hba *hba)
>  {
>  	int err;
> @@ -7394,9 +7429,15 @@ static int ufshcd_hba_init(struct ufs_hba *hba)
>  	if (err)
>  		goto out_disable_vreg;
>  
> +	err = ufshcd_init_device_reset(hba);
> +	if (err)
> +		goto out_disable_variant;
> +
>  	hba->is_powered = true;
>  	goto out;
>  
> +out_disable_variant:
> +	ufshcd_vops_setup_regulators(hba, false);
>  out_disable_vreg:
>  	ufshcd_setup_vreg(hba, false);
>  out_disable_clks:
> @@ -8290,6 +8331,9 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
>  		goto exit_gating;
>  	}
>  
> +	/* Reset the attached device */
> +	ufshcd_device_reset(hba);
> +
>  	/* Host controller enable */
>  	err = ufshcd_hba_enable(hba);
>  	if (err) {
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index ecfa898b9ccc..d8be67742168 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -72,6 +72,8 @@
>  #define UFSHCD "ufshcd"
>  #define UFSHCD_DRIVER_VERSION "0.2"
>  
> +struct gpio_desc;
> +
>  struct ufs_hba;
>  
>  enum dev_cmd_type {
> @@ -706,6 +708,8 @@ struct ufs_hba {
>  
>  	struct device		bsg_dev;
>  	struct request_queue	*bsg_queue;
> +
> +	struct gpio_desc *device_reset;
>  };
>  
>  /* Returns true if clocks can be gated. Otherwise false */
> 

Why is this needed on 845 and not on 8998?

On 8998 we already have:

			resets = <&gcc GCC_UFS_BCR>;
			reset-names = "rst";

The above reset line gets wiggled/frobbed when appropriate.

(What's the difference between gpio and pinctrl? vs a reset "clock" as above)

ufshcd_device_reset_ctrl() vs ufshcd_init_device_reset()

Sounds like the nomenclature could be unified or clarified.

Regards.



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux