scsi: ufs: set the device reference clock setting

From: Subhash Jadavani <[email protected]>

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 <[email protected]>

Signed-off-by Can Guo <[email protected]>

Signed-off-by Sayali Lokhande <[email protected]>

---
 drivers/scsi/ufs/ufs.h           | 21 ++++++++++
 drivers/scsi/ufs/ufshcd-pltfrm.c |  2 +
 drivers/scsi/ufs/ufshcd.c        | 89 ++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd.h        |  2 +
 4 files changed, 114 insertions(+)

drivers/scsi/ufs/ufs.h +21 -0

index 14e5bf7..c555ac0 100644
@@ -378,6 +378,27 @@ enum query_opcode {
 	UPIU_QUERY_OPCODE_TOGGLE_FLAG	= 0x8,
 };
 
+/* bRefClkFreq attribute values */
+enum ref_clk_freq_hz {
+	REF_CLK_FREQ_19_2_MHZ	= 19200000,
+	REF_CLK_FREQ_26_MHZ	= 26000000,
+	REF_CLK_FREQ_38_4_MHZ	= 38400000,
+	REF_CLK_FREQ_52_MHZ	= 52000000,
+};
+
+enum bref_clk_freq {
+	bREF_CLK_FREQ_0, /* 19.2 MHz */
+	bREF_CLK_FREQ_1, /* 26 MHz */
+	bREF_CLK_FREQ_2, /* 38.4 MHz */
+	bREF_CLK_FREQ_3, /* 52 MHz */
+	bREF_CLK_FREQ_INVAL,
+};
+
+struct ufs_ref_clk {
+	enum ref_clk_freq_hz freq_hz;
+	enum bref_clk_freq val;
+};
+
 /* Query response result code */
 enum {
 	QUERY_RESULT_SUCCESS                    = 0x00,

drivers/scsi/ufs/ufshcd-pltfrm.c +2 -0

index e82bde0..0953563 100644
@@ -343,6 +343,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);

drivers/scsi/ufs/ufshcd.c +23 -0

index c5b1bf1..0cbdde7 100644
@@ -6296,6 +6296,89 @@ static void ufshcd_def_desc_sizes(struct ufs_hba *hba)
 	hba->desc_size.hlth_desc = QUERY_DESC_HEALTH_DEF_SIZE;
 }
 
+static struct ufs_ref_clk ufs_ref_clk_freqs[] = {
+	{REF_CLK_FREQ_19_2_MHZ, bREF_CLK_FREQ_0},
+	{REF_CLK_FREQ_26_MHZ, bREF_CLK_FREQ_1},
+	{REF_CLK_FREQ_38_4_MHZ, bREF_CLK_FREQ_2},
+	{REF_CLK_FREQ_52_MHZ, bREF_CLK_FREQ_3},
+};
+
+static inline enum bref_clk_freq
+ufs_get_bref_clk_for_ref_clk_freq_hz(u32 freq)
+{
+	enum bref_clk_freq val;
+
+	for (val = bREF_CLK_FREQ_0; val <= bREF_CLK_FREQ_3; val++)
+		if (ufs_ref_clk_freqs[val].freq_hz == freq)
+			return val;
+
+	/* if no match found, return invalid*/
+	return bREF_CLK_FREQ_INVAL;
+}
+
+void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba)
+{
+	struct device *dev = hba->dev;

drivers/scsi/ufs/ufshcd.c +50 -0

+	if (!refclk)
+		return;
+
+	freq = clk_get_rate(refclk);
+	if (freq > REF_CLK_FREQ_52_MHZ) {
+		dev_err(hba->dev,
+		"%s: invalid ref_clk setting = %d\n",
+		__func__, freq);
+		return;
+	}
+
+	hba->dev_ref_clk_freq =
+		ufs_get_bref_clk_for_ref_clk_freq_hz(freq);
+}
+
+static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
+{
+	int err = 0;
+	int ref_clk = -1;
+
+	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 %d Hz failed\n",
+		__func__, ufs_ref_clk_freqs[hba->dev_ref_clk_freq].freq_hz);
+	/*
+	 * 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 %d Hz succeeded\n",
+		__func__, ufs_ref_clk_freqs[hba->dev_ref_clk_freq].freq_hz);
+
+out:
+	return err;
+}
+
 /**
  * ufshcd_probe_hba - probe hba to detect device and initialize
  * @hba: per-adapter instance

drivers/scsi/ufs/ufshcd.c +1 -0

+			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",

drivers/scsi/ufs/ufshcd.h +2 -0

index 8110dcd..101a75c 100644
@@ -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

@@ -746,6 +747,7 @@ static inline void ufshcd_rmwl(struct ufs_hba *hba, u32 mask, u32 val, u32 reg)
 int ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask,
 				u32 val, unsigned long interval_us,
 				unsigned long timeout_ms, bool can_sleep);
+void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba);
 
 static inline void check_upiu_size(void)
 {

Quoted 97 lines · Sayali Lokhande

From: Subhash Jadavani <[email protected]>

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 <[email protected]>

Signed-off-by Can Guo <[email protected]>

Signed-off-by Sayali Lokhande <[email protected]>

---
  drivers/scsi/ufs/ufs.h           | 21 ++++++++++
  drivers/scsi/ufs/ufshcd-pltfrm.c |  2 +
  drivers/scsi/ufs/ufshcd.c        | 89 ++++++++++++++++++++++++++++++++++++++++
  drivers/scsi/ufs/ufshcd.h        |  2 +
  4 files changed, 114 insertions(+)

drivers/scsi/ufs/ufs.h +21 -0

index 14e5bf7..c555ac0 100644
@@ -378,6 +378,27 @@ enum query_opcode {
  	UPIU_QUERY_OPCODE_TOGGLE_FLAG	= 0x8,
  };
 
+/* bRefClkFreq attribute values */
+enum ref_clk_freq_hz {
+	REF_CLK_FREQ_19_2_MHZ	= 19200000,
+	REF_CLK_FREQ_26_MHZ	= 26000000,
+	REF_CLK_FREQ_38_4_MHZ	= 38400000,
+	REF_CLK_FREQ_52_MHZ	= 52000000,
+};
+
+enum bref_clk_freq {
+	bREF_CLK_FREQ_0, /* 19.2 MHz */
+	bREF_CLK_FREQ_1, /* 26 MHz */
+	bREF_CLK_FREQ_2, /* 38.4 MHz */
+	bREF_CLK_FREQ_3, /* 52 MHz */
+	bREF_CLK_FREQ_INVAL,
+};
+
+struct ufs_ref_clk {
+	enum ref_clk_freq_hz freq_hz;
+	enum bref_clk_freq val;
+};
+
  /* Query response result code */
  enum {
  	QUERY_RESULT_SUCCESS                    = 0x00,

drivers/scsi/ufs/ufshcd-pltfrm.c +2 -0

index e82bde0..0953563 100644
@@ -343,6 +343,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);

