[PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Jonas Karlman]

Build fail with the following error when DM_USB_GADGET is enabled and
USB_DWC3_GADGET is disabled:

  dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
  dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
    undefined reference to `dwc3_gadget_uboot_handle_interrupt'

Build also fail with the following error when USB_GADGET_DWC2_OTG +
DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled:

  gadget/dwc2_udc_otg.o: in function `dm_usb_gadget_handle_interrupts':
  gadget/dwc2_udc_otg.c:947: multiple definition of `dm_usb_gadget_handle_interrupts';
    dwc3/dwc3-generic.o:dwc3/dwc3-generic.c:197: first defined here

Fix this by checking for USB_DWC3_GADGET in addition to DM_USB_GADGET.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
---
 drivers/usb/dwc3/dwc3-generic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
index 6fb2de8a5ace..891d01957619 100644
--- a/drivers/usb/dwc3/dwc3-generic.c
+++ b/drivers/usb/dwc3/dwc3-generic.c
@@ -192,7 +192,7 @@ static int dwc3_generic_of_to_plat(struct udevice *dev)
 	return 0;
 }
 
-#if CONFIG_IS_ENABLED(DM_USB_GADGET)
+#if IS_ENABLED(CONFIG_USB_DWC3_GADGET) && CONFIG_IS_ENABLED(DM_USB_GADGET)
 int dm_usb_gadget_handle_interrupts(struct udevice *dev)
 {
 	struct dwc3_generic_priv *priv = dev_get_priv(dev);
@@ -435,7 +435,7 @@ static int dwc3_glue_bind_common(struct udevice *parent, ofnode node)
 	if (!dr_mode)
 		dr_mode = usb_get_dr_mode(node);
 
-	if (CONFIG_IS_ENABLED(DM_USB_GADGET) &&
+	if (IS_ENABLED(CONFIG_USB_DWC3_GADGET) && CONFIG_IS_ENABLED(DM_USB_GADGET) &&
 	    (dr_mode == USB_DR_MODE_PERIPHERAL || dr_mode == USB_DR_MODE_OTG)) {
 		debug("%s: dr_mode: OTG or Peripheral\n", __func__);
 		driver = "dwc3-generic-peripheral";
-- 
2.43.0

Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Tom Rini]

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

On Sun, Feb 25, 2024 at 03:27:13PM +0000, Jonas Karlman wrote:

> Build fail with the following error when DM_USB_GADGET is enabled and
> USB_DWC3_GADGET is disabled:
> 
>   dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
>   dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
>     undefined reference to `dwc3_gadget_uboot_handle_interrupt'
> 
> Build also fail with the following error when USB_GADGET_DWC2_OTG +
> DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled:
> 
>   gadget/dwc2_udc_otg.o: in function `dm_usb_gadget_handle_interrupts':
>   gadget/dwc2_udc_otg.c:947: multiple definition of `dm_usb_gadget_handle_interrupts';
>     dwc3/dwc3-generic.o:dwc3/dwc3-generic.c:197: first defined here
> 
> Fix this by checking for USB_DWC3_GADGET in addition to DM_USB_GADGET.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

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

Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Marek Vasut]

On 2/25/24 4:27 PM, Jonas Karlman wrote:
> Build fail with the following error when DM_USB_GADGET is enabled and
> USB_DWC3_GADGET is disabled:
> 
>    dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
>    dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
>      undefined reference to `dwc3_gadget_uboot_handle_interrupt'
> 
> Build also fail with the following error when USB_GADGET_DWC2_OTG +
> DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled:
> 
>    gadget/dwc2_udc_otg.o: in function `dm_usb_gadget_handle_interrupts':
>    gadget/dwc2_udc_otg.c:947: multiple definition of `dm_usb_gadget_handle_interrupts';
>      dwc3/dwc3-generic.o:dwc3/dwc3-generic.c:197: first defined here
> 
> Fix this by checking for USB_DWC3_GADGET in addition to DM_USB_GADGET.
> 
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> ---
>   drivers/usb/dwc3/dwc3-generic.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
> index 6fb2de8a5ace..891d01957619 100644
> --- a/drivers/usb/dwc3/dwc3-generic.c
> +++ b/drivers/usb/dwc3/dwc3-generic.c
> @@ -192,7 +192,7 @@ static int dwc3_generic_of_to_plat(struct udevice *dev)
>   	return 0;
>   }
>   
> -#if CONFIG_IS_ENABLED(DM_USB_GADGET)
> +#if IS_ENABLED(CONFIG_USB_DWC3_GADGET) && CONFIG_IS_ENABLED(DM_USB_GADGET)

Maybe just make USB_DWC3_GADGET depend on (or select?) DM_USB_GADGET , 
since I think the DWC3 code doesn't work without DM anyway .

>   int dm_usb_gadget_handle_interrupts(struct udevice *dev)
>   {
>   	struct dwc3_generic_priv *priv = dev_get_priv(dev);
> @@ -435,7 +435,7 @@ static int dwc3_glue_bind_common(struct udevice *parent, ofnode node)
>   	if (!dr_mode)
>   		dr_mode = usb_get_dr_mode(node);
>   
> -	if (CONFIG_IS_ENABLED(DM_USB_GADGET) &&
> +	if (IS_ENABLED(CONFIG_USB_DWC3_GADGET) && CONFIG_IS_ENABLED(DM_USB_GADGET) &&
>   	    (dr_mode == USB_DR_MODE_PERIPHERAL || dr_mode == USB_DR_MODE_OTG)) {
>   		debug("%s: dr_mode: OTG or Peripheral\n", __func__);
>   		driver = "dwc3-generic-peripheral";

+CC Mattijs .

Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Jonas Karlman]

On 2024-02-25 23:01, Marek Vasut wrote:
> On 2/25/24 4:27 PM, Jonas Karlman wrote:
>> Build fail with the following error when DM_USB_GADGET is enabled and
>> USB_DWC3_GADGET is disabled:
>>
>>    dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
>>    dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
>>      undefined reference to `dwc3_gadget_uboot_handle_interrupt'
>>
>> Build also fail with the following error when USB_GADGET_DWC2_OTG +
>> DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled:
>>
>>    gadget/dwc2_udc_otg.o: in function `dm_usb_gadget_handle_interrupts':
>>    gadget/dwc2_udc_otg.c:947: multiple definition of `dm_usb_gadget_handle_interrupts';
>>      dwc3/dwc3-generic.o:dwc3/dwc3-generic.c:197: first defined here
>>
>> Fix this by checking for USB_DWC3_GADGET in addition to DM_USB_GADGET.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> ---
>>   drivers/usb/dwc3/dwc3-generic.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
>> index 6fb2de8a5ace..891d01957619 100644
>> --- a/drivers/usb/dwc3/dwc3-generic.c
>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>> @@ -192,7 +192,7 @@ static int dwc3_generic_of_to_plat(struct udevice *dev)
>>   	return 0;
>>   }
>>   
>> -#if CONFIG_IS_ENABLED(DM_USB_GADGET)
>> +#if IS_ENABLED(CONFIG_USB_DWC3_GADGET) && CONFIG_IS_ENABLED(DM_USB_GADGET)
> 
> Maybe just make USB_DWC3_GADGET depend on (or select?) DM_USB_GADGET , 
> since I think the DWC3 code doesn't work without DM anyway .

Do you mean in addition to this? I do not think that alone is something
that will address my intention to be able to disable the gadget part of
the dwc3-generic driver.

Before this patch it was possible to enable disable host or gadget by
using USB_HOST/SPL_USB_HOST and DM_USB_GADGET/SPL_DM_USB_GADGET options.

