linux-next: build failure after merge of the usb and usb-gadget trees

linux-next: build failure after merge of the usb and usb-gadget trees

[Stephen Rothwell]

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

Hi all,

After merging the usb tree, today's linux-next build (arm
multi_v7_defconfig) failed like this:

arm-linux-gnueabi-ld: drivers/usb/dwc3/trace.o: in function `trace_raw_output_dwc3_log_ctrl':
trace.c:(.text+0x119c): undefined reference to `usb_decode_ctrl'

Caused by commit

  3db1b636c07e ("usb:gadget Separated decoding functions from dwc3 driver.")

I have used the usb tree from next-20190703 for today.

This also occurs in the usb-gadget tree so I have used the version of
that from next-20190703 as well.

-- 
Cheers,
Stephen Rothwell

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

Re: linux-next: build failure after merge of the usb and usb-gadget trees

[Greg KH]

On Thu, Jul 04, 2019 at 04:34:58PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the usb tree, today's linux-next build (arm
> multi_v7_defconfig) failed like this:
> 
> arm-linux-gnueabi-ld: drivers/usb/dwc3/trace.o: in function `trace_raw_output_dwc3_log_ctrl':
> trace.c:(.text+0x119c): undefined reference to `usb_decode_ctrl'
> 
> Caused by commit
> 
>   3db1b636c07e ("usb:gadget Separated decoding functions from dwc3 driver.")
> 
> I have used the usb tree from next-20190703 for today.
> 
> This also occurs in the usb-gadget tree so I have used the version of
> that from next-20190703 as well.

Odd, I thought I pulled the usb-gadget tree into mine.  Felipe, can you
take a look at this to see if I messed something up?

thanks,

greg k-h

Re: linux-next: build failure after merge of the usb and usb-gadget trees

[Felipe Balbi]

Hi,

On Thu, Jul 4, 2019 at 9:59 AM Greg KH <greg@kroah.com> wrote:
>
> On Thu, Jul 04, 2019 at 04:34:58PM +1000, Stephen Rothwell wrote:
> > Hi all,
> >
> > After merging the usb tree, today's linux-next build (arm
> > multi_v7_defconfig) failed like this:
> >
> > arm-linux-gnueabi-ld: drivers/usb/dwc3/trace.o: in function `trace_raw_output_dwc3_log_ctrl':
> > trace.c:(.text+0x119c): undefined reference to `usb_decode_ctrl'
> >
> > Caused by commit
> >
> >   3db1b636c07e ("usb:gadget Separated decoding functions from dwc3 driver.")
> >
> > I have used the usb tree from next-20190703 for today.
> >
> > This also occurs in the usb-gadget tree so I have used the version of
> > that from next-20190703 as well.
>
> Odd, I thought I pulled the usb-gadget tree into mine.  Felipe, can you
> take a look at this to see if I messed something up?

This looks like it was caused by Pawel's patches.

I'll try to reproduce here and see what's causing it.

-- 
balbi

Re: linux-next: build failure after merge of the usb and usb-gadget trees

[Stephen Rothwell]

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

Hi Greg,

On Thu, 4 Jul 2019 08:59:49 +0200 Greg KH <greg@kroah.com> wrote:
> 
> Odd, I thought I pulled the usb-gadget tree into mine.

You did. So after it failed in your tree and I used yesterday's
version of your tree, the build then failed after I merged the
usb-gadget tree (which I do after merging your tree).  The usb-gadget
tree contains commits that were not there yesterday.
-- 
Cheers,
Stephen Rothwell

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

RE: linux-next: build failure after merge of the usb and usb-gadget trees

[Pawel Laszczak]

>
>Hi,
>
>On Thu, Jul 4, 2019 at 9:59 AM Greg KH <greg@kroah.com> wrote:
>>
>> On Thu, Jul 04, 2019 at 04:34:58PM +1000, Stephen Rothwell wrote:
>> > Hi all,
>> >
>> > After merging the usb tree, today's linux-next build (arm
>> > multi_v7_defconfig) failed like this:
>> >
>> > arm-linux-gnueabi-ld: drivers/usb/dwc3/trace.o: in function `trace_raw_output_dwc3_log_ctrl':
>> > trace.c:(.text+0x119c): undefined reference to `usb_decode_ctrl'
>> >
>> > Caused by commit
>> >
>> >   3db1b636c07e ("usb:gadget Separated decoding functions from dwc3 driver.")
>> >
>> > I have used the usb tree from next-20190703 for today.
>> >
>> > This also occurs in the usb-gadget tree so I have used the version of
>> > that from next-20190703 as well.
>>
>> Odd, I thought I pulled the usb-gadget tree into mine.  Felipe, can you
>> take a look at this to see if I messed something up?
>
>This looks like it was caused by Pawel's patches.
>
>I'll try to reproduce here and see what's causing it.

Problem is in my Patch. I reproduced it, but I don't understand why compiler 
complains about usb_decode_ctrl. It's compiled into libcomposite.ko and
declaration is in drivers/usb/gadget.h. 

>
>--
>balbi

RE: linux-next: build failure after merge of the usb and usb-gadget trees

[Felipe Balbi]

Hi,

Pawel Laszczak <pawell@cadence.com> writes:

>>
>>Hi,
>>
>>On Thu, Jul 4, 2019 at 9:59 AM Greg KH <greg@kroah.com> wrote:
>>>
>>> On Thu, Jul 04, 2019 at 04:34:58PM +1000, Stephen Rothwell wrote:
>>> > Hi all,
>>> >
>>> > After merging the usb tree, today's linux-next build (arm
>>> > multi_v7_defconfig) failed like this:
>>> >
>>> > arm-linux-gnueabi-ld: drivers/usb/dwc3/trace.o: in function `trace_raw_output_dwc3_log_ctrl':
>>> > trace.c:(.text+0x119c): undefined reference to `usb_decode_ctrl'
>>> >
>>> > Caused by commit
>>> >
>>> >   3db1b636c07e ("usb:gadget Separated decoding functions from dwc3 driver.")
>>> >
>>> > I have used the usb tree from next-20190703 for today.
>>> >
>>> > This also occurs in the usb-gadget tree so I have used the version of
>>> > that from next-20190703 as well.
>>>
>>> Odd, I thought I pulled the usb-gadget tree into mine.  Felipe, can you
>>> take a look at this to see if I messed something up?
>>
>>This looks like it was caused by Pawel's patches.
>>
>>I'll try to reproduce here and see what's causing it.
>
> Problem is in my Patch. I reproduced it, but I don't understand why compiler 
> complains about usb_decode_ctrl. It's compiled into libcomposite.ko and
> declaration is in drivers/usb/gadget.h. 

