Re: High CPU load produced by USB (DW2)

[Minas Harutyunyan]

On 2/19/2018 9:03 PM, Marek Vasut wrote:
> On 02/19/2018 11:11 AM, Minas Harutyunyan wrote:
>> On 2/19/2018 12:51 PM, Marek Vasut wrote:
>>> On 02/19/2018 09:19 AM, Minas Harutyunyan wrote:
 On 2/17/2018 12:07 AM, Marek Vasut wrote:
> On 02/16/2018 06:59 AM, Minas Harutyunyan wrote:
>> On 2/15/2018 5:20 PM, Mirza Krak wrote:
>>> On 14 February 2018 at 13:07, Minas Harutyunyan
>>>  wrote:
 On 2/14/2018 12:57 PM, Mirza Krak wrote:
> On 8 February 2018 at 14:53, Minas Harutyunyan
>  wrote:
>>>
>>> < snip >
>>>

 I reviewed your interrupt count log again. About 140,000 interrupts in 
 2
 seconds, obviously it's not SOF only interrupts. More probably, its NAK
 respond interrupts to SSPLIT/CSPLIT transactions. For this case I can
 recommend you to apply patch from Douglas Anderson: "[PATCH v2] usb:
 dwc2: host: Don't retry NAKed transactions right away" which already
 merged to 4.16-rc1.

 In your setups you see different behavior on different HUBs. Your HUBs
 have different "TT think time": 8 and 32. In USB2.0 spec "TT think 
 time"
 described as follow "TT requires at most 8/32 FS bit times of inter
 transaction gap on a full-/low-speed downstream bus". So, your "worst"
 HUB with "TT think time"=8 sending more frequently SSPLIT/CSPLIT
 transactions which replied by NAK. As result you see about 4 time more
 interrupts comparing to "good" HUB. Could you please check interrupts
 count for "good" HUB and check "4 time" hypothesis.
>>>
>>> Did some further testing. The "good" HUB is actually as bad as the
>>> "bad" HUB, and it was my setup that caused the different behavior. Did
>>> not use the same devices etc.
>>>
>>> Once I made sure that the configuration and setup was the same on both
>>> board I could see that the behaved similarly. And that is the
>>> following interrupt load:
>>>
>>> - BT USB (FS) = ~80k interrupts / second
>>> - Keyboard (FS) = ~80k interrupts / second
>>> - WiFI USB (HS) = ~8k interrupts / second
>>>
>>> After applying the suggested patch [1], it is steady around 8k
>>> interrupts / second no matter what device I connect (HS/FS,
>>> HUB/NO-HUB). Which is acceptable and usable.
>>
>> Great!
>
> So 8k IRQs per second is what I should expect from this HW with HS
> device attached, that's normal and cannot be reduced ?
>
 If core acting as Host in Buffer DMA mode then 8k IRQs per second (SOF
 interrupts) is expected if connected device(s) has periodic endpoint.
>>>
>>> Is there a way to reduce that or is that the absolute minimum in HS mode?
>>>
>> We already discussed, in this email thread earlier, why SOF interrupts
>> required and unmasked.
>> Only in case when connected device with CTRL+BLK EP's only (like flash
>> drive) and directly connected to cores root HUB, SOF's will be masked.
> 
> That's the setup I have on Altera SoCFPGA, yet the SOFs are still present.
> 
Could you please send verbose lsusb on connected to dwc2 device and 
driver debug log.
Thanks,
Minas

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

Re: [PATCH v5 00/17] Support for Qualcomm QUSBv2 and QMPv3 USB PHYs

[Manu Gautam]

Hi Kishon,


On 2/16/2018 5:19 PM, Kishon Vijay Abraham I wrote:
>
> On Thursday 01 February 2018 06:08 PM, Kishon Vijay Abraham I wrote:
>>
>> The series looks good. I'll start merging once -rc1 is tagged.
> merged, thanks!
>
> -Kishon

Following git isn't updated with these:
git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git

Should I be looking at somewhere else?
I have some follow-up PHY patches to send, hence wanted to make sure they
apply cleanly in your tree.

-Manu

-- 
The Qualcomm Innovation Center, 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

Re: [PATCH] phy: mapphone-mdm6600: Add USB PHY driver for MDM6600 on Droid 4

[Rob Herring]

On Sat, Feb 17, 2018 at 01:07:23PM -0800, Tony Lindgren wrote:
> Let's add support for the GPIO controlled USB PHY on the MDM6600 modem.
> It is used on some Motorola Mapphone series of phones and tablets such
> as Droid 4.
> 
> The MDM6600 is hardwired to the first OHCI port in the Droid 4 case, and
> is controlled by several GPIOs. The USB PHY is integrated into the MDM6600
> device it seems. We know this as we get L3 errors from omap-usb-host if
> trying to use the PHY before MDM6600 is configured.
> 
> The GPIOs controlling MDM6600 are used to power MDM660 on and off, to

MDM660 a typo?

> configure the USB start-up mode (normal mode versus USB flashing), and
> they also tell the state of the MDM6600 device.
> 
> The two start-up mode GPIOs are dual-purposed and used for out of band
> (OOB) wake-up for USB and TS 27.010 serial mux. But we need to configure
> the USB start-up mode first to get MDM6600 booted in the right mode to
> be usable in the first place.
> 
> For now, this driver just gives up the two start-up mode GPIOs after the
> modem has been configured to boot in normal mode. One of them we may
> want to configure for USB OOB wake in this driver later on, but that's a
> separate series of patches and needs more work.
> 
> Note that the Motorola Mapphone Linux kernel tree has a "radio-ctrl"
> driver for modems. But it really does not control the radio at all, it
> just controls the modem power and start-up mode for USB. So I came to
> the conclusion that we're better off having this done in the USB PHY
> driver. For adding support for USB flashing mode, we can later on add
> a kernel module option for flash_mode=1 or something similar.
> 
> Also note that currently there is no PM runtime support for the OHCI
> on omap variant SoCs. So for low(er) power idle states, currenty both
> ohci-platform and phy-mapphone-mdm6600 must be unloaded or unbound.
> 
> For reference here is what I measured for total power consumption on
> an idle Droid 4 with and without USB related MDM6600 modules:
> 
> idle lcd off  phy-mapphone-mdm6600ohci-platform
> 153mW 284mW   344mW
> 
> So it seems that MDM6600 is currently not yet idling even with it's
> radio turned off, but that's something that is beyond the control of
> this USB PHY driver.
> 
> Cc: devicet...@vger.kernel.org
> Cc: Mark Rutland 
> Cc: Marcel Partap 
> Cc: Michael Scott 
> Cc: Rob Herring 
> Cc: Sebastian Reichel 
> Signed-off-by: Tony Lindgren 
> ---
>  .../bindings/phy/phy-mapphone-mdm6600.txt  |  30 ++
>  drivers/phy/motorola/Kconfig   |   9 +
>  drivers/phy/motorola/Makefile  |   1 +
>  drivers/phy/motorola/phy-mapphone-mdm6600.c| 490 
> +
>  4 files changed, 530 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt
>  create mode 100644 drivers/phy/motorola/phy-mapphone-mdm6600.c
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt 
> b/Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/phy-mapphone-mdm6600.txt
> @@ -0,0 +1,30 @@
> +Device tree binding documentation for Motorola Mapphone MDM6600 USB PHY
> +
> +Required properties:
> +- compatible Must be "motorola,mapphone-mdm6600"
> +- enable-gpios   GPIO to enable the USB PHY
> +- power-gpiosGPIO to power on the device
> +- reset-gpiosGPIO to reset the device

