2021-06-16 22:54:02

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 1/7] ACPI: sysfs: Make sparse happy about address space in use

Sparse is not happy about address space in use in acpi_data_show():

drivers/acpi/sysfs.c:428:14: warning: incorrect type in assignment (different address spaces)
drivers/acpi/sysfs.c:428:14: expected void [noderef] __iomem *base
drivers/acpi/sysfs.c:428:14: got void *
drivers/acpi/sysfs.c:431:59: warning: incorrect type in argument 4 (different address spaces)
drivers/acpi/sysfs.c:431:59: expected void const *from
drivers/acpi/sysfs.c:431:59: got void [noderef] __iomem *base
drivers/acpi/sysfs.c:433:30: warning: incorrect type in argument 1 (different address spaces)
drivers/acpi/sysfs.c:433:30: expected void *logical_address
drivers/acpi/sysfs.c:433:30: got void [noderef] __iomem *base

Indeed, acpi_os_map_memory() returns a void pointer with dropped specific
address space. Hence, we don't need to carry out __iomem in acpi_data_show().

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/acpi/sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index 88629d26bd48..767aa65e4bdd 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -419,7 +419,7 @@ static ssize_t acpi_data_show(struct file *filp, struct kobject *kobj,
loff_t offset, size_t count)
{
struct acpi_data_attr *data_attr;
- void __iomem *base;
+ void *base;
ssize_t rc;

data_attr = container_of(bin_attr, struct acpi_data_attr, attr);
--
2.30.2


2021-06-16 22:54:16

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 5/7] ACPI: sysfs: Use __ATTR_RO() and __ATTR_RW() macros

We have a few open-coded __ATTR_RO() and __ATTR_RW() macros.
Replace the custom code with generic macros.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/acpi/sysfs.c | 23 +++++++----------------
1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index 7239d87e78e6..231aaa8b6c2c 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -915,27 +915,22 @@ static void __exit interrupt_stats_exit(void)
return;
}

-static ssize_t
-acpi_show_profile(struct kobject *kobj, struct kobj_attribute *attr,
- char *buf)
+static ssize_t pm_profile_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
{
return sprintf(buf, "%d\n", acpi_gbl_FADT.preferred_profile);
}

-static const struct kobj_attribute pm_profile_attr =
- __ATTR(pm_profile, S_IRUGO, acpi_show_profile, NULL);
+static const struct kobj_attribute pm_profile_attr = __ATTR_RO(pm_profile);

-static ssize_t hotplug_enabled_show(struct kobject *kobj,
- struct kobj_attribute *attr, char *buf)
+static ssize_t enabled_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
{
struct acpi_hotplug_profile *hotplug = to_acpi_hotplug_profile(kobj);

return sprintf(buf, "%d\n", hotplug->enabled);
}

