On 4/13/2016 7:04 PM, Arnd Bergmann wrote: > On Thursday 14 April 2016, dinguyen@xxxxxxxxxxxxxxxxxxxxx wrote: >> @@ -337,6 +338,17 @@ 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)) { >> + if (PTR_ERR(hsotg->reset) == -EPROBE_DEFER) >> + return -EPROBE_DEFER; >> + dev_dbg(hsotg->dev, "Could not get reset control.\n"); >> + hsotg->reset = NULL; >> + } >> + >> + if(hsotg->reset) >> + reset_control_deassert(hsotg->reset); > > > The error handling seems a bit odd here. If there is a failure to get the > reset control and it's actually needed, I would argue the init function should > not continue. Conversely, if there was no reset line specified in the > device, why even print a message about it? Yes it's optional. I think this needs to be checked like the PHY's are checked in the same function. That is, by checking for specific error codes that indicate a reset control is not specified and continue only in that case. All other errors should be returned. Dinh, care to make these changes? As for the message, it's only purpose is to aid debugging. John -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html