[PATCH] PNP support for i8042 driver

[PATCH] PNP support for i8042 driver

[matthieu castet]

[-- Attachment #1: Type: text/plain, Size: 319 bytes --]

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

Signed-Off-By: Matthieu Castet <castet.matthieu@free.fr>

[-- Attachment #2: i8042_pnp_acpi2.patch --]
[-- Type: text/x-patch, Size: 4090 bytes --]

--- linux-2.6.9/drivers/input/serio/i8042.c.old	2004-11-12 23:00:09.000000000 +0100
+++ linux-2.6.9/drivers/input/serio/i8042.c	2004-11-12 23:00:39.000000000 +0100
@@ -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;
--- linux-2.6.9/drivers/input/serio/i8042-x86ia64io.h.old	2004-11-12 23:00:02.000000000 +0100
+++ linux-2.6.9/drivers/input/serio/i8042-x86ia64io.h	2004-11-13 12:42:09.000000000 +0100
@@ -88,6 +88,116 @@
 };
 #endif
 
+#ifdef CONFIG_PNP
+#include <linux/pnp.h>
+
+static int i8042_pnp_kbd_registered;
+static int i8042_pnp_aux_registered;
+
+
+static int 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 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 pnp_kbd_devids[] = {
+	{ .id = "PNP0303", .driver_data = 0 },
+	{ .id = "PNP030b", .driver_data = 0 },
+	{ .id = "", },
+};
+
+static struct pnp_driver i8042_pnp_kbd_driver = {
+	.name           = "i8042 kbd",
+	.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 pnp_driver i8042_pnp_aux_driver = {
+	.name           = "i8042 aux",
+	.id_table       = pnp_aux_devids,
+	.probe          = i8042_pnp_aux_probe,
+};
+
+static int i8042_pnp_init(void)
+{
+	int result;
+
+	if (i8042_nopnp) {
+		printk("i8042: PNP detection disabled\n");
+		return 0;
+	}
+
+	result = pnp_register_driver(&i8042_pnp_kbd_driver);
+	if (result < 0)
+		return result;
+
+	if (result == 0) {
+		pnp_unregister_driver(&i8042_pnp_kbd_driver);
+		return -ENODEV;
+	}
+	i8042_pnp_kbd_registered = 1;
+
+	result = pnp_register_driver(&i8042_pnp_aux_driver);
+	if (result >= 0)
+		i8042_pnp_aux_registered = 1;
+	if (result == 0)
+		i8042_noaux = 1;
+
+	return 0;
+}
+
+static void i8042_pnp_exit(void)
+{
+	if (i8042_pnp_kbd_registered)
+		pnp_unregister_driver(&i8042_pnp_kbd_driver);
+
+	if (i8042_pnp_aux_registered)
+		pnp_unregister_driver(&i8042_pnp_aux_driver);
+}
+#endif
+
 #ifdef CONFIG_ACPI
 #include <linux/acpi.h>
 #include <acpi/acpi_bus.h>
@@ -286,10 +396,17 @@
 	i8042_aux_irq = I8042_MAP_IRQ(12);
 
 #ifdef CONFIG_ACPI
-	if (i8042_acpi_init() < 0)
+	if (i8042_acpi_init() < 0) /*ACPI don't detecte Kdb*/
+#endif
+#ifdef CONFIG_PNP
+		if (i8042_pnp_init())
+			return -1;
+#else
+#ifdef CONFIG_ACPI
 		return -1;
 #endif
-
+#endif
+	
 #if defined(__ia64__)
         i8042_reset = 1;
 #endif
@@ -304,6 +421,9 @@
 
 static inline void i8042_platform_exit(void)
 {
+#ifdef CONFIG_PNP
+	i8042_pnp_exit();
+#endif
 }
 
 #endif /* _I8042_X86IA64IO_H */

Re: [PATCH] PNP support for i8042 driver

[Dmitry Torokhov]

On Saturday 13 November 2004 08:23 am, matthieu castet wrote:
> 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
> 
> Signed-Off-By: Matthieu Castet <castet.matthieu@free.fr>
> 

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.

-- 
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 */

Re: [PATCH] PNP support for i8042 driver

[matthieu castet]

Dmitry Torokhov wrote:
> On Saturday 13 November 2004 08:23 am, matthieu castet wrote:
> 
>>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
>>
>>Signed-Off-By: Matthieu Castet <castet.matthieu@free.fr>
>>
> 
> Hi,
> 
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?
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...

> 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

Re: [PATCH] PNP support for i8042 driver

[Dmitry Torokhov]

On Sun, 14 Nov 2004 13:22:21 +0100, matthieu castet
<castet.matthieu@free.fr> wrote:
> Dmitry Torokhov wrote:
> 
> 
> > On Saturday 13 November 2004 08:23 am, matthieu castet wrote:
> >
> >>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
> >>
> >>Signed-Off-By: Matthieu Castet <castet.matthieu@free.fr>
> >>
> >
> > Hi,
> >
> 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?
> 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...

-- 
Dmitry

Re: [PATCH] PNP support for i8042 driver

[matthieu castet]

Dmitry Torokhov wrote:
> On Sun, 14 Nov 2004 13:22:21 +0100, matthieu castet
> <castet.matthieu@free.fr> wrote:
> 
>>Dmitry Torokhov wrote:
>>
>>
>>
>>>On Saturday 13 November 2004 08:23 am, matthieu castet wrote:
>>>
>>>
>>>>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
>>>>
>>>>Signed-Off-By: Matthieu Castet <castet.matthieu@free.fr>
>>>>
>>>Hi,
>>>
>>
>>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?
>>
>>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.

> 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

Re: [PATCH] PNP support for i8042 driver

[Dmitry Torokhov]

On Mon, 15 Nov 2004 20:51:20 +0100, matthieu castet
<castet.matthieu@free.fr> wrote:
> Dmitry Torokhov wrote:
> > 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).

> > 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 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. 

> - 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.

> - 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.

-- 
Dmitry

Re: [PATCH] PNP support for i8042 driver

[matthieu castet]

Hi,

Dmitry Torokhov wrote:
> On Mon, 15 Nov 2004 20:51:20 +0100, matthieu castet
 >>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...

>>
>>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 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. 
> 
> 
>>- 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.
> 
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...

> 
>>- 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

Re: [PATCH] PNP support for i8042 driver

[matthieu castet]

matthieu castet wrote:
> Hi,
> 

>>
>>
>> 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

Re: [PATCH] PNP support for i8042 driver

[Adam Belay]

On Mon, Nov 15, 2004 at 11:52:16PM +0100, matthieu castet wrote:
> Hi,
> 
> Dmitry Torokhov wrote:
> >On Mon, 15 Nov 2004 20:51:20 +0100, matthieu castet
> >>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).

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.

