[RESEND PATCH 0/5] scsi: ufs: ufs device as a temperature sensor

[RESEND PATCH 0/5] scsi: ufs: ufs device as a temperature sensor

[Avi Shchislowski]

UFS3.0 allows using the ufs device as a temperature sensor. The
purpose of this feature is to provide notification to the host of the
UFS device case temperature. It allows reading of a rough estimate
(+-10 degrees centigrade) of the current case temperature, And
setting a lower and upper temperature bounds, in which the device
will trigger an applicable exception event.

We added the capability of responding to such notifications, while
notifying the kernel's thermal core, which further exposes the thermal
zone attributes to user space. UFS temperature attributes are all
read-only, so only thermal read ops (.get_xxx) can be implemented.

We are re-spinning this patchset per the request of Daniel Lezcano
to cc the thermal maintainers.

Avi Shchislowski (5):
  scsi: ufs: Add ufs thermal support
  scsi: ufs: export ufshcd_enable_ee
  scsi: ufs: enable thermal exception event
  scsi: ufs-thermal: implement thermal file ops
  scsi: ufs: temperature atrributes add to ufs_sysfs

 drivers/scsi/ufs/Kconfig       |  11 ++
 drivers/scsi/ufs/Makefile      |   1 +
 drivers/scsi/ufs/ufs-sysfs.c   |   6 +
 drivers/scsi/ufs/ufs-thermal.c | 249 +++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufs-thermal.h |  25 +++++
 drivers/scsi/ufs/ufs.h         |  20 +++-
 drivers/scsi/ufs/ufshcd.c      |   9 +-
 drivers/scsi/ufs/ufshcd.h      |  12 ++
 8 files changed, 331 insertions(+), 2 deletions(-)
 create mode 100644 drivers/scsi/ufs/ufs-thermal.c
 create mode 100644 drivers/scsi/ufs/ufs-thermal.h

-- 
1.9.1

[RESEND PATCH 1/5] scsi: ufs: Add ufs thermal support

[Avi Shchislowski]

From: Avi Shchislowski <avi.shchislowski@sandisk.com>

Support the new temperature notification attributes introduced in
UFSv3.0. Add exception event mask, and ufs features attributes.

Signed-off-by: Avi Shchislowski <avi.shchislowski@sandisk.com>
---
 drivers/scsi/ufs/Kconfig       |  11 ++++
 drivers/scsi/ufs/Makefile      |   1 +
 drivers/scsi/ufs/ufs-thermal.c | 123 +++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/ufs/ufs-thermal.h |  19 +++++++
 drivers/scsi/ufs/ufs.h         |  11 ++++
 drivers/scsi/ufs/ufshcd.c      |   3 +
 drivers/scsi/ufs/ufshcd.h      |  10 ++++
 7 files changed, 178 insertions(+)
 create mode 100644 drivers/scsi/ufs/ufs-thermal.c
 create mode 100644 drivers/scsi/ufs/ufs-thermal.h

diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
index d14c224..bed56ee 100644
--- a/drivers/scsi/ufs/Kconfig
+++ b/drivers/scsi/ufs/Kconfig
@@ -160,3 +160,14 @@ config SCSI_UFS_BSG
 
 	  Select this if you need a bsg device node for your UFS controller.
 	  If unsure, say N.
+
+config THERMAL_UFS
+	bool "Thermal UFS"
+	depends on THERMAL && SCSI_UFSHCD
+	help
+	  A UFS3.0 feature that allows using the ufs device as a temperature
+	  sensor. it provide notification to the host when the UFS device
+	  case temperature approaches its pre-defined boundaries.
+
+	  Select Y to enable this feature, otherwise say N.
+	  If unsure, say N.
\ No newline at end of file
diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
index 94c6c5d..fd35941 100644
--- a/drivers/scsi/ufs/Makefile
+++ b/drivers/scsi/ufs/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
 obj-$(CONFIG_SCSI_UFS_HISI) += ufs-hisi.o
 obj-$(CONFIG_SCSI_UFS_MEDIATEK) += ufs-mediatek.o
 obj-$(CONFIG_SCSI_UFS_TI_J721E) += ti-j721e-ufs.o