drivers/scsi/ufs/ufshcd.c +26 -0

index c5b1bf1..0cbdde7 100644
@@ -6296,6 +6296,89 @@ static void ufshcd_def_desc_sizes(struct ufs_hba *hba)
  	hba->desc_size.hlth_desc = QUERY_DESC_HEALTH_DEF_SIZE;
  }
 
+static struct ufs_ref_clk ufs_ref_clk_freqs[] = {
+	{REF_CLK_FREQ_19_2_MHZ, bREF_CLK_FREQ_0},
+	{REF_CLK_FREQ_26_MHZ, bREF_CLK_FREQ_1},
+	{REF_CLK_FREQ_38_4_MHZ, bREF_CLK_FREQ_2},
+	{REF_CLK_FREQ_52_MHZ, bREF_CLK_FREQ_3},
+};
+
+static inline enum bref_clk_freq
+ufs_get_bref_clk_for_ref_clk_freq_hz(u32 freq)
+{
+	enum bref_clk_freq val;
+
+	for (val = bREF_CLK_FREQ_0; val <= bREF_CLK_FREQ_3; val++)
+		if (ufs_ref_clk_freqs[val].freq_hz == freq)
+			return val;
+
+	/* if no match found, return invalid*/
+	return bREF_CLK_FREQ_INVAL;
+}
+
+void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba)
+{
+	struct device *dev = hba->dev;
+	struct device_node *np = dev->of_node;
+	struct clk *refclk = NULL;
+	u32 freq = 0;

drivers/scsi/ufs/ufshcd.c static void ufshcd_def_desc_sizes(struct ufs_hba *hba) · line 6331

Quoted 7 lines

drivers/scsi/ufs/ufshcd.c +7 -0

+
+	if (!np)
+		return;
+
+	hba->dev_ref_clk_freq = bREF_CLK_FREQ_INVAL;
+
+	refclk = of_clk_get_by_name(np, "ref_clk");

What about users that don't use DT?

Quoted 56 lines

drivers/scsi/ufs/ufshcd.c +50 -0

+	if (!refclk)
+		return;
+
+	freq = clk_get_rate(refclk);
+	if (freq > REF_CLK_FREQ_52_MHZ) {
+		dev_err(hba->dev,
+		"%s: invalid ref_clk setting = %d\n",
+		__func__, freq);
+		return;
+	}
+
+	hba->dev_ref_clk_freq =
+		ufs_get_bref_clk_for_ref_clk_freq_hz(freq);
+}
+
+static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
+{
+	int err = 0;
+	int ref_clk = -1;
+
+	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 %d Hz failed\n",
+		__func__, ufs_ref_clk_freqs[hba->dev_ref_clk_freq].freq_hz);
+	/*
+	 * 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 %d Hz succeeded\n",
+		__func__, ufs_ref_clk_freqs[hba->dev_ref_clk_freq].freq_hz);
+
+out:
+	return err;
+}
+
  /**
   * ufshcd_probe_hba - probe hba to detect device and initialize
   * @hba: per-adapter instance

@@ -6361,6 +6444,12 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
  			"%s: Failed getting max supported power mode\n",

drivers/scsi/ufs/ufshcd.c static int ufshcd_probe_hba(struct ufs_hba *hba) · line 6451

Quoted 7 lines

drivers/scsi/ufs/ufshcd.c +5 -0

  			__func__);
  	} else {
+		/*
+		 * Set the right value to bRefClkFreq before attempting to
+		 * switch to HS gears.
+		 */
+		if (hba->dev_ref_clk_freq < bREF_CLK_FREQ_INVAL)

hba->dev_ref_clk_freq does not always get initialized

Quoted 26 lines

drivers/scsi/ufs/ufshcd.c +1 -0

+			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",

drivers/scsi/ufs/ufshcd.h +2 -0

index 8110dcd..101a75c 100644
@@ -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

@@ -746,6 +747,7 @@ static inline void ufshcd_rmwl(struct ufs_hba *hba, u32 mask, u32 val, u32 reg)
  int ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask,
  				u32 val, unsigned long interval_us,
  				unsigned long timeout_ms, bool can_sleep);
