[PATCH V3 1/3] scsi: ufs: set the device reference clock setting

[PATCH V3 1/3] scsi: ufs: set the device reference clock setting

[Sayali Lokhande]

From: Subhash Jadavani <subhashj@codeaurora.org>

UFS host supplies the reference clock to UFS device and UFS device
specification allows host to provide one of the 4 frequencies (19.2 MHz,
26 MHz, 38.4 MHz, 52 MHz) for reference clock. Host should set the
device reference clock frequency setting in the device based on what
frequency it is supplying to UFS device.

Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
---
 .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      |  7 +++
 drivers/scsi/ufs/ufs.h                             |  9 ++++
 drivers/scsi/ufs/ufshcd-pltfrm.c                   | 24 ++++++++++
 drivers/scsi/ufs/ufshcd.c                          | 52 ++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd.h                          |  1 +
 5 files changed, 93 insertions(+)

diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
index c39dfef..4522434 100644
--- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
+++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
@@ -41,6 +41,12 @@ Optional properties:
 -lanes-per-direction	: number of lanes available per direction - either 1 or 2.
 			  Note that it is assume same number of lanes is used both
 			  directions at once. If not specified, default is 2 lanes per direction.
+- dev-ref-clk-freq	: Specify the device reference clock frequency, must be one of the following:
+			  0: 19.2 MHz
+			  1: 26 MHz
+			  2: 38.4 MHz
+			  3: 52 MHz
+			  Defaults to 26 MHz if not specified.
 
 Note: If above properties are not defined it can be assumed that the supply
 regulators or clocks are always on.
@@ -66,4 +72,5 @@ Example:
 		freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
 		phys = <&ufsphy1>;
 		phy-names = "ufsphy";
+		dev-ref-clk-freq = <0>; /* reference clock freq: 19.2 MHz */
 	};
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 14e5bf7..e15deb0 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -378,6 +378,15 @@ enum query_opcode {
 	UPIU_QUERY_OPCODE_TOGGLE_FLAG	= 0x8,
 };
 
+/* bRefClkFreq attribute values */
+enum ref_clk_freq {
+	REF_CLK_FREQ_19_2_MHZ	= 0x0,
+	REF_CLK_FREQ_26_MHZ	= 0x1,
+	REF_CLK_FREQ_38_4_MHZ	= 0x2,
+	REF_CLK_FREQ_52_MHZ	= 0x3,
+	REF_CLK_FREQ_MAX	= REF_CLK_FREQ_52_MHZ,
+};
+
 /* Query response result code */
 enum {
 	QUERY_RESULT_SUCCESS                    = 0x00,
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index e82bde0..6c877f3 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -221,6 +221,28 @@ static int ufshcd_parse_regulator_info(struct ufs_hba *hba)
 	return err;
 }
 
