Re: EHCI and MUSB do not discover devices without CONFIG_PM

[Ladislav Michl]

On Tue, Nov 28, 2017 at 10:30:54AM +0100, Greg KH wrote:
> On Tue, Nov 28, 2017 at 09:57:51AM +0100, Ladislav Michl wrote:
> > Hi Greg,
> > 
> > On Tue, Nov 28, 2017 at 08:33:28AM +0100, Greg KH wrote:
> > > On Mon, Nov 27, 2017 at 11:08:33PM +0100, Ladislav Michl wrote:
> > > > Hi there,
> > > > 
> > > > USB hosts do not discover any connected device on OMAP3 based board
> > > > with CONFIG_PM=n. Just enabling this option is enough to restore working
> > > > behaviour. Nothing unusual in log. Tested 4.14.2 and 4.15-rc1. I know
> > > > a lot of stuff depends on CONFIG_PM, but is this expected behaviour?
> > > > Neither EHCI nor MUSB is working without CONFIG_PM.
> > > 
> > > What bus type is your controllers on?  PCI?  platform?  Something else?
> > 
> > Platform controllers inside OMAP3630 Soc.
> > 
> > > And yes, perhaps this is to be expected, why would you not want
> > > CONFIG_PM to be enabled?  :)
> > 
> > For a start, I know Linux is general purpose OS and I know I cannot expect
> > low latency or low jitter when dealing with interrupts.
> 
> Well, it's the best latency of any other OS out there :)

Indeed, with CONFIG_PM=n... And that makes it the only OS without USB support
out there :)

> Anyway, if you want guaranteed response time, you are going to have to
> use the RT patchset, no matter what.  Otherwise you have the potential
> to have bad jitter at times.

That will not help, as jitter is comming from some part of SoC sleeping...

> > Original problem is described here:
> > https://www.spinics.net/lists/linux-omap/msg140081.html
> > 
> > Shortly, with CONFIG_PM jitter of GPIO interrupt is about 350us which
> > renders IR receiver unuseable - is cannot reliably decode IR protocol
> > (gpio-ir-recv is used). With CONFIG_PM disabled, jitter is around 30us
> > and that's enough to make IR decoders work.
> 
> bit-banging an ir decoder, ugh, you are in for a world of hurt.  Can't
> you put a chip on the device that does this for you in hardware?

OMAP has DM timer which can be externally trigered on edge. Perfect for
that purpose. But I cannot pinmux its input as hw designers did poor job.
And there are thousands of devices deployed.

So it is about a lot of soldering or providing software solution.

> Anyway, good luck!

A little pointer would increase "luck" by several order of magnitudes.

Thank you,
ladis
--
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] PM / runtime: Drop children check from __pm_runtime_set_status()