+void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba);
 
  static inline void check_upiu_size(void)
  {

Quoted 97 lines · Adrian Hunter

Quoted 96 lines · Sayali Lokhande

From: Subhash Jadavani <[email protected]>

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 <[email protected]>

Signed-off-by Can Guo <[email protected]>

Signed-off-by Sayali Lokhande <[email protected]>

---
  drivers/scsi/ufs/ufs.h           | 21 ++++++++++ drivers/scsi/ufs/ufshcd-pltfrm.c |  2 + drivers/scsi/ufs/ufshcd.c        | 89
++++++++++++++++++++++++++++++++++++++++
  drivers/scsi/ufs/ufshcd.h        |  2 +

  4 files changed, 114 insertions(+)

drivers/scsi/ufs/ufs.h +0 -0

index 14e5bf7..c555ac0 100644
@@ -378,6 +378,27 @@ enum query_opcode {
       UPIU_QUERY_OPCODE_TOGGLE_FLAG    = 0x8,
   };

  +/* bRefClkFreq attribute values */

drivers/scsi/ufs/ufs.h +20 -0

+enum ref_clk_freq_hz {
+    REF_CLK_FREQ_19_2_MHZ    = 19200000,
+    REF_CLK_FREQ_26_MHZ    = 26000000,
+    REF_CLK_FREQ_38_4_MHZ    = 38400000,
+    REF_CLK_FREQ_52_MHZ    = 52000000,
+};
+
+enum bref_clk_freq {
+    bREF_CLK_FREQ_0, /* 19.2 MHz */
+    bREF_CLK_FREQ_1, /* 26 MHz */
+    bREF_CLK_FREQ_2, /* 38.4 MHz */
+    bREF_CLK_FREQ_3, /* 52 MHz */
+    bREF_CLK_FREQ_INVAL,
+};
+
+struct ufs_ref_clk {
+    enum ref_clk_freq_hz freq_hz;
+    enum bref_clk_freq val;
+};
+

  /* Query response result code */
  enum {
      QUERY_RESULT_SUCCESS                    = 0x00,

drivers/scsi/ufs/ufshcd-pltfrm.c +1 -0

index e82bde0..0953563 100644
@@ -343,6 +343,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);

drivers/scsi/ufs/ufshcd.c +25 -0

index c5b1bf1..0cbdde7 100644
@@ -6296,6 +6296,89 @@ static void ufshcd_def_desc_sizes(struct ufs_hba
 *hba)
       hba->desc_size.hlth_desc = QUERY_DESC_HEALTH_DEF_SIZE;
   }
   +static struct ufs_ref_clk ufs_ref_clk_freqs[] = {
+    {REF_CLK_FREQ_19_2_MHZ, bREF_CLK_FREQ_0},
+    {REF_CLK_FREQ_26_MHZ, bREF_CLK_FREQ_1},
+    {REF_CLK_FREQ_38_4_MHZ, bREF_CLK_FREQ_2},
+    {REF_CLK_FREQ_52_MHZ, bREF_CLK_FREQ_3},
+};
+
+static inline enum bref_clk_freq
+ufs_get_bref_clk_for_ref_clk_freq_hz(u32 freq)
+{
+    enum bref_clk_freq val;
+
+    for (val = bREF_CLK_FREQ_0; val <= bREF_CLK_FREQ_3; val++)
+        if (ufs_ref_clk_freqs[val].freq_hz == freq)
+            return val;
+
+    /* if no match found, return invalid*/
+    return bREF_CLK_FREQ_INVAL;
+}
+
+void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba)
+{
+    struct device *dev = hba->dev;
+    struct device_node *np = dev->of_node;
+    struct clk *refclk = NULL;
+    u32 freq = 0;

Quoted 8 lines

drivers/scsi/ufs/ufshcd.c static void ufshcd_def_desc_sizes(struct ufs_hba *hba) · line 6331

Quoted 7 lines

drivers/scsi/ufs/ufshcd.c +7 -0

+
+    if (!np)
+        return;
+
+    hba->dev_ref_clk_freq = bREF_CLK_FREQ_INVAL;
+
+    refclk = of_clk_get_by_name(np, "ref_clk");

What about users that don't use DT?

If there is no DT entry present for ref_clk, we simply set to bREF_CLK_FREQ_INVAL and thus ref clk will not be changed. It will maintain the Manufacturer default value whatever is set by the ufs device vendor.

Quoted 56 lines

Quoted 56 lines

drivers/scsi/ufs/ufshcd.c +50 -0

+    if (!refclk)
+        return;
+
+    freq = clk_get_rate(refclk);
+    if (freq > REF_CLK_FREQ_52_MHZ) {
+        dev_err(hba->dev,
+        "%s: invalid ref_clk setting = %d\n",
+        __func__, freq);
+        return;
+    }
+
+    hba->dev_ref_clk_freq =
+        ufs_get_bref_clk_for_ref_clk_freq_hz(freq);
+}
+
+static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
+{
+    int err = 0;
+    int ref_clk = -1;
+
+    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 %d Hz failed\n",
+        __func__, ufs_ref_clk_freqs[hba->dev_ref_clk_freq].freq_hz);
+    /*
+     * 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 %d Hz succeeded\n",
+        __func__, ufs_ref_clk_freqs[hba->dev_ref_clk_freq].freq_hz);
+
+out:
+    return err;
+}
+
   /**
    * ufshcd_probe_hba - probe hba to detect device and initialize

   * @hba: per-adapter instance

drivers/scsi/ufs/ufshcd.c +0 -0

@@ -6361,6 +6444,12 @@ static int ufshcd_probe_hba(struct ufs_hba *hba)
               "%s: Failed getting max supported power mode\n",

Quoted 8 lines

drivers/scsi/ufs/ufshcd.c static int ufshcd_probe_hba(struct ufs_hba *hba) · line 6451

Quoted 7 lines

drivers/scsi/ufs/ufshcd.c +5 -0

               __func__);
       } else {
+        /*
+         * Set the right value to bRefClkFreq before attempting to
+         * switch to HS gears.
+         */
+        if (hba->dev_ref_clk_freq < bREF_CLK_FREQ_INVAL)

hba->dev_ref_clk_freq does not always get initialized

In ufshcd_parse_dev_ref_clk_freq() function we are initializing the hba->dev_ref_clk_freqto inval as default.

This parse function will be always
called in init

and thus dev_ref_clk_freq should always get initialized to
called in init and thus dev_ref_clk_freq should always get initialized to defult inval or valid setting if found (in between 19.2 MHz to 52MHz).

Quoted 25 lines

Quoted 25 lines

drivers/scsi/ufs/ufshcd.c +1 -0

+            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",

drivers/scsi/ufs/ufshcd.h +2 -0

index 8110dcd..101a75c 100644
@@ -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

@@ -746,6 +747,7 @@ static inline void ufshcd_rmwl(struct ufs_hba *hba,
 u32 mask, u32 val, u32 reg)
   int ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask,
                   u32 val, unsigned long interval_us,
                   unsigned long timeout_ms, bool can_sleep);
