[PATCH 1/2 v3] scsi: ufs: introduce static sysfs entries

[PATCH 1/2 v3] scsi: ufs: introduce static sysfs entries

[Jaegeuk Kim]

From: Jaegeuk Kim <jaegeuk@google.com>

This patch introduces attribute group to show existing sysfs entries.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@google.com>
---

Change log from v1:
 - use ATTRIBUTE_GROUPS and sysfs_create_groups()

 drivers/scsi/ufs/ufshcd.c | 46 +++++++++++++++++-----------------------------
 drivers/scsi/ufs/ufshcd.h |  2 --
 2 files changed, 17 insertions(+), 31 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 011c3369082c..1e8a6a3e1f39 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -7605,7 +7605,7 @@ static inline ssize_t ufshcd_pm_lvl_store(struct device *dev,
 	return count;
 }
 
-static ssize_t ufshcd_rpm_lvl_show(struct device *dev,
+static ssize_t rpm_lvl_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
 	struct ufs_hba *hba = dev_get_drvdata(dev);
@@ -7634,24 +7634,13 @@ static ssize_t ufshcd_rpm_lvl_show(struct device *dev,
 	return curr_len;
 }
 
-static ssize_t ufshcd_rpm_lvl_store(struct device *dev,
+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 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,
+static ssize_t spm_lvl_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
 	struct ufs_hba *hba = dev_get_drvdata(dev);
@@ -7680,33 +7669,32 @@ static ssize_t ufshcd_spm_lvl_show(struct device *dev,
 	return curr_len;
 }
 
-static ssize_t ufshcd_spm_lvl_store(struct device *dev,
+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 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 DEVICE_ATTR_RW(rpm_lvl);
+static DEVICE_ATTR_RW(spm_lvl);
+
+static struct attribute *ufshcd_attrs[] = {
+	&dev_attr_rpm_lvl.attr,
+	&dev_attr_spm_lvl.attr,
+	NULL
+};
+
+ATTRIBUTE_GROUPS(ufshcd);
 
 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);
+	if (sysfs_create_groups(&hba->dev->kobj, ufshcd_groups))
+		dev_err(hba->dev, "Failed to create sysfs default groups\n");
 }
 
 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);
+	sysfs_remove_groups(&hba->dev->kobj, ufshcd_groups);
 }
 
 /**
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 1332e544da92..56e26eb15185 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -526,8 +526,6 @@ struct ufs_hba {
 	enum ufs_pm_level rpm_lvl;
 	/* Desired UFS power management level during system PM */
 	enum ufs_pm_level spm_lvl;
-	struct device_attribute rpm_lvl_attr;
-	struct device_attribute spm_lvl_attr;
 	int pm_op_in_progress;
 
 	struct ufshcd_lrb *lrb;
-- 
2.15.0.531.g2ccb3012c9-goog

[PATCH 2/2 v3] scsi: ufs: introduce sysfs entries exposing UFS health info

[Jaegeuk Kim]

From: Jaegeuk Kim <jaegeuk@google.com>

This patch adds a new sysfs group, namely health, via:

   /sys/devices/soc/X.ufshc/health/

This directory contains the below entries, each of which shows an 8-bytes
hex number representing different meanings defined by JEDEC specfication.

Users can simply read these entries to check how their underlying flash
storage is getting reached out to its end of life. For example, if
lifetimeA shows 0xb, it would be the right time to consider device swap.

 - length
   : must be 25h

 - type
   : must be 09h

 - eol
   00h: Not defined
   01h: Normal
   02h: Warning
   03h: Critical

 - lifetimeA/B
   00h: Not defined
   01h:  0% ~ 10% device life time used
   02h: 10% ~ 20% device life time used
   03h: 20% ~ 30% device life time used
   04h: 30% ~ 40% device life time used
   05h: 40% ~ 50% device life time used
   06h: 50% ~ 60% device life time used
   07h: 60% ~ 70% device life time used
   08h: 70% ~ 80% device life time used
   09h: 80% ~ 90% device life time used
   0Ah: 90% ~ 100% device life time used
   0Bh: Exceeded its maximum estimated device life time

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@google.com>
---
 Documentation/ABI/testing/sysfs-devices-soc-ufs | 25 +++++++++
 MAINTAINERS                                     |  1 +
 drivers/scsi/ufs/ufs.h                          |  2 +
 drivers/scsi/ufs/ufshcd.c                       | 68 ++++++++++++++++++++++++-
 drivers/scsi/ufs/ufshcd.h                       |  1 +
 5 files changed, 96 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-soc-ufs

