2020-05-13 20:41:58

by Roger Quadros

[permalink] [raw]
Subject: [PATCH 0/3] usb: dwc3: keystone: Convert binding to YAML

Hi Felipe,

This series converts keystone-usb.txt to YAML and prepares
for Super-Speed support for AM654 SoC.

cheers,
-roger

Roger Quadros (3):
dt-bindings: usb: convert keystone-usb.txt to YAML
dt-bindings: usb: ti,keystone-dwc3.yaml: Add USB3.0 PHY property
usb: dwc3: keystone: Turn on USB3 PHY before controller

.../devicetree/bindings/usb/keystone-usb.txt | 56 ------------
.../bindings/usb/ti,keystone-dwc3.yaml | 88 +++++++++++++++++++
drivers/usb/dwc3/dwc3-keystone.c | 47 +++++++++-
3 files changed, 134 insertions(+), 57 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/usb/keystone-usb.txt
create mode 100644 Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


2020-05-13 20:42:04

by Roger Quadros

[permalink] [raw]
Subject: [PATCH 3/3] usb: dwc3: keystone: Turn on USB3 PHY before controller

The Local Power Sleep Controller (LPSC) dependency on AM65
requires SERDES0 to be powered on before USB.

We need to power up SERDES0 power domain and hold it on
throughout the reset, init, power on sequence.

Signed-off-by: Roger Quadros <[email protected]>
---
drivers/usb/dwc3/dwc3-keystone.c | 47 +++++++++++++++++++++++++++++++-
1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
index 1e14a6f4884b..46d46f3507fc 100644
--- a/drivers/usb/dwc3/dwc3-keystone.c
+++ b/drivers/usb/dwc3/dwc3-keystone.c
@@ -14,6 +14,7 @@
#include <linux/dma-mapping.h>
#include <linux/io.h>
#include <linux/of_platform.h>
+#include <linux/phy/phy.h>
#include <linux/pm_runtime.h>

/* USBSS register offsets */
@@ -34,6 +35,7 @@
struct dwc3_keystone {
struct device *dev;
void __iomem *usbss;
+ struct phy *usb3_phy;
};

static inline u32 kdwc3_readl(void __iomem *base, u32 offset)
@@ -95,8 +97,44 @@ static int kdwc3_probe(struct platform_device *pdev)
if (IS_ERR(kdwc->usbss))
return PTR_ERR(kdwc->usbss);

- pm_runtime_enable(kdwc->dev);
+ /* PSC dependency on AM65 needs SERDES0 to be powered before USB0 */
+ kdwc->usb3_phy = devm_phy_get(dev, "usb3-phy");
+ if (IS_ERR(kdwc->usb3_phy)) {
+ error = PTR_ERR(kdwc->usb3_phy);
+ if (error == -ENOSYS || error == -ENODEV) {
+ kdwc->usb3_phy = NULL;
+ } else {
+ if (error != -EPROBE_DEFER) {
+ dev_err(dev, "couldn't get usb3 phy: %d\n",
+ error);
+ }
+
+ return error;
+ }
+ }
+
+ phy_pm_runtime_get_sync(kdwc->usb3_phy);
+
+ error = phy_reset(kdwc->usb3_phy);
+ if (error < 0) {
+ dev_err(dev, "usb3 phy reset failed: %d\n", error);
+ return error;
+ }
+
+ error = phy_init(kdwc->usb3_phy);
+ if (error < 0) {
+ dev_err(dev, "usb3 phy init failed: %d\n", error);
+ return error;
+ }

