Re: [PATCH] PnP update - drivers

[Shawn Starr <spstarr@sh0n.net>]

Agreed, Some of those improvements will help us fix old ISA devices that are 
misbehaving.

On Monday 13 January 2003 12:39 pm, Adam Belay wrote:
> On Sun, Jan 12, 2003 at 08:30:57PM +0100, Jaroslav Kysela wrote:
> > Hi,
> >
> > 	this patch must be applied after PnP patch v0.94. It contains my
> > small cleanups of PnP code and I tried to rewrite almost all ISA PnP
> > drivers to new PnP subsystem except sound drivers (ALSA & OSS). Please,
> > apply to get away compilation problems.
> >
> > 						Jaroslav
>
> Hi Jaroslav,
>
> Next time send pnp related changes to me directly.  I would have been happy
> to include your work after I carefully reviewed it.  I was planning to
> merge a very large resource algorithm improvement soon, but now because of
> these changes, that I do not even necessarily agree with, I will be unable
> to include this major improvement and bug fix for a while.  Furthermore
> many people of whom are counting on me to merge thier patches will now be
> dissappointed to hear that their changes, many of which are critical for
> certain pnp hardware configurations will be delayed.
>
> Although I am glad to see the drivers converted, this has been done in a
> way that is not desirable.  They use compat.c which was intended as a
> temporary solution.  In fact I may even remove compat.c all together.  This
> has been clearly stated in both the file compat.c and pnp.txt
> documentation. Attached is a copy of Zwane's ide conversion patch against
> 2.5.56. It can be used as an example of a correct driver conversion. 
> Notice how it is fully integrated into the driver model as all drivers
> should be.
>
> I'm now unhappy with the current pnp code and will most likely revert all
> pnp changes between 2.5.56 and 2.5.57 to avoid a merging nightmare.  I will
> then carefully remerge what I feel is acceptable.
>
> Once again, I'm sorry to those who will be unable to use thier systems due
> to this major set back.  All pnp changes should be sent to me and me only. 
> I believe these pnp change conflicts can easily be worked out with better
> communication.  I intend to make a better effort in the future to explain
> my goals and I hope that you will do the same.  I appreciate your work with
> the pnp layer.
>
> Sincerely,
>
> Adam Belay
> Linux Plug and Play Maintainer
>
>
>
>
>
> ===================================================================
> PnP IDE Conversion from Zwane Mwaikambo
>
>
>
> Index: linux-2.5.56/drivers/ide/ide-pnp.c
> ===================================================================
> RCS file: /build/cvsroot/linux-2.5.56/drivers/ide/ide-pnp.c,v
> retrieving revision 1.1.1.1
> diff -u -r1.1.1.1 ide-pnp.c
> --- linux-2.5.56/drivers/ide/ide-pnp.c	16 Dec 2002 05:16:23 -0000	1.1.1.1
> +++ linux-2.5.56/drivers/ide/ide-pnp.c	16 Dec 2002 08:30:39 -0000
> @@ -18,13 +18,11 @@
>
>  #include <linux/ide.h>
>  #include <linux/init.h>
> -
> -#include <linux/isapnp.h>
> +#include <linux/pnp.h>
>
>  #define DEV_IO(dev, index) (dev->resource[index].start)
>  #define DEV_IRQ(dev, index) (dev->irq_resource[index].start)
> -
> -#define DEV_NAME(dev) (dev->bus->name ? dev->bus->name : "ISA PnP")
> +#define DEV_NAME(dev) (dev->protocol->name ? dev->protocol->name : "ISA
> PnP")
>
>  #define GENERIC_HD_DATA		0
>  #define GENERIC_HD_ERROR	1
> @@ -35,125 +33,125 @@
>  #define GENERIC_HD_SELECT	6
>  #define GENERIC_HD_STATUS	7
>
> -static int generic_ide_offsets[IDE_NR_PORTS] __initdata = {
> +static int generic_ide_offsets[IDE_NR_PORTS] = {
>  	GENERIC_HD_DATA, GENERIC_HD_ERROR, GENERIC_HD_NSECTOR,
>  	GENERIC_HD_SECTOR, GENERIC_HD_LCYL, GENERIC_HD_HCYL,
>  	GENERIC_HD_SELECT, GENERIC_HD_STATUS, -1, -1
>  };
>
>  /* ISA PnP device table entry */
> -struct pnp_dev_t {
> -	unsigned short card_vendor, card_device, vendor, device;
> -	int (*init_fn)(struct pci_dev *dev, int enable);
> +struct idepnp_private {
> +	struct pnp_dev *dev;
> +	struct pci_dev pci_dev; /* we need this for the upper layers */
> +	int (*init_fn)(struct idepnp_private *device, int enable);
>  };
>
> -/* Generic initialisation function for ISA PnP IDE interface */
> +/* Barf bags at the ready! Enough to satisfy IDE core */
> +static void pnp_to_pci(struct pnp_dev *pnp_dev, struct pci_dev *pci_dev)
> +{
> +	pci_dev->dev = pnp_dev->dev;
> +	pci_set_drvdata(pci_dev, pnp_get_drvdata(pnp_dev));
> +	pci_dev->irq = DEV_IRQ(pnp_dev, 0);
> +	pci_set_dma_mask(pci_dev, 0x00ffffff);
> +}
>
> -static int __init pnpide_generic_init(struct pci_dev *dev, int enable)
> +/* Generic initialisation function for ISA PnP IDE interface */
> +static int pnpide_generic_init(struct idepnp_private *device, int enable)
>  {
>  	hw_regs_t hw;
>  	ide_hwif_t *hwif;
>  	int index;
> +	struct pnp_dev *dev = device->dev;
>
> -	if (!enable)
> +	if (!enable) {
> +		/* nothing to do for now */
>  		return 0;
> +	}
>
>  	if (!(DEV_IO(dev, 0) && DEV_IO(dev, 1) && DEV_IRQ(dev, 0)))
> -		return 1;
> +		return -EINVAL;
>
>  	ide_setup_ports(&hw, (ide_ioreg_t) DEV_IO(dev, 0),
>  			generic_ide_offsets,
>  			(ide_ioreg_t) DEV_IO(dev, 1),
>  			0, NULL,
> -//			generic_pnp_ide_iops,
> +			/* generic_pnp_ide_iops, */
>  			DEV_IRQ(dev, 0));
>
>  	index = ide_register_hw(&hw, &hwif);
>
>  	if (index != -1) {
>  	    	printk(KERN_INFO "ide%d: %s IDE interface\n", index, DEV_NAME(dev));
> -		hwif->pci_dev = dev;
> +		hwif->pci_dev = &device->pci_dev;
>  		return 0;
>  	}
>
> -	return 1;
> +	return -ENODEV;
>  }
>
>  /* Add your devices here :)) */
> -struct pnp_dev_t idepnp_devices[] __initdata = {
> -  	/* Generic ESDI/IDE/ATA compatible hard disk controller */
> -	{	ISAPNP_ANY_ID, ISAPNP_ANY_ID,
> -		ISAPNP_VENDOR('P', 'N', 'P'), ISAPNP_DEVICE(0x0600),
> -		pnpide_generic_init },
> -	{	0 }
> +#define IDEPNP_GENERIC_INIT	0
> +static const struct pnp_device_id pnp_ide_devs[] = {
> +	/* Generic ESDI/IDE/ATA compatible hard disk controller */
> +	{"PNP0600", IDEPNP_GENERIC_INIT},
> +	{"", 0}
>  };
>
>  #define NR_PNP_DEVICES 8
> -struct pnp_dev_inst {
> -	struct pci_dev *dev;
> -	struct pnp_dev_t *dev_type;
> -};
> -static struct pnp_dev_inst devices[NR_PNP_DEVICES];
> -static int pnp_ide_dev_idx = 0;
> +/* Nb. pnpide_generic_init is indexed as IDEPNP_GENERIC_INIT */
> +static int (*init_functions[])(struct idepnp_private *device, int enable)
> = {pnpide_generic_init}; +static struct idepnp_private
> devices[NR_PNP_DEVICES];
> +static int pnp_ide_dev_idx;
>
>  /*
>   * Probe for ISA PnP IDE interfaces.
>   */
> -
> -void __init pnpide_init(int enable)
> +static int pnp_ide_probe(struct pnp_dev *pdev, const struct pnp_device_id
> *dev_id) {
> -	struct pci_dev *dev = NULL;
> -	struct pnp_dev_t *dev_type;
> +	int ret;
> +	struct idepnp_private *p;
>
> -	if (!isapnp_present())
> -		return;
> +	/*
> +	 * Register device in the array to
> +	 * deactivate it on a module unload.
> +	 */
> +	if (pnp_ide_dev_idx >= NR_PNP_DEVICES)
> +		return -ENOSPC;
> +
> +	p = &devices[pnp_ide_dev_idx];
> +	p->init_fn = init_functions[dev_id->driver_data];
> +	p->dev = pdev;
> +	pnp_set_drvdata(pdev, p);
> +	pnp_to_pci(p->dev, &p->pci_dev);
> +	ret = p->init_fn(p, 1);
> +	if (!ret)
> +		pnp_ide_dev_idx++;
> +
> +	return ret;
> +}
>
> -	/* Module unload, deactivate all registered devices. */
> -	if (!enable) {
> -		int i;
> -		for (i = 0; i < pnp_ide_dev_idx; i++) {
> -			dev = devices[i].dev;
> -			devices[i].dev_type->init_fn(dev, 0);
> -			if (dev->deactivate)
> -				dev->deactivate(dev);
> -		}
> -		return;
> -	}
> +static void pnp_ide_remove(struct pnp_dev *dev)
> +{
> +	struct idepnp_private *p = pnp_get_drvdata(dev);
> +
> +	/* if p is null you have a bug elsewhere */
> +	p->init_fn(p, 0);
> +	pnp_ide_dev_idx--;
> +	return;
> +}
>
> -	for (dev_type = idepnp_devices; dev_type->vendor; dev_type++) {
> -		while ((dev = isapnp_find_dev(NULL, dev_type->vendor,
> -			dev_type->device, dev))) {
> -
> -			if (dev->active)
> -				continue;
> -
> -       			if (dev->prepare && dev->prepare(dev) < 0) {
> -				printk(KERN_ERR"ide-pnp: %s prepare failed\n", DEV_NAME(dev));
> -				continue;
> -			}
> -
> -			if (dev->activate && dev->activate(dev) < 0) {
> -				printk(KERN_ERR"ide: %s activate failed\n", DEV_NAME(dev));
> -				continue;
> -			}
> -
> -			/* Call device initialization function */
> -			if (dev_type->init_fn(dev, 1)) {
> -				if (dev->deactivate(dev))
> -					dev->deactivate(dev);
> -			} else {
> -#ifdef MODULE
> -				/*
> -				 * Register device in the array to
> -				 * deactivate it on a module unload.
> -				 */
> -				if (pnp_ide_dev_idx >= NR_PNP_DEVICES)
> -					return;
> -				devices[pnp_ide_dev_idx].dev = dev;
> -				devices[pnp_ide_dev_idx].dev_type = dev_type;
> -				pnp_ide_dev_idx++;
> -#endif
> -			}
> -		}
> -	}
> +static struct pnp_driver idepnp_driver = {
> +	.name		= "ide-pnp",
> +	.id_table	= pnp_ide_devs,
> +	.probe		= pnp_ide_probe,
> +	.remove		= pnp_ide_remove
> +};
> +
> +void pnpide_init(int enable)
> +{
> +	if (enable)
> +		pnp_register_driver(&idepnp_driver);
> +	else
> +		pnp_unregister_driver(&idepnp_driver);
>  }
> +
> Index: linux-2.5.56/drivers/ide/ide.c
> ===================================================================
> RCS file: /build/cvsroot/linux-2.5.56/drivers/ide/ide.c,v
> retrieving revision 1.1.1.1
> diff -u -r1.1.1.1 ide.c
> --- linux-2.5.56/drivers/ide/ide.c	16 Dec 2002 05:16:22 -0000	1.1.1.1
> +++ linux-2.5.56/drivers/ide/ide.c	16 Dec 2002 08:52:17 -0000
> @@ -2080,7 +2080,7 @@
>  		buddha_init();
>  	}
>  #endif /* CONFIG_BLK_DEV_BUDDHA */
> -#if defined(CONFIG_BLK_DEV_ISAPNP) && defined(CONFIG_ISAPNP)
> +#if defined(CONFIG_BLK_DEV_ISAPNP) && defined(CONFIG_PNP)
>  	{
>  		extern void pnpide_init(int enable);
>  		pnpide_init(1);
> @@ -2256,7 +2256,7 @@
>  		spin_unlock_irqrestore(&ide_lock, flags);
>  		return 1;
>  	}
> -#if defined(CONFIG_BLK_DEV_ISAPNP) && defined(CONFIG_ISAPNP) &&
> defined(MODULE) +#if defined(CONFIG_BLK_DEV_ISAPNP) && defined(CONFIG_PNP)
> && defined(MODULE) pnpide_init(0);
>  #endif /* CONFIG_BLK_DEV_ISAPNP */
>  #ifdef CONFIG_PROC_FS