[PATCH] staging: dwc2: add check on dwc2_core_reset return
If the GRSTCTL_CSFTRST self-clearing bit never comes
back to 0 for any reason, the controller is under reset
state and cannot be used. It's preferable to abort
initialization in such case.
Signed-off-by: Julien Delacou
---
drivers/staging/dwc2/core.c | 58 +--
1 file changed, 45 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/dwc2/core.c b/drivers/staging/dwc2/core.c
index 27e1964..ecce1ef 100644
--- a/drivers/staging/dwc2/core.c
+++ b/drivers/staging/dwc2/core.c
@@ -116,7 +116,7 @@ static void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg
*hsotg)
* Do core a soft reset of the core. Be careful with this because it
* resets all the internal state machines of the core.
*/
-static void dwc2_core_reset(struct dwc2_hsotg *hsotg)
+static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
{
u32 greset;
int count = 0;
@@ -131,7 +131,7 @@ static void dwc2_core_reset(struct dwc2_hsotg *hsotg)
dev_warn(hsotg->dev,
"%s() HANG! AHB Idle GRSTCTL=%0x\n",
__func__, greset);
- return;
+ return -EBUSY;
}
} while (!(greset & GRSTCTL_AHBIDLE));
@@ -146,7 +146,7 @@ static void dwc2_core_reset(struct dwc2_hsotg *hsotg)
dev_warn(hsotg->dev,
"%s() HANG! Soft Reset GRSTCTL=%0x\n",
__func__, greset);
- break;
+ return -EBUSY;
}
} while (greset & GRSTCTL_CSFTRST);
@@ -155,11 +155,14 @@ static void dwc2_core_reset(struct dwc2_hsotg *hsotg)
* not stay in host mode after a connector ID change!
*/
usleep_range(15, 20);
+
+ return 0;
}
-static void dwc2_fs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
+static int dwc2_fs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
{
u32 usbcfg, i2cctl;
+ int retval = 0;
/*
* core_init() is now called on every switch so only call the
@@ -172,7 +175,12 @@ static void dwc2_fs_phy_init(struct dwc2_hsotg *hsotg,
bool select_phy)
writel(usbcfg, hsotg->regs + GUSBCFG);
/* Reset after a PHY select */
- dwc2_core_reset(hsotg);
+ retval = dwc2_core_reset(hsotg);
+ if (retval) {
+ dev_err(hsotg->dev, "%s() Reset failed, aborting",
+ __func__);
+ return retval;
+ }
}
/*
@@ -200,14 +208,17 @@ static void dwc2_fs_phy_init(struct dwc2_hsotg *hsotg,
bool select_phy)
i2cctl |= GI2CCTL_I2CEN;
writel(i2cctl, hsotg->regs + GI2CCTL);
}
+
+ return retval;
}
-static void dwc2_hs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
+static int dwc2_hs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
{
u32 usbcfg;
+ int retval = 0;
if (!select_phy)
- return;
+ return -ENODEV;
usbcfg = readl(hsotg->regs + GUSBCFG);
@@ -240,20 +251,32 @@ static void dwc2_hs_phy_init(struct dwc2_hsotg *hsotg,
bool select_phy)
writel(usbcfg, hsotg->regs + GUSBCFG);
/* Reset after setting the PHY parameters */
- dwc2_core_reset(hsotg);
+ retval = dwc2_core_reset(hsotg);
+ if (retval) {
+ dev_err(hsotg->dev, "%s() Reset failed, aborting",
+ __func__);
+ return retval;
+ }
+
+ return retval;
}
-static void dwc2_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
+static int dwc2_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
{
u32 usbcfg;
+ int retval = 0;
if (hsotg->core_params->speed == DWC2_SPEED_PARAM_FULL &&
hsotg->core_params->phy_type == DWC2_PHY_TYPE_PARAM_FS) {
/* If FS mode with FS PHY */
- dwc2_fs_phy_init(hsotg, select_phy);
+ retval = dwc2_fs_phy_init(hsotg, select_phy);
+ if (retval)
+ return retval;
} else {
/* High speed PHY */
- dwc2_hs_phy_init(hsotg, select_phy);
+ retval = dwc2_hs_phy_init(hsotg, select_phy);
+ if (retval)
+ return retval;
}
if (hsotg->hw_params.hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI &&
@@ -270,6 +293,8 @@ static void dwc2_phy_init(struct dwc2_hsotg *hsotg, bool
select_phy)
usbcfg &= ~GUSBCFG_ULPI_CLK_SUSP_M;
writel(usbcfg, hsotg->regs + GUSBCFG);
}
+
+ return retval;
}
static int dwc2_gahbcfg_init(struct dwc2_hsotg *hsotg)
@@ -421,12 +446,19
[PATCH] staging: dwc2: add check on dwc2_core_reset return
If the GRSTCTL_CSFTRST self-clearing bit never comes
back to 0 for any reason, the controller is under reset
state and cannot be used. It's preferable to abort
initialization in such case.
Signed-off-by: Julien Delacou julien.dela...@st.com
---
drivers/staging/dwc2/core.c | 58 +--
1 file changed, 45 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/dwc2/core.c b/drivers/staging/dwc2/core.c
index 27e1964..ecce1ef 100644
--- a/drivers/staging/dwc2/core.c
+++ b/drivers/staging/dwc2/core.c
@@ -116,7 +116,7 @@ static void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg
*hsotg)
* Do core a soft reset of the core. Be careful with this because it
* resets all the internal state machines of the core.
*/
-static void dwc2_core_reset(struct dwc2_hsotg *hsotg)
+static int dwc2_core_reset(struct dwc2_hsotg *hsotg)
{
u32 greset;
int count = 0;
@@ -131,7 +131,7 @@ static void dwc2_core_reset(struct dwc2_hsotg *hsotg)
dev_warn(hsotg-dev,
%s() HANG! AHB Idle GRSTCTL=%0x\n,
__func__, greset);
- return;
+ return -EBUSY;
}
} while (!(greset GRSTCTL_AHBIDLE));
@@ -146,7 +146,7 @@ static void dwc2_core_reset(struct dwc2_hsotg *hsotg)
dev_warn(hsotg-dev,
%s() HANG! Soft Reset GRSTCTL=%0x\n,
__func__, greset);
- break;
+ return -EBUSY;
}
} while (greset GRSTCTL_CSFTRST);
@@ -155,11 +155,14 @@ static void dwc2_core_reset(struct dwc2_hsotg *hsotg)
* not stay in host mode after a connector ID change!
*/
usleep_range(15, 20);
+
+ return 0;
}
-static void dwc2_fs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
+static int dwc2_fs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
{
u32 usbcfg, i2cctl;
+ int retval = 0;
/*
* core_init() is now called on every switch so only call the
@@ -172,7 +175,12 @@ static void dwc2_fs_phy_init(struct dwc2_hsotg *hsotg,
bool select_phy)
writel(usbcfg, hsotg-regs + GUSBCFG);
/* Reset after a PHY select */
- dwc2_core_reset(hsotg);
+ retval = dwc2_core_reset(hsotg);
+ if (retval) {
+ dev_err(hsotg-dev, %s() Reset failed, aborting,
+ __func__);
+ return retval;
+ }
}
/*
@@ -200,14 +208,17 @@ static void dwc2_fs_phy_init(struct dwc2_hsotg *hsotg,
bool select_phy)
i2cctl |= GI2CCTL_I2CEN;
writel(i2cctl, hsotg-regs + GI2CCTL);
}
+
+ return retval;
}
-static void dwc2_hs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
+static int dwc2_hs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
{
u32 usbcfg;
+ int retval = 0;
if (!select_phy)
- return;
+ return -ENODEV;
usbcfg = readl(hsotg-regs + GUSBCFG);
@@ -240,20 +251,32 @@ static void dwc2_hs_phy_init(struct dwc2_hsotg *hsotg,
bool select_phy)
writel(usbcfg, hsotg-regs + GUSBCFG);
/* Reset after setting the PHY parameters */
- dwc2_core_reset(hsotg);
+ retval = dwc2_core_reset(hsotg);
+ if (retval) {
+ dev_err(hsotg-dev, %s() Reset failed, aborting,
+ __func__);
+ return retval;
+ }
+
+ return retval;
}
-static void dwc2_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
+static int dwc2_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
{
u32 usbcfg;
+ int retval = 0;
if (hsotg-core_params-speed == DWC2_SPEED_PARAM_FULL
hsotg-core_params-phy_type == DWC2_PHY_TYPE_PARAM_FS) {
/* If FS mode with FS PHY */
- dwc2_fs_phy_init(hsotg, select_phy);
+ retval = dwc2_fs_phy_init(hsotg, select_phy);
+ if (retval)
+ return retval;
} else {
/* High speed PHY */
- dwc2_hs_phy_init(hsotg, select_phy);
+ retval = dwc2_hs_phy_init(hsotg, select_phy);
+ if (retval)
+ return retval;
}
if (hsotg-hw_params.hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI
@@ -270,6 +293,8 @@ static void dwc2_phy_init(struct dwc2_hsotg *hsotg, bool
select_phy)
usbcfg = ~GUSBCFG_ULPI_CLK_SUSP_M;
writel(usbcfg, hsotg-regs + GUSBCFG);
}
+
+ return retval;
}
static int dwc2_gahbcfg_init(struct dwc2_hsotg *hsotg)
@@ -421,12 +446,19 @@ int dwc2_core_init(struct dwc2_hsotg *hsotg, bool
select_phy, int irq)
writel(usbcfg, hsotg-regs + GUSBCFG
Re: [PATCH] staging: dwc2: do not clear pending interrupts twice
On 11/15/2013 11:55 AM, Dan Carpenter wrote: > On Fri, Nov 15, 2013 at 11:39:38AM +0100, Julien DELACOU wrote: >> Pending interrupts clearing is done in dwc2_enable_common_interrupts >> so we don't need to do it twice. >> > Are there any user visible effects to this bug? How did you spot it? > > regards, > dan carpenter > > Honestly, there is not. I am working on another issue using that controller and noticed it during reviewing the global behavior. Best regards, Julien -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] staging: dwc2: do not clear pending interrupts twice
Pending interrupts clearing is done in dwc2_enable_common_interrupts so we don't need to do it twice. Signed-off-by: Julien Delacou --- drivers/staging/dwc2/core.c |3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/dwc2/core.c b/drivers/staging/dwc2/core.c index c8ff668..8374ec3 100644 --- a/drivers/staging/dwc2/core.c +++ b/drivers/staging/dwc2/core.c @@ -533,9 +533,6 @@ void dwc2_enable_host_interrupts(struct dwc2_hsotg *hsotg) writel(0, hsotg->regs + GINTMSK); writel(0, hsotg->regs + HAINTMSK); - /* Clear any pending interrupts */ - writel(0x, hsotg->regs + GINTSTS); - /* Enable the common interrupts */ dwc2_enable_common_interrupts(hsotg); -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] staging: dwc2: do not clear pending interrupts twice
Pending interrupts clearing is done in dwc2_enable_common_interrupts so we don't need to do it twice. Signed-off-by: Julien Delacou julien.dela...@st.com --- drivers/staging/dwc2/core.c |3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/staging/dwc2/core.c b/drivers/staging/dwc2/core.c index c8ff668..8374ec3 100644 --- a/drivers/staging/dwc2/core.c +++ b/drivers/staging/dwc2/core.c @@ -533,9 +533,6 @@ void dwc2_enable_host_interrupts(struct dwc2_hsotg *hsotg) writel(0, hsotg-regs + GINTMSK); writel(0, hsotg-regs + HAINTMSK); - /* Clear any pending interrupts */ - writel(0x, hsotg-regs + GINTSTS); - /* Enable the common interrupts */ dwc2_enable_common_interrupts(hsotg); -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: dwc2: do not clear pending interrupts twice
On 11/15/2013 11:55 AM, Dan Carpenter wrote: On Fri, Nov 15, 2013 at 11:39:38AM +0100, Julien DELACOU wrote: Pending interrupts clearing is done in dwc2_enable_common_interrupts so we don't need to do it twice. Are there any user visible effects to this bug? How did you spot it? regards, dan carpenter Honestly, there is not. I am working on another issue using that controller and noticed it during reviewing the global behavior. Best regards, Julien -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] staging: dwc2: fix value used in dwc2_set_all_params
From: Julien Delacou This fix uses 'value' parameter as it should be instead of hardcoded -1. Signed-off-by: Julien Delacou Acked-by: Paul Zimmerman --- drivers/staging/dwc2/hcd.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/dwc2/hcd.c b/drivers/staging/dwc2/hcd.c index 2ed54b1..ca38aaa 100644 --- a/drivers/staging/dwc2/hcd.c +++ b/drivers/staging/dwc2/hcd.c @@ -2690,7 +2690,7 @@ void dwc2_set_all_params(struct dwc2_core_params *params, int value) int i; for (i = 0; i < size; i++) - p[i] = -1; + p[i] = value; } EXPORT_SYMBOL_GPL(dwc2_set_all_params); -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] staging: dwc2: fix value used in dwc2_set_all_params
From: Julien Delacou julien.dela...@stericsson.com This fix uses 'value' parameter as it should be instead of hardcoded -1. Signed-off-by: Julien Delacou julien.dela...@stericsson.com Acked-by: Paul Zimmerman pa...@synopys.com --- drivers/staging/dwc2/hcd.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/dwc2/hcd.c b/drivers/staging/dwc2/hcd.c index 2ed54b1..ca38aaa 100644 --- a/drivers/staging/dwc2/hcd.c +++ b/drivers/staging/dwc2/hcd.c @@ -2690,7 +2690,7 @@ void dwc2_set_all_params(struct dwc2_core_params *params, int value) int i; for (i = 0; i size; i++) - p[i] = -1; + p[i] = value; } EXPORT_SYMBOL_GPL(dwc2_set_all_params); -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinctrl: add sleep mode management for hogs
On 12/12/2012 08:31 PM, Linus Walleij wrote: On Tue, Dec 11, 2012 at 10:11 PM, Stephen Warren wrote: Since I assume those function are only supposed to be used by pinctrl drivers themselves, should the prototypes go into drivers/pinctrl/core.h or similar, rather than something in include/linux/pinctrl? Good point, Julien can you fix this? Sure, I will. Hm, maybe pinctrl_force_sleep()/default() isn't such a good name, maybe it should be pinctrl_hogs_sleep()/pinctrl_hogs_default() intead? Oh, and don't you need EXPORT_SYMBOL in case the pinctrl driver is a module? True. I will do it too. Aside from that, Reviewed-by: Stephen Warren Thanks. Yours, Linus Walleij Thanks a lot. Regards, Julien -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] pinctrl: add sleep mode management for hogs
On 12/12/2012 08:31 PM, Linus Walleij wrote: On Tue, Dec 11, 2012 at 10:11 PM, Stephen Warrenswar...@wwwdotorg.org wrote: Since I assume those function are only supposed to be used by pinctrl drivers themselves, should the prototypes go into drivers/pinctrl/core.h or similar, rather than something in include/linux/pinctrl? Good point, Julien can you fix this? Sure, I will. Hm, maybe pinctrl_force_sleep()/default() isn't such a good name, maybe it should be pinctrl_hogs_sleep()/pinctrl_hogs_default() intead? Oh, and don't you need EXPORT_SYMBOL in case the pinctrl driver is a module? True. I will do it too. Aside from that, Reviewed-by: Stephen Warrenswar...@nvidia.com Thanks. Yours, Linus Walleij Thanks a lot. Regards, Julien -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
