LinuxLists.cc - [PATCH 0/6] Deduplicate string exposure in sysfs

2024-04-20 20:01:57

Subject: [PATCH 0/6] Deduplicate string exposure in sysfs

Introduce a generic ->show() callback to expose a string as a device
attribute in sysfs. Deduplicate various identical callbacks across
the tree.

Result: Minus 216 LoC, minus 1576 bytes vmlinux size (x86_64 allyesconfig).

This is a byproduct of my upcoming PCI device authentication v2 patches.

Lukas Wunner (6):
driver core: Add device_show_string() helper for sysfs attributes
hwmon: Use device_show_string() helper for sysfs attributes
IB/qib: Use device_show_string() helper for sysfs attributes
perf: Use device_show_string() helper for sysfs attributes
platform/x86: Use device_show_string() helper for sysfs attributes
scsi: Use device_show_string() helper for sysfs attributes

arch/powerpc/perf/hv-24x7.c | 10 ----
arch/x86/events/intel/core.c | 13 ++---
drivers/base/core.c | 9 ++++
drivers/hwmon/i5k_amb.c | 15 ++----
drivers/hwmon/ibmpex.c | 14 ++----
drivers/infiniband/hw/qib/qib.h | 1 -
drivers/infiniband/hw/qib/qib_driver.c | 6 ---
drivers/infiniband/hw/qib/qib_sysfs.c | 10 +---
drivers/perf/alibaba_uncore_drw_pmu.c | 12 +----
drivers/perf/arm-cci.c | 12 +----
drivers/perf/arm-ccn.c | 11 +----
drivers/perf/arm_cspmu/arm_cspmu.c | 10 ----
drivers/perf/arm_cspmu/arm_cspmu.h | 7 +--
drivers/perf/arm_dsu_pmu.c | 11 +----
drivers/perf/cxl_pmu.c | 13 +----
drivers/perf/hisilicon/hisi_pcie_pmu.c | 13 +----
drivers/perf/hisilicon/hisi_uncore_pmu.c | 14 ------
drivers/perf/hisilicon/hisi_uncore_pmu.h | 4 +-
drivers/perf/hisilicon/hns3_pmu.c | 12 +----
drivers/perf/qcom_l3_pmu.c | 11 +----
drivers/perf/xgene_pmu.c | 11 +----
drivers/platform/x86/asus-wmi.c | 62 ++++++------------------
drivers/platform/x86/thinkpad_acpi.c | 10 +---
drivers/platform/x86/toshiba_acpi.c | 9 +---
drivers/scsi/bfa/bfad_attr.c | 28 +++--------
drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 11 +----
drivers/scsi/mvsas/mv_init.c | 10 +---
drivers/scsi/qla2xxx/qla_attr.c | 11 +----
drivers/scsi/smartpqi/smartpqi_init.c | 11 ++---
include/linux/device.h | 15 ++++++
30 files changed, 85 insertions(+), 301 deletions(-)

--
2.43.0

2024-04-20 20:03:02

by Lukas Wunner

[permalink] [raw]

Subject: [PATCH 1/6] driver core: Add device_show_string() helper for sysfs attributes

For drivers wishing to expose an unsigned long, int or bool at a static
memory location in sysfs, the driver core provides ready-made helpers
such as device_show_ulong() to be used as ->show() callback.

Some drivers need to expose a string and so far they all provide their
own ->show() implementation. arch/powerpc/perf/hv-24x7.c went so far
as to create a device_show_string() helper but kept it private.

Make it public for reuse by other drivers. The pattern seems to be
sufficiently frequent to merit a public helper.

Add a DEVICE_STRING_ATTR_RO() macro in line with the existing
DEVICE_ULONG_ATTR() and similar macros to ease declaration of string
attributes.

Signed-off-by: Lukas Wunner <[email protected]>
---
arch/powerpc/perf/hv-24x7.c | 10 ----------
drivers/base/core.c | 9 +++++++++
include/linux/device.h | 15 +++++++++++++++
3 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 057ec2e3451d..d400fa391c27 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -425,16 +425,6 @@ static char *memdup_to_str(char *maybe_str, int max_len, gfp_t gfp)
return kasprintf(gfp, "%.*s", max_len, maybe_str);
}

-static ssize_t device_show_string(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct dev_ext_attribute *d;
-
- d = container_of(attr, struct dev_ext_attribute, attr);
-
- return sprintf(buf, "%s\n", (char *)d->var);
-}
-
static ssize_t cpumask_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 78dfa74ee18b..190d4a39c6a8 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2523,6 +2523,15 @@ ssize_t device_show_bool(struct device *dev, struct device_attribute *attr,
}
EXPORT_SYMBOL_GPL(device_show_bool);