+obj-$(CONFIG_THERMAL_UFS) += ufs-thermal.o
diff --git a/drivers/scsi/ufs/ufs-thermal.c b/drivers/scsi/ufs/ufs-thermal.c
new file mode 100644
index 0000000..469c1ed
--- /dev/null
+++ b/drivers/scsi/ufs/ufs-thermal.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * thermal ufs
+ *
+ * Copyright (C) 2020 Western Digital Corporation
+ */
+#include <linux/thermal.h>
+#include "ufs-thermal.h"
+
+enum {
+	UFS_THERM_MAX_TEMP,
+	UFS_THERM_HIGH_TEMP,
+	UFS_THERM_LOW_TEMP,
+	UFS_THERM_MIN_TEMP,
+
+	/* keep last */
+	UFS_THERM_MAX_TRIPS
+};
+
+/**
+ *struct ufs_thermal - thermal zone related data
+ * @tzone: thermal zone device data
+ */
+static struct ufs_thermal {
+	struct thermal_zone_device *zone;
+} thermal;
+
+static  struct thermal_zone_device_ops ufs_thermal_ops = {
+	.get_temp = NULL,
+	.get_trip_temp = NULL,
+	.get_trip_type = NULL,
+};
+
+static int ufs_thermal_enable_ee(struct ufs_hba *hba)
+{
+	/* later */
+	return -EINVAL;
+}
+
+static void ufs_thermal_zone_unregister(struct ufs_hba *hba)
+{
+	if (thermal.zone) {
+		dev_dbg(hba->dev, "Thermal zone device unregister\n");
+		thermal_zone_device_unregister(thermal.zone);
+		thermal.zone = NULL;
+	}
+}
+
+static int ufs_thermal_register(struct ufs_hba *hba)
+{
+	int err = 0;
+	char name[THERMAL_NAME_LENGTH] = {};
+
+	snprintf(name, THERMAL_NAME_LENGTH, "ufs_storage_%d",
+			hba->host->host_no);
+
+	thermal.zone = thermal_zone_device_register(name, UFS_THERM_MAX_TRIPS,
+			0, hba, &ufs_thermal_ops, NULL, 0, 0);
+	if (IS_ERR(thermal.zone)) {
+		err = PTR_ERR(thermal.zone);
+		dev_err(hba->dev, "Failed to register to thermal zone, err %d\n",
+				err);
+		thermal.zone = NULL;
+		goto out;
+	}
+
+	 /* thermal support is enabled only after successful
+	  * enablement of thermal exception
+	  */
+	if (ufs_thermal_enable_ee(hba)) {
+		dev_info(hba->dev, "Failed to enable thermal exception\n");
+		ufs_thermal_zone_unregister(hba);
+		err = -EINVAL;
+	}
+
+out:
+	return err;
+}
+
+int ufs_thermal_probe(struct ufs_hba *hba)
+{
+	u8 ufs_features;
+	u8 *desc_buf = NULL;
+	int err = -EINVAL;
+
+	if (!ufshcd_thermal_management_enabled(hba))
+		goto out;
+
+	desc_buf = kzalloc(hba->desc_size.dev_desc, GFP_KERNEL);
+	if (!desc_buf) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	if (ufshcd_read_desc_param(hba, QUERY_DESC_IDN_DEVICE, 0, 0, desc_buf,
+			hba->desc_size.dev_desc))
+		goto out;
+
+
+	ufs_features = desc_buf[DEVICE_DESC_PARAM_UFS_FEAT] &
+			(UFS_FEATURE_HTEMP | UFS_FEATURE_LTEMP);
+	if (!ufs_features)
+		goto out;
+
+	err = ufs_thermal_register(hba);
+	if (err)
+		goto out;
+
+	hba->thermal_features = ufs_features;
+
+out:
+	kfree(desc_buf);
+	return err;
+}
+
+void ufs_thermal_remove(struct ufs_hba *hba)
+{
+	if (!ufshcd_thermal_management_enabled(hba))
+		return;
+
+	 ufs_thermal_zone_unregister(hba);
+	 hba->thermal_features = 0;
+}
diff --git a/drivers/scsi/ufs/ufs-thermal.h b/drivers/scsi/ufs/ufs-thermal.h
new file mode 100644
index 0000000..7c0fcbe
--- /dev/null
+++ b/drivers/scsi/ufs/ufs-thermal.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018 Western Digital Corporation
+ */
+#ifndef UFS_THERMAL_H
+#define UFS_THERMAL_H
+
+#include "ufshcd.h"
+#include "ufs.h"
+
+#ifdef CONFIG_THERMAL_UFS
+void ufs_thermal_remove(struct ufs_hba *hba);
+int ufs_thermal_probe(struct ufs_hba *hba);
+#else
+static inline void ufs_thermal_remove(struct ufs_hba *hba) {}
+static inline int ufs_thermal_probe(struct ufs_hba *hba) {return 0; }
+#endif /* CONFIG_THERMAL_UFS */
+
+#endif /* UFS_THERMAL_H */
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index dde2eb0..eb729cc 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -332,6 +332,17 @@ enum {
 	UFSHCD_AMP		= 3,
 };
 
+/* UFS Features - to decode bUFSFeaturesSupport */
+enum {
+	UFS_FEATURE_FFU		= BIT(0),
+	UFS_FEATURE_PSA		= BIT(1),
+	UFS_FEATURE_LIFE		= BIT(2),
+	UFS_FEATURE_REFRESH		= BIT(3),
+	UFS_FEATURE_HTEMP		= BIT(4),
+	UFS_FEATURE_LTEMP		= BIT(5),
+	UFS_FEATURE_ETEMP		= BIT(6),
+};
+
 #define POWER_DESC_MAX_SIZE			0x62
 #define POWER_DESC_MAX_ACTV_ICC_LVLS		16
 
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index abd0e6b..099d2de 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -47,6 +47,7 @@
 #include "unipro.h"
 #include "ufs-sysfs.h"
 #include "ufs_bsg.h"
+#include "ufs-thermal.h"
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/ufs.h>
@@ -7111,6 +7112,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool async)
 
 	/* Enable Auto-Hibernate if configured */
 	ufshcd_auto_hibern8_enable(hba);
+	ufs_thermal_probe(hba);
 
 out:
 
@@ -8278,6 +8280,7 @@ int ufshcd_shutdown(struct ufs_hba *hba)
  */
 void ufshcd_remove(struct ufs_hba *hba)
 {
+	ufs_thermal_remove(hba);
 	ufs_bsg_remove(hba);
 	ufs_sysfs_remove_nodes(hba->dev);
 	blk_cleanup_queue(hba->tmf_queue);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 2ae6c7c..28c0063 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -730,6 +730,11 @@ struct ufs_hba {
 
 	struct device		bsg_dev;
 	struct request_queue	*bsg_queue;
+
+#define UFSHCD_CAP_THERMAL_MANAGEMENT (1 << 7)
+
+	u8 thermal_features;
+
 };
 
 /* Returns true if clocks can be gated. Otherwise false */
@@ -754,6 +759,11 @@ static inline bool ufshcd_is_rpm_autosuspend_allowed(struct ufs_hba *hba)
 	return hba->caps & UFSHCD_CAP_RPM_AUTOSUSPEND;
 }
 
