[PATCH] usb: gadget: Compile USB ethernet gadget only if NET is enabled

[Tom Rini]

On Wed, May 03, 2023 at 11:43:57PM +0200, Marek Vasut wrote:
> On 5/1/23 23:18, Tom Rini wrote:
> > On Mon, May 01, 2023 at 10:49:37PM +0200, Marek Vasut wrote:
> > > On 5/1/23 20:53, Tom Rini wrote:
> > > > On Mon, May 01, 2023 at 07:40:57PM +0200, Marek Vasut wrote:
> > > > > On 5/1/23 19:23, Tom Rini wrote:
> > > > > > On Mon, May 01, 2023 at 06:53:52PM +0200, Marek Vasut wrote:
> > > > > > > On 5/1/23 15:47, Tom Rini wrote:
> > > > > > > > On Sun, Apr 30, 2023 at 11:20:35PM +0200, Marek Vasut wrote:
> > > > > > > > 
> > > > > > > > > In case NET networking is not enabled, it is not possible to compile
> > > > > > > > > the USB ethernet gadget. Protect the symbols in Makefile to avoid build
> > > > > > > > > failure. Such build failure may occur e.g. in case NET and USB ethernet
> > > > > > > > > gadget is enabled in U-Boot proper, but not in SPL.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Marek Vasut <marex at denx.de>
> > > > > > > > > ---
> > > > > > > > > Cc: Lukasz Majewski <lukma at denx.de>
> > > > > > > > > ---
> > > > > > > > >      drivers/usb/gadget/Makefile | 2 ++
> > > > > > > > >      1 file changed, 2 insertions(+)
> > > > > > > > > 
> > > > > > > > > diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> > > > > > > > > index 6cfe0f3a041..36f65e7eb95 100644
> > > > > > > > > --- a/drivers/usb/gadget/Makefile
> > > > > > > > > +++ b/drivers/usb/gadget/Makefile
> > > > > > > > > @@ -34,8 +34,10 @@ endif
> > > > > > > > >      obj-$(CONFIG_CI_UDC) += ci_udc.o
> > > > > > > > > +ifeq ($(CONFIG_$(SPL_TPL_)NET),y)
> > > > > > > > >      obj-$(CONFIG_USB_ETHER) += ether.o
> > > > > > > > >      obj-$(CONFIG_USB_ETH_RNDIS) += rndis.o
> > > > > > > > > +endif
> > > > > > > > >      # Devices not related to the new gadget layer depend on CONFIG_USB_DEVICE
> > > > > > > > >      # This is really only N900 and USBTTY now.
> > > > > > > > 
> > > > > > > > Why can't we just enforce this via Kconfig?
> > > > > > > 
> > > > > > > Because there is no SPL/TPL USB_ETHER Kconfig .
> > > > > > > Do we want to grow the Kconfig file with those instead ?
> > > > > > 
> > > > > > Ah right.  Yes, we have SPL_USB_ETHER today
> > > > > 
> > > > > This is exactly the opposite of what I wrote.
> > > > > 
> > > > > And no, we do NOT have this symbol, see:
> > > > > 
> > > > > $ git grep SPL_USB_ETHER drivers/usb | wc -l
> > > > > 0
> > > > 
> > > > Yes, it resides in common/spl/Kconfig
> > > 
> > > Uhhhhh, such USB symbol is not supposed to be there in the first place.
> > 
> > There's long running debate on where some symbols should be for a better
> > overall experience of enabling features.
> 
> Putting my USB maintainer hat on, USB drivers related symbols should be in
> drivers/usb/ .

With everything in Kconfig finally, I have no objection to someone
making patches to clean up and make the menus more consistent.  However
you'd like to move all of SPL*USB* out of common/spl/Kconfig and under
drivers/usb/ is OK with me.

> > > However, looking at the meaning of that symbol, it seems it governs
> > > something else -- USB ethernet device(s) (like the USB-ethernet adapter
> > > which you plug into USB host port). So, the SPL_USB_ETHER symbol name is
> > > incorrectly named too and should be renamed to something else first I think
> > > ?
> > > 
> > > Do I read it right ?
> > > 
> > > And if so, then, back to my original reply -- there is no SPL_USB_ETHER
> > > symbol. Does it make more sense to really add one (not misnamed one) or not
> > > ?
> > 
> > We should re-work things as needed so that we have more Kconfig symbols,
> > as needed.  There's some overlap / overloading of things today as
> > platforms have been able to (and I think am335x_evm is still one that
> > builds for) supporting making a USB RNDIS gadget device happen in SPL,
> > so that we can then be fed the next stage of U-Boot.
> 
> I have a hardware where I only want USB ethernet support in U-Boot, not in
> SPL, hence this patch.
> 
> But the symbol which is currently named SPL_USB_ETHER as defined in common/
> does NOT ungate SPL CDC ethernet support, it ungates SPL USB host ethernet
> (like those ASIX USB-ethernet adapters), do you agree ?
> 
> If yes, then the symbol itself should be renamed .

What I see in terms of wording in common/spl/Kconfig doesn't match what
it builds, which is what it's intended to build.  Fire up a build of
am335x_evm_defconfig and look at what's under drivers/usb/ which is
what the platform wants.  Gadget ethernet via RNDIS.  So the help is
wrong on SPL_USB_ETHER.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20230503/bd4c1d92/attachment.sig>