On 04/14/2016 01:23 PM, John Youn wrote:
> 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?
Sure, I can make the change.
Dinh
--
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