That's because in multi_v7_defconfig dwc3 is built-in while libcomposite
is a module:

CONFIG_USB_DWC3=y
CONFIG_USB_LIBCOMPOSITE=m


I remember that when you were doing this work, I asked you to move
functions to usb/common. Why did you deviate from that suggestion? It's
clear that decoding a ctrl request can be used by peripheral and host
and we wouldn't have to deal with this problem if you had just followed
the suggestion.

Now we have to come up with a way to fix this that doesn't involve
reverting my part2 tag in its entirety because there are other important
things there.

This is what I get for trusting people to do their part. I couldn't even
compile test this since I don't have ARM compilers anymore (actually,
just installed to test). Your customer, however, uses ARM cores so I
would expect you to have at least compile tested this on ARM. How come
this wasn't verified by anybody at TI?

TI used to have automated testing for many of the important defconfigs,
is that completely gone? Are you guys relying entirely on linux-next?

Greg, if you prefer, please revert my part2 tag. If you do so, please
let me know so I can drop the tag and commits from my tree as well.

Pawel, please make sure this never happens again. It's pretty simple to
avoid this sort of thing. I'll keep ARM compiler installed for
build-testing as well.

-- 
balbi

Re: linux-next: build failure after merge of the usb and usb-gadget trees

[Greg KH]

On Thu, Jul 04, 2019 at 11:25:16AM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Pawel Laszczak <pawell@cadence.com> writes:
> 
> >>
> >>Hi,
> >>
> >>On Thu, Jul 4, 2019 at 9:59 AM Greg KH <greg@kroah.com> wrote:
> >>>
> >>> On Thu, Jul 04, 2019 at 04:34:58PM +1000, Stephen Rothwell wrote:
> >>> > Hi all,
> >>> >
> >>> > After merging the usb tree, today's linux-next build (arm
> >>> > multi_v7_defconfig) failed like this:
> >>> >
> >>> > arm-linux-gnueabi-ld: drivers/usb/dwc3/trace.o: in function `trace_raw_output_dwc3_log_ctrl':
> >>> > trace.c:(.text+0x119c): undefined reference to `usb_decode_ctrl'
> >>> >
> >>> > Caused by commit
> >>> >
> >>> >   3db1b636c07e ("usb:gadget Separated decoding functions from dwc3 driver.")
> >>> >
> >>> > I have used the usb tree from next-20190703 for today.
> >>> >
> >>> > This also occurs in the usb-gadget tree so I have used the version of
> >>> > that from next-20190703 as well.
> >>>
> >>> Odd, I thought I pulled the usb-gadget tree into mine.  Felipe, can you
> >>> take a look at this to see if I messed something up?
> >>
> >>This looks like it was caused by Pawel's patches.
> >>
> >>I'll try to reproduce here and see what's causing it.
> >
> > Problem is in my Patch. I reproduced it, but I don't understand why compiler 
> > complains about usb_decode_ctrl. It's compiled into libcomposite.ko and
> > declaration is in drivers/usb/gadget.h. 
> 
> That's because in multi_v7_defconfig dwc3 is built-in while libcomposite
> is a module:
> 
> CONFIG_USB_DWC3=y
> CONFIG_USB_LIBCOMPOSITE=m
> 
> 
> I remember that when you were doing this work, I asked you to move
> functions to usb/common. Why did you deviate from that suggestion? It's
> clear that decoding a ctrl request can be used by peripheral and host
> and we wouldn't have to deal with this problem if you had just followed
> the suggestion.
> 
> Now we have to come up with a way to fix this that doesn't involve
> reverting my part2 tag in its entirety because there are other important
> things there.
> 
> This is what I get for trusting people to do their part. I couldn't even
> compile test this since I don't have ARM compilers anymore (actually,
> just installed to test). Your customer, however, uses ARM cores so I
> would expect you to have at least compile tested this on ARM. How come
> this wasn't verified by anybody at TI?
> 
> TI used to have automated testing for many of the important defconfigs,
> is that completely gone? Are you guys relying entirely on linux-next?
> 
> Greg, if you prefer, please revert my part2 tag. If you do so, please
> let me know so I can drop the tag and commits from my tree as well.

How do I revert a tag?  How about I just revert individual commits,
which ones should I revert?

thanks,

greg k-h

Re: linux-next: build failure after merge of the usb and usb-gadget trees

[Felipe Balbi]

Hi,

Greg KH <greg@kroah.com> writes:

