I was trying to build the USB EHCI host controller support as modules
for a PowerPC 834x which also has an embedded EHCI (and PCI enabled).
I get the following build error:
In file included from drivers/usb/host/ehci-hcd.c:895:
drivers/usb/host/ehci-fsl.c:365: error: redefinition of '__inittest'
drivers/usb/host/ehci-pci.c:363: error: previous definition of
'__inittest' was here
drivers/usb/host/ehci-fsl.c:365: error: redefinition of 'init_module'
drivers/usb/host/ehci-pci.c:363: error: previous definition of
'init_module' was here
drivers/usb/host/ehci-fsl.c:365: error: redefinition of 'init_module'
drivers/usb/host/ehci-pci.c:363: error: previous definition of
'init_module' was here
drivers/usb/host/ehci-fsl.c:366: error: redefinition of '__exittest'
drivers/usb/host/ehci-pci.c:369: error: previous definition of
'__exittest' was here
drivers/usb/host/ehci-fsl.c:366: error: redefinition of 'cleanup_module'
drivers/usb/host/ehci-pci.c:369: error: previous definition of
'cleanup_module' was here
drivers/usb/host/ehci-fsl.c:366: error: redefinition of 'cleanup_module'
drivers/usb/host/ehci-pci.c:369: error: previous definition of
'cleanup_module' was here
Which makes sense based on how ehci-hcd.c includes "ehci-fsl.c" and
"ehci-pci.c". I was wondering if there were an thoughts on how to
address this so I can build as a module.
- kumar
On Thursday 23 March 2006 2:26 pm, Kumar Gala wrote:
> "ehci-pci.c". I was wondering if there were an thoughts on how to
> address this so I can build as a module.
Hmm, there was a patch to fix that for OHCI a while back, I'm not
sure what happened to it. Maybe the cleaned up version just never
got posted as promised.
The problem is that there need to be two different init (and exit)
section routines for the bus glue: platform_bus and/or pci_bus.
PCs typically just have PCI; battery-oriented SOCs tend to never
have PCI; and we're now starting to see some non-battery SOCs that
include PCI support as well as integrated USB host support.
The *hci-hcd.c file should be converted to have a single module_init()
and module_exit() routine at the end, looking something like
static int __init Xhci_init(void)
{
int status;
/* various shared stuff, dump version etc */
/* SOCs usually use only this path */
status = Xhci_platform_register();
if (status < 0)
return status;
/* PCs, and a few higher powered SOCs, use this */
status = Xhci_pci_register();
if (status < 0)
Xhci_platform_unregister();
return status;
}
module_init(Xhci_init);
Likewise for module_exit. The #includes for the platform-specific glue
(including PCI) would define those Xhci_platform_*() routine, and it's
a simple bit of #ifdeffery to make sure there's always at least some
NOP default available for those.
- Dave
On Mar 24, 2006, at 12:33 AM, David Brownell wrote:
> On Thursday 23 March 2006 2:26 pm, Kumar Gala wrote:
>
>> "ehci-pci.c". I was wondering if there were an thoughts on how to
>> address this so I can build as a module.
>
> Hmm, there was a patch to fix that for OHCI a while back, I'm not
> sure what happened to it. Maybe the cleaned up version just never
> got posted as promised.
>
> The problem is that there need to be two different init (and exit)
> section routines for the bus glue: platform_bus and/or pci_bus.
>
> PCs typically just have PCI; battery-oriented SOCs tend to never
> have PCI; and we're now starting to see some non-battery SOCs that
> include PCI support as well as integrated USB host support.
>
> The *hci-hcd.c file should be converted to have a single module_init()
> and module_exit() routine at the end, looking something like
>
> static int __init Xhci_init(void)
> {
> int status;
>
> /* various shared stuff, dump version etc */
>
> /* SOCs usually use only this path */
> status = Xhci_platform_register();
> if (status < 0)
> return status;
>
> /* PCs, and a few higher powered SOCs, use this */
> status = Xhci_pci_register();
> if (status < 0)
> Xhci_platform_unregister();
> return status;
> }
> module_init(Xhci_init);
>
> Likewise for module_exit. The #includes for the platform-specific
> glue
> (including PCI) would define those Xhci_platform_*() routine, and it's
> a simple bit of #ifdeffery to make sure there's always at least some
> NOP default available for those.
The issue I have this is that it makes two (or more) things that were
independent now dependent. What about just moving the module_init/
exit() functions into files that are built separately. For the ehci-
fsl case it was trivial, need to look at ehci-pci case.
- kumar
> The issue I have this is that it makes two (or more) things that were
> independent now dependent. What about just moving the module_init/
> exit() functions into files that are built separately. For the ehci-
> fsl case it was trivial, need to look at ehci-pci case.
Ok, my idea required exporting things I didn't really want to export, so
what about something like this or where you thinking of some more
sophisticated?
If this is good, I'll do the same for ohci.
diff --git a/drivers/usb/host/ehci-au1xxx.c b/drivers/usb/host/ehci-au1xxx.c
index 63eadee..036a1c0 100644
--- a/drivers/usb/host/ehci-au1xxx.c
+++ b/drivers/usb/host/ehci-au1xxx.c
@@ -280,18 +280,3 @@ static struct device_driver ehci_hcd_au1
/*.suspend = ehci_hcd_au1xxx_drv_suspend, */
/*.resume = ehci_hcd_au1xxx_drv_resume, */
};
-
-static int __init ehci_hcd_au1xxx_init(void)
-{
- pr_debug(DRIVER_INFO " (Au1xxx)\n");
-
- return driver_register(&ehci_hcd_au1xxx_driver);
-}
-
-static void __exit ehci_hcd_au1xxx_cleanup(void)
-{
- driver_unregister(&ehci_hcd_au1xxx_driver);
-}
-
-module_init(ehci_hcd_au1xxx_init);
-module_exit(ehci_hcd_au1xxx_cleanup);
diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
index f985f12..30410c2 100644
--- a/drivers/usb/host/ehci-fsl.c
+++ b/drivers/usb/host/ehci-fsl.c
@@ -339,28 +339,3 @@ static struct platform_driver ehci_fsl_m
.name = "fsl-usb2-mph",
},
};
-
-static int __init ehci_fsl_init(void)
-{
- int retval;
-
- pr_debug("%s: block sizes: qh %Zd qtd %Zd itd %Zd sitd %Zd\n",
- hcd_name,
- sizeof(struct ehci_qh), sizeof(struct ehci_qtd),
- sizeof(struct ehci_itd), sizeof(struct ehci_sitd));
-
- retval = platform_driver_register(&ehci_fsl_dr_driver);
- if (retval)
- return retval;
-
- return platform_driver_register(&ehci_fsl_mph_driver);
-}
-
-static void __exit ehci_fsl_cleanup(void)
-{
- platform_driver_unregister(&ehci_fsl_mph_driver);
- platform_driver_unregister(&ehci_fsl_dr_driver);
-}
-
-module_init(ehci_fsl_init);
-module_exit(ehci_fsl_cleanup);
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 79f2d8b..549ce59 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -905,3 +905,57 @@ MODULE_LICENSE ("GPL");
#ifndef EHCI_BUS_GLUED
#error "missing bus glue for ehci-hcd"
#endif
+
+static int __init ehci_hcd_init(void)
+{
+ int retval = 0;
+
+ pr_debug("%s: block sizes: qh %Zd qtd %Zd itd %Zd sitd %Zd\n",
+ hcd_name,
+ sizeof(struct ehci_qh), sizeof(struct ehci_qtd),
+ sizeof(struct ehci_itd), sizeof(struct ehci_sitd));
+
+#ifdef CONFIG_PPC_83xx
+ retval = platform_driver_register(&ehci_fsl_dr_driver);
+ if (retval < 0)
+ return retval;
+
+ retval = platform_driver_register(&ehci_fsl_dr_driver);
+ if (retval < 0)
+ return retval;
+#endif
+
+#ifdef CONFIG_SOC_AU1X00
+ pr_debug(DRIVER_INFO " (Au1xxx)\n");
+
+ retval = driver_register(&ehci_hcd_au1xxx_driver);
+ if (retval < 0)
+ return retval;
+#endif
+
+#ifdef CONFIG_PCI
+ retval = pci_register_driver(&ehci_pci_driver);
+ if (retval < 0)
+ return retval;
+#endif
+
+ return retval;
+}
+
+static void __exit ehci_hcd_cleanup(void)
+{
+#ifdef CONFIG_PPC_83xx
+ platform_driver_unregister(&ehci_fsl_mph_driver);
+ platform_driver_unregister(&ehci_fsl_dr_driver);
+#endif
+#ifdef CONFIG_SOC_AU1X00
+ driver_unregister(&ehci_hcd_au1xxx_driver);
+#endif
+#ifdef CONFIG_PCI
+ pci_unregister_driver(&ehci_pci_driver);
+#endif
+}
+
+module_init(ehci_hcd_init);
+module_exit(ehci_hcd_cleanup);
+
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 1e03f1a..e0641bc 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -370,23 +370,3 @@ static struct pci_driver ehci_pci_driver
.resume = usb_hcd_pci_resume,
#endif
};
-
-static int __init ehci_hcd_pci_init(void)
-{
- if (usb_disabled())
- return -ENODEV;
-
- pr_debug("%s: block sizes: qh %Zd qtd %Zd itd %Zd sitd %Zd\n",
- hcd_name,
- sizeof(struct ehci_qh), sizeof(struct ehci_qtd),
- sizeof(struct ehci_itd), sizeof(struct ehci_sitd));
-
- return pci_register_driver(&ehci_pci_driver);
-}
-module_init(ehci_hcd_pci_init);
-
-static void __exit ehci_hcd_pci_cleanup(void)
-{
- pci_unregister_driver(&ehci_pci_driver);
-}
-module_exit(ehci_hcd_pci_cleanup);
On Mar 24, 2006, at 2:32 PM, Kumar Gala wrote:
>> The issue I have this is that it makes two (or more) things that were
>> independent now dependent. What about just moving the module_init/
>> exit() functions into files that are built separately. For the ehci-
>> fsl case it was trivial, need to look at ehci-pci case.
>
> Ok, my idea required exporting things I didn't really want to
> export, so
> what about something like this or where you thinking of some more
> sophisticated?
>
> If this is good, I'll do the same for ohci.
David, any comments?
- k
> diff --git a/drivers/usb/host/ehci-au1xxx.c b/drivers/usb/host/ehci-
> au1xxx.c
> index 63eadee..036a1c0 100644
> --- a/drivers/usb/host/ehci-au1xxx.c
> +++ b/drivers/usb/host/ehci-au1xxx.c
> @@ -280,18 +280,3 @@ static struct device_driver ehci_hcd_au1
> /*.suspend = ehci_hcd_au1xxx_drv_suspend, */
> /*.resume = ehci_hcd_au1xxx_drv_resume, */
> };
> -
> -static int __init ehci_hcd_au1xxx_init(void)
> -{
> - pr_debug(DRIVER_INFO " (Au1xxx)\n");
> -
> - return driver_register(&ehci_hcd_au1xxx_driver);
> -}
> -
> -static void __exit ehci_hcd_au1xxx_cleanup(void)
> -{
> - driver_unregister(&ehci_hcd_au1xxx_driver);
> -}
> -
> -module_init(ehci_hcd_au1xxx_init);
> -module_exit(ehci_hcd_au1xxx_cleanup);
> diff --git a/drivers/usb/host/ehci-fsl.c b/drivers/usb/host/ehci-fsl.c
> index f985f12..30410c2 100644
> --- a/drivers/usb/host/ehci-fsl.c
> +++ b/drivers/usb/host/ehci-fsl.c
> @@ -339,28 +339,3 @@ static struct platform_driver ehci_fsl_m
> .name = "fsl-usb2-mph",
> },
> };
> -
> -static int __init ehci_fsl_init(void)
> -{
> - int retval;
> -
> - pr_debug("%s: block sizes: qh %Zd qtd %Zd itd %Zd sitd %Zd\n",
> - hcd_name,
> - sizeof(struct ehci_qh), sizeof(struct ehci_qtd),
> - sizeof(struct ehci_itd), sizeof(struct ehci_sitd));
> -
> - retval = platform_driver_register(&ehci_fsl_dr_driver);
> - if (retval)
> - return retval;
> -
> - return platform_driver_register(&ehci_fsl_mph_driver);
> -}
> -
> -static void __exit ehci_fsl_cleanup(void)
> -{
> - platform_driver_unregister(&ehci_fsl_mph_driver);
> - platform_driver_unregister(&ehci_fsl_dr_driver);
> -}
> -
> -module_init(ehci_fsl_init);
> -module_exit(ehci_fsl_cleanup);
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index 79f2d8b..549ce59 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -905,3 +905,57 @@ MODULE_LICENSE ("GPL");
> #ifndef EHCI_BUS_GLUED
> #error "missing bus glue for ehci-hcd"
> #endif
> +
> +static int __init ehci_hcd_init(void)
> +{
> + int retval = 0;
> +
> + pr_debug("%s: block sizes: qh %Zd qtd %Zd itd %Zd sitd %Zd\n",
> + hcd_name,
> + sizeof(struct ehci_qh), sizeof(struct ehci_qtd),
> + sizeof(struct ehci_itd), sizeof(struct ehci_sitd));
> +
> +#ifdef CONFIG_PPC_83xx
> + retval = platform_driver_register(&ehci_fsl_dr_driver);
> + if (retval < 0)
> + return retval;
> +
> + retval = platform_driver_register(&ehci_fsl_dr_driver);
> + if (retval < 0)
> + return retval;
> +#endif
> +
> +#ifdef CONFIG_SOC_AU1X00
> + pr_debug(DRIVER_INFO " (Au1xxx)\n");
> +
> + retval = driver_register(&ehci_hcd_au1xxx_driver);
> + if (retval < 0)
> + return retval;
> +#endif
> +
> +#ifdef CONFIG_PCI
> + retval = pci_register_driver(&ehci_pci_driver);
> + if (retval < 0)
> + return retval;
> +#endif
> +
> + return retval;
> +}
> +
> +static void __exit ehci_hcd_cleanup(void)
> +{
> +#ifdef CONFIG_PPC_83xx
> + platform_driver_unregister(&ehci_fsl_mph_driver);
> + platform_driver_unregister(&ehci_fsl_dr_driver);
> +#endif
> +#ifdef CONFIG_SOC_AU1X00
> + driver_unregister(&ehci_hcd_au1xxx_driver);
> +#endif
> +#ifdef CONFIG_PCI
> + pci_unregister_driver(&ehci_pci_driver);
> +#endif
> +}
> +
> +module_init(ehci_hcd_init);
> +module_exit(ehci_hcd_cleanup);
> +
> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
> index 1e03f1a..e0641bc 100644
> --- a/drivers/usb/host/ehci-pci.c
> +++ b/drivers/usb/host/ehci-pci.c
> @@ -370,23 +370,3 @@ static struct pci_driver ehci_pci_driver
> .resume = usb_hcd_pci_resume,
> #endif
> };
> -
> -static int __init ehci_hcd_pci_init(void)
> -{
> - if (usb_disabled())
> - return -ENODEV;
> -
> - pr_debug("%s: block sizes: qh %Zd qtd %Zd itd %Zd sitd %Zd\n",
> - hcd_name,
> - sizeof(struct ehci_qh), sizeof(struct ehci_qtd),
> - sizeof(struct ehci_itd), sizeof(struct ehci_sitd));
> -
> - return pci_register_driver(&ehci_pci_driver);
> -}
> -module_init(ehci_hcd_pci_init);
> -
> -static void __exit ehci_hcd_pci_cleanup(void)
> -{
> - pci_unregister_driver(&ehci_pci_driver);
> -}
> -module_exit(ehci_hcd_pci_cleanup);
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-
> kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
Hi,
Le Fri, 24 Mar 2006 14:32:57 -0600, Kumar Gala a ?crit?:
> +
> +static int __init ehci_hcd_init(void)
> +{
> + int retval = 0;
> +
> + pr_debug("%s: block sizes: qh %Zd qtd %Zd itd %Zd sitd %Zd\n",
> + hcd_name,
> + sizeof(struct ehci_qh), sizeof(struct ehci_qtd),
> + sizeof(struct ehci_itd), sizeof(struct ehci_sitd));
> +
> +#ifdef CONFIG_PPC_83xx
> + retval = platform_driver_register(&ehci_fsl_dr_driver);
> + if (retval < 0)
> + return retval;
This is wrong as the first driver could failed but the following one could
be correct : imagine generic kernel were multiple config options are
enabled.
On some board we could have only one controller, on other both.
> + retval = platform_driver_register(&ehci_fsl_dr_driver);
> + if (retval < 0)
> + return retval;
> +#endif
We should unregister all the previous drivers if we fail.
Matthieu