Convert ACPI fan driver to platform driver

PATCH In review Aaron Lu 3 participants 11 messages Dec 3, 2013 – Dec 5, 2013

Subsystems (2)

ACPI ACPI component architecture (acpica)

lore

Share & tools

lore

Aaron Lu

author

2013-12-03 08:28 UTC #0 lore raw

cover Convert ACPI fan driver to platform driver

This patchset converts ACPI fan driver to platform driver. Patch 1-3 are cleanups for existing fan driver and patch 4 does the convertion.

Tested on harris beach.
Apply on top of Rafael's linux-next branch.

Aaron Lu (4):
ACPI / fan: remove unused macro for debug
ACPI / fan: remove no need check for device pointer
ACPI / fan: use acpi_device_xxx_power instead of acpi_bus equivelant ACPI / fan: convert to platform driver

 drivers/acpi/acpi_platform.c |  3 ++
 drivers/acpi/device_pm.c     |  1 +
 drivers/acpi/fan.c           | 88 ++++++++++++++++----------------------------
 drivers/acpi/internal.h      |  2 -
 include/acpi/acpi_bus.h      |  1 +
 5 files changed, 36 insertions(+), 59 deletions(-)

Signature · 1 line

1.8.3.1

Aaron Lu

author

2013-12-03 08:28 UTC #1 lore raw

PATCH 1/4 ACPI / fan: remove unused macro for debug

The _COMPONENT and ACPI_MODULE_NAME(name) is used for ACPI_DEBUG_PRINT, since there is no such use in fan module we can remove them.

Signed-off-by Aaron Lu <[email protected]>

---
 drivers/acpi/fan.c | 3 ---
 1 file changed, 3 deletions(-)

drivers/acpi/fan.c +0 -3

index 1fb62900f32a..9638d28513e8 100644
@@ -36,9 +36,6 @@
 #define ACPI_FAN_CLASS			"fan"
 #define ACPI_FAN_FILE_STATE		"state"
 
-#define _COMPONENT		ACPI_FAN_COMPONENT
-ACPI_MODULE_NAME("fan");
-
 MODULE_AUTHOR("Paul Diefenbaugh");
 MODULE_DESCRIPTION("ACPI Fan Driver");
 MODULE_LICENSE("GPL");

Signature · 1 line

1.8.3.1

Aaron Lu

author

2013-12-03 08:29 UTC #2 lore raw

PATCH 2/4 ACPI / fan: remove no need check for device pointer

In reply to Aaron Lu

The device pointer will not be NULL in the PM callback and ACPI driver's add/remove callback, so checking NULL for them isn't necessary.

Signed-off-by Aaron Lu <[email protected]>

---
 drivers/acpi/fan.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

drivers/acpi/fan.c +1 ~~-17~~

index 9638d28513e8..4a624a35f07d 100644
@@ -124,9 +124,6 @@ static int acpi_fan_add(struct acpi_device *device)
 	int result = 0;
 	struct thermal_cooling_device *cdev;
 
-	if (!device)
-		return -EINVAL;
-
 	strcpy(acpi_device_name(device), "Fan");
 	strcpy(acpi_device_class(device), ACPI_FAN_CLASS);

