[RESEND][PATCH v3 0/2] UFS SysFS attributes

[RESEND][PATCH v3 0/2] UFS SysFS attributes

[Michal Potomski]

From: Michał Potomski <michalx.potomski@intel.com>

This patchset implements new SysFS attribute to handle
Auto-Hibern8 feature and additionally rewrites present
attributes to adhere to new standard.

Changes in V3:
 - rewrite new and present attributes to adhere to
   DEVICE_ATTR_RW/RO and sdev_attr host template member
   policy
 - read/write from/to auto-hibern8 attribute will return
   -EOPNOTSUPP, when host has no such capability, intead
   of attribute not being created

Changes in V2:
 - minor formatting/style changes


Michał Potomski (2):
  scsi: ufs: Use DEVICE_ATTR_RW macros and sdev_attrs structure
  scsi: ufs: Implement Auto-Hibern8 setup

 Documentation/ABI/testing/sysfs-driver-scsi-ufs |   8 +
 drivers/scsi/ufs/ufshcd.c                       | 310 ++++++++++++++----------
 drivers/scsi/ufs/ufshci.h                       |  23 +-
 3 files changed, 208 insertions(+), 133 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-scsi-ufs

-- 
2.13.0.67.g10c78a1

--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.

[RESEND][PATCH v3 1/2] scsi: ufs: Use DEVICE_ATTR_RW macros and sdev_attrs structure

[Michal Potomski]

From: Michał Potomski <michalx.potomski@intel.com>

Previous implementation was obsolete and needed to be updated.
Additionally device_create_file() call occurs after a UFS device
has been made visible in sysfs and hence will cause trouble
(race condition) if a udev rule tries to set this attribute
from inside a rule that is triggered by device creation.

Signed-off-by: Michał Potomski <michalx.potomski@intel.com>
---
 drivers/scsi/ufs/ufshcd.c | 232 ++++++++++++++++++++--------------------------
 1 file changed, 103 insertions(+), 129 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 5bc9dc14e075..5e0fee6ab0db 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6490,6 +6490,108 @@ static enum blk_eh_timer_return ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
 	return found ? BLK_EH_NOT_HANDLED : BLK_EH_RESET_TIMER;
 }
 
