Re: [PATCH] usb: dwc3: core: power on PHYs before initializing core

2018-03-08 Thread Brian Norris 
Hi,

On Thu, Mar 08, 2018 at 12:43:40PM +0200, Felipe Balbi wrote:
> William Wu  writes:
> > The dwc3_core_init() gets the PHYs and initializes the PHYs with
> > the usb_phy_init() and phy_init() functions before initializing
> > core, and power on the PHYs after core initialization is done.
> >
> > However, some platforms (e.g. Rockchip RK3399 DWC3 with Type-C
> > USB3 PHY), it needs to do some special operation while power on
> > the Type-C PHY before initializing DWC3 core. It's because that
> > the RK3399 Type-C PHY requires to hold the DWC3 controller in
> > reset state to keep the PIPE power state in P2 while configuring
> > the Type-C PHY, otherwise, it may cause waiting for the PIPE ready
> > timeout. In this case, if we power on the PHYs after the DWC3 core
> > initialization is done, the core will be reset to uninitialized
> > state after power on the PHYs.
> >
> > Fix this by powering on the PHYs before initializing core. And
> > because the GUID register may also be reset in this case, so we
> > need to configure the GUID register after powering on the PHYs.
> >
> > Signed-off-by: William Wu 
> 
> does this cause any regressions for your boards?

I'm not Roger, but I believe it was determined we don't need this for
the Rockchip systems for which William was originally sending this. At
least not right now. I believe our PHY init problems were mostly
resolved in other ways.

(Although I hear USB is currently pretty broken around suspend/resume
for us on -next. Likely unrelated.)

I guess we never clearly replied stating the above. I hope this isn't
merged anywhere? Or I guess it's no problem to me at the moment, but it
might be needless churn.

Brian
--
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: dwc3: core: power on PHYs before initializing core

2018-01-17 Thread Brian Norris 
On Fri, Jan 12, 2018 at 12:00:16PM +0800, William Wu wrote:
> The dwc3_core_init() gets the PHYs and initializes the PHYs with
> the usb_phy_init() and phy_init() functions before initializing
> core, and power on the PHYs after core initialization is done.
> 
> However, some platforms (e.g. Rockchip RK3399 DWC3 with Type-C
> USB3 PHY), it needs to do some special operation while power on
> the Type-C PHY before initializing DWC3 core. It's because that
> the RK3399 Type-C PHY requires to hold the DWC3 controller in
> reset state to keep the PIPE power state in P2 while configuring
> the Type-C PHY, otherwise, it may cause waiting for the PIPE ready
> timeout. In this case, if we power on the PHYs after the DWC3 core
> initialization is done, the core will be reset to uninitialized
> state after power on the PHYs.
> 
> Fix this by powering on the PHYs before initializing core. And
> because the GUID register may also be reset in this case, so we
> need to configure the GUID register after powering on the PHYs.
> 
> Signed-off-by: William Wu <william...@rock-chips.com>

This kinda should be part of your series:

[PATCH 0/3] Reset USB3 controller before initializing Type-C PHY on rk3399

or at least mentioned there, because the series there doesn't quite
right otherwise, no?

Anyway, I think this patch looks OK. I don't immediately see good
reasons for delaying the PHY init until later, and I do see reasons why
it could be useful earlier:

Reviewed-by: Brian Norris <briannor...@chromium.org>

> ---
>  drivers/usb/dwc3/core.c | 46 ++
>  1 file changed, 22 insertions(+), 24 deletions(-)
--
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: dwc3: Undo PHY init if soft reset fails

2018-01-17 Thread Brian Norris 
In this function, we init the USB2 and USB3 PHYs, but if soft reset
times out, we don't unwind this.

Noticed by inspection.

Signed-off-by: Brian Norris <briannor...@chromium.org>
---
 drivers/usb/dwc3/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 07832509584f..1cbbca9fcc52 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -233,6 +233,9 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
udelay(1);
} while (--retries);
 
+   phy_exit(dwc->usb3_generic_phy);
+   phy_exit(dwc->usb2_generic_phy);
+
return -ETIMEDOUT;
 }
 
-- 
2.16.0.rc1.238.g530d649a79-goog

--
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] Revert "Bluetooth: btusb: fix QCA Rome suspend/resume"

