[PATCH v2 0/2] reset/usb: Add reset status

In Cyclone 5 SoC platform, the first USB probing is failed but second probing is success. DWC2 driver read gsnpsid register right after de-assert reset, but controller is not ready yet and it returns gsnpsid 0. Polling reset status after de-assert reset to solve the issue.
This patchset also add .rst_status callback function to reset-socfpga driver to support this get reset status feature.
v1->v2: - Change from constant delay to poll for reset status; - Add reset status callback function.
History: v1: https://patchwork.ozlabs.org/patch/1214841/
Ley Foon Tan (2): reset: socfpga: Add reset status callback function usb: dwc2: Add polling for reset status
drivers/reset/reset-socfpga.c | 12 ++++++++++++ drivers/usb/host/dwc2.c | 10 +++++++++- 2 files changed, 21 insertions(+), 1 deletion(-)

Add .rst_status callback function. Read reset status from register, return 0 if deasserted, non-zero if asserted
Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com --- drivers/reset/reset-socfpga.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c index 93ec9cfdb6..b77c893b76 100644 --- a/drivers/reset/reset-socfpga.c +++ b/drivers/reset/reset-socfpga.c @@ -99,11 +99,23 @@ static int socfpga_reset_free(struct reset_ctl *reset_ctl) return 0; }
+static int socfpga_reset_status(struct reset_ctl *reset_ctl) +{ + struct socfpga_reset_data *data = dev_get_priv(reset_ctl->dev); + int id = reset_ctl->id; + int reg_width = sizeof(u32); + int bank = id / (reg_width * BITS_PER_BYTE); + int offset = id % (reg_width * BITS_PER_BYTE); + + return readl(data->modrst_base + (bank * BANK_INCREMENT)) & BIT(offset); +} + static const struct reset_ops socfpga_reset_ops = { .request = socfpga_reset_request, .free = socfpga_reset_free, .rst_assert = socfpga_reset_assert, .rst_deassert = socfpga_reset_deassert, + .rst_status = socfpga_reset_status, };
static int socfpga_reset_probe(struct udevice *dev)

On 12/24/19 9:23 AM, Ley Foon Tan wrote:
Add .rst_status callback function. Read reset status from register, return 0 if deasserted, non-zero if asserted
Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com
drivers/reset/reset-socfpga.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c index 93ec9cfdb6..b77c893b76 100644 --- a/drivers/reset/reset-socfpga.c +++ b/drivers/reset/reset-socfpga.c @@ -99,11 +99,23 @@ static int socfpga_reset_free(struct reset_ctl *reset_ctl) return 0; }
+static int socfpga_reset_status(struct reset_ctl *reset_ctl) +{
- struct socfpga_reset_data *data = dev_get_priv(reset_ctl->dev);
- int id = reset_ctl->id;
- int reg_width = sizeof(u32);
- int bank = id / (reg_width * BITS_PER_BYTE);
- int offset = id % (reg_width * BITS_PER_BYTE);
You want to flag these int values as const, since that's what they are. I think some of them are unsigned.
Otherwise looks good, thanks.

In Cyclone 5 SoC platform, the first USB probing is failed but second probing is success. DWC2 driver read gsnpsid register right after de-assert reset, but controller is not ready yet and it returns gsnpsid 0. Polling reset status after de-assert reset to solve the issue.
Retry with this fix more than 10 times without issue.
=> usb reset resetting USB... Bus usb@ffb40000: usb probe SNPSID invalid (not DWC2 OTG device): 00000000 Port not available. => usb reset resetting USB... Bus usb@ffb40000: usb probe scanning bus usb@ffb40000 for devices... 2 USB Device(s) found scanning usb for storage devices... 1 Storage Device(s) found
Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com
--- v2: - Change from constant delay to poll for reset status. --- drivers/usb/host/dwc2.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index b9c56f763b..1bc41990ee 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -1132,6 +1132,7 @@ int _submit_int_msg(struct dwc2_priv *priv, struct usb_device *dev, static int dwc2_reset(struct udevice *dev) { int ret; + int i = 1000; struct dwc2_priv *priv = dev_get_priv(dev);
ret = reset_get_bulk(dev, &priv->resets); @@ -1153,7 +1154,14 @@ static int dwc2_reset(struct udevice *dev) return ret; }
- return 0; + /* Poll until reset is completed. */ + do { + ret = reset_status(&priv->resets.resets[0]); + if (!ret) + return 0; + } while (i--); + + return -ETIMEDOUT; }
static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv)

On 12/24/19 9:23 AM, Ley Foon Tan wrote:
In Cyclone 5 SoC platform, the first USB probing is failed but second probing is success. DWC2 driver read gsnpsid register right after de-assert reset, but controller is not ready yet and it returns gsnpsid 0. Polling reset status after de-assert reset to solve the issue.
Retry with this fix more than 10 times without issue.
=> usb reset resetting USB... Bus usb@ffb40000: usb probe SNPSID invalid (not DWC2 OTG device): 00000000 Port not available. => usb reset resetting USB... Bus usb@ffb40000: usb probe scanning bus usb@ffb40000 for devices... 2 USB Device(s) found scanning usb for storage devices... 1 Storage Device(s) found
Signed-off-by: Ley Foon Tan ley.foon.tan@intel.com
v2:
- Change from constant delay to poll for reset status.
drivers/usb/host/dwc2.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index b9c56f763b..1bc41990ee 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -1132,6 +1132,7 @@ int _submit_int_msg(struct dwc2_priv *priv, struct usb_device *dev, static int dwc2_reset(struct udevice *dev) { int ret;
int i = 1000; struct dwc2_priv *priv = dev_get_priv(dev);
ret = reset_get_bulk(dev, &priv->resets);
@@ -1153,7 +1154,14 @@ static int dwc2_reset(struct udevice *dev) return ret; }
- return 0;
- /* Poll until reset is completed. */
- do {
ret = reset_status(&priv->resets.resets[0]);
if (!ret)
return 0;
- } while (i--);
- return -ETIMEDOUT;
[...]
Can't we implement some sort of "reset_wait_clear()" API instead of open-coding this in the driver ?
Or rather, shouldn't the socfpga_reset_deassert wait until the reset was actually de-asserted ? This might be even better, I think it should.
participants (2)
-
Ley Foon Tan
-
Marek Vasut