cpqarray update

ChangeSet 1.1288, 2004/01/26 16:58:21-02:00, [email protected]

[PATCH] cpqarray update

Yes, we should. I usually ask Mike Miller in our group to send my patches since he's been doing that and more known in the community, but since you already got a hold of it ...yes, please :)

CHANGELOG:

* Fix for eisa card not detecting partitions properly
* Use pci_module_init instead of pci_register_device to handle hotplug scenario and unregister if the driver can't find pci controller
* Add BLKSSZGET ioctl

Review line 627

Quoted 8 lines · Linux Kernel Mailing List

ChangeSet 1.1288, 2004/01/26 16:58:21-02:00, [email protected]

(quoted diff) +1 -1

@@ -616,7 +623,7 @@
  	/* detect controllers */
 	printk(DRIVER_NAME "\n");
-	pci_register_driver(&cpqarray_pci_driver);
+	pci_module_init(&cpqarray_pci_driver);
 	cpqarray_eisa_detect();
  	for(i=0; i< MAX_CTLR; i++) {

You need to check the return value of pci_module_init() for errors.

Eeek! I want to fix that code in pci_module_init() so it doesn't do this at all. Please don't copy that horrible function. Just register the driver with a call to vio_register_driver() and drop the whole vio_module_init() completly. I'll be doing that for pci soon, and there's no reason you want to duplicate this broken logic (you always want your module probe to succeed, for lots of reasons...)

Quoted 11 lines · Jeff Garzik

Review line 627

Quoted 8 lines · Linux Kernel Mailing List

ChangeSet 1.1288, 2004/01/26 16:58:21-02:00, [email protected]

(quoted diff) +1 -1

@@ -616,7 +623,7 @@
 	/* detect controllers */
 	printk(DRIVER_NAME "\n");
-	pci_register_driver(&cpqarray_pci_driver);
+	pci_module_init(&cpqarray_pci_driver);
 	cpqarray_eisa_detect();
 	for(i=0; i< MAX_CTLR; i++) {

You need to check the return value of pci_module_init() for errors.

I'm defining a new bus and had copied pci_module_init() to vio_module_init(). Here's what Greg KH had to say about that:

Quoted 6 lines

Eeek! I want to fix that code in pci_module_init() so it doesn't do this at all. Please don't copy that horrible function. Just register the driver with a call to vio_register_driver() and drop the whole vio_module_init() completly. I'll be doing that for pci soon, and there's no reason you want to duplicate this broken logic (you always want your module probe to succeed, for lots of reasons...)

So there's no need for the quoted patch hunk at all.

I'm defining a new bus and had copied pci_module_init() to vio_module_init(). Here's what Greg KH had to say about that:

Quoted 6 lines

Eeek! I want to fix that code in pci_module_init() so it doesn't do this at all. Please don't copy that horrible function. Just register the driver with a call to vio_register_driver() and drop the whole vio_module_init() completly. I'll be doing that for pci soon, and there's no reason you want to duplicate this broken logic (you always want your module probe to succeed, for lots of reasons...)

Actually I disagree with GregKH on this.

The register/unregister functions need to be returning error codes, _not_ the count of interfaces registered. It is trivial to count the registered interfaces in the driver itself, but IMO far more important to propagate fatal errors back to the original caller.

Quoted 6 lines · Jeff Garzik

Actually I disagree with GregKH on this.

The register/unregister functions need to be returning error codes, _not_ the count of interfaces registered. It is trivial to count the registered interfaces in the driver itself, but IMO far more important to propagate fatal errors back to the original caller.

Nevermind, this got fixed. I'm still worried about the '1' return value, though, for zero controllers found.