+void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba);
     static inline void check_upiu_size(void)
   {

From: Subhash Jadavani <[email protected]>

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 <[email protected]>

Signed-off-by Can Guo <[email protected]>

Signed-off-by Sayali Lokhande <[email protected]>

---
 drivers/scsi/ufs/ufs.h           | 21 ++++++++++
 drivers/scsi/ufs/ufshcd-pltfrm.c |  2 +
 drivers/scsi/ufs/ufshcd.c        | 89 ++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd.h        |  2 +
 4 files changed, 114 insertions(+)

drivers/scsi/ufs/ufs.h +5 -0

index 14e5bf7..c555ac0 100644
@@ -378,6 +378,27 @@ enum query_opcode {
        UPIU_QUERY_OPCODE_TOGGLE_FLAG   = 0x8,
 };
 
+/* bRefClkFreq attribute values */
+enum ref_clk_freq_hz {
+       REF_CLK_FREQ_19_2_MHZ   = 19200000,
+       REF_CLK_FREQ_26_MHZ     = 26000000,
+       REF_CLK_FREQ_38_4_MHZ   = 38400000,

drivers/scsi/ufs/ufs.h +3 -0

+       enum bref_clk_freq val;
+};
+
 /* Query response result code */
 enum {
        QUERY_RESULT_SUCCESS                    = 0x00,

drivers/scsi/ufs/ufshcd-pltfrm.c +2 -0

index e82bde0..0953563 100644
@@ -343,6 +343,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);

drivers/scsi/ufs/ufshcd.c +0 -0

index c5b1bf1..0cbdde7 100644

drivers/scsi/ufs/ufshcd.c +45 -0

+               if (ufs_ref_clk_freqs[val].freq_hz == freq)
+                       return val;
+
+       /* if no match found, return invalid*/
+       return bREF_CLK_FREQ_INVAL;
+}
+
+void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba)
+{
+       struct device *dev = hba->dev;
+       struct device_node *np = dev->of_node;
+       struct clk *refclk = NULL;
+       u32 freq = 0;
+
+       if (!np)
+               return;
+
+       hba->dev_ref_clk_freq = bREF_CLK_FREQ_INVAL;
+
+       refclk = of_clk_get_by_name(np, "ref_clk");
+       if (!refclk)
+               return;
+
+       freq = clk_get_rate(refclk);
+       if (freq > REF_CLK_FREQ_52_MHZ) {
+               dev_err(hba->dev,
+               "%s: invalid ref_clk setting = %d\n",
+               __func__, freq);
+               return;
+       }
+
+       hba->dev_ref_clk_freq =
+               ufs_get_bref_clk_for_ref_clk_freq_hz(freq);
+}
+
+static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
+{
+       int err = 0;
+       int ref_clk = -1;
+
+       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",

drivers/scsi/ufs/ufshcd.c +15 -0

+
+       if (err)
+               dev_err(hba->dev, "%s: bRefClkFreq setting to %d Hz failed\n",
+               __func__, ufs_ref_clk_freqs[hba->dev_ref_clk_freq].freq_hz);
+       /*
+        * 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 %d Hz succeeded\n",
+               __func__, ufs_ref_clk_freqs[hba->dev_ref_clk_freq].freq_hz);
+
+out:
+       return err;
+}
+
 /**
  * ufshcd_probe_hba - probe hba to detect device and initialize
  * @hba: per-adapter instance

drivers/scsi/ufs/ufshcd.h +0 -1

 static inline void check_upiu_size(void)
 {
--

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

Quoted 31 lines · Sayali Lokhande

From: Subhash Jadavani <[email protected]>

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 <[email protected]>

Signed-off-by Can Guo <[email protected]>

Signed-off-by Sayali Lokhande <[email protected]>

---
  drivers/scsi/ufs/ufs.h           | 21 ++++++++++
  drivers/scsi/ufs/ufshcd-pltfrm.c |  2 +
  drivers/scsi/ufs/ufshcd.c        | 89 ++++++++++++++++++++++++++++++++++++++++
  drivers/scsi/ufs/ufshcd.h        |  2 +
  4 files changed, 114 insertions(+)

drivers/scsi/ufs/ufs.h +5 -0

index 14e5bf7..c555ac0 100644
@@ -378,6 +378,27 @@ enum query_opcode {
         UPIU_QUERY_OPCODE_TOGGLE_FLAG   = 0x8,
  };
 
+/* bRefClkFreq attribute values */
+enum ref_clk_freq_hz {
+       REF_CLK_FREQ_19_2_MHZ   = 19200000,
+       REF_CLK_FREQ_26_MHZ     = 26000000,
+       REF_CLK_FREQ_38_4_MHZ   = 38400000,

drivers/scsi/ufs/ufs.h enum query_opcode { · line 395

Quoted 10 lines

drivers/scsi/ufs/ufs.h +10 -0

+       REF_CLK_FREQ_52_MHZ     = 52000000,
+};
+
+enum bref_clk_freq {
+       bREF_CLK_FREQ_0, /* 19.2 MHz */
+       bREF_CLK_FREQ_1, /* 26 MHz */
+       bREF_CLK_FREQ_2, /* 38.4 MHz */
+       bREF_CLK_FREQ_3, /* 52 MHz */
+       bREF_CLK_FREQ_INVAL,
+};

These enums are not helpful, roughly the equivalent of VALUE_1000 = 1000. Replace both with a single one, something like:

enum ufs_ref_clk_freq {
         UFS_REF_CLK_19P2MHZ = 0,
         UFS_REF_CLK_26MHZ = 1,
         UFS_REF_CLK_38P4MHZ = 2,
         UFS_REF_CLK_52MHZ = 3,
         UFS_REF_CLK_INVAL = -1
};

drivers/scsi/ufs/ufs.h enum query_opcode { · line 398

Quoted 3 lines

drivers/scsi/ufs/ufs.h +3 -0

+
+struct ufs_ref_clk {
+       enum ref_clk_freq_hz freq_hz;

Just make this an unsigned, no need for an enum of identity values.

Quoted 27 lines

drivers/scsi/ufs/ufs.h +3 -0

+       enum bref_clk_freq val;
+};
+
  /* Query response result code */
  enum {
         QUERY_RESULT_SUCCESS                    = 0x00,

drivers/scsi/ufs/ufshcd-pltfrm.c +2 -0

index e82bde0..0953563 100644
@@ -343,6 +343,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);

drivers/scsi/ufs/ufshcd.c +0 -0

index c5b1bf1..0cbdde7 100644
@@ -6296,6 +6296,89 @@ static void ufshcd_def_desc_sizes(struct ufs_hba *hba)
         hba->desc_size.hlth_desc = QUERY_DESC_HEALTH_DEF_SIZE;
  }
 

drivers/scsi/ufs/ufshcd.c static void ufshcd_def_desc_sizes(struct ufs_hba *hba) · line 6300

Quoted 2 lines

drivers/scsi/ufs/ufshcd.c +2 -0

+static struct ufs_ref_clk ufs_ref_clk_freqs[] = {
+       {REF_CLK_FREQ_19_2_MHZ, bREF_CLK_FREQ_0},

Then these should just be something like:

         {19200000, UFS_REF_CLK_19P2MHZ},
         ...
         {0, UFS_REF_CLK_INVAL},

drivers/scsi/ufs/ufshcd.c static void ufshcd_def_desc_sizes(struct ufs_hba *hba) · line 6311

Quoted 11 lines

drivers/scsi/ufs/ufshcd.c +11 -0

+       {REF_CLK_FREQ_26_MHZ, bREF_CLK_FREQ_1},
+       {REF_CLK_FREQ_38_4_MHZ, bREF_CLK_FREQ_2},
+       {REF_CLK_FREQ_52_MHZ, bREF_CLK_FREQ_3},
+};
+
+static inline enum bref_clk_freq
+ufs_get_bref_clk_for_ref_clk_freq_hz(u32 freq)
+{
+       enum bref_clk_freq val;
+
+       for (val = bREF_CLK_FREQ_0; val <= bREF_CLK_FREQ_3; val++)

In my suggestion above, I terminated the table with {0,

UFS_REF_CLK_INVAL}. Then you could change this to a while loop that stops when you see that sentinel node. I don't like the way it is now because 1) You're making assumptions about the enum values being equal to array indices, which may not line up if the next UFS spec adds values that aren't contiguous (and defeats the whole point of the table), and 2) Using <= LAST_VALID_VALUE makes this susceptible to bugs when more valid values are added but someone forgets to go find this loop and update it.

Quoted 45 lines

drivers/scsi/ufs/ufshcd.c +45 -0

+               if (ufs_ref_clk_freqs[val].freq_hz == freq)
+                       return val;
+
+       /* if no match found, return invalid*/
+       return bREF_CLK_FREQ_INVAL;
+}
+
+void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba)
+{
+       struct device *dev = hba->dev;
+       struct device_node *np = dev->of_node;
+       struct clk *refclk = NULL;
+       u32 freq = 0;
+
+       if (!np)
+               return;
+
+       hba->dev_ref_clk_freq = bREF_CLK_FREQ_INVAL;
+
+       refclk = of_clk_get_by_name(np, "ref_clk");
+       if (!refclk)
+               return;
+
+       freq = clk_get_rate(refclk);
+       if (freq > REF_CLK_FREQ_52_MHZ) {
+               dev_err(hba->dev,
+               "%s: invalid ref_clk setting = %d\n",
+               __func__, freq);
+               return;
+       }
+
+       hba->dev_ref_clk_freq =
+               ufs_get_bref_clk_for_ref_clk_freq_hz(freq);
+}
+
+static int ufshcd_set_dev_ref_clk(struct ufs_hba *hba)
+{
+       int err = 0;
+       int ref_clk = -1;
+
+       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",

drivers/scsi/ufs/ufshcd.c static void ufshcd_def_desc_sizes(struct ufs_hba *hba) · line 6366

Quoted 10 lines

drivers/scsi/ufs/ufshcd.c +10 -0

+                        __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);

This is probably a nit, but I think the compiler is allowed to choose any type for your enum that fits all of its values. This may not be a u32, so you should probably create a u32 local, and assign into it before passing a pointer into this function.

Ah wait, I see that you made the member a u32. It might be nicer to type the member as the enum, and then do this local conversion as I've mentioned. That will allow the compiler to help check more.

Quoted 27 lines

drivers/scsi/ufs/ufshcd.c +20 -0

+
+       if (err)
+               dev_err(hba->dev, "%s: bRefClkFreq setting to %d Hz failed\n",
+               __func__, ufs_ref_clk_freqs[hba->dev_ref_clk_freq].freq_hz);
+       /*
+        * 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 %d Hz succeeded\n",
+               __func__, ufs_ref_clk_freqs[hba->dev_ref_clk_freq].freq_hz);
+
+out:
+       return err;
+}
+
  /**
   * ufshcd_probe_hba - probe hba to detect device and initialize
   * @hba: per-adapter instance

@@ -6361,6 +6444,12 @@ 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.
+                */
+               if (hba->dev_ref_clk_freq < bREF_CLK_FREQ_INVAL)

drivers/scsi/ufs/ufshcd.c static int ufshcd_probe_hba(struct ufs_hba *hba) · line 6452

Quoted 1 line

drivers/scsi/ufs/ufshcd.c +1 -0

+                       ufshcd_set_dev_ref_clk(hba);

As mentioned by other reviewers, you're calling this function, but you haven't called ufshcd_parse_dev_ref_clk_freq in all cases that lead to this. So this function is now setting 19.2MHz into a whole set of devices who haven't specified it. Perhaps the initialization of the member needs to go in ufshcd_alloc_host. (Look for callers of that to find other paths that circumvent your parse function). I'm not sure how to advise on enabling this functionality for non-DT machines.

Quoted 5 lines

drivers/scsi/ufs/ufshcd.c +0 -0

                 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",

drivers/scsi/ufs/ufshcd.h +0 -0

index 8110dcd..101a75c 100644

drivers/scsi/ufs/ufshcd.h struct ufs_hba { · line 551

Quoted 7 lines

drivers/scsi/ufs/ufshcd.h +1 -0

@@ -548,6 +548,7 @@ struct ufs_hba {
         void *priv;
         unsigned int irq;
         bool is_irq_enabled;
+       u32 dev_ref_clk_freq;

As I mentioned above, this might be better as the enum type.

drivers/scsi/ufs/ufshcd.h static inline void ufshcd_rmwl(struct ufs_hba *hba, u32 mask, u32 val, u32 reg) · line 750

Quoted 7 lines

drivers/scsi/ufs/ufshcd.h +1 -0

         /* Interrupt aggregation support is broken */
         #define UFSHCD_QUIRK_BROKEN_INTR_AGGR                   0x1

@@ -746,6 +747,7 @@ static inline void ufshcd_rmwl(struct ufs_hba *hba, u32 mask, u32 val, u32 reg)
  int ufshcd_wait_for_register(struct ufs_hba *hba, u32 reg, u32 mask,
                                 u32 val, unsigned long interval_us,
                                 unsigned long timeout_ms, bool can_sleep);
+void ufshcd_parse_dev_ref_clk_freq(struct ufs_hba *hba);

I wonder if this should fail if you end up finding a DT property, but it's a crazy invalid value. What do you think?

Quoted 6 lines

drivers/scsi/ufs/ufshcd.h +0 -1

  static inline void check_upiu_size(void)
  {
--

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

This patch adds configfs support to provision UFS device at runtime. This feature can be primarily useful in factory or assembly line as some devices may be required to be configured multiple times during initial system development phase. Configuration Descriptors can be written multiple times until bConfigDescrLock attribute is zero.

Configuration descriptor buffer consists of Device and Unit descriptor configurable parameters which are parsed from vendor specific provisioning file and then passed via configfs node at runtime to provision ufs device.
CONFIG_CONFIGFS_FS needs to be enabled for using this feature.

Usage:
1) To read current configuration descriptor :
cat /config/XXXX.ufshc/ufs_provision
2) To provision ufs device:
echo <config_desc_buf> > /config/XXXX.ufshc/ufs_provision

Signed-off-by Sayali Lokhande <[email protected]>

---
 Documentation/ABI/testing/configfs-driver-ufs |  18 +++
 drivers/scsi/ufs/Kconfig                      |  10 ++
 drivers/scsi/ufs/Makefile                     |   1 +
 drivers/scsi/ufs/ufs-configfs.c               | 159 ++++++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd.c                     |   7 +-
 drivers/scsi/ufs/ufshcd.h                     |  10 ++
 6 files changed, 204 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/configfs-driver-ufs
 create mode 100644 drivers/scsi/ufs/ufs-configfs.c

Documentation/ABI/testing/configfs-driver-ufs new file +18 -0

new file mode 100644
index 0000000..eeee499c
@@ -0,0 +1,18 @@
+What:          /config/ufshcd/ufs_provision
+Date:          Jun 2018
+KernelVersion: 4.14
+Description:
+               This file shows the current ufs configuration descriptor set in device.
+               This can be used to provision ufs device if bConfigDescrLock is 0.
+               For more details, refer 14.1.6.3 Configuration Descriptor and
+               Table 14-12 - Unit Descriptor configurable parameters from specs for
+               description of each configuration descriptor parameter.
+               Configuration descriptor buffer needs to be passed in space separated
+               format specificied as below:
+               echo <bLength> <bDescriptorIDN> <bConfDescContinue> <bBootEnable>
+               <bDescrAccessEn> <bInitPowerMode> <bHighPriorityLUN>
+               <bSecureRemovalType> <bInitActiveICCLevel> <wPeriodicRTCUpdate>
+               <0Bh:0Fh_ReservedAs_0> <bLUEnable> <bBootLunID> <bLUWriteProtect>
+               <bMemoryType> <dNumAllocUnits> <bDataReliability> <bLogicalBlockSize>
+               <bProvisioningType> <wContextCapabilities> <0Dh:0Fh_ReservedAs_0>
+               > /config/XXXX.ufshc/ufs_provision

drivers/scsi/ufs/Kconfig +10 -0

index e27b4d4..a9a0a58 100644
@@ -100,3 +100,13 @@ config SCSI_UFS_QCOM
 
          Select this if you have UFS controller on QCOM chipset.
          If unsure, say N.
+
+config UFS_PROVISION
+       tristate "Runtime UFS Provisioning support"
+       depends on SCSI_UFSHCD && CONFIGFS_FS
+       help
+         This enables runtime UFS provisioning support. This can be used
+         primarily during assembly line as some devices may be required to
+         be configured multiple times during initial development phase.
+
+         If unsure, say N.

drivers/scsi/ufs/Makefile +1 -0

index 918f579..00110ea 100644
@@ -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_UFS_PROVISION) += ufs-configfs.o
 obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
 obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o

drivers/scsi/ufs/ufs-configfs.c new file +18 -0

new file mode 100644
index 0000000..cd1a227
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2018, Linux Foundation.
+
+#include <linux/configfs.h>
+#include <linux/err.h>
+#include <linux/string.h>
+
+#include "ufs.h"
+#include "ufshcd.h"
+
+static inline struct ufs_hba *config_item_to_hba(struct config_item *item)
+{
+       struct config_group *group = to_config_group(item);
+       struct configfs_subsystem *subsys = to_configfs_subsystem(group);
+       struct ufs_hba *hba = container_of(subsys, struct ufs_hba, subsys);
+
+       return subsys ? hba : NULL;
+}

drivers/scsi/ufs/ufs-configfs.c new file +14 -0

+
+       ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC,
+               QUERY_DESC_IDN_CONFIGURATION, 0, 0, desc_buf, &buff_len);
+
+       if (ret)
+               return ret;
+
+       for (i = 0; i < buff_len; i++)
+               curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
+                               "0x%x ", desc_buf[i]);
+
+       return curr_len;
+}
+

