Re: [PATCH] ARM: dts: Odroid XU4: fix USB3.0 ports

[Krzysztof Kozlowski]

On Wed, Jan 25, 2017 at 7:51 AM, Anand Moon  wrote:
> Hi Richard,
>
> On 24 January 2017 at 19:18, Richard Genoud  wrote:
>> Since commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores"),
>> the USB ports on odroid-XU4 don't work anymore.
>>
>> Inserting an usb key (USB2.0) on the USB3.0 port result in:
>> [   64.488264] xhci-hcd xhci-hcd.2.auto: Port resume took longer than 2 
>> msec, port status = 0xc400fe3
>> [   74.568156] xhci-hcd xhci-hcd.2.auto: xHCI host not responding to stop 
>> endpoint command.
>> [   74.574806] xhci-hcd xhci-hcd.2.auto: Assuming host is dying, halting 
>> host.
>> [   74.601970] xhci-hcd xhci-hcd.2.auto: HC died; cleaning up
>> [   74.606276] usb 3-1: USB disconnect, device number 2
>> [   74.613565] usb 4-1: USB disconnect, device number 2
>> [   74.621208] usb usb3-port1: couldn't allocate usb_device
>> NB: it's not related to USB2.0 devices, I get the same result with an USB3.0 
>> device (SATA to USB3 for instance).
>> NB2: it doesn't happen on an odriod-XU3 board, that doesn't have the realtek 
>> RTL8153 chip.
>>
>> If the lines:
>> if (dwc->revision > DWC3_REVISION_194A)
>> reg |= DWC3_GUSB3PIPECTL_SUSPHY;
>> are commented out, the USB3.0 start working.
>>
>> For a full analyse: https://lkml.org/lkml/2017/1/18/678
>>
>> Felipe suggested that suspend should be disabled temporarily while
>> what's wrong with DCW3 is figured out.
>>
>> Tested on Odroid XU4
>>
>> Suggested-by: Felipe Balbi 
>> Tested-by: Richard Genoud 
>> Signed-off-by: Richard Genoud 
>> Cc: sta...@vger.kernel.org # 4.4+
>> Fixes: 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores")
>> ---
>>  arch/arm/boot/dts/exynos5422-odroidxu4.dts | 9 +
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu4.dts 
>> b/arch/arm/boot/dts/exynos5422-odroidxu4.dts
>> index 2faf88627a48..f62dbd9b5ad3 100644
>> --- a/arch/arm/boot/dts/exynos5422-odroidxu4.dts
>> +++ b/arch/arm/boot/dts/exynos5422-odroidxu4.dts
>> @@ -43,6 +43,15 @@
>> status = "okay";
>>  };
>>
>> +&usbdrd_dwc3_0 {
>> +   /*
>> +* without that, usb devices are not recognized when
>> +* they are plugged.
>> +* cf: https://lkml.org/lkml/2017/1/18/678
>> +*/
>> +   snps,dis_u3_susphy_quirk;
>> +};
>> +
>>  &usbdrd_dwc3_1 {
>> dr_mode = "host";
>>  };
>> --
>
> Thanks for this patch.
> But could you consider moving this changes as below.
>
> https://lkml.org/lkml/2015/3/18/400

The patch above (and other DTS patches from the set) was not even sent
to linux-samsung-soc nor to me... It is sad how easily one's work can
disappear. Also, it is really worthless to solve the same problem
twice.

Cc Marek and Bartlomiej,
Guys, do you want to continue with Robert's patch (linked above by
Anand)? If yes, please take the ownership (Robert's address is not
valid anymore). If not, I will take Richard's patch after
resubmission.

Best regards,
Krzysztof
--
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: [PATCHv4 0/2] cppi41 dma fixes for v4.10-rc cycle

[Vinod Koul]

On Thu, Jan 19, 2017 at 08:49:06AM -0800, Tony Lindgren wrote:
> Hi all,
> 
> I'm using v4 naming here as the earlier patch "dmaengine: cppi41: Add dma
> support to da8xx" has been separated from the error -115 issue. We've
> identified so far three musb and cppi41 regressions:
> 
> 1. Error -71 regression with musb
> 
>This is not dma related, and fixed with recently posted patch
>"[PATCH 1/2] usb: musb: Fix host mode error -71 regression".
> 
> 2. Error -115 regression with cppi41 dma with musb
> 
>This regression was caused by commit 098de42ad670 ("dmaengine:
>cppi41: Fix unpaired pm runtime when only a USB hub is connected")
>and causes runtime PM autosuspend delay to time out on active dma
>transfers when connecting a mass storage device to a usb hub.
>This is fixed in the first patch in this series.
> 
> 3. Race with runtime PM and cppi41_dma_issue_pending()
> 
>This is really a separate issue from the error -115 problem, and
>fixed with the second patch in this series. That's minimal v4
>version of the "dmaengine: cppi41: Fix oops in cppi41_runtime_resume"
>patch.

Applied, thanks

-- 
~Vinod
--
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] ARM: dts: Odroid XU4: fix USB3.0 ports

[Anand Moon]

Hi Richard,

On 24 January 2017 at 19:18, Richard Genoud  wrote:
> Since commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores"),
> the USB ports on odroid-XU4 don't work anymore.
>
> Inserting an usb key (USB2.0) on the USB3.0 port result in:
> [   64.488264] xhci-hcd xhci-hcd.2.auto: Port resume took longer than 2 
> msec, port status = 0xc400fe3
> [   74.568156] xhci-hcd xhci-hcd.2.auto: xHCI host not responding to stop 
> endpoint command.
> [   74.574806] xhci-hcd xhci-hcd.2.auto: Assuming host is dying, halting host.
> [   74.601970] xhci-hcd xhci-hcd.2.auto: HC died; cleaning up
> [   74.606276] usb 3-1: USB disconnect, device number 2
> [   74.613565] usb 4-1: USB disconnect, device number 2
> [   74.621208] usb usb3-port1: couldn't allocate usb_device
> NB: it's not related to USB2.0 devices, I get the same result with an USB3.0 
> device (SATA to USB3 for instance).
> NB2: it doesn't happen on an odriod-XU3 board, that doesn't have the realtek 
> RTL8153 chip.
>
> If the lines:
> if (dwc->revision > DWC3_REVISION_194A)
> reg |= DWC3_GUSB3PIPECTL_SUSPHY;
> are commented out, the USB3.0 start working.
>
> For a full analyse: https://lkml.org/lkml/2017/1/18/678
>
> Felipe suggested that suspend should be disabled temporarily while
> what's wrong with DCW3 is figured out.
>
> Tested on Odroid XU4
>
> Suggested-by: Felipe Balbi 
> Tested-by: Richard Genoud 
> Signed-off-by: Richard Genoud 
> Cc: sta...@vger.kernel.org # 4.4+
> Fixes: 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores")
> ---
>  arch/arm/boot/dts/exynos5422-odroidxu4.dts | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu4.dts 
> b/arch/arm/boot/dts/exynos5422-odroidxu4.dts
> index 2faf88627a48..f62dbd9b5ad3 100644
> --- a/arch/arm/boot/dts/exynos5422-odroidxu4.dts
> +++ b/arch/arm/boot/dts/exynos5422-odroidxu4.dts
> @@ -43,6 +43,15 @@
> status = "okay";
>  };
>
> +&usbdrd_dwc3_0 {
> +   /*
> +* without that, usb devices are not recognized when
> +* they are plugged.
> +* cf: https://lkml.org/lkml/2017/1/18/678
> +*/
> +   snps,dis_u3_susphy_quirk;
> +};
> +
>  &usbdrd_dwc3_1 {
> dr_mode = "host";
>  };
> --

Thanks for this patch.
But could you consider moving this changes as below.

https://lkml.org/lkml/2015/3/18/400

Best Regards
-Anand

> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" 
> 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

[PATCH net-next] r8152: fix the wrong spelling

[Hayes Wang]

Replace rumtime with runtime.

Signed-off-by: Hayes Wang 
---
 drivers/net/usb/r8152.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f3b48ad..d59d773 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3576,7 +3576,7 @@ static bool delay_autosuspend(struct r8152 *tp)
return false;
 }
 
-static int rtl8152_rumtime_suspend(struct r8152 *tp)
+static int rtl8152_runtime_suspend(struct r8152 *tp)
 {
struct net_device *netdev = tp->netdev;
int ret = 0;
@@ -3653,7 +3653,7 @@ static int rtl8152_suspend(struct usb_interface *intf, 
pm_message_t message)
mutex_lock(&tp->control);
 
if (PMSG_IS_AUTO(message))
-   ret = rtl8152_rumtime_suspend(tp);
+   ret = rtl8152_runtime_suspend(tp);
else
ret = rtl8152_system_suspend(tp);
 
-- 
2.7.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: [PATCH v5 1/4] usb: dbc: early driver for xhci debug capability

[Lu Baolu]

Hi Ingo,

On 01/24/2017 04:20 PM, Ingo Molnar wrote:
> * Lu Baolu  wrote:
>
>> Hi Ingo,
>>
>> On 01/22/2017 05:04 PM, Ingo Molnar wrote:
>>> * Lu Baolu  wrote:
>>>
>> +static void xdbc_runtime_delay(unsigned long count)
>> +{
>> +udelay(count);
>> +}
>> +static void (*xdbc_delay)(unsigned long) = xdbc_early_delay;
> Is this udelay() complication really necessary? udelay() should work fine 
> even in 
> early code. It might not be precisely calibrated, but should be good 
> enough.
 I tried udelay() in the early code. It's not precise enough for the
 hardware handshaking.
>>> Possibly because on x86 early udelay() did not work at all - i.e. there's 
>>> no delay 
>>> whatsoever.
>> Yes.
>>
>>> Could you try it on top of this commit in tip:timers/core:
>>>
>>>   4c45c5167c95 x86/timer: Make delay() work during early bootup
>>>
>>> ?
>> I tried tip:timers/core. It's not precise enough for my context either.
>>
>> __const_udelay().
>>
>> 157 inline void __const_udelay(unsigned long xloops)
>> 158 {
>> 159 unsigned long lpj = this_cpu_read(cpu_info.loops_per_jiffy) ? : 
>> loops_per_jiffy;
>> 160 int d0;
>> 161
>> 162 xloops *= 4;
>> 163 asm("mull %%edx"
>> 164 :"=d" (xloops), "=&a" (d0)
>> 165 :"1" (xloops), "0" (lpj * (HZ / 4)));
>> 166
>> 167 __delay(++xloops);
>> 168 }
>>
>>
>> In my early  code, loops_per_jiffy is not initialized yet. Hence "lpj" for 
>> the asm line
>> is 4096 (default value).
>>
>> The  cpu_info.loops_per_jiffy actually reads 8832000 after initialization. 
>> They are
>> about 2000 times different.
>>
>> I did a hacky test in kernel to check the difference between these two 
>> different
>> "lpj" values. (The hacky patch is attached.) Below is the output for 100ms 
>> delay.
>>
>> [2.494751] udelay_test uninitialized >start
>> [2.494820] udelay_test uninitialized >end
>> [2.494828] udelay_test initialized >start
>> [2.595234] udelay_test initialized >end
>>
>> For 100ms delay, udelay() with uninitialized loops_per_jiffy only gives a 
>> delay of
>> only 69us.
> Ok, then could we add some simple calibration to make udelay work much better 
> - or 
> perhaps move the udelay calibration up earlier?
>
> Hiding essentially an early udelay() implementation in an early-printk driver 
> is 
> ugly and counterproductive.

Sure. How about below change?

diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
index d3f0c84..940989e 100644
--- a/drivers/usb/early/xhci-dbc.c
+++ b/drivers/usb/early/xhci-dbc.c
@@ -587,6 +587,35 @@ static int xdbc_bulk_transfer(void *data, int size, bool 
read)
return size;
 }
 
+static void __init xdbc_udelay_calibration(void)
+{
+   unsigned long lpj = 0;
+   unsigned int tsc_khz, cpu_khz;
+
+   if (!boot_cpu_has(X86_FEATURE_TSC))
+   goto calibration_out;
+
+   cpu_khz = x86_platform.calibrate_cpu();
+   tsc_khz = x86_platform.calibrate_tsc();
+
+   if (tsc_khz == 0)
+   tsc_khz = cpu_khz;
+   else if (abs(cpu_khz - tsc_khz) * 10 > tsc_khz)
+   cpu_khz = tsc_khz;
+
+   if (!tsc_khz)
+   goto calibration_out;
+
+   lpj = tsc_khz * 1000;
+   do_div(lpj, HZ);
+
+calibration_out:
+   if (!lpj)
+   lpj = 1 << 22;
+
+   loops_per_jiffy = lpj;
+}
+
 static int __init xdbc_early_setup(void)
 {
int ret;
@@ -686,6 +715,8 @@ int __init early_xdbc_parse_parameter(char *s)
}
xdbc.xdbc_reg = (struct xdbc_regs __iomem *)(xdbc.xhci_base + offset);
 
+   xdbc_udelay_calibration();
+
return 0;
 }

Best regards,
Lu Baolu
--
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 1/4] r8152: avoid start_xmit to call napi_schedule during autosuspend

[Stephen Hemminger]

On Wed, 25 Jan 2017 10:50:51 +0800
Hayes Wang  wrote:

> Adjust the setting of the flag of SELECTIVE_SUSPEND to prevent start_xmit()
> from calling napi_schedule() directly during runtime suspend.
> 
> After calling napi_disable() or clearing the flag of WORK_ENABLE,
> scheduling the napi is useless.
> 
> Signed-off-by: Hayes Wang 
> ---
>  drivers/net/usb/r8152.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index e1466b4..27b0b44 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -3585,10 +3585,13 @@ static int rtl8152_rumtime_suspend(struct r8152 *tp)
>   struct net_device *netdev = tp->netdev;
>   int ret = 0;
>  
> + set_bit(SELECTIVE_SUSPEND, &tp->flags);
> +
>   if (netif_running(netdev) && test_bit(WORK_ENABLE, &tp->flags)) {
>   u32 rcr = 0;
>  
>   if (delay_autosuspend(tp)) {
> + clear_bit(SELECTIVE_SUSPEND, &tp->flags);
>   ret = -EBUSY;
>   goto out1;
>   }
> @@ -3605,6 +3608,7 @@ static int rtl8152_rumtime_suspend(struct r8152 *tp)
>   if (!(ocp_data & RXFIFO_EMPTY)) {
>   rxdy_gated_en(tp, false);
>   ocp_write_dword(tp, MCU_TYPE_PLA, PLA_RCR, rcr);
> + clear_bit(SELECTIVE_SUSPEND, &tp->flags);
>   ret = -EBUSY;

If you are going to start using bit operations then you may need 
smp_mb_before/after_atomic.
--
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 net 1/4] r8152: avoid start_xmit to call napi_schedule during autosuspend

[Hayes Wang]

Adjust the setting of the flag of SELECTIVE_SUSPEND to prevent start_xmit()
from calling napi_schedule() directly during runtime suspend.

After calling napi_disable() or clearing the flag of WORK_ENABLE,
scheduling the napi is useless.

Signed-off-by: Hayes Wang 
---
 drivers/net/usb/r8152.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index e1466b4..27b0b44 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3585,10 +3585,13 @@ static int rtl8152_rumtime_suspend(struct r8152 *tp)
struct net_device *netdev = tp->netdev;
int ret = 0;
 
+   set_bit(SELECTIVE_SUSPEND, &tp->flags);
+
if (netif_running(netdev) && test_bit(WORK_ENABLE, &tp->flags)) {
u32 rcr = 0;
 
if (delay_autosuspend(tp)) {
+   clear_bit(SELECTIVE_SUSPEND, &tp->flags);
ret = -EBUSY;
goto out1;
}
@@ -3605,6 +3608,7 @@ static int rtl8152_rumtime_suspend(struct r8152 *tp)
if (!(ocp_data & RXFIFO_EMPTY)) {
rxdy_gated_en(tp, false);
ocp_write_dword(tp, MCU_TYPE_PLA, PLA_RCR, rcr);
+   clear_bit(SELECTIVE_SUSPEND, &tp->flags);
ret = -EBUSY;
goto out1;
}
@@ -3624,8 +3628,6 @@ static int rtl8152_rumtime_suspend(struct r8152 *tp)
}
}
 
-   set_bit(SELECTIVE_SUSPEND, &tp->flags);
-
 out1:
return ret;
 }
@@ -3681,12 +3683,12 @@ static int rtl8152_resume(struct usb_interface *intf)
if (netif_running(tp->netdev) && tp->netdev->flags & IFF_UP) {
if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
tp->rtl_ops.autosuspend_en(tp, false);
-   clear_bit(SELECTIVE_SUSPEND, &tp->flags);
napi_disable(&tp->napi);
set_bit(WORK_ENABLE, &tp->flags);
if (netif_carrier_ok(tp->netdev))
rtl_start_rx(tp);
napi_enable(&tp->napi);
+   clear_bit(SELECTIVE_SUSPEND, &tp->flags);
} else {
tp->rtl_ops.up(tp);
netif_carrier_off(tp->netdev);
-- 
2.7.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

[PATCH net 0/4] r8152: fix scheduling napi

[Hayes Wang]

Scheduling the napi during the following periods would let it be ignored.
And the events wouldn't be handled until next napi_schedule() is called.

1. after napi_disable and before napi_enable().
2. after all actions of napi function is completed and before calling
   napi_complete().

If no next napi_schedule() is called, tx or rx would stop working.

In order to avoid these situations, the followings solutions are applied.

1. prevent start_xmit() from calling napi_schedule() during runtime suspend
   or after napi_disable().
2. re-schedule the napi for tx if it is necessary.
3. check if any rx is finished or not after napi_enable().

Hayes Wang (4):
  r8152: avoid start_xmit to call napi_schedule during autosuspend
  r8152: avoid start_xmit to schedule napi when napi is disabled
  r8152: re-schedule napi for tx
  r8152: check rx after napi is enabled

 drivers/net/usb/r8152.c | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

-- 
2.7.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

[PATCH net 4/4] r8152: check rx after napi is enabled

[Hayes Wang]

Schedule the napi after napi_enable() for rx, if it is necessary.

If the rx is completed when napi is disabled, the sheduling of napi
would be lost. Then, no one handles the rx packet until next napi
is scheduled.

Signed-off-by: Hayes Wang 
---
 drivers/net/usb/r8152.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index f65109b..f0f55b3 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -32,7 +32,7 @@
 #define NETNEXT_VERSION"08"
 
 /* Information for net */
-#define NET_VERSION"7"
+#define NET_VERSION"8"
 
 #define DRIVER_VERSION "v1." NETNEXT_VERSION "." NET_VERSION
 #define DRIVER_AUTHOR "Realtek linux nic maintainers "
@@ -3561,6 +3561,9 @@ static int rtl8152_post_reset(struct usb_interface *intf)
netif_wake_queue(netdev);
usb_submit_urb(tp->intr_urb, GFP_KERNEL);
 
+   if (!list_empty(&tp->rx_done))
+   napi_schedule(&tp->napi);
+
return 0;
 }
 
@@ -3696,6 +3699,8 @@ static int rtl8152_resume(struct usb_interface *intf)
rtl_start_rx(tp);
napi_enable(&tp->napi);
clear_bit(SELECTIVE_SUSPEND, &tp->flags);
+   if (!list_empty(&tp->rx_done))
+   napi_schedule(&tp->napi);
} else {
tp->rtl_ops.up(tp);
netif_carrier_off(tp->netdev);
-- 
2.7.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

[PATCH net 3/4] r8152: re-schedule napi for tx

[Hayes Wang]

Re-schedule napi after napi_complete() for tx, if it is necessay.

In r8152_poll(), if the tx is completed after tx_bottom() and before
napi_complete(), the scheduling of napi would be lost. Then, no
one handles the next tx until the next napi_schedule() is called.

Signed-off-by: Hayes Wang 
---
 drivers/net/usb/r8152.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 3454238..f65109b 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1936,6 +1936,9 @@ static int r8152_poll(struct napi_struct *napi, int 
budget)
napi_complete(napi);
if (!list_empty(&tp->rx_done))
napi_schedule(napi);
+   else if (!skb_queue_empty(&tp->tx_queue) &&
+!list_empty(&tp->tx_free))
+   napi_schedule(&tp->napi);
}
 
return work_done;
-- 
2.7.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

[PATCH net 2/4] r8152: avoid start_xmit to schedule napi when napi is disabled

[Hayes Wang]

Stop the tx when the napi is disabled to prevent napi_schedule() is
called.

Signed-off-by: Hayes Wang 
---
 drivers/net/usb/r8152.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 27b0b44..3454238 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -3155,10 +3155,13 @@ static void set_carrier(struct r8152 *tp)
if (!netif_carrier_ok(netdev)) {
tp->rtl_ops.enable(tp);
set_bit(RTL8152_SET_RX_MODE, &tp->flags);
+   netif_stop_queue(netdev);
napi_disable(&tp->napi);
netif_carrier_on(netdev);
rtl_start_rx(tp);
napi_enable(&tp->napi);
+   netif_wake_queue(netdev);
+   netif_info(tp, link, netdev, "carrier on\n");
}
} else {
if (netif_carrier_ok(netdev)) {
@@ -3166,6 +3169,7 @@ static void set_carrier(struct r8152 *tp)
napi_disable(&tp->napi);
tp->rtl_ops.disable(tp);
napi_enable(&tp->napi);
+   netif_info(tp, link, netdev, "carrier off\n");
}
}
 }
@@ -3515,12 +3519,12 @@ static int rtl8152_pre_reset(struct usb_interface *intf)
if (!netif_running(netdev))
return 0;
 
+   netif_stop_queue(netdev);
napi_disable(&tp->napi);
clear_bit(WORK_ENABLE, &tp->flags);
usb_kill_urb(tp->intr_urb);
cancel_delayed_work_sync(&tp->schedule);
if (netif_carrier_ok(netdev)) {
-   netif_stop_queue(netdev);
mutex_lock(&tp->control);
tp->rtl_ops.disable(tp);
mutex_unlock(&tp->control);
@@ -3548,10 +3552,10 @@ static int rtl8152_post_reset(struct usb_interface 
*intf)
rtl_start_rx(tp);
rtl8152_set_rx_mode(netdev);
mutex_unlock(&tp->control);
-   netif_wake_queue(netdev);
}
 
napi_enable(&tp->napi);
+   netif_wake_queue(netdev);
usb_submit_urb(tp->intr_urb, GFP_KERNEL);
 
return 0;
-- 
2.7.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: functionfs on dwc3, xhci host: endpoint cannot be used in both directions ?

[Vincent Pelletier]

Hello,

On Tue, 24 Jan 2017 11:02:10 +0200, Felipe Balbi
 wrote:
> that's correct. Maybe I should always set bulk capability. Thanks for
> catching that, I'll send v2 shortly.

I just tested v2 from your branch, but the original "else" contains
several inits which are now skipped for IN endpoints, making them
unavailable and causing a NULL pointer dereference in
dwc3_ep_trb_ring_show.

Replacing the original usb_ep_set_maxpacket_limit call with:
  int size;
  if (direction) {
...
size /= num;
  }
  else {
size = 1024;
  }
  usb_ep_set_maxpacket_limit(&dep->endpoint, size);
seems to do the trick.