+ssize_t device_show_string(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct dev_ext_attribute *ea = to_ext_attr(attr);
+
+ return sysfs_emit(buf, "%s\n", (char *)ea->var);
+}
+EXPORT_SYMBOL_GPL(device_show_string);
+
/**
* device_release - free device structure.
* @kobj: device's kobject.
diff --git a/include/linux/device.h b/include/linux/device.h
index c515ba5756e4..63ac65db3ecb 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -132,6 +132,8 @@ ssize_t device_show_bool(struct device *dev, struct device_attribute *attr,
char *buf);
ssize_t device_store_bool(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count);
+ssize_t device_show_string(struct device *dev, struct device_attribute *attr,
+ char *buf);

/**
* DEVICE_ATTR - Define a device attribute.
@@ -251,6 +253,19 @@ ssize_t device_store_bool(struct device *dev, struct device_attribute *attr,
struct dev_ext_attribute dev_attr_##_name = \
{ __ATTR(_name, _mode, device_show_bool, device_store_bool), &(_var) }

+/**
+ * DEVICE_STRING_ATTR_RO - Define a device attribute backed by a r/o string.
+ * @_name: Attribute name.
+ * @_mode: File mode.
+ * @_var: Identifier of string.
+ *
+ * Like DEVICE_ULONG_ATTR(), but @_var is a string. Because the length of the
+ * string allocation is unknown, the attribute must be read-only.
+ */
+#define DEVICE_STRING_ATTR_RO(_name, _mode, _var) \
+ struct dev_ext_attribute dev_attr_##_name = \
+ { __ATTR(_name, (_mode) & ~0222, device_show_string, NULL), (_var) }
+
#define DEVICE_ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store) \
struct device_attribute dev_attr_##_name = \
__ATTR_IGNORE_LOCKDEP(_name, _mode, _show, _store)
--
2.43.0

2024-04-20 20:04:28

by Lukas Wunner

[permalink] [raw]

Subject: [PATCH 2/6] hwmon: Use device_show_string() helper for sysfs attributes

2024-04-20 20:05:16

by Lukas Wunner

[permalink] [raw]

Subject: [PATCH 3/6] IB/qib: Use device_show_string() helper for sysfs attributes

2024-04-20 20:06:26

by Lukas Wunner

[permalink] [raw]

Subject: [PATCH 4/6] perf: Use device_show_string() helper for sysfs attributes

Deduplicate sysfs ->show() callbacks which expose a string at a static
memory location. Use the newly introduced device_show_string() helper
in the driver core instead by declaring those sysfs attributes with
DEVICE_STRING_ATTR_RO().

No functional change intended.

Signed-off-by: Lukas Wunner <[email protected]>
---
arch/x86/events/intel/core.c | 13 +++----------
drivers/perf/alibaba_uncore_drw_pmu.c | 12 ++----------
drivers/perf/arm-cci.c | 12 +-----------
drivers/perf/arm-ccn.c | 11 +----------
drivers/perf/arm_cspmu/arm_cspmu.c | 10 ----------
drivers/perf/arm_cspmu/arm_cspmu.h | 7 +------
drivers/perf/arm_dsu_pmu.c | 11 +----------
drivers/perf/cxl_pmu.c | 13 +------------
drivers/perf/hisilicon/hisi_pcie_pmu.c | 13 +------------
drivers/perf/hisilicon/hisi_uncore_pmu.c | 14 --------------
drivers/perf/hisilicon/hisi_uncore_pmu.h | 4 +---
drivers/perf/hisilicon/hns3_pmu.c | 12 +-----------
drivers/perf/qcom_l3_pmu.c | 11 +----------
drivers/perf/xgene_pmu.c | 11 +----------
14 files changed, 15 insertions(+), 139 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 768d1414897f..38c1b1f1deaa 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -5645,18 +5645,11 @@ lbr_is_visible(struct kobject *kobj, struct attribute *attr, int i)

static char pmu_name_str[30];

-static ssize_t pmu_name_show(struct device *cdev,
- struct device_attribute *attr,
- char *buf)
-{
- return snprintf(buf, PAGE_SIZE, "%s\n", pmu_name_str);
-}
-
-static DEVICE_ATTR_RO(pmu_name);
+static DEVICE_STRING_ATTR_RO(pmu_name, 0444, pmu_name_str);

static struct attribute *intel_pmu_caps_attrs[] = {
- &dev_attr_pmu_name.attr,
- NULL
+ &dev_attr_pmu_name.attr.attr,
+ NULL
};

