[PATCH 2/2] gdth: fix Error: Driver 'gdth' is already registered, aborting...

[PATCH 2/2] gdth: fix Error: Driver 'gdth' is already registered, aborting...

[James Bottomley]

This message appears on modprobe/rmmod/modprobe of the driver.  It's
caused because if the driver has no instances, it returns an error
from gdth_init, which causes the module to fail to load.
Unfortunately, the module's pci driver is still registered at this
point.

Fix this by making gdth behave like a modern driver and insert even if
it doesn't find any instances (in case of hot plug or software driven
binding).

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/scsi/gdth.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index 16785a2..46771d4 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -550,7 +550,6 @@ static int __init gdth_search_isa(ulong32 bios_adr)
 #endif /* CONFIG_ISA */
 
 #ifdef CONFIG_PCI
-static bool gdth_pci_registered;
 
 static bool gdth_search_vortex(ushort device)
 {
@@ -5157,8 +5156,13 @@ static int __init gdth_init(void)
 
 #ifdef CONFIG_PCI
 	/* scanning for PCI controllers */
-	if (pci_register_driver(&gdth_pci_driver) == 0)
-		gdth_pci_registered = true;
+	if (pci_register_driver(&gdth_pci_driver)) {
+		gdth_ha_str *ha;
+
+		list_for_each_entry(ha, &gdth_instances, list)
+			gdth_remove_one(ha);
+		return -ENODEV;
+	}
 #endif /* CONFIG_PCI */
 
 	TRACE2(("gdth_detect() %d controller detected\n", gdth_ctr_count));
@@ -5181,8 +5185,7 @@ static void __exit gdth_exit(void)
 #endif
 
 #ifdef CONFIG_PCI
-	if (gdth_pci_registered)
-		pci_unregister_driver(&gdth_pci_driver);
+	pci_unregister_driver(&gdth_pci_driver);
 #endif
 
 	list_for_each_entry(ha, &gdth_instances, list)
-- 
1.5.4.4

Re: [PATCH 2/2] gdth: fix Error: Driver 'gdth' is already registered, aborting...

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 1629 bytes --]

James Bottomley wrote:
> This message appears on modprobe/rmmod/modprobe of the driver.  It's
> caused because if the driver has no instances, it returns an error
> from gdth_init, which causes the module to fail to load.
> Unfortunately, the module's pci driver is still registered at this
> point.
>
> Fix this by making gdth behave like a modern driver and insert even if
> it doesn't find any instances (in case of hot plug or software driven
> binding).
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>  drivers/scsi/gdth.c |   13 ++++++++-----
>  1 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
> index 16785a2..46771d4 100644
> --- a/drivers/scsi/gdth.c
> +++ b/drivers/scsi/gdth.c
> @@ -550,7 +550,6 @@ static int __init gdth_search_isa(ulong32 bios_adr)
>  #endif /* CONFIG_ISA */
>
>  #ifdef CONFIG_PCI
> -static bool gdth_pci_registered;
>
>  static bool gdth_search_vortex(ushort device)
>  {
> @@ -5157,8 +5156,13 @@ static int __init gdth_init(void)
>
>  #ifdef CONFIG_PCI
>  	/* scanning for PCI controllers */
> -	if (pci_register_driver(&gdth_pci_driver) == 0)
> -		gdth_pci_registered = true;
> +	if (pci_register_driver(&gdth_pci_driver)) {
> +		gdth_ha_str *ha;
> +
> +		list_for_each_entry(ha, &gdth_instances, list)
> +			gdth_remove_one(ha);
> +		return -ENODEV;
> +	}
>  #endif /* CONFIG_PCI */
>
>  	TRACE2(("gdth_detect() %d controller detected\n", gdth_ctr_count));

Wouldn't it be better to return the error code that pci_register_driver() sent 
back?

Eike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 194 bytes --]

Re: [PATCH 2/2] gdth: fix Error: Driver 'gdth' is already registered, aborting...

[James Bottomley]

On Mon, 2008-05-05 at 11:24 +0200, Rolf Eike Beer wrote:
> James Bottomley wrote:
> > This message appears on modprobe/rmmod/modprobe of the driver.  It's
> > caused because if the driver has no instances, it returns an error
> > from gdth_init, which causes the module to fail to load.
> > Unfortunately, the module's pci driver is still registered at this
> > point.
> >
> > Fix this by making gdth behave like a modern driver and insert even if
> > it doesn't find any instances (in case of hot plug or software driven
> > binding).
> >
> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> > ---
> >  drivers/scsi/gdth.c |   13 ++++++++-----
> >  1 files changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
> > index 16785a2..46771d4 100644
> > --- a/drivers/scsi/gdth.c
> > +++ b/drivers/scsi/gdth.c
> > @@ -550,7 +550,6 @@ static int __init gdth_search_isa(ulong32 bios_adr)
> >  #endif /* CONFIG_ISA */
> >
> >  #ifdef CONFIG_PCI
> > -static bool gdth_pci_registered;
> >
> >  static bool gdth_search_vortex(ushort device)
> >  {
> > @@ -5157,8 +5156,13 @@ static int __init gdth_init(void)
> >
> >  #ifdef CONFIG_PCI
> >  	/* scanning for PCI controllers */
> > -	if (pci_register_driver(&gdth_pci_driver) == 0)
> > -		gdth_pci_registered = true;
> > +	if (pci_register_driver(&gdth_pci_driver)) {
> > +		gdth_ha_str *ha;
> > +
> > +		list_for_each_entry(ha, &gdth_instances, list)
> > +			gdth_remove_one(ha);
> > +		return -ENODEV;
> > +	}
> >  #endif /* CONFIG_PCI */
> >
> >  	TRACE2(("gdth_detect() %d controller detected\n", gdth_ctr_count));
> 
> Wouldn't it be better to return the error code that pci_register_driver() sent 
> back?

Not really it's just a redo of the original behaviour.

James