+static void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba)
+{
+	struct device *dev = hba->dev;
+	struct device_node *np = dev->of_node;
+	int ret;
+
+	if (!np)
+		return;
+
+	ret = of_property_read_u32(np, "dev-ref-clk-freq",
+				   &hba->dev_ref_clk_freq);
+	if (ret ||
+	    (hba->dev_ref_clk_freq < 0) ||
+	    (hba->dev_ref_clk_freq > REF_CLK_FREQ_52_MHZ)) {
+		dev_err(hba->dev,
+		"%s: invalid ref_clk setting = %d, set to default\n",
+		__func__, hba->dev_ref_clk_freq);
+		/* default setting */
+		hba->dev_ref_clk_freq = REF_CLK_FREQ_26_MHZ;
+	}
+}
+
 #ifdef CONFIG_PM
 /**
  * ufshcd_pltfrm_suspend - suspend power management function
@@ -343,6 +365,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
 
+	ufshcd_parse_dev_ref_clk_freq(hba);
+
 	ufshcd_init_lanes_per_dir(hba);
 
 	err = ufshcd_init(hba, mmio_base, irq);
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c5b1bf1..4abc7ae 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6297,6 +6297,53 @@ static void ufshcd_def_desc_sizes(struct ufs_hba *hba)
 }
 
 /**
+ * ufshcd_set_dev_ref_clk - set the device bRefClkFreq
+ * @hba: per-adapter instance
+ *
+ * Read the current value of the bRefClkFreq attribute from device and update it
+ * if host is supplying different reference clock frequency than one mentioned
+ * in bRefClkFreq attribute.
+ *
+ * Returns zero on success, non-zero error value on failure.
+ */
+static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
+{
+	int err = 0;
+	int ref_clk = -1;
+	static const char * const ref_clk_freqs[] = {"19.2 MHz", "26 MHz",
+						     "38.4 MHz", "52 MHz"};
+
+	err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
+			QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0, &ref_clk);
+
+	if (err) {
+		dev_err(hba->dev, "%s: failed reading bRefClkFreq. err = %d\n",
+			 __func__, err);
+		goto out;
+	}
+
+	if (ref_clk == hba->dev_ref_clk_freq)
+		goto out; /* nothing to update */
+
+	err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
+			QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0,
+			&hba->dev_ref_clk_freq);
+
+	if (err)
+		dev_err(hba->dev, "%s: bRefClkFreq setting to %s failed\n",
+			__func__, ref_clk_freqs[hba->dev_ref_clk_freq]);
+	/*
+	 * It is good to print this out here to debug any later failures
+	 * related to gear switch.
+	 */
+	dev_dbg(hba->dev, "%s: bRefClkFreq setting to %s succeeded\n",
+			__func__, ref_clk_freqs[hba->dev_ref_clk_freq]);
+
+out:
+	return err;
+}
+
+/**
  * ufshcd_probe_hba - probe hba to detect device and initialize
  * @hba: per-adapter instance
  *
@@ -6361,6 +6408,11 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
 			"%s: Failed getting max supported power mode\n",
 			__func__);
 	} else {
+		/*
+		 * Set the right value to bRefClkFreq before attempting to
+		 * switch to HS gears.
+		 */
+		ufshcd_set_dev_ref_clk(hba);
 		ret = ufshcd_config_pwr_mode(hba, &hba->max_pwr_info.info);
 		if (ret) {
 			dev_err(hba->dev, "%s: Failed setting power mode, err = %d\n",
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 8110dcd..b026ad8 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -548,6 +548,7 @@ struct ufs_hba {
 	void *priv;
 	unsigned int irq;
 	bool is_irq_enabled;
+	u32 dev_ref_clk_freq;
 
 	/* Interrupt aggregation support is broken */
 	#define UFSHCD_QUIRK_BROKEN_INTR_AGGR			0x1
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH V3 1/3] scsi: ufs: set the device reference clock setting

[Sayali Lokhande]

From: Subhash Jadavani <subhashj@codeaurora.org>

UFS host supplies the reference clock to UFS device and UFS device
specification allows host to provide one of the 4 frequencies (19.2 MHz,
26 MHz, 38.4 MHz, 52 MHz) for reference clock. Host should set the
device reference clock frequency setting in the device based on what
frequency it is supplying to UFS device.

Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Can Guo <cang@codeaurora.org>
Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
---
 .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      |  7 +++
 drivers/scsi/ufs/ufs.h                             |  9 ++++
 drivers/scsi/ufs/ufshcd-pltfrm.c                   | 24 ++++++++++
 drivers/scsi/ufs/ufshcd.c                          | 52 ++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd.h                          |  1 +
 5 files changed, 93 insertions(+)

diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
index c39dfef..4522434 100644
--- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
+++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
@@ -41,6 +41,12 @@ Optional properties:
 -lanes-per-direction	: number of lanes available per direction - either 1 or 2.
 			  Note that it is assume same number of lanes is used both
 			  directions at once. If not specified, default is 2 lanes per direction.
+- dev-ref-clk-freq	: Specify the device reference clock frequency, must be one of the following:
+			  0: 19.2 MHz
+			  1: 26 MHz
+			  2: 38.4 MHz
+			  3: 52 MHz
+			  Defaults to 26 MHz if not specified.
 
 Note: If above properties are not defined it can be assumed that the supply
 regulators or clocks are always on.
@@ -66,4 +72,5 @@ Example:
 		freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
 		phys = <&ufsphy1>;
 		phy-names = "ufsphy";
+		dev-ref-clk-freq = <0>; /* reference clock freq: 19.2 MHz */
 	};
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 14e5bf7..e15deb0 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -378,6 +378,15 @@ enum query_opcode {
 	UPIU_QUERY_OPCODE_TOGGLE_FLAG	= 0x8,
 };
 
+/* bRefClkFreq attribute values */
+enum ref_clk_freq {
+	REF_CLK_FREQ_19_2_MHZ	= 0x0,
+	REF_CLK_FREQ_26_MHZ	= 0x1,
+	REF_CLK_FREQ_38_4_MHZ	= 0x2,
+	REF_CLK_FREQ_52_MHZ	= 0x3,
+	REF_CLK_FREQ_MAX	= REF_CLK_FREQ_52_MHZ,
+};
+
 /* Query response result code */
 enum {
 	QUERY_RESULT_SUCCESS                    = 0x00,
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index e82bde0..6c877f3 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -221,6 +221,28 @@ static int ufshcd_parse_regulator_info(struct ufs_hba *hba)
 	return err;
 }
 
+static void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba)
+{
+	struct device *dev = hba->dev;
+	struct device_node *np = dev->of_node;
+	int ret;
+
+	if (!np)
+		return;
+
+	ret = of_property_read_u32(np, "dev-ref-clk-freq",
+				   &hba->dev_ref_clk_freq);
+	if (ret ||
+	    (hba->dev_ref_clk_freq < 0) ||
+	    (hba->dev_ref_clk_freq > REF_CLK_FREQ_52_MHZ)) {
+		dev_err(hba->dev,
+		"%s: invalid ref_clk setting = %d, set to default\n",
+		__func__, hba->dev_ref_clk_freq);
+		/* default setting */
+		hba->dev_ref_clk_freq = REF_CLK_FREQ_26_MHZ;
+	}
+}
+
 #ifdef CONFIG_PM
 /**
  * ufshcd_pltfrm_suspend - suspend power management function
@@ -343,6 +365,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
 
+	ufshcd_parse_dev_ref_clk_freq(hba);
+
 	ufshcd_init_lanes_per_dir(hba);
 
 	err = ufshcd_init(hba, mmio_base, irq);
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c5b1bf1..4abc7ae 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6297,6 +6297,53 @@ static void ufshcd_def_desc_sizes(struct ufs_hba *hba)
 }
 
 /**
+ * ufshcd_set_dev_ref_clk - set the device bRefClkFreq
+ * @hba: per-adapter instance
+ *
+ * Read the current value of the bRefClkFreq attribute from device and update it
+ * if host is supplying different reference clock frequency than one mentioned
+ * in bRefClkFreq attribute.
+ *
+ * Returns zero on success, non-zero error value on failure.
+ */
+static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
+{
+	int err = 0;
+	int ref_clk = -1;
+	static const char * const ref_clk_freqs[] = {"19.2 MHz", "26 MHz",
+						     "38.4 MHz", "52 MHz"};
+
+	err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
+			QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0, &ref_clk);
+
+	if (err) {
+		dev_err(hba->dev, "%s: failed reading bRefClkFreq. err = %d\n",
+			 __func__, err);
+		goto out;
+	}
+
+	if (ref_clk == hba->dev_ref_clk_freq)
+		goto out; /* nothing to update */
+
+	err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
+			QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0,
+			&hba->dev_ref_clk_freq);
+
+	if (err)
+		dev_err(hba->dev, "%s: bRefClkFreq setting to %s failed\n",
+			__func__, ref_clk_freqs[hba->dev_ref_clk_freq]);
+	/*
+	 * It is good to print this out here to debug any later failures
+	 * related to gear switch.
+	 */
+	dev_dbg(hba->dev, "%s: bRefClkFreq setting to %s succeeded\n",
+			__func__, ref_clk_freqs[hba->dev_ref_clk_freq]);
+
+out:
+	return err;
+}
+
+/**
  * ufshcd_probe_hba - probe hba to detect device and initialize
  * @hba: per-adapter instance
  *
@@ -6361,6 +6408,11 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
 			"%s: Failed getting max supported power mode\n",
 			__func__);
 	} else {
+		/*
+		 * Set the right value to bRefClkFreq before attempting to
+		 * switch to HS gears.
+		 */
+		ufshcd_set_dev_ref_clk(hba);
 		ret = ufshcd_config_pwr_mode(hba, &hba->max_pwr_info.info);
 		if (ret) {
 			dev_err(hba->dev, "%s: Failed setting power mode, err = %d\n",
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 8110dcd..b026ad8 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -548,6 +548,7 @@ struct ufs_hba {
 	void *priv;
 	unsigned int irq;
 	bool is_irq_enabled;
+	u32 dev_ref_clk_freq;
 
 	/* Interrupt aggregation support is broken */
 	#define UFSHCD_QUIRK_BROKEN_INTR_AGGR			0x1
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH V3 1/3] scsi: ufs: set the device reference clock setting

[Adrian Hunter]

On 14/06/18 16:03, Sayali Lokhande wrote:
> From: Subhash Jadavani <subhashj@codeaurora.org>
> 
> UFS host supplies the reference clock to UFS device and UFS device
> specification allows host to provide one of the 4 frequencies (19.2 MHz,
> 26 MHz, 38.4 MHz, 52 MHz) for reference clock. Host should set the
> device reference clock frequency setting in the device based on what
> frequency it is supplying to UFS device.
> 
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>

I have repeated my V2 comments below.  Please address these when you post
V4.  Also please provide a change log for each patch version.

> ---
>  .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      |  7 +++
>  drivers/scsi/ufs/ufs.h                             |  9 ++++
>  drivers/scsi/ufs/ufshcd-pltfrm.c                   | 24 ++++++++++
>  drivers/scsi/ufs/ufshcd.c                          | 52 ++++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd.h                          |  1 +
>  5 files changed, 93 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> index c39dfef..4522434 100644
> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> @@ -41,6 +41,12 @@ Optional properties:
>  -lanes-per-direction	: number of lanes available per direction - either 1 or 2.
>  			  Note that it is assume same number of lanes is used both
>  			  directions at once. If not specified, default is 2 lanes per direction.
> +- dev-ref-clk-freq	: Specify the device reference clock frequency, must be one of the following:
> +			  0: 19.2 MHz
> +			  1: 26 MHz
> +			  2: 38.4 MHz
> +			  3: 52 MHz
> +			  Defaults to 26 MHz if not specified.
>  
>  Note: If above properties are not defined it can be assumed that the supply
>  regulators or clocks are always on.
> @@ -66,4 +72,5 @@ Example:
>  		freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
>  		phys = <&ufsphy1>;
>  		phy-names = "ufsphy";
> +		dev-ref-clk-freq = <0>; /* reference clock freq: 19.2 MHz */
>  	};
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index 14e5bf7..e15deb0 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -378,6 +378,15 @@ enum query_opcode {
>  	UPIU_QUERY_OPCODE_TOGGLE_FLAG	= 0x8,
>  };
>  
> +/* bRefClkFreq attribute values */
> +enum ref_clk_freq {
> +	REF_CLK_FREQ_19_2_MHZ	= 0x0,
> +	REF_CLK_FREQ_26_MHZ	= 0x1,
> +	REF_CLK_FREQ_38_4_MHZ	= 0x2,
> +	REF_CLK_FREQ_52_MHZ	= 0x3,
> +	REF_CLK_FREQ_MAX	= REF_CLK_FREQ_52_MHZ,
> +};
> +
>  /* Query response result code */
>  enum {
>  	QUERY_RESULT_SUCCESS                    = 0x00,
> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
> index e82bde0..6c877f3 100644
> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
> @@ -221,6 +221,28 @@ static int ufshcd_parse_regulator_info(struct ufs_hba *hba)
>  	return err;
>  }
>  
> +static void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba)
> +{
> +	struct device *dev = hba->dev;
> +	struct device_node *np = dev->of_node;
> +	int ret;
> +
> +	if (!np)
> +		return;
> +
> +	ret = of_property_read_u32(np, "dev-ref-clk-freq",
> +				   &hba->dev_ref_clk_freq);

This setting is useful for any UFSHC driver.  Please move it to ufshcd.c and
use device_property_read_u32().

> +	if (ret ||
> +	    (hba->dev_ref_clk_freq < 0) ||

u32 cannot be < 0

> +	    (hba->dev_ref_clk_freq > REF_CLK_FREQ_52_MHZ)) {
> +		dev_err(hba->dev,
> +		"%s: invalid ref_clk setting = %d, set to default\n",
> +		__func__, hba->dev_ref_clk_freq);
> +		/* default setting */
> +		hba->dev_ref_clk_freq = REF_CLK_FREQ_26_MHZ;

No, the default must be to leave the value unchanged.

> +	}
> +}
> +
>  #ifdef CONFIG_PM
>  /**
>   * ufshcd_pltfrm_suspend - suspend power management function
> @@ -343,6 +365,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
>  	pm_runtime_set_active(&pdev->dev);
>  	pm_runtime_enable(&pdev->dev);
>  
> +	ufshcd_parse_dev_ref_clk_freq(hba);
> +
>  	ufshcd_init_lanes_per_dir(hba);
>  
>  	err = ufshcd_init(hba, mmio_base, irq);
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index c5b1bf1..4abc7ae 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -6297,6 +6297,53 @@ static void ufshcd_def_desc_sizes(struct ufs_hba *hba)
>  }
>  
>  /**
> + * ufshcd_set_dev_ref_clk - set the device bRefClkFreq
> + * @hba: per-adapter instance
> + *
> + * Read the current value of the bRefClkFreq attribute from device and update it
> + * if host is supplying different reference clock frequency than one mentioned
> + * in bRefClkFreq attribute.
> + *
> + * Returns zero on success, non-zero error value on failure.
> + */
> +static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
> +{
> +	int err = 0;
> +	int ref_clk = -1;
> +	static const char * const ref_clk_freqs[] = {"19.2 MHz", "26 MHz",
> +						     "38.4 MHz", "52 MHz"};
> +
> +	err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> +			QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0, &ref_clk);
> +
> +	if (err) {
> +		dev_err(hba->dev, "%s: failed reading bRefClkFreq. err = %d\n",
> +			 __func__, err);
> +		goto out;
> +	}
> +
> +	if (ref_clk == hba->dev_ref_clk_freq)
> +		goto out; /* nothing to update */
> +
> +	err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
> +			QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0,
> +			&hba->dev_ref_clk_freq);
> +
> +	if (err)
> +		dev_err(hba->dev, "%s: bRefClkFreq setting to %s failed\n",
> +			__func__, ref_clk_freqs[hba->dev_ref_clk_freq]);
> +	/*
> +	 * It is good to print this out here to debug any later failures
> +	 * related to gear switch.
> +	 */
> +	dev_dbg(hba->dev, "%s: bRefClkFreq setting to %s succeeded\n",
> +			__func__, ref_clk_freqs[hba->dev_ref_clk_freq]);
> +
> +out:
> +	return err;
> +}
> +
> +/**
>   * ufshcd_probe_hba - probe hba to detect device and initialize
>   * @hba: per-adapter instance
>   *
> @@ -6361,6 +6408,11 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
>  			"%s: Failed getting max supported power mode\n",
>  			__func__);
>  	} else {
> +		/*
> +		 * Set the right value to bRefClkFreq before attempting to
> +		 * switch to HS gears.
> +		 */
> +		ufshcd_set_dev_ref_clk(hba);
>  		ret = ufshcd_config_pwr_mode(hba, &hba->max_pwr_info.info);
>  		if (ret) {
>  			dev_err(hba->dev, "%s: Failed setting power mode, err = %d\n",
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 8110dcd..b026ad8 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -548,6 +548,7 @@ struct ufs_hba {
>  	void *priv;
>  	unsigned int irq;
>  	bool is_irq_enabled;
> +	u32 dev_ref_clk_freq;
>  
>  	/* Interrupt aggregation support is broken */
>  	#define UFSHCD_QUIRK_BROKEN_INTR_AGGR			0x1
> 

Re: [PATCH V3 1/3] scsi: ufs: set the device reference clock setting

[Rob Herring]

On Wed, Jun 20, 2018 at 10:26:07AM +0300, Adrian Hunter wrote:
> On 14/06/18 16:03, Sayali Lokhande wrote:
> > From: Subhash Jadavani <subhashj@codeaurora.org>
> > 
> > UFS host supplies the reference clock to UFS device and UFS device
> > specification allows host to provide one of the 4 frequencies (19.2 MHz,
> > 26 MHz, 38.4 MHz, 52 MHz) for reference clock. Host should set the
> > device reference clock frequency setting in the device based on what
> > frequency it is supplying to UFS device.
> > 
> > Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> > Signed-off-by: Can Guo <cang@codeaurora.org>
> > Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
> 
> I have repeated my V2 comments below.  Please address these when you post
> V4.  Also please provide a change log for each patch version.

Do I have to repeat my V2 comments too? Please don't send a V4 until the 
discussion on V2 is resolved.

Rob

RE: [PATCH V3 1/3] scsi: ufs: set the device reference clock setting

[sayali]

Hi Adrian,

Please check my comment inline.

Thanks,
Sayali
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter@intel.com] 
Sent: Wednesday, June 20, 2018 12:56 PM
To: Sayali Lokhande <sayalil@codeaurora.org>; subhashj@codeaurora.org; cang@codeaurora.org; vivek.gautam@codeaurora.org; rnayak@codeaurora.org; vinholikatti@gmail.com; jejb@linux.vnet.ibm.com; martin.petersen@oracle.com; asutoshd@codeaurora.org; evgreen@chromium.org; riteshh@codeaurora.org
Cc: linux-scsi@vger.kernel.org; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Mathieu Malaterre <malat@debian.org>; open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH V3 1/3] scsi: ufs: set the device reference clock setting

On 14/06/18 16:03, Sayali Lokhande wrote:
> From: Subhash Jadavani <subhashj@codeaurora.org>
> 
> UFS host supplies the reference clock to UFS device and UFS device 
> specification allows host to provide one of the 4 frequencies (19.2 
> MHz,
> 26 MHz, 38.4 MHz, 52 MHz) for reference clock. Host should set the 
> device reference clock frequency setting in the device based on what 
> frequency it is supplying to UFS device.
> 
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>

I have repeated my V2 comments below.  Please address these when you post V4.  Also please provide a change log for each patch version.

> ---
>  .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      |  7 +++
>  drivers/scsi/ufs/ufs.h                             |  9 ++++
>  drivers/scsi/ufs/ufshcd-pltfrm.c                   | 24 ++++++++++
>  drivers/scsi/ufs/ufshcd.c                          | 52 ++++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd.h                          |  1 +
>  5 files changed, 93 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt 
> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> index c39dfef..4522434 100644
> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> @@ -41,6 +41,12 @@ Optional properties:
>  -lanes-per-direction	: number of lanes available per direction - either 1 or 2.
>  			  Note that it is assume same number of lanes is used both
>  			  directions at once. If not specified, default is 2 lanes per direction.
> +- dev-ref-clk-freq	: Specify the device reference clock frequency, must be one of the following:
> +			  0: 19.2 MHz
> +			  1: 26 MHz
> +			  2: 38.4 MHz
> +			  3: 52 MHz
> +			  Defaults to 26 MHz if not specified.
>  
>  Note: If above properties are not defined it can be assumed that the 
> supply  regulators or clocks are always on.
> @@ -66,4 +72,5 @@ Example:
>  		freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
>  		phys = <&ufsphy1>;
>  		phy-names = "ufsphy";
> +		dev-ref-clk-freq = <0>; /* reference clock freq: 19.2 MHz */
>  	};
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h index 
> 14e5bf7..e15deb0 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -378,6 +378,15 @@ enum query_opcode {
>  	UPIU_QUERY_OPCODE_TOGGLE_FLAG	= 0x8,
>  };
>  
> +/* bRefClkFreq attribute values */
> +enum ref_clk_freq {
> +	REF_CLK_FREQ_19_2_MHZ	= 0x0,
> +	REF_CLK_FREQ_26_MHZ	= 0x1,
> +	REF_CLK_FREQ_38_4_MHZ	= 0x2,
> +	REF_CLK_FREQ_52_MHZ	= 0x3,
> +	REF_CLK_FREQ_MAX	= REF_CLK_FREQ_52_MHZ,
> +};
> +
>  /* Query response result code */
>  enum {
>  	QUERY_RESULT_SUCCESS                    = 0x00,
> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c 
> b/drivers/scsi/ufs/ufshcd-pltfrm.c
> index e82bde0..6c877f3 100644
> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
> @@ -221,6 +221,28 @@ static int ufshcd_parse_regulator_info(struct ufs_hba *hba)
>  	return err;
>  }
>  
> +static void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba) {
> +	struct device *dev = hba->dev;
> +	struct device_node *np = dev->of_node;
> +	int ret;
> +
> +	if (!np)
> +		return;
> +
> +	ret = of_property_read_u32(np, "dev-ref-clk-freq",
> +				   &hba->dev_ref_clk_freq);

This setting is useful for any UFSHC driver.  Please move it to ufshcd.c and use device_property_read_u32().
[Sayali]: Ok. Will update this in next patch set.

> +	if (ret ||
> +	    (hba->dev_ref_clk_freq < 0) ||

u32 cannot be < 0
[Sayali]: Will remove this check.

> +	    (hba->dev_ref_clk_freq > REF_CLK_FREQ_52_MHZ)) {
> +		dev_err(hba->dev,
> +		"%s: invalid ref_clk setting = %d, set to default\n",
> +		__func__, hba->dev_ref_clk_freq);
> +		/* default setting */
> +		hba->dev_ref_clk_freq = REF_CLK_FREQ_26_MHZ;

No, the default must be to leave the value unchanged.
[Sayali]: As per UFS device specification , bRefClkFreq attribute's default value corresponds to 26 MHz (Manufacturer Default Value). Thus we should set it to default (26MHz) if it's not already set.

> +	}
> +}
> +
>  #ifdef CONFIG_PM
>  /**
>   * ufshcd_pltfrm_suspend - suspend power management function @@ 
> -343,6 +365,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
>  	pm_runtime_set_active(&pdev->dev);
>  	pm_runtime_enable(&pdev->dev);
>  
> +	ufshcd_parse_dev_ref_clk_freq(hba);
> +
>  	ufshcd_init_lanes_per_dir(hba);
>  
>  	err = ufshcd_init(hba, mmio_base, irq); diff --git 
> a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 
> c5b1bf1..4abc7ae 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -6297,6 +6297,53 @@ static void ufshcd_def_desc_sizes(struct 
> ufs_hba *hba)  }
>  
>  /**
> + * ufshcd_set_dev_ref_clk - set the device bRefClkFreq
> + * @hba: per-adapter instance
> + *
> + * Read the current value of the bRefClkFreq attribute from device 
> +and update it
> + * if host is supplying different reference clock frequency than one 
> +mentioned
> + * in bRefClkFreq attribute.
> + *
> + * Returns zero on success, non-zero error value on failure.
> + */
> +static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba) {
> +	int err = 0;
> +	int ref_clk = -1;
> +	static const char * const ref_clk_freqs[] = {"19.2 MHz", "26 MHz",
> +						     "38.4 MHz", "52 MHz"};
> +
> +	err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> +			QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0, &ref_clk);
> +
> +	if (err) {
> +		dev_err(hba->dev, "%s: failed reading bRefClkFreq. err = %d\n",
> +			 __func__, err);
> +		goto out;
> +	}
> +
> +	if (ref_clk == hba->dev_ref_clk_freq)
> +		goto out; /* nothing to update */
> +
> +	err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
> +			QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0,
> +			&hba->dev_ref_clk_freq);
> +
> +	if (err)
> +		dev_err(hba->dev, "%s: bRefClkFreq setting to %s failed\n",
> +			__func__, ref_clk_freqs[hba->dev_ref_clk_freq]);
> +	/*
> +	 * It is good to print this out here to debug any later failures
> +	 * related to gear switch.
> +	 */
> +	dev_dbg(hba->dev, "%s: bRefClkFreq setting to %s succeeded\n",
> +			__func__, ref_clk_freqs[hba->dev_ref_clk_freq]);
> +
> +out:
> +	return err;
> +}
> +
> +/**
>   * ufshcd_probe_hba - probe hba to detect device and initialize
>   * @hba: per-adapter instance
>   *
> @@ -6361,6 +6408,11 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
>  			"%s: Failed getting max supported power mode\n",
>  			__func__);
>  	} else {
> +		/*
> +		 * Set the right value to bRefClkFreq before attempting to
> +		 * switch to HS gears.
> +		 */
> +		ufshcd_set_dev_ref_clk(hba);
>  		ret = ufshcd_config_pwr_mode(hba, &hba->max_pwr_info.info);
>  		if (ret) {
>  			dev_err(hba->dev, "%s: Failed setting power mode, err = %d\n", 
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h 
> index 8110dcd..b026ad8 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -548,6 +548,7 @@ struct ufs_hba {
>  	void *priv;
>  	unsigned int irq;
>  	bool is_irq_enabled;
> +	u32 dev_ref_clk_freq;
>  
>  	/* Interrupt aggregation support is broken */
>  	#define UFSHCD_QUIRK_BROKEN_INTR_AGGR			0x1
> 

RE: [PATCH V3 1/3] scsi: ufs: set the device reference clock setting

[sayali]

Hi Adrian,

Please check my comment inline.

Thanks,
Sayali
-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter@intel.com] 
Sent: Wednesday, June 20, 2018 12:56 PM
To: Sayali Lokhande <sayalil@codeaurora.org>; subhashj@codeaurora.org; cang@codeaurora.org; vivek.gautam@codeaurora.org; rnayak@codeaurora.org; vinholikatti@gmail.com; jejb@linux.vnet.ibm.com; martin.petersen@oracle.com; asutoshd@codeaurora.org; evgreen@chromium.org; riteshh@codeaurora.org
Cc: linux-scsi@vger.kernel.org; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Mathieu Malaterre <malat@debian.org>; open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH V3 1/3] scsi: ufs: set the device reference clock setting

On 14/06/18 16:03, Sayali Lokhande wrote:
> From: Subhash Jadavani <subhashj@codeaurora.org>
> 
> UFS host supplies the reference clock to UFS device and UFS device 
> specification allows host to provide one of the 4 frequencies (19.2 
> MHz,
> 26 MHz, 38.4 MHz, 52 MHz) for reference clock. Host should set the 
> device reference clock frequency setting in the device based on what 
> frequency it is supplying to UFS device.
> 
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> Signed-off-by: Can Guo <cang@codeaurora.org>
> Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>

I have repeated my V2 comments below.  Please address these when you post V4.  Also please provide a change log for each patch version.

> ---
>  .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      |  7 +++
>  drivers/scsi/ufs/ufs.h                             |  9 ++++
>  drivers/scsi/ufs/ufshcd-pltfrm.c                   | 24 ++++++++++
>  drivers/scsi/ufs/ufshcd.c                          | 52 ++++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd.h                          |  1 +
>  5 files changed, 93 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt 
> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> index c39dfef..4522434 100644
> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> @@ -41,6 +41,12 @@ Optional properties:
>  -lanes-per-direction	: number of lanes available per direction - either 1 or 2.
>  			  Note that it is assume same number of lanes is used both
>  			  directions at once. If not specified, default is 2 lanes per direction.
> +- dev-ref-clk-freq	: Specify the device reference clock frequency, must be one of the following:
> +			  0: 19.2 MHz
> +			  1: 26 MHz
> +			  2: 38.4 MHz
> +			  3: 52 MHz
> +			  Defaults to 26 MHz if not specified.
>  
>  Note: If above properties are not defined it can be assumed that the 
> supply  regulators or clocks are always on.
> @@ -66,4 +72,5 @@ Example:
>  		freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
>  		phys = <&ufsphy1>;
>  		phy-names = "ufsphy";
> +		dev-ref-clk-freq = <0>; /* reference clock freq: 19.2 MHz */
>  	};
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h index 
> 14e5bf7..e15deb0 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -378,6 +378,15 @@ enum query_opcode {
>  	UPIU_QUERY_OPCODE_TOGGLE_FLAG	= 0x8,
>  };
>  
> +/* bRefClkFreq attribute values */
> +enum ref_clk_freq {
> +	REF_CLK_FREQ_19_2_MHZ	= 0x0,
> +	REF_CLK_FREQ_26_MHZ	= 0x1,
> +	REF_CLK_FREQ_38_4_MHZ	= 0x2,
> +	REF_CLK_FREQ_52_MHZ	= 0x3,
> +	REF_CLK_FREQ_MAX	= REF_CLK_FREQ_52_MHZ,
> +};
> +
>  /* Query response result code */
>  enum {
>  	QUERY_RESULT_SUCCESS                    = 0x00,
> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c 
> b/drivers/scsi/ufs/ufshcd-pltfrm.c
> index e82bde0..6c877f3 100644
> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
> @@ -221,6 +221,28 @@ static int ufshcd_parse_regulator_info(struct ufs_hba *hba)
>  	return err;
>  }
>  
> +static void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba) {
> +	struct device *dev = hba->dev;
> +	struct device_node *np = dev->of_node;
> +	int ret;
> +
> +	if (!np)
> +		return;
> +
> +	ret = of_property_read_u32(np, "dev-ref-clk-freq",
> +				   &hba->dev_ref_clk_freq);

This setting is useful for any UFSHC driver.  Please move it to ufshcd.c and use device_property_read_u32().
[Sayali]: Ok. Will update this in next patch set.

> +	if (ret ||
> +	    (hba->dev_ref_clk_freq < 0) ||

u32 cannot be < 0
[Sayali]: Will remove this check.

> +	    (hba->dev_ref_clk_freq > REF_CLK_FREQ_52_MHZ)) {
> +		dev_err(hba->dev,
> +		"%s: invalid ref_clk setting = %d, set to default\n",
> +		__func__, hba->dev_ref_clk_freq);
> +		/* default setting */
> +		hba->dev_ref_clk_freq = REF_CLK_FREQ_26_MHZ;

No, the default must be to leave the value unchanged.
[Sayali]: As per UFS device specification , bRefClkFreq attribute's default value corresponds to 26 MHz (Manufacturer Default Value). Thus we should set it to default (26MHz) if it's not already set.

> +	}
> +}
> +
>  #ifdef CONFIG_PM
>  /**
>   * ufshcd_pltfrm_suspend - suspend power management function @@ 
> -343,6 +365,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
>  	pm_runtime_set_active(&pdev->dev);
>  	pm_runtime_enable(&pdev->dev);
>  
> +	ufshcd_parse_dev_ref_clk_freq(hba);
> +
>  	ufshcd_init_lanes_per_dir(hba);
>  
>  	err = ufshcd_init(hba, mmio_base, irq); diff --git 
> a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 
> c5b1bf1..4abc7ae 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -6297,6 +6297,53 @@ static void ufshcd_def_desc_sizes(struct 
> ufs_hba *hba)  }
>  
>  /**
> + * ufshcd_set_dev_ref_clk - set the device bRefClkFreq
> + * @hba: per-adapter instance
> + *
> + * Read the current value of the bRefClkFreq attribute from device 
> +and update it
> + * if host is supplying different reference clock frequency than one 
> +mentioned
> + * in bRefClkFreq attribute.
> + *
> + * Returns zero on success, non-zero error value on failure.
> + */
> +static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba) {
> +	int err = 0;
> +	int ref_clk = -1;
> +	static const char * const ref_clk_freqs[] = {"19.2 MHz", "26 MHz",
> +						     "38.4 MHz", "52 MHz"};
> +
> +	err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
> +			QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0, &ref_clk);
> +
> +	if (err) {
> +		dev_err(hba->dev, "%s: failed reading bRefClkFreq. err = %d\n",
> +			 __func__, err);
> +		goto out;
> +	}
> +
> +	if (ref_clk == hba->dev_ref_clk_freq)
> +		goto out; /* nothing to update */
> +
> +	err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
> +			QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0,
> +			&hba->dev_ref_clk_freq);
> +
> +	if (err)
> +		dev_err(hba->dev, "%s: bRefClkFreq setting to %s failed\n",
> +			__func__, ref_clk_freqs[hba->dev_ref_clk_freq]);
> +	/*
> +	 * It is good to print this out here to debug any later failures
> +	 * related to gear switch.
> +	 */
> +	dev_dbg(hba->dev, "%s: bRefClkFreq setting to %s succeeded\n",
> +			__func__, ref_clk_freqs[hba->dev_ref_clk_freq]);
> +
> +out:
> +	return err;
> +}
> +
> +/**
>   * ufshcd_probe_hba - probe hba to detect device and initialize
>   * @hba: per-adapter instance
>   *
> @@ -6361,6 +6408,11 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
>  			"%s: Failed getting max supported power mode\n",
>  			__func__);
>  	} else {
> +		/*
> +		 * Set the right value to bRefClkFreq before attempting to
> +		 * switch to HS gears.
> +		 */
> +		ufshcd_set_dev_ref_clk(hba);
>  		ret = ufshcd_config_pwr_mode(hba, &hba->max_pwr_info.info);
>  		if (ret) {
>  			dev_err(hba->dev, "%s: Failed setting power mode, err = %d\n", 
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h 
> index 8110dcd..b026ad8 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -548,6 +548,7 @@ struct ufs_hba {
>  	void *priv;
>  	unsigned int irq;
>  	bool is_irq_enabled;
> +	u32 dev_ref_clk_freq;
>  
>  	/* Interrupt aggregation support is broken */
>  	#define UFSHCD_QUIRK_BROKEN_INTR_AGGR			0x1
> 

Re: [PATCH V3 1/3] scsi: ufs: set the device reference clock setting

[Adrian Hunter]

On 21/06/18 12:11, sayali wrote:
> Hi Adrian,
> 
> Please check my comment inline.
> 
> Thanks,
> Sayali
> -----Original Message-----
> From: Adrian Hunter [mailto:adrian.hunter@intel.com] 
> Sent: Wednesday, June 20, 2018 12:56 PM
> To: Sayali Lokhande <sayalil@codeaurora.org>; subhashj@codeaurora.org; cang@codeaurora.org; vivek.gautam@codeaurora.org; rnayak@codeaurora.org; vinholikatti@gmail.com; jejb@linux.vnet.ibm.com; martin.petersen@oracle.com; asutoshd@codeaurora.org; evgreen@chromium.org; riteshh@codeaurora.org
> Cc: linux-scsi@vger.kernel.org; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; Mathieu Malaterre <malat@debian.org>; open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH V3 1/3] scsi: ufs: set the device reference clock setting
> 
> On 14/06/18 16:03, Sayali Lokhande wrote:
>> From: Subhash Jadavani <subhashj@codeaurora.org>
>>
>> UFS host supplies the reference clock to UFS device and UFS device 
>> specification allows host to provide one of the 4 frequencies (19.2 
>> MHz,
>> 26 MHz, 38.4 MHz, 52 MHz) for reference clock. Host should set the 
>> device reference clock frequency setting in the device based on what 
>> frequency it is supplying to UFS device.
>>
>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
>> Signed-off-by: Can Guo <cang@codeaurora.org>
>> Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
> 
> I have repeated my V2 comments below.  Please address these when you post V4.  Also please provide a change log for each patch version.
> 
>> ---
>>  .../devicetree/bindings/ufs/ufshcd-pltfrm.txt      |  7 +++
>>  drivers/scsi/ufs/ufs.h                             |  9 ++++
>>  drivers/scsi/ufs/ufshcd-pltfrm.c                   | 24 ++++++++++
>>  drivers/scsi/ufs/ufshcd.c                          | 52 ++++++++++++++++++++++
>>  drivers/scsi/ufs/ufshcd.h                          |  1 +
>>  5 files changed, 93 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt 
>> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> index c39dfef..4522434 100644
>> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>> @@ -41,6 +41,12 @@ Optional properties:
>>  -lanes-per-direction	: number of lanes available per direction - either 1 or 2.
>>  			  Note that it is assume same number of lanes is used both
>>  			  directions at once. If not specified, default is 2 lanes per direction.
>> +- dev-ref-clk-freq	: Specify the device reference clock frequency, must be one of the following:
>> +			  0: 19.2 MHz
>> +			  1: 26 MHz
>> +			  2: 38.4 MHz
>> +			  3: 52 MHz
>> +			  Defaults to 26 MHz if not specified.
>>  
>>  Note: If above properties are not defined it can be assumed that the 
>> supply  regulators or clocks are always on.
>> @@ -66,4 +72,5 @@ Example:
>>  		freq-table-hz = <100000000 200000000>, <0 0>, <0 0>;
>>  		phys = <&ufsphy1>;
>>  		phy-names = "ufsphy";
>> +		dev-ref-clk-freq = <0>; /* reference clock freq: 19.2 MHz */
>>  	};
>> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h index 
>> 14e5bf7..e15deb0 100644
>> --- a/drivers/scsi/ufs/ufs.h
>> +++ b/drivers/scsi/ufs/ufs.h
>> @@ -378,6 +378,15 @@ enum query_opcode {
>>  	UPIU_QUERY_OPCODE_TOGGLE_FLAG	= 0x8,
>>  };
>>  
>> +/* bRefClkFreq attribute values */
>> +enum ref_clk_freq {
>> +	REF_CLK_FREQ_19_2_MHZ	= 0x0,
>> +	REF_CLK_FREQ_26_MHZ	= 0x1,
>> +	REF_CLK_FREQ_38_4_MHZ	= 0x2,
>> +	REF_CLK_FREQ_52_MHZ	= 0x3,
>> +	REF_CLK_FREQ_MAX	= REF_CLK_FREQ_52_MHZ,
>> +};
>> +
>>  /* Query response result code */
>>  enum {
>>  	QUERY_RESULT_SUCCESS                    = 0x00,
>> diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c 
>> b/drivers/scsi/ufs/ufshcd-pltfrm.c
>> index e82bde0..6c877f3 100644
>> --- a/drivers/scsi/ufs/ufshcd-pltfrm.c
>> +++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
>> @@ -221,6 +221,28 @@ static int ufshcd_parse_regulator_info(struct ufs_hba *hba)
>>  	return err;
>>  }
>>  
>> +static void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba) {
>> +	struct device *dev = hba->dev;
>> +	struct device_node *np = dev->of_node;
>> +	int ret;
>> +
>> +	if (!np)
>> +		return;
>> +
>> +	ret = of_property_read_u32(np, "dev-ref-clk-freq",
>> +				   &hba->dev_ref_clk_freq);
> 
> This setting is useful for any UFSHC driver.  Please move it to ufshcd.c and use device_property_read_u32().
> [Sayali]: Ok. Will update this in next patch set.
> 
>> +	if (ret ||
>> +	    (hba->dev_ref_clk_freq < 0) ||
> 
> u32 cannot be < 0
> [Sayali]: Will remove this check.
> 
>> +	    (hba->dev_ref_clk_freq > REF_CLK_FREQ_52_MHZ)) {
>> +		dev_err(hba->dev,
>> +		"%s: invalid ref_clk setting = %d, set to default\n",
>> +		__func__, hba->dev_ref_clk_freq);
>> +		/* default setting */
>> +		hba->dev_ref_clk_freq = REF_CLK_FREQ_26_MHZ;
> 
> No, the default must be to leave the value unchanged.
> [Sayali]: As per UFS device specification , bRefClkFreq attribute's default value corresponds to 26 MHz (Manufacturer Default Value). Thus we should set it to default (26MHz) if it's not already set.

No, bRefClkFreq is not volatile and a different value may have already been
programmed to the device.  You must not overwrite it if there is no
property.  Also writing 26MHz when the property value is invalid makes no sense.

> 
>> +	}
>> +}
>> +
>>  #ifdef CONFIG_PM
>>  /**
>>   * ufshcd_pltfrm_suspend - suspend power management function @@ 
>> -343,6 +365,8 @@ int ufshcd_pltfrm_init(struct platform_device *pdev,
>>  	pm_runtime_set_active(&pdev->dev);
>>  	pm_runtime_enable(&pdev->dev);
>>  
>> +	ufshcd_parse_dev_ref_clk_freq(hba);
>> +
>>  	ufshcd_init_lanes_per_dir(hba);
>>  
>>  	err = ufshcd_init(hba, mmio_base, irq); diff --git 
>> a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 
>> c5b1bf1..4abc7ae 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -6297,6 +6297,53 @@ static void ufshcd_def_desc_sizes(struct 
>> ufs_hba *hba)  }
>>  
>>  /**
>> + * ufshcd_set_dev_ref_clk - set the device bRefClkFreq
>> + * @hba: per-adapter instance
>> + *
>> + * Read the current value of the bRefClkFreq attribute from device 
>> +and update it
>> + * if host is supplying different reference clock frequency than one 
>> +mentioned
>> + * in bRefClkFreq attribute.
>> + *
>> + * Returns zero on success, non-zero error value on failure.
>> + */
>> +static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba) {
>> +	int err = 0;
>> +	int ref_clk = -1;
>> +	static const char * const ref_clk_freqs[] = {"19.2 MHz", "26 MHz",
>> +						     "38.4 MHz", "52 MHz"};
>> +
>> +	err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_READ_ATTR,
>> +			QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0, &ref_clk);
>> +
>> +	if (err) {
>> +		dev_err(hba->dev, "%s: failed reading bRefClkFreq. err = %d\n",
>> +			 __func__, err);
>> +		goto out;
>> +	}
>> +
>> +	if (ref_clk == hba->dev_ref_clk_freq)
>> +		goto out; /* nothing to update */
>> +
>> +	err = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
>> +			QUERY_ATTR_IDN_REF_CLK_FREQ, 0, 0,
>> +			&hba->dev_ref_clk_freq);
>> +
>> +	if (err)
>> +		dev_err(hba->dev, "%s: bRefClkFreq setting to %s failed\n",
>> +			__func__, ref_clk_freqs[hba->dev_ref_clk_freq]);
>> +	/*
>> +	 * It is good to print this out here to debug any later failures
>> +	 * related to gear switch.
>> +	 */
>> +	dev_dbg(hba->dev, "%s: bRefClkFreq setting to %s succeeded\n",
>> +			__func__, ref_clk_freqs[hba->dev_ref_clk_freq]);
>> +
>> +out:
>> +	return err;
>> +}
>> +
>> +/**
>>   * ufshcd_probe_hba - probe hba to detect device and initialize
>>   * @hba: per-adapter instance
>>   *
>> @@ -6361,6 +6408,11 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
>>  			"%s: Failed getting max supported power mode\n",
>>  			__func__);
>>  	} else {
>> +		/*
>> +		 * Set the right value to bRefClkFreq before attempting to
>> +		 * switch to HS gears.
>> +		 */
>> +		ufshcd_set_dev_ref_clk(hba);
>>  		ret = ufshcd_config_pwr_mode(hba, &hba->max_pwr_info.info);
>>  		if (ret) {
>>  			dev_err(hba->dev, "%s: Failed setting power mode, err = %d\n", 
>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h 
>> index 8110dcd..b026ad8 100644
>> --- a/drivers/scsi/ufs/ufshcd.h
>> +++ b/drivers/scsi/ufs/ufshcd.h
>> @@ -548,6 +548,7 @@ struct ufs_hba {
>>  	void *priv;
>>  	unsigned int irq;
>>  	bool is_irq_enabled;
>> +	u32 dev_ref_clk_freq;
>>  
>>  	/* Interrupt aggregation support is broken */
>>  	#define UFSHCD_QUIRK_BROKEN_INTR_AGGR			0x1
>>
> 
> 
> 

[PATCH V3 2/3] scsi: ufs: Add ufs provisioning support

[Sayali Lokhande]

A new api ufshcd_do_config_device() is added in driver
to support UFS provisioning at runtime. Configfs support
is added to trigger provisioning.
Device and Unit configurable parameters are parsed from
vendor specific provisioning data or file and
passed via configfs node at runtime to provision ufs device.

Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
---
 drivers/scsi/ufs/ufs.h    |  28 ++++++++
 drivers/scsi/ufs/ufshcd.c | 180 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd.h |   2 +
 3 files changed, 210 insertions(+)

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index e15deb0..1f99904 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -333,6 +333,7 @@ enum {
 	UFSHCD_AMP		= 3,
 };
 
+#define UFS_BLOCK_SIZE	4096
 #define POWER_DESC_MAX_SIZE			0x62
 #define POWER_DESC_MAX_ACTV_ICC_LVLS		16
 
@@ -425,6 +426,33 @@ enum {
 	MASK_TM_SERVICE_RESP		= 0xFF,
 };
 
+struct ufs_unit_desc {
+	u8     bLUEnable;              /* 1 for enabled LU */
+	u8     bBootLunID;             /* 0 for using this LU for boot */
+	u8     bLUWriteProtect;        /* 1 = power on WP, 2 = permanent WP */
+	u8     bMemoryType;            /* 0 for enhanced memory type */
+	u32    dNumAllocUnits;         /* Number of alloc unit for this LU */
+	u8     bDataReliability;       /* 0 for reliable write support */
+	u8     bLogicalBlockSize;      /* See section 13.2.3 of UFS standard */
+	u8     bProvisioningType;      /* 0 for thin provisioning */
+	u16    wContextCapabilities;   /* refer Unit Descriptor Description */
+};
+
+struct ufs_config_descr {
+	u8     bNumberLU;              /* Total number of active LUs */
+	u8     bBootEnable;            /* enabling device for partial init */
+	u8     bDescrAccessEn;         /* desc access during partial init */
+	u8     bInitPowerMode;         /* Initial device power mode */
+	u8     bHighPriorityLUN;       /* LUN of the high priority LU */
+	u8     bSecureRemovalType;     /* Erase config for data removal */
+	u8     bInitActiveICCLevel;    /* ICC level after reset */
+	u16    wPeriodicRTCUpdate;     /* 0 to set a priodic RTC update rate */
+	u32     bConfigDescrLock;      /* 1 to lock Configation Descriptor */
+	u32    qVendorConfigCode;      /* Vendor specific configuration code */
+	struct ufs_unit_desc unit[8];
+	u8	lun_to_grow;
+};
+
 /* Task management service response */
 enum {
 	UPIU_TASK_MANAGEMENT_FUNC_COMPL		= 0x00,
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 4abc7ae..c0235a4 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 <asm/unaligned.h>
 #include "ufshcd.h"
 #include "ufs_quirks.h"
 #include "unipro.h"
@@ -3063,6 +3064,14 @@ static inline int ufshcd_read_power_desc(struct ufs_hba *hba,
 	return ufshcd_read_desc(hba, QUERY_DESC_IDN_POWER, 0, buf, size);
 }
 
+static inline int ufshcd_read_geometry_desc(struct ufs_hba *hba,
+					 u8 *buf,
+					 u32 size)
+{
+	return ufshcd_read_desc(hba, QUERY_DESC_IDN_GEOMETRY, 0, buf, size);
+}
+
+
 static int ufshcd_read_device_desc(struct ufs_hba *hba, u8 *buf, u32 size)
 {
 	return ufshcd_read_desc(hba, QUERY_DESC_IDN_DEVICE, 0, buf, size);
@@ -6344,6 +6353,177 @@ static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
 }
 
 /**
+ * ufshcd_do_config_device - API function for UFS provisioning
+ * hba: per-adapter instance
+ * Returns 0 for success, non-zero in case of failure.
+ */
+int ufshcd_do_config_device(struct ufs_hba *hba)
+{
+	struct ufs_config_descr *cfg = &hba->cfgs;
+	int buff_len = QUERY_DESC_CONFIGURATION_DEF_SIZE;
+	u8 desc_buf[QUERY_DESC_CONFIGURATION_DEF_SIZE] = {0};
+	int i, ret = 0;
+	int lun_to_grow = -1;
+	u64 qTotalRawDeviceCapacity;
+	u16 wEnhanced1CapAdjFac, wEnhanced2CapAdjFac;
+	u32 dEnhanced1MaxNAllocU, dEnhanced2MaxNAllocU;
+	size_t alloc_units, units_to_create = 0;
+	size_t capacity_to_alloc_factor;
+	size_t enhanced1_units = 0, enhanced2_units = 0;
+	size_t conversion_ratio = 1;
+	u8 *pt;
+	u32 blocks_per_alloc_unit = 1024;
+	int geo_len = hba->desc_size.geom_desc;
+	u8 geo_buf[hba->desc_size.geom_desc];
+	unsigned int max_partitions = 8;
+
+	WARN_ON(!hba || !cfg);
+
+	ret = ufshcd_read_geometry_desc(hba, geo_buf, geo_len);
+	if (ret) {
+		dev_err(hba->dev, "%s: Failed getting geometry_desc %d\n",
+			__func__, ret);
+		return ret;
+	}
+
+	/*
+	 * Get Geomtric parameters like total configurable memory
+	 * quantity (Offset 0x04 to 0x0b), Capacity Adjustment
+	 * Factors (Offset 0x30, 0x31, 0x36, 0x37), Max allocation
+	 * units (Offset 0x2c to 0x2f, 0x32 to 0x35) used to configure
+	 * the device logical units.
+	 */
+	qTotalRawDeviceCapacity = get_unaligned_be64(&geo_buf[0x04]);
+	wEnhanced1CapAdjFac = get_unaligned_be16(&geo_buf[0x30]);
+	wEnhanced2CapAdjFac = get_unaligned_be16(&geo_buf[0x36]);
+	dEnhanced1MaxNAllocU = get_unaligned_be32(&geo_buf[0x2c]);
+	dEnhanced2MaxNAllocU = get_unaligned_be32(&geo_buf[0x32]);
+
+	capacity_to_alloc_factor =
+		(blocks_per_alloc_unit * UFS_BLOCK_SIZE) / 512;
+
+	if (qTotalRawDeviceCapacity % capacity_to_alloc_factor != 0) {
+		dev_err(hba->dev,
+			"%s: Raw capacity(%llu) not multiple of alloc factor(%zu)\n",
+			__func__, qTotalRawDeviceCapacity,
+			capacity_to_alloc_factor);
+		return -EINVAL;
+	}
+	alloc_units = (qTotalRawDeviceCapacity / capacity_to_alloc_factor);
+	units_to_create = 0;
+	enhanced1_units = enhanced2_units = 0;
+
+	/*
+	 * Calculate number of allocation units to be assigned to a logical unit
+	 * considering the capacity adjustment factor of respective memory type.
+	 */
+	for (i = 0; i < (max_partitions - 1) &&
+		units_to_create <= alloc_units; i++) {
+		if ((cfg->unit[i].dNumAllocUnits % blocks_per_alloc_unit) == 0)
+			cfg->unit[i].dNumAllocUnits /= blocks_per_alloc_unit;
+		else
+			cfg->unit[i].dNumAllocUnits =
+			cfg->unit[i].dNumAllocUnits / blocks_per_alloc_unit + 1;
+
+		if (cfg->unit[i].bMemoryType == 0)
+			units_to_create += cfg->unit[i].dNumAllocUnits;
+		else if (cfg->unit[i].bMemoryType == 3) {
+			enhanced1_units += cfg->unit[i].dNumAllocUnits;
+			cfg->unit[i].dNumAllocUnits *=
+				(wEnhanced1CapAdjFac / 0x100);
+			units_to_create += cfg->unit[i].dNumAllocUnits;
+		} else if (cfg->unit[i].bMemoryType == 4) {
+			enhanced2_units += cfg->unit[i].dNumAllocUnits;
+			cfg->unit[i].dNumAllocUnits *=
+				(wEnhanced1CapAdjFac / 0x100);
+			units_to_create += cfg->unit[i].dNumAllocUnits;
+		} else {
+			dev_err(hba->dev, "%s: Unsupported memory type %d\n",
+				__func__, cfg->unit[i].bMemoryType);
+			return -EINVAL;
+		}
+	}
+	if (enhanced1_units > dEnhanced1MaxNAllocU) {
+		dev_err(hba->dev, "%s: size %zu exceeds max enhanced1 area size %u\n",
+			__func__, enhanced1_units, dEnhanced1MaxNAllocU);
+		return -ERANGE;
+	}
+	if (enhanced2_units > dEnhanced2MaxNAllocU) {
+		dev_err(hba->dev, "%s: size %zu exceeds max enhanced2 area size %u\n",
+			__func__, enhanced2_units, dEnhanced2MaxNAllocU);
+		return -ERANGE;
+	}
+	if (units_to_create > alloc_units) {
+		dev_err(hba->dev, "%s: Specified size %zu exceeds device size %zu\n",
+			__func__, units_to_create, alloc_units);
+		return -ERANGE;
+	}
+	lun_to_grow = cfg->lun_to_grow;
+	if (lun_to_grow != -1) {
+		if (cfg->unit[i].bMemoryType == 0)
+			conversion_ratio = 1;
+		else if (cfg->unit[i].bMemoryType == 3)
+			conversion_ratio = (wEnhanced1CapAdjFac / 0x100);
+		else if (cfg->unit[i].bMemoryType == 4)
+			conversion_ratio = (wEnhanced2CapAdjFac / 0x100);
+
+		cfg->unit[lun_to_grow].dNumAllocUnits +=
+		((alloc_units - units_to_create) / conversion_ratio);
+		dev_dbg(hba->dev, "%s: conversion_ratio %zu for lun %d\n",
+			__func__, conversion_ratio, i);
+	}
+
+	/* Fill in the buffer with configuration data */
+	pt = desc_buf;
+	*pt++ = 0x90;        // bLength
+	*pt++ = 0x01;        // bDescriptorType
+	*pt++ = 0;           // Reserved in UFS2.0 and onward
+	*pt++ = cfg->bBootEnable;
+	*pt++ = cfg->bDescrAccessEn;
+	*pt++ = cfg->bInitPowerMode;
+	*pt++ = cfg->bHighPriorityLUN;
+	*pt++ = cfg->bSecureRemovalType;
+	*pt++ = cfg->bInitActiveICCLevel;
+	put_unaligned_be16(cfg->wPeriodicRTCUpdate, pt);
+	pt = pt + 7; // Reserved fields set to 0
+
+	/* Fill in the buffer with per logical unit data */
+	for (i = 0; i < UFS_UPIU_MAX_GENERAL_LUN; i++) {
+		*pt++ = cfg->unit[i].bLUEnable;
+		*pt++ = cfg->unit[i].bBootLunID;
+		*pt++ = cfg->unit[i].bLUWriteProtect;
+		*pt++ = cfg->unit[i].bMemoryType;
+		put_unaligned_be32(cfg->unit[i].dNumAllocUnits, pt);
+		pt = pt + 4;
+		*pt++ = cfg->unit[i].bDataReliability;
+		*pt++ = cfg->unit[i].bLogicalBlockSize;
+		*pt++ = cfg->unit[i].bProvisioningType;
+		put_unaligned_be16(cfg->unit[i].wContextCapabilities, pt);
+		pt = pt + 5; // Reserved fields set to 0
+	}
+
+	ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_WRITE_DESC,
+		QUERY_DESC_IDN_CONFIGURATION, 0, 0, desc_buf, &buff_len);
+
+	if (ret) {
+		dev_err(hba->dev, "%s: Failed writing descriptor. desc_idn %d, opcode %x ret %d\n",
+		__func__, QUERY_DESC_IDN_CONFIGURATION,
+		UPIU_QUERY_OPCODE_WRITE_DESC, ret);
+		return ret;
+	}
+
+	if (cfg->bConfigDescrLock == 1) {
+		ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
+		QUERY_ATTR_IDN_CONF_DESC_LOCK, 0, 0, &cfg->bConfigDescrLock);
+		if (ret)
+			dev_err(hba->dev, "%s: Failed writing bConfigDescrLock %d\n",
+				__func__, ret);
+	}
+
+	return ret;
+}
+
+/**
  * ufshcd_probe_hba - probe hba to detect device and initialize
  * @hba: per-adapter instance
  *
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index b026ad8..816b674 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -549,6 +549,7 @@ struct ufs_hba {
 	unsigned int irq;
 	bool is_irq_enabled;
 	u32 dev_ref_clk_freq;
+	struct ufs_config_descr cfgs;
 
 	/* Interrupt aggregation support is broken */
 	#define UFSHCD_QUIRK_BROKEN_INTR_AGGR			0x1
@@ -866,6 +867,7 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index,
 
 int ufshcd_hold(struct ufs_hba *hba, bool async);
 void ufshcd_release(struct ufs_hba *hba);
+int ufshcd_do_config_device(struct ufs_hba *hba);
 
 int ufshcd_map_desc_id_to_length(struct ufs_hba *hba, enum desc_idn desc_id,
 	int *desc_length);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH V3 2/3] scsi: ufs: Add ufs provisioning support

[Sayali Lokhande]

A new api ufshcd_do_config_device() is added in driver
to support UFS provisioning at runtime. Configfs support
is added to trigger provisioning.
Device and Unit configurable parameters are parsed from
vendor specific provisioning data or file and
passed via configfs node at runtime to provision ufs device.

Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
---
 drivers/scsi/ufs/ufs.h    |  28 ++++++++
 drivers/scsi/ufs/ufshcd.c | 180 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd.h |   2 +
 3 files changed, 210 insertions(+)

diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index e15deb0..1f99904 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -333,6 +333,7 @@ enum {
 	UFSHCD_AMP		= 3,
 };
 
+#define UFS_BLOCK_SIZE	4096
 #define POWER_DESC_MAX_SIZE			0x62
 #define POWER_DESC_MAX_ACTV_ICC_LVLS		16
 
@@ -425,6 +426,33 @@ enum {
 	MASK_TM_SERVICE_RESP		= 0xFF,
 };
 
+struct ufs_unit_desc {
+	u8     bLUEnable;              /* 1 for enabled LU */
+	u8     bBootLunID;             /* 0 for using this LU for boot */
+	u8     bLUWriteProtect;        /* 1 = power on WP, 2 = permanent WP */
+	u8     bMemoryType;            /* 0 for enhanced memory type */
+	u32    dNumAllocUnits;         /* Number of alloc unit for this LU */
+	u8     bDataReliability;       /* 0 for reliable write support */
+	u8     bLogicalBlockSize;      /* See section 13.2.3 of UFS standard */
+	u8     bProvisioningType;      /* 0 for thin provisioning */
+	u16    wContextCapabilities;   /* refer Unit Descriptor Description */
+};
+
+struct ufs_config_descr {
+	u8     bNumberLU;              /* Total number of active LUs */
+	u8     bBootEnable;            /* enabling device for partial init */
+	u8     bDescrAccessEn;         /* desc access during partial init */
+	u8     bInitPowerMode;         /* Initial device power mode */
+	u8     bHighPriorityLUN;       /* LUN of the high priority LU */
+	u8     bSecureRemovalType;     /* Erase config for data removal */
+	u8     bInitActiveICCLevel;    /* ICC level after reset */
+	u16    wPeriodicRTCUpdate;     /* 0 to set a priodic RTC update rate */
+	u32     bConfigDescrLock;      /* 1 to lock Configation Descriptor */
+	u32    qVendorConfigCode;      /* Vendor specific configuration code */
+	struct ufs_unit_desc unit[8];
+	u8	lun_to_grow;
+};
+
 /* Task management service response */
 enum {
 	UPIU_TASK_MANAGEMENT_FUNC_COMPL		= 0x00,
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 4abc7ae..c0235a4 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 <asm/unaligned.h>
 #include "ufshcd.h"
 #include "ufs_quirks.h"
 #include "unipro.h"
@@ -3063,6 +3064,14 @@ static inline int ufshcd_read_power_desc(struct ufs_hba *hba,
 	return ufshcd_read_desc(hba, QUERY_DESC_IDN_POWER, 0, buf, size);
 }
 
+static inline int ufshcd_read_geometry_desc(struct ufs_hba *hba,
+					 u8 *buf,
+					 u32 size)
+{
+	return ufshcd_read_desc(hba, QUERY_DESC_IDN_GEOMETRY, 0, buf, size);
+}
+
+
 static int ufshcd_read_device_desc(struct ufs_hba *hba, u8 *buf, u32 size)
 {
 	return ufshcd_read_desc(hba, QUERY_DESC_IDN_DEVICE, 0, buf, size);
@@ -6344,6 +6353,177 @@ static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
 }
 
 /**
+ * ufshcd_do_config_device - API function for UFS provisioning
+ * hba: per-adapter instance
+ * Returns 0 for success, non-zero in case of failure.
+ */
+int ufshcd_do_config_device(struct ufs_hba *hba)
+{
+	struct ufs_config_descr *cfg = &hba->cfgs;
+	int buff_len = QUERY_DESC_CONFIGURATION_DEF_SIZE;
+	u8 desc_buf[QUERY_DESC_CONFIGURATION_DEF_SIZE] = {0};
+	int i, ret = 0;
+	int lun_to_grow = -1;
+	u64 qTotalRawDeviceCapacity;
+	u16 wEnhanced1CapAdjFac, wEnhanced2CapAdjFac;
+	u32 dEnhanced1MaxNAllocU, dEnhanced2MaxNAllocU;
+	size_t alloc_units, units_to_create = 0;
+	size_t capacity_to_alloc_factor;
+	size_t enhanced1_units = 0, enhanced2_units = 0;
+	size_t conversion_ratio = 1;
+	u8 *pt;
+	u32 blocks_per_alloc_unit = 1024;
+	int geo_len = hba->desc_size.geom_desc;
+	u8 geo_buf[hba->desc_size.geom_desc];
+	unsigned int max_partitions = 8;
+
+	WARN_ON(!hba || !cfg);
+
+	ret = ufshcd_read_geometry_desc(hba, geo_buf, geo_len);
+	if (ret) {
+		dev_err(hba->dev, "%s: Failed getting geometry_desc %d\n",
+			__func__, ret);
+		return ret;
+	}
+
+	/*
+	 * Get Geomtric parameters like total configurable memory
+	 * quantity (Offset 0x04 to 0x0b), Capacity Adjustment
+	 * Factors (Offset 0x30, 0x31, 0x36, 0x37), Max allocation
+	 * units (Offset 0x2c to 0x2f, 0x32 to 0x35) used to configure
+	 * the device logical units.
+	 */
+	qTotalRawDeviceCapacity = get_unaligned_be64(&geo_buf[0x04]);
+	wEnhanced1CapAdjFac = get_unaligned_be16(&geo_buf[0x30]);
+	wEnhanced2CapAdjFac = get_unaligned_be16(&geo_buf[0x36]);
+	dEnhanced1MaxNAllocU = get_unaligned_be32(&geo_buf[0x2c]);
+	dEnhanced2MaxNAllocU = get_unaligned_be32(&geo_buf[0x32]);
+
+	capacity_to_alloc_factor =
+		(blocks_per_alloc_unit * UFS_BLOCK_SIZE) / 512;
+
+	if (qTotalRawDeviceCapacity % capacity_to_alloc_factor != 0) {
+		dev_err(hba->dev,
+			"%s: Raw capacity(%llu) not multiple of alloc factor(%zu)\n",
+			__func__, qTotalRawDeviceCapacity,
+			capacity_to_alloc_factor);
+		return -EINVAL;
+	}
+	alloc_units = (qTotalRawDeviceCapacity / capacity_to_alloc_factor);
+	units_to_create = 0;
+	enhanced1_units = enhanced2_units = 0;
+
+	/*
+	 * Calculate number of allocation units to be assigned to a logical unit
+	 * considering the capacity adjustment factor of respective memory type.
+	 */
+	for (i = 0; i < (max_partitions - 1) &&
+		units_to_create <= alloc_units; i++) {
+		if ((cfg->unit[i].dNumAllocUnits % blocks_per_alloc_unit) == 0)
+			cfg->unit[i].dNumAllocUnits /= blocks_per_alloc_unit;
+		else
+			cfg->unit[i].dNumAllocUnits =
+			cfg->unit[i].dNumAllocUnits / blocks_per_alloc_unit + 1;
+
+		if (cfg->unit[i].bMemoryType == 0)
+			units_to_create += cfg->unit[i].dNumAllocUnits;
+		else if (cfg->unit[i].bMemoryType == 3) {
+			enhanced1_units += cfg->unit[i].dNumAllocUnits;
+			cfg->unit[i].dNumAllocUnits *=
+				(wEnhanced1CapAdjFac / 0x100);
+			units_to_create += cfg->unit[i].dNumAllocUnits;
+		} else if (cfg->unit[i].bMemoryType == 4) {
+			enhanced2_units += cfg->unit[i].dNumAllocUnits;
+			cfg->unit[i].dNumAllocUnits *=
+				(wEnhanced1CapAdjFac / 0x100);
+			units_to_create += cfg->unit[i].dNumAllocUnits;
+		} else {
+			dev_err(hba->dev, "%s: Unsupported memory type %d\n",
+				__func__, cfg->unit[i].bMemoryType);
+			return -EINVAL;
+		}
+	}
+	if (enhanced1_units > dEnhanced1MaxNAllocU) {
+		dev_err(hba->dev, "%s: size %zu exceeds max enhanced1 area size %u\n",
+			__func__, enhanced1_units, dEnhanced1MaxNAllocU);
+		return -ERANGE;
+	}
+	if (enhanced2_units > dEnhanced2MaxNAllocU) {
+		dev_err(hba->dev, "%s: size %zu exceeds max enhanced2 area size %u\n",
+			__func__, enhanced2_units, dEnhanced2MaxNAllocU);
+		return -ERANGE;
+	}
+	if (units_to_create > alloc_units) {
+		dev_err(hba->dev, "%s: Specified size %zu exceeds device size %zu\n",
+			__func__, units_to_create, alloc_units);
+		return -ERANGE;
+	}
+	lun_to_grow = cfg->lun_to_grow;
+	if (lun_to_grow != -1) {
+		if (cfg->unit[i].bMemoryType == 0)
+			conversion_ratio = 1;
+		else if (cfg->unit[i].bMemoryType == 3)
+			conversion_ratio = (wEnhanced1CapAdjFac / 0x100);
+		else if (cfg->unit[i].bMemoryType == 4)
+			conversion_ratio = (wEnhanced2CapAdjFac / 0x100);
+
+		cfg->unit[lun_to_grow].dNumAllocUnits +=
+		((alloc_units - units_to_create) / conversion_ratio);
+		dev_dbg(hba->dev, "%s: conversion_ratio %zu for lun %d\n",
+			__func__, conversion_ratio, i);
+	}
+
+	/* Fill in the buffer with configuration data */
+	pt = desc_buf;
+	*pt++ = 0x90;        // bLength
+	*pt++ = 0x01;        // bDescriptorType
+	*pt++ = 0;           // Reserved in UFS2.0 and onward
+	*pt++ = cfg->bBootEnable;
+	*pt++ = cfg->bDescrAccessEn;
+	*pt++ = cfg->bInitPowerMode;
+	*pt++ = cfg->bHighPriorityLUN;
+	*pt++ = cfg->bSecureRemovalType;
+	*pt++ = cfg->bInitActiveICCLevel;
+	put_unaligned_be16(cfg->wPeriodicRTCUpdate, pt);
+	pt = pt + 7; // Reserved fields set to 0
+
+	/* Fill in the buffer with per logical unit data */
+	for (i = 0; i < UFS_UPIU_MAX_GENERAL_LUN; i++) {
+		*pt++ = cfg->unit[i].bLUEnable;
+		*pt++ = cfg->unit[i].bBootLunID;
+		*pt++ = cfg->unit[i].bLUWriteProtect;
+		*pt++ = cfg->unit[i].bMemoryType;
+		put_unaligned_be32(cfg->unit[i].dNumAllocUnits, pt);
+		pt = pt + 4;
+		*pt++ = cfg->unit[i].bDataReliability;
+		*pt++ = cfg->unit[i].bLogicalBlockSize;
+		*pt++ = cfg->unit[i].bProvisioningType;
+		put_unaligned_be16(cfg->unit[i].wContextCapabilities, pt);
+		pt = pt + 5; // Reserved fields set to 0
+	}
+
+	ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_WRITE_DESC,
+		QUERY_DESC_IDN_CONFIGURATION, 0, 0, desc_buf, &buff_len);
+
+	if (ret) {
+		dev_err(hba->dev, "%s: Failed writing descriptor. desc_idn %d, opcode %x ret %d\n",
+		__func__, QUERY_DESC_IDN_CONFIGURATION,
+		UPIU_QUERY_OPCODE_WRITE_DESC, ret);
+		return ret;
+	}
+
+	if (cfg->bConfigDescrLock == 1) {
+		ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
+		QUERY_ATTR_IDN_CONF_DESC_LOCK, 0, 0, &cfg->bConfigDescrLock);
+		if (ret)
+			dev_err(hba->dev, "%s: Failed writing bConfigDescrLock %d\n",
+				__func__, ret);
+	}
+
+	return ret;
+}
+
+/**
  * ufshcd_probe_hba - probe hba to detect device and initialize
  * @hba: per-adapter instance
  *
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index b026ad8..816b674 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -549,6 +549,7 @@ struct ufs_hba {
 	unsigned int irq;
 	bool is_irq_enabled;
 	u32 dev_ref_clk_freq;
+	struct ufs_config_descr cfgs;
 
 	/* Interrupt aggregation support is broken */
 	#define UFSHCD_QUIRK_BROKEN_INTR_AGGR			0x1