However, Rockchip RK3328 boards have dwc2 otg and dwc3 host, trying to
use DM_USB_GADGET for dwc2 and USB_HOST for dwc3 is currently not
possible and result in the build errors reported, i.e. multiple
definition of dm_usb_gadget_handle_interrupts() and undefined reference
to dwc3_gadget_uboot_handle_interrupt().

After this patch I can use dm dwc2 gadget and dwc3 host without issue.

Regards,
Jonas

> 
>>   int dm_usb_gadget_handle_interrupts(struct udevice *dev)
>>   {
>>   	struct dwc3_generic_priv *priv = dev_get_priv(dev);
>> @@ -435,7 +435,7 @@ static int dwc3_glue_bind_common(struct udevice *parent, ofnode node)
>>   	if (!dr_mode)
>>   		dr_mode = usb_get_dr_mode(node);
>>   
>> -	if (CONFIG_IS_ENABLED(DM_USB_GADGET) &&
>> +	if (IS_ENABLED(CONFIG_USB_DWC3_GADGET) && CONFIG_IS_ENABLED(DM_USB_GADGET) &&
>>   	    (dr_mode == USB_DR_MODE_PERIPHERAL || dr_mode == USB_DR_MODE_OTG)) {
>>   		debug("%s: dr_mode: OTG or Peripheral\n", __func__);
>>   		driver = "dwc3-generic-peripheral";
> 
> +CC Mattijs .

Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Tom Rini]

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

On Mon, Feb 26, 2024 at 01:02:04AM +0100, Jonas Karlman wrote:
> On 2024-02-25 23:01, Marek Vasut wrote:
> > On 2/25/24 4:27 PM, Jonas Karlman wrote:
> >> Build fail with the following error when DM_USB_GADGET is enabled and
> >> USB_DWC3_GADGET is disabled:
> >>
> >>    dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
> >>    dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
> >>      undefined reference to `dwc3_gadget_uboot_handle_interrupt'
> >>
> >> Build also fail with the following error when USB_GADGET_DWC2_OTG +
> >> DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled:
> >>
> >>    gadget/dwc2_udc_otg.o: in function `dm_usb_gadget_handle_interrupts':
> >>    gadget/dwc2_udc_otg.c:947: multiple definition of `dm_usb_gadget_handle_interrupts';
> >>      dwc3/dwc3-generic.o:dwc3/dwc3-generic.c:197: first defined here
> >>
> >> Fix this by checking for USB_DWC3_GADGET in addition to DM_USB_GADGET.
> >>
> >> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> >> ---
> >>   drivers/usb/dwc3/dwc3-generic.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
> >> index 6fb2de8a5ace..891d01957619 100644
> >> --- a/drivers/usb/dwc3/dwc3-generic.c
> >> +++ b/drivers/usb/dwc3/dwc3-generic.c
> >> @@ -192,7 +192,7 @@ static int dwc3_generic_of_to_plat(struct udevice *dev)
> >>   	return 0;
> >>   }
> >>   
> >> -#if CONFIG_IS_ENABLED(DM_USB_GADGET)
> >> +#if IS_ENABLED(CONFIG_USB_DWC3_GADGET) && CONFIG_IS_ENABLED(DM_USB_GADGET)
> > 
> > Maybe just make USB_DWC3_GADGET depend on (or select?) DM_USB_GADGET , 
> > since I think the DWC3 code doesn't work without DM anyway .
> 
> Do you mean in addition to this? I do not think that alone is something
> that will address my intention to be able to disable the gadget part of
> the dwc3-generic driver.
> 
> Before this patch it was possible to enable disable host or gadget by
> using USB_HOST/SPL_USB_HOST and DM_USB_GADGET/SPL_DM_USB_GADGET options.
> 
> However, Rockchip RK3328 boards have dwc2 otg and dwc3 host, trying to
> use DM_USB_GADGET for dwc2 and USB_HOST for dwc3 is currently not
> possible and result in the build errors reported, i.e. multiple
> definition of dm_usb_gadget_handle_interrupts() and undefined reference
> to dwc3_gadget_uboot_handle_interrupt().
> 
> After this patch I can use dm dwc2 gadget and dwc3 host without issue.

Note that *DM_* symbols can be confusing. At this point, in SPL
SPL_DM_USB and SPL_DM_USB_GADGET are encouraged but not required for
host/gadget drivers. In full U-Boot, DM_USB is required for all host
drivers and really should be enabled in all cases for DM_USB_GADGET. We
can't enforce that at the Kconfig level because of, iirc, some of the
armv7 part gadget drivers (more than one). But for newer and still very
active chips, we should be using DM_USB_GADGET unconditionally in full
U-Boot.

So perhaps part of the issue is that dwc2/dwc3 need some hopefully now
legacy code removed.

-- 
Tom

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

Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Jonas Karlman]

On 2024-02-26 02:47, Tom Rini wrote:
> On Mon, Feb 26, 2024 at 01:02:04AM +0100, Jonas Karlman wrote:
>> On 2024-02-25 23:01, Marek Vasut wrote:
>>> On 2/25/24 4:27 PM, Jonas Karlman wrote:
>>>> Build fail with the following error when DM_USB_GADGET is enabled and
>>>> USB_DWC3_GADGET is disabled:
>>>>
>>>>    dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
>>>>    dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
>>>>      undefined reference to `dwc3_gadget_uboot_handle_interrupt'
>>>>
>>>> Build also fail with the following error when USB_GADGET_DWC2_OTG +
>>>> DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled:
>>>>
>>>>    gadget/dwc2_udc_otg.o: in function `dm_usb_gadget_handle_interrupts':
>>>>    gadget/dwc2_udc_otg.c:947: multiple definition of `dm_usb_gadget_handle_interrupts';
>>>>      dwc3/dwc3-generic.o:dwc3/dwc3-generic.c:197: first defined here
>>>>
>>>> Fix this by checking for USB_DWC3_GADGET in addition to DM_USB_GADGET.
>>>>
>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>>> ---
>>>>   drivers/usb/dwc3/dwc3-generic.c | 4 ++--
>>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
>>>> index 6fb2de8a5ace..891d01957619 100644
>>>> --- a/drivers/usb/dwc3/dwc3-generic.c
>>>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>>>> @@ -192,7 +192,7 @@ static int dwc3_generic_of_to_plat(struct udevice *dev)
>>>>   	return 0;
>>>>   }
>>>>   
>>>> -#if CONFIG_IS_ENABLED(DM_USB_GADGET)
>>>> +#if IS_ENABLED(CONFIG_USB_DWC3_GADGET) && CONFIG_IS_ENABLED(DM_USB_GADGET)
>>>
>>> Maybe just make USB_DWC3_GADGET depend on (or select?) DM_USB_GADGET , 
>>> since I think the DWC3 code doesn't work without DM anyway .
>>
>> Do you mean in addition to this? I do not think that alone is something
>> that will address my intention to be able to disable the gadget part of
>> the dwc3-generic driver.
>>
>> Before this patch it was possible to enable disable host or gadget by
>> using USB_HOST/SPL_USB_HOST and DM_USB_GADGET/SPL_DM_USB_GADGET options.
>>
>> However, Rockchip RK3328 boards have dwc2 otg and dwc3 host, trying to
>> use DM_USB_GADGET for dwc2 and USB_HOST for dwc3 is currently not
>> possible and result in the build errors reported, i.e. multiple
>> definition of dm_usb_gadget_handle_interrupts() and undefined reference
>> to dwc3_gadget_uboot_handle_interrupt().
>>
>> After this patch I can use dm dwc2 gadget and dwc3 host without issue.
> 
> Note that *DM_* symbols can be confusing. At this point, in SPL
> SPL_DM_USB and SPL_DM_USB_GADGET are encouraged but not required for
> host/gadget drivers. In full U-Boot, DM_USB is required for all host
> drivers and really should be enabled in all cases for DM_USB_GADGET. We
> can't enforce that at the Kconfig level because of, iirc, some of the
> armv7 part gadget drivers (more than one). But for newer and still very
> active chips, we should be using DM_USB_GADGET unconditionally in full
> U-Boot.

The issue is that with DM_USB_GADGET we can only have one driver
providing the dm_usb_gadget_handle_interrupts() function.

On RK3328 boards there is a dwc2 otg port, a dwc2 host port and a dwc3
host port. Currently all these ports work without DM_USB_GADGET because
a board_usb_init() call dwc2_udc_probe().

CONFIG_USB_DWC2=y
CONFIG_USB_DWC3=y
# CONFIG_USB_DWC3_GADGET is not set
CONFIG_USB_DWC3_GENERIC=y
CONFIG_USB_GADGET=y
CONFIG_USB_GADGET_DWC2_OTG=y
# CONFIG_DM_USB_GADGET is not set

I want to migrate to use DM_USB_GADGET with DWC2_OTG instead of using
board_usb_init(), this is currently not possible because of the build
errors reported in this patch.

Regards,
Jonas

> 
> So perhaps part of the issue is that dwc2/dwc3 need some hopefully now
> legacy code removed.

Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Marek Vasut]

