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

[PATCH V2 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 V2 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. Sysfs support
is added to trigger provisioning.
Device and Unit configurable parameters are parsed from
vendor specific provisioning data or file and
passed via sysfs 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 | 181 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd.h |   1 +
 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..8975449 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"
@@ -237,6 +238,7 @@ static int ufshcd_config_pwr_mode(struct ufs_hba *hba,
 		struct ufs_pa_layer_attr *desired_pwr_mode);
 static int ufshcd_change_power_mode(struct ufs_hba *hba,
 			     struct ufs_pa_layer_attr *pwr_mode);
+static int ufshcd_do_config_device(struct ufs_hba *hba);
 static inline bool ufshcd_valid_tag(struct ufs_hba *hba, int tag)
 {
 	return tag >= 0 && tag < hba->nutrs;
@@ -3063,6 +3065,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 +6354,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.
+ */
+static 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..982bfdf 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
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH V2 3/3] scsi: ufs: Add sysfs support for ufs provision

[Sayali Lokhande]

Add sysfs support to trigger ufs provisioning at runtime.
Usage : echo <desc_buf> > /sys/bus/platform/drivers/*/
	config_descriptor/ufs_provision
To check provisioning status:
cat /sys/bus/platform/drivers/*/config_descriptor/ufs_provision
1- > Success (Reboot device to check updated provisioning)

Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
---
 Documentation/ABI/testing/sysfs-driver-ufs |  16 ++++
 drivers/scsi/ufs/ufs-sysfs.c               |  25 ++++++
 drivers/scsi/ufs/ufs.h                     |   2 +
 drivers/scsi/ufs/ufshcd.c                  | 128 +++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufshcd.h                  |   5 ++
 5 files changed, 176 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs
index 016724e..43419b5 100644
--- a/Documentation/ABI/testing/sysfs-driver-ufs
+++ b/Documentation/ABI/testing/sysfs-driver-ufs
@@ -883,3 +883,19 @@ Contact:	Subhash Jadavani <subhashj@codeaurora.org>
 Description:	This entry shows the target state of an UFS UIC link
 		for the chosen system power management level.
 		The file is read only.
+
+What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/ufs_provision
+Date:		February 2018
+Contact:	Sayali Lokhande <sayalil@codeaurora.org>
+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> > ufs_provision
+		To check updated configuration check unit_descriptor and
+		device_descriptor sysfs fields.
diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index 8d9332b..8b68813 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -252,6 +252,30 @@ static ssize_t ufs_sysfs_read_desc_param(struct ufs_hba *hba,
 	return ret;
 }
 
+static ssize_t ufs_provision_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	return ufshcd_desc_config_show(dev, attr, buf);
+}
+
+static ssize_t ufs_provision_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	return ufshcd_desc_config_store(dev, attr, buf, count);
+}
+
+DEVICE_ATTR_RW(ufs_provision);
+
+static struct attribute *ufs_sysfs_config_descriptor[] = {
+	&dev_attr_ufs_provision.attr,
+	NULL,
+};
+
+static const struct attribute_group ufs_sysfs_config_descriptor_group = {
+	.name = "config_descriptor",
+	.attrs = ufs_sysfs_config_descriptor,
+};
+
 #define UFS_DESC_PARAM(_name, _puname, _duname, _size)			\
 static ssize_t _name##_show(struct device *dev,				\
 	struct device_attribute *attr, char *buf)			\
@@ -713,6 +737,7 @@ static DEVICE_ATTR_RO(_name)
 static const struct attribute_group *ufs_sysfs_groups[] = {
 	&ufs_sysfs_default_group,
 	&ufs_sysfs_device_descriptor_group,
+	&ufs_sysfs_config_descriptor_group,
 	&ufs_sysfs_interconnect_descriptor_group,
 	&ufs_sysfs_geometry_descriptor_group,
 	&ufs_sysfs_health_descriptor_group,
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 8975449..26b39c8 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1581,6 +1581,131 @@ void ufshcd_release(struct ufs_hba *hba)
 }
 EXPORT_SYMBOL_GPL(ufshcd_release);
 