@@ -866,6 +867,7 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index,
 
 int ufshcd_hold(struct ufs_hba *hba, bool async);
 void ufshcd_release(struct ufs_hba *hba);
+int ufshcd_do_config_device(struct ufs_hba *hba);
 
 int ufshcd_map_desc_id_to_length(struct ufs_hba *hba, enum desc_idn desc_id,
 	int *desc_length);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH V3 2/3] scsi: ufs: Add ufs provisioning support

[Christoph Hellwig]

On Thu, Jun 14, 2018 at 06:33:51PM +0530, Sayali Lokhande wrote:
> A new api ufshcd_do_config_device() is added in driver
> to support UFS provisioning at runtime. Configfs support
> is added to trigger provisioning.
> Device and Unit configurable parameters are parsed from
> vendor specific provisioning data or file and
> passed via configfs node at runtime to provision ufs device.
> 
> Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>

Please make that code conditionally compiled based on a config option.
For most UFS users there is no need to ever enable it.

Re: [PATCH V3 2/3] scsi: ufs: Add ufs provisioning support

[Kyuho Choi]

Hi,

On 6/14/18, Sayali Lokhande <sayalil@codeaurora.org> wrote:
> A new api ufshcd_do_config_device() is added in driver
> to support UFS provisioning at runtime. Configfs support
> is added to trigger provisioning.
> Device and Unit configurable parameters are parsed from
> vendor specific provisioning data or file and
> passed via configfs node at runtime to provision ufs device.
>
> Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
> ---
>  drivers/scsi/ufs/ufs.h    |  28 ++++++++
>  drivers/scsi/ufs/ufshcd.c | 180
> ++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd.h |   2 +
>  3 files changed, 210 insertions(+)
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index e15deb0..1f99904 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -333,6 +333,7 @@ enum {
>  	UFSHCD_AMP		= 3,
>  };
>
> +#define UFS_BLOCK_SIZE	4096
>  #define POWER_DESC_MAX_SIZE			0x62
>  #define POWER_DESC_MAX_ACTV_ICC_LVLS		16
>
> @@ -425,6 +426,33 @@ enum {
>  	MASK_TM_SERVICE_RESP		= 0xFF,
>  };
>
> +struct ufs_unit_desc {
> +	u8     bLUEnable;              /* 1 for enabled LU */
> +	u8     bBootLunID;             /* 0 for using this LU for boot */
> +	u8     bLUWriteProtect;        /* 1 = power on WP, 2 = permanent WP */
> +	u8     bMemoryType;            /* 0 for enhanced memory type */
> +	u32    dNumAllocUnits;         /* Number of alloc unit for this LU */
> +	u8     bDataReliability;       /* 0 for reliable write support */
> +	u8     bLogicalBlockSize;      /* See section 13.2.3 of UFS standard */
> +	u8     bProvisioningType;      /* 0 for thin provisioning */
> +	u16    wContextCapabilities;   /* refer Unit Descriptor Description */
> +};
> +
> +struct ufs_config_descr {
> +	u8     bNumberLU;              /* Total number of active LUs */
> +	u8     bBootEnable;            /* enabling device for partial init */
> +	u8     bDescrAccessEn;         /* desc access during partial init */
> +	u8     bInitPowerMode;         /* Initial device power mode */
> +	u8     bHighPriorityLUN;       /* LUN of the high priority LU */
> +	u8     bSecureRemovalType;     /* Erase config for data removal */
> +	u8     bInitActiveICCLevel;    /* ICC level after reset */
> +	u16    wPeriodicRTCUpdate;     /* 0 to set a priodic RTC update rate */
> +	u32     bConfigDescrLock;      /* 1 to lock Configation Descriptor */
> +	u32    qVendorConfigCode;      /* Vendor specific configuration code */
> +	struct ufs_unit_desc unit[8];
> +	u8	lun_to_grow;
> +};
> +
>  /* Task management service response */
>  enum {
>  	UPIU_TASK_MANAGEMENT_FUNC_COMPL		= 0x00,
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 4abc7ae..c0235a4 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 <asm/unaligned.h>
>  #include "ufshcd.h"
>  #include "ufs_quirks.h"
>  #include "unipro.h"
> @@ -3063,6 +3064,14 @@ static inline int ufshcd_read_power_desc(struct
> ufs_hba *hba,
>  	return ufshcd_read_desc(hba, QUERY_DESC_IDN_POWER, 0, buf, size);
>  }
>
> +static inline int ufshcd_read_geometry_desc(struct ufs_hba *hba,
> +					 u8 *buf,
> +					 u32 size)
> +{
> +	return ufshcd_read_desc(hba, QUERY_DESC_IDN_GEOMETRY, 0, buf, size);
> +}
> +
> +
>  static int ufshcd_read_device_desc(struct ufs_hba *hba, u8 *buf, u32 size)
>  {
>  	return ufshcd_read_desc(hba, QUERY_DESC_IDN_DEVICE, 0, buf, size);
> @@ -6344,6 +6353,177 @@ static int ufshcd_set_dev_ref_clk(struct ufs_hba
> *hba)
>  }
>
>  /**
> + * ufshcd_do_config_device - API function for UFS provisioning
> + * hba: per-adapter instance
> + * Returns 0 for success, non-zero in case of failure.
> + */
> +int ufshcd_do_config_device(struct ufs_hba *hba)
> +{
> +	struct ufs_config_descr *cfg = &hba->cfgs;
> +	int buff_len = QUERY_DESC_CONFIGURATION_DEF_SIZE;
> +	u8 desc_buf[QUERY_DESC_CONFIGURATION_DEF_SIZE] = {0};
> +	int i, ret = 0;
> +	int lun_to_grow = -1;
> +	u64 qTotalRawDeviceCapacity;
> +	u16 wEnhanced1CapAdjFac, wEnhanced2CapAdjFac;
> +	u32 dEnhanced1MaxNAllocU, dEnhanced2MaxNAllocU;
> +	size_t alloc_units, units_to_create = 0;
> +	size_t capacity_to_alloc_factor;
> +	size_t enhanced1_units = 0, enhanced2_units = 0;
> +	size_t conversion_ratio = 1;
> +	u8 *pt;
> +	u32 blocks_per_alloc_unit = 1024;
> +	int geo_len = hba->desc_size.geom_desc;
> +	u8 geo_buf[hba->desc_size.geom_desc];
> +	unsigned int max_partitions = 8;
> +
> +	WARN_ON(!hba || !cfg);
> +
> +	ret = ufshcd_read_geometry_desc(hba, geo_buf, geo_len);
> +	if (ret) {
> +		dev_err(hba->dev, "%s: Failed getting geometry_desc %d\n",
> +			__func__, ret);
> +		return ret;
> +	}
> +
> +	/*
> +	 * Get Geomtric parameters like total configurable memory
> +	 * quantity (Offset 0x04 to 0x0b), Capacity Adjustment
> +	 * Factors (Offset 0x30, 0x31, 0x36, 0x37), Max allocation
> +	 * units (Offset 0x2c to 0x2f, 0x32 to 0x35) used to configure
> +	 * the device logical units.
> +	 */
> +	qTotalRawDeviceCapacity = get_unaligned_be64(&geo_buf[0x04]);
> +	wEnhanced1CapAdjFac = get_unaligned_be16(&geo_buf[0x30]);
> +	wEnhanced2CapAdjFac = get_unaligned_be16(&geo_buf[0x36]);
> +	dEnhanced1MaxNAllocU = get_unaligned_be32(&geo_buf[0x2c]);
> +	dEnhanced2MaxNAllocU = get_unaligned_be32(&geo_buf[0x32]);
> +
> +	capacity_to_alloc_factor =
> +		(blocks_per_alloc_unit * UFS_BLOCK_SIZE) / 512;
> +
> +	if (qTotalRawDeviceCapacity % capacity_to_alloc_factor != 0) {
> +		dev_err(hba->dev,
> +			"%s: Raw capacity(%llu) not multiple of alloc factor(%zu)\n",
> +			__func__, qTotalRawDeviceCapacity,
> +			capacity_to_alloc_factor);
> +		return -EINVAL;
> +	}
> +	alloc_units = (qTotalRawDeviceCapacity / capacity_to_alloc_factor);
> +	units_to_create = 0;
> +	enhanced1_units = enhanced2_units = 0;
> +
> +	/*
> +	 * Calculate number of allocation units to be assigned to a logical unit
> +	 * considering the capacity adjustment factor of respective memory type.
> +	 */
> +	for (i = 0; i < (max_partitions - 1) &&
> +		units_to_create <= alloc_units; i++) {
> +		if ((cfg->unit[i].dNumAllocUnits % blocks_per_alloc_unit) == 0)
> +			cfg->unit[i].dNumAllocUnits /= blocks_per_alloc_unit;
> +		else
> +			cfg->unit[i].dNumAllocUnits =
> +			cfg->unit[i].dNumAllocUnits / blocks_per_alloc_unit + 1;
> +
> +		if (cfg->unit[i].bMemoryType == 0)
> +			units_to_create += cfg->unit[i].dNumAllocUnits;
> +		else if (cfg->unit[i].bMemoryType == 3) {
> +			enhanced1_units += cfg->unit[i].dNumAllocUnits;
> +			cfg->unit[i].dNumAllocUnits *=
> +				(wEnhanced1CapAdjFac / 0x100);
> +			units_to_create += cfg->unit[i].dNumAllocUnits;
> +		} else if (cfg->unit[i].bMemoryType == 4) {
> +			enhanced2_units += cfg->unit[i].dNumAllocUnits;
> +			cfg->unit[i].dNumAllocUnits *=
> +				(wEnhanced1CapAdjFac / 0x100);
> +			units_to_create += cfg->unit[i].dNumAllocUnits;
> +		} else {
> +			dev_err(hba->dev, "%s: Unsupported memory type %d\n",
> +				__func__, cfg->unit[i].bMemoryType);
> +			return -EINVAL;
> +		}
> +	}
> +	if (enhanced1_units > dEnhanced1MaxNAllocU) {
> +		dev_err(hba->dev, "%s: size %zu exceeds max enhanced1 area size %u\n",
> +			__func__, enhanced1_units, dEnhanced1MaxNAllocU);
> +		return -ERANGE;
> +	}
> +	if (enhanced2_units > dEnhanced2MaxNAllocU) {
> +		dev_err(hba->dev, "%s: size %zu exceeds max enhanced2 area size %u\n",
> +			__func__, enhanced2_units, dEnhanced2MaxNAllocU);
> +		return -ERANGE;
> +	}
> +	if (units_to_create > alloc_units) {
> +		dev_err(hba->dev, "%s: Specified size %zu exceeds device size %zu\n",
> +			__func__, units_to_create, alloc_units);
> +		return -ERANGE;
> +	}
> +	lun_to_grow = cfg->lun_to_grow;
> +	if (lun_to_grow != -1) {
> +		if (cfg->unit[i].bMemoryType == 0)
> +			conversion_ratio = 1;
> +		else if (cfg->unit[i].bMemoryType == 3)
> +			conversion_ratio = (wEnhanced1CapAdjFac / 0x100);
> +		else if (cfg->unit[i].bMemoryType == 4)
> +			conversion_ratio = (wEnhanced2CapAdjFac / 0x100);
> +
> +		cfg->unit[lun_to_grow].dNumAllocUnits +=
> +		((alloc_units - units_to_create) / conversion_ratio);
> +		dev_dbg(hba->dev, "%s: conversion_ratio %zu for lun %d\n",
> +			__func__, conversion_ratio, i);
> +	}
> +
> +	/* Fill in the buffer with configuration data */
> +	pt = desc_buf;
> +	*pt++ = 0x90;        // bLength
> +	*pt++ = 0x01;        // bDescriptorType
> +	*pt++ = 0;           // Reserved in UFS2.0 and onward
> +	*pt++ = cfg->bBootEnable;
> +	*pt++ = cfg->bDescrAccessEn;
> +	*pt++ = cfg->bInitPowerMode;
> +	*pt++ = cfg->bHighPriorityLUN;
> +	*pt++ = cfg->bSecureRemovalType;
> +	*pt++ = cfg->bInitActiveICCLevel;
> +	put_unaligned_be16(cfg->wPeriodicRTCUpdate, pt);
> +	pt = pt + 7; // Reserved fields set to 0
> +
> +	/* Fill in the buffer with per logical unit data */
> +	for (i = 0; i < UFS_UPIU_MAX_GENERAL_LUN; i++) {
> +		*pt++ = cfg->unit[i].bLUEnable;
> +		*pt++ = cfg->unit[i].bBootLunID;
> +		*pt++ = cfg->unit[i].bLUWriteProtect;
> +		*pt++ = cfg->unit[i].bMemoryType;
> +		put_unaligned_be32(cfg->unit[i].dNumAllocUnits, pt);
> +		pt = pt + 4;
> +		*pt++ = cfg->unit[i].bDataReliability;
> +		*pt++ = cfg->unit[i].bLogicalBlockSize;
> +		*pt++ = cfg->unit[i].bProvisioningType;
> +		put_unaligned_be16(cfg->unit[i].wContextCapabilities, pt);
> +		pt = pt + 5; // Reserved fields set to 0
> +	}
> +
> +	ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_WRITE_DESC,
> +		QUERY_DESC_IDN_CONFIGURATION, 0, 0, desc_buf, &buff_len);
> +
> +	if (ret) {
> +		dev_err(hba->dev, "%s: Failed writing descriptor. desc_idn %d, opcode %x
> ret %d\n",
> +		__func__, QUERY_DESC_IDN_CONFIGURATION,
> +		UPIU_QUERY_OPCODE_WRITE_DESC, ret);
> +		return ret;
> +	}

To support stable operation and prevent race condition, how about
adding some functions during in runtime provisioning?.
pm_runtime_get/put_sync and scsi_block/unblock_requests.
How you thought?.

> +
> +	if (cfg->bConfigDescrLock == 1) {
> +		ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
> +		QUERY_ATTR_IDN_CONF_DESC_LOCK, 0, 0, &cfg->bConfigDescrLock);
> +		if (ret)
> +			dev_err(hba->dev, "%s: Failed writing bConfigDescrLock %d\n",
> +				__func__, ret);
> +	}
> +
> +	return ret;
> +}
> +
> +/**
>   * ufshcd_probe_hba - probe hba to detect device and initialize
>   * @hba: per-adapter instance
>   *
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index b026ad8..816b674 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -549,6 +549,7 @@ struct ufs_hba {
>  	unsigned int irq;
>  	bool is_irq_enabled;
>  	u32 dev_ref_clk_freq;
> +	struct ufs_config_descr cfgs;
>
>  	/* Interrupt aggregation support is broken */
>  	#define UFSHCD_QUIRK_BROKEN_INTR_AGGR			0x1
> @@ -866,6 +867,7 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, int
> desc_index,
>
>  int ufshcd_hold(struct ufs_hba *hba, bool async);
>  void ufshcd_release(struct ufs_hba *hba);
> +int ufshcd_do_config_device(struct ufs_hba *hba);
>
>  int ufshcd_map_desc_id_to_length(struct ufs_hba *hba, enum desc_idn
> desc_id,
>  	int *desc_length);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
>

