usb: host: ehci-tegra: fix Kconfig depencies

From: Arnd Bergmann <[email protected]>

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 <[email protected]>

---
 drivers/usb/host/Kconfig | 1 +
 1 file changed, 1 insertion(+)

drivers/usb/host/Kconfig +0 -0

index 160e5d3927e2..66b01b619ecd 100644

Quoted 4 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

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?

Quoted 20 lines

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 <[email protected]>

---
 drivers/usb/host/Kconfig | 1 +
 1 file changed, 1 insertion(+)

drivers/usb/host/Kconfig config USB_EHCI_HCD_AT91 · line 275

Quoted 12 lines

drivers/usb/host/Kconfig +1 -0

index 160e5d3927e2..66b01b619ecd 100644
@@ -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)

Quoted 9 lines · Arnd Bergmann

Quoted 4 lines

From: Arnd Bergmann <[email protected]>

Selecting the chipidea driver from the old Kconfig symbol can lead to a missing dependency:

Arnd:

Quoted 8 lines

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.

Quoted 20 lines

Quoted 20 lines

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 <[email protected]>

---
 drivers/usb/host/Kconfig | 1 +
 1 file changed, 1 insertion(+)

Quoted 16 lines

drivers/usb/host/Kconfig config USB_EHCI_HCD_AT91 · line 275

Quoted 12 lines

drivers/usb/host/Kconfig +1 -0

index 160e5d3927e2..66b01b619ecd 100644
@@ -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.

Quoted 8 lines

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.

Quoted 10 lines

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