+ error = phy_power_on(kdwc->usb3_phy);
+ if (error < 0) {
+ dev_err(dev, "usb3 phy power on failed: %d\n", error);
+ phy_exit(kdwc->usb3_phy);
+ return error;
+ }
+
+ pm_runtime_enable(kdwc->dev);
error = pm_runtime_get_sync(kdwc->dev);
if (error < 0) {
dev_err(kdwc->dev, "pm_runtime_get_sync failed, error %d\n",
@@ -138,6 +176,9 @@ static int kdwc3_probe(struct platform_device *pdev)
err_irq:
pm_runtime_put_sync(kdwc->dev);
pm_runtime_disable(kdwc->dev);
+ phy_power_off(kdwc->usb3_phy);
+ phy_exit(kdwc->usb3_phy);
+ phy_pm_runtime_put_sync(kdwc->usb3_phy);

return error;
}
@@ -163,6 +204,10 @@ static int kdwc3_remove(struct platform_device *pdev)
pm_runtime_put_sync(kdwc->dev);
pm_runtime_disable(kdwc->dev);

+ phy_power_off(kdwc->usb3_phy);
+ phy_exit(kdwc->usb3_phy);
+ phy_pm_runtime_put_sync(kdwc->usb3_phy);
+
platform_set_drvdata(pdev, NULL);

return 0;
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2020-05-14 01:41:44

by Chunfeng Yun

[permalink] [raw]
Subject: Re: [PATCH 3/3] usb: dwc3: keystone: Turn on USB3 PHY before controller

On Wed, 2020-05-13 at 16:07 +0300, Roger Quadros wrote:
> The Local Power Sleep Controller (LPSC) dependency on AM65
> requires SERDES0 to be powered on before USB.
>
> We need to power up SERDES0 power domain and hold it on
> throughout the reset, init, power on sequence.
>
> Signed-off-by: Roger Quadros <[email protected]>
> ---
> drivers/usb/dwc3/dwc3-keystone.c | 47 +++++++++++++++++++++++++++++++-
> 1 file changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
> index 1e14a6f4884b..46d46f3507fc 100644
> --- a/drivers/usb/dwc3/dwc3-keystone.c
> +++ b/drivers/usb/dwc3/dwc3-keystone.c
> @@ -14,6 +14,7 @@
> #include <linux/dma-mapping.h>
> #include <linux/io.h>
> #include <linux/of_platform.h>
> +#include <linux/phy/phy.h>
> #include <linux/pm_runtime.h>
>
> /* USBSS register offsets */
> @@ -34,6 +35,7 @@
> struct dwc3_keystone {
> struct device *dev;
> void __iomem *usbss;
> + struct phy *usb3_phy;
> };
>
> static inline u32 kdwc3_readl(void __iomem *base, u32 offset)
> @@ -95,8 +97,44 @@ static int kdwc3_probe(struct platform_device *pdev)
> if (IS_ERR(kdwc->usbss))
> return PTR_ERR(kdwc->usbss);
>
> - pm_runtime_enable(kdwc->dev);
> + /* PSC dependency on AM65 needs SERDES0 to be powered before USB0 */
> + kdwc->usb3_phy = devm_phy_get(dev, "usb3-phy");
Use devm_phy_optional_get() instead?

> + if (IS_ERR(kdwc->usb3_phy)) {
> + error = PTR_ERR(kdwc->usb3_phy);
> + if (error == -ENOSYS || error == -ENODEV) {
> + kdwc->usb3_phy = NULL;
> + } else {
> + if (error != -EPROBE_DEFER) {
> + dev_err(dev, "couldn't get usb3 phy: %d\n",
> + error);
> + }
> +
> + return error;
> + }
> + }
> +
> + phy_pm_runtime_get_sync(kdwc->usb3_phy);
> +
> + error = phy_reset(kdwc->usb3_phy);
> + if (error < 0) {
> + dev_err(dev, "usb3 phy reset failed: %d\n", error);
> + return error;
> + }
> +
> + error = phy_init(kdwc->usb3_phy);
> + if (error < 0) {
> + dev_err(dev, "usb3 phy init failed: %d\n", error);
> + return error;
> + }
>
> + error = phy_power_on(kdwc->usb3_phy);
> + if (error < 0) {
> + dev_err(dev, "usb3 phy power on failed: %d\n", error);
> + phy_exit(kdwc->usb3_phy);
> + return error;
> + }
> +
> + pm_runtime_enable(kdwc->dev);
> error = pm_runtime_get_sync(kdwc->dev);
> if (error < 0) {
> dev_err(kdwc->dev, "pm_runtime_get_sync failed, error %d\n",
> @@ -138,6 +176,9 @@ static int kdwc3_probe(struct platform_device *pdev)
> err_irq:
> pm_runtime_put_sync(kdwc->dev);
> pm_runtime_disable(kdwc->dev);
> + phy_power_off(kdwc->usb3_phy);
> + phy_exit(kdwc->usb3_phy);
> + phy_pm_runtime_put_sync(kdwc->usb3_phy);
>
> return error;
> }
> @@ -163,6 +204,10 @@ static int kdwc3_remove(struct platform_device *pdev)
> pm_runtime_put_sync(kdwc->dev);
> pm_runtime_disable(kdwc->dev);
>
> + phy_power_off(kdwc->usb3_phy);
> + phy_exit(kdwc->usb3_phy);
> + phy_pm_runtime_put_sync(kdwc->usb3_phy);
> +
> platform_set_drvdata(pdev, NULL);
>
> return 0;

2020-05-14 07:43:05

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH 3/3] usb: dwc3: keystone: Turn on USB3 PHY before controller



On 14/05/2020 04:37, Chunfeng Yun wrote:
> On Wed, 2020-05-13 at 16:07 +0300, Roger Quadros wrote:
>> The Local Power Sleep Controller (LPSC) dependency on AM65
>> requires SERDES0 to be powered on before USB.
>>
>> We need to power up SERDES0 power domain and hold it on
>> throughout the reset, init, power on sequence.
>>
>> Signed-off-by: Roger Quadros <[email protected]>
>> ---
>> drivers/usb/dwc3/dwc3-keystone.c | 47 +++++++++++++++++++++++++++++++-
>> 1 file changed, 46 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
>> index 1e14a6f4884b..46d46f3507fc 100644
>> --- a/drivers/usb/dwc3/dwc3-keystone.c
>> +++ b/drivers/usb/dwc3/dwc3-keystone.c
>> @@ -14,6 +14,7 @@
>> #include <linux/dma-mapping.h>
>> #include <linux/io.h>
>> #include <linux/of_platform.h>
>> +#include <linux/phy/phy.h>
>> #include <linux/pm_runtime.h>
>>
>> /* USBSS register offsets */
>> @@ -34,6 +35,7 @@
>> struct dwc3_keystone {
>> struct device *dev;
>> void __iomem *usbss;
>> + struct phy *usb3_phy;
>> };
>>
>> static inline u32 kdwc3_readl(void __iomem *base, u32 offset)
>> @@ -95,8 +97,44 @@ static int kdwc3_probe(struct platform_device *pdev)
>> if (IS_ERR(kdwc->usbss))
>> return PTR_ERR(kdwc->usbss);
>>
>> - pm_runtime_enable(kdwc->dev);
>> + /* PSC dependency on AM65 needs SERDES0 to be powered before USB0 */
>> + kdwc->usb3_phy = devm_phy_get(dev, "usb3-phy");
> Use devm_phy_optional_get() instead?

Indeed, it seems better suited.

cheers,
-roger

>
>> + if (IS_ERR(kdwc->usb3_phy)) {
>> + error = PTR_ERR(kdwc->usb3_phy);
>> + if (error == -ENOSYS || error == -ENODEV) {
>> + kdwc->usb3_phy = NULL;
>> + } else {
>> + if (error != -EPROBE_DEFER) {
>> + dev_err(dev, "couldn't get usb3 phy: %d\n",
>> + error);
>> + }
>> +
>> + return error;
>> + }
>> + }
>> +
>> + phy_pm_runtime_get_sync(kdwc->usb3_phy);
>> +
>> + error = phy_reset(kdwc->usb3_phy);
>> + if (error < 0) {
>> + dev_err(dev, "usb3 phy reset failed: %d\n", error);
>> + return error;
>> + }
>> +
>> + error = phy_init(kdwc->usb3_phy);
>> + if (error < 0) {
>> + dev_err(dev, "usb3 phy init failed: %d\n", error);
>> + return error;
>> + }
>>
>> + error = phy_power_on(kdwc->usb3_phy);
>> + if (error < 0) {
>> + dev_err(dev, "usb3 phy power on failed: %d\n", error);
>> + phy_exit(kdwc->usb3_phy);
>> + return error;
>> + }
>> +
>> + pm_runtime_enable(kdwc->dev);
>> error = pm_runtime_get_sync(kdwc->dev);
>> if (error < 0) {
>> dev_err(kdwc->dev, "pm_runtime_get_sync failed, error %d\n",
>> @@ -138,6 +176,9 @@ static int kdwc3_probe(struct platform_device *pdev)
>> err_irq:
>> pm_runtime_put_sync(kdwc->dev);
>> pm_runtime_disable(kdwc->dev);
>> + phy_power_off(kdwc->usb3_phy);
>> + phy_exit(kdwc->usb3_phy);
>> + phy_pm_runtime_put_sync(kdwc->usb3_phy);
>>
>> return error;
>> }
>> @@ -163,6 +204,10 @@ static int kdwc3_remove(struct platform_device *pdev)
>> pm_runtime_put_sync(kdwc->dev);
>> pm_runtime_disable(kdwc->dev);
>>
>> + phy_power_off(kdwc->usb3_phy);
>> + phy_exit(kdwc->usb3_phy);
>> + phy_pm_runtime_put_sync(kdwc->usb3_phy);
>> +
>> platform_set_drvdata(pdev, NULL);
>>
>> return 0;
>

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2020-05-14 10:24:15

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 3/3] usb: dwc3: keystone: Turn on USB3 PHY before controller

Roger Quadros <[email protected]> writes:

> On 14/05/2020 04:37, Chunfeng Yun wrote:
>> On Wed, 2020-05-13 at 16:07 +0300, Roger Quadros wrote:
>>> The Local Power Sleep Controller (LPSC) dependency on AM65
>>> requires SERDES0 to be powered on before USB.
>>>
>>> We need to power up SERDES0 power domain and hold it on
>>> throughout the reset, init, power on sequence.
>>>
>>> Signed-off-by: Roger Quadros <[email protected]>
>>> ---
>>> drivers/usb/dwc3/dwc3-keystone.c | 47 +++++++++++++++++++++++++++++++-
>>> 1 file changed, 46 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
>>> index 1e14a6f4884b..46d46f3507fc 100644
>>> --- a/drivers/usb/dwc3/dwc3-keystone.c
>>> +++ b/drivers/usb/dwc3/dwc3-keystone.c
>>> @@ -14,6 +14,7 @@
>>> #include <linux/dma-mapping.h>
>>> #include <linux/io.h>
>>> #include <linux/of_platform.h>
>>> +#include <linux/phy/phy.h>
>>> #include <linux/pm_runtime.h>
>>>
>>> /* USBSS register offsets */
>>> @@ -34,6 +35,7 @@
>>> struct dwc3_keystone {
>>> struct device *dev;
>>> void __iomem *usbss;
>>> + struct phy *usb3_phy;
>>> };
>>>
>>> static inline u32 kdwc3_readl(void __iomem *base, u32 offset)
>>> @@ -95,8 +97,44 @@ static int kdwc3_probe(struct platform_device *pdev)
>>> if (IS_ERR(kdwc->usbss))
>>> return PTR_ERR(kdwc->usbss);
>>>
>>> - pm_runtime_enable(kdwc->dev);
>>> + /* PSC dependency on AM65 needs SERDES0 to be powered before USB0 */
>>> + kdwc->usb3_phy = devm_phy_get(dev, "usb3-phy");
>> Use devm_phy_optional_get() instead?
>
> Indeed, it seems better suited.

patches 1 and 2 are in testing/next

--
balbi


Attachments:
signature.asc (847.00 B)

2020-05-25 06:41:42

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH 3/3] usb: dwc3: keystone: Turn on USB3 PHY before controller