static DEVICE_ATTR(allow_tsx_force_abort, 0644,
diff --git a/drivers/perf/alibaba_uncore_drw_pmu.c b/drivers/perf/alibaba_uncore_drw_pmu.c
index a9277dcf90ce..a82592e131ba 100644
--- a/drivers/perf/alibaba_uncore_drw_pmu.c
+++ b/drivers/perf/alibaba_uncore_drw_pmu.c
@@ -236,24 +236,16 @@ static const struct attribute_group ali_drw_pmu_cpumask_attr_group = {
.attrs = ali_drw_pmu_cpumask_attrs,
};

-static ssize_t ali_drw_pmu_identifier_show(struct device *dev,
- struct device_attribute *attr,
- char *page)
-{
- return sysfs_emit(page, "%s\n", "ali_drw_pmu");
-}
-
static umode_t ali_drw_pmu_identifier_attr_visible(struct kobject *kobj,
struct attribute *attr, int n)
{
return attr->mode;
}

-static struct device_attribute ali_drw_pmu_identifier_attr =
- __ATTR(identifier, 0444, ali_drw_pmu_identifier_show, NULL);
+static DEVICE_STRING_ATTR_RO(ali_drw_pmu_identifier, 0444, "ali_drw_pmu");

static struct attribute *ali_drw_pmu_identifier_attrs[] = {
- &ali_drw_pmu_identifier_attr.attr,
+ &dev_attr_ali_drw_pmu_identifier.attr.attr,
NULL
};

diff --git a/drivers/perf/arm-cci.c b/drivers/perf/arm-cci.c
index 6be03f81ae5d..e516edc933f2 100644
--- a/drivers/perf/arm-cci.c
+++ b/drivers/perf/arm-cci.c
@@ -127,8 +127,6 @@ enum cci_models {

static void pmu_write_counters(struct cci_pmu *cci_pmu,
unsigned long *mask);
-static ssize_t __maybe_unused cci_pmu_format_show(struct device *dev,
- struct device_attribute *attr, char *buf);
static ssize_t __maybe_unused cci_pmu_event_show(struct device *dev,
struct device_attribute *attr, char *buf);

@@ -138,7 +136,7 @@ static ssize_t __maybe_unused cci_pmu_event_show(struct device *dev,
})[0].attr.attr

#define CCI_FORMAT_EXT_ATTR_ENTRY(_name, _config) \
- CCI_EXT_ATTR_ENTRY(_name, cci_pmu_format_show, (char *)_config)
+ CCI_EXT_ATTR_ENTRY(_name, device_show_string, _config)
#define CCI_EVENT_EXT_ATTR_ENTRY(_name, _config) \
CCI_EXT_ATTR_ENTRY(_name, cci_pmu_event_show, (unsigned long)_config)

@@ -688,14 +686,6 @@ static void __cci_pmu_disable(struct cci_pmu *cci_pmu)
writel(val, cci_pmu->ctrl_base + CCI_PMCR);
}

-static ssize_t cci_pmu_format_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct dev_ext_attribute *eattr = container_of(attr,
- struct dev_ext_attribute, attr);
- return sysfs_emit(buf, "%s\n", (char *)eattr->var);
-}
-
static ssize_t cci_pmu_event_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
diff --git a/drivers/perf/arm-ccn.c b/drivers/perf/arm-ccn.c
index 641471bd5eff..e4408acd4450 100644
--- a/drivers/perf/arm-ccn.c
+++ b/drivers/perf/arm-ccn.c
@@ -215,18 +215,9 @@ static void arm_ccn_pmu_config_set(u64 *config, u32 node_xp, u32 type, u32 port)
*config |= (node_xp << 0) | (type << 8) | (port << 24);
}

-static ssize_t arm_ccn_pmu_format_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct dev_ext_attribute *ea = container_of(attr,
- struct dev_ext_attribute, attr);
-
- return sysfs_emit(buf, "%s\n", (char *)ea->var);
-}
-
#define CCN_FORMAT_ATTR(_name, _config) \
struct dev_ext_attribute arm_ccn_pmu_format_attr_##_name = \
- { __ATTR(_name, S_IRUGO, arm_ccn_pmu_format_show, \
+ { __ATTR(_name, S_IRUGO, device_show_string, \
NULL), _config }

static CCN_FORMAT_ATTR(node, "config:0-7");
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index b9a252272f1e..7aff616523ec 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -223,16 +223,6 @@ arm_cspmu_event_attr_is_visible(struct kobject *kobj,
return attr->mode;
}

-ssize_t arm_cspmu_sysfs_format_show(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct dev_ext_attribute *eattr =
- container_of(attr, struct dev_ext_attribute, attr);
- return sysfs_emit(buf, "%s\n", (char *)eattr->var);
-}
-EXPORT_SYMBOL_GPL(arm_cspmu_sysfs_format_show);
-
static struct attribute *arm_cspmu_format_attrs[] = {
ARM_CSPMU_FORMAT_EVENT_ATTR,
ARM_CSPMU_FORMAT_FILTER_ATTR,
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h b/drivers/perf/arm_cspmu/arm_cspmu.h
index c9163acfe810..2621f3111148 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.h
+++ b/drivers/perf/arm_cspmu/arm_cspmu.h
@@ -28,7 +28,7 @@
})[0].attr.attr)