drivers/scsi/ufs/ufs-configfs.c new file +85 -0

+       }
+
+       for (i = 0; i < QUERY_DESC_CONFIGURATION_DEF_SIZE; i++) {
+               token = strsep(&strbuf, " ");
+               if (!token && i)
+                       break;
+
+               ret = kstrtoint(token, 0, &value);
+               if (ret) {
+                       dev_err(hba->dev, "%s: kstrtoint failed %d %s\n",
+                               __func__, ret, token);
+                       goto out;
+               }
+               desc_buf[i] = (u8)value;
+       }
+
+       /* Write configuration descriptor to provision ufs */
+       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: UFS Provisioning done, reboot now!\n",
+               __func__);
+
+out:
+       kfree(strbuf_copy);
+       return count;
+}
+
+static ssize_t ufs_provision_store(struct config_item *item,
+               const char *buf, size_t count)
+{
+       struct ufs_hba *hba = config_item_to_hba(item);
+
+       return ufshcd_desc_configfs_store(hba, buf, count);
+}
+
+static struct configfs_attribute ufshcd_attr_provision = {
+       .ca_name        = "ufs_provision",
+       .ca_mode        = 0644,
+       .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_type = &ufscfg_type,
+               },
+       },
+};
+
+int ufshcd_configfs_init(struct ufs_hba *hba, const char *name)
+{
+       int ret;
+       struct configfs_subsystem *subsys = &hba->subsys;
+
+       strncpy(ufscfg_subsys.su_group.cg_item.ci_namebuf, name, strlen(name));
+       subsys->su_group = ufscfg_subsys.su_group;
+       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);
+}

