Re: [PATCH] PnP update - drivers

Re: [PATCH] PnP update - drivers

[Linus Torvalds]

On Sun, 12 Jan 2003, Jaroslav Kysela wrote:
>
> You can import this changeset into BK by piping this whole message to:
> '| bk receive [path to repository]' or apply the patch as usual.

STOP THIS F*CKING MADNESS ALREADY.

If you send BK patches, at least do it right, and make sure your BK patch 
is a proper child of something I actually _have_, so that I don't get 
results like:

	takepatch: can't find parent ID
	        perex@suse.cz|ChangeSet|20030111100546|48939
	        in RESYNC/SCCS/s.ChangeSet

and if you only want to send the patch and not use BK merges, then don't 
even bother to _use_ the BK merge stuff.

		Linus

Re: [PATCH] PnP update - drivers

[Jaroslav Kysela]

On Sun, 12 Jan 2003, Linus Torvalds wrote:

> 
> On Sun, 12 Jan 2003, Jaroslav Kysela wrote:
> >
> > You can import this changeset into BK by piping this whole message to:
> > '| bk receive [path to repository]' or apply the patch as usual.
> 
> STOP THIS F*CKING MADNESS ALREADY.
> 
> If you send BK patches, at least do it right, and make sure your BK patch 
> is a proper child of something I actually _have_, so that I don't get 
> results like:
> 
> 	takepatch: can't find parent ID
> 	        perex@suse.cz|ChangeSet|20030111100546|48939
> 	        in RESYNC/SCCS/s.ChangeSet
> 
> and if you only want to send the patch and not use BK merges, then don't 
> even bother to _use_ the BK merge stuff.

I'm sorry. The empty changesets are evil. I've overlooked one. I've 
modified the bksend script to compare repositories and generate all 
missing changesets (including empty ones). Code is attached.

						Jaroslav

--- linux/Documentation/BK-usage/bksend.old	Mon Jan 13 15:34:02 2003
+++ linux/Documentation/BK-usage/bksend	Mon Jan 13 13:42:02 2003
@@ -3,34 +3,86 @@
 # Andreas Dilger <adilger@turbolabs.com>  13/02/2002
 #
 # Add diffstat output after Changelog <adilger@turbolabs.com>   21/02/2002
+# Add compare and compare_split commands to compare and show changes with
+# another repository  Jaroslav Kysela <perex@suse.cz>
 
 PROG=bksend
+REPOSITORY="http://linux.bkbits.net/linux-2.5"
 
 usage() {
 	echo "usage: $PROG -r<rev>"
 	echo -e "\twhere <rev> is of the form '1.23', '1.23..', '1.23..1.27',"
 	echo -e "\tor '+' to indicate the most recent revision"
+	echo "usage: $PROG [-S] -R<repository>"
+	echo -e "\twhere <repository> is parent repository to sync with"
+	echo -e "\tand -S means split output"
 
 	exit 1
 }
 
-case $1 in
--r) REV=$2; shift ;;
--r*) REV=`echo $1 | sed 's/^-r//'` ;;
-*) echo "$PROG: no revision given, you probably don't want that";;
-esac
-
-[ -z "$REV" ] && usage
-
-echo "You can import this changeset into BK by piping this whole message to:"
-echo "'| bk receive [path to repository]' or apply the patch as usual."
+while [ ! -z $1 ]; do
+	case $1 in
+	-r) REV=$2; [ -z "$CMD" ] && CMD=rev; shift ;;
+	-r*) REV=`echo $1 | sed 's/^-r//'`; [ -z "$CMD" ] && CMD=rev ;;
+	-R) REP=$2; [ -z "$CMD" ] && CMD=compare; shift ;;
+	-R*) REP=`echo $1 | sed 's/^-R//'`; [ -z "$CMD" ] && CMD=compare ;;
+	-S) ([ -z "$CMD" ] || [ "$CMD" = "compare" ]) && CMD=compare_split ;;
+	-H) NOHEADER=1 ;;
+	*) echo "$PROG: no option given, you probably don't want that";;
+	esac
+	shift
+done
+
+[ -z "$CMD" ] && usage
+
+if [ "$CMD" = "compare" ] && [ ! -z "$REP" ]; then
+  REPOSITORY="$REP"
+fi
+
+if [ -z "$NOHEADER" ]; then
+  echo "You can import this changeset into BK by piping this whole message to:"
+  echo "'| bk receive [path to repository]' or apply the patch as usual."
+fi
 
 SEP="\n===================================================================\n\n"
