scsi: ufs: introduce static sysfs entries

PATCH In review Jaegeuk Kim 5 participants 12 messages Dec 20, 2017 – Dec 28, 2017 SCSI

lore

Share & tools

lore

Jaegeuk Kim

author

2017-12-20 19:16 UTC #0 lore raw

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

From: Jaegeuk Kim <[email protected]>

This patch introduces attribute group to show existing sysfs entries.

Cc Greg Kroah-Hartman <[email protected]>

Signed-off-by Jaegeuk Kim <[email protected]>

---

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(-)

drivers/scsi/ufs/ufshcd.c +17 ~~-29~~

index 011c3369082c..1e8a6a3e1f39 100644
@@ -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);
 }
 
 /**

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

index 1332e544da92..56e26eb15185 100644
@@ -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;

Signature · 1 line

2.15.0.531.g2ccb3012c9-goog

Jaegeuk Kim

author

2017-12-20 19:17 UTC #1 lore raw

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

From: Jaegeuk Kim <[email protected]>

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

Signed-off-by Jaegeuk Kim <[email protected]>

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

Documentation/ABI/testing/sysfs-devices-soc-ufs new file +25 -0

new file mode 100644
index 000000000000..313771a383e4
@@ -0,0 +1,25 @@
+What:		/sys/devices/soc/X.ufshc/health
+Date:		September 2017
+contact:	Jaegeuk Kim <[email protected]>
+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

MAINTAINERS +1 -0

index aa71ab52fd76..947034319bb4 100644
@@ -13999,6 +13999,7 @@ M:	Vinayak Holikatti <[email protected]>
 L:	[email protected]
 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

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

index 54deeb754db5..1af541d56c7d 100644
@@ -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*/

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

92 lines not on this page · raw message

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

index 56e26eb15185..ba44cbcf755e 100644
@@ -229,6 +229,7 @@ struct ufs_desc_size {
 	int interc_desc;
 	int unit_desc;
 	int conf_desc;
+	int health_desc;
 };
 
 /**

Signature · 1 line

2.15.0.531.g2ccb3012c9-goog

Jaegeuk Kim

author

2017-12-20 22:13 UTC #2 lore raw

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

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.

- length
: must be 25h

- type
: must be 09h

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

Cc Greg Kroah-Hartman <[email protected]>

Signed-off-by Jaegeuk Kim <[email protected]>

---

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

Documentation/ABI/testing/sysfs-devices-soc-ufs new file +25 -0

new file mode 100644
index 000000000000..313771a383e4
@@ -0,0 +1,25 @@
+What:		/sys/devices/soc/X.ufshc/health
+Date:		September 2017
+contact:	Jaegeuk Kim <[email protected]>
+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

MAINTAINERS +1 -0

index aa71ab52fd76..947034319bb4 100644
@@ -13999,6 +13999,7 @@ M:	Vinayak Holikatti <[email protected]>
 L:	[email protected]
 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

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

index 54deeb754db5..1af541d56c7d 100644
@@ -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*/

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

92 lines not on this page · raw message

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

index 56e26eb15185..ba44cbcf755e 100644
@@ -229,6 +229,7 @@ struct ufs_desc_size {
 	int interc_desc;
 	int unit_desc;
 	int conf_desc;
+	int health_desc;
 };
 
 /**

Signature · 1 line

2.15.0.531.g2ccb3012c9-goog

Greg Kroah-Hartman

Reviewed-by

2017-12-21 07:59 UTC #3 lore raw

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

Quoted 37 lines · 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

Quoted 3 lines

Cc Greg Kroah-Hartman <[email protected]>
Signed-off-by Jaegeuk Kim <[email protected]>
---

Reviewed-by Greg Kroah-Hartman <[email protected]>

Avri Altman

2017-12-27 09:00 UTC #4 lore raw

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

Quoted 12 lines

-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of Greg Kroah-Hartman
Sent: Thursday, December 21, 2017 10:00 AM
To: Jaegeuk Kim <[email protected]>
Cc: [email protected]; [email protected]; Jaegeuk Kim <[email protected]>
Subject: Re: [PATCH 2/2 v4] scsi: ufs: introduce sysfs entries exposing UFS health info

Quoted 4 lines

Quoted 3 lines · Jaegeuk Kim
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

Greg Kroah-Hartman

2017-12-27 13:10 UTC #5 lore raw

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

Quoted 12 lines · Avri Altman

Quoted 12 lines
-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of Greg Kroah-Hartman
Sent: Thursday, December 21, 2017 10:00 AM
To: Jaegeuk Kim <[email protected]>
Cc: [email protected]; [email protected]; Jaegeuk Kim <[email protected]>
Subject: Re: [PATCH 2/2 v4] scsi: ufs: introduce sysfs entries exposing UFS health info

Quoted 8 lines

Quoted 4 lines · Jaegeuk Kim
Quoted 3 lines
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 :)

Quoted 1 line

The attributes and flags should be placed in separate subfolders

Why? What is that going to help with?

Quoted 2 lines

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

Again, why?

Quoted 2 lines

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

Jaegeuk Kim

author

2017-12-28 20:10 UTC #6 lore raw

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

Quoted 39 lines · Greg Kroah-Hartman

Quoted 12 lines · Avri Altman
Quoted 12 lines
-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of Greg Kroah-Hartman
Sent: Thursday, December 21, 2017 10:00 AM
To: Jaegeuk Kim <[email protected]>
Cc: [email protected]; [email protected]; Jaegeuk Kim <[email protected]>
Subject: Re: [PATCH 2/2 v4] scsi: ufs: introduce sysfs entries exposing UFS health info
Quoted 8 lines
Quoted 4 lines · Jaegeuk Kim
Quoted 3 lines
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 :)
Quoted 1 line
The attributes and flags should be placed in separate subfolders
Why? What is that going to help with?
Quoted 2 lines
The LUN specific descriptors and attributes should be placed in a luns subfolder, and then per descriptor / attribute type
Again, why?
Quoted 2 lines
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?

Quoted 8 lines

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,

Quoted 4 lines

thanks,
greg k-h

Bart Van Assche

2017-12-20 19:20 UTC #7 lore raw

In reply to Jaegeuk Kim

Quoted 1 line · Jaegeuk Kim

From: Jaegeuk Kim <[email protected]>

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.

Jaegeuk Kim

author

2017-12-20 20:16 UTC #8 lore raw

Hi Bart,

Quoted 8 lines · Bart Van Assche

Quoted 1 line · Jaegeuk Kim
From: Jaegeuk Kim <[email protected]>
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,

Martin K. Petersen

2017-12-21 02:37 UTC #9 lore raw

Quoted 7 lines

Quoted 5 lines
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?

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering

[email protected]

2017-12-21 07:59 UTC #10 lore raw

Quoted 6 lines · Martin K. Petersen

Quoted 6 lines
Quoted 5 lines
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.

Quoted 7 lines

Quoted 1 line
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

Greg Kroah-Hartman

Reviewed-by on v3 1/2

2017-12-21 07:59 UTC #11 lore raw

In reply to Jaegeuk Kim

Quoted 10 lines · Jaegeuk Kim

From: Jaegeuk Kim <[email protected]>
This patch introduces attribute group to show existing sysfs entries.
Cc Greg Kroah-Hartman <[email protected]>
Signed-off-by Jaegeuk Kim <[email protected]>
---
Change log from v1:
- use ATTRIBUTE_GROUPS and sysfs_create_groups()

Nice change, thanks.

Reviewed-by Greg Kroah-Hartman <[email protected]>