Re: [PATCH v8] xhci : AMD Promontory USB disable port support

2017-12-01 Thread kbuild test robot
Hi Joe,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v4.14]
[also build test WARNING on next-20171201]
[cannot apply to usb/usb-testing v4.15-rc1]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Joe-Lee/xhci-AMD-Promontory-USB-disable-port-support/20171202-122325


coccinelle warnings: (new ones prefixed by >>)

>> drivers/usb/host/pci-quirks.c:541:9-10: WARNING: return of 0/1 in function 
>> 'usb_amd_pt_check_port' with return type bool

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
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] xhci : fix boolreturn.cocci warnings

2017-12-01 Thread kbuild test robot
drivers/usb/host/pci-quirks.c:541:9-10: WARNING: return of 0/1 in function 
'usb_amd_pt_check_port' with return type bool

 Return statements in functions returning bool should use
 true/false instead of 1/0.
Generated by: scripts/coccinelle/misc/boolreturn.cocci

Fixes: 5462b4b541a4 ("xhci : AMD Promontory USB disable port support")
CC: Joe Lee 
Signed-off-by: Fengguang Wu 
---

 pci-quirks.c |   32 
 1 file changed, 16 insertions(+), 16 deletions(-)

--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -538,25 +538,25 @@ bool usb_amd_pt_check_port(struct device
 
pci_read_config_byte(pdev, PT_READ_INDX, &value);
if (value != PT_SIG_1_DATA)
-   return 0;
+   return false;
 
pci_write_config_word(pdev, PT_ADDR_INDX, PT_SIG_2_ADDR);
 
pci_read_config_byte(pdev, PT_READ_INDX, &value);
if (value != PT_SIG_2_DATA)
-   return 0;
+   return false;
 
pci_write_config_word(pdev, PT_ADDR_INDX, PT_SIG_3_ADDR);
 
pci_read_config_byte(pdev, PT_READ_INDX, &value);
if (value != PT_SIG_3_DATA)
-   return 0;
+   return false;
 
pci_write_config_word(pdev, PT_ADDR_INDX, PT_SIG_4_ADDR);
 
pci_read_config_byte(pdev, PT_READ_INDX, &value);
if (value != PT_SIG_4_DATA)
-   return 0;
+   return false;
 
if ((pdev->device == 0x43b9) || (pdev->device == 0x43ba)) {
/* device is AMD_PROMONTORYA_4(0x43b9) or
@@ -572,18 +572,18 @@ bool usb_amd_pt_check_port(struct device
 
pci_read_config_byte(pdev, PT_READ_INDX, &value);
if (value & (1<<(port - 7)))
-   return 0;
+   return false;
else
-   return 1;
+   return true;
} else {
pci_write_config_word(pdev, PT_ADDR_INDX,
PT4_P1_REG);
 
pci_read_config_byte(pdev, PT_READ_INDX, &value);
if (value & (1<<(port + 1)))
-   return 0;
+   return false;
else
-   return 1;
+   return true;
}
} else if (pdev->device == 0x43bb) {
/* device is AMD_PROMONTORYA_2(0x43bb)
@@ -597,17 +597,17 @@ bool usb_amd_pt_check_port(struct device
 
pci_read_config_byte(pdev, PT_READ_INDX, &value);
if (value & (1<<(port - 3)))
-   return 0;
+   return false;
else
-   return 1;
+   return true;
} else {
pci_write_config_word(pdev, PT_ADDR_INDX, PT2_P1_REG);
 
pci_read_config_byte(pdev, PT_READ_INDX, &value);
if (value & (1<<(port + 5)))
-   return 0;
+   return false;
else
-   return 1;
+   return true;
}
} else {
/* device is AMD_PROMONTORYA_1(0x43bc)
@@ -621,18 +621,18 @@ bool usb_amd_pt_check_port(struct device
 
pci_read_config_byte(pdev, PT_READ_INDX, &value);
if (value & (1<<(port - 4)))
-   return 0;
+   return false;
else
-   return 1;
+   return true;
 
} else {
pci_write_config_word(pdev, PT_ADDR_INDX, PT1_P1_REG);
 
pci_read_config_byte(pdev, PT_READ_INDX, &value);
if (value & (1<<(port + 4)))
-   return 0;
+   return false;
else
-   return 1;
+   return true;
 
}
}
--
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: xhci: allow imod-interval to be configurable

2017-12-01 Thread Adam Wallis
Hi Chunfeng,

On 11/30/2017 10:32 PM, Chunfeng Yun wrote:
> Hi,
> On Wed, 2017-11-29 at 10:52 -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,
[..]
>>  #include "xhci-mvebu.h"
>>  #include "xhci-rcar.h"
>> +#include "xhci-mtk.h"
> Needn't it, MTK makes use of xhci-mtk.c but not xhci-plat.c
> 

Thanks - I made this change in V3.

[..]
>>  
>>  static struct hc_driver __read_mostly xhci_plat_hc_driver;
>>  
>> @@ -269,6 +270,18 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>  if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
>>  xhci->quirks |= XHCI_BROKEN_PORT_PED;
>>  
>> +/*
>> + * imod_interval is the interrupt modulation value in nanoseconds.
>> + * The increment interval is 8 times as much as that defined in
>> + * the xHCI spec on MTK's controller. This quirk check exists to provide
>> + * backwards compatibility, however, this should be pushed into
>> + * the device tree files at some point in the future and
>> + * checking the quirk should be removed from xhci_plat_probe.
>> + */
>> +xhci->imod_interval = xhci->quirks & XHCI_MTK_HOST ? 5000 : 4;
> Can be moved into xhci-mtk.c for MTK quirk, like as following:
> 

I moved this into V3 as suggested - could you test on an MTK platform (I don't
have one available) to see if it performs as expected?




-- 
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 v3] usb: xhci: allow imod-interval to be configurable

2017-12-01 Thread 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.

Signed-off-by: Adam Wallis 
---
changes from v2:
  * Added PCI default value [Mathias]
  * Removed xhci-mtk.h from xhci-plat.c [Chunfeng Yun]
  * Removed MTK quirk from xhci-plat and moved logic to xhci-mtk [Chunfeng]
  * Updated bindings Documentation to use proper units [Rob Herring]
  * Added imod-interval description and example to MTK binding documentation
changes from v1:
  * Removed device_property_read_u32() per suggestion from greg k-h
  * Used ER_IRQ_INTERVAL_MASK in place of (u16) cast

 Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt | 2 ++
 Documentation/devicetree/bindings/usb/usb-xhci.txt  | 1 +
 drivers/usb/host/xhci-mtk.c | 9 +
 drivers/usb/host/xhci-pci.c | 3 +++
 drivers/usb/host/xhci-plat.c| 4 
 drivers/usb/host/xhci.c | 7 ++-
 drivers/usb/host/xhci.h | 2 ++
 7 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt 
b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
index 3059596..45bbf18 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
@@ -46,6 +46,7 @@ Optional properties:
  - pinctrl-names : a pinctrl state named "default" must be defined
  - pinctrl-0 : pin control group
See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+ - imod-interval: Default interval is 5000ns
 
 Example:
 usb30: usb@1127 {
@@ -66,6 +67,7 @@ usb30: usb@1127 {
usb3-lpm-capable;
mediatek,syscon-wakeup = <&pericfg>;
mediatek,wakeup-src = <1>;
+   imod-interval = <1>;
 };
 
 2nd: dual-role mode with xHCI driver
diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt 
b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index ae6e484..89b68f1 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: Default interval is 4ns
 
 Example:
usb@f0931000 {
diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index b62a1d2..278ea3b 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -674,6 +674,15 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 
xhci = hcd_to_xhci(hcd);
xhci->main_hcd = hcd;
+
+   /*
+* imod_interval is the interrupt modulation value in nanoseconds.
+* The increment interval is 8 times as much as that defined in
+* the xHCI spec on MTK's controller.
+*/
+   xhci->imod_interval = 5000;
+   device_property_read_u32(dev, "imod-interval", &xhci->imod_interval);
+
xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
dev_name(dev), hcd);
if (!xhci->shared_hcd) {
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 7ef1274..efbe57b 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -234,6 +234,9 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
if (!xhci->sbrn)
pci_read_config_byte(pdev, XHCI_SBRN_OFFSET, &xhci->sbrn);
 
+   /* imod_interval is the interrupt modulation value in nanoseconds. */
+   xhci->imod_interval = 4;
+
retval = xhci_gen_setup(hcd, xhci_pci_quirks);
if (retval)
return retval;
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 09f164f..b78be87 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -269,6 +269,10 @@ static

Re: [PATCH 2/2] usb: xhci: fix panic in xhci_free_virt_devices_depth_first

2017-12-01 Thread Greg Kroah-Hartman
On Fri, Dec 01, 2017 at 06:38:16AM -0800, Guenter Roeck wrote:
> On Fri, Dec 1, 2017 at 3:41 AM, Mathias Nyman
>  wrote:
> > From: Yu Chen 
> >
> > Check vdev->real_port 0 to avoid panic
> > [9.261347] [] 
> > xhci_free_virt_devices_depth_first+0x58/0x108
> > [9.261352] [] xhci_mem_cleanup+0x1bc/0x570
> > [9.261355] [] xhci_stop+0x140/0x1c8
> > [9.261365] [] usb_remove_hcd+0xfc/0x1d0
> > [9.261369] [] xhci_plat_remove+0x6c/0xa8
> > [9.261377] [] platform_drv_remove+0x2c/0x70
> > [9.261384] [] __device_release_driver+0x80/0x108
> > [9.261387] [] device_release_driver+0x2c/0x40
> > [9.261392] [] bus_remove_device+0xe0/0x120
> > [9.261396] [] device_del+0x114/0x210
> > [9.261399] [] platform_device_del+0x30/0xa0
> > [9.261403] [] dwc3_otg_work+0x204/0x488
> > [9.261407] [] event_work+0x304/0x5b8
> > [9.261414] [] process_one_work+0x148/0x490
> > [9.261417] [] worker_thread+0x50/0x4a0
> > [9.261421] [] kthread+0xe8/0x100
> > [9.261427] [] ret_from_fork+0x10/0x50
> >
> > The problem can occur if xhci_plat_remove() is called shortly after
> > xhci_plat_probe(). While xhci_free_virt_devices_depth_first been
> > called before the device has been setup and get real_port initialized.
> > The problem occurred on Hikey960 and was reproduced by Guenter Roeck
> > on Kevin with chromeos-4.4.
> >
> > Cc: Guenter Roeck 
> > Cc:  # v4.10+
> 
> Fixes: ee8665e28e8d ("xhci: free xhci virtual devices with leaf nodes first")
> 
> would probably be better. That patch is in v4.4.y as well as in
> v4.9.y, and thus both releases are affected.

I've added this fixes line, 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 v8] xhci : AMD Promontory USB disable port support

2017-12-01 Thread Greg KH

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?


http://daringfireball.net/2007/07/on_top

On Fri, Dec 01, 2017 at 06:45:11PM +0800, Joe Lee wrote:
> Dear Greg:
> I need to re-upload PATCH V9?
> or use the attachment?

I have no idea what you are referring to here, what exactly are you
trying to do?

kernel patches are not attachments...

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 2/2] usb: xhci: fix panic in xhci_free_virt_devices_depth_first

2017-12-01 Thread Guenter Roeck
On Fri, Dec 1, 2017 at 3:41 AM, Mathias Nyman
 wrote:
> From: Yu Chen 
>
> Check vdev->real_port 0 to avoid panic
> [9.261347] [] 
> xhci_free_virt_devices_depth_first+0x58/0x108
> [9.261352] [] xhci_mem_cleanup+0x1bc/0x570
> [9.261355] [] xhci_stop+0x140/0x1c8
> [9.261365] [] usb_remove_hcd+0xfc/0x1d0
> [9.261369] [] xhci_plat_remove+0x6c/0xa8
> [9.261377] [] platform_drv_remove+0x2c/0x70
> [9.261384] [] __device_release_driver+0x80/0x108
> [9.261387] [] device_release_driver+0x2c/0x40
> [9.261392] [] bus_remove_device+0xe0/0x120
> [9.261396] [] device_del+0x114/0x210
> [9.261399] [] platform_device_del+0x30/0xa0
> [9.261403] [] dwc3_otg_work+0x204/0x488
> [9.261407] [] event_work+0x304/0x5b8
> [9.261414] [] process_one_work+0x148/0x490
> [9.261417] [] worker_thread+0x50/0x4a0
> [9.261421] [] kthread+0xe8/0x100
> [9.261427] [] ret_from_fork+0x10/0x50
>
> The problem can occur if xhci_plat_remove() is called shortly after
> xhci_plat_probe(). While xhci_free_virt_devices_depth_first been
> called before the device has been setup and get real_port initialized.
> The problem occurred on Hikey960 and was reproduced by Guenter Roeck
> on Kevin with chromeos-4.4.
>
> Cc: Guenter Roeck 
> Cc:  # v4.10+

Fixes: ee8665e28e8d ("xhci: free xhci virtual devices with leaf nodes first")

would probably be better. That patch is in v4.4.y as well as in
v4.9.y, and thus both releases are affected.

Thanks,
Guenter

> Reviewed-by: Guenter Roeck 
> Tested-by: Guenter Roeck 
> Signed-off-by: Fan Ning 
> Signed-off-by: Li Rui 
> Signed-off-by: yangdi 
> Signed-off-by: Yu Chen 
> Signed-off-by: Mathias Nyman 
> ---
>  drivers/usb/host/xhci-mem.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index e1fba46..15f7d42 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -934,6 +934,12 @@ void xhci_free_virt_devices_depth_first(struct xhci_hcd 
> *xhci, int slot_id)
> if (!vdev)
> return;
>
> +   if (vdev->real_port == 0 ||
> +   vdev->real_port > HCS_MAX_PORTS(xhci->hcs_params1)) {
> +   xhci_dbg(xhci, "Bad vdev->real_port.\n");
> +   goto out;
> +   }
> +
> tt_list_head = &(xhci->rh_bw[vdev->real_port - 1].tts);
> list_for_each_entry_safe(tt_info, next, tt_list_head, tt_list) {
> /* is this a hub device that added a tt_info to the tts list 
> */
> @@ -947,6 +953,7 @@ void xhci_free_virt_devices_depth_first(struct xhci_hcd 
> *xhci, int slot_id)
> }
> }
> }
> +out:
> /* we are now at a leaf device */
> xhci_debugfs_remove_slot(xhci, slot_id);
> xhci_free_virt_device(xhci, slot_id);
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] gadget event trace : add request pointer

