Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: Looking for help with Kconfig dependencies
To:     Matthias Kaehlcke <mka@chromium.org>,
        Masahiro Yamada <masahiroy@kernel.org>
Cc:     linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Douglas Anderson <dianders@chromium.org>,
        linux-usb@vger.kernel.org
References: <YMzSbDL+XvpLPaTb@google.com>
From:   "Enrico Weigelt, metux IT consult" <lkml@metux.net>
Message-ID: <dc68833d-e525-eeda-5c7c-fbbd8a3287c8@metux.net>
Date:   Mon, 21 Jun 2021 13:26:01 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <YMzSbDL+XvpLPaTb@google.com>
Content-Type: text/plain; charset=utf-8
Content-Language: tl
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 18.06.21 19:05, Matthias Kaehlcke wrote:

Hi,


Cc'ing to linux-usb ...

> Patch https://lore.kernel.org/patchwork/patch/1444212/ adds the new
> onboard_usb_hub driver which exports two functions,
> onboard_hub_create_pdevs() and onboard_hub_destroy_pdevs(). It also
> provides stubs for these functions which are used when the driver
> is not selected (CONFIG_USB_ONBOARD_HUB=n).
> 
> The new exported functions are called by the xhci-plat driver
> (https://lore.kernel.org/patchwork/patch/1444215/). Since xhci-plat
> now depends on symbols from the onboard_hub_driver the following
> dependency was added to its Kconfig entry:
> 
>   config USB_XHCI_PLATFORM
>     tristate "Generic xHCI driver for a platform device"
>     select USB_XHCI_RCAR if ARCH_RENESAS
>  +  depends on USB_ONBOARD_HUB || !USB_ONBOARD_HUB

What exactly do you intent to archieve with this ?

X or !X = 1, isn't it ?

Why should something depend on something present or absent ?

Is that depends on ... statement necessary at all ?

> This generally seems to work, however when USB_XHCI_PLATFORM is
> forced to be builtin by another driver that depends on it (e.g.
> USB_DWC3) it is still possible to build the onboard_hub driver
> as a module, which results in unresolved symbols:
> 
> aarch64-linux-gnu-ld: drivers/usb/host/xhci-plat.o: in function
> `xhci_plat_remove':
> drivers/usb/host/xhci-plat.c:427: undefined reference to
> `onboard_hub_destroy_pdevs'
> drivers/usb/host/xhci-plat.c:427:(.text+0x82c): relocation truncated
> to fit: R_AARCH64_CALL26 against undefined symbol
> `onboard_hub_destroy_pdevs'
> aarch64-linux-gnu-ld: drivers/usb/host/xhci-plat.o: in function
> `xhci_plat_probe':
> drivers/usb/host/xhci-plat.c:379: undefined reference to
> `onboard_hub_create_pdevs'
> drivers/usb/host/xhci-plat.c:379:(.text+0x131c): relocation truncated
> to fit: R_AARCH64_CALL26 against undefined symbol
> `onboard_hub_create_pdevs'
> 
> Kconfig generates the following warning with this configuration:
> 
> WARNING: unmet direct dependencies detected for USB_XHCI_PLATFORM
>   Depends on [m]: USB_SUPPORT [=y] && USB [=y] && USB_XHCI_HCD [=y] && (USB_ONBOARD_HUB [=m] || !USB_ONBOARD_HUB [=m])
>   Selected by [y]:
>   - USB_DWC3 [=y] && USB_SUPPORT [=y] && (USB [=y] || USB_GADGET [=y]) && HAS_DMA [=y] && USB_XHCI_HCD [=y]
>   Selected by [m]:
>   - USB_CDNS_SUPPORT [=m] && USB_SUPPORT [=y] && (USB [=y] || USB_GADGET [=y]) && HAS_DMA [=y] && USB_XHCI_HCD [=y]
>   - USB_BRCMSTB [=m] && USB_SUPPORT [=y] && USB [=y] && (ARCH_BRCMSTB [=y] && PHY_BRCM_USB [=m] || COMPILE_TEST [=y]) && USB_XHCI_HCD [=y]
>   - USB_XHCI_MVEBU [=m] && USB_SUPPORT [=y] && USB [=y] && USB_XHCI_HCD [=y] && HAS_IOMEM [=y] && (ARCH_MVEBU [=y] || COMPILE_TEST [=y])

It seems that Kconfig is confused by trying to enforce contradicting
dependencies.


Now for your driver:

If I understand it correctly, you've got a topology like this:


root hub -+--> 2ndary hub #0 -+--> usb-dev #0
          |                   \--> usb-dev #1
          |                     ..
          \--> 2ndary hub #1 -+--> usb-dev #3
                              \--> usb-dev #4


And in order to get usb-dev #foo running, you need the corresponding
hub on its path powered (which in turn is platform specific).

Correct ?

So, why not reflecting exactly this topology in the device tree ?
In that case, the power management *IMHO* could pretty automatically
(assuming you've implemented the corresponding pm functions on the
2ndary hub driver).

Okay, that could become a bit tricky when the usb-dev's are
automatically enumerated on the root hub and would need to be
reparented somehow ... @usb folks: it that possible ?

Another option could be implementing this as a regulator that the
individual usb devices will be attached to. Not completely semantically
correct (since a hub isn't exactly a regulator :o), but should at least
do the job: the regulator will be switched on when the device is used
and can be switched off when it isn't used anymore.

The cleanest approach, IMHO, might be adding an hub subsys, somewhat
similar to the existing phy subsys. I can imagine similar cases with
other interfaces, not just USB only, at least certainly not specific
to xhci.

Or could existing phy subsys already be sufficient for that ?


--mtx

-- 
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287