diff --git a/Documentation/ABI/testing/sysfs-devices-soc-ufs b/Documentation/ABI/testing/sysfs-devices-soc-ufs
new file mode 100644
index 000000000000..313771a383e4
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-soc-ufs
@@ -0,0 +1,25 @@
+What:		/sys/devices/soc/X.ufshc/health
+Date:		September 2017
+contact:	Jaegeuk Kim <jaegeuk@google.com>
+Description:
+		This directory contains health information reported by UFS.
+		- length must be 25h
+		- type must be 09h
+		- eol represent
+		  00h: Not defined
+		  01h: Normal
+		  02h: Warning
+		  03h: Critical
+		- lifetimeA/B
+		  00h: Not defined
+		  01h:  0% ~ 10% device life time used
+		  02h: 10% ~ 20% device life time used
+		  03h: 20% ~ 30% device life time used
+		  04h: 30% ~ 40% device life time used
+		  05h: 40% ~ 50% device life time used
+		  06h: 50% ~ 60% device life time used
+		  07h: 60% ~ 70% device life time used
+		  08h: 70% ~ 80% device life time used
+		  09h: 80% ~ 90% device life time used
+		  0Ah: 90% ~ 100% device life time used
+		  0Bh: Exceeded its maximum estimated device life time
diff --git a/MAINTAINERS b/MAINTAINERS
index aa71ab52fd76..947034319bb4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13999,6 +13999,7 @@ M:	Vinayak Holikatti <vinholikatti@gmail.com>
 L:	linux-scsi@vger.kernel.org
 S:	Supported
 F:	Documentation/scsi/ufs.txt
+F:	Documentation/ABI/testing/sysfs-devices-soc-ufs
 F:	drivers/scsi/ufs/
 
 UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER DWC HOOKS
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 54deeb754db5..1af541d56c7d 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -154,6 +154,7 @@ enum desc_idn {
 	QUERY_DESC_IDN_RFU_1		= 0x6,
 	QUERY_DESC_IDN_GEOMETRY		= 0x7,
 	QUERY_DESC_IDN_POWER		= 0x8,
+	QUERY_DESC_IDN_HEALTH		= 0x9,
 	QUERY_DESC_IDN_MAX,
 };
 
@@ -169,6 +170,7 @@ enum ufs_desc_def_size {
 	QUERY_DESC_INTERCONNECT_DEF_SIZE	= 0x06,
 	QUERY_DESC_GEOMETRY_DEF_SIZE		= 0x44,
 	QUERY_DESC_POWER_DEF_SIZE		= 0x62,
+	QUERY_DESC_HEALTH_DEF_SIZE		= 0x25,
 };
 
 /* Unit descriptor parameters offsets in bytes*/
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 1e8a6a3e1f39..ffec7142d7d8 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2991,6 +2991,9 @@ int ufshcd_map_desc_id_to_length(struct ufs_hba *hba,
 	case QUERY_DESC_IDN_RFU_1:
 		*desc_len = 0;
 		break;
+	case QUERY_DESC_IDN_HEALTH:
+		*desc_len = hba->desc_size.health_desc;
+		break;
 	default:
 		*desc_len = 0;
 		return -EINVAL;
@@ -6298,6 +6301,11 @@ static void ufshcd_init_desc_sizes(struct ufs_hba *hba)
 		&hba->desc_size.geom_desc);
 	if (err)
 		hba->desc_size.geom_desc = QUERY_DESC_GEOMETRY_DEF_SIZE;
+
+	err = ufshcd_read_desc_length(hba, QUERY_DESC_IDN_HEALTH, 0,
+		&hba->desc_size.health_desc);
+	if (err)
+		hba->desc_size.health_desc = QUERY_DESC_HEALTH_DEF_SIZE;
 }
 
 static void ufshcd_def_desc_sizes(struct ufs_hba *hba)
@@ -6308,6 +6316,7 @@ static void ufshcd_def_desc_sizes(struct ufs_hba *hba)
 	hba->desc_size.conf_desc = QUERY_DESC_CONFIGURATION_DEF_SIZE;
 	hba->desc_size.unit_desc = QUERY_DESC_UNIT_DEF_SIZE;
 	hba->desc_size.geom_desc = QUERY_DESC_GEOMETRY_DEF_SIZE;
