Hi John, Am Freitag, den 22.04.2016, 20:31 -0700 schrieb John Youn: > On 4/20/2016 2:31 PM, [email protected] wrote: > > From: Dinh Nguyen <[email protected]> > > > > Allow for platforms that have a reset controller driver in place to bring > > the USB IP out of reset. > > > > Signed-off-by: Dinh Nguyen <[email protected]> > > --- > > v6: fix 80 line checkpatch warning in dev_err print > > v5: updated error conditions for not finding the reset property > > v4: use dev_dbg() if not a -EPROBE_DEFER > > v3: fix compile error > > v2: move to lowlevel_hw_init() > > --- > > drivers/usb/dwc2/core.h | 1 + > > drivers/usb/dwc2/platform.c | 21 +++++++++++++++++++++ > > 2 files changed, 22 insertions(+) > > > > diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h > > index 3c58d63..f748132 100644 > > --- a/drivers/usb/dwc2/core.h > > +++ b/drivers/usb/dwc2/core.h > > @@ -837,6 +837,7 @@ struct dwc2_hsotg { > > void *priv; > > int irq; > > struct clk *clk; > > + struct reset_control *reset; > > > > unsigned int queuing_high_bandwidth:1; > > unsigned int srp_success:1; > > diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c > > index 88629be..fa2c097 100644 > > --- a/drivers/usb/dwc2/platform.c > > +++ b/drivers/usb/dwc2/platform.c > > @@ -45,6 +45,7 @@ > > #include <linux/platform_device.h> > > #include <linux/phy/phy.h> > > #include <linux/platform_data/s3c-hsotg.h> > > +#include <linux/reset.h> > > > > #include <linux/usb/of.h> > > > > @@ -337,6 +338,23 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg > > *hsotg) > > { > > int i, ret; > > > > + hsotg->reset = devm_reset_control_get(hsotg->dev, "dwc2"); > > + if (IS_ERR(hsotg->reset)) { > > + ret = PTR_ERR(hsotg->reset); > > + switch (ret) { > > + case -ENOENT: > > + hsotg->reset = NULL; > > + break; > > + default: > > + dev_err(hsotg->dev, "error getting reset control %d\n", > > + ret); > > + return ret; > > + } > > + } > > + > > + if (hsotg->reset) > > + reset_control_deassert(hsotg->reset); > > + > > /* Set default UTMI width */ > > hsotg->phyif = GUSBCFG_PHYIF16; > > > > @@ -434,6 +452,9 @@ static int dwc2_driver_remove(struct platform_device > > *dev) > > if (hsotg->ll_hw_enabled) > > dwc2_lowlevel_hw_disable(hsotg); > > > > + if (hsotg->reset) > > + reset_control_assert(hsotg->reset); > > + > > return 0; > > } > > > > > > Hi Dinh, > > On my system, with no reset controller configured, this fails > devm_reset_control_get() with -EINVAL, and it also spits out a > backtrace. > > Hi Philipp, > > I think devm_reset_control_get() should return either NULL or some > other error to indicate that the reset controller is not > configured. That way we can check it and handle that condition. If we > check for -EINVAL, we might miss a legitimate error that is returning > -EINVAL. > > Also I think the WARN_ON(1) should be removed for this case. > > Any thoughts on this or other suggestions to resolve it?
It is invalid to call (devm_)reset_control_get if the framework is disabled, so that error code is correct. The only reason there are even stubs for (devm_)reset_control_get is for compile tests. Either enable CONFIG_RESET_CONTROLLER or use (devm_)reset_control_get_optional. regards Philipp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