#define ARM_CSPMU_FORMAT_ATTR(_name, _config) \
- ARM_CSPMU_EXT_ATTR(_name, arm_cspmu_sysfs_format_show, (char *)_config)
+ ARM_CSPMU_EXT_ATTR(_name, device_show_string, _config)

#define ARM_CSPMU_EVENT_ATTR(_name, _config) \
PMU_EVENT_ATTR_ID(_name, arm_cspmu_sysfs_event_show, _config)
@@ -167,11 +167,6 @@ ssize_t arm_cspmu_sysfs_event_show(struct device *dev,
struct device_attribute *attr,
char *buf);

-/* Default function to show format attribute in sysfs. */
-ssize_t arm_cspmu_sysfs_format_show(struct device *dev,
- struct device_attribute *attr,
- char *buf);
-
/* Register vendor backend. */
int arm_cspmu_impl_register(const struct arm_cspmu_impl_match *impl_match);

diff --git a/drivers/perf/arm_dsu_pmu.c b/drivers/perf/arm_dsu_pmu.c
index bae3ca37f846..4e54ce9100a6 100644
--- a/drivers/perf/arm_dsu_pmu.c
+++ b/drivers/perf/arm_dsu_pmu.c
@@ -85,7 +85,7 @@
DSU_EXT_ATTR(_name, dsu_pmu_sysfs_event_show, (unsigned long)_config)

#define DSU_FORMAT_ATTR(_name, _config) \
- DSU_EXT_ATTR(_name, dsu_pmu_sysfs_format_show, (char *)_config)
+ DSU_EXT_ATTR(_name, device_show_string, _config)

#define DSU_CPUMASK_ATTR(_name, _config) \
DSU_EXT_ATTR(_name, dsu_pmu_cpumask_show, (unsigned long)_config)
@@ -139,15 +139,6 @@ static ssize_t dsu_pmu_sysfs_event_show(struct device *dev,
return sysfs_emit(buf, "event=0x%lx\n", (unsigned long)eattr->var);
}

-static ssize_t dsu_pmu_sysfs_format_show(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- struct dev_ext_attribute *eattr = container_of(attr,
- struct dev_ext_attribute, attr);
- return sysfs_emit(buf, "%s\n", (char *)eattr->var);
-}
-
static ssize_t dsu_pmu_cpumask_show(struct device *dev,
struct device_attribute *attr,
char *buf)
diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
index 308c9969642e..c41263c7fe5c 100644
--- a/drivers/perf/cxl_pmu.c
+++ b/drivers/perf/cxl_pmu.c
@@ -208,21 +208,10 @@ static int cxl_pmu_parse_caps(struct device *dev, struct cxl_pmu_info *info)
return 0;
}

-static ssize_t cxl_pmu_format_sysfs_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct dev_ext_attribute *eattr;
-
- eattr = container_of(attr, struct dev_ext_attribute, attr);
-
- return sysfs_emit(buf, "%s\n", (char *)eattr->var);
-}
-
#define CXL_PMU_FORMAT_ATTR(_name, _format)\
(&((struct dev_ext_attribute[]) { \
{ \
- .attr = __ATTR(_name, 0444, \
- cxl_pmu_format_sysfs_show, NULL), \
+ .attr = __ATTR(_name, 0444, device_show_string, NULL), \
.var = (void *)_format \
} \
})[0].attr.attr)
diff --git a/drivers/perf/hisilicon/hisi_pcie_pmu.c b/drivers/perf/hisilicon/hisi_pcie_pmu.c
index 5d1f0e9fdb08..dfd1f448f3fc 100644
--- a/drivers/perf/hisilicon/hisi_pcie_pmu.c
+++ b/drivers/perf/hisilicon/hisi_pcie_pmu.c
@@ -99,16 +99,6 @@ HISI_PCIE_PMU_FILTER_ATTR(len_mode, config1, 11, 10);
HISI_PCIE_PMU_FILTER_ATTR(port, config2, 15, 0);
HISI_PCIE_PMU_FILTER_ATTR(bdf, config2, 31, 16);