The are pretty standard, but...

> +- mode-gpios Two GPIOs to configure MDM6600 USB start-up mode for
> + normal mode versus USB flashing mode
> +- status-gpios   Three GPIOs to read the power state of the MDM6600
> +- cmd-gpios  Three GPIOs to control the power state of the MDM6600

These 3 should have vendor a prefix.

> +
> +Example:
> +
> +fsusb1_phy: fsusb1_phy {

usb-phy {

> + compatible = "motorola,mapphone-mdm6600";
> + enable-gpios = < 31 GPIO_ACTIVE_LOW>; /* gpio_95 */
> + power-gpios = < 22 GPIO_ACTIVE_HIGH>; /* gpio_54 */
> + reset-gpios = < 17 GPIO_ACTIVE_HIGH>; /* gpio_49 */
> + mode-gpios = < 20 GPIO_ACTIVE_HIGH>,  /* gpio_148 */
> +  < 21 GPIO_ACTIVE_HIGH>;  /* gpio_149 */
> + status-gpios = < 23 GPIO_ACTIVE_HIGH>,/* gpio_55 */
> +< 21 GPIO_ACTIVE_HIGH>,/* gpio_53 */
> +< 20 GPIO_ACTIVE_HIGH>;/* gpio_52 */
> + cmd-gpios = < 7 GPIO_ACTIVE_HIGH>,/* gpio_103 */
> + < 8 GPIO_ACTIVE_HIGH>,/* gpio_104 */
> + < 14 GPIO_ACTIVE_HIGH>;   /* gpio_142 */
> + #phy-cells = <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  

[PATCH v2] xhci: Fix front USB ports on ASUS PRIME B350M-A

[Kai-Heng Feng]

When a USB device gets plugged on ASUS PRIME B350M-A's front ports, the
xHC stops working:
[  549.114587] xhci_hcd :02:00.0: WARN: xHC CMD_RUN timeout
[  549.114608] suspend_common(): xhci_pci_suspend+0x0/0xc0 returns -110
[  549.114638] xhci_hcd :02:00.0: can't suspend (hcd_pci_runtime_suspend 
returned -110)

Delay before running xHC command CMD_RUN can workaround the issue.

Use a new quirk to make the delay only targets to the affected xHC.

Signed-off-by: Kai-Heng Feng 
---
v2: Instead of doing xHC reset and disabling D3cold, a simple delay can
workaround the issue. Now both high-speed and super-speed devices work
fine with the v2 quirk.

 drivers/usb/host/xhci-pci.c | 3 +++
 drivers/usb/host/xhci.c | 3 +++
 drivers/usb/host/xhci.h | 1 +
 3 files changed, 7 insertions(+)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 6c79037876db..9a820b3cae56 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -122,6 +122,9 @@ static void xhci_pci_quirks(struct device *dev, struct 
xhci_hcd *xhci)
if (pdev->vendor == PCI_VENDOR_ID_AMD && usb_amd_find_chipset_info())
xhci->quirks |= XHCI_AMD_PLL_FIX;
 
+   if (pdev->vendor == PCI_VENDOR_ID_AMD && pdev->device == 0x43bb)
+   xhci->quirks |= XHCI_SUSPEND_DELAY;
+
if (pdev->vendor == PCI_VENDOR_ID_AMD)
xhci->quirks |= XHCI_TRUST_TX_LENGTH;
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 1eeb3396300f..d554bbd694d3 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -878,6 +878,9 @@ int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup)
clear_bit(HCD_FLAG_POLL_RH, >shared_hcd->flags);
del_timer_sync(>shared_hcd->rh_timer);
 
+   if (xhci->quirks & XHCI_SUSPEND_DELAY)
+   usleep_range(1000, 1500);
+
spin_lock_irq(>lock);
clear_bit(HCD_FLAG_HW_ACCESSIBLE, >flags);
clear_bit(HCD_FLAG_HW_ACCESSIBLE, >shared_hcd->flags);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 96099a245c69..6f1f52f3cb40 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1825,6 +1825,7 @@ struct xhci_hcd {
 /* Reserved. It was XHCI_U2_DISABLE_WAKE */
 #define XHCI_ASMEDIA_MODIFY_FLOWCONTROL(1 << 28)
 #define XHCI_HW_LPM_DISABLE(1 << 29)
+#define XHCI_SUSPEND_DELAY (1 << 30)
 
unsigned intnum_active_eps;
unsigned intlimit_active_eps;
-- 
2.15.1

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

Re: High CPU load produced by USB (DW2)

[Marek Vasut]

On 02/19/2018 11:11 AM, Minas Harutyunyan wrote:
> On 2/19/2018 12:51 PM, Marek Vasut wrote:
>> On 02/19/2018 09:19 AM, Minas Harutyunyan wrote:
>>> On 2/17/2018 12:07 AM, Marek Vasut wrote:
 On 02/16/2018 06:59 AM, Minas Harutyunyan wrote:
> On 2/15/2018 5:20 PM, Mirza Krak wrote:
>> On 14 February 2018 at 13:07, Minas Harutyunyan
>>  wrote:
>>> On 2/14/2018 12:57 PM, Mirza Krak wrote:
 On 8 February 2018 at 14:53, Minas Harutyunyan
  wrote:
>>
>> < snip >
>>
>>>
>>> I reviewed your interrupt count log again. About 140,000 interrupts in 2
>>> seconds, obviously it's not SOF only interrupts. More probably, its NAK
>>> respond interrupts to SSPLIT/CSPLIT transactions. For this case I can
>>> recommend you to apply patch from Douglas Anderson: "[PATCH v2] usb:
>>> dwc2: host: Don't retry NAKed transactions right away" which already
>>> merged to 4.16-rc1.
>>>
>>> In your setups you see different behavior on different HUBs. Your HUBs
>>> have different "TT think time": 8 and 32. In USB2.0 spec "TT think time"
>>> described as follow "TT requires at most 8/32 FS bit times of inter
>>> transaction gap on a full-/low-speed downstream bus". So, your "worst"
>>> HUB with "TT think time"=8 sending more frequently SSPLIT/CSPLIT
>>> transactions which replied by NAK. As result you see about 4 time more
>>> interrupts comparing to "good" HUB. Could you please check interrupts
>>> count for "good" HUB and check "4 time" hypothesis.
>>
>> Did some further testing. The "good" HUB is actually as bad as the
>> "bad" HUB, and it was my setup that caused the different behavior. Did
>> not use the same devices etc.
>>
>> Once I made sure that the configuration and setup was the same on both
>> board I could see that the behaved similarly. And that is the
>> following interrupt load:
>>
>> - BT USB (FS) = ~80k interrupts / second
>> - Keyboard (FS) = ~80k interrupts / second
>> - WiFI USB (HS) = ~8k interrupts / second
>>
>> After applying the suggested patch [1], it is steady around 8k
>> interrupts / second no matter what device I connect (HS/FS,
>> HUB/NO-HUB). Which is acceptable and usable.
>
> Great!

 So 8k IRQs per second is what I should expect from this HW with HS
 device attached, that's normal and cannot be reduced ?

>>> If core acting as Host in Buffer DMA mode then 8k IRQs per second (SOF
>>> interrupts) is expected if connected device(s) has periodic endpoint.
>>
>> Is there a way to reduce that or is that the absolute minimum in HS mode?
>>
> We already discussed, in this email thread earlier, why SOF interrupts 
> required and unmasked.
> Only in case when connected device with CTRL+BLK EP's only (like flash 
> drive) and directly connected to cores root HUB, SOF's will be masked.

