Re: [PATCH v5 01/11] scsi: ufs: sysfs: attribute group for existing sysfs entries.

From: Jaegeuk Kim
Date: Sun Feb 11 2018 - 20:06:21 EST

Next message: Jaegeuk Kim: "Re: [PATCH v5 02/11] scsi: ufs: sysfs: device descriptor"
Previous message: Oleg Drokin: "Re: [lustre-devel] [PATCH 41/80] staging: lustre: lmv: separate master object with master stripe"
Next in thread: Philippe Ombredanne: "Re: [PATCH v5 01/11] scsi: ufs: sysfs: attribute group for existing sysfs entries."
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 02/06, Stanislav Nijnikov wrote:
> This patch introduces attribute group to show existing sysfs entries.
>
> Signed-off-by: Stanislav Nijnikov <stanislav.nijnikov@xxxxxxx>
> ---
> drivers/scsi/ufs/Makefile | 3 +-
> drivers/scsi/ufs/ufs-sysfs.c | 156 +++++++++++++++++++++++++++++++++++++++++++
> drivers/scsi/ufs/ufs-sysfs.h | 14 ++++
> drivers/scsi/ufs/ufshcd.c | 156 ++-----------------------------------------
> drivers/scsi/ufs/ufshcd.h | 2 +
> 5 files changed, 178 insertions(+), 153 deletions(-)
> create mode 100644 drivers/scsi/ufs/ufs-sysfs.c
> create mode 100644 drivers/scsi/ufs/ufs-sysfs.h
>
> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
> index 9310c6c..918f579 100644
> --- a/drivers/scsi/ufs/Makefile
> +++ b/drivers/scsi/ufs/Makefile
> @@ -3,6 +3,7 @@
> obj-$(CONFIG_SCSI_UFS_DWC_TC_PCI) += tc-dwc-g210-pci.o ufshcd-dwc.o tc-dwc-g210.o
> obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o tc-dwc-g210.o
> obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
> -obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
> +obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o
> +ufshcd-core-objs := ufshcd.o ufs-sysfs.o
> obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
> obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
> diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c
> new file mode 100644
> index 0000000..ce8dcb6
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-sysfs.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// Copyright (C) 2018 Western Digital Corporation
> +
> +#include <linux/err.h>
> +#include <linux/string.h>
> +
> +#include "ufs-sysfs.h"
> +
> +static const char *ufschd_uic_link_state_to_string(
> + enum uic_link_state state)
> +{
> + switch (state) {
> + case UIC_LINK_OFF_STATE: return "OFF";
> + case UIC_LINK_ACTIVE_STATE: return "ACTIVE";
> + case UIC_LINK_HIBERN8_STATE: return "HIBERN8";
> + default: return "UNKNOWN";
> + }
> +}
> +
> +static const char *ufschd_ufs_dev_pwr_mode_to_string(
> + enum ufs_dev_pwr_mode state)
> +{
> + switch (state) {
> + case UFS_ACTIVE_PWR_MODE: return "ACTIVE";
> + case UFS_SLEEP_PWR_MODE: return "SLEEP";
> + case UFS_POWERDOWN_PWR_MODE: return "POWERDOWN";
> + default: return "UNKNOWN";
> + }
> +}
> +
> +static inline ssize_t ufs_sysfs_pm_lvl_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count,
> + bool rpm)
> +{
> + struct ufs_hba *hba = dev_get_drvdata(dev);
> + unsigned long flags, value;
> +
> + if (kstrtoul(buf, 0, &value))
> + return -EINVAL;
> +
> + if (value >= UFS_PM_LVL_MAX)
> + return -EINVAL;
> +
> + spin_lock_irqsave(hba->host->host_lock, flags);
> + if (rpm)
> + hba->rpm_lvl = value;
> + else
> + hba->spm_lvl = value;
> + spin_unlock_irqrestore(hba->host->host_lock, flags);
> + return count;
> +}
> +
> +static ssize_t rpm_lvl_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ufs_hba *hba = dev_get_drvdata(dev);
> + int curr_len;
> + u8 lvl;
> +
> + curr_len = snprintf(buf, PAGE_SIZE,
> + "\nCurrent Runtime PM level [%d] => dev_state [%s] link_state [%s]\n",
> + hba->rpm_lvl,
> + ufschd_ufs_dev_pwr_mode_to_string(
> + ufs_pm_lvl_states[hba->rpm_lvl].dev_state),
> + ufschd_uic_link_state_to_string(
> + ufs_pm_lvl_states[hba->rpm_lvl].link_state));