-echo -e $SEP
-bk changes -r$REV
-echo
-bk export -tpatch -du -h -r$REV | diffstat
-echo; echo
-bk export -tpatch -du -h -r$REV
-echo -e $SEP
-bk send -wgzip_uu -r$REV -
+
+case $CMD in
+rev)
+	echo -e $SEP
+	bk changes -r$REV
+	echo
+	bk export -tpatch -du -h -r$REV | diffstat
+	echo; echo
+	bk export -tpatch -du -h -r$REV
+	echo -e $SEP
+	bk send -wgzip_uu -r$REV -
+	;;
+compare)
+	bk changes -e -f -L -d':CSETREV:\n' $REPOSITORY |
+		while read rev; do
+			echo -e $SEP
+			bk changes -r$rev
+			echo
+			bk export -tpatch -du -h -r$rev | diffstat
+		done
+	bk changes -e -f -L -d':CSETREV:\n' $REPOSITORY |
+		while read rev; do
+			echo
+			echo -e $SEP
+			echo "This patch contains changeset $rev"
+			echo; echo
+			bk export -tpatch -du -h -r$rev
+		done
+	bk changes -e -f -L -d':CSETREV:\n' $REPOSITORY |
+		while read rev; do
+			echo -e $SEP
+			bk send -wgzip_uu -r$rev -
+		done
+	;;
+compare_split)
+	bk changes -e -f -L -d':CSETREV:\n' $REPOSITORY |
+		while read rev; do \
+			$0 -H -r$rev ; \
+		done
+	;;
+esac

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project, SuSE Labs

Re: [PATCH] PnP update - drivers

[Adam Belay]

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

Re: [PATCH] PnP update - drivers

[Linus Torvalds]

On Mon, 13 Jan 2003, Adam Belay wrote:
> 
> 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.

I will _not_ accept just stupid reverts, if that means that a device 
driver is not available for people to test with.

We do not break drivers like that, and the reason you have clashes with 
other people is that so many drivers ended up being broken for too long. 

Feel free to revert the changes one by one _as_they_get_fixed_. But don't 
even try to send me a patch that reverts things to a broken state. The PnP 
changes have been painful enough as-is.

		Linus

Re: [PATCH] PnP update - drivers

[Adam Belay]

On Mon, Jan 13, 2003 at 02:48:45PM -0800, Linus Torvalds wrote:
> 
> On Mon, 13 Jan 2003, Adam Belay wrote:
> > 
> > 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.
> 
> I will _not_ accept just stupid reverts, if that means that a device 
> driver is not available for people to test with.
> 
> We do not break drivers like that, and the reason you have clashes with 
> other people is that so many drivers ended up being broken for too long. 
> 
> Feel free to revert the changes one by one _as_they_get_fixed_. But don't 
> even try to send me a patch that reverts things to a broken state. The PnP 
> changes have been painful enough as-is.

Hi Linus,

I agree that it is important not to cause further damage to the existing pnp 
drivers.  Therefore I will carefully modify the drivers one by one before making
any changes to the pnp layer that would conflict.  In this way, all the pnp
drivers will remain functional during development.

Thanks,
Adam

Re: [PATCH] PnP update - drivers

[Linus Torvalds]

On Mon, 13 Jan 2003, Adam Belay wrote:
>
> PnP IDE Conversion from Zwane Mwaikambo