+static inline bool ufshcd_thermal_management_enabled(struct ufs_hba *hba)
+{
+	return hba->caps & UFSHCD_CAP_THERMAL_MANAGEMENT;
+}
+
 static inline bool ufshcd_is_intr_aggr_allowed(struct ufs_hba *hba)
 {
 /* DWC UFS Core has the Interrupt aggregation feature but is not detectable*/
-- 
1.9.1

[RESEND PATCH 2/5] scsi: ufs: export ufshcd_enable_ee

[Avi Shchislowski]

From: Avi Shchislowski <avi.shchislowski@sandisk.com>

export ufshcd_enable_ee so that other modules can use it. this will
come handy in the next patch where we will need it to enable thermal
support

Signed-off-by: Avi Shchislowski <avi.shchislowski@sandisk.com>
---
 drivers/scsi/ufs/ufshcd.c | 2 +-
 drivers/scsi/ufs/ufshcd.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 099d2de..f25b93c 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -4923,7 +4923,7 @@ static int ufshcd_disable_ee(struct ufs_hba *hba, u16 mask)
  *
  * Returns zero on success, non-zero error value on failure.
  */
-static int ufshcd_enable_ee(struct ufs_hba *hba, u16 mask)
+int ufshcd_enable_ee(struct ufs_hba *hba, u16 mask)
 {
 	int err = 0;
 	u32 val;
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 28c0063..6d2a5fd 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -960,6 +960,8 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba *hba,
 			     u8 *desc_buff, int *buff_len,
 			     enum query_opcode desc_op);
 
+int ufshcd_enable_ee(struct ufs_hba *hba, u16 mask);
+
 /* Wrapper functions for safely calling variant operations */
 static inline const char *ufshcd_get_var_name(struct ufs_hba *hba)
 {
-- 
1.9.1

[RESEND PATCH 3/5] scsi: ufs: enable thermal exception event

[Avi Shchislowski]

From: Avi Shchislowski <avi.shchislowski@sandisk.com>

The host might need to be aware of the device's temperature, when it's
too high or too low. Should such event occur, the device is expected
to notify it to the host by using the exception event mechanism.

E.g. when TOO_HIGH_TEMP in wExceptionEventStatus is raised, it is
recommended to perform thermal throttling or other cooling activities
for lowering the device Tcase temperature. Similarly, when
TOO_LOW_TEMP is raised, it is recommended to take an applicable
actions to increase the device’s Tcase temperature.

Signed-off-by: Avi Shchislowski <avi.shchislowski@sandisk.com>
---
 drivers/scsi/ufs/ufs-thermal.c | 28 ++++++++++++++++++++++++----
 drivers/scsi/ufs/ufs-thermal.h |  6 ++++++
 drivers/scsi/ufs/ufs.h         |  6 +++++-
 drivers/scsi/ufs/ufshcd.c      |  4 ++++
 4 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-thermal.c b/drivers/scsi/ufs/ufs-thermal.c
index 469c1ed..dfa5d68 100644
--- a/drivers/scsi/ufs/ufs-thermal.c
+++ b/drivers/scsi/ufs/ufs-thermal.c
@@ -19,11 +19,32 @@ enum {
 
 /**
  *struct ufs_thermal - thermal zone related data
- * @tzone: thermal zone device data
+ * @trip: trip array
  */
 static struct ufs_thermal {
 	struct thermal_zone_device *zone;
-} thermal;
+	int trip[UFS_THERM_MAX_TRIPS];
+} thermal = {
+		.trip = {
+			[UFS_THERM_MAX_TEMP] = 170 * 1000,
+			[UFS_THERM_MIN_TEMP] = -79 * 1000
+		}
+};
+
+void ufs_thermal_exception_event_handler(struct ufs_hba *hba,
+		u32 exception_status)
+{
+	if (WARN_ON_ONCE(!hba->thermal_features))
+		return;
+
+	if (exception_status & MASK_EE_TOO_HIGH_TEMP) {
+		thermal_notify_framework(thermal.zone, UFS_THERM_HIGH_TEMP);
+		dev_info(hba->dev, "High temperature raised\n");
+	} else if (exception_status & MASK_EE_TOO_LOW_TEMP) {
+		thermal_notify_framework(thermal.zone, UFS_THERM_LOW_TEMP);
+		dev_info(hba->dev, "Low temperature raised\n");
+	}
+}
 
 static  struct thermal_zone_device_ops ufs_thermal_ops = {
 	.get_temp = NULL,
@@ -33,8 +54,7 @@ enum {
 
 static int ufs_thermal_enable_ee(struct ufs_hba *hba)
 {
-	/* later */
-	return -EINVAL;
+	return ufshcd_enable_ee(hba, MASK_EE_URGENT_TEMP);
 }
 
 static void ufs_thermal_zone_unregister(struct ufs_hba *hba)
diff --git a/drivers/scsi/ufs/ufs-thermal.h b/drivers/scsi/ufs/ufs-thermal.h
index 7c0fcbe..285049e 100644
--- a/drivers/scsi/ufs/ufs-thermal.h
+++ b/drivers/scsi/ufs/ufs-thermal.h
@@ -11,9 +11,15 @@
 #ifdef CONFIG_THERMAL_UFS
 void ufs_thermal_remove(struct ufs_hba *hba);
 int ufs_thermal_probe(struct ufs_hba *hba);
+void ufs_thermal_exception_event_handler(struct ufs_hba *hba,
+		u32 exception_status);
 #else
 static inline void ufs_thermal_remove(struct ufs_hba *hba) {}
 static inline int ufs_thermal_probe(struct ufs_hba *hba) {return 0; }
+void ufs_thermal_exception_event_handler(struct ufs_hba *hba,
+		u32 exception_status)
+{
+}
 #endif /* CONFIG_THERMAL_UFS */
 
 #endif /* UFS_THERMAL_H */
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index eb729cc..8fc0b0c 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -363,7 +363,9 @@ enum power_desc_param_offset {
 /* Exception event mask values */
 enum {
 	MASK_EE_STATUS		= 0xFFFF,
-	MASK_EE_URGENT_BKOPS	= (1 << 2),
+	MASK_EE_URGENT_BKOPS	= BIT(2),
+	MASK_EE_TOO_HIGH_TEMP	= BIT(3),
+	MASK_EE_TOO_LOW_TEMP	= BIT(4),
 };
 
 /* Background operation status */
@@ -375,6 +377,8 @@ enum bkops_status {
 	BKOPS_STATUS_MAX		 = BKOPS_STATUS_CRITICAL,
 };
 
+#define MASK_EE_URGENT_TEMP (MASK_EE_TOO_HIGH_TEMP | MASK_EE_TOO_LOW_TEMP)
+
 /* UTP QUERY Transaction Specific Fields OpCode */
 enum query_opcode {
 	UPIU_QUERY_OPCODE_NOP		= 0x0,
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index f25b93c..45fb52d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -42,6 +42,7 @@
 #include <linux/nls.h>
 #include <linux/of.h>
 #include <linux/bitfield.h>
+#include <linux/thermal.h>
 #include "ufshcd.h"
 #include "ufs_quirks.h"
 #include "unipro.h"
@@ -5183,6 +5184,9 @@ static void ufshcd_exception_event_handler(struct work_struct *work)
 	if (status & MASK_EE_URGENT_BKOPS)
 		ufshcd_bkops_exception_event_handler(hba);
 
+	if (status & MASK_EE_URGENT_TEMP)
+		ufs_thermal_exception_event_handler(hba, status);
+
 out:
 	ufshcd_scsi_unblock_requests(hba);
 	pm_runtime_put_sync(hba->dev);
-- 
1.9.1

[RESEND PATCH 4/5] scsi: ufs-thermal: implement thermal file ops

[Avi Shchislowski]

From: Avi Shchislowski <avi.shchislowski@sandisk.com>

The thermal interface adds a new thermal zone device sensor under
/sys/class/thermal/ folder.

Signed-off-by: Avi Shchislowski <avi.shchislowski@sandisk.com>
---
 drivers/scsi/ufs/ufs-thermal.c | 122 ++++++++++++++++++++++++++++++++++++++---
 drivers/scsi/ufs/ufs.h         |   3 +
 2 files changed, 117 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-thermal.c b/drivers/scsi/ufs/ufs-thermal.c
index dfa5d68..23e4ac1 100644
--- a/drivers/scsi/ufs/ufs-thermal.c
+++ b/drivers/scsi/ufs/ufs-thermal.c
@@ -31,6 +31,99 @@ enum {
 		}
 };
 
+#define attr2milicelcius(attr) (((0xFF & attr) - 80) * 1000)
+
+static int ufs_thermal_get_temp(struct thermal_zone_device *device,
+				  int *temperature)
+{
+	struct ufs_hba *hba = (struct ufs_hba *)device->devdata;
+	u32 temp;
+	int err;
+
+	err = ufshcd_query_attr(hba,
+			UPIU_QUERY_OPCODE_READ_ATTR,
+			QUERY_ATTR_IDN_ROUGH_TEMP,
+			0, 0, &temp);
+	if (err)
+		return -EINVAL;
+
+	*temperature = attr2milicelcius(temp);
+	return 0;
+}
+
+static int ufs_thermal_get_trip_temp(
+		struct thermal_zone_device *device,
+				 int trip, int *temp)
+{
+
+	if (trip < 0 || trip >= UFS_THERM_MAX_TRIPS)
+		return -EINVAL;
+
+	*temp = thermal.trip[trip];
+
+	return 0;
+}
+
+static int ufs_thermal_get_trip_type(
+		struct thermal_zone_device *device,
+		int trip, enum thermal_trip_type *type)
+{
+	if (trip < 0 || trip >= UFS_THERM_MAX_TRIPS)
+		return -EINVAL;
+
+	*type = THERMAL_TRIP_PASSIVE;
+
+	return 0;
+}
+
+static int ufs_thermal_get_boundary(struct ufs_hba *hba,
+					int trip, int *boundary)
+{
+	enum attr_idn idn;
+	int err = 0;
+	u32 val;
+
+	idn = trip == UFS_THERM_HIGH_TEMP ?
+			QUERY_ATTR_IDN_TOO_HIGH_TEMP :
+			QUERY_ATTR_IDN_TOO_LOW_TEMP;
+
+	err = ufshcd_query_attr(hba,
+			UPIU_QUERY_OPCODE_READ_ATTR,
+			idn, 0, 0, &val);
+	if (err) {
+		dev_err(hba->dev,
+		"Failed to get device too %s temperature boundary\n",
+		trip == UFS_THERM_HIGH_TEMP ? "high" : "low");
+		goto out;
+	}
+
+	if (val < 1 || val > 250) {
+		dev_err(hba->dev, "out of device temperature boundary\n");
+		err = -EINVAL;
+		goto out;
+	}
+
+	*boundary = attr2milicelcius(val);
+
+out:
+	return err;
+}
+
+static int ufs_thermal_set_trip(struct ufs_hba *hba, int trip)
+{
+	int temp;
+	int err = 0;
+
+	err = ufs_thermal_get_boundary(hba, trip, &temp);
+	if (err)
+		return err;
+
+	thermal.trip[trip] = temp;
+
+	return err;
+
+}
+
 void ufs_thermal_exception_event_handler(struct ufs_hba *hba,
 		u32 exception_status)
 {
@@ -46,17 +139,12 @@ void ufs_thermal_exception_event_handler(struct ufs_hba *hba,
 	}
 }
 
-static  struct thermal_zone_device_ops ufs_thermal_ops = {
-	.get_temp = NULL,
-	.get_trip_temp = NULL,
-	.get_trip_type = NULL,
-};
-
 static int ufs_thermal_enable_ee(struct ufs_hba *hba)
 {
 	return ufshcd_enable_ee(hba, MASK_EE_URGENT_TEMP);
 }
 
+
 static void ufs_thermal_zone_unregister(struct ufs_hba *hba)
 {
 	if (thermal.zone) {
@@ -66,7 +154,13 @@ static void ufs_thermal_zone_unregister(struct ufs_hba *hba)
 	}
 }
 
-static int ufs_thermal_register(struct ufs_hba *hba)
+static  struct thermal_zone_device_ops ufs_thermal_ops = {
+	.get_temp = ufs_thermal_get_temp,
+	.get_trip_temp = ufs_thermal_get_trip_temp,
+	.get_trip_type = ufs_thermal_get_trip_type,
+};
+
+static int ufs_thermal_register(struct ufs_hba *hba, u8 ufs_features)
 {
 	int err = 0;
 	char name[THERMAL_NAME_LENGTH] = {};
@@ -74,6 +168,18 @@ static int ufs_thermal_register(struct ufs_hba *hba)
 	snprintf(name, THERMAL_NAME_LENGTH, "ufs_storage_%d",
 			hba->host->host_no);
 
+	if (ufs_features & UFS_FEATURE_HTEMP) {
+		err = ufs_thermal_set_trip(hba, UFS_THERM_HIGH_TEMP);
+		if (err)
+			goto out;
+	}
+
+	if (ufs_features & UFS_FEATURE_LTEMP) {
+		err = ufs_thermal_set_trip(hba, UFS_THERM_LOW_TEMP);
+		if (err)
+			goto out;
+	}
+
 	thermal.zone = thermal_zone_device_register(name, UFS_THERM_MAX_TRIPS,
 			0, hba, &ufs_thermal_ops, NULL, 0, 0);
 	if (IS_ERR(thermal.zone)) {
@@ -122,7 +228,7 @@ int ufs_thermal_probe(struct ufs_hba *hba)
 	if (!ufs_features)
 		goto out;
 
-	err = ufs_thermal_register(hba);
+	err = ufs_thermal_register(hba, ufs_features);
 	if (err)
 		goto out;
 
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 8fc0b0c..9f8224b 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -167,6 +167,9 @@ enum attr_idn {
 	QUERY_ATTR_IDN_FFU_STATUS		= 0x14,
 	QUERY_ATTR_IDN_PSA_STATE		= 0x15,
 	QUERY_ATTR_IDN_PSA_DATA_SIZE		= 0x16,
+	QUERY_ATTR_IDN_ROUGH_TEMP		= 0x18,
+	QUERY_ATTR_IDN_TOO_HIGH_TEMP		= 0x19,
+	QUERY_ATTR_IDN_TOO_LOW_TEMP		= 0x1A,
 };
 
 /* Descriptor idn for Query requests */
-- 
1.9.1

[RESEND PATCH 5/5] scsi: ufs: temperature atrributes add to ufs_sysfs

[Avi Shchislowski]

From: Avi Shchislowski <avi.shchislowski@sandisk.com>

Signed-off-by: Avi Shchislowski <avi.shchislowski@sandisk.com>
---
 drivers/scsi/ufs/ufs-sysfs.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
index dbdf8b0..180f4db 100644
--- a/drivers/scsi/ufs/ufs-sysfs.c
+++ b/drivers/scsi/ufs/ufs-sysfs.c
@@ -667,6 +667,9 @@ static DEVICE_ATTR_RO(_name)
 UFS_ATTRIBUTE(ffu_status, _FFU_STATUS);
 UFS_ATTRIBUTE(psa_state, _PSA_STATE);
 UFS_ATTRIBUTE(psa_data_size, _PSA_DATA_SIZE);
+UFS_ATTRIBUTE(rough_temp, _ROUGH_TEMP);
+UFS_ATTRIBUTE(too_high_temp, _TOO_HIGH_TEMP);
+UFS_ATTRIBUTE(too_low_temp, _TOO_LOW_TEMP);
 
 static struct attribute *ufs_sysfs_attributes[] = {
 	&dev_attr_boot_lun_enabled.attr,
@@ -685,6 +688,9 @@ static DEVICE_ATTR_RO(_name)
 	&dev_attr_ffu_status.attr,
 	&dev_attr_psa_state.attr,
 	&dev_attr_psa_data_size.attr,
+	&dev_attr_rough_temp.attr,
+	&dev_attr_too_high_temp.attr,
+	&dev_attr_too_low_temp.attr,
 	NULL,
 };
 
-- 
1.9.1

Re: [RESEND PATCH 1/5] scsi: ufs: Add ufs thermal support

[Daniel Lezcano]

On 23/02/2020 10:35, Avi Shchislowski wrote:
> From: Avi Shchislowski <avi.shchislowski@sandisk.com>
> 
> Support the new temperature notification attributes introduced in
> UFSv3.0. Add exception event mask, and ufs features attributes.
> 
> Signed-off-by: Avi Shchislowski <avi.shchislowski@sandisk.com>
> ---
>  drivers/scsi/ufs/Kconfig       |  11 ++++
>  drivers/scsi/ufs/Makefile      |   1 +
>  drivers/scsi/ufs/ufs-thermal.c | 123 +++++++++++++++++++++++++++++++++++++++++

Why not put the driver under drivers/thermal/ ?

>  drivers/scsi/ufs/ufs-thermal.h |  19 +++++++
>  drivers/scsi/ufs/ufs.h         |  11 ++++
>  drivers/scsi/ufs/ufshcd.c      |   3 +
>  drivers/scsi/ufs/ufshcd.h      |  10 ++++
>  7 files changed, 178 insertions(+)
>  create mode 100644 drivers/scsi/ufs/ufs-thermal.c
>  create mode 100644 drivers/scsi/ufs/ufs-thermal.h
> 
> diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
> index d14c224..bed56ee 100644
> --- a/drivers/scsi/ufs/Kconfig
> +++ b/drivers/scsi/ufs/Kconfig
> @@ -160,3 +160,14 @@ config SCSI_UFS_BSG
>  
>  	  Select this if you need a bsg device node for your UFS controller.
>  	  If unsure, say N.
> +
> +config THERMAL_UFS
> +	bool "Thermal UFS"
> +	depends on THERMAL && SCSI_UFSHCD
> +	help
> +	  A UFS3.0 feature that allows using the ufs device as a temperature
> +	  sensor. it provide notification to the host when the UFS device
> +	  case temperature approaches its pre-defined boundaries.
> +
> +	  Select Y to enable this feature, otherwise say N.
> +	  If unsure, say N.
> \ No newline at end of file

  ^^^^

> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
> index 94c6c5d..fd35941 100644
> --- a/drivers/scsi/ufs/Makefile
> +++ b/drivers/scsi/ufs/Makefile
> @@ -12,3 +12,4 @@ obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
>  obj-$(CONFIG_SCSI_UFS_HISI) += ufs-hisi.o
>  obj-$(CONFIG_SCSI_UFS_MEDIATEK) += ufs-mediatek.o
>  obj-$(CONFIG_SCSI_UFS_TI_J721E) += ti-j721e-ufs.o
> +obj-$(CONFIG_THERMAL_UFS) += ufs-thermal.o
> diff --git a/drivers/scsi/ufs/ufs-thermal.c b/drivers/scsi/ufs/ufs-thermal.c
> new file mode 100644
> index 0000000..469c1ed
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-thermal.c
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * thermal ufs
> + *
> + * Copyright (C) 2020 Western Digital Corporation
> + */
> +#include <linux/thermal.h>
> +#include "ufs-thermal.h"
> +
> +enum {
> +	UFS_THERM_MAX_TEMP,
> +	UFS_THERM_HIGH_TEMP,
> +	UFS_THERM_LOW_TEMP,
> +	UFS_THERM_MIN_TEMP,
> +
> +	/* keep last */
> +	UFS_THERM_MAX_TRIPS
> +};
> +
> +/**
> + *struct ufs_thermal - thermal zone related data
> + * @tzone: thermal zone device data
> + */
> +static struct ufs_thermal {
> +	struct thermal_zone_device *zone;
> +} thermal;
> +
> +static  struct thermal_zone_device_ops ufs_thermal_ops = {
> +	.get_temp = NULL,
> +	.get_trip_temp = NULL,
> +	.get_trip_type = NULL,
> +};

Can you merge all the patches related to this driver into a single one?

> +static int ufs_thermal_enable_ee(struct ufs_hba *hba)
> +{
> +	/* later */
> +	return -EINVAL;
> +}
> +
> +static void ufs_thermal_zone_unregister(struct ufs_hba *hba)
> +{
> +	if (thermal.zone) {
> +		dev_dbg(hba->dev, "Thermal zone device unregister\n");
> +		thermal_zone_device_unregister(thermal.zone);
> +		thermal.zone = NULL;
> +	}
> +}
> +
> +static int ufs_thermal_register(struct ufs_hba *hba)
> +{
> +	int err = 0;
> +	char name[THERMAL_NAME_LENGTH] = {};
> +
> +	snprintf(name, THERMAL_NAME_LENGTH, "ufs_storage_%d",
> +			hba->host->host_no);
> +
> +	thermal.zone = thermal_zone_device_register(name, UFS_THERM_MAX_TRIPS,
> +			0, hba, &ufs_thermal_ops, NULL, 0, 0);
> +	if (IS_ERR(thermal.zone)) {
> +		err = PTR_ERR(thermal.zone);
> +		dev_err(hba->dev, "Failed to register to thermal zone, err %d\n",
> +				err);
> +		thermal.zone = NULL;
> +		goto out;
> +	}

It is pointless to reassign thermal.zone to NULL.

As there is no rollback involved here, This can be simplified to:

if (IS_ERR(thermal.zone)) {
	dev_err(hba->dev, "...");
	return PTR_ERR(thermal.zone);
}

> +
> +	 /* thermal support is enabled only after successful

nit: comment format

/*
 * thermal support ...
 * ...
 */

> +	  * enablement of thermal exception
> +	  */
> +	if (ufs_thermal_enable_ee(hba)) {
> +		dev_info(hba->dev, "Failed to enable thermal exception\n");
> +		ufs_thermal_zone_unregister(hba);
> +		err = -EINVAL;
> +	}

	err = ufs_thermal_enable_ee(hba);
	if (err) {
		...
		return err;
	}

	return 0;

> +
> +out:
> +	return err;
> +}
> +
> +int ufs_thermal_probe(struct ufs_hba *hba)
> +{
> +	u8 ufs_features;
> +	u8 *desc_buf = NULL;
> +	int err = -EINVAL;
> +
> +	if (!ufshcd_thermal_management_enabled(hba))
> +		goto out;
> +
> +	desc_buf = kzalloc(hba->desc_size.dev_desc, GFP_KERNEL);
> +	if (!desc_buf) {
> +		err = -ENOMEM;
> +		goto out;
> +	}
> +
> +	if (ufshcd_read_desc_param(hba, QUERY_DESC_IDN_DEVICE, 0, 0, desc_buf,
> +			hba->desc_size.dev_desc))
> +		goto out;
> +
> +
> +	ufs_features = desc_buf[DEVICE_DESC_PARAM_UFS_FEAT] &
> +			(UFS_FEATURE_HTEMP | UFS_FEATURE_LTEMP);
> +	if (!ufs_features)
> +		goto out;
> +
> +	err = ufs_thermal_register(hba);
> +	if (err)
> +		goto out;
> +
> +	hba->thermal_features = ufs_features;
> +
> +out:
> +	kfree(desc_buf);
> +	return err;
> +}
> +
> +void ufs_thermal_remove(struct ufs_hba *hba)
> +{
> +	if (!ufshcd_thermal_management_enabled(hba))
> +		return;
> +
> +	 ufs_thermal_zone_unregister(hba);
> +	 hba->thermal_features = 0;

Why is this needed ?

> +}
> diff --git a/drivers/scsi/ufs/ufs-thermal.h b/drivers/scsi/ufs/ufs-thermal.h
> new file mode 100644
> index 0000000..7c0fcbe
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-thermal.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2018 Western Digital Corporation
> + */
> +#ifndef UFS_THERMAL_H
> +#define UFS_THERMAL_H
> +
> +#include "ufshcd.h"
> +#include "ufs.h"
> +
> +#ifdef CONFIG_THERMAL_UFS
> +void ufs_thermal_remove(struct ufs_hba *hba);
> +int ufs_thermal_probe(struct ufs_hba *hba);
> +#else
> +static inline void ufs_thermal_remove(struct ufs_hba *hba) {}
> +static inline int ufs_thermal_probe(struct ufs_hba *hba) {return 0; }
> +#endif /* CONFIG_THERMAL_UFS */
> +
> +#endif /* UFS_THERMAL_H */
> diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> index dde2eb0..eb729cc 100644
> --- a/drivers/scsi/ufs/ufs.h
> +++ b/drivers/scsi/ufs/ufs.h
> @@ -332,6 +332,17 @@ enum {
>  	UFSHCD_AMP		= 3,
>  };
>  
> +/* UFS Features - to decode bUFSFeaturesSupport */
> +enum {
> +	UFS_FEATURE_FFU		= BIT(0),
> +	UFS_FEATURE_PSA		= BIT(1),
> +	UFS_FEATURE_LIFE		= BIT(2),
> +	UFS_FEATURE_REFRESH		= BIT(3),
> +	UFS_FEATURE_HTEMP		= BIT(4),
> +	UFS_FEATURE_LTEMP		= BIT(5),
> +	UFS_FEATURE_ETEMP		= BIT(6),
> +};
> +
>  #define POWER_DESC_MAX_SIZE			0x62
>  #define POWER_DESC_MAX_ACTV_ICC_LVLS		16
>  
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index abd0e6b..099d2de 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -47,6 +47,7 @@
>  #include "unipro.h"
>  #include "ufs-sysfs.h"
>  #include "ufs_bsg.h"
> +#include "ufs-thermal.h"
>  
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/ufs.h>
> @@ -7111,6 +7112,7 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool async)
>  
>  	/* Enable Auto-Hibernate if configured */
>  	ufshcd_auto_hibern8_enable(hba);
> +	ufs_thermal_probe(hba);
>  
>  out:
>  
> @@ -8278,6 +8280,7 @@ int ufshcd_shutdown(struct ufs_hba *hba)
>   */
>  void ufshcd_remove(struct ufs_hba *hba)
>  {
> +	ufs_thermal_remove(hba);
>  	ufs_bsg_remove(hba);
>  	ufs_sysfs_remove_nodes(hba->dev);
>  	blk_cleanup_queue(hba->tmf_queue);
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 2ae6c7c..28c0063 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -730,6 +730,11 @@ struct ufs_hba {
>  
>  	struct device		bsg_dev;
>  	struct request_queue	*bsg_queue;
> +
> +#define UFSHCD_CAP_THERMAL_MANAGEMENT (1 << 7)
> +
> +	u8 thermal_features;
> +
>  };
>  
>  /* Returns true if clocks can be gated. Otherwise false */
> @@ -754,6 +759,11 @@ static inline bool ufshcd_is_rpm_autosuspend_allowed(struct ufs_hba *hba)
>  	return hba->caps & UFSHCD_CAP_RPM_AUTOSUSPEND;
>  }
>  
> +static inline bool ufshcd_thermal_management_enabled(struct ufs_hba *hba)
> +{
> +	return hba->caps & UFSHCD_CAP_THERMAL_MANAGEMENT;
> +}
> +
>  static inline bool ufshcd_is_intr_aggr_allowed(struct ufs_hba *hba)
>  {
>  /* DWC UFS Core has the Interrupt aggregation feature but is not detectable*/
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

RE: [RESEND PATCH 1/5] scsi: ufs: Add ufs thermal support

[Avi Shchislowski]

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: Thursday, March 12, 2020 12:38 PM
> To: Avi Shchislowski <Avi.Shchislowski@wdc.com>; Avri Altman
> <Avri.Altman@wdc.com>; Guenter Roeck <linux@roeck-us.net>; Alim Akhtar
> <alim.akhtar@samsung.com>; James E.J. Bottomley <jejb@linux.ibm.com>;
> Martin K. Petersen <martin.petersen@oracle.com>; Zhang Rui
> <rui.zhang@intel.com>; linux-kernel@vger.kernel.org
> Cc: Avi Shchislowski <Avi.Shchislowski@wdc.com>
> Subject: Re: [RESEND PATCH 1/5] scsi: ufs: Add ufs thermal support
> 
> On 23/02/2020 10:35, Avi Shchislowski wrote:
> > From: Avi Shchislowski <avi.shchislowski@sandisk.com>
> >
> > Support the new temperature notification attributes introduced in
> > UFSv3.0. Add exception event mask, and ufs features attributes.
> >
> > Signed-off-by: Avi Shchislowski <avi.shchislowski@sandisk.com>
> > ---
> >  drivers/scsi/ufs/Kconfig       |  11 ++++
> >  drivers/scsi/ufs/Makefile      |   1 +
> >  drivers/scsi/ufs/ufs-thermal.c | 123
> > +++++++++++++++++++++++++++++++++++++++++
> 
> Why not put the driver under drivers/thermal/ ?
> 
This is not a platform driver, but as our 0 patch indicated, Adding a new feature added by the new ufs spec version (UFS3.0), that allows using the ufs device as a temperature sensor.
It require sending a device management commands which are privet to the ufs driver.
It also requires responding to a ufs exception events which are privet to the ufs driver.
Exposing those externally will potentially allow to brick the storage device

> >  drivers/scsi/ufs/ufs-thermal.h |  19 +++++++
> >  drivers/scsi/ufs/ufs.h         |  11 ++++
> >  drivers/scsi/ufs/ufshcd.c      |   3 +
> >  drivers/scsi/ufs/ufshcd.h      |  10 ++++
> >  7 files changed, 178 insertions(+)
> >  create mode 100644 drivers/scsi/ufs/ufs-thermal.c  create mode 100644
> > drivers/scsi/ufs/ufs-thermal.h
> >
> > diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig index
> > d14c224..bed56ee 100644
> > --- a/drivers/scsi/ufs/Kconfig
> > +++ b/drivers/scsi/ufs/Kconfig
> > @@ -160,3 +160,14 @@ config SCSI_UFS_BSG
> >
> >         Select this if you need a bsg device node for your UFS controller.
> >         If unsure, say N.
> > +
> > +config THERMAL_UFS
> > +     bool "Thermal UFS"
> > +     depends on THERMAL && SCSI_UFSHCD
> > +     help
> > +       A UFS3.0 feature that allows using the ufs device as a temperature
> > +       sensor. it provide notification to the host when the UFS device
> > +       case temperature approaches its pre-defined boundaries.
> > +
> > +       Select Y to enable this feature, otherwise say N.
> > +       If unsure, say N.
> > \ No newline at end of file
> 
>   ^^^^
> 
Done

> > diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
> > index 94c6c5d..fd35941 100644
> > --- a/drivers/scsi/ufs/Makefile
> > +++ b/drivers/scsi/ufs/Makefile
> > @@ -12,3 +12,4 @@ obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) +=
> > ufshcd-pltfrm.o
> >  obj-$(CONFIG_SCSI_UFS_HISI) += ufs-hisi.o
> >  obj-$(CONFIG_SCSI_UFS_MEDIATEK) += ufs-mediatek.o
> >  obj-$(CONFIG_SCSI_UFS_TI_J721E) += ti-j721e-ufs.o
> > +obj-$(CONFIG_THERMAL_UFS) += ufs-thermal.o
> > diff --git a/drivers/scsi/ufs/ufs-thermal.c
> > b/drivers/scsi/ufs/ufs-thermal.c new file mode 100644 index
> > 0000000..469c1ed
> > --- /dev/null
> > +++ b/drivers/scsi/ufs/ufs-thermal.c
> > @@ -0,0 +1,123 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * thermal ufs
> > + *
> > + * Copyright (C) 2020 Western Digital Corporation  */ #include
> > +<linux/thermal.h> #include "ufs-thermal.h"
> > +
> > +enum {
> > +     UFS_THERM_MAX_TEMP,
> > +     UFS_THERM_HIGH_TEMP,
> > +     UFS_THERM_LOW_TEMP,
> > +     UFS_THERM_MIN_TEMP,
> > +
> > +     /* keep last */
> > +     UFS_THERM_MAX_TRIPS
> > +};
> > +
> > +/**
> > + *struct ufs_thermal - thermal zone related data
> > + * @tzone: thermal zone device data
> > + */
> > +static struct ufs_thermal {
> > +     struct thermal_zone_device *zone; } thermal;
> > +
> > +static  struct thermal_zone_device_ops ufs_thermal_ops = {
> > +     .get_temp = NULL,
> > +     .get_trip_temp = NULL,
> > +     .get_trip_type = NULL,
> > +};
> 
> Can you merge all the patches related to this driver into a single one?
> 
Yes.  Will do.

> > +static int ufs_thermal_enable_ee(struct ufs_hba *hba) {
> > +     /* later */
> > +     return -EINVAL;
> > +}
> > +
> > +static void ufs_thermal_zone_unregister(struct ufs_hba *hba) {
> > +     if (thermal.zone) {
> > +             dev_dbg(hba->dev, "Thermal zone device unregister\n");
> > +             thermal_zone_device_unregister(thermal.zone);
> > +             thermal.zone = NULL;
> > +     }
> > +}
> > +
> > +static int ufs_thermal_register(struct ufs_hba *hba) {
> > +     int err = 0;
> > +     char name[THERMAL_NAME_LENGTH] = {};
> > +
> > +     snprintf(name, THERMAL_NAME_LENGTH, "ufs_storage_%d",
> > +                     hba->host->host_no);
> > +
> > +     thermal.zone = thermal_zone_device_register(name,
> UFS_THERM_MAX_TRIPS,
> > +                     0, hba, &ufs_thermal_ops, NULL, 0, 0);
> > +     if (IS_ERR(thermal.zone)) {
> > +             err = PTR_ERR(thermal.zone);
> > +             dev_err(hba->dev, "Failed to register to thermal zone, err %d\n",
> > +                             err);
> > +             thermal.zone = NULL;
> > +             goto out;
> > +     }
> 
> It is pointless to reassign thermal.zone to NULL.
> 
> As there is no rollback involved here, This can be simplified to:
> 
> if (IS_ERR(thermal.zone)) {
>         dev_err(hba->dev, "...");
>         return PTR_ERR(thermal.zone);
> }
> 
> > +
> > +      /* thermal support is enabled only after successful
> 
> nit: comment format
> 
> /*
>  * thermal support ...
>  * ...
>  */
> 
Ok. Done

> > +       * enablement of thermal exception
> > +       */
> > +     if (ufs_thermal_enable_ee(hba)) {
> > +             dev_info(hba->dev, "Failed to enable thermal exception\n");
> > +             ufs_thermal_zone_unregister(hba);
> > +             err = -EINVAL;
> > +     }
> 
>         err = ufs_thermal_enable_ee(hba);
>         if (err) {
>                 ...
>                 return err;
>         }
> 
>         return 0;
> 
Done
> > +
> > +out:
> > +     return err;
> > +}
> > +
> > +int ufs_thermal_probe(struct ufs_hba *hba) {
> > +     u8 ufs_features;
> > +     u8 *desc_buf = NULL;
> > +     int err = -EINVAL;
> > +
> > +     if (!ufshcd_thermal_management_enabled(hba))
> > +             goto out;
> > +
> > +     desc_buf = kzalloc(hba->desc_size.dev_desc, GFP_KERNEL);
> > +     if (!desc_buf) {
> > +             err = -ENOMEM;
> > +             goto out;
> > +     }
> > +
> > +     if (ufshcd_read_desc_param(hba, QUERY_DESC_IDN_DEVICE, 0, 0,
> desc_buf,
> > +                     hba->desc_size.dev_desc))
> > +             goto out;
> > +
> > +
> > +     ufs_features = desc_buf[DEVICE_DESC_PARAM_UFS_FEAT] &
> > +                     (UFS_FEATURE_HTEMP | UFS_FEATURE_LTEMP);
> > +     if (!ufs_features)
> > +             goto out;
> > +
> > +     err = ufs_thermal_register(hba);
> > +     if (err)
> > +             goto out;
> > +
> > +     hba->thermal_features = ufs_features;
> > +
> > +out:
> > +     kfree(desc_buf);
> > +     return err;
> > +}
> > +
> > +void ufs_thermal_remove(struct ufs_hba *hba) {
> > +     if (!ufshcd_thermal_management_enabled(hba))
> > +             return;
> > +
> > +      ufs_thermal_zone_unregister(hba);
> > +      hba->thermal_features = 0;
> 
> Why is this needed ?
> 
Not needed. Done.

> > +}
> > diff --git a/drivers/scsi/ufs/ufs-thermal.h
> > b/drivers/scsi/ufs/ufs-thermal.h new file mode 100644 index
> > 0000000..7c0fcbe
> > --- /dev/null
> > +++ b/drivers/scsi/ufs/ufs-thermal.h
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2018 Western Digital Corporation  */ #ifndef
> > +UFS_THERMAL_H #define UFS_THERMAL_H
> > +
> > +#include "ufshcd.h"
> > +#include "ufs.h"
> > +
> > +#ifdef CONFIG_THERMAL_UFS
> > +void ufs_thermal_remove(struct ufs_hba *hba); int
> > +ufs_thermal_probe(struct ufs_hba *hba); #else static inline void
> > +ufs_thermal_remove(struct ufs_hba *hba) {} static inline int
> > +ufs_thermal_probe(struct ufs_hba *hba) {return 0; } #endif /*
> > +CONFIG_THERMAL_UFS */
> > +
> > +#endif /* UFS_THERMAL_H */
> > diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h index
> > dde2eb0..eb729cc 100644
> > --- a/drivers/scsi/ufs/ufs.h
> > +++ b/drivers/scsi/ufs/ufs.h
> > @@ -332,6 +332,17 @@ enum {
> >       UFSHCD_AMP              = 3,
> >  };
> >
> > +/* UFS Features - to decode bUFSFeaturesSupport */ enum {
> > +     UFS_FEATURE_FFU         = BIT(0),
> > +     UFS_FEATURE_PSA         = BIT(1),
> > +     UFS_FEATURE_LIFE                = BIT(2),
> > +     UFS_FEATURE_REFRESH             = BIT(3),
> > +     UFS_FEATURE_HTEMP               = BIT(4),
> > +     UFS_FEATURE_LTEMP               = BIT(5),
> > +     UFS_FEATURE_ETEMP               = BIT(6),
> > +};
> > +
> >  #define POWER_DESC_MAX_SIZE                  0x62
> >  #define POWER_DESC_MAX_ACTV_ICC_LVLS         16
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index abd0e6b..099d2de 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -47,6 +47,7 @@
> >  #include "unipro.h"
> >  #include "ufs-sysfs.h"
> >  #include "ufs_bsg.h"
> > +#include "ufs-thermal.h"
> >
> >  #define CREATE_TRACE_POINTS
> >  #include <trace/events/ufs.h>
> > @@ -7111,6 +7112,7 @@ static int ufshcd_probe_hba(struct ufs_hba
> *hba,
> > bool async)
> >
> >       /* Enable Auto-Hibernate if configured */
> >       ufshcd_auto_hibern8_enable(hba);
> > +     ufs_thermal_probe(hba);
> >
> >  out:
> >
> > @@ -8278,6 +8280,7 @@ int ufshcd_shutdown(struct ufs_hba *hba)
> >   */
> >  void ufshcd_remove(struct ufs_hba *hba)  {
> > +     ufs_thermal_remove(hba);
> >       ufs_bsg_remove(hba);
> >       ufs_sysfs_remove_nodes(hba->dev);
> >       blk_cleanup_queue(hba->tmf_queue);
> > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> > index 2ae6c7c..28c0063 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -730,6 +730,11 @@ struct ufs_hba {
> >
> >       struct device           bsg_dev;
> >       struct request_queue    *bsg_queue;
> > +
> > +#define UFSHCD_CAP_THERMAL_MANAGEMENT (1 << 7)
> > +
> > +     u8 thermal_features;
> > +
> >  };
> >
> >  /* Returns true if clocks can be gated. Otherwise false */ @@ -754,6
> > +759,11 @@ static inline bool ufshcd_is_rpm_autosuspend_allowed(struct
> ufs_hba *hba)
> >       return hba->caps & UFSHCD_CAP_RPM_AUTOSUSPEND;  }
> >
> > +static inline bool ufshcd_thermal_management_enabled(struct ufs_hba
> > +*hba) {
> > +     return hba->caps & UFSHCD_CAP_THERMAL_MANAGEMENT; }
> > +
> >  static inline bool ufshcd_is_intr_aggr_allowed(struct ufs_hba *hba)
> > {
> >  /* DWC UFS Core has the Interrupt aggregation feature but is not
> > detectable*/
> >
> 
> 
> --
>  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
> 
> Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-
> blog/> Blog