BR,
Kyuho Choi

RE: [PATCH V3 2/3] scsi: ufs: Add ufs provisioning support

[sayali]

Hi Kyuho,

Comment inline.

Thanks,
Sayali
-----Original Message-----
From: Kyuho Choi [mailto:chlrbgh0@gmail.com] 
Sent: Friday, June 15, 2018 8:28 AM
To: Sayali Lokhande <sayalil@codeaurora.org>
Cc: subhashj@codeaurora.org; cang@codeaurora.org; vivek.gautam@codeaurora.org; rnayak@codeaurora.org; vinholikatti@gmail.com; jejb@linux.vnet.ibm.com; martin.petersen@oracle.com; asutoshd@codeaurora.org; evgreen@chromium.org; riteshh@codeaurora.org; linux-scsi@vger.kernel.org; open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH V3 2/3] scsi: ufs: Add ufs provisioning support

Hi,

On 6/14/18, Sayali Lokhande <sayalil@codeaurora.org> wrote:
> A new api ufshcd_do_config_device() is added in driver to support UFS 
> provisioning at runtime. Configfs support is added to trigger 
> provisioning.
> Device and Unit configurable parameters are parsed from vendor 
> specific provisioning data or file and passed via configfs node at 
> runtime to provision ufs device.
>
> Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
> ---
>  drivers/scsi/ufs/ufs.h    |  28 ++++++++
>  drivers/scsi/ufs/ufshcd.c | 180
> ++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd.h |   2 +
>  3 files changed, 210 insertions(+)
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h index 
> e15deb0..1f99904 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -333,6 +333,7 @@ enum {
>  	UFSHCD_AMP		= 3,
>  };
>
> +#define UFS_BLOCK_SIZE	4096
>  #define POWER_DESC_MAX_SIZE			0x62
>  #define POWER_DESC_MAX_ACTV_ICC_LVLS		16
>
> @@ -425,6 +426,33 @@ enum {
>  	MASK_TM_SERVICE_RESP		= 0xFF,
>  };
>
> +struct ufs_unit_desc {
> +	u8     bLUEnable;              /* 1 for enabled LU */
> +	u8     bBootLunID;             /* 0 for using this LU for boot */
> +	u8     bLUWriteProtect;        /* 1 = power on WP, 2 = permanent WP */
> +	u8     bMemoryType;            /* 0 for enhanced memory type */
> +	u32    dNumAllocUnits;         /* Number of alloc unit for this LU */
> +	u8     bDataReliability;       /* 0 for reliable write support */
> +	u8     bLogicalBlockSize;      /* See section 13.2.3 of UFS standard */
> +	u8     bProvisioningType;      /* 0 for thin provisioning */
> +	u16    wContextCapabilities;   /* refer Unit Descriptor Description */
> +};
> +
> +struct ufs_config_descr {
> +	u8     bNumberLU;              /* Total number of active LUs */
> +	u8     bBootEnable;            /* enabling device for partial init */
> +	u8     bDescrAccessEn;         /* desc access during partial init */
> +	u8     bInitPowerMode;         /* Initial device power mode */
> +	u8     bHighPriorityLUN;       /* LUN of the high priority LU */
> +	u8     bSecureRemovalType;     /* Erase config for data removal */
> +	u8     bInitActiveICCLevel;    /* ICC level after reset */
> +	u16    wPeriodicRTCUpdate;     /* 0 to set a priodic RTC update rate */
> +	u32     bConfigDescrLock;      /* 1 to lock Configation Descriptor */
> +	u32    qVendorConfigCode;      /* Vendor specific configuration code */
> +	struct ufs_unit_desc unit[8];
> +	u8	lun_to_grow;
> +};
> +
>  /* Task management service response */  enum {
>  	UPIU_TASK_MANAGEMENT_FUNC_COMPL		= 0x00,
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c 
> index 4abc7ae..c0235a4 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 <asm/unaligned.h>
>  #include "ufshcd.h"
>  #include "ufs_quirks.h"
>  #include "unipro.h"
> @@ -3063,6 +3064,14 @@ static inline int ufshcd_read_power_desc(struct 
> ufs_hba *hba,
>  	return ufshcd_read_desc(hba, QUERY_DESC_IDN_POWER, 0, buf, size);  }
>
> +static inline int ufshcd_read_geometry_desc(struct ufs_hba *hba,
> +					 u8 *buf,
> +					 u32 size)
> +{
> +	return ufshcd_read_desc(hba, QUERY_DESC_IDN_GEOMETRY, 0, buf, size); 
> +}
> +
> +
>  static int ufshcd_read_device_desc(struct ufs_hba *hba, u8 *buf, u32 
> size)  {
>  	return ufshcd_read_desc(hba, QUERY_DESC_IDN_DEVICE, 0, buf, size); 
> @@ -6344,6 +6353,177 @@ static int ufshcd_set_dev_ref_clk(struct 
> ufs_hba
> *hba)
>  }
>
>  /**
> + * ufshcd_do_config_device - API function for UFS provisioning
> + * hba: per-adapter instance
> + * Returns 0 for success, non-zero in case of failure.
> + */
> +int ufshcd_do_config_device(struct ufs_hba *hba) {
> +	struct ufs_config_descr *cfg = &hba->cfgs;
> +	int buff_len = QUERY_DESC_CONFIGURATION_DEF_SIZE;
> +	u8 desc_buf[QUERY_DESC_CONFIGURATION_DEF_SIZE] = {0};
> +	int i, ret = 0;
> +	int lun_to_grow = -1;
> +	u64 qTotalRawDeviceCapacity;
> +	u16 wEnhanced1CapAdjFac, wEnhanced2CapAdjFac;
> +	u32 dEnhanced1MaxNAllocU, dEnhanced2MaxNAllocU;
> +	size_t alloc_units, units_to_create = 0;
> +	size_t capacity_to_alloc_factor;
> +	size_t enhanced1_units = 0, enhanced2_units = 0;
> +	size_t conversion_ratio = 1;
> +	u8 *pt;
> +	u32 blocks_per_alloc_unit = 1024;
> +	int geo_len = hba->desc_size.geom_desc;
> +	u8 geo_buf[hba->desc_size.geom_desc];
> +	unsigned int max_partitions = 8;
> +
> +	WARN_ON(!hba || !cfg);
> +
> +	ret = ufshcd_read_geometry_desc(hba, geo_buf, geo_len);
> +	if (ret) {
> +		dev_err(hba->dev, "%s: Failed getting geometry_desc %d\n",
> +			__func__, ret);
> +		return ret;
> +	}
> +
> +	/*
> +	 * Get Geomtric parameters like total configurable memory
> +	 * quantity (Offset 0x04 to 0x0b), Capacity Adjustment
> +	 * Factors (Offset 0x30, 0x31, 0x36, 0x37), Max allocation
> +	 * units (Offset 0x2c to 0x2f, 0x32 to 0x35) used to configure
> +	 * the device logical units.
> +	 */
> +	qTotalRawDeviceCapacity = get_unaligned_be64(&geo_buf[0x04]);
> +	wEnhanced1CapAdjFac = get_unaligned_be16(&geo_buf[0x30]);
> +	wEnhanced2CapAdjFac = get_unaligned_be16(&geo_buf[0x36]);
> +	dEnhanced1MaxNAllocU = get_unaligned_be32(&geo_buf[0x2c]);
> +	dEnhanced2MaxNAllocU = get_unaligned_be32(&geo_buf[0x32]);
> +
> +	capacity_to_alloc_factor =
> +		(blocks_per_alloc_unit * UFS_BLOCK_SIZE) / 512;
> +
> +	if (qTotalRawDeviceCapacity % capacity_to_alloc_factor != 0) {
> +		dev_err(hba->dev,
> +			"%s: Raw capacity(%llu) not multiple of alloc factor(%zu)\n",
> +			__func__, qTotalRawDeviceCapacity,
> +			capacity_to_alloc_factor);
> +		return -EINVAL;
> +	}
> +	alloc_units = (qTotalRawDeviceCapacity / capacity_to_alloc_factor);
> +	units_to_create = 0;
> +	enhanced1_units = enhanced2_units = 0;
> +
> +	/*
> +	 * Calculate number of allocation units to be assigned to a logical unit
> +	 * considering the capacity adjustment factor of respective memory type.
> +	 */
> +	for (i = 0; i < (max_partitions - 1) &&
> +		units_to_create <= alloc_units; i++) {
> +		if ((cfg->unit[i].dNumAllocUnits % blocks_per_alloc_unit) == 0)
> +			cfg->unit[i].dNumAllocUnits /= blocks_per_alloc_unit;
> +		else
> +			cfg->unit[i].dNumAllocUnits =
> +			cfg->unit[i].dNumAllocUnits / blocks_per_alloc_unit + 1;
> +
> +		if (cfg->unit[i].bMemoryType == 0)
> +			units_to_create += cfg->unit[i].dNumAllocUnits;
> +		else if (cfg->unit[i].bMemoryType == 3) {
> +			enhanced1_units += cfg->unit[i].dNumAllocUnits;
> +			cfg->unit[i].dNumAllocUnits *=
> +				(wEnhanced1CapAdjFac / 0x100);
> +			units_to_create += cfg->unit[i].dNumAllocUnits;
> +		} else if (cfg->unit[i].bMemoryType == 4) {
> +			enhanced2_units += cfg->unit[i].dNumAllocUnits;
> +			cfg->unit[i].dNumAllocUnits *=
> +				(wEnhanced1CapAdjFac / 0x100);
> +			units_to_create += cfg->unit[i].dNumAllocUnits;
> +		} else {
> +			dev_err(hba->dev, "%s: Unsupported memory type %d\n",
> +				__func__, cfg->unit[i].bMemoryType);
> +			return -EINVAL;
> +		}
> +	}
> +	if (enhanced1_units > dEnhanced1MaxNAllocU) {
> +		dev_err(hba->dev, "%s: size %zu exceeds max enhanced1 area size %u\n",
> +			__func__, enhanced1_units, dEnhanced1MaxNAllocU);
> +		return -ERANGE;
> +	}
> +	if (enhanced2_units > dEnhanced2MaxNAllocU) {
> +		dev_err(hba->dev, "%s: size %zu exceeds max enhanced2 area size %u\n",
> +			__func__, enhanced2_units, dEnhanced2MaxNAllocU);
> +		return -ERANGE;
> +	}
> +	if (units_to_create > alloc_units) {
> +		dev_err(hba->dev, "%s: Specified size %zu exceeds device size %zu\n",
> +			__func__, units_to_create, alloc_units);
> +		return -ERANGE;
> +	}
> +	lun_to_grow = cfg->lun_to_grow;
> +	if (lun_to_grow != -1) {
> +		if (cfg->unit[i].bMemoryType == 0)
> +			conversion_ratio = 1;
> +		else if (cfg->unit[i].bMemoryType == 3)
> +			conversion_ratio = (wEnhanced1CapAdjFac / 0x100);
> +		else if (cfg->unit[i].bMemoryType == 4)
> +			conversion_ratio = (wEnhanced2CapAdjFac / 0x100);
> +
> +		cfg->unit[lun_to_grow].dNumAllocUnits +=
> +		((alloc_units - units_to_create) / conversion_ratio);
> +		dev_dbg(hba->dev, "%s: conversion_ratio %zu for lun %d\n",
> +			__func__, conversion_ratio, i);
> +	}
> +
> +	/* Fill in the buffer with configuration data */
> +	pt = desc_buf;
> +	*pt++ = 0x90;        // bLength
> +	*pt++ = 0x01;        // bDescriptorType
> +	*pt++ = 0;           // Reserved in UFS2.0 and onward
> +	*pt++ = cfg->bBootEnable;
> +	*pt++ = cfg->bDescrAccessEn;
> +	*pt++ = cfg->bInitPowerMode;
> +	*pt++ = cfg->bHighPriorityLUN;
> +	*pt++ = cfg->bSecureRemovalType;
> +	*pt++ = cfg->bInitActiveICCLevel;
> +	put_unaligned_be16(cfg->wPeriodicRTCUpdate, pt);
> +	pt = pt + 7; // Reserved fields set to 0
> +
> +	/* Fill in the buffer with per logical unit data */
> +	for (i = 0; i < UFS_UPIU_MAX_GENERAL_LUN; i++) {
> +		*pt++ = cfg->unit[i].bLUEnable;
> +		*pt++ = cfg->unit[i].bBootLunID;
> +		*pt++ = cfg->unit[i].bLUWriteProtect;
> +		*pt++ = cfg->unit[i].bMemoryType;
> +		put_unaligned_be32(cfg->unit[i].dNumAllocUnits, pt);
> +		pt = pt + 4;
> +		*pt++ = cfg->unit[i].bDataReliability;
> +		*pt++ = cfg->unit[i].bLogicalBlockSize;
> +		*pt++ = cfg->unit[i].bProvisioningType;
> +		put_unaligned_be16(cfg->unit[i].wContextCapabilities, pt);
> +		pt = pt + 5; // Reserved fields set to 0
> +	}
> +
> +	ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_WRITE_DESC,
> +		QUERY_DESC_IDN_CONFIGURATION, 0, 0, desc_buf, &buff_len);
> +
> +	if (ret) {
> +		dev_err(hba->dev, "%s: Failed writing descriptor. desc_idn %d, 
> +opcode %x
> ret %d\n",
> +		__func__, QUERY_DESC_IDN_CONFIGURATION,
> +		UPIU_QUERY_OPCODE_WRITE_DESC, ret);
> +		return ret;
> +	}

To support stable operation and prevent race condition, how about adding some functions during in runtime provisioning?.
pm_runtime_get/put_sync and scsi_block/unblock_requests.
How you thought?.
[Sayali] Agreed. Will check and update in next patch set.

> +
> +	if (cfg->bConfigDescrLock == 1) {
> +		ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
> +		QUERY_ATTR_IDN_CONF_DESC_LOCK, 0, 0, &cfg->bConfigDescrLock);
> +		if (ret)
> +			dev_err(hba->dev, "%s: Failed writing bConfigDescrLock %d\n",
> +				__func__, ret);
> +	}
> +
> +	return ret;
> +}
> +
> +/**
>   * ufshcd_probe_hba - probe hba to detect device and initialize
>   * @hba: per-adapter instance
>   *
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h 
> index b026ad8..816b674 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -549,6 +549,7 @@ struct ufs_hba {
>  	unsigned int irq;
>  	bool is_irq_enabled;
>  	u32 dev_ref_clk_freq;
> +	struct ufs_config_descr cfgs;
>
>  	/* Interrupt aggregation support is broken */
>  	#define UFSHCD_QUIRK_BROKEN_INTR_AGGR			0x1
> @@ -866,6 +867,7 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, 
> int desc_index,
>
>  int ufshcd_hold(struct ufs_hba *hba, bool async);  void 
> ufshcd_release(struct ufs_hba *hba);
> +int ufshcd_do_config_device(struct ufs_hba *hba);
>
>  int ufshcd_map_desc_id_to_length(struct ufs_hba *hba, enum desc_idn 
> desc_id,
>  	int *desc_length);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
> Forum, a Linux Foundation Collaborative Project
>
>