+	hba->desc_size.health_desc = QUERY_DESC_HEALTH_DEF_SIZE;
 }
 
 /**
@@ -7684,7 +7693,64 @@ static struct attribute *ufshcd_attrs[] = {
 	NULL
 };
 
-ATTRIBUTE_GROUPS(ufshcd);
+static const struct attribute_group ufshcd_default_group = {
+	.attrs = ufshcd_attrs,
+};
+
+static char ufshcd_read_desc_health(struct device *dev, int byte_offset)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+	int buff_len = hba->desc_size.health_desc;
+	u8 desc_buf[hba->desc_size.health_desc];
+	int err;
+
+	if (byte_offset >= buff_len)
+		return 0;
+
+	pm_runtime_get_sync(hba->dev);
+	err = ufshcd_read_desc(hba, QUERY_DESC_IDN_HEALTH, 0,
+					desc_buf, buff_len);
+	pm_runtime_put_sync(hba->dev);
+	if (err)
+		return 0;
+
+	return desc_buf[byte_offset];
+}
+
+#define HEALTH_ATTR_RO(_name, _byte_offset)				\
+static ssize_t _name##_show(struct device *_dev,			\
+			struct device_attribute *attr, char *buf)	\
+{									\
+	int ret = ufshcd_read_desc_health(_dev, _byte_offset);		\
+	return scnprintf(buf, PAGE_SIZE, "0x%02x", ret);		\
+}									\
+static DEVICE_ATTR_RO(_name);
+
+HEALTH_ATTR_RO(length, 0);
+HEALTH_ATTR_RO(type, 1);
+HEALTH_ATTR_RO(eol, 2);
+HEALTH_ATTR_RO(lifetimeA, 3);
+HEALTH_ATTR_RO(lifetimeB, 4);
+
+static struct attribute *ufshcd_health_attrs[] = {
+	&dev_attr_length.attr,
+	&dev_attr_type.attr,
+	&dev_attr_eol.attr,
+	&dev_attr_lifetimeA.attr,
+	&dev_attr_lifetimeB.attr,
+	NULL
+};
+
+static const struct attribute_group ufshcd_health_group = {
+	.name = "health",
+	.attrs = ufshcd_health_attrs,
+};
+
+static const struct attribute_group *ufshcd_groups[] = {
+	&ufshcd_default_group,
+	&ufshcd_health_group,
+	NULL,
+};
 
 static inline void ufshcd_add_sysfs_nodes(struct ufs_hba *hba)
 {
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 56e26eb15185..ba44cbcf755e 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -229,6 +229,7 @@ struct ufs_desc_size {
 	int interc_desc;
 	int unit_desc;
 	int conf_desc;
+	int health_desc;
 };
 
 /**
-- 
2.15.0.531.g2ccb3012c9-goog

Re: [PATCH 1/2 v3] scsi: ufs: introduce static sysfs entries

[Bart Van Assche]

On Wed, 2017-12-20 at 11:16 -0800, Jaegeuk Kim wrote:
> From: Jaegeuk Kim <jaegeuk@google.com>

Hello Jaegeuk,

For future patch series submissions, please include a cover letter that explains
the purpose of the patch series and please also document the changes between the
different versions of the patch series in the cover letter.

Thanks,

Bart.

Re: [PATCH 1/2 v3] scsi: ufs: introduce static sysfs entries

[Jaegeuk Kim]

Hi Bart,

On 12/20, Bart Van Assche wrote:
> On Wed, 2017-12-20 at 11:16 -0800, Jaegeuk Kim wrote:
> > From: Jaegeuk Kim <jaegeuk@google.com>
> 
> Hello Jaegeuk,
> 
> For future patch series submissions, please include a cover letter that explains
> the purpose of the patch series and please also document the changes between the
> different versions of the patch series in the cover letter.

Yup, I'll be aware of it. Thank you for the advice.

Thanks,

Re: [PATCH 2/2 v4] scsi: ufs: introduce sysfs entries exposing UFS health info

[Jaegeuk Kim]

This patch adds a new sysfs group, namely health, via:

   /sys/devices/soc/X.ufshc/health/

This directory contains the below entries, each of which shows an 8-bytes
hex number representing different meanings defined by JEDEC specfication.

Users can simply read these entries to check how their underlying flash
storage is getting reached out to its end of life. For example, if
lifetimeA shows 0xb, it would be the right time to consider device swap.

 - length
   : must be 25h

 - type
   : must be 09h

 - eol
   00h: Not defined
   01h: Normal
   02h: Warning
   03h: Critical

 - lifetimeA/B
   00h: Not defined
   01h:  0% ~ 10% device life time used
   02h: 10% ~ 20% device life time used
   03h: 20% ~ 30% device life time used
   04h: 30% ~ 40% device life time used
   05h: 40% ~ 50% device life time used
   06h: 50% ~ 60% device life time used
   07h: 60% ~ 70% device life time used
   08h: 70% ~ 80% device life time used
   09h: 80% ~ 90% device life time used
   0Ah: 90% ~ 100% device life time used
   0Bh: Exceeded its maximum estimated device life time

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Jaegeuk Kim <jaegeuk@google.com>
---

Change log from v3:
 - add '\n' at the end of print-out

 Documentation/ABI/testing/sysfs-devices-soc-ufs | 25 +++++++++
 MAINTAINERS                                     |  1 +
 drivers/scsi/ufs/ufs.h                          |  2 +
 drivers/scsi/ufs/ufshcd.c                       | 68 ++++++++++++++++++++++++-
 drivers/scsi/ufs/ufshcd.h                       |  1 +
 5 files changed, 96 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-devices-soc-ufs

diff --git a/Documentation/ABI/testing/sysfs-devices-soc-ufs b/Documentation/ABI/testing/sysfs-devices-soc-ufs
new file mode 100644
index 000000000000..313771a383e4
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-devices-soc-ufs
@@ -0,0 +1,25 @@
+What:		/sys/devices/soc/X.ufshc/health
+Date:		September 2017
+contact:	Jaegeuk Kim <jaegeuk@google.com>
+Description:
+		This directory contains health information reported by UFS.
+		- length must be 25h
+		- type must be 09h
+		- eol represent
+		  00h: Not defined
+		  01h: Normal
+		  02h: Warning
+		  03h: Critical
+		- lifetimeA/B
+		  00h: Not defined
+		  01h:  0% ~ 10% device life time used
+		  02h: 10% ~ 20% device life time used
+		  03h: 20% ~ 30% device life time used
+		  04h: 30% ~ 40% device life time used
+		  05h: 40% ~ 50% device life time used
+		  06h: 50% ~ 60% device life time used
+		  07h: 60% ~ 70% device life time used
+		  08h: 70% ~ 80% device life time used
+		  09h: 80% ~ 90% device life time used
+		  0Ah: 90% ~ 100% device life time used
+		  0Bh: Exceeded its maximum estimated device life time
diff --git a/MAINTAINERS b/MAINTAINERS
index aa71ab52fd76..947034319bb4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13999,6 +13999,7 @@ M:	Vinayak Holikatti <vinholikatti@gmail.com>
 L:	linux-scsi@vger.kernel.org
 S:	Supported
 F:	Documentation/scsi/ufs.txt
+F:	Documentation/ABI/testing/sysfs-devices-soc-ufs
 F:	drivers/scsi/ufs/
 
 UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER DWC HOOKS
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 54deeb754db5..1af541d56c7d 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -154,6 +154,7 @@ enum desc_idn {
 	QUERY_DESC_IDN_RFU_1		= 0x6,
 	QUERY_DESC_IDN_GEOMETRY		= 0x7,
 	QUERY_DESC_IDN_POWER		= 0x8,
+	QUERY_DESC_IDN_HEALTH		= 0x9,
 	QUERY_DESC_IDN_MAX,
 };
 
@@ -169,6 +170,7 @@ enum ufs_desc_def_size {
 	QUERY_DESC_INTERCONNECT_DEF_SIZE	= 0x06,
 	QUERY_DESC_GEOMETRY_DEF_SIZE		= 0x44,
 	QUERY_DESC_POWER_DEF_SIZE		= 0x62,
+	QUERY_DESC_HEALTH_DEF_SIZE		= 0x25,
 };
 
 /* Unit descriptor parameters offsets in bytes*/
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index fc93cd808a93..17b898ca820b 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -2991,6 +2991,9 @@ int ufshcd_map_desc_id_to_length(struct ufs_hba *hba,
 	case QUERY_DESC_IDN_RFU_1:
 		*desc_len = 0;
 		break;
+	case QUERY_DESC_IDN_HEALTH:
+		*desc_len = hba->desc_size.health_desc;
+		break;
 	default:
 		*desc_len = 0;
 		return -EINVAL;
@@ -6298,6 +6301,11 @@ static void ufshcd_init_desc_sizes(struct ufs_hba *hba)
 		&hba->desc_size.geom_desc);
 	if (err)
 		hba->desc_size.geom_desc = QUERY_DESC_GEOMETRY_DEF_SIZE;