On 2/26/24 8:54 AM, Jonas Karlman wrote:
> On 2024-02-26 02:47, Tom Rini wrote:
>> On Mon, Feb 26, 2024 at 01:02:04AM +0100, Jonas Karlman wrote:
>>> On 2024-02-25 23:01, Marek Vasut wrote:
>>>> On 2/25/24 4:27 PM, Jonas Karlman wrote:
>>>>> Build fail with the following error when DM_USB_GADGET is enabled and
>>>>> USB_DWC3_GADGET is disabled:
>>>>>
>>>>>     dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
>>>>>     dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
>>>>>       undefined reference to `dwc3_gadget_uboot_handle_interrupt'
>>>>>
>>>>> Build also fail with the following error when USB_GADGET_DWC2_OTG +
>>>>> DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled:
>>>>>
>>>>>     gadget/dwc2_udc_otg.o: in function `dm_usb_gadget_handle_interrupts':
>>>>>     gadget/dwc2_udc_otg.c:947: multiple definition of `dm_usb_gadget_handle_interrupts';
>>>>>       dwc3/dwc3-generic.o:dwc3/dwc3-generic.c:197: first defined here
>>>>>
>>>>> Fix this by checking for USB_DWC3_GADGET in addition to DM_USB_GADGET.
>>>>>
>>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>>>> ---
>>>>>    drivers/usb/dwc3/dwc3-generic.c | 4 ++--
>>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
>>>>> index 6fb2de8a5ace..891d01957619 100644
>>>>> --- a/drivers/usb/dwc3/dwc3-generic.c
>>>>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>>>>> @@ -192,7 +192,7 @@ static int dwc3_generic_of_to_plat(struct udevice *dev)
>>>>>    	return 0;
>>>>>    }
>>>>>    
>>>>> -#if CONFIG_IS_ENABLED(DM_USB_GADGET)
>>>>> +#if IS_ENABLED(CONFIG_USB_DWC3_GADGET) && CONFIG_IS_ENABLED(DM_USB_GADGET)
>>>>
>>>> Maybe just make USB_DWC3_GADGET depend on (or select?) DM_USB_GADGET ,
>>>> since I think the DWC3 code doesn't work without DM anyway .
>>>
>>> Do you mean in addition to this? I do not think that alone is something
>>> that will address my intention to be able to disable the gadget part of
>>> the dwc3-generic driver.
>>>
>>> Before this patch it was possible to enable disable host or gadget by
>>> using USB_HOST/SPL_USB_HOST and DM_USB_GADGET/SPL_DM_USB_GADGET options.
>>>
>>> However, Rockchip RK3328 boards have dwc2 otg and dwc3 host, trying to
>>> use DM_USB_GADGET for dwc2 and USB_HOST for dwc3 is currently not
>>> possible and result in the build errors reported, i.e. multiple
>>> definition of dm_usb_gadget_handle_interrupts() and undefined reference
>>> to dwc3_gadget_uboot_handle_interrupt().
>>>
>>> After this patch I can use dm dwc2 gadget and dwc3 host without issue.
>>
>> Note that *DM_* symbols can be confusing. At this point, in SPL
>> SPL_DM_USB and SPL_DM_USB_GADGET are encouraged but not required for
>> host/gadget drivers. In full U-Boot, DM_USB is required for all host
>> drivers and really should be enabled in all cases for DM_USB_GADGET. We
>> can't enforce that at the Kconfig level because of, iirc, some of the
>> armv7 part gadget drivers (more than one). But for newer and still very
>> active chips, we should be using DM_USB_GADGET unconditionally in full
>> U-Boot.
> 
> The issue is that with DM_USB_GADGET we can only have one driver
> providing the dm_usb_gadget_handle_interrupts() function.

DM was always intended to permit multiple drivers, so this is a bug and 
should be fixed, e.g. by turning the interrupt handling function into a 
driver-specific callback.

[...]

Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Jonas Karlman]

On 2024-02-26 09:22, Marek Vasut wrote:
> On 2/26/24 8:54 AM, Jonas Karlman wrote:
>> On 2024-02-26 02:47, Tom Rini wrote:
>>> On Mon, Feb 26, 2024 at 01:02:04AM +0100, Jonas Karlman wrote:
>>>> On 2024-02-25 23:01, Marek Vasut wrote:
>>>>> On 2/25/24 4:27 PM, Jonas Karlman wrote:
>>>>>> Build fail with the following error when DM_USB_GADGET is enabled and
>>>>>> USB_DWC3_GADGET is disabled:
>>>>>>
>>>>>>     dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
>>>>>>     dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
>>>>>>       undefined reference to `dwc3_gadget_uboot_handle_interrupt'
>>>>>>
>>>>>> Build also fail with the following error when USB_GADGET_DWC2_OTG +
>>>>>> DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled:
>>>>>>
>>>>>>     gadget/dwc2_udc_otg.o: in function `dm_usb_gadget_handle_interrupts':
>>>>>>     gadget/dwc2_udc_otg.c:947: multiple definition of `dm_usb_gadget_handle_interrupts';
>>>>>>       dwc3/dwc3-generic.o:dwc3/dwc3-generic.c:197: first defined here
>>>>>>
>>>>>> Fix this by checking for USB_DWC3_GADGET in addition to DM_USB_GADGET.
>>>>>>
>>>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>>>>> ---
>>>>>>    drivers/usb/dwc3/dwc3-generic.c | 4 ++--
>>>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
>>>>>> index 6fb2de8a5ace..891d01957619 100644
>>>>>> --- a/drivers/usb/dwc3/dwc3-generic.c
>>>>>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>>>>>> @@ -192,7 +192,7 @@ static int dwc3_generic_of_to_plat(struct udevice *dev)
>>>>>>    	return 0;
>>>>>>    }
>>>>>>    
>>>>>> -#if CONFIG_IS_ENABLED(DM_USB_GADGET)
>>>>>> +#if IS_ENABLED(CONFIG_USB_DWC3_GADGET) && CONFIG_IS_ENABLED(DM_USB_GADGET)
>>>>>
>>>>> Maybe just make USB_DWC3_GADGET depend on (or select?) DM_USB_GADGET ,
>>>>> since I think the DWC3 code doesn't work without DM anyway .
>>>>
>>>> Do you mean in addition to this? I do not think that alone is something
>>>> that will address my intention to be able to disable the gadget part of
>>>> the dwc3-generic driver.
>>>>
>>>> Before this patch it was possible to enable disable host or gadget by
>>>> using USB_HOST/SPL_USB_HOST and DM_USB_GADGET/SPL_DM_USB_GADGET options.
>>>>
>>>> However, Rockchip RK3328 boards have dwc2 otg and dwc3 host, trying to
>>>> use DM_USB_GADGET for dwc2 and USB_HOST for dwc3 is currently not
>>>> possible and result in the build errors reported, i.e. multiple
>>>> definition of dm_usb_gadget_handle_interrupts() and undefined reference
>>>> to dwc3_gadget_uboot_handle_interrupt().
>>>>
>>>> After this patch I can use dm dwc2 gadget and dwc3 host without issue.
>>>
>>> Note that *DM_* symbols can be confusing. At this point, in SPL
>>> SPL_DM_USB and SPL_DM_USB_GADGET are encouraged but not required for
>>> host/gadget drivers. In full U-Boot, DM_USB is required for all host
>>> drivers and really should be enabled in all cases for DM_USB_GADGET. We
>>> can't enforce that at the Kconfig level because of, iirc, some of the
>>> armv7 part gadget drivers (more than one). But for newer and still very
>>> active chips, we should be using DM_USB_GADGET unconditionally in full
>>> U-Boot.
>>
>> The issue is that with DM_USB_GADGET we can only have one driver
>> providing the dm_usb_gadget_handle_interrupts() function.
> 
> DM was always intended to permit multiple drivers, so this is a bug and 
> should be fixed, e.g. by turning the interrupt handling function into a 
> driver-specific callback.