[Yoshihiro Shimoda]

Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Tuesday, November 28, 2017 7:58 PM
> 
> Hi Rafael, Shimoda-san,
> 
> On Sun, Nov 12, 2017 at 1:27 AM, Rafael J. Wysocki  wrote:
> > From: Rafael J. Wysocki 
> >
> > The check for "active" children in __pm_runtime_set_status(), when
> > trying to set the parent device status to "suspended", doesn't
> > really make sense, because in fact it is not invalid to set the
> > status of a device with runtime PM disabled to "suspended" in any
> > case.  It is invalid to enable runtime PM for a device with its
> > status set to "suspended" while its child_count reference counter
> > is nonzero, but the check in __pm_runtime_set_status() doesn't
> > really cover that situation.
> >
> > For this reason, drop the children check from __pm_runtime_set_status()
> > and add a check against child_count reference counters of "suspended"
> > devices to pm_runtime_enable().
> >
> > Signed-off-by: Rafael J. Wysocki 
> > ---
> >  drivers/base/power/runtime.c |   30 ++
> >  1 file changed, 10 insertions(+), 20 deletions(-)
> >
> > Index: linux-pm/drivers/base/power/runtime.c
> > ===
> > --- linux-pm.orig/drivers/base/power/runtime.c
> > +++ linux-pm/drivers/base/power/runtime.c
> > @@ -1101,29 +1101,13 @@ int __pm_runtime_set_status(struct devic
> > goto out;
> > }
> >
> > -   if (dev->power.runtime_status == status)
> > +   if (dev->power.runtime_status == status || !parent)
> > goto out_set;
> >
> > if (status == RPM_SUSPENDED) {
> > -   /*
> > -* It is invalid to suspend a device with an active child,
> > -* unless it has been set to ignore its children.
> > -*/
> > -   if (!dev->power.ignore_children &&
> > -   atomic_read(>power.child_count)) {
> > -   dev_err(dev, "runtime PM trying to suspend device 
> > but active child\n");
> 
> JFTR, this triggered before during system resume on e.g. Salvator-XS with
> R-Car H3:
> 
> ohci-platform ee08.usb: runtime PM trying to suspend device
> but active child
> phy_rcar_gen3_usb2 ee080200.usb-phy: runtime PM trying to suspend
> device but active child
> ohci-platform ee0c.usb: runtime PM trying to suspend device
> but active child
> ohci-platform ee0a.usb: runtime PM trying to suspend device
> but active child
> phy_rcar_gen3_usb2 ee0c0200.usb-phy: runtime PM trying to suspend
> device but active child
> phy_rcar_gen3_usb2 ee0a0200.usb-phy: runtime PM trying to suspend
> device but active child
> 
> so this was an existing issue with USB before.

Thank you for the report!
I know that, but since this didn't cause any trouble until now,
I postponed to investigate the issue... But, I investigate it today.
I don't find the root cause yet. However, it seems related to usb host and/or 
usb core.
--> USB host related devices' child_count will be 1 in suspend timing.
 --> I guess remote wakeup feature is enabled? But, I don't find the point yet.

The renesas_usbhs also uses the phy_rcar_gen3_usb2 driver.
--> If I only used the renesas_usbhs driver (in other words, I don't install
[eo]hci-{hcd,platform} drivers), the issue disappeared.
 --> So, I think the phy_rcar_gen3_usb2 driver doesn't cause this issue.
(But, it is possible to be related though.)

I'll continue to investigate this issue tomorrow.

Best regards,
Yoshihiro Shimoda

Re: [PATCH] r8152: disable rx checksum offload on Dell TB dock

[Kai Heng Feng]

> On 27 Nov 2017, at 11:13 PM,  
>  wrote:
> 
> This is quite surprising to me too.  The externally plugged in r8153 dongle,
> was it connected over type C port or over type A port?  AFAIK Type C port is
> actually Alpine ridge pass through port.  It is not connected to XHCI 
> controller
> or USB hub.

Over the front type A port, which connects to SMSC hub then ASMedia controller.

The type C port indeed connects to AR directly, hence no such issue.

Kai-Heng

--
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 2/2 v7] typec: tcpm: Only request matching pdos

[Adam Thomson]

On 28 November 2017 02:07, Guenter Roeck wrote:

> On 11/27/2017 05:38 PM, Badhri Jagan Sridharan wrote:
> > On Thu, Nov 23, 2017 at 3:10 AM, Adam Thomson
> >  wrote:
> >> On 16 November 2017 01:02, Badhri Jagan Sridharan wrote:
> >>
> >>> At present, TCPM code assumes that local device supports
> >>> variable/batt pdos and always selects the pdo with highest
> >>> possible power within the board limit. This assumption
> >>> might not hold good for all devices. To overcome this,
> >>> this patch makes TCPM only accept a source_pdo when there is
> >>> a matching sink pdo.
> >>>
> >>> For Fixed pdos: The voltage should match between the
> >>> incoming source_cap and the registered snk_pdo
> >>> For Variable/Batt pdos: The incoming source_cap voltage
> >>> range should fall within the registered snk_pdo's voltage
> >>> range.
> >>>
> >>> Also, when the cap_mismatch bit is set, the max_power/current
> >>> should be set to the max_current/power of the sink_pdo.
> >>> This is according to:
> >>>
> >>> "If the Capability Mismatch bit is set to one
> >>> The Maximum Operating Current/Power field may contain a value
> >>> larger than the maximum current/power offered in the Source
> >>> Capabilities message’s PDO as referenced by the Object position field.
> >>> This enables the Sink to indicate that it requires more current/power
> >>> than is being offered. If the Sink requires a different voltage this
> >>> will be indicated by its Sink Capabilities message.
> >>
> >> Hi Badhri,
> >>
> >> I have some queries on this change. At the moment the src/snk PDOs are hard
> >> coded in the relevant PD controller driver (e.g. fusb302.c), and the only 
> >> way
> >> to update these is to call the tcpm_update_source_capabilities() or
> >> tcpm_update_sink_capabilities() functions. However there are no real world
> >
> > Good point ! But I dont see any code which calls this either. I believe
> > Guenter added this. Guenter do you know ?
> >
> 
> A source might change its power capabilities dynamically. Practical example is
> a system with two USB-C ports. If one is in use, it may support 3A output 
> power.
> If two are in use, it may support 1.5A each. This isn't currently used in the
> kernel, but some of our devices have this behavior, so I thought it would be
> prudent to have support for it. The same is true for a sink, which may change
> its power requirements at runtime (same situation - a device is inserted at
> the second USB port) and ask for more or less power.
> 
> Is that a problem ? I won't object if you want to take it out because there is
> no current user, but you should keep in mind that those are valid use cases,
> and that the code should at least be extensible (ie the patch taking it away
> should be revertible) to support those use cases.

No, I have no desire to remove it and definitely see the valid use-cases. I was
curious though as to where these functions might be called from within the
kernel as wherever that is it will need to have the ability to retrieve the port
instance to be able to call these functions.

Re: EHCI and MUSB do not discover devices without CONFIG_PM

[Ladislav Michl]

On Tue, Nov 28, 2017 at 11:03:50AM +0200, Felipe Balbi wrote:
> Ladislav Michl  writes:
> > On Tue, Nov 28, 2017 at 08:33:28AM +0100, Greg KH wrote:
> >> On Mon, Nov 27, 2017 at 11:08:33PM +0100, Ladislav Michl wrote:
> >> > Hi there,
> >> > 
> >> > USB hosts do not discover any connected device on OMAP3 based board
> >> > with CONFIG_PM=n. Just enabling this option is enough to restore working
> >> > behaviour. Nothing unusual in log. Tested 4.14.2 and 4.15-rc1. I know
> >> > a lot of stuff depends on CONFIG_PM, but is this expected behaviour?
> >> > Neither EHCI nor MUSB is working without CONFIG_PM.
> >> 
> >> What bus type is your controllers on?  PCI?  platform?  Something else?
> >
> > Platform controllers inside OMAP3630 Soc.
> >
> >> And yes, perhaps this is to be expected, why would you not want
> >> CONFIG_PM to be enabled?  :)
> >
> > For a start, I know Linux is general purpose OS and I know I cannot expect
> > low latency or low jitter when dealing with interrupts.
> >
> > Original problem is described here:
> > https://www.spinics.net/lists/linux-omap/msg140081.html
> >
> > Shortly, with CONFIG_PM jitter of GPIO interrupt is about 350us which
> > renders IR receiver unuseable - is cannot reliably decode IR protocol
> > (gpio-ir-recv is used). With CONFIG_PM disabled, jitter is around 30us
> > and that's enough to make IR decoders work.
> >
> > And as I was unable to fix it, nor anyone provided useful hint, I though
> > I could work around problem from another side. And here we are...
> 
> Isn't it enough to just set a huge timeout for GPIO's runtime_pm? You
> can do that through sysfs. Just write a big number to the GPIO's
> autosuspend_delay_ms file. This should help you:

That does not help. And I already have Tony's hack in the patch stack:
https://patchwork.kernel.org/patch/9643499/
So this part of SoC is not powered off at all.

> modified   drivers/gpio/gpio-omap.c
> @@ -1238,6 +1238,8 @@ static int omap_gpio_probe(struct platform_device *pdev)
>  
>   platform_set_drvdata(pdev, bank);
>  
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_set_autosuspend_delay(dev, 6);
>   pm_runtime_enable(dev);
>   pm_runtime_irq_safe(dev);
>   pm_runtime_get_sync(dev);
> 
> 
> -- 
> 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: [PATCH] PM / runtime: Drop children check from __pm_runtime_set_status()

[Geert Uytterhoeven]

Hi Rafael, Shimoda-san,

On Sun, Nov 12, 2017 at 1:27 AM, Rafael J. Wysocki  wrote:
> From: Rafael J. Wysocki 
>
> The check for "active" children in __pm_runtime_set_status(), when
> trying to set the parent device status to "suspended", doesn't
> really make sense, because in fact it is not invalid to set the
> status of a device with runtime PM disabled to "suspended" in any
> case.  It is invalid to enable runtime PM for a device with its
> status set to "suspended" while its child_count reference counter
> is nonzero, but the check in __pm_runtime_set_status() doesn't
> really cover that situation.
>
> For this reason, drop the children check from __pm_runtime_set_status()
> and add a check against child_count reference counters of "suspended"
> devices to pm_runtime_enable().
>
> Signed-off-by: Rafael J. Wysocki 
> ---
>  drivers/base/power/runtime.c |   30 ++
>  1 file changed, 10 insertions(+), 20 deletions(-)
>
> Index: linux-pm/drivers/base/power/runtime.c
> ===
> --- linux-pm.orig/drivers/base/power/runtime.c
> +++ linux-pm/drivers/base/power/runtime.c
> @@ -1101,29 +1101,13 @@ int __pm_runtime_set_status(struct devic
> goto out;
> }
>
> -   if (dev->power.runtime_status == status)
> +   if (dev->power.runtime_status == status || !parent)
> goto out_set;
>
> if (status == RPM_SUSPENDED) {
> -   /*
> -* It is invalid to suspend a device with an active child,
> -* unless it has been set to ignore its children.
> -*/
> -   if (!dev->power.ignore_children &&
> -   atomic_read(>power.child_count)) {
> -   dev_err(dev, "runtime PM trying to suspend device but 
> active child\n");

JFTR, this triggered before during system resume on e.g. Salvator-XS with
R-Car H3:

ohci-platform ee08.usb: runtime PM trying to suspend device
but active child
phy_rcar_gen3_usb2 ee080200.usb-phy: runtime PM trying to suspend
device but active child
ohci-platform ee0c.usb: runtime PM trying to suspend device
but active child
ohci-platform ee0a.usb: runtime PM trying to suspend device
but active child
phy_rcar_gen3_usb2 ee0c0200.usb-phy: runtime PM trying to suspend
device but active child
phy_rcar_gen3_usb2 ee0a0200.usb-phy: runtime PM trying to suspend
device but active child

so this was an existing issue with USB before.

> -   error = -EBUSY;
> -   goto out;
> -   }
> -
> -   if (parent) {
> -   atomic_add_unless(>power.child_count, -1, 0);
> -   notify_parent = !parent->power.ignore_children;
> -   }
> -   goto out_set;
> -   }
> -
> -   if (parent) {
> +   atomic_add_unless(>power.child_count, -1, 0);
> +   notify_parent = !parent->power.ignore_children;
> +   } else {
> spin_lock_nested(>power.lock, SINGLE_DEPTH_NESTING);
>
> /*
> @@ -1307,6 +1291,12 @@ void pm_runtime_enable(struct device *de
> else
> dev_warn(dev, "Unbalanced %s!\n", __func__);
>
> +   WARN(dev->power.runtime_status == RPM_SUSPENDED &&
> +!dev->power.ignore_children &&
> +atomic_read(>power.child_count) > 0,
> +"Enabling runtime PM for inactive device (%s) with active 
> children\n",
> +dev_name(dev));

And now it became a bit more noisy:

Enabling runtime PM for inactive device (ee0a0200.usb-phy) with active children
WARNING: CPU: 0 PID: 1697 at drivers/base/power/runtime.c:1299
pm_runtime_enable+0x94/0xd8
CPU: 0 PID: 1697 Comm: s2idle Not tainted
4.15.0-rc1-arm64-renesas-00381-g40d152b966c941dd #41
Hardware name: Renesas Salvator-X 2nd version board based on r8a7795 ES2.0+ (DT)
task: 8006f81bb100 task.stack: 0aa8
pstate: 6085 (nZCv daIf -PAN -UAO)
pc : pm_runtime_enable+0x94/0xd8
lr : pm_runtime_enable+0x94/0xd8
sp : 0aa83b50
x29: 0aa83b50 x28: 8006f81bb100
x27: 08841000 x26: 08b4b640
x25: 08b7f6e0 x24: 097a2f90
x23: 08b7f000 x22: 0010
x21:  x20: 8006fa9ad158
x19: 8006fa9ad010 x18: 013c6577
x17: 0947ea80 x16: 6370
x15: 636e x14: 646c696863206576
x13: 0001 x12: 8006f81bbaa8
x11: 8006f9479230 x10: 8006f97a63e0
x9 :  x8 : 8006f97a6408
x7 : 8006f97a63c0 x6 : 0975de80
x5 :  x4 : 
x3 :  x2 : 08b4bbf0
x1 : 8006f81bb100 x0 : 004f
Call trace:
 pm_runtime_enable+0x94/0xd8
 device_resume_early+0x50/0xec
 dpm_resume_early+0x118/0x204
 

Re: xhci_hcd 0000:00:14.0: WARN Event TRB for slot 1 ep 2 with no TDs queued?

[Yaroslav Isakov]

Mathias,
Hello again! Any news on progress of upstreaming this patch?

2017-10-18 1:21 GMT+03:00 Yaroslav Isakov :
> Applied it to 4.13.7, and it helps! Thanks a lot!
>
> 2017-10-16 14:33 GMT+03:00 Mathias Nyman :
>> On 13.10.2017 14:46, Yaroslav Isakov wrote:
>>>
>>> Hello, Mathias! Did you get a chance to look into this bug?
>>
>>
>> Other things came up and this got a bit delayed.
>> Attached is a patch to test
>>
>> -Mathias
>>>
>>>
>>> 2017-07-20 19:15 GMT+03:00 Yaroslav Isakov :

 Yes, I can definitely test the patch

 2017-07-20 19:16 GMT+03:00 Mathias Nyman :
>
> On 20.07.2017 18:07, Yaroslav Isakov wrote:
>>
>>
>> Here it is
>>
>> 2017-07-20 18:06 GMT+03:00 Mathias Nyman
>> :
>>>
>>>
>>> On 20.07.2017 17:43, Yaroslav Isakov wrote:



 Hello everyone! I saw this thread some months ago, but do not know
 how
 to properly reply to it. I have the same problem, and it's just not
 few messages - it was about 40k messages in 10 minutes. No functional
 of my USB device (which is CCID USB token) is broken, just enormous
 amount of spam in the logs. I'm on 4.12 kernel, but I've seen this
 error on 4.11 and 4.9. Could this problem be caused by a bug in e.g.
 libusb, not a device itself?
 --
>>>
>>>
>>>
>>>
>>> I'd start by looking at xhci, can you take xhci traces of this?
>>>
>>> mount -t debugfs none /sys/kernel/debug
>>> echo 81920 > /sys/kernel/debug/tracing/buffer_size_kb
>>> echo 1 > /sys/kernel/debug/tracing/events/xhci-hcd/enable
>>>
>>> and then send me the content of
>>> /sys/kernel/debug/tracing/trace
>>>
>>> I'll be away for a couple of weeks, so next response might take some
>>> time
>>>
>>> -Mathias
>>>
>>>
>
> Think I found something.   (details mostly for myself in to remember
> this)
> A short transfer response on a bulk in transfer which is not the last
> TRB of
> a TD
>
> According to specs xhci will issue short transfer events both on the
> short
> TRB, _and_
> on the last TRB of the TD in case any previous TRB in the TD was short
> (see xhci 4.11.3.1)
>
> After the first short transfer event the driver fast forward past this
> TD.
> (and the last TRB)
> when we get the second short transfer event there are no TRBs queued and
> you
> see the
> warning.
>
> Warning should be harmless in this case, but annoying.
> If I write a patch can you try it out?
>
> As a reference, a snippet when log when a 65556 byte transfer is queued,
> split into 5 pieces (TRBS),
> and we get short transfer events for first and last TRB
>
> * A URB asking for 65556 bytes is queues:
> 2369.450362: xhci_urb_enqueue: ep1in-bulk: urb 8801398f6540 pipe
> 3221258880 slot 1 length 0/65556 sgs 5/5 stream 0 flags 00040200
> * split into 5 pieces ( TRBs at 0x0002169cb520, ..530, ..540, ..550
> and
> ..560)
> 2369.450363: xhci_queue_trb: BULK: Buffer 00010a7dc000 length 16384
> TD
> size 31 intr 0 type 'Normal' flags b:i:i:C:s:I:e:C
> 2369.450363: xhci_inc_enq: BULK 88021726c780: enq
> 0x0002169cb530(0x0002169cb000) deq
> 0x0002169cb520(0x0002169cb000) segs 2 stream 0 free_trbs 508
> bounce
> 6\
> 2369.450363: xhci_queue_trb: BULK: Buffer 00011e97 length 16384
> TD
> size 31 intr 0 type 'Normal' flags b:i:i:C:s:I:e:c
> 2369.450363: xhci_inc_enq: BULK 88021726c780: enq
> 0x0002169cb540(0x0002169cb000) deq
> 0x0002169cb520(0x0002169cb000) segs 2 stream 0 free_trbs 507
> bounce
> 6\
> 2369.450363: xhci_queue_trb: BULK: Buffer 0001b63ac000 length 16384
> TD
> size 31 intr 0 type 'Normal' flags b:i:i:C:s:I:e:c
> 2369.450363: xhci_inc_enq: BULK 88021726c780: enq
> 0x0002169cb550(0x0002169cb000) deq
> 0x0002169cb520(0x0002169cb000) segs 2 stream 0 free_trbs 506
> bounce
> 6\
> 2369.450364: xhci_queue_trb: BULK: Buffer 00010a7ac000 length 16384
> TD
> size 1 intr 0 type 'Normal' flags b:i:i:C:s:I:e:c
> 2369.450364: xhci_inc_enq: BULK 88021726c780: enq
> 0x0002169cb560(0x0002169cb000) deq
> 0x0002169cb520(0x0002169cb000) segs 2 stream 0 free_trbs 505
> bounce
> 6\
> 2369.450364: xhci_queue_trb: BULK: Buffer 0001a26adf40 length 20 TD
> size
> 0 intr 0 type 'Normal' flags b:i:I:c:s:I:e:c
> 2369.450364: xhci_inc_enq: BULK 88021726c780: enq
> 0x0002169cb570(0x0002169cb000) deq
> 0x0002169cb520(0x0002169cb000) segs 2 stream 0 free_trbs 504
> bounce

RE: dwc2/dwc_otg drivers

[Felipe Balbi]

Hi,

"Araujo, Billy"  writes:
> Hi Felipe,
>
> I think the issue is very similar to the one indicated by Johan Hovold:
>
> https://lkml.kernel.org/r/20171030170802.14489-1-diand...@chromium.org
>
> Eventually we would like to test this patch on our system and check if it 
> works for us.

still top-posting.

> Please be advised that this email may contain confidential
> information. If you are not the intended recipient, please notify us
> by email by replying to the sender and delete this message. The sender
> disclaims that the content of this email constitutes an offer to enter
> into, or the acceptance of, any agreement; provided that the foregoing
> does not invalidate the binding effect of any digital or other
> electronic reproduction of a manual signature that is included in any
> attachment.

message deleted

-- 
balbi


signature.asc
Description: PGP signature

Re: [RFC PATCH v2 6/7] typec: tcpm: Represent source supply through power_supply class

[Heikki Krogerus]

On Mon, Nov 27, 2017 at 04:54:08PM +, Adam Thomson wrote:
> On 27 November 2017 14:12, Heikki Krogerus wrote:
> 
> > Hi Adam,
> > 
> > On Fri, Nov 24, 2017 at 02:05:27PM +, Adam Thomson wrote:
> > > On 24 November 2017 12:19, Heikki Krogerus wrote:
> > > > Is it OK to everybody that the type of the psy is changed like that?
> > > > Hans?!
> > > >
> > > > We do have drivers that already change the type, for example
> > > > drivers/power/supply/isp1704_charger.c, but what does the user space
> > > > expect? The ABI for the power supply class was never documented I
> > > > guess.
> > > >
> > >
> > > Hi Heikki,
> > >
> > > Appreciate your time in reviewing this.
> > >
> > > Yes, I actually saw that as an example when I considered this approach. I 
> > > didn't
> > > see anything obvious for this in the ABI documentation. Any ideas 
> > > Sebastian?
> > > What is user-space expectation for the 'type' property of a power_supply? 
> > > I
> > > assume having this dynamic is ok given existing drivers can already do 
> > > something
> > > like this, but would be good to have clarification.
> > >
> > > > I'm not against changing the type, but I think that we should have an
> > > > attribute file listing all supported types a psy can have if we go
> > > > forward with this. Ideally the type file would just list them as space
> > > > separated values, and show the current one with asterisk in front of
> > > > it. The output would be similar we have with some of the other files
> > > > under /sys/power, at least /sys/power/state, but that would break the
> > > > ABI.
> > > >
> > >
> > > I added this as I wanted the user to know what was connected rather than
> > > blindly trying to set the 'online' property to enable PPS, even if the 
> > > attached
> > > source partner didn't support this. As you say, am not sure we could 
> > > change the
> > > 'TYPE' property as that to my knowledge has always been a single string.
> > >
> > > Maybe the addition of a 'SUPPORTED_TYPES' property or something similar 
> > > could
> > > close this gap (as you eluded to), at least by providing a RO list of all
> > > supported types? Another option would be to add a type which indicates the
> > > supply supports multiple types, and then based on this we can read another
> > > property which does as you suggest with multiple strings and one being
> > > highlighted? Am certainly open to discussion on this.
> > 
> > It looks like this is USB specific problem. I think the type should
> > actually be just USB, and there should be an other USB only attribute
> > file which should list the current and supported connection types.
> > Well, maybe it does not even need to be USB only.
> 
> I believe in Android (BatteryMonitor) right now it maps the various USB
> types that can be reported by a power_supply driver to an internal Android 
> 'AC'
> type (or 'USB' for 'PD_DRP') for use in upper level software, so they handle 
> the
> various USB reported types but simplify reporting. That code seems like it 
> would
> also handle changes in type as well because the type is re-read when dealing
> with updates. So there is a wide spread user which already 'copes' with 
> current
> driver implementations. That's not saying it's necessarily the correct way to 
> go
> in the future but wanted to highlight current usage.
> 
> Personally what you're suggesting seems reasonable to me. I would however like
> to get a general consensus on this as I guess this could potentially change
> future power_supply driver implementations.

Agreed. What ever the final solution will be, the power supply ABI
should be documented at the same time.

> Sebastian, do you have any thoughts on this topic?


Cheers,

-- 
heikki
--
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: dwc2/dwc_otg drivers

[Araujo, Billy]

Hi Felipe,

I think the issue is very similar to the one indicated by Johan Hovold:

https://lkml.kernel.org/r/20171030170802.14489-1-diand...@chromium.org

Eventually we would like to test this patch on our system and check if it works 
for us.


Regards,

Billy.


-Original Message-
From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com]
Sent: 27 November 2017 11:50
To: Billy Araujo ; Minas Harutyunyan 

Cc: linux-usb@vger.kernel.org; Araujo, Billy 
Subject: Re: dwc2/dwc_otg drivers


Hi,

Billy Araujo  writes:
> Hi Felipe and Minas,
>
>
> Firstly, thank you very much for your responses.
>
> I work as a software engineer at Fluke.
> We compile our own version of the kernel but I am not the Linux kernel
> maintainer at Fluke, I work mostly on the application side using Qt
> and
> C++.
>
> I am still learning about the Linux kernel so sorry if this is a dumb
> question but can these SOF interrupts be disabled in the kernel/controller?

possibly, but it may be that SOF interrupts are needed for whatever Erratum 
implementation. Check the sources.

> We are using Linux based on Angstrom (v2016.06), built using Yocto
> recipes (I think) and using a USB analyser I see that are millions of
> NAKs even on a relatively short capture.The problem is that when we
> initialise a 3rd party kernel module/library the system slows down
> significantly. I think they tested it on Raspberry PI and it worked
> fine. So not sure where the problem is. But now I know where to look.

I'd ask for support from the vendor giving you the kernel and the other vendor 
giving you the 3rd party driver that slows down the system. Keep in mind you're 
already paying both suppliers for support, might as well use it.

good luck

--
balbi




Please be advised that this email may contain confidential information. If you 
are not the intended recipient, please notify us by email by replying to the 
sender and delete this message. The sender disclaims that the content of this 
email constitutes an offer to enter into, or the acceptance of, any agreement; 
provided that the foregoing does not invalidate the binding effect of any 
digital or other electronic reproduction of a manual signature that is included 
in any attachment.
--
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: EHCI and MUSB do not discover devices without CONFIG_PM

[Greg KH]

On Tue, Nov 28, 2017 at 09:57:51AM +0100, Ladislav Michl wrote:
> Hi Greg,
> 
> On Tue, Nov 28, 2017 at 08:33:28AM +0100, Greg KH wrote:
> > On Mon, Nov 27, 2017 at 11:08:33PM +0100, Ladislav Michl wrote:
> > > Hi there,
> > > 
> > > USB hosts do not discover any connected device on OMAP3 based board
> > > with CONFIG_PM=n. Just enabling this option is enough to restore working
> > > behaviour. Nothing unusual in log. Tested 4.14.2 and 4.15-rc1. I know
> > > a lot of stuff depends on CONFIG_PM, but is this expected behaviour?
> > > Neither EHCI nor MUSB is working without CONFIG_PM.
> > 
> > What bus type is your controllers on?  PCI?  platform?  Something else?
> 
> Platform controllers inside OMAP3630 Soc.
> 
> > And yes, perhaps this is to be expected, why would you not want
> > CONFIG_PM to be enabled?  :)
> 
> For a start, I know Linux is general purpose OS and I know I cannot expect
> low latency or low jitter when dealing with interrupts.

Well, it's the best latency of any other OS out there :)

Anyway, if you want guaranteed response time, you are going to have to
use the RT patchset, no matter what.  Otherwise you have the potential
to have bad jitter at times.

> Original problem is described here:
> https://www.spinics.net/lists/linux-omap/msg140081.html
> 
> Shortly, with CONFIG_PM jitter of GPIO interrupt is about 350us which
> renders IR receiver unuseable - is cannot reliably decode IR protocol
> (gpio-ir-recv is used). With CONFIG_PM disabled, jitter is around 30us
> and that's enough to make IR decoders work.

bit-banging an ir decoder, ugh, you are in for a world of hurt.  Can't
you put a chip on the device that does this for you in hardware?

Anyway, good luck!

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: dwc2/dwc_otg drivers

[Felipe Balbi]

Hi,

(no top-posting)

"Araujo, Billy"  writes:
> Hi Felipe,
>
> I think the issue is very similar to the one indicated by Johan Hovold:
>
> https://lkml.kernel.org/r/20171030170802.14489-1-diand...@chromium.org
>
> Eventually we would like to test this patch on our system and check if it 
> works for us.
>
>
> Regards,
>
> Billy.
>
>
> -Original Message-
> From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com]
> Sent: 27 November 2017 11:50
> To: Billy Araujo ; Minas Harutyunyan 
> 
> Cc: linux-usb@vger.kernel.org; Araujo, Billy 
> Subject: Re: dwc2/dwc_otg drivers
>
>
> Hi,
>
> Billy Araujo  writes:
>> Hi Felipe and Minas,
>>
>>
>> Firstly, thank you very much for your responses.
>>
>> I work as a software engineer at Fluke.
>> We compile our own version of the kernel but I am not the Linux kernel
>> maintainer at Fluke, I work mostly on the application side using Qt
>> and
>> C++.
>>
>> I am still learning about the Linux kernel so sorry if this is a dumb
>> question but can these SOF interrupts be disabled in the kernel/controller?
>
> possibly, but it may be that SOF interrupts are needed for whatever Erratum 
> implementation. Check the sources.
>
>> We are using Linux based on Angstrom (v2016.06), built using Yocto
>> recipes (I think) and using a USB analyser I see that are millions of
>> NAKs even on a relatively short capture.The problem is that when we
>> initialise a 3rd party kernel module/library the system slows down
>> significantly. I think they tested it on Raspberry PI and it worked
>> fine. So not sure where the problem is. But now I know where to look.
>
> I'd ask for support from the vendor giving you the kernel and the other 
> vendor giving you the 3rd party driver that slows down the system. Keep in 
> mind you're already paying both suppliers for support, might as well use it.
>
> good luck
>
> --
> balbi
> 
> 
>
>
> Please be advised that this email may contain confidential information. If 
> you are not the intended recipient, please notify us by email by replying to 
> the sender and delete this message. The sender disclaims that the content of 
> this email constitutes an offer to enter into, or the acceptance of, any 
> agreement; provided that the foregoing does not invalidate the binding effect 
> of any digital or other electronic reproduction of a manual signature that is 
> included in any attachment.

message deleted!

-- 
balbi


signature.asc
Description: PGP signature

[PATCH v1] usb: xhci: allow imod-interval to be configurable

[Adam Wallis]

The xHCI driver currently has the IMOD set to 160, which
translates to an IMOD interval of 40,000ns (160 * 250)ns

Commit 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller")
introduced a QUIRK for the MTK platform to adjust this interval to 20,
which translates to an IMOD interval of 5,000ns (20 * 250)ns. This is
due to the fact that the MTK controller IMOD interval is 8 times
as much as defined in xHCI spec.

Instead of adding more quirk bits for additional platforms, this patch
introduces the ability for vendors to set the IMOD_INTERVAL as is
optimal for their platform. By using device_property_read_u32() on
"imod-interval", the IMOD INTERVAL can be specified in nano seconds. If
no interval is specified, the default of 40,000ns (IMOD=160) will be
used.

No bounds checking has been implemented due to the fact that a vendor
may have violated the spec and would need to specify a value outside of
the max 8,000 IRQs/second limit specified in the xHCI spec.

Backwards compatibility is maintained for MTK_HOSTS through the quirk
bit, however, imod_interval should be pushed into device tree at a
future point and this quirk should be removed from xhci_plat_probe

Signed-off-by: Adam Wallis 
---
 Documentation/devicetree/bindings/usb/usb-xhci.txt |  1 +
 drivers/usb/host/xhci-plat.c   | 15 +++
 drivers/usb/host/xhci.c|  7 ++-
 drivers/usb/host/xhci.h|  2 ++
 4 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt 
b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index ae6e484..3998459 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -29,6 +29,7 @@ Optional properties:
   - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
   - usb3-lpm-capable: determines if platform is USB3 LPM capable
   - quirk-broken-port-ped: set if the controller has broken port disable 
mechanism
+  - imod-interval: IMOD_INTERVAL in nano-seconds. Default is 4
 
 Example:
usb@f0931000 {
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 09f164f..f7730c8 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -23,6 +23,7 @@
 #include "xhci-plat.h"
 #include "xhci-mvebu.h"
 #include "xhci-rcar.h"
+#include "xhci-mtk.h"
 
 static struct hc_driver __read_mostly xhci_plat_hc_driver;
 
@@ -269,6 +270,20 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (device_property_read_bool(>dev, "quirk-broken-port-ped"))
xhci->quirks |= XHCI_BROKEN_PORT_PED;
 
+   /* imod interval in nanoseconds */
+   if (device_property_read_u32(sysdev,
+   "imod-interval", >imod_interval))
+   xhci->imod_interval = 4;
+   else if (xhci->quirks & XHCI_MTK_HOST)
+   /*
+* The increment interval is 8 times as much as that defined in
+* the xHCI spec on MTK's controller. This is added to provide
+* backwards compatibility, however, this should be pushed into
+* the device tree files at some point in the future and
+* removed from xhci_plat_probe
+*/
+   xhci->imod_interval = 5000;
+
hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0);
if (IS_ERR(hcd->usb_phy)) {
ret = PTR_ERR(hcd->usb_phy);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2424d30..6a09311 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -586,11 +586,8 @@ int xhci_run(struct usb_hcd *hcd)
"// Set the interrupt modulation register");
temp = readl(>ir_set->irq_control);
temp &= ~ER_IRQ_INTERVAL_MASK;
-   /*
-* the increment interval is 8 times as much as that defined
-* in xHCI spec on MTK's controller
-*/
-   temp |= (u32) ((xhci->quirks & XHCI_MTK_HOST) ? 20 : 160);
+   temp |= (u16) (xhci->imod_interval / 250);
+
writel(temp, >ir_set->irq_control);
 
/* Set the HCD state before we enable the irqs */
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 99a014a..2a4177b 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1717,6 +1717,8 @@ struct xhci_hcd {
u8  max_interrupters;
u8  max_ports;
u8  isoc_threshold;
+   /* imod_interval in ns (I * 250ns) */
+   u32 imod_interval;
int event_ring_max;
/* 4KB min, 128MB max */
int page_size;
-- 
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

--
To unsubscribe 

Re: [PATCH] PM / runtime: Drop children check from __pm_runtime_set_status()

[Ulf Hansson]

On 28 November 2017 at 13:48, Yoshihiro Shimoda
 wrote:
> Hi Geert-san,
>
>> From: Geert Uytterhoeven, Sent: Tuesday, November 28, 2017 7:58 PM
>>
>> Hi Rafael, Shimoda-san,
>>
>> On Sun, Nov 12, 2017 at 1:27 AM, Rafael J. Wysocki  
>> wrote:
>> > From: Rafael J. Wysocki 
>> >
>> > The check for "active" children in __pm_runtime_set_status(), when
>> > trying to set the parent device status to "suspended", doesn't
>> > really make sense, because in fact it is not invalid to set the
>> > status of a device with runtime PM disabled to "suspended" in any
>> > case.  It is invalid to enable runtime PM for a device with its
>> > status set to "suspended" while its child_count reference counter
>> > is nonzero, but the check in __pm_runtime_set_status() doesn't
>> > really cover that situation.
>> >
>> > For this reason, drop the children check from __pm_runtime_set_status()
>> > and add a check against child_count reference counters of "suspended"
>> > devices to pm_runtime_enable().
>> >
>> > Signed-off-by: Rafael J. Wysocki 
>> > ---
>> >  drivers/base/power/runtime.c |   30 ++
>> >  1 file changed, 10 insertions(+), 20 deletions(-)
>> >
>> > Index: linux-pm/drivers/base/power/runtime.c
>> > ===
>> > --- linux-pm.orig/drivers/base/power/runtime.c
>> > +++ linux-pm/drivers/base/power/runtime.c
>> > @@ -1101,29 +1101,13 @@ int __pm_runtime_set_status(struct devic
>> > goto out;
>> > }
>> >
>> > -   if (dev->power.runtime_status == status)
>> > +   if (dev->power.runtime_status == status || !parent)
>> > goto out_set;
>> >
>> > if (status == RPM_SUSPENDED) {
>> > -   /*
>> > -* It is invalid to suspend a device with an active child,
>> > -* unless it has been set to ignore its children.
>> > -*/
>> > -   if (!dev->power.ignore_children &&
>> > -   atomic_read(>power.child_count)) {
>> > -   dev_err(dev, "runtime PM trying to suspend device 
>> > but active child\n");
>>
>> JFTR, this triggered before during system resume on e.g. Salvator-XS with
>> R-Car H3:
>>
>> ohci-platform ee08.usb: runtime PM trying to suspend device
>> but active child
>> phy_rcar_gen3_usb2 ee080200.usb-phy: runtime PM trying to suspend
>> device but active child
>> ohci-platform ee0c.usb: runtime PM trying to suspend device
>> but active child
>> ohci-platform ee0a.usb: runtime PM trying to suspend device
>> but active child
>> phy_rcar_gen3_usb2 ee0c0200.usb-phy: runtime PM trying to suspend
>> device but active child
>> phy_rcar_gen3_usb2 ee0a0200.usb-phy: runtime PM trying to suspend
>> device but active child
>>
>> so this was an existing issue with USB before.
>
> Thank you for the report!
> I know that, but since this didn't cause any trouble until now,
> I postponed to investigate the issue... But, I investigate it today.
> I don't find the root cause yet. However, it seems related to usb host and/or 
> usb core.
> --> USB host related devices' child_count will be 1 in suspend timing.
>  --> I guess remote wakeup feature is enabled? But, I don't find the point 
> yet.

I am guessing the issue is triggered by genpd in the suspend noirq
phase (genpd_suspend_noirq()). In there,  there is a call to
pm_runtime_force_suspend() (which calls pm_runtime_set_suspended() and
which triggered the earlier error messages being printed).

The reason why genpd calls pm_runtime_force_suspend(), is because when
validating wakeup configurations for the device "if
(dev->power.wakeup_path && genpd_is_active_wakeup(genpd))", it's
thinks wakeup isn't configured while it probably should be.

An additional note, only when genpd has the GENPD_FLAG_PM_CLK set,
which makes the genpd->dev_ops.stop|start() being assigned, genpd
calls pm_runtime_force_suspend() - else it doesn't.

Perhaps try out the series I recently posted improving the code
dealing with wakeups in genpd and the PM core:
https://www.spinics.net/lists/linux-renesas-soc/msg20122.html
To that, you need to set the new flag (invented in the above series)
DPM_FLAG_IN_BAND_WAKEUP in the driver that configures wakeup of its
device.

Hope this helps!

>
> The renesas_usbhs also uses the phy_rcar_gen3_usb2 driver.
> --> If I only used the renesas_usbhs driver (in other words, I don't install
> [eo]hci-{hcd,platform} drivers), the issue disappeared.
>  --> So, I think the phy_rcar_gen3_usb2 driver doesn't cause this issue.
> (But, it is possible to be related though.)
>
> I'll continue to investigate this issue tomorrow.

Please keep me posted, I am interested about the why the problem exists. :-)

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the 

Re: EHCI and MUSB do not discover devices without CONFIG_PM

[Alan Stern]

On Tue, 28 Nov 2017, Ladislav Michl wrote:

> On Tue, Nov 28, 2017 at 10:00:32AM -0500, Alan Stern wrote:
> > On Tue, 28 Nov 2017, Greg KH wrote:
> > 
> > > On Mon, Nov 27, 2017 at 11:08:33PM +0100, Ladislav Michl wrote:
> > > > Hi there,
> > > > 
> > > > USB hosts do not discover any connected device on OMAP3 based board
> > > > with CONFIG_PM=n. Just enabling this option is enough to restore working
> > > > behaviour. Nothing unusual in log. Tested 4.14.2 and 4.15-rc1. I know
> > > > a lot of stuff depends on CONFIG_PM, but is this expected behaviour?
> > > > Neither EHCI nor MUSB is working without CONFIG_PM.
> > > 
> > > What bus type is your controllers on?  PCI?  platform?  Something else?
> > > 
> > > And yes, perhaps this is to be expected, why would you not want
> > > CONFIG_PM to be enabled?  :)
> > 
> > Well, no, it's not expected.  Systems are supposed to work correctly 
> > with CONFIG_PM=n -- just not at an optimum power level.
> 
> Here's relevant part of bootlog:
...

> > EHCI certainly should work without CONFIG_PM.  When a device is plugged 
> > in to the EHCI controller, what shows up in 
> > /sys/kernel/debug/usb/ehci/*/registers (fill in the '*' with the 
> > correct bus ID)?
> 
> $ ls /sys/kernel/debug/usb
> devices  ohci
> 
> So no ehci.
...

> For ohci it is the same. Any hint why there's no ehco file?

The EHCI debugging files require CONFIG_DYNAMIC_DEBUG to be enabled.  
Oddly enough, the OHCI debugging files do not have the same 
requirement.  I don't know the reason for this difference.

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: usbip port number limits

[Shuah Khan]

On 11/26/2017 06:33 PM, Yuyang Du wrote:
> Hi Juan,
> 
> On Fri, Nov 24, 2017 at 12:42:22PM +0100, Juan Zea wrote:
>>The patch doesn't apply cleanly with the patch command, but given it is 
>> that simple I've changed it myself (I'm telling you just in case we're 
>> missing something).
>>
>>The fingerprint reader, usb stick and wacom tablet work :)
>  
> Good to hear that.
> 
>> I've also checked the same patch can be applied to kernel master and it 
>> works. So... is that the solution?
> 
> I'll post a patch later, but before that, Hi, Shuah, does it work
> for you, on your test notebook?
> 

I am going to test this today and if it is fixes the problem on
my system.

thanks,
-- Shuah
--
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: EHCI and MUSB do not discover devices without CONFIG_PM

[Ladislav Michl]

On Tue, Nov 28, 2017 at 06:39:13AM -0800, Tony Lindgren wrote:
> * Ladislav Michl  [171128 14:31]:
> > On Tue, Nov 28, 2017 at 06:11:31AM -0800, Tony Lindgren wrote:
> > > * Ladislav Michl  [171128 09:42]:
> > > > On Tue, Nov 28, 2017 at 10:30:54AM +0100, Greg KH wrote:
> > > > > bit-banging an ir decoder, ugh, you are in for a world of hurt.  Can't
> > > > > you put a chip on the device that does this for you in hardware?
> > > > 
> > > > OMAP has DM timer which can be externally trigered on edge. Perfect for
> > > > that purpose. But I cannot pinmux its input as hw designers did poor 
> > > > job.
> > > > And there are thousands of devices deployed.
> > > > 
> > > > So it is about a lot of soldering or providing software solution.
> > > > 
> > > > > Anyway, good luck!
> > > > 
> > > > A little pointer would increase "luck" by several order of magnitudes.
> > > 
> > > Hmm did you already try limiting cpuidle to C1 only in /sys?
> > 
> > I have CONFIG_CPU_IDLE=n, which should be the same. Is that right?
> 
> You will then use the omap3_pm_idle() that does not know much
> anything about latencies.
> 
> > > From what I recall you just set the latency to less than C2
> > > has. The cpuidle latencies are in struct omap3_idle_statedata
> > > omap3_idle_data[].
> > 
> > I disabled cpuidle and frequency scaling completely. The only thing that
> > make jitter difference is CONFIG_PM (see original thread).
> 
> Right as then omap3_pm_idle() is disabled and only WFI is done :)
> 
> Limiting things to C1 with cpuidle is probably what you need
> for having USB also working.

With echo 1 > /sys/devices/system/cpu/cpu0/cpuidle/state{1-6}/disable
jitter is about 100us which is about 3x worse than with CONFIG_PM
disabled (and much better when idling), but decoder works with
occassional errors.

Anyway, it would be nice to find out why USB does not work with CONFIG_PM
disabled :)

> > There's lot of pm_wkup interrupts with CONFIG_PM - order of magnitude
> > more than gp_timer.
> 
> Hmm yeah that is true. And idling and waking up devices also takes
> some time.

Thank you,
ladis
--
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: core: add support for USB_REQ_SET_ISOCH_DELAY

[Felipe Balbi]

Hi,

Greg Kroah-Hartman  writes:
>> Felipe Balbi  writes:
>> > Greg Kroah-Hartman  writes:
>> >> On Thu, Nov 09, 2017 at 03:41:54PM +0200, Felipe Balbi wrote:
>> >>> USB SS and SSP hubs provide wHubDelay values on their hub descriptor
>> >>> which we should inform the USB Device about.
>> >>> 
>> >>> The USB Specification 3.0 explains, on section 9.4.11, how to
>> >>> calculate the value and how to issue the request. Note that a
>> >>> USB_REQ_SET_ISOCH_DELAY is valid on all device states (Default,
>> >>> Address, Configured), we just *chose* to issue it from Address state
>> >>> right after successfully fetching the USB Device Descriptor.
>> >>
>> >> I missed that this was in the 3.0 spec, I had heard about it a long time
>> >> ago.  Do we have any devices out there that actually set this value?  Or
>> >> on the other hand, can anyone use it yet (I know audio wanted it...)
>> >
>> > Well, USB 3.0 devices are required to accept this request, so my comment
>> > that it's "optional", isn't entirely correct. It's part of Chapter9
>> > testing on USBCV. Those failing this test, wouldn't get a USB
>> > sticker. DWC3, for example, receives the number, but can't do much about
>> > it so far. We could, eventually, pass it to gadget drivers and let them
>> > use the value to figure out how many requests they should queue ahead of
>> > time for isoc endpoints.
>> >
>> >>> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
>> >>> index 371a07d874a3..c1db751163d5 100644
>> >>> --- a/drivers/usb/core/message.c
>> >>> +++ b/drivers/usb/core/message.c
>> >>> @@ -915,6 +915,30 @@ int usb_get_device_descriptor(struct usb_device 
>> >>> *dev, unsigned int size)
>> >>>  return ret;
>> >>>  }
>> >>>  
>> >>> +/*
>> >>> + * usb_set_isoch_delay - informs the device of the packet transmit delay
>> >>> + * @dev: the device whose delay is to be informed
>> >>> + * Context: !in_interrupt()
>> >>> + *
>> >>> + * Since this is an optional request, we don't bother if it fails.
>> >>
>> >> But we should return an error, right?  Are devices that don't expect
>> >
>> > Right, I'm not sure :-) Do we care if we get a stall on this one?
>> >
>> >> this request going to have problems?
>> >
>> > No idea. I assume we won't. I can add the error and handle it from the
>> > calling site.
>> 
>> FYI, here's a version with the error checks. If you prefer this version,
>> I can put it to Intel's testing farm for a while and see if any problems
>> are caught, then resend it once merge window closes.
>> 
>> Let me know
>
> Yes, that looks better.

It should be in Intel's testing now. I'll resend it in a week or so.

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH] PM / runtime: Drop children check from __pm_runtime_set_status()

[Alan Stern]

On Tue, 28 Nov 2017, Yoshihiro Shimoda wrote:

> Hi Geert-san,
> 
> > From: Geert Uytterhoeven, Sent: Tuesday, November 28, 2017 7:58 PM
> > 
> > Hi Rafael, Shimoda-san,
> > 
> > On Sun, Nov 12, 2017 at 1:27 AM, Rafael J. Wysocki  
> > wrote:
> > > From: Rafael J. Wysocki 
> > >
> > > The check for "active" children in __pm_runtime_set_status(), when
> > > trying to set the parent device status to "suspended", doesn't
> > > really make sense, because in fact it is not invalid to set the
> > > status of a device with runtime PM disabled to "suspended" in any
> > > case.  It is invalid to enable runtime PM for a device with its
> > > status set to "suspended" while its child_count reference counter
> > > is nonzero, but the check in __pm_runtime_set_status() doesn't
> > > really cover that situation.
> > >
> > > For this reason, drop the children check from __pm_runtime_set_status()
> > > and add a check against child_count reference counters of "suspended"
> > > devices to pm_runtime_enable().
> > >
> > > Signed-off-by: Rafael J. Wysocki 
> > > ---
> > >  drivers/base/power/runtime.c |   30 ++
> > >  1 file changed, 10 insertions(+), 20 deletions(-)
> > >
> > > Index: linux-pm/drivers/base/power/runtime.c
> > > ===
> > > --- linux-pm.orig/drivers/base/power/runtime.c
> > > +++ linux-pm/drivers/base/power/runtime.c
> > > @@ -1101,29 +1101,13 @@ int __pm_runtime_set_status(struct devic
> > > goto out;
> > > }
> > >
> > > -   if (dev->power.runtime_status == status)
> > > +   if (dev->power.runtime_status == status || !parent)
> > > goto out_set;
> > >
> > > if (status == RPM_SUSPENDED) {
> > > -   /*
> > > -* It is invalid to suspend a device with an active child,
> > > -* unless it has been set to ignore its children.
> > > -*/
> > > -   if (!dev->power.ignore_children &&
> > > -   atomic_read(>power.child_count)) {
> > > -   dev_err(dev, "runtime PM trying to suspend device 
> > > but active child\n");
> > 
> > JFTR, this triggered before during system resume on e.g. Salvator-XS with
> > R-Car H3:
> > 
> > ohci-platform ee08.usb: runtime PM trying to suspend device
> > but active child
> > phy_rcar_gen3_usb2 ee080200.usb-phy: runtime PM trying to suspend
> > device but active child
> > ohci-platform ee0c.usb: runtime PM trying to suspend device
> > but active child
> > ohci-platform ee0a.usb: runtime PM trying to suspend device
> > but active child
> > phy_rcar_gen3_usb2 ee0c0200.usb-phy: runtime PM trying to suspend
> > device but active child
> > phy_rcar_gen3_usb2 ee0a0200.usb-phy: runtime PM trying to suspend
> > device but active child
> > 
> > so this was an existing issue with USB before.
> 
> Thank you for the report!
> I know that, but since this didn't cause any trouble until now,
> I postponed to investigate the issue... But, I investigate it today.
> I don't find the root cause yet. However, it seems related to usb host and/or 
> usb core.
> --> USB host related devices' child_count will be 1 in suspend timing.
>  --> I guess remote wakeup feature is enabled? But, I don't find the point 
> yet.
> 
> The renesas_usbhs also uses the phy_rcar_gen3_usb2 driver.
> --> If I only used the renesas_usbhs driver (in other words, I don't install
> [eo]hci-{hcd,platform} drivers), the issue disappeared.
>  --> So, I think the phy_rcar_gen3_usb2 driver doesn't cause this issue.
> (But, it is possible to be related though.)
> 
> I'll continue to investigate this issue tomorrow.

Does the phy_rcar_gen3_usb2 driver use runtime PM?  It looks like the 
phy device somehow gets enabled for runtime PM when it shouldn't be.

(And by the way, what device is the child of ee0a0200.usb-phy?)

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: EHCI and MUSB do not discover devices without CONFIG_PM

[Tony Lindgren]

* Ladislav Michl  [171128 17:01]:
> On Tue, Nov 28, 2017 at 06:39:13AM -0800, Tony Lindgren wrote:
> > Limiting things to C1 with cpuidle is probably what you need
> > for having USB also working.
> 
> With echo 1 > /sys/devices/system/cpu/cpu0/cpuidle/state{1-6}/disable
> jitter is about 100us which is about 3x worse than with CONFIG_PM
> disabled (and much better when idling), but decoder works with
> occassional errors.

OK sounds like there is still something happening in the idle path
beyond WFI then.

> Anyway, it would be nice to find out why USB does not work with CONFIG_PM
> disabled :)

Sure.

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

Re: [PATCHv2] USB: usbfs: Filter flags passed in from user space

[Greg KH]

On Thu, Nov 23, 2017 at 10:53:13AM -0500, Alan Stern wrote:
> On Thu, 23 Nov 2017, Oliver Neukum wrote:
> 
> > USBDEVFS_URB_ISO_ASAP must be accepted only for ISO endpoints.
> > Improve sanity checking.
> > 
> > Reported-by: andreyk...@google.com
> 
> This should be
> 
> Reported-by: Andrey Konovalov 
> 
> > Signed-off-by: Oliver Neukum 
> > CC: sta...@vger.kernel.org
> > ---
> >  drivers/usb/core/devio.c | 16 ++--
> >  1 file changed, 10 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> > index 705c573d0257..701ddada389a 100644
> > --- a/drivers/usb/core/devio.c
> > +++ b/drivers/usb/core/devio.c
> > @@ -1442,14 +1442,18 @@ static int proc_do_submiturb(struct usb_dev_state 
> > *ps, struct usbdevfs_urb *uurb
> > int number_of_packets = 0;
> > unsigned int stream_id = 0;
> > void *buf;
> > -
> > -   if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP |
> > -   USBDEVFS_URB_SHORT_NOT_OK |
> > +   unsigned long mask =USBDEVFS_URB_SHORT_NOT_OK |
> > USBDEVFS_URB_BULK_CONTINUATION |
> > USBDEVFS_URB_NO_FSBR |
> > -   USBDEVFS_URB_ZERO_PACKET |
> > -   USBDEVFS_URB_NO_INTERRUPT))
> > -   return -EINVAL;
> > +   USBDEVFS_URB_ZERO_PACKET | 
> 
> Extra whitespace at end of line (doesn't checkpatch.pl catch this)?

I'll go edit it by hand...

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

usb: gadget: ffs: Fix BUG when userland exits with submitted AIO transfers

[Vincent Pelletier]

!! This is an RFC patch - hence sign-off absence !!

This bug happens only when the UDC needs to sleep during usb_ep_dequeue,
as is the case for (at least) dwc3.

RFC:
- I do not understand what eps_lock is supposed to protect in original
  ffs_aio_cancel implementation, especially once one considers
  usb_ep_dequeue should be allowed to sleep: no non-intricated locking
  scheme would allow this. I removed this lock, I may be doing something
  wrong.
- Because the 3 other locks also disable interrupts, I believe the only
  valid fix is to move usb_ep_dequeue to another call stack.
  Not being sure I could safely reuse ffs_io_data.work (completion
  function will be called during usb_ep_dequeue call, so during
  ffs_aio_cancel_worker call), I added a new work_struct.
  I believe it is safe to deffer cancellation from io_data point of view,
  as ffs owns its buffer, and from aio point of view as regular io_cancel
  return -EINPROGRESS when kiocb_cancel succeeds, implying kiocb_cancel
  is not required to be fully effective synchronously.
  Sadly, kiocb_set_cancel_fn is only used in ffs and its legacy
  counterpart, so I have very little precedents to look at.
  read_buffer gets freed in ffs_func_eps_disable, but it does not matter
  for AIO, only synchronous reads use it.
  Are there more checks needed ?
- This change should have no effect on existing cancel/complete race
  condition, which I expect is already handled in AIO. If not, then it's
  yet another ffs_aio_cancel bug, and this commit will get larger...
- Should anything be done with the return value of usb_ep_dequeue in
  ffs_aio_cancel_worker ? Failures to cancel because transfer is already
  completed or not known should be harmless, but could there be more
  serious issues lurking ?

[  382.200896] BUG: scheduling while atomic: screen/1808/0x0100
[  382.207124] 4 locks held by screen/1808:
[  382.211266]  #0:  (rcu_callback){}, at: [] 
rcu_process_callbacks+0x260/0x440
[  382.219949]  #1:  (rcu_read_lock_sched){}, at: [] 
percpu_ref_switch_to_atomic_rcu+0xb0/0x130
[  382.230034]  #2:  (&(>ctx_lock)->rlock){}, at: [] 
free_ioctx_users+0x23/0xd0
[  382.230096]  #3:  (&(>eps_lock)->rlock){}, at: [] 
ffs_aio_cancel+0x20/0x60 [usb_f_fs]
[  382.230160] Modules linked in: usb_f_fs libcomposite configfs bnep btsdio 
bluetooth ecdh_generic brcmfmac brcmutil intel_powerclamp coretemp dwc3 
kvm_intel ulpi udc_core kvm irqbypass crc32_pclmul crc32c_intel pcbc dwc3_pci 
aesni_intel aes_i586 crypto_simd cryptd ehci_pci ehci_hcd gpio_keys usbcore 
basincove_gpadc industrialio usb_common
[  382.230407] CPU: 1 PID: 1808 Comm: screen Not tainted 4.14.0-edison+ #117
[  382.230416] Hardware name: Intel Corporation Merrifield/BODEGA BAY, BIOS 542 
2015.01.21:18.19.48
[  382.230425] Call Trace:
[  382.230438]  
[  382.230466]  dump_stack+0x47/0x62
[  382.230498]  __schedule_bug+0x61/0x80
[  382.230522]  __schedule+0x43/0x7a0
[  382.230587]  schedule+0x5f/0x70
[  382.230625]  dwc3_gadget_ep_dequeue+0x14c/0x270 [dwc3]
[  382.230669]  ? do_wait_intr_irq+0x70/0x70
[  382.230724]  usb_ep_dequeue+0x19/0x90 [udc_core]
[  382.230770]  ffs_aio_cancel+0x37/0x60 [usb_f_fs]
[  382.230798]  kiocb_cancel+0x31/0x40
[  382.230822]  free_ioctx_users+0x4d/0xd0
[  382.230858]  percpu_ref_switch_to_atomic_rcu+0x10a/0x130
[  382.230881]  ? percpu_ref_exit+0x40/0x40
[  382.230904]  rcu_process_callbacks+0x2b3/0x440
[  382.230965]  __do_softirq+0xf8/0x26b
[  382.231011]  ? __softirqentry_text_start+0x8/0x8
[  382.231033]  do_softirq_own_stack+0x22/0x30
[  382.231042]  
[  382.231071]  irq_exit+0x45/0xc0
[  382.231089]  smp_apic_timer_interrupt+0x13c/0x150
[  382.231118]  apic_timer_interrupt+0x35/0x3c
[  382.231132] EIP: __copy_user_ll+0xe2/0xf0
[  382.231142] EFLAGS: 00210293 CPU: 1
[  382.231154] EAX: bfd4508c EBX: 0004 ECX: 0003 EDX: f3d8fe50
[  382.231165] ESI: f3d8fe51 EDI: bfd4508d EBP: f3d8fe14 ESP: f3d8fe08
[  382.231176]  DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[  382.231265]  core_sys_select+0x25f/0x320
[  382.231346]  ? __wake_up_common_lock+0x62/0x80
[  382.231399]  ? tty_ldisc_deref+0x13/0x20
[  382.231438]  ? ldsem_up_read+0x1b/0x40
[  382.231459]  ? tty_ldisc_deref+0x13/0x20
[  382.231479]  ? tty_write+0x29f/0x2e0
[  382.231514]  ? n_tty_ioctl+0xe0/0xe0
[  382.231541]  ? tty_write_unlock+0x30/0x30
[  382.231566]  ? __vfs_write+0x22/0x110
[  382.231604]  ? security_file_permission+0x2f/0xd0
[  382.231635]  ? rw_verify_area+0xac/0x120
[  382.231677]  ? vfs_write+0x103/0x180
[  382.231711]  SyS_select+0x87/0xc0
[  382.231739]  ? SyS_write+0x42/0x90
[  382.231781]  do_fast_syscall_32+0xd6/0x1a0
[  382.231836]  entry_SYSENTER_32+0x47/0x71
[  382.231848] EIP: 0xb7f75b05
[  382.231857] EFLAGS: 0246 CPU: 1
[  382.231868] EAX: ffda EBX: 0400 ECX: bfd4508c EDX: bfd4510c
[  382.231878] ESI:  EDI:  EBP:  ESP: bfd45020
[  382.231889]  DS: 007b ES: 007b FS:  GS: 0033 SS: 007b
[  382.232281] softirq: huh, entered 

Re: [PATCH] usb: core: add support for USB_REQ_SET_ISOCH_DELAY

[Greg Kroah-Hartman]

On Tue, Nov 14, 2017 at 12:24:04PM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Felipe Balbi  writes:
> > Greg Kroah-Hartman  writes:
> >> On Thu, Nov 09, 2017 at 03:41:54PM +0200, Felipe Balbi wrote:
> >>> USB SS and SSP hubs provide wHubDelay values on their hub descriptor
> >>> which we should inform the USB Device about.
> >>> 
> >>> The USB Specification 3.0 explains, on section 9.4.11, how to
> >>> calculate the value and how to issue the request. Note that a
> >>> USB_REQ_SET_ISOCH_DELAY is valid on all device states (Default,
> >>> Address, Configured), we just *chose* to issue it from Address state
> >>> right after successfully fetching the USB Device Descriptor.
> >>
> >> I missed that this was in the 3.0 spec, I had heard about it a long time
> >> ago.  Do we have any devices out there that actually set this value?  Or
> >> on the other hand, can anyone use it yet (I know audio wanted it...)
> >
> > Well, USB 3.0 devices are required to accept this request, so my comment
> > that it's "optional", isn't entirely correct. It's part of Chapter9
> > testing on USBCV. Those failing this test, wouldn't get a USB
> > sticker. DWC3, for example, receives the number, but can't do much about
> > it so far. We could, eventually, pass it to gadget drivers and let them
> > use the value to figure out how many requests they should queue ahead of
> > time for isoc endpoints.
> >
> >>> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> >>> index 371a07d874a3..c1db751163d5 100644
> >>> --- a/drivers/usb/core/message.c
> >>> +++ b/drivers/usb/core/message.c
> >>> @@ -915,6 +915,30 @@ int usb_get_device_descriptor(struct usb_device 
> >>> *dev, unsigned int size)
> >>>   return ret;
> >>>  }
> >>>  
> >>> +/*
> >>> + * usb_set_isoch_delay - informs the device of the packet transmit delay
> >>> + * @dev: the device whose delay is to be informed
> >>> + * Context: !in_interrupt()
> >>> + *
> >>> + * Since this is an optional request, we don't bother if it fails.
> >>
> >> But we should return an error, right?  Are devices that don't expect
> >
> > Right, I'm not sure :-) Do we care if we get a stall on this one?
> >
> >> this request going to have problems?
> >
> > No idea. I assume we won't. I can add the error and handle it from the
> > calling site.
> 
> FYI, here's a version with the error checks. If you prefer this version,
> I can put it to Intel's testing farm for a while and see if any problems
> are caught, then resend it once merge window closes.
> 
> Let me know

Yes, that looks better.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] usb: dwc2: host: Don't retry NAKed transactions right away

[Doug Anderson]

Hi,

On Tue, Nov 28, 2017 at 6:37 AM, Stefan Wahren  wrote:
> Hi Doug,
>
> Doug Anderson  hat am 30. Oktober 2017 um 18:14
> geschrieben:
>
> Hi,
>
> On Mon, Oct 30, 2017 at 1:32 AM, Felipe Balbi  wrote:
>>
>
> Hi,
>
> Doug Anderson  writes:
>
> Hi,
>
> On Sat, Oct 28, 2017 at 8:51 AM, Stefan Wahren 
> wrote:
>
> Hi Doug,
>
> [add Felipe since this should go through his tree]
>
> Ah. Sorry Felipe! I know you've landed some dwc2 stuff in the past
>
> No problems :-)
>
> but for some reason get_maintainer didn't ID you so I thought maybe
> you weren't doing it anymore. Please let me know if you'd like me to
> send this to you again with collected Reviewed-by and Tested-by tags.
>
> Yeah, please resend with all tags collected, however let's wait for a
> week or so and give other people time to catch up. I already sent my
> pull request to Greg, this would, anyway, go into the -rc cycle.
>
> Doh! I just re-read this one more time (after sending v3) and
> realized I had read it incorrectly. I read it as "please send the
> patch with the tags and I'll wait a week before landing", but you
> actually said "please wait a week before re-sending". Sorry for the
> noise. In the very least, you should be on the "To" line now so if
> anyone else has any extra tags it should be very easy for you to see
> them.
>
> Right that there's no massive urgency. It's been broken forever.
>
> since 4.15rc1 is out, how are your plans for the next Patch version?

Yeah, I was going to wait a week or so before pestering Felipe since
some people are still in Thanksgiving holiday mode and rc1 _just_ came
out...  My understanding is that the existing v3 patch is OK as-is and
I wasn't planning to send another one.  patchwork.kernel.org IDs for
the most recent version:

10032939 New  [v3,1/2] usb: dwc2: host: Don't retry NAKed
transactions right away
10032935 New  [v3,2/2] usb: dwc2: host: Convert hcd_queue to timer_setup

-Doug
--
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] PM / runtime: Drop children check from __pm_runtime_set_status()

[Rafael J. Wysocki]

On Tue, Nov 28, 2017 at 11:58 AM, Geert Uytterhoeven
 wrote:
> Hi Rafael, Shimoda-san,
>
> On Sun, Nov 12, 2017 at 1:27 AM, Rafael J. Wysocki  wrote:
>> From: Rafael J. Wysocki 
>>
>> The check for "active" children in __pm_runtime_set_status(), when
>> trying to set the parent device status to "suspended", doesn't
>> really make sense, because in fact it is not invalid to set the
>> status of a device with runtime PM disabled to "suspended" in any
>> case.  It is invalid to enable runtime PM for a device with its
>> status set to "suspended" while its child_count reference counter
>> is nonzero, but the check in __pm_runtime_set_status() doesn't
>> really cover that situation.
>>
>> For this reason, drop the children check from __pm_runtime_set_status()
>> and add a check against child_count reference counters of "suspended"
>> devices to pm_runtime_enable().
>>
>> Signed-off-by: Rafael J. Wysocki 
>> ---
>>  drivers/base/power/runtime.c |   30 ++
>>  1 file changed, 10 insertions(+), 20 deletions(-)
>>
>> Index: linux-pm/drivers/base/power/runtime.c
>> ===
>> --- linux-pm.orig/drivers/base/power/runtime.c
>> +++ linux-pm/drivers/base/power/runtime.c
>> @@ -1101,29 +1101,13 @@ int __pm_runtime_set_status(struct devic
>> goto out;
>> }
>>
>> -   if (dev->power.runtime_status == status)
>> +   if (dev->power.runtime_status == status || !parent)
>> goto out_set;
>>
>> if (status == RPM_SUSPENDED) {
>> -   /*
>> -* It is invalid to suspend a device with an active child,
>> -* unless it has been set to ignore its children.
>> -*/
>> -   if (!dev->power.ignore_children &&
>> -   atomic_read(>power.child_count)) {
>> -   dev_err(dev, "runtime PM trying to suspend device 
>> but active child\n");
>
> JFTR, this triggered before during system resume on e.g. Salvator-XS with
> R-Car H3:
>
> ohci-platform ee08.usb: runtime PM trying to suspend device
> but active child
> phy_rcar_gen3_usb2 ee080200.usb-phy: runtime PM trying to suspend
> device but active child
> ohci-platform ee0c.usb: runtime PM trying to suspend device
> but active child
> ohci-platform ee0a.usb: runtime PM trying to suspend device
> but active child
> phy_rcar_gen3_usb2 ee0c0200.usb-phy: runtime PM trying to suspend
> device but active child
> phy_rcar_gen3_usb2 ee0a0200.usb-phy: runtime PM trying to suspend
> device but active child
>
> so this was an existing issue with USB before.
>
>> -   error = -EBUSY;
>> -   goto out;
>> -   }
>> -
>> -   if (parent) {
>> -   atomic_add_unless(>power.child_count, -1, 0);
>> -   notify_parent = !parent->power.ignore_children;
>> -   }
>> -   goto out_set;
>> -   }
>> -
>> -   if (parent) {
>> +   atomic_add_unless(>power.child_count, -1, 0);
>> +   notify_parent = !parent->power.ignore_children;
>> +   } else {
>> spin_lock_nested(>power.lock, SINGLE_DEPTH_NESTING);
>>
>> /*
>> @@ -1307,6 +1291,12 @@ void pm_runtime_enable(struct device *de
>> else
>> dev_warn(dev, "Unbalanced %s!\n", __func__);
>>
>> +   WARN(dev->power.runtime_status == RPM_SUSPENDED &&
>> +!dev->power.ignore_children &&
>> +atomic_read(>power.child_count) > 0,
>> +"Enabling runtime PM for inactive device (%s) with active 
>> children\n",
>> +dev_name(dev));
>
> And now it became a bit more noisy:

Well, it's all existing issues, although the WARN() doesn't provide
additional information in this particular case.

I'm considering changing it to print a message without a stack trace.

Thanks,
Rafael
--
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: EHCI and MUSB do not discover devices without CONFIG_PM

[Ladislav Michl]

On Tue, Nov 28, 2017 at 06:11:31AM -0800, Tony Lindgren wrote:
> * Ladislav Michl  [171128 09:42]:
> > On Tue, Nov 28, 2017 at 10:30:54AM +0100, Greg KH wrote:
> > > bit-banging an ir decoder, ugh, you are in for a world of hurt.  Can't
> > > you put a chip on the device that does this for you in hardware?
> > 
> > OMAP has DM timer which can be externally trigered on edge. Perfect for
> > that purpose. But I cannot pinmux its input as hw designers did poor job.
> > And there are thousands of devices deployed.
> > 
> > So it is about a lot of soldering or providing software solution.
> > 
> > > Anyway, good luck!
> > 
> > A little pointer would increase "luck" by several order of magnitudes.
> 
> Hmm did you already try limiting cpuidle to C1 only in /sys?

I have CONFIG_CPU_IDLE=n, which should be the same. Is that right?

> From what I recall you just set the latency to less than C2
> has. The cpuidle latencies are in struct omap3_idle_statedata
> omap3_idle_data[].

I disabled cpuidle and frequency scaling completely. The only thing that
make jitter difference is CONFIG_PM (see original thread).

There's lot of pm_wkup interrupts with CONFIG_PM - order of magnitude
more than gp_timer.

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

usb: gadget: ffs: Make sparse happier

[Vincent Pelletier]

Silences the following warnings:
drivers/usb/gadget/function/f_fs.c:1253:37: warning: incorrect type in argument 
1 (different address spaces)
drivers/usb/gadget/function/f_fs.c:1253:37:expected void [noderef] 
*to
drivers/usb/gadget/function/f_fs.c:1253:37:got void *
drivers/usb/gadget/function/f_fs.c:2322:23: warning: cast to restricted __le32
drivers/usb/gadget/function/f_fs.c:2876:38: warning: cast to restricted __le32
drivers/usb/gadget/function/f_fs.c:272:12: warning: context imbalance in 
'__ffs_ep0_queue_wait' - unexpected unlock
drivers/usb/gadget/function/f_fs.c:450:17: warning: context imbalance in 
'ffs_ep0_write' - different lock contexts for basic block
drivers/usb/gadget/function/f_fs.c:490:24: warning: context imbalance in 
'__ffs_ep0_read_events' - unexpected unlock
drivers/usb/gadget/function/f_fs.c:496:16: warning: context imbalance in 
'ffs_ep0_read' - different lock contexts for basic block

Also, add an "unlocks spinlock" comment for consistency with existing ones.
No behaviour change is intended.

Signed-off-by: Vincent Pelletier 
---
 drivers/usb/gadget/function/f_fs.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index fc617515a2ec..c56f7afb841a 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -270,6 +270,7 @@ static void ffs_ep0_complete(struct usb_ep *ep, struct 
usb_request *req)
 }
 
 static int __ffs_ep0_queue_wait(struct ffs_data *ffs, char *data, size_t len)
+   __releases(>ev.waitq.lock)
 {
struct usb_request *req = ffs->ep0req;
int ret;
@@ -462,6 +463,7 @@ static ssize_t ffs_ep0_write(struct file *file, const char 
__user *buf,
 /* Called with ffs->ev.waitq.lock and ffs->mutex held, both released on exit. 
*/
 static ssize_t __ffs_ep0_read_events(struct ffs_data *ffs, char __user *buf,
 size_t n)
+   __releases(>ev.waitq.lock)
 {
/*
 * n cannot be bigger than ffs->ev.count, which cannot be bigger than
@@ -547,6 +549,7 @@ static ssize_t ffs_ep0_read(struct file *file, char __user 
*buf,
break;
}
 
+   /* unlocks spinlock */
return __ffs_ep0_read_events(ffs, buf,
 min(n, (size_t)ffs->ev.count));
 
@@ -1250,7 +1253,7 @@ static long ffs_epfile_ioctl(struct file *file, unsigned 
code,
desc = epfile->ep->descs[desc_idx];
 
spin_unlock_irq(>ffs->eps_lock);
-   ret = copy_to_user((void *)value, desc, desc->bLength);
+   ret = copy_to_user((void __user *)value, desc, desc->bLength);
if (ret)
ret = -EFAULT;
return ret;
@@ -2319,7 +2322,7 @@ static int __ffs_data_do_os_desc(enum ffs_os_desc_type 
type,
  length, pnl, type);
return -EINVAL;
}
-   pdl = le32_to_cpu(*(u32 *)((u8 *)data + 10 + pnl));
+   pdl = le32_to_cpu(*(__le32 *)((u8 *)data + 10 + pnl));
if (length != 14 + pnl + pdl) {
pr_vdebug("invalid os descriptor length: %d pnl:%d 
pdl:%d (descriptor %d)\n",
  length, pnl, pdl, type);
@@ -2873,7 +2876,7 @@ static int __ffs_func_bind_do_os_desc(enum 
ffs_os_desc_type type,
 
ext_prop->type = le32_to_cpu(desc->dwPropertyDataType);
ext_prop->name_len = le16_to_cpu(desc->wPropertyNameLength);
-   ext_prop->data_len = le32_to_cpu(*(u32 *)
+   ext_prop->data_len = le32_to_cpu(*(__le32 *)
usb_ext_prop_data_len_ptr(data, ext_prop->name_len));
length = ext_prop->name_len + ext_prop->data_len + 14;
 
-- 
2.15.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: EHCI and MUSB do not discover devices without CONFIG_PM

[Tony Lindgren]

* Ladislav Michl  [171128 14:31]:
> On Tue, Nov 28, 2017 at 06:11:31AM -0800, Tony Lindgren wrote:
> > * Ladislav Michl  [171128 09:42]:
> > > On Tue, Nov 28, 2017 at 10:30:54AM +0100, Greg KH wrote:
> > > > bit-banging an ir decoder, ugh, you are in for a world of hurt.  Can't
> > > > you put a chip on the device that does this for you in hardware?
> > > 
> > > OMAP has DM timer which can be externally trigered on edge. Perfect for
> > > that purpose. But I cannot pinmux its input as hw designers did poor job.
> > > And there are thousands of devices deployed.
> > > 
> > > So it is about a lot of soldering or providing software solution.
> > > 
> > > > Anyway, good luck!
> > > 
> > > A little pointer would increase "luck" by several order of magnitudes.
> > 
> > Hmm did you already try limiting cpuidle to C1 only in /sys?
> 
> I have CONFIG_CPU_IDLE=n, which should be the same. Is that right?

You will then use the omap3_pm_idle() that does not know much
anything about latencies.

> > From what I recall you just set the latency to less than C2
> > has. The cpuidle latencies are in struct omap3_idle_statedata
> > omap3_idle_data[].
> 
> I disabled cpuidle and frequency scaling completely. The only thing that
> make jitter difference is CONFIG_PM (see original thread).

Right as then omap3_pm_idle() is disabled and only WFI is done :)

Limiting things to C1 with cpuidle is probably what you need
for having USB also working.

> There's lot of pm_wkup interrupts with CONFIG_PM - order of magnitude
> more than gp_timer.

Hmm yeah that is true. And idling and waking up devices also takes
some time.

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

Re: EHCI and MUSB do not discover devices without CONFIG_PM

[Ladislav Michl]

On Tue, Nov 28, 2017 at 10:00:32AM -0500, Alan Stern wrote:
> On Tue, 28 Nov 2017, Greg KH wrote:
> 
> > On Mon, Nov 27, 2017 at 11:08:33PM +0100, Ladislav Michl wrote:
> > > Hi there,
> > > 
> > > USB hosts do not discover any connected device on OMAP3 based board
> > > with CONFIG_PM=n. Just enabling this option is enough to restore working
> > > behaviour. Nothing unusual in log. Tested 4.14.2 and 4.15-rc1. I know
> > > a lot of stuff depends on CONFIG_PM, but is this expected behaviour?
> > > Neither EHCI nor MUSB is working without CONFIG_PM.
> > 
> > What bus type is your controllers on?  PCI?  platform?  Something else?
> > 
> > And yes, perhaps this is to be expected, why would you not want
> > CONFIG_PM to be enabled?  :)
> 
> Well, no, it's not expected.  Systems are supposed to work correctly 
> with CONFIG_PM=n -- just not at an optimum power level.

Here's relevant part of bootlog:
[6.466369] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[6.710723] ehci-omap: OMAP-EHCI Host Controller driver
[7.027862] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
[7.336120] ohci-platform: OHCI generic platform driver
[7.336791] ohci-platform 48064400.ohci: Generic Platform OHCI controller
[7.336883] ohci-platform 48064400.ohci: new USB bus registered, assigned 
bus number 1
[7.357177] ohci-platform 48064400.ohci: irq 92, io mem 0x48064400
[7.402191] omap-mailbox 48094000.mailbox: omap mailbox rev 0x40
[7.402343] omap-mailbox: probe of 48094000.mailbox failed with error -38
[7.608337] usb usb1: New USB device found, idVendor=1d6b, idProduct=0001
[7.608367] usb usb1: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[7.608367] usb usb1: Product: Generic Platform OHCI controller
[7.608398] usb usb1: Manufacturer: Linux 4.15.0-rc1 ohci_hcd
[7.608398] usb usb1: SerialNumber: 48064400.ohci
[7.623779] hub 1-0:1.0: USB hub found
[7.627380] hub 1-0:1.0: 3 ports detected
[7.831878] omap-sham 480c3000.sham: hw accel on OMAP rev 0.9
[8.218322] twl4030_usb 4807.i2c:twl@48:twl4030-usb: Initialized TWL4030 
USB module
[8.224212] musb-hdrc musb-hdrc.0.auto: MUSB HDRC host driver
[8.224304] musb-hdrc musb-hdrc.0.auto: new USB bus registered, assigned bus 
number 2
[8.242736] usb usb2: New USB device found, idVendor=1d6b, idProduct=0002
[8.242767] usb usb2: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[8.242767] usb usb2: Product: MUSB HDRC host driver
[8.242767] usb usb2: Manufacturer: Linux 4.15.0-rc1 musb-hcd
[8.242797] usb usb2: SerialNumber: musb-hdrc.0.auto
[8.246093] hub 2-0:1.0: USB hub found
[8.246673] hub 2-0:1.0: 1 port detected
[8.461669] omap-aes 480c5000.aes: OMAP AES hw accel rev: 2.6
[8.463500] omap-aes 480c5000.aes: will run requests pump with realtime 
priority
[8.937469] input: beeper as /devices/platform/beeper/input/input1
[9.188262] ehci-omap 48064800.ehci: EHCI Host Controller
[9.188323] ehci-omap 48064800.ehci: new USB bus registered, assigned bus 
number 3
[9.188629] ehci-omap 48064800.ehci: irq 93, io mem 0x48064800
[9.217407] ehci-omap 48064800.ehci: USB 2.0 started, EHCI 1.00
[9.217803] usb usb3: New USB device found, idVendor=1d6b, idProduct=0002
[9.217803] usb usb3: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[9.217834] usb usb3: Product: EHCI Host Controller
[9.217834] usb usb3: Manufacturer: Linux 4.15.0-rc1 ehci_hcd
[9.217834] usb usb3: SerialNumber: 48064800.ehci
[9.218841] hub 3-0:1.0: USB hub found
[9.218963] hub 3-0:1.0: 3 ports detected

> EHCI certainly should work without CONFIG_PM.  When a device is plugged 
> in to the EHCI controller, what shows up in 
> /sys/kernel/debug/usb/ehci/*/registers (fill in the '*' with the 
> correct bus ID)?

$ ls /sys/kernel/debug/usb
devices  ohci

So no ehci.

$ cat /sys/kernel/debug/usb/ohci/48064400.ohci/registers 
bus platform, device 48064400.ohci
Generic Platform OHCI controller
ohci_hcd
OHCI 1.0, NO legacy support registers, rh state running
control 0x083 HCFS=operational CBSR=3
cmdstatus 0x0 SOC=0
intrstatus 0x0024 FNO SF
intrenable 0x805a MIE RHSC UE RD WDH
hcca frame 0xb9ae
fmintvl 0xa7782edf FIT FSMPS=0xa778 FI=0x2edf
fmremaining 0x8857 FRT FR=0x0857
periodicstart 0x2a2f
lsthresh 0x0628
hub poll timer off
roothub.a 0a000203 POTPGT=10 NPS NDP=3(3)
roothub.b  PPCM= DR=
roothub.status 8000 DRWE
roothub.portstatus [0] 0x0100 PPS
roothub.portstatus [1] 0x0100 PPS
roothub.portstatus [2] 0x0100 PPS

> For that matter, what does that file contain before any devices are 
> plugged in?

For ohci it is the same. Any hint why there's no ehco file?

> Alan Stern

Thank you,
ladis
--
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

4.14 dwc3 gadget mode WARNING on unplug

[Vincent Pelletier]

Hello,

Now that "unplugging" the userland process does not kill the kernel, I
tried unplugging the cable in the same starting conditions (two
submitted AIO read transfers, host having enabled the device, but no
data transfer actually happening).

And I got below WARNING.
Sadly, the error goes away when enabling traces (110 being ETIMEDOUT,
this could explain...) so I resorted to an ad-hoc printk (first line
below).

Platform CPU is slow: dual-core intel atom 500MHz. But reading the
code, a slow CPU would seem to be an advantage, as the timeout is
counted in loop iterations (busy loop on dwc3_readl), and apparently not
as wall-clock time.

[   81.326172] dwc3_send_gadget_ep_cmd -> -110
[   81.326273] [ cut here ]
[   81.335341] WARNING: CPU: 0 PID: 1874 at drivers/usb/dwc3/gadget.c:2627 
dwc3_stop_active_transfer.constprop.23+0x69/0xc0 [dwc3]
[   81.347094] Modules linked in: usb_f_fs libcomposite configfs bnep btsdio 
bluetooth ecdh_generic brcmfmac brcmutil dwc3 intel_powerclamp coretemp ulpi 
kvm_intel udc_core kvm irqbypass crc32_pclmul crc32c_intel pcbc dwc3_pci 
aesni_intel aes_i586 crypto_simd cryptd ehci_pci ehci_hcd basincove_gpadc 
industrialio gpio_keys usbcore usb_common
[   81.378142] CPU: 0 PID: 1874 Comm: irq/34-dwc3 Not tainted 4.14.0-edison+ 
#119
[   81.385545] Hardware name: Intel Corporation Merrifield/BODEGA BAY, BIOS 542 
2015.01.21:18.19.48
[   81.394548] task: f5b1be00 task.stack: f420a000
[   81.399219] EIP: dwc3_stop_active_transfer.constprop.23+0x69/0xc0 [dwc3]
[   81.406086] EFLAGS: 00010086 CPU: 0
[   81.409672] EAX: 001f EBX: f5729800 ECX: c132a2a2 EDX: 
[   81.416096] ESI: f4054014 EDI: f41cf400 EBP: f420be10 ESP: f420bdf4
[   81.422521]  DS: 007b ES: 007b FS: 00d8 GS:  SS: 0068
[   81.428061] CR0: 80050033 CR2: b7a3f000 CR3: 01d94000 CR4: 001006d0
[   81.434483] Call Trace:
[   81.437063]  __dwc3_gadget_ep_disable+0xa3/0x2b0 [dwc3]
[   81.442438]  ? _raw_spin_lock_irqsave+0x32/0x40
[   81.447135]  dwc3_gadget_ep_disable+0xbf/0xe0 [dwc3]
[   81.452269]  usb_ep_disable+0x1c/0xd0 [udc_core]
[   81.457048]  ffs_func_eps_disable.isra.15+0x3b/0x90 [usb_f_fs]
[   81.463070]  ffs_func_set_alt+0x7d/0x310 [usb_f_fs]
[   81.468132]  ffs_func_disable+0x14/0x20 [usb_f_fs]
[   81.473075]  reset_config+0x5b/0x90 [libcomposite]
[   81.478023]  composite_disconnect+0x2b/0x50 [libcomposite]
[   81.483685]  dwc3_disconnect_gadget+0x39/0x50 [dwc3]
[   81.488808]  dwc3_gadget_disconnect_interrupt+0x21b/0x250 [dwc3]
[   81.495014]  dwc3_thread_interrupt+0x2a8/0xf70 [dwc3]
[   81.500219]  ? __schedule+0x78c/0x7e0
[   81.504027]  irq_thread_fn+0x18/0x30
[   81.507715]  ? irq_thread+0xb7/0x180
[   81.511400]  irq_thread+0x111/0x180
[   81.515000]  ? irq_finalize_oneshot+0xe0/0xe0
[   81.519490]  ? wake_threads_waitq+0x30/0x30
[   81.523806]  kthread+0x107/0x110
[   81.527131]  ? disable_percpu_irq+0x50/0x50
[   81.531439]  ? kthread_stop+0x150/0x150
[   81.535397]  ret_from_fork+0x19/0x24
[   81.539136] Code: 89 d8 c7 45 ec 00 00 00 00 c7 45 f0 00 00 00 00 c7 45 f4 
00 00 00 00 e8 56 ef ff ff 85 c0 74 12 50 68 b9 1c 14 f8 e8 64 0f f7 c8 <0f> ff 
58 5a 8d 76 00 8b 83 98 00 00 00 c6 83 a0 00 00 00 00 83
[   81.559295] ---[ end trace f3133eec81a473b8 ]---
[   81.591665] dwc3 dwc3.1.auto: request f5abd960 was not queued to ep2out
[   81.598618] ffs_aio_cancel_worker: -22

Regards,
-- 
Vincent Pelletier
--
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

hid report: padding differs from report descriptor?

[Florian Dollinger]

I have a device that has the following hid report descriptor:

-

...

Collection (Logical) A1 02

Usage Page (Physical Input Device) 05

Usage (DC Enable Actuators) 09 97
Logical Minimum (0) 15 00
Logical Maximum (1) 25 01
Report Size (4) 75 04
Report Count (1) 95 01
Output (Data,Var,Abs,NWrp,Lin,Pref,NNul,NVol,Bit) 91 02

Logical Minimum (0) 15 00
Logical Maximum (0) 25 00
Output (Cnst,Var,Abs,NWrp,Lin,Pref,NNul,NVol,Bit) 91 03

Usage (Magnitude) 09 70
Logical Minimum (0) 15 00
Logical Maximum (100) 25 64
Report Size (8) 75 08
Report Count (4) 95 04
Output (Data,Var,Abs,NWrp,Lin,Pref,NNul,NVol,Bit) 91 02

...

End Collection C0

-

I would therefore expect that the device wants data in the following manner:

8 Bits: Report ID, 4 Bits: DC Enable Actuators, 4 Bits: Padding, 32 
Bits: Magnitude



Hence, my driver code in linux would look something like that:

-

...
static const u8 buf[] =  {0x03, 0b0001, 0x00, 0x00, 0x60, 0x60, 10, 
0x00, 10};

hid_hw_output_report(hid, buf, 9);
...

-

Unfortunately that's not what the device expects! In fact it does only 
react if the data is structured like:


8 Bits: Report ID, 4 Bits: Padding, 4 Bits: DC Enable Actuators, 32 
Bits: Magnitude


Which is:

-

...
static const u8 buf[] =  {0x03, 0b0001, 0x00, 0x00, 0x60, 0x60, 10, 
0x00, 10};

hid_hw_output_report(hid, buf, 9);
...

-

That means the padding comes before the DC Enable Actuators field, not 
after.


Does anyone understand why these two fields are switched? Is there 
something i am missing?


Thank you in advance!
--
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: EHCI and MUSB do not discover devices without CONFIG_PM

[Tony Lindgren]

* Ladislav Michl  [171128 09:42]:
> On Tue, Nov 28, 2017 at 10:30:54AM +0100, Greg KH wrote:
> > bit-banging an ir decoder, ugh, you are in for a world of hurt.  Can't
> > you put a chip on the device that does this for you in hardware?
> 
> OMAP has DM timer which can be externally trigered on edge. Perfect for
> that purpose. But I cannot pinmux its input as hw designers did poor job.
> And there are thousands of devices deployed.
> 
> So it is about a lot of soldering or providing software solution.
> 
> > Anyway, good luck!
> 
> A little pointer would increase "luck" by several order of magnitudes.

Hmm did you already try limiting cpuidle to C1 only in /sys?

>From what I recall you just set the latency to less than C2
has. The cpuidle latencies are in struct omap3_idle_statedata
omap3_idle_data[].

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

Re: [PATCH] USB: atm: use setup_timer instead of init_timer

[Greg Kroah-Hartman]

On Fri, Nov 24, 2017 at 01:31:54PM +, Colin King wrote:
> From: Colin Ian King 
> 
> Use setup_timer function instead of initializing timer with the
> function and data fields.
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/usb/atm/usbatm.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

This doesn't apply to 4.15-rc1 at all :(
--
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: EHCI and MUSB do not discover devices without CONFIG_PM

[Alan Stern]

On Tue, 28 Nov 2017, Greg KH wrote:

> On Mon, Nov 27, 2017 at 11:08:33PM +0100, Ladislav Michl wrote:
> > Hi there,
> > 
> > USB hosts do not discover any connected device on OMAP3 based board
> > with CONFIG_PM=n. Just enabling this option is enough to restore working
> > behaviour. Nothing unusual in log. Tested 4.14.2 and 4.15-rc1. I know
> > a lot of stuff depends on CONFIG_PM, but is this expected behaviour?
> > Neither EHCI nor MUSB is working without CONFIG_PM.
> 
> What bus type is your controllers on?  PCI?  platform?  Something else?
> 
> And yes, perhaps this is to be expected, why would you not want
> CONFIG_PM to be enabled?  :)

Well, no, it's not expected.  Systems are supposed to work correctly 
with CONFIG_PM=n -- just not at an optimum power level.

EHCI certainly should work without CONFIG_PM.  When a device is plugged 
in to the EHCI controller, what shows up in 
/sys/kernel/debug/usb/ehci/*/registers (fill in the '*' with the 
correct bus ID)?

For that matter, what does that file contain before any devices are 
plugged in?

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: very slow USB security token access after switch from 2.6.13 to 4.4.52

[Greg KH]

On Sun, Nov 26, 2017 at 07:05:12PM +0100, Nikola Ciprich wrote:
> Hello dear linux USB users and developers,
> 
> I've got a bit desperate question, but maybe somebody will have
> some idea on what to try..
> 
> we're migrating lots (thousands) of users from very old opensuse running
> 2.6.13 kernel to new centos based system running 4.4.52 kernel.

That's a huge jump :)

> There are security tokens in use on desktops - Ikey 4000. They work,
> but on new system initialisation of token and reading certificates is taking
> very long, slowing firefox (and other pkcs11 users) start by ~20seconds. Since
> users are running those token using apps very often, this is causing lots of 
> hassle..
> 
> The problem is, that token support is proprietary, I don't have any sources, 
> just
> those arcane opensc + openct binaries built for opensuse I'm forced to on new 
> system
> (therefore I'm also using 32bit firefox). Since the switch to new tokens with 
> better
> support is planned on next year, I'd really love to resolve this issue.

So is it using libusb to talk to the device, or a kernel module?

If libusb, you can use usbmon to see the data being sent to/from the
device, to see where the stalls are, and hopefully compare the two
systems to see what the difference is.

> I've tried tracing all related binaries, and I think the problem is in 
> ifdhandler
> slow access to device..
> 
> sample ifdhandler debug output is here:
> 
> Debug: ifdhandler_process: ifdhandler_process(cmd=CT_CMD_TRANSACT, unit=0)
> Debug: ifd_protocol_transceive: cmd:  00 b0 04 ec dd
> Debug: t1_xcv: sending  00 00 05 00 b0 04 ec dd 80
> Debug: ifd_usb_control: usb req type=x41 req=x17 val=x ind=x0005 len=5
> Debug: ifd_usb_control: send  b0 04 ec dd 80
> Debug: ifd_usb_control: usb req type=xc1 req=x01 val=x ind=x len=255
> Debug: ifd_usb_control: recv  00 c3 01 26 e4 04 ec dd 80 02 10 00 34 8b 9f 8e 
> 07 f4 22 de bc 60 fb a8 a0 8a a4 0c 11 0f 33 90 00 01 1c ad 80 1b 84 31 2a 40 
> 11 fe 4d 18 23 08 c6 cd 90 b5 44 01 21 24 88 a6 0b 40 94 21 78 78 b0 95 72 19 
> 20 e0 1a a6 07 e3 69 c4 41 4c 00 66 04 a3 01 b2 d0 c1 32 0a 60 05 56 d8 16 d1 
> b4 04 10 05 80 c4 87 8c 61 12 73 5a fa a8 61 b4 28 25 ba 4c 24 92 c9 28 9a 84 
> 49 0b 00 4a 27 94 45 02 81 11 ed 42 a2 90 c0 aa b0 91 59 04 6a 20 a1 2b 40 c3 
> 27 3c 05 a9 4a 2d 05 00 02 87 91 28 10 17 16 ad 00 41 42 92 20 94 18 20 83 cb 
> ae 42 0e 58 52 d1 00 29 41 12 3c 40 00 c3 01 26 e4 04 ec dd 80 02 10 00 34 8b 
> 9f 8e 07 f4 22 de bc 60 fb a8 a0 8a a4 0c 11 0f 33 90 00 01 1c ad 80 1b 84 31 
> 2a 40 11 fe 4d 18 23 08 c6 cd 90 b5 44 01 21 24 88 a6 0b 40 94 21 78 78 b0 95 
> 72 19 20 e0 1a
> Debug: t1_xcv: received  00 c3 01 26 e4
> Debug: t1_transceive: CT sent S-block with wtx=38
> Debug: t1_xcv: sending  00 e3 01 26 c4
> Debug: ifd_usb_control: usb req type=x41 req=x17 val=xe300 ind=x2601 len=1
> Debug: ifd_usb_control: send  c4
> Debug: ifd_usb_control: usb req type=xc1 req=x01 val=x ind=x len=255
> Debug: ifd_usb_control: recv  00 60 20 35 9c b6 98 cd e1 e2 18 08 f0 8b 42 8d 
> c9 b2 3f f7 09 35 29 dc 95 cf 58 f5 91 c1 2d 86 37 2a dc 1a 80 1b 84 31 2a 40 
> 11 fe 4d 18 23 08 c6 cd 90 b5 44 01 21 24 88 a6 0b 40 94 21 78 78 b0 95 72 19 
> 20 e0 1a a6 07 e3 69 c4 41 4c 00 66 04 a3 01 b2 d0 c1 32 0a 60 05 56 d8 16 d1 
> b4 04 10 05 80 c4 87 8c 61 12 73 5a fa a8 61 b4 28 25 ba 4c 24 92 c9 28 9a 84 
> 49 0b 00 4a 27 94 45 02 81 11 ed 42 a2 90 c0 aa b0 91 59 04 6a 20 a1 2b 40 c3 
> 27 3c 05 a9 4a 2d 05 00 02 87 91 28 10 17 16 ad 00 41 42 92 20 94 18 20 83 cb 
> ae 42 0e 58 52 d1 00 29 41 12 3c 40 00 60 20 35 9c b6 98 cd e1 e2 18 08 f0 8b 
> 42 8d c9 b2 3f f7 09 35 29 dc 95 cf 58 f5 91 c1 2d 86 37 2a dc 1a 80 1b 84 31 
> 2a 40 11 fe 4d 18 23 08 c6 cd 90 b5 44 01 21 24 88 a6 0b 40 94 21 78 78 b0 95 
> 72 19 20 e0 1a
> 
> (repeats many times).
> 
> strace of the same process:
> 
> 18:27:24 poll([{fd=3, events=POLLHUP}, {fd=4, events=POLLIN}, {fd=6, 
> events=POLLOUT}], 3, 1000) = 1 ([{fd=6, revents=POLLOUT}])
> 18:27:24 time(NULL) = 1511717244
> 18:27:24 rt_sigaction(SIGPIPE, {SIG_IGN, [], 0}, {SIG_IGN, [], 0}, 8) = 0
> 18:27:24 write(6, 
> "\v\0\0\0\0\0\0\0\0\0)\0E\0\t\355\17\17\17\17\17\17\0\0\1\0\0\0\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\0\0\1\0\220\0",
>  53) = 53
> 18:27:24 rt_sigaction(SIGPIPE, {SIG_IGN, [], 0}, {SIG_IGN, [], 0}, 8) = 0
> 18:27:24 time(NULL) = 1511717244
> 18:27:24 poll([{fd=3, events=POLLHUP}, {fd=4, events=POLLIN}, {fd=6, 
> events=POLLOUT}], 3, 1000) = 1 ([{fd=6, revents=POLLOUT}])
> 18:27:24 time(NULL) = 1511717244
> 18:27:24 rt_sigaction(SIGPIPE, {SIG_IGN, [], 0}, {SIG_IGN, [], 0}, 8) = 0
> 18:27:24 rt_sigaction(SIGPIPE, {SIG_IGN, [], 0}, {SIG_IGN, [], 0}, 8) = 0
> 18:27:24 time(NULL) = 1511717244
> 18:27:24 poll([{fd=3, events=POLLHUP}, {fd=4, events=POLLIN}, {fd=6, 
> events=POLLIN}], 3, 1000) = 1 ([{fd=6, revents=POLLIN}])
> 18:27:24 time(NULL)  

Re: EHCI and MUSB do not discover devices without CONFIG_PM

[Ladislav Michl]

On Tue, Nov 28, 2017 at 01:09:45PM -0500, Alan Stern wrote:
> The EHCI debugging files require CONFIG_DYNAMIC_DEBUG to be enabled.  
> Oddly enough, the OHCI debugging files do not have the same 
> requirement.  I don't know the reason for this difference.

I see, thanks for suggestion.

USB cable unplugged:
$ cat /sys/kernel/debug/usb/ehci/48064800.ehci/registers 
bus platform, device 48064800.ehci
EHCI Host Controller
EHCI 1.00, rh state running
structural params 0x1313
capability params 0x0016
status 
command 0010005 (park)=0 ithresh=1 period=512 RUN
intrenable 37 IAA FATAL PCD ERR INT
uframe 
port:1 status 001000 0  ACK POWER sig=se0
port:2 status 001000 0  ACK POWER sig=se0
port:3 status 001000 0  ACK POWER sig=se0
irq normal 0 err 0 iaa 0 (lost 0)
complete 0 unlink 0

Plugged:
$ cat /sys/kernel/debug/usb/ehci/48064800.ehci/registers 
bus platform, device 48064800.ehci
EHCI Host Controller
EHCI 1.00, rh state running
structural params 0x1313
capability params 0x0016
status 
command 0010005 (park)=0 ithresh=1 period=512 RUN
intrenable 37 IAA FATAL PCD ERR INT
uframe 
port:1 status 001000 0  ACK POWER sig=se0
port:2 status 001000 0  ACK POWER sig=se0
port:3 status 001000 0  ACK POWER sig=se0
irq normal 0 err 0 iaa 0 (lost 0)
complete 0 unlink 0

$ lsusb -t
/:  Bus 03.Port 1: Dev 1, Class=root_hub, Driver=ehci-omap/3p, 480M
/:  Bus 02.Port 1: Dev 1, Class=root_hub, Driver=musb-hdrc/1p, 480M
/:  Bus 01.Port 1: Dev 1, Class=root_hub, Driver=ohci-platform/3p, 12M

ladis
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1] usb: xhci: allow imod-interval to be configurable

[Greg Kroah-Hartman]

On Tue, Nov 28, 2017 at 12:11:46PM -0500, Adam Wallis wrote:
> The xHCI driver currently has the IMOD set to 160, which
> translates to an IMOD interval of 40,000ns (160 * 250)ns
> 
> Commit 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller")
> introduced a QUIRK for the MTK platform to adjust this interval to 20,
> which translates to an IMOD interval of 5,000ns (20 * 250)ns. This is
> due to the fact that the MTK controller IMOD interval is 8 times
> as much as defined in xHCI spec.
> 
> Instead of adding more quirk bits for additional platforms, this patch
> introduces the ability for vendors to set the IMOD_INTERVAL as is
> optimal for their platform. By using device_property_read_u32() on
> "imod-interval", the IMOD INTERVAL can be specified in nano seconds. If
> no interval is specified, the default of 40,000ns (IMOD=160) will be
> used.
> 
> No bounds checking has been implemented due to the fact that a vendor
> may have violated the spec and would need to specify a value outside of
> the max 8,000 IRQs/second limit specified in the xHCI spec.
> 
> Backwards compatibility is maintained for MTK_HOSTS through the quirk
> bit, however, imod_interval should be pushed into device tree at a
> future point and this quirk should be removed from xhci_plat_probe
> 
> Signed-off-by: Adam Wallis 
> ---
>  Documentation/devicetree/bindings/usb/usb-xhci.txt |  1 +
>  drivers/usb/host/xhci-plat.c   | 15 +++
>  drivers/usb/host/xhci.c|  7 ++-
>  drivers/usb/host/xhci.h|  2 ++
>  4 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt 
> b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> index ae6e484..3998459 100644
> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> @@ -29,6 +29,7 @@ Optional properties:
>- usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
>- usb3-lpm-capable: determines if platform is USB3 LPM capable
>- quirk-broken-port-ped: set if the controller has broken port disable 
> mechanism
> +  - imod-interval: IMOD_INTERVAL in nano-seconds. Default is 4
>  
>  Example:
>   usb@f0931000 {
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 09f164f..f7730c8 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -23,6 +23,7 @@
>  #include "xhci-plat.h"
>  #include "xhci-mvebu.h"
>  #include "xhci-rcar.h"
> +#include "xhci-mtk.h"
>  
>  static struct hc_driver __read_mostly xhci_plat_hc_driver;
>  
> @@ -269,6 +270,20 @@ static int xhci_plat_probe(struct platform_device *pdev)
>   if (device_property_read_bool(>dev, "quirk-broken-port-ped"))
>   xhci->quirks |= XHCI_BROKEN_PORT_PED;
>  
> + /* imod interval in nanoseconds */
> + if (device_property_read_u32(sysdev,
> + "imod-interval", >imod_interval))
> + xhci->imod_interval = 4;

So no matter what value you read, you set it to 4?  Or am I reading
this code incorrectly?

There's a good reason putting function calls inside if() is considered a
bad coding style :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: EHCI and MUSB do not discover devices without CONFIG_PM

[Alan Stern]

On Tue, 28 Nov 2017, Ladislav Michl wrote:

> On Tue, Nov 28, 2017 at 01:09:45PM -0500, Alan Stern wrote:
> > The EHCI debugging files require CONFIG_DYNAMIC_DEBUG to be enabled.  
> > Oddly enough, the OHCI debugging files do not have the same 
> > requirement.  I don't know the reason for this difference.
> 
> I see, thanks for suggestion.
> 
> USB cable unplugged:
> $ cat /sys/kernel/debug/usb/ehci/48064800.ehci/registers 
> bus platform, device 48064800.ehci
> EHCI Host Controller
> EHCI 1.00, rh state running
> structural params 0x1313
> capability params 0x0016
> status 
> command 0010005 (park)=0 ithresh=1 period=512 RUN
> intrenable 37 IAA FATAL PCD ERR INT
> uframe 
> port:1 status 001000 0  ACK POWER sig=se0
> port:2 status 001000 0  ACK POWER sig=se0
> port:3 status 001000 0  ACK POWER sig=se0
> irq normal 0 err 0 iaa 0 (lost 0)
> complete 0 unlink 0
> 
> Plugged:
> $ cat /sys/kernel/debug/usb/ehci/48064800.ehci/registers 
> bus platform, device 48064800.ehci
> EHCI Host Controller
> EHCI 1.00, rh state running
> structural params 0x1313
> capability params 0x0016
> status 
> command 0010005 (park)=0 ithresh=1 period=512 RUN
> intrenable 37 IAA FATAL PCD ERR INT
> uframe 
> port:1 status 001000 0  ACK POWER sig=se0
> port:2 status 001000 0  ACK POWER sig=se0
> port:3 status 001000 0  ACK POWER sig=se0
> irq normal 0 err 0 iaa 0 (lost 0)
> complete 0 unlink 0

So no change at all.  In particular, no change to the port statuses.  
This suggests the hardware isn't configured properly; maybe a phy isn't
detecting the new connection.

I don't know anything about the phy drivers, however.  You should bring 
this to the attention of the maintainer for whichever phy is on your 
board.

By the way, the behavior when you plug in a device with CONFIG_PM=n
should be just about the same as the behavior with CONFIG_PM=y when you
plug in a _second_ device.  Have you tried doing that, say with two 
flash drives?  (I'm assuming your board has more than one EHCI port 
available.)

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: EHCI and MUSB do not discover devices without CONFIG_PM

[Ladislav Michl]

On Tue, Nov 28, 2017 at 02:59:32PM -0500, Alan Stern wrote:
> On Tue, 28 Nov 2017, Ladislav Michl wrote:
> 
> > On Tue, Nov 28, 2017 at 01:09:45PM -0500, Alan Stern wrote:
> > > The EHCI debugging files require CONFIG_DYNAMIC_DEBUG to be enabled.  
> > > Oddly enough, the OHCI debugging files do not have the same 
> > > requirement.  I don't know the reason for this difference.
> > 
> > I see, thanks for suggestion.
> > 
> > USB cable unplugged:
> > $ cat /sys/kernel/debug/usb/ehci/48064800.ehci/registers 
> > bus platform, device 48064800.ehci
> > EHCI Host Controller
> > EHCI 1.00, rh state running
> > structural params 0x1313
> > capability params 0x0016
> > status 
> > command 0010005 (park)=0 ithresh=1 period=512 RUN
> > intrenable 37 IAA FATAL PCD ERR INT
> > uframe 
> > port:1 status 001000 0  ACK POWER sig=se0
> > port:2 status 001000 0  ACK POWER sig=se0
> > port:3 status 001000 0  ACK POWER sig=se0
> > irq normal 0 err 0 iaa 0 (lost 0)
> > complete 0 unlink 0
> > 
> > Plugged:
> > $ cat /sys/kernel/debug/usb/ehci/48064800.ehci/registers 
> > bus platform, device 48064800.ehci
> > EHCI Host Controller
> > EHCI 1.00, rh state running
> > structural params 0x1313
> > capability params 0x0016
> > status 
> > command 0010005 (park)=0 ithresh=1 period=512 RUN
> > intrenable 37 IAA FATAL PCD ERR INT
> > uframe 
> > port:1 status 001000 0  ACK POWER sig=se0
> > port:2 status 001000 0  ACK POWER sig=se0
> > port:3 status 001000 0  ACK POWER sig=se0
> > irq normal 0 err 0 iaa 0 (lost 0)
> > complete 0 unlink 0
> 
> So no change at all.  In particular, no change to the port statuses.  
> This suggests the hardware isn't configured properly; maybe a phy isn't
> detecting the new connection.
> 
> I don't know anything about the phy drivers, however.  You should bring 
> this to the attention of the maintainer for whichever phy is on your 
> board.

Thanks for suggestion, there's USB3326 ULPI PHY with nEN_USB_PWR connected
to TWL4030 LEDA output.

> By the way, the behavior when you plug in a device with CONFIG_PM=n
> should be just about the same as the behavior with CONFIG_PM=y when you
> plug in a _second_ device.  Have you tried doing that, say with two 
> flash drives?  (I'm assuming your board has more than one EHCI port 
> available.)

Unfortunately there's only one EHCI port, however your suggestions are
enough to start debugging.

Thank you,
ladis
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1] usb: xhci: allow imod-interval to be configurable

[Adam Wallis]

On 11/28/2017 2:35 PM, Greg Kroah-Hartman wrote:
> On Tue, Nov 28, 2017 at 12:11:46PM -0500, Adam Wallis wrote:
>> The xHCI driver currently has the IMOD set to 160, which
>> translates to an IMOD interval of 40,000ns (160 * 250)ns
>>
[..]
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -23,6 +23,7 @@
>>  #include "xhci-plat.h"
>>  #include "xhci-mvebu.h"
>>  #include "xhci-rcar.h"
>> +#include "xhci-mtk.h"
>>  
>>  static struct hc_driver __read_mostly xhci_plat_hc_driver;
>>  
>> @@ -269,6 +270,20 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>  if (device_property_read_bool(>dev, "quirk-broken-port-ped"))
>>  xhci->quirks |= XHCI_BROKEN_PORT_PED;
>>  
>> +/* imod interval in nanoseconds */
>> +if (device_property_read_u32(sysdev,
>> +"imod-interval", >imod_interval))
>> +xhci->imod_interval = 4;
> 
> So no matter what value you read, you set it to 4?  Or am I reading
> this code incorrectly?

I think you may be reading the code incorrectly. device_property_read_u32()
returns 0 when the property is found and valid...and stored into
xhci->imod_interval. When 0 is returned in this case, the default value of
40,000 is skipped over.

If the property is not found, a number of different errors could be returned,
but any of them will cause the default value to be used.

> There's a good reason putting function calls inside if() is considered a
> bad coding style :)

I do not disagree with you, however, I was trying to maintain style consistency
with the device property reads with the xhci_plat_probe function.

If I break that consistency, a couple of ways I might write this cleaner

1) set xhci->imod_interval to 40,000 before the call to
device_property_read_u32. If the property exists in a firmware node, it will
update the imod_interval value...if it does not exist, it will not update this
value and the default will be used. In this case, I would not even check the
return value. This method is used quite a bit in the kernel.

2) use a if (device_property_present()) and check to see if that property is
even present. If so, call device_property_read_u32() without check return value.
This has the downside of still using a function call within the if() which you
have already indicated is not your preference.

3) add a retval and test off of this instead of using the function directly in
the if()

> thanks,
> 
> greg k-h

Thanks for taking time to review my patch. I really have no strong preference on
how call device_property_read_u32() is tested and I am open to any suggestions.
Adam


-- 
Adam Wallis
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1] USB: storage: Notify the subdrivers that they need to reinitialize the device.

[Mikhail Zaytsev]

This patch adds the device_reinit function into the us_data structure.
The usb-storage driver uses this function for notify the subdrivers that
 they need to reinitialize the device.

Signed-off-by: Mikhail Zaytsev 
---
 drivers/usb/storage/usb.c | 16 
 drivers/usb/storage/usb.h |  8 +---
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index a0c07e05a8f1..60c47b2e877d 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -210,10 +210,10 @@ int usb_stor_reset_resume(struct usb_interface *iface)
/* Report the reset to the SCSI core */
usb_stor_report_bus_reset(us);

-   /*
-* FIXME: Notify the subdrivers that they need to reinitialize
-* the device
-*/
+   if (us->device_reinit) {
+   usb_stor_dbg(us, "-- calling device_reinit()\n");
+   us->device_reinit(us);
+   }
return 0;
 }
 EXPORT_SYMBOL_GPL(usb_stor_reset_resume);
@@ -242,10 +242,10 @@ int usb_stor_post_reset(struct usb_interface *iface)
/* Report the reset to the SCSI core */
usb_stor_report_bus_reset(us);

-   /*
-* FIXME: Notify the subdrivers that they need to reinitialize
-* the device
-*/
+   if (us->device_reinit) {
+   usb_stor_dbg(us, "-- calling device_reinit()\n");
+   us->device_reinit(us);
+   }

mutex_unlock(>dev_mutex);
return 0;
diff --git a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
index 90133e16bec5..c646a0464a51 100644
--- a/drivers/usb/storage/usb.h
+++ b/drivers/usb/storage/usb.h
@@ -77,9 +77,10 @@ struct us_unusual_dev {
 #define US_IOBUF_SIZE  64  /* Size of the DMA-mapped I/O buffer */
 #define US_SENSE_SIZE  18  /* Size of the autosense data buffer */

-typedef int (*trans_cmnd)(struct scsi_cmnd *, struct us_data*);
-typedef int (*trans_reset)(struct us_data*);
-typedef void (*proto_cmnd)(struct scsi_cmnd*, struct us_data*);
+typedef int (*trans_cmnd)(struct scsi_cmnd *, struct us_data *);
+typedef int (*trans_reset)(struct us_data *);
+typedef int (*dev_reinit)(struct us_data *);
+typedef void (*proto_cmnd)(struct scsi_cmnd *, struct us_data *);
 typedef void (*extra_data_destructor)(void *); /* extra data destructor */
 typedef void (*pm_hook)(struct us_data *, int);/* power management 
hook */

@@ -119,6 +120,7 @@ struct us_data {
/* function pointers for this device */
trans_cmnd  transport;   /* transport function */
trans_reset transport_reset; /* transport device reset */
+   dev_reinit  device_reinit;   /* device reinitialize*/
proto_cmnd  proto_handler;   /* protocol handler   */

/* SCSI interfaces */
--
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: xhci_hcd 0000:00:14.0: WARN Event TRB for slot 1 ep 2 with no TDs queued?

[Mathias Nyman]

On 28.11.2017 13:16, Yaroslav Isakov wrote:

Mathias,
Hello again! Any news on progress of upstreaming this patch?



Applied to my for-usb-linus branch on top of 4.15-rc1

I pushed it to internal testing and I'll let it be there a couple of days before
sending it forward to Greg

-Mathias  
--

To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] usb: serial: usb_debug: Add new USB device id

[Lu Baolu]

Hi Johan,

On 11/28/2017 05:01 PM, Johan Hovold wrote:
> On Tue, Nov 28, 2017 at 12:40:59PM +0800, Lu Baolu wrote:
>> USB vendor id and product id for Linux USB Debug Target is added.
>>
>> Signed-off-by: Lu Baolu 
>> ---
>>  drivers/usb/serial/usb_debug.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/usb/serial/usb_debug.c b/drivers/usb/serial/usb_debug.c
>> index ab5a2ac..47636b6 100644
>> --- a/drivers/usb/serial/usb_debug.c
>> +++ b/drivers/usb/serial/usb_debug.c
>> @@ -32,12 +32,14 @@ static const struct usb_device_id id_table[] = {
>>  
>>  static const struct usb_device_id dbc_id_table[] = {
>>  { USB_DEVICE(0x1d6b, 0x0011) },
>> +{ USB_DEVICE(0x1d6b, 0x0010) },
>>  { },
>>  };
>>  
>>  static const struct usb_device_id id_table_combined[] = {
>>  { USB_DEVICE(0x0525, 0x127a) },
>>  { USB_DEVICE(0x1d6b, 0x0011) },
>> +{ USB_DEVICE(0x1d6b, 0x0010) },
>>  { },
>>  };
>>  MODULE_DEVICE_TABLE(usb, id_table_combined);
> For next time, please try to keep the device-id tables sorted when
> adding new entries. I fixed it up before applying with a stable tag.
>

Sure. Thanks for the reminder.

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 1/1] usb: gadget: u_serial: Use kfifo instead of homemade circular buffer

[Lu Baolu]

Hi Felipe,

On 11/28/2017 04:05 PM, Felipe Balbi wrote:
> Hi,
>
> Lu Baolu  writes:
>> The kernel FIFO implementation, kfifo, provides interfaces to manipulate
>> a first-in-first-out circular buffer.  Use kfifo instead of the homemade
>> one to make the code more concise and readable.
>>
>> Signed-off-by: Lu Baolu 
> Thanks :-) Can you give a little description of how you tested this?
>

I tested it on an Intel Skylake box. On the gadget side, I loaded a serial
device (#modprobe g_serial). And then, connected it with a host.

On the gadget side, I run below scripts:

#!/bin/bash

for j in `seq 1 200`;
do
for i in `seq 1 5`;
do
echo $i.$j > /dev/ttyGS0
done
done

And, on the host side,

#cat /dev/ttyACM0

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: very slow USB security token access after switch from 2.6.13 to 4.4.52

[Nikola Ciprich]

Hi Greg,

thanks a lot for your reply! I've found the difference just yesterday,
it's not kernel problem after all, the reason it was much faster
under suse is  that there was public key caching mechanism which
I wasn't aware of..

(I had the same opensc  config, but cache needed to be created using
pkcs15-tool).

So now it's even faster under new OS :-)


> > we're migrating lots (thousands) of users from very old opensuse running
> > 2.6.13 kernel to new centos based system running 4.4.52 kernel.
> 
> That's a huge jump :)

don't tell me about that, we had to deal with some really old hardware
as well :)

> 
> > There are security tokens in use on desktops - Ikey 4000. They work,
> > but on new system initialisation of token and reading certificates is taking
> > very long, slowing firefox (and other pkcs11 users) start by ~20seconds. 
> > Since
> > users are running those token using apps very often, this is causing lots 
> > of hassle..
> > 
> > The problem is, that token support is proprietary, I don't have any 
> > sources, just
> > those arcane opensc + openct binaries built for opensuse I'm forced to on 
> > new system
> > (therefore I'm also using 32bit firefox). Since the switch to new tokens 
> > with better
> > support is planned on next year, I'd really love to resolve this issue.
> 
> So is it using libusb to talk to the device, or a kernel module?
> 
> If libusb, you can use usbmon to see the data being sent to/from the
> device, to see where the stalls are, and hopefully compare the two
> systems to see what the difference is.

after all, I didn't need to use that, but it's  good to know..


> I don't know what ifdhandler is, but the usbdevfs calls are userspace
> calls to the usb device.
> 
> > I know that I'm probably providing insufficient information here, but does
> > somebody have some Idea on what I should check or how to better debug this?
> 
> Get the company you bought the device from to support it as you paid for
> it?  :)

devices were bought years ago and are unsupported unfortunately.. 

but it's solved after all.

anyways, thanks once more for your time, I really appreciate it

bye

nik


> Sorry, other than that, try usbmon, and see if you can see any timing
> differences.
> 
> good luck!
> 
> greg k-h
> 

-- 
-
Ing. Nikola CIPRICH
LinuxBox.cz, s.r.o.
28.rijna 168, 709 00 Ostrava

tel.:   +420 591 166 214
fax:+420 596 621 273
mobil:  +420 777 093 799
www.linuxbox.cz

mobil servis: +420 737 238 656
email servis: ser...@linuxbox.cz
-
--
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: drivers/net/usb/r8152.c USB net driver outdated

[Greg KH]

On Tue, Nov 28, 2017 at 07:51:06PM -0700, Cameron Seader wrote:
> Greetings,
> The upstream kernel seems to be outdated with version 1.09.9 of the
> drivers/net/usb/r8152.c driver. There is newer hardware now which requires
> the newer version where running with the old one becomes unstable. For
> example the latest Dell Precision 5520 uses this driver. I have currently
> tested the newest driver I can find which is dated 8/30/2017 at version
> 2.09.0.
> 
> http://www.realtek.com.tw/downloads/downloadsView.aspx?Langid=1=56=56=5=4=3=false#RTL8153
> 
> Attached is the driver.
> 
> I have compiled this newer driver on Kernel 4.14.1 on openSUSE Tumbleweed
> and have been successful with no issues. It certainly is a lot more stable
> than the previous which caused issues coming out of sleep mode where it
> would drop the speed of the NIC to 100 and set it to half duplex.
> 
> I'm asking that we please look at updating this driver in the latest kernel.

Care to submit patches to update the existing in-kernel driver to
provide the support that this out-of-tree driver has?  That's the only
way this is going to happen :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

drivers/net/usb/r8152.c USB net driver outdated

[Cameron Seader]

Greetings,
The upstream kernel seems to be outdated with version 1.09.9 of the 
drivers/net/usb/r8152.c driver. There is newer hardware now which 
requires the newer version where running with the old one becomes 
unstable. For example the latest Dell Precision 5520 uses this driver. I 
have currently tested the newest driver I can find which is dated 
8/30/2017 at version 2.09.0.


http://www.realtek.com.tw/downloads/downloadsView.aspx?Langid=1=56=56=5=4=3=false#RTL8153

Attached is the driver.

I have compiled this newer driver on Kernel 4.14.1 on openSUSE 
Tumbleweed and have been successful with no issues. It certainly is a 
lot more stable than the previous which caused issues coming out of 
sleep mode where it would drop the speed of the NIC to 100 and set it to 
half duplex.


I'm asking that we please look at updating this driver in the latest 
kernel.


Thanks,
--
Cameron Seader
Technology Strategist
SUSE
c...@suse.com
(P)+1 208.572.0095
(M)+1 208.420.2167


0008-r8152.53-2.09.0.tar.bz2
Description: application/bzip

Re: [PATCH 1/1] usb: gadget: u_serial: Use kfifo instead of homemade circular buffer

[Felipe Balbi]

Hi,

Lu Baolu  writes:
> The kernel FIFO implementation, kfifo, provides interfaces to manipulate
> a first-in-first-out circular buffer.  Use kfifo instead of the homemade
> one to make the code more concise and readable.
>
> Signed-off-by: Lu Baolu 

Thanks :-) Can you give a little description of how you tested this?

