ACPI / CPPC: set an error code on probe error path

PATCH In review Dan Carpenter 3 participants 6 messages Nov 30, 2016 – Dec 1, 2016

Subsystems (3)

Kernel janitors ACPI ACPI CPPC

lore

Share & tools

lore

Dan Carpenter

author

2016-11-30 19:22 UTC #0 lore raw

PATCH 1/1 ACPI / CPPC: set an error code on probe error path

We should return -EINVAL if get_cpu_device() fails.

Fixes 158c998ea44b ("ACPI / CPPC: add sysfs support to compute delivered performance")

Signed-off-by Dan Carpenter <[email protected]>

drivers/acpi/cppc_acpi.c +3 -1

index d0d0504..e0ea8f5 100644
@@ -784,8 +784,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 
 	/* Add per logical CPU nodes for reading its feedback counters. */
 	cpu_dev = get_cpu_device(pr->id);
-	if (!cpu_dev)
+	if (!cpu_dev) {
+		ret = -EINVAL;
 		goto out_free;
+	}
 
 	ret = kobject_init_and_add(&cpc_ptr->kobj, &cppc_ktype, &cpu_dev->kobj,
 			"acpi_cppc");

Rafael J. Wysocki

maintainer

2016-11-30 21:53 UTC #1 lore raw

Quoted 10 lines · Dan Carpenter

We should return -EINVAL if get_cpu_device() fails.
Fixes 158c998ea44b ("ACPI / CPPC: add sysfs support to compute delivered performance")
Signed-off-by Dan Carpenter <[email protected]>
drivers/acpi/cppc_acpi.c +0 -0
index d0d0504..e0ea8f5 100644

drivers/acpi/cppc_acpi.c int acpi_cppc_processor_probe(struct acpi_processor *pr) · line 788

Quoted 6 lines

drivers/acpi/cppc_acpi.c +2 -1

@@ -784,8 +784,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 
        /* Add per logical CPU nodes for reading its feedback counters. */
        cpu_dev = get_cpu_device(pr->id);
-       if (!cpu_dev)
+       if (!cpu_dev) {
+               ret = -EINVAL;

ret is initialized here AFAICS.

Do you that its value is not the right one?

drivers/acpi/cppc_acpi.c int acpi_cppc_processor_probe(struct acpi_processor *pr)

Quoted 6 lines

drivers/acpi/cppc_acpi.c +1 -1

                goto out_free;
+       }
 
        ret = kobject_init_and_add(&cpc_ptr->kobj, &cppc_ktype, &cpu_dev->kobj,
                        "acpi_cppc");
--

Thanks,
Rafael

Rafael J. Wysocki

maintainer

2016-11-30 21:55 UTC #2 lore raw

Quoted 20 lines · Rafael J. Wysocki

Quoted 10 lines · Dan Carpenter
We should return -EINVAL if get_cpu_device() fails.
Fixes 158c998ea44b ("ACPI / CPPC: add sysfs support to compute delivered performance")
Signed-off-by Dan Carpenter <[email protected]>
drivers/acpi/cppc_acpi.c +0 -0
index d0d0504..e0ea8f5 100644
drivers/acpi/cppc_acpi.c int acpi_cppc_processor_probe(struct acpi_processor *pr) · line 788
Quoted 6 lines
drivers/acpi/cppc_acpi.c +2 -1
@@ -784,8 +784,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 
        /* Add per logical CPU nodes for reading its feedback counters. */
        cpu_dev = get_cpu_device(pr->id);
-       if (!cpu_dev)
+       if (!cpu_dev) {
+               ret = -EINVAL;
ret is initialized here AFAICS.

Quoted 1 line

Do you that its value is not the right one?

This was supposed to be "Do you mean that ...", sorry.

Quoted 7 lines

drivers/acpi/cppc_acpi.c int acpi_cppc_processor_probe(struct acpi_processor *pr)

Quoted 7 lines

drivers/acpi/cppc_acpi.c +1 -1

                goto out_free;
+       }
 
        ret = kobject_init_and_add(&cpc_ptr->kobj, &cppc_ktype, &cpu_dev->kobj,
                        "acpi_cppc");