That's the setup I have on Altera SoCFPGA, yet the SOFs are still present.

-- 
Best regards,
Marek Vasut
--
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: introduce per-port over-current counters

[Greg KH]

On Mon, Feb 19, 2018 at 05:05:34PM +0100, Richard Leitner wrote:
> 
> On 02/19/2018 04:59 PM, Greg KH wrote:
> > On Mon, Feb 19, 2018 at 01:01:07PM +0100, Richard Leitner wrote:
> >> From: Richard Leitner 
> >>
> >> For some userspace applications information on the number of
> >> over-current conditions at specific USB hub ports is relevant. Therefore
> >> introduce a oc_counter in the usb port struct which is exported via
> >> sysfs.
> >>
> >> Signed-off-by: Richard Leitner 
> >> ---
> >> Tested on an i.MX6DL based board.
> >> ---
> >>  drivers/usb/core/hub.c  |  4 +++-
> >>  drivers/usb/core/hub.h  |  1 +
> >>  drivers/usb/core/port.c | 10 ++
> >>  3 files changed, 14 insertions(+), 1 deletion(-)
> > 
> > When you add/remove/modify a sysfs attribute, you always have to
> > document in Documentation/ABI/
> 
> Ok. Thank you. Seems I missed that, Sorry!
> 
> >>
> >> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> >> index c5c1f6cf3228..448fba1e1827 100644
> >> --- a/drivers/usb/core/hub.c
> >> +++ b/drivers/usb/core/hub.c
> >> @@ -5104,8 +5104,10 @@ static void port_event(struct usb_hub *hub, int 
> >> port1)
> >>  
> >>if (portchange & USB_PORT_STAT_C_OVERCURRENT) {
> >>u16 status = 0, unused;
> >> +  port_dev->oc_count++;
> >>  
> >> -  dev_dbg(_dev->dev, "over-current change\n");
> >> +  dev_dbg(_dev->dev, "over-current change #%u\n",
> >> +  port_dev->oc_count);
> >>usb_clear_port_feature(hdev, port1,
> >>USB_PORT_FEAT_C_OVER_CURRENT);
> >>msleep(100);/* Cool down */
> >> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> >> index 2a700ccc868c..b5cf567bf9e2 100644
> >> --- a/drivers/usb/core/hub.h
> >> +++ b/drivers/usb/core/hub.h
> >> @@ -100,6 +100,7 @@ struct usb_port {
> >>unsigned int is_superspeed:1;
> >>unsigned int usb3_lpm_u1_permit:1;
> >>unsigned int usb3_lpm_u2_permit:1;
> >> +  unsigned int oc_count;
> >>  };
> >>  
> >>  #define to_usb_port(_dev) \
> >> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> >> index 1a01e9ad3804..0bfe410eb8a7 100644
> >> --- a/drivers/usb/core/port.c
> >> +++ b/drivers/usb/core/port.c
> >> @@ -41,6 +41,15 @@ static ssize_t connect_type_show(struct device *dev,
> >>  }
> >>  static DEVICE_ATTR_RO(connect_type);
> >>  
> >> +static ssize_t oc_count_show(struct device *dev, struct device_attribute 
> >> *attr,
> >> +   char *buf)
> >> +{
> >> +  struct usb_port *port_dev = to_usb_port(dev);
> >> +
> >> +  return sprintf(buf, "%u\n", port_dev->oc_count);
> >> +}
> >> +static DEVICE_ATTR_RO(oc_count);
> > 
> > I don't see what userspace can do with this number, as there's not much
> > it can do with it.
> 
> I've answered this question to Felipe already:



It's not described in the patch changelog, which is where it is required
:)

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] usb: core: introduce per-port over-current counters

[Richard Leitner]

On 02/19/2018 04:59 PM, Greg KH wrote:
> On Mon, Feb 19, 2018 at 01:01:07PM +0100, Richard Leitner wrote:
>> From: Richard Leitner 
>>
>> For some userspace applications information on the number of
>> over-current conditions at specific USB hub ports is relevant. Therefore
>> introduce a oc_counter in the usb port struct which is exported via
>> sysfs.
>>
>> Signed-off-by: Richard Leitner 
>> ---
>> Tested on an i.MX6DL based board.
>> ---
>>  drivers/usb/core/hub.c  |  4 +++-
>>  drivers/usb/core/hub.h  |  1 +
>>  drivers/usb/core/port.c | 10 ++
>>  3 files changed, 14 insertions(+), 1 deletion(-)
> 
> When you add/remove/modify a sysfs attribute, you always have to
> document in Documentation/ABI/

Ok. Thank you. Seems I missed that, Sorry!

