[PATCH 2/5] usb: dwc2: Use regulator_set_enable_if_allowed

[Marek Vasut]

On 8/11/23 21:53, Jonas Karlman wrote:
> On 2023-08-11 20:59, Marek Vasut wrote:
>> On 7/19/23 23:20, Jonas Karlman wrote:
>>> With the commit 4fcba5d556b4 ("regulator: implement basic reference
>>> counter") the return value of regulator_set_enable may be EALREADY or
>>> EBUSY for fixed/gpio regulators.
>>>
>>> Change to use the more relaxed regulator_set_enable_if_allowed to
>>> continue if regulator already was enabled or disabled.
>>
>> Sigh, can you please send series per-subsystem, so respective
>> maintainers can pick the whole series relevant to the subsystem ?
> 
> I have no idea on different subsystems, I use get_maintainer.pl to send
> relevant patches to the listed maintainers. This series fixes an issue
> in the same way, just different drivers affected, so a single series
> make much sense to me.

And that is why the USB patches were ignored, they were burried 
somewhere in the middle of this series. If you were to send out ADC / 
USB / MMC as separate series, with matching adc/usb/mmc Subject tags, 
they would not be ignored.

>> If it wasn't for Tom, I would've missed this patch entirely, since I
>> only saw 1/5 which was some ADC patch I have no interest in.
>>
>>> diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
>>> index 9818f9be94e0..637eb2dd06f5 100644
>>> --- a/drivers/usb/host/dwc2.c
>>> +++ b/drivers/usb/host/dwc2.c
>>> @@ -194,8 +194,8 @@ static int dwc_vbus_supply_init(struct udevice *dev)
>>>    		return 0;
>>>    	}
>>>    
>>> -	ret = regulator_set_enable(priv->vbus_supply, true);
>>> -	if (ret) {
>>> +	ret = regulator_set_enable_if_allowed(priv->vbus_supply, true);
>>> +	if (ret && ret != -ENOSYS) {
>>>    		dev_err(dev, "Error enabling vbus supply\n");
>>>    		return ret;
>>>    	}
>>> @@ -208,12 +208,10 @@ static int dwc_vbus_supply_exit(struct udevice *dev)
>>>    	struct dwc2_priv *priv = dev_get_priv(dev);
>>>    	int ret;
>>>    
>>> -	if (priv->vbus_supply) {
>>> -		ret = regulator_set_enable(priv->vbus_supply, false);
>>> -		if (ret) {
>>> -			dev_err(dev, "Error disabling vbus supply\n");
>>> -			return ret;
>>> -		}
>>> +	ret = regulator_set_enable_if_allowed(priv->vbus_supply, false);
>>> +	if (ret && ret != -ENOSYS) {
>>
>> Is this going to work if priv->vbus_supply is NULL ?
>> (I think it won't work, so you need to re-add the check)
> 
> It does, please see the cover letter, I made a note of it there.
> 
> """
> The regulator_set_enable_if_allowed function is more relaxed and will
> return ENOSYS if the provided regulator is NULL or when DM_REGULATOR
> was disabled. Using the following call convention should be safe:

Thank you for clarifying.

Reviewed-by: Marek Vasut <marex at denx.de>