SABLE_PARKMODE_SS to 1 can work around this bug.
Is this something that affects some versions but not others? If the
case, we should teach the driver to handle this based on a revision
check.
cheers
--
balbi
signature.asc
Description: PGP signature
return DIV_ROUND_UP(min(val, 500U), 2);
> else
> - return DIV_ROUND_UP(val, 8);
> + /* USB 3.x supports 900mA, but that isn't divisible by 8... */
> + return DIV_ROUND_UP(min(val, 896U), 8);
DIV_ROUND_UP(896, 8) = 112
DIV_ROUND_UP(900, 8) = 113
Why value do you want here?
--
balbi
signature.asc
Description: PGP signature
Hi,
(sorry for the long delay)
Vicente Bergas writes:
> On Tuesday, August 27, 2019 2:16:20 PM CEST, Vicente Bergas wrote:
>> On Tuesday, August 27, 2019 1:53:04 PM CEST, Felipe Balbi wrote:
>>> Hi,
>>>
>>> Vicente Bergas writes:
>>>> On Sa
> + /* Don't send link state change request */
> + reg &= ~DWC3_DCTL_ULSTCHNGREQ_MASK;
> dwc3_writel(dwc->regs, DWC3_DCTL, reg);
Let's a small macro or a little function to wrap this. Something
dwc3_dctl_write_safe() or something long those line.
Then that macro/function will make sure to clear those bits.
--
balbi
signature.asc
Description: PGP signature
ot;terminate on short read" convention;
> @@ -215,6 +215,7 @@ static void defer_kevent(struct eth_dev *dev, int flag)
> if (dev->port_usb->is_fixed)
> size = max_t(size_t, size, dev->port_usb->fixed_out_len);
>
> + spin_unlock_irqrestore(&dev->lock, flags);
> skb = __netdev_alloc_skb(dev->net, size + NET_IP_ALIGN, gfp_flags);
> if (skb == NULL) {
> DBG(dev, "no rx skb\n");
> --
Doesn't apply. Please rebase and resend.
checking file drivers/usb/gadget/function/u_ether.c
Hunk #1 FAILED at 186.
Hunk #2 succeeded at 217 with fuzz 2 (offset 2 lines).
1 out of 2 hunks FAILED
--
balbi
4 zsI ==> -11
>>> uvc-gadget-275 [001] .n.. 215.960619: dwc3_free_request: ep2in:
>>> req d9961026 length 0/1024 zsI ==> -11
>>
>> So, first things first:
>>
>> Let's figure out what those class requests are and why are they *always*
>> stalled. UVC class spec should answer that.
>
> I tried to decipher 2 class specific requests that we get.
>
>>
>> Then we need to find out why it takes 1.5 seconds for uvc-gadget to
>> queue more data.
>>
>
> Not sure how to do that.
Look at the source for uvc-gadget. If I were to guess, I'd say
uvc-gadget prepares frames as a function of the requested bandwidth.
It probably goes to sleep periodically until it thinks there's more data
to send.
Best
--
balbi
Hi,
Roger Quadros writes:
>>> I'm having a hard time to figure out how to get g_webcam working with
>>> the tip of http://git.ideasonboard.org/uvc-gadget.git
>>>
>>> Platform I'm using is dra7-evm with dwc3 controller.
>>
>> which arguments are you passing to g_webcam?
>
> I've tried a couple
ebcam?
> What platform was g_webcam and uvc-gadget last tested with?
I think we went through this a while back, but the problem back then was
related to bad arguments being passed to g_webcam. You should still be
able to find our discussion on linux-usb.
--
balbi
e
> free.
>
> Hence, Fix this issue by setting request and buffer pointer to NULL after
> kfree.
>
> Signed-off-by: Chandana Kishori Chiluveru
>
> Changes in v2:
> - Modified commit text.
These two lines...
> ---
... should be after this tearline :-)
We don't need that in the commit log
--
balbi
signature.asc
Description: PGP signature
ncy (or flushing (etc)) issue.
XHCI has a HW-configurable maximum number of segments in a ring. AFAICT,
xhci driver doesn't take that into consideration today. Perhaps the HW
in question doesn't like more than 3 segments.
Mathias, what was the register to check this? Do you remember?
--
balbi
signature.asc
Description: PGP signature
ses may lead the link TRB trigger a interrupt when
> the IOC bit is not setted?
No idea, perhaps you should have a deeper look at both Synopsys databook
and xHCI specification.
In any case, v4.9 is really old.
Good luck
--
balbi
ff0
> occurs a transfer event, but this DMA address is not in the trb ring,
> then the driver report an error(and followed a few error logs witch
> invalid DMA address).
> 2. I dump the data of the address(0x1eb0fff0) and find the IOC bit is
> not set, see as below:
> # dump_reg.sh 0x1eb0fff0 4
> 0x1eb0fff0:0x1EB1 0x 0x 0x1800
--
balbi
^__^
\ (oo)\___
(__)\ )\/\
||w |
|| ||
The following changes since commit e21a712a9685488f5ce80495b37b9fdbe96c230d:
Linux 5.3-rc3 (2019-08-04 18:40:12 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/ker
t;is_utmi_l1_suspend)
> - params->besl_deep = min_t(u8, dwc->hird_threshold, 15);
> + params->besl_deep =
> + clamp_t(u8, dwc->hird_threshold, 2, 15);
> }
>
> /* U1 Device exit Latency */
> --
> 2.11.0
>
--
balbi
Hi,
Benjamin Herrenschmidt writes:
> On Wed, 2019-08-28 at 13:09 +0300, Felipe Balbi wrote:
>> Hi,
>>
>> Benjamin Herrenschmidt writes:
>>
>> > The split into multiple structures of the "ll" register bank is
>> > impractical. It makes
ted in build break. Can you
collect all the dependencies and send a single series? I'm applying on
top of my testing/next branch.
cheers
--
balbi
signature.asc
Description: PGP signature
>> >> >> >
>> >> >> > reg = dwc3_readl(dwc->regs, DWC3_GFLADJ);
>> >> >> > dft = reg & DWC3_GFLADJ_30MHZ_MASK;
>> >> >> > - if (!dev_WARN_ONCE(dwc->dev, dft == dwc->fladj,
>> >> >&g
gt;> {
>> struct dwc3_of_simple *simple = dev_get_drvdata(dev);
>> @@ -190,6 +195,7 @@ MODULE_DEVICE_TABLE(of, of_dwc3_simple_match);
>> static struct platform_driver dwc3_of_simple_driver = {
>> .probe = dwc3_of_simple_probe,
>> .remove = d
Hi,
Peter Chen writes:
> Hi Balbi,
>
> When do configfs function add and remove stress test, I find dwc3
> gadget .pullup will timeout if there is a request on the way. Even I
what do you mean by "a request on the way"?
> enlarge the delay, there is still timeout fo
Hi,
Greg Kroah-Hartman writes:
> Platform drivers now have the option to have the platform core create
> and remove any needed sysfs attribute files. So take advantage of that
> and do not register "by hand" any sysfs files.
>
> Cc: Felipe Balbi
> Signed-off-by: G
vers/usb/cdns3/host.c
>> create mode 100644 drivers/usb/cdns3/trace.c
>> create mode 100644 drivers/usb/cdns3/trace.h
>>
>
>
>
> It is getting really hard to review this patch as it is so large.
> I would still suggest to split host/gadget/drd if possible.
> Felipe, any objections?
Maybe you guys can do a few rounds off-list then?
--
balbi
r_resetc_put;
@@ -121,9 +129,6 @@ static int dwc3_of_simple_remove(struct platform_device
*pdev)
clk_bulk_put_all(simple->num_clocks, simple->clks);
simple->num_clocks = 0;
- if (!simple->pulse_resets)
- reset_control_assert(simple->resets);
-
reset_control_put(simple->resets);
pm_runtime_disable(dev);
Can you test?
--
balbi
tatic int dwc3_core_init_for_resume(struct dwc3 *dwc)
> {
> @@ -1866,6 +1873,7 @@
> static struct platform_driver dwc3_driver = {
> .probe = dwc3_probe,
> .remove = dwc3_remove,
> + .shutdown = dwc3_shutdown,
> .driver = {
> .name = "dwc3",
> .of_match_table = of_match_ptr(of_dwc3_match),
>
> and leaving dwc3-of-simple.c as is, the issue persisted.
That's because your reset controller is not passed to dwc3 core, only to
your glue layer.
--
balbi
dft = reg & DWC3_GFLADJ_30MHZ_MASK;
>> >> > - if (!dev_WARN_ONCE(dwc->dev, dft == dwc->fladj,
>> >> > - "request value same as default, ignoring\n")) {
>> >> > + if (dft != dwc->fladj) {
>> >>
>> >> if the
.of_match_table = of_dwc3_simple_match,
>
> If this patch is OK after review i can resubmit it as a pull request.
not a pull request, just send a patch using git send-email
> Should a similar change be applied to drivers/usb/dwc3/core.c ?
Is it necessary? We haven't had any bu
ROR: "xhci_handshake" [drivers/usb/host/xhci-pci.ko] undefined!
>
> So I write my own function to check CNR.
yeah, move that code to xhci_suspend(). It's valid for any XHCI host.
--
balbi
river know to fall back to HW based role
>>> switching?
>>
>> Use a 'disconnect' or 'suspend' event to go reset it? But that should,
>> probably, be done at kernel space, no?
>>
>
> Yes that could be one option.
> So after a disconnect, sysfs role should reflect actual hardware role.
> correct?
that would be my expectation
--
balbi
aps?
#define DB_STREAMID(x) (((x) << 16) & DB_STREAMID_MASK)
Otherwise you end up defining the mask twice.
--
balbi
signature.asc
Description: PGP signature
e role so writes "device"
> to
> mode switch sysfs. port transitions to "device" role.
>
> Now, how does controller driver know to fall back to HW based role switching?
Use a 'disconnect' or 'suspend' event to go reset it? But that should,
probably, be done at kernel space, no?
--
balbi
signature.asc
Description: PGP signature
e coherent API anyway.
*/
WARN_ON(irqs_disabled());
if (!cpu_addr)
return;
debug_dma_free_coherent(dev, size, cpu_addr, dma_handle);
if (dma_is_direct(ops))
dma_direct_free(dev, size, cpu_addr, dma_handle, attrs);
else if (ops->free)
ops->free(dev, size, cpu_addr, dma_handle, attrs);
}
EXPORT_SYMBOL(dma_free_attrs);
maybe you're gonna have to fire up a workqueue to free this memory for
you :-(
Unless someone else has better ideas. Alan, Greg, any ideas?
--
balbi
signature.asc
Description: PGP signature
n the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
tags/fixes-for-v5.3-rc4
for you to fetch changes up to 4a56a478a525d6427be90753451c40e1327caa1a:
usb: gadget: mass_storage: Fix races between fsg_disable and fsg_set_alt
(2019-08-12 08:55:24
In free time.
>>
>>Yet another thread? Can't you just run this right before giving back the
>>USB request? So, don't do it from IRQ handler, but from giveback path?
>
> Do you mean in:
> if (request->complete) {
> spin_unlock(&priv_dev->lock);
> if (priv_dev->run_garbage_collector) {
>
> }
> usb_gadget_giveback_request(&priv_ep->endpoint,
> request);
> spin_lock(&priv_dev->lock);
> }
> ??
right, you can do it right before giving back the request. Or right
after.
> I ask because this is finally also called from IRQ handler:
>
> cdns3_device_thread_irq_handler
> -> cdns3_check_ep_interrupt_proceed
> -> cdns3_transfer_completed
> -> cdns3_gadget_giveback
> -> usb_gadget_giveback_request
Did you notice that it doesn't reenable interrupts, though?
--
balbi
signature.asc
Description: PGP signature
o free the buffer, you end up creating the possibility for a race
>>condition. Specially since you don't mask all interrupt events. The
>>moment you reenable interrupts, one of your not-unmasked interrupt
>>sources could trigger, then top-half gets scheduled which tries to wake
>>up the IRQ thread again and things go boom.
>
> Ok, I think I understand. So I have 3 options:
> 1. Mask the USB_IEN and EP_IEN interrupts, but then I can lost some USB_ISTS
> events. It's dangerous options.
sure sounds dangerous, but also sounds quite "peculiar" :-)
> 2. Remove implementation of handling unaligned buffers and assume that
> upper layer will worry about this. What with vendor specific drivers that
> can be used by companies and not upstreamed ?
> It could be good to have such safety mechanism even if it is not
> currently used.
dunno. It may become dead code that's NEVER used :-)
> 3. Delegate this part of code for instance to separate thread that will be
> called
>In free time.
Yet another thread? Can't you just run this right before giving back the
USB request? So, don't do it from IRQ handler, but from giveback path?
--
balbi
signature.asc
Description: PGP signature
dor == PCI_VENDOR_ID_NVIDIA) {
> + retval = xhci_poll_cnr(hcd);
you could just use xhci_handshake() here, right?
--
balbi
signature.asc
Description: PGP signature
equest value same as default, ignoring\n")) {
>> > + if (dft != dwc->fladj) {
>>
>> if the value isn't different, why do you want to change it?
>>
>> --
>> Balbi
> Hi Balbi,
>
> I don't change any value. I was remove that call tr
test.
>>> So, driver has been tested for such case. During this test driver during
>>> transferring data generate a huge number of LPM interrupts which
>>> are usb interrupts.
>>>
>>> I can't block usb interrupts interrupts because:
>>> /*
>>> * WORKAROUND: CDNS3 controller has issue with hardware resuming
>>> * from L1. To fix it, if any DMA transfer is pending driver
>>> * must starts driving resume signal immediately.
>>> */
>>
>>I can't see why this would prevent you from defering handling to thread
>>handler.
>>
>
> I also will try to move it, but this change can has impact on performance.
how much is the impact? What's the impact? Why does this impact performance?
>>>>> + struct cdns3_aligned_buf *buf, *tmp;
>>>>> +
>>>>> + list_for_each_entry_safe(buf, tmp, &priv_dev->aligned_buf_list,
>>>>> + list) {
>>>>> + if (!buf->in_use) {
>>>>> + list_del(&buf->list);
>>>>> +
>>>>> + spin_unlock_irqrestore(&priv_dev->lock, flags);
>>>>
>>>>creates the possibility of a race condition
>>> Why? In this place the buf can't be used.
>>
>>but you're reenabling interrupts, right?
>
> Yes, driver frees not used buffers here.
> I think that it's the safest place for this purpose.
I guess you missed the point a little. Since you reenable interrupts
just to free the buffer, you end up creating the possibility for a race
condition. Specially since you don't mask all interrupt events. The
moment you reenable interrupts, one of your not-unmasked interrupt
sources could trigger, then top-half gets scheduled which tries to wake
up the IRQ thread again and things go boom.
>>>>> + dma_free_coherent(priv_dev->sysdev, buf->size,
>>>>> + buf->buf,
>>>>> + buf->dma);
>>>>> + spin_lock_irqsave(&priv_dev->lock, flags);
>>>>> +
>>>>> + kfree(buf);
>>>>
>>>>why do you even need this "garbage collector"?
>>>
>>> I need to free not used memory. The once allocated buffer will be
>>> associated with
>>> request, but if request.length will be increased in usb_request then driver
>>> will
>>> must allocate the bigger buffer. As I remember I couldn't call
>>> dma_free_coherent
>>> in interrupt context so I had to move it to thread handled. This flag was
>>> used to avoid
>>> going through whole aligned_buf_list every time.
>>> In most cases this part will never called int this place
>>
>>Did you try, btw, setting the quirk flag which tells gadget drivers to
>>always allocate buffers aligned to MaxPacketSize? Wouldn't that be enough?
>
> If found only quirk_ep_out_aligned_size flag, but it align only buffer size.
>
> DMA used by this controller must have buffer address aligned to 8.
> I think that on most architecture kmalloc should guarantee such aligned.
right, it should be aligned on PAGE_SIZE
> The problem was detected on NXP testing board.
and what was the alignment on that? IIRC, ARM had the same alignment
requirements as x86. Where you sing SLUB allocator on that NXP board,
perhaps?
--
balbi
ucceeded at 1363 (offset -2 lines).
1 out of 10 hunks FAILED
Could you rebase on my testing/next?
--
balbi
> dynamically linked module called "udc-xilinx" and force all
> gadget drivers to also be dynamically linked.
>
> +config USB_TEGRA_XUDC
> + tristate "NVIDIA Tegra Superspeed USB 3.0 Device Controller"
> + depends on ARCH_TEGRA
I need at least a COMPILE_TEST here.
--
balbi
I agree with Roger. Use role switch framework for production, not debugfs.
--
balbi
Hi,
Benjamin Herrenschmidt writes:
> On Fri, 2019-08-09 at 11:08 +0300, Felipe Balbi wrote:
>>
>> that works too. Another option would to introduce two options,
>> has_input_report and has_output_report and have them true by default.
>>
>> Then user ca
Hi,
Benjamin Herrenschmidt writes:
> On Fri, 2019-08-09 at 08:31 +0300, Felipe Balbi wrote:
>> Hi,
>>
>> Benjamin Herrenschmidt writes:
>>
>> > Some host drivers really do not like keyboards having an OUT
>> > endpoint.
>> >
>> &
) along with
> the computation.
>
> Similar, a portion of the ohci-omap driver is just there for configuring
> the memory translation, this too can get moved into usb.c
>
> Signed-off-by: Arnd Bergmann
For all of these patches related to usb:
Acked-by: Felipe Balbi
Thanks for cleaning this up, Arnd.
--
balbi
ds
> if they are ever to be used with such systems as host.
>
> Signed-off-by: Benjamin Herrenschmidt
Could you come up with a slightly more descriptive name? single_ep
doesn't give me any hint of which endpoint will be left around.
Perhaps call it 'disable_output_report'?
--
balbi
Hi,
kbuild test robot writes:
> tree:
> https://kernel.googlesource.com/pub/scm/linux/kernel/git/balbi/usb.git
> testing/next
> head: d06a2c3f683a591efce9d02b2b60ef346df5ae02
> commit: 2a714ea6d90d9d1b510ba424652a2e3dfd547267 [2/13] USB: phy: tahvo:
> convert plat
> void dwc3_host_exit(struct dwc3 *dwc)
> {
> - phy_remove_lookup(dwc->usb2_generic_phy, "usb2-phy",
> - dev_name(dwc->dev));
> - phy_remove_lookup(dwc->usb3_generic_phy, "usb3-phy",
> - dev_name(dwc->dev));
> platform_device_unregister(dwc->xhci);
> }
Roger, could you verify that this doesn't regress any of your platforms?
Thanks
--
balbi
signature.asc
Description: PGP signature
t; >
>> > ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req,
>> > - trb, event, status, true);
>> > + trb, event, status, chain);
>> > if (ret)
>> > break;
>> > }
>>
>> There was already a fix a long time ago by Anurag. But it never made it
>> to the kernel mainline. You can check this out:
>> https://patchwork.kernel.org/patch/10640137/
>
> So, back from a vacation last week, and just validated that both Fei's
> patch and a forward ported version of this patch Thinh pointed out
> both seem to resolve the usb stalls I've been seeing sinice 4.20 w/
> dwc3 hardware on both hikey960 and dragonboard 845c.
>
> Felipe: Does Anurag's patch above make more sense as a proper fix?
I think it's enough to check only the TRB. We won't get events for bits
we didn't enable on the TRB. The only problem here is when we get IOC
event for multiple TRBs where only the last one has IOC.
So, instead of checking:
if (event->status & IOC && trb->ctrl & IOC)
It's probably enough to check:
if (tbc->ctrl & IOC)
Could you check that?
Cheers
--
balbi
signature.asc
Description: PGP signature
sb/gadget/udc/renesas_usb3.c | 4 +---
> drivers/usb/gadget/udc/s3c-hsudc.c| 4 +---
> drivers/usb/gadget/udc/udc-xilinx.c | 4 +---
For dwc2, dwc3, gadget and usb/phy:
Acked-by: Felipe Balbi
--
balbi
signature.asc
Description: PGP signature
Hi,
Marek Szyprowski writes:
> On 2019-08-08 11:51, Felipe Balbi wrote:
>> Marek Szyprowski writes:
>>> Calls to USB2 generic PHY calibrate() method has been moved to HCD core,
>>> which now successfully handles generic PHYs and their calibration after
>>>
wc->usb2_generic_phy);
are you sure you're the only one using phy_calibrate()? I don't want any
regressions because of this :-p
--
balbi
signature.asc
Description: PGP signature
/16384 zsI ==> 0
<...>-3165 [001] d..1 164.191778: dwc3_event: event (4084):
ep1out: Transfer In Progress [0] (sIm)
<...>-3165 [001] d..1 164.191779: dwc3_complete_trb: ep1out: trb
73eb9c36 buf 77c62000 size 0 ctrl 0818 (hlcS:sC:normal)
<...>-3165 [001] d..1 164.191788: dwc3_gadget_giveback: ep1out: req
ca060a32 length 16384/16384 zsI ==> 0
kworker/u8:5-1165 [001] 164.191815: dwc3_free_request: ep1out: req
ca060a32 length 16384/16384 zsI ==> 0
<...>-3165 [001] d..1 164.192315: dwc3_event: event (4084):
ep1out: Transfer In Progress [0] (sIm)
<...>-3165 [001] d..1 164.192316: dwc3_complete_trb: ep1out: trb
17050f80 buf 77c63000 size 0 ctrl 001c (hlCS:sc:normal)
<...>-3165 [001] d..1 164.192325: dwc3_gadget_giveback: ep1out: req
af8aa80e length 16384/16384 zsI ==> 0
kworker/u8:5-1165 [001] 164.192353: dwc3_free_request: ep1out: req
af8aa80e length 16384/16384 zsI ==> 0
<...>-3165 [001] d..1 164.192849: dwc3_event: event (4084):
ep1out: Transfer In Progress [0] (sIm)
<...>-3165 [001] d..1 164.192851: dwc3_complete_trb: ep1out: trb
c1f0fd23 buf 77c64000 size 0 ctrl 001c (hlCS:sc:normal)
<...>-3165 [001] d..1 164.192860: dwc3_gadget_giveback: ep1out: req
66963d3c length 16384/16384 zsI ==> 0
kworker/u8:5-1165 [002] 164.192981: dwc3_free_request: ep1out: req
66963d3c length 16384/16384 zsI ==> 0
Please give further of how you're reproducing this. Which kernel, which
gadget driver, do you have out-of-tree patches? Why do we see these
non-sequential TRB addresses. Are you using IOMMU, perhaps? Something
made dwc3 super confused, at least judging by the logs.
cheers
--
balbi
t;
> reg = dwc3_readl(dwc->regs, DWC3_GFLADJ);
> dft = reg & DWC3_GFLADJ_30MHZ_MASK;
> - if (!dev_WARN_ONCE(dwc->dev, dft == dwc->fladj,
> - "request value same as default, ignoring\n")) {
> + if (dft != dwc->fladj) {
if the value isn't different, why do you want to change it?
--
balbi
e condition
> Why? In this place the buf can't be used.
but you're reenabling interrupts, right?
>>> + dma_free_coherent(priv_dev->sysdev, buf->size,
>>> + buf->buf,
>>> + buf->dma);
>>> + spin_lock_irqsave(&priv_dev->lock, flags);
>>> +
>>> + kfree(buf);
>>
>>why do you even need this "garbage collector"?
>
> I need to free not used memory. The once allocated buffer will be associated
> with
> request, but if request.length will be increased in usb_request then driver
> will
> must allocate the bigger buffer. As I remember I couldn't call
> dma_free_coherent
> in interrupt context so I had to move it to thread handled. This flag was
> used to avoid
> going through whole aligned_buf_list every time.
> In most cases this part will never called int this place
Did you try, btw, setting the quirk flag which tells gadget drivers to
always allocate buffers aligned to MaxPacketSize? Wouldn't that be enough?
>>> + TP_printk("%s: req: %p, req buff %p, length: %u/%u %s%s%s, status: %d,"
>>> + cd " trb: [start:%d, end:%d: virt addr %pa], flags:%x ",
>>> + __get_str(name), __entry->req, __entry->buf, __entry->actual,
>>> + __entry->length,
>>> + __entry->zero ? "zero | " : "",
>>> + __entry->short_not_ok ? "short | " : "",
>>> + __entry->no_interrupt ? "no int" : "",
>>
>>I guess you didn't really think the formatting through. Think about what
>>happens if you get a request with only zero flag or only short flag. How
>>will this log look like?
>
> Like this:
> cdns3_gadget_giveback: ep0: req: 71a6a5f5, req buff 8d40c4db,
> length: 60/60 zero | , status: 0, trb: [start:0, end:0: virt addr (null)],
> flags:0
>
> Is it something wrong with this?. Maybe one extra sign |.
yes, the extra | :-)
This is one reason why I switched to character flags where a lower case
character means flag is cleared while uppercase means it's set.
--
balbi
e #ifdefe is not sufficient.
>
> You might need to do something like
>
> #if defined(CONFIG_TRACING)
>
> extern const char *usb_decode_ctrl(..)
>
> #else
>
> static inline const char *usb_decode_ctrl(..) {
> return NULL;
> }
>
> #endif
This is what I mean. They shouldn't be used outside of TRACING, but it's
far safer to have the stubs.
--
balbi
but we're not able to set the
>> appropriate port features for some reason.
>>
>> My next thought is to double-check the logic of the ehset driver as it
>> relates to
>> xhci_hcd. I would like to make sure that we are enabling the USB 2.0 test
>> modes
>> according to the xHCI spec. I'm concerned that we might be experiencing an
>> error
>> because we aren't setting the test mode acording to section 4.19.6 of the
>> xHCI
>> specification.
>>
>> Thanks in advance for reviewing the logs, Mathias! I appreciate any time and
>> feedback you may be able to provide. Let me know if you have any further
>> questions or need more information from me. I look forward to hearing from
>> you
>> soon!
>>
>
> You may not enter test mode at all.
>
> Check flow: xhci_hub_control->xhci_enter_test_mode.
The thing is that we need to enable certification test as well,
otherwise we will never have a chance of getting linux products with a
USB-IF logo out of vanilla mainline tree.
Seems to me that the only thing missing is a way to enter a particular
test mode based on VID/PID pair.
All the functionality is already in place, just this little extra hook
is missing.
--
balbi
mkdir blah
> cd blah/configs
> mkdir foo.1
> cd foo.1
> ln -s /sys/kernel/config/usb_gadget/blah/configs/foo.1/no-such-dir/ barf
I'll try to reproduce on my end. Added linux-usb to the loop
--
balbi
est, and used latest mainline kernel.
> Now the issue can be reproduced at the very first time I plugged the
> docking station.
>
> Attach dmesg to BKO since there are lots of message after XHCI dyndbg is
> enabled.
I saw that you annotated the plug, but not the unplug. Where does the
unplug start? There are many places where it could be, but I need to be
sure.
Also, wasn't it so that the problem is when you *replug* the dock? So
can you better describe what you're doing? Are you booting with dock
connected then disconnect and reconnect or are you booting without dock
and it fails on first plug?
What are you consider a fail here? Can't you see the xhci bus? USB
Devices don't show? What do you have on lsusb -t?
Best regards
--
balbi
signature.asc
Description: PGP signature
ret = dwc3_gadget_ep_reclaim_completed_trb(dep, req,
> - trb, event, status, true);
> + trb, event, status, chain);
this is definitely a valid fix :-) I'm not convinced about that IOC &&
!chain above, however. Also, if "chain" is always trb->ctrl &
DWC3_TRB_CTRL_CHN, we can get rid of that argument altogether and have
the callee handle it internally, but that's something else, subject to
another patch.
--
balbi
signature.asc
Description: PGP signature
near case is correct or not.
That function *must* be called for all cases. We want to reduce the
amount of special cases so code is more straight forward and easier to
maintain. Again, please collect tracepoints of the failure case with the
latest tag from Linus, otherwise you won't be able to c
ccording to what Kai-Heng said, xHCI stops working even after
repluggin the Dock. We could be dealing with two bugs here:
1. Spurious PME event being generated by an unexistent device
2. xHCI not handling hot-plug very well
Kai-Heng,
please run your tests again and make note of when you
1;
This can't be true because we cleared HWO up above
| if (event->status & DEPEVT_STATUS_SHORT && !chain)
| return 1;
can only be true for last TRB
| if (event->status & DEPEVT_STATUS_IOC)
| return 1;
If we have a short packet, then we may fall here. Is that the case?
Please share dwc3 tracepoints of the problem happening so I can verify
what's going on.
--
balbi
else
reg &= ~DWC3_GCTL_DISSCRAMBLE;
+ reg |= DWC3_GCTL_DISSCRAMBLE;
if (dwc->u2exit_lfps_quirk)
reg |= DWC3_GCTL_U2EXIT_LFPS;
Note that you should NOT run your product with this bit set (meaning,
without scrambling disabled) as that will have an effect in EMI. Still,
it may help understand if the redriver is getting confused with the
scrambling or not.
Best
--
balbi
> -0 [003] dNs2 184.588232: dwc3_core: Compliance Mode
>> > detected. Attempting recovery routine
>>
>> Don't we get an interrupt for Compliance mode entry?
>
> Not that I've seen, surprisingly. My compliance mode recovery mechanism
> looks
5 [002] d..1 182.541976: dwc3_event: event (00150301):
> Link Change [RX.Detect]
> irq/23-dwc3-1115 [002] d..1 182.541982: dwc3_event: event (00170301):
> Link Change [Polling]
> -0 [003] .Ns1 184.588211: dwc3_core: Tick! Checking for
> compliance mode
>
> -0 [003] dNs2 184.588232: dwc3_core: Compliance Mode
> detected. Attempting recovery routine
Don't we get an interrupt for Compliance mode entry?
cheers
--
balbi
following changes since commit aa23ce847ddac1fd5ffe987ff12e12ff48318e45:
usb: dwc3: remove unused @lock member of dwc3_ep struct (2019-06-20 11:50:19
+0300)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
tags/usb-for-v5.3-part2
for yo
This patch adds the necessary PCI ID for TGP-LP devices.
Signed-off-by: Felipe Balbi
---
drivers/usb/dwc3/dwc3-pci.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index f9b550081550..5e8e18222f92 100644
--- a/drivers/usb/dwc3
Hi,
Benjamin Herrenschmidt writes:
> On Tue, 2019-07-02 at 15:56 +0300, Felipe Balbi wrote:
>> We already have an interface for disconnecting from the host
>> programatically by disconnecting data pullup.
>>
>> static ssize_t soft_connect_store(struct device *dev
>> > > > the default hardware values it might be deficient as compared to the
>> > > > working endpoint that gets assigned in your 2-function config.
>> > >
>> > > Jack,
>> > >
>> > > thanks for the pointer, it is indee
usb_gadget_disconnect(udc->gadget);
usb_gadget_udc_stop(udc);
} else {
dev_err(dev, "unsupported command '%s'\n", buf);
return -EINVAL;
}
return n;
}
static DEVICE_ATTR_WO(soft_connect);
part of udc/core.c
--
balbi
19-06-17 11:23:24 +0200)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git tags/usb-for-v5.3
for you to fetch changes up to aa23ce847ddac1fd5ffe987ff12e12ff48318e45:
usb: dwc3: remove unused @lock member of dwc3_ep struct (2019-06-20 11:50:19
bus resume
afff6067b305 usb: chipidea: Drop lock across event_notify during gadget stop
732a4af85e87 usb: chipidea: Remove locking in ci_udc_start()
34445fb4333f usb: chipidea: Properly mark little endian descriptors
63b9e901e461 usb: chipidea: udc: remove unnecessary & operation
a98e25e71d11 usb: chipidea: udc: make use of new usb_endpoint_maxp_mult()
Peter, have you seen the problem described before?
--
balbi
used : y
> state : RNDIS_UNINITIALIZED
> medium: 0x
> speed : 425984000
> cable : connected
> vendor ID : 0x
> vendor: (null)
>
> Thanks for any help.
Which peripheral controller is this board using? Is it chipidea? dwc2?
dwc3? High Speed or Super Speed?
--
balbi
/linux/kernel/git/balbi/usb.git
tags/fixes-for-v5.2-rc5
for you to fetch changes up to 42de8afc40c97002fceb500e2331f6a722be3c14:
usb: dwc2: Use generic PHY width in params setup (2019-06-18 10:27:14 +0300)
usb: fixes for v5.2-rc5
This patch simply adds a new PCI Device ID
Signed-off-by: Felipe Balbi
---
drivers/usb/dwc3/dwc3-pci.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 8cced3609e24..f9b550081550 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
Hi,
Lianwei Wang writes:
> On Tue, Jun 18, 2019 at 11:21 PM Felipe Balbi wrote:
>>
>>
>> Hi,
>>
>> Lianwei Wang writes:
>> >> Lianwei Wang writes:
>> >> > On Mon, Jun 17, 2019 at 5:40 AM Felipe Balbi wrote:
>> >>
l
> ongoing control transfers.
Considering that the gadget API handles one stage at a time, it would be
impossible for this to not be the case :-)
UDCs start a trasnfer only for the setup phase, then process the
control request to decide what to do next.
--
balbi
signature.asc
Description: PGP signature
Hi,
Johan Hovold writes:
>> Johan Hovold writes:
>> > On Tue, Jun 11, 2019 at 08:24:16PM +0300, Felipe Balbi wrote:
>> >> If we happen to have two XHCI controllers with DbC capability, then
>> >> there's no hope this will ever work as the glo
>>
>> To me, it seems like this part of the controller wasn't well
>> thought-out. These extra two bits, perhaps, should be internal to the
>> controller and SW should have no knowledge that they exist.
>
> These values are internal. SW should not have knowledge of it. This
> implementation will not follow the programming guide and should be used
> as a quirk for devices that are too slow to handle the XferNotReady
> event but want to schedule isoc immediately after handling the event.
They are *not* internal if SW needs to know that to start a transfer
properly it needs these extra two bits :-) What I meant to say was that
we should never have a 16-bit frame number. Only 14 bits. But in any
case, we can't change the HW now :-)
--
balbi
signature.asc
Description: PGP signature
Hi,
Lianwei Wang writes:
>> Lianwei Wang writes:
>> > On Mon, Jun 17, 2019 at 5:40 AM Felipe Balbi wrote:
>> >>
>> >> Lianwei Wang writes:
>> >>
>> >> > The udc and gadget device will be deleted when udc device is
&g
adget: split out network core")
Cc:
Signed-off-by: Kiruthika Varadarajan
Signed-off-by: Felipe Balbi
---
drivers/usb/gadget/function/u_ether.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/u_ether.c
b/drivers/usb/gadget/function/u_eth
>>> Signed-off-by: Jules Maselbas
>>
>> Acked-by: Minas Harutyunyan
>
> Gentle reminder, Felipe, could you take this to the fixes for v5.2?
darn it, I applied for 'next'. I'll remove from that branch and place it
in fixes. Pull request, hopefully, tomorrow.
--
balbi
signature.asc
Description: PGP signature
Hi,
Lianwei Wang writes:
> On Mon, Jun 17, 2019 at 5:40 AM Felipe Balbi wrote:
>>
>> Lianwei Wang writes:
>>
>> > The udc and gadget device will be deleted when udc device is
>> > disconnected and the related function will be unbind with it.
>> &g
ldn't have any
issues. But since DSTS only contains 14 our of the 16 bits the
controller needs, then we can't really use that.
To me, it seems like this part of the controller wasn't well
thought-out. These extra two bits, perhaps, should be internal to the
controller and SW should have no knowledge that they exist.
In any case, this is the biggest sort of issues in DWC3 right now :-)
Anything else seems to behave nicely without any problems.
--
balbi
signature.asc
Description: PGP signature
r driver? That way they
can be reviewed separately.
Thanks
--
balbi
signature.asc
Description: PGP signature
using after freed issue, always set the gadget
> object to netdev in bind interface.
>
> Signed-off-by: Lianwei Wang
I can't actually understand what's the problem here. The gadget is not
deleted when we disconnect the cable.
--
balbi
signature.asc
Description: PGP signature
gt; usb-phy = <&usb2_phy>, <&usb3_phy>;
> dr_mode = "host";
> power-domains = <&sps S900_PD_USB3>;
> snps,hsphy_interface = "utmi";
> };
> };
>
> After going through the mailing list looking for similar issue, found that
> some host controllers works with following quirk:
>
> snps,dis_u3_susphy_quirk;
>
> I tried that also but it didn't work. Can anyone shed some light on this?
>
> PS: USB3 is working fine with vendor kernel 3.10.
Check what's different between mainline and your v3.10 code. Specially
look for changes in xhci and usbcore.
Good luck
--
balbi
signature.asc
Description: PGP signature
Hi,
Johan Hovold writes:
> On Tue, Jun 11, 2019 at 08:24:16PM +0300, Felipe Balbi wrote:
>> If we happen to have two XHCI controllers with DbC capability, then
>> there's no hope this will ever work as the global pointer will be
>> overwritten by the controller that
>> >
>> > Where can I get one of those?
>> Here is one example:
>> https://www.amazon.com/SIIG-SuperSpeed-Cable-Meters-CB-US0212-S1/dp/B0032ANCBO
>
> Ah, nice! I'll try to see if I can get that in my country...
>
> Nope, not available in Europe
xhci: Add DbC support in xHCI driver
Cc: # v4.16+
Signed-off-by: Felipe Balbi
---
drivers/usb/host/xhci-dbgcap.c | 4 +--
drivers/usb/host/xhci-dbgcap.h | 3 +-
drivers/usb/host/xhci-dbgtty.c | 54 +-
3 files changed, 31 insertions(+), 30 deletions(-)
diff --
Hi,
Joakim Tjernlund writes:
> On Mon, 2019-06-10 at 10:56 +0300, Felipe Balbi wrote:
>> Hi,
>>
>> Joakim Tjernlund writes:
>> > We are trying to get fsl_udc up and running on a T1042 with without
>> > success.
>> > Seems like this
can try that version of the kernel, we can't
really help you here.
Good luck
--
balbi
signature.asc
Description: PGP signature
> wLength (during previous stage).
>>
>> - Status Stage
>> A zero length transfer to communicate successful end of transfer
>> (in case it completes fine) or an error (in case of STALL
>> condition).
>
> Hm, then why does the usb_control_msg() function accepts a data and
> size arguments? Which are described in the comment as "pointer to the
> data to send" and "length in bytes of the data to send" accordingly?
> Or is this the buffer for the response?
That's for the data stage :-)
usb_control_msg() is an upper lever API to encode and entire Control
Transfer (all stages of it).
What is the problem you see, then?
--
balbi
signature.asc
Description: PGP signature
completes fine) or an error (in case of STALL
condition).
> I've faced this with a custom implementation of a gadget driver module
> while using the dummy_hcd module, but I AFAIU it's not relevant to
> those two, but rather to the whole gadget subsystem.
What is this custom gadget doing? Which kernel version are you using?
What error are you facing? Could it be that you misunderstood how USB
works?
Best regards
--
balbi
signature.asc
Description: PGP signature
field of the control structure.
Also, host side does *not* pass its usb_ctrlrequest struct to the
gadget, it passes a series of 8 bytes which are oblivious to where in
memory they were from the host point of view.
If if you have the same machine acting as both host and device, each
side has no kno
seems like we will *always* have some corner case
where we can't guarantee that we can even start a transfer since there's
no upper-bound between XferNotReady and gadget driver finally queueing a
request. Also, I can't simply read DSTS for the frame number because of
top-most 2 bits.
best
--
balbi
signature.asc
Description: PGP signature
3 (2019-06-02 13:55:33 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git
tags/fixes-for-v5.2-rc4
for you to fetch changes up to 42cc68868ce9d5f5277f561bb17b4746a332bb28:
usb: gadget: udc: lpc32xx: fix return value check in lpc32xx_udc_p
6&w=2>),
> also the size of the driver has shrunken considerably.
>
> If there are still some other bigger issues with this driver, please
> let me know.
Looks pretty clean to me. I would simply break that single file into
smaller files if possible (see xhci/ehci for example).
my
u32 timeout = 5000;
u32 saved_config = 0;
u32 reg;
Let me know if it helps or not. I guess it's also time to switch this
block of code to readl_poll_timeout_atomic().
--
balbi
b/scm/linux/kernel/git/balbi/usb.git tags/usb-for-v5.2
for you to fetch changes up to 2e487d280525b91b03976203b15aba365ec5b4e6:
usb: dwc3: Rename DWC3_DCTL_LPM_ERRATA (2019-05-03 09:13:49 +0300)
USB: changes for v5.2 merge window
W
c-xilinx" and force all
> gadget drivers to also be dynamically linked.
>
> +config USB_TEGRA_XUDC
> + tristate "NVIDIA Superspeed USB 3.0 Device Controller"
> + depends on ARCH_TEGRA
no compile_test?
--
balbi
signature.asc
Description: PGP signature
Jules Maselbas writes:
> Makes GHWCFG4_UTMI_PHY_DATA* defines closer to their relative shift and
> mask defines to improve readability.
>
> Signed-off-by: Jules Maselbas
Doesn't apply. Please make sure to rebase on testing/next
--
balbi
signature.asc
Description: PGP signature
1 - 100 of 8015 matches
Mail list logo