+ssize_t ufshcd_desc_config_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "provision_enabled = %d\n",
+		hba->provision_enabled);
+}
+
+ssize_t ufshcd_desc_config_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+	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 ufshcd_clkgate_delay_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
@@ -6503,6 +6628,9 @@ static int ufshcd_do_config_device(struct ufs_hba *hba)
 		pt = pt + 5; // Reserved fields set to 0
 	}
 
+	for (i = 0; i < buff_len; i++)
+		dev_dbg(hba->dev, " desc_buf[%d] 0x%x", i, desc_buf[i]);
+
 	ret = ufshcd_query_descriptor_retry(hba, UPIU_QUERY_OPCODE_WRITE_DESC,
 		QUERY_DESC_IDN_CONFIGURATION, 0, 0, desc_buf, &buff_len);
 
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 982bfdf..1b8304f 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -651,6 +651,7 @@ struct ufs_hba {
 	struct ufs_pwr_mode_info max_pwr_info;
 
 	struct ufs_clk_gating clk_gating;
+	bool provision_enabled;
 	/* Control to enable/disable host capabilities */
 	u32 caps;
 	/* Allow dynamic clk gating */
@@ -867,6 +868,10 @@ 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);
+ssize_t ufshcd_desc_config_show(struct device *dev,
+		struct device_attribute *attr, char *buf);
+ssize_t ufshcd_desc_config_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count);
 
 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 V2 1/3] scsi: ufs: set the device reference clock setting

[Adrian Hunter]

On 08/06/18 14:06, 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>
> ---
>  .../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 is 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 V2 3/3] scsi: ufs: Add sysfs support for ufs provision

[Greg Kroah-Hartman]