2017-12-01 Thread Matthieu CASTET
This allows to identify transfer request, if more than one are in queue.

This is allowed by usb_ep_queue : "Any endpoint
(except control endpoints like ep0) may have more than one transfer
request queued; they complete in FIFO order."

For example with adb gadget with function fs :

 <-transport-225   [002] d..147.136641: usb_ep_queue: ep1out: req 
ffc07b93ef10 length 0/24 sgs 0/0 stream 0 zsI status -115 --> 0
 ->transport-224   [000] d..147.153947: usb_ep_queue: ep1in: req 
ffc07c0def10 length 0/24 sgs 0/0 stream 0 zsI status -115 --> 0
  sh-452   [000] d.h247.153984: usb_gadget_giveback_request: 
ep1in: req ffc07c0def10 length 24/24 sgs 0/0 stream 0 zsI status 0 --> 0
  sh-452   [000] d.h.47.154305: usb_gadget_giveback_request: 
ep1out: req ffc07b93ef10 length 24/24 sgs 0/0 stream 0 zsI status 0 --> 0
 <-transport-225   [002] d..147.154363: usb_ep_queue: ep1out: req 
ffc07b93ef10 length 0/21 sgs 0/0 stream 0 zsI status -115 --> 0
  sh-452   [000] d.h.47.154378: usb_gadget_giveback_request: 