If there is no objection regarding to backward compatibility, can we also
clean this up by adding multiple entries having single string each, as Greg
recommended?

For example,

/rpm_level
1

/rpm_dev_state
ACTIVE

/rpm_link_state
HIBERN8

/spm_level
2

/spm_dev_state
SLEEP

/spm_link_state
ACTIVE

/avail_dev_states
0:ACTIVE 1:ACTIVE 2:SLEEP 3:SLEEP 4:POWERDOWN 5:POWERDOWN

/avail_link_states
0:ACTIVE 1:HIBERN8 2:ACTIVE 3:HIBERN8 4:HIBERN8 5:OFF

Thanks,

> +
> + curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> + "\nAll available Runtime PM levels info:\n");
> + for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
> + curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> + "\tRuntime PM level [%d] => dev_state [%s] link_state [%s]\n",
> + lvl,
> + ufschd_ufs_dev_pwr_mode_to_string(
> + ufs_pm_lvl_states[lvl].dev_state),
> + ufschd_uic_link_state_to_string(
> + ufs_pm_lvl_states[lvl].link_state));
> +
> + return curr_len;
> +}
> +
> +static ssize_t rpm_lvl_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + return ufs_sysfs_pm_lvl_store(dev, attr, buf, count, true);
> +}
> +
> +static ssize_t spm_lvl_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ufs_hba *hba = dev_get_drvdata(dev);
> + int curr_len;
> + u8 lvl;
> +
> + curr_len = snprintf(buf, PAGE_SIZE,
> + "\nCurrent System PM level [%d] => dev_state [%s] link_state [%s]\n",
> + hba->spm_lvl,
> + ufschd_ufs_dev_pwr_mode_to_string(
> + ufs_pm_lvl_states[hba->spm_lvl].dev_state),
> + ufschd_uic_link_state_to_string(
> + ufs_pm_lvl_states[hba->spm_lvl].link_state));
> +
> + curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> + "\nAll available System PM levels info:\n");
> + for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
> + curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> + "\tSystem PM level [%d] => dev_state [%s] link_state [%s]\n",
> + lvl,
> + ufschd_ufs_dev_pwr_mode_to_string(
> + ufs_pm_lvl_states[lvl].dev_state),
> + ufschd_uic_link_state_to_string(
> + ufs_pm_lvl_states[lvl].link_state));
> +
> + return curr_len;
> +}
> +
> +static ssize_t spm_lvl_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t count)
> +{
> + return ufs_sysfs_pm_lvl_store(dev, attr, buf, count, false);
> +}
> +
> +static DEVICE_ATTR_RW(rpm_lvl);
> +static DEVICE_ATTR_RW(spm_lvl);
> +
> +static struct attribute *ufs_sysfs_ufshcd_attrs[] = {
> + &dev_attr_rpm_lvl.attr,
> + &dev_attr_spm_lvl.attr,
> + NULL
> +};
> +
> +static const struct attribute_group ufs_sysfs_default_group = {
> + .attrs = ufs_sysfs_ufshcd_attrs,
> +};
> +
> +static const struct attribute_group *ufs_sysfs_groups[] = {
> + &ufs_sysfs_default_group,
> + NULL,
> +};
> +
> +void ufs_sysfs_add_nodes(struct device *dev)
> +{
> + int ret;
> +
> + ret = sysfs_create_groups(&dev->kobj, ufs_sysfs_groups);
> + if (ret)
> + dev_err(dev,
> + "%s: sysfs groups creation failed (err = %d)\n",
> + __func__, ret);
> +}
> +
> +void ufs_sysfs_remove_nodes(struct device *dev)
> +{
> + sysfs_remove_groups(&dev->kobj, ufs_sysfs_groups);
> +}
> diff --git a/drivers/scsi/ufs/ufs-sysfs.h b/drivers/scsi/ufs/ufs-sysfs.h
> new file mode 100644
> index 0000000..4a984ca
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-sysfs.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0-only
> + * Copyright (C) 2018 Western Digital Corporation
> + */
> +
> +#ifndef __UFS_SYSFS_H__
> +#define __UFS_SYSFS_H__
> +
> +#include <linux/sysfs.h>
> +
> +#include "ufshcd.h"
> +
> +void ufs_sysfs_add_nodes(struct device *dev);
> +void ufs_sysfs_remove_nodes(struct device *dev);
> +#endif
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index a355d98..e7621a0a 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -44,6 +44,7 @@
> #include "ufshcd.h"
> #include "ufs_quirks.h"
> #include "unipro.h"
> +#include "ufs-sysfs.h"
>
> #define CREATE_TRACE_POINTS
> #include <trace/events/ufs.h>
> @@ -150,7 +151,7 @@ enum {
> #define ufshcd_is_ufs_dev_poweroff(h) \
> ((h)->curr_dev_pwr_mode == UFS_POWERDOWN_PWR_MODE)
>
> -static struct ufs_pm_lvl_states ufs_pm_lvl_states[] = {
> +struct ufs_pm_lvl_states ufs_pm_lvl_states[] = {
> {UFS_ACTIVE_PWR_MODE, UIC_LINK_ACTIVE_STATE},
> {UFS_ACTIVE_PWR_MODE, UIC_LINK_HIBERN8_STATE},
> {UFS_SLEEP_PWR_MODE, UIC_LINK_ACTIVE_STATE},
> @@ -813,28 +814,6 @@ static inline bool ufshcd_is_hba_active(struct ufs_hba *hba)
> ? false : true;
> }
>
> -static const char *ufschd_uic_link_state_to_string(
> - enum uic_link_state state)
> -{
> - switch (state) {
> - case UIC_LINK_OFF_STATE: return "OFF";
> - case UIC_LINK_ACTIVE_STATE: return "ACTIVE";
> - case UIC_LINK_HIBERN8_STATE: return "HIBERN8";
> - default: return "UNKNOWN";
> - }
> -}
> -
> -static const char *ufschd_ufs_dev_pwr_mode_to_string(
> - enum ufs_dev_pwr_mode state)
> -{
> - switch (state) {
> - case UFS_ACTIVE_PWR_MODE: return "ACTIVE";
> - case UFS_SLEEP_PWR_MODE: return "SLEEP";
> - case UFS_POWERDOWN_PWR_MODE: return "POWERDOWN";
> - default: return "UNKNOWN";
> - }
> -}
> -
> u32 ufshcd_get_local_unipro_ver(struct ufs_hba *hba)
> {
> /* HCI version 1.0 and 1.1 supports UniPro 1.41 */
> @@ -7585,133 +7564,6 @@ int ufshcd_runtime_idle(struct ufs_hba *hba)
> }
> EXPORT_SYMBOL(ufshcd_runtime_idle);
>
> -static inline ssize_t ufshcd_pm_lvl_store(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf, size_t count,
> - bool rpm)
> -{
> - struct ufs_hba *hba = dev_get_drvdata(dev);
> - unsigned long flags, value;
> -
> - if (kstrtoul(buf, 0, &value))
> - return -EINVAL;
> -
> - if (value >= UFS_PM_LVL_MAX)
> - return -EINVAL;
> -
> - spin_lock_irqsave(hba->host->host_lock, flags);
> - if (rpm)
> - hba->rpm_lvl = value;
> - else
> - hba->spm_lvl = value;
> - spin_unlock_irqrestore(hba->host->host_lock, flags);
> - return count;
> -}
> -
> -static ssize_t ufshcd_rpm_lvl_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct ufs_hba *hba = dev_get_drvdata(dev);
> - int curr_len;
> - u8 lvl;
> -
> - curr_len = snprintf(buf, PAGE_SIZE,
> - "\nCurrent Runtime PM level [%d] => dev_state [%s] link_state [%s]\n",
> - hba->rpm_lvl,
> - ufschd_ufs_dev_pwr_mode_to_string(
> - ufs_pm_lvl_states[hba->rpm_lvl].dev_state),
> - ufschd_uic_link_state_to_string(
> - ufs_pm_lvl_states[hba->rpm_lvl].link_state));
> -
> - curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> - "\nAll available Runtime PM levels info:\n");
> - for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
> - curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> - "\tRuntime PM level [%d] => dev_state [%s] link_state [%s]\n",
> - lvl,
> - ufschd_ufs_dev_pwr_mode_to_string(
> - ufs_pm_lvl_states[lvl].dev_state),
> - ufschd_uic_link_state_to_string(
> - ufs_pm_lvl_states[lvl].link_state));
> -
> - return curr_len;
> -}
> -
> -static ssize_t ufshcd_rpm_lvl_store(struct device *dev,
> - struct device_attribute *attr, const char *buf, size_t count)
> -{
> - return ufshcd_pm_lvl_store(dev, attr, buf, count, true);
> -}
> -
> -static void ufshcd_add_rpm_lvl_sysfs_nodes(struct ufs_hba *hba)
> -{
> - hba->rpm_lvl_attr.show = ufshcd_rpm_lvl_show;
> - hba->rpm_lvl_attr.store = ufshcd_rpm_lvl_store;
> - sysfs_attr_init(&hba->rpm_lvl_attr.attr);
> - hba->rpm_lvl_attr.attr.name = "rpm_lvl";
> - hba->rpm_lvl_attr.attr.mode = 0644;
> - if (device_create_file(hba->dev, &hba->rpm_lvl_attr))
> - dev_err(hba->dev, "Failed to create sysfs for rpm_lvl\n");
> -}
> -
> -static ssize_t ufshcd_spm_lvl_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct ufs_hba *hba = dev_get_drvdata(dev);
> - int curr_len;
> - u8 lvl;
> -
> - curr_len = snprintf(buf, PAGE_SIZE,
> - "\nCurrent System PM level [%d] => dev_state [%s] link_state [%s]\n",
> - hba->spm_lvl,
> - ufschd_ufs_dev_pwr_mode_to_string(
> - ufs_pm_lvl_states[hba->spm_lvl].dev_state),
> - ufschd_uic_link_state_to_string(
> - ufs_pm_lvl_states[hba->spm_lvl].link_state));
> -
> - curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> - "\nAll available System PM levels info:\n");
> - for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
> - curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> - "\tSystem PM level [%d] => dev_state [%s] link_state [%s]\n",
> - lvl,
> - ufschd_ufs_dev_pwr_mode_to_string(
> - ufs_pm_lvl_states[lvl].dev_state),
> - ufschd_uic_link_state_to_string(
> - ufs_pm_lvl_states[lvl].link_state));
> -
> - return curr_len;
> -}
> -
> -static ssize_t ufshcd_spm_lvl_store(struct device *dev,
> - struct device_attribute *attr, const char *buf, size_t count)
> -{
> - return ufshcd_pm_lvl_store(dev, attr, buf, count, false);
> -}
> -
> -static void ufshcd_add_spm_lvl_sysfs_nodes(struct ufs_hba *hba)
> -{
> - hba->spm_lvl_attr.show = ufshcd_spm_lvl_show;
> - hba->spm_lvl_attr.store = ufshcd_spm_lvl_store;
> - sysfs_attr_init(&hba->spm_lvl_attr.attr);
> - hba->spm_lvl_attr.attr.name = "spm_lvl";
> - hba->spm_lvl_attr.attr.mode = 0644;
> - if (device_create_file(hba->dev, &hba->spm_lvl_attr))
> - dev_err(hba->dev, "Failed to create sysfs for spm_lvl\n");
> -}
> -
> -static inline void ufshcd_add_sysfs_nodes(struct ufs_hba *hba)
> -{
> - ufshcd_add_rpm_lvl_sysfs_nodes(hba);
> - ufshcd_add_spm_lvl_sysfs_nodes(hba);
> -}
> -
> -static inline void ufshcd_remove_sysfs_nodes(struct ufs_hba *hba)
> -{
> - device_remove_file(hba->dev, &hba->rpm_lvl_attr);
> - device_remove_file(hba->dev, &hba->spm_lvl_attr);
> -}
> -
> /**
> * ufshcd_shutdown - shutdown routine
> * @hba: per adapter instance
> @@ -7749,7 +7601,7 @@ EXPORT_SYMBOL(ufshcd_shutdown);
> */
> void ufshcd_remove(struct ufs_hba *hba)
> {
> - ufshcd_remove_sysfs_nodes(hba);
> + ufs_sysfs_remove_nodes(hba->dev);
> scsi_remove_host(hba->host);
> /* disable interrupts */
> ufshcd_disable_intr(hba, hba->intr_mask);
> @@ -7996,7 +7848,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
> ufshcd_set_ufs_dev_active(hba);
>
> async_schedule(ufshcd_async_scan, hba);
> - ufshcd_add_sysfs_nodes(hba);
> + ufs_sysfs_add_nodes(hba->dev);
>
> return 0;
>
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 1332e54..53e2779 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -985,4 +985,6 @@ static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba)
> hba->vops->dbg_register_dump(hba);
> }
>
> +extern struct ufs_pm_lvl_states ufs_pm_lvl_states[];
> +
> #endif /* End of Header */
> --
> 2.7.4

Next message: Jaegeuk Kim: "Re: [PATCH v5 02/11] scsi: ufs: sysfs: device descriptor"
Previous message: Oleg Drokin: "Re: [lustre-devel] [PATCH 41/80] staging: lustre: lmv: separate master object with master stripe"
Next in thread: Philippe Ombredanne: "Re: [PATCH v5 01/11] scsi: ufs: sysfs: attribute group for existing sysfs entries."
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]