Hi John,
Am Freitag, den 22.04.2016, 20:31 -0700 schrieb John Youn:
> On 4/20/2016 2:31 PM, dinguyen@xxxxxxxxxxxxxxxxxxxxx wrote:
> > From: Dinh Nguyen <dinguyen@xxxxxxxxxxxxxxxxxxxxx>
> >
> > Allow for platforms that have a reset controller driver in place to bring
> > the USB IP out of reset.
> >
> > Signed-off-by: Dinh Nguyen <dinguyen@xxxxxxxxxxxxxxxxxxxxx>
> > ---
> > 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