ep1out: req ffc07b93ef10 length 21/21 sgs 0/0 stream 0 zsI status 0 --> 0
 <-transport-225   [002] d..147.154463: usb_ep_queue: ep1out: req 
ffc07b93ef10 length 0/24 sgs 0/0 stream 0 zsI status -115 --> 0
 ->transport-224   [000] d..147.154583: usb_ep_queue: ep1in: req 
ffc07c0def10 length 0/24 sgs 0/0 stream 0 zsI status -115 --> 0
  sh-452   [000] d.h247.154600: usb_gadget_giveback_request: 
ep1in: req ffc07c0def10 length 24/24 sgs 0/0 stream 0 zsI status 0 --> 0
 ->transport-224   [000] d..147.164863: usb_ep_queue: ep1in: req 
ffc07c0def10 length 0/24 sgs 0/0 stream 0 zsI status -115 --> 0
 ->transport-224   [000] d.h147.164887: usb_gadget_giveback_request: 
ep1in: req ffc07c0def10 length 24/24 sgs 0/0 stream 0 zsI status 0 --> 0
 ->transport-224   [000] d..147.164907: usb_ep_queue: ep1in: req 
ffc07c0def10 length 0/122 sgs 0/0 stream 0 zsI status -115 --> 0

Signed-off-by: Matthieu CASTET 
---
 drivers/usb/gadget/udc/trace.h | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/trace.h b/drivers/usb/gadget/udc/trace.h
