On Mon, Sep 29, 2014 at 11:48:41PM -0500, Felipe Balbi wrote:
> Hi,
>
> On Tue, Sep 30, 2014 at 11:12:55AM +0800, Huang Rui wrote:
> > > > > > > > > On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> > > > > > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > > > > > > > index b0f4d52..6138c5d 100644
> > > > > > > > > > --- a/drivers/usb/dwc3/core.c
> > > > > > > > > > +++ b/drivers/usb/dwc3/core.c
> > > > > > > > > > @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > /**
> > > > > > > > > > + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface for NL
> > > > > > > > > > + * @dwc: Pointer to our controller context structure
> > > > > > > > > > + */
> > > > > > > > > > +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
> > > > > > > > > > +{
> > > > > > > > > > + u32 reg = 0;
> > > > > > > > > > +
> > > > > > > > > > + reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK | DWC3_GUSB3PIPECTL_UX_EXITINPX
> > > > > > > > > > + | DWC3_GUSB3PIPECTL_UX_EXITINPX | DWC3_GUSB3PIPECTL_U1U2EXITFAIL
> > > > > > > > > > + | DWC3_GUSB3PIPECTL_DEPOCHANGE | DWC3_GUSB3PIPECTL_RX_DETOPOLL;
> > > > > > > > >
> > > > > > > > > UX_EXITINPX is supposed to be used with a faulty PHY, I need to see an
> > > > > > > > > erratum before I can accept it. You have also duplicated the bit in this
> > > > > > > > > initialization.
> > > > > > > > >
> > > > > > > > > U1U2EXITFAIL -> also a workaround bit and I need to see an erratum.
> > > > > > > > >
> > > > > > > > > RX_DETOPOLL -> it seems like it's safe to leave this one out as it can
> > > > > > > > > only be proven to work with this bit after going through full USB
> > > > > > > > > certification.
> > > > > > > > >
> > > > > > > >
> > > > > > > > What do you mean of the faulty PHY?
> > > > > > > > We would use AMD PHY to talk with DWC3 controller.
> > > > > > >
> > > > > > > Look at the description of each of those bits and you'll see it mentions
> > > > > > > they're supposed to be used for workarounds. Here's UX_EXIT_IN_PX, as an
> > > > > > > example:
> > > > > > >
> > > > > > > "
> > > > > > > This bit is added for SS PHY workaround where SS PHY ...
> > > > > > > "
> > > > > > >
> > > > > >
> > > > > > Got it, so faulty PHY you meant that some special PHYs didn't not meet
> > > > > > the standards someplace.
> > > > > >
> > > > > > For simulation board, we used vendor provided PHY. But on SOC, we
> > > > > > will use AMD PHY. I will re-check again to confirm which workarounds
> > > > > > need on simulation board and which workarounds need on SOC.
> > > > >
> > > > > Thanks, that's great. I wonder if there's a way to detect that we're
> > > > > running on FPGA. Can you check with your HW designers ? I'll go dig on
> > > > > Synopsys databook myself too on Monday.
> > > > >
> > > >
> > > > I checked with HW designers, they can update the origin value of GUID
> > > > register of FPGA to add ASCII codes as prefix of "fpga". I can checked
> > > > it before driver re-writes it as kernel version (I see you latest
> > > > testing branch have this behavior).
> > >
> > > while that's a nice idea, it wouldn't work for everybody; only AMD.
> > >
> > > If there's no "generic" way implemented by Synopsys into the RTL, then
> > > we better not add anything.
> >
> > Because the RTL is frozen.
> >
> > I checked the spec again of GUID:
> >
> > 1) To store the version or revision of your system
> > 2) To store hardware configurations that are outside the core
> > 3) As a scratch register.
> >
> > As 2) described, it also makes sence the store the HW configuration (FPGA
> > version) on this register.
> >
> > Can we split the 32bit space to two 16bit area, one of them stores the HW
> > configuration, and the other stores the kernel version? I think other SOC
> > (especially x86-based) would use this method. :)
> >
> > It looks like there isn't another register can program into the version info.
>
> the problem is that we won't have something that will work for
> everybody. Note that the register can be used as scratch register as
> well and there's really no way we will be able to get every RTL designer
> in every company out there who's licensing this core to agree on using
> this register the exact same way.
>
> Unless it ships already built into the RTL Synopsys delivers, there's
> no way we can use it in the core dwc3 driver ;-)
>
> > > > > > > Also, I'd have to ask you to provide full boot logs of your platform
> > > > > > > booting with my testing/next (where all the latest patches are) plus
> > > > > > > your patches.
> > > > > >
> > > > > > I will provide the boot logs to you. Actually, I already did the
> > > > > > gadget mass storage and gadget zero testing with my patches under 3.14
> > > > > > before.
> > > > >
> > > > > v3.14 is rather old, if you're sending patches upstream. I'd rather see
> > > > > patches tested with either next or latest Linus' tag. My testing/next
> > > > > branch is also usually fine too.
> > > > >
> > > >
> > > > Yes, I am doing these testing.
> > > >
> > > > An issue meet your msc.c.
> > > >
> > > > ray@hr-slim:~/felipe/usb-tools$ gcc -Wall -O2 -D_GNU_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -g -o msc msc.c
> > > > /tmp/ccUBcDC4.o: In function `do_write':
> > > > /home/ray/felipe/usb-tools/msc.c:275: undefined reference to `clock_gettime'
> > > > /home/ray/felipe/usb-tools/msc.c:308: undefined reference to `clock_gettime'
> > > > /tmp/ccUBcDC4.o: In function `do_read':
> > > > /home/ray/felipe/usb-tools/msc.c:332: undefined reference to `clock_gettime'
> > > > /home/ray/felipe/usb-tools/msc.c:349: undefined reference to `clock_gettime'
> > > > /tmp/ccUBcDC4.o: In function `do_writev':
> > > > /home/ray/felipe/usb-tools/msc.c:401: undefined reference to `clock_gettime'
> > > > /tmp/ccUBcDC4.o:/home/ray/felipe/usb-tools/msc.c:407: more undefined references to `clock_gettime' follow
> > > > collect2: ld returned 1 exit status
> > > >
> > > > Any idea?
> > >
> > > builds fine here:
> > >
> > > $ make clean
> > > CLEAN clean
> > > $ make
> > > CC companion-desc.o
> > > LINK companion-desc
> > > CC testmode.o
> > > LINK testmode
> > > CC cleware.o
> > > LINK cleware
> > > CC control.o
> > > LINK control
> > > CC device-reset.o
> > > LINK device-reset
> > > CC msc.o
> > > LINK msc
> > > CC testusb.o
> > > LINK testusb
> > > CC serialc.o
> > > LINK serialc
> > > CC acmc.o
> > > LINK acmc
> > > CC switchbox.o
> > > LINK switchbox
> > > CC seriald.o
> > > LINK seriald
> > > CC acmd.o
> > > LINK acmd
> > >
> > > Perhaps something with the GCC version you're using ?
> > >
> > > $ gcc -v
> > > Using built-in specs.
> > > COLLECT_GCC=gcc
> > > COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.9/lto-wrapper
> > > Target: x86_64-linux-gnu
> > > Configured with: ../src/configure -v --with-pkgversion='Debian 4.9.1-15' --with-bugurl=file:///usr/share/doc/gcc-4.9/README.Bugs --enable-languages=c,c++,java,go,d,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-4.9 --enable-shared --enable-linker-build-id --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.9 --libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-gnu-unique-object --disable-vtable-verify --enable-plugin --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64/jre --enable-java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-4.9-amd64 --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-4.9-amd64 --with-arch-directory=amd64 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc --enable-multiarch --with-arch-32=i586 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
> > > Thread model: posix
> > > gcc version 4.9.1 (Debian 4.9.1-15)
> > >
> >
> > My gcc version is 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) before. After I
> > upgrade to gcc version 4.8.2 (Ubuntu 4.8.2-19ubuntu1), then make the repo
> > again.
> >
> > ray@hr-ub:~/felipe/usb-tools$ make
> > LINK companion-desc
> > companion-desc.o: In function `main':
> > /home/ray/felipe/usb-tools/companion-desc.c:219: undefined reference to `libusb_init'
> > /home/ray/felipe/usb-tools/companion-desc.c:221: undefined reference to `libusb_open_device_with_vid_pid'
> > companion-desc.o: In function `do_test':
> > /home/ray/felipe/usb-tools/companion-desc.c:176: undefined reference to `libusb_get_device'
> > /home/ray/felipe/usb-tools/companion-desc.c:178: undefined reference to `libusb_get_device_descriptor'
> > companion-desc.o: In function `check_configurations':
> > /home/ray/felipe/usb-tools/companion-desc.c:153: undefined reference to `libusb_get_config_descriptor'
> > companion-desc.o: In function `main':
> > /home/ray/felipe/usb-tools/companion-desc.c:239: undefined reference to `libusb_close'
> > /home/ray/felipe/usb-tools/companion-desc.c:242: undefined reference to `libusb_exit'
> > companion-desc.o: In function `do_test':
> > /home/ray/felipe/usb-tools/companion-desc.c:163: undefined reference to `libusb_free_config_descriptor'
> > collect2: error: ld returned 1 exit status
> > make: *** [companion-desc] Error 1
alright, libraries should be listed last. But only a few GCC versions
are more anal about it. Anyway, pushed a patch on Makefile, see if it
works for you.
--
balbi
Attachment:
signature.asc
Description: Digital signature