-- 
balbi


signature.asc
Description: PGP signature

Re: Fwd: Performance regression in dwc3 ecm for linux 4.4.60

[Felipe Balbi]

Hi,

Arjav Parikh  writes:
> Hi Team,
>
> Hope you all are doing good.
>
> Currently I am facing an bandwidth issue in dwc3 driver using ecm
> gadget with linux kernel version 4.4.

did you try mainline? Try mainline and let me know how it goes.

-- 
balbi


signature.asc
Description: PGP signature

Re: Fwd: Performance regression in dwc3 ecm for linux 4.4.60

[Felipe Balbi]

Hi,

Arjav Parikh  writes:

> Hi,
>
> Thank You for your response.
>
> I will update you after trying main line kernel.

read this:

https://www.kernel.org/doc/html/v4.14/process/2.Process.html?highlight=process#mailing-lists

In short: don't top-post ;-)

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH] USB: serial: Clean up the return values of read_reg and write_reg

[Johan Hovold]

On Mon, Nov 27, 2017 at 03:36:51PM +, Gimcuan Hui wrote:
> write_reg returns 0 on success, we can make it more explicit by returning
> number 0 instead of result variable.
> 
> read_reg should return 0 on success since this is a more common pattern.
> 
> The user of read_reg has been clean-up and should be at the same commit.
> 
> Signed-off-by: Gimcuan Hui 

