changed from pci_find_device to pci_get_device

Hello friends!

I've made some changes and are interested in feedback (drivers/ide/pci/via82cxxx.c).

Some notes:
* The main change was to switch from pci_find_device to pci_get_device.
* I unlock "isa" when it won't be used anymore
* Changed a return value because init_chipset_via82cxxx returns an unsigned value.

This is the patch in a readable format:

static unsigned int __devinit init_chipset_via82cxxx(struct pci_dev *dev, const char *name)

{
	struct pci_dev *isa = NULL;

	for (via_config = via_isa_bridges; via_config->id; via_config++)
-		if ((isa = pci_find_device(PCI_VENDOR_ID_VIA +
+		if ((isa = pci_get_device(PCI_VENDOR_ID_VIA +
			!!(via_config->flags & VIA_BAD_ID),
			via_config->id, NULL))) {

			pci_read_config_byte(isa, PCI_REVISION_ID, &t);
			if (t >= via_config->rev_min &&
			    t <= via_config->rev_max)
				break;
+
+			pci_dev_put(isa);
		}

	if (!via_config->id) {
		printk(KERN_WARNING "VP_IDE: Unknown VIA SouthBridge, disabling DMA.\n");
-		return -ENODEV;
+		return ENODEV;
	}

	/* DO STUFF (I don't include it here for your convenience) */

#if defined(DISPLAY_VIA_TIMINGS) && defined(CONFIG_PROC_FS)
- if (!via_proc) {
+ if (via_proc) {
+ isa_dev = NULL;
+ pci_dev_put(isa);

+	} else {
		via_base = pci_resource_start(dev, 4);
		bmide_dev = dev;
		isa_dev = isa;
		ide_pci_create_host_proc("via", via_get_info);
		via_proc = 1;
	}

+#else
+ pci_dev_put(isa);
#endif /* DISPLAY_VIA_TIMINGS && CONFIG_PROC_FS */

	return 0;
}

+#if defined(DISPLAY_VIA_TIMINGS) && defined(CONFIG_PROC_FS) +static void via_ide_exit(void)
+{
+ if (isa_dev)
+ pci_dev_put(isa_dev);
+}
+module_exit(via_ide_exit);
+#endif /* DISPLAY_VIA_TIMINGS && CONFIG_PROC_FS */

Best regards,
Daniel Marjamäki

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

But it's not in a format that could be applied :(

Also, try splitting this up into 3 different patches, as you are doing three different things.

thanks,

greg k-h

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

The patch I posted was made against the wrong kernel version. Sorry.

The patch below is made against linux-2.6.15-rc2..

@@ -247,7 +247,7 @@ static struct via_isa_bridge *via_config
 	u8 t;
 
 	for (via_config = via_isa_bridges; via_config->id; via_config++)
-		if ((*isa = pci_find_device(PCI_VENDOR_ID_VIA +
+		if ((*isa = pci_get_device(PCI_VENDOR_ID_VIA +
 			!!(via_config->flags & VIA_BAD_ID),
 			via_config->id, NULL))) {
 

@@ -255,6 +255,9 @@ static struct via_isa_bridge *via_config
 			if (t >= via_config->rev_min &&
 			    t <= via_config->rev_max)
 				break;
+
+			pci_dev_put(*isa);
+			*isa = NULL;
 		}
 
 	return via_config;

@@ -282,7 +285,7 @@ static unsigned int __devinit init_chips
 	via_config = via_config_find(&isa);
 	if (!via_config->id) {
 		printk(KERN_WARNING "VP_IDE: Unknown VIA SouthBridge, disabling DMA.\n");
-		return -ENODEV;
+		return ENODEV;
 	}
 
 	/*

@@ -360,6 +363,8 @@ static unsigned int __devinit init_chips
 		via_dma[via_config->flags & VIA_UDMA],
 		pci_name(dev));
 
+	pci_dev_put(isa);
+
 	return 0;
 }
 

@@ -430,6 +435,7 @@ static void __devinit init_hwif_via82cxx
 	ide_set_hwifdata(hwif, vdev);
 
 	vdev->via_config = via_config_find(&isa);
+	if (isa) pci_dev_put(isa);
 	via_cable_detect(hwif->pci_dev, vdev);
 
 	hwif->autodma = 0;

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Thank you Alexey for reviewing my patch.

About the chunk you didn't understand.
The function must return an unsigned int, therefore "return -ENODEV;" is wrong.

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

->init_chipset method returns either small positive number (irq) or BIG positive number in case of error ((unsigned int) -ENODEV). In do_ide_setup_pci_device() function ->init_chipset return value is casted to int and checked for "< 0".

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

I made a HUGE mistake then. Sorry.
Thank you Alexey!!

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Hello friends!

I have reworked my patch that I sent a few days ago.
I made huge mistakes in the previous patch but I hope this patch is perfect.

@@ -247,7 +247,7 @@ static struct via_isa_bridge *via_config
 	u8 t;
 
 	for (via_config = via_isa_bridges; via_config->id; via_config++)
-		if ((*isa = pci_find_device(PCI_VENDOR_ID_VIA +
+		if ((*isa = pci_get_device(PCI_VENDOR_ID_VIA +
 			!!(via_config->flags & VIA_BAD_ID),
 			via_config->id, NULL))) {
 

@@ -255,6 +255,9 @@ static struct via_isa_bridge *via_config
 			if (t >= via_config->rev_min &&
 			    t <= via_config->rev_max)
 				break;
+
+			pci_dev_put(*isa);
+			*isa = NULL;
 		}
 
 	return via_config;

@@ -360,6 +363,8 @@ static unsigned int __devinit init_chips
 		via_dma[via_config->flags & VIA_UDMA],
 		pci_name(dev));
 
+	pci_dev_put(isa);
+
 	return 0;
 }
 

@@ -430,6 +435,8 @@ static void __devinit init_hwif_via82cxx
 	ide_set_hwifdata(hwif, vdev);
 
 	vdev->via_config = via_config_find(&isa);
+	if (isa)
+		pci_dev_put(isa);
 	via_cable_detect(hwif->pci_dev, vdev);
 
 	hwif->autodma = 0;

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Please include the whole list of what you are changing in the patch, as sometimes we don't remember what is supposed to be happening here :)

Also, you forgot the "Signed-off-by:" line...