> On Thu, Jul 04, 2019 at 11:25:16AM +0300, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Pawel Laszczak <pawell@cadence.com> writes:
>> 
>> >>
>> >>Hi,
>> >>
>> >>On Thu, Jul 4, 2019 at 9:59 AM Greg KH <greg@kroah.com> wrote:
>> >>>
>> >>> On Thu, Jul 04, 2019 at 04:34:58PM +1000, Stephen Rothwell wrote:
>> >>> > Hi all,
>> >>> >
>> >>> > After merging the usb tree, today's linux-next build (arm
>> >>> > multi_v7_defconfig) failed like this:
>> >>> >
>> >>> > arm-linux-gnueabi-ld: drivers/usb/dwc3/trace.o: in function `trace_raw_output_dwc3_log_ctrl':
>> >>> > trace.c:(.text+0x119c): undefined reference to `usb_decode_ctrl'
>> >>> >
>> >>> > Caused by commit
>> >>> >
>> >>> >   3db1b636c07e ("usb:gadget Separated decoding functions from dwc3 driver.")
>> >>> >
>> >>> > I have used the usb tree from next-20190703 for today.
>> >>> >
>> >>> > This also occurs in the usb-gadget tree so I have used the version of
>> >>> > that from next-20190703 as well.
>> >>>
>> >>> Odd, I thought I pulled the usb-gadget tree into mine.  Felipe, can you
>> >>> take a look at this to see if I messed something up?
>> >>
>> >>This looks like it was caused by Pawel's patches.
>> >>
>> >>I'll try to reproduce here and see what's causing it.
>> >
>> > Problem is in my Patch. I reproduced it, but I don't understand why compiler 
>> > complains about usb_decode_ctrl. It's compiled into libcomposite.ko and
>> > declaration is in drivers/usb/gadget.h. 
>> 
>> That's because in multi_v7_defconfig dwc3 is built-in while libcomposite
>> is a module:
>> 
>> CONFIG_USB_DWC3=y
>> CONFIG_USB_LIBCOMPOSITE=m
>> 
>> 
>> I remember that when you were doing this work, I asked you to move
>> functions to usb/common. Why did you deviate from that suggestion? It's
>> clear that decoding a ctrl request can be used by peripheral and host
>> and we wouldn't have to deal with this problem if you had just followed
>> the suggestion.
>> 
>> Now we have to come up with a way to fix this that doesn't involve
>> reverting my part2 tag in its entirety because there are other important
>> things there.
>> 
>> This is what I get for trusting people to do their part. I couldn't even
>> compile test this since I don't have ARM compilers anymore (actually,
>> just installed to test). Your customer, however, uses ARM cores so I
>> would expect you to have at least compile tested this on ARM. How come
>> this wasn't verified by anybody at TI?
>> 
>> TI used to have automated testing for many of the important defconfigs,
>> is that completely gone? Are you guys relying entirely on linux-next?
>> 
>> Greg, if you prefer, please revert my part2 tag. If you do so, please
>> let me know so I can drop the tag and commits from my tree as well.
>
> How do I revert a tag?  How about I just revert individual commits,
> which ones should I revert?

Anything from Pawel. Here's the full list:

573aff747ee3 usb:cdns3 Fix for stuck packets in on-chip OUT buffer.
8bc1901ca7b0 usb:cdns3 Add Cadence USB3 DRD Driver
c2af6b07803e usb:gadget Simplify usb_decode_get_set_descriptor function.
ca888ce7495e usb:gadget Patch simplify usb_decode_set_clear_feature function.
3db1b636c07e usb:gadget Separated decoding functions from dwc3 driver.
e8a8b40cc892 dt-bindings: add binding for USBSS-DRD controller.

I just tested a branch without these patches and it builds fine:

$ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- multi_v7_defconfig
  HOSTCC  scripts/kconfig/conf.o
  HOSTCC  scripts/kconfig/confdata.o
  HOSTCC  scripts/kconfig/expr.o
  HOSTCC  scripts/kconfig/lexer.lex.o
  HOSTCC  scripts/kconfig/parser.tab.o
  HOSTCC  scripts/kconfig/preprocess.o
  HOSTCC  scripts/kconfig/symbol.o
  HOSTLD  scripts/kconfig/conf
#
# configuration written to .config
#
$ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- -j32 -s
$

-- 
balbi

RE: linux-next: build failure after merge of the usb and usb-gadget trees

[Pawel Laszczak]

>
>
>Hi,
>
>Pawel Laszczak <pawell@cadence.com> writes:
>
>>>
>>>Hi,
>>>
>>>On Thu, Jul 4, 2019 at 9:59 AM Greg KH <greg@kroah.com> wrote:
>>>>
>>>> On Thu, Jul 04, 2019 at 04:34:58PM +1000, Stephen Rothwell wrote:
>>>> > Hi all,
>>>> >
>>>> > After merging the usb tree, today's linux-next build (arm
>>>> > multi_v7_defconfig) failed like this:
>>>> >
>>>> > arm-linux-gnueabi-ld: drivers/usb/dwc3/trace.o: in function `trace_raw_output_dwc3_log_ctrl':
>>>> > trace.c:(.text+0x119c): undefined reference to `usb_decode_ctrl'
>>>> >
>>>> > Caused by commit
>>>> >
>>>> >   3db1b636c07e ("usb:gadget Separated decoding functions from dwc3 driver.")
>>>> >
>>>> > I have used the usb tree from next-20190703 for today.
>>>> >
>>>> > This also occurs in the usb-gadget tree so I have used the version of
>>>> > that from next-20190703 as well.
>>>>
>>>> Odd, I thought I pulled the usb-gadget tree into mine.  Felipe, can you
>>>> take a look at this to see if I messed something up?
>>>
>>>This looks like it was caused by Pawel's patches.
>>>
>>>I'll try to reproduce here and see what's causing it.
>>
>> Problem is in my Patch. I reproduced it, but I don't understand why compiler
>> complains about usb_decode_ctrl. It's compiled into libcomposite.ko and
>> declaration is in drivers/usb/gadget.h.
>
>That's because in multi_v7_defconfig dwc3 is built-in while libcomposite
>is a module:
>
>CONFIG_USB_DWC3=y
>CONFIG_USB_LIBCOMPOSITE=m
>
>
>I remember that when you were doing this work, I asked you to move
>functions to usb/common. Why did you deviate from that suggestion? It's
>clear that decoding a ctrl request can be used by peripheral and host
>and we wouldn't have to deal with this problem if you had just followed
>the suggestion.

Some time ago Greg wrote: 
" It's nice to have these in a common place, but you just bloated all of
the USB-enabled systems in the world for the use of 2 odd-ball system
controllers that almost no one has :) "

So I moved these functions to gadget directory. 

It was mistake that I added debug.c file to libcomposite.ko.

>
>Now we have to come up with a way to fix this that doesn't involve
>reverting my part2 tag in its entirety because there are other important
>things there.
>
>This is what I get for trusting people to do their part. I couldn't even
>compile test this since I don't have ARM compilers anymore (actually,
>just installed to test). Your customer, however, uses ARM cores so I
>would expect you to have at least compile tested this on ARM. How come
>this wasn't verified by anybody at TI?
>
>TI used to have automated testing for many of the important defconfigs,
>is that completely gone? Are you guys relying entirely on linux-next?
>
>Greg, if you prefer, please revert my part2 tag. If you do so, please
>let me know so I can drop the tag and commits from my tree as well.
>
>Pawel, please make sure this never happens again. It's pretty simple to
>avoid this sort of thing. I'll keep ARM compiler installed for
>build-testing as well.
>
>--
>balbi