>>
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index c5c1f6cf3228..448fba1e1827 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -5104,8 +5104,10 @@ static void port_event(struct usb_hub *hub, int port1)
>>  
>>  if (portchange & USB_PORT_STAT_C_OVERCURRENT) {
>>  u16 status = 0, unused;
>> +port_dev->oc_count++;
>>  
>> -dev_dbg(_dev->dev, "over-current change\n");
>> +dev_dbg(_dev->dev, "over-current change #%u\n",
>> +port_dev->oc_count);
>>  usb_clear_port_feature(hdev, port1,
>>  USB_PORT_FEAT_C_OVER_CURRENT);
>>  msleep(100);/* Cool down */
>> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
>> index 2a700ccc868c..b5cf567bf9e2 100644
>> --- a/drivers/usb/core/hub.h
>> +++ b/drivers/usb/core/hub.h
>> @@ -100,6 +100,7 @@ struct usb_port {
>>  unsigned int is_superspeed:1;
>>  unsigned int usb3_lpm_u1_permit:1;
>>  unsigned int usb3_lpm_u2_permit:1;
>> +unsigned int oc_count;
>>  };
>>  
>>  #define to_usb_port(_dev) \
>> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
>> index 1a01e9ad3804..0bfe410eb8a7 100644
>> --- a/drivers/usb/core/port.c
>> +++ b/drivers/usb/core/port.c
>> @@ -41,6 +41,15 @@ static ssize_t connect_type_show(struct device *dev,
>>  }
>>  static DEVICE_ATTR_RO(connect_type);
>>  
>> +static ssize_t oc_count_show(struct device *dev, struct device_attribute 
>> *attr,
>> + char *buf)
>> +{
>> +struct usb_port *port_dev = to_usb_port(dev);
>> +
>> +return sprintf(buf, "%u\n", port_dev->oc_count);
>> +}
>> +static DEVICE_ATTR_RO(oc_count);
> 
> I don't see what userspace can do with this number, as there's not much
> it can do with it.

I've answered this question to Felipe already:

On 02/19/2018 03:12 PM, Richard Leitner wrote:
> In our case we have a series of USB hardware (using the cp210x driver) which
> communicates using a proprietary protocol. These devices sometimes trigger an
> over-current situation on some hubs. In case of such an over-current situation
> the USB devices offer an interface for reducing the max used power.
> As these conditions are quite rare and imply performance reductions of the
> device we don't want to use reduce the max power always.
> 
> With this patch I want to give the user-space application the possibility to
> react adequately to such over-current situations.
> 
> I hope this explains my motivation for this patch in a comprehensible way.

I'm of course always open for a better/cleaner/safer way of doing this ;-)

Thank you!

regards;Richard.L
--
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: introduce per-port over-current counters

[Greg KH]

On Mon, Feb 19, 2018 at 01:01:07PM +0100, Richard Leitner wrote:
> From: Richard Leitner 
> 
> For some userspace applications information on the number of
> over-current conditions at specific USB hub ports is relevant. Therefore
> introduce a oc_counter in the usb port struct which is exported via
> sysfs.
> 
> Signed-off-by: Richard Leitner 
> ---
> Tested on an i.MX6DL based board.
> ---
>  drivers/usb/core/hub.c  |  4 +++-
>  drivers/usb/core/hub.h  |  1 +
>  drivers/usb/core/port.c | 10 ++
>  3 files changed, 14 insertions(+), 1 deletion(-)

When you add/remove/modify a sysfs attribute, you always have to
document in Documentation/ABI/


> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index c5c1f6cf3228..448fba1e1827 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -5104,8 +5104,10 @@ static void port_event(struct usb_hub *hub, int port1)
>  
>   if (portchange & USB_PORT_STAT_C_OVERCURRENT) {
>   u16 status = 0, unused;
> + port_dev->oc_count++;
>  
> - dev_dbg(_dev->dev, "over-current change\n");
> + dev_dbg(_dev->dev, "over-current change #%u\n",
> + port_dev->oc_count);
>   usb_clear_port_feature(hdev, port1,
>   USB_PORT_FEAT_C_OVER_CURRENT);
>   msleep(100);/* Cool down */
> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> index 2a700ccc868c..b5cf567bf9e2 100644
> --- a/drivers/usb/core/hub.h
> +++ b/drivers/usb/core/hub.h
> @@ -100,6 +100,7 @@ struct usb_port {
>   unsigned int is_superspeed:1;
>   unsigned int usb3_lpm_u1_permit:1;
>   unsigned int usb3_lpm_u2_permit:1;
> + unsigned int oc_count;
>  };
>  
>  #define to_usb_port(_dev) \
> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> index 1a01e9ad3804..0bfe410eb8a7 100644
> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c
> @@ -41,6 +41,15 @@ static ssize_t connect_type_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(connect_type);
>  
> +static ssize_t oc_count_show(struct device *dev, struct device_attribute 
> *attr,
> +  char *buf)
> +{
> + struct usb_port *port_dev = to_usb_port(dev);
> +
> + return sprintf(buf, "%u\n", port_dev->oc_count);
> +}
> +static DEVICE_ATTR_RO(oc_count);

I don't see what userspace can do with this number, as there's not much
it can do with it.

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 1/2] dt-bindings: usb: fix the STM32F7 DWC2 OTG HS core binding

[Rob Herring]

On Thu, Feb 15, 2018 at 04:46:40PM +0100, Amelie Delaunay wrote:
> This patch fixes binding documentation for DWC2 controller in HS mode
> found on STMicroelectronics STM32F7 SoC.
> The v2 former patch [1] had been acked by Rob Herring, but v1 was merged.
> 
> [1] https://patchwork.kernel.org/patch/9925575/
> 
> Fixes: 000777dadc7e ("dt-bindings: usb: Document the STM32F7xx DWC2 ...")
> Signed-off-by: Amelie Delaunay 
> ---
>  Documentation/devicetree/bindings/usb/dwc2.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

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

Re: [RFC PATCH v2 2/6] dt-bindings: add bindings for Samsung micro-USB 11-pin connector

[Rob Herring]

On Thu, Feb 15, 2018 at 11:39:16AM +0100, Andrzej Hajda wrote:
> Samsung micro-USB 11-pin connector beside standard micro-USB pins,
> has pins dedicated to route MHL traffic.
> 
> Signed-off-by: Andrzej Hajda 
> ---
>  .../connector/samsung,usb-connector-11pin.txt  | 51 
> ++
>  1 file changed, 51 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/connector/samsung,usb-connector-11pin.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/connector/samsung,usb-connector-11pin.txt 
> b/Documentation/devicetree/bindings/connector/samsung,usb-connector-11pin.txt
> new file mode 100644
> index ..c8ef1ad6732f
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/connector/samsung,usb-connector-11pin.txt
> @@ -0,0 +1,51 @@
> +Samsung micro-USB 11-pin connector
> +==
> +
> +Samsung micro-USB 11-pin connector is an extension of micro-USB connector.
> +It is present in multiple Samsung mobile devices.
> +It has additional pins to route MHL traffic simultanously with USB.
> +
> +The bindings are superset of usb-connector bindings for micro-USB 
> connector[1].
> +
> +Required properties:
> +- compatible: must be: "samsung,usb-connector-11pin", "usb-b-connector",
> +- type: must be "micro".
> +
> +Optional properties:
> +- label: symbolic name for the connector.

This is already defined in [1] so you don't need it here.

Otherwise,

Reviewed-by: Rob Herring 

> +
> +Required nodes:
> +- any data bus to the connector should be modeled using the OF graph bindings
> +  specified in bindings/graph.txt, unless the bus is between parent node and
> +  the connector. Since single connector can have multpile data buses every 
> bus
> +  has assigned OF graph port number as follows:
> +0: High Speed (HS),
> +3: Mobile High-Definition Link (MHL), specific to 11-pin Samsung 
> micro-USB.
> +
> +[1]: bindings/connector/usb-connector.txt
> +
> +Example
> +---
> +
> +Micro-USB connector with HS lines routed via controller (MUIC) and :
> +
> +muic-max77843@66 {
> + ...
> + usb_con: connector {
> + compatible = "samsung,usb-connector-11pin", "usb-b-connector";
> + label = "micro-USB";
> + type = "micro";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@3 {
> + reg = <3>;
> + usb_con_mhl: endpoint {
> + remote-endpoint = <_mhl>;
> + };
> + };
> + };
> + };
> +};
> -- 
> 2.16.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v2 1/6] dt-bindings: add bindings for USB physical connector

[Rob Herring]