@@ -170,14 +167,7 @@ end:
 
 static int acpi_fan_remove(struct acpi_device *device)
 {
-	struct thermal_cooling_device *cdev;
-
-	if (!device)
-		return -EINVAL;
-
-	cdev =  acpi_driver_data(device);
-	if (!cdev)
-		return -EINVAL;
+	struct thermal_cooling_device *cdev = acpi_driver_data(device);
 
 	sysfs_remove_link(&device->dev.kobj, "thermal_cooling");
 	sysfs_remove_link(&cdev->device.kobj, "device");

@@ -189,9 +179,6 @@ static int acpi_fan_remove(struct acpi_device *device)
 #ifdef CONFIG_PM_SLEEP
 static int acpi_fan_suspend(struct device *dev)
 {
-	if (!dev)
-		return -EINVAL;
-
 	acpi_bus_set_power(to_acpi_device(dev)->handle, ACPI_STATE_D0);
 
 	return AE_OK;

@@ -201,9 +188,6 @@ static int acpi_fan_resume(struct device *dev)
 {
 	int result;
 
-	if (!dev)
-		return -EINVAL;
-
 	result = acpi_bus_update_power(to_acpi_device(dev)->handle, NULL);
 	if (result)
 		printk(KERN_ERR PREFIX "Error updating fan power state\n");

Signature · 1 line

1.8.3.1

Aaron Lu

author

2013-12-03 08:28 UTC #3 lore raw

PATCH 3/4 ACPI / fan: use acpi_device_xxx_power instead of acpi_bus equivelant

In reply to Aaron Lu

When we have the acpi_device pointer, there is no need to pass the device's handle to the acpi_bus_xxx_power functions to get/set/update the device's power state, instead, use the acpi_device_xxx_power functions directly.

To make this happen for fan module, export acpi_device_update_power.

Signed-off-by Aaron Lu <[email protected]>

---
 drivers/acpi/device_pm.c |  1 +
 drivers/acpi/fan.c       | 10 +++++-----
 drivers/acpi/internal.h  |  2 --
 include/acpi/acpi_bus.h  |  1 +
 4 files changed, 7 insertions(+), 7 deletions(-)

drivers/acpi/device_pm.c +1 -0

index d49f1e464703..d85b88f88979 100644
@@ -343,6 +343,7 @@ int acpi_device_update_power(struct acpi_device *device, int *state_p)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(acpi_device_update_power);
 
 int acpi_bus_update_power(acpi_handle handle, int *state_p)
 {

drivers/acpi/fan.c +5 -5

index 4a624a35f07d..8683bed14dac 100644
@@ -85,7 +85,7 @@ static int fan_get_cur_state(struct thermal_cooling_device *cdev, unsigned long
 	if (!device)
 		return -EINVAL;
 
-	result = acpi_bus_update_power(device->handle, &acpi_state);
+	result = acpi_device_update_power(device, &acpi_state);
 	if (result)
 		return result;

@@ -103,7 +103,7 @@ fan_set_cur_state(struct thermal_cooling_device *cdev, unsigned long state)
 	if (!device || (state != 0 && state != 1))
 		return -EINVAL;
 
-	result = acpi_bus_set_power(device->handle,
+	result = acpi_device_set_power(device,
 				state ? ACPI_STATE_D0 : ACPI_STATE_D3_COLD);
 
 	return result;

@@ -127,7 +127,7 @@ static int acpi_fan_add(struct acpi_device *device)
 	strcpy(acpi_device_name(device), "Fan");
 	strcpy(acpi_device_class(device), ACPI_FAN_CLASS);
 
-	result = acpi_bus_update_power(device->handle, NULL);
+	result = acpi_device_update_power(device, NULL);
 	if (result) {
 		printk(KERN_ERR PREFIX "Setting initial power state\n");
 		goto end;

@@ -179,7 +179,7 @@ static int acpi_fan_remove(struct acpi_device *device)
 #ifdef CONFIG_PM_SLEEP
 static int acpi_fan_suspend(struct device *dev)
 {
-	acpi_bus_set_power(to_acpi_device(dev)->handle, ACPI_STATE_D0);
+	acpi_device_set_power(to_acpi_device(dev), ACPI_STATE_D0);
 
 	return AE_OK;
 }

@@ -188,7 +188,7 @@ static int acpi_fan_resume(struct device *dev)
 {
 	int result;
 
-	result = acpi_bus_update_power(to_acpi_device(dev)->handle, NULL);
+	result = acpi_device_update_power(to_acpi_device(dev), NULL);
 	if (result)
 		printk(KERN_ERR PREFIX "Error updating fan power state\n");

drivers/acpi/internal.h +0 -2

index b125fdb0b30c..d13e66cac8e1 100644
@@ -106,8 +106,6 @@ int acpi_power_get_inferred_state(struct acpi_device *device, int *state);
 int acpi_power_on_resources(struct acpi_device *device, int state);
 int acpi_power_transition(struct acpi_device *device, int state);
 
-int acpi_device_update_power(struct acpi_device *device, int *state_p);
-
 int acpi_wakeup_device_init(void);
 void acpi_early_processor_set_pdc(void);

include/acpi/acpi_bus.h +1 -0

index 7135fe3d6daa..4b0df6e6bd86 100644
@@ -363,6 +363,7 @@ int acpi_device_set_power(struct acpi_device *device, int state);
 int acpi_bus_init_power(struct acpi_device *device);
 int acpi_device_fix_up_power(struct acpi_device *device);
 int acpi_bus_update_power(acpi_handle handle, int *state_p);
+int acpi_device_update_power(struct acpi_device *device, int *state_p);
 bool acpi_bus_power_manageable(acpi_handle handle);
 
 #ifdef CONFIG_PM

Signature · 1 line

1.8.3.1

Aaron Lu

author

2013-12-03 08:29 UTC #4 lore raw

PATCH 4/4 ACPI / fan: convert to platform driver

In reply to Aaron Lu

Convert ACPI fan driver to a platform driver for the purpose of phasing out ACPI bus.

Signed-off-by Aaron Lu <[email protected]>

---
 drivers/acpi/acpi_platform.c |  3 +++
 drivers/acpi/fan.c           | 63 ++++++++++++++++++++------------------------
 2 files changed, 31 insertions(+), 35 deletions(-)

drivers/acpi/acpi_platform.c +3 -0

index dbfe49e5fd63..69e5317c7753 100644
@@ -37,6 +37,9 @@ static const struct acpi_device_id acpi_platform_device_ids[] = {
 	{ "INT33C8" },
 	{ "80860F28" },
 
+	/* ACPI fan device */
+	{ "PNP0C0B" },
+
 	{ }
 };

drivers/acpi/fan.c +28 ~~-35~~

132 lines not on this page · raw message

Signature · 1 line

1.8.3.1

Rafael J. Wysocki

2013-12-04 22:54 UTC #5 lore raw

In reply to Aaron Lu

Quoted 12 lines · Aaron Lu

This patchset converts ACPI fan driver to platform driver. Patch 1-3 are cleanups for existing fan driver and patch 4 does the convertion.
Tested on harris beach.
Apply on top of Rafael's linux-next branch.
Aaron Lu (4):
ACPI / fan: remove unused macro for debug
ACPI / fan: remove no need check for device pointer
ACPI / fan: use acpi_device_xxx_power instead of acpi_bus equivelant ACPI / fan: convert to platform driver

Quoted 6 lines

 drivers/acpi/acpi_platform.c |  3 ++
 drivers/acpi/device_pm.c     |  1 +
 drivers/acpi/fan.c           | 88 ++++++++++++++++----------------------------
 drivers/acpi/internal.h      |  2 -
 include/acpi/acpi_bus.h      |  1 +
 5 files changed, 36 insertions(+), 59 deletions(-)

Unfortunately, we need to postpone these conversions, because Matthew Garrett has problems with adding more entries to acpi_platform_device_ids[]. He seems to be concerned that that list will grow indefinitely and will become difficult to maintain eventually.

For this reason, he would prefer it if we did the following:
- Figure out the list of ACPI device IDs we need to create PNP devices for via ACPI PNP.
- Make ACPI PNP create PNP devices for these IDs only and make the ACPI core create platform devices for all "unassigned" ACPI device objects by default.
- Do the conversions at that point.

I'm slightly worried that we'll encounter ordering issues while doing that, but this is the only way forward I can see without going straight against the Matthew's objections, which I'd prefer to avoid.

Thanks,
Rafael

Rafael J. Wysocki

2013-12-04 22:56 UTC #6 lore raw

Cc +Matthew (sorry, omitted by mistake previously)

Quoted 44 lines · Rafael J. Wysocki

Quoted 12 lines · Aaron Lu
This patchset converts ACPI fan driver to platform driver. Patch 1-3 are cleanups for existing fan driver and patch 4 does the convertion.
Tested on harris beach.
Apply on top of Rafael's linux-next branch.
Aaron Lu (4):
ACPI / fan: remove unused macro for debug
ACPI / fan: remove no need check for device pointer
ACPI / fan: use acpi_device_xxx_power instead of acpi_bus equivelant ACPI / fan: convert to platform driver
Quoted 6 lines
 drivers/acpi/acpi_platform.c |  3 ++
 drivers/acpi/device_pm.c     |  1 +
 drivers/acpi/fan.c           | 88 ++++++++++++++++----------------------------
 drivers/acpi/internal.h      |  2 -
 include/acpi/acpi_bus.h      |  1 +
 5 files changed, 36 insertions(+), 59 deletions(-)
Unfortunately, we need to postpone these conversions, because Matthew Garrett has problems with adding more entries to acpi_platform_device_ids[]. He seems to be concerned that that list will grow indefinitely and will become difficult to maintain eventually.
For this reason, he would prefer it if we did the following:
- Figure out the list of ACPI device IDs we need to create PNP devices for via ACPI PNP.
- Make ACPI PNP create PNP devices for these IDs only and make the ACPI core create platform devices for all "unassigned" ACPI device objects by default.
- Do the conversions at that point.
I'm slightly worried that we'll encounter ordering issues while doing that, but this is the only way forward I can see without going straight against the Matthew's objections, which I'd prefer to avoid.
Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 2 lines

I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Zhang, Rui

2013-12-05 13:56 UTC #7 lore raw

Quoted 42 lines

-----Original Message-----
From: Rafael J. Wysocki [mailto:[email protected]]
Sent: Thursday, December 05, 2013 7:10 AM
To: Lu, Aaron
Cc: Zhang, Rui; [email protected]; linux- [email protected]; Lan, Tianyu; Brown, Len; Matthew Garrett
Subject: Re: [PATCH 0/4] Convert ACPI fan driver to platform driver
Importance: High
Cc +Matthew (sorry, omitted by mistake previously)
Quoted 2 lines · Rafael J. Wysocki
Quoted 1 line · Aaron Lu
This patchset converts ACPI fan driver to platform driver. Patch 1-
3
Quoted 1 line
Quoted 1 line
are cleanups for existing fan driver and patch 4 does the
convertion.
Quoted 8 lines
Quoted 8 lines
Tested on harris beach.
Apply on top of Rafael's linux-next branch.
Aaron Lu (4):
ACPI / fan: remove unused macro for debug
ACPI / fan: remove no need check for device pointer ACPI / fan: use acpi_device_xxx_power instead of acpi_bus
equivelant
Quoted 5 lines
Quoted 5 lines
ACPI / fan: convert to platform driver
 drivers/acpi/acpi_platform.c |  3 ++
 drivers/acpi/device_pm.c     |  1 +
 drivers/acpi/fan.c           | 88 ++++++++++++++++----------------
------------
Quoted 8 lines
Quoted 3 lines
 drivers/acpi/internal.h      |  2 -
 include/acpi/acpi_bus.h      |  1 +
 5 files changed, 36 insertions(+), 59 deletions(-)
Unfortunately, we need to postpone these conversions, because Matthew Garrett has problems with adding more entries to acpi_platform_device_ids[]. He seems to be concerned that that list will grow indefinitely and will become difficult to maintain

Quoted 6 lines

eventually.
Quoted 3 lines
For this reason, he would prefer it if we did the following:
- Figure out the list of ACPI device IDs we need to create PNP
devices for
Quoted 1 line
via ACPI PNP.

Agreed.
I'd also prefer a whitelist for pnpacpi instead of a backlist. And this is helpful for us to clean up the ACPI code in PNP bus, Including pnpacpi code and PNP drivers that actually bind to ACPI enumerated PNP devices.

Question is how to get this white list?
Can we just collect the pnp_device_id in all the PNP drivers as the first step?

Quoted 4 lines

Quoted 1 line
- Make ACPI PNP create PNP devices for these IDs only and make the
ACPI core create
Quoted 1 line
platform devices for all "unassigned" ACPI device objects by
default.

So the above statement means that an ACPI device will be enumerated to either PNP bus or platform bus, right?
Then I'm wondering how to handle the following cases,
1. an ACPI device may have both _HID and _CID, if its _CID is in the PNP whitelist, does this mean the device will always be enumerated to PNP bus?
2. is it possible to see the following ASL code?
Device (ABCDEFG) /* should be enumerated to platform bus */

   {
     Device (PNPABCD) /* should be enumerated to PNP bus */
     {
     }

}
If yes, it seems that we will introduce some ugly parent/child relationship in driver core?

Quoted 6 lines

Quoted 6 lines
- Do the conversions at that point.
I'm slightly worried that we'll encounter ordering issues while doing that, but this is the only way forward I can see without going straight against the Matthew's objections, which I'd prefer to avoid.

Maybe a stupid question, why can't we enumerate all ACPI devices to platform bus? Here are some of my thoughts,
1. enumerate all ACPI devices to platform bus, thus our ACPI -> platform bus conversion work can continue.
2. workout a white list for PNPACPI (by collecting all the pnp_device_ids)
3. cleanup the PNP drivers.
If a PNP driver is supposed to bind to ACPI enumerated PNP devices, convert it to a platform driver.
If a PNP driver can probe device enumerated from either PNPBIOS or PNPACPI, make it a dual-head driver that can probe both PNP and platform devices. If a PNP driver is supposed to bind to PNPBIOS enumerated PNP devices, leave it as it is.
Note: every time we have done the cleanup for a driver,
we can delete one entry in the PNPACPI whitelist in step 2.
4. after cleanup all the PNP drivers, the whitelist is empty
and we can remove all the PNPACPI code.

Thanks,
rui

Quoted 11 lines

Quoted 8 lines
Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Rafael J. Wysocki

2013-12-05 22:01 UTC #8 lore raw

Quoted 48 lines · Rui

Quoted 41 lines
-----Original Message-----
From: Rafael J. Wysocki [mailto:[email protected]]
Sent: Thursday, December 05, 2013 7:10 AM
To: Lu, Aaron
Cc: Zhang, Rui; [email protected]; linux- [email protected]; Lan, Tianyu; Brown, Len; Matthew Garrett
Subject: Re: [PATCH 0/4] Convert ACPI fan driver to platform driver
Importance: High
Cc +Matthew (sorry, omitted by mistake previously)
Quoted 2 lines · Rafael J. Wysocki
Quoted 1 line · Aaron Lu
This patchset converts ACPI fan driver to platform driver. Patch 1-
3
Quoted 1 line
Quoted 1 line
are cleanups for existing fan driver and patch 4 does the
convertion.
Quoted 8 lines
Quoted 8 lines
Tested on harris beach.
Apply on top of Rafael's linux-next branch.
Aaron Lu (4):
ACPI / fan: remove unused macro for debug
ACPI / fan: remove no need check for device pointer ACPI / fan: use acpi_device_xxx_power instead of acpi_bus
equivelant
Quoted 5 lines
Quoted 5 lines
ACPI / fan: convert to platform driver
 drivers/acpi/acpi_platform.c |  3 ++
 drivers/acpi/device_pm.c     |  1 +
 drivers/acpi/fan.c           | 88 ++++++++++++++++----------------
------------
Quoted 8 lines
Quoted 3 lines
 drivers/acpi/internal.h      |  2 -
 include/acpi/acpi_bus.h      |  1 +
 5 files changed, 36 insertions(+), 59 deletions(-)
Unfortunately, we need to postpone these conversions, because Matthew Garrett has problems with adding more entries to acpi_platform_device_ids[]. He seems to be concerned that that list will grow indefinitely and will become difficult to maintain
Quoted 6 lines
eventually.
Quoted 3 lines
For this reason, he would prefer it if we did the following:
- Figure out the list of ACPI device IDs we need to create PNP
devices for
Quoted 1 line
via ACPI PNP.

Quoted 8 lines

Agreed.
I'd also prefer a whitelist for pnpacpi instead of a backlist. And this is helpful for us to clean up the ACPI code in PNP bus, Including pnpacpi code and PNP drivers that actually bind to ACPI enumerated PNP devices.
Question is how to get this white list?
Can we just collect the pnp_device_id in all the PNP drivers as the first step?

Yes, we can.

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
- Make ACPI PNP create PNP devices for these IDs only and make the
ACPI core create
Quoted 1 line
platform devices for all "unassigned" ACPI device objects by
default.
So the above statement means that an ACPI device will be enumerated to either PNP bus or platform bus, right?

No. Processors, memory, containers etc will have their own device representations.

Quoted 4 lines

Then I'm wondering how to handle the following cases,
1. an ACPI device may have both _HID and _CID, if its _CID is in the PNP whitelist, does this mean the device will always be enumerated to PNP bus?

No. We need to check _HID first as per the spec.

Quoted 9 lines

2. is it possible to see the following ASL code? Device (ABCDEFG) /* should be enumerated to platform bus */
   {
     Device (PNPABCD) /* should be enumerated to PNP bus */
     {
     }
}
If yes, it seems that we will introduce some ugly parent/child relationship in driver core?

There's no rule that PNP devices cannot be children of platform devices as long as we handle resources assignments correctly.

Quoted 8 lines

Quoted 6 lines
Quoted 6 lines
- Do the conversions at that point.
I'm slightly worried that we'll encounter ordering issues while doing that, but this is the only way forward I can see without going straight against the Matthew's objections, which I'd prefer to avoid.
Maybe a stupid question, why can't we enumerate all ACPI devices to platform bus?

First of all, as I said, processors, memory modules and such will have their own device representations anyway.

Second, if the question why we need *both* platform and PNP, my answer is that we probably don't need them both in the long run, but we need to carry out the conversion "nicely". That is, keep PNP devices used by the existing PNP drivers for the time being and convert them gradually over time.

Quoted 3 lines

Here are some of my thoughts,
1. enumerate all ACPI devices to platform bus, thus our ACPI -> platform bus conversion work can continue.

The objection is against adding more entries to the list of IDs in acpi_platform.c. To avoid that we need to create platform devices for ACPI device nodes by default (that is, if they have no scan handlers attached etc.). And for that we need to stop creating PNP devices for them by default (which we do today). And for that we need a list of ACPI device IDs for which we need corresponding PNP devices, because the existing drivers use them (and we need to continue to create PNP devices for them until we convert the PNP drivers into platform drivers - if we can do that).

Quoted 4 lines

2. workout a white list for PNPACPI (by collecting all the pnp_device_ids)
3. cleanup the PNP drivers.
If a PNP driver is supposed to bind to ACPI enumerated PNP devices, convert it to a platform driver.

Quoted 8 lines

If a PNP driver can probe device enumerated from either PNPBIOS or PNPACPI, make it a dual-head driver that can probe both PNP and platform devices. If a PNP driver is supposed to bind to PNPBIOS enumerated PNP devices, leave it as it is.
Note: every time we have done the cleanup for a driver,
we can delete one entry in the PNPACPI whitelist in step 2.
4. after cleanup all the PNP drivers, the whitelist is empty
and we can remove all the PNPACPI code.

Yes, something like that.

Thanks,
Rafael

Aaron Lu

author

2013-12-05 07:44 UTC #9 lore raw

In reply to Rafael J. Wysocki

Quoted 20 lines · Rafael J. Wysocki

Quoted 12 lines · Aaron Lu
This patchset converts ACPI fan driver to platform driver. Patch 1-3 are cleanups for existing fan driver and patch 4 does the convertion.
Tested on harris beach.
Apply on top of Rafael's linux-next branch.
Aaron Lu (4):
ACPI / fan: remove unused macro for debug
ACPI / fan: remove no need check for device pointer
ACPI / fan: use acpi_device_xxx_power instead of acpi_bus equivelant ACPI / fan: convert to platform driver
Quoted 6 lines
 drivers/acpi/acpi_platform.c |  3 ++
 drivers/acpi/device_pm.c     |  1 +
 drivers/acpi/fan.c           | 88 ++++++++++++++++----------------------------
 drivers/acpi/internal.h      |  2 -
 include/acpi/acpi_bus.h      |  1 +
 5 files changed, 36 insertions(+), 59 deletions(-)

Quoted 8 lines

Unfortunately, we need to postpone these conversions, because Matthew Garrett has problems with adding more entries to acpi_platform_device_ids[]. He seems to be concerned that that list will grow indefinitely and will become difficult to maintain eventually.
For this reason, he would prefer it if we did the following:
- Figure out the list of ACPI device IDs we need to create PNP devices for via ACPI PNP.

I'm not sure how to tell this, is it that as long as the ACPI node has a PNPxxxx ID we will need to create a PNP device for it? And in this case, do we only check the _HID or both _HID and _CID?

Quoted 2 lines

- Make ACPI PNP create PNP devices for these IDs only and make the ACPI core create platform devices for all "unassigned" ACPI device objects by default.

Does "unassigned" mean (all ACPI devices) - (ACPI devices that have a PNP device created already)?

Thanks,
Aaron

Quoted 9 lines

- Do the conversions at that point.
I'm slightly worried that we'll encounter ordering issues while doing that, but this is the only way forward I can see without going straight against the Matthew's objections, which I'd prefer to avoid.
Thanks,
Rafael

Rafael J. Wysocki

2013-12-05 21:49 UTC #10 lore raw

Quoted 26 lines · Aaron Lu

Quoted 25 lines · Rafael J. Wysocki
Quoted 12 lines · Aaron Lu
This patchset converts ACPI fan driver to platform driver. Patch 1-3 are cleanups for existing fan driver and patch 4 does the convertion.
Tested on harris beach.
Apply on top of Rafael's linux-next branch.
Aaron Lu (4):
ACPI / fan: remove unused macro for debug
ACPI / fan: remove no need check for device pointer
ACPI / fan: use acpi_device_xxx_power instead of acpi_bus equivelant ACPI / fan: convert to platform driver
Quoted 6 lines
 drivers/acpi/acpi_platform.c |  3 ++
 drivers/acpi/device_pm.c     |  1 +
 drivers/acpi/fan.c           | 88 ++++++++++++++++----------------------------
 drivers/acpi/internal.h      |  2 -
 include/acpi/acpi_bus.h      |  1 +
 5 files changed, 36 insertions(+), 59 deletions(-)
Unfortunately, we need to postpone these conversions, because Matthew Garrett has problems with adding more entries to acpi_platform_device_ids[]. He seems to be concerned that that list will grow indefinitely and will become difficult to maintain eventually.

Quoted 7 lines

Quoted 3 lines
For this reason, he would prefer it if we did the following:
- Figure out the list of ACPI device IDs we need to create PNP devices for via ACPI PNP.
I'm not sure how to tell this, is it that as long as the ACPI node has a PNPxxxx ID we will need to create a PNP device for it? And in this case, do we only check the _HID or both _HID and _CID?

Just check the existing PNP drivers and see what device IDs they use.

Quoted 5 lines

Quoted 2 lines
- Make ACPI PNP create PNP devices for these IDs only and make the ACPI core create platform devices for all "unassigned" ACPI device objects by default.
Does "unassigned" mean (all ACPI devices) - (ACPI devices that have a PNP device created already)?

(all ACPI devices nodes) - (nodes taken care of by scan handlers + PNP) (and I *think* we can turn the PNP ACPI driver into a scan handler).

Thanks,
Rafael