Thanks for the update. Now applied for -next with a slightly modified
summary:

USB: serial: ark3116: clean up return values of register accessors

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: EHCI and MUSB do not discover devices without CONFIG_PM

[Ladislav Michl]

Hi Greg,

On Tue, Nov 28, 2017 at 08:33:28AM +0100, Greg KH wrote:
> On Mon, Nov 27, 2017 at 11:08:33PM +0100, Ladislav Michl wrote:
> > Hi there,
> > 
> > USB hosts do not discover any connected device on OMAP3 based board
> > with CONFIG_PM=n. Just enabling this option is enough to restore working
> > behaviour. Nothing unusual in log. Tested 4.14.2 and 4.15-rc1. I know
> > a lot of stuff depends on CONFIG_PM, but is this expected behaviour?
> > Neither EHCI nor MUSB is working without CONFIG_PM.
> 
> What bus type is your controllers on?  PCI?  platform?  Something else?

Platform controllers inside OMAP3630 Soc.

> And yes, perhaps this is to be expected, why would you not want
> CONFIG_PM to be enabled?  :)

For a start, I know Linux is general purpose OS and I know I cannot expect
low latency or low jitter when dealing with interrupts.

Original problem is described here:
https://www.spinics.net/lists/linux-omap/msg140081.html

