Re: gt-s5570 and pl2303.ko
On 02/20/2015 11:48 PM, Greg KH wrote: On Fri, Feb 20, 2015 at 11:37:17PM +0100, Milutin Aksic wrote: On 02/20/2015 09:22 PM, Greg KH wrote: On Fri, Feb 20, 2015 at 05:37:37PM +0100, Milutin Aksic wrote: On 02/20/2015 04:31 PM, Greg KH wrote: On Fri, Feb 20, 2015 at 11:22:54AM +0100, Milutin Aksic wrote: Hello Greg, I supose that serial communication should work with this older model gt-s5570. I found custom rom with that old kernel version but I don't know how to find newer version for this phone. When I plug the converter to phone I can't see anything in the log that is related to converter but I send you the log as attachment. It is log when I insmod pl2303.ko via adb and after that pluged the converter. Looks like you haven't loaded a usb host controller driver for your hardware. Are you sure you have such a thing loaded and working? Does other types of USB devices work when plugged into this machine? You really are on your own here, sorry. 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 I found host controller drivers in menuconfig and compiled them and inserted with insmod and again nothing happened.I tried with usb flash but nothing happened. I send you log for this. You will see in the log those drivers that I loaded (there is compliant about kernel version but insmod command passed and the modules are in /sys/module directory) and I didn't see anything related to usb converter and mass storage. The host controller drivers I loaded are : isp116x-hcd.ko, isp1760.ko, oxu210hp-hcd.ko, sl811-hcd.ko. That's it. Milutin. isp1362_hcd: version magic '2.6.35.7+-androidarmv6 preempt mod_unload ARMv6 ' should be '2.6.35.7-androidarmv6+ preempt mod_unload ARMv6 ' <4>[66113.287000] isp1362_hcd: Unknown symbol bitmap_find_next_zero_area (err 0) This means that the driver really didn't get loaded, so there's no way any USB devices could work on your system. Try fixing that up first... I fixed it and pl2303.ko and isp1362-hcd.ko loaded and registered without any errors and then pluged the converter and I think nothing happened sending you a log. Best regards, Milutin Aksic. If the host driver doesn't bind to the device and start up (you should see some log messages about finding a root hub) then no device will ever be found. sorry, 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 Ok, then what I should try else what can be the problem why host driver doesn't bind to the device? Maybe the phone gt-s5570 doesn't have any hardware (but I doubt it is the truth) or something else. Thanks Greg anyway, Milutin. -- 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] xhci: fix reporting of 0-sized URBs in control endpoint
Hey Alan, On Sat, Feb 21, 2015 at 3:47 AM, Alan Stern wrote: > On Sat, 21 Feb 2015, Aleksander Morgado wrote: > >> When the control TD doesn't have TRBs in the data stage, the URB actual >> length >> is set equal to the transfer buffer length. E.g. with a 64 byte transfer >> buffer >> length: >> >> Transfer event len = 0, COMP_SHORT_SUCCESS (status) >>|-> URB actual length set to transfer buffer length = 64 > > How can a control TD have a transfer buffer length that is > 0 and also > have no TRBs in the data stage? That doesn't make sense. > Probably didn't explain well, sorry, likely mixing terms. What I mean is that when the data length received is equal to the transfer buffer length, we get a single IRQ event saying COMP_SUCCESS, with an event len = 0; the URB length in that case is set to be equal to the transfer buffer length. >> When the control TD has TRBs in the data stage, the URB actual length is >> computed based on the transfer event length reported in the data stage TRB. >> In >> this case, the URB actual length won't be modified in the status stage. E.g. >> again with a 64 byte transfer buffer length: >> >> Transfer event len = 63, COMP_SHORT_TX (data) >>|-> URB actual length = 64 - 63 = 1 >> Transfer event len = 0, COMP_SHORT_SUCCESS (status) >>|-> URB actual length not modified = 1 >> When the received data length is less that what it was requested in the transfer buffer length, we get 2 IRQ events: one with COMP_SHORT_TX status and the event length giving the 'unfilled' size of the buffer; and one with COMP_SUCCESS status and an event length set to 0. In this case, the URB length is set when the first event arrived, not when the second one arrived. >> The current logic, though, doesn't seem to contemplate the case where a TD >> has a >> TDR in the data stage which actually reports 0 bytes (i.e. transfer event len >> equal to transfer buffer len). The logic is currently handling this case in >> the >> following way: >> >> Transfer event len = 64, COMP_SHORT_TX (data) >>|-> URB actual length = 64 - 64 = 0 >> Transfer event len = 0, COMP_SHORT_SUCCESS (status) >>|-> URB actual length set to transfer buffer length = 64 <-- >> >> In this case, the logic shouldn't update the URB actual length during the >> status >> stage; instead it should leave the URB actual length that was computed during >> the data stage, even if it's 0: >> >> Transfer event len = 63, COMP_SHORT_TX (data) >>|-> URB actual length = 64 - 64 = 0 >> Transfer event len = 0, COMP_SHORT_SUCCESS (status) >>|-> URB actual length not modified = 0 <-- > > In fact, is there _ever_ any reason for changing the actual length > during the status stage? It seems to me that by the end of the data > stage, we already know the actual length. > The missing use case I'm trying to cover is when we do get 2 IRQ events for the same TD: one with COMP_SHORT_TX but where the "unfilled" length is equal to the transfer buffer length (hence, 0 bytes transferred), and the second event with COMP_SUCCESS. >> The proposed fix re-uses the 'last_td_was_short' flag in the endpoint ring. >> The >> setting of this flag is moved to after the actual TRB processing, so that the >> flag value indicating whether the last TRB was short can be read during the >> processing. The control TDR processing will use this flag to determine >> whether >> it has to reset the URB actual length or instead leave it as it was already >> precomputed by the previous short TRB. > > Why not always leave the actual length as is? > The current logic sets the final URB length in 2 different cases: * If the transferred length is equal to the transfer buffer length, the URB length is set when the event for the last TRB is received, notifying COMP_SUCESSS. * If the transferred length is less than the transfer buffer length but > 0, the URB length is set when the COMP_SHORT_TX event arrives; when the COMP_SUCCESS one arrives the URB length that was previously set is not modified. Now, there is this 3rd case, where the transferred length is 0; in this case we get both COMP_SHORT_TX and COMP_SUCCESS events; and the first one states that the "untransferred" length is equal to the transfer buffer length. With this patch in, which truth be told seems a bit like a hack (is there any cleaner way?), the case is covered, and we do get the 0-length URB notified. Currently the xhci driver doesn't report 0-sized URBs in the control endpoint, and that totally breaks the HSO driver usecase (which relies on the 0-sized URBs to stop re-submitting the RX URB), so Option HSO modems don't work properly on USB3 ports :/ Can also reword the commit message to try to make it clearer. Actually, I talk about COMP_SHORT_SUCCESS in the commit message when it really is COMP_SUCCESS... Just let me know how to move it forward :) -- Aleksander https://aleksander.es -- To
Re: [PATCH v4 4/4] phy: add phy-hi6220-usb
Hi, Balbi
On 02/21/2015 12:06 AM, Felipe Balbi wrote:
Hi,
On Fri, Feb 20, 2015 at 11:44:37PM +0800, zhangfei wrote:
Hi, Balbi
On 02/20/2015 10:41 PM, Felipe Balbi wrote:
+static void hi6220_start_peripheral(struct hi6220_priv *priv, bool on)
+{
+ struct usb_otg *otg = priv->phy.otg;
+
+ if (!otg->gadget)
+ return;
+
+ if (on)
+ usb_gadget_connect(otg->gadget);
+ else
+ usb_gadget_disconnect(otg->gadget);
why is the PHY fiddling with pullups ?
We use this to enable/disable otg gadget mode.
I got that, but the pullups don't belong to the PHY, they belong to the
gadget.
The gpio_id & gpio_vbus are used to distinguish otg gadget mode or
host mode.
When micro usb or otg device attached to otg, gpio_vbus falling down.
And gpio_id = 1 is micro usb, gpio_id = 0 is otg device.
all of that I understood clearly :-)
So when micro usb attached, we enable gadget mode; while micro usb
detached, we disable gadget mode, and dwc2 will automatically set to
host mode.
that's all fine, I'm concerned about letting the PHY fiddle with
something it doesn't own. If I am to change pullups rules in udc-core,
this is likely to break down miserably and I don't want to have to go
through that.
Thanks for the clarifying.
How about using usb_gadget_vbus_connect/disconnect, which are used in
many files under drivers/usb/phy.
There is no vbus_session in dwc2/gadget.c, I thought it would be same as
pullup.
However, usb_gadget_vbus_connect still need para gadget, where should we
put this file, drivers/usb/phy or drivers/phy
+static void hi6220_detect_work(struct work_struct *work)
+{
+ struct hi6220_priv *priv =
+ container_of(work, struct hi6220_priv, work.work);
+ int gpio_id, gpio_vbus;
+ enum usb_otg_state state;
+
+ if (!gpio_is_valid(priv->gpio_id) || !gpio_is_valid(priv->gpio_vbus))
+ return;
+
+ gpio_id = gpio_get_value_cansleep(priv->gpio_id);
+ gpio_vbus = gpio_get_value_cansleep(priv->gpio_vbus);
looks like this should be using extcon
Not used extcon before.
However, we need gpio_vbus interrupt.
Checked phy-tahvo.c and phy-omap-otg.c, not find extcon related with
interrupt.
Will investigate tomorrow.
drivers/extcon/extcon-gpio.c
I think there is no need to use extcon, gpio is clear enough.
extcon-gpio.c even do not support dt.
+ if (gpio_vbus == 0) {
+ if (gpio_id == 1)
+ state = OTG_STATE_B_PERIPHERAL;
+ else
+ state = OTG_STATE_A_HOST;
+ } else {
+ state = OTG_STATE_A_HOST;
+ }
+
+ if (priv->state != state) {
+ hi6220_start_peripheral(priv, state == OTG_STATE_B_PERIPHERAL);
+ priv->state = state;
+ }
+}
+
+static irqreturn_t hiusb_gpio_intr(int irq, void *data)
+{
+ struct hi6220_priv *priv = (struct hi6220_priv *)data;
+
+ /* add debounce time */
+ schedule_delayed_work(&priv->work, msecs_to_jiffies(100));
this is really bad. We have threaded interrupt support, right ?
Since we use two gpio to distinguish gadget mode or host mode.
Debounce time can introduce more accuracy.
gpio_set_debounce() ?
Not all gpio.c support set_debounce, including gpio-pl061.c.
I think threaded interrupt can not be used for adding debounce time.
Here add debounce is just for safety.
add the debounce to the gpio itself.
Here the debounce added only for safety.
gpio_id may mis-report when unplug usb, but it is correct for plug usb &
otg device.
So debounce can be omitted.
If you think using delayed work for debounce is ugly, it is fine switch
to threaded_irq.
Thanks
--
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] xhci: fix reporting of 0-sized URBs in control endpoint
On Sat, 21 Feb 2015, Aleksander Morgado wrote: > Probably didn't explain well, sorry, likely mixing terms. What I mean > is that when the data length received is equal to the transfer buffer > length, we get a single IRQ event saying COMP_SUCCESS, with an event > len = 0; the URB length in that case is set to be equal to the > transfer buffer length. > When the received data length is less that what it was requested in > the transfer buffer length, we get 2 IRQ events: one with > COMP_SHORT_TX status and the event length giving the 'unfilled' size > of the buffer; and one with COMP_SUCCESS status and an event length > set to 0. In this case, the URB length is set when the first event > arrived, not when the second one arrived. > The missing use case I'm trying to cover is when we do get 2 IRQ > events for the same TD: one with COMP_SHORT_TX but where the > "unfilled" length is equal to the transfer buffer length (hence, 0 > bytes transferred), and the second event with COMP_SUCCESS. It sounds like this should be handled in the same way as the previous case, but it isn't because of the peculiar way the driver is written. > The current logic sets the final URB length in 2 different cases: > * If the transferred length is equal to the transfer buffer length, > the URB length is set when the event for the last TRB is received, > notifying COMP_SUCESSS. > * If the transferred length is less than the transfer buffer length > but > 0, the URB length is set when the COMP_SHORT_TX event arrives; > when the COMP_SUCCESS one arrives the URB length that was previously > set is not modified. > > Now, there is this 3rd case, where the transferred length is 0; in > this case we get both COMP_SHORT_TX and COMP_SUCCESS events; and the > first one states that the "untransferred" length is equal to the > transfer buffer length. With this patch in, which truth be told seems > a bit like a hack (is there any cleaner way?), the case is covered, > and we do get the 0-length URB notified. I see the problem. The driver needs to distinguish between two cases: COMP_SHORT_TX previously received and COMP_SHORT_TX not previously received. The driver uses actual_length == 0 to make this distinction, but that is not correct because it is possible to have actual_length == 0 even after a COMP_SHORT_TX event. Using the last_td_was_short flag instead seems like a reasonable solution. > Currently the xhci driver doesn't report 0-sized URBs in the control > endpoint, and that totally breaks the HSO driver usecase (which relies > on the 0-sized URBs to stop re-submitting the RX URB), so Option HSO > modems don't work properly on USB3 ports :/ Clearly this is a bug. > Can also reword the commit message to try to make it clearer. > Actually, I talk about COMP_SHORT_SUCCESS in the commit message when > it really is COMP_SUCCESS... Just let me know how to move it forward > :) In general, shorter commit messages are easier to understand. Try to avoid the tendency to report too much information. :-) It wouldn't hurt simply to explain the situation as you did to me just now (but perhaps in a more consise form). Something like this: When a control transfer has a short data stage, the xHCI controller generates two transfer events: a COMP_SHORT_TX event that includes the untransferred amount, and a COMP_SUCCESS event. But when the data stage is not short, only the COMP_SUCCESS event occurs. Therefore, xhci-hcd must set urb->actual_length to urb->transfer_buffer_length while processing the COMP_SUCCESS event, unless actual_length was set previously. The driver checks this by seeing whether actual_length == 0. But this is the wrong test, because it is entirely possible for a short transfer to have an actual_length of 0. This patch changes the driver to use the last_td_was_short flag instead of checking actual_length. This fixes a bug affecting the HSO plugin ... etc. 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
Re: [PATCH] xhci: fix reporting of 0-sized URBs in control endpoint
On Sat, Feb 21, 2015 at 4:34 PM, Alan Stern wrote: > On Sat, 21 Feb 2015, Aleksander Morgado wrote: > >> Probably didn't explain well, sorry, likely mixing terms. What I mean >> is that when the data length received is equal to the transfer buffer >> length, we get a single IRQ event saying COMP_SUCCESS, with an event >> len = 0; the URB length in that case is set to be equal to the >> transfer buffer length. > >> When the received data length is less that what it was requested in >> the transfer buffer length, we get 2 IRQ events: one with >> COMP_SHORT_TX status and the event length giving the 'unfilled' size >> of the buffer; and one with COMP_SUCCESS status and an event length >> set to 0. In this case, the URB length is set when the first event >> arrived, not when the second one arrived. > >> The missing use case I'm trying to cover is when we do get 2 IRQ >> events for the same TD: one with COMP_SHORT_TX but where the >> "unfilled" length is equal to the transfer buffer length (hence, 0 >> bytes transferred), and the second event with COMP_SUCCESS. > > It sounds like this should be handled in the same way as the previous > case, but it isn't because of the peculiar way the driver is written. > >> The current logic sets the final URB length in 2 different cases: >> * If the transferred length is equal to the transfer buffer length, >> the URB length is set when the event for the last TRB is received, >> notifying COMP_SUCESSS. >> * If the transferred length is less than the transfer buffer length >> but > 0, the URB length is set when the COMP_SHORT_TX event arrives; >> when the COMP_SUCCESS one arrives the URB length that was previously >> set is not modified. >> >> Now, there is this 3rd case, where the transferred length is 0; in >> this case we get both COMP_SHORT_TX and COMP_SUCCESS events; and the >> first one states that the "untransferred" length is equal to the >> transfer buffer length. With this patch in, which truth be told seems >> a bit like a hack (is there any cleaner way?), the case is covered, >> and we do get the 0-length URB notified. > > I see the problem. The driver needs to distinguish between two cases: > COMP_SHORT_TX previously received and COMP_SHORT_TX not previously > received. The driver uses actual_length == 0 to make this distinction, > but that is not correct because it is possible to have actual_length == > 0 even after a COMP_SHORT_TX event. Using the last_td_was_short flag > instead seems like a reasonable solution. > >> Currently the xhci driver doesn't report 0-sized URBs in the control >> endpoint, and that totally breaks the HSO driver usecase (which relies >> on the 0-sized URBs to stop re-submitting the RX URB), so Option HSO >> modems don't work properly on USB3 ports :/ > > Clearly this is a bug. > >> Can also reword the commit message to try to make it clearer. >> Actually, I talk about COMP_SHORT_SUCCESS in the commit message when >> it really is COMP_SUCCESS... Just let me know how to move it forward >> :) > > In general, shorter commit messages are easier to understand. Try to > avoid the tendency to report too much information. :-) > > It wouldn't hurt simply to explain the situation as you did to me just > now (but perhaps in a more consise form). Something like this: > > When a control transfer has a short data stage, the xHCI > controller generates two transfer events: a COMP_SHORT_TX event > that includes the untransferred amount, and a COMP_SUCCESS > event. But when the data stage is not short, only the > COMP_SUCCESS event occurs. > > Therefore, xhci-hcd must set urb->actual_length to > urb->transfer_buffer_length while processing the COMP_SUCCESS > event, unless actual_length was set previously. The driver > checks this by seeing whether actual_length == 0. But this is > the wrong test, because it is entirely possible for a short > transfer to have an actual_length of 0. > > This patch changes the driver to use the last_td_was_short flag > instead of checking actual_length. This fixes a bug affecting > the HSO plugin ... etc. > I think that part of the mess in the long explanation was because I was trying to explain to myself the issue in the first place. But yes, as soon as the issue is understood, a shorter explanation is clearly more efficient. I'll resubmit the patch with the shorter explanation. Thanks! -- Aleksander https://aleksander.es -- 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 v4 4/4] phy: add phy-hi6220-usb
Hi,
On Sat, Feb 21, 2015 at 11:03:05PM +0800, zhangfei wrote:
> +static void hi6220_start_peripheral(struct hi6220_priv *priv, bool on)
> +{
> + struct usb_otg *otg = priv->phy.otg;
> +
> + if (!otg->gadget)
> + return;
> +
> + if (on)
> + usb_gadget_connect(otg->gadget);
> + else
> + usb_gadget_disconnect(otg->gadget);
> >>>
> >>>why is the PHY fiddling with pullups ?
> >>
> >>We use this to enable/disable otg gadget mode.
> >
> >I got that, but the pullups don't belong to the PHY, they belong to the
> >gadget.
> >
> >>The gpio_id & gpio_vbus are used to distinguish otg gadget mode or
> >>host mode.
> >>When micro usb or otg device attached to otg, gpio_vbus falling down.
> >>And gpio_id = 1 is micro usb, gpio_id = 0 is otg device.
> >
> >all of that I understood clearly :-)
> >
> >>So when micro usb attached, we enable gadget mode; while micro usb
> >>detached, we disable gadget mode, and dwc2 will automatically set to
> >>host mode.
> >
> >that's all fine, I'm concerned about letting the PHY fiddle with
> >something it doesn't own. If I am to change pullups rules in udc-core,
> >this is likely to break down miserably and I don't want to have to go
> >through that.
>
> Thanks for the clarifying.
no problem.
> How about using usb_gadget_vbus_connect/disconnect, which are used in many
> files under drivers/usb/phy.
> There is no vbus_session in dwc2/gadget.c, I thought it would be same as
> pullup.
>
> However, usb_gadget_vbus_connect still need para gadget, where should we put
> this file, drivers/usb/phy or drivers/phy
drivers/phy, if the framework misses anything you need, it's a great
opportunity to give back to the community by extending the framework.
Scratching one's own itch kinda thing...
> +static void hi6220_detect_work(struct work_struct *work)
> +{
> + struct hi6220_priv *priv =
> + container_of(work, struct hi6220_priv, work.work);
> + int gpio_id, gpio_vbus;
> + enum usb_otg_state state;
> +
> + if (!gpio_is_valid(priv->gpio_id) || !gpio_is_valid(priv->gpio_vbus))
> + return;
> +
> + gpio_id = gpio_get_value_cansleep(priv->gpio_id);
> + gpio_vbus = gpio_get_value_cansleep(priv->gpio_vbus);
> >>>
> >>>looks like this should be using extcon
> >>Not used extcon before.
> >>However, we need gpio_vbus interrupt.
> >>Checked phy-tahvo.c and phy-omap-otg.c, not find extcon related with
> >>interrupt.
> >>Will investigate tomorrow.
> >
> >drivers/extcon/extcon-gpio.c
> I think there is no need to use extcon, gpio is clear enough.
> extcon-gpio.c even do not support dt.
well, add DT. The whole idea of free software is that we improve on
things we already have. EXTCON is *the* API to handle such things.
Quite frankly, though, Roger Quadros (now in Cc) has been working to get
DT support on extcon-gpio, so perhaps wait for that and base your
patches on top of his.
Now your statement that GPIO is clear enough is completely bogus to me.
Why do we have fixed regulators with GPIO enable signals, right ? Might
as well just fiddle with the GPIO directly, right ? Wrong, the idea of
using these frameworks is to encapsulate implementation details and make
sure that if things change from one board to another, we can still use
our SW without major modifications.
> + if (gpio_vbus == 0) {
> + if (gpio_id == 1)
> + state = OTG_STATE_B_PERIPHERAL;
> + else
> + state = OTG_STATE_A_HOST;
> + } else {
> + state = OTG_STATE_A_HOST;
> + }
> +
> + if (priv->state != state) {
> + hi6220_start_peripheral(priv, state == OTG_STATE_B_PERIPHERAL);
> + priv->state = state;
> + }
> +}
> +
> +static irqreturn_t hiusb_gpio_intr(int irq, void *data)
> +{
> + struct hi6220_priv *priv = (struct hi6220_priv *)data;
> +
> + /* add debounce time */
> + schedule_delayed_work(&priv->work, msecs_to_jiffies(100));
> >>>
> >>>this is really bad. We have threaded interrupt support, right ?
> >>
> >>Since we use two gpio to distinguish gadget mode or host mode.
> >>Debounce time can introduce more accuracy.
> >
> >gpio_set_debounce() ?
> Not all gpio.c support set_debounce, including gpio-pl061.c.
then the framework should implement it in SW. That was the whole idea of
adding set_debounce() to gpiolib. If the controller can't handle it,
gpiolib handles it in SW. Again, encapsulating details.
> >>I think threaded interrupt can not be used for adding debounce time.
> >>Here add debounce is just for safety.
> >
> >add the debounce to the gpio itself.
>
> Here the debounce added only for safety.
> gpio_id may mis-report when unplug usb, but it is correct for plug usb & otg
> device.
> So debounce can be omitted.
> If you think using delayed work for debounce is ugly, it is fine switch to
> threaded_irq.
debounce might be very well needed. We *do* wan
Re: gt-s5570 and pl2303.ko
On Sat, Feb 21, 2015 at 09:38:20AM +0100, Milutin Aksic wrote: > Ok, then what I should try else what can be the problem why host driver > doesn't bind to the device? Maybe the phone gt-s5570 doesn't have > any hardware (but I doubt it is the truth) or something else. I have no idea what hardware this phone has, nor how it would talk USB if it could. I suggest getting the hardware details and working it out, there's not much we can do here, sorry. 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
[PATCH v2] xhci: fix reporting of 0-sized URBs in control endpoint
When a control transfer has a short data stage, the xHCI controller generates
two transfer events: a COMP_SHORT_TX event that specifies the untransferred
amount, and a COMP_SUCCESS event. But when the data stage is not short, only the
COMP_SUCCESS event occurs. Therefore, xhci-hcd must set urb->actual_length to
urb->transfer_buffer_length while processing the COMP_SUCCESS event, unless
urb->actual_length was set already by a previous COMP_SHORT_TX event.
The driver checks this by seeing whether urb->actual_length == 0, but this alone
is the wrong test, as it is entirely possible for a short transfer to have an
urb->actual_length = 0.
This patch changes the xhci driver to rely not only on the urb->actual_length,
but also on the ep_ring->last_td_was_short flag, which is set to true when a
COMP_SHORT_TX event is received.
This fixes a bug which affected the HSO plugin, which relies on URBs with
urb->actual_length == 0 to halt re-submitting the RX URB in the control
endpoint.
Signed-off-by: Aleksander Morgado
---
drivers/usb/host/xhci-ring.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 88da8d6..6b050f1 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1955,7 +1955,7 @@ static int process_ctrl_td(struct xhci_hcd *xhci, struct
xhci_td *td,
/* Did we already see a short data
* stage? */
*status = -EREMOTEIO;
- } else {
+ } else if (!ep_ring->last_td_was_short) {
td->urb->actual_length =
td->urb->transfer_buffer_length;
}
@@ -2447,10 +2447,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
ret = skip_isoc_td(xhci, td, event, ep, &status);
goto cleanup;
}
- if (trb_comp_code == COMP_SHORT_TX)
- ep_ring->last_td_was_short = true;
- else
- ep_ring->last_td_was_short = false;
if (ep->skip) {
xhci_dbg(xhci, "Found td. Clear skip flag.\n");
@@ -2484,6 +2480,14 @@ static int handle_tx_event(struct xhci_hcd *xhci,
ret = process_bulk_intr_td(xhci, td, event_trb, event,
ep, &status);
+ /* Flag whether the just processed TRB was short. Do it after
+* processing, so that the processor methods can also use this
+* flag. */
+ if (trb_comp_code == COMP_SHORT_TX)
+ ep_ring->last_td_was_short = true;
+ else
+ ep_ring->last_td_was_short = false;
+
cleanup:
/*
* Do not update event ring dequeue pointer if ep->skip is set.
--
2.3.0
--
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
[PATCH 10/27] pxa27x_udc: Remove use of seq_printf return value
The seq_printf return value, because it's frequently misused,
will eventually be converted to void.
See: commit 1f33c41c03da ("seq_file: Rename seq_overflow() to
seq_has_overflowed() and make public")
While there, simplify the error handler logic by returning
immediately and remove the unnecessary labels.
Signed-off-by: Joe Perches
---
drivers/usb/gadget/udc/pxa27x_udc.c | 132
1 file changed, 60 insertions(+), 72 deletions(-)
diff --git a/drivers/usb/gadget/udc/pxa27x_udc.c
b/drivers/usb/gadget/udc/pxa27x_udc.c
index 6a855fc..486f754 100644
--- a/drivers/usb/gadget/udc/pxa27x_udc.c
+++ b/drivers/usb/gadget/udc/pxa27x_udc.c
@@ -93,50 +93,46 @@ static void handle_ep(struct pxa_ep *ep);
static int state_dbg_show(struct seq_file *s, void *p)
{
struct pxa_udc *udc = s->private;
- int pos = 0, ret;
u32 tmp;
- ret = -ENODEV;
if (!udc->driver)
- goto out;
+ return -ENODEV;
/* basic device status */
- pos += seq_printf(s, DRIVER_DESC "\n"
-"%s version: %s\nGadget driver: %s\n",
-driver_name, DRIVER_VERSION,
-udc->driver ? udc->driver->driver.name : "(none)");
+ seq_printf(s, DRIVER_DESC "\n"
+ "%s version: %s\n"
+ "Gadget driver: %s\n",
+ driver_name, DRIVER_VERSION,
+ udc->driver ? udc->driver->driver.name : "(none)");
tmp = udc_readl(udc, UDCCR);
- pos += seq_printf(s,
-"udccr=0x%0x(%s%s%s%s%s%s%s%s%s%s), "
-"con=%d,inter=%d,altinter=%d\n", tmp,
-(tmp & UDCCR_OEN) ? " oen":"",
-(tmp & UDCCR_AALTHNP) ? " aalthnp":"",
-(tmp & UDCCR_AHNP) ? " rem" : "",
-(tmp & UDCCR_BHNP) ? " rstir" : "",
-(tmp & UDCCR_DWRE) ? " dwre" : "",
-(tmp & UDCCR_SMAC) ? " smac" : "",
-(tmp & UDCCR_EMCE) ? " emce" : "",
-(tmp & UDCCR_UDR) ? " udr" : "",
-(tmp & UDCCR_UDA) ? " uda" : "",
-(tmp & UDCCR_UDE) ? " ude" : "",
-(tmp & UDCCR_ACN) >> UDCCR_ACN_S,
-(tmp & UDCCR_AIN) >> UDCCR_AIN_S,
-(tmp & UDCCR_AAISN) >> UDCCR_AAISN_S);
+ seq_printf(s,
+ "udccr=0x%0x(%s%s%s%s%s%s%s%s%s%s),
con=%d,inter=%d,altinter=%d\n",
+ tmp,
+ (tmp & UDCCR_OEN) ? " oen":"",
+ (tmp & UDCCR_AALTHNP) ? " aalthnp":"",
+ (tmp & UDCCR_AHNP) ? " rem" : "",
+ (tmp & UDCCR_BHNP) ? " rstir" : "",
+ (tmp & UDCCR_DWRE) ? " dwre" : "",
+ (tmp & UDCCR_SMAC) ? " smac" : "",
+ (tmp & UDCCR_EMCE) ? " emce" : "",
+ (tmp & UDCCR_UDR) ? " udr" : "",
+ (tmp & UDCCR_UDA) ? " uda" : "",
+ (tmp & UDCCR_UDE) ? " ude" : "",
+ (tmp & UDCCR_ACN) >> UDCCR_ACN_S,
+ (tmp & UDCCR_AIN) >> UDCCR_AIN_S,
+ (tmp & UDCCR_AAISN) >> UDCCR_AAISN_S);
/* registers for device and ep0 */
- pos += seq_printf(s, "udcicr0=0x%08x udcicr1=0x%08x\n",
- udc_readl(udc, UDCICR0), udc_readl(udc, UDCICR1));
- pos += seq_printf(s, "udcisr0=0x%08x udcisr1=0x%08x\n",
- udc_readl(udc, UDCISR0), udc_readl(udc, UDCISR1));
- pos += seq_printf(s, "udcfnr=%d\n", udc_readl(udc, UDCFNR));
- pos += seq_printf(s, "irqs: reset=%lu, suspend=%lu, resume=%lu, "
- "reconfig=%lu\n",
- udc->stats.irqs_reset, udc->stats.irqs_suspend,
- udc->stats.irqs_resume, udc->stats.irqs_reconfig);
-
- ret = 0;
-out:
- return ret;
+ seq_printf(s, "udcicr0=0x%08x udcicr1=0x%08x\n",
+ udc_readl(udc, UDCICR0), udc_readl(udc, UDCICR1));
+ seq_printf(s, "udcisr0=0x%08x udcisr1=0x%08x\n",
+ udc_readl(udc, UDCISR0), udc_readl(udc, UDCISR1));
+ seq_printf(s, "udcfnr=%d\n", udc_readl(udc, UDCFNR));
+ seq_printf(s, "irqs: reset=%lu, suspend=%lu, resume=%lu,
reconfig=%lu\n",
+ udc->stats.irqs_reset, udc->stats.irqs_suspend,
+ udc->stats.irqs_resume, udc->stats.irqs_reconfig);
+
+ return 0;
}
static int queues_dbg_show(struct seq_file *s, void *p)
@@ -144,75 +140,67 @@ static int queues_dbg_show(struct seq_file *s, void *p)
struct pxa_udc *udc = s->private;
struct pxa_ep *ep;
struct pxa27x_request *req;
- int pos = 0, i, maxpkt, ret;
+ int i, maxpkt;
- ret = -ENODEV;
if (!udc->driver)
- goto out;
+ return -ENODEV;
/* dump
[PATCH 00/27] Convert seq_ output calls to return void
As Al Viro said: we are getting well-meaning folks who try to check that return value, again and again, getting it wrong every time. Typical idiocies: * return some kind of error out of ->show() on overflows. Pointless *and* wrong - only hard errors (== fail read(2) with that) should be reported that way; the caller does detect overflow and retires with bigger buffer just fine. * keep checking it after every sodding call of seq_...(), screwing the cleanups up more often than not. Pointless, unless you are doing some seriously expensive calculations to produce something you are going to print. seq_...() are no-ops in case when overflow has already happened. seq_has_overflowed() is only for situations when you really want to skip a serious amount of work generating the data that would end up being discarded and recalculated again when the caller grabs a bigger buffer and calls you again. And more often than not it's an indication of ->show() trying to do the work of iterator - e.g. when you have single_open() with ->show() printing the entire hash table of some sort all in one record. Most of the time checking return value of seq_...() is better replaced with not doing that. And "must check return value and Do Something(tm)" is too strong habit for enough people to cause recurring trouble. Joe Perches (27): staging: lustre: Convert "return seq_printf(...)" uses staging: lustre: Convert seq_ hash functions to return void staging: lustre: Convert uses of "int rc = seq_printf(...)" staging: lustre: Convert remaining uses of "= seq_printf(...)" x86: mtrr: if: Remove use of seq_printf return value power: wakeup: Remove use of seq_printf return value ipmi: Remove use of seq_printf return value rtc: Remove use of seq_printf return value ipc: Remove use of seq_printf return value pxa27x_udc: Remove use of seq_printf return value microblaze: mb: Remove use of seq_printf return value nios2: cpuinfo: Remove use of seq_printf return value ARM: plat-pxa: Remove use of seq_printf return value openrisc: Remove use of seq_printf return value cris: Remove use of seq_printf return value mfd: ab8500-debugfs: Remove use of seq_printf return value staging: i2o: Remove use of seq_printf return value staging: rtl8192x: Remove use of seq_printf return value s390: Remove use of seq_printf return value i8k: Remove use of seq_printf return value watchdog: bcm281xx: Remove use of seq_printf return value proc: Remove use of seq_printf return value cgroup: Remove use of seq_printf return value tracing: Remove use of seq_printf return value lru_cache: Remove use of seq_printf return value parisc: Remove use of seq_printf return value regulator: dbx500: Remove use of seq_puts/seq_printf return value arch/arm/plat-pxa/dma.c| 111 ++-- arch/cris/arch-v10/kernel/setup.c | 58 +++--- arch/cris/arch-v32/kernel/setup.c | 62 +++ arch/microblaze/kernel/cpu/mb.c| 149 arch/nios2/kernel/cpuinfo.c| 77 arch/openrisc/kernel/setup.c | 50 +++--- arch/s390/pci/pci_debug.c | 6 +- arch/x86/kernel/cpu/mtrr/if.c | 12 +- drivers/base/power/wakeup.c| 16 +- drivers/char/i8k.c | 16 +- drivers/char/ipmi/ipmi_msghandler.c| 12 +- drivers/char/ipmi/ipmi_si_intf.c | 26 +-- drivers/char/ipmi/ipmi_ssif.c | 4 +- drivers/mfd/ab8500-debugfs.c | 196 + drivers/parisc/ccio-dma.c | 54 +++--- drivers/parisc/sba_iommu.c | 86 + drivers/regulator/dbx500-prcmu.c | 32 +--- drivers/rtc/rtc-cmos.c | 36 ++-- drivers/rtc/rtc-ds1305.c | 6 +- drivers/rtc/rtc-mrst.c | 16 +- drivers/rtc/rtc-tegra.c| 4 +- drivers/s390/cio/blacklist.c | 12 +- drivers/staging/i2o/i2o_proc.c | 18 +- .../lustre/include/linux/libcfs/libcfs_hash.h | 4 +- drivers/staging/lustre/lustre/fid/lproc_fid.c | 23 ++- drivers/staging/lustre/lustre/libcfs/hash.c| 13 +- drivers/staging/lustre/lustre/llite/lproc_llite.c | 117 ++-- drivers/staging/lustre/lustre/lmv/lproc_lmv.c | 18 +- drivers/staging/lustre/lustre/lov/lproc_lov.c | 30 ++-- drivers/staging/lustre/lustre/mdc/lproc_mdc.c | 6 +- .../lustre/lustre/obdclass/linux/linux-module.c| 38 ++-- .../lustre/lustre/obdclass/lprocfs_status.c| 108 ++-- drivers/staging/lustre/lustre/obdclass/lu_object.c | 25 +-- drivers/staging/lustre/lustre/osc/lproc_osc.c | 67 +++ .../staging/lustre/
Re: [PATCH v4 4/4] phy: add phy-hi6220-usb
Hi, Balbi
On 02/22/2015 12:21 AM, Felipe Balbi wrote:
Hi,
On Sat, Feb 21, 2015 at 11:03:05PM +0800, zhangfei wrote:
+static void hi6220_start_peripheral(struct hi6220_priv *priv, bool on)
+{
+ struct usb_otg *otg = priv->phy.otg;
+
+ if (!otg->gadget)
+ return;
+
+ if (on)
+ usb_gadget_connect(otg->gadget);
+ else
+ usb_gadget_disconnect(otg->gadget);
why is the PHY fiddling with pullups ?
We use this to enable/disable otg gadget mode.
I got that, but the pullups don't belong to the PHY, they belong to the
gadget.
The gpio_id & gpio_vbus are used to distinguish otg gadget mode or
host mode.
When micro usb or otg device attached to otg, gpio_vbus falling down.
And gpio_id = 1 is micro usb, gpio_id = 0 is otg device.
all of that I understood clearly :-)
So when micro usb attached, we enable gadget mode; while micro usb
detached, we disable gadget mode, and dwc2 will automatically set to
host mode.
that's all fine, I'm concerned about letting the PHY fiddle with
something it doesn't own. If I am to change pullups rules in udc-core,
this is likely to break down miserably and I don't want to have to go
through that.
Thanks for the clarifying.
no problem.
How about using usb_gadget_vbus_connect/disconnect, which are used in many
files under drivers/usb/phy.
There is no vbus_session in dwc2/gadget.c, I thought it would be same as
pullup.
However, usb_gadget_vbus_connect still need para gadget, where should we put
this file, drivers/usb/phy or drivers/phy
drivers/phy, if the framework misses anything you need, it's a great
opportunity to give back to the community by extending the framework.
Sorry, I am a little confused.
I need some concrete suggestion for the next step of this patch, which
is required for the community board, hikey board.
Do you mean in the future we need use hsotg->phy instead of hsotg->uphy.
struct phy *phy;
struct usb_phy *uphy;
usb_phy has many members that struct phy does not have, including otg.
struct usb_otg *otg;
Is that mean we need port such member from usb_phy to phy.
Besides, are you ok with using usb_gadget_vbus_connect/disconnect.
Scratching one's own itch kinda thing...
+static void hi6220_detect_work(struct work_struct *work)
+{
+ struct hi6220_priv *priv =
+ container_of(work, struct hi6220_priv, work.work);
+ int gpio_id, gpio_vbus;
+ enum usb_otg_state state;
+
+ if (!gpio_is_valid(priv->gpio_id) || !gpio_is_valid(priv->gpio_vbus))
+ return;
+
+ gpio_id = gpio_get_value_cansleep(priv->gpio_id);
+ gpio_vbus = gpio_get_value_cansleep(priv->gpio_vbus);
looks like this should be using extcon
Not used extcon before.
However, we need gpio_vbus interrupt.
Checked phy-tahvo.c and phy-omap-otg.c, not find extcon related with
interrupt.
Will investigate tomorrow.
drivers/extcon/extcon-gpio.c
I think there is no need to use extcon, gpio is clear enough.
extcon-gpio.c even do not support dt.
well, add DT. The whole idea of free software is that we improve on
things we already have. EXTCON is *the* API to handle such things.
I think I am still not understanding extcon-gpio, not sure why need use
this API here.
Here two gpio requires, one gpio as interrupt, in the interrupt handler,
we detect the gpio status judging the otg status.
extcon-gpio.c use the interrupt, then can we also use the gpio interrupt.
Using extcon-gpio is used for saving gpio_request?
Quite frankly, though, Roger Quadros (now in Cc) has been working to get
DT support on extcon-gpio, so perhaps wait for that and base your
patches on top of his.
Now your statement that GPIO is clear enough is completely bogus to me.
Why do we have fixed regulators with GPIO enable signals, right ? Might
as well just fiddle with the GPIO directly, right ? Wrong, the idea of
using these frameworks is to encapsulate implementation details and make
sure that if things change from one board to another, we can still use
our SW without major modifications.
+ if (gpio_vbus == 0) {
+ if (gpio_id == 1)
+ state = OTG_STATE_B_PERIPHERAL;
+ else
+ state = OTG_STATE_A_HOST;
+ } else {
+ state = OTG_STATE_A_HOST;
+ }
+
+ if (priv->state != state) {
+ hi6220_start_peripheral(priv, state == OTG_STATE_B_PERIPHERAL);
+ priv->state = state;
+ }
+}
+
+static irqreturn_t hiusb_gpio_intr(int irq, void *data)
+{
+ struct hi6220_priv *priv = (struct hi6220_priv *)data;
+
+ /* add debounce time */
+ schedule_delayed_work(&priv->work, msecs_to_jiffies(100));
this is really bad. We have threaded interrupt support, right ?
Since we use two gpio to distinguish gadget mode or host mode.
Debounce time can introduce more accuracy.
gpio_set_debounce() ?
Not all gpio.c support set_debounce, incl
[PATCH v5 0/5] Add support for Fujitsu USB host controller
These patches add support for XHCI compliant Host controller found on Fujitsu Socs, and are based on http://lwn.net/Articles/629162/ The first patch is to add Fujitsu glue layer of Synopsis DesignWare USB3 driver and last four patch is about quirk implementation of errata in Synopsis DesignWare USB3 IP. Patch 1 introduces a quirk with device disconnection management necessary Synopsys Designware USB3 IP with versions < 3.00a and hardware configuration DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1. It solves a problem where without the quirk, that host controller will die after a usb device is disconnected from port of root hub. Patch 2 is to set Synopsis quirk in xhci platform driver based on xhci platform data. Patch 3 is to add a revison number 2.90a and 3.00a of Synopsis DesignWare USB3 IP core driver. Patch 4 introduces using a quirk based on a errata of Synopsis DesignWare USB3 IP which is versions < 3.00a and has hardware configuration DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, which cannot be read from software. As a result this quirk has to be enabled via platform data or device tree. Patch 5 introduces Fujitsu Specific Glue layer in Synopsis DesignWare USB3 IP driver. Successfully tested on Fujitsu mb86s7x board. Changes since v4 (RFC): [https://lkml.org/lkml/2015/2/17/13] - based on Felipe's comment, rename dwc3-mb86s70.c to dwc3-fujitsu.c which is more generic. - based on Mathias's comment, remove unhelpful comment, and change the function name of xhci_try_to_clear_csc() to xhci_late_csc_clear_quirk() which is more appropriate. Changes since v3 (RFC): [https://lkml.org/lkml/2015/1/25/8] - based on Mathias's comment, fix bug and using xhci_port_state_to_neutral() helper function to mask out some RW1C bits, prevent writing back all the bits read from the PORTSC register. Changes since v2 (RFC): [https://lkml.org/lkml/2015/1/19/55] - based on Felipe's comment, re-order patches to avoid breaking bisectability, - based on Felipe's comment, add comment to structure's member, and sort it alphabetically, - based on Felipe's comment, add another v2.90 revision number in dwc3 IP. Changes since v1 (RFC): [https://lkml.org/lkml/2014/12/15/929] - based on Arnd's comment, remove entire unnecessary Fujitsu EHCI/OHCI glue, - based on Felipe's comment, fix mis-using of runtime-pm API and setting dma mask, remove unnecessary comment, and refactor suspend/resume handler in Fujitsu Specific Glue layer in dwc3, - based on Felipe's comment, add more commit log and comments in Synopsis quirk implementation, and separate it into four patches. Sneeker Yeh (5): xhci: add a quirk for device disconnection errata for Synopsis Designware USB3 core xhci: Platform: Set Synopsis device disconnection quirk based on platform data usb: dwc3: add revision number DWC3_REVISION_290A and DWC3_REVISION_300A usb: dwc3: Add quirk for Synopsis device disconnection errata usb: dwc3: add Fujitsu Specific Glue layer Documentation/devicetree/bindings/usb/dwc3.txt | 17 ++ .../devicetree/bindings/usb/fujitsu-dwc3.txt | 33 drivers/usb/dwc3/Kconfig | 10 + drivers/usb/dwc3/Makefile |1 + drivers/usb/dwc3/core.c|6 + drivers/usb/dwc3/core.h|6 + drivers/usb/dwc3/dwc3-fujitsu.c| 206 drivers/usb/dwc3/host.c|4 + drivers/usb/dwc3/platform_data.h |8 + drivers/usb/host/xhci-hub.c|4 + drivers/usb/host/xhci-plat.c |3 + drivers/usb/host/xhci.c| 33 drivers/usb/host/xhci.h| 24 +++ include/linux/usb/xhci_pdriver.h |4 + 14 files changed, 359 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt create mode 100644 drivers/usb/dwc3/dwc3-fujitsu.c -- 1.7.9.5 -- 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
[PATCH v5 3/5] usb: dwc3: add revision number DWC3_REVISION_290A and DWC3_REVISION_300A
Add the contstant for v2.90a and v3.00a dwc3 IP detection
Signed-off-by: Sneeker Yeh
---
drivers/usb/dwc3/core.h |2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index d201910..0b3bb0f 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -772,6 +772,8 @@ struct dwc3 {
#define DWC3_REVISION_260A 0x5533260a
#define DWC3_REVISION_270A 0x5533270a
#define DWC3_REVISION_280A 0x5533280a
+#define DWC3_REVISION_290A 0x5533290a
+#define DWC3_REVISION_300A 0x5533300a
enum dwc3_ep0_next ep0_next_event;
enum dwc3_ep0_state ep0state;
--
1.7.9.5
--
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
[PATCH v5 4/5] usb: dwc3: Add quirk for Synopsis device disconnection errata
Synopsis Designware USB3 IP earlier than v3.00a which is configured in silicon
with DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, would need a specific quirk to prevent
xhci host controller from dying when device is disconnected.
Since DWC_USB3_SUSPEND_ON_DISCONNECT_EN is an IP configuration whose state
cannot be checked from software in runtime, it has to be enabled via platform
data or device tree.
Signed-off-by: Sneeker Yeh
---
Documentation/devicetree/bindings/usb/dwc3.txt | 17 +
drivers/usb/dwc3/core.c|6 ++
drivers/usb/dwc3/core.h|4
drivers/usb/dwc3/host.c|4
drivers/usb/dwc3/platform_data.h |8
5 files changed, 39 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt
b/Documentation/devicetree/bindings/usb/dwc3.txt
index cd7f045..1b78b29 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -37,6 +37,23 @@ Optional properties:
- snps,is-utmi-l1-suspend: true when DWC3 asserts output signal
utmi_l1_suspend_n, false when asserts utmi_sleep_n
- snps,hird-threshold: HIRD threshold
+ - snps,has_suspend_on_disconnect: true when IP is configured in silicon with
+ DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1, it can inject a
+ specific quirk to prevent xhci host controller from
+ dying when usb device is disconnected from root hub.
+ Since DWC_USB3_SUSPEND_ON_DISCONNECT_EN is an IP
+ configuration whose state cannot be checked from
+ software in runtime, it has to be enabled via platform
+ data or device tree.
+
+ xhci host dying symptom here is caused by that
+ DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1
+ configuration makes IP auto-suspended after PORTCSC is
+ cleared when usb device detached, then an asynchronous
+ disconnection procedure might fail using endpoint
+ command that suspened IP won't have any response to.
+
+ this issue is fixed when IP version >= 3.00a.
This is usually a subnode to DWC3 glue to which it is connected.
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 9f0e209..705980c 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -838,6 +838,9 @@ static int dwc3_probe(struct platform_device *pdev)
"snps,tx_de_emphasis_quirk");
of_property_read_u8(node, "snps,tx_de_emphasis",
&tx_de_emphasis);
+
+ dwc->suspend_on_disconnect_quirk = of_property_read_bool(node,
+ "snps,has_suspend_on_disconnect");
} else if (pdata) {
dwc->maximum_speed = pdata->maximum_speed;
dwc->has_lpm_erratum = pdata->has_lpm_erratum;
@@ -864,6 +867,9 @@ static int dwc3_probe(struct platform_device *pdev)
dwc->tx_de_emphasis_quirk = pdata->tx_de_emphasis_quirk;
if (pdata->tx_de_emphasis)
tx_de_emphasis = pdata->tx_de_emphasis;
+
+ dwc->suspend_on_disconnect_quirk =
+ pdata->has_suspend_on_disconnect;
}
/* default to superspeed if no maximum_speed passed */
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 0b3bb0f..dfc7d63 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -688,6 +688,9 @@ struct dwc3_scratchpad_array {
* @resize_fifos: tells us it's ok to reconfigure our TxFIFO sizes.
* @setup_packet_pending: true when there's a Setup Packet in FIFO. Workaround
* @start_config_issued: true when StartConfig command has been issued
+ * @suspend_on_disconnect_quirk: set if core was configured with
+ * DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1. Note that there's
+ * now way for software to detect this in runtime.
* @three_stage_setup: set if we perform a three phase setup
* @disable_scramble_quirk: set if we enable the disable scramble quirk
* @u2exit_lfps_quirk: set if we enable u2exit lfps quirk
@@ -813,6 +816,7 @@ struct dwc3 {
unsignedresize_fifos:1;
unsignedsetup_packet_pending:1;
unsignedstart_config_issued:1;
+ unsignedsuspend_on_disconnect_quirk:1;
unsignedthree_stage_setup:1;
unsigneddisable_scramble_quirk:1;
diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index 12bfd3c..9c42074 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -53,6 +53,10 @@ int dwc3_host_init(struct dwc3 *dwc)
pdata.usb3_lpm_capable =
[PATCH v5 5/5] usb: dwc3: add Fujitsu Specific Glue layer
This patch adds support for Synopsis DesignWare USB3 IP Core found
on Fujitsu Socs.
Signed-off-by: Sneeker Yeh
---
.../devicetree/bindings/usb/fujitsu-dwc3.txt | 33
drivers/usb/dwc3/Kconfig | 10 +
drivers/usb/dwc3/Makefile |1 +
drivers/usb/dwc3/dwc3-fujitsu.c| 206
4 files changed, 250 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
create mode 100644 drivers/usb/dwc3/dwc3-fujitsu.c
diff --git a/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
b/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
new file mode 100644
index 000..fd16fe9
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/fujitsu-dwc3.txt
@@ -0,0 +1,33 @@
+FUJITSU GLUE COMPONENTS
+
+MB86S7x DWC3 GLUE
+- compatible: Should be "fujitsu,mb86s7x-dwc3"
+- clocks: from common clock binding, handle to usb clock.
+- clock-names: Should contain the following:
+ "core" Master/Core clock needs to run at a minimum of 125 MHz to
+ support a 4 Gbps IN or 4 Gbps OUT
+ transfer at a given time.
+
+Sub-nodes:
+The dwc3 core should be added as subnode to MB86S7x dwc3 glue.
+- dwc3 :
+ The binding details of dwc3 can be found in:
+ Documentation/devicetree/bindings/usb/dwc3.txt
+
+Example device nodes:
+
+ usb3host: mb86s7x_usb3host {
+ compatible = "fujitsu,mb86s7x-dwc3";
+ clocks = <&clk_alw_1_1>;
+ clock-names = "core";
+ #address-cells = <2>;
+ #size-cells = <1>;
+ ranges;
+
+ dwc3@3220 {
+ compatible = "synopsys,dwc3";
+ reg = <0 0x3230 0x10>;
+ interrupts = <0 412 0x4>,
+ <0 414 0x4>;
+ };
+ };
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index edbf9c8..2e145ca 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -97,6 +97,16 @@ config USB_DWC3_QCOM
Recent Qualcomm SoCs ship with one DesignWare Core USB3 IP inside,
say 'Y' or 'M' if you have one such device.
+config USB_DWC3_FUJITSU
+ tristate "Fujitsu MB86S7x Designware USB3 Platform code"
+ default USB_DWC3
+ help
+ MB86S7X SOC ship with DesignWare Core USB3 IP inside,
+ this implementation also integrated Fujitsu USB PHY inside
+ this Core USB3 IP.
+
+ say 'Y' or 'M' if you have one such device.
+
comment "Debugging features"
config USB_DWC3_DEBUG
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 46172f4..a92c747 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -37,3 +37,4 @@ obj-$(CONFIG_USB_DWC3_PCI)+= dwc3-pci.o
obj-$(CONFIG_USB_DWC3_KEYSTONE)+= dwc3-keystone.o
obj-$(CONFIG_USB_DWC3_QCOM)+= dwc3-qcom.o
obj-$(CONFIG_USB_DWC3_ST) += dwc3-st.o
+obj-$(CONFIG_USB_DWC3_FUJITSU) += dwc3-fujitsu.o
diff --git a/drivers/usb/dwc3/dwc3-fujitsu.c b/drivers/usb/dwc3/dwc3-fujitsu.c
new file mode 100644
index 000..307a44c
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-fujitsu.c
@@ -0,0 +1,206 @@
+/**
+ * dwc3-fujitsu.c - Fujitsu mb86s7x DWC3 Specific Glue layer
+ *
+ * Copyright (c) 2013 - 2015 FUJITSU SEMICONDUCTOR LIMITED
+ * http://jp.fujitsu.com/group/fsl
+ *
+ * Authors: Alice Chan
+ * Sneeker Yeh
+ * based on dwc3-exynos.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct dwc3_mb86s7x {
+ struct device *dev;
+ struct clk *clks[5];
+ u8 clk_cnt;
+};
+
+static int dwc3_mb86s7x_clk_control(struct device *dev, bool on)
+{
+ struct dwc3_mb86s7x *priv = dev_get_drvdata(dev);
+ int ret, i = priv->clk_cnt;
+
+ if (!on)
+ goto clock_off;
+
+ for (i = 0; i < priv->clk_cnt; i++) {
+ ret = clk_prepare_enable(priv->clks[i]);
+ if (ret) {
+ dev_err(dev, "failed to enable clock[%d]\n", i);
+ on = ret;
+ goto clock_off;
+ }
+ }
+
+ return 0;
+
+clock_off:
+ for (; i > 0;)
+ clk_disable_unprepare(priv->clks[--i]);
+
+ return on;
+}
+
+static int dwc3_mb86s7x_remove_child(struct device *dev, void *unused)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+
+ of_device_unregister(pdev);
+
+ return 0;
+}
+
+static int dwc3_mb86s7x_probe(struct platform_device
[PATCH v5 2/5] xhci: Platform: Set Synopsis device disconnection quirk based on platform data
If an xhci platform has Synopsis device disconnection errata then enable
XHCI_DISCONNECT_QUIRK quirk flag.
Signed-off-by: Sneeker Yeh
---
drivers/usb/host/xhci-plat.c |3 +++
include/linux/usb/xhci_pdriver.h |4
2 files changed, 7 insertions(+)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 08d402b..40beb95 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -147,6 +147,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
if ((node && of_property_read_bool(node, "usb3-lpm-capable")) ||
(pdata && pdata->usb3_lpm_capable))
xhci->quirks |= XHCI_LPM_SUPPORT;
+
+ if (pdata && pdata->delay_portcsc_clear)
+ xhci->quirks |= XHCI_DISCONNECT_QUIRK;
/*
* Set the xHCI pointer before xhci_plat_setup() (aka hcd_driver.reset)
* is called by usb_add_hcd().
diff --git a/include/linux/usb/xhci_pdriver.h b/include/linux/usb/xhci_pdriver.h
index 376654b..a37a3a5 100644
--- a/include/linux/usb/xhci_pdriver.h
+++ b/include/linux/usb/xhci_pdriver.h
@@ -18,10 +18,14 @@
*
* @usb3_lpm_capable: determines if this xhci platform supports USB3
* LPM capability
+ * @delay_portcsc_clear: determines if Synopsis USB3 core has errata in
+ * "DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1" hardware
+ * configuration.
*
*/
struct usb_xhci_pdata {
unsignedusb3_lpm_capable:1;
+ unsigneddelay_portcsc_clear:1;
};
#endif /* __USB_CORE_XHCI_PDRIVER_H */
--
1.7.9.5
--
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
[PATCH v5 1/5] xhci: add a quirk for device disconnection errata for Synopsis Designware USB3 core
This issue is defined by a three-way race at disconnect, between
1) Class driver interrupt endpoint resheduling attempts if the ISR gave an ep
error event due to device detach (it would try 3 times)
2) Disconnect interrupt on PORTSC_CSC, which is cleared by hub thread
asynchronously
3) The hardware IP was configured in silicon with
- DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1
- Synopsys IP version is < 3.00a
The IP will auto-suspend itself on device detach with some phy-specific interval
after CSC is cleared by 2)
If 2) and 3) complete before 1), the interrupts it expects will not be generated
by the autosuspended IP, leading to a deadlock. Even later disconnection
procedure would detect that corresponding urb is still in-progress and issue a
ep stop command, auto-suspended IP still won't respond to that command.
this defect would result in this when device detached:
---
[ 99.603544] usb 4-1: USB disconnect, device number 2
[ 104.615254] xhci-hcd xhci-hcd.0.auto: xHCI host not responding to stop
endpoint command.
[ 104.623362] xhci-hcd xhci-hcd.0.auto: Assuming host is dying, halting host.
[ 104.653261] xhci-hcd xhci-hcd.0.auto: Host not halted after 16000
microseconds.
[ 104.660584] xhci-hcd xhci-hcd.0.auto: Non-responsive xHCI host is not
halting.
[ 104.667817] xhci-hcd xhci-hcd.0.auto: Completing active URBs anyway.
[ 104.674198] xhci-hcd xhci-hcd.0.auto: HC died; cleaning up
--
As a result, when device detached, we desired to postpone "PORTCSC clear" behind
"disable slot". it's found that all executed ep command related to disconnetion,
are executed before "disable slot".
Signed-off-by: Sneeker Yeh
---
drivers/usb/host/xhci-hub.c |4
drivers/usb/host/xhci.c | 33 +
drivers/usb/host/xhci.h | 24
3 files changed, 61 insertions(+)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index a7865c4..3b8f7fc 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -368,6 +368,10 @@ static void xhci_clear_port_change_bit(struct xhci_hcd
*xhci, u16 wValue,
port_change_bit = "warm(BH) reset";
break;
case USB_PORT_FEAT_C_CONNECTION:
+ if ((xhci->quirks & XHCI_DISCONNECT_QUIRK) &&
+ !(readl(addr) & PORT_CONNECT))
+ return;
+
status = PORT_CSC;
port_change_bit = "connect";
break;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index ec8ac16..df0125d 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3580,6 +3580,37 @@ command_cleanup:
return ret;
}
+static void xhci_late_csc_clear_quirk(struct usb_hcd *hcd, int port_num)
+{
+ int max_ports;
+ struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+ __le32 __iomem **port_array;
+ u32 status;
+
+ if (hcd->speed == HCD_USB3) {
+ max_ports = xhci->num_usb3_ports;
+ port_array = xhci->usb3_ports;
+ } else {
+ max_ports = xhci->num_usb2_ports;
+ port_array = xhci->usb2_ports;
+ }
+
+ if (port_num > max_ports) {
+ xhci_err(xhci, "%s() port number invalid", __func__);
+ return;
+ }
+ status = readl(port_array[port_num - 1]);
+
+ /*
+* clearing the connect status bit will now immediately suspend these
+* quirky controllers.
+*/
+ if (!(status & PORT_CONNECT) && (status & PORT_CSC)) {
+ status = xhci_port_state_to_neutral(status);
+ writel(status | PORT_CSC, port_array[port_num - 1]);
+ }
+}
+
/*
* At this point, the struct usb_device is about to go away, the device has
* disconnected, and all traffic has been stopped and the endpoints have been
@@ -3645,6 +3676,8 @@ void xhci_free_dev(struct usb_hcd *hcd, struct usb_device
*udev)
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);
+ if (xhci->quirks & XHCI_DISCONNECT_QUIRK)
+ xhci_late_csc_clear_quirk(hcd, udev->portnum);
/*
* Event command completion handler will free any data structures
* associated with the slot. XXX Can free sleep?
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 9745147..cb74706 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1560,6 +1560,30 @@ struct xhci_hcd {
#define XHCI_SPURIOUS_WAKEUP (1 << 18)
/* For controllers with a broken beyond repair streams implementation */
#define XHCI_BROKEN_STREAMS(1 << 19)
+/*
+ * This issue is defined by a three-way race at disconnect in Synopsis USB3 IP,
+ * between
+ * 1) Class driver interrupt endpoint resheduling attempts if the ISR gave an
ep
+ *error event due to device detach (it would try 3 time