-static ssize_t hisi_pcie_format_sysfs_show(struct device *dev, struct device_attribute *attr,
- char *buf)
-{
- struct dev_ext_attribute *eattr;
-
- eattr = container_of(attr, struct dev_ext_attribute, attr);
-
- return sysfs_emit(buf, "%s\n", (char *)eattr->var);
-}
-
static ssize_t hisi_pcie_event_sysfs_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
@@ -120,8 +110,7 @@ static ssize_t hisi_pcie_event_sysfs_show(struct device *dev, struct device_attr

#define HISI_PCIE_PMU_FORMAT_ATTR(_name, _format) \
(&((struct dev_ext_attribute[]){ \
- { .attr = __ATTR(_name, 0444, hisi_pcie_format_sysfs_show, \
- NULL), \
+ { .attr = __ATTR(_name, 0444, device_show_string, NULL), \
.var = (void *)_format } \
})[0].attr.attr)

diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.c b/drivers/perf/hisilicon/hisi_uncore_pmu.c
index 04031450d5fe..382c5567e4e2 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.c
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.c
@@ -22,20 +22,6 @@

#define HISI_MAX_PERIOD(nr) (GENMASK_ULL((nr) - 1, 0))

-/*
- * PMU format attributes
- */
-ssize_t hisi_format_sysfs_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct dev_ext_attribute *eattr;
-
- eattr = container_of(attr, struct dev_ext_attribute, attr);
-
- return sysfs_emit(buf, "%s\n", (char *)eattr->var);
-}
-EXPORT_SYMBOL_GPL(hisi_format_sysfs_show);
-
/*
* PMU event attributes
*/
diff --git a/drivers/perf/hisilicon/hisi_uncore_pmu.h b/drivers/perf/hisilicon/hisi_uncore_pmu.h
index 92402aa69d70..25b2d43b72bf 100644
--- a/drivers/perf/hisilicon/hisi_uncore_pmu.h
+++ b/drivers/perf/hisilicon/hisi_uncore_pmu.h
@@ -33,7 +33,7 @@
})[0].attr.attr)

#define HISI_PMU_FORMAT_ATTR(_name, _config) \
- HISI_PMU_ATTR(_name, hisi_format_sysfs_show, (void *)_config)
+ HISI_PMU_ATTR(_name, device_show_string, _config)
#define HISI_PMU_EVENT_ATTR(_name, _config) \
HISI_PMU_ATTR(_name, hisi_event_sysfs_show, (unsigned long)_config)

@@ -122,8 +122,6 @@ void hisi_uncore_pmu_enable(struct pmu *pmu);
void hisi_uncore_pmu_disable(struct pmu *pmu);
ssize_t hisi_event_sysfs_show(struct device *dev,
struct device_attribute *attr, char *buf);
-ssize_t hisi_format_sysfs_show(struct device *dev,
- struct device_attribute *attr, char *buf);
ssize_t hisi_cpumask_sysfs_show(struct device *dev,
struct device_attribute *attr, char *buf);
int hisi_uncore_pmu_online_cpu(unsigned int cpu, struct hlist_node *node);
diff --git a/drivers/perf/hisilicon/hns3_pmu.c b/drivers/perf/hisilicon/hns3_pmu.c
index 16869bf5bf4c..6d9725bdff83 100644
--- a/drivers/perf/hisilicon/hns3_pmu.c
+++ b/drivers/perf/hisilicon/hns3_pmu.c
@@ -363,16 +363,6 @@ HNS3_PMU_FILTER_ATTR(global, config1, 52, 52);
HNS3_PMU_EVT_PPS_##_name##_TIME, \
HNS3_PMU_FILTER_INTR_##_name})

-static ssize_t hns3_pmu_format_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct dev_ext_attribute *eattr;
-
- eattr = container_of(attr, struct dev_ext_attribute, attr);
-
- return sysfs_emit(buf, "%s\n", (char *)eattr->var);
-}
-
static ssize_t hns3_pmu_event_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -421,7 +411,7 @@ static ssize_t hns3_pmu_filter_mode_show(struct device *dev,
})[0].attr.attr)

