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

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

[Marek Vasut]

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@denx.de>
---
Cc: Lukasz Majewski <lukma@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.
-- 
2.39.2

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1106 bytes --]

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@denx.de>
> ---
> Cc: Lukasz Majewski <lukma@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?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Marek Vasut]

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@denx.de>
>> ---
>> Cc: Lukasz Majewski <lukma@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 ?

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1575 bytes --]

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@denx.de>
> > > ---
> > > Cc: Lukasz Majewski <lukma@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, and could just add
SPL_USB_ETH_RNDIS.  Are there TPL users today for this? There's no
TPL_USB at all.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Marek Vasut]

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@denx.de>
>>>> ---
>>>> Cc: Lukasz Majewski <lukma@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

, and could just add
> SPL_USB_ETH_RNDIS.  Are there TPL users today for this? There's no
> TPL_USB at all.

[...]

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1914 bytes --]

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@denx.de>
> > > > > ---
> > > > > Cc: Lukasz Majewski <lukma@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

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Marek Vasut]

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@denx.de>
>>>>>> ---
>>>>>> Cc: Lukasz Majewski <lukma@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.

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 ?

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3239 bytes --]

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@denx.de>
> > > > > > > ---
> > > > > > > Cc: Lukasz Majewski <lukma@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.

> 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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Marek Vasut]

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@denx.de>
>>>>>>>> ---
>>>>>>>> Cc: Lukasz Majewski <lukma@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/ .

>> 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 .

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4663 bytes --]

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@denx.de>
> > > > > > > > > ---
> > > > > > > > > Cc: Lukasz Majewski <lukma@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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Lukasz Majewski]

[-- Attachment #1: Type: text/plain, Size: 1364 bytes --]

On Sun, 30 Apr 2023 23:20:35 +0200
Marek Vasut <marex@denx.de> 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@denx.de>
> ---
> Cc: Lukasz Majewski <lukma@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.

Reviewed-by: Lukasz Majewski <lukma@denx.de>


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Marek Vasut]

On 5/4/23 11:04, Lukasz Majewski wrote:
> On Sun, 30 Apr 2023 23:20:35 +0200
> Marek Vasut <marex@denx.de> 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@denx.de>
>> ---
>> Cc: Lukasz Majewski <lukma@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.
> 
> Reviewed-by: Lukasz Majewski <lukma@denx.de>

NAK

(so this does not get applied by accident, because ongoing discussion 
about Kconfig symbols)