Also, I noticed there are two (consistent) macros to extract MDWIDTH,
each used once:
core.h:#define DWC3_GHWPARAMS0_MDWIDTH(n)   (((n) >> 8) & 0xff)
core.h:#define DWC3_MDWIDTH(n)  (((n) & 0xff00) >> 8)
Neither a bug nor new, but I thought I should mention it.

Regards,
-- 
Vincent Pelletier


pgptUQkJbqZpi.pgp
Description: Signature digitale OpenPGP

Re: [PATCH 2/6] usb: mtu3: add reference clock

[Matthias Brugger]

On 01/20/2017 03:20 AM, Chunfeng Yun wrote:

On Thu, 2017-01-19 at 13:22 +0100, Matthias Brugger wrote:


On 18/01/17 07:08, Chunfeng Yun wrote:

usually, the reference clock comes from 26M oscillator directly,
but some SoCs are not, add it for compatibility.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/mtu3/mtu3.h  |1 +
 drivers/usb/mtu3/mtu3_plat.c |   21 +++--
 2 files changed, 20 insertions(+), 2 deletions(-)

[...]

@@ -154,6 +162,7 @@ static int ssusb_rscs_init(struct ssusb_mtk *ssusb)
 static void ssusb_rscs_exit(struct ssusb_mtk *ssusb)
 {
clk_disable_unprepare(ssusb->sys_clk);
+   clk_disable_unprepare(ssusb->ref_clk);
regulator_disable(ssusb->vusb33);
ssusb_phy_power_off(ssusb);
ssusb_phy_exit(ssusb);
@@ -216,6 +225,12 @@ static int get_ssusb_rscs(struct platform_device *pdev, 
struct ssusb_mtk *ssusb)
return PTR_ERR(ssusb->sys_clk);
}

+   ssusb->ref_clk = devm_clk_get(dev, "ref_ck");
+   if (IS_ERR(ssusb->ref_clk)) {
+   dev_err(dev, "failed to get ref clock\n");
+   return PTR_ERR(ssusb->ref_clk);
+   }
+


That would break older dts bindings, right?

Yes, So I send a new patch for the related dts. Maybe it's not a
problem, only one dts file need be updated currently.


ref_ck must be optional for the code.

I tend to make it be optional for the dts, but not for the code.
There are some "fixed-clock" which can be treated as dummy ones, and if
a clock is really optional, we can use one fixed-clock in dts, and keep
the code simple.
In fact, the reference clock is essential for usb controller.


Well the thing is that there are devices in the field with an older dtb 
which would break on a newer kernel. That's why we need to make it work 
with the old dtb in the code as well.


Regards,
Matthias



Regards,
Matthias




--
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] usb: dwc3: handle DWC_USB3_NUM == DWC_USB3_NUM_IN_EPS