+
+	err = ufshcd_read_desc_length(hba, QUERY_DESC_IDN_HEALTH, 0,
+		&hba->desc_size.health_desc);
+	if (err)
+		hba->desc_size.health_desc = QUERY_DESC_HEALTH_DEF_SIZE;
 }
 
 static void ufshcd_def_desc_sizes(struct ufs_hba *hba)
@@ -6308,6 +6316,7 @@ static void ufshcd_def_desc_sizes(struct ufs_hba *hba)
 	hba->desc_size.conf_desc = QUERY_DESC_CONFIGURATION_DEF_SIZE;
 	hba->desc_size.unit_desc = QUERY_DESC_UNIT_DEF_SIZE;
 	hba->desc_size.geom_desc = QUERY_DESC_GEOMETRY_DEF_SIZE;
+	hba->desc_size.health_desc = QUERY_DESC_HEALTH_DEF_SIZE;
 }
 
 /**
@@ -7684,7 +7693,64 @@ static struct attribute *ufshcd_attrs[] = {
 	NULL
 };
 
-ATTRIBUTE_GROUPS(ufshcd);
+static const struct attribute_group ufshcd_default_group = {
+	.attrs = ufshcd_attrs,
+};
+
+static u8 ufshcd_read_desc_health(struct device *dev, int byte_offset)
+{
+	struct ufs_hba *hba = dev_get_drvdata(dev);
+	int buff_len = hba->desc_size.health_desc;
+	u8 desc_buf[hba->desc_size.health_desc];
+	int err;
+
+	if (byte_offset >= buff_len)
+		return 0;
+
+	pm_runtime_get_sync(hba->dev);
+	err = ufshcd_read_desc(hba, QUERY_DESC_IDN_HEALTH, 0,
+					desc_buf, buff_len);
+	pm_runtime_put_sync(hba->dev);
+	if (err)
+		return 0;
+
+	return desc_buf[byte_offset];
+}
+
+#define HEALTH_ATTR_RO(_name, _byte_offset)				\
+static ssize_t _name##_show(struct device *_dev,			\
+			struct device_attribute *attr, char *buf)	\
+{									\
+	u8 byte = ufshcd_read_desc_health(_dev, _byte_offset);		\
+	return scnprintf(buf, PAGE_SIZE, "0x%02x\n", byte);		\
+}									\
+static DEVICE_ATTR_RO(_name);
+
+HEALTH_ATTR_RO(length, 0);
+HEALTH_ATTR_RO(type, 1);
+HEALTH_ATTR_RO(eol, 2);
+HEALTH_ATTR_RO(lifetimeA, 3);
+HEALTH_ATTR_RO(lifetimeB, 4);
+
+static struct attribute *ufshcd_health_attrs[] = {
+	&dev_attr_length.attr,
+	&dev_attr_type.attr,
+	&dev_attr_eol.attr,
+	&dev_attr_lifetimeA.attr,
+	&dev_attr_lifetimeB.attr,
+	NULL
+};
+
+static const struct attribute_group ufshcd_health_group = {
+	.name = "health",
+	.attrs = ufshcd_health_attrs,
+};
+
+static const struct attribute_group *ufshcd_groups[] = {
+	&ufshcd_default_group,
+	&ufshcd_health_group,
+	NULL,
+};
 
 static inline void ufshcd_add_sysfs_nodes(struct ufs_hba *hba)
 {
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 56e26eb15185..ba44cbcf755e 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -229,6 +229,7 @@ struct ufs_desc_size {
 	int interc_desc;
 	int unit_desc;
 	int conf_desc;
+	int health_desc;
 };
 
 /**
-- 
2.15.0.531.g2ccb3012c9-goog

Re: [PATCH 1/2 v3] scsi: ufs: introduce static sysfs entries

[Martin K. Petersen]

>> For future patch series submissions, please include a cover letter
>> that explains the purpose of the patch series and please also
>> document the changes between the different versions of the patch
>> series in the cover letter.
>
> Yup, I'll be aware of it. Thank you for the advice.

These look good to me.

Greg: Jaegeuk didn't carry over your review tags (despite the pretty
modest tweaks). Can I add your Reviewed-by: to these two when I queue
them up?

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 1/2 v3] scsi: ufs: introduce static sysfs entries

[Martin K. Petersen]

>> For future patch series submissions, please include a cover letter
>> that explains the purpose of the patch series and please also
>> document the changes between the different versions of the patch
>> series in the cover letter.
>
> Yup, I'll be aware of it. Thank you for the advice.

These look good to me.

Greg: Jaegeuk didn't carry over your review tags (despite the pretty
modest tweaks). Can I add your Reviewed-by: to these two when I queue
them up?

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 1/2 v3] scsi: ufs: introduce static sysfs entries

[gregkh]

On Wed, Dec 20, 2017 at 09:37:39PM -0500, Martin K. Petersen wrote:
> 
> >> For future patch series submissions, please include a cover letter
> >> that explains the purpose of the patch series and please also
> >> document the changes between the different versions of the patch
> >> series in the cover letter.
> >
> > Yup, I'll be aware of it. Thank you for the advice.
> 
> These look good to me.
> 
> Greg: Jaegeuk didn't carry over your review tags (despite the pretty
> modest tweaks). Can I add your Reviewed-by: to these two when I queue
> them up?

Yes, please do, they look fine to me.

greg k-h

Re: [PATCH 2/2 v4] scsi: ufs: introduce sysfs entries exposing UFS health info

[Greg Kroah-Hartman]

On Wed, Dec 20, 2017 at 02:13:25PM -0800, Jaegeuk Kim wrote:
> This patch adds a new sysfs group, namely health, via:
> 
>    /sys/devices/soc/X.ufshc/health/
> 
> This directory contains the below entries, each of which shows an 8-bytes
> hex number representing different meanings defined by JEDEC specfication.
> 
> Users can simply read these entries to check how their underlying flash
> storage is getting reached out to its end of life. For example, if
> lifetimeA shows 0xb, it would be the right time to consider device swap.
> 
>  - length
>    : must be 25h
> 
>  - type
>    : must be 09h
> 
>  - eol
>    00h: Not defined
>    01h: Normal
>    02h: Warning
>    03h: Critical
> 
>  - lifetimeA/B
>    00h: Not defined
>    01h:  0% ~ 10% device life time used
>    02h: 10% ~ 20% device life time used
>    03h: 20% ~ 30% device life time used
>    04h: 30% ~ 40% device life time used
>    05h: 40% ~ 50% device life time used
>    06h: 50% ~ 60% device life time used
>    07h: 60% ~ 70% device life time used
>    08h: 70% ~ 80% device life time used
>    09h: 80% ~ 90% device life time used
>    0Ah: 90% ~ 100% device life time used
>    0Bh: Exceeded its maximum estimated device life time
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Jaegeuk Kim <jaegeuk@google.com>
> ---

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 1/2 v3] scsi: ufs: introduce static sysfs entries

[Greg Kroah-Hartman]

On Wed, Dec 20, 2017 at 11:16:30AM -0800, Jaegeuk Kim wrote:
> From: Jaegeuk Kim <jaegeuk@google.com>
> 
> This patch introduces attribute group to show existing sysfs entries.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Jaegeuk Kim <jaegeuk@google.com>
> ---
> 
> Change log from v1:
>  - use ATTRIBUTE_GROUPS and sysfs_create_groups()

Nice change, thanks.

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

RE: [PATCH 2/2 v4] scsi: ufs: introduce sysfs entries exposing UFS health info

[Avri Altman]

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Greg Kroah-Hartman
> Sent: Thursday, December 21, 2017 10:00 AM
> To: Jaegeuk Kim <jaegeuk@kernel.org>
> Cc: linux-kernel@vger.kernel.org; linux-scsi@vger.kernel.org; Jaegeuk Kim
> <jaegeuk@google.com>
> Subject: Re: [PATCH 2/2 v4] scsi: ufs: introduce sysfs entries exposing UFS
> health info
> 
> On Wed, Dec 20, 2017 at 02:13:25PM -0800, Jaegeuk Kim wrote:
> > This patch adds a new sysfs group, namely health, via:
> >
> >    /sys/devices/soc/X.ufshc/health/
As device health is just one piece of information out of the device management,
I think that you should address this in a more comprehensive way,
And set hooks for much more device info:
Allow access to device descriptors, attributes and flags.
The attributes and flags should be placed in separate subfolders
The LUN specific descriptors and attributes should be placed in a luns subfolder, and then per descriptor / attribute type
You might also would like to consider differentiating read and write - to control those type of accesses as well.

Cheers,
Avri

Re: [PATCH 2/2 v4] scsi: ufs: introduce sysfs entries exposing UFS health info

[Greg Kroah-Hartman]

On Wed, Dec 27, 2017 at 09:00:10AM +0000, Avri Altman wrote:
> 
> 
> > -----Original Message-----
> > From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> > owner@vger.kernel.org] On Behalf Of Greg Kroah-Hartman
> > Sent: Thursday, December 21, 2017 10:00 AM
> > To: Jaegeuk Kim <jaegeuk@kernel.org>
> > Cc: linux-kernel@vger.kernel.org; linux-scsi@vger.kernel.org; Jaegeuk Kim
> > <jaegeuk@google.com>
> > Subject: Re: [PATCH 2/2 v4] scsi: ufs: introduce sysfs entries exposing UFS
> > health info
> > 
> > On Wed, Dec 20, 2017 at 02:13:25PM -0800, Jaegeuk Kim wrote:
> > > This patch adds a new sysfs group, namely health, via:
> > >
> > >    /sys/devices/soc/X.ufshc/health/
> As device health is just one piece of information out of the device management,
> I think that you should address this in a more comprehensive way,
> And set hooks for much more device info:
> Allow access to device descriptors, attributes and flags.

Add on patches are easy to create for this if people really want and
need it :)

> The attributes and flags should be placed in separate subfolders

Why?  What is that going to help with?

> The LUN specific descriptors and attributes should be placed in a luns
> subfolder, and then per descriptor / attribute type

Again, why?

> You might also would like to consider differentiating read and write -
> to control those type of accesses as well.

What do you mean by this exactly?

As it is, this is a step forward in getting attributes that people are
asking for and already using, into the kernel tree.  Please don't object
because not all attributes that are possible are being added here, it
should be trivial to add more as needed, right?

I'm really tired of seeing all of the various out-of-tree forks of this
driver, it's about time that someone works to get those features merged,
right?

thanks,

greg k-h

Re: [PATCH 2/2 v4] scsi: ufs: introduce sysfs entries exposing UFS health info

[Jaegeuk Kim]

On 12/27, Greg Kroah-Hartman wrote:
> On Wed, Dec 27, 2017 at 09:00:10AM +0000, Avri Altman wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> > > owner@vger.kernel.org] On Behalf Of Greg Kroah-Hartman
> > > Sent: Thursday, December 21, 2017 10:00 AM
> > > To: Jaegeuk Kim <jaegeuk@kernel.org>
> > > Cc: linux-kernel@vger.kernel.org; linux-scsi@vger.kernel.org; Jaegeuk Kim
> > > <jaegeuk@google.com>
> > > Subject: Re: [PATCH 2/2 v4] scsi: ufs: introduce sysfs entries exposing UFS
> > > health info
> > > 
> > > On Wed, Dec 20, 2017 at 02:13:25PM -0800, Jaegeuk Kim wrote:
> > > > This patch adds a new sysfs group, namely health, via:
> > > >
> > > >    /sys/devices/soc/X.ufshc/health/
> > As device health is just one piece of information out of the device management,
> > I think that you should address this in a more comprehensive way,
> > And set hooks for much more device info:
> > Allow access to device descriptors, attributes and flags.
> 
> Add on patches are easy to create for this if people really want and
> need it :)
> 
> > The attributes and flags should be placed in separate subfolders
> 
> Why?  What is that going to help with?
> 
> > The LUN specific descriptors and attributes should be placed in a luns
> > subfolder, and then per descriptor / attribute type
> 
> Again, why?
> 
> > You might also would like to consider differentiating read and write -
> > to control those type of accesses as well.
> 
> What do you mean by this exactly?
> 
> As it is, this is a step forward in getting attributes that people are
> asking for and already using, into the kernel tree.  Please don't object
> because not all attributes that are possible are being added here, it
> should be trivial to add more as needed, right?
> 
> I'm really tired of seeing all of the various out-of-tree forks of this
> driver, it's about time that someone works to get those features merged,
> right?

Indeed, it seems someone has already done similar work before. Let me drop
my patches and dive into that, although I have to clean up all the mess once
again. :(

Thanks,

> 
> thanks,
> 
> greg k-h