I fully agree, and I hope that someone can work on that separate issue
and that it does not block this build error fix patch.

Will send out a series that migrates RK3328 boards to use DM_USB_GADGET
with DWC2_OTG later, a series that will have a dependency on this patch.

Regards,
Jonas

> 
> [...]

Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Marek Vasut]

On 2/26/24 10:50 AM, Jonas Karlman wrote:
> On 2024-02-26 09:22, Marek Vasut wrote:
>> On 2/26/24 8:54 AM, Jonas Karlman wrote:
>>> On 2024-02-26 02:47, Tom Rini wrote:
>>>> On Mon, Feb 26, 2024 at 01:02:04AM +0100, Jonas Karlman wrote:
>>>>> On 2024-02-25 23:01, Marek Vasut wrote:
>>>>>> On 2/25/24 4:27 PM, Jonas Karlman wrote:
>>>>>>> Build fail with the following error when DM_USB_GADGET is enabled and
>>>>>>> USB_DWC3_GADGET is disabled:
>>>>>>>
>>>>>>>      dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
>>>>>>>      dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
>>>>>>>        undefined reference to `dwc3_gadget_uboot_handle_interrupt'
>>>>>>>
>>>>>>> Build also fail with the following error when USB_GADGET_DWC2_OTG +
>>>>>>> DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled:
>>>>>>>
>>>>>>>      gadget/dwc2_udc_otg.o: in function `dm_usb_gadget_handle_interrupts':
>>>>>>>      gadget/dwc2_udc_otg.c:947: multiple definition of `dm_usb_gadget_handle_interrupts';
>>>>>>>        dwc3/dwc3-generic.o:dwc3/dwc3-generic.c:197: first defined here
>>>>>>>
>>>>>>> Fix this by checking for USB_DWC3_GADGET in addition to DM_USB_GADGET.
>>>>>>>
>>>>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>>>>>> ---
>>>>>>>     drivers/usb/dwc3/dwc3-generic.c | 4 ++--
>>>>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
>>>>>>> index 6fb2de8a5ace..891d01957619 100644
>>>>>>> --- a/drivers/usb/dwc3/dwc3-generic.c
>>>>>>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>>>>>>> @@ -192,7 +192,7 @@ static int dwc3_generic_of_to_plat(struct udevice *dev)
>>>>>>>     	return 0;
>>>>>>>     }
>>>>>>>     
>>>>>>> -#if CONFIG_IS_ENABLED(DM_USB_GADGET)
>>>>>>> +#if IS_ENABLED(CONFIG_USB_DWC3_GADGET) && CONFIG_IS_ENABLED(DM_USB_GADGET)
>>>>>>
>>>>>> Maybe just make USB_DWC3_GADGET depend on (or select?) DM_USB_GADGET ,
>>>>>> since I think the DWC3 code doesn't work without DM anyway .
>>>>>
>>>>> Do you mean in addition to this? I do not think that alone is something
>>>>> that will address my intention to be able to disable the gadget part of
>>>>> the dwc3-generic driver.
>>>>>
>>>>> Before this patch it was possible to enable disable host or gadget by
>>>>> using USB_HOST/SPL_USB_HOST and DM_USB_GADGET/SPL_DM_USB_GADGET options.
>>>>>
>>>>> However, Rockchip RK3328 boards have dwc2 otg and dwc3 host, trying to
>>>>> use DM_USB_GADGET for dwc2 and USB_HOST for dwc3 is currently not
>>>>> possible and result in the build errors reported, i.e. multiple
>>>>> definition of dm_usb_gadget_handle_interrupts() and undefined reference
>>>>> to dwc3_gadget_uboot_handle_interrupt().
>>>>>
>>>>> After this patch I can use dm dwc2 gadget and dwc3 host without issue.
>>>>
>>>> Note that *DM_* symbols can be confusing. At this point, in SPL
>>>> SPL_DM_USB and SPL_DM_USB_GADGET are encouraged but not required for
>>>> host/gadget drivers. In full U-Boot, DM_USB is required for all host
>>>> drivers and really should be enabled in all cases for DM_USB_GADGET. We
>>>> can't enforce that at the Kconfig level because of, iirc, some of the
>>>> armv7 part gadget drivers (more than one). But for newer and still very
>>>> active chips, we should be using DM_USB_GADGET unconditionally in full
>>>> U-Boot.
>>>
>>> The issue is that with DM_USB_GADGET we can only have one driver
>>> providing the dm_usb_gadget_handle_interrupts() function.
>>
>> DM was always intended to permit multiple drivers, so this is a bug and
>> should be fixed, e.g. by turning the interrupt handling function into a
>> driver-specific callback.
> 
> I fully agree, and I hope that someone can work on that separate issue
> and that it does not block this build error fix patch.

Please, let's fix this properly instead of piling up another ifdef 
workaround.

Since CI does pass, where is there a build error ?

Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Jonas Karlman]

On 2024-02-26 11:18, Marek Vasut wrote:
> On 2/26/24 10:50 AM, Jonas Karlman wrote:
>> On 2024-02-26 09:22, Marek Vasut wrote:
>>> On 2/26/24 8:54 AM, Jonas Karlman wrote:
>>>> On 2024-02-26 02:47, Tom Rini wrote:
>>>>> On Mon, Feb 26, 2024 at 01:02:04AM +0100, Jonas Karlman wrote:
>>>>>> On 2024-02-25 23:01, Marek Vasut wrote:
>>>>>>> On 2/25/24 4:27 PM, Jonas Karlman wrote:
>>>>>>>> Build fail with the following error when DM_USB_GADGET is enabled and
>>>>>>>> USB_DWC3_GADGET is disabled:
>>>>>>>>
>>>>>>>>      dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
>>>>>>>>      dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
>>>>>>>>        undefined reference to `dwc3_gadget_uboot_handle_interrupt'
>>>>>>>>
>>>>>>>> Build also fail with the following error when USB_GADGET_DWC2_OTG +
>>>>>>>> DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled:
>>>>>>>>
>>>>>>>>      gadget/dwc2_udc_otg.o: in function `dm_usb_gadget_handle_interrupts':
>>>>>>>>      gadget/dwc2_udc_otg.c:947: multiple definition of `dm_usb_gadget_handle_interrupts';
>>>>>>>>        dwc3/dwc3-generic.o:dwc3/dwc3-generic.c:197: first defined here
>>>>>>>>
>>>>>>>> Fix this by checking for USB_DWC3_GADGET in addition to DM_USB_GADGET.
>>>>>>>>
>>>>>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>>>>>>> ---
>>>>>>>>     drivers/usb/dwc3/dwc3-generic.c | 4 ++--
>>>>>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
>>>>>>>> index 6fb2de8a5ace..891d01957619 100644
>>>>>>>> --- a/drivers/usb/dwc3/dwc3-generic.c
>>>>>>>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>>>>>>>> @@ -192,7 +192,7 @@ static int dwc3_generic_of_to_plat(struct udevice *dev)
>>>>>>>>     	return 0;
>>>>>>>>     }
>>>>>>>>     
>>>>>>>> -#if CONFIG_IS_ENABLED(DM_USB_GADGET)
>>>>>>>> +#if IS_ENABLED(CONFIG_USB_DWC3_GADGET) && CONFIG_IS_ENABLED(DM_USB_GADGET)
>>>>>>>
>>>>>>> Maybe just make USB_DWC3_GADGET depend on (or select?) DM_USB_GADGET ,
>>>>>>> since I think the DWC3 code doesn't work without DM anyway .
>>>>>>
>>>>>> Do you mean in addition to this? I do not think that alone is something
>>>>>> that will address my intention to be able to disable the gadget part of
>>>>>> the dwc3-generic driver.
>>>>>>
>>>>>> Before this patch it was possible to enable disable host or gadget by
>>>>>> using USB_HOST/SPL_USB_HOST and DM_USB_GADGET/SPL_DM_USB_GADGET options.
>>>>>>
>>>>>> However, Rockchip RK3328 boards have dwc2 otg and dwc3 host, trying to
>>>>>> use DM_USB_GADGET for dwc2 and USB_HOST for dwc3 is currently not
>>>>>> possible and result in the build errors reported, i.e. multiple
>>>>>> definition of dm_usb_gadget_handle_interrupts() and undefined reference
>>>>>> to dwc3_gadget_uboot_handle_interrupt().
>>>>>>
>>>>>> After this patch I can use dm dwc2 gadget and dwc3 host without issue.
>>>>>
>>>>> Note that *DM_* symbols can be confusing. At this point, in SPL
>>>>> SPL_DM_USB and SPL_DM_USB_GADGET are encouraged but not required for
>>>>> host/gadget drivers. In full U-Boot, DM_USB is required for all host
>>>>> drivers and really should be enabled in all cases for DM_USB_GADGET. We
>>>>> can't enforce that at the Kconfig level because of, iirc, some of the
>>>>> armv7 part gadget drivers (more than one). But for newer and still very
>>>>> active chips, we should be using DM_USB_GADGET unconditionally in full
>>>>> U-Boot.
>>>>
>>>> The issue is that with DM_USB_GADGET we can only have one driver
>>>> providing the dm_usb_gadget_handle_interrupts() function.
>>>
>>> DM was always intended to permit multiple drivers, so this is a bug and
>>> should be fixed, e.g. by turning the interrupt handling function into a
>>> driver-specific callback.
>>
>> I fully agree, and I hope that someone can work on that separate issue
>> and that it does not block this build error fix patch.
> 
> Please, let's fix this properly instead of piling up another ifdef 
> workaround.