[Bryan O'Donoghue]

On 24/01/17 19:25, John Youn wrote:

On 1/24/2017 3:05 AM, Bryan O'Donoghue wrote:

On 23/01/17 22:34, John Youn wrote:

On 1/23/2017 2:10 PM, Alan Stern wrote:

On Mon, 23 Jan 2017, John Youn wrote:


On 1/22/2017 5:29 PM, Bryan O'Donoghue wrote:

- DWC_USB3_NUM indicates the number of Device mode single directional
  endpoints, including OUT and IN endpoint 0.

- DWC_USB3_NUM_IN_EPS indicates the maximum number of Device mode IN
  endpoints active at any time, including control endpoint 0.

It's possible to configure RTL such that DWC_USB3_NUM_EPS is equal to
DWC_USB3_NUM_IN_EPS.

dwc3-core calculates the number of OUT endpoints as DWC_USB3_NUM minus
DWC_USB3_NUM_IN_EPS. If RTL has been configured with DWC_USB3_NUM_IN_EPS
equal to DWC_USB3_NUM then dwc3-core will calculate the number of OUT
endpoints as zero.

For example a from dwc3_core_num_eps() shows:
[1.565000]  /usb0@f01d: found 8 IN and 0 OUT endpoints

This patch works around this case by detecting when DWC_USB3_NUM_EPS is
equal to DWC3_USB3_NUM_IN_EPS and over-rides the calculation of the number


What if NUM_IN_EPS=7 and NUM_EPS=8? You will still have a problem.

It's possible to fix this for the general case rather than for this
specific case.


What is the reason for computing NUM_OUT_EPS in the first place?
Isn't it true that any endpoint can be used as an OUT endpoint?

So the real restrictions on a configuration are:

number of IN endpoints <= NUM_IN_EPS, and

number of IN endpoints + number of OUT endpoints <= NUM_EPS,

where ep0 must be counted twice, as both an IN and an OUT endpoint.
The value of NUM_OUT_EPS isn't used and shouldn't matter.



Yes that's correct. The general fix should take all that into account
so that any combination of NUM_EPS and NUM_IN_EPS will work fine.

However it must also account for FPGA configs where each HW endpoint
has a fixed number/direction, which the current code is compatible
with.


I'm not scanning something here..

If FPGA configurations can have fixed endpoint directions then you could
conceivably have a control IN/OUT pair plus a number a fixed
configuration of all IN or all OUT endpoints, even checking
GHWPARAMS6.EN_FPGA wouldn't let you know which direction those endpoints
had been configured to.

If I understand you correctly an FPGA could have pretty much any bit-map
of fixed endpoint directions - so just calculating the number of IN/OUT
endpoints based on some combination of DWC_USB3_NUM_IN_EPS and
DWC_USB3_NUM_EPS wouldn't inform you of the fixed direction of an
individual endpoint ... You'd need a descriptor specifying the fixed
direction of each endpoint right ?



Hi Bryan,

The EP num and direction are fixed in a predefined way.


Fair enough.

See

DWC_USB3_EN_LOG_PHYS_EP_SUPT in the databook and my earlier reply to
Felipe.


Thanks, I'll take a look tomorrow when I have the databook in front of me.


Unfortunately GHWPARAMS6.EN_FPGA doesn't directly tell you that
DWC_USB3_EN_LOG_PHYS_EP_SUPT is disabled. We just have to make that
assumption. Or add a quirk.


So, if I'm understanding the conclusion.

1. We'll go ahead and make a change to just grab the # of endpoints
   I'll send out that patch once its tested.
2. For 4.12 Filipe will do something for the # of IN endpoints
3. A quirk may eventually be required for DWC_USB3_EN_LOG_PHYS_EP_SUPT
   but lets no do anything on that unless someone actually complains.

---
bod
--
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: [PATCHv15 2/3] usb: USB Type-C connector class

[Guenter Roeck]

On Mon, Jan 16, 2017 at 05:56:13PM +0300, Heikki Krogerus wrote:
> The purpose of USB Type-C connector class is to provide
> unified interface for the user space to get the status and
> basic information about USB Type-C connectors on a system,
> control over data role swapping, and when the port supports
> USB Power Delivery, also control over power role swapping
> and Alternate Modes.
> 
> Signed-off-by: Heikki Krogerus 
> Reviewed-by: Mika Westerberg 
> ---
>  Documentation/ABI/testing/sysfs-class-typec |  289 ++
>  Documentation/usb/typec.rst |  185 
>  MAINTAINERS |9 +
>  drivers/usb/Kconfig |2 +
>  drivers/usb/Makefile|2 +
>  drivers/usb/typec/Kconfig   |7 +
>  drivers/usb/typec/Makefile  |1 +
>  drivers/usb/typec/typec.c   | 1263 
> +++
>  include/linux/usb/typec.h   |  243 ++
>  9 files changed, 2001 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-typec
>  create mode 100644 Documentation/usb/typec.rst
>  create mode 100644 drivers/usb/typec/Kconfig
>  create mode 100644 drivers/usb/typec/Makefile
>  create mode 100644 drivers/usb/typec/typec.c
>  create mode 100644 include/linux/usb/typec.h
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-typec 
> b/Documentation/ABI/testing/sysfs-class-typec
> new file mode 100644
> index ..09eb439246b0
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -0,0 +1,289 @@
> +USB Type-C port devices (eg. /sys/class/typec/port0/)
> +
> +What:/sys/class/typec//data_role
> +Date:February 2017
> +Contact: Heikki Krogerus 
> +Description:
> + The supported USB data roles. This attribute can be used for
> + requesting data role swapping on the port. Swapping is supported
> + as synchronous operation, so write(2) to the attribute will not
> + return until the operation has finished. The attribute is
> + notified about role changes so that poll(2) on the attribute
> + wakes up. Change on the role will also generate uevent
> + KOBJ_CHANGE on the port. The current role is show in brackets,
> + for example "[host] device" when DRP port is in host mode.
> +
> + Valid values:
> + - host
> + - device
> +
> +What:/sys/class/typec//power_role
> +Date:February 2017
> +Contact: Heikki Krogerus 
> +Description:
> + The supported power roles. This attribute can be used to request
> + power role swap on the port when the port supports USB Power
> + Delivery. Swapping is supported as synchronous operation, so
> + write(2) to the attribute will not return until the operation
> + has finished. The attribute is notified about role changes so
> + that poll(2) on the attribute wakes up. Change on the role will
> + also generate uevent KOBJ_CHANGE. The current role is show in
> + brackets, for example "[source] sink" when in source mode.
> +
> + Valid values:
> + - source
> + - sink
> +
> +What:/sys/class/typec//vconn_source
> +Date:February 2017
> +Contact: Heikki Krogerus 
> +Description:
> + Shows is the port VCONN Source. This attribute can be used to
> + request VCONN swap to change the VCONN Source during connection
> + when both the port and the partner support USB Power Delivery.
> + Swapping is supported as synchronous operation, so write(2) to
> + the attribute will not return until the operation has finished.
> + The attribute is notified about VCONN source changes so that
> + poll(2) on the attribute wakes up. Change on VCONN source also
> + generates uevent KOBJ_CHANGE.
> +
> + Valid values are:
> + - 0 when the port is not the VCONN Source
> + - 1 when the port is the VCONN Source
> +
> +What:/sys/class/typec//power_operation_mode
> +Date:February 2017
> +Contact: Heikki Krogerus 
> +Description:
> + Shows the current power operational mode the port is in. The
> + power operation mode means current level for VBUS. In case USB
> + Power Delivery communication is used for negotiating the levels,
> + power operation mode should show "usb_power_delivery".
> +
> + Valid values:
> + - default
> + - 1.5A
> + - 3.0A
> + - usb_power_delivery
> +
> +What:/sys/class/typec//preferred_role
> +Date:February 2017
> +Contact: Heikki Krogerus 
> +Des

Re: [RESENT PATCH net,stable] qmi_wwan/cdc_ether: add device ID for HP lt2523 (Novatel E371) WWAN card

[David Miller]

From: Bjørn Mork 
Date: Tue, 24 Jan 2017 10:45:38 +0100

> Another rebranded Novatel E371.  qmi_wwan should drive this device, while
> cdc_ether should ignore it.  Even though the USB descriptors are plain
> CDC-ETHER that USB interface is a QMI interface.  Ref commit 7fdb7846c9ca
> ("qmi_wwan/cdc_ether: add device IDs for Dell 5804 (Novatel E371) WWAN
> card")
> 
> Cc: Dan Williams 
> Signed-off-by: Bjørn Mork 

Applied.
--
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: storage: sddr09: Remove a set-but-not-used variable

[Alan Stern]

On Tue, 24 Jan 2017, Augusto Mecking Caringi wrote:

> The 'isnew' variable in 'sddr09_write_lba' function is set but never
> used.
> 
> This has been detected by building the driver with W=1:
> 
> drivers/usb/storage/sddr09.c: In function ‘sddr09_write_lba’:
> drivers/usb/storage/sddr09.c:873:17: warning: variable ‘isnew’ set but
> not used [-Wunused-but-set-variable]
> int i, result, isnew;
>  ^
> 
> Signed-off-by: Augusto Mecking Caringi 
> ---
>  drivers/usb/storage/sddr09.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/storage/sddr09.c b/drivers/usb/storage/sddr09.c
> index 3aeaa53..44f8ffc 100644
> --- a/drivers/usb/storage/sddr09.c
> +++ b/drivers/usb/storage/sddr09.c
> @@ -870,13 +870,12 @@ sddr09_write_lba(struct us_data *us, unsigned int lba,
>   unsigned int pagelen;
>   unsigned char *bptr, *cptr, *xptr;
>   unsigned char ecc[3];
> - int i, result, isnew;
> + int i, result;
>  
>   lbap = ((lba % 1000) << 1) | 0x1000;
>   if (parity[MSB_of(lbap) ^ LSB_of(lbap)])
>   lbap ^= 1;
>   pba = info->lba_to_pba[lba];
> - isnew = 0;
>  
>   if (pba == UNDEF) {
>   pba = sddr09_find_unused_pba(info, lba);
> @@ -887,7 +886,6 @@ sddr09_write_lba(struct us_data *us, unsigned int lba,
>   }
>   info->pba_to_lba[pba] = lba;
>   info->lba_to_pba[lba] = pba;
> - isnew = 1;
>   }
>  
>   if (pba == 1) {

Acked-by: 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 v2] usb: dwc3: handle DWC_USB3_NUM == DWC_USB3_NUM_IN_EPS

[John Youn]

On 1/24/2017 11:39 AM, Felipe Balbi wrote:
> Hi,
>
> (sorry if formatting sucks, using web interface)
>
> On Tue, Jan 24, 2017 at 9:21 PM John Youn  > wrote:
>
> On 1/24/2017 12:53 AM, Felipe Balbi wrote:
> >
> > Hi,
> >
> > John Youn mailto:john.y...@synopsys.com>>
> writes:
>  Bryan O'Donoghue  > writes:
> > - DWC_USB3_NUM indicates the number of Device mode single
> directional
> >   endpoints, including OUT and IN endpoint 0.
> >
> > - DWC_USB3_NUM_IN_EPS indicates the maximum number of Device
> mode IN
> >   endpoints active at any time, including control endpoint 0.
> >
> > It's possible to configure RTL such that DWC_USB3_NUM_EPS is
> equal to
> > DWC_USB3_NUM_IN_EPS.
> >
> > dwc3-core calculates the number of OUT endpoints as
> DWC_USB3_NUM minus
> > DWC_USB3_NUM_IN_EPS. If RTL has been configured with
> DWC_USB3_NUM_IN_EPS
> > equal to DWC_USB3_NUM then dwc3-core will calculate the number
> of OUT
> > endpoints as zero.
> >
> > For example a from dwc3_core_num_eps() shows:
> > [1.565000]  /usb0@f01d: found 8 IN and 0 OUT endpoints
> >
> > This patch works around this case by detecting when
> DWC_USB3_NUM_EPS is
> > equal to DWC3_USB3_NUM_IN_EPS and over-rides the calculation
> of the number
> > of OUT and IN endpoints to make dwc->num_in_eps equal to half
> > DWC_USB3_NUM_EPS.
> >
> > If DWC_USB3_NUM_EPS is equal to DWC3_USB3_NUM_IN_EPS and the
> endpoint count
> > is an odd number then dwc->num_out_eps will be assigned the
> extra endpoint.
> 
>  sorry, now that I spent some more time with this. Isn't
> something like
>  below solving all problems?
> 
>  diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>  index 369bab16a824..68c9c84b7216 100644
>  --- a/drivers/usb/dwc3/core.c
>  +++ b/drivers/usb/dwc3/core.c
>  @@ -397,8 +397,7 @@ static void dwc3_core_num_eps(struct dwc3 *dwc)
>   {
> struct dwc3_hwparams*parms = &dwc->hwparams;
> 
>  -  dwc->num_in_eps = DWC3_NUM_IN_EPS(parms);
>  -  dwc->num_out_eps = DWC3_NUM_EPS(parms) - dwc->num_in_eps;
>  +  dwc->num_eps = DWC3_NUM_EPS(parms);
>   }
> 
>   static void dwc3_cache_hwparams(struct dwc3 *dwc)
>  diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>  index 0a664d8eba3f..8c187df0aa42 100644
>  --- a/drivers/usb/dwc3/gadget.c
>  +++ b/drivers/usb/dwc3/gadget.c
>  @@ -2001,23 +2002,7 @@ static int
> dwc3_gadget_init_hw_endpoints(struct dwc3 *dwc,
> 
>   static int dwc3_gadget_init_endpoints(struct dwc3 *dwc)
>   {
>  -  int ret;
>  -
>  -  INIT_LIST_HEAD(&dwc->gadget.ep_list);
>  -
>  -  ret = dwc3_gadget_init_hw_endpoints(dwc, dwc->num_out_eps, 0);
>  -  if (ret < 0) {
>  -  dev_err(dwc->dev, "failed to initialize OUT
> endpoints\n");
>  -  return ret;
>  -  }
>  -
>  -  ret = dwc3_gadget_init_hw_endpoints(dwc, dwc->num_in_eps, 1);
>  -  if (ret < 0) {
>  -  dev_err(dwc->dev, "failed to initialize IN
> endpoints\n");
>  -  return ret;
>  -  }
>  -
>  -  return 0;
>  +  return dwc3_gadget_init_hw_endpoints(dwc, dwc->num_eps);
> >>>
> >>> Well I hadn't considered that level of change myself but, it
> should work.
> >>>
> >>
> >> It should work for ASICS.
> >>
> >> But it won't work for our FPGA platform. It needs to work the
> >> existing, more limited, way. You can check this with the
> >> GHWPARAMS6.EN_FPGA.
> >
> > Can you clarify a little more? Why wouldn't above work for FPGA?
> >
>
> Actually my mistake, this should still work fine as long as you are
> predefining the number and direction the same as before.
>
> See DWC_USB3_EN_LOG_PHYS_EP_SUPT for the FPGA configuration.
>
> This configuration should be enabled (the default), but can be
> disabled for FPGA configs to meet timing and save space. Looks like
> DWC3 driver is currently programming as if this is always disabled,
> which should be fine, it will just be more limiting than necessary.
>
>
> We always map endpoints as if that option was disabled. We gain nothing
> at all by mapping physical ep 30 to USB ep 1 OUT. I'd say this is rather
> pointless :-) We might need some more flexible endpoint mapping, but
> should we really care about that now? We haven't thus far and nobody
> complained. Perhaps we should add a

Re: [PATCH v3 00/24] usb: dwc2: Rework params, FIFO, and other changes

[John Youn]

On 1/23/2017 2:52 PM, John Youn wrote:
> Hi Felipe,
>
> This is a consolidation, rebase, and resend of several dwc2 patch
> series that were submitted to this list in the past month or so. See
> v2 [1] of this series for a summary.
>
> This latest revision is rebased to apply cleanly to your latest
> testing/next.
>
> This series also rebases the FIFO series from Sevak [2]. So there's no
> need to pick that up separately.
>
> The first 4 patches revert commits that were mistakenly added to your
> next branch.
>
> Regards,
> John
>
> [1] https://www.spinics.net/lists/linux-usb/msg152258.html
> [2] https://www.spinics.net/lists/linux-usb/msg152322.html
>


Hi Felipe,

Thanks for adding to your testing/next. It looks good so far.

Can you rebase it on -rc5 or later? There might be merge conflicts
otherwise

Thanks,
John
--
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] usb: dwc3: handle DWC_USB3_NUM == DWC_USB3_NUM_IN_EPS

[John Youn]

On 1/24/2017 3:05 AM, Bryan O'Donoghue wrote:
> On 23/01/17 22:34, John Youn wrote:
>> On 1/23/2017 2:10 PM, Alan Stern wrote:
>>> On Mon, 23 Jan 2017, John Youn wrote:
>>>
 On 1/22/2017 5:29 PM, Bryan O'Donoghue wrote:
> - DWC_USB3_NUM indicates the number of Device mode single directional
>   endpoints, including OUT and IN endpoint 0.
>
> - DWC_USB3_NUM_IN_EPS indicates the maximum number of Device mode IN
>   endpoints active at any time, including control endpoint 0.
>
> It's possible to configure RTL such that DWC_USB3_NUM_EPS is equal to
> DWC_USB3_NUM_IN_EPS.
>
> dwc3-core calculates the number of OUT endpoints as DWC_USB3_NUM minus
> DWC_USB3_NUM_IN_EPS. If RTL has been configured with DWC_USB3_NUM_IN_EPS
> equal to DWC_USB3_NUM then dwc3-core will calculate the number of OUT
> endpoints as zero.
>
> For example a from dwc3_core_num_eps() shows:
> [1.565000]  /usb0@f01d: found 8 IN and 0 OUT endpoints
>
> This patch works around this case by detecting when DWC_USB3_NUM_EPS is
> equal to DWC3_USB3_NUM_IN_EPS and over-rides the calculation of the number

 What if NUM_IN_EPS=7 and NUM_EPS=8? You will still have a problem.

 It's possible to fix this for the general case rather than for this
 specific case.
>>>
>>> What is the reason for computing NUM_OUT_EPS in the first place?
>>> Isn't it true that any endpoint can be used as an OUT endpoint?
>>>
>>> So the real restrictions on a configuration are:
>>>
>>> number of IN endpoints <= NUM_IN_EPS, and
>>>
>>> number of IN endpoints + number of OUT endpoints <= NUM_EPS,
>>>
>>> where ep0 must be counted twice, as both an IN and an OUT endpoint.
>>> The value of NUM_OUT_EPS isn't used and shouldn't matter.
>>>
>>
>> Yes that's correct. The general fix should take all that into account
>> so that any combination of NUM_EPS and NUM_IN_EPS will work fine.
>>
>> However it must also account for FPGA configs where each HW endpoint
>> has a fixed number/direction, which the current code is compatible
>> with.
>
> I'm not scanning something here..
>
> If FPGA configurations can have fixed endpoint directions then you could
> conceivably have a control IN/OUT pair plus a number a fixed
> configuration of all IN or all OUT endpoints, even checking
> GHWPARAMS6.EN_FPGA wouldn't let you know which direction those endpoints
> had been configured to.
>
> If I understand you correctly an FPGA could have pretty much any bit-map
> of fixed endpoint directions - so just calculating the number of IN/OUT
> endpoints based on some combination of DWC_USB3_NUM_IN_EPS and
> DWC_USB3_NUM_EPS wouldn't inform you of the fixed direction of an
> individual endpoint ... You'd need a descriptor specifying the fixed
> direction of each endpoint right ?
>

Hi Bryan,

The EP num and direction are fixed in a predefined way. See
DWC_USB3_EN_LOG_PHYS_EP_SUPT in the databook and my earlier reply to
Felipe.

Unfortunately GHWPARAMS6.EN_FPGA doesn't directly tell you that
DWC_USB3_EN_LOG_PHYS_EP_SUPT is disabled. We just have to make that
assumption. Or add a quirk.

Regards,
John
--
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] usb: musb: constify musb_hdrc_config structures

[Bhumika Goyal]

Declare musb_hdrc_config structures as const as they are only stored in
the config field of a musb_hdrc_platform_data structure. This field is of
type const, so musb_hdrc_config structures having this property can be
made const too.
Done using Coccinelle:

@r disable optional_qualifier@
identifier x;
position p;
@@
static struct musb_hdrc_config x@p={...};

@ok@
struct musb_hdrc_platform_data pdata;
identifier r.x;
position p;
@@
pdata.config=&x@p;

@bad@
position p != {r.p,ok.p};
identifier r.x;
@@
x@p

@depends on !bad disable optional_qualifier@
identifier r.x;
@@
+const
struct musb_hdrc_config x;

File size before:
   textdata bss dec hex filename
   1212 338   01550 60e drivers/usb/musb/jz4740.o

File size after:
   textdata bss dec hex filename
   1268 290   01558 616 drivers/usb/musb/jz4740.o

File size before:
   textdata bss dec hex filename
   6151 333  1665001964 drivers/usb/musb/sunxi.o

File size after:
   textdata bss dec hex filename
   6215 269  1665001964 drivers/usb/musb/sunxi.o

File size before:
   textdata bss dec hex filename
   3668 864   0453211b4 drivers/usb/musb/ux500.o

File size after:
   textdata bss dec hex filename
   3724 808   0453211b4 drivers/usb/musb/ux500.o

Signed-off-by: Bhumika Goyal 
---
 drivers/usb/musb/jz4740.c | 2 +-
 drivers/usb/musb/sunxi.c  | 2 +-
 drivers/usb/musb/ux500.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/musb/jz4740.c b/drivers/usb/musb/jz4740.c
index bc88899..40c68c2 100644
--- a/drivers/usb/musb/jz4740.c
+++ b/drivers/usb/musb/jz4740.c
@@ -63,7 +63,7 @@ static struct musb_fifo_cfg jz4740_musb_fifo_cfg[] = {
 { .hw_ep_num = 2, .style = FIFO_TX, .maxpacket = 64, },
 };
 
-static struct musb_hdrc_config jz4740_musb_config = {
+static const struct musb_hdrc_config jz4740_musb_config = {
/* Silicon does not implement USB OTG. */
.multipoint = 0,
/* Max EPs scanned, driver will decide which EP can be used. */
diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index d0be0ea..64545de 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -645,7 +645,7 @@ static struct musb_fifo_cfg sunxi_musb_mode_cfg[] = {
MUSB_EP_FIFO_SINGLE(5, FIFO_RX, 512),
 };
 
-static struct musb_hdrc_config sunxi_musb_hdrc_config = {
+static const struct musb_hdrc_config sunxi_musb_hdrc_config = {
.fifo_cfg   = sunxi_musb_mode_cfg,
.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg),
.multipoint = true,
diff --git a/drivers/usb/musb/ux500.c b/drivers/usb/musb/ux500.c
index 3eaa4ba..5a57250 100644
--- a/drivers/usb/musb/ux500.c
+++ b/drivers/usb/musb/ux500.c
@@ -30,7 +30,7 @@
 
 #include "musb_core.h"
 
-static struct musb_hdrc_config ux500_musb_hdrc_config = {
+static const struct musb_hdrc_config ux500_musb_hdrc_config = {
.multipoint = true,
.dyn_fifo   = true,
.num_eps= 16,
-- 
2.7.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: [PATCH v2] usb: dwc3: handle DWC_USB3_NUM == DWC_USB3_NUM_IN_EPS

[John Youn]

On 1/24/2017 12:53 AM, Felipe Balbi wrote:
>
> Hi,
>
> John Youn  writes:
 Bryan O'Donoghue  writes:
> - DWC_USB3_NUM indicates the number of Device mode single directional
>   endpoints, including OUT and IN endpoint 0.
>
> - DWC_USB3_NUM_IN_EPS indicates the maximum number of Device mode IN
>   endpoints active at any time, including control endpoint 0.
>
> It's possible to configure RTL such that DWC_USB3_NUM_EPS is equal to
> DWC_USB3_NUM_IN_EPS.
>
> dwc3-core calculates the number of OUT endpoints as DWC_USB3_NUM minus
> DWC_USB3_NUM_IN_EPS. If RTL has been configured with DWC_USB3_NUM_IN_EPS
> equal to DWC_USB3_NUM then dwc3-core will calculate the number of OUT
> endpoints as zero.
>
> For example a from dwc3_core_num_eps() shows:
> [1.565000]  /usb0@f01d: found 8 IN and 0 OUT endpoints
>
> This patch works around this case by detecting when DWC_USB3_NUM_EPS is
> equal to DWC3_USB3_NUM_IN_EPS and over-rides the calculation of the number
> of OUT and IN endpoints to make dwc->num_in_eps equal to half
> DWC_USB3_NUM_EPS.
>
> If DWC_USB3_NUM_EPS is equal to DWC3_USB3_NUM_IN_EPS and the endpoint 
> count
> is an odd number then dwc->num_out_eps will be assigned the extra 
> endpoint.

 sorry, now that I spent some more time with this. Isn't something like
 below solving all problems?

 diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
 index 369bab16a824..68c9c84b7216 100644
 --- a/drivers/usb/dwc3/core.c
 +++ b/drivers/usb/dwc3/core.c
 @@ -397,8 +397,7 @@ static void dwc3_core_num_eps(struct dwc3 *dwc)
  {
struct dwc3_hwparams*parms = &dwc->hwparams;

 -  dwc->num_in_eps = DWC3_NUM_IN_EPS(parms);
 -  dwc->num_out_eps = DWC3_NUM_EPS(parms) - dwc->num_in_eps;
 +  dwc->num_eps = DWC3_NUM_EPS(parms);
  }

  static void dwc3_cache_hwparams(struct dwc3 *dwc)
 diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
 index 0a664d8eba3f..8c187df0aa42 100644
 --- a/drivers/usb/dwc3/gadget.c
 +++ b/drivers/usb/dwc3/gadget.c
 @@ -2001,23 +2002,7 @@ static int dwc3_gadget_init_hw_endpoints(struct 
 dwc3 *dwc,

  static int dwc3_gadget_init_endpoints(struct dwc3 *dwc)
  {
 -  int ret;
 -
 -  INIT_LIST_HEAD(&dwc->gadget.ep_list);
 -
 -  ret = dwc3_gadget_init_hw_endpoints(dwc, dwc->num_out_eps, 0);
 -  if (ret < 0) {
 -  dev_err(dwc->dev, "failed to initialize OUT endpoints\n");
 -  return ret;
 -  }
 -
 -  ret = dwc3_gadget_init_hw_endpoints(dwc, dwc->num_in_eps, 1);
 -  if (ret < 0) {
 -  dev_err(dwc->dev, "failed to initialize IN endpoints\n");
 -  return ret;
 -  }
 -
 -  return 0;
 +  return dwc3_gadget_init_hw_endpoints(dwc, dwc->num_eps);
>>>
>>> Well I hadn't considered that level of change myself but, it should work.
>>>
>>
>> It should work for ASICS.
>>
>> But it won't work for our FPGA platform. It needs to work the
>> existing, more limited, way. You can check this with the
>> GHWPARAMS6.EN_FPGA.
>
> Can you clarify a little more? Why wouldn't above work for FPGA?
>

Actually my mistake, this should still work fine as long as you are
predefining the number and direction the same as before.

See DWC_USB3_EN_LOG_PHYS_EP_SUPT for the FPGA configuration.

This configuration should be enabled (the default), but can be
disabled for FPGA configs to meet timing and save space. Looks like
DWC3 driver is currently programming as if this is always disabled,
which should be fine, it will just be more limiting than necessary.

Regards,
John
--
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 24/24] usb: dwc2: gadget: Add checking for g-tx-fifo-size parameter

[John Youn]

On 1/24/2017 10:40 AM, Stefan Wahren wrote:
> Hi John,
>
> could you please push this series to the github synopsys-usb repo?
>
> I didn't have the chance to test combination of your last series.
>
> Stefan
>

Updated.

Regards,
John

--
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 24/24] usb: dwc2: gadget: Add checking for g-tx-fifo-size parameter

[Stefan Wahren]

Hi John,

could you please push this series to the github synopsys-usb repo?

I didn't have the chance to test combination of your last series.

Stefan
--
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: gadget: Kernel panic (NULL pointer dereference) when using fsl_udc2_core on i.MX31 PDK

[Magnus Lilja]

On 24 January 2017 at 19:34, Felipe Balbi  wrote:
>
> Hi,
>
> Magnus Lilja  writes:
>>> Magnus Lilja  writes:
> Magnus Lilja  writes:
 I tried the fsl_udc_core gadget driver on the i.MX31 PDK board and got 
 a
 kernel panic (NULL pointer dereference) when connecting the USB cable. 
 I
 had the g_serial module loaded as well.

 The NULL pointer panic comes from gadget/udc/core.c
 usb_gadget_giveback_request() which calls req->complete() and in some
 cases req->complete is NULL.

 Commit 304f7e5e1d08 ("usb: gadget: Refactor request completion") 
 changed
 fsl_udc2_core.c (and several other files) and in fsl_udc2_core.c a 
 check
 that req->complete is non-NULL was removed:

 --- a/drivers/usb/gadget/udc/fsl_udc_core.c
 +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
 @@ -197,10 +197,8 @@ __acquires(ep->udc->lock)
  ep->stopped = 1;

  spin_unlock(&ep->udc->lock);
 -   /* complete() is from gadget layer,
 -* eg fsg->bulk_in_complete() */
 -   if (req->req.complete)
 -   req->req.complete(&ep->ep, &req->req);
 +
 +   usb_gadget_giveback_request(&ep->ep, &req->req);

  spin_lock(&ep->udc->lock);
  ep->stopped = stopped;

 If I re-introduce the check (either in fsl_udc_core.c or core.c) at
 least USB gadget operation using g_serial seems to work just fine.

 I don't know the logic in detail to understand whether this is a proper
 fix or if there is some other more problem with the fls_udc_core 
 driver.
 Does anyone have input in this matter?

 I can produce a proper patch that fixes this problem by re-introducing
 the check (in either fsl_udc_core.c or core.c) if that is a proper
 solution and I can also assist in testing other fixes to the problem.
>>>
>>> ->complete() is supposed to be mandatory. Which gadget do you have that
>>> ->doesn't set ->complete() to a valid function pointer?
>>
>> I'm modprobing g_serial so the following modules are loaded (using my 
>> patch):
>>
>> ~ # lsmod
>> usb_f_acm
>> u_serial
>> g_serial
>> libcomposite
>> configfs
>> fsl_usb2_udc
>
> okay, can you figure out which request is coming without ->complete()
> set? To which endpoint is this request being queued? It would be nice to
> know these details. Maybe this is an old bug which ought to be fixed.

 Sure, I can try figure that out. Any input to make the debug of the
 faster is appreciated if you have any.
>>>
>>> well, the easiest way is to add something like:
>>>
>>> if (!req->complete)
>>> dump_stack();
>>>
>>> to fsl udc driver. Then you would know who queued the request without
>>> ->complete. A slightly better approach would be to:
>>>
>>> if (WARN(!req->complete,
>>> "%s: queueing request without ->complete\n", ep->name))
>>> return;
>>>
>>> Or something like that.
>>
>> Well, I think I found it.
>>
>> fsl_udc_core.c:ep0_prime_status() sets req->req.complete = NULL before
>> it queues a transfer and my printk()'s indicate that this is indeed
>> the offending function.
>>
>> fsl_udc_core.c:ch9getstatus() also sets complete to NULL but in my
>> tests right now I haven't seen that one.
>>
>> So it's an internal problem in the fsl_udc_core.c file.
>
> seems like it. It's rather odd that fsl_udc doesn't wanna know about
> completion of Status stage. Oh well, I guess in this case it doesn't
> matter if you add a complete function or reinstate the previous check
> for valid complete.
>
> If you decide to reinstate the check, please add a comment above the
> check explaining that fsl_udc itself queues requests with NULL
> ->complete().
>
> I must say, however, that I would suggest adding a complete callback
> since that will help us BUG with NULL pointer deref on bad gadget
> drivers ;-)

I can do that. Such a complete() callback function would be a no-op
then I assume (with a comment in it why it is a no-op).

Regards, Magnus
--
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] r8152: don't execute runtime suspend if the tx is not empty

[David Miller]

From: Hayes Wang 
Date: Mon, 23 Jan 2017 14:18:43 +0800

> Runtime suspend shouldn't be executed if the tx queue is not empty,
> because the device is not idle.
> 
> Signed-off-by: Hayes Wang 

Applied and queued up for -stable, 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: usb: gadget: Kernel panic (NULL pointer dereference) when using fsl_udc2_core on i.MX31 PDK

[Felipe Balbi]

Hi,

Magnus Lilja  writes:
>> Magnus Lilja  writes:
 Magnus Lilja  writes:
>>> I tried the fsl_udc_core gadget driver on the i.MX31 PDK board and got a
>>> kernel panic (NULL pointer dereference) when connecting the USB cable. I
>>> had the g_serial module loaded as well.
>>>
>>> The NULL pointer panic comes from gadget/udc/core.c
>>> usb_gadget_giveback_request() which calls req->complete() and in some
>>> cases req->complete is NULL.
>>>
>>> Commit 304f7e5e1d08 ("usb: gadget: Refactor request completion") changed
>>> fsl_udc2_core.c (and several other files) and in fsl_udc2_core.c a check
>>> that req->complete is non-NULL was removed:
>>>
>>> --- a/drivers/usb/gadget/udc/fsl_udc_core.c
>>> +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
>>> @@ -197,10 +197,8 @@ __acquires(ep->udc->lock)
>>>  ep->stopped = 1;
>>>
>>>  spin_unlock(&ep->udc->lock);
>>> -   /* complete() is from gadget layer,
>>> -* eg fsg->bulk_in_complete() */
>>> -   if (req->req.complete)
>>> -   req->req.complete(&ep->ep, &req->req);
>>> +
>>> +   usb_gadget_giveback_request(&ep->ep, &req->req);
>>>
>>>  spin_lock(&ep->udc->lock);
>>>  ep->stopped = stopped;
>>>
>>> If I re-introduce the check (either in fsl_udc_core.c or core.c) at
>>> least USB gadget operation using g_serial seems to work just fine.
>>>
>>> I don't know the logic in detail to understand whether this is a proper
>>> fix or if there is some other more problem with the fls_udc_core driver.
>>> Does anyone have input in this matter?
>>>
>>> I can produce a proper patch that fixes this problem by re-introducing
>>> the check (in either fsl_udc_core.c or core.c) if that is a proper
>>> solution and I can also assist in testing other fixes to the problem.
>>
>> ->complete() is supposed to be mandatory. Which gadget do you have that
>> ->doesn't set ->complete() to a valid function pointer?
>
> I'm modprobing g_serial so the following modules are loaded (using my 
> patch):
>
> ~ # lsmod
> usb_f_acm
> u_serial
> g_serial
> libcomposite
> configfs
> fsl_usb2_udc

 okay, can you figure out which request is coming without ->complete()
 set? To which endpoint is this request being queued? It would be nice to
 know these details. Maybe this is an old bug which ought to be fixed.
>>>
>>> Sure, I can try figure that out. Any input to make the debug of the
>>> faster is appreciated if you have any.
>>
>> well, the easiest way is to add something like:
>>
>> if (!req->complete)
>> dump_stack();
>>
>> to fsl udc driver. Then you would know who queued the request without
>> ->complete. A slightly better approach would be to:
>>
>> if (WARN(!req->complete,
>> "%s: queueing request without ->complete\n", ep->name))
>> return;
>>
>> Or something like that.
>
> Well, I think I found it.
>
> fsl_udc_core.c:ep0_prime_status() sets req->req.complete = NULL before
> it queues a transfer and my printk()'s indicate that this is indeed
> the offending function.
>
> fsl_udc_core.c:ch9getstatus() also sets complete to NULL but in my
> tests right now I haven't seen that one.
>
> So it's an internal problem in the fsl_udc_core.c file.

seems like it. It's rather odd that fsl_udc doesn't wanna know about
completion of Status stage. Oh well, I guess in this case it doesn't
matter if you add a complete function or reinstate the previous check
for valid complete.

If you decide to reinstate the check, please add a comment above the
check explaining that fsl_udc itself queues requests with NULL
->complete().

I must say, however, that I would suggest adding a complete callback
since that will help us BUG with NULL pointer deref on bad gadget
drivers ;-)

-- 
balbi
--
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: gadget: Kernel panic (NULL pointer dereference) when using fsl_udc2_core on i.MX31 PDK

[Magnus Lilja]

Hi

On 24 January 2017 at 11:54, Felipe Balbi  wrote:
>
> Hi,
>
> Magnus Lilja  writes:
>>> Magnus Lilja  writes:
>> I tried the fsl_udc_core gadget driver on the i.MX31 PDK board and got a
>> kernel panic (NULL pointer dereference) when connecting the USB cable. I
>> had the g_serial module loaded as well.
>>
>> The NULL pointer panic comes from gadget/udc/core.c
>> usb_gadget_giveback_request() which calls req->complete() and in some
>> cases req->complete is NULL.
>>
>> Commit 304f7e5e1d08 ("usb: gadget: Refactor request completion") changed
>> fsl_udc2_core.c (and several other files) and in fsl_udc2_core.c a check
>> that req->complete is non-NULL was removed:
>>
>> --- a/drivers/usb/gadget/udc/fsl_udc_core.c
>> +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
>> @@ -197,10 +197,8 @@ __acquires(ep->udc->lock)
>>  ep->stopped = 1;
>>
>>  spin_unlock(&ep->udc->lock);
>> -   /* complete() is from gadget layer,
>> -* eg fsg->bulk_in_complete() */
>> -   if (req->req.complete)
>> -   req->req.complete(&ep->ep, &req->req);
>> +
>> +   usb_gadget_giveback_request(&ep->ep, &req->req);
>>
>>  spin_lock(&ep->udc->lock);
>>  ep->stopped = stopped;
>>
>> If I re-introduce the check (either in fsl_udc_core.c or core.c) at
>> least USB gadget operation using g_serial seems to work just fine.
>>
>> I don't know the logic in detail to understand whether this is a proper
>> fix or if there is some other more problem with the fls_udc_core driver.
>> Does anyone have input in this matter?
>>
>> I can produce a proper patch that fixes this problem by re-introducing
>> the check (in either fsl_udc_core.c or core.c) if that is a proper
>> solution and I can also assist in testing other fixes to the problem.
>
> ->complete() is supposed to be mandatory. Which gadget do you have that
> ->doesn't set ->complete() to a valid function pointer?

 I'm modprobing g_serial so the following modules are loaded (using my 
 patch):

 ~ # lsmod
 usb_f_acm
 u_serial
 g_serial
 libcomposite
 configfs
 fsl_usb2_udc
>>>
>>> okay, can you figure out which request is coming without ->complete()
>>> set? To which endpoint is this request being queued? It would be nice to
>>> know these details. Maybe this is an old bug which ought to be fixed.
>>
>> Sure, I can try figure that out. Any input to make the debug of the
>> faster is appreciated if you have any.
>
> well, the easiest way is to add something like:
>
> if (!req->complete)
> dump_stack();
>
> to fsl udc driver. Then you would know who queued the request without
> ->complete. A slightly better approach would be to:
>
> if (WARN(!req->complete,
> "%s: queueing request without ->complete\n", ep->name))
> return;
>
> Or something like that.

Well, I think I found it.

fsl_udc_core.c:ep0_prime_status() sets req->req.complete = NULL before
it queues a transfer and my printk()'s indicate that this is indeed
the offending function.

fsl_udc_core.c:ch9getstatus() also sets complete to NULL but in my
tests right now I haven't seen that one.

So it's an internal problem in the fsl_udc_core.c file.

Regards, Magnus¨
--
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] ARM: dts: Odroid XU4: fix USB3.0 ports

[Krzysztof Kozlowski]

On Tue, Jan 24, 2017 at 02:48:09PM +0100, Richard Genoud wrote:
> Since commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores"),
> the USB ports on odroid-XU4 don't work anymore.

Hi,

Thanks for the patch. Please:
1. Use recent kernel to test it, which could be one of: mainline
   (Linus'), recent linux-next or my for-next branch. Why am I thinking
   that you did not test it on thse? Because you sent the patch to
   k.kozlow...@samsung.com. The guy disappeared four months ago and
   recent kernel has all addresses updated (including mailmap).
2. Fix the title to match subsystem (git log --oneline 
arch/arm/boot/dts/exynos*).


> 
> Inserting an usb key (USB2.0) on the USB3.0 port result in:
> [   64.488264] xhci-hcd xhci-hcd.2.auto: Port resume took longer than 2 
> msec, port status = 0xc400fe3
> [   74.568156] xhci-hcd xhci-hcd.2.auto: xHCI host not responding to stop 
> endpoint command.
> [   74.574806] xhci-hcd xhci-hcd.2.auto: Assuming host is dying, halting host.
> [   74.601970] xhci-hcd xhci-hcd.2.auto: HC died; cleaning up
> [   74.606276] usb 3-1: USB disconnect, device number 2
> [   74.613565] usb 4-1: USB disconnect, device number 2
> [   74.621208] usb usb3-port1: couldn't allocate usb_device
> NB: it's not related to USB2.0 devices, I get the same result with an USB3.0 
> device (SATA to USB3 for instance).
> NB2: it doesn't happen on an odriod-XU3 board, that doesn't have the realtek 
> RTL8153 chip.

Odroid-XU3
s/realtek/Realtek/

However I do not think that Realtek matters here. The USB3 ports are
directly accessible on XU3. On XU4 on the other hand, the port USB3-0 is
connected to USB hub. I think the sentence about Realtek is then
misleading, so just mention the XU3.

> 
> If the lines:
>   if (dwc->revision > DWC3_REVISION_194A)
>   reg |= DWC3_GUSB3PIPECTL_SUSPHY;
> are commented out, the USB3.0 start working.

s/start/starts/

> 
> For a full analyse: https://lkml.org/lkml/2017/1/18/678

Documentation/process/submitting-patches.rst
Instead of this above (lkml.org is highly discouraged) use proper
https://lkml.kernel.org/ in "Link:" put next to other tags (look at
recent commits), however I do not find this link as necessary.  Just
describe here what is wrong and how you are going to fix it.

> 
> Felipe suggested that suspend should be disabled temporarily while
> what's wrong with DCW3 is figured out.
> 
> Tested on Odroid XU4
> 
> Suggested-by: Felipe Balbi 
> Tested-by: Richard Genoud 

Being an author implies testing. Please remove the tag.

> Signed-off-by: Richard Genoud 
> Cc: sta...@vger.kernel.org # 4.4+

Malformed tag - missing <>.

> Fixes: 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores")

No need for such long hash (2164a476205c).

I wonder about pointing the commit which introduced the issue. Usually
the fixes directly indicates how far we want the patch to be backported.
In this case this should be backported to kernel bringing XU4 DTS. The
commit which was not valid, was the commit adding DTS without this
property. 2164a476205c was innocent for XU4 because XU4 did not exist
that time.

Definitely something is wrong if Fixes tag does not match indicated
backport version.

> ---
>  arch/arm/boot/dts/exynos5422-odroidxu4.dts | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/exynos5422-odroidxu4.dts 
> b/arch/arm/boot/dts/exynos5422-odroidxu4.dts
> index 2faf88627a48..f62dbd9b5ad3 100644
> --- a/arch/arm/boot/dts/exynos5422-odroidxu4.dts
> +++ b/arch/arm/boot/dts/exynos5422-odroidxu4.dts
> @@ -43,6 +43,15 @@
>   status = "okay";
>  };
>  
> +&usbdrd_dwc3_0 {
> + /*
> +  * without that, usb devices are not recognized when
> +  * they are plugged.

s/without/Without/
s/usb/USB.

> +  * cf: https://lkml.org/lkml/2017/1/18/678

No external resources. You can extend a little bit the sentence above to
two sentences. Combined with "git log" this would be sufficient.

Best regards,
Krzysztof

> +  */
> + snps,dis_u3_susphy_quirk;
> +};
> +
>  &usbdrd_dwc3_1 {
>   dr_mode = "host";
>  };
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" 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/2] dt-bindings: usb: add DT binding for s3c2410 USB device controller

[Sergio Prado]

On Wed, Jan 18, 2017 at 02:08:45PM -0600, Rob Herring wrote:
> On Wed, Jan 11, 2017 at 08:02:29PM -0200, Sergio Prado wrote:
> > + - clocks: Should reference the bus and host clocks
> > + - clock-names: Should contain two strings
> > +   "uclk" for the USB bus clock
> > +   "usb-device" for the USB device clock
> 
> Perhaps just "bus" and "device".

OK, I'll rename.

> > +
> > +Optional properties:
> > + - samsung,vbus-gpio: specifies a gpio that allows to detect whether
> > +   vbus is present - USB is connected (active high, input).
> > + - samsung,pullup-gpio: If present, specifies a gpio to control the
> > +   USB D+ pullup (active high, output).
> 
> "-gpios", not "-gpio" is preferred.
> 
> These should be common property names if we're going to have them. The 
> problem with just "vbus-gpios" is does that mean an enable control or 
> status as you have. I guess in the former case, that should always be 
> modeled as a regulator.
> 
> Also, these should all be part of a connector node as these controls are 
> more related to the USB connector than the controller. And I don't mean 
> extcon here because those bindings are a mess.

There are other bindings that are doing the same thing I did, like the
property "atmel,vbus-gpio" in atmel-usb.txt and "samsung,vbus-gpio" in
exynos-usb.txt. Also, I did not find any example of a connector node doing
this. Can you point me out to an example, or should I just rename to
"-gpios" in this case?

> 
> Rob

-- 
Sergio Prado
Embedded Labworks
Office: +55 11 2628-3461
Mobile: +55 11 97123-3420
--
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: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget

[Felipe Balbi]

Hi,

Petr Cvek  writes:
>> Greg KH  writes:
>> fine by me. Just lsusb will look funky ;-)
>
> Heh, true, but I thought lsusb would use a string if the device provided
> it.  Haven't looked at that portion of the code in a very long time...
>

 My lsusb shows separate strings (using usbutils from slackware64-current):

 Bus 004 Device 003: ID 1d6b:0102 Linux Foundation EEM Gadget
 ...
   idVendor   0x1d6b Linux Foundation
   idProduct  0x0102 EEM Gadget
   bcdDevice4.07
   iManufacturer   1 Linux Foundation
   iProduct2 Webcam gadget
 ...