> >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

Re: [PATCH] PNP support for i8042 driver

[Dmitry Torokhov]

On Monday 15 November 2004 05:52 pm, matthieu castet wrote:
> Hi,
> 
> Dmitry Torokhov wrote:
> > 
> > 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...
> 

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

-- 
Dmitry

Re: [PATCH] PNP support for i8042 driver

[Adam Belay]

On Sun, Nov 14, 2004 at 01:48:00AM -0500, Dmitry Torokhov wrote:
> On Saturday 13 November 2004 08:23 am, matthieu castet wrote:
> > 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
> > 
> > Signed-Off-By: Matthieu Castet <castet.matthieu@free.fr>
> > 
> 
> 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.
> 
> -- 
> 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

Re: [PATCH] PNP support for i8042 driver

[Greg KH]

On Tue, Nov 16, 2004 at 12:37:41AM -0500, Adam Belay wrote:
> 
> 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

Re: [PATCH] PNP support for i8042 driver

[Dmitry Torokhov]

On Tuesday 16 November 2004 12:37 am, Adam Belay wrote:
> On Sun, Nov 14, 2004 at 01:48:00AM -0500, Dmitry Torokhov wrote:
> > On Saturday 13 November 2004 08:23 am, matthieu castet wrote:
> > > 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
> > > 
> > > Signed-Off-By: Matthieu Castet <castet.matthieu@free.fr>
> > > 
> > 
> > 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.
> > 
> > -- 
> > 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.
> 

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