RE: linux-next: build failure after merge of the usb and usb-gadget trees

[Pawel Laszczak]

>>
>>
>>Hi,
>>
>>Pawel Laszczak <pawell@cadence.com> writes:
>>
>>>>
>>>>Hi,
>>>>
>>>>On Thu, Jul 4, 2019 at 9:59 AM Greg KH <greg@kroah.com> wrote:
>>>>>
>>>>> On Thu, Jul 04, 2019 at 04:34:58PM +1000, Stephen Rothwell wrote:
>>>>> > Hi all,
>>>>> >
>>>>> > After merging the usb tree, today's linux-next build (arm
>>>>> > multi_v7_defconfig) failed like this:
>>>>> >
>>>>> > arm-linux-gnueabi-ld: drivers/usb/dwc3/trace.o: in function `trace_raw_output_dwc3_log_ctrl':
>>>>> > trace.c:(.text+0x119c): undefined reference to `usb_decode_ctrl'
>>>>> >
>>>>> > Caused by commit
>>>>> >
>>>>> >   3db1b636c07e ("usb:gadget Separated decoding functions from dwc3 driver.")
>>>>> >
>>>>> > I have used the usb tree from next-20190703 for today.
>>>>> >
>>>>> > This also occurs in the usb-gadget tree so I have used the version of
>>>>> > that from next-20190703 as well.
>>>>>
>>>>> Odd, I thought I pulled the usb-gadget tree into mine.  Felipe, can you
>>>>> take a look at this to see if I messed something up?
>>>>
>>>>This looks like it was caused by Pawel's patches.
>>>>
>>>>I'll try to reproduce here and see what's causing it.
>>>
>>> Problem is in my Patch. I reproduced it, but I don't understand why compiler
>>> complains about usb_decode_ctrl. It's compiled into libcomposite.ko and
>>> declaration is in drivers/usb/gadget.h.
>>
>>That's because in multi_v7_defconfig dwc3 is built-in while libcomposite
>>is a module:
>>
>>CONFIG_USB_DWC3=y
>>CONFIG_USB_LIBCOMPOSITE=m
>>
>>
>>I remember that when you were doing this work, I asked you to move
>>functions to usb/common. Why did you deviate from that suggestion? It's
>>clear that decoding a ctrl request can be used by peripheral and host
>>and we wouldn't have to deal with this problem if you had just followed
>>the suggestion.
>
>Some time ago Greg wrote:
>" It's nice to have these in a common place, but you just bloated all of
>the USB-enabled systems in the world for the use of 2 odd-ball system
>controllers that almost no one has :) "
>
>So I moved these functions to gadget directory.
>
>It was mistake that I added debug.c file to libcomposite.ko.
>

I think that the best choice is leaving debug.c file 
In drivers/usb/gadget/ directory. 

But to do this I must to add this file to drivers/usb/dwc3/Makefile file and 
drivers/usb/cdns3/Makefile. The code will be compiled into both drivers,
It will increase the size of kernel only when these driver will be enabled.

What do you think about such solution ? 

>>
>>Now we have to come up with a way to fix this that doesn't involve
>>reverting my part2 tag in its entirety because there are other important
>>things there.
>>
>>This is what I get for trusting people to do their part. I couldn't even
>>compile test this since I don't have ARM compilers anymore (actually,
>>just installed to test). Your customer, however, uses ARM cores so I
>>would expect you to have at least compile tested this on ARM. How come
>>this wasn't verified by anybody at TI?
>>
>>TI used to have automated testing for many of the important defconfigs,
>>is that completely gone? Are you guys relying entirely on linux-next?
>>
>>Greg, if you prefer, please revert my part2 tag. If you do so, please
>>let me know so I can drop the tag and commits from my tree as well.
>>
>>Pawel, please make sure this never happens again. It's pretty simple to
>>avoid this sort of thing. I'll keep ARM compiler installed for
>>build-testing as well.
>>
>>--
>>balbi

RE: linux-next: build failure after merge of the usb and usb-gadget trees

[Felipe Balbi]

Hi,

Pawel Laszczak <pawell@cadence.com> writes:

>>
>>
>>Hi,
>>
>>Pawel Laszczak <pawell@cadence.com> writes:
>>
>>>>
>>>>Hi,
>>>>
>>>>On Thu, Jul 4, 2019 at 9:59 AM Greg KH <greg@kroah.com> wrote:
>>>>>
>>>>> On Thu, Jul 04, 2019 at 04:34:58PM +1000, Stephen Rothwell wrote:
>>>>> > Hi all,
>>>>> >
>>>>> > After merging the usb tree, today's linux-next build (arm
>>>>> > multi_v7_defconfig) failed like this:
>>>>> >
>>>>> > arm-linux-gnueabi-ld: drivers/usb/dwc3/trace.o: in function `trace_raw_output_dwc3_log_ctrl':
>>>>> > trace.c:(.text+0x119c): undefined reference to `usb_decode_ctrl'
>>>>> >
>>>>> > Caused by commit
>>>>> >
>>>>> >   3db1b636c07e ("usb:gadget Separated decoding functions from dwc3 driver.")
>>>>> >
>>>>> > I have used the usb tree from next-20190703 for today.
>>>>> >
>>>>> > This also occurs in the usb-gadget tree so I have used the version of
>>>>> > that from next-20190703 as well.
>>>>>
>>>>> Odd, I thought I pulled the usb-gadget tree into mine.  Felipe, can you
>>>>> take a look at this to see if I messed something up?
>>>>
>>>>This looks like it was caused by Pawel's patches.
>>>>
>>>>I'll try to reproduce here and see what's causing it.
>>>
>>> Problem is in my Patch. I reproduced it, but I don't understand why compiler
>>> complains about usb_decode_ctrl. It's compiled into libcomposite.ko and
>>> declaration is in drivers/usb/gadget.h.
>>
>>That's because in multi_v7_defconfig dwc3 is built-in while libcomposite
>>is a module:
>>
>>CONFIG_USB_DWC3=y
>>CONFIG_USB_LIBCOMPOSITE=m
>>
>>
>>I remember that when you were doing this work, I asked you to move
>>functions to usb/common. Why did you deviate from that suggestion? It's
>>clear that decoding a ctrl request can be used by peripheral and host
>>and we wouldn't have to deal with this problem if you had just followed
>>the suggestion.
>
> Some time ago Greg wrote: 
> " It's nice to have these in a common place, but you just bloated all of
> the USB-enabled systems in the world for the use of 2 odd-ball system
> controllers that almost no one has :) "
>
> So I moved these functions to gadget directory. 
>
> It was mistake that I added debug.c file to libcomposite.ko.