Reworking the core usb gadget interrupt handling is unfortunately not
something I can spend any of my already limited hobbyist free time on.

If this workaround is not acceptable I can just drop the RK3328 cleanup
and leave usb in current semi-broken state.

> 
> Since CI does pass, where is there a build error ?

Currently there is no board with a configuration that cause build errors,
that happens when I add DM_USB_GADGET=y to RK3328 boards in a separate
series.

Previously you have requested that I do not send usb patches as part of
a larger series targeted to rockchip maintainers. So this time I sent
the usb part as a standalone patch ahead of such series.

Will send out that series later, and if no one wants to pick this up or
have time to rework gadget interrupt handling that is fine with me :-)

Regards,
Jonas

Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Mattijs Korpershoek]

Hi Jonas, thank you for the patch.

On lun., févr. 26, 2024 at 13:36, Jonas Karlman <jonas@kwiboo.se> wrote:

> On 2024-02-26 11:18, Marek Vasut wrote:
>> On 2/26/24 10:50 AM, Jonas Karlman wrote:
>>> On 2024-02-26 09:22, Marek Vasut wrote:
>>>> On 2/26/24 8:54 AM, Jonas Karlman wrote:
>>>>> On 2024-02-26 02:47, Tom Rini wrote:
>>>>>> On Mon, Feb 26, 2024 at 01:02:04AM +0100, Jonas Karlman wrote:
>>>>>>> On 2024-02-25 23:01, Marek Vasut wrote:
>>>>>>>> On 2/25/24 4:27 PM, Jonas Karlman wrote:
>>>>>>>>> Build fail with the following error when DM_USB_GADGET is enabled and
>>>>>>>>> USB_DWC3_GADGET is disabled:
>>>>>>>>>
>>>>>>>>>      dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
>>>>>>>>>      dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
>>>>>>>>>        undefined reference to `dwc3_gadget_uboot_handle_interrupt'
>>>>>>>>>
>>>>>>>>> Build also fail with the following error when USB_GADGET_DWC2_OTG +
>>>>>>>>> DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled:
>>>>>>>>>
>>>>>>>>>      gadget/dwc2_udc_otg.o: in function `dm_usb_gadget_handle_interrupts':
>>>>>>>>>      gadget/dwc2_udc_otg.c:947: multiple definition of `dm_usb_gadget_handle_interrupts';
>>>>>>>>>        dwc3/dwc3-generic.o:dwc3/dwc3-generic.c:197: first defined here
>>>>>>>>>
>>>>>>>>> Fix this by checking for USB_DWC3_GADGET in addition to DM_USB_GADGET.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>>>>>>>> ---
>>>>>>>>>     drivers/usb/dwc3/dwc3-generic.c | 4 ++--
>>>>>>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
>>>>>>>>> index 6fb2de8a5ace..891d01957619 100644
>>>>>>>>> --- a/drivers/usb/dwc3/dwc3-generic.c
>>>>>>>>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>>>>>>>>> @@ -192,7 +192,7 @@ static int dwc3_generic_of_to_plat(struct udevice *dev)
>>>>>>>>>     	return 0;
>>>>>>>>>     }
>>>>>>>>>     
>>>>>>>>> -#if CONFIG_IS_ENABLED(DM_USB_GADGET)
>>>>>>>>> +#if IS_ENABLED(CONFIG_USB_DWC3_GADGET) && CONFIG_IS_ENABLED(DM_USB_GADGET)
>>>>>>>>
>>>>>>>> Maybe just make USB_DWC3_GADGET depend on (or select?) DM_USB_GADGET ,
>>>>>>>> since I think the DWC3 code doesn't work without DM anyway .
>>>>>>>
>>>>>>> Do you mean in addition to this? I do not think that alone is something
>>>>>>> that will address my intention to be able to disable the gadget part of
>>>>>>> the dwc3-generic driver.
>>>>>>>
>>>>>>> Before this patch it was possible to enable disable host or gadget by
>>>>>>> using USB_HOST/SPL_USB_HOST and DM_USB_GADGET/SPL_DM_USB_GADGET options.
>>>>>>>
>>>>>>> However, Rockchip RK3328 boards have dwc2 otg and dwc3 host, trying to
>>>>>>> use DM_USB_GADGET for dwc2 and USB_HOST for dwc3 is currently not
>>>>>>> possible and result in the build errors reported, i.e. multiple
>>>>>>> definition of dm_usb_gadget_handle_interrupts() and undefined reference
>>>>>>> to dwc3_gadget_uboot_handle_interrupt().
>>>>>>>
>>>>>>> After this patch I can use dm dwc2 gadget and dwc3 host without issue.
>>>>>>
>>>>>> Note that *DM_* symbols can be confusing. At this point, in SPL
>>>>>> SPL_DM_USB and SPL_DM_USB_GADGET are encouraged but not required for
>>>>>> host/gadget drivers. In full U-Boot, DM_USB is required for all host
>>>>>> drivers and really should be enabled in all cases for DM_USB_GADGET. We
>>>>>> can't enforce that at the Kconfig level because of, iirc, some of the
>>>>>> armv7 part gadget drivers (more than one). But for newer and still very
>>>>>> active chips, we should be using DM_USB_GADGET unconditionally in full
>>>>>> U-Boot.
>>>>>
>>>>> The issue is that with DM_USB_GADGET we can only have one driver
>>>>> providing the dm_usb_gadget_handle_interrupts() function.
>>>>
>>>> DM was always intended to permit multiple drivers, so this is a bug and
>>>> should be fixed, e.g. by turning the interrupt handling function into a
>>>> driver-specific callback.
>>>
>>> I fully agree, and I hope that someone can work on that separate issue
>>> and that it does not block this build error fix patch.
>> 
>> Please, let's fix this properly instead of piling up another ifdef 
>> workaround.

I agree with Marek.

>
> Reworking the core usb gadget interrupt handling is unfortunately not
> something I can spend any of my already limited hobbyist free time on.

I understand you don't have time for this.

I will try to tackle it, but I have limited time in the upcoming weeks.

I will keep you posted.

>
> If this workaround is not acceptable I can just drop the RK3328 cleanup
> and leave usb in current semi-broken state.
>
>> 
>> Since CI does pass, where is there a build error ?
>
> Currently there is no board with a configuration that cause build errors,
> that happens when I add DM_USB_GADGET=y to RK3328 boards in a separate
> series.
>
> Previously you have requested that I do not send usb patches as part of
> a larger series targeted to rockchip maintainers. So this time I sent
> the usb part as a standalone patch ahead of such series.
>
> Will send out that series later, and if no one wants to pick this up or
> have time to rework gadget interrupt handling that is fine with me :-)
>
> Regards,
> Jonas

Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Jonas Karlman]

Hi Mattijs,

On 2024-03-01 16:18, Mattijs Korpershoek wrote:
> Hi Jonas, thank you for the patch.
> 
> On lun., févr. 26, 2024 at 13:36, Jonas Karlman <jonas@kwiboo.se> wrote:
> 
>> On 2024-02-26 11:18, Marek Vasut wrote:
>>> On 2/26/24 10:50 AM, Jonas Karlman wrote:
>>>> On 2024-02-26 09:22, Marek Vasut wrote:
>>>>> On 2/26/24 8:54 AM, Jonas Karlman wrote:
>>>>>> On 2024-02-26 02:47, Tom Rini wrote:
>>>>>>> On Mon, Feb 26, 2024 at 01:02:04AM +0100, Jonas Karlman wrote:
>>>>>>>> On 2024-02-25 23:01, Marek Vasut wrote:
>>>>>>>>> On 2/25/24 4:27 PM, Jonas Karlman wrote:
>>>>>>>>>> Build fail with the following error when DM_USB_GADGET is enabled and
>>>>>>>>>> USB_DWC3_GADGET is disabled:
>>>>>>>>>>
>>>>>>>>>>      dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
>>>>>>>>>>      dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
>>>>>>>>>>        undefined reference to `dwc3_gadget_uboot_handle_interrupt'
>>>>>>>>>>
>>>>>>>>>> Build also fail with the following error when USB_GADGET_DWC2_OTG +
>>>>>>>>>> DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled:
>>>>>>>>>>
>>>>>>>>>>      gadget/dwc2_udc_otg.o: in function `dm_usb_gadget_handle_interrupts':
>>>>>>>>>>      gadget/dwc2_udc_otg.c:947: multiple definition of `dm_usb_gadget_handle_interrupts';
>>>>>>>>>>        dwc3/dwc3-generic.o:dwc3/dwc3-generic.c:197: first defined here
>>>>>>>>>>
>>>>>>>>>> Fix this by checking for USB_DWC3_GADGET in addition to DM_USB_GADGET.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>>>>>>>>> ---
>>>>>>>>>>     drivers/usb/dwc3/dwc3-generic.c | 4 ++--
>>>>>>>>>>     1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
>>>>>>>>>> index 6fb2de8a5ace..891d01957619 100644
>>>>>>>>>> --- a/drivers/usb/dwc3/dwc3-generic.c
>>>>>>>>>> +++ b/drivers/usb/dwc3/dwc3-generic.c
>>>>>>>>>> @@ -192,7 +192,7 @@ static int dwc3_generic_of_to_plat(struct udevice *dev)
>>>>>>>>>>     	return 0;
>>>>>>>>>>     }
>>>>>>>>>>     
>>>>>>>>>> -#if CONFIG_IS_ENABLED(DM_USB_GADGET)
>>>>>>>>>> +#if IS_ENABLED(CONFIG_USB_DWC3_GADGET) && CONFIG_IS_ENABLED(DM_USB_GADGET)
>>>>>>>>>
>>>>>>>>> Maybe just make USB_DWC3_GADGET depend on (or select?) DM_USB_GADGET ,
>>>>>>>>> since I think the DWC3 code doesn't work without DM anyway .
>>>>>>>>
>>>>>>>> Do you mean in addition to this? I do not think that alone is something
>>>>>>>> that will address my intention to be able to disable the gadget part of
>>>>>>>> the dwc3-generic driver.
>>>>>>>>
>>>>>>>> Before this patch it was possible to enable disable host or gadget by
>>>>>>>> using USB_HOST/SPL_USB_HOST and DM_USB_GADGET/SPL_DM_USB_GADGET options.
>>>>>>>>
>>>>>>>> However, Rockchip RK3328 boards have dwc2 otg and dwc3 host, trying to
>>>>>>>> use DM_USB_GADGET for dwc2 and USB_HOST for dwc3 is currently not
>>>>>>>> possible and result in the build errors reported, i.e. multiple
>>>>>>>> definition of dm_usb_gadget_handle_interrupts() and undefined reference
>>>>>>>> to dwc3_gadget_uboot_handle_interrupt().
>>>>>>>>
>>>>>>>> After this patch I can use dm dwc2 gadget and dwc3 host without issue.
>>>>>>>
>>>>>>> Note that *DM_* symbols can be confusing. At this point, in SPL
>>>>>>> SPL_DM_USB and SPL_DM_USB_GADGET are encouraged but not required for
>>>>>>> host/gadget drivers. In full U-Boot, DM_USB is required for all host
>>>>>>> drivers and really should be enabled in all cases for DM_USB_GADGET. We
>>>>>>> can't enforce that at the Kconfig level because of, iirc, some of the
>>>>>>> armv7 part gadget drivers (more than one). But for newer and still very
>>>>>>> active chips, we should be using DM_USB_GADGET unconditionally in full
>>>>>>> U-Boot.
>>>>>>
>>>>>> The issue is that with DM_USB_GADGET we can only have one driver
>>>>>> providing the dm_usb_gadget_handle_interrupts() function.
>>>>>
>>>>> DM was always intended to permit multiple drivers, so this is a bug and
>>>>> should be fixed, e.g. by turning the interrupt handling function into a
>>>>> driver-specific callback.
>>>>
>>>> I fully agree, and I hope that someone can work on that separate issue
>>>> and that it does not block this build error fix patch.
>>>
>>> Please, let's fix this properly instead of piling up another ifdef 
>>> workaround.
> 
> I agree with Marek.
> 
>>
>> Reworking the core usb gadget interrupt handling is unfortunately not
>> something I can spend any of my already limited hobbyist free time on.
> 
> I understand you don't have time for this.
> 
> I will try to tackle it, but I have limited time in the upcoming weeks.
> 
> I will keep you posted.

