[PATCH] usb: onboard-hub: Don't disable regulator in remove() callback

[Marek Vasut]

On 4/18/23 11:14, Patrice CHOTARD wrote:
> Hi Marek

Hello Patrice,

> On 4/18/23 10:25, Marek Vasut wrote:
>> On 4/18/23 09:21, Patrice CHOTARD wrote:
>>> Hi Marek
>>
>> Hello Patrice,
>>
>>> On 4/17/23 19:00, Marek Vasut wrote:
>>>> On 4/17/23 18:56, Patrice Chotard wrote:
>>>>> In case USB hub regulator is shared, unexpected behavior occurs.
>>>>> On stm32mp135f-dk, stm32mp157c-ev1 and stm32mp157x-dkx, regulator
>>>>> v3v3 is shared between several IP/devices (USB, panel, ethernet phy,
>>>>> camera, ...).
>>>>> Running command "usb stop", v3v3 regulator is switched off and
>>>>> the splashscreen content disappear.
>>>>>
>>>>> v3v3 shouldn't be disabled on usb_onboard_hub_remove() callback.
>>>>
>>>> Isn't the regulator enable/disable refcounted ?
>>>
>>> There is no refcount on regulator that's why we let regulator enable.
>>
>> Can we add that, instead of hacking around the missing refcount in every driver ?
> 
> I agree, it should a nice solution but, there is a but ;-)
>   _ it's the same situation for clocks and resets, there is no ref counter.

Those should indeed be fixed too. Hayashi-san from Socionext ran into 
the same problem recently IIRC.

>   _ what about side effects by adding ref counter where some platforms does
>     several enable on the same regulator (or even clock or reset) and only one
>     disable on it thinking that it's off ? or introduce new regulator_shared_xxx() API
>     to avoid side-effect with existing code.

Such platforms should be fixed if they misuse the regulators.

>   _ unfortunately, currently, i didn't get enough bandwidth to address these points :-(.

I also don't want to perpetuate these workarounds, can you find someone 
to add the refcounting ? It shouldn't be that hard, probably a new 
regulator struct member and ++/-- in enable/disable , maybe with some 
underflow check here and there.