On Thu, Feb 15, 2018 at 11:39:15AM +0100, Andrzej Hajda wrote:
> These bindings allow to describe most known standard USB connectors
> and it should be possible to extend it if necessary.
> USB connectors, beside USB can be used to route other protocols,
> for example UART, Audio, MHL. In such case every device passing data
> through the connector should have appropriate graph bindings.
> 
> Signed-off-by: Andrzej Hajda 
> ---
> v3:
> - removed MHL port (samsung connector will have separate bindings),
> - added 2nd example for USB-C,
> - improved formatting
> v2:
> - moved connector type(A,B,C) to compatible string (Rob),
> - renamed size property to type (Rob),
> - changed type description to be less confusing (Laurent),
> - removed vendor specific compatibles (implied by graph port number),
> - added requirement of connector being a child of IC (Rob),
> - removed max-mode (subtly suggested by Rob, it should be detected anyway
>   by USB Controller in runtime, downside is that device is not able to
>   report its real capabilities, maybe better would be to make it optional(?)),
> - assigned port numbers to data buses (Rob).
> 
> Regards
> Andrzej
> 
> Signed-off-by: Andrzej Hajda 
> 
> dt-bindings: add bindings for USB physical connector v3
> ---
>  .../bindings/connector/usb-connector.txt   | 74 
> ++
>  1 file changed, 74 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/connector/usb-connector.txt
> 
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt 
> b/Documentation/devicetree/bindings/connector/usb-connector.txt
> new file mode 100644
> index ..1efda92639da
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt
> @@ -0,0 +1,74 @@
> +USB Connector
> +=
> +
> +USB connector node represents physical USB connector. It should be
> +a child of USB interface controller.
> +
> +Required properties:
> +- compatible: describes type of the connector, must be one of:
> +"usb-a-connector",
> +"usb-b-connector",
> +"usb-c-connector".
> +
> +Optional properties:
> +- label: symbolic name for the connector,
> +- type: size of the connector, should be specified in case of USB-A, USB-B
> +  non-standard (large) connector sizes: "mini", "micro".

The smaller connectors are standard too. Perhaps "non-fullsize connector 
sizes".

We're missing a micro-AB connector, but I think those are actually 
pretty rare. Most phones are micro-B connectors, but do both host and 
device.

> +
> +Required nodes:
> +- any data bus to the connector should be modeled using the OF graph bindings
> +  specified in bindings/graph.txt, unless the bus is between parent node and
> +  the connector. Since single connector can have multpile data buses every 
> bus
> +  has assigned OF graph port number as follows:
> +0: High Speed (HS), present in all connectors,
> +1: Super Speed (SS), present in SS capable connectors,

This should also say endpoint 0 is USB-SS, endpoint 1 (and higher?) is 
Alternate Mode. And show in the example.

> +2: Sideband use (SBU), present in USB-C.
> +
> +Examples
> +
> +
> +1. Micro-USB connector with HS lines routed via controller (MUIC):
> +
> +muic-max77843@66 {
> + ...
> + usb_con: connector {
> + compatible = "usb-b-connector";
> + label = "micro-USB";
> + type = "micro";
> + };
> +};
> +
> +2. USB-C connector attached to CC controller (s2mm005), HS lines routed
> +to companion PMIC (max77865), SS lines to USB3 PHY and SBU to DisplayPort:

Having SBU to DP but no DP video path connection is wrong.

> +
> +ccic: s2mm005@33 {
> + ...
> + usb_con: connector {
> + compatible = "usb-c-connector";
> + label = "USB-C";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + usb_con_hs: endpoint {
> + remote-endpoint = <_usbc_hs>;
> + };
> + };
> + port@1 {
> + reg = <1>;
> + usb_con_ss: endpoint {
> + remote-endpoint = <_phy_ss>;
> + };
> + };
> + port@2 {
> + reg = <2>;
> + usb_con_sbu: endpoint {
> + remote-endpoint = <_aux>;
> + };
> + };
> + };
> + };
> +};
> -- 
> 2.16.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

Re: [PATCH] usb: core: introduce per-port over-current counters

[Richard Leitner]

Hi,

On 02/19/2018 02:53 PM, Felipe Balbi wrote:
> 
> Hi,
> 
> Richard Leitner  writes:
> 
>> From: Richard Leitner 
>>
>> For some userspace applications information on the number of
>> over-current conditions at specific USB hub ports is relevant. Therefore
>> introduce a oc_counter in the usb port struct which is exported via
>> sysfs.
> 
> relevant how? What can the application do with that knowledge?

In our case we have a series of USB hardware (using the cp210x driver) which
communicates using a proprietary protocol. These devices sometimes trigger an
over-current situation on some hubs. In case of such an over-current situation
the USB devices offer an interface for reducing the max used power.
As these conditions are quite rare and imply performance reductions of the
device we don't want to use reduce the max power always.

With this patch I want to give the user-space application the possibility to
react adequately to such over-current situations.

I hope this explains my motivation for this patch in a comprehensible way.