On Fri, Jun 08, 2018 at 04:36:30PM +0530, Sayali Lokhande wrote:
> Add sysfs support to trigger ufs provisioning at runtime.
> Usage : echo <desc_buf> > /sys/bus/platform/drivers/*/
> 	config_descriptor/ufs_provision
> To check provisioning status:
> cat /sys/bus/platform/drivers/*/config_descriptor/ufs_provision
> 1- > Success (Reboot device to check updated provisioning)
> 
> Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
> ---
>  Documentation/ABI/testing/sysfs-driver-ufs |  16 ++++
>  drivers/scsi/ufs/ufs-sysfs.c               |  25 ++++++
>  drivers/scsi/ufs/ufs.h                     |   2 +
>  drivers/scsi/ufs/ufshcd.c                  | 128 +++++++++++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd.h                  |   5 ++
>  5 files changed, 176 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-ufs b/Documentation/ABI/testing/sysfs-driver-ufs
> index 016724e..43419b5 100644
> --- a/Documentation/ABI/testing/sysfs-driver-ufs
> +++ b/Documentation/ABI/testing/sysfs-driver-ufs
> @@ -883,3 +883,19 @@ Contact:	Subhash Jadavani <subhashj@codeaurora.org>
>  Description:	This entry shows the target state of an UFS UIC link
>  		for the chosen system power management level.
>  		The file is read only.
> +
> +What:		/sys/bus/platform/drivers/ufshcd/*/config_descriptor/ufs_provision
> +Date:		February 2018
> +Contact:	Sayali Lokhande <sayalil@codeaurora.org>
> +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> > ufs_provision

That's crazy big, please use configfs for this if you really want to
"provision" hardware with a new api.  That is what configfs was created
for, do not abuse sysfs like this, it is not a "single" value.

thanks,

greg k-h

[RFC PATCH] scsi: ufs: dev_attr_ufs_provision can be static

[kbuild test robot]

Fixes: 2f7e32b8cdc9 ("scsi: ufs: Add sysfs support for ufs provision")
Signed-off-by: kbuild test robot <fengguang.wu@intel.com>
---
 ufs-sysfs.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index 8b68813..36c28cc 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -264,7 +264,7 @@ static ssize_t ufs_provision_store(struct device *dev,
 	return ufshcd_desc_config_store(dev, attr, buf, count);
 }
 
-DEVICE_ATTR_RW(ufs_provision);
+static DEVICE_ATTR_RW(ufs_provision);
 
 static struct attribute *ufs_sysfs_config_descriptor[] = {
 	&dev_attr_ufs_provision.attr,

Re: [PATCH V2 3/3] scsi: ufs: Add sysfs support for ufs provision

[kbuild test robot]

Hi Sayali,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.17 next-20180608]
[cannot apply to mkp-scsi/for-next scsi/for-next]
[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/20180609-151315
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/scsi/ufs/ufs-sysfs.c:267:1: sparse: symbol 'dev_attr_ufs_provision' was not declared. Should it be static?

Please review and possibly fold the followup patch.

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

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

[Rob Herring]

On Fri, Jun 08, 2018 at 04:36:28PM +0530, 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>
> ---
>  .../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.

I must have misunderstood your last response. I thought you could handle 
things without DT. If not, my question remains.

Rob

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

[sayali]

Comment inline.

Thanks,
Sayali
-----Original Message-----
From: Rob Herring [mailto:robh@kernel.org] 
Sent: Wednesday, June 13, 2018 12:57 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; linux-scsi@vger.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 V2 1/3] scsi: ufs: set the device reference clock
setting

On Fri, Jun 08, 2018 at 04:36:28PM +0530, 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>
> ---
>  .../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.

I must have misunderstood your last response. I thought you could handle
things without DT. If not, my question remains.
[Sayali]: Ref clk frequency setting could vary from
platfrom-to-platform(vendor specific). Hence we need to pass it via DT.
	Currently in DT we do not set/mention any ref clk frequency
parameter. Hence I have added one new DT entry to configure
	required ref clk freq.

Rob

RE: [PATCH V2 3/3] scsi: ufs: Add sysfs support for ufs provision

[sayali]

-----Original Message-----
From: Greg Kroah-Hartman [mailto:gregkh@linuxfoundation.org] 
Sent: Saturday, June 09, 2018 12:55 PM
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; linux-scsi@vger.kernel.org;
Stanislav Nijnikov <stanislav.nijnikov@wdc.com>; Adrian Hunter
<adrian.hunter@intel.com>; open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH V2 3/3] scsi: ufs: Add sysfs support for ufs provision

On Fri, Jun 08, 2018 at 04:36:30PM +0530, Sayali Lokhande wrote:
> Add sysfs support to trigger ufs provisioning at runtime.
> Usage : echo <desc_buf> > /sys/bus/platform/drivers/*/
> 	config_descriptor/ufs_provision
> To check provisioning status:
> cat /sys/bus/platform/drivers/*/config_descriptor/ufs_provision
> 1- > Success (Reboot device to check updated provisioning)
> 
> Signed-off-by: Sayali Lokhande <sayalil@codeaurora.org>
> ---
>  Documentation/ABI/testing/sysfs-driver-ufs |  16 ++++
>  drivers/scsi/ufs/ufs-sysfs.c               |  25 ++++++
>  drivers/scsi/ufs/ufs.h                     |   2 +
>  drivers/scsi/ufs/ufshcd.c                  | 128
+++++++++++++++++++++++++++++
>  drivers/scsi/ufs/ufshcd.h                  |   5 ++
>  5 files changed, 176 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-ufs 
> b/Documentation/ABI/testing/sysfs-driver-ufs
> index 016724e..43419b5 100644
> --- a/Documentation/ABI/testing/sysfs-driver-ufs
> +++ b/Documentation/ABI/testing/sysfs-driver-ufs
> @@ -883,3 +883,19 @@ Contact:	Subhash Jadavani <subhashj@codeaurora.org>
>  Description:	This entry shows the target state of an UFS UIC link
>  		for the chosen system power management level.
>  		The file is read only.
> +
> +What:
/sys/bus/platform/drivers/ufshcd/*/config_descriptor/ufs_provision
> +Date:		February 2018
> +Contact:	Sayali Lokhande <sayalil@codeaurora.org>
> +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> > ufs_provision

That's crazy big, please use configfs for this if you really want to
"provision" hardware with a new api.  That is what configfs was created for,
do not abuse sysfs like this, it is not a "single" value.
[Sayali] Sure. I will check and update my next patchset accordingly to use
configfs for provisioning at runtime.

thanks,

greg k-h

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

[Bart Van Assche]

On Thu, 2018-06-14 at 17:03 +0530, sayali wrote:
> Comment inline.