BR,
Kyuho Choi

RE: [PATCH V3 2/3] scsi: ufs: Add ufs provisioning support

[sayali]

Hi Kyuho,

Comment inline.

Thanks,
Sayali
-----Original Message-----
From: Kyuho Choi [mailto:chlrbgh0@gmail.com] 
Sent: Friday, June 15, 2018 8:28 AM
To: Sayali Lokhande <sayalil@codeaurora.org>
Cc: subhashj@codeaurora.org; cang@codeaurora.org; vivek.gautam@codeaurora.org; rnayak@codeaurora.org; vinholikatti@gmail.com; jejb@linux.vnet.ibm.com; martin.petersen@oracle.com; asutoshd@codeaurora.org; evgreen@chromium.org; riteshh@codeaurora.org; linux-scsi@vger.kernel.org; open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH V3 2/3] scsi: ufs: Add ufs provisioning support

Hi,

On 6/14/18, Sayali Lokhande <sayalil@codeaurora.org> wrote:
> A new api ufshcd_do_config_device() is added in driver to support UFS 
> provisioning at runtime. Configfs support is added to trigger 
> provisioning.
> Device and Unit configurable parameters are parsed from vendor 
> specific provisioning data or file and passed via configfs node at 
> runtime to provision ufs device.
>
> Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
> ---
>  drivers/scsi/ufs/ufs.h    |  28 ++++++++
>  drivers/scsi/ufs/ufshcd.c | 180
> ++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd.h |   2 +
>  3 files changed, 210 insertions(+)
>
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h index 
> e15deb0..1f99904 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -333,6 +333,7 @@ enum {
>  	UFSHCD_AMP		= 3,
>  };
>
> +#define UFS_BLOCK_SIZE	4096
>  #define POWER_DESC_MAX_SIZE			0x62
>  #define POWER_DESC_MAX_ACTV_ICC_LVLS		16
>
> @@ -425,6 +426,33 @@ enum {
>  	MASK_TM_SERVICE_RESP		= 0xFF,
>  };
>
> +struct ufs_unit_desc {
> +	u8     bLUEnable;              /* 1 for enabled LU */
> +	u8     bBootLunID;             /* 0 for using this LU for boot */
> +	u8     bLUWriteProtect;        /* 1 = power on WP, 2 = permanent WP */
> +	u8     bMemoryType;            /* 0 for enhanced memory type */
> +	u32    dNumAllocUnits;         /* Number of alloc unit for this LU */
> +	u8     bDataReliability;       /* 0 for reliable write support */
> +	u8     bLogicalBlockSize;      /* See section 13.2.3 of UFS standard */
> +	u8     bProvisioningType;      /* 0 for thin provisioning */
> +	u16    wContextCapabilities;   /* refer Unit Descriptor Description */
> +};
> +
> +struct ufs_config_descr {
> +	u8     bNumberLU;              /* Total number of active LUs */
> +	u8     bBootEnable;            /* enabling device for partial init */
> +	u8     bDescrAccessEn;         /* desc access during partial init */
> +	u8     bInitPowerMode;         /* Initial device power mode */
> +	u8     bHighPriorityLUN;       /* LUN of the high priority LU */
> +	u8     bSecureRemovalType;     /* Erase config for data removal */
> +	u8     bInitActiveICCLevel;    /* ICC level after reset */
> +	u16    wPeriodicRTCUpdate;     /* 0 to set a priodic RTC update rate */
> +	u32     bConfigDescrLock;      /* 1 to lock Configation Descriptor */
> +	u32    qVendorConfigCode;      /* Vendor specific configuration code */
> +	struct ufs_unit_desc unit[8];
> +	u8	lun_to_grow;
> +};
> +
>  /* Task management service response */  enum {
>  	UPIU_TASK_MANAGEMENT_FUNC_COMPL		= 0x00,
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c 
> index 4abc7ae..c0235a4 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 <asm/unaligned.h>
>  #include "ufshcd.h"
>  #include "ufs_quirks.h"
>  #include "unipro.h"
> @@ -3063,6 +3064,14 @@ static inline int ufshcd_read_power_desc(struct 
> ufs_hba *hba,
>  	return ufshcd_read_desc(hba, QUERY_DESC_IDN_POWER, 0, buf, size);  }
>
> +static inline int ufshcd_read_geometry_desc(struct ufs_hba *hba,
> +					 u8 *buf,
> +					 u32 size)
> +{
> +	return ufshcd_read_desc(hba, QUERY_DESC_IDN_GEOMETRY, 0, buf, size); 
> +}
> +
> +
>  static int ufshcd_read_device_desc(struct ufs_hba *hba, u8 *buf, u32 
> size)  {
>  	return ufshcd_read_desc(hba, QUERY_DESC_IDN_DEVICE, 0, buf, size); 
> @@ -6344,6 +6353,177 @@ static int ufshcd_set_dev_ref_clk(struct 
> ufs_hba
> *hba)
>  }
>
>  /**
> + * ufshcd_do_config_device - API function for UFS provisioning
> + * hba: per-adapter instance
> + * Returns 0 for success, non-zero in case of failure.
> + */
> +int ufshcd_do_config_device(struct ufs_hba *hba) {
> +	struct ufs_config_descr *cfg = &hba->cfgs;
> +	int buff_len = QUERY_DESC_CONFIGURATION_DEF_SIZE;
> +	u8 desc_buf[QUERY_DESC_CONFIGURATION_DEF_SIZE] = {0};
> +	int i, ret = 0;
> +	int lun_to_grow = -1;
> +	u64 qTotalRawDeviceCapacity;
> +	u16 wEnhanced1CapAdjFac, wEnhanced2CapAdjFac;
> +	u32 dEnhanced1MaxNAllocU, dEnhanced2MaxNAllocU;
> +	size_t alloc_units, units_to_create = 0;
> +	size_t capacity_to_alloc_factor;
> +	size_t enhanced1_units = 0, enhanced2_units = 0;
> +	size_t conversion_ratio = 1;
> +	u8 *pt;
> +	u32 blocks_per_alloc_unit = 1024;
> +	int geo_len = hba->desc_size.geom_desc;
> +	u8 geo_buf[hba->desc_size.geom_desc];
> +	unsigned int max_partitions = 8;
> +
> +	WARN_ON(!hba || !cfg);
> +
> +	ret = ufshcd_read_geometry_desc(hba, geo_buf, geo_len);
> +	if (ret) {
> +		dev_err(hba->dev, "%s: Failed getting geometry_desc %d\n",
> +			__func__, ret);
> +		return ret;
> +	}
> +
> +	/*
> +	 * Get Geomtric parameters like total configurable memory
> +	 * quantity (Offset 0x04 to 0x0b), Capacity Adjustment
> +	 * Factors (Offset 0x30, 0x31, 0x36, 0x37), Max allocation
> +	 * units (Offset 0x2c to 0x2f, 0x32 to 0x35) used to configure
> +	 * the device logical units.
> +	 */
> +	qTotalRawDeviceCapacity = get_unaligned_be64(&geo_buf[0x04]);
> +	wEnhanced1CapAdjFac = get_unaligned_be16(&geo_buf[0x30]);
> +	wEnhanced2CapAdjFac = get_unaligned_be16(&geo_buf[0x36]);
> +	dEnhanced1MaxNAllocU = get_unaligned_be32(&geo_buf[0x2c]);
> +	dEnhanced2MaxNAllocU = get_unaligned_be32(&geo_buf[0x32]);
> +
> +	capacity_to_alloc_factor =
> +		(blocks_per_alloc_unit * UFS_BLOCK_SIZE) / 512;
> +
> +	if (qTotalRawDeviceCapacity % capacity_to_alloc_factor != 0) {
> +		dev_err(hba->dev,
> +			"%s: Raw capacity(%llu) not multiple of alloc factor(%zu)\n",
> +			__func__, qTotalRawDeviceCapacity,
> +			capacity_to_alloc_factor);
> +		return -EINVAL;
> +	}
> +	alloc_units = (qTotalRawDeviceCapacity / capacity_to_alloc_factor);
> +	units_to_create = 0;
> +	enhanced1_units = enhanced2_units = 0;
> +
> +	/*
> +	 * Calculate number of allocation units to be assigned to a logical unit
> +	 * considering the capacity adjustment factor of respective memory type.
> +	 */
> +	for (i = 0; i < (max_partitions - 1) &&
> +		units_to_create <= alloc_units; i++) {
> +		if ((cfg->unit[i].dNumAllocUnits % blocks_per_alloc_unit) == 0)
> +			cfg->unit[i].dNumAllocUnits /= blocks_per_alloc_unit;
> +		else
> +			cfg->unit[i].dNumAllocUnits =
> +			cfg->unit[i].dNumAllocUnits / blocks_per_alloc_unit + 1;
> +
> +		if (cfg->unit[i].bMemoryType == 0)
> +			units_to_create += cfg->unit[i].dNumAllocUnits;
> +		else if (cfg->unit[i].bMemoryType == 3) {
> +			enhanced1_units += cfg->unit[i].dNumAllocUnits;
> +			cfg->unit[i].dNumAllocUnits *=
> +				(wEnhanced1CapAdjFac / 0x100);
> +			units_to_create += cfg->unit[i].dNumAllocUnits;
> +		} else if (cfg->unit[i].bMemoryType == 4) {
> +			enhanced2_units += cfg->unit[i].dNumAllocUnits;
> +			cfg->unit[i].dNumAllocUnits *=
> +				(wEnhanced1CapAdjFac / 0x100);
> +			units_to_create += cfg->unit[i].dNumAllocUnits;
> +		} else {
> +			dev_err(hba->dev, "%s: Unsupported memory type %d\n",
> +				__func__, cfg->unit[i].bMemoryType);
> +			return -EINVAL;
> +		}
> +	}
> +	if (enhanced1_units > dEnhanced1MaxNAllocU) {
> +		dev_err(hba->dev, "%s: size %zu exceeds max enhanced1 area size %u\n",
> +			__func__, enhanced1_units, dEnhanced1MaxNAllocU);
> +		return -ERANGE;
> +	}
> +	if (enhanced2_units > dEnhanced2MaxNAllocU) {
> +		dev_err(hba->dev, "%s: size %zu exceeds max enhanced2 area size %u\n",
> +			__func__, enhanced2_units, dEnhanced2MaxNAllocU);
> +		return -ERANGE;
> +	}
> +	if (units_to_create > alloc_units) {
> +		dev_err(hba->dev, "%s: Specified size %zu exceeds device size %zu\n",
> +			__func__, units_to_create, alloc_units);
> +		return -ERANGE;
> +	}
> +	lun_to_grow = cfg->lun_to_grow;
> +	if (lun_to_grow != -1) {
> +		if (cfg->unit[i].bMemoryType == 0)
> +			conversion_ratio = 1;
> +		else if (cfg->unit[i].bMemoryType == 3)
> +			conversion_ratio = (wEnhanced1CapAdjFac / 0x100);
> +		else if (cfg->unit[i].bMemoryType == 4)
> +			conversion_ratio = (wEnhanced2CapAdjFac / 0x100);
> +
> +		cfg->unit[lun_to_grow].dNumAllocUnits +=
> +		((alloc_units - units_to_create) / conversion_ratio);
> +		dev_dbg(hba->dev, "%s: conversion_ratio %zu for lun %d\n",
> +			__func__, conversion_ratio, i);
> +	}
> +
> +	/* Fill in the buffer with configuration data */
> +	pt = desc_buf;
> +	*pt++ = 0x90;        // bLength
> +	*pt++ = 0x01;        // bDescriptorType
> +	*pt++ = 0;           // Reserved in UFS2.0 and onward
> +	*pt++ = cfg->bBootEnable;
> +	*pt++ = cfg->bDescrAccessEn;
> +	*pt++ = cfg->bInitPowerMode;
> +	*pt++ = cfg->bHighPriorityLUN;
> +	*pt++ = cfg->bSecureRemovalType;
> +	*pt++ = cfg->bInitActiveICCLevel;
> +	put_unaligned_be16(cfg->wPeriodicRTCUpdate, pt);
> +	pt = pt + 7; // Reserved fields set to 0
> +
> +	/* Fill in the buffer with per logical unit data */
> +	for (i = 0; i < UFS_UPIU_MAX_GENERAL_LUN; i++) {
> +		*pt++ = cfg->unit[i].bLUEnable;
> +		*pt++ = cfg->unit[i].bBootLunID;
> +		*pt++ = cfg->unit[i].bLUWriteProtect;
> +		*pt++ = cfg->unit[i].bMemoryType;
> +		put_unaligned_be32(cfg->unit[i].dNumAllocUnits, pt);
> +		pt = pt + 4;
> +		*pt++ = cfg->unit[i].bDataReliability;
> +		*pt++ = cfg->unit[i].bLogicalBlockSize;
> +		*pt++ = cfg->unit[i].bProvisioningType;
> +		put_unaligned_be16(cfg->unit[i].wContextCapabilities, pt);
> +		pt = pt + 5; // Reserved fields set to 0
> +	}
> +
> +	ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_WRITE_DESC,
> +		QUERY_DESC_IDN_CONFIGURATION, 0, 0, desc_buf, &buff_len);
> +
> +	if (ret) {
> +		dev_err(hba->dev, "%s: Failed writing descriptor. desc_idn %d, 
> +opcode %x
> ret %d\n",
> +		__func__, QUERY_DESC_IDN_CONFIGURATION,
> +		UPIU_QUERY_OPCODE_WRITE_DESC, ret);
> +		return ret;
> +	}

To support stable operation and prevent race condition, how about adding some functions during in runtime provisioning?.
pm_runtime_get/put_sync and scsi_block/unblock_requests.
How you thought?.
[Sayali] Agreed. Will check and update in next patch set.

> +
> +	if (cfg->bConfigDescrLock == 1) {
> +		ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_WRITE_ATTR,
> +		QUERY_ATTR_IDN_CONF_DESC_LOCK, 0, 0, &cfg->bConfigDescrLock);
> +		if (ret)
> +			dev_err(hba->dev, "%s: Failed writing bConfigDescrLock %d\n",
> +				__func__, ret);
> +	}
> +
> +	return ret;
> +}
> +
> +/**
>   * ufshcd_probe_hba - probe hba to detect device and initialize
>   * @hba: per-adapter instance
>   *
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h 
> index b026ad8..816b674 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -549,6 +549,7 @@ struct ufs_hba {
>  	unsigned int irq;
>  	bool is_irq_enabled;
>  	u32 dev_ref_clk_freq;
> +	struct ufs_config_descr cfgs;
>
>  	/* Interrupt aggregation support is broken */
>  	#define UFSHCD_QUIRK_BROKEN_INTR_AGGR			0x1
> @@ -866,6 +867,7 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, 
> int desc_index,
>
>  int ufshcd_hold(struct ufs_hba *hba, bool async);  void 
> ufshcd_release(struct ufs_hba *hba);
> +int ufshcd_do_config_device(struct ufs_hba *hba);
>
>  int ufshcd_map_desc_id_to_length(struct ufs_hba *hba, enum desc_idn 
> desc_id,
>  	int *desc_length);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
> Forum, a Linux Foundation Collaborative Project
>
>