The plan is to use this decoding function for xHCI as well. Other host
controllers can use it as well.

The biggest mistake was to put this under gadget. What you should have
done was create a file under usb/common that only gets compile in if
tracing is enabled.

Then there's no bloating unless you have a kernel purposefuly built for
debugging and tracing.

Greg, does that work for you?

-- 
balbi

RE: linux-next: build failure after merge of the usb and usb-gadget trees

[Felipe Balbi]

Hi,

Pawel Laszczak <pawell@cadence.com> writes:

>>>
>>>
>>>Hi,
>>>
>>>Pawel Laszczak <pawell@cadence.com> writes:
>>>
>>>>>
>>>>>Hi,
>>>>>
>>>>>On Thu, Jul 4, 2019 at 9:59 AM Greg KH <greg@kroah.com> wrote:
>>>>>>
>>>>>> On Thu, Jul 04, 2019 at 04:34:58PM +1000, Stephen Rothwell wrote:
>>>>>> > Hi all,
>>>>>> >
>>>>>> > After merging the usb tree, today's linux-next build (arm
>>>>>> > multi_v7_defconfig) failed like this:
>>>>>> >
>>>>>> > arm-linux-gnueabi-ld: drivers/usb/dwc3/trace.o: in function `trace_raw_output_dwc3_log_ctrl':
>>>>>> > trace.c:(.text+0x119c): undefined reference to `usb_decode_ctrl'
>>>>>> >
>>>>>> > Caused by commit
>>>>>> >
>>>>>> >   3db1b636c07e ("usb:gadget Separated decoding functions from dwc3 driver.")
>>>>>> >
>>>>>> > I have used the usb tree from next-20190703 for today.
>>>>>> >
>>>>>> > This also occurs in the usb-gadget tree so I have used the version of
>>>>>> > that from next-20190703 as well.
>>>>>>
>>>>>> Odd, I thought I pulled the usb-gadget tree into mine.  Felipe, can you
>>>>>> take a look at this to see if I messed something up?
>>>>>
>>>>>This looks like it was caused by Pawel's patches.
>>>>>
>>>>>I'll try to reproduce here and see what's causing it.
>>>>
>>>> Problem is in my Patch. I reproduced it, but I don't understand why compiler
>>>> complains about usb_decode_ctrl. It's compiled into libcomposite.ko and
>>>> declaration is in drivers/usb/gadget.h.
>>>
>>>That's because in multi_v7_defconfig dwc3 is built-in while libcomposite
>>>is a module:
>>>
>>>CONFIG_USB_DWC3=y
>>>CONFIG_USB_LIBCOMPOSITE=m
>>>
>>>
>>>I remember that when you were doing this work, I asked you to move
>>>functions to usb/common. Why did you deviate from that suggestion? It's
>>>clear that decoding a ctrl request can be used by peripheral and host
>>>and we wouldn't have to deal with this problem if you had just followed
>>>the suggestion.
>>
>>Some time ago Greg wrote:
>>" It's nice to have these in a common place, but you just bloated all of
>>the USB-enabled systems in the world for the use of 2 odd-ball system
>>controllers that almost no one has :) "
>>
>>So I moved these functions to gadget directory.
>>
>>It was mistake that I added debug.c file to libcomposite.ko.
>>
>
> I think that the best choice is leaving debug.c file 
> In drivers/usb/gadget/ directory. 
>
> But to do this I must to add this file to drivers/usb/dwc3/Makefile file and 
> drivers/usb/cdns3/Makefile. The code will be compiled into both drivers,
> It will increase the size of kernel only when these driver will be enabled.
>
> What do you think about such solution ? 

Frankly, I think it's not a solution :-)

If we _must_ keep it under drivers/usb/gadget, then we need to find a
way to build this so that if any user is built-in, that file has to be
built-in as well.

-- 
balbi

Re: linux-next: build failure after merge of the usb and usb-gadget trees

[Greg KH]