Shortly, with CONFIG_PM jitter of GPIO interrupt is about 350us which
renders IR receiver unuseable - is cannot reliably decode IR protocol
(gpio-ir-recv is used). With CONFIG_PM disabled, jitter is around 30us
and that's enough to make IR decoders work.

And as I was unable to fix it, nor anyone provided useful hint, I though
I could work around problem from another side. And here we are...

Thank you,
ladis
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] usb: serial: usb_debug: Add new USB device id

[Johan Hovold]

On Tue, Nov 28, 2017 at 12:40:59PM +0800, Lu Baolu wrote:
> USB vendor id and product id for Linux USB Debug Target is added.
> 
> Signed-off-by: Lu Baolu 
> ---
>  drivers/usb/serial/usb_debug.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/serial/usb_debug.c b/drivers/usb/serial/usb_debug.c
> index ab5a2ac..47636b6 100644
> --- a/drivers/usb/serial/usb_debug.c
> +++ b/drivers/usb/serial/usb_debug.c
> @@ -32,12 +32,14 @@ static const struct usb_device_id id_table[] = {
>  
>  static const struct usb_device_id dbc_id_table[] = {
>   { USB_DEVICE(0x1d6b, 0x0011) },
> + { USB_DEVICE(0x1d6b, 0x0010) },
>   { },
>  };
>  
>  static const struct usb_device_id id_table_combined[] = {
>   { USB_DEVICE(0x0525, 0x127a) },
>   { USB_DEVICE(0x1d6b, 0x0011) },
> + { USB_DEVICE(0x1d6b, 0x0010) },
>   { },
>  };
>  MODULE_DEVICE_TABLE(usb, id_table_combined);

For next time, please try to keep the device-id tables sorted when
adding new entries. I fixed it up before applying with a stable tag.

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: EHCI and MUSB do not discover devices without CONFIG_PM

[Felipe Balbi]

Hi,

Ladislav Michl  writes:
> On Tue, Nov 28, 2017 at 08:33:28AM +0100, Greg KH wrote:
>> On Mon, Nov 27, 2017 at 11:08:33PM +0100, Ladislav Michl wrote:
>> > Hi there,
>> > 
>> > USB hosts do not discover any connected device on OMAP3 based board
>> > with CONFIG_PM=n. Just enabling this option is enough to restore working
>> > behaviour. Nothing unusual in log. Tested 4.14.2 and 4.15-rc1. I know
>> > a lot of stuff depends on CONFIG_PM, but is this expected behaviour?
>> > Neither EHCI nor MUSB is working without CONFIG_PM.
>> 
>> What bus type is your controllers on?  PCI?  platform?  Something else?
>
> Platform controllers inside OMAP3630 Soc.
>
>> And yes, perhaps this is to be expected, why would you not want
>> CONFIG_PM to be enabled?  :)
>
> For a start, I know Linux is general purpose OS and I know I cannot expect
> low latency or low jitter when dealing with interrupts.
>
> Original problem is described here:
> https://www.spinics.net/lists/linux-omap/msg140081.html
>
> Shortly, with CONFIG_PM jitter of GPIO interrupt is about 350us which
> renders IR receiver unuseable - is cannot reliably decode IR protocol
> (gpio-ir-recv is used). With CONFIG_PM disabled, jitter is around 30us
> and that's enough to make IR decoders work.
>
> And as I was unable to fix it, nor anyone provided useful hint, I though
> I could work around problem from another side. And here we are...