Thanks, much appreciated!

Please also keep in mind that changing the interrupt handling probably
only fixes the second of the two build errors reported and fixed by this
patch.

Trying to build with following will trigger the first build error, and
should not change because use of dm_usb_gadget_handle_interrupts() is
reworked.

CONFIG_DM_USB_GADGET=y
CONFIG_USB_DWC3=y
# CONFIG_USB_DWC3_GADGET is not set
CONFIG_USB_DWC3_GENERIC=y
CONFIG_USB_GADGET=y

E.g to only include host part of dwc3 and gadget from another driver,
to i.e. save on binary size, produce following build error:

aarch64-linux-gnu-ld.bfd: drivers/usb/dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
drivers/usb/dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
  undefined reference to `dwc3_gadget_uboot_handle_interrupt'

I guess force select USB_DWC3_GADGET for USB_DWC3_GENERIC would make
that build error disappear, and increase binary size as a result.

For my RK3328 series [1] I will just revert to use USB_XHCI_DWC3 instead
of using USB_DWC3_GENERIC on boards that enable peripheral use of otg
port.

[1] https://patchwork.ozlabs.org/patch/1904779/

Regards,
Jonas

> 
>>
>> If this workaround is not acceptable I can just drop the RK3328 cleanup
>> and leave usb in current semi-broken state.
>>
>>>
>>> Since CI does pass, where is there a build error ?
>>
>> Currently there is no board with a configuration that cause build errors,
>> that happens when I add DM_USB_GADGET=y to RK3328 boards in a separate
>> series.
>>
>> Previously you have requested that I do not send usb patches as part of
>> a larger series targeted to rockchip maintainers. So this time I sent
>> the usb part as a standalone patch ahead of such series.
>>
>> Will send out that series later, and if no one wants to pick this up or
>> have time to rework gadget interrupt handling that is fine with me :-)
>>
>> Regards,
>> Jonas

Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Mattijs Korpershoek]

Hi Jonas,

On sam., mars 02, 2024 at 14:00, Jonas Karlman <jonas@kwiboo.se> wrote:

[...]

>> 
>> I will keep you posted.
>
> Thanks, much appreciated!
>
> Please also keep in mind that changing the interrupt handling probably
> only fixes the second of the two build errors reported and fixed by this
> patch.
>
> Trying to build with following will trigger the first build error, and
> should not change because use of dm_usb_gadget_handle_interrupts() is
> reworked.
>
> CONFIG_DM_USB_GADGET=y
> CONFIG_USB_DWC3=y
> # CONFIG_USB_DWC3_GADGET is not set
> CONFIG_USB_DWC3_GENERIC=y
> CONFIG_USB_GADGET=y
>
> E.g to only include host part of dwc3 and gadget from another driver,
> to i.e. save on binary size, produce following build error:
>
> aarch64-linux-gnu-ld.bfd: drivers/usb/dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
> drivers/usb/dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
>   undefined reference to `dwc3_gadget_uboot_handle_interrupt'
>
> I guess force select USB_DWC3_GADGET for USB_DWC3_GENERIC would make
> that build error disappear, and increase binary size as a result.
>
> For my RK3328 series [1] I will just revert to use USB_XHCI_DWC3 instead
> of using USB_DWC3_GENERIC on boards that enable peripheral use of otg
> port.
>
> [1] https://patchwork.ozlabs.org/patch/1904779/