#define HNS3_PMU_FORMAT_ATTR(_name, _format) \
- HNS3_PMU_ATTR(_name, hns3_pmu_format_show, (void *)_format)
+ HNS3_PMU_ATTR(_name, device_show_string, _format)
#define HNS3_PMU_EVENT_ATTR(_name, _event) \
HNS3_PMU_ATTR(_name, hns3_pmu_event_show, (void *)_event)
#define HNS3_PMU_FLT_MODE_ATTR(_name, _event) \
diff --git a/drivers/perf/qcom_l3_pmu.c b/drivers/perf/qcom_l3_pmu.c
index f16783d03db7..5fc53781afba 100644
--- a/drivers/perf/qcom_l3_pmu.c
+++ b/drivers/perf/qcom_l3_pmu.c
@@ -609,18 +609,9 @@ static void qcom_l3_cache__event_read(struct perf_event *event)

/* formats */

-static ssize_t l3cache_pmu_format_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct dev_ext_attribute *eattr;
-
- eattr = container_of(attr, struct dev_ext_attribute, attr);
- return sysfs_emit(buf, "%s\n", (char *) eattr->var);
-}
-
#define L3CACHE_PMU_FORMAT_ATTR(_name, _config) \
(&((struct dev_ext_attribute[]) { \
- { .attr = __ATTR(_name, 0444, l3cache_pmu_format_show, NULL), \
+ { .attr = __ATTR(_name, 0444, device_show_string, NULL), \
.var = (void *) _config, } \
})[0].attr.attr)

diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
index 0d49343d704b..79279a359995 100644
--- a/drivers/perf/xgene_pmu.c
+++ b/drivers/perf/xgene_pmu.c
@@ -162,18 +162,9 @@ enum xgene_pmu_dev_type {
/*
* sysfs format attributes
*/
-static ssize_t xgene_pmu_format_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct dev_ext_attribute *eattr;
-
- eattr = container_of(attr, struct dev_ext_attribute, attr);
- return sysfs_emit(buf, "%s\n", (char *) eattr->var);
-}
-
#define XGENE_PMU_FORMAT_ATTR(_name, _config) \
(&((struct dev_ext_attribute[]) { \
- { .attr = __ATTR(_name, S_IRUGO, xgene_pmu_format_show, NULL), \
+ { .attr = __ATTR(_name, S_IRUGO, device_show_string, NULL), \
.var = (void *) _config, } \
})[0].attr.attr)

--
2.43.0

2024-04-20 20:07:45

by Lukas Wunner

[permalink] [raw]

Subject: [PATCH 5/6] platform/x86: Use device_show_string() helper for sysfs attributes

2024-04-20 20:09:14

by Lukas Wunner

[permalink] [raw]

Subject: [PATCH 6/6] scsi: Use device_show_string() helper for sysfs attributes

2024-04-20 21:46:31

by Guenter Roeck

[permalink] [raw]

Subject: Re: [PATCH 2/6] hwmon: Use device_show_string() helper for sysfs attributes

On Sat, Apr 20, 2024 at 10:00:02PM +0200, Lukas Wunner wrote:
> Deduplicate sysfs ->show() callbacks which expose a string at a static
> memory location. Use the newly introduced device_show_string() helper
> in the driver core instead by declaring those sysfs attributes with
> DEVICE_STRING_ATTR_RO().
>
> No functional change intended.
>
> Signed-off-by: Lukas Wunner <[email protected]>

Acked-by: Guenter Roeck <[email protected]>

Guenter

2024-04-22 05:47:51

by Michael Ellerman

[permalink] [raw]

Subject: Re: [PATCH 1/6] driver core: Add device_show_string() helper for sysfs attributes

Lukas Wunner <[email protected]> writes:
> For drivers wishing to expose an unsigned long, int or bool at a static
> memory location in sysfs, the driver core provides ready-made helpers
> such as device_show_ulong() to be used as ->show() callback.
>
> Some drivers need to expose a string and so far they all provide their
> own ->show() implementation. arch/powerpc/perf/hv-24x7.c went so far
> as to create a device_show_string() helper but kept it private.
>
> Make it public for reuse by other drivers. The pattern seems to be
> sufficiently frequent to merit a public helper.
>
> Add a DEVICE_STRING_ATTR_RO() macro in line with the existing
> DEVICE_ULONG_ATTR() and similar macros to ease declaration of string
> attributes.
>
> Signed-off-by: Lukas Wunner <[email protected]>
> ---
> arch/powerpc/perf/hv-24x7.c | 10 ----------
> drivers/base/core.c | 9 +++++++++
> include/linux/device.h | 15 +++++++++++++++
> 3 files changed, 24 insertions(+), 10 deletions(-)

Acked-by: Michael Ellerman <[email protected]> (powerpc)

cheers

2024-04-22 13:53:16

by Hans de Goede

[permalink] [raw]

Subject: Re: [PATCH 5/6] platform/x86: Use device_show_string() helper for sysfs attributes

Hi,

On 4/20/24 10:00 PM, Lukas Wunner wrote:
> Deduplicate sysfs ->show() callbacks which expose a string at a static
> memory location. Use the newly introduced device_show_string() helper
> in the driver core instead by declaring those sysfs attributes with
> DEVICE_STRING_ATTR_RO().
>
> No functional change intended.
>
> Signed-off-by: Lukas Wunner <[email protected]>

Thanks, patch looks good to me:

Acked-by: Hans de Goede <[email protected]>

Feel free to upstream this though whatever git tree is convenient.

Regards,

Hans

> ---
> drivers/platform/x86/asus-wmi.c | 62 +++++++---------------------
> drivers/platform/x86/thinkpad_acpi.c | 10 +----
> drivers/platform/x86/toshiba_acpi.c | 9 +---
> 3 files changed, 20 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 3f07bbf809ef..78d7579b2fdd 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -915,17 +915,12 @@ static ssize_t kbd_rgb_mode_store(struct device *dev,
> }
> static DEVICE_ATTR_WO(kbd_rgb_mode);
>
> -static ssize_t kbd_rgb_mode_index_show(struct device *device,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - return sysfs_emit(buf, "%s\n", "cmd mode red green blue speed");
> -}
> -static DEVICE_ATTR_RO(kbd_rgb_mode_index);
> +static DEVICE_STRING_ATTR_RO(kbd_rgb_mode_index, 0444,
> + "cmd mode red green blue speed");
>
> static struct attribute *kbd_rgb_mode_attrs[] = {
> &dev_attr_kbd_rgb_mode.attr,
> - &dev_attr_kbd_rgb_mode_index.attr,
> + &dev_attr_kbd_rgb_mode_index.attr.attr,
> NULL,
> };
>
> @@ -967,17 +962,12 @@ static ssize_t kbd_rgb_state_store(struct device *dev,
> }
> static DEVICE_ATTR_WO(kbd_rgb_state);
>
> -static ssize_t kbd_rgb_state_index_show(struct device *device,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - return sysfs_emit(buf, "%s\n", "cmd boot awake sleep keyboard");
> -}
> -static DEVICE_ATTR_RO(kbd_rgb_state_index);
> +static DEVICE_STRING_ATTR_RO(kbd_rgb_state_index, 0444,
> + "cmd boot awake sleep keyboard");
>
> static struct attribute *kbd_rgb_state_attrs[] = {
> &dev_attr_kbd_rgb_state.attr,
> - &dev_attr_kbd_rgb_state_index.attr,
> + &dev_attr_kbd_rgb_state_index.attr.attr,
> NULL,
> };
>
> @@ -2493,13 +2483,6 @@ static ssize_t pwm1_enable_store(struct device *dev,
> return count;
> }
>
> -static ssize_t fan1_label_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - return sysfs_emit(buf, "%s\n", ASUS_FAN_DESC);
> -}
> -
> static ssize_t asus_hwmon_temp1(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> @@ -2534,13 +2517,6 @@ static ssize_t fan2_input_show(struct device *dev,
> return sysfs_emit(buf, "%d\n", value * 100);
> }
>
> -static ssize_t fan2_label_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - return sysfs_emit(buf, "%s\n", ASUS_GPU_FAN_DESC);
> -}
> -
> /* Middle/Center fan on modern ROG laptops */
> static ssize_t fan3_input_show(struct device *dev,
> struct device_attribute *attr,
> @@ -2559,13 +2535,6 @@ static ssize_t fan3_input_show(struct device *dev,
> return sysfs_emit(buf, "%d\n", value * 100);
> }
>
> -static ssize_t fan3_label_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - return sysfs_emit(buf, "%s\n", ASUS_MID_FAN_DESC);
> -}
> -
> static ssize_t pwm2_enable_show(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> @@ -2662,15 +2631,16 @@ static ssize_t pwm3_enable_store(struct device *dev,
> static DEVICE_ATTR_RW(pwm1);
> static DEVICE_ATTR_RW(pwm1_enable);
> static DEVICE_ATTR_RO(fan1_input);
> -static DEVICE_ATTR_RO(fan1_label);
> +static DEVICE_STRING_ATTR_RO(fan1_label, 0444, ASUS_FAN_DESC);
> +
> /* Fan2 - GPU fan */
> static DEVICE_ATTR_RW(pwm2_enable);
> static DEVICE_ATTR_RO(fan2_input);
> -static DEVICE_ATTR_RO(fan2_label);
> +static DEVICE_STRING_ATTR_RO(fan2_label, 0444, ASUS_GPU_FAN_DESC);
> /* Fan3 - Middle/center fan */
> static DEVICE_ATTR_RW(pwm3_enable);
> static DEVICE_ATTR_RO(fan3_input);
> -static DEVICE_ATTR_RO(fan3_label);
> +static DEVICE_STRING_ATTR_RO(fan3_label, 0444, ASUS_MID_FAN_DESC);
>
> /* Temperature */
> static DEVICE_ATTR(temp1_input, S_IRUGO, asus_hwmon_temp1, NULL);
> @@ -2681,11 +2651,11 @@ static struct attribute *hwmon_attributes[] = {
> &dev_attr_pwm2_enable.attr,
> &dev_attr_pwm3_enable.attr,
> &dev_attr_fan1_input.attr,
> - &dev_attr_fan1_label.attr,
> + &dev_attr_fan1_label.attr.attr,
> &dev_attr_fan2_input.attr,
> - &dev_attr_fan2_label.attr,
> + &dev_attr_fan2_label.attr.attr,
> &dev_attr_fan3_input.attr,
> - &dev_attr_fan3_label.attr,
> + &dev_attr_fan3_label.attr.attr,
>
> &dev_attr_temp1_input.attr,
> NULL
> @@ -2702,17 +2672,17 @@ static umode_t asus_hwmon_sysfs_is_visible(struct kobject *kobj,
> if (asus->fan_type != FAN_TYPE_AGFN)
> return 0;
> } else if (attr == &dev_attr_fan1_input.attr
> - || attr == &dev_attr_fan1_label.attr
> + || attr == &dev_attr_fan1_label.attr.attr
> || attr == &dev_attr_pwm1_enable.attr) {
> if (asus->fan_type == FAN_TYPE_NONE)
> return 0;
> } else if (attr == &dev_attr_fan2_input.attr
> - || attr == &dev_attr_fan2_label.attr
> + || attr == &dev_attr_fan2_label.attr.attr
> || attr == &dev_attr_pwm2_enable.attr) {
> if (asus->gpu_fan_type == FAN_TYPE_NONE)
> return 0;
> } else if (attr == &dev_attr_fan3_input.attr
> - || attr == &dev_attr_fan3_label.attr
> + || attr == &dev_attr_fan3_label.attr.attr
> || attr == &dev_attr_pwm3_enable.attr) {
> if (asus->mid_fan_type == FAN_TYPE_NONE)
> return 0;
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 82429e59999d..47a64a213d14 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -10991,13 +10991,7 @@ static struct ibm_struct auxmac_data = {
> .name = "auxmac",
> };
>
> -static ssize_t auxmac_show(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> -{
> - return sysfs_emit(buf, "%s\n", auxmac);
> -}
> -static DEVICE_ATTR_RO(auxmac);
> +static DEVICE_STRING_ATTR_RO(auxmac, 0444, auxmac);
>
> static umode_t auxmac_attr_is_visible(struct kobject *kobj,
> struct attribute *attr, int n)
> @@ -11006,7 +11000,7 @@ static umode_t auxmac_attr_is_visible(struct kobject *kobj,
> }
>
> static struct attribute *auxmac_attributes[] = {
> - &dev_attr_auxmac.attr,
> + &dev_attr_auxmac.attr.attr,
> NULL
> };
>
> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> index 291f14ef6702..01cf60a015bf 100644
> --- a/drivers/platform/x86/toshiba_acpi.c
> +++ b/drivers/platform/x86/toshiba_acpi.c
> @@ -1814,12 +1814,7 @@ static DECLARE_WORK(kbd_bl_work, toshiba_acpi_kbd_bl_work);
> /*
> * Sysfs files
> */
> -static ssize_t version_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - return sprintf(buf, "%s\n", TOSHIBA_ACPI_VERSION);
> -}
> -static DEVICE_ATTR_RO(version);
> +static DEVICE_STRING_ATTR_RO(version, 0444, TOSHIBA_ACPI_VERSION);
>
> static ssize_t fan_store(struct device *dev,
> struct device_attribute *attr,
> @@ -2428,7 +2423,7 @@ static ssize_t cooling_method_store(struct device *dev,
> static DEVICE_ATTR_RW(cooling_method);
>
> static struct attribute *toshiba_attributes[] = {
> - &dev_attr_version.attr,
> + &dev_attr_version.attr.attr,
> &dev_attr_fan.attr,
> &dev_attr_kbd_backlight_mode.attr,
> &dev_attr_kbd_type.attr,