> 
>> Signed-off-by: Richard Leitner 
>> ---
>> Tested on an i.MX6DL based board.
>> ---
>>  drivers/usb/core/hub.c  |  4 +++-
>>  drivers/usb/core/hub.h  |  1 +
>>  drivers/usb/core/port.c | 10 ++
>>  3 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index c5c1f6cf3228..448fba1e1827 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -5104,8 +5104,10 @@ static void port_event(struct usb_hub *hub, int port1)
>>  
>>  if (portchange & USB_PORT_STAT_C_OVERCURRENT) {
>>  u16 status = 0, unused;
>> +port_dev->oc_count++;
>>  
>> -dev_dbg(_dev->dev, "over-current change\n");
>> +dev_dbg(_dev->dev, "over-current change #%u\n",
>> +port_dev->oc_count);
>>  usb_clear_port_feature(hdev, port1,
>>  USB_PORT_FEAT_C_OVER_CURRENT);
>>  msleep(100);/* Cool down */
>> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
>> index 2a700ccc868c..b5cf567bf9e2 100644
>> --- a/drivers/usb/core/hub.h
>> +++ b/drivers/usb/core/hub.h
>> @@ -100,6 +100,7 @@ struct usb_port {
>>  unsigned int is_superspeed:1;
>>  unsigned int usb3_lpm_u1_permit:1;
>>  unsigned int usb3_lpm_u2_permit:1;
>> +unsigned int oc_count;
>>  };
>>  
>>  #define to_usb_port(_dev) \
>> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
>> index 1a01e9ad3804..0bfe410eb8a7 100644
>> --- a/drivers/usb/core/port.c
>> +++ b/drivers/usb/core/port.c
>> @@ -41,6 +41,15 @@ static ssize_t connect_type_show(struct device *dev,
>>  }
>>  static DEVICE_ATTR_RO(connect_type);
>>  
>> +static ssize_t oc_count_show(struct device *dev, struct device_attribute 
>> *attr,
>> + char *buf)
>> +{
>> +struct usb_port *port_dev = to_usb_port(dev);
>> +
>> +return sprintf(buf, "%u\n", port_dev->oc_count);
>> +}
>> +static DEVICE_ATTR_RO(oc_count);
> 
> I would actually spell this out human readable: over_current_count
> 

Would be fine for me.

regards;Richard.L
--
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: introduce per-port over-current counters

[Felipe Balbi]

Hi,

Richard Leitner  writes:

> From: Richard Leitner 
>
> For some userspace applications information on the number of
> over-current conditions at specific USB hub ports is relevant. Therefore
> introduce a oc_counter in the usb port struct which is exported via
> sysfs.

relevant how? What can the application do with that knowledge?

> Signed-off-by: Richard Leitner 
> ---
> Tested on an i.MX6DL based board.
> ---
>  drivers/usb/core/hub.c  |  4 +++-
>  drivers/usb/core/hub.h  |  1 +
>  drivers/usb/core/port.c | 10 ++
>  3 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index c5c1f6cf3228..448fba1e1827 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -5104,8 +5104,10 @@ static void port_event(struct usb_hub *hub, int port1)
>  
>   if (portchange & USB_PORT_STAT_C_OVERCURRENT) {
>   u16 status = 0, unused;
> + port_dev->oc_count++;
>  
> - dev_dbg(_dev->dev, "over-current change\n");
> + dev_dbg(_dev->dev, "over-current change #%u\n",
> + port_dev->oc_count);
>   usb_clear_port_feature(hdev, port1,
>   USB_PORT_FEAT_C_OVER_CURRENT);
>   msleep(100);/* Cool down */
> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> index 2a700ccc868c..b5cf567bf9e2 100644
> --- a/drivers/usb/core/hub.h
> +++ b/drivers/usb/core/hub.h
> @@ -100,6 +100,7 @@ struct usb_port {
>   unsigned int is_superspeed:1;
>   unsigned int usb3_lpm_u1_permit:1;
>   unsigned int usb3_lpm_u2_permit:1;
> + unsigned int oc_count;
>  };
>  
>  #define to_usb_port(_dev) \
> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> index 1a01e9ad3804..0bfe410eb8a7 100644
> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c
> @@ -41,6 +41,15 @@ static ssize_t connect_type_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(connect_type);
>  
> +static ssize_t oc_count_show(struct device *dev, struct device_attribute 
> *attr,
> +  char *buf)
> +{
> + struct usb_port *port_dev = to_usb_port(dev);
> +
> + return sprintf(buf, "%u\n", port_dev->oc_count);
> +}
> +static DEVICE_ATTR_RO(oc_count);

I would actually spell this out human readable: over_current_count

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

[PATCH] usb: core: introduce per-port over-current counters

[Richard Leitner]

From: Richard Leitner 

For some userspace applications information on the number of
over-current conditions at specific USB hub ports is relevant. Therefore
introduce a oc_counter in the usb port struct which is exported via
sysfs.

Signed-off-by: Richard Leitner 
---
Tested on an i.MX6DL based board.
---
 drivers/usb/core/hub.c  |  4 +++-
 drivers/usb/core/hub.h  |  1 +
 drivers/usb/core/port.c | 10 ++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index c5c1f6cf3228..448fba1e1827 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -5104,8 +5104,10 @@ static void port_event(struct usb_hub *hub, int port1)
 
if (portchange & USB_PORT_STAT_C_OVERCURRENT) {
u16 status = 0, unused;
+   port_dev->oc_count++;
 
-   dev_dbg(_dev->dev, "over-current change\n");
+   dev_dbg(_dev->dev, "over-current change #%u\n",
+   port_dev->oc_count);
usb_clear_port_feature(hdev, port1,
USB_PORT_FEAT_C_OVER_CURRENT);
msleep(100);/* Cool down */
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 2a700ccc868c..b5cf567bf9e2 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -100,6 +100,7 @@ struct usb_port {
unsigned int is_superspeed:1;
unsigned int usb3_lpm_u1_permit:1;
unsigned int usb3_lpm_u2_permit:1;
+   unsigned int oc_count;
 };
 
 #define to_usb_port(_dev) \
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 1a01e9ad3804..0bfe410eb8a7 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -41,6 +41,15 @@ static ssize_t connect_type_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(connect_type);
 
+static ssize_t oc_count_show(struct device *dev, struct device_attribute *attr,
+char *buf)
+{
+   struct usb_port *port_dev = to_usb_port(dev);
+
+   return sprintf(buf, "%u\n", port_dev->oc_count);
+}
+static DEVICE_ATTR_RO(oc_count);
+
 static ssize_t usb3_lpm_permit_show(struct device *dev,
  struct device_attribute *attr, char *buf)
 {
@@ -109,6 +118,7 @@ static DEVICE_ATTR_RW(usb3_lpm_permit);
 
 static struct attribute *port_dev_attrs[] = {
_attr_connect_type.attr,
+   _attr_oc_count.attr,
NULL,
 };
 
-- 
2.11.0

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

[4.9-stable 8/9] usb: phy: msm add regulator dependency

[Arnd Bergmann]

On linux-4.4 and linux-4.9 we get a warning about an array that is
never initialized when CONFIG_REGULATOR is disabled:

drivers/usb/phy/phy-msm-usb.c: In function 'msm_otg_probe':
drivers/usb/phy/phy-msm-usb.c:1911:14: error: 'regs[0].consumer' may be used 
uninitialized in this function [-Werror=maybe-uninitialized]
  motg->vddcx = regs[0].consumer;
  ^~
drivers/usb/phy/phy-msm-usb.c:1912:14: error: 'regs[1].consumer' may be used 
uninitialized in this function [-Werror=maybe-uninitialized]
  motg->v3p3  = regs[1].consumer;
  ^~
drivers/usb/phy/phy-msm-usb.c:1913:14: error: 'regs[2].consumer' may be used 
uninitialized in this function [-Werror=maybe-uninitialized]
  motg->v1p8  = regs[2].consumer;

This adds a Kconfig dependency for it. In newer kernels, the driver no
longer exists, so this is only needed for stable kernels.

Signed-off-by: Arnd Bergmann 
---
 drivers/usb/phy/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index b9c409a18faa..125cea1c3c8d 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -147,6 +147,7 @@ config USB_MSM_OTG
depends on (USB || USB_GADGET) && (ARCH_QCOM || COMPILE_TEST)
depends on USB_GADGET || !USB_GADGET # if USB_GADGET=m, this can't be 
'y'
depends on RESET_CONTROLLER
+   depends on REGULATOR
depends on EXTCON
select USB_PHY
help
-- 
2.9.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: High CPU load produced by USB (DW2)

[Minas Harutyunyan]

On 2/19/2018 12:51 PM, Marek Vasut wrote:
> On 02/19/2018 09:19 AM, Minas Harutyunyan wrote:
>> On 2/17/2018 12:07 AM, Marek Vasut wrote:
>>> On 02/16/2018 06:59 AM, Minas Harutyunyan wrote:
 On 2/15/2018 5:20 PM, Mirza Krak wrote:
> On 14 February 2018 at 13:07, Minas Harutyunyan
>  wrote:
>> On 2/14/2018 12:57 PM, Mirza Krak wrote:
>>> On 8 February 2018 at 14:53, Minas Harutyunyan
>>>  wrote:
>
> < snip >
>
>>
>> I reviewed your interrupt count log again. About 140,000 interrupts in 2
>> seconds, obviously it's not SOF only interrupts. More probably, its NAK
>> respond interrupts to SSPLIT/CSPLIT transactions. For this case I can
>> recommend you to apply patch from Douglas Anderson: "[PATCH v2] usb:
>> dwc2: host: Don't retry NAKed transactions right away" which already
>> merged to 4.16-rc1.
>>
>> In your setups you see different behavior on different HUBs. Your HUBs
>> have different "TT think time": 8 and 32. In USB2.0 spec "TT think time"
>> described as follow "TT requires at most 8/32 FS bit times of inter
>> transaction gap on a full-/low-speed downstream bus". So, your "worst"
>> HUB with "TT think time"=8 sending more frequently SSPLIT/CSPLIT
>> transactions which replied by NAK. As result you see about 4 time more
>> interrupts comparing to "good" HUB. Could you please check interrupts
>> count for "good" HUB and check "4 time" hypothesis.
>
> Did some further testing. The "good" HUB is actually as bad as the
> "bad" HUB, and it was my setup that caused the different behavior. Did
> not use the same devices etc.
>
> Once I made sure that the configuration and setup was the same on both
> board I could see that the behaved similarly. And that is the
> following interrupt load:
>
> - BT USB (FS) = ~80k interrupts / second
> - Keyboard (FS) = ~80k interrupts / second
> - WiFI USB (HS) = ~8k interrupts / second
>
> After applying the suggested patch [1], it is steady around 8k
> interrupts / second no matter what device I connect (HS/FS,
> HUB/NO-HUB). Which is acceptable and usable.

 Great!
>>>
>>> So 8k IRQs per second is what I should expect from this HW with HS
>>> device attached, that's normal and cannot be reduced ?
>>>
>> If core acting as Host in Buffer DMA mode then 8k IRQs per second (SOF
>> interrupts) is expected if connected device(s) has periodic endpoint.
> 
> Is there a way to reduce that or is that the absolute minimum in HS mode?
> 
We already discussed, in this email thread earlier, why SOF interrupts 
required and unmasked.
Only in case when connected device with CTRL+BLK EP's only (like flash 
drive) and directly connected to cores root HUB, SOF's will be masked.
Thanks,
Minas

--
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 RFC] ehci-omap: simple suspend implementation

[Roger Quadros]

Andreas,

On 16/02/18 20:35, Andreas Kemnade wrote:
> On Fri, 16 Feb 2018 13:13:11 -0500 (EST)
> Alan Stern  wrote:
> 
>> On Fri, 16 Feb 2018, Andreas Kemnade wrote:
>>
>>> This powers down the phy and on a gta04 it reduces
>>> suspend current by 13 mA.
>>> For unknown reasons usb does not power on properly.
>>> Also calling usb_phy_shutdown() here feels wrong
>>> apparently the reset line has to be activated.
>>> usb_phy_set_suspend is not enough here. The power
>>> consumption still stays approximately the same as
>>> without any patch.
>>>
>>> With a device connected the device does not enumerate
>>> after resume. A rmmod ehci-omap ; modprobe ehci-omap
>>> does not make it reenumerade.
>>> So there is still something wrong here.
>>>
>>> Signed-off-by: Andreas Kemnade 
>>> ---
>>>  drivers/usb/host/ehci-omap.c | 59 
>>> ++--
>>>  1 file changed, 57 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
>>> index 8d8bafc70c1f..0be2ccf8182a 100644
>>> --- a/drivers/usb/host/ehci-omap.c
>>> +++ b/drivers/usb/host/ehci-omap.c
>>> @@ -266,6 +266,58 @@ static int ehci_hcd_omap_remove(struct platform_device 
>>> *pdev)
>>> return 0;
>>>  }
>>>  
>>> +
>>> +static int __maybe_unused ehci_omap_suspend(struct device *dev)
>>> +{
>>> +   struct usb_hcd *hcd = dev_get_drvdata(dev);
>>> +   struct omap_hcd *omap = (struct omap_hcd *)hcd_to_ehci(hcd)->priv;
>>> +   int ret;
>>> +   int i;
>>> +
>>> +   ret = ehci_suspend(hcd, false);
>>> +   if (ret) {
>>> +   dev_err(dev, "ehci suspend failed: %d\n", ret);
>>> +   return ret;
>>> +   }
>>> +   for (i = 0; i < omap->nports; i++) {
>>> +   if (omap->phy[i])
>>> +   usb_phy_shutdown(omap->phy[i]);
>>> +   }
>>> +   pm_runtime_put_sync(dev);  
>>
>> Why do you include a runtime PM call here, given that the driver
>> doesn't support runtime suspend or resume?
>>
> Well, the parent (drivers/mfd/omap-usb-host.c) has, and there are runtime PM
> calls here in the _probe/_remove functions, so it seems to be sane to do it
> here, too.
> 
> 
>>> +
>>> +   return 0;
>>> +}
>>> +
>>> +static int __maybe_unused ehci_omap_resume(struct device *dev)
>>> +{
>>> +   struct usb_hcd *hcd = dev_get_drvdata(dev);
>>> +   struct omap_hcd *omap = (struct omap_hcd *)hcd_to_ehci(hcd)->priv;
>>> +   int i;
>>> +
>>> +   pm_runtime_get_sync(dev);
>>> +   /*
>>> +* An undocumented "feature" in the OMAP3 EHCI controller,
>>> +* causes suspended ports to be taken out of suspend when
>>> +* the USBCMD.Run/Stop bit is cleared (for example when
>>> +* we do ehci_bus_suspend).
>>> +* This breaks suspend-resume if the root-hub is allowed
>>> +* to suspend. Writing 1 to this undocumented register bit
>>> +* disables this feature and restores normal behavior.
>>> +*/
>>> +   ehci_write(hcd->regs, EHCI_INSNREG04,
>>> +  EHCI_INSNREG04_DISABLE_UNSUSPEND);  
>>
>> Doesn't this code belong in ehci_hcd_omap_probe()?  I assume you only
>> need to set this undocumented bit once, not every time the controller
>> is suspended.  And in any case, according to the comment, you would
>> need to take care of this before the root hub is suspended -- by the
>> time the controller is suspended, it is already too late.
>>
>  
> I am not really sure about this one. It is set in ehci_hcd_omap_probe()
> so it is set before suspend. I set it here to ensure it is re-set when
> the controller is powered down too much to keep register contents. I
> do not know if that is the case. But I thought it would not harm.
> 

If the Hardware SAR (Save and restore) functionality is enabled then
everything will be restored by hardware after a sleep to wake transition.

But you will need this patch to enable SAR for the USB power domain.
https://lkml.org/lkml/2013/7/10/356

Missing this might be the reason why things break for you after a system
suspend/resume.

-- 
cheers,
-roger

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
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 04/12] usb: common: Small class for USB role switches

[Hans de Goede]

Hi,

On 16-02-18 15:22, Heikki Krogerus wrote:

On Fri, Feb 16, 2018 at 04:07:59PM +0200, Andy Shevchenko wrote:

On Fri, Feb 16, 2018 at 12:47 PM, Hans de Goede  wrote:


USB role switch is a device that can be used to choose the
data role for USB connector. With dual-role capable USB
controllers, the controller itself will be the switch, but
on some platforms the USB host and device controllers are
separate IPs and there is a mux between them and the
connector. On those platforms the mux driver will need to
register the switch.

With USB Type-C connectors, the host-to-device relationship
is negotiated over the Configuration Channel (CC). That
means the USB Type-C drivers need to be in control of the
role switch. The class provides a simple API for the USB
Type-C drivers for the control.

For other types of USB connectors (mainly microAB) the class
provides user space control via sysfs attribute file that
can be used to request role swapping from the switch.



+static int __switch_match(struct device *dev, const void *name)


bool?


No, this is callback for class_find_device(). It takes int so int it
is.


+{
+   return !strcmp((const char *)name, dev_name(dev));
+}




+static ssize_t role_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+   struct usb_role_switch *sw = to_role_switch(dev);
+   int ret;
+
+   ret = sysfs_match_string(usb_roles, buf);
+   if (ret < 0) {
+   bool res;
+
+   /* Extra check if the user wants to disable the switch */
+   ret = kstrtobool(buf, );
+   if (ret || res)
+   return -EINVAL;
+   }
+



+   ret = usb_role_switch_set_role(sw, ret);
+   if (!ret)
+   ret = size;
+
+   return ret;


Perhaps

ret = ...
if (ret)
  return ret;

return size;


Sure. Hans, can you clean-up this as well?


Yes I can, not sure when exactly I will get around to this, but I will
try to get out a v2 addressing all comments made this week.

And thank you for your reviews.

Andy, thank you for all the reviews too.

Regards,

Hans




+}



