Hi Alan,

On 2016/4/25 22:43, Alan Stern wrote:
> On Sat, 23 Apr 2016, Jiancheng Xue wrote:
> 
>> Some generic-ehci compatible controllers have more than one reset signal
>> lines, e.g., Synopsys DWC USB2.0 Host-AHB Controller has two resets bus_reset
>> and roothub_reset. Two more resets are added in this patch in order for this
>> kind of controller to use this driver directly.
>>
>> Signed-off-by: Jiancheng Xue <xuejianch...@hisilicon.com>
> 
> 
>> +    for (rst = 0; rst < EHCI_MAX_RSTS; rst++) {
>> +            priv->rsts[rst] = of_reset_control_get_by_index(
>> +                                    dev->dev.of_node, rst);
>> +            if (IS_ERR(priv->rsts[rst])) {
>> +                    err = PTR_ERR(priv->rsts[rst]);
>> +                    if (err == -EPROBE_DEFER)
>> +                            goto err_reset;
>> +                    priv->rsts[rst] = NULL;
>> +                    break;
>> +            }
>> +
>> +            err = reset_control_deassert(priv->rsts[rst]);
>>              if (err)
>> -                    goto err_put_clks;
>> +                    goto err_reset;
> 
> If an error occurs here...
> 
Sorry. It's really a problem. I'll modify it in v2 like below:

err = reset_control_deassert(priv->rsts[rst]);
if(err) {
        reset_control_put(priv->rsts[rst]);
        goto err_reset;
}

>>      }
>>  
>>      if (pdata->big_endian_desc)
>> @@ -300,8 +305,10 @@ err_power:
>>      if (pdata->power_off)
>>              pdata->power_off(dev);
>>  err_reset:
>> -    if (priv->rst)
>> -            reset_control_assert(priv->rst);
>> +    while (--rst >= 0) {
>> +            reset_control_assert(priv->rsts[rst]);
>> +            reset_control_put(priv->rsts[rst]);
>> +    }
> 
> You won't call reset_control_put() for the offending reset line.
> 
See above.

Is it OK except this bug? If so, I'll send out v2 patch.

Thank you very much!

Regards.
Jiancheng


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to