Felipe,

On 14/05/2020 13:21, Felipe Balbi wrote:
> Roger Quadros <[email protected]> writes:
>
>> On 14/05/2020 04:37, Chunfeng Yun wrote:
>>> On Wed, 2020-05-13 at 16:07 +0300, Roger Quadros wrote:
>>>> The Local Power Sleep Controller (LPSC) dependency on AM65
>>>> requires SERDES0 to be powered on before USB.
>>>>
>>>> We need to power up SERDES0 power domain and hold it on
>>>> throughout the reset, init, power on sequence.
>>>>
>>>> Signed-off-by: Roger Quadros <[email protected]>
>>>> ---
>>>> drivers/usb/dwc3/dwc3-keystone.c | 47 +++++++++++++++++++++++++++++++-
>>>> 1 file changed, 46 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
>>>> index 1e14a6f4884b..46d46f3507fc 100644
>>>> --- a/drivers/usb/dwc3/dwc3-keystone.c
>>>> +++ b/drivers/usb/dwc3/dwc3-keystone.c
>>>> @@ -14,6 +14,7 @@
>>>> #include <linux/dma-mapping.h>
>>>> #include <linux/io.h>
>>>> #include <linux/of_platform.h>
>>>> +#include <linux/phy/phy.h>
>>>> #include <linux/pm_runtime.h>
>>>>
>>>> /* USBSS register offsets */
>>>> @@ -34,6 +35,7 @@
>>>> struct dwc3_keystone {
>>>> struct device *dev;
>>>> void __iomem *usbss;
>>>> + struct phy *usb3_phy;
>>>> };
>>>>
>>>> static inline u32 kdwc3_readl(void __iomem *base, u32 offset)
>>>> @@ -95,8 +97,44 @@ static int kdwc3_probe(struct platform_device *pdev)
>>>> if (IS_ERR(kdwc->usbss))
>>>> return PTR_ERR(kdwc->usbss);
>>>>
>>>> - pm_runtime_enable(kdwc->dev);
>>>> + /* PSC dependency on AM65 needs SERDES0 to be powered before USB0 */
>>>> + kdwc->usb3_phy = devm_phy_get(dev, "usb3-phy");
>>> Use devm_phy_optional_get() instead?
>>
>> Indeed, it seems better suited.
>
> patches 1 and 2 are in testing/next
>