BR,
Kyuho Choi

[PATCH V3 3/3] scsi: ufs: Add configfs support for ufs provisioning

[Sayali Lokhande]

Add configfs support to provision ufs device at runtime.
Usage:
echo <desc_buf> > /config/ufshcd/ufs_provision
To check provisioning status:
cat /config/ufshcd/ufs_provision
1 -> Success (Reboot device to check updated provisioning)

Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
---
 Documentation/ABI/testing/configfs-driver-ufs |  14 ++
 drivers/scsi/ufs/Makefile                     |   1 +
 drivers/scsi/ufs/ufs-configfs.c               | 191 ++++++++++++++++++++++++++
 drivers/scsi/ufs/ufs.h                        |   2 +
 drivers/scsi/ufs/ufshcd.c                     |   2 +
 drivers/scsi/ufs/ufshcd.h                     |   7 +
 6 files changed, 217 insertions(+)
 create mode 100644 Documentation/ABI/testing/configfs-driver-ufs
 create mode 100644 drivers/scsi/ufs/ufs-configfs.c

diff --git a/Documentation/ABI/testing/configfs-driver-ufs b/Documentation/ABI/testing/configfs-driver-ufs
new file mode 100644
index 0000000..a17fe15
--- /dev/null
+++ b/Documentation/ABI/testing/configfs-driver-ufs
@@ -0,0 +1,14 @@
+What:		/config/ufshcd/ufs_provision
+Date:		Jun 2018
+KernelVersion:	4.14
+Description:
+		This file shows the status of runtime ufs provisioning.
+		This can be used to provision ufs device if bConfigDescrLock is 0.
+		Configuration buffer needs to be written in space separated format
+		specificied as:
+		echo <bNumberLU> <bBootEnable> <bDescrAccessEn> <bInitPowerMode>
+		<bHighPriorityLUN> <bSecureRemovalType> <bInitActiveICCLevel>
+		<wPeriodicRTCUpdate> <bConfigDescrLock> <LUNum> <bLUEnable>
+		<bBootLunID> <size_in_kb> <bDataReliability> <bLUWriteProtect>
+		<bMemoryType> <bLogicalBlockSize> <bProvisioningType>
+		<wContextCapabilities> > /config/ufshcd/ufs_provision
diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
index 918f579..d438e74 100644
--- a/drivers/scsi/ufs/Makefile
+++ b/drivers/scsi/ufs/Makefile
@@ -5,5 +5,6 @@ obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o tc-d
 obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
 obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
 ufshcd-core-objs := ufshcd.o ufs-sysfs.o
+obj-$(CONFIG_CONFIGFS_FS) += ufs-configfs.o
 obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
 obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