>>>
>>> Ah, I guess it doesn't, but who knows how old that version of usbutils
>>> is, considering the last release I did was well over a year ago.  I
>>> should do a new one one of these days...
>>>
>>> Anyway, I'd like to not assign a product id to a chunk of code that is
>>> going to be eventually deleted.  Felipe, what's the plan for the
>>> "legacy" gadget code.  Is it ever going away?
>> 
>> Well, I wasn't really planning on deleting them just stopped accepting
>> any new one. I wanted to avoid angry mobs complaining about not having a
>> g_mass_storage.ko anymore.
>> 
>> Personally, I don't feel strongly about the legacy gadget
>> drivers. They're not really needed anymore as everything they do can be
>> done with configfs already. Perhaps we could schedule their removal for
>> v5.0?
>> 
>
> If you want to remove legacy g_webcam then there should be a way to
> set its module parameters from somewhere (it seems I can't find it
> anywhere). For PXA27x especially "opts->streaming_maxpacket" from
> f_uvc.c is critical. Default max packet size in PXA27x UDC (but
> lowering the limit to 256 by g_webcam parameter works).

it should be part of the function's configfs interface. If it's not,
please send a patch. Have a look at
Documentation/usb/gadget_configfs.txt for more info.

-- 
balbi
--
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] usb: storage: sddr09: Remove a set-but-not-used variable

[Augusto Mecking Caringi]

The 'isnew' variable in 'sddr09_write_lba' function is set but never
used.

This has been detected by building the driver with W=1:

drivers/usb/storage/sddr09.c: In function ‘sddr09_write_lba’:
drivers/usb/storage/sddr09.c:873:17: warning: variable ‘isnew’ set but
not used [-Wunused-but-set-variable]
int i, result, isnew;
 ^

Signed-off-by: Augusto Mecking Caringi 
---
 drivers/usb/storage/sddr09.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/usb/storage/sddr09.c b/drivers/usb/storage/sddr09.c