Could you please drop them as I need to make changes to make the PHY optional.
I will send v2 of entire series.

cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

2020-05-25 06:43:40

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCH 3/3] usb: dwc3: keystone: Turn on USB3 PHY before controller



On 25/05/2020 09:39, Roger Quadros wrote:
> Felipe,
>
> On 14/05/2020 13:21, Felipe Balbi wrote:
>> Roger Quadros <[email protected]> writes:
>>
>>> On 14/05/2020 04:37, Chunfeng Yun wrote:
>>>> On Wed, 2020-05-13 at 16:07 +0300, Roger Quadros wrote:
>>>>> The Local Power Sleep Controller (LPSC) dependency on AM65
>>>>> requires SERDES0 to be powered on before USB.
>>>>>
>>>>> We need to power up SERDES0 power domain and hold it on
>>>>> throughout the reset, init, power on sequence.
>>>>>
>>>>> Signed-off-by: Roger Quadros <[email protected]>
>>>>> ---
>>>>>    drivers/usb/dwc3/dwc3-keystone.c | 47 +++++++++++++++++++++++++++++++-
>>>>>    1 file changed, 46 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/usb/dwc3/dwc3-keystone.c b/drivers/usb/dwc3/dwc3-keystone.c
>>>>> index 1e14a6f4884b..46d46f3507fc 100644
>>>>> --- a/drivers/usb/dwc3/dwc3-keystone.c
>>>>> +++ b/drivers/usb/dwc3/dwc3-keystone.c
>>>>> @@ -14,6 +14,7 @@
>>>>>    #include <linux/dma-mapping.h>
>>>>>    #include <linux/io.h>
>>>>>    #include <linux/of_platform.h>
>>>>> +#include <linux/phy/phy.h>
>>>>>    #include <linux/pm_runtime.h>
>>>>>    /* USBSS register offsets */
>>>>> @@ -34,6 +35,7 @@
>>>>>    struct dwc3_keystone {
>>>>>        struct device            *dev;
>>>>>        void __iomem            *usbss;
>>>>> +    struct phy            *usb3_phy;
>>>>>    };
>>>>>    static inline u32 kdwc3_readl(void __iomem *base, u32 offset)
>>>>> @@ -95,8 +97,44 @@ static int kdwc3_probe(struct platform_device *pdev)
>>>>>        if (IS_ERR(kdwc->usbss))
>>>>>            return PTR_ERR(kdwc->usbss);
>>>>> -    pm_runtime_enable(kdwc->dev);
>>>>> +    /* PSC dependency on AM65 needs SERDES0 to be powered before USB0 */
>>>>> +    kdwc->usb3_phy = devm_phy_get(dev, "usb3-phy");
>>>> Use devm_phy_optional_get() instead?
>>>
>>> Indeed, it seems better suited.
>>
>> patches 1 and 2 are in testing/next
>>
>
> Could you please drop them as I need to make changes to make the PHY optional.
> I will send v2 of entire series.

Actually only patch 2 and 3 need to be revised. Patch 1 is fine.

cheers,
-roger
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki