Re: [PATCH] usb: ulpi: don't register drivers if bus doesn't exist
On Wed, May 20, 2015 at 03:33:26PM -0400, Sasha Levin wrote: > ULPI registers it's bus at module_init so if the bus fails to register, the > module will fail to load and all will be well in the world. > > However, if the ULPI code is built-in rather than a module, the bus > initialization may fail but we'd still try to register drivers later onto > a non-existant bus, which will panic the kernel. > > Fix that by checking that the bus was indeed initialized before trying to > register drivers on top of it. > > Signed-off-by: Sasha Levin > --- > drivers/usb/common/ulpi.c |4 > 1 file changed, 4 insertions(+) > > diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c > index 0e6f968..0b0a5e7 100644 > --- a/drivers/usb/common/ulpi.c > +++ b/drivers/usb/common/ulpi.c > @@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv) > if (!drv->probe) > return -EINVAL; > > + /* Was the bus registered successfully? */ > + if (!ulpi_bus.p) > + return -ENODEV; Ick, no, don't go mucking around in the bus internals like this, that's not ok. You should either "know" the bus is registered, or something is really wrong with the design here. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: ulpi: don't register drivers if bus doesn't exist
On Sun, May 24, 2015 at 12:19:48AM -0700, Greg KH wrote: > On Wed, May 20, 2015 at 03:33:26PM -0400, Sasha Levin wrote: > > ULPI registers it's bus at module_init so if the bus fails to register, the > > module will fail to load and all will be well in the world. > > > > However, if the ULPI code is built-in rather than a module, the bus > > initialization may fail but we'd still try to register drivers later onto > > a non-existant bus, which will panic the kernel. > > > > Fix that by checking that the bus was indeed initialized before trying to > > register drivers on top of it. > > > > Signed-off-by: Sasha Levin > > --- > > drivers/usb/common/ulpi.c |4 > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c > > index 0e6f968..0b0a5e7 100644 > > --- a/drivers/usb/common/ulpi.c > > +++ b/drivers/usb/common/ulpi.c > > @@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv) > > if (!drv->probe) > > return -EINVAL; > > > > + /* Was the bus registered successfully? */ > > + if (!ulpi_bus.p) > > + return -ENODEV; > > Ick, no, don't go mucking around in the bus internals like this, that's > not ok. You should either "know" the bus is registered, or something is > really wrong with the design here. can't we use a variable which can be initialized to 1 in ulpi_init() if the bus registers successfully and later check that? regards sudip > > greg k-h > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: ulpi: don't register drivers if bus doesn't exist
Why do we even need that? If you take patch that makes ulpi_init a subsys_initcall you won't have this problem, and no additional weird hacks and errors will be needed On Sun, May 24, 2015 at 11:09 AM, Sudip Mukherjee wrote: > On Sun, May 24, 2015 at 12:19:48AM -0700, Greg KH wrote: >> On Wed, May 20, 2015 at 03:33:26PM -0400, Sasha Levin wrote: >> > ULPI registers it's bus at module_init so if the bus fails to register, the >> > module will fail to load and all will be well in the world. >> > >> > However, if the ULPI code is built-in rather than a module, the bus >> > initialization may fail but we'd still try to register drivers later onto >> > a non-existant bus, which will panic the kernel. >> > >> > Fix that by checking that the bus was indeed initialized before trying to >> > register drivers on top of it. >> > >> > Signed-off-by: Sasha Levin >> > --- >> > drivers/usb/common/ulpi.c |4 >> > 1 file changed, 4 insertions(+) >> > >> > diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c >> > index 0e6f968..0b0a5e7 100644 >> > --- a/drivers/usb/common/ulpi.c >> > +++ b/drivers/usb/common/ulpi.c >> > @@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv) >> > if (!drv->probe) >> > return -EINVAL; >> > >> > + /* Was the bus registered successfully? */ >> > + if (!ulpi_bus.p) >> > + return -ENODEV; >> >> Ick, no, don't go mucking around in the bus internals like this, that's >> not ok. You should either "know" the bus is registered, or something is >> really wrong with the design here. > can't we use a variable which can be initialized to 1 in ulpi_init() if > the bus registers successfully and later check that? > > regards > sudip >> >> greg k-h >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
On Thu, May 21, 2015 at 08:11:27PM -0700, David Cohen wrote: > On Thu, May 21, 2015 at 08:09:54PM -0700, David Cohen wrote: > > Hi, > > > > On Fri, May 22, 2015 at 10:07:05AM +0800, Lu Baolu wrote: > > > Many drivers and modules depend on ULPI bus registeration to > > > register ULPI interfaces and drivers. It's more appropriate > > > to register ULPI bus in subsys_initcall instead of module_init. > > > > > > Kernel panic has been reported with some kind of kernel config. > > > > Even though I agree subsys_initcall is better to register ulpi bus, it's > > still no excuse to have kernel panic. What about ULPI bus being compiled > > as module? > > IMHO this is avoiding the proper kernel panic fix which should be > > failing gracefully (or defer probe) from tusb1210 driver. > > Maybe I need to express myself better :) > IMHO we should not consider work done with this patch only. It's still > incomplete. Then please fix it properly, this is not the correct solution. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] usb: ulpi: don't register drivers if bus doesn't exist
On Thu, May 21, 2015 at 04:57:52PM +0800, Lu Baolu wrote: > ULPI registers its bus at module_init, so if the bus fails to register, the > module will fail to load and all will be well in the world. > > However, if the ULPI code is built-in rather than a module, the bus > initialization may fail, but we'd still try to register drivers later onto > a non-existent bus, which will panic the kernel. > > Fix that by checking that the bus was indeed initialized before trying to > register drivers on top of it. > > Signed-off-by: Sasha Levin > Signed-off-by: Heikki Krogerus > Signed-off-by: Lu Baolu > Reported-by: Zhuo Qiuxu > Reviewed-by: David Cohen > --- > drivers/usb/common/ulpi.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c > index 0e6f968..af52b46 100644 > --- a/drivers/usb/common/ulpi.c > +++ b/drivers/usb/common/ulpi.c > @@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv) > if (!drv->probe) > return -EINVAL; > > + /* Was the bus registered successfully? */ > + if (unlikely(WARN_ON(!ulpi_bus.p))) never use unlikely/likely unless you can actually measure the difference if the marking is not used. Hint, on driver probe time, you can't. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: XHCI, "brain-dead scanner", and microframe rounding
Aww, that's too bad. Let me know if you'd like me to test a modified version when you get the time. --Mike Mammarella On May 21, 2015, at 4:18 AM, Mathias Nyman wrote: > Hi > > The fix went upstream, but caused regression for other users, and had to be > reverted. > The cause of the regression was found but the new version was never properly > tested and > got left behind as more urgent issues arrived. > > I still need to attend a few other issues before taking up this again > > -Mathias > > On 21.05.2015 13:38, Hans-Peter Jansen wrote: >> Dear Mathias, >> >> just a heads up: retesting with 4.0.4 revealed, that this issue isn't fixed >> for my scanner still. To recap: driving the scanner through a ehci port is >> fine, and fails miserably with xhci. >> >> OK: >> >> T: Bus=03 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#= 1 Spd=480 MxCh= 4 >> D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 >> P: Vendor=1d6b ProdID=0002 Rev=04.00 >> S: Manufacturer=Linux 4.0.4-2.g4f5e0d5-desktop ehci_hcd >> S: Product=EHCI Host Controller >> S: SerialNumber=:06:04.2 >> C: #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=0mA >> I: If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub >> >> T: Bus=03 Lev=01 Prnt=01 Port=01 Cnt=02 Dev#= 3 Spd=480 MxCh= 0 >> D: Ver= 2.00 Cls=ff(vend.) Sub=ff Prot=ff MxPS=64 #Cfgs= 1 >> P: Vendor=04b8 ProdID=0119 Rev=01.00 >> S: Manufacturer=EPSON >> S: Product=EPSON Scanner >> C: #Ifs= 1 Cfg#= 1 Atr=c0 MxPwr=2mA >> I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none) >> >> NOT OK: >> >> T: Bus=06 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#= 1 Spd=480 MxCh=14 >> D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=01 MxPS=64 #Cfgs= 1 >> P: Vendor=1d6b ProdID=0002 Rev=04.00 >> S: Manufacturer=Linux 4.0.4-2.g4f5e0d5-desktop xhci-hcd >> S: Product=xHCI Host Controller >> S: SerialNumber=:00:14.0 >> C: #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=0mA >> I: If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub >> >> T: Bus=06 Lev=01 Prnt=01 Port=10 Cnt=02 Dev#= 10 Spd=480 MxCh= 0 >> D: Ver= 2.00 Cls=ff(vend.) Sub=ff Prot=ff MxPS=64 #Cfgs= 1 >> P: Vendor=04b8 ProdID=0119 Rev=01.00 >> S: Manufacturer=EPSON >> S: Product=EPSON Scanner >> C: #Ifs= 1 Cfg#= 1 Atr=c0 MxPwr=2mA >> I: If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=usbfs >> >> Additional notes: >> >> xsane scanner discovery takes ages (20-30 secs) to find the scanner in the >> failing case. After selecting the correct device, it takes another delay of >> 20-30 secs. for presenting the error dialog: error during device I/O. The >> same >> procedure with ehci takes about a second until the device selection is >> shown, >> and another 0.5 secs later it presents the fully functional scanning UI. >> >> This behavior persists since Linux 3.16.x (where I setup this box). >> >> Please let me know, if I can be of any help for you for resolving this >> issue. >> I find it a little sad, that at the dawn of USB 3.1, we still fight with >> such >> issues on the linux USB 3.0 front. Don't forget the many frustrated users >> observing this, that will not speak up. >> >> Cheers, >> Pete >> >> On Donnerstag, 29. Januar 2015 18:42:05 Mathias Nyman wrote: >>> On 27.01.2015 14:12, Gunter Königsmann wrote: That's very good news indeed. Will re-run the tests on my scanner and looking forward to the fix entering mainline. In the meantime I can acknowledge that with the fix my computer accepts USB memory sticks that normally didn't work. Kind regards, Gunter. >>> >>> Did some cleaning of the patch, and noticed it still had a few bits wrong, >>> but apparently it worked anyway. >>> >>> I added the fixes on top of the ep_reset_halt_test branch. >>> >>> Can any of you with a failing scanner test that it still works? >>> >>> git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git >>> and the ep_reset_halt_test branch, >>> >>> Thanks >>> >>> -Mathias >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-usb" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: renesas_usbhs: avoid uninitialized variable use
On Fri, May 22, 2015 at 11:33:57AM +, Yoshihiro Shimoda wrote:
> Hi Arnd,
>
> > Sent: Friday, May 22, 2015 8:07 PM
> >
> > After the renesas_usbhs driver is enabled in ARM multi_v7_defconfig,
> > we now get a new warning:
> >
> > renesas_usbhs/mod.c: In function 'usbhs_interrupt':
> > renesas_usbhs/mod.c:246:7: warning: 'intenb1' may be used uninitialized in
> > this function [-Wmaybe-uninitialized]
> >
> > gcc correctly points to a problem here, for the case that the
> > device is in host mode, we use the intenb1 variable without
> > having assigned it first. The state->intsts1 has a similar
> > problem, but gcc cannot know that.
> >
> > This avoids the problem by initializing both sides of the
> > comparison to zero when we don't read them from the respective
> > registers.
> >
> > Signed-off-by: Arnd Bergmann
> > Fixes: 88a25e02f3 ("usb: renesas_usbhs: Add access control for INTSTS1 and
> > INTENB1 register")
>
> Thank you very much for the patch!
>
> Acked-by: Yoshihiro Shimoda
>
> (I'm not sure why a toolchain I used (Linaro GCC 2014.11) doesn't show this
> warning...)
>
> Best regards,
> Yoshihiro Shimoda
Reviewed-by: Simon Horman
>
> > diff --git a/drivers/usb/renesas_usbhs/mod.c
> > b/drivers/usb/renesas_usbhs/mod.c
> > index e5ce6e6d4f51..d4be5d594896 100644
> > --- a/drivers/usb/renesas_usbhs/mod.c
> > +++ b/drivers/usb/renesas_usbhs/mod.c
> > @@ -223,6 +223,8 @@ static int usbhs_status_get_each_irq(struct usbhs_priv
> > *priv,
> > if (usbhs_mod_is_host(priv)) {
> > state->intsts1 = usbhs_read(priv, INTSTS1);
> > intenb1 = usbhs_read(priv, INTENB1);
> > + } else {
> > + state->intsts1 = intenb1 = 0;
> > }
> >
> > /* mask */
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 00/11] usbip: features to USB over WebSocket
Hello, > I see your point and what you have done in patches. > I'm only showing you that you may achieve almost the same effect > without any changes in kernel. I tested wstunnel. The performance comparison in my environment is as following. Round trip time of keyboard key down and up URBs at host. wstunnel: 4.4msec usbws(my patch): 2.7msec wstunnel passes TCP/IP stack twice so it's slower. I'd like to keep the usbws implementation because it's worth for performance. memory usage (VSZ/RSS) wstunnel usbws Server wstunnel 734K/20K usbipa25K/2K usbwsa 25K/2K Client wstunnel 75K/17K usbipa 113K/3K Memo about wstunnel 1) Implementation node.js application 2) installation # yum install npm # npm install -g wstunnel 3) Usage for USB/IP app# insmod usbip-core.ko app# isnmod vhci-hcd.ko app# usbipa -t 3240 app$ wstunnel -s 8080 -t localhost:3240 dev# insmod usbip-core.ko dev# isnmod usbip-host.ko dev$ wstunnel -t 3240 ws://:8080 dev# usbip -t 3240 connect -r localhost -b usbip -- wstunnel -- wstunnel -- usbipa 3240 8080 3240 Some issues are found in interoperability (to use usbws with wstunnel). a) wstunnel needs Sec-WebSocket-Protocol header with value 'tunnel-protocol'. It means that the other end must be wstunnel too. b) wstunnel sends multiple packets into one frame. usbws must implement buffering. c) wstunnel sends WebSocket with FIN bit=1. Poco library translates it to OP_CLOSE and succeeding readFrame() doesn't work. I may fix later regarding b) if it's needed by other implementations. Sorry for my late response. I concentrated to other project. Thank you for your help, n.iwata //
RE: usb: gadget: f_fs: O_NONBLOCK waits MAX_SCHEDULE_TIMEOUT
> >>> > >>> FunctionFS is very specific, because read/write operations are > >>> directly translated into USB requests, which are asynchronous, so > >>> you cannot use O_NONBLOCK. > >>> > >>> If you need non-blocking API you can use Asynchronous I/O (AIO). You > >>> can find some examples in kernel sources (tools/usb/ffs-aio-example/). > >>> > >>> Br, > >>> Robert Baldyga > >>> > >> > >> Thank you, that sounds like the best approach. > >> In this case I think perhaps the long wait without any data is an > >> problem with the imx6 Chipidea USB controller. > > > > What's the possible problem? > > Sorry for the delay in replying, I have been getting some more details with a > USB Analyser. > > The scenario is that the NCM device is enumerating so we see the messages to: > > SetAddress (1) > GetDescriptor (Device) > GetDescriptor (StringN) > GetDescriptor (Configuration) > SetConfiguration (1) > GetDescriptor (String iInterface) > GetDescriptor (String iInterface) > > At this point the NCM host sends Writes to the F_FS EP0 but for some reason > the host device does not respond and only issues SOF packets for hours. This > happens occasionally and is fixed by turning the device off and on again. > > We may find this 'some reason', is it device error or host error? Do you have below patch in your code: commit 953c66469735aed8d2ada639a72b150f01dae605 Author: Abbas Raza Date: Thu Jul 17 19:34:31 2014 +0800 usb: chipidea: udc: Disable auto ZLP generation on ep0 There are 2 methods for ZLP (zero-length packet) generation: 1) In software 2) Automatic generation by device controller 1) is implemented in UDC driver and it attaches ZLP to IN packet if descriptor->size < wLength 2) can be enabled/disabled by setting ZLT bit in the QH Peter > Unless I am mistaken from a NCM gadget point of view the attached device is > working correctly and there is no way to know it has failed, is that correct? > > > > >> > >> I guess it should suspend and drop the connections if there is no > >> traffic for more than 10ms? > >> > > > > If the Device side NAK host's IN/OUT token continually, the pipe will > > not be stopped, the host will send token continually until the > > application cancel this request. > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] drivers: usb :fsl: Add support for USB controller version-2.5
> -Original Message-
> From: Greg KH [mailto:g...@kroah.com]
> Sent: Monday, September 29, 2014 11:56 AM
> To: Badola Nikhil-B46172
> Cc: linux-usb@vger.kernel.org
> Subject: Re: [PATCH] drivers: usb :fsl: Add support for USB controller
> version-
> 2.5
>
> On Mon, Sep 29, 2014 at 03:46:02AM +, nikhil.bad...@freescale.com
> wrote:
> > >-Original Message-
> > >From: Greg KH [mailto:g...@kroah.com]
> > >Sent: Wednesday, September 24, 2014 10:19 AM
> > >To: Badola Nikhil-B46172
> > >Cc: linux-usb@vger.kernel.org
> > >Subject: Re: [PATCH] drivers: usb :fsl: Add support for USB
> > >controller version-2.5
> > >
> > >On Thu, Aug 21, 2014 at 12:56:22PM +0530, Nikhil Badola wrote:
> > >> Add support for USB controller version-2.5 used in
> > >> T4240 rev2.0, T1024, B3421, T1040, T2080, LS1021A.
> > >>
> > >> Signed-off-by: Nikhil Badola
> > >> ---
> > >> - Depends on commit 990c2c7829d98517228f2b2ff14919c83b75e124
> > >>drivers: usb: fsl: Check IP version 2.4 for mph USB controller
> > >>
> > >> drivers/usb/host/fsl-mph-dr-of.c | 5 +
> > >> include/linux/fsl_devices.h | 1 +
> > >> 2 files changed, 6 insertions(+)
> > >>
> > >> diff --git a/drivers/usb/host/fsl-mph-dr-of.c
> > >> b/drivers/usb/host/fsl-mph-dr-of.c
> > >> index e0315de..45b9e36 100644
> > >> --- a/drivers/usb/host/fsl-mph-dr-of.c
> > >> +++ b/drivers/usb/host/fsl-mph-dr-of.c
> > >> @@ -127,6 +127,7 @@ static int usb_get_ver_info(struct device_node
> *np)
> > >> * returns 1 for usb controller version 1.6
> > >> * returns 2 for usb controller version 2.2
> > >> * returns 3 for usb controller version 2.4
> > >> + * returns 4 for usb controller version 2.5
> > >> * returns 0 otherwise
> > >> */
> > >> if (of_device_is_compatible(np, "fsl-usb2-dr")) { @@ -136,6
> > >> +137,8 @@ static int usb_get_ver_info(struct device_node *np)
> > >> ver = FSL_USB_VER_2_2;
> > >> else if (of_device_is_compatible(np,
> > >> "fsl-usb2-dr-v2.4"))
> > >> ver = FSL_USB_VER_2_4;
> > >> +else if (of_device_is_compatible(np,
> > >> "fsl-usb2-dr-v2.5"))
> > >> +ver = FSL_USB_VER_2_5;
> > >> else /* for previous controller versions */
> > >> ver = FSL_USB_VER_OLD;
> > >>
> > >> @@ -153,6 +156,8 @@ static int usb_get_ver_info(struct device_node
> *np)
> > >> ver = FSL_USB_VER_2_2;
> > >> else if (of_device_is_compatible(np,
> > >> "fsl-usb2-mph-v2.4"))
> > >> ver = FSL_USB_VER_2_4;
> > >> +else if (of_device_is_compatible(np,
> > >> "fsl-usb2-mph-v2.5"))
> > >> +ver = FSL_USB_VER_2_5;
> > >> else /* for previous controller versions */
> > >> ver = FSL_USB_VER_OLD;
> > >> }
> > >> diff --git a/include/linux/fsl_devices.h
> > >> b/include/linux/fsl_devices.h index a82296a..2a2f56b 100644
> > >> --- a/include/linux/fsl_devices.h
> > >> +++ b/include/linux/fsl_devices.h
> > >> @@ -24,6 +24,7 @@
> > >> #define FSL_USB_VER_1_6 1
> > >> #define FSL_USB_VER_2_2 2
> > >> #define FSL_USB_VER_2_4 3
> > >> +#define FSL_USB_VER_2_5 4
> > >
> > >Why not just keep going and add the rest of the numbers while you are at
> it?
> >
> > This is the last controller version of this family hence there would
> > not be any requirement to add further numbers.
>
> People always bring products back, you never know this :)
>
> > >Seriously, what are these two patches doing? You just set the value,
> > >never do anything with it, what good is it?
> >
> > We indeed use these macros for controller version specific code, for
> > example in following snippet from ehci-fsl.c if (pdata->have_sysif_regs &&
> > pdata->controller_ver > FSL_USB_VER_1_6 &&
> > (phy_mode == FSL_USB2_PHY_ULPI)) {
> > /* check PHY_CLK_VALID to get phy clk valid */ .
> > .
> > If we don't set the macros then by default FSL_USB_VER_OLD, which is
> > 0, is assigned as controller version and in that case phy_clk_valid bit
> > would
> not be checked for controller version 2.4 and 2.5.
>
> You are relying on a define to be > a specific value, which seems wrong as
> it's
> impossible to figure this type of thing out. Please use an enumerated type at
> the very least if you want to test this type of thing.
>
Patch for replacing macros with enumerated type sent
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall
On 05/23/2015 12:08 AM, David Cohen wrote:
Hi,
On Fri, May 22, 2015 at 07:29:15PM +0800, Lu Baolu wrote:
Phy drivers and the ulpi interface providers depend on the
registeration of the ulpi bus. Ulpi registers the bus in
module_init(). This could result in a load order issue, i.e.
It's still not an issue :(
I'd say "unnecessary probe delays".
I managed to boot a kernel built from the top of Felipe's
remotes/origin/next branch under an Ubuntu environment
on Intel's Bay Trail tablet.
The same panic (as I found in the Android environment previously)
shows up as well. And if I replace module_init() with sys_initcall(),
the panic disappears.
Thanks,
-Baolu
But of cource it's Felipe's call :) Description looks better now.
BR, David
ulpi phy drivers or the ulpi interface providers loading
before the bus registeration.
This patch fixes this load order issue by putting ulpi_init
in subsys_initcall().
Reported-by: Zhuo Qiuxu
Signed-off-by: Lu Baolu
---
drivers/usb/common/ulpi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0e6f968..01c0c04 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -242,7 +242,7 @@ static int __init ulpi_init(void)
{
return bus_register(&ulpi_bus);
}
-module_init(ulpi_init);
+subsys_initcall(ulpi_init);
static void __exit ulpi_exit(void)
{
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB Host Controller no PCD interrupt and CCS and CSC update on the device disconnect
On Fri, May 22, 2015 at 10:06 PM, Alan Stern wrote: > On Fri, 22 May 2015, Rong Wang wrote: > >> Hi, >> >> We have one USB 2.0 Controller on an ARM SoC, with internal PHY >> confirming to UTMI. >> The PHY would detect unexpected disconnect (amplitude of the >> differential envelop still < 625 mV) >> and assert the HostDisconnect signal to the Controller to indicate a >> disconnection event. >> When the HostDisconnect signal is asserted, the Controller will first >> do some internal clean work >> before it update CCS and CSC in PORTSCx and reports a Port Change >> Detect interrupt. >> We want to improve the situation > > Why do you want to improve the situation? What's wrong with the way it > is now? Detecting a disconnect while the differential amplitude is < > 625 mv is perfectly legal. The USB-2.0 spec requires: "Signals with > differential amplitudes <= 525 mV must never activate the Disconnection > Envelope Detector" (section 7.1.7.3). > Our customer oberserved some unexpected disconnections during the "shaking" test (user scenario simulation). They want the disconnection detection to be less sensitive (>= 625 mV). Because once the discoonection is reported, the APP SW will be re-launched and the USB device will also notify user the disconnection event, which is not accepted by our customer. >> by masking the HostDisconnect signal >> from PHY (take the >> cost into consideration), but it will cause no CCS and CSC update in >> PORTSCx, and no PCD interrupt. Thus the true disconnection only can be >> determined by an XactErr. Once the driver >> determine that it's a true disconnect, the HostDisconnect signal can >> be unmasked and the >> Controller will detect this disconnect. >> We are worried about the workaroud, since there may be many corner >> cases we have not taken into consideration, such as the race condition >> on connect and disconnect. > > It sounds like you are going to make the situation worse, not improve > it. We are going to filter the intermittent disconnection signal from PHY, but we must determine a true disconnection by driver with a retry scheme. I agree that it will make the situation more complicated because there may be many corner cases that we don't forsee and one of them may even make this workaround not workable. > >> So have you encountered this situation before? Or is there any >> suggestion on this workaround? > > I don't see any reason to make any changes. > > Alan Stern > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
