[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20171220092626.GA14791@kroah.com>
Date: Wed, 20 Dec 2017 10:26:26 +0100
From: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
To: Jaegeuk Kim <jaegeuk@...nel.org>
Cc: Bart Van Assche <Bart.VanAssche@....com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
"jaegeuk@...gle.com" <jaegeuk@...gle.com>
Subject: Re: [PATCH 2/2 v2] scsi: ufs: use sysfs entry for health info
On Tue, Dec 19, 2017 at 02:46:44PM -0800, Jaegeuk Kim wrote:
> >From 3368207da5988b8fed4e41e6c0f49a60ac014222 Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim <jaegeuk@...gle.com>
> Date: Tue, 26 Sep 2017 20:53:48 -0700
> Subject: [PATCH 2/2] 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.
>
> 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 KH <gregkh@...uxfoundation.org>
> Signed-off-by: Jaegeuk Kim <jaegeuk@...gle.com>
> ---
> Documentation/ABI/testing/sysfs-devices-soc-ufs | 25 +++++++++
> MAINTAINERS | 1 +
> drivers/scsi/ufs/ufs.h | 2 +
> drivers/scsi/ufs/ufshcd.c | 69 ++++++++++++++++++++++++-
> drivers/scsi/ufs/ufshcd.h | 1 +
> 5 files changed, 97 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@...gle.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@...il.com>
> L: linux-scsi@...r.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 12ff7daebb00..5cbb08fff0f4 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;
> }
>
> /**
> @@ -7688,14 +7697,72 @@ static const struct attribute_group ufshcd_attr_group = {
> .attrs = ufshcd_attrs,
> };
>
> +struct health_attr {
> + struct attribute attr;
> + ssize_t (*show)(struct device *dev,
> + struct health_attr *attr, char *buf);
> + int byte_offset;
> +};
> +
> +static ssize_t health_attr_show(struct device *dev,
> + struct health_attr *attr, char *buf)
> +{
> + 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 (attr->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 scnprintf(buf, PAGE_SIZE, "0x%02x", desc_buf[attr->byte_offset]);
> +}
> +
> +#define HEALTH_ATTR_RO(_name, _byte_offset) \
> +static struct health_attr ufs_health_##_name = { \
> + .attr = {.name = __stringify(_name), .mode = 0444}, \
> + .show = health_attr_show, \
> + .byte_offset = _byte_offset, \
> +}
This is a nice "hack" to make things simpler, but I worry about not
using the "normal" __ATTR_* macros here. But I can't think of a simpler
way to do this now, except if you had a table of strings and offsets
that you then used to look up to get the offset based on the name of the
attribute.
Anyway, nice job, I'm just rambling:
Reviewed-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Powered by blists - more mailing lists