[U-Boot] [PATCH v2] usb: dwc2: Add reset ctrl to driver

newer
[U-Boot] Problem with v2018-09-rc2...

Ley Foon Tan

16 Aug 2018 16 Aug '18

7:45 a.m.

Add code to reset all reset signals as in usb DT node. A reset property is an optional feature, so only print out a warning and do not fail if a reset property is not present.

If a reset property is discovered, then use it to deassert, thus bringing the IP out of reset.

Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com

--- v2: - Assert reset when .remove. --- drivers/usb/host/dwc2.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index cbe065b..bee3b33 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -15,6 +15,7 @@ #include <wait_bit.h> #include <asm/io.h> #include <power/regulator.h> +#include <reset.h>

#include "dwc2.h"

@@ -49,6 +50,8 @@ struct dwc2_priv { */ bool hnp_srp_disable; bool oc_disable; + + struct reset_ctl_bulk resets; };

#ifndef CONFIG_DM_USB @@ -1124,11 +1127,37 @@ int _submit_int_msg(struct dwc2_priv *priv, struct usb_device *dev, } }

+static int dwc2_reset(struct udevice *dev) +{ + int ret; + struct dwc2_priv *priv = dev_get_priv(dev); + + ret = reset_get_bulk(dev, &priv->resets); + if (ret) { + dev_warn(dev, "Can't get reset: %d\n", ret); + return ret; + } + + ret = reset_deassert_bulk(&priv->resets); + if (ret) { + reset_release_bulk(&priv->resets); + dev_err(dev, "Failed to reset: %d\n", ret); + return ret; + } + + return 0; +} + static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv) { struct dwc2_core_regs *regs = priv->regs; uint32_t snpsid; int i, j; + int ret; + + ret = dwc2_reset(dev); + if (ret) + return ret;

snpsid = readl(&regs->gsnpsid); dev_info(dev, "Core Release: %x.%03x\n", @@ -1303,6 +1332,8 @@ static int dwc2_usb_remove(struct udevice *dev)

dwc2_uninit_common(priv->regs);

+ reset_release_bulk(&priv->resets); + return 0; }

-- 2.2.2

Show replies by date

Marek Vasut

16 Aug 16 Aug

10:26 a.m.

On 08/16/2018 07:45 AM, Ley Foon Tan wrote:

...

Add code to reset all reset signals as in usb DT node. A reset property is an optional feature, so only print out a warning and do not fail if a reset property is not present.

If a reset property is discovered, then use it to deassert, thus bringing the IP out of reset.

Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com

v2:

Assert reset when .remove.

drivers/usb/host/dwc2.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index cbe065b..bee3b33 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -15,6 +15,7 @@ #include <wait_bit.h> #include <asm/io.h> #include <power/regulator.h> +#include <reset.h>

#include "dwc2.h"

@@ -49,6 +50,8 @@ struct dwc2_priv { */ bool hnp_srp_disable; bool oc_disable;

struct reset_ctl_bulk resets;

};

#ifndef CONFIG_DM_USB @@ -1124,11 +1127,37 @@ int _submit_int_msg(struct dwc2_priv *priv, struct usb_device *dev, } }

+static int dwc2_reset(struct udevice *dev) +{
int ret;

struct dwc2_priv *priv = dev_get_priv(dev);

ret = reset_get_bulk(dev, &priv->resets);

if (ret) {
dev_warn(dev, "Can't get reset: %d\n", ret);

So this now generates a ton of warnings on Gen5 , right ? Did you test it on any Gen5 board recently ?

...

return ret;
}

ret = reset_deassert_bulk(&priv->resets);

if (ret) {
reset_release_bulk(&priv->resets);
dev_err(dev, "Failed to reset: %d\n", ret);
return ret;
}

return 0;
+}

static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv) { struct dwc2_core_regs *regs = priv->regs; uint32_t snpsid; int i, j;
int ret;

ret = dwc2_reset(dev);

if (ret)
return ret;
snpsid = readl(&regs->gsnpsid); dev_info(dev, "Core Release: %x.%03x\n",
@@ -1303,6 +1332,8 @@ static int dwc2_usb_remove(struct udevice *dev)

dwc2_uninit_common(priv->regs);

reset_release_bulk(&priv->resets);

Dont you need to assert the resets too ?

...

return 0; }

-- Best regards, Marek Vasut

Ley Foon Tan

11:18 a.m.

On Thu, Aug 16, 2018 at 4:26 PM, Marek Vasut marex@denx.de wrote:

...

On 08/16/2018 07:45 AM, Ley Foon Tan wrote:

...
Add code to reset all reset signals as in usb DT node. A reset property is an optional feature, so only print out a warning and do not fail if a reset property is not present.

If a reset property is discovered, then use it to deassert, thus bringing the IP out of reset.

Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com

v2:

Assert reset when .remove.

drivers/usb/host/dwc2.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index cbe065b..bee3b33 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -15,6 +15,7 @@ #include <wait_bit.h> #include <asm/io.h> #include <power/regulator.h> +#include <reset.h>

#include "dwc2.h"

@@ -49,6 +50,8 @@ struct dwc2_priv { */ bool hnp_srp_disable; bool oc_disable;
struct reset_ctl_bulk   resets;
};

#ifndef CONFIG_DM_USB @@ -1124,11 +1127,37 @@ int _submit_int_msg(struct dwc2_priv *priv, struct usb_device *dev, } }

+static int dwc2_reset(struct udevice *dev) +{
int ret;
struct dwc2_priv *priv = dev_get_priv(dev);
ret = reset_get_bulk(dev, &priv->resets);
if (ret) {
        dev_warn(dev, "Can't get reset: %d\n", ret);
So this now generates a ton of warnings on Gen5 , right ? Did you test it on any Gen5 board recently ?

Will change to "return 0" if can't find reset in dts, so it will not fail the probe.

It will not print warning by default. I don't have Gen5 board, but I tested with Stratix 10 by removing reset from USB DT node. We will not see the warning.

...

...
        return ret;
}
ret = reset_deassert_bulk(&priv->resets);
if (ret) {
        reset_release_bulk(&priv->resets);
        dev_err(dev, "Failed to reset: %d\n", ret);
        return ret;
}
return 0;
+}

static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv) { struct dwc2_core_regs *regs = priv->regs; uint32_t snpsid; int i, j;
int ret;
ret = dwc2_reset(dev);
if (ret)
        return ret;

snpsid = readl(&regs->gsnpsid);
dev_info(dev, "Core Release: %x.%03x\n",
@@ -1303,6 +1332,8 @@ static int dwc2_usb_remove(struct udevice *dev)
  dwc2_uninit_common(priv->regs);
reset_release_bulk(&priv->resets);
Dont you need to assert the resets too ?

reset_release_bulk will assert the reset if it is requested before.

...

  return 0;

}

Regards Ley Foon

Marek Vasut

11:21 a.m.

On 08/16/2018 11:18 AM, Ley Foon Tan wrote:

...

On Thu, Aug 16, 2018 at 4:26 PM, Marek Vasut marex@denx.de wrote:

...
On 08/16/2018 07:45 AM, Ley Foon Tan wrote:

...
Add code to reset all reset signals as in usb DT node. A reset property is an optional feature, so only print out a warning and do not fail if a reset property is not present.

If a reset property is discovered, then use it to deassert, thus bringing the IP out of reset.

Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com

v2:

Assert reset when .remove.

drivers/usb/host/dwc2.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index cbe065b..bee3b33 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -15,6 +15,7 @@ #include <wait_bit.h> #include <asm/io.h> #include <power/regulator.h> +#include <reset.h>

#include "dwc2.h"

@@ -49,6 +50,8 @@ struct dwc2_priv { */ bool hnp_srp_disable; bool oc_disable;
struct reset_ctl_bulk   resets;
};

#ifndef CONFIG_DM_USB @@ -1124,11 +1127,37 @@ int _submit_int_msg(struct dwc2_priv *priv, struct usb_device *dev, } }

+static int dwc2_reset(struct udevice *dev) +{
int ret;
struct dwc2_priv *priv = dev_get_priv(dev);
ret = reset_get_bulk(dev, &priv->resets);
if (ret) {
        dev_warn(dev, "Can't get reset: %d\n", ret);
So this now generates a ton of warnings on Gen5 , right ? Did you test it on any Gen5 board recently ?
Will change to "return 0" if can't find reset in dts, so it will not fail the probe.

But that's a valid error. Maybe some ifdef CONFIG_RESET would be more appropriate ? Look at what the other drivers do.

...

It will not print warning by default. I don't have Gen5 board, but I tested with Stratix 10 by removing reset from USB DT node. We will not see the warning.

Paging Simon, he's been doing a lot of good work on Gen5 recently. You should CC him on changes too :)

...

...
...
        return ret;
}
ret = reset_deassert_bulk(&priv->resets);
if (ret) {
        reset_release_bulk(&priv->resets);
        dev_err(dev, "Failed to reset: %d\n", ret);
        return ret;
}
return 0;
+}

static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv) { struct dwc2_core_regs *regs = priv->regs; uint32_t snpsid; int i, j;
int ret;
ret = dwc2_reset(dev);
if (ret)
        return ret;

snpsid = readl(&regs->gsnpsid);
dev_info(dev, "Core Release: %x.%03x\n",
@@ -1303,6 +1332,8 @@ static int dwc2_usb_remove(struct udevice *dev)
  dwc2_uninit_common(priv->regs);
reset_release_bulk(&priv->resets);
Dont you need to assert the resets too ?
reset_release_bulk will assert the reset if it is requested before.

...
...
  return 0;
}
Regards Ley Foon

-- Best regards, Marek Vasut

Ley Foon Tan

11:48 a.m.

On Thu, Aug 16, 2018 at 5:21 PM, Marek Vasut marex@denx.de wrote:

...

On 08/16/2018 11:18 AM, Ley Foon Tan wrote:

...
On Thu, Aug 16, 2018 at 4:26 PM, Marek Vasut marex@denx.de wrote:

...
On 08/16/2018 07:45 AM, Ley Foon Tan wrote:

...
Add code to reset all reset signals as in usb DT node. A reset property is an optional feature, so only print out a warning and do not fail if a reset property is not present.

If a reset property is discovered, then use it to deassert, thus bringing the IP out of reset.

Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com

v2:

Assert reset when .remove.

drivers/usb/host/dwc2.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index cbe065b..bee3b33 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -15,6 +15,7 @@ #include <wait_bit.h> #include <asm/io.h> #include <power/regulator.h> +#include <reset.h>

#include "dwc2.h"

@@ -49,6 +50,8 @@ struct dwc2_priv { */ bool hnp_srp_disable; bool oc_disable;
struct reset_ctl_bulk   resets;
};

#ifndef CONFIG_DM_USB @@ -1124,11 +1127,37 @@ int _submit_int_msg(struct dwc2_priv *priv, struct usb_device *dev, } }

+static int dwc2_reset(struct udevice *dev) +{
int ret;
struct dwc2_priv *priv = dev_get_priv(dev);
ret = reset_get_bulk(dev, &priv->resets);
if (ret) {
        dev_warn(dev, "Can't get reset: %d\n", ret);
So this now generates a ton of warnings on Gen5 , right ? Did you test it on any Gen5 board recently ?
Will change to "return 0" if can't find reset in dts, so it will not fail the probe.
But that's a valid error. Maybe some ifdef CONFIG_RESET would be more appropriate ? Look at what the other drivers do.

Platforms that doesn't support reset framework shouldn't treat it as error. Other drivers don't have CONFIG_RESET too.

...

...
It will not print warning by default. I don't have Gen5 board, but I tested with Stratix 10 by removing reset from USB DT node. We will not see the warning.

Paging Simon, he's been doing a lot of good work on Gen5 recently. You should CC him on changes too :)

...
...
...
        return ret;
}
ret = reset_deassert_bulk(&priv->resets);
if (ret) {
        reset_release_bulk(&priv->resets);
        dev_err(dev, "Failed to reset: %d\n", ret);
        return ret;
}
return 0;
+}

static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv) { struct dwc2_core_regs *regs = priv->regs; uint32_t snpsid; int i, j;
int ret;
ret = dwc2_reset(dev);
if (ret)
        return ret;

snpsid = readl(&regs->gsnpsid);
dev_info(dev, "Core Release: %x.%03x\n",
@@ -1303,6 +1332,8 @@ static int dwc2_usb_remove(struct udevice *dev)
  dwc2_uninit_common(priv->regs);
reset_release_bulk(&priv->resets);
Dont you need to assert the resets too ?
reset_release_bulk will assert the reset if it is requested before.

...
...
  return 0;
}
Regards Ley Foon

Regards Ley Foon

Marek Vasut

11:51 a.m.

On 08/16/2018 11:48 AM, Ley Foon Tan wrote:

...

On Thu, Aug 16, 2018 at 5:21 PM, Marek Vasut marex@denx.de wrote:

...
On 08/16/2018 11:18 AM, Ley Foon Tan wrote:

...
On Thu, Aug 16, 2018 at 4:26 PM, Marek Vasut marex@denx.de wrote:

...
On 08/16/2018 07:45 AM, Ley Foon Tan wrote:

...
Add code to reset all reset signals as in usb DT node. A reset property is an optional feature, so only print out a warning and do not fail if a reset property is not present.

If a reset property is discovered, then use it to deassert, thus bringing the IP out of reset.

Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com

v2:

Assert reset when .remove.

drivers/usb/host/dwc2.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index cbe065b..bee3b33 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -15,6 +15,7 @@ #include <wait_bit.h> #include <asm/io.h> #include <power/regulator.h> +#include <reset.h>

#include "dwc2.h"

@@ -49,6 +50,8 @@ struct dwc2_priv { */ bool hnp_srp_disable; bool oc_disable;
struct reset_ctl_bulk   resets;
};

#ifndef CONFIG_DM_USB @@ -1124,11 +1127,37 @@ int _submit_int_msg(struct dwc2_priv *priv, struct usb_device *dev, } }

+static int dwc2_reset(struct udevice *dev) +{
int ret;
struct dwc2_priv *priv = dev_get_priv(dev);
ret = reset_get_bulk(dev, &priv->resets);
if (ret) {
        dev_warn(dev, "Can't get reset: %d\n", ret);
So this now generates a ton of warnings on Gen5 , right ? Did you test it on any Gen5 board recently ?
Will change to "return 0" if can't find reset in dts, so it will not fail the probe.
But that's a valid error. Maybe some ifdef CONFIG_RESET would be more appropriate ? Look at what the other drivers do.
Platforms that doesn't support reset framework shouldn't treat it as error. Other drivers don't have CONFIG_RESET too.

What about platforms that do have reset and fail requesting it ? The error would be ignored on those ?

-- Best regards, Marek Vasut

2460

Age (days ago)

2460

Last active (days ago)

List overview

Download

5 comments

3 participants

tags (0)

participants (3)

Ley Foon Tan
Ley Foon Tan
Marek Vasut