-static ssize_t hotplug_enabled_store(struct kobject *kobj,
- struct kobj_attribute *attr,
- const char *buf, size_t size)
+static ssize_t enabled_store(struct kobject *kobj, struct kobj_attribute *attr,
+ const char *buf, size_t size)
{
struct acpi_hotplug_profile *hotplug = to_acpi_hotplug_profile(kobj);
unsigned int val;
@@ -947,9 +942,7 @@ static ssize_t hotplug_enabled_store(struct kobject *kobj,
return size;
}

-static struct kobj_attribute hotplug_enabled_attr =
- __ATTR(enabled, S_IRUGO | S_IWUSR, hotplug_enabled_show,
- hotplug_enabled_store);
+static struct kobj_attribute hotplug_enabled_attr = __ATTR_RW(enabled);

static struct attribute *hotplug_profile_attrs[] = {
&hotplug_enabled_attr.attr,
@@ -1007,9 +1000,7 @@ static ssize_t force_remove_store(struct kobject *kobj,
return size;
}

-static const struct kobj_attribute force_remove_attr =
- __ATTR(force_remove, S_IRUGO | S_IWUSR, force_remove_show,
- force_remove_store);
+static const struct kobj_attribute force_remove_attr = __ATTR_RW(force_remove);

int __init acpi_sysfs_init(void)
{
--
2.30.2

2021-06-16 22:54:24

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 4/7] ACPI: sysfs: Refactor param_get_trace_state() to drop dead code

The param_get_trace_state() has a few dead code issues:
- 'return 0;' is never reachable
- a few 'else' keywords are redundant

Refactor param_get_trace_state() to drop dead code.

Note, leave one 'else' in order to have the best readability.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/acpi/sysfs.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index efb8c2dfa9cb..7239d87e78e6 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -255,16 +255,12 @@ static int param_get_trace_state(char *buffer, const struct kernel_param *kp)
{
if (!(acpi_gbl_trace_flags & ACPI_TRACE_ENABLED))
return sprintf(buffer, "disable\n");
- else {
- if (acpi_gbl_trace_method_name) {
- if (acpi_gbl_trace_flags & ACPI_TRACE_ONESHOT)
- return sprintf(buffer, "method-once\n");
- else
- return sprintf(buffer, "method\n");
- } else
- return sprintf(buffer, "enable\n");
- }
- return 0;
+ if (!acpi_gbl_trace_method_name)
+ return sprintf(buffer, "enable\n");
+ if (acpi_gbl_trace_flags & ACPI_TRACE_ONESHOT)
+ return sprintf(buffer, "method-once\n");
+ else
+ return sprintf(buffer, "method\n");
}

module_param_call(trace_state, param_set_trace_state, param_get_trace_state,
--
2.30.2

2021-06-16 22:54:59

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 7/7] ACPI: sysfs: Sort headers alphabetically

For the sake of better maintenance, sort included headers alphabetically.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/acpi/sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index 5474563d72b8..00c0ebaab29f 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -5,11 +5,11 @@

#define pr_fmt(fmt) "ACPI: " fmt

+#include <linux/acpi.h>
#include <linux/bitmap.h>
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/moduleparam.h>
-#include <linux/acpi.h>

#include "internal.h"

--
2.30.2

2021-06-16 22:55:35

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 3/7] ACPI: sysfs: Unify pattern of memory allocations

Use the form of foo = kmalloc(sizeof(*foo)) everywhere in order to
unify pattern of memory allocations.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/acpi/sysfs.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index 44e50c36e70a..efb8c2dfa9cb 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -388,8 +388,7 @@ acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context)

switch (event) {
case ACPI_TABLE_EVENT_INSTALL:
- table_attr =
- kzalloc(sizeof(struct acpi_table_attr), GFP_KERNEL);
+ table_attr = kzalloc(sizeof(*table_attr), GFP_KERNEL);
if (!table_attr)
return AE_NO_MEMORY;

@@ -846,13 +845,11 @@ void acpi_irq_stats_init(void)
num_gpes = acpi_current_gpe_count;
num_counters = num_gpes + ACPI_NUM_FIXED_EVENTS + NUM_COUNTERS_EXTRA;

- all_attrs = kcalloc(num_counters + 1, sizeof(struct attribute *),
- GFP_KERNEL);
+ all_attrs = kcalloc(num_counters + 1, sizeof(*all_attrs), GFP_KERNEL);
if (all_attrs == NULL)
return;

- all_counters = kcalloc(num_counters, sizeof(struct event_counter),
- GFP_KERNEL);
+ all_counters = kcalloc(num_counters, sizeof(*all_counters), GFP_KERNEL);
if (all_counters == NULL)
goto fail;

@@ -860,8 +857,7 @@ void acpi_irq_stats_init(void)
if (ACPI_FAILURE(status))
goto fail;

- counter_attrs = kcalloc(num_counters, sizeof(struct kobj_attribute),
- GFP_KERNEL);
+ counter_attrs = kcalloc(num_counters, sizeof(*counter_attrs), GFP_KERNEL);
if (counter_attrs == NULL)
goto fail;

--
2.30.2

2021-06-16 22:55:38

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 2/7] ACPI: sysfs: Allow bitmap list to be supplied to acpi_mask_gpe

Currently we need to use as many acpi_mask_gpe options as we want to have
GPEs to be masked. Even with two it already becomes inconveniently large
the kernel command line.

Instead, allow acpi_mask_gpe to represent bitmap list.