diff --git a/drivers/scsi/ufs/ufs-configfs.c b/drivers/scsi/ufs/ufs-configfs.c
new file mode 100644
index 0000000..7655e6b
--- /dev/null
+++ b/drivers/scsi/ufs/ufs-configfs.c
@@ -0,0 +1,191 @@
+/*
+ * drivers/scsi/ufs/ufs-configfs.c
+ *
+ * Copyright (c) 2018, Qualcomm Technologies, Inc.
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/string.h>
+#include <asm/unaligned.h>
+#include <linux/configfs.h>
+
+#include "ufs.h"
+#include "ufshcd.h"
+
+struct ufs_hba *hba;
+
+static ssize_t ufs_provision_show(struct config_item *item, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "provision_enabled = %x\n",
+		hba->provision_enabled);
+}
+
+ssize_t ufshcd_desc_configfs_store(const char *buf, size_t count)
+{
+	struct ufs_config_descr *cfg = &hba->cfgs;
+	char *strbuf;
+	char *strbuf_copy;
+	int desc_buf[count];
+	int *pt;
+	char *token;
+	int i, ret;
+	int value, commit = 0;
+	int num_luns = 0;
+	int KB_per_block = 4;
+
+	/* reserve one byte for null termination */
+	strbuf = kmalloc(count + 1, GFP_KERNEL);
+	if (!strbuf)
+		return -ENOMEM;
+
+	strbuf_copy = strbuf;
+	strlcpy(strbuf, buf, count + 1);
+	memset(desc_buf, 0, count);
+
+	/* Just return if bConfigDescrLock is already set */
+	ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,
+		QUERY_ATTR_IDN_CONF_DESC_LOCK, 0, 0, &cfg->bConfigDescrLock);
+	if (ret) {
+		dev_err(hba->dev, "%s: Failed reading bConfigDescrLock %d, cannot re-provision device!\n",
+			__func__, ret);
+		hba->provision_enabled = 0;
+		goto out;
+	}
+	if (cfg->bConfigDescrLock == 1) {
+		dev_err(hba->dev, "%s: bConfigDescrLock already set to %u, cannot re-provision device!\n",
+		__func__, cfg->bConfigDescrLock);
+		hba->provision_enabled = 0;
+		goto out;
+	}
+
+	for (i = 0; i < count; i++) {
+		token = strsep(&strbuf, " ");
+		if (!token && i) {
+			num_luns = desc_buf[i-1];
+			dev_dbg(hba->dev, "%s: token %s, num_luns %d\n",
+				__func__, token, num_luns);
+			if (num_luns > 8) {
+				dev_err(hba->dev, "%s: Invalid num_luns %d\n",
+				__func__, num_luns);
+				hba->provision_enabled = 0;
+				goto out;
+			}
+			break;
+		}
+
+		ret = kstrtoint(token, 0, &value);
+		if (ret) {
+			dev_err(hba->dev, "%s: kstrtoint failed %d %s\n",
+				__func__, ret, token);
+			break;
+		}
+		desc_buf[i] = value;
+		dev_dbg(hba->dev, " desc_buf[%d] 0x%x", i, desc_buf[i]);
+	}
+
+	/* Fill in the descriptors with parsed configuration data */
+	pt = desc_buf;
+	cfg->bNumberLU = *pt++;
+	cfg->bBootEnable = *pt++;
+	cfg->bDescrAccessEn = *pt++;
+	cfg->bInitPowerMode = *pt++;
+	cfg->bHighPriorityLUN = *pt++;
+	cfg->bSecureRemovalType = *pt++;
+	cfg->bInitActiveICCLevel = *pt++;
+	cfg->wPeriodicRTCUpdate = *pt++;
+	cfg->bConfigDescrLock = *pt++;
+	dev_dbg(hba->dev, "%s: %u %u %u %u %u %u %u %u %u\n", __func__,
+	cfg->bNumberLU, cfg->bBootEnable, cfg->bDescrAccessEn,
+	cfg->bInitPowerMode, cfg->bHighPriorityLUN, cfg->bSecureRemovalType,
+	cfg->bInitActiveICCLevel, cfg->wPeriodicRTCUpdate,
+	cfg->bConfigDescrLock);
+
+	for (i = 0; i < num_luns; i++) {
+		cfg->unit[i].LUNum = *pt++;
+		cfg->unit[i].bLUEnable = *pt++;
+		cfg->unit[i].bBootLunID = *pt++;
+		/* dNumAllocUnits = size_in_kb/KB_per_block */
+		cfg->unit[i].dNumAllocUnits = (u32)(*pt++ / KB_per_block);
+		cfg->unit[i].bDataReliability = *pt++;
+		cfg->unit[i].bLUWriteProtect = *pt++;
+		cfg->unit[i].bMemoryType = *pt++;
+		cfg->unit[i].bLogicalBlockSize = *pt++;
+		cfg->unit[i].bProvisioningType = *pt++;
+		cfg->unit[i].wContextCapabilities = *pt++;
+	}
+
+	cfg->lun_to_grow = *pt++;
+	commit = *pt++;
+	cfg->num_luns = *pt;
+	dev_dbg(hba->dev, "%s: lun_to_grow %u, commit %u num_luns %u\n",
+		__func__, cfg->lun_to_grow, commit, cfg->num_luns);
+	if (commit == 1) {
+		ret = ufshcd_do_config_device(hba);
+		if (!ret) {
+			hba->provision_enabled = 1;
+			dev_err(hba->dev,
+			"%s: UFS Provisioning completed,num_luns %u, reboot now !\n",
+			__func__, cfg->num_luns);
+		}
+	} else
+		dev_err(hba->dev, "%s: Invalid commit %u\n", __func__, commit);
+out:
+	kfree(strbuf_copy);
+	return count;
+}
+
+static ssize_t ufs_provision_store(struct config_item *item,
+		const char *buf, size_t count)
+{
+	return ufshcd_desc_configfs_store(buf, count);
+}
+
+static struct configfs_attribute ufshcd_attr_provision = {
+	.ca_name	= "ufs_provision",
+	.ca_mode	= S_IRUGO | S_IWUGO,
+	.ca_owner	= THIS_MODULE,
+	.show		= ufs_provision_show,
+	.store		= ufs_provision_store,
+};
+
+static struct configfs_attribute *ufshcd_attrs[] = {
+	&ufshcd_attr_provision,
+	NULL,
+};
+
+static struct config_item_type ufscfg_type = {
+	.ct_attrs	= ufshcd_attrs,
+	.ct_owner	= THIS_MODULE,
+};
+
+static struct configfs_subsystem ufscfg_subsys = {
+	.su_group = {
+		.cg_item = {
+			.ci_namebuf = "ufshcd",
+			.ci_type = &ufscfg_type,
+		},
+	},
+};
+
+int ufshcd_configfs_init(struct ufs_hba *hba_ufs)
+{
+	int ret;
+	struct configfs_subsystem *subsys = &ufscfg_subsys;
+	hba = hba_ufs;
+
+	config_group_init(&subsys->su_group);
+	mutex_init(&subsys->su_mutex);
+	ret = configfs_register_subsystem(subsys);
+	if (ret) {
+		pr_err("Error %d while registering subsystem %s\n",
+		       ret,
+		       subsys->su_group.cg_item.ci_namebuf);
+	}
+	return ret;
+}
+
+void ufshcd_configfs_exit(void)
+{
+	configfs_unregister_subsystem(&ufscfg_subsys);
+}
\ No newline at end of file
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 1f99904..0b497fc 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -427,6 +427,7 @@ enum {
 };
 
 struct ufs_unit_desc {
+	u8	   LUNum;
 	u8     bLUEnable;              /* 1 for enabled LU */
 	u8     bBootLunID;             /* 0 for using this LU for boot */
 	u8     bLUWriteProtect;        /* 1 = power on WP, 2 = permanent WP */
@@ -451,6 +452,7 @@ struct ufs_config_descr {
 	u32    qVendorConfigCode;      /* Vendor specific configuration code */
 	struct ufs_unit_desc unit[8];
 	u8	lun_to_grow;
+	u8 num_luns;
 };
 
 /* Task management service response */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c0235a4..63d6532 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7854,6 +7854,7 @@ int ufshcd_shutdown(struct ufs_hba *hba)
 void ufshcd_remove(struct ufs_hba *hba)
 {
 	ufs_sysfs_remove_nodes(hba->dev);
+	ufshcd_configfs_exit();
 	scsi_remove_host(hba->host);
 	/* disable interrupts */
 	ufshcd_disable_intr(hba, hba->intr_mask);
@@ -8107,6 +8108,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 
 	async_schedule(ufshcd_async_scan, hba);
 	ufs_sysfs_add_nodes(hba->dev);
+	ufshcd_configfs_init(hba);
 
 	return 0;
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 816b674..fb254c8 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -41,6 +41,7 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/configfs.h>
 #include <linux/io.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
@@ -550,6 +551,7 @@ struct ufs_hba {
 	bool is_irq_enabled;
 	u32 dev_ref_clk_freq;
 	struct ufs_config_descr cfgs;
+	bool provision_enabled;
 
 	/* Interrupt aggregation support is broken */
 	#define UFSHCD_QUIRK_BROKEN_INTR_AGGR			0x1
@@ -867,7 +869,12 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index,
 
 int ufshcd_hold(struct ufs_hba *hba, bool async);
 void ufshcd_release(struct ufs_hba *hba);
+
+/* Expose UFS configfs API's */
 int ufshcd_do_config_device(struct ufs_hba *hba);
+ssize_t ufshcd_desc_configfs_store(const char *buf, size_t count);
+int ufshcd_configfs_init(struct ufs_hba *hba_ufs);
+void ufshcd_configfs_exit(void);
 
 int ufshcd_map_desc_id_to_length(struct ufs_hba *hba, enum desc_idn desc_id,
 	int *desc_length);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH V3 3/3] scsi: ufs: Add configfs support for ufs provisioning

[Sayali Lokhande]

Add configfs support to provision ufs device at runtime.
Usage:
echo <desc_buf> > /config/ufshcd/ufs_provision
To check provisioning status:
cat /config/ufshcd/ufs_provision
1 -> Success (Reboot device to check updated provisioning)

Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
---
 Documentation/ABI/testing/configfs-driver-ufs |  14 ++
 drivers/scsi/ufs/Makefile                     |   1 +
 drivers/scsi/ufs/ufs-configfs.c               | 191 ++++++++++++++++++++++++++
 drivers/scsi/ufs/ufs.h                        |   2 +
 drivers/scsi/ufs/ufshcd.c                     |   2 +
 drivers/scsi/ufs/ufshcd.h                     |   7 +
 6 files changed, 217 insertions(+)
 create mode 100644 Documentation/ABI/testing/configfs-driver-ufs
 create mode 100644 drivers/scsi/ufs/ufs-configfs.c

diff --git a/Documentation/ABI/testing/configfs-driver-ufs b/Documentation/ABI/testing/configfs-driver-ufs
new file mode 100644
index 0000000..a17fe15
--- /dev/null
+++ b/Documentation/ABI/testing/configfs-driver-ufs
@@ -0,0 +1,14 @@
+What:		/config/ufshcd/ufs_provision
+Date:		Jun 2018
+KernelVersion:	4.14
+Description:
+		This file shows the status of runtime ufs provisioning.
+		This can be used to provision ufs device if bConfigDescrLock is 0.
+		Configuration buffer needs to be written in space separated format
+		specificied as:
+		echo <bNumberLU> <bBootEnable> <bDescrAccessEn> <bInitPowerMode>
+		<bHighPriorityLUN> <bSecureRemovalType> <bInitActiveICCLevel>
+		<wPeriodicRTCUpdate> <bConfigDescrLock> <LUNum> <bLUEnable>
+		<bBootLunID> <size_in_kb> <bDataReliability> <bLUWriteProtect>
+		<bMemoryType> <bLogicalBlockSize> <bProvisioningType>
+		<wContextCapabilities> > /config/ufshcd/ufs_provision
diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
index 918f579..d438e74 100644
--- a/drivers/scsi/ufs/Makefile
+++ b/drivers/scsi/ufs/Makefile
@@ -5,5 +5,6 @@ obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o tc-d
 obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
 obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
 ufshcd-core-objs := ufshcd.o ufs-sysfs.o
+obj-$(CONFIG_CONFIGFS_FS) += ufs-configfs.o
 obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
 obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
diff --git a/drivers/scsi/ufs/ufs-configfs.c b/drivers/scsi/ufs/ufs-configfs.c
new file mode 100644
index 0000000..7655e6b
--- /dev/null
+++ b/drivers/scsi/ufs/ufs-configfs.c
@@ -0,0 +1,191 @@
+/*
+ * drivers/scsi/ufs/ufs-configfs.c
+ *
+ * Copyright (c) 2018, Qualcomm Technologies, Inc.
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/string.h>
+#include <asm/unaligned.h>
+#include <linux/configfs.h>
+
+#include "ufs.h"
+#include "ufshcd.h"
+
+struct ufs_hba *hba;
+
+static ssize_t ufs_provision_show(struct config_item *item, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "provision_enabled = %x\n",
+		hba->provision_enabled);
+}
+
+ssize_t ufshcd_desc_configfs_store(const char *buf, size_t count)
+{
+	struct ufs_config_descr *cfg = &hba->cfgs;
+	char *strbuf;
+	char *strbuf_copy;
+	int desc_buf[count];
+	int *pt;
+	char *token;
+	int i, ret;
+	int value, commit = 0;
+	int num_luns = 0;
+	int KB_per_block = 4;
+
+	/* reserve one byte for null termination */
+	strbuf = kmalloc(count + 1, GFP_KERNEL);
+	if (!strbuf)
+		return -ENOMEM;
+
+	strbuf_copy = strbuf;
+	strlcpy(strbuf, buf, count + 1);
+	memset(desc_buf, 0, count);
+
+	/* Just return if bConfigDescrLock is already set */
+	ret = ufshcd_query_attr(hba, UPIU_QUERY_OPCODE_READ_ATTR,
+		QUERY_ATTR_IDN_CONF_DESC_LOCK, 0, 0, &cfg->bConfigDescrLock);
+	if (ret) {
+		dev_err(hba->dev, "%s: Failed reading bConfigDescrLock %d, cannot re-provision device!\n",
+			__func__, ret);
+		hba->provision_enabled = 0;
+		goto out;
+	}
+	if (cfg->bConfigDescrLock == 1) {
+		dev_err(hba->dev, "%s: bConfigDescrLock already set to %u, cannot re-provision device!\n",
+		__func__, cfg->bConfigDescrLock);
+		hba->provision_enabled = 0;
+		goto out;
+	}
+
+	for (i = 0; i < count; i++) {
+		token = strsep(&strbuf, " ");
+		if (!token && i) {
+			num_luns = desc_buf[i-1];
+			dev_dbg(hba->dev, "%s: token %s, num_luns %d\n",
+				__func__, token, num_luns);
+			if (num_luns > 8) {
+				dev_err(hba->dev, "%s: Invalid num_luns %d\n",
+				__func__, num_luns);
+				hba->provision_enabled = 0;
+				goto out;
+			}
+			break;
+		}
+
+		ret = kstrtoint(token, 0, &value);
+		if (ret) {
+			dev_err(hba->dev, "%s: kstrtoint failed %d %s\n",
+				__func__, ret, token);
+			break;
+		}
+		desc_buf[i] = value;
+		dev_dbg(hba->dev, " desc_buf[%d] 0x%x", i, desc_buf[i]);
+	}
+
+	/* Fill in the descriptors with parsed configuration data */
+	pt = desc_buf;
+	cfg->bNumberLU = *pt++;
+	cfg->bBootEnable = *pt++;
+	cfg->bDescrAccessEn = *pt++;
+	cfg->bInitPowerMode = *pt++;
+	cfg->bHighPriorityLUN = *pt++;
+	cfg->bSecureRemovalType = *pt++;
+	cfg->bInitActiveICCLevel = *pt++;
+	cfg->wPeriodicRTCUpdate = *pt++;
+	cfg->bConfigDescrLock = *pt++;
+	dev_dbg(hba->dev, "%s: %u %u %u %u %u %u %u %u %u\n", __func__,
+	cfg->bNumberLU, cfg->bBootEnable, cfg->bDescrAccessEn,
+	cfg->bInitPowerMode, cfg->bHighPriorityLUN, cfg->bSecureRemovalType,
+	cfg->bInitActiveICCLevel, cfg->wPeriodicRTCUpdate,
+	cfg->bConfigDescrLock);
+
+	for (i = 0; i < num_luns; i++) {
+		cfg->unit[i].LUNum = *pt++;
+		cfg->unit[i].bLUEnable = *pt++;
+		cfg->unit[i].bBootLunID = *pt++;
+		/* dNumAllocUnits = size_in_kb/KB_per_block */
+		cfg->unit[i].dNumAllocUnits = (u32)(*pt++ / KB_per_block);
+		cfg->unit[i].bDataReliability = *pt++;
+		cfg->unit[i].bLUWriteProtect = *pt++;
+		cfg->unit[i].bMemoryType = *pt++;
+		cfg->unit[i].bLogicalBlockSize = *pt++;
+		cfg->unit[i].bProvisioningType = *pt++;
+		cfg->unit[i].wContextCapabilities = *pt++;
+	}
+
+	cfg->lun_to_grow = *pt++;
+	commit = *pt++;
+	cfg->num_luns = *pt;
+	dev_dbg(hba->dev, "%s: lun_to_grow %u, commit %u num_luns %u\n",
+		__func__, cfg->lun_to_grow, commit, cfg->num_luns);
+	if (commit == 1) {
+		ret = ufshcd_do_config_device(hba);
+		if (!ret) {
+			hba->provision_enabled = 1;
+			dev_err(hba->dev,
+			"%s: UFS Provisioning completed,num_luns %u, reboot now !\n",
+			__func__, cfg->num_luns);
+		}
+	} else
+		dev_err(hba->dev, "%s: Invalid commit %u\n", __func__, commit);
+out:
+	kfree(strbuf_copy);
+	return count;
+}
+
+static ssize_t ufs_provision_store(struct config_item *item,
+		const char *buf, size_t count)
+{
+	return ufshcd_desc_configfs_store(buf, count);
+}
+
+static struct configfs_attribute ufshcd_attr_provision = {
+	.ca_name	= "ufs_provision",
+	.ca_mode	= S_IRUGO | S_IWUGO,
+	.ca_owner	= THIS_MODULE,
+	.show		= ufs_provision_show,
+	.store		= ufs_provision_store,
+};
+
+static struct configfs_attribute *ufshcd_attrs[] = {
+	&ufshcd_attr_provision,
+	NULL,
+};
+
+static struct config_item_type ufscfg_type = {
+	.ct_attrs	= ufshcd_attrs,
+	.ct_owner	= THIS_MODULE,
+};
+
+static struct configfs_subsystem ufscfg_subsys = {
+	.su_group = {
+		.cg_item = {
+			.ci_namebuf = "ufshcd",
+			.ci_type = &ufscfg_type,
+		},
+	},
+};
+
+int ufshcd_configfs_init(struct ufs_hba *hba_ufs)
+{
+	int ret;
+	struct configfs_subsystem *subsys = &ufscfg_subsys;
+	hba = hba_ufs;
+
+	config_group_init(&subsys->su_group);
+	mutex_init(&subsys->su_mutex);
+	ret = configfs_register_subsystem(subsys);
+	if (ret) {
+		pr_err("Error %d while registering subsystem %s\n",
+		       ret,
+		       subsys->su_group.cg_item.ci_namebuf);
+	}
+	return ret;
+}
+
+void ufshcd_configfs_exit(void)
+{
+	configfs_unregister_subsystem(&ufscfg_subsys);
+}
\ No newline at end of file
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 1f99904..0b497fc 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -427,6 +427,7 @@ enum {
 };
 
 struct ufs_unit_desc {
+	u8	   LUNum;
 	u8     bLUEnable;              /* 1 for enabled LU */
 	u8     bBootLunID;             /* 0 for using this LU for boot */
 	u8     bLUWriteProtect;        /* 1 = power on WP, 2 = permanent WP */
@@ -451,6 +452,7 @@ struct ufs_config_descr {
 	u32    qVendorConfigCode;      /* Vendor specific configuration code */
 	struct ufs_unit_desc unit[8];
 	u8	lun_to_grow;
+	u8 num_luns;
 };
 
 /* Task management service response */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index c0235a4..63d6532 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7854,6 +7854,7 @@ int ufshcd_shutdown(struct ufs_hba *hba)
 void ufshcd_remove(struct ufs_hba *hba)
 {
 	ufs_sysfs_remove_nodes(hba->dev);
+	ufshcd_configfs_exit();
 	scsi_remove_host(hba->host);
 	/* disable interrupts */
 	ufshcd_disable_intr(hba, hba->intr_mask);
@@ -8107,6 +8108,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 
 	async_schedule(ufshcd_async_scan, hba);
 	ufs_sysfs_add_nodes(hba->dev);
+	ufshcd_configfs_init(hba);
 
 	return 0;
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 816b674..fb254c8 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -41,6 +41,7 @@
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
+#include <linux/configfs.h>
 #include <linux/io.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
@@ -550,6 +551,7 @@ struct ufs_hba {
 	bool is_irq_enabled;
 	u32 dev_ref_clk_freq;
 	struct ufs_config_descr cfgs;
+	bool provision_enabled;
 
 	/* Interrupt aggregation support is broken */
 	#define UFSHCD_QUIRK_BROKEN_INTR_AGGR			0x1
@@ -867,7 +869,12 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index,
 
 int ufshcd_hold(struct ufs_hba *hba, bool async);
 void ufshcd_release(struct ufs_hba *hba);
+
+/* Expose UFS configfs API's */
 int ufshcd_do_config_device(struct ufs_hba *hba);
+ssize_t ufshcd_desc_configfs_store(const char *buf, size_t count);
+int ufshcd_configfs_init(struct ufs_hba *hba_ufs);
+void ufshcd_configfs_exit(void);
 
 int ufshcd_map_desc_id_to_length(struct ufs_hba *hba, enum desc_idn desc_id,
 	int *desc_length);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH V3 3/3] scsi: ufs: Add configfs support for ufs provisioning

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1374 bytes --]

Hi Sayali,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mkp-scsi/for-next]
[also build test ERROR on v4.17 next-20180614]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sayali-Lokhande/Add-ufs-provisioning-support-in-driver/20180614-211513
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 8.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.1.0 make.cross ARCH=ia64 

All errors (new ones prefixed by >>):

   ERROR: "ia64_delay_loop" [drivers/spi/spi-thunderx.ko] undefined!
>> ERROR: "ufshcd_configfs_init" [drivers/scsi/ufs/ufshcd-core.ko] undefined!
>> ERROR: "ufshcd_configfs_exit" [drivers/scsi/ufs/ufshcd-core.ko] undefined!
   ERROR: "__sw_hweight8" [drivers/net/wireless/mediatek/mt76/mt76.ko] undefined!
   ERROR: "ia64_delay_loop" [drivers/net/phy/mdio-cavium.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 49872 bytes --]

Re: [PATCH V3 3/3] scsi: ufs: Add configfs support for ufs provisioning

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1374 bytes --]

