* [PATCH] usb: host: ehci-tegra: fix Kconfig depencies
@ 2021-01-25 11:32 Arnd Bergmann
2021-01-25 16:22 ` Alan Stern
0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2021-01-25 11:32 UTC (permalink / raw)
To: Greg Kroah-Hartman, Alan Stern, Peter Chen, Dmitry Osipenko,
Thierry Reding
Cc: Arnd Bergmann, linux-usb, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
Selecting the chipidea driver from the old Kconfig symbol
can lead to a missing dependency:
WARNING: unmet direct dependencies detected for USB_CHIPIDEA
Depends on [m]: USB_SUPPORT [=y] && (USB_EHCI_HCD [=y] && USB_GADGET [=m] || USB_EHCI_HCD [=y] && !USB_GADGET [=m] || !USB_EHCI_HCD [=y] && USB_GADGET [=m]) && HAS_DMA [=y]
Selected by [y]:
- USB_EHCI_TEGRA [=y] && USB_SUPPORT [=y] && USB [=y] && USB_EHCI_HCD [=y] && ARCH_TEGRA [=y]
aarch64-linux-ld: drivers/usb/chipidea/otg.o: in function `ci_handle_vbus_change':
otg.c:(.text+0x3c8): undefined reference to `usb_gadget_vbus_connect'
aarch64-linux-ld: otg.c:(.text+0x42c): undefined reference to `usb_gadget_vbus_disconnect'
aarch64-linux-ld: drivers/usb/chipidea/otg.o: in function `ci_otg_work':
otg.c:(.text+0x5d4): undefined reference to `usb_gadget_vbus_disconnect'
...
Duplicate the dependency to ensure that this driver can
only be a loadable module if one of its dependencies is.
Fixes: c3590c7656fb ("usb: host: ehci-tegra: Remove the driver")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/usb/host/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 160e5d3927e2..66b01b619ecd 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -269,6 +269,7 @@ config USB_EHCI_HCD_AT91
config USB_EHCI_TEGRA
tristate "NVIDIA Tegra HCD support"
depends on ARCH_TEGRA
+ depends on ((USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD && !USB_GADGET) || (!USB_EHCI_HCD && USB_GADGET)) && HAS_DMA
select USB_CHIPIDEA
select USB_CHIPIDEA_HOST
select USB_CHIPIDEA_TEGRA
--
2.29.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] usb: host: ehci-tegra: fix Kconfig depencies 2021-01-25 11:32 [PATCH] usb: host: ehci-tegra: fix Kconfig depencies Arnd Bergmann @ 2021-01-25 16:22 ` Alan Stern 2021-01-25 21:10 ` Arnd Bergmann 0 siblings, 1 reply; 4+ messages in thread From: Alan Stern @ 2021-01-25 16:22 UTC (permalink / raw) To: Arnd Bergmann Cc: Greg Kroah-Hartman, Peter Chen, Dmitry Osipenko, Thierry Reding, Arnd Bergmann, linux-usb, linux-kernel On Mon, Jan 25, 2021 at 12:32:30PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > Selecting the chipidea driver from the old Kconfig symbol > can lead to a missing dependency: Arnd: I found this whole patch a little confusing. For example, in the sentence above, what does "the old Kconfig symbol" refer to? Comparing the various Kconfig files, I see what the problem is. The commit which this one fixes made CONFIG_EHCI_TEGRA select CONFIG_USB_CHIPIDEA, but it didn't make EHCI_TEGRA depend on the things that USB_CHIPIDEA depends on. Can you please state this more explicitly in the patch description? > WARNING: unmet direct dependencies detected for USB_CHIPIDEA > Depends on [m]: USB_SUPPORT [=y] && (USB_EHCI_HCD [=y] && USB_GADGET [=m] || USB_EHCI_HCD [=y] && !USB_GADGET [=m] || !USB_EHCI_HCD [=y] && USB_GADGET [=m]) && HAS_DMA [=y] > Selected by [y]: > - USB_EHCI_TEGRA [=y] && USB_SUPPORT [=y] && USB [=y] && USB_EHCI_HCD [=y] && ARCH_TEGRA [=y] > aarch64-linux-ld: drivers/usb/chipidea/otg.o: in function `ci_handle_vbus_change': > otg.c:(.text+0x3c8): undefined reference to `usb_gadget_vbus_connect' > aarch64-linux-ld: otg.c:(.text+0x42c): undefined reference to `usb_gadget_vbus_disconnect' > aarch64-linux-ld: drivers/usb/chipidea/otg.o: in function `ci_otg_work': > otg.c:(.text+0x5d4): undefined reference to `usb_gadget_vbus_disconnect' > ... > > Duplicate the dependency to ensure that this driver can > only be a loadable module if one of its dependencies is. > > Fixes: c3590c7656fb ("usb: host: ehci-tegra: Remove the driver") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/usb/host/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig > index 160e5d3927e2..66b01b619ecd 100644 > --- a/drivers/usb/host/Kconfig > +++ b/drivers/usb/host/Kconfig > @@ -269,6 +269,7 @@ config USB_EHCI_HCD_AT91 > config USB_EHCI_TEGRA > tristate "NVIDIA Tegra HCD support" > depends on ARCH_TEGRA > + depends on ((USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD && !USB_GADGET) || (!USB_EHCI_HCD && USB_GADGET)) && HAS_DMA > select USB_CHIPIDEA > select USB_CHIPIDEA_HOST > select USB_CHIPIDEA_TEGRA Isn't there at least one other missing dependency? This entry selects USB_CHIPIDEA_TEGRA, which depends on OF. So shouldn't this entry also depend on OF? Or does the Kconfig system detect that for us? Also, while I'm no expert on the Kconfig language, it seems that the new "depends" line could be a lot easier to understand if it was refactored with some comments added. Yes, I realize you just copied the existing dependency from the USB_CHIPIDEA entry -- that one could stand to be cleaned up as well. For instance, how about putting the HAS_DMA part into a separate line, since it's unrelated to the other stuff? And the rest looks like it could be changed to: depends on USB_EHCI_HCD || USB_GADGET although that probably isn't quite valid. Still, can't it be changed to something simpler than (USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD && !USB_GADGET) || (!USB_EHCI_HCD && USB_GADGET) ? Alan Stern ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] usb: host: ehci-tegra: fix Kconfig depencies 2021-01-25 16:22 ` Alan Stern @ 2021-01-25 21:10 ` Arnd Bergmann 2021-03-09 15:09 ` Dmitry Osipenko 0 siblings, 1 reply; 4+ messages in thread From: Arnd Bergmann @ 2021-01-25 21:10 UTC (permalink / raw) To: Alan Stern Cc: Greg Kroah-Hartman, Peter Chen, Dmitry Osipenko, Thierry Reding, Arnd Bergmann, USB list, linux-kernel On Mon, Jan 25, 2021 at 5:22 PM Alan Stern <stern@rowland.harvard.edu> wrote: > > On Mon, Jan 25, 2021 at 12:32:30PM +0100, Arnd Bergmann wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > > > Selecting the chipidea driver from the old Kconfig symbol > > can lead to a missing dependency: > > Arnd: > > I found this whole patch a little confusing. For example, in the > sentence above, what does "the old Kconfig symbol" refer to? > > Comparing the various Kconfig files, I see what the problem is. The > commit which this one fixes made CONFIG_EHCI_TEGRA select > CONFIG_USB_CHIPIDEA, but it didn't make EHCI_TEGRA depend on the things > that USB_CHIPIDEA depends on. Can you please state this more explicitly > in the patch description? Sure, I'll resend in a bit. > > WARNING: unmet direct dependencies detected for USB_CHIPIDEA > > Depends on [m]: USB_SUPPORT [=y] && (USB_EHCI_HCD [=y] && USB_GADGET [=m] || USB_EHCI_HCD [=y] && !USB_GADGET [=m] || !USB_EHCI_HCD [=y] && USB_GADGET [=m]) && HAS_DMA [=y] > > Selected by [y]: > > - USB_EHCI_TEGRA [=y] && USB_SUPPORT [=y] && USB [=y] && USB_EHCI_HCD [=y] && ARCH_TEGRA [=y] > > aarch64-linux-ld: drivers/usb/chipidea/otg.o: in function `ci_handle_vbus_change': > > otg.c:(.text+0x3c8): undefined reference to `usb_gadget_vbus_connect' > > aarch64-linux-ld: otg.c:(.text+0x42c): undefined reference to `usb_gadget_vbus_disconnect' > > aarch64-linux-ld: drivers/usb/chipidea/otg.o: in function `ci_otg_work': > > otg.c:(.text+0x5d4): undefined reference to `usb_gadget_vbus_disconnect' > > ... > > > > Duplicate the dependency to ensure that this driver can > > only be a loadable module if one of its dependencies is. > > > > Fixes: c3590c7656fb ("usb: host: ehci-tegra: Remove the driver") > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > --- > > drivers/usb/host/Kconfig | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig > > index 160e5d3927e2..66b01b619ecd 100644 > > --- a/drivers/usb/host/Kconfig > > +++ b/drivers/usb/host/Kconfig > > @@ -269,6 +269,7 @@ config USB_EHCI_HCD_AT91 > > config USB_EHCI_TEGRA > > tristate "NVIDIA Tegra HCD support" > > depends on ARCH_TEGRA > > + depends on ((USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD && !USB_GADGET) || (!USB_EHCI_HCD && USB_GADGET)) && HAS_DMA > > select USB_CHIPIDEA > > select USB_CHIPIDEA_HOST > > select USB_CHIPIDEA_TEGRA > > Isn't there at least one other missing dependency? This entry selects > USB_CHIPIDEA_TEGRA, which depends on OF. So shouldn't this entry also > depend on OF? Or does the Kconfig system detect that for us? Yes, there is a hard dependency on ARCH_TEGRA, which implies OF. > Also, while I'm no expert on the Kconfig language, it seems that the new > "depends" line could be a lot easier to understand if it was refactored > with some comments added. Yes, I realize you just copied the existing > dependency from the USB_CHIPIDEA entry -- that one could stand to be > cleaned up as well. > > For instance, how about putting the HAS_DMA part into a separate line, > since it's unrelated to the other stuff? Actually it's probably best to just drop the HAS_DMA here, as it is also implied by ARCH_TEGRA. > And the rest looks like it > could be changed to: > > depends on USB_EHCI_HCD || USB_GADGET > > although that probably isn't quite valid. Still, can't it be changed to > something simpler than > > (USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD && !USB_GADGET) || > (!USB_EHCI_HCD && USB_GADGET) The problem is that if either USB_EHCI_HCD or USB_GADGET are loadable modules, then USB_CHIPIDEA must not be built-in, but if one of the two is disabled, we must still have a dependency on the other. I guess it could be rewritten into depends on USB_EHCI_HCD || USB_GADGET # needs at least one of the two depends on m || USB_EHCI_HCD!=m # must be =m if USB_EHCI_HCD=m depends on m || USB_GADGET!=m # must be =m if USB_GADGET=m I see that USB_EHCI_TEGRA already depends on USB_EHCI_HCD, so I think this collapses into a much simpler depends on USB_GADGET || USB_GADGET=n # for USB_CHIPIDEA Arnd ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] usb: host: ehci-tegra: fix Kconfig depencies 2021-01-25 21:10 ` Arnd Bergmann @ 2021-03-09 15:09 ` Dmitry Osipenko 0 siblings, 0 replies; 4+ messages in thread From: Dmitry Osipenko @ 2021-03-09 15:09 UTC (permalink / raw) To: Arnd Bergmann, Alan Stern Cc: Greg Kroah-Hartman, Peter Chen, Thierry Reding, Arnd Bergmann, USB list, linux-kernel 26.01.2021 00:10, Arnd Bergmann пишет: > On Mon, Jan 25, 2021 at 5:22 PM Alan Stern <stern@rowland.harvard.edu> wrote: >> >> On Mon, Jan 25, 2021 at 12:32:30PM +0100, Arnd Bergmann wrote: >>> From: Arnd Bergmann <arnd@arndb.de> >>> >>> Selecting the chipidea driver from the old Kconfig symbol >>> can lead to a missing dependency: >> >> Arnd: >> >> I found this whole patch a little confusing. For example, in the >> sentence above, what does "the old Kconfig symbol" refer to? >> >> Comparing the various Kconfig files, I see what the problem is. The >> commit which this one fixes made CONFIG_EHCI_TEGRA select >> CONFIG_USB_CHIPIDEA, but it didn't make EHCI_TEGRA depend on the things >> that USB_CHIPIDEA depends on. Can you please state this more explicitly >> in the patch description? > > Sure, I'll resend in a bit. > >>> WARNING: unmet direct dependencies detected for USB_CHIPIDEA >>> Depends on [m]: USB_SUPPORT [=y] && (USB_EHCI_HCD [=y] && USB_GADGET [=m] || USB_EHCI_HCD [=y] && !USB_GADGET [=m] || !USB_EHCI_HCD [=y] && USB_GADGET [=m]) && HAS_DMA [=y] >>> Selected by [y]: >>> - USB_EHCI_TEGRA [=y] && USB_SUPPORT [=y] && USB [=y] && USB_EHCI_HCD [=y] && ARCH_TEGRA [=y] >>> aarch64-linux-ld: drivers/usb/chipidea/otg.o: in function `ci_handle_vbus_change': >>> otg.c:(.text+0x3c8): undefined reference to `usb_gadget_vbus_connect' >>> aarch64-linux-ld: otg.c:(.text+0x42c): undefined reference to `usb_gadget_vbus_disconnect' >>> aarch64-linux-ld: drivers/usb/chipidea/otg.o: in function `ci_otg_work': >>> otg.c:(.text+0x5d4): undefined reference to `usb_gadget_vbus_disconnect' >>> ... >>> >>> Duplicate the dependency to ensure that this driver can >>> only be a loadable module if one of its dependencies is. >>> >>> Fixes: c3590c7656fb ("usb: host: ehci-tegra: Remove the driver") >>> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >>> --- >>> drivers/usb/host/Kconfig | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig >>> index 160e5d3927e2..66b01b619ecd 100644 >>> --- a/drivers/usb/host/Kconfig >>> +++ b/drivers/usb/host/Kconfig >>> @@ -269,6 +269,7 @@ config USB_EHCI_HCD_AT91 >>> config USB_EHCI_TEGRA >>> tristate "NVIDIA Tegra HCD support" >>> depends on ARCH_TEGRA >>> + depends on ((USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD && !USB_GADGET) || (!USB_EHCI_HCD && USB_GADGET)) && HAS_DMA >>> select USB_CHIPIDEA >>> select USB_CHIPIDEA_HOST >>> select USB_CHIPIDEA_TEGRA >> >> Isn't there at least one other missing dependency? This entry selects >> USB_CHIPIDEA_TEGRA, which depends on OF. So shouldn't this entry also >> depend on OF? Or does the Kconfig system detect that for us? > > Yes, there is a hard dependency on ARCH_TEGRA, which implies OF. > >> Also, while I'm no expert on the Kconfig language, it seems that the new >> "depends" line could be a lot easier to understand if it was refactored >> with some comments added. Yes, I realize you just copied the existing >> dependency from the USB_CHIPIDEA entry -- that one could stand to be >> cleaned up as well. >> >> For instance, how about putting the HAS_DMA part into a separate line, >> since it's unrelated to the other stuff? > > Actually it's probably best to just drop the HAS_DMA here, as it is also > implied by ARCH_TEGRA. > >> And the rest looks like it >> could be changed to: >> >> depends on USB_EHCI_HCD || USB_GADGET >> >> although that probably isn't quite valid. Still, can't it be changed to >> something simpler than >> >> (USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD && !USB_GADGET) || >> (!USB_EHCI_HCD && USB_GADGET) > > The problem is that if either USB_EHCI_HCD or USB_GADGET are loadable > modules, then USB_CHIPIDEA must not be built-in, but if one of the two is > disabled, we must still have a dependency on the other. I guess it could be > rewritten into > > depends on USB_EHCI_HCD || USB_GADGET # needs at least one of the two > depends on m || USB_EHCI_HCD!=m # must be =m if USB_EHCI_HCD=m > depends on m || USB_GADGET!=m # must be =m if USB_GADGET=m > > I see that USB_EHCI_TEGRA already depends on USB_EHCI_HCD, > so I think this collapses into a much simpler > > depends on USB_GADGET || USB_GADGET=n # for USB_CHIPIDEA > > Arnd > It looks like the Chipidea's Kconfig is the problem here because it unconditionally compiles otg.c which uses usb_gadget_* API, but the Kconfig doesn't select nor depend on the USB_GADGET properly. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-09 15:10 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-25 11:32 [PATCH] usb: host: ehci-tegra: fix Kconfig depencies Arnd Bergmann 2021-01-25 16:22 ` Alan Stern 2021-01-25 21:10 ` Arnd Bergmann 2021-03-09 15:09 ` Dmitry Osipenko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.