index 3aeaa53..44f8ffc 100644
--- a/drivers/usb/storage/sddr09.c
+++ b/drivers/usb/storage/sddr09.c
@@ -870,13 +870,12 @@ sddr09_write_lba(struct us_data *us, unsigned int lba,
unsigned int pagelen;
unsigned char *bptr, *cptr, *xptr;
unsigned char ecc[3];
-   int i, result, isnew;
+   int i, result;
 
lbap = ((lba % 1000) << 1) | 0x1000;
if (parity[MSB_of(lbap) ^ LSB_of(lbap)])
lbap ^= 1;
pba = info->lba_to_pba[lba];
-   isnew = 0;
 
if (pba == UNDEF) {
pba = sddr09_find_unused_pba(info, lba);
@@ -887,7 +886,6 @@ sddr09_write_lba(struct us_data *us, unsigned int lba,
}
info->pba_to_lba[pba] = lba;
info->lba_to_pba[lba] = pba;
-   isnew = 1;
}
 
if (pba == 1) {
-- 
2.7.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: [PATCH v2, 6/6] dt-bindings: phy-mt65xx-usb: add support for new version phy

[Matthias Brugger]

Hi Chunfeng,

On 01/20/2017 09:18 AM, Chunfeng Yun wrote:

add a new compatible string for "mt2712", and move reference clock
into each port node;

Signed-off-by: Chunfeng Yun 
---
 .../devicetree/bindings/phy/phy-mt65xx-usb.txt |   91 +---
 1 file changed, 77 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt 
b/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt
index 33a2b1e..1d06604 100644
--- a/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt
+++ b/Documentation/devicetree/bindings/phy/phy-mt65xx-usb.txt
@@ -6,21 +6,27 @@ This binding describes a usb3.0 phy for mt65xx platforms of 
Medaitek SoC.
 Required properties (controller (parent) node):
  - compatible  : should be one of
  "mediatek,mt2701-u3phy"
+ "mediatek,mt2712-u3phy"
  "mediatek,mt8173-u3phy"
- - reg : offset and length of register for phy, exclude port's
- register.
- - clocks  : a list of phandle + clock-specifier pairs, one for each
- entry in clock-names
- - clock-names : must contain
- "u3phya_ref": for reference clock of usb3.0 analog phy.

 Required nodes : a sub-node is required for each port the controller
  provides. Address range information including the usual
  'reg' property is used inside these nodes to describe
  the controller's topology.

+Optional properties (controller (parent) node):
+ - reg : offset and length of register shared by multiple ports,
+ exclude port's private register. It is needed on mt2701
+ and mt8173, but not on mt2712.
+
 Required properties (port (child) node):
 - reg  : address and length of the register set for the port.
+- clocks   : a list of phandle + clock-specifier pairs, one for each
+ entry in clock-names
+- clock-names  : must contain
+ "ref_clk": 48M reference clock for HighSpeed analog phy; and
+   26M reference clock for SuperSpeed analog phy, 
sometimes is
+   24M, 25M or 27M, depended on platform.
 - #phy-cells   : should be 1 (See second example)
  cell after port phandle is phy type from:
- PHY_TYPE_USB2


The old bindings will need to be supported by the driver, they have to 
stay here with a comment that they are deprecated.


Regards,
Matthias


@@ -31,21 +37,31 @@ Example:
 u3phy: usb-phy@1129 {
compatible = "mediatek,mt8173-u3phy";
reg = <0 0x1129 0 0x800>;
-   clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>;
-   clock-names = "u3phya_ref";
#address-cells = <2>;
#size-cells = <2>;
ranges;
status = "okay";

-   phy_port0: port@11290800 {
-   reg = <0 0x11290800 0 0x800>;
+   u2port0: port@11290800 {
+   reg = <0 0x11290800 0 0x100>;
+   clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>;
+   clock-names = "ref_clk";
#phy-cells = <1>;
status = "okay";
};

-   phy_port1: port@11291000 {
-   reg = <0 0x11291000 0 0x800>;
+   u3port0: port@11290900 {
+   reg = <0 0x11290800 0 0x700>;
+   clocks = <&clk26m>;
+   clock-names = "ref_clk";
+   #phy-cells = <1>;
+   status = "okay";
+   };
+
+   u2port1: port@11291000 {
+   reg = <0 0x11291000 0 0x100>;
+   clocks = <&apmixedsys CLK_APMIXED_REF2USB_TX>;
+   clock-names = "ref_clk";
#phy-cells = <1>;
status = "okay";
};
@@ -64,7 +80,54 @@ Example:

 usb30: usb@1127 {
...
-   phys = <&phy_port0 PHY_TYPE_USB3>;
-   phy-names = "usb3-0";
+   phys = <&u2port0 PHY_TYPE_USB2>, <&u3port0 PHY_TYPE_USB3>;
+   phy-names = "usb2-0", "usb3-0";
...
 };
+
+
+Layout differences of banks between mt8173/mt2701 and mt2712
+-
+mt8173 and mt2701:
+portoffsetbank
+shared  0xSPLLC
+0x0100FMREG
+u2 port00x0800U2PHY_COM
+u3 port00x0900U3PHYD
+0x0a00U3PHYD_BANK2
+0x0b00U3PHYA
+0x0c00U3PHYA_DA
+u2 port10x1000U2PHY_COM
+u3 port10x1100U3PHYD
+0x1200U3PHYD_BANK2
+0x1300U3PHYA
+0x1400U3PHYA_DA
+u2 port20x1800U2PHY_COM
+...
+
+mt2712:
+portoffsetbank
+u2 port00xMISC
+0x0100FMREG
+0x0300U2PHY_COM
+u3 port00x0700SPLLC
+0x0800CHIP
+0x0900U3PHYD
+0x0a00U3PHYD_BANK2
+0x0b00U3PHYA
+0x0c00U3PHYA_DA
+u2 port10x1000MISC
+0x1100   

[PATCH 0/2] musb fixes for v4.10-rc6

[Bin Liu]

Hi Greg,

Here is musb fixes for v4.10-rc6, it is to fix a couple more regressions caused
by runtime PM in musb drivers.  Please let me know if any change is needed.

Regards,
-Bin.


Tony Lindgren (2):
  usb: musb: Fix host mode error -71 regression
  usb: musb: Fix external abort on non-linefetch for musb_irq_work()

 drivers/usb/musb/musb_core.c | 26 +-
 drivers/usb/musb/musb_core.h |  1 -
 2 files changed, 13 insertions(+), 14 deletions(-)

-- 
1.9.1

--
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 1/2] usb: musb: Fix host mode error -71 regression

[Bin Liu]

From: Tony Lindgren 

Commit 467d5c980709 ("usb: musb: Implement session bit based runtime PM for
musb-core") started implementing musb generic runtime PM support by
introducing devctl register session bit based state control.

This caused a regression where if a USB mass storage device is connected
to a USB hub, we can get:

usb 1-1: reset high-speed USB device number 2 using musb-hdrc
usb 1-1: device descriptor read/64, error -71
usb 1-1.1: new high-speed USB device number 4 using musb-hdrc

This is because before the USB storage device is connected, musb is
in OTG_STATE_A_SUSPEND. And we currently only set need_finish_resume
in musb_stage0_irq() and the related code calling finish_resume_work
in musb_resume() and musb_runtime_resume() never gets called.

To fix the issue, we can call schedule_delayed_work() directly in
musb_stage0_irq() to have finish_resume_work run.

And we should no longer never get interrupts when when suspended.
We have changed musb to no longer need pm_runtime_irqsafe().
The need_finish_resume flag was added in commit 9298b4aad37e ("usb:
musb: fix device hotplug behind hub") and no longer applies as far
as I can tell. So let's just remove the earlier code that no longer
is needed.

Fixes: 467d5c980709 ("usb: musb: Implement session bit based
runtime PM for musb-core")
Reported-by: Bin Liu 
Signed-off-by: Tony Lindgren 
Signed-off-by: Bin Liu 
---
 drivers/usb/musb/musb_core.c | 15 ++-
 drivers/usb/musb/musb_core.h |  1 -
 2 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index fca288bbc800..cd40467be3c5 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -594,11 +594,11 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 
int_usb,
| MUSB_PORT_STAT_RESUME;
musb->rh_timer = jiffies
+ msecs_to_jiffies(USB_RESUME_TIMEOUT);
-   musb->need_finish_resume = 1;
-
musb->xceiv->otg->state = OTG_STATE_A_HOST;
musb->is_active = 1;
musb_host_resume_root_hub(musb);
+   schedule_delayed_work(&musb->finish_resume_work,
+   msecs_to_jiffies(USB_RESUME_TIMEOUT));
break;
case OTG_STATE_B_WAIT_ACON:
musb->xceiv->otg->state = 
OTG_STATE_B_PERIPHERAL;
@@ -2710,11 +2710,6 @@ static int musb_resume(struct device *dev)
mask = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_FSDEV | MUSB_DEVCTL_LSDEV;
if ((devctl & mask) != (musb->context.devctl & mask))
musb->port1_status = 0;
-   if (musb->need_finish_resume) {
-   musb->need_finish_resume = 0;
-   schedule_delayed_work(&musb->finish_resume_work,
- msecs_to_jiffies(USB_RESUME_TIMEOUT));
-   }
 
/*
 * The USB HUB code expects the device to be in RPM_ACTIVE once it came
@@ -2766,12 +2761,6 @@ static int musb_runtime_resume(struct device *dev)
 
musb_restore_context(musb);
 
-   if (musb->need_finish_resume) {
-   musb->need_finish_resume = 0;
-   schedule_delayed_work(&musb->finish_resume_work,
-   msecs_to_jiffies(USB_RESUME_TIMEOUT));
-   }
-
spin_lock_irqsave(&musb->lock, flags);
error = musb_run_resume_work(musb);
if (error)
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index ade902ea1221..ce5a18c98c6d 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -410,7 +410,6 @@ struct musb {
 
/* is_suspended means USB B_PERIPHERAL suspend */
unsignedis_suspended:1;
-   unsignedneed_finish_resume :1;
 
/* may_wakeup means remote wakeup is enabled */
unsignedmay_wakeup:1;
-- 
1.9.1

--
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 2/2] usb: musb: Fix external abort on non-linefetch for musb_irq_work()

[Bin Liu]

From: Tony Lindgren 

While testing musb host mode cable plugging on a BeagleBone, I came across this
error:

Unhandled fault: external abort on non-linefetch (0x1008) at 0xd1dcfc60
...
[] (musb_default_readb [musb_hdrc]) from [] 
(musb_irq_work+0x1c/0x180 [musb_hdrc])
[] (musb_irq_work [musb_hdrc]) from [] 
(process_one_work+0x2b4/0x808)
[] (process_one_work) from [] (worker_thread+0x3c/0x550)
[] (worker_thread) from [] (kthread+0x104/0x148)
[] (kthread) from [] (ret_from_fork+0x14/0x24)

Signed-off-by: Tony Lindgren 
Signed-off-by: Bin Liu 
---
 drivers/usb/musb/musb_core.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index cd40467be3c5..772f15821242 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1925,6 +1925,14 @@ static void musb_pm_runtime_check_session(struct musb 
*musb)
 static void musb_irq_work(struct work_struct *data)
 {
struct musb *musb = container_of(data, struct musb, irq_work.work);
+   int error;
+
+   error = pm_runtime_get_sync(musb->controller);
+   if (error < 0) {
+   dev_err(musb->controller, "Could not enable: %i\n", error);
+
+   return;
+   }
 
musb_pm_runtime_check_session(musb);
 
@@ -1932,6 +1940,9 @@ static void musb_irq_work(struct work_struct *data)
musb->xceiv_old_state = musb->xceiv->otg->state;
sysfs_notify(&musb->controller->kobj, NULL, "mode");
}
+
+   pm_runtime_mark_last_busy(musb->controller);
+   pm_runtime_put_autosuspend(musb->controller);
 }
 
 static void musb_recover_from_babble(struct musb *musb)
-- 
1.9.1

--
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

[BUG] usb: gadget: Kernel oops with UVC USB gadget and configfs

[Petr Cvek]

Setting the UVC gadget with configfs and then reloading UDC controler driver
(pxa27x_udc) causes kernel to fail.

UDC subsystem was patched only in decreasing maxpacket size for UVC, addition
of more predefined endpoints for pxa27x_udc and addition of some debugging 
pr_info.

Practically same behaviour was observed on 4.7.0 too.

Script:

modprobe udc-core
modprobe pxa27x-udc
modprobe libcomposite
modprobe usb_f_uvc

mkdir -p /tmp/udc
cd /tmp/udc

mount none /tmp/udc -t configfs
mkdir -p usb_gadget/gcam

cd usb_gadget/gcam
mkdir -p configs/c.1

mkdir -p functions/uvc.usb0
mkdir -p strings/0x409
mkdir -p configs/c.1/strings/0x409

echo 0x1d6b > idVendor
echo 0x0102 > idProduct
echo 16 > bMaxPacketSize0
echo "Magician" > strings/0x409/serialnumber
echo "Linux Foundation" > strings/0x409/manufacturer
echo "Webcam/EEM" > strings/0x409/product
echo "Video" > configs/c.1/strings/0x409/configuration
echo 239 > bDeviceClass
echo 2 > bDeviceSubClass
echo 1 > bDeviceProtocol
echo 500 > configs/c.1/MaxPower
echo 0xc0 > configs/c.1/bmAttributes

mkdir -p functions/uvc.usb0/streaming/mjpeg/m/240p
cat < functions/uvc.usb0/streaming/mjpeg/m/240p/dwFrameInterval
66
100
500
EOF
echo "320" > functions/uvc.usb0/streaming/mjpeg/m/240p/wWidth 
echo "240" > functions/uvc.usb0/streaming/mjpeg/m/240p/wHeight
echo "200" >  
functions/uvc.usb0/streaming/mjpeg/m/240p/dwDefaultFrameInterval

mkdir -p functions/uvc.usb0/streaming/header/h
cd functions/uvc.usb0/streaming/header/h
ln -s ../../mjpeg/m
cd ../../class/fs
ln -s ../../header/h

cd ../../class/hs
ln -s ../../header/h

cd ../../../control
mkdir header/h
ln -s header/h class/fs

ln -s header/h class/ss

cd ../../../
ln -s functions/uvc.usb0 configs/c.1

echo pxa27x-udc > UDC

rmmod pxa27x_udc

modprobe pxa27x_udc
Segmentation fault

dmesg
configfs-gadget gadget: uvc_unbind
configfs-gadget gadget: uvc_function_bind
kobject (c2902870): tried to init an initialized object, something is 
seriously wrong.
CPU: 0 PID: 340 Comm: modprobe Not tainted 4.10.0-rc5+ #1
Hardware name: HTC Magician
[] (unwind_backtrace) from [] (show_stack+0x10/0x14)
[] (show_stack) from [] (kobject_init+0x74/0x94)
[] (kobject_init) from [] 
(device_initialize+0x20/0x9c)
[] (device_initialize) from [] 
(device_register+0xc/0x18)
[] (device_register) from [] 
(__video_register_device+0xdd0/0x1684 [videodev])
[] (__video_register_device [videodev]) from [] 
(uvc_function_bind+0x324/0x470 [usb_f_uvc])
[] (uvc_function_bind [usb_f_uvc]) from [] 
(usb_add_function+0x70/0x1bc [libcomposite])
[] (usb_add_function [libcomposite]) from [] 
(configfs_composite_bind+0x224/0x320 [libcomposite])
[] (configfs_composite_bind [libcomposite]) from [] 
(udc_bind_to_driver+0x34/0xf8 [udc_core])
[] (udc_bind_to_driver [udc_core]) from [] 
(usb_add_gadget_udc_release+0x190/0x23c [udc_core])
[] (usb_add_gadget_udc_release [udc_core]) from [] 
(pxa_udc_probe+0x238/0x320 [pxa27x_udc])
[] (pxa_udc_probe [pxa27x_udc]) from [] 
(platform_drv_probe+0x38/0x94)
[] (platform_drv_probe) from [] 
(driver_probe_device+0x230/0x420)
[] (driver_probe_device) from [] 
(__driver_attach+0xe4/0xf8)
[] (__driver_attach) from [] 
(bus_for_each_dev+0x58/0x88)
[] (bus_for_each_dev) from [] 
(bus_add_driver+0x148/0x234)
[] (bus_add_driver) from [] 
(driver_register+0x78/0xf4)
[] (driver_register) from [] 
(do_one_initcall+0x48/0x19c)
[] (do_one_initcall) from [] 
(do_init_module+0x54/0x37c)
[] (do_init_module) from [] 
(load_module+0x1c98/0x1f6c)
[] (load_module) from [] 
(SyS_finit_module+0x88/0xbc)
[] (SyS_finit_module) from [] 
(ret_fast_syscall+0x0/0x38)
Unable to handle kernel paging request at virtual address bf5a0718
pgd = c28dc000
[bf5a0718] *pgd=a28b8811, *pte=, *ppte=
Internal error: Oops: 7 [#1] ARM
Modules linked in: pxa27x_udc(+) usb_f_uvc videobuf2_vmalloc 
libcomposite udc_core ppp_deflate zlib_inflate zlib_deflate bsd_comp ppp_async 
ppp_generic slhc ircomm_tty ircomm configfs btusb btintel bluetooth 
firmware_class ads7846 max1586 pxaficp_ir soc_mediabus ohci_pxa27x irda 
ohci_hcd fixed snd_soc_pxa2xx spi_pxa2xx_platform snd_pxa2xx_lib 
snd_pcm_dmaengine snd_soc_pxa_ssp videobuf2_dma_sg usbcore snd_soc_core 
videobuf2_memops v4l2_common videobuf2_v4l2 i2c_pxa pwm_bl videodev backlight 
snd_pcm videobuf2_core usb_common snd_timer pwm_pxa i2c_core snd ssp rtc_pxa 
rtc_sa1100 soundcore leds_gpio htc_pasic3 led_class mfd_core pda_power [last 
unloaded: pxa27x_udc]
CPU: 0 PID: 340 Comm: modprobe Not tainted 4.10.0-rc5+ #1
Hardware name: HTC Magician
task: c3b40c00 task.stack: c33fc000
PC is at kobject_get+0x10/0xa4
LR is at get_device+0x14/0x1c
pc : []lr : []psr: a013
sp : c33fdbb8  

Re: [RFC] usb: gadget: uvc: webcam gadget USB PID is using value from a different gadget

[Petr Cvek]

Dne 23.1.2017 v 15:27 Felipe Balbi napsal(a):
> 
> Hi,
> 
> Greg KH  writes:
> fine by me. Just lsusb will look funky ;-)

 Heh, true, but I thought lsusb would use a string if the device provided
 it.  Haven't looked at that portion of the code in a very long time...

>>>
>>> My lsusb shows separate strings (using usbutils from slackware64-current):
>>>
>>> Bus 004 Device 003: ID 1d6b:0102 Linux Foundation EEM Gadget
>>> ...
>>>   idVendor   0x1d6b Linux Foundation
>>>   idProduct  0x0102 EEM Gadget
>>>   bcdDevice4.07
>>>   iManufacturer   1 Linux Foundation
>>>   iProduct2 Webcam gadget
>>> ...
>>
>> Ah, I guess it doesn't, but who knows how old that version of usbutils
>> is, considering the last release I did was well over a year ago.  I
>> should do a new one one of these days...
>>
>> Anyway, I'd like to not assign a product id to a chunk of code that is
>> going to be eventually deleted.  Felipe, what's the plan for the
>> "legacy" gadget code.  Is it ever going away?
> 
> Well, I wasn't really planning on deleting them just stopped accepting
> any new one. I wanted to avoid angry mobs complaining about not having a
> g_mass_storage.ko anymore.
> 
> Personally, I don't feel strongly about the legacy gadget
> drivers. They're not really needed anymore as everything they do can be
> done with configfs already. Perhaps we could schedule their removal for
> v5.0?
> 

If you want to remove legacy g_webcam then there should be a way to set its 
module parameters from somewhere (it  seems I can't find it anywhere). For 
PXA27x especially "opts->streaming_maxpacket" from f_uvc.c is critical. Default 
max packet size in PXA27x UDC (but lowering the limit to 256 by g_webcam 
parameter works).

Petr

--
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 0/3] usb: xhci: Add broken port disable quirk

[Roger Quadros]

Felipe,

On 03/01/17 14:53, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros  writes:
>> Mathias & Felipe,
>>
>> On 17/11/16 17:01, Roger Quadros wrote:
>>> Hi,
>>>
>>> Some XHCI controllers e.g. dwc3 based have a broken Port disable [1].
>>>
>>> If the attached high-speed device is misbehaving, the USB stack typically
>>> disables the port using the PED bit in PORTSC. For the controllers that
>>> have broken port disable, the port fails to detect further attach/detach
>>> events and so high-speed devices can no longer be enumerated on the
>>> port. The workaround is to prevent port disable using PED on such
>>> controllers.
>>>
>>> We add a new BROKEN_PED quirk flag and 'quirk-broken-port-ped' device
>>> property and prevent port disable using PED if we encounter the quirk flag.
>>>
>>> [1] - AM572x Silicon Errata http://www.ti.com/lit/er/sprz429j/sprz429j.pdf
>>> Section i896— USB xHCI Port Disable Feature Does Not Work
>>
>> Any comments on this series?
>> patch 1 is at v3. Rest 2 are original.
> 
> none from me. Mathias?
> 

Mathias has queued patches 1 and 2 for v4.11.
Can you please pick patch 3? Thanks.

cheers,
-roger
--
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: v4.9 to v4.10 regression: oops when USB cable is plugged in.

[Tony Lindgren]

* Pali Rohár  [170124 02:02]:
> On Tuesday 24 January 2017 10:18:17 Pavel Machek wrote:
> > Hi!
> > On Mon 2017-01-23 14:44:54, Tony Lindgren wrote:
> > > * Pavel Machek  [170123 14:26]:
> > > > [25392.239837] Unhandled fault: external abort on non-linefetch 
> > > > (0x1028) at 0xfa0ab060
> > > > [25392.239868] pgd = c0004000
> > > > [25392.239898] [fa0ab060] *pgd=48011452(bad)
> > > > [25392.239929] Internal error: : 1028 [#1] ARM
> > > > [25392.239929] Modules linked in:
> > > > [25392.239959] CPU: 0 PID: 24322 Comm: kworker/0:1 Not tainted 
> > > > 4.10.0-rc5-142127-g41f2839-dirty #222
> > > > [25392.239990] Hardware name: Nokia RX-51 board
> > > > [25392.240020] Workqueue: events musb_irq_work
> > > > [25392.240051] task: cd44d5c0 task.stack: cd308000
> > > > [25392.240051] PC is at musb_default_readb+0x0/0xc
> > > > [25392.240081] LR is at musb_irq_work+0x1c/0x1b0
> > > 
> > > OK I'm pretty sure the patch I posted few days ago fixes
> > > this. Can you please test patch "[PATCH] usb: musb: Fix
> > > external abort on non-linefetch for musb_irq_work()"?
> > 
> > Can I get the copy of the patch?
> > 
> > http://www.spinics.net/lists/linux-usb/msg152542.html
> > 
> > ...but it is html mangled with no obvious way to unmangle it.

Bounced it to you. FYI, patchwork.kernel.org should have it too, the
"mbox" option there works the best.

> Another place when caller of pm_runtime_get_sync forgot to check return
> value? This is not first time I see this problem related to Nokia N900!

No that's a completely missing pm_runtime_get on this one that's the
most likely cause.

> In past I already suggested to use gcc attribute for pm_runtime_get_sync
> to issue warning when caller does not check return value.

Yeah that would be nice, needs all the missing use cases fixed first
though..

> > > I was able to hit that only once so far, do you hit it
> > > every time with your built-in g_ether .config?
> > 
> > I get it "way too often", like once a day. I don't yet know how to hit
> > it reliably :-(.

OK, well let's hope the patch linked above fixes it. At some point the
number of musb fixes should just start going down if I'm predicting right :)

Anyways, hitting these issues during late -rc cycle is too late. We
really should have some n900 usability testing for core features with
Linux next on at least weekly basis.

I've noticed that testing with Linux next is way less effort than chasing
bugs every -rc cycle when it's too late. For about past four months or so
next has been usable for me with only occasional minor issues that get
fixed within a day or two.

Regards,

Tony





--
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] ARM: dts: Odroid XU4: fix USB3.0 ports

[Richard Genoud]

Since commit 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores"),
the USB ports on odroid-XU4 don't work anymore.

Inserting an usb key (USB2.0) on the USB3.0 port result in:
[   64.488264] xhci-hcd xhci-hcd.2.auto: Port resume took longer than 2 
msec, port status = 0xc400fe3
[   74.568156] xhci-hcd xhci-hcd.2.auto: xHCI host not responding to stop 
endpoint command.
[   74.574806] xhci-hcd xhci-hcd.2.auto: Assuming host is dying, halting host.
[   74.601970] xhci-hcd xhci-hcd.2.auto: HC died; cleaning up
[   74.606276] usb 3-1: USB disconnect, device number 2
[   74.613565] usb 4-1: USB disconnect, device number 2
[   74.621208] usb usb3-port1: couldn't allocate usb_device
NB: it's not related to USB2.0 devices, I get the same result with an USB3.0 
device (SATA to USB3 for instance).
NB2: it doesn't happen on an odriod-XU3 board, that doesn't have the realtek 
RTL8153 chip.

If the lines:
if (dwc->revision > DWC3_REVISION_194A)
reg |= DWC3_GUSB3PIPECTL_SUSPHY;
are commented out, the USB3.0 start working.

For a full analyse: https://lkml.org/lkml/2017/1/18/678

Felipe suggested that suspend should be disabled temporarily while
what's wrong with DCW3 is figured out.

Tested on Odroid XU4

Suggested-by: Felipe Balbi 
Tested-by: Richard Genoud 
Signed-off-by: Richard Genoud 
Cc: sta...@vger.kernel.org # 4.4+
Fixes: 2164a476205ccc ("usb: dwc3: set SUSPHY bit for all cores")
---
 arch/arm/boot/dts/exynos5422-odroidxu4.dts | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5422-odroidxu4.dts 
b/arch/arm/boot/dts/exynos5422-odroidxu4.dts
index 2faf88627a48..f62dbd9b5ad3 100644
--- a/arch/arm/boot/dts/exynos5422-odroidxu4.dts
+++ b/arch/arm/boot/dts/exynos5422-odroidxu4.dts
@@ -43,6 +43,15 @@
status = "okay";
 };
 
+&usbdrd_dwc3_0 {
+   /*
+* without that, usb devices are not recognized when
+* they are plugged.
+* cf: https://lkml.org/lkml/2017/1/18/678
+*/
+   snps,dis_u3_susphy_quirk;
+};
+
 &usbdrd_dwc3_1 {
dr_mode = "host";
 };
--
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/5] usb: musb: musb_cppi41: Workaround DMA stall issue during teardown

[Sergei Shtylyov]

Hello.

On 1/24/2017 1:11 PM, Alexandre Bailon wrote:


The DMA may hung up if a teardown is initiated while an endpoint is still


   Hang up.


active (Advisory 2.3.27 of DA8xx errata).
To workaround this issue, add a delay before to initiate the teardown.

Signed-off-by: Alexandre Bailon 

[...]

MBR, Sergei

--
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: functionfs on dwc3, xhci host: endpoint cannot be used in both directions ?

[Vincent Pelletier]

On Tue, 24 Jan 2017 11:02:10 +0200, Felipe Balbi
 wrote:
> yeah, order matters. That's documented for f_fs.

The order in the EP0 write call yes.
The mangles-HS-wMaxPacketSize-if-no-LS/FS-given is not (or I could not
find it).

> Well, we can't easily change the way it works because it's an ABI to
> userspace. I would have very much preferred for us to pass descriptors
> using ioctl(). That way we could have:
>
>         ioctl(fd, FFS_IOCTL_SS_EP_DESC, ep1_ss_desc);
>         ioctl(fd, FFS_IOCTL_SS_EP_COMP_DESC, ep1_ss_comp_desc);
>
> and so on.

If you mean the EP0 writes, indeed it's userspace ABI and I agree a
more structured/fine-grained access method would be nicer (current
error reporting is terrible... too many way to trigger the same errno).

But I was actually thinking of the epautoconf API.
Is it also exposed to userspace ?

Actually, thinking a bit more about it I think epautoconf should only
alter wMaxPacketSize when caller is probing the max supported value,
and in such case provide the minimum between speed-dependent limit and
controller limit. And I would remove the hardcoded 64B limit on non-SS
bulk descriptors.

Regards,
-- 
Vincent Pelletier


pgpHmFVvwnWI4.pgp
Description: Signature digitale OpenPGP

Re: [PATCH v2 28/37] usb: host: xhci: combine event TRB completion debugging messages

[Janusz Dziedzic]

On 23 January 2017 at 13:20, Mathias Nyman
 wrote:
> From: Felipe Balbi 
>
> If we just provide a helper to convert completion code to string, we can
> combine all debugging messages into a single print.
>
> [keep the old debug messages, for warn and grep -Mathias]
> Signed-off-by: Felipe Balbi 
> Signed-off-by: Mathias Nyman 
> ---
>  drivers/usb/host/xhci.h | 80 
> +
>  1 file changed, 80 insertions(+)
>
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index aa63e38..ebdd920 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1097,6 +1097,86 @@ struct xhci_transfer_event {
>  #define COMP_SECONDARY_BANDWIDTH_ERROR 35
>  #define COMP_SPLIT_TRANSACTION_ERROR   36
>
> +static inline const char *xhci_trb_comp_code_string(u8 status)
> +{

BTW, maybe in the future we should use enum for status
and next:

#define C2S(x) case x: return #x
static inline const char *xhci_trb_comp_code_string(enum
xhci_trb_comp_code code)
{
switch(code) {
C2S(COMP_INVALID);
C2S(COMP_SUCCESS);
...
   default:
return "Unknown!!";
}
}
#undef C2S

After that in dbgmsg/trace we will have same enum and we don't need
translate msg->enum while debugging.

> +   switch (status) {
> +   case COMP_INVALID:
> +   return "Invalid";
> +   case COMP_SUCCESS:
> +   return "Success";
> +   case COMP_DATA_BUFFER_ERROR:
> +   return "Data Buffer Error";
> +   case COMP_BABBLE_DETECTED_ERROR:
> +   return "Babble Detected";
> +   case COMP_USB_TRANSACTION_ERROR:
> +   return "USB Transaction Error";
> +   case COMP_TRB_ERROR:
> +   return "TRB Error";
> +   case COMP_STALL_ERROR:
> +   return "Stall Error";
> +   case COMP_RESOURCE_ERROR:
> +   return "Resource Error";
> +   case COMP_BANDWIDTH_ERROR:
> +   return "Bandwidth Error";
> +   case COMP_NO_SLOTS_AVAILABLE_ERROR:
> +   return "No Slots Available Error";
> +   case COMP_INVALID_STREAM_TYPE_ERROR:
> +   return "Invalid Stream Type Error";
> +   case COMP_SLOT_NOT_ENABLED_ERROR:
> +   return "Slot Not Enabled Error";
> +   case COMP_ENDPOINT_NOT_ENABLED_ERROR:
> +   return "Endpoint Not Enabled Error";
> +   case COMP_SHORT_PACKET:
> +   return "Short Packet";
> +   case COMP_RING_UNDERRUN:
> +   return "Ring Underrun";
> +   case COMP_RING_OVERRUN:
> +   return "Ring Overrun";
> +   case COMP_VF_EVENT_RING_FULL_ERROR:
> +   return "VF Event Ring Full Error";
> +   case COMP_PARAMETER_ERROR:
> +   return "Parameter Error";
> +   case COMP_BANDWIDTH_OVERRUN_ERROR:
> +   return "Bandwidth Overrun Error";
> +   case COMP_CONTEXT_STATE_ERROR:
> +   return "Context State Error";
> +   case COMP_NO_PING_RESPONSE_ERROR:
> +   return "No Ping Response Error";
> +   case COMP_EVENT_RING_FULL_ERROR:
> +   return "Event Ring Full Error";
> +   case COMP_INCOMPATIBLE_DEVICE_ERROR:
> +   return "Incompatible Device Error";
> +   case COMP_MISSED_SERVICE_ERROR:
> +   return "Missed Service Error";
> +   case COMP_COMMAND_RING_STOPPED:
> +   return "Command Ring Stopped";
> +   case COMP_COMMAND_ABORTED:
> +   return "Command Aborted";
> +   case COMP_STOPPED:
> +   return "Stopped";
> +   case COMP_STOPPED_LENGTH_INVALID:
> +   return "Stopped - Length Invalid";
> +   case COMP_STOPPED_SHORT_PACKET:
> +   return "Stopped - Short Packet";
> +   case COMP_MAX_EXIT_LATENCY_TOO_LARGE_ERROR:
> +   return "Max Exit Latency Too Large Error";
> +   case COMP_ISOCH_BUFFER_OVERRUN:
> +   return "Isoch Buffer Overrun";
> +   case COMP_EVENT_LOST_ERROR:
> +   return "Event Lost Error";
> +   case COMP_UNDEFINED_ERROR:
> +   return "Undefined Error";
> +   case COMP_INVALID_STREAM_ID_ERROR:
> +   return "Invalid Stream ID Error";
> +   case COMP_SECONDARY_BANDWIDTH_ERROR:
> +   return "Secondary Bandwidth Error";
> +   case COMP_SPLIT_TRANSACTION_ERROR:
> +   return "Split Transaction Error";
> +   default:
> +   return "Unknown!!";
> +   }
> +}
> +
>  struct xhci_link_trb {
> /* 64-bit segment pointer*/
> __le64 segment_ptr;
> --
> 1.9.1
>
> --
> 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
Mo

RE: gadget: f_fs: Accept up to 30 endpoints.

[David Laight]

From: Felipe Balbi
> Sent: 24 January 2017 10:50
> David Laight  writes:
> > From: Of Vincent Pelletier
> >> Sent: 23 January 2017 14:41
> >> It is allowed by the USB specification to enabled same-address, opposite-
> >> direction endpoints simultaneously, which means 30 non-zero endpoints
> >> are allowed. So double eps_addrmap length to 30.
> >> The original code only accepted 14 descriptors out of a likely intended 15
> >> (as there are 15 endpoint addresses, ignoring direction), because the first
> >> eps_addrmap entry is unused (it is a placeholder for endpoint zero). So
> >> increase eps_addrmap length by one to 31.
> >>
> >> Signed-off-by: Vincent Pelletier 
> >> ---
> >>  drivers/usb/gadget/function/f_fs.c | 2 +-
> >>  drivers/usb/gadget/function/u_fs.h | 3 ++-
> >>  2 files changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/usb/gadget/function/f_fs.c 
> >> b/drivers/usb/gadget/function/f_fs.c
> >> index 5c91a6f4613b..c4364c87 100644
> >> --- a/drivers/usb/gadget/function/f_fs.c
> >> +++ b/drivers/usb/gadget/function/f_fs.c
> >> @@ -2101,7 +2101,7 @@ static int __ffs_data_do_entity(enum ffs_entity_type 
> >> type,
> >>case FFS_ENDPOINT:
> >>d = (void *)desc;
> >>helper->eps_count++;
> >> -  if (helper->eps_count >= 15)
> >> +  if (helper->eps_count >= FFS_MAX_EPS_COUNT)
> >
> > Use ARRAY_COUNT(helper->eps_addrmap)
> 
> what's the benefit here? We have a macro that defines the size of the
> array. ARRAY_SIZE() is useful when we don't know the size of the array
> ahead of time ;-)

It saves you having to check that the array is defined with the same
constant as is used in the test.

David

--
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: option: add device ID for HP lt2523 (Novatel E371)

[Johan Hovold]

On Tue, Jan 24, 2017 at 10:31:18AM +0100, Bjørn Mork wrote:
> Yet another laptop vendor rebranded Novatel E371.
> 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Bjørn Mork 

Applied, thanks.

Johan
--
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] usb: dwc3: handle DWC_USB3_NUM == DWC_USB3_NUM_IN_EPS

[Bryan O'Donoghue]

On 23/01/17 22:34, John Youn wrote:
> On 1/23/2017 2:10 PM, Alan Stern wrote:
>> On Mon, 23 Jan 2017, John Youn wrote:
>>
>>> On 1/22/2017 5:29 PM, Bryan O'Donoghue wrote:
 - DWC_USB3_NUM indicates the number of Device mode single directional
   endpoints, including OUT and IN endpoint 0.

 - DWC_USB3_NUM_IN_EPS indicates the maximum number of Device mode IN
   endpoints active at any time, including control endpoint 0.

 It's possible to configure RTL such that DWC_USB3_NUM_EPS is equal to
 DWC_USB3_NUM_IN_EPS.

 dwc3-core calculates the number of OUT endpoints as DWC_USB3_NUM minus
 DWC_USB3_NUM_IN_EPS. If RTL has been configured with DWC_USB3_NUM_IN_EPS
 equal to DWC_USB3_NUM then dwc3-core will calculate the number of OUT
 endpoints as zero.

 For example a from dwc3_core_num_eps() shows:
 [1.565000]  /usb0@f01d: found 8 IN and 0 OUT endpoints

 This patch works around this case by detecting when DWC_USB3_NUM_EPS is
 equal to DWC3_USB3_NUM_IN_EPS and over-rides the calculation of the number
>>>
>>> What if NUM_IN_EPS=7 and NUM_EPS=8? You will still have a problem.
>>>
>>> It's possible to fix this for the general case rather than for this
>>> specific case.
>>
>> What is the reason for computing NUM_OUT_EPS in the first place?
>> Isn't it true that any endpoint can be used as an OUT endpoint?
>>
>> So the real restrictions on a configuration are:
>>
>>  number of IN endpoints <= NUM_IN_EPS, and
>>
>>  number of IN endpoints + number of OUT endpoints <= NUM_EPS,
>>
>> where ep0 must be counted twice, as both an IN and an OUT endpoint.
>> The value of NUM_OUT_EPS isn't used and shouldn't matter.
>>
> 
> Yes that's correct. The general fix should take all that into account
> so that any combination of NUM_EPS and NUM_IN_EPS will work fine.
> 
> However it must also account for FPGA configs where each HW endpoint
> has a fixed number/direction, which the current code is compatible
> with.

I'm not scanning something here..

If FPGA configurations can have fixed endpoint directions then you could
conceivably have a control IN/OUT pair plus a number a fixed
configuration of all IN or all OUT endpoints, even checking
GHWPARAMS6.EN_FPGA wouldn't let you know which direction those endpoints
had been configured to.

If I understand you correctly an FPGA could have pretty much any bit-map
of fixed endpoint directions - so just calculating the number of IN/OUT
endpoints based on some combination of DWC_USB3_NUM_IN_EPS and
DWC_USB3_NUM_EPS wouldn't inform you of the fixed direction of an
individual endpoint ... You'd need a descriptor specifying the fixed
direction of each endpoint right ?

---
bod

--
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: gadget: Kernel panic (NULL pointer dereference) when using fsl_udc2_core on i.MX31 PDK

[Felipe Balbi]

Hi,

Magnus Lilja  writes:
>> Magnus Lilja  writes:
> I tried the fsl_udc_core gadget driver on the i.MX31 PDK board and got a
> kernel panic (NULL pointer dereference) when connecting the USB cable. I
> had the g_serial module loaded as well.
>
> The NULL pointer panic comes from gadget/udc/core.c
> usb_gadget_giveback_request() which calls req->complete() and in some
> cases req->complete is NULL.
>
> Commit 304f7e5e1d08 ("usb: gadget: Refactor request completion") changed
> fsl_udc2_core.c (and several other files) and in fsl_udc2_core.c a check
> that req->complete is non-NULL was removed:
>
> --- a/drivers/usb/gadget/udc/fsl_udc_core.c
> +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
> @@ -197,10 +197,8 @@ __acquires(ep->udc->lock)
>  ep->stopped = 1;
>
>  spin_unlock(&ep->udc->lock);
> -   /* complete() is from gadget layer,
> -* eg fsg->bulk_in_complete() */
> -   if (req->req.complete)
> -   req->req.complete(&ep->ep, &req->req);
> +
> +   usb_gadget_giveback_request(&ep->ep, &req->req);
>
>  spin_lock(&ep->udc->lock);
>  ep->stopped = stopped;
>
> If I re-introduce the check (either in fsl_udc_core.c or core.c) at
> least USB gadget operation using g_serial seems to work just fine.
>
> I don't know the logic in detail to understand whether this is a proper
> fix or if there is some other more problem with the fls_udc_core driver.
> Does anyone have input in this matter?
>
> I can produce a proper patch that fixes this problem by re-introducing
> the check (in either fsl_udc_core.c or core.c) if that is a proper
> solution and I can also assist in testing other fixes to the problem.

 ->complete() is supposed to be mandatory. Which gadget do you have that
 ->doesn't set ->complete() to a valid function pointer?
>>>
>>> I'm modprobing g_serial so the following modules are loaded (using my 
>>> patch):
>>>
>>> ~ # lsmod
>>> usb_f_acm
>>> u_serial
>>> g_serial
>>> libcomposite
>>> configfs
>>> fsl_usb2_udc
>>
>> okay, can you figure out which request is coming without ->complete()
>> set? To which endpoint is this request being queued? It would be nice to
>> know these details. Maybe this is an old bug which ought to be fixed.
>
> Sure, I can try figure that out. Any input to make the debug of the
> faster is appreciated if you have any.

well, the easiest way is to add something like:

if (!req->complete)
dump_stack();

to fsl udc driver. Then you would know who queued the request without
->complete. A slightly better approach would be to:

if (WARN(!req->complete,
"%s: queueing request without ->complete\n", ep->name))
return;

Or something like that.

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH v2 28/37] usb: host: xhci: combine event TRB completion debugging messages

[Felipe Balbi]

Hi,

David Laight  writes:
> From: Mathias Nyman
>> Sent: 23 January 2017 12:20
>> If we just provide a helper to convert completion code to string, we can
>> combine all debugging messages into a single print.
> ...
>> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
>> index aa63e38..ebdd920 100644
>> --- a/drivers/usb/host/xhci.h
>> +++ b/drivers/usb/host/xhci.h
>> @@ -1097,6 +1097,86 @@ struct xhci_transfer_event {
>>  #define COMP_SECONDARY_BANDWIDTH_ERROR  35
>>  #define COMP_SPLIT_TRANSACTION_ERROR36
>> 
>> +static inline const char *xhci_trb_comp_code_string(u8 status)
>> +{
>> +switch (status) {
>> +case COMP_INVALID:
>> +return "Invalid";
> ...
>> +case COMP_SPLIT_TRANSACTION_ERROR:
>> +return "Split Transaction Error";
>> +default:
>> +return "Unknown!!";
>> +}
>> +}
>
> That ought to be a real function, not a static inline.
> Will generate a lot of code and data if inlined.

it's only used for tracing and compiler can uninline inline functions.

> If the error codes are reasonable dense then an array is much better
> than the switch statement.

Even though I haven't looked at the resulting binary, I'm pretty sure
compiler can optimize this.

-- 
balbi


signature.asc
Description: PGP signature

RE: gadget: f_fs: Accept up to 30 endpoints.

[Felipe Balbi]

Hi,

David Laight  writes:
> From: Of Vincent Pelletier
>> Sent: 23 January 2017 14:41
>> It is allowed by the USB specification to enabled same-address, opposite-
>> direction endpoints simultaneously, which means 30 non-zero endpoints
>> are allowed. So double eps_addrmap length to 30.
>> The original code only accepted 14 descriptors out of a likely intended 15
>> (as there are 15 endpoint addresses, ignoring direction), because the first
>> eps_addrmap entry is unused (it is a placeholder for endpoint zero). So
>> increase eps_addrmap length by one to 31.
>> 
>> Signed-off-by: Vincent Pelletier 
>> ---
>>  drivers/usb/gadget/function/f_fs.c | 2 +-
>>  drivers/usb/gadget/function/u_fs.h | 3 ++-
>>  2 files changed, 3 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/usb/gadget/function/f_fs.c 
>> b/drivers/usb/gadget/function/f_fs.c
>> index 5c91a6f4613b..c4364c87 100644
>> --- a/drivers/usb/gadget/function/f_fs.c
>> +++ b/drivers/usb/gadget/function/f_fs.c
>> @@ -2101,7 +2101,7 @@ static int __ffs_data_do_entity(enum ffs_entity_type 
>> type,
>>  case FFS_ENDPOINT:
>>  d = (void *)desc;
>>  helper->eps_count++;
>> -if (helper->eps_count >= 15)
>> +if (helper->eps_count >= FFS_MAX_EPS_COUNT)
>
> Use ARRAY_COUNT(helper->eps_addrmap)

what's the benefit here? We have a macro that defines the size of the
array. ARRAY_SIZE() is useful when we don't know the size of the array
ahead of time ;-)

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v2] arm: davinci: Make the usb20 clock available to PM runtime

[Sekhar Nori]

On Tuesday 24 January 2017 04:02 PM, Alexandre Bailon wrote:
> Add usb20 to the list of clock supported by PM runtime.

Patch description does not match contents. You should rather talk about:
since USB20 subsystem uses just one clock, there is no need of a con_id,
so we are using NULL.

Also, "ARM: " prefix should be capitalized .

> 
> Signed-off-by: Alexandre Bailon 
> Suggested-by: Sekhar Nori 

Thanks,
Sekhar
--
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: gadget: f_fs: Accept up to 30 endpoints.

[David Laight]

From: Of Vincent Pelletier
> Sent: 23 January 2017 14:41
> It is allowed by the USB specification to enabled same-address, opposite-
> direction endpoints simultaneously, which means 30 non-zero endpoints
> are allowed. So double eps_addrmap length to 30.
> The original code only accepted 14 descriptors out of a likely intended 15
> (as there are 15 endpoint addresses, ignoring direction), because the first
> eps_addrmap entry is unused (it is a placeholder for endpoint zero). So
> increase eps_addrmap length by one to 31.
> 
> Signed-off-by: Vincent Pelletier 
> ---
>  drivers/usb/gadget/function/f_fs.c | 2 +-
>  drivers/usb/gadget/function/u_fs.h | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_fs.c 
> b/drivers/usb/gadget/function/f_fs.c
> index 5c91a6f4613b..c4364c87 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -2101,7 +2101,7 @@ static int __ffs_data_do_entity(enum ffs_entity_type 
> type,
>   case FFS_ENDPOINT:
>   d = (void *)desc;
>   helper->eps_count++;
> - if (helper->eps_count >= 15)
> + if (helper->eps_count >= FFS_MAX_EPS_COUNT)

Use ARRAY_COUNT(helper->eps_addrmap)

...

David

--
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 28/37] usb: host: xhci: combine event TRB completion debugging messages

[David Laight]

From: Mathias Nyman
> Sent: 23 January 2017 12:20
> If we just provide a helper to convert completion code to string, we can
> combine all debugging messages into a single print.
...
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index aa63e38..ebdd920 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1097,6 +1097,86 @@ struct xhci_transfer_event {
>  #define COMP_SECONDARY_BANDWIDTH_ERROR   35
>  #define COMP_SPLIT_TRANSACTION_ERROR 36
> 
> +static inline const char *xhci_trb_comp_code_string(u8 status)
> +{
> + switch (status) {
> + case COMP_INVALID:
> + return "Invalid";
...
> + case COMP_SPLIT_TRANSACTION_ERROR:
> + return "Split Transaction Error";
> + default:
> + return "Unknown!!";
> + }
> +}

That ought to be a real function, not a static inline.
Will generate a lot of code and data if inlined.

If the error codes are reasonable dense then an array is much better
than the switch statement.

David

--
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] arm: davinci: Make the usb20 clock available to PM runtime

[Alexandre Bailon]

Add usb20 to the list of clock supported by PM runtime.

Signed-off-by: Alexandre Bailon 
Suggested-by: Sekhar Nori 
---
 arch/arm/mach-davinci/da830.c | 2 +-
 arch/arm/mach-davinci/da850.c | 2 +-
 arch/arm/mach-davinci/usb-da8xx.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c
index 073c458..2cfd9d7 100644
--- a/arch/arm/mach-davinci/da830.c
+++ b/arch/arm/mach-davinci/da830.c
@@ -412,7 +412,7 @@ static struct clk_lookup da830_clks[] = {
CLK("davinci-mcasp.0",  NULL,   &mcasp0_clk),
CLK("davinci-mcasp.1",  NULL,   &mcasp1_clk),
CLK("davinci-mcasp.2",  NULL,   &mcasp2_clk),
-   CLK("musb-da8xx",   "usb20",&usb20_clk),
+   CLK("musb-da8xx",   NULL,   &usb20_clk),
CLK(NULL,   "aemif",&aemif_clk),
CLK(NULL,   "aintc",&aintc_clk),
CLK(NULL,   "secu_mgr", &secu_mgr_clk),
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 9780829..5fe32ae 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -559,7 +559,7 @@ static struct clk_lookup da850_clks[] = {
CLK("ti-aemif", NULL,   &aemif_clk),
CLK("davinci-nand.0",   "aemif",&aemif_nand_clk),
CLK("ohci-da8xx",   "usb11",&usb11_clk),
-   CLK("musb-da8xx",   "usb20",&usb20_clk),
+   CLK("musb-da8xx",   NULL,   &usb20_clk),
CLK("spi_davinci.0",NULL,   &spi0_clk),
CLK("spi_davinci.1",NULL,   &spi1_clk),
CLK("vpif", NULL,   &vpif_clk),
diff --git a/arch/arm/mach-davinci/usb-da8xx.c 
b/arch/arm/mach-davinci/usb-da8xx.c
index 9a6af0b..9b66768 100644
--- a/arch/arm/mach-davinci/usb-da8xx.c
+++ b/arch/arm/mach-davinci/usb-da8xx.c
@@ -275,7 +275,7 @@ int __init da8xx_register_usb20_phy_clk(bool 
use_usb_refclkin)
struct clk *parent;
int ret;
 
-   usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
+   usb20_clk = clk_get(&da8xx_usb20_dev.dev, NULL);
ret = PTR_ERR_OR_ZERO(usb20_clk);
if (ret)
return ret;
-- 
2.10.2

--
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 v4] ARM: dts: da850: Add the CPPI 4.1 DMA to the USB OTG controller

[Alexandre Bailon]

This adds the CPPI 4.1 DMA controller to the USB OTG controller.

Changes since v3:
- Don't use a wildcard for compatible property.

Signed-off-by: Alexandre Bailon 
---
 arch/arm/boot/dts/da850.dtsi | 25 +
 1 file changed, 25 insertions(+)

diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
index 104155d..f4a637a 100644
--- a/arch/arm/boot/dts/da850.dtsi
+++ b/arch/arm/boot/dts/da850.dtsi
@@ -396,12 +396,37 @@
usb0: usb@20 {
compatible = "ti,da830-musb";
reg = <0x20 0x1>;
+   ranges;
interrupts = <58>;
interrupt-names = "mc";
dr_mode = "otg";
phys = <&usb_phy 0>;
phy-names = "usb-phy";
status = "disabled";
+
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   dmas = <&cppi41dma 0 0 &cppi41dma 1 0
+   &cppi41dma 2 0 &cppi41dma 3 0
+   &cppi41dma 0 1 &cppi41dma 1 1
+   &cppi41dma 2 1 &cppi41dma 3 1>;
+   dma-names =
+   "rx1", "rx2", "rx3", "rx4",
+   "tx1", "tx2", "tx3", "tx4";
+
+   cppi41dma: dma-controller@201000 {
+   compatible = "ti,da830-cppi41";
+   reg =  <0x201000 0x1000
+   0x202000 0x1000
+   0x204000 0x4000>;
+   reg-names = "controller",
+   "scheduler", "queuemgr";
+   interrupts = <58>;
+   #dma-cells = <2>;
+   #dma-channels = <4>;
+   status = "okay";
+   };
};
mdio: mdio@224000 {
compatible = "ti,davinci_mdio";
-- 
2.10.2

--
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 v4 1/5] usb: musb: da8xx: Remove CPPI 3.0 quirk and methods

[Alexandre Bailon]

DA8xx driver is registering and using the CPPI 3.0 DMA controller but
actually, the DA8xx has a CPPI 4.1 DMA controller.
Remove the CPPI 3.0 quirk and methods.

Fixes: f8e9f34f80a2 ("usb: musb: Fix up DMA related macros")
Fixes: 7f6283ed6fe8 ("usb: musb: Set up function pointers for DMA")
Signed-off-by: Alexandre Bailon 
Acked-by: Sergei Shtylyov 
Acked-by: Tony Lindgren 
---
 drivers/usb/musb/da8xx.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index e89708d..cd3d763 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -458,15 +458,11 @@ static inline u8 get_vbus_power(struct device *dev)
 }
 
 static const struct musb_platform_ops da8xx_ops = {
-   .quirks = MUSB_DMA_CPPI | MUSB_INDEXED_EP,
+   .quirks = MUSB_INDEXED_EP,
.init   = da8xx_musb_init,
.exit   = da8xx_musb_exit,
 
.fifo_mode  = 2,
-#ifdef CONFIG_USB_TI_CPPI_DMA
-   .dma_init   = cppi_dma_controller_create,
-   .dma_exit   = cppi_dma_controller_destroy,
-#endif
.enable = da8xx_musb_enable,
.disable= da8xx_musb_disable,
 
-- 
2.10.2

--
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 v4 4/5] usb: musb: musb_cppi41: Workaround DMA stall issue during teardown

[Alexandre Bailon]

The DMA may hung up if a teardown is initiated while an endpoint is still
active (Advisory 2.3.27 of DA8xx errata).
To workaround this issue, add a delay before to initiate the teardown.

Signed-off-by: Alexandre Bailon 
---
 drivers/usb/musb/da8xx.c   | 2 +-
 drivers/usb/musb/musb_core.h   | 1 +
 drivers/usb/musb/musb_cppi41.c | 4 
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index d279438..d87fb9b 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -483,7 +483,7 @@ da8xx_dma_controller_create(struct musb *musb, void __iomem 
*base)
 #endif
 
 static const struct musb_platform_ops da8xx_ops = {
-   .quirks = MUSB_INDEXED_EP | MUSB_DMA_CPPI41,
+   .quirks = MUSB_INDEXED_EP | MUSB_DMA_CPPI41 | MUSB_DA8XX,
.init   = da8xx_musb_init,
.exit   = da8xx_musb_exit,
 
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index ade902e..d129278 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -172,6 +172,7 @@ struct musb_io;
  */
 struct musb_platform_ops {
 
+#define MUSB_DA8XX BIT(7)
 #define MUSB_DMA_UX500 BIT(6)
 #define MUSB_DMA_CPPI41BIT(5)
 #define MUSB_DMA_CPPI  BIT(4)
diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index 7253ea1..2fb2b81 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -553,6 +553,10 @@ static int cppi41_dma_channel_abort(struct dma_channel 
*channel)
}
}
 
+   /* DA8xx Advisory 2.3.27: wait 250 ms before to start the teardown */
+   if (musb->io.quirks & MUSB_DA8XX)
+   mdelay(250);
+
tdbit = 1 << cppi41_channel->port_num;
if (is_tx)
tdbit <<= 16;
-- 
2.10.2

--
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 v4 0/5] usb: musb: da8xx: Add DMA support

[Alexandre Bailon]

This series update MUSB driver to add DMA support to DA8xx.
It should be applied on top of
"[PATCH v3 0/3] usb: musb: cppi41: Add a way to manage DMA irq" but
"[PATCH v3 0/3] dmaengine: cppi41: Add dma support to da8xx" and
"[PATCH] arm: davinci: Make the usb20 clock available to PM runtime"
are required to make it work.

Changes in v4:
- Update and clarify the commit message of patch 5
- Fix the typo in patch 3

Changes in v3:
- Remove PM runtime callbacks.
  I have update arch/arm/mach-davinci/pm_domain.c to let PM runtime control
  the usb20 clock.
- Only use PM runtime sync operation.

Changes in v2:
- Clock and IRQ management has been moved to MUSB DA8xx glue
  (was in CPPI 4.1 driver)
- I have added a partial support PM runtime. The goal was to use PM
  runtime to manage clock of MUSB and CPPI 4.1 (they use the same clock).
- CPPI 4.1 is now achild of MUSB DA8xx glue.

Alexandre Bailon (5):
  usb: musb: da8xx: Remove CPPI 3.0 quirk and methods
  usb: musb: Use shared irq
  usb: musb: Add support of CPPI 4.1 DMA controller to DA8xx
  usb: musb: musb_cppi41: Workaround DMA stall issue during teardown
  usb: musb: da8xx: Add a primary support of PM runtime

 drivers/usb/musb/Kconfig   |  4 +--
 drivers/usb/musb/da8xx.c   | 60 +++---
 drivers/usb/musb/musb_core.c   |  2 +-
 drivers/usb/musb/musb_core.h   |  1 +
 drivers/usb/musb/musb_cppi41.c |  4 +++
 5 files changed, 47 insertions(+), 24 deletions(-)

-- 
2.10.2

--
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 v4 2/5] usb: musb: Use shared irq

[Alexandre Bailon]

In the DA8xx, USB and CPPI 4.1 are sharing the same interrupt line.
Update the driver to request a shared irq.

Signed-off-by: Alexandre Bailon 
---
 drivers/usb/musb/musb_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index fca288bb..cf40adf 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2329,7 +2329,7 @@ musb_init_controller(struct device *dev, int nIrq, void 
__iomem *ctrl)
setup_timer(&musb->otg_timer, musb_otg_timer_func, (unsigned long) 
musb);
 
/* attach to the IRQ */
-   if (request_irq(nIrq, musb->isr, 0, dev_name(dev), musb)) {
+   if (request_irq(nIrq, musb->isr, IRQF_SHARED, dev_name(dev), musb)) {
dev_err(dev, "request_irq %d failed!\n", nIrq);
status = -ENODEV;
goto fail3;
-- 
2.10.2

--
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 v4 3/5] usb: musb: Add support of CPPI 4.1 DMA controller to DA8xx

[Alexandre Bailon]

Currently, only the PIO mode is supported.
This add support of CPPI 4.1 to DA8xx.
As the In DA8xx the CPPI 4.1 DMA is a part of the USB.
Create the CPPI 4.1 device as a child of USB.

Signed-off-by: Alexandre Bailon 
---
 drivers/usb/musb/Kconfig |  4 ++--
 drivers/usb/musb/da8xx.c | 35 ++-
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 72a2a50..5506a9c 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -160,8 +160,8 @@ config USB_TI_CPPI_DMA
  Enable DMA transfers when TI CPPI DMA is available.
 
 config USB_TI_CPPI41_DMA
-   bool 'TI CPPI 4.1 (AM335x)'
-   depends on ARCH_OMAP && DMADEVICES
+   bool 'TI CPPI 4.1'
+   depends on (ARCH_OMAP || ARCH_DAVINCI_DA8XX) && DMADEVICES
select TI_CPPI41
 
 config USB_TUSB_OMAP_DMA
diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index cd3d763..d279438 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -33,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -457,12 +458,40 @@ static inline u8 get_vbus_power(struct device *dev)
return current_uA / 1000 / 2;
 }
 
+#ifdef CONFIG_USB_TI_CPPI41_DMA
+static void da8xx_dma_controller_callback(struct dma_controller *c)
+{
+   struct musb *musb = c->musb;
+   void __iomem *reg_base = musb->ctrl_base;
+
+   musb_writel(reg_base, DA8XX_USB_END_OF_INTR_REG, 0);
+}
+
+static struct dma_controller *
+da8xx_dma_controller_create(struct musb *musb, void __iomem *base)
+{
+   struct dma_controller *controller;
+
+   controller = cppi41_dma_controller_create(musb, base);
+   if (IS_ERR_OR_NULL(controller))
+   return controller;
+
+   controller->dma_callback = da8xx_dma_controller_callback;
+
+   return controller;
+}
+#endif
+
 static const struct musb_platform_ops da8xx_ops = {
-   .quirks = MUSB_INDEXED_EP,
+   .quirks = MUSB_INDEXED_EP | MUSB_DMA_CPPI41,
.init   = da8xx_musb_init,
.exit   = da8xx_musb_exit,
 
.fifo_mode  = 2,
+#ifdef CONFIG_USB_TI_CPPI41_DMA
+   .dma_init   = da8xx_dma_controller_create,
+   .dma_exit   = cppi41_dma_controller_destroy,
+#endif
.enable = da8xx_musb_enable,
.disable= da8xx_musb_disable,
 
@@ -534,6 +563,10 @@ static int da8xx_probe(struct platform_device *pdev)
}
platform_set_drvdata(pdev, glue);
 
+   ret = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
+   if (ret)
+   return ret;
+
memset(musb_resources, 0x00, sizeof(*musb_resources) *
ARRAY_SIZE(musb_resources));
 
-- 
2.10.2

--
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 v4 5/5] usb: musb: da8xx: Add a primary support of PM runtime

[Alexandre Bailon]

Currently, MUSB DA8xx glue driver doesn't have PM runtime support.
Because the CPPI 4.1 is using the same clock as MUSB DA8xx and
CPPI 4.1 is a child of MUSB DA8xx glue, add support of PM runtime
to the DA8xx glue driver in order to let the CPPI 4.1 driver manage
the clock by using PM runtime.

Signed-off-by: Alexandre Bailon 
---
 drivers/usb/musb/da8xx.c | 23 ++-
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index d87fb9b..bebc9ed 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -30,7 +30,6 @@
  */
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -86,7 +85,6 @@ struct da8xx_glue {
struct device   *dev;
struct platform_device  *musb;
struct platform_device  *usb_phy;
-   struct clk  *clk;
struct phy  *phy;
 };
 
@@ -377,11 +375,7 @@ static int da8xx_musb_init(struct musb *musb)
 
musb->mregs += DA8XX_MENTOR_CORE_OFFSET;
 
-   ret = clk_prepare_enable(glue->clk);
-   if (ret) {
-   dev_err(glue->dev, "failed to enable clock\n");
-   return ret;
-   }
+   pm_runtime_get_sync(musb->controller->parent);
 
/* Returns zero if e.g. not clocked */
rev = musb_readl(reg_base, DA8XX_USB_REVISION_REG);
@@ -424,7 +418,7 @@ static int da8xx_musb_init(struct musb *musb)
 err_phy_power_on:
phy_exit(glue->phy);
 fail:
-   clk_disable_unprepare(glue->clk);
+   pm_runtime_put(musb->controller->parent);
return ret;
 }
 