index f07ddb3f4bb9..2d1f68b5ea76 100644
--- a/drivers/usb/gadget/udc/trace.h
+++ b/drivers/usb/gadget/udc/trace.h
@@ -225,6 +225,7 @@ DECLARE_EVENT_CLASS(udc_log_req,
__field(unsigned, short_not_ok)
__field(int, status)
__field(int, ret)
+   __field(struct usb_request *, req)
),
TP_fast_assign(
snprintf(__get_str(name), UDC_TRACE_STR_MAX, "%s", ep->name);
@@ -238,9 +239,10 @@ DECLARE_EVENT_CLASS(udc_log_req,
__entry->short_not_ok = req->short_not_ok;
__entry->status = req->status;
__entry->ret = ret;
+   __entry->req = req;
),
-   TP_printk("%s: length %d/%d sgs %d/%d stream %d %s%s%s status %d --> 
%d",
-   __get_str(name), __entry->actual, __entry->length,
+   TP_printk("%s: req %p length %d/%d sgs %d/%d stream %d %s%s%s status %d 
--> %d",
+   __get_str(name),__entry->req,  __entry->actual, __entry->length,
__entry->num_mapped_sgs, __entry->num_sgs, __entry->stream_id,
__entry->zero ? "Z" : "z",
__entry->short_not_ok ? "S" : "s",
-- 
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: [PATCH] gadget event trace : add request pointer

2017-12-01 Thread Felipe Balbi

Hi,

Matthieu CASTET  writes:
> Hi Felipe,
>
> Le Fri, 1 Dec 2017 12:49:07 +0200,
> Felipe Balbi  a écrit :
>
>> Hi,
>> 
>> 
>> This is, indeed, really useful. However do you mind removing the [] and
>> adding a space so that output looks like:
>> 
>> [00:59:36.784082]  ->transport-222   [002] d..1   112.602331: 
>> usb_ep_queue: ep1in: req ffc07c234710 length 0/7 sgs 0/0 stream 0 zsI 
>> status -115 --> 0
>> 
> Ok, I will update the patch with these modifications.
>
> For information I took the "[%p]" from the mmc layer.

oh, okay. Thanks for the info

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH] gadget event trace : add request pointer

2017-12-01 Thread Matthieu CASTET
Hi Felipe,

Le Fri, 1 Dec 2017 12:49:07 +0200,
Felipe Balbi  a écrit :

> Hi,
> 
> 
> This is, indeed, really useful. However do you mind removing the [] and
> adding a space so that output looks like:
> 
> [00:59:36.784082]  ->transport-222   [002] d..1   112.602331: 
> usb_ep_queue: ep1in: req ffc07c234710 length 0/7 sgs 0/0 stream 0 zsI 
> status -115 --> 0
> 
Ok, I will update the patch with these modifications.

For information I took the "[%p]" from the mmc layer.

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


[PATCH v2] HID: add quirk for another PIXART OEM mouse used by HP

2017-12-01 Thread Dave Young
This mouse keep disconnecting in runleve 3 like below, add it needs the
quirk to mute the anoying messages.

[  111.230555] usb 2-2: USB disconnect, device number 6
[  112.718156] usb 2-2: new low-speed USB device number 7 using xhci_hcd
[  112.941594] usb 2-2: New USB device found, idVendor=03f0, idProduct=094a
[  112.984866] usb 2-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[  113.027731] usb 2-2: Product: HP USB Optical Mouse
[  113.069977] usb 2-2: Manufacturer: PixArt
[  113.113500] input: PixArt HP USB Optical Mouse as 
/devices/pci:00/:00:14.0/usb2/2-2/2-2:1.0/0003:03F0:094A.0002/input/input14
[  113.156787] hid-generic 0003:03F0:094A.0002: input: USB HID v1.11 Mouse 
[PixArt HP USB Optical Mouse] on usb-:00:14.0-2/input0
[  173.262642] usb 2-2: USB disconnect, device number 7
[  174.750244] usb 2-2: new low-speed USB device number 8 using xhci_hcd
[  174.935740] usb 2-2: New USB device found, idVendor=03f0, idProduct=094a
[  174.990435] usb 2-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[  175.014984] usb 2-2: Product: HP USB Optical Mouse
[  175.037886] usb 2-2: Manufacturer: PixArt
[  175.061794] input: PixArt HP USB Optical Mouse as 
/devices/pci:00/:00:14.0/usb2/2-2/2-2:1.0/0003:03F0:094A.0003/input/input15
[  175.084946] hid-generic 0003:03F0:094A.0003: input: USB HID v1.11 Mouse 
[PixArt HP USB Optical Mouse] on usb-:00:14.0-2/input0

Signed-off-by: Dave Young 
Cc: sta...@vger.kernel.org
---
v1->v2: rebase to usb tree hid-quirks-cleanup branch
 drivers/hid/hid-ids.h|1 +
 drivers/hid/hid-quirks.c |1 +
 2 files changed, 2 insertions(+)

--- linux-x86.orig/drivers/hid/hid-ids.h
+++ linux-x86/drivers/hid/hid-ids.h
@@ -535,6 +535,7 @@
 #define USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0A4A  0x0a4a
 #define USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0B4A  0x0b4a
 #define USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE 0x134a
+#define USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_094A0x094a
 
 #define USB_VENDOR_ID_HUION0x256c
 #define USB_DEVICE_ID_HUION_TABLET 0x006e
--- linux-x86.orig/drivers/hid/hid-quirks.c
+++ linux-x86/drivers/hid/hid-quirks.c
@@ -90,6 +90,7 @@ const struct hid_device_id hid_quirks[]
{ HID_USB_DEVICE(USB_VENDOR_ID_HP, 
USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0A4A), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_HP, 
USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0B4A), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_HP, 
USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE), HID_QUIRK_ALWAYS_POLL },
+   { HID_USB_DEVICE(USB_VENDOR_ID_HP, 
USB_PRODUCT_ID_HP_PIXART_OEM_USB_OPTICAL_MOUSE_094A), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_IDEACOM, USB_DEVICE_ID_IDEACOM_IDC6680), 
HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_INNOMEDIA, 
USB_DEVICE_ID_INNEX_GENESIS_ATARI), HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M610X), 
HID_QUIRK_MULTI_INPUT },
--
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()

2017-12-01 Thread Yoshihiro Shimoda
Hi again,

> From: Yoshihiro Shimoda, Sent: Friday, December 1, 2017 8:04 PM
> 
> Hi,
> 

> However, the following message still exists.
> 
>Enabling runtime PM for inactive device (ee08.usb) with active children
> 
> So, I guess ohci-platform.c also has similar issue.
> 
> JFYI, the ehci-platform.c doesn't have runtime PM handling.
> So, I think that error message doesn't output from ehci devices.

I have update.
If I added to call pm_runtime_forbid() in ohci-platform.c like xhci-plat.c,
the issue disappeared. I don't understand the pm_runtime_forbid() behavior yet,
but is this acceptable?

Best regards,
Yoshihiro Shimoda

> Best regards,
> Yoshihiro Shimoda



[PATCH 2/2] usb: xhci: fix panic in xhci_free_virt_devices_depth_first

