[PATCH v2] usb: dwc3: core: Fix timeout check
dwc3_core_init loops 'timeout' times to check if the IP block is out of reset using 'while (timeout--)'. If there is some issue and the block doesn't come out of reset, the loop will run till 'timeout' becomes zero and the post decrement operator would set timeout to 0xffffffff. Though the IP block is not out reset, the subsequent if check 'if !timeout' would fail as timeout is not equal to zero and the function proceeds with the initialization.
Use poll API instead to resolve this.
Tested-By: Varadarajan Narayanan quic_varada@quicinc.com Signed-off-by: Varadarajan Narayanan quic_varada@quicinc.com --- v2: * Use poll API and remove the timeout variable based loop as suggested by 'Marek Vasut' * Drop 'Reviewed-by: Neil Armstrong' as the patch has taken a very different shape --- drivers/usb/dwc3/core.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index a35b8c2f64..847fa1f82c 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -23,6 +23,7 @@ #include <linux/delay.h> #include <linux/dma-mapping.h> #include <linux/err.h> +#include <linux/iopoll.h> #include <linux/ioport.h> #include <dm.h> #include <generic-phy.h> @@ -587,7 +588,6 @@ static void dwc3_set_incr_burst_type(struct dwc3 *dwc) */ static int dwc3_core_init(struct dwc3 *dwc) { - unsigned long timeout; u32 hwparams4 = dwc->hwparams.hwparams4; u32 reg; int ret; @@ -610,15 +610,11 @@ static int dwc3_core_init(struct dwc3 *dwc) }
/* issue device SoftReset too */ - timeout = 5000; dwc3_writel(dwc->regs, DWC3_DCTL, DWC3_DCTL_CSFTRST); - while (timeout--) { - reg = dwc3_readl(dwc->regs, DWC3_DCTL); - if (!(reg & DWC3_DCTL_CSFTRST)) - break; - }; - - if (!timeout) { + ret = read_poll_timeout(dwc3_readl, reg, + !(reg & DWC3_DCTL_CSFTRST), + 1, 5000, dwc->regs, DWC3_DCTL); + if (ret) { dev_err(dwc->dev, "Reset Timed Out\n"); ret = -ETIMEDOUT; goto err0;
Hi Varadarajan,
Thank you for the patch.
On mer., janv. 15, 2025 at 11:50, Varadarajan Narayanan quic_varada@quicinc.com wrote:
dwc3_core_init loops 'timeout' times to check if the IP block is out of reset using 'while (timeout--)'. If there is some issue and the block doesn't come out of reset, the loop will run till 'timeout' becomes zero and the post decrement operator would set timeout to 0xffffffff. Though the IP block is not out reset, the subsequent if check 'if !timeout' would fail as timeout is not equal to zero and the function proceeds with the initialization.
Use poll API instead to resolve this.
Tested-By: Varadarajan Narayanan quic_varada@quicinc.com
s/Tested-By/Tested-by
See ./scripts/checkpatch.pl:
λ ~/work/upstream/src/u-boot/ varadarajan/dwc3-timeout ./scripts/checkpatch.pl --u-boot --git HEAD^..HEAD WARNING: 'Tested-by:' is the preferred signature form #16: Tested-By: Varadarajan Narayanan quic_varada@quicinc.com
total: 0 errors, 1 warnings, 0 checks, 33 lines checked
Signed-off-by: Varadarajan Narayanan quic_varada@quicinc.com
I think Marek can fix-up that when he applies, but if you send a v3 to correct this, feel free to add:
Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com
Thanks Mattijs
v2: * Use poll API and remove the timeout variable based loop as suggested by 'Marek Vasut' * Drop 'Reviewed-by: Neil Armstrong' as the patch has taken a very different shape
drivers/usb/dwc3/core.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index a35b8c2f64..847fa1f82c 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -23,6 +23,7 @@ #include <linux/delay.h> #include <linux/dma-mapping.h> #include <linux/err.h> +#include <linux/iopoll.h> #include <linux/ioport.h> #include <dm.h> #include <generic-phy.h> @@ -587,7 +588,6 @@ static void dwc3_set_incr_burst_type(struct dwc3 *dwc) */ static int dwc3_core_init(struct dwc3 *dwc) {
- unsigned long timeout; u32 hwparams4 = dwc->hwparams.hwparams4; u32 reg; int ret;
@@ -610,15 +610,11 @@ static int dwc3_core_init(struct dwc3 *dwc) }
/* issue device SoftReset too */
- timeout = 5000; dwc3_writel(dwc->regs, DWC3_DCTL, DWC3_DCTL_CSFTRST);
- while (timeout--) {
reg = dwc3_readl(dwc->regs, DWC3_DCTL);if (!(reg & DWC3_DCTL_CSFTRST))break;- };
- if (!timeout) {
- ret = read_poll_timeout(dwc3_readl, reg,
!(reg & DWC3_DCTL_CSFTRST),1, 5000, dwc->regs, DWC3_DCTL);- if (ret) { dev_err(dwc->dev, "Reset Timed Out\n"); ret = -ETIMEDOUT; goto err0;
-- 2.34.1
On 1/16/25 10:31 AM, Mattijs Korpershoek wrote:
Hi Varadarajan,
Thank you for the patch.
On mer., janv. 15, 2025 at 11:50, Varadarajan Narayanan quic_varada@quicinc.com wrote:
dwc3_core_init loops 'timeout' times to check if the IP block is out of reset using 'while (timeout--)'. If there is some issue and the block doesn't come out of reset, the loop will run till 'timeout' becomes zero and the post decrement operator would set timeout to 0xffffffff. Though the IP block is not out reset, the subsequent if check 'if !timeout' would fail as timeout is not equal to zero and the function proceeds with the initialization.
Use poll API instead to resolve this.
Tested-By: Varadarajan Narayanan quic_varada@quicinc.com
s/Tested-By/Tested-by
See ./scripts/checkpatch.pl:
λ ~/work/upstream/src/u-boot/ varadarajan/dwc3-timeout ./scripts/checkpatch.pl --u-boot --git HEAD^..HEAD WARNING: 'Tested-by:' is the preferred signature form #16: Tested-By: Varadarajan Narayanan quic_varada@quicinc.com
total: 0 errors, 1 warnings, 0 checks, 33 lines checked
Signed-off-by: Varadarajan Narayanan quic_varada@quicinc.com
I think Marek can fix-up that when he applies, but if you send a v3 to correct this, feel free to add:
TB by author is a bit odd, so I dropped both TBs and kept RBs only.
On 15/01/2025 07:20, Varadarajan Narayanan wrote:
dwc3_core_init loops 'timeout' times to check if the IP block is out of reset using 'while (timeout--)'. If there is some issue and the block doesn't come out of reset, the loop will run till 'timeout' becomes zero and the post decrement operator would set timeout to 0xffffffff. Though the IP block is not out reset, the subsequent if check 'if !timeout' would fail as timeout is not equal to zero and the function proceeds with the initialization.
Nice catch, this is a nasty one.
Use poll API instead to resolve this.
Tested-By: Varadarajan Narayanan quic_varada@quicinc.com Signed-off-by: Varadarajan Narayanan quic_varada@quicinc.com
Reviewed-by: Caleb Connolly caleb.connolly@linaro.org
v2: * Use poll API and remove the timeout variable based loop as suggested by 'Marek Vasut' * Drop 'Reviewed-by: Neil Armstrong' as the patch has taken a very different shape
drivers/usb/dwc3/core.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index a35b8c2f64..847fa1f82c 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -23,6 +23,7 @@ #include <linux/delay.h> #include <linux/dma-mapping.h> #include <linux/err.h> +#include <linux/iopoll.h> #include <linux/ioport.h> #include <dm.h> #include <generic-phy.h> @@ -587,7 +588,6 @@ static void dwc3_set_incr_burst_type(struct dwc3 *dwc) */ static int dwc3_core_init(struct dwc3 *dwc) {
- unsigned long timeout; u32 hwparams4 = dwc->hwparams.hwparams4; u32 reg; int ret;
@@ -610,15 +610,11 @@ static int dwc3_core_init(struct dwc3 *dwc) }
/* issue device SoftReset too */
- timeout = 5000; dwc3_writel(dwc->regs, DWC3_DCTL, DWC3_DCTL_CSFTRST);
- while (timeout--) {
reg = dwc3_readl(dwc->regs, DWC3_DCTL);if (!(reg & DWC3_DCTL_CSFTRST))break;- };
- if (!timeout) {
- ret = read_poll_timeout(dwc3_readl, reg,
!(reg & DWC3_DCTL_CSFTRST),1, 5000, dwc->regs, DWC3_DCTL);- if (ret) { dev_err(dwc->dev, "Reset Timed Out\n"); ret = -ETIMEDOUT; goto err0;
participants (4)
-
Caleb Connolly -
Marek Vasut -
Mattijs Korpershoek -
Varadarajan Narayanan