Hi Sayali,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mkp-scsi/for-next]
[also build test ERROR on v4.17 next-20180614]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sayali-Lokhande/Add-ufs-provisioning-support-in-driver/20180614-211513
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 8.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.1.0 make.cross ARCH=ia64 

All errors (new ones prefixed by >>):

   ERROR: "ia64_delay_loop" [drivers/spi/spi-thunderx.ko] undefined!
>> ERROR: "ufshcd_configfs_init" [drivers/scsi/ufs/ufshcd-core.ko] undefined!
>> ERROR: "ufshcd_configfs_exit" [drivers/scsi/ufs/ufshcd-core.ko] undefined!
   ERROR: "__sw_hweight8" [drivers/net/wireless/mediatek/mt76/mt76.ko] undefined!
   ERROR: "ia64_delay_loop" [drivers/net/phy/mdio-cavium.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 49872 bytes --]

Re: [PATCH V3 3/3] scsi: ufs: Add configfs support for ufs provisioning

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 7798 bytes --]

Hi Sayali,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mkp-scsi/for-next]
[also build test ERROR on v4.17 next-20180614]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sayali-Lokhande/Add-ufs-provisioning-support-in-driver/20180614-211513
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-randconfig-s5-06142227 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/scsi/ufs/ufshcd.o: In function `ufshcd_init':
>> drivers/scsi/ufs/ufshcd.c:8311: undefined reference to `ufshcd_configfs_init'
   drivers/scsi/ufs/ufshcd.o: In function `ufshcd_remove':
>> drivers/scsi/ufs/ufshcd.c:8057: undefined reference to `ufshcd_configfs_exit'

vim +8311 drivers/scsi/ufs/ufshcd.c

  8133	
  8134	/**
  8135	 * ufshcd_init - Driver initialization routine
  8136	 * @hba: per-adapter instance
  8137	 * @mmio_base: base register address
  8138	 * @irq: Interrupt line of device
  8139	 * Returns 0 on success, non-zero value on failure
  8140	 */
  8141	int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
  8142	{
  8143		int err;
  8144		struct Scsi_Host *host = hba->host;
  8145		struct device *dev = hba->dev;
  8146	
  8147		if (!mmio_base) {
  8148			dev_err(hba->dev,
  8149			"Invalid memory reference for mmio_base is NULL\n");
  8150			err = -ENODEV;
  8151			goto out_error;
  8152		}
  8153	
  8154		hba->mmio_base = mmio_base;
  8155		hba->irq = irq;
  8156	
  8157		/* Set descriptor lengths to specification defaults */
  8158		ufshcd_def_desc_sizes(hba);
  8159	
  8160		err = ufshcd_hba_init(hba);
  8161		if (err)
  8162			goto out_error;
  8163	
  8164		/* Read capabilities registers */
  8165		ufshcd_hba_capabilities(hba);
  8166	
  8167		/* Get UFS version supported by the controller */
  8168		hba->ufs_version = ufshcd_get_ufs_version(hba);
  8169	
  8170		if ((hba->ufs_version != UFSHCI_VERSION_10) &&
  8171		    (hba->ufs_version != UFSHCI_VERSION_11) &&
  8172		    (hba->ufs_version != UFSHCI_VERSION_20) &&
  8173		    (hba->ufs_version != UFSHCI_VERSION_21))
  8174			dev_err(hba->dev, "invalid UFS version 0x%x\n",
  8175				hba->ufs_version);
  8176	
  8177		/* Get Interrupt bit mask per version */
  8178		hba->intr_mask = ufshcd_get_intr_mask(hba);
  8179	
  8180		err = ufshcd_set_dma_mask(hba);
  8181		if (err) {
  8182			dev_err(hba->dev, "set dma mask failed\n");
  8183			goto out_disable;
  8184		}
  8185	
  8186		/* Allocate memory for host memory space */
  8187		err = ufshcd_memory_alloc(hba);
  8188		if (err) {
  8189			dev_err(hba->dev, "Memory allocation failed\n");
  8190			goto out_disable;
  8191		}
  8192	
  8193		/* Configure LRB */
  8194		ufshcd_host_memory_configure(hba);
  8195	
  8196		host->can_queue = hba->nutrs;
  8197		host->cmd_per_lun = hba->nutrs;
  8198		host->max_id = UFSHCD_MAX_ID;
  8199		host->max_lun = UFS_MAX_LUNS;
  8200		host->max_channel = UFSHCD_MAX_CHANNEL;
  8201		host->unique_id = host->host_no;
  8202		host->max_cmd_len = MAX_CDB_SIZE;
  8203	
  8204		hba->max_pwr_info.is_valid = false;
  8205	
  8206		/* Initailize wait queue for task management */
  8207		init_waitqueue_head(&hba->tm_wq);
  8208		init_waitqueue_head(&hba->tm_tag_wq);
  8209	
  8210		/* Initialize work queues */
  8211		INIT_WORK(&hba->eh_work, ufshcd_err_handler);
  8212		INIT_WORK(&hba->eeh_work, ufshcd_exception_event_handler);
  8213	
  8214		/* Initialize UIC command mutex */
  8215		mutex_init(&hba->uic_cmd_mutex);
  8216	
  8217		/* Initialize mutex for device management commands */
  8218		mutex_init(&hba->dev_cmd.lock);
  8219	
  8220		init_rwsem(&hba->clk_scaling_lock);
  8221	
  8222		/* Initialize device management tag acquire wait queue */
  8223		init_waitqueue_head(&hba->dev_cmd.tag_wq);
  8224	
  8225		ufshcd_init_clk_gating(hba);
  8226	
  8227		/*
  8228		 * In order to avoid any spurious interrupt immediately after
  8229		 * registering UFS controller interrupt handler, clear any pending UFS
  8230		 * interrupt status and disable all the UFS interrupts.
  8231		 */
  8232		ufshcd_writel(hba, ufshcd_readl(hba, REG_INTERRUPT_STATUS),
  8233			      REG_INTERRUPT_STATUS);
  8234		ufshcd_writel(hba, 0, REG_INTERRUPT_ENABLE);
  8235		/*
  8236		 * Make sure that UFS interrupts are disabled and any pending interrupt
  8237		 * status is cleared before registering UFS interrupt handler.
  8238		 */
  8239		mb();
  8240	
  8241		/* IRQ registration */
  8242		err = devm_request_irq(dev, irq, ufshcd_intr, IRQF_SHARED, UFSHCD, hba);
  8243		if (err) {
  8244			dev_err(hba->dev, "request irq failed\n");
  8245			goto exit_gating;
  8246		} else {
  8247			hba->is_irq_enabled = true;
  8248		}
  8249	
  8250		err = scsi_add_host(host, hba->dev);
  8251		if (err) {
  8252			dev_err(hba->dev, "scsi_add_host failed\n");
  8253			goto exit_gating;
  8254		}
  8255	
  8256		/* Host controller enable */
  8257		err = ufshcd_hba_enable(hba);
  8258		if (err) {
  8259			dev_err(hba->dev, "Host controller enable failed\n");
  8260			ufshcd_print_host_regs(hba);
  8261			ufshcd_print_host_state(hba);
  8262			goto out_remove_scsi_host;
  8263		}
  8264	
  8265		if (ufshcd_is_clkscaling_supported(hba)) {
  8266			char wq_name[sizeof("ufs_clkscaling_00")];
  8267	
  8268			INIT_WORK(&hba->clk_scaling.suspend_work,
  8269				  ufshcd_clk_scaling_suspend_work);
  8270			INIT_WORK(&hba->clk_scaling.resume_work,
  8271				  ufshcd_clk_scaling_resume_work);
  8272	
  8273			snprintf(wq_name, sizeof(wq_name), "ufs_clkscaling_%d",
  8274				 host->host_no);
  8275			hba->clk_scaling.workq = create_singlethread_workqueue(wq_name);
  8276	
  8277			ufshcd_clkscaling_init_sysfs(hba);
  8278		}
  8279	
  8280		/*
  8281		 * Set the default power management level for runtime and system PM.
  8282		 * Default power saving mode is to keep UFS link in Hibern8 state
  8283		 * and UFS device in sleep state.
  8284		 */
  8285		hba->rpm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
  8286							UFS_SLEEP_PWR_MODE,
  8287							UIC_LINK_HIBERN8_STATE);
  8288		hba->spm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
  8289							UFS_SLEEP_PWR_MODE,
  8290							UIC_LINK_HIBERN8_STATE);
  8291	
  8292		/* Set the default auto-hiberate idle timer value to 150 ms */
  8293		if (hba->capabilities & MASK_AUTO_HIBERN8_SUPPORT) {
  8294			hba->ahit = FIELD_PREP(UFSHCI_AHIBERN8_TIMER_MASK, 150) |
  8295				    FIELD_PREP(UFSHCI_AHIBERN8_SCALE_MASK, 3);
  8296		}
  8297	
  8298		/* Hold auto suspend until async scan completes */
  8299		pm_runtime_get_sync(dev);
  8300		atomic_set(&hba->scsi_block_reqs_cnt, 0);
  8301		/*
  8302		 * We are assuming that device wasn't put in sleep/power-down
  8303		 * state exclusively during the boot stage before kernel.
  8304		 * This assumption helps avoid doing link startup twice during
  8305		 * ufshcd_probe_hba().
  8306		 */
  8307		ufshcd_set_ufs_dev_active(hba);
  8308	
  8309		async_schedule(ufshcd_async_scan, hba);
  8310		ufs_sysfs_add_nodes(hba->dev);
> 8311		ufshcd_configfs_init(hba);
  8312	
  8313		return 0;
  8314	
  8315	out_remove_scsi_host:
  8316		scsi_remove_host(hba->host);
  8317	exit_gating:
  8318		ufshcd_exit_clk_gating(hba);
  8319	out_disable:
  8320		hba->is_irq_enabled = false;
  8321		ufshcd_hba_exit(hba);
  8322	out_error:
  8323		return err;
  8324	}
  8325	EXPORT_SYMBOL_GPL(ufshcd_init);
  8326	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30458 bytes --]

Re: [PATCH V3 3/3] scsi: ufs: Add configfs support for ufs provisioning

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 7798 bytes --]

Hi Sayali,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mkp-scsi/for-next]
[also build test ERROR on v4.17 next-20180614]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sayali-Lokhande/Add-ufs-provisioning-support-in-driver/20180614-211513
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-randconfig-s5-06142227 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/scsi/ufs/ufshcd.o: In function `ufshcd_init':
>> drivers/scsi/ufs/ufshcd.c:8311: undefined reference to `ufshcd_configfs_init'
   drivers/scsi/ufs/ufshcd.o: In function `ufshcd_remove':
>> drivers/scsi/ufs/ufshcd.c:8057: undefined reference to `ufshcd_configfs_exit'

vim +8311 drivers/scsi/ufs/ufshcd.c

  8133	
  8134	/**
  8135	 * ufshcd_init - Driver initialization routine
  8136	 * @hba: per-adapter instance
  8137	 * @mmio_base: base register address
  8138	 * @irq: Interrupt line of device
  8139	 * Returns 0 on success, non-zero value on failure
  8140	 */
  8141	int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
  8142	{
  8143		int err;
  8144		struct Scsi_Host *host = hba->host;
  8145		struct device *dev = hba->dev;
  8146	
  8147		if (!mmio_base) {
  8148			dev_err(hba->dev,
  8149			"Invalid memory reference for mmio_base is NULL\n");
  8150			err = -ENODEV;
  8151			goto out_error;
  8152		}
  8153	
  8154		hba->mmio_base = mmio_base;
  8155		hba->irq = irq;
  8156	
  8157		/* Set descriptor lengths to specification defaults */
  8158		ufshcd_def_desc_sizes(hba);
  8159	
  8160		err = ufshcd_hba_init(hba);
  8161		if (err)
  8162			goto out_error;
  8163	
  8164		/* Read capabilities registers */
  8165		ufshcd_hba_capabilities(hba);
  8166	
  8167		/* Get UFS version supported by the controller */
  8168		hba->ufs_version = ufshcd_get_ufs_version(hba);
  8169	
  8170		if ((hba->ufs_version != UFSHCI_VERSION_10) &&
  8171		    (hba->ufs_version != UFSHCI_VERSION_11) &&
  8172		    (hba->ufs_version != UFSHCI_VERSION_20) &&
  8173		    (hba->ufs_version != UFSHCI_VERSION_21))
  8174			dev_err(hba->dev, "invalid UFS version 0x%x\n",
  8175				hba->ufs_version);
  8176	
  8177		/* Get Interrupt bit mask per version */
  8178		hba->intr_mask = ufshcd_get_intr_mask(hba);
  8179	
  8180		err = ufshcd_set_dma_mask(hba);
  8181		if (err) {
  8182			dev_err(hba->dev, "set dma mask failed\n");
  8183			goto out_disable;
  8184		}
  8185	
  8186		/* Allocate memory for host memory space */
  8187		err = ufshcd_memory_alloc(hba);
  8188		if (err) {
  8189			dev_err(hba->dev, "Memory allocation failed\n");
  8190			goto out_disable;
  8191		}
  8192	
  8193		/* Configure LRB */
  8194		ufshcd_host_memory_configure(hba);
  8195	
  8196		host->can_queue = hba->nutrs;
  8197		host->cmd_per_lun = hba->nutrs;
  8198		host->max_id = UFSHCD_MAX_ID;
  8199		host->max_lun = UFS_MAX_LUNS;
  8200		host->max_channel = UFSHCD_MAX_CHANNEL;
  8201		host->unique_id = host->host_no;
  8202		host->max_cmd_len = MAX_CDB_SIZE;
  8203	
  8204		hba->max_pwr_info.is_valid = false;
  8205	
  8206		/* Initailize wait queue for task management */
  8207		init_waitqueue_head(&hba->tm_wq);
  8208		init_waitqueue_head(&hba->tm_tag_wq);
  8209	
  8210		/* Initialize work queues */
  8211		INIT_WORK(&hba->eh_work, ufshcd_err_handler);
  8212		INIT_WORK(&hba->eeh_work, ufshcd_exception_event_handler);
  8213	
  8214		/* Initialize UIC command mutex */
  8215		mutex_init(&hba->uic_cmd_mutex);
  8216	
  8217		/* Initialize mutex for device management commands */
  8218		mutex_init(&hba->dev_cmd.lock);
  8219	
  8220		init_rwsem(&hba->clk_scaling_lock);
  8221	
  8222		/* Initialize device management tag acquire wait queue */
  8223		init_waitqueue_head(&hba->dev_cmd.tag_wq);
  8224	
  8225		ufshcd_init_clk_gating(hba);
  8226	
  8227		/*
  8228		 * In order to avoid any spurious interrupt immediately after
  8229		 * registering UFS controller interrupt handler, clear any pending UFS
  8230		 * interrupt status and disable all the UFS interrupts.
  8231		 */
  8232		ufshcd_writel(hba, ufshcd_readl(hba, REG_INTERRUPT_STATUS),
  8233			      REG_INTERRUPT_STATUS);
  8234		ufshcd_writel(hba, 0, REG_INTERRUPT_ENABLE);
  8235		/*
  8236		 * Make sure that UFS interrupts are disabled and any pending interrupt
  8237		 * status is cleared before registering UFS interrupt handler.
  8238		 */
  8239		mb();
  8240	
  8241		/* IRQ registration */
  8242		err = devm_request_irq(dev, irq, ufshcd_intr, IRQF_SHARED, UFSHCD, hba);
  8243		if (err) {
  8244			dev_err(hba->dev, "request irq failed\n");
  8245			goto exit_gating;
  8246		} else {
  8247			hba->is_irq_enabled = true;
  8248		}
  8249	
  8250		err = scsi_add_host(host, hba->dev);
  8251		if (err) {
  8252			dev_err(hba->dev, "scsi_add_host failed\n");
  8253			goto exit_gating;
  8254		}
  8255	
  8256		/* Host controller enable */
  8257		err = ufshcd_hba_enable(hba);
  8258		if (err) {
  8259			dev_err(hba->dev, "Host controller enable failed\n");
  8260			ufshcd_print_host_regs(hba);
  8261			ufshcd_print_host_state(hba);
  8262			goto out_remove_scsi_host;
  8263		}
  8264	
  8265		if (ufshcd_is_clkscaling_supported(hba)) {
  8266			char wq_name[sizeof("ufs_clkscaling_00")];
  8267	
  8268			INIT_WORK(&hba->clk_scaling.suspend_work,
  8269				  ufshcd_clk_scaling_suspend_work);
  8270			INIT_WORK(&hba->clk_scaling.resume_work,
  8271				  ufshcd_clk_scaling_resume_work);
  8272	
  8273			snprintf(wq_name, sizeof(wq_name), "ufs_clkscaling_%d",
  8274				 host->host_no);
  8275			hba->clk_scaling.workq = create_singlethread_workqueue(wq_name);
  8276	
  8277			ufshcd_clkscaling_init_sysfs(hba);
  8278		}
  8279	
  8280		/*
  8281		 * Set the default power management level for runtime and system PM.
  8282		 * Default power saving mode is to keep UFS link in Hibern8 state
  8283		 * and UFS device in sleep state.
  8284		 */
  8285		hba->rpm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
  8286							UFS_SLEEP_PWR_MODE,
  8287							UIC_LINK_HIBERN8_STATE);
  8288		hba->spm_lvl = ufs_get_desired_pm_lvl_for_dev_link_state(
  8289							UFS_SLEEP_PWR_MODE,
  8290							UIC_LINK_HIBERN8_STATE);
  8291	
  8292		/* Set the default auto-hiberate idle timer value to 150 ms */
  8293		if (hba->capabilities & MASK_AUTO_HIBERN8_SUPPORT) {
  8294			hba->ahit = FIELD_PREP(UFSHCI_AHIBERN8_TIMER_MASK, 150) |
  8295				    FIELD_PREP(UFSHCI_AHIBERN8_SCALE_MASK, 3);
  8296		}
  8297	
  8298		/* Hold auto suspend until async scan completes */
  8299		pm_runtime_get_sync(dev);
  8300		atomic_set(&hba->scsi_block_reqs_cnt, 0);
  8301		/*
  8302		 * We are assuming that device wasn't put in sleep/power-down
  8303		 * state exclusively during the boot stage before kernel.
  8304		 * This assumption helps avoid doing link startup twice during
  8305		 * ufshcd_probe_hba().
  8306		 */
  8307		ufshcd_set_ufs_dev_active(hba);
  8308	
  8309		async_schedule(ufshcd_async_scan, hba);
  8310		ufs_sysfs_add_nodes(hba->dev);
> 8311		ufshcd_configfs_init(hba);
  8312	
  8313		return 0;
  8314	
  8315	out_remove_scsi_host:
  8316		scsi_remove_host(hba->host);
  8317	exit_gating:
  8318		ufshcd_exit_clk_gating(hba);
  8319	out_disable:
  8320		hba->is_irq_enabled = false;
  8321		ufshcd_hba_exit(hba);
  8322	out_error:
  8323		return err;
  8324	}
  8325	EXPORT_SYMBOL_GPL(ufshcd_init);
  8326	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30458 bytes --]