Hello Sayali,

Please don't use inline replies. That makes it very hard to follow the
conversation. BTW, in the headers of your e-mail I found the following:
"X-Mailer: Microsoft Outlook 16.0". Please use another e-mail client that is
better suited for collaboration on open source mailing lists. If outgoing
e-mails have to pass through an Exchange server, the following e-mail clients
support Exchange servers and are better suited for open source collaboration:
* Evolution (Linux only).
* Thunderbird + ExQuilla plugin.
* If IMAP has been enabled on the Exchange server that you are using then
  that means that you can choose from the many open source e-mail clients
  that support IMAP.

Thanks,

Bart.

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

[Rob Herring]

On Thu, Jun 14, 2018 at 5:33 AM, sayali <sayalil@codeaurora.org> wrote:
> Comment inline.
>
> Thanks,
> Sayali
> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Wednesday, June 13, 2018 12:57 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; linux-scsi@vger.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 V2 1/3] scsi: ufs: set the device reference clock
> setting
>
> On Fri, Jun 08, 2018 at 04:36:28PM +0530, 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>
>> ---
>>  .../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.
>
> I must have misunderstood your last response. I thought you could handle
> things without DT. If not, my question remains.
> [Sayali]: Ref clk frequency setting could vary from
> platfrom-to-platform(vendor specific). Hence we need to pass it via DT.
>         Currently in DT we do not set/mention any ref clk frequency
> parameter. Hence I have added one new DT entry to configure
>         required ref clk freq.

The clocks property contains "ref_clk". Is that not the same clock?
Why can't you read what that frequency is? Or you need to be able to
change it to a specific frequency? These all look like typical
oscillator frequencies, so I wouldn't expect you could change them
(other than divide by 2 maybe).

Rob

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

[valdis.kletnieks]

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

Actually, it makes it easier to follow the conversation.

On Thu, 14 Jun 2018 13:35:08 -0000, Bart Van Assche said:
> On Thu, 2018-06-14 at 17:03 +0530, sayali wrote:
> > Comment inline.
>
> Hello Sayali,
>
> Please don't use inline replies. That makes it very hard to follow the
> conversation.

Actually, it makes it easier to follow the conversation.

> * If IMAP has been enabled on the Exchange server that you are using then
>   that means that you can choose from the many open source e-mail clients
>   that support IMAP.

Which of the two is easier to follow?


[-- Attachment #2: Type: application/pgp-signature, Size: 486 bytes --]

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

[Bart Van Assche]

On Fri, 2018-06-15 at 16:37 -0400, valdis.kletnieks@vt.edu wrote:
> Actually, it makes it easier to follow the conversation.

In case I wasn't clear enough: I was referring to replying inline
*without* quoting the original message. That makes a conversation
*really* hard to follow.

Bart.

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

[valdis.kletnieks]

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

On Fri, 15 Jun 2018 20:42:41 -0000, Bart Van Assche said:

> In case I wasn't clear enough: I was referring to replying inline
> *without* quoting the original message. That makes a conversation
> *really* hard to follow.

So if there's no quoted message, what is the inline reply inlined into? :)

[-- Attachment #2: Type: application/pgp-signature, Size: 486 bytes --]

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

[Bart Van Assche]

On Fri, 2018-06-15 at 16:53 -0400, valdis.kletnieks@vt.edu wrote:
> On Fri, 15 Jun 2018 20:42:41 -0000, Bart Van Assche said:
> > In case I wasn't clear enough: I was referring to replying inline
> > *without* quoting the original message. That makes a conversation
> > *really* hard to follow.
> 
> So if there's no quoted message, what is the inline reply inlined into? :)

I was referring to quoting without indenting the quote with "> ". That
should already have been clear if you had a look at the message I complained
about.

Bart.

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

[sayali]

Hi Rob,

Please check my comment inline.

Thanks,
Sayali
-----Original Message-----
From: Rob Herring [mailto:robh@kernel.org] 
Sent: Thursday, June 14, 2018 7:59 PM
To: sayali <sayalil@codeaurora.org>
Cc: Subhash Jadavani <subhashj@codeaurora.org>; Can Guo <cang@codeaurora.org>; Vivek Gautam <vivek.gautam@codeaurora.org>; Rajendra Nayak <rnayak@codeaurora.org>; Vinayak Holikatti <vinholikatti@gmail.com>; James E.J. Bottomley <jejb@linux.vnet.ibm.com>; Martin K. Petersen <martin.petersen@oracle.com>; asutoshd@codeaurora.org; Evan Green <evgreen@chromium.org>; linux-scsi@vger.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 V2 1/3] scsi: ufs: set the device reference clock setting

On Thu, Jun 14, 2018 at 5:33 AM, sayali <sayalil@codeaurora.org> wrote:
> Comment inline.
>
> Thanks,
> Sayali
> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Wednesday, June 13, 2018 12:57 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; linux-scsi@vger.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 V2 1/3] scsi: ufs: set the device reference clock 
> setting
>
> On Fri, Jun 08, 2018 at 04:36:28PM +0530, 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>
>> ---
>>  .../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.
>
> I must have misunderstood your last response. I thought you could 
> handle things without DT. If not, my question remains.
> [Sayali]: Ref clk frequency setting could vary from 
> platfrom-to-platform(vendor specific). Hence we need to pass it via DT.
>         Currently in DT we do not set/mention any ref clk frequency 
> parameter. Hence I have added one new DT entry to configure
>         required ref clk freq.