@@ -436,7 +430,7 @@ static int da8xx_musb_exit(struct musb *musb)
 
phy_power_off(glue->phy);
phy_exit(glue->phy);
-   clk_disable_unprepare(glue->clk);
+   pm_runtime_put(musb->controller->parent);
 
usb_put_phy(musb->xceiv);
 
@@ -519,7 +513,6 @@ static int da8xx_probe(struct platform_device *pdev)
struct musb_hdrc_platform_data  *pdata = dev_get_platdata(&pdev->dev);
struct da8xx_glue   *glue;
struct platform_device_info pinfo;
-   struct clk  *clk;
struct device_node  *np = pdev->dev.of_node;
int ret;
 
@@ -527,12 +520,6 @@ static int da8xx_probe(struct platform_device *pdev)
if (!glue)
return -ENOMEM;
 
-   clk = devm_clk_get(&pdev->dev, "usb20");
-   if (IS_ERR(clk)) {
-   dev_err(&pdev->dev, "failed to get clock\n");
-   return PTR_ERR(clk);
-   }
-
glue->phy = devm_phy_get(&pdev->dev, "usb-phy");
if (IS_ERR(glue->phy)) {
if (PTR_ERR(glue->phy) != -EPROBE_DEFER)
@@ -541,7 +528,6 @@ static int da8xx_probe(struct platform_device *pdev)
}
 
glue->dev   = &pdev->dev;
-   glue->clk   = clk;
 