2017-12-01 Thread Mathias Nyman
From: Yu Chen 

Check vdev->real_port 0 to avoid panic
[9.261347] [] 
xhci_free_virt_devices_depth_first+0x58/0x108
[9.261352] [] xhci_mem_cleanup+0x1bc/0x570
[9.261355] [] xhci_stop+0x140/0x1c8
[9.261365] [] usb_remove_hcd+0xfc/0x1d0
[9.261369] [] xhci_plat_remove+0x6c/0xa8
[9.261377] [] platform_drv_remove+0x2c/0x70
[9.261384] [] __device_release_driver+0x80/0x108
[9.261387] [] device_release_driver+0x2c/0x40
[9.261392] [] bus_remove_device+0xe0/0x120
[9.261396] [] device_del+0x114/0x210
[9.261399] [] platform_device_del+0x30/0xa0
[9.261403] [] dwc3_otg_work+0x204/0x488
[9.261407] [] event_work+0x304/0x5b8
[9.261414] [] process_one_work+0x148/0x490
[9.261417] [] worker_thread+0x50/0x4a0
[9.261421] [] kthread+0xe8/0x100
[9.261427] [] ret_from_fork+0x10/0x50

The problem can occur if xhci_plat_remove() is called shortly after
xhci_plat_probe(). While xhci_free_virt_devices_depth_first been
called before the device has been setup and get real_port initialized.
The problem occurred on Hikey960 and was reproduced by Guenter Roeck
on Kevin with chromeos-4.4.

Cc: Guenter Roeck 
Cc:  # v4.10+
Reviewed-by: Guenter Roeck 
Tested-by: Guenter Roeck 
Signed-off-by: Fan Ning 
Signed-off-by: Li Rui 
Signed-off-by: yangdi 
Signed-off-by: Yu Chen 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-mem.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index e1fba46..15f7d42 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -934,6 +934,12 @@ void xhci_free_virt_devices_depth_first(struct xhci_hcd 
*xhci, int slot_id)
if (!vdev)
return;
 
+   if (vdev->real_port == 0 ||
+   vdev->real_port > HCS_MAX_PORTS(xhci->hcs_params1)) {
+   xhci_dbg(xhci, "Bad vdev->real_port.\n");
+   goto out;
+   }
+
tt_list_head = &(xhci->rh_bw[vdev->real_port - 1].tts);
list_for_each_entry_safe(tt_info, next, tt_list_head, tt_list) {
/* is this a hub device that added a tt_info to the tts list */
@@ -947,6 +953,7 @@ void xhci_free_virt_devices_depth_first(struct xhci_hcd 
*xhci, int slot_id)
}
}
}
+out:
/* we are now at a leaf device */
xhci_debugfs_remove_slot(xhci, slot_id);
xhci_free_virt_device(xhci, slot_id);
-- 
2.7.4

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


[PATCH 1/2] xhci: Don't show incorrect WARN message about events for empty rings

2017-12-01 Thread Mathias Nyman
xHC can generate two events for a short transfer if the short TRB and
last TRB in the TD are not the same TRB.

The driver will handle the TD after the first short event, and remove
it from its internal list. Driver then incorrectly prints a warning
for the second event:

"WARN Event TRB for slot x ep y with no TDs queued"

Fix this by not printing a warning if we get a event on a empty list
if the previous event was a short event.

