Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

[Manu Gautam]

Hi,


On 3/14/2018 2:20 PM, Felipe Balbi wrote:
> Hi,
>
> Manu Gautam  writes:
>
[snip]
  - Support to replace pip3 clock going to DWC3 with utmi clock
for hardware configuration where SSPHY is not used with DWC3.
>>> Is that SW configurable? Really? In any case seems like this and SESSVLD
>>> valid should be handled using Hans' and Heikki's mux support.
>> Yes, with this we can use dwc3 without using SSPHY. Please point me to
>> those patches. There are only bunch of register writes in glue wrapper to
>> achieve the same.
> https://www.spinics.net/lists/linux-usb/msg160868.html

I looked at the patchset and thinking that adding mux for this may
not be of much help here. Qscratch is part of dwc3 wrapper
and uses same clock domain for its registers. Hence, I can't
have a separate mux driver for same. Will have to register
mux controller from this driver for these and use only in this driver
as I dont expect any other client for same. So, can I proceed with
existing logic?

>
 +static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
 +{
 +  struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
>>> nope! Glue shouldn't touch dwc3 at all.
>> Other than PHY handles, need this to fail runtime suspend if dwc3 hasn't
>> probed yet.
> Will that even happen? You should pm_runtime_forbid() by default,
> anyway and expect it to be enabled via sysfs later, no?

It happens if I enable runtime_pm from probe which is the case right now.
I will disable it from probe as you suggested.
In any case dwc3 uses pm_runtime_forbid and expects it to be enabled from sysfs,
so I dont get any benefit of enabling it from glue driver probe.

Other than this, I need to access 'dwc->xhci' to resume root hub on remote 
wakeup.
That I missed to mention earlier.

>
 +  dwc3_qcom_suspend_hsphy(qcom);
 +
 +  if (dwc->usb2_generic_phy)
 +  phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
 +  if (dwc->usb3_generic_phy)
 +  phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
>>> core.c should do this.
>> Recommended sequence from h/w programming guide is that:
>> 1. PHY autosuspend must be left disabled - 
>> snps,dis_u2_susphy_quirk/dis_enblslpm_quirk
>> 2. During runtime-suspend (say on xhci bus_suspend) , PHY should be suspended
>>     using GUSB2PHYCFG register
> this is something that dwc3 core can do on its own suspend implementation.
>
>> 3. Wait until pwr_event_irq_stat in qscratch reflects PHY transition to L2.
> this is interesting part. Is this register accessible by the PHY driver?
> Seems like that would be the best place to stuff this...

This register is in controller wrapper which PHY driver can't access.
Also clock domain is different.

>
>> 3. USB2 PHY driver can suspend - enable wakeup events and turns off clocks 
>> etc.
> ... together with this.
>
>> 4. dwc3 glue driver can suspend.
>>
>> Since, pwr_event_irq stat can't be checked in core driver, I added this 
>> handling
>> in glue driver. Alternative approach I can think of is to let dwc3 core 
>> suspend
>> PHY using GUSBPHYCFG register on suspend,  add some delay before
>> suspending PHY. Glue driver can check for pwr_event_irq stat and throw a
>> warning if PHY not in L2.
> almost :-) core_suspend fiddles with GUSB2PHYCFG for suspend and calls
> phy_suspend() (or whatever the function is called heh), that will go to
> your phy driver's suspend callback, which checks pwr_event_irq_stat and
> then pm_runtime_put() to schedule ->runtime_suspend() so that can enable
> wakeups and switch off clocks.

Since phy driver can not access pwr_event_irq_stat, should I just use some delay
and assume PHY gets suspended?

>
 +  irq = platform_get_irq_byname(pdev, "dp_hs_phy_irq");
 +  if (irq > 0) {
 +  irq_set_status_flags(irq, IRQ_NOAUTOEN);
>>> why do you need to set this flag?
>> These wakeup_irqs should be enabled only during suspend. With this flag I
>> don't need to disable irq immediately after requesting it.
> oh, okay. You may want to add a comment here :-)
Sure.

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

[Manu Gautam]

Hi,


On 3/14/2018 2:20 PM, Felipe Balbi wrote:
> Hi,
>
> Manu Gautam  writes:
>
[snip]
  - Support to replace pip3 clock going to DWC3 with utmi clock
for hardware configuration where SSPHY is not used with DWC3.
>>> Is that SW configurable? Really? In any case seems like this and SESSVLD
>>> valid should be handled using Hans' and Heikki's mux support.
>> Yes, with this we can use dwc3 without using SSPHY. Please point me to
>> those patches. There are only bunch of register writes in glue wrapper to
>> achieve the same.
> https://www.spinics.net/lists/linux-usb/msg160868.html

I looked at the patchset and thinking that adding mux for this may
not be of much help here. Qscratch is part of dwc3 wrapper
and uses same clock domain for its registers. Hence, I can't
have a separate mux driver for same. Will have to register
mux controller from this driver for these and use only in this driver
as I dont expect any other client for same. So, can I proceed with
existing logic?

>
 +static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
 +{
 +  struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
>>> nope! Glue shouldn't touch dwc3 at all.
>> Other than PHY handles, need this to fail runtime suspend if dwc3 hasn't
>> probed yet.
> Will that even happen? You should pm_runtime_forbid() by default,
> anyway and expect it to be enabled via sysfs later, no?

It happens if I enable runtime_pm from probe which is the case right now.
I will disable it from probe as you suggested.
In any case dwc3 uses pm_runtime_forbid and expects it to be enabled from sysfs,
so I dont get any benefit of enabling it from glue driver probe.

Other than this, I need to access 'dwc->xhci' to resume root hub on remote 
wakeup.
That I missed to mention earlier.

>
 +  dwc3_qcom_suspend_hsphy(qcom);
 +
 +  if (dwc->usb2_generic_phy)
 +  phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
 +  if (dwc->usb3_generic_phy)
 +  phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
>>> core.c should do this.
>> Recommended sequence from h/w programming guide is that:
>> 1. PHY autosuspend must be left disabled - 
>> snps,dis_u2_susphy_quirk/dis_enblslpm_quirk
>> 2. During runtime-suspend (say on xhci bus_suspend) , PHY should be suspended
>>     using GUSB2PHYCFG register
> this is something that dwc3 core can do on its own suspend implementation.
>
>> 3. Wait until pwr_event_irq_stat in qscratch reflects PHY transition to L2.
> this is interesting part. Is this register accessible by the PHY driver?
> Seems like that would be the best place to stuff this...

This register is in controller wrapper which PHY driver can't access.
Also clock domain is different.

>
>> 3. USB2 PHY driver can suspend - enable wakeup events and turns off clocks 
>> etc.
> ... together with this.
>
>> 4. dwc3 glue driver can suspend.
>>
>> Since, pwr_event_irq stat can't be checked in core driver, I added this 
>> handling
>> in glue driver. Alternative approach I can think of is to let dwc3 core 
>> suspend
>> PHY using GUSBPHYCFG register on suspend,  add some delay before
>> suspending PHY. Glue driver can check for pwr_event_irq stat and throw a
>> warning if PHY not in L2.
> almost :-) core_suspend fiddles with GUSB2PHYCFG for suspend and calls
> phy_suspend() (or whatever the function is called heh), that will go to
> your phy driver's suspend callback, which checks pwr_event_irq_stat and
> then pm_runtime_put() to schedule ->runtime_suspend() so that can enable
> wakeups and switch off clocks.