drivers/scsi/ufs/ufshcd.c +3 -0

index 0cbdde7..bc7bffc 100644
@@ -7711,6 +7711,9 @@ int ufshcd_shutdown(struct ufs_hba *hba)
 void ufshcd_remove(struct ufs_hba *hba)
 {
        ufs_sysfs_remove_nodes(hba->dev);
+#ifdef CONFIG_UFS_PROVISION
+       ufshcd_configfs_exit();
+#endif
        scsi_remove_host(hba->host);
        /* disable interrupts */
        ufshcd_disable_intr(hba, hba->intr_mask);

drivers/scsi/ufs/ufshcd.c +0 -0

        return 0;
 
 out_remove_scsi_host:

drivers/scsi/ufs/ufshcd.h +10 -1

index 101a75c..7b6cc88 100644
@@ -37,6 +37,7 @@
 #ifndef _UFSHCD_H
 #define _UFSHCD_H
 
+#include <linux/configfs.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/init.h>

@@ -515,6 +516,9 @@ struct ufs_hba {
 
        struct Scsi_Host *host;
        struct device *dev;
+#ifdef CONFIG_UFS_PROVISION
+       struct configfs_subsystem subsys;
+#endif
        /*
         * This field is to keep a reference to "scsi_device" corresponding to
         * "UFS device" W-LU.

@@ -868,6 +872,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 */
+#ifdef CONFIG_UFS_PROVISION
+extern int ufshcd_configfs_init(struct ufs_hba *hba, const char *name);
+extern void ufshcd_configfs_exit(void);
+#endif
+
 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

Quoted 112 lines · Sayali Lokhande

This patch adds configfs support to provision UFS device at runtime. This feature can be primarily useful in factory or assembly line as some devices may be required to be configured multiple times during initial system development phase. Configuration Descriptors can be written multiple times until bConfigDescrLock attribute is zero.

Configuration descriptor buffer consists of Device and Unit descriptor configurable parameters which are parsed from vendor specific provisioning file and then passed via configfs node at runtime to provision ufs device.
CONFIG_CONFIGFS_FS needs to be enabled for using this feature.

Usage:
1) To read current configuration descriptor :
cat /config/XXXX.ufshc/ufs_provision
2) To provision ufs device:
echo <config_desc_buf> > /config/XXXX.ufshc/ufs_provision

