Patchwork [U-Boot,2/9] Fix bug when both DFU & ETHER are defined

login
register
mail settings
Submitter Pantelis Antoniou
Date Nov. 28, 2012, 12:43 p.m.
Message ID <1354106642-4587-3-git-send-email-panto@antoniou-consulting.com>
Download mbox | patch
Permalink /patch/202245/
State Superseded
Delegated to: Marek Vasut
Headers show

Comments

Marek Vasut - Nov. 28, 2012, 2:43 a.m.
Dear Pantelis Antoniou,

> When both CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined
> the makefile links objects twice.
> 
> Detect this and fix it with a not very elegant way in the
> makefile. Revisit and clean it later.
> 
> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> ---
>  drivers/usb/gadget/Makefile | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> index 040eaba..15206cd 100644
> --- a/drivers/usb/gadget/Makefile
> +++ b/drivers/usb/gadget/Makefile
> @@ -26,14 +26,23 @@ include $(TOPDIR)/config.mk
>  LIB	:= $(obj)libusb_gadget.o
> 
>  # new USB gadget layer dependencies
> +
> +# ugh; ugh; ugh common objects included twice
> +ifdef CONFIG_USB_GADGET
> +   COBJS-y += epautoconf.o config.o usbstring.o
> +else
> +  ifdef CONFIG_USB_ETHER
> +     COBJS-y += epautoconf.o config.o usbstring.o
> +  endif
> +endif

COBJS-$(CONFIG_ST) += st.o
COBJS-$(CONFIG_ST_ELSE) += st_else.o

if (both selected)
 scream and die.

> +
>  ifdef CONFIG_USB_GADGET
> -COBJS-y += epautoconf.o config.o usbstring.o
>  COBJS-$(CONFIG_USB_GADGET_S3C_UDC_OTG) += s3c_udc_otg.o
>  COBJS-$(CONFIG_USBDOWNLOAD_GADGET) += g_dnl.o
>  COBJS-$(CONFIG_DFU_FUNCTION) += f_dfu.o
>  endif
>  ifdef CONFIG_USB_ETHER
> -COBJS-y += ether.o epautoconf.o config.o usbstring.o
> +COBJS-y += ether.o
>  COBJS-$(CONFIG_USB_ETH_RNDIS) += rndis.o
>  COBJS-$(CONFIG_MV_UDC)	+= mv_udc.o
>  COBJS-$(CONFIG_CPU_PXA25X) += pxa25x_udc.o

Best regards,
Marek Vasut
Pantelis Antoniou - Nov. 28, 2012, 8:24 a.m.
Hi Marek,

On Nov 28, 2012, at 4:43 AM, Marek Vasut wrote:

> Dear Pantelis Antoniou,
> 
>> When both CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined
>> the makefile links objects twice.
>> 
>> Detect this and fix it with a not very elegant way in the
>> makefile. Revisit and clean it later.
>> 
>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>> ---
>> drivers/usb/gadget/Makefile | 13 +++++++++++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
>> index 040eaba..15206cd 100644
>> --- a/drivers/usb/gadget/Makefile
>> +++ b/drivers/usb/gadget/Makefile
>> @@ -26,14 +26,23 @@ include $(TOPDIR)/config.mk
>> LIB	:= $(obj)libusb_gadget.o
>> 
>> # new USB gadget layer dependencies
>> +
>> +# ugh; ugh; ugh common objects included twice
>> +ifdef CONFIG_USB_GADGET
>> +   COBJS-y += epautoconf.o config.o usbstring.o
>> +else
>> +  ifdef CONFIG_USB_ETHER
>> +     COBJS-y += epautoconf.o config.o usbstring.o
>> +  endif
>> +endif
> 
> COBJS-$(CONFIG_ST) += st.o
> COBJS-$(CONFIG_ST_ELSE) += st_else.o
> 
> if (both selected)
> scream and die.
> 

I fail to see how that would work for the problem at hand, namely
that if both CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined
epautoconf.o config.o usbstring.o are linked twice.

The proper thing to do would be to have a config symbol enabled
when either CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined.

Something like 

#if defined(CONFIG_USB_GADGET) || defined(CONFIG_USB_ETHER)
#define CONFIG_USB_GADGET_PLUMPING
#endif

in a header file someplace and do

COBJS-$(CONFIG_USB_GADGET_PLUMPING) += epautoconf.o config.o usbstring.o

etc.

This would require putting it either in every board file, or in a core
usb header.

The patch was the least intrusive method.

Regards

-- Pantelis