Since phy driver can not access pwr_event_irq_stat, should I just use some delay
and assume PHY gets suspended?

>
 +  irq = platform_get_irq_byname(pdev, "dp_hs_phy_irq");
 +  if (irq > 0) {
 +  irq_set_status_flags(irq, IRQ_NOAUTOEN);
>>> why do you need to set this flag?
>> These wakeup_irqs should be enabled only during suspend. With this flag I
>> don't need to disable irq immediately after requesting it.
> oh, okay. You may want to add a comment here :-)
Sure.

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

[Heikki Krogerus]

Hi,

On Wed, Mar 14, 2018 at 10:50:12AM +0200, Felipe Balbi wrote:
> >>>  - Support to replace pip3 clock going to DWC3 with utmi clock
> >>>for hardware configuration where SSPHY is not used with DWC3.
> >> Is that SW configurable? Really? In any case seems like this and SESSVLD
> >> valid should be handled using Hans' and Heikki's mux support.
> >
> > Yes, with this we can use dwc3 without using SSPHY. Please point me to
> > those patches. There are only bunch of register writes in glue wrapper to
> > achieve the same.
> 
> https://www.spinics.net/lists/linux-usb/msg160868.html

I'm not sure if that series is usable in this case. Though we do
control the VBUSVLD pin on DWC3 in the driver for the "Intel USB
mux" Hans is introducing, the framework for the USB role switches does
not provide any support for it.

The Intel USB mux driver simply enables VBUSVLD when device mode is
selected, and disables it if host mode is selected, and when the
selection is cleared (disconnection).


Br,

-- 
heikki

Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

[Heikki Krogerus]

Hi,

On Wed, Mar 14, 2018 at 10:50:12AM +0200, Felipe Balbi wrote:
> >>>  - Support to replace pip3 clock going to DWC3 with utmi clock
> >>>for hardware configuration where SSPHY is not used with DWC3.
> >> Is that SW configurable? Really? In any case seems like this and SESSVLD
> >> valid should be handled using Hans' and Heikki's mux support.
> >
> > Yes, with this we can use dwc3 without using SSPHY. Please point me to
> > those patches. There are only bunch of register writes in glue wrapper to
> > achieve the same.
> 
> https://www.spinics.net/lists/linux-usb/msg160868.html

I'm not sure if that series is usable in this case. Though we do
control the VBUSVLD pin on DWC3 in the driver for the "Intel USB
mux" Hans is introducing, the framework for the USB role switches does
not provide any support for it.

The Intel USB mux driver simply enables VBUSVLD when device mode is
selected, and disables it if host mode is selected, and when the
selection is cleared (disconnection).


Br,

-- 
heikki

Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

[kbuild test robot]

Hi Manu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180314]
[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/Manu-Gautam/dt-bindings-usb-Update-documentation-for-Qualcomm-DWC3-driver/20180314-095557
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> ERROR: "__tracepoint_dwc3_writel" [drivers/usb/dwc3/dwc3-qcom.ko] undefined!
>> ERROR: "__tracepoint_dwc3_readl" [drivers/usb/dwc3/dwc3-qcom.ko] undefined!

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

[kbuild test robot]

Hi Manu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180314]
[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/Manu-Gautam/dt-bindings-usb-Update-documentation-for-Qualcomm-DWC3-driver/20180314-095557
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> ERROR: "__tracepoint_dwc3_writel" [drivers/usb/dwc3/dwc3-qcom.ko] undefined!
>> ERROR: "__tracepoint_dwc3_readl" [drivers/usb/dwc3/dwc3-qcom.ko] undefined!

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

[Felipe Balbi]

Hi,

Manu Gautam  writes:
> Hi,
>
>
> On 3/13/2018 4:38 PM, Felipe Balbi wrote:
>> Hi,
>>
>> +Andy
>>
>> Manu Gautam  writes:
>>> DWC3 controller on Qualcomm SOCs has a Qscratch wrapper.
>>> Some of its uses are described below resulting in need to
>>> have a separate glue driver instead of using dwc3-of-simple:
>>>  - It exposes register interface to override vbus-override
>>>and lane0-pwr-present signals going to hardware. These
>>>must be updated in peripheral mode for DWC3 if vbus lines
>>>are not connected to hardware block. Otherwise RX termination
>>>in SS mode or DP pull-up is not applied by device controller.
>> right, core needs to know that VBUS is above 4.4V. Why wasn't this a
>> problem when the original glue layer was first published?
>
> Thanks for reviewing.
> Original glue layer supported only host mode, hence this wasn't needed.

okay

>>>  - pwr_events_irq_stat support to ensure USB2 PHY is in L2 state
>>>before glue driver can turn-off clocks and suspend PHY.
>> Core manages PHY suspend automatically. Isn't that working for you? Why?
>
> Yes, it is not supported with QUSB2 PHY (usb2-phy on Qualcomm SOCs).

but the PHY doesn't need to support it, DWC3 does :-)

> Issue is that If core suspends USB2 PHY (say in host mode if some SS device 
> connected),
> USB2 PHY stops responding to any attach event as it can't exit suspend state 
> by itself.

Okay, so we miss remote wakeup events. Fair enough.

>>>  - Support to replace pip3 clock going to DWC3 with utmi clock
>>>for hardware configuration where SSPHY is not used with DWC3.
>> Is that SW configurable? Really? In any case seems like this and SESSVLD
>> valid should be handled using Hans' and Heikki's mux support.
>
> Yes, with this we can use dwc3 without using SSPHY. Please point me to
> those patches. There are only bunch of register writes in glue wrapper to
> achieve the same.

https://www.spinics.net/lists/linux-usb/msg160868.html

>>> +static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
>>> +{
>>> +   struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
>> nope! Glue shouldn't touch dwc3 at all.
> Other than PHY handles, need this to fail runtime suspend if dwc3 hasn't
> probed yet.

Will that even happen? You should pm_runtime_forbid() by default,
anyway and expect it to be enabled via sysfs later, no?

>>> +   dwc3_qcom_suspend_hsphy(qcom);
>>> +
>>> +   if (dwc->usb2_generic_phy)
>>> +   phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
>>> +   if (dwc->usb3_generic_phy)
>>> +   phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
>> core.c should do this.
> Recommended sequence from h/w programming guide is that:
> 1. PHY autosuspend must be left disabled - 
> snps,dis_u2_susphy_quirk/dis_enblslpm_quirk
> 2. During runtime-suspend (say on xhci bus_suspend) , PHY should be suspended
>     using GUSB2PHYCFG register

this is something that dwc3 core can do on its own suspend implementation.

> 3. Wait until pwr_event_irq_stat in qscratch reflects PHY transition to L2.

this is interesting part. Is this register accessible by the PHY driver?
Seems like that would be the best place to stuff this...

> 3. USB2 PHY driver can suspend - enable wakeup events and turns off clocks 
> etc.

... together with this.

> 4. dwc3 glue driver can suspend.
>
> Since, pwr_event_irq stat can't be checked in core driver, I added this 
> handling
> in glue driver. Alternative approach I can think of is to let dwc3 core 
> suspend
> PHY using GUSBPHYCFG register on suspend,  add some delay before
> suspending PHY. Glue driver can check for pwr_event_irq stat and throw a
> warning if PHY not in L2.

almost :-) core_suspend fiddles with GUSB2PHYCFG for suspend and calls
phy_suspend() (or whatever the function is called heh), that will go to
your phy driver's suspend callback, which checks pwr_event_irq_stat and
then pm_runtime_put() to schedule ->runtime_suspend() so that can enable
wakeups and switch off clocks.

>>> +   irq = platform_get_irq_byname(pdev, "dp_hs_phy_irq");
>>> +   if (irq > 0) {
>>> +   irq_set_status_flags(irq, IRQ_NOAUTOEN);
>> why do you need to set this flag?
> These wakeup_irqs should be enabled only during suspend. With this flag I
> don't need to disable irq immediately after requesting it.

oh, okay. You may want to add a comment here :-)

>
>>
>>> +   ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
>>> +   qcom_dwc3_resume_irq,
>>> +   IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
>>> +   "qcom_dwc3 DP_HS", qcom);
>> this is the same as devm_request_irq()
> I am passing hard_irq handler as NULL whereas thread_fn is not NULL.
> devm_request_irq is just the opposite.

oh, indeed. I misparsed it.

>>> +static const struct dev_pm_ops dwc3_qcom_dev_pm_ops = {
>>> +   SET_SYSTEM_SLEEP_PM_OPS(dwc3_qcom_pm_suspend, dwc3_qcom_pm_resume)
>>> + 

Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

[Felipe Balbi]

Hi,

Manu Gautam  writes:
> Hi,
>
>
> On 3/13/2018 4:38 PM, Felipe Balbi wrote:
>> Hi,
>>
>> +Andy
>>
>> Manu Gautam  writes:
>>> DWC3 controller on Qualcomm SOCs has a Qscratch wrapper.
>>> Some of its uses are described below resulting in need to
>>> have a separate glue driver instead of using dwc3-of-simple:
>>>  - It exposes register interface to override vbus-override
>>>and lane0-pwr-present signals going to hardware. These
>>>must be updated in peripheral mode for DWC3 if vbus lines
>>>are not connected to hardware block. Otherwise RX termination
>>>in SS mode or DP pull-up is not applied by device controller.
>> right, core needs to know that VBUS is above 4.4V. Why wasn't this a
>> problem when the original glue layer was first published?
>
> Thanks for reviewing.
> Original glue layer supported only host mode, hence this wasn't needed.

okay

>>>  - pwr_events_irq_stat support to ensure USB2 PHY is in L2 state
>>>before glue driver can turn-off clocks and suspend PHY.
>> Core manages PHY suspend automatically. Isn't that working for you? Why?
>
> Yes, it is not supported with QUSB2 PHY (usb2-phy on Qualcomm SOCs).

but the PHY doesn't need to support it, DWC3 does :-)

> Issue is that If core suspends USB2 PHY (say in host mode if some SS device 
> connected),
> USB2 PHY stops responding to any attach event as it can't exit suspend state 
> by itself.

Okay, so we miss remote wakeup events. Fair enough.

>>>  - Support to replace pip3 clock going to DWC3 with utmi clock
>>>for hardware configuration where SSPHY is not used with DWC3.
>> Is that SW configurable? Really? In any case seems like this and SESSVLD
>> valid should be handled using Hans' and Heikki's mux support.
>
> Yes, with this we can use dwc3 without using SSPHY. Please point me to
> those patches. There are only bunch of register writes in glue wrapper to
> achieve the same.

https://www.spinics.net/lists/linux-usb/msg160868.html

>>> +static int dwc3_qcom_suspend(struct dwc3_qcom *qcom)
>>> +{
>>> +   struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
>> nope! Glue shouldn't touch dwc3 at all.
> Other than PHY handles, need this to fail runtime suspend if dwc3 hasn't
> probed yet.

Will that even happen? You should pm_runtime_forbid() by default,
anyway and expect it to be enabled via sysfs later, no?

>>> +   dwc3_qcom_suspend_hsphy(qcom);
>>> +
>>> +   if (dwc->usb2_generic_phy)
>>> +   phy_pm_runtime_put_sync(dwc->usb2_generic_phy);
>>> +   if (dwc->usb3_generic_phy)
>>> +   phy_pm_runtime_put_sync(dwc->usb3_generic_phy);
>> core.c should do this.
> Recommended sequence from h/w programming guide is that:
> 1. PHY autosuspend must be left disabled - 
> snps,dis_u2_susphy_quirk/dis_enblslpm_quirk
> 2. During runtime-suspend (say on xhci bus_suspend) , PHY should be suspended
>     using GUSB2PHYCFG register

this is something that dwc3 core can do on its own suspend implementation.

> 3. Wait until pwr_event_irq_stat in qscratch reflects PHY transition to L2.

this is interesting part. Is this register accessible by the PHY driver?
Seems like that would be the best place to stuff this...

> 3. USB2 PHY driver can suspend - enable wakeup events and turns off clocks 
> etc.

... together with this.

> 4. dwc3 glue driver can suspend.
>
> Since, pwr_event_irq stat can't be checked in core driver, I added this 
> handling
> in glue driver. Alternative approach I can think of is to let dwc3 core 
> suspend
> PHY using GUSBPHYCFG register on suspend,  add some delay before
> suspending PHY. Glue driver can check for pwr_event_irq stat and throw a
> warning if PHY not in L2.

almost :-) core_suspend fiddles with GUSB2PHYCFG for suspend and calls
phy_suspend() (or whatever the function is called heh), that will go to
your phy driver's suspend callback, which checks pwr_event_irq_stat and
then pm_runtime_put() to schedule ->runtime_suspend() so that can enable
wakeups and switch off clocks.

>>> +   irq = platform_get_irq_byname(pdev, "dp_hs_phy_irq");
>>> +   if (irq > 0) {
>>> +   irq_set_status_flags(irq, IRQ_NOAUTOEN);
>> why do you need to set this flag?
> These wakeup_irqs should be enabled only during suspend. With this flag I
> don't need to disable irq immediately after requesting it.

oh, okay. You may want to add a comment here :-)

>
>>
>>> +   ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
>>> +   qcom_dwc3_resume_irq,
>>> +   IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
>>> +   "qcom_dwc3 DP_HS", qcom);
>> this is the same as devm_request_irq()
> I am passing hard_irq handler as NULL whereas thread_fn is not NULL.
> devm_request_irq is just the opposite.

oh, indeed. I misparsed it.