Signed-off-by Sayali Lokhande <[email protected]>

---
  Documentation/ABI/testing/configfs-driver-ufs |  18 +++
  drivers/scsi/ufs/Kconfig                      |  10 ++
  drivers/scsi/ufs/Makefile                     |   1 +
  drivers/scsi/ufs/ufs-configfs.c               | 159 ++++++++++++++++++++++++++
  drivers/scsi/ufs/ufshcd.c                     |   7 +-
  drivers/scsi/ufs/ufshcd.h                     |  10 ++
  6 files changed, 204 insertions(+), 1 deletion(-)

create mode 100644 Documentation/ABI/testing/configfs-driver-ufs create mode 100644 drivers/scsi/ufs/ufs-configfs.c

Documentation/ABI/testing/configfs-driver-ufs new file +18 -0

new file mode 100644
index 0000000..eeee499c
@@ -0,0 +1,18 @@
+What:          /config/ufshcd/ufs_provision
+Date:          Jun 2018
+KernelVersion: 4.14
+Description:
+               This file shows the current ufs configuration descriptor set in device.
+               This can be used to provision ufs device if bConfigDescrLock is 0.
+               For more details, refer 14.1.6.3 Configuration Descriptor and
+               Table 14-12 - Unit Descriptor configurable parameters from specs for
+               description of each configuration descriptor parameter.
+               Configuration descriptor buffer needs to be passed in space separated
+               format specificied as below:
+               echo <bLength> <bDescriptorIDN> <bConfDescContinue> <bBootEnable>
+               <bDescrAccessEn> <bInitPowerMode> <bHighPriorityLUN>
+               <bSecureRemovalType> <bInitActiveICCLevel> <wPeriodicRTCUpdate>
+               <0Bh:0Fh_ReservedAs_0> <bLUEnable> <bBootLunID> <bLUWriteProtect>
+               <bMemoryType> <dNumAllocUnits> <bDataReliability> <bLogicalBlockSize>
+               <bProvisioningType> <wContextCapabilities> <0Dh:0Fh_ReservedAs_0>
+               > /config/XXXX.ufshc/ufs_provision

drivers/scsi/ufs/Kconfig +10 -0

index e27b4d4..a9a0a58 100644
@@ -100,3 +100,13 @@ config SCSI_UFS_QCOM
 
           Select this if you have UFS controller on QCOM chipset.
           If unsure, say N.
+
+config UFS_PROVISION
+       tristate "Runtime UFS Provisioning support"
+       depends on SCSI_UFSHCD && CONFIGFS_FS
+       help
+         This enables runtime UFS provisioning support. This can be used
+         primarily during assembly line as some devices may be required to
+         be configured multiple times during initial development phase.
+
+         If unsure, say N.

drivers/scsi/ufs/Makefile +1 -0

index 918f579..00110ea 100644
@@ -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_UFS_PROVISION) += ufs-configfs.o
  obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
  obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o

drivers/scsi/ufs/ufs-configfs.c new file +22 -0