if (IS_ENABLED(CONFIG_OF) && np) {
pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
@@ -587,6 +573,8 @@ static int da8xx_probe(struct platform_device *pdev)
pinfo.data = pdata;
pinfo.size_data = sizeof(*pdata);
 
+   pm_runtime_enable(&pdev->dev);
+
glue->musb = platform_device_register_full(&pinfo);
ret = PTR_ERR_OR_ZERO(glue->musb);
if (ret) {
@@ -603,6 +591,7 @@ static int da8xx_remove(struct platform_device *pdev)
 
platform_device_unregister(glue->musb);
usb_phy_generic_unregister(glue->usb_phy);
+   pm_runtime_disable(&pdev->dev);
 
return 0;
 }
-- 
2.10.2

--
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: v4.9 to v4.10 regression: oops when USB cable is plugged in.

[Pali Rohár]

On Tuesday 24 January 2017 10:18:17 Pavel Machek wrote:
> Hi!
> On Mon 2017-01-23 14:44:54, Tony Lindgren wrote:
> > * Pavel Machek  [170123 14:26]:
> > > [25392.239837] Unhandled fault: external abort on non-linefetch (0x1028) 
> > > at 0xfa0ab060
> > > [25392.239868] pgd = c0004000
> > > [25392.239898] [fa0ab060] *pgd=48011452(bad)
> > > [25392.239929] Internal error: : 1028 [#1] ARM
> > > [25392.239929] Modules linked in:
> > > [25392.239959] CPU: 0 PID: 24322 Comm: kworker/0:1 Not tainted 
> > > 4.10.0-rc5-142127-g41f2839-dirty #222
> > > [25392.239990] Hardware name: Nokia RX-51 board
> > > [25392.240020] Workqueue: events musb_irq_work
> > > [25392.240051] task: cd44d5c0 task.stack: cd308000
> > > [25392.240051] PC is at musb_default_readb+0x0/0xc
> > > [25392.240081] LR is at musb_irq_work+0x1c/0x1b0
> > 
> > OK I'm pretty sure the patch I posted few days ago fixes
> > this. Can you please test patch "[PATCH] usb: musb: Fix
> > external abort on non-linefetch for musb_irq_work()"?
> 
> Can I get the copy of the patch?
> 
> http://www.spinics.net/lists/linux-usb/msg152542.html
> 
> ...but it is html mangled with no obvious way to unmangle it.

Another place when caller of pm_runtime_get_sync forgot to check return
value? This is not first time I see this problem related to Nokia N900!

In past I already suggested to use gcc attribute for pm_runtime_get_sync
to issue warning when caller does not check return value.

> > I was able to hit that only once so far, do you hit it
> > every time with your built-in g_ether .config?
> 
> I get it "way too often", like once a day. I don't yet know how to hit
> it reliably :-(.

-- 
Pali Rohár
pali.ro...@gmail.com
--
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 v3 3/3] dmaengine: cppi41: Fix teardown warnings

[Alexandre Bailon]

During the teardown of a RX channel, because there is only one
completion queue available for RX channel, descriptor of another
channel may be popped which will cause 2 warnings:
- the first one because we popped a wrong descriptor
  (neither the channel's descriptor, nor the teardown descriptor).
- the second one happen during the teardown of another channel,
  because we can't find the channel descriptor
  (that is, the one that caused the first warning).
To avoid that, use one free queue instead of a transmit completion queue.

Note that fix doesn't fix all the teardown warnings:
I still get some when I run some corner case.

Signed-off-by: Alexandre Bailon 
---
 drivers/dma/cppi41.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 5c501da..9fdd824 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -629,7 +629,7 @@ static int cppi41_tear_down_chan(struct cppi41_channel *c)
if (!c->is_tx) {
reg |= GCR_STARV_RETRY;
reg |= GCR_DESC_TYPE_HOST;
-   reg |= c->q_comp_num;
+   reg |= cdd->td_queue.complete;
}
reg |= GCR_TEARDOWN;
cppi_writel(reg, c->gcr_reg);
@@ -640,7 +640,7 @@ static int cppi41_tear_down_chan(struct cppi41_channel *c)
if (!c->td_seen || !c->td_desc_seen) {
 
desc_phys = cppi41_pop_desc(cdd, cdd->td_queue.complete);
-   if (!desc_phys)
+   if (!desc_phys && c->is_tx)
desc_phys = cppi41_pop_desc(cdd, c->q_comp_num);
 
if (desc_phys == c->desc_phys) {
-- 
2.10.2

--
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 v3 2/3] dmaengine: cppi41: Add support of DA8xx to CPPI 4.1

[Alexandre Bailon]

The DA8xx has a CPPI 4.1 DMA controller.
This is add the glue layer required to make it work on DA8xx.

Signed-off-by: Alexandre Bailon 
---
 drivers/dma/Kconfig  |  6 +++---
 drivers/dma/cppi41.c | 23 +++
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 0d6a96e..2a31e1a 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -514,12 +514,12 @@ config TIMB_DMA
  Enable support for the Timberdale FPGA DMA engine.
 
 config TI_CPPI41
-   tristate "AM33xx CPPI41 DMA support"
-   depends on ARCH_OMAP
+   tristate "CPPI 4.1 DMA support"
+   depends on (ARCH_OMAP || ARCH_DAVINCI_DA8XX)
select DMA_ENGINE
help
  The Communications Port Programming Interface (CPPI) 4.1 DMA engine
- is currently used by the USB driver on AM335x platforms.
+ is currently used by the USB driver on AM335x and DA8xx platforms.
 
 config TI_DMA_CROSSBAR
bool
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index 21a4f79..5c501da 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -221,6 +221,20 @@ static const struct chan_queues am335x_usb_queues_rx[] = {
[29] = { .submit = 30, .complete = 155},
 };
 
+static const struct chan_queues da8xx_usb_queues_tx[] = {
+   [0] = { .submit =  16, .complete = 24},
+   [1] = { .submit =  18, .complete = 24},
+   [2] = { .submit =  20, .complete = 24},
+   [3] = { .submit =  22, .complete = 24},
+};
+
+static const struct chan_queues da8xx_usb_queues_rx[] = {
+   [0] = { .submit =  1, .complete = 26},
+   [1] = { .submit =  3, .complete = 26},
+   [2] = { .submit =  5, .complete = 26},
+   [3] = { .submit =  7, .complete = 26},
+};
+
 struct cppi_glue_infos {
const struct chan_queues *queues_rx;
const struct chan_queues *queues_tx;
@@ -949,8 +963,17 @@ static const struct cppi_glue_infos am335x_usb_infos = {
.qmgr_num_pend = 5,
 };
 
+static const struct cppi_glue_infos da8xx_usb_infos = {
+   .queues_rx = da8xx_usb_queues_rx,
+   .queues_tx = da8xx_usb_queues_tx,
+   .td_queue = { .submit = 31, .complete = 0 },
+   .first_completion_queue = 24,
+   .qmgr_num_pend = 2,
+};
+
 static const struct of_device_id cppi41_dma_ids[] = {
{ .compatible = "ti,am3359-cppi41", .data = &am335x_usb_infos},
+   { .compatible = "ti,da830-cppi41", .data = &da8xx_usb_infos},
{},
 };
 MODULE_DEVICE_TABLE(of, cppi41_dma_ids);
-- 
2.10.2

--
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 v3 1/3] dt/bindings: da8xx-usb: Add binding for the CPPI 4.1 DMA controller

[Alexandre Bailon]

DT binding for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx CPPI 4.1 DMA controller.

Signed-off-by: Alexandre Bailon 
---
 .../devicetree/bindings/usb/da8xx-usb.txt  | 39 ++
 1 file changed, 39 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt 
b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
index ccb844a..c3944a6 100644
--- a/Documentation/devicetree/bindings/usb/da8xx-usb.txt
+++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
@@ -18,10 +18,26 @@ Required properties:
 
  - phy-names: Should be "usb-phy"
 
+ - dmas: specifies the dma channels
+
+ - dma-names: specifies the names of the channels. Use "rxN" for receive
+   and "txN" for transmit endpoints. N specifies the endpoint number.
+
 Optional properties:
 
  - vbus-supply: Phandle to a regulator providing the USB bus power.
 
+DMA
+~~~
+- compatible: ti,da830-cppi41
+- reg: offset and length of the following register spaces: CPPI DMA Controller,
+  CPPI DMA Scheduler, Queue Manager
+- reg-names: "controller", "scheduler", "queuemgr"
+- #dma-cells: should be set to 2. The first number represents the
+  channel number (0 … 3 for endpoints 1 … 4).
+  The second number is 0 for RX and 1 for TX transfers.
+- #dma-channels: should be set to 4 representing the 4 endpoints.
+
 Example:
usb_phy: usb-phy {
compatible = "ti,da830-usb-phy";
@@ -31,6 +47,9 @@ Example:
usb0: usb@20 {
compatible = "ti,da830-musb";
reg =   <0x0020 0x1>;
+   ranges;
+   #address-cells = <1>;
+   #size-cells = <1>;
interrupts = <58>;
interrupt-names = "mc";
 
@@ -39,5 +58,25 @@ Example:
phys = <&usb_phy 0>;
phy-names = "usb-phy";
 
+   dmas = <&cppi41dma 0 0 &cppi41dma 1 0
+   &cppi41dma 2 0 &cppi41dma 3 0
+   &cppi41dma 0 1 &cppi41dma 1 1
+   &cppi41dma 2 1 &cppi41dma 3 1>;
+   dma-names =
+   "rx1", "rx2", "rx3", "rx4",
+   "tx1", "tx2", "tx3", "tx4";
+
status = "okay";
+
+   cppi41dma: dma-controller@201000 {
+   compatible = "ti,da830-cppi41";
+   reg =  <0x201000 0x1000
+   0x202000 0x1000
+   0x204000 0x4000>;
+   reg-names = "controller", "scheduler", "queuemgr";
+   interrupts = <58>;
+   #dma-cells = <2>;
+   #dma-channels = <4>;
+   };
+
};
-- 
2.10.2

--
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 v3 0/3] dmaengine: cppi41: Add dma support to da8xx

[Alexandre Bailon]

This series add support of DA8xx to CPPI 4.1 driver.
As the CPPI 4.1 is now generic, we only had to add the glue for DA8xx.

This serie should applied on top of 
"[PATCH v3 0/7] dmaengine: cppi41: Make CPPI 4.1 driver more generic".

Changes in v3:
 - Fix few typos
 - Don't use a wildcard for the compatible property

Changes in v2:
- most of patches of v1 has been moved to the series
  "[PATCH v2 0/7] dmaengine: cppi41: Make CPPI 4.1 driver more generic".
- some patches of v1 has been removed because they were no required
  anymore because CPPI 4.1 driver has been made more generic.
- In v1, the driver were managing the clock for DA8xx paltform.
  This is not needed as CPPI 4.1 will be a child of MUSB DA8xx glue,
  we can use PM runtime and let the DA8xx glue driver manage it.

Alexandre Bailon (3):
  dt/bindings: da8xx-usb: Add binding for the CPPI 4.1 DMA controller
  dmaengine: cppi41: Add support of DA8xx to CPPI 4.1
  dmaengine: cppi41: Fix teardown warnings

 .../devicetree/bindings/usb/da8xx-usb.txt  | 39 ++
 drivers/dma/Kconfig|  6 ++--
 drivers/dma/cppi41.c   | 27 +--
 3 files changed, 67 insertions(+), 5 deletions(-)

-- 
2.10.2

--
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

[RESENT PATCH net,stable] qmi_wwan/cdc_ether: add device ID for HP lt2523 (Novatel E371) WWAN card

[Bjørn Mork]

Another rebranded Novatel E371.  qmi_wwan should drive this device, while
cdc_ether should ignore it.  Even though the USB descriptors are plain
CDC-ETHER that USB interface is a QMI interface.  Ref commit 7fdb7846c9ca
("qmi_wwan/cdc_ether: add device IDs for Dell 5804 (Novatel E371) WWAN
card")

Cc: Dan Williams 
Signed-off-by: Bjørn Mork 
---
Resending with correct author and to address. I didn't manage to spell
kernel correctly... I guess I should have had one more cup of coffee
this morning.

No changes to the diff.


 drivers/net/usb/cdc_ether.c | 8 
 drivers/net/usb/qmi_wwan.c  | 7 +++
 2 files changed, 15 insertions(+)

diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index fe7b2886cb6b..86144f9a80ee 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -531,6 +531,7 @@ static const struct driver_info wwan_info = {
 #define SAMSUNG_VENDOR_ID  0x04e8
 #define LENOVO_VENDOR_ID   0x17ef
 #define NVIDIA_VENDOR_ID   0x0955
+#define HP_VENDOR_ID   0x03f0
 
 static const struct usb_device_id  products[] = {
 /* BLACKLIST !!
@@ -677,6 +678,13 @@ static const struct usb_device_id  products[] = {
.driver_info = 0,
 },
 
+/* HP lt2523 (Novatel E371) - handled by qmi_wwan */
+{
+   USB_DEVICE_AND_INTERFACE_INFO(HP_VENDOR_ID, 0x421d, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_ETHERNET, 
USB_CDC_PROTO_NONE),
+   .driver_info = 0,
+},
+
 /* AnyDATA ADU960S - handled by qmi_wwan */
 {
USB_DEVICE_AND_INTERFACE_INFO(0x16d5, 0x650a, USB_CLASS_COMM,
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 6fe1cdb0174f..24d5272cdce5 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -654,6 +654,13 @@ static const struct usb_device_id products[] = {
  USB_CDC_PROTO_NONE),
.driver_info= (unsigned long)&qmi_wwan_info,
},
+   {   /* HP lt2523 (Novatel E371) */
+   USB_DEVICE_AND_INTERFACE_INFO(0x03f0, 0x421d,
+ USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_ETHERNET,
+ USB_CDC_PROTO_NONE),
+   .driver_info= (unsigned long)&qmi_wwan_info,
+   },
{   /* HP lt4112 LTE/HSPA+ Gobi 4G Module (Huawei me906e) */
USB_DEVICE_AND_INTERFACE_INFO(0x03f0, 0x581d, 
USB_CLASS_VENDOR_SPEC, 1, 7),
.driver_info = (unsigned long)&qmi_wwan_info,
-- 
2.11.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: usb: gadget: Kernel panic (NULL pointer dereference) when using fsl_udc2_core on i.MX31 PDK

[Magnus Lilja]

On 24 January 2017 at 09:52, Felipe Balbi  wrote:
>
> Hi,
>
> Magnus Lilja  writes:
 I tried the fsl_udc_core gadget driver on the i.MX31 PDK board and got a
 kernel panic (NULL pointer dereference) when connecting the USB cable. I
 had the g_serial module loaded as well.

 The NULL pointer panic comes from gadget/udc/core.c
 usb_gadget_giveback_request() which calls req->complete() and in some
 cases req->complete is NULL.

 Commit 304f7e5e1d08 ("usb: gadget: Refactor request completion") changed
 fsl_udc2_core.c (and several other files) and in fsl_udc2_core.c a check
 that req->complete is non-NULL was removed:

 --- a/drivers/usb/gadget/udc/fsl_udc_core.c
 +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
 @@ -197,10 +197,8 @@ __acquires(ep->udc->lock)
  ep->stopped = 1;

  spin_unlock(&ep->udc->lock);
 -   /* complete() is from gadget layer,
 -* eg fsg->bulk_in_complete() */
 -   if (req->req.complete)
 -   req->req.complete(&ep->ep, &req->req);
 +
 +   usb_gadget_giveback_request(&ep->ep, &req->req);

  spin_lock(&ep->udc->lock);
  ep->stopped = stopped;

 If I re-introduce the check (either in fsl_udc_core.c or core.c) at
 least USB gadget operation using g_serial seems to work just fine.

 I don't know the logic in detail to understand whether this is a proper
 fix or if there is some other more problem with the fls_udc_core driver.
 Does anyone have input in this matter?

 I can produce a proper patch that fixes this problem by re-introducing
 the check (in either fsl_udc_core.c or core.c) if that is a proper
 solution and I can also assist in testing other fixes to the problem.
>>>
>>> ->complete() is supposed to be mandatory. Which gadget do you have that
>>> ->doesn't set ->complete() to a valid function pointer?
>>
>> I'm modprobing g_serial so the following modules are loaded (using my patch):
>>
>> ~ # lsmod
>> usb_f_acm
>> u_serial
>> g_serial
>> libcomposite
>> configfs
>> fsl_usb2_udc
>
> okay, can you figure out which request is coming without ->complete()
> set? To which endpoint is this request being queued? It would be nice to
> know these details. Maybe this is an old bug which ought to be fixed.

Sure, I can try figure that out. Any input to make the debug of the
faster is appreciated if you have any.

Regards, Magnus
--
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,stable] qmi_wwan/cdc_ether: add device ID for HP lt2523 (Novatel E371) WWAN card

[Bjørn Mork]

Bjørn Mork  writes:

> From: Dan Williams 

Woops! I didn't intend to blame this on you Dan.  Sorry. I reused your
commit message, and obviously unintentionally also the "author" line...

But you did create the pattern, so it was only fair to give you the
credit :)



Bjørn
--
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] USB: option: add device ID for HP lt2523 (Novatel E371)

[Bjørn Mork]

Yet another laptop vendor rebranded Novatel E371.

Cc: sta...@vger.kernel.org
Signed-off-by: Bjørn Mork 
---
 drivers/usb/serial/option.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 7ce31a4c7e7f..42cc72e54c05 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -2007,6 +2007,7 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE_AND_INTERFACE_INFO(WETELECOM_VENDOR_ID, 
WETELECOM_PRODUCT_WMD200, 0xff, 0xff, 0xff) },
{ USB_DEVICE_AND_INTERFACE_INFO(WETELECOM_VENDOR_ID, 
WETELECOM_PRODUCT_6802, 0xff, 0xff, 0xff) },
{ USB_DEVICE_AND_INTERFACE_INFO(WETELECOM_VENDOR_ID, 
WETELECOM_PRODUCT_WMD300, 0xff, 0xff, 0xff) },
+   { USB_DEVICE_AND_INTERFACE_INFO(0x03f0, 0x421d, 0xff, 0xff, 0xff) }, /* 
HP lt2523 (Novatel E371) */
{ } /* Terminating entry */
 };
 MODULE_DEVICE_TABLE(usb, option_ids);
-- 
2.11.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 net,stable] qmi_wwan/cdc_ether: add device ID for HP lt2523 (Novatel E371) WWAN card

[Bjørn Mork]

From: Dan Williams 

Another rebranded Novatel E371.  qmi_wwan should drive this device, while
cdc_ether should ignore it.  Even though the USB descriptors are plain
CDC-ETHER that USB interface is a QMI interface.  Ref commit 7fdb7846c9ca
("qmi_wwan/cdc_ether: add device IDs for Dell 5804 (Novatel E371) WWAN
card")

Signed-off-by: Bjørn Mork 
---
 drivers/net/usb/cdc_ether.c | 8 
 drivers/net/usb/qmi_wwan.c  | 7 +++
 2 files changed, 15 insertions(+)

diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index fe7b2886cb6b..86144f9a80ee 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -531,6 +531,7 @@ static const struct driver_info wwan_info = {
 #define SAMSUNG_VENDOR_ID  0x04e8
 #define LENOVO_VENDOR_ID   0x17ef
 #define NVIDIA_VENDOR_ID   0x0955
+#define HP_VENDOR_ID   0x03f0
 
 static const struct usb_device_id  products[] = {
 /* BLACKLIST !!
@@ -677,6 +678,13 @@ static const struct usb_device_id  products[] = {
.driver_info = 0,
 },
 
+/* HP lt2523 (Novatel E371) - handled by qmi_wwan */
+{
+   USB_DEVICE_AND_INTERFACE_INFO(HP_VENDOR_ID, 0x421d, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_ETHERNET, 
USB_CDC_PROTO_NONE),
+   .driver_info = 0,
+},
+
 /* AnyDATA ADU960S - handled by qmi_wwan */
 {
USB_DEVICE_AND_INTERFACE_INFO(0x16d5, 0x650a, USB_CLASS_COMM,
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 6fe1cdb0174f..24d5272cdce5 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -654,6 +654,13 @@ static const struct usb_device_id products[] = {
  USB_CDC_PROTO_NONE),
.driver_info= (unsigned long)&qmi_wwan_info,
},
+   {   /* HP lt2523 (Novatel E371) */
+   USB_DEVICE_AND_INTERFACE_INFO(0x03f0, 0x421d,
+ USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_ETHERNET,
+ USB_CDC_PROTO_NONE),
+   .driver_info= (unsigned long)&qmi_wwan_info,
+   },
{   /* HP lt4112 LTE/HSPA+ Gobi 4G Module (Huawei me906e) */
USB_DEVICE_AND_INTERFACE_INFO(0x03f0, 0x581d, 
USB_CLASS_VENDOR_SPEC, 1, 7),
.driver_info = (unsigned long)&qmi_wwan_info,
-- 
2.11.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 v7 3/5] phy: Add set_vbus callback

[Kishon Vijay Abraham I]

Hi,

On Tuesday 24 January 2017 01:28 AM, Stephen Boyd wrote:
> Quoting Kishon Vijay Abraham I (2017-01-22 00:46:21)
>> Hi,
>>
>> On Saturday 21 January 2017 12:20 AM, Stephen Boyd wrote:
>>> Some USB PHYs need to be told about vbus changing state
>>> explicitly. For example the qcom USB HS PHY needs to toggle a bit
>>> when vbus goes from low to high (VBUSVLDEXT) to cause the
>>> "session valid" signal to toggle. This signal will pull up D+
>>> when the phy starts running. If the vbus signal isn't routed to
>>> the PHY this "session valid" signal won't ever toggle, so we have
>>> to toggle it explicitly. This callback is used to do that.
>>>
>>> Cc: Peter Chen 
>>> Signed-off-by: Stephen Boyd 
>>> ---
>>>
>>> New patch
>>>
>>>  drivers/phy/phy-core.c  | 15 +++
>>>  include/linux/phy/phy.h | 10 ++
>>>  2 files changed, 25 insertions(+)
>>>
>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>>> index a268f4d6f3e9..8b1a6bfa5133 100644
>>> --- a/drivers/phy/phy-core.c
>>> +++ b/drivers/phy/phy-core.c
>>> @@ -357,6 +357,21 @@ int phy_set_mode(struct phy *phy, enum phy_mode mode)
>>>  }
>>>  EXPORT_SYMBOL_GPL(phy_set_mode);
>>>  
>>> +int phy_set_vbus(struct phy *phy, int on)
>>> +{
>>> + int ret;
>>> +
>>> + if (!phy || !phy->ops->set_vbus)
>>> + return 0;
>>> +
>>> + mutex_lock(&phy->mutex);
>>> + ret = phy->ops->set_vbus(phy, on);
>>> + mutex_unlock(&phy->mutex);
>>> +
>>> + return ret;
>>> +}
>>> +EXPORT_SYMBOL_GPL(phy_set_vbus);
>>> +
>>>  int phy_reset(struct phy *phy)
>>>  {
>>>   int ret;
>>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
>>> index 78bb0d7f6b11..4d1ebde7fb14 100644
>>> --- a/include/linux/phy/phy.h
>>> +++ b/include/linux/phy/phy.h
>>> @@ -36,6 +36,7 @@ enum phy_mode {
>>>   * @power_on: powering on the phy
>>>   * @power_off: powering off the phy
>>>   * @set_mode: set the mode of the phy
>>> + * @set_vbus: enable/disable vbus in the phy (USB)
>>>   * @reset: resetting the phy
>>>   * @owner: the module owner containing the ops
>>>   */
>>> @@ -45,6 +46,7 @@ struct phy_ops {
>>>   int (*power_on)(struct phy *phy);
>>>   int (*power_off)(struct phy *phy);
>>>   int (*set_mode)(struct phy *phy, enum phy_mode mode);
>>> + int (*set_vbus)(struct phy *phy, int on);
>>
>> please avoid adding usb specific ops in generic phy ops.
>>
> 
> Is there any alternative? Something has to happen here.
> 
> The only other thing I can think of is putting back the vbus extcon in
> the phy driver so it can be notified when vbus is present or not. I can

I think extcon should be used here to get vbus notification.

Thanks
Kishon
--
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: v4.9 to v4.10 regression: oops when USB cable is plugged in.

[Pavel Machek]

Hi!
On Mon 2017-01-23 14:44:54, Tony Lindgren wrote:
> * Pavel Machek  [170123 14:26]:
> > [25392.239837] Unhandled fault: external abort on non-linefetch (0x1028) at 
> > 0xfa0ab060
> > [25392.239868] pgd = c0004000
> > [25392.239898] [fa0ab060] *pgd=48011452(bad)
> > [25392.239929] Internal error: : 1028 [#1] ARM
> > [25392.239929] Modules linked in:
> > [25392.239959] CPU: 0 PID: 24322 Comm: kworker/0:1 Not tainted 
> > 4.10.0-rc5-142127-g41f2839-dirty #222
> > [25392.239990] Hardware name: Nokia RX-51 board
> > [25392.240020] Workqueue: events musb_irq_work
> > [25392.240051] task: cd44d5c0 task.stack: cd308000
> > [25392.240051] PC is at musb_default_readb+0x0/0xc
> > [25392.240081] LR is at musb_irq_work+0x1c/0x1b0
> 
> OK I'm pretty sure the patch I posted few days ago fixes
> this. Can you please test patch "[PATCH] usb: musb: Fix
> external abort on non-linefetch for musb_irq_work()"?

Can I get the copy of the patch?

http://www.spinics.net/lists/linux-usb/msg152542.html

...but it is html mangled with no obvious way to unmangle it.

> I was able to hit that only once so far, do you hit it
> every time with your built-in g_ether .config?

I get it "way too often", like once a day. I don't yet know how to hit
it reliably :-(.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v4 0/6] usb: musb: cppi41: Add a way to manage DMA irq

[Alexandre Bailon]

On 01/23/2017 10:26 PM, Bin Liu wrote:
> On Mon, Jan 23, 2017 at 05:48:02PM +0100, Alexandre Bailon wrote:
> > This series was "dmaengine: cppi41: Make the driver more generic".
> > I have tried to separate as munch I could CPPI 4.1 MUSB driver changes.
> >
> > Currently, the DMA interrupt is managed by the CPPI 4.1 driver.
> > The issue here is the CPPI 4.1 driver must access to MUSB glue registers
> > to manage its interrupt.
> > In order to move the interrupts management from CPPI 4.1 driver to MUSB
> > (and then make it more generic), update the MUSB CPPI 4.1 driver with
> > changes that will help to manage DMA interrupt from MUSB driver.
> >
> > Changes in v4:
> > - Remove musb pointer from struct cppi, cppi41_dma_controller and
> >   tusb_omap_dma.
> >
> > Changes in v3:
> > - Move a patch from another series to this one to avoid build error report
> >   from kbuild test robot
> > - Instead of adding and exporting function, add one callback and a pointer
> >   to musb in struct dma_controller
> > - Surround the DMA function introduced in musb_dsps with #ifdef / #endif.
> >
> > Changes in v2:
> > - Fix some typo in commit messages
> > - Add more explanation about some changes made by patch 2 in commit message
> >
> > Alexandre Bailon (6):
> >   usb: musb: dma: Add a DMA completion platform callback
> >   usb: musb: cppi41: Detect aborted transfers in cppi41_dma_callback()
> >   usb: musb: cppi_dma: Clean up cppi structure
> >   usb: musb: cppi_dma: Clean up cppi41_dma_controller structure
> >   usb: musb: cppi_dma: Clean up tusb_omap_dma structure
> >   usb: musb: dsps: Manage CPPI 4.1 DMA interrupt in DSPS
> >
> >  drivers/dma/cppi41.c | 28 --
> >  drivers/usb/musb/cppi_dma.c  | 26 ++---
> >  drivers/usb/musb/cppi_dma.h  |  1 -
> >  drivers/usb/musb/musb_cppi41.c   | 48 +---
> >  drivers/usb/musb/musb_dma.h  |  5 +++
> >  drivers/usb/musb/musb_dsps.c | 81 
> > +++-
> >  drivers/usb/musb/tusb6010_omap.c |  7 ++--
> >  7 files changed, 134 insertions(+), 62 deletions(-)
> >
>
> Please keep linux-usb list cc'd.
Sorry about that. I will be more careful.
>
> Regards,
> -Bin.
>
Regards,
Alexandre
--
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: functionfs on dwc3, xhci host: endpoint cannot be used in both directions ?

[Felipe Balbi]

Hi,

Vincent Pelletier  writes:
> Hello,
>
> On Tue, Jan 24, 2017 at 12:20 AM, Felipe Balbi
>  wrote:
>> Seems like HW engineer wanted these last few endpoints to *not* support
>> full USB3 packets. They are probably supposed to be used for Isochronous
>> and/or Interrupts endpoints. At some point we need to support this as
>> well. During initialization we should read FIFO size configuration and
>> extract wMaxPacketSize for $endpoint from the HW.
>
> If my understanding of your register explanation is correct, the >9 IN
> endpoints could still work as bulk in LS/FS speeds (I don't have the
> board at hand to try right now). If this is correct, won't your patch
> (in your testing/next branch) make these endpoints unusable for bulk
> use even in LS/FS ?

that's correct. Maybe I should always set bulk capability. Thanks for
catching that, I'll send v2 shortly.

> A bit more generally, I have the feeling (from reading epautoconf.c
> and f_fs.c) that the endpoint dispatching (and hence, endpoint
> capabilities) lacks the notion of speed (SS has it, kind of, via the
> companion descriptor argument in epautoconf.c). I noticed something a
> few weeks back which may come from this lack: when I tried only
> populating the HS descriptors, the host xHCI would complain about
> non-standard endpoint size (64B instead of HS-required 512B). In my
> understanding, this is because f_fs.c allocates endpoints using the
> first populated descriptor set (in LS/FS, then HS, then SS order), and
> epautoconf.c overwriting the buffer size to 64 on non-SS bulk
> descriptors.

yeah, order matters. That's documented for f_fs. 

> I think extending such API is over my head still. Do you have ideas on
> this ?

Well, we can't easily change the way it works because it's an ABI to
userspace. I would have very much preferred for us to pass descriptors
using ioctl(). That way we could have:

ioctl(fd, FFS_IOCTL_SS_EP_DESC, ep1_ss_desc);
ioctl(fd, FFS_IOCTL_SS_EP_COMP_DESC, ep1_ss_comp_desc);

and so on.

> FWIW, the Intel Edison (the board I'm using as device) is not USB 3
> capable despite dwc3 usage. If my undrstanding is correct, it is
> because it lacks the USB 3 companion phy, and of course does not

right

> expose the corresponding tracks on the expansion connector (would it
> be possible to host the companion outside the edison module ? I have
> no idea how it is supposed to interract with the dwc3 and USB 2 phy).

I'm pretty sure PIPE3 interface doesn't go outside the SoC, so no.

One thing you _can_ try, however, is to pass maximum-speed property to
tell dwc3 you wanna run in high-speed, not super.

-- 
balbi


signature.asc
Description: PGP signature

Re: usb: gadget: Kernel panic (NULL pointer dereference) when using fsl_udc2_core on i.MX31 PDK

[Felipe Balbi]

Hi,

Magnus Lilja  writes:
>>> I tried the fsl_udc_core gadget driver on the i.MX31 PDK board and got a
>>> kernel panic (NULL pointer dereference) when connecting the USB cable. I
>>> had the g_serial module loaded as well.
>>>
>>> The NULL pointer panic comes from gadget/udc/core.c
>>> usb_gadget_giveback_request() which calls req->complete() and in some
>>> cases req->complete is NULL.
>>>
>>> Commit 304f7e5e1d08 ("usb: gadget: Refactor request completion") changed
>>> fsl_udc2_core.c (and several other files) and in fsl_udc2_core.c a check
>>> that req->complete is non-NULL was removed:
>>>
>>> --- a/drivers/usb/gadget/udc/fsl_udc_core.c
>>> +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
>>> @@ -197,10 +197,8 @@ __acquires(ep->udc->lock)
>>>  ep->stopped = 1;
>>>
>>>  spin_unlock(&ep->udc->lock);
>>> -   /* complete() is from gadget layer,
>>> -* eg fsg->bulk_in_complete() */
>>> -   if (req->req.complete)
>>> -   req->req.complete(&ep->ep, &req->req);
>>> +
>>> +   usb_gadget_giveback_request(&ep->ep, &req->req);
>>>
>>>  spin_lock(&ep->udc->lock);
>>>  ep->stopped = stopped;
>>>
>>> If I re-introduce the check (either in fsl_udc_core.c or core.c) at
>>> least USB gadget operation using g_serial seems to work just fine.
>>>
>>> I don't know the logic in detail to understand whether this is a proper
>>> fix or if there is some other more problem with the fls_udc_core driver.
>>> Does anyone have input in this matter?
>>>
>>> I can produce a proper patch that fixes this problem by re-introducing
>>> the check (in either fsl_udc_core.c or core.c) if that is a proper
>>> solution and I can also assist in testing other fixes to the problem.
>>
>> ->complete() is supposed to be mandatory. Which gadget do you have that
>> ->doesn't set ->complete() to a valid function pointer?
>
> I'm modprobing g_serial so the following modules are loaded (using my patch):
>
> ~ # lsmod
> usb_f_acm
> u_serial
> g_serial
> libcomposite
> configfs
> fsl_usb2_udc

okay, can you figure out which request is coming without ->complete()
set? To which endpoint is this request being queued? It would be nice to
know these details. Maybe this is an old bug which ought to be fixed.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v2] usb: dwc3: handle DWC_USB3_NUM == DWC_USB3_NUM_IN_EPS

[Felipe Balbi]

Hi,

John Youn  writes:
>>> Bryan O'Donoghue  writes:
 - DWC_USB3_NUM indicates the number of Device mode single directional
   endpoints, including OUT and IN endpoint 0.

 - DWC_USB3_NUM_IN_EPS indicates the maximum number of Device mode IN
   endpoints active at any time, including control endpoint 0.

 It's possible to configure RTL such that DWC_USB3_NUM_EPS is equal to
 DWC_USB3_NUM_IN_EPS.

 dwc3-core calculates the number of OUT endpoints as DWC_USB3_NUM minus
 DWC_USB3_NUM_IN_EPS. If RTL has been configured with DWC_USB3_NUM_IN_EPS
 equal to DWC_USB3_NUM then dwc3-core will calculate the number of OUT
 endpoints as zero.

 For example a from dwc3_core_num_eps() shows:
 [1.565000]  /usb0@f01d: found 8 IN and 0 OUT endpoints

 This patch works around this case by detecting when DWC_USB3_NUM_EPS is
 equal to DWC3_USB3_NUM_IN_EPS and over-rides the calculation of the number
 of OUT and IN endpoints to make dwc->num_in_eps equal to half
 DWC_USB3_NUM_EPS.

 If DWC_USB3_NUM_EPS is equal to DWC3_USB3_NUM_IN_EPS and the endpoint count
 is an odd number then dwc->num_out_eps will be assigned the extra endpoint.
>>>
>>> sorry, now that I spent some more time with this. Isn't something like
>>> below solving all problems?
>>>
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index 369bab16a824..68c9c84b7216 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -397,8 +397,7 @@ static void dwc3_core_num_eps(struct dwc3 *dwc)
>>>  {
>>> struct dwc3_hwparams*parms = &dwc->hwparams;
>>>
>>> -   dwc->num_in_eps = DWC3_NUM_IN_EPS(parms);
>>> -   dwc->num_out_eps = DWC3_NUM_EPS(parms) - dwc->num_in_eps;
>>> +   dwc->num_eps = DWC3_NUM_EPS(parms);
>>>  }
>>>
>>>  static void dwc3_cache_hwparams(struct dwc3 *dwc)
>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>>> index 0a664d8eba3f..8c187df0aa42 100644
>>> --- a/drivers/usb/dwc3/gadget.c
>>> +++ b/drivers/usb/dwc3/gadget.c
>>> @@ -2001,23 +2002,7 @@ static int dwc3_gadget_init_hw_endpoints(struct dwc3 
>>> *dwc,
>>>
>>>  static int dwc3_gadget_init_endpoints(struct dwc3 *dwc)
>>>  {
>>> -   int ret;
>>> -
>>> -   INIT_LIST_HEAD(&dwc->gadget.ep_list);
>>> -
>>> -   ret = dwc3_gadget_init_hw_endpoints(dwc, dwc->num_out_eps, 0);
>>> -   if (ret < 0) {
>>> -   dev_err(dwc->dev, "failed to initialize OUT endpoints\n");
>>> -   return ret;
>>> -   }
>>> -
>>> -   ret = dwc3_gadget_init_hw_endpoints(dwc, dwc->num_in_eps, 1);
>>> -   if (ret < 0) {
>>> -   dev_err(dwc->dev, "failed to initialize IN endpoints\n");
>>> -   return ret;
>>> -   }
>>> -
>>> -   return 0;
>>> +   return dwc3_gadget_init_hw_endpoints(dwc, dwc->num_eps);
>>
>> Well I hadn't considered that level of change myself but, it should work.
>>
>
> It should work for ASICS.
>
> But it won't work for our FPGA platform. It needs to work the
> existing, more limited, way. You can check this with the
> GHWPARAMS6.EN_FPGA.

Can you clarify a little more? Why wouldn't above work for FPGA?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] usb: dwc2: fix "iomem 0x00000000" message by setting iomem parameters in usb_hcd

[Felipe Balbi]

Hi,

John Youn  writes:
>> John Youn  writes:
>>> @@ -1229,7 +1229,8 @@ static inline void dwc2_hcd_connect(struct 
>>> dwc2_hsotg *hsotg) {}
>>>  static inline void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool 
>>> force) {}
>>>  static inline void dwc2_hcd_start(struct dwc2_hsotg *hsotg) {}
>>>  static inline void dwc2_hcd_remove(struct dwc2_hsotg *hsotg) {}
>>> -static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
>>> +static inline int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
>>> +   struct resource *res)
>>>  { return 0; }
>>>  static inline int dwc2_backup_host_registers(struct dwc2_hsotg *hsotg)
>>>  { return 0; }
>>> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
>>> index 911c3b36..2cfbd10e 100644
>>> --- a/drivers/usb/dwc2/hcd.c
>>> +++ b/drivers/usb/dwc2/hcd.c
>>> @@ -4964,7 +4964,7 @@ static void dwc2_hcd_release(struct dwc2_hsotg 
>>> *hsotg)
>>>   * USB bus with the core and calls the hc_driver->start() function. It 
>>> returns
>>>   * a negative error on failure.
>>>   */
>>> -int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq)
>>> +int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq, struct resource 
>>> *res)
>>>  {
>>> struct usb_hcd *hcd;
>>> struct dwc2_host_chan *channel;
>>> @@ -5021,6 +5021,9 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int 
>>> irq)
>>>
>>> hcd->has_tt = 1;
>>>
>>> +   hcd->rsrc_start = res->start;
>>> +   hcd->rsrc_len = resource_size(res);
>>> +
>>> ((struct wrapper_priv_data *) &hcd->hcd_priv)->hsotg = hsotg;
>>> hsotg->priv = hcd;
>>>
>>> diff --git a/drivers/usb/dwc2/hcd.h b/drivers/usb/dwc2/hcd.h
>>> index 1ed5fa2b..2305b5fb 100644
>>> --- a/drivers/usb/dwc2/hcd.h
>>> +++ b/drivers/usb/dwc2/hcd.h
>>> @@ -521,7 +521,8 @@ static inline u8 dwc2_hcd_is_pipe_out(struct 
>>> dwc2_hcd_pipe_info *pipe)
>>> return !dwc2_hcd_is_pipe_in(pipe);
>>>  }
>>>
>>> -extern int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq);
>>> +extern int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq,
>>> +struct resource *res);
>>>  extern void dwc2_hcd_remove(struct dwc2_hsotg *hsotg);
>>>
>>>  /* Transaction Execution Functions */
>>> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
>>> index 4fc8c603..5ddc8860 100644
>>> --- a/drivers/usb/dwc2/platform.c
>>> +++ b/drivers/usb/dwc2/platform.c
>>> @@ -445,7 +445,7 @@ static int dwc2_driver_probe(struct platform_device 
>>> *dev)
>>> }
>>>
>>> if (hsotg->dr_mode != USB_DR_MODE_PERIPHERAL) {
>>> -   retval = dwc2_hcd_init(hsotg, hsotg->irq);
>>> +   retval = dwc2_hcd_init(hsotg, hsotg->irq, res);
>>
>> This is a good idea, but there's more work that can come out of this, if
>> you're interested:
>>
>> i) devm_ioremap_resource() could be called by the generic layer
>> ii) devm_request_irq() could be move to the generic layer
>> iii) dwc2_hcd_init() could also be called generically as long as dr_mode
>>  is set properly.
>> iv) dwc2_debugfs_init() could be called generically as well
>>
>> IOW, dwc2_driver_probe() could be as minimal as:
>>
>> static int dwc2_driver_probe(struct platform_device *dev)
>> {
>>  struct dwc2_hsotg *hsotg;
>>  struct resource *res;
>>  int retval;
>>
>>  hsotg = devm_kzalloc(&dev->dev, sizeof(*hsotg), GFP_KERNEL);
>>  if (!hsotg)
>>  return -ENOMEM;
>>
>>  hsotg->dev = &dev->dev;
>>
>>  if (!dev->dev.dma_mask)
>>  dev->dev.dma_mask = &dev->dev.coherent_dma_mask;
>>
>>  retval = dma_set_coherent_mask(&dev->dev, DMA_BIT_MASK(32));
>>  if (retval)
>>  return retval;
>>
>>  hsotg->res = platform_get_resource(dev, IORESOURCE_MEM, 0);
>>  hsotg->irq = platform_get_irq(dev, 0);
>>
>>  retval = dwc2_get_dr_mode(hsotg);
>>  if (retval)
>>  return retval;
>>
>>  retval = dwc2_get_hwparams(hsotg);
>>  if (retval)
>>  return retval;
>>
>>  platform_set_drvdata(dev, hsotg);
>>
>>  retval = dwc2_core_init(hsotg);
>>  if (retval)
>>  return retval;
>>
>>  return 0;
>> }
>>
>> dwc2_core_init() needs to implemented, of course, but it could hide all
>> details about what to do with IRQs and resources and what not. Assuming
>> you can properly initialize clocks, it could even handle clocks
>> generically for you.
>>
> I see what you mean. I'm just a little confused about the term "generic" 
> here:
> For me something generic has more than one user. Here we seem to speak 
> just

Re: [PATCH] USB: Add quirk for WORLDE easykey.25 MIDI keyboard

[Lukáš Lalinský]

On Tue, Jan 24, 2017 at 8:37 AM, Lukáš Lalinský  wrote:
> On Tue, Jan 24, 2017 at 8:32 AM, Oliver Neukum  wrote:
>> Am Montag, den 23.01.2017, 19:36 +0100 schrieb Lukáš Lalinský:
>>> I have uploaded both captures here -
>>> https://gist.github.com/lalinsky/83148a827d5cd43e79e377d8e1b5ed0d
>>
>> Indeed it is does not set a configuration. Either the capture
>> is incomplete or device and host violate the standard. A device
>> may be left unconfigured.
>
> Is this may or may not? I'm not familiar with USB, so I assumed if
> there is only one configuration and there is always one active, it
> does not need to be set explicitly because the correct one is already
> active.
>
>> We need to read the descriptors even if we
>> see only one configuration to get the power budgeting right.
>
> Aren't those in the CONFIGURATION descriptors? Reading the STRING
> descriptor is probably only useful if you need to print the
> configuration details somewhere.

I re-ran the capture on a Windows 7 host. The previous capture was
missing data, probably due to interactions of the Linux host and
Windows VM.

https://gist.github.com/lalinsky/2ec7d74b049b448b1f7032d8861ca4a2

It does set the configuration, but does not request the string descriptor.

Lukas
--
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: Add quirk for WORLDE easykey.25 MIDI keyboard

[Oliver Neukum]

Am Dienstag, den 24.01.2017, 08:37 +0100 schrieb Lukáš Lalinský:
> On Tue, Jan 24, 2017 at 8:32 AM, Oliver Neukum 
> wrote:
> > 
> > Am Montag, den 23.01.2017, 19:36 +0100 schrieb Lukáš Lalinský:
> > > 
> > > I have uploaded both captures here -
> > > https://gist.github.com/lalinsky/83148a827d5cd43e79e377d8e1b5ed0d
> > 
> > Indeed it is does not set a configuration. Either the capture
> > is incomplete or device and host violate the standard. A device
> > may be left unconfigured.
> 
> Is this may or may not? I'm not familiar with USB, so I assumed if

You can leave a device unconfigured. In that case it stays in the
addressed state, where its power draw is strictly limited.
That is exactly what the kernel does when it is confronted with
a device that would overdraw the power budget.

> there is only one configuration and there is always one active, it
> does not need to be set explicitly because the correct one is already
> active.

No, it is not. Or rather it should not be. Can you please recheck
that you are capturing the whole exchange?

> > We need to read the descriptors even if we
> > see only one configuration to get the power budgeting right.
> 
> Aren't those in the CONFIGURATION descriptors? Reading the STRING

True.

> descriptor is probably only useful if you need to print the
> configuration details somewhere.

Also true.

> > 
> > Does the device work without any .ini file?
> 
> Yes. It's a standard MIDI device, no configuration is required.

So Windows by default does not read string descriptors. That is worth
a thought, but we need to check. Yet I am not sure how useful that is.
The first lsusb would crash the device. We'd need a quirk anyway.

Regards
Oliver

--
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 v5 1/4] usb: dbc: early driver for xhci debug capability