+static inline ssize_t ufshcd_pm_lvl_store(struct device *dev,
+					   struct device_attribute *attr,
+					   const char *buf, size_t count,
+					   bool rpm)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+	unsigned long flags, value;
+
+	if (kstrtoul(buf, 0, &value))
+		return -EINVAL;
+
+	if (value >= UFS_PM_LVL_MAX)
+		return -EINVAL;
+
+	spin_lock_irqsave(hba->host->host_lock, flags);
+	if (rpm)
+		hba->rpm_lvl = value;
+	else
+		hba->spm_lvl = value;
+	spin_unlock_irqrestore(hba->host->host_lock, flags);
+	return count;
+}
+
+static ssize_t rpm_lvl_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+	int curr_len;
+	u8 lvl;
+
+	curr_len = snprintf(buf, PAGE_SIZE,
+			    "\nCurrent Runtime PM level [%d] => dev_state [%s] link_state [%s]\n",
+			    hba->rpm_lvl,
+			    ufschd_ufs_dev_pwr_mode_to_string(
+				ufs_pm_lvl_states[hba->rpm_lvl].dev_state),
+			    ufschd_uic_link_state_to_string(
+				ufs_pm_lvl_states[hba->rpm_lvl].link_state));
+
+	curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
+			     "\nAll available Runtime PM levels info:\n");
+	for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
+		curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
+				     "\tRuntime PM level [%d] => dev_state [%s] link_state [%s]\n",
+				    lvl,
+				    ufschd_ufs_dev_pwr_mode_to_string(
+					ufs_pm_lvl_states[lvl].dev_state),
+				    ufschd_uic_link_state_to_string(
+					ufs_pm_lvl_states[lvl].link_state));
+
+	return curr_len;
+}
+
+static ssize_t rpm_lvl_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	return ufshcd_pm_lvl_store(dev, attr, buf, count, true);
+}
+
+static ssize_t spm_lvl_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+	int curr_len;
+	u8 lvl;
+
+	curr_len = snprintf(buf, PAGE_SIZE,
+			    "\nCurrent System PM level [%d] => dev_state [%s] link_state [%s]\n",
+			    hba->spm_lvl,
+			    ufschd_ufs_dev_pwr_mode_to_string(
+				ufs_pm_lvl_states[hba->spm_lvl].dev_state),
+			    ufschd_uic_link_state_to_string(
+				ufs_pm_lvl_states[hba->spm_lvl].link_state));
+
+	curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
+			     "\nAll available System PM levels info:\n");
+	for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
+		curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
+				     "\tSystem PM level [%d] => dev_state [%s] link_state [%s]\n",
+				    lvl,
+				    ufschd_ufs_dev_pwr_mode_to_string(
+					ufs_pm_lvl_states[lvl].dev_state),
+				    ufschd_uic_link_state_to_string(
+					ufs_pm_lvl_states[lvl].link_state));
+
+	return curr_len;
+}
+
+static ssize_t spm_lvl_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	return ufshcd_pm_lvl_store(dev, attr, buf, count, false);
+}
+
+static DEVICE_ATTR_RW(rpm_lvl);
+static DEVICE_ATTR_RW(spm_lvl);
+
+static struct device_attribute *ufshcd_dev_attrs[] = {
+	&dev_attr_rpm_lvl,
+	&dev_attr_spm_lvl,
+	NULL,
+};
+
 static struct scsi_host_template ufshcd_driver_template = {
 	.module			= THIS_MODULE,
 	.name			= UFSHCD,
@@ -6509,6 +6611,7 @@ static struct scsi_host_template ufshcd_driver_template = {
 	.can_queue		= UFSHCD_CAN_QUEUE,
 	.max_host_blocked	= 1,
 	.track_queue_depth	= 1,
+	.sdev_attrs		= ufshcd_dev_attrs,
 };
 
 static int ufshcd_config_vreg_load(struct device *dev, struct ufs_vreg *vreg,
@@ -7578,133 +7681,6 @@ int ufshcd_runtime_idle(struct ufs_hba *hba)
 }
 EXPORT_SYMBOL(ufshcd_runtime_idle);
 
-static inline ssize_t ufshcd_pm_lvl_store(struct device *dev,
-					   struct device_attribute *attr,
-					   const char *buf, size_t count,
-					   bool rpm)
-{
-	struct ufs_hba *hba = dev_get_drvdata(dev);
-	unsigned long flags, value;
-
-	if (kstrtoul(buf, 0, &value))
-		return -EINVAL;
-
-	if (value >= UFS_PM_LVL_MAX)
-		return -EINVAL;
-
-	spin_lock_irqsave(hba->host->host_lock, flags);
-	if (rpm)
-		hba->rpm_lvl = value;
-	else
-		hba->spm_lvl = value;
-	spin_unlock_irqrestore(hba->host->host_lock, flags);
-	return count;
-}
-
-static ssize_t ufshcd_rpm_lvl_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
-{
-	struct ufs_hba *hba = dev_get_drvdata(dev);
-	int curr_len;
-	u8 lvl;
-
-	curr_len = snprintf(buf, PAGE_SIZE,
-			    "\nCurrent Runtime PM level [%d] => dev_state [%s] link_state [%s]\n",
-			    hba->rpm_lvl,
-			    ufschd_ufs_dev_pwr_mode_to_string(
-				ufs_pm_lvl_states[hba->rpm_lvl].dev_state),
-			    ufschd_uic_link_state_to_string(
-				ufs_pm_lvl_states[hba->rpm_lvl].link_state));
-
-	curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
-			     "\nAll available Runtime PM levels info:\n");
-	for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
-		curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
-				     "\tRuntime PM level [%d] => dev_state [%s] link_state [%s]\n",
-				    lvl,
-				    ufschd_ufs_dev_pwr_mode_to_string(
-					ufs_pm_lvl_states[lvl].dev_state),
-				    ufschd_uic_link_state_to_string(
-					ufs_pm_lvl_states[lvl].link_state));
-
-	return curr_len;
-}
-
-static ssize_t ufshcd_rpm_lvl_store(struct device *dev,
-		struct device_attribute *attr, const char *buf, size_t count)
-{
-	return ufshcd_pm_lvl_store(dev, attr, buf, count, true);
-}
-
-static void ufshcd_add_rpm_lvl_sysfs_nodes(struct ufs_hba *hba)
-{
-	hba->rpm_lvl_attr.show = ufshcd_rpm_lvl_show;
-	hba->rpm_lvl_attr.store = ufshcd_rpm_lvl_store;
-	sysfs_attr_init(&hba->rpm_lvl_attr.attr);
-	hba->rpm_lvl_attr.attr.name = "rpm_lvl";
-	hba->rpm_lvl_attr.attr.mode = 0644;
-	if (device_create_file(hba->dev, &hba->rpm_lvl_attr))
-		dev_err(hba->dev, "Failed to create sysfs for rpm_lvl\n");
-}
-
-static ssize_t ufshcd_spm_lvl_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
-{
-	struct ufs_hba *hba = dev_get_drvdata(dev);
-	int curr_len;
-	u8 lvl;
-
-	curr_len = snprintf(buf, PAGE_SIZE,
-			    "\nCurrent System PM level [%d] => dev_state [%s] link_state [%s]\n",
-			    hba->spm_lvl,
-			    ufschd_ufs_dev_pwr_mode_to_string(
-				ufs_pm_lvl_states[hba->spm_lvl].dev_state),
-			    ufschd_uic_link_state_to_string(
-				ufs_pm_lvl_states[hba->spm_lvl].link_state));
-
-	curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
-			     "\nAll available System PM levels info:\n");
-	for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
-		curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
-				     "\tSystem PM level [%d] => dev_state [%s] link_state [%s]\n",
-				    lvl,
-				    ufschd_ufs_dev_pwr_mode_to_string(
-					ufs_pm_lvl_states[lvl].dev_state),
-				    ufschd_uic_link_state_to_string(
-					ufs_pm_lvl_states[lvl].link_state));
-
-	return curr_len;
-}
-
-static ssize_t ufshcd_spm_lvl_store(struct device *dev,
-		struct device_attribute *attr, const char *buf, size_t count)
-{
-	return ufshcd_pm_lvl_store(dev, attr, buf, count, false);
-}
-
-static void ufshcd_add_spm_lvl_sysfs_nodes(struct ufs_hba *hba)
-{
-	hba->spm_lvl_attr.show = ufshcd_spm_lvl_show;
-	hba->spm_lvl_attr.store = ufshcd_spm_lvl_store;
-	sysfs_attr_init(&hba->spm_lvl_attr.attr);
-	hba->spm_lvl_attr.attr.name = "spm_lvl";
-	hba->spm_lvl_attr.attr.mode = 0644;
-	if (device_create_file(hba->dev, &hba->spm_lvl_attr))
-		dev_err(hba->dev, "Failed to create sysfs for spm_lvl\n");
-}
-
-static inline void ufshcd_add_sysfs_nodes(struct ufs_hba *hba)
-{
-	ufshcd_add_rpm_lvl_sysfs_nodes(hba);
-	ufshcd_add_spm_lvl_sysfs_nodes(hba);
-}
-
-static inline void ufshcd_remove_sysfs_nodes(struct ufs_hba *hba)
-{
-	device_remove_file(hba->dev, &hba->rpm_lvl_attr);
-	device_remove_file(hba->dev, &hba->spm_lvl_attr);
-}
-
 /**
  * ufshcd_shutdown - shutdown routine
  * @hba: per adapter instance
@@ -7742,7 +7718,6 @@ EXPORT_SYMBOL(ufshcd_shutdown);
  */
 void ufshcd_remove(struct ufs_hba *hba)
 {
-	ufshcd_remove_sysfs_nodes(hba);
 	scsi_remove_host(hba->host);
 	/* disable interrupts */
 	ufshcd_disable_intr(hba, hba->intr_mask);
@@ -7989,7 +7964,6 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
 	ufshcd_set_ufs_dev_active(hba);
 
 	async_schedule(ufshcd_async_scan, hba);
-	ufshcd_add_sysfs_nodes(hba);
 
 	return 0;
 
-- 
2.13.0.67.g10c78a1

--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.

[RESEND][PATCH v3 2/2] scsi: ufs: Implement Auto-Hibern8 setup

[Michal Potomski]

From: Michał Potomski <michalx.potomski@intel.com>

Since Auto-Hibern8 feature has to be enabled by the user
proper API has been given via SysFS.

We expose this API to user-space, since we don't know
in driver, what kind of additional Power Management rules
user wants to use. Due to that we want to expose API, to
let user or high level S/W decide, whether it wants to
use Auto-Hibern8 feature for Power Saving and give him
"slider" to decide between performance and power efficiency.
This is important because different platforms using
the same UFS host and/or device might want different
options on this one, e.g. High-End Desktop PC might
want to have this feature disabled, while Mobile or
Server platforms might want to have this feature enabled,
but levels will vary depending on what's to be acheived.

Setting up this feature should be fully transparent to
driver, since all states changing coming from setup
of this feature shall be handled by UFS Host, without any
interactions from driver. Only observable side effects
will be throughtput loss for transfers sporadic in terms
of set up timer.

Signed-off-by: Michał Potomski <michalx.potomski@intel.com>
---
 Documentation/ABI/testing/sysfs-driver-scsi-ufs |  8 +++
 drivers/scsi/ufs/ufshcd.c                       | 78 +++++++++++++++++++++++++
 drivers/scsi/ufs/ufshci.h                       | 23 ++++++--
 3 files changed, 105 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-scsi-ufs

diff --git a/Documentation/ABI/testing/sysfs-driver-scsi-ufs b/Documentation/ABI/testing/sysfs-driver-scsi-ufs
new file mode 100644
index 000000000000..db500197f6a9
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-scsi-ufs
@@ -0,0 +1,8 @@
+What:		/sys/bus/pci/devices/<bus>:<vid>:<pid>.<n>/auto_hibern8
+Date:		July 2017
+Contact:	linux-scsi@vger.kernel.org
+Description:
+		This attribiute provides information on current setting of
+		Auto-Hibern8 UFS Host feature (read <unit: microseconds>)
+		and ability to set the feature on - by writing desired
+		timeout value <unit: microseconds>, or off (by writing '0').
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 5e0fee6ab0db..8c6ae132bda4 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -931,6 +931,31 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up)
 }
 
 /**
+ * ufshcd_read_auto_hibern8_state - Reads hosts auto-hibern8 feature state
+ * @hba: per adapter instance
+ */
+u32 ufshcd_read_auto_hibern8_state(struct ufs_hba *hba)
+{
+	return ufshcd_readl(hba, REG_AUTO_HIBERN8_IDLE_TIMER);
+}
+
+/**
+ * ufshcd_setup_auto_hibern8 - Sets up hosts auto-hibern8 feature
+ * @hba: per adapter instance
+ * @scale: timer scale (1/10/100us/1/10/100ms)
+ * @timer_val: value to be multipled with scale (idle timeout)
+ */
+void ufshcd_setup_auto_hibern8(struct ufs_hba *hba, u8 scale, u16 timer_val)
+{
+	u32 val = (scale << UFSHCI_AHIBERN8_SCALE_OFFSET)
+			& UFSHCI_AHIBERN8_SCALE_MASK;
+
+	val |= timer_val & UFSHCI_AHIBERN8_TIMER_MASK;
+
+	ufshcd_writel(hba, val, REG_AUTO_HIBERN8_IDLE_TIMER);
+}
+
+/**
  * ufshcd_is_devfreq_scaling_required - check if scaling is required or not
  * @hba: per adapter instance
  * @scale_up: True if scaling up and false if scaling down
@@ -6583,12 +6608,65 @@ static ssize_t spm_lvl_store(struct device *dev,
 	return ufshcd_pm_lvl_store(dev, attr, buf, count, false);
 }
 
+#define UFS_AHIBERN8_SCALE_STEP_MAGNITUDE	10
+
+static ssize_t auto_hibern8_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+	u32 val;
+	unsigned long timer;
+	u8 scale;
+
+	if (!(hba->capabilities & MASK_AUTO_HIBERN8_SUPPORT))
+		return -EOPNOTSUPP;
+
+	val = ufshcd_read_auto_hibern8_state(hba);
+	timer = val & UFSHCI_AHIBERN8_TIMER_MASK;
+	scale =	(val & UFSHCI_AHIBERN8_SCALE_MASK)
+			>> UFSHCI_AHIBERN8_SCALE_OFFSET;
+
+	for (; scale > 0; --scale)
+		timer *= UFS_AHIBERN8_SCALE_STEP_MAGNITUDE;
+
+	return snprintf(buf, PAGE_SIZE, "%ld\n", timer);
+}
+
+static ssize_t auto_hibern8_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+	unsigned long timer;
+	u8 scale = UFSHCI_AHIBERN8_SCALE_1US;
+
+	if (!(hba->capabilities & MASK_AUTO_HIBERN8_SUPPORT))
+		return -EOPNOTSUPP;
+
+	if (kstrtoul(buf, 0, &timer))
+		return -EINVAL;
+
+	while (timer > UFSHCI_AHIBERN8_TIMER_MASK &&
+	       scale < UFSHCI_AHIBERN8_SCALE_MAX) {
+		timer /= UFS_AHIBERN8_SCALE_STEP_MAGNITUDE;
+		++scale;
+	}
+
+	if (scale >= UFSHCI_AHIBERN8_SCALE_MAX)
+		return -EINVAL;
+
+	ufshcd_setup_auto_hibern8(hba, scale, (u16) timer);
+
+	return count;
+}
+
 static DEVICE_ATTR_RW(rpm_lvl);
 static DEVICE_ATTR_RW(spm_lvl);
+static DEVICE_ATTR_RW(auto_hibern8);
 
 static struct device_attribute *ufshcd_dev_attrs[] = {
 	&dev_attr_rpm_lvl,
 	&dev_attr_spm_lvl,
+	&dev_attr_auto_hibern8,
 	NULL,
 };
 
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index f60145d4a66e..3bf8826318b0 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -48,7 +48,7 @@ enum {
 	REG_UFS_VERSION				= 0x08,
 	REG_CONTROLLER_DEV_ID			= 0x10,
 	REG_CONTROLLER_PROD_ID			= 0x14,
-	REG_AUTO_HIBERNATE_IDLE_TIMER		= 0x18,
+	REG_AUTO_HIBERN8_IDLE_TIMER		= 0x18,
 	REG_INTERRUPT_STATUS			= 0x20,
 	REG_INTERRUPT_ENABLE			= 0x24,
 	REG_CONTROLLER_STATUS			= 0x30,
@@ -86,6 +86,7 @@ enum {
 enum {
 	MASK_TRANSFER_REQUESTS_SLOTS		= 0x0000001F,
 	MASK_TASK_MANAGEMENT_REQUEST_SLOTS	= 0x00070000,
+	MASK_AUTO_HIBERN8_SUPPORT		= 0x00800000,
 	MASK_64_ADDRESSING_SUPPORT		= 0x01000000,
 	MASK_OUT_OF_ORDER_DATA_DELIVERY_SUPPORT	= 0x02000000,
 	MASK_UIC_DME_TEST_MODE_SUPPORT		= 0x04000000,
@@ -136,9 +137,9 @@ enum {
 #define CONTROLLER_FATAL_ERROR			UFS_BIT(16)
 #define SYSTEM_BUS_FATAL_ERROR			UFS_BIT(17)
 
-#define UFSHCD_UIC_PWR_MASK	(UIC_HIBERNATE_ENTER |\
-				UIC_HIBERNATE_EXIT |\
-				UIC_POWER_MODE)
+#define UFSHCD_UHS_MASK		(UIC_HIBERNATE_EXIT | UIC_HIBERNATE_ENTER)
+
+#define UFSHCD_UIC_PWR_MASK	(UFSHCD_UHS_MASK | UIC_POWER_MODE)
 
 #define UFSHCD_UIC_MASK		(UIC_COMMAND_COMPL | UFSHCD_UIC_PWR_MASK)
 
@@ -428,4 +429,18 @@ struct utp_task_req_desc {
 	__le32 task_rsp_upiu[TASK_RSP_UPIU_SIZE_DWORDS];
 };
 
+enum {
+	UFSHCI_AHIBERN8_SCALE_1US	= 0,
+	UFSHCI_AHIBERN8_SCALE_10US	= 1,
+	UFSHCI_AHIBERN8_SCALE_100US	= 2,
+	UFSHCI_AHIBERN8_SCALE_1MS	= 3,
+	UFSHCI_AHIBERN8_SCALE_10MS	= 4,
+	UFSHCI_AHIBERN8_SCALE_100MS	= 5,
+	UFSHCI_AHIBERN8_SCALE_MAX,
+};
+
+#define UFSHCI_AHIBERN8_TIMER_MASK		0x03ff
+#define UFSHCI_AHIBERN8_SCALE_MASK		0x1C00
+#define UFSHCI_AHIBERN8_SCALE_OFFSET		10
+
 #endif /* End of Header */