2017-12-21 Thread Brian Norris 
On Thu, Dec 21, 2017 at 3:43 AM, Daniel Drake <dr...@endlessm.com> wrote:
> On Wed, Dec 20, 2017 at 6:53 PM, Brian Norris <briannor...@chromium.org> 
> wrote:
>>
>> On Wed, Dec 20, 2017 at 07:00:07PM +0800, Kai-Heng Feng wrote:
>> > This commit causes a regression on some QCA ROME chips. The USB device
>> > reset happens in btusb_open(), hence firmware loading gets interrupted.
>>
>> Oh, did you really confirm that's the root of the problem? I was only
>> hypothesizing, with some informed observation and code review; but I
>> didn't fully convince myself. If so, that's interesting.
>
> I have the same doubt. Can you explain how/why firmware uploading and
> btusb_open() overlap, and how this is avoided with your patch?
> If they do overlap, is that not a bug in the stack that should be fixed 
> instead?
> If the fix belongs in btusb and this BTUSB_RESET_RESUME thing really
> is problematic, should it be totally removed instead?

To be clear: this is a regression in mainline and should definitely be
fixed by reverting it. The path forward for patch 2/2 should be
determined by a better understanding of where the real problem is.

Brian
--
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] Revert "Bluetooth: btusb: fix QCA Rome suspend/resume"

2017-12-20 Thread Brian Norris 
On Wed, Dec 20, 2017 at 07:00:07PM +0800, Kai-Heng Feng wrote:
> This reverts commit fd865802c66bc451dc515ed89360f84376ce1a56.
> 
> This commit causes a regression on some QCA ROME chips. The USB device
> reset happens in btusb_open(), hence firmware loading gets interrupted.

Oh, did you really confirm that's the root of the problem? I was only
hypothesizing, with some informed observation and code review; but I
didn't fully convince myself. If so, that's interesting.

> Furthermore, this commit stops working after commit
> ("a0085f2510e8976614ad8f766b209448b385492f Bluetooth: btusb: driver to
> enable the usb-wakeup feature"). Reset-resume quirk only gets enabled in
> btusb_suspend() when it's not a wakeup source.
> 
> If we really want to reset the USB device, we need to do it before
> btusb_open(). Let's handle it in drivers/usb/core/quirks.c.
> 
> Cc: sta...@vger.kernel.org
> Cc: Leif Liddy <leif.li...@gmail.com>
> Cc: Matthias Kaehlcke <m...@chromium.org>
> Cc: Brian Norris <briannor...@chromium.org>
> Cc: Daniel Drake <dr...@endlessm.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.f...@canonical.com>

Looks good to me. Definitely a regression and we need to clear that up
in mainline and stable before reintroducing the intended fix:

Reviewed-by: Brian Norris <briannor...@chromium.org>
Tested-by: Brian Norris <briannor...@chromium.org>

Thanks!

> ---
> 
> Daniel, Cc you because this also affects your original quirk patch for
> Realtek btusb.
> 
>  drivers/bluetooth/btusb.c | 6 --
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index f7120c9eb9bd..da353c4acdc7 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -3117,12 +3117,6 @@ static int btusb_probe(struct usb_interface *intf,
>   if (id->driver_info & BTUSB_QCA_ROME) {
>   data->setup_on_usb = btusb_setup_qca;
>   hdev->set_bdaddr = btusb_set_bdaddr_ath3012;
> -
> - /* QCA Rome devices lose their updated firmware over suspend,
> -  * but the USB hub doesn't notice any status change.
> -  * Explicitly request a device reset on resume.
> -  */
> - set_bit(BTUSB_RESET_RESUME, >flags);
>   }
>  
>  #ifdef CONFIG_BT_HCIBTUSB_RTL
> -- 
> 2.14.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: [PATCH v5 1/6] usb: separate out sysdev pointer from usb_bus

2016-12-02 Thread Brian Norris 
Hi all,

On Thu, Nov 17, 2016 at 05:13:43PM +0530, Sriram Dash wrote:
> From: Arnd Bergmann <a...@arndb.de>
> 
> For xhci-hcd platform device, all the DMA parameters are not
> configured properly, notably dma ops for dwc3 devices.
> 
> The idea here is that you pass in the parent of_node along with
> the child device pointer, so it would behave exactly like the
> parent already does. The difference is that it also handles all
> the other attributes besides the mask.
> 
> sysdev will represent the physical device, as seen from firmware
> or bus.Splitting the usb_bus->controller field into the
> Linux-internal device (used for the sysfs hierarchy, for printks
> and for power management) and a new pointer (used for DMA,
> DT enumeration and phy lookup) probably covers all that we really
> need.
> 
> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> Signed-off-by: Sriram Dash <sriram.d...@nxp.com>
> Tested-by: Baolin Wang <baolin.w...@linaro.org>
> Cc: Felipe Balbi <felipe.ba...@linux.intel.com>
> Cc: Grygorii Strashko <grygorii.stras...@ti.com>
> Cc: Sinjan Kumar <sinj...@codeaurora.org>
> Cc: David Fisher <david.fish...@synopsys.com>
> Cc: Catalin Marinas <catalin.mari...@arm.com>
> Cc: "Thang Q. Nguyen" <tqngu...@apm.com>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda...@renesas.com>
> Cc: Stephen Boyd <sb...@codeaurora.org>
> Cc: Bjorn Andersson <bjorn.anders...@linaro.org>
> Cc: Ming Lei <tom.leim...@gmail.com>
> Cc: Jon Masters <j...@redhat.com>
> Cc: Dann Frazier <dann.fraz...@canonical.com>
> Cc: Peter Chen <peter.c...@nxp.com>
> Cc: Leo Li <pku@gmail.com>
> ---
> Changes in v5:
>   - No update
> 
> Changes in v4:
>   - No update
> 
> Changes in v3: 
>   - usb is_device_dma_capable instead of directly accessing 
> dma props. 
>  
> Changes in v2: 
>   - Split the patch wrt driver

I didn't notice this series had been reposted a few times. For some
reason, this wasn't that easy to find in search engines... Anyway, when
the whole series is applied, this fixes my XHCI probe issues for DWC3
host mode. Thanks!

Tested-by: Brian Norris <briannor...@chromium.org>

But I noticed that Felipe has applied patches 5 and 6 in -next, while
the rest are still outstanding. That means I hit the dma_mask WARN_ON()
in xhci-plat.c, and it eventually fails to probe with -EIO still:

[2.060272] [ cut here ]
[2.064908] WARNING: CPU: 5 PID: 1 at drivers/usb/host/xhci-plat.c:159 
xhci_plat_probe+0x5c/0x444
...
[2.25] [] xhci_plat_probe+0x5c/0x444
[2.294456] [] platform_drv_probe+0x60/0xac
[2.300200] [] driver_probe_device+0x12c/0x2a0
[2.306204] [] __driver_attach+0x84/0xb0
[2.311687] [] bus_for_each_dev+0x9c/0xcc
[2.317256] [] driver_attach+0x2c/0x34
[2.322566] [] bus_add_driver+0xf0/0x1f4
[2.328049] [] driver_register+0x9c/0xe8
[2.333530] [] __platform_driver_register+0x60/0x6c
[2.339968] [] xhci_plat_init+0x2c/0x34
[2.345366] [] do_one_initcall+0xa4/0x13c
[2.350936] [] kernel_init_freeable+0x1bc/0x274
[2.357026] [] kernel_init+0x18/0x104
[2.362247] [] ret_from_fork+0x10/0x50
[2.374615] xhci-hcd: probe of xhci-hcd.1.auto failed with error -5
[2.380962] [ cut here ]
[2.385588] WARNING: CPU: 4 PID: 1 at drivers/usb/host/xhci-plat.c:159 
xhci_plat_probe+0x5c/0x444
...
[2.637372] [] xhci_plat_probe+0x5c/0x444
[2.642941] [] platform_drv_probe+0x60/0xac
[2.648685] [] driver_probe_device+0x12c/0x2a0
[2.654688] [] __driver_attach+0x84/0xb0
[2.660170] [] bus_for_each_dev+0x9c/0xcc
[2.665739] [] driver_attach+0x2c/0x34
[2.671048] [] bus_add_driver+0xf0/0x1f4
[2.676532] [] driver_register+0x9c/0xe8
[2.682012] [] __platform_driver_register+0x60/0x6c
[2.688450] [] xhci_plat_init+0x2c/0x34
[2.693845] [] do_one_initcall+0xa4/0x13c
[2.699415] [] kernel_init_freeable+0x1bc/0x274
[2.705505] [] kernel_init+0x18/0x104
[2.710726] [] ret_from_fork+0x10/0x50
[2.716075] xhci-hcd: probe of xhci-hcd.2.auto failed with error -5

What's happening with patches 1-4?

Regards,
Brian
--
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

[RFC PATCH] usb: core: correct usb_get_dev() documentation

2016-10-27 Thread Brian Norris 
In reading through a USB interface driver, I noticed that it called
usb_{get,put}_dev() in its probe() and disconnect() methods. This seemed
unnecessary, but a look at the comments here matched the usage.

USB interface devices seem to be well covered by the parent/child
relationship of the device model, and so it should be unnecessary for a
child device to grab a refcount on its parent device.

Signed-off-by: Brian Norris <computersforpe...@gmail.com>
---
This reflects my understanding (and testing), as well as the majority of usage
-- there are *very* few interface drivers that actually call usb_get_dev(). If
I'm wrong, please feel free to tell me so! But I thought patching the
documentation would be the best way to solicit a response :)

 drivers/usb/core/usb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 592151461017..0ba7e070f04e 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -539,9 +539,9 @@ EXPORT_SYMBOL_GPL(usb_alloc_dev);
  *
  * Each live reference to a device should be refcounted.
  *
- * Drivers for USB interfaces should normally record such references in
- * their probe() methods, when they bind to an interface, and release
- * them by calling usb_put_dev(), in their disconnect() methods.
+ * The device driver core automatically handles this refcounting for USB
+ * interface drivers, but this API can be used for non-parent/child
+ * relationships.
  *
  * Return: A pointer to the device with the incremented reference counter.
  */
-- 
2.8.0.rc3.226.g39d4020

--
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: [PACTH,v6,1/2] usb: xhci: plat: Enable runtime PM

2016-08-26 Thread Brian Norris 
Corrected a bouncing email address (sorry for the noise)

On Fri, Aug 26, 2016 at 03:11:36PM -0700, Brian Norris wrote:
> Hi,
> 
> On Wed, Aug 24, 2016 at 04:48:01PM -0400, Robert Foss wrote:
> > On 2016-08-22 11:23 PM, Brian Norris wrote:
> > >+ others
> > >
> > >Hi Robert and Felipe,
> > >
> > >I have a few questions for one or both of you. I'm not really an expert
> > >on runtime PM, so please take my questions with a grain of salt.
> > >
> > >On Wed, Aug 10, 2016 at 04:32:15PM -0400, robert.f...@collabora.com wrote:
> > >>From: Robert Foss <robert.f...@collabora.com>
> > >>
> > >>Enable runtime PM for the xhci-plat device so that the parent device
> > >>may implement runtime PM.
> > >>
> > >>Signed-off-by: Robert Foss <robert.f...@collabora.com>
> > >>
> > >>Tested-by: Robert Foss <robert.f...@collabora.com>
> > >>---
> > >> drivers/usb/host/xhci-plat.c | 29 +++--
> > >> 1 file changed, 27 insertions(+), 2 deletions(-)
> > >>
> > >>diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> > >>index ed56bf9..ba4efe7 100644
> > >>--- a/drivers/usb/host/xhci-plat.c
> > >>+++ b/drivers/usb/host/xhci-plat.c
> > >>@@ -246,6 +246,9 @@ static int xhci_plat_probe(struct platform_device 
> > >>*pdev)
> > >>  if (ret)
> > >>  goto dealloc_usb2_hcd;
> > >>
> > >>+ pm_runtime_set_active(>dev);
> > >>+ pm_runtime_enable(>dev);
> > >>+
> > >
> > >How does it help to enable PM runtime like this, if you don't have any
> > >kind of runtime_{suspend,resume}() callbacks?
> > 
> > Andrew, I think you understand the inner workings of this code
> > better than me, maybe you could give a short summary?
> 
> I believe Andrew is fairly busy, and I already talked with him a bit
> about this. This is needed (as per your (or his?) commit message) only
> if you have a parent device that wants to implement runtime PM. Now
> depending on what you want to do with "runtime PM", this might mean
> the parent device has to suspend xhci_{suspend,resume}() on behalf of
> xhci-plat.c. Not very nice layering IMO, but it has been done before...
> 
> So I guess this comes down to "what does a parent device/driver want to
> do"? If that's (e.g.) just putting some PHY into a slightly lower power
> mode, then maybe it's fine for xhci-plat not to do anything else. But if
> you actually want to completely power down the parent bus, reset an
> accompanying dual-role/OTG controller, etc., then this really isn't
> sufficient, AFAICT. But maybe that's just an indictment of the poor
> structure of dwc3's host-mode support, more than it is an indictment of
> this patch.
> 
> Brian
--
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: [PACTH,v6,1/2] usb: xhci: plat: Enable runtime PM

2016-08-26 Thread Brian Norris 
Hi,

On Wed, Aug 24, 2016 at 04:48:01PM -0400, Robert Foss wrote:
> On 2016-08-22 11:23 PM, Brian Norris wrote:
> >+ others
> >
> >Hi Robert and Felipe,
> >
> >I have a few questions for one or both of you. I'm not really an expert
> >on runtime PM, so please take my questions with a grain of salt.
> >
> >On Wed, Aug 10, 2016 at 04:32:15PM -0400, robert.f...@collabora.com wrote:
> >>From: Robert Foss <robert.f...@collabora.com>
> >>
> >>Enable runtime PM for the xhci-plat device so that the parent device
> >>may implement runtime PM.
> >>
> >>Signed-off-by: Robert Foss <robert.f...@collabora.com>
> >>
> >>Tested-by: Robert Foss <robert.f...@collabora.com>
> >>---
> >> drivers/usb/host/xhci-plat.c | 29 +++--
> >> 1 file changed, 27 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> >>index ed56bf9..ba4efe7 100644
> >>--- a/drivers/usb/host/xhci-plat.c
> >>+++ b/drivers/usb/host/xhci-plat.c
> >>@@ -246,6 +246,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
> >>if (ret)
> >>goto dealloc_usb2_hcd;
> >>
> >>+   pm_runtime_set_active(>dev);
> >>+   pm_runtime_enable(>dev);
> >>+
> >
> >How does it help to enable PM runtime like this, if you don't have any
> >kind of runtime_{suspend,resume}() callbacks?
> 
> Andrew, I think you understand the inner workings of this code
> better than me, maybe you could give a short summary?

I believe Andrew is fairly busy, and I already talked with him a bit
about this. This is needed (as per your (or his?) commit message) only
if you have a parent device that wants to implement runtime PM. Now
depending on what you want to do with "runtime PM", this might mean
the parent device has to suspend xhci_{suspend,resume}() on behalf of
xhci-plat.c. Not very nice layering IMO, but it has been done before...

So I guess this comes down to "what does a parent device/driver want to
do"? If that's (e.g.) just putting some PHY into a slightly lower power
mode, then maybe it's fine for xhci-plat not to do anything else. But if
you actually want to completely power down the parent bus, reset an
accompanying dual-role/OTG controller, etc., then this really isn't
sufficient, AFAICT. But maybe that's just an indictment of the poor
structure of dwc3's host-mode support, more than it is an indictment of
this patch.

Brian
--
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: [PACTH,v6,1/2] usb: xhci: plat: Enable runtime PM

2016-08-22 Thread Brian Norris 
+ others

Hi Robert and Felipe,

I have a few questions for one or both of you. I'm not really an expert
on runtime PM, so please take my questions with a grain of salt.

On Wed, Aug 10, 2016 at 04:32:15PM -0400, robert.f...@collabora.com wrote:
> From: Robert Foss 
> 
> Enable runtime PM for the xhci-plat device so that the parent device
> may implement runtime PM.
> 
> Signed-off-by: Robert Foss 
> 
> Tested-by: Robert Foss 
> ---
>  drivers/usb/host/xhci-plat.c | 29 +++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index ed56bf9..ba4efe7 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -246,6 +246,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
>   if (ret)
>   goto dealloc_usb2_hcd;
>  
> + pm_runtime_set_active(>dev);
> + pm_runtime_enable(>dev);
> +

How does it help to enable PM runtime like this, if you don't have any
kind of runtime_{suspend,resume}() callbacks?

I suspect that this patch set was derived from the Chromium OS kernel
tree, where we were supporting a Tegra XHCI chipset:

https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.10/drivers/usb/host/xhci-tegra.c#1920

It looks like the driver was refactored to not use xhci-plat.c before it
was upstreamed (and runtime PM support was dropped along the way).

So, I'm wondering how I might actually use this? Particularly, I'm
looking at trying out runtime suspend for a DWC3 controller in host
mode, and it looks like I'd have to do some layer-violating calls to
xhci_suspend()/xhci_resume() from the parent dwc3 device, or else
rewrite drivers/usb/dwc3/host.c to avoid using xhci-plat.c.

(I also see that Baolin, CC'd here, was interested in dwc3 [1].)

Or possibly an enlightening question for me: if you don't mind, how are
you utilizing runtime PM in conjunction with xhci-plat.c, Robert?
Presumably some other parent device/driver is doing some additional
management of the XHCI core?

Regards,
Brian

[1] [PATCH 4/4] usb: dwc3: core: Support the dwc3 host suspend/resume
https://lkml.org/lkml/2016/7/15/181
https://patchwork.kernel.org/patch/9231417/

>   return 0;
>  
>  
> @@ -274,6 +277,8 @@ static int xhci_plat_remove(struct platform_device *dev)
>   struct xhci_hcd *xhci = hcd_to_xhci(hcd);
>   struct clk *clk = xhci->clk;
>  
> + pm_runtime_disable(>dev);
> +
>   usb_remove_hcd(xhci->shared_hcd);
>   usb_phy_shutdown(hcd->usb_phy);
>  
> @@ -292,6 +297,13 @@ static int xhci_plat_suspend(struct device *dev)
>  {
>   struct usb_hcd  *hcd = dev_get_drvdata(dev);
>   struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> + int ret;
> +
> + ret = pm_runtime_get_sync(dev);
> + if (ret < 0) {
> + pm_runtime_put(dev);
> + return ret;
> + }
>  
>   /*
>* xhci_suspend() needs `do_wakeup` to know whether host is allowed
> @@ -301,15 +313,28 @@ static int xhci_plat_suspend(struct device *dev)
>* reconsider this when xhci_plat_suspend enlarges its scope, e.g.,
>* also applies to runtime suspend.
>*/
> - return xhci_suspend(xhci, device_may_wakeup(dev));
> + ret = xhci_suspend(xhci, device_may_wakeup(dev));
> + pm_runtime_put(dev);
> +
> + return ret;
>  }
>  
>  static int xhci_plat_resume(struct device *dev)
>  {
>   struct usb_hcd  *hcd = dev_get_drvdata(dev);
>   struct xhci_hcd *xhci = hcd_to_xhci(hcd);
> + int ret;
>  
> - return xhci_resume(xhci, 0);
> + ret = pm_runtime_get_sync(dev);
> + if (ret < 0) {
> + pm_runtime_put(dev);
> + return ret;
> + }
> +
> + ret = xhci_resume(xhci, 0);
> + pm_runtime_put(dev);
> +
> + return ret;
>  }
>  
>  static const struct dev_pm_ops xhci_plat_pm_ops = {
--
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] Documentation: dt: dwc3: note the supported phy-names

2016-08-18 Thread Brian Norris 
The dwc3 driver expicitly looks for "usb2-phy" or "usb3-phy", but we
never noted these names in the documentation.

Signed-off-by: Brian Norris <briannor...@chromium.org>
---
 Documentation/devicetree/bindings/usb/dwc3.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
b/Documentation/devicetree/bindings/usb/dwc3.txt
index 7d7ce089b003..2358779cf6cb 100644
--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt
@@ -13,7 +13,8 @@ Optional properties:
in the array is expected to be a handle to the USB2/HS PHY and
the second element is expected to be a handle to the USB3/SS PHY
  - phys: from the *Generic PHY* bindings
- - phy-names: from the *Generic PHY* bindings
+ - phy-names: from the *Generic PHY* bindings; supported names are "usb2-phy"
+   or "usb3-phy".
  - snps,usb3_lpm_capable: determines if platform is USB3 LPM capable
  - snps,disable_scramble_quirk: true when SW should disable data scrambling.
Only really useful for FPGA builds.
-- 
2.8.0.rc3.226.g39d4020

--
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/4] usb: dwc3: of-simple: add compatible for rockchip

2016-05-09 Thread Brian Norris 
Hi William,

Did you leave off linux-rockc...@lists.infradead.org intentionally? IMO,
it's nice to have that list in CC, so interested parties can follow your
work, even if they aren't as fortunate as me to have been CC'd on your
patch directly.

On Mon, May 09, 2016 at 07:46:14PM +0800, William Wu wrote:
> Signed-off-by: William Wu 
> ---
>  drivers/usb/dwc3/dwc3-of-simple.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
> b/drivers/usb/dwc3/dwc3-of-simple.c
> index 9743353..1f3665b 100644
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -162,6 +162,7 @@ static const struct dev_pm_ops dwc3_of_simple_dev_pm_ops 
> = {
>  static const struct of_device_id of_dwc3_simple_match[] = {
>   { .compatible = "qcom,dwc3" },
>   { .compatible = "xlnx,zynqmp-dwc3" },
> + { .compatible = "rockchip,dwc3" },

Add to Documentation/devicetree/bindings/. Do we need a new
Documentation/devicetree/bindings/usb/rockchip,dwc3.txt, to match the
pattern of qcom and xlnx? Or can we just add to dwc3.txt, since so far,
all bindings are documented in the common file?

Brian

>   { /* Sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, of_dwc3_simple_match);
> -- 
> 1.9.1
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: usb: dwc3: host: inherit dma configuration from parent dev

2016-05-05 Thread Brian Norris 
On Mon, Apr 25, 2016 at 10:21:34PM +0300, Strashko, Grygorii wrote:
> Now not all DMA paremters configured properly for "xhci-hcd" platform
> device which is created manually. For example: dma_pfn_offset, dam_ops
> and iommu configuration will not corresponds "dwc3" devices
> configuration. As result, this will cause problems like wrong DMA
> addresses translation on platforms with LPAE enabled like Keystone 2.
> 
> When platform is using DT boot mode the DMA configuration will be
> parsed and applied from DT, so, to fix this issue, reuse
> of_dma_configure() API and retrieve DMA configuartion for "xhci-hcd"
> from DWC3 device node.
> 
> Cc: David Fisher <david.fish...@synopsys.com>
> Cc: Catalin Marinas <catalin.mari...@arm.com>
> Cc: "Thang Q. Nguyen" <tqngu...@apm.com>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda...@renesas.com>
> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com>

Tested-by: Brian Norris <briannor...@chromium.org>

What's the status of this? I see that there was some divergent
discussion about the merits of a dma_inherit() API...

FWIW, I'll reiterate Grygorii's note that Felipe's alternative patch
does NOT resolve the problem with the creation of the xhci-hcd platform
device:

https://patchwork.kernel.org/patch/8952721/

Brian

> ---
> drivers/usb/dwc3/host.c | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> index c679f63..93c8ef9 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -17,6 +17,7 @@
>  
>  #include 
>  #include 
> +#include 
>  
>  #include "core.h"
>  
> @@ -32,12 +33,7 @@ int dwc3_host_init(struct dwc3 *dwc)
>   return -ENOMEM;
>   }
>  
> - dma_set_coherent_mask(>dev, dwc->dev->coherent_dma_mask);
> -
>   xhci->dev.parent= dwc->dev;
> - xhci->dev.dma_mask  = dwc->dev->dma_mask;
> - xhci->dev.dma_parms = dwc->dev->dma_parms;
> -
>   dwc->xhci = xhci;
>  
>   ret = platform_device_add_resources(xhci, dwc->xhci_resources,
> @@ -62,6 +58,15 @@ int dwc3_host_init(struct dwc3 *dwc)
>   phy_create_lookup(dwc->usb3_generic_phy, "usb3-phy",
> dev_name(>dev));
>  
> + if (IS_ENABLED(CONFIG_OF) && dwc->dev->of_node) {
> + of_dma_configure(>dev, dwc->dev->of_node);
> + } else {
> + dma_set_coherent_mask(>dev, dwc->dev->coherent_dma_mask);
> +
> + xhci->dev.dma_mask  = dwc->dev->dma_mask;
> + xhci->dev.dma_parms = dwc->dev->dma_parms;
> + }
> +
>   ret = platform_device_add(xhci);
>   if (ret) {
>   dev_err(dwc->dev, "failed to register xHCI device\n");
--
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] alauda: do not use stack for URB transfer_buffers

2013-08-21 Thread Brian Norris 
On Tue, Aug 06, 2013 at 03:03:29PM +0300, Jussi Kivilinna wrote:
 Patch fixes alauda not to use stack as URB transfer_buffer. URB buffers need 
 to
 be DMA-able, which stack is not.
 
 Patch is only compile tested.
 
 Cc: sta...@vger.kernel.org
 Signed-off-by: Jussi Kivilinna jussi.kivili...@iki.fi
 ---
  drivers/mtd/nand/alauda.c |   74 
 ++---
  1 file changed, 56 insertions(+), 18 deletions(-)

Just FYI, this driver is being removed, so I'm obviously not taking this
patch :)

Brian
--
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] alauda: do not use stack for URB transfer_buffers

2013-08-21 Thread Brian Norris 
On Wed, Aug 21, 2013 at 09:59:27AM +0200, Bjørn Mork wrote:
 Brian Norris computersforpe...@gmail.com writes:
  On Tue, Aug 06, 2013 at 03:03:29PM +0300, Jussi Kivilinna wrote:
  Patch fixes alauda not to use stack as URB transfer_buffer. URB buffers 
  need to
  be DMA-able, which stack is not.
 
  Patch is only compile tested.
 
  Cc: sta...@vger.kernel.org
  Signed-off-by: Jussi Kivilinna jussi.kivili...@iki.fi
  ---
   drivers/mtd/nand/alauda.c |   74 
  ++---
   1 file changed, 56 insertions(+), 18 deletions(-)
 
  Just FYI, this driver is being removed, so I'm obviously not taking this
  patch :)

 I think you should apply it anyway. The driver is still in v3.11 AFAICS,
 and the patch should also go to the maintained stable kernels. You
 cannot remove the driver from them, and I don't see a later driver
 removal as a valid reason not to fix a known bug with a patch.

Seriously?

The reasons given for removal:

  The driver has very low utility.  Devices in question are limited to
  about 400kB/s and the only known user (me) discarded the hardware
  several years back.

And:

  Maybe we should just remove the driver and not spend any more time on
  it?

So you're suggesting applying an untested (compile-only) fix for an
unobserved bug for the theoretical user of old, slow hardware who wants
to use a recent stable kernel, when the last known user has given up on
the driver entirely?

Anyway, just because you complained, I rebased and applied this to
l2-mtd.git before the driver removal. I'll defer to dwmw2 whether this
patch gets squashed out of existence.

Brian
--
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] alauda: do not use stack for URB transfer_buffers

2013-08-21 Thread Brian Norris 
On Wed, Aug 21, 2013 at 2:59 AM, Jussi Kivilinna jussi.kivili...@iki.fi wrote:
 On 21.08.2013 11:41, Brian Norris wrote:
 On Wed, Aug 21, 2013 at 09:59:27AM +0200, Bjørn Mork wrote:
 Brian Norris computersforpe...@gmail.com writes:
 On Tue, Aug 06, 2013 at 03:03:29PM +0300, Jussi Kivilinna wrote:
 Patch fixes alauda not to use stack as URB transfer_buffer. URB buffers 
 need to
 be DMA-able, which stack is not.

 Patch is only compile tested.

 Cc: sta...@vger.kernel.org
 Signed-off-by: Jussi Kivilinna jussi.kivili...@iki.fi
 ---
  drivers/mtd/nand/alauda.c |   74 
 ++---
  1 file changed, 56 insertions(+), 18 deletions(-)

 Just FYI, this driver is being removed, so I'm obviously not taking this
 patch :)

 I think you should apply it anyway. The driver is still in v3.11 AFAICS,
 and the patch should also go to the maintained stable kernels. You
 cannot remove the driver from them, and I don't see a later driver
 removal as a valid reason not to fix a known bug with a patch.

 Seriously?

 The reasons given for removal:

   The driver has very low utility.  Devices in question are limited to
   about 400kB/s and the only known user (me) discarded the hardware
   several years back.

 And:

   Maybe we should just remove the driver and not spend any more time on
   it?

 So you're suggesting applying an untested (compile-only) fix for an
 unobserved bug for the theoretical user of old, slow hardware who wants
 to use a recent stable kernel, when the last known user has given up on
 the driver entirely?

 Anyway, just because you complained, I rebased and applied this to
 l2-mtd.git before the driver removal. I'll defer to dwmw2 whether this
 patch gets squashed out of existence.

 If this is the case, please ignore this patch. I should not have marked it 
 for stable in first place, since it is just compile-tested.

Yes, that's a good point. Quoting Documentation/stable_kernel_rules.txt:

It must be obviously correct and tested.

Seeing as it was not tested, I am dropping the patch entirely (it is
not stable material, and there is no point including it along with the
driver removal).

Thanks,
Brian
--
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