Isn't it enough to just set a huge timeout for GPIO's runtime_pm? You
can do that through sysfs. Just write a big number to the GPIO's
autosuspend_delay_ms file. This should help you:

modified   drivers/gpio/gpio-omap.c
@@ -1238,6 +1238,8 @@ static int omap_gpio_probe(struct platform_device *pdev)
 
platform_set_drvdata(pdev, bank);
 
+   pm_runtime_use_autosuspend(dev);
+   pm_runtime_set_autosuspend_delay(dev, 6);
pm_runtime_enable(dev);
pm_runtime_irq_safe(dev);
pm_runtime_get_sync(dev);


-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH 2/2 v7] typec: tcpm: Only request matching pdos

[Adam Thomson]

On 28 November 2017 01:38, Badhri Jagan Sridharan wrote:

> > examples in the kernel where this happens. Where are these functions likely 
> > to
> > be called from, as wherever that is it will need a reference to the port?
> > Actually should we be adding DT bindings to set supported src/snk PDOs from 
> > FW,
> > if you're taking this approach to PDO selection?
> >
> > This patch also seemingly leaves 'max_snk_mv', 'max_snk_ma' and 'max_snk_mv'
> > redundant, although those values can be configured from a PD controller 
> > driver
> > (e.g. fusb302 actually supports DT bindings which allow these to be set 
> > through
> > FW). Now these DT bindings are basically made redundant by your change as 
> > they
> > have no impact. Are we expecting these to be used again in the future, or 
> > should
> 
> Yes, I think  'max_snk_mv', 'max_snk_ma' and 'max_snk_mw' etc should be
> removed.
> The problem here is that maintaining these values implies that tcpm is
> not going to
> request pdo based on the sink_caps that are published. All these
> values can be derived from
> the sink_pdo objects that were declared, hence, they are redundant,
> I will update the patch to remove this.