+struct usb_role_switch *
+usb_role_switch_register(struct device *parent,
+const struct usb_role_switch_desc *desc)
+{
+   struct usb_role_switch *sw;
+   int ret;
+
+   if (!desc || !desc->set)
+   return ERR_PTR(-EINVAL);
+
+   sw = kzalloc(sizeof(*sw), GFP_KERNEL);
+   if (!sw)
+   return ERR_PTR(-ENOMEM);



+   ret = device_register(>dev);
+   if (ret) {
+   put_device(>dev);


Memory leak?


No. Check usb_role_switch_release().


Thanks,


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

Re: High CPU load produced by USB (DW2)

[Marek Vasut]

On 02/19/2018 09:19 AM, Minas Harutyunyan wrote:
> On 2/17/2018 12:07 AM, Marek Vasut wrote:
>> On 02/16/2018 06:59 AM, Minas Harutyunyan wrote:
>>> On 2/15/2018 5:20 PM, Mirza Krak wrote:
 On 14 February 2018 at 13:07, Minas Harutyunyan
  wrote:
> On 2/14/2018 12:57 PM, Mirza Krak wrote:
>> On 8 February 2018 at 14:53, Minas Harutyunyan
>>  wrote:

 < snip >

>
> I reviewed your interrupt count log again. About 140,000 interrupts in 2
> seconds, obviously it's not SOF only interrupts. More probably, its NAK
> respond interrupts to SSPLIT/CSPLIT transactions. For this case I can
> recommend you to apply patch from Douglas Anderson: "[PATCH v2] usb:
> dwc2: host: Don't retry NAKed transactions right away" which already
> merged to 4.16-rc1.
>
> In your setups you see different behavior on different HUBs. Your HUBs
> have different "TT think time": 8 and 32. In USB2.0 spec "TT think time"
> described as follow "TT requires at most 8/32 FS bit times of inter
> transaction gap on a full-/low-speed downstream bus". So, your "worst"
> HUB with "TT think time"=8 sending more frequently SSPLIT/CSPLIT
> transactions which replied by NAK. As result you see about 4 time more
> interrupts comparing to "good" HUB. Could you please check interrupts
> count for "good" HUB and check "4 time" hypothesis.

 Did some further testing. The "good" HUB is actually as bad as the
 "bad" HUB, and it was my setup that caused the different behavior. Did
 not use the same devices etc.

 Once I made sure that the configuration and setup was the same on both
 board I could see that the behaved similarly. And that is the
 following interrupt load:

 - BT USB (FS) = ~80k interrupts / second
 - Keyboard (FS) = ~80k interrupts / second
 - WiFI USB (HS) = ~8k interrupts / second

 After applying the suggested patch [1], it is steady around 8k
 interrupts / second no matter what device I connect (HS/FS,
 HUB/NO-HUB). Which is acceptable and usable.
>>>
>>> Great!
>>
>> So 8k IRQs per second is what I should expect from this HW with HS
>> device attached, that's normal and cannot be reduced ?
>>
> If core acting as Host in Buffer DMA mode then 8k IRQs per second (SOF 
> interrupts) is expected if connected device(s) has periodic endpoint.

Is there a way to reduce that or is that the absolute minimum in HS mode?

-- 
Best regards,
Marek Vasut
--
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: High CPU load produced by USB (DW2)

[Minas Harutyunyan]

On 2/17/2018 12:07 AM, Marek Vasut wrote:
> On 02/16/2018 06:59 AM, Minas Harutyunyan wrote:
>> On 2/15/2018 5:20 PM, Mirza Krak wrote:
>>> On 14 February 2018 at 13:07, Minas Harutyunyan
>>>  wrote:
 On 2/14/2018 12:57 PM, Mirza Krak wrote:
> On 8 February 2018 at 14:53, Minas Harutyunyan
>  wrote:
>>>
>>> < snip >
>>>

 I reviewed your interrupt count log again. About 140,000 interrupts in 2
 seconds, obviously it's not SOF only interrupts. More probably, its NAK
 respond interrupts to SSPLIT/CSPLIT transactions. For this case I can
 recommend you to apply patch from Douglas Anderson: "[PATCH v2] usb:
 dwc2: host: Don't retry NAKed transactions right away" which already
 merged to 4.16-rc1.

 In your setups you see different behavior on different HUBs. Your HUBs
 have different "TT think time": 8 and 32. In USB2.0 spec "TT think time"
 described as follow "TT requires at most 8/32 FS bit times of inter
 transaction gap on a full-/low-speed downstream bus". So, your "worst"
 HUB with "TT think time"=8 sending more frequently SSPLIT/CSPLIT
 transactions which replied by NAK. As result you see about 4 time more
 interrupts comparing to "good" HUB. Could you please check interrupts
 count for "good" HUB and check "4 time" hypothesis.
>>>
>>> Did some further testing. The "good" HUB is actually as bad as the
>>> "bad" HUB, and it was my setup that caused the different behavior. Did
>>> not use the same devices etc.
>>>
>>> Once I made sure that the configuration and setup was the same on both
>>> board I could see that the behaved similarly. And that is the
>>> following interrupt load:
>>>
>>> - BT USB (FS) = ~80k interrupts / second
>>> - Keyboard (FS) = ~80k interrupts / second
>>> - WiFI USB (HS) = ~8k interrupts / second
>>>
>>> After applying the suggested patch [1], it is steady around 8k
>>> interrupts / second no matter what device I connect (HS/FS,
>>> HUB/NO-HUB). Which is acceptable and usable.
>>
>> Great!
> 
> So 8k IRQs per second is what I should expect from this HW with HS
> device attached, that's normal and cannot be reduced ?
> 
If core acting as Host in Buffer DMA mode then 8k IRQs per second (SOF 
interrupts) is expected if connected device(s) has periodic endpoint.

Thanks,
Minas

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