Signed-off-by: Andy Shevchenko <[email protected]>
---
Documentation/admin-guide/kernel-parameters.txt | 2 +-
drivers/acpi/sysfs.c | 13 ++++++++++---
2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a06a8bfb02a4..c00792362d28 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -113,7 +113,7 @@
the GPE dispatcher.
This facility can be used to prevent such uncontrolled
GPE floodings.
- Format: <byte>
+ Format: <byte> or <bitmap-list>

acpi_no_auto_serialize [HW,ACPI]
Disable auto-serialization of AML methods
diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
index 767aa65e4bdd..44e50c36e70a 100644
--- a/drivers/acpi/sysfs.c
+++ b/drivers/acpi/sysfs.c
@@ -5,6 +5,7 @@

#define pr_fmt(fmt) "ACPI: " fmt

+#include <linux/bitmap.h>
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/moduleparam.h>
@@ -794,6 +795,7 @@ static ssize_t counter_set(struct kobject *kobj,
* the GPE flooding for GPE 00, they need to specify the following boot
* parameter:
* acpi_mask_gpe=0x00
+ * Note, the parameter can be a list (see bitmap_parselist() for the details).
* The masking status can be modified by the following runtime controlling
* interface:
* echo unmask > /sys/firmware/acpi/interrupts/gpe00
@@ -803,11 +805,16 @@ static DECLARE_BITMAP(acpi_masked_gpes_map, ACPI_MASKABLE_GPE_MAX) __initdata;

static int __init acpi_gpe_set_masked_gpes(char *val)
{
+ int ret;
u8 gpe;

- if (kstrtou8(val, 0, &gpe))
- return -EINVAL;
- set_bit(gpe, acpi_masked_gpes_map);
+ ret = kstrtou8(val, 0, &gpe);
+ if (ret) {
+ ret = bitmap_parselist(val, acpi_masked_gpes_map, ACPI_MASKABLE_GPE_MAX);
+ if (ret)
+ return ret;
+ } else
+ set_bit(gpe, acpi_masked_gpes_map);

return 1;
}
--
2.30.2

2021-06-17 19:20:48

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v1 1/7] ACPI: sysfs: Make sparse happy about address space in use

On Wed, Jun 16, 2021 at 7:03 PM Andy Shevchenko
<[email protected]> wrote:
>
> Sparse is not happy about address space in use in acpi_data_show():
>
> drivers/acpi/sysfs.c:428:14: warning: incorrect type in assignment (different address spaces)
> drivers/acpi/sysfs.c:428:14: expected void [noderef] __iomem *base
> drivers/acpi/sysfs.c:428:14: got void *
> drivers/acpi/sysfs.c:431:59: warning: incorrect type in argument 4 (different address spaces)
> drivers/acpi/sysfs.c:431:59: expected void const *from
> drivers/acpi/sysfs.c:431:59: got void [noderef] __iomem *base
> drivers/acpi/sysfs.c:433:30: warning: incorrect type in argument 1 (different address spaces)
> drivers/acpi/sysfs.c:433:30: expected void *logical_address
> drivers/acpi/sysfs.c:433:30: got void [noderef] __iomem *base
>
> Indeed, acpi_os_map_memory() returns a void pointer with dropped specific
> address space. Hence, we don't need to carry out __iomem in acpi_data_show().
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/acpi/sysfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c
> index 88629d26bd48..767aa65e4bdd 100644
> --- a/drivers/acpi/sysfs.c
> +++ b/drivers/acpi/sysfs.c
> @@ -419,7 +419,7 @@ static ssize_t acpi_data_show(struct file *filp, struct kobject *kobj,
> loff_t offset, size_t count)
> {
> struct acpi_data_attr *data_attr;
> - void __iomem *base;
> + void *base;
> ssize_t rc;
>
> data_attr = container_of(bin_attr, struct acpi_data_attr, attr);
> --

Applied as 5.14 material along with patches [2-4,7/7] from this series.

Patches [5-6/7] did not apply for me on top of my acpi-sysfs branch
(that is included into my linux-next branch), so I have not applied
them.

Thanks!