I have no problem really with this approach, other than right now with your
patch there's no way to actually set the PDOs other than the 2 functions to
update source and sink capabilities. Previously you had the option, at least
through the fusb302 driver, to configure the max_snk_* values from DT, but your
patch obviously changes this behaviour. I think we need a FW based method of
configuring these at startup at least, as with your current patch the values
being used are hard coded. As this is generic for TCPM I would guess DT bindings
(and maybe equivalent ACPI properties as well I guess) would be a sensible
approach.
 
> > these be removed? FYI, as part of my PPS patch set I have been using them as
> > part of the PPS APDO selection criteria, based on TCPM code prior to your
> > modifications, as for PPS we're interested in a wide range of voltages and
> > currents but want to stay within the platforms limits.
> 
> Arent you defining a new PDO type similar to PDO_FIXED, PDO_VARIABLE etc ?
> If so values such as " 'max_snk_mv', 'max_snk_ma' and
> 'max_snk_mw' " should be part of the APDO object right ? So why would
> you still need
>  'max_snk_mv', 'max_snk_ma' and 'max_snk_mw'  ?

My initial implementation was based on the approach before your changes, and
actually for PPS this to me made sense, at least from the sink side. We are
dealing with a range of voltages and currents so the important points are the
maximum values. However, if we're now making decisions based on sink PDOs then
I can look at adapting.