Re: [PATCH] PNP support for i8042 driver

[Adam Belay]

On Tue, Nov 16, 2004 at 01:06:46AM -0500, Dmitry Torokhov wrote:
> 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.

> 
> 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.

> 
> 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

Re: [PATCH] PNP support for i8042 driver

[Vojtech Pavlik]

On Sat, Nov 13, 2004 at 02:23:53PM +0100, matthieu castet wrote:
> 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.

-- 
Vojtech Pavlik
SuSE Labs, SuSE CR

Re: [PATCH] PNP support for i8042 driver

[matthieu castet]

Hi,

Vojtech Pavlik wrote:
> On Sat, Nov 13, 2004 at 02:23:53PM +0100, matthieu castet wrote:
> 
>>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.
> 
Any news about this ?


Matthieu CASTET

Re: [PATCH] PNP support for i8042 driver

[Vojtech Pavlik]

On Fri, Feb 04, 2005 at 06:37:29PM +0100, matthieu castet wrote:
> Hi,
> 
> Vojtech Pavlik wrote:
> >On Sat, Nov 13, 2004 at 02:23:53PM +0100, matthieu castet wrote:
> >
> >>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.
> >
> Any news about this ?
 
Sort of fell off my radar, can you resend?

-- 
Vojtech Pavlik
SuSE Labs, SuSE CR

Re: [PATCH] PNP support for i8042 driver

[matthieu castet]