>> +
>> ifdef CONFIG_USB_GADGET
>> -COBJS-y += epautoconf.o config.o usbstring.o
>> COBJS-$(CONFIG_USB_GADGET_S3C_UDC_OTG) += s3c_udc_otg.o
>> COBJS-$(CONFIG_USBDOWNLOAD_GADGET) += g_dnl.o
>> COBJS-$(CONFIG_DFU_FUNCTION) += f_dfu.o
>> endif
>> ifdef CONFIG_USB_ETHER
>> -COBJS-y += ether.o epautoconf.o config.o usbstring.o
>> +COBJS-y += ether.o
>> COBJS-$(CONFIG_USB_ETH_RNDIS) += rndis.o
>> COBJS-$(CONFIG_MV_UDC)	+= mv_udc.o
>> COBJS-$(CONFIG_CPU_PXA25X) += pxa25x_udc.o
> 
> Best regards,
> Marek Vasut
Pantelis Antoniou - Nov. 28, 2012, 12:43 p.m.
When both CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined
the makefile links objects twice.

Detect this and fix it with a not very elegant way in the
makefile. Revisit and clean it later.

Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
---
 drivers/usb/gadget/Makefile | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
Marek Vasut - Nov. 29, 2012, 6:30 a.m.
Dear Pantelis Antoniou,

> Hi Marek,
> 
> On Nov 28, 2012, at 4:43 AM, Marek Vasut wrote:
> > Dear Pantelis Antoniou,
> > 
> >> When both CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined
> >> the makefile links objects twice.
> >> 
> >> Detect this and fix it with a not very elegant way in the
> >> makefile. Revisit and clean it later.
> >> 
> >> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> >> ---
> >> drivers/usb/gadget/Makefile | 13 +++++++++++--
> >> 1 file changed, 11 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> >> index 040eaba..15206cd 100644
> >> --- a/drivers/usb/gadget/Makefile
> >> +++ b/drivers/usb/gadget/Makefile
> >> @@ -26,14 +26,23 @@ include $(TOPDIR)/config.mk
> >> LIB	:= $(obj)libusb_gadget.o
> >> 
> >> # new USB gadget layer dependencies
> >> +
> >> +# ugh; ugh; ugh common objects included twice
> >> +ifdef CONFIG_USB_GADGET
> >> +   COBJS-y += epautoconf.o config.o usbstring.o
> >> +else
> >> +  ifdef CONFIG_USB_ETHER
> >> +     COBJS-y += epautoconf.o config.o usbstring.o
> >> +  endif
> >> +endif
> > 
> > COBJS-$(CONFIG_ST) += st.o
> > COBJS-$(CONFIG_ST_ELSE) += st_else.o
> > 
> > if (both selected)
> > scream and die.
> 
> I fail to see how that would work for the problem at hand, namely
> that if both CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined
> epautoconf.o config.o usbstring.o are linked twice.

Ah, now I understand the problem at hand.

> The proper thing to do would be to have a config symbol enabled
> when either CONFIG_USB_GADGET & CONFIG_USB_ETHER are defined.
> 
> Something like
> 
> #if defined(CONFIG_USB_GADGET) || defined(CONFIG_USB_ETHER)
> #define CONFIG_USB_GADGET_PLUMPING
> #endif
> 
> in a header file someplace and do

Can you not do "or" in conditional expression in Makefile?

> COBJS-$(CONFIG_USB_GADGET_PLUMPING) += epautoconf.o config.o usbstring.o
> 
> etc.
> 
> This would require putting it either in every board file, or in a core
> usb header.
> 
> The patch was the least intrusive method.
> 
> Regards
[...]

Patch

diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
index 040eaba..15206cd 100644
--- a/drivers/usb/gadget/Makefile
+++ b/drivers/usb/gadget/Makefile
@@ -26,14 +26,23 @@  include $(TOPDIR)/config.mk
 LIB	:= $(obj)libusb_gadget.o
 
 # new USB gadget layer dependencies
+
+# ugh; ugh; ugh common objects included twice
+ifdef CONFIG_USB_GADGET
+   COBJS-y += epautoconf.o config.o usbstring.o
+else
+  ifdef CONFIG_USB_ETHER
+     COBJS-y += epautoconf.o config.o usbstring.o
+  endif
+endif
+
 ifdef CONFIG_USB_GADGET
-COBJS-y += epautoconf.o config.o usbstring.o
 COBJS-$(CONFIG_USB_GADGET_S3C_UDC_OTG) += s3c_udc_otg.o
 COBJS-$(CONFIG_USBDOWNLOAD_GADGET) += g_dnl.o
 COBJS-$(CONFIG_DFU_FUNCTION) += f_dfu.o
 endif
 ifdef CONFIG_USB_ETHER
-COBJS-y += ether.o epautoconf.o config.o usbstring.o
+COBJS-y += ether.o
 COBJS-$(CONFIG_USB_ETH_RNDIS) += rndis.o
 COBJS-$(CONFIG_MV_UDC)	+= mv_udc.o
 COBJS-$(CONFIG_CPU_PXA25X) += pxa25x_udc.o