Re: btusb_intr_complete returns -EPIPE

2014-11-09 Thread Naveen Kumar Parna
I am sorry for the late response.

I applied the patch and here is the dmesg log:

[  713.125709] ehci-pci :00:1a.0: split intr info2 42821c01 token
80108d46 overlay token 80108d46
[  713.125796] ehci-pci :00:1a.0: split intr info2 42821c01 token
80108d46 overlay token 80108d46
[  713.125853] hci4 urb 8800b89a7c00 status -32 count 0
[  713.125857] hci3 urb 8800b7399c00 status -32 count 0
[  713.126801] hci4
[  713.127003] hci3
[ 3046.032153] ehci-pci :00:1a.0: split intr info2 42821c01 token
108d46 overlay token 108d46
[ 3046.032227] ehci-pci :00:1a.0: split intr info2 42821c01 token
108d46 overlay token 108d46
[ 3046.032272] hci3 urb 8800b30f5a80 status -32 count 0
[ 3046.032278] hci4 urb 8800b30f53c0 status -32 count 0
[ 3046.033326] hci4
[ 3046.033344] hci3

Does it gives the reason for -32 status code?

Thanks,
Naveen

On Thu, Nov 6, 2014 at 10:14 PM, Alan Stern  wrote:
> On Thu, 6 Nov 2014, Naveen Kumar Parna wrote:
>
>> > Any idea why you see the CSPLITs now but didn't see them before?
>>
>> It looks like , I failed to locate the exact portion of the analyzer
>> log that corresponds to one of those -32 events in the usbmon log.
>
> Well, I still don't understand that, but never mind...
>
>> USB Analyzer records several megabytes of data very quickly, so it’s
>> very hard to find the portion of the analyzer log that corresponds to
>> one of those -32 events in the usbmon log. To avoid this difficulty I
>> applied the attached patch
>> (0002-btusb-clear-halt-if-intr-in-stalls.patch – got it from Oliver
>> Neukum) to btusb driver and reloaded this driver.
>>
>> The applied patch calls usb_clear_halt() on receiving the stall
>> response, so now I can easily search for ClearFeature(ENDPOINT_HALT)
>> request in the analyzer log. It can be found at "2.304 252 217" &
>> "2.316 264 600" time instance in the attached log.
>>
>> Re ran the Analyzer again and attached it’s exported text
>> log([2014-11-06 session 125138] Trace_only_ep0_ep1.txt). Here
>> filtered out the BulkIN transactions.
>>
>> usbmon log:
>> 8800b7670a80 506095728 C Ii:1:108:1 -32:1 0
>> 8800affdccc0 506107757 C Ii:1:108:1 -32:1 0
>
>> Here is the portion of the log that corresponds to “8800b7670a80
>> 506095728 C Ii:1:108:1 -32:1 0”:
>>
>> Start of Frame (6) HS 553.2 -> 553.7 2.302 964 717
>> SSPLIT IN transaction 105 1 HS No data 2.303 590 367
>> SSPLIT IN transaction 106 1 HS No data 2.303 591 283
>> SSPLIT IN transaction 107 1 HS No data 2.303 600 283
>> SSPLIT IN transaction 108 1 HS No data 2.303 601 350
>> Start of Frame (2) HS 554.0 -> 554.1 2.303 714 817
>> CSPLIT IN transaction 105 1 NAK HS No data 2.303 840 400
>> CSPLIT IN transaction 106 1 NAK HS No data 2.303 842 033
>> CSPLIT IN transaction 107 1 NAK HS No data 2.303 855 317
>> Start of Frame (3) HS 554.2 -> 554.4 2.303 964 850
>
> Obviously, there aren't any CSPLITs for device 108 ep 1.
>
>> Here is the portion of the log that corresponds to “8800affdccc0
>> 506107757 C Ii:1:108:1 -32:1 0":
>>
>> Start of Frame (6) HS 565.2 -> 565.7 2.314 966 383
>> SSPLIT IN transaction 105 1 HS No data 2.315 592 033
>> SSPLIT IN transaction 106 1 HS No data 2.315 592 967
>> SSPLIT IN transaction 107 1 HS No data 2.315 612 800
>> SSPLIT IN transaction 108 1 HS No data 2.315 613 850
>> Start of Frame (2) HS 566.0 -> 566.1 2.315 716 483
>> CSPLIT IN transaction 105 1 NAK HS No data 2.315 842 067
>> CSPLIT IN transaction 106 1 NAK HS No data 2.315 843 683
>> CSPLIT IN transaction 107 1 NAK HS No data 2.315 928 750
>> Start of Frame (3) HS 566.2 -> 566.4 2.315 966 517
>
>> In both the cases, CSPLIT of Dev-108 is missing in this portion of the log.
>>
>> So, Does this test log gives some conclusion?
>
> It indicates that the EHCI host controller hardware isn't working
> right.  Every now and then it skips sending CSPLIT packets when it
> should send them.
>
> I suppose it's possible that the host controller is okay and the
> problem is a bad memory chip.  That could also cause this sort of
> error.  Regardless, it has to be a hardware problem.
>
> Now, this doesn't explain why you get the -32 status code.  Maybe the
> patch below will provide more information.  Try running your test with
> this patch installed and see what shows up in the dmesg log.
>
> Alan Stern
>
>
>
> Index: usb-3.18/drivers/usb/host/ehci-q.c
> ===
> --- usb-3.18.orig/drivers/usb/host/ehci-q.c
> +++ usb-3.18/drivers/usb/host/ehci-q.c
> @@ -346,6 +346,12 @@ qh_completions (struct ehci_hcd *ehci, s
> /* always clean up qtds the hc de-activated */
>   retry_xacterr:
> if ((token & QTD_STS_ACTIVE) == 0) {
> +   u32 info2 = hc32_to_cpu(ehci, hw->hw_info2);
> +
> +   if ((info2 & QH_SMASK) && (token & 0x7e))
> +   ehci_info(ehci, "split intr info2 %x token %x 
> overlay token %x\n",
> +   

RE: [PATCH net-next v2 2/3] r8152: cleartheflagofSCHEDULE_TASKLETintasklet

2014-11-09 Thread Hayes Wang
 Francois Romieu [mailto:rom...@fr.zoreil.com] 
> Sent: Sunday, November 09, 2014 6:12 AM
[...]
> The performance explanation leaves me a bit unconvinced. Without any
> figure one could simply go for the always locked clear_bit because of:
> 1. the "I'm racy" message that the open-coded test + set sends
> 2. the extra work needed to avoid 1 (comment, explain, ...).

Thanks. I would modify this patch with clear_bit only.

> The extra time could thus be used to see what happens when napi is
> shoehorned in this tasklet machinery. I'd naively expect it to be
> relevant for efficiency.

I thought about NAPI, but I gave up. The reasons are
1. I don't sure if it would run when autosuspending.
2. There is no hw interrupt for USB device. And I have
   no idea about how to check if the USB transfer is
   completed by polling.
3. I have to control the rx packets numbers in poll().
   However, I couldn't control the packets number for
   each bulk-in transfer. I have to do extra works to
   deal with the rx flow.
4. I don't find much different between tasklet and NAPI.

Best Regards,
Hayes
--
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 net-next 2/2] r8152: adjust rtl_start_rx

2014-11-09 Thread Hayes Wang
 David Miller [mailto:da...@davemloft.net] 
> Sent: Saturday, November 08, 2014 12:35 AM
[...]
> Does this even work?
> 
> If you leave a hole in the ring, the device is going to stop there
> anyways.

Excuse me. I don't sure I understand your meaning clearly.

The behavior is different for PCI(e) and USB ethernet device.
The PCI nic could know the ring buffer by certain way, so
the device could fill the data into the buffer one by one
automatically. However, for usb nic, the driver has to
indicate (i.e. submit) each buffer for each data. The device
doesn't know what is the next buffer by itself. That is,
the driver determines the order by which the data would be
filled.

Therefore, when I try to submit 10 rx buffers and some of
them fail, I could get the data depending on the order of
the successful ones. Besides, the driver has to submit the
buffer for next data continually, so the previous unsuccessful
ones could be tried again for the same time.
 
Best Regards,
Hayes
--
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 v1] usb: phy: generic: add vbus support

2014-11-09 Thread Robert Jarzmik
Add support for vbus detection and power supply. This code is more or
less stolen from phy-gpio-vbus-usb.c, and aims at providing a detection
mechanism for VBus (ie. usb cable plug) based on a GPIO line, and a
power supply activation which draws current from the VBus.

Signed-off-by: Robert Jarzmik 
---
 drivers/usb/phy/phy-generic.c   | 90 -
 drivers/usb/phy/phy-generic.h   |  6 +++
 include/linux/usb/usb_phy_generic.h |  2 +
 3 files changed, 97 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index 71e061d..2f383a1 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -41,6 +42,9 @@
 
 #include "phy-generic.h"
 
+#define VBUS_IRQ_FLAGS \
+   (IRQF_SHARED | IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING)
+
 struct platform_device *usb_phy_generic_register(void)
 {
return platform_device_register_simple("usb_phy_generic",
@@ -68,6 +72,73 @@ static void nop_reset_set(struct usb_phy_generic *nop, int 
asserted)
usleep_range(1, 2);
 }
 
+/* interface to regulator framework */
+static void nop_set_vbus_draw(struct usb_phy_generic *nop, unsigned mA)
+{
+   struct regulator *vbus_draw = nop->vbus_draw;
+   int enabled;
+   int ret;
+
+   if (!vbus_draw)
+   return;
+
+   enabled = nop->vbus_draw_enabled;
+   if (mA) {
+   regulator_set_current_limit(vbus_draw, 0, 1000 * mA);
+   if (!enabled) {
+   ret = regulator_enable(vbus_draw);
+   if (ret < 0)
+   return;
+   nop->vbus_draw_enabled = 1;
+   }
+   } else {
+   if (enabled) {
+   ret = regulator_disable(vbus_draw);
+   if (ret < 0)
+   return;
+   nop->vbus_draw_enabled = 0;
+   }
+   }
+   nop->mA = mA;
+}
+
+
+static irqreturn_t nop_gpio_vbus_thread(int irq, void *data)
+{
+   struct usb_phy_generic *nop = data;
+   struct usb_otg *otg = nop->phy.otg;
+   int vbus, status;
+
+   vbus = gpiod_get_value(nop->gpiod_vbus);
+   if ((vbus ^ nop->vbus) == 0)
+   return IRQ_HANDLED;
+   nop->vbus = vbus;
+
+   if (vbus) {
+   status = USB_EVENT_VBUS;
+   nop->phy.state = OTG_STATE_B_PERIPHERAL;
+   nop->phy.last_event = status;
+   usb_gadget_vbus_connect(otg->gadget);
+
+   /* drawing a "unit load" is *always* OK, except for OTG */
+   nop_set_vbus_draw(nop, 100);
+
+   atomic_notifier_call_chain(&nop->phy.notifier, status,
+  otg->gadget);
+   } else {
+   nop_set_vbus_draw(nop, 0);
+
+   usb_gadget_vbus_disconnect(otg->gadget);
+   status = USB_EVENT_NONE;
+   nop->phy.state = OTG_STATE_B_IDLE;
+   nop->phy.last_event = status;
+
+   atomic_notifier_call_chain(&nop->phy.notifier, status,
+  otg->gadget);
+   }
+   return IRQ_HANDLED;
+}
+
 int usb_gen_phy_init(struct usb_phy *phy)
 {
struct usb_phy_generic *nop = dev_get_drvdata(phy->dev);
@@ -151,17 +222,22 @@ int usb_phy_gen_create_phy(struct device *dev, struct 
usb_phy_generic *nop,
needs_vcc = of_property_read_bool(node, "vcc-supply");
nop->gpiod_reset = devm_gpiod_get(dev, "reset-gpios");
err = PTR_ERR(nop->gpiod_reset);
+   if (!err) {
+   nop->gpiod_vbus = devm_gpiod_get(dev, "vbus-gpios");
+   err = PTR_ERR(nop->gpiod_vbus);
+   }
} else if (pdata) {
type = pdata->type;
clk_rate = pdata->clk_rate;
needs_vcc = pdata->needs_vcc;
-   if (gpio_is_valid(gpio->gpio_reset)) {
+   if (gpio_is_valid(pdata->gpio_reset)) {
err = devm_gpio_request_one(dev, pdata->gpio_reset, 0,
dev_name(dev));
if (!err)
nop->gpiod_reset =
gpio_to_desc(pdata->gpio_reset);
}
+   nop->gpiod_vbus = pdata->gpiod_vbus;
}
 
if (err == -EPROBE_DEFER)
@@ -226,6 +302,18 @@ static int usb_phy_generic_probe(struct platform_device 
*pdev)
err = usb_phy_gen_create_phy(dev, nop, dev_get_platdata(&pdev->dev));
if (err)
return err;
+   if (nop->gpiod_vbus) {
+   err = devm_request_threaded_irq(&pdev->dev,
+   gpiod_to_irq(nop->gpiod_vbus

[PATCH v1] usb: phy: nop: device tree documentation for vbus

2014-11-09 Thread Robert Jarzmik
Enhance the phy documentation by adding 2 new optional bindings :
 - the vbus gpio, which detects usb insertion
 - the vbus regulator, which provides current drawn from the usb cable

Signed-off-by: Robert Jarzmik 
---
 Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt | 8 
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt 
b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
index 1bd37fa..65dfe4b 100644
--- a/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
+++ b/Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt
@@ -17,6 +17,11 @@ Optional properties:
 
 - reset-gpios: Should specify the GPIO for reset.
 
+- vbus-gpios: should specify the GPIO detecting a VBus insertion
+  (see Documentation/devicetree/bindings/gpio/gpio.txt)
+- vbus-regulator : should specifiy the regulator supplying current drawn from
+  the VBus line (see 
Documentation/devicetree/bindings/regulator/regulator.txt).
+
 Example:
 
hsusb1_phy {
@@ -26,8 +31,11 @@ Example:
clock-names = "main_clk";
vcc-supply = <&hsusb1_vcc_regulator>;
reset-gpios = <&gpio1 7 GPIO_ACTIVE_LOW>;
+   vbus-gpios = <&gpio2 13 GPIO_ACTIVE_HIGH>;
+   vbus-regulator = <&vbus_regulator>;
};
 
 hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator
 and expects that clock to be configured to 19.2MHz by the NOP PHY driver.
 hsusb1_vcc_regulator provides power to the PHY and GPIO 7 controls RESET.
+GPIO 13 detects VBus insertion, and accordingly notifies the vbus-regulator.
-- 
2.1.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


Re: [PATCH v1] usb: phy: generic: migrate to gpio_desc

2014-11-09 Thread Robert Jarzmik
Robert Jarzmik  writes:

>   } else if (pdata) {
>   type = pdata->type;
>   clk_rate = pdata->clk_rate;
>   needs_vcc = pdata->needs_vcc;
> - nop->gpio_reset = pdata->gpio_reset;
> - } else {
> - nop->gpio_reset = -1;


> + err = devm_gpio_request_one(dev, pdata->gpio_reset, 0,
> + dev_name(dev));
> + if (!err)
> + nop->gpiod_reset = gpio_to_desc(pdata->gpio_reset);
This 4 lines block should be conditionally encompassed with a :
 if (gpio_is_valid(pdata->gpio_reset))

When I reread our previous conversation the "optional" part hit me there.

Cheers.

-- 
Robert
--
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 v1] usb: phy: generic: migrate to gpio_desc

2014-11-09 Thread Robert Jarzmik
Change internal gpio handling from integer gpios into gpio
descriptors. This change only addresses the internal API and
device-tree/ACPI, while the legacy platform data remains integer space
based.

This change is only build compile tested, and very prone to error. I
leave this comment for now in the commit message so that this patch gets
some testing as I'm pretty sure it's buggy.

Signed-off-by: Robert Jarzmik 
---
 drivers/usb/phy/phy-generic.c | 58 ---
 drivers/usb/phy/phy-generic.h |  4 +--
 2 files changed, 18 insertions(+), 44 deletions(-)

diff --git a/drivers/usb/phy/phy-generic.c b/drivers/usb/phy/phy-generic.c
index 7594e50..eba097a 100644
--- a/drivers/usb/phy/phy-generic.c
+++ b/drivers/usb/phy/phy-generic.c
@@ -61,16 +61,8 @@ static int nop_set_suspend(struct usb_phy *x, int suspend)
 
 static void nop_reset_set(struct usb_phy_generic *nop, int asserted)
 {
-   int value;
-
-   if (!gpio_is_valid(nop->gpio_reset))
-   return;
-
-   value = asserted;
-   if (nop->reset_active_low)
-   value = !value;
-
-   gpio_set_value_cansleep(nop->gpio_reset, value);
+   if (nop->gpiod_reset)
+   gpiod_set_value(nop->gpiod_reset, asserted);
 
if (!asserted)
usleep_range(1, 2);
@@ -145,35 +137,35 @@ int usb_phy_gen_create_phy(struct device *dev, struct 
usb_phy_generic *nop,
struct usb_phy_generic_platform_data *pdata)
 {
enum usb_phy_type type = USB_PHY_TYPE_USB2;
-   int err;
+   int err = 0;
 
u32 clk_rate = 0;
bool needs_vcc = false;
 
-   nop->reset_active_low = true;   /* default behaviour */
-
if (dev->of_node) {
struct device_node *node = dev->of_node;
-   enum of_gpio_flags flags = 0;
 
if (of_property_read_u32(node, "clock-frequency", &clk_rate))
clk_rate = 0;
 
needs_vcc = of_property_read_bool(node, "vcc-supply");
-   nop->gpio_reset = of_get_named_gpio_flags(node, "reset-gpios",
-   0, &flags);
-   if (nop->gpio_reset == -EPROBE_DEFER)
-   return -EPROBE_DEFER;
-
-   nop->reset_active_low = flags & OF_GPIO_ACTIVE_LOW;
-
+   nop->gpiod_reset = devm_gpiod_get(dev, "reset-gpios");
+   err = PTR_ERR(nop->gpiod_reset);
} else if (pdata) {
type = pdata->type;
clk_rate = pdata->clk_rate;
needs_vcc = pdata->needs_vcc;
-   nop->gpio_reset = pdata->gpio_reset;
-   } else {
-   nop->gpio_reset = -1;
+   err = devm_gpio_request_one(dev, pdata->gpio_reset, 0,
+   dev_name(dev));
+   if (!err)
+   nop->gpiod_reset = gpio_to_desc(pdata->gpio_reset);
+   }
+
+   if (err == -EPROBE_DEFER)
+   return -EPROBE_DEFER;
+   if (err) {
+   dev_err(dev, "Error requesting RESET GPIO\n");
+   return err;
}
 
nop->phy.otg = devm_kzalloc(dev, sizeof(*nop->phy.otg),
@@ -203,24 +195,6 @@ int usb_phy_gen_create_phy(struct device *dev, struct 
usb_phy_generic *nop,
return -EPROBE_DEFER;
}
 
-   if (gpio_is_valid(nop->gpio_reset)) {
-   unsigned long gpio_flags;
-
-   /* Assert RESET */
-   if (nop->reset_active_low)
-   gpio_flags = GPIOF_OUT_INIT_LOW;
-   else
-   gpio_flags = GPIOF_OUT_INIT_HIGH;
-
-   err = devm_gpio_request_one(dev, nop->gpio_reset,
-   gpio_flags, dev_name(dev));
-   if (err) {
-   dev_err(dev, "Error requesting RESET GPIO %d\n",
-   nop->gpio_reset);
-   return err;
-   }
-   }
-
nop->dev= dev;
nop->phy.dev= nop->dev;
nop->phy.label  = "nop-xceiv";
diff --git a/drivers/usb/phy/phy-generic.h b/drivers/usb/phy/phy-generic.h
index d8feacc..09924fd 100644
--- a/drivers/usb/phy/phy-generic.h
+++ b/drivers/usb/phy/phy-generic.h
@@ -2,14 +2,14 @@
 #define _PHY_GENERIC_H_
 
 #include 
+#include 
 
 struct usb_phy_generic {
struct usb_phy phy;
struct device *dev;
struct clk *clk;
struct regulator *vcc;
-   int gpio_reset;
-   bool reset_active_low;
+   struct gpio_desc *gpiod_reset;
 };
 
 int usb_gen_phy_init(struct usb_phy *phy);
-- 
2.1.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