usb: fotg210: fix OTG-only build

Commit Message

Arnd Bergmann Dec. 15, 2022, 4:57 p.m. UTC

From: Arnd Bergmann <[e-mail]>

The fotg210 module combines the HCD and OTG drivers, which then
fails to build when only the USB gadget support is enabled
in the kernel but host support is not:

aarch64-linux-ld: drivers/usb/fotg210/fotg210-core.o: in function `fotg210_init':
fotg210-core.c:(.init.text+0xc): undefined reference to `usb_disabled'

Move the check for usb_disabled() after the check for the HCD module,
and let the OTG driver still be probed in this configuration.

A nicer approach might be to have the common portion built as a
library module, with the two platform other files registering
their own platform_driver instances separately.

Fixes: ddacd6ef44ca ("usb: fotg210: Fix Kconfig for USB host modules")
Signed-off-by: Arnd Bergmann <[e-mail]>
---
 drivers/usb/fotg210/fotg210-core.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Comments

Linus Walleij Dec. 16, 2022, 7:44 a.m. UTC | #1

On Thu, Dec 15, 2022 at 5:57 PM Arnd Bergmann <[e-mail]> wrote:

> From: Arnd Bergmann <[e-mail]>
>
> The fotg210 module combines the HCD and OTG drivers, which then
> fails to build when only the USB gadget support is enabled
> in the kernel but host support is not:
>
> aarch64-linux-ld: drivers/usb/fotg210/fotg210-core.o: in function `fotg210_init':
> fotg210-core.c:(.init.text+0xc): undefined reference to `usb_disabled'
>
> Move the check for usb_disabled() after the check for the HCD module,
> and let the OTG driver still be probed in this configuration.
>
> A nicer approach might be to have the common portion built as a
> library module, with the two platform other files registering
> their own platform_driver instances separately.
>
> Fixes: ddacd6ef44ca ("usb: fotg210: Fix Kconfig for USB host modules")
> Signed-off-by: Arnd Bergmann <[e-mail]>

My mistake, I thought this function was a generic USB function
and not related to host. I might just push it down to the host again.

Reviewed-by: Linus Walleij <[e-mail]>

Yours,
Linus Walleij

Daniel Palmer Jan. 2, 2023, 6:10 a.m. UTC | #2

Hi Linus,

Maybe a little unrelated to this patch but this IP is also used on the
MStar/SigmaStar chips (albeit only the host (FUSBH200) part that used
to have its own driver that got removed at some point...) and I
noticed this when rebasing my tree and hitting conflicts with your
recent changes...

For what it's worth I could not get this driver to function correctly
with UVC cameras etc and was fed up with hacking it apart to try to
get it to work when it's mostly a copy/paste of an old copy of the
common EHCI driver.
So I added some extra quirks to the common EHCI driver to allow it to
run this IP too and deleted the custom driver in my tree (hence the
conflicts..).

I'm not sure how using the common EHCI driver works with the OTG part
but if it's possible to make that work maybe my way is a better
solution than trying to maintain this driver?

Cheers,

Daniel

Linus Walleij Jan. 4, 2023, 12:14 a.m. UTC | #3

On Mon, Jan 2, 2023 at 7:10 AM Daniel Palmer <[e-mail]> wrote:

> Maybe a little unrelated to this patch but this IP is also used on the
> MStar/SigmaStar chips

I actually think Arnd Bergmann at one point sent me a device with
this line of chips in it. I suppose you will be upstreaming this
support?

> (albeit only the host (FUSBH200) part that used
> to have its own driver that got removed at some point...)

Sorry for reading this after sending out my latest changes, here:
https://lore.kernel.org/linux-usb/[e-mail]/T/#t

The first patch in this series differentiates between
FOTG200 and FOTG210, do you mean to say there is
rather FUSBH200 as well? There is also FUSB220...
(Faraday used to have datasheets on their webpage
but I can't access them anymore.)

> For what it's worth I could not get this driver to function correctly
> with UVC cameras etc and was fed up with hacking it apart to try to
> get it to work when it's mostly a copy/paste of an old copy of the
> common EHCI driver.
> So I added some extra quirks to the common EHCI driver to allow it to
> run this IP too and deleted the custom driver in my tree (hence the
> conflicts..).
>
> I'm not sure how using the common EHCI driver works with the OTG part
> but if it's possible to make that work maybe my way is a better
> solution than trying to maintain this driver?

That's a good point, do you have a pointer to your EHCI
quirk patch so I can take a look?

Yours,
Linus Walleij

Daniel Palmer Jan. 4, 2023, 7:56 a.m. UTC | #4

Hi Linus,

On Wed, 4 Jan 2023 at 09:14, Linus Walleij <[e-mail]> wrote:
>
> On Mon, Jan 2, 2023 at 7:10 AM Daniel Palmer <[e-mail]> wrote:
>
> > Maybe a little unrelated to this patch but this IP is also used on the
> > MStar/SigmaStar chips
>
> I actually think Arnd Bergmann at one point sent me a device with
> this line of chips in it. I suppose you will be upstreaming this
> support?

We've got some of it upstreamed already.
We actually have most of these chips working now but the progress is
slow because it's a hobby/spare time thing..

> The first patch in this series differentiates between
> FOTG200 and FOTG210, do you mean to say there is
> rather FUSBH200 as well?

Ah this is real confusing. There is an FUSBH200 that I thought was the
FOTG210 without the device (OTG) registers.
I think they all are probably fairly close though.

Here's the series that added the FUSBH200 to the kernel:
https://lore.kernel.org/all/[e-mail]/

> > I'm not sure how using the common EHCI driver works with the OTG part
> > but if it's possible to make that work maybe my way is a better
> > solution than trying to maintain this driver?
>
> That's a good point, do you have a pointer to your EHCI
> quirk patch so I can take a look?

I don't have a clean series but here are some links to the patches I
needed to get something that worked with UVC cameras and ethernet
dongles.
There are some bits of platform garbage and working out mixed in. I
can clean it up into a proper series if it's worth doing.

https://github.com/linux-chenxing/linux/commit/cafda0a6588c125d93a513b3a86f26f287c68fa3
https://github.com/linux-chenxing/linux/commit/f628cc1e915b4c7beb37a33e1a255e88cc90e609
https://github.com/linux-chenxing/linux/commit/a03f6312e4cba20ae5058822ae57e94ba989475d
https://github.com/linux-chenxing/linux/commit/f4f10314c23370376f2bcffa653709535775bcd8
https://github.com/linux-chenxing/linux/commit/fc8e4548b89d5c984284f5c085046be37889bc88
https://github.com/linux-chenxing/linux/commit/2a6ef17ad41a7228a13756d50ac1449862857b64

Cheers,

Daniel

Patch

diff --git a/drivers/usb/fotg210/fotg210-core.c b/drivers/usb/fotg210/fotg210-core.c
index 8a54edf921ac..ee740a6da463 100644
--- a/drivers/usb/fotg210/fotg210-core.c
+++ b/drivers/usb/fotg210/fotg210-core.c
@@ -144,10 +144,7 @@  static struct platform_driver fotg210_driver = {
 
 static int __init fotg210_init(void)
 {
-	if (usb_disabled())
-		return -ENODEV;
-
-	if (IS_ENABLED(CONFIG_USB_FOTG210_HCD))
+	if (IS_ENABLED(CONFIG_USB_FOTG210_HCD) && !usb_disabled())
 		fotg210_hcd_init();
 	return platform_driver_register(&fotg210_driver);
 }