On Thu, Jul 04, 2019 at 11:45:11AM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Greg KH <greg@kroah.com> writes:
> 
> > On Thu, Jul 04, 2019 at 11:25:16AM +0300, Felipe Balbi wrote:
> >> 
> >> Hi,
> >> 
> >> Pawel Laszczak <pawell@cadence.com> writes:
> >> 
> >> >>
> >> >>Hi,
> >> >>
> >> >>On Thu, Jul 4, 2019 at 9:59 AM Greg KH <greg@kroah.com> wrote:
> >> >>>
> >> >>> On Thu, Jul 04, 2019 at 04:34:58PM +1000, Stephen Rothwell wrote:
> >> >>> > Hi all,
> >> >>> >
> >> >>> > After merging the usb tree, today's linux-next build (arm
> >> >>> > multi_v7_defconfig) failed like this:
> >> >>> >
> >> >>> > arm-linux-gnueabi-ld: drivers/usb/dwc3/trace.o: in function `trace_raw_output_dwc3_log_ctrl':
> >> >>> > trace.c:(.text+0x119c): undefined reference to `usb_decode_ctrl'
> >> >>> >
> >> >>> > Caused by commit
> >> >>> >
> >> >>> >   3db1b636c07e ("usb:gadget Separated decoding functions from dwc3 driver.")
> >> >>> >
> >> >>> > I have used the usb tree from next-20190703 for today.
> >> >>> >
> >> >>> > This also occurs in the usb-gadget tree so I have used the version of
> >> >>> > that from next-20190703 as well.
> >> >>>
> >> >>> Odd, I thought I pulled the usb-gadget tree into mine.  Felipe, can you
> >> >>> take a look at this to see if I messed something up?
> >> >>
> >> >>This looks like it was caused by Pawel's patches.
> >> >>
> >> >>I'll try to reproduce here and see what's causing it.
> >> >
> >> > Problem is in my Patch. I reproduced it, but I don't understand why compiler 
> >> > complains about usb_decode_ctrl. It's compiled into libcomposite.ko and
> >> > declaration is in drivers/usb/gadget.h. 
> >> 
> >> That's because in multi_v7_defconfig dwc3 is built-in while libcomposite
> >> is a module:
> >> 
> >> CONFIG_USB_DWC3=y
> >> CONFIG_USB_LIBCOMPOSITE=m
> >> 
> >> 
> >> I remember that when you were doing this work, I asked you to move
> >> functions to usb/common. Why did you deviate from that suggestion? It's
> >> clear that decoding a ctrl request can be used by peripheral and host
> >> and we wouldn't have to deal with this problem if you had just followed
> >> the suggestion.
> >> 
> >> Now we have to come up with a way to fix this that doesn't involve
> >> reverting my part2 tag in its entirety because there are other important
> >> things there.
> >> 
> >> This is what I get for trusting people to do their part. I couldn't even
> >> compile test this since I don't have ARM compilers anymore (actually,
> >> just installed to test). Your customer, however, uses ARM cores so I
> >> would expect you to have at least compile tested this on ARM. How come
> >> this wasn't verified by anybody at TI?
> >> 
> >> TI used to have automated testing for many of the important defconfigs,
> >> is that completely gone? Are you guys relying entirely on linux-next?
> >> 
> >> Greg, if you prefer, please revert my part2 tag. If you do so, please
> >> let me know so I can drop the tag and commits from my tree as well.
> >
> > How do I revert a tag?  How about I just revert individual commits,
> > which ones should I revert?
> 
> Anything from Pawel. Here's the full list:
> 
> 573aff747ee3 usb:cdns3 Fix for stuck packets in on-chip OUT buffer.
> 8bc1901ca7b0 usb:cdns3 Add Cadence USB3 DRD Driver
> c2af6b07803e usb:gadget Simplify usb_decode_get_set_descriptor function.
> ca888ce7495e usb:gadget Patch simplify usb_decode_set_clear_feature function.
> 3db1b636c07e usb:gadget Separated decoding functions from dwc3 driver.
> e8a8b40cc892 dt-bindings: add binding for USBSS-DRD controller.
> 
> I just tested a branch without these patches and it builds fine:
> 
> $ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- multi_v7_defconfig
>   HOSTCC  scripts/kconfig/conf.o
>   HOSTCC  scripts/kconfig/confdata.o
>   HOSTCC  scripts/kconfig/expr.o
>   HOSTCC  scripts/kconfig/lexer.lex.o
>   HOSTCC  scripts/kconfig/parser.tab.o
>   HOSTCC  scripts/kconfig/preprocess.o
>   HOSTCC  scripts/kconfig/symbol.o
>   HOSTLD  scripts/kconfig/conf
> #
> # configuration written to .config
> #
> $ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- -j32 -s
> $

All now reverted.

When these are resent, I want to review them as well as I thought the
cdns3 driver still needed work before it could be merged...

thanks,

greg k-h

Re: linux-next: build failure after merge of the usb and usb-gadget trees

[Greg KH]

On Thu, Jul 04, 2019 at 12:44:08PM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Pawel Laszczak <pawell@cadence.com> writes:
> 
> >>
> >>
> >>Hi,
> >>
> >>Pawel Laszczak <pawell@cadence.com> writes:
> >>
> >>>>
> >>>>Hi,
> >>>>
> >>>>On Thu, Jul 4, 2019 at 9:59 AM Greg KH <greg@kroah.com> wrote:
> >>>>>
> >>>>> On Thu, Jul 04, 2019 at 04:34:58PM +1000, Stephen Rothwell wrote:
> >>>>> > Hi all,
> >>>>> >
> >>>>> > After merging the usb tree, today's linux-next build (arm
> >>>>> > multi_v7_defconfig) failed like this:
> >>>>> >
> >>>>> > arm-linux-gnueabi-ld: drivers/usb/dwc3/trace.o: in function `trace_raw_output_dwc3_log_ctrl':
> >>>>> > trace.c:(.text+0x119c): undefined reference to `usb_decode_ctrl'
> >>>>> >
> >>>>> > Caused by commit
> >>>>> >
> >>>>> >   3db1b636c07e ("usb:gadget Separated decoding functions from dwc3 driver.")
> >>>>> >
> >>>>> > I have used the usb tree from next-20190703 for today.
> >>>>> >
> >>>>> > This also occurs in the usb-gadget tree so I have used the version of
> >>>>> > that from next-20190703 as well.
> >>>>>
> >>>>> Odd, I thought I pulled the usb-gadget tree into mine.  Felipe, can you
> >>>>> take a look at this to see if I messed something up?
> >>>>
> >>>>This looks like it was caused by Pawel's patches.
> >>>>
> >>>>I'll try to reproduce here and see what's causing it.
> >>>
> >>> Problem is in my Patch. I reproduced it, but I don't understand why compiler
> >>> complains about usb_decode_ctrl. It's compiled into libcomposite.ko and
> >>> declaration is in drivers/usb/gadget.h.
> >>
> >>That's because in multi_v7_defconfig dwc3 is built-in while libcomposite
> >>is a module:
> >>
> >>CONFIG_USB_DWC3=y
> >>CONFIG_USB_LIBCOMPOSITE=m
> >>
> >>
> >>I remember that when you were doing this work, I asked you to move
> >>functions to usb/common. Why did you deviate from that suggestion? It's
> >>clear that decoding a ctrl request can be used by peripheral and host
> >>and we wouldn't have to deal with this problem if you had just followed
> >>the suggestion.
> >
> > Some time ago Greg wrote: 
> > " It's nice to have these in a common place, but you just bloated all of
> > the USB-enabled systems in the world for the use of 2 odd-ball system
> > controllers that almost no one has :) "
> >
> > So I moved these functions to gadget directory. 
> >
> > It was mistake that I added debug.c file to libcomposite.ko.
> 
> The plan is to use this decoding function for xHCI as well. Other host
> controllers can use it as well.
> 
> The biggest mistake was to put this under gadget. What you should have
> done was create a file under usb/common that only gets compile in if
> tracing is enabled.
> 
> Then there's no bloating unless you have a kernel purposefuly built for
> debugging and tracing.
> 
> Greg, does that work for you?