[-- Attachment #1: Type: text/plain, Size: 1253 bytes --]

Hi,

Vojtech Pavlik wrote:
> On Fri, Feb 04, 2005 at 06:37:29PM +0100, matthieu castet wrote:
> 
>>Hi,
>>
>>Vojtech Pavlik wrote:
>>
>>>On Sat, Nov 13, 2004 at 02:23:53PM +0100, matthieu castet wrote:
>>>
>>>
>>>>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.
>>>
>>
>>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

[-- Attachment #2: i8042_pnp.patch --]
[-- Type: text/x-patch, Size: 8655 bytes --]

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
diff -u -u -r1.2 i8042-x86ia64io.h
--- drivers/input/serio/i8042-x86ia64io.h	4 Jan 2005 08:06:19 -0000	1.2
+++ drivers/input/serio/i8042-x86ia64io.h	4 Feb 2005 22:37:12 -0000
@@ -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
diff -u -u -r1.45 i8042.c
--- drivers/input/serio/i8042.c	3 Feb 2005 00:21:36 -0000	1.45
+++ drivers/input/serio/i8042.c	4 Feb 2005 22:37:12 -0000
@@ -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

[-- Attachment #3: i8042_pnp_acpi.patch --]
[-- Type: text/x-patch, Size: 4473 bytes --]

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
diff -u -u -r1.2 i8042-x86ia64io.h
--- drivers/input/serio/i8042-x86ia64io.h	4 Jan 2005 08:06:19 -0000	1.2
+++ drivers/input/serio/i8042-x86ia64io.h	4 Feb 2005 22:50:02 -0000
@@ -88,6 +88,116 @@
 };
 #endif
 
+#ifdef CONFIG_PNP
+#include <linux/pnp.h>
+
+static int i8042_pnp_kbd_registered;
+static int i8042_pnp_aux_registered;
+
+
+static int 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 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 pnp_kbd_devids[] = {
+	{ .id = "PNP0303", .driver_data = 0 },
+	{ .id = "PNP030b", .driver_data = 0 },
+	{ .id = "", },
+};
+
+static struct pnp_driver i8042_pnp_kbd_driver = {
+	.name           = "i8042 kbd",
+	.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 pnp_driver i8042_pnp_aux_driver = {
+	.name           = "i8042 aux",
+	.id_table       = pnp_aux_devids,
+	.probe          = i8042_pnp_aux_probe,
+};
+
+static int i8042_pnp_init(void)
+{
+	int result;
+
+	if (i8042_nopnp) {
+		printk("i8042: PNP detection disabled\n");
+		return 0;
+	}
+
+	result = pnp_register_driver(&i8042_pnp_kbd_driver);
+	if (result < 0)
+		return result;
+
+	if (result == 0) {
+		pnp_unregister_driver(&i8042_pnp_kbd_driver);
+		return -ENODEV;
+	}
+	i8042_pnp_kbd_registered = 1;
+
+	result = pnp_register_driver(&i8042_pnp_aux_driver);
+	if (result >= 0)
+		i8042_pnp_aux_registered = 1;
+	if (result == 0)
+		i8042_noaux = 1;
+
+	return 0;
+}
+
+static void i8042_pnp_exit(void)
+{
+	if (i8042_pnp_kbd_registered)
+		pnp_unregister_driver(&i8042_pnp_kbd_driver);
+
+	if (i8042_pnp_aux_registered)
+		pnp_unregister_driver(&i8042_pnp_aux_driver);
+}
+#endif
+
 #ifdef CONFIG_ACPI
 #include <linux/acpi.h>
 #include <acpi/acpi_bus.h>
@@ -282,8 +392,13 @@
 	i8042_aux_irq = I8042_MAP_IRQ(12);
 
 #ifdef CONFIG_ACPI
-	if (i8042_acpi_init())
-		return -1;
+	if (i8042_acpi_init() < 0) /*ACPI don't detecte Kdb*/
+#endif
+#ifdef CONFIG_PNP
+		if (i8042_pnp_init())
+#endif
+#if defined(CONFIG_ACPI) || defined(CONFIG_PNP)
+			return -1;
 #endif
 
 #if defined(__ia64__)
@@ -303,6 +418,9 @@
 #ifdef CONFIG_ACPI
 	i8042_acpi_exit();
 #endif
+#ifdef CONFIG_PNP
+	i8042_pnp_exit();
+#endif
 }
 
 #endif /* _I8042_X86IA64IO_H */
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
diff -u -u -r1.45 i8042.c
--- drivers/input/serio/i8042.c	3 Feb 2005 00:21:36 -0000	1.45
+++ drivers/input/serio/i8042.c	4 Feb 2005 22:45:07 -0000
@@ -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;

Re: [PATCH] PNP support for i8042 driver

[matthieu castet]

[-- Attachment #1: Type: text/plain, Size: 1323 bytes --]

Hi,

Vojtech Pavlik wrote:
> On Fri, Feb 04, 2005 at 06:37:29PM +0100, matthieu castet wrote:
> 
>>Hi,
>>
>>Vojtech Pavlik wrote:
>>
>>>On Sat, Nov 13, 2004 at 02:23:53PM +0100, matthieu castet wrote:
>>>
>>>
>>>>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.
>>>
>>
>>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.

[-- Attachment #2: i8042_pnp.patch --]
[-- Type: text/x-patch, Size: 8656 bytes --]

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
diff -u -u -r1.2 i8042-x86ia64io.h
--- drivers/input/serio/i8042-x86ia64io.h	4 Jan 2005 08:06:19 -0000	1.2
+++ drivers/input/serio/i8042-x86ia64io.h	4 Feb 2005 22:37:12 -0000
@@ -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
diff -u -u -r1.45 i8042.c
--- drivers/input/serio/i8042.c	3 Feb 2005 00:21:36 -0000	1.45
+++ drivers/input/serio/i8042.c	4 Feb 2005 22:37:12 -0000
@@ -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


[-- Attachment #3: i8042_pnp_acpi.patch --]
[-- Type: text/x-patch, Size: 4474 bytes --]

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
diff -u -u -r1.2 i8042-x86ia64io.h
--- drivers/input/serio/i8042-x86ia64io.h	4 Jan 2005 08:06:19 -0000	1.2
+++ drivers/input/serio/i8042-x86ia64io.h	4 Feb 2005 22:50:02 -0000
@@ -88,6 +88,116 @@
 };
 #endif
 
+#ifdef CONFIG_PNP
+#include <linux/pnp.h>
+
+static int i8042_pnp_kbd_registered;
+static int i8042_pnp_aux_registered;
+
+
+static int 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 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 pnp_kbd_devids[] = {
+	{ .id = "PNP0303", .driver_data = 0 },
+	{ .id = "PNP030b", .driver_data = 0 },
+	{ .id = "", },
+};
+
+static struct pnp_driver i8042_pnp_kbd_driver = {
+	.name           = "i8042 kbd",
+	.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 pnp_driver i8042_pnp_aux_driver = {
+	.name           = "i8042 aux",
+	.id_table       = pnp_aux_devids,
+	.probe          = i8042_pnp_aux_probe,
+};
+
+static int i8042_pnp_init(void)
+{
+	int result;
+
+	if (i8042_nopnp) {
+		printk("i8042: PNP detection disabled\n");
+		return 0;
+	}
+
+	result = pnp_register_driver(&i8042_pnp_kbd_driver);
+	if (result < 0)
+		return result;
+
+	if (result == 0) {
+		pnp_unregister_driver(&i8042_pnp_kbd_driver);
+		return -ENODEV;
+	}
+	i8042_pnp_kbd_registered = 1;
+
+	result = pnp_register_driver(&i8042_pnp_aux_driver);
+	if (result >= 0)
+		i8042_pnp_aux_registered = 1;
+	if (result == 0)
+		i8042_noaux = 1;
+
+	return 0;
+}
+
+static void i8042_pnp_exit(void)
+{
+	if (i8042_pnp_kbd_registered)
+		pnp_unregister_driver(&i8042_pnp_kbd_driver);
+
+	if (i8042_pnp_aux_registered)
+		pnp_unregister_driver(&i8042_pnp_aux_driver);
+}
+#endif
+
 #ifdef CONFIG_ACPI
 #include <linux/acpi.h>
 #include <acpi/acpi_bus.h>
@@ -282,8 +392,13 @@
 	i8042_aux_irq = I8042_MAP_IRQ(12);
 
 #ifdef CONFIG_ACPI
-	if (i8042_acpi_init())
-		return -1;
+	if (i8042_acpi_init() < 0) /*ACPI don't detecte Kdb*/
+#endif
+#ifdef CONFIG_PNP
+		if (i8042_pnp_init())
+#endif
+#if defined(CONFIG_ACPI) || defined(CONFIG_PNP)
+			return -1;
 #endif
 
 #if defined(__ia64__)
@@ -303,6 +418,9 @@
 #ifdef CONFIG_ACPI
 	i8042_acpi_exit();
 #endif
+#ifdef CONFIG_PNP
+	i8042_pnp_exit();
+#endif
 }
 
 #endif /* _I8042_X86IA64IO_H */
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
diff -u -u -r1.45 i8042.c
--- drivers/input/serio/i8042.c	3 Feb 2005 00:21:36 -0000	1.45
+++ drivers/input/serio/i8042.c	4 Feb 2005 22:45:07 -0000
@@ -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;

Re: [PATCH] PNP support for i8042 driver

[Dmitry Torokhov]

On Saturday 05 February 2005 08:48, matthieu castet wrote:
> Hi,
> 
> Vojtech Pavlik wrote:
> > On Fri, Feb 04, 2005 at 06:37:29PM +0100, matthieu castet wrote:
> > 
> >>Hi,
> >>
> >>Vojtech Pavlik wrote:
> >>
> >>>On Sat, Nov 13, 2004 at 02:23:53PM +0100, matthieu castet wrote:
> >>>
> >>>
> >>>>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.
> >>>
> >>
> >>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.

> PS : I resend it, because, it seem it have failed for Vojtech Pavlik.

Vojtech asked to use his vojtech@ucw.cz address over the weekend as his
suse.sz has problems.

-- 
Dmitry