new file mode 100644
index 0000000..cd1a227
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright (c) 2018, Linux Foundation.
+
+#include <linux/configfs.h>
+#include <linux/err.h>
+#include <linux/string.h>
+
+#include "ufs.h"
+#include "ufshcd.h"
+
+static inline struct ufs_hba *config_item_to_hba(struct config_item *item)
+{
+       struct config_group *group = to_config_group(item);
+       struct configfs_subsystem *subsys = to_configfs_subsystem(group);
+       struct ufs_hba *hba = container_of(subsys, struct ufs_hba, subsys);
+
+       return subsys ? hba : NULL;
+}
+
+static ssize_t ufs_provision_show(struct config_item *item, char *buf)
+{
+       u8 desc_buf[QUERY_DESC_CONFIGURATION_DEF_SIZE] = {0};

drivers/scsi/ufs/ufs-configfs.c line 28

Quoted 6 lines

drivers/scsi/ufs/ufs-configfs.c new file +6 -0

+       int buff_len = QUERY_DESC_CONFIGURATION_DEF_SIZE;
+       int i, ret, curr_len = 0;
+       struct ufs_hba *hba = config_item_to_hba(item);
+
+       if (!hba)
+               return -EINVAL;

I don't think this is actually possible. Is a configfs show function ever called with a null item?

Quoted 19 lines

drivers/scsi/ufs/ufs-configfs.c new file +19 -0

+
+       ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_READ_DESC,
+               QUERY_DESC_IDN_CONFIGURATION, 0, 0, desc_buf, &buff_len);
+
+       if (ret)
+               return ret;
+
+       for (i = 0; i < buff_len; i++)
+               curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
+                               "0x%x ", desc_buf[i]);
+
+       return curr_len;
+}
+
+ssize_t ufshcd_desc_configfs_store(struct ufs_hba *hba,
+               const char *buf, size_t count)
+{
+       char *strbuf;
+       char *strbuf_copy;

drivers/scsi/ufs/ufs-configfs.c line 52

Quoted 5 lines

drivers/scsi/ufs/ufs-configfs.c new file +5 -0

+       u8 desc_buf[QUERY_DESC_CONFIGURATION_DEF_SIZE] = {0};
+       int buff_len = QUERY_DESC_CONFIGURATION_DEF_SIZE;
+       char *token;
+       int i, ret, value;
+       u32 bConfigDescrLock = 0;

I haven't seen many examples of variables named this way. I know it follows what the UFS spec does, but I haven't noticed examples of people naming their locals outside of normal Linux conventions.

Quoted 4 lines

drivers/scsi/ufs/ufs-configfs.c new file +4 -0

+
+       /* reserve one byte for null termination */
+       strbuf = kmalloc(count + 1, GFP_KERNEL);
+       if (!strbuf)

drivers/scsi/ufs/ufs-configfs.c line 63

Quoted 7 lines

drivers/scsi/ufs/ufs-configfs.c new file +7 -0

+               return -ENOMEM;
+
+       strbuf_copy = strbuf;
+       strlcpy(strbuf, buf, count + 1);
+
+       if (!hba)
+               goto out;

Again, is this possible?

Quoted 6 lines

drivers/scsi/ufs/ufs-configfs.c new file +6 -0

+
+       /* 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, &bConfigDescrLock);
+       if (ret)
+               goto out;

drivers/scsi/ufs/ufs-configfs.c line 74

Quoted 5 lines

drivers/scsi/ufs/ufs-configfs.c new file +5 -0

+
+       if (bConfigDescrLock) {
+               dev_err(hba->dev, "%s: bConfigDescrLock already set to %u, cannot re-provision device!\n",
+               __func__, bConfigDescrLock);
+               goto out;

You're succeeding here, shouldn't you fail the write? In fact, you succeed the write in a whole bunch of failure cases. Please fail the write in error cases.

Quoted 101 lines

drivers/scsi/ufs/ufs-configfs.c new file +85 -0

+       }
+
+       for (i = 0; i < QUERY_DESC_CONFIGURATION_DEF_SIZE; i++) {
+               token = strsep(&strbuf, " ");
+               if (!token && i)
+                       break;
+
+               ret = kstrtoint(token, 0, &value);
+               if (ret) {
+                       dev_err(hba->dev, "%s: kstrtoint failed %d %s\n",
+                               __func__, ret, token);
+                       goto out;
+               }
+               desc_buf[i] = (u8)value;
+       }
+
+       /* Write configuration descriptor to provision ufs */
+       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: UFS Provisioning done, reboot now!\n",
+               __func__);
+
+out:
+       kfree(strbuf_copy);
+       return count;
+}
+
+static ssize_t ufs_provision_store(struct config_item *item,
+               const char *buf, size_t count)
+{
+       struct ufs_hba *hba = config_item_to_hba(item);
+
+       return ufshcd_desc_configfs_store(hba, buf, count);
+}
+
+static struct configfs_attribute ufshcd_attr_provision = {
+       .ca_name        = "ufs_provision",
+       .ca_mode        = 0644,
+       .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_type = &ufscfg_type,
+               },
+       },
+};
+
+int ufshcd_configfs_init(struct ufs_hba *hba, const char *name)
+{
+       int ret;
+       struct configfs_subsystem *subsys = &hba->subsys;
+
+       strncpy(ufscfg_subsys.su_group.cg_item.ci_namebuf, name, strlen(name));
+       subsys->su_group = ufscfg_subsys.su_group;
+       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);
+}

drivers/scsi/ufs/ufshcd.c +3 -0

index 0cbdde7..bc7bffc 100644
@@ -7711,6 +7711,9 @@ int ufshcd_shutdown(struct ufs_hba *hba)
  void ufshcd_remove(struct ufs_hba *hba)
  {
         ufs_sysfs_remove_nodes(hba->dev);
+#ifdef CONFIG_UFS_PROVISION
+       ufshcd_configfs_exit();
+#endif
         scsi_remove_host(hba->host);
         /* disable interrupts */
         ufshcd_disable_intr(hba, hba->intr_mask);

@@ -7964,7 +7967,9 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 

drivers/scsi/ufs/ufshcd.c int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) · line 7972

Quoted 6 lines

drivers/scsi/ufs/ufshcd.c +3 -1

         async_schedule(ufshcd_async_scan, hba);
         ufs_sysfs_add_nodes(hba->dev);
-
+#ifdef CONFIG_UFS_PROVISION
+       ufshcd_configfs_init(hba, dev_name(hba->dev));
+#endif

Normally the way I see this done is to define stub functions up in the header when !CONFIG_UFS_PROVISION, rather than ifdefs in the code. Can you do it that way?

Quoted 42 lines

drivers/scsi/ufs/ufshcd.c +0 -0

         return 0;
 
  out_remove_scsi_host:

drivers/scsi/ufs/ufshcd.h +10 -1

index 101a75c..7b6cc88 100644
@@ -37,6 +37,7 @@
  #ifndef _UFSHCD_H
  #define _UFSHCD_H
 
+#include <linux/configfs.h>
  #include <linux/module.h>
  #include <linux/kernel.h>
  #include <linux/init.h>

@@ -515,6 +516,9 @@ struct ufs_hba {
 
         struct Scsi_Host *host;
         struct device *dev;
+#ifdef CONFIG_UFS_PROVISION
+       struct configfs_subsystem subsys;
+#endif
         /*
          * This field is to keep a reference to "scsi_device" corresponding to
          * "UFS device" W-LU.

@@ -868,6 +872,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 */
+#ifdef CONFIG_UFS_PROVISION
+extern int ufshcd_configfs_init(struct ufs_hba *hba, const char *name);
+extern void ufshcd_configfs_exit(void);
+#endif
+
  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