I guess, but I'd like to see patches before answering that :)

thanks,

greg k-h

Re: linux-next: build failure after merge of the usb and usb-gadget trees

[Felipe Balbi]

Hi,

Greg KH <greg@kroah.com> writes:

> On Thu, Jul 04, 2019 at 12:44:08PM +0300, Felipe Balbi wrote:
>> 
>> Hi,
>> 
>> Pawel Laszczak <pawell@cadence.com> writes:
>> 
>> >>
>> >>
>> >>Hi,
>> >>
>> >>Pawel Laszczak <pawell@cadence.com> writes:
>> >>
>> >>>>
>> >>>>Hi,
>> >>>>
>> >>>>On Thu, Jul 4, 2019 at 9:59 AM Greg KH <greg@kroah.com> wrote:
>> >>>>>
>> >>>>> On Thu, Jul 04, 2019 at 04:34:58PM +1000, Stephen Rothwell wrote:
>> >>>>> > Hi all,
>> >>>>> >
>> >>>>> > After merging the usb tree, today's linux-next build (arm
>> >>>>> > multi_v7_defconfig) failed like this:
>> >>>>> >
>> >>>>> > arm-linux-gnueabi-ld: drivers/usb/dwc3/trace.o: in function `trace_raw_output_dwc3_log_ctrl':
>> >>>>> > trace.c:(.text+0x119c): undefined reference to `usb_decode_ctrl'
>> >>>>> >
>> >>>>> > Caused by commit
>> >>>>> >
>> >>>>> >   3db1b636c07e ("usb:gadget Separated decoding functions from dwc3 driver.")
>> >>>>> >
>> >>>>> > I have used the usb tree from next-20190703 for today.
>> >>>>> >
>> >>>>> > This also occurs in the usb-gadget tree so I have used the version of
>> >>>>> > that from next-20190703 as well.
>> >>>>>
>> >>>>> Odd, I thought I pulled the usb-gadget tree into mine.  Felipe, can you
>> >>>>> take a look at this to see if I messed something up?
>> >>>>
>> >>>>This looks like it was caused by Pawel's patches.
>> >>>>
>> >>>>I'll try to reproduce here and see what's causing it.
>> >>>
>> >>> Problem is in my Patch. I reproduced it, but I don't understand why compiler
>> >>> complains about usb_decode_ctrl. It's compiled into libcomposite.ko and
>> >>> declaration is in drivers/usb/gadget.h.
>> >>
>> >>That's because in multi_v7_defconfig dwc3 is built-in while libcomposite
>> >>is a module:
>> >>
>> >>CONFIG_USB_DWC3=y
>> >>CONFIG_USB_LIBCOMPOSITE=m
>> >>
>> >>
>> >>I remember that when you were doing this work, I asked you to move
>> >>functions to usb/common. Why did you deviate from that suggestion? It's
>> >>clear that decoding a ctrl request can be used by peripheral and host
>> >>and we wouldn't have to deal with this problem if you had just followed
>> >>the suggestion.
>> >
>> > Some time ago Greg wrote: 
>> > " It's nice to have these in a common place, but you just bloated all of
>> > the USB-enabled systems in the world for the use of 2 odd-ball system
>> > controllers that almost no one has :) "
>> >
>> > So I moved these functions to gadget directory. 
>> >
>> > It was mistake that I added debug.c file to libcomposite.ko.
>> 
>> The plan is to use this decoding function for xHCI as well. Other host
>> controllers can use it as well.
>> 
>> The biggest mistake was to put this under gadget. What you should have
>> done was create a file under usb/common that only gets compile in if
>> tracing is enabled.
>> 
>> Then there's no bloating unless you have a kernel purposefuly built for
>> debugging and tracing.
>> 
>> Greg, does that work for you?
>
> I guess, but I'd like to see patches before answering that :)

Sure, understandable. I should've done a better job at filtering that
out. Sorry about htat

-- 
balbi

Re: linux-next: build failure after merge of the usb and usb-gadget trees

[Nishanth Menon]

On 11:25-20190704, Felipe Balbi wrote:
[...]

> This is what I get for trusting people to do their part. I couldn't even
> compile test this since I don't have ARM compilers anymore (actually,
> just installed to test). Your customer, however, uses ARM cores so I
> would expect you to have at least compile tested this on ARM. How come
> this wasn't verified by anybody at TI?

Sorry about that Felipe/Greg, will try and make sure we put in steps so
that this does'nt happen again.

> 
> TI used to have automated testing for many of the important defconfigs,
> is that completely gone? Are you guys relying entirely on linux-next?

We still do. Kind of a unfortunately co-incidence, there has been for a
few weeks a downtime given the test infrastructure has been changing and
the continual automated testing env is down for community kernel. But,
that said, we also did move to focussing on linux-next, which should be
revisited.


-- 
Regards,
Nishanth Menon

RE: linux-next: build failure after merge of the usb and usb-gadget trees

[Pawel Laszczak]