The clocks property contains "ref_clk". Is that not the same clock?
Why can't you read what that frequency is? Or you need to be able to change it to a specific frequency? These all look like typical oscillator frequencies, so I wouldn't expect you could change them (other than divide by 2 maybe).
[Sayali] : It is the same "ref_clk", but we need to be able to change it to a specific frequency as per requirement. Thus, we need new DT entry to specify/override reference clock frequency as per need.

Rob

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

[Rob Herring]

On Thu, Jun 21, 2018 at 2:52 AM sayali <sayalil@codeaurora.org> wrote:
>
> Hi Rob,
>
> Please check my comment inline.

As mentioned in the back and forth comments previously in this thread,
please fix your email client (hint: you can't use Outlook) and
properly quote your replies (i.e. the leading ">")

> Thanks,
> Sayali
> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Thursday, June 14, 2018 7:59 PM
> To: sayali <sayalil@codeaurora.org>
> Cc: Subhash Jadavani <subhashj@codeaurora.org>; Can Guo <cang@codeaurora.org>; Vivek Gautam <vivek.gautam@codeaurora.org>; Rajendra Nayak <rnayak@codeaurora.org>; Vinayak Holikatti <vinholikatti@gmail.com>; James E.J. Bottomley <jejb@linux.vnet.ibm.com>; Martin K. Petersen <martin.petersen@oracle.com>; asutoshd@codeaurora.org; Evan Green <evgreen@chromium.org>; linux-scsi@vger.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 V2 1/3] scsi: ufs: set the device reference clock setting
>
> On Thu, Jun 14, 2018 at 5:33 AM, sayali <sayalil@codeaurora.org> wrote:
> > Comment inline.
> >
> > Thanks,
> > Sayali
> > -----Original Message-----
> > From: Rob Herring [mailto:robh@kernel.org]
> > Sent: Wednesday, June 13, 2018 12:57 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; linux-scsi@vger.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 V2 1/3] scsi: ufs: set the device reference clock
> > setting
> >
> > On Fri, Jun 08, 2018 at 04:36:28PM +0530, 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>
> >> ---
> >>  .../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.
> >
> > I must have misunderstood your last response. I thought you could
> > handle things without DT. If not, my question remains.
> > [Sayali]: Ref clk frequency setting could vary from
> > platfrom-to-platform(vendor specific). Hence we need to pass it via DT.
> >         Currently in DT we do not set/mention any ref clk frequency
> > parameter. Hence I have added one new DT entry to configure
> >         required ref clk freq.
>
> The clocks property contains "ref_clk". Is that not the same clock?
> Why can't you read what that frequency is? Or you need to be able to change it to a specific frequency? These all look like typical oscillator frequencies, so I wouldn't expect you could change them (other than divide by 2 maybe).
> [Sayali] : It is the same "ref_clk", but we need to be able to change it to a specific frequency as per requirement. Thus, we need new DT entry to specify/override reference clock frequency as per need.