Marek ended up doing this rework.
It's available for review here, if you want to have a look:
https://lore.kernel.org/all/20240614005309.34433-1-marek.vasut+renesas@mailbox.org/

>
> Regards,
> Jonas
>
>> 
>>>

[...]

Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Marek Vasut]

On 6/18/24 9:15 AM, Mattijs Korpershoek wrote:
> Hi Jonas,

Hello all,

> On sam., mars 02, 2024 at 14:00, Jonas Karlman <jonas@kwiboo.se> wrote:
> 
> [...]
> 
>>>
>>> I will keep you posted.
>>
>> Thanks, much appreciated!
>>
>> Please also keep in mind that changing the interrupt handling probably
>> only fixes the second of the two build errors reported and fixed by this
>> patch.
>>
>> Trying to build with following will trigger the first build error, and
>> should not change because use of dm_usb_gadget_handle_interrupts() is
>> reworked.
>>
>> CONFIG_DM_USB_GADGET=y
>> CONFIG_USB_DWC3=y
>> # CONFIG_USB_DWC3_GADGET is not set
>> CONFIG_USB_DWC3_GENERIC=y
>> CONFIG_USB_GADGET=y
>>
>> E.g to only include host part of dwc3 and gadget from another driver,
>> to i.e. save on binary size, produce following build error:
>>
>> aarch64-linux-gnu-ld.bfd: drivers/usb/dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
>> drivers/usb/dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
>>    undefined reference to `dwc3_gadget_uboot_handle_interrupt'
>>
>> I guess force select USB_DWC3_GADGET for USB_DWC3_GENERIC would make
>> that build error disappear, and increase binary size as a result.
>>
>> For my RK3328 series [1] I will just revert to use USB_XHCI_DWC3 instead
>> of using USB_DWC3_GENERIC on boards that enable peripheral use of otg
>> port.
>>
>> [1] https://patchwork.ozlabs.org/patch/1904779/
> 
> Marek ended up doing this rework.
> It's available for review here, if you want to have a look:
> https://lore.kernel.org/all/20240614005309.34433-1-marek.vasut+renesas@mailbox.org/
This should be upstream in some form now, is this thread still an open 
topic or is this solved now ?

Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Mattijs Korpershoek]

Hi Marek,

On dim., janv. 05, 2025 at 20:29, Marek Vasut <marex@denx.de> wrote:

> On 6/18/24 9:15 AM, Mattijs Korpershoek wrote:
>> Hi Jonas,
>
> Hello all,
>
>> On sam., mars 02, 2024 at 14:00, Jonas Karlman <jonas@kwiboo.se> wrote:
>> 
>> [...]
>> 
>>>>
>>>> I will keep you posted.
>>>
>>> Thanks, much appreciated!
>>>
>>> Please also keep in mind that changing the interrupt handling probably
>>> only fixes the second of the two build errors reported and fixed by this
>>> patch.
>>>
>>> Trying to build with following will trigger the first build error, and
>>> should not change because use of dm_usb_gadget_handle_interrupts() is
>>> reworked.
>>>
>>> CONFIG_DM_USB_GADGET=y
>>> CONFIG_USB_DWC3=y
>>> # CONFIG_USB_DWC3_GADGET is not set
>>> CONFIG_USB_DWC3_GENERIC=y
>>> CONFIG_USB_GADGET=y
>>>
>>> E.g to only include host part of dwc3 and gadget from another driver,
>>> to i.e. save on binary size, produce following build error:
>>>
>>> aarch64-linux-gnu-ld.bfd: drivers/usb/dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
>>> drivers/usb/dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
>>>    undefined reference to `dwc3_gadget_uboot_handle_interrupt'
>>>
>>> I guess force select USB_DWC3_GADGET for USB_DWC3_GENERIC would make
>>> that build error disappear, and increase binary size as a result.
>>>
>>> For my RK3328 series [1] I will just revert to use USB_XHCI_DWC3 instead
>>> of using USB_DWC3_GENERIC on boards that enable peripheral use of otg
>>> port.
>>>
>>> [1] https://patchwork.ozlabs.org/patch/1904779/
>> 
>> Marek ended up doing this rework.
>> It's available for review here, if you want to have a look:
>> https://lore.kernel.org/all/20240614005309.34433-1-marek.vasut+renesas@mailbox.org/
> This should be upstream in some form now, is this thread still an open 
> topic or is this solved now ?