Re: usbip port number limits

[Shuah Khan]

On 11/28/2017 11:32 AM, Shuah Khan wrote:
> On 11/26/2017 06:33 PM, Yuyang Du wrote:
>> Hi Juan,
>>
>> On Fri, Nov 24, 2017 at 12:42:22PM +0100, Juan Zea wrote:
>>>The patch doesn't apply cleanly with the patch command, but given it is 
>>> that simple I've changed it myself (I'm telling you just in case we're 
>>> missing something).
>>>
>>>The fingerprint reader, usb stick and wacom tablet work :)
>>  
>> Good to hear that.
>>
>>> I've also checked the same patch can be applied to kernel master and it 
>>> works. So... is that the solution?
>>
>> I'll post a patch later, but before that, Hi, Shuah, does it work
>> for you, on your test notebook?
>>
> 
> I am going to test this today and if it is fixes the problem on
> my system.
> 

Hi Yuyang,

I tried the patch on 4.15-rc1 and looks good to me. Please send a
proper patch to fix this. It need to go into 4.14 stable for sure.

There are no more 4.13 stable releases at this time. So let's get
this into 4.15 first and them go from there.

I am working on a patch for the other issue Juan reported that prevents
finding a free-port when the first port is usb3. You don't have to worry
about that.

thanks,
-- Shuah


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