-- 
2.13.0.67.g10c78a1

--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.

Re: [RESEND][PATCH v3 1/2] scsi: ufs: Use DEVICE_ATTR_RW macros and sdev_attrs structure

[Bart Van Assche]

On Wed, 2017-07-26 at 16:55 +0200, Michal Potomski wrote:
> Previous implementation was obsolete and needed to be updated.
> Additionally device_create_file() call occurs after a UFS device
> has been made visible in sysfs and hence will cause trouble
> (race condition) if a udev rule tries to set this attribute
> from inside a rule that is triggered by device creation.

Hello Michal,

Thank you for having done this work!

> +static ssize_t rpm_lvl_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct ufs_hba *hba = dev_get_drvdata(dev);
> +	int curr_len;
> +	u8 lvl;
> +
> +	curr_len = snprintf(buf, PAGE_SIZE,
> +			    "\nCurrent Runtime PM level [%d] => dev_state [%s] link_state [%s]\n",
> +			    hba->rpm_lvl,
> +			    ufschd_ufs_dev_pwr_mode_to_string(
> +				ufs_pm_lvl_states[hba->rpm_lvl].dev_state),
> +			    ufschd_uic_link_state_to_string(
> +				ufs_pm_lvl_states[hba->rpm_lvl].link_state));
> +
> +	curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> +			     "\nAll available Runtime PM levels info:\n");
> +	for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
> +		curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> +				     "\tRuntime PM level [%d] => dev_state [%s] link_state [%s]\n",
> +				    lvl,
> +				    ufschd_ufs_dev_pwr_mode_to_string(
> +					ufs_pm_lvl_states[lvl].dev_state),
> +				    ufschd_uic_link_state_to_string(
> +					ufs_pm_lvl_states[lvl].link_state));
> +
> +	return curr_len;
> +}

I am aware that this code is existing code that has been moved up. But are
the UFS contributors aware that this kind of output violates the sysfs rules?
Please consider either to move this attribute to debugfs (the one value per
file rule only applies to sysfs and configfs) or to split it into multiple
sysfs attributes if that is possible without breaking existing user space
applications. A quote from
https://www.kernel.org/doc/Documentation/filesystems/sysfs.txt: "Attributes
should be ASCII text files, preferably with only one value per file. It is
noted that it may not be efficient to contain only one value per file, so it
is socially acceptable to express an array of values of the same type."

Anyway, since this patch is an improvement compared to the current code base:

Reviewed-by: Bart Van Assche <bart.vanassche@wdc.com>