PNP support for i8042 driver
PATCH 1/1 PNP support for i8042 driver
Hi,
this patch add PNP support for the i8042 driver in 2.6.10-rc1-mm5. Acpi is try before the pnp driver so if you don't disable ACPI or apply others pnpacpi patches, it won't change anything.
Please review it and apply if possible
thanks,
Matthieu CASTET
drivers/input/serio/i8042.c renamed from drivers/input/serio/i8042.c.old +6 -0
@@ -61,6 +61,12 @@MODULE_PARM_DESC (noacpi,"Do not use ACPI to detect controller settings" );#endif +#ifdef CONFIG_PNP +static int i8042_nopnp; +module_param_named (nopnp, i8042_nopnp, bool,0 ); +MODULE_PARM_DESC (nopnp,"Do not use PNP to detect controller settings" ); +#endif +#define DEBUG #ifdef DEBUG static int i8042_debug;
drivers/input/serio/i8042-x86ia64io.h renamed from drivers/input/serio/i8042-x86ia64io.h.old +122 -2
144 lines not on this page · raw message
Quoted 5 lines · matthieu castet
Hi,
this patch add PNP support for the i8042 driver in 2.6.10-rc1-mm5. Acpi is try before the pnp driver so if you don't disable ACPI or apply others pnpacpi patches, it won't change anything.
Quoted 8 lines
Please review it and apply if possible
thanks,
Matthieu CASTET
Signed-off-by Matthieu Castet <[email protected]>
Hi,
Do we really need to keep those drivers loaded - i8042 will not be hotplugged and ports are reserved anyway. We are only interested in presence of the keyboard and mouse ports. Can we unregister the drivers (both ACPI and PNP) right after registering and mark all that stuff as __init/__initdata as in the patch below? I also adjusted init logic so ACPI/PNP can be enabled/disabled independently of each other.
Signature · 275 lines
Dmitry
===== drivers/input/serio/i8042.c 1.72 vs edited =====
--- 1.72/drivers/input/serio/i8042.c 2004-11-12 00:25:04 -05:00
+++ edited/drivers/input/serio/i8042.c 2004-11-13 21:43:54 -05:00
@@ -65,6 +65,12 @@ module_param_named(noacpi, i8042_noacpi,
MODULE_PARM_DESC(noacpi, "Do not use ACPI to detect controller settings");
#endif
+#ifdef CONFIG_PNP
+static int i8042_nopnp;
+module_param_named(nopnp, i8042_nopnp, bool, 0);
+MODULE_PARM_DESC(nopnp, "Do not use PNP to detect controller settings");
+#endif
+
#define DEBUG
#ifdef DEBUG
static int i8042_debug;
===== drivers/input/serio/i8042-x86ia64io.h 1.2 vs edited =====
--- 1.2/drivers/input/serio/i8042-x86ia64io.h 2004-10-19 05:58:22 -05:00
+++ edited/drivers/input/serio/i8042-x86ia64io.h 2004-11-13 23:44:32 -05:00
@@ -88,6 +88,103 @@ static struct dmi_system_id __initdata i
};
#endif
+#ifdef CONFIG_PNP
+#include <linux/pnp.h>
+
+static int __init i8042_pnp_kbd_probe(struct pnp_dev *dev, const struct pnp_device_id *did)
+{
+ if (pnp_port_valid(dev, 0) && pnp_port_len(dev, 0) == 1)
+ i8042_data_reg = pnp_port_start(dev, 0);
+ else
+ printk(KERN_WARNING "PNP: [%s] has no data port; default is 0x%x\n",
+ pnp_dev_name(dev), i8042_data_reg);
+
+ if (pnp_port_valid(dev, 1) && pnp_port_len(dev, 1) == 1)
+ i8042_command_reg = pnp_port_start(dev, 1);
+ else
+ printk(KERN_WARNING "PNP: [%s] has no command port; default is 0x%x\n",
+ pnp_dev_name(dev), i8042_command_reg);
+
+ if (pnp_irq_valid(dev, 0))
+ i8042_kbd_irq = pnp_irq(dev, 0);
+ else
+ printk(KERN_WARNING "PNP: [%s] has no IRQ; default is %d\n",
+ pnp_dev_name(dev), i8042_kbd_irq);
+
+ printk("PNP: %s [%s] at I/O 0x%x, 0x%x, irq %d\n",
+ "PS/2 Keyboard Controller", pnp_dev_name(dev),
+ i8042_data_reg, i8042_command_reg, i8042_kbd_irq);
+
+ return 0;
+}
+
+static int __init i8042_pnp_aux_probe(struct pnp_dev *dev, const struct pnp_device_id *did)
+{
+ if (pnp_irq_valid(dev, 0))
+ i8042_aux_irq = pnp_irq(dev, 0);
+ else
+ printk(KERN_WARNING "PNP: [%s] has no IRQ; default is %d\n",
+ pnp_dev_name(dev), i8042_aux_irq);
+
+ printk("PNP: %s [%s] at irq %d\n",
+ "PS/2 Mouse Controller", pnp_dev_name(dev), i8042_aux_irq);
+
+ return 0;
+}
+
+static struct pnp_device_id __initdata pnp_kbd_devids[] = {
+ { .id = "PNP0303", .driver_data = 0 },
+ { .id = "PNP030b", .driver_data = 0 },
+ { .id = "", },
+};
+
+static struct pnp_driver __initdata i8042_pnp_kbd_driver = {
+ .name = "i8042 kbd",
+ .id_table = pnp_kbd_devids,
+ .probe = i8042_pnp_kbd_probe,
+};
+
+static struct pnp_device_id __initdata pnp_aux_devids[] = {
+ { .id = "PNP0f13", .driver_data = 0 },
+ { .id = "SYN0801", .driver_data = 0 },
+ { .id = "", },
+};
+
+static struct pnp_driver __initdata i8042_pnp_aux_driver = {
+ .name = "i8042 aux",
+ .id_table = pnp_aux_devids,
+ .probe = i8042_pnp_aux_probe,
+};
+
+static int __init i8042_pnp_init(void)
+{
+ int result;
+
+ result = pnp_register_driver(&i8042_pnp_kbd_driver);
+ if (result < 0)
+ return result;
+ /*
+ * Unregister the driver right away as we need it only to
+ * make sure that the keyboard port is present. We do not
+ * expect i8042 to be hotplugged nor some other device
+ * claiming its resources.
+ */
+ pnp_unregister_driver(&i8042_pnp_kbd_driver);
+
+ if (result == 0)
+ return -ENODEV;
+
+ result = pnp_register_driver(&i8042_pnp_aux_driver);
+ if (result >= 0)
+ pnp_unregister_driver(&i8042_pnp_aux_driver);
+
+ if (result <= 0)
+ i8042_noaux = 1;
+
+ return 0;
+}
+#endif
+
#ifdef CONFIG_ACPI
#include <linux/acpi.h>
#include <acpi/acpi_bus.h>
@@ -98,10 +195,7 @@ struct i8042_acpi_resources {
unsigned int irq;
};
-static int i8042_acpi_kbd_registered;
-static int i8042_acpi_aux_registered;
-
-static acpi_status i8042_acpi_parse_resource(struct acpi_resource *res, void *data)
+static acpi_status __init i8042_acpi_parse_resource(struct acpi_resource *res, void *data)
{
struct i8042_acpi_resources *i8042_res = data;
struct acpi_resource_io *io;
@@ -151,7 +245,7 @@ static acpi_status i8042_acpi_parse_reso
return AE_OK;
}
-static int i8042_acpi_kbd_add(struct acpi_device *device)
+static int __init i8042_acpi_kbd_add(struct acpi_device *device)
{
struct i8042_acpi_resources kbd_res;
acpi_status status;
@@ -189,7 +283,7 @@ static int i8042_acpi_kbd_add(struct acp
return 0;
}
-static int i8042_acpi_aux_add(struct acpi_device *device)
+static int __init i8042_acpi_aux_add(struct acpi_device *device)
{
struct i8042_acpi_resources aux_res;
acpi_status status;
@@ -214,7 +308,7 @@ static int i8042_acpi_aux_add(struct acp
return 0;
}
-static struct acpi_driver i8042_acpi_kbd_driver = {
+static struct acpi_driver __initdata i8042_acpi_kbd_driver = {
.name = "i8042",
.ids = "PNP0303,PNP030B",
.ops = {
@@ -222,7 +316,7 @@ static struct acpi_driver i8042_acpi_kbd
},
};
-static struct acpi_driver i8042_acpi_aux_driver = {
+static struct acpi_driver __initdata i8042_acpi_aux_driver = {
.name = "i8042",
.ids = "PNP0F13,SYN0801",
.ops = {
@@ -230,7 +324,7 @@ static struct acpi_driver i8042_acpi_aux
},
};
-static int i8042_acpi_init(void)
+static int __init i8042_acpi_init(void)
{
int result;
@@ -243,33 +337,25 @@ static int i8042_acpi_init(void)
if (result < 0)
return result;
- if (result == 0) {
- acpi_bus_unregister_driver(&i8042_acpi_kbd_driver);
+ acpi_bus_unregister_driver(&i8042_acpi_kbd_driver);
+
+ if (result == 0)
return -ENODEV;
- }
- i8042_acpi_kbd_registered = 1;
result = acpi_bus_register_driver(&i8042_acpi_aux_driver);
if (result >= 0)
- i8042_acpi_aux_registered = 1;
- if (result == 0)
+ acpi_bus_unregister_driver(&i8042_acpi_aux_driver);
+
+ if (result <= 0)
i8042_noaux = 1;
return 0;
}
-
-static void i8042_acpi_exit(void)
-{
- if (i8042_acpi_kbd_registered)
- acpi_bus_unregister_driver(&i8042_acpi_kbd_driver);
-
- if (i8042_acpi_aux_registered)
- acpi_bus_unregister_driver(&i8042_acpi_aux_driver);
-}
#endif
-static inline int i8042_platform_init(void)
+static int __init i8042_platform_init(void)
{
+ int probe_failed = 0;
/*
* On ix86 platforms touching the i8042 data register region can do really
* bad things. Because of this the region is always reserved on ix86 boxes.
@@ -278,14 +364,6 @@ static inline int i8042_platform_init(vo
* return -1;
*/
- i8042_kbd_irq = I8042_MAP_IRQ(1);
- i8042_aux_irq = I8042_MAP_IRQ(12);
-
-#ifdef CONFIG_ACPI
- if (i8042_acpi_init())
- return -1;
-#endif
-
#if defined(__ia64__)
i8042_reset = 1;
#endif
@@ -295,14 +373,31 @@ static inline int i8042_platform_init(vo
i8042_noloop = 1;
#endif
- return 0;
+ i8042_kbd_irq = I8042_MAP_IRQ(1);
+ i8042_aux_irq = I8042_MAP_IRQ(12);
+
+#ifdef CONFIG_ACPI
+ if (acpi_disabled || i8042_noacpi)
+ printk("i8042: ACPI detection disabled\n");
+ else if (i8042_acpi_init() < 0)
+ probe_failed = 1;
+ else
+ return 0;
+#endif
+#ifdef CONFIG_PNP
+ if (i8042_nopnp)
+ printk("i8042: PNP detection disabled\n");
+ else if (i8042_pnp_init() < 0)
+ probe_failed = 1;
+ else
+ return 0;
+#endif
+
+ return probe_failed ? -1 : 0;
}
static inline void i8042_platform_exit(void)
{
-#ifdef CONFIG_ACPI
- i8042_acpi_exit();
-#endif
}
#endif /* _I8042_X86IA64IO_H */Quoted 7 lines · Dmitry Torokhov
Quoted 5 lines · matthieu castet
Hi,
this patch add PNP support for the i8042 driver in 2.6.10-rc1-mm5. Acpi is try before the pnp driver so if you don't disable ACPI or apply others pnpacpi patches, it won't change anything.Please review it and apply if possible
Quoted 9 lines
Quoted 6 lines
thanks,
Matthieu CASTET
Signed-off-by Matthieu Castet <[email protected]>Hi,
Hi,
Quoted 5 lines
Do we really need to keep those drivers loaded - i8042 will not be hotplugged and ports are reserved anyway. We are only interested in presence of the keyboard and mouse ports. Can we unregister the drivers (both ACPI and PNP) right after registering and mark all that stuff as __init/__initdata as in the patch below?
It is better to keep pnp driver loaded because when it unload, the resources will be disabled, so for the motherboards that allow it the irq won't work anymore, and so the keyboard and mouse won't work... Also it avoid the user to do
"echo disable > /sys/bus/pnp/xx\:xx/resources". Actually, it disables the resources for the mouse even if the driver is using the resource...
Quoted 3 lines
I also adjusted init logic so ACPI/PNP can be enabled/disabled independently of each other.
No problem as long as :
-if there is no acpi support, pnp can be used
-if acpi driver detect nothing, and there is pnp support, pnp driver will be tried before returning an error (it is important because, in the future pnpacpi will "lock" the acpi device, so the acpi driver will find nothing even if there are devices)
Regards,
Matthieu
Dmitry Torokhov
maintainerQuoted 22 lines · matthieu castet
Quoted 9 lines · Dmitry Torokhov
Quoted 7 lines · matthieu castet
Hi,
this patch add PNP support for the i8042 driver in 2.6.10-rc1-mm5. Acpi is try before the pnp driver so if you don't disable ACPI or apply others pnpacpi patches, it won't change anything.Please review it and apply if possible
Quoted 9 lines
Quoted 6 lines
thanks,
Matthieu CASTET
Signed-off-by Matthieu Castet <[email protected]>Hi,
Hi,
Quoted 8 lines
Quoted 5 lines
Do we really need to keep those drivers loaded - i8042 will not be hotplugged and ports are reserved anyway. We are only interested in presence of the keyboard and mouse ports. Can we unregister the drivers (both ACPI and PNP) right after registering and mark all that stuff as __init/__initdata as in the patch below?
It is better to keep pnp driver loaded because when it unload, the resources will be disabled, so for the motherboards that allow it the irq won't work anymore, and so the keyboard and mouse won't work...
Is it possible to leave the device in enabled state or enable device after unloading the driver with PNP? All we need from PNP layer for i8042 is to verify presence of the KBC, we don't need resource management.The ports range is already marked as reserved, IRQ will be requested if needed...
Signature · 1 line
Dmitry
Quoted 34 lines · Dmitry Torokhov
Quoted 31 lines · matthieu castet
Quoted 10 lines · Dmitry Torokhov
Quoted 7 lines · matthieu castet
Hi,
this patch add PNP support for the i8042 driver in 2.6.10-rc1-mm5. Acpi is try before the pnp driver so if you don't disable ACPI or apply others pnpacpi patches, it won't change anything.Please review it and apply if possible
Quoted 8 lines
Quoted 6 lines
thanks,
Matthieu CASTET
Signed-off-by Matthieu Castet <[email protected]>Hi,
Hi,
Quoted 5 lines
Do we really need to keep those drivers loaded - i8042 will not be hotplugged and ports are reserved anyway. We are only interested in presence of the keyboard and mouse ports. Can we unregister the drivers (both ACPI and PNP) right after registering and mark all that stuff as __init/__initdata as in the patch below?
Quoted 7 lines
Quoted 3 lines
It is better to keep pnp driver loaded because when it unload, the resources will be disabled, so for the motherboards that allow it the irq won't work anymore, and so the keyboard and mouse won't work...
Is it possible to leave the device in enabled state or enable device after unloading the driver with PNP?
Yes you could do a very ugly hack : set pnp_can_disable(dev) to 0 before
unregister. With that the device won't be disabled (no resource
desalocation), but the device will be mark as not active in pnp layer.
Quoted 5 lines
All we need from PNP layer
for i8042 is to verify presence of the KBC, we don't need resource management.The ports range is already marked as reserved, IRQ will be requested if needed...
I don't agree at all :
- the pci layer allow you to find the device like pnp layer, then you register resource with request_region or equivalent. Do we need to do the same for all pci driver ?
- actually the resources are registered in the kernel, but not in the bios, why some strange bios can allow to use irq 12 to an other device if it isn't used ?
- Do you save lot's of memory with __init/__initdata ? The pnp code is quite small.
Matthieu
Dmitry Torokhov
maintainerQuoted 7 lines · matthieu castet
Quoted 2 lines · Dmitry Torokhov
Is it possible to leave the device in enabled state or enable device after unloading the driver with PNP?
Yes you could do a very ugly hack : set
pnp_can_disable(dev)to 0 before
unregister. With that the device won't be disabled (no resource
desalocation), but the device will be mark as not active in pnp layer.
I'd like to release resoures al well (interrupts only really, as ports are always reserved by the system even before PNP is initialized).
Quoted 9 lines
Quoted 5 lines
All we need from PNP layer
for i8042 is to verify presence of the KBC, we don't need resource management.The ports range is already marked as reserved, IRQ will be requested if needed...I don't agree at all :
- the pci layer allow you to find the device like pnp layer, then you register resource withrequest_regionor equivalent. Do we need to do the same for all pci drivers?
While PCI devices can be very flexible i8042 is extremely rigid. Its resources are pretty much fixed and will not move. Its IO port region is reserved by the kernel right off the bat and is not available to anyone including PNP subsystem to ensure that nothing will touch it or bad things might happen.
Quoted 3 lines
- actually the resources are registered in the kernel, but not in the bios, why some strange bios can allow to use irq 12 to an other device if it isn't used ?
I think you need to make an effort to make a PCI device use IRQ12 but the idea is that if you don't have a mouse attached (but you do have i8042) and you are short on free interrupts and your HW can use IRQ12 for some other stuff let it have it. That is the reqson why i8042 requests IRQ only when corresponding port is open. No mouse - IRQ is free.
Quoted 3 lines
- Do you save lot's of memory with __init/__initdata ? The pnp code is quite small.
Well it is not needed one i8042 has been initialized at all so why keep it? Even if it saves 1K it is good enough.
Signature · 1 line
Dmitry
Hi,
Quoted 11 lines · Dmitry Torokhov
On Mon, 15 Nov 2004 20:51:20 +0100, matthieu castet
Quoted 4 lines
Yes you could do a very ugly hack : set
pnp_can_disable(dev)to 0 before unregister. With that the device won't be disabled (no resource desalocation), but the device will be mark as not active in pnp layer.I'd like to release resoures al well (interrupts only really, as ports are always reserved by the system even before PNP is initialized).
that's the case : well I said no resource desalocation it was for the motherboard. We haven't other choice : if we disable them, they won't be unusable until an other driver ask the motherboard to activate them. And for the kbd and mouse these resources are always activated on motherboard : the systems that don't support dynamic allocation should be able to use them, so with "pnp_can_disable(dev) = 0", we let the resources like they were before. Note also if there weren't activate on boot, the acpi driver couldn't find them, because it don't know how to allocate a resource...
Quoted 19 lines
Quoted 5 lines
I don't agree at all :
- the pci layer allow you to find the device like pnp layer, then you register resource withrequest_regionor equivalent. Do we need to do the same for all pci drivers?While PCI devices can be very flexible i8042 is extremely rigid. Its resources are pretty much fixed and will not move. Its IO port region is reserved by the kernel right off the bat and is not available to anyone including PNP subsystem to ensure that nothing will touch it or bad things might happen.
Quoted 3 lines
- actually the resources are registered in the kernel, but not in the bios, why some strange bios can allow to use irq 12 to an other device if it isn't used ?
Quoted 7 lines
I think you need to make an effort to make a PCI device use IRQ12 but the idea is that if you don't have a mouse attached (but you do have i8042) and you are short on free interrupts and your HW can use IRQ12 for some other stuff let it have it. That is the reqson why i8042 requests IRQ only when corresponding port is open. No mouse - IRQ is free.
And what happen if you use irq12 for an other stuff and you plug your mouse and try to use it. The motherboard hasn't desalocated the irq12 for mouse, so there will be a big conflict...
Quoted 9 lines
Quoted 4 lines
- Do you save lot's of memory with __init/__initdata ? The pnp code is quite small.
Well it is not needed one i8042 has been initialized at all so why keep it? Even if it saves 1K it is good enough.
well that less that 400 between i8042 with pnp support and without.
And why not "__init" init code, "__devinit" probe, "__devinitdata" array, "__exit" exit ?
And if you are sure that hotpluging is not possible for i8042, then you could even __init the probe and array since there will be not used again, but that more ugly than the pnp_can_disable(dev) hack ;)
regards,
Matthieu
Quoted 5 lines · matthieu castet
Hi,
Quoted 2 lines
Quoted 9 lines
Quoted 7 lines
I think you need to make an effort to make a PCI device use IRQ12 but the idea is that if you don't have a mouse attached (but you do have i8042) and you are short on free interrupts and your HW can use IRQ12 for some other stuff let it have it. That is the reqson why i8042 requests IRQ only when corresponding port is open. No mouse - IRQ is free.
And what happen if you use irq12 for an other stuff and you plug your mouse and try to use it. The motherboard hasn't desalocated the irq12 for mouse, so there will be a big conflict...
In that case a boot param solve the problem, it says you could
desactivate the resource in the bios/acpi for the mouse.
So even if a mouse is plug, the other driver shouldn't receive others interrupts (I don't know exactly what happen if for example irq12 is used for pci and mouse in acpi config and if there are other protection again that case...)
Matthieu
Quoted 17 lines · Dmitry Torokhov
Quoted 5 lines · matthieu castet
Hi,
this patch add PNP support for the i8042 driver in 2.6.10-rc1-mm5. Acpi is try before the pnp driver so if you don't disable ACPI or apply others pnpacpi patches, it won't change anything.Quoted 8 lines
Please review it and apply if possible
thanks,
Matthieu CASTET
Signed-off-by Matthieu Castet <[email protected]>Hi,
Quoted 10 lines
Do we really need to keep those drivers loaded - i8042 will not be hotplugged and ports are reserved anyway. We are only interested in presence of the keyboard and mouse ports. Can we unregister the drivers (both ACPI and PNP) right after registering and mark all that stuff as __init/__initdata as in the patch below? I also adjusted init logic so ACPI/PNP can be enabled/disabled independently of each other.
Signature · 1 line
Dmitry
As a general convention, I think we do. This should apply to every bus and driver. Every hardware component should have a driver bound to it. Otherwise, we can't take full advantage of sysfs. We really need a driver to link the physical device to logical "class" abstractions . Futhermore, as we extend the functionality of the driver core (ex. manual binding and unbinding) _init will cause many headaches. Also, unloading the driver is not good for power management. This case may not apply to every device, but in most cases we need to have a driver loaded before suspending a piece of hardware, as only that driver can be sure of how to handle the device correctly.
It may not always be the most efficient solution for legacy hardware, but, at least, if we require legacy drivers to behave like any other driver, then we can ensure a minimum level of functionalily and flexability. This set of standards will help ensure that legacy hardware can coexist nicely with more modern solutions. That's really one of the main advantages of a layered design like the driver model.
Thanks,
Adam
Quoted 4 lines · matthieu castet
Hi,
Quoted 1 line · Dmitry Torokhov
On Mon, 15 Nov 2004 20:51:20 +0100, matthieu castet
Quoted 9 lines
Quoted 7 lines
Quoted 4 lines
Yes you could do a very ugly hack : set
pnp_can_disable(dev)to 0 before unregister. With that the device won't be disabled (no resource desalocation), but the device will be mark as not active in pnp layer.I'd like to release resoures al well (interrupts only really, as
ports are
Quoted 1 line
always reserved by the system even before PNP is initialized).
They shouldn't be. PnP detection should occur before the system assumes the location of a device. I realize that it can be difficult given the current state of many drivers. Still, I think assuming information about a device, especially if you consider how easy it is to get from ACPI etc., can be potentially dangerous.
Quoted 10 lines
Quoted 7 lines
I think you need to make an effort to make a PCI device use IRQ12 but the idea is that if you don't have a mouse attached (but you do have i8042) and you are short on free interrupts and your HW can use IRQ12 for some other stuff let it have it. That is the reqson why i8042 requests IRQ only when corresponding port is open. No mouse - IRQ is free.
And what happen if you use irq12 for an other stuff and you plug your mouse and try to use it. The motherboard hasn't desalocated the irq12 for mouse, so there will be a big conflict...
I agree. Disabling the device is fine, but we _really_ should disable the device with the BIOS before assuming a resource is free.
Thanks,
Adam
Quoted 3 lines · Adam Belay
As a general convention, I think we do. This should apply to every bus and driver. Every hardware component should have a driver bound to it.
I agree too, for the very reasons you list.
</aol>
greg k-h
Quoted 39 lines · Adam Belay
Quoted 17 lines · Dmitry Torokhov
Quoted 5 lines · matthieu castet
Hi,
this patch add PNP support for the i8042 driver in 2.6.10-rc1-mm5. Acpi is try before the pnp driver so if you don't disable ACPI or apply others pnpacpi patches, it won't change anything.Quoted 8 lines
Please review it and apply if possible
thanks,
Matthieu CASTET
Signed-off-by Matthieu Castet <[email protected]>Hi,
Quoted 10 lines
Do we really need to keep those drivers loaded - i8042 will not be hotplugged and ports are reserved anyway. We are only interested in presence of the keyboard and mouse ports. Can we unregister the drivers (both ACPI and PNP) right after registering and mark all that stuff as __init/__initdata as in the patch below? I also adjusted init logic so ACPI/PNP can be enabled/disabled independently of each other.
Signature · 1 line
DmitryAs a general convention, I think we do. This should apply to every bus and driver. Every hardware component should have a driver bound to it. Otherwise, we can't take full advantage of sysfs. We really need a driver to link the physical device to logical "class" abstractions . Futhermore, as we extend the functionality of the driver core (ex. manual binding and unbinding)
_initwill cause many headaches. Also, unloading the driver is not good for power management. This case may not apply to every device, but in most cases we need to have a driver loaded before suspending a piece of hardware, as only that driver can be sure of how to handle the device correctly.
Quoted 7 lines
It may not always be the most efficient solution for legacy hardware, but, at least, if we require legacy drivers to behave like any other driver, then we can ensure a minimum level of functionalily and flexability. This set of standards will help ensure that legacy hardware can coexist nicely with more modern solutions. That's really one of the main advantages of a layered design like the driver model.
Adam,
I agree with your point that every device in the system should have a driver attached. And i8042 does have one bound to it. It is i8042 platform driver that does power management and ensures proper integration into driver model.
There is no need to keep secondary "drivers" around, their sole purpose is to provide information about avalilable resources. It would be ok if the code was shared among several devices on a bus but for most (all?) legacy devices it has to be programmed explicitely and will not be reused.
Also i8042 should not rely on either ACPI or PNP simply because the driver/ chip works on boxes other than x86/ia64 so we can't make ACPI or PNP drivers "main" ones.
As far as binding/rebinding goes I guess sysdevs and platform devices will just disable this functionality.
--
Dmitry
Quoted 11 lines · Dmitry Torokhov
Adam,
I agree with your point that every device in the system should have a driver attached. And i8042 does have one bound to it. It is i8042 platform driver that does power management and ensures proper integration into driver model.
There is no need to keep secondary "drivers" around, their sole purpose is to provide information about avalilable resources. It would be ok if the code was shared among several devices on a bus but for most (all?) legacy devices it has to be programmed explicitely and will not be reused.
Platform drivers are secondary drivers. They're crude hacks used to get a minimal device into the driver model. They're necessary because ISA devices cannot be detected like other types of devices. Why not use a system with actual complete bus functionality like ACPI? If ACPI is available, then there should be no need to create a platform device.
Quoted 4 lines
Also i8042 should not rely on either ACPI or PNP simply because the driver/ chip works on boxes other than x86/ia64 so we can't make ACPI or PNP drivers "main" ones.
No, but they should take priority when they are available. Don't forget that there is also Open Firmware, which has similar functionalility to ACPI if I understand correctly.
Quoted 3 lines
As far as binding/rebinding goes I guess sysdevs and platform devices will just disable this functionality.
Exactly, they have limits because they are not real devices. ACPI devices are real devices. They have resource management capabilities, power dependencies, physical parents, and other features found in buses like PCI.
Thanks,
Adam
Quoted 11 lines · matthieu castet
Hi,
Quoted 8 lines · Dmitry Torokhov
I think you need to make an effort to make a PCI device use IRQ12 but the idea is that if you don't have a mouse attached (but you do have i8042) and you are short on free interrupts and your HW can use IRQ12 for some other stuff let it have it. That is the reqson why i8042 requests IRQ only when corresponding port is open. No mouse - IRQ is free.
Quoted 4 lines
And what happen if you use irq12 for an other stuff and you plug your mouse and try to use it. The motherboard hasn't desalocated the irq12 for mouse, so there will be a big conflict...
Actually no. The driver will not enable the AUX port unless IRQ12 (or whatever ACPI/PNP IRQ reported) is available and therefore mouse movement will not generate any interrupts.
Anyway, I'll shut up if you promise to merge ACPI and PNP together so i8042 does not have 2 nearly identical implementations in i8042-x86ia64.h
Signature · 1 line
Dmitry
Quoted 6 lines · matthieu castet
Hi,
this patch add PNP support for the i8042 driver in 2.6.10-rc1-mm5. Acpi is try before the pnp driver so if you don't disable ACPI or apply others pnpacpi patches, it won't change anything.Please review it and apply if possible
Ok, my thoughts on this:
It's OK to keep the device allocated to this driver via the PnP subsystem, and not bother with releasing the code via __initcall.
I agree that if there is a way to enumerate the device, (like PnP, ACPI or OpenFirmware), we should use that instead of probing and using a platform device for the controller.
I think that we should drop the ACPI support from i8042, in favor of pnpacpi, because PnP is more generic and if the keyboard device was listed in PnPBIOS instead of ACPI, it'll still work.
Signature · 2 lines
Vojtech Pavlik SuSE Labs, SuSE CR
Hi,
Quoted 14 lines · Vojtech Pavlik
Quoted 4 lines · matthieu castet
Hi,
this patch add PNP support for the i8042 driver in 2.6.10-rc1-mm5. Acpi is try before the pnp driver so if you don't disable ACPI or apply others pnpacpi patches, it won't change anything.Please review it and apply if possible
Ok, my thoughts on this:
It's OK to keep the device allocated to this driver via the PnP subsystem, and not bother with releasing the code via
__initcall.
Quoted 9 lines
I agree that if there is a way to enumerate the device, (like PnP, ACPI or OpenFirmware), we should use that instead of probing and using a platform device for the controller.
I think that we should drop the ACPI support from i8042, in favor of pnpacpi, because PnP is more generic and if the keyboard device was listed in PnPBIOS instead of ACPI, it'll still work.
Any news about this ?
Matthieu CASTET
Quoted 23 lines · matthieu castet
Hi,
Quoted 20 lines · Vojtech Pavlik
Quoted 6 lines · matthieu castet
Hi,
this patch add PNP support for the i8042 driver in 2.6.10-rc1-mm5. Acpi is try before the pnp driver so if you don't disable ACPI or apply others pnpacpi patches, it won't change anything.Please review it and apply if possible
Ok, my thoughts on this:
It's OK to keep the device allocated to this driver via the PnP
subsystem, and not bother with releasing the code via
__initcall.I agree that if there is a way to enumerate the device, (like PnP, ACPI or OpenFirmware), we should use that instead of probing and using a platform device for the controller.
Quoted 6 lines
Quoted 5 lines
I think that we should drop the ACPI support from i8042, in favor of pnpacpi, because PnP is more generic and if the keyboard device was listed in PnPBIOS instead of ACPI, it'll still work.
Any news about this ?
Sort of fell off my radar, can you resend?
Signature · 2 lines
Vojtech Pavlik SuSE Labs, SuSE CR
Hi,
Quoted 31 lines · Vojtech Pavlik
Quoted 29 lines · matthieu castet
Hi,
Quoted 24 lines · Vojtech Pavlik
Quoted 4 lines · matthieu castet
Hi,
this patch add PNP support for the i8042 driver in 2.6.10-rc1-mm5. Acpi is try before the pnp driver so if you don't disable ACPI or apply others pnpacpi patches, it won't change anything.Please review it and apply if possible
Ok, my thoughts on this:
It's OK to keep the device allocated to this driver via the PnP
subsystem, and not bother with releasing the code via
__initcall.I agree that if there is a way to enumerate the device, (like PnP, ACPI or OpenFirmware), we should use that instead of probing and using a platform device for the controller.
I think that we should drop the ACPI support from i8042, in favor of pnpacpi, because PnP is more generic and if the keyboard device was listed in PnPBIOS instead of ACPI, it'll still work.
Quoted 5 lines
Quoted 1 line
Any news about this ?
Sort of fell off my radar, can you resend?
attached 2 versions : the one that kill acpi detection and the one with acpi but a complex init.
Matthieu
Index: drivers/input/serio/i8042-x86ia64io.h
===================================================================
RCS file: /home/mat/dev/linux-cvs-rep/linux-cvs/drivers/input/serio/i8042-x86ia64io.h,v retrieving revision 1.2
input/serio/i8042-x86ia64io.h +63 -133
@@ -88,183 +88,113 @@ };#endif -#ifdef CONFIG_ACPI -#include <linux/acpi.h> -#include <acpi/acpi_bus.h> - -struct i8042_acpi_resources { -unsigned int port1; -unsigned int port2; -unsigned int irq; -}; +#ifdef CONFIG_PNP +#include <linux/pnp.h> -static int i8042_acpi_kbd_registered; -static int i8042_acpi_aux_registered; +static int i8042_pnp_kbd_registered; +static int i8042_pnp_aux_registered; -static acpi_status i8042_acpi_parse_resource (struct acpi_resource *res,void *data) -{ -struct i8042_acpi_resources *i8042_res = data; -struct acpi_resource_io *io; -struct acpi_resource_fixed_io *fixed_io; -struct acpi_resource_irq *irq; -struct acpi_resource_ext_irq *ext_irq; - -switch (res->id) { -case ACPI_RSTYPE_IO : - io = &res->data.io; -if (io->range_length) { -if (!i8042_res->port1) - i8042_res->port1 = io->min_base_address; -else - i8042_res->port2 = io->min_base_address; - } -break ; - -case ACPI_RSTYPE_FIXED_IO : - fixed_io = &res->data.fixed_io; -if (fixed_io->range_length) { -if (!i8042_res->port1) - i8042_res->port1 = fixed_io->base_address; -else - i8042_res->port2 = fixed_io->base_address; - } -break ; - -case ACPI_RSTYPE_IRQ : - irq = &res->data.irq; -if (irq->number_of_interrupts >0 ) - i8042_res->irq = -acpi_register_gsi (irq->interrupts[0 ], - irq->edge_level, - irq->active_high_low); -break ; - -case ACPI_RSTYPE_EXT_IRQ : - ext_irq = &res->data.extended_irq; -if (ext_irq->number_of_interrupts >0 ) - i8042_res->irq = -acpi_register_gsi (ext_irq->interrupts[0 ], - ext_irq->edge_level, - ext_irq->active_high_low); -break ; - } -return AE_OK ; -} -static int i8042_acpi_kbd_add (struct acpi_device *device) +static int i8042_pnp_kbd_probe (struct pnp_dev *dev,const struct pnp_device_id *did) { -struct i8042_acpi_resources kbd_res; -acpi_status status; - -memset (&kbd_res,0 ,sizeof (kbd_res)); - status =acpi_walk_resources (device->handle,METHOD_NAME__CRS , - i8042_acpi_parse_resource, &kbd_res); -if (ACPI_FAILURE (status)) -return -ENODEV ; - -if (kbd_res.port1) - i8042_data_reg = kbd_res.port1; +if (pnp_port_valid (dev,0 ) &&pnp_port_len (dev,0 ) ==1 ) + i8042_data_reg =pnp_port_start (dev,0 );else -printk (KERN_WARNING "ACPI: [%s] has no data port; default is 0x%x\n" , -acpi_device_bid (device), i8042_data_reg); +printk (KERN_WARNING "PNP: [%s] has no data port; default is 0x%x\n" , +pnp_dev_name (dev), i8042_data_reg); -if (kbd_res.port2) - i8042_command_reg = kbd_res.port2; +if (pnp_port_valid (dev,1 ) &&pnp_port_len (dev,1 ) ==1 ) + i8042_command_reg =pnp_port_start (dev,1 );else -printk (KERN_WARNING "ACPI: [%s] has no command port; default is 0x%x\n" , -acpi_device_bid (device), i8042_command_reg); +printk (KERN_WARNING "PNP: [%s] has no command port; default is 0x%x\n" , +pnp_dev_name (dev), i8042_command_reg); -if (kbd_res.irq) - i8042_kbd_irq = kbd_res.irq; +if (pnp_irq_valid (dev,0 )) + i8042_kbd_irq =pnp_irq (dev,0 );else -printk (KERN_WARNING "ACPI: [%s] has no IRQ; default is %d\n" , -acpi_device_bid (device), i8042_kbd_irq); +printk (KERN_WARNING "PNP: [%s] has no IRQ; default is %d\n" , +pnp_dev_name (dev), i8042_kbd_irq); -strncpy (acpi_device_name (device),"PS/2 Keyboard Controller" , -sizeof (acpi_device_name (device))); -printk ("ACPI: %s [%s] at I/O 0x%x, 0x%x, irq %d\n" , -acpi_device_name (device),acpi_device_bid (device), +printk ("PNP: %s [%s] at I/O 0x%x, 0x%x, irq %d\n" , +"PS/2 Keyboard Controller" ,pnp_dev_name (dev), i8042_data_reg, i8042_command_reg, i8042_kbd_irq);return 0 ; } -static int i8042_acpi_aux_add (struct acpi_device *device) +static int i8042_pnp_aux_probe (struct pnp_dev *dev,const struct pnp_device_id *did) { -struct i8042_acpi_resources aux_res; -acpi_status status; - -memset (&aux_res,0 ,sizeof (aux_res)); - status =acpi_walk_resources (device->handle,METHOD_NAME__CRS , - i8042_acpi_parse_resource, &aux_res); -if (ACPI_FAILURE (status)) -return -ENODEV ; - -if (aux_res.irq) - i8042_aux_irq = aux_res.irq; +if (pnp_irq_valid (dev,0 )) + i8042_aux_irq =pnp_irq (dev,0 );else -printk (KERN_WARNING "ACPI: [%s] has no IRQ; default is %d\n" , -acpi_device_bid (device), i8042_aux_irq); +printk (KERN_WARNING "PNP: [%s] has no IRQ; default is %d\n" , +pnp_dev_name (dev), i8042_aux_irq); -strncpy (acpi_device_name (device),"PS/2 Mouse Controller" , -sizeof (acpi_device_name (device))); -printk ("ACPI: %s [%s] at irq %d\n" , -acpi_device_name (device),acpi_device_bid (device), i8042_aux_irq); +printk ("PNP: %s [%s] at irq %d\n" , +"PS/2 Mouse Controller" ,pnp_dev_name (dev), i8042_aux_irq);return 0 ; } -static struct acpi_driver i8042_acpi_kbd_driver = { - .name ="i8042" , - .ids ="PNP0303,PNP030B" , - .ops = { - .add = i8042_acpi_kbd_add, - }, +static struct pnp_device_id pnp_kbd_devids[] = { + { .id ="PNP0303" , .driver_data =0 }, + { .id ="PNP030b" , .driver_data =0 }, + { .id ="" , }, +}; + +static struct pnp_driver i8042_pnp_kbd_driver = { + .name ="i8042 kdb" , + .id_table = pnp_kbd_devids, + .probe = i8042_pnp_kbd_probe, +}; + +static struct pnp_device_id pnp_aux_devids[] = { + { .id ="PNP0f13" , .driver_data =0 }, + { .id ="SYN0801" , .driver_data =0 }, + { .id ="" , }, }; -static struct acpi_driver i8042_acpi_aux_driver = { - .name ="i8042" , - .ids ="PNP0F13,SYN0801" , - .ops = { - .add = i8042_acpi_aux_add, - }, +static struct pnp_driver i8042_pnp_aux_driver = { + .name ="i8042 aux" , + .id_table = pnp_aux_devids, + .probe = i8042_pnp_aux_probe, }; -static int i8042_acpi_init (void ) +static int i8042_pnp_init (void ) {int result; -if (acpi_disabled || i8042_noacpi) { -printk ("i8042: ACPI detection disabled\n" ); +if (i8042_nopnp) { +printk ("i8042: PNP detection disabled\n" );return 0 ; } - result =acpi_bus_register_driver (&i8042_acpi_kbd_driver); + result =pnp_register_driver (&i8042_pnp_kbd_driver);if (result <0 )return result;if (result ==0 ) { -acpi_bus_unregister_driver (&i8042_acpi_kbd_driver); +pnp_unregister_driver (&i8042_pnp_kbd_driver);return -ENODEV ; } - i8042_acpi_kbd_registered =1 ; + i8042_pnp_kbd_registered =1 ; - result =acpi_bus_register_driver (&i8042_acpi_aux_driver); + result =pnp_register_driver (&i8042_pnp_aux_driver);if (result >=0 ) - i8042_acpi_aux_registered =1 ; + i8042_pnp_aux_registered =1 ;if (result ==0 ) i8042_noaux =1 ;return 0 ; } -static void i8042_acpi_exit (void ) +static void i8042_pnp_exit (void ) { -if (i8042_acpi_kbd_registered) -acpi_bus_unregister_driver (&i8042_acpi_kbd_driver); +if (i8042_pnp_kbd_registered) +pnp_unregister_driver (&i8042_pnp_kbd_driver); -if (i8042_acpi_aux_registered) -acpi_bus_unregister_driver (&i8042_acpi_aux_driver); +if (i8042_pnp_aux_registered) +pnp_unregister_driver (&i8042_pnp_aux_driver); }#endif
@@ -281,8 +211,8 @@ i8042_kbd_irq =I8042_MAP_IRQ (1 ); i8042_aux_irq =I8042_MAP_IRQ (12 ); -#ifdef CONFIG_ACPI -if (i8042_acpi_init ()) +#ifdef CONFIG_PNP +if (i8042_pnp_init ())return -1 ;#endif
@@ -300,8 +230,8 @@static inline void i8042_platform_exit (void ) { -#ifdef CONFIG_ACPI -i8042_acpi_exit (); +#ifdef CONFIG_PNP +i8042_pnp_exit ();#endif }
Index: drivers/input/serio/i8042.c
===================================================================
RCS file: /home/mat/dev/linux-cvs-rep/linux-cvs/drivers/input/serio/i8042.c,v retrieving revision 1.45
input/serio/i8042.c +4 -4
@@ -54,10 +54,10 @@module_param_named (noloop, i8042_noloop, bool,0 );MODULE_PARM_DESC (dumbkbd,"Disable the AUX Loopback command while probing for the AUX port" ); -#ifdef CONFIG_ACPI -static int i8042_noacpi; -module_param_named (noacpi, i8042_noacpi, bool,0 ); -MODULE_PARM_DESC (noacpi,"Do not use ACPI to detect controller settings" ); +#ifdef CONFIG_PNP +static int i8042_nopnp; +module_param_named (nopnp, i8042_nopnp, bool,0 ); +MODULE_PARM_DESC (nopnp,"Do not use PNP to detect controller settings" );#endif #define DEBUG
Index: drivers/input/serio/i8042-x86ia64io.h
===================================================================
RCS file: /home/mat/dev/linux-cvs-rep/linux-cvs/drivers/input/serio/i8042-x86ia64io.h,v retrieving revision 1.2
input/serio/i8042-x86ia64io.h +120 -2
140 lines not on this page · raw message
Index: drivers/input/serio/i8042.c
===================================================================
RCS file: /home/mat/dev/linux-cvs-rep/linux-cvs/drivers/input/serio/i8042.c,v retrieving revision 1.45
input/serio/i8042.c +6 -0
@@ -60,6 +60,12 @@MODULE_PARM_DESC (noacpi,"Do not use ACPI to detect controller settings" );#endif +#ifdef CONFIG_PNP +static int i8042_nopnp; +module_param_named (nopnp, i8042_nopnp, bool,0 ); +MODULE_PARM_DESC (nopnp,"Do not use PNP to detect controller settings" ); +#endif +#define DEBUG #ifdef DEBUG static int i8042_debug;
Hi,
Quoted 31 lines · Vojtech Pavlik
Quoted 29 lines · matthieu castet
Hi,
Quoted 24 lines · Vojtech Pavlik
Quoted 4 lines · matthieu castet
Hi,
this patch add PNP support for the i8042 driver in 2.6.10-rc1-mm5. Acpi is try before the pnp driver so if you don't disable ACPI or apply others pnpacpi patches, it won't change anything.Please review it and apply if possible
Ok, my thoughts on this:
It's OK to keep the device allocated to this driver via the PnP
subsystem, and not bother with releasing the code via
__initcall.I agree that if there is a way to enumerate the device, (like PnP, ACPI or OpenFirmware), we should use that instead of probing and using a platform device for the controller.
I think that we should drop the ACPI support from i8042, in favor of pnpacpi, because PnP is more generic and if the keyboard device was listed in PnPBIOS instead of ACPI, it'll still work.
Quoted 5 lines
Quoted 1 line
Any news about this ?
Sort of fell off my radar, can you resend?
attached 2 versions : the one that kill acpi detection and the one with acpi but a complex init.
Matthieu
PS : I resend it, because, it seem it have failed for Vojtech Pavlik.
Index: drivers/input/serio/i8042-x86ia64io.h
===================================================================
RCS file: /home/mat/dev/linux-cvs-rep/linux-cvs/drivers/input/serio/i8042-x86ia64io.h,v retrieving revision 1.2
input/serio/i8042-x86ia64io.h +63 -133
@@ -88,183 +88,113 @@ };#endif -#ifdef CONFIG_ACPI -#include <linux/acpi.h> -#include <acpi/acpi_bus.h> - -struct i8042_acpi_resources { -unsigned int port1; -unsigned int port2; -unsigned int irq; -}; +#ifdef CONFIG_PNP +#include <linux/pnp.h> -static int i8042_acpi_kbd_registered; -static int i8042_acpi_aux_registered; +static int i8042_pnp_kbd_registered; +static int i8042_pnp_aux_registered; -static acpi_status i8042_acpi_parse_resource (struct acpi_resource *res,void *data) -{ -struct i8042_acpi_resources *i8042_res = data; -struct acpi_resource_io *io; -struct acpi_resource_fixed_io *fixed_io; -struct acpi_resource_irq *irq; -struct acpi_resource_ext_irq *ext_irq; - -switch (res->id) { -case ACPI_RSTYPE_IO : - io = &res->data.io; -if (io->range_length) { -if (!i8042_res->port1) - i8042_res->port1 = io->min_base_address; -else - i8042_res->port2 = io->min_base_address; - } -break ; - -case ACPI_RSTYPE_FIXED_IO : - fixed_io = &res->data.fixed_io; -if (fixed_io->range_length) { -if (!i8042_res->port1) - i8042_res->port1 = fixed_io->base_address; -else - i8042_res->port2 = fixed_io->base_address; - } -break ; - -case ACPI_RSTYPE_IRQ : - irq = &res->data.irq; -if (irq->number_of_interrupts >0 ) - i8042_res->irq = -acpi_register_gsi (irq->interrupts[0 ], - irq->edge_level, - irq->active_high_low); -break ; - -case ACPI_RSTYPE_EXT_IRQ : - ext_irq = &res->data.extended_irq; -if (ext_irq->number_of_interrupts >0 ) - i8042_res->irq = -acpi_register_gsi (ext_irq->interrupts[0 ], - ext_irq->edge_level, - ext_irq->active_high_low); -break ; - } -return AE_OK ; -} -static int i8042_acpi_kbd_add (struct acpi_device *device) +static int i8042_pnp_kbd_probe (struct pnp_dev *dev,const struct pnp_device_id *did) { -struct i8042_acpi_resources kbd_res; -acpi_status status; - -memset (&kbd_res,0 ,sizeof (kbd_res)); - status =acpi_walk_resources (device->handle,METHOD_NAME__CRS , - i8042_acpi_parse_resource, &kbd_res); -if (ACPI_FAILURE (status)) -return -ENODEV ; - -if (kbd_res.port1) - i8042_data_reg = kbd_res.port1; +if (pnp_port_valid (dev,0 ) &&pnp_port_len (dev,0 ) ==1 ) + i8042_data_reg =pnp_port_start (dev,0 );else -printk (KERN_WARNING "ACPI: [%s] has no data port; default is 0x%x\n" , -acpi_device_bid (device), i8042_data_reg); +printk (KERN_WARNING "PNP: [%s] has no data port; default is 0x%x\n" , +pnp_dev_name (dev), i8042_data_reg); -if (kbd_res.port2) - i8042_command_reg = kbd_res.port2; +if (pnp_port_valid (dev,1 ) &&pnp_port_len (dev,1 ) ==1 ) + i8042_command_reg =pnp_port_start (dev,1 );else -printk (KERN_WARNING "ACPI: [%s] has no command port; default is 0x%x\n" , -acpi_device_bid (device), i8042_command_reg); +printk (KERN_WARNING "PNP: [%s] has no command port; default is 0x%x\n" , +pnp_dev_name (dev), i8042_command_reg); -if (kbd_res.irq) - i8042_kbd_irq = kbd_res.irq; +if (pnp_irq_valid (dev,0 )) + i8042_kbd_irq =pnp_irq (dev,0 );else -printk (KERN_WARNING "ACPI: [%s] has no IRQ; default is %d\n" , -acpi_device_bid (device), i8042_kbd_irq); +printk (KERN_WARNING "PNP: [%s] has no IRQ; default is %d\n" , +pnp_dev_name (dev), i8042_kbd_irq); -strncpy (acpi_device_name (device),"PS/2 Keyboard Controller" , -sizeof (acpi_device_name (device))); -printk ("ACPI: %s [%s] at I/O 0x%x, 0x%x, irq %d\n" , -acpi_device_name (device),acpi_device_bid (device), +printk ("PNP: %s [%s] at I/O 0x%x, 0x%x, irq %d\n" , +"PS/2 Keyboard Controller" ,pnp_dev_name (dev), i8042_data_reg, i8042_command_reg, i8042_kbd_irq);return 0 ; } -static int i8042_acpi_aux_add (struct acpi_device *device) +static int i8042_pnp_aux_probe (struct pnp_dev *dev,const struct pnp_device_id *did) { -struct i8042_acpi_resources aux_res; -acpi_status status; - -memset (&aux_res,0 ,sizeof (aux_res)); - status =acpi_walk_resources (device->handle,METHOD_NAME__CRS , - i8042_acpi_parse_resource, &aux_res); -if (ACPI_FAILURE (status)) -return -ENODEV ; - -if (aux_res.irq) - i8042_aux_irq = aux_res.irq; +if (pnp_irq_valid (dev,0 )) + i8042_aux_irq =pnp_irq (dev,0 );else -printk (KERN_WARNING "ACPI: [%s] has no IRQ; default is %d\n" , -acpi_device_bid (device), i8042_aux_irq); +printk (KERN_WARNING "PNP: [%s] has no IRQ; default is %d\n" , +pnp_dev_name (dev), i8042_aux_irq); -strncpy (acpi_device_name (device),"PS/2 Mouse Controller" , -sizeof (acpi_device_name (device))); -printk ("ACPI: %s [%s] at irq %d\n" , -acpi_device_name (device),acpi_device_bid (device), i8042_aux_irq); +printk ("PNP: %s [%s] at irq %d\n" , +"PS/2 Mouse Controller" ,pnp_dev_name (dev), i8042_aux_irq);return 0 ; } -static struct acpi_driver i8042_acpi_kbd_driver = { - .name ="i8042" , - .ids ="PNP0303,PNP030B" , - .ops = { - .add = i8042_acpi_kbd_add, - }, +static struct pnp_device_id pnp_kbd_devids[] = { + { .id ="PNP0303" , .driver_data =0 }, + { .id ="PNP030b" , .driver_data =0 }, + { .id ="" , }, +}; + +static struct pnp_driver i8042_pnp_kbd_driver = { + .name ="i8042 kdb" , + .id_table = pnp_kbd_devids, + .probe = i8042_pnp_kbd_probe, +}; + +static struct pnp_device_id pnp_aux_devids[] = { + { .id ="PNP0f13" , .driver_data =0 }, + { .id ="SYN0801" , .driver_data =0 }, + { .id ="" , }, }; -static struct acpi_driver i8042_acpi_aux_driver = { - .name ="i8042" , - .ids ="PNP0F13,SYN0801" , - .ops = { - .add = i8042_acpi_aux_add, - }, +static struct pnp_driver i8042_pnp_aux_driver = { + .name ="i8042 aux" , + .id_table = pnp_aux_devids, + .probe = i8042_pnp_aux_probe, }; -static int i8042_acpi_init (void ) +static int i8042_pnp_init (void ) {int result; -if (acpi_disabled || i8042_noacpi) { -printk ("i8042: ACPI detection disabled\n" ); +if (i8042_nopnp) { +printk ("i8042: PNP detection disabled\n" );return 0 ; } - result =acpi_bus_register_driver (&i8042_acpi_kbd_driver); + result =pnp_register_driver (&i8042_pnp_kbd_driver);if (result <0 )return result;if (result ==0 ) { -acpi_bus_unregister_driver (&i8042_acpi_kbd_driver); +pnp_unregister_driver (&i8042_pnp_kbd_driver);return -ENODEV ; } - i8042_acpi_kbd_registered =1 ; + i8042_pnp_kbd_registered =1 ; - result =acpi_bus_register_driver (&i8042_acpi_aux_driver); + result =pnp_register_driver (&i8042_pnp_aux_driver);if (result >=0 ) - i8042_acpi_aux_registered =1 ; + i8042_pnp_aux_registered =1 ;if (result ==0 ) i8042_noaux =1 ;return 0 ; } -static void i8042_acpi_exit (void ) +static void i8042_pnp_exit (void ) { -if (i8042_acpi_kbd_registered) -acpi_bus_unregister_driver (&i8042_acpi_kbd_driver); +if (i8042_pnp_kbd_registered) +pnp_unregister_driver (&i8042_pnp_kbd_driver); -if (i8042_acpi_aux_registered) -acpi_bus_unregister_driver (&i8042_acpi_aux_driver); +if (i8042_pnp_aux_registered) +pnp_unregister_driver (&i8042_pnp_aux_driver); }#endif
@@ -281,8 +211,8 @@ i8042_kbd_irq =I8042_MAP_IRQ (1 ); i8042_aux_irq =I8042_MAP_IRQ (12 ); -#ifdef CONFIG_ACPI -if (i8042_acpi_init ()) +#ifdef CONFIG_PNP +if (i8042_pnp_init ())return -1 ;#endif
@@ -300,8 +230,8 @@static inline void i8042_platform_exit (void ) { -#ifdef CONFIG_ACPI -i8042_acpi_exit (); +#ifdef CONFIG_PNP +i8042_pnp_exit ();#endif }
Index: drivers/input/serio/i8042.c
===================================================================
RCS file: /home/mat/dev/linux-cvs-rep/linux-cvs/drivers/input/serio/i8042.c,v retrieving revision 1.45
input/serio/i8042.c +4 -4
@@ -54,10 +54,10 @@module_param_named (noloop, i8042_noloop, bool,0 );MODULE_PARM_DESC (dumbkbd,"Disable the AUX Loopback command while probing for the AUX port" ); -#ifdef CONFIG_ACPI -static int i8042_noacpi; -module_param_named (noacpi, i8042_noacpi, bool,0 ); -MODULE_PARM_DESC (noacpi,"Do not use ACPI to detect controller settings" ); +#ifdef CONFIG_PNP +static int i8042_nopnp; +module_param_named (nopnp, i8042_nopnp, bool,0 ); +MODULE_PARM_DESC (nopnp,"Do not use PNP to detect controller settings" );#endif #define DEBUG
Index: drivers/input/serio/i8042-x86ia64io.h
===================================================================
RCS file: /home/mat/dev/linux-cvs-rep/linux-cvs/drivers/input/serio/i8042-x86ia64io.h,v retrieving revision 1.2
input/serio/i8042-x86ia64io.h +120 -2
140 lines not on this page · raw message
Index: drivers/input/serio/i8042.c
===================================================================
RCS file: /home/mat/dev/linux-cvs-rep/linux-cvs/drivers/input/serio/i8042.c,v retrieving revision 1.45
input/serio/i8042.c +6 -0
@@ -60,6 +60,12 @@MODULE_PARM_DESC (noacpi,"Do not use ACPI to detect controller settings" );#endif +#ifdef CONFIG_PNP +static int i8042_nopnp; +module_param_named (nopnp, i8042_nopnp, bool,0 ); +MODULE_PARM_DESC (nopnp,"Do not use PNP to detect controller settings" ); +#endif +#define DEBUG #ifdef DEBUG static int i8042_debug;
Quoted 36 lines · matthieu castet
Hi,
Quoted 33 lines · Vojtech Pavlik
Quoted 31 lines · matthieu castet
Hi,
Quoted 26 lines · Vojtech Pavlik
Quoted 6 lines · matthieu castet
Hi,
this patch add PNP support for the i8042 driver in 2.6.10-rc1-mm5. Acpi is try before the pnp driver so if you don't disable ACPI or apply others pnpacpi patches, it won't change anything.Please review it and apply if possible
Ok, my thoughts on this:
It's OK to keep the device allocated to this driver via the PnP
subsystem, and not bother with releasing the code via
__initcall.I agree that if there is a way to enumerate the device, (like PnP, ACPI or OpenFirmware), we should use that instead of probing and using a platform device for the controller.
I think that we should drop the ACPI support from i8042, in favor of pnpacpi, because PnP is more generic and if the keyboard device was listed in PnPBIOS instead of ACPI, it'll still work.
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Any news about this ?
Sort of fell off my radar, can you resend?
attached 2 versions : the one that kill acpi detection and the one with acpi but a complex init.
Hi Matthieu,
I think that we should kill ACPI now that ACPIPNP is available.
I have a concern though - having PNP driver activated means that we now have i8042 in 2 or 3 places in driver model hierarchy, once as a platform device and the as kbd and aux PNP devices. I wonder how the power management will be coordinated - we normally need to reset controller so BIOS will not be upset and then I need parent for the KBD and AUX serio ports. Plus I guess PNP system enables and disables resources so serio suspend/resume calls should be in right order.
With ACPI we don't have this problem at the moment since ACPI drivers are not integrated into driver model yet.
Quoted 1 line
PS : I resend it, because, it seem it have failed for Vojtech Pavlik.
Vojtech asked to use his [email protected] address over the weekend as his suse.sz has problems.
Signature · 1 line
Dmitry