>>> +static const struct dev_pm_ops dwc3_qcom_dev_pm_ops = {
>>> +   SET_SYSTEM_SLEEP_PM_OPS(dwc3_qcom_pm_suspend, dwc3_qcom_pm_resume)
>>> +   SET_RUNTIME_PM_OPS(dwc3_qcom_runtime_suspend, 

Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

[Manu Gautam]

Hi,


On 3/13/2018 4:38 PM, Felipe Balbi wrote:
> Hi,
>
> +Andy
>
> Manu Gautam  writes:
>> DWC3 controller on Qualcomm SOCs has a Qscratch wrapper.
>> Some of its uses are described below resulting in need to
>> have a separate glue driver instead of using dwc3-of-simple:
>>  - It exposes register interface to override vbus-override
>>and lane0-pwr-present signals going to hardware. These
>>must be updated in peripheral mode for DWC3 if vbus lines
>>are not connected to hardware block. Otherwise RX termination
>>in SS mode or DP pull-up is not applied by device controller.
> right, core needs to know that VBUS is above 4.4V. Why wasn't this a
> problem when the original glue layer was first published?

Thanks for reviewing.
Original glue layer supported only host mode, hence this wasn't needed.

>
>>  - pwr_events_irq_stat support to ensure USB2 PHY is in L2 state
>>before glue driver can turn-off clocks and suspend PHY.
> Core manages PHY suspend automatically. Isn't that working for you? Why?

Yes, it is not supported with QUSB2 PHY (usb2-phy on Qualcomm SOCs).
Issue is that If core suspends USB2 PHY (say in host mode if some SS device 
connected),
USB2 PHY stops responding to any attach event as it can't exit suspend state by 
itself.
But in case of driver suspend, along with PHY suspend wakeup events are enabled.
Glue driver can register for wakeup interrupts to exit suspend.

>
>>  - Support for wakeup interrupts lines that are asserted whenever
>>there is any wakeup event on USB3 or USB2 bus.
> ok
>
>>  - Support to replace pip3 clock going to DWC3 with utmi clock
>>for hardware configuration where SSPHY is not used with DWC3.
> Is that SW configurable? Really? In any case seems like this and SESSVLD
> valid should be handled using Hans' and Heikki's mux support.

Yes, with this we can use dwc3 without using SSPHY. Please point me to
those patches. There are only bunch of register writes in glue wrapper to
achieve the same.

>
>> Other than above hardware features in Qscratch wrapper there
>> are some limitations on QCOM SOCs that require special handling
>> of power management e.g. suspending PHY using GUSB2PHYCFG
>> register and ensuring PHY enters L2 before turning off clocks etc.
>>
>> Signed-off-by: Manu Gautam 
>> ---
>>  drivers/usb/dwc3/Kconfig  |  11 +
>>  drivers/usb/dwc3/Makefile |   1 +
>>  drivers/usb/dwc3/dwc3-of-simple.c |   1 -
>>  drivers/usb/dwc3/dwc3-qcom.c  | 635 
>> ++
>>  4 files changed, 647 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/usb/dwc3/dwc3-qcom.c
>>
>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>> index ab8c0e0..f5bb4f1 100644
>> --- a/drivers/usb/dwc3/Kconfig
>> +++ b/drivers/usb/dwc3/Kconfig
>> @@ -106,4 +106,15 @@ config USB_DWC3_ST
>>inside (i.e. STiH407).
>>Say 'Y' or 'M' if you have one such device.
>>  
>> +config USB_DWC3_QCOM
>> +tristate "Qualcomm Platform"
>> +depends on ARCH_QCOM || COMPILE_TEST
>> +depends on OF
>> +default USB_DWC3
>> +help
>> +  Some Qualcomm SoCs use DesignWare Core IP for USB2/3
>> +  functionality.
>> +
>> +  Say 'Y' or 'M' if you have one such device.
>> +
>>  endif
>> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
>> index 7ac7250..c3ce697 100644
>> --- a/drivers/usb/dwc3/Makefile
>> +++ b/drivers/usb/dwc3/Makefile
>> @@ -48,3 +48,4 @@ obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o
>>  obj-$(CONFIG_USB_DWC3_KEYSTONE) += dwc3-keystone.o
>>  obj-$(CONFIG_USB_DWC3_OF_SIMPLE)+= dwc3-of-simple.o
>>  obj-$(CONFIG_USB_DWC3_ST)   += dwc3-st.o
>> +obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o
>> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
>> b/drivers/usb/dwc3/dwc3-of-simple.c
>> index cb2ee96..0fd0e8e 100644
>> --- a/drivers/usb/dwc3/dwc3-of-simple.c
>> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
>> @@ -208,7 +208,6 @@ static int dwc3_of_simple_runtime_resume(struct device 
>> *dev)
>>  };
>>  
>>  static const struct of_device_id of_dwc3_simple_match[] = {
>> -{ .compatible = "qcom,dwc3" },
>>  { .compatible = "rockchip,rk3399-dwc3" },
>>  { .compatible = "xlnx,zynqmp-dwc3" },
>>  { .compatible = "cavium,octeon-7130-usb-uctl" },
>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>> new file mode 100644
>> index 000..917199e
>> --- /dev/null
>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>> @@ -0,0 +1,635 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
>> + *
>> + * Inspired by dwc3-of-simple.c
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "core.h"
>> +#include "io.h"
> you must be kidding, right? 

Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

[Manu Gautam]

Hi,


On 3/13/2018 4:38 PM, Felipe Balbi wrote:
> Hi,
>
> +Andy
>
> Manu Gautam  writes:
>> DWC3 controller on Qualcomm SOCs has a Qscratch wrapper.
>> Some of its uses are described below resulting in need to
>> have a separate glue driver instead of using dwc3-of-simple:
>>  - It exposes register interface to override vbus-override
>>and lane0-pwr-present signals going to hardware. These
>>must be updated in peripheral mode for DWC3 if vbus lines
>>are not connected to hardware block. Otherwise RX termination
>>in SS mode or DP pull-up is not applied by device controller.
> right, core needs to know that VBUS is above 4.4V. Why wasn't this a
> problem when the original glue layer was first published?

Thanks for reviewing.
Original glue layer supported only host mode, hence this wasn't needed.

>
>>  - pwr_events_irq_stat support to ensure USB2 PHY is in L2 state
>>before glue driver can turn-off clocks and suspend PHY.
> Core manages PHY suspend automatically. Isn't that working for you? Why?

Yes, it is not supported with QUSB2 PHY (usb2-phy on Qualcomm SOCs).
Issue is that If core suspends USB2 PHY (say in host mode if some SS device 
connected),
USB2 PHY stops responding to any attach event as it can't exit suspend state by 
itself.
But in case of driver suspend, along with PHY suspend wakeup events are enabled.
Glue driver can register for wakeup interrupts to exit suspend.

>
>>  - Support for wakeup interrupts lines that are asserted whenever
>>there is any wakeup event on USB3 or USB2 bus.
> ok
>
>>  - Support to replace pip3 clock going to DWC3 with utmi clock
>>for hardware configuration where SSPHY is not used with DWC3.
> Is that SW configurable? Really? In any case seems like this and SESSVLD
> valid should be handled using Hans' and Heikki's mux support.

Yes, with this we can use dwc3 without using SSPHY. Please point me to
those patches. There are only bunch of register writes in glue wrapper to
achieve the same.

>
>> Other than above hardware features in Qscratch wrapper there
>> are some limitations on QCOM SOCs that require special handling
>> of power management e.g. suspending PHY using GUSB2PHYCFG
>> register and ensuring PHY enters L2 before turning off clocks etc.
>>
>> Signed-off-by: Manu Gautam 
>> ---
>>  drivers/usb/dwc3/Kconfig  |  11 +
>>  drivers/usb/dwc3/Makefile |   1 +
>>  drivers/usb/dwc3/dwc3-of-simple.c |   1 -
>>  drivers/usb/dwc3/dwc3-qcom.c  | 635 
>> ++
>>  4 files changed, 647 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/usb/dwc3/dwc3-qcom.c
>>
>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>> index ab8c0e0..f5bb4f1 100644
>> --- a/drivers/usb/dwc3/Kconfig
>> +++ b/drivers/usb/dwc3/Kconfig
>> @@ -106,4 +106,15 @@ config USB_DWC3_ST
>>inside (i.e. STiH407).
>>Say 'Y' or 'M' if you have one such device.
>>  
>> +config USB_DWC3_QCOM
>> +tristate "Qualcomm Platform"
>> +depends on ARCH_QCOM || COMPILE_TEST
>> +depends on OF
>> +default USB_DWC3
>> +help
>> +  Some Qualcomm SoCs use DesignWare Core IP for USB2/3
>> +  functionality.
>> +
>> +  Say 'Y' or 'M' if you have one such device.
>> +
>>  endif
>> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
>> index 7ac7250..c3ce697 100644
>> --- a/drivers/usb/dwc3/Makefile
>> +++ b/drivers/usb/dwc3/Makefile
>> @@ -48,3 +48,4 @@ obj-$(CONFIG_USB_DWC3_PCI) += dwc3-pci.o
>>  obj-$(CONFIG_USB_DWC3_KEYSTONE) += dwc3-keystone.o
>>  obj-$(CONFIG_USB_DWC3_OF_SIMPLE)+= dwc3-of-simple.o
>>  obj-$(CONFIG_USB_DWC3_ST)   += dwc3-st.o
>> +obj-$(CONFIG_USB_DWC3_QCOM) += dwc3-qcom.o
>> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
>> b/drivers/usb/dwc3/dwc3-of-simple.c
>> index cb2ee96..0fd0e8e 100644
>> --- a/drivers/usb/dwc3/dwc3-of-simple.c
>> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
>> @@ -208,7 +208,6 @@ static int dwc3_of_simple_runtime_resume(struct device 
>> *dev)
>>  };
>>  
>>  static const struct of_device_id of_dwc3_simple_match[] = {
>> -{ .compatible = "qcom,dwc3" },
>>  { .compatible = "rockchip,rk3399-dwc3" },
>>  { .compatible = "xlnx,zynqmp-dwc3" },
>>  { .compatible = "cavium,octeon-7130-usb-uctl" },
>> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
>> new file mode 100644
>> index 000..917199e
>> --- /dev/null
>> +++ b/drivers/usb/dwc3/dwc3-qcom.c
>> @@ -0,0 +1,635 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
>> + *
>> + * Inspired by dwc3-of-simple.c
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "core.h"
>> +#include "io.h"
> you must be kidding, right? There's no way I'm gonna let a glue poke at
> 

Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

[kbuild test robot]

Hi Manu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180313]
[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/Manu-Gautam/dt-bindings-usb-Update-documentation-for-Qualcomm-DWC3-driver/20180314-095557
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

   drivers/usb/dwc3/dwc3-qcom.c: In function 'dwc3_qcom_probe':
>> drivers/usb/dwc3/dwc3-qcom.c:407:33: error: implicit declaration of function 
>> 'of_clk_get_parent_count'; did you mean 'clk_get_parent'? 
>> [-Werror=implicit-function-declaration]
 ret = dwc3_qcom_clk_init(qcom, of_clk_get_parent_count(np));
^~~
clk_get_parent
   cc1: some warnings being treated as errors

vim +407 drivers/usb/dwc3/dwc3-qcom.c

   380  
   381  static int dwc3_qcom_probe(struct platform_device *pdev)
   382  {
   383  struct device_node  *np = pdev->dev.of_node, *dwc3_np;
   384  struct dwc3_qcom*qcom;
   385  struct resource *res;
   386  int irq, ret, i;
   387  boolignore_pipe_clk;
   388  
   389  qcom = devm_kzalloc(>dev, sizeof(*qcom), GFP_KERNEL);
   390  if (!qcom)
   391  return -ENOMEM;
   392  
   393  platform_set_drvdata(pdev, qcom);
   394  qcom->dev = >dev;
   395  
   396  qcom->resets = 
of_reset_control_array_get_optional_exclusive(np);
   397  if (IS_ERR(qcom->resets)) {
   398  ret = PTR_ERR(qcom->resets);
   399  dev_err(>dev, "failed to get resets, err=%d\n", 
ret);
   400  return ret;
   401  }
   402  
   403  ret = reset_control_deassert(qcom->resets);
   404  if (ret)
   405  goto reset_put;
   406  
 > 407  ret = dwc3_qcom_clk_init(qcom, of_clk_get_parent_count(np));
   408  if (ret) {
   409  dev_err(qcom->dev, "failed to get clocks\n");
   410  goto reset_assert;
   411  }
   412  
   413  res = platform_get_resource_byname(pdev, IORESOURCE_MEM, 
"qscratch");
   414  qcom->qscratch_base = devm_ioremap_resource(qcom->dev, res);
   415  if (IS_ERR(qcom->qscratch_base)) {
   416  dev_err(qcom->dev, "failed to map qscratch - %d\n", 
ret);
   417  ret = PTR_ERR(qcom->qscratch_base);
   418  goto clk_disable;
   419  }
   420  
   421  irq = platform_get_irq_byname(pdev, "hs_phy_irq");
   422  if (irq > 0) {
   423  ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
   424  qcom_dwc3_resume_irq,
   425  IRQF_TRIGGER_HIGH | 
IRQF_ONESHOT,
   426  "qcom_dwc3 HS", qcom);
   427  if (ret) {
   428  dev_err(qcom->dev, "hs_phy_irq failed: %d\n", 
ret);
   429  goto clk_disable;
   430  }
   431  }
   432  
   433  irq = platform_get_irq_byname(pdev, "dp_hs_phy_irq");
   434  if (irq > 0) {
   435  irq_set_status_flags(irq, IRQ_NOAUTOEN);
   436  ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
   437  qcom_dwc3_resume_irq,
   438  IRQF_TRIGGER_HIGH | 
IRQF_ONESHOT,
   439  "qcom_dwc3 DP_HS", qcom);
   440  if (ret) {
   441  dev_err(qcom->dev, "dp_hs_phy_irq failed: 
%d\n", ret);
   442  goto clk_disable;
   443  }
   444  qcom->dp_hs_phy_irq = irq;
   445  }
   446  
   447  irq = platform_get_irq_byname(pdev, "dm_hs_phy_irq");
   448  if (irq > 0) {
   449  irq_set_status_flags(irq, IRQ_NOAUTOEN);
   450  ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
   451  qcom_dwc3_resume_irq,
   452  IRQF_TRIGGER_HIGH | 
IRQF_ONESHOT,
   453  "qcom_dwc3 DM_HS", qcom);
   454  if (ret) {
   455  dev_err(qcom->dev, "dm_hs_phy_irq failed: 
%d\n", ret);
   456

Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

[kbuild test robot]

Hi Manu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[also build test ERROR on next-20180313]
[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/Manu-Gautam/dt-bindings-usb-Update-documentation-for-Qualcomm-DWC3-driver/20180314-095557
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64 

All errors (new ones prefixed by >>):

   drivers/usb/dwc3/dwc3-qcom.c: In function 'dwc3_qcom_probe':
>> drivers/usb/dwc3/dwc3-qcom.c:407:33: error: implicit declaration of function 
>> 'of_clk_get_parent_count'; did you mean 'clk_get_parent'? 
>> [-Werror=implicit-function-declaration]
 ret = dwc3_qcom_clk_init(qcom, of_clk_get_parent_count(np));
^~~
clk_get_parent
   cc1: some warnings being treated as errors

vim +407 drivers/usb/dwc3/dwc3-qcom.c

   380  
   381  static int dwc3_qcom_probe(struct platform_device *pdev)
   382  {
   383  struct device_node  *np = pdev->dev.of_node, *dwc3_np;
   384  struct dwc3_qcom*qcom;
   385  struct resource *res;
   386  int irq, ret, i;
   387  boolignore_pipe_clk;
   388  
   389  qcom = devm_kzalloc(>dev, sizeof(*qcom), GFP_KERNEL);
   390  if (!qcom)
   391  return -ENOMEM;
   392  
   393  platform_set_drvdata(pdev, qcom);
   394  qcom->dev = >dev;
   395  
   396  qcom->resets = 
of_reset_control_array_get_optional_exclusive(np);
   397  if (IS_ERR(qcom->resets)) {
   398  ret = PTR_ERR(qcom->resets);
   399  dev_err(>dev, "failed to get resets, err=%d\n", 
ret);
   400  return ret;
   401  }
   402  
   403  ret = reset_control_deassert(qcom->resets);
   404  if (ret)
   405  goto reset_put;
   406  
 > 407  ret = dwc3_qcom_clk_init(qcom, of_clk_get_parent_count(np));
   408  if (ret) {
   409  dev_err(qcom->dev, "failed to get clocks\n");
   410  goto reset_assert;
   411  }
   412  
   413  res = platform_get_resource_byname(pdev, IORESOURCE_MEM, 
"qscratch");
   414  qcom->qscratch_base = devm_ioremap_resource(qcom->dev, res);
   415  if (IS_ERR(qcom->qscratch_base)) {
   416  dev_err(qcom->dev, "failed to map qscratch - %d\n", 
ret);
   417  ret = PTR_ERR(qcom->qscratch_base);
   418  goto clk_disable;
   419  }
   420  
   421  irq = platform_get_irq_byname(pdev, "hs_phy_irq");
   422  if (irq > 0) {
   423  ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
   424  qcom_dwc3_resume_irq,
   425  IRQF_TRIGGER_HIGH | 
IRQF_ONESHOT,
   426  "qcom_dwc3 HS", qcom);
   427  if (ret) {
   428  dev_err(qcom->dev, "hs_phy_irq failed: %d\n", 
ret);
   429  goto clk_disable;
   430  }
   431  }
   432  
   433  irq = platform_get_irq_byname(pdev, "dp_hs_phy_irq");
   434  if (irq > 0) {
   435  irq_set_status_flags(irq, IRQ_NOAUTOEN);
   436  ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
   437  qcom_dwc3_resume_irq,
   438  IRQF_TRIGGER_HIGH | 
IRQF_ONESHOT,
   439  "qcom_dwc3 DP_HS", qcom);
   440  if (ret) {
   441  dev_err(qcom->dev, "dp_hs_phy_irq failed: 
%d\n", ret);
   442  goto clk_disable;
   443  }
   444  qcom->dp_hs_phy_irq = irq;
   445  }
   446  
   447  irq = platform_get_irq_byname(pdev, "dm_hs_phy_irq");
   448  if (irq > 0) {
   449  irq_set_status_flags(irq, IRQ_NOAUTOEN);
   450  ret = devm_request_threaded_irq(qcom->dev, irq, NULL,
   451  qcom_dwc3_resume_irq,
   452  IRQF_TRIGGER_HIGH | 
IRQF_ONESHOT,
   453  "qcom_dwc3 DM_HS", qcom);
   454  if (ret) {
   455  dev_err(qcom->dev, "dm_hs_phy_irq failed: 
%d\n", ret);
   456

Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

[Felipe Balbi]

Hi,

+Andy

Manu Gautam  writes:
> DWC3 controller on Qualcomm SOCs has a Qscratch wrapper.
> Some of its uses are described below resulting in need to
> have a separate glue driver instead of using dwc3-of-simple:
>  - It exposes register interface to override vbus-override
>and lane0-pwr-present signals going to hardware. These
>must be updated in peripheral mode for DWC3 if vbus lines
>are not connected to hardware block. Otherwise RX termination
>in SS mode or DP pull-up is not applied by device controller.

right, core needs to know that VBUS is above 4.4V. Why wasn't this a
problem when the original glue layer was first published?

>  - pwr_events_irq_stat support to ensure USB2 PHY is in L2 state
>before glue driver can turn-off clocks and suspend PHY.

Core manages PHY suspend automatically. Isn't that working for you? Why?

>  - Support for wakeup interrupts lines that are asserted whenever
>there is any wakeup event on USB3 or USB2 bus.

ok

>  - Support to replace pip3 clock going to DWC3 with utmi clock
>for hardware configuration where SSPHY is not used with DWC3.

Is that SW configurable? Really? In any case seems like this and SESSVLD
valid should be handled using Hans' and Heikki's mux support.

> Other than above hardware features in Qscratch wrapper there
> are some limitations on QCOM SOCs that require special handling
> of power management e.g. suspending PHY using GUSB2PHYCFG
> register and ensuring PHY enters L2 before turning off clocks etc.
>
> Signed-off-by: Manu Gautam 
> ---
>  drivers/usb/dwc3/Kconfig  |  11 +
>  drivers/usb/dwc3/Makefile |   1 +
>  drivers/usb/dwc3/dwc3-of-simple.c |   1 -
>  drivers/usb/dwc3/dwc3-qcom.c  | 635 
> ++
>  4 files changed, 647 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/usb/dwc3/dwc3-qcom.c
>
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index ab8c0e0..f5bb4f1 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -106,4 +106,15 @@ config USB_DWC3_ST
> inside (i.e. STiH407).
> Say 'Y' or 'M' if you have one such device.
>  
> +config USB_DWC3_QCOM
> + tristate "Qualcomm Platform"
> + depends on ARCH_QCOM || COMPILE_TEST
> + depends on OF
> + default USB_DWC3
> + help
> +   Some Qualcomm SoCs use DesignWare Core IP for USB2/3
> +   functionality.
> +
> +   Say 'Y' or 'M' if you have one such device.
> +
>  endif
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index 7ac7250..c3ce697 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -48,3 +48,4 @@ obj-$(CONFIG_USB_DWC3_PCI)  += dwc3-pci.o
>  obj-$(CONFIG_USB_DWC3_KEYSTONE)  += dwc3-keystone.o
>  obj-$(CONFIG_USB_DWC3_OF_SIMPLE) += dwc3-of-simple.o
>  obj-$(CONFIG_USB_DWC3_ST)+= dwc3-st.o
> +obj-$(CONFIG_USB_DWC3_QCOM)  += dwc3-qcom.o
> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
> b/drivers/usb/dwc3/dwc3-of-simple.c
> index cb2ee96..0fd0e8e 100644
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -208,7 +208,6 @@ static int dwc3_of_simple_runtime_resume(struct device 
> *dev)
>  };
>  
>  static const struct of_device_id of_dwc3_simple_match[] = {
> - { .compatible = "qcom,dwc3" },
>   { .compatible = "rockchip,rk3399-dwc3" },
>   { .compatible = "xlnx,zynqmp-dwc3" },
>   { .compatible = "cavium,octeon-7130-usb-uctl" },
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> new file mode 100644
> index 000..917199e
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -0,0 +1,635 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + *
> + * Inspired by dwc3-of-simple.c
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "core.h"
> +#include "io.h"

you must be kidding, right? There's no way I'm gonna let a glue poke at
core registers.

> +static void dwc3_qcom_suspend_hsphy(struct dwc3_qcom *qcom)
> +{
> + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
> + int ret;
> + u32 val;
> +
> + /* Clear previous L2 events */
> + dwc3_qcom_setbits(qcom->qscratch_base, PWR_EVNT_IRQ_STAT_REG,
> +   PWR_EVNT_LPM_IN_L2_MASK | PWR_EVNT_LPM_OUT_L2_MASK);
> +
> + /* Allow controller to suspend HSPHY */
> + val = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> + val |=  DWC3_GUSB2PHYCFG_ENBLSLPM | DWC3_GUSB2PHYCFG_SUSPHY;
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), val);

core should handle PHY bits. In any case, why don't you let core handle
PHY suspend? It handles it automatically for us, 

Re: [PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

[Felipe Balbi]

Hi,

+Andy

Manu Gautam  writes:
> DWC3 controller on Qualcomm SOCs has a Qscratch wrapper.
> Some of its uses are described below resulting in need to
> have a separate glue driver instead of using dwc3-of-simple:
>  - It exposes register interface to override vbus-override
>and lane0-pwr-present signals going to hardware. These
>must be updated in peripheral mode for DWC3 if vbus lines
>are not connected to hardware block. Otherwise RX termination
>in SS mode or DP pull-up is not applied by device controller.

right, core needs to know that VBUS is above 4.4V. Why wasn't this a
problem when the original glue layer was first published?

>  - pwr_events_irq_stat support to ensure USB2 PHY is in L2 state
>before glue driver can turn-off clocks and suspend PHY.

Core manages PHY suspend automatically. Isn't that working for you? Why?

>  - Support for wakeup interrupts lines that are asserted whenever
>there is any wakeup event on USB3 or USB2 bus.

ok

>  - Support to replace pip3 clock going to DWC3 with utmi clock
>for hardware configuration where SSPHY is not used with DWC3.

Is that SW configurable? Really? In any case seems like this and SESSVLD
valid should be handled using Hans' and Heikki's mux support.

> Other than above hardware features in Qscratch wrapper there
> are some limitations on QCOM SOCs that require special handling
> of power management e.g. suspending PHY using GUSB2PHYCFG
> register and ensuring PHY enters L2 before turning off clocks etc.
>
> Signed-off-by: Manu Gautam 
> ---
>  drivers/usb/dwc3/Kconfig  |  11 +
>  drivers/usb/dwc3/Makefile |   1 +
>  drivers/usb/dwc3/dwc3-of-simple.c |   1 -
>  drivers/usb/dwc3/dwc3-qcom.c  | 635 
> ++
>  4 files changed, 647 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/usb/dwc3/dwc3-qcom.c
>
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index ab8c0e0..f5bb4f1 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -106,4 +106,15 @@ config USB_DWC3_ST
> inside (i.e. STiH407).
> Say 'Y' or 'M' if you have one such device.
>  
> +config USB_DWC3_QCOM
> + tristate "Qualcomm Platform"
> + depends on ARCH_QCOM || COMPILE_TEST
> + depends on OF
> + default USB_DWC3
> + help
> +   Some Qualcomm SoCs use DesignWare Core IP for USB2/3
> +   functionality.
> +
> +   Say 'Y' or 'M' if you have one such device.
> +
>  endif
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index 7ac7250..c3ce697 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -48,3 +48,4 @@ obj-$(CONFIG_USB_DWC3_PCI)  += dwc3-pci.o
>  obj-$(CONFIG_USB_DWC3_KEYSTONE)  += dwc3-keystone.o
>  obj-$(CONFIG_USB_DWC3_OF_SIMPLE) += dwc3-of-simple.o
>  obj-$(CONFIG_USB_DWC3_ST)+= dwc3-st.o
> +obj-$(CONFIG_USB_DWC3_QCOM)  += dwc3-qcom.o
> diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
> b/drivers/usb/dwc3/dwc3-of-simple.c
> index cb2ee96..0fd0e8e 100644
> --- a/drivers/usb/dwc3/dwc3-of-simple.c
> +++ b/drivers/usb/dwc3/dwc3-of-simple.c
> @@ -208,7 +208,6 @@ static int dwc3_of_simple_runtime_resume(struct device 
> *dev)
>  };
>  
>  static const struct of_device_id of_dwc3_simple_match[] = {
> - { .compatible = "qcom,dwc3" },
>   { .compatible = "rockchip,rk3399-dwc3" },
>   { .compatible = "xlnx,zynqmp-dwc3" },
>   { .compatible = "cavium,octeon-7130-usb-uctl" },
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> new file mode 100644
> index 000..917199e
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -0,0 +1,635 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + *
> + * Inspired by dwc3-of-simple.c
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "core.h"
> +#include "io.h"

you must be kidding, right? There's no way I'm gonna let a glue poke at
core registers.

> +static void dwc3_qcom_suspend_hsphy(struct dwc3_qcom *qcom)
> +{
> + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
> + int ret;
> + u32 val;
> +
> + /* Clear previous L2 events */
> + dwc3_qcom_setbits(qcom->qscratch_base, PWR_EVNT_IRQ_STAT_REG,
> +   PWR_EVNT_LPM_IN_L2_MASK | PWR_EVNT_LPM_OUT_L2_MASK);
> +
> + /* Allow controller to suspend HSPHY */
> + val = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> + val |=  DWC3_GUSB2PHYCFG_ENBLSLPM | DWC3_GUSB2PHYCFG_SUSPHY;
> + dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), val);

core should handle PHY bits. In any case, why don't you let core handle
PHY suspend? It handles it automatically for us, there should be no need
for SW intervention.

> 

[PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

[Manu Gautam]

DWC3 controller on Qualcomm SOCs has a Qscratch wrapper.
Some of its uses are described below resulting in need to
have a separate glue driver instead of using dwc3-of-simple:
 - It exposes register interface to override vbus-override
   and lane0-pwr-present signals going to hardware. These
   must be updated in peripheral mode for DWC3 if vbus lines
   are not connected to hardware block. Otherwise RX termination
   in SS mode or DP pull-up is not applied by device controller.
 - pwr_events_irq_stat support to ensure USB2 PHY is in L2 state
   before glue driver can turn-off clocks and suspend PHY.
 - Support for wakeup interrupts lines that are asserted whenever
   there is any wakeup event on USB3 or USB2 bus.
 - Support to replace pip3 clock going to DWC3 with utmi clock
   for hardware configuration where SSPHY is not used with DWC3.

Other than above hardware features in Qscratch wrapper there
are some limitations on QCOM SOCs that require special handling
of power management e.g. suspending PHY using GUSB2PHYCFG
register and ensuring PHY enters L2 before turning off clocks etc.

Signed-off-by: Manu Gautam 
---
 drivers/usb/dwc3/Kconfig  |  11 +
 drivers/usb/dwc3/Makefile |   1 +
 drivers/usb/dwc3/dwc3-of-simple.c |   1 -
 drivers/usb/dwc3/dwc3-qcom.c  | 635 ++
 4 files changed, 647 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/dwc3/dwc3-qcom.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index ab8c0e0..f5bb4f1 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -106,4 +106,15 @@ config USB_DWC3_ST
  inside (i.e. STiH407).
  Say 'Y' or 'M' if you have one such device.
 
+config USB_DWC3_QCOM
+   tristate "Qualcomm Platform"
+   depends on ARCH_QCOM || COMPILE_TEST
+   depends on OF
+   default USB_DWC3
+   help
+ Some Qualcomm SoCs use DesignWare Core IP for USB2/3
+ functionality.
+
+ Say 'Y' or 'M' if you have one such device.
+
 endif
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 7ac7250..c3ce697 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -48,3 +48,4 @@ obj-$(CONFIG_USB_DWC3_PCI)+= dwc3-pci.o
 obj-$(CONFIG_USB_DWC3_KEYSTONE)+= dwc3-keystone.o
 obj-$(CONFIG_USB_DWC3_OF_SIMPLE)   += dwc3-of-simple.o
 obj-$(CONFIG_USB_DWC3_ST)  += dwc3-st.o
+obj-$(CONFIG_USB_DWC3_QCOM)+= dwc3-qcom.o
diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
b/drivers/usb/dwc3/dwc3-of-simple.c
index cb2ee96..0fd0e8e 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -208,7 +208,6 @@ static int dwc3_of_simple_runtime_resume(struct device *dev)
 };
 
 static const struct of_device_id of_dwc3_simple_match[] = {
-   { .compatible = "qcom,dwc3" },
{ .compatible = "rockchip,rk3399-dwc3" },
{ .compatible = "xlnx,zynqmp-dwc3" },
{ .compatible = "cavium,octeon-7130-usb-uctl" },
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
new file mode 100644
index 000..917199e
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -0,0 +1,635 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ *
+ * Inspired by dwc3-of-simple.c
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "core.h"
+#include "io.h"
+
+/* USB QSCRATCH Hardware registers */
+#define QSCRATCH_HS_PHY_CTRL   0x10
+#define UTMI_OTG_VBUS_VALIDBIT(20)
+#define SW_SESSVLD_SEL BIT(28)
+
+#define QSCRATCH_SS_PHY_CTRL   0x30
+#define LANE0_PWR_PRESENT  BIT(24)
+
+#define QSCRATCH_GENERAL_CFG   0x08
+#define PIPE_UTMI_CLK_SEL  BIT(0)
+#define PIPE3_PHYSTATUS_SW BIT(3)
+#define PIPE_UTMI_CLK_DIS  BIT(8)
+
+#define PWR_EVNT_IRQ_STAT_REG  0x58
+#define PWR_EVNT_LPM_IN_L2_MASKBIT(4)
+#define PWR_EVNT_LPM_OUT_L2_MASK   BIT(5)
+
+#define HSPHY_L2_ENTER_TIMEOUT_US  5000
+
+struct dwc3_qcom {
+   struct device   *dev;
+   void __iomem*qscratch_base;
+   struct platform_device  *dwc3;
+   struct clk  **clks;
+   int num_clocks;
+   struct reset_control*resets;
+
+   int dp_hs_phy_irq;
+   int dm_hs_phy_irq;
+   int ss_phy_irq;
+
+   struct extcon_dev   *edev;
+   struct extcon_dev   *host_edev;
+   struct notifier_block   vbus_nb;
+   struct notifier_block   host_nb;
+
+   enum usb_dr_modemode;
+   bool  

[PATCH v1 2/2] usb: dwc3: Add Qualcomm DWC3 glue driver

[Manu Gautam]

DWC3 controller on Qualcomm SOCs has a Qscratch wrapper.
Some of its uses are described below resulting in need to
have a separate glue driver instead of using dwc3-of-simple:
 - It exposes register interface to override vbus-override
   and lane0-pwr-present signals going to hardware. These
   must be updated in peripheral mode for DWC3 if vbus lines
   are not connected to hardware block. Otherwise RX termination
   in SS mode or DP pull-up is not applied by device controller.
 - pwr_events_irq_stat support to ensure USB2 PHY is in L2 state
   before glue driver can turn-off clocks and suspend PHY.
 - Support for wakeup interrupts lines that are asserted whenever
   there is any wakeup event on USB3 or USB2 bus.
 - Support to replace pip3 clock going to DWC3 with utmi clock
   for hardware configuration where SSPHY is not used with DWC3.

Other than above hardware features in Qscratch wrapper there
are some limitations on QCOM SOCs that require special handling
of power management e.g. suspending PHY using GUSB2PHYCFG
register and ensuring PHY enters L2 before turning off clocks etc.

Signed-off-by: Manu Gautam 
---
 drivers/usb/dwc3/Kconfig  |  11 +
 drivers/usb/dwc3/Makefile |   1 +
 drivers/usb/dwc3/dwc3-of-simple.c |   1 -
 drivers/usb/dwc3/dwc3-qcom.c  | 635 ++
 4 files changed, 647 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/dwc3/dwc3-qcom.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index ab8c0e0..f5bb4f1 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -106,4 +106,15 @@ config USB_DWC3_ST
  inside (i.e. STiH407).
  Say 'Y' or 'M' if you have one such device.
 
+config USB_DWC3_QCOM
+   tristate "Qualcomm Platform"
+   depends on ARCH_QCOM || COMPILE_TEST
+   depends on OF
+   default USB_DWC3
+   help
+ Some Qualcomm SoCs use DesignWare Core IP for USB2/3
+ functionality.
+
+ Say 'Y' or 'M' if you have one such device.
+
 endif
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 7ac7250..c3ce697 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -48,3 +48,4 @@ obj-$(CONFIG_USB_DWC3_PCI)+= dwc3-pci.o
 obj-$(CONFIG_USB_DWC3_KEYSTONE)+= dwc3-keystone.o
 obj-$(CONFIG_USB_DWC3_OF_SIMPLE)   += dwc3-of-simple.o
 obj-$(CONFIG_USB_DWC3_ST)  += dwc3-st.o
+obj-$(CONFIG_USB_DWC3_QCOM)+= dwc3-qcom.o
diff --git a/drivers/usb/dwc3/dwc3-of-simple.c 
b/drivers/usb/dwc3/dwc3-of-simple.c
index cb2ee96..0fd0e8e 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -208,7 +208,6 @@ static int dwc3_of_simple_runtime_resume(struct device *dev)
 };
 
 static const struct of_device_id of_dwc3_simple_match[] = {
-   { .compatible = "qcom,dwc3" },
{ .compatible = "rockchip,rk3399-dwc3" },
{ .compatible = "xlnx,zynqmp-dwc3" },
{ .compatible = "cavium,octeon-7130-usb-uctl" },
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
new file mode 100644
index 000..917199e
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -0,0 +1,635 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ *
+ * Inspired by dwc3-of-simple.c
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "core.h"
+#include "io.h"
+
+/* USB QSCRATCH Hardware registers */
+#define QSCRATCH_HS_PHY_CTRL   0x10
+#define UTMI_OTG_VBUS_VALIDBIT(20)
+#define SW_SESSVLD_SEL BIT(28)
+
+#define QSCRATCH_SS_PHY_CTRL   0x30
+#define LANE0_PWR_PRESENT  BIT(24)
+
+#define QSCRATCH_GENERAL_CFG   0x08
+#define PIPE_UTMI_CLK_SEL  BIT(0)
+#define PIPE3_PHYSTATUS_SW BIT(3)
+#define PIPE_UTMI_CLK_DIS  BIT(8)
+
+#define PWR_EVNT_IRQ_STAT_REG  0x58
+#define PWR_EVNT_LPM_IN_L2_MASKBIT(4)
+#define PWR_EVNT_LPM_OUT_L2_MASK   BIT(5)
+
+#define HSPHY_L2_ENTER_TIMEOUT_US  5000
+
+struct dwc3_qcom {
+   struct device   *dev;
+   void __iomem*qscratch_base;
+   struct platform_device  *dwc3;
+   struct clk  **clks;
+   int num_clocks;
+   struct reset_control*resets;
+
+   int dp_hs_phy_irq;
+   int dm_hs_phy_irq;
+   int ss_phy_irq;
+
+   struct extcon_dev   *edev;
+   struct extcon_dev   *host_edev;
+   struct notifier_block   vbus_nb;
+   struct notifier_block   host_nb;
+
+   enum usb_dr_modemode;
+   bool