--

Thanks,
Rafael

Dan Carpenter

author

2016-11-30 22:03 UTC #3 lore raw

In reply to Rafael J. Wysocki

Quoted 20 lines · Rafael J. Wysocki

Quoted 10 lines · Dan Carpenter
We should return -EINVAL if get_cpu_device() fails.
Fixes 158c998ea44b ("ACPI / CPPC: add sysfs support to compute delivered performance")
Signed-off-by Dan Carpenter <[email protected]>
drivers/acpi/cppc_acpi.c +0 -0
index d0d0504..e0ea8f5 100644
drivers/acpi/cppc_acpi.c int acpi_cppc_processor_probe(struct acpi_processor *pr) · line 788
Quoted 6 lines
drivers/acpi/cppc_acpi.c +2 -1
@@ -784,8 +784,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 
        /* Add per logical CPU nodes for reading its feedback counters. */
        cpu_dev = get_cpu_device(pr->id);
-       if (!cpu_dev)
+       if (!cpu_dev) {
+               ret = -EINVAL;
ret is initialized here AFAICS.

Quoted 1 line

Do you that its value is not the right one?

I'm looking at linux-next. It's set to zero but we presumably want to return an error code.

regards,
dan carpenter

Rafael J. Wysocki

maintainer

2016-11-30 22:14 UTC #4 lore raw

Quoted 21 lines · Dan Carpenter

Quoted 20 lines · Rafael J. Wysocki
Quoted 10 lines · Dan Carpenter
We should return -EINVAL if get_cpu_device() fails.
Fixes 158c998ea44b ("ACPI / CPPC: add sysfs support to compute delivered performance")
Signed-off-by Dan Carpenter <[email protected]>
drivers/acpi/cppc_acpi.c +0 -0
index d0d0504..e0ea8f5 100644
drivers/acpi/cppc_acpi.c int acpi_cppc_processor_probe(struct acpi_processor *pr) · line 788
Quoted 6 lines
drivers/acpi/cppc_acpi.c +2 -1
@@ -784,8 +784,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 
        /* Add per logical CPU nodes for reading its feedback counters. */
        cpu_dev = get_cpu_device(pr->id);
-       if (!cpu_dev)
+       if (!cpu_dev) {
+               ret = -EINVAL;
ret is initialized here AFAICS.

Quoted 4 lines

Quoted 1 line
Do you that its value is not the right one?
I'm looking at linux-next. It's set to zero but we presumably want to return an error code.

It was slightly unclear what the bug was.

Thanks,
Rafael

Rafael J. Wysocki

2016-12-01 14:14 UTC #5 lore raw

Quoted 23 lines · Rafael J. Wysocki

Quoted 21 lines · Dan Carpenter
Quoted 20 lines · Rafael J. Wysocki
Quoted 10 lines · Dan Carpenter
We should return -EINVAL if get_cpu_device() fails.
Fixes 158c998ea44b ("ACPI / CPPC: add sysfs support to compute delivered performance")
Signed-off-by Dan Carpenter <[email protected]>
drivers/acpi/cppc_acpi.c +0 -0
index d0d0504..e0ea8f5 100644
drivers/acpi/cppc_acpi.c int acpi_cppc_processor_probe(struct acpi_processor *pr) · line 788
Quoted 6 lines
drivers/acpi/cppc_acpi.c +2 -1
@@ -784,8 +784,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 
        /* Add per logical CPU nodes for reading its feedback counters. */
        cpu_dev = get_cpu_device(pr->id);
-       if (!cpu_dev)
+       if (!cpu_dev) {
+               ret = -EINVAL;
ret is initialized here AFAICS.

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
Do you that its value is not the right one?
I'm looking at linux-next. It's set to zero but we presumably want to return an error code.
OK
It was slightly unclear what the bug was.

Applied now.

Thanks,
Rafael