[Ingo Molnar]

* Lu Baolu  wrote:

> Hi Ingo,
> 
> On 01/22/2017 05:04 PM, Ingo Molnar wrote:
> > * Lu Baolu  wrote:
> >
>  +static void xdbc_runtime_delay(unsigned long count)
>  +{
>  +udelay(count);
>  +}
>  +static void (*xdbc_delay)(unsigned long) = xdbc_early_delay;
> >>> Is this udelay() complication really necessary? udelay() should work fine 
> >>> even in 
> >>> early code. It might not be precisely calibrated, but should be good 
> >>> enough.
> >> I tried udelay() in the early code. It's not precise enough for the
> >> hardware handshaking.
> > Possibly because on x86 early udelay() did not work at all - i.e. there's 
> > no delay 
> > whatsoever.
> 
> Yes.
> 
> >
> > Could you try it on top of this commit in tip:timers/core:
> >
> >   4c45c5167c95 x86/timer: Make delay() work during early bootup
> >
> > ?
> 
> I tried tip:timers/core. It's not precise enough for my context either.
> 
> __const_udelay().
> 
> 157 inline void __const_udelay(unsigned long xloops)
> 158 {
> 159 unsigned long lpj = this_cpu_read(cpu_info.loops_per_jiffy) ? : 
> loops_per_jiffy;
> 160 int d0;
> 161
> 162 xloops *= 4;
> 163 asm("mull %%edx"
> 164 :"=d" (xloops), "=&a" (d0)
> 165 :"1" (xloops), "0" (lpj * (HZ / 4)));
> 166
> 167 __delay(++xloops);
> 168 }
> 
> 
> In my early  code, loops_per_jiffy is not initialized yet. Hence "lpj" for 
> the asm line
> is 4096 (default value).
> 
> The  cpu_info.loops_per_jiffy actually reads 8832000 after initialization. 
> They are
> about 2000 times different.
> 
> I did a hacky test in kernel to check the difference between these two 
> different
> "lpj" values. (The hacky patch is attached.) Below is the output for 100ms 
> delay.
> 
> [2.494751] udelay_test uninitialized >start
> [2.494820] udelay_test uninitialized >end
> [2.494828] udelay_test initialized >start
> [2.595234] udelay_test initialized >end
> 
> For 100ms delay, udelay() with uninitialized loops_per_jiffy only gives a 
> delay of
> only 69us.

Ok, then could we add some simple calibration to make udelay work much better - 
or 
perhaps move the udelay calibration up earlier?

Hiding essentially an early udelay() implementation in an early-printk driver 
is 
ugly and counterproductive.

Thanks,

Ingo
--
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