Hi,
>
>Hi,
>
>Greg KH <greg@kroah.com> writes:
>
>> On Thu, Jul 04, 2019 at 12:44:08PM +0300, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> Pawel Laszczak <pawell@cadence.com> writes:
>>>
>>> >>
>>> >>
>>> >>Hi,
>>> >>
>>> >>Pawel Laszczak <pawell@cadence.com> writes:
>>> >>
>>> >>>>
>>> >>>>Hi,
>>> >>>>
>>> >>>>On Thu, Jul 4, 2019 at 9:59 AM Greg KH <greg@kroah.com> wrote:
>>> >>>>>
>>> >>>>> On Thu, Jul 04, 2019 at 04:34:58PM +1000, Stephen Rothwell wrote:
>>> >>>>> > Hi all,
>>> >>>>> >
>>> >>>>> > After merging the usb tree, today's linux-next build (arm
>>> >>>>> > multi_v7_defconfig) failed like this:
>>> >>>>> >
>>> >>>>> > arm-linux-gnueabi-ld: drivers/usb/dwc3/trace.o: in function `trace_raw_output_dwc3_log_ctrl':
>>> >>>>> > trace.c:(.text+0x119c): undefined reference to `usb_decode_ctrl'
>>> >>>>> >
>>> >>>>> > Caused by commit
>>> >>>>> >
>>> >>>>> >   3db1b636c07e ("usb:gadget Separated decoding functions from dwc3 driver.")
>>> >>>>> >
>>> >>>>> > I have used the usb tree from next-20190703 for today.
>>> >>>>> >
>>> >>>>> > This also occurs in the usb-gadget tree so I have used the version of
>>> >>>>> > that from next-20190703 as well.
>>> >>>>>
>>> >>>>> Odd, I thought I pulled the usb-gadget tree into mine.  Felipe, can you
>>> >>>>> take a look at this to see if I messed something up?
>>> >>>>
>>> >>>>This looks like it was caused by Pawel's patches.
>>> >>>>
>>> >>>>I'll try to reproduce here and see what's causing it.
>>> >>>
>>> >>> Problem is in my Patch. I reproduced it, but I don't understand why compiler
>>> >>> complains about usb_decode_ctrl. It's compiled into libcomposite.ko and
>>> >>> declaration is in drivers/usb/gadget.h.
>>> >>
>>> >>That's because in multi_v7_defconfig dwc3 is built-in while libcomposite
>>> >>is a module:
>>> >>
>>> >>CONFIG_USB_DWC3=y
>>> >>CONFIG_USB_LIBCOMPOSITE=m
>>> >>
>>> >>
>>> >>I remember that when you were doing this work, I asked you to move
>>> >>functions to usb/common. Why did you deviate from that suggestion? It's
>>> >>clear that decoding a ctrl request can be used by peripheral and host
>>> >>and we wouldn't have to deal with this problem if you had just followed
>>> >>the suggestion.
>>> >
>>> > Some time ago Greg wrote:
>>> > " It's nice to have these in a common place, but you just bloated all of
>>> > the USB-enabled systems in the world for the use of 2 odd-ball system
>>> > controllers that almost no one has :) "
>>> >
>>> > So I moved these functions to gadget directory.
>>> >
>>> > It was mistake that I added debug.c file to libcomposite.ko.
>>>
>>> The plan is to use this decoding function for xHCI as well. Other host
>>> controllers can use it as well.
>>>
>>> The biggest mistake was to put this under gadget. What you should have
>>> done was create a file under usb/common that only gets compile in if
>>> tracing is enabled.
>>>
>>> Then there's no bloating unless you have a kernel purposefuly built for
>>> debugging and tracing.
>>>
>>> Greg, does that work for you?
>>
>> I guess, but I'd like to see patches before answering that :)
>
>Sure, understandable. I should've done a better job at filtering that
>out. Sorry about htat

I will return debug.c again to usb/common directory. 
I made it as suggested by Felipe.
I will try correct this patches on Monday. 

I apologize for my mistake and for wasting your time.

Regards,
Pawel.
  

Re: linux-next: build failure after merge of the usb and usb-gadget trees

[Sekhar Nori]

On 04/07/19 1:55 PM, Felipe Balbi wrote:
> 
> Hi,
> 
> Pawel Laszczak <pawell@cadence.com> writes:
> 
>>>
>>> Hi,
>>>
>>> On Thu, Jul 4, 2019 at 9:59 AM Greg KH <greg@kroah.com> wrote:
>>>>
>>>> On Thu, Jul 04, 2019 at 04:34:58PM +1000, Stephen Rothwell wrote:
>>>>> Hi all,
>>>>>
>>>>> After merging the usb tree, today's linux-next build (arm
>>>>> multi_v7_defconfig) failed like this:
>>>>>
>>>>> arm-linux-gnueabi-ld: drivers/usb/dwc3/trace.o: in function `trace_raw_output_dwc3_log_ctrl':
>>>>> trace.c:(.text+0x119c): undefined reference to `usb_decode_ctrl'
>>>>>
>>>>> Caused by commit
>>>>>
>>>>>   3db1b636c07e ("usb:gadget Separated decoding functions from dwc3 driver.")
>>>>>
>>>>> I have used the usb tree from next-20190703 for today.
>>>>>
>>>>> This also occurs in the usb-gadget tree so I have used the version of
>>>>> that from next-20190703 as well.
>>>>
>>>> Odd, I thought I pulled the usb-gadget tree into mine.  Felipe, can you
>>>> take a look at this to see if I messed something up?
>>>
>>> This looks like it was caused by Pawel's patches.
>>>
>>> I'll try to reproduce here and see what's causing it.
>>
>> Problem is in my Patch. I reproduced it, but I don't understand why compiler 
>> complains about usb_decode_ctrl. It's compiled into libcomposite.ko and
>> declaration is in drivers/usb/gadget.h. 
> 
> That's because in multi_v7_defconfig dwc3 is built-in while libcomposite
> is a module:
> 
> CONFIG_USB_DWC3=y
> CONFIG_USB_LIBCOMPOSITE=m
> 
> 
> I remember that when you were doing this work, I asked you to move
> functions to usb/common. Why did you deviate from that suggestion? It's
> clear that decoding a ctrl request can be used by peripheral and host
> and we wouldn't have to deal with this problem if you had just followed
> the suggestion.
> 
> Now we have to come up with a way to fix this that doesn't involve
> reverting my part2 tag in its entirety because there are other important
> things there.
> 
> This is what I get for trusting people to do their part. I couldn't even
> compile test this since I don't have ARM compilers anymore (actually,
> just installed to test). Your customer, however, uses ARM cores so I
> would expect you to have at least compile tested this on ARM. How come
> this wasn't verified by anybody at TI?
> 
> TI used to have automated testing for many of the important defconfigs,
> is that completely gone? Are you guys relying entirely on linux-next?

I just checked back to see what happened here. In our product kernel
(which does undergo build test with multiple defconfigs), we are using
v6 of these patches which did not have this issue.

I think 0-day kbuild tester could have caught the issue as well. Within
boundaries of existing infrastructure, that probably was the best bet
here. Not sure why that did not happen (probably just build scheduling).

Thanks,
Sekhar