[PATCH] usb: ehci-mx6: replace regulator_set_enable with *_if_allowed

Marek Vasut marex at denx.de
Thu May 4 13:56:36 CEST 2023


On 5/4/23 13:51, Eugen Hristev wrote:
> On 5/4/23 14:41, Marek Vasut wrote:
>> On 5/4/23 10:12, Eugen Hristev wrote:
>>> On 5/4/23 00:52, Marek Vasut wrote:
>>>> On 5/2/23 18:13, Tim Harvey wrote:
>>>>> On Mon, May 1, 2023 at 11:51 PM Eugen Hristev
>>>>> <eugen.hristev at collabora.com> wrote:
>>>>>>
>>>>>> regulator_set_enable_if_allowed already handles cases when the
>>>>>> regulator is already enabled, or already disabled, and does not
>>>>>> treat these as errors.
>>>>>> With this change, the driver can work correctly even if the regulator
>>>>>> is already taken or already disabled by another consumer.
>>>>>>
>>>>>> Signed-off-by: Eugen Hristev <eugen.hristev at collabora.com>
>>>>>> ---
>>>>>> Hi Tim,
>>>>>>
>>>>>> I have not tested this as I do not have a mx6 board. can you please
>>>>>> try it ?
>>>>>>
>>>>>> Thanks,
>>>>>> Eugen
>>>>>
>>>>> Eugen,
>>>>>
>>>>> This does resolve the issue that occurs after your refcount series 
>>>>> [1].
>>>>>
>>>>> However after thinking about this more and seeing Marek's responses
>>>>> this wasn't an issue of multiple consumers sharing the same regulator.
>>>>> Instead this particular issue was that the vbus regulator is getting
>>>>> enabled twice without being disabled.
>>>
>>> This kind of issues can be uncovered when we add the refcounter :)
>>
>> Yeah, and that's a good thing. The refcounter is much appreciated (am 
>> I repeating myself?)
>>
>>> On one hand I am happy to uncover such bugs.
>>> However, this leads me to wonder, even without a refcount, even 
>>> without my series, enabling a regulator that is already enabled 
>>> should return -EALREADY
>>
>> I don't that so, you can have regulators which supply multiple things 
>> and which are claimed by multiple driver instances, at which point 
>> they will each enable the regulator (refcount++) and that shouldn't 
>> end up returning -EALREADY . Right ?
> 
> That was my thought. And I was returning success if regulator was 
> already enabled.
> Then Simon asked me to return error codes on each path (review on the v2 
> of the patches):
> 
> https://marc.info/?l=u-boot&m=168012024923488&w=2

I would argue the regulator API should behave the same way as Linux one, 
to avoid confusing people developing on both. Simon ?


More information about the U-Boot mailing list