Re: [PATCH] PNPACPI

[matthieu castet <castet.matthieu@free.fr>]

matthieu castet wrote:
> Hi,
> 
> this patch allow to choose the behavior of the pnpacpi driver, it could 
>  don't lock the acpi device like in mm5 or it could lock it like in mm2.
> 
> I have add extra check (CRS presence), so it shouldn't lock too much 
> driver like in mm2.
> I also add the hpet id in the blacklist, because we need to do an pnp 
> driver for it.
> 
> Battery, Button and Fan may be remove from the blacklist because they 
> don't seem to have CRS.
> 
> Please review it and apply if possible
> 
> thanks,
> 
> Matthieu CASTET
> 
Could I have some comments ?

If you don't want the binding part, could the other fix be applied ?
What about in this case about the other patch(to be improved) I made in 
order to give priority to acpi driver over pnp one ?

It could be good to have something clean : I know that the blacklist is 
not a good solution : I even told it before Shaohua implemented it. But 
could you propose something else? It is ugly to stay with 2 layers that 
accepts the same device.


Regards

Matthieu CASTET


> Signed-Off-By: Matthieu Castet <castet.matthieu@free.fr>
> 
> 
> ------------------------------------------------------------------------
> 
> --- linux-2.6.9/drivers/pnp/pnpacpi/core.c.old	2004-11-13 11:53:23.000000000 +0100
> +++ linux-2.6.9/drivers/pnp/pnpacpi/core.c	2004-11-13 13:17:53.000000000 +0100
> @@ -26,15 +26,16 @@
>  
>  static int num = 0;
>  
> -static char __initdata excluded_id_list[] =
> -	"PNP0C0A," /* Battery */
> -	"PNP0C0C,PNP0C0E,PNP0C0D," /* Button */
> +static char excluded_id_list[] =
> +	"PNP0C0A," /* Battery */ /* is there a CRS ?*/
> +	"PNP0C0C,PNP0C0E,PNP0C0D," /* Button */ /* is there a CRS ?*/
> +	"PNP0C0B," /* Fan */ /* is there a CRS ?*/
>  	"PNP0C09," /* EC */
> -	"PNP0C0B," /* Fan */
>  	"PNP0A03," /* PCI root */
>  	"PNP0C0F," /* Link device */
>  	"PNP0000," /* PIC */
>  	"PNP0100," /* Timer */
> +	"PNP0103," /* hpet could be converted, but need work on irq/address */
>  	;
>  static inline int is_exclusive_device(struct acpi_device *dev)
>  {
> @@ -58,7 +59,7 @@
>  #define TEST_ALPHA(c) \
>  	if (!('@' <= (c) || (c) <= 'Z')) \
>  		return 0
> -static int __init ispnpidacpi(char *id)
> +static int ispnpidacpi(char *id)
>  {
>  	TEST_ALPHA(id[0]);
>  	TEST_ALPHA(id[1]);
> @@ -72,7 +73,7 @@
>  	return 1;
>  }
>  
> -static void __init pnpidacpi_to_pnpid(char *id, char *str)
> +static void pnpidacpi_to_pnpid(char *id, char *str)
>  {
>  	str[0] = id[0];
>  	str[1] = id[1];
> @@ -131,17 +132,13 @@
>  	.disable = pnpacpi_disable_resources,
>  };
>  
> -static int __init pnpacpi_add_device(struct acpi_device *device)
> +static int acpi_pnp_add(struct acpi_device *device)
>  {
>  	acpi_handle temp = NULL;
>  	acpi_status status;
>  	struct pnp_id *dev_id;
>  	struct pnp_dev *dev;
>  
> -	if (!ispnpidacpi(acpi_device_hid(device)) ||
> -		is_exclusive_device(device))
> -		return 0;
> -
>  	pnp_dbg("ACPI device : hid %s", acpi_device_hid(device));
>  	dev =  pnpacpi_kmalloc(sizeof(struct pnp_dev), GFP_KERNEL);
>  	if (!dev) {
> @@ -221,6 +218,9 @@
>  	pnp_add_device(dev);
>  	num ++;
>  
> +#ifndef CONFIG_PNPACPI_NOLOCK
> +	acpi_driver_data(device) = dev;
> +#endif
>  	return AE_OK;
>  err1:
>  	kfree(dev_id);
> @@ -229,15 +229,50 @@
>  	return -EINVAL;
>  }
>  
> +static int acpi_pnp_match(struct acpi_device *device,
> +	struct acpi_driver	*driver)
> +{
> +	acpi_handle temp = NULL;
> +	acpi_status status;
> +	/* don't lock non standard pnp device */
> +	status = acpi_get_handle(device->handle, "_CRS", &temp);
> +	return (ACPI_FAILURE(status) || !ispnpidacpi(acpi_device_hid(device)) ||
> +		is_exclusive_device(device));
> +}
> +
> +#ifndef CONFIG_PNPACPI_NOLOCK
> +static int acpi_pnp_remove (struct acpi_device *device, int type)
> +{
> +	struct pnp_dev *dev = acpi_driver_data(device);
> +	if (!dev)
> +		return AE_ERROR;
> +
> +	pnp_remove_device(dev);
> +	return AE_OK;
> +}
> +
> +/* default acpi PNP device driver, support hotplug */
> +static struct acpi_driver acpi_pnp_driver = {
> +	.name =		"ACPI PNP Driver",
> +	.class =	"acpi_pnp",
> +	.ops =		{
> +				.add = acpi_pnp_add,
> +				.remove = acpi_pnp_remove,
> +				.match = acpi_pnp_match,
> +			},
> +};
> +#else
>  static acpi_status __init pnpacpi_add_device_handler(acpi_handle handle,
>  	u32 lvl, void *context, void **rv)
>  {
>  	struct acpi_device *device;
>  
> -	if (!acpi_bus_get_device(handle, &device))
> -		pnpacpi_add_device(device);
> +	if (!acpi_bus_get_device(handle, &device) &&
> +		!acpi_pnp_match(device, NULL))
> +		acpi_pnp_add(device);
>  	return AE_OK;
>  }
> +#endif
>  
>  int __init pnpacpi_init(void)
>  {
> @@ -247,9 +282,14 @@
>  	}
>  	pnp_info("PnP ACPI init");
>  	pnp_register_protocol(&pnpacpi_protocol);
> +#ifndef CONFIG_PNPACPI_NOLOCK
> +	if (acpi_bus_register_driver(&acpi_pnp_driver) < 0)
> +		return -ENODEV;
> +#else
>  	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
>  			ACPI_UINT32_MAX, pnpacpi_add_device_handler,
>  			NULL, NULL);
> +#endif
>  	pnp_info("PnP ACPI: found %d devices", num);
>  	return 0;
>  }
> --- linux-2.6.9/drivers/pnp/pnpacpi/Kconfig.old	2004-11-13 12:04:44.000000000 +0100
> +++ linux-2.6.9/drivers/pnp/pnpacpi/Kconfig	2004-11-13 12:20:48.000000000 +0100
> @@ -16,3 +16,13 @@
>            your mainboard devices (on some systems they are disabled by the
>            BIOS) say Y here.  Also the PNPACPI can help prevent resource
>            conflicts between mainboard devices and other bus devices.
> +
> +config PNPACPI_NOLOCK
> +	bool "Don't lock acpi device(OBSOLETE)"
> +	depends on PNPACPI
> +	default y
> +	---help---
> +	  This option allow you to don't lock acpi devices that are used
> +	  by pnpacpi. It could avoid locking some devices that shouldn't
> +	  be locked, or allow to used acpi drivers instead of pnp drivers.
> +	  Note that it will disable hotplug support for pnpacpi devices.