This, btw, is _worse_ than the current conversion, as far as I can tell. 
In particular:

> -/* 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);
> +}

That "pci_dev->dev = pnp_dev->dev" looks totally bletcherous, and does a
structure copy that potentially copies pointers that simply ARE NOT VALID
after the copy.

Not good.

		Linus

Re: [PATCH] PnP update - drivers

[Zwane Mwaikambo]

On Mon, 13 Jan 2003, Linus Torvalds wrote:

> 
> On Mon, 13 Jan 2003, Adam Belay wrote:
> >
> > PnP IDE Conversion from Zwane Mwaikambo
> 
> This, btw, is _worse_ than the current conversion, as far as I can tell. 
> In particular:

I just had a look (and test booted), the current one definitely is better 
and less kludgy.

> > -/* 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);
> > +}
> 
> That "pci_dev->dev = pnp_dev->dev" looks totally bletcherous, and does a
> structure copy that potentially copies pointers that simply ARE NOT VALID
> after the copy.
> 
> Not good.

Hey, i did offer barf bags ;) I admit, that part is complete bollocks, due 
to my bending over for pci_dev.

	Zwane
-- 
function.linuxpower.ca

Re: [PATCH] PnP update - drivers

[Greg KH]

On Mon, Jan 13, 2003 at 05:39:06PM +0000, Adam Belay wrote:
> On Sun, Jan 12, 2003 at 08:30:57PM +0100, Jaroslav Kysela wrote:
> > 	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.
> 
> Hi Jaroslav,
> 
> Next time send pnp related changes to me directly.

And Adam, sorry for taking so long in getting your previous changes you
sent to me to Linus.  That's my fault, it was in my queue to review when
Jaroslav sent his patches.  I guess now I'll just wait for this merge
mess to get cleaned up :(

greg k-h

Re: [PATCH] PnP update - drivers

[Shawn Starr]

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

Re: [PATCH] PnP update - drivers

[Jaroslav Kysela]

On Mon, 13 Jan 2003, 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,

Hi,

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

I'm sorry Adam, but you are doing this work very slowly. While I'm one of 
people who approved your changes, I'm feeling a bit responsible for 
situation when most of older driver cannot be compiled. So I tried to do 
all changes which I need for ALSA and also revert all PnP drivers to state 
when they can be USED!!!

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

My work is not indended to do the right conversion. It's up to maintainers 
of drivers, but I tried to convert drivers to some USEABLE STATE. Also, 
you have still the chance to do it better..

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

Sorry, but I think that Linus won't accept your changes in this way.

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

But, please, be faster, much faster.... I'm waiting for stabilizing PnP
over two months to make our ALSA ISA PnP drivers working again with all
features. You simply deleted older ISA PnP API leaving all drivers in
non-functional stage. That's bad, very bad. Also, I have other work to do 
than rewrite the PnP drivers.

						Jaroslav

BTW: I tested ISA PnP code outside kernel (in ALSA drivers) more than half 
of year.

BTW2: I also think that I'm author of the auto-configuration mechanism and 
resource descriptions, so it would be nice to have also credit in
the pnp_init() function.

BTW3: I'm happy, that I can start with rewritting of ALSA drivers now.

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project, SuSE Labs

Re: [PATCH] PnP update - drivers

[Thomas Hood]

Adam Belay wrote:
> Next time send pnp related changes to me directly.  I would
> have been happy to include your work after I carefully
> reviewed it.

Well, if you are going to criticize others ...

Next time you rewrite a subsystem such as pnpbios, I suggest
you communicate with those who have most recently been working
on that subsystem.  They might be able to help you with the
work or at least review your changes for errors.  They won't
be much inclined to help if the first information they have
about your work is the appearance of a huge unreadable
(due to file mvs) patch which you describe as "ready".

-- 
Thomas Hood <jdthood@yahoo.co.uk>