Cc: 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c239c68..6eb87c6 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2477,12 +2477,16 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 */
if (list_empty(&ep_ring->td_list)) {
/*
-* A stopped endpoint may generate an extra completion
-* event if the device was suspended.  Don't print
-* warnings.
+* Don't print wanings if it's due to a stopped endpoint
+* generating an extra completion event if the device
+* was suspended. Or, a event for the last TRB of a
+* short TD we already got a short event for.
+* The short TD is already removed from the TD list.
 */
+
if (!(trb_comp_code == COMP_STOPPED ||
-   trb_comp_code == COMP_STOPPED_LENGTH_INVALID)) {
+ trb_comp_code == COMP_STOPPED_LENGTH_INVALID ||
+ ep_ring->last_td_was_short)) {
xhci_warn(xhci, "WARN Event TRB for slot %d ep 
%d with no TDs queued?\n",

TRB_TO_SLOT_ID(le32_to_cpu(event->flags)),
ep_index);
-- 
2.7.4

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


[PATCH 0/2] xhci fixes for usb-linus

2017-12-01 Thread Mathias Nyman

Hi Greg

A couple xhci fixes for usb-linus, one fixes a panic if driver is
removed shortly after probing, the other a flood of false warning
messages.

-Mathias

Mathias Nyman (1):
  xhci: Don't show incorrect WARN message about events for empty rings

Yu Chen (1):
  usb: xhci: fix panic in xhci_free_virt_devices_depth_first

 drivers/usb/host/xhci-mem.c  |  7 +++
 drivers/usb/host/xhci-ring.c | 12 
 2 files changed, 15 insertions(+), 4 deletions(-)

-- 
2.7.4

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


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

2017-12-01 Thread Yoshihiro Shimoda
Hi,

> From: Ulf Hansson, Sent: Friday, December 1, 2017 6:22 PM
> 
> + Kishon
> 
> On 30 November 2017 at 13:51, Yoshihiro Shimoda
>  wrote:
> > Hi,
> >
> >> From: Ulf Hansson, Sent: Wednesday, November 29, 2017 6:59 PM
> >>
> >> On 29 November 2017 at 10:43, Geert Uytterhoeven  
> >> wrote:
> >> > Hi Ulf,
> > 
> >> Okay, so the problem remains no matter which solution for wakeup you
> >> pick in genpd.
> >
> > Yes. Today I could reproduce this issue without usb host driver.
> > - The renesas_usb3 usb peripheral driver has generic phy handling.
> >   (The peripheral driver uses different generic phy driver 
> > (phy-rcar-gen3-usb3.c) though.)
> >  --> If I used the current renesas_usb3 (this means doesn't call 
> > phy_power_{on,off}(),
> >  the issue didn't happen.
> >  --> If I added phy_power_{on,off}() calling, the issue happened.
> >   --> So, I'm thinking the APIs are related to the issue.
> 
> Yes.
> 
> >
> > - The generic phy APIs are in drivers/phy/phy-core.c.
> >  --> The phy-rcar-gen3-usb[23] drivers call only pm_runtime_enable() before 
> > devm_phy_create().
> >   --> The phy-core will call pm_runtime_{get_sync,put}() in 
> > phy_{init,exit,power_{on,off}}.
> >--> So, IIUC, both devices of phy-. and  will be 
> > handled by runtime PM APIs.
> >  --> The runtime PM implementation of phy-core seems good to me. But...?
> 
> 
> I have digested the information that you and Geert provided, thanks!
> 
> So, my conclusions so far is:
> 
> The phy core is using runtime PM reference counting at
> phy_power_on|off(). Although it does that on the phy core device,
> which is a child device of the phy provider device.
> 
> Because phy_power_off() is called during system suspend from phy
> consumer drivers like usb, the phy core device (child) and the phy
> provider device (parent) will never become runtime suspended (because
> the PM core has invoked pm_runtime_get_no_resume() for all device in
> the device prepare phase).
> 
> Then, when genpd calls pm_runtime_force_suspend() at the suspend noirq
> phase for the phy provider device, the call to
> pm_runtime_set_suspended() in there, triggers the earlier error
> message, which is because the child (phy core device) is still runtime
> resumed.

Thank you very much for the conclusions!
It's helpful to me about runtime PM behavior.

> >> Then this seems to point to that the driver may be misbehaving in some
> >> way. I can help to check what is going on.
> >
> > I guess so. But, I don't find yet...
> 
> I think the below patch will help, although I am not sure if that is
> sufficient as a long term fix.

Thank you very much for your help!
Also, I'm not sure how to fix for a long term kernels though...

> Can you please try and see if it solves the problems?

Sure! I tested your patch, and then the following message disappeared!

   Enabling runtime PM for inactive device (ee080200.usb-phy) with active 
children

However, the following message still exists.

   Enabling runtime PM for inactive device (ee08.usb) with active children

So, I guess ohci-platform.c also has similar issue.

JFYI, the ehci-platform.c doesn't have runtime PM handling.
So, I think that error message doesn't output from ehci devices.

Best regards,
Yoshihiro Shimoda



Re: [PATCH] gadget event trace : add request pointer

2017-12-01 Thread Felipe Balbi

Hi,

Matthieu CASTET  writes:
> This allow to identify transfert request, if more than one are in queue.
   ^ ^
   allowstransfer

> @@ -238,9 +239,10 @@ DECLARE_EVENT_CLASS(udc_log_req,
>   __entry->short_not_ok = req->short_not_ok;
>   __entry->status = req->status;
>   __entry->ret = ret;
> + __entry->req = req;
>   ),
> - TP_printk("%s: length %d/%d sgs %d/%d stream %d %s%s%s status %d --> 
> %d",
> - __get_str(name), __entry->actual, __entry->length,
> + TP_printk("%s: req[%p] length %d/%d sgs %d/%d stream %d %s%s%s status 
> %d --> %d",

This is, indeed, really useful. However do you mind removing the [] and
adding a space so that output looks like:

[00:59:36.784082]  ->transport-222   [002] d..1   112.602331: usb_ep_queue: 
ep1in: req ffc07c234710 length 0/7 sgs 0/0 stream 0 zsI status -115 --> 0

IMHO it's a little easier to read and matches the format we already have
on dwc3 (not that it matters, it'd just be easier to write a parser :-)

cheers

-- 
balbi


signature.asc
Description: PGP signature


[PATCH] gadget event trace : add request pointer

2017-12-01 Thread Matthieu CASTET
This allow to identify transfert request, if more than one are in queue.

This is allowed by usb_ep_queue : "Any endpoint
(except control endpoints like ep0) may have more than one transfer
request queued; they complete in FIFO order."

For example with adb gadget with function fs :

[00:59:36.655980]  <-transport-223   [002] d..1   112.529723: usb_ep_queue: 
ep1out: req[ffc07cbab610] length 0/24 sgs 0/0 stream 0 zsI status -115 --> 0
[00:59:36.672086]   -0 [000] d.h1   112.600725: 
usb_gadget_giveback_request: ep1out: req[ffc07cbab610] length 24/24 sgs 0/0 
stream 0 zsI status 0 --> 0
[00:59:36.688087]  <-transport-223   [002] d..1   112.600878: usb_ep_queue: 
ep1out: req[ffc07cbab610] length 0/6 sgs 0/0 stream 0 zsI status -115 --> 0
[00:59:36.704085]   -0 [000] d.h1   112.600918: 
usb_gadget_giveback_request: ep1out: req[ffc07cbab610] length 6/6 sgs 0/0 
stream 0 zsI status 0 --> 0
[00:59:36.704090]  <-transport-223   [002] d..1   112.600994: usb_ep_queue: 
ep1out: req[ffc07cbab610] length 0/24 sgs 0/0 stream 0 zsI status -115 --> 0
[00:59:36.720080]  ->transport-222   [002] d..1   112.601150: usb_ep_queue: 
ep1in: req[ffc07c234710] length 0/24 sgs 0/0 stream 0 zsI status -115 --> 0
[00:59:36.736087]   -0 [000] d.h1   112.601171: 
usb_gadget_giveback_request: ep1in: req[ffc07c234710] length 24/24 sgs 0/0 
stream 0 zsI status 0 --> 0
[00:59:36.752082]  ->transport-222   [002] d..1   112.602262: usb_ep_queue: 
ep1in: req[ffc07c234710] length 0/24 sgs 0/0 stream 0 zsI status -115 --> 0
[00:59:36.768084]   sh-448   [000] dnh1   112.602300: 
usb_gadget_giveback_request: ep1in: req[ffc07c234710] length 24/24 sgs 0/0 
stream 0 zsI status 0 --> 0
[00:59:36.784082]  ->transport-222   [002] d..1   112.602331: usb_ep_queue: 
ep1in: req[ffc07c234710] length 0/7 sgs 0/0 stream 0 zsI status -115 --> 0
[00:59:36.784088]   ls-452   [000] d.h1   112.602411: 
usb_gadget_giveback_request: ep1in: req[ffc07c234710] length 7/7 sgs 0/0 
stream 0 zsI status 0 --> 0
[00:59:36.800117]   ls-452   [000] d.h1   112.602721: 
usb_gadget_giveback_request: ep1out: req[ffc07cbab610] length 24/24 sgs 0/0 
stream 0 zsI status 0 --> 0

Signed-off-by: Matthieu CASTET 
---
 drivers/usb/gadget/udc/trace.h | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/trace.h b/drivers/usb/gadget/udc/trace.h
index f07ddb3f4bb9..99feb4a07e20 100644
--- a/drivers/usb/gadget/udc/trace.h
+++ b/drivers/usb/gadget/udc/trace.h
@@ -225,6 +225,7 @@ DECLARE_EVENT_CLASS(udc_log_req,
__field(unsigned, short_not_ok)
__field(int, status)
__field(int, ret)
+   __field(struct usb_request *, req)
),
TP_fast_assign(
snprintf(__get_str(name), UDC_TRACE_STR_MAX, "%s", ep->name);
@@ -238,9 +239,10 @@ DECLARE_EVENT_CLASS(udc_log_req,
__entry->short_not_ok = req->short_not_ok;
__entry->status = req->status;
__entry->ret = ret;
+   __entry->req = req;
),
-   TP_printk("%s: length %d/%d sgs %d/%d stream %d %s%s%s status %d --> 
%d",
-   __get_str(name), __entry->actual, __entry->length,
+   TP_printk("%s: req[%p] length %d/%d sgs %d/%d stream %d %s%s%s status 
%d --> %d",
+   __get_str(name),__entry->req,  __entry->actual, __entry->length,
__entry->num_mapped_sgs, __entry->num_sgs, __entry->stream_id,
__entry->zero ? "Z" : "z",
__entry->short_not_ok ? "S" : "s",
-- 
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: [PATCH v8] xhci : AMD Promontory USB disable port support

2017-12-01 Thread Greg KH
On Fri, Dec 01, 2017 at 11:54:20AM +0800, Joe Lee wrote:
> Signed-off-by: Joe Lee 
> ---

I can't take patches without any changelog text at all.  Please fix that
up.

Also always list what the difference between this and the previous
versions below the --- line.  How to do this is documented in the
SubmittingPatches file in the kernel documentation.

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

2017-12-01 Thread Ulf Hansson
+ Kishon

On 30 November 2017 at 13:51, Yoshihiro Shimoda
 wrote:
> Hi,
>
>> From: Ulf Hansson, Sent: Wednesday, November 29, 2017 6:59 PM
>>
>> On 29 November 2017 at 10:43, Geert Uytterhoeven  
>> wrote:
>> > Hi Ulf,
> 
>> Okay, so the problem remains no matter which solution for wakeup you
>> pick in genpd.
>
> Yes. Today I could reproduce this issue without usb host driver.
> - The renesas_usb3 usb peripheral driver has generic phy handling.
>   (The peripheral driver uses different generic phy driver 
> (phy-rcar-gen3-usb3.c) though.)
>  --> If I used the current renesas_usb3 (this means doesn't call 
> phy_power_{on,off}(),
>  the issue didn't happen.
>  --> If I added phy_power_{on,off}() calling, the issue happened.
>   --> So, I'm thinking the APIs are related to the issue.

Yes.

>
> - The generic phy APIs are in drivers/phy/phy-core.c.
>  --> The phy-rcar-gen3-usb[23] drivers call only pm_runtime_enable() before 
> devm_phy_create().
>   --> The phy-core will call pm_runtime_{get_sync,put}() in 
> phy_{init,exit,power_{on,off}}.
>--> So, IIUC, both devices of phy-. and  will be 
> handled by runtime PM APIs.
>  --> The runtime PM implementation of phy-core seems good to me. But...?


I have digested the information that you and Geert provided, thanks!

So, my conclusions so far is:

The phy core is using runtime PM reference counting at
phy_power_on|off(). Although it does that on the phy core device,
which is a child device of the phy provider device.

Because phy_power_off() is called during system suspend from phy
consumer drivers like usb, the phy core device (child) and the phy
provider device (parent) will never become runtime suspended (because
the PM core has invoked pm_runtime_get_no_resume() for all device in
the device prepare phase).

Then, when genpd calls pm_runtime_force_suspend() at the suspend noirq
phase for the phy provider device, the call to
pm_runtime_set_suspended() in there, triggers the earlier error
message, which is because the child (phy core device) is still runtime
resumed.

>
>> Then this seems to point to that the driver may be misbehaving in some
>> way. I can help to check what is going on.
>
> I guess so. But, I don't find yet...

I think the below patch will help, although I am not sure if that is
sufficient as a long term fix.

Can you please try and see if it solves the problems?

From: Ulf Hansson 
Date: Fri, 1 Dec 2017 09:07:54 +0100
Subject: [PATCH] phy: core: Move runtime PM reference counting to be done on
 the parent

This is temporary hack, do not merge!

The runtime PM deployment in the phy core is a bit unnecessary complicated,
due to enabling runtime PM for the phy device. Moreover it causes problems
for parent devices (phy providers) when dealing with system wide PM.

Therefore, move the runtime PM reference counting to be done on the phy's
parent device and drop to enable runtime PM for the phy device altogether.
This simplifies for the phy provider driver, to deal with system wide PM,
in case it also cares about keeping the runtime PM status of the device in
sync with the state of the HW.

Signed-off-by: Ulf Hansson 
---
 drivers/phy/phy-core.c | 35 +--
 1 file changed, 9 insertions(+), 26 deletions(-)

diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index b4964b0..837e50d 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -217,15 +217,12 @@ EXPORT_SYMBOL_GPL(phy_pm_runtime_forbid);

 int phy_init(struct phy *phy)
 {
-   int ret;
+   int ret = 0;

if (!phy)
return 0;

-   ret = phy_pm_runtime_get_sync(phy);
-   if (ret < 0 && ret != -ENOTSUPP)
-   return ret;
-   ret = 0; /* Override possible ret == -ENOTSUPP */
+   pm_runtime_get_sync(phy->dev.parent);

mutex_lock(&phy->mutex);
if (phy->init_count == 0 && phy->ops->init) {
@@ -239,22 +236,19 @@ int phy_init(struct phy *phy)

 out:
mutex_unlock(&phy->mutex);
-   phy_pm_runtime_put(phy);
+   pm_runtime_put(phy->dev.parent);
return ret;
 }
 EXPORT_SYMBOL_GPL(phy_init);

 int phy_exit(struct phy *phy)
 {
-   int ret;
+   int ret = 0;

if (!phy)
return 0;

-   ret = phy_pm_runtime_get_sync(phy);
-   if (ret < 0 && ret != -ENOTSUPP)
-   return ret;
-   ret = 0; /* Override possible ret == -ENOTSUPP */
+   pm_runtime_get_sync(phy->dev.parent);

mutex_lock(&phy->mutex);
if (phy->init_count == 1 && phy->ops->exit) {
@@ -268,7 +262,7 @@ int phy_exit(struct phy *phy)

 out:
mutex_unlock(&phy->mutex);
-   phy_pm_runtime_put(phy);
+   pm_runtime_put(phy->dev.parent);
return ret;
 }
 EXPORT_SYMBOL_GPL(phy_exit);
@@ -286,11 +280,7 @@ int phy_power_on(struct phy *phy)
goto out;
}

-   ret = phy_pm_runtime_get_sync(phy);
-   if (ret < 0 && ret != -ENOTSUPP)
-   goto err_pm_sync;
-
-   ret = 

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

2017-12-01 Thread Hayes Wang
Cameron Seader [mailto:csea...@suse.com]
> Sent: Thursday, November 30, 2017 11:40 PM
[...]
> I have experienced with kernel 4.14 and the driver 1.09.9 that when a
> system goes into sleep mode and then comes out of sleep that the network
> interface gets set to speed 100 in half duplex. I then have to manually
> set it back to full duplex and speed 1000. The 2.09.0 driver fixes this
> problem and the upstream 4.14 has this bug. So there seems to be
> something significantly different in the way the upstream driver handles
> coming out of sleep.

Maybe you could send me the kernel log first.
I would try to reproduce this issue.

Best Regards,
Hayes




Re: [RFC PATCH 0/2] usb: typec: alternate mode bus

2017-12-01 Thread Heikki Krogerus
Hi,

Thanks for taking a look at this..

On Sun, Nov 26, 2017 at 12:23:31PM +0100, Hans de Goede wrote:
> Hi Heiko,
> 
> On 28-09-17 13:35, Heikki Krogerus wrote:
> > Hi guys,
> > 
> > The bus allows SVID specific communication with the partners to be
> > handled in separate drivers for each alternate mode.
> > 
> > Alternate mode handling happens with two separate logical devices:
> > 1. Partner alternate mode devices which represent the alternate modes
> > on the partner. The driver for them will handle the alternate mode
> > specific communication with the partner using VDMs.
> > 2. Port alternate mode devices which represent connections from the
> > USB Type-C port to devices on the platform.
> > 
> > The drivers will be bind to the partner alternate modes. The alternate
> > mode drivers will need to deliver the result of the negotiated pin
> > configurations to the rest of the platform (towards the port alternate
> > mode devices). This series includes API for that, however, not the
> > final implementation yet.
> > 
> > The connections to the other devices on the platform the ports have
> > can be described by using the remote endpoint concept [1][2] on ACPI
> > and DT platforms, but I have no solution for the "platform data" case
> > where we have neither DT nor ACPI to describe the connections for us.
> 
> Sorry about the slow reply, I've been a bit swamped with other stuff,
> but now I would like to get back to this.
> 
> I've been trying to wrap my head around what you're proposing here and
> I see how this can help with implementing display-port alternate mode
> support, but I don't see how it is going to help with regular superspeed
> USB support / the mux problem.
> 
> The problems I see / questions I have are:
> 
> 1) This seems to be driven by having a bus using svid-s as match functions,
> but the standard USB function does not have any svid, or at least currently
> does not show as such under e.g. /sys/class/typec/port0/port0-partner

USB is the "normal" mode, not alt-mode. We don't need any specific
driver for the USB mode. In alternate modes, we have to communicate
with the partner using SVID specific messages, and that is what we
need the drivers for.

> 2) The alt-mode drivers you are suggesting seem to be about 2 things:
> a) Alt-mode specific PD communication
> b) Telling other components about pin-configs, e.g. telling the i915 driver 
> how
> much display port lanes are configured
> 
> What this seems to miss a mechanism to control the mux between the 
> "superspeed"
> data-pairs on the port and the dp-port pins on the SoC / the superspeed USB
> pins on the SoC. Even leaving display-port out of the picture for now we still
> need to control the port -> SoC superspeed pins routing which need to be
> one of: tristated (default) / normal / upside-down routing.

We will need to deliver the orientation to the GPU/DP drivers and I'm
not supporting that yet in this draft. I'm preparing a more complete
version of these, and I'll propose something for the orientation as
well, though it is a little bit out side the scope of this series.
That information comes from the device drivers or tcpm, not the
alternate mode drivers.

The USB MUX question is a separate topic, however, the idea of
describing the connections a Type-C port (or any usb port) has to
other components in ACPI and DT, including the USB mux, should be done
using device graph (remote-endpoints) in the future. For the existing
boards we need to figure out somekind of a lookup method to do the
same, and that is the biggest missing thing we still have IMO.

---
| Type-C port |
---
   |
---
   /  MUX  \
  ---
/  \
   /\
  /  \
   -- -
   |   GPU  |/ USB MUX \
   --   -
  /  \
 /\
/  \
 ----
 | Host controller || Device controller |
 ----

> ### end Type-c discussion ###
> 
> ### Related USB device/host mode switch discussion ###
> 
> Another problem for USB is even once the right pins on the port are routed to
> the right pins on the SoC then the SoC may have an internal mux to route
> all the USB pins (both USB-2 and superspeed) to either the host or device
> USB controller.
> 
> My previous patches for this tried to use the mux framework for this, but that
> was nacked because of a misunderstanding how the current mux framemork works.
> 
> The current mux framework is based on the notion of there being a shared
> bus between e.g. the SoC and various devices, where the devices are not
> directly addressable from the bus, but there is