That is not what your patch does. It just tells the device what the
frequency is. If you need to get the rate, use "clk_get_rate" on
"ref_clk". If you need to actually set it to a specific frequency,
then we have properties for that already (assigned-clock-rates).

Seems to me that by the time you get to Linux, the bootloader would
have already set this. Otherwise, how do you boot? Seems like you
would want to read the attr and ensure "ref_clk" freq matches.

Rob

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

[Sayali Lokhande]

On 7/3/2018 11:34 PM, Rob Herring wrote:
> On Thu, Jun 21, 2018 at 2:52 AM sayali <sayalil@codeaurora.org> wrote:
>> Hi Rob,
>>
>> Please check my comment inline.
> As mentioned in the back and forth comments previously in this thread,
> please fix your email client (hint: you can't use Outlook) and
> properly quote your replies (i.e. the leading ">")
I have started using Thunderbird now. Hope you can see my replies 
correctly this time.
>
>> Thanks,
>> Sayali
>> -----Original Message-----
>> From: Rob Herring [mailto:robh@kernel.org]
>> Sent: Thursday, June 14, 2018 7:59 PM
>> To: sayali <sayalil@codeaurora.org>
>> Cc: Subhash Jadavani <subhashj@codeaurora.org>; Can Guo <cang@codeaurora.org>; Vivek Gautam <vivek.gautam@codeaurora.org>; Rajendra Nayak <rnayak@codeaurora.org>; Vinayak Holikatti <vinholikatti@gmail.com>; James E.J. Bottomley <jejb@linux.vnet.ibm.com>; Martin K. Petersen <martin.petersen@oracle.com>; asutoshd@codeaurora.org; Evan Green <evgreen@chromium.org>; linux-scsi@vger.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 V2 1/3] scsi: ufs: set the device reference clock setting
>>
>> On Thu, Jun 14, 2018 at 5:33 AM, sayali <sayalil@codeaurora.org> wrote:
>>> Comment inline.
>>>
>>> Thanks,
>>> Sayali
>>> -----Original Message-----
>>> From: Rob Herring [mailto:robh@kernel.org]
>>> Sent: Wednesday, June 13, 2018 12:57 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; linux-scsi@vger.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 V2 1/3] scsi: ufs: set the device reference clock
>>> setting
>>>
>>> On Fri, Jun 08, 2018 at 04:36:28PM +0530, 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>
>>>> ---
>>>>   .../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.
>>> I must have misunderstood your last response. I thought you could
>>> handle things without DT. If not, my question remains.
>>> [Sayali]: Ref clk frequency setting could vary from
>>> platfrom-to-platform(vendor specific). Hence we need to pass it via DT.
>>>          Currently in DT we do not set/mention any ref clk frequency
>>> parameter. Hence I have added one new DT entry to configure
>>>          required ref clk freq.
>> The clocks property contains "ref_clk". Is that not the same clock?
>> Why can't you read what that frequency is? Or you need to be able to change it to a specific frequency? These all look like typical oscillator frequencies, so I wouldn't expect you could change them (other than divide by 2 maybe).
>> [Sayali] : It is the same "ref_clk", but we need to be able to change it to a specific frequency as per requirement. Thus, we need new DT entry to specify/override reference clock frequency as per need.
> That is not what your patch does. It just tells the device what the
> frequency is. If you need to get the rate, use "clk_get_rate" on
> "ref_clk". If you need to actually set it to a specific frequency,
> then we have properties for that already (assigned-clock-rates).
Yes, we need to set it to a specific frequency. I will use 
"assigned-clock-rates" for passing ref_clk frequency in next patch set.
>
> Seems to me that by the time you get to Linux, the bootloader would
> have already set this. Otherwise, how do you boot? Seems like you
> would want to read the attr and ensure "ref_clk" freq matches.
Yes, ref_clk freq will be already set in device via bootloader. In 
Kernel, before updating ref_clk freq (if required and passed via DT), we 
are reading the current ref_clk set in device and only if it is 
different than what has been passed via DT, we will be updating ref_clk 
freq, otherwise we just return.
> Rob
Thanks,
Sayali