The thread reported 2 problems:

Using khadas-vim3_android_defconfig, I tried:
1. DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled
$ grep -e DM_USB_GADGET -e USB_DWC3_GADGET .config
CONFIG_DM_USB_GADGET=y
# CONFIG_USB_DWC3_GADGET is not set

2. USB_GADGET_DWC2_OTG + DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled.
$ grep -e USB_GADGET_DWC2_OTG -e DM_USB_GADGET -e USB_DWC3_GADGET .config
CONFIG_DM_USB_GADGET=y
# CONFIG_USB_DWC3_GADGET is not set
CONFIG_USB_GADGET_DWC2_OTG=y
# CONFIG_USB_GADGET_DWC2_OTG_PHY is not set
CONFIG_USB_GADGET_DWC2_OTG_PHY_BUS_WIDTH_8=y

In both case, I could build succesfully on master with commit 6d41f0a39d64 ("Prepare v2025.01")

So I think this is solved now. I'll let Jonas confirm.

Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Jonas Karlman]

Hi Marek and Mattijs,

On 2025-01-07 13:48, Mattijs Korpershoek wrote:
> Hi Marek,
> 
> On dim., janv. 05, 2025 at 20:29, Marek Vasut <marex@denx.de> wrote:
> 
>> On 6/18/24 9:15 AM, Mattijs Korpershoek wrote:
>>> Hi Jonas,
>>
>> Hello all,
>>
>>> On sam., mars 02, 2024 at 14:00, Jonas Karlman <jonas@kwiboo.se> wrote:
>>>
>>> [...]
>>>
>>>>>
>>>>> I will keep you posted.
>>>>
>>>> Thanks, much appreciated!
>>>>
>>>> Please also keep in mind that changing the interrupt handling probably
>>>> only fixes the second of the two build errors reported and fixed by this
>>>> patch.
>>>>
>>>> Trying to build with following will trigger the first build error, and
>>>> should not change because use of dm_usb_gadget_handle_interrupts() is
>>>> reworked.
>>>>
>>>> CONFIG_DM_USB_GADGET=y
>>>> CONFIG_USB_DWC3=y
>>>> # CONFIG_USB_DWC3_GADGET is not set
>>>> CONFIG_USB_DWC3_GENERIC=y
>>>> CONFIG_USB_GADGET=y
>>>>
>>>> E.g to only include host part of dwc3 and gadget from another driver,
>>>> to i.e. save on binary size, produce following build error:
>>>>
>>>> aarch64-linux-gnu-ld.bfd: drivers/usb/dwc3/dwc3-generic.o: in function `dm_usb_gadget_handle_interrupts':
>>>> drivers/usb/dwc3/dwc3-generic.c:201:(.text.dm_usb_gadget_handle_interrupts+0x10):
>>>>    undefined reference to `dwc3_gadget_uboot_handle_interrupt'
>>>>
>>>> I guess force select USB_DWC3_GADGET for USB_DWC3_GENERIC would make
>>>> that build error disappear, and increase binary size as a result.
>>>>
>>>> For my RK3328 series [1] I will just revert to use USB_XHCI_DWC3 instead
>>>> of using USB_DWC3_GENERIC on boards that enable peripheral use of otg
>>>> port.
>>>>
>>>> [1] https://patchwork.ozlabs.org/patch/1904779/
>>>
>>> Marek ended up doing this rework.
>>> It's available for review here, if you want to have a look:
>>> https://lore.kernel.org/all/20240614005309.34433-1-marek.vasut+renesas@mailbox.org/
>> This should be upstream in some form now, is this thread still an open 
>> topic or is this solved now ?
> 
> The thread reported 2 problems:
> 
> Using khadas-vim3_android_defconfig, I tried:
> 1. DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled
> $ grep -e DM_USB_GADGET -e USB_DWC3_GADGET .config
> CONFIG_DM_USB_GADGET=y
> # CONFIG_USB_DWC3_GADGET is not set
> 
> 2. USB_GADGET_DWC2_OTG + DM_USB_GADGET is enabled and USB_DWC3_GADGET is disabled.
> $ grep -e USB_GADGET_DWC2_OTG -e DM_USB_GADGET -e USB_DWC3_GADGET .config
> CONFIG_DM_USB_GADGET=y
> # CONFIG_USB_DWC3_GADGET is not set
> CONFIG_USB_GADGET_DWC2_OTG=y
> # CONFIG_USB_GADGET_DWC2_OTG_PHY is not set
> CONFIG_USB_GADGET_DWC2_OTG_PHY_BUS_WIDTH_8=y
> 
> In both case, I could build succesfully on master with commit 6d41f0a39d64 ("Prepare v2025.01")
> 
> So I think this is solved now. I'll let Jonas confirm.

Correct, this patch can now be ignored, this issue has been resolved and
I was able to complete the move to use USB_DWC3_GENERIC with DWC2 GADGET
for RK3328 in [2].

[2] https://patchwork.ozlabs.org/project/uboot/patch/20241008192719.2192430-1-jonas@kwiboo.se/

Please also take a look at an old unmerged cleanup patch at [3] :-)

https://patchwork.ozlabs.org/project/uboot/patch/20240121203818.2075381-1-jonas@kwiboo.se/

Regards,
Jonas

Re: [PATCH] usb: dwc3-generic: Fix build errors when USB_DWC3_GADGET is disabled

[Marek Vasut]

On 1/7/25 3:16 PM, Jonas Karlman wrote:

Hello everyone,

[...]

>> In both case, I could build succesfully on master with commit 6d41f0a39d64 ("Prepare v2025.01")
>>
>> So I think this is solved now. I'll let Jonas confirm.
> 
> Correct, this patch can now be ignored, this issue has been resolved and
> I was able to complete the move to use USB_DWC3_GENERIC with DWC2 GADGET
> for RK3328 in [2].
> 
> [2] https://patchwork.ozlabs.org/project/uboot/patch/20241008192719.2192430-1-jonas@kwiboo.se/
> 
> Please also take a look at an old unmerged cleanup patch at [3] :-)
> 
> https://patchwork.ozlabs.org/project/uboot/patch/20240121203818.2075381-1-jonas@kwiboo.se/
Thank you for confirming.

Shall I pick up the last patch ?